From 8599b6eba2ece862422658287fb2af0567743717 Mon Sep 17 00:00:00 2001 From: Bas Timmer Date: Wed, 10 Apr 2019 22:10:02 +0200 Subject: [PATCH] Add support for multiple target addresses in CloudFlare provider --- provider/cloudflare.go | 75 ++++++++++++++++++++++--------------- provider/cloudflare_test.go | 24 ++++++------ 2 files changed, 56 insertions(+), 43 deletions(-) diff --git a/provider/cloudflare.go b/provider/cloudflare.go index 6fa7c3db5e..aa3aba2c71 100644 --- a/provider/cloudflare.go +++ b/provider/cloudflare.go @@ -20,6 +20,7 @@ import ( "context" "fmt" "os" + "sort" "strconv" "strings" @@ -111,7 +112,7 @@ type CloudFlareProvider struct { // cloudFlareChange differentiates between ChangActions type cloudFlareChange struct { Action string - ResourceRecordSet cloudflare.DNSRecord + ResourceRecordSet []cloudflare.DNSRecord } // NewCloudFlareProvider initializes a new CloudFlare DNS based Provider. @@ -224,11 +225,12 @@ func (p *CloudFlareProvider) submitChanges(changes []*cloudFlareChange) error { } for _, change := range changes { logFields := log.Fields{ - "record": change.ResourceRecordSet.Name, - "type": change.ResourceRecordSet.Type, - "ttl": change.ResourceRecordSet.TTL, - "action": change.Action, - "zone": zoneID, + "record": change.ResourceRecordSet[0].Name, + "type": change.ResourceRecordSet[0].Type, + "ttl": change.ResourceRecordSet[0].TTL, + "targets": len(change.ResourceRecordSet), + "action": change.Action, + "zone": zoneID, } log.WithFields(logFields).Info("Changing record.") @@ -236,22 +238,25 @@ func (p *CloudFlareProvider) submitChanges(changes []*cloudFlareChange) error { if p.DryRun { continue } - recordID := p.getRecordID(records, change.ResourceRecordSet) - switch change.Action { - case cloudFlareCreate: - _, err := p.Client.CreateDNSRecord(zoneID, change.ResourceRecordSet) - if err != nil { - log.WithFields(logFields).Errorf("failed to create record: %v", err) - } - case cloudFlareDelete: - err := p.Client.DeleteDNSRecord(zoneID, recordID) - if err != nil { - log.WithFields(logFields).Errorf("failed to delete record: %v", err) + + recordIDs := p.getRecordIDs(records, change.ResourceRecordSet[0]) + + // to simplify bookkeeping for multiple records, an update is executed as delete+create + if change.Action == cloudFlareDelete || change.Action == cloudFlareUpdate { + for _, recordID := range recordIDs { + err := p.Client.DeleteDNSRecord(zoneID, recordID) + if err != nil { + log.WithFields(logFields).Errorf("failed to delete record: %v", err) + } } - case cloudFlareUpdate: - err := p.Client.UpdateDNSRecord(zoneID, recordID, change.ResourceRecordSet) - if err != nil { - log.WithFields(logFields).Errorf("failed to update record: %v", err) + } + + if change.Action == cloudFlareCreate || change.Action == cloudFlareUpdate { + for _, record := range change.ResourceRecordSet { + _, err := p.Client.CreateDNSRecord(zoneID, record) + if err != nil { + log.WithFields(logFields).Errorf("failed to create record: %v", err) + } } } } @@ -270,9 +275,9 @@ func (p *CloudFlareProvider) changesByZone(zones []cloudflare.Zone, changeSet [] } for _, c := range changeSet { - zoneID, _ := zoneNameIDMapper.FindZone(c.ResourceRecordSet.Name) + zoneID, _ := zoneNameIDMapper.FindZone(c.ResourceRecordSet[0].Name) if zoneID == "" { - log.Debugf("Skipping record %s because no hosted zone matching record DNS Name was detected ", c.ResourceRecordSet.Name) + log.Debugf("Skipping record %s because no hosted zone matching record DNS Name was detected ", c.ResourceRecordSet[0].Name) continue } changes[zoneID] = append(changes[zoneID], c) @@ -281,13 +286,15 @@ func (p *CloudFlareProvider) changesByZone(zones []cloudflare.Zone, changeSet [] return changes } -func (p *CloudFlareProvider) getRecordID(records []cloudflare.DNSRecord, record cloudflare.DNSRecord) string { +func (p *CloudFlareProvider) getRecordIDs(records []cloudflare.DNSRecord, record cloudflare.DNSRecord) []string { + recordIDs := make([]string, 0) for _, zoneRecord := range records { if zoneRecord.Name == record.Name && zoneRecord.Type == record.Type { - return zoneRecord.ID + recordIDs = append(recordIDs, zoneRecord.ID) } } - return "" + sort.Strings(recordIDs) + return recordIDs } // newCloudFlareChanges returns a collection of Changes based on the given records and action. @@ -309,15 +316,21 @@ func newCloudFlareChange(action string, endpoint *endpoint.Endpoint, proxiedByDe ttl = int(endpoint.RecordTTL) } - return &cloudFlareChange{ - Action: action, - ResourceRecordSet: cloudflare.DNSRecord{ + resourceRecordSet := make([]cloudflare.DNSRecord, len(endpoint.Targets)) + + for i := range endpoint.Targets { + resourceRecordSet[i] = cloudflare.DNSRecord{ Name: endpoint.DNSName, TTL: ttl, Proxied: proxied, Type: endpoint.RecordType, - Content: endpoint.Targets[0], - }, + Content: endpoint.Targets[i], + } + } + + return &cloudFlareChange{ + Action: action, + ResourceRecordSet: resourceRecordSet, } } diff --git a/provider/cloudflare_test.go b/provider/cloudflare_test.go index dbad258deb..b25b14085f 100644 --- a/provider/cloudflare_test.go +++ b/provider/cloudflare_test.go @@ -399,7 +399,7 @@ func TestNewCloudFlareChanges(t *testing.T) { for i, change := range changes { assert.Equal( t, - change.ResourceRecordSet.TTL, + change.ResourceRecordSet[0].TTL, expect[i].TTL, expect[i].Name) } @@ -407,7 +407,7 @@ func TestNewCloudFlareChanges(t *testing.T) { func TestNewCloudFlareChangeNoProxied(t *testing.T) { change := newCloudFlareChange(cloudFlareCreate, &endpoint.Endpoint{DNSName: "new", RecordType: "A", Targets: endpoint.Targets{"target"}}, false) - assert.False(t, change.ResourceRecordSet.Proxied) + assert.False(t, change.ResourceRecordSet[0].Proxied) } func TestNewCloudFlareProxiedAnnotationTrue(t *testing.T) { @@ -417,7 +417,7 @@ func TestNewCloudFlareProxiedAnnotationTrue(t *testing.T) { Value: "true", }, }}, false) - assert.True(t, change.ResourceRecordSet.Proxied) + assert.True(t, change.ResourceRecordSet[0].Proxied) } func TestNewCloudFlareProxiedAnnotationFalse(t *testing.T) { @@ -427,7 +427,7 @@ func TestNewCloudFlareProxiedAnnotationFalse(t *testing.T) { Value: "false", }, }}, true) - assert.False(t, change.ResourceRecordSet.Proxied) + assert.False(t, change.ResourceRecordSet[0].Proxied) } func TestNewCloudFlareProxiedAnnotationIllegalValue(t *testing.T) { @@ -437,7 +437,7 @@ func TestNewCloudFlareProxiedAnnotationIllegalValue(t *testing.T) { Value: "asdaslkjndaslkdjals", }, }}, false) - assert.False(t, change.ResourceRecordSet.Proxied) + assert.False(t, change.ResourceRecordSet[0].Proxied) } func TestNewCloudFlareChangeProxiable(t *testing.T) { @@ -459,14 +459,14 @@ func TestNewCloudFlareChangeProxiable(t *testing.T) { change := newCloudFlareChange(cloudFlareCreate, &endpoint.Endpoint{DNSName: "new", RecordType: cloudFlareType.recordType, Targets: endpoint.Targets{"target"}}, true) if cloudFlareType.proxiable { - assert.True(t, change.ResourceRecordSet.Proxied) + assert.True(t, change.ResourceRecordSet[0].Proxied) } else { - assert.False(t, change.ResourceRecordSet.Proxied) + assert.False(t, change.ResourceRecordSet[0].Proxied) } } change := newCloudFlareChange(cloudFlareCreate, &endpoint.Endpoint{DNSName: "*.foo", RecordType: "A", Targets: endpoint.Targets{"target"}}, true) - assert.False(t, change.ResourceRecordSet.Proxied) + assert.False(t, change.ResourceRecordSet[0].Proxied) } func TestCloudFlareZones(t *testing.T) { @@ -574,14 +574,14 @@ func TestCloudFlareGetRecordID(t *testing.T) { }, } - assert.Equal(t, "", p.getRecordID(records, cloudflare.DNSRecord{ + assert.Equal(t, "", p.getRecordIDs(records, cloudflare.DNSRecord{ Name: "foo.com", Type: endpoint.RecordTypeA, - })) - assert.Equal(t, "2", p.getRecordID(records, cloudflare.DNSRecord{ + })[0]) + assert.Equal(t, "2", p.getRecordIDs(records, cloudflare.DNSRecord{ Name: "bar.de", Type: endpoint.RecordTypeA, - })) + })[0]) } func validateCloudFlareZones(t *testing.T, zones []cloudflare.Zone, expected []cloudflare.Zone) {