fix: provider/FindZone: transform zone name to unicode as well

Signed-off-by: Romain Beuque <[email protected]>

Author: rbeuque74

provider/ovh/ovh.go

@@ -33,6 +33,7 @@ import (
 	"golang.org/x/sync/errgroup"
 
 	"sigs.k8s.io/external-dns/endpoint"
+	"sigs.k8s.io/external-dns/internal/idna"
 	"sigs.k8s.io/external-dns/pkg/apis/externaldns"
 	"sigs.k8s.io/external-dns/plan"
 	"sigs.k8s.io/external-dns/provider"
@@ -156,43 +157,55 @@ func (p *OVHProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, error)
 	return endpoints, nil
 }
 
-func planChangesByZoneName(zones []string, changes *plan.Changes) map[string]*plan.Changes {
+func planChangesByZoneName(zones []string, changes *plan.Changes) (map[string]*plan.Changes, error) {
 	zoneNameIDMapper := provider.ZoneIDName{}
 	for _, zone := range zones {
 		zoneNameIDMapper.Add(zone, zone)
 	}
 
 	output := map[string]*plan.Changes{}
 	for _, endpt := range changes.Delete {
-		_, zoneName := zoneNameIDMapper.FindZone(endpt.DNSName)
+		zoneName, _ := zoneNameIDMapper.FindZone(endpt.DNSName)
+		if zoneName == "" {
+			return nil, provider.NewSoftError(fmt.Errorf("record %q have not found matching DNS zone in OVH provider", endpt.DNSName))
+		}
 		if _, ok := output[zoneName]; !ok {
 			output[zoneName] = &plan.Changes{}
 		}
 		output[zoneName].Delete = append(output[zoneName].Delete, endpt)
 	}
 	for _, endpt := range changes.Create {
-		_, zoneName := zoneNameIDMapper.FindZone(endpt.DNSName)
+		zoneName, _ := zoneNameIDMapper.FindZone(endpt.DNSName)
+		if zoneName == "" {
+			return nil, provider.NewSoftError(fmt.Errorf("record %q have not found matching DNS zone in OVH provider", endpt.DNSName))
+		}
 		if _, ok := output[zoneName]; !ok {
 			output[zoneName] = &plan.Changes{}
 		}
 		output[zoneName].Create = append(output[zoneName].Create, endpt)
 	}
 	for _, endpt := range changes.UpdateOld {
-		_, zoneName := zoneNameIDMapper.FindZone(endpt.DNSName)
+		zoneName, _ := zoneNameIDMapper.FindZone(endpt.DNSName)
+		if zoneName == "" {
+			return nil, provider.NewSoftError(fmt.Errorf("record %q have not found matching DNS zone in OVH provider", endpt.DNSName))
+		}
 		if _, ok := output[zoneName]; !ok {
 			output[zoneName] = &plan.Changes{}
 		}
 		output[zoneName].UpdateOld = append(output[zoneName].UpdateOld, endpt)
 	}
 	for _, endpt := range changes.UpdateNew {
-		_, zoneName := zoneNameIDMapper.FindZone(endpt.DNSName)
+		zoneName, _ := zoneNameIDMapper.FindZone(endpt.DNSName)
+		if zoneName == "" {
+			return nil, provider.NewSoftError(fmt.Errorf("record %q have not found matching DNS zone in OVH provider", endpt.DNSName))
+		}
 		if _, ok := output[zoneName]; !ok {
 			output[zoneName] = &plan.Changes{}
 		}
 		output[zoneName].UpdateNew = append(output[zoneName].UpdateNew, endpt)
 	}
 
-	return output
+	return output, nil
 }
 
 func (p *OVHProvider) computeSingleZoneChanges(_ context.Context, zoneName string, existingRecords []ovhRecord, changes *plan.Changes) ([]ovhChange, error) {
@@ -264,7 +277,10 @@ func (p *OVHProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) e
 		}
 	}
 
