Merge pull request #3747 from cronik/bugfix/dual-stack-records

Fix planning for multi-cluster dual stack record types
This commit is contained in:
Kubernetes Prow Robot 2023-09-14 22:24:12 -07:00 committed by GitHub
commit 030342f445
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
13 changed files with 782 additions and 67 deletions

View File

@ -223,6 +223,7 @@ func (c *Controller) RunOnce(ctx context.Context) error {
Desired: endpoints,
DomainFilter: endpoint.MatchAllDomainFilters{&c.DomainFilter, &registryFilter},
ManagedRecords: c.ManagedRecordTypes,
OwnerID: c.Registry.OwnerID(),
}
plan = plan.Calculate()

View File

@ -277,10 +277,31 @@ func (e *Endpoint) Key() EndpointKey {
}
}
// IsOwnedBy returns true if the endpoint owner label matches the given ownerID, false otherwise
func (e *Endpoint) IsOwnedBy(ownerID string) bool {
endpointOwner, ok := e.Labels[OwnerLabelKey]
return ok && endpointOwner == ownerID
}
func (e *Endpoint) String() string {
return fmt.Sprintf("%s %d IN %s %s %s %s", e.DNSName, e.RecordTTL, e.RecordType, e.SetIdentifier, e.Targets, e.ProviderSpecific)
}
// Apply filter to slice of endpoints and return new filtered slice that includes
// only endpoints that match.
func FilterEndpointsByOwnerID(ownerID string, eps []*Endpoint) []*Endpoint {
filtered := []*Endpoint{}
for _, ep := range eps {
if endpointOwner, ok := ep.Labels[OwnerLabelKey]; !ok || endpointOwner != ownerID {
log.Debugf(`Skipping endpoint %v because owner id does not match, found: "%s", required: "%s"`, ep, endpointOwner, ownerID)
} else {
filtered = append(filtered, ep)
}
}
return filtered
}
// DNSEndpointSpec defines the desired state of DNSEndpoint
type DNSEndpointSpec struct {
Endpoints []*Endpoint `json:"endpoints,omitempty"`

View File

@ -17,6 +17,7 @@ limitations under the License.
package endpoint
import (
"reflect"
"testing"
)
@ -115,3 +116,103 @@ func TestIsLess(t *testing.T) {
}
}
}
func TestFilterEndpointsByOwnerID(t *testing.T) {
foo1 := &Endpoint{
DNSName: "foo.com",
RecordType: RecordTypeA,
Labels: Labels{
OwnerLabelKey: "foo",
},
}
foo2 := &Endpoint{
DNSName: "foo.com",
RecordType: RecordTypeCNAME,
Labels: Labels{
OwnerLabelKey: "foo",
},
}
bar := &Endpoint{
DNSName: "foo.com",
RecordType: RecordTypeA,
Labels: Labels{
OwnerLabelKey: "bar",
},
}
type args struct {
ownerID string
eps []*Endpoint
}
tests := []struct {
name string
args args
want []*Endpoint
}{
{
name: "filter values",
args: args{
ownerID: "foo",
eps: []*Endpoint{
foo1,
foo2,
bar,
},
},
want: []*Endpoint{
foo1,
foo2,
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
if got := FilterEndpointsByOwnerID(tt.args.ownerID, tt.args.eps); !reflect.DeepEqual(got, tt.want) {
t.Errorf("ApplyEndpointFilter() = %v, want %v", got, tt.want)
}
})
}
}
func TestIsOwnedBy(t *testing.T) {
type fields struct {
Labels Labels
}
type args struct {
ownerID string
}
tests := []struct {
name string
fields fields
args args
want bool
}{
{
name: "empty labels",
fields: fields{Labels: Labels{}},
args: args{ownerID: "foo"},
want: false,
},
{
name: "owner label not match",
fields: fields{Labels: Labels{OwnerLabelKey: "bar"}},
args: args{ownerID: "foo"},
want: false,
},
{
name: "owner label match",
fields: fields{Labels: Labels{OwnerLabelKey: "foo"}},
args: args{ownerID: "foo"},
want: true,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
e := &Endpoint{
Labels: tt.fields.Labels,
}
if got := e.IsOwnedBy(tt.args.ownerID); got != tt.want {
t.Errorf("Endpoint.IsOwnedBy() = %v, want %v", got, tt.want)
}
})
}
}

View File

