From 400906eee9e51426b6d6c81621cc251aa61f7674 Mon Sep 17 00:00:00 2001 From: ivan katliarchuk Date: Mon, 27 Jan 2025 01:25:04 +0000 Subject: [PATCH 01/12] feat(tag-filter): improve tag handling logic Signed-off-by: ivan katliarchuk --- README.md | 4 + main.go | 4 +- pkg/filters/.gitkeep | 0 pkg/filters/zonetagfilter/testdoubles_test.go | 43 +++++++ .../filters/zonetagfilter}/zone_tag_filter.go | 3 +- .../zonetagfilter/zone_tag_filter_test.go | 106 ++++++++++++++++++ provider/zone_tag_filter_test.go | 62 ---------- 7 files changed, 158 insertions(+), 64 deletions(-) create mode 100644 pkg/filters/.gitkeep create mode 100644 pkg/filters/zonetagfilter/testdoubles_test.go rename {provider => pkg/filters/zonetagfilter}/zone_tag_filter.go (95%) create mode 100644 pkg/filters/zonetagfilter/zone_tag_filter_test.go delete mode 100644 provider/zone_tag_filter_test.go diff --git a/README.md b/README.md index 72ab4ce861..2df56e347e 100644 --- a/README.md +++ b/README.md @@ -271,6 +271,10 @@ Now you can experiment and watch how ExternalDNS makes sure that your DNS record The **tutorials** section contains examples, including Ingress resources, and shows you how to set up ExternalDNS in different environments such as other cloud providers and alternative Ingress controllers. +## Directory Structure + +> TODO + # Note If using a txt registry and attempting to use a CNAME the `--txt-prefix` must be set to avoid conflicts. Changing `--txt-prefix` will result in lost ownership over previously created records. diff --git a/main.go b/main.go index 7d32da4a94..70650134fb 100644 --- a/main.go +++ b/main.go @@ -39,6 +39,7 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/pkg/apis/externaldns" "sigs.k8s.io/external-dns/pkg/apis/externaldns/validation" + "sigs.k8s.io/external-dns/pkg/filters/zonetagfilter" "sigs.k8s.io/external-dns/plan" "sigs.k8s.io/external-dns/provider" "sigs.k8s.io/external-dns/provider/akamai" @@ -186,7 +187,8 @@ func main() { zoneNameFilter := endpoint.NewDomainFilter(cfg.ZoneNameFilter) zoneIDFilter := provider.NewZoneIDFilter(cfg.ZoneIDFilter) zoneTypeFilter := provider.NewZoneTypeFilter(cfg.AWSZoneType) - zoneTagFilter := provider.NewZoneTagFilter(cfg.AWSZoneTagFilter) + // TODO: make all filters available from pkg/filters + zoneTagFilter := zonetagfilter.NewZoneTagFilter(cfg.AWSZoneTagFilter) var p provider.Provider switch cfg.Provider { diff --git a/pkg/filters/.gitkeep b/pkg/filters/.gitkeep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/pkg/filters/zonetagfilter/testdoubles_test.go b/pkg/filters/zonetagfilter/testdoubles_test.go new file mode 100644 index 0000000000..f1b0a1c3e5 --- /dev/null +++ b/pkg/filters/zonetagfilter/testdoubles_test.go @@ -0,0 +1,43 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package zonetagfilter + +import "fmt" + +type filterAndZoneTags struct { + filterTags []string + zoneTags map[string]string +} + +func generateTagFilterAndZoneTagsForMatch(filter, zone int) filterAndZoneTags { + if zone > 50 { + panic("zone number is too high") + } + if filter > zone { + panic("filter number is too high") + } + + filterTags := make([]string, 0, filter) + for i := filter - 1; i >= 0; i-- { + filterTags = append(filterTags, fmt.Sprintf("tag-%d=value-%d", i, i)) + } + zoneTags := make(map[string]string, zone) + for i := 0; i < zone; i++ { + zoneTags[fmt.Sprintf("tag-%d", i)] = fmt.Sprintf("value-%d", i) + } + return filterAndZoneTags{filterTags, zoneTags} +} diff --git a/provider/zone_tag_filter.go b/pkg/filters/zonetagfilter/zone_tag_filter.go similarity index 95% rename from provider/zone_tag_filter.go rename to pkg/filters/zonetagfilter/zone_tag_filter.go index c40ab06e96..3e4805900c 100644 --- a/provider/zone_tag_filter.go +++ b/pkg/filters/zonetagfilter/zone_tag_filter.go @@ -14,7 +14,7 @@ See the License for the specific language governing permissions and limitations under the License. */ -package provider +package zonetagfilter import ( "strings" @@ -33,6 +33,7 @@ func NewZoneTagFilter(tags []string) ZoneTagFilter { return ZoneTagFilter{zoneTags: tags} } +// TODO: pre-process tags on Filter creation // Match checks whether a zone's set of tags matches the provided tag values func (f ZoneTagFilter) Match(tagsMap map[string]string) bool { for _, tagFilter := range f.zoneTags { diff --git a/pkg/filters/zonetagfilter/zone_tag_filter_test.go b/pkg/filters/zonetagfilter/zone_tag_filter_test.go new file mode 100644 index 0000000000..d820de1dea --- /dev/null +++ b/pkg/filters/zonetagfilter/zone_tag_filter_test.go @@ -0,0 +1,106 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package zonetagfilter + +import ( + "fmt" + "testing" + + "github.com/stretchr/testify/assert" +) + +var basicZoneTags = []struct { + name string + zoneTagFilter []string + zoneTags map[string]string + matches bool +}{ + { + "single tag no match", []string{"tag1=value1"}, map[string]string{"tag0": "value0"}, false, + }, + { + "single tag matches", []string{"tag1=value1"}, map[string]string{"tag1": "value1"}, true, + }, + { + "multiple tags no value match", []string{"tag1=value1"}, map[string]string{"tag0": "value0", "tag1": "value2"}, false, + }, + { + "multiple tags matches", []string{"tag1=value1"}, map[string]string{"tag0": "value0", "tag1": "value1"}, true, + }, + { + "tag name no match", []string{"tag1"}, map[string]string{"tag0": "value0"}, false, + }, + { + "tag name matches", []string{"tag1"}, map[string]string{"tag1": "value1"}, true, + }, + { + "multiple filter no match", []string{"tag1=value1", "tag2=value2"}, map[string]string{"tag1": "value1"}, false, + }, + { + "multiple filter matches", []string{"tag1=value1", "tag2=value2"}, map[string]string{"tag2": "value2", "tag1": "value1", "tag3": "value3"}, true, + }, +} + +func TestZoneTagFilterMatch(t *testing.T) { + for _, tc := range basicZoneTags { + zoneTagFilter := NewZoneTagFilter(tc.zoneTagFilter) + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.matches, zoneTagFilter.Match(tc.zoneTags)) + }) + } + + tests := []struct { + filters int + zones int + values filterAndZoneTags + }{ + {10, 30, generateTagFilterAndZoneTagsForMatch(10, 30)}, + {5, 40, generateTagFilterAndZoneTagsForMatch(5, 40)}, + {30, 50, generateTagFilterAndZoneTagsForMatch(30, 50)}, + } + for _, tc := range tests { + t.Run(fmt.Sprintf("filters:%d zones:%d", tc.filters, tc.zones), func(t *testing.T) { + zoneTagFilter := NewZoneTagFilter(tc.values.filterTags) + assert.Equal(t, true, zoneTagFilter.Match(tc.values.zoneTags)) + }) + } +} + +func BenchmarkZoneTagFilterMatchBasic(b *testing.B) { + for _, tc := range basicZoneTags { + zoneTagFilter := NewZoneTagFilter(tc.zoneTagFilter) + for i := 0; i < b.N; i++ { + zoneTagFilter.Match(tc.zoneTags) + } + } +} + +func BenchmarkZoneTagFilterMatchComplex(b *testing.B) { + tests := []struct { + values filterAndZoneTags + }{ + {generateTagFilterAndZoneTagsForMatch(10, 30)}, + {generateTagFilterAndZoneTagsForMatch(5, 40)}, + {generateTagFilterAndZoneTagsForMatch(30, 50)}, + } + for _, tc := range tests { + zoneTagFilter := NewZoneTagFilter(tc.values.filterTags) + for i := 0; i < b.N; i++ { + zoneTagFilter.Match(tc.values.zoneTags) + } + } +} diff --git a/provider/zone_tag_filter_test.go b/provider/zone_tag_filter_test.go deleted file mode 100644 index 9574e68eb0..0000000000 --- a/provider/zone_tag_filter_test.go +++ /dev/null @@ -1,62 +0,0 @@ -/* -Copyright 2017 The Kubernetes Authors. - -Licensed under the Apache License, Version 2.0 (the "License"); -you may not use this file except in compliance with the License. -You may obtain a copy of the License at - - http://www.apache.org/licenses/LICENSE-2.0 - -Unless required by applicable law or agreed to in writing, software -distributed under the License is distributed on an "AS IS" BASIS, -WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -See the License for the specific language governing permissions and -limitations under the License. -*/ - -package provider - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestZoneTagFilterMatch(t *testing.T) { - for _, tc := range []struct { - name string - zoneTagFilter []string - zoneTags map[string]string - matches bool - }{ - { - "single tag no match", []string{"tag1=value1"}, map[string]string{"tag0": "value0"}, false, - }, - { - "single tag matches", []string{"tag1=value1"}, map[string]string{"tag1": "value1"}, true, - }, - { - "multiple tags no value match", []string{"tag1=value1"}, map[string]string{"tag0": "value0", "tag1": "value2"}, false, - }, - { - "multiple tags matches", []string{"tag1=value1"}, map[string]string{"tag0": "value0", "tag1": "value1"}, true, - }, - { - "tag name no match", []string{"tag1"}, map[string]string{"tag0": "value0"}, false, - }, - { - "tag name matches", []string{"tag1"}, map[string]string{"tag1": "value1"}, true, - }, - { - "multiple filter no match", []string{"tag1=value1", "tag2=value2"}, map[string]string{"tag1": "value1"}, false, - }, - { - "multiple filter matches", []string{"tag1=value1", "tag2=value2"}, map[string]string{"tag2": "value2", "tag1": "value1", "tag3": "value3"}, true, - }, - } { - zoneTagFilter := NewZoneTagFilter(tc.zoneTagFilter) - t.Run(tc.name, func(t *testing.T) { - assert.Equal(t, tc.matches, zoneTagFilter.Match(tc.zoneTags)) - }) - } -} From e9ded56d949f24def854b126090eef97acfc9a88 Mon Sep 17 00:00:00 2001 From: ivan katliarchuk Date: Mon, 27 Jan 2025 01:47:26 +0000 Subject: [PATCH 02/12] feat(tag-filter): improve tag handling logic. tags pre-processing Signed-off-by: ivan katliarchuk --- pkg/filters/zonetagfilter/zone_tag_filter.go | 44 ++++++++++++++----- .../zonetagfilter/zone_tag_filter_test.go | 2 + 2 files changed, 36 insertions(+), 10 deletions(-) diff --git a/pkg/filters/zonetagfilter/zone_tag_filter.go b/pkg/filters/zonetagfilter/zone_tag_filter.go index 3e4805900c..656a67d6b3 100644 --- a/pkg/filters/zonetagfilter/zone_tag_filter.go +++ b/pkg/filters/zonetagfilter/zone_tag_filter.go @@ -22,7 +22,8 @@ import ( // ZoneTagFilter holds a list of zone tags to filter by type ZoneTagFilter struct { - zoneTags []string + zoneTags []string + zoneTagsMap map[string]string } // NewZoneTagFilter returns a new ZoneTagFilter given a list of zone tags @@ -30,25 +31,48 @@ func NewZoneTagFilter(tags []string) ZoneTagFilter { if len(tags) == 1 && len(tags[0]) == 0 { tags = []string{} } - return ZoneTagFilter{zoneTags: tags} + z := ZoneTagFilter{} + z.zoneTags = tags + z.zoneTagsMap = make(map[string]string, len(tags)) + for _, tag := range z.zoneTags { + filterParts := strings.SplitN(tag, "=", 2) + if len(filterParts) == 2 { + z.zoneTagsMap[filterParts[0]] = filterParts[1] + } else { + z.zoneTagsMap[filterParts[0]] = "" + } + } + return z } -// TODO: pre-process tags on Filter creation +// Match TODO: pre-process tags on Filter creation // Match checks whether a zone's set of tags matches the provided tag values func (f ZoneTagFilter) Match(tagsMap map[string]string) bool { - for _, tagFilter := range f.zoneTags { - filterParts := strings.SplitN(tagFilter, "=", 2) - switch len(filterParts) { - case 1: - if _, hasTag := tagsMap[filterParts[0]]; !hasTag { + for key, v := range f.zoneTagsMap { + switch len(v) { + case 0: + if _, hasTag := tagsMap[key]; !hasTag { return false } - case 2: - if value, hasTag := tagsMap[filterParts[0]]; !hasTag || value != filterParts[1] { + default: + if value, hasTag := tagsMap[key]; !hasTag || value != v { return false } } } + // for _, tagFilter := range f.zoneTags { + // filterParts := strings.SplitN(tagFilter, "=", 2) + // switch len(filterParts) { + // case 1: + // if _, hasTag := tagsMap[filterParts[0]]; !hasTag { + // return false + // } + // case 2: + // if value, hasTag := tagsMap[filterParts[0]]; !hasTag || value != filterParts[1] { + // return false + // } + // } + // } return true } diff --git a/pkg/filters/zonetagfilter/zone_tag_filter_test.go b/pkg/filters/zonetagfilter/zone_tag_filter_test.go index d820de1dea..e33e7be1ed 100644 --- a/pkg/filters/zonetagfilter/zone_tag_filter_test.go +++ b/pkg/filters/zonetagfilter/zone_tag_filter_test.go @@ -62,7 +62,9 @@ func TestZoneTagFilterMatch(t *testing.T) { assert.Equal(t, tc.matches, zoneTagFilter.Match(tc.zoneTags)) }) } +} +func TestZoneTagFilterMatchGeneratedValues(t *testing.T) { tests := []struct { filters int zones int From 65c2252c2ae322b2ff4cf62f53e9704926f8780c Mon Sep 17 00:00:00 2001 From: ivan katliarchuk Date: Mon, 27 Jan 2025 01:54:55 +0000 Subject: [PATCH 03/12] feat(tag-filter): improve tag handling logic. tags pre-processing Signed-off-by: ivan katliarchuk --- pkg/filters/zonetagfilter/zone_tag_filter.go | 28 ++++++++------------ 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/pkg/filters/zonetagfilter/zone_tag_filter.go b/pkg/filters/zonetagfilter/zone_tag_filter.go index 656a67d6b3..8712a502f8 100644 --- a/pkg/filters/zonetagfilter/zone_tag_filter.go +++ b/pkg/filters/zonetagfilter/zone_tag_filter.go @@ -20,6 +20,8 @@ import ( "strings" ) +// For AWS every zone could have up to 50 tags, and it could be up-to 500 zones in a region + // ZoneTagFilter holds a list of zone tags to filter by type ZoneTagFilter struct { zoneTags []string @@ -35,11 +37,16 @@ func NewZoneTagFilter(tags []string) ZoneTagFilter { z.zoneTags = tags z.zoneTagsMap = make(map[string]string, len(tags)) for _, tag := range z.zoneTags { - filterParts := strings.SplitN(tag, "=", 2) - if len(filterParts) == 2 { - z.zoneTagsMap[filterParts[0]] = filterParts[1] + parts := strings.SplitN(tag, "=", 2) + key := strings.TrimSpace(parts[0]) + if key == "" { + continue + } + if len(parts) == 2 { + value := strings.TrimSpace(parts[1]) + z.zoneTagsMap[key] = value } else { - z.zoneTagsMap[filterParts[0]] = "" + z.zoneTagsMap[key] = "" } } return z @@ -60,19 +67,6 @@ func (f ZoneTagFilter) Match(tagsMap map[string]string) bool { } } } - // for _, tagFilter := range f.zoneTags { - // filterParts := strings.SplitN(tagFilter, "=", 2) - // switch len(filterParts) { - // case 1: - // if _, hasTag := tagsMap[filterParts[0]]; !hasTag { - // return false - // } - // case 2: - // if value, hasTag := tagsMap[filterParts[0]]; !hasTag || value != filterParts[1] { - // return false - // } - // } - // } return true } From d88d736f8b95ed0d0fed138c77ca2816c0630422 Mon Sep 17 00:00:00 2001 From: ivan katliarchuk Date: Mon, 27 Jan 2025 08:55:23 +0000 Subject: [PATCH 04/12] wip Signed-off-by: ivan katliarchuk --- main.go | 5 +-- pkg/filters/filters.go | 11 +++++ pkg/filters/zonetagfilter/testdoubles_test.go | 41 ++++++++++++++----- pkg/filters/zonetagfilter/zone_tag_filter.go | 12 ++---- .../zonetagfilter/zone_tag_filter_test.go | 34 +++++++++++++-- provider/aws/aws.go | 7 ++-- provider/aws/aws_test.go | 3 +- 7 files changed, 83 insertions(+), 30 deletions(-) create mode 100644 pkg/filters/filters.go diff --git a/main.go b/main.go index 70650134fb..4a44877f54 100644 --- a/main.go +++ b/main.go @@ -39,7 +39,7 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/pkg/apis/externaldns" "sigs.k8s.io/external-dns/pkg/apis/externaldns/validation" - "sigs.k8s.io/external-dns/pkg/filters/zonetagfilter" + "sigs.k8s.io/external-dns/pkg/filters" "sigs.k8s.io/external-dns/plan" "sigs.k8s.io/external-dns/provider" "sigs.k8s.io/external-dns/provider/akamai" @@ -187,8 +187,7 @@ func main() { zoneNameFilter := endpoint.NewDomainFilter(cfg.ZoneNameFilter) zoneIDFilter := provider.NewZoneIDFilter(cfg.ZoneIDFilter) zoneTypeFilter := provider.NewZoneTypeFilter(cfg.AWSZoneType) - // TODO: make all filters available from pkg/filters - zoneTagFilter := zonetagfilter.NewZoneTagFilter(cfg.AWSZoneTagFilter) + zoneTagFilter := filters.NewZoneTagFilter(cfg.AWSZoneTagFilter) var p provider.Provider switch cfg.Provider { diff --git a/pkg/filters/filters.go b/pkg/filters/filters.go new file mode 100644 index 0000000000..ef9ef577dd --- /dev/null +++ b/pkg/filters/filters.go @@ -0,0 +1,11 @@ +package filters + +import ( + "sigs.k8s.io/external-dns/pkg/filters/zonetagfilter" +) + +var ( + NewZoneTagFilter = zonetagfilter.NewZoneTagFilter +) + +type ZoneTagFilter = zonetagfilter.ZoneTagFilter diff --git a/pkg/filters/zonetagfilter/testdoubles_test.go b/pkg/filters/zonetagfilter/testdoubles_test.go index f1b0a1c3e5..bac579ec41 100644 --- a/pkg/filters/zonetagfilter/testdoubles_test.go +++ b/pkg/filters/zonetagfilter/testdoubles_test.go @@ -16,20 +16,17 @@ limitations under the License. package zonetagfilter -import "fmt" +import ( + "fmt" +) -type filterAndZoneTags struct { +type filterZoneTags struct { filterTags []string zoneTags map[string]string } -func generateTagFilterAndZoneTagsForMatch(filter, zone int) filterAndZoneTags { - if zone > 50 { - panic("zone number is too high") - } - if filter > zone { - panic("filter number is too high") - } +func generateTagFilterAndZoneTagsForMatch(filter, zone int) filterZoneTags { + validate(filter, zone) filterTags := make([]string, 0, filter) for i := filter - 1; i >= 0; i-- { @@ -39,5 +36,29 @@ func generateTagFilterAndZoneTagsForMatch(filter, zone int) filterAndZoneTags { for i := 0; i < zone; i++ { zoneTags[fmt.Sprintf("tag-%d", i)] = fmt.Sprintf("value-%d", i) } - return filterAndZoneTags{filterTags, zoneTags} + return filterZoneTags{filterTags, zoneTags} +} + +// TODO: explain what is doing this function +func generateTagFilterAndZoneTagsForNotMatch(filter, zone int) filterZoneTags { + validate(filter, zone) + + filterTags := make([]string, 0, filter) + for i := filter - 1; i >= 0; i-- { + filterTags = append(filterTags, fmt.Sprintf("tag-%d=value-%d", i+50, i)) + } + zoneTags := make(map[string]string, zone) + for i := 0; i < zone; i++ { + zoneTags[fmt.Sprintf("tag-%d", i)] = fmt.Sprintf("value-%d", i+2) + } + return filterZoneTags{filterTags, zoneTags} +} + +func validate(filter int, zone int) { + if zone > 50 { + panic("zone number is too high") + } + if filter > zone { + panic("filter number is too high") + } } diff --git a/pkg/filters/zonetagfilter/zone_tag_filter.go b/pkg/filters/zonetagfilter/zone_tag_filter.go index 8712a502f8..64d9a683a1 100644 --- a/pkg/filters/zonetagfilter/zone_tag_filter.go +++ b/pkg/filters/zonetagfilter/zone_tag_filter.go @@ -37,6 +37,7 @@ func NewZoneTagFilter(tags []string) ZoneTagFilter { z.zoneTags = tags z.zoneTagsMap = make(map[string]string, len(tags)) for _, tag := range z.zoneTags { + // TODO: what if tag is empty, this needs a test? parts := strings.SplitN(tag, "=", 2) key := strings.TrimSpace(parts[0]) if key == "" { @@ -56,15 +57,8 @@ func NewZoneTagFilter(tags []string) ZoneTagFilter { // Match checks whether a zone's set of tags matches the provided tag values func (f ZoneTagFilter) Match(tagsMap map[string]string) bool { for key, v := range f.zoneTagsMap { - switch len(v) { - case 0: - if _, hasTag := tagsMap[key]; !hasTag { - return false - } - default: - if value, hasTag := tagsMap[key]; !hasTag || value != v { - return false - } + if value, hasTag := tagsMap[key]; !hasTag || (v != "" && value != v) { + return false } } return true diff --git a/pkg/filters/zonetagfilter/zone_tag_filter_test.go b/pkg/filters/zonetagfilter/zone_tag_filter_test.go index e33e7be1ed..c8073fceaa 100644 --- a/pkg/filters/zonetagfilter/zone_tag_filter_test.go +++ b/pkg/filters/zonetagfilter/zone_tag_filter_test.go @@ -53,6 +53,9 @@ var basicZoneTags = []struct { { "multiple filter matches", []string{"tag1=value1", "tag2=value2"}, map[string]string{"tag2": "value2", "tag1": "value1", "tag3": "value3"}, true, }, + { + "empty tag filter matches all", []string{""}, map[string]string{"tag0": "value0"}, true, + }, } func TestZoneTagFilterMatch(t *testing.T) { @@ -68,7 +71,7 @@ func TestZoneTagFilterMatchGeneratedValues(t *testing.T) { tests := []struct { filters int zones int - values filterAndZoneTags + values filterZoneTags }{ {10, 30, generateTagFilterAndZoneTagsForMatch(10, 30)}, {5, 40, generateTagFilterAndZoneTagsForMatch(5, 40)}, @@ -77,7 +80,25 @@ func TestZoneTagFilterMatchGeneratedValues(t *testing.T) { for _, tc := range tests { t.Run(fmt.Sprintf("filters:%d zones:%d", tc.filters, tc.zones), func(t *testing.T) { zoneTagFilter := NewZoneTagFilter(tc.values.filterTags) - assert.Equal(t, true, zoneTagFilter.Match(tc.values.zoneTags)) + assert.True(t, zoneTagFilter.Match(tc.values.zoneTags)) + }) + } +} + +func TestZoneTagFilterNotMatchGeneratedValues(t *testing.T) { + tests := []struct { + filters int + zones int + values filterZoneTags + }{ + {10, 30, generateTagFilterAndZoneTagsForNotMatch(10, 30)}, + {5, 40, generateTagFilterAndZoneTagsForNotMatch(5, 40)}, + {30, 50, generateTagFilterAndZoneTagsForNotMatch(30, 50)}, + } + for _, tc := range tests { + t.Run(fmt.Sprintf("filters:%d zones:%d", tc.filters, tc.zones), func(t *testing.T) { + zoneTagFilter := NewZoneTagFilter(tc.values.filterTags) + assert.False(t, zoneTagFilter.Match(tc.values.zoneTags)) }) } } @@ -91,13 +112,18 @@ func BenchmarkZoneTagFilterMatchBasic(b *testing.B) { } } -func BenchmarkZoneTagFilterMatchComplex(b *testing.B) { +func BenchmarkZoneTagFilterComplex(b *testing.B) { tests := []struct { - values filterAndZoneTags + values filterZoneTags }{ + // match {generateTagFilterAndZoneTagsForMatch(10, 30)}, {generateTagFilterAndZoneTagsForMatch(5, 40)}, {generateTagFilterAndZoneTagsForMatch(30, 50)}, + // no match + {generateTagFilterAndZoneTagsForNotMatch(10, 30)}, + {generateTagFilterAndZoneTagsForNotMatch(5, 40)}, + {generateTagFilterAndZoneTagsForNotMatch(30, 50)}, } for _, tc := range tests { zoneTagFilter := NewZoneTagFilter(tc.values.filterTags) diff --git a/provider/aws/aws.go b/provider/aws/aws.go index db1ce06c16..c0edf5246d 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -32,6 +32,7 @@ import ( log "github.com/sirupsen/logrus" "sigs.k8s.io/external-dns/endpoint" + "sigs.k8s.io/external-dns/pkg/filters" "sigs.k8s.io/external-dns/plan" "sigs.k8s.io/external-dns/provider" ) @@ -208,7 +209,7 @@ type Route53API interface { ListTagsForResource(ctx context.Context, input *route53.ListTagsForResourceInput, optFns ...func(options *route53.Options)) (*route53.ListTagsForResourceOutput, error) } -// wrapper to handle ownership relation throughout the provider implementation +// Route53Change wrapper to handle ownership relation throughout the provider implementation type Route53Change struct { route53types.Change OwnedRecord string @@ -254,7 +255,7 @@ type AWSProvider struct { // filter hosted zones by type (e.g. private or public) zoneTypeFilter provider.ZoneTypeFilter // filter hosted zones by tags - zoneTagFilter provider.ZoneTagFilter + zoneTagFilter filters.ZoneTagFilter // extend filter for subdomains in the zone (e.g. first.us-east-1.example.com) zoneMatchParent bool preferCNAME bool @@ -268,7 +269,7 @@ type AWSConfig struct { DomainFilter endpoint.DomainFilter ZoneIDFilter provider.ZoneIDFilter ZoneTypeFilter provider.ZoneTypeFilter - ZoneTagFilter provider.ZoneTagFilter + ZoneTagFilter filters.ZoneTagFilter ZoneMatchParent bool BatchChangeSize int BatchChangeSizeBytes int diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index 12d9f6940d..e90525762d 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -35,6 +35,7 @@ import ( "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/internal/testutils" + "sigs.k8s.io/external-dns/pkg/filters" "sigs.k8s.io/external-dns/plan" "sigs.k8s.io/external-dns/provider" ) @@ -319,7 +320,7 @@ func TestAWSZones(t *testing.T) { msg string zoneIDFilter provider.ZoneIDFilter zoneTypeFilter provider.ZoneTypeFilter - zoneTagFilter provider.ZoneTagFilter + zoneTagFilter filters.ZoneTagFilter expectedZones map[string]*route53types.HostedZone }{ {"no filter", provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), provider.NewZoneTagFilter([]string{}), allZones}, From 8784341d5ef4172393ed45e580f45fa9b097a28e Mon Sep 17 00:00:00 2001 From: ivan katliarchuk Date: Tue, 28 Jan 2025 17:37:08 +0000 Subject: [PATCH 05/12] chore: tags filtering Signed-off-by: ivan katliarchuk --- README.md | 4 -- pkg/filters/.gitkeep | 0 pkg/filters/filters.go | 16 +++++++ pkg/filters/zonetagfilter/testdoubles_test.go | 42 ++++++++++--------- pkg/filters/zonetagfilter/zone_tag_filter.go | 3 +- provider/aws/aws_test.go | 18 ++++---- 6 files changed, 49 insertions(+), 34 deletions(-) delete mode 100644 pkg/filters/.gitkeep diff --git a/README.md b/README.md index 2df56e347e..72ab4ce861 100644 --- a/README.md +++ b/README.md @@ -271,10 +271,6 @@ Now you can experiment and watch how ExternalDNS makes sure that your DNS record The **tutorials** section contains examples, including Ingress resources, and shows you how to set up ExternalDNS in different environments such as other cloud providers and alternative Ingress controllers. -## Directory Structure - -> TODO - # Note If using a txt registry and attempting to use a CNAME the `--txt-prefix` must be set to avoid conflicts. Changing `--txt-prefix` will result in lost ownership over previously created records. diff --git a/pkg/filters/.gitkeep b/pkg/filters/.gitkeep deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/pkg/filters/filters.go b/pkg/filters/filters.go index ef9ef577dd..4b78584ff5 100644 --- a/pkg/filters/filters.go +++ b/pkg/filters/filters.go @@ -1,3 +1,19 @@ +/* +Copyright 2017 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package filters import ( diff --git a/pkg/filters/zonetagfilter/testdoubles_test.go b/pkg/filters/zonetagfilter/testdoubles_test.go index bac579ec41..833d60c33b 100644 --- a/pkg/filters/zonetagfilter/testdoubles_test.go +++ b/pkg/filters/zonetagfilter/testdoubles_test.go @@ -16,41 +16,45 @@ limitations under the License. package zonetagfilter -import ( - "fmt" -) +import "fmt" type filterZoneTags struct { filterTags []string zoneTags map[string]string } +// generateTagFilterAndZoneTagsForMatch generates filter tags and zone tags that do match. func generateTagFilterAndZoneTagsForMatch(filter, zone int) filterZoneTags { - validate(filter, zone) - - filterTags := make([]string, 0, filter) - for i := filter - 1; i >= 0; i-- { - filterTags = append(filterTags, fmt.Sprintf("tag-%d=value-%d", i, i)) - } - zoneTags := make(map[string]string, zone) - for i := 0; i < zone; i++ { - zoneTags[fmt.Sprintf("tag-%d", i)] = fmt.Sprintf("value-%d", i) - } - return filterZoneTags{filterTags, zoneTags} + return generateTagFilterAndZoneTags(filter, zone, true) } -// TODO: explain what is doing this function +// generateTagFilterAndZoneTagsForNotMatch generates filter tags and zone tags that do not match. func generateTagFilterAndZoneTagsForNotMatch(filter, zone int) filterZoneTags { - validate(filter, zone) + return generateTagFilterAndZoneTags(filter, zone, false) +} +// generateTagFilterAndZoneTags generates filter tags and zone tags based on the match parameter. +func generateTagFilterAndZoneTags(filter, zone int, match bool) filterZoneTags { + validate(filter, zone) filterTags := make([]string, 0, filter) + zoneTags := make(map[string]string, zone) + for i := filter - 1; i >= 0; i-- { - filterTags = append(filterTags, fmt.Sprintf("tag-%d=value-%d", i+50, i)) + if match { + filterTags = append(filterTags, fmt.Sprintf("tag-%d=value-%d", i, i)) + } else { + filterTags = append(filterTags, fmt.Sprintf("tag-%d=value-%d", i+50, i)) + } } - zoneTags := make(map[string]string, zone) + for i := 0; i < zone; i++ { - zoneTags[fmt.Sprintf("tag-%d", i)] = fmt.Sprintf("value-%d", i+2) + if match { + zoneTags[fmt.Sprintf("tag-%d", i)] = fmt.Sprintf("value-%d", i) + } else { + zoneTags[fmt.Sprintf("tag-%d", i)] = fmt.Sprintf("value-%d", i+2) + } } + return filterZoneTags{filterTags, zoneTags} } diff --git a/pkg/filters/zonetagfilter/zone_tag_filter.go b/pkg/filters/zonetagfilter/zone_tag_filter.go index 64d9a683a1..7fcb076d65 100644 --- a/pkg/filters/zonetagfilter/zone_tag_filter.go +++ b/pkg/filters/zonetagfilter/zone_tag_filter.go @@ -36,8 +36,8 @@ func NewZoneTagFilter(tags []string) ZoneTagFilter { z := ZoneTagFilter{} z.zoneTags = tags z.zoneTagsMap = make(map[string]string, len(tags)) + // tags pre-processing, to make sure the pre-processing is not happening at the time of filtering for _, tag := range z.zoneTags { - // TODO: what if tag is empty, this needs a test? parts := strings.SplitN(tag, "=", 2) key := strings.TrimSpace(parts[0]) if key == "" { @@ -53,7 +53,6 @@ func NewZoneTagFilter(tags []string) ZoneTagFilter { return z } -// Match TODO: pre-process tags on Filter creation // Match checks whether a zone's set of tags matches the provided tag values func (f ZoneTagFilter) Match(tagsMap map[string]string) bool { for key, v := range f.zoneTagsMap { diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index 58a11df4d2..b481a24ee9 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -329,12 +329,12 @@ func TestAWSZones(t *testing.T) { zoneTagFilter filters.ZoneTagFilter expectedZones map[string]*route53types.HostedZone }{ - {"no filter", provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), provider.NewZoneTagFilter([]string{}), allZones}, - {"public filter", provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter("public"), provider.NewZoneTagFilter([]string{}), publicZones}, - {"private filter", provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter("private"), provider.NewZoneTagFilter([]string{}), privateZones}, - {"unknown filter", provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter("unknown"), provider.NewZoneTagFilter([]string{}), noZones}, - {"zone id filter", provider.NewZoneIDFilter([]string{"/hostedzone/zone-3.ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneTypeFilter(""), provider.NewZoneTagFilter([]string{}), privateZones}, - {"tag filter", provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), provider.NewZoneTagFilter([]string{"zone=3"}), privateZones}, + {"no filter", provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), filters.NewZoneTagFilter([]string{}), allZones}, + {"public filter", provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter("public"), filters.NewZoneTagFilter([]string{}), publicZones}, + {"private filter", provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter("private"), filters.NewZoneTagFilter([]string{}), privateZones}, + {"unknown filter", provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter("unknown"), filters.NewZoneTagFilter([]string{}), noZones}, + {"zone id filter", provider.NewZoneIDFilter([]string{"/hostedzone/zone-3.ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneTypeFilter(""), filters.NewZoneTagFilter([]string{}), privateZones}, + {"tag filter", provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), filters.NewZoneTagFilter([]string{"zone=3"}), privateZones}, } { t.Run(ti.msg, func(t *testing.T) { provider, _ := newAWSProviderWithTagFilter(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), ti.zoneIDFilter, ti.zoneTypeFilter, ti.zoneTagFilter, defaultEvaluateTargetHealth, false, nil) @@ -349,7 +349,7 @@ func TestAWSZonesWithTagFilterError(t *testing.T) { client := NewRoute53APIStub(t) provider := &AWSProvider{ clients: map[string]Route53API{defaultAWSProfile: client}, - zoneTagFilter: provider.NewZoneTagFilter([]string{"zone=2"}), + zoneTagFilter: filters.NewZoneTagFilter([]string{"zone=2"}), dryRun: false, zonesCache: &zonesListCache{duration: 1 * time.Minute}, } @@ -2030,10 +2030,10 @@ func listAWSRecords(t *testing.T, client Route53API, zone string) []route53types } func newAWSProvider(t *testing.T, domainFilter endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, zoneTypeFilter provider.ZoneTypeFilter, evaluateTargetHealth, dryRun bool, records []route53types.ResourceRecordSet) (*AWSProvider, *Route53APIStub) { - return newAWSProviderWithTagFilter(t, domainFilter, zoneIDFilter, zoneTypeFilter, provider.NewZoneTagFilter([]string{}), evaluateTargetHealth, dryRun, records) + return newAWSProviderWithTagFilter(t, domainFilter, zoneIDFilter, zoneTypeFilter, filters.NewZoneTagFilter([]string{}), evaluateTargetHealth, dryRun, records) } -func newAWSProviderWithTagFilter(t *testing.T, domainFilter endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, zoneTypeFilter provider.ZoneTypeFilter, zoneTagFilter provider.ZoneTagFilter, evaluateTargetHealth, dryRun bool, records []route53types.ResourceRecordSet) (*AWSProvider, *Route53APIStub) { +func newAWSProviderWithTagFilter(t *testing.T, domainFilter endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, zoneTypeFilter provider.ZoneTypeFilter, zoneTagFilter filters.ZoneTagFilter, evaluateTargetHealth, dryRun bool, records []route53types.ResourceRecordSet) (*AWSProvider, *Route53APIStub) { client := NewRoute53APIStub(t) provider := &AWSProvider{ From 71832d2708dd51a6cbe691bb3f40d65a28eb4180 Mon Sep 17 00:00:00 2001 From: ivan katliarchuk Date: Tue, 28 Jan 2025 18:25:40 +0000 Subject: [PATCH 06/12] chore: tags filtering Signed-off-by: ivan katliarchuk --- pkg/filters/zonetagfilter/testdoubles_test.go | 6 +++--- pkg/filters/zonetagfilter/zone_tag_filter_test.go | 15 ++++++--------- 2 files changed, 9 insertions(+), 12 deletions(-) diff --git a/pkg/filters/zonetagfilter/testdoubles_test.go b/pkg/filters/zonetagfilter/testdoubles_test.go index 833d60c33b..2c222d5e70 100644 --- a/pkg/filters/zonetagfilter/testdoubles_test.go +++ b/pkg/filters/zonetagfilter/testdoubles_test.go @@ -19,8 +19,8 @@ package zonetagfilter import "fmt" type filterZoneTags struct { - filterTags []string - zoneTags map[string]string + ZoneTagFilter + zoneTags map[string]string } // generateTagFilterAndZoneTagsForMatch generates filter tags and zone tags that do match. @@ -55,7 +55,7 @@ func generateTagFilterAndZoneTags(filter, zone int, match bool) filterZoneTags { } } - return filterZoneTags{filterTags, zoneTags} + return filterZoneTags{NewZoneTagFilter(filterTags), zoneTags} } func validate(filter int, zone int) { diff --git a/pkg/filters/zonetagfilter/zone_tag_filter_test.go b/pkg/filters/zonetagfilter/zone_tag_filter_test.go index c8073fceaa..15ff82df5f 100644 --- a/pkg/filters/zonetagfilter/zone_tag_filter_test.go +++ b/pkg/filters/zonetagfilter/zone_tag_filter_test.go @@ -71,7 +71,7 @@ func TestZoneTagFilterMatchGeneratedValues(t *testing.T) { tests := []struct { filters int zones int - values filterZoneTags + source filterZoneTags }{ {10, 30, generateTagFilterAndZoneTagsForMatch(10, 30)}, {5, 40, generateTagFilterAndZoneTagsForMatch(5, 40)}, @@ -79,8 +79,7 @@ func TestZoneTagFilterMatchGeneratedValues(t *testing.T) { } for _, tc := range tests { t.Run(fmt.Sprintf("filters:%d zones:%d", tc.filters, tc.zones), func(t *testing.T) { - zoneTagFilter := NewZoneTagFilter(tc.values.filterTags) - assert.True(t, zoneTagFilter.Match(tc.values.zoneTags)) + assert.True(t, tc.source.ZoneTagFilter.Match(tc.source.zoneTags)) }) } } @@ -89,7 +88,7 @@ func TestZoneTagFilterNotMatchGeneratedValues(t *testing.T) { tests := []struct { filters int zones int - values filterZoneTags + source filterZoneTags }{ {10, 30, generateTagFilterAndZoneTagsForNotMatch(10, 30)}, {5, 40, generateTagFilterAndZoneTagsForNotMatch(5, 40)}, @@ -97,8 +96,7 @@ func TestZoneTagFilterNotMatchGeneratedValues(t *testing.T) { } for _, tc := range tests { t.Run(fmt.Sprintf("filters:%d zones:%d", tc.filters, tc.zones), func(t *testing.T) { - zoneTagFilter := NewZoneTagFilter(tc.values.filterTags) - assert.False(t, zoneTagFilter.Match(tc.values.zoneTags)) + assert.False(t, tc.source.ZoneTagFilter.Match(tc.source.zoneTags)) }) } } @@ -114,7 +112,7 @@ func BenchmarkZoneTagFilterMatchBasic(b *testing.B) { func BenchmarkZoneTagFilterComplex(b *testing.B) { tests := []struct { - values filterZoneTags + source filterZoneTags }{ // match {generateTagFilterAndZoneTagsForMatch(10, 30)}, @@ -126,9 +124,8 @@ func BenchmarkZoneTagFilterComplex(b *testing.B) { {generateTagFilterAndZoneTagsForNotMatch(30, 50)}, } for _, tc := range tests { - zoneTagFilter := NewZoneTagFilter(tc.values.filterTags) for i := 0; i < b.N; i++ { - zoneTagFilter.Match(tc.values.zoneTags) + tc.source.ZoneTagFilter.Match(tc.source.zoneTags) } } } From 62db4c6edeaaa76e9e57d0471b3672c0ffca2061 Mon Sep 17 00:00:00 2001 From: ivan katliarchuk Date: Tue, 28 Jan 2025 18:45:31 +0000 Subject: [PATCH 07/12] chore: tags filtering update bench Signed-off-by: ivan katliarchuk --- .../zonetagfilter/zone_tag_filter_test.go | 31 ++++++++++--------- 1 file changed, 16 insertions(+), 15 deletions(-) diff --git a/pkg/filters/zonetagfilter/zone_tag_filter_test.go b/pkg/filters/zonetagfilter/zone_tag_filter_test.go index 15ff82df5f..c7abc47c94 100644 --- a/pkg/filters/zonetagfilter/zone_tag_filter_test.go +++ b/pkg/filters/zonetagfilter/zone_tag_filter_test.go @@ -104,27 +104,28 @@ func TestZoneTagFilterNotMatchGeneratedValues(t *testing.T) { func BenchmarkZoneTagFilterMatchBasic(b *testing.B) { for _, tc := range basicZoneTags { zoneTagFilter := NewZoneTagFilter(tc.zoneTagFilter) - for i := 0; i < b.N; i++ { + for range b.N { zoneTagFilter.Match(tc.zoneTags) } } } +var benchFixtures = []struct { + source filterZoneTags +}{ + // match + {generateTagFilterAndZoneTagsForMatch(10, 30)}, + {generateTagFilterAndZoneTagsForMatch(5, 40)}, + {generateTagFilterAndZoneTagsForMatch(30, 50)}, + // no match + {generateTagFilterAndZoneTagsForNotMatch(10, 30)}, + {generateTagFilterAndZoneTagsForNotMatch(5, 40)}, + {generateTagFilterAndZoneTagsForNotMatch(30, 50)}, +} + func BenchmarkZoneTagFilterComplex(b *testing.B) { - tests := []struct { - source filterZoneTags - }{ - // match - {generateTagFilterAndZoneTagsForMatch(10, 30)}, - {generateTagFilterAndZoneTagsForMatch(5, 40)}, - {generateTagFilterAndZoneTagsForMatch(30, 50)}, - // no match - {generateTagFilterAndZoneTagsForNotMatch(10, 30)}, - {generateTagFilterAndZoneTagsForNotMatch(5, 40)}, - {generateTagFilterAndZoneTagsForNotMatch(30, 50)}, - } - for _, tc := range tests { - for i := 0; i < b.N; i++ { + for _, tc := range benchFixtures { + for range b.N { tc.source.ZoneTagFilter.Match(tc.source.zoneTags) } } From 70b2141691921946853aa18a822722c554a5ad7a Mon Sep 17 00:00:00 2001 From: ivan katliarchuk Date: Tue, 28 Jan 2025 19:02:50 +0000 Subject: [PATCH 08/12] chore: tags filtering update test with not supported tag formats Signed-off-by: ivan katliarchuk --- .../zonetagfilter/zone_tag_filter_test.go | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/pkg/filters/zonetagfilter/zone_tag_filter_test.go b/pkg/filters/zonetagfilter/zone_tag_filter_test.go index c7abc47c94..dcc88865ca 100644 --- a/pkg/filters/zonetagfilter/zone_tag_filter_test.go +++ b/pkg/filters/zonetagfilter/zone_tag_filter_test.go @@ -67,6 +67,24 @@ func TestZoneTagFilterMatch(t *testing.T) { } } +func TestZoneTagFilterNotSupportedFormat(t *testing.T) { + tests := []struct { + desc string + tags []string + want []string + }{ + {desc: "multiple or separate values with commas", tags: []string{"key1=val1,key2=val2"}, want: []string{"key1=val1,key2=val2"}}, + {desc: "exclude tag", tags: []string{"!key1"}, want: []string{"!key1"}}, + {desc: "exclude tags", tags: []string{"!key1=val"}, want: []string{"!key1=val"}}, + } + for _, tc := range tests { + t.Run(fmt.Sprintf("%s", tc.desc), func(t *testing.T) { + got := NewZoneTagFilter(tc.tags) + assert.Equal(t, tc.want, got.zoneTags) + }) + } +} + func TestZoneTagFilterMatchGeneratedValues(t *testing.T) { tests := []struct { filters int From 1ae27cfa6d66ee52d772d6a50c62feeacf60865e Mon Sep 17 00:00:00 2001 From: ivan katliarchuk Date: Thu, 30 Jan 2025 21:24:17 +0000 Subject: [PATCH 09/12] feat(filters): remove zonetags as and rename to tagsMap Signed-off-by: ivan katliarchuk --- pkg/filters/zonetagfilter/zone_tag_filter.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/pkg/filters/zonetagfilter/zone_tag_filter.go b/pkg/filters/zonetagfilter/zone_tag_filter.go index 7fcb076d65..7c21ea50a1 100644 --- a/pkg/filters/zonetagfilter/zone_tag_filter.go +++ b/pkg/filters/zonetagfilter/zone_tag_filter.go @@ -24,7 +24,6 @@ import ( // ZoneTagFilter holds a list of zone tags to filter by type ZoneTagFilter struct { - zoneTags []string zoneTagsMap map[string]string } @@ -34,10 +33,9 @@ func NewZoneTagFilter(tags []string) ZoneTagFilter { tags = []string{} } z := ZoneTagFilter{} - z.zoneTags = tags z.zoneTagsMap = make(map[string]string, len(tags)) // tags pre-processing, to make sure the pre-processing is not happening at the time of filtering - for _, tag := range z.zoneTags { + for _, tag := range tags { parts := strings.SplitN(tag, "=", 2) key := strings.TrimSpace(parts[0]) if key == "" { From 6aeb7c38f9179050c15c1dfabfc1559ac88e877c Mon Sep 17 00:00:00 2001 From: ivan katliarchuk Date: Thu, 30 Jan 2025 21:36:08 +0000 Subject: [PATCH 10/12] feat(filters): remove zonetags as and rename to tagsMap Signed-off-by: ivan katliarchuk --- pkg/filters/zonetagfilter/zone_tag_filter.go | 14 ++++++-------- pkg/filters/zonetagfilter/zone_tag_filter_test.go | 10 +++++----- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/pkg/filters/zonetagfilter/zone_tag_filter.go b/pkg/filters/zonetagfilter/zone_tag_filter.go index 7c21ea50a1..f489a0c25c 100644 --- a/pkg/filters/zonetagfilter/zone_tag_filter.go +++ b/pkg/filters/zonetagfilter/zone_tag_filter.go @@ -20,11 +20,9 @@ import ( "strings" ) -// For AWS every zone could have up to 50 tags, and it could be up-to 500 zones in a region - // ZoneTagFilter holds a list of zone tags to filter by type ZoneTagFilter struct { - zoneTagsMap map[string]string + tagsMap map[string]string } // NewZoneTagFilter returns a new ZoneTagFilter given a list of zone tags @@ -33,7 +31,7 @@ func NewZoneTagFilter(tags []string) ZoneTagFilter { tags = []string{} } z := ZoneTagFilter{} - z.zoneTagsMap = make(map[string]string, len(tags)) + z.tagsMap = make(map[string]string, len(tags)) // tags pre-processing, to make sure the pre-processing is not happening at the time of filtering for _, tag := range tags { parts := strings.SplitN(tag, "=", 2) @@ -43,9 +41,9 @@ func NewZoneTagFilter(tags []string) ZoneTagFilter { } if len(parts) == 2 { value := strings.TrimSpace(parts[1]) - z.zoneTagsMap[key] = value + z.tagsMap[key] = value } else { - z.zoneTagsMap[key] = "" + z.tagsMap[key] = "" } } return z @@ -53,7 +51,7 @@ func NewZoneTagFilter(tags []string) ZoneTagFilter { // Match checks whether a zone's set of tags matches the provided tag values func (f ZoneTagFilter) Match(tagsMap map[string]string) bool { - for key, v := range f.zoneTagsMap { + for key, v := range f.tagsMap { if value, hasTag := tagsMap[key]; !hasTag || (v != "" && value != v) { return false } @@ -63,5 +61,5 @@ func (f ZoneTagFilter) Match(tagsMap map[string]string) bool { // IsEmpty returns true if there are no tags for the filter func (f ZoneTagFilter) IsEmpty() bool { - return len(f.zoneTags) == 0 + return len(f.tagsMap) == 0 } diff --git a/pkg/filters/zonetagfilter/zone_tag_filter_test.go b/pkg/filters/zonetagfilter/zone_tag_filter_test.go index dcc88865ca..93faaa095c 100644 --- a/pkg/filters/zonetagfilter/zone_tag_filter_test.go +++ b/pkg/filters/zonetagfilter/zone_tag_filter_test.go @@ -71,16 +71,16 @@ func TestZoneTagFilterNotSupportedFormat(t *testing.T) { tests := []struct { desc string tags []string - want []string + want map[string]string }{ - {desc: "multiple or separate values with commas", tags: []string{"key1=val1,key2=val2"}, want: []string{"key1=val1,key2=val2"}}, - {desc: "exclude tag", tags: []string{"!key1"}, want: []string{"!key1"}}, - {desc: "exclude tags", tags: []string{"!key1=val"}, want: []string{"!key1=val"}}, + {desc: "multiple or separate values with commas", tags: []string{"key1=val1,key2=val2"}, want: map[string]string{"key1": "val1,key2=val2"}}, + {desc: "exclude tag", tags: []string{"!key1"}, want: map[string]string{"!key1": ""}}, + {desc: "exclude tags", tags: []string{"!key1=val"}, want: map[string]string{"!key1": "val"}}, } for _, tc := range tests { t.Run(fmt.Sprintf("%s", tc.desc), func(t *testing.T) { got := NewZoneTagFilter(tc.tags) - assert.Equal(t, tc.want, got.zoneTags) + assert.Equal(t, tc.want, got.tagsMap) }) } } From 9293a2a63f8540a4ac7fecc241ca377345be4926 Mon Sep 17 00:00:00 2001 From: ivan katliarchuk Date: Thu, 30 Jan 2025 21:42:11 +0000 Subject: [PATCH 11/12] feat(filters): remove zonetags as and rename to tagsMap Signed-off-by: ivan katliarchuk --- pkg/filters/zonetagfilter/testdoubles_test.go | 27 ++++++++++--------- .../zonetagfilter/zone_tag_filter_test.go | 6 ++--- 2 files changed, 17 insertions(+), 16 deletions(-) diff --git a/pkg/filters/zonetagfilter/testdoubles_test.go b/pkg/filters/zonetagfilter/testdoubles_test.go index 2c222d5e70..87ec957e2f 100644 --- a/pkg/filters/zonetagfilter/testdoubles_test.go +++ b/pkg/filters/zonetagfilter/testdoubles_test.go @@ -20,7 +20,7 @@ import "fmt" type filterZoneTags struct { ZoneTagFilter - zoneTags map[string]string + inputTags map[string]string } // generateTagFilterAndZoneTagsForMatch generates filter tags and zone tags that do match. @@ -36,26 +36,27 @@ func generateTagFilterAndZoneTagsForNotMatch(filter, zone int) filterZoneTags { // generateTagFilterAndZoneTags generates filter tags and zone tags based on the match parameter. func generateTagFilterAndZoneTags(filter, zone int, match bool) filterZoneTags { validate(filter, zone) - filterTags := make([]string, 0, filter) - zoneTags := make(map[string]string, zone) + toFilterTags := make([]string, 0, filter) + inputTags := make(map[string]string, zone) - for i := filter - 1; i >= 0; i-- { - if match { - filterTags = append(filterTags, fmt.Sprintf("tag-%d=value-%d", i, i)) - } else { - filterTags = append(filterTags, fmt.Sprintf("tag-%d=value-%d", i+50, i)) + for i := 0; i < filter; i++ { + tagIndex := i + if !match { + tagIndex += 50 } + toFilterTags = append(toFilterTags, fmt.Sprintf("tag-%d=value-%d", tagIndex, i)) } for i := 0; i < zone; i++ { - if match { - zoneTags[fmt.Sprintf("tag-%d", i)] = fmt.Sprintf("value-%d", i) - } else { - zoneTags[fmt.Sprintf("tag-%d", i)] = fmt.Sprintf("value-%d", i+2) + tagIndex := i + if !match { + // Make sure the input tags are different from the filter tags + tagIndex += 2 } + inputTags[fmt.Sprintf("tag-%d", i)] = fmt.Sprintf("value-%d", tagIndex) } - return filterZoneTags{NewZoneTagFilter(filterTags), zoneTags} + return filterZoneTags{NewZoneTagFilter(toFilterTags), inputTags} } func validate(filter int, zone int) { diff --git a/pkg/filters/zonetagfilter/zone_tag_filter_test.go b/pkg/filters/zonetagfilter/zone_tag_filter_test.go index 93faaa095c..2065c102de 100644 --- a/pkg/filters/zonetagfilter/zone_tag_filter_test.go +++ b/pkg/filters/zonetagfilter/zone_tag_filter_test.go @@ -97,7 +97,7 @@ func TestZoneTagFilterMatchGeneratedValues(t *testing.T) { } for _, tc := range tests { t.Run(fmt.Sprintf("filters:%d zones:%d", tc.filters, tc.zones), func(t *testing.T) { - assert.True(t, tc.source.ZoneTagFilter.Match(tc.source.zoneTags)) + assert.True(t, tc.source.ZoneTagFilter.Match(tc.source.inputTags)) }) } } @@ -114,7 +114,7 @@ func TestZoneTagFilterNotMatchGeneratedValues(t *testing.T) { } for _, tc := range tests { t.Run(fmt.Sprintf("filters:%d zones:%d", tc.filters, tc.zones), func(t *testing.T) { - assert.False(t, tc.source.ZoneTagFilter.Match(tc.source.zoneTags)) + assert.False(t, tc.source.ZoneTagFilter.Match(tc.source.inputTags)) }) } } @@ -144,7 +144,7 @@ var benchFixtures = []struct { func BenchmarkZoneTagFilterComplex(b *testing.B) { for _, tc := range benchFixtures { for range b.N { - tc.source.ZoneTagFilter.Match(tc.source.zoneTags) + tc.source.ZoneTagFilter.Match(tc.source.inputTags) } } } From 14bf06679708b219483fae19dba34def104ea76c Mon Sep 17 00:00:00 2001 From: ivan katliarchuk Date: Thu, 30 Jan 2025 21:47:54 +0000 Subject: [PATCH 12/12] feat(filters): remove zonetags as and rename to tagsMap Signed-off-by: ivan katliarchuk --- .../zonetagfilter/zone_tag_filter_test.go | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/pkg/filters/zonetagfilter/zone_tag_filter_test.go b/pkg/filters/zonetagfilter/zone_tag_filter_test.go index 2065c102de..130f0ebf24 100644 --- a/pkg/filters/zonetagfilter/zone_tag_filter_test.go +++ b/pkg/filters/zonetagfilter/zone_tag_filter_test.go @@ -24,10 +24,10 @@ import ( ) var basicZoneTags = []struct { - name string - zoneTagFilter []string - zoneTags map[string]string - matches bool + name string + tagsFilter []string + zoneTags map[string]string + matches bool }{ { "single tag no match", []string{"tag1=value1"}, map[string]string{"tag0": "value0"}, false, @@ -56,13 +56,16 @@ var basicZoneTags = []struct { { "empty tag filter matches all", []string{""}, map[string]string{"tag0": "value0"}, true, }, + { + "tag filter with empty key is ignored", []string{"tag1=value1", "=haha"}, map[string]string{"tag1": "value1"}, true, + }, } func TestZoneTagFilterMatch(t *testing.T) { for _, tc := range basicZoneTags { - zoneTagFilter := NewZoneTagFilter(tc.zoneTagFilter) + filter := NewZoneTagFilter(tc.tagsFilter) t.Run(tc.name, func(t *testing.T) { - assert.Equal(t, tc.matches, zoneTagFilter.Match(tc.zoneTags)) + assert.Equal(t, tc.matches, filter.Match(tc.zoneTags)) }) } } @@ -121,7 +124,7 @@ func TestZoneTagFilterNotMatchGeneratedValues(t *testing.T) { func BenchmarkZoneTagFilterMatchBasic(b *testing.B) { for _, tc := range basicZoneTags { - zoneTagFilter := NewZoneTagFilter(tc.zoneTagFilter) + zoneTagFilter := NewZoneTagFilter(tc.tagsFilter) for range b.N { zoneTagFilter.Match(tc.zoneTags) }