Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: soft error on cloudflare rate limits #4437

Merged
merged 3 commits into from
May 10, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading