diff --git a/controller/controller.go b/controller/controller.go index 72058037cb..6058af46aa 100644 --- a/controller/controller.go +++ b/controller/controller.go @@ -18,6 +18,7 @@ package controller import ( "context" + "fmt" "sync" "time" @@ -214,7 +215,10 @@ func (c *Controller) RunOnce(ctx context.Context) error { vARecords, vAAAARecords := countMatchingAddressRecords(endpoints, records) verifiedARecords.Set(float64(vARecords)) verifiedAAAARecords.Set(float64(vAAAARecords)) - endpoints = c.Registry.AdjustEndpoints(endpoints) + endpoints, err = c.Registry.AdjustEndpoints(endpoints) + if err != nil { + return fmt.Errorf("adjusting endpoints: %w", err) + } registryFilter := c.Registry.GetDomainFilter() plan := &plan.Plan{ diff --git a/provider/aws/aws.go b/provider/aws/aws.go index 9d237873f6..aeeccf04eb 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -660,7 +660,7 @@ func (p *AWSProvider) newChanges(action string, endpoints []*endpoint.Endpoint) // unneeded (potentially failing) changes. // Example: CNAME endpoints pointing to ELBs will have a `alias` provider-specific property // added to match the endpoints generated from existing alias records in Route53. -func (p *AWSProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint { +func (p *AWSProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) { for _, ep := range endpoints { alias := false if ep.RecordType != endpoint.RecordTypeCNAME { @@ -692,7 +692,7 @@ func (p *AWSProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoin ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) } } - return endpoints + return endpoints, nil } // newChange returns a route53 Change and a boolean indicating if there should also be a change to a AAAA record diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index 6f82e05bec..d8efb8a85e 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -529,7 +529,8 @@ func TestAWSAdjustEndpoints(t *testing.T) { endpoint.NewEndpoint("cname-test-elb-no-eth.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeCNAME, "foo.eu-central-1.elb.amazonaws.com").WithProviderSpecific(providerSpecificEvaluateTargetHealth, "false"), // eth = evaluate target health } - records = provider.AdjustEndpoints(records) + records, err := provider.AdjustEndpoints(records) + assert.NoError(t, err) validateEndpoints(t, provider, records, []*endpoint.Endpoint{ endpoint.NewEndpoint("a-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "8.8.8.8"), @@ -1610,7 +1611,8 @@ func TestAWSBatchChangeSetExceedingNameChange(t *testing.T) { func validateEndpoints(t *testing.T, provider *AWSProvider, endpoints []*endpoint.Endpoint, expected []*endpoint.Endpoint) { assert.True(t, testutils.SameEndpoints(endpoints, expected), "actual and expected endpoints don't match. %+v:%+v", endpoints, expected) - normalized := provider.AdjustEndpoints(endpoints) + normalized, err := provider.AdjustEndpoints(endpoints) + assert.NoError(t, err) assert.True(t, testutils.SameEndpoints(normalized, expected), "actual and normalized endpoints don't match. %+v:%+v", endpoints, normalized) } diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go index 3c2129c056..349cdd9def 100644 --- a/provider/cloudflare/cloudflare.go +++ b/provider/cloudflare/cloudflare.go @@ -368,7 +368,7 @@ func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloud } // AdjustEndpoints modifies the endpoints as needed by the specific provider -func (p *CloudFlareProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint { +func (p *CloudFlareProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) { adjustedEndpoints := []*endpoint.Endpoint{} for _, e := range endpoints { proxied := shouldBeProxied(e, p.proxiedByDefault) @@ -379,7 +379,7 @@ func (p *CloudFlareProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) []* adjustedEndpoints = append(adjustedEndpoints, e) } - return adjustedEndpoints + return adjustedEndpoints, nil } // changesByZone separates a multi-zone change into a single change per zone. diff --git a/provider/cloudflare/cloudflare_test.go b/provider/cloudflare/cloudflare_test.go index 41453f29bb..e70be6a6ca 100644 --- a/provider/cloudflare/cloudflare_test.go +++ b/provider/cloudflare/cloudflare_test.go @@ -302,7 +302,8 @@ func AssertActions(t *testing.T, provider *CloudFlareProvider, endpoints []*endp t.Fatalf("cannot fetch records, %s", err) } - endpoints = provider.AdjustEndpoints(endpoints) + endpoints, err = provider.AdjustEndpoints(endpoints) + assert.NoError(t, err) domainFilter := endpoint.NewDomainFilter([]string{"bar.com"}) plan := &plan.Plan{ Current: records, @@ -1147,7 +1148,8 @@ func TestProviderPropertiesIdempotency(t *testing.T) { }) } - desired = provider.AdjustEndpoints(desired) + desired, err = provider.AdjustEndpoints(desired) + assert.NoError(t, err) plan := plan.Plan{ Current: current, @@ -1190,23 +1192,25 @@ func TestCloudflareComplexUpdate(t *testing.T) { } domainFilter := endpoint.NewDomainFilter([]string{"bar.com"}) - plan := &plan.Plan{ - Current: records, - Desired: provider.AdjustEndpoints([]*endpoint.Endpoint{ - { - DNSName: "foobar.bar.com", - Targets: endpoint.Targets{"1.2.3.4", "2.3.4.5"}, - RecordType: endpoint.RecordTypeA, - RecordTTL: endpoint.TTL(defaultCloudFlareRecordTTL), - Labels: endpoint.Labels{}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "true", - }, + endpoints, err := provider.AdjustEndpoints([]*endpoint.Endpoint{ + { + DNSName: "foobar.bar.com", + Targets: endpoint.Targets{"1.2.3.4", "2.3.4.5"}, + RecordType: endpoint.RecordTypeA, + RecordTTL: endpoint.TTL(defaultCloudFlareRecordTTL), + Labels: endpoint.Labels{}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", + Value: "true", }, }, - }), + }, + }) + assert.NoError(t, err) + plan := &plan.Plan{ + Current: records, + Desired: endpoints, DomainFilter: endpoint.MatchAllDomainFilters{&domainFilter}, ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME}, } diff --git a/provider/ibmcloud/ibmcloud.go b/provider/ibmcloud/ibmcloud.go index ed9e07f346..f442c76a36 100644 --- a/provider/ibmcloud/ibmcloud.go +++ b/provider/ibmcloud/ibmcloud.go @@ -386,7 +386,7 @@ func (p *IBMCloudProvider) ApplyChanges(ctx context.Context, changes *plan.Chang } // AdjustEndpoints modifies the endpoints as needed by the specific provider -func (p *IBMCloudProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint { +func (p *IBMCloudProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) { adjustedEndpoints := []*endpoint.Endpoint{} for _, e := range endpoints { log.Debugf("adjusting endpont: %v", *e) @@ -398,7 +398,7 @@ func (p *IBMCloudProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*en adjustedEndpoints = append(adjustedEndpoints, e) } - return adjustedEndpoints + return adjustedEndpoints, nil } // submitChanges takes a zone and a collection of Changes and sends them as a single transaction. diff --git a/provider/ibmcloud/ibmcloud_test.go b/provider/ibmcloud/ibmcloud_test.go index 077869b62f..6c94db504c 100644 --- a/provider/ibmcloud/ibmcloud_test.go +++ b/provider/ibmcloud/ibmcloud_test.go @@ -276,33 +276,46 @@ func TestPublic_ApplyChanges(t *testing.T) { func TestPrivate_ApplyChanges(t *testing.T) { p := newTestIBMCloudProvider(true) - changes := plan.Changes{ - Create: p.AdjustEndpoints([]*endpoint.Endpoint{ - { - DNSName: "newA.example.com", - RecordType: "A", - RecordTTL: 120, - Targets: endpoint.NewTargets("4.3.2.1"), - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "ibmcloud-vpc", - Value: "crn:v1:staging:public:is:us-south:a/0821fa9f9ebcc7b7c9a0d6e9bf9442a4::vpc:be33cdad-9a03-4bfa-82ca-eadb9f1de688", - }, + endpointsCreate, err := p.AdjustEndpoints([]*endpoint.Endpoint{ + { + DNSName: "newA.example.com", + RecordType: "A", + RecordTTL: 120, + Targets: endpoint.NewTargets("4.3.2.1"), + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "ibmcloud-vpc", + Value: "crn:v1:staging:public:is:us-south:a/0821fa9f9ebcc7b7c9a0d6e9bf9442a4::vpc:be33cdad-9a03-4bfa-82ca-eadb9f1de688", }, }, - { - DNSName: "newCNAME.example.com", - RecordType: "CNAME", - RecordTTL: 180, - Targets: endpoint.NewTargets("newA.example.com"), - }, - { - DNSName: "newTXT.example.com", - RecordType: "TXT", - RecordTTL: 240, - Targets: endpoint.NewTargets("\"heritage=external-dns,external-dns/owner=tower-pdns\""), - }, - }), + }, + { + DNSName: "newCNAME.example.com", + RecordType: "CNAME", + RecordTTL: 180, + Targets: endpoint.NewTargets("newA.example.com"), + }, + { + DNSName: "newTXT.example.com", + RecordType: "TXT", + RecordTTL: 240, + Targets: endpoint.NewTargets("\"heritage=external-dns,external-dns/owner=tower-pdns\""), + }, + }) + assert.NoError(t, err) + + endpointsUpdate, err := p.AdjustEndpoints([]*endpoint.Endpoint{ + { + DNSName: "test.example.com", + RecordType: "A", + RecordTTL: 180, + Targets: endpoint.NewTargets("1.2.3.4", "5.6.7.8"), + }, + }) + assert.NoError(t, err) + + changes := plan.Changes{ + Create: endpointsCreate, UpdateOld: []*endpoint.Endpoint{ { DNSName: "test.example.com", @@ -311,14 +324,7 @@ func TestPrivate_ApplyChanges(t *testing.T) { Targets: endpoint.NewTargets("1.2.3.4"), }, }, - UpdateNew: p.AdjustEndpoints([]*endpoint.Endpoint{ - { - DNSName: "test.example.com", - RecordType: "A", - RecordTTL: 180, - Targets: endpoint.NewTargets("1.2.3.4", "5.6.7.8"), - }, - }), + UpdateNew: endpointsUpdate, Delete: []*endpoint.Endpoint{ { DNSName: "test.example.com", @@ -329,7 +335,7 @@ func TestPrivate_ApplyChanges(t *testing.T) { }, } ctx := context.Background() - err := p.ApplyChanges(ctx, &changes) + err = p.ApplyChanges(ctx, &changes) if err != nil { t.Errorf("should not fail, %s", err) } @@ -353,7 +359,8 @@ func TestAdjustEndpoints(t *testing.T) { }, } - ep := p.AdjustEndpoints(endpoints) + ep, err := p.AdjustEndpoints(endpoints) + assert.NoError(t, err) assert.Equal(t, endpoint.TTL(0), ep[0].RecordTTL) assert.Equal(t, "test.example.com", ep[0].DNSName) diff --git a/provider/infoblox/infoblox.go b/provider/infoblox/infoblox.go index fa4a0c8c3b..06b18fc918 100644 --- a/provider/infoblox/infoblox.go +++ b/provider/infoblox/infoblox.go @@ -376,14 +376,14 @@ func (p *ProviderConfig) Records(ctx context.Context) (endpoints []*endpoint.End return endpoints, nil } -func (p *ProviderConfig) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint { +func (p *ProviderConfig) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) { // Update user specified TTL (0 == disabled) for i := range endpoints { endpoints[i].RecordTTL = endpoint.TTL(p.cacheDuration) } if !p.createPTR { - return endpoints + return endpoints, nil } // for all A records, we want to create PTR records @@ -403,7 +403,7 @@ func (p *ProviderConfig) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endp } } - return endpoints + return endpoints, nil } // ApplyChanges applies the given changes. diff --git a/provider/plural/plural.go b/provider/plural/plural.go index bcf87f1d5c..a73c29059d 100644 --- a/provider/plural/plural.go +++ b/provider/plural/plural.go @@ -83,8 +83,8 @@ func (p *PluralProvider) Records(_ context.Context) (endpoints []*endpoint.Endpo return } -func (p *PluralProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint { - return endpoints +func (p *PluralProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) { + return endpoints, nil } func (p *PluralProvider) ApplyChanges(_ context.Context, diffs *plan.Changes) error { diff --git a/provider/provider.go b/provider/provider.go index c1414bb538..68492183bf 100644 --- a/provider/provider.go +++ b/provider/provider.go @@ -36,14 +36,14 @@ type Provider interface { // the endpoints that the provider returns in `Records` so that the change plan will not have // unnecessary (potentially failing) changes. It may also modify other fields, add, or remove // Endpoints. It is permitted to modify the supplied endpoints. - AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint + AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) GetDomainFilter() endpoint.DomainFilter } type BaseProvider struct{} -func (b BaseProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint { - return endpoints +func (b BaseProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) { + return endpoints, nil } func (b BaseProvider) GetDomainFilter() endpoint.DomainFilter { diff --git a/provider/scaleway/scaleway.go b/provider/scaleway/scaleway.go index 0aa7606d45..469314a263 100644 --- a/provider/scaleway/scaleway.go +++ b/provider/scaleway/scaleway.go @@ -92,7 +92,7 @@ func NewScalewayProvider(ctx context.Context, domainFilter endpoint.DomainFilter } // AdjustEndpoints is used to normalize the endoints -func (p *ScalewayProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint { +func (p *ScalewayProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) { eps := make([]*endpoint.Endpoint, len(endpoints)) for i := range endpoints { eps[i] = endpoints[i] @@ -103,7 +103,7 @@ func (p *ScalewayProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*en eps[i] = eps[i].WithProviderSpecific(scalewayPriorityKey, fmt.Sprintf("%d", scalewayDefaultPriority)) } } - return eps + return eps, nil } // Zones returns the list of hosted zones. diff --git a/provider/scaleway/scaleway_test.go b/provider/scaleway/scaleway_test.go index a6276a3091..58ec73c964 100644 --- a/provider/scaleway/scaleway_test.go +++ b/provider/scaleway/scaleway_test.go @@ -220,7 +220,8 @@ func TestScalewayProvider_AdjustEndpoints(t *testing.T) { }, } - after := provider.AdjustEndpoints(before) + after, err := provider.AdjustEndpoints(before) + assert.NoError(t, err) for i := range after { if !checkRecordEquality(after[i], expected[i]) { t.Errorf("got record %s instead of %s", after[i], expected[i]) diff --git a/registry/aws_sd_registry.go b/registry/aws_sd_registry.go index bfb82b75ae..8a2b023c4f 100644 --- a/registry/aws_sd_registry.go +++ b/registry/aws_sd_registry.go @@ -100,6 +100,6 @@ func (sdr *AWSSDRegistry) updateLabels(endpoints []*endpoint.Endpoint) { } // AdjustEndpoints modifies the endpoints as needed by the specific provider -func (sdr *AWSSDRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint { +func (sdr *AWSSDRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) { return sdr.provider.AdjustEndpoints(endpoints) } diff --git a/registry/dynamodb.go b/registry/dynamodb.go index 056587e10b..334e228720 100644 --- a/registry/dynamodb.go +++ b/registry/dynamodb.go @@ -337,7 +337,7 @@ func (im *DynamoDBRegistry) ApplyChanges(ctx context.Context, changes *plan.Chan } // AdjustEndpoints modifies the endpoints as needed by the specific provider. -func (im *DynamoDBRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint { +func (im *DynamoDBRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) { return im.provider.AdjustEndpoints(endpoints) } diff --git a/registry/noop.go b/registry/noop.go index b581874059..9e06bbb92d 100644 --- a/registry/noop.go +++ b/registry/noop.go @@ -55,6 +55,6 @@ func (im *NoopRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) } // AdjustEndpoints modifies the endpoints as needed by the specific provider -func (im *NoopRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint { +func (im *NoopRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) { return im.provider.AdjustEndpoints(endpoints) } diff --git a/registry/registry.go b/registry/registry.go index 5b59a27b29..d2955365d0 100644 --- a/registry/registry.go +++ b/registry/registry.go @@ -30,7 +30,7 @@ import ( type Registry interface { Records(ctx context.Context) ([]*endpoint.Endpoint, error) ApplyChanges(ctx context.Context, changes *plan.Changes) error - AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint + AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) GetDomainFilter() endpoint.DomainFilter OwnerID() string } diff --git a/registry/txt.go b/registry/txt.go index 3f37ff01cc..943a3d0166 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -296,7 +296,7 @@ func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) } // AdjustEndpoints modifies the endpoints as needed by the specific provider -func (im *TXTRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint { +func (im *TXTRegistry) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoint.Endpoint, error) { return im.provider.AdjustEndpoints(endpoints) }