From 7108979df156ad5a68b83f22bfdf82201207f178 Mon Sep 17 00:00:00 2001 From: vflaux <38909103+vflaux@users.noreply.github.com> Date: Sun, 22 Jun 2025 12:22:52 +0200 Subject: [PATCH] improve cloudflare regional hostname implementation (#5309) - add flag to enable regional hostname feature - support deletion of regional hostname on annotation edit - correctly support differences detection with cloudflare state - increased tests coverage Co-authored-by: Michel Loiseleur <97035654+mloiseleur@users.noreply.github.com> --- controller/execute.go | 5 +- docs/flags.md | 3 +- docs/tutorials/cloudflare.md | 17 +- pkg/apis/externaldns/types.go | 5 +- pkg/apis/externaldns/types_test.go | 3 + provider/cloudflare/cloudflare.go | 81 +- provider/cloudflare/cloudflare_regional.go | 281 ++-- .../cloudflare/cloudflare_regional_test.go | 1191 ++++++++++++++--- provider/cloudflare/cloudflare_test.go | 168 +-- 9 files changed, 1260 insertions(+), 494 deletions(-) diff --git a/controller/execute.go b/controller/execute.go index 851bccf28..be412fa5e 100644 --- a/controller/execute.go +++ b/controller/execute.go @@ -215,7 +215,10 @@ func buildProvider( zoneIDFilter, cfg.CloudflareProxied, cfg.DryRun, - cfg.CloudflareRegionKey, + cloudflare.RegionalServicesConfig{ + Enabled: cfg.CloudflareRegionalServices, + RegionKey: cfg.CloudflareRegionKey, + }, cloudflare.CustomHostnamesConfig{ Enabled: cfg.CloudflareCustomHostnames, MinTLSVersion: cfg.CloudflareCustomHostnamesMinTLSVersion, diff --git a/docs/flags.md b/docs/flags.md index 3825292fd..d0ec9e318 100644 --- a/docs/flags.md +++ b/docs/flags.md @@ -93,7 +93,8 @@ | `--cloudflare-custom-hostnames-min-tls-version=1.0` | When using the Cloudflare provider with the Custom Hostnames, specify which Minimum TLS Version will be used by default. (default: 1.0, options: 1.0, 1.1, 1.2, 1.3) | | `--cloudflare-custom-hostnames-certificate-authority=none` | When using the Cloudflare provider with the Custom Hostnames, specify which Certificate Authority will be used. A value of none indicates no Certificate Authority will be sent to the Cloudflare API (default: none, options: google, ssl_com, lets_encrypt, none) | | `--cloudflare-dns-records-per-page=100` | When using the Cloudflare provider, specify how many DNS records listed per page, max possible 5,000 (default: 100) | -| `--cloudflare-region-key=CLOUDFLARE-REGION-KEY` | When using the Cloudflare provider, specify the region (default: earth) | +| `--[no-]cloudflare-regional-services` | When using the Cloudflare provider, specify if Regional Services feature will be used (default: disabled) | +| `--cloudflare-region-key=CLOUDFLARE-REGION-KEY` | When using the Cloudflare provider, specify the default region for Regional Services. Any value other than an empty string will enable the Regional Services feature (optional) | | `--cloudflare-record-comment=""` | When using the Cloudflare provider, specify the comment for the DNS records (default: '') | | `--coredns-prefix="/skydns/"` | When using the CoreDNS provider, specify the prefix name | | `--akamai-serviceconsumerdomain=""` | When using the Akamai provider, specify the base URL (required when --provider=akamai and edgerc-path not specified) | diff --git a/docs/tutorials/cloudflare.md b/docs/tutorials/cloudflare.md index c9d024544..7fd08ce95 100644 --- a/docs/tutorials/cloudflare.md +++ b/docs/tutorials/cloudflare.md @@ -128,6 +128,7 @@ spec: - --provider=cloudflare - --cloudflare-proxied # (optional) enable the proxy feature of Cloudflare (DDOS protection, CDN...) - --cloudflare-dns-records-per-page=5000 # (optional) configure how many DNS records to fetch per request + - --cloudflare-regional-services # (optional) enable the regional hostname feature that configure which region can decrypt HTTPS requests - --cloudflare-region-key="eu" # (optional) configure which region can decrypt HTTPS requests - --cloudflare-record-comment="provisioned by external-dns" # (optional) configure comments for provisioned records; <=100 chars for free zones; <=500 chars for paid zones env: @@ -205,6 +206,7 @@ spec: - --provider=cloudflare - --cloudflare-proxied # (optional) enable the proxy feature of Cloudflare (DDOS protection, CDN...) - --cloudflare-dns-records-per-page=5000 # (optional) configure how many DNS records to fetch per request + - --cloudflare-regional-services # (optional) enable the regional hostname feature that configure which region can decrypt HTTPS requests - --cloudflare-region-key="eu" # (optional) configure which region can decrypt HTTPS requests - --cloudflare-record-comment="provisioned by external-dns" # (optional) configure comments for provisioned records; <=100 chars for free zones; <=500 chars for paid zones env: @@ -303,13 +305,19 @@ kubectl delete -f externaldns.yaml Using the `external-dns.alpha.kubernetes.io/cloudflare-proxied: "true"` annotation on your ingress, you can specify if the proxy feature of Cloudflare should be enabled for that record. This setting will override the global `--cloudflare-proxied` setting. -## Setting cloudflare-region-key to configure regional services +## Setting cloudlfare regional services -Using the `external-dns.alpha.kubernetes.io/cloudflare-region-key` annotation on your ingress, you can restrict which data centers can decrypt and serve HTTPS traffic. +With Cloudflare regional services you can restrict which data centers can decrypt and serve HTTPS traffic. + +Configuration of Cloudflare Regional Services is enabled by the `--cloudflare-regional-services` flag. +A default region can be defined using the `--cloudflare-region-key` flag. + +Using the `external-dns.alpha.kubernetes.io/cloudflare-region-key` annotation on your ingress, you can specify the region for that record. + +An empty string will result in no regional hostname configured. **Accepted values for region key include:** -- `earth` (default): All data centers (global) - `eu`: European Union data centers only - `us`: United States data centers only - `ap`: Asia-Pacific data centers only @@ -321,14 +329,11 @@ Using the `external-dns.alpha.kubernetes.io/cloudflare-region-key` annotation on - `br`: Brazil data centers only - `za`: South Africa data centers only - `ae`: United Arab Emirates data centers only -- `global`: Alias for `earth` For the most up-to-date list and details, see the [Cloudflare Regional Services documentation](https://developers.cloudflare.com/data-localization/regional-services/get-started/). Currently, requires SuperAdmin or Admin role. -If not set the value will default to `global`. - ## Setting cloudflare-custom-hostname Automatic configuration of Cloudflare custom hostnames (using A/CNAME DNS records as custom origin servers) is enabled by the `--cloudflare-custom-hostnames` flag and the `external-dns.alpha.kubernetes.io/cloudflare-custom-hostname: ` annotation. diff --git a/pkg/apis/externaldns/types.go b/pkg/apis/externaldns/types.go index 6ea0b4a2a..ee0125ff7 100644 --- a/pkg/apis/externaldns/types.go +++ b/pkg/apis/externaldns/types.go @@ -113,6 +113,7 @@ type Config struct { CloudflareDNSRecordsComment string CloudflareCustomHostnamesMinTLSVersion string CloudflareCustomHostnamesCertificateAuthority string + CloudflareRegionalServices bool CloudflareRegionKey string CloudflareRecordComment string CoreDNSPrefix string @@ -256,6 +257,7 @@ var defaultConfig = &Config{ CloudflareCustomHostnamesMinTLSVersion: "1.0", CloudflareDNSRecordsPerPage: 100, CloudflareProxied: false, + CloudflareRegionalServices: false, CloudflareRegionKey: "earth", CombineFQDNAndAnnotation: false, @@ -533,7 +535,8 @@ func App(cfg *Config) *kingpin.Application { app.Flag("cloudflare-custom-hostnames-min-tls-version", "When using the Cloudflare provider with the Custom Hostnames, specify which Minimum TLS Version will be used by default. (default: 1.0, options: 1.0, 1.1, 1.2, 1.3)").Default("1.0").EnumVar(&cfg.CloudflareCustomHostnamesMinTLSVersion, "1.0", "1.1", "1.2", "1.3") app.Flag("cloudflare-custom-hostnames-certificate-authority", "When using the Cloudflare provider with the Custom Hostnames, specify which Certificate Authority will be used. A value of none indicates no Certificate Authority will be sent to the Cloudflare API (default: none, options: google, ssl_com, lets_encrypt, none)").Default("none").EnumVar(&cfg.CloudflareCustomHostnamesCertificateAuthority, "google", "ssl_com", "lets_encrypt", "none") app.Flag("cloudflare-dns-records-per-page", "When using the Cloudflare provider, specify how many DNS records listed per page, max possible 5,000 (default: 100)").Default(strconv.Itoa(defaultConfig.CloudflareDNSRecordsPerPage)).IntVar(&cfg.CloudflareDNSRecordsPerPage) - app.Flag("cloudflare-region-key", "When using the Cloudflare provider, specify the region (default: earth)").StringVar(&cfg.CloudflareRegionKey) + app.Flag("cloudflare-regional-services", "When using the Cloudflare provider, specify if Regional Services feature will be used (default: disabled)").Default(strconv.FormatBool(defaultConfig.CloudflareRegionalServices)).BoolVar(&cfg.CloudflareRegionalServices) + app.Flag("cloudflare-region-key", "When using the Cloudflare provider, specify the default region for Regional Services. Any value other than an empty string will enable the Regional Services feature (optional)").StringVar(&cfg.CloudflareRegionKey) app.Flag("cloudflare-record-comment", "When using the Cloudflare provider, specify the comment for the DNS records (default: '')").Default("").StringVar(&cfg.CloudflareRecordComment) app.Flag("coredns-prefix", "When using the CoreDNS provider, specify the prefix name").Default(defaultConfig.CoreDNSPrefix).StringVar(&cfg.CoreDNSPrefix) diff --git a/pkg/apis/externaldns/types_test.go b/pkg/apis/externaldns/types_test.go index 357672dc3..3c90b667f 100644 --- a/pkg/apis/externaldns/types_test.go +++ b/pkg/apis/externaldns/types_test.go @@ -186,6 +186,7 @@ var ( CloudflareCustomHostnamesMinTLSVersion: "1.3", CloudflareCustomHostnamesCertificateAuthority: "google", CloudflareDNSRecordsPerPage: 5000, + CloudflareRegionalServices: true, CloudflareRegionKey: "us", CoreDNSPrefix: "/coredns/", AkamaiServiceConsumerDomain: "oooo-xxxxxxxxxxxxxxxx-xxxxxxxxxxxxxxxx.luna.akamaiapis.net", @@ -296,6 +297,7 @@ func TestParseFlags(t *testing.T) { "--cloudflare-custom-hostnames-min-tls-version=1.3", "--cloudflare-custom-hostnames-certificate-authority=google", "--cloudflare-dns-records-per-page=5000", + "--cloudflare-regional-services", "--cloudflare-region-key=us", "--coredns-prefix=/coredns/", "--akamai-serviceconsumerdomain=oooo-xxxxxxxxxxxxxxxx-xxxxxxxxxxxxxxxx.luna.akamaiapis.net", @@ -424,6 +426,7 @@ func TestParseFlags(t *testing.T) { "EXTERNAL_DNS_CLOUDFLARE_CUSTOM_HOSTNAMES_MIN_TLS_VERSION": "1.3", "EXTERNAL_DNS_CLOUDFLARE_CUSTOM_HOSTNAMES_CERTIFICATE_AUTHORITY": "google", "EXTERNAL_DNS_CLOUDFLARE_DNS_RECORDS_PER_PAGE": "5000", + "EXTERNAL_DNS_CLOUDFLARE_REGIONAL_SERVICES": "1", "EXTERNAL_DNS_CLOUDFLARE_REGION_KEY": "us", "EXTERNAL_DNS_COREDNS_PREFIX": "/coredns/", "EXTERNAL_DNS_AKAMAI_SERVICECONSUMERDOMAIN": "oooo-xxxxxxxxxxxxxxxx-xxxxxxxxxxxxxxxx.luna.akamaiapis.net", diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go index 58627d0de..bdd72ee28 100644 --- a/provider/cloudflare/cloudflare.go +++ b/provider/cloudflare/cloudflare.go @@ -119,6 +119,7 @@ type cloudFlareDNS interface { CreateDNSRecord(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.CreateDNSRecordParams) (cloudflare.DNSRecord, error) DeleteDNSRecord(ctx context.Context, rc *cloudflare.ResourceContainer, recordID string) error UpdateDNSRecord(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.UpdateDNSRecordParams) error + ListDataLocalizationRegionalHostnames(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.ListDataLocalizationRegionalHostnamesParams) ([]cloudflare.RegionalHostname, error) CreateDataLocalizationRegionalHostname(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.CreateDataLocalizationRegionalHostnameParams) error UpdateDataLocalizationRegionalHostname(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.UpdateDataLocalizationRegionalHostnameParams) error DeleteDataLocalizationRegionalHostname(ctx context.Context, rc *cloudflare.ResourceContainer, hostname string) error @@ -216,13 +217,13 @@ type CloudFlareProvider struct { provider.BaseProvider Client cloudFlareDNS // only consider hosted zones managing domains ending in this suffix - domainFilter *endpoint.DomainFilter - zoneIDFilter provider.ZoneIDFilter - proxiedByDefault bool - DryRun bool - CustomHostnamesConfig CustomHostnamesConfig - DNSRecordsConfig DNSRecordsConfig - RegionKey string + domainFilter *endpoint.DomainFilter + zoneIDFilter provider.ZoneIDFilter + proxiedByDefault bool + DryRun bool + CustomHostnamesConfig CustomHostnamesConfig + DNSRecordsConfig DNSRecordsConfig + RegionalServicesConfig RegionalServicesConfig } // cloudFlareChange differentiates between ChangActions @@ -279,7 +280,15 @@ func convertCloudflareError(err error) error { } // 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) { +func NewCloudFlareProvider( + domainFilter *endpoint.DomainFilter, + zoneIDFilter provider.ZoneIDFilter, + proxiedByDefault bool, + dryRun bool, + regionalServicesConfig RegionalServicesConfig, + customHostnamesConfig CustomHostnamesConfig, + dnsRecordsConfig DNSRecordsConfig, +) (*CloudFlareProvider, error) { // initialize via chosen auth method and returns new API object var ( config *cloudflare.API @@ -302,15 +311,19 @@ func NewCloudFlareProvider(domainFilter *endpoint.DomainFilter, zoneIDFilter pro return nil, fmt.Errorf("failed to initialize cloudflare provider: %w", err) } + if regionalServicesConfig.RegionKey != "" { + regionalServicesConfig.Enabled = true + } + return &CloudFlareProvider{ - Client: zoneService{config}, - domainFilter: domainFilter, - zoneIDFilter: zoneIDFilter, - proxiedByDefault: proxiedByDefault, - CustomHostnamesConfig: customHostnamesConfig, - DryRun: dryRun, - RegionKey: regionKey, - DNSRecordsConfig: dnsRecordsConfig, + Client: zoneService{config}, + domainFilter: domainFilter, + zoneIDFilter: zoneIDFilter, + proxiedByDefault: proxiedByDefault, + CustomHostnamesConfig: customHostnamesConfig, + DryRun: dryRun, + RegionalServicesConfig: regionalServicesConfig, + DNSRecordsConfig: dnsRecordsConfig, }, nil } @@ -379,7 +392,13 @@ func (p *CloudFlareProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, // As CloudFlare does not support "sets" of targets, but instead returns // a single entry for each name/type/target, we have to group by name // and record to allow the planner to calculate the correct plan. See #992. - endpoints = append(endpoints, groupByNameAndTypeWithCustomHostnames(records, chs)...) + zoneEndpoints := groupByNameAndTypeWithCustomHostnames(records, chs) + + if err := p.addEnpointsProviderSpecificRegionKeyProperty(ctx, zone.ID, zoneEndpoints); err != nil { + return nil, err + } + + endpoints = append(endpoints, zoneEndpoints...) } return endpoints, nil @@ -595,16 +614,21 @@ func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloud } } - if regionalHostnamesChanges, err := dataLocalizationRegionalHostnamesChanges(zoneChanges); err == nil { - if !p.submitDataLocalizationRegionalHostnameChanges(ctx, regionalHostnamesChanges, resourceContainer) { - failedChange = true + if p.RegionalServicesConfig.Enabled { + desiredRegionalHostnames, err := desiredRegionalHostnames(zoneChanges) + if err != nil { + return fmt.Errorf("failed to build desired regional hostnames: %w", err) } - } else { - logFields := log.Fields{ - "zone": zoneID, + if len(desiredRegionalHostnames) > 0 { + regionalHostnames, err := p.listDataLocalisationRegionalHostnames(ctx, resourceContainer) + if err != nil { + return fmt.Errorf("could not fetch regional hostnames from zone, %w", err) + } + regionalHostnamesChanges := regionalHostnamesChanges(desiredRegionalHostnames, regionalHostnames) + if !p.submitRegionalHostnameChanges(ctx, regionalHostnamesChanges, resourceContainer) { + failedChange = true + } } - log.WithFields(logFields).Errorf("failed to build data localization regional hostname changes: %v", err) - failedChange = true } if failedChange { @@ -640,6 +664,13 @@ func (p *CloudFlareProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([] e.DeleteProviderSpecificProperty(annotations.CloudflareCustomHostnameKey) } + if p.RegionalServicesConfig.Enabled { + // Add default region key if not set + if _, ok := e.GetProviderSpecificProperty(annotations.CloudflareRegionKey); !ok { + e.SetProviderSpecificProperty(annotations.CloudflareRegionKey, p.RegionalServicesConfig.RegionKey) + } + } + adjustedEndpoints = append(adjustedEndpoints, e) } return adjustedEndpoints, nil diff --git a/provider/cloudflare/cloudflare_regional.go b/provider/cloudflare/cloudflare_regional.go index f2b355952..6e93e2c72 100644 --- a/provider/cloudflare/cloudflare_regional.go +++ b/provider/cloudflare/cloudflare_regional.go @@ -18,30 +18,40 @@ package cloudflare import ( "context" - "errors" "fmt" "maps" - "net/http" "slices" - "github.com/cloudflare/cloudflare-go" + cloudflare "github.com/cloudflare/cloudflare-go" log "github.com/sirupsen/logrus" "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/source/annotations" ) +type RegionalServicesConfig struct { + Enabled bool + RegionKey string +} + var recordTypeRegionalHostnameSupported = map[string]bool{ "A": true, "AAAA": true, "CNAME": true, } +// RegionalHostnamesMap is a map of regional hostnames keyed by hostname. +type RegionalHostnamesMap map[string]cloudflare.RegionalHostname + type regionalHostnameChange struct { action changeAction cloudflare.RegionalHostname } +func (z zoneService) ListDataLocalizationRegionalHostnames(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.ListDataLocalizationRegionalHostnamesParams) ([]cloudflare.RegionalHostname, error) { + return z.service.ListDataLocalizationRegionalHostnames(ctx, rc, rp) +} + func (z zoneService) CreateDataLocalizationRegionalHostname(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.CreateDataLocalizationRegionalHostnameParams) error { _, err := z.service.CreateDataLocalizationRegionalHostname(ctx, rc, rp) return err @@ -72,89 +82,78 @@ func updateDataLocalizationRegionalHostnameParams(rhc regionalHostnameChange) cl } } -// submitDataLocalizationRegionalHostnameChanges applies a set of data localization regional hostname changes, returns false if it fails -func (p *CloudFlareProvider) submitDataLocalizationRegionalHostnameChanges(ctx context.Context, rhChanges []regionalHostnameChange, resourceContainer *cloudflare.ResourceContainer) bool { +// submitRegionalHostnameChanges applies a set of regional hostname changes, returns false if at least one fails +func (p *CloudFlareProvider) submitRegionalHostnameChanges(ctx context.Context, rhChanges []regionalHostnameChange, resourceContainer *cloudflare.ResourceContainer) bool { failedChange := false for _, rhChange := range rhChanges { - logFields := log.Fields{ - "hostname": rhChange.Hostname, - "region_key": rhChange.RegionKey, - "action": rhChange.action, - "zone": resourceContainer.Identifier, - } - log.WithFields(logFields).Info("Changing regional hostname") - switch rhChange.action { - case cloudFlareCreate: - log.WithFields(logFields).Debug("Creating regional hostname") - if p.DryRun { - continue - } - regionalHostnameParam := createDataLocalizationRegionalHostnameParams(rhChange) - err := p.Client.CreateDataLocalizationRegionalHostname(ctx, resourceContainer, regionalHostnameParam) - if err != nil { - var apiErr *cloudflare.Error - if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusConflict { - log.WithFields(logFields).Debug("Regional hostname already exists, updating instead") - params := updateDataLocalizationRegionalHostnameParams(rhChange) - err := p.Client.UpdateDataLocalizationRegionalHostname(ctx, resourceContainer, params) - if err != nil { - failedChange = true - log.WithFields(logFields).Errorf("failed to update regional hostname: %v", err) - } - continue - } - failedChange = true - log.WithFields(logFields).Errorf("failed to create regional hostname: %v", err) - } - case cloudFlareUpdate: - log.WithFields(logFields).Debug("Updating regional hostname") - if p.DryRun { - continue - } - regionalHostnameParam := updateDataLocalizationRegionalHostnameParams(rhChange) - err := p.Client.UpdateDataLocalizationRegionalHostname(ctx, resourceContainer, regionalHostnameParam) - if err != nil { - var apiErr *cloudflare.Error - if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound { - log.WithFields(logFields).Debug("Regional hostname not does not exists, creating instead") - params := createDataLocalizationRegionalHostnameParams(rhChange) - err := p.Client.CreateDataLocalizationRegionalHostname(ctx, resourceContainer, params) - if err != nil { - failedChange = true - log.WithFields(logFields).Errorf("failed to create regional hostname: %v", err) - } - continue - } - failedChange = true - log.WithFields(logFields).Errorf("failed to update regional hostname: %v", err) - } - case cloudFlareDelete: - log.WithFields(logFields).Debug("Deleting regional hostname") - if p.DryRun { - continue - } - err := p.Client.DeleteDataLocalizationRegionalHostname(ctx, resourceContainer, rhChange.Hostname) - if err != nil { - var apiErr *cloudflare.Error - if errors.As(err, &apiErr) && apiErr.StatusCode == http.StatusNotFound { - log.WithFields(logFields).Debug("Regional hostname does not exists, nothing to do") - continue - } - failedChange = true - log.WithFields(logFields).Errorf("failed to delete regional hostname: %v", err) - } + if !p.submitRegionalHostnameChange(ctx, rhChange, resourceContainer) { + failedChange = true } } return !failedChange } +// submitRegionalHostnameChange applies a single regional hostname change, returns false if it fails +func (p *CloudFlareProvider) submitRegionalHostnameChange(ctx context.Context, rhChange regionalHostnameChange, resourceContainer *cloudflare.ResourceContainer) bool { + changeLog := log.WithFields(log.Fields{ + "hostname": rhChange.Hostname, + "region_key": rhChange.RegionKey, + "action": rhChange.action, + "zone": resourceContainer.Identifier, + }) + if p.DryRun { + changeLog.Debug("Dry run: skipping regional hostname change", rhChange.action) + return true + } + switch rhChange.action { + case cloudFlareCreate: + changeLog.Debug("Creating regional hostname") + regionalHostnameParam := createDataLocalizationRegionalHostnameParams(rhChange) + if err := p.Client.CreateDataLocalizationRegionalHostname(ctx, resourceContainer, regionalHostnameParam); err != nil { + changeLog.Errorf("failed to create regional hostname: %v", err) + return false + } + case cloudFlareUpdate: + changeLog.Debug("Updating regional hostname") + regionalHostnameParam := updateDataLocalizationRegionalHostnameParams(rhChange) + if err := p.Client.UpdateDataLocalizationRegionalHostname(ctx, resourceContainer, regionalHostnameParam); err != nil { + changeLog.Errorf("failed to update regional hostname: %v", err) + return false + } + case cloudFlareDelete: + changeLog.Debug("Deleting regional hostname") + if err := p.Client.DeleteDataLocalizationRegionalHostname(ctx, resourceContainer, rhChange.Hostname); err != nil { + changeLog.Errorf("failed to delete regional hostname: %v", err) + return false + } + } + return true +} + +func (p *CloudFlareProvider) listDataLocalisationRegionalHostnames(ctx context.Context, resourceContainer *cloudflare.ResourceContainer) (RegionalHostnamesMap, error) { + rhs, err := p.Client.ListDataLocalizationRegionalHostnames(ctx, resourceContainer, cloudflare.ListDataLocalizationRegionalHostnamesParams{}) + if err != nil { + return nil, convertCloudflareError(err) + } + rhsMap := make(RegionalHostnamesMap) + for _, r := range rhs { + rhsMap[r.Hostname] = r + } + return rhsMap, nil +} + +// regionalHostname returns a RegionalHostname for the given endpoint. +// +// If the regional services feature is not enabled or the record type does not support regional hostnames, +// it returns an empty RegionalHostname. +// If the endpoint has a specific region key set, it uses that; otherwise, it defaults to the region key configured in the provider. func (p *CloudFlareProvider) regionalHostname(ep *endpoint.Endpoint) cloudflare.RegionalHostname { - if p.RegionKey == "" || !recordTypeRegionalHostnameSupported[ep.RecordType] { + if !p.RegionalServicesConfig.Enabled || !recordTypeRegionalHostnameSupported[ep.RecordType] { return cloudflare.RegionalHostname{} } - regionKey := p.RegionKey + regionKey := p.RegionalServicesConfig.RegionKey if epRegionKey, exists := ep.GetProviderSpecificProperty(annotations.CloudflareRegionKey); exists { regionKey = epRegionKey } @@ -164,47 +163,109 @@ func (p *CloudFlareProvider) regionalHostname(ep *endpoint.Endpoint) cloudflare. } } -// dataLocalizationRegionalHostnamesChanges processes a slice of cloudFlare changes and consolidates them -// into a list of data localization regional hostname changes. -// returns nil if no changes are needed -func dataLocalizationRegionalHostnamesChanges(changes []*cloudFlareChange) ([]regionalHostnameChange, error) { - regionalHostnameChanges := make(map[string]regionalHostnameChange) +// addEnpointsProviderSpecificRegionKeyProperty fetch the regional hostnames on cloudflare and +// adds Cloudflare-specific region keys to the provided endpoints. +// +// Do nothing if the regional services feature is not enabled. +// Defaults to the region key configured in the provider config if not found in the regional hostnames. +func (p *CloudFlareProvider) addEnpointsProviderSpecificRegionKeyProperty(ctx context.Context, zoneID string, endpoints []*endpoint.Endpoint) error { + if !p.RegionalServicesConfig.Enabled { + return nil + } + + // Filter endpoints to only those that support regional hostnames + // so we can skip regional hostname lookups if not needed. + var supportedEndpoints []*endpoint.Endpoint + for _, ep := range endpoints { + if recordTypeRegionalHostnameSupported[ep.RecordType] { + supportedEndpoints = append(supportedEndpoints, ep) + } + } + if len(supportedEndpoints) == 0 { + return nil + } + + regionalHostnames, err := p.listDataLocalisationRegionalHostnames(ctx, cloudflare.ZoneIdentifier(zoneID)) + if err != nil { + return err + } + + for _, ep := range supportedEndpoints { + if rh, found := regionalHostnames[ep.DNSName]; found { + ep.SetProviderSpecificProperty(annotations.CloudflareRegionKey, rh.RegionKey) + } + } + return nil +} + +// desiredRegionalHostnames builds a list of desired regional hostnames from changes. +// +// If there is a delete and a create or update action for the same hostname, +// The create or update takes precedence. +// Returns an error for conflicting region keys. +func desiredRegionalHostnames(changes []*cloudFlareChange) ([]cloudflare.RegionalHostname, error) { + rhs := make(map[string]cloudflare.RegionalHostname) for _, change := range changes { if change.RegionalHostname.Hostname == "" { continue } - if change.RegionalHostname.RegionKey == "" { - return nil, fmt.Errorf("region key is empty for regional hostname %q", change.RegionalHostname.Hostname) + rh, found := rhs[change.RegionalHostname.Hostname] + if !found { + if change.Action == cloudFlareDelete { + rhs[change.RegionalHostname.Hostname] = cloudflare.RegionalHostname{ + Hostname: change.RegionalHostname.Hostname, + RegionKey: "", // Indicate that this regional hostname should not exists + } + continue + } + rhs[change.RegionalHostname.Hostname] = change.RegionalHostname + continue } - regionalHostname, ok := regionalHostnameChanges[change.RegionalHostname.Hostname] - switch change.Action { - case cloudFlareCreate, cloudFlareUpdate: - if !ok { - regionalHostnameChanges[change.RegionalHostname.Hostname] = regionalHostnameChange{ - action: change.Action, - RegionalHostname: change.RegionalHostname, - } - continue - } - if regionalHostname.RegionKey != change.RegionalHostname.RegionKey { - return nil, fmt.Errorf("conflicting region keys for regional hostname %q: %q and %q", change.RegionalHostname.Hostname, regionalHostname.RegionKey, change.RegionalHostname.RegionKey) - } - if (change.Action == cloudFlareUpdate && regionalHostname.action != cloudFlareUpdate) || - regionalHostname.action == cloudFlareDelete { - regionalHostnameChanges[change.RegionalHostname.Hostname] = regionalHostnameChange{ - action: cloudFlareUpdate, - RegionalHostname: change.RegionalHostname, - } - } - case cloudFlareDelete: - if !ok { - regionalHostnameChanges[change.RegionalHostname.Hostname] = regionalHostnameChange{ - action: cloudFlareDelete, - RegionalHostname: change.RegionalHostname, - } - continue - } + if change.Action == cloudFlareDelete { + // A previous regional hostname exists so we can skip this delete action + continue + } + if rh.RegionKey == "" { + // If the existing regional hostname has no region key, we can overwrite it + rhs[change.RegionalHostname.Hostname] = change.RegionalHostname + continue + } + if rh.RegionKey != change.RegionalHostname.RegionKey { + return nil, fmt.Errorf("conflicting region keys for regional hostname %q: %q and %q", change.RegionalHostname.Hostname, rh.RegionKey, change.RegionalHostname.RegionKey) } } - return slices.Collect(maps.Values(regionalHostnameChanges)), nil + return slices.Collect(maps.Values(rhs)), nil +} + +// regionalHostnamesChanges build a list of changes needed to synchronize the current regional hostnames state with the desired state. +func regionalHostnamesChanges(desired []cloudflare.RegionalHostname, regionalHostnames RegionalHostnamesMap) []regionalHostnameChange { + changes := make([]regionalHostnameChange, 0) + for _, rh := range desired { + current, found := regionalHostnames[rh.Hostname] + if rh.RegionKey == "" { + // If the region key is empty, we don't want a regional hostname + if !found { + continue + } + changes = append(changes, regionalHostnameChange{ + action: cloudFlareDelete, + RegionalHostname: rh, + }) + continue + } + if !found { + changes = append(changes, regionalHostnameChange{ + action: cloudFlareCreate, + RegionalHostname: rh, + }) + continue + } + if rh.RegionKey != current.RegionKey { + changes = append(changes, regionalHostnameChange{ + action: cloudFlareUpdate, + RegionalHostname: rh, + }) + } + } + return changes } diff --git a/provider/cloudflare/cloudflare_regional_test.go b/provider/cloudflare/cloudflare_regional_test.go index 0a79647f6..fbbca0d90 100644 --- a/provider/cloudflare/cloudflare_regional_test.go +++ b/provider/cloudflare/cloudflare_regional_test.go @@ -17,21 +17,326 @@ limitations under the License. package cloudflare import ( - "reflect" - "slices" + "context" + "fmt" + "sort" + "strings" "testing" - "github.com/cloudflare/cloudflare-go" + cloudflare "github.com/cloudflare/cloudflare-go" + log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "sigs.k8s.io/external-dns/endpoint" + "sigs.k8s.io/external-dns/internal/testutils" + "sigs.k8s.io/external-dns/plan" ) +func (m *mockCloudFlareClient) ListDataLocalizationRegionalHostnames(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.ListDataLocalizationRegionalHostnamesParams) ([]cloudflare.RegionalHostname, error) { + if strings.Contains(rc.Identifier, "rherror") { + return nil, fmt.Errorf("failed to list regional hostnames") + } + return m.regionalHostnames[rc.Identifier], nil +} + +func (m *mockCloudFlareClient) CreateDataLocalizationRegionalHostname(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.CreateDataLocalizationRegionalHostnameParams) error { + if strings.Contains(rp.Hostname, "rherror") { + return fmt.Errorf("failed to create regional hostname") + } + + m.Actions = append(m.Actions, MockAction{ + Name: "CreateDataLocalizationRegionalHostname", + ZoneId: rc.Identifier, + RecordId: "", + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: rp.Hostname, + RegionKey: rp.RegionKey, + }, + }) + return nil +} + +func (m *mockCloudFlareClient) UpdateDataLocalizationRegionalHostname(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.UpdateDataLocalizationRegionalHostnameParams) error { + if strings.Contains(rp.Hostname, "rherror") { + return fmt.Errorf("failed to update regional hostname") + } + + m.Actions = append(m.Actions, MockAction{ + Name: "UpdateDataLocalizationRegionalHostname", + ZoneId: rc.Identifier, + RecordId: "", + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: rp.Hostname, + RegionKey: rp.RegionKey, + }, + }) + return nil +} + +func (m *mockCloudFlareClient) DeleteDataLocalizationRegionalHostname(ctx context.Context, rc *cloudflare.ResourceContainer, hostname string) error { + if strings.Contains(hostname, "rherror") { + return fmt.Errorf("failed to delete regional hostname") + } + m.Actions = append(m.Actions, MockAction{ + Name: "DeleteDataLocalizationRegionalHostname", + ZoneId: rc.Identifier, + RecordId: "", + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: hostname, + }, + }) + return nil +} + +func TestCloudflareRegionalHostnameActions(t *testing.T) { + tests := []struct { + name string + records map[string]cloudflare.DNSRecord + regionalHostnames []cloudflare.RegionalHostname + endpoints []*endpoint.Endpoint + want []MockAction + }{ + { + name: "create", + records: map[string]cloudflare.DNSRecord{}, + regionalHostnames: []cloudflare.RegionalHostname{}, + endpoints: []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "create.bar.com", + Targets: endpoint.Targets{"127.0.0.1"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", + Value: "eu", + }, + }, + }, + }, + want: []MockAction{ + { + Name: "Create", + ZoneId: "001", + RecordId: generateDNSRecordID("A", "create.bar.com", "127.0.0.1"), + RecordData: cloudflare.DNSRecord{ + ID: generateDNSRecordID("A", "create.bar.com", "127.0.0.1"), + Type: "A", + Name: "create.bar.com", + Content: "127.0.0.1", + TTL: 1, + Proxied: proxyDisabled, + }, + }, + { + Name: "CreateDataLocalizationRegionalHostname", + ZoneId: "001", + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "create.bar.com", + RegionKey: "eu", + }, + }, + }, + }, + { + name: "Update", + records: map[string]cloudflare.DNSRecord{ + "update.bar.com": { + ID: generateDNSRecordID("A", "update.bar.com", "127.0.0.1"), + Type: "A", + Name: "update.bar.com", + Content: "127.0.0.1", + TTL: 1, + Proxied: proxyDisabled, + }, + }, + regionalHostnames: []cloudflare.RegionalHostname{ + { + Hostname: "update.bar.com", + RegionKey: "us", + }, + }, + endpoints: []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "update.bar.com", + Targets: endpoint.Targets{"127.0.0.1"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", + Value: "eu", + }, + }, + }, + }, + want: []MockAction{ + { + Name: "Update", + ZoneId: "001", + RecordId: generateDNSRecordID("A", "update.bar.com", "127.0.0.1"), + RecordData: cloudflare.DNSRecord{ + ID: generateDNSRecordID("A", "update.bar.com", "127.0.0.1"), + Type: "A", + Name: "update.bar.com", + Content: "127.0.0.1", + TTL: 1, + Proxied: proxyDisabled, + }, + }, + { + Name: "UpdateDataLocalizationRegionalHostname", + ZoneId: "001", + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "update.bar.com", + RegionKey: "eu", + }, + }, + }, + }, + { + name: "Delete", + records: map[string]cloudflare.DNSRecord{ + "update.bar.com": { + ID: generateDNSRecordID("A", "delete.bar.com", "127.0.0.1"), + Type: "A", + Name: "delete.bar.com", + Content: "127.0.0.1", + TTL: 1, + Proxied: proxyDisabled, + }, + }, + regionalHostnames: []cloudflare.RegionalHostname{ + { + Hostname: "delete.bar.com", + RegionKey: "us", + }, + }, + endpoints: []*endpoint.Endpoint{}, + want: []MockAction{ + { + Name: "Delete", + ZoneId: "001", + RecordId: generateDNSRecordID("A", "delete.bar.com", "127.0.0.1"), + RecordData: cloudflare.DNSRecord{}, + }, + { + Name: "DeleteDataLocalizationRegionalHostname", + ZoneId: "001", + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "delete.bar.com", + }, + }, + }, + }, + { + name: "No change", + records: map[string]cloudflare.DNSRecord{ + "nochange.bar.com": { + ID: generateDNSRecordID("A", "nochange.bar.com", "127.0.0.1"), + Type: "A", + Name: "nochange.bar.com", + Content: "127.0.0.1", + TTL: 1, + Proxied: proxyDisabled, + }, + }, + regionalHostnames: []cloudflare.RegionalHostname{ + { + Hostname: "nochange.bar.com", + RegionKey: "eu", + }, + }, + endpoints: []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "nochange.bar.com", + Targets: endpoint.Targets{"127.0.0.1"}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", + Value: "eu", + }, + }, + }, + }, + want: nil, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + provider := &CloudFlareProvider{ + RegionalServicesConfig: RegionalServicesConfig{Enabled: true, RegionKey: "us"}, + Client: &mockCloudFlareClient{ + Zones: map[string]string{ + "001": "bar.com", + }, + Records: map[string]map[string]cloudflare.DNSRecord{ + "001": tt.records, + }, + regionalHostnames: map[string][]cloudflare.RegionalHostname{ + "001": tt.regionalHostnames, + }, + }, + } + + AssertActions(t, provider, tt.endpoints, tt.want, []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME}) + }) + } +} + +func TestCloudflareRegionalHostnameDefaults(t *testing.T) { + endpoints := []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "bar.com", + Targets: endpoint.Targets{"127.0.0.1", "127.0.0.2"}, + }, + } + + AssertActions(t, &CloudFlareProvider{RegionalServicesConfig: RegionalServicesConfig{Enabled: true, RegionKey: "us"}}, endpoints, []MockAction{ + { + Name: "Create", + ZoneId: "001", + RecordId: generateDNSRecordID("A", "bar.com", "127.0.0.1"), + RecordData: cloudflare.DNSRecord{ + ID: generateDNSRecordID("A", "bar.com", "127.0.0.1"), + Type: "A", + Name: "bar.com", + Content: "127.0.0.1", + TTL: 1, + Proxied: proxyDisabled, + }, + }, + { + Name: "Create", + ZoneId: "001", + RecordId: generateDNSRecordID("A", "bar.com", "127.0.0.2"), + RecordData: cloudflare.DNSRecord{ + ID: generateDNSRecordID("A", "bar.com", "127.0.0.2"), + Type: "A", + Name: "bar.com", + Content: "127.0.0.2", + TTL: 1, + Proxied: proxyDisabled, + }, + }, + { + Name: "CreateDataLocalizationRegionalHostname", + ZoneId: "001", + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "bar.com", + RegionKey: "us", + }, + }, + }, + []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME}, + ) +} + func Test_regionalHostname(t *testing.T) { type args struct { - endpoint *endpoint.Endpoint - defaultRegionKey string + endpoint *endpoint.Endpoint + config RegionalServicesConfig } tests := []struct { name string @@ -45,9 +350,15 @@ func Test_regionalHostname(t *testing.T) { RecordType: "A", DNSName: "example.com", }, - defaultRegionKey: "", + config: RegionalServicesConfig{ + Enabled: true, + RegionKey: "", + }, + }, + want: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "", }, - want: cloudflare.RegionalHostname{}, }, { name: "default region key", @@ -56,7 +367,10 @@ func Test_regionalHostname(t *testing.T) { RecordType: "A", DNSName: "example.com", }, - defaultRegionKey: "us", + config: RegionalServicesConfig{ + Enabled: true, + RegionKey: "us", + }, }, want: cloudflare.RegionalHostname{ Hostname: "example.com", @@ -76,7 +390,10 @@ func Test_regionalHostname(t *testing.T) { }, }, }, - defaultRegionKey: "us", + config: RegionalServicesConfig{ + Enabled: true, + RegionKey: "us", + }, }, want: cloudflare.RegionalHostname{ Hostname: "example.com", @@ -96,7 +413,10 @@ func Test_regionalHostname(t *testing.T) { }, }, }, - defaultRegionKey: "us", + config: RegionalServicesConfig{ + Enabled: true, + RegionKey: "us", + }, }, want: cloudflare.RegionalHostname{ Hostname: "example.com", @@ -116,258 +436,741 @@ func Test_regionalHostname(t *testing.T) { }, }, }, - defaultRegionKey: "us", + config: RegionalServicesConfig{ + Enabled: true, + RegionKey: "us", + }, }, want: cloudflare.RegionalHostname{}, }, + { + name: "disabled", + args: args{ + endpoint: &endpoint.Endpoint{ + RecordType: "A", + DNSName: "example.com", + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", + Value: "us", + }, + }, + }, + config: RegionalServicesConfig{ + Enabled: false, + }, + }, + want: cloudflare.RegionalHostname{ + Hostname: "", + RegionKey: "", + }, + }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - p := CloudFlareProvider{RegionKey: tt.args.defaultRegionKey} + p := CloudFlareProvider{RegionalServicesConfig: tt.args.config} got := p.regionalHostname(tt.args.endpoint) assert.Equal(t, tt.want, got) }) } } -func Test_dataLocalizationRegionalHostnamesChanges(t *testing.T) { - cmpDataLocalizationRegionalHostnameChange := func(i, j regionalHostnameChange) int { - if i.action == j.action { - return 0 - } - if i.Hostname < j.Hostname { - return -1 - } - return 1 - } - type args struct { - changes []*cloudFlareChange - } +func Test_desiredDataLocalizationRegionalHostnames(t *testing.T) { tests := []struct { name string - args args - want []regionalHostnameChange + changes []*cloudFlareChange + want []cloudflare.RegionalHostname wantErr bool }{ { - name: "empty input", - args: args{ - changes: []*cloudFlareChange{}, - }, + name: "empty input", + changes: []*cloudFlareChange{}, want: nil, wantErr: false, }, { - name: "changes without RegionalHostname", - args: args{ - changes: []*cloudFlareChange{ - { - Action: cloudFlareCreate, - ResourceRecord: cloudflare.DNSRecord{ - Name: "example.com", - }, - RegionalHostname: cloudflare.RegionalHostname{}, // Empty - }, - }, - }, + name: "change without regional hostname config", + changes: []*cloudFlareChange{{ + Action: cloudFlareCreate, + }}, want: nil, wantErr: false, }, { - name: "change with empty RegionKey", - args: args{ - changes: []*cloudFlareChange{ - { - Action: cloudFlareCreate, - ResourceRecord: cloudflare.DNSRecord{ - Name: "example.com", - }, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example.com", - RegionKey: "", // Empty region key - }, - }, - }, - }, - wantErr: true, - }, - { - name: "conflicting region keys", - args: args{ - changes: []*cloudFlareChange{ - { - Action: cloudFlareCreate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example.com", - RegionKey: "eu", - }, - }, - { - Action: cloudFlareCreate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example.com", - RegionKey: "us", // Different region key for same hostname - }, - }, - }, - }, - wantErr: true, - }, - { - name: "update takes precedence over create & delete", - args: args{ - changes: []*cloudFlareChange{ - { - Action: cloudFlareCreate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example.com", - RegionKey: "eu", - }, - }, - { - Action: cloudFlareUpdate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example.com", - RegionKey: "eu", - }, - }, - { - Action: cloudFlareDelete, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example.com", - RegionKey: "eu", - }, - }, - }, - }, - want: []regionalHostnameChange{ + name: "changes with same hostname and region key", + changes: []*cloudFlareChange{ { - action: cloudFlareUpdate, + Action: cloudFlareCreate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "eu", + }, + }, + { + Action: cloudFlareUpdate, RegionalHostname: cloudflare.RegionalHostname{ Hostname: "example.com", RegionKey: "eu", }, }, }, + want: []cloudflare.RegionalHostname{ + { + Hostname: "example.com", + RegionKey: "eu", + }, + }, wantErr: false, }, { - name: "create after delete becomes update", - args: args{ - changes: []*cloudFlareChange{ - { - Action: cloudFlareDelete, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example.com", - RegionKey: "eu", - }, - }, - { - Action: cloudFlareCreate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example.com", - RegionKey: "eu", - }, - }, - }, - }, - want: []regionalHostnameChange{ + name: "changes with same hostname but different region keys", + changes: []*cloudFlareChange{ { - action: cloudFlareUpdate, + Action: cloudFlareCreate, RegionalHostname: cloudflare.RegionalHostname{ Hostname: "example.com", RegionKey: "eu", }, }, - }, - wantErr: false, - }, - { - name: "consolidate mixed actions for different hostnames", - args: args{ - changes: []*cloudFlareChange{ - { - Action: cloudFlareCreate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example1.com", - RegionKey: "eu", - }, - }, - { - Action: cloudFlareUpdate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example2.com", - RegionKey: "us", - }, - }, - { - Action: cloudFlareDelete, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example3.com", - RegionKey: "ap", - }, - }, - // duplicated actions - { - Action: cloudFlareCreate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example1.com", - RegionKey: "eu", - }, - }, - { - Action: cloudFlareUpdate, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example2.com", - RegionKey: "us", - }, - }, - { - Action: cloudFlareDelete, - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "example3.com", - RegionKey: "ap", - }, + { + Action: cloudFlareUpdate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "us", // Different region key }, }, }, - want: []regionalHostnameChange{ + want: nil, + wantErr: true, + }, + { + name: "changes with different hostnames", + changes: []*cloudFlareChange{ { - action: cloudFlareCreate, + Action: cloudFlareCreate, RegionalHostname: cloudflare.RegionalHostname{ Hostname: "example1.com", RegionKey: "eu", }, }, { - action: cloudFlareUpdate, + Action: cloudFlareUpdate, RegionalHostname: cloudflare.RegionalHostname{ Hostname: "example2.com", RegionKey: "us", }, }, { - action: cloudFlareDelete, + Action: cloudFlareDelete, RegionalHostname: cloudflare.RegionalHostname{ Hostname: "example3.com", - RegionKey: "ap", + RegionKey: "us", }, }, }, + want: []cloudflare.RegionalHostname{ + { + Hostname: "example1.com", + RegionKey: "eu", + }, + { + Hostname: "example2.com", + RegionKey: "us", + }, + { + Hostname: "example3.com", + RegionKey: "", + }, + }, + wantErr: false, + }, + { + name: "change with empty region key", + changes: []*cloudFlareChange{ + { + Action: cloudFlareCreate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "", // Empty region key + }, + }, + }, + want: []cloudflare.RegionalHostname{ + { + Hostname: "example.com", + RegionKey: "", + }, + }, + wantErr: false, + }, + { + name: "empty region key followed by region key", + changes: []*cloudFlareChange{ + { + Action: cloudFlareCreate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "", // Empty region key + }, + }, + { + Action: cloudFlareUpdate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "eu", + }, + }, + }, + want: []cloudflare.RegionalHostname{ + { + Hostname: "example.com", + RegionKey: "eu", + }, + }, + wantErr: false, + }, + { + name: "region key followed by empty region key", + changes: []*cloudFlareChange{ + { + Action: cloudFlareCreate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "eu", + }, + }, + { + Action: cloudFlareUpdate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "eu", // Empty region key + }, + }, + }, + want: []cloudflare.RegionalHostname{ + { + Hostname: "example.com", + RegionKey: "eu", + }, + }, + wantErr: false, + }, + { + name: "delete followed by create for the same hostname", + changes: []*cloudFlareChange{ + { + Action: cloudFlareDelete, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "eu", + }, + }, + { + Action: cloudFlareCreate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "eu", + }, + }, + }, + want: []cloudflare.RegionalHostname{ + { + Hostname: "example.com", + RegionKey: "eu", + }, + }, + wantErr: false, + }, + { + name: "create followed by delete for the same hostname", + changes: []*cloudFlareChange{ + { + Action: cloudFlareCreate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "eu", + }, + }, + { + Action: cloudFlareDelete, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "example.com", + RegionKey: "eu", + }, + }, + }, + want: []cloudflare.RegionalHostname{ + { + Hostname: "example.com", + RegionKey: "eu", + }, + }, wantErr: false, }, } + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - got, err := dataLocalizationRegionalHostnamesChanges(tt.args.changes) - if (err != nil) != tt.wantErr { - t.Errorf("dataLocalizationRegionalHostnamesChanges() error = %v, wantErr %v", err, tt.wantErr) - return + got, err := desiredRegionalHostnames(tt.changes) + if tt.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) } - slices.SortFunc(got, cmpDataLocalizationRegionalHostnameChange) - slices.SortFunc(tt.want, cmpDataLocalizationRegionalHostnameChange) - if !reflect.DeepEqual(got, tt.want) { - t.Errorf("dataLocalizationRegionalHostnamesChanges() = %v, want %v", got, tt.want) + sort.Slice(got, func(i, j int) bool { + return got[i].Hostname < got[j].Hostname + }) + sort.Slice(tt.want, func(i, j int) bool { + return tt.want[i].Hostname < tt.want[j].Hostname + }) + assert.Equal(t, tt.want, got) + }) + } +} + +func Test_dataLocalizationRegionalHostnamesChanges(t *testing.T) { + tests := []struct { + name string + desired []cloudflare.RegionalHostname + regionalHostnames RegionalHostnamesMap + want []regionalHostnameChange + }{ + { + name: "empty desired and current lists", + desired: []cloudflare.RegionalHostname{}, + regionalHostnames: RegionalHostnamesMap{}, + want: []regionalHostnameChange{}, + }, + { + name: "multiple changes", + desired: []cloudflare.RegionalHostname{ + { + Hostname: "create.example.com", + RegionKey: "eu", + }, + { + Hostname: "update.example.com", + RegionKey: "eu", + }, + { + Hostname: "delete.example.com", + RegionKey: "", + }, + { + Hostname: "nochange.example.com", + RegionKey: "us", + }, + { + Hostname: "absent.example.com", + RegionKey: "", + }, + }, + regionalHostnames: RegionalHostnamesMap{ + "update.example.com": cloudflare.RegionalHostname{ + Hostname: "update.example.com", + RegionKey: "us", + }, + "delete.example.com": cloudflare.RegionalHostname{ + Hostname: "delete.example.com", + RegionKey: "ap", + }, + "nochange.example.com": cloudflare.RegionalHostname{ + Hostname: "nochange.example.com", + RegionKey: "us", + }, + }, + want: []regionalHostnameChange{ + { + action: cloudFlareCreate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "create.example.com", + RegionKey: "eu", + }, + }, + { + action: cloudFlareUpdate, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "update.example.com", + RegionKey: "eu", + }, + }, + { + action: cloudFlareDelete, + RegionalHostname: cloudflare.RegionalHostname{ + Hostname: "delete.example.com", + RegionKey: "", + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := regionalHostnamesChanges(tt.desired, tt.regionalHostnames) + assert.Equal(t, tt.want, got) + }) + } +} + +func TestRecordsWithListRegionalHostnameFaillure(t *testing.T) { + client := &mockCloudFlareClient{ + Zones: map[string]string{ + "rherror": "error.com", + }, + Records: map[string]map[string]cloudflare.DNSRecord{ + "rherror": {"foo.error.com": {Type: "A"}}, + }, + } + failingProvider := &CloudFlareProvider{ + Client: client, + RegionalServicesConfig: RegionalServicesConfig{Enabled: true}, + } + _, err := failingProvider.Records(t.Context()) + assert.Error(t, err, "listing regional hostnames should fail") +} + +func TestApplyChangesWithRegionalHostnamesFaillures(t *testing.T) { + t.Parallel() + type fields struct { + Records map[string]cloudflare.DNSRecord + RegionalHostnames []cloudflare.RegionalHostname + RegionKey string + } + type args struct { + changes *plan.Changes + } + tests := []struct { + name string + fields fields + args args + errMsg string + expectDebug string + }{ + { + name: "list zone fails", + args: args{ + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "foo.error.com", + Targets: endpoint.Targets{"127.0.0.1"}, + ProviderSpecific: endpoint.ProviderSpecific{ + {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + }, + }, + }, + }, + }, + errMsg: "failed to list regional hostnames", + }, + { + name: "create fails", + fields: fields{ + Records: map[string]cloudflare.DNSRecord{}, + RegionalHostnames: []cloudflare.RegionalHostname{}, + RegionKey: "us", + }, + args: args{ + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "rherror.bar.com", + Targets: endpoint.Targets{"127.0.0.1"}, + ProviderSpecific: endpoint.ProviderSpecific{ + {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + }, + }, + }, + }, + }, + expectDebug: "failed to create regional hostname", + }, + { + name: "update fails", + fields: fields{ + Records: map[string]cloudflare.DNSRecord{ + "rherror.bar.com": { + ID: "123", + Type: "A", + Name: "rherror.bar.com", + Content: "127.0.0.1", + }, + }, + RegionalHostnames: []cloudflare.RegionalHostname{ + {Hostname: "rherror.bar.com", RegionKey: "us"}, + }, + RegionKey: "us", + }, + args: args{ + changes: &plan.Changes{ + UpdateOld: []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "rherror.bar.com", + Targets: endpoint.Targets{"127.0.0.1"}, + ProviderSpecific: endpoint.ProviderSpecific{ + {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + }, + }, + }, + UpdateNew: []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "rherror.bar.com", + Targets: endpoint.Targets{"127.0.0.2"}, + ProviderSpecific: endpoint.ProviderSpecific{ + {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + }, + }, + }, + }, + }, + expectDebug: "failed to update regional hostname", + }, + { + name: "delete fails", + fields: fields{ + Records: map[string]cloudflare.DNSRecord{ + "rherror.bar.com": { + ID: "123", + Type: "A", + Name: "newerror.bar.com", + Content: "127.0.0.1", + }, + }, + RegionalHostnames: []cloudflare.RegionalHostname{ + {Hostname: "rherror.bar.com", RegionKey: "us"}, + }, + RegionKey: "us", + }, + args: args{ + changes: &plan.Changes{ + Delete: []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "rherror.bar.com", + Targets: endpoint.Targets{"127.0.0.1"}, + }, + }, + }, + }, + expectDebug: "failed to delete regional hostname", + }, + { + // This should not happen in practice, but we test it to ensure we return an error. + name: "conflicting regional keys", + fields: fields{ + Records: map[string]cloudflare.DNSRecord{}, + RegionalHostnames: []cloudflare.RegionalHostname{}, + RegionKey: "us", + }, + args: args{ + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "foo.bar.com", + Targets: endpoint.Targets{"127.0.0.1"}, + ProviderSpecific: endpoint.ProviderSpecific{ + {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + }, + }, + { + RecordType: "A", + DNSName: "foo.bar.com", + Targets: endpoint.Targets{"127.0.0.1"}, + ProviderSpecific: endpoint.ProviderSpecific{ + {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "us"}, + }, + }, + }, + }, + }, + errMsg: "conflicting region keys", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + records := tt.fields.Records + if records == nil { + records = map[string]cloudflare.DNSRecord{} + } + p := &CloudFlareProvider{ + Client: &mockCloudFlareClient{ + Zones: map[string]string{ + "001": "bar.com", + "rherror": "error.com", + }, + Records: map[string]map[string]cloudflare.DNSRecord{ + "001": records, + }, + regionalHostnames: map[string][]cloudflare.RegionalHostname{ + "001": tt.fields.RegionalHostnames, + }, + }, + RegionalServicesConfig: RegionalServicesConfig{ + Enabled: true, + RegionKey: tt.fields.RegionKey, + }, + } + hook := testutils.LogsUnderTestWithLogLevel(log.DebugLevel, t) + err := p.ApplyChanges(t.Context(), tt.args.changes) + assert.Error(t, err, "ApplyChanges should return an error") + if tt.errMsg != "" && err != nil { + assert.Contains(t, err.Error(), tt.errMsg, "Expected error message to contain: %s", tt.errMsg) + } + if tt.expectDebug != "" { + testutils.TestHelperLogContains(tt.expectDebug, hook, t) + } + }) + } +} + +func TestApplyChangesWithRegionalHostnamesDryRun(t *testing.T) { + t.Parallel() + type fields struct { + Records map[string]cloudflare.DNSRecord + RegionalHostnames []cloudflare.RegionalHostname + RegionKey string + } + type args struct { + changes *plan.Changes + } + tests := []struct { + name string + fields fields + args args + expectDebug string + }{ + { + name: "create dry run", + fields: fields{ + Records: map[string]cloudflare.DNSRecord{}, + RegionalHostnames: []cloudflare.RegionalHostname{}, + RegionKey: "us", + }, + args: args{ + changes: &plan.Changes{ + Create: []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "foo.bar.com", + Targets: endpoint.Targets{"127.0.0.1"}, + ProviderSpecific: endpoint.ProviderSpecific{ + {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + }, + }, + }, + }, + }, + expectDebug: "Dry run: skipping regional hostname change", + }, + { + name: "update fails", + fields: fields{ + Records: map[string]cloudflare.DNSRecord{ + "foo.bar.com": { + ID: "123", + Type: "A", + Name: "foo.bar.com", + Content: "127.0.0.1", + }, + }, + RegionalHostnames: []cloudflare.RegionalHostname{ + {Hostname: "foo.bar.com", RegionKey: "us"}, + }, + RegionKey: "us", + }, + args: args{ + changes: &plan.Changes{ + UpdateOld: []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "foo.bar.com", + Targets: endpoint.Targets{"127.0.0.1"}, + ProviderSpecific: endpoint.ProviderSpecific{ + {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + }, + }, + }, + UpdateNew: []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "foo.bar.com", + Targets: endpoint.Targets{"127.0.0.2"}, + ProviderSpecific: endpoint.ProviderSpecific{ + {Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", Value: "eu"}, + }, + }, + }, + }, + }, + expectDebug: "Dry run: skipping regional hostname change", + }, + { + name: "delete fails", + fields: fields{ + Records: map[string]cloudflare.DNSRecord{ + "foo.bar.com": { + ID: "123", + Type: "A", + Name: "foo.bar.com", + Content: "127.0.0.1", + }, + }, + RegionalHostnames: []cloudflare.RegionalHostname{ + {Hostname: "foo.bar.com", RegionKey: "us"}, + }, + RegionKey: "us", + }, + args: args{ + changes: &plan.Changes{ + Delete: []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "foo.bar.com", + Targets: endpoint.Targets{"127.0.0.1"}, + }, + }, + }, + }, + expectDebug: "Dry run: skipping regional hostname change", + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + records := tt.fields.Records + if records == nil { + records = map[string]cloudflare.DNSRecord{} + } + p := &CloudFlareProvider{ + DryRun: true, + Client: &mockCloudFlareClient{ + Zones: map[string]string{ + "001": "bar.com", + }, + Records: map[string]map[string]cloudflare.DNSRecord{ + "001": records, + }, + regionalHostnames: map[string][]cloudflare.RegionalHostname{ + "001": tt.fields.RegionalHostnames, + }, + }, + RegionalServicesConfig: RegionalServicesConfig{ + Enabled: true, + RegionKey: tt.fields.RegionKey, + }, + } + hook := testutils.LogsUnderTestWithLogLevel(log.DebugLevel, t) + err := p.ApplyChanges(t.Context(), tt.args.changes) + assert.NoError(t, err, "ApplyChanges should not fail") + if tt.expectDebug != "" { + testutils.TestHelperLogContains(tt.expectDebug, hook, t) } }) } diff --git a/provider/cloudflare/cloudflare_test.go b/provider/cloudflare/cloudflare_test.go index b07eb7ec2..7babb58ac 100644 --- a/provider/cloudflare/cloudflare_test.go +++ b/provider/cloudflare/cloudflare_test.go @@ -30,7 +30,6 @@ import ( "github.com/maxatome/go-testdeep/td" log "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" - "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/internal/testutils" "sigs.k8s.io/external-dns/plan" @@ -55,6 +54,7 @@ type mockCloudFlareClient struct { listZonesContextError error dnsRecordsError error customHostnames map[string][]cloudflare.CustomHostname + regionalHostnames map[string][]cloudflare.RegionalHostname } var ExampleDomain = []cloudflare.DNSRecord{ @@ -95,7 +95,8 @@ func NewMockCloudFlareClient() *mockCloudFlareClient { "001": {}, "002": {}, }, - customHostnames: map[string][]cloudflare.CustomHostname{}, + customHostnames: map[string][]cloudflare.CustomHostname{}, + regionalHostnames: map[string][]cloudflare.RegionalHostname{}, } } @@ -227,43 +228,6 @@ func (m *mockCloudFlareClient) UpdateDNSRecord(ctx context.Context, rc *cloudfla return nil } -func (m *mockCloudFlareClient) CreateDataLocalizationRegionalHostname(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.CreateDataLocalizationRegionalHostnameParams) error { - m.Actions = append(m.Actions, MockAction{ - Name: "CreateDataLocalizationRegionalHostname", - ZoneId: rc.Identifier, - RecordId: "", - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: rp.Hostname, - RegionKey: rp.RegionKey, - }, - }) - return nil -} - -func (m *mockCloudFlareClient) UpdateDataLocalizationRegionalHostname(ctx context.Context, rc *cloudflare.ResourceContainer, rp cloudflare.UpdateDataLocalizationRegionalHostnameParams) error { - m.Actions = append(m.Actions, MockAction{ - Name: "UpdateDataLocalizationRegionalHostname", - ZoneId: rc.Identifier, - RecordId: "", - RecordData: cloudflare.DNSRecord{ - Name: rp.Hostname, - }, - }) - return nil -} - -func (m *mockCloudFlareClient) DeleteDataLocalizationRegionalHostname(ctx context.Context, rc *cloudflare.ResourceContainer, hostname string) error { - m.Actions = append(m.Actions, MockAction{ - Name: "DeleteDataLocalizationRegionalHostname", - ZoneId: rc.Identifier, - RecordId: "", - RecordData: cloudflare.DNSRecord{ - Name: hostname, - }, - }) - return nil -} - func (m *mockCloudFlareClient) DeleteDNSRecord(ctx context.Context, rc *cloudflare.ResourceContainer, recordID string) error { m.Actions = append(m.Actions, MockAction{ Name: "Delete", @@ -756,110 +720,6 @@ func TestCloudflareSetProxied(t *testing.T) { } } -func TestCloudflareRegionalHostname(t *testing.T) { - endpoints := []*endpoint.Endpoint{ - { - RecordType: "A", - DNSName: "bar.com", - Targets: endpoint.Targets{"127.0.0.1", "127.0.0.2"}, - ProviderSpecific: endpoint.ProviderSpecific{ - { - Name: "external-dns.alpha.kubernetes.io/cloudflare-region-key", - Value: "eu", - }, - }, - }, - } - - AssertActions(t, &CloudFlareProvider{RegionKey: "us"}, endpoints, []MockAction{ - { - Name: "Create", - ZoneId: "001", - RecordId: generateDNSRecordID("A", "bar.com", "127.0.0.1"), - RecordData: cloudflare.DNSRecord{ - ID: generateDNSRecordID("A", "bar.com", "127.0.0.1"), - Type: "A", - Name: "bar.com", - Content: "127.0.0.1", - TTL: 1, - Proxied: proxyDisabled, - }, - }, - { - Name: "Create", - ZoneId: "001", - RecordId: generateDNSRecordID("A", "bar.com", "127.0.0.2"), - RecordData: cloudflare.DNSRecord{ - ID: generateDNSRecordID("A", "bar.com", "127.0.0.2"), - Type: "A", - Name: "bar.com", - Content: "127.0.0.2", - TTL: 1, - Proxied: proxyDisabled, - }, - }, - { - Name: "CreateDataLocalizationRegionalHostname", - ZoneId: "001", - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "bar.com", - RegionKey: "eu", - }, - }, - }, - []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME}, - ) -} - -func TestCloudflareRegionalHostnameDefaults(t *testing.T) { - endpoints := []*endpoint.Endpoint{ - { - RecordType: "A", - DNSName: "bar.com", - Targets: endpoint.Targets{"127.0.0.1", "127.0.0.2"}, - }, - } - - AssertActions(t, &CloudFlareProvider{RegionKey: "us"}, endpoints, []MockAction{ - { - Name: "Create", - ZoneId: "001", - RecordId: generateDNSRecordID("A", "bar.com", "127.0.0.1"), - RecordData: cloudflare.DNSRecord{ - ID: generateDNSRecordID("A", "bar.com", "127.0.0.1"), - Type: "A", - Name: "bar.com", - Content: "127.0.0.1", - TTL: 1, - Proxied: proxyDisabled, - }, - }, - { - Name: "Create", - ZoneId: "001", - RecordId: generateDNSRecordID("A", "bar.com", "127.0.0.2"), - RecordData: cloudflare.DNSRecord{ - ID: generateDNSRecordID("A", "bar.com", "127.0.0.2"), - Type: "A", - Name: "bar.com", - Content: "127.0.0.2", - TTL: 1, - Proxied: proxyDisabled, - }, - }, - { - Name: "CreateDataLocalizationRegionalHostname", - ZoneId: "001", - RegionalHostname: cloudflare.RegionalHostname{ - Hostname: "bar.com", - RegionKey: "us", - }, - }, - }, - []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME}, - ) -} - func TestCloudflareZones(t *testing.T) { provider := &CloudFlareProvider{ Client: NewMockCloudFlareClient(), @@ -1089,7 +949,7 @@ func TestCloudflareProvider(t *testing.T) { provider.NewZoneIDFilter([]string{""}), false, true, - "", + RegionalServicesConfig{Enabled: false}, CustomHostnamesConfig{Enabled: false}, DNSRecordsConfig{PerPage: 5000, Comment: ""}, ) @@ -1751,24 +1611,20 @@ func TestCustomTTLWithEnabledProxyNotChanged(t *testing.T) { } func TestCloudFlareProvider_Region(t *testing.T) { - _ = os.Setenv("CF_API_TOKEN", "abc123def") - _ = os.Setenv("CF_API_EMAIL", "test@test.com") + t.Setenv("CF_API_TOKEN", "abc123def") + t.Setenv("CF_API_EMAIL", "test@test.com") provider, err := NewCloudFlareProvider( endpoint.NewDomainFilter([]string{"example.com"}), provider.ZoneIDFilter{}, true, false, - "us", + RegionalServicesConfig{Enabled: false, RegionKey: "us"}, CustomHostnamesConfig{Enabled: false}, DNSRecordsConfig{PerPage: 50, Comment: ""}, ) - if err != nil { - t.Fatal(err) - } - - if provider.RegionKey != "us" { - t.Errorf("expected region key to be 'us', but got '%s'", provider.RegionKey) - } + assert.NoError(t, err, "should not fail to create provider") + assert.True(t, provider.RegionalServicesConfig.Enabled, "expect regional services to be enabled") + assert.Equal(t, "us", provider.RegionalServicesConfig.RegionKey, "expected region key to be 'us'") } func TestCloudFlareProvider_newCloudFlareChange(t *testing.T) { @@ -1780,7 +1636,7 @@ func TestCloudFlareProvider_newCloudFlareChange(t *testing.T) { provider.ZoneIDFilter{}, true, false, - "us", + RegionalServicesConfig{Enabled: true, RegionKey: "us"}, CustomHostnamesConfig{Enabled: false}, DNSRecordsConfig{PerPage: 50}, ) @@ -1823,7 +1679,7 @@ func TestCloudFlareProvider_newCloudFlareChange(t *testing.T) { provider.ZoneIDFilter{}, true, false, - "us", + RegionalServicesConfig{Enabled: true, RegionKey: "us"}, CustomHostnamesConfig{Enabled: false}, DNSRecordsConfig{PerPage: 50, Comment: paidValidCommentBuilder.String()}, )