From f525e884edce9bcd2dc6f7707ec24e15f64b1cc7 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Sun, 10 Jan 2021 18:29:37 +0000 Subject: [PATCH 1/3] Timeout on metadata server --- builder/googlecompute/driver_gce.go | 37 ++++++++++++++++++++++------- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/builder/googlecompute/driver_gce.go b/builder/googlecompute/driver_gce.go index ef2e5601b..c587f702a 100644 --- a/builder/googlecompute/driver_gce.go +++ b/builder/googlecompute/driver_gce.go @@ -149,14 +149,7 @@ func NewDriverGCE(config GCEDriverConfig) (Driver, error) { return nil, err } - if metadata.OnGCE() { - log.Printf("[INFO] On GCE, capture service account for OSLogin...") - thisGCEUser, err = metadata.NewClient(&http.Client{}).Email("") - - if err != nil { - return nil, err - } - } + thisGCEUser = getGCEUser() log.Printf("[INFO] Instantiating OS Login client...") osLoginService, err := oslogin.NewService(context.TODO(), opts) @@ -799,3 +792,31 @@ 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 { + + var thisGCEUser 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: + fmt.Printf("[INFO] GCE service account %s will be used for OSLogin", thisGCEUser) + case <-time.After(metadataCheckTimeout): + fmt.Printf("[INFO] Timeout after %s whilst waiting for google metadata server.", metadataCheckTimeout) + } + + return thisGCEUser +} From 707d2d8236cb4dbeafdb8a8eedcbb85a1d9b8542 Mon Sep 17 00:00:00 2001 From: seventieskid Date: Mon, 11 Jan 2021 23:04:36 +0000 Subject: [PATCH 2/3] Refined logging and return scope --- builder/googlecompute/driver_gce.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/builder/googlecompute/driver_gce.go b/builder/googlecompute/driver_gce.go index c587f702a..081c9d451 100644 --- a/builder/googlecompute/driver_gce.go +++ b/builder/googlecompute/driver_gce.go @@ -136,8 +136,6 @@ func NewClientOptionGoogle(account *ServiceAccount, vaultOauth string, impersona func NewDriverGCE(config GCEDriverConfig) (Driver, error) { - var thisGCEUser string - opts, err := NewClientOptionGoogle(config.Account, config.VaultOauthEngineName, config.ImpersonateServiceAccountName) if err != nil { return nil, err @@ -149,7 +147,7 @@ func NewDriverGCE(config GCEDriverConfig) (Driver, error) { return nil, err } - thisGCEUser = getGCEUser() + thisGCEUser := getGCEUser() log.Printf("[INFO] Instantiating OS Login client...") osLoginService, err := oslogin.NewService(context.TODO(), opts) @@ -798,8 +796,6 @@ func (d *driverGCE) AddToInstanceMetadata(zone string, name string, metadata map // 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 { - var thisGCEUser string - metadataCheckTimeout := 5 * time.Second metadataCheckChl := make(chan string, 1) @@ -813,10 +809,10 @@ func getGCEUser() string { select { case thisGCEUser := <-metadataCheckChl: - fmt.Printf("[INFO] GCE service account %s will be used for OSLogin", thisGCEUser) + log.Printf("[INFO] GCE service account %s will be used for OSLogin", thisGCEUser) + return thisGCEUser case <-time.After(metadataCheckTimeout): - fmt.Printf("[INFO] Timeout after %s whilst waiting for google metadata server.", metadataCheckTimeout) + log.Printf("[INFO] Timeout after %s whilst waiting for google metadata server.", metadataCheckTimeout) + return "" } - - return thisGCEUser } From 2dd2ef3c49bb9a52ac87e79f1a836f1e1abfd79f Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Tue, 12 Jan 2021 21:38:41 +0000 Subject: [PATCH 3/3] - 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) - } -}