Skip to content

Commit

Permalink
Kops Compatibility #1: Set RecordType at source (#248)
Browse files Browse the repository at this point in the history
* set RecordType at source

* add comments for linting
  • Loading branch information
sethpollack authored and linki committed Aug 25, 2017
1 parent a7a1cbc commit f14a953
Show file tree
Hide file tree
Showing 24 changed files with 467 additions and 468 deletions.
6 changes: 6 additions & 0 deletions endpoint/endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
8 changes: 4 additions & 4 deletions internal/testutils/endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
14 changes: 7 additions & 7 deletions plan/plan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
}
8 changes: 4 additions & 4 deletions provider/aws.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}
}

Expand Down Expand Up @@ -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{
{
Expand Down Expand Up @@ -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.
Expand Down
197 changes: 97 additions & 100 deletions provider/aws_test.go

Large diffs are not rendered by default.

5 changes: 2 additions & 3 deletions provider/azure.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}

Expand Down
92 changes: 46 additions & 46 deletions provider/azure_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"),
},
}

Expand All @@ -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)
Expand All @@ -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),
})
}

Expand Down Expand Up @@ -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{
Expand Down
10 changes: 4 additions & 6 deletions provider/cloudflare.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
},
}
Expand Down
10 changes: 5 additions & 5 deletions provider/cloudflare_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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,
}))
}

Expand Down
2 changes: 1 addition & 1 deletion provider/digital_ocean.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
}
Expand Down
8 changes: 4 additions & 4 deletions provider/digital_ocean_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}))
}

Expand Down
Loading

0 comments on commit f14a953

Please sign in to comment.