mirrors/external-dns
1
0
Fork 0
mirror of https://github.com/kubernetes-sigs/external-dns.git synced 2025-08-07 10:06:57 +02:00
Code Issues Packages Projects Releases Wiki Activity

fix(cloudflare): custom hostnames edge-cases causing duplicates (#5183)

Browse Source 
* fix(cloudflare): custom hostnames edge-cases causing duplicates

* syntax/style

* Use %q log fmt for cloudflare provider code

* move custom hostnames related submitChanges() implementation to a separate method submitCustomHostnameChanges(); extend truncated logging

* use maps for DNS records getRecordID() and custom hostnames getCustomHostname() for faster lookups

* types for records/custom hostnames maps

* tidy up using underlying maps for dns records and custom hostnames

* style/naming

* fix private names

* combine unnecessarily separated conditions
This commit is contained in:
mrozentsvayg 2025-03-24 02:06:33 -07:00 committed by GitHub
parent f6d49ddbe8
commit 04f6a99c99
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 285 additions and 125 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

209
provider/cloudflare/cloudflare.go
View File

@ -55,6 +55,22 @@ var proxyEnabled *bool = boolPtr(true)
// proxyDisabled is a pointer to a bool false showing the record should not be proxied through cloudflare // proxyDisabled is a pointer to a bool false showing the record should not be proxied through cloudflare
var proxyDisabled *bool = boolPtr(false) var proxyDisabled *bool = boolPtr(false)
// for faster getRecordID() lookup
type DNSRecordIndex struct {
Name string
Type string
Content string
}
type DNSRecordsMap map[DNSRecordIndex]cloudflare.DNSRecord
// for faster getCustomHostname() lookup
type CustomHostnameIndex struct {
Hostname string
}
type CustomHostnamesMap map[CustomHostnameIndex]cloudflare.CustomHostname
var recordTypeProxyNotSupported = map[string]bool{ var recordTypeProxyNotSupported = map[string]bool{
"LOC": true, "LOC": true,
"MX": true, "MX": true,
@ -229,7 +245,7 @@ func NewCloudFlareProvider(domainFilter endpoint.DomainFilter, zoneIDFilter prov
config, err = cloudflare.New(os.Getenv("CF_API_KEY"), os.Getenv("CF_API_EMAIL")) config, err = cloudflare.New(os.Getenv("CF_API_KEY"), os.Getenv("CF_API_EMAIL"))
} }
if err != nil { if err != nil {
return nil, fmt.Errorf("failed to initialize cloudflare provider: %v", err) return nil, fmt.Errorf("failed to initialize cloudflare provider: %w", err)
} }
provider := &CloudFlareProvider{ provider := &CloudFlareProvider{
// Client: config, // Client: config,
@ -254,10 +270,10 @@ func (p *CloudFlareProvider) Zones(ctx context.Context) ([]cloudflare.Zone, erro
if len(p.zoneIDFilter.ZoneIDs) > 0 && p.zoneIDFilter.ZoneIDs[0] != "" { if len(p.zoneIDFilter.ZoneIDs) > 0 && p.zoneIDFilter.ZoneIDs[0] != "" {
log.Debugln("zoneIDFilter configured. only looking up zone IDs defined") log.Debugln("zoneIDFilter configured. only looking up zone IDs defined")
for _, zoneID := range p.zoneIDFilter.ZoneIDs { for _, zoneID := range p.zoneIDFilter.ZoneIDs {
log.Debugf("looking up zone %s", zoneID) log.Debugf("looking up zone %q", zoneID)
detailResponse, err := p.Client.ZoneDetails(ctx, zoneID) detailResponse, err := p.Client.ZoneDetails(ctx, zoneID)
if err != nil { if err != nil {
log.Errorf("zone %s lookup failed, %v", zoneID, err) log.Errorf("zone %q lookup failed, %v", zoneID, err)
return result, err return result, err
} }
log.WithFields(log.Fields{ log.WithFields(log.Fields{
@ -285,7 +301,7 @@ func (p *CloudFlareProvider) Zones(ctx context.Context) ([]cloudflare.Zone, erro
for _, zone := range zonesResponse.Result { for _, zone := range zonesResponse.Result {
if !p.domainFilter.Match(zone.Name) { if !p.domainFilter.Match(zone.Name) {
log.Debugf("zone %s not in domain filter", zone.Name) log.Debugf("zone %q not in domain filter", zone.Name)
continue continue
} }
result = append(result, zone) result = append(result, zone)
@ -359,6 +375,75 @@ func (p *CloudFlareProvider) ApplyChanges(ctx context.Context, changes *plan.Cha
return p.submitChanges(ctx, cloudflareChanges) return p.submitChanges(ctx, cloudflareChanges)
} }
// submitCustomHostnameChanges implements Custom Hostname functionality for the Change, returns false if it fails
func (p *CloudFlareProvider) submitCustomHostnameChanges(ctx context.Context, zoneID string, change *cloudFlareChange, chs CustomHostnamesMap, logFields log.Fields) bool {
failedChange := false
// return early if disabled
if !p.CustomHostnamesConfig.Enabled {
return !failedChange
}
switch change.Action {
case cloudFlareUpdate:
if recordTypeCustomHostnameSupported[change.ResourceRecord.Type] {
prevChName := change.CustomHostnamePrev
newChName := change.CustomHostname.Hostname
if prevCh, err := getCustomHostname(chs, prevChName); err == nil {
prevChID := prevCh.ID
if prevChID != "" && prevChName != newChName {
log.WithFields(logFields).Infof("Removing previous custom hostname %q/%q", prevChID, prevChName)
chErr := p.Client.DeleteCustomHostname(ctx, zoneID, prevChID)
if chErr != nil {
failedChange = true
log.WithFields(logFields).Errorf("failed to remove previous custom hostname %q/%q: %v", prevChID, prevChName, chErr)
}
}
}
if newChName != "" && prevChName != newChName {
log.WithFields(logFields).Infof("Adding custom hostname %q", newChName)
_, chErr := p.Client.CreateCustomHostname(ctx, zoneID, change.CustomHostname)
if chErr != nil {
failedChange = true
log.WithFields(logFields).Errorf("failed to add custom hostname %q: %v", newChName, chErr)
}
}
}
case cloudFlareDelete:
if recordTypeCustomHostnameSupported[change.ResourceRecord.Type] && change.CustomHostname.Hostname != "" {
log.WithFields(logFields).Infof("Deleting custom hostname %q", change.CustomHostname.Hostname)
if ch, err := getCustomHostname(chs, change.CustomHostname.Hostname); err == nil {
chID := ch.ID
chErr := p.Client.DeleteCustomHostname(ctx, zoneID, chID)
if chErr != nil {
failedChange = true
log.WithFields(logFields).Errorf("failed to delete custom hostname %q/%q: %v", chID, change.CustomHostname.Hostname, chErr)
}
} else {
log.WithFields(logFields).Warnf("failed to delete custom hostname %q: %v", change.CustomHostname.Hostname, err)
}
}
case cloudFlareCreate:
if recordTypeCustomHostnameSupported[change.ResourceRecord.Type] && change.CustomHostname.Hostname != "" {
log.WithFields(logFields).Infof("Creating custom hostname %q", change.CustomHostname.Hostname)
if ch, err := getCustomHostname(chs, change.CustomHostname.Hostname); err == nil {
if change.CustomHostname.CustomOriginServer == ch.CustomOriginServer {
log.WithFields(logFields).Warnf("custom hostname %q already exists with the same origin %q, continue", change.CustomHostname.Hostname, ch.CustomOriginServer)
} else {
failedChange = true
log.WithFields(logFields).Errorf("failed to create custom hostname, %q already exists with origin %q", change.CustomHostname.Hostname, ch.CustomOriginServer)
}
} else {
_, chErr := p.Client.CreateCustomHostname(ctx, zoneID, change.CustomHostname)
if chErr != nil {
failedChange = true
log.WithFields(logFields).Errorf("failed to create custom hostname %q: %v", change.CustomHostname.Hostname, chErr)
}
}
}
}
return !failedChange
}
// submitChanges takes a zone and a collection of Changes and sends them as a single transaction. // submitChanges takes a zone and a collection of Changes and sends them as a single transaction.
func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloudFlareChange) error { func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloudFlareChange) error {
// return early if there is nothing to change // return early if there is nothing to change
@ -395,37 +480,15 @@ func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloud
resourceContainer := cloudflare.ZoneIdentifier(zoneID) resourceContainer := cloudflare.ZoneIdentifier(zoneID)
records, err := p.listDNSRecordsWithAutoPagination(ctx, zoneID) records, err := p.listDNSRecordsWithAutoPagination(ctx, zoneID)
if err != nil { if err != nil {
return fmt.Errorf("could not fetch records from zone, %v", err) return fmt.Errorf("could not fetch records from zone, %w", err)
} }
chs, chErr := p.listCustomHostnamesWithPagination(ctx, zoneID) chs, chErr := p.listCustomHostnamesWithPagination(ctx, zoneID)
if chErr != nil { if chErr != nil {
return fmt.Errorf("could not fetch custom hostnames from zone, %v", chErr) return fmt.Errorf("could not fetch custom hostnames from zone, %v", chErr)
} }
if change.Action == cloudFlareUpdate { if change.Action == cloudFlareUpdate {
if recordTypeCustomHostnameSupported[change.ResourceRecord.Type] { if !p.submitCustomHostnameChanges(ctx, zoneID, change, chs, logFields) {
prevCh := change.CustomHostnamePrev failedChange = true
newCh := change.CustomHostname.Hostname
if prevCh != "" {
prevChID, _ := p.getCustomHostnameOrigin(chs, prevCh)
if prevChID != "" && prevCh != newCh {
log.WithFields(logFields).Infof("Removing previous custom hostname %v/%v", prevChID, prevCh)
chErr := p.Client.DeleteCustomHostname(ctx, zoneID, prevChID)
if chErr != nil {
failedChange = true
log.WithFields(logFields).Errorf("failed to remove previous custom hostname %v/%v: %v", prevChID, prevCh, chErr)
}
}
}
if newCh != "" {
if prevCh != newCh {
log.WithFields(logFields).Infof("Adding custom hostname %v", newCh)
_, chErr := p.Client.CreateCustomHostname(ctx, zoneID, change.CustomHostname)
if chErr != nil {
failedChange = true
log.WithFields(logFields).Errorf("failed to add custom hostname %v: %v", newCh, chErr)
}
}
}
} }
recordID := p.getRecordID(records, change.ResourceRecord) recordID := p.getRecordID(records, change.ResourceRecord)
if recordID == "" { if recordID == "" {
@ -457,19 +520,8 @@ func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloud
failedChange = true failedChange = true
log.WithFields(logFields).Errorf("failed to delete record: %v", err) log.WithFields(logFields).Errorf("failed to delete record: %v", err)
} }
if change.CustomHostname.Hostname == "" { if !p.submitCustomHostnameChanges(ctx, zoneID, change, chs, logFields) {
continue
}
log.WithFields(logFields).Infof("Deleting custom hostname %v", change.CustomHostname.Hostname)
chID, _ := p.getCustomHostnameOrigin(chs, change.CustomHostname.Hostname)
if chID == "" {
log.WithFields(logFields).Infof("Custom hostname %v not found", change.CustomHostname.Hostname)
continue
}
chErr := p.Client.DeleteCustomHostname(ctx, zoneID, chID)
if chErr != nil {
failedChange = true failedChange = true
log.WithFields(logFields).Errorf("failed to delete custom hostname %v/%v: %v", chID, change.CustomHostname.Hostname, chErr)
} }
} else if change.Action == cloudFlareCreate { } else if change.Action == cloudFlareCreate {
recordParam := getCreateDNSRecordParam(*change) recordParam := getCreateDNSRecordParam(*change)
@ -478,20 +530,8 @@ func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloud
failedChange = true failedChange = true
log.WithFields(logFields).Errorf("failed to create record: %v", err) log.WithFields(logFields).Errorf("failed to create record: %v", err)
} }
if change.CustomHostname.Hostname == "" { if !p.submitCustomHostnameChanges(ctx, zoneID, change, chs, logFields) {
continue
}
log.WithFields(logFields).Infof("Creating custom hostname %v", change.CustomHostname.Hostname)
chID, chOrigin := p.getCustomHostnameOrigin(chs, change.CustomHostname.Hostname)
if chID != "" {
failedChange = true failedChange = true
log.WithFields(logFields).Errorf("failed to create custom hostname, %v already exists for origin %v", change.CustomHostname.Hostname, chOrigin)
continue
}
_, chErr := p.Client.CreateCustomHostname(ctx, zoneID, change.CustomHostname)
if chErr != nil {
failedChange = true
log.WithFields(logFields).Errorf("failed to create custom hostname %v: %v", change.CustomHostname.Hostname, chErr)
} }
} }
} }
@ -501,7 +541,7 @@ func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloud
} }
if len(failedZones) > 0 { if len(failedZones) > 0 {
return fmt.Errorf("failed to submit all changes for the following zones: %v", failedZones) return fmt.Errorf("failed to submit all changes for the following zones: %q", failedZones)
} }
return nil return nil
@ -535,7 +575,7 @@ func (p *CloudFlareProvider) changesByZone(zones []cloudflare.Zone, changeSet []
for _, c := range changeSet { for _, c := range changeSet {
zoneID, _ := zoneNameIDMapper.FindZone(c.ResourceRecord.Name) zoneID, _ := zoneNameIDMapper.FindZone(c.ResourceRecord.Name)
if zoneID == "" { if zoneID == "" {
log.Debugf("Skipping record %s because no hosted zone matching record DNS Name was detected", c.ResourceRecord.Name) log.Debugf("Skipping record %q because no hosted zone matching record DNS Name was detected", c.ResourceRecord.Name)
continue continue
} }
changes[zoneID] = append(changes[zoneID], c) changes[zoneID] = append(changes[zoneID], c)
@ -544,22 +584,21 @@ func (p *CloudFlareProvider) changesByZone(zones []cloudflare.Zone, changeSet []
return changes return changes
} }
func (p *CloudFlareProvider) getRecordID(records []cloudflare.DNSRecord, record cloudflare.DNSRecord) string { func (p *CloudFlareProvider) getRecordID(records DNSRecordsMap, record cloudflare.DNSRecord) string {
for _, zoneRecord := range records { if zoneRecord, ok := records[DNSRecordIndex{Name: record.Name, Type: record.Type, Content: record.Content}]; ok {
if zoneRecord.Name == record.Name && zoneRecord.Type == record.Type && zoneRecord.Content == record.Content { return zoneRecord.ID
return zoneRecord.ID
}
} }
return "" return ""
} }
func (p *CloudFlareProvider) getCustomHostnameOrigin(chs []cloudflare.CustomHostname, hostname string) (string, string) { func getCustomHostname(chs CustomHostnamesMap, chName string) (cloudflare.CustomHostname, error) {
for _, zoneCh := range chs { if chName == "" {
if zoneCh.Hostname == hostname { return cloudflare.CustomHostname{}, fmt.Errorf("failed to get custom hostname: %q is empty", chName)
return zoneCh.ID, zoneCh.CustomOriginServer
}
} }
return "", "" if ch, ok := chs[CustomHostnameIndex{Hostname: chName}]; ok {
return ch, nil
}
return cloudflare.CustomHostname{}, fmt.Errorf("failed to get custom hostname: %q not found", chName)
} }
func (p *CloudFlareProvider) newCloudFlareChange(action string, endpoint *endpoint.Endpoint, target string, current *endpoint.Endpoint) *cloudFlareChange { func (p *CloudFlareProvider) newCloudFlareChange(action string, endpoint *endpoint.Endpoint, target string, current *endpoint.Endpoint) *cloudFlareChange {
@ -580,7 +619,7 @@ func (p *CloudFlareProvider) newCloudFlareChange(action string, endpoint *endpoi
newCustomHostname = cloudflare.CustomHostname{ newCustomHostname = cloudflare.CustomHostname{
Hostname: getEndpointCustomHostname(endpoint), Hostname: getEndpointCustomHostname(endpoint),
CustomOriginServer: endpoint.DNSName, CustomOriginServer: endpoint.DNSName,
SSL: getCustomHostnamesSSLOptions(endpoint, p.CustomHostnamesConfig), SSL: getCustomHostnamesSSLOptions(p.CustomHostnamesConfig),
} }
} }
return &cloudFlareChange{ return &cloudFlareChange{
@ -607,9 +646,14 @@ func (p *CloudFlareProvider) newCloudFlareChange(action string, endpoint *endpoi
} }
} }
func newDNSRecordIndex(r cloudflare.DNSRecord) DNSRecordIndex {
return DNSRecordIndex{Name: r.Name, Type: r.Type, Content: r.Content}
}
// listDNSRecordsWithAutoPagination performs automatic pagination of results on requests to cloudflare.ListDNSRecords with custom per_page values // listDNSRecordsWithAutoPagination performs automatic pagination of results on requests to cloudflare.ListDNSRecords with custom per_page values
func (p *CloudFlareProvider) listDNSRecordsWithAutoPagination(ctx context.Context, zoneID string) ([]cloudflare.DNSRecord, error) { func (p *CloudFlareProvider) listDNSRecordsWithAutoPagination(ctx context.Context, zoneID string) (DNSRecordsMap, error) {
var records []cloudflare.DNSRecord // for faster getRecordID lookup
records := make(DNSRecordsMap)
resultInfo := cloudflare.ResultInfo{PerPage: p.DNSRecordsPerPage, Page: 1} resultInfo := cloudflare.ResultInfo{PerPage: p.DNSRecordsPerPage, Page: 1}
params := cloudflare.ListDNSRecordsParams{ResultInfo: resultInfo} params := cloudflare.ListDNSRecordsParams{ResultInfo: resultInfo}
for { for {
@ -625,7 +669,9 @@ func (p *CloudFlareProvider) listDNSRecordsWithAutoPagination(ctx context.Contex
return nil, err return nil, err
} }
records = append(records, pageRecords...) for _, r := range pageRecords {
records[newDNSRecordIndex(r)] = r
}
params.ResultInfo = resultInfo.Next() params.ResultInfo = resultInfo.Next()
if params.ResultInfo.Done() { if params.ResultInfo.Done() {
break break
@ -634,12 +680,16 @@ func (p *CloudFlareProvider) listDNSRecordsWithAutoPagination(ctx context.Contex
return records, nil return records, nil
} }
func newCustomHostnameIndex(ch cloudflare.CustomHostname) CustomHostnameIndex {
return CustomHostnameIndex{Hostname: ch.Hostname}
}
// listCustomHostnamesWithPagination performs automatic pagination of results on requests to cloudflare.CustomHostnames // listCustomHostnamesWithPagination performs automatic pagination of results on requests to cloudflare.CustomHostnames
func (p *CloudFlareProvider) listCustomHostnamesWithPagination(ctx context.Context, zoneID string) ([]cloudflare.CustomHostname, error) { func (p *CloudFlareProvider) listCustomHostnamesWithPagination(ctx context.Context, zoneID string) (CustomHostnamesMap, error) {
if !p.CustomHostnamesConfig.Enabled { if !p.CustomHostnamesConfig.Enabled {
return nil, nil return nil, nil
} }
var chs []cloudflare.CustomHostname chs := make(CustomHostnamesMap)
resultInfo := cloudflare.ResultInfo{Page: 1} resultInfo := cloudflare.ResultInfo{Page: 1}
for { for {
pageCustomHostnameListResponse, result, err := p.Client.CustomHostnames(ctx, zoneID, resultInfo.Page, cloudflare.CustomHostname{}) pageCustomHostnameListResponse, result, err := p.Client.CustomHostnames(ctx, zoneID, resultInfo.Page, cloudflare.CustomHostname{})
@ -651,11 +701,12 @@ func (p *CloudFlareProvider) listCustomHostnamesWithPagination(ctx context.Conte
return nil, provider.NewSoftError(err) return nil, provider.NewSoftError(err)
} }
} }
log.Errorf("zone %s failed to fetch custom hostnames. Please check if \"Cloudflare for SaaS\" is enabled and API key permissions, %v", zoneID, err) log.Errorf("zone %q failed to fetch custom hostnames. Please check if \"Cloudflare for SaaS\" is enabled and API key permissions, %v", zoneID, err)
return nil, err return nil, err
} }
for _, ch := range pageCustomHostnameListResponse {
chs = append(chs, pageCustomHostnameListResponse...) chs[newCustomHostnameIndex(ch)] = ch
}
resultInfo = result.Next() resultInfo = result.Next()
if resultInfo.Done() { if resultInfo.Done() {
break break
@ -664,7 +715,7 @@ func (p *CloudFlareProvider) listCustomHostnamesWithPagination(ctx context.Conte
return chs, nil return chs, nil
} }
func getCustomHostnamesSSLOptions(endpoint *endpoint.Endpoint, customHostnamesConfig CustomHostnamesConfig) *cloudflare.CustomHostnameSSL { func getCustomHostnamesSSLOptions(customHostnamesConfig CustomHostnamesConfig) *cloudflare.CustomHostnameSSL {
return &cloudflare.CustomHostnameSSL{ return &cloudflare.CustomHostnameSSL{
Type: "dv", Type: "dv",
Method: "http", Method: "http",
@ -683,7 +734,7 @@ func shouldBeProxied(endpoint *endpoint.Endpoint, proxiedByDefault bool) bool {
if v.Name == source.CloudflareProxiedKey { if v.Name == source.CloudflareProxiedKey {
b, err := strconv.ParseBool(v.Value) b, err := strconv.ParseBool(v.Value)
if err != nil { if err != nil {
log.Errorf("Failed to parse annotation [%s]: %v", source.CloudflareProxiedKey, err) log.Errorf("Failed to parse annotation [%q]: %v", source.CloudflareProxiedKey, err)
} else { } else {
proxied = b proxied = b
} }
@ -706,7 +757,7 @@ func getEndpointCustomHostname(endpoint *endpoint.Endpoint) string {
return "" return ""
} }
func groupByNameAndTypeWithCustomHostnames(records []cloudflare.DNSRecord, chs []cloudflare.CustomHostname) []*endpoint.Endpoint { func groupByNameAndTypeWithCustomHostnames(records DNSRecordsMap, chs CustomHostnamesMap) []*endpoint.Endpoint {
endpoints := []*endpoint.Endpoint{} endpoints := []*endpoint.Endpoint{}
// group supported records by name and type // group supported records by name and type

201
provider/cloudflare/cloudflare_test.go
View File

@ -21,6 +21,7 @@ import (
"errors" "errors"
"fmt" "fmt"
"os" "os"
"slices"
"sort" "sort"
"strings" "strings"
"testing" "testing"
@ -404,10 +405,10 @@ func getCustomHostnameIdxByID(chs []cloudflare.CustomHostname, customHostnameID
return -1 return -1
} }
func (p *CloudFlareProvider) getCustomHostnameIDbyCustomHostnameAndOrigin(chs []cloudflare.CustomHostname, customHostname string, origin string) (string, string) { func getCustomHostnameIDbyCustomHostnameAndOrigin(chs CustomHostnamesMap, customHostname string, origin string) (string, string) {
for _, zoneCh := range chs { for _, ch := range chs {
if zoneCh.Hostname == customHostname && zoneCh.CustomOriginServer == origin { if ch.Hostname == customHostname && ch.CustomOriginServer == origin {
return zoneCh.ID, zoneCh.Hostname return ch.ID, ch.Hostname
} }
} }
@ -1029,19 +1030,19 @@ func TestCloudflareApplyChangesError(t *testing.T) {
func TestCloudflareGetRecordID(t *testing.T) { func TestCloudflareGetRecordID(t *testing.T) {
p := &CloudFlareProvider{} p := &CloudFlareProvider{}
records := []cloudflare.DNSRecord{ recordsMap := DNSRecordsMap{
{ {Name: "foo.com", Type: endpoint.RecordTypeCNAME, Content: "foobar"}: {
Name: "foo.com", Name: "foo.com",
Type: endpoint.RecordTypeCNAME, Type: endpoint.RecordTypeCNAME,
Content: "foobar", Content: "foobar",
ID: "1", ID: "1",
}, },
{ {Name: "bar.de", Type: endpoint.RecordTypeA}: {
Name: "bar.de", Name: "bar.de",
Type: endpoint.RecordTypeA, Type: endpoint.RecordTypeA,
ID: "2", ID: "2",
}, },
{ {Name: "bar.de", Type: endpoint.RecordTypeA, Content: "1.2.3.4"}: {
Name: "bar.de", Name: "bar.de",
Type: endpoint.RecordTypeA, Type: endpoint.RecordTypeA,
Content: "1.2.3.4", Content: "1.2.3.4",
@ -1049,29 +1050,29 @@ func TestCloudflareGetRecordID(t *testing.T) {
}, },
} }
assert.Equal(t, "", p.getRecordID(records, cloudflare.DNSRecord{ assert.Equal(t, "", p.getRecordID(recordsMap, cloudflare.DNSRecord{
Name: "foo.com", Name: "foo.com",
Type: endpoint.RecordTypeA, Type: endpoint.RecordTypeA,
Content: "foobar", Content: "foobar",
})) }))
assert.Equal(t, "", p.getRecordID(records, cloudflare.DNSRecord{ assert.Equal(t, "", p.getRecordID(recordsMap, cloudflare.DNSRecord{
Name: "foo.com", Name: "foo.com",
Type: endpoint.RecordTypeCNAME, Type: endpoint.RecordTypeCNAME,
Content: "fizfuz", Content: "fizfuz",
})) }))
assert.Equal(t, "1", p.getRecordID(records, cloudflare.DNSRecord{ assert.Equal(t, "1", p.getRecordID(recordsMap, cloudflare.DNSRecord{
Name: "foo.com", Name: "foo.com",
Type: endpoint.RecordTypeCNAME, Type: endpoint.RecordTypeCNAME,
Content: "foobar", Content: "foobar",
})) }))
assert.Equal(t, "", p.getRecordID(records, cloudflare.DNSRecord{ assert.Equal(t, "", p.getRecordID(recordsMap, cloudflare.DNSRecord{
Name: "bar.de", Name: "bar.de",
Type: endpoint.RecordTypeA, Type: endpoint.RecordTypeA,
Content: "2.3.4.5", Content: "2.3.4.5",
})) }))
assert.Equal(t, "2", p.getRecordID(records, cloudflare.DNSRecord{ assert.Equal(t, "2", p.getRecordID(recordsMap, cloudflare.DNSRecord{
Name: "bar.de", Name: "bar.de",
Type: endpoint.RecordTypeA, Type: endpoint.RecordTypeA,
Content: "1.2.3.4", Content: "1.2.3.4",
@ -1309,7 +1310,19 @@ func TestCloudflareGroupByNameAndType(t *testing.T) {
} }
for _, tc := range testCases { for _, tc := range testCases {
assert.ElementsMatch(t, groupByNameAndTypeWithCustomHostnames(tc.Records, []cloudflare.CustomHostname{}), tc.ExpectedEndpoints) records := make(DNSRecordsMap)
for _, r := range tc.Records {
records[newDNSRecordIndex(r)] = r
}
endpoints := groupByNameAndTypeWithCustomHostnames(records, CustomHostnamesMap{})
// Targets order could be random with underlying map
for _, ep := range endpoints {
slices.Sort(ep.Targets)
}
for _, ep := range tc.ExpectedEndpoints {
slices.Sort(ep.Targets)
}
assert.ElementsMatch(t, endpoints, tc.ExpectedEndpoints)
} }
} }
@ -1866,9 +1879,9 @@ func TestCloudflareDNSRecordsOperationsFail(t *testing.T) {
err = provider.ApplyChanges(context.Background(), planned.Changes) err = provider.ApplyChanges(context.Background(), planned.Changes)
if err == nil && tc.shouldFail { if err == nil && tc.shouldFail {
t.Errorf("should fail - %s, %s", tc.Name, err) t.Errorf("should fail - %q, %v", tc.Name, err)
} else if err != nil && !tc.shouldFail { } else if err != nil && !tc.shouldFail {
t.Errorf("should not fail - %s, %s", tc.Name, err) t.Errorf("should not fail - %q, %v", tc.Name, err)
} }
} }
} }
@ -1908,10 +1921,10 @@ func TestCloudflareCustomHostnameOperations(t *testing.T) {
shouldFail: true, shouldFail: true,
}, },
{ {
Name: "add custom hostname to more than one endpoint", Name: "custom hostname to the same origin",
Endpoints: []*endpoint.Endpoint{ Endpoints: []*endpoint.Endpoint{
{ {
DNSName: "fail.foo.bar.com", DNSName: "origin.foo.bar.com",
Targets: endpoint.Targets{"1.2.3.4", "2.3.4.5"}, Targets: endpoint.Targets{"1.2.3.4", "2.3.4.5"},
RecordType: endpoint.RecordTypeA, RecordType: endpoint.RecordTypeA,
RecordTTL: endpoint.TTL(defaultCloudFlareRecordTTL), RecordTTL: endpoint.TTL(defaultCloudFlareRecordTTL),
@ -1919,13 +1932,72 @@ func TestCloudflareCustomHostnameOperations(t *testing.T) {
ProviderSpecific: endpoint.ProviderSpecific{ ProviderSpecific: endpoint.ProviderSpecific{
{ {
Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname", Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname",
Value: "fail.foo.fancybar.com", Value: "custom.foo.fancybar.com",
},
},
},
},
shouldFail: false,
},
{
Name: "same custom hostname to the another origin",
Endpoints: []*endpoint.Endpoint{
{
DNSName: "another-origin.foo.bar.com",
Targets: endpoint.Targets{"3.4.5.6"},
RecordType: endpoint.RecordTypeA,
RecordTTL: endpoint.TTL(defaultCloudFlareRecordTTL),
Labels: endpoint.Labels{},
ProviderSpecific: endpoint.ProviderSpecific{
{
Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname",
Value: "custom.foo.fancybar.com",
}, },
}, },
}, },
}, },
shouldFail: true, shouldFail: true,
}, },
{
Name: "create CNAME records with custom hostname",
Endpoints: []*endpoint.Endpoint{
{
DNSName: "c.foo.bar.com",
Targets: endpoint.Targets{"c.cname.foo.bar.com"},
RecordType: endpoint.RecordTypeCNAME,
RecordTTL: endpoint.TTL(defaultCloudFlareRecordTTL),
Labels: endpoint.Labels{},
ProviderSpecific: endpoint.ProviderSpecific{
{
Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname",
Value: "c.foo.fancybar.com",
},
},
},
},
shouldFail: false,
},
{
Name: "TXT registry record should not attempt to create custom hostname",
Endpoints: []*endpoint.Endpoint{
{
DNSName: "cname-c.foo.bar.com",
Targets: endpoint.Targets{
"heritage=external-dns,external-dns/owner=default,external-dns/resource=service/external-dns/my-domain-here-app",
},
RecordType: endpoint.RecordTypeTXT,
RecordTTL: endpoint.TTL(defaultCloudFlareRecordTTL),
Labels: endpoint.Labels{},
ProviderSpecific: endpoint.ProviderSpecific{
{
Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname",
Value: "c.foo.fancybar.com",
},
},
},
},
shouldFail: false,
},
{ {
Name: "failing to update custom hostname", Name: "failing to update custom hostname",
Endpoints: []*endpoint.Endpoint{ Endpoints: []*endpoint.Endpoint{
@ -2157,7 +2229,7 @@ func TestCloudflareCustomHostnameOperations(t *testing.T) {
for _, tc := range testFailCases { for _, tc := range testFailCases {
records, err := provider.Records(ctx) records, err := provider.Records(ctx)
if err != nil { if err != nil {
t.Errorf("should not fail, %s", err) t.Errorf("should not fail, %v", err)
} }
endpoints, err := provider.AdjustEndpoints(tc.Endpoints) endpoints, err := provider.AdjustEndpoints(tc.Endpoints)
@ -2167,23 +2239,23 @@ func TestCloudflareCustomHostnameOperations(t *testing.T) {
Current: records, Current: records,
Desired: endpoints, Desired: endpoints,
DomainFilter: endpoint.MatchAllDomainFilters{&domainFilter}, DomainFilter: endpoint.MatchAllDomainFilters{&domainFilter},
ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME}, ManagedRecords: []string{endpoint.RecordTypeA, endpoint.RecordTypeCNAME, endpoint.RecordTypeTXT},
} }
planned := plan.Calculate() planned := plan.Calculate()
err = provider.ApplyChanges(context.Background(), planned.Changes) err = provider.ApplyChanges(context.Background(), planned.Changes)
if err == nil && tc.shouldFail { if err == nil && tc.shouldFail {
t.Errorf("should fail - %s, %s", tc.Name, err) t.Errorf("should fail - %q, %v", tc.Name, err)
} else if err != nil && !tc.shouldFail { } else if err != nil && !tc.shouldFail {
t.Errorf("should not fail - %s, %s", tc.Name, err) t.Errorf("should not fail - %q, %v", tc.Name, err)
} }
} }
for _, tc := range testCases { for _, tc := range testCases {
records, err := provider.Records(ctx) records, err := provider.Records(ctx)
if err != nil { if err != nil {
t.Errorf("should not fail, %s", err) t.Errorf("should not fail, %v", err)
} }
endpoints, err := provider.AdjustEndpoints(tc.Endpoints) endpoints, err := provider.AdjustEndpoints(tc.Endpoints)
@ -2200,16 +2272,16 @@ func TestCloudflareCustomHostnameOperations(t *testing.T) {
err = provider.ApplyChanges(context.Background(), planned.Changes) err = provider.ApplyChanges(context.Background(), planned.Changes)
if err != nil { if err != nil {
t.Errorf("should not fail - %s, %s", tc.Name, err) t.Errorf("should not fail - %q, %v", tc.Name, err)
} }
chs, chErr := provider.listCustomHostnamesWithPagination(ctx, "001") chs, chErr := provider.listCustomHostnamesWithPagination(ctx, "001")
if chErr != nil { if chErr != nil {
t.Errorf("should not fail - %s, %s", tc.Name, chErr) t.Errorf("should not fail - %q, %v", tc.Name, chErr)
} }
for expectedOrigin, expectedCustomHostname := range tc.ExpectedCustomHostnames { for expectedOrigin, expectedCustomHostname := range tc.ExpectedCustomHostnames {
_, ch := provider.getCustomHostnameIDbyCustomHostnameAndOrigin(chs, expectedCustomHostname, expectedOrigin) _, ch := getCustomHostnameIDbyCustomHostnameAndOrigin(chs, expectedCustomHostname, expectedOrigin)
assert.Equal(t, expectedCustomHostname, ch) assert.Equal(t, expectedCustomHostname, ch)
} }
} }
@ -2229,7 +2301,8 @@ func TestCloudflareCustomHostnameNotFoundOnRecordDeletion(t *testing.T) {
Name string Name string
Endpoints []*endpoint.Endpoint Endpoints []*endpoint.Endpoint
ExpectedCustomHostnames map[string]string ExpectedCustomHostnames map[string]string
preApplyHook bool preApplyHook string
logOutput string
}{ }{
{ {
Name: "create DNS record with custom hostname", Name: "create DNS record with custom hostname",
@ -2248,20 +2321,49 @@ func TestCloudflareCustomHostnameNotFoundOnRecordDeletion(t *testing.T) {
}, },
}, },
}, },
preApplyHook: false, preApplyHook: "",
logOutput: "",
}, },
{ {
Name: "remove DNS record with unexpectedly missing custom hostname", Name: "remove DNS record with unexpectedly missing custom hostname",
Endpoints: []*endpoint.Endpoint{}, Endpoints: []*endpoint.Endpoint{},
preApplyHook: true, preApplyHook: "corrupt",
logOutput: "level=warning msg=\"failed to delete custom hostname \\\"newerror-getCustomHostnameOrigin.foo.fancybar.com\\\": failed to get custom hostname: \\\"newerror-getCustomHostnameOrigin.foo.fancybar.com\\\" not found\" action=DELETE record=create.foo.bar.com",
},
{
Name: "duplicate custom hostname",
Endpoints: []*endpoint.Endpoint{},
preApplyHook: "duplicate",
logOutput: "",
},
{
Name: "create DNS record with custom hostname",
Endpoints: []*endpoint.Endpoint{
{
DNSName: "a.foo.bar.com",
Targets: endpoint.Targets{"1.2.3.4"},
RecordType: endpoint.RecordTypeA,
RecordTTL: endpoint.TTL(defaultCloudFlareRecordTTL),
Labels: endpoint.Labels{},
ProviderSpecific: endpoint.ProviderSpecific{
{
Name: "external-dns.alpha.kubernetes.io/cloudflare-custom-hostname",
Value: "a.foo.fancybar.com",
},
},
},
},
preApplyHook: "",
logOutput: "custom hostname \\\"a.foo.fancybar.com\\\" already exists with the same origin \\\"a.foo.bar.com\\\", continue",
}, },
} }
b := testutils.LogsToBuffer(log.InfoLevel, t)
for _, tc := range testCases { for _, tc := range testCases {
b := testutils.LogsToBuffer(log.InfoLevel, t)
records, err := provider.Records(ctx) records, err := provider.Records(ctx)
if err != nil { if err != nil {
t.Errorf("should not fail, %s", err) t.Errorf("should not fail, %v", err)
} }
endpoints, err := provider.AdjustEndpoints(tc.Endpoints) endpoints, err := provider.AdjustEndpoints(tc.Endpoints)
@ -2278,14 +2380,14 @@ func TestCloudflareCustomHostnameNotFoundOnRecordDeletion(t *testing.T) {
// manually corrupt custom hostname before the deletion step // manually corrupt custom hostname before the deletion step
// the purpose is to cause getCustomHostnameOrigin() to fail on change.Action == cloudFlareDelete // the purpose is to cause getCustomHostnameOrigin() to fail on change.Action == cloudFlareDelete
if tc.preApplyHook { chs, chErr := provider.listCustomHostnamesWithPagination(ctx, zoneID)
chs, chErr := provider.listCustomHostnamesWithPagination(ctx, zoneID) if chErr != nil {
if chErr != nil { t.Errorf("should not fail - %q, %v", tc.Name, chErr)
t.Errorf("should not fail - %s, %s", tc.Name, chErr) }
} if tc.preApplyHook == "corrupt" {
chID, _ := provider.getCustomHostnameOrigin(chs, "newerror-getCustomHostnameOrigin.foo.fancybar.com") if ch, err := getCustomHostname(chs, "newerror-getCustomHostnameOrigin.foo.fancybar.com"); err == nil {
if chID != "" { chID := ch.ID
t.Logf("corrupting custom hostname %v", chID) t.Logf("corrupting custom hostname %q", chID)
oldIdx := getCustomHostnameIdxByID(client.customHostnames[zoneID], chID) oldIdx := getCustomHostnameIdxByID(client.customHostnames[zoneID], chID)
oldCh := client.customHostnames[zoneID][oldIdx] oldCh := client.customHostnames[zoneID][oldIdx]
ch := cloudflare.CustomHostname{ ch := cloudflare.CustomHostname{
@ -2295,14 +2397,21 @@ func TestCloudflareCustomHostnameNotFoundOnRecordDeletion(t *testing.T) {
} }
client.customHostnames[zoneID][oldIdx] = ch client.customHostnames[zoneID][oldIdx] = ch
} }
} } else if tc.preApplyHook == "duplicate" { // manually inject duplicating custom hostname with the same name and origin
ch := cloudflare.CustomHostname{
ID: "ID-random-123",
Hostname: "a.foo.fancybar.com",
CustomOriginServer: "a.foo.bar.com",
}
client.customHostnames[zoneID] = append(client.customHostnames[zoneID], ch)
}
err = provider.ApplyChanges(context.Background(), planned.Changes) err = provider.ApplyChanges(context.Background(), planned.Changes)
if err != nil { if err != nil {
t.Errorf("should not fail - %s, %s", tc.Name, err) t.Errorf("should not fail - %q, %v", tc.Name, err)
} }
assert.Contains(t, b.String(), tc.logOutput)
} }
assert.Contains(t, b.String(), "level=info msg=\"Custom hostname newerror-getCustomHostnameOrigin.foo.fancybar.com not found\" action=DELETE record=create.foo.bar.com")
} }
func TestCloudflareListCustomHostnamesWithPagionation(t *testing.T) { func TestCloudflareListCustomHostnamesWithPagionation(t *testing.T) {
@ -2337,7 +2446,7 @@ func TestCloudflareListCustomHostnamesWithPagionation(t *testing.T) {
records, err := provider.Records(ctx) records, err := provider.Records(ctx)
if err != nil { if err != nil {
t.Errorf("should not fail, %s", err) t.Errorf("should not fail, %v", err)
} }
endpoints, err := provider.AdjustEndpoints(generatedEndpoints) endpoints, err := provider.AdjustEndpoints(generatedEndpoints)
@ -2354,12 +2463,12 @@ func TestCloudflareListCustomHostnamesWithPagionation(t *testing.T) {
err = provider.ApplyChanges(context.Background(), planned.Changes) err = provider.ApplyChanges(context.Background(), planned.Changes)
if err != nil { if err != nil {
t.Errorf("should not fail - %s", err) t.Errorf("should not fail - %v", err)
} }
chs, chErr := provider.listCustomHostnamesWithPagination(ctx, "001") chs, chErr := provider.listCustomHostnamesWithPagination(ctx, "001")
if chErr != nil { if chErr != nil {
t.Errorf("should not fail - %s", chErr) t.Errorf("should not fail - %v", chErr)
} }
assert.Equal(t, len(chs), CustomHostnamesNumber) assert.Equal(t, len(chs), CustomHostnamesNumber)
} }