Remove PropertyValuesEqual method from Provider interface

controller/controller.go

@@ -217,12 +217,11 @@ func (c *Controller) RunOnce(ctx context.Context) error {
 	endpoints = c.Registry.AdjustEndpoints(endpoints)
 
 	plan := &plan.Plan{
-		Policies:           []plan.Policy{c.Policy},
-		Current:            records,
-		Desired:            endpoints,
-		DomainFilter:       endpoint.MatchAllDomainFilters{c.DomainFilter, c.Registry.GetDomainFilter()},
-		PropertyComparator: c.Registry.PropertyValuesEqual,
-		ManagedRecords:     c.ManagedRecordTypes,
+		Policies:       []plan.Policy{c.Policy},
+		Current:        records,
+		Desired:        endpoints,
+		DomainFilter:   endpoint.MatchAllDomainFilters{c.DomainFilter, c.Registry.GetDomainFilter()},
+		ManagedRecords: c.ManagedRecordTypes,
 	}
 
 	plan = plan.Calculate()

plan/plan.go

@@ -44,8 +44,6 @@ type Plan struct {
 	Changes *Changes
 	// DomainFilter matches DNS names
 	DomainFilter endpoint.DomainFilterInterface
-	// Property comparator compares custom properties of providers
-	PropertyComparator PropertyComparator
 	// DNS record types that will be considered for management
 	ManagedRecords []string
 }
@@ -217,21 +215,9 @@ func (p *Plan) shouldUpdateProviderSpecific(desired, current *endpoint.Endpoint)
 	if current.ProviderSpecific != nil {
 		for _, c := range current.ProviderSpecific {
 			if d, ok := desiredProperties[c.Name]; ok {
-				if p.PropertyComparator != nil {
-					if !p.PropertyComparator(c.Name, c.Value, d.Value) {
-						return true
-					}
-				} else if c.Value != d.Value {
-					return true
-				}
+				return c.Value != d.Value
 			} else {
-				if p.PropertyComparator != nil {
-					if !p.PropertyComparator(c.Name, c.Value, "") {
-						return true
-					}
-				} else if c.Value != "" {
-					return true
-				}
+				return c.Value != ""
 			}
 		}
 	}

plan/plan_test.go

@@ -342,21 +342,18 @@ func (suite *PlanTestSuite) TestSyncSecondRoundWithProviderSpecificChange() {
 	validateEntries(suite.T(), changes.Delete, expectedDelete)
 }
 
-func (suite *PlanTestSuite) TestSyncSecondRoundWithProviderSpecificDefaultFalse() {
+func (suite *PlanTestSuite) TestSyncSecondRoundWithProviderSpecificRemoval() {
 	current := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificFalse}
 	desired := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificUnset}
 	expectedCreate := []*endpoint.Endpoint{}
-	expectedUpdateOld := []*endpoint.Endpoint{}
-	expectedUpdateNew := []*endpoint.Endpoint{}
+	expectedUpdateOld := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificFalse}
+	expectedUpdateNew := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificUnset}
 	expectedDelete := []*endpoint.Endpoint{}
 
 	p := &Plan{
-		Policies: []Policy{&SyncPolicy{}},
-		Current:  current,
-		Desired:  desired,
-		PropertyComparator: func(name, previous, current string) bool {
-			return CompareBoolean(false, name, previous, current)
-		},
+		Policies:       []Policy{&SyncPolicy{}},
+		Current:        current,
+		Desired:        desired,
 		ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME},
 	}
 
@@ -367,29 +364,28 @@ func (suite *PlanTestSuite) TestSyncSecondRoundWithProviderSpecificDefaultFalse(
 	validateEntries(suite.T(), changes.Delete, expectedDelete)
 }
 
-func (suite *PlanTestSuite) TestSyncSecondRoundWithProviderSpecificDefualtTrue() {
-	current := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificTrue}
-	desired := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificUnset}
-	expectedCreate := []*endpoint.Endpoint{}
-	expectedUpdateOld := []*endpoint.Endpoint{}
-	expectedUpdateNew := []*endpoint.Endpoint{}
-	expectedDelete := []*endpoint.Endpoint{}
-
-	p := &Plan{
-		Policies: []Policy{&SyncPolicy{}},
-		Current:  current,
-		Desired:  desired,
-		PropertyComparator: func(name, previous, current string) bool {
-			return CompareBoolean(true, name, previous, current)
-		},
-	}
-
-	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)
-}
+// todo: this is currently an expect-fail
+//func (suite *PlanTestSuite) TestSyncSecondRoundWithProviderSpecificAddition() {
+//	current := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificUnset}
+//	desired := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificTrue}
+//	expectedCreate := []*endpoint.Endpoint{}
+//	expectedUpdateOld := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificUnset}
+//	expectedUpdateNew := []*endpoint.Endpoint{suite.bar127AWithProviderSpecificTrue}
+//	expectedDelete := []*endpoint.Endpoint{}
+//
+//	p := &Plan{
+//		Policies:       []Policy{&SyncPolicy{}},
+//		Current:        current,
+//		Desired:        desired,
+//		ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME},
+//	}
+//
+//	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) TestSyncSecondRoundWithOwnerInherited() {
 	current := []*endpoint.Endpoint{suite.fooV1Cname}
