Kops Compatibility #1: Set RecordType at source (kubernetes-sigs#248)

* set RecordType at source

* add comments for linting

endpoint/endpoint.go

@@ -24,6 +24,12 @@ import (
 const (
 	// OwnerLabelKey is the name of the label that defines the owner of an Endpoint.
 	OwnerLabelKey = "owner"
+	// RecordTypeA is a RecordType enum value
+	RecordTypeA = "A"
+	// RecordTypeCNAME is a RecordType enum value
+	RecordTypeCNAME = "CNAME"
+	// RecordTypeTXT is a RecordType enum value
+	RecordTypeTXT = "TXT"
 )
 
 // Endpoint is a high-level way of a connection between a service and an IP

internal/testutils/endpoint_test.go

@@ -32,22 +32,22 @@ func ExampleSameEndpoints() {
 		{
 			DNSName:    "example.org",
 			Target:     "load-balancer.org",
-			RecordType: "TXT",
+			RecordType: endpoint.RecordTypeTXT,
 		},
 		{
 			DNSName:    "abc.com",
 			Target:     "something",
-			RecordType: "TXT",
+			RecordType: endpoint.RecordTypeTXT,
 		},
 		{
 			DNSName:    "abc.com",
 			Target:     "1.2.3.4",
-			RecordType: "A",
+			RecordType: endpoint.RecordTypeA,
 		},
 		{
 			DNSName:    "bbc.com",
 			Target:     "foo.com",
-			RecordType: "CNAME",
+			RecordType: endpoint.RecordTypeCNAME,
 		},
 	}
 	sort.Sort(byAllFields(eps))

plan/plan_test.go

@@ -30,21 +30,21 @@ func TestCalculate(t *testing.T) {
 	// empty list of records
 	empty := []*endpoint.Endpoint{}
 	// a simple entry
-	fooV1 := []*endpoint.Endpoint{endpoint.NewEndpoint("foo", "v1", "CNAME")}
+	fooV1 := []*endpoint.Endpoint{endpoint.NewEndpoint("foo", "v1", endpoint.RecordTypeCNAME)}
 	// the same entry but with different target
-	fooV2 := []*endpoint.Endpoint{endpoint.NewEndpoint("foo", "v2", "CNAME")}
+	fooV2 := []*endpoint.Endpoint{endpoint.NewEndpoint("foo", "v2", endpoint.RecordTypeCNAME)}
 	// another simple entry
-	bar := []*endpoint.Endpoint{endpoint.NewEndpoint("bar", "v1", "CNAME")}
+	bar := []*endpoint.Endpoint{endpoint.NewEndpoint("bar", "v1", endpoint.RecordTypeCNAME)}
 
 	// test case with labels
-	noLabels := []*endpoint.Endpoint{endpoint.NewEndpoint("foo", "v2", "CNAME")}
+	noLabels := []*endpoint.Endpoint{endpoint.NewEndpoint("foo", "v2", endpoint.RecordTypeCNAME)}
 	labeledV2 := []*endpoint.Endpoint{newEndpointWithOwner("foo", "v2", "123")}
 	labeledV1 := []*endpoint.Endpoint{newEndpointWithOwner("foo", "v1", "123")}
 
 	// test case with type inheritance
 	noType := []*endpoint.Endpoint{endpoint.NewEndpoint("foo", "v2", "")}
-	typedV2 := []*endpoint.Endpoint{endpoint.NewEndpoint("foo", "v2", "A")}
-	typedV1 := []*endpoint.Endpoint{endpoint.NewEndpoint("foo", "v1", "A")}
+	typedV2 := []*endpoint.Endpoint{endpoint.NewEndpoint("foo", "v2", endpoint.RecordTypeA)}
+	typedV1 := []*endpoint.Endpoint{endpoint.NewEndpoint("foo", "v1", endpoint.RecordTypeA)}
 
 	for _, tc := range []struct {
 		policies                             []Policy
@@ -165,7 +165,7 @@ func validateEntries(t *testing.T, entries, expected []*endpoint.Endpoint) {
 }
 
 func newEndpointWithOwner(dnsName, target, ownerID string) *endpoint.Endpoint {
-	e := endpoint.NewEndpoint(dnsName, target, "CNAME")
+	e := endpoint.NewEndpoint(dnsName, target, endpoint.RecordTypeCNAME)
 	e.Labels[endpoint.OwnerLabelKey] = ownerID
 	return e
 }

provider/aws.go

@@ -157,7 +157,7 @@ func (p *AWSProvider) Records() (endpoints []*endpoint.Endpoint, _ error) {
 			}
 
 			if r.AliasTarget != nil {
-				endpoints = append(endpoints, endpoint.NewEndpoint(wildcardUnescape(aws.StringValue(r.Name)), aws.StringValue(r.AliasTarget.DNSName), "ALIAS"))
+				endpoints = append(endpoints, endpoint.NewEndpoint(wildcardUnescape(aws.StringValue(r.Name)), aws.StringValue(r.AliasTarget.DNSName), endpoint.RecordTypeCNAME))
 			}
 		}
 
@@ -301,7 +301,7 @@ func newChange(action string, endpoint *endpoint.Endpoint) *route53.Change {
 			EvaluateTargetHealth: aws.Bool(evaluateTargetHealth),
 		}
 	} else {
-		change.ResourceRecordSet.Type = aws.String(suitableType(endpoint))
+		change.ResourceRecordSet.Type = aws.String(endpoint.RecordType)
 		change.ResourceRecordSet.TTL = aws.Int64(recordTTL)
 		change.ResourceRecordSet.ResourceRecords = []*route53.ResourceRecord{
 			{
@@ -330,11 +330,11 @@ func suitableZone(hostname string, zones map[string]*route53.HostedZone) *route5
 
 // isAWSLoadBalancer determines if a given hostname belongs to an AWS load balancer.
 func isAWSLoadBalancer(ep *endpoint.Endpoint) bool {
-	if ep.RecordType == "" {
+	if ep.RecordType == endpoint.RecordTypeCNAME {
 		return canonicalHostedZone(ep.Target) != ""
 	}
 
-	return ep.RecordType == "ALIAS"
+	return false
 }
 
 // canonicalHostedZone returns the matching canonical zone for a given hostname.

provider/azure.go

@@ -21,10 +21,10 @@ import (
 	"io/ioutil"
 	"strings"
 
-	"gopkg.in/yaml.v2"
-
 	log "github.com/Sirupsen/logrus"
 
+	yaml "gopkg.in/yaml.v2"
+
 	"github.com/Azure/azure-sdk-for-go/arm/dns"
 	"github.com/Azure/go-autorest/autorest"
 	"github.com/Azure/go-autorest/autorest/adal"
@@ -249,7 +249,6 @@ func (p *AzureProvider) mapChanges(zones []dns.Zone, changes *plan.Changes) (azu
 			return
 		}
 		// Ensure the record type is suitable
-		change.RecordType = suitableType(change)
 		changeMap[zone] = append(changeMap[zone], change)
 	}
 

provider/azure_test.go

@@ -89,11 +89,11 @@ func createMockRecordSet(name, recordType, value string) dns.RecordSet {
 	var getterFunc func(value string) *dns.RecordSetProperties
 
 	switch recordType {
-	case "A":
+	case endpoint.RecordTypeA:
 		getterFunc = aRecordSetPropertiesGetter
-	case "CNAME":
+	case endpoint.RecordTypeCNAME:
 		getterFunc = cNameRecordSetPropertiesGetter
-	case "TXT":
+	case endpoint.RecordTypeTXT:
 		getterFunc = txtRecordSetPropertiesGetter
 	default:
 		getterFunc = othersRecordSetPropertiesGetter
@@ -161,11 +161,11 @@ func TestAzureRecord(t *testing.T) {
 		mockRecordSet: &[]dns.RecordSet{
 			createMockRecordSet("@", "NS", "ns1-03.azure-dns.com."),
 			createMockRecordSet("@", "SOA", "Email: azuredns-hostmaster.microsoft.com"),
-			createMockRecordSet("@", "A", "123.123.123.122"),
-			createMockRecordSet("@", "TXT", "heritage=external-dns,external-dns/owner=default"),
-			createMockRecordSet("nginx", "A", "123.123.123.123"),
-			createMockRecordSet("nginx", "TXT", "heritage=external-dns,external-dns/owner=default"),
-			createMockRecordSet("hack", "CNAME", "hack.azurewebsites.net"),
+			createMockRecordSet("@", endpoint.RecordTypeA, "123.123.123.122"),
+			createMockRecordSet("@", endpoint.RecordTypeTXT, "heritage=external-dns,external-dns/owner=default"),
+			createMockRecordSet("nginx", endpoint.RecordTypeA, "123.123.123.123"),
+			createMockRecordSet("nginx", endpoint.RecordTypeTXT, "heritage=external-dns,external-dns/owner=default"),
+			createMockRecordSet("hack", endpoint.RecordTypeCNAME, "hack.azurewebsites.net"),
 		},
 	}
 
@@ -176,11 +176,11 @@ func TestAzureRecord(t *testing.T) {
 		t.Fatal(err)
 	}
 	expected := []*endpoint.Endpoint{
-		endpoint.NewEndpoint("example.com", "123.123.123.122", "A"),
-		endpoint.NewEndpoint("example.com", "heritage=external-dns,external-dns/owner=default", "TXT"),
-		endpoint.NewEndpoint("nginx.example.com", "123.123.123.123", "A"),
-		endpoint.NewEndpoint("nginx.example.com", "heritage=external-dns,external-dns/owner=default", "TXT"),
-		endpoint.NewEndpoint("hack.example.com", "hack.azurewebsites.net", "CNAME"),
+		endpoint.NewEndpoint("example.com", "123.123.123.122", endpoint.RecordTypeA),
+		endpoint.NewEndpoint("example.com", "heritage=external-dns,external-dns/owner=default", endpoint.RecordTypeTXT),
+		endpoint.NewEndpoint("nginx.example.com", "123.123.123.123", endpoint.RecordTypeA),
+		endpoint.NewEndpoint("nginx.example.com", "heritage=external-dns,external-dns/owner=default", endpoint.RecordTypeTXT),
+		endpoint.NewEndpoint("hack.example.com", "hack.azurewebsites.net", endpoint.RecordTypeCNAME),
 	}
 
 	validateEndpoints(t, actual, expected)
@@ -192,23 +192,23 @@ func TestAzureApplyChanges(t *testing.T) {
 	testAzureApplyChangesInternal(t, false, &recordsClient)
 
 	validateEndpoints(t, recordsClient.deletedEndpoints, []*endpoint.Endpoint{
-		endpoint.NewEndpoint("old.example.com", "", "A"),
-		endpoint.NewEndpoint("oldcname.example.com", "", "CNAME"),
-		endpoint.NewEndpoint("deleted.example.com", "", "A"),
-		endpoint.NewEndpoint("deletedcname.example.com", "", "CNAME"),
+		endpoint.NewEndpoint("old.example.com", "", endpoint.RecordTypeA),
+		endpoint.NewEndpoint("oldcname.example.com", "", endpoint.RecordTypeCNAME),
+		endpoint.NewEndpoint("deleted.example.com", "", endpoint.RecordTypeA),
+		endpoint.NewEndpoint("deletedcname.example.com", "", endpoint.RecordTypeCNAME),
 	})
 
 	validateEndpoints(t, recordsClient.updatedEndpoints, []*endpoint.Endpoint{
-		endpoint.NewEndpoint("example.com", "1.2.3.4", "A"),
-		endpoint.NewEndpoint("example.com", "tag", "TXT"),
-		endpoint.NewEndpoint("foo.example.com", "1.2.3.4", "A"),
-		endpoint.NewEndpoint("foo.example.com", "tag", "TXT"),
-		endpoint.NewEndpoint("bar.example.com", "other.com", "CNAME"),
-		endpoint.NewEndpoint("bar.example.com", "tag", "TXT"),
-		endpoint.NewEndpoint("other.com", "5.6.7.8", "A"),
-		endpoint.NewEndpoint("other.com", "tag", "TXT"),
-		endpoint.NewEndpoint("new.example.com", "111.222.111.222", "A"),
-		endpoint.NewEndpoint("newcname.example.com", "other.com", "CNAME"),
+		endpoint.NewEndpoint("example.com", "1.2.3.4", endpoint.RecordTypeA),
+		endpoint.NewEndpoint("example.com", "tag", endpoint.RecordTypeTXT),
+		endpoint.NewEndpoint("foo.example.com", "1.2.3.4", endpoint.RecordTypeA),
+		endpoint.NewEndpoint("foo.example.com", "tag", endpoint.RecordTypeTXT),
+		endpoint.NewEndpoint("bar.example.com", "other.com", endpoint.RecordTypeCNAME),
+		endpoint.NewEndpoint("bar.example.com", "tag", endpoint.RecordTypeTXT),
+		endpoint.NewEndpoint("other.com", "5.6.7.8", endpoint.RecordTypeA),
+		endpoint.NewEndpoint("other.com", "tag", endpoint.RecordTypeTXT),
+		endpoint.NewEndpoint("new.example.com", "111.222.111.222", endpoint.RecordTypeA),
+		endpoint.NewEndpoint("newcname.example.com", "other.com", endpoint.RecordTypeCNAME),
 	})
 }
 
@@ -239,33 +239,33 @@ func testAzureApplyChangesInternal(t *testing.T, dryRun bool, client RecordsClie
 	)
 
 	createRecords := []*endpoint.Endpoint{
-		endpoint.NewEndpoint("example.com", "1.2.3.4", "A"),
-		endpoint.NewEndpoint("example.com", "tag", "TXT"),
-		endpoint.NewEndpoint("foo.example.com", "1.2.3.4", ""),
-		endpoint.NewEndpoint("foo.example.com", "tag", "TXT"),
-		endpoint.NewEndpoint("bar.example.com", "other.com", ""),
-		endpoint.NewEndpoint("bar.example.com", "tag", "TXT"),
-		endpoint.NewEndpoint("other.com", "5.6.7.8", "A"),
-		endpoint.NewEndpoint("other.com", "tag", "TXT"),
-		endpoint.NewEndpoint("nope.com", "4.4.4.4", ""),
-		endpoint.NewEndpoint("nope.com", "tag", "TXT"),
+		endpoint.NewEndpoint("example.com", "1.2.3.4", endpoint.RecordTypeA),
+		endpoint.NewEndpoint("example.com", "tag", endpoint.RecordTypeTXT),
+		endpoint.NewEndpoint("foo.example.com", "1.2.3.4", endpoint.RecordTypeA),
+		endpoint.NewEndpoint("foo.example.com", "tag", endpoint.RecordTypeTXT),
+		endpoint.NewEndpoint("bar.example.com", "other.com", endpoint.RecordTypeCNAME),
+		endpoint.NewEndpoint("bar.example.com", "tag", endpoint.RecordTypeTXT),
+		endpoint.NewEndpoint("other.com", "5.6.7.8", endpoint.RecordTypeA),
+		endpoint.NewEndpoint("other.com", "tag", endpoint.RecordTypeTXT),
+		endpoint.NewEndpoint("nope.com", "4.4.4.4", endpoint.RecordTypeA),
+		endpoint.NewEndpoint("nope.com", "tag", endpoint.RecordTypeTXT),
 	}
 
 	currentRecords := []*endpoint.Endpoint{
-		endpoint.NewEndpoint("old.example.com", "121.212.121.212", "A"),
-		endpoint.NewEndpoint("oldcname.example.com", "other.com", ""),
-		endpoint.NewEndpoint("old.nope.com", "121.212.121.212", ""),
+		endpoint.NewEndpoint("old.example.com", "121.212.121.212", endpoint.RecordTypeA),
+		endpoint.NewEndpoint("oldcname.example.com", "other.com", endpoint.RecordTypeCNAME),
+		endpoint.NewEndpoint("old.nope.com", "121.212.121.212", endpoint.RecordTypeA),
 	}
 	updatedRecords := []*endpoint.Endpoint{
-		endpoint.NewEndpoint("new.example.com", "111.222.111.222", ""),
-		endpoint.NewEndpoint("newcname.example.com", "other.com", ""),
-		endpoint.NewEndpoint("new.nope.com", "222.111.222.111", "A"),
+		endpoint.NewEndpoint("new.example.com", "111.222.111.222", endpoint.RecordTypeA),
+		endpoint.NewEndpoint("newcname.example.com", "other.com", endpoint.RecordTypeCNAME),
+		endpoint.NewEndpoint("new.nope.com", "222.111.222.111", endpoint.RecordTypeA),
 	}
 
 	deleteRecords := []*endpoint.Endpoint{
-		endpoint.NewEndpoint("deleted.example.com", "111.222.111.222", ""),
-		endpoint.NewEndpoint("deletedcname.example.com", "other.com", ""),
-		endpoint.NewEndpoint("deleted.nope.com", "222.111.222.111", "A"),
+		endpoint.NewEndpoint("deleted.example.com", "111.222.111.222", endpoint.RecordTypeA),
+		endpoint.NewEndpoint("deletedcname.example.com", "other.com", endpoint.RecordTypeCNAME),
+		endpoint.NewEndpoint("deleted.nope.com", "222.111.222.111", endpoint.RecordTypeA),
 	}
 
 	changes := &plan.Changes{

provider/cloudflare.go

@@ -21,9 +21,9 @@ import (
 	"os"
 	"strings"
 
-	log "github.com/Sirupsen/logrus"
+	cloudflare "github.com/cloudflare/cloudflare-go"
 
-	"github.com/cloudflare/cloudflare-go"
+	log "github.com/Sirupsen/logrus"
 
 	"github.com/kubernetes-incubator/external-dns/endpoint"
 	"github.com/kubernetes-incubator/external-dns/plan"
@@ -143,7 +143,7 @@ func (p *CloudFlareProvider) Records() ([]*endpoint.Endpoint, error) {
 
 		for _, r := range records {
 			switch r.Type {
-			case "A", "CNAME", "TXT":
+			case endpoint.RecordTypeA, endpoint.RecordTypeCNAME, endpoint.RecordTypeTXT:
 				break
 			default:
 				continue
@@ -277,16 +277,14 @@ func newCloudFlareChanges(action string, endpoints []*endpoint.Endpoint) []*clou
 }
 
 func newCloudFlareChange(action string, endpoint *endpoint.Endpoint) *cloudFlareChange {
-	typ := suitableType(endpoint)
-
 	return &cloudFlareChange{
 		Action: action,
 		ResourceRecordSet: cloudflare.DNSRecord{
 			Name: endpoint.DNSName,
 			// TTL Value of 1 is 'automatic'
 			TTL:     1,
 			Proxied: false,
-			Type:    typ,
+			Type:    endpoint.RecordType,
 			Content: endpoint.Target,
 		},
 	}

provider/cloudflare_test.go

@@ -39,7 +39,7 @@ func (m *mockCloudFlareClient) CreateDNSRecord(zoneID string, rr cloudflare.DNSR
 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: "A"},
+				{ID: "1234567890", Name: "foobar.ext-dns-test.zalando.to.", Type: endpoint.RecordTypeA},
 				{ID: "1231231233", Name: "foo.bar.com"}},
 			nil
 	}
@@ -425,23 +425,23 @@ func TestCloudFlareGetRecordID(t *testing.T) {
 	records := []cloudflare.DNSRecord{
 		{
 			Name: "foo.com",
-			Type: "CNAME",
+			Type: endpoint.RecordTypeCNAME,
 			ID:   "1",
 		},
 		{
 			Name: "bar.de",
-			Type: "A",
+			Type: endpoint.RecordTypeA,
 			ID:   "2",
 		},
 	}
 
 	assert.Equal(t, "", p.getRecordID(records, cloudflare.DNSRecord{
 		Name: "foo.com",
-		Type: "A",
+		Type: endpoint.RecordTypeA,
 	}))
 	assert.Equal(t, "2", p.getRecordID(records, cloudflare.DNSRecord{
 		Name: "bar.de",
-		Type: "A",
+		Type: endpoint.RecordTypeA,
 	}))
 }
 

provider/digital_ocean.go

@@ -256,7 +256,7 @@ func newDigitalOceanChange(action string, endpoint *endpoint.Endpoint) *DigitalO
 		Action: action,
 		ResourceRecordSet: godo.DomainRecord{
 			Name: endpoint.DNSName,
-			Type: suitableType(endpoint),
+			Type: endpoint.RecordType,
 			Data: endpoint.Target,
 		},
 	}

provider/digital_ocean_test.go

@@ -459,22 +459,22 @@ func TestDigitalOceanGetRecordID(t *testing.T) {
 		{
 			ID:   1,
 			Name: "foo.com",
-			Type: "CNAME",
+			Type: endpoint.RecordTypeCNAME,
 		},
 		{
 			ID:   2,
 			Name: "baz.de",
-			Type: "A",
+			Type: endpoint.RecordTypeA,
 		},
 	}
 	assert.Equal(t, 1, p.getRecordID(records, godo.DomainRecord{
 		Name: "foo.com",
-		Type: "CNAME",
+		Type: endpoint.RecordTypeCNAME,
 	}))
 
 	assert.Equal(t, 0, p.getRecordID(records, godo.DomainRecord{
 		Name: "foo.com",
-		Type: "A",
+		Type: endpoint.RecordTypeA,
 	}))
 }