From 7fda2407fb4228f9607729499b2fd236220d9af1 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Thu, 11 Dec 2025 07:56:20 +0900 Subject: [PATCH 1/9] test: add regression test to ensure behavior before refactor Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- internal/testutils/endpoint.go | 3 + provider/aws/aws.go | 102 ++++++++++++++++++--------------- provider/aws/aws_test.go | 75 ++++++++++++++++++------ 3 files changed, 118 insertions(+), 62 deletions(-) diff --git a/internal/testutils/endpoint.go b/internal/testutils/endpoint.go index 0a33d3a218..33b0c28881 100644 --- a/internal/testutils/endpoint.go +++ b/internal/testutils/endpoint.go @@ -63,6 +63,9 @@ 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 { + if a == nil || b == nil { + return a == b + } 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] && diff --git a/provider/aws/aws.go b/provider/aws/aws.go index a5d964b67f..d2ae05c34e 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -827,65 +827,77 @@ func (p *AWSProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoi var aliasCnameAaaaEndpoints []*endpoint.Endpoint for _, ep := range endpoints { - alias := false - - if aliasString, ok := ep.GetProviderSpecificProperty(providerSpecificAlias); ok { - alias = aliasString == "true" - if alias { - if !slices.Contains([]string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME}, ep.RecordType) { - ep.DeleteProviderSpecificProperty(providerSpecificAlias) - } - } else { - if ep.RecordType == endpoint.RecordTypeCNAME { - if aliasString != "false" { - ep.SetProviderSpecificProperty(providerSpecificAlias, "false") - } - } else { - ep.DeleteProviderSpecificProperty(providerSpecificAlias) - } - } - } else if ep.RecordType == endpoint.RecordTypeCNAME { - alias = useAlias(ep, p.preferCNAME) - log.Debugf("Modifying endpoint: %v, setting %s=%v", ep, providerSpecificAlias, alias) - ep.SetProviderSpecificProperty(providerSpecificAlias, strconv.FormatBool(alias)) + if aaaa := p.adjustEndpointAndNewAaaaIfNeeded(ep); aaaa != nil { + aliasCnameAaaaEndpoints = append(aliasCnameAaaaEndpoints, aaaa) } + } + return append(endpoints, aliasCnameAaaaEndpoints...), nil +} + +func (p *AWSProvider) adjustEndpointAndNewAaaaIfNeeded(ep *endpoint.Endpoint) *endpoint.Endpoint { + // TODO: implement new logic here + return p.oldAdjustEndpointAndNewAaaaIfNeeded(ep) +} + +func (p *AWSProvider) oldAdjustEndpointAndNewAaaaIfNeeded(ep *endpoint.Endpoint) *endpoint.Endpoint { + var result *endpoint.Endpoint + + alias := false + if aliasString, ok := ep.GetProviderSpecificProperty(providerSpecificAlias); ok { + alias = aliasString == "true" if alias { - if ep.RecordTTL.IsConfigured() { - log.Debugf("Modifying endpoint: %v, setting ttl=%v", ep, defaultTTL) - ep.RecordTTL = defaultTTL + if !slices.Contains([]string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME}, ep.RecordType) { + ep.DeleteProviderSpecificProperty(providerSpecificAlias) } - if prop, ok := ep.GetProviderSpecificProperty(providerSpecificEvaluateTargetHealth); ok { - if prop != "true" && prop != "false" { - ep.SetProviderSpecificProperty(providerSpecificEvaluateTargetHealth, "false") + } else { + if ep.RecordType == endpoint.RecordTypeCNAME { + if aliasString != "false" { + ep.SetProviderSpecificProperty(providerSpecificAlias, "false") } } else { - ep.SetProviderSpecificProperty(providerSpecificEvaluateTargetHealth, strconv.FormatBool(p.evaluateTargetHealth)) + ep.DeleteProviderSpecificProperty(providerSpecificAlias) } + } + } else if ep.RecordType == endpoint.RecordTypeCNAME { + alias = useAlias(ep, p.preferCNAME) + log.Debugf("Modifying endpoint: %v, setting %s=%v", ep, providerSpecificAlias, alias) + ep.SetProviderSpecificProperty(providerSpecificAlias, strconv.FormatBool(alias)) + } - if ep.RecordType == endpoint.RecordTypeCNAME { - // This needs to match two records from Route53, one alias for 'A' (IPv4) - // and one alias for 'AAAA' (IPv6). - aliasCnameAaaaEndpoints = append(aliasCnameAaaaEndpoints, &endpoint.Endpoint{ - DNSName: ep.DNSName, - Targets: ep.Targets, - RecordType: endpoint.RecordTypeAAAA, - RecordTTL: ep.RecordTTL, - Labels: ep.Labels, - ProviderSpecific: ep.ProviderSpecific, - SetIdentifier: ep.SetIdentifier, - }) - ep.RecordType = endpoint.RecordTypeA + if alias { + if ep.RecordTTL.IsConfigured() { + log.Debugf("Modifying endpoint: %v, setting ttl=%v", ep, defaultTTL) + ep.RecordTTL = defaultTTL + } + if prop, ok := ep.GetProviderSpecificProperty(providerSpecificEvaluateTargetHealth); ok { + if prop != "true" && prop != "false" { + ep.SetProviderSpecificProperty(providerSpecificEvaluateTargetHealth, "false") } } else { - ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) + ep.SetProviderSpecificProperty(providerSpecificEvaluateTargetHealth, strconv.FormatBool(p.evaluateTargetHealth)) } - adjustGeoProximityLocationEndpoint(ep) + if ep.RecordType == endpoint.RecordTypeCNAME { + // This needs to match two records from Route53, one alias for 'A' (IPv4) + // and one alias for 'AAAA' (IPv6). + result = &endpoint.Endpoint{ + DNSName: ep.DNSName, + Targets: ep.Targets, + RecordType: endpoint.RecordTypeAAAA, + RecordTTL: ep.RecordTTL, + Labels: ep.Labels, + ProviderSpecific: ep.ProviderSpecific, + SetIdentifier: ep.SetIdentifier, + } + ep.RecordType = endpoint.RecordTypeA + } + } else { + ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) } - endpoints = append(endpoints, aliasCnameAaaaEndpoints...) - return endpoints, nil + adjustGeoProximityLocationEndpoint(ep) + return result } // if the endpoint is using geoproximity, set the bias to 0 if not set diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index a269c3ede8..50580e0297 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -341,7 +341,7 @@ func TestAWSZones(t *testing.T) { {"tag filter single zone match", provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), provider.NewZoneTagFilter([]string{"zone=3"}), privateZones}, } { t.Run(ti.msg, func(t *testing.T) { - provider, _ := newAWSProviderWithTagFilter(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), ti.zoneIDFilter, ti.zoneTypeFilter, ti.zoneTagFilter, defaultEvaluateTargetHealth, false, nil) + provider, _ := newAWSProviderWithTagFilter(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), ti.zoneIDFilter, ti.zoneTypeFilter, ti.zoneTagFilter, defaultEvaluateTargetHealth, false, false, nil) zones, err := provider.Zones(context.Background()) require.NoError(t, err) validateAWSZones(t, zones, ti.expectedZones) @@ -373,7 +373,7 @@ func TestAWSZonesWithTagFilterError(t *testing.T) { } func TestAWSRecordsFilter(t *testing.T) { - provider, _ := newAWSProvider(t, &endpoint.DomainFilter{}, provider.ZoneIDFilter{}, provider.ZoneTypeFilter{}, false, false, nil) + provider, _ := newAWSProvider(t, &endpoint.DomainFilter{}, provider.ZoneIDFilter{}, provider.ZoneTypeFilter{}, false, false, false, nil) domainFilter := provider.GetDomainFilter() require.NotNil(t, domainFilter) require.IsType(t, &endpoint.DomainFilter{}, domainFilter) @@ -396,7 +396,7 @@ func TestAWSRecordsFilter(t *testing.T) { } func TestAWSRecords(t *testing.T) { - provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), false, false, []route53types.ResourceRecordSet{ + provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), false, false, false, []route53types.ResourceRecordSet{ { Name: aws.String("list-test.zone-1.ext-dns-test-2.teapot.zalan.do."), Type: route53types.RRTypeA, @@ -682,7 +682,7 @@ func TestAWSRecords(t *testing.T) { } func TestAWSRecordsSoftError(t *testing.T) { - pvd, subClient := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), false, false, []route53types.ResourceRecordSet{ + pvd, subClient := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), false, false, false, []route53types.ResourceRecordSet{ { Name: aws.String("list-test.zone-1.ext-dns-test-2.teapot.zalan.do."), Type: route53types.RRTypeA, @@ -698,7 +698,7 @@ func TestAWSRecordsSoftError(t *testing.T) { } func TestAWSAdjustEndpoints(t *testing.T) { - provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, nil) + provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, false, nil) records := []*endpoint.Endpoint{ endpoint.NewEndpoint("a-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "8.8.8.8"), @@ -747,7 +747,7 @@ func TestAWSApplyChanges(t *testing.T) { } for _, tt := range tests { - provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, []route53types.ResourceRecordSet{ + provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, false, []route53types.ResourceRecordSet{ { Name: aws.String("update-test.zone-1.ext-dns-test-2.teapot.zalan.do."), Type: route53types.RRTypeA, @@ -1378,7 +1378,7 @@ func TestAWSApplyChangesDryRun(t *testing.T) { }, } - provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, true, originalRecords) + provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, true, originalRecords) createRecords := []*endpoint.Endpoint{ endpoint.NewEndpoint("create-test.zone-1.ext-dns-test-2.teapot.zalan.do", endpoint.RecordTypeA, "8.8.8.8"), @@ -1565,7 +1565,7 @@ func TestAWSChangesByZones(t *testing.T) { } func TestAWSsubmitChanges(t *testing.T) { - provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, nil) + provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, false, nil) const subnets = 16 const hosts = defaultBatchChangeSize / subnets @@ -1594,7 +1594,7 @@ func TestAWSsubmitChanges(t *testing.T) { } func TestAWSsubmitChangesError(t *testing.T) { - provider, clientStub := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, nil) + provider, clientStub := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, false, nil) clientStub.MockMethod("ChangeResourceRecordSets", mock.Anything).Return(nil, fmt.Errorf("Mock route53 failure")) ctx := context.Background() @@ -1608,7 +1608,7 @@ func TestAWSsubmitChangesError(t *testing.T) { } func TestAWSsubmitChangesRetryOnError(t *testing.T) { - provider, clientStub := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, nil) + provider, clientStub := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, false, nil) ctx := context.Background() zones, err := provider.zones(ctx) @@ -2043,7 +2043,7 @@ func validateAWSChangeRecord(t *testing.T, record *Route53Change, expected *Rout } func TestAWSCreateRecordsWithCNAME(t *testing.T) { - provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, nil) + provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, false, nil) records := []*endpoint.Endpoint{ {DNSName: "create-test.zone-1.ext-dns-test-2.teapot.zalan.do", Targets: endpoint.Targets{"foo.example.org"}, RecordType: endpoint.RecordTypeCNAME}, @@ -2077,7 +2077,7 @@ func TestAWSCreateRecordsWithALIAS(t *testing.T) { "false": false, "": false, } { - provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, nil) + provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"ext-dns-test-2.teapot.zalan.do."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, false, nil) records := []*endpoint.Endpoint{ { DNSName: "create-test.zone-1.ext-dns-test-2.teapot.zalan.do", @@ -2318,11 +2318,11 @@ func listAWSRecords(t *testing.T, client Route53API, zone string) []route53types return resp.ResourceRecordSets } -func newAWSProvider(t *testing.T, domainFilter *endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, zoneTypeFilter provider.ZoneTypeFilter, evaluateTargetHealth, dryRun bool, records []route53types.ResourceRecordSet) (*AWSProvider, *Route53APIStub) { - return newAWSProviderWithTagFilter(t, domainFilter, zoneIDFilter, zoneTypeFilter, provider.NewZoneTagFilter([]string{}), evaluateTargetHealth, dryRun, records) +func newAWSProvider(t *testing.T, domainFilter *endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, zoneTypeFilter provider.ZoneTypeFilter, evaluateTargetHealth, preferCNAME, dryRun bool, records []route53types.ResourceRecordSet) (*AWSProvider, *Route53APIStub) { + return newAWSProviderWithTagFilter(t, domainFilter, zoneIDFilter, zoneTypeFilter, provider.NewZoneTagFilter([]string{}), evaluateTargetHealth, preferCNAME, dryRun, records) } -func newAWSProviderWithTagFilter(t *testing.T, domainFilter *endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, zoneTypeFilter provider.ZoneTypeFilter, zoneTagFilter provider.ZoneTagFilter, evaluateTargetHealth, dryRun bool, records []route53types.ResourceRecordSet) (*AWSProvider, *Route53APIStub) { +func newAWSProviderWithTagFilter(t *testing.T, domainFilter *endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, zoneTypeFilter provider.ZoneTypeFilter, zoneTagFilter provider.ZoneTagFilter, evaluateTargetHealth, preferCNAME, dryRun bool, records []route53types.ResourceRecordSet) (*AWSProvider, *Route53APIStub) { client := NewRoute53APIStub(t) provider := &AWSProvider{ @@ -2336,6 +2336,7 @@ func newAWSProviderWithTagFilter(t *testing.T, domainFilter *endpoint.DomainFilt zoneIDFilter: zoneIDFilter, zoneTypeFilter: zoneTypeFilter, zoneTagFilter: zoneTagFilter, + preferCNAME: preferCNAME, dryRun: false, zonesCache: &zonesListCache{duration: 1 * time.Minute}, failedChangesQueue: make(map[string]Route53Changes), @@ -2423,7 +2424,7 @@ func containsRecordWithDNSName(records []*endpoint.Endpoint, dnsName string) boo } func TestRequiresDeleteCreate(t *testing.T) { - provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"foo.bar."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, nil) + provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"foo.bar."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), defaultEvaluateTargetHealth, false, false, nil) oldRecordType := endpoint.NewEndpointWithTTL("recordType", endpoint.RecordTypeA, endpoint.TTL(defaultTTL), "8.8.8.8") newRecordType := endpoint.NewEndpointWithTTL("recordType", endpoint.RecordTypeCNAME, endpoint.TTL(defaultTTL), "bar").WithProviderSpecific(providerSpecificAlias, "false") @@ -2819,7 +2820,7 @@ func TestGeoProximityWithBias(t *testing.T) { } func TestAWSProvider_createUpdateChanges_NewMoreThanOld(t *testing.T) { - provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"foo.bar."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), true, false, nil) + provider, _ := newAWSProvider(t, endpoint.NewDomainFilter([]string{"foo.bar."}), provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), true, false, false, nil) oldEndpoints := []*endpoint.Endpoint{ endpoint.NewEndpointWithTTL("record1.foo.bar.", endpoint.RecordTypeA, endpoint.TTL(300), "1.1.1.1"), @@ -2850,3 +2851,43 @@ func TestAWSProvider_createUpdateChanges_NewMoreThanOld(t *testing.T) { require.Equal(t, 1, upserts, "should upsert the matching endpoint") require.Equal(t, 0, deletes, "should not delete anything") } + +func TestAdjustEndpoint_RefactorDoesNotChangeBehavior(t *testing.T) { + for _, aliasProp := range []string{"", "true", "false", "invalid"} { + for _, rt := range []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME, endpoint.RecordTypeMX} { + for _, preferCNAME := range []bool{true, false} { + for _, evalTH := range []bool{true, false} { + for _, ttl := range []int{300, 600} { + name := fmt.Sprintf("alias=%v,rt=%s,prefer=%v,eval=%v,ttl=%v", aliasProp, rt, preferCNAME, evalTH, ttl) + t.Run(name, func(t *testing.T) { + t.Parallel() + p, _ := newAWSProvider(t, nil, provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), evalTH, preferCNAME, false, nil) + + ep := &endpoint.Endpoint{ + DNSName: "test.example.com", + Targets: endpoint.Targets{"target.example.com"}, + RecordType: rt, + RecordTTL: endpoint.TTL(ttl), + } + if aliasProp != "" { + ep.ProviderSpecific = endpoint.ProviderSpecific{ + endpoint.ProviderSpecificProperty{ + Name: providerSpecificAlias, + Value: aliasProp, + }, + } + } + epOld := ep.DeepCopy() + epNew := ep.DeepCopy() + oldLogicAAAA := p.oldAdjustEndpointAndNewAaaaIfNeeded(epOld) + newLogicAAAA := p.adjustEndpointAndNewAaaaIfNeeded(epNew) + + assert.True(t, testutils.SameEndpoint(epOld, epNew), "input endpoints differ between old and new logic") + assert.True(t, testutils.SameEndpoint(oldLogicAAAA, newLogicAAAA), "adjusted endpoints differ between old and new logic") + }) + } + } + } + } + } +} From b50b0a59225888525684d4c49beb2c69d3aa5d25 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Thu, 11 Dec 2025 08:53:02 +0900 Subject: [PATCH 2/9] refactor aws adjustEndpointAndAaaaIfNeeded Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- provider/aws/aws.go | 79 +++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 77 insertions(+), 2 deletions(-) diff --git a/provider/aws/aws.go b/provider/aws/aws.go index d2ae05c34e..320164ab59 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -835,8 +835,83 @@ func (p *AWSProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]*endpoi } func (p *AWSProvider) adjustEndpointAndNewAaaaIfNeeded(ep *endpoint.Endpoint) *endpoint.Endpoint { - // TODO: implement new logic here - return p.oldAdjustEndpointAndNewAaaaIfNeeded(ep) + setAliasConf := func(ep *endpoint.Endpoint) { + if ep.RecordTTL.IsConfigured() { + log.Debugf("Modifying endpoint: %v, setting ttl=%v", ep, defaultTTL) + ep.RecordTTL = defaultTTL + } + if prop, ok := ep.GetProviderSpecificProperty(providerSpecificEvaluateTargetHealth); ok { + if prop != "true" && prop != "false" { + ep.SetProviderSpecificProperty(providerSpecificEvaluateTargetHealth, "false") + } + } else { + ep.SetProviderSpecificProperty(providerSpecificEvaluateTargetHealth, strconv.FormatBool(p.evaluateTargetHealth)) + } + } + cnameAliasCase := func(ep *endpoint.Endpoint) *endpoint.Endpoint { + setAliasConf(ep) + result := &endpoint.Endpoint{ + DNSName: ep.DNSName, + Targets: ep.Targets, + RecordType: endpoint.RecordTypeAAAA, + RecordTTL: ep.RecordTTL, + Labels: ep.Labels, + ProviderSpecific: ep.ProviderSpecific, + SetIdentifier: ep.SetIdentifier, + } + ep.RecordType = endpoint.RecordTypeA + return result + } + + var additionalAAAA *endpoint.Endpoint + switch ep.RecordType { + case endpoint.RecordTypeA, endpoint.RecordTypeAAAA: + aliasString, ok := ep.GetProviderSpecificProperty(providerSpecificAlias) + switch aliasString { + case "true": + setAliasConf(ep) + case "": + if ok { + ep.DeleteProviderSpecificProperty(providerSpecificAlias) + } + ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) + default: + ep.DeleteProviderSpecificProperty(providerSpecificAlias) + ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) + } + case endpoint.RecordTypeCNAME: + aliasString, _ := ep.GetProviderSpecificProperty(providerSpecificAlias) + switch aliasString { + case "true": + additionalAAAA = cnameAliasCase(ep) + case "": + alias := useAlias(ep, p.preferCNAME) + log.Debugf("Modifying endpoint: %v, setting %s=%v", ep, providerSpecificAlias, alias) + ep.SetProviderSpecificProperty(providerSpecificAlias, strconv.FormatBool(alias)) + if alias { + additionalAAAA = cnameAliasCase(ep) + } else { + ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) + } + default: + ep.SetProviderSpecificProperty(providerSpecificAlias, "false") + ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) + } + default: + // TODO fix + aliasString, _ := ep.GetProviderSpecificProperty(providerSpecificAlias) + switch aliasString { + case "true": + ep.DeleteProviderSpecificProperty(providerSpecificAlias) + setAliasConf(ep) + default: + ep.DeleteProviderSpecificProperty(providerSpecificAlias) + ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) + } + } + + adjustGeoProximityLocationEndpoint(ep) + return additionalAAAA } func (p *AWSProvider) oldAdjustEndpointAndNewAaaaIfNeeded(ep *endpoint.Endpoint) *endpoint.Endpoint { From 94dd0d39892078561b58f1b391d6322b4e58ba94 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Fri, 12 Dec 2025 07:18:23 +0900 Subject: [PATCH 3/9] fix clean up alias properties for unsupported record types Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- provider/aws/aws.go | 12 ++---------- provider/aws/aws_test.go | 5 +++++ 2 files changed, 7 insertions(+), 10 deletions(-) diff --git a/provider/aws/aws.go b/provider/aws/aws.go index 320164ab59..a6a8e04f1e 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -898,16 +898,8 @@ func (p *AWSProvider) adjustEndpointAndNewAaaaIfNeeded(ep *endpoint.Endpoint) *e ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) } default: - // TODO fix - aliasString, _ := ep.GetProviderSpecificProperty(providerSpecificAlias) - switch aliasString { - case "true": - ep.DeleteProviderSpecificProperty(providerSpecificAlias) - setAliasConf(ep) - default: - ep.DeleteProviderSpecificProperty(providerSpecificAlias) - ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) - } + ep.DeleteProviderSpecificProperty(providerSpecificAlias) + ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) } adjustGeoProximityLocationEndpoint(ep) diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index 50580e0297..94177b92e0 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -2880,6 +2880,11 @@ func TestAdjustEndpoint_RefactorDoesNotChangeBehavior(t *testing.T) { epOld := ep.DeepCopy() epNew := ep.DeepCopy() oldLogicAAAA := p.oldAdjustEndpointAndNewAaaaIfNeeded(epOld) + if rt == endpoint.RecordTypeMX { + epOld.RecordTTL = endpoint.TTL(ttl) + epOld.DeleteProviderSpecificProperty(providerSpecificAlias) + epOld.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) + } newLogicAAAA := p.adjustEndpointAndNewAaaaIfNeeded(epNew) assert.True(t, testutils.SameEndpoint(epOld, epNew), "input endpoints differ between old and new logic") From 881e5c4eda5cec2b97c2a5436f8fbbf4f5aaa7f7 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Fri, 12 Dec 2025 08:52:31 +0900 Subject: [PATCH 4/9] test add comprehensive tests and remove old logic Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- provider/aws/aws.go | 68 +---- provider/aws/aws_test.go | 530 +++++++++++++++++++++++++++++++++++---- 2 files changed, 489 insertions(+), 109 deletions(-) diff --git a/provider/aws/aws.go b/provider/aws/aws.go index a6a8e04f1e..580d2abf9c 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -866,15 +866,10 @@ func (p *AWSProvider) adjustEndpointAndNewAaaaIfNeeded(ep *endpoint.Endpoint) *e var additionalAAAA *endpoint.Endpoint switch ep.RecordType { case endpoint.RecordTypeA, endpoint.RecordTypeAAAA: - aliasString, ok := ep.GetProviderSpecificProperty(providerSpecificAlias) + aliasString, _ := ep.GetProviderSpecificProperty(providerSpecificAlias) switch aliasString { case "true": setAliasConf(ep) - case "": - if ok { - ep.DeleteProviderSpecificProperty(providerSpecificAlias) - } - ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) default: ep.DeleteProviderSpecificProperty(providerSpecificAlias) ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) @@ -906,67 +901,6 @@ func (p *AWSProvider) adjustEndpointAndNewAaaaIfNeeded(ep *endpoint.Endpoint) *e return additionalAAAA } -func (p *AWSProvider) oldAdjustEndpointAndNewAaaaIfNeeded(ep *endpoint.Endpoint) *endpoint.Endpoint { - var result *endpoint.Endpoint - - alias := false - - if aliasString, ok := ep.GetProviderSpecificProperty(providerSpecificAlias); ok { - alias = aliasString == "true" - if alias { - if !slices.Contains([]string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME}, ep.RecordType) { - ep.DeleteProviderSpecificProperty(providerSpecificAlias) - } - } else { - if ep.RecordType == endpoint.RecordTypeCNAME { - if aliasString != "false" { - ep.SetProviderSpecificProperty(providerSpecificAlias, "false") - } - } else { - ep.DeleteProviderSpecificProperty(providerSpecificAlias) - } - } - } else if ep.RecordType == endpoint.RecordTypeCNAME { - alias = useAlias(ep, p.preferCNAME) - log.Debugf("Modifying endpoint: %v, setting %s=%v", ep, providerSpecificAlias, alias) - ep.SetProviderSpecificProperty(providerSpecificAlias, strconv.FormatBool(alias)) - } - - if alias { - if ep.RecordTTL.IsConfigured() { - log.Debugf("Modifying endpoint: %v, setting ttl=%v", ep, defaultTTL) - ep.RecordTTL = defaultTTL - } - if prop, ok := ep.GetProviderSpecificProperty(providerSpecificEvaluateTargetHealth); ok { - if prop != "true" && prop != "false" { - ep.SetProviderSpecificProperty(providerSpecificEvaluateTargetHealth, "false") - } - } else { - ep.SetProviderSpecificProperty(providerSpecificEvaluateTargetHealth, strconv.FormatBool(p.evaluateTargetHealth)) - } - - if ep.RecordType == endpoint.RecordTypeCNAME { - // This needs to match two records from Route53, one alias for 'A' (IPv4) - // and one alias for 'AAAA' (IPv6). - result = &endpoint.Endpoint{ - DNSName: ep.DNSName, - Targets: ep.Targets, - RecordType: endpoint.RecordTypeAAAA, - RecordTTL: ep.RecordTTL, - Labels: ep.Labels, - ProviderSpecific: ep.ProviderSpecific, - SetIdentifier: ep.SetIdentifier, - } - ep.RecordType = endpoint.RecordTypeA - } - } else { - ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) - } - - adjustGeoProximityLocationEndpoint(ep) - return result -} - // if the endpoint is using geoproximity, set the bias to 0 if not set // this is needed to avoid unnecessary Upserts if the desired endpoint doesn't specify a bias func adjustGeoProximityLocationEndpoint(ep *endpoint.Endpoint) { diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index 94177b92e0..5c4629f87b 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -2852,47 +2852,493 @@ func TestAWSProvider_createUpdateChanges_NewMoreThanOld(t *testing.T) { require.Equal(t, 0, deletes, "should not delete anything") } -func TestAdjustEndpoint_RefactorDoesNotChangeBehavior(t *testing.T) { - for _, aliasProp := range []string{"", "true", "false", "invalid"} { - for _, rt := range []string{endpoint.RecordTypeA, endpoint.RecordTypeAAAA, endpoint.RecordTypeCNAME, endpoint.RecordTypeMX} { - for _, preferCNAME := range []bool{true, false} { - for _, evalTH := range []bool{true, false} { - for _, ttl := range []int{300, 600} { - name := fmt.Sprintf("alias=%v,rt=%s,prefer=%v,eval=%v,ttl=%v", aliasProp, rt, preferCNAME, evalTH, ttl) - t.Run(name, func(t *testing.T) { - t.Parallel() - p, _ := newAWSProvider(t, nil, provider.NewZoneIDFilter([]string{}), provider.NewZoneTypeFilter(""), evalTH, preferCNAME, false, nil) - - ep := &endpoint.Endpoint{ - DNSName: "test.example.com", - Targets: endpoint.Targets{"target.example.com"}, - RecordType: rt, - RecordTTL: endpoint.TTL(ttl), - } - if aliasProp != "" { - ep.ProviderSpecific = endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: providerSpecificAlias, - Value: aliasProp, - }, - } - } - epOld := ep.DeepCopy() - epNew := ep.DeepCopy() - oldLogicAAAA := p.oldAdjustEndpointAndNewAaaaIfNeeded(epOld) - if rt == endpoint.RecordTypeMX { - epOld.RecordTTL = endpoint.TTL(ttl) - epOld.DeleteProviderSpecificProperty(providerSpecificAlias) - epOld.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) - } - newLogicAAAA := p.adjustEndpointAndNewAaaaIfNeeded(epNew) - - assert.True(t, testutils.SameEndpoint(epOld, epNew), "input endpoints differ between old and new logic") - assert.True(t, testutils.SameEndpoint(oldLogicAAAA, newLogicAAAA), "adjusted endpoints differ between old and new logic") - }) - } - } - } - } +func TestAWSProvider_adjustEndpointAndNewAaaaIfNeeded(t *testing.T) { + tests := []struct { + name string + preferCNAME bool + ep *endpoint.Endpoint + expected *endpoint.Endpoint + expectedAaaa *endpoint.Endpoint + }{ + // --- A / AAAA --- + { + name: "A record without provider specific should not change and not create AAAA", + ep: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeA, + Targets: endpoint.Targets{"1.1.1.1"}, + }, + expected: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeA, + Targets: endpoint.Targets{"1.1.1.1"}, + }, + expectedAaaa: nil, + }, + { + name: "A record with alias=true should set default ttl, add evaluateTargetHealth and not create AAAA", + ep: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeA, + Targets: endpoint.Targets{"1.1.1.1"}, + RecordTTL: 600, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + }, + }, + expected: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeA, + Targets: endpoint.Targets{"1.1.1.1"}, + RecordTTL: defaultTTL, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "false", // p.evaluateTargetHealth=false in this test + }, + }, + }, + expectedAaaa: nil, + }, + { + name: "A record with alias!=true value should remove alias and evaluateTargetHealth and not create AAAA", + ep: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeA, + Targets: endpoint.Targets{"1.1.1.1"}, + RecordTTL: 600, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "false", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "true", + }, + }, + }, + expected: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeA, + Targets: endpoint.Targets{"1.1.1.1"}, + RecordTTL: 600, + ProviderSpecific: endpoint.ProviderSpecific{}, + }, + expectedAaaa: nil, + }, + { + name: "A record with alias=true and invalid evaluateTargetHealth should normalize it to false and set default ttl", + ep: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeA, + Targets: endpoint.Targets{"1.1.1.1"}, + RecordTTL: 600, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "invalid", + }, + }, + }, + expected: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeA, + Targets: endpoint.Targets{"1.1.1.1"}, + RecordTTL: defaultTTL, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "false", + }, + }, + }, + expectedAaaa: nil, + }, + { + name: "AAAA record with alias=true should behave like A record", + ep: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeAAAA, + Targets: endpoint.Targets{"2001:db8::1"}, + RecordTTL: 600, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + }, + }, + expected: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeAAAA, + Targets: endpoint.Targets{"2001:db8::1"}, + RecordTTL: defaultTTL, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "false", + }, + }, + }, + expectedAaaa: nil, + }, + + // --- CNAME --- + { + name: "CNAME record with alias=false should keep alias=false, remove evaluateTargetHealth and not create AAAA", + ep: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeCNAME, + RecordTTL: 600, + Targets: endpoint.Targets{"target.foo.bar."}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "false", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "true", + }, + }, + }, + expected: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeCNAME, + RecordTTL: 600, + Targets: endpoint.Targets{"target.foo.bar."}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "false", + }, + }, + }, + expectedAaaa: nil, + }, + { + name: "CNAME record with invalid alias value should normalize to alias=false and not create AAAA", + ep: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeCNAME, + RecordTTL: 600, + Targets: endpoint.Targets{"target.foo.bar."}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "invalid", + }, + }, + }, + expected: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeCNAME, + RecordTTL: 600, + Targets: endpoint.Targets{"target.foo.bar."}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "false", + }, + }, + }, + expectedAaaa: nil, + }, + { + name: "CNAME record with alias=true should set default ttl, add evaluateTargetHealth and create AAAA", + ep: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeCNAME, + RecordTTL: 600, + Targets: endpoint.Targets{"target.foo.bar."}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + }, + }, + expected: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeA, + RecordTTL: defaultTTL, + Targets: endpoint.Targets{"target.foo.bar."}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "false", // p.evaluateTargetHealth=false in this test + }, + }, + }, + expectedAaaa: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeAAAA, + RecordTTL: defaultTTL, + Targets: endpoint.Targets{"target.foo.bar."}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "false", // p.evaluateTargetHealth=false in this test + }, + }, + }, + }, + { + name: "CNAME record with alias=true and evaluateTargetHealth=true should keep evaluateTargetHealth and create AAAA", + ep: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeCNAME, + RecordTTL: 600, + Targets: endpoint.Targets{"target.foo.bar."}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "true", + }, + }, + }, + expected: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeA, + RecordTTL: defaultTTL, + Targets: endpoint.Targets{"target.foo.bar."}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "true", + }, + }, + }, + expectedAaaa: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeAAAA, + RecordTTL: defaultTTL, + Targets: endpoint.Targets{"target.foo.bar."}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "true", + }, + }, + }, + }, + { + name: "CNAME record with alias=true and invalid evaluateTargetHealth should normalize it to false and create AAAA", + ep: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeCNAME, + RecordTTL: 600, + Targets: endpoint.Targets{"target.foo.bar."}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "invalid", + }, + }, + }, + expected: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeA, + RecordTTL: defaultTTL, + Targets: endpoint.Targets{"target.foo.bar."}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "false", + }, + }, + }, + expectedAaaa: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeAAAA, + RecordTTL: defaultTTL, + Targets: endpoint.Targets{"target.foo.bar."}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "false", + }, + }, + }, + }, + { + name: "CNAME without alias to ELB target should enable alias and create AAAA", + ep: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeCNAME, + Targets: endpoint.Targets{"test-123.us-east-1.elb.amazonaws.com"}, + }, + expected: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeA, + Targets: endpoint.Targets{"test-123.us-east-1.elb.amazonaws.com"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "false", + }, + }, + }, + expectedAaaa: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeAAAA, + Targets: endpoint.Targets{"test-123.us-east-1.elb.amazonaws.com"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "false", + }, + }, + }, + }, + { + name: "CNAME with preferCNAME=true should set alias=false and not create AAAA even for ELB target", + preferCNAME: true, + ep: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeCNAME, + Targets: endpoint.Targets{"test-123.us-east-1.elb.amazonaws.com."}, + }, + expected: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeCNAME, + Targets: endpoint.Targets{"test-123.us-east-1.elb.amazonaws.com."}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "false", + }, + }, + }, + expectedAaaa: nil, + }, + + // --- MX / other records --- + { + name: "MX record without provider specific should not change and not create AAAA", + ep: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeMX, + Targets: endpoint.Targets{"10 mail.example.com."}, + }, + expected: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeMX, + Targets: endpoint.Targets{"10 mail.example.com."}, + }, + expectedAaaa: nil, + }, + { + name: "MX record with alias-related properties should drop them, keep ttl and not create AAAA", + ep: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeMX, + Targets: endpoint.Targets{"10 mail.example.com."}, + RecordTTL: 600, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "true", + }, + }, + }, + expected: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeMX, + Targets: endpoint.Targets{"10 mail.example.com."}, + RecordTTL: 600, + ProviderSpecific: endpoint.ProviderSpecific{}, + }, + expectedAaaa: nil, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + p, _ := newAWSProvider( + t, + endpoint.NewDomainFilter([]string{"foo.bar."}), + provider.NewZoneIDFilter([]string{}), + provider.NewZoneTypeFilter(""), + false, + tt.preferCNAME, + false, + nil, + ) + + aaaa := p.adjustEndpointAndNewAaaaIfNeeded(tt.ep) + + assert.True(t, testutils.SameEndpoint(tt.ep, tt.expected), + "actual and expected endpoints don't match. %+v:%+v", tt.ep, tt.expected) + + assert.True(t, testutils.SameEndpoint(aaaa, tt.expectedAaaa), + "actual and expected AAAA endpoints don't match. %+v:%+v", aaaa, tt.expectedAaaa) + }) } } From 1ff3f5d2f95acc4e8cbc931ede31ef8dbf770a04 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Wed, 28 Jan 2026 21:58:22 +0900 Subject: [PATCH 5/9] fix: adjustOtherRecord Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- provider/aws/aws.go | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/provider/aws/aws.go b/provider/aws/aws.go index a6ea12422a..7535f9858a 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -899,15 +899,9 @@ func (p *AWSProvider) adjustCNAMERecord(ep *endpoint.Endpoint) { } func (p *AWSProvider) adjustOtherRecord(ep *endpoint.Endpoint) { - // TODO: fix For records other than A, AAAA, and CNAME, if an alias record is set, the alias record processing is not performed. - // This will be fixed in another PR. - if isAlias, _ := ep.GetBoolProviderSpecificProperty(providerSpecificAlias); isAlias { - p.adjustAliasRecord(ep) - ep.DeleteProviderSpecificProperty(providerSpecificAlias) - } else { - ep.DeleteProviderSpecificProperty(providerSpecificAlias) - ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) - } + // if not A, AAAA, CNAME, ensure alias properties are removed + ep.DeleteProviderSpecificProperty(providerSpecificAlias) + ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) } // if the endpoint is using geoproximity, set the bias to 0 if not set From 9bb249bba4125c71852551dff51e6469de794fd5 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Sun, 1 Feb 2026 13:58:41 +0900 Subject: [PATCH 6/9] fix(aws): support alias records for non-A/AAAA/CNAME types except NS and SOA Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- provider/aws/aws.go | 18 +++++++++-- provider/aws/aws_test.go | 65 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 79 insertions(+), 4 deletions(-) diff --git a/provider/aws/aws.go b/provider/aws/aws.go index 7535f9858a..2a5687d8d4 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -898,10 +898,22 @@ func (p *AWSProvider) adjustCNAMERecord(ep *endpoint.Endpoint) { } } +// See: https://docs.aws.amazon.com/Route53/latest/DeveloperGuide/resource-record-sets-values-alias.html func (p *AWSProvider) adjustOtherRecord(ep *endpoint.Endpoint) { - // if not A, AAAA, CNAME, ensure alias properties are removed - ep.DeleteProviderSpecificProperty(providerSpecificAlias) - ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) + // NS and SOA records do not support alias + if ep.RecordType == endpoint.RecordTypeNS || ep.RecordType == "SOA" { + ep.DeleteProviderSpecificProperty(providerSpecificAlias) + ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) + return + } + // All other record types support alias when pointing to another record in the same hosted zone + isAlias, _ := ep.GetBoolProviderSpecificProperty(providerSpecificAlias) + if isAlias { + p.adjustAliasRecord(ep) + } else { + ep.DeleteProviderSpecificProperty(providerSpecificAlias) + ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) + } } // if the endpoint is using geoproximity, set the bias to 0 if not set diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index 3d1c4fb468..006c5e5439 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -3324,7 +3324,7 @@ func TestAWSProvider_adjustEndpointAndNewAaaaIfNeeded(t *testing.T) { expectedAaaa: nil, }, { - name: "MX record with alias-related properties should drop them, keep ttl and not create AAAA", + name: "MX record with alias=true should be treated as alias and not create AAAA", ep: &endpoint.Endpoint{ DNSName: "test.foo.bar.", RecordType: endpoint.RecordTypeMX, @@ -3341,6 +3341,42 @@ func TestAWSProvider_adjustEndpointAndNewAaaaIfNeeded(t *testing.T) { }, }, }, + expected: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeMX, + Targets: endpoint.Targets{"10 mail.example.com."}, + RecordTTL: 300, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "true", + }, + }, + }, + expectedAaaa: nil, + }, + { + name: "MX record with alias=false should drop alias properties and not create AAAA", + ep: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeMX, + Targets: endpoint.Targets{"10 mail.example.com."}, + RecordTTL: 600, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "false", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "true", + }, + }, + }, expected: &endpoint.Endpoint{ DNSName: "test.foo.bar.", RecordType: endpoint.RecordTypeMX, @@ -3350,6 +3386,33 @@ func TestAWSProvider_adjustEndpointAndNewAaaaIfNeeded(t *testing.T) { }, expectedAaaa: nil, }, + + // --- NS records --- + { + name: "NS record with alias=true should drop alias properties since NS does not support alias", + ep: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeNS, + Targets: endpoint.Targets{"ns1.example.com."}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: providerSpecificAlias, + Value: "true", + }, + { + Name: providerSpecificEvaluateTargetHealth, + Value: "true", + }, + }, + }, + expected: &endpoint.Endpoint{ + DNSName: "test.foo.bar.", + RecordType: endpoint.RecordTypeNS, + Targets: endpoint.Targets{"ns1.example.com."}, + ProviderSpecific: endpoint.ProviderSpecific{}, + }, + expectedAaaa: nil, + }, } for _, tt := range tests { From ef7b98a9ca8ac91ed5177c93799a44c0ffb9ac56 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Sat, 7 Feb 2026 15:02:31 +0900 Subject: [PATCH 7/9] refactor(aws): simplify adjustOtherRecord with clarifying comment Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- provider/aws/aws.go | 21 ++++--------- provider/aws/aws_test.go | 65 +--------------------------------------- 2 files changed, 7 insertions(+), 79 deletions(-) diff --git a/provider/aws/aws.go b/provider/aws/aws.go index 2a5687d8d4..ffca678bce 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -898,22 +898,13 @@ func (p *AWSProvider) adjustCNAMERecord(ep *endpoint.Endpoint) { } } -// See: https://docs.aws.amazon.com/Route53/latest/DeveloperGuide/resource-record-sets-values-alias.html func (p *AWSProvider) adjustOtherRecord(ep *endpoint.Endpoint) { - // NS and SOA records do not support alias - if ep.RecordType == endpoint.RecordTypeNS || ep.RecordType == "SOA" { - ep.DeleteProviderSpecificProperty(providerSpecificAlias) - ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) - return - } - // All other record types support alias when pointing to another record in the same hosted zone - isAlias, _ := ep.GetBoolProviderSpecificProperty(providerSpecificAlias) - if isAlias { - p.adjustAliasRecord(ep) - } else { - ep.DeleteProviderSpecificProperty(providerSpecificAlias) - ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) - } + // Although Route53 supports alias records for all types except NS and SOA + // (see: https://docs.aws.amazon.com/Route53/latest/DeveloperGuide/resource-record-sets-values-alias.html), + // ExternalDNS only uses alias for A/AAAA records converted from CNAME. + // For other record types, we simply remove the alias-related properties. + ep.DeleteProviderSpecificProperty(providerSpecificAlias) + ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) } // if the endpoint is using geoproximity, set the bias to 0 if not set diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index 006c5e5439..3d1c4fb468 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -3324,7 +3324,7 @@ func TestAWSProvider_adjustEndpointAndNewAaaaIfNeeded(t *testing.T) { expectedAaaa: nil, }, { - name: "MX record with alias=true should be treated as alias and not create AAAA", + name: "MX record with alias-related properties should drop them, keep ttl and not create AAAA", ep: &endpoint.Endpoint{ DNSName: "test.foo.bar.", RecordType: endpoint.RecordTypeMX, @@ -3341,42 +3341,6 @@ func TestAWSProvider_adjustEndpointAndNewAaaaIfNeeded(t *testing.T) { }, }, }, - expected: &endpoint.Endpoint{ - DNSName: "test.foo.bar.", - RecordType: endpoint.RecordTypeMX, - Targets: endpoint.Targets{"10 mail.example.com."}, - RecordTTL: 300, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: providerSpecificAlias, - Value: "true", - }, - { - Name: providerSpecificEvaluateTargetHealth, - Value: "true", - }, - }, - }, - expectedAaaa: nil, - }, - { - name: "MX record with alias=false should drop alias properties and not create AAAA", - ep: &endpoint.Endpoint{ - DNSName: "test.foo.bar.", - RecordType: endpoint.RecordTypeMX, - Targets: endpoint.Targets{"10 mail.example.com."}, - RecordTTL: 600, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: providerSpecificAlias, - Value: "false", - }, - { - Name: providerSpecificEvaluateTargetHealth, - Value: "true", - }, - }, - }, expected: &endpoint.Endpoint{ DNSName: "test.foo.bar.", RecordType: endpoint.RecordTypeMX, @@ -3386,33 +3350,6 @@ func TestAWSProvider_adjustEndpointAndNewAaaaIfNeeded(t *testing.T) { }, expectedAaaa: nil, }, - - // --- NS records --- - { - name: "NS record with alias=true should drop alias properties since NS does not support alias", - ep: &endpoint.Endpoint{ - DNSName: "test.foo.bar.", - RecordType: endpoint.RecordTypeNS, - Targets: endpoint.Targets{"ns1.example.com."}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: providerSpecificAlias, - Value: "true", - }, - { - Name: providerSpecificEvaluateTargetHealth, - Value: "true", - }, - }, - }, - expected: &endpoint.Endpoint{ - DNSName: "test.foo.bar.", - RecordType: endpoint.RecordTypeNS, - Targets: endpoint.Targets{"ns1.example.com."}, - ProviderSpecific: endpoint.ProviderSpecific{}, - }, - expectedAaaa: nil, - }, } for _, tt := range tests { From b4b27fd428f1820975338dc0677dc8341e876dc5 Mon Sep 17 00:00:00 2001 From: u-kai <76635578+u-kai@users.noreply.github.com> Date: Sun, 1 Mar 2026 10:49:25 +0900 Subject: [PATCH 8/9] chore(aws): remove unnecessary adjustRecord method and its tests Signed-off-by: u-kai <76635578+u-kai@users.noreply.github.com> --- provider/aws/aws.go | 11 ----------- provider/aws/aws_test.go | 29 ++--------------------------- 2 files changed, 2 insertions(+), 38 deletions(-) diff --git a/provider/aws/aws.go b/provider/aws/aws.go index ffca678bce..adcd997dbd 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -840,8 +840,6 @@ func (p *AWSProvider) adjustEndpointAndNewAaaaIfNeeded(ep *endpoint.Endpoint) *e aaaa.RecordType = endpoint.RecordTypeAAAA } return aaaa - default: - p.adjustOtherRecord(ep) } adjustGeoProximityLocationEndpoint(ep) return aaaa @@ -898,15 +896,6 @@ func (p *AWSProvider) adjustCNAMERecord(ep *endpoint.Endpoint) { } } -func (p *AWSProvider) adjustOtherRecord(ep *endpoint.Endpoint) { - // Although Route53 supports alias records for all types except NS and SOA - // (see: https://docs.aws.amazon.com/Route53/latest/DeveloperGuide/resource-record-sets-values-alias.html), - // ExternalDNS only uses alias for A/AAAA records converted from CNAME. - // For other record types, we simply remove the alias-related properties. - ep.DeleteProviderSpecificProperty(providerSpecificAlias) - ep.DeleteProviderSpecificProperty(providerSpecificEvaluateTargetHealth) -} - // if the endpoint is using geoproximity, set the bias to 0 if not set // this is needed to avoid unnecessary Upserts if the desired endpoint doesn't specify a bias func adjustGeoProximityLocationEndpoint(ep *endpoint.Endpoint) { diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index 48b908bb12..8383b03c11 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -3323,33 +3323,8 @@ func TestAWSProvider_adjustEndpointAndNewAaaaIfNeeded(t *testing.T) { }, expectedAaaa: nil, }, - { - name: "MX record with alias-related properties should drop them, keep ttl and not create AAAA", - ep: &endpoint.Endpoint{ - DNSName: "test.foo.bar.", - RecordType: endpoint.RecordTypeMX, - Targets: endpoint.Targets{"10 mail.example.com."}, - RecordTTL: 600, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: providerSpecificAlias, - Value: "true", - }, - { - Name: providerSpecificEvaluateTargetHealth, - Value: "true", - }, - }, - }, - expected: &endpoint.Endpoint{ - DNSName: "test.foo.bar.", - RecordType: endpoint.RecordTypeMX, - Targets: endpoint.Targets{"10 mail.example.com."}, - RecordTTL: 600, - ProviderSpecific: endpoint.ProviderSpecific{}, - }, - expectedAaaa: nil, - }, + // Other record types that has alias properites should be rejected by endpoint validation, + // so we don't need to test them here as adjustEndpointAndNewAaaaIfNeeded should not be called for them. } for _, tt := range tests { From ba3279df127e9394c6b928522e285f59fd04b6e8 Mon Sep 17 00:00:00 2001 From: Kai Udo <76635578+u-kai@users.noreply.github.com> Date: Fri, 3 Apr 2026 07:31:33 +0900 Subject: [PATCH 9/9] fix typo provider/aws/aws_test.go Co-authored-by: Ivan Ka <5395690+ivankatliarchuk@users.noreply.github.com> --- provider/aws/aws_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/provider/aws/aws_test.go b/provider/aws/aws_test.go index 8383b03c11..1f36028432 100644 --- a/provider/aws/aws_test.go +++ b/provider/aws/aws_test.go @@ -3323,7 +3323,7 @@ func TestAWSProvider_adjustEndpointAndNewAaaaIfNeeded(t *testing.T) { }, expectedAaaa: nil, }, - // Other record types that has alias properites should be rejected by endpoint validation, + // Other record types that has alias properties should be rejected by endpoint validation, // so we don't need to test them here as adjustEndpointAndNewAaaaIfNeeded should not be called for them. }