Merge pull request #3364 from arturhoo/cloudflare-paginated-list-requests

cloudflare - customizable pagination when listing DNS records

docs/tutorials/cloudflare.md

@@ -25,6 +25,10 @@ When using API Token authentication, the token should be granted Zone `Read`, DN
 
 If you would like to further restrict the API permissions to a specific zone (or zones), you also need to use the `--zone-id-filter` so that the underlying API requests only access the zones that you explicitly specify, as opposed to accessing all zones.
 
+## Throttling
+
+Cloudflare API has a [global rate limit of 1,200 requests per five minutes](https://developers.cloudflare.com/fundamentals/api/reference/limits/). Running several fast polling ExternalDNS instances in a given account can easily hit that limit. The AWS Provider [docs](./aws.md#throttling) has some recommendations that can be followed here too, but in particular, consider passing `--cloudflare-dns-records-per-page` with a high value (maximum is 5,000).
+
 ## Deploy ExternalDNS
 
 Connect your `kubectl` client to the cluster you want to test ExternalDNS with.
@@ -57,6 +61,7 @@ spec:
         - --zone-id-filter=023e105f4ecef8ad9ca31a8372d0c353 # (optional) limit to a specific zone.
         - --provider=cloudflare
         - --cloudflare-proxied # (optional) enable the proxy feature of Cloudflare (DDOS protection, CDN...)
+        - --cloudflare-dns-records-per-page=5000 # (optional) configure how many DNS records to fetch per request
         env:
         - name: CF_API_KEY
           value: "YOUR_CLOUDFLARE_API_KEY"
@@ -125,6 +130,7 @@ spec:
         - --zone-id-filter=023e105f4ecef8ad9ca31a8372d0c353 # (optional) limit to a specific zone.
         - --provider=cloudflare
         - --cloudflare-proxied # (optional) enable the proxy feature of Cloudflare (DDOS protection, CDN...)
+        - --cloudflare-dns-records-per-page=5000 # (optional) configure how many DNS records to fetch per request
         env:
         - name: CF_API_KEY
           value: "YOUR_CLOUDFLARE_API_KEY"

main.go

@@ -233,7 +233,7 @@ func main() {
 	case "civo":
 		p, err = civo.NewCivoProvider(domainFilter, cfg.DryRun)
 	case "cloudflare":
-		p, err = cloudflare.NewCloudFlareProvider(domainFilter, zoneIDFilter, cfg.CloudflareZonesPerPage, cfg.CloudflareProxied, cfg.DryRun)
+		p, err = cloudflare.NewCloudFlareProvider(domainFilter, zoneIDFilter, cfg.CloudflareProxied, cfg.DryRun, cfg.CloudflareDNSRecordsPerPage)
 	case "rcodezero":
 		p, err = rcode0.NewRcodeZeroProvider(domainFilter, cfg.DryRun, cfg.RcodezeroTXTEncrypt)
 	case "google":

pkg/apis/externaldns/types.go

@@ -105,7 +105,7 @@ type Config struct {
 	BluecatDNSDeployType              string
 	BluecatSkipTLSVerify              bool
 	CloudflareProxied                 bool
-	CloudflareZonesPerPage            int
+	CloudflareDNSRecordsPerPage       int
 	CoreDNSPrefix                     string
 	RcodezeroTXTEncrypt               bool
 	AkamaiServiceConsumerDomain       string
@@ -254,7 +254,7 @@ var defaultConfig = &Config{
 	BluecatConfigFile:           "/etc/kubernetes/bluecat.json",
 	BluecatDNSDeployType:        "no-deploy",
 	CloudflareProxied:           false,
-	CloudflareZonesPerPage:      50,
+	CloudflareDNSRecordsPerPage: 100,
 	CoreDNSPrefix:               "/skydns/",
 	RcodezeroTXTEncrypt:         false,
 	AkamaiServiceConsumerDomain: "",
@@ -473,7 +473,7 @@ func (cfg *Config) ParseFlags(args []string) error {
 	app.Flag("bluecat-dns-deploy-type", "When using the Bluecat provider, specify the type of DNS deployment to initiate after records are updated. Valid options are 'full-deploy' and 'no-deploy'. Deploy will only execute if --bluecat-dns-server-name is set (optional when --provider=bluecat)").Default(defaultConfig.BluecatDNSDeployType).StringVar(&cfg.BluecatDNSDeployType)
 
 	app.Flag("cloudflare-proxied", "When using the Cloudflare provider, specify if the proxy mode must be enabled (default: disabled)").BoolVar(&cfg.CloudflareProxied)
-	app.Flag("cloudflare-zones-per-page", "When using the Cloudflare provider, specify how many zones per page listed, max. possible 50 (default: 50)").Default(strconv.Itoa(defaultConfig.CloudflareZonesPerPage)).IntVar(&cfg.CloudflareZonesPerPage)
+	app.Flag("cloudflare-dns-records-per-page", "When using the Cloudflare provider, specify how many DNS records listed per page, max possible 5,000 (default: 100)").Default(strconv.Itoa(defaultConfig.CloudflareDNSRecordsPerPage)).IntVar(&cfg.CloudflareDNSRecordsPerPage)
 	app.Flag("coredns-prefix", "When using the CoreDNS provider, specify the prefix name").Default(defaultConfig.CoreDNSPrefix).StringVar(&cfg.CoreDNSPrefix)
 	app.Flag("akamai-serviceconsumerdomain", "When using the Akamai provider, specify the base URL (required when --provider=akamai and edgerc-path not specified)").Default(defaultConfig.AkamaiServiceConsumerDomain).StringVar(&cfg.AkamaiServiceConsumerDomain)
 	app.Flag("akamai-client-token", "When using the Akamai provider, specify the client token (required when --provider=akamai and edgerc-path not specified)").Default(defaultConfig.AkamaiClientToken).StringVar(&cfg.AkamaiClientToken)

pkg/apis/externaldns/types_test.go

@@ -77,7 +77,7 @@ var (
 		BluecatDNSDeployType:        defaultConfig.BluecatDNSDeployType,
 		BluecatSkipTLSVerify:        false,
 		CloudflareProxied:           false,
-		CloudflareZonesPerPage:      50,
+		CloudflareDNSRecordsPerPage: 100,
 		CoreDNSPrefix:               "/skydns/",
 		AkamaiServiceConsumerDomain: "",
 		AkamaiClientToken:           "",
@@ -182,7 +182,7 @@ var (
 		BluecatDNSDeployType:        "full-deploy",
 		BluecatSkipTLSVerify:        true,
 		CloudflareProxied:           true,
-		CloudflareZonesPerPage:      20,
+		CloudflareDNSRecordsPerPage: 5000,
 		CoreDNSPrefix:               "/coredns/",
 		AkamaiServiceConsumerDomain: "oooo-xxxxxxxxxxxxxxxx-xxxxxxxxxxxxxxxx.luna.akamaiapis.net",
 		AkamaiClientToken:           "o184671d5307a388180fbf7f11dbdf46",
@@ -294,7 +294,7 @@ func TestParseFlags(t *testing.T) {
 				"--bluecat-dns-deploy-type=full-deploy",
 				"--bluecat-skip-tls-verify",
 				"--cloudflare-proxied",
-				"--cloudflare-zones-per-page=20",
+				"--cloudflare-dns-records-per-page=5000",
 				"--coredns-prefix=/coredns/",
 				"--akamai-serviceconsumerdomain=oooo-xxxxxxxxxxxxxxxx-xxxxxxxxxxxxxxxx.luna.akamaiapis.net",
 				"--akamai-client-token=o184671d5307a388180fbf7f11dbdf46",
@@ -417,7 +417,7 @@ func TestParseFlags(t *testing.T) {
 				"EXTERNAL_DNS_BLUECAT_ROOT_ZONE":               "arg",
 				"EXTERNAL_DNS_BLUECAT_SKIP_TLS_VERIFY":         "1",
 				"EXTERNAL_DNS_CLOUDFLARE_PROXIED":              "1",
-				"EXTERNAL_DNS_CLOUDFLARE_ZONES_PER_PAGE":       "20",
+				"EXTERNAL_DNS_CLOUDFLARE_DNS_RECORDS_PER_PAGE": "5000",
 				"EXTERNAL_DNS_COREDNS_PREFIX":                  "/coredns/",
 				"EXTERNAL_DNS_AKAMAI_SERVICECONSUMERDOMAIN":    "oooo-xxxxxxxxxxxxxxxx-xxxxxxxxxxxxxxxx.luna.akamaiapis.net",
 				"EXTERNAL_DNS_AKAMAI_CLIENT_TOKEN":             "o184671d5307a388180fbf7f11dbdf46",

provider/cloudflare/cloudflare.go

@@ -122,7 +122,7 @@ type CloudFlareProvider struct {
 	zoneIDFilter      provider.ZoneIDFilter
 	proxiedByDefault  bool
 	DryRun            bool
-	PaginationOptions cloudflare.PaginationOptions
+	DNSRecordsPerPage int
 }
 
 // cloudFlareChange differentiates between ChangActions
@@ -148,7 +148,7 @@ func getRecordParam[T RecordParamsTypes](cfc cloudFlareChange) T {
 }
 
 // NewCloudFlareProvider initializes a new CloudFlare DNS based Provider.
-func NewCloudFlareProvider(domainFilter endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, zonesPerPage int, proxiedByDefault bool, dryRun bool) (*CloudFlareProvider, error) {
+func NewCloudFlareProvider(domainFilter endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, proxiedByDefault bool, dryRun bool, dnsRecordsPerPage int) (*CloudFlareProvider, error) {
 	// initialize via chosen auth method and returns new API object
 	var (
 		config *cloudflare.API
@@ -169,10 +169,7 @@ func NewCloudFlareProvider(domainFilter endpoint.DomainFilter, zoneIDFilter prov
 		zoneIDFilter:      zoneIDFilter,
 		proxiedByDefault:  proxiedByDefault,
 		DryRun:            dryRun,
-		PaginationOptions: cloudflare.PaginationOptions{
-			PerPage: zonesPerPage,
-			Page:    1,
-		},
+		DNSRecordsPerPage: dnsRecordsPerPage,
 	}
 	return provider, nil
 }
@@ -180,7 +177,6 @@ func NewCloudFlareProvider(domainFilter endpoint.DomainFilter, zoneIDFilter prov
 // Zones returns the list of hosted zones.
 func (p *CloudFlareProvider) Zones(ctx context.Context) ([]cloudflare.Zone, error) {
 	result := []cloudflare.Zone{}
-	p.PaginationOptions.Page = 1
 
 	// if there is a zoneIDfilter configured
 	// && if the filter isn't just a blank string (used in tests)
@@ -229,7 +225,7 @@ func (p *CloudFlareProvider) Records(ctx context.Context) ([]*endpoint.Endpoint,
 
 	endpoints := []*endpoint.Endpoint{}
 	for _, zone := range zones {
-		records, _, err := p.Client.ListDNSRecords(ctx, cloudflare.ZoneIdentifier(zone.ID), cloudflare.ListDNSRecordsParams{})
+		records, err := p.listDNSRecordsWithAutoPagination(ctx, zone.ID)
 		if err != nil {
 			return nil, err
 		}
@@ -303,7 +299,7 @@ func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloud
 	changesByZone := p.changesByZone(zones, changes)
 
 	for zoneID, changes := range changesByZone {
-		records, _, err := p.Client.ListDNSRecords(ctx, cloudflare.ZoneIdentifier(zoneID), cloudflare.ListDNSRecordsParams{})
+		records, err := p.listDNSRecordsWithAutoPagination(ctx, zoneID)
 		if err != nil {
 			return fmt.Errorf("could not fetch records from zone, %v", err)
 		}
@@ -420,6 +416,26 @@ func (p *CloudFlareProvider) newCloudFlareChange(action string, endpoint *endpoi
 	}
 }
 
+// listDNSRecords performs automatic pagination of results on requests to cloudflare.ListDNSRecords with custom per_page values
+func (p *CloudFlareProvider) listDNSRecordsWithAutoPagination(ctx context.Context, zoneID string) ([]cloudflare.DNSRecord, error) {
+	var records []cloudflare.DNSRecord
+	resultInfo := cloudflare.ResultInfo{PerPage: p.DNSRecordsPerPage, Page: 1}
+	params := cloudflare.ListDNSRecordsParams{ResultInfo: resultInfo}
+	for {
+		pageRecords, resultInfo, err := p.Client.ListDNSRecords(ctx, cloudflare.ZoneIdentifier(zoneID), params)
+		if err != nil {
+			return nil, err
+		}
+
+		records = append(records, pageRecords...)
+		params.ResultInfo = resultInfo.Next()
+		if params.ResultInfo.Done() {
+			break
+		}
+	}
+	return records, nil
+}
+
 func shouldBeProxied(endpoint *endpoint.Endpoint, proxiedByDefault bool) bool {
 	proxied := proxiedByDefault
 

provider/cloudflare/cloudflare_test.go

@@ -20,6 +20,8 @@ import (
 	"context"
 	"errors"
 	"os"
+	"sort"
+	"strings"
 	"testing"
 
 	cloudflare "github.com/cloudflare/cloudflare-go"
@@ -151,9 +153,36 @@ func (m *mockCloudFlareClient) ListDNSRecords(ctx context.Context, rc *cloudflar
 		for _, record := range zone {
 			result = append(result, record)
 		}
-		return result, &cloudflare.ResultInfo{}, nil
 	}
-	return result, &cloudflare.ResultInfo{}, nil
+
+	if len(result) == 0 || rp.PerPage == 0 {
+		return result, &cloudflare.ResultInfo{Page: 1, TotalPages: 1, Count: 0, Total: 0}, nil
+	}
+
+	// if not pagination options were passed in, return the result as is
+	if rp.Page == 0 {
+		return result, &cloudflare.ResultInfo{Page: 1, TotalPages: 1, Count: len(result), Total: len(result)}, nil
+	}
+
+	// otherwise, split the result into chunks of size rp.PerPage to simulate the pagination from the API
+	chunks := [][]cloudflare.DNSRecord{}
+
+	// to ensure consistency in the multiple calls to this function, sort the result slice
+	sort.Slice(result, func(i, j int) bool { return strings.Compare(result[i].ID, result[j].ID) > 0 })
+	for rp.PerPage < len(result) {
+		result, chunks = result[rp.PerPage:], append(chunks, result[0:rp.PerPage])
+	}
+	chunks = append(chunks, result)
+
+	// return the requested page
+	partialResult := chunks[rp.Page-1]
+	return partialResult, &cloudflare.ResultInfo{
+		PerPage:    rp.PerPage,
+		Page:       rp.Page,
+		TotalPages: len(chunks),
+		Count:      len(partialResult),
+		Total:      len(result),
+	}, nil
 }
 
 func (m *mockCloudFlareClient) UpdateDNSRecord(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.UpdateDNSRecordParams) error {
@@ -611,8 +640,10 @@ func TestCloudflareRecords(t *testing.T) {
 		"001": ExampleDomain,
 	})
 
+	// Set DNSRecordsPerPage to 1 test the pagination behaviour
 	provider := &CloudFlareProvider{
 		Client:            client,
+		DNSRecordsPerPage: 1,
 	}
 	ctx := context.Background()
 
@@ -640,9 +671,9 @@ func TestCloudflareProvider(t *testing.T) {
 	_, err := NewCloudFlareProvider(
 		endpoint.NewDomainFilter([]string{"bar.com"}),
 		provider.NewZoneIDFilter([]string{""}),
-		25,
 		false,
-		true)
+		true,
+		5000)
 	if err != nil {
 		t.Errorf("should not fail, %s", err)
 	}
@@ -652,9 +683,9 @@ func TestCloudflareProvider(t *testing.T) {
 	_, err = NewCloudFlareProvider(
 		endpoint.NewDomainFilter([]string{"bar.com"}),
 		provider.NewZoneIDFilter([]string{""}),
-		1,
 		false,
-		true)
+		true,
+		5000)
 	if err != nil {
 		t.Errorf("should not fail, %s", err)
 	}
@@ -663,9 +694,9 @@ func TestCloudflareProvider(t *testing.T) {
 	_, err = NewCloudFlareProvider(
 		endpoint.NewDomainFilter([]string{"bar.com"}),
 		provider.NewZoneIDFilter([]string{""}),
-		50,
 		false,
-		true)
+		true,
+		5000)
 	if err == nil {
 		t.Errorf("expected to fail")
 	}