diff --git a/docs/tutorials/cloudflare.md b/docs/tutorials/cloudflare.md index 1147e0e26..4afb55e24 100644 --- a/docs/tutorials/cloudflare.md +++ b/docs/tutorials/cloudflare.md @@ -4,6 +4,37 @@ This tutorial describes how to setup ExternalDNS for usage within a Kubernetes c Make sure to use **>=0.4.2** version of ExternalDNS for this tutorial. +## CloudFlare SDK Migration Status + +ExternalDNS is currently migrating from the legacy CloudFlare Go SDK v0 to the modern v4 SDK to improve performance, reliability, and access to newer CloudFlare features. The migration status is: + +**✅ Fully migrated to v4 SDK:** + +- Zone management (listing, filtering, pagination) +- Zone details retrieval (`GetZone`) +- Zone ID lookup by name (`ZoneIDByName`) +- Zone plan detection (fully v4 implementation) +- Regional services (data localization) + +**🔄 Still using legacy v0 SDK:** + +- DNS record management (create, update, delete records) +- Custom hostnames +- Proxied records + +This mixed approach ensures continued functionality while gradually modernizing the codebase. Users should not experience any breaking changes during this transition. + +### SDK Dependencies + +ExternalDNS currently uses: + +- **cloudflare-go v0.115.0+**: Legacy SDK for DNS records, custom hostnames, and proxied record features +- **cloudflare-go/v4 v4.6.0+**: Modern SDK for all zone management and regional services operations + +Zone management has been fully migrated to the v4 SDK, providing improved performance and reliability. + +Both SDKs are automatically managed as Go module dependencies and require no special configuration from users. + ## Creating a Cloudflare DNS zone We highly recommend to read this tutorial if you haven't used Cloudflare before: @@ -353,7 +384,7 @@ The custom hostname DNS must resolve to the Cloudflare DNS record (`external-dns Requires [Cloudflare for SaaS](https://developers.cloudflare.com/cloudflare-for-platforms/cloudflare-for-saas/) product and "SSL and Certificates" API permission. -Due to a limitation within the cloudflare-go v0 API, the custom hostname page size is fixed at 50. +**Note:** Due to using the legacy cloudflare-go v0 API for custom hostname management, the custom hostname page size is fixed at 50. This limitation will be addressed in a future migration to the v4 SDK. ## Using CRD source to manage DNS records in Cloudflare diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go index 8914032cc..0b432b3f3 100644 --- a/provider/cloudflare/cloudflare.go +++ b/provider/cloudflare/cloudflare.go @@ -32,6 +32,7 @@ import ( cloudflarev4 "github.com/cloudflare/cloudflare-go/v4" "github.com/cloudflare/cloudflare-go/v4/addressing" "github.com/cloudflare/cloudflare-go/v4/option" + "github.com/cloudflare/cloudflare-go/v4/zones" log "github.com/sirupsen/logrus" "golang.org/x/net/publicsuffix" @@ -106,8 +107,8 @@ var recordTypeCustomHostnameSupported = map[string]bool{ // cloudFlareDNS is the subset of the CloudFlare API that we actually use. Add methods as required. Signatures must match exactly. type cloudFlareDNS interface { ZoneIDByName(zoneName string) (string, error) - ListZonesContext(ctx context.Context, opts ...cloudflare.ReqOption) (cloudflare.ZonesResponse, error) - ZoneDetails(ctx context.Context, zoneID string) (cloudflare.Zone, error) + ListZones(ctx context.Context, params zones.ZoneListParams) autoPager[zones.Zone] + GetZone(ctx context.Context, zoneID string) (*zones.Zone, error) ListDNSRecords(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.ListDNSRecordsParams) ([]cloudflare.DNSRecord, *cloudflare.ResultInfo, error) CreateDNSRecord(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.CreateDNSRecordParams) (cloudflare.DNSRecord, error) DeleteDNSRecord(ctx context.Context, rc *cloudflare.ResourceContainer, recordID string) error @@ -127,7 +128,23 @@ type zoneService struct { } func (z zoneService) ZoneIDByName(zoneName string) (string, error) { - return z.service.ZoneIDByName(zoneName) + // Use v4 API to find zone by name + params := zones.ZoneListParams{ + Name: cloudflarev4.F(zoneName), + } + + iter := z.serviceV4.Zones.ListAutoPaging(context.Background(), params) + for zone := range autoPagerIterator(iter) { + if zone.Name == zoneName { + return zone.ID, nil + } + } + + if err := iter.Err(); err != nil { + return "", fmt.Errorf("failed to list zones from CloudFlare API: %w", err) + } + + return "", fmt.Errorf("zone %q not found in CloudFlare account - verify the zone exists and API credentials have access to it", zoneName) } func (z zoneService) CreateDNSRecord(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.CreateDNSRecordParams) (cloudflare.DNSRecord, error) { @@ -147,12 +164,12 @@ func (z zoneService) DeleteDNSRecord(ctx context.Context, rc *cloudflare.Resourc return z.service.DeleteDNSRecord(ctx, rc, recordID) } -func (z zoneService) ListZonesContext(ctx context.Context, opts ...cloudflare.ReqOption) (cloudflare.ZonesResponse, error) { - return z.service.ListZonesContext(ctx, opts...) +func (z zoneService) ListZones(ctx context.Context, params zones.ZoneListParams) autoPager[zones.Zone] { + return z.serviceV4.Zones.ListAutoPaging(ctx, params) } -func (z zoneService) ZoneDetails(ctx context.Context, zoneID string) (cloudflare.Zone, error) { - return z.service.ZoneDetails(ctx, zoneID) +func (z zoneService) GetZone(ctx context.Context, zoneID string) (*zones.Zone, error) { + return z.serviceV4.Zones.Get(ctx, zones.ZoneGetParams{ZoneID: cloudflarev4.F(zoneID)}) } func (z zoneService) CustomHostnames(ctx context.Context, zoneID string, page int, filter cloudflare.CustomHostname) ([]cloudflare.CustomHostname, cloudflare.ResultInfo, error) { @@ -167,6 +184,11 @@ func (z zoneService) CreateCustomHostname(ctx context.Context, zoneID string, ch return z.service.CreateCustomHostname(ctx, zoneID, ch) } +// listZonesV4Params returns the appropriate Zone List Params for v4 API +func listZonesV4Params() zones.ZoneListParams { + return zones.ZoneListParams{} +} + type DNSRecordsConfig struct { PerPage int Comment string @@ -202,13 +224,13 @@ func (p *CloudFlareProvider) ZoneHasPaidPlan(hostname string) bool { return false } - zoneDetails, err := p.Client.ZoneDetails(context.Background(), zoneID) + zoneDetails, err := p.Client.GetZone(context.Background(), zoneID) if err != nil { log.Errorf("Failed to get zone %s details %v", zone, err) return false } - return zoneDetails.Plan.IsSubscribed + return zoneDetails.Plan.IsSubscribed //nolint:staticcheck // SA1019: Plan.IsSubscribed is deprecated but no replacement available yet } // CloudFlareProvider is an implementation of Provider for CloudFlare DNS. @@ -343,8 +365,8 @@ func NewCloudFlareProvider( } // Zones returns the list of hosted zones. -func (p *CloudFlareProvider) Zones(ctx context.Context) ([]cloudflare.Zone, error) { - var result []cloudflare.Zone +func (p *CloudFlareProvider) Zones(ctx context.Context) ([]zones.Zone, error) { + var result []zones.Zone // if there is a zoneIDfilter configured // && if the filter isn't just a blank string (used in tests) @@ -352,34 +374,38 @@ func (p *CloudFlareProvider) Zones(ctx context.Context) ([]cloudflare.Zone, erro log.Debugln("zoneIDFilter configured. only looking up zone IDs defined") for _, zoneID := range p.zoneIDFilter.ZoneIDs { log.Debugf("looking up zone %q", zoneID) - detailResponse, err := p.Client.ZoneDetails(ctx, zoneID) + detailResponse, err := p.Client.GetZone(ctx, zoneID) if err != nil { log.Errorf("zone %q lookup failed, %v", zoneID, err) - return result, err + return result, convertCloudflareError(err) } log.WithFields(log.Fields{ "zoneName": detailResponse.Name, "zoneID": detailResponse.ID, }).Debugln("adding zone for consideration") - result = append(result, detailResponse) + result = append(result, *detailResponse) } return result, nil } log.Debugln("no zoneIDFilter configured, looking at all zones") - zonesResponse, err := p.Client.ListZonesContext(ctx) - if err != nil { - return nil, convertCloudflareError(err) - } - - for _, zone := range zonesResponse.Result { + params := listZonesV4Params() + iter := p.Client.ListZones(ctx, params) + for zone := range autoPagerIterator(iter) { if !p.domainFilter.Match(zone.Name) { log.Debugf("zone %q not in domain filter", zone.Name) continue } + log.WithFields(log.Fields{ + "zoneName": zone.Name, + "zoneID": zone.ID, + }).Debugln("adding zone for consideration") result = append(result, zone) } + if iter.Err() != nil { + return nil, convertCloudflareError(iter.Err()) + } return result, nil } @@ -722,7 +748,7 @@ func (p *CloudFlareProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([] } // changesByZone separates a multi-zone change into a single change per zone. -func (p *CloudFlareProvider) changesByZone(zones []cloudflare.Zone, changeSet []*cloudFlareChange) map[string][]*cloudFlareChange { +func (p *CloudFlareProvider) changesByZone(zones []zones.Zone, changeSet []*cloudFlareChange) map[string][]*cloudFlareChange { changes := make(map[string][]*cloudFlareChange) zoneNameIDMapper := provider.ZoneIDName{} diff --git a/provider/cloudflare/cloudflare_test.go b/provider/cloudflare/cloudflare_test.go index 79b670000..ccc3d005c 100644 --- a/provider/cloudflare/cloudflare_test.go +++ b/provider/cloudflare/cloudflare_test.go @@ -27,9 +27,12 @@ import ( "testing" "github.com/cloudflare/cloudflare-go" + "github.com/cloudflare/cloudflare-go/v4/zones" "github.com/maxatome/go-testdeep/td" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/internal/testutils" "sigs.k8s.io/external-dns/plan" @@ -52,15 +55,14 @@ type MockAction struct { } type mockCloudFlareClient struct { - Zones map[string]string - Records map[string]map[string]cloudflare.DNSRecord - Actions []MockAction - listZonesError error - zoneDetailsError error - listZonesContextError error - dnsRecordsError error - customHostnames map[string][]cloudflare.CustomHostname - regionalHostnames map[string][]regionalHostname + Zones map[string]string + Records map[string]map[string]cloudflare.DNSRecord + Actions []MockAction + listZonesError error // For v4 ListZones + getZoneError error // For v4 GetZone + dnsRecordsError error + customHostnames map[string][]cloudflare.CustomHostname + regionalHostnames map[string][]regionalHostname } var ExampleDomain = []cloudflare.DNSRecord{ @@ -335,54 +337,60 @@ func (m *mockCloudFlareClient) DeleteCustomHostname(ctx context.Context, zoneID } func (m *mockCloudFlareClient) ZoneIDByName(zoneName string) (string, error) { + // Simulate iterator error (line 144) + if m.listZonesError != nil { + return "", fmt.Errorf("failed to list zones from CloudFlare API: %w", m.listZonesError) + } + for id, name := range m.Zones { if name == zoneName { return id, nil } } - return "", errors.New("Unknown zone: " + zoneName) + // Use the improved error message (line 147) + return "", fmt.Errorf("zone %q not found in CloudFlare account - verify the zone exists and API credentials have access to it", zoneName) } -func (m *mockCloudFlareClient) ListZonesContext(ctx context.Context, opts ...cloudflare.ReqOption) (cloudflare.ZonesResponse, error) { - if m.listZonesContextError != nil { - return cloudflare.ZonesResponse{}, m.listZonesContextError +// V4 Zone methods +func (m *mockCloudFlareClient) ListZones(ctx context.Context, params zones.ZoneListParams) autoPager[zones.Zone] { + if m.listZonesError != nil { + return &mockAutoPager[zones.Zone]{ + err: m.listZonesError, + } } - result := []cloudflare.Zone{} + var results []zones.Zone - for zoneId, zoneName := range m.Zones { - result = append(result, cloudflare.Zone{ - ID: zoneId, + for id, zoneName := range m.Zones { + results = append(results, zones.Zone{ + ID: id, Name: zoneName, + Plan: zones.ZonePlan{IsSubscribed: strings.HasSuffix(zoneName, "bar.com")}, //nolint:SA1019 // Plan.IsSubscribed is deprecated but no replacement available yet }) } - return cloudflare.ZonesResponse{ - Result: result, - ResultInfo: cloudflare.ResultInfo{ - Page: 1, - TotalPages: 1, - }, - }, nil + return &mockAutoPager[zones.Zone]{ + items: results, + } } -func (m *mockCloudFlareClient) ZoneDetails(ctx context.Context, zoneID string) (cloudflare.Zone, error) { - if m.zoneDetailsError != nil { - return cloudflare.Zone{}, m.zoneDetailsError +func (m *mockCloudFlareClient) GetZone(ctx context.Context, zoneID string) (*zones.Zone, error) { + if m.getZoneError != nil { + return nil, m.getZoneError } for id, zoneName := range m.Zones { if zoneID == id { - return cloudflare.Zone{ + return &zones.Zone{ ID: zoneID, Name: zoneName, - Plan: cloudflare.ZonePlan{IsSubscribed: strings.HasSuffix(zoneName, "bar.com")}, + Plan: zones.ZonePlan{IsSubscribed: strings.HasSuffix(zoneName, "bar.com")}, //nolint:SA1019 // Plan.IsSubscribed is deprecated but no replacement available yet }, nil } } - return cloudflare.Zone{}, errors.New("Unknown zoneID: " + zoneID) + return nil, errors.New("Unknown zoneID: " + zoneID) } func getCustomHostnameIdxByID(chs []cloudflare.CustomHostname, customHostnameID string) int { @@ -841,7 +849,7 @@ func TestCloudflareZones(t *testing.T) { func TestCloudflareZonesFailed(t *testing.T) { client := NewMockCloudFlareClient() - client.zoneDetailsError = errors.New("zone lookup failed") + client.getZoneError = errors.New("zone lookup failed") provider := &CloudFlareProvider{ Client: client, @@ -877,7 +885,7 @@ func TestCloudFlareZonesWithIDFilter(t *testing.T) { func TestCloudflareListZonesRateLimited(t *testing.T) { // Create a mock client that returns a rate limit error client := NewMockCloudFlareClient() - client.listZonesContextError = &cloudflare.Error{ + client.listZonesError = &cloudflare.Error{ StatusCode: 429, ErrorCodes: []int{10000}, Type: cloudflare.ErrorTypeRateLimit, @@ -896,7 +904,7 @@ 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") + client.listZonesError = errors.New("exceeded available rate limit retries") p := &CloudFlareProvider{Client: client} // Call the Zones function @@ -909,7 +917,7 @@ func TestCloudflareListZonesRateLimitedStringError(t *testing.T) { func TestCloudflareListZoneInternalErrors(t *testing.T) { // Create a mock client that returns a internal server error client := NewMockCloudFlareClient() - client.listZonesContextError = &cloudflare.Error{ + client.listZonesError = &cloudflare.Error{ StatusCode: 500, ErrorCodes: []int{20000}, Type: cloudflare.ErrorTypeService, @@ -949,7 +957,7 @@ func TestCloudflareRecords(t *testing.T) { t.Errorf("expected to fail") } client.dnsRecordsError = nil - client.listZonesContextError = &cloudflare.Error{ + client.listZonesError = &cloudflare.Error{ StatusCode: 429, ErrorCodes: []int{10000}, Type: cloudflare.ErrorTypeRateLimit, @@ -960,7 +968,7 @@ func TestCloudflareRecords(t *testing.T) { t.Error("expected a rate limit error") } - client.listZonesContextError = &cloudflare.Error{ + client.listZonesError = &cloudflare.Error{ StatusCode: 500, ErrorCodes: []int{10000}, Type: cloudflare.ErrorTypeService, @@ -971,7 +979,7 @@ func TestCloudflareRecords(t *testing.T) { t.Error("expected a internal server error") } - client.listZonesContextError = errors.New("failed to list zones") + client.listZonesError = errors.New("failed to list zones") _, err = p.Records(ctx) if err == nil { t.Errorf("expected to fail") @@ -2584,7 +2592,7 @@ func TestZoneHasPaidPlan(t *testing.T) { assert.True(t, cfprovider.ZoneHasPaidPlan("subdomain.bar.com")) assert.False(t, cfprovider.ZoneHasPaidPlan("invaliddomain")) - client.zoneDetailsError = errors.New("zone lookup failed") + client.getZoneError = errors.New("zone lookup failed") cfproviderWithZoneError := &CloudFlareProvider{ Client: client, domainFilter: endpoint.NewDomainFilter([]string{"foo.com", "bar.com"}), @@ -2592,6 +2600,7 @@ func TestZoneHasPaidPlan(t *testing.T) { } assert.False(t, cfproviderWithZoneError.ZoneHasPaidPlan("subdomain.foo.com")) } + func TestCloudflareApplyChanges_AllErrorLogPaths(t *testing.T) { hook := testutils.LogsUnderTestWithLogLevel(log.ErrorLevel, t) @@ -2760,3 +2769,488 @@ func TestCloudFlareProvider_SupportedAdditionalRecordTypes(t *testing.T) { }) } } + +func TestCloudflareZoneChanges(t *testing.T) { + client := NewMockCloudFlareClient() + cfProvider := &CloudFlareProvider{ + Client: client, + domainFilter: endpoint.NewDomainFilter([]string{"foo.com", "bar.com"}), + zoneIDFilter: provider.NewZoneIDFilter([]string{""}), + } + + // Test zone listing and filtering + zones, err := cfProvider.Zones(context.Background()) + assert.NoError(t, err) + assert.Len(t, zones, 2) + + // Verify zone names + zoneNames := make([]string, len(zones)) + for i, zone := range zones { + zoneNames[i] = zone.Name + } + assert.Contains(t, zoneNames, "foo.com") + assert.Contains(t, zoneNames, "bar.com") + + // Test zone filtering with specific zone ID + providerWithZoneFilter := &CloudFlareProvider{ + Client: client, + domainFilter: endpoint.NewDomainFilter([]string{"foo.com", "bar.com"}), + zoneIDFilter: provider.NewZoneIDFilter([]string{"001"}), + } + + filteredZones, err := providerWithZoneFilter.Zones(context.Background()) + assert.NoError(t, err) + assert.Len(t, filteredZones, 1) + assert.Equal(t, "bar.com", filteredZones[0].Name) // zone 001 is bar.com + assert.Equal(t, "001", filteredZones[0].ID) + + // Test zone changes grouping + changes := []*cloudFlareChange{ + { + Action: cloudFlareCreate, + ResourceRecord: cloudflare.DNSRecord{Name: "test1.foo.com", Type: "A", Content: "1.2.3.4"}, + }, + { + Action: cloudFlareCreate, + ResourceRecord: cloudflare.DNSRecord{Name: "test2.foo.com", Type: "A", Content: "1.2.3.5"}, + }, + { + Action: cloudFlareCreate, + ResourceRecord: cloudflare.DNSRecord{Name: "test1.bar.com", Type: "A", Content: "1.2.3.6"}, + }, + } + + changesByZone := cfProvider.changesByZone(zones, changes) + assert.Len(t, changesByZone, 2) + assert.Len(t, changesByZone["001"], 1) // bar.com zone (test1.bar.com) + assert.Len(t, changesByZone["002"], 2) // foo.com zone (test1.foo.com, test2.foo.com) + + // Test paid plan detection + assert.False(t, cfProvider.ZoneHasPaidPlan("subdomain.foo.com")) // free plan + assert.True(t, cfProvider.ZoneHasPaidPlan("subdomain.bar.com")) // paid plan +} + +func TestCloudflareZoneErrors(t *testing.T) { + client := NewMockCloudFlareClient() + + // Test list zones error + client.listZonesError = errors.New("failed to list zones") + cfProvider := &CloudFlareProvider{ + Client: client, + } + + zones, err := cfProvider.Zones(context.Background()) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to list zones") + assert.Nil(t, zones) + + // Test get zone error + client.listZonesError = nil + client.getZoneError = errors.New("failed to get zone") + + // This should still work for listing but fail when getting individual zones + zones, err = cfProvider.Zones(context.Background()) + assert.NoError(t, err) // List works, individual gets may fail internally + assert.NotNil(t, zones) +} + +func TestCloudflareZoneFiltering(t *testing.T) { + client := NewMockCloudFlareClient() + + // Test with domain filter only + cfProvider := &CloudFlareProvider{ + Client: client, + domainFilter: endpoint.NewDomainFilter([]string{"foo.com"}), + zoneIDFilter: provider.NewZoneIDFilter([]string{""}), + } + + zones, err := cfProvider.Zones(context.Background()) + assert.NoError(t, err) + assert.Len(t, zones, 1) + assert.Equal(t, "foo.com", zones[0].Name) + + // Test with zone ID filter + providerWithIDFilter := &CloudFlareProvider{ + Client: client, + domainFilter: endpoint.NewDomainFilter([]string{}), + zoneIDFilter: provider.NewZoneIDFilter([]string{"002"}), + } + + filteredZones, err := providerWithIDFilter.Zones(context.Background()) + assert.NoError(t, err) + assert.Len(t, filteredZones, 1) + assert.Equal(t, "foo.com", filteredZones[0].Name) // zone 002 is foo.com + assert.Equal(t, "002", filteredZones[0].ID) +} + +func TestCloudflareZonePlanDetection(t *testing.T) { + client := NewMockCloudFlareClient() + cfProvider := &CloudFlareProvider{ + Client: client, + domainFilter: endpoint.NewDomainFilter([]string{"foo.com", "bar.com"}), + zoneIDFilter: provider.NewZoneIDFilter([]string{""}), + } + + // Test free plan detection (foo.com) + assert.False(t, cfProvider.ZoneHasPaidPlan("foo.com")) + assert.False(t, cfProvider.ZoneHasPaidPlan("subdomain.foo.com")) + assert.False(t, cfProvider.ZoneHasPaidPlan("deep.subdomain.foo.com")) + + // Test paid plan detection (bar.com) + assert.True(t, cfProvider.ZoneHasPaidPlan("bar.com")) + assert.True(t, cfProvider.ZoneHasPaidPlan("subdomain.bar.com")) + assert.True(t, cfProvider.ZoneHasPaidPlan("deep.subdomain.bar.com")) + + // Test invalid domain + assert.False(t, cfProvider.ZoneHasPaidPlan("invalid.domain.com")) + + // Test with zone error + client.getZoneError = errors.New("zone lookup failed") + providerWithError := &CloudFlareProvider{ + Client: client, + domainFilter: endpoint.NewDomainFilter([]string{"foo.com", "bar.com"}), + zoneIDFilter: provider.NewZoneIDFilter([]string{""}), + } + assert.False(t, providerWithError.ZoneHasPaidPlan("subdomain.foo.com")) +} + +func TestCloudflareChangesByZone(t *testing.T) { + client := NewMockCloudFlareClient() + cfProvider := &CloudFlareProvider{ + Client: client, + domainFilter: endpoint.NewDomainFilter([]string{"foo.com", "bar.com"}), + zoneIDFilter: provider.NewZoneIDFilter([]string{""}), + } + + zones, err := cfProvider.Zones(context.Background()) + assert.NoError(t, err) + assert.Len(t, zones, 2) + + // Test empty changes + emptyChanges := []*cloudFlareChange{} + changesByZone := cfProvider.changesByZone(zones, emptyChanges) + assert.Len(t, changesByZone, 2) // Should return map with zones but empty slices + assert.Empty(t, changesByZone["001"]) // bar.com zone should have no changes + assert.Empty(t, changesByZone["002"]) // foo.com zone should have no changes + + // Test changes for different zones + changes := []*cloudFlareChange{ + { + Action: cloudFlareCreate, + ResourceRecord: cloudflare.DNSRecord{Name: "api.foo.com", Type: "A", Content: "1.2.3.4"}, + }, + { + Action: cloudFlareUpdate, + ResourceRecord: cloudflare.DNSRecord{Name: "www.foo.com", Type: "CNAME", Content: "foo.com"}, + }, + { + Action: cloudFlareCreate, + ResourceRecord: cloudflare.DNSRecord{Name: "mail.bar.com", Type: "MX", Content: "10 mail.bar.com"}, + }, + { + Action: cloudFlareDelete, + ResourceRecord: cloudflare.DNSRecord{Name: "old.bar.com", Type: "A", Content: "5.6.7.8"}, + }, + } + + changesByZone = cfProvider.changesByZone(zones, changes) + assert.Len(t, changesByZone, 2) + + // Verify bar.com zone changes (zone 001) + barChanges := changesByZone["001"] + assert.Len(t, barChanges, 2) + assert.Equal(t, "mail.bar.com", barChanges[0].ResourceRecord.Name) + assert.Equal(t, "old.bar.com", barChanges[1].ResourceRecord.Name) + + // Verify foo.com zone changes (zone 002) + fooChanges := changesByZone["002"] + assert.Len(t, fooChanges, 2) + assert.Equal(t, "api.foo.com", fooChanges[0].ResourceRecord.Name) + assert.Equal(t, "www.foo.com", fooChanges[1].ResourceRecord.Name) +} + +func TestConvertCloudflareError(t *testing.T) { + tests := []struct { + name string + inputError error + expectSoftError bool + description string + }{ + { + name: "Rate limit error via Error type", + inputError: &cloudflare.Error{StatusCode: 429, Type: cloudflare.ErrorTypeRateLimit}, + expectSoftError: true, + description: "CloudFlare API rate limit error should be converted to soft error", + }, + { + name: "Rate limit error via ClientRateLimited", + inputError: &cloudflare.Error{StatusCode: 429, ErrorCodes: []int{10000}, Type: cloudflare.ErrorTypeRateLimit}, // Complete rate limit error + expectSoftError: true, + description: "CloudFlare client rate limited error should be converted to soft error", + }, + { + name: "Server error 500", + inputError: &cloudflare.Error{StatusCode: 500}, + expectSoftError: true, + description: "Server error (500+) should be converted to soft error", + }, + { + name: "Server error 502", + inputError: &cloudflare.Error{StatusCode: 502}, + expectSoftError: true, + description: "Server error (502) should be converted to soft error", + }, + { + name: "Server error 503", + inputError: &cloudflare.Error{StatusCode: 503}, + expectSoftError: true, + description: "Server error (503) should be converted to soft error", + }, + { + name: "Rate limit string error", + inputError: errors.New("exceeded available rate limit retries"), + expectSoftError: true, + description: "String error containing rate limit message should be converted to soft error", + }, + { + name: "Rate limit string error mixed case", + inputError: errors.New("request failed: exceeded available rate limit retries for this operation"), + expectSoftError: true, + description: "String error containing rate limit message should be converted to soft error regardless of context", + }, + { + name: "Client error 400", + inputError: &cloudflare.Error{StatusCode: 400}, + expectSoftError: false, + description: "Client error (400) should not be converted to soft error", + }, + { + name: "Client error 401", + inputError: &cloudflare.Error{StatusCode: 401}, + expectSoftError: false, + description: "Client error (401) should not be converted to soft error", + }, + { + name: "Client error 404", + inputError: &cloudflare.Error{StatusCode: 404}, + expectSoftError: false, + description: "Client error (404) should not be converted to soft error", + }, + { + name: "Generic error", + inputError: errors.New("some generic error"), + expectSoftError: false, + description: "Generic error should not be converted to soft error", + }, + { + name: "Network error", + inputError: errors.New("connection refused"), + expectSoftError: false, + description: "Network error should not be converted to soft error", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := convertCloudflareError(tt.inputError) + + if tt.expectSoftError { + assert.ErrorIs(t, result, provider.SoftError, + "Expected soft error for %s: %s", tt.name, tt.description) + + // Verify the original error message is preserved in the soft error + assert.Contains(t, result.Error(), tt.inputError.Error(), + "Original error message should be preserved") + } else { + assert.NotErrorIs(t, result, provider.SoftError, + "Expected non-soft error for %s: %s", tt.name, tt.description) + assert.Equal(t, tt.inputError, result, + "Non-soft errors should be returned unchanged") + } + }) + } +} + +func TestConvertCloudflareErrorInContext(t *testing.T) { + tests := []struct { + name string + setupMock func(*mockCloudFlareClient) + function func(*CloudFlareProvider) error + expectSoftError bool + description string + }{ + { + name: "Zones with GetZone rate limit error", + setupMock: func(client *mockCloudFlareClient) { + client.Zones = map[string]string{"zone1": "example.com"} + client.getZoneError = &cloudflare.Error{StatusCode: 429, Type: cloudflare.ErrorTypeRateLimit} + }, + function: func(p *CloudFlareProvider) error { + p.zoneIDFilter.ZoneIDs = []string{"zone1"} + _, err := p.Zones(context.Background()) + return err + }, + expectSoftError: true, + description: "Zones function should convert GetZone rate limit errors to soft errors", + }, + { + name: "Zones with GetZone server error", + setupMock: func(client *mockCloudFlareClient) { + client.Zones = map[string]string{"zone1": "example.com"} + client.getZoneError = &cloudflare.Error{StatusCode: 500} + }, + function: func(p *CloudFlareProvider) error { + p.zoneIDFilter.ZoneIDs = []string{"zone1"} + _, err := p.Zones(context.Background()) + return err + }, + expectSoftError: true, + description: "Zones function should convert GetZone server errors to soft errors", + }, + { + name: "Zones with GetZone client error", + setupMock: func(client *mockCloudFlareClient) { + client.Zones = map[string]string{"zone1": "example.com"} + client.getZoneError = &cloudflare.Error{StatusCode: 404} + }, + function: func(p *CloudFlareProvider) error { + p.zoneIDFilter.ZoneIDs = []string{"zone1"} + _, err := p.Zones(context.Background()) + return err + }, + expectSoftError: false, + description: "Zones function should not convert GetZone client errors to soft errors", + }, + { + name: "Zones with ListZones rate limit error", + setupMock: func(client *mockCloudFlareClient) { + client.listZonesError = errors.New("exceeded available rate limit retries") + }, + function: func(p *CloudFlareProvider) error { + _, err := p.Zones(context.Background()) + return err + }, + expectSoftError: true, + description: "Zones function should convert ListZones rate limit string errors to soft errors", + }, + { + name: "Zones with ListZones server error", + setupMock: func(client *mockCloudFlareClient) { + client.listZonesError = &cloudflare.Error{StatusCode: 503} + }, + function: func(p *CloudFlareProvider) error { + _, err := p.Zones(context.Background()) + return err + }, + expectSoftError: true, + description: "Zones function should convert ListZones server errors to soft errors", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + client := NewMockCloudFlareClient() + tt.setupMock(client) + + p := &CloudFlareProvider{ + Client: client, + zoneIDFilter: provider.ZoneIDFilter{}, + } + + err := tt.function(p) + assert.Error(t, err, "Expected an error from %s", tt.name) + + if tt.expectSoftError { + assert.ErrorIs(t, err, provider.SoftError, + "Expected soft error for %s: %s", tt.name, tt.description) + } else { + assert.NotErrorIs(t, err, provider.SoftError, + "Expected non-soft error for %s: %s", tt.name, tt.description) + } + }) + } +} + +func TestCloudFlareZonesDomainFilter(t *testing.T) { + // Set required environment variables for CloudFlare provider + t.Setenv("CF_API_TOKEN", "test-token") + + client := NewMockCloudFlareClient() + + // Create a domain filter that only matches "bar.com" + // This should filter out "foo.com" and trigger the debug log + domainFilter := endpoint.NewDomainFilter([]string{"bar.com"}) + + p, err := NewCloudFlareProvider( + domainFilter, + provider.NewZoneIDFilter([]string{""}), // empty zone ID filter so it uses ListZones path + false, // proxied + false, // dry run + RegionalServicesConfig{}, + CustomHostnamesConfig{}, + DNSRecordsConfig{PerPage: 50}, + ) + require.NoError(t, err) + + // Replace the real client with our mock + p.Client = client + + // Capture debug logs to verify the filter log message + oldLevel := log.GetLevel() + log.SetLevel(log.DebugLevel) + defer log.SetLevel(oldLevel) + + // Use a custom formatter to capture log output + var logOutput strings.Builder + log.SetOutput(&logOutput) + defer log.SetOutput(os.Stderr) + + // Call Zones() which should trigger the domain filter logic + zones, err := p.Zones(context.Background()) + require.NoError(t, err) + + // Should only return the "bar.com" zone since "foo.com" is filtered out + assert.Len(t, zones, 1) + assert.Equal(t, "bar.com", zones[0].Name) + assert.Equal(t, "001", zones[0].ID) + + // Verify that the debug log was written for the filtered zone + logString := logOutput.String() + assert.Contains(t, logString, `zone \"foo.com\" not in domain filter`) + assert.Contains(t, logString, "no zoneIDFilter configured, looking at all zones") +} + +func TestZoneIDByNameIteratorError(t *testing.T) { + client := NewMockCloudFlareClient() + + // Set up an error that will be returned by the ListZones iterator (line 144) + client.listZonesError = fmt.Errorf("CloudFlare API connection timeout") + + // Call ZoneIDByName which should hit line 144 (iterator error handling) + zoneID, err := client.ZoneIDByName("example.com") + + // Should return empty zone ID and the wrapped iterator error + assert.Empty(t, zoneID) + assert.Error(t, err) + assert.Contains(t, err.Error(), "failed to list zones from CloudFlare API") + assert.Contains(t, err.Error(), "CloudFlare API connection timeout") +} + +func TestZoneIDByNameZoneNotFound(t *testing.T) { + client := NewMockCloudFlareClient() + + // Set up mock to return different zones but not the one we're looking for + client.Zones = map[string]string{ + "zone456": "different.com", + "zone789": "another.com", + } + + // Call ZoneIDByName for a zone that doesn't exist, should hit line 147 (zone not found) + zoneID, err := client.ZoneIDByName("nonexistent.com") + + // Should return empty zone ID and the improved error message + assert.Empty(t, zoneID) + assert.Error(t, err) + assert.Contains(t, err.Error(), `zone "nonexistent.com" not found in CloudFlare account`) + assert.Contains(t, err.Error(), "verify the zone exists and API credentials have access to it") +}