Merge branch 'master' into property-comparator

.travis.yml

@@ -13,16 +13,28 @@ matrix:
   allow_failures:
   - go: tip
 
-env:
-- GOLANGCI_RELEASE="v1.26.0"
-
-before_install:
-- GO111MODULE=off go get github.com/mattn/goveralls
-- GO111MODULE=off go get github.com/lawrencewoodman/roveralls
-- curl -sfL https://install.goreleaser.com/github.com/golangci/golangci-lint.sh | sh -s -- -b $(go env GOPATH)/bin ${GOLANGCI_RELEASE}
+cache:
+  directories:
+    - $GOPATH/pkg/mod
 
 script:
 - make test
-- make lint
-- travis_wait 20 roveralls
-- goveralls -coverprofile=roveralls.coverprofile -service=travis-ci
+
+jobs:
+  include:
+  - name: "Linting"
+    go: "1.14.x"
+    env:
+    - GOLANGCI_RELEASE="v1.26.0"
+    before_install:
+    - curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/master/install.sh | sh -s -- -b $(go env GOPATH)/bin ${GOLANGCI_RELEASE}
+    script:
+    - make lint
+  - name: "Coverage"
+    go: "1.14.x"
+    before_install:
+    - GO111MODULE=off go get github.com/mattn/goveralls
+    - GO111MODULE=off go get github.com/lawrencewoodman/roveralls
+    script:
+    - travis_wait 20 roveralls
+    - goveralls -coverprofile=roveralls.coverprofile -service=travis-ci

Makefile

@@ -38,7 +38,7 @@ lint:
 .PHONY: verify test
 
 test:
-	go test -v -race $(shell go list ./... | grep -v /vendor/)
+	go test -v -race ./...
 
 # The build targets allow to build the binary and docker image
 .PHONY: build build.docker build.mini

go.mod

@@ -28,7 +28,6 @@ require (
 	github.com/gobs/pretty v0.0.0-20180724170744-09732c25a95b // indirect
 	github.com/golang/sync v0.0.0-20180314180146-1d60e4601c6f
 	github.com/gophercloud/gophercloud v0.1.0
-	github.com/heptio/contour v0.15.0
 	github.com/infobloxopen/infoblox-go-client v0.0.0-20180606155407-61dc5f9b0a65
 	github.com/linki/instrumented_http v0.2.0
 	github.com/linode/linodego v0.3.0
@@ -42,7 +41,8 @@ require (
 	github.com/oracle/oci-go-sdk v1.8.0
 	github.com/ovh/go-ovh v0.0.0-20181109152953-ba5adb4cf014
 	github.com/pkg/errors v0.9.1
-	github.com/prometheus/client_golang v1.0.0
+	github.com/projectcontour/contour v1.4.0
+	github.com/prometheus/client_golang v1.1.0
 	github.com/sanyu/dynectsoap v0.0.0-20181203081243-b83de5edc4e0
 	github.com/sergi/go-diff v1.1.0 // indirect
 	github.com/sirupsen/logrus v1.4.2
@@ -53,17 +53,17 @@ require (
 	github.com/vinyldns/go-vinyldns v0.0.0-20190611170422-7119fe55ed92
 	github.com/vultr/govultr v0.3.2
 	go.etcd.io/etcd v0.5.0-alpha.5.0.20200401174654-e694b7bb0875
-	golang.org/x/net v0.0.0-20190923162816-aa69164e4478
+	golang.org/x/net v0.0.0-20191004110552-13f9640d40b9
 	golang.org/x/oauth2 v0.0.0-20190604053449-0f29369cfe45
 	google.golang.org/api v0.9.0
 	gopkg.in/ns1/ns1-go.v2 v2.0.0-20190322154155-0dafb5275fd1
-	gopkg.in/yaml.v2 v2.2.5
+	gopkg.in/yaml.v2 v2.2.7
 	istio.io/api v0.0.0-20190820204432-483f2547d882
 	istio.io/istio v0.0.0-20190322063008-2b1331886076
-	k8s.io/api v0.0.0-20190620084959-7cf5895f2711
-	k8s.io/apimachinery v0.0.0-20190612205821-1799e75a0719
+	k8s.io/api v0.17.0
+	k8s.io/apimachinery v0.17.0
 	k8s.io/client-go v10.0.0+incompatible
-	k8s.io/klog v0.3.1
+	k8s.io/klog v1.0.0
 )
 
 replace (
@@ -79,4 +79,5 @@ replace (
 	k8s.io/apimachinery => k8s.io/apimachinery v0.0.0-20190817221809-bf4de9df677c
 	k8s.io/client-go => k8s.io/client-go v0.0.0-20190817222206-ee6c071a42cf
 	k8s.io/klog => github.com/mikkeloscar/knolog v0.0.0-20190326191552-80742771eb6b
+	github.com/envoyproxy/go-control-plane => github.com/envoyproxy/go-control-plane v0.8.7-0.20190821215049-f062b07a671a
 )

internal/config/config.go

@@ -0,0 +1,4 @@
+package config
+
+// FAST_POLL used for fast testing
+var FAST_POLL = false

internal/testutils/init.go

@@ -0,0 +1,23 @@
+package testutils
+
+import (
+	"io/ioutil"
+	"os"
+
+	"log"
+
+	"github.com/sirupsen/logrus"
+	"sigs.k8s.io/external-dns/internal/config"
+)
+
+func init() {
+	config.FAST_POLL = true
+	if os.Getenv("DEBUG") == "" {
+		logrus.SetOutput(ioutil.Discard)
+		log.SetOutput(ioutil.Discard)
+	} else {
+		if level, err := logrus.ParseLevel(os.Getenv("DEBUG")); err == nil {
+			logrus.SetLevel(level)
+		}
+	}
+}

provider/cloudflare/OWNERS

@@ -0,0 +1,2 @@
+approvers:
+  - sheerun

provider/cloudflare/cloudflare.go

@@ -20,7 +20,6 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"sort"
 	"strconv"
 	"strings"
 
@@ -113,8 +112,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.
@@ -201,15 +200,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))
+		}
+	}
+
+	for i, desired := range changes.UpdateNew {
+		current := changes.UpdateOld[i]
+
+		add, remove, leave := provider.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))
+		}
 
-	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 _, 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, combinedChanges)
+	return p.submitChanges(ctx, cloudflareChanges)
 }
 
 func (p *CloudFlareProvider) PropertyValuesEqual(name string, previous string, current string) bool {
@@ -241,12 +264,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.")
@@ -255,24 +277,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)
 				}
 			}
 		}
@@ -291,9 +319,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)
@@ -302,51 +330,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
-}
-
-// 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
+	return ""
 }
 
-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,
+		},
 	}
 }