Plan record type conflict should prefer A and AAAA records

endpoint.Targets.Less gives priority to A and AAAA over CNAME records so the
plan record type conflict resolver should also prefer A/AAAA.
This commit is contained in:
Kyle Cronin 2023-09-02 16:35:16 -04:00
parent cf756a76b2
commit 55ae13ef21
4 changed files with 60 additions and 59 deletions

View File

@ -20,6 +20,7 @@ import (
"sort"
log "github.com/sirupsen/logrus"
"sigs.k8s.io/external-dns/endpoint"
)
@ -67,11 +68,12 @@ func (s PerResource) ResolveUpdate(current *endpoint.Endpoint, candidates []*end
// 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 CNAME record types when a conflict is
// detected.
// 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
}
@ -92,19 +94,19 @@ func (s PerResource) ResolveRecordTypes(key planKey, row *planTableRow) map[stri
// conflict was found, remove candiates of non-preferred record types
if cname && other {
log.Warnf("Domain %s contains conflicting record type candidates, keeping CNAME record", key.dnsName)
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 {
// policy is to prefer the CNAME record type when a conflic is found
records[recordType] = recs
} else {
// 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
}
}

View File

@ -34,7 +34,7 @@ type ResolverSuite struct {
fooV2Cname *endpoint.Endpoint
fooV2CnameDuplicate *endpoint.Endpoint
fooA5 *endpoint.Endpoint
fooAAA5 *endpoint.Endpoint
fooAAAA5 *endpoint.Endpoint
bar127A *endpoint.Endpoint
bar192A *endpoint.Endpoint
bar127AAnother *endpoint.Endpoint
@ -77,7 +77,7 @@ func (suite *ResolverSuite) SetupTest() {
endpoint.ResourceLabelKey: "ingress/default/foo-5",
},
}
suite.fooAAA5 = &endpoint.Endpoint{
suite.fooAAAA5 = &endpoint.Endpoint{
DNSName: "foo",
Targets: endpoint.Targets{"2001:DB8::1"},
RecordType: "AAAA",
@ -194,17 +194,17 @@ func (suite *ResolverSuite) TestPerResource_ResolveRecordTypes() {
},
},
{
name: "no conflict: a and aaa records",
name: "no conflict: a and aaaa records",
args: args{
key: planKey{dnsName: "foo"},
row: &planTableRow{
candidates: []*endpoint.Endpoint{suite.fooA5, suite.fooAAA5},
candidates: []*endpoint.Endpoint{suite.fooA5, suite.fooAAAA5},
records: map[string]*domainEndpoints{
endpoint.RecordTypeA: {
candidates: []*endpoint.Endpoint{suite.fooA5},
},
endpoint.RecordTypeAAAA: {
candidates: []*endpoint.Endpoint{suite.fooAAA5},
candidates: []*endpoint.Endpoint{suite.fooAAAA5},
},
},
},
@ -214,7 +214,7 @@ func (suite *ResolverSuite) TestPerResource_ResolveRecordTypes() {
candidates: []*endpoint.Endpoint{suite.fooA5},
},
endpoint.RecordTypeAAAA: {
candidates: []*endpoint.Endpoint{suite.fooAAA5},
candidates: []*endpoint.Endpoint{suite.fooAAAA5},
},
},
},
@ -223,8 +223,36 @@ func (suite *ResolverSuite) TestPerResource_ResolveRecordTypes() {
args: args{
key: planKey{dnsName: "foo"},
row: &planTableRow{
current: []*endpoint.Endpoint{suite.fooA5},
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},
@ -233,52 +261,24 @@ func (suite *ResolverSuite) TestPerResource_ResolveRecordTypes() {
current: suite.fooA5,
candidates: []*endpoint.Endpoint{suite.fooA5},
},
},
},
},
want: map[string]*domainEndpoints{
endpoint.RecordTypeCNAME: {
candidates: []*endpoint.Endpoint{suite.fooV1Cname},
},
endpoint.RecordTypeA: {
current: suite.fooA5,
candidates: []*endpoint.Endpoint{},
},
},
},
{
name: "conflict: cname, a, and aaa records",
args: args{
key: planKey{dnsName: "foo"},
row: &planTableRow{
current: []*endpoint.Endpoint{suite.fooA5, suite.fooAAA5},
candidates: []*endpoint.Endpoint{suite.fooV1Cname, suite.fooA5, suite.fooAAA5},
records: map[string]*domainEndpoints{
endpoint.RecordTypeCNAME: {
candidates: []*endpoint.Endpoint{suite.fooV1Cname},
},
endpoint.RecordTypeA: {
current: suite.fooA5,
candidates: []*endpoint.Endpoint{},
},
endpoint.RecordTypeAAAA: {
current: suite.fooAAA5,
candidates: []*endpoint.Endpoint{},
current: suite.fooAAAA5,
candidates: []*endpoint.Endpoint{suite.fooAAAA5},
},
},
},
},
want: map[string]*domainEndpoints{
endpoint.RecordTypeCNAME: {
candidates: []*endpoint.Endpoint{suite.fooV1Cname},
candidates: []*endpoint.Endpoint{},
},
endpoint.RecordTypeA: {
current: suite.fooA5,
candidates: []*endpoint.Endpoint{},
candidates: []*endpoint.Endpoint{suite.fooA5},
},
endpoint.RecordTypeAAAA: {
current: suite.fooAAA5,
candidates: []*endpoint.Endpoint{},
current: suite.fooAAAA5,
candidates: []*endpoint.Endpoint{suite.fooAAAA5},
},
},
},

