Repository: kubernetes-sigs/external-dns (language: go)

Issue:
Cloudflare provider rate limits cause a fatal error and pod restarts into CLBO
<!-- Please use this template while reporting a bug and provide as much info as possible. Not doing so may result in your bug not being addressed in a timely manner. Thanks!

-->

**What happened**:  External DNS for Cloudflare often enters CrashLoopBackOff when it hits a rate limit.  Since this was merged https://github.com/kubernetes-sigs/external-dns/pull/3009 with the v0.13.5 release this has been occurring.  We caught it on an upgrade to 0.14.1

**What you expected to happen**:  Rate limit errors should be handled as the new SoftError type to prevent bailing and restarting.

**How to reproduce it (as minimally and precisely as possible)**:
* Run a version of external-dns >= 0.13.5
* Use up the rate limit of Cloudflare API calls (or mock this)
* Have external-dns running through a loop
* It will then exit and bail.

**Anything else we need to know?**:

I'm willing to attempt to fix this, though my golang isn't super

**Environment**:
- External-DNS version (use `external-dns --version`): 0.14.1
- DNS provider: cloudflare
- Others:



Discussion / hints:

Fix patch:
diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go
index 5ed089ae0b..8779cd7b50 100644
--- a/provider/cloudflare/cloudflare.go
+++ b/provider/cloudflare/cloudflare.go
@@ -18,6 +18,7 @@ package cloudflare
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"os"
 	"strconv"
@@ -223,6 +224,13 @@ func (p *CloudFlareProvider) Zones(ctx context.Context) ([]cloudflare.Zone, erro
 
 	zonesResponse, err := p.Client.ListZonesContext(ctx)
 	if err != nil {
+		var apiErr *cloudflare.Error
+		if errors.As(err, &apiErr) {
+			if apiErr.ClientRateLimited() {
+				// Handle rate limit error as a soft error
+				return nil, provider.NewSoftError(err)
+			}
+		}
 		return nil, err
 	}
 
@@ -456,6 +464,13 @@ func (p *CloudFlareProvider) listDNSRecordsWithAutoPagination(ctx context.Contex
 	for {
 		pageRecords, resultInfo, err := p.Client.ListDNSRecords(ctx, cloudflare.ZoneIdentifier(zoneID), params)
 		if err != nil {
+			var apiErr *cloudflare.Error
+			if errors.As(err, &apiErr) {
+				if apiErr.ClientRateLimited() {
+					// Handle rate limit error as a soft error
+					return nil, provider.NewSoftError(err)
+				}
+			}
 			return nil, err
 		}
 


Test patch:
diff --git a/provider/cloudflare/cloudflare_test.go b/provider/cloudflare/cloudflare_test.go
index 82450b0e55..22e0b753cc 100644
--- a/provider/cloudflare/cloudflare_test.go
+++ b/provider/cloudflare/cloudflare_test.go
@@ -42,12 +42,13 @@ type MockAction struct {
 }
 
 type mockCloudFlareClient struct {
-	User            cloudflare.User
-	Zones           map[string]string
-	Records         map[string]map[string]cloudflare.DNSRecord
-	Actions         []MockAction
-	listZonesError  error
-	dnsRecordsError error
+	User                  cloudflare.User
+	Zones                 map[string]string
+	Records               map[string]map[string]cloudflare.DNSRecord
+	Actions               []MockAction
+	listZonesError        error
+	listZonesContextError error
+	dnsRecordsError       error
 }
 
 var ExampleDomain = []cloudflare.DNSRecord{
@@ -253,8 +254,8 @@ func (m *mockCloudFlareClient) ListZones(ctx context.Context, zoneID ...string)
 }
 
 func (m *mockCloudFlareClient) ListZonesContext(ctx context.Context, opts ...cloudflare.ReqOption) (cloudflare.ZonesResponse, error) {
-	if m.listZonesError != nil {
-		return cloudflare.ZonesResponse{}, m.listZonesError
+	if m.listZonesContextError != nil {
+		return cloudflare.ZonesResponse{}, m.listZonesContextError
 	}
 
 	result := []cloudflare.Zone{}
@@ -643,32 +644,60 @@ func TestCloudFlareZonesWithIDFilter(t *testing.T) {
 	assert.Equal(t, "bar.com", zones[0].Name)
 }
 
+func TestCloudflareListZonesRateLimited(t *testing.T) {
+	// Create a mock client that returns a rate limit error
+	client := NewMockCloudFlareClient()
+	client.listZonesContextError = &cloudflare.Error{
+		StatusCode: 429,
+		ErrorCodes: []int{10000},
+		Type:       cloudflare.ErrorTypeRateLimit,
+	}
+	p := &CloudFlareProvider{Client: client}
+
+	// Call the Zones function
+	_, err := p.Zones(context.Background())
+
+	// Assert that a soft error was returned
+	if !errors.Is(err, provider.SoftError) {
+		t.Error("expected a rate limit error")
+	}
+}
+
 func TestCloudflareRecords(t *testing.T) {
 	client := NewMockCloudFlareClientWithRecords(map[string][]cloudflare.DNSRecord{
 		"001": ExampleDomain,
 	})
 
 	// Set DNSRecordsPerPage to 1 test the pagination behaviour
-	provider := &CloudFlareProvider{
+	p := &CloudFlareProvider{
 		Client:            client,
 		DNSRecordsPerPage: 1,
 	}
 	ctx := context.Background()
 
-	records, err := provider.Records(ctx)
+	records, err := p.Records(ctx)
 	if err != nil {
 		t.Errorf("should not fail, %s", err)
 	}
-
 	assert.Equal(t, 2, len(records))
 	client.dnsRecordsError = errors.New("failed to list dns records")
-	_, err = provider.Records(ctx)
+	_, err = p.Records(ctx)
 	if err == nil {
 		t.Errorf("expected to fail")
 	}
 	client.dnsRecordsError = nil
-	client.listZonesError = errors.New("failed to list zones")
-	_, err = provider.Records(ctx)
+	client.listZonesContextError = &cloudflare.Error{
+		StatusCode: 429,
+		ErrorCodes: []int{10000},
+		Type:       cloudflare.ErrorTypeRateLimit,
+	}
+	_, err = p.Records(ctx)
+	// Assert that a soft error was returned
+	if !errors.Is(err, provider.SoftError) {
+		t.Error("expected a rate limit error")
+	}
+	client.listZonesContextError = errors.New("failed to list zones")
+	_, err = p.Records(ctx)
 	if err == nil {
 		t.Errorf("expected to fail")
 	}