Skip to content

Commit 3861441

Browse files
authored
fix(cloudflare): unneeded records updates (#5770)
1 parent fe142a8 commit 3861441

File tree

3 files changed

+130
-7
lines changed

3 files changed

+130
-7
lines changed

provider/cloudflare/cloudflare.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -744,12 +744,7 @@ func (p *CloudFlareProvider) AdjustEndpoints(endpoints []*endpoint.Endpoint) ([]
744744
e.DeleteProviderSpecificProperty(annotations.CloudflareCustomHostnameKey)
745745
}
746746

747-
if p.RegionalServicesConfig.Enabled {
748-
// Add default region key if not set
749-
if _, ok := e.GetProviderSpecificProperty(annotations.CloudflareRegionKey); !ok {
750-
e.SetProviderSpecificProperty(annotations.CloudflareRegionKey, p.RegionalServicesConfig.RegionKey)
751-
}
752-
}
747+
p.adjustEndpointProviderSpecificRegionKeyProperty(e)
753748

754749
adjustedEndpoints = append(adjustedEndpoints, e)
755750
}

provider/cloudflare/cloudflare_regional.go

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -222,13 +222,34 @@ func (p *CloudFlareProvider) addEnpointsProviderSpecificRegionKeyProperty(ctx co
222222
}
223223

224224
for _, ep := range supportedEndpoints {
225+
var regionKey string
225226
if rh, found := regionalHostnames[ep.DNSName]; found {
226-
ep.SetProviderSpecificProperty(annotations.CloudflareRegionKey, rh.regionKey)
227+
regionKey = rh.regionKey
227228
}
229+
ep.SetProviderSpecificProperty(annotations.CloudflareRegionKey, regionKey)
228230
}
229231
return nil
230232
}
231233

234+
// adjustEnpointProviderSpecificRegionKeyProperty updates the given endpoint's provider-specific
235+
// Cloudflare region key based on the provider's RegionalServicesConfig.
236+
// - If regional services are disabled or the endpoint's record type does not
237+
// support regional hostnames, the Cloudflare region key is removed.
238+
// - If enabled and supported, and the key is not already set, it is initialized
239+
// to the provider's default RegionKey.
240+
//
241+
// The endpoint is modified in place and any explicitly set region key is left unchanged.
242+
func (p *CloudFlareProvider) adjustEndpointProviderSpecificRegionKeyProperty(ep *endpoint.Endpoint) {
243+
if !p.RegionalServicesConfig.Enabled || !recordTypeRegionalHostnameSupported[ep.RecordType] {
244+
ep.DeleteProviderSpecificProperty(annotations.CloudflareRegionKey)
245+
return
246+
}
247+
// Add default region key if not set
248+
if _, ok := ep.GetProviderSpecificProperty(annotations.CloudflareRegionKey); !ok {
249+
ep.SetProviderSpecificProperty(annotations.CloudflareRegionKey, p.RegionalServicesConfig.RegionKey)
250+
}
251+
}
252+
232253
// desiredRegionalHostnames builds a list of desired regional hostnames from changes.
233254
//
234255
// If there is a delete and a create or update action for the same hostname,

provider/cloudflare/cloudflare_regional_test.go

Lines changed: 107 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import (
3232
"sigs.k8s.io/external-dns/endpoint"
3333
"sigs.k8s.io/external-dns/internal/testutils"
3434
"sigs.k8s.io/external-dns/plan"
35+
"sigs.k8s.io/external-dns/source/annotations"
3536
)
3637

