Skip to content

Commit 234e225

Browse files
committed
fix: ovh: handling capitalized DNS records + prevent panic
Better handling of case aware endpoint by lower-casing all the sub-domain part. Fixes #5384 Signed-off-by: Romain Beuque <[email protected]>
1 parent df28bbe commit 234e225

File tree

2 files changed

+27
-12
lines changed

2 files changed

+27
-12
lines changed

provider/ovh/ovh.go

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -195,7 +195,7 @@ func planChangesByZoneName(zones []string, changes *plan.Changes) map[string]*pl
195195
return output
196196
}
197197

198-
func (p OVHProvider) computeSingleZoneChanges(_ context.Context, zoneName string, existingRecords []ovhRecord, changes *plan.Changes) []ovhChange {
198+
func (p OVHProvider) computeSingleZoneChanges(_ context.Context, zoneName string, existingRecords []ovhRecord, changes *plan.Changes) ([]ovhChange, error) {
199199
allChanges := []ovhChange{}
200200
var computedChanges []ovhChange
201201

@@ -204,14 +204,21 @@ func (p OVHProvider) computeSingleZoneChanges(_ context.Context, zoneName string
204204
computedChanges, existingRecords = p.newOvhChangeCreateDelete(ovhDelete, changes.Delete, zoneName, existingRecords)
205205
allChanges = append(allChanges, computedChanges...)
206206

207-
computedChanges = p.newOvhChangeUpdate(changes.UpdateOld, changes.UpdateNew, zoneName, existingRecords)
207+
var err error
208+
computedChanges, err = p.newOvhChangeUpdate(changes.UpdateOld, changes.UpdateNew, zoneName, existingRecords)
209+
if err != nil {
210+
return nil, err
211+
}
208212
allChanges = append(allChanges, computedChanges...)
209213

210-
return allChanges
214+
return allChanges, nil
211215
}
212216

213217
func (p *OVHProvider) handleSingleZoneUpdate(ctx context.Context, zoneName string, existingRecords []ovhRecord, changes *plan.Changes) error {
214-
allChanges := p.computeSingleZoneChanges(ctx, zoneName, existingRecords, changes)
218+
allChanges, err := p.computeSingleZoneChanges(ctx, zoneName, existingRecords, changes)
219+
if err != nil {
220+
return err
221+
}
215222
log.Infof("OVH: %q: %d changes will be done", zoneName, len(allChanges))
216223

217224
eg, ctxErrGroup := errgroup.WithContext(ctx)
@@ -222,7 +229,7 @@ func (p *OVHProvider) handleSingleZoneUpdate(ctx context.Context, zoneName strin
222229
})
223230
}
224231

225-
err := eg.Wait()
232+
err = eg.Wait()
226233

227234
// do not refresh zone if errors: some records might haven't been processed yet, hence the zone will be in an inconsistent state
228235
// if modification of the zone was in error, invalidating the cache to make sure next run will start freshly
@@ -554,7 +561,11 @@ func convertDNSNameIntoSubDomain(DNSName string, zoneName string) string {
554561
return strings.TrimSuffix(DNSName, "."+zoneName)
555562
}
556563

557-
func (p OVHProvider) newOvhChangeUpdate(endpointsOld []*endpoint.Endpoint, endpointsNew []*endpoint.Endpoint, zone string, existingRecords []ovhRecord) []ovhChange {
564+
func normalizeDNSName(dnsName string) string {
565+
return strings.TrimSpace(strings.ToLower(dnsName))
566+
}
567+
568+
func (p OVHProvider) newOvhChangeUpdate(endpointsOld []*endpoint.Endpoint, endpointsNew []*endpoint.Endpoint, zone string, existingRecords []ovhRecord) ([]ovhChange, error) {
558569
zoneNameIDMapper := provider.ZoneIDName{}
559570
zoneNameIDMapper.Add(zone, zone)
560571

@@ -564,16 +575,16 @@ func (p OVHProvider) newOvhChangeUpdate(endpointsOld []*endpoint.Endpoint, endpo
564575

565576
for _, e := range endpointsOld {
566577
sub := convertDNSNameIntoSubDomain(e.DNSName, zone)
567-
oldEndpointByTypeAndName[e.RecordType+"//"+sub] = e
578+
oldEndpointByTypeAndName[normalizeDNSName(e.RecordType+"//"+sub)] = e
568579
}
569580
for _, e := range endpointsNew {
570581
sub := convertDNSNameIntoSubDomain(e.DNSName, zone)
571-
newEndpointByTypeAndName[e.RecordType+"//"+sub] = e
582+
newEndpointByTypeAndName[normalizeDNSName(e.RecordType+"//"+sub)] = e
572583
}
573584

574585
for id := range oldEndpointByTypeAndName {
575586
for _, record := range existingRecords {
576-
if id == record.FieldType+"//"+record.SubDomain {
587+
if id == normalizeDNSName(record.FieldType+"//"+record.SubDomain) {
577588
oldRecordsInZone[id] = append(oldRecordsInZone[id], record)
578589
}
579590
}
@@ -583,7 +594,10 @@ func (p OVHProvider) newOvhChangeUpdate(endpointsOld []*endpoint.Endpoint, endpo
583594

584595
for id := range oldEndpointByTypeAndName {
585596
oldRecords := slices.Clone(oldRecordsInZone[id])
586-
endpointsNew := newEndpointByTypeAndName[id]
597+
endpointsNew, ok := newEndpointByTypeAndName[id]
598+
if !ok {
599+
return nil, errors.New("unrecoverable error: couldn't find the matching record in the update.New")
600+
}
587601

588602
var toInsertTarget []string
589603

@@ -668,7 +682,7 @@ func (p OVHProvider) newOvhChangeUpdate(endpointsOld []*endpoint.Endpoint, endpo
668682
}
669683
}
670684

671-
return changes
685+
return changes, nil
672686
}
673687

674688
func (c *ovhChange) String() string {

provider/ovh/ovh_test.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,8 @@ func TestOvhComputeChanges(t *testing.T) {
328328
}
329329

330330
provider := &OVHProvider{client: nil, apiRateLimiter: ratelimit.New(10), cacheInstance: cache.New(cache.NoExpiration, cache.NoExpiration)}
331-
ovhChanges := provider.computeSingleZoneChanges(t.Context(), "example.net", existingRecords, &changes)
331+
ovhChanges, err := provider.computeSingleZoneChanges(t.Context(), "example.net", existingRecords, &changes)
332+
td.CmpNoError(t, err)
332333
td.Cmp(t, ovhChanges, []ovhChange{
333334
{
334335
Action: ovhCreate,

0 commit comments

Comments
 (0)