tidy up using underlying maps for dns records and custom hostnames

This commit is contained in:
Mikhail Rozentsvayg 2025-03-19 10:09:37 -07:00
parent e47d83b224
commit 2e29a19ab7
4 changed files with 47 additions and 33 deletions

1
go.mod
View File

@ -64,7 +64,6 @@ require (
github.com/ultradns/ultradns-sdk-go v1.3.7
go.etcd.io/etcd/client/v3 v3.5.19
go.uber.org/ratelimit v0.3.1
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56
golang.org/x/net v0.36.0
golang.org/x/oauth2 v0.28.0
golang.org/x/sync v0.12.0

2
go.sum
View File

@ -1157,8 +1157,6 @@ golang.org/x/exp v0.0.0-20190121172915-509febef88a4/go.mod h1:CJ0aWSM057203Lf6IL
golang.org/x/exp v0.0.0-20190125153040-c74c464bbbf2/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
golang.org/x/exp v0.0.0-20190306152737-a1d7652674e8/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA=
golang.org/x/exp v0.0.0-20191030013958-a1ab85dbe136/go.mod h1:JXzH8nQsPlswgeRAPE3MuO9GYsAcnJvJ4vnMwN/5qkY=
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 h1:2dVuKD2vS7b0QIHQbpyTISPd0LeHDbnYEryqj5Q1ug8=
golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56/go.mod h1:M4RDyNAINzryxdtnbRXRL/OHtkFuWGRjvuhBJpk2IlY=
golang.org/x/image v0.0.0-20180708004352-c73c2afc3b81/go.mod h1:ux5Hcp/YLpHSI86hEcLt0YII63i6oz57MZXIpbrjZUs=
golang.org/x/image v0.0.0-20190227222117-0694c2d4d067/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
golang.org/x/image v0.0.0-20190802002840-cff245a6509b/go.mod h1:FeLwcggjj3mMvU+oOTbSwawSJRM1uh48EjtB4UJZlP0=

View File

@ -26,8 +26,6 @@ import (
"strings"
"time"
"golang.org/x/exp/maps"
cloudflare "github.com/cloudflare/cloudflare-go"
log "github.com/sirupsen/logrus"
@ -64,14 +62,14 @@ type DNSRecordIndex struct {
Content string
}
type DNSRecordMap map[DNSRecordIndex]cloudflare.DNSRecord
type DNSRecordsMap map[DNSRecordIndex]cloudflare.DNSRecord
// for faster getCustomHostname() lookup
type CustomHostnameIndex struct {
Hostname string
}
type CustomHostnameMap map[CustomHostnameIndex]cloudflare.CustomHostname
type CustomHostnamesMap map[CustomHostnameIndex]cloudflare.CustomHostname
var recordTypeProxyNotSupported = map[string]bool{
"LOC": true,
@ -321,12 +319,12 @@ func (p *CloudFlareProvider) Records(ctx context.Context) ([]*endpoint.Endpoint,
endpoints := []*endpoint.Endpoint{}
for _, zone := range zones {
recordsMap, err := p.listDNSRecordsWithAutoPagination(ctx, zone.ID)
records, err := p.listDNSRecordsWithAutoPagination(ctx, zone.ID)
if err != nil {
return nil, err
}
chsMap, chErr := p.listCustomHostnamesWithPagination(ctx, zone.ID)
chs, chErr := p.listCustomHostnamesWithPagination(ctx, zone.ID)
if chErr != nil {
return nil, chErr
}
@ -334,7 +332,7 @@ func (p *CloudFlareProvider) Records(ctx context.Context) ([]*endpoint.Endpoint,
// As CloudFlare does not support "sets" of targets, but instead returns
// a single entry for each name/type/target, we have to group by name
// and record to allow the planner to calculate the correct plan. See #992.
endpoints = append(endpoints, groupByNameAndTypeWithCustomHostnames(maps.Values(recordsMap), maps.Values(chsMap))...)
endpoints = append(endpoints, groupByNameAndTypeWithCustomHostnames(records, chs)...)
}
return endpoints, nil
@ -378,7 +376,7 @@ func (p *CloudFlareProvider) ApplyChanges(ctx context.Context, changes *plan.Cha
}
// submitCustomHostnameChanges implements Custom Hostname functionality for the Change, returns false if it fails
func (p *CloudFlareProvider) submitCustomHostnameChanges(ctx context.Context, zoneID string, change *cloudFlareChange, chsMap CustomHostnameMap, logFields log.Fields) bool {
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 {
@ -390,7 +388,7 @@ func (p *CloudFlareProvider) submitCustomHostnameChanges(ctx context.Context, zo
if recordTypeCustomHostnameSupported[change.ResourceRecord.Type] {
prevChName := change.CustomHostnamePrev
newChName := change.CustomHostname.Hostname
if prevCh, err := getCustomHostname(chsMap, prevChName); err == nil {
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)
@ -415,7 +413,7 @@ func (p *CloudFlareProvider) submitCustomHostnameChanges(ctx context.Context, zo
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(chsMap, change.CustomHostname.Hostname); err == nil {
if ch, err := getCustomHostname(chs, change.CustomHostname.Hostname); err == nil {
chID := ch.ID
chErr := p.Client.DeleteCustomHostname(ctx, zoneID, chID)
if chErr != nil {
@ -429,7 +427,7 @@ func (p *CloudFlareProvider) submitCustomHostnameChanges(ctx context.Context, zo
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(chsMap, change.CustomHostname.Hostname); err == nil {
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 {
@ -588,18 +586,18 @@ func (p *CloudFlareProvider) changesByZone(zones []cloudflare.Zone, changeSet []
return changes
}
func (p *CloudFlareProvider) getRecordID(recordsMap DNSRecordMap, record cloudflare.DNSRecord) string {
if zoneRecord, ok := recordsMap[DNSRecordIndex{Name: record.Name, Type: record.Type, Content: record.Content}]; ok {
func (p *CloudFlareProvider) getRecordID(records DNSRecordsMap, record cloudflare.DNSRecord) string {
if zoneRecord, ok := records[DNSRecordIndex{Name: record.Name, Type: record.Type, Content: record.Content}]; ok {
return zoneRecord.ID
}
return ""
}
func getCustomHostname(chsMap CustomHostnameMap, chName string) (cloudflare.CustomHostname, error) {
func getCustomHostname(chs CustomHostnamesMap, chName string) (cloudflare.CustomHostname, error) {
if chName == "" {
return cloudflare.CustomHostname{}, fmt.Errorf("failed to get custom hostname: %q is empty", chName)
}
if ch, ok := chsMap[CustomHostnameIndex{Hostname: chName}]; ok {
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)
@ -650,10 +648,14 @@ func (p *CloudFlareProvider) newCloudFlareChange(action string, endpoint *endpoi
}
}
func getDNSRecordIndex(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
func (p *CloudFlareProvider) listDNSRecordsWithAutoPagination(ctx context.Context, zoneID string) (DNSRecordMap, error) {
func (p *CloudFlareProvider) listDNSRecordsWithAutoPagination(ctx context.Context, zoneID string) (DNSRecordsMap, error) {
// for faster getRecordID lookup
recordsMap := make(DNSRecordMap)
records := make(DNSRecordsMap)
resultInfo := cloudflare.ResultInfo{PerPage: p.DNSRecordsPerPage, Page: 1}
params := cloudflare.ListDNSRecordsParams{ResultInfo: resultInfo}
for {
@ -670,22 +672,26 @@ func (p *CloudFlareProvider) listDNSRecordsWithAutoPagination(ctx context.Contex
}
for _, r := range pageRecords {
recordsMap[DNSRecordIndex{Name: r.Name, Type: r.Type, Content: r.Content}] = r
records[getDNSRecordIndex(r)] = r
}
params.ResultInfo = resultInfo.Next()
if params.ResultInfo.Done() {
break
}
}
return recordsMap, nil
return records, nil
}
func getCustomHostnameIndex(ch cloudflare.CustomHostname) CustomHostnameIndex {
return CustomHostnameIndex{Hostname: ch.Hostname}
}
// listCustomHostnamesWithPagination performs automatic pagination of results on requests to cloudflare.CustomHostnames
func (p *CloudFlareProvider) listCustomHostnamesWithPagination(ctx context.Context, zoneID string) (CustomHostnameMap, error) {
func (p *CloudFlareProvider) listCustomHostnamesWithPagination(ctx context.Context, zoneID string) (CustomHostnamesMap, error) {
if !p.CustomHostnamesConfig.Enabled {
return nil, nil
}
chsMap := make(CustomHostnameMap)
chs := make(CustomHostnamesMap)
resultInfo := cloudflare.ResultInfo{Page: 1}
for {
pageCustomHostnameListResponse, result, err := p.Client.CustomHostnames(ctx, zoneID, resultInfo.Page, cloudflare.CustomHostname{})
@ -701,14 +707,14 @@ func (p *CloudFlareProvider) listCustomHostnamesWithPagination(ctx context.Conte
return nil, err
}
for _, ch := range pageCustomHostnameListResponse {
chsMap[CustomHostnameIndex{Hostname: ch.Hostname}] = ch
chs[getCustomHostnameIndex(ch)] = ch
}
resultInfo = result.Next()
if resultInfo.Done() {
break
}
}
return chsMap, nil
return chs, nil
}
func getCustomHostnamesSSLOptions(customHostnamesConfig CustomHostnamesConfig) *cloudflare.CustomHostnameSSL {
@ -753,7 +759,7 @@ func getEndpointCustomHostname(endpoint *endpoint.Endpoint) string {
return ""
}
func groupByNameAndTypeWithCustomHostnames(records []cloudflare.DNSRecord, chs []cloudflare.CustomHostname) []*endpoint.Endpoint {
func groupByNameAndTypeWithCustomHostnames(records DNSRecordsMap, chs CustomHostnamesMap) []*endpoint.Endpoint {
endpoints := []*endpoint.Endpoint{}
// group supported records by name and type

View File

@ -21,12 +21,11 @@ import (
"errors"
"fmt"
"os"
"slices"
"sort"
"strings"
"testing"
"golang.org/x/exp/maps"
cloudflare "github.com/cloudflare/cloudflare-go"
log "github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
@ -406,7 +405,7 @@ func getCustomHostnameIdxByID(chs []cloudflare.CustomHostname, customHostnameID
return -1
}
func getCustomHostnameIDbyCustomHostnameAndOrigin(chs []cloudflare.CustomHostname, customHostname string, origin string) (string, string) {
func getCustomHostnameIDbyCustomHostnameAndOrigin(chs CustomHostnamesMap, customHostname string, origin string) (string, string) {
for _, ch := range chs {
if ch.Hostname == customHostname && ch.CustomOriginServer == origin {
return ch.ID, ch.Hostname
@ -1031,7 +1030,7 @@ func TestCloudflareApplyChangesError(t *testing.T) {
func TestCloudflareGetRecordID(t *testing.T) {
p := &CloudFlareProvider{}
recordsMap := DNSRecordMap{
recordsMap := DNSRecordsMap{
{Name: "foo.com", Type: endpoint.RecordTypeCNAME, Content: "foobar"}: {
Name: "foo.com",
Type: endpoint.RecordTypeCNAME,
@ -1311,7 +1310,19 @@ func TestCloudflareGroupByNameAndType(t *testing.T) {
}
for _, tc := range testCases {
assert.ElementsMatch(t, groupByNameAndTypeWithCustomHostnames(tc.Records, []cloudflare.CustomHostname{}), tc.ExpectedEndpoints)
records := make(DNSRecordsMap)
for _, r := range tc.Records {
records[getDNSRecordIndex(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)
}
}
@ -2270,7 +2281,7 @@ func TestCloudflareCustomHostnameOperations(t *testing.T) {
}
for expectedOrigin, expectedCustomHostname := range tc.ExpectedCustomHostnames {
_, ch := getCustomHostnameIDbyCustomHostnameAndOrigin(maps.Values(chs), expectedCustomHostname, expectedOrigin)
_, ch := getCustomHostnameIDbyCustomHostnameAndOrigin(chs, expectedCustomHostname, expectedOrigin)
assert.Equal(t, expectedCustomHostname, ch)
}
}