diff --git a/builder/alicloud/ecs/builder_test.go b/builder/alicloud/ecs/builder_test.go index c6e54d793..e67861f5d 100644 --- a/builder/alicloud/ecs/builder_test.go +++ b/builder/alicloud/ecs/builder_test.go @@ -4,6 +4,7 @@ import ( "reflect" "testing" + helperconfig "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/packer" ) @@ -126,13 +127,15 @@ func TestBuilderPrepare_Devices(t *testing.T) { if err != nil { t.Fatalf("should not have error: %s", err) } - if !reflect.DeepEqual(b.config.ECSSystemDiskMapping, AlicloudDiskDevice{ + expected := AlicloudDiskDevice{ DiskCategory: "cloud", Description: "system disk", DiskName: "system_disk", DiskSize: 60, - }) { - t.Fatalf("system disk is not set properly, actual: %#v", b.config.ECSSystemDiskMapping) + Encrypted: helperconfig.TriUnset, + } + if !reflect.DeepEqual(b.config.ECSSystemDiskMapping, expected) { + t.Fatalf("system disk is not set properly, actual: %v; expected: %v", b.config.ECSSystemDiskMapping, expected) } if !reflect.DeepEqual(b.config.ECSImagesDiskMappings, []AlicloudDiskDevice{ { diff --git a/builder/alicloud/ecs/image_config.go b/builder/alicloud/ecs/image_config.go index 80193ff56..b0d82dd81 100644 --- a/builder/alicloud/ecs/image_config.go +++ b/builder/alicloud/ecs/image_config.go @@ -5,18 +5,19 @@ import ( "regexp" "strings" + "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/template/interpolate" ) type AlicloudDiskDevice struct { - DiskName string `mapstructure:"disk_name"` - DiskCategory string `mapstructure:"disk_category"` - DiskSize int `mapstructure:"disk_size"` - SnapshotId string `mapstructure:"disk_snapshot_id"` - Description string `mapstructure:"disk_description"` - DeleteWithInstance bool `mapstructure:"disk_delete_with_instance"` - Device string `mapstructure:"disk_device"` - Encrypted *bool `mapstructure:"disk_encrypted"` + DiskName string `mapstructure:"disk_name"` + DiskCategory string `mapstructure:"disk_category"` + DiskSize int `mapstructure:"disk_size"` + SnapshotId string `mapstructure:"disk_snapshot_id"` + Description string `mapstructure:"disk_description"` + DeleteWithInstance bool `mapstructure:"disk_delete_with_instance"` + Device string `mapstructure:"disk_device"` + Encrypted config.Trilean `mapstructure:"disk_encrypted"` } type AlicloudDiskDevices struct { @@ -32,7 +33,7 @@ type AlicloudImageConfig struct { AlicloudImageUNShareAccounts []string `mapstructure:"image_unshare_account"` AlicloudImageDestinationRegions []string `mapstructure:"image_copy_regions"` AlicloudImageDestinationNames []string `mapstructure:"image_copy_names"` - ImageEncrypted *bool `mapstructure:"image_encrypted"` + ImageEncrypted config.Trilean `mapstructure:"image_encrypted"` AlicloudImageForceDelete bool `mapstructure:"image_force_delete"` AlicloudImageForceDeleteSnapshots bool `mapstructure:"image_force_delete_snapshots"` AlicloudImageForceDeleteInstances bool `mapstructure:"image_force_delete_instances"` diff --git a/builder/alicloud/ecs/run_config.go b/builder/alicloud/ecs/run_config.go index ce4eda5b0..12e982aae 100644 --- a/builder/alicloud/ecs/run_config.go +++ b/builder/alicloud/ecs/run_config.go @@ -8,31 +8,32 @@ import ( "github.com/hashicorp/packer/common/uuid" "github.com/hashicorp/packer/helper/communicator" + "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/template/interpolate" ) type RunConfig struct { - AssociatePublicIpAddress bool `mapstructure:"associate_public_ip_address"` - ZoneId string `mapstructure:"zone_id"` - IOOptimized *bool `mapstructure:"io_optimized"` - InstanceType string `mapstructure:"instance_type"` - Description string `mapstructure:"description"` - AlicloudSourceImage string `mapstructure:"source_image"` - ForceStopInstance bool `mapstructure:"force_stop_instance"` - DisableStopInstance bool `mapstructure:"disable_stop_instance"` - SecurityGroupId string `mapstructure:"security_group_id"` - SecurityGroupName string `mapstructure:"security_group_name"` - UserData string `mapstructure:"user_data"` - UserDataFile string `mapstructure:"user_data_file"` - VpcId string `mapstructure:"vpc_id"` - VpcName string `mapstructure:"vpc_name"` - CidrBlock string `mapstructure:"vpc_cidr_block"` - VSwitchId string `mapstructure:"vswitch_id"` - VSwitchName string `mapstructure:"vswitch_id"` - InstanceName string `mapstructure:"instance_name"` - InternetChargeType string `mapstructure:"internet_charge_type"` - InternetMaxBandwidthOut int `mapstructure:"internet_max_bandwidth_out"` - WaitSnapshotReadyTimeout int `mapstructure:"wait_snapshot_ready_timeout"` + AssociatePublicIpAddress bool `mapstructure:"associate_public_ip_address"` + ZoneId string `mapstructure:"zone_id"` + IOOptimized config.Trilean `mapstructure:"io_optimized"` + InstanceType string `mapstructure:"instance_type"` + Description string `mapstructure:"description"` + AlicloudSourceImage string `mapstructure:"source_image"` + ForceStopInstance bool `mapstructure:"force_stop_instance"` + DisableStopInstance bool `mapstructure:"disable_stop_instance"` + SecurityGroupId string `mapstructure:"security_group_id"` + SecurityGroupName string `mapstructure:"security_group_name"` + UserData string `mapstructure:"user_data"` + UserDataFile string `mapstructure:"user_data_file"` + VpcId string `mapstructure:"vpc_id"` + VpcName string `mapstructure:"vpc_name"` + CidrBlock string `mapstructure:"vpc_cidr_block"` + VSwitchId string `mapstructure:"vswitch_id"` + VSwitchName string `mapstructure:"vswitch_id"` + InstanceName string `mapstructure:"instance_name"` + InternetChargeType string `mapstructure:"internet_charge_type"` + InternetMaxBandwidthOut int `mapstructure:"internet_max_bandwidth_out"` + WaitSnapshotReadyTimeout int `mapstructure:"wait_snapshot_ready_timeout"` // Communicator settings Comm communicator.Config `mapstructure:",squash"` diff --git a/builder/alicloud/ecs/step_create_image.go b/builder/alicloud/ecs/step_create_image.go index f69842733..9c8c45ec6 100644 --- a/builder/alicloud/ecs/step_create_image.go +++ b/builder/alicloud/ecs/step_create_image.go @@ -30,7 +30,7 @@ func (s *stepCreateAlicloudImage) Run(ctx context.Context, state multistep.State ui := state.Get("ui").(packer.Ui) tempImageName := config.AlicloudImageName - if config.ImageEncrypted != nil && *config.ImageEncrypted { + if config.ImageEncrypted.True() { tempImageName = fmt.Sprintf("packer_%s", random.AlphaNum(7)) ui.Say(fmt.Sprintf("Creating temporary image for encryption: %s", tempImageName)) } else { @@ -85,7 +85,7 @@ func (s *stepCreateAlicloudImage) Cleanup(state multistep.StateBag) { } config := state.Get("config").(*Config) - encryptedSet := config.ImageEncrypted != nil && *config.ImageEncrypted + encryptedSet := config.ImageEncrypted.True() _, cancelled := state.GetOk(multistep.StateCancelled) _, halted := state.GetOk(multistep.StateHalted) diff --git a/builder/alicloud/ecs/step_create_instance.go b/builder/alicloud/ecs/step_create_instance.go index ad3854751..909720e2d 100644 --- a/builder/alicloud/ecs/step_create_instance.go +++ b/builder/alicloud/ecs/step_create_instance.go @@ -12,12 +12,13 @@ import ( "github.com/aliyun/alibaba-cloud-sdk-go/sdk/requests" "github.com/aliyun/alibaba-cloud-sdk-go/sdk/responses" "github.com/aliyun/alibaba-cloud-sdk-go/services/ecs" + confighelper "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) type stepCreateAlicloudInstance struct { - IOOptimized *bool + IOOptimized confighelper.Trilean InstanceType string UserData string UserDataFile string @@ -142,12 +143,10 @@ func (s *stepCreateAlicloudInstance) buildCreateInstanceRequest(state multistep. request.InternetChargeType = s.InternetChargeType request.InternetMaxBandwidthOut = requests.Integer(convertNumber(s.InternetMaxBandwidthOut)) - if s.IOOptimized != nil { - if *s.IOOptimized { - request.IoOptimized = IOOptimizedOptimized - } else { - request.IoOptimized = IOOptimizedNone - } + if s.IOOptimized.True() { + request.IoOptimized = IOOptimizedOptimized + } else if s.IOOptimized.False() { + request.IoOptimized = IOOptimizedNone } config := state.Get("config").(*Config) @@ -174,8 +173,8 @@ func (s *stepCreateAlicloudInstance) buildCreateInstanceRequest(state multistep. dataDisk.Description = imageDisk.Description dataDisk.DeleteWithInstance = strconv.FormatBool(imageDisk.DeleteWithInstance) dataDisk.Device = imageDisk.Device - if imageDisk.Encrypted != nil { - dataDisk.Encrypted = strconv.FormatBool(*imageDisk.Encrypted) + if imageDisk.Encrypted != confighelper.TriUnset { + dataDisk.Encrypted = strconv.FormatBool(imageDisk.Encrypted.True()) } dataDisks = append(dataDisks, dataDisk) diff --git a/builder/alicloud/ecs/step_region_copy_image.go b/builder/alicloud/ecs/step_region_copy_image.go index 8fb30551b..ae5ebc693 100644 --- a/builder/alicloud/ecs/step_region_copy_image.go +++ b/builder/alicloud/ecs/step_region_copy_image.go @@ -7,6 +7,7 @@ import ( "github.com/aliyun/alibaba-cloud-sdk-go/sdk/requests" "github.com/aliyun/alibaba-cloud-sdk-go/services/ecs" + confighelper "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) @@ -20,7 +21,7 @@ type stepRegionCopyAlicloudImage struct { func (s *stepRegionCopyAlicloudImage) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { config := state.Get("config").(*Config) - if config.ImageEncrypted != nil { + if config.ImageEncrypted != confighelper.TriUnset { s.AlicloudImageDestinationRegions = append(s.AlicloudImageDestinationRegions, s.RegionId) s.AlicloudImageDestinationNames = append(s.AlicloudImageDestinationNames, config.AlicloudImageName) } @@ -38,7 +39,7 @@ func (s *stepRegionCopyAlicloudImage) Run(ctx context.Context, state multistep.S ui.Say(fmt.Sprintf("Coping image %s from %s...", srcImageId, s.RegionId)) for index, destinationRegion := range s.AlicloudImageDestinationRegions { - if destinationRegion == s.RegionId && config.ImageEncrypted == nil { + if destinationRegion == s.RegionId && config.ImageEncrypted == confighelper.TriUnset { continue } @@ -52,8 +53,8 @@ func (s *stepRegionCopyAlicloudImage) Run(ctx context.Context, state multistep.S copyImageRequest.ImageId = srcImageId copyImageRequest.DestinationRegionId = destinationRegion copyImageRequest.DestinationImageName = ecsImageName - if config.ImageEncrypted != nil { - copyImageRequest.Encrypted = requests.NewBoolean(*config.ImageEncrypted) + if config.ImageEncrypted != confighelper.TriUnset { + copyImageRequest.Encrypted = requests.NewBoolean(config.ImageEncrypted.True()) } imageResponse, err := client.CopyImage(copyImageRequest) @@ -65,7 +66,7 @@ func (s *stepRegionCopyAlicloudImage) Run(ctx context.Context, state multistep.S ui.Message(fmt.Sprintf("Copy image from %s(%s) to %s(%s)", s.RegionId, srcImageId, destinationRegion, imageResponse.ImageId)) } - if config.ImageEncrypted != nil { + if config.ImageEncrypted != confighelper.TriUnset { if _, err := client.WaitForImageStatus(s.RegionId, alicloudImages[s.RegionId], ImageStatusAvailable, time.Duration(ALICLOUD_DEFAULT_LONG_TIMEOUT)*time.Second); err != nil { return halt(state, err, fmt.Sprintf("Timeout waiting image %s finish copying", alicloudImages[s.RegionId])) } diff --git a/builder/amazon/chroot/step_register_ami.go b/builder/amazon/chroot/step_register_ami.go index 229c79dcd..64bc1bc7a 100644 --- a/builder/amazon/chroot/step_register_ami.go +++ b/builder/amazon/chroot/step_register_ami.go @@ -7,6 +7,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" awscommon "github.com/hashicorp/packer/builder/amazon/common" + confighelper "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) @@ -14,7 +15,7 @@ import ( // StepRegisterAMI creates the AMI. type StepRegisterAMI struct { RootVolumeSize int64 - EnableAMIENASupport *bool + EnableAMIENASupport confighelper.Trilean EnableAMISriovNetSupport bool } @@ -41,7 +42,7 @@ func (s *StepRegisterAMI) Run(ctx context.Context, state multistep.StateBag) mul // As of February 2017, this applies to C3, C4, D2, I2, R3, and M4 (excluding m4.16xlarge) registerOpts.SriovNetSupport = aws.String("simple") } - if s.EnableAMIENASupport != nil && *s.EnableAMIENASupport { + if s.EnableAMIENASupport.True() { // Set EnaSupport to true // As of February 2017, this applies to C5, I3, P2, R4, X1, and m4.16xlarge registerOpts.EnaSupport = aws.Bool(true) diff --git a/builder/amazon/common/ami_config.go b/builder/amazon/common/ami_config.go index 2069ab23f..c842a4aca 100644 --- a/builder/amazon/common/ami_config.go +++ b/builder/amazon/common/ami_config.go @@ -5,6 +5,7 @@ import ( "log" "regexp" + "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/template/interpolate" ) @@ -19,11 +20,11 @@ type AMIConfig struct { AMIRegions []string `mapstructure:"ami_regions"` AMISkipRegionValidation bool `mapstructure:"skip_region_validation"` AMITags TagMap `mapstructure:"tags"` - AMIENASupport *bool `mapstructure:"ena_support"` + AMIENASupport config.Trilean `mapstructure:"ena_support"` AMISriovNetSupport bool `mapstructure:"sriov_support"` AMIForceDeregister bool `mapstructure:"force_deregister"` AMIForceDeleteSnapshot bool `mapstructure:"force_delete_snapshot"` - AMIEncryptBootVolume *bool `mapstructure:"encrypt_boot"` + AMIEncryptBootVolume config.Trilean `mapstructure:"encrypt_boot"` AMIKmsKeyId string `mapstructure:"kms_key_id"` AMIRegionKMSKeyIDs map[string]string `mapstructure:"region_kms_key_ids"` SnapshotTags TagMap `mapstructure:"snapshot_tags"` @@ -62,7 +63,7 @@ func (c *AMIConfig) Prepare(accessConfig *AccessConfig, ctx *interpolate.Context // Prevent sharing of default KMS key encrypted volumes with other aws users if len(c.AMIUsers) > 0 { - if len(c.AMIKmsKeyId) == 0 && c.AMIEncryptBootVolume != nil && *c.AMIEncryptBootVolume { + if len(c.AMIKmsKeyId) == 0 && c.AMIEncryptBootVolume.True() { errs = append(errs, fmt.Errorf("Cannot share AMI encrypted with default KMS key")) } if len(c.AMIRegionKMSKeyIDs) > 0 { @@ -92,7 +93,7 @@ func (c *AMIConfig) Prepare(accessConfig *AccessConfig, ctx *interpolate.Context } if len(c.SnapshotUsers) > 0 { - if len(c.AMIKmsKeyId) == 0 && c.AMIEncryptBootVolume != nil && *c.AMIEncryptBootVolume { + if len(c.AMIKmsKeyId) == 0 && c.AMIEncryptBootVolume.True() { errs = append(errs, fmt.Errorf("Cannot share snapshot encrypted with default KMS key")) } if len(c.AMIRegionKMSKeyIDs) > 0 { diff --git a/builder/amazon/common/ami_config_test.go b/builder/amazon/common/ami_config_test.go index 0e72d30dc..8430c964c 100644 --- a/builder/amazon/common/ami_config_test.go +++ b/builder/amazon/common/ami_config_test.go @@ -8,6 +8,7 @@ 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/helper/config" ) func testAMIConfig() *AMIConfig { @@ -138,7 +139,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) { c.SnapshotUsers = []string{"foo", "bar"} c.AMIKmsKeyId = "123-abc-456" - c.AMIEncryptBootVolume = &[]bool{true}[0] + c.AMIEncryptBootVolume = config.TriTrue c.AMIRegions = []string{"us-east-1", "us-west-1"} c.AMIRegionKMSKeyIDs = map[string]string{ "us-east-1": "123-456-7890", @@ -161,7 +162,7 @@ func TestAMIConfigPrepare_regions(t *testing.T) { func TestAMIConfigPrepare_Share_EncryptedBoot(t *testing.T) { c := testAMIConfig() c.AMIUsers = []string{"testAccountID"} - c.AMIEncryptBootVolume = &[]bool{true}[0] + c.AMIEncryptBootVolume = config.TriTrue accessConf := testAccessConfig() @@ -177,7 +178,7 @@ func TestAMIConfigPrepare_Share_EncryptedBoot(t *testing.T) { func TestAMIConfigPrepare_ValidateKmsKey(t *testing.T) { c := testAMIConfig() - c.AMIEncryptBootVolume = aws.Bool(true) + c.AMIEncryptBootVolume = config.TriTrue accessConf := testAccessConfig() diff --git a/builder/amazon/common/block_device.go b/builder/amazon/common/block_device.go index 88e066031..49b87c84a 100644 --- a/builder/amazon/common/block_device.go +++ b/builder/amazon/common/block_device.go @@ -6,21 +6,22 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/template/interpolate" ) // BlockDevice type BlockDevice struct { - DeleteOnTermination bool `mapstructure:"delete_on_termination"` - DeviceName string `mapstructure:"device_name"` - Encrypted *bool `mapstructure:"encrypted"` - IOPS int64 `mapstructure:"iops"` - NoDevice bool `mapstructure:"no_device"` - SnapshotId string `mapstructure:"snapshot_id"` - VirtualName string `mapstructure:"virtual_name"` - VolumeType string `mapstructure:"volume_type"` - VolumeSize int64 `mapstructure:"volume_size"` - KmsKeyId string `mapstructure:"kms_key_id"` + DeleteOnTermination bool `mapstructure:"delete_on_termination"` + DeviceName string `mapstructure:"device_name"` + Encrypted config.Trilean `mapstructure:"encrypted"` + IOPS int64 `mapstructure:"iops"` + NoDevice bool `mapstructure:"no_device"` + SnapshotId string `mapstructure:"snapshot_id"` + VirtualName string `mapstructure:"virtual_name"` + VolumeType string `mapstructure:"volume_type"` + VolumeSize int64 `mapstructure:"volume_size"` + KmsKeyId string `mapstructure:"kms_key_id"` // ebssurrogate only OmitFromArtifact bool `mapstructure:"omit_from_artifact"` } @@ -74,7 +75,8 @@ func buildBlockDevices(b []BlockDevice) []*ec2.BlockDeviceMapping { if blockDevice.SnapshotId != "" { ebsBlockDevice.SnapshotId = aws.String(blockDevice.SnapshotId) } - ebsBlockDevice.Encrypted = blockDevice.Encrypted + + ebsBlockDevice.Encrypted = blockDevice.Encrypted.ToBoolPointer() if blockDevice.KmsKeyId != "" { ebsBlockDevice.KmsKeyId = aws.String(blockDevice.KmsKeyId) @@ -93,8 +95,9 @@ func (b *BlockDevice) Prepare(ctx *interpolate.Context) error { return fmt.Errorf("The `device_name` must be specified " + "for every device in the block device mapping.") } + // Warn that encrypted must be true or nil when setting kms_key_id - if b.KmsKeyId != "" && b.Encrypted != nil && *b.Encrypted == false { + if b.KmsKeyId != "" && b.Encrypted.False() { return fmt.Errorf("The device %v, must also have `encrypted: "+ "true` when setting a kms_key_id.", b.DeviceName) } diff --git a/builder/amazon/common/block_device_test.go b/builder/amazon/common/block_device_test.go index b2c655110..a390c805b 100644 --- a/builder/amazon/common/block_device_test.go +++ b/builder/amazon/common/block_device_test.go @@ -6,6 +6,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/hashicorp/packer/helper/config" ) func TestBlockDevice(t *testing.T) { @@ -71,7 +72,7 @@ func TestBlockDevice(t *testing.T) { VolumeType: "gp2", VolumeSize: 8, DeleteOnTermination: true, - Encrypted: aws.Bool(true), + Encrypted: config.TriTrue, }, Result: &ec2.BlockDeviceMapping{ @@ -90,7 +91,7 @@ func TestBlockDevice(t *testing.T) { VolumeType: "gp2", VolumeSize: 8, DeleteOnTermination: true, - Encrypted: aws.Bool(true), + Encrypted: config.TriTrue, KmsKeyId: "2Fa48a521f-3aff-4b34-a159-376ac5d37812", }, diff --git a/builder/amazon/common/step_ami_region_copy.go b/builder/amazon/common/step_ami_region_copy.go index ca77c6e63..323a9a0db 100644 --- a/builder/amazon/common/step_ami_region_copy.go +++ b/builder/amazon/common/step_ami_region_copy.go @@ -8,6 +8,7 @@ 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/helper/config" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) @@ -17,7 +18,7 @@ type StepAMIRegionCopy struct { Regions []string AMIKmsKeyId string RegionKeyIds map[string]string - EncryptBootVolume *bool // nil means preserve + EncryptBootVolume config.Trilean // nil means preserve Name string OriginalRegion string @@ -74,7 +75,7 @@ func (s *StepAMIRegionCopy) Run(ctx context.Context, state multistep.StateBag) m s.toDelete = ami } - if s.EncryptBootVolume != nil && *s.EncryptBootVolume { + if s.EncryptBootVolume.True() { // encrypt_boot is true, so we have to copy the temporary // AMI with required encryption setting. // temp image was created by stepCreateAMI. @@ -102,7 +103,7 @@ func (s *StepAMIRegionCopy) Run(ctx context.Context, state multistep.StateBag) m var regKeyID string ui.Message(fmt.Sprintf("Copying to: %s", region)) - if s.EncryptBootVolume != nil && *s.EncryptBootVolume { + if s.EncryptBootVolume.True() { // Encrypt is true, explicitly regKeyID = s.RegionKeyIds[region] } else { @@ -112,7 +113,9 @@ func (s *StepAMIRegionCopy) Run(ctx context.Context, state multistep.StateBag) m go func(region string) { defer wg.Done() - id, snapshotIds, err := s.amiRegionCopy(ctx, state, s.AccessConfig, s.Name, ami, region, s.OriginalRegion, regKeyID, s.EncryptBootVolume) + id, snapshotIds, err := s.amiRegionCopy(ctx, state, s.AccessConfig, + s.Name, ami, region, s.OriginalRegion, regKeyID, + s.EncryptBootVolume.ToBoolPointer()) lock.Lock() defer lock.Unlock() amis[region] = id diff --git a/builder/amazon/common/step_ami_region_copy_test.go b/builder/amazon/common/step_ami_region_copy_test.go index 2e1628aab..9fc636519 100644 --- a/builder/amazon/common/step_ami_region_copy_test.go +++ b/builder/amazon/common/step_ami_region_copy_test.go @@ -11,6 +11,7 @@ import ( "github.com/aws/aws-sdk-go/aws/request" "github.com/aws/aws-sdk-go/service/ec2" "github.com/aws/aws-sdk-go/service/ec2/ec2iface" + "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) @@ -109,7 +110,7 @@ func TestStepAMIRegionCopy_duplicates(t *testing.T) { AMIKmsKeyId: "12345", // Original region key in regionkeyids is different than in amikmskeyid RegionKeyIds: map[string]string{"us-east-1": "12345"}, - EncryptBootVolume: aws.Bool(true), + EncryptBootVolume: config.TriTrue, Name: "fake-ami-name", OriginalRegion: "us-east-1", } @@ -153,7 +154,7 @@ func TestStepAMIRegionCopy_duplicates(t *testing.T) { stepAMIRegionCopy = StepAMIRegionCopy{ AccessConfig: testAccessConfig(), Regions: []string{"us-east-1"}, - EncryptBootVolume: aws.Bool(false), + EncryptBootVolume: config.TriFalse, Name: "fake-ami-name", OriginalRegion: "us-east-1", } @@ -179,7 +180,7 @@ func TestStepAMIRegionCopy_duplicates(t *testing.T) { AMIKmsKeyId: "IlikePancakes", // Original region key in regionkeyids is different than in amikmskeyid RegionKeyIds: map[string]string{"us-east-1": "12345", "us-west-2": "abcde", "ap-east-1": "xyz"}, - EncryptBootVolume: aws.Bool(true), + EncryptBootVolume: config.TriTrue, Name: "fake-ami-name", OriginalRegion: "us-east-1", } @@ -226,7 +227,7 @@ func TestStepAmiRegionCopy_nil_encryption(t *testing.T) { Regions: make([]string, 0), AMIKmsKeyId: "", RegionKeyIds: make(map[string]string), - EncryptBootVolume: nil, + EncryptBootVolume: config.TriUnset, Name: "fake-ami-name", OriginalRegion: "us-east-1", } @@ -252,7 +253,7 @@ func TestStepAmiRegionCopy_true_encryption(t *testing.T) { Regions: make([]string, 0), AMIKmsKeyId: "", RegionKeyIds: make(map[string]string), - EncryptBootVolume: aws.Bool(true), + EncryptBootVolume: config.TriTrue, Name: "fake-ami-name", OriginalRegion: "us-east-1", } @@ -278,7 +279,7 @@ func TestStepAmiRegionCopy_nil_intermediary(t *testing.T) { Regions: make([]string, 0), AMIKmsKeyId: "", RegionKeyIds: make(map[string]string), - EncryptBootVolume: aws.Bool(false), + EncryptBootVolume: config.TriFalse, Name: "fake-ami-name", OriginalRegion: "us-east-1", } @@ -360,7 +361,7 @@ func TestStepAmiRegionCopy_AMISkipBuildRegion(t *testing.T) { Name: "fake-ami-name", OriginalRegion: "us-east-1", AMISkipBuildRegion: false, - EncryptBootVolume: aws.Bool(true), + EncryptBootVolume: config.TriTrue, } // mock out the region connection code stepAMIRegionCopy.getRegionConn = getMockConn @@ -386,7 +387,7 @@ func TestStepAmiRegionCopy_AMISkipBuildRegion(t *testing.T) { Name: "fake-ami-name", OriginalRegion: "us-east-1", AMISkipBuildRegion: true, - EncryptBootVolume: aws.Bool(true), + EncryptBootVolume: config.TriTrue, } // mock out the region connection code stepAMIRegionCopy.getRegionConn = getMockConn diff --git a/builder/amazon/common/step_modify_ebs_instance.go b/builder/amazon/common/step_modify_ebs_instance.go index a8e5a256e..1902c1645 100644 --- a/builder/amazon/common/step_modify_ebs_instance.go +++ b/builder/amazon/common/step_modify_ebs_instance.go @@ -5,19 +5,20 @@ import ( "fmt" "strings" - "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/ec2/ec2iface" + confighelper "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) type StepModifyEBSBackedInstance struct { - EnableAMIENASupport *bool + EnableAMIENASupport confighelper.Trilean EnableAMISriovNetSupport bool } func (s *StepModifyEBSBackedInstance) Run(ctx context.Context, state multistep.StateBag) multistep.StepAction { - ec2conn := state.Get("ec2").(*ec2.EC2) + ec2conn := state.Get("ec2").(ec2iface.EC2API) instance := state.Get("instance").(*ec2.Instance) ui := state.Get("ui").(packer.Ui) @@ -40,9 +41,9 @@ func (s *StepModifyEBSBackedInstance) Run(ctx context.Context, state multistep.S // Handle EnaSupport flag. // As of February 2017, this applies to C5, I3, P2, R4, X1, and m4.16xlarge - if s.EnableAMIENASupport != nil { + if s.EnableAMIENASupport != confighelper.TriUnset { var prefix string - if *s.EnableAMIENASupport { + if s.EnableAMIENASupport.True() { prefix = "En" } else { prefix = "Dis" @@ -50,7 +51,7 @@ func (s *StepModifyEBSBackedInstance) Run(ctx context.Context, state multistep.S ui.Say(fmt.Sprintf("%sabling Enhanced Networking (ENA)...", prefix)) _, err := ec2conn.ModifyInstanceAttribute(&ec2.ModifyInstanceAttributeInput{ InstanceId: instance.InstanceId, - EnaSupport: &ec2.AttributeBooleanValue{Value: aws.Bool(*s.EnableAMIENASupport)}, + EnaSupport: &ec2.AttributeBooleanValue{Value: s.EnableAMIENASupport.ToBoolPointer()}, }) if err != nil { err := fmt.Errorf("Error %sabling Enhanced Networking (ENA) on %s: %s", strings.ToLower(prefix), *instance.InstanceId, err) diff --git a/builder/amazon/common/step_modify_ebs_instance_test.go b/builder/amazon/common/step_modify_ebs_instance_test.go new file mode 100644 index 000000000..cb8a4b020 --- /dev/null +++ b/builder/amazon/common/step_modify_ebs_instance_test.go @@ -0,0 +1,105 @@ +package common + +import ( + "bytes" + "context" + "fmt" + "testing" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/service/ec2" + "github.com/aws/aws-sdk-go/service/ec2/ec2iface" + helperconfig "github.com/hashicorp/packer/helper/config" + "github.com/hashicorp/packer/helper/multistep" + "github.com/hashicorp/packer/packer" +) + +// Define a mock struct to be used in unit tests for common aws steps. +type mockEC2Conn_ModifyEBS struct { + ec2iface.EC2API + Config *aws.Config + + // Counters to figure out what code path was taken + shouldError bool + modifyImageAttrCount int +} + +func (m *mockEC2Conn_ModifyEBS) ModifyInstanceAttribute(modifyInput *ec2.ModifyInstanceAttributeInput) (*ec2.ModifyInstanceAttributeOutput, error) { + m.modifyImageAttrCount++ + // don't need to define output since we always discard it anyway. + output := &ec2.ModifyInstanceAttributeOutput{} + if m.shouldError { + return output, fmt.Errorf("fake ModifyInstanceAttribute error") + } + return output, nil +} + +// Create statebag for running test +func fakeModifyEBSBackedInstanceState() multistep.StateBag { + state := new(multistep.BasicStateBag) + state.Put("ui", &packer.BasicUi{ + Reader: new(bytes.Buffer), + Writer: new(bytes.Buffer), + }) + state.Put("instance", "i-12345") + return state +} + +func StepModifyEBSBackedInstance_EnableAMIENASupport(t *testing.T) { + // Value is unset, so we shouldn't modify + stepModifyEBSBackedInstance := StepModifyEBSBackedInstance{ + EnableAMIENASupport: helperconfig.TriUnset, + EnableAMISriovNetSupport: false, + } + + // mock out the region connection code + mockConn := &mockEC2Conn_ModifyEBS{ + Config: aws.NewConfig(), + } + + state := fakeModifyEBSBackedInstanceState() + state.Put("ec2", mockConn) + stepModifyEBSBackedInstance.Run(context.Background(), state) + + if mockConn.modifyImageAttrCount > 0 { + t.Fatalf("Should not have modified image since EnableAMIENASupport is unset") + } + + // Value is true, so we should modify + stepModifyEBSBackedInstance = StepModifyEBSBackedInstance{ + EnableAMIENASupport: helperconfig.TriTrue, + EnableAMISriovNetSupport: false, + } + + // mock out the region connection code + mockConn = &mockEC2Conn_ModifyEBS{ + Config: aws.NewConfig(), + } + + state = fakeModifyEBSBackedInstanceState() + state.Put("ec2", mockConn) + stepModifyEBSBackedInstance.Run(context.Background(), state) + + if mockConn.modifyImageAttrCount != 1 { + t.Fatalf("Should have modified image, since EnableAMIENASupport is true") + } + + // Value is false, so we should modify + stepModifyEBSBackedInstance = StepModifyEBSBackedInstance{ + EnableAMIENASupport: helperconfig.TriFalse, + EnableAMISriovNetSupport: false, + } + + // mock out the region connection code + mockConn = &mockEC2Conn_ModifyEBS{ + Config: aws.NewConfig(), + } + + state = fakeModifyEBSBackedInstanceState() + state.Put("ec2", mockConn) + stepModifyEBSBackedInstance.Run(context.Background(), state) + + if mockConn.modifyImageAttrCount != 1 { + t.Fatalf("Should have modified image, since EnableAMIENASupport is true") + } +} diff --git a/builder/amazon/common/step_source_ami_info.go b/builder/amazon/common/step_source_ami_info.go index 09bbe53e8..c80f6a6cb 100644 --- a/builder/amazon/common/step_source_ami_info.go +++ b/builder/amazon/common/step_source_ami_info.go @@ -8,6 +8,7 @@ import ( "time" "github.com/aws/aws-sdk-go/service/ec2" + confighelper "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) @@ -20,7 +21,7 @@ import ( type StepSourceAMIInfo struct { SourceAmi string EnableAMISriovNetSupport bool - EnableAMIENASupport *bool + EnableAMIENASupport confighelper.Trilean AMIVirtType string AmiFilters AmiFilterOptions } @@ -94,7 +95,7 @@ func (s *StepSourceAMIInfo) Run(ctx context.Context, state multistep.StateBag) m // Enhanced Networking can only be enabled on HVM AMIs. // See http://goo.gl/icuXh5 - if (s.EnableAMIENASupport != nil && *s.EnableAMIENASupport) || s.EnableAMISriovNetSupport { + if s.EnableAMIENASupport.True() || s.EnableAMISriovNetSupport { err = s.canEnableEnhancedNetworking(image) if err != nil { state.Put("error", err) diff --git a/builder/amazon/ebs/builder.go b/builder/amazon/ebs/builder.go index d52539812..b25550818 100644 --- a/builder/amazon/ebs/builder.go +++ b/builder/amazon/ebs/builder.go @@ -72,7 +72,7 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { errs = packer.MultiErrorAppend(errs, b.config.BlockDevices.Prepare(&b.config.ctx)...) errs = packer.MultiErrorAppend(errs, b.config.RunConfig.Prepare(&b.config.ctx)...) - if b.config.IsSpotInstance() && ((b.config.AMIENASupport != nil && *b.config.AMIENASupport) || b.config.AMISriovNetSupport) { + if b.config.IsSpotInstance() && (b.config.AMIENASupport.True() || b.config.AMISriovNetSupport) { errs = packer.MultiErrorAppend(errs, fmt.Errorf("Spot instances do not support modification, which is required "+ "when either `ena_support` or `sriov_support` are set. Please ensure "+ diff --git a/builder/amazon/ebs/step_create_ami.go b/builder/amazon/ebs/step_create_ami.go index d4d35638f..46347f7a3 100644 --- a/builder/amazon/ebs/step_create_ami.go +++ b/builder/amazon/ebs/step_create_ami.go @@ -27,7 +27,7 @@ func (s *stepCreateAMI) Run(ctx context.Context, state multistep.StateBag) multi // Create the image amiName := config.AMIName state.Put("intermediary_image", false) - if config.AMIEncryptBootVolume != nil && *config.AMIEncryptBootVolume != false || s.AMISkipBuildRegion { + if config.AMIEncryptBootVolume.True() || s.AMISkipBuildRegion { state.Put("intermediary_image", true) // From AWS SDK docs: You can encrypt a copy of an unencrypted snapshot, diff --git a/builder/amazon/ebssurrogate/builder.go b/builder/amazon/ebssurrogate/builder.go index 0c1219063..1b18821ae 100644 --- a/builder/amazon/ebssurrogate/builder.go +++ b/builder/amazon/ebssurrogate/builder.go @@ -90,7 +90,7 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { errs = packer.MultiErrorAppend(errs, fmt.Errorf("no volume with name '%s' is found", b.config.RootDevice.SourceDeviceName)) } - if b.config.IsSpotInstance() && ((b.config.AMIENASupport != nil && *b.config.AMIENASupport) || b.config.AMISriovNetSupport) { + if b.config.IsSpotInstance() && (b.config.AMIENASupport.True() || b.config.AMISriovNetSupport) { errs = packer.MultiErrorAppend(errs, fmt.Errorf("Spot instances do not support modification, which is required "+ "when either `ena_support` or `sriov_support` are set. Please ensure "+ diff --git a/builder/amazon/ebssurrogate/step_register_ami.go b/builder/amazon/ebssurrogate/step_register_ami.go index 743d17a1a..6886b49bd 100644 --- a/builder/amazon/ebssurrogate/step_register_ami.go +++ b/builder/amazon/ebssurrogate/step_register_ami.go @@ -7,6 +7,7 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" awscommon "github.com/hashicorp/packer/builder/amazon/common" + confighelper "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) @@ -16,7 +17,7 @@ type StepRegisterAMI struct { RootDevice RootBlockDevice AMIDevices []*ec2.BlockDeviceMapping LaunchDevices []*ec2.BlockDeviceMapping - EnableAMIENASupport *bool + EnableAMIENASupport confighelper.Trilean EnableAMISriovNetSupport bool Architecture string image *ec2.Image @@ -46,7 +47,7 @@ func (s *StepRegisterAMI) Run(ctx context.Context, state multistep.StateBag) mul // As of February 2017, this applies to C3, C4, D2, I2, R3, and M4 (excluding m4.16xlarge) registerOpts.SriovNetSupport = aws.String("simple") } - if s.EnableAMIENASupport != nil && *s.EnableAMIENASupport { + if s.EnableAMIENASupport.True() { // Set EnaSupport to true // As of February 2017, this applies to C5, I3, P2, R4, X1, and m4.16xlarge registerOpts.EnaSupport = aws.Bool(true) diff --git a/builder/amazon/ebsvolume/builder.go b/builder/amazon/ebsvolume/builder.go index 8a5b840ae..29861174b 100644 --- a/builder/amazon/ebsvolume/builder.go +++ b/builder/amazon/ebsvolume/builder.go @@ -23,7 +23,7 @@ type Config struct { awscommon.AccessConfig `mapstructure:",squash"` awscommon.RunConfig `mapstructure:",squash"` - AMIENASupport *bool `mapstructure:"ena_support"` + AMIENASupport config.Trilean `mapstructure:"ena_support"` AMISriovNetSupport bool `mapstructure:"sriov_support"` VolumeMappings []BlockDevice `mapstructure:"ebs_volumes"` VolumeRunTags awscommon.TagMap `mapstructure:"run_volume_tags"` @@ -76,7 +76,7 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { errs = packer.MultiErrorAppend(errs, err) } - if b.config.IsSpotInstance() && ((b.config.AMIENASupport != nil && *b.config.AMIENASupport) || b.config.AMISriovNetSupport) { + if b.config.IsSpotInstance() && ((b.config.AMIENASupport.True()) || b.config.AMISriovNetSupport) { errs = packer.MultiErrorAppend(errs, fmt.Errorf("Spot instances do not support modification, which is required "+ "when either `ena_support` or `sriov_support` are set. Please ensure "+ diff --git a/builder/amazon/instance/builder.go b/builder/amazon/instance/builder.go index 6622a0610..c880cb0f2 100644 --- a/builder/amazon/instance/builder.go +++ b/builder/amazon/instance/builder.go @@ -157,7 +157,7 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { errs, fmt.Errorf("x509_key_path points to bad file: %s", err)) } - if b.config.IsSpotInstance() && ((b.config.AMIENASupport != nil && *b.config.AMIENASupport) || b.config.AMISriovNetSupport) { + if b.config.IsSpotInstance() && ((b.config.AMIENASupport.True()) || b.config.AMISriovNetSupport) { errs = packer.MultiErrorAppend(errs, fmt.Errorf("Spot instances do not support modification, which is required "+ "when either `ena_support` or `sriov_support` are set. Please ensure "+ diff --git a/builder/amazon/instance/step_register_ami.go b/builder/amazon/instance/step_register_ami.go index 2dfcda02b..000fc3189 100644 --- a/builder/amazon/instance/step_register_ami.go +++ b/builder/amazon/instance/step_register_ami.go @@ -7,12 +7,13 @@ import ( "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/service/ec2" awscommon "github.com/hashicorp/packer/builder/amazon/common" + confighelper "github.com/hashicorp/packer/helper/config" "github.com/hashicorp/packer/helper/multistep" "github.com/hashicorp/packer/packer" ) type StepRegisterAMI struct { - EnableAMIENASupport *bool + EnableAMIENASupport confighelper.Trilean EnableAMISriovNetSupport bool } @@ -38,7 +39,7 @@ func (s *StepRegisterAMI) Run(ctx context.Context, state multistep.StateBag) mul // As of February 2017, this applies to C3, C4, D2, I2, R3, and M4 (excluding m4.16xlarge) registerOpts.SriovNetSupport = aws.String("simple") } - if s.EnableAMIENASupport != nil && *s.EnableAMIENASupport { + if s.EnableAMIENASupport.True() { // Set EnaSupport to true // As of February 2017, this applies to C5, I3, P2, R4, X1, and m4.16xlarge registerOpts.EnaSupport = aws.Bool(true) diff --git a/helper/config/custom_types.go b/helper/config/custom_types.go new file mode 100644 index 000000000..df7038459 --- /dev/null +++ b/helper/config/custom_types.go @@ -0,0 +1,72 @@ +package config + +import ( + "strconv" +) + +type Trilean uint8 + +const ( + // This will assign unset to 0, which is the default value in interpolation + TriUnset Trilean = iota + TriTrue + TriFalse +) + +func (t Trilean) ToString() string { + if t == TriTrue { + return "TriTrue" + } else if t == TriFalse { + return "TriFalse" + } + return "TriUnset" +} + +func (t Trilean) ToBoolPointer() *bool { + if t == TriTrue { + return boolPointer(true) + } else if t == TriFalse { + return boolPointer(false) + } + return nil +} + +func (t Trilean) True() bool { + if t == TriTrue { + return true + } + return false +} + +func (t Trilean) False() bool { + if t == TriFalse { + return true + } + return false +} + +func TrileanFromString(s string) (Trilean, error) { + if s == "" { + return TriUnset, nil + } + + b, err := strconv.ParseBool(s) + if err != nil { + return TriUnset, err + } else if b == true { + return TriTrue, nil + } else { + return TriFalse, nil + } +} + +func TrileanFromBool(b bool) Trilean { + if b { + return TriTrue + } + return TriFalse +} + +func boolPointer(b bool) *bool { + return &b +} diff --git a/helper/config/custom_types_test.go b/helper/config/custom_types_test.go new file mode 100644 index 000000000..2a7cb933a --- /dev/null +++ b/helper/config/custom_types_test.go @@ -0,0 +1,30 @@ +package config + +import ( + "testing" +) + +func TestTrilianParsing(t *testing.T) { + type testCase struct { + Input string + Output Trilean + ErrExpected bool + } + testCases := []testCase{ + {"true", TriTrue, false}, {"True", TriTrue, false}, + {"false", TriFalse, false}, {"False", TriFalse, false}, + {"", TriUnset, false}, {"badvalue", TriUnset, true}, + {"FAlse", TriUnset, true}, {"TrUe", TriUnset, true}, + } + for _, tc := range testCases { + tril, err := TrileanFromString(tc.Input) + if err != nil { + if tc.ErrExpected == false { + t.Fatalf("Didn't expect error: %v", tc) + } + } + if tc.Output != tril { + t.Fatalf("Didn't return proper trilean. %v", tc) + } + } +} diff --git a/helper/config/decode.go b/helper/config/decode.go index efb17cb00..43673e1dc 100644 --- a/helper/config/decode.go +++ b/helper/config/decode.go @@ -29,6 +29,7 @@ type DecodeOpts struct { var DefaultDecodeHookFuncs = []mapstructure.DecodeHookFunc{ uint8ToStringHook, + stringToTrilean, mapstructure.StringToSliceHookFunc(","), mapstructure.StringToTimeDurationHookFunc(), } @@ -154,3 +155,30 @@ func uint8ToStringHook(f reflect.Kind, t reflect.Kind, v interface{}) (interface return v, nil } + +func stringToTrilean(f reflect.Type, t reflect.Type, v interface{}) (interface{}, error) { + // We have a custom data type, config, which we read from a string and + // then cast to a *bool. Why? So that we can appropriately read "unset" + // *bool values in order to intelligently default, even when the values are + // being set by a template variable. + + testTril, _ := TrileanFromString("") + if t == reflect.TypeOf(testTril) { + // From value is string + if f == reflect.TypeOf("") { + tril, err := TrileanFromString(v.(string)) + if err != nil { + return v, fmt.Errorf("Error parsing bool from given var: %s", err) + } + return tril, nil + } else { + // From value is boolean + if f == reflect.TypeOf(true) { + tril := TrileanFromBool(v.(bool)) + return tril, nil + } + } + + } + return v, nil +} diff --git a/helper/config/decode_test.go b/helper/config/decode_test.go index a04329f27..b08fb2af1 100644 --- a/helper/config/decode_test.go +++ b/helper/config/decode_test.go @@ -13,6 +13,7 @@ func TestDecode(t *testing.T) { Name string Address string Time time.Duration + Trilean Trilean } cases := map[string]struct { @@ -23,13 +24,27 @@ func TestDecode(t *testing.T) { "basic": { []interface{}{ map[string]interface{}{ - "name": "bar", - "time": "5s", + "name": "bar", + "time": "5s", + "trilean": "true", }, }, &Target{ - Name: "bar", - Time: 5 * time.Second, + Name: "bar", + Time: 5 * time.Second, + Trilean: TriTrue, + }, + nil, + }, + + "empty-string-trilean": { + []interface{}{ + map[string]interface{}{ + "trilean": "", + }, + }, + &Target{ + Trilean: TriUnset, }, nil, },