From 3c3f7f26cefef0f33509130b3db77e59ccafdd5a Mon Sep 17 00:00:00 2001 From: Megan Marsh Date: Mon, 19 Aug 2019 09:48:32 -0700 Subject: [PATCH] implement custom data type "trilean" (tri-state-boolean) to track booleans which have a "null" or "unset" state. Previously we used *bool for these template options, but it turns out that those won't work because "unset" will evaluate to "false" if a user is using template variables to set the option that maps to a *bool. --- builder/alicloud/ecs/builder_test.go | 10 +++- builder/alicloud/ecs/image_config.go | 30 +++++++--- builder/alicloud/ecs/run_config.go | 46 ++++++++------- builder/amazon/common/ami_config.go | 11 +++- builder/amazon/common/ami_config_test.go | 5 +- builder/amazon/common/block_device.go | 24 ++++---- builder/amazon/ebsvolume/builder.go | 10 +++- helper/config/custom_types.go | 72 ++++++++++++++++++++++++ helper/config/custom_types_test.go | 30 ++++++++++ helper/config/decode.go | 28 +++++++++ 10 files changed, 216 insertions(+), 50 deletions(-) create mode 100644 helper/config/custom_types.go create mode 100644 helper/config/custom_types_test.go diff --git a/builder/alicloud/ecs/builder_test.go b/builder/alicloud/ecs/builder_test.go index c6e54d793..72c3d480f 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,16 @@ 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) + RawEncrypted: helperconfig.TriUnset, + Encrypted: nil, + } + 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..f59894a14 100644 --- a/builder/alicloud/ecs/image_config.go +++ b/builder/alicloud/ecs/image_config.go @@ -5,18 +5,21 @@ 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"` + RawEncrypted config.Trilean `mapstructure:"disk_encrypted"` + + Encrypted *bool } type AlicloudDiskDevices struct { @@ -32,7 +35,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"` + RawImageEncrypted 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"` @@ -40,6 +43,8 @@ type AlicloudImageConfig struct { AlicloudImageSkipRegionValidation bool `mapstructure:"skip_region_validation"` AlicloudImageTags map[string]string `mapstructure:"tags"` AlicloudDiskDevices `mapstructure:",squash"` + + ImageEncrypted *bool } func (c *AlicloudImageConfig) Prepare(ctx *interpolate.Context) []error { @@ -75,6 +80,13 @@ func (c *AlicloudImageConfig) Prepare(ctx *interpolate.Context) []error { c.AlicloudImageDestinationRegions = regions } + c.ImageEncrypted = c.RawImageEncrypted.ToBoolPointer() + + c.ECSSystemDiskMapping.Encrypted = c.RawImageEncrypted.ToBoolPointer() + for i := range c.ECSImagesDiskMappings { + c.ECSImagesDiskMappings[i].Encrypted = c.RawImageEncrypted.ToBoolPointer() + } + if len(errs) > 0 { return errs } diff --git a/builder/alicloud/ecs/run_config.go b/builder/alicloud/ecs/run_config.go index ce4eda5b0..9feb3032d 100644 --- a/builder/alicloud/ecs/run_config.go +++ b/builder/alicloud/ecs/run_config.go @@ -8,35 +8,37 @@ 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"` + RawIOOptimized 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"` SSHPrivateIp bool `mapstructure:"ssh_private_ip"` + IOOptimized *bool } func (c *RunConfig) Prepare(ctx *interpolate.Context) []error { @@ -68,5 +70,7 @@ func (c *RunConfig) Prepare(ctx *interpolate.Context) []error { } } + c.IOOptimized = c.RawIOOptimized.ToBoolPointer() + return errs } diff --git a/builder/amazon/common/ami_config.go b/builder/amazon/common/ami_config.go index 2069ab23f..570a1b5f3 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,17 +20,21 @@ type AMIConfig struct { AMIRegions []string `mapstructure:"ami_regions"` AMISkipRegionValidation bool `mapstructure:"skip_region_validation"` AMITags TagMap `mapstructure:"tags"` - AMIENASupport *bool `mapstructure:"ena_support"` + RawAMIENASupport 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"` + RawAMIEncryptBootVolume 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"` SnapshotUsers []string `mapstructure:"snapshot_users"` SnapshotGroups []string `mapstructure:"snapshot_groups"` AMISkipBuildRegion bool `mapstructure:"skip_save_build_region"` + + // parsed from RawAMIENASupport above. Used in steps. + AMIENASupport *bool + AMIEncryptBootVolume *bool } func stringInSlice(s []string, searchstr string) bool { @@ -60,6 +65,8 @@ func (c *AMIConfig) Prepare(accessConfig *AccessConfig, ctx *interpolate.Context errs = append(errs, c.prepareRegions(accessConfig)...) + c.AMIENASupport = c.RawAMIENASupport.ToBoolPointer() + c.AMIEncryptBootVolume = c.RawAMIEncryptBootVolume.ToBoolPointer() // 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 { diff --git a/builder/amazon/common/ami_config_test.go b/builder/amazon/common/ami_config_test.go index 0e72d30dc..27fc91266 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.RawAMIEncryptBootVolume = 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.RawAMIEncryptBootVolume = config.TriTrue accessConf := testAccessConfig() diff --git a/builder/amazon/common/block_device.go b/builder/amazon/common/block_device.go index 88e066031..a3481f2bb 100644 --- a/builder/amazon/common/block_device.go +++ b/builder/amazon/common/block_device.go @@ -6,23 +6,26 @@ 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"` + RawEncrypted 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"` + + Encrypted *bool } type BlockDevices struct { @@ -93,6 +96,7 @@ 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.") } + b.Encrypted = b.RawEncrypted.ToBoolPointer() // Warn that encrypted must be true or nil when setting kms_key_id if b.KmsKeyId != "" && b.Encrypted != nil && *b.Encrypted == false { return fmt.Errorf("The device %v, must also have `encrypted: "+ diff --git a/builder/amazon/ebsvolume/builder.go b/builder/amazon/ebsvolume/builder.go index 7047c1734..f5ad9abb2 100644 --- a/builder/amazon/ebsvolume/builder.go +++ b/builder/amazon/ebsvolume/builder.go @@ -23,12 +23,14 @@ type Config struct { awscommon.AccessConfig `mapstructure:",squash"` awscommon.RunConfig `mapstructure:",squash"` - VolumeMappings []BlockDevice `mapstructure:"ebs_volumes"` - AMIENASupport *bool `mapstructure:"ena_support"` - AMISriovNetSupport bool `mapstructure:"sriov_support"` + VolumeMappings []BlockDevice `mapstructure:"ebs_volumes"` + RawAMIENASupport config.Trilean `mapstructure:"ena_support"` + AMISriovNetSupport bool `mapstructure:"sriov_support"` launchBlockDevices awscommon.BlockDevices ctx interpolate.Context + + AMIENASupport *bool } type Builder struct { @@ -75,6 +77,8 @@ func (b *Builder) Prepare(raws ...interface{}) ([]string, error) { errs = packer.MultiErrorAppend(errs, err) } + b.config.AMIENASupport = b.config.RawAMIENASupport.ToBoolPointer() + if b.config.IsSpotInstance() && ((b.config.AMIENASupport != nil && *b.config.AMIENASupport) || b.config.AMISriovNetSupport) { errs = packer.MultiErrorAppend(errs, fmt.Errorf("Spot instances do not support modification, which is required "+ 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 +}