Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make sure route53 records really exist after creation and add support for all ASCII characters #4183

Merged
merged 4 commits into from
Apr 12, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
62 changes: 44 additions & 18 deletions aws/resource_aws_route53_record.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"fmt"
"log"
"regexp"
"strconv"
"strings"
"time"

Expand All @@ -20,8 +21,8 @@ import (
"github.com/aws/aws-sdk-go/service/route53"
)

var r53NoRecordsFound = errors.New("No matching Hosted Zone found")
var r53NoHostedZoneFound = errors.New("No matching records found")
var r53NoRecordsFound = errors.New("No matching records found")
var r53NoHostedZoneFound = errors.New("No matching Hosted Zone found")
var r53ValidRecordTypes = regexp.MustCompile("^(A|AAAA|CAA|CNAME|MX|NAPTR|NS|PTR|SOA|SPF|SRV|TXT)$")

func resourceAwsRoute53Record() *schema.Resource {
Expand Down Expand Up @@ -374,7 +375,8 @@ func resourceAwsRoute53RecordUpdate(d *schema.ResourceData, meta interface{}) er
return err
}

return resourceAwsRoute53RecordRead(d, meta)
_, err = findRecord(d, meta)
return err
}

func resourceAwsRoute53RecordCreate(d *schema.ResourceData, meta interface{}) error {
Expand Down Expand Up @@ -451,7 +453,8 @@ func resourceAwsRoute53RecordCreate(d *schema.ResourceData, meta interface{}) er
return err
}

return resourceAwsRoute53RecordRead(d, meta)
_, err = findRecord(d, meta)
return err
}

func changeRoute53RecordSet(conn *route53.Route53, input *route53.ChangeResourceRecordSetsInput) (interface{}, error) {
Expand Down Expand Up @@ -630,27 +633,49 @@ func findRecord(d *schema.ResourceData, meta interface{}) (*route53.ResourceReco
log.Printf("[DEBUG] Expanded record name: %s", en)
d.Set("fqdn", en)

recordName := FQDN(strings.ToLower(en))
recordType := d.Get("type").(string)
recordSetIdentifier := d.Get("set_identifier")

// If this isn't a Weighted, Latency, Geo, or Failover resource with
// a SetIdentifier we only need to look at the first record in the response since there can be
// only one
maxItems := "1"
if recordSetIdentifier != "" {
maxItems = "100"
}

lopts := &route53.ListResourceRecordSetsInput{
HostedZoneId: aws.String(cleanZoneID(zone)),
StartRecordName: aws.String(en),
StartRecordType: aws.String(d.Get("type").(string)),
StartRecordName: aws.String(recordName),
StartRecordType: aws.String(recordType),
MaxItems: aws.String(maxItems),
}

log.Printf("[DEBUG] List resource records sets for zone: %s, opts: %s",
zone, lopts)

var record *route53.ResourceRecordSet

// We need to loop over all records starting from the record we are looking for because
Copy link
Contributor

Choose a reason for hiding this comment

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

❤️ thank you!

// Weighted, Latency, Geo, and Failover resource record sets have a special option
// called SetIdentifier which allows multiple entries with the same name and type but
// a different SetIdentifier
// For all other records we are setting the maxItems to 1 so that we don't return extra
// unneeded records
err = conn.ListResourceRecordSetsPages(lopts, func(resp *route53.ListResourceRecordSetsOutput, lastPage bool) bool {
for _, recordSet := range resp.ResourceRecordSets {
name := cleanRecordName(*recordSet.Name)
if FQDN(strings.ToLower(name)) != FQDN(strings.ToLower(*lopts.StartRecordName)) {

responseName := strings.ToLower(cleanRecordName(*recordSet.Name))
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor nitpick: To prevent crashes in the very unlikely scenario that AWS returns nil for either of these, it would be safer to wrap them in aws.StringValue() 👍 e.g. aws.StringValue(recordSet.Name)

responseType := strings.ToUpper(*recordSet.Type)

if recordName != responseName {
continue
}
if strings.ToUpper(*recordSet.Type) != strings.ToUpper(*lopts.StartRecordType) {
if recordType != responseType {
continue
}

if recordSet.SetIdentifier != nil && *recordSet.SetIdentifier != d.Get("set_identifier") {
if recordSet.SetIdentifier != nil && *recordSet.SetIdentifier != recordSetIdentifier {
continue
}

Expand Down Expand Up @@ -883,16 +908,17 @@ func FQDN(name string) string {
}
}

// Route 53 stores the "*" wildcard indicator as ASCII 42 and returns the
// octal equivalent, "\\052". Here we look for that, and convert back to "*"
// as needed.
// Route 53 stores certain characters with the octal equivalent in ASCII format.
// This function converts all of these characters back into the original character
// E.g. "*" is stored as "\\052" and "@" as "\\100"

func cleanRecordName(name string) string {
str := name
if strings.HasPrefix(name, "\\052") {
str = strings.Replace(name, "\\052", "*", 1)
log.Printf("[DEBUG] Replacing octal \\052 for * in: %s", name)
s, err := strconv.Unquote(`"` + str + `"`)
if err != nil {
return str
}
return str
return s
}

// Check if the current record name contains the zone suffix.
Expand Down
2 changes: 2 additions & 0 deletions aws/resource_aws_route53_record_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ func TestCleanRecordName(t *testing.T) {
}{
{"www.nonexample.com", "www.nonexample.com"},
{"\\052.nonexample.com", "*.nonexample.com"},
{"\\100.nonexample.com", "@.nonexample.com"},
{"\\043.nonexample.com", "#.nonexample.com"},
{"nonexample.com", "nonexample.com"},
}

Expand Down