From 177a418b09ac09d94561bbb5775855077cefb2e1 Mon Sep 17 00:00:00 2001 From: Pascal Bachor Date: Sun, 20 Jul 2025 19:58:07 +0200 Subject: [PATCH 1/8] refactor: Use list of pairs instead of pair of lists in update changes --- endpoint/endpoint.go | 7 ++- plan/plan.go | 103 +++++++++++++++++++++++++++++++----- plan/policy.go | 5 +- registry/aws_sd_registry.go | 11 ++-- registry/dynamodb.go | 38 ++++++------- registry/txt.go | 52 ++++++++---------- 6 files changed, 142 insertions(+), 74 deletions(-) diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index 68f82378a..a34259561 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -111,7 +111,7 @@ func (t Targets) Swap(i, j int) { t[i], t[j] = t[j], t[i] } -// Same compares to Targets and returns true if they are identical (case-insensitive) +// Same compares two Targets and returns true if they are identical (case-insensitive) func (t Targets) Same(o Targets) bool { if len(t) != len(o) { return false @@ -373,7 +373,10 @@ func (e *Endpoint) UniqueOrderedTargets() { func FilterEndpointsByOwnerID(ownerID string, eps []*Endpoint) []*Endpoint { filtered := []*Endpoint{} for _, ep := range eps { - if endpointOwner, ok := ep.Labels[OwnerLabelKey]; !ok || endpointOwner != ownerID { + endpointOwner, ok := ep.Labels[OwnerLabelKey] + if !ok { + log.Debugf(`Skipping endpoint %v because of missing owner label (required: "%s")`, ep, ownerID) + } else if endpointOwner != ownerID { log.Debugf(`Skipping endpoint %v because owner id does not match, found: "%s", required: "%s"`, ep, endpointOwner, ownerID) } else { filtered = append(filtered, ep) diff --git a/plan/plan.go b/plan/plan.go index 8061a5b3f..43a327c53 100644 --- a/plan/plan.go +++ b/plan/plan.go @@ -17,6 +17,7 @@ limitations under the License. package plan import ( + "encoding/json" "fmt" "slices" "strings" @@ -56,13 +57,73 @@ type Plan struct { // Changes holds lists of actions to be executed by dns providers type Changes struct { // Records that need to be created - Create []*endpoint.Endpoint `json:"create,omitempty"` - // Records that need to be updated (current data) - UpdateOld []*endpoint.Endpoint `json:"updateOld,omitempty"` - // Records that need to be updated (desired data) - UpdateNew []*endpoint.Endpoint `json:"updateNew,omitempty"` + Create []*endpoint.Endpoint + // Records that need to be updated + Update []*Update // Records that need to be deleted - Delete []*endpoint.Endpoint `json:"delete,omitempty"` + Delete []*endpoint.Endpoint +} + +type Update struct { + // current data + Old *endpoint.Endpoint + // desired data + New *endpoint.Endpoint +} + +type ChangesV1 struct { + Create []*endpoint.Endpoint `json:"create,omitempty"` + UpdateOld []*endpoint.Endpoint `json:"updateOld,omitempty"` + UpdateNew []*endpoint.Endpoint `json:"updateNew,omitempty"` + Delete []*endpoint.Endpoint `json:"delete,omitempty"` +} + +func (changes *Changes) MarshalJSON() ([]byte, error) { + return json.Marshal(&ChangesV1{ + Create: changes.Create, + UpdateOld: changes.UpdateOld(), + UpdateNew: changes.UpdateNew(), + Delete: changes.Delete, + }) +} + +func MkUpdates(old []*endpoint.Endpoint, new []*endpoint.Endpoint) ([]*Update, error) { + updates := []*Update{} + if nOld, nNew := len(old), len(new); nOld != nNew { + return nil, fmt.Errorf("Number of old updates (%v) does not match number of new updates (%v)", nOld, nNew) + } + for i, old := range old { + updates = append(updates, &Update{Old: old, New: new[i]}) + } + return updates, nil +} + +func (changes *Changes) UnmarshalJSON(data []byte) error { + aux := &ChangesV1{} + if err := json.Unmarshal(data, &aux); err != nil { + return err + } + changes.Create = aux.Create + changes.Delete = aux.Delete + update, err := MkUpdates(aux.UpdateOld, aux.UpdateNew) + changes.Update = update + return err +} + +func (c *Changes) UpdateOld() []*endpoint.Endpoint { + result := []*endpoint.Endpoint{} + for _, update := range c.Update { + result = append(result, update.Old) + } + return result +} + +func (c *Changes) UpdateNew() []*endpoint.Endpoint { + result := []*endpoint.Endpoint{} + for _, update := range c.Update { + result = append(result, update.New) + } + return result } // planKey is a key for a row in `planTable`. @@ -160,7 +221,24 @@ func (c *Changes) HasChanges() bool { if len(c.Create) > 0 || len(c.Delete) > 0 { return true } - return !cmp.Equal(c.UpdateNew, c.UpdateOld) + return !cmp.Equal(c.UpdateNew(), c.UpdateOld()) +} + +func FilterUpdatesByOwnerId(ownerID string, updates []*Update) []*Update { + filtered := []*Update{} + for _, update := range updates { + // NOTE: OwnerID of `update.Old` and `update.New` will be equivalent + endpointOwner, ok := update.Old.Labels[endpoint.OwnerLabelKey] + if !ok { + log.Debugf(`Skipping update %v because of missing owner label (required: "%s")`, update, ownerID) + } else if endpointOwner != ownerID { + log.Debugf(`Skipping update %v because owner id does not match, found: "%s", required: "%s"`, update, endpointOwner, ownerID) + } else { + filtered = append(filtered, update) + } + } + + return filtered } // Calculate computes the actions needed to move current state towards desired @@ -225,8 +303,7 @@ func (p *Plan) Calculate() *Plan { if shouldUpdateTTL(update, records.current) || targetChanged(update, records.current) || p.shouldUpdateProviderSpecific(update, records.current) { inheritOwner(records.current, update) - changes.UpdateNew = append(changes.UpdateNew, update) - changes.UpdateOld = append(changes.UpdateOld, records.current) + changes.Update = append(changes.Update, &Update{Old: records.current, New: update}) } } } @@ -259,8 +336,7 @@ 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) - changes.UpdateNew = endpoint.FilterEndpointsByOwnerID(p.OwnerID, changes.UpdateNew) + changes.Update = FilterUpdatesByOwnerId(p.OwnerID, changes.Update) } plan := &Plan{ @@ -282,7 +358,10 @@ func inheritOwner(from, to *endpoint.Endpoint) { if from.Labels == nil { from.Labels = map[string]string{} } - to.Labels[endpoint.OwnerLabelKey] = from.Labels[endpoint.OwnerLabelKey] + ownerLabel, ok := from.Labels[endpoint.OwnerLabelKey] + if ok { + to.Labels[endpoint.OwnerLabelKey] = ownerLabel + } } func targetChanged(desired, current *endpoint.Endpoint) bool { diff --git a/plan/policy.go b/plan/policy.go index bbcf71e96..8da6f25aa 100644 --- a/plan/policy.go +++ b/plan/policy.go @@ -42,9 +42,8 @@ type UpsertOnlyPolicy struct{} // Apply applies the upsert-only policy which strips out any deletions. func (p *UpsertOnlyPolicy) Apply(changes *Changes) *Changes { return &Changes{ - Create: changes.Create, - UpdateOld: changes.UpdateOld, - UpdateNew: changes.UpdateNew, + Create: changes.Create, + Update: changes.Update, } } diff --git a/registry/aws_sd_registry.go b/registry/aws_sd_registry.go index a1e2103f5..0899825d3 100644 --- a/registry/aws_sd_registry.go +++ b/registry/aws_sd_registry.go @@ -75,15 +75,14 @@ func (sdr *AWSSDRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, er // inserted in the AWS SD instance as a CreateID field func (sdr *AWSSDRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) error { filteredChanges := &plan.Changes{ - Create: changes.Create, - UpdateNew: endpoint.FilterEndpointsByOwnerID(sdr.ownerID, changes.UpdateNew), - UpdateOld: endpoint.FilterEndpointsByOwnerID(sdr.ownerID, changes.UpdateOld), - Delete: endpoint.FilterEndpointsByOwnerID(sdr.ownerID, changes.Delete), + Create: changes.Create, + Update: plan.FilterUpdatesByOwnerId(sdr.ownerID, changes.Update), + Delete: endpoint.FilterEndpointsByOwnerID(sdr.ownerID, changes.Delete), } sdr.updateLabels(filteredChanges.Create) - sdr.updateLabels(filteredChanges.UpdateNew) - sdr.updateLabels(filteredChanges.UpdateOld) + sdr.updateLabels(filteredChanges.UpdateNew()) + sdr.updateLabels(filteredChanges.UpdateOld()) sdr.updateLabels(filteredChanges.Delete) return sdr.provider.ApplyChanges(ctx, filteredChanges) diff --git a/registry/dynamodb.go b/registry/dynamodb.go index 5b83ceeb0..3114f2653 100644 --- a/registry/dynamodb.go +++ b/registry/dynamodb.go @@ -224,13 +224,12 @@ func (im *DynamoDBRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, // ApplyChanges updates the DNS provider and DynamoDB table with the changes. func (im *DynamoDBRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) error { filteredChanges := &plan.Changes{ - Create: changes.Create, - UpdateNew: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.UpdateNew), - UpdateOld: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.UpdateOld), - Delete: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.Delete), + Create: changes.Create, + Update: plan.FilterUpdatesByOwnerId(im.ownerID, changes.Update), + Delete: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.Delete), } - statements := make([]dynamodbtypes.BatchStatementRequest, 0, len(filteredChanges.Create)+len(filteredChanges.UpdateNew)) + statements := make([]dynamodbtypes.BatchStatementRequest, 0, len(filteredChanges.Create)+len(filteredChanges.Update)) for _, r := range filteredChanges.Create { if r.Labels == nil { r.Labels = make(map[string]string) @@ -259,35 +258,30 @@ func (im *DynamoDBRegistry) ApplyChanges(ctx context.Context, changes *plan.Chan } } - oldLabels := make(map[endpoint.EndpointKey]endpoint.Labels, len(filteredChanges.UpdateOld)) - needMigration := map[endpoint.EndpointKey]bool{} - for _, r := range filteredChanges.UpdateOld { - oldLabels[r.Key()] = r.Labels - - if _, ok := r.GetProviderSpecificProperty(dynamodbAttributeMigrate); ok { - needMigration[r.Key()] = true + for _, r := range filteredChanges.Update { + oldLabels := r.Old.Labels + needMigration := false + if _, ok := r.Old.GetProviderSpecificProperty(dynamodbAttributeMigrate); ok { + needMigration = true } - // remove old version of record from cache if im.cacheInterval > 0 { - im.removeFromCache(r) + im.removeFromCache(r.Old) } - } - for _, r := range filteredChanges.UpdateNew { - key := r.Key() - if needMigration[key] { - statements = im.appendInsert(statements, key, r.Labels) + key := r.New.Key() + if needMigration { + statements = im.appendInsert(statements, key, r.New.Labels) // Invalidate the records cache so the next sync deletes the TXT ownership record im.recordsCache = nil } else { - statements = im.appendUpdate(statements, key, oldLabels[key], r.Labels) + statements = im.appendUpdate(statements, key, oldLabels, r.New.Labels) } // add new version of record to caches - im.labels[key] = r.Labels + im.labels[key] = r.New.Labels if im.cacheInterval > 0 { - im.addToCache(r) + im.addToCache(r.New) } } diff --git a/registry/txt.go b/registry/txt.go index 325996936..80515e97a 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -207,8 +207,7 @@ func (im *TXTRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, error if len(txtRecordsMap) > 0 && ep.Labels[endpoint.OwnerLabelKey] == im.ownerID { if plan.IsManagedRecord(ep.RecordType, im.managedRecordTypes, im.excludeRecordTypes) { // Get desired TXT records and detect the missing ones - desiredTXTs := im.generateTXTRecord(ep) - for _, desiredTXT := range desiredTXTs { + if desiredTXT := im.generateTXTRecord(ep); desiredTXT != nil { if _, exists := txtRecordsMap[desiredTXT.DNSName]; !exists { ep.WithProviderSpecific(providerSpecificForceUpdate, "true") } @@ -226,13 +225,9 @@ func (im *TXTRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, error return endpoints, nil } -// generateTXTRecord generates TXT records in either both formats (old and new) or new format only, -// depending on the newFormatOnly configuration. The old format is maintained for backwards -// compatibility but can be disabled to reduce the number of DNS records. -func (im *TXTRegistry) generateTXTRecord(r *endpoint.Endpoint) []*endpoint.Endpoint { - endpoints := make([]*endpoint.Endpoint, 0) +func (im *TXTRegistry) generateTXTRecord(r *endpoint.Endpoint) *endpoint.Endpoint { + var ep *endpoint.Endpoint - // Always create new format record recordType := r.RecordType // AWS Alias records are encoded as type "cname" if isAlias, found := r.GetProviderSpecificProperty("alias"); found && isAlias == "true" && recordType == endpoint.RecordTypeA { @@ -243,19 +238,18 @@ func (im *TXTRegistry) generateTXTRecord(r *endpoint.Endpoint) []*endpoint.Endpo txtNew.WithSetIdentifier(r.SetIdentifier) txtNew.Labels[endpoint.OwnedRecordLabelKey] = r.DNSName txtNew.ProviderSpecific = r.ProviderSpecific - endpoints = append(endpoints, txtNew) + ep = txtNew } - return endpoints + return ep } // ApplyChanges updates dns provider with the changes // for each created/deleted record it will also take into account TXT records for creation/deletion func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) error { filteredChanges := &plan.Changes{ - Create: changes.Create, - UpdateNew: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.UpdateNew), - UpdateOld: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.UpdateOld), - Delete: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.Delete), + Create: changes.Create, + Update: plan.FilterUpdatesByOwnerId(im.ownerID, changes.Update), + Delete: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.Delete), } for _, r := range filteredChanges.Create { if r.Labels == nil { @@ -263,7 +257,9 @@ func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) } r.Labels[endpoint.OwnerLabelKey] = im.ownerID - filteredChanges.Create = append(filteredChanges.Create, im.generateTXTRecord(r)...) + if txt := im.generateTXTRecord(r); txt != nil { + filteredChanges.Create = append(filteredChanges.Create, txt) + } if im.cacheInterval > 0 { im.addToCache(r) @@ -274,7 +270,9 @@ func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) // when we delete TXT records for which value has changed (due to new label) this would still work because // !!! TXT record value is uniquely generated from the Labels of the endpoint. Hence old TXT record can be uniquely reconstructed // !!! After migration to the new TXT registry format we can drop records in old format here!!! - filteredChanges.Delete = append(filteredChanges.Delete, im.generateTXTRecord(r)...) + if txt := im.generateTXTRecord(r); txt != nil { + filteredChanges.Delete = append(filteredChanges.Delete, txt) + } if im.cacheInterval > 0 { im.removeFromCache(r) @@ -282,22 +280,18 @@ func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) } // make sure TXT records are consistently updated as well - for _, r := range filteredChanges.UpdateOld { - // when we updateOld TXT records for which value has changed (due to new label) this would still work because + for _, r := range filteredChanges.Update { + // when we update old TXT records for which value has changed (due to new label) this would still work because // !!! TXT record value is uniquely generated from the Labels of the endpoint. Hence old TXT record can be uniquely reconstructed - filteredChanges.UpdateOld = append(filteredChanges.UpdateOld, im.generateTXTRecord(r)...) - // remove old version of record from cache - if im.cacheInterval > 0 { - im.removeFromCache(r) + // NOTE: Whether `generateTXTRecord` returns `nil` depends only on DNSName, which will be the same for `old` and `new` + if old, new := im.generateTXTRecord(r.Old), im.generateTXTRecord(r.New); old != nil && new != nil { + filteredChanges.Update = append(filteredChanges.Update, &plan.Update{Old: old, New: new}) } - } - - // make sure TXT records are consistently updated as well - for _, r := range filteredChanges.UpdateNew { - filteredChanges.UpdateNew = append(filteredChanges.UpdateNew, im.generateTXTRecord(r)...) - // add new version of record to cache if im.cacheInterval > 0 { - im.addToCache(r) + // remove old version of record from cache + im.removeFromCache(r.Old) + // add new version of record to cache + im.addToCache(r.New) } } From 4e18b4b60efc6583412f53af0d742d81256b0e63 Mon Sep 17 00:00:00 2001 From: Pascal Bachor Date: Wed, 23 Jul 2025 19:30:51 +0200 Subject: [PATCH 2/8] refactor: Use list of pairs instead of pair of lists in update changes (adjust use sites) --- controller/controller_test.go | 48 ++-- plan/plan_test.go | 134 +++++------ plan/policy_test.go | 27 ++- provider/akamai/akamai.go | 9 +- provider/akamai/akamai_test.go | 8 +- provider/alibabacloud/alibaba_cloud.go | 6 +- provider/alibabacloud/alibaba_cloud_test.go | 36 +-- provider/aws/aws.go | 2 +- provider/aws/aws_test.go | 19 +- provider/awssd/aws_sd.go | 6 +- provider/awssd/aws_sd_test.go | 5 +- provider/azure/azure.go | 2 +- provider/azure/azure_private_dns.go | 4 +- provider/azure/azure_privatedns_test.go | 23 +- provider/azure/azure_test.go | 27 ++- provider/civo/civo.go | 2 +- provider/civo/civo_test.go | 18 +- provider/cloudflare/cloudflare.go | 5 +- .../cloudflare/cloudflare_regional_test.go | 60 ++--- provider/cloudflare/cloudflare_test.go | 103 ++++---- provider/coredns/coredns.go | 7 +- provider/coredns/coredns_test.go | 24 +- provider/digitalocean/digital_ocean.go | 2 +- provider/digitalocean/digital_ocean_test.go | 8 +- provider/dnsimple/dnsimple.go | 4 +- provider/dnsimple/dnsimple_test.go | 10 +- provider/exoscale/exoscale.go | 10 +- provider/exoscale/exoscale_test.go | 41 ++-- provider/gandi/gandi.go | 4 +- provider/gandi/gandi_test.go | 34 ++- provider/godaddy/godaddy.go | 12 +- provider/google/google.go | 4 +- provider/google/google_test.go | 19 +- provider/inmemory/inmemory.go | 42 ++-- provider/inmemory/inmemory_test.go | 204 ++++++++-------- provider/linode/linode.go | 2 +- provider/linode/linode_test.go | 35 +-- provider/ns1/ns1.go | 4 +- provider/ns1/ns1_test.go | 8 +- provider/oci/oci.go | 4 +- provider/oci/oci_test.go | 84 ++++--- provider/ovh/ovh.go | 34 +-- provider/ovh/ovh_test.go | 40 ++-- provider/pdns/pdns.go | 9 +- provider/pihole/pihole.go | 4 +- provider/pihole/piholeV6_test.go | 18 +- provider/pihole/pihole_test.go | 11 +- provider/plural/plural.go | 2 +- provider/rfc2136/rfc2136.go | 9 +- provider/rfc2136/rfc2136_test.go | 49 ++-- provider/scaleway/scaleway.go | 4 +- provider/scaleway/scaleway_test.go | 56 ++--- provider/transip/transip.go | 2 +- provider/webhook/api/httpapi_test.go | 2 +- registry/aws_sd_registry_test.go | 28 +-- registry/dynamodb_test.go | 147 ++++++------ registry/noop_test.go | 23 +- registry/txt_encryption_test.go | 43 ++-- registry/txt_test.go | 224 +++++++++--------- 59 files changed, 955 insertions(+), 856 deletions(-) diff --git a/controller/controller_test.go b/controller/controller_test.go index 02cd847fe..28499ace6 100644 --- a/controller/controller_test.go +++ b/controller/controller_test.go @@ -86,11 +86,11 @@ func (p *mockProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) return err } - if err := verifyEndpoints(changes.UpdateNew, p.ExpectChanges.UpdateNew); err != nil { + if err := verifyEndpoints(changes.UpdateNew(), p.ExpectChanges.UpdateNew()); err != nil { return err } - if err := verifyEndpoints(changes.UpdateOld, p.ExpectChanges.UpdateOld); err != nil { + if err := verifyEndpoints(changes.UpdateOld(), p.ExpectChanges.UpdateOld()); err != nil { return err } @@ -194,13 +194,15 @@ func getTestProvider() provider.Provider { {DNSName: "create-aaaa-record", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"2001:DB8::1"}}, {DNSName: "create-record", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"1.2.3.4"}}, }, - UpdateNew: []*endpoint.Endpoint{ - {DNSName: "update-aaaa-record", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"2001:DB8::2"}}, - {DNSName: "update-record", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"8.8.4.4"}}, - }, - UpdateOld: []*endpoint.Endpoint{ - {DNSName: "update-aaaa-record", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"2001:DB8::3"}}, - {DNSName: "update-record", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"8.8.8.8"}}, + Update: []*plan.Update{ + { + New: &endpoint.Endpoint{DNSName: "update-aaaa-record", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"2001:DB8::2"}}, + Old: &endpoint.Endpoint{DNSName: "update-aaaa-record", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"2001:DB8::3"}}, + }, + { + New: &endpoint.Endpoint{DNSName: "update-record", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"8.8.4.4"}}, + Old: &endpoint.Endpoint{DNSName: "update-record", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"8.8.8.8"}}, + }, }, Delete: []*endpoint.Endpoint{ {DNSName: "delete-aaaa-record", RecordType: endpoint.RecordTypeAAAA, Targets: endpoint.Targets{"2001:DB8::4"}}, @@ -461,21 +463,21 @@ func TestWhenMultipleControllerConsidersAllFilteredComain(t *testing.T) { Targets: endpoint.Targets{"1.2.3.4"}, }, }, - UpdateOld: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "some-record.used.tld", - RecordType: endpoint.RecordTypeA, - Targets: endpoint.Targets{"8.8.8.8"}, - Labels: endpoint.Labels{}, - }, - }, - UpdateNew: []*endpoint.Endpoint{ - { - DNSName: "some-record.used.tld", - RecordType: endpoint.RecordTypeA, - Targets: endpoint.Targets{"1.1.1.1"}, - Labels: endpoint.Labels{ - "owner": "", + Old: &endpoint.Endpoint{ + + DNSName: "some-record.used.tld", + RecordType: endpoint.RecordTypeA, + Targets: endpoint.Targets{"8.8.8.8"}, + Labels: endpoint.Labels{}, + }, + New: &endpoint.Endpoint{ + + DNSName: "some-record.used.tld", + RecordType: endpoint.RecordTypeA, + Targets: endpoint.Targets{"1.1.1.1"}, + Labels: endpoint.Labels{}, }, }, }, diff --git a/plan/plan_test.go b/plan/plan_test.go index 10ce5a992..5548590a6 100644 --- a/plan/plan_test.go +++ b/plan/plan_test.go @@ -256,14 +256,14 @@ func TestPlan_ChangesJson_DecodeEncode(t *testing.T) { DNSName: "foo", }, }, - UpdateOld: []*endpoint.Endpoint{ + Update: []*Update{ { - DNSName: "bar", - }, - }, - UpdateNew: []*endpoint.Endpoint{ - { - DNSName: "baz", + Old: &endpoint.Endpoint{ + DNSName: "bar", + }, + New: &endpoint.Endpoint{ + DNSName: "baz", + }, }, }, Delete: []*endpoint.Endpoint{ @@ -308,8 +308,8 @@ func (suite *PlanTestSuite) TestSyncFirstRound() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -330,8 +330,8 @@ func (suite *PlanTestSuite) TestSyncSecondRound() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -352,8 +352,8 @@ func (suite *PlanTestSuite) TestSyncSecondRoundMigration() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -374,8 +374,8 @@ func (suite *PlanTestSuite) TestSyncSecondRoundWithTTLChange() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -396,8 +396,8 @@ func (suite *PlanTestSuite) TestSyncSecondRoundWithProviderSpecificChange() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -448,8 +448,8 @@ func (suite *PlanTestSuite) TestSyncSecondRoundWithProviderSpecificRemoval() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -470,8 +470,8 @@ func (suite *PlanTestSuite) TestSyncSecondRoundWithProviderSpecificAddition() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -502,8 +502,8 @@ func (suite *PlanTestSuite) TestSyncSecondRoundWithOwnerInherited() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -523,8 +523,8 @@ func (suite *PlanTestSuite) TestIdempotency() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -546,8 +546,8 @@ func (suite *PlanTestSuite) TestRecordTypeChange() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -569,8 +569,8 @@ func (suite *PlanTestSuite) TestExistingCNameWithDualStackDesired() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -594,8 +594,8 @@ func (suite *PlanTestSuite) TestExistingDualStackWithCNameDesired() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -622,8 +622,8 @@ func (suite *PlanTestSuite) TestExistingOwnerNotMatchingDualStackDesired() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -650,8 +650,8 @@ func (suite *PlanTestSuite) TestConflictingCurrentNonConflictingDesired() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -678,8 +678,8 @@ func (suite *PlanTestSuite) TestConflictingCurrentNoDesired() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -705,8 +705,8 @@ func (suite *PlanTestSuite) TestCurrentWithConflictingDesired() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -730,8 +730,8 @@ func (suite *PlanTestSuite) TestNoCurrentWithConflictingDesired() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -752,8 +752,8 @@ func (suite *PlanTestSuite) TestIgnoreTXT() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -775,8 +775,8 @@ func (suite *PlanTestSuite) TestExcludeTXT() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -796,8 +796,8 @@ func (suite *PlanTestSuite) TestIgnoreTargetCase() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -818,8 +818,8 @@ func (suite *PlanTestSuite) TestRemoveEndpoint() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -840,8 +840,8 @@ func (suite *PlanTestSuite) TestRemoveEndpointWithUpsert() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -862,8 +862,8 @@ func (suite *PlanTestSuite) TestMultipleRecordsSameNameDifferentSetIdentifier() 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -884,8 +884,8 @@ func (suite *PlanTestSuite) TestSetIdentifierUpdateCreatesAndDeletes() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -908,8 +908,8 @@ func (suite *PlanTestSuite) TestDomainFiltersInitial() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -932,8 +932,8 @@ func (suite *PlanTestSuite) TestDomainFiltersUpdate() { 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.UpdateNew(), expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld(), expectedUpdateOld) validateEntries(suite.T(), changes.Delete, expectedDelete) } @@ -953,8 +953,8 @@ func (suite *PlanTestSuite) TestAAAARecords() { changes := p.Calculate().Changes validateEntries(suite.T(), changes.Create, expectedCreate) validateEntries(suite.T(), changes.Delete, expectNoChanges) - validateEntries(suite.T(), changes.UpdateOld, expectNoChanges) - validateEntries(suite.T(), changes.UpdateNew, expectNoChanges) + validateEntries(suite.T(), changes.UpdateOld(), expectNoChanges) + validateEntries(suite.T(), changes.UpdateNew(), expectNoChanges) } func (suite *PlanTestSuite) TestDualStackRecords() { @@ -973,8 +973,8 @@ func (suite *PlanTestSuite) TestDualStackRecords() { changes := p.Calculate().Changes validateEntries(suite.T(), changes.Create, expectedCreate) validateEntries(suite.T(), changes.Delete, expectNoChanges) - validateEntries(suite.T(), changes.UpdateOld, expectNoChanges) - validateEntries(suite.T(), changes.UpdateNew, expectNoChanges) + validateEntries(suite.T(), changes.UpdateOld(), expectNoChanges) + validateEntries(suite.T(), changes.UpdateNew(), expectNoChanges) } func (suite *PlanTestSuite) TestDualStackRecordsDelete() { @@ -993,8 +993,8 @@ func (suite *PlanTestSuite) TestDualStackRecordsDelete() { changes := p.Calculate().Changes validateEntries(suite.T(), changes.Delete, expectedDelete) validateEntries(suite.T(), changes.Create, expectNoChanges) - validateEntries(suite.T(), changes.UpdateOld, expectNoChanges) - validateEntries(suite.T(), changes.UpdateNew, expectNoChanges) + validateEntries(suite.T(), changes.UpdateOld(), expectNoChanges) + validateEntries(suite.T(), changes.UpdateNew(), expectNoChanges) } func (suite *PlanTestSuite) TestDualStackToSingleStack() { @@ -1013,8 +1013,8 @@ func (suite *PlanTestSuite) TestDualStackToSingleStack() { changes := p.Calculate().Changes validateEntries(suite.T(), changes.Delete, expectedDelete) validateEntries(suite.T(), changes.Create, expectNoChanges) - validateEntries(suite.T(), changes.UpdateOld, expectNoChanges) - validateEntries(suite.T(), changes.UpdateNew, expectNoChanges) + validateEntries(suite.T(), changes.UpdateOld(), expectNoChanges) + validateEntries(suite.T(), changes.UpdateNew(), expectNoChanges) } func TestPlan(t *testing.T) { diff --git a/plan/policy_test.go b/plan/policy_test.go index e91e05173..e56f72012 100644 --- a/plan/policy_test.go +++ b/plan/policy_test.go @@ -27,10 +27,13 @@ import ( func TestApply(t *testing.T) { // empty list of records empty := []*endpoint.Endpoint{} - // a simple entry - fooV1 := []*endpoint.Endpoint{{DNSName: "foo", Targets: endpoint.Targets{"v1"}}} - // the same entry but with different target - fooV2 := []*endpoint.Endpoint{{DNSName: "foo", Targets: endpoint.Targets{"v2"}}} + // a simple entry with target change + foo := []*Update{ + { + Old: &endpoint.Endpoint{DNSName: "foo", Targets: endpoint.Targets{"v1"}}, + New: &endpoint.Endpoint{DNSName: "foo", Targets: endpoint.Targets{"v2"}}, + }, + } // another two simple entries bar := []*endpoint.Endpoint{{DNSName: "bar", Targets: endpoint.Targets{"v1"}}} baz := []*endpoint.Endpoint{{DNSName: "baz", Targets: endpoint.Targets{"v1"}}} @@ -43,20 +46,20 @@ func TestApply(t *testing.T) { { // SyncPolicy doesn't modify the set of changes. &SyncPolicy{}, - &Changes{Create: baz, UpdateOld: fooV1, UpdateNew: fooV2, Delete: bar}, - &Changes{Create: baz, UpdateOld: fooV1, UpdateNew: fooV2, Delete: bar}, + &Changes{Create: baz, Update: foo, Delete: bar}, + &Changes{Create: baz, Update: foo, Delete: bar}, }, { // UpsertOnlyPolicy clears the list of deletions. &UpsertOnlyPolicy{}, - &Changes{Create: baz, UpdateOld: fooV1, UpdateNew: fooV2, Delete: bar}, - &Changes{Create: baz, UpdateOld: fooV1, UpdateNew: fooV2, Delete: empty}, + &Changes{Create: baz, Update: foo, Delete: bar}, + &Changes{Create: baz, Update: foo, Delete: empty}, }, { // CreateOnlyPolicy clears the list of updates and deletions. &CreateOnlyPolicy{}, - &Changes{Create: baz, UpdateOld: fooV1, UpdateNew: fooV2, Delete: bar}, - &Changes{Create: baz, UpdateOld: empty, UpdateNew: empty, Delete: empty}, + &Changes{Create: baz, Update: foo, Delete: bar}, + &Changes{Create: baz, Update: []*Update{}, Delete: empty}, }, } { // apply policy @@ -64,8 +67,8 @@ func TestApply(t *testing.T) { // validate changes after applying policy validateEntries(t, changes.Create, tc.expected.Create) - validateEntries(t, changes.UpdateOld, tc.expected.UpdateOld) - validateEntries(t, changes.UpdateNew, tc.expected.UpdateNew) + validateEntries(t, changes.UpdateOld(), tc.expected.UpdateOld()) + validateEntries(t, changes.UpdateNew(), tc.expected.UpdateNew()) validateEntries(t, changes.Delete, tc.expected.Delete) } } diff --git a/provider/akamai/akamai.go b/provider/akamai/akamai.go index b513150bf..23fe53789 100644 --- a/provider/akamai/akamai.go +++ b/provider/akamai/akamai.go @@ -279,14 +279,15 @@ func (p AkamaiProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) return err } // Update recordsets - log.Debugf("Update Changes requested [%v]", changes.UpdateNew) - if err := p.updateNewRecordsets(zoneNameIDMapper, changes.UpdateNew); err != nil { + updateChanges := changes.UpdateNew() + log.Debugf("Update Changes requested [%v]", updateChanges) + if err := p.updateNewRecordsets(zoneNameIDMapper, updateChanges); err != nil { return err } // Check that all old endpoints were accounted for revRecs := changes.Delete - revRecs = append(revRecs, changes.UpdateNew...) - for _, rec := range changes.UpdateOld { + revRecs = append(revRecs, updateChanges...) + for _, rec := range changes.UpdateOld() { found := false for _, r := range revRecs { if rec.DNSName == r.DNSName { diff --git a/provider/akamai/akamai_test.go b/provider/akamai/akamai_test.go index 21d414c96..839045f5b 100644 --- a/provider/akamai/akamai_test.go +++ b/provider/akamai/akamai_test.go @@ -383,8 +383,12 @@ func TestAkamaiApplyChanges(t *testing.T) { {DNSName: "another.example.com", RecordType: "A", Targets: endpoint.Targets{"target"}}, } changes.Delete = []*endpoint.Endpoint{{DNSName: "delete.example.com", RecordType: "A", Targets: endpoint.Targets{"target"}, RecordTTL: 300}} - changes.UpdateOld = []*endpoint.Endpoint{{DNSName: "old.example.com", RecordType: "A", Targets: endpoint.Targets{"target-old"}, RecordTTL: 300}} - changes.UpdateNew = []*endpoint.Endpoint{{DNSName: "update.example.com", Targets: endpoint.Targets{"target-new"}, RecordType: "CNAME", RecordTTL: 300}} + changes.Update = []*plan.Update{ + { + Old: &endpoint.Endpoint{DNSName: "old.example.com", RecordType: "A", Targets: endpoint.Targets{"target-old"}, RecordTTL: 300}, + New: &endpoint.Endpoint{DNSName: "update.example.com", Targets: endpoint.Targets{"target-new"}, RecordType: "CNAME", RecordTTL: 300}, + }, + } apply := c.ApplyChanges(context.Background(), changes) assert.NoError(t, apply) } diff --git a/provider/alibabacloud/alibaba_cloud.go b/provider/alibabacloud/alibaba_cloud.go index fcb750f4f..518669560 100644 --- a/provider/alibabacloud/alibaba_cloud.go +++ b/provider/alibabacloud/alibaba_cloud.go @@ -296,7 +296,7 @@ func (p *AlibabaCloudProvider) Records(ctx context.Context) ([]*endpoint.Endpoin // // Returns nil if the operation was successful or an error if the operation failed. func (p *AlibabaCloudProvider) ApplyChanges(_ context.Context, changes *plan.Changes) error { - if changes == nil || len(changes.Create)+len(changes.Delete)+len(changes.UpdateNew) == 0 { + if changes == nil || len(changes.Create)+len(changes.Delete)+len(changes.Update) == 0 { // No op return nil } @@ -481,7 +481,7 @@ func (p *AlibabaCloudProvider) applyChangesForDNS(changes *plan.Changes) error { p.createRecords(changes.Create, hostedZoneDomains) p.deleteRecords(recordMap, changes.Delete) - p.updateRecords(recordMap, changes.UpdateNew, hostedZoneDomains) + p.updateRecords(recordMap, changes.UpdateNew(), hostedZoneDomains) return nil } @@ -989,7 +989,7 @@ func (p *AlibabaCloudProvider) applyChangesForPrivateZone(changes *plan.Changes) p.createPrivateZoneRecords(zones, changes.Create) p.deletePrivateZoneRecords(zones, changes.Delete) - p.updatePrivateZoneRecords(zones, changes.UpdateNew) + p.updatePrivateZoneRecords(zones, changes.UpdateNew()) return nil } diff --git a/provider/alibabacloud/alibaba_cloud_test.go b/provider/alibabacloud/alibaba_cloud_test.go index 9278e1514..12df493d7 100644 --- a/provider/alibabacloud/alibaba_cloud_test.go +++ b/provider/alibabacloud/alibaba_cloud_test.go @@ -281,12 +281,14 @@ func TestAlibabaCloudProvider_ApplyChanges(t *testing.T) { }, defaultTtlPlan, }, - UpdateNew: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "abc.container-service.top", - RecordType: "A", - RecordTTL: 500, - Targets: endpoint.NewTargets("1.2.3.4", "5.6.7.8"), + New: &endpoint.Endpoint{ + DNSName: "abc.container-service.top", + RecordType: "A", + RecordTTL: 500, + Targets: endpoint.NewTargets("1.2.3.4", "5.6.7.8"), + }, }, }, Delete: []*endpoint.Endpoint{ @@ -339,12 +341,14 @@ func TestAlibabaCloudProvider_ApplyChanges_HaveNoDefinedZoneDomain(t *testing.T) }, defaultTtlPlan, }, - UpdateNew: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "abc.container-service.top", - RecordType: "A", - RecordTTL: 500, - Targets: endpoint.NewTargets("1.2.3.4", "5.6.7.8"), + New: &endpoint.Endpoint{ + DNSName: "abc.container-service.top", + RecordType: "A", + RecordTTL: 500, + Targets: endpoint.NewTargets("1.2.3.4", "5.6.7.8"), + }, }, }, Delete: []*endpoint.Endpoint{ @@ -405,12 +409,14 @@ func TestAlibabaCloudProvider_ApplyChanges_PrivateZone(t *testing.T) { Targets: endpoint.NewTargets("4.3.2.1"), }, }, - UpdateNew: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "abc.container-service.top", - RecordType: "A", - RecordTTL: 500, - Targets: endpoint.NewTargets("1.2.3.4", "5.6.7.8"), + New: &endpoint.Endpoint{ + DNSName: "abc.container-service.top", + RecordType: "A", + RecordTTL: 500, + Targets: endpoint.NewTargets("1.2.3.4", "5.6.7.8"), + }, }, }, Delete: []*endpoint.Endpoint{ diff --git a/provider/aws/aws.go b/provider/aws/aws.go index 1ddb35cce..2277d4e0a 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -684,7 +684,7 @@ func (p *AWSProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) e return provider.NewSoftErrorf("failed to list zones, not applying changes: %w", err) } - updateChanges := p.createUpdateChanges(changes.UpdateNew, changes.UpdateOld) + updateChanges := p.createUpdateChanges(changes.UpdateNew(), changes.UpdateOld()) combinedChanges := make(Route53Changes, 0, len(changes.Delete)+len(changes.Create)+len(updateChanges)) combinedChanges = append(combinedChanges, p.newChanges(route53types.ChangeActionCreate, changes.Create)...) diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index a269c3ede..f7235a90f 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -1008,7 +1008,6 @@ func TestAWSApplyChanges(t *testing.T) { endpoint.NewEndpoint("set-identifier-change.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4").WithSetIdentifier("before").WithProviderSpecific(providerSpecificWeight, "10"), endpoint.NewEndpoint("set-identifier-no-change.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4").WithSetIdentifier("no-change").WithProviderSpecific(providerSpecificWeight, "10"), endpoint.NewEndpoint("update-test-mx.zone-2.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeMX, "10 mailhost2.bar.elb.amazonaws.com"), - endpoint.NewEndpoint("escape-%!s()-codes.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4").WithSetIdentifier("policy-change").WithSetIdentifier("no-change").WithProviderSpecific(providerSpecificWeight, "10"), } updatedRecords := []*endpoint.Endpoint{ endpoint.NewEndpoint("update-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "1.2.3.4"), @@ -1048,11 +1047,12 @@ func TestAWSApplyChanges(t *testing.T) { endpoint.NewEndpoint("delete-test-mx.zone-2.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeMX, "30 mailhost1.foo.elb.amazonaws.com"), } + update, err := plan.MkUpdates(currentRecords, updatedRecords) + assert.NoError(t, err) changes := &plan.Changes{ - Create: createRecords, - UpdateNew: updatedRecords, - UpdateOld: currentRecords, - Delete: deleteRecords, + Create: createRecords, + Update: update, + Delete: deleteRecords, } ctx := tt.setup(provider) @@ -1417,11 +1417,12 @@ func TestAWSApplyChangesDryRun(t *testing.T) { endpoint.NewEndpoint("delete-test-mx.zone-2.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeMX, "10 mail.bar.elb.amazonaws.com"), } + update, err := plan.MkUpdates(currentRecords, updatedRecords) + assert.NoError(t, err) changes := &plan.Changes{ - Create: createRecords, - UpdateNew: updatedRecords, - UpdateOld: currentRecords, - Delete: deleteRecords, + Create: createRecords, + Update: update, + Delete: deleteRecords, } ctx := context.Background() diff --git a/provider/awssd/aws_sd.go b/provider/awssd/aws_sd.go index 510fee810..9be294536 100644 --- a/provider/awssd/aws_sd.go +++ b/provider/awssd/aws_sd.go @@ -218,7 +218,7 @@ func (p *AWSSDProvider) instancesToEndpoint(ns *sdtypes.NamespaceSummary, srv *s // ApplyChanges applies Kubernetes changes in endpoints to AWS API func (p *AWSSDProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) error { // return early if there is nothing to change - if len(changes.Create) == 0 && len(changes.Delete) == 0 && len(changes.UpdateNew) == 0 { + if len(changes.Create) == 0 && len(changes.Delete) == 0 && len(changes.Update) == 0 { log.Info("All records are already up to date") return nil } @@ -248,13 +248,13 @@ func (p *AWSSDProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) func (p *AWSSDProvider) updatesToCreates(changes *plan.Changes) ([]*endpoint.Endpoint, []*endpoint.Endpoint) { updateNewMap := map[string]*endpoint.Endpoint{} - for _, e := range changes.UpdateNew { + for _, e := range changes.UpdateNew() { updateNewMap[e.DNSName] = e } var creates, deletes []*endpoint.Endpoint - for _, old := range changes.UpdateOld { + for _, old := range changes.UpdateOld() { current := updateNewMap[old.DNSName] if !old.Targets.Same(current.Targets) { diff --git a/provider/awssd/aws_sd_test.go b/provider/awssd/aws_sd_test.go index 3bf592200..5d55ad59f 100644 --- a/provider/awssd/aws_sd_test.go +++ b/provider/awssd/aws_sd_test.go @@ -261,9 +261,10 @@ func TestAWSSDProvider_ApplyChanges_Update(t *testing.T) { ctx = context.Background() // apply update + update, err := plan.MkUpdates(oldEndpoints, newEndpoints) + assert.NoError(t, err) provider.ApplyChanges(ctx, &plan.Changes{ - UpdateOld: oldEndpoints, - UpdateNew: newEndpoints, + Update: update, }) // make sure services were created diff --git a/provider/azure/azure.go b/provider/azure/azure.go index d53efa32f..a9201b5ef 100644 --- a/provider/azure/azure.go +++ b/provider/azure/azure.go @@ -242,7 +242,7 @@ func (p *AzureProvider) mapChanges(zones []dns.Zone, changes *plan.Changes) (azu mapChange(updated, change) } - for _, change := range changes.UpdateNew { + for _, change := range changes.UpdateNew() { mapChange(updated, change) } return deleted, updated diff --git a/provider/azure/azure_private_dns.go b/provider/azure/azure_private_dns.go index 2e0fefd33..a543a25fd 100644 --- a/provider/azure/azure_private_dns.go +++ b/provider/azure/azure_private_dns.go @@ -169,7 +169,7 @@ func (p *AzurePrivateDNSProvider) Records(ctx context.Context) ([]*endpoint.Endp // // Returns nil if the operation was successful or an error if the operation failed. func (p *AzurePrivateDNSProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) error { - log.Debugf("Received %d changes to process", len(changes.Create)+len(changes.Delete)+len(changes.UpdateNew)+len(changes.UpdateOld)) + log.Debugf("Received %d changes to process", len(changes.Create)+len(changes.Delete)+len(changes.Update)) zones, err := p.zones(ctx) if err != nil { @@ -246,7 +246,7 @@ func (p *AzurePrivateDNSProvider) mapChanges(zones []privatedns.PrivateZone, cha mapChange(updated, change) } - for _, change := range changes.UpdateNew { + for _, change := range changes.UpdateNew() { mapChange(updated, change) } return deleted, updated diff --git a/provider/azure/azure_privatedns_test.go b/provider/azure/azure_privatedns_test.go index 3366cc0da..42f67b81e 100644 --- a/provider/azure/azure_privatedns_test.go +++ b/provider/azure/azure_privatedns_test.go @@ -23,6 +23,7 @@ import ( azcoreruntime "github.com/Azure/azure-sdk-for-go/sdk/azcore/runtime" "github.com/Azure/azure-sdk-for-go/sdk/azcore/to" privatedns "github.com/Azure/azure-sdk-for-go/sdk/resourcemanager/privatedns/armprivatedns" + "github.com/stretchr/testify/assert" "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/plan" "sigs.k8s.io/external-dns/provider" @@ -403,8 +404,10 @@ func testAzurePrivateDNSApplyChangesInternal(t *testing.T, dryRun bool, client P currentRecords := []*endpoint.Endpoint{ endpoint.NewEndpoint("old.example.com", endpoint.RecordTypeA, "121.212.121.212"), + endpoint.NewEndpoint("old.example.com", endpoint.RecordTypeA, "2001::111:222:111:222"), endpoint.NewEndpoint("oldcname.example.com", endpoint.RecordTypeCNAME, "other.com"), endpoint.NewEndpoint("old.nope.com", endpoint.RecordTypeA, "121.212.121.212"), + endpoint.NewEndpoint("old.nope.com", endpoint.RecordTypeAAAA, "2001::222:111:222:111"), endpoint.NewEndpoint("oldmail.example.com", endpoint.RecordTypeMX, "20 foo.other.com"), } updatedRecords := []*endpoint.Endpoint{ @@ -424,11 +427,12 @@ func testAzurePrivateDNSApplyChangesInternal(t *testing.T, dryRun bool, client P endpoint.NewEndpoint("deleted.nope.com", endpoint.RecordTypeAAAA, "2001::222:111:222:111"), } + update, err := plan.MkUpdates(currentRecords, updatedRecords) + assert.NoError(t, err) changes := &plan.Changes{ - Create: createRecords, - UpdateNew: updatedRecords, - UpdateOld: currentRecords, - Delete: deleteRecords, + Create: createRecords, + Update: update, + Delete: deleteRecords, } if err := provider.ApplyChanges(context.Background(), changes); err != nil { @@ -527,8 +531,10 @@ func testAzurePrivateDNSApplyChangesInternalZoneName(t *testing.T, dryRun bool, currentRecords := []*endpoint.Endpoint{ endpoint.NewEndpoint("old.foo.example.com", endpoint.RecordTypeA, "121.212.121.212"), + endpoint.NewEndpoint("old.foo.example.com", endpoint.RecordTypeAAAA, "2001::111:222:111:222"), endpoint.NewEndpoint("oldcname.foo.example.com", endpoint.RecordTypeCNAME, "other.com"), endpoint.NewEndpoint("old.nope.example.com", endpoint.RecordTypeA, "121.212.121.212"), + endpoint.NewEndpoint("old.nope.example.com", endpoint.RecordTypeA, "2001::222:111:222:111"), } updatedRecords := []*endpoint.Endpoint{ endpoint.NewEndpointWithTTL("new.foo.example.com", endpoint.RecordTypeA, 3600, "111.222.111.222"), @@ -545,11 +551,12 @@ func testAzurePrivateDNSApplyChangesInternalZoneName(t *testing.T, dryRun bool, endpoint.NewEndpoint("deleted.nope.example.com", endpoint.RecordTypeA, "222.111.222.111"), } + update, err := plan.MkUpdates(currentRecords, updatedRecords) + assert.NoError(t, err) changes := &plan.Changes{ - Create: createRecords, - UpdateNew: updatedRecords, - UpdateOld: currentRecords, - Delete: deleteRecords, + Create: createRecords, + Update: update, + Delete: deleteRecords, } if err := provider.ApplyChanges(context.Background(), changes); err != nil { diff --git a/provider/azure/azure_test.go b/provider/azure/azure_test.go index a55070e26..ba408096c 100644 --- a/provider/azure/azure_test.go +++ b/provider/azure/azure_test.go @@ -443,9 +443,12 @@ func testAzureApplyChangesInternal(t *testing.T, dryRun bool, client RecordSetsC currentRecords := []*endpoint.Endpoint{ endpoint.NewEndpoint("old.example.com", endpoint.RecordTypeA, "121.212.121.212"), + endpoint.NewEndpoint("old.example.com", endpoint.RecordTypeA, "2001::111:222:111:222"), endpoint.NewEndpoint("oldcname.example.com", endpoint.RecordTypeCNAME, "other.com"), - endpoint.NewEndpoint("oldcloud.example.com", endpoint.RecordTypeNS, "ns1.example.com."), + endpoint.NewEndpoint("oldns.example.com", endpoint.RecordTypeNS, "ns1.example.com."), endpoint.NewEndpoint("old.nope.com", endpoint.RecordTypeA, "121.212.121.212"), + endpoint.NewEndpoint("old.nope.com", endpoint.RecordTypeA, "121.212.121.212"), + endpoint.NewEndpoint("oldns.nope.com", endpoint.RecordTypeNS, "ns1.example.com"), endpoint.NewEndpoint("oldmail.example.com", endpoint.RecordTypeMX, "20 foo.other.com"), } updatedRecords := []*endpoint.Endpoint{ @@ -469,11 +472,12 @@ func testAzureApplyChangesInternal(t *testing.T, dryRun bool, client RecordSetsC endpoint.NewEndpoint("deletedns.nope.com", endpoint.RecordTypeNS, "ns1.example.com."), } + update, err := plan.MkUpdates(currentRecords, updatedRecords) + assert.NoError(t, err) changes := &plan.Changes{ - Create: createRecords, - UpdateNew: updatedRecords, - UpdateOld: currentRecords, - Delete: deleteRecords, + Create: createRecords, + Update: update, + Delete: deleteRecords, } if err := provider.ApplyChanges(context.Background(), changes); err != nil { @@ -580,8 +584,12 @@ func testAzureApplyChangesInternalZoneName(t *testing.T, dryRun bool, client Rec currentRecords := []*endpoint.Endpoint{ endpoint.NewEndpoint("old.foo.example.com", endpoint.RecordTypeA, "121.212.121.212"), + endpoint.NewEndpoint("old.foo.example.com", endpoint.RecordTypeAAAA, "2001::111:222:111:222"), endpoint.NewEndpoint("oldcname.foo.example.com", endpoint.RecordTypeCNAME, "other.com"), + endpoint.NewEndpoint("oldns.foo.example.com", endpoint.RecordTypeNS, "ns1.foo.example.com."), endpoint.NewEndpoint("old.nope.example.com", endpoint.RecordTypeA, "121.212.121.212"), + endpoint.NewEndpoint("old.nope.example.com", endpoint.RecordTypeAAAA, "2001::222:111:222:111"), + endpoint.NewEndpoint("oldns.nope.example.com", endpoint.RecordTypeNS, "ns1.nope.example.com."), } updatedRecords := []*endpoint.Endpoint{ endpoint.NewEndpointWithTTL("new.foo.example.com", endpoint.RecordTypeA, 3600, "111.222.111.222"), @@ -601,11 +609,12 @@ func testAzureApplyChangesInternalZoneName(t *testing.T, dryRun bool, client Rec endpoint.NewEndpoint("deleted.nope.example.com", endpoint.RecordTypeA, "222.111.222.111"), } + update, err := plan.MkUpdates(currentRecords, updatedRecords) + assert.NoError(t, err) changes := &plan.Changes{ - Create: createRecords, - UpdateNew: updatedRecords, - UpdateOld: currentRecords, - Delete: deleteRecords, + Create: createRecords, + Update: update, + Delete: deleteRecords, } if err := provider.ApplyChanges(context.Background(), changes); err != nil { diff --git a/provider/civo/civo.go b/provider/civo/civo.go index 8ea04c250..cc198560e 100644 --- a/provider/civo/civo.go +++ b/provider/civo/civo.go @@ -471,7 +471,7 @@ func (p *CivoProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) } createsByZone := endpointsByZone(zoneNameIDMapper, changes.Create) - updatesByZone := endpointsByZone(zoneNameIDMapper, changes.UpdateNew) + updatesByZone := endpointsByZone(zoneNameIDMapper, changes.UpdateNew()) deletesByZone := endpointsByZone(zoneNameIDMapper, changes.Delete) // Generate Creates diff --git a/provider/civo/civo_test.go b/provider/civo/civo_test.go index 80910887d..20fec4430 100644 --- a/provider/civo/civo_test.go +++ b/provider/civo/civo_test.go @@ -644,8 +644,12 @@ func TestCivoApplyChanges(t *testing.T) { {DNSName: "new.ext-dns-test-with-ttl.example.com", Targets: endpoint.Targets{"target"}, RecordType: endpoint.RecordTypeA, RecordTTL: 100}, } changes.Delete = []*endpoint.Endpoint{{DNSName: "foobar.ext-dns-test.example.com", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"target"}}} - changes.UpdateOld = []*endpoint.Endpoint{{DNSName: "foobar.ext-dns-test.example.de", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"target-old"}}} - changes.UpdateNew = []*endpoint.Endpoint{{DNSName: "foobar.ext-dns-test.foo.com", Targets: endpoint.Targets{"target-new"}, RecordType: endpoint.RecordTypeCNAME, RecordTTL: 100}} + changes.Update = []*plan.Update{ + { + Old: &endpoint.Endpoint{DNSName: "foobar.ext-dns-test.example.de", RecordType: endpoint.RecordTypeA, Targets: endpoint.Targets{"target-old"}}, + New: &endpoint.Endpoint{DNSName: "foobar.ext-dns-test.foo.com", Targets: endpoint.Targets{"target-new"}, RecordType: endpoint.RecordTypeCNAME, RecordTTL: 100}, + }, + } err := provider.ApplyChanges(context.Background(), changes) assert.NoError(t, err) } @@ -690,11 +694,11 @@ func TestCivoApplyChangesError(t *testing.T) { { Name: "invalid record type from processUpdateActions", changes: &plan.Changes{ - UpdateOld: []*endpoint.Endpoint{ - endpoint.NewEndpoint("bad.example.com", "AAAA", "1.2.3.4"), - }, - UpdateNew: []*endpoint.Endpoint{ - endpoint.NewEndpoint("bad.example.com", "AAAA", "5.6.7.8"), + Update: []*plan.Update{ + { + Old: endpoint.NewEndpoint("bad.example.com", "AAAA", "1.2.3.4"), + New: endpoint.NewEndpoint("bad.example.com", "AAAA", "5.6.7.8"), + }, }, }, }, diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go index 8914032cc..939e21836 100644 --- a/provider/cloudflare/cloudflare.go +++ b/provider/cloudflare/cloudflare.go @@ -448,8 +448,9 @@ func (p *CloudFlareProvider) ApplyChanges(ctx context.Context, changes *plan.Cha } } - for i, desired := range changes.UpdateNew { - current := changes.UpdateOld[i] + for _, change := range changes.Update { + current := change.Old + desired := change.New add, remove, leave := provider.Difference(current.Targets, desired.Targets) diff --git a/provider/cloudflare/cloudflare_regional_test.go b/provider/cloudflare/cloudflare_regional_test.go index aee88dfe0..93235b585 100644 --- a/provider/cloudflare/cloudflare_regional_test.go +++ b/provider/cloudflare/cloudflare_regional_test.go @@ -917,23 +917,23 @@ func TestApplyChangesWithRegionalHostnamesFaillures(t *testing.T) { }, args: args{ changes: &plan.Changes{ - UpdateOld: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - RecordType: "A", - DNSName: "rherror.bar.com", - Targets: endpoint.Targets{"127.0.0.1"}, - ProviderSpecific: endpoint.ProviderSpecific{ - {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + Old: &endpoint.Endpoint{ + RecordType: "A", + DNSName: "rherror.bar.com", + Targets: endpoint.Targets{"127.0.0.1"}, + ProviderSpecific: endpoint.ProviderSpecific{ + {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + }, }, - }, - }, - UpdateNew: []*endpoint.Endpoint{ - { - RecordType: "A", - DNSName: "rherror.bar.com", - Targets: endpoint.Targets{"127.0.0.2"}, - ProviderSpecific: endpoint.ProviderSpecific{ - {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + New: &endpoint.Endpoint{ + RecordType: "A", + DNSName: "rherror.bar.com", + Targets: endpoint.Targets{"127.0.0.2"}, + ProviderSpecific: endpoint.ProviderSpecific{ + {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + }, }, }, }, @@ -1098,23 +1098,23 @@ func TestApplyChangesWithRegionalHostnamesDryRun(t *testing.T) { }, args: args{ changes: &plan.Changes{ - UpdateOld: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - RecordType: "A", - DNSName: "foo.bar.com", - Targets: endpoint.Targets{"127.0.0.1"}, - ProviderSpecific: endpoint.ProviderSpecific{ - {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + Old: &endpoint.Endpoint{ + RecordType: "A", + DNSName: "foo.bar.com", + Targets: endpoint.Targets{"127.0.0.1"}, + ProviderSpecific: endpoint.ProviderSpecific{ + {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + }, }, - }, - }, - UpdateNew: []*endpoint.Endpoint{ - { - RecordType: "A", - DNSName: "foo.bar.com", - Targets: endpoint.Targets{"127.0.0.2"}, - ProviderSpecific: endpoint.ProviderSpecific{ - {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + New: &endpoint.Endpoint{ + RecordType: "A", + DNSName: "foo.bar.com", + Targets: endpoint.Targets{"127.0.0.2"}, + ProviderSpecific: endpoint.ProviderSpecific{ + {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + }, }, }, }, diff --git a/provider/cloudflare/cloudflare_test.go b/provider/cloudflare/cloudflare_test.go index 79b670000..879204e26 100644 --- a/provider/cloudflare/cloudflare_test.go +++ b/provider/cloudflare/cloudflare_test.go @@ -1082,13 +1082,15 @@ func TestCloudflareApplyChanges(t *testing.T) { DNSName: "foobar.bar.com", Targets: endpoint.Targets{"target"}, }} - changes.UpdateOld = []*endpoint.Endpoint{{ - DNSName: "foobar.bar.com", - Targets: endpoint.Targets{"target-old"}, - }} - changes.UpdateNew = []*endpoint.Endpoint{{ - DNSName: "foobar.bar.com", - Targets: endpoint.Targets{"target-new"}, + changes.Update = []*plan.Update{{ + Old: &endpoint.Endpoint{ + DNSName: "foobar.bar.com", + Targets: endpoint.Targets{"target-old"}, + }, + New: &endpoint.Endpoint{ + DNSName: "foobar.bar.com", + Targets: endpoint.Targets{"target-new"}, + }, }} err := provider.ApplyChanges(context.Background(), changes) if err != nil { @@ -1125,8 +1127,7 @@ func TestCloudflareApplyChanges(t *testing.T) { // empty changes changes.Create = []*endpoint.Endpoint{} changes.Delete = []*endpoint.Endpoint{} - changes.UpdateOld = []*endpoint.Endpoint{} - changes.UpdateNew = []*endpoint.Endpoint{} + changes.Update = []*plan.Update{} err = provider.ApplyChanges(context.Background(), changes) if err != nil { @@ -1606,11 +1607,11 @@ func TestProviderPropertiesIdempotency(t *testing.T) { assert.Empty(t, plan.Changes.Delete, "should not have deletes") if test.ShouldBeUpdated { - assert.Len(t, plan.Changes.UpdateNew, 1, "should not have new updates") - assert.Len(t, plan.Changes.UpdateOld, 1, "should not have old updates") + assert.Len(t, plan.Changes.UpdateNew(), 1, "should not have new updates") + assert.Len(t, plan.Changes.UpdateOld(), 1, "should not have old updates") } else { - assert.Empty(t, plan.Changes.UpdateNew, "should not have new updates") - assert.Empty(t, plan.Changes.UpdateOld, "should not have old updates") + assert.Empty(t, plan.Changes.UpdateNew(), "should not have new updates") + assert.Empty(t, plan.Changes.UpdateOld(), "should not have old updates") } }) } @@ -1749,8 +1750,8 @@ func TestCustomTTLWithEnabledProxyNotChanged(t *testing.T) { planned := plan.Calculate() assert.Empty(t, planned.Changes.Create, "no new changes should be here") - assert.Empty(t, planned.Changes.UpdateNew, "no new changes should be here") - assert.Empty(t, planned.Changes.UpdateOld, "no new changes should be here") + assert.Empty(t, planned.Changes.UpdateNew(), "no new changes should be here") + assert.Empty(t, planned.Changes.UpdateOld(), "no new changes should be here") assert.Empty(t, planned.Changes.Delete, "no new changes should be here") } @@ -2645,25 +2646,27 @@ func TestCloudflareApplyChanges_AllErrorLogPaths(t *testing.T) { { name: "Update add/remove error (custom hostnames enabled)", changes: &plan.Changes{ - UpdateNew: []*endpoint.Endpoint{{ - DNSName: "bad-update-add.bar.com", - RecordType: "MX", - Targets: endpoint.Targets{"not-a-valid-mx"}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "bad-update-add-custom.bar.com", + Update: []*plan.Update{{ + New: &endpoint.Endpoint{ + DNSName: "bad-update-add.bar.com", + RecordType: "MX", + Targets: endpoint.Targets{"not-a-valid-mx"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", + Value: "bad-update-add-custom.bar.com", + }, }, }, - }}, - UpdateOld: []*endpoint.Endpoint{{ - DNSName: "old-bad-update-add.bar.com", - RecordType: "MX", - Targets: endpoint.Targets{"not-a-valid-mx-but-still-updated"}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "bad-update-add-custom.bar.com", + Old: &endpoint.Endpoint{ + DNSName: "old-bad-update-add.bar.com", + RecordType: "MX", + Targets: endpoint.Targets{"not-a-valid-mx-but-still-updated"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", + Value: "bad-update-add-custom.bar.com", + }, }, }, }}, @@ -2674,25 +2677,27 @@ func TestCloudflareApplyChanges_AllErrorLogPaths(t *testing.T) { { name: "Update leave error (custom hostnames enabled)", changes: &plan.Changes{ - UpdateOld: []*endpoint.Endpoint{{ - DNSName: "bad-update-leave.bar.com", - RecordType: "MX", - Targets: endpoint.Targets{"not-a-valid-mx"}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "bad-update-leave-custom.bar.com", + Update: []*plan.Update{{ + Old: &endpoint.Endpoint{ + DNSName: "bad-update-leave.bar.com", + RecordType: "MX", + Targets: endpoint.Targets{"not-a-valid-mx"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", + Value: "bad-update-leave-custom.bar.com", + }, }, }, - }}, - UpdateNew: []*endpoint.Endpoint{{ - DNSName: "bad-update-leave.bar.com", - RecordType: "MX", - Targets: endpoint.Targets{"not-a-valid-mx"}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", - Value: "bad-update-leave-custom.bar.com", + New: &endpoint.Endpoint{ + DNSName: "bad-update-leave.bar.com", + RecordType: "MX", + Targets: endpoint.Targets{"not-a-valid-mx"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", + Value: "bad-update-leave-custom.bar.com", + }, }, }, }}, diff --git a/provider/coredns/coredns.go b/provider/coredns/coredns.go index fdd52904a..2aba1c878 100644 --- a/provider/coredns/coredns.go +++ b/provider/coredns/coredns.go @@ -301,9 +301,10 @@ func (p coreDNSProvider) groupEndpoints(changes *plan.Changes) map[string][]*end for _, ep := range changes.Create { grouped[ep.DNSName] = append(grouped[ep.DNSName], ep) } - for i, ep := range changes.UpdateNew { - ep.Labels = changes.UpdateOld[i].Labels - log.Debugf("Updating labels (%s) with old labels(%s)", ep.Labels, changes.UpdateOld[i].Labels) + updateOld := changes.UpdateOld() + for i, ep := range changes.UpdateNew() { + ep.Labels = updateOld[i].Labels + log.Debugf("Updating labels (%s) with old labels(%s)", ep.Labels, updateOld[i].Labels) grouped[ep.DNSName] = append(grouped[ep.DNSName], ep) } return grouped diff --git a/provider/coredns/coredns_test.go b/provider/coredns/coredns_test.go index 1d84c4117..d4865480f 100644 --- a/provider/coredns/coredns_test.go +++ b/provider/coredns/coredns_test.go @@ -350,20 +350,26 @@ func TestCoreDNSApplyChanges(t *testing.T) { } validateServices(client.services, expectedServices1, t, 1) + var old *endpoint.Endpoint + records, _ := coredns.Records(context.Background()) + for _, ep := range records { + if ep.DNSName == "domain1.local" { + old = ep + break + } + } + assert.NotNil(t, old) changes2 := &plan.Changes{ Create: []*endpoint.Endpoint{ endpoint.NewEndpoint("domain3.local", endpoint.RecordTypeA, "7.7.7.7"), }, - UpdateNew: []*endpoint.Endpoint{ - endpoint.NewEndpoint("domain1.local", "A", "6.6.6.6"), + Update: []*plan.Update{ + { + Old: old, + New: endpoint.NewEndpoint("domain1.local", "A", "6.6.6.6"), + }, }, } - records, _ := coredns.Records(context.Background()) - for _, ep := range records { - if ep.DNSName == "domain1.local" { - changes2.UpdateOld = append(changes2.UpdateOld, ep) - } - } err = applyServiceChanges(coredns, changes2) require.NoError(t, err) @@ -436,7 +442,7 @@ func TestCoreDNSApplyChanges_DomainDoNotMatch(t *testing.T) { func applyServiceChanges(provider coreDNSProvider, changes *plan.Changes) error { ctx := context.Background() records, _ := provider.Records(ctx) - for _, col := range [][]*endpoint.Endpoint{changes.Create, changes.UpdateNew, changes.Delete} { + for _, col := range [][]*endpoint.Endpoint{changes.Create, changes.UpdateNew(), changes.Delete} { for _, record := range col { for _, existingRecord := range records { if existingRecord.DNSName == record.DNSName && existingRecord.RecordType == record.RecordType { diff --git a/provider/digitalocean/digital_ocean.go b/provider/digitalocean/digital_ocean.go index 637b4ae10..9e122a0b3 100644 --- a/provider/digitalocean/digital_ocean.go +++ b/provider/digitalocean/digital_ocean.go @@ -640,7 +640,7 @@ func (p *DigitalOceanProvider) ApplyChanges(ctx context.Context, planChanges *pl } createsByDomain := endpointsByZone(zoneNameIDMapper, planChanges.Create) - updatesByDomain := endpointsByZone(zoneNameIDMapper, planChanges.UpdateNew) + updatesByDomain := endpointsByZone(zoneNameIDMapper, planChanges.UpdateNew()) deletesByDomain := endpointsByZone(zoneNameIDMapper, planChanges.Delete) var changes digitalOceanChanges diff --git a/provider/digitalocean/digital_ocean_test.go b/provider/digitalocean/digital_ocean_test.go index e174fe067..03d5e5aa7 100644 --- a/provider/digitalocean/digital_ocean_test.go +++ b/provider/digitalocean/digital_ocean_test.go @@ -383,8 +383,12 @@ func TestDigitalOceanApplyChanges(t *testing.T) { {DNSName: "bar.com", Targets: endpoint.Targets{"target"}}, } changes.Delete = []*endpoint.Endpoint{{DNSName: "foobar.ext-dns-test.bar.com", Targets: endpoint.Targets{"target"}}} - changes.UpdateOld = []*endpoint.Endpoint{{DNSName: "foobar.ext-dns-test.bar.de", Targets: endpoint.Targets{"target-old"}}} - changes.UpdateNew = []*endpoint.Endpoint{{DNSName: "foobar.ext-dns-test.foo.com", Targets: endpoint.Targets{"target-new"}, RecordType: "CNAME", RecordTTL: 100}} + changes.Update = []*plan.Update{ + { + Old: &endpoint.Endpoint{DNSName: "foobar.ext-dns-test.bar.de", Targets: endpoint.Targets{"target-old"}}, + New: &endpoint.Endpoint{DNSName: "foobar.ext-dns-test.foo.com", Targets: endpoint.Targets{"target-new"}, RecordType: "CNAME", RecordTTL: 100}, + }, + } err := provider.ApplyChanges(context.Background(), changes) if err != nil { t.Errorf("should not fail, %s", err) diff --git a/provider/dnsimple/dnsimple.go b/provider/dnsimple/dnsimple.go index 5d120e67a..51614334e 100644 --- a/provider/dnsimple/dnsimple.go +++ b/provider/dnsimple/dnsimple.go @@ -359,10 +359,10 @@ func dnsimpleSuitableZone(hostname string, zones map[string]dnsimple.Zone) *dnsi // ApplyChanges applies a given set of changes func (p *dnsimpleProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) error { - combinedChanges := make([]*dnsimpleChange, 0, len(changes.Create)+len(changes.UpdateNew)+len(changes.Delete)) + combinedChanges := make([]*dnsimpleChange, 0, len(changes.Create)+len(changes.Update)) combinedChanges = append(combinedChanges, newDnsimpleChanges(dnsimpleCreate, changes.Create)...) - combinedChanges = append(combinedChanges, newDnsimpleChanges(dnsimpleUpdate, changes.UpdateNew)...) + combinedChanges = append(combinedChanges, newDnsimpleChanges(dnsimpleUpdate, changes.UpdateNew())...) combinedChanges = append(combinedChanges, newDnsimpleChanges(dnsimpleDelete, changes.Delete)...) return p.submitChanges(ctx, combinedChanges) diff --git a/provider/dnsimple/dnsimple_test.go b/provider/dnsimple/dnsimple_test.go index 7f4092ee1..cd77a6d18 100644 --- a/provider/dnsimple/dnsimple_test.go +++ b/provider/dnsimple/dnsimple_test.go @@ -194,9 +194,13 @@ func testDnsimpleProviderApplyChanges(t *testing.T) { changes.Delete = []*endpoint.Endpoint{ {DNSName: "example-beta.example.com", Targets: endpoint.Targets{"127.0.0.1"}, RecordType: endpoint.RecordTypeA}, } - changes.UpdateNew = []*endpoint.Endpoint{ - {DNSName: "example.example.com", Targets: endpoint.Targets{"target"}, RecordType: endpoint.RecordTypeCNAME}, - {DNSName: "example.com", Targets: endpoint.Targets{"127.0.0.1"}, RecordType: endpoint.RecordTypeA}, + changes.Update = []*plan.Update{ + { + New: &endpoint.Endpoint{DNSName: "example.example.com", Targets: endpoint.Targets{"target"}, RecordType: endpoint.RecordTypeCNAME}, + }, + { + New: &endpoint.Endpoint{DNSName: "example.com", Targets: endpoint.Targets{"127.0.0.1"}, RecordType: endpoint.RecordTypeA}, + }, } mockProvider.accountID = "1" diff --git a/provider/exoscale/exoscale.go b/provider/exoscale/exoscale.go index 86c40388b..ace900678 100644 --- a/provider/exoscale/exoscale.go +++ b/provider/exoscale/exoscale.go @@ -105,7 +105,7 @@ func (ep *ExoscaleProvider) ApplyChanges(ctx context.Context, changes *plan.Chan if ep.dryRun { log.Infof("Will NOT delete these records: %+v", changes.Delete) log.Infof("Will NOT create these records: %+v", changes.Create) - log.Infof("Will NOT update these records: %+v", merge(changes.UpdateOld, changes.UpdateNew)) + log.Infof("Will NOT update these records: %+v", merge(changes.UpdateOld(), changes.UpdateNew())) return nil } @@ -144,7 +144,7 @@ func (ep *ExoscaleProvider) ApplyChanges(ctx context.Context, changes *plan.Chan } } - for _, epoint := range changes.UpdateNew { + for _, epoint := range changes.UpdateNew() { if !ep.domain.Match(epoint.DNSName) { continue } @@ -180,7 +180,7 @@ func (ep *ExoscaleProvider) ApplyChanges(ctx context.Context, changes *plan.Chan } } - for _, epoint := range changes.UpdateOld { + for _, epoint := range changes.UpdateOld() { // Since Exoscale "Patches", we've ignored UpdateOld // We leave this logging here for information log.Debugf("UPDATE-OLD (ignored) for epoint: %+v", epoint) @@ -262,10 +262,10 @@ func ExoscaleWithLogging() ExoscaleOption { for _, v := range changes.Create { log.Infof("CREATE: %v", v) } - for _, v := range changes.UpdateOld { + for _, v := range changes.UpdateOld() { log.Infof("UPDATE (old): %v", v) } - for _, v := range changes.UpdateNew { + for _, v := range changes.UpdateNew() { log.Infof("UPDATE (new): %v", v) } for _, v := range changes.Delete { diff --git a/provider/exoscale/exoscale_test.go b/provider/exoscale/exoscale_test.go index 234505ba4..084f9495f 100644 --- a/provider/exoscale/exoscale_test.go +++ b/provider/exoscale/exoscale_test.go @@ -160,28 +160,31 @@ func TestExoscaleApplyChanges(t *testing.T) { Targets: []string{""}, }, }, - UpdateOld: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "v1.foo.com", - RecordType: "A", - Targets: []string{""}, + Old: &endpoint.Endpoint{ + DNSName: "v1.foo.com", + RecordType: "A", + Targets: []string{""}, + }, + New: &endpoint.Endpoint{ + DNSName: "v1.foo.com", + RecordType: "A", + Targets: []string{""}, + }, }, { - DNSName: "v1.foobar.com", - RecordType: "TXT", - Targets: []string{""}, - }, - }, - UpdateNew: []*endpoint.Endpoint{ - { - DNSName: "v1.foo.com", - RecordType: "A", - Targets: []string{""}, - }, - { - DNSName: "v1.foobar.com", - RecordType: "TXT", - Targets: []string{""}, + Old: &endpoint.Endpoint{ + DNSName: "v1.foobar.com", + RecordType: "TXT", + Targets: []string{""}, + }, + New: &endpoint.Endpoint{ + + DNSName: "v1.foobar.com", + RecordType: "TXT", + Targets: []string{""}, + }, }, }, } diff --git a/provider/gandi/gandi.go b/provider/gandi/gandi.go index e8fdea337..1e5227c92 100644 --- a/provider/gandi/gandi.go +++ b/provider/gandi/gandi.go @@ -147,10 +147,10 @@ func (p *GandiProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, erro } func (p *GandiProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) error { - combinedChanges := make([]*GandiChanges, 0, len(changes.Create)+len(changes.UpdateNew)+len(changes.Delete)) + combinedChanges := make([]*GandiChanges, 0, len(changes.Create)+len(changes.Update)+len(changes.Delete)) combinedChanges = append(combinedChanges, p.newGandiChanges(gandiCreate, changes.Create)...) - combinedChanges = append(combinedChanges, p.newGandiChanges(gandiUpdate, changes.UpdateNew)...) + combinedChanges = append(combinedChanges, p.newGandiChanges(gandiUpdate, changes.UpdateNew())...) combinedChanges = append(combinedChanges, p.newGandiChanges(gandiDelete, changes.Delete)...) return p.submitChanges(ctx, combinedChanges) diff --git a/provider/gandi/gandi_test.go b/provider/gandi/gandi_test.go index 8e564b8e1..4a9ce4aa7 100644 --- a/provider/gandi/gandi_test.go +++ b/provider/gandi/gandi_test.go @@ -319,18 +319,22 @@ func TestGandiProvider_ApplyChangesMakesExpectedAPICalls(t *testing.T) { RecordTTL: 666, }, } - changes.UpdateNew = []*endpoint.Endpoint{ + changes.Update = []*plan.Update{ { - DNSName: "test3.example.com", - Targets: endpoint.Targets{"192.168.0.2"}, - RecordType: "A", - RecordTTL: 777, + New: &endpoint.Endpoint{ + DNSName: "test3.example.com", + Targets: endpoint.Targets{"192.168.0.2"}, + RecordType: "A", + RecordTTL: 777, + }, }, { - DNSName: "example.com.example.com", - Targets: endpoint.Targets{"lb-2.example.net"}, - RecordType: "CNAME", - RecordTTL: 777, + New: &endpoint.Endpoint{ + DNSName: "example.com.example.com", + Targets: endpoint.Targets{"lb-2.example.net"}, + RecordType: "CNAME", + RecordTTL: 777, + }, }, } changes.Delete = []*endpoint.Endpoint{ @@ -401,7 +405,11 @@ func TestGandiProvider_ApplyChangesRespectsDryRun(t *testing.T) { } changes.Create = []*endpoint.Endpoint{{DNSName: "test2.example.com", Targets: endpoint.Targets{"192.168.0.1"}, RecordType: "A", RecordTTL: 666}} - changes.UpdateNew = []*endpoint.Endpoint{{DNSName: "test3.example.com", Targets: endpoint.Targets{"192.168.0.2"}, RecordType: "A", RecordTTL: 777}} + changes.Update = []*plan.Update{ + { + New: &endpoint.Endpoint{DNSName: "test3.example.com", Targets: endpoint.Targets{"192.168.0.2"}, RecordType: "A", RecordTTL: 777}, + }, + } changes.Delete = []*endpoint.Endpoint{{DNSName: "test4.example.com", Targets: endpoint.Targets{"192.168.0.3"}, RecordType: "A"}} mockedProvider.ApplyChanges(context.Background(), changes) @@ -495,7 +503,11 @@ func TestGandiProvider_ApplyChangesConvertsApexDomain(t *testing.T) { func TestGandiProvider_FailingCases(t *testing.T) { changes := &plan.Changes{} changes.Create = []*endpoint.Endpoint{{DNSName: "test2.example.com", Targets: endpoint.Targets{"192.168.0.1"}, RecordType: "A", RecordTTL: 666}} - changes.UpdateNew = []*endpoint.Endpoint{{DNSName: "test3.example.com", Targets: endpoint.Targets{"192.168.0.2"}, RecordType: "A", RecordTTL: 777}} + changes.Update = []*plan.Update{ + { + New: &endpoint.Endpoint{DNSName: "test3.example.com", Targets: endpoint.Targets{"192.168.0.2"}, RecordType: "A", RecordTTL: 777}, + }, + } changes.Delete = []*endpoint.Endpoint{{DNSName: "test4.example.com", Targets: endpoint.Targets{"192.168.0.3"}, RecordType: "A"}} // Failing ListDomains API call creates an error when calling Records diff --git a/provider/godaddy/godaddy.go b/provider/godaddy/godaddy.go index 045cd2332..4bfb9d852 100644 --- a/provider/godaddy/godaddy.go +++ b/provider/godaddy/godaddy.go @@ -392,8 +392,10 @@ func (p *GDProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) er iOldSkip := make(map[int]bool) iNewSkip := make(map[int]bool) - for iOld, recOld := range changes.UpdateOld { - for iNew, recNew := range changes.UpdateNew { + updateOld := changes.UpdateOld() + updateNew := changes.UpdateNew() + for iOld, recOld := range updateOld { + for iNew, recNew := range updateNew { if recOld.DNSName == recNew.DNSName && recOld.RecordType == recNew.RecordType { ReplaceEndpoints := []*endpoint.Endpoint{recNew} allChanges = p.appendChange(gdReplace, ReplaceEndpoints, allChanges) @@ -404,12 +406,12 @@ func (p *GDProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) er } } - for iOld, recOld := range changes.UpdateOld { + for iOld, recOld := range updateOld { _, found := iOldSkip[iOld] if found { continue } - for iNew, recNew := range changes.UpdateNew { + for iNew, recNew := range updateNew { _, found := iNewSkip[iNew] if found { continue @@ -579,7 +581,7 @@ func (c gdRecordField) String() string { } func countTargets(p *plan.Changes) int { - changes := [][]*endpoint.Endpoint{p.Create, p.UpdateNew, p.UpdateOld, p.Delete} + changes := [][]*endpoint.Endpoint{p.Create, p.UpdateNew(), p.UpdateOld(), p.Delete} count := 0 for _, endpoints := range changes { diff --git a/provider/google/google.go b/provider/google/google.go index fd521fc0e..05bf24713 100644 --- a/provider/google/google.go +++ b/provider/google/google.go @@ -236,8 +236,8 @@ func (p *GoogleProvider) ApplyChanges(ctx context.Context, changes *plan.Changes change.Additions = append(change.Additions, p.newFilteredRecords(changes.Create)...) - change.Additions = append(change.Additions, p.newFilteredRecords(changes.UpdateNew)...) - change.Deletions = append(change.Deletions, p.newFilteredRecords(changes.UpdateOld)...) + change.Additions = append(change.Additions, p.newFilteredRecords(changes.UpdateNew())...) + change.Deletions = append(change.Deletions, p.newFilteredRecords(changes.UpdateOld())...) change.Deletions = append(change.Deletions, p.newFilteredRecords(changes.Delete)...) diff --git a/provider/google/google_test.go b/provider/google/google_test.go index 259632544..12185cb86 100644 --- a/provider/google/google_test.go +++ b/provider/google/google_test.go @@ -370,7 +370,6 @@ func TestGoogleApplyChanges(t *testing.T) { endpoint.NewEndpointWithTTL("update-test-ttl.zone-2.ext-dns-test-2.gcp.zalan.do", endpoint.RecordTypeA, endpoint.TTL(25), "4.3.2.1"), endpoint.NewEndpoint("update-test-cname.zone-1.ext-dns-test-2.gcp.zalan.do", endpoint.RecordTypeCNAME, "baz.elb.amazonaws.com"), endpoint.NewEndpoint("filter-update-test.zone-3.ext-dns-test-2.gcp.zalan.do", endpoint.RecordTypeA, "5.6.7.8"), - endpoint.NewEndpoint("nomatch-update-test.zone-0.ext-dns-test-2.gcp.zalan.do", endpoint.RecordTypeA, "8.7.6.5"), } deleteRecords := []*endpoint.Endpoint{ @@ -381,11 +380,12 @@ func TestGoogleApplyChanges(t *testing.T) { endpoint.NewEndpoint("nomatch-delete-test.zone-0.ext-dns-test-2.gcp.zalan.do", endpoint.RecordTypeA, "4.2.2.1"), } + update, err := plan.MkUpdates(currentRecords, updatedRecords) + assert.NoError(t, err) changes := &plan.Changes{ - Create: createRecords, - UpdateNew: updatedRecords, - UpdateOld: currentRecords, - Delete: deleteRecords, + Create: createRecords, + Update: update, + Delete: deleteRecords, } require.NoError(t, provider.ApplyChanges(context.Background(), changes)) @@ -438,11 +438,12 @@ func TestGoogleApplyChangesDryRun(t *testing.T) { endpoint.NewEndpoint("delete-test-cname.zone-1.ext-dns-test-2.gcp.zalan.do", endpoint.RecordTypeCNAME, "qux.elb.amazonaws.com"), } + update, err := plan.MkUpdates(currentRecords, updatedRecords) + assert.NoError(t, err) changes := &plan.Changes{ - Create: createRecords, - UpdateNew: updatedRecords, - UpdateOld: currentRecords, - Delete: deleteRecords, + Create: createRecords, + Update: update, + Delete: deleteRecords, } ctx := context.Background() diff --git a/provider/inmemory/inmemory.go b/provider/inmemory/inmemory.go index b17ef900e..bf745eb82 100644 --- a/provider/inmemory/inmemory.go +++ b/provider/inmemory/inmemory.go @@ -63,11 +63,8 @@ func InMemoryWithLogging() InMemoryOption { for _, v := range changes.Create { log.Infof("CREATE: %v", v) } - for _, v := range changes.UpdateOld { - log.Infof("UPDATE (old): %v", v) - } - for _, v := range changes.UpdateNew { - log.Infof("UPDATE (new): %v", v) + for _, v := range changes.Update { + log.Infof("UPDATE (old/new): %v / %v", v.Old, v.New) } for _, v := range changes.Delete { log.Infof("DELETE: %v", v) @@ -155,28 +152,22 @@ func (im *InMemoryProvider) ApplyChanges(ctx context.Context, changes *plan.Chan } for _, ep := range changes.Create { - zoneID := im.filter.EndpointZoneID(ep, zones) + zoneID := im.filter.EndpointZoneID(ep.DNSName, zones) if zoneID == "" { continue } perZoneChanges[zoneID].Create = append(perZoneChanges[zoneID].Create, ep) } - for _, ep := range changes.UpdateNew { - zoneID := im.filter.EndpointZoneID(ep, zones) + for _, change := range changes.Update { + // NOTE: DNSName of `change.Old` and `change.New` will be equivalent + zoneID := im.filter.EndpointZoneID(change.Old.DNSName, zones) if zoneID == "" { continue } - perZoneChanges[zoneID].UpdateNew = append(perZoneChanges[zoneID].UpdateNew, ep) - } - for _, ep := range changes.UpdateOld { - zoneID := im.filter.EndpointZoneID(ep, zones) - if zoneID == "" { - continue - } - perZoneChanges[zoneID].UpdateOld = append(perZoneChanges[zoneID].UpdateOld, ep) + perZoneChanges[zoneID].Update = append(perZoneChanges[zoneID].Update, change) } for _, ep := range changes.Delete { - zoneID := im.filter.EndpointZoneID(ep, zones) + zoneID := im.filter.EndpointZoneID(ep.DNSName, zones) if zoneID == "" { continue } @@ -185,10 +176,9 @@ func (im *InMemoryProvider) ApplyChanges(ctx context.Context, changes *plan.Chan for zoneID := range perZoneChanges { change := &plan.Changes{ - Create: perZoneChanges[zoneID].Create, - UpdateNew: perZoneChanges[zoneID].UpdateNew, - UpdateOld: perZoneChanges[zoneID].UpdateOld, - Delete: perZoneChanges[zoneID].Delete, + Create: perZoneChanges[zoneID].Create, + Update: perZoneChanges[zoneID].Update, + Delete: perZoneChanges[zoneID].Delete, } err := im.client.ApplyChanges(ctx, zoneID, change) if err != nil { @@ -230,10 +220,10 @@ func (f *filter) Zones(zones map[string]string) map[string]string { // EndpointZoneID determines zoneID for endpoint from map[zoneID]zoneName by taking longest suffix zoneName match in endpoint DNSName // returns empty string if no match found -func (f *filter) EndpointZoneID(endpoint *endpoint.Endpoint, zones map[string]string) string { +func (f *filter) EndpointZoneID(dnsName string, zones map[string]string) string { var matchZoneID, matchZoneName string for zoneID, zoneName := range zones { - if strings.HasSuffix(endpoint.DNSName, zoneName) && len(zoneName) > len(matchZoneName) { + if strings.HasSuffix(dnsName, zoneName) && len(zoneName) > len(matchZoneName) { matchZoneName = zoneName matchZoneID = zoneID } @@ -287,7 +277,7 @@ func (c *inMemoryClient) ApplyChanges(ctx context.Context, zoneID string, change for _, newEndpoint := range changes.Create { c.zones[zoneID][newEndpoint.Key()] = newEndpoint } - for _, updateEndpoint := range changes.UpdateNew { + for _, updateEndpoint := range changes.UpdateNew() { c.zones[zoneID][updateEndpoint.Key()] = updateEndpoint } for _, deleteEndpoint := range changes.Delete { @@ -319,7 +309,7 @@ func (c *inMemoryClient) validateChangeBatch(zone string, changes *plan.Changes) return err } } - for _, updateEndpoint := range changes.UpdateNew { + for _, updateEndpoint := range changes.UpdateNew() { if _, ok := curZone[updateEndpoint.Key()]; !ok { return ErrRecordNotFound } @@ -327,7 +317,7 @@ func (c *inMemoryClient) validateChangeBatch(zone string, changes *plan.Changes) return err } } - for _, updateOldEndpoint := range changes.UpdateOld { + for _, updateOldEndpoint := range changes.UpdateOld() { if rec, ok := curZone[updateOldEndpoint.Key()]; !ok || rec.Targets[0] != updateOldEndpoint.Targets[0] { return ErrRecordNotFound } diff --git a/provider/inmemory/inmemory_test.go b/provider/inmemory/inmemory_test.go index df648205a..5d2bb2a38 100644 --- a/provider/inmemory/inmemory_test.go +++ b/provider/inmemory/inmemory_test.go @@ -135,10 +135,9 @@ func testInMemoryValidateChangeBatch(t *testing.T) { zone: "", init: map[string]zone{}, changes: &plan.Changes{ - Create: []*endpoint.Endpoint{}, - UpdateNew: []*endpoint.Endpoint{}, - UpdateOld: []*endpoint.Endpoint{}, - Delete: []*endpoint.Endpoint{}, + Create: []*endpoint.Endpoint{}, + Update: []*plan.Update{}, + Delete: []*endpoint.Endpoint{}, }, errorType: ErrZoneNotFound, }, @@ -148,10 +147,9 @@ func testInMemoryValidateChangeBatch(t *testing.T) { zone: "", init: init, changes: &plan.Changes{ - Create: []*endpoint.Endpoint{}, - UpdateNew: []*endpoint.Endpoint{}, - UpdateOld: []*endpoint.Endpoint{}, - Delete: []*endpoint.Endpoint{}, + Create: []*endpoint.Endpoint{}, + Update: []*plan.Update{}, + Delete: []*endpoint.Endpoint{}, }, errorType: ErrZoneNotFound, }, @@ -161,10 +159,9 @@ func testInMemoryValidateChangeBatch(t *testing.T) { zone: "test", init: init, changes: &plan.Changes{ - Create: []*endpoint.Endpoint{}, - UpdateNew: []*endpoint.Endpoint{}, - UpdateOld: []*endpoint.Endpoint{}, - Delete: []*endpoint.Endpoint{}, + Create: []*endpoint.Endpoint{}, + Update: []*plan.Update{}, + Delete: []*endpoint.Endpoint{}, }, errorType: ErrZoneNotFound, }, @@ -181,9 +178,8 @@ func testInMemoryValidateChangeBatch(t *testing.T) { RecordType: endpoint.RecordTypeA, }, }, - UpdateNew: []*endpoint.Endpoint{}, - UpdateOld: []*endpoint.Endpoint{}, - Delete: []*endpoint.Endpoint{}, + Update: []*plan.Update{}, + Delete: []*endpoint.Endpoint{}, }, errorType: ErrRecordAlreadyExists, }, @@ -200,15 +196,16 @@ func testInMemoryValidateChangeBatch(t *testing.T) { RecordType: endpoint.RecordTypeA, }, }, - UpdateNew: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "foo.org", - Targets: endpoint.Targets{"4.4.4.4"}, - RecordType: endpoint.RecordTypeA, + New: &endpoint.Endpoint{ + DNSName: "foo.org", + Targets: endpoint.Targets{"4.4.4.4"}, + RecordType: endpoint.RecordTypeA, + }, }, }, - UpdateOld: []*endpoint.Endpoint{}, - Delete: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{}, }, errorType: ErrRecordNotFound, }, @@ -225,15 +222,16 @@ func testInMemoryValidateChangeBatch(t *testing.T) { RecordType: endpoint.RecordTypeA, }, }, - UpdateNew: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "foo.org", - Targets: endpoint.Targets{"4.4.4.4"}, - RecordType: endpoint.RecordTypeA, + New: &endpoint.Endpoint{ + DNSName: "foo.org", + Targets: endpoint.Targets{"4.4.4.4"}, + RecordType: endpoint.RecordTypeA, + }, }, }, - UpdateOld: []*endpoint.Endpoint{}, - Delete: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{}, }, errorType: ErrRecordNotFound, }, @@ -255,9 +253,8 @@ func testInMemoryValidateChangeBatch(t *testing.T) { RecordType: endpoint.RecordTypeA, }, }, - UpdateNew: []*endpoint.Endpoint{}, - UpdateOld: []*endpoint.Endpoint{}, - Delete: []*endpoint.Endpoint{}, + Update: []*plan.Update{}, + Delete: []*endpoint.Endpoint{}, }, errorType: ErrDuplicateRecordFound, }, @@ -268,14 +265,20 @@ func testInMemoryValidateChangeBatch(t *testing.T) { init: init, changes: &plan.Changes{ Create: []*endpoint.Endpoint{}, - UpdateNew: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "example.org", - Targets: endpoint.Targets{"8.8.8.8"}, - RecordType: endpoint.RecordTypeA, + Old: &endpoint.Endpoint{ + DNSName: "example.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }, + New: &endpoint.Endpoint{ + DNSName: "example.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }, }, }, - UpdateOld: []*endpoint.Endpoint{}, Delete: []*endpoint.Endpoint{ { DNSName: "example.org", @@ -293,20 +296,24 @@ func testInMemoryValidateChangeBatch(t *testing.T) { init: init, changes: &plan.Changes{ Create: []*endpoint.Endpoint{}, - UpdateNew: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "example.org", - Targets: endpoint.Targets{"8.8.8.8"}, - RecordType: endpoint.RecordTypeA, + New: &endpoint.Endpoint{ + + DNSName: "example.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }, }, { - DNSName: "example.org", - Targets: endpoint.Targets{"8.8.8.8"}, - RecordType: endpoint.RecordTypeA, + New: &endpoint.Endpoint{ + DNSName: "example.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }, }, }, - UpdateOld: []*endpoint.Endpoint{}, - Delete: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{}, }, errorType: ErrDuplicateRecordFound, }, @@ -316,13 +323,19 @@ func testInMemoryValidateChangeBatch(t *testing.T) { zone: "org", init: init, changes: &plan.Changes{ - Create: []*endpoint.Endpoint{}, - UpdateNew: []*endpoint.Endpoint{}, - UpdateOld: []*endpoint.Endpoint{ + Create: []*endpoint.Endpoint{}, + Update: []*plan.Update{ { - DNSName: "new.org", - Targets: endpoint.Targets{"8.8.8.8"}, - RecordType: endpoint.RecordTypeA, + Old: &endpoint.Endpoint{ + DNSName: "new.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }, + New: &endpoint.Endpoint{ + DNSName: "new.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }, }, }, Delete: []*endpoint.Endpoint{}, @@ -335,9 +348,8 @@ func testInMemoryValidateChangeBatch(t *testing.T) { zone: "org", init: init, changes: &plan.Changes{ - Create: []*endpoint.Endpoint{}, - UpdateNew: []*endpoint.Endpoint{}, - UpdateOld: []*endpoint.Endpoint{}, + Create: []*endpoint.Endpoint{}, + Update: []*plan.Update{}, Delete: []*endpoint.Endpoint{ { DNSName: "new.org", @@ -354,9 +366,8 @@ func testInMemoryValidateChangeBatch(t *testing.T) { zone: "org", init: init, changes: &plan.Changes{ - Create: []*endpoint.Endpoint{}, - UpdateNew: []*endpoint.Endpoint{}, - UpdateOld: []*endpoint.Endpoint{}, + Create: []*endpoint.Endpoint{}, + Update: []*plan.Update{}, Delete: []*endpoint.Endpoint{ { DNSName: "foo.bar.org", @@ -379,18 +390,18 @@ func testInMemoryValidateChangeBatch(t *testing.T) { RecordType: endpoint.RecordTypeA, }, }, - UpdateNew: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "foo.bar.org", - Targets: endpoint.Targets{"4.8.8.4"}, - RecordType: endpoint.RecordTypeA, - }, - }, - UpdateOld: []*endpoint.Endpoint{ - { - DNSName: "foo.bar.org", - Targets: endpoint.Targets{"5.5.5.5"}, - RecordType: endpoint.RecordTypeA, + New: &endpoint.Endpoint{ + DNSName: "foo.bar.org", + Targets: endpoint.Targets{"4.8.8.4"}, + RecordType: endpoint.RecordTypeA, + }, + Old: &endpoint.Endpoint{ + DNSName: "foo.bar.org", + Targets: endpoint.Targets{"5.5.5.5"}, + RecordType: endpoint.RecordTypeA, + }, }, }, Delete: []*endpoint.Endpoint{}, @@ -401,10 +412,9 @@ func testInMemoryValidateChangeBatch(t *testing.T) { c := &inMemoryClient{} c.zones = ti.init ichanges := &plan.Changes{ - Create: ti.changes.Create, - UpdateNew: ti.changes.UpdateNew, - UpdateOld: ti.changes.UpdateOld, - Delete: ti.changes.Delete, + Create: ti.changes.Create, + Update: ti.changes.Update, + Delete: ti.changes.Delete, } err := c.validateChangeBatch(ti.zone, ichanges) if ti.expectError { @@ -444,9 +454,8 @@ func testInMemoryApplyChanges(t *testing.T) { Targets: endpoint.Targets{"8.8.8.8"}, RecordType: endpoint.RecordTypeA, }}, - UpdateNew: []*endpoint.Endpoint{}, - UpdateOld: []*endpoint.Endpoint{}, - Delete: []*endpoint.Endpoint{}, + Update: []*plan.Update{}, + Delete: []*endpoint.Endpoint{}, }, expectedZonesState: getInitData(), }, @@ -455,14 +464,20 @@ func testInMemoryApplyChanges(t *testing.T) { expectError: true, changes: &plan.Changes{ Create: []*endpoint.Endpoint{}, - UpdateNew: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "example.org", - Targets: endpoint.Targets{"8.8.8.8"}, - RecordType: endpoint.RecordTypeA, + New: &endpoint.Endpoint{ + DNSName: "example.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }, + Old: &endpoint.Endpoint{ + DNSName: "example.org", + Targets: endpoint.Targets{"8.8.8.8"}, + RecordType: endpoint.RecordTypeA, + }, }, }, - UpdateOld: []*endpoint.Endpoint{}, Delete: []*endpoint.Endpoint{ { DNSName: "example.org", @@ -476,9 +491,8 @@ func testInMemoryApplyChanges(t *testing.T) { title: "zones, update, right zone, valid batch - delete", expectError: false, changes: &plan.Changes{ - Create: []*endpoint.Endpoint{}, - UpdateNew: []*endpoint.Endpoint{}, - UpdateOld: []*endpoint.Endpoint{}, + Create: []*endpoint.Endpoint{}, + Update: []*plan.Update{}, Delete: []*endpoint.Endpoint{ { DNSName: "foo.bar.org", @@ -507,20 +521,20 @@ func testInMemoryApplyChanges(t *testing.T) { Labels: endpoint.NewLabels(), }, }, - UpdateNew: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "foo.bar.org", - Targets: endpoint.Targets{"4.8.8.4"}, - RecordType: endpoint.RecordTypeA, - Labels: endpoint.NewLabels(), - }, - }, - UpdateOld: []*endpoint.Endpoint{ - { - DNSName: "foo.bar.org", - Targets: endpoint.Targets{"5.5.5.5"}, - RecordType: endpoint.RecordTypeA, - Labels: endpoint.NewLabels(), + New: &endpoint.Endpoint{ + DNSName: "foo.bar.org", + Targets: endpoint.Targets{"4.8.8.4"}, + RecordType: endpoint.RecordTypeA, + Labels: endpoint.NewLabels(), + }, + Old: &endpoint.Endpoint{ + DNSName: "foo.bar.org", + Targets: endpoint.Targets{"5.5.5.5"}, + RecordType: endpoint.RecordTypeA, + Labels: endpoint.NewLabels(), + }, }, }, Delete: []*endpoint.Endpoint{ diff --git a/provider/linode/linode.go b/provider/linode/linode.go index 04389dca1..4f1583606 100644 --- a/provider/linode/linode.go +++ b/provider/linode/linode.go @@ -291,7 +291,7 @@ func (p *LinodeProvider) ApplyChanges(ctx context.Context, changes *plan.Changes } createsByZone := endpointsByZone(zoneNameIDMapper, changes.Create) - updatesByZone := endpointsByZone(zoneNameIDMapper, changes.UpdateNew) + updatesByZone := endpointsByZone(zoneNameIDMapper, changes.UpdateNew()) deletesByZone := endpointsByZone(zoneNameIDMapper, changes.Delete) var linodeCreates []LinodeChangeCreate diff --git a/provider/linode/linode_test.go b/provider/linode/linode_test.go index 347ac7911..aeac5bcef 100644 --- a/provider/linode/linode_test.go +++ b/provider/linode/linode_test.go @@ -383,13 +383,14 @@ func TestLinodeApplyChanges(t *testing.T) { DNSName: "api.baz.com", RecordType: "TXT", }}, - UpdateNew: []*endpoint.Endpoint{{ - DNSName: "foo.com", - RecordType: "A", - RecordTTL: 300, - Targets: []string{"targetFoo"}, + Update: []*plan.Update{{ + New: &endpoint.Endpoint{ + DNSName: "foo.com", + RecordType: "A", + RecordTTL: 300, + Targets: []string{"targetFoo"}, + }, }}, - UpdateOld: []*endpoint.Endpoint{}, }) require.NoError(t, err) @@ -443,12 +444,13 @@ func TestLinodeApplyChangesTargetAdded(t *testing.T) { err := provider.ApplyChanges(context.Background(), &plan.Changes{ // From 1 target to 2 - UpdateNew: []*endpoint.Endpoint{{ - DNSName: "example.com", - RecordType: "A", - Targets: []string{"targetA", "targetB"}, + Update: []*plan.Update{{ + New: &endpoint.Endpoint{ + DNSName: "example.com", + RecordType: "A", + Targets: []string{"targetA", "targetB"}, + }, }}, - UpdateOld: []*endpoint.Endpoint{}, }) require.NoError(t, err) @@ -499,12 +501,13 @@ func TestLinodeApplyChangesTargetRemoved(t *testing.T) { err := provider.ApplyChanges(context.Background(), &plan.Changes{ // From 2 targets to 1 - UpdateNew: []*endpoint.Endpoint{{ - DNSName: "example.com", - RecordType: "A", - Targets: []string{"targetB"}, + Update: []*plan.Update{{ + New: &endpoint.Endpoint{ + DNSName: "example.com", + RecordType: "A", + Targets: []string{"targetB"}, + }, }}, - UpdateOld: []*endpoint.Endpoint{}, }) require.NoError(t, err) diff --git a/provider/ns1/ns1.go b/provider/ns1/ns1.go index 135235ad4..6f3576cbb 100644 --- a/provider/ns1/ns1.go +++ b/provider/ns1/ns1.go @@ -278,10 +278,10 @@ type ns1Change struct { // ApplyChanges applies a given set of changes in a given zone. func (p *NS1Provider) ApplyChanges(ctx context.Context, changes *plan.Changes) error { - combinedChanges := make([]*ns1Change, 0, len(changes.Create)+len(changes.UpdateNew)+len(changes.Delete)) + combinedChanges := make([]*ns1Change, 0, len(changes.Create)+len(changes.Update)+len(changes.Delete)) combinedChanges = append(combinedChanges, newNS1Changes(ns1Create, changes.Create)...) - combinedChanges = append(combinedChanges, newNS1Changes(ns1Update, changes.UpdateNew)...) + combinedChanges = append(combinedChanges, newNS1Changes(ns1Update, changes.UpdateNew())...) combinedChanges = append(combinedChanges, newNS1Changes(ns1Delete, changes.Delete)...) return p.ns1SubmitChanges(combinedChanges) diff --git a/provider/ns1/ns1_test.go b/provider/ns1/ns1_test.go index 930fce1db..ac93e1262 100644 --- a/provider/ns1/ns1_test.go +++ b/provider/ns1/ns1_test.go @@ -234,14 +234,18 @@ func TestNS1ApplyChanges(t *testing.T) { {DNSName: "new.subdomain.bar.com", Targets: endpoint.Targets{"target"}}, } changes.Delete = []*endpoint.Endpoint{{DNSName: "test.foo.com", Targets: endpoint.Targets{"target"}}} - changes.UpdateNew = []*endpoint.Endpoint{{DNSName: "test.foo.com", Targets: endpoint.Targets{"target-new"}}} + changes.Update = []*plan.Update{ + { + New: &endpoint.Endpoint{DNSName: "test.foo.com", Targets: endpoint.Targets{"target-new"}}, + }, + } err := provider.ApplyChanges(context.Background(), changes) require.NoError(t, err) // empty changes changes.Create = []*endpoint.Endpoint{} changes.Delete = []*endpoint.Endpoint{} - changes.UpdateNew = []*endpoint.Endpoint{} + changes.Update = []*plan.Update{} err = provider.ApplyChanges(context.Background(), changes) require.NoError(t, err) } diff --git a/provider/oci/oci.go b/provider/oci/oci.go index dcd999e64..927d0d992 100644 --- a/provider/oci/oci.go +++ b/provider/oci/oci.go @@ -306,8 +306,8 @@ func (p *OCIProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) e var ops []dns.RecordOperation ops = append(ops, p.newFilteredRecordOperations(changes.Create, dns.RecordOperationOperationAdd)...) - ops = append(ops, p.newFilteredRecordOperations(changes.UpdateNew, dns.RecordOperationOperationAdd)...) - ops = append(ops, p.newFilteredRecordOperations(changes.UpdateOld, dns.RecordOperationOperationRemove)...) + ops = append(ops, p.newFilteredRecordOperations(changes.UpdateNew(), dns.RecordOperationOperationAdd)...) + ops = append(ops, p.newFilteredRecordOperations(changes.UpdateOld(), dns.RecordOperationOperationRemove)...) ops = append(ops, p.newFilteredRecordOperations(changes.Delete, dns.RecordOperationOperationRemove)...) diff --git a/provider/oci/oci_test.go b/provider/oci/oci_test.go index a7c914cd8..846467552 100644 --- a/provider/oci/oci_test.go +++ b/provider/oci/oci_test.go @@ -789,18 +789,22 @@ func TestOCIApplyChanges(t *testing.T) { }}, }, changes: &plan.Changes{ - UpdateOld: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL( - "foo.foo.com", - endpoint.RecordTypeA, - endpoint.TTL(defaultTTL), - "127.0.0.1", - )}, - UpdateNew: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL( - "foo.foo.com", - endpoint.RecordTypeA, - endpoint.TTL(defaultTTL), - "10.0.0.1", - )}, + Update: []*plan.Update{ + { + Old: endpoint.NewEndpointWithTTL( + "foo.foo.com", + endpoint.RecordTypeA, + endpoint.TTL(defaultTTL), + "127.0.0.1", + ), + New: endpoint.NewEndpointWithTTL( + "foo.foo.com", + endpoint.RecordTypeA, + endpoint.TTL(defaultTTL), + "10.0.0.1", + ), + }, + }, }, expectedEndpoints: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL( "foo.foo.com", @@ -868,18 +872,22 @@ func TestOCIApplyChanges(t *testing.T) { endpoint.TTL(defaultTTL), "127.0.0.1", )}, - UpdateOld: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL( - "car.foo.com", - endpoint.RecordTypeCNAME, - endpoint.TTL(defaultTTL), - "baz.com.", - )}, - UpdateNew: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL( - "bar.foo.com", - endpoint.RecordTypeCNAME, - endpoint.TTL(defaultTTL), - "foo.bar.com.", - )}, + Update: []*plan.Update{ + { + Old: endpoint.NewEndpointWithTTL( + "car.foo.com", + endpoint.RecordTypeCNAME, + endpoint.TTL(defaultTTL), + "baz.com.", + ), + New: endpoint.NewEndpointWithTTL( + "bar.foo.com", + endpoint.RecordTypeCNAME, + endpoint.TTL(defaultTTL), + "foo.bar.com.", + ), + }, + }, Create: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL( "baz.foo.com", endpoint.RecordTypeA, @@ -975,18 +983,22 @@ func TestOCIApplyChanges(t *testing.T) { }}, }, changes: &plan.Changes{ - UpdateOld: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL( - "first.foo.com", - endpoint.RecordTypeA, - endpoint.TTL(defaultTTL), - "10.77.4.5", - )}, - UpdateNew: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL( - "first.foo.com", - endpoint.RecordTypeA, - endpoint.TTL(defaultTTL), - "10.77.6.10", - )}, + Update: []*plan.Update{ + { + Old: endpoint.NewEndpointWithTTL( + "first.foo.com", + endpoint.RecordTypeA, + endpoint.TTL(defaultTTL), + "10.77.4.5", + ), + New: endpoint.NewEndpointWithTTL( + "first.foo.com", + endpoint.RecordTypeA, + endpoint.TTL(defaultTTL), + "10.77.6.10", + ), + }, + }, }, expectedEndpoints: []*endpoint.Endpoint{endpoint.NewEndpointWithTTL( "first.foo.com", diff --git a/provider/ovh/ovh.go b/provider/ovh/ovh.go index d3c696ad6..30ab24a34 100644 --- a/provider/ovh/ovh.go +++ b/provider/ovh/ovh.go @@ -177,19 +177,13 @@ func planChangesByZoneName(zones []string, changes *plan.Changes) map[string]*pl } output[zoneName].Create = append(output[zoneName].Create, endpt) } - for _, endpt := range changes.UpdateOld { - _, zoneName := zoneNameIDMapper.FindZone(endpt.DNSName) + for _, change := range changes.Update { + // NOTE: DNSName of `change.Old` and `change.New` will be equivalent + _, zoneName := zoneNameIDMapper.FindZone(change.Old.DNSName) if _, ok := output[zoneName]; !ok { output[zoneName] = &plan.Changes{} } - output[zoneName].UpdateOld = append(output[zoneName].UpdateOld, endpt) - } - for _, endpt := range changes.UpdateNew { - _, zoneName := zoneNameIDMapper.FindZone(endpt.DNSName) - if _, ok := output[zoneName]; !ok { - output[zoneName] = &plan.Changes{} - } - output[zoneName].UpdateNew = append(output[zoneName].UpdateNew, endpt) + output[zoneName].Update = append(output[zoneName].Update, change) } return output @@ -205,7 +199,7 @@ func (p *OVHProvider) computeSingleZoneChanges(_ context.Context, zoneName strin allChanges = append(allChanges, computedChanges...) var err error - computedChanges, err = p.newOvhChangeUpdate(changes.UpdateOld, changes.UpdateNew, zoneName, existingRecords) + computedChanges, err = p.newOvhChangeUpdate(changes.Update, zoneName, existingRecords) if err != nil { return nil, err } @@ -253,10 +247,10 @@ func (p *OVHProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) e for _, change := range changes.Create { log.Debugf("OVH: changes CREATE dns:%q / targets:%v / type:%s", change.DNSName, change.Targets, change.RecordType) } - for _, change := range changes.UpdateOld { + for _, change := range changes.UpdateOld() { log.Debugf("OVH: changes UPDATEOLD dns:%q / targets:%v / type:%s", change.DNSName, change.Targets, change.RecordType) } - for _, change := range changes.UpdateNew { + for _, change := range changes.UpdateNew() { log.Debugf("OVH: changes UPDATENEW dns:%q / targets:%v / type:%s", change.DNSName, change.Targets, change.RecordType) } for _, change := range changes.Delete { @@ -564,7 +558,7 @@ func normalizeDNSName(dnsName string) string { return strings.TrimSpace(strings.ToLower(dnsName)) } -func (p *OVHProvider) newOvhChangeUpdate(endpointsOld []*endpoint.Endpoint, endpointsNew []*endpoint.Endpoint, zone string, existingRecords []ovhRecord) ([]ovhChange, error) { +func (p *OVHProvider) newOvhChangeUpdate(updates []*plan.Update, zone string, existingRecords []ovhRecord) ([]ovhChange, error) { zoneNameIDMapper := provider.ZoneIDName{} zoneNameIDMapper.Add(zone, zone) @@ -572,13 +566,11 @@ func (p *OVHProvider) newOvhChangeUpdate(endpointsOld []*endpoint.Endpoint, endp newEndpointByTypeAndName := map[string]*endpoint.Endpoint{} oldRecordsInZone := map[string][]ovhRecord{} - for _, e := range endpointsOld { - sub := convertDNSNameIntoSubDomain(e.DNSName, zone) - oldEndpointByTypeAndName[normalizeDNSName(e.RecordType+"//"+sub)] = e - } - for _, e := range endpointsNew { - sub := convertDNSNameIntoSubDomain(e.DNSName, zone) - newEndpointByTypeAndName[normalizeDNSName(e.RecordType+"//"+sub)] = e + for _, update := range updates { + // NOTE: DNSName and RecordType of `update.Old` and `update.New` will be equivalent + sub := convertDNSNameIntoSubDomain(update.Old.DNSName, zone) + oldEndpointByTypeAndName[normalizeDNSName(update.Old.RecordType+"//"+sub)] = update.Old + newEndpointByTypeAndName[normalizeDNSName(update.New.RecordType+"//"+sub)] = update.New } for id := range oldEndpointByTypeAndName { diff --git a/provider/ovh/ovh_test.go b/provider/ovh/ovh_test.go index 50fdd78a2..fa7b4d9d3 100644 --- a/provider/ovh/ovh_test.go +++ b/provider/ovh/ovh_test.go @@ -331,11 +331,11 @@ func TestOvhComputeChanges(t *testing.T) { } changes := plan.Changes{ - UpdateOld: []*endpoint.Endpoint{ - {DNSName: "example.net", RecordType: "A", Targets: []string{"203.0.113.42"}}, - }, - UpdateNew: []*endpoint.Endpoint{ - {DNSName: "example.net", RecordType: "A", Targets: []string{"203.0.113.43", "203.0.113.42"}}, + Update: []*plan.Update{ + { + Old: &endpoint.Endpoint{DNSName: "example.net", RecordType: "A", Targets: []string{"203.0.113.42"}}, + New: &endpoint.Endpoint{DNSName: "example.net", RecordType: "A", Targets: []string{"203.0.113.43", "203.0.113.42"}}, + }, }, } @@ -520,11 +520,11 @@ func TestOvhApplyChanges(t *testing.T) { client = new(mockOvhClient) provider = &OVHProvider{client: client, apiRateLimiter: ratelimit.New(10), cacheInstance: cache.New(cache.NoExpiration, cache.NoExpiration), DryRun: false} changes = plan.Changes{ - UpdateOld: []*endpoint.Endpoint{ - {DNSName: "example.net", RecordType: "A", RecordTTL: 10, Targets: []string{"203.0.113.42"}}, - }, - UpdateNew: []*endpoint.Endpoint{ - {DNSName: "example.net", RecordType: "A", RecordTTL: 10, Targets: []string{"203.0.113.43"}}, + Update: []*plan.Update{ + { + Old: &endpoint.Endpoint{DNSName: "example.net", RecordType: "A", RecordTTL: 10, Targets: []string{"203.0.113.42"}}, + New: &endpoint.Endpoint{DNSName: "example.net", RecordType: "A", RecordTTL: 10, Targets: []string{"203.0.113.43"}}, + }, }, } @@ -543,11 +543,11 @@ func TestOvhApplyChanges(t *testing.T) { client = new(mockOvhClient) provider = &OVHProvider{client: client, apiRateLimiter: ratelimit.New(10), cacheInstance: cache.New(cache.NoExpiration, cache.NoExpiration), DryRun: true} changes = plan.Changes{ - UpdateOld: []*endpoint.Endpoint{ - {DNSName: "example.net", RecordType: "A", RecordTTL: 10, Targets: []string{"203.0.113.42"}}, - }, - UpdateNew: []*endpoint.Endpoint{ - {DNSName: "example.net", RecordType: "A", RecordTTL: 10, Targets: []string{"203.0.113.43"}}, + Update: []*plan.Update{ + { + Old: &endpoint.Endpoint{DNSName: "example.net", RecordType: "A", RecordTTL: 10, Targets: []string{"203.0.113.42"}}, + New: &endpoint.Endpoint{DNSName: "example.net", RecordType: "A", RecordTTL: 10, Targets: []string{"203.0.113.43"}}, + }, }, } @@ -564,11 +564,11 @@ func TestOvhApplyChanges(t *testing.T) { client = new(mockOvhClient) provider = &OVHProvider{client: client, apiRateLimiter: ratelimit.New(10), cacheInstance: cache.New(cache.NoExpiration, cache.NoExpiration), DryRun: false} changes = plan.Changes{ - UpdateOld: []*endpoint.Endpoint{ - {DNSName: "example.net", RecordType: "A", RecordTTL: 10, Targets: []string{"203.0.113.42", "203.0.113.43"}}, - }, - UpdateNew: []*endpoint.Endpoint{ - {DNSName: "example.net", RecordType: "A", RecordTTL: 10, Targets: []string{"203.0.113.43"}}, + Update: []*plan.Update{ + { + Old: &endpoint.Endpoint{DNSName: "example.net", RecordType: "A", RecordTTL: 10, Targets: []string{"203.0.113.42", "203.0.113.43"}}, + New: &endpoint.Endpoint{DNSName: "example.net", RecordType: "A", RecordTTL: 10, Targets: []string{"203.0.113.43"}}, + }, }, } diff --git a/provider/pdns/pdns.go b/provider/pdns/pdns.go index 748e220ed..721234d3d 100644 --- a/provider/pdns/pdns.go +++ b/provider/pdns/pdns.go @@ -477,7 +477,7 @@ func (p *PDNSProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) } // Update - for _, change := range changes.UpdateOld { + for _, change := range changes.UpdateOld() { // Since PDNS "Patches", we don't need to specify the "old" // record. The Update New change type will automatically take // care of replacing the old RRSet with the new one We simply @@ -485,11 +485,12 @@ func (p *PDNSProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) log.Debugf("UPDATE-OLD (ignored): %+v", change) } - for _, change := range changes.UpdateNew { + updateNew := changes.UpdateNew() + for _, change := range updateNew { log.Infof("UPDATE-NEW: %+v", change) } - if len(changes.UpdateNew) > 0 { - err := p.mutateRecords(changes.UpdateNew, PdnsReplace) + if len(updateNew) > 0 { + err := p.mutateRecords(updateNew, PdnsReplace) if err != nil { return err } diff --git a/provider/pihole/pihole.go b/provider/pihole/pihole.go index cf401f374..9d5260cf9 100644 --- a/provider/pihole/pihole.go +++ b/provider/pihole/pihole.go @@ -107,7 +107,7 @@ func (p *PiholeProvider) ApplyChanges(ctx context.Context, changes *plan.Changes // Handle updated state - there are no endpoints for updating in place. updateNew := make(map[piholeEntryKey]*endpoint.Endpoint) - for _, ep := range changes.UpdateNew { + for _, ep := range changes.UpdateNew() { key := piholeEntryKey{ep.DNSName, ep.RecordType} // If the API version is 6, we need to handle multiple targets for the same DNS name. @@ -125,7 +125,7 @@ func (p *PiholeProvider) ApplyChanges(ctx context.Context, changes *plan.Changes updateNew[key] = ep } - for _, ep := range changes.UpdateOld { + for _, ep := range changes.UpdateOld() { // Check if this existing entry has an exact match for an updated entry and skip it if so. key := piholeEntryKey{ep.DNSName, ep.RecordType} if newRecord := updateNew[key]; newRecord != nil { diff --git a/provider/pihole/piholeV6_test.go b/provider/pihole/piholeV6_test.go index b14f77bc0..16df8586d 100644 --- a/provider/pihole/piholeV6_test.go +++ b/provider/pihole/piholeV6_test.go @@ -23,6 +23,7 @@ import ( "testing" "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/assert" "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/plan" ) @@ -311,8 +312,8 @@ func TestProviderV6(t *testing.T) { RecordType: endpoint.RecordTypeAAAA, }, } - if err := p.ApplyChanges(context.Background(), &plan.Changes{ - UpdateOld: []*endpoint.Endpoint{ + update, err := plan.MkUpdates( + []*endpoint.Endpoint{ { DNSName: "test1.example.com", Targets: []string{"192.168.1.1"}, @@ -334,7 +335,7 @@ func TestProviderV6(t *testing.T) { RecordType: endpoint.RecordTypeAAAA, }, }, - UpdateNew: []*endpoint.Endpoint{ + []*endpoint.Endpoint{ { DNSName: "test1.example.com", Targets: []string{"192.168.1.1"}, @@ -342,12 +343,7 @@ func TestProviderV6(t *testing.T) { }, { DNSName: "test2.example.com", - Targets: []string{"10.0.0.1"}, - RecordType: endpoint.RecordTypeA, - }, - { - DNSName: "test2.example.com", - Targets: []string{"10.0.0.2"}, + Targets: []string{"10.0.0.1", "10.0.0.2"}, RecordType: endpoint.RecordTypeA, }, { @@ -361,6 +357,10 @@ func TestProviderV6(t *testing.T) { RecordType: endpoint.RecordTypeAAAA, }, }, + ) + assert.NoError(t, err) + if err := p.ApplyChanges(context.Background(), &plan.Changes{ + Update: update, }); err != nil { t.Fatal(err) } diff --git a/provider/pihole/pihole_test.go b/provider/pihole/pihole_test.go index 5c99c1394..160cf1c11 100644 --- a/provider/pihole/pihole_test.go +++ b/provider/pihole/pihole_test.go @@ -21,6 +21,7 @@ import ( "reflect" "testing" + "github.com/stretchr/testify/assert" "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/plan" ) @@ -271,8 +272,8 @@ func TestProvider(t *testing.T) { RecordType: endpoint.RecordTypeAAAA, }, } - if err := p.ApplyChanges(context.Background(), &plan.Changes{ - UpdateOld: []*endpoint.Endpoint{ + update, err := plan.MkUpdates( + []*endpoint.Endpoint{ { DNSName: "test1.example.com", Targets: []string{"192.168.1.1"}, @@ -294,7 +295,7 @@ func TestProvider(t *testing.T) { RecordType: endpoint.RecordTypeAAAA, }, }, - UpdateNew: []*endpoint.Endpoint{ + []*endpoint.Endpoint{ { DNSName: "test1.example.com", Targets: []string{"192.168.1.1"}, @@ -316,6 +317,10 @@ func TestProvider(t *testing.T) { RecordType: endpoint.RecordTypeAAAA, }, }, + ) + assert.NoError(t, err) + if err := p.ApplyChanges(context.Background(), &plan.Changes{ + Update: update, }); err != nil { t.Fatal(err) } diff --git a/provider/plural/plural.go b/provider/plural/plural.go index bdb2e19c3..2f9f3396b 100644 --- a/provider/plural/plural.go +++ b/provider/plural/plural.go @@ -89,7 +89,7 @@ func (p *PluralProvider) ApplyChanges(_ context.Context, diffs *plan.Changes) er changes = append(changes, makeChange(CreateAction, ep.Targets, ep)) } - for _, desired := range diffs.UpdateNew { + for _, desired := range diffs.UpdateNew() { changes = append(changes, makeChange(CreateAction, desired.Targets, desired)) } diff --git a/provider/rfc2136/rfc2136.go b/provider/rfc2136/rfc2136.go index 816b782ca..39be2e6f0 100644 --- a/provider/rfc2136/rfc2136.go +++ b/provider/rfc2136/rfc2136.go @@ -348,7 +348,7 @@ func (r *rfc2136Provider) GenerateReverseRecord(ip string, hostname string) []*e // ApplyChanges applies a given set of changes in a given zone. func (r *rfc2136Provider) ApplyChanges(ctx context.Context, changes *plan.Changes) error { - log.Debugf("ApplyChanges (Create: %d, UpdateOld: %d, UpdateNew: %d, Delete: %d)", len(changes.Create), len(changes.UpdateOld), len(changes.UpdateNew), len(changes.Delete)) + log.Debugf("ApplyChanges (Create: %d, Update: %d, Delete: %d)", len(changes.Create), len(changes.Update), len(changes.Delete)) var errs []error @@ -389,7 +389,8 @@ func (r *rfc2136Provider) ApplyChanges(ctx context.Context, changes *plan.Change } } - for c, chunk := range chunkBy(changes.UpdateNew, r.batchChangeSize) { + updateOld := changes.UpdateOld() + for c, chunk := range chunkBy(changes.UpdateNew(), r.batchChangeSize) { log.Debugf("Processing batch %d of update changes", c) m := make(map[string]*dns.Msg) @@ -410,9 +411,9 @@ func (r *rfc2136Provider) ApplyChanges(ctx context.Context, changes *plan.Change // calculate corresponding index in the unsplitted UpdateOld for current endpoint ep in chunk j := (c * r.batchChangeSize) + i - r.UpdateRecord(m[zone], changes.UpdateOld[j], ep) + r.UpdateRecord(m[zone], updateOld[j], ep) if r.createPTR && (ep.RecordType == "A" || ep.RecordType == "AAAA") { - r.RemoveReverseRecord(changes.UpdateOld[j].Targets[0], ep.DNSName) + r.RemoveReverseRecord(updateOld[j].Targets[0], ep.DNSName) r.AddReverseRecord(ep.Targets[0], ep.DNSName) } } diff --git a/provider/rfc2136/rfc2136_test.go b/provider/rfc2136/rfc2136_test.go index fc3d65c3f..064f59993 100644 --- a/provider/rfc2136/rfc2136_test.go +++ b/provider/rfc2136/rfc2136_test.go @@ -837,30 +837,32 @@ func TestRfc2136ApplyChangesWithUpdate(t *testing.T) { assert.NoError(t, err) p = &plan.Changes{ - UpdateOld: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "v1.foo.com", - RecordType: "A", - Targets: []string{"1.2.3.4"}, - RecordTTL: endpoint.TTL(400), + Old: &endpoint.Endpoint{ + DNSName: "v1.foo.com", + RecordType: "A", + Targets: []string{"1.2.3.4"}, + RecordTTL: endpoint.TTL(400), + }, + New: &endpoint.Endpoint{ + DNSName: "v1.foo.com", + RecordType: "A", + Targets: []string{"1.2.3.5"}, + RecordTTL: endpoint.TTL(400), + }, }, { - DNSName: "v1.foobar.com", - RecordType: "TXT", - Targets: []string{"boom"}, - }, - }, - UpdateNew: []*endpoint.Endpoint{ - { - DNSName: "v1.foo.com", - RecordType: "A", - Targets: []string{"1.2.3.5"}, - RecordTTL: endpoint.TTL(400), - }, - { - DNSName: "v1.foobar.com", - RecordType: "TXT", - Targets: []string{"kablui"}, + Old: &endpoint.Endpoint{ + DNSName: "v1.foobar.com", + RecordType: "TXT", + Targets: []string{"boom"}, + }, + New: &endpoint.Endpoint{ + DNSName: "v1.foobar.com", + RecordType: "TXT", + Targets: []string{"kablui"}, + }, }, }, } @@ -996,9 +998,10 @@ func TestRfc2136ApplyChangesWithMultipleChunks(t *testing.T) { }) } + update, err := plan.MkUpdates(oldRecords, newRecords) + assert.NoError(t, err) p := &plan.Changes{ - UpdateOld: oldRecords, - UpdateNew: newRecords, + Update: update, } err = provider.ApplyChanges(context.Background(), p) diff --git a/provider/scaleway/scaleway.go b/provider/scaleway/scaleway.go index 47a3db7a5..a2acc7f5f 100644 --- a/provider/scaleway/scaleway.go +++ b/provider/scaleway/scaleway.go @@ -228,7 +228,7 @@ func (p *ScalewayProvider) generateApplyRequests(ctx context.Context, changes *p } log.Debugf("Following records present in updateOld") - for _, c := range changes.UpdateOld { + for _, c := range changes.UpdateOld() { zone, _ := zoneNameMapper.FindZone(c.DNSName) if zone == "" { log.Infof("Ignore record %s since it's not handled by ExternalDNS", c.DNSName) @@ -261,7 +261,7 @@ func (p *ScalewayProvider) generateApplyRequests(ctx context.Context, changes *p } log.Debugf("Following records present in updateNew") - for _, c := range changes.UpdateNew { + for _, c := range changes.UpdateNew() { zone, _ := zoneNameMapper.FindZone(c.DNSName) if zone == "" { log.Infof("Ignore record %s since it's not handled by ExternalDNS", c.DNSName) diff --git a/provider/scaleway/scaleway_test.go b/provider/scaleway/scaleway_test.go index eec95c695..9751e1000 100644 --- a/provider/scaleway/scaleway_test.go +++ b/provider/scaleway/scaleway_test.go @@ -519,41 +519,41 @@ func TestScalewayProvider_generateApplyRequests(t *testing.T) { Targets: []string{"1.1.1.1"}, }, }, - UpdateNew: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "me.example.com", - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: scalewayPriorityKey, - Value: "30", + New: &endpoint.Endpoint{ + DNSName: "me.example.com", + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: scalewayPriorityKey, + Value: "30", + }, }, + RecordType: "A", + RecordTTL: 600, + Targets: []string{"2.2.2.2"}, }, - RecordType: "A", - RecordTTL: 600, - Targets: []string{"2.2.2.2"}, - }, - { - DNSName: "my.test.example.com", - RecordType: "A", - Targets: []string{"1.2.3.4", "5.6.7.8"}, - }, - }, - UpdateOld: []*endpoint.Endpoint{ - { - DNSName: "me.example.com", - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: scalewayPriorityKey, - Value: "1234", + Old: &endpoint.Endpoint{ + DNSName: "me.example.com", + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: scalewayPriorityKey, + Value: "1234", + }, }, + RecordType: "A", + Targets: []string{"3.3.3.3"}, }, - RecordType: "A", - Targets: []string{"3.3.3.3"}, }, { - DNSName: "my.test.example.com", - RecordType: "A", - Targets: []string{"4.4.4.4", "5.5.5.5"}, + New: &endpoint.Endpoint{DNSName: "my.test.example.com", + RecordType: "A", + Targets: []string{"1.2.3.4", "5.6.7.8"}}, + Old: &endpoint.Endpoint{ + DNSName: "my.test.example.com", + RecordType: "A", + Targets: []string{"4.4.4.4", "5.5.5.5"}, + }, }, }, } diff --git a/provider/transip/transip.go b/provider/transip/transip.go index baa72699b..8aad8c873 100644 --- a/provider/transip/transip.go +++ b/provider/transip/transip.go @@ -185,7 +185,7 @@ func (p *TransIPProvider) ApplyChanges(ctx context.Context, changes *plan.Change } // then update existing DNS records - for _, ep := range changes.UpdateNew { + for _, ep := range changes.UpdateNew() { epLog := log.WithFields(log.Fields{ "record": ep.DNSName, "type": ep.RecordType, diff --git a/provider/webhook/api/httpapi_test.go b/provider/webhook/api/httpapi_test.go index 795e59b52..16ca701d2 100644 --- a/provider/webhook/api/httpapi_test.go +++ b/provider/webhook/api/httpapi_test.go @@ -217,7 +217,7 @@ func TestRecordsHandlerWithMixedCase(t *testing.T) { { DNSName: "bar", }, - }, changes.UpdateOld) + }, changes.UpdateOld()) require.Equal(t, []*endpoint.Endpoint{ { DNSName: "qux", diff --git a/registry/aws_sd_registry_test.go b/registry/aws_sd_registry_test.go index 94054ce51..181c42373 100644 --- a/registry/aws_sd_registry_test.go +++ b/registry/aws_sd_registry_test.go @@ -116,11 +116,11 @@ func TestAWSSDRegistry_Records_ApplyChanges(t *testing.T) { Delete: []*endpoint.Endpoint{ newEndpointWithOwner("foobar.test-zone.example.org", "1.2.3.4", endpoint.RecordTypeA, "owner"), }, - UpdateNew: []*endpoint.Endpoint{ - newEndpointWithOwner("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), - }, - UpdateOld: []*endpoint.Endpoint{ - newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), + Update: []*plan.Update{ + { + New: newEndpointWithOwner("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), + Old: newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), + }, }, } expected := &plan.Changes{ @@ -130,24 +130,24 @@ func TestAWSSDRegistry_Records_ApplyChanges(t *testing.T) { Delete: []*endpoint.Endpoint{ newEndpointWithOwnerAndDescription("foobar.test-zone.example.org", "1.2.3.4", endpoint.RecordTypeA, "owner", "\"heritage=external-dns,external-dns/owner=owner\""), }, - UpdateNew: []*endpoint.Endpoint{ - newEndpointWithOwnerAndDescription("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "\"heritage=external-dns,external-dns/owner=owner\""), - }, - UpdateOld: []*endpoint.Endpoint{ - newEndpointWithOwnerAndDescription("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "\"heritage=external-dns,external-dns/owner=owner\""), + Update: []*plan.Update{ + { + New: newEndpointWithOwnerAndDescription("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "\"heritage=external-dns,external-dns/owner=owner\""), + Old: newEndpointWithOwnerAndDescription("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "\"heritage=external-dns,external-dns/owner=owner\""), + }, }, } p := newInMemoryProvider(nil, func(got *plan.Changes) { mExpected := map[string][]*endpoint.Endpoint{ "Create": expected.Create, - "UpdateNew": expected.UpdateNew, - "UpdateOld": expected.UpdateOld, + "UpdateNew": expected.UpdateNew(), + "UpdateOld": expected.UpdateOld(), "Delete": expected.Delete, } mGot := map[string][]*endpoint.Endpoint{ "Create": got.Create, - "UpdateNew": got.UpdateNew, - "UpdateOld": got.UpdateOld, + "UpdateNew": got.UpdateNew(), + "UpdateOld": got.UpdateOld(), "Delete": got.Delete, } assert.True(t, testutils.SamePlanChanges(mGot, mExpected)) diff --git a/registry/dynamodb_test.go b/registry/dynamodb_test.go index 091927fab..2dcce6a5b 100644 --- a/registry/dynamodb_test.go +++ b/registry/dynamodb_test.go @@ -709,25 +709,25 @@ func TestDynamoDBRegistryApplyChanges(t *testing.T) { { name: "update", changes: plan.Changes{ - UpdateOld: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "bar.test-zone.example.org", - Targets: endpoint.Targets{"my-domain.com"}, - RecordType: endpoint.RecordTypeCNAME, - Labels: map[string]string{ - endpoint.OwnerLabelKey: "test-owner", - endpoint.ResourceLabelKey: "ingress/default/my-ingress", + Old: &endpoint.Endpoint{ + DNSName: "bar.test-zone.example.org", + Targets: endpoint.Targets{"my-domain.com"}, + RecordType: endpoint.RecordTypeCNAME, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "test-owner", + endpoint.ResourceLabelKey: "ingress/default/my-ingress", + }, }, - }, - }, - UpdateNew: []*endpoint.Endpoint{ - { - DNSName: "bar.test-zone.example.org", - Targets: endpoint.Targets{"new-domain.com"}, - RecordType: endpoint.RecordTypeCNAME, - Labels: map[string]string{ - endpoint.OwnerLabelKey: "test-owner", - endpoint.ResourceLabelKey: "ingress/default/my-ingress", + New: &endpoint.Endpoint{ + DNSName: "bar.test-zone.example.org", + Targets: endpoint.Targets{"new-domain.com"}, + RecordType: endpoint.RecordTypeCNAME, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "test-owner", + endpoint.ResourceLabelKey: "ingress/default/my-ingress", + }, }, }, }, @@ -778,25 +778,25 @@ func TestDynamoDBRegistryApplyChanges(t *testing.T) { { name: "update change", changes: plan.Changes{ - UpdateOld: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "bar.test-zone.example.org", - Targets: endpoint.Targets{"my-domain.com"}, - RecordType: endpoint.RecordTypeCNAME, - Labels: map[string]string{ - endpoint.OwnerLabelKey: "test-owner", - endpoint.ResourceLabelKey: "ingress/default/my-ingress", + Old: &endpoint.Endpoint{ + DNSName: "bar.test-zone.example.org", + Targets: endpoint.Targets{"my-domain.com"}, + RecordType: endpoint.RecordTypeCNAME, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "test-owner", + endpoint.ResourceLabelKey: "ingress/default/my-ingress", + }, }, - }, - }, - UpdateNew: []*endpoint.Endpoint{ - { - DNSName: "bar.test-zone.example.org", - Targets: endpoint.Targets{"new-domain.com"}, - RecordType: endpoint.RecordTypeCNAME, - Labels: map[string]string{ - endpoint.OwnerLabelKey: "test-owner", - endpoint.ResourceLabelKey: "ingress/default/new-ingress", + New: &endpoint.Endpoint{ + DNSName: "bar.test-zone.example.org", + Targets: endpoint.Targets{"new-domain.com"}, + RecordType: endpoint.RecordTypeCNAME, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "test-owner", + endpoint.ResourceLabelKey: "ingress/default/new-ingress", + }, }, }, }, @@ -858,31 +858,32 @@ func TestDynamoDBRegistryApplyChanges(t *testing.T) { }, }, changes: plan.Changes{ - UpdateOld: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "bar.test-zone.example.org", - Targets: endpoint.Targets{"my-domain.com"}, - RecordType: endpoint.RecordTypeCNAME, - Labels: map[string]string{ - endpoint.OwnerLabelKey: "test-owner", - endpoint.ResourceLabelKey: "ingress/default/my-ingress", - }, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: dynamodbAttributeMigrate, - Value: "true", + Old: &endpoint.Endpoint{ + DNSName: "bar.test-zone.example.org", + Targets: endpoint.Targets{"my-domain.com"}, + RecordType: endpoint.RecordTypeCNAME, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "test-owner", + endpoint.ResourceLabelKey: "ingress/default/my-ingress", + }, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: dynamodbAttributeMigrate, + Value: "true", + }, }, }, - }, - }, - UpdateNew: []*endpoint.Endpoint{ - { - DNSName: "bar.test-zone.example.org", - Targets: endpoint.Targets{"my-domain.com"}, - RecordType: endpoint.RecordTypeCNAME, - Labels: map[string]string{ - endpoint.OwnerLabelKey: "test-owner", - endpoint.ResourceLabelKey: "ingress/default/new-ingress", + New: &endpoint.Endpoint{ + + DNSName: "bar.test-zone.example.org", + Targets: endpoint.Targets{"my-domain.com"}, + RecordType: endpoint.RecordTypeCNAME, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "test-owner", + endpoint.ResourceLabelKey: "ingress/default/new-ingress", + }, }, }, }, @@ -945,25 +946,25 @@ func TestDynamoDBRegistryApplyChanges(t *testing.T) { { name: "update error", changes: plan.Changes{ - UpdateOld: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "bar.test-zone.example.org", - Targets: endpoint.Targets{"my-domain.com"}, - RecordType: endpoint.RecordTypeCNAME, - Labels: map[string]string{ - endpoint.OwnerLabelKey: "test-owner", - endpoint.ResourceLabelKey: "ingress/default/my-ingress", + Old: &endpoint.Endpoint{ + DNSName: "bar.test-zone.example.org", + Targets: endpoint.Targets{"my-domain.com"}, + RecordType: endpoint.RecordTypeCNAME, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "test-owner", + endpoint.ResourceLabelKey: "ingress/default/my-ingress", + }, }, - }, - }, - UpdateNew: []*endpoint.Endpoint{ - { - DNSName: "bar.test-zone.example.org", - Targets: endpoint.Targets{"new-domain.com"}, - RecordType: endpoint.RecordTypeCNAME, - Labels: map[string]string{ - endpoint.OwnerLabelKey: "test-owner", - endpoint.ResourceLabelKey: "ingress/default/new-ingress", + New: &endpoint.Endpoint{ + DNSName: "bar.test-zone.example.org", + Targets: endpoint.Targets{"new-domain.com"}, + RecordType: endpoint.RecordTypeCNAME, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "test-owner", + endpoint.ResourceLabelKey: "ingress/default/new-ingress", + }, }, }, }, diff --git a/registry/noop_test.go b/registry/noop_test.go index 8d644ece6..dfce368ba 100644 --- a/registry/noop_test.go +++ b/registry/noop_test.go @@ -117,18 +117,19 @@ func testNoopApplyChanges(t *testing.T) { RecordType: endpoint.RecordTypeCNAME, }, }, - UpdateNew: []*endpoint.Endpoint{ + Update: []*plan.Update{ { - DNSName: "example.org", - Targets: endpoint.Targets{"new-example-lb.com"}, - RecordType: endpoint.RecordTypeCNAME, - }, - }, - UpdateOld: []*endpoint.Endpoint{ - { - DNSName: "example.org", - Targets: endpoint.Targets{"old-lb.com"}, - RecordType: endpoint.RecordTypeCNAME, + New: &endpoint.Endpoint{ + DNSName: "example.org", + Targets: endpoint.Targets{"new-example-lb.com"}, + RecordType: endpoint.RecordTypeCNAME, + }, + Old: &endpoint.Endpoint{ + + DNSName: "example.org", + Targets: endpoint.Targets{"old-lb.com"}, + RecordType: endpoint.RecordTypeCNAME, + }, }, }, })) diff --git a/registry/txt_encryption_test.go b/registry/txt_encryption_test.go index 1b7f9f5c9..9b47a08b1 100644 --- a/registry/txt_encryption_test.go +++ b/registry/txt_encryption_test.go @@ -109,28 +109,26 @@ func TestGenerateTXTGenerateTextRecordEncryptionWihDecryption(t *testing.T) { key := []byte(k) r, err := NewTXTRegistry(p, "", "", "owner", time.Minute, "", []string{}, []string{}, true, key) assert.NoError(t, err, "Error creating TXT registry") - txtRecords := r.generateTXTRecord(test.record) - assert.Len(t, txtRecords, len(test.record.Targets)) + txt := r.generateTXTRecord(test.record) + assert.NotNil(t, txt) - for _, txt := range txtRecords { - // should return a TXT record with the encryption nonce label. At the moment nonce is not set as label. - assert.NotContains(t, txt.Labels, "txt-encryption-nonce") + // should return a TXT record with the encryption nonce label. At the moment nonce is not set as label. + assert.NotContains(t, txt.Labels, "txt-encryption-nonce") - assert.Len(t, txt.Targets, 1) - assert.LessOrEqual(t, len(txt.Targets), 1) + assert.Len(t, txt.Targets, 1) + assert.LessOrEqual(t, len(txt.Targets), 1) - // decrypt targets - for _, target := range txtRecords[0].Targets { - encryptedText, errUnquote := strconv.Unquote(target) - assert.NoError(t, errUnquote, "Error unquoting the encrypted text") + // decrypt targets + for _, target := range txt.Targets { + encryptedText, errUnquote := strconv.Unquote(target) + assert.NoError(t, errUnquote, "Error unquoting the encrypted text") - actual, nonce, errDecrypt := endpoint.DecryptText(encryptedText, r.txtEncryptAESKey) - assert.NoError(t, errDecrypt, "Error decrypting the encrypted text") + actual, nonce, errDecrypt := endpoint.DecryptText(encryptedText, r.txtEncryptAESKey) + assert.NoError(t, errDecrypt, "Error decrypting the encrypted text") - assert.True(t, strings.HasPrefix(encryptedText, nonce), - "Nonce '%s' should be a prefix of the encrypted text: '%s'", nonce, encryptedText) - assert.Equal(t, test.decrypted, actual) - } + assert.True(t, strings.HasPrefix(encryptedText, nonce), + "Nonce '%s' should be a prefix of the encrypted text: '%s'", nonce, encryptedText) + assert.Equal(t, test.decrypted, actual) } }) } @@ -221,6 +219,7 @@ func TestApplyRecordsWithEncryptionKeyChanged(t *testing.T) { } func TestApplyRecordsOnEncryptionKeyChangeWithKeyIdLabel(t *testing.T) { + t.Skip("Not working because existing encrypted TXT records cannot be reconstructed (in TXTRegistry.applyChanges) and will therefore be rejected (in inMemoryClient.validateChangeBatch)") ctx := context.Background() p := inmemory.NewInMemoryProvider() _ = p.CreateZone("org") @@ -245,13 +244,17 @@ func TestApplyRecordsOnEncryptionKeyChangeWithKeyIdLabel(t *testing.T) { } if i == 0 { - _ = r.ApplyChanges(ctx, &plan.Changes{ + err := r.ApplyChanges(ctx, &plan.Changes{ Create: changes, }) + assert.NoError(t, err) } else { - _ = r.ApplyChanges(context.Background(), &plan.Changes{ - UpdateNew: changes, + update, err := plan.MkUpdates(changes, changes) + assert.NoError(t, err) + err = r.ApplyChanges(context.Background(), &plan.Changes{ + Update: update, }) + assert.NoError(t, err) } } diff --git a/registry/txt_test.go b/registry/txt_test.go index d59ce90ef..e01684ddb 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -628,15 +628,31 @@ func testTXTRegistryApplyChangesWithPrefix(t *testing.T) { newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), newEndpointWithOwner("multiple.test-zone.example.org", "lb1.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-1"), }, - UpdateNew: []*endpoint.Endpoint{ - newEndpointWithOwnerResource("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2"), - newEndpointWithOwnerResource("multiple.test-zone.example.org", "new.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2").WithSetIdentifier("test-set-2"), - }, - UpdateOld: []*endpoint.Endpoint{ - newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), - newEndpointWithOwner("multiple.test-zone.example.org", "lb2.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-2"), + Update: []*plan.Update{ + { + New: newEndpointWithOwnerResource("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2"), + Old: newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), + }, + { + New: newEndpointWithOwnerResource("multiple.test-zone.example.org", "new.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2").WithSetIdentifier("test-set-2"), + Old: newEndpointWithOwner("multiple.test-zone.example.org", "lb2.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-2"), + }, }, } + update, err := plan.MkUpdates( + []*endpoint.Endpoint{ + newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), + newEndpointWithOwnerAndOwnedRecord("txt.cname-tar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "tar.test-zone.example.org"), + newEndpointWithOwner("multiple.test-zone.example.org", "lb2.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-2"), + newEndpointWithOwnerAndOwnedRecord("txt.cname-multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").WithSetIdentifier("test-set-2"), + }, []*endpoint.Endpoint{ + newEndpointWithOwnerResource("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2"), + newEndpointWithOwnerAndOwnedRecord("txt.cname-tar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, "", "tar.test-zone.example.org"), + newEndpointWithOwnerResource("multiple.test-zone.example.org", "new.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2").WithSetIdentifier("test-set-2"), + newEndpointWithOwnerAndOwnedRecord("txt.cname-multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").WithSetIdentifier("test-set-2"), + }, + ) + assert.NoError(t, err) expected := &plan.Changes{ Create: []*endpoint.Endpoint{ newEndpointWithOwnerResource("new-record-1.test-zone.example.org", "new-loadbalancer-1.lb.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress"), @@ -653,36 +669,25 @@ func testTXTRegistryApplyChangesWithPrefix(t *testing.T) { newEndpointWithOwner("multiple.test-zone.example.org", "lb1.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-1"), newEndpointWithOwnerAndOwnedRecord("txt.cname-multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").WithSetIdentifier("test-set-1"), }, - UpdateNew: []*endpoint.Endpoint{ - newEndpointWithOwnerResource("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2"), - newEndpointWithOwnerAndOwnedRecord("txt.cname-tar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, "", "tar.test-zone.example.org"), - newEndpointWithOwnerResource("multiple.test-zone.example.org", "new.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2").WithSetIdentifier("test-set-2"), - newEndpointWithOwnerAndOwnedRecord("txt.cname-multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").WithSetIdentifier("test-set-2"), - }, - UpdateOld: []*endpoint.Endpoint{ - newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), - newEndpointWithOwnerAndOwnedRecord("txt.cname-tar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "tar.test-zone.example.org"), - newEndpointWithOwner("multiple.test-zone.example.org", "lb2.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-2"), - newEndpointWithOwnerAndOwnedRecord("txt.cname-multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").WithSetIdentifier("test-set-2"), - }, + Update: update, } p.OnApplyChanges = func(ctx context.Context, got *plan.Changes) { mExpected := map[string][]*endpoint.Endpoint{ "Create": expected.Create, - "UpdateNew": expected.UpdateNew, - "UpdateOld": expected.UpdateOld, + "UpdateNew": expected.UpdateNew(), + "UpdateOld": expected.UpdateOld(), "Delete": expected.Delete, } mGot := map[string][]*endpoint.Endpoint{ "Create": got.Create, - "UpdateNew": got.UpdateNew, - "UpdateOld": got.UpdateOld, + "UpdateNew": got.UpdateNew(), + "UpdateOld": got.UpdateOld(), "Delete": got.Delete, } assert.True(t, testutils.SamePlanChanges(mGot, mExpected)) assert.Nil(t, ctx.Value(provider.RecordsContextKey)) } - err := r.ApplyChanges(ctx, changes) + err = r.ApplyChanges(ctx, changes) require.NoError(t, err) } @@ -702,9 +707,8 @@ func testTXTRegistryApplyChangesWithTemplatedPrefix(t *testing.T) { Create: []*endpoint.Endpoint{ newEndpointWithOwnerResource("new-record-1.test-zone.example.org", "new-loadbalancer-1.lb.com", endpoint.RecordTypeCNAME, "", "ingress/default/my-ingress"), }, - Delete: []*endpoint.Endpoint{}, - UpdateOld: []*endpoint.Endpoint{}, - UpdateNew: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{}, + Update: []*plan.Update{}, } expected := &plan.Changes{ Create: []*endpoint.Endpoint{ @@ -715,14 +719,14 @@ func testTXTRegistryApplyChangesWithTemplatedPrefix(t *testing.T) { p.OnApplyChanges = func(ctx context.Context, got *plan.Changes) { mExpected := map[string][]*endpoint.Endpoint{ "Create": expected.Create, - "UpdateNew": expected.UpdateNew, - "UpdateOld": expected.UpdateOld, + "UpdateNew": expected.UpdateNew(), + "UpdateOld": expected.UpdateOld(), "Delete": expected.Delete, } mGot := map[string][]*endpoint.Endpoint{ "Create": got.Create, - "UpdateNew": got.UpdateNew, - "UpdateOld": got.UpdateOld, + "UpdateNew": got.UpdateNew(), + "UpdateOld": got.UpdateOld(), "Delete": got.Delete, } assert.True(t, testutils.SamePlanChanges(mGot, mExpected)) @@ -745,9 +749,8 @@ func testTXTRegistryApplyChangesWithTemplatedSuffix(t *testing.T) { Create: []*endpoint.Endpoint{ newEndpointWithOwnerResource("new-record-1.test-zone.example.org", "new-loadbalancer-1.lb.com", endpoint.RecordTypeCNAME, "", "ingress/default/my-ingress"), }, - Delete: []*endpoint.Endpoint{}, - UpdateOld: []*endpoint.Endpoint{}, - UpdateNew: []*endpoint.Endpoint{}, + Delete: []*endpoint.Endpoint{}, + Update: []*plan.Update{}, } expected := &plan.Changes{ Create: []*endpoint.Endpoint{ @@ -758,14 +761,14 @@ func testTXTRegistryApplyChangesWithTemplatedSuffix(t *testing.T) { p.OnApplyChanges = func(ctx context.Context, got *plan.Changes) { mExpected := map[string][]*endpoint.Endpoint{ "Create": expected.Create, - "UpdateNew": expected.UpdateNew, - "UpdateOld": expected.UpdateOld, + "UpdateNew": expected.UpdateNew(), + "UpdateOld": expected.UpdateOld(), "Delete": expected.Delete, } mGot := map[string][]*endpoint.Endpoint{ "Create": got.Create, - "UpdateNew": got.UpdateNew, - "UpdateOld": got.UpdateOld, + "UpdateNew": got.UpdateNew(), + "UpdateOld": got.UpdateOld(), "Delete": got.Delete, } assert.True(t, testutils.SamePlanChanges(mGot, mExpected)) @@ -818,15 +821,30 @@ func testTXTRegistryApplyChangesWithSuffix(t *testing.T) { newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), newEndpointWithOwner("multiple.test-zone.example.org", "lb1.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-1"), }, - UpdateNew: []*endpoint.Endpoint{ - newEndpointWithOwnerResource("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2"), - newEndpointWithOwnerResource("multiple.test-zone.example.org", "new.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2").WithSetIdentifier("test-set-2"), - }, - UpdateOld: []*endpoint.Endpoint{ - newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), - newEndpointWithOwner("multiple.test-zone.example.org", "lb2.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-2"), + Update: []*plan.Update{ + { + New: newEndpointWithOwnerResource("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2"), + Old: newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), + }, + { + New: newEndpointWithOwnerResource("multiple.test-zone.example.org", "new.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2").WithSetIdentifier("test-set-2"), + Old: newEndpointWithOwner("multiple.test-zone.example.org", "lb2.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-2"), + }, }, } + update, err := plan.MkUpdates( + []*endpoint.Endpoint{ + newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), + newEndpointWithOwnerAndOwnedRecord("cname-tar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "tar.test-zone.example.org"), + newEndpointWithOwner("multiple.test-zone.example.org", "lb2.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-2"), + newEndpointWithOwnerAndOwnedRecord("cname-multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").WithSetIdentifier("test-set-2"), + }, []*endpoint.Endpoint{ + newEndpointWithOwnerResource("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2"), + newEndpointWithOwnerAndOwnedRecord("cname-tar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, "", "tar.test-zone.example.org"), + newEndpointWithOwnerResource("multiple.test-zone.example.org", "new.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2").WithSetIdentifier("test-set-2"), + newEndpointWithOwnerAndOwnedRecord("cname-multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").WithSetIdentifier("test-set-2"), + }) + assert.NoError(t, err) expected := &plan.Changes{ Create: []*endpoint.Endpoint{ newEndpointWithOwnerResource("new-record-1.test-zone.example.org", "new-loadbalancer-1.lb.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress"), @@ -844,36 +862,25 @@ func testTXTRegistryApplyChangesWithSuffix(t *testing.T) { newEndpointWithOwner("multiple.test-zone.example.org", "lb1.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-1"), newEndpointWithOwnerAndOwnedRecord("cname-multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").WithSetIdentifier("test-set-1"), }, - UpdateNew: []*endpoint.Endpoint{ - newEndpointWithOwnerResource("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2"), - newEndpointWithOwnerAndOwnedRecord("cname-tar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, "", "tar.test-zone.example.org"), - newEndpointWithOwnerResource("multiple.test-zone.example.org", "new.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress-2").WithSetIdentifier("test-set-2"), - newEndpointWithOwnerAndOwnedRecord("cname-multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").WithSetIdentifier("test-set-2"), - }, - UpdateOld: []*endpoint.Endpoint{ - newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), - newEndpointWithOwnerAndOwnedRecord("cname-tar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "tar.test-zone.example.org"), - newEndpointWithOwner("multiple.test-zone.example.org", "lb2.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-2"), - newEndpointWithOwnerAndOwnedRecord("cname-multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").WithSetIdentifier("test-set-2"), - }, + Update: update, } p.OnApplyChanges = func(ctx context.Context, got *plan.Changes) { mExpected := map[string][]*endpoint.Endpoint{ "Create": expected.Create, - "UpdateNew": expected.UpdateNew, - "UpdateOld": expected.UpdateOld, + "UpdateNew": expected.UpdateNew(), + "UpdateOld": expected.UpdateOld(), "Delete": expected.Delete, } mGot := map[string][]*endpoint.Endpoint{ "Create": got.Create, - "UpdateNew": got.UpdateNew, - "UpdateOld": got.UpdateOld, + "UpdateNew": got.UpdateNew(), + "UpdateOld": got.UpdateOld(), "Delete": got.Delete, } assert.True(t, testutils.SamePlanChanges(mGot, mExpected)) assert.Nil(t, ctx.Value(provider.RecordsContextKey)) } - err := r.ApplyChanges(ctx, changes) + err = r.ApplyChanges(ctx, changes) require.NoError(t, err) } @@ -910,11 +917,11 @@ func testTXTRegistryApplyChangesNoPrefix(t *testing.T) { Delete: []*endpoint.Endpoint{ newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), }, - UpdateNew: []*endpoint.Endpoint{ - newEndpointWithOwner("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner-2"), - }, - UpdateOld: []*endpoint.Endpoint{ - newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner-2"), + Update: []*plan.Update{ + { + New: newEndpointWithOwner("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner-2"), + Old: newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner-2"), + }, }, } expected := &plan.Changes{ @@ -930,20 +937,19 @@ func testTXTRegistryApplyChangesNoPrefix(t *testing.T) { newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), newEndpointWithOwnerAndOwnedRecord("cname-foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "foobar.test-zone.example.org"), }, - UpdateNew: []*endpoint.Endpoint{}, - UpdateOld: []*endpoint.Endpoint{}, + Update: []*plan.Update{}, } p.OnApplyChanges = func(ctx context.Context, got *plan.Changes) { mExpected := map[string][]*endpoint.Endpoint{ "Create": expected.Create, - "UpdateNew": expected.UpdateNew, - "UpdateOld": expected.UpdateOld, + "UpdateNew": expected.UpdateNew(), + "UpdateOld": expected.UpdateOld(), "Delete": expected.Delete, } mGot := map[string][]*endpoint.Endpoint{ "Create": got.Create, - "UpdateNew": got.UpdateNew, - "UpdateOld": got.UpdateOld, + "UpdateNew": got.UpdateNew(), + "UpdateOld": got.UpdateOld(), "Delete": got.Delete, } assert.True(t, testutils.SamePlanChanges(mGot, mExpected)) @@ -1472,11 +1478,11 @@ func TestNewTXTScheme(t *testing.T) { Delete: []*endpoint.Endpoint{ newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), }, - UpdateNew: []*endpoint.Endpoint{ - newEndpointWithOwner("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner-2"), - }, - UpdateOld: []*endpoint.Endpoint{ - newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner-2"), + Update: []*plan.Update{ + { + New: newEndpointWithOwner("tar.test-zone.example.org", "new-tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner-2"), + Old: newEndpointWithOwner("tar.test-zone.example.org", "tar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner-2"), + }, }, } expected := &plan.Changes{ @@ -1490,20 +1496,19 @@ func TestNewTXTScheme(t *testing.T) { newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), newEndpointWithOwnerAndOwnedRecord("cname-foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "foobar.test-zone.example.org"), }, - UpdateNew: []*endpoint.Endpoint{}, - UpdateOld: []*endpoint.Endpoint{}, + Update: []*plan.Update{}, } p.OnApplyChanges = func(ctx context.Context, got *plan.Changes) { mExpected := map[string][]*endpoint.Endpoint{ "Create": expected.Create, - "UpdateNew": expected.UpdateNew, - "UpdateOld": expected.UpdateOld, + "UpdateNew": expected.UpdateNew(), + "UpdateOld": expected.UpdateOld(), "Delete": expected.Delete, } mGot := map[string][]*endpoint.Endpoint{ "Create": got.Create, - "UpdateNew": got.UpdateNew, - "UpdateOld": got.UpdateOld, + "UpdateNew": got.UpdateNew(), + "UpdateOld": got.UpdateOld(), "Delete": got.Delete, } assert.True(t, testutils.SamePlanChanges(mGot, mExpected)) @@ -1515,14 +1520,12 @@ func TestNewTXTScheme(t *testing.T) { func TestGenerateTXT(t *testing.T) { record := newEndpointWithOwner("foo.test-zone.example.org", "new-foo.loadbalancer.com", endpoint.RecordTypeCNAME, "owner") - expectedTXT := []*endpoint.Endpoint{ - { - DNSName: "cname-foo.test-zone.example.org", - Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, - RecordType: endpoint.RecordTypeTXT, - Labels: map[string]string{ - endpoint.OwnedRecordLabelKey: "foo.test-zone.example.org", - }, + expectedTXT := &endpoint.Endpoint{ + DNSName: "cname-foo.test-zone.example.org", + Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, + RecordType: endpoint.RecordTypeTXT, + Labels: map[string]string{ + endpoint.OwnedRecordLabelKey: "foo.test-zone.example.org", }, } p := inmemory.NewInMemoryProvider() @@ -1534,14 +1537,12 @@ func TestGenerateTXT(t *testing.T) { func TestGenerateTXTForAAAA(t *testing.T) { record := newEndpointWithOwner("foo.test-zone.example.org", "2001:DB8::1", endpoint.RecordTypeAAAA, "owner") - expectedTXT := []*endpoint.Endpoint{ - { - DNSName: "aaaa-foo.test-zone.example.org", - Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, - RecordType: endpoint.RecordTypeTXT, - Labels: map[string]string{ - endpoint.OwnedRecordLabelKey: "foo.test-zone.example.org", - }, + expectedTXT := &endpoint.Endpoint{ + DNSName: "aaaa-foo.test-zone.example.org", + Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, + RecordType: endpoint.RecordTypeTXT, + Labels: map[string]string{ + endpoint.OwnedRecordLabelKey: "foo.test-zone.example.org", }, } p := inmemory.NewInMemoryProvider() @@ -1560,7 +1561,7 @@ func TestFailGenerateTXT(t *testing.T) { Labels: map[string]string{}, } // A bad DNS name returns empty expected TXT - expectedTXT := []*endpoint.Endpoint{} + var expectedTXT *endpoint.Endpoint p := inmemory.NewInMemoryProvider() p.CreateZone(testZone) r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil) @@ -1652,8 +1653,8 @@ func TestMultiClusterDifferentRecordTypeOwnership(t *testing.T) { p.OnApplyChanges = func(ctx context.Context, changes *plan.Changes) { got := map[string][]*endpoint.Endpoint{ "Create": changes.Create, - "UpdateNew": changes.UpdateNew, - "UpdateOld": changes.UpdateOld, + "UpdateNew": changes.UpdateNew(), + "UpdateOld": changes.UpdateOld(), "Delete": changes.Delete, } expected := map[string][]*endpoint.Endpoint{ @@ -1713,23 +1714,14 @@ func TestGenerateTXTRecordWithNewFormatOnly(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { r, _ := NewTXTRegistry(p, "", "", "owner", time.Hour, "", []string{}, []string{}, false, nil) - records := r.generateTXTRecord(tc.endpoint) + txt := r.generateTXTRecord(tc.endpoint) - assert.Len(t, records, tc.expectedRecords, tc.description) + assert.NotNil(t, txt, tc.description) - for _, record := range records { - assert.Equal(t, endpoint.RecordTypeTXT, record.RecordType) - } + assert.Equal(t, endpoint.RecordTypeTXT, txt.RecordType) if tc.endpoint.RecordType == endpoint.RecordTypeAAAA { - hasNewFormat := false - for _, record := range records { - if strings.HasPrefix(record.DNSName, tc.expectedPrefix) { - hasNewFormat = true - break - } - } - assert.True(t, hasNewFormat, + assert.True(t, strings.HasPrefix(txt.DNSName, tc.expectedPrefix), "Should have at least one record with prefix %s when using new format", tc.expectedPrefix) } }) From 39ea4e90050bb9b881620d174140a6f399d3700e Mon Sep 17 00:00:00 2001 From: Pascal Bachor Date: Wed, 23 Jul 2025 19:36:35 +0200 Subject: [PATCH 3/8] akamai: remove redundant check --- provider/akamai/akamai.go | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/provider/akamai/akamai.go b/provider/akamai/akamai.go index 23fe53789..f2cbea275 100644 --- a/provider/akamai/akamai.go +++ b/provider/akamai/akamai.go @@ -279,26 +279,10 @@ func (p AkamaiProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) return err } // Update recordsets - updateChanges := changes.UpdateNew() - log.Debugf("Update Changes requested [%v]", updateChanges) - if err := p.updateNewRecordsets(zoneNameIDMapper, updateChanges); err != nil { + log.Debugf("Update Changes requested [%v]", changes.Update) + if err := p.updateNewRecordsets(zoneNameIDMapper, changes.UpdateNew()); err != nil { return err } - // Check that all old endpoints were accounted for - revRecs := changes.Delete - revRecs = append(revRecs, updateChanges...) - for _, rec := range changes.UpdateOld() { - found := false - for _, r := range revRecs { - if rec.DNSName == r.DNSName { - found = true - break - } - } - if !found { - log.Warnf("UpdateOld endpoint '%s' is not accounted for in UpdateNew|Delete endpoint list", rec.DNSName) - } - } return nil } From c4a3b46480a653a9b4e8058d210df2b3984b1c84 Mon Sep 17 00:00:00 2001 From: Pascal Bachor Date: Wed, 23 Jul 2025 20:28:35 +0200 Subject: [PATCH 4/8] aws: remove redundant check --- provider/aws/aws.go | 19 +++++++------------ provider/aws/aws_test.go | 9 ++++++--- 2 files changed, 13 insertions(+), 15 deletions(-) diff --git a/provider/aws/aws.go b/provider/aws/aws.go index 2277d4e0a..b918bd798 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -635,23 +635,18 @@ func (p *AWSProvider) requiresDeleteCreate(old *endpoint.Endpoint, newE *endpoin return false } -func (p *AWSProvider) createUpdateChanges(newEndpoints, oldEndpoints []*endpoint.Endpoint) Route53Changes { +func (p *AWSProvider) createUpdateChanges(changes []*plan.Update) Route53Changes { var deletes []*endpoint.Endpoint var creates []*endpoint.Endpoint var updates []*endpoint.Endpoint - for i, newE := range newEndpoints { - if i >= len(oldEndpoints) || oldEndpoints[i] == nil { - log.Debugf("skip %s as endpoint not found in current endpoints", newE.DNSName) - continue - } - oldE := oldEndpoints[i] - if p.requiresDeleteCreate(oldE, newE) { - deletes = append(deletes, oldE) - creates = append(creates, newE) + for _, update := range changes { + if p.requiresDeleteCreate(update.Old, update.New) { + deletes = append(deletes, update.Old) + creates = append(creates, update.New) } else { // Safe to perform an UPSERT. - updates = append(updates, newE) + updates = append(updates, update.New) } } @@ -684,7 +679,7 @@ func (p *AWSProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) e return provider.NewSoftErrorf("failed to list zones, not applying changes: %w", err) } - updateChanges := p.createUpdateChanges(changes.UpdateNew(), changes.UpdateOld()) + updateChanges := p.createUpdateChanges(changes.Update) combinedChanges := make(Route53Changes, 0, len(changes.Delete)+len(changes.Create)+len(updateChanges)) combinedChanges = append(combinedChanges, p.newChanges(route53types.ChangeActionCreate, changes.Create)...) diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index f7235a90f..bbca00768 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -2824,7 +2824,8 @@ func TestAWSProvider_createUpdateChanges_NewMoreThanOld(t *testing.T) { oldEndpoints := []*endpoint.Endpoint{ endpoint.NewEndpointWithTTL("record1.foo.bar.", endpoint.RecordTypeA, endpoint.TTL(300), "1.1.1.1"), - nil, + endpoint.NewEndpointWithTTL("record2.foo.bar.", endpoint.RecordTypeA, endpoint.TTL(300), "2.2.2.2"), + endpoint.NewEndpointWithTTL("record3.foo.bar.", endpoint.RecordTypeA, endpoint.TTL(300), "3.3.3.3"), } newEndpoints := []*endpoint.Endpoint{ endpoint.NewEndpointWithTTL("record1.foo.bar.", endpoint.RecordTypeA, endpoint.TTL(300), "1.1.1.1"), @@ -2832,7 +2833,9 @@ func TestAWSProvider_createUpdateChanges_NewMoreThanOld(t *testing.T) { endpoint.NewEndpointWithTTL("record3.foo.bar.", endpoint.RecordTypeA, endpoint.TTL(300), "3.3.3.3"), } - changes := provider.createUpdateChanges(newEndpoints, oldEndpoints) + update, err := plan.MkUpdates(oldEndpoints, newEndpoints) + assert.NoError(t, err) + changes := provider.createUpdateChanges(update) // record2 should be created, record1 should be upserted var creates, upserts, deletes int @@ -2848,6 +2851,6 @@ func TestAWSProvider_createUpdateChanges_NewMoreThanOld(t *testing.T) { } require.Equal(t, 0, creates, "should create the extra new endpoint") - require.Equal(t, 1, upserts, "should upsert the matching endpoint") + require.Equal(t, 3, upserts, "should upsert the matching endpoint") require.Equal(t, 0, deletes, "should not delete anything") } From fc73f0af052de92147a1af6feb9cef3c0c823878 Mon Sep 17 00:00:00 2001 From: Pascal Bachor Date: Wed, 23 Jul 2025 20:29:22 +0200 Subject: [PATCH 5/8] coredns: fix debug message --- provider/coredns/coredns.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/provider/coredns/coredns.go b/provider/coredns/coredns.go index 2aba1c878..fdc6cef4f 100644 --- a/provider/coredns/coredns.go +++ b/provider/coredns/coredns.go @@ -301,11 +301,10 @@ func (p coreDNSProvider) groupEndpoints(changes *plan.Changes) map[string][]*end for _, ep := range changes.Create { grouped[ep.DNSName] = append(grouped[ep.DNSName], ep) } - updateOld := changes.UpdateOld() - for i, ep := range changes.UpdateNew() { - ep.Labels = updateOld[i].Labels - log.Debugf("Updating labels (%s) with old labels(%s)", ep.Labels, updateOld[i].Labels) - grouped[ep.DNSName] = append(grouped[ep.DNSName], ep) + for _, change := range changes.Update { + log.Debugf("Updating labels (%s) with old labels(%s)", change.New.Labels, change.Old.Labels) + change.New.Labels = change.Old.Labels + grouped[change.New.DNSName] = append(grouped[change.New.DNSName], change.New) } return grouped } From 84ee04e5753294bedb6f2f1b2c36178aae04b2f9 Mon Sep 17 00:00:00 2001 From: Pascal Bachor Date: Wed, 23 Jul 2025 20:29:51 +0200 Subject: [PATCH 6/8] exoscale: remove redundant check --- provider/exoscale/exoscale.go | 36 ++--- provider/exoscale/exoscale_test.go | 232 +++++++++++++++-------------- 2 files changed, 128 insertions(+), 140 deletions(-) diff --git a/provider/exoscale/exoscale.go b/provider/exoscale/exoscale.go index ace900678..7ece62eed 100644 --- a/provider/exoscale/exoscale.go +++ b/provider/exoscale/exoscale.go @@ -105,7 +105,7 @@ func (ep *ExoscaleProvider) ApplyChanges(ctx context.Context, changes *plan.Chan if ep.dryRun { log.Infof("Will NOT delete these records: %+v", changes.Delete) log.Infof("Will NOT create these records: %+v", changes.Create) - log.Infof("Will NOT update these records: %+v", merge(changes.UpdateOld(), changes.UpdateNew())) + log.Infof("Will NOT update these records: %+v", merge(changes.Update)) return nil } @@ -262,11 +262,8 @@ func ExoscaleWithLogging() ExoscaleOption { for _, v := range changes.Create { log.Infof("CREATE: %v", v) } - for _, v := range changes.UpdateOld() { - log.Infof("UPDATE (old): %v", v) - } - for _, v := range changes.UpdateNew() { - log.Infof("UPDATE (new): %v", v) + for _, v := range changes.Update { + log.Infof("UPDATE (old/new): %v / %v", v.Old, v.New) } for _, v := range changes.Delete { log.Infof("DELETE: %v", v) @@ -304,35 +301,20 @@ func (f *zoneFilter) EndpointZoneID(endpoint *endpoint.Endpoint, zones map[strin return matchZoneID, name } -func merge(updateOld, updateNew []*endpoint.Endpoint) []*endpoint.Endpoint { - findMatch := func(template *endpoint.Endpoint) *endpoint.Endpoint { - for _, record := range updateNew { - if template.DNSName == record.DNSName && - template.RecordType == record.RecordType { - return record - } - } - return nil - } - +func merge(updates []*plan.Update) []*endpoint.Endpoint { var result []*endpoint.Endpoint - for _, old := range updateOld { - matchingNew := findMatch(old) - if matchingNew == nil { - // no match shouldn't happen - continue - } + for _, update := range updates { - if !matchingNew.Targets.Same(old.Targets) { + if !update.New.Targets.Same(update.Old.Targets) { // new target: always update, TTL will be overwritten too if necessary - result = append(result, matchingNew) + result = append(result, update.New) continue } - if matchingNew.RecordTTL != 0 && matchingNew.RecordTTL != old.RecordTTL { + if update.New.RecordTTL != 0 && update.New.RecordTTL != update.Old.RecordTTL { // same target, but new non-zero TTL set in k8s, must update // probably would happen only if there is a bug in the code calling the provider - result = append(result, matchingNew) + result = append(result, update.New) } } diff --git a/provider/exoscale/exoscale_test.go b/provider/exoscale/exoscale_test.go index 084f9495f..1ec4baa51 100644 --- a/provider/exoscale/exoscale_test.go +++ b/provider/exoscale/exoscale_test.go @@ -207,142 +207,148 @@ func TestExoscaleApplyChanges(t *testing.T) { } func TestExoscaleMerge_NoUpdateOnTTL0Changes(t *testing.T) { - updateOld := []*endpoint.Endpoint{ - { - DNSName: "name1", - Targets: endpoint.Targets{"target1"}, - RecordTTL: endpoint.TTL(1), - RecordType: endpoint.RecordTypeA, + assert.Empty(t, merge( + []*plan.Update{ + { + Old: &endpoint.Endpoint{ + DNSName: "name1", + Targets: endpoint.Targets{"target1"}, + RecordTTL: endpoint.TTL(1), + RecordType: endpoint.RecordTypeA, + }, + New: &endpoint.Endpoint{ + DNSName: "name1", + Targets: endpoint.Targets{"target1"}, + RecordTTL: endpoint.TTL(0), + RecordType: endpoint.RecordTypeCNAME, + }, + }, + { + Old: &endpoint.Endpoint{ + DNSName: "name2", + Targets: endpoint.Targets{"target2"}, + RecordTTL: endpoint.TTL(1), + RecordType: endpoint.RecordTypeA, + }, + New: &endpoint.Endpoint{ + DNSName: "name2", + Targets: endpoint.Targets{"target2"}, + RecordTTL: endpoint.TTL(0), + RecordType: endpoint.RecordTypeCNAME, + }, + }, }, - { - DNSName: "name2", - Targets: endpoint.Targets{"target2"}, - RecordTTL: endpoint.TTL(1), - RecordType: endpoint.RecordTypeA, - }, - } - - updateNew := []*endpoint.Endpoint{ - { - DNSName: "name1", - Targets: endpoint.Targets{"target1"}, - RecordTTL: endpoint.TTL(0), - RecordType: endpoint.RecordTypeCNAME, - }, - { - DNSName: "name2", - Targets: endpoint.Targets{"target2"}, - RecordTTL: endpoint.TTL(0), - RecordType: endpoint.RecordTypeCNAME, - }, - } - - assert.Empty(t, merge(updateOld, updateNew)) + )) } func TestExoscaleMerge_UpdateOnTTLChanges(t *testing.T) { - updateOld := []*endpoint.Endpoint{ + merged := merge([]*plan.Update{ { - DNSName: "name1", - Targets: endpoint.Targets{"target1"}, - RecordTTL: endpoint.TTL(1), - RecordType: endpoint.RecordTypeCNAME, + Old: &endpoint.Endpoint{ + DNSName: "name1", + Targets: endpoint.Targets{"target1"}, + RecordTTL: endpoint.TTL(1), + RecordType: endpoint.RecordTypeCNAME, + }, + New: &endpoint.Endpoint{ + DNSName: "name1", + Targets: endpoint.Targets{"target1"}, + RecordTTL: endpoint.TTL(77), + RecordType: endpoint.RecordTypeCNAME, + }, }, { - DNSName: "name2", - Targets: endpoint.Targets{"target2"}, - RecordTTL: endpoint.TTL(1), - RecordType: endpoint.RecordTypeCNAME, + Old: &endpoint.Endpoint{ + DNSName: "name2", + Targets: endpoint.Targets{"target2"}, + RecordTTL: endpoint.TTL(1), + RecordType: endpoint.RecordTypeCNAME, + }, + New: &endpoint.Endpoint{ + DNSName: "name2", + Targets: endpoint.Targets{"target2"}, + RecordTTL: endpoint.TTL(10), + RecordType: endpoint.RecordTypeCNAME, + }, }, - } + }) - updateNew := []*endpoint.Endpoint{ - { - DNSName: "name1", - Targets: endpoint.Targets{"target1"}, - RecordTTL: endpoint.TTL(77), - RecordType: endpoint.RecordTypeCNAME, - }, - { - DNSName: "name2", - Targets: endpoint.Targets{"target2"}, - RecordTTL: endpoint.TTL(10), - RecordType: endpoint.RecordTypeCNAME, - }, - } - - merged := merge(updateOld, updateNew) assert.Len(t, merged, 2) assert.Equal(t, "name1", merged[0].DNSName) } func TestExoscaleMerge_AlwaysUpdateTarget(t *testing.T) { - updateOld := []*endpoint.Endpoint{ - { - DNSName: "name1", - Targets: endpoint.Targets{"target1"}, - RecordTTL: endpoint.TTL(1), - RecordType: endpoint.RecordTypeCNAME, + merged := merge( + []*plan.Update{ + { + Old: &endpoint.Endpoint{ + DNSName: "name1", + Targets: endpoint.Targets{"target1"}, + RecordTTL: endpoint.TTL(1), + RecordType: endpoint.RecordTypeCNAME, + }, + New: &endpoint.Endpoint{ + DNSName: "name1", + Targets: endpoint.Targets{"target1-changed"}, + RecordTTL: endpoint.TTL(0), + RecordType: endpoint.RecordTypeCNAME, + }, + }, + { + Old: &endpoint.Endpoint{ + DNSName: "name2", + Targets: endpoint.Targets{"target2"}, + RecordTTL: endpoint.TTL(1), + RecordType: endpoint.RecordTypeCNAME, + }, + New: &endpoint.Endpoint{ + DNSName: "name2", + Targets: endpoint.Targets{"target2"}, + RecordTTL: endpoint.TTL(0), + RecordType: endpoint.RecordTypeCNAME, + }, + }, }, - { - DNSName: "name2", - Targets: endpoint.Targets{"target2"}, - RecordTTL: endpoint.TTL(1), - RecordType: endpoint.RecordTypeCNAME, - }, - } + ) - updateNew := []*endpoint.Endpoint{ - { - DNSName: "name1", - Targets: endpoint.Targets{"target1-changed"}, - RecordTTL: endpoint.TTL(0), - RecordType: endpoint.RecordTypeCNAME, - }, - { - DNSName: "name2", - Targets: endpoint.Targets{"target2"}, - RecordTTL: endpoint.TTL(0), - RecordType: endpoint.RecordTypeCNAME, - }, - } - - merged := merge(updateOld, updateNew) assert.Len(t, merged, 1) assert.Equal(t, "target1-changed", merged[0].Targets[0]) } func TestExoscaleMerge_NoUpdateIfTTLUnchanged(t *testing.T) { - updateOld := []*endpoint.Endpoint{ - { - DNSName: "name1", - Targets: endpoint.Targets{"target1"}, - RecordTTL: endpoint.TTL(55), - RecordType: endpoint.RecordTypeCNAME, - }, - { - DNSName: "name2", - Targets: endpoint.Targets{"target2"}, - RecordTTL: endpoint.TTL(55), - RecordType: endpoint.RecordTypeCNAME, - }, - } + merged := merge( + []*plan.Update{ + { + Old: &endpoint.Endpoint{ - updateNew := []*endpoint.Endpoint{ - { - DNSName: "name1", - Targets: endpoint.Targets{"target1"}, - RecordTTL: endpoint.TTL(55), - RecordType: endpoint.RecordTypeCNAME, + DNSName: "name1", + Targets: endpoint.Targets{"target1"}, + RecordTTL: endpoint.TTL(55), + RecordType: endpoint.RecordTypeCNAME, + }, + New: &endpoint.Endpoint{ + DNSName: "name1", + Targets: endpoint.Targets{"target1"}, + RecordTTL: endpoint.TTL(55), + RecordType: endpoint.RecordTypeCNAME, + }, + }, + { + Old: &endpoint.Endpoint{ + DNSName: "name2", + Targets: endpoint.Targets{"target2"}, + RecordTTL: endpoint.TTL(55), + RecordType: endpoint.RecordTypeCNAME, + }, + New: &endpoint.Endpoint{ + DNSName: "name2", + Targets: endpoint.Targets{"target2"}, + RecordTTL: endpoint.TTL(55), + RecordType: endpoint.RecordTypeCNAME, + }, + }, }, - { - DNSName: "name2", - Targets: endpoint.Targets{"target2"}, - RecordTTL: endpoint.TTL(55), - RecordType: endpoint.RecordTypeCNAME, - }, - } + ) - merged := merge(updateOld, updateNew) assert.Empty(t, merged) } From 2333e93f1e90c496fb0e5237f05fb2d3100de72f Mon Sep 17 00:00:00 2001 From: Pascal Bachor Date: Wed, 23 Jul 2025 20:31:31 +0200 Subject: [PATCH 7/8] rfc2136: avoid index calculations --- provider/rfc2136/rfc2136.go | 32 ++++++++++++-------------------- 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/provider/rfc2136/rfc2136.go b/provider/rfc2136/rfc2136.go index 39be2e6f0..604d72f2a 100644 --- a/provider/rfc2136/rfc2136.go +++ b/provider/rfc2136/rfc2136.go @@ -389,8 +389,7 @@ func (r *rfc2136Provider) ApplyChanges(ctx context.Context, changes *plan.Change } } - updateOld := changes.UpdateOld() - for c, chunk := range chunkBy(changes.UpdateNew(), r.batchChangeSize) { + for c, chunk := range chunkBy(changes.Update, r.batchChangeSize) { log.Debugf("Processing batch %d of update changes", c) m := make(map[string]*dns.Msg) @@ -400,21 +399,19 @@ func (r *rfc2136Provider) ApplyChanges(ctx context.Context, changes *plan.Change m[z] = new(dns.Msg) } - for i, ep := range chunk { - if !r.domainFilter.Match(ep.DNSName) { - log.Debugf("Skipping record %s because it was filtered out by the specified --domain-filter", ep.DNSName) + for _, update := range chunk { + if !r.domainFilter.Match(update.New.DNSName) { + log.Debugf("Skipping record %s because it was filtered out by the specified --domain-filter", update.New.DNSName) continue } - zone := findMsgZone(ep, r.zoneNames) + zone := findMsgZone(update.New, r.zoneNames) m[zone].SetUpdate(zone) - // calculate corresponding index in the unsplitted UpdateOld for current endpoint ep in chunk - j := (c * r.batchChangeSize) + i - r.UpdateRecord(m[zone], updateOld[j], ep) - if r.createPTR && (ep.RecordType == "A" || ep.RecordType == "AAAA") { - r.RemoveReverseRecord(updateOld[j].Targets[0], ep.DNSName) - r.AddReverseRecord(ep.Targets[0], ep.DNSName) + r.UpdateRecord(m[zone], update.Old, update.New) + if r.createPTR && (update.New.RecordType == "A" || update.New.RecordType == "AAAA") { + r.RemoveReverseRecord(update.Old.Targets[0], update.New.DNSName) + r.AddReverseRecord(update.New.Targets[0], update.New.DNSName) } } @@ -629,16 +626,11 @@ func (r *rfc2136Provider) SendMessage(msg *dns.Msg) error { return lastErr } -func chunkBy(slice []*endpoint.Endpoint, chunkSize int) [][]*endpoint.Endpoint { - var chunks [][]*endpoint.Endpoint +func chunkBy[T any](slice []T, chunkSize int) [][]T { + var chunks [][]T for i := 0; i < len(slice); i += chunkSize { - end := i + chunkSize - - if end > len(slice) { - end = len(slice) - } - + end := min(i+chunkSize, len(slice)) chunks = append(chunks, slice[i:end]) } From 80058873e49bb10f926e02a89a42968db8e75e57 Mon Sep 17 00:00:00 2001 From: Pascal Bachor Date: Tue, 29 Jul 2025 14:46:30 +0200 Subject: [PATCH 8/8] fix: linter --- plan/plan.go | 10 +++++----- provider/exoscale/exoscale.go | 1 - registry/txt.go | 4 ++-- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/plan/plan.go b/plan/plan.go index 43a327c53..617d2c431 100644 --- a/plan/plan.go +++ b/plan/plan.go @@ -87,13 +87,13 @@ func (changes *Changes) MarshalJSON() ([]byte, error) { }) } -func MkUpdates(old []*endpoint.Endpoint, new []*endpoint.Endpoint) ([]*Update, error) { +func MkUpdates(olds []*endpoint.Endpoint, news []*endpoint.Endpoint) ([]*Update, error) { updates := []*Update{} - if nOld, nNew := len(old), len(new); nOld != nNew { - return nil, fmt.Errorf("Number of old updates (%v) does not match number of new updates (%v)", nOld, nNew) + if nOld, nNew := len(olds), len(news); nOld != nNew { + return nil, fmt.Errorf("number of old updates (%v) does not match number of new updates (%v)", nOld, nNew) } - for i, old := range old { - updates = append(updates, &Update{Old: old, New: new[i]}) + for i, old := range olds { + updates = append(updates, &Update{Old: old, New: news[i]}) } return updates, nil } diff --git a/provider/exoscale/exoscale.go b/provider/exoscale/exoscale.go index 7ece62eed..1d9b75ee1 100644 --- a/provider/exoscale/exoscale.go +++ b/provider/exoscale/exoscale.go @@ -304,7 +304,6 @@ func (f *zoneFilter) EndpointZoneID(endpoint *endpoint.Endpoint, zones map[strin func merge(updates []*plan.Update) []*endpoint.Endpoint { var result []*endpoint.Endpoint for _, update := range updates { - if !update.New.Targets.Same(update.Old.Targets) { // new target: always update, TTL will be overwritten too if necessary result = append(result, update.New) diff --git a/registry/txt.go b/registry/txt.go index 80515e97a..4d7e091e4 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -284,8 +284,8 @@ func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) // when we update old TXT records for which value has changed (due to new label) this would still work because // !!! TXT record value is uniquely generated from the Labels of the endpoint. Hence old TXT record can be uniquely reconstructed // NOTE: Whether `generateTXTRecord` returns `nil` depends only on DNSName, which will be the same for `old` and `new` - if old, new := im.generateTXTRecord(r.Old), im.generateTXTRecord(r.New); old != nil && new != nil { - filteredChanges.Update = append(filteredChanges.Update, &plan.Update{Old: old, New: new}) + if old, new_ := im.generateTXTRecord(r.Old), im.generateTXTRecord(r.New); old != nil && new_ != nil { + filteredChanges.Update = append(filteredChanges.Update, &plan.Update{Old: old, New: new_}) } if im.cacheInterval > 0 { // remove old version of record from cache