From e9ec3acdaa1eca791102725a4d32f21fb11af656 Mon Sep 17 00:00:00 2001 From: ivan katliarchuk Date: Fri, 25 Jul 2025 13:22:05 +0100 Subject: [PATCH] fix(idna): fix handling of domains Signed-off-by: ivan katliarchuk --- endpoint/domain_filter.go | 7 ++-- internal/idna/idna.go | 29 ++++++++++++++ internal/idna/idna_test.go | 59 +++++++++++++++++++++++++++ plan/plan.go | 10 +---- provider/aws/aws.go | 80 +++++++++++++++++++------------------ provider/zonefinder_test.go | 16 +++++--- 6 files changed, 146 insertions(+), 55 deletions(-) create mode 100644 internal/idna/idna.go create mode 100644 internal/idna/idna_test.go diff --git a/endpoint/domain_filter.go b/endpoint/domain_filter.go index 47402bb76..8d8aad2dc 100644 --- a/endpoint/domain_filter.go +++ b/endpoint/domain_filter.go @@ -25,7 +25,8 @@ import ( "strings" log "github.com/sirupsen/logrus" - "golang.org/x/net/idna" + + "sigs.k8s.io/external-dns/internal/idna" ) type MatchAllDomainFilters []DomainFilterInterface @@ -247,9 +248,9 @@ func (df *DomainFilter) MatchParent(domain string) bool { } // normalizeDomain converts a domain to a canonical form, so that we can filter on it -// it: trim "." suffix, get Unicode version of domain complient with Section 5 of RFC 5891 +// it: trim "." suffix, get Unicode version of domain compliant with Section 5 of RFC 5891 func normalizeDomain(domain string) string { - s, err := idna.Lookup.ToUnicode(strings.TrimSuffix(domain, ".")) + s, err := idna.Profile.ToUnicode(strings.TrimSuffix(domain, ".")) if err != nil { log.Warnf(`Got error while parsing domain %s: %v`, domain, err) } diff --git a/internal/idna/idna.go b/internal/idna/idna.go new file mode 100644 index 000000000..9290e8a51 --- /dev/null +++ b/internal/idna/idna.go @@ -0,0 +1,29 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package idna + +import ( + "golang.org/x/net/idna" +) + +var ( + Profile = idna.New( + idna.MapForLookup(), + idna.Transitional(true), + idna.StrictDomainName(false), + ) +) diff --git a/internal/idna/idna_test.go b/internal/idna/idna_test.go new file mode 100644 index 000000000..f3ae93c44 --- /dev/null +++ b/internal/idna/idna_test.go @@ -0,0 +1,59 @@ +/* +Copyright 2025 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package idna + +import ( + "strings" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestProfileWithDefault(t *testing.T) { + tets := []struct { + input string + expected string + }{ + { + input: "*.GÖPHER.com", + expected: "*.göpher.com", + }, + { + input: "*._abrakadabra.com", + expected: "*._abrakadabra.com", + }, + { + input: "_abrakadabra.com", + expected: "_abrakadabra.com", + }, + { + input: "*.foo.kube.example.com", + expected: "*.foo.kube.example.com", + }, + { + input: "xn--bcher-kva.example.com", + expected: "bücher.example.com", + }, + } + for _, tt := range tets { + t.Run(strings.ToLower(tt.input), func(t *testing.T) { + result, err := Profile.ToUnicode(tt.input) + assert.NoError(t, err) + assert.Equal(t, tt.expected, result) + }) + } +} diff --git a/plan/plan.go b/plan/plan.go index 8061a5b3f..cf7db8858 100644 --- a/plan/plan.go +++ b/plan/plan.go @@ -23,9 +23,9 @@ import ( "github.com/google/go-cmp/cmp" log "github.com/sirupsen/logrus" - "golang.org/x/net/idna" "sigs.k8s.io/external-dns/endpoint" + "sigs.k8s.io/external-dns/internal/idna" ) // PropertyComparator is used in Plan for comparing the previous and current custom annotations. @@ -340,16 +340,10 @@ func filterRecordsForPlan(records []*endpoint.Endpoint, domainFilter endpoint.Ma return filtered } -var idnaProfile = idna.New( - idna.MapForLookup(), - idna.Transitional(true), - idna.StrictDomainName(false), -) - // normalizeDNSName converts a DNS name to a canonical form, so that we can use string equality // it: removes space, get ASCII version of dnsName complient with Section 5 of RFC 5891, ensures there is a trailing dot func normalizeDNSName(dnsName string) string { - s, err := idnaProfile.ToASCII(strings.TrimSpace(dnsName)) + s, err := idna.Profile.ToASCII(strings.TrimSpace(dnsName)) if err != nil { log.Warnf(`Got error while parsing DNSName %s: %v`, dnsName, err) } diff --git a/provider/aws/aws.go b/provider/aws/aws.go index 1ddb35cce..c394d2bfd 100644 --- a/provider/aws/aws.go +++ b/provider/aws/aws.go @@ -734,56 +734,58 @@ func (p *AWSProvider) submitChanges(ctx context.Context, changes Route53Changes, log.Infof("Desired change: %s %s %s", c.Action, *c.ResourceRecordSet.Name, c.ResourceRecordSet.Type) } - if !p.dryRun { - params := &route53.ChangeResourceRecordSetsInput{ - HostedZoneId: aws.String(z), - ChangeBatch: &route53types.ChangeBatch{ - Changes: b.Route53Changes(), - }, - } + if p.dryRun { + continue + } - successfulChanges := 0 + params := &route53.ChangeResourceRecordSetsInput{ + HostedZoneId: aws.String(z), + ChangeBatch: &route53types.ChangeBatch{ + Changes: b.Route53Changes(), + }, + } - client := p.clients[zones[z].profile] - if _, err := client.ChangeResourceRecordSets(ctx, params); err != nil { - log.Errorf("Failure in zone %s when submitting change batch: %v", *zones[z].zone.Name, err) + successfulChanges := 0 - changesByOwnership := groupChangesByNameAndOwnershipRelation(b) + client := p.clients[zones[z].profile] + if _, err := client.ChangeResourceRecordSets(ctx, params); err != nil { + log.Errorf("Failure in zone %s when submitting change batch: %v", *zones[z].zone.Name, err) - if len(changesByOwnership) > 1 { - log.Debug("Trying to submit change sets one-by-one instead") - for _, changes := range changesByOwnership { - if log.Logger.IsLevelEnabled(debugLevel) { - for _, c := range changes { - log.Debugf("Desired change: %s %s %s", c.Action, *c.ResourceRecordSet.Name, c.ResourceRecordSet.Type) - } - } - params.ChangeBatch = &route53types.ChangeBatch{ - Changes: changes.Route53Changes(), - } - if _, err := client.ChangeResourceRecordSets(ctx, params); err != nil { - failedUpdate = true - log.Errorf("Failed submitting change (error: %v), it will be retried in a separate change batch in the next iteration", err) - p.failedChangesQueue[z] = append(p.failedChangesQueue[z], changes...) - } else { - successfulChanges = successfulChanges + len(changes) + changesByOwnership := groupChangesByNameAndOwnershipRelation(b) + + if len(changesByOwnership) > 1 { + log.Debug("Trying to submit change sets one-by-one instead") + for _, changes := range changesByOwnership { + if log.Logger.IsLevelEnabled(debugLevel) { + for _, c := range changes { + log.Debugf("Desired change: %s %s %s", c.Action, *c.ResourceRecordSet.Name, c.ResourceRecordSet.Type) } } - } else { - failedUpdate = true + params.ChangeBatch = &route53types.ChangeBatch{ + Changes: changes.Route53Changes(), + } + if _, err := client.ChangeResourceRecordSets(ctx, params); err != nil { + failedUpdate = true + log.Errorf("Failed submitting change (error: %v), it will be retried in a separate change batch in the next iteration", err) + p.failedChangesQueue[z] = append(p.failedChangesQueue[z], changes...) + } else { + successfulChanges = successfulChanges + len(changes) + } } } else { - successfulChanges = len(b) + failedUpdate = true } + } else { + successfulChanges = len(b) + } - if successfulChanges > 0 { - // z is the R53 Hosted Zone ID already as aws.StringValue - log.Infof("%d record(s) were successfully updated", successfulChanges) - } + if successfulChanges > 0 { + // z is the R53 Hosted Zone ID already as aws.StringValue + log.Infof("%d record(s) were successfully updated", successfulChanges) + } - if i != len(batchCs)-1 { - time.Sleep(p.batchChangeInterval) - } + if i != len(batchCs)-1 { + time.Sleep(p.batchChangeInterval) } } diff --git a/provider/zonefinder_test.go b/provider/zonefinder_test.go index dccfe5adb..4eddd36cb 100644 --- a/provider/zonefinder_test.go +++ b/provider/zonefinder_test.go @@ -32,14 +32,16 @@ func TestZoneIDName(t *testing.T) { z.Add("654321", "foo.qux.baz") z.Add("987654", "エイミー.みんな") z.Add("123123", "_metadata.example.com") + z.Add("1231231", "_foo._metadata.example.com") z.Add("456456", "_metadata.エイミー.みんな") assert.Equal(t, ZoneIDName{ - "123456": "qux.baz", - "654321": "foo.qux.baz", - "987654": "エイミー.みんな", - "123123": "_metadata.example.com", - "456456": "_metadata.エイミー.みんな", + "123456": "qux.baz", + "654321": "foo.qux.baz", + "987654": "エイミー.みんな", + "123123": "_metadata.example.com", + "1231231": "_foo._metadata.example.com", + "456456": "_metadata.エイミー.みんな", }, z) // simple entry in a domain @@ -77,6 +79,10 @@ func TestZoneIDName(t *testing.T) { assert.Equal(t, "エイミー.みんな", zoneName) assert.Equal(t, "987654", zoneID) + zoneID, zoneName = z.FindZone("_foo._metadata.example.com") + assert.Equal(t, "_foo._metadata.example.com", zoneName) + assert.Equal(t, "1231231", zoneID) + hook := testutils.LogsUnderTestWithLogLevel(log.WarnLevel, t) _, _ = z.FindZone("???")