diff --git a/spinifex/filterutil/filterutil.go b/spinifex/filterutil/filterutil.go index 52fbf4cdc..515659e97 100644 --- a/spinifex/filterutil/filterutil.go +++ b/spinifex/filterutil/filterutil.go @@ -54,10 +54,29 @@ func MatchesAny(filterValues []string, value string) bool { return false } -// MatchesTags checks whether a resource's tags satisfy all tag:Key filters in the map. -// Each tag:Key filter uses OR logic across its values, with wildcard support. +// IsTagFilter reports whether MatchesTags evaluates the named filter, so a +// resource matcher's switch can skip it. +func IsTagFilter(name string) bool { + return strings.HasPrefix(name, "tag:") || name == "tag-key" || name == "tag-value" +} + +// MatchesTags checks whether a resource's tags satisfy every tag:Key, tag-key +// and tag-value filter in the map. Each filter uses OR logic across its values, +// with wildcard support; tag-key and tag-value match any one of the tags. func MatchesTags(filters map[string][]string, tags map[string]string) bool { for name, values := range filters { + switch name { + case "tag-key": + if !matchesAnyTag(tags, values, func(k, _ string) string { return k }) { + return false + } + continue + case "tag-value": + if !matchesAnyTag(tags, values, func(_, v string) string { return v }) { + return false + } + continue + } tagKey, ok := strings.CutPrefix(name, "tag:") if !ok { continue @@ -73,6 +92,17 @@ func MatchesTags(filters map[string][]string, tags map[string]string) bool { return true } +// matchesAnyTag reports whether any tag's selected field (key or value) +// matches any of the filter values. +func matchesAnyTag(tags map[string]string, values []string, field func(k, v string) string) bool { + for k, v := range tags { + if MatchesAny(values, field(k, v)) { + return true + } + } + return false +} + // EC2TagsToMap converts []*ec2.Tag to map[string]string for MatchesTags. func EC2TagsToMap(tags []*ec2.Tag) map[string]string { if len(tags) == 0 { diff --git a/spinifex/filterutil/filterutil_test.go b/spinifex/filterutil/filterutil_test.go index 90c389b24..46f57c24e 100644 --- a/spinifex/filterutil/filterutil_test.go +++ b/spinifex/filterutil/filterutil_test.go @@ -262,6 +262,36 @@ func TestMatchesTags_EmptyFilters(t *testing.T) { } } +func TestMatchesTags_TagKeyAndTagValue(t *testing.T) { + tags := map[string]string{"Name": "web", "awsdiff": "yes"} + cases := []struct { + name string + filters map[string][]string + want bool + }{ + {"tag-key present", map[string][]string{"tag-key": {"awsdiff"}}, true}, + {"tag-key absent", map[string][]string{"tag-key": {"zz-awsdiff-none"}}, false}, + {"tag-key any of values", map[string][]string{"tag-key": {"missing", "Name"}}, true}, + {"tag-key wildcard", map[string][]string{"tag-key": {"aws*"}}, true}, + {"tag-key does not match a value", map[string][]string{"tag-key": {"web"}}, false}, + {"tag-value on any tag", map[string][]string{"tag-value": {"yes"}}, true}, + {"tag-value absent", map[string][]string{"tag-value": {"zz-awsdiff-none"}}, false}, + {"tag-value does not match a key", map[string][]string{"tag-value": {"Name"}}, false}, + {"tag-key and tag-value from different tags", map[string][]string{"tag-key": {"Name"}, "tag-value": {"yes"}}, true}, + {"tag-key ANDed with tag:", map[string][]string{"tag-key": {"awsdiff"}, "tag:Name": {"db"}}, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := MatchesTags(tc.filters, tags); got != tc.want { + t.Fatalf("MatchesTags(%v) = %v, want %v", tc.filters, got, tc.want) + } + }) + } + if MatchesTags(map[string][]string{"tag-key": {"*"}}, nil) { + t.Fatal("expected an untagged resource to fail tag-key") + } +} + func TestEC2TagsToMap(t *testing.T) { tags := []*ec2.Tag{ {Key: aws.String("Env"), Value: aws.String("prod")}, diff --git a/spinifex/handlers/ec2/eigw/service_impl.go b/spinifex/handlers/ec2/eigw/service_impl.go index acc77b1a8..54d3665b3 100644 --- a/spinifex/handlers/ec2/eigw/service_impl.go +++ b/spinifex/handlers/ec2/eigw/service_impl.go @@ -147,6 +147,8 @@ func (s *EgressOnlyIGWServiceImpl) DeleteEgressOnlyInternetGateway(ctx context.C // describeEIGWValidFilters defines the set of filter names accepted by DescribeEgressOnlyInternetGateways. var describeEIGWValidFilters = map[string]bool{ "egress-only-internet-gateway-id": true, + "tag-key": true, + "tag-value": true, } // DescribeEgressOnlyInternetGateways describes Egress-only Internet Gateways. @@ -213,7 +215,7 @@ func (s *EgressOnlyIGWServiceImpl) DescribeEgressOnlyInternetGateways(ctx contex // eigwMatchesFilters checks whether an EgressOnlyIGWRecord satisfies all parsed filters. func eigwMatchesFilters(record *EgressOnlyIGWRecord, filters map[string][]string) bool { for name, values := range filters { - if strings.HasPrefix(name, "tag:") { + if filterutil.IsTagFilter(name) { continue } diff --git a/spinifex/handlers/ec2/eigw/service_impl_test.go b/spinifex/handlers/ec2/eigw/service_impl_test.go index def1cac6b..2249ad916 100644 --- a/spinifex/handlers/ec2/eigw/service_impl_test.go +++ b/spinifex/handlers/ec2/eigw/service_impl_test.go @@ -270,6 +270,36 @@ func TestDescribeEgressOnlyInternetGateways_FilterByTag(t *testing.T) { assert.Equal(t, *out.EgressOnlyInternetGateway.EgressOnlyInternetGatewayId, *desc.EgressOnlyInternetGateways[0].EgressOnlyInternetGatewayId) } +func TestDescribeEgressOnlyInternetGateways_FilterByTagKeyAndTagValue(t *testing.T) { + svc := setupTestEIGWService(t) + out, err := svc.CreateEgressOnlyInternetGateway(context.Background(), &ec2.CreateEgressOnlyInternetGatewayInput{ + VpcId: aws.String("vpc-tagged"), + TagSpecifications: []*ec2.TagSpecification{ + { + ResourceType: aws.String("egress-only-internet-gateway"), + Tags: []*ec2.Tag{{Key: aws.String("Env"), Value: aws.String("prod")}}, + }, + }, + }, testAccountID) + require.NoError(t, err) + createTestEIGW(t, svc) // untagged + + for _, tc := range []struct{ name, value string }{{"tag-key", "Env"}, {"tag-value", "prod"}} { + desc, err := svc.DescribeEgressOnlyInternetGateways(context.Background(), &ec2.DescribeEgressOnlyInternetGatewaysInput{ + Filters: []*ec2.Filter{{Name: aws.String(tc.name), Values: []*string{aws.String(tc.value)}}}, + }, testAccountID) + require.NoError(t, err, tc.name) + require.Len(t, desc.EgressOnlyInternetGateways, 1, tc.name) + assert.Equal(t, *out.EgressOnlyInternetGateway.EgressOnlyInternetGatewayId, *desc.EgressOnlyInternetGateways[0].EgressOnlyInternetGatewayId, tc.name) + + desc, err = svc.DescribeEgressOnlyInternetGateways(context.Background(), &ec2.DescribeEgressOnlyInternetGatewaysInput{ + Filters: []*ec2.Filter{{Name: aws.String(tc.name), Values: []*string{aws.String("zz-awsdiff-none")}}}, + }, testAccountID) + require.NoError(t, err, tc.name) + assert.Empty(t, desc.EgressOnlyInternetGateways, tc.name) + } +} + // TestCreateEgressOnlyInternetGateway_CrossAccountVPCRejected tests that creating an EIGW in another account's VPC is rejected. func TestCreateEgressOnlyInternetGateway_CrossAccountVPCRejected(t *testing.T) { // Set up with manual NATS to get VPC KV access diff --git a/spinifex/handlers/ec2/igw/service_impl.go b/spinifex/handlers/ec2/igw/service_impl.go index 0957c8379..4004c8947 100644 --- a/spinifex/handlers/ec2/igw/service_impl.go +++ b/spinifex/handlers/ec2/igw/service_impl.go @@ -187,6 +187,8 @@ var describeIGWValidFilters = map[string]bool{ "internet-gateway-id": true, "attachment.vpc-id": true, "attachment.state": true, + "tag-key": true, + "tag-value": true, } // DescribeInternetGateways lists Internet Gateways, optionally filtered by ID. @@ -263,7 +265,7 @@ func (s *IGWServiceImpl) DescribeInternetGateways(ctx context.Context, input *ec // igwMatchesFilters checks whether an IGWRecord satisfies all parsed filters. func igwMatchesFilters(record *IGWRecord, filters map[string][]string) bool { for name, values := range filters { - if strings.HasPrefix(name, "tag:") { + if filterutil.IsTagFilter(name) { continue } diff --git a/spinifex/handlers/ec2/igw/service_impl_test.go b/spinifex/handlers/ec2/igw/service_impl_test.go index 059dba9ed..19bae8d5e 100644 --- a/spinifex/handlers/ec2/igw/service_impl_test.go +++ b/spinifex/handlers/ec2/igw/service_impl_test.go @@ -948,6 +948,35 @@ func TestDescribeInternetGateways_FilterByTag(t *testing.T) { assert.Equal(t, *out.InternetGateway.InternetGatewayId, *desc.InternetGateways[0].InternetGatewayId) } +func TestDescribeInternetGateways_FilterByTagKeyAndTagValue(t *testing.T) { + svc, _ := setupTestIGWService(t) + out, err := svc.CreateInternetGateway(context.Background(), &ec2.CreateInternetGatewayInput{ + TagSpecifications: []*ec2.TagSpecification{ + { + ResourceType: aws.String("internet-gateway"), + Tags: []*ec2.Tag{{Key: aws.String("Env"), Value: aws.String("prod")}}, + }, + }, + }, testAccountID) + require.NoError(t, err) + createTestIGW(t, svc) // untagged + + for _, tc := range []struct{ name, value string }{{"tag-key", "Env"}, {"tag-value", "prod"}} { + desc, err := svc.DescribeInternetGateways(context.Background(), &ec2.DescribeInternetGatewaysInput{ + Filters: []*ec2.Filter{{Name: aws.String(tc.name), Values: []*string{aws.String(tc.value)}}}, + }, testAccountID) + require.NoError(t, err, tc.name) + require.Len(t, desc.InternetGateways, 1, tc.name) + assert.Equal(t, *out.InternetGateway.InternetGatewayId, *desc.InternetGateways[0].InternetGatewayId, tc.name) + + desc, err = svc.DescribeInternetGateways(context.Background(), &ec2.DescribeInternetGatewaysInput{ + Filters: []*ec2.Filter{{Name: aws.String(tc.name), Values: []*string{aws.String("zz-awsdiff-none")}}}, + }, testAccountID) + require.NoError(t, err, tc.name) + assert.Empty(t, desc.InternetGateways, tc.name) + } +} + func TestDeleteInternetGateway_PublishesNoEvent(t *testing.T) { svc, nc := setupTestIGWService(t) igwID := createTestIGW(t, svc) diff --git a/spinifex/handlers/ec2/placementgroup/service_impl.go b/spinifex/handlers/ec2/placementgroup/service_impl.go index aeeba5955..734503001 100644 --- a/spinifex/handlers/ec2/placementgroup/service_impl.go +++ b/spinifex/handlers/ec2/placementgroup/service_impl.go @@ -289,7 +289,7 @@ func (s *PlacementGroupServiceImpl) DescribePlacementGroups(ctx context.Context, // pgMatchesFilters checks whether a placement group record matches all parsed filters. func pgMatchesFilters(record *PlacementGroupRecord, filters map[string][]string) bool { for name, values := range filters { - if strings.HasPrefix(name, "tag:") { + if filterutil.IsTagFilter(name) { continue } switch name { @@ -313,14 +313,6 @@ func pgMatchesFilters(record *PlacementGroupRecord, filters map[string][]string) if !filterutil.MatchesAny(values, record.GroupName) { return false } - case "tag-key": - if !pgMatchesAnyTag(record.Tags, values, func(k, _ string) string { return k }) { - return false - } - case "tag-value": - if !pgMatchesAnyTag(record.Tags, values, func(_, v string) string { return v }) { - return false - } default: return false } @@ -328,17 +320,6 @@ func pgMatchesFilters(record *PlacementGroupRecord, filters map[string][]string) return filterutil.MatchesTags(filters, record.Tags) } -// pgMatchesAnyTag reports whether any tag's selected field (key or value) -// matches any of the filter values. -func pgMatchesAnyTag(tags map[string]string, values []string, field func(k, v string) string) bool { - for k, v := range tags { - if filterutil.MatchesAny(values, field(k, v)) { - return true - } - } - return false -} - const maxCASRetries = 5 // errPGAbsent and errPGContended mark the two kvutil outcomes that map to diff --git a/spinifex/handlers/ec2/routetable/service_impl.go b/spinifex/handlers/ec2/routetable/service_impl.go index 26a9da63e..c2a6b32de 100644 --- a/spinifex/handlers/ec2/routetable/service_impl.go +++ b/spinifex/handlers/ec2/routetable/service_impl.go @@ -599,6 +599,8 @@ var describeRouteTablesValidFilters = map[string]bool{ "route.state": true, "route.origin": true, "owner-id": true, + "tag-key": true, + "tag-value": true, } // DescribeRouteTables lists route tables, optionally filtered. @@ -1195,7 +1197,7 @@ func (s *RouteTableServiceImpl) ReplaceRouteTableAssociation(ctx context.Context // rtbMatchesFilters checks if a route table record matches all parsed filters. func rtbMatchesFilters(record *RouteTableRecord, filters map[string][]string) bool { for name, values := range filters { - if strings.HasPrefix(name, "tag:") { + if filterutil.IsTagFilter(name) { continue } switch name { diff --git a/spinifex/handlers/ec2/routetable/service_impl_test.go b/spinifex/handlers/ec2/routetable/service_impl_test.go index e9702ec78..4fa0ac18d 100644 --- a/spinifex/handlers/ec2/routetable/service_impl_test.go +++ b/spinifex/handlers/ec2/routetable/service_impl_test.go @@ -155,6 +155,35 @@ func TestCreateRouteTable_PersistsTagsForTagFilterDiscovery(t *testing.T) { assert.Equal(t, "cp-private-rt", tags["spinifex:eks-role"]) } +func TestDescribeRouteTables_FilterByTagKeyAndTagValue(t *testing.T) { + t.Parallel() + svc := setupTestService(t) + out, err := svc.CreateRouteTable(t.Context(), &ec2.CreateRouteTableInput{ + VpcId: aws.String("vpc-test1"), + TagSpecifications: []*ec2.TagSpecification{{ + ResourceType: aws.String(ec2.ResourceTypeRouteTable), + Tags: []*ec2.Tag{{Key: aws.String("Env"), Value: aws.String("prod")}}, + }}, + }, testAccountID) + require.NoError(t, err) + createTestRtb(t, svc) // untagged + + for _, tc := range []struct{ name, value string }{{"tag-key", "Env"}, {"tag-value", "prod"}} { + desc, err := svc.DescribeRouteTables(t.Context(), &ec2.DescribeRouteTablesInput{ + Filters: []*ec2.Filter{{Name: aws.String(tc.name), Values: aws.StringSlice([]string{tc.value})}}, + }, testAccountID) + require.NoError(t, err, tc.name) + require.Len(t, desc.RouteTables, 1, tc.name) + assert.Equal(t, *out.RouteTable.RouteTableId, *desc.RouteTables[0].RouteTableId, tc.name) + + desc, err = svc.DescribeRouteTables(t.Context(), &ec2.DescribeRouteTablesInput{ + Filters: []*ec2.Filter{{Name: aws.String(tc.name), Values: aws.StringSlice([]string{"zz-awsdiff-none"})}}, + }, testAccountID) + require.NoError(t, err, tc.name) + assert.Empty(t, desc.RouteTables, tc.name) + } +} + func TestCreateRouteTable_VpcNotFound(t *testing.T) { t.Parallel() svc := setupTestService(t) diff --git a/spinifex/handlers/ec2/tags/service_impl.go b/spinifex/handlers/ec2/tags/service_impl.go index d00fe4974..fac37be80 100644 --- a/spinifex/handlers/ec2/tags/service_impl.go +++ b/spinifex/handlers/ec2/tags/service_impl.go @@ -289,6 +289,8 @@ var describeTagsValidFilters = map[string]bool{ "resource-type": true, "key": true, "value": true, + "tag-key": true, + "tag-value": true, } // DescribeTags returns tags matching the specified filters. @@ -337,6 +339,11 @@ func (s *TagsServiceImpl) DescribeTags(ctx context.Context, input *ec2.DescribeT if !filterutil.MatchesAny(filters["value"], value) { continue } + // On DescribeTags, AWS applies tag-key and tag-value to each tag row, + // exactly as it applies key and value. + if !filterutil.MatchesAny(filters["tag-key"], key) || !filterutil.MatchesAny(filters["tag-value"], value) { + continue + } tags = append(tags, &ec2.TagDescription{ ResourceId: aws.String(resourceID), diff --git a/spinifex/handlers/ec2/tags/service_impl_test.go b/spinifex/handlers/ec2/tags/service_impl_test.go index 90d757c96..a17e4a738 100644 --- a/spinifex/handlers/ec2/tags/service_impl_test.go +++ b/spinifex/handlers/ec2/tags/service_impl_test.go @@ -287,6 +287,41 @@ func TestDescribeTags_FilterByValue(t *testing.T) { assert.Equal(t, "production", *result.Tags[0].Value) } +// TestDescribeTags_FilterByTagKeyAndTagValue checks that tag-key and tag-value +// select tag rows, as key and value do, rather than whole resources. +func TestDescribeTags_FilterByTagKeyAndTagValue(t *testing.T) { + svc, _ := setupTestTagsService(t) + + _, err := svc.CreateTags(context.Background(), &ec2.CreateTagsInput{ + Resources: []*string{aws.String("vpc-test1")}, + Tags: []*ec2.Tag{ + {Key: aws.String("Name"), Value: aws.String("web")}, + {Key: aws.String("awsdiff"), Value: aws.String("yes")}, + }, + }, testAccountID) + require.NoError(t, err) + + describe := func(name, value string) []*ec2.TagDescription { + t.Helper() + result, err := svc.DescribeTags(context.Background(), &ec2.DescribeTagsInput{ + Filters: []*ec2.Filter{{Name: aws.String(name), Values: []*string{aws.String(value)}}}, + }, testAccountID) + require.NoError(t, err) + return result.Tags + } + + byKey := describe("tag-key", "awsdiff") + require.Len(t, byKey, 1) + assert.Equal(t, "awsdiff", *byKey[0].Key) + + byValue := describe("tag-value", "web") + require.Len(t, byValue, 1) + assert.Equal(t, "Name", *byValue[0].Key) + + assert.Empty(t, describe("tag-key", "zz-awsdiff-none")) + assert.Empty(t, describe("tag-value", "zz-awsdiff-none")) +} + // TestDescribeTags_Empty tests listing tags when none exist. func TestDescribeTags_Empty(t *testing.T) { svc, _ := setupTestTagsService(t) diff --git a/spinifex/handlers/ec2/vpc/eni.go b/spinifex/handlers/ec2/vpc/eni.go index bb62643ba..6f2e222c7 100644 --- a/spinifex/handlers/ec2/vpc/eni.go +++ b/spinifex/handlers/ec2/vpc/eni.go @@ -486,6 +486,8 @@ var describeNetworkInterfacesValidFilters = map[string]bool{ "attachment.attachment-id": true, "attachment.instance-id": true, "attachment.status": true, + "tag-key": true, + "tag-value": true, } // DescribeNetworkInterfaces lists ENIs with optional filters. @@ -566,7 +568,7 @@ func (s *VPCServiceImpl) DescribeNetworkInterfaces(ctx context.Context, input *e // eniMatchesFilters checks whether an ENI record matches all parsed filters. func eniMatchesFilters(record *ENIRecord, filters map[string][]string) bool { for name, values := range filters { - if strings.HasPrefix(name, "tag:") { + if filterutil.IsTagFilter(name) { continue } switch name { diff --git a/spinifex/handlers/ec2/vpc/security_group.go b/spinifex/handlers/ec2/vpc/security_group.go index 4f0f60c83..d7fb4b10b 100644 --- a/spinifex/handlers/ec2/vpc/security_group.go +++ b/spinifex/handlers/ec2/vpc/security_group.go @@ -439,6 +439,8 @@ var describeSecurityGroupsValidFilters = map[string]bool{ "vpc-id": true, "description": true, "ip-permission.cidr": true, + "tag-key": true, + "tag-value": true, } // DescribeSecurityGroups lists security groups with optional filters. @@ -520,7 +522,7 @@ func (s *VPCServiceImpl) DescribeSecurityGroups(ctx context.Context, input *ec2. // sgMatchesFilters checks whether a SecurityGroupRecord satisfies all parsed filters. func sgMatchesFilters(record *SecurityGroupRecord, filters map[string][]string) bool { for name, values := range filters { - if strings.HasPrefix(name, "tag:") { + if filterutil.IsTagFilter(name) { continue } @@ -732,6 +734,7 @@ var describeSecurityGroupRulesValidFilters = map[string]bool{ "group-id": true, "security-group-rule-id": true, "tag-key": true, + "tag-value": true, } // DescribeSecurityGroupRules returns a flat list of SecurityGroupRule objects @@ -823,7 +826,7 @@ func (s *VPCServiceImpl) DescribeSecurityGroupRules(ctx context.Context, input * // single rule. func sgRuleMatchesFilters(record *SecurityGroupRecord, rule SGRule, filters map[string][]string) bool { for name, values := range filters { - if strings.HasPrefix(name, "tag:") { + if filterutil.IsTagFilter(name) { continue } switch name { @@ -835,10 +838,6 @@ func sgRuleMatchesFilters(record *SecurityGroupRecord, rule SGRule, filters map[ if !filterutil.MatchesAny(values, rule.RuleId) { return false } - case "tag-key": - if !sgRuleMatchesTagKey(rule.Tags, values) { - return false - } default: // Unreachable: ParseFilters rejects names not in the valid set. // Logs an error if a future map entry lacks a matching case. @@ -917,17 +916,6 @@ func findSGRuleByID(rules []SGRule, ruleID string) *SGRule { return nil } -// sgRuleMatchesTagKey reports whether the rule carries any of the named tag -// keys, whatever their values. -func sgRuleMatchesTagKey(tags map[string]string, keys []string) bool { - for key := range tags { - if filterutil.MatchesAny(keys, key) { - return true - } - } - return false -} - // applySGRuleTags stamps the security-group-rule TagSpecification onto every // rule an authorize call creates. AWS tags the rules the call creates and no // others, so this runs on the new rules alone. diff --git a/spinifex/handlers/ec2/vpc/security_group_test.go b/spinifex/handlers/ec2/vpc/security_group_test.go index ddfb50863..757999ec6 100644 --- a/spinifex/handlers/ec2/vpc/security_group_test.go +++ b/spinifex/handlers/ec2/vpc/security_group_test.go @@ -1415,6 +1415,39 @@ func TestDescribeSecurityGroups_FilterByTag(t *testing.T) { assert.Equal(t, *out.GroupId, *desc.SecurityGroups[0].GroupId) } +func TestDescribeSecurityGroups_FilterByTagKeyAndTagValue(t *testing.T) { + t.Parallel() + svc := setupTestVPCService(t) + vpcID := createTestVPC(t, svc, "10.0.0.0/16") + + out, err := svc.CreateSecurityGroup(context.Background(), &ec2.CreateSecurityGroupInput{ + GroupName: aws.String("tagged-sg"), + Description: aws.String("tagged"), + VpcId: aws.String(vpcID), + TagSpecifications: []*ec2.TagSpecification{{ + ResourceType: aws.String("security-group"), + Tags: []*ec2.Tag{{Key: aws.String("Env"), Value: aws.String("prod")}}, + }}, + }, testAccountID) + require.NoError(t, err) + createTestSG(t, svc, vpcID, "untagged-sg") + + for _, tc := range []struct{ name, value string }{{"tag-key", "Env"}, {"tag-value", "prod"}} { + desc, err := svc.DescribeSecurityGroups(context.Background(), &ec2.DescribeSecurityGroupsInput{ + Filters: []*ec2.Filter{{Name: aws.String(tc.name), Values: []*string{aws.String(tc.value)}}}, + }, testAccountID) + require.NoError(t, err, tc.name) + require.Len(t, desc.SecurityGroups, 1, tc.name) + assert.Equal(t, *out.GroupId, *desc.SecurityGroups[0].GroupId, tc.name) + + desc, err = svc.DescribeSecurityGroups(context.Background(), &ec2.DescribeSecurityGroupsInput{ + Filters: []*ec2.Filter{{Name: aws.String(tc.name), Values: []*string{aws.String("zz-awsdiff-none")}}}, + }, testAccountID) + require.NoError(t, err, tc.name) + assert.Empty(t, desc.SecurityGroups, tc.name) + } +} + func TestDescribeSecurityGroups_FilterNoResults(t *testing.T) { t.Parallel() svc := setupTestVPCService(t) @@ -2440,6 +2473,13 @@ func TestDescribeSecurityGroupRules_FiltersByTag(t *testing.T) { require.NoError(t, err) require.Len(t, byKey.SecurityGroupRules, 1) assert.Equal(t, taggedID, aws.StringValue(byKey.SecurityGroupRules[0].SecurityGroupRuleId)) + + byTagValue, err := svc.DescribeSecurityGroupRules(context.Background(), &ec2.DescribeSecurityGroupRulesInput{ + Filters: []*ec2.Filter{{Name: aws.String("tag-value"), Values: []*string{aws.String("alb")}}}, + }, testAccountID) + require.NoError(t, err) + require.Len(t, byTagValue.SecurityGroupRules, 1) + assert.Equal(t, taggedID, aws.StringValue(byTagValue.SecurityGroupRules[0].SecurityGroupRuleId)) } func TestDescribeSecurityGroupRules_AllProtocolReportsMinusOnePorts(t *testing.T) { diff --git a/spinifex/handlers/ec2/vpc/service_impl.go b/spinifex/handlers/ec2/vpc/service_impl.go index ffde22e09..28b6ee932 100644 --- a/spinifex/handlers/ec2/vpc/service_impl.go +++ b/spinifex/handlers/ec2/vpc/service_impl.go @@ -569,6 +569,8 @@ var describeVpcsValidFilters = map[string]bool{ "isDefault": true, "owner-id": true, "cidr-block-association.cidr-block": true, + "tag-key": true, + "tag-value": true, } // SupportsDescribeVpcsFilter reports whether DescribeVpcs accepts a filter name. @@ -1149,7 +1151,7 @@ func (s *VPCServiceImpl) updateRecordTags(ctx context.Context, accountID, resour // vpcMatchesFilters checks whether a VPCRecord satisfies all parsed filters. func vpcMatchesFilters(record *VPCRecord, accountID string, filters map[string][]string) bool { for name, values := range filters { - if strings.HasPrefix(name, "tag:") { + if filterutil.IsTagFilter(name) { continue } @@ -1189,12 +1191,14 @@ var describeSubnetsValidFilters = map[string]bool{ "cidr-block": true, "state": true, "default-for-az": true, + "tag-key": true, + "tag-value": true, } // subnetMatchesFilters checks whether a SubnetRecord satisfies all parsed filters. func subnetMatchesFilters(record *SubnetRecord, filters map[string][]string) bool { for name, values := range filters { - if strings.HasPrefix(name, "tag:") { + if filterutil.IsTagFilter(name) { continue } diff --git a/spinifex/handlers/ec2/vpc/service_impl_test.go b/spinifex/handlers/ec2/vpc/service_impl_test.go index 7d91887b9..446b5dba4 100644 --- a/spinifex/handlers/ec2/vpc/service_impl_test.go +++ b/spinifex/handlers/ec2/vpc/service_impl_test.go @@ -1882,6 +1882,36 @@ func TestDescribeVpcs_FilterByTag(t *testing.T) { assert.Equal(t, "10.0.0.0/16", *desc.Vpcs[0].CidrBlock) } +func TestDescribeVpcs_FilterByTagKeyAndTagValue(t *testing.T) { + t.Parallel() + svc := setupTestVPCService(t) + + out, err := svc.CreateVpc(context.Background(), &ec2.CreateVpcInput{ + CidrBlock: aws.String("10.0.0.0/16"), + TagSpecifications: []*ec2.TagSpecification{{ + ResourceType: aws.String("vpc"), + Tags: []*ec2.Tag{{Key: aws.String("awsdiff"), Value: aws.String("yes")}}, + }}, + }, testAccountID) + require.NoError(t, err) + createTestVPC(t, svc, "172.16.0.0/16") + + for _, tc := range []struct{ name, value string }{{"tag-key", "awsdiff"}, {"tag-value", "yes"}} { + desc, err := svc.DescribeVpcs(context.Background(), &ec2.DescribeVpcsInput{ + Filters: []*ec2.Filter{{Name: aws.String(tc.name), Values: []*string{aws.String(tc.value)}}}, + }, testAccountID) + require.NoError(t, err, tc.name) + require.Len(t, desc.Vpcs, 1, tc.name) + assert.Equal(t, *out.Vpc.VpcId, *desc.Vpcs[0].VpcId, tc.name) + + desc, err = svc.DescribeVpcs(context.Background(), &ec2.DescribeVpcsInput{ + Filters: []*ec2.Filter{{Name: aws.String(tc.name), Values: []*string{aws.String("zz-awsdiff-none")}}}, + }, testAccountID) + require.NoError(t, err, tc.name) + assert.Empty(t, desc.Vpcs, tc.name) + } +} + func TestDescribeVpcs_FilterByVpcId(t *testing.T) { t.Parallel() svc := setupTestVPCService(t) @@ -2132,6 +2162,38 @@ func TestDescribeSubnets_FilterByTag(t *testing.T) { assert.Equal(t, *out.Subnet.SubnetId, *desc.Subnets[0].SubnetId) } +func TestDescribeSubnets_FilterByTagKeyAndTagValue(t *testing.T) { + t.Parallel() + svc := setupTestVPCService(t) + vpcID := createTestVPC(t, svc, "10.0.0.0/16") + + out, err := svc.CreateSubnet(context.Background(), &ec2.CreateSubnetInput{ + VpcId: aws.String(vpcID), + CidrBlock: aws.String("10.0.1.0/24"), + TagSpecifications: []*ec2.TagSpecification{{ + ResourceType: aws.String("subnet"), + Tags: []*ec2.Tag{{Key: aws.String("Env"), Value: aws.String("prod")}}, + }}, + }, testAccountID) + require.NoError(t, err) + createTestSubnet(t, svc, vpcID, "10.0.2.0/24") + + for _, tc := range []struct{ name, value string }{{"tag-key", "Env"}, {"tag-value", "prod"}} { + desc, err := svc.DescribeSubnets(context.Background(), &ec2.DescribeSubnetsInput{ + Filters: []*ec2.Filter{{Name: aws.String(tc.name), Values: []*string{aws.String(tc.value)}}}, + }, testAccountID) + require.NoError(t, err, tc.name) + require.Len(t, desc.Subnets, 1, tc.name) + assert.Equal(t, *out.Subnet.SubnetId, *desc.Subnets[0].SubnetId, tc.name) + + desc, err = svc.DescribeSubnets(context.Background(), &ec2.DescribeSubnetsInput{ + Filters: []*ec2.Filter{{Name: aws.String(tc.name), Values: []*string{aws.String("zz-awsdiff-none")}}}, + }, testAccountID) + require.NoError(t, err, tc.name) + assert.Empty(t, desc.Subnets, tc.name) + } +} + // --- SetExternalIPAM / GetSubnet --- func TestGetSubnet_Success(t *testing.T) { @@ -2322,6 +2384,35 @@ func TestApplyRecordTags_ENITagFilteredDescribe(t *testing.T) { assert.Equal(t, "primary", findTag(out.NetworkInterfaces[0].TagSet, "Name")) } +func TestDescribeNetworkInterfaces_FilterByTagKeyAndTagValue(t *testing.T) { + t.Parallel() + svc := setupTestVPCService(t) + vpcID := createTestVPC(t, svc, "10.5.0.0/16") + subnetID := createTestSubnet(t, svc, vpcID, "10.5.1.0/24") + eniID := createTestENI(t, svc, subnetID) + createTestENI(t, svc, subnetID) // untagged + + require.NoError(t, svc.ApplyRecordTags(&ec2.CreateTagsInput{ + Resources: []*string{aws.String(eniID)}, + Tags: []*ec2.Tag{{Key: aws.String("Name"), Value: aws.String("primary")}}, + }, testAccountID)) + + for _, tc := range []struct{ name, value string }{{"tag-key", "Name"}, {"tag-value", "primary"}} { + out, err := svc.DescribeNetworkInterfaces(context.Background(), &ec2.DescribeNetworkInterfacesInput{ + Filters: []*ec2.Filter{{Name: aws.String(tc.name), Values: []*string{aws.String(tc.value)}}}, + }, testAccountID) + require.NoError(t, err, tc.name) + require.Len(t, out.NetworkInterfaces, 1, tc.name) + assert.Equal(t, eniID, *out.NetworkInterfaces[0].NetworkInterfaceId, tc.name) + + out, err = svc.DescribeNetworkInterfaces(context.Background(), &ec2.DescribeNetworkInterfacesInput{ + Filters: []*ec2.Filter{{Name: aws.String(tc.name), Values: []*string{aws.String("zz-awsdiff-none")}}}, + }, testAccountID) + require.NoError(t, err, tc.name) + assert.Empty(t, out.NetworkInterfaces, tc.name) + } +} + func TestApplyRecordTags_UnknownResourceNoError(t *testing.T) { t.Parallel() svc := setupTestVPCService(t)