-	changesByZoneName := planChangesByZoneName(zones, changes)
+	changesByZoneName, err := planChangesByZoneName(zones, changes)
+	if err != nil {
+		return err
+	}
 	eg, ctx := errgroup.WithContext(ctx)
 
 	for zoneName, changes := range changesByZoneName {
@@ -553,6 +569,13 @@ func (p *OVHProvider) newOvhChangeCreateDelete(action int, endpoints []*endpoint
 }
 
 func convertDNSNameIntoSubDomain(DNSName string, zoneName string) string { // nolint: gocritic // captLocal
+	if name, err := idna.Profile.ToUnicode(DNSName); err == nil {
+		DNSName = name
+	}
+	if name, err := idna.Profile.ToUnicode(zoneName); err == nil {
+		zoneName = name
+	}
+
 	if DNSName == zoneName {
 		return ""
 	}

provider/ovh/ovh_test.go

@@ -585,6 +585,27 @@ func TestOvhApplyChanges(t *testing.T) {
 	client.AssertExpectations(t)
 }
 
+func TestOvhApplyChangesPunyCode(t *testing.T) {
+	client := new(mockOvhClient)
+	provider := &OVHProvider{client: client, apiRateLimiter: ratelimit.New(10), cacheInstance: cache.New(cache.NoExpiration, cache.NoExpiration)}
+	changes := plan.Changes{
+		Create: []*endpoint.Endpoint{
+			{DNSName: "example.testécassé.fr", RecordType: "A", RecordTTL: 10, Targets: []string{"203.0.113.42"}},
+		},
+	}
+
+	client.On("GetWithContext", "/domain/zone").Return([]string{"xn--testcass-e1ae.fr"}, nil).Once()
+	client.On("GetWithContext", "/domain/zone/xn--testcass-e1ae.fr/record").Return([]uint64{}, nil).Once()
+	client.On("PostWithContext", "/domain/zone/xn--testcass-e1ae.fr/record", ovhRecordFields{FieldType: "A", ovhRecordFieldUpdate: ovhRecordFieldUpdate{SubDomain: "example", TTL: 10, Target: "203.0.113.42"}}).Return(nil, nil).Once()
+	client.On("PostWithContext", "/domain/zone/xn--testcass-e1ae.fr/refresh", nil).Return(nil, nil).Once()
+
+	_, err := provider.Records(t.Context())
+	td.CmpNoError(t, err)
+	// Basic changes
+	td.CmpNoError(t, provider.ApplyChanges(t.Context(), &changes))
+	client.AssertExpectations(t)
+}
+
 func TestOvhChange(t *testing.T) {
 	assert := assert.New(t)
 	client := new(mockOvhClient)

provider/zonefinder.go

@@ -27,7 +27,11 @@ import (
 type ZoneIDName map[string]string
 
 func (z ZoneIDName) Add(zoneID, zoneName string) {
-	z[zoneID] = zoneName
+	if zone, err := idna.Profile.ToUnicode(zoneName); err == nil {
+		z[zoneID] = zone
+	} else {
+		z[zoneID] = zoneName
+	}
 }
 
 // FindZone identifies the most suitable DNS zone for a given hostname.

provider/zonefinder_test.go

@@ -35,6 +35,8 @@ func TestZoneIDName(t *testing.T) {
 	z.Add("1231231", "_foo._metadata.example.com")
 	z.Add("456456", "_metadata.エイミー.みんな")
 	z.Add("123412", "*.example.com")
+	// adding a zone as punycode, see that it is injected as unicode/international format
+	z.Add("234567", "xn--testcass-e1ae.fr")
 
 	assert.Equal(t, ZoneIDName{
 		"123456":  "qux.baz",
@@ -44,6 +46,7 @@ func TestZoneIDName(t *testing.T) {
 		"1231231": "_foo._metadata.example.com",
 		"456456":  "_metadata.エイミー.みんな",
 		"123412":  "*.example.com",
+		"234567":  "testécassé.fr",
 	}, z)
 
 	// simple entry in a domain
@@ -89,6 +92,15 @@ func TestZoneIDName(t *testing.T) {
 	assert.Equal(t, "*.example.com", zoneName)
 	assert.Equal(t, "123412", zoneID)
 
+	// looking for a zone that has been inserted as punycode
+	zoneID, zoneName = z.FindZone("example.testécassé.fr")
+	assert.Equal(t, "testécassé.fr", zoneName)
+	assert.Equal(t, "234567", zoneID)
+
+	zoneID, zoneName = z.FindZone("example.xn--testcass-e1ae.fr")
+	assert.Equal(t, "testécassé.fr", zoneName)
+	assert.Equal(t, "234567", zoneID)
+
 	hook := testutils.LogsUnderTestWithLogLevel(log.WarnLevel, t)
 	_, _ = z.FindZone("xn--not-a-valid-punycode")