Skip to content

Commit

Permalink
cloudflare: bugfix - do not attempt to create unconfigured empty cust…
Browse files Browse the repository at this point in the history 
…om hostnames; improve tests; streamline logic
  • Loading branch information
@mrozentsvayg
mrozentsvayg committed Mar 6, 2025
1 parent e64e536 commit 52816dc
Show file tree
Hide file tree
Showing 2 changed files with 225 additions and 95 deletions.
117 changes: 65 additions & 52 deletions provider/cloudflare/cloudflare.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,10 +159,11 @@ type CloudFlareProvider struct {

// cloudFlareChange differentiates between ChangActions
type cloudFlareChange struct {
Action string
ResourceRecord cloudflare.DNSRecord
RegionalHostname cloudflare.RegionalHostname
CustomHostname cloudflare.CustomHostname
Action string
ResourceRecord cloudflare.DNSRecord
RegionalHostname cloudflare.RegionalHostname
CustomHostname cloudflare.CustomHostname
CustomHostnamePrev string
}

// RecordParamsTypes is a typeset of the possible Record Params that can be passed to cloudflare-go library
Expand Down Expand Up @@ -319,7 +320,7 @@ func (p *CloudFlareProvider) ApplyChanges(ctx context.Context, changes *plan.Cha

for _, endpoint := range changes.Create {
for _, target := range endpoint.Targets {
cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareCreate, endpoint, target))
cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareCreate, endpoint, target, nil))
}
}

Expand All @@ -329,21 +330,21 @@ func (p *CloudFlareProvider) ApplyChanges(ctx context.Context, changes *plan.Cha
add, remove, leave := provider.Difference(current.Targets, desired.Targets)

for _, a := range remove {
cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareDelete, current, a))
cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareDelete, current, a, current))
}

for _, a := range add {
cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareCreate, desired, a))
cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareCreate, desired, a, current))
}

for _, a := range leave {
cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareUpdate, desired, a))
cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareUpdate, desired, a, current))
}
}

for _, endpoint := range changes.Delete {
for _, target := range endpoint.Targets {
cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareDelete, endpoint, target))
cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareDelete, endpoint, target, nil))
}
}

Expand All @@ -367,16 +368,6 @@ func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloud

