From 0a9daa9e9b16307610f9c2cda88402390a8f2494 Mon Sep 17 00:00:00 2001 From: Rob Selway Date: Sun, 14 Feb 2021 13:40:25 +0000 Subject: [PATCH 1/4] Case insensitivity when comparing targets --- endpoint/endpoint.go | 4 ++-- endpoint/endpoint_test.go | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index bb082e2a5..07bd3ea4b 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -71,7 +71,7 @@ func (t Targets) Swap(i, j int) { t[i], t[j] = t[j], t[i] } -// Same compares to Targets and returns true if they are completely identical +// Same compares to Targets and returns true if they are identical (case-insensitive) func (t Targets) Same(o Targets) bool { if len(t) != len(o) { return false @@ -80,7 +80,7 @@ func (t Targets) Same(o Targets) bool { sort.Stable(o) for i, e := range t { - if e != o[i] { + if !strings.EqualFold(e, o[i]) { return false } } diff --git a/endpoint/endpoint_test.go b/endpoint/endpoint_test.go index 18a79a02c..05104adbf 100644 --- a/endpoint/endpoint_test.go +++ b/endpoint/endpoint_test.go @@ -40,6 +40,7 @@ func TestTargetsSame(t *testing.T) { {""}, {"1.2.3.4"}, {"8.8.8.8", "8.8.4.4"}, + {"example.org", "EXAMPLE.ORG"}, } for _, d := range tests { From 091b8a8f825f7b6c2b5a90e0fdbd7fbb3bd1d4dc Mon Sep 17 00:00:00 2001 From: Rob Selway Date: Sun, 14 Feb 2021 13:41:38 +0000 Subject: [PATCH 2/4] Plan test for ignoring case when comparing targets --- plan/plan_test.go | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/plan/plan_test.go b/plan/plan_test.go index 5ea0f55ab..992263cd4 100644 --- a/plan/plan_test.go +++ b/plan/plan_test.go @@ -30,6 +30,7 @@ type PlanTestSuite struct { suite.Suite fooV1Cname *endpoint.Endpoint fooV2Cname *endpoint.Endpoint + fooV2CnameUppercase *endpoint.Endpoint fooV2TXT *endpoint.Endpoint fooV2CnameNoLabel *endpoint.Endpoint fooV3CnameSameResource *endpoint.Endpoint @@ -77,6 +78,14 @@ func (suite *PlanTestSuite) SetupTest() { endpoint.ResourceLabelKey: "ingress/default/foo-v2", }, } + suite.fooV2CnameUppercase = &endpoint.Endpoint{ + DNSName: "foo", + Targets: endpoint.Targets{"V2"}, + RecordType: "CNAME", + Labels: map[string]string{ + endpoint.ResourceLabelKey: "ingress/default/foo-v2", + }, + } suite.fooV2TXT = &endpoint.Endpoint{ DNSName: "foo", RecordType: "TXT", @@ -443,6 +452,27 @@ func (suite *PlanTestSuite) TestIgnoreTXT() { validateEntries(suite.T(), changes.Delete, expectedDelete) } +func (suite *PlanTestSuite) TestIgnoreTargetCase() { + current := []*endpoint.Endpoint{suite.fooV2Cname} + desired := []*endpoint.Endpoint{suite.fooV2CnameUppercase} + expectedCreate := []*endpoint.Endpoint{} + expectedUpdateOld := []*endpoint.Endpoint{} + expectedUpdateNew := []*endpoint.Endpoint{} + expectedDelete := []*endpoint.Endpoint{} + + p := &Plan{ + Policies: []Policy{&SyncPolicy{}}, + Current: current, + Desired: desired, + } + + changes := p.Calculate().Changes + validateEntries(suite.T(), changes.Create, expectedCreate) + validateEntries(suite.T(), changes.UpdateNew, expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld, expectedUpdateOld) + validateEntries(suite.T(), changes.Delete, expectedDelete) +} + func (suite *PlanTestSuite) TestRemoveEndpoint() { current := []*endpoint.Endpoint{suite.fooV1Cname, suite.bar192A} desired := []*endpoint.Endpoint{suite.fooV1Cname} From 261779c468c9600088c15290dfa20bfa1fdcb5fc Mon Sep 17 00:00:00 2001 From: Rob Selway Date: Sun, 14 Feb 2021 13:44:36 +0000 Subject: [PATCH 3/4] Cloudflare fix for deletion of entry with casing difference --- provider/cloudflare/cloudflare_test.go | 46 ++++++++++++++++++++++++++ provider/provider.go | 4 +-- provider/provider_test.go | 9 +++++ 3 files changed, 57 insertions(+), 2 deletions(-) diff --git a/provider/cloudflare/cloudflare_test.go b/provider/cloudflare/cloudflare_test.go index 440cc8b65..b6bfc3b0d 100644 --- a/provider/cloudflare/cloudflare_test.go +++ b/provider/cloudflare/cloudflare_test.go @@ -693,6 +693,52 @@ func TestCloudflareApplyChanges(t *testing.T) { } } +func TestCloudflareApplyChangesUpsertOnly(t *testing.T) { + changes := &plan.Changes{} + client := NewMockCloudFlareClient() + provider := &CloudFlareProvider{ + Client: client, + } + changes.Create = []*endpoint.Endpoint{} + changes.Delete = []*endpoint.Endpoint{} + changes.UpdateOld = []*endpoint.Endpoint{{ + DNSName: "foobar.bar.com", + Targets: endpoint.Targets{"target-foo"}, + }} + changes.UpdateNew = []*endpoint.Endpoint{{ + DNSName: "new.bar.com", + Targets: endpoint.Targets{"target-new"}, + }} + err := provider.ApplyChanges(context.Background(), changes) + + if err != nil { + t.Errorf("should not fail, %s", err) + } + + td.Cmp(t, client.Actions, []MockAction{ + { + Name: "Create", + ZoneId: "001", + RecordData: cloudflare.DNSRecord{ + Name: "new.bar.com", + Content: "target-new", + TTL: 1, + }, + }, + }) + + // empty changes + changes.Create = []*endpoint.Endpoint{} + changes.Delete = []*endpoint.Endpoint{} + changes.UpdateOld = []*endpoint.Endpoint{} + changes.UpdateNew = []*endpoint.Endpoint{} + + err = provider.ApplyChanges(context.Background(), changes) + if err != nil { + t.Errorf("should not fail, %s", err) + } +} + func TestCloudflareGetRecordID(t *testing.T) { p := &CloudFlareProvider{} records := []cloudflare.DNSRecord{ diff --git a/provider/provider.go b/provider/provider.go index c0c66cff0..648e888a5 100644 --- a/provider/provider.go +++ b/provider/provider.go @@ -73,9 +73,9 @@ func Difference(current, desired []string) ([]string, []string, []string) { index[x] = struct{}{} } for _, x := range desired { - if _, found := index[x]; found { + if _, found := index[strings.ToLower(x)]; found { leave = append(leave, x) - delete(index, x) + delete(index, strings.ToLower(x)) } else { add = append(add, x) delete(index, x) diff --git a/provider/provider_test.go b/provider/provider_test.go index 0e24ca4f0..b6c3d7f52 100644 --- a/provider/provider_test.go +++ b/provider/provider_test.go @@ -55,6 +55,15 @@ func TestDifference(t *testing.T) { assert.Equal(t, leave, []string{"bar"}) } +func TestDifferenceCaseInsensitivity(t *testing.T) { + current := []string{"foo", "bar"} + desired := []string{"BAR", "baz"} + add, remove, leave := Difference(current, desired) + assert.Equal(t, []string{"baz"}, add) + assert.Equal(t, []string{"foo"}, remove) + assert.Equal(t, []string{"BAR"}, leave) +} + func TestBaseProviderPropertyEquality(t *testing.T) { p := BaseProvider{} assert.True(t, p.PropertyValuesEqual("some.property", "", ""), "Both properties not present") From 642744abacc337e7f4b950eebe19ffc08510f003 Mon Sep 17 00:00:00 2001 From: Rob Selway Date: Sun, 14 Feb 2021 14:52:13 +0000 Subject: [PATCH 4/4] Revert "Cloudflare fix for deletion of entry with casing difference" This reverts commit 261779c468c9600088c15290dfa20bfa1fdcb5fc. --- provider/cloudflare/cloudflare_test.go | 46 -------------------------- provider/provider.go | 4 +-- provider/provider_test.go | 9 ----- 3 files changed, 2 insertions(+), 57 deletions(-) diff --git a/provider/cloudflare/cloudflare_test.go b/provider/cloudflare/cloudflare_test.go index 9295d6828..fae840b2b 100644 --- a/provider/cloudflare/cloudflare_test.go +++ b/provider/cloudflare/cloudflare_test.go @@ -709,52 +709,6 @@ func TestCloudflareApplyChanges(t *testing.T) { } } -func TestCloudflareApplyChangesUpsertOnly(t *testing.T) { - changes := &plan.Changes{} - client := NewMockCloudFlareClient() - provider := &CloudFlareProvider{ - Client: client, - } - changes.Create = []*endpoint.Endpoint{} - changes.Delete = []*endpoint.Endpoint{} - changes.UpdateOld = []*endpoint.Endpoint{{ - DNSName: "foobar.bar.com", - Targets: endpoint.Targets{"target-foo"}, - }} - changes.UpdateNew = []*endpoint.Endpoint{{ - DNSName: "new.bar.com", - Targets: endpoint.Targets{"target-new"}, - }} - err := provider.ApplyChanges(context.Background(), changes) - - if err != nil { - t.Errorf("should not fail, %s", err) - } - - td.Cmp(t, client.Actions, []MockAction{ - { - Name: "Create", - ZoneId: "001", - RecordData: cloudflare.DNSRecord{ - Name: "new.bar.com", - Content: "target-new", - TTL: 1, - }, - }, - }) - - // empty changes - changes.Create = []*endpoint.Endpoint{} - changes.Delete = []*endpoint.Endpoint{} - changes.UpdateOld = []*endpoint.Endpoint{} - changes.UpdateNew = []*endpoint.Endpoint{} - - err = provider.ApplyChanges(context.Background(), changes) - if err != nil { - t.Errorf("should not fail, %s", err) - } -} - func TestCloudflareGetRecordID(t *testing.T) { p := &CloudFlareProvider{} records := []cloudflare.DNSRecord{ diff --git a/provider/provider.go b/provider/provider.go index 648e888a5..c0c66cff0 100644 --- a/provider/provider.go +++ b/provider/provider.go @@ -73,9 +73,9 @@ func Difference(current, desired []string) ([]string, []string, []string) { index[x] = struct{}{} } for _, x := range desired { - if _, found := index[strings.ToLower(x)]; found { + if _, found := index[x]; found { leave = append(leave, x) - delete(index, strings.ToLower(x)) + delete(index, x) } else { add = append(add, x) delete(index, x) diff --git a/provider/provider_test.go b/provider/provider_test.go index b6c3d7f52..0e24ca4f0 100644 --- a/provider/provider_test.go +++ b/provider/provider_test.go @@ -55,15 +55,6 @@ func TestDifference(t *testing.T) { assert.Equal(t, leave, []string{"bar"}) } -func TestDifferenceCaseInsensitivity(t *testing.T) { - current := []string{"foo", "bar"} - desired := []string{"BAR", "baz"} - add, remove, leave := Difference(current, desired) - assert.Equal(t, []string{"baz"}, add) - assert.Equal(t, []string{"foo"}, remove) - assert.Equal(t, []string{"BAR"}, leave) -} - func TestBaseProviderPropertyEquality(t *testing.T) { p := BaseProvider{} assert.True(t, p.PropertyValuesEqual("some.property", "", ""), "Both properties not present")