View File

@ -97,9 +97,9 @@ func newPlanTable() planTable { // TODO: make resolver configurable
// planTableRow represents a set of current and desired domain resource records.
type planTableRow struct {
// current corresponds to the records currently occupying dns name on the dns provider. More than 1 record may
// be represented here, for example A and AAAA. If current domain record is CNAME, no other record types are allowed
// per [RFC 1034 3.6.2]
// 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

View File

@ -609,23 +609,22 @@ func (suite *PlanTestSuite) TestConflictingCurrentNoDesired() {
// 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 CNAME record candidate and delete the other records.
// resolver should prefer the A and AAAA record candidate and delete the other records.
func (suite *PlanTestSuite) TestCurrentWithConflictingDesired() {
suite.fooA5.Labels[endpoint.OwnerLabelKey] = "nerf"
suite.fooAAAA.Labels[endpoint.OwnerLabelKey] = "nerf"
current := []*endpoint.Endpoint{suite.fooA5, suite.fooAAAA}
suite.fooV1Cname.Labels[endpoint.OwnerLabelKey] = "nerf"
current := []*endpoint.Endpoint{suite.fooV1Cname}
desired := []*endpoint.Endpoint{suite.fooV1Cname, suite.fooA5, suite.fooAAAA}
expectedCreate := []*endpoint.Endpoint{suite.fooV1Cname}
expectedCreate := []*endpoint.Endpoint{suite.fooA5, suite.fooAAAA}
expectedUpdateOld := []*endpoint.Endpoint{}
expectedUpdateNew := []*endpoint.Endpoint{}
expectedDelete := []*endpoint.Endpoint{suite.fooA5, suite.fooAAAA}
expectedDelete := []*endpoint.Endpoint{suite.fooV1Cname}
p := &Plan{
Policies: []Policy{&SyncPolicy{}},
Current: current,
Desired: desired,
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME},
OwnerID: suite.fooA5.Labels[endpoint.OwnerLabelKey],
OwnerID: suite.fooV1Cname.Labels[endpoint.OwnerLabelKey],
}
changes := p.Calculate().Changes
@ -637,11 +636,11 @@ func (suite *PlanTestSuite) TestCurrentWithConflictingDesired() {
// 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 CNAME record and drop the other candidate record types.
// 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.fooV1Cname}
expectedCreate := []*endpoint.Endpoint{suite.fooA5, suite.fooAAAA}
expectedUpdateOld := []*endpoint.Endpoint{}
expectedUpdateNew := []*endpoint.Endpoint{}
expectedDelete := []*endpoint.Endpoint{}