From 83414cfac27ec9503000c72626006a1bd04cd28d Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Thu, 13 Dec 2018 09:54:16 -0500 Subject: [PATCH] resource/aws_acm_certificate: Automatically trim trailing period from domain_name and subject_alternative_names Trailing periods (which may come from other Terraform AWS provider resource references, notably the aws_route53_zone name attribute in version 1.42.0+) will generate errors with the ACM API. Rather than requiring everyone to implement the same workaround, we can automatically trim the trailing period in the resource. Previous output from acceptance testing: ``` --- FAIL: TestAccAWSAcmCertificate_root_TrailingPeriod (2.74s) testing.go:538: Step 0 error: Error applying: 1 error occurred: * aws_acm_certificate.cert: 1 error occurred: * aws_acm_certificate.cert: Error requesting certificate: ValidationException: 1 validation error detected: Value 'OMITTED.' at 'domainName' failed to satisfy constraint: Member must satisfy regular expression pattern: ^(\*\.)?(((?!-)[A-Za-z0-9-]{0,62}[A-Za-z0-9])\.)+((?!-)[A-Za-z0-9-]{1,62}[A-Za-z0-9])$ status code: 400, request id: 6715d56a-fee5-11e8-9bd0-4dcda69c2aed --- FAIL: TestAccAWSAcmCertificate_san_TrailingPeriod (2.74s) testing.go:538: Step 0 error: Error applying: 1 error occurred: * aws_acm_certificate.cert: 1 error occurred: * aws_acm_certificate.cert: Error requesting certificate: ValidationException: 1 validation error detected: Value '[tf-acc-7681782492806076456-san.OMITTED.]' at 'subjectAlternativeNames' failed to satisfy constraint: Member must satisfy constraint: [Member must have length less than or equal to 253, Member must have length greater than or equal to 1, Member must satisfy regular expression pattern: ^(\*\.)?(((?!-)[A-Za-z0-9-]{0,62}[A-Za-z0-9])\.)+((?!-)[A-Za-z0-9-]{1,62}[A-Za-z0-9])$] status code: 400, request id: 6715fc72-fee5-11e8-8015-e9da8b2fcdff ``` Output from acceptance testing: ``` --- PASS: TestAccAWSAcmCertificate_dnsValidation (11.10s) --- PASS: TestAccAWSAcmCertificate_emailValidation (12.18s) --- PASS: TestAccAWSAcmCertificate_root (10.92s) --- PASS: TestAccAWSAcmCertificate_root_TrailingPeriod (13.64s) --- PASS: TestAccAWSAcmCertificate_rootAndWildcardSan (10.15s) --- PASS: TestAccAWSAcmCertificate_san_multiple (17.00s) --- PASS: TestAccAWSAcmCertificate_san_single (16.21s) --- PASS: TestAccAWSAcmCertificate_san_TrailingPeriod (15.51s) --- PASS: TestAccAWSAcmCertificate_tags (24.08s) --- PASS: TestAccAWSAcmCertificate_wildcard (10.24s) --- PASS: TestAccAWSAcmCertificate_wildcardAndRootSan (15.84s) ``` --- aws/resource_aws_acm_certificate.go | 27 ++++++-- aws/resource_aws_acm_certificate_test.go | 78 ++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 6 deletions(-) diff --git a/aws/resource_aws_acm_certificate.go b/aws/resource_aws_acm_certificate.go index db49e7d0973d..7a3e0b3e2e02 100644 --- a/aws/resource_aws_acm_certificate.go +++ b/aws/resource_aws_acm_certificate.go @@ -3,6 +3,7 @@ package aws import ( "fmt" "log" + "strings" "time" "github.com/aws/aws-sdk-go/aws" @@ -26,12 +27,24 @@ func resourceAwsAcmCertificate() *schema.Resource { Type: schema.TypeString, Required: true, ForceNew: true, + 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), ".") + }, }, "subject_alternative_names": { Type: schema.TypeList, Optional: true, ForceNew: true, - Elem: &schema.Schema{Type: schema.TypeString}, + 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), ".") + }, + }, }, "validation_method": { Type: schema.TypeString, @@ -79,14 +92,16 @@ func resourceAwsAcmCertificate() *schema.Resource { func resourceAwsAcmCertificateCreate(d *schema.ResourceData, meta interface{}) error { acmconn := meta.(*AWSClient).acmconn params := &acm.RequestCertificateInput{ - DomainName: aws.String(d.Get("domain_name").(string)), + DomainName: aws.String(strings.TrimSuffix(d.Get("domain_name").(string), ".")), ValidationMethod: aws.String(d.Get("validation_method").(string)), } - sans, ok := d.GetOk("subject_alternative_names") - if ok { - sanStrings := sans.([]interface{}) - params.SubjectAlternativeNames = expandStringList(sanStrings) + if sans, ok := d.GetOk("subject_alternative_names"); ok { + subjectAlternativeNames := make([]*string, len(sans.([]interface{}))) + for i, sanRaw := range sans.([]interface{}) { + subjectAlternativeNames[i] = aws.String(strings.TrimSuffix(sanRaw.(string), ".")) + } + params.SubjectAlternativeNames = subjectAlternativeNames } log.Printf("[DEBUG] ACM Certificate Request: %#v", params) diff --git a/aws/resource_aws_acm_certificate_test.go b/aws/resource_aws_acm_certificate_test.go index 21a5ca8cfbf9..7d25727af6c4 100644 --- a/aws/resource_aws_acm_certificate_test.go +++ b/aws/resource_aws_acm_certificate_test.go @@ -3,6 +3,7 @@ package aws import ( "fmt" "strconv" + "strings" "testing" "os" @@ -132,6 +133,40 @@ func TestAccAWSAcmCertificate_root(t *testing.T) { }) } +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"), + resource.TestCheckResourceAttr(resourceName, "domain_validation_options.0.domain_name", strings.TrimSuffix(domain, ".")), + resource.TestCheckResourceAttrSet(resourceName, "domain_validation_options.0.resource_record_name"), + resource.TestCheckResourceAttr(resourceName, "domain_validation_options.0.resource_record_type", "CNAME"), + resource.TestCheckResourceAttrSet(resourceName, "domain_validation_options.0.resource_record_value"), + 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, + }, + }, + }) +} + func TestAccAWSAcmCertificate_rootAndWildcardSan(t *testing.T) { rootDomain := testAccAwsAcmCertificateDomainFromEnv(t) wildcardDomain := fmt.Sprintf("*.%s", rootDomain) @@ -260,6 +295,49 @@ func TestAccAWSAcmCertificate_san_multiple(t *testing.T) { }) } +func TestAccAWSAcmCertificate_san_TrailingPeriod(t *testing.T) { + rootDomain := testAccAwsAcmCertificateDomainFromEnv(t) + + rInt1 := acctest.RandInt() + + domain := fmt.Sprintf("tf-acc-%d.%s", rInt1, rootDomain) + sanDomain := fmt.Sprintf("tf-acc-%d-san.%s.", rInt1, rootDomain) + resourceName := "aws_acm_certificate.cert" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckAcmCertificateDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAcmCertificateConfig_subjectAlternativeNames(domain, strconv.Quote(sanDomain), acm.ValidationMethodDns), + Check: resource.ComposeTestCheckFunc( + testAccMatchResourceAttrRegionalARN(resourceName, "arn", "acm", regexp.MustCompile(`certificate/.+`)), + resource.TestCheckResourceAttr(resourceName, "domain_name", domain), + resource.TestCheckResourceAttr(resourceName, "domain_validation_options.#", "2"), + resource.TestCheckResourceAttr(resourceName, "domain_validation_options.0.domain_name", domain), + resource.TestCheckResourceAttrSet(resourceName, "domain_validation_options.0.resource_record_name"), + resource.TestCheckResourceAttr(resourceName, "domain_validation_options.0.resource_record_type", "CNAME"), + resource.TestCheckResourceAttrSet(resourceName, "domain_validation_options.0.resource_record_value"), + resource.TestCheckResourceAttr(resourceName, "domain_validation_options.1.domain_name", strings.TrimSuffix(sanDomain, ".")), + resource.TestCheckResourceAttrSet(resourceName, "domain_validation_options.1.resource_record_name"), + resource.TestCheckResourceAttr(resourceName, "domain_validation_options.1.resource_record_type", "CNAME"), + resource.TestCheckResourceAttrSet(resourceName, "domain_validation_options.1.resource_record_value"), + resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "1"), + resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.0", strings.TrimSuffix(sanDomain, ".")), + resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"), + resource.TestCheckResourceAttr(resourceName, "validation_method", acm.ValidationMethodDns), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + }, + }, + }) +} + func TestAccAWSAcmCertificate_wildcard(t *testing.T) { rootDomain := testAccAwsAcmCertificateDomainFromEnv(t) wildcardDomain := fmt.Sprintf("*.%s", rootDomain)