diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go index fe7d17e8b8..9042201776 100644 --- a/provider/cloudflare/cloudflare.go +++ b/provider/cloudflare/cloudflare.go @@ -197,6 +197,13 @@ func (p *CloudFlareProvider) ZoneHasPaidPlan(hostname string) bool { log.Errorf("Failed to get effective TLD+1 for hostname %s %v", hostname, err) return false } + + if p.zonePlanCache != nil { + if paid, ok := p.zonePlanCache.get(zone); ok { + return paid + } + } + zoneID, err := p.Client.ZoneIDByName(zone) if err != nil { log.Errorf("Failed to get zone %s by name %v", zone, err) @@ -209,7 +216,11 @@ func (p *CloudFlareProvider) ZoneHasPaidPlan(hostname string) bool { return false } - return zoneDetails.Plan.IsSubscribed //nolint:staticcheck // SA1019: Plan.IsSubscribed is deprecated but no replacement available yet + paid := zoneDetails.Plan.IsSubscribed //nolint:staticcheck // SA1019: Plan.IsSubscribed is deprecated but no replacement available yet + if p.zonePlanCache != nil { + p.zonePlanCache.set(zone, paid) + } + return paid } // CloudFlareProvider is an implementation of Provider for CloudFlare DNS. @@ -224,6 +235,10 @@ type CloudFlareProvider struct { CustomHostnamesConfig CustomHostnamesConfig DNSRecordsConfig DNSRecordsConfig RegionalServicesConfig RegionalServicesConfig + // zonePlanCache memoises ZoneHasPaidPlan to avoid firing ListZones+GetZone + // per DNS record change whose comment exceeds the free-zone max length. + // See https://github.com/kubernetes-sigs/external-dns/issues/6391. + zonePlanCache *zonePlanCache } // cloudFlareChange differentiates between ChangeActions @@ -323,6 +338,7 @@ func newProvider( DryRun: dryRun, RegionalServicesConfig: regionalServicesConfig, DNSRecordsConfig: dnsRecordsConfig, + zonePlanCache: newZonePlanCache(defaultZonePlanCacheTTL), }, nil } diff --git a/provider/cloudflare/zone_plan_cache.go b/provider/cloudflare/zone_plan_cache.go new file mode 100644 index 0000000000..9d386d9d2d --- /dev/null +++ b/provider/cloudflare/zone_plan_cache.go @@ -0,0 +1,77 @@ +/* +Copyright 2026 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cloudflare + +import ( + "sync" + "time" +) + +// defaultZonePlanCacheTTL is how long a cached "zone has a paid plan" answer +// is trusted. Zone plan changes are rare (billing action by the account +// owner), so 15 minutes is generous without introducing meaningful staleness. +const defaultZonePlanCacheTTL = 15 * time.Minute + +// zonePlanCache memoises ZoneHasPaidPlan answers. It exists to prevent any +// call site that fires per DNS record change (notably the long-comment +// truncation path) from amplifying into O(n) ListZones+GetZone calls against +// the operator's Cloudflare account every sync cycle. +type zonePlanCache struct { + mu sync.Mutex + entries map[string]zonePlanCacheEntry + ttl time.Duration + now func() time.Time // overridable for tests +} + +type zonePlanCacheEntry struct { + paid bool + cachedAt time.Time +} + +func newZonePlanCache(ttl time.Duration) *zonePlanCache { + return &zonePlanCache{ + entries: make(map[string]zonePlanCacheEntry), + ttl: ttl, + now: time.Now, + } +} + +// get returns the cached "paid" value for zone, or (_, false) if there is no +// unexpired entry. Expired entries are evicted lazily on lookup. +func (c *zonePlanCache) get(zone string) (bool, bool) { + c.mu.Lock() + defer c.mu.Unlock() + entry, ok := c.entries[zone] + if !ok { + return false, false + } + if c.now().Sub(entry.cachedAt) > c.ttl { + delete(c.entries, zone) + return false, false + } + return entry.paid, true +} + +// set stores a fresh answer for zone. +func (c *zonePlanCache) set(zone string, paid bool) { + c.mu.Lock() + defer c.mu.Unlock() + c.entries[zone] = zonePlanCacheEntry{ + paid: paid, + cachedAt: c.now(), + } +} diff --git a/provider/cloudflare/zone_plan_cache_test.go b/provider/cloudflare/zone_plan_cache_test.go new file mode 100644 index 0000000000..a6fd7e9c31 --- /dev/null +++ b/provider/cloudflare/zone_plan_cache_test.go @@ -0,0 +1,109 @@ +/* +Copyright 2026 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package cloudflare + +import ( + "errors" + "sync/atomic" + "testing" + "time" + + "github.com/stretchr/testify/assert" + + "sigs.k8s.io/external-dns/endpoint" + "sigs.k8s.io/external-dns/provider" +) + +func TestZonePlanCache_HitAndMiss(t *testing.T) { + c := newZonePlanCache(time.Minute) + + if _, ok := c.get("foo.com"); ok { + t.Fatalf("expected miss on empty cache") + } + + c.set("foo.com", true) + + paid, ok := c.get("foo.com") + if !ok || !paid { + t.Fatalf("expected hit with paid=true, got paid=%v ok=%v", paid, ok) + } +} + +func TestZonePlanCache_TTLEvicts(t *testing.T) { + var now atomic.Pointer[time.Time] + t0 := time.Unix(0, 0) + now.Store(&t0) + + c := newZonePlanCache(time.Minute) + c.now = func() time.Time { return *now.Load() } + + c.set("foo.com", true) + if _, ok := c.get("foo.com"); !ok { + t.Fatalf("expected hit immediately after set") + } + + // Advance past TTL. + later := t0.Add(2 * time.Minute) + now.Store(&later) + + if _, ok := c.get("foo.com"); ok { + t.Fatalf("expected miss after TTL expiry") + } +} + +// TestZoneHasPaidPlan_Cached verifies the provider answers repeat calls from +// cache rather than hitting ZoneIDByName + GetZone on every call. This is the +// fix for the per-change API amplification flagged in #6391. +func TestZoneHasPaidPlan_Cached(t *testing.T) { + client := NewMockCloudFlareClient() + cfProvider := &CloudFlareProvider{ + Client: client, + domainFilter: endpoint.NewDomainFilter([]string{"foo.com", "bar.com"}), + zoneIDFilter: provider.NewZoneIDFilter([]string{""}), + zonePlanCache: newZonePlanCache(time.Minute), + } + + // First call populates the cache; subsequent calls must not re-fetch. + assert.True(t, cfProvider.ZoneHasPaidPlan("subdomain.bar.com")) + + // Break the API: if cache isn't consulted, ZoneHasPaidPlan falls through + // to ZoneIDByName/GetZone and returns false on error. + client.getZoneError = errors.New("should not be called — cached answer expected") + + for range 10 { + assert.True(t, cfProvider.ZoneHasPaidPlan("subdomain.bar.com"), + "expected cached paid=true; cache miss re-hit the broken API") + } + + // Different hostname in a different zone still uses the API. + assert.False(t, cfProvider.ZoneHasPaidPlan("subdomain.foo.com"), + "expected miss path to surface the API error as paid=false") +} + +// TestZoneHasPaidPlan_NilCache confirms the provider is resilient to a nil +// cache (e.g. hand-constructed in tests without calling newProvider). +func TestZoneHasPaidPlan_NilCache(t *testing.T) { + client := NewMockCloudFlareClient() + cfProvider := &CloudFlareProvider{ + Client: client, + domainFilter: endpoint.NewDomainFilter([]string{"foo.com", "bar.com"}), + zoneIDFilter: provider.NewZoneIDFilter([]string{""}), + } + + assert.True(t, cfProvider.ZoneHasPaidPlan("subdomain.bar.com")) + assert.False(t, cfProvider.ZoneHasPaidPlan("subdomain.foo.com")) +}