Skip to content

Commit

Permalink
resource/aws_acm_certificate: Various small improvements and fixes (#…
Browse files Browse the repository at this point in the history 
…13513)

Reference: #3855
Reference: #6082
Reference: #8755
Reference: #12075
Reference: #13053

Notable changes:

```
ENHANCEMENTS:

* resource/aws_acm_certificate: Add `status` attribute

BUG FIXES:

* resource/aws_acm_certificate: Detect `AMAZON_ISSUED` type `validation_method` value directly from API response instead of custom logic
* resource/aws_acm_certificate: Increase deletion retries from 10 minutes to 20 minutes (better support API Gateway Custom Domain deletion)
```

Other changes:

- Documents `subject_alternative_names` argument removal procedures
- Improves potentially confusing error message during asynchronous ACM validation assignment

Output from acceptance testing:

```
--- PASS: TestAccAWSAcmCertificate_imported_IpAddress (36.64s)
--- PASS: TestAccAWSAcmCertificate_root_TrailingPeriod (37.74s)
--- PASS: TestAccAWSAcmCertificate_wildcard (38.86s)
--- PASS: TestAccAWSAcmCertificate_wildcardAndRootSan (39.08s)
--- PASS: TestAccAWSAcmCertificate_emailValidation (39.11s)
--- PASS: TestAccAWSAcmCertificate_dnsValidation (39.35s)
--- PASS: TestAccAWSAcmCertificate_rootAndWildcardSan (39.62s)
--- PASS: TestAccAWSAcmCertificate_disableCTLogging (41.62s)
--- PASS: TestAccAWSAcmCertificate_san_single (41.93s)
--- PASS: TestAccAWSAcmCertificate_san_TrailingPeriod (42.00s)
--- PASS: TestAccAWSAcmCertificate_san_multiple (42.05s)
--- PASS: TestAccAWSAcmCertificate_root (42.07s)
--- PASS: TestAccAWSAcmCertificate_privateCert (47.46s)
--- PASS: TestAccAWSAcmCertificate_imported_DomainName (54.51s)
--- PASS: TestAccAWSAcmCertificate_tags (85.67s)
```
  • Loading branch information
@bflad
bflad authored Jun 4, 2020
1 parent 47a7974 commit 53e6f8c
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 16 deletions.
44 changes: 30 additions & 14 deletions aws/resource_aws_acm_certificate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@ import (
"github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags"
)

const (
// Maximum amount of time for ACM Certificate cross-service reference propagation.
// Removal of ACM Certificates from API Gateway Custom Domains can take >15 minutes.
AcmCertificateCrossServicePropagationTimeout = 20 * time.Minute

// Maximum amount of time for ACM Certificate asynchronous DNS validation record assignment.
// This timeout is unrelated to any creation or validation of those assigned DNS records.
AcmCertificateDnsValidationAssignmentTimeout = 5 * time.Minute
)

func resourceAwsAcmCertificate() *schema.Resource {
return &schema.Resource{
Create: resourceAwsAcmCertificateCreate,
Expand Down Expand Up @@ -143,6 +153,10 @@ func resourceAwsAcmCertificate() *schema.Resource {
},
},
},
"status": {
Type: schema.TypeString,
Computed: true,
},
"tags": tagsSchema(),
},
}
Expand Down Expand Up @@ -227,7 +241,7 @@ func resourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) err
CertificateArn: aws.String(d.Id()),
}

