From 0dfb3cc56f45ca546b2ae4443c4b4c7da05fa67f Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Tue, 2 Jul 2019 16:16:13 -0700 Subject: [PATCH 1/2] replace some bespoke google auth code with code from golang's oauth2 library --- builder/googlecompute/account.go | 44 ++++++------------- builder/googlecompute/builder.go | 2 +- builder/googlecompute/config.go | 7 ++- builder/googlecompute/driver_gce.go | 21 +++------ .../step_create_windows_password.go | 2 +- .../googlecompute-export/post-processor.go | 11 +++-- .../googlecompute-import/post-processor.go | 9 ++-- 7 files changed, 40 insertions(+), 56 deletions(-) diff --git a/builder/googlecompute/account.go b/builder/googlecompute/account.go index 75734e279..c8cb3fc13 100644 --- a/builder/googlecompute/account.go +++ b/builder/googlecompute/account.go @@ -1,52 +1,34 @@ package googlecompute import ( - "encoding/json" "fmt" "io/ioutil" "os" - "strings" + + "golang.org/x/oauth2/google" + "golang.org/x/oauth2/jwt" ) -// accountFile represents the structure of the account file JSON file. -type AccountFile struct { - PrivateKeyId string `json:"private_key_id"` - PrivateKey string `json:"private_key"` - ClientEmail string `json:"client_email"` - ClientId string `json:"client_id"` -} - -func parseJSON(result interface{}, text string) error { - r := strings.NewReader(text) - dec := json.NewDecoder(r) - return dec.Decode(result) -} - -func ProcessAccountFile(account_file *AccountFile, text string) error { +func ProcessAccountFile(text string) (*jwt.Config, error) { // Assume text is a JSON string - if err := parseJSON(account_file, text); err != nil { + conf, err := google.JWTConfigFromJSON([]byte(text), DriverScopes...) + if err != nil { // If text was not JSON, assume it is a file path instead if _, err := os.Stat(text); os.IsNotExist(err) { - return fmt.Errorf( + return nil, fmt.Errorf( "account_file path does not exist: %s", text) } - - b, err := ioutil.ReadFile(text) + data, err := ioutil.ReadFile(text) if err != nil { - return fmt.Errorf( + return nil, fmt.Errorf( "Error reading account_file from path '%s': %s", text, err) } - - contents := string(b) - - if err := parseJSON(account_file, contents); err != nil { - return fmt.Errorf( - "Error parsing account file '%s': %s", - contents, err) + conf, err = google.JWTConfigFromJSON(data, DriverScopes...) + if err != nil { + return nil, fmt.Errorf("Error parsing account_file: %s", err) } } - - return nil + return conf, nil } diff --git a/builder/googlecompute/builder.go b/builder/googlecompute/builder.go index 9a2b4fecc..9e38c6dc7 100644 --- a/builder/googlecompute/builder.go +++ b/builder/googlecompute/builder.go @@ -36,7 +36,7 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { // representing a GCE machine image. func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (packer.Artifact, error) { driver, err := NewDriverGCE( - ui, b.config.ProjectId, &b.config.Account) + ui, b.config.ProjectId, b.config.Account) if err != nil { return nil, err } diff --git a/builder/googlecompute/config.go b/builder/googlecompute/config.go index 7c78a93cd..add586483 100644 --- a/builder/googlecompute/config.go +++ b/builder/googlecompute/config.go @@ -13,6 +13,7 @@ import ( "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/template/interpolate" + "golang.org/x/oauth2/jwt" compute "google.golang.org/api/compute/v1" ) @@ -65,7 +66,7 @@ type Config struct { MetadataFiles map[string]string `mapstructure:"metadata_files"` Zone string `mapstructure:"zone"` - Account AccountFile + Account *jwt.Config stateTimeout time.Duration imageAlreadyExists bool ctx interpolate.Context @@ -209,9 +210,11 @@ func NewConfig(raws ...interface{}) (*Config, []string, error) { } if c.AccountFile != "" { - if err := ProcessAccountFile(&c.Account, c.AccountFile); err != nil { + cfg, err := ProcessAccountFile(c.AccountFile) + if err != nil { errs = packer.MultiErrorAppend(errs, err) } + c.Account = cfg } if c.OmitExternalIP && c.Address != "" { diff --git a/builder/googlecompute/driver_gce.go b/builder/googlecompute/driver_gce.go index 400ec9c02..f2adaa591 100644 --- a/builder/googlecompute/driver_gce.go +++ b/builder/googlecompute/driver_gce.go @@ -35,24 +35,17 @@ type driverGCE struct { var DriverScopes = []string{"https://www.googleapis.com/auth/compute", "https://www.googleapis.com/auth/devstorage.full_control"} -func NewClientGCE(a *AccountFile) (*http.Client, error) { +func NewClientGCE(conf *jwt.Config) (*http.Client, error) { var err error var client *http.Client // Auth with AccountFile first if provided - if a.PrivateKey != "" { - log.Printf("[INFO] Requesting Google token via AccountFile...") - log.Printf("[INFO] -- Email: %s", a.ClientEmail) + if len(conf.PrivateKey) > 0 { + log.Printf("[INFO] Requesting Google token via account_file...") + log.Printf("[INFO] -- Email: %s", conf.Email) log.Printf("[INFO] -- Scopes: %s", DriverScopes) - log.Printf("[INFO] -- Private Key Length: %d", len(a.PrivateKey)) - - conf := jwt.Config{ - Email: a.ClientEmail, - PrivateKey: []byte(a.PrivateKey), - Scopes: DriverScopes, - TokenURL: "https://accounts.google.com/o/oauth2/token", - } + log.Printf("[INFO] -- Private Key Length: %d", len(conf.PrivateKey)) // Initiate an http.Client. The following GET request will be // authorized and authenticated on the behalf of @@ -82,8 +75,8 @@ func NewClientGCE(a *AccountFile) (*http.Client, error) { return client, nil } -func NewDriverGCE(ui packer.Ui, p string, a *AccountFile) (Driver, error) { - client, err := NewClientGCE(a) +func NewDriverGCE(ui packer.Ui, p string, conf *jwt.Config) (Driver, error) { + client, err := NewClientGCE(conf) if err != nil { return nil, err } diff --git a/builder/googlecompute/step_create_windows_password.go b/builder/googlecompute/step_create_windows_password.go index fda606245..2fe8595ff 100644 --- a/builder/googlecompute/step_create_windows_password.go +++ b/builder/googlecompute/step_create_windows_password.go @@ -60,7 +60,7 @@ func (s *StepCreateWindowsPassword) Run(ctx context.Context, state multistep.Sta UserName: c.Comm.WinRMUser, Modulus: base64.StdEncoding.EncodeToString(priv.N.Bytes()), Exponent: base64.StdEncoding.EncodeToString(buf[1:]), - Email: c.Account.ClientEmail, + Email: c.Account.Email, ExpireOn: time.Now().Add(time.Minute * 5), } diff --git a/post-processor/googlecompute-export/post-processor.go b/post-processor/googlecompute-export/post-processor.go index b0e15e296..b5ddc8906 100644 --- a/post-processor/googlecompute-export/post-processor.go +++ b/post-processor/googlecompute-export/post-processor.go @@ -11,6 +11,7 @@ import ( "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/template/interpolate" + "golang.org/x/oauth2/jwt" ) type Config struct { @@ -26,7 +27,7 @@ type Config struct { Subnetwork string `mapstructure:"subnetwork"` Zone string `mapstructure:"zone"` - Account googlecompute.AccountFile + Account *jwt.Config ctx interpolate.Context } @@ -96,16 +97,18 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact // Set up credentials for GCE driver. if builderAccountFile != "" { - err := googlecompute.ProcessAccountFile(&p.config.Account, builderAccountFile) + cfg, err := googlecompute.ProcessAccountFile(builderAccountFile) if err != nil { return nil, false, false, err } + p.config.Account = cfg } if p.config.AccountFile != "" { - err := googlecompute.ProcessAccountFile(&p.config.Account, p.config.AccountFile) + cfg, err := googlecompute.ProcessAccountFile(p.config.AccountFile) if err != nil { return nil, false, false, err } + p.config.Account = cfg } // Set up exporter instance configuration. @@ -139,7 +142,7 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact } exporterConfig.CalcTimeout() - driver, err := googlecompute.NewDriverGCE(ui, builderProjectId, &p.config.Account) + driver, err := googlecompute.NewDriverGCE(ui, builderProjectId, p.config.Account) if err != nil { return nil, false, false, err } diff --git a/post-processor/googlecompute-import/post-processor.go b/post-processor/googlecompute-import/post-processor.go index 7cef5f899..32073f993 100644 --- a/post-processor/googlecompute-import/post-processor.go +++ b/post-processor/googlecompute-import/post-processor.go @@ -8,6 +8,7 @@ import ( "strings" "time" + "golang.org/x/oauth2/jwt" "google.golang.org/api/compute/v1" "google.golang.org/api/storage/v1" @@ -34,7 +35,7 @@ type Config struct { ImageName string `mapstructure:"image_name"` SkipClean bool `mapstructure:"skip_clean"` - Account googlecompute.AccountFile + Account *jwt.Config ctx interpolate.Context } @@ -70,9 +71,11 @@ func (p *PostProcessor) Configure(raws ...interface{}) error { } if p.config.AccountFile != "" { - if err := googlecompute.ProcessAccountFile(&p.config.Account, p.config.AccountFile); err != nil { + cfg, err := googlecompute.ProcessAccountFile(p.config.AccountFile) + if err != nil { errs = packer.MultiErrorAppend(errs, err) } + p.config.Account = cfg } templates := map[string]*string{ @@ -95,7 +98,7 @@ func (p *PostProcessor) Configure(raws ...interface{}) error { } func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact packer.Artifact) (packer.Artifact, bool, bool, error) { - client, err := googlecompute.NewClientGCE(&p.config.Account) + client, err := googlecompute.NewClientGCE(p.config.Account) if err != nil { return nil, false, false, err } From 617fd18255e6f17d6980586da4f520b31d523310 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 3 Jul 2019 15:58:08 -0700 Subject: [PATCH 2/2] supply complete dummy data for googlecompute tests --- builder/googlecompute/config_test.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/builder/googlecompute/config_test.go b/builder/googlecompute/config_test.go index f562d4b8f..f247775d6 100644 --- a/builder/googlecompute/config_test.go +++ b/builder/googlecompute/config_test.go @@ -500,6 +500,16 @@ func testMetadataFile(t *testing.T) string { return tf.Name() } -// This is just some dummy data that doesn't actually work (it was revoked -// a long time ago). -const testAccountContent = `{}` +// This is just some dummy data that doesn't actually work +const testAccountContent = `{ + "type": "service_account", + "project_id": "test-project-123456789", + "private_key_id": "bananaphone", + "private_key": "-----BEGIN PRIVATE KEY-----\nring_ring_ring_ring_ring_ring_ring_BANANAPHONE\n-----END PRIVATE KEY-----\n", + "client_email": "raffi-compute@developer.gserviceaccount.com", + "client_id": "1234567890", + "auth_uri": "https://accounts.google.com/o/oauth2/auth", + "token_uri": "https://accounts.google.com/o/oauth2/token", + "auth_provider_x509_cert_url": "https://www.googleapis.com/oauth2/v1/certs", + "client_x509_cert_url": "https://www.googleapis.com/robot/v1/metadata/x509/12345-compute%40developer.gserviceaccount.com" +}`