var failedZones []string
for zoneID, changes := range changesByZone {
records, err := p.listDNSRecordsWithAutoPagination(ctx, zoneID)
if err != nil {
return fmt.Errorf("could not fetch records from zone, %v", err)
}

chs, chErr := p.listCustomHostnamesWithPagination(ctx, zoneID)
if chErr != nil {
return fmt.Errorf("could not fetch custom hostnames from zone, %v", chErr)
}

var failedChange bool
for _, change := range changes {
logFields := log.Fields{
Expand All @@ -394,34 +385,37 @@ func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloud
}

resourceContainer := cloudflare.ZoneIdentifier(zoneID)
records, err := p.listDNSRecordsWithAutoPagination(ctx, zoneID)
if err != nil {
return fmt.Errorf("could not fetch records from zone, %v", err)
}
chs, chErr := p.listCustomHostnamesWithPagination(ctx, zoneID)
if chErr != nil {
return fmt.Errorf("could not fetch custom hostnames from zone, %v", chErr)
}
if change.Action == cloudFlareUpdate {
if recordTypeCustomHostnameSupported[change.ResourceRecord.Type] {
chID, oldCh := p.getCustomHostnameIDbyOrigin(chs, change.ResourceRecord.Name)
if chID == "" && change.CustomHostname.Hostname != "" {
log.WithFields(logFields).Infof("Adding custom hostname %v", change.CustomHostname.Hostname)
_, chErr := p.Client.CreateCustomHostname(ctx, zoneID, change.CustomHostname)
if chErr != nil {
failedChange = true
log.WithFields(logFields).Errorf("failed to add custom hostname %v: %v", change.CustomHostname.Hostname, chErr)
}
} else if chID != "" && oldCh != "" && change.CustomHostname.Hostname == "" {
log.WithFields(logFields).Infof("Removing custom hostname %v", change.CustomHostname.Hostname)
chErr := p.Client.DeleteCustomHostname(ctx, zoneID, chID)
if chErr != nil {
failedChange = true
log.WithFields(logFields).Errorf("failed to remove custom hostname %v: %v", change.CustomHostname.Hostname, chErr)
prevCh := change.CustomHostnamePrev
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)
}
}
} else if chID != "" && change.CustomHostname.Hostname != "" && oldCh != change.CustomHostname.Hostname {
log.WithFields(logFields).Infof("Replacing custom hostname: %v/%v to %v", chID, oldCh, change.CustomHostname.Hostname)
chDelErr := p.Client.DeleteCustomHostname(ctx, zoneID, chID)
if chDelErr != nil {
failedChange = true
log.WithFields(logFields).Errorf("failed to remove replacing custom hostname %v/%v: %v", chID, oldCh, chDelErr)
}
_, chAddErr := p.Client.CreateCustomHostname(ctx, zoneID, change.CustomHostname)
if chAddErr != nil {
failedChange = true
log.WithFields(logFields).Errorf("failed to add replacing custom hostname %v: %v", change.CustomHostname.Hostname, chAddErr)
}
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)
}
}
}
}
Expand Down Expand Up @@ -455,14 +449,19 @@ func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloud
failedChange = true
log.WithFields(logFields).Errorf("failed to delete record: %v", err)
}
chID, oldCh := p.getCustomHostnameIDbyOrigin(chs, change.ResourceRecord.Name)
if change.CustomHostname.Hostname == "" {
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
log.WithFields(logFields).Errorf("failed to delete custom hostname %v/%v: %v", chID, oldCh, chErr)
log.WithFields(logFields).Errorf("failed to delete custom hostname %v/%v: %v", chID, change.CustomHostname.Hostname, chErr)
}
} else if change.Action == cloudFlareCreate {
recordParam := getCreateDNSRecordParam(*change)
Expand All @@ -471,7 +470,16 @@ func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloud
failedChange = true
log.WithFields(logFields).Errorf("failed to create record: %v", err)
}
if change.CustomHostname.Hostname == "" {
continue
}
log.WithFields(logFields).Infof("Creating custom hostname %v", change.CustomHostname.Hostname)
chID, chOrigin := p.getCustomHostnameOrigin(chs, change.CustomHostname.Hostname)
if chID != "" {
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
Expand Down Expand Up @@ -537,23 +545,27 @@ func (p *CloudFlareProvider) getRecordID(records []cloudflare.DNSRecord, record
return ""
}

func (p *CloudFlareProvider) getCustomHostnameIDbyOrigin(chs []cloudflare.CustomHostname, origin string) (string, string) {
func (p *CloudFlareProvider) getCustomHostnameOrigin(chs []cloudflare.CustomHostname, hostname string) (string, string) {
for _, zoneCh := range chs {
if zoneCh.CustomOriginServer == origin {
return zoneCh.ID, zoneCh.Hostname
if zoneCh.Hostname == hostname {
return zoneCh.ID, zoneCh.CustomOriginServer
}
}
return "", ""
}

func (p *CloudFlareProvider) newCloudFlareChange(action string, endpoint *endpoint.Endpoint, target string) *cloudFlareChange {
func (p *CloudFlareProvider) newCloudFlareChange(action string, endpoint *endpoint.Endpoint, target string, current *endpoint.Endpoint) *cloudFlareChange {
ttl := defaultCloudFlareRecordTTL
proxied := shouldBeProxied(endpoint, p.proxiedByDefault)

if endpoint.RecordTTL.IsConfigured() {
ttl = int(endpoint.RecordTTL)
}
dt := time.Now()
customHostnamePrev := ""
if current != nil {
customHostnamePrev = getEndpointCustomHostname(current)
}
return &cloudFlareChange{
Action: action,
ResourceRecord: cloudflare.DNSRecord{
Expand All @@ -573,6 +585,7 @@ func (p *CloudFlareProvider) newCloudFlareChange(action string, endpoint *endpoi
RegionKey: p.RegionKey,
CreatedOn: &dt,
},
CustomHostnamePrev: customHostnamePrev,
CustomHostname: cloudflare.CustomHostname{
Hostname: getEndpointCustomHostname(endpoint),
CustomOriginServer: endpoint.DNSName,
Expand Down Expand Up @@ -721,9 +734,9 @@ func groupByNameAndTypeWithCustomHostnames(records []cloudflare.DNSRecord, chs [
if ep == nil {
continue
}
ep.WithProviderSpecific(source.CloudflareProxiedKey, strconv.FormatBool(proxied))
ep = ep.WithProviderSpecific(source.CloudflareProxiedKey, strconv.FormatBool(proxied))
if customHostname, ok := customOriginServers[records[0].Name]; ok {
ep.WithProviderSpecific(source.CloudflareCustomHostnameKey, customHostname)
ep = ep.WithProviderSpecific(source.CloudflareCustomHostnameKey, customHostname)
}

endpoints = append(endpoints, ep)
Expand Down
Loading

0 comments on commit 52816dc

Please sign in to comment.