diff --git a/endpoint/labels.go b/endpoint/labels.go index 544eb3e7a5..797190db41 100644 --- a/endpoint/labels.go +++ b/endpoint/labels.go @@ -32,6 +32,8 @@ const ( OwnerLabelKey = "owner" // ResourceLabelKey is the name of the label that identifies k8s resource which wants to acquire the DNS name ResourceLabelKey = "resource" + // OwnedRecordLabelKey is the name of the label that identifies the record that is owned by the labeled TXT registry record + OwnedRecordLabelKey = "ownedRecord" // AWSSDDescriptionLabel label responsible for storing raw owner/resource combination information in the Labels // supposed to be inserted by AWS SD Provider, and parsed into OwnerLabelKey and ResourceLabelKey key by AWS SD Registry diff --git a/internal/testutils/endpoint.go b/internal/testutils/endpoint.go index 033b2e03de..f8b4657958 100644 --- a/internal/testutils/endpoint.go +++ b/internal/testutils/endpoint.go @@ -62,6 +62,7 @@ func SameEndpoint(a, b *endpoint.Endpoint) bool { return a.DNSName == b.DNSName && a.Targets.Same(b.Targets) && a.RecordType == b.RecordType && a.SetIdentifier == b.SetIdentifier && a.Labels[endpoint.OwnerLabelKey] == b.Labels[endpoint.OwnerLabelKey] && a.RecordTTL == b.RecordTTL && a.Labels[endpoint.ResourceLabelKey] == b.Labels[endpoint.ResourceLabelKey] && + a.Labels[endpoint.OwnedRecordLabelKey] == b.Labels[endpoint.OwnedRecordLabelKey] && SameProviderSpecific(a.ProviderSpecific, b.ProviderSpecific) } diff --git a/provider/aws/aws.go b/provider/aws/aws.go index b1b5e6aa59..b50930d0cb 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -136,6 +136,22 @@ type Route53API interface { ListTagsForResourceWithContext(ctx context.Context, input *route53.ListTagsForResourceInput, opts ...request.Option) (*route53.ListTagsForResourceOutput, error) } +// wrapper to handle ownership relation throughout the provider implementation +type Route53Change struct { + route53.Change + OwnedRecord string +} + +type Route53Changes []*Route53Change + +func (cs Route53Changes) Route53Changes() []*route53.Change { + ret := []*route53.Change{} + for _, c := range cs { + ret = append(ret, &c.Change) + } + return ret +} + type zonesListCache struct { age time.Time duration time.Duration @@ -160,6 +176,8 @@ type AWSProvider struct { zoneTagFilter provider.ZoneTagFilter preferCNAME bool zonesCache *zonesListCache + // queue for collecting changes to submit them in the next iteration, but after all other changes + failedChangesQueue map[string]Route53Changes } // AWSConfig contains configuration to create a new AWS provider. @@ -224,6 +242,7 @@ func NewAWSProvider(awsConfig AWSConfig) (*AWSProvider, error) { preferCNAME: awsConfig.PreferCNAME, dryRun: awsConfig.DryRun, zonesCache: &zonesListCache{duration: awsConfig.ZoneCacheDuration}, + failedChangesQueue: make(map[string]Route53Changes), } return provider, nil @@ -467,7 +486,7 @@ func (p *AWSProvider) requiresDeleteCreate(old *endpoint.Endpoint, new *endpoint return false } -func (p *AWSProvider) createUpdateChanges(newEndpoints, oldEndpoints []*endpoint.Endpoint) []*route53.Change { +func (p *AWSProvider) createUpdateChanges(newEndpoints, oldEndpoints []*endpoint.Endpoint) Route53Changes { var deletes []*endpoint.Endpoint var creates []*endpoint.Endpoint var updates []*endpoint.Endpoint @@ -483,7 +502,7 @@ func (p *AWSProvider) createUpdateChanges(newEndpoints, oldEndpoints []*endpoint } } - combined := make([]*route53.Change, 0, len(deletes)+len(creates)+len(updates)) + combined := make(Route53Changes, 0, len(deletes)+len(creates)+len(updates)) combined = append(combined, p.newChanges(route53.ChangeActionCreate, creates)...) combined = append(combined, p.newChanges(route53.ChangeActionUpsert, updates)...) combined = append(combined, p.newChanges(route53.ChangeActionDelete, deletes)...) @@ -514,7 +533,7 @@ func (p *AWSProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) e updateChanges := p.createUpdateChanges(changes.UpdateNew, changes.UpdateOld) - combinedChanges := make([]*route53.Change, 0, len(changes.Delete)+len(changes.Create)+len(updateChanges)) + combinedChanges := make(Route53Changes, 0, len(changes.Delete)+len(changes.Create)+len(updateChanges)) combinedChanges = append(combinedChanges, p.newChanges(route53.ChangeActionCreate, changes.Create)...) combinedChanges = append(combinedChanges, p.newChanges(route53.ChangeActionDelete, changes.Delete)...) combinedChanges = append(combinedChanges, updateChanges...) @@ -523,7 +542,7 @@ func (p *AWSProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) e } // submitChanges takes a zone and a collection of Changes and sends them as a single transaction. -func (p *AWSProvider) submitChanges(ctx context.Context, changes []*route53.Change, zones map[string]*route53.HostedZone) error { +func (p *AWSProvider) submitChanges(ctx context.Context, changes Route53Changes, zones map[string]*route53.HostedZone) error { // return early if there is nothing to change if len(changes) == 0 { log.Info("All records are already up to date") @@ -540,9 +559,16 @@ func (p *AWSProvider) submitChanges(ctx context.Context, changes []*route53.Chan for z, cs := range changesByZone { var failedUpdate bool - batchCs := batchChangeSet(cs, p.batchChangeSize) + // group changes into new changes and into changes that failed in a previous iteration and are retried + retriedChanges, newChanges := findChangesInQueue(cs, p.failedChangesQueue[z]) + p.failedChangesQueue[z] = nil + batchCs := append(batchChangeSet(newChanges, p.batchChangeSize), batchChangeSet(retriedChanges, p.batchChangeSize)...) for i, b := range batchCs { + if len(b) == 0 { + continue + } + for _, c := range b { log.Infof("Desired change: %s %s %s [Id: %s]", *c.Action, *c.ResourceRecordSet.Name, *c.ResourceRecordSet.Type, z) } @@ -551,17 +577,45 @@ func (p *AWSProvider) submitChanges(ctx context.Context, changes []*route53.Chan params := &route53.ChangeResourceRecordSetsInput{ HostedZoneId: aws.String(z), ChangeBatch: &route53.ChangeBatch{ - Changes: b, + Changes: b.Route53Changes(), }, } + successfulChanges := 0 + if _, err := p.client.ChangeResourceRecordSetsWithContext(ctx, params); err != nil { - log.Errorf("Failure in zone %s [Id: %s]", aws.StringValue(zones[z].Name), z) - log.Error(err) // TODO(ideahitme): consider changing the interface in cases when this error might be a concern for other components - failedUpdate = true + log.Errorf("Failure in zone %s [Id: %s] when submitting change batch: %v", aws.StringValue(zones[z].Name), z, err) + + changesByOwnership := groupChangesByNameAndOwnershipRelation(b) + + if len(changesByOwnership) > 1 { + log.Debug("Trying to submit change sets one-by-one instead") + + for _, changes := range changesByOwnership { + for _, c := range changes { + log.Debugf("Desired change: %s %s %s [Id: %s]", *c.Action, *c.ResourceRecordSet.Name, *c.ResourceRecordSet.Type, z) + } + params.ChangeBatch = &route53.ChangeBatch{ + Changes: changes.Route53Changes(), + } + if _, err := p.client.ChangeResourceRecordSetsWithContext(ctx, params); err != nil { + failedUpdate = true + log.Errorf("Failed submitting change (error: %v), it will be retried in a separate change batch in the next iteration", err) + p.failedChangesQueue[z] = append(p.failedChangesQueue[z], changes...) + } else { + successfulChanges = successfulChanges + len(changes) + } + } + } else { + failedUpdate = true + } } else { + successfulChanges = len(b) + } + + if successfulChanges > 0 { // z is the R53 Hosted Zone ID already as aws.StringValue - log.Infof("%d record(s) in zone %s [Id: %s] were successfully updated", len(b), aws.StringValue(zones[z].Name), z) + log.Infof("%d record(s) in zone %s [Id: %s] were successfully updated", successfulChanges, aws.StringValue(zones[z].Name), z) } if i != len(batchCs)-1 { @@ -583,8 +637,8 @@ func (p *AWSProvider) submitChanges(ctx context.Context, changes []*route53.Chan } // newChanges returns a collection of Changes based on the given records and action. -func (p *AWSProvider) newChanges(action string, endpoints []*endpoint.Endpoint) []*route53.Change { - changes := make([]*route53.Change, 0, len(endpoints)) +func (p *AWSProvider) newChanges(action string, endpoints []*endpoint.Endpoint) Route53Changes { + changes := make(Route53Changes, 0, len(endpoints)) for _, endpoint := range endpoints { change, dualstack := p.newChange(action, endpoint) @@ -592,7 +646,7 @@ func (p *AWSProvider) newChanges(action string, endpoints []*endpoint.Endpoint) if dualstack { // make a copy of change, modify RRS type to AAAA, then add new change rrs := *change.ResourceRecordSet - change2 := &route53.Change{Action: change.Action, ResourceRecordSet: &rrs} + change2 := &Route53Change{Change: route53.Change{Action: change.Action, ResourceRecordSet: &rrs}} change2.ResourceRecordSet.Type = aws.String(route53.RRTypeAaaa) changes = append(changes, change2) } @@ -635,11 +689,13 @@ func (p *AWSProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoin // returned Change is based on the given record by the given action, e.g. // action=ChangeActionCreate returns a change for creation of the record and // action=ChangeActionDelete returns a change for deletion of the record. -func (p *AWSProvider) newChange(action string, ep *endpoint.Endpoint) (*route53.Change, bool) { - change := &route53.Change{ - Action: aws.String(action), - ResourceRecordSet: &route53.ResourceRecordSet{ - Name: aws.String(ep.DNSName), +func (p *AWSProvider) newChange(action string, ep *endpoint.Endpoint) (*Route53Change, bool) { + change := &Route53Change{ + Change: route53.Change{ + Action: aws.String(action), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: aws.String(ep.DNSName), + }, }, } dualstack := false @@ -718,9 +774,51 @@ func (p *AWSProvider) newChange(action string, ep *endpoint.Endpoint) (*route53. change.ResourceRecordSet.HealthCheckId = aws.String(prop.Value) } + if ownedRecord, ok := ep.Labels[endpoint.OwnedRecordLabelKey]; ok { + change.OwnedRecord = ownedRecord + } + return change, dualstack } +// searches for `changes` that are contained in `queue` and returns the `changes` separated by whether they were found in the queue (`foundChanges`) or not (`notFoundChanges`) +func findChangesInQueue(changes Route53Changes, queue Route53Changes) (foundChanges, notFoundChanges Route53Changes) { + if queue == nil { + return Route53Changes{}, changes + } + + for _, c := range changes { + found := false + for _, qc := range queue { + if c == qc { + foundChanges = append(foundChanges, c) + found = true + break + } + } + if !found { + notFoundChanges = append(notFoundChanges, c) + } + } + + return +} + +// group the given changes by name and ownership relation to ensure these are always submitted in the same transaction to Route53; +// grouping by name is done to always submit changes with the same name but different set identifier together, +// grouping by ownership relation is done to always submit changes of records and e.g. their corresponding TXT registry records together +func groupChangesByNameAndOwnershipRelation(cs Route53Changes) map[string]Route53Changes { + changesByOwnership := make(map[string]Route53Changes) + for _, v := range cs { + key := v.OwnedRecord + if key == "" { + key = aws.StringValue(v.ResourceRecordSet.Name) + } + changesByOwnership[key] = append(changesByOwnership[key], v) + } + return changesByOwnership +} + func (p *AWSProvider) tagsForZone(ctx context.Context, zoneID string) (map[string]string, error) { response, err := p.client.ListTagsForResourceWithContext(ctx, &route53.ListTagsForResourceInput{ ResourceType: aws.String("hostedzone"), @@ -736,55 +834,48 @@ func (p *AWSProvider) tagsForZone(ctx context.Context, zoneID string) (map[strin return tagMap, nil } -func batchChangeSet(cs []*route53.Change, batchSize int) [][]*route53.Change { +func batchChangeSet(cs Route53Changes, batchSize int) []Route53Changes { if len(cs) <= batchSize { res := sortChangesByActionNameType(cs) - return [][]*route53.Change{res} + return []Route53Changes{res} } - batchChanges := make([][]*route53.Change, 0) + batchChanges := make([]Route53Changes, 0) - changesByName := make(map[string][]*route53.Change) - for _, v := range cs { - changesByName[*v.ResourceRecordSet.Name] = append(changesByName[*v.ResourceRecordSet.Name], v) - } + changesByOwnership := groupChangesByNameAndOwnershipRelation(cs) names := make([]string, 0) - for v := range changesByName { + for v := range changesByOwnership { names = append(names, v) } sort.Strings(names) - for _, name := range names { - totalChangesByName := len(changesByName[name]) - - if totalChangesByName > batchSize { - log.Warnf("Total changes for %s exceeds max batch size of %d, total changes: %d", name, - batchSize, totalChangesByName) + currentBatch := Route53Changes{} + for k, name := range names { + v := changesByOwnership[name] + if len(v) > batchSize { + log.Warnf("Total changes for %v exceeds max batch size of %d, total changes: %d; changes will not be performed", k, batchSize, len(v)) continue } - var existingBatch bool - for i, b := range batchChanges { - if len(b)+totalChangesByName <= batchSize { - batchChanges[i] = append(batchChanges[i], changesByName[name]...) - existingBatch = true - break - } - } - if !existingBatch { - batchChanges = append(batchChanges, changesByName[name]) + if len(currentBatch)+len(v) > batchSize { + // currentBatch would be too large if we add this changeset; + // add currentBatch to batchChanges and start a new currentBatch + batchChanges = append(batchChanges, sortChangesByActionNameType(currentBatch)) + currentBatch = append(Route53Changes{}, v...) + } else { + currentBatch = append(currentBatch, v...) } } - - for i, batch := range batchChanges { - batchChanges[i] = sortChangesByActionNameType(batch) + if len(currentBatch) > 0 { + // add final currentBatch + batchChanges = append(batchChanges, sortChangesByActionNameType(currentBatch)) } return batchChanges } -func sortChangesByActionNameType(cs []*route53.Change) []*route53.Change { +func sortChangesByActionNameType(cs Route53Changes) Route53Changes { sort.SliceStable(cs, func(i, j int) bool { if *cs[i].Action > *cs[j].Action { return true @@ -805,11 +896,11 @@ func sortChangesByActionNameType(cs []*route53.Change) []*route53.Change { } // changesByZone separates a multi-zone change into a single change per zone. -func changesByZone(zones map[string]*route53.HostedZone, changeSet []*route53.Change) map[string][]*route53.Change { - changes := make(map[string][]*route53.Change) +func changesByZone(zones map[string]*route53.HostedZone, changeSet Route53Changes) map[string]Route53Changes { + changes := make(map[string]Route53Changes) for _, z := range zones { - changes[aws.StringValue(z.Id)] = []*route53.Change{} + changes[aws.StringValue(z.Id)] = Route53Changes{} } for _, c := range changeSet { @@ -828,9 +919,11 @@ func changesByZone(zones map[string]*route53.HostedZone, changeSet []*route53.Ch aliasTarget := *rrset.AliasTarget aliasTarget.HostedZoneId = aws.String(cleanZoneID(aws.StringValue(z.Id))) rrset.AliasTarget = &aliasTarget - c = &route53.Change{ - Action: c.Action, - ResourceRecordSet: &rrset, + c = &Route53Change{ + Change: route53.Change{ + Action: c.Action, + ResourceRecordSet: &rrset, + }, } } changes[aws.StringValue(z.Id)] = append(changes[aws.StringValue(z.Id)], c) diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index 58e18f353e..280842bea7 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -688,29 +688,37 @@ func TestAWSApplyChangesDryRun(t *testing.T) { } func TestAWSChangesByZones(t *testing.T) { - changes := []*route53.Change{ + changes := Route53Changes{ { - Action: aws.String(route53.ChangeActionCreate), - ResourceRecordSet: &route53.ResourceRecordSet{ - Name: aws.String("qux.foo.example.org"), TTL: aws.Int64(1), + Change: route53.Change{ + Action: aws.String(route53.ChangeActionCreate), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: aws.String("qux.foo.example.org"), TTL: aws.Int64(1), + }, }, }, { - Action: aws.String(route53.ChangeActionCreate), - ResourceRecordSet: &route53.ResourceRecordSet{ - Name: aws.String("qux.bar.example.org"), TTL: aws.Int64(2), + Change: route53.Change{ + Action: aws.String(route53.ChangeActionCreate), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: aws.String("qux.bar.example.org"), TTL: aws.Int64(2), + }, }, }, { - Action: aws.String(route53.ChangeActionDelete), - ResourceRecordSet: &route53.ResourceRecordSet{ - Name: aws.String("wambo.foo.example.org"), TTL: aws.Int64(10), + Change: route53.Change{ + Action: aws.String(route53.ChangeActionDelete), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: aws.String("wambo.foo.example.org"), TTL: aws.Int64(10), + }, }, }, { - Action: aws.String(route53.ChangeActionDelete), - ResourceRecordSet: &route53.ResourceRecordSet{ - Name: aws.String("wambo.bar.example.org"), TTL: aws.Int64(20), + Change: route53.Change{ + Action: aws.String(route53.ChangeActionDelete), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: aws.String("wambo.bar.example.org"), TTL: aws.Int64(20), + }, }, }, } @@ -738,47 +746,59 @@ func TestAWSChangesByZones(t *testing.T) { changesByZone := changesByZone(zones, changes) require.Len(t, changesByZone, 3) - validateAWSChangeRecords(t, changesByZone["foo-example-org"], []*route53.Change{ + validateAWSChangeRecords(t, changesByZone["foo-example-org"], Route53Changes{ { - Action: aws.String(route53.ChangeActionCreate), - ResourceRecordSet: &route53.ResourceRecordSet{ - Name: aws.String("qux.foo.example.org"), TTL: aws.Int64(1), + Change: route53.Change{ + Action: aws.String(route53.ChangeActionCreate), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: aws.String("qux.foo.example.org"), TTL: aws.Int64(1), + }, }, }, { - Action: aws.String(route53.ChangeActionDelete), - ResourceRecordSet: &route53.ResourceRecordSet{ - Name: aws.String("wambo.foo.example.org"), TTL: aws.Int64(10), + Change: route53.Change{ + Action: aws.String(route53.ChangeActionDelete), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: aws.String("wambo.foo.example.org"), TTL: aws.Int64(10), + }, }, }, }) - validateAWSChangeRecords(t, changesByZone["bar-example-org"], []*route53.Change{ + validateAWSChangeRecords(t, changesByZone["bar-example-org"], Route53Changes{ { - Action: aws.String(route53.ChangeActionCreate), - ResourceRecordSet: &route53.ResourceRecordSet{ - Name: aws.String("qux.bar.example.org"), TTL: aws.Int64(2), + Change: route53.Change{ + Action: aws.String(route53.ChangeActionCreate), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: aws.String("qux.bar.example.org"), TTL: aws.Int64(2), + }, }, }, { - Action: aws.String(route53.ChangeActionDelete), - ResourceRecordSet: &route53.ResourceRecordSet{ - Name: aws.String("wambo.bar.example.org"), TTL: aws.Int64(20), + Change: route53.Change{ + Action: aws.String(route53.ChangeActionDelete), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: aws.String("wambo.bar.example.org"), TTL: aws.Int64(20), + }, }, }, }) - validateAWSChangeRecords(t, changesByZone["bar-example-org-private"], []*route53.Change{ + validateAWSChangeRecords(t, changesByZone["bar-example-org-private"], Route53Changes{ { - Action: aws.String(route53.ChangeActionCreate), - ResourceRecordSet: &route53.ResourceRecordSet{ - Name: aws.String("qux.bar.example.org"), TTL: aws.Int64(2), + Change: route53.Change{ + Action: aws.String(route53.ChangeActionCreate), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: aws.String("qux.bar.example.org"), TTL: aws.Int64(2), + }, }, }, { - Action: aws.String(route53.ChangeActionDelete), - ResourceRecordSet: &route53.ResourceRecordSet{ - Name: aws.String("wambo.bar.example.org"), TTL: aws.Int64(20), + Change: route53.Change{ + Action: aws.String(route53.ChangeActionDelete), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: aws.String("wambo.bar.example.org"), TTL: aws.Int64(20), + }, }, }, }) @@ -802,7 +822,7 @@ func TestAWSsubmitChanges(t *testing.T) { ctx := context.Background() zones, _ := provider.Zones(ctx) records, _ := provider.Records(ctx) - cs := make([]*route53.Change, 0, len(endpoints)) + cs := make(Route53Changes, 0, len(endpoints)) cs = append(cs, provider.newChanges(route53.ChangeActionCreate, endpoints)...) require.NoError(t, provider.submitChanges(ctx, cs, zones)) @@ -827,22 +847,85 @@ func TestAWSsubmitChangesError(t *testing.T) { require.Error(t, provider.submitChanges(ctx, cs, zones)) } +func TestAWSsubmitChangesRetryOnError(t *testing.T) { + provider, clientStub := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, []*endpoint.Endpoint{}) + + ctx := context.Background() + zones, err := provider.Zones(ctx) + require.NoError(t, err) + + ep1 := endpoint.NewEndpointWithTTL("success.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.0.0.1") + ep2 := endpoint.NewEndpointWithTTL("fail.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.0.0.2") + ep3 := endpoint.NewEndpointWithTTL("success2.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.0.0.3") + + ep2txt := endpoint.NewEndpointWithTTL("fail__edns_housekeeping.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeTXT, endpoint.TTL(recordTTL), "something") // "__edns_housekeeping" is the TXT suffix + ep2txt.Labels = map[string]string{ + endpoint.OwnedRecordLabelKey: "fail.zone-1.ext-dns-test-2.teapot.zalan.do", + } + + // "success" and "fail" are created in the first step, both are submitted in the same batch; this should fail + cs1 := provider.newChanges(route53.ChangeActionCreate, []*endpoint.Endpoint{ep2, ep2txt, ep1}) + input1 := &route53.ChangeResourceRecordSetsInput{ + HostedZoneId: aws.String("/hostedzone/zone-1.ext-dns-test-2.teapot.zalan.do."), + ChangeBatch: &route53.ChangeBatch{ + Changes: cs1.Route53Changes(), + }, + } + clientStub.MockMethod("ChangeResourceRecordSets", input1).Return(nil, fmt.Errorf("Mock route53 failure")) + + // because of the failure, changes will be retried one by one; make "fail" submitted in its own batch fail as well + cs2 := provider.newChanges(route53.ChangeActionCreate, []*endpoint.Endpoint{ep2, ep2txt}) + input2 := &route53.ChangeResourceRecordSetsInput{ + HostedZoneId: aws.String("/hostedzone/zone-1.ext-dns-test-2.teapot.zalan.do."), + ChangeBatch: &route53.ChangeBatch{ + Changes: cs2.Route53Changes(), + }, + } + clientStub.MockMethod("ChangeResourceRecordSets", input2).Return(nil, fmt.Errorf("Mock route53 failure")) + + // "success" should have been created, verify that we still get an error because "fail" failed + require.Error(t, provider.submitChanges(ctx, cs1, zones)) + + // assert that "success" was successfully created and "fail" and its TXT record were not + records, err := provider.Records(ctx) + require.NoError(t, err) + require.True(t, containsRecordWithDNSName(records, "success.zone-1.ext-dns-test-2.teapot.zalan.do")) + require.False(t, containsRecordWithDNSName(records, "fail.zone-1.ext-dns-test-2.teapot.zalan.do")) + require.False(t, containsRecordWithDNSName(records, "fail__edns_housekeeping.zone-1.ext-dns-test-2.teapot.zalan.do")) + + // next batch should contain "fail" and "success2", should succeed this time + cs3 := provider.newChanges(route53.ChangeActionCreate, []*endpoint.Endpoint{ep2, ep2txt, ep3}) + require.NoError(t, provider.submitChanges(ctx, cs3, zones)) + + // verify all records are there + records, err = provider.Records(ctx) + require.NoError(t, err) + require.True(t, containsRecordWithDNSName(records, "success.zone-1.ext-dns-test-2.teapot.zalan.do")) + require.True(t, containsRecordWithDNSName(records, "fail.zone-1.ext-dns-test-2.teapot.zalan.do")) + require.True(t, containsRecordWithDNSName(records, "success2.zone-1.ext-dns-test-2.teapot.zalan.do")) + require.True(t, containsRecordWithDNSName(records, "fail__edns_housekeeping.zone-1.ext-dns-test-2.teapot.zalan.do")) +} + func TestAWSBatchChangeSet(t *testing.T) { - var cs []*route53.Change + var cs Route53Changes for i := 1; i <= defaultBatchChangeSize; i += 2 { - cs = append(cs, &route53.Change{ - Action: aws.String(route53.ChangeActionCreate), - ResourceRecordSet: &route53.ResourceRecordSet{ - Name: aws.String(fmt.Sprintf("host-%d", i)), - Type: aws.String("A"), + cs = append(cs, &Route53Change{ + Change: route53.Change{ + Action: aws.String(route53.ChangeActionCreate), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: aws.String(fmt.Sprintf("host-%d", i)), + Type: aws.String("A"), + }, }, }) - cs = append(cs, &route53.Change{ - Action: aws.String(route53.ChangeActionCreate), - ResourceRecordSet: &route53.ResourceRecordSet{ - Name: aws.String(fmt.Sprintf("host-%d", i)), - Type: aws.String("TXT"), + cs = append(cs, &Route53Change{ + Change: route53.Change{ + Action: aws.String(route53.ChangeActionCreate), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: aws.String(fmt.Sprintf("host-%d", i)), + Type: aws.String("TXT"), + }, }, }) } @@ -856,27 +939,33 @@ func TestAWSBatchChangeSet(t *testing.T) { } func TestAWSBatchChangeSetExceeding(t *testing.T) { - var cs []*route53.Change + var cs Route53Changes const testCount = 50 const testLimit = 11 const expectedBatchCount = 5 const expectedChangesCount = 10 for i := 1; i <= testCount; i += 2 { - cs = append(cs, &route53.Change{ - Action: aws.String(route53.ChangeActionCreate), - ResourceRecordSet: &route53.ResourceRecordSet{ - Name: aws.String(fmt.Sprintf("host-%d", i)), - Type: aws.String("A"), + cs = append(cs, + &Route53Change{ + Change: route53.Change{ + Action: aws.String(route53.ChangeActionCreate), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: aws.String(fmt.Sprintf("host-%d", i)), + Type: aws.String("A"), + }, + }, }, - }) - cs = append(cs, &route53.Change{ - Action: aws.String(route53.ChangeActionCreate), - ResourceRecordSet: &route53.ResourceRecordSet{ - Name: aws.String(fmt.Sprintf("host-%d", i)), - Type: aws.String("TXT"), + &Route53Change{ + Change: route53.Change{ + Action: aws.String(route53.ChangeActionCreate), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: aws.String(fmt.Sprintf("host-%d", i)), + Type: aws.String("TXT"), + }, + }, }, - }) + ) } batchCs := batchChangeSet(cs, testLimit) @@ -890,25 +979,31 @@ func TestAWSBatchChangeSetExceeding(t *testing.T) { } func TestAWSBatchChangeSetExceedingNameChange(t *testing.T) { - var cs []*route53.Change + var cs Route53Changes const testCount = 10 const testLimit = 1 for i := 1; i <= testCount; i += 2 { - cs = append(cs, &route53.Change{ - Action: aws.String(route53.ChangeActionCreate), - ResourceRecordSet: &route53.ResourceRecordSet{ - Name: aws.String(fmt.Sprintf("host-%d", i)), - Type: aws.String("A"), + cs = append(cs, + &Route53Change{ + Change: route53.Change{ + Action: aws.String(route53.ChangeActionCreate), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: aws.String(fmt.Sprintf("host-%d", i)), + Type: aws.String("A"), + }, + }, }, - }) - cs = append(cs, &route53.Change{ - Action: aws.String(route53.ChangeActionCreate), - ResourceRecordSet: &route53.ResourceRecordSet{ - Name: aws.String(fmt.Sprintf("host-%d", i)), - Type: aws.String("TXT"), + &Route53Change{ + Change: route53.Change{ + Action: aws.String(route53.ChangeActionCreate), + ResourceRecordSet: &route53.ResourceRecordSet{ + Name: aws.String(fmt.Sprintf("host-%d", i)), + Type: aws.String("TXT"), + }, + }, }, - }) + ) } batchCs := batchChangeSet(cs, testLimit) @@ -933,7 +1028,7 @@ func validateAWSZone(t *testing.T, zone *route53.HostedZone, expected *route53.H assert.Equal(t, aws.StringValue(expected.Name), aws.StringValue(zone.Name)) } -func validateAWSChangeRecords(t *testing.T, records []*route53.Change, expected []*route53.Change) { +func validateAWSChangeRecords(t *testing.T, records Route53Changes, expected Route53Changes) { require.Len(t, records, len(expected)) for i := range records { @@ -941,7 +1036,7 @@ func validateAWSChangeRecords(t *testing.T, records []*route53.Change, expected } } -func validateAWSChangeRecord(t *testing.T, record *route53.Change, expected *route53.Change) { +func validateAWSChangeRecord(t *testing.T, record *Route53Change, expected *Route53Change) { assert.Equal(t, aws.StringValue(expected.Action), aws.StringValue(record.Action)) assert.Equal(t, aws.StringValue(expected.ResourceRecordSet.Name), aws.StringValue(record.ResourceRecordSet.Name)) assert.Equal(t, aws.StringValue(expected.ResourceRecordSet.Type), aws.StringValue(record.ResourceRecordSet.Type)) @@ -1339,6 +1434,7 @@ func newAWSProviderWithTagFilter(t *testing.T, domainFilter endpoint.DomainFilte zoneTagFilter: zoneTagFilter, dryRun: false, zonesCache: &zonesListCache{duration: 1 * time.Minute}, + failedChangesQueue: make(map[string]Route53Changes), } createAWSZone(t, provider, &route53.HostedZone{ @@ -1413,6 +1509,15 @@ func validateRecords(t *testing.T, records []*route53.ResourceRecordSet, expecte assert.ElementsMatch(t, expected, records) } +func containsRecordWithDNSName(records []*endpoint.Endpoint, dnsName string) bool { + for _, record := range records { + if record.DNSName == dnsName { + return true + } + } + return false +} + func TestRequiresDeleteCreate(t *testing.T) { provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"foo.bar."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, []*endpoint.Endpoint{}) diff --git a/registry/txt.go b/registry/txt.go index 5a652fec0e..faa3b7453a 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -198,6 +198,7 @@ func (im *TXTRegistry) generateTXTRecord(r *endpoint.Endpoint) []*endpoint.Endpo txt := endpoint.NewEndpoint(im.mapper.toTXTName(r.DNSName), endpoint.RecordTypeTXT, r.Labels.Serialize(true)) if txt != nil { txt.WithSetIdentifier(r.SetIdentifier) + txt.Labels[endpoint.OwnedRecordLabelKey] = r.DNSName txt.ProviderSpecific = r.ProviderSpecific endpoints = append(endpoints, txt) } @@ -206,6 +207,7 @@ func (im *TXTRegistry) generateTXTRecord(r *endpoint.Endpoint) []*endpoint.Endpo txtNew := endpoint.NewEndpoint(im.mapper.toNewTXTName(r.DNSName, r.RecordType), endpoint.RecordTypeTXT, r.Labels.Serialize(true)) if txtNew != nil { txtNew.WithSetIdentifier(r.SetIdentifier) + txtNew.Labels[endpoint.OwnedRecordLabelKey] = r.DNSName txtNew.ProviderSpecific = r.ProviderSpecific endpoints = append(endpoints, txtNew) } diff --git a/registry/txt_test.go b/registry/txt_test.go index 9d5f263b95..5e8168045e 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -436,38 +436,38 @@ func testTXTRegistryApplyChangesWithPrefix(t *testing.T) { 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"), - newEndpointWithOwner("txt.new-record-1.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("txt.cname-new-record-1.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("txt.new-record-1.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "new-record-1.test-zone.example.org"), + newEndpointWithOwnerAndOwnedRecord("txt.cname-new-record-1.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "new-record-1.test-zone.example.org"), newEndpointWithOwnerResource("multiple.test-zone.example.org", "lb3.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress").WithSetIdentifier("test-set-3"), - newEndpointWithOwner("txt.multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-3"), - newEndpointWithOwner("txt.cname-multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-3"), + newEndpointWithOwnerAndOwnedRecord("txt.multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").WithSetIdentifier("test-set-3"), + newEndpointWithOwnerAndOwnedRecord("txt.cname-multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").WithSetIdentifier("test-set-3"), newEndpointWithOwnerResource("example", "new-loadbalancer-1.lb.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress"), - newEndpointWithOwner("txt.example", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("txt.cname-example", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("txt.example", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "example"), + newEndpointWithOwnerAndOwnedRecord("txt.cname-example", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "example"), }, Delete: []*endpoint.Endpoint{ newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), - newEndpointWithOwner("txt.foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("txt.cname-foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("txt.foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "foobar.test-zone.example.org"), + newEndpointWithOwnerAndOwnedRecord("txt.cname-foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "foobar.test-zone.example.org"), newEndpointWithOwner("multiple.test-zone.example.org", "lb1.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-1"), - newEndpointWithOwner("txt.multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-1"), - newEndpointWithOwner("txt.cname-multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-1"), + newEndpointWithOwnerAndOwnedRecord("txt.multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").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"), - newEndpointWithOwner("txt.tar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("txt.cname-tar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("txt.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"), + 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"), - newEndpointWithOwner("txt.multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), - newEndpointWithOwner("txt.cname-multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), + newEndpointWithOwnerAndOwnedRecord("txt.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"), + 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"), - newEndpointWithOwner("txt.tar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("txt.cname-tar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("txt.tar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "tar.test-zone.example.org"), + 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"), - newEndpointWithOwner("txt.multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), - newEndpointWithOwner("txt.cname-multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), + newEndpointWithOwnerAndOwnedRecord("txt.multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").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"), }, } p.OnApplyChanges = func(ctx context.Context, got *plan.Changes) { @@ -513,8 +513,8 @@ func testTXTRegistryApplyChangesWithTemplatedPrefix(t *testing.T) { 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"), - newEndpointWithOwner("prefix.new-record-1.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("prefixcname.new-record-1.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("prefix.new-record-1.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "new-record-1.test-zone.example.org"), + newEndpointWithOwnerAndOwnedRecord("prefixcname.new-record-1.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "new-record-1.test-zone.example.org"), }, } p.OnApplyChanges = func(ctx context.Context, got *plan.Changes) { @@ -557,8 +557,8 @@ func testTXTRegistryApplyChangesWithTemplatedSuffix(t *testing.T) { 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"), - newEndpointWithOwner("new-record-1-cnamesuffix.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("new-record-1-suffix.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("new-record-1-cnamesuffix.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "new-record-1.test-zone.example.org"), + newEndpointWithOwnerAndOwnedRecord("new-record-1-suffix.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "new-record-1.test-zone.example.org"), }, } p.OnApplyChanges = func(ctx context.Context, got *plan.Changes) { @@ -636,41 +636,41 @@ func testTXTRegistryApplyChangesWithSuffix(t *testing.T) { 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"), - newEndpointWithOwner("new-record-1-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("cname-new-record-1-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("new-record-1-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "new-record-1.test-zone.example.org"), + newEndpointWithOwnerAndOwnedRecord("cname-new-record-1-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "new-record-1.test-zone.example.org"), newEndpointWithOwnerResource("multiple.test-zone.example.org", "lb3.loadbalancer.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress").WithSetIdentifier("test-set-3"), - newEndpointWithOwner("multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-3"), - newEndpointWithOwner("cname-multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-3"), + newEndpointWithOwnerAndOwnedRecord("multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").WithSetIdentifier("test-set-3"), + newEndpointWithOwnerAndOwnedRecord("cname-multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").WithSetIdentifier("test-set-3"), newEndpointWithOwnerResource("example", "new-loadbalancer-1.lb.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress"), - newEndpointWithOwner("example-txt", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("cname-example-txt", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("example-txt", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "example"), + newEndpointWithOwnerAndOwnedRecord("cname-example-txt", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "example"), newEndpointWithOwnerResource("*.wildcard.test-zone.example.org", "new-loadbalancer-1.lb.com", endpoint.RecordTypeCNAME, "owner", "ingress/default/my-ingress"), - newEndpointWithOwner("wildcard-txt.wildcard.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("cname-wildcard-txt.wildcard.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("wildcard-txt.wildcard.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "*.wildcard.test-zone.example.org"), + newEndpointWithOwnerAndOwnedRecord("cname-wildcard-txt.wildcard.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress\"", endpoint.RecordTypeTXT, "", "*.wildcard.test-zone.example.org"), }, Delete: []*endpoint.Endpoint{ newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), - newEndpointWithOwner("foobar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("cname-foobar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("foobar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "foobar.test-zone.example.org"), + newEndpointWithOwnerAndOwnedRecord("cname-foobar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "foobar.test-zone.example.org"), newEndpointWithOwner("multiple.test-zone.example.org", "lb1.loadbalancer.com", endpoint.RecordTypeCNAME, "owner").WithSetIdentifier("test-set-1"), - newEndpointWithOwner("multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-1"), - newEndpointWithOwner("cname-multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-1"), + newEndpointWithOwnerAndOwnedRecord("multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").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"), - newEndpointWithOwner("tar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("cname-tar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("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"), + 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"), - newEndpointWithOwner("multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), - newEndpointWithOwner("cname-multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner,external-dns/resource=ingress/default/my-ingress-2\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), + newEndpointWithOwnerAndOwnedRecord("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"), + 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"), - newEndpointWithOwner("tar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("cname-tar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("tar-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "tar.test-zone.example.org"), + 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"), - newEndpointWithOwner("multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), - newEndpointWithOwner("cname-multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), + newEndpointWithOwnerAndOwnedRecord("multiple-txt.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "multiple.test-zone.example.org").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"), }, } p.OnApplyChanges = func(ctx context.Context, got *plan.Changes) { @@ -735,16 +735,16 @@ func testTXTRegistryApplyChangesNoPrefix(t *testing.T) { expected := &plan.Changes{ Create: []*endpoint.Endpoint{ newEndpointWithOwner("new-record-1.test-zone.example.org", "new-loadbalancer-1.lb.com", endpoint.RecordTypeCNAME, "owner"), - newEndpointWithOwner("new-record-1.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("cname-new-record-1.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("new-record-1.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "new-record-1.test-zone.example.org"), + newEndpointWithOwnerAndOwnedRecord("cname-new-record-1.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "new-record-1.test-zone.example.org"), newEndpointWithOwner("example", "new-loadbalancer-1.lb.com", endpoint.RecordTypeCNAME, "owner"), - newEndpointWithOwner("example", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("cname-example", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("example", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "example"), + newEndpointWithOwnerAndOwnedRecord("cname-example", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "example"), }, Delete: []*endpoint.Endpoint{ newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), - newEndpointWithOwner("foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("cname-foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "foobar.test-zone.example.org"), + 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{}, @@ -867,11 +867,17 @@ func testTXTRegistryMissingRecordsNoPrefix(t *testing.T) { // owner is taken from the source record (A, CNAME, etc.) Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, RecordType: endpoint.RecordTypeTXT, + Labels: endpoint.Labels{ + endpoint.OwnedRecordLabelKey: "oldformat.test-zone.example.org", + }, }, { DNSName: "a-oldformat2.test-zone.example.org", Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, RecordType: endpoint.RecordTypeTXT, + Labels: endpoint.Labels{ + endpoint.OwnedRecordLabelKey: "oldformat2.test-zone.example.org", + }, }, } @@ -965,11 +971,17 @@ func testTXTRegistryMissingRecordsWithPrefix(t *testing.T) { // owner is taken from the source record (A, CNAME, etc.) Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, RecordType: endpoint.RecordTypeTXT, + Labels: endpoint.Labels{ + endpoint.OwnedRecordLabelKey: "oldformat.test-zone.example.org", + }, }, { DNSName: "txt.a-oldformat2.test-zone.example.org", Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, RecordType: endpoint.RecordTypeTXT, + Labels: endpoint.Labels{ + endpoint.OwnedRecordLabelKey: "oldformat2.test-zone.example.org", + }, }, } @@ -1112,16 +1124,16 @@ func TestNewTXTScheme(t *testing.T) { expected := &plan.Changes{ Create: []*endpoint.Endpoint{ newEndpointWithOwner("new-record-1.test-zone.example.org", "new-loadbalancer-1.lb.com", endpoint.RecordTypeCNAME, "owner"), - newEndpointWithOwner("new-record-1.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("cname-new-record-1.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("new-record-1.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "new-record-1.test-zone.example.org"), + newEndpointWithOwnerAndOwnedRecord("cname-new-record-1.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "new-record-1.test-zone.example.org"), newEndpointWithOwner("example", "new-loadbalancer-1.lb.com", endpoint.RecordTypeCNAME, "owner"), - newEndpointWithOwner("example", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("cname-example", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("example", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "example"), + newEndpointWithOwnerAndOwnedRecord("cname-example", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "example"), }, Delete: []*endpoint.Endpoint{ newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, "owner"), - newEndpointWithOwner("foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), - newEndpointWithOwner("cname-foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwnerAndOwnedRecord("foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "", "foobar.test-zone.example.org"), + 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{}, @@ -1153,13 +1165,17 @@ func TestGenerateTXT(t *testing.T) { DNSName: "foo.test-zone.example.org", Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, RecordType: endpoint.RecordTypeTXT, - Labels: map[string]string{}, + Labels: map[string]string{ + endpoint.OwnedRecordLabelKey: "foo.test-zone.example.org", + }, }, { DNSName: "cname-foo.test-zone.example.org", Targets: endpoint.Targets{"\"heritage=external-dns,external-dns/owner=owner\""}, RecordType: endpoint.RecordTypeTXT, - Labels: map[string]string{}, + Labels: map[string]string{ + endpoint.OwnedRecordLabelKey: "foo.test-zone.example.org", + }, }, } p := inmemory.NewInMemoryProvider() @@ -1196,6 +1212,10 @@ func newEndpointWithOwner(dnsName, target, recordType, ownerID string) *endpoint return newEndpointWithOwnerAndLabels(dnsName, target, recordType, ownerID, nil) } +func newEndpointWithOwnerAndOwnedRecord(dnsName, target, recordType, ownerID, ownedRecord string) *endpoint.Endpoint { + return newEndpointWithOwnerAndLabels(dnsName, target, recordType, ownerID, endpoint.Labels{endpoint.OwnedRecordLabelKey: ownedRecord}) +} + func newEndpointWithOwnerAndLabels(dnsName, target, recordType, ownerID string, labels endpoint.Labels) *endpoint.Endpoint { e := endpoint.NewEndpoint(dnsName, recordType, target) e.Labels[endpoint.OwnerLabelKey] = ownerID