Merge pull request #10320 from seventieskid/gcp-wait-to-add-ssh-keys-10312

Gcp wait to add ssh keys 10312

builder/googlecompute/config.go

@@ -285,6 +285,15 @@ 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 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"`

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

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",

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) error
 }
 
 type InstanceConfig struct {

builder/googlecompute/driver_gce.go

@@ -727,3 +727,51 @@ 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) error {
+
+	instance, err := d.service.Instances.Get(d.projectId, zone, name).Do()
+	if err != nil {
+		return 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 err
+	}
+
+	newErrCh := make(chan error, 1)
+
+	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
+		return err
+	}
+
+	return nil
+}

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,17 @@ 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) error {
+	d.AddToInstanceMetadataZone = zone
+	d.AddToInstanceMetadataName = name
+	d.AddToInstanceMetadataKVPairs = metadata
+
+	resultCh := d.AddToInstanceMetadataErrCh
+	if resultCh == nil {
+		ch := make(chan error)
+		close(ch)
+	}
+
+	return nil
+}

builder/googlecompute/step_create_instance.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"io/ioutil"
+	"log"
 	"strings"
 	"time"
 
@@ -17,14 +18,23 @@ type StepCreateInstance struct {
 	Debug bool
 }
 
-func (c *Config) createInstanceMetadata(sourceImage *Image, sshPublicKey string) (map[string]string, error) {
+func (c *Config) createInstanceMetadata(sourceImage *Image, sshPublicKey string) (map[string]string, map[string]string, error) {
-	instanceMetadata := make(map[string]string)
+
+	instanceMetadataNoSSHKeys := make(map[string]string)
+	instanceMetadataSSHKeys := make(map[string]string)
+
+	sshMetaKey := "ssh-keys"
+
 	var err error
 	var errs *packersdk.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 {
+		if confSSHKeys, exists := instanceMetadataSSHKeys[sshMetaKey]; exists {
-			sshKeys = fmt.Sprintf("%s\n%s", sshKeys, confSshKeys)
+			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
+		instanceMetadataNoSSHKeys[StartupScriptKey] = StartupScriptLinux
-		instanceMetadata[StartupWrappedScriptKey] = startupScript
+		instanceMetadataNoSSHKeys[StartupWrappedScriptKey] = startupScript
-		instanceMetadata[StartupScriptStatusKey] = StartupScriptStatusNotDone
+		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
+		instanceMetadataNoSSHKeys[StartupScriptKey] = StartupScriptWindows
-		instanceMetadata[StartupScriptStatusKey] = StartupScriptStatusDone
+		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,13 +86,13 @@ func (c *Config) createInstanceMetadata(sourceImage *Image, sshPublicKey string)
 		if err != nil {
 			errs = packersdk.MultiErrorAppend(errs, err)
 		}
-		instanceMetadata[key] = string(content)
+		instanceMetadataNoSSHKeys[key] = string(content)
 	}
 
 	if errs != nil && len(errs.Errors) > 0 {
-		return instanceMetadata, errs
+		return instanceMetadataNoSSHKeys, instanceMetadataSSHKeys, errs
 	}
-	return instanceMetadata, nil
+	return instanceMetadataNoSSHKeys, instanceMetadataSSHKeys, nil
 }
 
 func getImage(c *Config, d Driver) (*Image, error) {
@@ -131,14 +141,28 @@ func (s *StepCreateInstance) Run(ctx context.Context, state multistep.StateBag)
 	name := c.InstanceName
 
 	var errCh <-chan error
-	var metadata map[string]string
+	var metadataNoSSHKeys map[string]string
-	metadata, errs := c.createInstanceMetadata(sourceImage, string(c.Comm.SSHPublicKey))
+	var metadataSSHKeys 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 {
+		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{
 		AcceleratorType:              c.AcceleratorType,
 		AcceleratorCount:             c.AcceleratorCount,
@@ -153,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,
@@ -199,9 +223,40 @@ 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.Message(fmt.Sprintf("Waiting %s before adding SSH keys...",
+			c.WaitToAddSSHKeys.String()))
+		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())
+		err = d.AddToInstanceMetadata(c.Zone, name, metadataSSHKeys)
+
+		if err != nil {
+			err := fmt.Errorf("Error adding SSH keys to existing instance: %s", err)
+			state.Put("error", err)
+			ui.Error(err.Error())
+			return multistep.ActionHalt
+		}
+	}
+
 	return multistep.ActionContinue
 }
 
+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):
+	}
+
+	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")
@@ -260,3 +315,10 @@ 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
+	}
+}

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.StartupScriptContents, metadataNoSSHKeys[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.WrappedStartupScriptContents, metadataNoSSHKeys[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.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)
+}

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`.