Add tests to internal shell command executor (#31)

provisioner/docker-mailserver/provisioner.go

@@ -8,8 +8,8 @@ package mailserver
 import (
 	"context"
 	"fmt"
-	basicProvisioner "github.com/QubitPi/packer-plugin-hashicorp-aws/provisioner/basic-provisioner"
 	fileProvisioner "github.com/QubitPi/packer-plugin-hashicorp-aws/provisioner/file-provisioner"
+	basicProvisioner "github.com/QubitPi/packer-plugin-hashicorp-aws/provisioner/shell"
 	sslProvisioner "github.com/QubitPi/packer-plugin-hashicorp-aws/provisioner/ssl-provisioner"
 	"github.com/hashicorp/hcl/v2/hcldec"
 	packersdk "github.com/hashicorp/packer-plugin-sdk/packer"

provisioner/basic-provisioner/basic-provisioner.go → provisioner/shell/provisioner.go

@@ -1,30 +1,9 @@
 // Copyright (c) Jiaqi Liu
 // SPDX-License-Identifier: MPL-2.0
 
-// This package implements a provisioner for Packer that executes a specified list of shell commands within the remote
-// machine. It doesn't use Packer's original shell provisioner
-// (https://github.com/hashicorp/packer/blob/main/provisioner/shell/provisioner.go), which is essentially not fully
-// exported for public use due to the unexported config parameter -
-// https://github.com/hashicorp/packer/blob/1e446de977e93b7119ebbaa6f55268bd29240e4f/provisioner/shell/provisioner.go#L73
-// This parameter is unexported because is not capitalized
-//
-// This provisioner works by loading all provided amiConfigCommands into a shell scripts. We do this because executing
-// amiConfigCommands separately in the following way simply doesn't work:
-//
-//	if len(amiConfigCommands) > 0 {
-//		   for _, command := range amiConfigCommands {
-//		   	   err := (&packersdk.RemoteCmd{Command: command}).RunWithUi(ctx, communicator, ui)
-//		   	   if err != nil {
-//		   	   	   return fmt.Errorf("CMD error: %s", err)
-//		   	   }
-//		   }
-//	}
-//
-// because each command is executed in separate shell, meaning their state is not preserved unless the state is written
-// to the remote machines hard disk. For example, the regular env variable export like "export JAVA_HOME=..." won't
-// carry over to the next command. The only way to preserve all in-memory states is to run everything in a one-time
-// script, which is how this provisioner is implemented
-package basicProvisioner
+// Package shell This package implements an internal provisioner for Packer that executes a specified list of shell
+// commands within the remote machine
+package shell
 
 import (
 	"bufio"
@@ -37,34 +16,35 @@ import (
 	"os"
 )
 
-func Provision(ctx context.Context, ui packersdk.Ui, communicator packersdk.Communicator, amiConfigCommands []string) error {
+func loadCommandsIntoScript(commands []string) (*os.File, error) {
 	scriptFile, err := tmp.File("packer-shell")
 	if err != nil {
-		return fmt.Errorf("Error while trying to load commands into a shell script: %s", err)
+		return nil, fmt.Errorf("error while trying to load commands into a shell script: %s", err)
 	}
-	defer os.Remove(scriptFile.Name())
 
 	writer := bufio.NewWriter(scriptFile)
 	writer.WriteString("#!/bin/bash\n")
 	writer.WriteString("set -x\n")
 	writer.WriteString("set -e\n")
 	writer.WriteString("\n")
-	for _, command := range amiConfigCommands {
+	for _, command := range commands {
 		if _, err := writer.WriteString(command + "\n"); err != nil {
-			return fmt.Errorf("Error flushing command '%s' into a shell script: %s", command, err)
+			return nil, fmt.Errorf("error flushing command '%s' into a shell script: %s", command, err)
 		}
 	}
 	if err := writer.Flush(); err != nil {
-		return fmt.Errorf("Error flushing shell script: %s", err)
+		return nil, fmt.Errorf("error flushing shell script: %s", err)
 	}
 
 	scriptFile.Close()
 
-	ui.Say(fmt.Sprintf("Provisioning with %s", amiConfigCommands))
+	return scriptFile, err
+}
 
+func executeScript(ctx context.Context, ui packersdk.Ui, communicator packersdk.Communicator, scriptFile *os.File) error {
 	f, err := os.Open(scriptFile.Name())
 	if err != nil {
-		return fmt.Errorf("Error opening shell script: %s", err)
+		return fmt.Errorf("error opening shell script: %s", err)
 	}
 	defer f.Close()
 
@@ -76,14 +56,14 @@ func Provision(ctx context.Context, ui packersdk.Ui, communicator packersdk.Comm
 
 		remotePath := fmt.Sprintf("%s/%s", "/tmp", fmt.Sprintf("script_%d.sh", rand.Intn(9999)))
 		if err := communicator.Upload(remotePath, f, nil); err != nil {
-			return fmt.Errorf("Error uploading script: %s", err)
+			return fmt.Errorf("error uploading script: %s", err)
 		}
 
 		cmd = &packersdk.RemoteCmd{
 			Command: fmt.Sprintf("chmod 0755 %s", remotePath),
 		}
 		if err := communicator.Start(ctx, cmd); err != nil {
-			return fmt.Errorf("Error chmodding script file to 0755 in remote machine: %s", err)
+			return fmt.Errorf("error chmodding script file to 0755 in remote machine: %s", err)
 		}
 		cmd.Wait()
 
@@ -97,3 +77,39 @@ func Provision(ctx context.Context, ui packersdk.Ui, communicator packersdk.Comm
 
 	return nil
 }
+
+// Provision Batch executes a list of ordered bash shell commands.
+//
+// It doesn't reuse Packer's original shell provisioner
+// (https://github.com/hashicorp/packer/blob/main/provisioner/shell/provisioner.go), which is not fully exported for
+// public use due to the unexported config parameter -
+// https://github.com/hashicorp/packer/blob/1e446de977e93b7119ebbaa6f55268bd29240e4f/provisioner/shell/provisioner.go#L73
+// This parameter is unexported because is not capitalized
+//
+// This provisioner works by loading all provided commands into a shell script. We do this because executing commands
+// separately in the following way simply doesn't work:
+//
+//	if len(amiConfigCommands) > 0 {
+//		   for _, command := range amiConfigCommands {
+//		   	   err := (&packersdk.RemoteCmd{Command: command}).RunWithUi(ctx, communicator, ui)
+//		   	   if err != nil {
+//		   	   	   return fmt.Errorf("CMD error: %s", err)
+//		   	   }
+//		   }
+//	}
+//
+// each command is executed in a separate shell, meaning their state is not preserved unless the state is flushed to the
+// hard disk of the remote machine. For example, the regular env variable export like "export JAVA_HOME=..." won't carry
+// over to the next command's execution context. The only way to preserve all in-memory states is to run everything in a
+// one-time script, which is how this function is implemented
+func Provision(ctx context.Context, ui packersdk.Ui, communicator packersdk.Communicator, commands []string) error {
+	scriptFile, err := loadCommandsIntoScript(commands)
+	if err != nil {
+		return err
+	}
+	defer os.Remove(scriptFile.Name())
+
+	ui.Say(fmt.Sprintf("Provisioning with %s", commands))
+
+	return executeScript(ctx, ui, communicator, scriptFile)
+}

provisioner/shell/provisioner_test.go

@@ -0,0 +1,37 @@
+// Copyright (c) Jiaqi Liu
+// SPDX-License-Identifier: MPL-2.0
+
+package shell
+
+import (
+	"os"
+	"testing"
+)
+
+func Test_loadCommandsIntoScript(t *testing.T) {
+	actualScript, err := loadCommandsIntoScript([]string{
+		"sudo apt update && sudo apt upgrade -y",
+		"sudo apt install software-properties-common -y",
+	})
+	if err != nil {
+		t.Error(err)
+	}
+	defer os.Remove(actualScript.Name())
+
+	b, err := os.ReadFile(actualScript.Name())
+	if err != nil {
+		t.Error(err)
+	}
+
+	expectedScript := `#!/bin/bash
+set -x
+set -e
+
+sudo apt update && sudo apt upgrade -y
+sudo apt install software-properties-common -y
+`
+
+	if string(b) != expectedScript {
+		t.Errorf("Expected and actual scripts do not match: %s\n\n%s", expectedScript, string(b))
+	}
+}

provisioner/ssl-provisioner/ssl-provisioner.go

@@ -7,8 +7,8 @@ import (
 	"context"
 	"encoding/base64"
 	"fmt"
-	basicProvisioner "github.com/QubitPi/packer-plugin-hashicorp-aws/provisioner/basic-provisioner"
 	fileProvisioner "github.com/QubitPi/packer-plugin-hashicorp-aws/provisioner/file-provisioner"
+	basicProvisioner "github.com/QubitPi/packer-plugin-hashicorp-aws/provisioner/shell"
 	packersdk "github.com/hashicorp/packer-plugin-sdk/packer"
 	"github.com/hashicorp/packer-plugin-sdk/template/interpolate"
 	"github.com/hashicorp/packer-plugin-sdk/tmp"

provisioner/webservice/provisioner.go

@@ -8,8 +8,8 @@ package webservice
 import (
 	"context"
 	"fmt"
-	basicProvisioner "github.com/QubitPi/packer-plugin-hashicorp-aws/provisioner/basic-provisioner"
 	fileProvisioner "github.com/QubitPi/packer-plugin-hashicorp-aws/provisioner/file-provisioner"
+	basicProvisioner "github.com/QubitPi/packer-plugin-hashicorp-aws/provisioner/shell"
 	sslProvisioner "github.com/QubitPi/packer-plugin-hashicorp-aws/provisioner/ssl-provisioner"
 	"github.com/hashicorp/hcl/v2/hcldec"
 	packersdk "github.com/hashicorp/packer-plugin-sdk/packer"