mirrors/external-dns
1
0
Fork 0
mirror of https://github.com/kubernetes-sigs/external-dns.git synced 2025-08-05 09:06:58 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(cloudflare): improve handling of rate limiting errors (#5524)

Browse Source 
* fix cloudflare

* factorize error conversion

* only log if not a soft error

* add comment for workaround

* Remove debug print

Co-authored-by: vflaux <38909103+vflaux@users.noreply.github.com>

* Use assert

Co-authored-by: vflaux <38909103+vflaux@users.noreply.github.com>

* Improve error check

Co-authored-by: vflaux <38909103+vflaux@users.noreply.github.com>

* fix lint

---------

Co-authored-by: vflaux <38909103+vflaux@users.noreply.github.com>
This commit is contained in:
Edouard Benauw 2025-06-20 00:30:53 +02:00 committed by GitHub
parent 0f0e05ef86
commit 0fae060dff
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 36 additions and 24 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

47
provider/cloudflare/cloudflare.go
View File

@ -271,6 +271,23 @@ func getCreateDNSRecordParam(cfc cloudFlareChange) cloudflare.CreateDNSRecordPar
}
}
func convertCloudflareError(err error) error {
var apiErr *cloudflare.Error
if errors.As(err, &apiErr) {
if apiErr.ClientRateLimited() || apiErr.StatusCode >= http.StatusInternalServerError {
// Handle rate limit error as a soft error
return provider.NewSoftError(err)
}
}
// This is a workaround because Cloudflare library does not return a specific error type for rate limit exceeded.
// See https://github.com/cloudflare/cloudflare-go/issues/4155 and https://github.com/kubernetes-sigs/external-dns/pull/5524
// This workaround can be removed once Cloudflare library returns a specific error type.
if strings.Contains(err.Error(), "exceeded available rate limit retries") {
return provider.NewSoftError(err)
}
return err
}
// NewCloudFlareProvider initializes a new CloudFlare DNS based Provider.
func NewCloudFlareProvider(domainFilter endpoint.DomainFilter, zoneIDFilter provider.ZoneIDFilter, proxiedByDefault bool, dryRun bool, regionKey string, customHostnamesConfig CustomHostnamesConfig, dnsRecordsConfig DNSRecordsConfig) (*CloudFlareProvider, error) {
// initialize via chosen auth method and returns new API object
@ -335,14 +352,7 @@ 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() || apiErr.StatusCode >= http.StatusInternalServerError {
// Handle rate limit error as a soft error
return nil, provider.NewSoftError(err)
}
}
return nil, err
return nil, convertCloudflareError(err)
}
for _, zone := range zonesResponse.Result {
@ -753,14 +763,7 @@ 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() || apiErr.StatusCode >= http.StatusInternalServerError {
// Handle rate limit error as a soft error
return nil, provider.NewSoftError(err)
}
}
return nil, err
return nil, convertCloudflareError(err)
}
for _, r := range pageRecords {
@ -788,15 +791,11 @@ func (p *CloudFlareProvider) listCustomHostnamesWithPagination(ctx context.Conte
for {
pageCustomHostnameListResponse, result, err := p.Client.CustomHostnames(ctx, zoneID, resultInfo.Page, cloudflare.CustomHostname{})
if err != nil {
var apiErr *cloudflare.Error
if errors.As(err, &apiErr) {
if apiErr.ClientRateLimited() || apiErr.StatusCode >= http.StatusInternalServerError {
// Handle rate limit error as a soft error
return nil, provider.NewSoftError(err)
}
convertedError := convertCloudflareError(err)
if !errors.Is(convertedError, provider.SoftError) {
log.Errorf("zone %q failed to fetch custom hostnames. Please check if \"Cloudflare for SaaS\" is enabled and API key permissions, %v", zoneID, err)
}
log.Errorf("zone %q failed to fetch custom hostnames. Please check if \"Cloudflare for SaaS\" is enabled and API key permissions, %v", zoneID, err)
return nil, err
return nil, convertedError
}
for _, ch := range pageCustomHostnameListResponse {
chs[newCustomHostnameIndex(ch)] = ch

13
provider/cloudflare/cloudflare_test.go
View File

@ -955,6 +955,19 @@ func TestCloudflareListZonesRateLimited(t *testing.T) {
}
}
func TestCloudflareListZonesRateLimitedStringError(t *testing.T) {
// Create a mock client that returns a rate limit error
client := NewMockCloudFlareClient()
client.listZonesContextError = errors.New("exceeded available rate limit retries")
p := &CloudFlareProvider{Client: client}
// Call the Zones function
_, err := p.Zones(context.Background())
// Assert that a soft error was returned
assert.ErrorIs(t, err, provider.SoftError, "expected a rate limit error")
}
func TestCloudflareListZoneInternalErrors(t *testing.T) {
// Create a mock client that returns a internal server error
client := NewMockCloudFlareClient()