diff --git a/provider/ovh/ovh.go b/provider/ovh/ovh.go index ac5d31b2c4..f78f16e40a 100644 --- a/provider/ovh/ovh.go +++ b/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,7 +157,7 @@ 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) @@ -164,35 +165,47 @@ func planChangesByZoneName(zones []string, changes *plan.Changes) map[string]*pl 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 "" } diff --git a/provider/ovh/ovh_test.go b/provider/ovh/ovh_test.go index 50fdd78a2a..b3f68a6e45 100644 --- a/provider/ovh/ovh_test.go +++ b/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) diff --git a/provider/zonefinder.go b/provider/zonefinder.go index a529e17e79..3683cbd22a 100644 --- a/provider/zonefinder.go +++ b/provider/zonefinder.go @@ -27,7 +27,12 @@ import ( type ZoneIDName map[string]string func (z ZoneIDName) Add(zoneID, zoneName string) { - z[zoneID] = zoneName + var err error + z[zoneID], err = idna.Profile.ToUnicode(zoneName) + if err != nil { + log.Warnf("failed to convert zonename %q to its Unicode form: %v", zoneName, err) + z[zoneID] = zoneName + } } // FindZone identifies the most suitable DNS zone for a given hostname. diff --git a/provider/zonefinder_test.go b/provider/zonefinder_test.go index b02a03880b..26ed4785f1 100644 --- a/provider/zonefinder_test.go +++ b/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")