From e6073bcec7eb57b908eed04cb5a3c7b74612a43d Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Tue, 14 Apr 2020 10:28:07 -0700 Subject: [PATCH 01/23] implement iap proxy for googlecompute. ssh-only so far --- builder/googlecompute/account.go | 7 +- builder/googlecompute/builder.go | 8 +- builder/googlecompute/config.go | 22 +- builder/googlecompute/config.hcl2spec.go | 8 + builder/googlecompute/driver_gce.go | 18 +- builder/googlecompute/step_start_tunnel.go | 260 ++++++++++++++++++ .../step_start_tunnel.hcl2spec.go | 36 +++ .../googlecompute-export/post-processor.go | 7 +- .../post-processor.hcl2spec.go | 2 + .../googlecompute-import/post-processor.go | 5 +- .../post-processor.hcl2spec.go | 2 + website/pages/docs/builders/googlecompute.mdx | 2 + .../googlecompute/IAPConfig-not-required.mdx | 25 ++ .../builder/googlecompute/IAPConfig.mdx | 2 + 14 files changed, 388 insertions(+), 16 deletions(-) create mode 100644 builder/googlecompute/step_start_tunnel.go create mode 100644 builder/googlecompute/step_start_tunnel.hcl2spec.go create mode 100644 website/pages/partials/builder/googlecompute/IAPConfig-not-required.mdx create mode 100644 website/pages/partials/builder/googlecompute/IAPConfig.mdx diff --git a/builder/googlecompute/account.go b/builder/googlecompute/account.go index c8cb3fc13..e2d630732 100644 --- a/builder/googlecompute/account.go +++ b/builder/googlecompute/account.go @@ -9,9 +9,10 @@ import ( "golang.org/x/oauth2/jwt" ) -func ProcessAccountFile(text string) (*jwt.Config, error) { +func ProcessAccountFile(text string, iap bool) (*jwt.Config, error) { + driverScopes := getDriverScopes(iap) // Assume text is a JSON string - conf, err := google.JWTConfigFromJSON([]byte(text), DriverScopes...) + 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) { @@ -25,7 +26,7 @@ func ProcessAccountFile(text string) (*jwt.Config, error) { "Error reading account_file from path '%s': %s", text, err) } - conf, err = google.JWTConfigFromJSON(data, DriverScopes...) + conf, err = google.JWTConfigFromJSON(data, driverScopes...) if err != nil { return nil, fmt.Errorf("Error parsing account_file: %s", err) } diff --git a/builder/googlecompute/builder.go b/builder/googlecompute/builder.go index 47167a43f..1257e59a4 100644 --- a/builder/googlecompute/builder.go +++ b/builder/googlecompute/builder.go @@ -37,7 +37,8 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, []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, b.config.VaultGCPOauthEngine) + ui, b.config.ProjectId, b.config.account, b.config.VaultGCPOauthEngine, + b.config.IAP) if err != nil { return nil, err } @@ -66,6 +67,11 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack &StepInstanceInfo{ Debug: b.config.PackerDebug, }, + &StepStartTunnel{ + IAPConf: &b.config.IAPConfig, + CommConf: &b.config.Comm, + AccountFile: b.config.AccountFile, + }, &communicator.StepConnect{ Config: &b.config.Comm, Host: communicator.CommHost(b.config.Comm.Host(), "instance_ip"), diff --git a/builder/googlecompute/config.go b/builder/googlecompute/config.go index abe8e4531..195d487c1 100644 --- a/builder/googlecompute/config.go +++ b/builder/googlecompute/config.go @@ -72,6 +72,8 @@ type Config struct { // state of your VM instances. Note: integrity monitoring relies on having // vTPM enabled. [Details](https://cloud.google.com/security/shielded-cloud/shielded-vm) EnableIntegrityMonitoring bool `mapstructure:"enable_integrity_monitoring" required:"false"` + // Whether to use an IAP proxy. + IAPConfig `mapstructure:",squash"` // The unique name of the resulting image. Defaults to // `packer-{{timestamp}}`. ImageName string `mapstructure:"image_name" required:"false"` @@ -320,10 +322,28 @@ func (c *Config) Prepare(raws ...interface{}) ([]string, error) { c.StateTimeout = 5 * time.Minute } + // Set up communicator if es := c.Comm.Prepare(&c.ctx); len(es) > 0 { errs = packer.MultiErrorAppend(errs, es...) } + // set defaults for IAP + if c.IAPConfig.IAPHashBang == "" { + c.IAPConfig.IAPHashBang = "/bin/sh" + } + if c.IAPConfig.IAPExt == "" { + c.IAPConfig.IAPExt = ".sh" + } + + // Configure IAP: Update SSH config to use localhost proxy instead + if c.Comm.Type == "ssh" { + c.Comm.SSHHost = "localhost" + } else { + err := fmt.Errorf("Error: IAP tunnel currently only implemnted for" + + " SSH communicator") + errs = packer.MultiErrorAppend(errs, err) + } + // Process required parameters. if c.ProjectId == "" { errs = packer.MultiErrorAppend( @@ -359,7 +379,7 @@ func (c *Config) Prepare(raws ...interface{}) ([]string, error) { errs = packer.MultiErrorAppend(errs, fmt.Errorf("You cannot "+ "specify both account_file and vault_gcp_oauth_engine.")) } - cfg, err := ProcessAccountFile(c.AccountFile) + cfg, err := ProcessAccountFile(c.AccountFile, c.IAP) if err != nil { errs = packer.MultiErrorAppend(errs, err) } diff --git a/builder/googlecompute/config.hcl2spec.go b/builder/googlecompute/config.hcl2spec.go index 124435d7d..e000224f4 100644 --- a/builder/googlecompute/config.hcl2spec.go +++ b/builder/googlecompute/config.hcl2spec.go @@ -70,6 +70,10 @@ type FlatConfig struct { EnableSecureBoot *bool `mapstructure:"enable_secure_boot" required:"false" cty:"enable_secure_boot"` EnableVtpm *bool `mapstructure:"enable_vtpm" required:"false" cty:"enable_vtpm"` EnableIntegrityMonitoring *bool `mapstructure:"enable_integrity_monitoring" required:"false" cty:"enable_integrity_monitoring"` + IAP *bool `mapstructure:"use_iap" required:"false" cty:"use_iap"` + IAPLocalhostPort *int `mapstructure:"iap_localhost_port" cty:"iap_localhost_port"` + IAPHashBang *string `mapstructure:"iap_hashbang" required:"false" cty:"iap_hashbang"` + IAPExt *string `mapstructure:"iap_ext" required:"false" cty:"iap_ext"` ImageName *string `mapstructure:"image_name" required:"false" cty:"image_name"` ImageDescription *string `mapstructure:"image_description" required:"false" cty:"image_description"` ImageEncryptionKey *FlatCustomerEncryptionKey `mapstructure:"image_encryption_key" required:"false" cty:"image_encryption_key"` @@ -175,6 +179,10 @@ func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec { "enable_secure_boot": &hcldec.AttrSpec{Name: "enable_secure_boot", Type: cty.Bool, Required: false}, "enable_vtpm": &hcldec.AttrSpec{Name: "enable_vtpm", Type: cty.Bool, Required: false}, "enable_integrity_monitoring": &hcldec.AttrSpec{Name: "enable_integrity_monitoring", Type: cty.Bool, Required: false}, + "use_iap": &hcldec.AttrSpec{Name: "use_iap", Type: cty.Bool, Required: false}, + "iap_localhost_port": &hcldec.AttrSpec{Name: "iap_localhost_port", Type: cty.Number, Required: false}, + "iap_hashbang": &hcldec.AttrSpec{Name: "iap_hashbang", Type: cty.String, Required: false}, + "iap_ext": &hcldec.AttrSpec{Name: "iap_ext", Type: cty.String, Required: false}, "image_name": &hcldec.AttrSpec{Name: "image_name", Type: cty.String, Required: false}, "image_description": &hcldec.AttrSpec{Name: "image_description", Type: cty.String, Required: false}, "image_encryption_key": &hcldec.BlockSpec{TypeName: "image_encryption_key", Nested: hcldec.ObjectSpec((*FlatCustomerEncryptionKey)(nil).HCL2Spec())}, diff --git a/builder/googlecompute/driver_gce.go b/builder/googlecompute/driver_gce.go index f98f964c4..9209fbd17 100644 --- a/builder/googlecompute/driver_gce.go +++ b/builder/googlecompute/driver_gce.go @@ -34,7 +34,13 @@ type driverGCE struct { ui packer.Ui } -var DriverScopes = []string{"https://www.googleapis.com/auth/compute", "https://www.googleapis.com/auth/devstorage.full_control"} +func getDriverScopes(iap bool) []string { + ds := []string{"https://www.googleapis.com/auth/compute", "https://www.googleapis.com/auth/devstorage.full_control"} + // if iap { + // ds = append(ds, "https://www.googleapis.com/auth/iap.tunnelResourceAccessor") + // } + return ds +} // Define a TokenSource that gets tokens from Vault type OauthTokenSource struct { @@ -69,7 +75,7 @@ func (ots OauthTokenSource) Token() (*oauth2.Token, error) { } -func NewClientGCE(conf *jwt.Config, vaultOauth string) (*http.Client, error) { +func NewClientGCE(conf *jwt.Config, vaultOauth string, iap bool) (*http.Client, error) { var err error var client *http.Client @@ -84,7 +90,7 @@ func NewClientGCE(conf *jwt.Config, vaultOauth string) (*http.Client, error) { // Auth with AccountFile if provided 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] -- Scopes: %s", getDriverScopes(iap)) log.Printf("[INFO] -- Private Key Length: %d", len(conf.PrivateKey)) // Initiate an http.Client. The following GET request will be @@ -93,7 +99,7 @@ func NewClientGCE(conf *jwt.Config, vaultOauth string) (*http.Client, error) { client = conf.Client(context.TODO()) } else { log.Printf("[INFO] Requesting Google token via GCE API Default Client Token Source...") - client, err = google.DefaultClient(context.TODO(), DriverScopes...) + client, err = google.DefaultClient(context.TODO(), getDriverScopes(iap)...) // The DefaultClient uses the DefaultTokenSource of the google lib. // The DefaultTokenSource uses the "Application Default Credentials" // It looks for credentials in the following places, preferring the first location found: @@ -115,8 +121,8 @@ func NewClientGCE(conf *jwt.Config, vaultOauth string) (*http.Client, error) { return client, nil } -func NewDriverGCE(ui packer.Ui, p string, conf *jwt.Config, vaultOauth string) (Driver, error) { - client, err := NewClientGCE(conf, vaultOauth) +func NewDriverGCE(ui packer.Ui, p string, conf *jwt.Config, vaultOauth string, iap bool) (Driver, error) { + client, err := NewClientGCE(conf, vaultOauth, iap) if err != nil { return nil, err } diff --git a/builder/googlecompute/step_start_tunnel.go b/builder/googlecompute/step_start_tunnel.go new file mode 100644 index 000000000..4be633a48 --- /dev/null +++ b/builder/googlecompute/step_start_tunnel.go @@ -0,0 +1,260 @@ +//go:generate struct-markdown +//go:generate mapstructure-to-hcl2 -type IAPConfig + +package googlecompute + +import ( + "bufio" + "bytes" + "context" + "fmt" + "log" + "os" + "os/exec" + "strconv" + "strings" + "syscall" + "time" + + "github.com/hashicorp/packer/common/net" + "github.com/hashicorp/packer/common/retry" + "github.com/hashicorp/packer/helper/communicator" + "github.com/hashicorp/packer/helper/multistep" + "github.com/hashicorp/packer/packer" + "github.com/hashicorp/packer/packer/tmp" +) + +// StepStartTunnel represents a Packer build step that launches an IAP tunnel +type IAPConfig struct { + // Whether to use an IAP proxy. + // Prerequisites and limitations for using IAP: + // - You must manually enable the IAP API in the Google Cloud console. + // - You must have the gcloud sdk installed on the computer running Packer. + // - You must be using a Service Account with a credentials file (using the + // account_file option in the Packer template) + // - This is currently only implemented for the SSH communicator, not the + // WinRM Communicator. + // - You must add the given service account to project level IAP permissions + // in https://console.cloud.google.com/security/iap. To do so, click + // "project" > "SSH and TCP resoures" > "All Tunnel Resources" > + // "Add Member". Then add your service account and choose the role + // "IAP-secured Tunnel User" and add any conditions you may care about. + IAP bool `mapstructure:"use_iap" required:"false"` + // Which port to connect the local end of the IAM localhost proxy to. If + // left blank, Packer will choose a port for you from available ports. + IAPLocalhostPort int `mapstructure:"iap_localhost_port"` + // What "hashbang" to use to invoke script that sets up gcloud. + // Default: "/bin/sh" + IAPHashBang string `mapstructure:"iap_hashbang" required:"false"` + // What file extension to use for script that sets up gcloud. + // Default: ".sh" + IAPExt string `mapstructure:"iap_ext" required:"false"` +} + +type StepStartTunnel struct { + IAPConf *IAPConfig + CommConf *communicator.Config + AccountFile string + + ctxCancel context.CancelFunc + cmd *exec.Cmd +} + +func (s *StepStartTunnel) ConfigureLocalHostPort(ctx context.Context) error { + if s.IAPConf.IAPLocalhostPort == 0 { + log.Printf("Finding an available TCP port for IAP proxy") + l, err := net.ListenRangeConfig{ + Min: 8000, + Max: 9000, + Addr: "0.0.0.0", + Network: "tcp", + }.Listen(ctx) + + if err != nil { + err := fmt.Errorf("error finding an available port to initiate a session tunnel: %s", err) + return err + } + + s.IAPConf.IAPLocalhostPort = l.Port + l.Close() + log.Printf("Setting up proxy to listen on localhost at %d", + s.IAPConf.IAPLocalhostPort) + } + return nil +} + +func (s *StepStartTunnel) createTempGcloudScript(args []string) (string, error) { + // Generate temp script that contains both gcloud auth and gcloud compute + // iap launch call. + + // Create temp file. + tf, err := tmp.File("gcloud-setup") + if err != nil { + return "", fmt.Errorf("Error preparing gcloud setup script: %s", err) + } + defer tf.Close() + // Write our contents to it + writer := bufio.NewWriter(tf) + + s.IAPConf.IAPHashBang = fmt.Sprintf("#!%s\n", s.IAPConf.IAPHashBang) + log.Printf("[INFO] (google): Prepending inline gcloud setup script with %s", + s.IAPConf.IAPHashBang) + writer.WriteString(s.IAPConf.IAPHashBang) + + // authenticate to gcloud + _, err = writer.WriteString( + fmt.Sprintf("gcloud auth activate-service-account --key-file='%s'\n", + s.AccountFile)) + if err != nil { + return "", fmt.Errorf("Error preparing gcloud shell script: %s", err) + } + // call command + args = append([]string{"gcloud"}, args...) + argString := strings.Join(args, " ") + if _, err := writer.WriteString(argString + "\n"); err != nil { + return "", fmt.Errorf("Error preparing gcloud shell script: %s", err) + } + + if err := writer.Flush(); err != nil { + return "", fmt.Errorf("Error preparing shell script: %s", err) + } + + err = os.Chmod(tf.Name(), 0700) + if err != nil { + log.Printf("[ERROR] (google): error modifying permissions of temp script file: %s", err.Error()) + } + + // figure out what extension the file should have, and rename it. + tempScriptFileName := tf.Name() + if s.IAPConf.IAPExt != "" { + os.Rename(tempScriptFileName, fmt.Sprintf("%s%s", tempScriptFileName, s.IAPConf.IAPExt)) + tempScriptFileName = fmt.Sprintf("%s%s", tempScriptFileName, s.IAPConf.IAPExt) + } + + return tempScriptFileName, nil +} + +// Run executes the Packer build step that creates an IAP tunnel. +func (s *StepStartTunnel) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { + if !s.IAPConf.IAP { + log.Printf("Skipping step launch IAP tunnel; \"iap\" is false.") + return multistep.ActionContinue + } + + // shell out to create the tunnel. + ui := state.Get("ui").(packer.Ui) + instanceName := state.Get("instance_name").(string) + c := state.Get("config").(*Config) + + ui.Say("Step Launch IAP Tunnel...") + + err := s.ConfigureLocalHostPort(ctx) + if err != nil { + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + + // Generate list of args to use to call gcloud cli. + args := []string{"compute", "start-iap-tunnel", instanceName, + strconv.Itoa(s.CommConf.Port()), + fmt.Sprintf("--local-host-port=localhost:%d", s.IAPConf.IAPLocalhostPort), + "--zone", c.Zone, + } + + // This is the port the IAP tunnel listens on, on localhost. + // TODO make setting LocalHostPort optional + s.CommConf.SSHPort = s.IAPConf.IAPLocalhostPort + + log.Printf("Calling tunnel launch with args %#v", args) + + // Create temp file that contains both gcloud authentication, and gcloud + // proxy setup call. + tempScriptFileName, err := s.createTempGcloudScript(args) + if err != nil { + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } + defer os.Remove(tempScriptFileName) + + // Shell out to gcloud. + cancelCtx, cancel := context.WithCancel(ctx) + s.ctxCancel = cancel + + err = retry.Config{ + Tries: 11, + ShouldRetry: func(err error) bool { + // Example of error you get as the tunnel is still getting users + // configured in the cloud: + //"ERROR: (gcloud.compute.start-iap-tunnel) Error while connecting + // [4033: u'not authorized']." + return true + // if strings.Contains(err.Error(), "[4033: u'not authorized']") { + // log.Printf("Waiting for tunnel permissions to update. Retrying...") + // return true + // } + // return false + }, + RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 30 * time.Second, Multiplier: 2}).Linear, + }.Run(ctx, func(ctx context.Context) error { + // set stdout and stderr so we can read what's going on. + var stdout, stderr bytes.Buffer + + cmd := exec.CommandContext(cancelCtx, tempScriptFileName) + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + + err := cmd.Start() + log.Printf("Waiting 30s for tunnel to create...") + if err != nil { + err := fmt.Errorf("Error calling gcloud sdk to launch IAP tunnel: %s", + err) + cmd.Process.Kill() + return err + } + // Wait for tunnel to launch and gather response. TODO: do this without + // a sleep. + time.Sleep(30 * time.Second) + + // Track stdout. + sout := stdout.String() + if sout != "" { + log.Printf("[start-iap-tunnel] stdout is:") + } + + log.Printf("[start-iap-tunnel] stderr is:") + serr := stderr.String() + log.Println(serr) + if strings.Contains(serr, "ERROR") { + cmd.Process.Kill() + errIdx := strings.Index(serr, "ERROR:") + return fmt.Errorf("ERROR: %s", serr[errIdx+7:len(serr)]) + } + // Store successful command on step so we can access it to cancel it + // later. + s.cmd = cmd + return nil + }) + + return multistep.ActionContinue +} + +// Cleanup destroys the GCE instance created during the image creation process. +func (s *StepStartTunnel) Cleanup(state multistep.StateBag) { + if s.cmd != nil && s.cmd.Process != nil { + log.Printf("Cleaning up the IAP tunnel...") + // Why not just s.cmd.Process.Kill()? I'm glad you asked. The gcloud + // call spawns a python subprocess that listens on the port, and you + // need to use the process _group_ id to kill this process and its + // daemon child. We create the group ID with the syscall.SysProcAttr + // call inside the retry loop above, and then store that ID on the + // command so we can destroy it here. + syscall.Kill(-s.cmd.Process.Pid, syscall.SIGKILL) + } else { + log.Printf("Couldn't find IAP tunnel process to kill. Continuing.") + } + return +} diff --git a/builder/googlecompute/step_start_tunnel.hcl2spec.go b/builder/googlecompute/step_start_tunnel.hcl2spec.go new file mode 100644 index 000000000..ac617884a --- /dev/null +++ b/builder/googlecompute/step_start_tunnel.hcl2spec.go @@ -0,0 +1,36 @@ +// Code generated by "mapstructure-to-hcl2 -type IAPConfig"; DO NOT EDIT. +package googlecompute + +import ( + "github.com/hashicorp/hcl/v2/hcldec" + "github.com/zclconf/go-cty/cty" +) + +// FlatIAPConfig is an auto-generated flat version of IAPConfig. +// Where the contents of a field with a `mapstructure:,squash` tag are bubbled up. +type FlatIAPConfig struct { + IAP *bool `mapstructure:"use_iap" required:"false" cty:"use_iap"` + IAPLocalhostPort *int `mapstructure:"iap_localhost_port" cty:"iap_localhost_port"` + IAPHashBang *string `mapstructure:"iap_hashbang" required:"false" cty:"iap_hashbang"` + IAPExt *string `mapstructure:"iap_ext" required:"false" cty:"iap_ext"` +} + +// FlatMapstructure returns a new FlatIAPConfig. +// FlatIAPConfig is an auto-generated flat version of IAPConfig. +// Where the contents a fields with a `mapstructure:,squash` tag are bubbled up. +func (*IAPConfig) FlatMapstructure() interface{ HCL2Spec() map[string]hcldec.Spec } { + return new(FlatIAPConfig) +} + +// HCL2Spec returns the hcl spec of a IAPConfig. +// This spec is used by HCL to read the fields of IAPConfig. +// The decoded values from this spec will then be applied to a FlatIAPConfig. +func (*FlatIAPConfig) HCL2Spec() map[string]hcldec.Spec { + s := map[string]hcldec.Spec{ + "use_iap": &hcldec.AttrSpec{Name: "use_iap", Type: cty.Bool, Required: false}, + "iap_localhost_port": &hcldec.AttrSpec{Name: "iap_localhost_port", Type: cty.Number, Required: false}, + "iap_hashbang": &hcldec.AttrSpec{Name: "iap_hashbang", Type: cty.String, Required: false}, + "iap_ext": &hcldec.AttrSpec{Name: "iap_ext", Type: cty.String, Required: false}, + } + return s +} diff --git a/post-processor/googlecompute-export/post-processor.go b/post-processor/googlecompute-export/post-processor.go index 4f21e4fde..f02c99974 100644 --- a/post-processor/googlecompute-export/post-processor.go +++ b/post-processor/googlecompute-export/post-processor.go @@ -22,6 +22,7 @@ type Config struct { common.PackerConfig `mapstructure:",squash"` AccountFile string `mapstructure:"account_file"` + IAP bool `mapstructure:"iap"` DiskSizeGb int64 `mapstructure:"disk_size"` DiskType string `mapstructure:"disk_type"` @@ -111,14 +112,14 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact // Set up credentials for GCE driver. if builderAccountFile != "" { - cfg, err := googlecompute.ProcessAccountFile(builderAccountFile) + cfg, err := googlecompute.ProcessAccountFile(builderAccountFile, p.config.IAP) if err != nil { return nil, false, false, err } p.config.account = cfg } if p.config.AccountFile != "" { - cfg, err := googlecompute.ProcessAccountFile(p.config.AccountFile) + cfg, err := googlecompute.ProcessAccountFile(p.config.AccountFile, p.config.IAP) if err != nil { return nil, false, false, err } @@ -159,7 +160,7 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact } driver, err := googlecompute.NewDriverGCE(ui, builderProjectId, - p.config.account, p.config.VaultGCPOauthEngine) + p.config.account, p.config.VaultGCPOauthEngine, p.config.IAP) if err != nil { return nil, false, false, err } diff --git a/post-processor/googlecompute-export/post-processor.hcl2spec.go b/post-processor/googlecompute-export/post-processor.hcl2spec.go index ed91bcccf..156c852b8 100644 --- a/post-processor/googlecompute-export/post-processor.hcl2spec.go +++ b/post-processor/googlecompute-export/post-processor.hcl2spec.go @@ -17,6 +17,7 @@ type FlatConfig struct { PackerUserVars map[string]string `mapstructure:"packer_user_variables" cty:"packer_user_variables"` PackerSensitiveVars []string `mapstructure:"packer_sensitive_variables" cty:"packer_sensitive_variables"` AccountFile *string `mapstructure:"account_file" cty:"account_file"` + IAP *bool `mapstructure:"iap" cty:"iap"` DiskSizeGb *int64 `mapstructure:"disk_size" cty:"disk_size"` DiskType *string `mapstructure:"disk_type" cty:"disk_type"` MachineType *string `mapstructure:"machine_type" cty:"machine_type"` @@ -48,6 +49,7 @@ func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec { "packer_user_variables": &hcldec.AttrSpec{Name: "packer_user_variables", Type: cty.Map(cty.String), Required: false}, "packer_sensitive_variables": &hcldec.AttrSpec{Name: "packer_sensitive_variables", Type: cty.List(cty.String), Required: false}, "account_file": &hcldec.AttrSpec{Name: "account_file", Type: cty.String, Required: false}, + "iap": &hcldec.AttrSpec{Name: "iap", Type: cty.Bool, Required: false}, "disk_size": &hcldec.AttrSpec{Name: "disk_size", Type: cty.Number, Required: false}, "disk_type": &hcldec.AttrSpec{Name: "disk_type", Type: cty.String, Required: false}, "machine_type": &hcldec.AttrSpec{Name: "machine_type", Type: cty.String, Required: false}, diff --git a/post-processor/googlecompute-import/post-processor.go b/post-processor/googlecompute-import/post-processor.go index bca834c98..a37d7f8fc 100644 --- a/post-processor/googlecompute-import/post-processor.go +++ b/post-processor/googlecompute-import/post-processor.go @@ -28,6 +28,7 @@ type Config struct { AccountFile string `mapstructure:"account_file"` ProjectId string `mapstructure:"project_id"` + IAP bool `mapstructure:"iap"` Bucket string `mapstructure:"bucket"` GCSObjectName string `mapstructure:"gcs_object_name"` @@ -77,7 +78,7 @@ func (p *PostProcessor) Configure(raws ...interface{}) error { } if p.config.AccountFile != "" { - cfg, err := googlecompute.ProcessAccountFile(p.config.AccountFile) + cfg, err := googlecompute.ProcessAccountFile(p.config.AccountFile, p.config.IAP) if err != nil { errs = packer.MultiErrorAppend(errs, err) } @@ -117,7 +118,7 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact } p.config.ctx.Data = generatedData - client, err := googlecompute.NewClientGCE(p.config.account, p.config.VaultGCPOauthEngine) + client, err := googlecompute.NewClientGCE(p.config.account, p.config.VaultGCPOauthEngine, p.config.IAP) if err != nil { return nil, false, false, err } diff --git a/post-processor/googlecompute-import/post-processor.hcl2spec.go b/post-processor/googlecompute-import/post-processor.hcl2spec.go index 96e597813..c773769f9 100644 --- a/post-processor/googlecompute-import/post-processor.hcl2spec.go +++ b/post-processor/googlecompute-import/post-processor.hcl2spec.go @@ -18,6 +18,7 @@ type FlatConfig struct { PackerSensitiveVars []string `mapstructure:"packer_sensitive_variables" cty:"packer_sensitive_variables"` AccountFile *string `mapstructure:"account_file" cty:"account_file"` ProjectId *string `mapstructure:"project_id" cty:"project_id"` + IAP *bool `mapstructure:"iap" cty:"iap"` Bucket *string `mapstructure:"bucket" cty:"bucket"` GCSObjectName *string `mapstructure:"gcs_object_name" cty:"gcs_object_name"` ImageDescription *string `mapstructure:"image_description" cty:"image_description"` @@ -50,6 +51,7 @@ func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec { "packer_sensitive_variables": &hcldec.AttrSpec{Name: "packer_sensitive_variables", Type: cty.List(cty.String), Required: false}, "account_file": &hcldec.AttrSpec{Name: "account_file", Type: cty.String, Required: false}, "project_id": &hcldec.AttrSpec{Name: "project_id", Type: cty.String, Required: false}, + "iap": &hcldec.AttrSpec{Name: "iap", Type: cty.Bool, Required: false}, "bucket": &hcldec.AttrSpec{Name: "bucket", Type: cty.String, Required: false}, "gcs_object_name": &hcldec.AttrSpec{Name: "gcs_object_name", Type: cty.String, Required: false}, "image_description": &hcldec.AttrSpec{Name: "image_description", Type: cty.String, Required: false}, diff --git a/website/pages/docs/builders/googlecompute.mdx b/website/pages/docs/builders/googlecompute.mdx index 0bb5f8c25..5ed73746b 100644 --- a/website/pages/docs/builders/googlecompute.mdx +++ b/website/pages/docs/builders/googlecompute.mdx @@ -217,6 +217,8 @@ builder. @include 'builder/googlecompute/Config-not-required.mdx' +@include 'builder/googlecompute/IAPConfig-not-required.mdx' + ## Startup Scripts Startup scripts can be a powerful tool for configuring the instance from which diff --git a/website/pages/partials/builder/googlecompute/IAPConfig-not-required.mdx b/website/pages/partials/builder/googlecompute/IAPConfig-not-required.mdx new file mode 100644 index 000000000..28bdeb1d7 --- /dev/null +++ b/website/pages/partials/builder/googlecompute/IAPConfig-not-required.mdx @@ -0,0 +1,25 @@ + + +- `use_iap` (bool) - Whether to use an IAP proxy. + Prerequisites and limitations for using IAP: + - You must manually enable the IAP API in the Google Cloud console. + - You must have the gcloud sdk installed on the computer running Packer. + - You must be using a Service Account with a credentials file (using the + account_file option in the Packer template) + - This is currently only implemented for the SSH communicator, not the + WinRM Communicator. + - You must add the given service account to project level IAP permissions + in https://console.cloud.google.com/security/iap. To do so, click + "project" > "SSH and TCP resoures" > "All Tunnel Resources" > + "Add Member". Then add your service account and choose the role + "IAP-secured Tunnel User" and add any conditions you may care about. + +- `iap_localhost_port` (int) - Which port to connect the local end of the IAM localhost proxy to. If + left blank, Packer will choose a port for you from available ports. + +- `iap_hashbang` (string) - What "hashbang" to use to invoke script that sets up gcloud. + Default: "/bin/sh" + +- `iap_ext` (string) - What file extension to use for script that sets up gcloud. + Default: ".sh" + \ No newline at end of file diff --git a/website/pages/partials/builder/googlecompute/IAPConfig.mdx b/website/pages/partials/builder/googlecompute/IAPConfig.mdx new file mode 100644 index 000000000..b43f7ce04 --- /dev/null +++ b/website/pages/partials/builder/googlecompute/IAPConfig.mdx @@ -0,0 +1,2 @@ + +StepStartTunnel represents a Packer build step that launches an IAP tunnel From d713f7ec64bb42b268fa498e82fa9abb2ff82c0b Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 22 Apr 2020 13:59:29 -0700 Subject: [PATCH 02/23] add conditional building becasue windows support is still forthcoming --- builder/googlecompute/step_start_tunnel.go | 81 +++---------------- builder/googlecompute/tunnel_driver.go | 79 ++++++++++++++++++ .../googlecompute/tunnel_driver_windows.go | 28 +++++++ 3 files changed, 120 insertions(+), 68 deletions(-) create mode 100644 builder/googlecompute/tunnel_driver.go create mode 100644 builder/googlecompute/tunnel_driver_windows.go diff --git a/builder/googlecompute/step_start_tunnel.go b/builder/googlecompute/step_start_tunnel.go index 4be633a48..031fb06be 100644 --- a/builder/googlecompute/step_start_tunnel.go +++ b/builder/googlecompute/step_start_tunnel.go @@ -5,15 +5,12 @@ package googlecompute import ( "bufio" - "bytes" "context" "fmt" "log" "os" - "os/exec" "strconv" "strings" - "syscall" "time" "github.com/hashicorp/packer/common/net" @@ -51,13 +48,17 @@ type IAPConfig struct { IAPExt string `mapstructure:"iap_ext" required:"false"` } +type TunnelDriver interface { + StartTunnel(context.Context, string) error + StopTunnel() +} + type StepStartTunnel struct { IAPConf *IAPConfig CommConf *communicator.Config AccountFile string - ctxCancel context.CancelFunc - cmd *exec.Cmd + tunnelDriver TunnelDriver } func (s *StepStartTunnel) ConfigureLocalHostPort(ctx context.Context) error { @@ -179,82 +180,26 @@ func (s *StepStartTunnel) Run(ctx context.Context, state multistep.StateBag) mul defer os.Remove(tempScriptFileName) // Shell out to gcloud. - cancelCtx, cancel := context.WithCancel(ctx) - s.ctxCancel = cancel + s.tunnelDriver = NewTunnelDriver() err = retry.Config{ Tries: 11, ShouldRetry: func(err error) bool { - // Example of error you get as the tunnel is still getting users - // configured in the cloud: - //"ERROR: (gcloud.compute.start-iap-tunnel) Error while connecting - // [4033: u'not authorized']." + // TODO be stricter with retries. return true - // if strings.Contains(err.Error(), "[4033: u'not authorized']") { - // log.Printf("Waiting for tunnel permissions to update. Retrying...") - // return true - // } - // return false }, RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 30 * time.Second, Multiplier: 2}).Linear, }.Run(ctx, func(ctx context.Context) error { - // set stdout and stderr so we can read what's going on. - var stdout, stderr bytes.Buffer - - cmd := exec.CommandContext(cancelCtx, tempScriptFileName) - cmd.Stdout = &stdout - cmd.Stderr = &stderr - - cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - - err := cmd.Start() - log.Printf("Waiting 30s for tunnel to create...") - if err != nil { - err := fmt.Errorf("Error calling gcloud sdk to launch IAP tunnel: %s", - err) - cmd.Process.Kill() - return err - } - // Wait for tunnel to launch and gather response. TODO: do this without - // a sleep. - time.Sleep(30 * time.Second) - - // Track stdout. - sout := stdout.String() - if sout != "" { - log.Printf("[start-iap-tunnel] stdout is:") - } - - log.Printf("[start-iap-tunnel] stderr is:") - serr := stderr.String() - log.Println(serr) - if strings.Contains(serr, "ERROR") { - cmd.Process.Kill() - errIdx := strings.Index(serr, "ERROR:") - return fmt.Errorf("ERROR: %s", serr[errIdx+7:len(serr)]) - } - // Store successful command on step so we can access it to cancel it - // later. - s.cmd = cmd - return nil + // tunnel launcher/destroyer has to be different on windows vs. unix. + err := s.tunnelDriver.StartTunnel(ctx, tempScriptFileName) + return err }) return multistep.ActionContinue } -// Cleanup destroys the GCE instance created during the image creation process. +// Cleanup stops the IAP tunnel and cleans up processes. func (s *StepStartTunnel) Cleanup(state multistep.StateBag) { - if s.cmd != nil && s.cmd.Process != nil { - log.Printf("Cleaning up the IAP tunnel...") - // Why not just s.cmd.Process.Kill()? I'm glad you asked. The gcloud - // call spawns a python subprocess that listens on the port, and you - // need to use the process _group_ id to kill this process and its - // daemon child. We create the group ID with the syscall.SysProcAttr - // call inside the retry loop above, and then store that ID on the - // command so we can destroy it here. - syscall.Kill(-s.cmd.Process.Pid, syscall.SIGKILL) - } else { - log.Printf("Couldn't find IAP tunnel process to kill. Continuing.") - } + s.tunnelDriver.StopTunnel() return } diff --git a/builder/googlecompute/tunnel_driver.go b/builder/googlecompute/tunnel_driver.go new file mode 100644 index 000000000..b07bc2ef2 --- /dev/null +++ b/builder/googlecompute/tunnel_driver.go @@ -0,0 +1,79 @@ +// +build !windows + +package googlecompute + +import ( + "bytes" + "context" + "fmt" + "log" + "os/exec" + "strings" + "syscall" + "time" +) + +func NewTunnelDriver() TunnelDriver { + return &TunnelDriverLinux{} +} + +type TunnelDriverLinux struct { + cmd *exec.Cmd +} + +func (t *TunnelDriverLinux) StartTunnel(cancelCtx context.Context, tempScriptFileName string) error { + // set stdout and stderr so we can read what's going on. + var stdout, stderr bytes.Buffer + + cmd := exec.CommandContext(cancelCtx, tempScriptFileName) + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} + + err := cmd.Start() + log.Printf("Waiting 30s for tunnel to create...") + if err != nil { + err := fmt.Errorf("Error calling gcloud sdk to launch IAP tunnel: %s", + err) + cmd.Process.Kill() + return err + } + // Wait for tunnel to launch and gather response. TODO: do this without + // a sleep. + time.Sleep(30 * time.Second) + + // Track stdout. + sout := stdout.String() + if sout != "" { + log.Printf("[start-iap-tunnel] stdout is:") + } + + log.Printf("[start-iap-tunnel] stderr is:") + serr := stderr.String() + log.Println(serr) + if strings.Contains(serr, "ERROR") { + cmd.Process.Kill() + errIdx := strings.Index(serr, "ERROR:") + return fmt.Errorf("ERROR: %s", serr[errIdx+7:len(serr)]) + } + // Store successful command on step so we can access it to cancel it + // later. + t.cmd = cmd + return nil +} + +func (t *TunnelDriverLinux) StopTunnel() { + if t.cmd != nil && t.cmd.Process != nil { + log.Printf("Cleaning up the IAP tunnel...") + // Why not just s.cmd.Process.Kill()? I'm glad you asked. The gcloud + // call spawns a python subprocess that listens on the port, and you + // need to use the process _group_ id to kill this process and its + // daemon child. We create the group ID with the syscall.SysProcAttr + // call inside the retry loop above, and then store that ID on the + // command so we can destroy it here. + syscall.Kill(-t.cmd.Process.Pid, syscall.SIGKILL) + } else { + log.Printf("Couldn't find IAP tunnel process to kill. Continuing.") + } +} diff --git a/builder/googlecompute/tunnel_driver_windows.go b/builder/googlecompute/tunnel_driver_windows.go new file mode 100644 index 000000000..f96f329c4 --- /dev/null +++ b/builder/googlecompute/tunnel_driver_windows.go @@ -0,0 +1,28 @@ +// +build windows + +package googlecompute + +import ( + "bytes" + "context" + "fmt" + "log" + "os/exec" + "strings" + "syscall" + "time" +) + +func NewTunnelDriver() TunnelDriver { + return &TunnelDriverWindows{} +} + +type TunnelDriverLinux struct { + cmd *exec.Cmd +} + +func (t *TunnelDriverWindows) StartTunnel(cancelCtx context.Context, tempScriptFileName string) error { + return fmt.Errorf("Windows support for IAP tunnel not yet supported.") +} + +func (t *TunnelDriverWindows) StopTunnel() {} From eb05f6ff887d3aaff31d737e76b994227c612606 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 22 Apr 2020 14:05:38 -0700 Subject: [PATCH 03/23] fix windows --- builder/googlecompute/tunnel_driver_windows.go | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/builder/googlecompute/tunnel_driver_windows.go b/builder/googlecompute/tunnel_driver_windows.go index f96f329c4..df301e3a1 100644 --- a/builder/googlecompute/tunnel_driver_windows.go +++ b/builder/googlecompute/tunnel_driver_windows.go @@ -3,22 +3,14 @@ package googlecompute import ( - "bytes" - "context" "fmt" - "log" - "os/exec" - "strings" - "syscall" - "time" ) func NewTunnelDriver() TunnelDriver { return &TunnelDriverWindows{} } -type TunnelDriverLinux struct { - cmd *exec.Cmd +type TunnelDriverWindows struct { } func (t *TunnelDriverWindows) StartTunnel(cancelCtx context.Context, tempScriptFileName string) error { From 365eb09842a9524ea33d4c7bc1295bac7858e70e Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 22 Apr 2020 14:09:19 -0700 Subject: [PATCH 04/23] import context --- builder/googlecompute/tunnel_driver_windows.go | 1 + 1 file changed, 1 insertion(+) diff --git a/builder/googlecompute/tunnel_driver_windows.go b/builder/googlecompute/tunnel_driver_windows.go index df301e3a1..9d5031ebc 100644 --- a/builder/googlecompute/tunnel_driver_windows.go +++ b/builder/googlecompute/tunnel_driver_windows.go @@ -3,6 +3,7 @@ package googlecompute import ( + "context" "fmt" ) From 3cd28e98a7288a6787bf2fd52c1e33638e41cf47 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 22 Apr 2020 14:20:29 -0700 Subject: [PATCH 05/23] linting --- builder/googlecompute/step_start_tunnel.go | 12 ++++++++---- builder/googlecompute/tunnel_driver.go | 9 +++++---- 2 files changed, 13 insertions(+), 8 deletions(-) diff --git a/builder/googlecompute/step_start_tunnel.go b/builder/googlecompute/step_start_tunnel.go index 031fb06be..7536f705c 100644 --- a/builder/googlecompute/step_start_tunnel.go +++ b/builder/googlecompute/step_start_tunnel.go @@ -100,7 +100,10 @@ func (s *StepStartTunnel) createTempGcloudScript(args []string) (string, error) s.IAPConf.IAPHashBang = fmt.Sprintf("#!%s\n", s.IAPConf.IAPHashBang) log.Printf("[INFO] (google): Prepending inline gcloud setup script with %s", s.IAPConf.IAPHashBang) - writer.WriteString(s.IAPConf.IAPHashBang) + err = writer.WriteString(s.IAPConf.IAPHashBang) + if err != nil { + return "", fmt.Errorf("Error preparing inline hashbang: %s", err) + } // authenticate to gcloud _, err = writer.WriteString( @@ -128,7 +131,10 @@ func (s *StepStartTunnel) createTempGcloudScript(args []string) (string, error) // figure out what extension the file should have, and rename it. tempScriptFileName := tf.Name() if s.IAPConf.IAPExt != "" { - os.Rename(tempScriptFileName, fmt.Sprintf("%s%s", tempScriptFileName, s.IAPConf.IAPExt)) + err := os.Rename(tempScriptFileName, fmt.Sprintf("%s%s", tempScriptFileName, s.IAPConf.IAPExt)) + if err != nil { + return "", fmt.Errorf("Error setting the correct temp file extension: %s", err) + } tempScriptFileName = fmt.Sprintf("%s%s", tempScriptFileName, s.IAPConf.IAPExt) } @@ -179,7 +185,6 @@ func (s *StepStartTunnel) Run(ctx context.Context, state multistep.StateBag) mul } defer os.Remove(tempScriptFileName) - // Shell out to gcloud. s.tunnelDriver = NewTunnelDriver() err = retry.Config{ @@ -201,5 +206,4 @@ func (s *StepStartTunnel) Run(ctx context.Context, state multistep.StateBag) mul // Cleanup stops the IAP tunnel and cleans up processes. func (s *StepStartTunnel) Cleanup(state multistep.StateBag) { s.tunnelDriver.StopTunnel() - return } diff --git a/builder/googlecompute/tunnel_driver.go b/builder/googlecompute/tunnel_driver.go index b07bc2ef2..78d9be634 100644 --- a/builder/googlecompute/tunnel_driver.go +++ b/builder/googlecompute/tunnel_driver.go @@ -36,7 +36,6 @@ func (t *TunnelDriverLinux) StartTunnel(cancelCtx context.Context, tempScriptFil if err != nil { err := fmt.Errorf("Error calling gcloud sdk to launch IAP tunnel: %s", err) - cmd.Process.Kill() return err } // Wait for tunnel to launch and gather response. TODO: do this without @@ -53,9 +52,8 @@ func (t *TunnelDriverLinux) StartTunnel(cancelCtx context.Context, tempScriptFil serr := stderr.String() log.Println(serr) if strings.Contains(serr, "ERROR") { - cmd.Process.Kill() errIdx := strings.Index(serr, "ERROR:") - return fmt.Errorf("ERROR: %s", serr[errIdx+7:len(serr)]) + return fmt.Errorf("ERROR: %s", serr[errIdx+7:]) } // Store successful command on step so we can access it to cancel it // later. @@ -72,7 +70,10 @@ func (t *TunnelDriverLinux) StopTunnel() { // daemon child. We create the group ID with the syscall.SysProcAttr // call inside the retry loop above, and then store that ID on the // command so we can destroy it here. - syscall.Kill(-t.cmd.Process.Pid, syscall.SIGKILL) + err := syscall.Kill(-t.cmd.Process.Pid, syscall.SIGKILL) + if err != nil { + log.Printf("Issue stopping IAP tunnel: %s", err) + } } else { log.Printf("Couldn't find IAP tunnel process to kill. Continuing.") } From 80ecd2013da105d35830a5e5009fce22c17098e7 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 22 Apr 2020 14:26:50 -0700 Subject: [PATCH 06/23] basic untested implementation for Windows. May leave zombie processes lying around --- builder/googlecompute/step_start_tunnel.go | 2 +- builder/googlecompute/tunnel_driver.go | 2 +- .../googlecompute/tunnel_driver_windows.go | 53 ++++++++++++++++++- 3 files changed, 53 insertions(+), 4 deletions(-) diff --git a/builder/googlecompute/step_start_tunnel.go b/builder/googlecompute/step_start_tunnel.go index 7536f705c..202cf1819 100644 --- a/builder/googlecompute/step_start_tunnel.go +++ b/builder/googlecompute/step_start_tunnel.go @@ -100,7 +100,7 @@ func (s *StepStartTunnel) createTempGcloudScript(args []string) (string, error) s.IAPConf.IAPHashBang = fmt.Sprintf("#!%s\n", s.IAPConf.IAPHashBang) log.Printf("[INFO] (google): Prepending inline gcloud setup script with %s", s.IAPConf.IAPHashBang) - err = writer.WriteString(s.IAPConf.IAPHashBang) + _, err = writer.WriteString(s.IAPConf.IAPHashBang) if err != nil { return "", fmt.Errorf("Error preparing inline hashbang: %s", err) } diff --git a/builder/googlecompute/tunnel_driver.go b/builder/googlecompute/tunnel_driver.go index 78d9be634..1a6ef0bd5 100644 --- a/builder/googlecompute/tunnel_driver.go +++ b/builder/googlecompute/tunnel_driver.go @@ -64,7 +64,7 @@ func (t *TunnelDriverLinux) StartTunnel(cancelCtx context.Context, tempScriptFil func (t *TunnelDriverLinux) StopTunnel() { if t.cmd != nil && t.cmd.Process != nil { log.Printf("Cleaning up the IAP tunnel...") - // Why not just s.cmd.Process.Kill()? I'm glad you asked. The gcloud + // Why not just cmd.Process.Kill()? I'm glad you asked. The gcloud // call spawns a python subprocess that listens on the port, and you // need to use the process _group_ id to kill this process and its // daemon child. We create the group ID with the syscall.SysProcAttr diff --git a/builder/googlecompute/tunnel_driver_windows.go b/builder/googlecompute/tunnel_driver_windows.go index 9d5031ebc..73d739b5a 100644 --- a/builder/googlecompute/tunnel_driver_windows.go +++ b/builder/googlecompute/tunnel_driver_windows.go @@ -3,8 +3,13 @@ package googlecompute import ( + "bytes" "context" "fmt" + "log" + "os/exec" + "strings" + "time" ) func NewTunnelDriver() TunnelDriver { @@ -12,10 +17,54 @@ func NewTunnelDriver() TunnelDriver { } type TunnelDriverWindows struct { + cmd *exec.Cmd } func (t *TunnelDriverWindows) StartTunnel(cancelCtx context.Context, tempScriptFileName string) error { - return fmt.Errorf("Windows support for IAP tunnel not yet supported.") + // set stdout and stderr so we can read what's going on. + var stdout, stderr bytes.Buffer + + cmd := exec.CommandContext(cancelCtx, tempScriptFileName) + cmd.Stdout = &stdout + cmd.Stderr = &stderr + + err := cmd.Start() + log.Printf("Waiting 30s for tunnel to create...") + if err != nil { + err := fmt.Errorf("Error calling gcloud sdk to launch IAP tunnel: %s", + err) + return err + } + // Wait for tunnel to launch and gather response. TODO: do this without + // a sleep. + time.Sleep(30 * time.Second) + + // Track stdout. + sout := stdout.String() + if sout != "" { + log.Printf("[start-iap-tunnel] stdout is:") + } + + log.Printf("[start-iap-tunnel] stderr is:") + serr := stderr.String() + log.Println(serr) + if strings.Contains(serr, "ERROR") { + errIdx := strings.Index(serr, "ERROR:") + return fmt.Errorf("ERROR: %s", serr[errIdx+7:]) + } + // Store successful command on step so we can access it to cancel it + // later. + t.cmd = cmd + return nil } -func (t *TunnelDriverWindows) StopTunnel() {} +func (t *TunnelDriverWindows) StopTunnel() { + if t.cmd != nil && t.cmd.Process != nil { + err := t.cmd.Process.Kill() + if err != nil { + log.Printf("Issue stopping IAP tunnel: %s", err) + } + } else { + log.Printf("Couldn't find IAP tunnel process to kill. Continuing.") + } +} From f67a8ab431e469ab5ba35ea4f857de9ccbb7e8ed Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Thu, 23 Apr 2020 14:19:39 -0700 Subject: [PATCH 07/23] revert unneeded changes to driver roles --- builder/googlecompute/account.go | 7 +++---- builder/googlecompute/builder.go | 3 +-- builder/googlecompute/config.go | 2 +- builder/googlecompute/driver_gce.go | 18 ++++++------------ .../googlecompute-export/post-processor.go | 6 +++--- .../googlecompute-import/post-processor.go | 4 ++-- 6 files changed, 16 insertions(+), 24 deletions(-) diff --git a/builder/googlecompute/account.go b/builder/googlecompute/account.go index e2d630732..c8cb3fc13 100644 --- a/builder/googlecompute/account.go +++ b/builder/googlecompute/account.go @@ -9,10 +9,9 @@ import ( "golang.org/x/oauth2/jwt" ) -func ProcessAccountFile(text string, iap bool) (*jwt.Config, error) { - driverScopes := getDriverScopes(iap) +func ProcessAccountFile(text string) (*jwt.Config, error) { // Assume text is a JSON string - conf, err := google.JWTConfigFromJSON([]byte(text), driverScopes...) + 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) { @@ -26,7 +25,7 @@ func ProcessAccountFile(text string, iap bool) (*jwt.Config, error) { "Error reading account_file from path '%s': %s", text, err) } - conf, err = google.JWTConfigFromJSON(data, driverScopes...) + conf, err = google.JWTConfigFromJSON(data, DriverScopes...) if err != nil { return nil, fmt.Errorf("Error parsing account_file: %s", err) } diff --git a/builder/googlecompute/builder.go b/builder/googlecompute/builder.go index 1257e59a4..3e8fae9da 100644 --- a/builder/googlecompute/builder.go +++ b/builder/googlecompute/builder.go @@ -37,8 +37,7 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, []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, b.config.VaultGCPOauthEngine, - b.config.IAP) + ui, b.config.ProjectId, b.config.account, b.config.VaultGCPOauthEngine) if err != nil { return nil, err } diff --git a/builder/googlecompute/config.go b/builder/googlecompute/config.go index 195d487c1..e1cea263c 100644 --- a/builder/googlecompute/config.go +++ b/builder/googlecompute/config.go @@ -379,7 +379,7 @@ func (c *Config) Prepare(raws ...interface{}) ([]string, error) { errs = packer.MultiErrorAppend(errs, fmt.Errorf("You cannot "+ "specify both account_file and vault_gcp_oauth_engine.")) } - cfg, err := ProcessAccountFile(c.AccountFile, c.IAP) + cfg, err := ProcessAccountFile(c.AccountFile) if err != nil { errs = packer.MultiErrorAppend(errs, err) } diff --git a/builder/googlecompute/driver_gce.go b/builder/googlecompute/driver_gce.go index 9209fbd17..f98f964c4 100644 --- a/builder/googlecompute/driver_gce.go +++ b/builder/googlecompute/driver_gce.go @@ -34,13 +34,7 @@ type driverGCE struct { ui packer.Ui } -func getDriverScopes(iap bool) []string { - ds := []string{"https://www.googleapis.com/auth/compute", "https://www.googleapis.com/auth/devstorage.full_control"} - // if iap { - // ds = append(ds, "https://www.googleapis.com/auth/iap.tunnelResourceAccessor") - // } - return ds -} +var DriverScopes = []string{"https://www.googleapis.com/auth/compute", "https://www.googleapis.com/auth/devstorage.full_control"} // Define a TokenSource that gets tokens from Vault type OauthTokenSource struct { @@ -75,7 +69,7 @@ func (ots OauthTokenSource) Token() (*oauth2.Token, error) { } -func NewClientGCE(conf *jwt.Config, vaultOauth string, iap bool) (*http.Client, error) { +func NewClientGCE(conf *jwt.Config, vaultOauth string) (*http.Client, error) { var err error var client *http.Client @@ -90,7 +84,7 @@ func NewClientGCE(conf *jwt.Config, vaultOauth string, iap bool) (*http.Client, // Auth with AccountFile if provided log.Printf("[INFO] Requesting Google token via account_file...") log.Printf("[INFO] -- Email: %s", conf.Email) - log.Printf("[INFO] -- Scopes: %s", getDriverScopes(iap)) + log.Printf("[INFO] -- Scopes: %s", DriverScopes) log.Printf("[INFO] -- Private Key Length: %d", len(conf.PrivateKey)) // Initiate an http.Client. The following GET request will be @@ -99,7 +93,7 @@ func NewClientGCE(conf *jwt.Config, vaultOauth string, iap bool) (*http.Client, client = conf.Client(context.TODO()) } else { log.Printf("[INFO] Requesting Google token via GCE API Default Client Token Source...") - client, err = google.DefaultClient(context.TODO(), getDriverScopes(iap)...) + client, err = google.DefaultClient(context.TODO(), DriverScopes...) // The DefaultClient uses the DefaultTokenSource of the google lib. // The DefaultTokenSource uses the "Application Default Credentials" // It looks for credentials in the following places, preferring the first location found: @@ -121,8 +115,8 @@ func NewClientGCE(conf *jwt.Config, vaultOauth string, iap bool) (*http.Client, return client, nil } -func NewDriverGCE(ui packer.Ui, p string, conf *jwt.Config, vaultOauth string, iap bool) (Driver, error) { - client, err := NewClientGCE(conf, vaultOauth, iap) +func NewDriverGCE(ui packer.Ui, p string, conf *jwt.Config, vaultOauth string) (Driver, error) { + client, err := NewClientGCE(conf, vaultOauth) if err != nil { return nil, err } diff --git a/post-processor/googlecompute-export/post-processor.go b/post-processor/googlecompute-export/post-processor.go index f02c99974..2d4a2a995 100644 --- a/post-processor/googlecompute-export/post-processor.go +++ b/post-processor/googlecompute-export/post-processor.go @@ -112,14 +112,14 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact // Set up credentials for GCE driver. if builderAccountFile != "" { - cfg, err := googlecompute.ProcessAccountFile(builderAccountFile, p.config.IAP) + cfg, err := googlecompute.ProcessAccountFile(builderAccountFile) if err != nil { return nil, false, false, err } p.config.account = cfg } if p.config.AccountFile != "" { - cfg, err := googlecompute.ProcessAccountFile(p.config.AccountFile, p.config.IAP) + cfg, err := googlecompute.ProcessAccountFile(p.config.AccountFile) if err != nil { return nil, false, false, err } @@ -160,7 +160,7 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact } driver, err := googlecompute.NewDriverGCE(ui, builderProjectId, - p.config.account, p.config.VaultGCPOauthEngine, p.config.IAP) + p.config.account, p.config.VaultGCPOauthEngine) 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 a37d7f8fc..33f2501b9 100644 --- a/post-processor/googlecompute-import/post-processor.go +++ b/post-processor/googlecompute-import/post-processor.go @@ -78,7 +78,7 @@ func (p *PostProcessor) Configure(raws ...interface{}) error { } if p.config.AccountFile != "" { - cfg, err := googlecompute.ProcessAccountFile(p.config.AccountFile, p.config.IAP) + cfg, err := googlecompute.ProcessAccountFile(p.config.AccountFile) if err != nil { errs = packer.MultiErrorAppend(errs, err) } @@ -118,7 +118,7 @@ func (p *PostProcessor) PostProcess(ctx context.Context, ui packer.Ui, artifact } p.config.ctx.Data = generatedData - client, err := googlecompute.NewClientGCE(p.config.account, p.config.VaultGCPOauthEngine, p.config.IAP) + client, err := googlecompute.NewClientGCE(p.config.account, p.config.VaultGCPOauthEngine) if err != nil { return nil, false, false, err } From 9353635b43833f4a99b6a666485cf60aca01ff69 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Thu, 23 Apr 2020 14:30:38 -0700 Subject: [PATCH 08/23] send sigint instead of sigkill; we can at least ask the tunnel nicely to shut down. --- builder/googlecompute/tunnel_driver.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/googlecompute/tunnel_driver.go b/builder/googlecompute/tunnel_driver.go index 1a6ef0bd5..2c657ffcb 100644 --- a/builder/googlecompute/tunnel_driver.go +++ b/builder/googlecompute/tunnel_driver.go @@ -70,7 +70,7 @@ func (t *TunnelDriverLinux) StopTunnel() { // daemon child. We create the group ID with the syscall.SysProcAttr // call inside the retry loop above, and then store that ID on the // command so we can destroy it here. - err := syscall.Kill(-t.cmd.Process.Pid, syscall.SIGKILL) + err := syscall.Kill(-t.cmd.Process.Pid, syscall.SIGINT) if err != nil { log.Printf("Issue stopping IAP tunnel: %s", err) } From 3e1ddad0c7073dc68493585fb8b63e64b99cb257 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Thu, 23 Apr 2020 15:30:33 -0700 Subject: [PATCH 09/23] fix behavior when not using IAP, try to use more sophisticated streaming than buffer.String() --- builder/googlecompute/config.go | 14 +++++----- builder/googlecompute/step_start_tunnel.go | 4 +++ builder/googlecompute/tunnel_driver.go | 30 ++++++++++++---------- 3 files changed, 28 insertions(+), 20 deletions(-) diff --git a/builder/googlecompute/config.go b/builder/googlecompute/config.go index e1cea263c..5b5c7c564 100644 --- a/builder/googlecompute/config.go +++ b/builder/googlecompute/config.go @@ -336,12 +336,14 @@ func (c *Config) Prepare(raws ...interface{}) ([]string, error) { } // Configure IAP: Update SSH config to use localhost proxy instead - if c.Comm.Type == "ssh" { - c.Comm.SSHHost = "localhost" - } else { - err := fmt.Errorf("Error: IAP tunnel currently only implemnted for" + - " SSH communicator") - errs = packer.MultiErrorAppend(errs, err) + if c.IAPConfig.IAP { + if c.Comm.Type == "ssh" { + c.Comm.SSHHost = "localhost" + } else { + err := fmt.Errorf("Error: IAP tunnel currently only implemnted for" + + " SSH communicator") + errs = packer.MultiErrorAppend(errs, err) + } } // Process required parameters. diff --git a/builder/googlecompute/step_start_tunnel.go b/builder/googlecompute/step_start_tunnel.go index 202cf1819..e7935ab59 100644 --- a/builder/googlecompute/step_start_tunnel.go +++ b/builder/googlecompute/step_start_tunnel.go @@ -205,5 +205,9 @@ func (s *StepStartTunnel) Run(ctx context.Context, state multistep.StateBag) mul // Cleanup stops the IAP tunnel and cleans up processes. func (s *StepStartTunnel) Cleanup(state multistep.StateBag) { + if !s.IAPConf.IAP { + log.Printf("Skipping cleanup of IAP tunnel; \"iap\" is false.") + return + } s.tunnelDriver.StopTunnel() } diff --git a/builder/googlecompute/tunnel_driver.go b/builder/googlecompute/tunnel_driver.go index 2c657ffcb..e98d0fe40 100644 --- a/builder/googlecompute/tunnel_driver.go +++ b/builder/googlecompute/tunnel_driver.go @@ -3,6 +3,7 @@ package googlecompute import ( + "bufio" "bytes" "context" "fmt" @@ -38,23 +39,24 @@ func (t *TunnelDriverLinux) StartTunnel(cancelCtx context.Context, tempScriptFil err) return err } - // Wait for tunnel to launch and gather response. TODO: do this without - // a sleep. - time.Sleep(30 * time.Second) - - // Track stdout. - sout := stdout.String() - if sout != "" { - log.Printf("[start-iap-tunnel] stdout is:") - } + time.Sleep(10 * time.Second) + // read stdout + scanner := bufio.NewScanner(&stderr) log.Printf("[start-iap-tunnel] stderr is:") - serr := stderr.String() - log.Println(serr) - if strings.Contains(serr, "ERROR") { - errIdx := strings.Index(serr, "ERROR:") - return fmt.Errorf("ERROR: %s", serr[errIdx+7:]) + for scanner.Scan() { + line := scanner.Text() + log.Println(line) + + if strings.Contains(line, "ERROR") { + errIdx := strings.Index(line, "ERROR:") + return fmt.Errorf("ERROR: %s", line[errIdx+7:]) + } + } + if err := scanner.Err(); err != nil { + log.Printf("Error scanning tunnel stderr: %s", err) } + // Store successful command on step so we can access it to cancel it // later. t.cmd = cmd From 937a4859d46f0893856e67b3c413b4d2ece7b133 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 24 Apr 2020 11:09:00 -0700 Subject: [PATCH 10/23] Make retries a bit smarter, clean up language to be gentler, and give up on parsing stdout for tunnel launch --- builder/googlecompute/step_start_tunnel.go | 16 +++++- builder/googlecompute/tunnel_driver.go | 67 ++++++++++++++++------ 2 files changed, 63 insertions(+), 20 deletions(-) diff --git a/builder/googlecompute/step_start_tunnel.go b/builder/googlecompute/step_start_tunnel.go index e7935ab59..b085cbd95 100644 --- a/builder/googlecompute/step_start_tunnel.go +++ b/builder/googlecompute/step_start_tunnel.go @@ -53,6 +53,14 @@ type TunnelDriver interface { StopTunnel() } +type RetryableTunnelError struct { + s string +} + +func (e RetryableTunnelError) Error() string { + return "Tunnel start: " + e.s +} + type StepStartTunnel struct { IAPConf *IAPConfig CommConf *communicator.Config @@ -190,8 +198,12 @@ func (s *StepStartTunnel) Run(ctx context.Context, state multistep.StateBag) mul err = retry.Config{ Tries: 11, ShouldRetry: func(err error) bool { - // TODO be stricter with retries. - return true + switch err.(type) { + case RetryableTunnelError: + return true + default: + return false + } }, RetryDelay: (&retry.Backoff{InitialBackoff: 200 * time.Millisecond, MaxBackoff: 30 * time.Second, Multiplier: 2}).Linear, }.Run(ctx, func(ctx context.Context) error { diff --git a/builder/googlecompute/tunnel_driver.go b/builder/googlecompute/tunnel_driver.go index e98d0fe40..123867eea 100644 --- a/builder/googlecompute/tunnel_driver.go +++ b/builder/googlecompute/tunnel_driver.go @@ -3,10 +3,10 @@ package googlecompute import ( - "bufio" "bytes" "context" "fmt" + "io" "log" "os/exec" "strings" @@ -33,29 +33,60 @@ func (t *TunnelDriverLinux) StartTunnel(cancelCtx context.Context, tempScriptFil cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} err := cmd.Start() - log.Printf("Waiting 30s for tunnel to create...") if err != nil { err := fmt.Errorf("Error calling gcloud sdk to launch IAP tunnel: %s", err) return err } - time.Sleep(10 * time.Second) - // read stdout - scanner := bufio.NewScanner(&stderr) - log.Printf("[start-iap-tunnel] stderr is:") - for scanner.Scan() { - line := scanner.Text() - log.Println(line) - - if strings.Contains(line, "ERROR") { - errIdx := strings.Index(line, "ERROR:") - return fmt.Errorf("ERROR: %s", line[errIdx+7:]) + // Give tunnel 30 seconds to either launch, or return an error. + // Unfortunately, the SDK doesn't provide any official acknowledgment that + // the tunnel is launched when it's not being run through a TTY so we + // are just trusting here that 30s is enough to know whether the tunnel + // launch was going to fail. Yep, feels icky to me too. But I spent an + // afternoon trying to figure out how to get the SDK to actually send + // the "Listening on port [n]" line I see when I run it manually, and I + // can't justify spending more time than that on aesthetics. + for i := 0; i < 30; i++ { + time.Sleep(1 * time.Second) + + lineStderr, err := stderr.ReadString('\n') + if err != nil && err != io.EOF { + log.Printf("Err from scanning stderr is %s", err) + return fmt.Errorf("Error reading stderr from tunnel launch: %s", err) + } + if lineStderr != "" { + log.Printf("stderr: %s", lineStderr) + } + + lineStdout, err := stdout.ReadString('\n') + if err != nil && err != io.EOF { + log.Printf("Err from scanning stdout is %s", err) + return fmt.Errorf("Error reading stdout from tunnel launch: %s", err) + } + if lineStdout != "" { + log.Printf("stdout: %s", lineStdout) + } + + if strings.Contains(lineStderr, "ERROR") { + // 4033: Either you don't have permission to access the instance, + // the instance doesn't exist, or the instance is stopped. + // The two sub-errors we may see while the permissions settle are + // "not authorized" and "failed to connect to backend," but after + // about a minute of retries this goes away and we're able to + // connect. + if strings.Contains(lineStderr, "4033") { + return RetryableTunnelError{lineStderr} + } else { + log.Printf("NOT RETRYABLE: %s", lineStderr) + return fmt.Errorf("Non-retryable tunnel error: %s", lineStderr) + } + + return nil } } - if err := scanner.Err(); err != nil { - log.Printf("Error scanning tunnel stderr: %s", err) - } + + log.Printf("No error detected after tunnel launch; continuing...") // Store successful command on step so we can access it to cancel it // later. @@ -68,10 +99,10 @@ func (t *TunnelDriverLinux) StopTunnel() { log.Printf("Cleaning up the IAP tunnel...") // Why not just cmd.Process.Kill()? I'm glad you asked. The gcloud // call spawns a python subprocess that listens on the port, and you - // need to use the process _group_ id to kill this process and its + // need to use the process _group_ id to halt this process and its // daemon child. We create the group ID with the syscall.SysProcAttr // call inside the retry loop above, and then store that ID on the - // command so we can destroy it here. + // command so we can halt it here. err := syscall.Kill(-t.cmd.Process.Pid, syscall.SIGINT) if err != nil { log.Printf("Issue stopping IAP tunnel: %s", err) From 4fc92d7858a466855649a61c3f8128dc50f5cfce Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 24 Apr 2020 13:30:38 -0700 Subject: [PATCH 11/23] add tests --- .../googlecompute/step_start_tunnel_test.go | 123 ++++++++++++++++++ builder/googlecompute/tunnel_driver.go | 3 +- 2 files changed, 124 insertions(+), 2 deletions(-) create mode 100644 builder/googlecompute/step_start_tunnel_test.go diff --git a/builder/googlecompute/step_start_tunnel_test.go b/builder/googlecompute/step_start_tunnel_test.go new file mode 100644 index 000000000..5ef0fdc8d --- /dev/null +++ b/builder/googlecompute/step_start_tunnel_test.go @@ -0,0 +1,123 @@ +package googlecompute + +import ( + "context" + "fmt" + "io/ioutil" + "os" + "testing" + + "github.com/hashicorp/packer/helper/communicator" +) + +type MockTunnelDriver struct { + StopTunnelCalled bool + StartTunnelCalled bool +} + +func (m *MockTunnelDriver) StopTunnel() { + m.StopTunnelCalled = true + return +} + +func (m *MockTunnelDriver) StartTunnel(context.Context, string) error { + m.StartTunnelCalled = true + return nil +} + +func getTestStepStartTunnel() *StepStartTunnel { + return &StepStartTunnel{ + IAPConf: &IAPConfig{ + IAP: true, + IAPLocalhostPort: 0, + IAPHashBang: "/bin/bash", + IAPExt: "", + }, + CommConf: &communicator.Config{ + SSH: communicator.SSH{ + SSHPort: 1234, + }, + }, + AccountFile: "/path/to/account_file.json", + } +} + +func TestStepStartTunnel_CreateTempScript(t *testing.T) { + s := getTestStepStartTunnel() + + args := []string{"compute", "start-iap-tunnel", "fakeinstance-12345", + "1234", "--local-host-port=localhost:8774", "--zone", "us-central-b"} + + scriptPath, err := s.createTempGcloudScript(args) + if err != nil { + t.Fatalf("Shouldn't have error building script file.") + } + defer os.Remove(scriptPath) + + f, err := ioutil.ReadFile(scriptPath) + if err != nil { + t.Fatalf("couldn't read created inventoryfile: %s", err) + } + expected := `#!/bin/bash +gcloud auth activate-service-account --key-file='/path/to/account_file.json' +gcloud compute start-iap-tunnel fakeinstance-12345 1234 --local-host-port=localhost:8774 --zone us-central-b +` + if fmt.Sprintf("%s", f) != expected { + t.Fatalf("script didn't match expected:\n\n expected: \n%s\n; recieved: \n%s\n", expected, f) + } +} + +func TestStepStartTunnel_Cleanup(t *testing.T) { + // Check IAP true + s := getTestStepStartTunnel() + td := &MockTunnelDriver{} + s.tunnelDriver = td + + state := testState(t) + s.Cleanup(state) + + if !td.StopTunnelCalled { + t.Fatalf("Should have called StopTunnel, since IAP is true") + } + + // Check IAP false + s = getTestStepStartTunnel() + td = &MockTunnelDriver{} + s.tunnelDriver = td + + s.IAPConf.IAP = false + + s.Cleanup(state) + + if td.StopTunnelCalled { + t.Fatalf("Should not have called StopTunnel, since IAP is false") + } +} + +func TestStepStartTunnel_ConfigurePort_port_set_by_user(t *testing.T) { + s := getTestStepStartTunnel() + s.IAPConf.IAPLocalhostPort = 8447 + + ctx := context.TODO() + err := s.ConfigureLocalHostPort(ctx) + if err != nil { + t.Fatalf("Shouldn't have error detecting port") + } + if s.IAPConf.IAPLocalhostPort != 8447 { + t.Fatalf("Shouldn't have found new port; one was configured.") + } +} + +func TestStepStartTunnel_ConfigurePort_port_not_set_by_user(t *testing.T) { + s := getTestStepStartTunnel() + s.IAPConf.IAPLocalhostPort = 0 + + ctx := context.TODO() + err := s.ConfigureLocalHostPort(ctx) + if err != nil { + t.Fatalf("Shouldn't have error detecting port") + } + if s.IAPConf.IAPLocalhostPort == 0 { + t.Fatalf("Should have found new port; none was configured.") + } +} diff --git a/builder/googlecompute/tunnel_driver.go b/builder/googlecompute/tunnel_driver.go index 123867eea..06447b3c1 100644 --- a/builder/googlecompute/tunnel_driver.go +++ b/builder/googlecompute/tunnel_driver.go @@ -81,9 +81,8 @@ func (t *TunnelDriverLinux) StartTunnel(cancelCtx context.Context, tempScriptFil log.Printf("NOT RETRYABLE: %s", lineStderr) return fmt.Errorf("Non-retryable tunnel error: %s", lineStderr) } - - return nil } + return nil } log.Printf("No error detected after tunnel launch; continuing...") From f583674cd66e44852d81df43bf8171ef9d317c43 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 24 Apr 2020 13:44:10 -0700 Subject: [PATCH 12/23] linding --- builder/googlecompute/step_start_tunnel_test.go | 1 - builder/googlecompute/tunnel_driver.go | 1 - 2 files changed, 2 deletions(-) diff --git a/builder/googlecompute/step_start_tunnel_test.go b/builder/googlecompute/step_start_tunnel_test.go index 5ef0fdc8d..48634008d 100644 --- a/builder/googlecompute/step_start_tunnel_test.go +++ b/builder/googlecompute/step_start_tunnel_test.go @@ -17,7 +17,6 @@ type MockTunnelDriver struct { func (m *MockTunnelDriver) StopTunnel() { m.StopTunnelCalled = true - return } func (m *MockTunnelDriver) StartTunnel(context.Context, string) error { diff --git a/builder/googlecompute/tunnel_driver.go b/builder/googlecompute/tunnel_driver.go index 06447b3c1..fee1cff76 100644 --- a/builder/googlecompute/tunnel_driver.go +++ b/builder/googlecompute/tunnel_driver.go @@ -82,7 +82,6 @@ func (t *TunnelDriverLinux) StartTunnel(cancelCtx context.Context, tempScriptFil return fmt.Errorf("Non-retryable tunnel error: %s", lineStderr) } } - return nil } log.Printf("No error detected after tunnel launch; continuing...") From b5744efb4224226d5afd9b0fb8b60de197b5d1fb Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 24 Apr 2020 14:11:00 -0700 Subject: [PATCH 13/23] add config prepare tests --- builder/googlecompute/config_test.go | 51 ++++++++++++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/builder/googlecompute/config_test.go b/builder/googlecompute/config_test.go index 7753e1d8a..2b4eeb33a 100644 --- a/builder/googlecompute/config_test.go +++ b/builder/googlecompute/config_test.go @@ -382,6 +382,57 @@ func TestConfigPrepareStartupScriptFile(t *testing.T) { } } +func TestConfigPrepareIAP(t *testing.T) { + config := map[string]interface{}{ + "project_id": "project", + "source_image": "foo", + "ssh_username": "packer", + "startup_script_file": "no-such-file", + "zone": "us-central1-a", + "communicator": "ssh", + "use_iap": true, + } + + var c Config + c.Prepare(config) + + if c.IAPHashBang != "/bin/sh" { + t.Fatalf("IAP hashbang didn't default correctly to /bin/sh.") + } + if c.IAPExt != ".sh" { + t.Fatalf("IAP tempfile extension didn't default correctly to .sh") + } + if c.Comm.SSHHost != "localhost" { + t.Fatalf("Didn't correctly override the ssh host.") + } +} + +func TestConfigPrepareIAP_failures(t *testing.T) { + config := map[string]interface{}{ + "project_id": "project", + "source_image": "foo", + "winrm_username": "packer", + "startup_script_file": "no-such-file", + "zone": "us-central1-a", + "communicator": "winrm", + "iap_hashbang": "/bin/bash", + "iap_ext": ".ps1", + "use_iap": true, + } + + var c Config + _, errs := c.Prepare(config) + if errs == nil { + t.Fatalf("Should have errored because we're using winrm.") + } + if c.IAPHashBang != "/bin/bash" { + t.Fatalf("IAP hashbang defaulted even though set.") + } + if c.IAPExt != ".ps1" { + t.Fatalf("IAP tempfile defaulted even though set.") + } +} + func TestConfigDefaults(t *testing.T) { cases := []struct { Read func(c *Config) interface{} From 32752d45776a727a20a098c95304ee7465a12909 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 24 Apr 2020 14:27:56 -0700 Subject: [PATCH 14/23] fix linting and tests --- builder/googlecompute/config_test.go | 35 ++++++++++++++-------------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/builder/googlecompute/config_test.go b/builder/googlecompute/config_test.go index 2b4eeb33a..438a1bd06 100644 --- a/builder/googlecompute/config_test.go +++ b/builder/googlecompute/config_test.go @@ -384,17 +384,19 @@ func TestConfigPrepareStartupScriptFile(t *testing.T) { func TestConfigPrepareIAP(t *testing.T) { config := map[string]interface{}{ - "project_id": "project", - "source_image": "foo", - "ssh_username": "packer", - "startup_script_file": "no-such-file", - "zone": "us-central1-a", - "communicator": "ssh", - "use_iap": true, + "project_id": "project", + "source_image": "foo", + "ssh_username": "packer", + "zone": "us-central1-a", + "communicator": "ssh", + "use_iap": true, } var c Config - c.Prepare(config) + _, err := c.Prepare(config) + if err != nil { + t.Fatalf("Shouldn't have errors. Err = %s", err) + } if c.IAPHashBang != "/bin/sh" { t.Fatalf("IAP hashbang didn't default correctly to /bin/sh.") @@ -409,15 +411,14 @@ func TestConfigPrepareIAP(t *testing.T) { func TestConfigPrepareIAP_failures(t *testing.T) { config := map[string]interface{}{ - "project_id": "project", - "source_image": "foo", - "winrm_username": "packer", - "startup_script_file": "no-such-file", - "zone": "us-central1-a", - "communicator": "winrm", - "iap_hashbang": "/bin/bash", - "iap_ext": ".ps1", - "use_iap": true, + "project_id": "project", + "source_image": "foo", + "winrm_username": "packer", + "zone": "us-central1-a", + "communicator": "winrm", + "iap_hashbang": "/bin/bash", + "iap_ext": ".ps1", + "use_iap": true, } var c Config From c578afc62cc1b5a735459677247cff229bcf2992 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 29 Apr 2020 12:13:22 -0700 Subject: [PATCH 15/23] working on windows --- builder/googlecompute/config.go | 11 ++- builder/googlecompute/step_start_tunnel.go | 30 +++++--- .../googlecompute/tunnel_driver_windows.go | 68 ++++++++++++++----- 3 files changed, 81 insertions(+), 28 deletions(-) diff --git a/builder/googlecompute/config.go b/builder/googlecompute/config.go index 5b5c7c564..3c26b6f4b 100644 --- a/builder/googlecompute/config.go +++ b/builder/googlecompute/config.go @@ -8,6 +8,7 @@ import ( "fmt" "os" "regexp" + "runtime" "time" "github.com/hashicorp/packer/common" @@ -329,10 +330,16 @@ func (c *Config) Prepare(raws ...interface{}) ([]string, error) { // set defaults for IAP if c.IAPConfig.IAPHashBang == "" { - c.IAPConfig.IAPHashBang = "/bin/sh" + if runtime.GOOS == "windows" { + c.IAPConfig.IAPHashBang = "" + } else { + c.IAPConfig.IAPHashBang = "/bin/sh" + } } if c.IAPConfig.IAPExt == "" { - c.IAPConfig.IAPExt = ".sh" + if runtime.GOOS == "windows" { + c.IAPConfig.IAPExt = ".cmd" + } } // Configure IAP: Update SSH config to use localhost proxy instead diff --git a/builder/googlecompute/step_start_tunnel.go b/builder/googlecompute/step_start_tunnel.go index b085cbd95..59393c49a 100644 --- a/builder/googlecompute/step_start_tunnel.go +++ b/builder/googlecompute/step_start_tunnel.go @@ -105,12 +105,15 @@ func (s *StepStartTunnel) createTempGcloudScript(args []string) (string, error) // Write our contents to it writer := bufio.NewWriter(tf) - s.IAPConf.IAPHashBang = fmt.Sprintf("#!%s\n", s.IAPConf.IAPHashBang) - log.Printf("[INFO] (google): Prepending inline gcloud setup script with %s", - s.IAPConf.IAPHashBang) - _, err = writer.WriteString(s.IAPConf.IAPHashBang) - if err != nil { - return "", fmt.Errorf("Error preparing inline hashbang: %s", err) + if s.IAPConf.IAPHashBang != "" { + s.IAPConf.IAPHashBang = fmt.Sprintf("#!%s\n", s.IAPConf.IAPHashBang) + log.Printf("[INFO] (google): Prepending inline gcloud setup script with %s", + s.IAPConf.IAPHashBang) + _, err = writer.WriteString(s.IAPConf.IAPHashBang) + if err != nil { + return "", fmt.Errorf("Error preparing inline hashbang: %s", err) + } + } // authenticate to gcloud @@ -130,7 +133,8 @@ func (s *StepStartTunnel) createTempGcloudScript(args []string) (string, error) if err := writer.Flush(); err != nil { return "", fmt.Errorf("Error preparing shell script: %s", err) } - + // Have to close temp file before renaming it or Windows will complain. + tf.Close() err = os.Chmod(tf.Name(), 0700) if err != nil { log.Printf("[ERROR] (google): error modifying permissions of temp script file: %s", err.Error()) @@ -181,8 +185,7 @@ func (s *StepStartTunnel) Run(ctx context.Context, state multistep.StateBag) mul // TODO make setting LocalHostPort optional s.CommConf.SSHPort = s.IAPConf.IAPLocalhostPort - log.Printf("Calling tunnel launch with args %#v", args) - + log.Printf("Creating tunnel launch script with args %#v", args) // Create temp file that contains both gcloud authentication, and gcloud // proxy setup call. tempScriptFileName, err := s.createTempGcloudScript(args) @@ -211,6 +214,11 @@ func (s *StepStartTunnel) Run(ctx context.Context, state multistep.StateBag) mul err := s.tunnelDriver.StartTunnel(ctx, tempScriptFileName) return err }) + if err != nil { + state.Put("error", err) + ui.Error(err.Error()) + return multistep.ActionHalt + } return multistep.ActionContinue } @@ -221,5 +229,7 @@ func (s *StepStartTunnel) Cleanup(state multistep.StateBag) { log.Printf("Skipping cleanup of IAP tunnel; \"iap\" is false.") return } - s.tunnelDriver.StopTunnel() + if s.tunnelDriver != nil { + s.tunnelDriver.StopTunnel() + } } diff --git a/builder/googlecompute/tunnel_driver_windows.go b/builder/googlecompute/tunnel_driver_windows.go index 73d739b5a..16c10d8c9 100644 --- a/builder/googlecompute/tunnel_driver_windows.go +++ b/builder/googlecompute/tunnel_driver_windows.go @@ -6,6 +6,7 @@ import ( "bytes" "context" "fmt" + "io" "log" "os/exec" "strings" @@ -13,6 +14,7 @@ import ( ) func NewTunnelDriver() TunnelDriver { + log.Printf("Megan created driver for windows") return &TunnelDriverWindows{} } @@ -21,37 +23,71 @@ type TunnelDriverWindows struct { } func (t *TunnelDriverWindows) StartTunnel(cancelCtx context.Context, tempScriptFileName string) error { + log.Printf("Megan inside StartTunnel for windows") // set stdout and stderr so we can read what's going on. var stdout, stderr bytes.Buffer - cmd := exec.CommandContext(cancelCtx, tempScriptFileName) + args := []string{"/C", "call", tempScriptFileName} + + cmd := exec.CommandContext(cancelCtx, "cmd", args...) cmd.Stdout = &stdout cmd.Stderr = &stderr err := cmd.Start() - log.Printf("Waiting 30s for tunnel to create...") if err != nil { + log.Printf("Megan: error calling start.") err := fmt.Errorf("Error calling gcloud sdk to launch IAP tunnel: %s", err) return err } - // Wait for tunnel to launch and gather response. TODO: do this without - // a sleep. - time.Sleep(30 * time.Second) - // Track stdout. - sout := stdout.String() - if sout != "" { - log.Printf("[start-iap-tunnel] stdout is:") - } + // Give tunnel 30 seconds to either launch, or return an error. + // Unfortunately, the SDK doesn't provide any official acknowledgment that + // the tunnel is launched when it's not being run through a TTY so we + // are just trusting here that 30s is enough to know whether the tunnel + // launch was going to fail. Yep, feels icky to me too. But I spent an + // afternoon trying to figure out how to get the SDK to actually send + // the "Listening on port [n]" line I see when I run it manually, and I + // can't justify spending more time than that on aesthetics. + for i := 0; i < 30; i++ { + time.Sleep(1 * time.Second) + + lineStderr, err := stderr.ReadString('\n') + if err != nil && err != io.EOF { + log.Printf("Err from scanning stderr is %s", err) + return fmt.Errorf("Error reading stderr from tunnel launch: %s", err) + } + if lineStderr != "" { + log.Printf("stderr: %s", lineStderr) + } - log.Printf("[start-iap-tunnel] stderr is:") - serr := stderr.String() - log.Println(serr) - if strings.Contains(serr, "ERROR") { - errIdx := strings.Index(serr, "ERROR:") - return fmt.Errorf("ERROR: %s", serr[errIdx+7:]) + lineStdout, err := stdout.ReadString('\n') + if err != nil && err != io.EOF { + log.Printf("Err from scanning stdout is %s", err) + return fmt.Errorf("Error reading stdout from tunnel launch: %s", err) + } + if lineStdout != "" { + log.Printf("stdout: %s", lineStdout) + } + + if strings.Contains(lineStderr, "ERROR") { + // 4033: Either you don't have permission to access the instance, + // the instance doesn't exist, or the instance is stopped. + // The two sub-errors we may see while the permissions settle are + // "not authorized" and "failed to connect to backend," but after + // about a minute of retries this goes away and we're able to + // connect. + if strings.Contains(lineStderr, "4033") { + return RetryableTunnelError{lineStderr} + } else { + log.Printf("NOT RETRYABLE: %s", lineStderr) + return fmt.Errorf("Non-retryable tunnel error: %s", lineStderr) + } + } } + + log.Printf("No error detected after tunnel launch; continuing...") + // Store successful command on step so we can access it to cancel it // later. t.cmd = cmd From 850303b8b8281303075db1dc38a8f4bb08745412 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 6 May 2020 14:16:35 -0700 Subject: [PATCH 16/23] get gcloud integration working on Windows --- builder/googlecompute/builder.go | 1 + builder/googlecompute/step_start_tunnel.go | 38 ++++++++++++++++++---- builder/googlecompute/tunnel_driver.go | 3 +- 3 files changed, 34 insertions(+), 8 deletions(-) diff --git a/builder/googlecompute/builder.go b/builder/googlecompute/builder.go index 3e8fae9da..f083066c4 100644 --- a/builder/googlecompute/builder.go +++ b/builder/googlecompute/builder.go @@ -70,6 +70,7 @@ func (b *Builder) Run(ctx context.Context, ui packer.Ui, hook packer.Hook) (pack IAPConf: &b.config.IAPConfig, CommConf: &b.config.Comm, AccountFile: b.config.AccountFile, + ProjectId: b.config.ProjectId, }, &communicator.StepConnect{ Config: &b.config.Comm, diff --git a/builder/googlecompute/step_start_tunnel.go b/builder/googlecompute/step_start_tunnel.go index 59393c49a..83447a6c9 100644 --- a/builder/googlecompute/step_start_tunnel.go +++ b/builder/googlecompute/step_start_tunnel.go @@ -5,12 +5,15 @@ package googlecompute import ( "bufio" + "bytes" "context" "fmt" "log" "os" + "runtime" "strconv" "strings" + "text/template" "time" "github.com/hashicorp/packer/common/net" @@ -65,6 +68,7 @@ type StepStartTunnel struct { IAPConf *IAPConfig CommConf *communicator.Config AccountFile string + ProjectId string tunnelDriver TunnelDriver } @@ -116,17 +120,37 @@ func (s *StepStartTunnel) createTempGcloudScript(args []string) (string, error) } - // authenticate to gcloud - _, err = writer.WriteString( - fmt.Sprintf("gcloud auth activate-service-account --key-file='%s'\n", - s.AccountFile)) - if err != nil { - return "", fmt.Errorf("Error preparing gcloud shell script: %s", err) + launchTemplate := ` +gcloud auth activate-service-account --key-file='{{.AccountFile}}' +gcloud config set project {{.ProjectID}} +{{.Args}} +` + if runtime.GOOS == "windows" { + launchTemplate = ` +call gcloud auth activate-service-account --key-file {{.AccountFile}} +call gcloud config set project {{.ProjectID}} +call {{.Args}} +` } // call command args = append([]string{"gcloud"}, args...) argString := strings.Join(args, " ") - if _, err := writer.WriteString(argString + "\n"); err != nil { + + var tpl = template.Must(template.New("createTunnel").Parse(launchTemplate)) + var b bytes.Buffer + + opts := map[string]string{ + "AccountFile": s.AccountFile, + "ProjectID": s.ProjectId, + "Args": argString, + } + + err = tpl.Execute(&b, opts) + if err != nil { + fmt.Println(err) + } + + if _, err := writer.WriteString(b.String()); err != nil { return "", fmt.Errorf("Error preparing gcloud shell script: %s", err) } diff --git a/builder/googlecompute/tunnel_driver.go b/builder/googlecompute/tunnel_driver.go index fee1cff76..cd067b1ea 100644 --- a/builder/googlecompute/tunnel_driver.go +++ b/builder/googlecompute/tunnel_driver.go @@ -75,7 +75,8 @@ func (t *TunnelDriverLinux) StartTunnel(cancelCtx context.Context, tempScriptFil // "not authorized" and "failed to connect to backend," but after // about a minute of retries this goes away and we're able to // connect. - if strings.Contains(lineStderr, "4033") { + // 4003: "failed to connect to backend". Network blip. + if strings.Contains(lineStderr, "4033") || strings.Contains(lineStderr, "4003") { return RetryableTunnelError{lineStderr} } else { log.Printf("NOT RETRYABLE: %s", lineStderr) From 54b33ad8d1af1ece98140d9474d106f7cd08f0ef Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 6 May 2020 14:28:22 -0700 Subject: [PATCH 17/23] fix tests --- builder/googlecompute/config_test.go | 11 +++++++++-- builder/googlecompute/step_start_tunnel_test.go | 12 ++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) diff --git a/builder/googlecompute/config_test.go b/builder/googlecompute/config_test.go index 438a1bd06..d7dd11ebc 100644 --- a/builder/googlecompute/config_test.go +++ b/builder/googlecompute/config_test.go @@ -4,6 +4,7 @@ import ( "fmt" "io/ioutil" "os" + "runtime" "strings" "testing" ) @@ -401,8 +402,14 @@ func TestConfigPrepareIAP(t *testing.T) { if c.IAPHashBang != "/bin/sh" { t.Fatalf("IAP hashbang didn't default correctly to /bin/sh.") } - if c.IAPExt != ".sh" { - t.Fatalf("IAP tempfile extension didn't default correctly to .sh") + if runtime.GOOS == "windows" { + if c.IAPExt != ".cmd" { + t.Fatalf("IAP tempfile extension didn't default correctly to .cmd") + } + } else { + if c.IAPExt != "" { + t.Fatalf("IAP tempfile extension should default to empty on unix mahcines") + } } if c.Comm.SSHHost != "localhost" { t.Fatalf("Didn't correctly override the ssh host.") diff --git a/builder/googlecompute/step_start_tunnel_test.go b/builder/googlecompute/step_start_tunnel_test.go index 48634008d..cf8a8b56f 100644 --- a/builder/googlecompute/step_start_tunnel_test.go +++ b/builder/googlecompute/step_start_tunnel_test.go @@ -5,6 +5,7 @@ import ( "fmt" "io/ioutil" "os" + "runtime" "testing" "github.com/hashicorp/packer/helper/communicator" @@ -38,6 +39,7 @@ func getTestStepStartTunnel() *StepStartTunnel { }, }, AccountFile: "/path/to/account_file.json", + ProjectId: "fake-project-123", } } @@ -57,10 +59,20 @@ func TestStepStartTunnel_CreateTempScript(t *testing.T) { if err != nil { t.Fatalf("couldn't read created inventoryfile: %s", err) } + expected := `#!/bin/bash + gcloud auth activate-service-account --key-file='/path/to/account_file.json' +gcloud config set project fake-project-123 gcloud compute start-iap-tunnel fakeinstance-12345 1234 --local-host-port=localhost:8774 --zone us-central-b ` + if runtime.GOOS == "windows" { + expected = ` +call gcloud auth activate-service-account --key-file /path/to/account_file.json +call gcloud config set project fake-project-123 +call gcloud compute start-iap-tunnel fakeinstance-12345 1234 --local-host-port=localhost:8774 --zone us-central-b +` + } if fmt.Sprintf("%s", f) != expected { t.Fatalf("script didn't match expected:\n\n expected: \n%s\n; recieved: \n%s\n", expected, f) } From fda55fe928e8d2c7032a329f424c9774ad0693d4 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 6 May 2020 14:49:36 -0700 Subject: [PATCH 18/23] deduplicate excess code --- builder/googlecompute/step_start_tunnel.go | 64 ++++++++++++++++++ .../googlecompute/step_start_tunnel_test.go | 5 ++ builder/googlecompute/tunnel_driver.go | 63 +---------------- .../googlecompute/tunnel_driver_windows.go | 67 +------------------ 4 files changed, 71 insertions(+), 128 deletions(-) diff --git a/builder/googlecompute/step_start_tunnel.go b/builder/googlecompute/step_start_tunnel.go index 83447a6c9..6b774a57e 100644 --- a/builder/googlecompute/step_start_tunnel.go +++ b/builder/googlecompute/step_start_tunnel.go @@ -8,8 +8,10 @@ import ( "bytes" "context" "fmt" + "io" "log" "os" + "os/exec" "runtime" "strconv" "strings" @@ -56,6 +58,68 @@ type TunnelDriver interface { StopTunnel() } +func RunTunnelCommand(cmd *exec.Cmd) error { + // set stdout and stderr so we can read what's going on. + var stdout, stderr bytes.Buffer + cmd.Stdout = &stdout + cmd.Stderr = &stderr + err := cmd.Start() + if err != nil { + err := fmt.Errorf("Error calling gcloud sdk to launch IAP tunnel: %s", + err) + return err + } + + // Give tunnel 30 seconds to either launch, or return an error. + // Unfortunately, the SDK doesn't provide any official acknowledgment that + // the tunnel is launched when it's not being run through a TTY so we + // are just trusting here that 30s is enough to know whether the tunnel + // launch was going to fail. Yep, feels icky to me too. But I spent an + // afternoon trying to figure out how to get the SDK to actually send + // the "Listening on port [n]" line I see when I run it manually, and I + // can't justify spending more time than that on aesthetics. + for i := 0; i < 30; i++ { + time.Sleep(1 * time.Second) + + lineStderr, err := stderr.ReadString('\n') + if err != nil && err != io.EOF { + log.Printf("Err from scanning stderr is %s", err) + return fmt.Errorf("Error reading stderr from tunnel launch: %s", err) + } + if lineStderr != "" { + log.Printf("stderr: %s", lineStderr) + } + + lineStdout, err := stdout.ReadString('\n') + if err != nil && err != io.EOF { + log.Printf("Err from scanning stdout is %s", err) + return fmt.Errorf("Error reading stdout from tunnel launch: %s", err) + } + if lineStdout != "" { + log.Printf("stdout: %s", lineStdout) + } + + if strings.Contains(lineStderr, "ERROR") { + // 4033: Either you don't have permission to access the instance, + // the instance doesn't exist, or the instance is stopped. + // The two sub-errors we may see while the permissions settle are + // "not authorized" and "failed to connect to backend," but after + // about a minute of retries this goes away and we're able to + // connect. + // 4003: "failed to connect to backend". Network blip. + if strings.Contains(lineStderr, "4033") || strings.Contains(lineStderr, "4003") { + return RetryableTunnelError{lineStderr} + } else { + log.Printf("NOT RETRYABLE: %s", lineStderr) + return fmt.Errorf("Non-retryable tunnel error: %s", lineStderr) + } + } + } + + log.Printf("No error detected after tunnel launch; continuing...") + return nil +} + type RetryableTunnelError struct { s string } diff --git a/builder/googlecompute/step_start_tunnel_test.go b/builder/googlecompute/step_start_tunnel_test.go index cf8a8b56f..176b14ac4 100644 --- a/builder/googlecompute/step_start_tunnel_test.go +++ b/builder/googlecompute/step_start_tunnel_test.go @@ -1,12 +1,17 @@ package googlecompute import ( + "bytes" "context" "fmt" + "io" "io/ioutil" "os" + "os/exec" "runtime" + "strings" "testing" + "time" "github.com/hashicorp/packer/helper/communicator" ) diff --git a/builder/googlecompute/tunnel_driver.go b/builder/googlecompute/tunnel_driver.go index cd067b1ea..8bd5e0a56 100644 --- a/builder/googlecompute/tunnel_driver.go +++ b/builder/googlecompute/tunnel_driver.go @@ -3,15 +3,10 @@ package googlecompute import ( - "bytes" "context" - "fmt" - "io" "log" "os/exec" - "strings" "syscall" - "time" ) func NewTunnelDriver() TunnelDriver { @@ -23,70 +18,14 @@ type TunnelDriverLinux struct { } func (t *TunnelDriverLinux) StartTunnel(cancelCtx context.Context, tempScriptFileName string) error { - // set stdout and stderr so we can read what's going on. - var stdout, stderr bytes.Buffer - cmd := exec.CommandContext(cancelCtx, tempScriptFileName) - cmd.Stdout = &stdout - cmd.Stderr = &stderr - cmd.SysProcAttr = &syscall.SysProcAttr{Setpgid: true} - err := cmd.Start() + err := RunTunnelCommand(cmd) if err != nil { - err := fmt.Errorf("Error calling gcloud sdk to launch IAP tunnel: %s", - err) return err } - // Give tunnel 30 seconds to either launch, or return an error. - // Unfortunately, the SDK doesn't provide any official acknowledgment that - // the tunnel is launched when it's not being run through a TTY so we - // are just trusting here that 30s is enough to know whether the tunnel - // launch was going to fail. Yep, feels icky to me too. But I spent an - // afternoon trying to figure out how to get the SDK to actually send - // the "Listening on port [n]" line I see when I run it manually, and I - // can't justify spending more time than that on aesthetics. - for i := 0; i < 30; i++ { - time.Sleep(1 * time.Second) - - lineStderr, err := stderr.ReadString('\n') - if err != nil && err != io.EOF { - log.Printf("Err from scanning stderr is %s", err) - return fmt.Errorf("Error reading stderr from tunnel launch: %s", err) - } - if lineStderr != "" { - log.Printf("stderr: %s", lineStderr) - } - - lineStdout, err := stdout.ReadString('\n') - if err != nil && err != io.EOF { - log.Printf("Err from scanning stdout is %s", err) - return fmt.Errorf("Error reading stdout from tunnel launch: %s", err) - } - if lineStdout != "" { - log.Printf("stdout: %s", lineStdout) - } - - if strings.Contains(lineStderr, "ERROR") { - // 4033: Either you don't have permission to access the instance, - // the instance doesn't exist, or the instance is stopped. - // The two sub-errors we may see while the permissions settle are - // "not authorized" and "failed to connect to backend," but after - // about a minute of retries this goes away and we're able to - // connect. - // 4003: "failed to connect to backend". Network blip. - if strings.Contains(lineStderr, "4033") || strings.Contains(lineStderr, "4003") { - return RetryableTunnelError{lineStderr} - } else { - log.Printf("NOT RETRYABLE: %s", lineStderr) - return fmt.Errorf("Non-retryable tunnel error: %s", lineStderr) - } - } - } - - log.Printf("No error detected after tunnel launch; continuing...") - // Store successful command on step so we can access it to cancel it // later. t.cmd = cmd diff --git a/builder/googlecompute/tunnel_driver_windows.go b/builder/googlecompute/tunnel_driver_windows.go index 16c10d8c9..110b945a3 100644 --- a/builder/googlecompute/tunnel_driver_windows.go +++ b/builder/googlecompute/tunnel_driver_windows.go @@ -3,18 +3,12 @@ package googlecompute import ( - "bytes" "context" - "fmt" - "io" "log" "os/exec" - "strings" - "time" ) func NewTunnelDriver() TunnelDriver { - log.Printf("Megan created driver for windows") return &TunnelDriverWindows{} } @@ -23,71 +17,12 @@ type TunnelDriverWindows struct { } func (t *TunnelDriverWindows) StartTunnel(cancelCtx context.Context, tempScriptFileName string) error { - log.Printf("Megan inside StartTunnel for windows") - // set stdout and stderr so we can read what's going on. - var stdout, stderr bytes.Buffer - args := []string{"/C", "call", tempScriptFileName} - cmd := exec.CommandContext(cancelCtx, "cmd", args...) - cmd.Stdout = &stdout - cmd.Stderr = &stderr - - err := cmd.Start() + err := RunTunnelCommand(cmd) if err != nil { - log.Printf("Megan: error calling start.") - err := fmt.Errorf("Error calling gcloud sdk to launch IAP tunnel: %s", - err) return err } - - // Give tunnel 30 seconds to either launch, or return an error. - // Unfortunately, the SDK doesn't provide any official acknowledgment that - // the tunnel is launched when it's not being run through a TTY so we - // are just trusting here that 30s is enough to know whether the tunnel - // launch was going to fail. Yep, feels icky to me too. But I spent an - // afternoon trying to figure out how to get the SDK to actually send - // the "Listening on port [n]" line I see when I run it manually, and I - // can't justify spending more time than that on aesthetics. - for i := 0; i < 30; i++ { - time.Sleep(1 * time.Second) - - lineStderr, err := stderr.ReadString('\n') - if err != nil && err != io.EOF { - log.Printf("Err from scanning stderr is %s", err) - return fmt.Errorf("Error reading stderr from tunnel launch: %s", err) - } - if lineStderr != "" { - log.Printf("stderr: %s", lineStderr) - } - - lineStdout, err := stdout.ReadString('\n') - if err != nil && err != io.EOF { - log.Printf("Err from scanning stdout is %s", err) - return fmt.Errorf("Error reading stdout from tunnel launch: %s", err) - } - if lineStdout != "" { - log.Printf("stdout: %s", lineStdout) - } - - if strings.Contains(lineStderr, "ERROR") { - // 4033: Either you don't have permission to access the instance, - // the instance doesn't exist, or the instance is stopped. - // The two sub-errors we may see while the permissions settle are - // "not authorized" and "failed to connect to backend," but after - // about a minute of retries this goes away and we're able to - // connect. - if strings.Contains(lineStderr, "4033") { - return RetryableTunnelError{lineStderr} - } else { - log.Printf("NOT RETRYABLE: %s", lineStderr) - return fmt.Errorf("Non-retryable tunnel error: %s", lineStderr) - } - } - } - - log.Printf("No error detected after tunnel launch; continuing...") - // Store successful command on step so we can access it to cancel it // later. t.cmd = cmd From dae60799bcf6598828eef5c175df9479acd57740 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 6 May 2020 15:01:40 -0700 Subject: [PATCH 19/23] fix tests --- builder/googlecompute/step_start_tunnel_test.go | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/builder/googlecompute/step_start_tunnel_test.go b/builder/googlecompute/step_start_tunnel_test.go index 176b14ac4..3cf65ac3b 100644 --- a/builder/googlecompute/step_start_tunnel_test.go +++ b/builder/googlecompute/step_start_tunnel_test.go @@ -1,17 +1,12 @@ package googlecompute import ( - "bytes" "context" "fmt" - "io" "io/ioutil" "os" - "os/exec" "runtime" - "strings" "testing" - "time" "github.com/hashicorp/packer/helper/communicator" ) @@ -73,7 +68,7 @@ gcloud compute start-iap-tunnel fakeinstance-12345 1234 --local-host-port=localh ` if runtime.GOOS == "windows" { expected = ` -call gcloud auth activate-service-account --key-file /path/to/account_file.json +call gcloud auth activate-service-account --key-file "/path/to/account_file.json" call gcloud config set project fake-project-123 call gcloud compute start-iap-tunnel fakeinstance-12345 1234 --local-host-port=localhost:8774 --zone us-central-b ` From 60600e6cf687e4a8c11bf23dce1cad44f77309c8 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 6 May 2020 15:08:49 -0700 Subject: [PATCH 20/23] make windows work with account files containing spaces --- builder/googlecompute/step_start_tunnel.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/googlecompute/step_start_tunnel.go b/builder/googlecompute/step_start_tunnel.go index 6b774a57e..739263351 100644 --- a/builder/googlecompute/step_start_tunnel.go +++ b/builder/googlecompute/step_start_tunnel.go @@ -191,7 +191,7 @@ gcloud config set project {{.ProjectID}} ` if runtime.GOOS == "windows" { launchTemplate = ` -call gcloud auth activate-service-account --key-file {{.AccountFile}} +call gcloud auth activate-service-account --key-file "{{.AccountFile}}" call gcloud config set project {{.ProjectID}} call {{.Args}} ` From 81e043f2b05d34b39eeefb505dbf319ac2f2fefc Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Wed, 6 May 2020 15:18:21 -0700 Subject: [PATCH 21/23] fix windows tests --- builder/googlecompute/config_test.go | 9 ++++++--- builder/googlecompute/step_start_tunnel_test.go | 4 +++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/builder/googlecompute/config_test.go b/builder/googlecompute/config_test.go index d7dd11ebc..b7c46ea56 100644 --- a/builder/googlecompute/config_test.go +++ b/builder/googlecompute/config_test.go @@ -399,17 +399,20 @@ func TestConfigPrepareIAP(t *testing.T) { t.Fatalf("Shouldn't have errors. Err = %s", err) } - if c.IAPHashBang != "/bin/sh" { - t.Fatalf("IAP hashbang didn't default correctly to /bin/sh.") - } if runtime.GOOS == "windows" { if c.IAPExt != ".cmd" { t.Fatalf("IAP tempfile extension didn't default correctly to .cmd") } + if c.IAPHashBang != "" { + t.Fatalf("IAP hashbang didn't default correctly to nothing.") + } } else { if c.IAPExt != "" { t.Fatalf("IAP tempfile extension should default to empty on unix mahcines") } + if c.IAPHashBang != "/bin/sh" { + t.Fatalf("IAP hashbang didn't default correctly to /bin/sh.") + } } if c.Comm.SSHHost != "localhost" { t.Fatalf("Didn't correctly override the ssh host.") diff --git a/builder/googlecompute/step_start_tunnel_test.go b/builder/googlecompute/step_start_tunnel_test.go index 3cf65ac3b..47813df8e 100644 --- a/builder/googlecompute/step_start_tunnel_test.go +++ b/builder/googlecompute/step_start_tunnel_test.go @@ -67,7 +67,9 @@ gcloud config set project fake-project-123 gcloud compute start-iap-tunnel fakeinstance-12345 1234 --local-host-port=localhost:8774 --zone us-central-b ` if runtime.GOOS == "windows" { - expected = ` + // in real life you'd not be passing a HashBang here, but GIGO. + expected = `#!/bin/bash + call gcloud auth activate-service-account --key-file "/path/to/account_file.json" call gcloud config set project fake-project-123 call gcloud compute start-iap-tunnel fakeinstance-12345 1234 --local-host-port=localhost:8774 --zone us-central-b From dde162622d0787fae282d96914cc038d0e236386 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 8 May 2020 10:20:47 -0700 Subject: [PATCH 22/23] use local port in listener config, when set --- builder/googlecompute/step_start_tunnel.go | 36 +++++++++++++--------- 1 file changed, 22 insertions(+), 14 deletions(-) diff --git a/builder/googlecompute/step_start_tunnel.go b/builder/googlecompute/step_start_tunnel.go index 739263351..f8c0bc571 100644 --- a/builder/googlecompute/step_start_tunnel.go +++ b/builder/googlecompute/step_start_tunnel.go @@ -138,25 +138,33 @@ type StepStartTunnel struct { } func (s *StepStartTunnel) ConfigureLocalHostPort(ctx context.Context) error { + minPortNumber, maxPortNumber := 8000, 9000 + if s.IAPConf.IAPLocalhostPort == 0 { + minPortNumber = s.IAPConf.IAPLocalhostPort + maxPortNumber = minPortNumber + log.Printf("Using TCP port for %d IAP proxy", s.IAPConf.IAPLocalhostPort) + } else { log.Printf("Finding an available TCP port for IAP proxy") - l, err := net.ListenRangeConfig{ - Min: 8000, - Max: 9000, - Addr: "0.0.0.0", - Network: "tcp", - }.Listen(ctx) + } - if err != nil { - err := fmt.Errorf("error finding an available port to initiate a session tunnel: %s", err) - return err - } + l, err := net.ListenRangeConfig{ + Min: minPortNumber, + Max: maxPortNumber, + Addr: "0.0.0.0", + Network: "tcp", + }.Listen(ctx) - s.IAPConf.IAPLocalhostPort = l.Port - l.Close() - log.Printf("Setting up proxy to listen on localhost at %d", - s.IAPConf.IAPLocalhostPort) + if err != nil { + err := fmt.Errorf("error finding an available port to initiate a session tunnel: %s", err) + return err } + + s.IAPConf.IAPLocalhostPort = l.Port + l.Close() + log.Printf("Setting up proxy to listen on localhost at %d", + s.IAPConf.IAPLocalhostPort) + return nil } From a55b73473a25b366f22e2413cd6aa9fd95d3ca38 Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Fri, 8 May 2020 10:24:28 -0700 Subject: [PATCH 23/23] fix logic --- builder/googlecompute/step_start_tunnel.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/builder/googlecompute/step_start_tunnel.go b/builder/googlecompute/step_start_tunnel.go index f8c0bc571..434d7fe4e 100644 --- a/builder/googlecompute/step_start_tunnel.go +++ b/builder/googlecompute/step_start_tunnel.go @@ -140,7 +140,7 @@ type StepStartTunnel struct { func (s *StepStartTunnel) ConfigureLocalHostPort(ctx context.Context) error { minPortNumber, maxPortNumber := 8000, 9000 - if s.IAPConf.IAPLocalhostPort == 0 { + if s.IAPConf.IAPLocalhostPort != 0 { minPortNumber = s.IAPConf.IAPLocalhostPort maxPortNumber = minPortNumber log.Printf("Using TCP port for %d IAP proxy", s.IAPConf.IAPLocalhostPort)