return resource.Retry(time.Duration(5)*time.Minute, func() *resource.RetryError {
return resource.Retry(AcmCertificateDnsValidationAssignmentTimeout, func() *resource.RetryError {
resp, err := acmconn.DescribeCertificate(params)

if err != nil {
Expand Down Expand Up @@ -259,12 +273,14 @@ func resourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) err
return resource.NonRetryableError(err)
}

d.Set("validation_method", resourceAwsAcmCertificateGuessValidationMethod(domainValidationOptions, emailValidationOptions))
d.Set("validation_method", resourceAwsAcmCertificateValidationMethod(resp.Certificate))

if err := d.Set("options", flattenAcmCertificateOptions(resp.Certificate.Options)); err != nil {
return resource.NonRetryableError(fmt.Errorf("error setting certificate options: %s", err))
}

d.Set("status", resp.Certificate.Status)

tags, err := keyvaluetags.AcmListTags(acmconn, d.Id())

if err != nil {
Expand All @@ -278,16 +294,16 @@ func resourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) err
return nil
})
}
func resourceAwsAcmCertificateGuessValidationMethod(domainValidationOptions []map[string]interface{}, emailValidationOptions []string) string {
// The DescribeCertificate Response doesn't have information on what validation method was used
// so we need to guess from the validation options we see...
if len(domainValidationOptions) > 0 {
return acm.ValidationMethodDns
} else if len(emailValidationOptions) > 0 {
return acm.ValidationMethodEmail
} else {
return "NONE"
func resourceAwsAcmCertificateValidationMethod(certificate *acm.CertificateDetail) string {
if aws.StringValue(certificate.Type) == acm.CertificateTypeAmazonIssued {
for _, domainValidation := range certificate.DomainValidationOptions {
if domainValidation.ValidationMethod != nil {
return aws.StringValue(domainValidation.ValidationMethod)
}
}
}

return "NONE"
}

func resourceAwsAcmCertificateUpdate(d *schema.ResourceData, meta interface{}) error {
Expand Down Expand Up @@ -345,8 +361,8 @@ func convertValidationOptions(certificate *acm.CertificateDetail) ([]map[string]
emailValidationResult = append(emailValidationResult, *validationEmail)
}
} else if o.ValidationStatus == nil || aws.StringValue(o.ValidationStatus) == acm.DomainStatusPendingValidation {
log.Printf("[DEBUG] No validation options need to retry: %#v", o)
return nil, nil, fmt.Errorf("No validation options need to retry: %#v", o)
log.Printf("[DEBUG] Asynchronous ACM service domain validation assignment not complete, need to retry: %#v", o)
return nil, nil, fmt.Errorf("asynchronous ACM service domain validation assignment not complete, need to retry: %#v", o)
}
}
case acm.CertificateTypePrivate:
Expand All @@ -369,7 +385,7 @@ func resourceAwsAcmCertificateDelete(d *schema.ResourceData, meta interface{}) e
CertificateArn: aws.String(d.Id()),
}

