From 2dd2ef3c49bb9a52ac87e79f1a836f1e1abfd79f Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Tue, 12 Jan 2021 21:38:41 +0000 Subject: [PATCH] - Isolate OSLogin service account derivation from google metadata server to OSLogin step only --- builder/googlecompute/driver.go | 3 -- builder/googlecompute/driver_gce.go | 36 ------------------- builder/googlecompute/driver_mock.go | 6 ---- .../step_import_os_login_ssh_key.go | 31 +++++++++++++++- .../step_import_os_login_ssh_key_test.go | 34 ------------------ 5 files changed, 30 insertions(+), 80 deletions(-) diff --git a/builder/googlecompute/driver.go b/builder/googlecompute/driver.go index c31b88a79..0c8afbda5 100644 --- a/builder/googlecompute/driver.go +++ b/builder/googlecompute/driver.go @@ -70,9 +70,6 @@ type Driver interface { // DeleteOSLoginSSHKey deletes the SSH public key for OSLogin with the given key. DeleteOSLoginSSHKey(user, fingerprint string) error - // If packer is running on a GCE, derives the user from it for use with OSLogin. - GetOSLoginUserFromGCE() string - // Add to the instance metadata for the existing instance AddToInstanceMetadata(zone string, name string, metadata map[string]string) error } diff --git a/builder/googlecompute/driver_gce.go b/builder/googlecompute/driver_gce.go index 081c9d451..dfd1c9c06 100644 --- a/builder/googlecompute/driver_gce.go +++ b/builder/googlecompute/driver_gce.go @@ -10,11 +10,9 @@ import ( "errors" "fmt" "log" - "net/http" "strings" "time" - metadata "cloud.google.com/go/compute/metadata" compute "google.golang.org/api/compute/v1" "google.golang.org/api/option" oslogin "google.golang.org/api/oslogin/v1" @@ -34,7 +32,6 @@ import ( type driverGCE struct { projectId string service *compute.Service - thisGCEUser string osLoginService *oslogin.Service ui packersdk.Ui } @@ -147,8 +144,6 @@ func NewDriverGCE(config GCEDriverConfig) (Driver, error) { return nil, err } - thisGCEUser := getGCEUser() - log.Printf("[INFO] Instantiating OS Login client...") osLoginService, err := oslogin.NewService(context.TODO(), opts) if err != nil { @@ -161,7 +156,6 @@ func NewDriverGCE(config GCEDriverConfig) (Driver, error) { return &driverGCE{ projectId: config.ProjectId, service: service, - thisGCEUser: thisGCEUser, osLoginService: osLoginService, ui: config.Ui, }, nil @@ -635,10 +629,6 @@ func (d *driverGCE) getPasswordResponses(zone, instance string) ([]windowsPasswo return passwordResponses, nil } -func (d *driverGCE) GetOSLoginUserFromGCE() string { - return d.thisGCEUser -} - func (d *driverGCE) ImportOSLoginSSHKey(user, sshPublicKey string) (*oslogin.LoginProfile, error) { parent := fmt.Sprintf("users/%s", user) @@ -790,29 +780,3 @@ func (d *driverGCE) AddToInstanceMetadata(zone string, name string, metadata map return nil } - -// getGCEUser determines if we're running packer on a GCE, and if we are, gets the associated service account email for subsequent use with OSLogin. -// There are cases where we are running on a GCE, but the GCP metadata server isn't accessible. GitLab docker-engine runners are an edge case example of this. -// It makes little sense to run packer on GCP in this way, however, we defensively timeout in those cases, rather than abort. -func getGCEUser() string { - - metadataCheckTimeout := 5 * time.Second - metadataCheckChl := make(chan string, 1) - - go func() { - if metadata.OnGCE() { - log.Printf("[INFO] Attempt to capture the GCE service account for use with OSLogin...") - GCEUser, _ := metadata.NewClient(&http.Client{}).Email("") - metadataCheckChl <- GCEUser - } - }() - - select { - case thisGCEUser := <-metadataCheckChl: - log.Printf("[INFO] GCE service account %s will be used for OSLogin", thisGCEUser) - return thisGCEUser - case <-time.After(metadataCheckTimeout): - log.Printf("[INFO] Timeout after %s whilst waiting for google metadata server.", metadataCheckTimeout) - return "" - } -} diff --git a/builder/googlecompute/driver_mock.go b/builder/googlecompute/driver_mock.go index 17f9799bd..76b1fcee5 100644 --- a/builder/googlecompute/driver_mock.go +++ b/builder/googlecompute/driver_mock.go @@ -94,8 +94,6 @@ type DriverMock struct { AddToInstanceMetadataKVPairs map[string]string AddToInstanceMetadataErrCh <-chan error AddToInstanceMetadataErr error - - OSLoginUserFromGCE string } func (d *DriverMock) CreateImage(name, description, family, zone, disk string, image_labels map[string]string, image_licenses []string, image_encryption_key *compute.CustomerEncryptionKey, imageStorageLocations []string) (<-chan *Image, <-chan error) { @@ -310,7 +308,3 @@ func (d *DriverMock) AddToInstanceMetadata(zone string, name string, metadata ma return nil } - -func (d *DriverMock) GetOSLoginUserFromGCE() string { - return d.OSLoginUserFromGCE -} diff --git a/builder/googlecompute/step_import_os_login_ssh_key.go b/builder/googlecompute/step_import_os_login_ssh_key.go index 155691fd0..16894c057 100644 --- a/builder/googlecompute/step_import_os_login_ssh_key.go +++ b/builder/googlecompute/step_import_os_login_ssh_key.go @@ -5,7 +5,11 @@ import ( "crypto/sha256" "encoding/hex" "fmt" + "log" + "net/http" + "time" + metadata "cloud.google.com/go/compute/metadata" "github.com/hashicorp/packer-plugin-sdk/multistep" packersdk "github.com/hashicorp/packer-plugin-sdk/packer" "google.golang.org/api/oauth2/v2" @@ -37,7 +41,7 @@ func (s *StepImportOSLoginSSHKey) Run(ctx context.Context, state multistep.State } // Are we running packer on a GCE ? - s.accountEmail = driver.GetOSLoginUserFromGCE() + s.accountEmail = getGCEUser() if s.TokeninfoFunc == nil && s.accountEmail == "" { s.TokeninfoFunc = tokeninfo @@ -140,3 +144,28 @@ func tokeninfo(ctx context.Context) (*oauth2.Tokeninfo, error) { return svc.Tokeninfo().Context(ctx).Do() } + +// getGCEUser determines if we're running packer on a GCE, and if we are, gets the associated service account email for subsequent use with OSLogin. +// There are cases where we are running on a GCE, but the GCP metadata server isn't accessible. GitLab docker-engine runners are an edge case example of this. +// It makes little sense to run packer on GCP in this way, however, we defensively timeout in those cases, rather than abort. +func getGCEUser() string { + + metadataCheckTimeout := 5 * time.Second + metadataCheckChl := make(chan string, 1) + + go func() { + if metadata.OnGCE() { + GCEUser, _ := metadata.NewClient(&http.Client{}).Email("") + metadataCheckChl <- GCEUser + } + }() + + select { + case thisGCEUser := <-metadataCheckChl: + log.Printf("[INFO] OSLogin: GCE service account %s will be used for identity", thisGCEUser) + return thisGCEUser + case <-time.After(metadataCheckTimeout): + log.Printf("[INFO] OSLogin: Could not derive a GCE service account from google metadata server after %s", metadataCheckTimeout) + return "" + } +} diff --git a/builder/googlecompute/step_import_os_login_ssh_key_test.go b/builder/googlecompute/step_import_os_login_ssh_key_test.go index 525879ad4..a8dbae215 100644 --- a/builder/googlecompute/step_import_os_login_ssh_key_test.go +++ b/builder/googlecompute/step_import_os_login_ssh_key_test.go @@ -150,37 +150,3 @@ func TestStepImportOSLoginSSHKey_withPrivateSSHKey(t *testing.T) { t.Errorf("expected to not see a public key when using a dedicated private key, but got %q", pubKey) } } - -func TestStepImportOSLoginSSHKey_onGCE(t *testing.T) { - - state := testState(t) - d := state.Get("driver").(*DriverMock) - step := new(StepImportOSLoginSSHKey) - defer step.Cleanup(state) - - fakeAccountEmail := "testing@packer.io" - - config := state.Get("config").(*Config) - config.UseOSLogin = true - config.Comm.SSHPublicKey = []byte{'k', 'e', 'y'} - - d.OSLoginUserFromGCE = fakeAccountEmail - - if action := step.Run(context.Background(), state); action != multistep.ActionContinue { - t.Fatalf("bad action: %#v", action) - } - - if step.accountEmail != fakeAccountEmail { - t.Fatalf("expected accountEmail to be %q but got %q", fakeAccountEmail, step.accountEmail) - } - - pubKey, ok := state.GetOk("ssh_key_public_sha256") - if !ok { - t.Fatal("expected to see a public key") - } - - sha256sum := sha256.Sum256(config.Comm.SSHPublicKey) - if pubKey != hex.EncodeToString(sha256sum[:]) { - t.Errorf("expected to see a matching public key, but got %q", pubKey) - } -}