From 485b828dde04a0e1be256b63a1aeefa58618d701 Mon Sep 17 00:00:00 2001 From: Julian Sommer <115516918+juliansommer@users.noreply.github.com> Date: Wed, 30 Sep 2026 10:28:04 +1000 Subject: [PATCH] fix: reject sg- group names, aws: tag keys and multi-attribute Modify*Attribute calls as AWS does --- spinifex/daemon/daemon_handlers_test.go | 4 +- spinifex/gateway/ec2/tags/CreateTags.go | 6 +++ spinifex/gateway/ec2/tags/CreateTags_test.go | 19 ++++++++ .../vpc/ModifyNetworkInterfaceAttribute.go | 9 +--- .../ModifyNetworkInterfaceAttribute_test.go | 13 ++++++ .../ec2/vpc/GetSecurityGroupsForVpc_test.go | 2 +- spinifex/handlers/ec2/vpc/eni.go | 46 +++++++++++++++---- spinifex/handlers/ec2/vpc/eni_test.go | 41 ++++++++++++----- spinifex/handlers/ec2/vpc/security_group.go | 4 ++ .../handlers/ec2/vpc/security_group_test.go | 40 ++++++++++++---- spinifex/handlers/ec2/vpc/service_impl.go | 15 +++++- .../handlers/ec2/vpc/service_impl_test.go | 19 ++++++++ 12 files changed, 179 insertions(+), 39 deletions(-) diff --git a/spinifex/daemon/daemon_handlers_test.go b/spinifex/daemon/daemon_handlers_test.go index cf4a963b5..28fdd1042 100644 --- a/spinifex/daemon/daemon_handlers_test.go +++ b/spinifex/daemon/daemon_handlers_test.go @@ -608,7 +608,7 @@ func runInstancesAndCheckENISGs(t *testing.T, mutator func(input *ec2.RunInstanc subnetID := *subnetOut.Subnet.SubnetId sg1Out, err := daemon.vpcService.CreateSecurityGroup(t.Context(), &ec2.CreateSecurityGroupInput{ - GroupName: aws.String("sg-prop-1"), + GroupName: aws.String("grp-prop-1"), Description: aws.String("test"), VpcId: aws.String(vpcID), }, testAccountID) @@ -616,7 +616,7 @@ func runInstancesAndCheckENISGs(t *testing.T, mutator func(input *ec2.RunInstanc sg1 = *sg1Out.GroupId sg2Out, err := daemon.vpcService.CreateSecurityGroup(t.Context(), &ec2.CreateSecurityGroupInput{ - GroupName: aws.String("sg-prop-2"), + GroupName: aws.String("grp-prop-2"), Description: aws.String("test"), VpcId: aws.String(vpcID), }, testAccountID) diff --git a/spinifex/gateway/ec2/tags/CreateTags.go b/spinifex/gateway/ec2/tags/CreateTags.go index 254f69da1..06b493fe1 100644 --- a/spinifex/gateway/ec2/tags/CreateTags.go +++ b/spinifex/gateway/ec2/tags/CreateTags.go @@ -3,6 +3,7 @@ package gateway_ec2_tags import ( "context" "errors" + "strings" "github.com/aws/aws-sdk-go/service/ec2" "github.com/mulgadc/spinifex/spinifex/awserrors" @@ -28,6 +29,11 @@ func ValidateCreateTagsInput(input *ec2.CreateTagsInput) error { if tag.Key == nil || *tag.Key == "" { return errors.New(awserrors.ErrorInvalidParameterValue) } + // AWS reserves the prefix in any case. + if strings.HasPrefix(strings.ToLower(*tag.Key), "aws:") { + return awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + "Value ( %s ) for parameter key is invalid. Tag keys starting with 'aws:' are reserved for internal use", *tag.Key) + } } return nil diff --git a/spinifex/gateway/ec2/tags/CreateTags_test.go b/spinifex/gateway/ec2/tags/CreateTags_test.go index bdea13e57..c257f65fa 100644 --- a/spinifex/gateway/ec2/tags/CreateTags_test.go +++ b/spinifex/gateway/ec2/tags/CreateTags_test.go @@ -7,6 +7,7 @@ import ( "github.com/aws/aws-sdk-go/service/ec2" "github.com/mulgadc/spinifex/spinifex/awserrors" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestValidateCreateTagsInput(t *testing.T) { @@ -138,3 +139,21 @@ func TestValidateDeleteTagsInput(t *testing.T) { }) } } + +func TestValidateCreateTagsInput_RejectsReservedAWSPrefix(t *testing.T) { + for _, key := range []string{"aws:foo", "AWS:Foo"} { + err := ValidateCreateTagsInput(&ec2.CreateTagsInput{ + Resources: []*string{aws.String("vpc-1234567890abcdef0")}, + Tags: []*ec2.Tag{{Key: aws.String(key), Value: aws.String("v")}}, + }) + code, msg, ok := awserrors.ResolveErrorDetail(err) + require.True(t, ok, "error %v carries no AWS code", err) + assert.Equal(t, awserrors.ErrorInvalidParameterValue, code) + assert.Equal(t, "Value ( "+key+" ) for parameter key is invalid. Tag keys starting with 'aws:' are reserved for internal use", msg) + } + + assert.NoError(t, ValidateCreateTagsInput(&ec2.CreateTagsInput{ + Resources: []*string{aws.String("vpc-1234567890abcdef0")}, + Tags: []*ec2.Tag{{Key: aws.String("team:aws:owner"), Value: aws.String("v")}}, + })) +} diff --git a/spinifex/gateway/ec2/vpc/ModifyNetworkInterfaceAttribute.go b/spinifex/gateway/ec2/vpc/ModifyNetworkInterfaceAttribute.go index 1ecbbfda4..772118b7c 100644 --- a/spinifex/gateway/ec2/vpc/ModifyNetworkInterfaceAttribute.go +++ b/spinifex/gateway/ec2/vpc/ModifyNetworkInterfaceAttribute.go @@ -17,14 +17,7 @@ func ValidateModifyNetworkInterfaceAttributeInput(input *ec2.ModifyNetworkInterf if input.NetworkInterfaceId == nil || *input.NetworkInterfaceId == "" { return errors.New(awserrors.ErrorMissingParameter) } - if len(input.Groups) == 0 && input.Description == nil && input.SourceDestCheck == nil { - return errors.New(awserrors.ErrorInvalidParameterValue) - } - // Disabling source/dest check is unsupported: OVN port security enforces it. - if input.SourceDestCheck != nil && input.SourceDestCheck.Value != nil && !*input.SourceDestCheck.Value { - return errors.New(awserrors.ErrorUnsupported) - } - return nil + return handlers_ec2_vpc.ValidateModifyNetworkInterfaceAttributeAttributes(input) } // ModifyNetworkInterfaceAttribute handles the EC2 ModifyNetworkInterfaceAttribute API call. diff --git a/spinifex/gateway/ec2/vpc/ModifyNetworkInterfaceAttribute_test.go b/spinifex/gateway/ec2/vpc/ModifyNetworkInterfaceAttribute_test.go index 0510ddcc7..603c2c82a 100644 --- a/spinifex/gateway/ec2/vpc/ModifyNetworkInterfaceAttribute_test.go +++ b/spinifex/gateway/ec2/vpc/ModifyNetworkInterfaceAttribute_test.go @@ -57,3 +57,16 @@ func TestModifyNetworkInterfaceAttribute_NilNATS(t *testing.T) { }, nil, "123456789012") assert.Error(t, err) } + +// AWS rejects the combination before judging any one attribute, so disabling +// source/dest check alongside a description is not reported as Unsupported. +func TestModifyNetworkInterfaceAttribute_CombinationBeforeUnsupported(t *testing.T) { + err := ValidateModifyNetworkInterfaceAttributeInput(&ec2.ModifyNetworkInterfaceAttributeInput{ + NetworkInterfaceId: aws.String("eni-abc123"), + SourceDestCheck: &ec2.AttributeBooleanValue{Value: aws.Bool(false)}, + Description: &ec2.AttributeValue{Value: aws.String("desc")}, + }) + code, ok := awserrors.ResolveErrorCode(err) + assert.True(t, ok, "error %v carries no AWS code", err) + assert.Equal(t, awserrors.ErrorInvalidParameterCombination, code) +} diff --git a/spinifex/handlers/ec2/vpc/GetSecurityGroupsForVpc_test.go b/spinifex/handlers/ec2/vpc/GetSecurityGroupsForVpc_test.go index 47b5388f4..3ceb95832 100644 --- a/spinifex/handlers/ec2/vpc/GetSecurityGroupsForVpc_test.go +++ b/spinifex/handlers/ec2/vpc/GetSecurityGroupsForVpc_test.go @@ -161,7 +161,7 @@ func TestGetSecurityGroupsForVpc_PagesEveryGroupOnce(t *testing.T) { vpcID := createTestVPC(t, svc, "10.0.0.0/16") expected := describedGroupIDsForVpc(t, svc, vpcID) - for _, name := range []string{"sg-a", "sg-b", "sg-c", "sg-d", "sg-e", "sg-f", "sg-g"} { + for _, name := range []string{"grp-a", "grp-b", "grp-c", "grp-d", "grp-e", "grp-f", "grp-g"} { expected = append(expected, createTestSG(t, svc, vpcID, name)) } diff --git a/spinifex/handlers/ec2/vpc/eni.go b/spinifex/handlers/ec2/vpc/eni.go index db51dca05..683645718 100644 --- a/spinifex/handlers/ec2/vpc/eni.go +++ b/spinifex/handlers/ec2/vpc/eni.go @@ -405,18 +405,48 @@ func (s *VPCServiceImpl) DetachAndDeleteENI(ctx context.Context, accountID, eniI return false, fmt.Errorf("DetachAndDeleteENI: exhausted %d CAS attempts for %s: %w", maxAttempts, eniID, lastErr) } -// ModifyNetworkInterfaceAttribute modifies ENI attributes (security groups, -// description). SourceDestCheck=true is accepted as a no-op. +// ValidateModifyNetworkInterfaceAttributeAttributes checks the attribute fields +// of a ModifyNetworkInterfaceAttribute request, which AWS does before it looks +// the interface up. The gateway and the handler share it so their order agrees. +func ValidateModifyNetworkInterfaceAttributeAttributes(input *ec2.ModifyNetworkInterfaceAttributeInput) error { + var set []string + if input.SourceDestCheck != nil { + set = append(set, "sourceDestCheck") + } + if input.Description != nil { + set = append(set, "description") + } + if len(input.Groups) > 0 { + set = append(set, "securityGroups") + } + if len(set) == 0 { + return errors.New(awserrors.ErrorInvalidParameterValue) + } + if len(set) > 1 { + return multipleAttributesError(set) + } + // Disabling source/dest check is unsupported: OVN port security enforces it. + if input.SourceDestCheck != nil && input.SourceDestCheck.Value != nil && !*input.SourceDestCheck.Value { + return errors.New(awserrors.ErrorUnsupported) + } + return nil +} + +// multipleAttributesError is AWS's answer to a Modify*Attribute request that +// sets more than one attribute. AWS's name order is not stable; this one is. +func multipleAttributesError(names []string) error { + return awserrors.Errorf(awserrors.ErrorInvalidParameterCombination, + "Fields for multiple attribute types specified: %s", strings.Join(names, ", ")) +} + +// ModifyNetworkInterfaceAttribute modifies one ENI attribute (security groups +// or description). SourceDestCheck=true is accepted as a no-op. func (s *VPCServiceImpl) ModifyNetworkInterfaceAttribute(ctx context.Context, input *ec2.ModifyNetworkInterfaceAttributeInput, accountID string) (*ec2.ModifyNetworkInterfaceAttributeOutput, error) { if input.NetworkInterfaceId == nil || *input.NetworkInterfaceId == "" { return nil, errors.New(awserrors.ErrorMissingParameter) } - if len(input.Groups) == 0 && input.Description == nil && input.SourceDestCheck == nil { - return nil, errors.New(awserrors.ErrorInvalidParameterValue) - } - // Disabling source/dest check is unsupported: OVN port security enforces it. - if input.SourceDestCheck != nil && input.SourceDestCheck.Value != nil && !*input.SourceDestCheck.Value { - return nil, errors.New(awserrors.ErrorUnsupported) + if err := ValidateModifyNetworkInterfaceAttributeAttributes(input); err != nil { + return nil, err } eniId := *input.NetworkInterfaceId diff --git a/spinifex/handlers/ec2/vpc/eni_test.go b/spinifex/handlers/ec2/vpc/eni_test.go index 1c1670fb6..e6508ea9f 100644 --- a/spinifex/handlers/ec2/vpc/eni_test.go +++ b/spinifex/handlers/ec2/vpc/eni_test.go @@ -830,8 +830,8 @@ func TestModifyNetworkInterfaceAttribute_SecurityGroups(t *testing.T) { vpcId := createTestVPC(t, svc, "10.0.0.0/16") subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24") eniId := createTestENI(t, svc, subnetId) - sg1 := createTestSG(t, svc, vpcId, "sg-one") - sg2 := createTestSG(t, svc, vpcId, "sg-two") + sg1 := createTestSG(t, svc, vpcId, "grp-one") + sg2 := createTestSG(t, svc, vpcId, "grp-two") _, err := svc.ModifyNetworkInterfaceAttribute(context.Background(), &ec2.ModifyNetworkInterfaceAttributeInput{ NetworkInterfaceId: aws.String(eniId), @@ -855,8 +855,8 @@ func TestModifyNetworkInterfaceAttribute_PublishesUpdatePortSGs(t *testing.T) { vpcId := createTestVPC(t, svc, "10.0.0.0/16") subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24") eniId := createTestENI(t, svc, subnetId) - sg1 := createTestSG(t, svc, vpcId, "sg-mod-1") - sg2 := createTestSG(t, svc, vpcId, "sg-mod-2") + sg1 := createTestSG(t, svc, vpcId, "grp-mod-1") + sg2 := createTestSG(t, svc, vpcId, "grp-mod-2") eventCh := make(chan *nats.Msg, 1) sub, err := nc.Subscribe("vpc.update-port-sgs", func(msg *nats.Msg) { @@ -913,7 +913,7 @@ func TestCreateNetworkInterface_PublishesEventCarriesSGs(t *testing.T) { svc, nc := setupTestVPCServiceWithNC(t) vpcId := createTestVPC(t, svc, "10.0.0.0/16") subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24") - sgA := createTestSG(t, svc, vpcId, "sg-evt-A") + sgA := createTestSG(t, svc, vpcId, "grp-evt-A") eventCh := make(chan *nats.Msg, 1) sub, err := nc.Subscribe("vpc.create-port", func(msg *nats.Msg) { @@ -1062,8 +1062,8 @@ func TestCreateNetworkInterface_WithSecurityGroups(t *testing.T) { svc := setupTestVPCService(t) vpcId := createTestVPC(t, svc, "10.0.0.0/16") subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24") - sgA := createTestSG(t, svc, vpcId, "sg-aaa") - sgB := createTestSG(t, svc, vpcId, "sg-bbb") + sgA := createTestSG(t, svc, vpcId, "grp-aaa") + sgB := createTestSG(t, svc, vpcId, "grp-bbb") out, err := svc.CreateNetworkInterface(context.Background(), &ec2.CreateNetworkInterfaceInput{ SubnetId: aws.String(subnetId), @@ -1222,8 +1222,8 @@ func TestDescribeNetworkInterfaces_FilterByGroupId(t *testing.T) { svc := setupTestVPCService(t) vpcId := createTestVPC(t, svc, "10.0.0.0/16") subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24") - sgA := createTestSG(t, svc, vpcId, "sg-aaa") - sgB := createTestSG(t, svc, vpcId, "sg-bbb") + sgA := createTestSG(t, svc, vpcId, "grp-aaa") + sgB := createTestSG(t, svc, vpcId, "grp-bbb") // Create ENI with security groups out, err := svc.CreateNetworkInterface(context.Background(), &ec2.CreateNetworkInterfaceInput{ @@ -1465,7 +1465,7 @@ func TestValidateSGAttachment_TooMany(t *testing.T) { vpcID := createTestVPC(t, svc, "10.0.0.0/16") sgs := make([]string, 6) for i := range sgs { - sgs[i] = createTestSG(t, svc, vpcID, fmt.Sprintf("sg-%d", i)) + sgs[i] = createTestSG(t, svc, vpcID, fmt.Sprintf("grp-%d", i)) } err := svc.validateSGAttachment(t.Context(), testAccountID, sgs, vpcID) @@ -1508,7 +1508,7 @@ func TestModifyNetworkInterfaceAttribute_VpcdError_Propagated(t *testing.T) { vpcId := createTestVPC(t, svc, "10.0.0.0/16") subnetId := createTestSubnet(t, svc, vpcId, "10.0.1.0/24") eniId := createTestENI(t, svc, subnetId) - sg1 := createTestSG(t, svc, vpcId, "sg-mod-fail-1") + sg1 := createTestSG(t, svc, vpcId, "grp-mod-fail-1") // Swap the stub's vpc.update-port-sgs reply to an error in-place so // there's exactly one responder (no race with a layered subscriber). @@ -1552,3 +1552,22 @@ func TestUpdateENIPublicIP_NotFound(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "eni-missing") } + +// AWS checks the attribute combination before it looks the interface up, and +// its order of names is unstable, so only the names are asserted. +func TestModifyNetworkInterfaceAttribute_MultipleAttributes(t *testing.T) { + t.Parallel() + svc := setupTestVPCService(t) + + _, err := svc.ModifyNetworkInterfaceAttribute(context.Background(), &ec2.ModifyNetworkInterfaceAttributeInput{ + NetworkInterfaceId: aws.String("eni-nonexistent"), + Groups: []*string{aws.String("sg-111")}, + Description: &ec2.AttributeValue{Value: aws.String("desc")}, + }, testAccountID) + code, msg, ok := awserrors.ResolveErrorDetail(err) + require.True(t, ok, "error %v carries no AWS code", err) + assert.Equal(t, awserrors.ErrorInvalidParameterCombination, code) + assert.Contains(t, msg, "Fields for multiple attribute types specified: ") + assert.Contains(t, msg, "description") + assert.Contains(t, msg, "securityGroups") +} diff --git a/spinifex/handlers/ec2/vpc/security_group.go b/spinifex/handlers/ec2/vpc/security_group.go index d9d9782d8..61e42e876 100644 --- a/spinifex/handlers/ec2/vpc/security_group.go +++ b/spinifex/handlers/ec2/vpc/security_group.go @@ -193,6 +193,10 @@ func (s *VPCServiceImpl) CreateSecurityGroup(ctx context.Context, input *ec2.Cre if err := s.requireVPCExists(ctx, accountID, vpcId); err != nil { return nil, err } + if strings.HasPrefix(groupName, "sg-") { + return nil, awserrors.Errorf(awserrors.ErrorInvalidParameterValue, + "Value (%s) for parameter GroupName is invalid. Group names may not be in the format sg-*.", groupName) + } // Check for duplicate group name in the same VPC and enforce the per-VPC // SG quota in the same bucket walk. diff --git a/spinifex/handlers/ec2/vpc/security_group_test.go b/spinifex/handlers/ec2/vpc/security_group_test.go index 53e056845..793c240dc 100644 --- a/spinifex/handlers/ec2/vpc/security_group_test.go +++ b/spinifex/handlers/ec2/vpc/security_group_test.go @@ -131,8 +131,8 @@ func TestDescribeSecurityGroups_All(t *testing.T) { t.Parallel() svc := setupTestVPCService(t) vpcID := createTestVPC(t, svc, "10.0.0.0/16") - createTestSG(t, svc, vpcID, "sg-a") - createTestSG(t, svc, vpcID, "sg-b") + createTestSG(t, svc, vpcID, "grp-a") + createTestSG(t, svc, vpcID, "grp-b") desc, err := svc.DescribeSecurityGroups(context.Background(), &ec2.DescribeSecurityGroupsInput{}, testAccountID) require.NoError(t, err) @@ -1301,8 +1301,8 @@ func TestDescribeSecurityGroups_FilterByVpcId(t *testing.T) { svc := setupTestVPCService(t) vpc1 := createTestVPC(t, svc, "10.0.0.0/16") vpc2 := createTestVPC(t, svc, "172.16.0.0/16") - createTestSG(t, svc, vpc1, "sg-in-vpc1") - createTestSG(t, svc, vpc2, "sg-in-vpc2") + createTestSG(t, svc, vpc1, "grp-in-vpc1") + createTestSG(t, svc, vpc2, "grp-in-vpc2") out, err := svc.DescribeSecurityGroups(context.Background(), &ec2.DescribeSecurityGroupsInput{ Filters: []*ec2.Filter{ @@ -1354,13 +1354,13 @@ func TestDescribeSecurityGroups_FilterMultipleValues_OR(t *testing.T) { t.Parallel() svc := setupTestVPCService(t) vpcID := createTestVPC(t, svc, "10.0.0.0/16") - createTestSG(t, svc, vpcID, "sg-alpha") - createTestSG(t, svc, vpcID, "sg-beta") - createTestSG(t, svc, vpcID, "sg-gamma") + createTestSG(t, svc, vpcID, "grp-alpha") + createTestSG(t, svc, vpcID, "grp-beta") + createTestSG(t, svc, vpcID, "grp-gamma") out, err := svc.DescribeSecurityGroups(context.Background(), &ec2.DescribeSecurityGroupsInput{ Filters: []*ec2.Filter{ - {Name: aws.String("group-name"), Values: []*string{aws.String("sg-alpha"), aws.String("sg-gamma")}}, + {Name: aws.String("group-name"), Values: []*string{aws.String("grp-alpha"), aws.String("grp-gamma")}}, }, }, testAccountID) require.NoError(t, err) @@ -1554,6 +1554,30 @@ func TestCreateSecurityGroup_RejectsReservedDefaultName(t *testing.T) { assert.Contains(t, err.Error(), "InvalidGroup.Reserved") } +func TestCreateSecurityGroup_RejectsSgPrefixedName(t *testing.T) { + t.Parallel() + svc := setupTestVPCService(t) + vpcID := createTestVPC(t, svc, "10.0.0.0/16") + + _, err := svc.CreateSecurityGroup(context.Background(), &ec2.CreateSecurityGroupInput{ + GroupName: aws.String("sg-foo"), + Description: aws.String("d"), + VpcId: aws.String(vpcID), + }, testAccountID) + code, msg, ok := awserrors.ResolveErrorDetail(err) + require.True(t, ok, "error %v carries no AWS code", err) + assert.Equal(t, awserrors.ErrorInvalidParameterValue, code) + assert.Equal(t, "Value (sg-foo) for parameter GroupName is invalid. Group names may not be in the format sg-*.", msg) + + // AWS reports a missing VPC ahead of the name. + _, err = svc.CreateSecurityGroup(context.Background(), &ec2.CreateSecurityGroupInput{ + GroupName: aws.String("sg-foo"), + Description: aws.String("d"), + VpcId: aws.String("vpc-nonexistent"), + }, testAccountID) + assert.ErrorContains(t, err, awserrors.ErrorInvalidVpcIDNotFound) +} + func TestDeleteSecurityGroup_RejectsDefault(t *testing.T) { t.Parallel() svc := setupTestVPCService(t) diff --git a/spinifex/handlers/ec2/vpc/service_impl.go b/spinifex/handlers/ec2/vpc/service_impl.go index 68d950ad4..f0f881799 100644 --- a/spinifex/handlers/ec2/vpc/service_impl.go +++ b/spinifex/handlers/ec2/vpc/service_impl.go @@ -1363,9 +1363,22 @@ func (s *VPCServiceImpl) ModifyVpcAttribute(ctx context.Context, input *ec2.Modi if input.VpcId == nil || *input.VpcId == "" { return nil, errors.New(awserrors.ErrorMissingParameter) } - if input.EnableDnsHostnames == nil && input.EnableDnsSupport == nil && input.EnableNetworkAddressUsageMetrics == nil { + var set []string + if input.EnableDnsSupport != nil { + set = append(set, "enableDnsSupport") + } + if input.EnableDnsHostnames != nil { + set = append(set, "enableDnsHostnames") + } + if input.EnableNetworkAddressUsageMetrics != nil { + set = append(set, "enableNetworkAddressUsageMetrics") + } + if len(set) == 0 { return nil, errors.New(awserrors.ErrorInvalidParameterValue) } + if len(set) > 1 { + return nil, multipleAttributesError(set) + } vpcID := *input.VpcId key := utils.AccountKey(accountID, vpcID) diff --git a/spinifex/handlers/ec2/vpc/service_impl_test.go b/spinifex/handlers/ec2/vpc/service_impl_test.go index 24fbebc4d..e56a777ef 100644 --- a/spinifex/handlers/ec2/vpc/service_impl_test.go +++ b/spinifex/handlers/ec2/vpc/service_impl_test.go @@ -2485,3 +2485,22 @@ func findTag(tags []*ec2.Tag, key string) string { } return "" } + +// AWS checks the attribute combination before it looks the VPC up, and its +// order of names is unstable, so only the names are asserted. +func TestVpc_ModifyVpcAttribute_MultipleAttributes(t *testing.T) { + t.Parallel() + svc := setupTestVPCService(t) + + _, err := svc.ModifyVpcAttribute(context.Background(), &ec2.ModifyVpcAttributeInput{ + VpcId: aws.String("vpc-nonexistent"), + EnableDnsSupport: &ec2.AttributeBooleanValue{Value: aws.Bool(true)}, + EnableDnsHostnames: &ec2.AttributeBooleanValue{Value: aws.Bool(true)}, + }, testAccountID) + code, msg, ok := awserrors.ResolveErrorDetail(err) + require.True(t, ok, "error %v carries no AWS code", err) + assert.Equal(t, awserrors.ErrorInvalidParameterCombination, code) + assert.Contains(t, msg, "Fields for multiple attribute types specified: ") + assert.Contains(t, msg, "enableDnsSupport") + assert.Contains(t, msg, "enableDnsHostnames") +}