diff --git a/plan/conflict.go b/plan/conflict.go index 283f36d404..8f60c0622f 100644 --- a/plan/conflict.go +++ b/plan/conflict.go @@ -67,8 +67,8 @@ 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 { @@ -95,8 +95,8 @@ func (s PerResource) ResolveRecordTypes(key planKey, row *planTableRow) map[stri log.Warnf("Domain %s contains conflicting record type candidates, keeping CNAME record", key.dnsName) records := map[string]*domainEndpoints{} for recordType, recs := range row.records { - if recordType == endpoint.RecordTypeCNAME { - // policy is to prefer the CNAME record type when a conflic is found + if recordType != endpoint.RecordTypeCNAME { + // policy is to prefer the A and AAAA record type when a conflict is found records[recordType] = recs } else { // discard candidates of conflicting records diff --git a/plan/conflict_test.go b/plan/conflict_test.go index 3fcba8220c..d09d028374 100644 --- a/plan/conflict_test.go +++ b/plan/conflict_test.go @@ -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,14 +223,14 @@ 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: { - current: suite.fooA5, candidates: []*endpoint.Endpoint{suite.fooA5}, }, }, @@ -238,47 +238,47 @@ func (suite *ResolverSuite) TestPerResource_ResolveRecordTypes() { }, want: map[string]*domainEndpoints{ endpoint.RecordTypeCNAME: { - candidates: []*endpoint.Endpoint{suite.fooV1Cname}, + current: suite.fooV1Cname, + candidates: []*endpoint.Endpoint{}, }, endpoint.RecordTypeA: { - current: suite.fooA5, - candidates: []*endpoint.Endpoint{}, + candidates: []*endpoint.Endpoint{suite.fooA5}, }, }, }, { - name: "conflict: cname, a, and aaa records", + name: "conflict: cname, a, and aaaa 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}, + 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{}, + candidates: []*endpoint.Endpoint{suite.fooA5}, }, 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}, }, }, }, diff --git a/plan/plan_test.go b/plan/plan_test.go index 577e34773e..acd5116c83 100644 --- a/plan/plan_test.go +++ b/plan/plan_test.go @@ -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{}