move shouldTTLUpdate from plan to provider

controller/controller.go

@@ -140,11 +140,12 @@ func (c *Controller) RunOnce(ctx context.Context) error {
 	sourceEndpointsTotal.Set(float64(len(endpoints)))
 
 	plan := &plan.Plan{
-		Policies:           []plan.Policy{c.Policy},
-		Current:            records,
-		Desired:            endpoints,
-		DomainFilter:       c.DomainFilter,
-		PropertyComparator: c.Registry.PropertyValuesEqual,
+		Policies:              []plan.Policy{c.Policy},
+		Current:               records,
+		Desired:               endpoints,
+		DomainFilter:          c.DomainFilter,
+		PropertyComparator:    c.Registry.PropertyValuesEqual,
+		EndpointTTLComparator: c.Registry.ShouldUpdateTTL,
 	}
 
 	plan = plan.Calculate()

plan/plan.go

@@ -26,6 +26,7 @@ import (
 
 // PropertyComparator is used in Plan for comparing the previous and current custom annotations.
 type PropertyComparator func(name string, previous string, current string) bool
+type EndpointTTLComparator func(desired, current *endpoint.Endpoint) bool
 
 // Plan can convert a list of desired and current records to a series of create,
 // update and delete actions.
@@ -43,6 +44,8 @@ type Plan struct {
 	DomainFilter endpoint.DomainFilter
 	// Property comparator compares custom properties of providers
 	PropertyComparator PropertyComparator
+	// Endpoint TTL comparator compares provider-custom logic for endpoints ttl
+	EndpointTTLComparator EndpointTTLComparator
 }
 
 // Changes holds lists of actions to be executed by dns providers
@@ -141,7 +144,7 @@ func (p *Plan) Calculate() *Plan {
 			if row.current != nil && len(row.candidates) > 0 { //dns name is taken
 				update := t.resolver.ResolveUpdate(row.current, row.candidates)
 				// compare "update" to "current" to figure out if actual update is required
-				if shouldUpdateTTL(update, row.current) || targetChanged(update, row.current) || p.shouldUpdateProviderSpecific(update, row.current) {
+				if p.EndpointTTLComparator(update, row.current) || targetChanged(update, row.current) || p.shouldUpdateProviderSpecific(update, row.current) {
 					inheritOwner(row.current, update)
 					changes.UpdateNew = append(changes.UpdateNew, update)
 					changes.UpdateOld = append(changes.UpdateOld, row.current)
@@ -177,13 +180,6 @@ func targetChanged(desired, current *endpoint.Endpoint) bool {
 	return !desired.Targets.Same(current.Targets)
 }
 
-func shouldUpdateTTL(desired, current *endpoint.Endpoint) bool {
-	if !desired.RecordTTL.IsConfigured() {
-		return false
-	}
-	return desired.RecordTTL != current.RecordTTL
-}
-
 func (p *Plan) shouldUpdateProviderSpecific(desired, current *endpoint.Endpoint) bool {
 	desiredProperties := map[string]endpoint.ProviderSpecificProperty{}
 

plan/plan_test.go

@@ -205,9 +205,10 @@ func (suite *PlanTestSuite) TestSyncFirstRound() {
 	expectedDelete := []*endpoint.Endpoint{}
 
 	p := &Plan{
-		Policies: []Policy{&SyncPolicy{}},
-		Current:  current,
-		Desired:  desired,
+		Policies:              []Policy{&SyncPolicy{}},
+		Current:               current,
+		Desired:               desired,
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -226,9 +227,10 @@ func (suite *PlanTestSuite) TestSyncSecondRound() {
 	expectedDelete := []*endpoint.Endpoint{}
 
 	p := &Plan{
-		Policies: []Policy{&SyncPolicy{}},
-		Current:  current,
-		Desired:  desired,
+		Policies:              []Policy{&SyncPolicy{}},
+		Current:               current,
+		Desired:               desired,
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -247,9 +249,10 @@ func (suite *PlanTestSuite) TestSyncSecondRoundMigration() {
 	expectedDelete := []*endpoint.Endpoint{}
 
 	p := &Plan{
-		Policies: []Policy{&SyncPolicy{}},
-		Current:  current,
-		Desired:  desired,
+		Policies:              []Policy{&SyncPolicy{}},
+		Current:               current,
+		Desired:               desired,
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -268,9 +271,10 @@ func (suite *PlanTestSuite) TestSyncSecondRoundWithTTLChange() {
 	expectedDelete := []*endpoint.Endpoint{}
 
 	p := &Plan{
-		Policies: []Policy{&SyncPolicy{}},
-		Current:  current,
-		Desired:  desired,
+		Policies:              []Policy{&SyncPolicy{}},
+		Current:               current,
+		Desired:               desired,
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -289,9 +293,10 @@ func (suite *PlanTestSuite) TestSyncSecondRoundWithProviderSpecificChange() {
 	expectedDelete := []*endpoint.Endpoint{}
 
 	p := &Plan{
-		Policies: []Policy{&SyncPolicy{}},
-		Current:  current,
-		Desired:  desired,
+		Policies:              []Policy{&SyncPolicy{}},
+		Current:               current,
+		Desired:               desired,
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -316,6 +321,7 @@ func (suite *PlanTestSuite) TestSyncSecondRoundWithProviderSpecificDefaultFalse(
 		PropertyComparator: func(name, previous, current string) bool {
 			return CompareBoolean(false, name, previous, current)
 		},
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -340,6 +346,7 @@ func (suite *PlanTestSuite) TestSyncSecondRoundWithProviderSpecificDefualtTrue()
 		PropertyComparator: func(name, previous, current string) bool {
 			return CompareBoolean(true, name, previous, current)
 		},
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -368,9 +375,10 @@ func (suite *PlanTestSuite) TestSyncSecondRoundWithOwnerInherited() {
 	expectedDelete := []*endpoint.Endpoint{}
 
 	p := &Plan{
-		Policies: []Policy{&SyncPolicy{}},
-		Current:  current,
-		Desired:  desired,
+		Policies:              []Policy{&SyncPolicy{}},
+		Current:               current,
+		Desired:               desired,
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -389,9 +397,10 @@ func (suite *PlanTestSuite) TestIdempotency() {
 	expectedDelete := []*endpoint.Endpoint{}
 
 	p := &Plan{
-		Policies: []Policy{&SyncPolicy{}},
-		Current:  current,
-		Desired:  desired,
+		Policies:              []Policy{&SyncPolicy{}},
+		Current:               current,
+		Desired:               desired,
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -410,9 +419,10 @@ func (suite *PlanTestSuite) TestDifferentTypes() {
 	expectedDelete := []*endpoint.Endpoint{}
 
 	p := &Plan{
-		Policies: []Policy{&SyncPolicy{}},
-		Current:  current,
-		Desired:  desired,
+		Policies:              []Policy{&SyncPolicy{}},
+		Current:               current,
+		Desired:               desired,
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -431,9 +441,10 @@ func (suite *PlanTestSuite) TestIgnoreTXT() {
 	expectedDelete := []*endpoint.Endpoint{}
 
 	p := &Plan{
-		Policies: []Policy{&SyncPolicy{}},
-		Current:  current,
-		Desired:  desired,
+		Policies:              []Policy{&SyncPolicy{}},
+		Current:               current,
+		Desired:               desired,
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -452,9 +463,10 @@ func (suite *PlanTestSuite) TestRemoveEndpoint() {
 	expectedDelete := []*endpoint.Endpoint{suite.bar192A}
 
 	p := &Plan{
-		Policies: []Policy{&SyncPolicy{}},
-		Current:  current,
-		Desired:  desired,
+		Policies:              []Policy{&SyncPolicy{}},
+		Current:               current,
+		Desired:               desired,
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -473,9 +485,10 @@ func (suite *PlanTestSuite) TestRemoveEndpointWithUpsert() {
 	expectedDelete := []*endpoint.Endpoint{}
 
 	p := &Plan{
-		Policies: []Policy{&UpsertOnlyPolicy{}},
-		Current:  current,
-		Desired:  desired,
+		Policies:              []Policy{&UpsertOnlyPolicy{}},
+		Current:               current,
+		Desired:               desired,
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -495,9 +508,10 @@ func (suite *PlanTestSuite) TestDuplicatedEndpointsForSameResourceReplace() {
 	expectedDelete := []*endpoint.Endpoint{suite.bar192A}
 
 	p := &Plan{
-		Policies: []Policy{&SyncPolicy{}},
-		Current:  current,
-		Desired:  desired,
+		Policies:              []Policy{&SyncPolicy{}},
+		Current:               current,
+		Desired:               desired,
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -518,9 +532,10 @@ func (suite *PlanTestSuite) TestDuplicatedEndpointsForSameResourceRetain() {
 	expectedDelete := []*endpoint.Endpoint{suite.bar192A}
 
 	p := &Plan{
-		Policies: []Policy{&SyncPolicy{}},
-		Current:  current,
-		Desired:  desired,
+		Policies:              []Policy{&SyncPolicy{}},
+		Current:               current,
+		Desired:               desired,
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -540,9 +555,10 @@ func (suite *PlanTestSuite) TestMultipleRecordsSameNameDifferentSetIdentifier()
 	expectedDelete := []*endpoint.Endpoint{}
 
 	p := &Plan{
-		Policies: []Policy{&SyncPolicy{}},
-		Current:  current,
-		Desired:  desired,
+		Policies:              []Policy{&SyncPolicy{}},
+		Current:               current,
+		Desired:               desired,
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -562,9 +578,10 @@ func (suite *PlanTestSuite) TestSetIdentifierUpdateCreatesAndDeletes() {
 	expectedDelete := []*endpoint.Endpoint{suite.multiple2}
 
 	p := &Plan{
-		Policies: []Policy{&SyncPolicy{}},
-		Current:  current,
-		Desired:  desired,
+		Policies:              []Policy{&SyncPolicy{}},
+		Current:               current,
+		Desired:               desired,
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -584,10 +601,11 @@ func (suite *PlanTestSuite) TestDomainFiltersInitial() {
 	expectedDelete := []*endpoint.Endpoint{}
 
 	p := &Plan{
-		Policies:     []Policy{&SyncPolicy{}},
-		Current:      current,
-		Desired:      desired,
-		DomainFilter: endpoint.NewDomainFilterWithExclusions([]string{"domain.tld"}, []string{"ex.domain.tld"}),
+		Policies:              []Policy{&SyncPolicy{}},
+		Current:               current,
+		Desired:               desired,
+		DomainFilter:          endpoint.NewDomainFilterWithExclusions([]string{"domain.tld"}, []string{"ex.domain.tld"}),
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -607,10 +625,11 @@ func (suite *PlanTestSuite) TestDomainFiltersUpdate() {
 	expectedDelete := []*endpoint.Endpoint{}
 
 	p := &Plan{
-		Policies:     []Policy{&SyncPolicy{}},
-		Current:      current,
-		Desired:      desired,
-		DomainFilter: endpoint.NewDomainFilterWithExclusions([]string{"domain.tld"}, []string{"ex.domain.tld"}),
+		Policies:              []Policy{&SyncPolicy{}},
+		Current:               current,
+		Desired:               desired,
+		DomainFilter:          endpoint.NewDomainFilterWithExclusions([]string{"domain.tld"}, []string{"ex.domain.tld"}),
+		EndpointTTLComparator: endpointTTLComparator,
 	}
 
 	changes := p.Calculate().Changes
@@ -686,3 +705,10 @@ func TestNormalizeDNSName(t *testing.T) {
 		assert.Equal(t, r.expect, gotName)
 	}
 }
+
+func endpointTTLComparator(desired, current *endpoint.Endpoint) bool {
+	if !desired.RecordTTL.IsConfigured() {
+		return false
+	}
+	return desired.RecordTTL != current.RecordTTL
+}

provider/cloudflare/cloudflare.go

@@ -266,6 +266,15 @@ func (p *CloudFlareProvider) PropertyValuesEqual(name string, previous string, c
 	return p.BaseProvider.PropertyValuesEqual(name, previous, current)
 }
 
+func (p *CloudFlareProvider) ShouldUpdateTTL(desired, current *endpoint.Endpoint) bool {
+	proxied := shouldBeProxied(desired, p.proxiedByDefault)
+	if proxied {
+		return false
+	}
+
+	return p.BaseProvider.ShouldUpdateTTL(desired, current)
+}
+
 // submitChanges takes a zone and a collection of Changes and sends them as a single transaction.
 func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloudFlareChange) error {
 	// return early if there is nothing to change

provider/cloudflare/cloudflare_test.go

@@ -1035,9 +1035,10 @@ func TestProviderPropertiesIdempotency(t *testing.T) {
 		}
 
 		plan := plan.Plan{
-			Current:            current,
-			Desired:            desired,
-			PropertyComparator: provider.PropertyValuesEqual,
+			Current:               current,
+			Desired:               desired,
+			PropertyComparator:    provider.PropertyValuesEqual,
+			EndpointTTLComparator: provider.ShouldUpdateTTL,
 		}
 
 		plan = *plan.Calculate()
@@ -1091,7 +1092,8 @@ func TestCloudflareComplexUpdate(t *testing.T) {
 				},
 			},
 		},
-		DomainFilter: endpoint.NewDomainFilter([]string{"bar.com"}),
+		DomainFilter:          endpoint.NewDomainFilter([]string{"bar.com"}),
+		EndpointTTLComparator: provider.ShouldUpdateTTL,
 	}
 
 	planned := plan.Calculate()
@@ -1133,3 +1135,57 @@ func TestCloudflareComplexUpdate(t *testing.T) {
 		},
 	})
 }
+
+func TestCustomTTLWithEnabeledProxyNotChanged(t *testing.T) {
+	client := NewMockCloudFlareClientWithRecords(map[string][]cloudflare.DNSRecord{
+		"001": []cloudflare.DNSRecord{
+			{
+				ID:      "1234567890",
+				ZoneID:  "001",
+				Name:    "foobar.bar.com",
+				Type:    endpoint.RecordTypeA,
+				TTL:     1,
+				Content: "1.2.3.4",
+				Proxied: true,
+			},
+		},
+	})
+
+	provider := &CloudFlareProvider{
+		Client: client,
+	}
+
+	records, err := provider.Records(context.Background())
+
+	if err != nil {
+		t.Errorf("should not fail, %s", err)
+	}
+
+	plan := &plan.Plan{
+		Current: records,
+		Desired: []*endpoint.Endpoint{
+			{
+				DNSName:    "foobar.bar.com",
+				Targets:    endpoint.Targets{"1.2.3.4"},
+				RecordType: endpoint.RecordTypeA,
+				RecordTTL:  300,
+				Labels:     endpoint.Labels{},
+				ProviderSpecific: endpoint.ProviderSpecific{
+					{
+						Name:  "external-dns.alpha.kubernetes.io/cloudflare-proxied",
+						Value: "true",
+					},
+				},
+			},
+		},
+		DomainFilter:          endpoint.NewDomainFilter([]string{"bar.com"}),
+		EndpointTTLComparator: provider.ShouldUpdateTTL,
+	}
+
+	planned := plan.Calculate()
+
+	assert.Equal(t, 0, len(planned.Changes.Create), "no new changes should be here")
+	assert.Equal(t, 0, len(planned.Changes.UpdateNew), "no new changes should be here")
+	assert.Equal(t, 0, len(planned.Changes.UpdateOld), "no new changes should be here")
+	assert.Equal(t, 0, len(planned.Changes.Delete), "no new changes should be here")
+}

provider/provider.go

@@ -30,6 +30,7 @@ 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
+	ShouldUpdateTTL(desired, current *endpoint.Endpoint) bool
 }
 
 type BaseProvider struct {
@@ -39,6 +40,13 @@ func (b BaseProvider) PropertyValuesEqual(name, previous, current string) bool {
 	return previous == current
 }
 
+func (b BaseProvider) ShouldUpdateTTL(desired, current *endpoint.Endpoint) bool {
+	if !desired.RecordTTL.IsConfigured() {
+		return false
+	}
+	return desired.RecordTTL != current.RecordTTL
+}
+
 type contextKey struct {
 	name string
 }

registry/aws_sd_registry.go

@@ -91,3 +91,7 @@ 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)
 }
+
+func (sdr *AWSSDRegistry) ShouldUpdateTTL(desired, current *endpoint.Endpoint) bool {
+	return sdr.provider.ShouldUpdateTTL(desired, current)
+}

registry/noop.go

@@ -50,3 +50,7 @@ func (im *NoopRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes)
 func (im *NoopRegistry) PropertyValuesEqual(attribute string, previous string, current string) bool {
 	return im.provider.PropertyValuesEqual(attribute, previous, current)
 }
+
+func (im *NoopRegistry) ShouldUpdateTTL(desired, current *endpoint.Endpoint) bool {
+	return im.provider.ShouldUpdateTTL(desired, current)
+}

registry/registry.go

@@ -33,6 +33,7 @@ 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
+	ShouldUpdateTTL(desired, current *endpoint.Endpoint) bool
 }
 
 //TODO(ideahitme): consider moving this to Plan

registry/txt.go

@@ -196,6 +196,10 @@ func (im *TXTRegistry) PropertyValuesEqual(name string, previous string, current
 	return im.provider.PropertyValuesEqual(name, previous, current)
 }
 
+func (im *TXTRegistry) ShouldUpdateTTL(desired, current *endpoint.Endpoint) bool {
+	return im.provider.ShouldUpdateTTL(desired, current)
+}
+
 /**
   TXT registry specific private methods
 */