diff --git a/controller/controller.go b/controller/controller.go index 0012ac088..af3e3219a 100644 --- a/controller/controller.go +++ b/controller/controller.go @@ -195,8 +195,6 @@ func (c *Controller) RunOnce(ctx context.Context) error { return err } - missingRecords := c.Registry.MissingRecords() - registryEndpointsTotal.Set(float64(len(records))) regARecords, regAAAARecords := countAddressRecords(records) registryARecords.Set(float64(regARecords)) @@ -218,29 +216,6 @@ func (c *Controller) RunOnce(ctx context.Context) error { verifiedAAAARecords.Set(float64(vAAAARecords)) endpoints = c.Registry.AdjustEndpoints(endpoints) - if len(missingRecords) > 0 { - // Add missing records before the actual plan is applied. - // This prevents the problems when the missing TXT record needs to be - // created and deleted/upserted in the same batch. - missingRecordsPlan := &plan.Plan{ - Policies: []plan.Policy{c.Policy}, - Missing: missingRecords, - DomainFilter: endpoint.MatchAllDomainFilters{c.DomainFilter, c.Registry.GetDomainFilter()}, - PropertyComparator: c.Registry.PropertyValuesEqual, - ManagedRecords: c.ManagedRecordTypes, - } - missingRecordsPlan = missingRecordsPlan.Calculate() - if missingRecordsPlan.Changes.HasChanges() { - err = c.Registry.ApplyChanges(ctx, missingRecordsPlan.Changes) - if err != nil { - registryErrorsTotal.Inc() - deprecatedRegistryErrors.Inc() - return err - } - log.Info("All missing records are created") - } - } - plan := &plan.Plan{ Policies: []plan.Policy{c.Policy}, Current: records, diff --git a/controller/controller_test.go b/controller/controller_test.go index f6b03d433..9f37ab295 100644 --- a/controller/controller_test.go +++ b/controller/controller_test.go @@ -311,51 +311,6 @@ func testControllerFiltersDomains(t *testing.T, configuredEndpoints []*endpoint. } } -type noopRegistryWithMissing struct { - *registry.NoopRegistry - missingRecords []*endpoint.Endpoint -} - -func (r *noopRegistryWithMissing) MissingRecords() []*endpoint.Endpoint { - return r.missingRecords -} - -func testControllerFiltersDomainsWithMissing(t *testing.T, configuredEndpoints []*endpoint.Endpoint, domainFilter endpoint.DomainFilterInterface, providerEndpoints, missingEndpoints []*endpoint.Endpoint, expectedChanges []*plan.Changes) { - t.Helper() - cfg := externaldns.NewConfig() - cfg.ManagedDNSRecordTypes = []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME} - - source := new(testutils.MockSource) - source.On("Endpoints").Return(configuredEndpoints, nil) - - // Fake some existing records in our DNS provider and validate some desired changes. - provider := &filteredMockProvider{ - RecordsStore: providerEndpoints, - } - noop, err := registry.NewNoopRegistry(provider) - require.NoError(t, err) - - r := &noopRegistryWithMissing{ - NoopRegistry: noop, - missingRecords: missingEndpoints, - } - - ctrl := &Controller{ - Source: source, - Registry: r, - Policy: &plan.SyncPolicy{}, - DomainFilter: domainFilter, - ManagedRecordTypes: cfg.ManagedDNSRecordTypes, - } - - assert.NoError(t, ctrl.RunOnce(context.Background())) - assert.Equal(t, 1, provider.RecordsCallCount) - require.Len(t, provider.ApplyChangesCalls, len(expectedChanges)) - for i, change := range expectedChanges { - assert.Equal(t, *change, *provider.ApplyChangesCalls[i]) - } -} - func TestControllerSkipsEmptyChanges(t *testing.T) { testControllerFiltersDomains( t, @@ -683,60 +638,6 @@ func TestARecords(t *testing.T) { assert.Equal(t, math.Float64bits(1), valueFromMetric(registryARecords)) } -// TestMissingRecordsApply validates that the missing records result in the dedicated plan apply. -func TestMissingRecordsApply(t *testing.T) { - testControllerFiltersDomainsWithMissing( - t, - []*endpoint.Endpoint{ - { - DNSName: "record1.used.tld", - RecordType: endpoint.RecordTypeA, - Targets: endpoint.Targets{"1.2.3.4"}, - }, - { - DNSName: "record2.used.tld", - RecordType: endpoint.RecordTypeA, - Targets: endpoint.Targets{"8.8.8.8"}, - }, - }, - endpoint.NewDomainFilter([]string{"used.tld"}), - []*endpoint.Endpoint{ - { - DNSName: "record1.used.tld", - RecordType: endpoint.RecordTypeA, - Targets: endpoint.Targets{"1.2.3.4"}, - }, - }, - []*endpoint.Endpoint{ - { - DNSName: "a-record1.used.tld", - RecordType: endpoint.RecordTypeTXT, - Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, - }, - }, - []*plan.Changes{ - // Missing record had its own plan applied. - { - Create: []*endpoint.Endpoint{ - { - DNSName: "a-record1.used.tld", - RecordType: endpoint.RecordTypeTXT, - Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, - }, - }, - }, - { - Create: []*endpoint.Endpoint{ - { - DNSName: "record2.used.tld", - RecordType: endpoint.RecordTypeA, - Targets: endpoint.Targets{"8.8.8.8"}, - }, - }, - }, - }) -} - func TestAAAARecords(t *testing.T) { testControllerFiltersDomains( t, diff --git a/plan/plan.go b/plan/plan.go index 1e8a38f1e..312bb261b 100644 --- a/plan/plan.go +++ b/plan/plan.go @@ -37,8 +37,6 @@ type Plan struct { Current []*endpoint.Endpoint // List of desired records Desired []*endpoint.Endpoint - // List of missing records to be created, use for the migrations (e.g. old-new TXT format) - Missing []*endpoint.Endpoint // Policies under which the desired changes are calculated Policies []Policy // List of changes necessary to move towards desired state @@ -177,11 +175,6 @@ func (p *Plan) Calculate() *Plan { changes = pol.Apply(changes) } - // Handle the migration of the TXT records created before the new format (introduced in v0.12.0) - if len(p.Missing) > 0 { - changes.Create = append(changes.Create, filterRecordsForPlan(p.Missing, p.DomainFilter, append(p.ManagedRecords, endpoint.RecordTypeTXT))...) - } - plan := &Plan{ Current: p.Current, Desired: p.Desired, diff --git a/plan/plan_test.go b/plan/plan_test.go index 2b107d0e8..cc9e56bd5 100644 --- a/plan/plan_test.go +++ b/plan/plan_test.go @@ -51,9 +51,6 @@ type PlanTestSuite struct { domainFilterFiltered2 *endpoint.Endpoint domainFilterFiltered3 *endpoint.Endpoint domainFilterExcluded *endpoint.Endpoint - domainFilterFilteredTXT1 *endpoint.Endpoint - domainFilterFilteredTXT2 *endpoint.Endpoint - domainFilterExcludedTXT *endpoint.Endpoint } func (suite *PlanTestSuite) SetupTest() { @@ -233,21 +230,6 @@ func (suite *PlanTestSuite) SetupTest() { Targets: endpoint.Targets{"1.1.1.1"}, RecordType: "A", } - suite.domainFilterFilteredTXT1 = &endpoint.Endpoint{ - DNSName: "a-foo.domain.tld", - Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, - RecordType: "TXT", - } - suite.domainFilterFilteredTXT2 = &endpoint.Endpoint{ - DNSName: "cname-bar.domain.tld", - Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, - RecordType: "TXT", - } - suite.domainFilterExcludedTXT = &endpoint.Endpoint{ - DNSName: "cname-bar.otherdomain.tld", - Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, - RecordType: "TXT", - } } func (suite *PlanTestSuite) TestSyncFirstRound() { @@ -661,21 +643,6 @@ func (suite *PlanTestSuite) TestDomainFiltersUpdate() { validateEntries(suite.T(), changes.Delete, expectedDelete) } -func (suite *PlanTestSuite) TestMissing() { - missing := []*endpoint.Endpoint{suite.domainFilterFilteredTXT1, suite.domainFilterFilteredTXT2, suite.domainFilterExcludedTXT} - expectedCreate := []*endpoint.Endpoint{suite.domainFilterFilteredTXT1, suite.domainFilterFilteredTXT2} - - p := &Plan{ - Policies: []Policy{&SyncPolicy{}}, - Missing: missing, - DomainFilter: endpoint.NewDomainFilter([]string{"domain.tld"}), - ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME}, - } - - changes := p.Calculate().Changes - validateEntries(suite.T(), changes.Create, expectedCreate) -} - func (suite *PlanTestSuite) TestAAAARecords() { current := []*endpoint.Endpoint{} diff --git a/registry/aws_sd_registry.go b/registry/aws_sd_registry.go index f0fef584d..1ecd88546 100644 --- a/registry/aws_sd_registry.go +++ b/registry/aws_sd_registry.go @@ -67,11 +67,6 @@ func (sdr *AWSSDRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, er return records, nil } -// MissingRecords returns nil because there is no missing records for AWSSD registry -func (sdr *AWSSDRegistry) MissingRecords() []*endpoint.Endpoint { - return nil -} - // ApplyChanges filters out records not owned the External-DNS, additionally it adds the required label // inserted in the AWS SD instance as a CreateID field func (sdr *AWSSDRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) error { diff --git a/registry/noop.go b/registry/noop.go index d48cd82fe..73257730c 100644 --- a/registry/noop.go +++ b/registry/noop.go @@ -45,11 +45,6 @@ func (im *NoopRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, erro return im.provider.Records(ctx) } -// MissingRecords returns nil because there is no missing records for Noop registry -func (im *NoopRegistry) MissingRecords() []*endpoint.Endpoint { - return nil -} - // ApplyChanges propagates changes to the dns provider func (im *NoopRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) error { return im.provider.ApplyChanges(ctx, changes) diff --git a/registry/registry.go b/registry/registry.go index 7f219a846..fa39fb8ec 100644 --- a/registry/registry.go +++ b/registry/registry.go @@ -35,7 +35,6 @@ type Registry interface { PropertyValuesEqual(attribute string, previous string, current string) bool AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint GetDomainFilter() endpoint.DomainFilterInterface - MissingRecords() []*endpoint.Endpoint } // TODO(ideahitme): consider moving this to Plan diff --git a/registry/txt.go b/registry/txt.go index 2c7cccbdf..e9411185b 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -30,7 +30,10 @@ import ( "sigs.k8s.io/external-dns/provider" ) -const recordTemplate = "%{record_type}" +const ( + recordTemplate = "%{record_type}" + providerSpecificForceUpdate = "txt/force-update" +) // TXTRegistry implements registry interface with ownership implemented via associated TXT records type TXTRegistry struct { @@ -50,9 +53,6 @@ type TXTRegistry struct { managedRecordTypes []string - // missingTXTRecords stores TXT records which are missing after the migration to the new format - missingTXTRecords []*endpoint.Endpoint - // encrypt text records txtEncryptEnabled bool txtEncryptAESKey []byte @@ -117,7 +117,6 @@ func (im *TXTRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, error } endpoints := []*endpoint.Endpoint{} - missingEndpoints := []*endpoint.Endpoint{} labelMap := map[string]endpoint.Labels{} txtRecordsMap := map[string]struct{}{} @@ -174,17 +173,11 @@ func (im *TXTRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, error if plan.IsManagedRecord(ep.RecordType, im.managedRecordTypes) { // Get desired TXT records and detect the missing ones desiredTXTs := im.generateTXTRecord(ep) - missingDesiredTXTs := []*endpoint.Endpoint{} for _, desiredTXT := range desiredTXTs { if _, exists := txtRecordsMap[desiredTXT.DNSName]; !exists { - missingDesiredTXTs = append(missingDesiredTXTs, desiredTXT) + ep.WithProviderSpecific(providerSpecificForceUpdate, "true") } } - if len(desiredTXTs) > len(missingDesiredTXTs) { - // Add missing TXT records only if those are managed (by externaldns) ones. - // The unmanaged record has both of the desired TXT records missing. - missingEndpoints = append(missingEndpoints, missingDesiredTXTs...) - } } } } @@ -195,17 +188,9 @@ func (im *TXTRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, error im.recordsCacheRefreshTime = time.Now() } - im.missingTXTRecords = missingEndpoints - return endpoints, nil } -// MissingRecords returns the TXT record to be created. -// The missing records are collected during the run of Records method. -func (im *TXTRegistry) MissingRecords() []*endpoint.Endpoint { - return im.missingTXTRecords -} - // generateTXTRecord generates both "old" and "new" TXT records. // Once we decide to drop old format we need to drop toTXTName() and rename toNewTXTName func (im *TXTRegistry) generateTXTRecord(r *endpoint.Endpoint) []*endpoint.Endpoint { diff --git a/registry/txt_test.go b/registry/txt_test.go index 80075c81d..2493fbb11 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -875,6 +875,12 @@ func testTXTRegistryMissingRecordsNoPrefix(t *testing.T) { // owner was added from the TXT record's target endpoint.OwnerLabelKey: "owner", }, + ProviderSpecific: []endpoint.ProviderSpecificProperty{ + { + Name: "txt/force-update", + Value: "true", + }, + }, }, { DNSName: "oldformat2.test-zone.example.org", @@ -883,6 +889,12 @@ func testTXTRegistryMissingRecordsNoPrefix(t *testing.T) { Labels: map[string]string{ endpoint.OwnerLabelKey: "owner", }, + ProviderSpecific: []endpoint.ProviderSpecificProperty{ + { + Name: "txt/force-update", + Value: "true", + }, + }, }, { DNSName: "newformat.test-zone.example.org", @@ -931,32 +943,10 @@ func testTXTRegistryMissingRecordsNoPrefix(t *testing.T) { }, } - expectedMissingRecords := []*endpoint.Endpoint{ - { - DNSName: "cname-oldformat.test-zone.example.org", - // owner is taken from the source record (A, CNAME, etc.) - Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, - RecordType: endpoint.RecordTypeTXT, - Labels: endpoint.Labels{ - endpoint.OwnedRecordLabelKey: "oldformat.test-zone.example.org", - }, - }, - { - DNSName: "a-oldformat2.test-zone.example.org", - Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, - RecordType: endpoint.RecordTypeTXT, - Labels: endpoint.Labels{ - endpoint.OwnedRecordLabelKey: "oldformat2.test-zone.example.org", - }, - }, - } - r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "wc", []string{endpoint.RecordTypeCNAME, endpoint.RecordTypeA, endpoint.RecordTypeNS}, false, nil) records, _ := r.Records(ctx) - missingRecords := r.MissingRecords() assert.True(t, testutils.SameEndpoints(records, expectedRecords)) - assert.True(t, testutils.SameEndpoints(missingRecords, expectedMissingRecords)) } func testTXTRegistryMissingRecordsWithPrefix(t *testing.T) { @@ -988,6 +978,12 @@ func testTXTRegistryMissingRecordsWithPrefix(t *testing.T) { // owner was added from the TXT record's target endpoint.OwnerLabelKey: "owner", }, + ProviderSpecific: []endpoint.ProviderSpecificProperty{ + { + Name: "txt/force-update", + Value: "true", + }, + }, }, { DNSName: "oldformat2.test-zone.example.org", @@ -996,6 +992,12 @@ func testTXTRegistryMissingRecordsWithPrefix(t *testing.T) { Labels: map[string]string{ endpoint.OwnerLabelKey: "owner", }, + ProviderSpecific: []endpoint.ProviderSpecificProperty{ + { + Name: "txt/force-update", + Value: "true", + }, + }, }, { DNSName: "newformat.test-zone.example.org", @@ -1035,32 +1037,10 @@ func testTXTRegistryMissingRecordsWithPrefix(t *testing.T) { }, } - expectedMissingRecords := []*endpoint.Endpoint{ - { - DNSName: "txt.cname-oldformat.test-zone.example.org", - // owner is taken from the source record (A, CNAME, etc.) - Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, - RecordType: endpoint.RecordTypeTXT, - Labels: endpoint.Labels{ - endpoint.OwnedRecordLabelKey: "oldformat.test-zone.example.org", - }, - }, - { - DNSName: "txt.a-oldformat2.test-zone.example.org", - Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, - RecordType: endpoint.RecordTypeTXT, - Labels: endpoint.Labels{ - endpoint.OwnedRecordLabelKey: "oldformat2.test-zone.example.org", - }, - }, - } - r, _ := NewTXTRegistry(p, "txt.", "", "owner", time.Hour, "wc", []string{endpoint.RecordTypeCNAME, endpoint.RecordTypeA, endpoint.RecordTypeNS}, false, nil) records, _ := r.Records(ctx) - missingRecords := r.MissingRecords() assert.True(t, testutils.SameEndpoints(records, expectedRecords)) - assert.True(t, testutils.SameEndpoints(missingRecords, expectedMissingRecords)) } func TestCacheMethods(t *testing.T) {