Skip to content

Commit

Permalink
Fix CloudFlare provider to return a single endpoint for each name/type.
Browse files Browse the repository at this point in the history
  • Loading branch information
@shasderias
shasderias committed May 17, 2019
1 parent aa03558 commit 095c0dd
Show file tree
Hide file tree
Showing 2 changed files with 263 additions and 5 deletions.
46 changes: 41 additions & 5 deletions provider/cloudflare.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,11 +181,10 @@ func (p *CloudFlareProvider) Records() ([]*endpoint.Endpoint, error) {
return nil, err
}

for _, r := range records {
if supportedRecordType(r.Type) {
endpoints = append(endpoints, endpoint.NewEndpointWithTTL(r.Name, r.Type, endpoint.TTL(r.TTL), r.Content).WithProviderSpecific(source.CloudflareProxiedKey, strconv.FormatBool(r.Proxied)))
}
}
// 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, groupByNameAndType(records)...)
}

return endpoints, nil
Expand Down Expand Up @@ -354,3 +353,40 @@ func shouldBeProxied(endpoint *endpoint.Endpoint, proxiedByDefault bool) bool {
}
return proxied
}

func groupByNameAndType(records []cloudflare.DNSRecord) []*endpoint.Endpoint {
endpoints := []*endpoint.Endpoint{}

// group supported records by name and type
groups := map[string][]cloudflare.DNSRecord{}

for _, r := range records {
if !supportedRecordType(r.Type) {
continue
}

groupBy := r.Name + r.Type
if _, ok := groups[groupBy]; !ok {
groups[groupBy] = []cloudflare.DNSRecord{}
}

groups[groupBy] = append(groups[groupBy], r)
}

// create single endpoint with all the targets for each name/type
for _, records := range groups {
targets := make([]string, len(records))
for i, record := range records {
targets[i] = record.Content
}
endpoints = append(endpoints,
endpoint.NewEndpointWithTTL(
records[0].Name,
records[0].Type,
endpoint.TTL(records[0].TTL),
targets...).
WithProviderSpecific(source.CloudflareProxiedKey, strconv.FormatBool(records[0].Proxied)))
}

return endpoints
}
222 changes: 222 additions & 0 deletions provider/cloudflare_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -595,3 +595,225 @@ func validateCloudFlareZones(t *testing.T, zones []cloudflare.Zone, expected []c
assert.Equal(t, expected[i].Name, zone.Name)
}
}

