Skip to content

Commit 61ca17c

Browse files
vflauxvflaux
authored
chore(cloudflare): migrate ListRecords() to new lib (#5778)
* chore(cloudflare): migrate ListRecords() to new lib * chore(cloudflare): test zoneService.ListDNSRecord()
1 parent e88b94b commit 61ca17c

File tree

2 files changed

+46
-67
lines changed

2 files changed

+46
-67
lines changed

‎provider/cloudflare/cloudflare.go‎

Lines changed: 16 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ type cloudFlareDNS interface {
110110
ZoneIDByName(zoneName string) (string, error)
111111
ListZones(ctx context.Context, params zones.ZoneListParams) autoPager[zones.Zone]
112112
GetZone(ctx context.Context, zoneID string) (*zones.Zone, error)
113-
ListDNSRecords(ctx context.Context, rc *cloudflarev0.ResourceContainer, rp cloudflarev0.ListDNSRecordsParams) ([]dns.RecordResponse, *cloudflarev0.ResultInfo, error)
113+
ListDNSRecords(ctx context.Context, params dns.RecordListParams) autoPager[dns.RecordResponse]
114114
CreateDNSRecord(ctx context.Context, params dns.RecordNewParams) (*dns.RecordResponse, error)
115115
DeleteDNSRecord(ctx context.Context, rc *cloudflarev0.ResourceContainer, recordID string) error
116116
UpdateDNSRecord(ctx context.Context, rc *cloudflarev0.ResourceContainer, rp cloudflarev0.UpdateDNSRecordParams) error
@@ -152,13 +152,8 @@ func (z zoneService) CreateDNSRecord(ctx context.Context, params dns.RecordNewPa
152152
return z.service.DNS.Records.New(ctx, params)
153153
}
154154

155-
func (z zoneService) ListDNSRecords(ctx context.Context, rc *cloudflarev0.ResourceContainer, rp cloudflarev0.ListDNSRecordsParams) ([]dns.RecordResponse, *cloudflarev0.ResultInfo, error) {
156-
records, info, err := z.serviceV0.ListDNSRecords(ctx, rc, rp)
157-
convertedRecords := make([]dns.RecordResponse, 0, len(records))
158-
for _, record := range records {
159-
convertedRecords = append(convertedRecords, dnsRecordResponseFromLegacyDNSRecord(record))
160-
}
161-
return convertedRecords, info, err
155+
func (z zoneService) ListDNSRecords(ctx context.Context, params dns.RecordListParams) autoPager[dns.RecordResponse] {
156+
return z.service.DNS.Records.ListAutoPaging(ctx, params)
162157
}
163158

164159
func (z zoneService) UpdateDNSRecord(ctx context.Context, rc *cloudflarev0.ResourceContainer, rp cloudflarev0.UpdateDNSRecordParams) error {
@@ -428,7 +423,7 @@ func (p *CloudFlareProvider) Records(ctx context.Context) ([]*endpoint.Endpoint,
428423

429424
var endpoints []*endpoint.Endpoint
430425
for _, zone := range zones {
431-
records, err := p.listDNSRecordsWithAutoPagination(ctx, zone.ID)
426+
records, err := p.getDNSRecordsMap(ctx, zone.ID)
432427
if err != nil {
433428
return nil, err
434429
}
@@ -643,7 +638,7 @@ func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloud
643638
continue
644639
}
645640

646-
records, err := p.listDNSRecordsWithAutoPagination(ctx, zoneID)
641+
records, err := p.getDNSRecordsMap(ctx, zoneID)
647642
if err != nil {
648643
return fmt.Errorf("could not fetch records from zone, %w", err)
649644
}
@@ -860,27 +855,19 @@ func newDNSRecordIndex(r dns.RecordResponse) DNSRecordIndex {
860855
return DNSRecordIndex{Name: r.Name, Type: string(r.Type), Content: r.Content}
861856
}
862857

863-
// listDNSRecordsWithAutoPagination performs automatic pagination of results on requests to cloudflare.ListDNSRecords with custom per_page values
864-
func (p *CloudFlareProvider) listDNSRecordsWithAutoPagination(ctx context.Context, zoneID string) (DNSRecordsMap, error) {
858+
// getDNSRecordsMap retrieves all DNS records for a given zone and returns them as a DNSRecordsMap.
859+
func (p *CloudFlareProvider) getDNSRecordsMap(ctx context.Context, zoneID string) (DNSRecordsMap, error) {
865860
// for faster getRecordID lookup
866-
records := make(DNSRecordsMap)
867-
resultInfo := cloudflarev0.ResultInfo{PerPage: p.DNSRecordsConfig.PerPage, Page: 1}
868-
params := cloudflarev0.ListDNSRecordsParams{ResultInfo: resultInfo}
869-
for {
870-
pageRecords, resultInfo, err := p.Client.ListDNSRecords(ctx, cloudflarev0.ZoneIdentifier(zoneID), params)
871-
if err != nil {
872-
return nil, convertCloudflareError(err)
873-
}
874-
875-
for _, r := range pageRecords {
876-
records[newDNSRecordIndex(r)] = r
877-
}
878-
params.ResultInfo = resultInfo.Next()
879-
if params.Done() {
880-
break
881-
}
861+
recordsMap := make(DNSRecordsMap)
862+
params := dns.RecordListParams{ZoneID: cloudflare.F(zoneID)}
863+
iter := p.Client.ListDNSRecords(ctx, params)
864+
for record := range autoPagerIterator(iter) {
865+
recordsMap[newDNSRecordIndex(record)] = record
866+
}
867+
if iter.Err() != nil {
868+
return nil, convertCloudflareError(iter.Err())
882869
}
883-
return records, nil
870+
return recordsMap, nil
884871
}
885872

886873
func newCustomHostnameIndex(ch cloudflarev0.CustomHostname) CustomHostnameIndex {

‎provider/cloudflare/cloudflare_test.go‎

Lines changed: 30 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -22,12 +22,12 @@ import (
2222
"fmt"
2323
"os"
2424
"slices"
25-
"sort"
2625
"strings"
2726
"testing"
2827
"time"
2928

3029
cloudflarev0 "github.com/cloudflare/cloudflare-go"
30+
"github.com/cloudflare/cloudflare-go/v5"
3131
"github.com/cloudflare/cloudflare-go/v5/dns"
3232
"github.com/cloudflare/cloudflare-go/v5/zones"
3333
"github.com/maxatome/go-testdeep/td"
@@ -171,49 +171,22 @@ func (m *mockCloudFlareClient) CreateDNSRecord(ctx context.Context, params dns.R
171171
return &record, nil
172172
}
173173

174-
func (m *mockCloudFlareClient) ListDNSRecords(ctx context.Context, rc *cloudflarev0.ResourceContainer, rp cloudflarev0.ListDNSRecordsParams) ([]dns.RecordResponse, *cloudflarev0.ResultInfo, error) {
174+
func (m *mockCloudFlareClient) ListDNSRecords(ctx context.Context, params dns.RecordListParams) autoPager[dns.RecordResponse] {
175175
if m.dnsRecordsError != nil {
176-
return nil, &cloudflarev0.ResultInfo{}, m.dnsRecordsError
176+
return &mockAutoPager[dns.RecordResponse]{err: m.dnsRecordsError}
177177
}
178-
result := []dns.RecordResponse{}
179-
if zone, ok := m.Records[rc.Identifier]; ok {
178+
iter := &mockAutoPager[dns.RecordResponse]{}
179+
if zone, ok := m.Records[params.ZoneID.Value]; ok {
180180
for _, record := range zone {
181181
if strings.HasPrefix(record.Name, "newerror-list-") {
182-
m.DeleteDNSRecord(ctx, rc, record.ID)
183-
return nil, &cloudflarev0.ResultInfo{}, errors.New("failed to list erroring DNS record")
182+
m.DeleteDNSRecord(ctx, cloudflarev0.ResourceIdentifier(params.ZoneID.Value), record.ID)
183+
iter.err = errors.New("failed to list erroring DNS record")
184+
return iter
184185
}
185-
result = append(result, record)
186+
iter.items = append(iter.items, record)
186187
}
187188
}
188-
189-
if len(result) == 0 || rp.PerPage == 0 {
190-
return result, &cloudflarev0.ResultInfo{Page: 1, TotalPages: 1, Count: 0, Total: 0}, nil
191-
}
192-
193-
// if not pagination options were passed in, return the result as is
194-
if rp.Page == 0 {
195-
return result, &cloudflarev0.ResultInfo{Page: 1, TotalPages: 1, Count: len(result), Total: len(result)}, nil
196-
}
197-
198-
// otherwise, split the result into chunks of size rp.PerPage to simulate the pagination from the API
199-
chunks := [][]dns.RecordResponse{}
200-
201-
// to ensure consistency in the multiple calls to this function, sort the result slice
202-
sort.Slice(result, func(i, j int) bool { return strings.Compare(result[i].ID, result[j].ID) > 0 })
203-
for rp.PerPage < len(result) {
204-
result, chunks = result[rp.PerPage:], append(chunks, result[0:rp.PerPage])
205-
}
206-
chunks = append(chunks, result)
207-
208-
// return the requested page
209-
partialResult := chunks[rp.Page-1]
210-
return partialResult, &cloudflarev0.ResultInfo{
211-
PerPage: rp.PerPage,
212-
Page: rp.Page,
213-
TotalPages: len(chunks),
214-
Count: len(partialResult),
215-
Total: len(result),
216-
}, nil
189+
return iter
217190
}
218191

219192
func (m *mockCloudFlareClient) UpdateDNSRecord(ctx context.Context, rc *cloudflarev0.ResourceContainer, rp cloudflarev0.UpdateDNSRecordParams) error {
@@ -1500,7 +1473,7 @@ func TestGroupByNameAndTypeWithCustomHostnames_MX(t *testing.T) {
15001473
}
15011474
ctx := context.Background()
15021475
chs := CustomHostnamesMap{}
1503-
records, err := provider.listDNSRecordsWithAutoPagination(ctx, "001")
1476+
records, err := provider.getDNSRecordsMap(ctx, "001")
15041477
assert.NoError(t, err)
15051478

15061479
endpoints := provider.groupByNameAndTypeWithCustomHostnames(records, chs)
@@ -3346,3 +3319,22 @@ func TestDnsRecordFromLegacyAPI(t *testing.T) {
33463319
})
33473320
}
33483321
}
3322+
3323+
func TestZoneService(t *testing.T) {
3324+
t.Parallel()
3325+
3326+
ctx, cancel := context.WithCancel(t.Context())
3327+
cancel()
3328+
3329+
client := &zoneService{
3330+
service: cloudflare.NewClient(),
3331+
}
3332+
3333+
t.Run("UpdateDNSRecord", func(t *testing.T) {
3334+
t.Parallel()
3335+
iter := client.ListDNSRecords(ctx, dns.RecordListParams{ZoneID: cloudflare.F("foo")})
3336+
require.False(t, iter.Next())
3337+
require.Empty(t, iter.Current())
3338+
require.ErrorIs(t, iter.Err(), context.Canceled)
3339+
})
3340+
}

0 commit comments

Comments
 (0)