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

resource/route53_zone and data-source/route53_zone: remove trailing period from name attributes #14220

Merged
merged 6 commits into from
Jul 30, 2020
Merged
Show file tree
Hide file tree
Changes from 5 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
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)))
anGie44 marked this conversation as resolved.
Show resolved Hide resolved
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 {
anGie44 marked this conversation as resolved.
Show resolved Hide resolved
// "." 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