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 570a1b5f3..716697c73 100644 --- a/builder/amazon/common/ami_config.go +++ b/builder/amazon/common/ami_config.go @@ -20,7 +20,7 @@ type AMIConfig struct { AMIRegions []string `mapstructure:"ami_regions"` AMISkipRegionValidation bool `mapstructure:"skip_region_validation"` AMITags TagMap `mapstructure:"tags"` - RawAMIENASupport config.Trilean `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"` @@ -32,8 +32,6 @@ type AMIConfig struct { SnapshotGroups []string `mapstructure:"snapshot_groups"` AMISkipBuildRegion bool `mapstructure:"skip_save_build_region"` - // parsed from RawAMIENASupport above. Used in steps. - AMIENASupport *bool AMIEncryptBootVolume *bool } @@ -65,7 +63,6 @@ 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 { 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/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 f5ad9abb2..9fcbb7ac7 100644 --- a/builder/amazon/ebsvolume/builder.go +++ b/builder/amazon/ebsvolume/builder.go @@ -24,13 +24,11 @@ type Config struct { awscommon.RunConfig `mapstructure:",squash"` VolumeMappings []BlockDevice `mapstructure:"ebs_volumes"` - RawAMIENASupport config.Trilean `mapstructure:"ena_support"` + AMIENASupport config.Trilean `mapstructure:"ena_support"` AMISriovNetSupport bool `mapstructure:"sriov_support"` launchBlockDevices awscommon.BlockDevices ctx interpolate.Context - - AMIENASupport *bool } type Builder struct { @@ -77,9 +75,7 @@ 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) { + 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)