Skip to content

Commit

Permalink
r/aws_route53_zone: prevent re-creation with uppercase name
Browse files Browse the repository at this point in the history
This change handles normalizing the `name` argument such that the value returned from the Route53 API matches what is stored in state. Previously, if the `name` argument included an uppercase letter a persistent difference would be present, causing the resource to be re-created.

```console
% make testacc PKG=route53 TESTS=TestAccRoute53Zone_
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.21.8 test ./internal/service/route53/... -v -count 1 -parallel 20 -run='TestAccRoute53Zone_'  -timeout 360m

--- PASS: TestAccRoute53Zone_disappears (57.95s)
--- PASS: TestAccRoute53Zone_delegationSetID (65.40s)
--- PASS: TestAccRoute53Zone_basic (66.34s)
--- PASS: TestAccRoute53Zone_VPC_single (70.98s)
--- PASS: TestAccRoute53Zone_comment (71.97s)
--- PASS: TestAccRoute53Zone_multiple (74.73s)
--- PASS: TestAccRoute53Zone_tags (76.63s)
--- PASS: TestAccRoute53Zone_VPC_multiple (143.04s)
--- PASS: TestAccRoute53Zone_VPC_single_forceDestroy (190.01s)
--- PASS: TestAccRoute53Zone_VPC_updates (212.37s)
--- PASS: TestAccRoute53Zone_forceDestroy (258.91s)
--- PASS: TestAccRoute53Zone_ForceDestroy_trailingPeriod (274.02s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/route53    279.439
```

```console
% make testacc PKG=route53 TESTS=TestAccRoute53ZoneDataSource_
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.21.8 test ./internal/service/route53/... -v -count 1 -parallel 20 -run='TestAccRoute53ZoneDataSource_'  -timeout 360m

--- PASS: TestAccRoute53ZoneDataSource_id (45.26s)
--- PASS: TestAccRoute53ZoneDataSource_name (48.35s)
--- PASS: TestAccRoute53ZoneDataSource_tags (65.21s)
--- PASS: TestAccRoute53ZoneDataSource_vpc (83.68s)
--- PASS: TestAccRoute53ZoneDataSource_serviceDiscovery (102.34s)
PASS
ok      github.com/hashicorp/terraform-provider-aws/internal/service/route53    107.890s
```
  • Loading branch information
