builder: no delete logs OSLogin ssh key if not set

builder/googlecompute/builder_acc_test.go

@@ -4,9 +4,15 @@
 package googlecompute
 
 import (
+	"crypto/rand"
+	"crypto/rsa"
+	"crypto/x509"
 	"embed"
+	"encoding/pem"
 	"fmt"
+	"os"
 	"os/exec"
+	"strings"
 	"testing"
 
 	"github.com/hashicorp/packer-plugin-sdk/acctest"
@@ -55,6 +61,76 @@ func TestAccBuilder_DefaultTokenSource(t *testing.T) {
 	acctest.TestPlugin(t, testCase)
 }
 
+// generateSSHPrivateKey generates a PEM encoded ssh private key file
+//
+// The file's deletion is the responsibility of the caller.
+func generateSSHPrivateKey() (string, error) {
+	outFile := fmt.Sprintf("%s/temp_key", os.TempDir())
+
+	priv, err := rsa.GenerateKey(rand.Reader, 4096)
+	if err != nil {
+		return "", fmt.Errorf("failed to generate SSH key: %s", err)
+	}
+
+	x509key := x509.MarshalPKCS1PrivateKey(priv)
+
+	pemKey := pem.EncodeToMemory(&pem.Block{
+		Type:  "RSA PRIVATE KEY",
+		Bytes: x509key,
+	})
+
+	err = os.WriteFile(outFile, pemKey, 0600)
+	if err != nil {
+		return "", fmt.Errorf("failed to write private key to %q: %s", outFile, err)
+	}
+
+	return outFile, nil
+}
+
+func TestAccBuilder_DefaultTokenSourceWithPrivateKey(t *testing.T) {
+	keyFile, err := generateSSHPrivateKey()
+	if err != nil {
+		t.Fatalf("failed to generate SSH private key: %s", err)
+	}
+
+	defer os.Remove(keyFile)
+
+	tmpl, err := testDataFs.ReadFile("testdata/oslogin/default-token-and-pkey.pkr.hcl")
+	if err != nil {
+		t.Fatalf("failed to read testdata file %s", err)
+	}
+
+	testCase := &acctest.PluginTestCase{
+		Name:     "googlecompute-packer-default-ts",
+		Template: fmt.Sprintf(string(tmpl), keyFile),
+		Check: func(buildCommand *exec.Cmd, logfile string) error {
+			if buildCommand.ProcessState != nil {
+				if buildCommand.ProcessState.ExitCode() == 0 {
+					return fmt.Errorf("Packer build should have failed because of the unknown SSH key for the target instance, but succeeded. Logfile: %s", logfile)
+				}
+			}
+
+			rawLogs, err := os.ReadFile(logfile)
+			if err != nil {
+				return fmt.Errorf("failed to read logfile %q: %s", logfile, err)
+			}
+
+			logs := string(rawLogs)
+
+			if !strings.Contains(logs, "Private key file specified, won't import SSH key for OSLogin") {
+				return fmt.Errorf("did not find message stating that a private key file was specified")
+			}
+
+			if strings.Contains(logs, "Deleting SSH public key for OSLogin...") {
+				return fmt.Errorf("found a message about deleting OSLogin SSH public key, shouldn't have")
+			}
+
+			return nil
+		},
+	}
+	acctest.TestPlugin(t, testCase)
+}
+
 func TestAccBuilder_WrappedStartupScriptSuccess(t *testing.T) {
 	tmpl, err := testDataFs.ReadFile("testdata/wrapped-startup-scripts/successful.pkr.hcl")
 	if err != nil {

builder/googlecompute/step_create_instance.go

@@ -42,10 +42,7 @@ func (c *Config) createInstanceMetadata(sourceImage *Image, sshPublicKey string)
 		}
 	}
 
-	// Merge any existing ssh keys with our public key, unless there is no
-	// supplied public key. This is possible if a private_key_file was
-	// specified.
-	if sshPublicKey != "" {
+	if c.Comm.SSHPrivateKeyFile == "" && sshPublicKey != "" {
 		sshMetaKey := "ssh-keys"
 		sshPublicKey = strings.TrimSuffix(sshPublicKey, "\n")
 		sshKeys := fmt.Sprintf("%s:%s %s", c.Comm.SSHUsername, sshPublicKey, c.Comm.SSHUsername)

builder/googlecompute/step_import_os_login_ssh_key.go

@@ -38,8 +38,16 @@ func (s *StepImportOSLoginSSHKey) Run(ctx context.Context, state multistep.State
 		return multistep.ActionContinue
 	}
 
-	// If no public key information is available chances are that a private key was provided
-	//  or that the user is using a SSH agent for authentication.
+	// If the user specified a private key, the assumption is that the instance
+	// will already know the private key, and therefore doesn't need to be
+	// registered for OSLogin.
+	if config.Comm.SSHPrivateKeyFile != "" {
+		ui.Say("Private key file specified, won't import SSH key for OSLogin")
+		return multistep.ActionContinue
+	}
+
+	// If no public key information is available chances are that the user
+	// is using a SSH agent for authentication.
 	if config.Comm.SSHPublicKey == nil {
 		ui.Say("No public SSH key found; skipping SSH public key import for OSLogin...")
 		return multistep.ActionContinue
@@ -122,14 +130,9 @@ func (s *StepImportOSLoginSSHKey) Run(ctx context.Context, state multistep.State
 
 // Cleanup the SSH Key that we added to the POSIX account
 func (s *StepImportOSLoginSSHKey) Cleanup(state multistep.StateBag) {
-	config := state.Get("config").(*Config)
 	driver := state.Get("driver").(Driver)
 	ui := state.Get("ui").(packersdk.Ui)
 
-	if !config.UseOSLogin {
-		return
-	}
-
 	fingerprint, ok := state.Get("ssh_key_public_sha256").(string)
 	if !ok || fingerprint == "" {
 		return

builder/googlecompute/step_import_os_login_ssh_key_test.go

@@ -7,6 +7,7 @@ import (
 	"context"
 	"crypto/sha256"
 	"encoding/hex"
+	"os"
 	"testing"
 
 	"github.com/hashicorp/packer-plugin-sdk/multistep"
@@ -203,10 +204,15 @@ func TestStepImportOSLoginSSHKey_withPrivateSSHKey(t *testing.T) {
 	step := new(StepImportOSLoginSSHKey)
 	defer step.Cleanup(state)
 
+	pkey, err := generateSSHPrivateKey()
+	if err != nil {
+		t.Fatalf("failed to generate SSH key: %s", err)
+	}
+	defer os.Remove(pkey)
+
 	config := state.Get("config").(*Config)
 	config.UseOSLogin = true
-	config.Comm.SSHPrivateKey = []byte{'k', 'e', 'y'}
-	config.Comm.SSHPublicKey = nil
+	config.Comm.SSHPrivateKeyFile = pkey
 
 	if action := step.Run(context.Background(), state); action != multistep.ActionContinue {
 		t.Fatalf("bad action: %#v", action)

builder/googlecompute/testdata/oslogin/default-token-and-pkey.pkr.hcl

@@ -0,0 +1,45 @@
+# Copyright (c) HashiCorp, Inc.
+# SPDX-License-Identifier: MPL-2.0
+
+variable "project" {
+  type    = string
+  default = env("GOOGLE_PROJECT_ID")
+}
+
+variable "ssh_private_key" {
+  type    = string
+  default = ""
+}
+
+variable "ssh_username" {
+  type    = string
+  default = "root"
+}
+
+variable "zone" {
+  type    = string
+  default = "us-central1-a"
+}
+
+locals { timestamp = regex_replace(timestamp(), "[- TZ:]", "") }
+
+# No provided access_token or account_file should read contents of env GOOGLE_APPLICATION_CREDENTIALS
+source "googlecompute" "autogenerated_1" {
+  image_name           = "packer-oslogin-tester-${local.timestamp}"
+  project_id           = var.project
+  source_image_family  = "centos-7"
+  ssh_username         = var.ssh_username
+  ssh_private_key_file = "%s"
+  ssh_timeout          = "30s"
+  use_os_login         = true
+  skip_create_image    = true
+  zone                 = var.zone
+}
+
+build {
+  sources = ["source.googlecompute.autogenerated_1"]
+
+  provisioner "shell" {
+    inline = ["echo hello from the other side, username is $(whoami)"]
+  }
+}