3738
func (m *mockCloudFlareClient) ListDataLocalizationRegionalHostnames(ctx context.Context, params addressing.RegionalHostnameListParams) autoPager[addressing.RegionalHostnameListResponse] {
@@ -1186,3 +1187,109 @@ func TestApplyChangesWithRegionalHostnamesDryRun(t *testing.T) {
11861187
})
11871188
}
11881189
}
1190+
1191+
func TestCloudflareAdjustEndpointsRegionalServices(t *testing.T) {
1192+
testCases := []struct {
1193+
name string
1194+
recordType string
1195+
regionalServicesConfig RegionalServicesConfig
1196+
initialRegionKey string // existing region key on endpoint
1197+
expectedRegionKey *string // expected region key after AdjustEndpoints (nil = should not be present)
1198+
}{
1199+
// Supported types should get region key when enabled
1200+
{
1201+
name: "A record with regional services enabled",
1202+
recordType: "A",
1203+
regionalServicesConfig: RegionalServicesConfig{Enabled: true, RegionKey: "us"},
1204+
initialRegionKey: "",
1205+
expectedRegionKey: testutils.ToPtr("us"),
1206+
},
1207+
{
1208+
name: "AAAA record with regional services enabled",
1209+
recordType: "AAAA",
1210+
regionalServicesConfig: RegionalServicesConfig{Enabled: true, RegionKey: "us"},
1211+
initialRegionKey: "",
1212+
expectedRegionKey: testutils.ToPtr("us"),
1213+
},
1214+
{
1215+
name: "CNAME record with regional services enabled",
1216+
recordType: "CNAME",
1217+
regionalServicesConfig: RegionalServicesConfig{Enabled: true, RegionKey: "us"},
1218+
initialRegionKey: "",
1219+
expectedRegionKey: testutils.ToPtr("us"),
1220+
},
1221+
1222+
// Unsupported types should NOT get region key even when enabled
1223+
{
1224+
name: "TXT record with regional services enabled",
1225+
recordType: "TXT",
1226+
regionalServicesConfig: RegionalServicesConfig{Enabled: true, RegionKey: "us"},
1227+
initialRegionKey: "",
1228+
expectedRegionKey: nil,
1229+
},
1230+
1231+
// Disabled regional services should remove region key for all types
1232+
{
1233+
name: "A record with regional services disabled",
1234+
recordType: "A",
1235+
regionalServicesConfig: RegionalServicesConfig{Enabled: false},
1236+
initialRegionKey: "existing-region",
1237+
expectedRegionKey: nil,
1238+
},
1239+
{
1240+
name: "TXT record with regional services disabled",
1241+
recordType: "TXT",
1242+
regionalServicesConfig: RegionalServicesConfig{Enabled: false},
1243+
initialRegionKey: "existing-region",
1244+
expectedRegionKey: nil,
1245+
},
1246+
1247+
// Existing region key should be preserved when already set
1248+
{
1249+
name: "A record with existing custom region key",
1250+
recordType: "A",
1251+
regionalServicesConfig: RegionalServicesConfig{Enabled: true, RegionKey: "us"},
1252+
initialRegionKey: "eu",
1253+
expectedRegionKey: testutils.ToPtr("eu"),
1254+
},
1255+
}
1256+
1257+
for _, tc := range testCases {
1258+
t.Run(tc.name, func(t *testing.T) {
1259+
// Create endpoint with initial region key if specified
1260+
testEndpoint := &endpoint.Endpoint{
1261+
RecordType: tc.recordType,
1262+
DNSName: "test.bar.com",
1263+
Targets: endpoint.Targets{"127.0.0.1"},
1264+
}
1265+
1266+
if tc.initialRegionKey != "" {
1267+
testEndpoint.ProviderSpecific = endpoint.ProviderSpecific{
1268+
endpoint.ProviderSpecificProperty{
1269+
Name: annotations.CloudflareRegionKey,
1270+
Value: tc.initialRegionKey,
1271+
},
1272+
}
1273+
}
1274+
1275+
provider := &CloudFlareProvider{
1276+
RegionalServicesConfig: tc.regionalServicesConfig,
1277+
}
1278+
1279+
adjustedEndpoints, err := provider.AdjustEndpoints([]*endpoint.Endpoint{testEndpoint})
1280+
assert.NoError(t, err)
1281+
assert.Len(t, adjustedEndpoints, 1)
1282+
1283+
regionKey, exists := adjustedEndpoints[0].GetProviderSpecificProperty(annotations.CloudflareRegionKey)
1284+
1285+
if tc.expectedRegionKey != nil {
1286+
// Region key should be present with expected value
1287+
assert.True(t, exists, "Region key should be present")
1288+
assert.Equal(t, *tc.expectedRegionKey, regionKey, "Region key value should match expected")
1289+
} else {
1290+
// Region key should not be present
1291+
assert.False(t, exists, "Region key should not be present")
1292+
}
1293+
})
1294+
}
1295+
}

0 commit comments

Comments
 (0)