jar-b committed Mar 25, 2024
1 parent 7845a0b commit 2f6ab11
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 11 deletions.
3 changes: 3 additions & 0 deletions .changelog/36563.txt
@@ -0,0 +1,3 @@
```release-note:bug
resource/aws_route53_zone: Prevent re-creation when `name` casing changes
```
17 changes: 10 additions & 7 deletions internal/service/route53/zone.go
Expand Up @@ -82,7 +82,7 @@ func ResourceZone() *schema.Resource {
Type: schema.TypeString,
Required: true,
ForceNew: true,
StateFunc: TrimTrailingPeriod,
StateFunc: NormalizeZoneName,
ValidateFunc: validation.StringLenBetween(1, 1024),
},
"name_servers": {
Expand Down Expand Up @@ -209,7 +209,7 @@ func resourceZoneRead(ctx context.Context, d *schema.ResourceData, meta interfac
d.Set("delegation_set_id", "")
// To be consistent with other AWS services (e.g. ACM) that do not accept a trailing period,
// we remove the suffix from the Hosted Zone Name returned from the API
d.Set("name", TrimTrailingPeriod(aws.StringValue(output.HostedZone.Name)))
d.Set("name", NormalizeZoneName(aws.StringValue(output.HostedZone.Name)))
d.Set("zone_id", CleanZoneID(aws.StringValue(output.HostedZone.Id)))

var nameServers []string
Expand Down Expand Up @@ -549,11 +549,14 @@ func CleanZoneID(ID string) string {
return strings.TrimPrefix(ID, "/hostedzone/")
}

// TrimTrailingPeriod is used to remove the trailing period
// of "name" or "domain name" attributes often returned from
// the Route53 API or provided as user input.
// NormalizeZoneName is used to remove the trailing period
// and apply consistent casing to "name" or "domain name"
// attributes returned from the Route53 API or provided as
// user input.
//
// The single dot (".") domain name is returned as-is.
func TrimTrailingPeriod(v interface{}) string {
// Uppercase letters are converted to lowercase.
func NormalizeZoneName(v interface{}) string {
var str string
switch value := v.(type) {
case *string:
Expand All @@ -568,7 +571,7 @@ func TrimTrailingPeriod(v interface{}) string {
return str
}

return strings.TrimSuffix(str, ".")
return strings.ToLower(strings.TrimSuffix(str, "."))
}

func findNameServers(ctx context.Context, conn *route53.Route53, zoneId string, zoneName string) ([]string, error) {
Expand Down
4 changes: 2 additions & 2 deletions internal/service/route53/zone_data_source.go
Expand Up @@ -123,7 +123,7 @@ func dataSourceZoneRead(ctx context.Context, d *schema.ResourceData, meta interf
hostedZoneFound = hostedZone
break
// we check if the name is the same as requested and if private zone field is the same as requested or if there is a vpc_id
} else if (TrimTrailingPeriod(aws.StringValue(hostedZone.Name)) == TrimTrailingPeriod(name)) && (aws.BoolValue(hostedZone.Config.PrivateZone) == d.Get("private_zone").(bool) || (aws.BoolValue(hostedZone.Config.PrivateZone) && vpcIdExists)) {
} else if (NormalizeZoneName(aws.StringValue(hostedZone.Name)) == NormalizeZoneName(name)) && (aws.BoolValue(hostedZone.Config.PrivateZone) == d.Get("private_zone").(bool) || (aws.BoolValue(hostedZone.Config.PrivateZone) && vpcIdExists)) {
matchingVPC := false
if vpcIdExists {
reqHostedZone := &route53.GetHostedZoneInput{}
Expand Down Expand Up @@ -178,7 +178,7 @@ func dataSourceZoneRead(ctx context.Context, d *schema.ResourceData, meta interf
d.Set("zone_id", idHostedZone)
// To be consistent with other AWS services (e.g. ACM) that do not accept a trailing period,
// we remove the suffix from the Hosted Zone Name returned from the API
d.Set("name", TrimTrailingPeriod(aws.StringValue(hostedZoneFound.Name)))
d.Set("name", NormalizeZoneName(aws.StringValue(hostedZoneFound.Name)))
d.Set("comment", hostedZoneFound.Config.Comment)
d.Set("private_zone", hostedZoneFound.Config.PrivateZone)
d.Set("caller_reference", hostedZoneFound.CallerReference)
Expand Down
9 changes: 7 additions & 2 deletions internal/service/route53/zone_test.go
Expand Up @@ -60,7 +60,7 @@ func TestCleanChangeID(t *testing.T) {
}
}

func TestTrimTrailingPeriod(t *testing.T) {
func TestNormalizeZoneName(t *testing.T) {
t.Parallel()

cases := []struct {
Expand All @@ -70,17 +70,22 @@ func TestTrimTrailingPeriod(t *testing.T) {
{"example.com", "example.com"},
{"example.com.", "example.com"},
{"www.example.com.", "www.example.com"},
{"www.ExAmPlE.COM.", "www.example.com"},
{"", ""},
{".", "."},
{aws.String("example.com"), "example.com"},
{aws.String("example.com."), "example.com"},
{aws.String("www.example.com."), "www.example.com"},
{aws.String("www.ExAmPlE.COM."), "www.example.com"},
{aws.String(""), ""},
{aws.String("."), "."},
{(*string)(nil), ""},
{42, ""},
{nil, ""},
}

for _, tc := range cases {
actual := tfroute53.TrimTrailingPeriod(tc.Input)
actual := tfroute53.NormalizeZoneName(tc.Input)
if actual != tc.Output {
t.Fatalf("input: %s\noutput: %s", tc.Input, actual)
}
Expand Down

0 comments on commit 2f6ab11

Please sign in to comment.