Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 10 commits into from
Mar 24, 2025
Merged

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

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
8d37820
fix(cloudflare): custom hostnames edge-cases causing duplicates
mrozentsvayg Mar 15, 2025
2a15aba
syntax/style
mrozentsvayg Mar 15, 2025
12cce6f
Use %q log fmt for cloudflare provider code
mrozentsvayg Mar 16, 2025
b1f2051
move custom hostnames related submitChanges() implementation to a sep…
mrozentsvayg Mar 18, 2025
037a101
use maps for DNS records getRecordID() and custom hostnames getCustom…
mrozentsvayg Mar 18, 2025
e47d83b
types for records/custom hostnames maps
mrozentsvayg Mar 18, 2025
2e29a19
tidy up using underlying maps for dns records and custom hostnames
mrozentsvayg Mar 19, 2025
47236fd
style/naming
mrozentsvayg Mar 19, 2025
43b2188
fix private names
mrozentsvayg Mar 19, 2025
d8b31bc
combine unnecessarily separated conditions
mrozentsvayg Mar 21, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
112 changes: 58 additions & 54 deletions provider/cloudflare/cloudflare.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,7 +229,7 @@ func NewCloudFlareProvider(domainFilter endpoint.DomainFilter, zoneIDFilter prov
config, err = cloudflare.New(os.Getenv("CF_API_KEY"), os.Getenv("CF_API_EMAIL"))
}
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{
// Client: config,
Expand All @@ -254,10 +254,10 @@ func (p *CloudFlareProvider) Zones(ctx context.Context) ([]cloudflare.Zone, erro
if len(p.zoneIDFilter.ZoneIDs) > 0 && p.zoneIDFilter.ZoneIDs[0] != "" {
log.Debugln("zoneIDFilter configured. only looking up zone IDs defined")
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)
if err != nil {
log.Errorf("zone %s lookup failed, %v", zoneID, err)
log.Errorf("zone %q lookup failed, %v", zoneID, err)
return result, err
}
log.WithFields(log.Fields{
Expand Down Expand Up @@ -285,7 +285,7 @@ func (p *CloudFlareProvider) Zones(ctx context.Context) ([]cloudflare.Zone, erro

for _, zone := range zonesResponse.Result {
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
}
result = append(result, zone)
Expand Down Expand Up @@ -395,34 +395,34 @@ 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)
return fmt.Errorf("could not fetch records from zone, %w", 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] {
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)
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 %v/%v: %v", prevChID, prevCh, chErr)
log.WithFields(logFields).Errorf("failed to remove previous custom hostname %q/%q: %v", prevChID, prevChName, chErr)
}
}
}
if newCh != "" {
if prevCh != newCh {
log.WithFields(logFields).Infof("Adding custom hostname %v", newCh)
if newChName != "" {
if 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 %v: %v", newCh, chErr)
log.WithFields(logFields).Errorf("failed to add custom hostname %q: %v", newChName, chErr)
}
}
}
Expand Down Expand Up @@ -457,19 +457,18 @@ func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloud
failedChange = true
log.WithFields(logFields).Errorf("failed to delete record: %v", err)
}
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, change.CustomHostname.Hostname, chErr)
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)
}
}
} else if change.Action == cloudFlareCreate {
recordParam := getCreateDNSRecordParam(*change)
Expand All @@ -478,20 +477,22 @@ 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
log.WithFields(logFields).Errorf("failed to create custom hostname %v: %v", change.CustomHostname.Hostname, chErr)
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)
}
}
}
}
}
Expand All @@ -501,7 +502,7 @@ func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloud
}

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
Expand Down Expand Up @@ -535,7 +536,7 @@ func (p *CloudFlareProvider) changesByZone(zones []cloudflare.Zone, changeSet []
for _, c := range changeSet {
zoneID, _ := zoneNameIDMapper.FindZone(c.ResourceRecord.Name)
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
}
changes[zoneID] = append(changes[zoneID], c)
Expand All @@ -553,13 +554,16 @@ func (p *CloudFlareProvider) getRecordID(records []cloudflare.DNSRecord, record
return ""
}

func (p *CloudFlareProvider) getCustomHostnameOrigin(chs []cloudflare.CustomHostname, hostname string) (string, string) {
for _, zoneCh := range chs {
if zoneCh.Hostname == hostname {
return zoneCh.ID, zoneCh.CustomOriginServer
func getCustomHostname(chs []cloudflare.CustomHostname, chName string) (cloudflare.CustomHostname, error) {
if chName == "" {
return cloudflare.CustomHostname{}, fmt.Errorf("empty")
}
for _, ch := range chs {
if ch.Hostname == chName {
return ch, nil
}
}
return "", ""
return cloudflare.CustomHostname{}, fmt.Errorf("not found")
}

func (p *CloudFlareProvider) newCloudFlareChange(action string, endpoint *endpoint.Endpoint, target string, current *endpoint.Endpoint) *cloudFlareChange {
Expand All @@ -580,7 +584,7 @@ func (p *CloudFlareProvider) newCloudFlareChange(action string, endpoint *endpoi
newCustomHostname = cloudflare.CustomHostname{
Hostname: getEndpointCustomHostname(endpoint),
CustomOriginServer: endpoint.DNSName,
SSL: getCustomHostnamesSSLOptions(endpoint, p.CustomHostnamesConfig),
SSL: getCustomHostnamesSSLOptions(p.CustomHostnamesConfig),
}
}
return &cloudFlareChange{
Expand Down Expand Up @@ -651,7 +655,7 @@ func (p *CloudFlareProvider) listCustomHostnamesWithPagination(ctx context.Conte
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
}

Expand All @@ -664,7 +668,7 @@ func (p *CloudFlareProvider) listCustomHostnamesWithPagination(ctx context.Conte
return chs, nil
}

func getCustomHostnamesSSLOptions(endpoint *endpoint.Endpoint, customHostnamesConfig CustomHostnamesConfig) *cloudflare.CustomHostnameSSL {
func getCustomHostnamesSSLOptions(customHostnamesConfig CustomHostnamesConfig) *cloudflare.CustomHostnameSSL {
return &cloudflare.CustomHostnameSSL{
Type: "dv",
Method: "http",
Expand All @@ -683,7 +687,7 @@ func shouldBeProxied(endpoint *endpoint.Endpoint, proxiedByDefault bool) bool {
if v.Name == source.CloudflareProxiedKey {
b, err := strconv.ParseBool(v.Value)
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 {
proxied = b
}
Expand Down
Loading
Loading