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

provider/aws: Fix aws_route53_record alias perpetual diff #9704

Merged
merged 1 commit into from
Oct 31, 2016
Merged
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
18 changes: 14 additions & 4 deletions builtin/providers/aws/resource_aws_route53_record.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,9 @@ func resourceAwsRoute53Record() *schema.Resource {
},

"name": &schema.Schema{
Type: schema.TypeString,
Required: true,
Type: schema.TypeString,
Required: true,
StateFunc: normalizeAwsAliasName,
},

"evaluate_target_health": &schema.Schema{
Expand Down Expand Up @@ -374,7 +375,7 @@ func resourceAwsRoute53RecordRead(d *schema.ResourceData, meta interface{}) erro
}

if alias := record.AliasTarget; alias != nil {
name := strings.TrimSuffix(*alias.DNSName, ".")
name := normalizeAwsAliasName(*alias.DNSName)
d.Set("alias", []interface{}{
map[string]interface{}{
"zone_id": *alias.HostedZoneId,
Expand Down Expand Up @@ -732,7 +733,7 @@ func expandRecordName(name, zone string) string {
func resourceAwsRoute53AliasRecordHash(v interface{}) int {
var buf bytes.Buffer
m := v.(map[string]interface{})
buf.WriteString(fmt.Sprintf("%s-", m["name"].(string)))
buf.WriteString(fmt.Sprintf("%s-", normalizeAwsAliasName(m["name"].(string))))
buf.WriteString(fmt.Sprintf("%s-", m["zone_id"].(string)))
Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry for the late comment here @stack72... but when I've seen this class of error so far I've seen zone_id change too, as if Route53 uses a different zone for the dualstack. form of the hostname than for the regular form. You can see this e.g. in the example in the summary of #9628.

Do you think we need to do something additional here to catch that case, or is this fine because the non-prefixed hostname happens to resolve in the other zone too?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TBH I haven't come across a zone change yet - that is a legit change IMO - I have only come across the issue where f we chose / edit an alias in the console, it adds dualstack. so we are guarding here against that.

I have no doubt that this normalize func will expand in the future though :)

buf.WriteString(fmt.Sprintf("%t-", m["evaluate_target_health"].(bool)))

Expand All @@ -748,3 +749,12 @@ func nilString(s string) *string {
}
return aws.String(s)
}

func normalizeAwsAliasName(alias interface{}) string {
Copy link
Contributor

Choose a reason for hiding this comment

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

will alias here ever not be a string? All the invocations thus far are a string that I can see

input := alias.(string)
if strings.HasPrefix(input, "dualstack.") {
return strings.Replace(input, "dualstack.", "", -1)
}

return strings.TrimRight(input, ".")
}