Skip to content

Commit cf1992c

Browse files
7onn7onn
committed
Improve Endpoint prop GET syntax + Test missing coverage
1 parent 2b4c0ee commit cf1992c

File tree

2 files changed

+71
-33
lines changed

2 files changed

+71
-33
lines changed

‎provider/cloudflare/cloudflare.go

Lines changed: 21 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -875,30 +875,35 @@ func (p *CloudFlareProvider) newCloudFlareChange(action string, ep *endpoint.End
875875

876876
comment := p.DNSRecordsConfig.Comment
877877
tags := p.DNSRecordsConfig.GetTags()
878-
for _, providerSpecific := range ep.ProviderSpecific {
879-
if providerSpecific.Name == source.CloudflareRecordCommentKey {
880-
comment = providerSpecific.Value
881-
}
882-
if providerSpecific.Name == source.CloudflareRecordTagsKey {
883-
tags = strings.Split(providerSpecific.Value, ",")
884-
}
878+
antComment, ok := ep.GetProviderSpecificProperty(source.CloudflareRecordCommentKey)
879+
if ok {
880+
comment = antComment
881+
}
882+
883+
if len(comment) > paidZoneCommentMaxLength {
884+
log.Warnf("DNS record comment is limited to %d chars, trimming comment of %s. Please set it to less than %d for free zones and less than %d for paid zones, to avoid endless syncs.", paidZoneCommentMaxLength, ep.DNSName, freeZoneCommentMaxLength, paidZoneCommentMaxLength)
885+
comment = comment[:paidZoneCommentMaxLength-1]
886+
}
887+
888+
antTags, ok := ep.GetProviderSpecificProperty(source.CloudflareRecordTagsKey)
889+
if ok {
890+
tags = strings.Split(antTags, ",")
885891
}
886-
sort.Strings(tags)
887892

888893
// Free account checks
889894
if tags != nil || len(comment) > freeZoneCommentMaxLength {
890-
freeAccount := !p.ZoneHasPaidPlan(ep.DNSName)
891-
if freeAccount && tags != nil {
892-
log.Infof("DNS tags are only available for paid accounts, skipping for %s", ep.DNSName)
895+
free := !p.ZoneHasPaidPlan(ep.DNSName)
896+
if free && tags != nil {
897+
log.Warnf("DNS tags are only available for paid accounts, skipping for %s. Please remove it from the config to avoid endless syncs.", ep.DNSName)
893898
tags = nil
894899
}
895-
if freeAccount && len(comment) > freeZoneCommentMaxLength {
896-
log.Infof("DNS record comment is limited to %d chars for free zones, trimming comment of %s", freeZoneCommentMaxLength, ep.DNSName)
900+
if free && len(comment) > freeZoneCommentMaxLength {
901+
log.Warnf("DNS record comment is limited to %d chars for free zones, trimming comment of %s. Please set it to less than %d characters to avoid endless syncs.", freeZoneCommentMaxLength, ep.DNSName, freeZoneCommentMaxLength)
897902
comment = comment[:freeZoneCommentMaxLength-1]
898903
}
899-
if len(comment) > paidZoneCommentMaxLength {
900-
log.Infof("DNS record comment is limited to %d chars, trimming comment of %s", paidZoneCommentMaxLength, ep.DNSName)
901-
comment = comment[:paidZoneCommentMaxLength-1]
904+
905+
if len(tags) > 0 {
906+
sort.Strings(tags)
902907
}
903908
}
904909

‎provider/cloudflare/cloudflare_test.go

Lines changed: 50 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ import (
3535
"sigs.k8s.io/external-dns/internal/testutils"
3636
"sigs.k8s.io/external-dns/plan"
3737
"sigs.k8s.io/external-dns/provider"
38+
"sigs.k8s.io/external-dns/source"
3839
)
3940

4041
type MockAction struct {
@@ -905,18 +906,6 @@ func TestCloudflareZones(t *testing.T) {
905906
assert.Equal(t, "bar.com", zones[0].Name)
906907
}
907908

908-
func TestZoneHasPaidPlan(t *testing.T) {
909-
provider := &CloudFlareProvider{
910-
Client: NewMockCloudFlareClient(),
911-
domainFilter: endpoint.NewDomainFilter([]string{"foo.com", "bar.com"}),
912-
zoneIDFilter: provider.NewZoneIDFilter([]string{""}),
913-
}
914-
915-
assert.Equal(t, false, provider.ZoneHasPaidPlan("subdomain.foo.com"))
916-
assert.Equal(t, true, provider.ZoneHasPaidPlan("subdomain.bar.com"))
917-
918-
}
919-
920909
// test failures on zone lookup
921910
func TestCloudflareZonesFailed(t *testing.T) {
922911

@@ -1811,7 +1800,7 @@ func TestCloudFlareProvider_newCloudFlareChange(t *testing.T) {
18111800
_ = os.Setenv("CF_API_KEY", "xxxxxxxxxxxxxxxxx")
18121801
_ = os.Setenv("CF_API_EMAIL", "[email protected]")
18131802
provider, err := NewCloudFlareProvider(
1814-
endpoint.NewDomainFilter([]string{"example.com"}),
1803+
endpoint.NewDomainFilter([]string{"example.com", "bar.com"}),
18151804
provider.ZoneIDFilter{},
18161805
true,
18171806
false,
@@ -1827,16 +1816,60 @@ func TestCloudFlareProvider_newCloudFlareChange(t *testing.T) {
18271816
t.Fatal(err)
18281817
}
18291818

1830-
endpoint := &endpoint.Endpoint{
1819+
endpointFreeZone := &endpoint.Endpoint{
18311820
DNSName: "example.com",
18321821
RecordType: "A",
18331822
Targets: []string{"192.0.2.1"},
18341823
}
1824+
var freeCommentBuilder strings.Builder
1825+
for range freeZoneCommentMaxLength + 1 {
1826+
freeCommentBuilder.WriteString("x")
1827+
}
1828+
endpointFreeZone = endpointFreeZone.WithProviderSpecific(source.CloudflareRecordTagsKey, "non-supported-tag")
1829+
freeComment := freeCommentBuilder.String()
1830+
endpointFreeZone = endpointFreeZone.WithProviderSpecific(source.CloudflareRecordCommentKey, freeComment)
1831+
changeFreeZone := provider.newCloudFlareChange(cloudFlareCreate, endpointFreeZone, endpointFreeZone.Targets[0], nil)
1832+
if changeFreeZone.RegionalHostname.RegionKey != "us" {
1833+
t.Errorf("expected region key to be 'us', but got '%s'", changeFreeZone.RegionalHostname.RegionKey)
1834+
}
1835+
1836+
endpointPaidZone := &endpoint.Endpoint{
1837+
DNSName: "bar.com",
1838+
RecordType: "A",
1839+
Targets: []string{"192.0.2.1"},
1840+
}
1841+
var paidCommentBuilder strings.Builder
1842+
for range paidZoneCommentMaxLength + 1 {
1843+
paidCommentBuilder.WriteString("x")
1844+
}
1845+
endpointPaidZone = endpointPaidZone.WithProviderSpecific(source.CloudflareRecordTagsKey, "kubernetes,external-dns")
1846+
paidComment := paidCommentBuilder.String()
1847+
endpointPaidZone = endpointPaidZone.WithProviderSpecific(source.CloudflareRecordCommentKey, paidComment)
1848+
changePaidZone := provider.newCloudFlareChange(cloudFlareCreate, endpointPaidZone, endpointPaidZone.Targets[0], nil)
1849+
if changePaidZone.RegionalHostname.RegionKey != "us" {
1850+
t.Errorf("expected region key to be 'us', but got '%s'", changePaidZone.RegionalHostname.RegionKey)
1851+
}
1852+
}
1853+
1854+
func TestZoneHasPaidPlan(t *testing.T) {
1855+
client := NewMockCloudFlareClient()
1856+
cfprovider := &CloudFlareProvider{
1857+
Client: client,
1858+
domainFilter: endpoint.NewDomainFilter([]string{"foo.com", "bar.com"}),
1859+
zoneIDFilter: provider.NewZoneIDFilter([]string{""}),
1860+
}
1861+
1862+
assert.Equal(t, false, cfprovider.ZoneHasPaidPlan("subdomain.foo.com"))
1863+
assert.Equal(t, true, cfprovider.ZoneHasPaidPlan("subdomain.bar.com"))
1864+
assert.Equal(t, false, cfprovider.ZoneHasPaidPlan("invaliddomain"))
18351865

1836-
change := provider.newCloudFlareChange(cloudFlareCreate, endpoint, endpoint.Targets[0], nil)
1837-
if change.RegionalHostname.RegionKey != "us" {
1838-
t.Errorf("expected region key to be 'us', but got '%s'", change.RegionalHostname.RegionKey)
1866+
client.zoneDetailsError = errors.New("zone lookup failed")
1867+
cfproviderWithZoneError := &CloudFlareProvider{
1868+
Client: client,
1869+
domainFilter: endpoint.NewDomainFilter([]string{"foo.com", "bar.com"}),
1870+
zoneIDFilter: provider.NewZoneIDFilter([]string{""}),
18391871
}
1872+
assert.Equal(t, false, cfproviderWithZoneError.ZoneHasPaidPlan("subdomain.foo.com"))
18401873
}
18411874

18421875
func TestCloudFlareProvider_submitChangesCNAME(t *testing.T) {

0 commit comments

Comments
 (0)