@ -19,6 +19,8 @@ package plan
import (
"sort"
log "github.com/sirupsen/logrus"
"sigs.k8s.io/external-dns/endpoint"
)
@ -27,6 +29,7 @@ import (
type ConflictResolver interface {
ResolveCreate(candidates []*endpoint.Endpoint) *endpoint.Endpoint
ResolveUpdate(current *endpoint.Endpoint, candidates []*endpoint.Endpoint) *endpoint.Endpoint
ResolveRecordTypes(key planKey, row *planTableRow) map[string]*domainEndpoints
}
// PerResource allows only one resource to own a given dns name
@ -62,6 +65,58 @@ func (s PerResource) ResolveUpdate(current *endpoint.Endpoint, candidates []*end
return s.ResolveCreate(candidates)
}
// ResolveRecordTypes attempts to detect and resolve record type conflicts in desired
// endpoints for a domain. For eample if the there is more than 1 candidate and at lease one
// of them is a CNAME. Per [RFC 1034 3.6.2] domains that contain a CNAME can not contain any
// other record types. The default policy will prefer A and AAAA record types when a conflict is
// detected (consistent with [endpoint.Targets.Less]).
//
// [RFC 1034 3.6.2]: https://datatracker.ietf.org/doc/html/rfc1034#autoid-15
func (s PerResource) ResolveRecordTypes(key planKey, row *planTableRow) map[string]*domainEndpoints {
// no conflicts if only a single desired record type for the domain
if len(row.candidates) <= 1 {
return row.records
}
cname := false
other := false
for _, c := range row.candidates {
if c.RecordType == endpoint.RecordTypeCNAME {
cname = true
} else {
other = true
}
if cname && other {
break
}
}
// conflict was found, remove candiates of non-preferred record types
if cname && other {
log.Infof("Domain %s contains conflicting record type candidates; discarding CNAME record", key.dnsName)
records := map[string]*domainEndpoints{}
for recordType, recs := range row.records {
// policy is to prefer the non-CNAME record types when a conflict is found
if recordType == endpoint.RecordTypeCNAME {
// discard candidates of conflicting records
// keep currect so they can be deleted
records[recordType] = &domainEndpoints{
current: recs.current,
candidates: []*endpoint.Endpoint{},
}
} else {
records[recordType] = recs
}
}
return records
}
// no conflict, return all records types
return row.records
}
// less returns true if endpoint x is less than y
func (s PerResource) less(x, y *endpoint.Endpoint) bool {
return x.Targets.IsLess(y.Targets)

View File

@ -17,10 +17,10 @@ limitations under the License.
package plan
import (
"reflect"
"testing"
"github.com/stretchr/testify/suite"
"sigs.k8s.io/external-dns/endpoint"
)
@ -34,6 +34,7 @@ type ResolverSuite struct {
fooV2Cname *endpoint.Endpoint
fooV2CnameDuplicate *endpoint.Endpoint
fooA5 *endpoint.Endpoint
fooAAAA5 *endpoint.Endpoint
bar127A *endpoint.Endpoint
bar192A *endpoint.Endpoint
bar127AAnother *endpoint.Endpoint
@ -76,6 +77,14 @@ func (suite *ResolverSuite) SetupTest() {
endpoint.ResourceLabelKey: "ingress/default/foo-5",
},
}
suite.fooAAAA5 = &endpoint.Endpoint{
DNSName: "foo",
Targets: endpoint.Targets{"2001:DB8::1"},
RecordType: "AAAA",
Labels: map[string]string{
endpoint.ResourceLabelKey: "ingress/default/foo-5",
},
}
suite.bar127A = &endpoint.Endpoint{
DNSName: "bar",
Targets: endpoint.Targets{"127.0.0.1"},
@ -133,6 +142,156 @@ func (suite *ResolverSuite) TestStrictResolver() {
suite.Equal(suite.bar127A, suite.perResource.ResolveUpdate(suite.legacyBar192A, []*endpoint.Endpoint{suite.bar127A, suite.bar192A}), " legacy record's resource value will not match, should pick minimum")
}
func (suite *ResolverSuite) TestPerResource_ResolveRecordTypes() {
type args struct {
key planKey
row *planTableRow
}
tests := []struct {
name string
args args
want map[string]*domainEndpoints
}{
{
name: "no conflict: cname record",
args: args{
key: planKey{dnsName: "foo"},
row: &planTableRow{
candidates: []*endpoint.Endpoint{suite.fooV1Cname},
records: map[string]*domainEndpoints{
endpoint.RecordTypeCNAME: {
candidates: []*endpoint.Endpoint{suite.fooV1Cname},
},
},
},
},
want: map[string]*domainEndpoints{
endpoint.RecordTypeCNAME: {
candidates: []*endpoint.Endpoint{suite.fooV1Cname},
},
},
},
{
name: "no conflict: a record",
args: args{
key: planKey{dnsName: "foo"},
row: &planTableRow{
current: []*endpoint.Endpoint{suite.fooA5},
candidates: []*endpoint.Endpoint{suite.fooA5},
records: map[string]*domainEndpoints{
endpoint.RecordTypeA: {
current: suite.fooA5,
candidates: []*endpoint.Endpoint{suite.fooA5},
},
},
},
},
want: map[string]*domainEndpoints{
endpoint.RecordTypeA: {
current: suite.fooA5,
candidates: []*endpoint.Endpoint{suite.fooA5},
},
},
},
{
name: "no conflict: a and aaaa records",
args: args{
key: planKey{dnsName: "foo"},
row: &planTableRow{
candidates: []*endpoint.Endpoint{suite.fooA5, suite.fooAAAA5},
records: map[string]*domainEndpoints{
endpoint.RecordTypeA: {
candidates: []*endpoint.Endpoint{suite.fooA5},
},
endpoint.RecordTypeAAAA: {
candidates: []*endpoint.Endpoint{suite.fooAAAA5},
},
},
},
},
want: map[string]*domainEndpoints{
endpoint.RecordTypeA: {
candidates: []*endpoint.Endpoint{suite.fooA5},
},
endpoint.RecordTypeAAAA: {
candidates: []*endpoint.Endpoint{suite.fooAAAA5},
},
},
},
{
name: "conflict: cname and a records",
args: args{
key: planKey{dnsName: "foo"},
row: &planTableRow{
current: []*endpoint.Endpoint{suite.fooV1Cname},
candidates: []*endpoint.Endpoint{suite.fooV1Cname, suite.fooA5},
records: map[string]*domainEndpoints{
endpoint.RecordTypeCNAME: {
current: suite.fooV1Cname,
candidates: []*endpoint.Endpoint{suite.fooV1Cname},
},
endpoint.RecordTypeA: {
candidates: []*endpoint.Endpoint{suite.fooA5},
},
},
},
},
want: map[string]*domainEndpoints{
endpoint.RecordTypeCNAME: {
current: suite.fooV1Cname,
candidates: []*endpoint.Endpoint{},
},
endpoint.RecordTypeA: {
candidates: []*endpoint.Endpoint{suite.fooA5},
},
},
},
{
name: "conflict: cname, a, and aaaa records",
args: args{
key: planKey{dnsName: "foo"},
row: &planTableRow{
current: []*endpoint.Endpoint{suite.fooA5, suite.fooAAAA5},
candidates: []*endpoint.Endpoint{suite.fooV1Cname, suite.fooA5, suite.fooAAAA5},
records: map[string]*domainEndpoints{
endpoint.RecordTypeCNAME: {
candidates: []*endpoint.Endpoint{suite.fooV1Cname},
},
endpoint.RecordTypeA: {
current: suite.fooA5,
candidates: []*endpoint.Endpoint{suite.fooA5},
},
endpoint.RecordTypeAAAA: {
current: suite.fooAAAA5,
candidates: []*endpoint.Endpoint{suite.fooAAAA5},
},
},
},
},
want: map[string]*domainEndpoints{
endpoint.RecordTypeCNAME: {
candidates: []*endpoint.Endpoint{},
},
endpoint.RecordTypeA: {
current: suite.fooA5,
candidates: []*endpoint.Endpoint{suite.fooA5},
},
endpoint.RecordTypeAAAA: {
current: suite.fooAAAA5,
candidates: []*endpoint.Endpoint{suite.fooAAAA5},
},
},
},
}
for _, tt := range tests {
suite.Run(tt.name, func() {
if got := suite.perResource.ResolveRecordTypes(tt.args.key, tt.args.row); !reflect.DeepEqual(got, tt.want) {
suite.T().Errorf("PerResource.ResolveRecordTypes() = %v, want %v", got, tt.want)
}
})
}
}
func TestConflictResolver(t *testing.T) {
suite.Run(t, new(ResolverSuite))
}

View File

@ -46,6 +46,8 @@ type Plan struct {
DomainFilter endpoint.MatchAllDomainFilters
// DNS record types that will be considered for management
ManagedRecords []string
// OwnerID of records to manage
OwnerID string
}
// Changes holds lists of actions to be executed by dns providers
@ -64,22 +66,26 @@ type Changes struct {
type planKey struct {
dnsName string
setIdentifier string
recordType string
}
// planTable is a supplementary struct for Plan
// each row correspond to a planKey -> (current record + all desired records)
/*
planTable: (-> = target)
--------------------------------------------------------
DNSName | Current record | Desired Records |
--------------------------------------------------------
foo.com | -> 1.1.1.1 | [->1.1.1.1, ->elb.com] | = no action
--------------------------------------------------------
bar.com | | [->191.1.1.1, ->190.1.1.1] | = create (bar.com -> 190.1.1.1)
--------------------------------------------------------
"=", i.e. result of calculation relies on supplied ConflictResolver
*/
// each row correspond to a planKey -> (current records + all desired records)
//
// planTable (-> = target)
// --------------------------------------------------------------
// DNSName | Current record | Desired Records |
// --------------------------------------------------------------
// foo.com | [->1.1.1.1 ] | [->1.1.1.1] | = no action
// --------------------------------------------------------------
// bar.com | | [->191.1.1.1, ->190.1.1.1] | = create (bar.com [-> 190.1.1.1])
// --------------------------------------------------------------
// dog.com | [->1.1.1.2] | | = delete (dog.com [-> 1.1.1.2])
// --------------------------------------------------------------
// cat.com | [->::1, ->1.1.1.3] | [->1.1.1.3] | = update old (cat.com [-> ::1, -> 1.1.1.3]) new (cat.com [-> 1.1.1.3])
// --------------------------------------------------------------
// big.com | [->1.1.1.4] | [->ing.elb.com] | = update old (big.com [-> 1.1.1.4]) new (big.com [-> ing.elb.com])
// --------------------------------------------------------------
// "=", i.e. result of calculation relies on supplied ConflictResolver
type planTable struct {
rows map[planKey]*planTableRow
resolver ConflictResolver
@ -89,11 +95,26 @@ func newPlanTable() planTable { // TODO: make resolver configurable
return planTable{map[planKey]*planTableRow{}, PerResource{}}
}
// planTableRow
// current corresponds to the record currently occupying dns name on the dns provider
// candidates corresponds to the list of records which would like to have this dnsName
// planTableRow represents a set of current and desired domain resource records.
type planTableRow struct {
current *endpoint.Endpoint
// current corresponds to the records currently occupying dns name on the dns provider. More than one record may
// be represented here: for example A and AAAA. If the current domain record is a CNAME, no other record types
// are allowed per [RFC 1034 3.6.2]
//
// [RFC 1034 3.6.2]: https://datatracker.ietf.org/doc/html/rfc1034#autoid-15
current []*endpoint.Endpoint
// candidates corresponds to the list of records which would like to have this dnsName.
candidates []*endpoint.Endpoint
// records is a grouping of current and candidates by record type, for example A, AAAA, CNAME.
records map[string]*domainEndpoints
}
// domainEndpoints is a grouping of current, which are existing records from the registry, and candidates,
// which are desired records from the source. All records in this grouping have the same record type.
type domainEndpoints struct {
// current corresponds to existing record from the registry. Maybe nil if no current record of the type exists.
current *endpoint.Endpoint
// candidates corresponds to the list of records which would like to have this dnsName.
candidates []*endpoint.Endpoint
}
@ -103,23 +124,32 @@ func (t planTableRow) String() string {
func (t planTable) addCurrent(e *endpoint.Endpoint) {
key := t.newPlanKey(e)
t.rows[key].current = e
t.rows[key].current = append(t.rows[key].current, e)
t.rows[key].records[e.RecordType].current = e
}
func (t planTable) addCandidate(e *endpoint.Endpoint) {
key := t.newPlanKey(e)
t.rows[key].candidates = append(t.rows[key].candidates, e)
t.rows[key].records[e.RecordType].candidates = append(t.rows[key].records[e.RecordType].candidates, e)
}
func (t *planTable) newPlanKey(e *endpoint.Endpoint) planKey {
key := planKey{
dnsName: normalizeDNSName(e.DNSName),
setIdentifier: e.SetIdentifier,
recordType: e.RecordType,
}
if _, ok := t.rows[key]; !ok {
t.rows[key] = &planTableRow{}
t.rows[key] = &planTableRow{
records: make(map[string]*domainEndpoints),
}
}
if _, ok := t.rows[key].records[e.RecordType]; !ok {
t.rows[key].records[e.RecordType] = &domainEndpoints{}
}
return key
}
@ -149,30 +179,82 @@ func (p *Plan) Calculate() *Plan {
changes := &Changes{}
for _, row := range t.rows {
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)
for key, row := range t.rows {
// dns name not taken
if len(row.current) == 0 {
recordsByType := t.resolver.ResolveRecordTypes(key, row)
for _, records := range recordsByType {
if len(records.candidates) > 0 {
changes.Create = append(changes.Create, t.resolver.ResolveCreate(records.candidates))
}
}
}
// 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) || p.shouldUpdateProviderSpecific(update, row.current) {
inheritOwner(row.current, update)
changes.UpdateNew = append(changes.UpdateNew, update)
changes.UpdateOld = append(changes.UpdateOld, row.current)
// dns name released or possibly owned by a different external dns
if len(row.current) > 0 && len(row.candidates) == 0 {
changes.Delete = append(changes.Delete, row.current...)
}
// dns name is taken
if len(row.current) > 0 && len(row.candidates) > 0 {
creates := []*endpoint.Endpoint{}
// apply changes for each record type
recordsByType := t.resolver.ResolveRecordTypes(key, row)
for _, records := range recordsByType {
// record type not desired
if records.current != nil && len(records.candidates) == 0 {
changes.Delete = append(changes.Delete, records.current)
}
// new record type desired
if records.current == nil && len(records.candidates) > 0 {
update := t.resolver.ResolveCreate(records.candidates)
// creates are evaluated after all domain records have been processed to
// validate that this external dns has ownership claim on the domain before
// adding the records to planned changes.
creates = append(creates, update)
}
// update existing record
if records.current != nil && len(records.candidates) > 0 {
update := t.resolver.ResolveUpdate(records.current, records.candidates)
if shouldUpdateTTL(update, records.current) || targetChanged(update, records.current) || p.shouldUpdateProviderSpecific(update, records.current) {
inheritOwner(records.current, update)
changes.UpdateNew = append(changes.UpdateNew, update)
changes.UpdateOld = append(changes.UpdateOld, records.current)
}
}
}
if len(creates) > 0 {
// only add creates if the external dns has ownership claim on the domain
ownersMatch := true
for _, current := range row.current {
if p.OwnerID != "" && !current.IsOwnedBy(p.OwnerID) {
ownersMatch = false
}
}
if ownersMatch {
changes.Create = append(changes.Create, creates...)
}
}
continue
}
}
for _, pol := range p.Policies {
changes = pol.Apply(changes)
}
// filter out updates this external dns does not have ownership claim over
if p.OwnerID != "" {
changes.Delete = endpoint.FilterEndpointsByOwnerID(p.OwnerID, changes.Delete)
changes.UpdateOld = endpoint.FilterEndpointsByOwnerID(p.OwnerID, changes.UpdateOld)
changes.UpdateNew = endpoint.FilterEndpointsByOwnerID(p.OwnerID, changes.UpdateNew)
}
plan := &Plan{
Current: p.Current,
Desired: p.Desired,

View File

@ -124,7 +124,7 @@ func (suite *PlanTestSuite) SetupTest() {
}
suite.dsAAAA = &endpoint.Endpoint{
DNSName: "ds",
Targets: endpoint.Targets{"1.1.1.1"},
Targets: endpoint.Targets{"2001:DB8::1"},
RecordType: "AAAA",
Labels: map[string]string{
endpoint.ResourceLabelKey: "ingress/default/ds-AAAAA",
@ -452,19 +452,204 @@ func (suite *PlanTestSuite) TestIdempotency() {
validateEntries(suite.T(), changes.Delete, expectedDelete)
}
func (suite *PlanTestSuite) TestDifferentTypes() {
func (suite *PlanTestSuite) TestRecordTypeChange() {
current := []*endpoint.Endpoint{suite.fooV1Cname}
desired := []*endpoint.Endpoint{suite.fooV2Cname, suite.fooA5}
desired := []*endpoint.Endpoint{suite.fooA5}
expectedCreate := []*endpoint.Endpoint{suite.fooA5}
expectedUpdateOld := []*endpoint.Endpoint{suite.fooV1Cname}
expectedUpdateNew := []*endpoint.Endpoint{suite.fooV2Cname}
expectedUpdateOld := []*endpoint.Endpoint{}
expectedUpdateNew := []*endpoint.Endpoint{}
expectedDelete := []*endpoint.Endpoint{suite.fooV1Cname}
p := &Plan{
Policies: []Policy{&SyncPolicy{}},
Current: current,
Desired: desired,
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME},
OwnerID: suite.fooV1Cname.Labels[endpoint.OwnerLabelKey],
}
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) TestExistingCNameWithDualStackDesired() {
current := []*endpoint.Endpoint{suite.fooV1Cname}
desired := []*endpoint.Endpoint{suite.fooA5, suite.fooAAAA}
expectedCreate := []*endpoint.Endpoint{suite.fooA5, suite.fooAAAA}
expectedUpdateOld := []*endpoint.Endpoint{}
expectedUpdateNew := []*endpoint.Endpoint{}
expectedDelete := []*endpoint.Endpoint{suite.fooV1Cname}
p := &Plan{
Policies: []Policy{&SyncPolicy{}},
Current: current,
Desired: desired,
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME},
OwnerID: suite.fooV1Cname.Labels[endpoint.OwnerLabelKey],
}
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) TestExistingDualStackWithCNameDesired() {
suite.fooA5.Labels[endpoint.OwnerLabelKey] = "nerf"
suite.fooAAAA.Labels[endpoint.OwnerLabelKey] = "nerf"
current := []*endpoint.Endpoint{suite.fooA5, suite.fooAAAA}
desired := []*endpoint.Endpoint{suite.fooV2Cname}
expectedCreate := []*endpoint.Endpoint{suite.fooV2Cname}
expectedUpdateOld := []*endpoint.Endpoint{}
expectedUpdateNew := []*endpoint.Endpoint{}
expectedDelete := []*endpoint.Endpoint{suite.fooA5, suite.fooAAAA}
p := &Plan{
Policies: []Policy{&SyncPolicy{}},
Current: current,
Desired: desired,
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME},
OwnerID: suite.fooA5.Labels[endpoint.OwnerLabelKey],
}
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)
}
// TestExistingOwnerNotMatchingDualStackDesired validates that if there is an existing
// record for a domain but there is no ownership claim over it and there are desired
// records no changes are planed. Only domains that have explicit ownership claims should
// be updated.
func (suite *PlanTestSuite) TestExistingOwnerNotMatchingDualStackDesired() {
suite.fooA5.Labels = nil
current := []*endpoint.Endpoint{suite.fooA5}
desired := []*endpoint.Endpoint{suite.fooV2Cname}
expectedCreate := []*endpoint.Endpoint{}
expectedUpdateOld := []*endpoint.Endpoint{}
expectedUpdateNew := []*endpoint.Endpoint{}
expectedDelete := []*endpoint.Endpoint{}
p := &Plan{
Policies: []Policy{&SyncPolicy{}},
Current: current,
Desired: desired,
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME},
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME},
OwnerID: "pwner",
}
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)
}
// TestConflictingCurrentNonConflictingDesired is a bit of a corner case as it would indicate
// that the provider is not following valid DNS rules or there may be some
// caching issues. In this case since the desired records are not conflicting
// the updates will end up with the conflict resolved.
func (suite *PlanTestSuite) TestConflictingCurrentNonConflictingDesired() {
suite.fooA5.Labels[endpoint.OwnerLabelKey] = suite.fooV1Cname.Labels[endpoint.OwnerLabelKey]
current := []*endpoint.Endpoint{suite.fooV1Cname, suite.fooA5}
desired := []*endpoint.Endpoint{suite.fooA5}
expectedCreate := []*endpoint.Endpoint{}
expectedUpdateOld := []*endpoint.Endpoint{}
expectedUpdateNew := []*endpoint.Endpoint{}
expectedDelete := []*endpoint.Endpoint{suite.fooV1Cname}
p := &Plan{
Policies: []Policy{&SyncPolicy{}},
Current: current,
Desired: desired,
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME},
OwnerID: suite.fooV1Cname.Labels[endpoint.OwnerLabelKey],
}
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)
}
// TestConflictingCurrentNoDesired is a bit of a corner case as it would indicate
// that the provider is not following valid DNS rules or there may be some
// caching issues. In this case there are no desired enpoint candidates so plan
// on deleting the records.
func (suite *PlanTestSuite) TestConflictingCurrentNoDesired() {
suite.fooA5.Labels[endpoint.OwnerLabelKey] = suite.fooV1Cname.Labels[endpoint.OwnerLabelKey]
current := []*endpoint.Endpoint{suite.fooV1Cname, suite.fooA5}
desired := []*endpoint.Endpoint{}
expectedCreate := []*endpoint.Endpoint{}
expectedUpdateOld := []*endpoint.Endpoint{}
expectedUpdateNew := []*endpoint.Endpoint{}
expectedDelete := []*endpoint.Endpoint{suite.fooV1Cname, suite.fooA5}
p := &Plan{
Policies: []Policy{&SyncPolicy{}},
Current: current,
Desired: desired,
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME},
OwnerID: suite.fooV1Cname.Labels[endpoint.OwnerLabelKey],
}
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)
}
// TestCurrentWithConflictingDesired simulates where the desired records result in conflicting records types.
// This could be the result of multiple sources generating conflicting records types. In this case the conflict
// resolver should prefer the A and AAAA record candidate and delete the other records.
func (suite *PlanTestSuite) TestCurrentWithConflictingDesired() {
suite.fooV1Cname.Labels[endpoint.OwnerLabelKey] = "nerf"
current := []*endpoint.Endpoint{suite.fooV1Cname}
desired := []*endpoint.Endpoint{suite.fooV1Cname, suite.fooA5, suite.fooAAAA}
expectedCreate := []*endpoint.Endpoint{suite.fooA5, suite.fooAAAA}
expectedUpdateOld := []*endpoint.Endpoint{}
expectedUpdateNew := []*endpoint.Endpoint{}
expectedDelete := []*endpoint.Endpoint{suite.fooV1Cname}
p := &Plan{
Policies: []Policy{&SyncPolicy{}},
Current: current,
Desired: desired,
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME},
OwnerID: suite.fooV1Cname.Labels[endpoint.OwnerLabelKey],
}
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)
}
// TestNoCurrentWithConflictingDesired simulates where the desired records result in conflicting records types.
// This could be the result of multiple sources generating conflicting records types. In this case there the
// conflict resolver should prefer the A and AAAA record and drop the other candidate record types.
func (suite *PlanTestSuite) TestNoCurrentWithConflictingDesired() {
current := []*endpoint.Endpoint{}
desired := []*endpoint.Endpoint{suite.fooV1Cname, suite.fooA5, suite.fooAAAA}
expectedCreate := []*endpoint.Endpoint{suite.fooA5, suite.fooAAAA}
expectedUpdateOld := []*endpoint.Endpoint{}
expectedUpdateNew := []*endpoint.Endpoint{}
expectedDelete := []*endpoint.Endpoint{}
p := &Plan{
Policies: []Policy{&SyncPolicy{}},
Current: current,
Desired: desired,
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME},
}
changes := p.Calculate().Changes
@ -654,10 +839,10 @@ func (suite *PlanTestSuite) TestDomainFiltersUpdate() {
}
func (suite *PlanTestSuite) TestAAAARecords() {
current := []*endpoint.Endpoint{}
desired := []*endpoint.Endpoint{suite.fooAAAA}
expectedCreate := []*endpoint.Endpoint{suite.fooAAAA}
expectNoChanges := []*endpoint.Endpoint{}
p := &Plan{
Policies: []Policy{&SyncPolicy{}},
@ -668,12 +853,16 @@ func (suite *PlanTestSuite) TestAAAARecords() {
changes := p.Calculate().Changes
validateEntries(suite.T(), changes.Create, expectedCreate)
validateEntries(suite.T(), changes.Delete, expectNoChanges)
validateEntries(suite.T(), changes.UpdateOld, expectNoChanges)
validateEntries(suite.T(), changes.UpdateNew, expectNoChanges)
}
func (suite *PlanTestSuite) TestDualStackRecords() {
current := []*endpoint.Endpoint{}
desired := []*endpoint.Endpoint{suite.dsA, suite.dsAAAA}
expectedCreate := []*endpoint.Endpoint{suite.dsA, suite.dsAAAA}
expectNoChanges := []*endpoint.Endpoint{}
p := &Plan{
Policies: []Policy{&SyncPolicy{}},
@ -684,6 +873,49 @@ func (suite *PlanTestSuite) TestDualStackRecords() {
changes := p.Calculate().Changes
validateEntries(suite.T(), changes.Create, expectedCreate)
validateEntries(suite.T(), changes.Delete, expectNoChanges)
validateEntries(suite.T(), changes.UpdateOld, expectNoChanges)
validateEntries(suite.T(), changes.UpdateNew, expectNoChanges)
}
func (suite *PlanTestSuite) TestDualStackRecordsDelete() {
current := []*endpoint.Endpoint{suite.dsA, suite.dsAAAA}
desired := []*endpoint.Endpoint{}
expectedDelete := []*endpoint.Endpoint{suite.dsA, suite.dsAAAA}
expectNoChanges := []*endpoint.Endpoint{}
p := &Plan{
Policies: []Policy{&SyncPolicy{}},
Current: current,
Desired: desired,
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME},
}
changes := p.Calculate().Changes
validateEntries(suite.T(), changes.Delete, expectedDelete)
validateEntries(suite.T(), changes.Create, expectNoChanges)
validateEntries(suite.T(), changes.UpdateOld, expectNoChanges)
validateEntries(suite.T(), changes.UpdateNew, expectNoChanges)
}
func (suite *PlanTestSuite) TestDualStackToSingleStack() {
current := []*endpoint.Endpoint{suite.dsA, suite.dsAAAA}
desired := []*endpoint.Endpoint{suite.dsA}
expectedDelete := []*endpoint.Endpoint{suite.dsAAAA}
expectNoChanges := []*endpoint.Endpoint{}
p := &Plan{
Policies: []Policy{&SyncPolicy{}},
Current: current,
Desired: desired,
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME},
}
changes := p.Calculate().Changes
validateEntries(suite.T(), changes.Delete, expectedDelete)
validateEntries(suite.T(), changes.Create, expectNoChanges)
validateEntries(suite.T(), changes.UpdateOld, expectNoChanges)
validateEntries(suite.T(), changes.UpdateNew, expectNoChanges)
}
func TestPlan(t *testing.T) {

View File

@ -46,6 +46,10 @@ func (sdr *AWSSDRegistry) GetDomainFilter() endpoint.DomainFilter {
return sdr.provider.GetDomainFilter()
}
func (im *AWSSDRegistry) OwnerID() string {
return im.ownerID
}
// Records calls AWS SD API and expects AWS SD provider to provider Owner/Resource information as a serialized
// value in the AWSSDDescriptionLabel value in the Labels map
func (sdr *AWSSDRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, error) {
@ -72,9 +76,9 @@ func (sdr *AWSSDRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, er
func (sdr *AWSSDRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) error {
filteredChanges := &plan.Changes{
Create: changes.Create,
UpdateNew: filterOwnedRecords(sdr.ownerID, changes.UpdateNew),
UpdateOld: filterOwnedRecords(sdr.ownerID, changes.UpdateOld),
Delete: filterOwnedRecords(sdr.ownerID, changes.Delete),
UpdateNew: endpoint.FilterEndpointsByOwnerID(sdr.ownerID, changes.UpdateNew),
UpdateOld: endpoint.FilterEndpointsByOwnerID(sdr.ownerID, changes.UpdateOld),
Delete: endpoint.FilterEndpointsByOwnerID(sdr.ownerID, changes.Delete),
}
sdr.updateLabels(filteredChanges.Create)

View File

@ -104,6 +104,10 @@ func (im *DynamoDBRegistry) GetDomainFilter() endpoint.DomainFilter {
return im.provider.GetDomainFilter()
}
func (im *DynamoDBRegistry) OwnerID() string {
return im.ownerID
}
// Records returns the current records from the registry.
func (im *DynamoDBRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, error) {
// If we have the zones cached AND we have refreshed the cache since the
@ -211,9 +215,9 @@ func (im *DynamoDBRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint,
func (im *DynamoDBRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) error {
filteredChanges := &plan.Changes{
Create: changes.Create,
UpdateNew: filterOwnedRecords(im.ownerID, changes.UpdateNew),
UpdateOld: filterOwnedRecords(im.ownerID, changes.UpdateOld),
Delete: filterOwnedRecords(im.ownerID, changes.Delete),
UpdateNew: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.UpdateNew),
UpdateOld: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.UpdateOld),
Delete: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.Delete),
}
statements := make([]*dynamodb.BatchStatementRequest, 0, len(filteredChanges.Create)+len(filteredChanges.UpdateNew))

View File

@ -40,6 +40,10 @@ func (im *NoopRegistry) GetDomainFilter() endpoint.DomainFilter {
return im.provider.GetDomainFilter()
}
func (im *NoopRegistry) OwnerID() string {
return ""
}
// Records returns the current records from the dns provider
func (im *NoopRegistry) Records(ctx context.Context) ([]*endpoint.Endpoint, error) {
return im.provider.Records(ctx)

View File

@ -19,8 +19,6 @@ package registry
import (
"context"
log "github.com/sirupsen/logrus"
"sigs.k8s.io/external-dns/endpoint"
"sigs.k8s.io/external-dns/plan"
)
@ -34,17 +32,5 @@ type Registry interface {
ApplyChanges(ctx context.Context, changes *plan.Changes) error
AdjustEndpoints(endpoints []*endpoint.Endpoint) []*endpoint.Endpoint
GetDomainFilter() endpoint.DomainFilter
}
// TODO(ideahitme): consider moving this to Plan
func filterOwnedRecords(ownerID string, eps []*endpoint.Endpoint) []*endpoint.Endpoint {
filtered := []*endpoint.Endpoint{}
for _, ep := range eps {
if endpointOwner, ok := ep.Labels[endpoint.OwnerLabelKey]; !ok || endpointOwner != ownerID {
log.Debugf(`Skipping endpoint %v because owner id does not match, found: "%s", required: "%s"`, ep, endpointOwner, ownerID)
continue
}
filtered = append(filtered, ep)
}
return filtered
OwnerID() string
}

View File

@ -97,6 +97,10 @@ func (im *TXTRegistry) GetDomainFilter() endpoint.DomainFilter {
return im.provider.GetDomainFilter()
}
func (im *TXTRegistry) OwnerID() string {
return im.ownerID
}
// Records returns the current records from the registry excluding TXT Records
// If TXT records was created previously to indicate ownership its corresponding value
// will be added to the endpoints Labels map
@ -236,9 +240,9 @@ func (im *TXTRegistry) generateTXTRecord(r *endpoint.Endpoint) []*endpoint.Endpo
func (im *TXTRegistry) ApplyChanges(ctx context.Context, changes *plan.Changes) error {
filteredChanges := &plan.Changes{
Create: changes.Create,
UpdateNew: filterOwnedRecords(im.ownerID, changes.UpdateNew),
UpdateOld: filterOwnedRecords(im.ownerID, changes.UpdateOld),
Delete: filterOwnedRecords(im.ownerID, changes.Delete),
UpdateNew: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.UpdateNew),
UpdateOld: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.UpdateOld),
Delete: endpoint.FilterEndpointsByOwnerID(im.ownerID, changes.Delete),
}
for _, r := range filteredChanges.Create {
if r.Labels == nil {

View File

@ -1440,6 +1440,68 @@ func TestFailGenerateTXT(t *testing.T) {
assert.Equal(t, expectedTXT, gotTXT)
}
// TestMultiClusterDifferentRecordTypeOwnership validates the registry handles environments where the same zone is managed by
// external-dns in different clusters and the ingress record type is different. For example one uses A records and the other
// uses CNAME. In this environment the first cluster that establishes the owner record should maintain ownership even
// if the same ingress host is deployed to the other. With the introduction of Dual Record support each record type
// was treated independently and would cause each cluster to fight over ownership. This tests ensure that the default
// Dual Stack record support only treats AAAA records independently and while keeping A and CNAME record ownership intact.
func TestMultiClusterDifferentRecordTypeOwnership(t *testing.T) {
ctx := context.Background()
p := inmemory.NewInMemoryProvider()
p.CreateZone(testZone)
p.ApplyChanges(ctx, &plan.Changes{
Create: []*endpoint.Endpoint{
// records on cluster using A record for ingress address
newEndpointWithOwner("bar.test-zone.example.org", "\"heritage=external-dns,external-dns/owner=cat,external-dns/resource=ingress/default/foo\"", endpoint.RecordTypeTXT, ""),
newEndpointWithOwner("bar.test-zone.example.org", "1.2.3.4", endpoint.RecordTypeA, ""),
},
})
r, _ := NewTXTRegistry(p, "_owner.", "", "bar", time.Hour, "", []string{}, false, nil)
records, _ := r.Records(ctx)
// new cluster has same ingress host as other cluster and uses CNAME ingress address
cname := &endpoint.Endpoint{
DNSName: "bar.test-zone.example.org",
Targets: endpoint.Targets{"cluster-b"},
RecordType: "CNAME",
Labels: map[string]string{
endpoint.ResourceLabelKey: "ingress/default/foo-127",
},
}
desired := []*endpoint.Endpoint{cname}
pl := &plan.Plan{
Policies: []plan.Policy{&plan.SyncPolicy{}},
Current: records,
Desired: desired,
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME},
}
changes := pl.Calculate()
p.OnApplyChanges = func(ctx context.Context, changes *plan.Changes) {
got := map[string][]*endpoint.Endpoint{
"Create": changes.Create,
"UpdateNew": changes.UpdateNew,
"UpdateOld": changes.UpdateOld,
"Delete": changes.Delete,
}
expected := map[string][]*endpoint.Endpoint{
"Create": {},
"UpdateNew": {},
"UpdateOld": {},
"Delete": {},
}
testutils.SamePlanChanges(got, expected)
}
err := r.ApplyChanges(ctx, changes.Changes)
if err != nil {
t.Error(err)
}
}
/**
helper methods