diff --git a/endpoint/endpoint.go b/endpoint/endpoint.go index 2d292fb0d..3c0187935 100644 --- a/endpoint/endpoint.go +++ b/endpoint/endpoint.go @@ -126,6 +126,8 @@ type Endpoint struct { Targets Targets `json:"targets,omitempty"` // RecordType type of record, e.g. CNAME, A, SRV, TXT etc RecordType string `json:"recordType,omitempty"` + // Identifier to distinguish multiple records with the same name and type (e.g. Route53 records with routing policies other than 'simple') + SetIdentifier string `json:"setIdentifier,omitempty"` // TTL for the record RecordTTL TTL `json:"recordTTL,omitempty"` // Labels stores labels defined for the Endpoint @@ -157,6 +159,11 @@ func NewEndpointWithTTL(dnsName, recordType string, ttl TTL, targets ...string) } } +func (e *Endpoint) WithSetIdentifier(setIdentifier string) *Endpoint { + e.SetIdentifier = setIdentifier + return e +} + // WithProviderSpecific attaches a key/value pair to the Endpoint and returns the Endpoint. // This can be used to pass additional data through the stages of ExternalDNS's Endpoint processing. // The assumption is that most of the time this will be provider specific metadata that doesn't @@ -182,7 +189,7 @@ func (e *Endpoint) GetProviderSpecificProperty(key string) (ProviderSpecificProp } func (e *Endpoint) String() string { - return fmt.Sprintf("%s %d IN %s %s %s", e.DNSName, e.RecordTTL, e.RecordType, e.Targets, e.ProviderSpecific) + return fmt.Sprintf("%s %d IN %s %s %s %s", e.DNSName, e.RecordTTL, e.RecordType, e.SetIdentifier, e.Targets, e.ProviderSpecific) } // DNSEndpointSpec defines the desired state of DNSEndpoint diff --git a/internal/testutils/endpoint.go b/internal/testutils/endpoint.go index b4dfa4376..98df7d252 100644 --- a/internal/testutils/endpoint.go +++ b/internal/testutils/endpoint.go @@ -47,10 +47,10 @@ func (b byAllFields) Less(i, j int) bool { // SameEndpoint returns true if two endpoints are same // considers example.org. and example.org DNSName/Target as different endpoints func SameEndpoint(a, b *endpoint.Endpoint) bool { - return a.DNSName == b.DNSName && a.Targets.Same(b.Targets) && a.RecordType == b.RecordType && + 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] && - SameProverSpecific(a.ProviderSpecific, b.ProviderSpecific) + SameProviderSpecific(a.ProviderSpecific, b.ProviderSpecific) } // SameEndpoints compares two slices of endpoints regardless of order @@ -100,7 +100,7 @@ func SamePlanChanges(a, b map[string][]*endpoint.Endpoint) bool { SameEndpoints(a["UpdateOld"], b["UpdateOld"]) && SameEndpoints(a["UpdateNew"], b["UpdateNew"]) } -// SameProverSpecific verifies that two maps contain the same string/string key/value pairs -func SameProverSpecific(a, b endpoint.ProviderSpecific) bool { +// SameProviderSpecific verifies that two maps contain the same string/string key/value pairs +func SameProviderSpecific(a, b endpoint.ProviderSpecific) bool { return reflect.DeepEqual(a, b) } diff --git a/internal/testutils/endpoint_test.go b/internal/testutils/endpoint_test.go index 2f1204fd6..3f3a2bbce 100644 --- a/internal/testutils/endpoint_test.go +++ b/internal/testutils/endpoint_test.go @@ -40,9 +40,10 @@ func ExampleSameEndpoints() { RecordType: endpoint.RecordTypeTXT, }, { - DNSName: "abc.com", - Targets: endpoint.Targets{"1.2.3.4"}, - RecordType: endpoint.RecordTypeA, + DNSName: "abc.com", + Targets: endpoint.Targets{"1.2.3.4"}, + RecordType: endpoint.RecordTypeA, + SetIdentifier: "test-set-1", }, { DNSName: "bbc.com", @@ -68,11 +69,11 @@ func ExampleSameEndpoints() { fmt.Println(ep) } // Output: - // abc.com 0 IN A 1.2.3.4 [] - // abc.com 0 IN TXT something [] - // bbc.com 0 IN CNAME foo.com [] - // cbc.com 60 IN CNAME foo.com [] - // example.org 0 IN load-balancer.org [] - // example.org 0 IN load-balancer.org [{foo bar}] - // example.org 0 IN TXT load-balancer.org [] + // abc.com 0 IN A test-set-1 1.2.3.4 [] + // abc.com 0 IN TXT something [] + // bbc.com 0 IN CNAME foo.com [] + // cbc.com 60 IN CNAME foo.com [] + // example.org 0 IN load-balancer.org [] + // example.org 0 IN load-balancer.org [{foo bar}] + // example.org 0 IN TXT load-balancer.org [] } diff --git a/plan/plan.go b/plan/plan.go index efe949be6..7b805bc76 100644 --- a/plan/plan.go +++ b/plan/plan.go @@ -63,12 +63,12 @@ bar.com | | [->191.1.1.1, ->190.1.1.1] | = create (bar.com -> 1 "=", i.e. result of calculation relies on supplied ConflictResolver */ type planTable struct { - rows map[string]*planTableRow + rows map[string]map[string]*planTableRow resolver ConflictResolver } func newPlanTable() planTable { //TODO: make resolver configurable - return planTable{map[string]*planTableRow{}, PerResource{}} + return planTable{map[string]map[string]*planTableRow{}, PerResource{}} } // planTableRow @@ -86,52 +86,23 @@ func (t planTableRow) String() string { func (t planTable) addCurrent(e *endpoint.Endpoint) { dnsName := normalizeDNSName(e.DNSName) if _, ok := t.rows[dnsName]; !ok { - t.rows[dnsName] = &planTableRow{} + t.rows[dnsName] = make(map[string]*planTableRow) } - t.rows[dnsName].current = e + if _, ok := t.rows[dnsName][e.SetIdentifier]; !ok { + t.rows[dnsName][e.SetIdentifier] = &planTableRow{} + } + t.rows[dnsName][e.SetIdentifier].current = e } func (t planTable) addCandidate(e *endpoint.Endpoint) { dnsName := normalizeDNSName(e.DNSName) if _, ok := t.rows[dnsName]; !ok { - t.rows[dnsName] = &planTableRow{} + t.rows[dnsName] = make(map[string]*planTableRow) } - t.rows[dnsName].candidates = append(t.rows[dnsName].candidates, e) -} - -// TODO: allows record type change, which might not be supported by all dns providers -func (t planTable) getUpdates() (updateNew []*endpoint.Endpoint, updateOld []*endpoint.Endpoint) { - for _, row := range t.rows { - if row.current != nil && len(row.candidates) > 0 { //dns name is taken - update := t.resolver.ResolveUpdate(row.current, row.candidates) - // compare "update" to "current" to figure out if actual update is required - if shouldUpdateTTL(update, row.current) || targetChanged(update, row.current) || shouldUpdateProviderSpecific(update, row.current) { - inheritOwner(row.current, update) - updateNew = append(updateNew, update) - updateOld = append(updateOld, row.current) - } - continue - } + if _, ok := t.rows[dnsName][e.SetIdentifier]; !ok { + t.rows[dnsName][e.SetIdentifier] = &planTableRow{} } - return -} - -func (t planTable) getCreates() (createList []*endpoint.Endpoint) { - for _, row := range t.rows { - if row.current == nil { //dns name not taken - createList = append(createList, t.resolver.ResolveCreate(row.candidates)) - } - } - return -} - -func (t planTable) getDeletes() (deleteList []*endpoint.Endpoint) { - for _, row := range t.rows { - if row.current != nil && len(row.candidates) == 0 { - deleteList = append(deleteList, row.current) - } - } - return + t.rows[dnsName][e.SetIdentifier].candidates = append(t.rows[dnsName][e.SetIdentifier].candidates, e) } // Calculate computes the actions needed to move current state towards desired @@ -148,9 +119,29 @@ func (p *Plan) Calculate() *Plan { } changes := &Changes{} - changes.Create = t.getCreates() - changes.Delete = t.getDeletes() - changes.UpdateNew, changes.UpdateOld = t.getUpdates() + + for _, topRow := range t.rows { + for _, row := range topRow { + if row.current == nil { //dns name not taken + changes.Create = append(changes.Create, t.resolver.ResolveCreate(row.candidates)) + } + if row.current != nil && len(row.candidates) == 0 { + changes.Delete = append(changes.Delete, row.current) + } + + // TODO: allows record type change, which might not be supported by all dns providers + if row.current != nil && len(row.candidates) > 0 { //dns name is taken + update := t.resolver.ResolveUpdate(row.current, row.candidates) + // compare "update" to "current" to figure out if actual update is required + if shouldUpdateTTL(update, row.current) || targetChanged(update, row.current) || shouldUpdateProviderSpecific(update, row.current) { + inheritOwner(row.current, update) + changes.UpdateNew = append(changes.UpdateNew, update) + changes.UpdateOld = append(changes.UpdateOld, row.current) + } + continue + } + } + } for _, pol := range p.Policies { changes = pol.Apply(changes) } diff --git a/plan/plan_test.go b/plan/plan_test.go index eb742e11a..f217768f9 100644 --- a/plan/plan_test.go +++ b/plan/plan_test.go @@ -38,6 +38,9 @@ type PlanTestSuite struct { bar127AWithProviderSpecificTrue *endpoint.Endpoint bar127AWithProviderSpecificFalse *endpoint.Endpoint bar192A *endpoint.Endpoint + multiple1 *endpoint.Endpoint + multiple2 *endpoint.Endpoint + multiple3 *endpoint.Endpoint } func (suite *PlanTestSuite) SetupTest() { @@ -138,7 +141,24 @@ func (suite *PlanTestSuite) SetupTest() { endpoint.ResourceLabelKey: "ingress/default/bar-192", }, } - + suite.multiple1 = &endpoint.Endpoint{ + DNSName: "multiple", + Targets: endpoint.Targets{"192.168.0.1"}, + RecordType: "A", + SetIdentifier: "test-set-1", + } + suite.multiple2 = &endpoint.Endpoint{ + DNSName: "multiple", + Targets: endpoint.Targets{"192.168.0.2"}, + RecordType: "A", + SetIdentifier: "test-set-1", + } + suite.multiple3 = &endpoint.Endpoint{ + DNSName: "multiple", + Targets: endpoint.Targets{"192.168.0.2"}, + RecordType: "A", + SetIdentifier: "test-set-2", + } } func (suite *PlanTestSuite) TestSyncFirstRound() { @@ -427,6 +447,50 @@ func (suite *PlanTestSuite) TestDuplicatedEndpointsForSameResourceRetain() { validateEntries(suite.T(), changes.Delete, expectedDelete) } +func (suite *PlanTestSuite) TestMultipleRecordsSameNameDifferentSetIdentifier() { + + current := []*endpoint.Endpoint{suite.multiple1} + desired := []*endpoint.Endpoint{suite.multiple2, suite.multiple3} + expectedCreate := []*endpoint.Endpoint{suite.multiple3} + expectedUpdateOld := []*endpoint.Endpoint{suite.multiple1} + expectedUpdateNew := []*endpoint.Endpoint{suite.multiple2} + expectedDelete := []*endpoint.Endpoint{} + + p := &Plan{ + Policies: []Policy{&SyncPolicy{}}, + Current: current, + Desired: desired, + } + + changes := p.Calculate().Changes + validateEntries(suite.T(), changes.Create, expectedCreate) + validateEntries(suite.T(), changes.UpdateNew, expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld, expectedUpdateOld) + validateEntries(suite.T(), changes.Delete, expectedDelete) +} + +func (suite *PlanTestSuite) TestSetIdentifierUpdateCreatesAndDeletes() { + + current := []*endpoint.Endpoint{suite.multiple2} + desired := []*endpoint.Endpoint{suite.multiple3} + expectedCreate := []*endpoint.Endpoint{suite.multiple3} + expectedUpdateOld := []*endpoint.Endpoint{} + expectedUpdateNew := []*endpoint.Endpoint{} + expectedDelete := []*endpoint.Endpoint{suite.multiple2} + + p := &Plan{ + Policies: []Policy{&SyncPolicy{}}, + Current: current, + Desired: desired, + } + + changes := p.Calculate().Changes + validateEntries(suite.T(), changes.Create, expectedCreate) + validateEntries(suite.T(), changes.UpdateNew, expectedUpdateNew) + validateEntries(suite.T(), changes.UpdateOld, expectedUpdateOld) + validateEntries(suite.T(), changes.Delete, expectedDelete) +} + func TestPlan(t *testing.T) { suite.Run(t, new(PlanTestSuite)) } diff --git a/provider/aws.go b/provider/aws.go index 819cad636..2ca25d648 100644 --- a/provider/aws.go +++ b/provider/aws.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "sort" + "strconv" "strings" "time" @@ -37,7 +38,16 @@ const ( recordTTL = 300 // provider specific key that designates whether an AWS ALIAS record has the EvaluateTargetHealth // field set to true. - providerSpecificEvaluateTargetHealth = "aws/evaluate-target-health" + providerSpecificEvaluateTargetHealth = "aws/evaluate-target-health" + providerSpecificSetIdentifier = "aws/set-identifier" + providerSpecificWeight = "aws/weight" + providerSpecificRegion = "aws/region" + providerSpecificFailover = "aws/failover" + providerSpecificGeolocation = "aws/geolocation" + providerSpecificGeolocationContinentCode = "aws/geolocation-continent-code" + providerSpecificGeolocationCountryCode = "aws/geolocation-country-code" + providerSpecificGeolocationSubdivisionCode = "aws/geolocation-subdivision-code" + providerSpecificMultiValueAnswer = "aws/multi-value-answer" ) var ( @@ -245,6 +255,8 @@ func (p *AWSProvider) records(zones map[string]*route53.HostedZone) ([]*endpoint endpoints := make([]*endpoint.Endpoint, 0) f := func(resp *route53.ListResourceRecordSetsOutput, lastPage bool) (shouldContinue bool) { for _, r := range resp.ResourceRecordSets { + newEndpoints := make([]*endpoint.Endpoint, 0) + // TODO(linki, ownership): Remove once ownership system is in place. // See: https://github.com/kubernetes-incubator/external-dns/pull/122/files/74e2c3d3e237411e619aefc5aab694742001cdec#r109863370 @@ -263,7 +275,7 @@ func (p *AWSProvider) records(zones map[string]*route53.HostedZone) ([]*endpoint targets[idx] = aws.StringValue(rr.Value) } - endpoints = append(endpoints, endpoint.NewEndpointWithTTL(wildcardUnescape(aws.StringValue(r.Name)), aws.StringValue(r.Type), ttl, targets...)) + newEndpoints = append(newEndpoints, endpoint.NewEndpointWithTTL(wildcardUnescape(aws.StringValue(r.Name)), aws.StringValue(r.Type), ttl, targets...)) } if r.AliasTarget != nil { @@ -274,6 +286,37 @@ func (p *AWSProvider) records(zones map[string]*route53.HostedZone) ([]*endpoint ep := endpoint. NewEndpointWithTTL(wildcardUnescape(aws.StringValue(r.Name)), endpoint.RecordTypeCNAME, ttl, aws.StringValue(r.AliasTarget.DNSName)). WithProviderSpecific(providerSpecificEvaluateTargetHealth, fmt.Sprintf("%t", aws.BoolValue(r.AliasTarget.EvaluateTargetHealth))) + newEndpoints = append(newEndpoints, ep) + } + + for _, ep := range newEndpoints { + if r.SetIdentifier != nil { + ep.SetIdentifier = aws.StringValue(r.SetIdentifier) + if r.Weight != nil { + ep.WithProviderSpecific(providerSpecificWeight, fmt.Sprintf("%d", aws.Int64Value(r.Weight))) + } + if r.Region != nil { + ep.WithProviderSpecific(providerSpecificRegion, aws.StringValue(r.Region)) + } + if r.Failover != nil { + ep.WithProviderSpecific(providerSpecificFailover, aws.StringValue(r.Failover)) + } + if r.MultiValueAnswer != nil && aws.BoolValue(r.MultiValueAnswer) { + ep.WithProviderSpecific(providerSpecificMultiValueAnswer, "") + } + if r.GeoLocation != nil { + if r.GeoLocation.ContinentCode != nil { + ep.WithProviderSpecific(providerSpecificGeolocationContinentCode, aws.StringValue(r.GeoLocation.ContinentCode)) + } else { + if r.GeoLocation.CountryCode != nil { + ep.WithProviderSpecific(providerSpecificGeolocationCountryCode, aws.StringValue(r.GeoLocation.CountryCode)) + } + if r.GeoLocation.SubdivisionCode != nil { + ep.WithProviderSpecific(providerSpecificGeolocationSubdivisionCode, aws.StringValue(r.GeoLocation.SubdivisionCode)) + } + } + } + } endpoints = append(endpoints, ep) } } @@ -479,6 +522,47 @@ func (p *AWSProvider) newChange(action string, ep *endpoint.Endpoint, recordsCac } } + setIdentifier := endpoint.SetIdentifier + if setIdentifier != "" { + change.ResourceRecordSet.SetIdentifier = aws.String(setIdentifier) + if prop, ok := endpoint.GetProviderSpecificProperty(providerSpecificWeight); ok { + weight, err := strconv.ParseInt(prop.Value, 10, 64) + if err != nil { + log.Errorf("Failed parsing value of %s: %s: %v; using weight of 0", providerSpecificWeight, prop.Value, err) + weight = 0 + } + change.ResourceRecordSet.Weight = aws.Int64(weight) + } + if prop, ok := endpoint.GetProviderSpecificProperty(providerSpecificRegion); ok { + change.ResourceRecordSet.Region = aws.String(prop.Value) + } + if prop, ok := endpoint.GetProviderSpecificProperty(providerSpecificFailover); ok { + change.ResourceRecordSet.Failover = aws.String(prop.Value) + } + if _, ok := endpoint.GetProviderSpecificProperty(providerSpecificMultiValueAnswer); ok { + change.ResourceRecordSet.MultiValueAnswer = aws.Bool(true) + } + + var geolocation = &route53.GeoLocation{} + useGeolocation := false + if prop, ok := endpoint.GetProviderSpecificProperty(providerSpecificGeolocationContinentCode); ok { + geolocation.ContinentCode = aws.String(prop.Value) + useGeolocation = true + } else { + if prop, ok := endpoint.GetProviderSpecificProperty(providerSpecificGeolocationCountryCode); ok { + geolocation.CountryCode = aws.String(prop.Value) + useGeolocation = true + } + if prop, ok := endpoint.GetProviderSpecificProperty(providerSpecificGeolocationSubdivisionCode); ok { + geolocation.SubdivisionCode = aws.String(prop.Value) + useGeolocation = true + } + } + if useGeolocation { + change.ResourceRecordSet.GeoLocation = geolocation + } + } + return change, dualstack } diff --git a/provider/aws_test.go b/provider/aws_test.go index 898f0b2eb..240593402 100644 --- a/provider/aws_test.go +++ b/provider/aws_test.go @@ -184,7 +184,11 @@ func (r *Route53APIStub) ChangeResourceRecordSets(input *route53.ChangeResourceR change.ResourceRecordSet.AliasTarget.DNSName = aws.String(wildcardEscape(ensureTrailingDot(aws.StringValue(change.ResourceRecordSet.AliasTarget.DNSName)))) } - key := aws.StringValue(change.ResourceRecordSet.Name) + "::" + aws.StringValue(change.ResourceRecordSet.Type) + setId := "" + if change.ResourceRecordSet.SetIdentifier != nil { + setId = aws.StringValue(change.ResourceRecordSet.SetIdentifier) + } + key := aws.StringValue(change.ResourceRecordSet.Name) + "::" + aws.StringValue(change.ResourceRecordSet.Type) + "::" + setId switch aws.StringValue(change.Action) { case route53.ChangeActionCreate: if _, found := recordSets[key]; found { @@ -314,6 +318,13 @@ func TestAWSRecords(t *testing.T) { endpoint.NewEndpoint("list-test-alias-evaluate.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeCNAME, "foo.eu-central-1.elb.amazonaws.com").WithProviderSpecific(providerSpecificEvaluateTargetHealth, "true"), endpoint.NewEndpointWithTTL("list-test-multiple.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "8.8.8.8", "8.8.4.4"), endpoint.NewEndpointWithTTL("prefix-*.wildcard.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeTXT, endpoint.TTL(recordTTL), "random"), + endpoint.NewEndpointWithTTL("weight-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("test-set-1").WithProviderSpecific(providerSpecificWeight, "10"), + endpoint.NewEndpointWithTTL("weight-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "4.3.2.1").WithSetIdentifier("test-set-2").WithProviderSpecific(providerSpecificWeight, "20"), + endpoint.NewEndpointWithTTL("latency-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("test-set").WithProviderSpecific(providerSpecificRegion, "us-east-1"), + endpoint.NewEndpointWithTTL("failover-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("test-set").WithProviderSpecific(providerSpecificFailover, "PRIMARY"), + endpoint.NewEndpointWithTTL("multi-value-answer-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("test-set").WithProviderSpecific(providerSpecificMultiValueAnswer, ""), + endpoint.NewEndpointWithTTL("geolocation-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("test-set-1").WithProviderSpecific(providerSpecificGeolocationContinentCode, "EU"), + endpoint.NewEndpointWithTTL("geolocation-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "4.3.2.1").WithSetIdentifier("test-set-2").WithProviderSpecific(providerSpecificGeolocationCountryCode, "DE"), }) records, err := provider.Records() @@ -328,6 +339,13 @@ func TestAWSRecords(t *testing.T) { endpoint.NewEndpointWithTTL("list-test-alias-evaluate.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeCNAME, endpoint.TTL(recordTTL), "foo.eu-central-1.elb.amazonaws.com").WithProviderSpecific(providerSpecificEvaluateTargetHealth, "true"), endpoint.NewEndpointWithTTL("list-test-multiple.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "8.8.8.8", "8.8.4.4"), endpoint.NewEndpointWithTTL("prefix-*.wildcard.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeTXT, endpoint.TTL(recordTTL), "random"), + endpoint.NewEndpointWithTTL("weight-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("test-set-1").WithProviderSpecific(providerSpecificWeight, "10"), + endpoint.NewEndpointWithTTL("weight-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "4.3.2.1").WithSetIdentifier("test-set-2").WithProviderSpecific(providerSpecificWeight, "20"), + endpoint.NewEndpointWithTTL("latency-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("test-set").WithProviderSpecific(providerSpecificRegion, "us-east-1"), + endpoint.NewEndpointWithTTL("failover-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("test-set").WithProviderSpecific(providerSpecificFailover, "PRIMARY"), + endpoint.NewEndpointWithTTL("multi-value-answer-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("test-set").WithProviderSpecific(providerSpecificMultiValueAnswer, ""), + endpoint.NewEndpointWithTTL("geolocation-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "1.2.3.4").WithSetIdentifier("test-set-1").WithProviderSpecific(providerSpecificGeolocationContinentCode, "EU"), + endpoint.NewEndpointWithTTL("geolocation-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, endpoint.TTL(recordTTL), "4.3.2.1").WithSetIdentifier("test-set-2").WithProviderSpecific(providerSpecificGeolocationCountryCode, "DE"), }) } @@ -797,7 +815,7 @@ func TestAWSBatchChangeSetExceedingNameChange(t *testing.T) { } func validateEndpoints(t *testing.T, endpoints []*endpoint.Endpoint, expected []*endpoint.Endpoint) { - assert.True(t, testutils.SameEndpoints(endpoints, expected), "expected and actual endpoints don't match. %s:%s", endpoints, expected) + assert.True(t, testutils.SameEndpoints(endpoints, expected), "actual and expected endpoints don't match. %s:%s", endpoints, expected) } func validateAWSZones(t *testing.T, zones map[string]*route53.HostedZone, expected map[string]*route53.HostedZone) { diff --git a/provider/inmemory.go b/provider/inmemory.go index d480fe160..0a16d2be0 100644 --- a/provider/inmemory.go +++ b/provider/inmemory.go @@ -131,7 +131,7 @@ func (im *InMemoryProvider) Records() ([]*endpoint.Endpoint, error) { } for _, record := range records { - ep := endpoint.NewEndpoint(record.Name, record.Type, record.Target) + ep := endpoint.NewEndpoint(record.Name, record.Type, record.Target).WithSetIdentifier(record.SetIdentifier) ep.Labels = record.Labels endpoints = append(endpoints, ep) } @@ -204,10 +204,11 @@ func convertToInMemoryRecord(endpoints []*endpoint.Endpoint) []*inMemoryRecord { records := []*inMemoryRecord{} for _, ep := range endpoints { records = append(records, &inMemoryRecord{ - Type: ep.RecordType, - Name: ep.DNSName, - Target: ep.Targets[0], - Labels: ep.Labels, + Type: ep.RecordType, + Name: ep.DNSName, + Target: ep.Targets[0], + SetIdentifier: ep.SetIdentifier, + Labels: ep.Labels, }) } return records @@ -246,10 +247,11 @@ func (f *filter) EndpointZoneID(endpoint *endpoint.Endpoint, zones map[string]st // Name - DNS name assigned to the record // Target - target of the record type inMemoryRecord struct { - Type string - Name string - Target string - Labels endpoint.Labels + Type string + SetIdentifier string + Name string + Target string + Labels endpoint.Labels } type zone map[string][]*inMemoryRecord @@ -328,15 +330,19 @@ func (c *inMemoryClient) ApplyChanges(ctx context.Context, zoneID string, change return nil } -func (c *inMemoryClient) updateMesh(mesh map[string]map[string]bool, record *inMemoryRecord) error { +func (c *inMemoryClient) updateMesh(mesh map[string]map[string]map[string]bool, record *inMemoryRecord) error { if _, exists := mesh[record.Name]; exists { - if mesh[record.Name][record.Type] { - return ErrDuplicateRecordFound + if _, exists := mesh[record.Name][record.Type]; exists { + if mesh[record.Name][record.Type][record.SetIdentifier] { + return ErrDuplicateRecordFound + } + mesh[record.Name][record.Type][record.SetIdentifier] = true + return nil } - mesh[record.Name][record.Type] = true + mesh[record.Name][record.Type] = map[string]bool{record.SetIdentifier: true} return nil } - mesh[record.Name] = map[string]bool{record.Type: true} + mesh[record.Name] = map[string]map[string]bool{record.Type: {record.SetIdentifier: true}} return nil } @@ -346,9 +352,9 @@ func (c *inMemoryClient) validateChangeBatch(zone string, changes *inMemoryChang if !ok { return ErrZoneNotFound } - mesh := map[string]map[string]bool{} + mesh := map[string]map[string]map[string]bool{} for _, newEndpoint := range changes.Create { - if c.findByType(newEndpoint.Type, curZone[newEndpoint.Name]) != nil { + if c.findByTypeAndSetIdentifier(newEndpoint.Type, newEndpoint.SetIdentifier, curZone[newEndpoint.Name]) != nil { return ErrRecordAlreadyExists } if err := c.updateMesh(mesh, newEndpoint); err != nil { @@ -356,7 +362,7 @@ func (c *inMemoryClient) validateChangeBatch(zone string, changes *inMemoryChang } } for _, updateEndpoint := range changes.UpdateNew { - if c.findByType(updateEndpoint.Type, curZone[updateEndpoint.Name]) == nil { + if c.findByTypeAndSetIdentifier(updateEndpoint.Type, updateEndpoint.SetIdentifier, curZone[updateEndpoint.Name]) == nil { return ErrRecordNotFound } if err := c.updateMesh(mesh, updateEndpoint); err != nil { @@ -364,12 +370,12 @@ func (c *inMemoryClient) validateChangeBatch(zone string, changes *inMemoryChang } } for _, updateOldEndpoint := range changes.UpdateOld { - if rec := c.findByType(updateOldEndpoint.Type, curZone[updateOldEndpoint.Name]); rec == nil || rec.Target != updateOldEndpoint.Target { + if rec := c.findByTypeAndSetIdentifier(updateOldEndpoint.Type, updateOldEndpoint.SetIdentifier, curZone[updateOldEndpoint.Name]); rec == nil || rec.Target != updateOldEndpoint.Target { return ErrRecordNotFound } } for _, deleteEndpoint := range changes.Delete { - if rec := c.findByType(deleteEndpoint.Type, curZone[deleteEndpoint.Name]); rec == nil || rec.Target != deleteEndpoint.Target { + if rec := c.findByTypeAndSetIdentifier(deleteEndpoint.Type, deleteEndpoint.SetIdentifier, curZone[deleteEndpoint.Name]); rec == nil || rec.Target != deleteEndpoint.Target { return ErrRecordNotFound } if err := c.updateMesh(mesh, deleteEndpoint); err != nil { @@ -379,9 +385,9 @@ func (c *inMemoryClient) validateChangeBatch(zone string, changes *inMemoryChang return nil } -func (c *inMemoryClient) findByType(recordType string, records []*inMemoryRecord) *inMemoryRecord { +func (c *inMemoryClient) findByTypeAndSetIdentifier(recordType, setIdentifier string, records []*inMemoryRecord) *inMemoryRecord { for _, record := range records { - if record.Type == recordType { + if record.Type == recordType && record.SetIdentifier == setIdentifier { return record } } diff --git a/provider/inmemory_test.go b/provider/inmemory_test.go index 0d61a9480..b4297dd6d 100644 --- a/provider/inmemory_test.go +++ b/provider/inmemory_test.go @@ -43,11 +43,12 @@ func TestInMemoryProvider(t *testing.T) { func testInMemoryFindByType(t *testing.T) { for _, ti := range []struct { - title string - findType string - records []*inMemoryRecord - expected *inMemoryRecord - expectedEmpty bool + title string + findType string + findSetIdentifier string + records []*inMemoryRecord + expected *inMemoryRecord + expectedEmpty bool }{ { title: "no records, empty type", @@ -112,10 +113,32 @@ func testInMemoryFindByType(t *testing.T) { Type: endpoint.RecordTypeA, }, }, + { + title: "multiple records, right type and set identifier", + findType: endpoint.RecordTypeA, + findSetIdentifier: "test-set-1", + records: []*inMemoryRecord{ + { + Type: endpoint.RecordTypeA, + SetIdentifier: "test-set-1", + }, + { + Type: endpoint.RecordTypeA, + SetIdentifier: "test-set-2", + }, + { + Type: endpoint.RecordTypeTXT, + }, + }, + expected: &inMemoryRecord{ + Type: endpoint.RecordTypeA, + SetIdentifier: "test-set-1", + }, + }, } { t.Run(ti.title, func(t *testing.T) { c := newInMemoryClient() - record := c.findByType(ti.findType, ti.records) + record := c.findByTypeAndSetIdentifier(ti.findType, ti.findSetIdentifier, ti.records) if ti.expectedEmpty { assert.Nil(t, record) } else { diff --git a/registry/txt.go b/registry/txt.go index 3305568f0..84a15040a 100644 --- a/registry/txt.go +++ b/registry/txt.go @@ -21,6 +21,7 @@ import ( "errors" "time" + "fmt" "strings" "github.com/kubernetes-incubator/external-dns/endpoint" @@ -94,15 +95,16 @@ func (im *TXTRegistry) Records() ([]*endpoint.Endpoint, error) { if err != nil { return nil, err } - endpointDNSName := im.mapper.toEndpointName(record.DNSName) - labelMap[endpointDNSName] = labels + key := fmt.Sprintf("%s::%s", im.mapper.toEndpointName(record.DNSName), record.SetIdentifier) + labelMap[key] = labels } for _, ep := range endpoints { if ep.Labels == nil { ep.Labels = endpoint.NewLabels() } - if labels, ok := labelMap[ep.DNSName]; ok { + key := fmt.Sprintf("%s::%s", ep.DNSName, ep.SetIdentifier) + if labels, ok := labelMap[key]; ok { for k, v := range labels { ep.Labels[k] = v } @@ -132,7 +134,7 @@ func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) r.Labels = make(map[string]string) } r.Labels[endpoint.OwnerLabelKey] = im.ownerID - txt := endpoint.NewEndpoint(im.mapper.toTXTName(r.DNSName), endpoint.RecordTypeTXT, r.Labels.Serialize(true)) + txt := endpoint.NewEndpoint(im.mapper.toTXTName(r.DNSName), endpoint.RecordTypeTXT, r.Labels.Serialize(true)).WithSetIdentifier(r.SetIdentifier) filteredChanges.Create = append(filteredChanges.Create, txt) if im.cacheInterval > 0 { @@ -141,7 +143,7 @@ func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) } for _, r := range filteredChanges.Delete { - txt := endpoint.NewEndpoint(im.mapper.toTXTName(r.DNSName), endpoint.RecordTypeTXT, r.Labels.Serialize(true)) + txt := endpoint.NewEndpoint(im.mapper.toTXTName(r.DNSName), endpoint.RecordTypeTXT, r.Labels.Serialize(true)).WithSetIdentifier(r.SetIdentifier) // 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 @@ -154,7 +156,7 @@ func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) // make sure TXT records are consistently updated as well for _, r := range filteredChanges.UpdateOld { - txt := endpoint.NewEndpoint(im.mapper.toTXTName(r.DNSName), endpoint.RecordTypeTXT, r.Labels.Serialize(true)) + txt := endpoint.NewEndpoint(im.mapper.toTXTName(r.DNSName), endpoint.RecordTypeTXT, r.Labels.Serialize(true)).WithSetIdentifier(r.SetIdentifier) // when we updateOld 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, txt) @@ -166,7 +168,7 @@ func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) // make sure TXT records are consistently updated as well for _, r := range filteredChanges.UpdateNew { - txt := endpoint.NewEndpoint(im.mapper.toTXTName(r.DNSName), endpoint.RecordTypeTXT, r.Labels.Serialize(true)) + txt := endpoint.NewEndpoint(im.mapper.toTXTName(r.DNSName), endpoint.RecordTypeTXT, r.Labels.Serialize(true)).WithSetIdentifier(r.SetIdentifier) filteredChanges.UpdateNew = append(filteredChanges.UpdateNew, txt) // add new version of record to cache if im.cacheInterval > 0 { @@ -230,7 +232,7 @@ func (im *TXTRegistry) removeFromCache(ep *endpoint.Endpoint) { } for i, e := range im.recordsCache { - if e.DNSName == ep.DNSName && e.RecordType == ep.RecordType && e.Targets.Same(ep.Targets) { + if e.DNSName == ep.DNSName && e.RecordType == ep.RecordType && e.SetIdentifier == ep.SetIdentifier && e.Targets.Same(ep.Targets) { // We found a match delete the endpoint from the cache. im.recordsCache = append(im.recordsCache[:i], im.recordsCache[i+1:]...) return diff --git a/registry/txt_test.go b/registry/txt_test.go index 6f7cf7db7..e11378c95 100644 --- a/registry/txt_test.go +++ b/registry/txt_test.go @@ -80,6 +80,10 @@ func testTXTRegistryRecordsPrefixed(t *testing.T) { newEndpointWithOwner("TxT.tar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner-2\"", endpoint.RecordTypeTXT, ""), // case-insensitive TXT prefix newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, ""), newEndpointWithOwner("foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("multiple.test-zone.example.org", "lb1.loadbalancer.com", endpoint.RecordTypeCNAME, "").WithSetIdentifier("test-set-1"), + newEndpointWithOwner("multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-1"), + newEndpointWithOwner("multiple.test-zone.example.org", "lb2.loadbalancer.com", endpoint.RecordTypeCNAME, "").WithSetIdentifier("test-set-2"), + newEndpointWithOwner("multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), }, }) expectedRecords := []*endpoint.Endpoint{ @@ -134,6 +138,24 @@ func testTXTRegistryRecordsPrefixed(t *testing.T) { endpoint.OwnerLabelKey: "", }, }, + { + DNSName: "multiple.test-zone.example.org", + Targets: endpoint.Targets{"lb1.loadbalancer.com"}, + SetIdentifier: "test-set-1", + RecordType: endpoint.RecordTypeCNAME, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "", + }, + }, + { + DNSName: "multiple.test-zone.example.org", + Targets: endpoint.Targets{"lb2.loadbalancer.com"}, + SetIdentifier: "test-set-2", + RecordType: endpoint.RecordTypeCNAME, + Labels: map[string]string{ + endpoint.OwnerLabelKey: "", + }, + }, } r, _ := NewTXTRegistry(p, "txt.", "owner", time.Hour) @@ -246,6 +268,10 @@ func testTXTRegistryApplyChangesWithPrefix(t *testing.T) { newEndpointWithOwner("txt.tar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), newEndpointWithOwner("foobar.test-zone.example.org", "foobar.loadbalancer.com", endpoint.RecordTypeCNAME, ""), newEndpointWithOwner("txt.foobar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, ""), + newEndpointWithOwner("multiple.test-zone.example.org", "lb1.loadbalancer.com", endpoint.RecordTypeCNAME, "").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("multiple.test-zone.example.org", "lb2.loadbalancer.com", endpoint.RecordTypeCNAME, "").WithSetIdentifier("test-set-2"), + newEndpointWithOwner("txt.multiple.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=owner\"", endpoint.RecordTypeTXT, "").WithSetIdentifier("test-set-2"), }, }) r, _ := NewTXTRegistry(p, "txt.", "owner", time.Hour) @@ -253,33 +279,45 @@ func testTXTRegistryApplyChangesWithPrefix(t *testing.T) { changes := &plan.Changes{ Create: []*endpoint.Endpoint{ newEndpointWithOwnerResource("new-record-1.test-zone.example.org", "new-loadbalancer-1.lb.com", "", "", "ingress/default/my-ingress"), + newEndpointWithOwnerResource("multiple.test-zone.example.org", "lb3.loadbalancer.com", "", "", "ingress/default/my-ingress").WithSetIdentifier("test-set-3"), }, Delete: []*endpoint.Endpoint{ 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"), }, } expected := &plan.Changes{ Create: []*endpoint.Endpoint{ newEndpointWithOwnerResource("new-record-1.test-zone.example.org", "new-loadbalancer-1.lb.com", "", "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, ""), + newEndpointWithOwnerResource("multiple.test-zone.example.org", "lb3.loadbalancer.com", "", "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"), }, 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("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"), }, 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, ""), + 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"), }, 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("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"), }, } p.OnApplyChanges = func(ctx context.Context, got *plan.Changes) {