Skip to content

Commit

Permalink
Merge pull request #4437 from ebachle/master
Browse files Browse the repository at this point in the history
fix: soft error on cloudflare rate limits
  • Loading branch information
k8s-ci-robot committed May 10, 2024
2 parents fac5b44 + c9e5f59 commit 49c6c26
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 14 deletions.
15 changes: 15 additions & 0 deletions provider/cloudflare/cloudflare.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package cloudflare

import (
"context"
"errors"
"fmt"
"os"
"strconv"
Expand Down Expand Up @@ -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
}

Expand Down Expand Up @@ -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
}

Expand Down
57 changes: 43 additions & 14 deletions provider/cloudflare/cloudflare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{}
Expand Down Expand Up @@ -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")
}
Expand Down

0 comments on commit 49c6c26

Please sign in to comment.