refactor aws test tooling so we can apply common test function helpers to ebsvolume tests

builder/amazon/common/access_config_test.go

@@ -5,20 +5,10 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/aws/session"
-	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
 )
 
-func testAccessConfig() *AccessConfig {
-	return &AccessConfig{
-		getEC2Connection: func() ec2iface.EC2API {
-			return &mockEC2Client{}
-		},
-		PollingConfig: new(AWSPollingConfig),
-	}
-}
-
 func TestAccessConfigPrepare_Region(t *testing.T) {
-	c := testAccessConfig()
+	c := FakeAccessConfig()
 
 	c.RawRegion = "us-east-12"
 	err := c.ValidateRegion(c.RawRegion)
@@ -40,7 +30,7 @@ func TestAccessConfigPrepare_Region(t *testing.T) {
 }
 
 func TestAccessConfigPrepare_RegionRestricted(t *testing.T) {
-	c := testAccessConfig()
+	c := FakeAccessConfig()
 
 	// Create a Session with a custom region
 	c.session = session.Must(session.NewSession(&aws.Config{

builder/amazon/common/ami_config_test.go

@@ -7,7 +7,6 @@ import (
 
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
-	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
 	"github.com/hashicorp/packer-plugin-sdk/template/config"
 )
 
@@ -18,14 +17,14 @@ func testAMIConfig() *AMIConfig {
 }
 
 func getFakeAccessConfig(region string) *AccessConfig {
-	c := testAccessConfig()
+	c := FakeAccessConfig()
 	c.RawRegion = region
 	return c
 }
 
 func TestAMIConfigPrepare_name(t *testing.T) {
 	c := testAMIConfig()
-	accessConf := testAccessConfig()
+	accessConf := FakeAccessConfig()
 	if err := c.Prepare(accessConf, nil); err != nil {
 		t.Fatalf("shouldn't have err: %s", err)
 	}
@@ -36,10 +35,6 @@ func TestAMIConfigPrepare_name(t *testing.T) {
 	}
 }
 
-type mockEC2Client struct {
-	ec2iface.EC2API
-}
-
 func (m *mockEC2Client) DescribeRegions(*ec2.DescribeRegionsInput) (*ec2.DescribeRegionsOutput, error) {
 	return &ec2.DescribeRegionsOutput{
 		Regions: []*ec2.Region{
@@ -56,7 +51,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) {
 
 	var errs []error
 	var err error
-	accessConf := testAccessConfig()
+	accessConf := FakeAccessConfig()
 	mockConn := &mockEC2Client{}
 	if errs = c.prepareRegions(accessConf); len(errs) > 0 {
 		t.Fatalf("shouldn't have err: %#v", errs)
@@ -163,7 +158,7 @@ func TestAMIConfigPrepare_Share_EncryptedBoot(t *testing.T) {
 	c.AMIUsers = []string{"testAccountID"}
 	c.AMIEncryptBootVolume = config.TriTrue
 
-	accessConf := testAccessConfig()
+	accessConf := FakeAccessConfig()
 
 	c.AMIKmsKeyId = ""
 	if err := c.Prepare(accessConf, nil); err == nil {
@@ -179,7 +174,7 @@ func TestAMIConfigPrepare_ValidateKmsKey(t *testing.T) {
 	c := testAMIConfig()
 	c.AMIEncryptBootVolume = config.TriTrue
 
-	accessConf := testAccessConfig()
+	accessConf := FakeAccessConfig()
 
 	validCases := []string{
 		"abcd1234-e567-890f-a12b-a123b4cd56ef",
@@ -215,7 +210,7 @@ func TestAMIConfigPrepare_ValidateKmsKey(t *testing.T) {
 func TestAMINameValidation(t *testing.T) {
 	c := testAMIConfig()
 
-	accessConf := testAccessConfig()
+	accessConf := FakeAccessConfig()
 
 	c.AMIName = "aa"
 	if err := c.Prepare(accessConf, nil); err == nil {

builder/amazon/common/step_ami_region_copy_test.go

@@ -105,7 +105,7 @@ func TestStepAMIRegionCopy_duplicates(t *testing.T) {
 	// ------------------------------------------------------------------------
 
 	stepAMIRegionCopy := StepAMIRegionCopy{
-		AccessConfig: testAccessConfig(),
+		AccessConfig: FakeAccessConfig(),
 		Regions:      []string{"us-east-1"},
 		AMIKmsKeyId:  "12345",
 		// Original region key in regionkeyids is different than in amikmskeyid
@@ -131,7 +131,7 @@ func TestStepAMIRegionCopy_duplicates(t *testing.T) {
 
 	// the ami is only copied once.
 	stepAMIRegionCopy = StepAMIRegionCopy{
-		AccessConfig:   testAccessConfig(),
+		AccessConfig:   FakeAccessConfig(),
 		Regions:        []string{"us-east-1"},
 		Name:           "fake-ami-name",
 		OriginalRegion: "us-east-1",
@@ -152,7 +152,7 @@ func TestStepAMIRegionCopy_duplicates(t *testing.T) {
 
 	// the ami is only copied once.
 	stepAMIRegionCopy = StepAMIRegionCopy{
-		AccessConfig:      testAccessConfig(),
+		AccessConfig:      FakeAccessConfig(),
 		Regions:           []string{"us-east-1"},
 		EncryptBootVolume: config.TriFalse,
 		Name:              "fake-ami-name",
@@ -174,7 +174,7 @@ func TestStepAMIRegionCopy_duplicates(t *testing.T) {
 	// ------------------------------------------------------------------------
 
 	stepAMIRegionCopy = StepAMIRegionCopy{
-		AccessConfig: testAccessConfig(),
+		AccessConfig: FakeAccessConfig(),
 		// Many duplicates for only 3 actual values
 		Regions:     []string{"us-east-1", "us-west-2", "us-west-2", "ap-east-1", "ap-east-1", "ap-east-1"},
 		AMIKmsKeyId: "IlikePancakes",
@@ -203,7 +203,7 @@ func TestStepAMIRegionCopy_duplicates(t *testing.T) {
 	// ------------------------------------------------------------------------
 
 	stepAMIRegionCopy = StepAMIRegionCopy{
-		AccessConfig: testAccessConfig(),
+		AccessConfig: FakeAccessConfig(),
 		// Many duplicates for only 3 actual values
 		Regions:        []string{"us-east-1", "us-west-2", "us-west-2", "ap-east-1", "ap-east-1", "ap-east-1"},
 		Name:           "fake-ami-name",
@@ -223,7 +223,7 @@ func TestStepAMIRegionCopy_duplicates(t *testing.T) {
 func TestStepAmiRegionCopy_nil_encryption(t *testing.T) {
 	// create step
 	stepAMIRegionCopy := StepAMIRegionCopy{
-		AccessConfig:      testAccessConfig(),
+		AccessConfig:      FakeAccessConfig(),
 		Regions:           make([]string, 0),
 		AMIKmsKeyId:       "",
 		RegionKeyIds:      make(map[string]string),
@@ -249,7 +249,7 @@ func TestStepAmiRegionCopy_nil_encryption(t *testing.T) {
 func TestStepAmiRegionCopy_true_encryption(t *testing.T) {
 	// create step
 	stepAMIRegionCopy := StepAMIRegionCopy{
-		AccessConfig:      testAccessConfig(),
+		AccessConfig:      FakeAccessConfig(),
 		Regions:           make([]string, 0),
 		AMIKmsKeyId:       "",
 		RegionKeyIds:      make(map[string]string),
@@ -275,7 +275,7 @@ func TestStepAmiRegionCopy_true_encryption(t *testing.T) {
 func TestStepAmiRegionCopy_nil_intermediary(t *testing.T) {
 	// create step
 	stepAMIRegionCopy := StepAMIRegionCopy{
-		AccessConfig:      testAccessConfig(),
+		AccessConfig:      FakeAccessConfig(),
 		Regions:           make([]string, 0),
 		AMIKmsKeyId:       "",
 		RegionKeyIds:      make(map[string]string),
@@ -303,7 +303,7 @@ func TestStepAmiRegionCopy_AMISkipBuildRegion(t *testing.T) {
 	// ------------------------------------------------------------------------
 
 	stepAMIRegionCopy := StepAMIRegionCopy{
-		AccessConfig:       testAccessConfig(),
+		AccessConfig:       FakeAccessConfig(),
 		Regions:            []string{"us-west-1"},
 		AMIKmsKeyId:        "",
 		RegionKeyIds:       map[string]string{"us-west-1": "abcde"},
@@ -329,7 +329,7 @@ func TestStepAmiRegionCopy_AMISkipBuildRegion(t *testing.T) {
 	// skip build region is false.
 	// ------------------------------------------------------------------------
 	stepAMIRegionCopy = StepAMIRegionCopy{
-		AccessConfig:       testAccessConfig(),
+		AccessConfig:       FakeAccessConfig(),
 		Regions:            []string{"us-west-1"},
 		AMIKmsKeyId:        "",
 		RegionKeyIds:       make(map[string]string),
@@ -354,7 +354,7 @@ func TestStepAmiRegionCopy_AMISkipBuildRegion(t *testing.T) {
 	// skip build region is false, but encrypt is true
 	// ------------------------------------------------------------------------
 	stepAMIRegionCopy = StepAMIRegionCopy{
-		AccessConfig:       testAccessConfig(),
+		AccessConfig:       FakeAccessConfig(),
 		Regions:            []string{"us-west-1"},
 		AMIKmsKeyId:        "",
 		RegionKeyIds:       map[string]string{"us-west-1": "abcde"},
@@ -380,7 +380,7 @@ func TestStepAmiRegionCopy_AMISkipBuildRegion(t *testing.T) {
 	// skip build region is true, and encrypt is true
 	// ------------------------------------------------------------------------
 	stepAMIRegionCopy = StepAMIRegionCopy{
-		AccessConfig:       testAccessConfig(),
+		AccessConfig:       FakeAccessConfig(),
 		Regions:            []string{"us-west-1"},
 		AMIKmsKeyId:        "",
 		RegionKeyIds:       map[string]string{"us-west-1": "abcde"},

builder/amazon/common/test_helper_funcs.go

@@ -0,0 +1,25 @@
+package common
+
+import (
+	"github.com/aws/aws-sdk-go/aws"
+	"github.com/aws/aws-sdk-go/aws/session"
+	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
+)
+
+type mockEC2Client struct {
+	ec2iface.EC2API
+}
+
+func FakeAccessConfig() *AccessConfig {
+	accessConfig := AccessConfig{
+		getEC2Connection: func() ec2iface.EC2API {
+			return &mockEC2Client{}
+		},
+		PollingConfig: new(AWSPollingConfig),
+	}
+	accessConfig.session = session.Must(session.NewSession(&aws.Config{
+		Region: aws.String("us-west-1"),
+	}))
+
+	return &accessConfig
+}

builder/amazon/ebsvolume/builder.go

@@ -317,7 +317,7 @@ func (b *Builder) Run(ctx context.Context, ui packersdk.Ui, hook packersdk.Hook)
 		&stepSnapshotEBSVolumes{
 			PollingConfig: b.config.PollingConfig,
 			VolumeMapping: b.config.VolumeMappings,
-			AccessConfig:  b.config.AccessConfig,
+			AccessConfig:  &b.config.AccessConfig,
 			Ctx:           b.config.ctx,
 		},
 	}

builder/amazon/ebsvolume/step_snapshot_ebs_volumes.go

@@ -7,18 +7,18 @@ import (
 	"github.com/aws/aws-sdk-go/aws"
 	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
-	awscommon "github.com/hashicorp/packer/builder/amazon/common"
 	"github.com/hashicorp/packer-plugin-sdk/multistep"
 	"github.com/hashicorp/packer-plugin-sdk/packer"
 	"github.com/hashicorp/packer-plugin-sdk/template/interpolate"
+	awscommon "github.com/hashicorp/packer/builder/amazon/common"
 )
 
 type stepSnapshotEBSVolumes struct {
 	PollingConfig *awscommon.AWSPollingConfig
-	AccessConfig  awscommon.AccessConfig
+	AccessConfig  *awscommon.AccessConfig
 	VolumeMapping []BlockDevice
 	//Map of SnapshotID: BlockDevice, Where *BlockDevice is in VolumeMapping
-	SnapshotMap map[string]*BlockDevice
+	snapshotMap map[string]*BlockDevice
 	Ctx         interpolate.Context
 }
 
@@ -27,7 +27,7 @@ func (s *stepSnapshotEBSVolumes) Run(ctx context.Context, state multistep.StateB
 	instance := state.Get("instance").(*ec2.Instance)
 	ui := state.Get("ui").(packer.Ui)
 
-	s.SnapshotMap = make(map[string]*BlockDevice)
+	s.snapshotMap = make(map[string]*BlockDevice)
 
 	for _, instanceBlockDevices := range instance.BlockDeviceMappings {
 		for _, configVolumeMapping := range s.VolumeMapping {
@@ -72,13 +72,13 @@ func (s *stepSnapshotEBSVolumes) Run(ctx context.Context, state multistep.StateB
 					return multistep.ActionHalt
 				}
 				ui.Message(fmt.Sprintf("Requested Snapshot of Volume %s: %s", *instanceBlockDevices.Ebs.VolumeId, *snapshot.SnapshotId))
-				s.SnapshotMap[*snapshot.SnapshotId] = &configVolumeMapping
+				s.snapshotMap[*snapshot.SnapshotId] = &configVolumeMapping
 			}
 		}
 	}
 
 	ui.Say("Waiting for Snapshots to become ready...")
-	for snapID := range s.SnapshotMap {
+	for snapID := range s.snapshotMap {
 		ui.Message(fmt.Sprintf("Waiting for %s to be ready.", snapID))
 		err := s.PollingConfig.WaitUntilSnapshotDone(ctx, ec2conn, snapID)
 		if err != nil {
@@ -93,7 +93,7 @@ func (s *stepSnapshotEBSVolumes) Run(ctx context.Context, state multistep.StateB
 
 	//Attach User and Group permissions to snapshots
 	ui.Say("Setting User/Group Permissions for Snapshots...")
-	for snapID, bd := range s.SnapshotMap {
+	for snapID, bd := range s.snapshotMap {
 		snapshotOptions := make(map[string]*ec2.ModifySnapshotAttributeInput)
 
 		if len(bd.SnapshotGroups) > 0 {
@@ -151,7 +151,7 @@ func (s *stepSnapshotEBSVolumes) Run(ctx context.Context, state multistep.StateB
 	snapshots := make(EbsSnapshots)
 	currentregion := s.AccessConfig.SessionRegion()
 
-	for snapID := range s.SnapshotMap {
+	for snapID := range s.snapshotMap {
 		snapshots[currentregion] = append(
 			snapshots[currentregion],
 			snapID)

builder/amazon/ebsvolume/step_snapshot_ebs_volumes_test.go

@@ -7,11 +7,13 @@ import (
 	"testing"
 
 	"github.com/aws/aws-sdk-go/aws"
+
 	//"github.com/aws/aws-sdk-go/service/ec2"
+	"github.com/aws/aws-sdk-go/service/ec2"
 	"github.com/aws/aws-sdk-go/service/ec2/ec2iface"
-	"github.com/hashicorp/packer/builder/amazon/common"
 	"github.com/hashicorp/packer-plugin-sdk/multistep"
 	"github.com/hashicorp/packer-plugin-sdk/packer"
+	"github.com/hashicorp/packer/builder/amazon/common"
 )
 
 // Define a mock struct to be used in unit tests for common aws steps.
@@ -76,17 +78,43 @@ func TestStepSnapshot_run_simple(t *testing.T) {
 	}
 
 	state := tState(t)
+	state.Put("instance", &ec2.Instance{
+		InstanceId: aws.String("instance-id"),
+	})
+
+	accessConfig := common.FakeAccessConfig()
+
+	volMap := BlockDevices{
+		{
+	awscommon.BlockDevice `mapstructure:",squash"`
+	// Key/value pair tags to apply to the volume. These are retained after the builder
+	// completes. This is a [template engine](/docs/templates/legacy_json_templates/engine), see
+	// [Build template data](#build-template-data) for more information.
+	Tags map[string]string `mapstructure:"tags" required:"false"`
+	// Same as [`tags`](#tags) but defined as a singular repeatable block
+	// containing a `key` and a `value` field. In HCL2 mode the
+	// [`dynamic_block`](/docs/templates/hcl_templates/expressions#dynamic-blocks)
+	// will allow you to create those programatically.
+	Tag config.KeyValues `mapstructure:"tag" required:"false"`
+
+	// Create a Snapshot of this Volume.
+	SnapshotVolume bool `mapstructure:"snapshot_volume" required:"false"`
+
+	awscommon.SnapshotConfig `mapstructure:",squash"`
+}
+}
 	//Todo add fake volumes, for the snap shot step to Snapshot
 
 	step := stepSnapshotEBSVolumes{
 		PollingConfig: new(common.AWSPollingConfig), //Dosnt look like builder sets this up
+		AccessConfig:  accessConfig,
 		VolumeMapping: b.config.VolumeMappings,
 		Ctx:           b.config.ctx,
 	}
 
 	step.Run(context.Background(), state)
 
-	if len(step.SnapshotMap) != 1 {
+	if len(step.snapshotMap) != 1 {
 		t.Fatalf("Missing Snapshot from step")
 	}
 }