@@ -744,15 +740,11 @@ func TestNormalizeDNSName(t *testing.T) {
 }
 
 func TestShouldUpdateProviderSpecific(tt *testing.T) {
-	comparator := func(name, previous, current string) bool {
-		return previous == current
-	}
 	for _, test := range []struct {
-		name               string
-		current            *endpoint.Endpoint
-		desired            *endpoint.Endpoint
-		propertyComparator func(name, previous, current string) bool
-		shouldUpdate       bool
+		name         string
+		current      *endpoint.Endpoint
+		desired      *endpoint.Endpoint
+		shouldUpdate bool
 	}{
 		{
 			name: "skip AWS target health",
@@ -768,8 +760,7 @@ func TestShouldUpdateProviderSpecific(tt *testing.T) {
 					{Name: "aws/evaluate-target-health", Value: "true"},
 				},
 			},
-			propertyComparator: comparator,
-			shouldUpdate:       false,
+			shouldUpdate: false,
 		},
 		{
 			name: "custom property unchanged",
@@ -783,55 +774,10 @@ func TestShouldUpdateProviderSpecific(tt *testing.T) {
 					{Name: "custom/property", Value: "true"},
 				},
 			},
-			propertyComparator: comparator,
-			shouldUpdate:       false,
-		},
-		{
-			name: "custom property value changed",
-			current: &endpoint.Endpoint{
-				ProviderSpecific: []endpoint.ProviderSpecificProperty{
-					{Name: "custom/property", Value: "true"},
-				},
-			},
-			desired: &endpoint.Endpoint{
-				ProviderSpecific: []endpoint.ProviderSpecificProperty{
-					{Name: "custom/property", Value: "false"},
-				},
-			},
-			propertyComparator: comparator,
-			shouldUpdate:       true,
-		},
-		{
-			name: "custom property key changed",
-			current: &endpoint.Endpoint{
-				ProviderSpecific: []endpoint.ProviderSpecificProperty{
-					{Name: "custom/property", Value: "true"},
-				},
-			},
-			desired: &endpoint.Endpoint{
-				ProviderSpecific: []endpoint.ProviderSpecificProperty{
-					{Name: "new/property", Value: "true"},
-				},
-			},
-			propertyComparator: comparator,
-			shouldUpdate:       true,
-		},
-		{
-			name: "desired has same key and value as current but not comparator is set",
-			current: &endpoint.Endpoint{
-				ProviderSpecific: []endpoint.ProviderSpecificProperty{
-					{Name: "custom/property", Value: "true"},
-				},
-			},
-			desired: &endpoint.Endpoint{
-				ProviderSpecific: []endpoint.ProviderSpecificProperty{
-					{Name: "custom/property", Value: "true"},
-				},
-			},
 			shouldUpdate: false,
 		},
 		{
-			name: "desired has same key and different value as current but not comparator is set",
+			name: "custom property value changed",
 			current: &endpoint.Endpoint{
 				ProviderSpecific: []endpoint.ProviderSpecificProperty{
 					{Name: "custom/property", Value: "true"},
@@ -845,7 +791,7 @@ func TestShouldUpdateProviderSpecific(tt *testing.T) {
 			shouldUpdate: true,
 		},
 		{
-			name: "desired has different key from current but not comparator is set",
+			name: "custom property key changed",
 			current: &endpoint.Endpoint{
 				ProviderSpecific: []endpoint.ProviderSpecificProperty{
 					{Name: "custom/property", Value: "true"},
@@ -861,10 +807,9 @@ func TestShouldUpdateProviderSpecific(tt *testing.T) {
 	} {
 		tt.Run(test.name, func(t *testing.T) {
 			plan := &Plan{
-				Current:            []*endpoint.Endpoint{test.current},
-				Desired:            []*endpoint.Endpoint{test.desired},
-				PropertyComparator: test.propertyComparator,
-				ManagedRecords:     []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME},
+				Current:        []*endpoint.Endpoint{test.current},
+				Desired:        []*endpoint.Endpoint{test.desired},
+				ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME},
 			}
 			b := plan.shouldUpdateProviderSpecific(test.desired, test.current)
 			assert.Equal(t, test.shouldUpdate, b)

provider/cloudflare/cloudflare_test.go

@@ -1150,10 +1150,9 @@ func TestProviderPropertiesIdempotency(t *testing.T) {
 			desired = provider.AdjustEndpoints(desired)
 
 			plan := plan.Plan{
-				Current:            current,
-				Desired:            desired,
-				PropertyComparator: provider.PropertyValuesEqual,
-				ManagedRecords:     []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME},
+				Current:        current,
+				Desired:        desired,
+				ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME},
 			}
 
 			plan = *plan.Calculate()

provider/designate/designate.go

@@ -340,14 +340,6 @@ func (p designateProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, e
 	return result, nil
 }
 
-func (p designateProvider) PropertyValuesEqual(name, previous, current string) bool {
-	if name == designateRecordSetID || name == designateZoneID || name == designateOriginalRecords {
-		return true
-	}
-
-	return previous == current
-}
-
 // temporary structure to hold recordset parameters so that we could aggregate endpoints into recordsets
 type recordSet struct {
 	dnsName     string

provider/plural/plural.go

@@ -83,10 +83,6 @@ func (p *PluralProvider) Records(_ context.Context) (endpoints []*endpoint.Endpo
 	return
 }
 
-func (p *PluralProvider) PropertyValuesEqual(name, previous, current string) bool {
-	return p.BaseProvider.PropertyValuesEqual(name, previous, current)
-}
-
 func (p *PluralProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint {
 	return endpoints
 }

provider/provider.go

@@ -29,7 +29,6 @@ import (
 type Provider interface {
 	Records(ctx context.Context) ([]*endpoint.Endpoint, error)
 	ApplyChanges(ctx context.Context, changes *plan.Changes) error
-	PropertyValuesEqual(name string, previous string, current string) bool
 	// AdjustEndpoints canonicalizes a set of candidate endpoints.
 	// It is called with a set of candidate endpoints obtained from the various sources.
 	// It returns a set modified as required by the provider. The provider is responsible for
@@ -47,10 +46,6 @@ func (b BaseProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoin
 	return endpoints
 }
 
-func (b BaseProvider) PropertyValuesEqual(name, previous, current string) bool {
-	return previous == current
-}
-
 func (b BaseProvider) GetDomainFilter() endpoint.DomainFilter {
 	return endpoint.DomainFilter{}
 }

provider/provider_test.go

@@ -54,12 +54,3 @@ func TestDifference(t *testing.T) {
 	assert.Equal(t, remove, []string{"foo"})
 	assert.Equal(t, leave, []string{"bar"})
 }
-
-func TestBaseProviderPropertyEquality(t *testing.T) {
-	p := BaseProvider{}
-	assert.True(t, p.PropertyValuesEqual("some.property", "", ""), "Both properties not present")
-	assert.False(t, p.PropertyValuesEqual("some.property", "", "Foo"), "First property missing")
-	assert.False(t, p.PropertyValuesEqual("some.property", "Foo", ""), "Second property missing")
-	assert.True(t, p.PropertyValuesEqual("some.property", "Foo", "Foo"), "Properties the same")
-	assert.False(t, p.PropertyValuesEqual("some.property", "Foo", "Bar"), "Attributes differ")
-}

registry/aws_sd_registry.go

@@ -95,10 +95,6 @@ func (sdr *AWSSDRegistry) updateLabels(endpoints []*endpoint.Endpoint) {
 	}
 }
 
-func (sdr *AWSSDRegistry) PropertyValuesEqual(name string, previous string, current string) bool {
-	return sdr.provider.PropertyValuesEqual(name, previous, current)
-}
-
 // AdjustEndpoints modifies the endpoints as needed by the specific provider
 func (sdr *AWSSDRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint {
 	return sdr.provider.AdjustEndpoints(endpoints)

registry/dynamodb.go

@@ -248,11 +248,6 @@ func (im *DynamoDBRegistry) ApplyChanges(ctx context.Context, changes *plan.Chan
 	})
 }
 
-// PropertyValuesEqual compares two attribute values for equality.
-func (im *DynamoDBRegistry) PropertyValuesEqual(name string, previous string, current string) bool {
-	return im.provider.PropertyValuesEqual(name, previous, current)
-}
-
 // AdjustEndpoints modifies the endpoints as needed by the specific provider.
 func (im *DynamoDBRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint {
 	return im.provider.AdjustEndpoints(endpoints)

registry/noop.go

@@ -50,11 +50,6 @@ func (im *NoopRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes)
 	return im.provider.ApplyChanges(ctx, changes)
 }
 
-// PropertyValuesEqual compares two property values for equality
-func (im *NoopRegistry) PropertyValuesEqual(attribute string, previous string, current string) bool {
-	return im.provider.PropertyValuesEqual(attribute, previous, current)
-}
-
 // AdjustEndpoints modifies the endpoints as needed by the specific provider
 func (im *NoopRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint {
 	return im.provider.AdjustEndpoints(endpoints)

registry/registry.go

@@ -32,7 +32,6 @@ import (
 type Registry interface {
 	Records(ctx context.Context) ([]*endpoint.Endpoint, error)
 	ApplyChanges(ctx context.Context, changes *plan.Changes) error
-	PropertyValuesEqual(attribute string, previous string, current string) bool
 	AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint
 	GetDomainFilter() endpoint.DomainFilter
 }

registry/txt.go

@@ -284,11 +284,6 @@ func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes)
 	return im.provider.ApplyChanges(ctx, filteredChanges)
 }
 
-// PropertyValuesEqual compares two attribute values for equality
-func (im *TXTRegistry) PropertyValuesEqual(name string, previous string, current string) bool {
-	return im.provider.PropertyValuesEqual(name, previous, current)
-}
-
 // AdjustEndpoints modifies the endpoints as needed by the specific provider
 func (im *TXTRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint {
 	return im.provider.AdjustEndpoints(endpoints)