From a05c554d14dcd363279da3d85ddc9eb9de1250bb Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Fri, 27 Nov 2020 18:13:10 +0000 Subject: [PATCH 1/5] Amend commit author for license pass --- builder/googlecompute/config.go | 5 ++ builder/googlecompute/config.hcl2spec.go | 2 + builder/googlecompute/config_test.go | 11 +++ builder/googlecompute/step_create_instance.go | 69 +++++++++++++++++++ 4 files changed, 87 insertions(+) diff --git a/builder/googlecompute/config.go b/builder/googlecompute/config.go index cdf4581e5..7f4325a20 100644 --- a/builder/googlecompute/config.go +++ b/builder/googlecompute/config.go @@ -287,6 +287,11 @@ type Config struct { VaultGCPOauthEngine string `mapstructure:"vault_gcp_oauth_engine"` // The zone in which to launch the instance used to create the image. // Example: "us-central1-a" + + // The time to wait between instance creation and adding SSH keys. + // Example value: `5m`. + WaitToAddSSHKeys time.Duration `mapstructure:"wait_to_add_ssh_keys"` + Zone string `mapstructure:"zone" required:"true"` account *ServiceAccount diff --git a/builder/googlecompute/config.hcl2spec.go b/builder/googlecompute/config.hcl2spec.go index c235087f0..6c98d8a2a 100644 --- a/builder/googlecompute/config.hcl2spec.go +++ b/builder/googlecompute/config.hcl2spec.go @@ -117,6 +117,7 @@ type FlatConfig struct { UseInternalIP *bool `mapstructure:"use_internal_ip" required:"false" cty:"use_internal_ip" hcl:"use_internal_ip"` UseOSLogin *bool `mapstructure:"use_os_login" required:"false" cty:"use_os_login" hcl:"use_os_login"` VaultGCPOauthEngine *string `mapstructure:"vault_gcp_oauth_engine" cty:"vault_gcp_oauth_engine" hcl:"vault_gcp_oauth_engine"` + WaitToAddSSHKeys *string `mapstructure:"wait_to_add_ssh_keys" cty:"wait_to_add_ssh_keys" hcl:"wait_to_add_ssh_keys"` Zone *string `mapstructure:"zone" required:"true" cty:"zone" hcl:"zone"` } @@ -240,6 +241,7 @@ func (*FlatConfig) HCL2Spec() map[string]hcldec.Spec { "use_internal_ip": &hcldec.AttrSpec{Name: "use_internal_ip", Type: cty.Bool, Required: false}, "use_os_login": &hcldec.AttrSpec{Name: "use_os_login", Type: cty.Bool, Required: false}, "vault_gcp_oauth_engine": &hcldec.AttrSpec{Name: "vault_gcp_oauth_engine", Type: cty.String, Required: false}, + "wait_to_add_ssh_keys": &hcldec.AttrSpec{Name: "wait_to_add_ssh_keys", Type: cty.String, Required: false}, "zone": &hcldec.AttrSpec{Name: "zone", Type: cty.String, Required: false}, } return s diff --git a/builder/googlecompute/config_test.go b/builder/googlecompute/config_test.go index dab12c9f9..7e73ec14d 100644 --- a/builder/googlecompute/config_test.go +++ b/builder/googlecompute/config_test.go @@ -84,6 +84,17 @@ func TestConfigPrepare(t *testing.T) { false, }, + { + "wait_to_add_ssh_keys", + "SO BAD", + true, + }, + { + "wait_to_add_ssh_keys", + "5s", + false, + }, + { "state_timeout", "SO BAD", diff --git a/builder/googlecompute/step_create_instance.go b/builder/googlecompute/step_create_instance.go index 371fee64a..9e9eb5094 100644 --- a/builder/googlecompute/step_create_instance.go +++ b/builder/googlecompute/step_create_instance.go @@ -85,6 +85,43 @@ func (c *Config) createInstanceMetadata(sourceImage *Image, sshPublicKey string) return instanceMetadata, nil } +func (s *StepCreateInstance) addMetadataToInstance(ctx context.Context, errCh chan<- error, name, state multistep.StateBag, metadata map[string]string) { + + c := state.Get("config").(*Config) + d := state.Get("driver").(Driver) + + instance, err := d.service.Instances.Get(d.projectId, c.Zone, name).Do() + if err != nil { + errCh <- err + return + } + instance.Metadata.Items = append(instance.Metadata.Items, &compute.MetadataItems{Key: "windows-keys", Value: &sshPublicKey}) + + op, err := d.service.Instances.SetMetadata(d.projectId, zone, name, &compute.Metadata{ + Fingerprint: instance.Metadata.Fingerprint, + Items: instance.Metadata.Items, + }).Do() + + if err != nil { + errCh <- err + return + } + + newErrCh := make(chan error, 1) + go waitForState(newErrCh, "DONE", d.refreshZoneOp(zone, op)) + + select { + case err = <-newErrCh: + case <-time.After(time.Second * 30): + err = errors.New("time out while waiting for SSH public key to be added to instance") + } + + if err != nil { + errCh <- err + return + } +} + func getImage(c *Config, d Driver) (*Image, error) { name := c.SourceImageFamily fromFamily := true @@ -139,6 +176,11 @@ func (s *StepCreateInstance) Run(ctx context.Context, state multistep.StateBag) return multistep.ActionHalt } + if c.WaitToAddSSHKeys == 0 { + ui.Message("Adding SSH keys during instance creation...") + metadata = make(map[string]string) + } + errCh, err = d.RunInstance(&InstanceConfig{ AcceleratorType: c.AcceleratorType, AcceleratorCount: c.AcceleratorCount, @@ -199,9 +241,36 @@ func (s *StepCreateInstance) Run(ctx context.Context, state multistep.StateBag) // instance id inside of the provisioners, used in step_provision. state.Put("instance_id", name) + if c.WaitToAddSSHKeys > 0 { + ui.Say(fmt.Sprintf("Waiting %s before adding SSH keys...", + c.WaitToAddSSHKeys.String())) + cancelled := s.wait(c.WaitToAddSSHKeys, ctx) + if cancelled { + return multistep.ActionHalt + } + + metadata, errs = s.addMetadataToInstance(metadata, d, c) + if errs != nil { + state.Put("error", errs.Error()) + ui.Error(errs.Error()) + return multistep.ActionHalt + } + } + return multistep.ActionContinue } +func (s *StepCreateInstance) wait(waitLen time.Duration, ctx context.Context) bool { + // Use a select to determine if we get cancelled during the wait + select { + case <-ctx.Done(): + return true + case <-time.After(waitLen): + } + ui.Message("Wait over; adding SSH keys to instance...") + return false +} + // Cleanup destroys the GCE instance created during the image creation process. func (s *StepCreateInstance) Cleanup(state multistep.StateBag) { nameRaw, ok := state.GetOk("instance_name") From 3ab9bae79c594ecfc13e116e040292cffa917cbc Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Tue, 1 Dec 2020 14:50:36 +0000 Subject: [PATCH 2/5] Amend commit author for license pass $ make test find: -executable: unknown primary or operator find: -executable: unknown primary or operator ==> Checking that only certain files are executable... Check passed. ok github.com/hashicorp/packer 0.098s ok github.com/hashicorp/packer/builder/alicloud/ecs 70.102s ? github.com/hashicorp/packer/builder/alicloud/version [no test files] ok github.com/hashicorp/packer/builder/amazon/chroot 0.076s ok github.com/hashicorp/packer/builder/amazon/common 0.052s ? github.com/hashicorp/packer/builder/amazon/common/awserrors [no test files] ? github.com/hashicorp/packer/builder/amazon/common/ssm [no test files] ok github.com/hashicorp/packer/builder/amazon/ebs 0.053s ? github.com/hashicorp/packer/builder/amazon/ebs/acceptance [no test files] ok github.com/hashicorp/packer/builder/amazon/ebssurrogate 0.057s ok github.com/hashicorp/packer/builder/amazon/ebsvolume 0.052s ok github.com/hashicorp/packer/builder/amazon/instance 0.092s ? github.com/hashicorp/packer/builder/amazon/version [no test files] ok github.com/hashicorp/packer/builder/azure/arm 8.929s ok github.com/hashicorp/packer/builder/azure/chroot 0.071s ok github.com/hashicorp/packer/builder/azure/common 0.032s ok github.com/hashicorp/packer/builder/azure/common/client 2.768s ? github.com/hashicorp/packer/builder/azure/common/constants [no test files] ? github.com/hashicorp/packer/builder/azure/common/lin [no test files] ? github.com/hashicorp/packer/builder/azure/common/logutil [no test files] ok github.com/hashicorp/packer/builder/azure/common/template 0.038s ok github.com/hashicorp/packer/builder/azure/dtl 1.508s ok github.com/hashicorp/packer/builder/azure/pkcs12 0.250s ok github.com/hashicorp/packer/builder/azure/pkcs12/rc2 0.018s ? github.com/hashicorp/packer/builder/azure/version [no test files] ok github.com/hashicorp/packer/builder/cloudstack 0.074s ? github.com/hashicorp/packer/builder/cloudstack/version [no test files] ok github.com/hashicorp/packer/builder/digitalocean 0.078s ? github.com/hashicorp/packer/builder/digitalocean/version [no test files] ok github.com/hashicorp/packer/builder/docker 0.054s ? github.com/hashicorp/packer/builder/docker/version [no test files] ok github.com/hashicorp/packer/builder/file 0.037s ? github.com/hashicorp/packer/builder/file/version [no test files] ok github.com/hashicorp/packer/builder/googlecompute 7.982s ? github.com/hashicorp/packer/builder/googlecompute/version [no test files] ok github.com/hashicorp/packer/builder/hcloud 0.037s ? github.com/hashicorp/packer/builder/hcloud/version [no test files] ok github.com/hashicorp/packer/builder/hyperone 0.031s ? github.com/hashicorp/packer/builder/hyperone/version [no test files] ok github.com/hashicorp/packer/builder/hyperv/common 0.042s ok github.com/hashicorp/packer/builder/hyperv/common/powershell 0.017s ok github.com/hashicorp/packer/builder/hyperv/common/powershell/hyperv 0.027s ok github.com/hashicorp/packer/builder/hyperv/iso 0.193s ? github.com/hashicorp/packer/builder/hyperv/version [no test files] ok github.com/hashicorp/packer/builder/hyperv/vmcx 0.160s ok github.com/hashicorp/packer/builder/jdcloud 0.038s ? github.com/hashicorp/packer/builder/jdcloud/version [no test files] ok github.com/hashicorp/packer/builder/linode 0.074s ? github.com/hashicorp/packer/builder/linode/version [no test files] ok github.com/hashicorp/packer/builder/lxc 0.038s ? github.com/hashicorp/packer/builder/lxc/version [no test files] ok github.com/hashicorp/packer/builder/lxd 0.033s ? github.com/hashicorp/packer/builder/lxd/version [no test files] ok github.com/hashicorp/packer/builder/ncloud 0.038s ? github.com/hashicorp/packer/builder/ncloud/version [no test files] ok github.com/hashicorp/packer/builder/null 0.036s ? github.com/hashicorp/packer/builder/null/version [no test files] ok github.com/hashicorp/packer/builder/oneandone 0.038s ? github.com/hashicorp/packer/builder/oneandone/version [no test files] ok github.com/hashicorp/packer/builder/openstack 0.048s ? github.com/hashicorp/packer/builder/openstack/version [no test files] ok github.com/hashicorp/packer/builder/oracle/classic 0.055s ? github.com/hashicorp/packer/builder/oracle/common [no test files] ok github.com/hashicorp/packer/builder/oracle/oci 6.349s ? github.com/hashicorp/packer/builder/oracle/version [no test files] ok github.com/hashicorp/packer/builder/osc/bsu 0.045s ok github.com/hashicorp/packer/builder/osc/bsusurrogate 0.043s ok github.com/hashicorp/packer/builder/osc/bsuvolume 0.048s ok github.com/hashicorp/packer/builder/osc/chroot 0.035s ok github.com/hashicorp/packer/builder/osc/common 0.030s ok github.com/hashicorp/packer/builder/osc/common/retry 0.018s ? github.com/hashicorp/packer/builder/osc/version [no test files] ok github.com/hashicorp/packer/builder/parallels/common 1.546s ok github.com/hashicorp/packer/builder/parallels/iso 0.047s ok github.com/hashicorp/packer/builder/parallels/pvm 0.044s ? github.com/hashicorp/packer/builder/parallels/version [no test files] ok github.com/hashicorp/packer/builder/profitbricks 0.046s ? github.com/hashicorp/packer/builder/profitbricks/version [no test files] ? github.com/hashicorp/packer/builder/proxmox [no test files] ? github.com/hashicorp/packer/builder/proxmox/clone [no test files] ok github.com/hashicorp/packer/builder/proxmox/common 0.070s ok github.com/hashicorp/packer/builder/proxmox/iso 0.072s ? github.com/hashicorp/packer/builder/proxmox/version [no test files] ok github.com/hashicorp/packer/builder/qemu 0.088s ? github.com/hashicorp/packer/builder/qemu/version [no test files] ok github.com/hashicorp/packer/builder/scaleway 0.062s ? github.com/hashicorp/packer/builder/scaleway/version [no test files] ok github.com/hashicorp/packer/builder/tencentcloud/cvm 0.037s ? github.com/hashicorp/packer/builder/tencentcloud/version [no test files] ok github.com/hashicorp/packer/builder/triton 0.057s ? github.com/hashicorp/packer/builder/triton/version [no test files] ok github.com/hashicorp/packer/builder/ucloud/common 0.039s ok github.com/hashicorp/packer/builder/ucloud/uhost 0.045s ? github.com/hashicorp/packer/builder/ucloud/version [no test files] ok github.com/hashicorp/packer/builder/vagrant 0.046s ? github.com/hashicorp/packer/builder/vagrant/version [no test files] ok github.com/hashicorp/packer/builder/virtualbox/common 2.546s ok github.com/hashicorp/packer/builder/virtualbox/iso 0.053s ? github.com/hashicorp/packer/builder/virtualbox/iso/acceptance [no test files] ok github.com/hashicorp/packer/builder/virtualbox/ovf 0.043s ? github.com/hashicorp/packer/builder/virtualbox/version [no test files] ? github.com/hashicorp/packer/builder/virtualbox/vm [no test files] ok github.com/hashicorp/packer/builder/vmware/common 5.228s ok github.com/hashicorp/packer/builder/vmware/iso 0.104s ? github.com/hashicorp/packer/builder/vmware/version [no test files] ok github.com/hashicorp/packer/builder/vmware/vmx 0.055s ok github.com/hashicorp/packer/builder/vsphere/clone 0.072s ok github.com/hashicorp/packer/builder/vsphere/common 0.037s ? github.com/hashicorp/packer/builder/vsphere/common/testing [no test files] ok github.com/hashicorp/packer/builder/vsphere/driver 0.443s ? github.com/hashicorp/packer/builder/vsphere/examples/driver [no test files] ok github.com/hashicorp/packer/builder/vsphere/iso 0.063s ? github.com/hashicorp/packer/builder/vsphere/version [no test files] ok github.com/hashicorp/packer/builder/yandex 0.098s ? github.com/hashicorp/packer/builder/yandex/version [no test files] ? github.com/hashicorp/packer/cmd/generate-fixer-deprecations [no test files] ? github.com/hashicorp/packer/cmd/mapstructure-to-hcl2 [no test files] ? github.com/hashicorp/packer/cmd/snippet-extractor [no test files] ? github.com/hashicorp/packer/cmd/ssh-keygen [no test files] ? github.com/hashicorp/packer/cmd/struct-markdown [no test files] ok github.com/hashicorp/packer/command 3.717s ? github.com/hashicorp/packer/command/enumflag [no test files] ok github.com/hashicorp/packer/command/flag-kv 0.019s ok github.com/hashicorp/packer/command/flag-slice 0.018s ok github.com/hashicorp/packer/fix 0.026s ok github.com/hashicorp/packer/hcl2template 0.281s ? github.com/hashicorp/packer/hcl2template/addrs [no test files] ok github.com/hashicorp/packer/hcl2template/function 0.028s ? github.com/hashicorp/packer/hcl2template/internal [no test files] ? github.com/hashicorp/packer/hcl2template/repl [no test files] ok github.com/hashicorp/packer/hcl2template/shim 0.019s ok github.com/hashicorp/packer/helper/builder/testing 0.027s ok github.com/hashicorp/packer/helper/communicator 0.034s ok github.com/hashicorp/packer/helper/communicator/ssh 4.204s ok github.com/hashicorp/packer/helper/communicator/sshkey 2.744s ? github.com/hashicorp/packer/helper/tests [no test files] ? github.com/hashicorp/packer/helper/tests/acc [no test files] ? github.com/hashicorp/packer/helper/wrappedreadline [no test files] ? github.com/hashicorp/packer/helper/wrappedstreams [no test files] ok github.com/hashicorp/packer/packer 0.221s ok github.com/hashicorp/packer/packer/plugin 0.417s ok github.com/hashicorp/packer/packer/rpc 0.059s ok github.com/hashicorp/packer/packer-plugin-sdk/adapter 0.063s ok github.com/hashicorp/packer/packer-plugin-sdk/bootcommand 2.778s ok github.com/hashicorp/packer/packer-plugin-sdk/chroot 0.038s ? github.com/hashicorp/packer/packer-plugin-sdk/common [no test files] ? github.com/hashicorp/packer/packer-plugin-sdk/filelock [no test files] ok github.com/hashicorp/packer/packer-plugin-sdk/guestexec 0.029s ok github.com/hashicorp/packer/packer-plugin-sdk/iochan 0.019s ok github.com/hashicorp/packer/packer-plugin-sdk/json 0.017s [no tests to run] ok github.com/hashicorp/packer/packer-plugin-sdk/multistep 0.126s ok github.com/hashicorp/packer/packer-plugin-sdk/multistep/commonsteps 0.136s ok github.com/hashicorp/packer/packer-plugin-sdk/net 1.041s ok github.com/hashicorp/packer/packer-plugin-sdk/packerbuilderdata 0.018s ? github.com/hashicorp/packer/packer-plugin-sdk/random [no test files] ok github.com/hashicorp/packer/packer-plugin-sdk/retry 0.021s ok github.com/hashicorp/packer/packer-plugin-sdk/sdk-internals/communicator/none 0.026s ok github.com/hashicorp/packer/packer-plugin-sdk/sdk-internals/communicator/ssh 0.095s ok github.com/hashicorp/packer/packer-plugin-sdk/sdk-internals/communicator/winrm 0.062s ok github.com/hashicorp/packer/packer-plugin-sdk/shell 0.023s ok github.com/hashicorp/packer/packer-plugin-sdk/shell-local 0.045s ok github.com/hashicorp/packer/packer-plugin-sdk/shell-local/localexec 0.031s ok github.com/hashicorp/packer/packer-plugin-sdk/shutdowncommand 0.032s ok github.com/hashicorp/packer/packer-plugin-sdk/template 0.041s ok github.com/hashicorp/packer/packer-plugin-sdk/template/config 0.033s ok github.com/hashicorp/packer/packer-plugin-sdk/template/interpolate 0.032s ok github.com/hashicorp/packer/packer-plugin-sdk/template/interpolate/aws/secretsmanager 0.030s ? github.com/hashicorp/packer/packer-plugin-sdk/tmp [no test files] ok github.com/hashicorp/packer/packer-plugin-sdk/useragent 0.018s ok github.com/hashicorp/packer/packer-plugin-sdk/uuid 0.019s ? github.com/hashicorp/packer/packer-plugin-sdk/version [no test files] ok github.com/hashicorp/packer/plugin/example 0.040s [no tests to run] ? github.com/hashicorp/packer/post-processor/alicloud-import [no test files] ? github.com/hashicorp/packer/post-processor/alicloud-import/version [no test files] ? github.com/hashicorp/packer/post-processor/amazon-import [no test files] ? github.com/hashicorp/packer/post-processor/amazon-import/version [no test files] ? github.com/hashicorp/packer/post-processor/artifice [no test files] ? github.com/hashicorp/packer/post-processor/artifice/version [no test files] ok github.com/hashicorp/packer/post-processor/checksum 0.032s ? github.com/hashicorp/packer/post-processor/checksum/version [no test files] ok github.com/hashicorp/packer/post-processor/compress 0.046s ? github.com/hashicorp/packer/post-processor/compress/version [no test files] ok github.com/hashicorp/packer/post-processor/digitalocean-import 0.038s ? github.com/hashicorp/packer/post-processor/digitalocean-import/version [no test files] ok github.com/hashicorp/packer/post-processor/docker-import 0.036s ? github.com/hashicorp/packer/post-processor/docker-import/version [no test files] ok github.com/hashicorp/packer/post-processor/docker-push 0.037s ? github.com/hashicorp/packer/post-processor/docker-push/version [no test files] ok github.com/hashicorp/packer/post-processor/docker-save 0.038s ? github.com/hashicorp/packer/post-processor/docker-save/version [no test files] ok github.com/hashicorp/packer/post-processor/docker-tag 0.042s ? github.com/hashicorp/packer/post-processor/docker-tag/version [no test files] ? github.com/hashicorp/packer/post-processor/exoscale-import [no test files] ? github.com/hashicorp/packer/post-processor/exoscale-import/version [no test files] ok github.com/hashicorp/packer/post-processor/googlecompute-export 0.044s [no tests to run] ? github.com/hashicorp/packer/post-processor/googlecompute-export/version [no test files] ok github.com/hashicorp/packer/post-processor/googlecompute-import 0.035s ? github.com/hashicorp/packer/post-processor/googlecompute-import/version [no test files] ? github.com/hashicorp/packer/post-processor/manifest [no test files] ? github.com/hashicorp/packer/post-processor/manifest/version [no test files] ok github.com/hashicorp/packer/post-processor/shell-local 0.047s ? github.com/hashicorp/packer/post-processor/shell-local/version [no test files] ? github.com/hashicorp/packer/post-processor/ucloud-import [no test files] ? github.com/hashicorp/packer/post-processor/ucloud-import/version [no test files] ok github.com/hashicorp/packer/post-processor/vagrant 0.045s ? github.com/hashicorp/packer/post-processor/vagrant/version [no test files] ok github.com/hashicorp/packer/post-processor/vagrant-cloud 0.089s ? github.com/hashicorp/packer/post-processor/vagrant-cloud/version [no test files] ok github.com/hashicorp/packer/post-processor/vsphere 0.038s ? github.com/hashicorp/packer/post-processor/vsphere/version [no test files] ok github.com/hashicorp/packer/post-processor/vsphere-template 0.047s ? github.com/hashicorp/packer/post-processor/vsphere-template/version [no test files] ok github.com/hashicorp/packer/post-processor/yandex-export 0.055s ? github.com/hashicorp/packer/post-processor/yandex-export/version [no test files] ok github.com/hashicorp/packer/post-processor/yandex-import 0.052s ? github.com/hashicorp/packer/post-processor/yandex-import/version [no test files] ok github.com/hashicorp/packer/provisioner/ansible 0.469s ? github.com/hashicorp/packer/provisioner/ansible/version [no test files] ok github.com/hashicorp/packer/provisioner/ansible-local 0.055s ? github.com/hashicorp/packer/provisioner/ansible-local/version [no test files] ? github.com/hashicorp/packer/provisioner/azure-dtlartifact [no test files] ? github.com/hashicorp/packer/provisioner/azure-dtlartifact/version [no test files] ? github.com/hashicorp/packer/provisioner/breakpoint [no test files] ? github.com/hashicorp/packer/provisioner/breakpoint/version [no test files] ok github.com/hashicorp/packer/provisioner/chef-client 0.056s ? github.com/hashicorp/packer/provisioner/chef-client/version [no test files] ok github.com/hashicorp/packer/provisioner/chef-solo 0.045s ? github.com/hashicorp/packer/provisioner/chef-solo/version [no test files] ok github.com/hashicorp/packer/provisioner/converge 0.033s ? github.com/hashicorp/packer/provisioner/converge/version [no test files] ok github.com/hashicorp/packer/provisioner/file 0.051s ? github.com/hashicorp/packer/provisioner/file/version [no test files] ok github.com/hashicorp/packer/provisioner/inspec 0.050s ? github.com/hashicorp/packer/provisioner/inspec/version [no test files] ok github.com/hashicorp/packer/provisioner/powershell 2.095s ? github.com/hashicorp/packer/provisioner/powershell/version [no test files] ok github.com/hashicorp/packer/provisioner/puppet-masterless 0.049s ? github.com/hashicorp/packer/provisioner/puppet-masterless/version [no test files] ok github.com/hashicorp/packer/provisioner/puppet-server 0.043s ? github.com/hashicorp/packer/provisioner/puppet-server/version [no test files] ok github.com/hashicorp/packer/provisioner/salt-masterless 0.054s ? github.com/hashicorp/packer/provisioner/salt-masterless/version [no test files] ok github.com/hashicorp/packer/provisioner/shell 0.085s ? github.com/hashicorp/packer/provisioner/shell/version [no test files] ok github.com/hashicorp/packer/provisioner/shell-local 0.071s ? github.com/hashicorp/packer/provisioner/shell-local/version [no test files] ok github.com/hashicorp/packer/provisioner/sleep 0.027s ? github.com/hashicorp/packer/provisioner/sleep/version [no test files] ok github.com/hashicorp/packer/provisioner/windows-restart 0.051s ? github.com/hashicorp/packer/provisioner/windows-restart/version [no test files] ok github.com/hashicorp/packer/provisioner/windows-shell 0.039s ? github.com/hashicorp/packer/provisioner/windows-shell/version [no test files] ? github.com/hashicorp/packer/scripts [no test files] ? github.com/hashicorp/packer/version [no test files] --- builder/googlecompute/config.go | 14 +- builder/googlecompute/driver.go | 3 + builder/googlecompute/driver_gce.go | 40 ++++++ builder/googlecompute/driver_mock.go | 21 +++ builder/googlecompute/startup.go | 4 +- builder/googlecompute/step_create_instance.go | 125 ++++++++---------- .../step_create_instance_test.go | 108 +++++++++++++-- .../googlecompute/Config-not-required.mdx | 9 ++ 8 files changed, 240 insertions(+), 84 deletions(-) diff --git a/builder/googlecompute/config.go b/builder/googlecompute/config.go index 7f4325a20..41c36d517 100644 --- a/builder/googlecompute/config.go +++ b/builder/googlecompute/config.go @@ -285,13 +285,17 @@ type Config struct { // https://www.vaultproject.io/docs/commands/#environment-variables // Example:`"vault_gcp_oauth_engine": "gcp/token/my-project-editor",` VaultGCPOauthEngine string `mapstructure:"vault_gcp_oauth_engine"` - // The zone in which to launch the instance used to create the image. - // Example: "us-central1-a" - - // The time to wait between instance creation and adding SSH keys. + // The time to wait between the creation of the instance used to create the image, + // and the addition of SSH configuration, including SSH keys, to that instance. + // The delay is intended to protect packer from anything in the instance boot + // sequence that has potential to disrupt the creation of SSH configuration + // (e.g. SSH user creation, SSH key creation) on the instance. + // Note: All other instance metadata, including startup scripts, are still added to the instance + // during it's creation. // Example value: `5m`. WaitToAddSSHKeys time.Duration `mapstructure:"wait_to_add_ssh_keys"` - + // The zone in which to launch the instance used to create the image. + // Example: "us-central1-a" Zone string `mapstructure:"zone" required:"true"` account *ServiceAccount diff --git a/builder/googlecompute/driver.go b/builder/googlecompute/driver.go index 65243250b..96fef6574 100644 --- a/builder/googlecompute/driver.go +++ b/builder/googlecompute/driver.go @@ -69,6 +69,9 @@ type Driver interface { // DeleteOSLoginSSHKey deletes the SSH public key for OSLogin with the given key. DeleteOSLoginSSHKey(user, fingerprint string) error + + // Add to the instance metadata for the existing instance + AddToInstanceMetadata(zone string, name string, metadata map[string]string) (<-chan error, error) } type InstanceConfig struct { diff --git a/builder/googlecompute/driver_gce.go b/builder/googlecompute/driver_gce.go index 5ade084c2..8678fd7eb 100644 --- a/builder/googlecompute/driver_gce.go +++ b/builder/googlecompute/driver_gce.go @@ -727,3 +727,43 @@ func waitForState(errCh chan<- error, target string, refresh stateRefreshFunc) e errCh <- err return err } + +func (d *driverGCE) AddToInstanceMetadata(zone string, name string, metadata map[string]string) (<-chan error, error) { + + instance, err := d.service.Instances.Get(d.projectId, zone, name).Do() + if err != nil { + return nil, err + } + + // Build up the metadata + metadataForInstance := make([]*compute.MetadataItems, len(metadata)) + for k, v := range metadata { + vCopy := v + metadataForInstance = append(metadataForInstance, &compute.MetadataItems{ + Key: k, + Value: &vCopy, + }) + } + + instance.Metadata.Items = append(instance.Metadata.Items, metadataForInstance...) + + op, err := d.service.Instances.SetMetadata(d.projectId, zone, name, &compute.Metadata{ + Fingerprint: instance.Metadata.Fingerprint, + Items: instance.Metadata.Items, + }).Do() + + if err != nil { + return nil, err + } + + newErrCh := make(chan error, 1) + go waitForState(newErrCh, "DONE", d.refreshZoneOp(zone, op)) + + select { + case err = <-newErrCh: + case <-time.After(time.Second * 30): + err = errors.New("time out while waiting for SSH keys to be added to instance") + } + + return newErrCh, err +} \ No newline at end of file diff --git a/builder/googlecompute/driver_mock.go b/builder/googlecompute/driver_mock.go index da5599bc9..bd455841b 100644 --- a/builder/googlecompute/driver_mock.go +++ b/builder/googlecompute/driver_mock.go @@ -88,6 +88,12 @@ type DriverMock struct { WaitForInstanceZone string WaitForInstanceName string WaitForInstanceErrCh <-chan error + + AddToInstanceMetadataZone string + AddToInstanceMetadataName string + AddToInstanceMetadataKVPairs map[string]string + AddToInstanceMetadataErrCh <-chan error + AddToInstanceMetadataErr error } func (d *DriverMock) CreateImage(name, description, family, zone, disk string, image_labels map[string]string, image_licenses []string, image_encryption_key *compute.CustomerEncryptionKey, imageStorageLocations []string) (<-chan *Image, <-chan error) { @@ -288,3 +294,18 @@ func (d *DriverMock) ImportOSLoginSSHKey(user, key string) (*oslogin.LoginProfil func (d *DriverMock) DeleteOSLoginSSHKey(user, fingerprint string) error { return nil } + +func (d *DriverMock) AddToInstanceMetadata(zone string, name string, metadata map[string]string) (<-chan error, error) { + d.AddToInstanceMetadataZone = zone + d.AddToInstanceMetadataName = name + d.AddToInstanceMetadataKVPairs = metadata + + resultCh := d.AddToInstanceMetadataErrCh + if resultCh == nil { + ch := make(chan error) + close(ch) + resultCh = ch + } + + return resultCh, d.AddToInstanceMetadataErr +} \ No newline at end of file diff --git a/builder/googlecompute/startup.go b/builder/googlecompute/startup.go index 743778a2c..bdb147479 100644 --- a/builder/googlecompute/startup.go +++ b/builder/googlecompute/startup.go @@ -1,7 +1,7 @@ package googlecompute import ( - "fmt" + "fmt" ) const StartupScriptKey string = "startup-script" @@ -57,4 +57,4 @@ SetMetadata %s %s exit $RETVAL `, StartupWrappedScriptKey, StartupScriptStatusKey, StartupScriptStatusDone) -var StartupScriptWindows string = "" +var StartupScriptWindows string = "" \ No newline at end of file diff --git a/builder/googlecompute/step_create_instance.go b/builder/googlecompute/step_create_instance.go index 9e9eb5094..fa52ae8b6 100644 --- a/builder/googlecompute/step_create_instance.go +++ b/builder/googlecompute/step_create_instance.go @@ -7,6 +7,7 @@ import ( "io/ioutil" "strings" "time" + "log" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/packer-plugin-sdk/multistep" @@ -17,14 +18,23 @@ type StepCreateInstance struct { Debug bool } -func (c *Config) createInstanceMetadata(sourceImage *Image, sshPublicKey string) (map[string]string, error) { - instanceMetadata := make(map[string]string) +func (c *Config) createInstanceMetadata(sourceImage *Image, sshPublicKey string) (map[string]string, map[string]string, error) { + + instanceMetadataNoSSHKeys := make(map[string]string) + instanceMetadataSSHKeys := make(map[string]string) + + sshMetaKey := "ssh-keys" + var err error var errs *packer.MultiError // Copy metadata from config. for k, v := range c.Metadata { - instanceMetadata[k] = v + if k == sshMetaKey { + instanceMetadataSSHKeys[k] = v + } else { + instanceMetadataNoSSHKeys[k] = v + } } // Merge any existing ssh keys with our public key, unless there is no @@ -34,40 +44,40 @@ func (c *Config) createInstanceMetadata(sourceImage *Image, sshPublicKey string) sshMetaKey := "ssh-keys" sshPublicKey = strings.TrimSuffix(sshPublicKey, "\n") sshKeys := fmt.Sprintf("%s:%s %s", c.Comm.SSHUsername, sshPublicKey, c.Comm.SSHUsername) - if confSshKeys, exists := instanceMetadata[sshMetaKey]; exists { - sshKeys = fmt.Sprintf("%s\n%s", sshKeys, confSshKeys) + if confSSHKeys, exists := instanceMetadataSSHKeys[sshMetaKey]; exists { + sshKeys = fmt.Sprintf("%s\n%s", sshKeys, confSSHKeys) } - instanceMetadata[sshMetaKey] = sshKeys + instanceMetadataSSHKeys[sshMetaKey] = sshKeys } - startupScript := instanceMetadata[StartupScriptKey] + startupScript := instanceMetadataNoSSHKeys[StartupScriptKey] if c.StartupScriptFile != "" { var content []byte content, err = ioutil.ReadFile(c.StartupScriptFile) if err != nil { - return nil, err + return nil, instanceMetadataNoSSHKeys, err } startupScript = string(content) } - instanceMetadata[StartupScriptKey] = startupScript + instanceMetadataNoSSHKeys[StartupScriptKey] = startupScript // Wrap any found startup script with our own startup script wrapper. if startupScript != "" && c.WrapStartupScriptFile.True() { - instanceMetadata[StartupScriptKey] = StartupScriptLinux - instanceMetadata[StartupWrappedScriptKey] = startupScript - instanceMetadata[StartupScriptStatusKey] = StartupScriptStatusNotDone + instanceMetadataNoSSHKeys[StartupScriptKey] = StartupScriptLinux + instanceMetadataNoSSHKeys[StartupWrappedScriptKey] = startupScript + instanceMetadataNoSSHKeys[StartupScriptStatusKey] = StartupScriptStatusNotDone } if sourceImage.IsWindows() { // Windows startup script support is not yet implemented so clear any script data and set status to done - instanceMetadata[StartupScriptKey] = StartupScriptWindows - instanceMetadata[StartupScriptStatusKey] = StartupScriptStatusDone + instanceMetadataNoSSHKeys[StartupScriptKey] = StartupScriptWindows + instanceMetadataNoSSHKeys[StartupScriptStatusKey] = StartupScriptStatusDone } // If UseOSLogin is true, force `enable-oslogin` in metadata // In the event that `enable-oslogin` is not enabled at project level if c.UseOSLogin { - instanceMetadata[EnableOSLoginKey] = "TRUE" + instanceMetadataNoSSHKeys[EnableOSLoginKey] = "TRUE" } for key, value := range c.MetadataFiles { @@ -76,50 +86,13 @@ func (c *Config) createInstanceMetadata(sourceImage *Image, sshPublicKey string) if err != nil { errs = packer.MultiErrorAppend(errs, err) } - instanceMetadata[key] = string(content) + instanceMetadataNoSSHKeys[key] = string(content) } if errs != nil && len(errs.Errors) > 0 { - return instanceMetadata, errs - } - return instanceMetadata, nil -} - -func (s *StepCreateInstance) addMetadataToInstance(ctx context.Context, errCh chan<- error, name, state multistep.StateBag, metadata map[string]string) { - - c := state.Get("config").(*Config) - d := state.Get("driver").(Driver) - - instance, err := d.service.Instances.Get(d.projectId, c.Zone, name).Do() - if err != nil { - errCh <- err - return - } - instance.Metadata.Items = append(instance.Metadata.Items, &compute.MetadataItems{Key: "windows-keys", Value: &sshPublicKey}) - - op, err := d.service.Instances.SetMetadata(d.projectId, zone, name, &compute.Metadata{ - Fingerprint: instance.Metadata.Fingerprint, - Items: instance.Metadata.Items, - }).Do() - - if err != nil { - errCh <- err - return - } - - newErrCh := make(chan error, 1) - go waitForState(newErrCh, "DONE", d.refreshZoneOp(zone, op)) - - select { - case err = <-newErrCh: - case <-time.After(time.Second * 30): - err = errors.New("time out while waiting for SSH public key to be added to instance") - } - - if err != nil { - errCh <- err - return + return instanceMetadataNoSSHKeys, instanceMetadataSSHKeys, errs } + return instanceMetadataNoSSHKeys, instanceMetadataSSHKeys, nil } func getImage(c *Config, d Driver) (*Image, error) { @@ -168,17 +141,26 @@ func (s *StepCreateInstance) Run(ctx context.Context, state multistep.StateBag) name := c.InstanceName var errCh <-chan error - var metadata map[string]string - metadata, errs := c.createInstanceMetadata(sourceImage, string(c.Comm.SSHPublicKey)) + metadataNoSSHKeys := make(map[string]string) + metadataSSHKeys := make(map[string]string) + metadataForInstance := make(map[string]string) + + metadataNoSSHKeys, metadataSSHKeys, errs := c.createInstanceMetadata(sourceImage, string(c.Comm.SSHPublicKey)) if errs != nil { state.Put("error", errs.Error()) ui.Error(errs.Error()) return multistep.ActionHalt } - if c.WaitToAddSSHKeys == 0 { - ui.Message("Adding SSH keys during instance creation...") - metadata = make(map[string]string) + if c.WaitToAddSSHKeys > 0 { + log.Printf("[DEBUG] Adding metadata during instance creation, but not SSH keys...") + metadataForInstance = metadataNoSSHKeys + } else { + log.Printf("[DEBUG] Adding metadata during instance creation...") + + // Union of both non-SSH key meta data and SSH key meta data + addmap(metadataForInstance, metadataSSHKeys) + addmap(metadataForInstance, metadataNoSSHKeys) } errCh, err = d.RunInstance(&InstanceConfig{ @@ -195,7 +177,7 @@ func (s *StepCreateInstance) Run(ctx context.Context, state multistep.StateBag) Image: sourceImage, Labels: c.Labels, MachineType: c.MachineType, - Metadata: metadata, + Metadata: metadataForInstance, MinCpuPlatform: c.MinCpuPlatform, Name: name, Network: c.Network, @@ -242,15 +224,18 @@ func (s *StepCreateInstance) Run(ctx context.Context, state multistep.StateBag) state.Put("instance_id", name) if c.WaitToAddSSHKeys > 0 { - ui.Say(fmt.Sprintf("Waiting %s before adding SSH keys...", + ui.Message(fmt.Sprintf("Waiting %s before adding SSH keys...", c.WaitToAddSSHKeys.String())) - cancelled := s.wait(c.WaitToAddSSHKeys, ctx) + cancelled := s.waitForBoot(ctx, c.WaitToAddSSHKeys) if cancelled { return multistep.ActionHalt } + + log.Printf("[DEBUG] %s wait is over. Adding SSH keys to existing instance...", + c.WaitToAddSSHKeys.String()) + errCh, err = d.AddToInstanceMetadata(c.Zone, name, metadataSSHKeys) - metadata, errs = s.addMetadataToInstance(metadata, d, c) - if errs != nil { + if err != nil { state.Put("error", errs.Error()) ui.Error(errs.Error()) return multistep.ActionHalt @@ -260,14 +245,14 @@ func (s *StepCreateInstance) Run(ctx context.Context, state multistep.StateBag) return multistep.ActionContinue } -func (s *StepCreateInstance) wait(waitLen time.Duration, ctx context.Context) bool { +func (s *StepCreateInstance) waitForBoot(ctx context.Context, waitLen time.Duration) bool { // Use a select to determine if we get cancelled during the wait select { case <-ctx.Done(): return true case <-time.After(waitLen): } - ui.Message("Wait over; adding SSH keys to instance...") + return false } @@ -329,3 +314,9 @@ func (s *StepCreateInstance) Cleanup(state multistep.StateBag) { return } + +func addmap(a map[string]string, b map[string]string) { + for k,v := range b { + a[k] = v + } +} \ No newline at end of file diff --git a/builder/googlecompute/step_create_instance_test.go b/builder/googlecompute/step_create_instance_test.go index 42fa74d8c..86870d86f 100644 --- a/builder/googlecompute/step_create_instance_test.go +++ b/builder/googlecompute/step_create_instance_test.go @@ -305,12 +305,12 @@ func TestCreateInstanceMetadata(t *testing.T) { key := "abcdefgh12345678" // create our metadata - metadata, err := c.createInstanceMetadata(image, key) + _, metadataSSHKeys, err := c.createInstanceMetadata(image, key) assert.True(t, err == nil, "Metadata creation should have succeeded.") // ensure our key is listed - assert.True(t, strings.Contains(metadata["ssh-keys"], key), "Instance metadata should contain provided key") + assert.True(t, strings.Contains(metadataSSHKeys["ssh-keys"], key), "Instance metadata should contain provided key") } func TestCreateInstanceMetadata_noPublicKey(t *testing.T) { @@ -320,12 +320,12 @@ func TestCreateInstanceMetadata_noPublicKey(t *testing.T) { sshKeys := c.Metadata["ssh-keys"] // create our metadata - metadata, err := c.createInstanceMetadata(image, "") + _, metadataSSHKeys, err := c.createInstanceMetadata(image, "") assert.True(t, err == nil, "Metadata creation should have succeeded.") // ensure the ssh metadata hasn't changed - assert.Equal(t, metadata["ssh-keys"], sshKeys, "Instance metadata should not have been modified") + assert.Equal(t, metadataSSHKeys["ssh-keys"], sshKeys, "Instance metadata should not have been modified") } func TestCreateInstanceMetadata_metadataFile(t *testing.T) { @@ -337,12 +337,12 @@ func TestCreateInstanceMetadata_metadataFile(t *testing.T) { c.MetadataFiles["user-data"] = fileName // create our metadata - metadata, err := c.createInstanceMetadata(image, "") + metadataNoSSHKeys, _,err := c.createInstanceMetadata(image, "") assert.True(t, err == nil, "Metadata creation should have succeeded.") // ensure the user-data key in metadata is updated with file content - assert.Equal(t, metadata["user-data"], content, "user-data field of the instance metadata should have been updated.") + assert.Equal(t, metadataNoSSHKeys["user-data"], content, "user-data field of the instance metadata should have been updated.") } func TestCreateInstanceMetadata_withWrapStartupScript(t *testing.T) { @@ -377,11 +377,99 @@ func TestCreateInstanceMetadata_withWrapStartupScript(t *testing.T) { c.WrapStartupScriptFile = tc.WrapStartupScript // create our metadata - metadata, err := c.createInstanceMetadata(image, "") + metadataNoSSHKeys, _, err := c.createInstanceMetadata(image, "") assert.True(t, err == nil, "Metadata creation should have succeeded.") - assert.Equal(t, tc.StartupScriptContents, metadata[StartupScriptKey], fmt.Sprintf("Instance metadata for startup script should be %q.", tc.StartupScriptContents)) - assert.Equal(t, tc.WrappedStartupScriptContents, metadata[StartupWrappedScriptKey], fmt.Sprintf("Instance metadata for wrapped startup script should be %q.", tc.WrappedStartupScriptContents)) - assert.Equal(t, tc.WrappedStartupScriptStatus, metadata[StartupScriptStatusKey], fmt.Sprintf("Instance metadata startup script status should be %q.", tc.WrappedStartupScriptStatus)) + assert.Equal(t, tc.StartupScriptContents, metadataNoSSHKeys[StartupScriptKey], fmt.Sprintf("Instance metadata for startup script should be %q.", tc.StartupScriptContents)) + assert.Equal(t, tc.WrappedStartupScriptContents, metadataNoSSHKeys[StartupWrappedScriptKey], fmt.Sprintf("Instance metadata for wrapped startup script should be %q.", tc.WrappedStartupScriptContents)) + assert.Equal(t, tc.WrappedStartupScriptStatus, metadataNoSSHKeys[StartupScriptStatusKey], fmt.Sprintf("Instance metadata startup script status should be %q.", tc.WrappedStartupScriptStatus)) } } + +func TestCreateInstanceMetadataWaitToAddSSHKeys(t *testing.T) { + state := testState(t) + c := state.Get("config").(*Config) + image := StubImage("test-image", "test-project", []string{}, 100) + key := "abcdefgh12345678" + + var waitTime int = 4 + c.WaitToAddSSHKeys = time.Duration(waitTime) * time.Second + c.Metadata = map[string]string{ + "metadatakey1": "xyz", + "metadatakey2": "123", + } + + // create our metadata + metadataNoSSHKeys, metadataSSHKeys, err := c.createInstanceMetadata(image, key) + + assert.True(t, err == nil, "Metadata creation should have succeeded.") + + // ensure our metadata is listed + assert.True(t, strings.Contains(metadataSSHKeys["ssh-keys"], key), "Instance metadata should contain provided SSH key") + assert.True(t, strings.Contains(metadataNoSSHKeys["metadatakey1"], "xyz"), "Instance metadata should contain provided key: metadatakey1") + assert.True(t, strings.Contains(metadataNoSSHKeys["metadatakey2"], "123"), "Instance metadata should contain provided key: metadatakey2") +} + +func TestStepCreateInstanceWaitToAddSSHKeys(t *testing.T) { + state := testState(t) + step := new(StepCreateInstance) + defer step.Cleanup(state) + + state.Put("ssh_public_key", "key") + + c := state.Get("config").(*Config) + d := state.Get("driver").(*DriverMock) + d.GetImageResult = StubImage("test-image", "test-project", []string{}, 100) + + key := "abcdefgh12345678" + + var waitTime int = 5 + c.WaitToAddSSHKeys = time.Duration(waitTime) * time.Second + c.Comm.SSHPublicKey = []byte(key) + + c.Metadata = map[string]string{ + "metadatakey1": "xyz", + "metadatakey2": "123", + } + + // run the step + assert.Equal(t, step.Run(context.Background(), state), multistep.ActionContinue, "Step should have passed and continued.") + + // Verify state + _, ok := state.GetOk("instance_name") + assert.True(t, ok, "State should have an instance name.") + + // cleanup + step.Cleanup(state) +} + +func TestStepCreateInstanceNoWaitToAddSSHKeys(t *testing.T) { + state := testState(t) + step := new(StepCreateInstance) + defer step.Cleanup(state) + + state.Put("ssh_public_key", "key") + + c := state.Get("config").(*Config) + d := state.Get("driver").(*DriverMock) + d.GetImageResult = StubImage("test-image", "test-project", []string{}, 100) + + key := "abcdefgh12345678" + + c.Comm.SSHPublicKey = []byte(key) + + c.Metadata = map[string]string{ + "metadatakey1": "xyz", + "metadatakey2": "123", + } + + // run the step + assert.Equal(t, step.Run(context.Background(), state), multistep.ActionContinue, "Step should have passed and continued.") + + // Verify state + _, ok := state.GetOk("instance_name") + assert.True(t, ok, "State should have an instance name.") + + // cleanup + step.Cleanup(state) +} \ No newline at end of file diff --git a/website/pages/partials/builder/googlecompute/Config-not-required.mdx b/website/pages/partials/builder/googlecompute/Config-not-required.mdx index f1cb5a9eb..3bc4a6647 100644 --- a/website/pages/partials/builder/googlecompute/Config-not-required.mdx +++ b/website/pages/partials/builder/googlecompute/Config-not-required.mdx @@ -241,3 +241,12 @@ instance. For more information, see the Vault docs: https://www.vaultproject.io/docs/commands/#environment-variables Example:`"vault_gcp_oauth_engine": "gcp/token/my-project-editor",` + +- `wait_to_add_ssh_keys` (duration string | ex: "1h5m2s") - The time to wait between the creation of the instance used to create the image, + and the addition of SSH configuration, including SSH keys, to that instance. + The delay is intended to protect packer from anything in the instance boot + sequence that has potential to disrupt the creation of SSH configuration + (e.g. SSH user creation, SSH key creation) on the instance. + Note: All other instance metadata, including startup scripts, are still added to the instance + during it's creation. + Example value: `5m`. From 833855eec596fe05491aca1a7e7e7ad7703864a4 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Wed, 2 Dec 2020 11:16:16 +0000 Subject: [PATCH 3/5] Amend commit author for license pass --- builder/googlecompute/config.go | 4 ++-- builder/googlecompute/driver_gce.go | 4 ++-- builder/googlecompute/driver_mock.go | 8 ++++---- builder/googlecompute/startup.go | 4 ++-- builder/googlecompute/step_create_instance.go | 18 +++++++++--------- .../googlecompute/step_create_instance_test.go | 18 +++++++++--------- 6 files changed, 28 insertions(+), 28 deletions(-) diff --git a/builder/googlecompute/config.go b/builder/googlecompute/config.go index 41c36d517..dde30fb59 100644 --- a/builder/googlecompute/config.go +++ b/builder/googlecompute/config.go @@ -287,8 +287,8 @@ type Config struct { VaultGCPOauthEngine string `mapstructure:"vault_gcp_oauth_engine"` // The time to wait between the creation of the instance used to create the image, // and the addition of SSH configuration, including SSH keys, to that instance. - // The delay is intended to protect packer from anything in the instance boot - // sequence that has potential to disrupt the creation of SSH configuration + // The delay is intended to protect packer from anything in the instance boot + // sequence that has potential to disrupt the creation of SSH configuration // (e.g. SSH user creation, SSH key creation) on the instance. // Note: All other instance metadata, including startup scripts, are still added to the instance // during it's creation. diff --git a/builder/googlecompute/driver_gce.go b/builder/googlecompute/driver_gce.go index 8678fd7eb..4766c6260 100644 --- a/builder/googlecompute/driver_gce.go +++ b/builder/googlecompute/driver_gce.go @@ -764,6 +764,6 @@ func (d *driverGCE) AddToInstanceMetadata(zone string, name string, metadata map case <-time.After(time.Second * 30): err = errors.New("time out while waiting for SSH keys to be added to instance") } - + return newErrCh, err -} \ No newline at end of file +} diff --git a/builder/googlecompute/driver_mock.go b/builder/googlecompute/driver_mock.go index bd455841b..38179d53f 100644 --- a/builder/googlecompute/driver_mock.go +++ b/builder/googlecompute/driver_mock.go @@ -93,7 +93,7 @@ type DriverMock struct { AddToInstanceMetadataName string AddToInstanceMetadataKVPairs map[string]string AddToInstanceMetadataErrCh <-chan error - AddToInstanceMetadataErr error + AddToInstanceMetadataErr error } func (d *DriverMock) CreateImage(name, description, family, zone, disk string, image_labels map[string]string, image_licenses []string, image_encryption_key *compute.CustomerEncryptionKey, imageStorageLocations []string) (<-chan *Image, <-chan error) { @@ -296,8 +296,8 @@ func (d *DriverMock) DeleteOSLoginSSHKey(user, fingerprint string) error { } func (d *DriverMock) AddToInstanceMetadata(zone string, name string, metadata map[string]string) (<-chan error, error) { - d.AddToInstanceMetadataZone = zone - d.AddToInstanceMetadataName = name + d.AddToInstanceMetadataZone = zone + d.AddToInstanceMetadataName = name d.AddToInstanceMetadataKVPairs = metadata resultCh := d.AddToInstanceMetadataErrCh @@ -308,4 +308,4 @@ func (d *DriverMock) AddToInstanceMetadata(zone string, name string, metadata ma } return resultCh, d.AddToInstanceMetadataErr -} \ No newline at end of file +} diff --git a/builder/googlecompute/startup.go b/builder/googlecompute/startup.go index bdb147479..743778a2c 100644 --- a/builder/googlecompute/startup.go +++ b/builder/googlecompute/startup.go @@ -1,7 +1,7 @@ package googlecompute import ( - "fmt" + "fmt" ) const StartupScriptKey string = "startup-script" @@ -57,4 +57,4 @@ SetMetadata %s %s exit $RETVAL `, StartupWrappedScriptKey, StartupScriptStatusKey, StartupScriptStatusDone) -var StartupScriptWindows string = "" \ No newline at end of file +var StartupScriptWindows string = "" diff --git a/builder/googlecompute/step_create_instance.go b/builder/googlecompute/step_create_instance.go index fa52ae8b6..f895a1058 100644 --- a/builder/googlecompute/step_create_instance.go +++ b/builder/googlecompute/step_create_instance.go @@ -5,9 +5,9 @@ import ( "errors" "fmt" "io/ioutil" + "log" "strings" "time" - "log" "github.com/hashicorp/packer/packer" "github.com/hashicorp/packer/packer-plugin-sdk/multistep" @@ -142,7 +142,7 @@ func (s *StepCreateInstance) Run(ctx context.Context, state multistep.StateBag) var errCh <-chan error metadataNoSSHKeys := make(map[string]string) - metadataSSHKeys := make(map[string]string) + metadataSSHKeys := make(map[string]string) metadataForInstance := make(map[string]string) metadataNoSSHKeys, metadataSSHKeys, errs := c.createInstanceMetadata(sourceImage, string(c.Comm.SSHPublicKey)) @@ -160,7 +160,7 @@ func (s *StepCreateInstance) Run(ctx context.Context, state multistep.StateBag) // Union of both non-SSH key meta data and SSH key meta data addmap(metadataForInstance, metadataSSHKeys) - addmap(metadataForInstance, metadataNoSSHKeys) + addmap(metadataForInstance, metadataNoSSHKeys) } errCh, err = d.RunInstance(&InstanceConfig{ @@ -230,9 +230,9 @@ func (s *StepCreateInstance) Run(ctx context.Context, state multistep.StateBag) if cancelled { return multistep.ActionHalt } - + log.Printf("[DEBUG] %s wait is over. Adding SSH keys to existing instance...", - c.WaitToAddSSHKeys.String()) + c.WaitToAddSSHKeys.String()) errCh, err = d.AddToInstanceMetadata(c.Zone, name, metadataSSHKeys) if err != nil { @@ -241,7 +241,7 @@ func (s *StepCreateInstance) Run(ctx context.Context, state multistep.StateBag) return multistep.ActionHalt } } - + return multistep.ActionContinue } @@ -252,7 +252,7 @@ func (s *StepCreateInstance) waitForBoot(ctx context.Context, waitLen time.Durat return true case <-time.After(waitLen): } - + return false } @@ -316,7 +316,7 @@ func (s *StepCreateInstance) Cleanup(state multistep.StateBag) { } func addmap(a map[string]string, b map[string]string) { - for k,v := range b { + for k, v := range b { a[k] = v } -} \ No newline at end of file +} diff --git a/builder/googlecompute/step_create_instance_test.go b/builder/googlecompute/step_create_instance_test.go index 86870d86f..9db780d02 100644 --- a/builder/googlecompute/step_create_instance_test.go +++ b/builder/googlecompute/step_create_instance_test.go @@ -337,7 +337,7 @@ func TestCreateInstanceMetadata_metadataFile(t *testing.T) { c.MetadataFiles["user-data"] = fileName // create our metadata - metadataNoSSHKeys, _,err := c.createInstanceMetadata(image, "") + metadataNoSSHKeys, _, err := c.createInstanceMetadata(image, "") assert.True(t, err == nil, "Metadata creation should have succeeded.") @@ -391,14 +391,14 @@ func TestCreateInstanceMetadataWaitToAddSSHKeys(t *testing.T) { c := state.Get("config").(*Config) image := StubImage("test-image", "test-project", []string{}, 100) key := "abcdefgh12345678" - + var waitTime int = 4 c.WaitToAddSSHKeys = time.Duration(waitTime) * time.Second c.Metadata = map[string]string{ "metadatakey1": "xyz", "metadatakey2": "123", } - + // create our metadata metadataNoSSHKeys, metadataSSHKeys, err := c.createInstanceMetadata(image, key) @@ -406,8 +406,8 @@ func TestCreateInstanceMetadataWaitToAddSSHKeys(t *testing.T) { // ensure our metadata is listed assert.True(t, strings.Contains(metadataSSHKeys["ssh-keys"], key), "Instance metadata should contain provided SSH key") - assert.True(t, strings.Contains(metadataNoSSHKeys["metadatakey1"], "xyz"), "Instance metadata should contain provided key: metadatakey1") - assert.True(t, strings.Contains(metadataNoSSHKeys["metadatakey2"], "123"), "Instance metadata should contain provided key: metadatakey2") + assert.True(t, strings.Contains(metadataNoSSHKeys["metadatakey1"], "xyz"), "Instance metadata should contain provided key: metadatakey1") + assert.True(t, strings.Contains(metadataNoSSHKeys["metadatakey2"], "123"), "Instance metadata should contain provided key: metadatakey2") } func TestStepCreateInstanceWaitToAddSSHKeys(t *testing.T) { @@ -422,7 +422,7 @@ func TestStepCreateInstanceWaitToAddSSHKeys(t *testing.T) { d.GetImageResult = StubImage("test-image", "test-project", []string{}, 100) key := "abcdefgh12345678" - + var waitTime int = 5 c.WaitToAddSSHKeys = time.Duration(waitTime) * time.Second c.Comm.SSHPublicKey = []byte(key) @@ -455,14 +455,14 @@ func TestStepCreateInstanceNoWaitToAddSSHKeys(t *testing.T) { d.GetImageResult = StubImage("test-image", "test-project", []string{}, 100) key := "abcdefgh12345678" - + c.Comm.SSHPublicKey = []byte(key) c.Metadata = map[string]string{ "metadatakey1": "xyz", "metadatakey2": "123", } - + // run the step assert.Equal(t, step.Run(context.Background(), state), multistep.ActionContinue, "Step should have passed and continued.") @@ -472,4 +472,4 @@ func TestStepCreateInstanceNoWaitToAddSSHKeys(t *testing.T) { // cleanup step.Cleanup(state) -} \ No newline at end of file +} From d6831a4de30b7640e0bf928d3cbb2cd60693622a Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Wed, 2 Dec 2020 13:55:00 +0000 Subject: [PATCH 4/5] Amend commit author for license pass --- builder/googlecompute/driver.go | 2 +- builder/googlecompute/driver_gce.go | 15 ++++++++++----- builder/googlecompute/driver_mock.go | 4 ++-- builder/googlecompute/step_create_instance.go | 12 +++++++----- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/builder/googlecompute/driver.go b/builder/googlecompute/driver.go index 96fef6574..0c8afbda5 100644 --- a/builder/googlecompute/driver.go +++ b/builder/googlecompute/driver.go @@ -71,7 +71,7 @@ type Driver interface { DeleteOSLoginSSHKey(user, fingerprint string) error // Add to the instance metadata for the existing instance - AddToInstanceMetadata(zone string, name string, metadata map[string]string) (<-chan error, error) + AddToInstanceMetadata(zone string, name string, metadata map[string]string) error } type InstanceConfig struct { diff --git a/builder/googlecompute/driver_gce.go b/builder/googlecompute/driver_gce.go index 4766c6260..b63c293ad 100644 --- a/builder/googlecompute/driver_gce.go +++ b/builder/googlecompute/driver_gce.go @@ -728,11 +728,11 @@ func waitForState(errCh chan<- error, target string, refresh stateRefreshFunc) e return err } -func (d *driverGCE) AddToInstanceMetadata(zone string, name string, metadata map[string]string) (<-chan error, error) { +func (d *driverGCE) AddToInstanceMetadata(zone string, name string, metadata map[string]string) error { instance, err := d.service.Instances.Get(d.projectId, zone, name).Do() if err != nil { - return nil, err + return err } // Build up the metadata @@ -753,7 +753,7 @@ func (d *driverGCE) AddToInstanceMetadata(zone string, name string, metadata map }).Do() if err != nil { - return nil, err + return err } newErrCh := make(chan error, 1) @@ -762,8 +762,13 @@ func (d *driverGCE) AddToInstanceMetadata(zone string, name string, metadata map select { case err = <-newErrCh: case <-time.After(time.Second * 30): - err = errors.New("time out while waiting for SSH keys to be added to instance") + err = errors.New("time out while waiting for instance to create") } - return newErrCh, err + if err != nil { + newErrCh <- err + return err + } + + return nil } diff --git a/builder/googlecompute/driver_mock.go b/builder/googlecompute/driver_mock.go index 38179d53f..cbe89dd76 100644 --- a/builder/googlecompute/driver_mock.go +++ b/builder/googlecompute/driver_mock.go @@ -295,7 +295,7 @@ func (d *DriverMock) DeleteOSLoginSSHKey(user, fingerprint string) error { return nil } -func (d *DriverMock) AddToInstanceMetadata(zone string, name string, metadata map[string]string) (<-chan error, error) { +func (d *DriverMock) AddToInstanceMetadata(zone string, name string, metadata map[string]string) error { d.AddToInstanceMetadataZone = zone d.AddToInstanceMetadataName = name d.AddToInstanceMetadataKVPairs = metadata @@ -307,5 +307,5 @@ func (d *DriverMock) AddToInstanceMetadata(zone string, name string, metadata ma resultCh = ch } - return resultCh, d.AddToInstanceMetadataErr + return nil } diff --git a/builder/googlecompute/step_create_instance.go b/builder/googlecompute/step_create_instance.go index f895a1058..4ae4892f9 100644 --- a/builder/googlecompute/step_create_instance.go +++ b/builder/googlecompute/step_create_instance.go @@ -141,8 +141,8 @@ func (s *StepCreateInstance) Run(ctx context.Context, state multistep.StateBag) name := c.InstanceName var errCh <-chan error - metadataNoSSHKeys := make(map[string]string) - metadataSSHKeys := make(map[string]string) + var metadataNoSSHKeys map[string]string + var metadataSSHKeys map[string]string metadataForInstance := make(map[string]string) metadataNoSSHKeys, metadataSSHKeys, errs := c.createInstanceMetadata(sourceImage, string(c.Comm.SSHPublicKey)) @@ -233,11 +233,12 @@ func (s *StepCreateInstance) Run(ctx context.Context, state multistep.StateBag) log.Printf("[DEBUG] %s wait is over. Adding SSH keys to existing instance...", c.WaitToAddSSHKeys.String()) - errCh, err = d.AddToInstanceMetadata(c.Zone, name, metadataSSHKeys) + err = d.AddToInstanceMetadata(c.Zone, name, metadataSSHKeys) if err != nil { - state.Put("error", errs.Error()) - ui.Error(errs.Error()) + err := fmt.Errorf("Error adding SSH keys to existing instance: %s", err) + state.Put("error", err) + ui.Error(err.Error()) return multistep.ActionHalt } } @@ -316,6 +317,7 @@ func (s *StepCreateInstance) Cleanup(state multistep.StateBag) { } func addmap(a map[string]string, b map[string]string) { + for k, v := range b { a[k] = v } From 937e39d9f49e5c7d967510445c3de1594b2838d9 Mon Sep 17 00:00:00 2001 From: Gareth Rees Date: Wed, 2 Dec 2020 14:50:28 +0000 Subject: [PATCH 5/5] Amend commit author to align with Hashicorp license --- builder/googlecompute/driver_gce.go | 15 +++++++++------ builder/googlecompute/driver_mock.go | 1 - 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/builder/googlecompute/driver_gce.go b/builder/googlecompute/driver_gce.go index b63c293ad..b5c8c62e5 100644 --- a/builder/googlecompute/driver_gce.go +++ b/builder/googlecompute/driver_gce.go @@ -757,13 +757,16 @@ func (d *driverGCE) AddToInstanceMetadata(zone string, name string, metadata map } newErrCh := make(chan error, 1) - go waitForState(newErrCh, "DONE", d.refreshZoneOp(zone, op)) - select { - case err = <-newErrCh: - case <-time.After(time.Second * 30): - err = errors.New("time out while waiting for instance to create") - } + go func() { + err = waitForState(newErrCh, "DONE", d.refreshZoneOp(zone, op)) + + select { + case err = <-newErrCh: + case <-time.After(time.Second * 30): + err = errors.New("time out while waiting for instance to create") + } + }() if err != nil { newErrCh <- err diff --git a/builder/googlecompute/driver_mock.go b/builder/googlecompute/driver_mock.go index cbe89dd76..76b1fcee5 100644 --- a/builder/googlecompute/driver_mock.go +++ b/builder/googlecompute/driver_mock.go @@ -304,7 +304,6 @@ func (d *DriverMock) AddToInstanceMetadata(zone string, name string, metadata ma if resultCh == nil { ch := make(chan error) close(ch) - resultCh = ch } return nil