From 08dd8b4fba6001c95732f996197026f68605fc78 Mon Sep 17 00:00:00 2001 From: Thibaut R Date: Tue, 29 Oct 2024 11:42:34 +0100 Subject: [PATCH] Introduced flags to migrate ownerID with TXT registry --- controller/controller.go | 20 +++-- docs/registry/txt.md | 10 +++ main.go | 7 +- pkg/apis/externaldns/types.go | 6 ++ pkg/apis/externaldns/types_test.go | 6 ++ plan/plan.go | 28 +++++- plan/plan_test.go | 45 ++++++++++ provider/ovh/ovh.go | 3 +- registry/txt.go | 12 ++- registry/txt_test.go | 137 ++++++++++++++++++++++------- 10 files changed, 232 insertions(+), 42 deletions(-) diff --git a/controller/controller.go b/controller/controller.go index 2521b4436a..aa63ad36a5 100644 --- a/controller/controller.go +++ b/controller/controller.go @@ -199,6 +199,10 @@ type Controller struct { ExcludeRecordTypes []string // MinEventSyncInterval is used as window for batching events MinEventSyncInterval time.Duration + // migrate txt-owner flag + TXTOwnerMigrate bool + // old txt-owner whitch needed to modify + TXTOwnerOld string } // RunOnce runs a single iteration of a reconciliation loop. @@ -242,13 +246,15 @@ func (c *Controller) RunOnce(ctx context.Context) error { registryFilter := c.Registry.GetDomainFilter() plan := &plan.Plan{ - Policies: []plan.Policy{c.Policy}, - Current: records, - Desired: endpoints, - DomainFilter: endpoint.MatchAllDomainFilters{c.DomainFilter, registryFilter}, - ManagedRecords: c.ManagedRecordTypes, - ExcludeRecords: c.ExcludeRecordTypes, - OwnerID: c.Registry.OwnerID(), + Policies: []plan.Policy{c.Policy}, + Current: records, + Desired: endpoints, + DomainFilter: endpoint.MatchAllDomainFilters{c.DomainFilter, registryFilter}, + ManagedRecords: c.ManagedRecordTypes, + ExcludeRecords: c.ExcludeRecordTypes, + OwnerID: c.Registry.OwnerID(), + TXTOwnerMigrate: c.TXTOwnerMigrate, + TXTOwnerOld: c.TXTOwnerOld, } plan = plan.Calculate() diff --git a/docs/registry/txt.md b/docs/registry/txt.md index c6ea26bfbd..c2a523f793 100644 --- a/docs/registry/txt.md +++ b/docs/registry/txt.md @@ -95,3 +95,13 @@ The TXT registry can optionally cache DNS records read from the provider. This c rate limits imposed by the provider. Caching is enabled by specifying a cache duration with the `--txt-cache-interval` flag. + +## Owner migration + +It is possible to migrate the owner of your records when using the TXT registry. + +Simply use the following flags when setting up your instance : `--migrate-txt-owner`, which is the boolean enabling +the migration feature, `--from-txt-owner=` and set `--txt-owner-id` to +the new value you want it to be. + +Take not that if you didn't set `--txt-owner-id` previously, the value automatically set by ExternalDNS is 'default'. diff --git a/main.go b/main.go index 567f5423c3..1d7cbf3627 100644 --- a/main.go +++ b/main.go @@ -93,6 +93,9 @@ func main() { if cfg.DryRun { log.Info("running in dry-run mode. No changes to DNS records will be made.") } + if cfg.TXTOwnerMigrate { + log.Infof("modify the previous txt-owner (%s) to the current txt-owner (%s)", cfg.TXTOwnerOld, cfg.TXTOwnerID) + } ll, err := log.ParseLevel(cfg.LogLevel) if err != nil { @@ -387,7 +390,7 @@ func main() { case "noop": r, err = registry.NewNoopRegistry(p) case "txt": - r, err = registry.NewTXTRegistry(p, cfg.TXTPrefix, cfg.TXTSuffix, cfg.TXTOwnerID, cfg.TXTCacheInterval, cfg.TXTWildcardReplacement, cfg.ManagedDNSRecordTypes, cfg.ExcludeDNSRecordTypes, cfg.TXTEncryptEnabled, []byte(cfg.TXTEncryptAESKey)) + r, err = registry.NewTXTRegistry(p, cfg.TXTPrefix, cfg.TXTSuffix, cfg.TXTOwnerID, cfg.TXTCacheInterval, cfg.TXTWildcardReplacement, cfg.ManagedDNSRecordTypes, cfg.ExcludeDNSRecordTypes, cfg.TXTEncryptEnabled, []byte(cfg.TXTEncryptAESKey), cfg.TXTOwnerMigrate, cfg.TXTOwnerOld) case "aws-sd": r, err = registry.NewAWSSDRegistry(p, cfg.TXTOwnerID) default: @@ -412,6 +415,8 @@ func main() { ManagedRecordTypes: cfg.ManagedDNSRecordTypes, ExcludeRecordTypes: cfg.ExcludeDNSRecordTypes, MinEventSyncInterval: cfg.MinEventSyncInterval, + TXTOwnerMigrate: cfg.TXTOwnerMigrate, + TXTOwnerOld: cfg.TXTOwnerOld, } if cfg.Once { diff --git a/pkg/apis/externaldns/types.go b/pkg/apis/externaldns/types.go index de9ce3381e..bb2a45097f 100644 --- a/pkg/apis/externaldns/types.go +++ b/pkg/apis/externaldns/types.go @@ -133,6 +133,8 @@ type Config struct { Policy string Registry string TXTOwnerID string + TXTOwnerOld string + TXTOwnerMigrate bool TXTPrefix string TXTSuffix string TXTEncryptEnabled bool @@ -290,6 +292,8 @@ var defaultConfig = &Config{ Policy: "sync", Registry: "txt", TXTOwnerID: "default", + TXTOwnerOld: "default", + TXTOwnerMigrate: false, TXTPrefix: "", TXTSuffix: "", TXTCacheInterval: 0, @@ -572,6 +576,8 @@ func (cfg *Config) ParseFlags(args []string) error { // Flags related to the registry app.Flag("registry", "The registry implementation to use to keep track of DNS record ownership (default: txt, options: txt, noop, dynamodb, aws-sd)").Default(defaultConfig.Registry).EnumVar(&cfg.Registry, "txt", "noop", "dynamodb", "aws-sd") app.Flag("txt-owner-id", "When using the TXT or DynamoDB registry, a name that identifies this instance of ExternalDNS (default: default)").Default(defaultConfig.TXTOwnerID).StringVar(&cfg.TXTOwnerID) + app.Flag("from-txt-owner", "Old txt-owner-id that needs to be overwritten (default: default)").StringVar(&cfg.TXTOwnerOld) + app.Flag("migrate-txt-owner", "When enabled, modify the previous txt-owner to the current txt-owner (default: disabled)").BoolVar(&cfg.TXTOwnerMigrate) app.Flag("txt-prefix", "When using the TXT registry, a custom string that's prefixed to each ownership DNS record (optional). Could contain record type template like '%{record_type}-prefix-'. Mutual exclusive with txt-suffix!").Default(defaultConfig.TXTPrefix).StringVar(&cfg.TXTPrefix) app.Flag("txt-suffix", "When using the TXT registry, a custom string that's suffixed to the host portion of each ownership DNS record (optional). Could contain record type template like '-%{record_type}-suffix'. Mutual exclusive with txt-prefix!").Default(defaultConfig.TXTSuffix).StringVar(&cfg.TXTSuffix) app.Flag("txt-wildcard-replacement", "When using the TXT registry, a custom string that's used instead of an asterisk for TXT records corresponding to wildcard DNS records (optional)").Default(defaultConfig.TXTWildcardReplacement).StringVar(&cfg.TXTWildcardReplacement) diff --git a/pkg/apis/externaldns/types_test.go b/pkg/apis/externaldns/types_test.go index f06e54f165..73fbc86b2e 100644 --- a/pkg/apis/externaldns/types_test.go +++ b/pkg/apis/externaldns/types_test.go @@ -198,6 +198,8 @@ var ( Policy: "upsert-only", Registry: "noop", TXTOwnerID: "owner-1", + TXTOwnerOld: "owner-old", + TXTOwnerMigrate: true, TXTPrefix: "associated-txt-record", TXTCacheInterval: 12 * time.Hour, Interval: 10 * time.Minute, @@ -333,6 +335,8 @@ func TestParseFlags(t *testing.T) { "--policy=upsert-only", "--registry=noop", "--txt-owner-id=owner-1", + "--migrate-txt-owner", + "--from-txt-owner=owner-old", "--txt-prefix=associated-txt-record", "--txt-cache-interval=12h", "--dynamodb-table=custom-table", @@ -445,6 +449,8 @@ func TestParseFlags(t *testing.T) { "EXTERNAL_DNS_POLICY": "upsert-only", "EXTERNAL_DNS_REGISTRY": "noop", "EXTERNAL_DNS_TXT_OWNER_ID": "owner-1", + "EXTERNAL_DNS_FROM_TXT_OWNER": "owner-old", + "EXTERNAL_DNS_MIGRATE_TXT_OWNER": "true", "EXTERNAL_DNS_TXT_PREFIX": "associated-txt-record", "EXTERNAL_DNS_TXT_CACHE_INTERVAL": "12h", "EXTERNAL_DNS_INTERVAL": "10m", diff --git a/plan/plan.go b/plan/plan.go index d0ca7f96a2..52ba4ef002 100644 --- a/plan/plan.go +++ b/plan/plan.go @@ -49,6 +49,10 @@ type Plan struct { ExcludeRecords []string // OwnerID of records to manage OwnerID string + // modify owner flag + TXTOwnerMigrate bool + // old txt-owner whitch needed to modify + TXTOwnerOld string } // Changes holds lists of actions to be executed by dns providers @@ -179,6 +183,7 @@ func (p *Plan) Calculate() *Plan { } changes := &Changes{} + var hasMig bool for key, row := range t.rows { // dns name not taken @@ -221,6 +226,23 @@ func (p *Plan) Calculate() *Plan { if records.current != nil && len(records.candidates) > 0 { update := t.resolver.ResolveUpdate(records.current, records.candidates) + // Change the specified old txt-owner to the new txt-owner (if TXTOwnerMigrate==true and set the from-txt-owner) + if p.TXTOwnerMigrate && records.current.Labels[endpoint.OwnerLabelKey] == p.TXTOwnerOld { + hasMig = true + oldRecord := records.current + changes.UpdateOld = append(changes.UpdateOld, oldRecord) + recordToMigrate := update + recordToMigrate.Labels[endpoint.OwnerLabelKey] = p.OwnerID + log.WithFields(log.Fields{ + "previousOwner": oldRecord.Labels[endpoint.OwnerLabelKey], + "newOwner": p.OwnerID, + "dnsName": records.current.DNSName, + "recordType": records.current.RecordType, + }).Info("Found record to migrate") + changes.UpdateNew = append(changes.UpdateNew, update) + continue + } + if shouldUpdateTTL(update, records.current) || targetChanged(update, records.current) || p.shouldUpdateProviderSpecific(update, records.current) { inheritOwner(records.current, update) changes.UpdateNew = append(changes.UpdateNew, update) @@ -253,7 +275,11 @@ func (p *Plan) Calculate() *Plan { if p.OwnerID != "" { changes.Delete = endpoint.FilterEndpointsByOwnerID(p.OwnerID, changes.Delete) changes.Delete = endpoint.RemoveDuplicates(changes.Delete) - changes.UpdateOld = endpoint.FilterEndpointsByOwnerID(p.OwnerID, changes.UpdateOld) + // When running a migration we don't want to filter out endpoint duplicates on ownerId value so that old records + // are deleted correctly + if !hasMig { + changes.UpdateOld = endpoint.FilterEndpointsByOwnerID(p.OwnerID, changes.UpdateOld) + } changes.UpdateNew = endpoint.FilterEndpointsByOwnerID(p.OwnerID, changes.UpdateNew) } diff --git a/plan/plan_test.go b/plan/plan_test.go index 5aecc8632e..a05089064d 100644 --- a/plan/plan_test.go +++ b/plan/plan_test.go @@ -51,6 +51,8 @@ type PlanTestSuite struct { domainFilterFiltered2 *endpoint.Endpoint domainFilterFiltered3 *endpoint.Endpoint domainFilterExcluded *endpoint.Endpoint + migrateTXTOwnerOld *endpoint.Endpoint + migrateTXTOwnerNew *endpoint.Endpoint } func (suite *PlanTestSuite) SetupTest() { @@ -243,6 +245,24 @@ func (suite *PlanTestSuite) SetupTest() { Targets: endpoint.Targets{"1.1.1.1"}, RecordType: "A", } + suite.migrateTXTOwnerOld = &endpoint.Endpoint{ + DNSName: "migrated.domain.tld", + Targets: endpoint.Targets{"1.2.3.4"}, + RecordType: "A", + Labels: map[string]string{ + endpoint.ResourceLabelKey: "ingress/default/foo-v1", + endpoint.OwnerLabelKey: "owner-old", + }, + } + suite.migrateTXTOwnerNew = &endpoint.Endpoint{ + DNSName: "migrated.domain.tld", + Targets: endpoint.Targets{"1.2.3.4"}, + RecordType: "A", + Labels: map[string]string{ + endpoint.ResourceLabelKey: "ingress/default/foo-v1", + endpoint.OwnerLabelKey: "owner", + }, + } } func (suite *PlanTestSuite) TestSyncFirstRound() { @@ -742,6 +762,31 @@ func (suite *PlanTestSuite) TestIgnoreTargetCase() { validateEntries(suite.T(), changes.Delete, expectedDelete) } +func (suite *PlanTestSuite) TestMigrateTXTRecord() { + current := []*endpoint.Endpoint{suite.migrateTXTOwnerOld} + desired := []*endpoint.Endpoint{suite.migrateTXTOwnerNew} + expectedCreate := []*endpoint.Endpoint{} + expectedUpdateOld := []*endpoint.Endpoint{suite.migrateTXTOwnerOld} + expectedUpdateNew := []*endpoint.Endpoint{suite.migrateTXTOwnerNew} + expectedDelete := []*endpoint.Endpoint{} + + p := &Plan{ + Policies: []Policy{&SyncPolicy{}}, + Current: current, + Desired: desired, + TXTOwnerMigrate: true, + TXTOwnerOld: "owner-old", + OwnerID: "owner", + ManagedRecords: []string{endpoint.RecordTypeA}, + } + + 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} diff --git a/provider/ovh/ovh.go b/provider/ovh/ovh.go index d75d343bc5..19cd0113ee 100644 --- a/provider/ovh/ovh.go +++ b/provider/ovh/ovh.go @@ -192,10 +192,11 @@ func (p *OVHProvider) change(change ovhChange) error { log.Debugf("OVH: Add an entry to %s", change.String()) return p.client.Post(fmt.Sprintf("/domain/zone/%s/record", change.Zone), change.ovhRecordFields, nil) case ovhDelete: + log.Debugf("OVH: Delete an entry to %s", change.String()) if change.ID == 0 { + log.Debugf("OVH: No ID found when deleting entry: %s", change.String()) return ErrRecordToMutateNotFound } - log.Debugf("OVH: Delete an entry to %s", change.String()) return p.client.Delete(fmt.Sprintf("/domain/zone/%s/record/%d", change.Zone, change.ID), nil) } return nil diff --git a/registry/txt.go b/registry/txt.go index 12445bcb1d..bc637c1062 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -56,10 +56,14 @@ type TXTRegistry struct { // encrypt text records txtEncryptEnabled bool txtEncryptAESKey []byte + + //Handle Owner ID migration + isMigrationEnabled bool + oldOwnerID string } // NewTXTRegistry returns new TXTRegistry object -func NewTXTRegistry(provider provider.Provider, txtPrefix, txtSuffix, ownerID string, cacheInterval time.Duration, txtWildcardReplacement string, managedRecordTypes, excludeRecordTypes []string, txtEncryptEnabled bool, txtEncryptAESKey []byte) (*TXTRegistry, error) { +func NewTXTRegistry(provider provider.Provider, txtPrefix, txtSuffix, ownerID string, cacheInterval time.Duration, txtWildcardReplacement string, managedRecordTypes, excludeRecordTypes []string, txtEncryptEnabled bool, txtEncryptAESKey []byte, isMigrationEnabled bool, oldOwnerID string) (*TXTRegistry, error) { if ownerID == "" { return nil, errors.New("owner id cannot be empty") } @@ -88,6 +92,8 @@ func NewTXTRegistry(provider provider.Provider, txtPrefix, txtSuffix, ownerID st excludeRecordTypes: excludeRecordTypes, txtEncryptEnabled: txtEncryptEnabled, txtEncryptAESKey: txtEncryptAESKey, + isMigrationEnabled: isMigrationEnabled, + oldOwnerID: oldOwnerID, }, nil } @@ -250,6 +256,10 @@ func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) UpdateOld: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.UpdateOld), Delete: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.Delete), } + if im.isMigrationEnabled { + filteredChanges.UpdateOld = append(filteredChanges.UpdateOld, endpoint.FilterEndpointsByOwnerID(im.oldOwnerID, changes.UpdateOld)...) + filteredChanges.Delete = append(filteredChanges.Delete, endpoint.FilterEndpointsByOwnerID(im.oldOwnerID, changes.Delete)...) + } for _, r := range filteredChanges.Create { if r.Labels == nil { r.Labels = make(map[string]string) diff --git a/registry/txt_test.go b/registry/txt_test.go index 515163cfa8..b3f771dca0 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -46,20 +46,30 @@ func TestTXTRegistry(t *testing.T) { func testTXTRegistryNew(t *testing.T) { p := inmemory.NewInMemoryProvider() - _, err := NewTXTRegistry(p, "txt", "", "", time.Hour, "", []string{}, []string{}, false, nil) + _, err := NewTXTRegistry(p, "txt", "", "", time.Hour, "", []string{}, []string{}, false, nil, false, "") require.Error(t, err) - _, err = NewTXTRegistry(p, "", "txt", "", time.Hour, "", []string{}, []string{}, false, nil) + _, err = NewTXTRegistry(p, "", "txt", "", time.Hour, "", []string{}, []string{}, false, nil, false, "") require.Error(t, err) - r, err := NewTXTRegistry(p, "txt", "", "owner", time.Hour, "", []string{}, []string{}, false, nil) + r, err := NewTXTRegistry(p, "txt", "", "owner", time.Hour, "", []string{}, []string{}, false, nil, false, "") require.NoError(t, err) assert.Equal(t, p, r.provider) - r, err = NewTXTRegistry(p, "", "txt", "owner", time.Hour, "", []string{}, []string{}, false, nil) + r, err = NewTXTRegistry(p, "", "txt", "owner", time.Hour, "", []string{}, []string{}, false, nil, false, "") require.NoError(t, err) - _, err = NewTXTRegistry(p, "txt", "txt", "owner", time.Hour, "", []string{}, []string{}, false, nil) + _, err = NewTXTRegistry(p, "txt", "txt", "owner", time.Hour, "", []string{}, []string{}, false, nil, false, "") + require.Error(t, err) + + r, err = NewTXTRegistry(p, "txt", "", "owner", time.Hour, "", []string{}, []string{}, false, nil, true, "old-owner") + require.NoError(t, err) + assert.Equal(t, p, r.provider) + + r, err = NewTXTRegistry(p, "", "txt", "owner", time.Hour, "", []string{}, []string{}, false, nil, true, "old-owner") + require.NoError(t, err) + + _, err = NewTXTRegistry(p, "txt", "txt", "owner", time.Hour, "", []string{}, []string{}, false, nil, true, "old-owner") require.Error(t, err) _, ok := r.mapper.(affixNameMapper) @@ -68,16 +78,16 @@ func testTXTRegistryNew(t *testing.T) { assert.Equal(t, p, r.provider) aesKey := []byte(";k&l)nUC/33:{?d{3)54+,AD?]SX%yh^") - _, err = NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil) + _, err = NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil, false, "") require.NoError(t, err) - _, err = NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, aesKey) + _, err = NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, aesKey, false, "") require.NoError(t, err) - _, err = NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, true, nil) + _, err = NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, true, nil, false, "") require.Error(t, err) - r, err = NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, true, aesKey) + r, err = NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, true, aesKey, false, "") require.NoError(t, err) _, ok = r.mapper.(affixNameMapper) @@ -215,13 +225,13 @@ func testTXTRegistryRecordsPrefixed(t *testing.T) { }, } - r, _ := NewTXTRegistry(p, "txt.", "", "owner", time.Hour, "wc", []string{}, []string{}, false, nil) + r, _ := NewTXTRegistry(p, "txt.", "", "owner", time.Hour, "wc", []string{}, []string{}, false, nil, false, "") records, _ := r.Records(ctx) assert.True(t, testutils.SameEndpoints(records, expectedRecords)) // Ensure prefix is case-insensitive - r, _ = NewTXTRegistry(p, "TxT.", "", "owner", time.Hour, "wc", []string{}, []string{}, false, nil) + r, _ = NewTXTRegistry(p, "TxT.", "", "owner", time.Hour, "wc", []string{}, []string{}, false, nil, false, "") records, _ = r.Records(ctx) assert.True(t, testutils.SameEndpoints(records, expectedRecords)) @@ -340,13 +350,13 @@ func testTXTRegistryRecordsSuffixed(t *testing.T) { }, } - r, _ := NewTXTRegistry(p, "", "-txt", "owner", time.Hour, "", []string{}, []string{}, false, nil) + r, _ := NewTXTRegistry(p, "", "-txt", "owner", time.Hour, "", []string{}, []string{}, false, nil, false, "") records, _ := r.Records(ctx) assert.True(t, testutils.SameEndpoints(records, expectedRecords)) // Ensure prefix is case-insensitive - r, _ = NewTXTRegistry(p, "", "-TxT", "owner", time.Hour, "", []string{}, []string{}, false, nil) + r, _ = NewTXTRegistry(p, "", "-TxT", "owner", time.Hour, "", []string{}, []string{}, false, nil, false, "") records, _ = r.Records(ctx) assert.True(t, testutils.SameEndpointLabels(records, expectedRecords)) @@ -457,7 +467,7 @@ func testTXTRegistryRecordsNoPrefix(t *testing.T) { }, } - r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil) + r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil, false, "") records, _ := r.Records(ctx) assert.True(t, testutils.SameEndpoints(records, expectedRecords)) @@ -484,12 +494,12 @@ func testTXTRegistryRecordsPrefixedTemplated(t *testing.T) { }, } - r, _ := NewTXTRegistry(p, "txt-%{record_type}.", "", "owner", time.Hour, "wc", []string{}, []string{}, false, nil) + r, _ := NewTXTRegistry(p, "txt-%{record_type}.", "", "owner", time.Hour, "wc", []string{}, []string{}, false, nil, false, "") records, _ := r.Records(ctx) assert.True(t, testutils.SameEndpoints(records, expectedRecords)) - r, _ = NewTXTRegistry(p, "TxT-%{record_type}.", "", "owner", time.Hour, "wc", []string{}, []string{}, false, nil) + r, _ = NewTXTRegistry(p, "TxT-%{record_type}.", "", "owner", time.Hour, "wc", []string{}, []string{}, false, nil, false, "") records, _ = r.Records(ctx) assert.True(t, testutils.SameEndpoints(records, expectedRecords)) @@ -516,12 +526,12 @@ func testTXTRegistryRecordsSuffixedTemplated(t *testing.T) { }, } - r, _ := NewTXTRegistry(p, "", "txt%{record_type}", "owner", time.Hour, "wc", []string{}, []string{}, false, nil) + r, _ := NewTXTRegistry(p, "", "txt%{record_type}", "owner", time.Hour, "wc", []string{}, []string{}, false, nil, false, "") records, _ := r.Records(ctx) assert.True(t, testutils.SameEndpoints(records, expectedRecords)) - r, _ = NewTXTRegistry(p, "", "TxT%{record_type}", "owner", time.Hour, "wc", []string{}, []string{}, false, nil) + r, _ = NewTXTRegistry(p, "", "TxT%{record_type}", "owner", time.Hour, "wc", []string{}, []string{}, false, nil, false, "") records, _ = r.Records(ctx) assert.True(t, testutils.SameEndpoints(records, expectedRecords)) @@ -564,7 +574,7 @@ func testTXTRegistryApplyChangesWithPrefix(t *testing.T) { newEndpointWithOwner("txt.cname-multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), }, }) - r, _ := NewTXTRegistry(p, "txt.", "", "owner", time.Hour, "", []string{}, []string{}, false, nil) + r, _ := NewTXTRegistry(p, "txt.", "", "owner", time.Hour, "", []string{}, []string{}, false, nil, false, "") changes := &plan.Changes{ Create: []*endpoint.Endpoint{ @@ -653,7 +663,7 @@ func testTXTRegistryApplyChangesWithTemplatedPrefix(t *testing.T) { p.ApplyChanges(ctx, &plan.Changes{ Create: []*endpoint.Endpoint{}, }) - r, _ := NewTXTRegistry(p, "prefix%{record_type}.", "", "owner", time.Hour, "", []string{}, []string{}, false, nil) + r, _ := NewTXTRegistry(p, "prefix%{record_type}.", "", "owner", time.Hour, "", []string{}, []string{}, false, nil, false, "") changes := &plan.Changes{ Create: []*endpoint.Endpoint{ newEndpointWithOwnerResource("new-record-1.test-zone.example.org", "new-loadbalancer-1.lb.com", endpoint.RecordTypeCNAME, "", "ingress/default/my-ingress"), @@ -696,7 +706,7 @@ func testTXTRegistryApplyChangesWithTemplatedSuffix(t *testing.T) { p.OnApplyChanges = func(ctx context.Context, got *plan.Changes) { assert.Equal(t, ctxEndpoints, ctx.Value(provider.RecordsContextKey)) } - r, _ := NewTXTRegistry(p, "", "-%{record_type}suffix", "owner", time.Hour, "", []string{}, []string{}, false, nil) + r, _ := NewTXTRegistry(p, "", "-%{record_type}suffix", "owner", time.Hour, "", []string{}, []string{}, false, nil, false, "") changes := &plan.Changes{ Create: []*endpoint.Endpoint{ newEndpointWithOwnerResource("new-record-1.test-zone.example.org", "new-loadbalancer-1.lb.com", endpoint.RecordTypeCNAME, "", "ingress/default/my-ingress"), @@ -761,7 +771,7 @@ func testTXTRegistryApplyChangesWithSuffix(t *testing.T) { newEndpointWithOwner("cname-multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), }, }) - r, _ := NewTXTRegistry(p, "", "-txt", "owner", time.Hour, "wildcard", []string{}, []string{}, false, nil) + r, _ := NewTXTRegistry(p, "", "-txt", "owner", time.Hour, "wildcard", []string{}, []string{}, false, nil, false, "") changes := &plan.Changes{ Create: []*endpoint.Endpoint{ @@ -865,7 +875,7 @@ func testTXTRegistryApplyChangesNoPrefix(t *testing.T) { newEndpointWithOwner("cname-foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), }, }) - r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil) + r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil, false, "") changes := &plan.Changes{ Create: []*endpoint.Endpoint{ @@ -1028,7 +1038,7 @@ func testTXTRegistryMissingRecordsNoPrefix(t *testing.T) { }, } - r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "wc", []string{endpoint.RecordTypeCNAME, endpoint.RecordTypeA, endpoint.RecordTypeNS}, []string{}, false, nil) + r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "wc", []string{endpoint.RecordTypeCNAME, endpoint.RecordTypeA, endpoint.RecordTypeNS}, []string{}, false, nil, false, "") records, _ := r.Records(ctx) assert.True(t, testutils.SameEndpoints(records, expectedRecords)) @@ -1138,7 +1148,7 @@ func testTXTRegistryMissingRecordsWithPrefix(t *testing.T) { }, } - r, _ := NewTXTRegistry(p, "txt.", "", "owner", time.Hour, "wc", []string{endpoint.RecordTypeCNAME, endpoint.RecordTypeA, endpoint.RecordTypeNS, endpoint.RecordTypeTXT}, []string{}, false, nil) + r, _ := NewTXTRegistry(p, "txt.", "", "owner", time.Hour, "wc", []string{endpoint.RecordTypeCNAME, endpoint.RecordTypeA, endpoint.RecordTypeNS, endpoint.RecordTypeTXT}, []string{}, false, nil, false, "") records, _ := r.Records(ctx) assert.True(t, testutils.SameEndpoints(records, expectedRecords)) @@ -1426,7 +1436,7 @@ func TestNewTXTScheme(t *testing.T) { newEndpointWithOwner("cname-foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), }, }) - r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil) + r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil, false, "") changes := &plan.Changes{ Create: []*endpoint.Endpoint{ @@ -1502,7 +1512,7 @@ func TestGenerateTXT(t *testing.T) { } p := inmemory.NewInMemoryProvider() p.CreateZone(testZone) - r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil) + r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil, false, "") gotTXT := r.generateTXTRecord(record) assert.Equal(t, expectedTXT, gotTXT) } @@ -1521,7 +1531,7 @@ func TestGenerateTXTForAAAA(t *testing.T) { } p := inmemory.NewInMemoryProvider() p.CreateZone(testZone) - r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil) + r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil, false, "") gotTXT := r.generateTXTRecord(record) assert.Equal(t, expectedTXT, gotTXT) } @@ -1538,7 +1548,7 @@ func TestFailGenerateTXT(t *testing.T) { expectedTXT := []*endpoint.Endpoint{} p := inmemory.NewInMemoryProvider() p.CreateZone(testZone) - r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil) + r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil, false, "") gotTXT := r.generateTXTRecord(cnameRecord) assert.Equal(t, expectedTXT, gotTXT) } @@ -1556,7 +1566,7 @@ func TestTXTRegistryApplyChangesEncrypt(t *testing.T) { }, }) - r, _ := NewTXTRegistry(p, "txt.", "", "owner", time.Hour, "", []string{}, []string{}, true, []byte("12345678901234567890123456789012")) + r, _ := NewTXTRegistry(p, "txt.", "", "owner", time.Hour, "", []string{}, []string{}, true, []byte("12345678901234567890123456789012"), false, "") records, _ := r.Records(ctx) changes := &plan.Changes{ Delete: records, @@ -1602,7 +1612,7 @@ func TestMultiClusterDifferentRecordTypeOwnership(t *testing.T) { }, }) - r, _ := NewTXTRegistry(p, "_owner.", "", "bar", time.Hour, "", []string{}, []string{}, false, nil) + r, _ := NewTXTRegistry(p, "_owner.", "", "bar", time.Hour, "", []string{}, []string{}, false, nil, false, "") records, _ := r.Records(ctx) // new cluster has same ingress host as other cluster and uses CNAME ingress address @@ -1646,6 +1656,71 @@ func TestMultiClusterDifferentRecordTypeOwnership(t *testing.T) { } } +// TestTXTRecordMigration validates that when migration is enabled from an older owner ID the existing TXT records are updated +// or deleted according to the Provider's implementation and that we're not left with phantom records that can make the +// plan behave badly when it tries to reconciliate eventual duplicates +func TestTXTRecordMigration(t *testing.T) { + ctx := context.Background() + p := inmemory.NewInMemoryProvider() + p.CreateZone(testZone) + p.ApplyChanges(ctx, &plan.Changes{ + Create: []*endpoint.Endpoint{ + // records on cluster using A record for ingress address + newEndpointWithOwner("bar.test-zone.example.org", "1.2.3.4", endpoint.RecordTypeA, "foo"), + }, + }) + + r, _ := NewTXTRegistry(p, "%{record_type}-", "", "foo", time.Hour, "", []string{}, []string{}, false, nil, false, "") + createdRecords, _ := p.Records(ctx) + // Safe to use index here + txtRecords := r.generateTXTRecord(createdRecords[0]) + + expectedTXTRecords := []*endpoint.Endpoint{ + { + DNSName: "a-bar.test-zone.example.org", + Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=foo\""}, + RecordType: endpoint.RecordTypeTXT, + Labels: map[string]string{ + endpoint.OwnedRecordLabelKey: "bar.test-zone.example.org", + }, + }, + } + + assert.Equal(t, expectedTXTRecords, txtRecords) + + p.ApplyChanges(ctx, &plan.Changes{ + UpdateNew: []*endpoint.Endpoint{ + // records on cluster using A record for ingress address + newEndpointWithOwner("bar.test-zone.example.org", "1.2.3.4", endpoint.RecordTypeA, "foobar"), + }, + UpdateOld: []*endpoint.Endpoint{ + // records on cluster using A record for ingress address + newEndpointWithOwner("bar.test-zone.example.org", "1.2.3.4", endpoint.RecordTypeA, "foo"), + }, + }) + + updatedRecords, _ := p.Records(ctx) + + r, _ = NewTXTRegistry(p, "%{record_type}-", "", "foobar", time.Hour, "", []string{}, []string{}, false, nil, true, "foo") + + migratedTXTRecords := r.generateTXTRecord(updatedRecords[0]) + + expectedFinalTXT := []*endpoint.Endpoint{ + { + DNSName: "a-bar.test-zone.example.org", + Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=foobar\""}, + RecordType: endpoint.RecordTypeTXT, + Labels: map[string]string{ + endpoint.OwnedRecordLabelKey: "bar.test-zone.example.org", + }, + }, + } + + assert.Len(t, migratedTXTRecords, 1) + assert.Equal(t, migratedTXTRecords, expectedFinalTXT) + +} + /** helper methods