Skip to content

Commit

Permalink
Merge pull request #14220 from terraform-providers/b-route53-zone-domain
Browse files Browse the repository at this point in the history
resource/route53_zone and data-source/route53_zone: remove trailing period from name attributes
  • Loading branch information
anGie44 authored Jul 30, 2020
2 parents 79fe48e + 45cc659 commit c1592e4
Show file tree
Hide file tree
Showing 17 changed files with 244 additions and 208 deletions.
4 changes: 3 additions & 1 deletion aws/data_source_aws_route53_resolver_rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,9 @@ func dataSourceAwsRoute53ResolverRuleRead(d *schema.ResourceData, meta interface
d.SetId(aws.StringValue(rule.Id))
arn := *rule.Arn
d.Set("arn", arn)
d.Set("domain_name", rule.DomainName)
// To be consistent with other AWS services that do not accept a trailing period,
// we remove the suffix from the Domain Name returned from the API
d.Set("domain_name", trimTrailingPeriod(aws.StringValue(rule.DomainName)))
d.Set("name", rule.Name)
d.Set("owner_id", rule.OwnerId)
d.Set("resolver_endpoint_id", rule.ResolverEndpointId)
Expand Down
32 changes: 13 additions & 19 deletions aws/data_source_aws_route53_zone.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package aws
import (
"fmt"
"log"
"strings"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/route53"
Expand Down Expand Up @@ -72,7 +71,7 @@ func dataSourceAwsRoute53ZoneRead(d *schema.ResourceData, meta interface{}) erro
ignoreTagsConfig := meta.(*AWSClient).IgnoreTagsConfig

name, nameExists := d.GetOk("name")
name = hostedZoneName(name.(string))
name = name.(string)
id, idExists := d.GetOk("zone_id")
vpcId, vpcIdExists := d.GetOk("vpc_id")
tags := keyvaluetags.New(d.Get("tags").(map[string]interface{})).IgnoreAws()
Expand Down Expand Up @@ -101,12 +100,12 @@ func dataSourceAwsRoute53ZoneRead(d *schema.ResourceData, meta interface{}) erro
return fmt.Errorf("Error finding Route 53 Hosted Zone: %v", err)
}
for _, hostedZone := range resp.HostedZones {
hostedZoneId := cleanZoneID(*hostedZone.Id)
hostedZoneId := cleanZoneID(aws.StringValue(hostedZone.Id))
if idExists && hostedZoneId == id.(string) {
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 *hostedZone.Name == name && (*hostedZone.Config.PrivateZone == d.Get("private_zone").(bool) || (*hostedZone.Config.PrivateZone && vpcIdExists)) {
} 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)) {
matchingVPC := false
if vpcIdExists {
reqHostedZone := &route53.GetHostedZoneInput{}
Expand All @@ -118,7 +117,7 @@ func dataSourceAwsRoute53ZoneRead(d *schema.ResourceData, meta interface{}) erro
}
// we go through all VPCs
for _, vpc := range respHostedZone.VPCs {
if *vpc.VPCId == vpcId.(string) {
if aws.StringValue(vpc.VPCId) == vpcId.(string) {
matchingVPC = true
break
}
Expand All @@ -132,7 +131,7 @@ func dataSourceAwsRoute53ZoneRead(d *schema.ResourceData, meta interface{}) erro
listTags, err := keyvaluetags.Route53ListTags(conn, hostedZoneId, route53.TagResourceTypeHostedzone)

if err != nil {
return fmt.Errorf("Error finding Route 53 Hosted Zone: %v", err)
return fmt.Errorf("Error finding Route 53 Hosted Zone: %w", err)
}
matchingTags = listTags.ContainsAll(tags)
}
Expand All @@ -156,10 +155,12 @@ func dataSourceAwsRoute53ZoneRead(d *schema.ResourceData, meta interface{}) erro
return fmt.Errorf("no matching Route53Zone found")
}

idHostedZone := cleanZoneID(*hostedZoneFound.Id)
idHostedZone := cleanZoneID(aws.StringValue(hostedZoneFound.Id))
d.SetId(idHostedZone)
d.Set("zone_id", idHostedZone)
d.Set("name", hostedZoneFound.Name)
// 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("comment", hostedZoneFound.Config.Comment)
d.Set("private_zone", hostedZoneFound.Config.PrivateZone)
d.Set("caller_reference", hostedZoneFound.CallerReference)
Expand All @@ -173,7 +174,9 @@ func dataSourceAwsRoute53ZoneRead(d *schema.ResourceData, meta interface{}) erro
if err != nil {
return fmt.Errorf("Error finding Route 53 Hosted Zone: %v", err)
}
d.Set("name_servers", nameServers)
if err := d.Set("name_servers", nameServers); err != nil {
return fmt.Errorf("error setting name_servers: %w", err)
}

tags, err = keyvaluetags.Route53ListTags(conn, idHostedZone, route53.TagResourceTypeHostedzone)

Expand All @@ -188,15 +191,6 @@ func dataSourceAwsRoute53ZoneRead(d *schema.ResourceData, meta interface{}) erro
return nil
}

// used to manage trailing .
func hostedZoneName(name string) string {
if strings.HasSuffix(name, ".") {
return name
}

return name + "."
}

// used to retrieve name servers
func hostedZoneNameServers(id string, conn *route53.Route53) ([]string, error) {
req := &route53.GetHostedZoneInput{}
Expand All @@ -214,7 +208,7 @@ func hostedZoneNameServers(id string, conn *route53.Route53) ([]string, error) {
servers := []string{}
for _, server := range resp.DelegationSet.NameServers {
if server != nil {
servers = append(servers, *server)
servers = append(servers, aws.StringValue(server))
}
}
return servers, nil
Expand Down
2 changes: 1 addition & 1 deletion aws/data_source_aws_route53_zone_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,7 @@ resource "aws_vpc" "test" {
}
resource "aws_service_discovery_private_dns_namespace" "test" {
name = "test.acc-sd-%[1]d."
name = "test.acc-sd-%[1]d"
vpc = "${aws_vpc.test.id}"
}
Expand Down
8 changes: 0 additions & 8 deletions aws/diff_suppress_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,14 +115,6 @@ func suppressCloudFormationTemplateBodyDiffs(k, old, new string, d *schema.Resou
return normalizedOld == normalizedNew
}

func suppressRoute53ZoneNameWithTrailingDot(k, old, new string, d *schema.ResourceData) bool {
// "." is different from "".
if old == "." || new == "." {
return old == new
}
return strings.TrimSuffix(old, ".") == strings.TrimSuffix(new, ".")
}

// suppressEqualCIDRBlockDiffs provides custom difference suppression for CIDR blocks
// that have different string values but represent the same CIDR.
func suppressEqualCIDRBlockDiffs(k, old, new string, d *schema.ResourceData) bool {
Expand Down
56 changes: 0 additions & 56 deletions aws/diff_suppress_funcs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -264,59 +264,3 @@ Outputs:
}
}
}

func TestSuppressRoute53ZoneNameWithTrailingDot(t *testing.T) {
testCases := []struct {
old string
new string
equivalent bool
}{
{
old: "example.com",
new: "example.com",
equivalent: true,
},
{
old: "example.com.",
new: "example.com.",
equivalent: true,
},
{
old: "example.com.",
new: "example.com",
equivalent: true,
},
{
old: "example.com",
new: "example.com.",
equivalent: true,
},
{
old: ".",
new: "",
equivalent: false,
},
{
old: "",
new: ".",
equivalent: false,
},
{
old: ".",
new: ".",
equivalent: true,
},
}

for i, tc := range testCases {
value := suppressRoute53ZoneNameWithTrailingDot("test_property", tc.old, tc.new, nil)

if tc.equivalent && !value {
t.Fatalf("expected test case %d to be equivalent", i)
}

if !tc.equivalent && value {
t.Fatalf("expected test case %d to not be equivalent", i)
}
}
}
43 changes: 24 additions & 19 deletions aws/resource_aws_acm_certificate.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import (
"errors"
"fmt"
"log"
"regexp"
"strings"
"time"

Expand Down Expand Up @@ -57,29 +58,27 @@ func resourceAwsAcmCertificate() *schema.Resource {
ForceNew: true,
},
"domain_name": {
// AWS Provider 3.0.0 aws_route53_zone references no longer contain a
// trailing period, no longer requiring a custom StateFunc
// to prevent ACM API error
Type: schema.TypeString,
Optional: true,
Computed: true,
ForceNew: true,
ConflictsWith: []string{"private_key", "certificate_body", "certificate_chain"},
StateFunc: func(v interface{}) string {
// AWS Provider 1.42.0+ aws_route53_zone references may contain a
// trailing period, which generates an ACM API error
return strings.TrimSuffix(v.(string), ".")
},
ValidateFunc: validation.StringDoesNotMatch(regexp.MustCompile(`\.$`), "cannot end with a period"),
},
"subject_alternative_names": {
Type: schema.TypeSet,
Optional: true,
Computed: true,
ForceNew: true,
Elem: &schema.Schema{
Type: schema.TypeString,
StateFunc: func(v interface{}) string {
// AWS Provider 1.42.0+ aws_route53_zone references may contain a
// trailing period, which generates an ACM API error
return strings.TrimSuffix(v.(string), ".")
},
// AWS Provider 3.0.0 aws_route53_zone references no longer contain a
// trailing period, no longer requiring a custom StateFunc
// to prevent ACM API error
Type: schema.TypeString,
ValidateFunc: validation.StringDoesNotMatch(regexp.MustCompile(`\.$`), "cannot end with a period"),
},
Set: schema.HashString,
ConflictsWith: []string{"private_key", "certificate_body", "certificate_chain"},
Expand Down Expand Up @@ -164,7 +163,10 @@ func resourceAwsAcmCertificate() *schema.Resource {
// Attempt to calculate the domain validation options based on domains present in domain_name and subject_alternative_names
if diff.Get("validation_method").(string) == "DNS" && (diff.HasChange("domain_name") || diff.HasChange("subject_alternative_names")) {
domainValidationOptionsList := []interface{}{map[string]interface{}{
"domain_name": strings.TrimSuffix(diff.Get("domain_name").(string), "."),
// AWS Provider 3.0 -- plan-time validation prevents "domain_name"
// argument to accept a string with trailing period; thus, trim of trailing period
// no longer required here
"domain_name": diff.Get("domain_name").(string),
}}

if sanSet, ok := diff.Get("subject_alternative_names").(*schema.Set); ok {
Expand All @@ -176,7 +178,10 @@ func resourceAwsAcmCertificate() *schema.Resource {
}

m := map[string]interface{}{
"domain_name": strings.TrimSuffix(san, "."),
// AWS Provider 3.0 -- plan-time validation prevents "subject_alternative_names"
// argument to accept strings with trailing period; thus, trim of trailing period
// no longer required here
"domain_name": san,
}

domainValidationOptionsList = append(domainValidationOptionsList, m)
Expand Down Expand Up @@ -227,7 +232,7 @@ func resourceAwsAcmCertificateCreateImported(d *schema.ResourceData, meta interf
func resourceAwsAcmCertificateCreateRequested(d *schema.ResourceData, meta interface{}) error {
acmconn := meta.(*AWSClient).acmconn
params := &acm.RequestCertificateInput{
DomainName: aws.String(strings.TrimSuffix(d.Get("domain_name").(string), ".")),
DomainName: aws.String(d.Get("domain_name").(string)),
IdempotencyToken: aws.String(resource.PrefixedUniqueId("tf")), // 32 character limit
Options: expandAcmCertificateOptions(d.Get("options").([]interface{})),
}
Expand All @@ -243,7 +248,7 @@ func resourceAwsAcmCertificateCreateRequested(d *schema.ResourceData, meta inter
if sans, ok := d.GetOk("subject_alternative_names"); ok {
subjectAlternativeNames := make([]*string, len(sans.(*schema.Set).List()))
for i, sanRaw := range sans.(*schema.Set).List() {
subjectAlternativeNames[i] = aws.String(strings.TrimSuffix(sanRaw.(string), "."))
subjectAlternativeNames[i] = aws.String(sanRaw.(string))
}
params.SubjectAlternativeNames = subjectAlternativeNames
}
Expand Down Expand Up @@ -389,10 +394,10 @@ func convertValidationOptions(certificate *acm.CertificateDetail) ([]map[string]
for _, o := range certificate.DomainValidationOptions {
if o.ResourceRecord != nil {
validationOption := map[string]interface{}{
"domain_name": *o.DomainName,
"resource_record_name": *o.ResourceRecord.Name,
"resource_record_type": *o.ResourceRecord.Type,
"resource_record_value": *o.ResourceRecord.Value,
"domain_name": aws.StringValue(o.DomainName),
"resource_record_name": aws.StringValue(o.ResourceRecord.Name),
"resource_record_type": aws.StringValue(o.ResourceRecord.Type),
"resource_record_value": aws.StringValue(o.ResourceRecord.Value),
}
domainValidationResult = append(domainValidationResult, validationOption)
} else if o.ValidationEmails != nil && len(o.ValidationEmails) > 0 {
Expand Down
24 changes: 4 additions & 20 deletions aws/resource_aws_acm_certificate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -249,36 +249,20 @@ func TestAccAWSAcmCertificate_privateCert(t *testing.T) {
})
}

// TestAccAWSAcmCertificate_root_TrailingPeriod updated in 3.0 to account for domain_name plan-time validation
// Reference: https:/terraform-providers/terraform-provider-aws/issues/13510
func TestAccAWSAcmCertificate_root_TrailingPeriod(t *testing.T) {
rootDomain := testAccAwsAcmCertificateDomainFromEnv(t)
domain := fmt.Sprintf("%s.", rootDomain)
resourceName := "aws_acm_certificate.cert"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAcmCertificateDestroy,
Steps: []resource.TestStep{
{
Config: testAccAcmCertificateConfig(domain, acm.ValidationMethodDns),
Check: resource.ComposeTestCheckFunc(
testAccMatchResourceAttrRegionalARN(resourceName, "arn", "acm", regexp.MustCompile(`certificate/.+`)),
resource.TestCheckResourceAttr(resourceName, "domain_name", strings.TrimSuffix(domain, ".")),
resource.TestCheckResourceAttr(resourceName, "domain_validation_options.#", "1"),
tfawsresource.TestCheckTypeSetElemNestedAttrs(resourceName, "domain_validation_options.*", map[string]string{
"domain_name": strings.TrimSuffix(domain, "."),
"resource_record_type": "CNAME",
}),
resource.TestCheckResourceAttr(resourceName, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_method", acm.ValidationMethodDns),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
Config: testAccAcmCertificateConfig(domain, acm.ValidationMethodDns),
ExpectError: regexp.MustCompile(`config is invalid: invalid value for domain_name \(cannot end with a period\)`),
},
},
})
Expand Down
Loading

0 comments on commit c1592e4

Please sign in to comment.