Skip to content
This repository has been archived by the owner on Sep 16, 2019. It is now read-only.

support plain A record if ingress is using IP #86

Merged
merged 5 commits into from
Feb 23, 2017
Merged

support plain A record if ingress is using IP #86

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a7021c0
support plain A record if ingress is using IP
Feb 23, 2017
c09e1df
fix and add new tests
Feb 23, 2017
8a37d7e
bug fix target retrieving
Feb 23, 2017
c5abaeb
address PR review, improve looping syntax, check for hostname not empty
Feb 23, 2017
5b123ae
add tests for IP records
Feb 23, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
53 changes: 33 additions & 20 deletions consumers/aws.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ type awsConsumer struct {
const (
evaluateTargetHealth = true
defaultTxtTTL = int64(300)
defaultATTL = int64(300)
)

// NewAWSRoute53Consumer reates a Consumer instance to sync and process DNS
Expand All @@ -50,7 +51,7 @@ func withClient(c AWSClient, groupID string) *awsConsumer {
}

func (a *awsConsumer) Sync(endpoints []*pkg.Endpoint) error {
kubeRecords, err := a.endpointsToAlias(endpoints)
kubeRecords, err := a.endpointsToRecords(endpoints)
if err != nil {
return err
}
Expand All @@ -68,7 +69,7 @@ func (a *awsConsumer) Sync(endpoints []*pkg.Endpoint) error {
for _, record := range kubeRecords {
zoneID := getZoneIDForEndpoint(hostedZonesMap, record) //this guarantees that the endpoint will not be created in multiple hosted zones
if zoneID == "" {
log.Warnf("Hosted zone for endpoint: %s is not found. Skipping record...", aws.StringValue(record.Name))
log.Warnf("Hosted zone for endpoint: %s was not found. Skipping record...", aws.StringValue(record.Name))
continue
}
inputByZoneID[zoneID] = append(inputByZoneID[zoneID], record)
Expand Down Expand Up @@ -103,11 +104,11 @@ func (a *awsConsumer) syncPerHostedZone(kubeRecords []*route53.ResourceRecordSet
upsertedMap := make(map[string]bool) // keep track of records to be upserted
targetMap := map[string][]*string{} // map dnsname -> list of targets
for _, kr := range kubeRecords {
targetMap[aws.StringValue(kr.Name)] = append(targetMap[aws.StringValue(kr.Name)], kr.AliasTarget.DNSName)
targetMap[aws.StringValue(kr.Name)] = append(targetMap[aws.StringValue(kr.Name)], aws.String(a.getRecordTarget(kr)))
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you use aws.StringValue(kr.Name) instead of *kr.Name

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

safeguard if kr.Name is nil

}
//find records to be upserted
for _, kubeRecord := range kubeRecords {
//make sure that another record with same DNS name is not already included into upsert slice
//make sure that another record with same DNS name was not already included into upsert slice
if _, upserted := upsertedMap[aws.StringValue(kubeRecord.Name)]; upserted {
continue
}
Expand Down Expand Up @@ -202,19 +203,19 @@ func (a *awsConsumer) Process(endpoint *pkg.Endpoint) error {
return err
}

aliasRecords, err := a.endpointsToAlias([]*pkg.Endpoint{endpoint})
ARecords, err := a.endpointsToRecords([]*pkg.Endpoint{endpoint})
if err != nil {
return err
}
if len(aliasRecords) != 1 {
return fmt.Errorf("Failed to process endpoint. Alias could not be constructed for: %s:%s.", endpoint.DNSName, endpoint.Hostname)
if len(ARecords) != 1 {
return fmt.Errorf("failed to process endpoint. A record could not be constructed for: %s:%s:%s", endpoint.DNSName, endpoint.Hostname, endpoint.IP)
}

create := []*route53.ResourceRecordSet{aliasRecords[0], a.getAssignedTXTRecordObject(aliasRecords[0])}
create := []*route53.ResourceRecordSet{ARecords[0], a.getAssignedTXTRecordObject(ARecords[0])}

zoneID := getZoneIDForEndpoint(hostedZonesMap, aliasRecords[0])
zoneID := getZoneIDForEndpoint(hostedZonesMap, ARecords[0])
if zoneID == "" {
log.Warnf("Hosted zone for endpoint: %s is not found. Skipping record...", endpoint.DNSName)
log.Warnf("Hosted zone for endpoint: %s was not found. Skipping record...", endpoint.DNSName)
return nil
}

Expand Down Expand Up @@ -300,11 +301,11 @@ func (a *awsConsumer) getRecordTarget(r *route53.ResourceRecordSet) string {
return aws.StringValue(r.ResourceRecords[0].Value)
}

//endpointsToAlias converts pkg Endpoint to route53 Alias Records
func (a *awsConsumer) endpointsToAlias(endpoints []*pkg.Endpoint) ([]*route53.ResourceRecordSet, error) {
lbDNS := make([]string, len(endpoints))
//endpointsToRecords converts pkg Endpoint to route53 A [Alias] Records depending whether IP/LB Hostname is used
func (a *awsConsumer) endpointsToRecords(endpoints []*pkg.Endpoint) ([]*route53.ResourceRecordSet, error) {
lbDNS := make([]string, 0)
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if you don't filter you can set the capacity here.

for i := range endpoints {
lbDNS[i] = endpoints[i].Hostname
lbDNS = append(lbDNS, endpoints[i].Hostname)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you use the index (i) an not the value?

for _, endpoint := range endpoints {
    lbDNS = append(lbDNS, endpoint.Hostname)
}

Copy link
Contributor Author

@ideahitme ideahitme Feb 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right, left over from previous loop impl where index was a better option :)

}
zoneIDs, err := a.client.GetCanonicalZoneIDs(lbDNS)
if err != nil {
Expand All @@ -314,24 +315,36 @@ func (a *awsConsumer) endpointsToAlias(endpoints []*pkg.Endpoint) ([]*route53.Re

for _, ep := range endpoints {
if loadBalancerZoneID, exist := zoneIDs[ep.Hostname]; exist {
rset = append(rset, a.endpointToAlias(ep, aws.String(loadBalancerZoneID)))
rset = append(rset, a.endpointToRecord(ep, aws.String(loadBalancerZoneID)))
} else if ep.IP != "" {
rset = append(rset, a.endpointToRecord(ep, aws.String("")))
} else {
log.Errorf("Canonical Zone ID for endpoint: %s is not found", ep.Hostname)
log.Errorf("Canonical Zone ID for endpoint: %s was not found", ep.Hostname)
}
}
return rset, nil
}

//endpointToAlias convert endpoint to an AWS A Alias record
func (a *awsConsumer) endpointToAlias(ep *pkg.Endpoint, canonicalZoneID *string) *route53.ResourceRecordSet {
//endpointToRecord convert endpoint to an AWS A [Alias] record depending whether IP of LB hostname is used
//if both are specified hostname takes precedence and Alias record is to be created
func (a *awsConsumer) endpointToRecord(ep *pkg.Endpoint, canonicalZoneID *string) *route53.ResourceRecordSet {
rs := &route53.ResourceRecordSet{
Type: aws.String("A"),
Name: aws.String(pkg.SanitizeDNSName(ep.DNSName)),
AliasTarget: &route53.AliasTarget{
}
if ep.Hostname != "" {
rs.AliasTarget = &route53.AliasTarget{
DNSName: aws.String(pkg.SanitizeDNSName(ep.Hostname)),
EvaluateTargetHealth: aws.Bool(evaluateTargetHealth),
HostedZoneId: canonicalZoneID,
},
}
} else {
rs.TTL = aws.Int64(defaultATTL)
rs.ResourceRecords = []*route53.ResourceRecord{
&route53.ResourceRecord{
Value: aws.String(ep.IP),
},
}
}
return rs
}
27 changes: 25 additions & 2 deletions consumers/aws_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,17 +27,40 @@ func TestEndpointToAlias(t *testing.T) {
client := &awsConsumer{
groupID: groupID,
}
//both Hostname and IP specified -> Alias Record
ep := &pkg.Endpoint{
DNSName: "example.com",
IP: "10.202.10.123",
Hostname: "amazon.elb.com",
}
rsA := client.endpointToAlias(ep, &zoneID)
rsA := client.endpointToRecord(ep, &zoneID)
Copy link
Collaborator

@szuecs szuecs Feb 23, 2017 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then the TestName should be fixed, too :)

if *rsA.Type != "A" || *rsA.Name != pkg.SanitizeDNSName(ep.DNSName) ||
*rsA.AliasTarget.DNSName != pkg.SanitizeDNSName(ep.Hostname) ||
*rsA.AliasTarget.HostedZoneId != zoneID {
t.Error("Should create an Alias A record")
}
// only IP specified -> plain A Record
ep = &pkg.Endpoint{
DNSName: "example.com",
IP: "10.202.10.123",
}
rsA = client.endpointToRecord(ep, &zoneID)
if *rsA.Type != "A" || *rsA.Name != pkg.SanitizeDNSName(ep.DNSName) ||
len(rsA.ResourceRecords) != 1 || *rsA.ResourceRecords[0].Value != ep.IP {
t.Error("Should create an A record")
}
//only Hostname specified -> Alias Record
ep = &pkg.Endpoint{
DNSName: "example.com",
Hostname: "amazon.elb.com",
}
rsA = client.endpointToRecord(ep, &zoneID)
if *rsA.Type != "A" || *rsA.Name != pkg.SanitizeDNSName(ep.DNSName) ||
*rsA.AliasTarget.DNSName != pkg.SanitizeDNSName(ep.Hostname) ||
*rsA.AliasTarget.HostedZoneId != zoneID {
t.Error("Should create an Alias A record")
}

}

func TestGetAssignedTXTRecordObject(t *testing.T) {
Expand All @@ -51,7 +74,7 @@ func TestGetAssignedTXTRecordObject(t *testing.T) {
IP: "10.202.10.123",
Hostname: "amazon.elb.com",
}
rsA := client.endpointToAlias(ep, &zoneID)
rsA := client.endpointToRecord(ep, &zoneID)
rsTXT := client.getAssignedTXTRecordObject(rsA)
if *rsTXT.Type != "TXT" ||
*rsTXT.Name != "example.com." ||
Expand Down