func TestGroupByNameAndType(t *testing.T) {
testCases := []struct {
Name string
Records []cloudflare.DNSRecord
ExpectedEndpoints []*endpoint.Endpoint
}{
{
Name: "empty",
Records: []cloudflare.DNSRecord{},
ExpectedEndpoints: []*endpoint.Endpoint{},
},
{
Name: "single record - single target",
Records: []cloudflare.DNSRecord{
{
Name: "foo.com",
Type: endpoint.RecordTypeA,
Content: "10.10.10.1",
TTL: defaultCloudFlareRecordTTL,
},
},
ExpectedEndpoints: []*endpoint.Endpoint{
{
DNSName: "foo.com",
Targets: endpoint.Targets{"10.10.10.1"},
RecordType: endpoint.RecordTypeA,
RecordTTL: endpoint.TTL(defaultCloudFlareRecordTTL),
Labels: endpoint.Labels{},
ProviderSpecific: endpoint.ProviderSpecific{
{
Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied",
Value: "false",
},
},
},
},
},
{
Name: "single record - multiple targets",
Records: []cloudflare.DNSRecord{
{
Name: "foo.com",
Type: endpoint.RecordTypeA,
Content: "10.10.10.1",
TTL: defaultCloudFlareRecordTTL,
},
{
Name: "foo.com",
Type: endpoint.RecordTypeA,
Content: "10.10.10.2",
TTL: defaultCloudFlareRecordTTL,
},
},
ExpectedEndpoints: []*endpoint.Endpoint{
{
DNSName: "foo.com",
Targets: endpoint.Targets{"10.10.10.1", "10.10.10.2"},
RecordType: endpoint.RecordTypeA,
RecordTTL: endpoint.TTL(defaultCloudFlareRecordTTL),
Labels: endpoint.Labels{},
ProviderSpecific: endpoint.ProviderSpecific{
{
Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied",
Value: "false",
},
},
},
},
},
{
Name: "multiple record - multiple targets",
Records: []cloudflare.DNSRecord{
{
Name: "foo.com",
Type: endpoint.RecordTypeA,
Content: "10.10.10.1",
TTL: defaultCloudFlareRecordTTL,
},
{
Name: "foo.com",
Type: endpoint.RecordTypeA,
Content: "10.10.10.2",
TTL: defaultCloudFlareRecordTTL,
},
{
Name: "bar.de",
Type: endpoint.RecordTypeA,
Content: "10.10.10.1",
TTL: defaultCloudFlareRecordTTL,
},
{
Name: "bar.de",
Type: endpoint.RecordTypeA,
Content: "10.10.10.2",
TTL: defaultCloudFlareRecordTTL,
},
},
ExpectedEndpoints: []*endpoint.Endpoint{
{
DNSName: "foo.com",
Targets: endpoint.Targets{"10.10.10.1", "10.10.10.2"},
RecordType: endpoint.RecordTypeA,
RecordTTL: endpoint.TTL(defaultCloudFlareRecordTTL),
Labels: endpoint.Labels{},
ProviderSpecific: endpoint.ProviderSpecific{
{
Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied",
Value: "false",
},
},
},
{
DNSName: "bar.de",
Targets: endpoint.Targets{"10.10.10.1", "10.10.10.2"},
RecordType: endpoint.RecordTypeA,
RecordTTL: endpoint.TTL(defaultCloudFlareRecordTTL),
Labels: endpoint.Labels{},
ProviderSpecific: endpoint.ProviderSpecific{
{
Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied",
Value: "false",
},
},
},
},
},
{
Name: "multiple record - mixed single/multiple targets",
Records: []cloudflare.DNSRecord{
{
Name: "foo.com",
Type: endpoint.RecordTypeA,
Content: "10.10.10.1",
TTL: defaultCloudFlareRecordTTL,
},
{
Name: "foo.com",
Type: endpoint.RecordTypeA,
Content: "10.10.10.2",
TTL: defaultCloudFlareRecordTTL,
},
{
Name: "bar.de",
Type: endpoint.RecordTypeA,
Content: "10.10.10.1",
TTL: defaultCloudFlareRecordTTL,
},
},
ExpectedEndpoints: []*endpoint.Endpoint{
{
DNSName: "foo.com",
Targets: endpoint.Targets{"10.10.10.1", "10.10.10.2"},
RecordType: endpoint.RecordTypeA,
RecordTTL: endpoint.TTL(defaultCloudFlareRecordTTL),
Labels: endpoint.Labels{},
ProviderSpecific: endpoint.ProviderSpecific{
{
Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied",
Value: "false",
},
},
},
{
DNSName: "bar.de",
Targets: endpoint.Targets{"10.10.10.1"},
RecordType: endpoint.RecordTypeA,
RecordTTL: endpoint.TTL(defaultCloudFlareRecordTTL),
Labels: endpoint.Labels{},
ProviderSpecific: endpoint.ProviderSpecific{
{
Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied",
Value: "false",
},
},
},
},
},
{
Name: "unsupported record type",
Records: []cloudflare.DNSRecord{
{
Name: "foo.com",
Type: endpoint.RecordTypeA,
Content: "10.10.10.1",
TTL: defaultCloudFlareRecordTTL,
},
{
Name: "foo.com",
Type: endpoint.RecordTypeA,
Content: "10.10.10.2",
TTL: defaultCloudFlareRecordTTL,
},
{
Name: "bar.de",
Type: "NOT SUPPORTED",
Content: "10.10.10.1",
TTL: defaultCloudFlareRecordTTL,
},
},
ExpectedEndpoints: []*endpoint.Endpoint{
{
DNSName: "foo.com",
Targets: endpoint.Targets{"10.10.10.1", "10.10.10.2"},
RecordType: endpoint.RecordTypeA,
RecordTTL: endpoint.TTL(defaultCloudFlareRecordTTL),
Labels: endpoint.Labels{},
ProviderSpecific: endpoint.ProviderSpecific{
{
Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied",
Value: "false",
},
},
},
},
},
}

for _, tc := range testCases {
assert.Equal(t, groupByNameAndType(tc.Records), tc.ExpectedEndpoints)
}
}

0 comments on commit 095c0dd

Please sign in to comment.