err := resource.Retry(10*time.Minute, func() *resource.RetryError {
err := resource.Retry(AcmCertificateCrossServicePropagationTimeout, func() *resource.RetryError {
_, err := acmconn.DeleteCertificate(params)
if err != nil {
if isAWSErr(err, acm.ErrCodeResourceInUseException, "") {
Expand Down
15 changes: 15 additions & 0 deletions aws/resource_aws_acm_certificate_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,7 @@ func TestAccAWSAcmCertificate_emailValidation(t *testing.T) {
testAccMatchResourceAttrRegionalARN(resourceName, "arn", "acm", regexp.MustCompile("certificate/.+$")),
resource.TestCheckResourceAttr(resourceName, "domain_name", domain),
resource.TestCheckResourceAttr(resourceName, "domain_validation_options.#", "0"),
resource.TestCheckResourceAttr(resourceName, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "0"),
resource.TestMatchResourceAttr(resourceName, "validation_emails.0", regexp.MustCompile(`^[^@]+@.+$`)),
resource.TestCheckResourceAttr(resourceName, "validation_method", acm.ValidationMethodEmail),
Expand Down Expand Up @@ -166,6 +167,7 @@ func TestAccAWSAcmCertificate_dnsValidation(t *testing.T) {
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, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_method", acm.ValidationMethodDns),
Expand Down Expand Up @@ -199,6 +201,7 @@ func TestAccAWSAcmCertificate_root(t *testing.T) {
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, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_method", acm.ValidationMethodDns),
Expand Down Expand Up @@ -229,6 +232,7 @@ func TestAccAWSAcmCertificate_privateCert(t *testing.T) {
testAccMatchResourceAttrRegionalARN(resourceName, "arn", "acm", regexp.MustCompile("certificate/.+$")),
resource.TestCheckResourceAttr(resourceName, "domain_name", fmt.Sprintf("%s.terraformtesting.com", rName)),
resource.TestCheckResourceAttr(resourceName, "domain_validation_options.#", "0"),
resource.TestCheckResourceAttr(resourceName, "status", acm.CertificateStatusFailed), // FailureReason: PCA_INVALID_STATE (PCA State: PENDING_CERTIFICATE)
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_method", "NONE"),
Expand Down Expand Up @@ -264,6 +268,7 @@ func TestAccAWSAcmCertificate_root_TrailingPeriod(t *testing.T) {
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, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_method", acm.ValidationMethodDns),
Expand Down Expand Up @@ -302,6 +307,7 @@ func TestAccAWSAcmCertificate_rootAndWildcardSan(t *testing.T) {
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, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "1"),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.0", wildcardDomain),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
Expand Down Expand Up @@ -342,6 +348,7 @@ func TestAccAWSAcmCertificate_san_single(t *testing.T) {
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, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "1"),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.0", sanDomain),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
Expand Down Expand Up @@ -387,6 +394,7 @@ func TestAccAWSAcmCertificate_san_multiple(t *testing.T) {
resource.TestCheckResourceAttrSet(resourceName, "domain_validation_options.2.resource_record_name"),
resource.TestCheckResourceAttr(resourceName, "domain_validation_options.2.resource_record_type", "CNAME"),
resource.TestCheckResourceAttrSet(resourceName, "domain_validation_options.2.resource_record_value"),
resource.TestCheckResourceAttr(resourceName, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "2"),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.0", sanDomain1),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.1", sanDomain2),
Expand Down Expand Up @@ -428,6 +436,7 @@ func TestAccAWSAcmCertificate_san_TrailingPeriod(t *testing.T) {
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, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "1"),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.0", strings.TrimSuffix(sanDomain, ".")),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
Expand Down Expand Up @@ -463,6 +472,7 @@ func TestAccAWSAcmCertificate_wildcard(t *testing.T) {
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, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_method", acm.ValidationMethodDns),
Expand Down Expand Up @@ -501,6 +511,7 @@ func TestAccAWSAcmCertificate_wildcardAndRootSan(t *testing.T) {
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, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "1"),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.0", rootDomain),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
Expand Down Expand Up @@ -536,6 +547,7 @@ func TestAccAWSAcmCertificate_disableCTLogging(t *testing.T) {
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, "status", acm.CertificateStatusPendingValidation),
resource.TestCheckResourceAttr(resourceName, "validation_emails.#", "0"),
resource.TestCheckResourceAttr(resourceName, "validation_method", acm.ValidationMethodDns),
resource.TestCheckResourceAttr(resourceName, "options.#", "1"),
Expand Down Expand Up @@ -611,13 +623,15 @@ func TestAccAWSAcmCertificate_imported_DomainName(t *testing.T) {
{
Config: testAccAcmCertificateConfigPrivateKey("example.com"),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "status", acm.CertificateStatusIssued),
resource.TestCheckResourceAttr(resourceName, "tags.%", "0"),
resource.TestCheckResourceAttr(resourceName, "domain_name", "example.com"),
),
},
{
Config: testAccAcmCertificateConfigPrivateKey("example.org"),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "status", acm.CertificateStatusIssued),
resource.TestCheckResourceAttr(resourceName, "tags.%", "0"),
resource.TestCheckResourceAttr(resourceName, "domain_name", "example.org"),
),
Expand Down Expand Up @@ -646,6 +660,7 @@ func TestAccAWSAcmCertificate_imported_IpAddress(t *testing.T) { // Reference: h
Config: testAccAcmCertificateConfigPrivateKey("1.2.3.4"),
Check: resource.ComposeTestCheckFunc(
resource.TestCheckResourceAttr(resourceName, "domain_name", ""),
resource.TestCheckResourceAttr(resourceName, "status", acm.CertificateStatusIssued),
resource.TestCheckResourceAttr(resourceName, "subject_alternative_names.#", "0"),
),
},
Expand Down
5 changes: 3 additions & 2 deletions website/docs/r/acm_certificate.html.markdown
View file
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ The following arguments are supported:

* Creating an amazon issued certificate
* `domain_name` - (Required) A domain name for which the certificate should be issued
* `subject_alternative_names` - (Optional) A list of domains that should be SANs in the issued certificate
* `subject_alternative_names` - (Optional) A list of domains that should be SANs in the issued certificate. To remove all elements of a previously configured list, set this value equal to an empty list (`[]`) or use the [`terraform taint` command](https://www.terraform.io/docs/commands/taint.html) to trigger recreation.
* `validation_method` - (Required) Which method to use for validation. `DNS` or `EMAIL` are valid, `NONE` can be used for certificates that were imported into ACM and then into Terraform.
* `options` - (Optional) Configuration block used to set certificate options. Detailed below.
* Importing an existing certificate
Expand All @@ -92,7 +92,7 @@ The following arguments are supported:
* Creating a private CA issued certificate
* `domain_name` - (Required) A domain name for which the certificate should be issued
* `certificate_authority_arn` - (Required) ARN of an ACMPCA
* `subject_alternative_names` - (Optional) A list of domains that should be SANs in the issued certificate
* `subject_alternative_names` - (Optional) A list of domains that should be SANs in the issued certificate. To remove all elements of a previously configured list, set this value equal to an empty list (`[]`) or use the [`terraform taint` command](https://www.terraform.io/docs/commands/taint.html) to trigger recreation.
* `tags` - (Optional) A map of tags to assign to the resource.

## options Configuration Block
Expand All @@ -109,6 +109,7 @@ In addition to all arguments above, the following attributes are exported:
* `arn` - The ARN of the certificate
* `domain_name` - The domain name for which the certificate is issued
* `domain_validation_options` - A list of attributes to feed into other resources to complete certificate validation. Can have more than one element, e.g. if SANs are defined. Only set if `DNS`-validation was used.
* `status` - Status of the certificate.
* `validation_emails` - A list of addresses that received a validation E-Mail. Only set if `EMAIL`-validation was used.

Domain validation objects export the following attributes:
Expand Down

0 comments on commit 53e6f8c

Please sign in to comment.