From 8f291943d33c4bc3d80fc81add417d7f8d7d8394 Mon Sep 17 00:00:00 2001 From: Adam Stankiewicz Date: Fri, 17 Apr 2020 21:10:28 +0200 Subject: [PATCH 1/4] Improve Cloudflare tests --- go.mod | 1 + go.sum | 2 + provider/cloudflare_test.go | 781 ++++++++++++++++++++++++++---------- provider/provider_test.go | 9 + 4 files changed, 588 insertions(+), 205 deletions(-) diff --git a/go.mod b/go.mod index aaa08b0a9..09f657895 100644 --- a/go.mod +++ b/go.mod @@ -33,6 +33,7 @@ require ( github.com/linki/instrumented_http v0.2.0 github.com/linode/linodego v0.3.0 github.com/mattn/go-isatty v0.0.11 // indirect + github.com/maxatome/go-testdeep v1.4.0 github.com/miekg/dns v1.0.14 github.com/nesv/go-dynect v0.6.0 github.com/nic-at/rc0go v1.1.0 diff --git a/go.sum b/go.sum index dd29693c8..126226d09 100644 --- a/go.sum +++ b/go.sum @@ -404,6 +404,8 @@ github.com/mattn/go-runewidth v0.0.2/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzp github.com/mattn/go-runewidth v0.0.4/go.mod h1:LwmH8dsx7+W8Uxz3IHJYH5QSwggIsqBzpuz5H//U1FU= github.com/matttproud/golang_protobuf_extensions v1.0.1 h1:4hp9jkHxhMHkqkrB3Ix0jegS5sx/RkqARlsWZ6pIwiU= github.com/matttproud/golang_protobuf_extensions v1.0.1/go.mod h1:D8He9yQNgCq6Z5Ld7szi9bcBfOoFv/3dc6xSMkL2PC0= +github.com/maxatome/go-testdeep v1.4.0 h1:vKQh3/lHKAMsxggya/fXB6fLbf70c7k6wlLveuS9sKE= +github.com/maxatome/go-testdeep v1.4.0/go.mod h1:011SgQ6efzZYAen6fDn4BqQ+lUR72ysdyKe7Dyogw70= github.com/mdempsky/unconvert v0.0.0-20190325185700-2f5dc3378ed3/go.mod h1:9+3Wp2ccIz73BJqVfc7n2+1A+mzvnEwtDTqEjeRngBQ= github.com/mholt/archiver v3.1.1+incompatible/go.mod h1:Dh2dOXnSdiLxRiPoVfIr/fI1TwETms9B8CTWfeh7ROU= github.com/miekg/dns v1.0.14 h1:9jZdLNd/P4+SfEJ0TNyxYpsK8N4GtfylBLqtbYN1sbA= diff --git a/provider/cloudflare_test.go b/provider/cloudflare_test.go index 87daddb29..93ed53d29 100644 --- a/provider/cloudflare_test.go +++ b/provider/cloudflare_test.go @@ -18,59 +18,187 @@ package provider import ( "context" - "fmt" + "errors" "os" "testing" cloudflare "github.com/cloudflare/cloudflare-go" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" + "github.com/maxatome/go-testdeep/td" "sigs.k8s.io/external-dns/endpoint" "sigs.k8s.io/external-dns/plan" ) -type mockCloudFlareClient struct{} - -func (m *mockCloudFlareClient) CreateDNSRecord(zoneID string, rr cloudflare.DNSRecord) (*cloudflare.DNSRecordResponse, error) { - return nil, nil +type MockAction struct { + Name string + ZoneId string + RecordId string + RecordData cloudflare.DNSRecord } -func (m *mockCloudFlareClient) DNSRecords(zoneID string, rr cloudflare.DNSRecord) ([]cloudflare.DNSRecord, error) { - if zoneID == "1234567890" { - return []cloudflare.DNSRecord{ - {ID: "1234567890", Name: "foobar.ext-dns-test.zalando.to.", Type: endpoint.RecordTypeA, TTL: 120}, - {ID: "1231231233", Name: "foo.bar.com", TTL: 1}}, - nil +type mockCloudFlareClient struct { + User cloudflare.User + Zones map[string]string + Records map[string]map[string]cloudflare.DNSRecord + Actions []MockAction + listZonesError error + dnsRecordsError error +} + +var ExampleDomain = []cloudflare.DNSRecord{ + { + ID: "1234567890", + ZoneID: "001", + Name: "foobar.bar.com", + Type: endpoint.RecordTypeA, + TTL: 120, + Content: "1.2.3.4", + Proxied: false, + }, + { + ID: "1231231233", + ZoneID: "002", + Name: "bar.foo.com", + Type: endpoint.RecordTypeA, + TTL: 1, + Content: "2.3.4.5", + Proxied: false, + }, +} + +func NewMockCloudFlareClient() *mockCloudFlareClient { + return &mockCloudFlareClient{ + User: cloudflare.User{ID: "xxxxxxxxxxxxxxxxxxx"}, + Zones: map[string]string{ + "001": "bar.com", + "002": "foo.com", + }, + Records: map[string]map[string]cloudflare.DNSRecord{ + "001": {}, + "002": {}, + }, + } +} + +func NewMockCloudFlareClientWithRecords(records map[string][]cloudflare.DNSRecord) *mockCloudFlareClient { + m := NewMockCloudFlareClient() + + for zoneID, zoneRecords := range records { + if zone, ok := m.Records[zoneID]; ok { + for _, record := range zoneRecords { + zone[record.ID] = record + } + } + } + + return m +} + +func (m *mockCloudFlareClient) CreateDNSRecord(zoneID string, rr cloudflare.DNSRecord) (*cloudflare.DNSRecordResponse, error) { + m.Actions = append(m.Actions, MockAction{ + Name: "Create", + ZoneId: zoneID, + RecordId: rr.ID, + RecordData: rr, + }) + if zone, ok := m.Records[zoneID]; ok { + zone[rr.ID] = rr } return nil, nil } +func (m *mockCloudFlareClient) DNSRecords(zoneID string, rr cloudflare.DNSRecord) ([]cloudflare.DNSRecord, error) { + if m.dnsRecordsError != nil { + return nil, m.dnsRecordsError + } + result := []cloudflare.DNSRecord{} + if zone, ok := m.Records[zoneID]; ok { + for _, record := range zone { + result = append(result, record) + } + return result, nil + } + return result, nil +} + func (m *mockCloudFlareClient) UpdateDNSRecord(zoneID, recordID string, rr cloudflare.DNSRecord) error { + m.Actions = append(m.Actions, MockAction{ + Name: "Update", + ZoneId: zoneID, + RecordId: recordID, + RecordData: rr, + }) + if zone, ok := m.Records[zoneID]; ok { + if _, ok := zone[recordID]; ok { + zone[recordID] = rr + } + } return nil } func (m *mockCloudFlareClient) DeleteDNSRecord(zoneID, recordID string) error { + m.Actions = append(m.Actions, MockAction{ + Name: "Delete", + ZoneId: zoneID, + RecordId: recordID, + }) + if zone, ok := m.Records[zoneID]; ok { + if _, ok := zone[recordID]; ok { + delete(zone, recordID) + return nil + } + } return nil } func (m *mockCloudFlareClient) UserDetails() (cloudflare.User, error) { - return cloudflare.User{ID: "xxxxxxxxxxxxxxxxxxx"}, nil + return m.User, nil } func (m *mockCloudFlareClient) ZoneIDByName(zoneName string) (string, error) { - return "1234567890", nil + for id, name := range m.Zones { + if name == zoneName { + return id, nil + } + } + + return "", errors.New("Unknown zone: " + zoneName) } func (m *mockCloudFlareClient) ListZones(zoneID ...string) ([]cloudflare.Zone, error) { - return []cloudflare.Zone{{ID: "1234567890", Name: "ext-dns-test.zalando.to."}, {ID: "1234567891", Name: "foo.com."}}, nil + if m.listZonesError != nil { + return nil, m.listZonesError + } + + result := []cloudflare.Zone{} + + for zoneId, zoneName := range m.Zones { + result = append(result, cloudflare.Zone{ + ID: zoneId, + Name: zoneName, + }) + } + + return result, nil } func (m *mockCloudFlareClient) ListZonesContext(ctx context.Context, opts ...cloudflare.ReqOption) (cloudflare.ZonesResponse, error) { + if m.listZonesError != nil { + return cloudflare.ZonesResponse{}, m.listZonesError + } + + result := []cloudflare.Zone{} + + for zoneId, zoneName := range m.Zones { + result = append(result, cloudflare.Zone{ + ID: zoneId, + Name: zoneName, + }) + } + return cloudflare.ZonesResponse{ - Result: []cloudflare.Zone{ - {ID: "1234567890", Name: "ext-dns-test.zalando.to."}, - {ID: "1234567891", Name: "foo.com."}}, + Result: result, ResultInfo: cloudflare.ResultInfo{ Page: 1, TotalPages: 1, @@ -78,176 +206,310 @@ func (m *mockCloudFlareClient) ListZonesContext(ctx context.Context, opts ...clo }, nil } -type mockCloudFlareDNSRecordsFail struct{} +func AssertActions(t *testing.T, provider *CloudFlareProvider, endpoints []*endpoint.Endpoint, actions []MockAction, args ...interface{}) { + t.Helper() -func (m *mockCloudFlareDNSRecordsFail) CreateDNSRecord(zoneID string, rr cloudflare.DNSRecord) (*cloudflare.DNSRecordResponse, error) { - return nil, nil -} + var client *mockCloudFlareClient -func (m *mockCloudFlareDNSRecordsFail) DNSRecords(zoneID string, rr cloudflare.DNSRecord) ([]cloudflare.DNSRecord, error) { - return []cloudflare.DNSRecord{}, fmt.Errorf("can not get records from zone") -} -func (m *mockCloudFlareDNSRecordsFail) UpdateDNSRecord(zoneID, recordID string, rr cloudflare.DNSRecord) error { - return nil -} - -func (m *mockCloudFlareDNSRecordsFail) DeleteDNSRecord(zoneID, recordID string) error { - return nil -} - -func (m *mockCloudFlareDNSRecordsFail) UserDetails() (cloudflare.User, error) { - return cloudflare.User{ID: "xxxxxxxxxxxxxxxxxxx"}, nil -} - -func (m *mockCloudFlareDNSRecordsFail) ZoneIDByName(zoneName string) (string, error) { - return "", nil -} - -func (m *mockCloudFlareDNSRecordsFail) ListZones(zoneID ...string) ([]cloudflare.Zone, error) { - return []cloudflare.Zone{{Name: "ext-dns-test.zalando.to."}}, nil -} - -func (m *mockCloudFlareDNSRecordsFail) ListZonesContext(ctx context.Context, opts ...cloudflare.ReqOption) (cloudflare.ZonesResponse, error) { - return cloudflare.ZonesResponse{ - Result: []cloudflare.Zone{ - {ID: "1234567890", Name: "ext-dns-test.zalando.to."}, - {ID: "1234567891", Name: "foo.com."}}, - ResultInfo: cloudflare.ResultInfo{ - TotalPages: 1, - }, - }, nil -} - -type mockCloudFlareListZonesFail struct{} - -func (m *mockCloudFlareListZonesFail) CreateDNSRecord(zoneID string, rr cloudflare.DNSRecord) (*cloudflare.DNSRecordResponse, error) { - return nil, nil -} - -func (m *mockCloudFlareListZonesFail) DNSRecords(zoneID string, rr cloudflare.DNSRecord) ([]cloudflare.DNSRecord, error) { - return []cloudflare.DNSRecord{}, nil -} - -func (m *mockCloudFlareListZonesFail) UpdateDNSRecord(zoneID, recordID string, rr cloudflare.DNSRecord) error { - return nil -} - -func (m *mockCloudFlareListZonesFail) DeleteDNSRecord(zoneID, recordID string) error { - return nil -} - -func (m *mockCloudFlareListZonesFail) UserDetails() (cloudflare.User, error) { - return cloudflare.User{}, nil -} - -func (m *mockCloudFlareListZonesFail) ZoneIDByName(zoneName string) (string, error) { - return "1234567890", nil -} - -func (m *mockCloudFlareListZonesFail) ListZones(zoneID ...string) ([]cloudflare.Zone, error) { - return []cloudflare.Zone{{}}, fmt.Errorf("no zones available") -} - -func (m *mockCloudFlareListZonesFail) ListZonesContext(ctx context.Context, opts ...cloudflare.ReqOption) (cloudflare.ZonesResponse, error) { - return cloudflare.ZonesResponse{}, fmt.Errorf("no zones available") -} - -func TestNewCloudFlareChanges(t *testing.T) { - expect := []struct { - Name string - TTL int - }{ - { - "CustomRecordTTL", - 120, - }, - { - "DefaultRecordTTL", - 1, - }, - } - endpoints := []*endpoint.Endpoint{ - {DNSName: "new", Targets: endpoint.Targets{"target"}, RecordTTL: 120}, - {DNSName: "new2", Targets: endpoint.Targets{"target2"}}, - } - changes := newCloudFlareChanges(cloudFlareCreate, endpoints, true) - for i, change := range changes { - assert.Equal( - t, - change.ResourceRecordSet[0].TTL, - expect[i].TTL, - expect[i].Name) - } -} - -func TestNewCloudFlareChangeNoProxied(t *testing.T) { - change := newCloudFlareChange(cloudFlareCreate, &endpoint.Endpoint{DNSName: "new", RecordType: "A", Targets: endpoint.Targets{"target"}}, false) - assert.False(t, change.ResourceRecordSet[0].Proxied) -} - -func TestNewCloudFlareProxiedAnnotationTrue(t *testing.T) { - change := newCloudFlareChange(cloudFlareCreate, &endpoint.Endpoint{DNSName: "new", RecordType: "A", Targets: endpoint.Targets{"target"}, ProviderSpecific: endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "true", - }, - }}, false) - assert.True(t, change.ResourceRecordSet[0].Proxied) -} - -func TestNewCloudFlareProxiedAnnotationFalse(t *testing.T) { - change := newCloudFlareChange(cloudFlareCreate, &endpoint.Endpoint{DNSName: "new", RecordType: "A", Targets: endpoint.Targets{"target"}, ProviderSpecific: endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "false", - }, - }}, true) - assert.False(t, change.ResourceRecordSet[0].Proxied) -} - -func TestNewCloudFlareProxiedAnnotationIllegalValue(t *testing.T) { - change := newCloudFlareChange(cloudFlareCreate, &endpoint.Endpoint{DNSName: "new", RecordType: "A", Targets: endpoint.Targets{"target"}, ProviderSpecific: endpoint.ProviderSpecific{ - endpoint.ProviderSpecificProperty{ - Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", - Value: "asdaslkjndaslkdjals", - }, - }}, false) - assert.False(t, change.ResourceRecordSet[0].Proxied) -} - -func TestNewCloudFlareChangeProxiable(t *testing.T) { - var cloudFlareTypes = []struct { - recordType string - proxiable bool - }{ - {"A", true}, - {"CNAME", true}, - {"LOC", false}, - {"MX", false}, - {"NS", false}, - {"SPF", false}, - {"TXT", false}, - {"SRV", false}, + if provider.Client == nil { + client = NewMockCloudFlareClient() + provider.Client = client + } else { + client = provider.Client.(*mockCloudFlareClient) } - for _, cloudFlareType := range cloudFlareTypes { - change := newCloudFlareChange(cloudFlareCreate, &endpoint.Endpoint{DNSName: "new", RecordType: cloudFlareType.recordType, Targets: endpoint.Targets{"target"}}, true) + ctx := context.Background() - if cloudFlareType.proxiable { - assert.True(t, change.ResourceRecordSet[0].Proxied) - } else { - assert.False(t, change.ResourceRecordSet[0].Proxied) + records, err := provider.Records(ctx) + + if err != nil { + t.Fatalf("cannot fetch records, %s", err) + } + + plan := &plan.Plan{ + Current: records, + Desired: endpoints, + DomainFilter: endpoint.NewDomainFilter([]string{"bar.com"}), + } + + changes := plan.Calculate().Changes + + // Records other than A and CNAME are not supported by planner, just create them + for _, endpoint := range endpoints { + if endpoint.RecordType != "A" && endpoint.RecordType != "CNAME" { + changes.Create = append(changes.Create, endpoint) } } - change := newCloudFlareChange(cloudFlareCreate, &endpoint.Endpoint{DNSName: "*.foo", RecordType: "A", Targets: endpoint.Targets{"target"}}, true) - assert.False(t, change.ResourceRecordSet[0].Proxied) + err = provider.ApplyChanges(context.Background(), changes) + + if err != nil { + t.Fatalf("cannot apply changes, %s", err) + } + + td.Cmp(t, client.Actions, actions, args...) } -func TestCloudFlareZones(t *testing.T) { +func TestCloudflareA(t *testing.T) { + endpoints := []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "bar.com", + Targets: endpoint.Targets{"127.0.0.1", "127.0.0.2"}, + }, + } + + AssertActions(t, &CloudFlareProvider{}, endpoints, []MockAction{ + { + Name: "Create", + ZoneId: "001", + RecordData: cloudflare.DNSRecord{ + Type: "A", + Name: "bar.com", + Content: "127.0.0.1", + TTL: 1, + Proxied: false, + }, + }, + { + Name: "Create", + ZoneId: "001", + RecordData: cloudflare.DNSRecord{ + Type: "A", + Name: "bar.com", + Content: "127.0.0.2", + TTL: 1, + Proxied: false, + }, + }, + }) +} + +func TestCloudflareCname(t *testing.T) { + endpoints := []*endpoint.Endpoint{ + { + RecordType: "CNAME", + DNSName: "cname.bar.com", + Targets: endpoint.Targets{"google.com", "facebook.com"}, + }, + } + + AssertActions(t, &CloudFlareProvider{}, endpoints, []MockAction{ + { + Name: "Create", + ZoneId: "001", + RecordData: cloudflare.DNSRecord{ + Type: "CNAME", + Name: "cname.bar.com", + Content: "google.com", + TTL: 1, + Proxied: false, + }, + }, + { + Name: "Create", + ZoneId: "001", + RecordData: cloudflare.DNSRecord{ + Type: "CNAME", + Name: "cname.bar.com", + Content: "facebook.com", + TTL: 1, + Proxied: false, + }, + }, + }) +} + +func TestCloudflareCustomTTL(t *testing.T) { + endpoints := []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "ttl.bar.com", + Targets: endpoint.Targets{"127.0.0.1"}, + RecordTTL: 120, + }, + } + + AssertActions(t, &CloudFlareProvider{}, endpoints, []MockAction{ + { + Name: "Create", + ZoneId: "001", + RecordData: cloudflare.DNSRecord{ + Type: "A", + Name: "ttl.bar.com", + Content: "127.0.0.1", + TTL: 120, + Proxied: false, + }, + }, + }) +} + +func TestCloudflareProxiedDefault(t *testing.T) { + endpoints := []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "bar.com", + Targets: endpoint.Targets{"127.0.0.1"}, + }, + } + + AssertActions(t, &CloudFlareProvider{proxiedByDefault: true}, endpoints, []MockAction{ + { + Name: "Create", + ZoneId: "001", + RecordData: cloudflare.DNSRecord{ + Type: "A", + Name: "bar.com", + Content: "127.0.0.1", + TTL: 1, + Proxied: true, + }, + }, + }) +} + +func TestCloudflareProxiedOverrideTrue(t *testing.T) { + endpoints := []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "bar.com", + Targets: endpoint.Targets{"127.0.0.1"}, + ProviderSpecific: endpoint.ProviderSpecific{ + endpoint.ProviderSpecificProperty{ + Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", + Value: "true", + }, + }, + }, + } + + AssertActions(t, &CloudFlareProvider{}, endpoints, []MockAction{ + { + Name: "Create", + ZoneId: "001", + RecordData: cloudflare.DNSRecord{ + Type: "A", + Name: "bar.com", + Content: "127.0.0.1", + TTL: 1, + Proxied: true, + }, + }, + }) +} + +func TestCloudflareProxiedOverrideFalse(t *testing.T) { + endpoints := []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "bar.com", + Targets: endpoint.Targets{"127.0.0.1"}, + ProviderSpecific: endpoint.ProviderSpecific{ + endpoint.ProviderSpecificProperty{ + Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", + Value: "false", + }, + }, + }, + } + + AssertActions(t, &CloudFlareProvider{proxiedByDefault: true}, endpoints, []MockAction{ + { + Name: "Create", + ZoneId: "001", + RecordData: cloudflare.DNSRecord{ + Type: "A", + Name: "bar.com", + Content: "127.0.0.1", + TTL: 1, + Proxied: false, + }, + }, + }) +} + +func TestCloudflareProxiedOverrideIllegal(t *testing.T) { + endpoints := []*endpoint.Endpoint{ + { + RecordType: "A", + DNSName: "bar.com", + Targets: endpoint.Targets{"127.0.0.1"}, + ProviderSpecific: endpoint.ProviderSpecific{ + endpoint.ProviderSpecificProperty{ + Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", + Value: "asfasdfa", + }, + }, + }, + } + + AssertActions(t, &CloudFlareProvider{proxiedByDefault: true}, endpoints, []MockAction{ + { + Name: "Create", + ZoneId: "001", + RecordData: cloudflare.DNSRecord{ + Type: "A", + Name: "bar.com", + Content: "127.0.0.1", + TTL: 1, + Proxied: true, + }, + }, + }) +} + +func TestCloudflareSetProxied(t *testing.T) { + var testCases = []struct { + recordType string + domain string + proxiable bool + }{ + {"A", "bar.com", true}, + {"CNAME", "bar.com", true}, + {"TXT", "bar.com", false}, + {"MX", "bar.com", false}, + {"NS", "bar.com", false}, + {"SPF", "bar.com", false}, + {"SRV", "bar.com", false}, + {"A", "*.bar.com", false}, + } + + for _, testCase := range testCases { + endpoints := []*endpoint.Endpoint{ + { + RecordType: testCase.recordType, + DNSName: testCase.domain, + Targets: endpoint.Targets{"127.0.0.1"}, + ProviderSpecific: endpoint.ProviderSpecific{ + endpoint.ProviderSpecificProperty{ + Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", + Value: "true", + }, + }, + }, + } + + AssertActions(t, &CloudFlareProvider{}, endpoints, []MockAction{ + { + Name: "Create", + ZoneId: "001", + RecordData: cloudflare.DNSRecord{ + Type: testCase.recordType, + Name: testCase.domain, + Content: "127.0.0.1", + TTL: 1, + Proxied: testCase.proxiable, + }, + }, + }, testCase.recordType+" record on "+testCase.domain) + } +} + +func TestCloudflareZones(t *testing.T) { provider := &CloudFlareProvider{ - Client: &mockCloudFlareClient{}, - domainFilter: endpoint.NewDomainFilter([]string{"zalando.to."}), + Client: NewMockCloudFlareClient(), + domainFilter: endpoint.NewDomainFilter([]string{"bar.com"}), zoneIDFilter: NewZoneIDFilter([]string{""}), } @@ -256,14 +518,17 @@ func TestCloudFlareZones(t *testing.T) { t.Fatal(err) } - validateCloudFlareZones(t, zones, []cloudflare.Zone{ - {Name: "ext-dns-test.zalando.to."}, - }) + assert.Equal(t, 1, len(zones)) + assert.Equal(t, "bar.com", zones[0].Name) } -func TestRecords(t *testing.T) { +func TestCloudflareRecords(t *testing.T) { + client := NewMockCloudFlareClientWithRecords(map[string][]cloudflare.DNSRecord{ + "001": ExampleDomain, + }) + provider := &CloudFlareProvider{ - Client: &mockCloudFlareClient{}, + Client: client, } ctx := context.Background() @@ -272,23 +537,24 @@ func TestRecords(t *testing.T) { t.Errorf("should not fail, %s", err) } - assert.Equal(t, 1, len(records)) - provider.Client = &mockCloudFlareDNSRecordsFail{} + assert.Equal(t, 2, len(records)) + client.dnsRecordsError = errors.New("failed to list dns records") _, err = provider.Records(ctx) if err == nil { t.Errorf("expected to fail") } - provider.Client = &mockCloudFlareListZonesFail{} + client.dnsRecordsError = nil + client.listZonesError = errors.New("failed to list zones") _, err = provider.Records(ctx) if err == nil { t.Errorf("expected to fail") } } -func TestNewCloudFlareProvider(t *testing.T) { +func TestCloudflareProvider(t *testing.T) { _ = os.Setenv("CF_API_TOKEN", "abc123def") _, err := NewCloudFlareProvider( - endpoint.NewDomainFilter([]string{"ext-dns-test.zalando.to."}), + endpoint.NewDomainFilter([]string{"bar.com"}), NewZoneIDFilter([]string{""}), 25, false, @@ -300,7 +566,7 @@ func TestNewCloudFlareProvider(t *testing.T) { _ = os.Setenv("CF_API_KEY", "xxxxxxxxxxxxxxxxx") _ = os.Setenv("CF_API_EMAIL", "test@test.com") _, err = NewCloudFlareProvider( - endpoint.NewDomainFilter([]string{"ext-dns-test.zalando.to."}), + endpoint.NewDomainFilter([]string{"bar.com"}), NewZoneIDFilter([]string{""}), 1, false, @@ -311,7 +577,7 @@ func TestNewCloudFlareProvider(t *testing.T) { _ = os.Unsetenv("CF_API_KEY") _ = os.Unsetenv("CF_API_EMAIL") _, err = NewCloudFlareProvider( - endpoint.NewDomainFilter([]string{"ext-dns-test.zalando.to."}), + endpoint.NewDomainFilter([]string{"bar.com"}), NewZoneIDFilter([]string{""}), 50, false, @@ -321,20 +587,58 @@ func TestNewCloudFlareProvider(t *testing.T) { } } -func TestApplyChanges(t *testing.T) { +func TestCloudflareApplyChanges(t *testing.T) { changes := &plan.Changes{} + client := NewMockCloudFlareClient() provider := &CloudFlareProvider{ - Client: &mockCloudFlareClient{}, + Client: client, } - changes.Create = []*endpoint.Endpoint{{DNSName: "new.ext-dns-test.zalando.to.", Targets: endpoint.Targets{"target"}}, {DNSName: "new.ext-dns-test.unrelated.to.", Targets: endpoint.Targets{"target"}}} - changes.Delete = []*endpoint.Endpoint{{DNSName: "foobar.ext-dns-test.zalando.to.", Targets: endpoint.Targets{"target"}}} - changes.UpdateOld = []*endpoint.Endpoint{{DNSName: "foobar.ext-dns-test.zalando.to.", Targets: endpoint.Targets{"target-old"}}} - changes.UpdateNew = []*endpoint.Endpoint{{DNSName: "foobar.ext-dns-test.zalando.to.", Targets: endpoint.Targets{"target-new"}}} + changes.Create = []*endpoint.Endpoint{{ + DNSName: "new.bar.com", + Targets: endpoint.Targets{"target"}, + }, { + DNSName: "new.ext-dns-test.unrelated.to", + Targets: endpoint.Targets{"target"}, + }} + changes.Delete = []*endpoint.Endpoint{{ + DNSName: "foobar.bar.com", + Targets: endpoint.Targets{"target"}, + }} + changes.UpdateOld = []*endpoint.Endpoint{{ + DNSName: "foobar.bar.com", + Targets: endpoint.Targets{"target-old"}, + }} + changes.UpdateNew = []*endpoint.Endpoint{{ + DNSName: "foobar.bar.com", + Targets: endpoint.Targets{"target-new"}, + }} err := provider.ApplyChanges(context.Background(), changes) + if err != nil { t.Errorf("should not fail, %s", err) } + td.Cmp(t, client.Actions, []MockAction{ + MockAction{ + Name: "Create", + ZoneId: "001", + RecordData: cloudflare.DNSRecord{ + Name: "new.bar.com", + Content: "target", + TTL: 1, + }, + }, + MockAction{ + Name: "Create", + ZoneId: "001", + RecordData: cloudflare.DNSRecord{ + Name: "foobar.bar.com", + Content: "target-new", + TTL: 1, + }, + }, + }) + // empty changes changes.Create = []*endpoint.Endpoint{} changes.Delete = []*endpoint.Endpoint{} @@ -347,7 +651,7 @@ func TestApplyChanges(t *testing.T) { } } -func TestCloudFlareGetRecordID(t *testing.T) { +func TestCloudflareGetRecordID(t *testing.T) { p := &CloudFlareProvider{} records := []cloudflare.DNSRecord{ { @@ -376,15 +680,7 @@ func TestCloudFlareGetRecordID(t *testing.T) { })[0]) } -func validateCloudFlareZones(t *testing.T, zones []cloudflare.Zone, expected []cloudflare.Zone) { - require.Len(t, zones, len(expected)) - - for i, zone := range zones { - assert.Equal(t, expected[i].Name, zone.Name) - } -} - -func TestGroupByNameAndType(t *testing.T) { +func TestCloudflareGroupByNameAndType(t *testing.T) { testCases := []struct { Name string Records []cloudflare.DNSRecord @@ -605,3 +901,78 @@ func TestGroupByNameAndType(t *testing.T) { assert.ElementsMatch(t, groupByNameAndType(tc.Records), tc.ExpectedEndpoints) } } + +func TestCloudflareComplexUpdate(t *testing.T) { + client := NewMockCloudFlareClientWithRecords(map[string][]cloudflare.DNSRecord{ + "001": ExampleDomain, + }) + + provider := &CloudFlareProvider{ + Client: client, + } + ctx := context.Background() + + records, err := provider.Records(ctx) + + if err != nil { + t.Errorf("should not fail, %s", err) + } + + plan := &plan.Plan{ + Current: records, + Desired: []*endpoint.Endpoint{ + { + DNSName: "foobar.bar.com", + Targets: endpoint.Targets{"1.2.3.4", "2.3.4.5"}, + RecordType: endpoint.RecordTypeA, + RecordTTL: endpoint.TTL(defaultCloudFlareRecordTTL), + Labels: endpoint.Labels{}, + ProviderSpecific: endpoint.ProviderSpecific{ + { + Name: "external-dns.alpha.kubernetes.io/cloudflare-proxied", + Value: "true", + }, + }, + }, + }, + DomainFilter: endpoint.NewDomainFilter([]string{"bar.com"}), + } + + planned := plan.Calculate() + + err = provider.ApplyChanges(context.Background(), planned.Changes) + + if err != nil { + t.Errorf("should not fail, %s", err) + } + + td.CmpDeeply(t, client.Actions, []MockAction{ + MockAction{ + Name: "Delete", + ZoneId: "001", + RecordId: "1234567890", + }, + MockAction{ + Name: "Create", + ZoneId: "001", + RecordData: cloudflare.DNSRecord{ + Name: "foobar.bar.com", + Type: "A", + Content: "1.2.3.4", + TTL: 1, + Proxied: true, + }, + }, + MockAction{ + Name: "Create", + ZoneId: "001", + RecordData: cloudflare.DNSRecord{ + Name: "foobar.bar.com", + Type: "A", + Content: "2.3.4.5", + TTL: 1, + Proxied: true, + }, + }, + }) +} diff --git a/provider/provider_test.go b/provider/provider_test.go index 08fc2989d..9e3c7c26e 100644 --- a/provider/provider_test.go +++ b/provider/provider_test.go @@ -17,9 +17,18 @@ limitations under the License. package provider import ( + "io/ioutil" + "os" "testing" + + log "github.com/sirupsen/logrus" ) +func TestMain(m *testing.M) { + log.SetOutput(ioutil.Discard) + os.Exit(m.Run()) +} + func TestEnsureTrailingDot(t *testing.T) { for _, tc := range []struct { input, expected string From 7a0a9382b98965067828c376bbe2e828bd982d36 Mon Sep 17 00:00:00 2001 From: Adam Stankiewicz Date: Wed, 22 Apr 2020 16:01:00 +0200 Subject: [PATCH 2/4] Fix bugs in CloudFlare update logic --- provider/cloudflare.go | 139 ++++++++++++++++++++---------------- provider/cloudflare_test.go | 95 +++++++++++++++--------- provider/provider.go | 22 ++++++ provider/provider_test.go | 11 +++ 4 files changed, 170 insertions(+), 97 deletions(-) diff --git a/provider/cloudflare.go b/provider/cloudflare.go index 5fbf19351..01c464b37 100644 --- a/provider/cloudflare.go +++ b/provider/cloudflare.go @@ -20,7 +20,6 @@ import ( "context" "fmt" "os" - "sort" "strconv" "strings" @@ -111,8 +110,8 @@ type CloudFlareProvider struct { // cloudFlareChange differentiates between ChangActions type cloudFlareChange struct { - Action string - ResourceRecordSet []cloudflare.DNSRecord + Action string + ResourceRecord cloudflare.DNSRecord } // NewCloudFlareProvider initializes a new CloudFlare DNS based Provider. @@ -199,15 +198,39 @@ func (p *CloudFlareProvider) Records(ctx context.Context) ([]*endpoint.Endpoint, // ApplyChanges applies a given set of changes in a given zone. func (p *CloudFlareProvider) ApplyChanges(ctx context.Context, changes *plan.Changes) error { - proxiedByDefault := p.proxiedByDefault + cloudflareChanges := []*cloudFlareChange{} - combinedChanges := make([]*cloudFlareChange, 0, len(changes.Create)+len(changes.UpdateNew)+len(changes.Delete)) + for _, endpoint := range changes.Create { + for _, target := range endpoint.Targets { + cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareCreate, endpoint, target)) + } + } - combinedChanges = append(combinedChanges, newCloudFlareChanges(cloudFlareCreate, changes.Create, proxiedByDefault)...) - combinedChanges = append(combinedChanges, newCloudFlareChanges(cloudFlareUpdate, changes.UpdateNew, proxiedByDefault)...) - combinedChanges = append(combinedChanges, newCloudFlareChanges(cloudFlareDelete, changes.Delete, proxiedByDefault)...) + for i, desired := range changes.UpdateNew { + current := changes.UpdateOld[i] - return p.submitChanges(ctx, combinedChanges) + add, remove, leave := difference(current.Targets, desired.Targets) + + for _, a := range add { + cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareCreate, desired, a)) + } + + for _, a := range leave { + cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareUpdate, desired, a)) + } + + for _, a := range remove { + cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareDelete, current, a)) + } + } + + for _, endpoint := range changes.Delete { + for _, target := range endpoint.Targets { + cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareDelete, endpoint, target)) + } + } + + return p.submitChanges(ctx, cloudflareChanges) } // submitChanges takes a zone and a collection of Changes and sends them as a single transaction. @@ -231,12 +254,11 @@ func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloud } for _, change := range changes { logFields := log.Fields{ - "record": change.ResourceRecordSet[0].Name, - "type": change.ResourceRecordSet[0].Type, - "ttl": change.ResourceRecordSet[0].TTL, - "targets": len(change.ResourceRecordSet), - "action": change.Action, - "zone": zoneID, + "record": change.ResourceRecord.Name, + "type": change.ResourceRecord.Type, + "ttl": change.ResourceRecord.TTL, + "action": change.Action, + "zone": zoneID, } log.WithFields(logFields).Info("Changing record.") @@ -245,24 +267,30 @@ func (p *CloudFlareProvider) submitChanges(ctx context.Context, changes []*cloud continue } - 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) - } + if change.Action == cloudFlareUpdate { + recordID := p.getRecordID(records, change.ResourceRecord) + if recordID == "" { + log.WithFields(logFields).Errorf("failed to find previous record: %v", change.ResourceRecord) + continue } - } - - 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) - } + err := p.Client.UpdateDNSRecord(zoneID, recordID, change.ResourceRecord) + if err != nil { + log.WithFields(logFields).Errorf("failed to delete record: %v", err) + } + } else if change.Action == cloudFlareDelete { + recordID := p.getRecordID(records, change.ResourceRecord) + if recordID == "" { + log.WithFields(logFields).Errorf("failed to find previous record: %v", change.ResourceRecord) + continue + } + err := p.Client.DeleteDNSRecord(zoneID, recordID) + if err != nil { + log.WithFields(logFields).Errorf("failed to delete record: %v", err) + } + } else if change.Action == cloudFlareCreate { + _, err := p.Client.CreateDNSRecord(zoneID, change.ResourceRecord) + if err != nil { + log.WithFields(logFields).Errorf("failed to create record: %v", err) } } } @@ -281,9 +309,9 @@ func (p *CloudFlareProvider) changesByZone(zones []cloudflare.Zone, changeSet [] } for _, c := range changeSet { - zoneID, _ := zoneNameIDMapper.FindZone(c.ResourceRecordSet[0].Name) + zoneID, _ := zoneNameIDMapper.FindZone(c.ResourceRecord.Name) if zoneID == "" { - log.Debugf("Skipping record %s because no hosted zone matching record DNS Name was detected", c.ResourceRecordSet[0].Name) + log.Debugf("Skipping record %s because no hosted zone matching record DNS Name was detected", c.ResourceRecord.Name) continue } changes[zoneID] = append(changes[zoneID], c) @@ -292,51 +320,36 @@ func (p *CloudFlareProvider) changesByZone(zones []cloudflare.Zone, changeSet [] return changes } -func (p *CloudFlareProvider) getRecordIDs(records []cloudflare.DNSRecord, record cloudflare.DNSRecord) []string { - recordIDs := make([]string, 0) +func (p *CloudFlareProvider) getRecordID(records []cloudflare.DNSRecord, record cloudflare.DNSRecord) string { for _, zoneRecord := range records { - if zoneRecord.Name == record.Name && zoneRecord.Type == record.Type { - recordIDs = append(recordIDs, zoneRecord.ID) + if zoneRecord.Name == record.Name && zoneRecord.Type == record.Type && zoneRecord.Content == record.Content { + return zoneRecord.ID } } - sort.Strings(recordIDs) - return recordIDs + return "" } -// newCloudFlareChanges returns a collection of Changes based on the given records and action. -func newCloudFlareChanges(action string, endpoints []*endpoint.Endpoint, proxiedByDefault bool) []*cloudFlareChange { - changes := make([]*cloudFlareChange, 0, len(endpoints)) - - for _, endpoint := range endpoints { - changes = append(changes, newCloudFlareChange(action, endpoint, proxiedByDefault)) - } - - return changes -} - -func newCloudFlareChange(action string, endpoint *endpoint.Endpoint, proxiedByDefault bool) *cloudFlareChange { +func (p *CloudFlareProvider) newCloudFlareChange(action string, endpoint *endpoint.Endpoint, target string) *cloudFlareChange { ttl := defaultCloudFlareRecordTTL - proxied := shouldBeProxied(endpoint, proxiedByDefault) + proxied := shouldBeProxied(endpoint, p.proxiedByDefault) if endpoint.RecordTTL.IsConfigured() { ttl = int(endpoint.RecordTTL) } - resourceRecordSet := make([]cloudflare.DNSRecord, len(endpoint.Targets)) + if len(endpoint.Targets) > 1 { + log.Errorf("Updates should have just one target") + } - for i := range endpoint.Targets { - resourceRecordSet[i] = cloudflare.DNSRecord{ + return &cloudFlareChange{ + Action: action, + ResourceRecord: cloudflare.DNSRecord{ Name: endpoint.DNSName, TTL: ttl, Proxied: proxied, Type: endpoint.RecordType, - Content: endpoint.Targets[i], - } - } - - return &cloudFlareChange{ - Action: action, - ResourceRecordSet: resourceRecordSet, + Content: target, + }, } } diff --git a/provider/cloudflare_test.go b/provider/cloudflare_test.go index 93ed53d29..d8880bcca 100644 --- a/provider/cloudflare_test.go +++ b/provider/cloudflare_test.go @@ -56,6 +56,15 @@ var ExampleDomain = []cloudflare.DNSRecord{ Content: "1.2.3.4", Proxied: false, }, + { + ID: "2345678901", + ZoneID: "001", + Name: "foobar.bar.com", + Type: endpoint.RecordTypeA, + TTL: 120, + Content: "3.4.5.6", + Proxied: false, + }, { ID: "1231231233", ZoneID: "002", @@ -655,29 +664,46 @@ func TestCloudflareGetRecordID(t *testing.T) { p := &CloudFlareProvider{} records := []cloudflare.DNSRecord{ { - Name: "foo.com", - Type: endpoint.RecordTypeCNAME, - ID: "1", + Name: "foo.com", + Type: endpoint.RecordTypeCNAME, + Content: "foobar", + ID: "1", }, { - Name: "bar.de", - Type: endpoint.RecordTypeA, - ID: "2", + Name: "bar.de", + Type: endpoint.RecordTypeA, + Content: "1.2.3.4", + ID: "2", }, } - assert.Len(t, p.getRecordIDs(records, cloudflare.DNSRecord{ - Name: "foo.com", - Type: endpoint.RecordTypeA, - }), 0) - assert.Len(t, p.getRecordIDs(records, cloudflare.DNSRecord{ - Name: "bar.de", - Type: endpoint.RecordTypeA, - }), 1) - assert.Equal(t, "2", p.getRecordIDs(records, cloudflare.DNSRecord{ - Name: "bar.de", - Type: endpoint.RecordTypeA, - })[0]) + assert.Equal(t, "", p.getRecordID(records, cloudflare.DNSRecord{ + Name: "foo.com", + Type: endpoint.RecordTypeA, + Content: "foobar", + })) + + assert.Equal(t, "", p.getRecordID(records, cloudflare.DNSRecord{ + Name: "foo.com", + Type: endpoint.RecordTypeCNAME, + Content: "fizfuz", + })) + + assert.Equal(t, "1", p.getRecordID(records, cloudflare.DNSRecord{ + Name: "foo.com", + Type: endpoint.RecordTypeCNAME, + Content: "foobar", + })) + assert.Equal(t, "", p.getRecordID(records, cloudflare.DNSRecord{ + Name: "bar.de", + Type: endpoint.RecordTypeA, + Content: "2.3.4.5", + })) + assert.Equal(t, "2", p.getRecordID(records, cloudflare.DNSRecord{ + Name: "bar.de", + Type: endpoint.RecordTypeA, + Content: "1.2.3.4", + })) } func TestCloudflareGroupByNameAndType(t *testing.T) { @@ -947,22 +973,6 @@ func TestCloudflareComplexUpdate(t *testing.T) { } td.CmpDeeply(t, client.Actions, []MockAction{ - MockAction{ - Name: "Delete", - ZoneId: "001", - RecordId: "1234567890", - }, - MockAction{ - Name: "Create", - ZoneId: "001", - RecordData: cloudflare.DNSRecord{ - Name: "foobar.bar.com", - Type: "A", - Content: "1.2.3.4", - TTL: 1, - Proxied: true, - }, - }, MockAction{ Name: "Create", ZoneId: "001", @@ -974,5 +984,22 @@ func TestCloudflareComplexUpdate(t *testing.T) { Proxied: true, }, }, + MockAction{ + Name: "Update", + ZoneId: "001", + RecordId: "1234567890", + RecordData: cloudflare.DNSRecord{ + Name: "foobar.bar.com", + Type: "A", + Content: "1.2.3.4", + TTL: 1, + Proxied: true, + }, + }, + MockAction{ + Name: "Delete", + ZoneId: "001", + RecordId: "2345678901", + }, }) } diff --git a/provider/provider.go b/provider/provider.go index fcd12018c..1a5f7c194 100644 --- a/provider/provider.go +++ b/provider/provider.go @@ -50,3 +50,25 @@ func ensureTrailingDot(hostname string) string { return strings.TrimSuffix(hostname, ".") + "." } + +// Tells which entries need to be respectively +// added, removed, or left untouched for "current" to be transformed to "desired" +func difference(current, desired []string) (add []string, remove []string, leave []string) { + index := make(map[string]struct{}, len(current)) + for _, x := range current { + index[x] = struct{}{} + } + for _, x := range desired { + if _, found := index[x]; found { + leave = append(leave, x) + delete(index, x) + } else { + add = append(add, x) + delete(index, x) + } + } + for x := range index { + remove = append(remove, x) + } + return +} diff --git a/provider/provider_test.go b/provider/provider_test.go index 9e3c7c26e..a5198deb4 100644 --- a/provider/provider_test.go +++ b/provider/provider_test.go @@ -22,6 +22,8 @@ import ( "testing" log "github.com/sirupsen/logrus" + + "github.com/stretchr/testify/assert" ) func TestMain(m *testing.M) { @@ -44,3 +46,12 @@ func TestEnsureTrailingDot(t *testing.T) { } } } + +func TestDifference(t *testing.T) { + current := []string{"foo", "bar"} + desired := []string{"bar", "baz"} + add, remove, leave := difference(current, desired) + assert.Equal(t, add, []string{"baz"}) + assert.Equal(t, remove, []string{"foo"}) + assert.Equal(t, leave, []string{"bar"}) +} From 884379e4b3189e32da3b8e59a7630a0fb2dbe1c3 Mon Sep 17 00:00:00 2001 From: Adam Stankiewicz Date: Mon, 11 May 2020 20:49:11 +0200 Subject: [PATCH 3/4] Fix tests --- provider/cloudflare/cloudflare.go | 2 +- provider/provider.go | 2 +- provider/provider_test.go | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/provider/cloudflare/cloudflare.go b/provider/cloudflare/cloudflare.go index c32026b0a..d910226fc 100644 --- a/provider/cloudflare/cloudflare.go +++ b/provider/cloudflare/cloudflare.go @@ -210,7 +210,7 @@ func (p *CloudFlareProvider) ApplyChanges(ctx context.Context, changes *plan.Cha for i, desired := range changes.UpdateNew { current := changes.UpdateOld[i] - add, remove, leave := difference(current.Targets, desired.Targets) + add, remove, leave := provider.Difference(current.Targets, desired.Targets) for _, a := range add { cloudflareChanges = append(cloudflareChanges, p.newCloudFlareChange(cloudFlareCreate, desired, a)) diff --git a/provider/provider.go b/provider/provider.go index 29debdb5c..74f691164 100644 --- a/provider/provider.go +++ b/provider/provider.go @@ -53,7 +53,7 @@ func EnsureTrailingDot(hostname string) string { // Tells which entries need to be respectively // added, removed, or left untouched for "current" to be transformed to "desired" -func difference(current, desired []string) (add []string, remove []string, leave []string) { +func Difference(current, desired []string) (add []string, remove []string, leave []string) { index := make(map[string]struct{}, len(current)) for _, x := range current { index[x] = struct{}{} diff --git a/provider/provider_test.go b/provider/provider_test.go index e9f8deb9b..57d48e70e 100644 --- a/provider/provider_test.go +++ b/provider/provider_test.go @@ -50,7 +50,7 @@ func TestEnsureTrailingDot(t *testing.T) { func TestDifference(t *testing.T) { current := []string{"foo", "bar"} desired := []string{"bar", "baz"} - add, remove, leave := difference(current, desired) + add, remove, leave := Difference(current, desired) assert.Equal(t, add, []string{"baz"}) assert.Equal(t, remove, []string{"foo"}) assert.Equal(t, leave, []string{"bar"}) From a1ffcb6f8e58a846e74fe9617433b5a482e2de8f Mon Sep 17 00:00:00 2001 From: Adam Stankiewicz Date: Sat, 16 May 2020 17:17:39 +0200 Subject: [PATCH 4/4] Apply changes --- provider/provider.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/provider/provider.go b/provider/provider.go index 74f691164..fbe8945f0 100644 --- a/provider/provider.go +++ b/provider/provider.go @@ -51,9 +51,10 @@ func EnsureTrailingDot(hostname string) string { return strings.TrimSuffix(hostname, ".") + "." } -// Tells which entries need to be respectively +// Difference tells which entries need to be respectively // added, removed, or left untouched for "current" to be transformed to "desired" -func Difference(current, desired []string) (add []string, remove []string, leave []string) { +func Difference(current, desired []string) ([]string, []string, []string) { + add, remove, leave := []string{}, []string{}, []string{} index := make(map[string]struct{}, len(current)) for _, x := range current { index[x] = struct{}{} @@ -70,5 +71,5 @@ func Difference(current, desired []string) (add []string, remove []string, leave for x := range index { remove = append(remove, x) } - return + return add, remove, leave }