Skip to content

Commit

Permalink
Revert "resource/aws_acm_certificate: Retry logic refactor" (#10184)
Browse files Browse the repository at this point in the history 
* Revert "Merge pull request #9863 from terraform-providers/rfd-retry-acm"

This reverts commit 4879c29, reversing
changes made to 38fc3b9.

Before Change
```
--- FAIL: TestAccAWSAcmCertificate_tags (76.71s)
    testing.go:569: Step 0 error: errors during apply:

        Error: error getting validation options for ACM certificate: no validation options need to retry.

          on /tmp/tf-test030858259/main.tf line 2:
          (source code not available)

--- FAIL: TestAccAWSAcmCertificate_san_TrailingPeriod (76.93s)
--- FAIL: TestAccAWSAcmCertificate_disableCTLogging (77.52s)
--- FAIL: TestAccAWSAcmCertificate_san_single (78.13s)
--- FAIL: TestAccAWSAcmCertificate_root_TrailingPeriod (81.75s)

--- FAIL: TestAccAWSAcmCertificateValidation_validationRecordFqdnsEmail (74.93s)
    testing.go:562: Step 0, expected error:

        errors during apply: error getting validation options for ACM certificate: no validation options need to retry.

        To match:

        validation_record_fqdns is only valid for DNS validation
--- FAIL: TestAccAWSAcmCertificateValidation_timeout (75.22s)
--- FAIL: TestAccAWSAcmCertificateValidation_validationRecordFqdns (77.35s)
--- FAIL: TestAccAWSAcmCertificateValidation_validationRecordFqdnsRoot (79.41s)
--- FAIL: TestAccAWSAcmCertificateValidation_validationRecordFqdnsWildcardAndRoot (79.65s)
--- FAIL: TestAccAWSAcmCertificateValidation_basic (79.71s)
--- FAIL: TestAccAWSAcmCertificateValidation_validationRecordFqdnsRootAndWildcard (80.34s)
--- FAIL: TestAccAWSAcmCertificateValidation_validationRecordFqdnsSan (80.93s)
--- FAIL: TestAccAWSAcmCertificateValidation_validationRecordFqdnsWildcard (82.78s)

```

After Revert
```
--- PASS: TestAccAWSAcmCertificate_imported_IpAddress (24.53s)
--- PASS: TestAccAWSAcmCertificate_emailValidation (27.48s)
--- PASS: TestAccAWSAcmCertificate_root (27.82s)
--- PASS: TestAccAWSAcmCertificate_root_TrailingPeriod (29.32s)
--- PASS: TestAccAWSAcmCertificate_dnsValidation (29.96s)
--- PASS: TestAccAWSAcmCertificate_disableCTLogging (30.17s)
--- PASS: TestAccAWSAcmCertificate_san_TrailingPeriod (30.38s)
--- PASS: TestAccAWSAcmCertificate_wildcardAndRootSan (30.80s)
--- PASS: TestAccAWSAcmCertificate_wildcard (30.85s)
--- PASS: TestAccAWSAcmCertificate_rootAndWildcardSan (32.26s)
--- PASS: TestAccAWSAcmCertificate_privateCert (34.07s)
--- PASS: TestAccAWSAcmCertificate_san_single (36.02s)
--- PASS: TestAccAWSAcmCertificate_imported_DomainName (37.99s)
--- PASS: TestAccAWSAcmCertificate_tags (66.36s)

--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsEmail (17.38s)
--- PASS: TestAccAWSAcmCertificateValidation_timeout (24.80s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsWildcard (668.03s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsRootAndWildcard (672.25s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsWildcardAndRoot (672.72s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdns (698.98s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsSan (700.61s)
--- PASS: TestAccAWSAcmCertificateValidation_validationRecordFqdnsRoot (702.39s)
--- PASS: TestAccAWSAcmCertificateValidation_basic (708.19s)
```

* CHANGELOG: remove entry for #9863
  • Loading branch information
@nywilken
nywilken authored Sep 20, 2019
1 parent 8aa2cac commit 73663bf
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 66 deletions.
9 changes: 4 additions & 5 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,7 @@ ENHANCEMENTS:
* resource/aws_waf_rate_based_rule: Update rate based rule limit for WAF ([#9946](https://github.com/terraform-providers/terraform-provider-aws/pull/9946))
* resource/aws_wafregional_rate_based_rule: Update rate based rule limit for WAF ([#9946](https://github.com/terraform-providers/terraform-provider-aws/pull/9946))

BUG FIXES:
* resource/aws_acm_certificate: Final retries after timeouts reading and deleting certificates [GH-9863]
BUG FIXES:
* ecs_task_definition_equivalency: Fix a crash if environment name is missing [GH-10074]

## 2.28.1 (September 12, 2019)
Expand Down Expand Up @@ -348,7 +347,7 @@ ENHANCEMENTS:

BUG FIXES:

* resource/aws_appautoscaling_policy: Properly support importing of dynamodb policies [[#8397](https://github.com/terraform-providers/terraform-provider-aws/issues/8397)]
* resource/aws_appautoscaling_policy: Properly support importing of dynamodb policies [[#8397](https://github.com/terraform-providers/terraform-provider-aws/issues/8397)]
* resource/aws_cloudwatch_event_permissions: Clean up error handling when reading event permissions ([#9065](https://github.com/terraform-providers/terraform-provider-aws/issues/9065))
* resource/aws_cloudwatch_event_rule: Retry error handling when creating and updating event rules ([#9065](https://github.com/terraform-providers/terraform-provider-aws/issues/9065))
* resource/aws_cloudwatch_log_destination: Clean up error handling when putting log destination ([#9065](https://github.com/terraform-providers/terraform-provider-aws/issues/9065))
Expand Down Expand Up @@ -478,7 +477,7 @@ ENHANCEMENTS:
* resource/aws_backup_vault: Support resource import ([#9041](https://github.com/terraform-providers/terraform-provider-aws/issues/9041))
* resource/aws_codepipeline: Add `tags` argument ([#8993](https://github.com/terraform-providers/terraform-provider-aws/issues/8993))
* resource/aws_codepipeline_webhook: Add `tags` argument ([#8993](https://github.com/terraform-providers/terraform-provider-aws/issues/8993))
* resource/aws_ecs_task_definition: Add `proxy_configuration` configuration block (support AppMesh proxying) [[#8780](https://github.com/terraform-providers/terraform-provider-aws/issues/8780)]
* resource/aws_ecs_task_definition: Add `proxy_configuration` configuration block (support AppMesh proxying) [[#8780](https://github.com/terraform-providers/terraform-provider-aws/issues/8780)]
* resource/aws_instance: Prevent panic when `credit_specification` configuration block is missing arguments ([#9003](https://github.com/terraform-providers/terraform-provider-aws/issues/9003))
* resource/aws_organizations_organization: Add `non_master_accounts` attribute ([#8926](https://github.com/terraform-providers/terraform-provider-aws/issues/8926))
* resource/aws_secretsmanager_secret: Tag on create (support tag limiting IAM policies) ([#9023](https://github.com/terraform-providers/terraform-provider-aws/issues/9023))
Expand Down Expand Up @@ -639,7 +638,7 @@ ENHANCEMENTS:
* resource/aws_elastic_beanstalk_application: Add `tags` argument and `arn` attribute ([#8614](https://github.com/terraform-providers/terraform-provider-aws/issues/8614))
* resource/aws_elastic_beanstalk_application_version: Add `tags` argument and `arn` attribute ([#8614](https://github.com/terraform-providers/terraform-provider-aws/issues/8614))
* resource/aws_emr_cluster: Add `master_instance_group` and `core_instance_group` configuration blocks (deprecates other instance group configuration methods) ([#8459](https://github.com/terraform-providers/terraform-provider-aws/issues/8459))
* resource/aws_emr_instance_group: Add support for `autoscaling_policy`, `bid_price`, and resource import [[#8078](https://github.com/terraform-providers/terraform-provider-aws/issues/8078)]
* resource/aws_emr_instance_group: Add support for `autoscaling_policy`, `bid_price`, and resource import [[#8078](https://github.com/terraform-providers/terraform-provider-aws/issues/8078)]
* resource/aws_kinesis_analytics_application: Add `tags` argument ([#8643](https://github.com/terraform-providers/terraform-provider-aws/issues/8643))
* resource/aws_lambda_function: Support `nodejs10.x` in `runtime` validation ([#8622](https://github.com/terraform-providers/terraform-provider-aws/issues/8622))
* resource/aws_organizations_account: Add parent_id argument (support moving accounts) ([#8583](https://github.com/terraform-providers/terraform-provider-aws/issues/8583))
Expand Down
108 changes: 47 additions & 61 deletions aws/resource_aws_acm_certificate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func resourceAwsAcmCertificateCreateImported(d *schema.ResourceData, meta interf
acmconn := meta.(*AWSClient).acmconn
resp, err := resourceAwsAcmCertificateImport(acmconn, d, false)
if err != nil {
return fmt.Errorf("error importing certificate: %s", err)
return fmt.Errorf("Error importing certificate: %s", err)
}

d.SetId(*resp.CertificateArn)
Expand All @@ -182,7 +182,7 @@ func resourceAwsAcmCertificateCreateImported(d *schema.ResourceData, meta interf
_, err := acmconn.AddTagsToCertificate(params)

if err != nil {
return fmt.Errorf("error requesting certificate: %s", err)
return fmt.Errorf("Error requesting certificate: %s", err)
}
}

Expand Down Expand Up @@ -217,7 +217,7 @@ func resourceAwsAcmCertificateCreateRequested(d *schema.ResourceData, meta inter
resp, err := acmconn.RequestCertificate(params)

if err != nil {
return fmt.Errorf("error requesting certificate: %s", err)
return fmt.Errorf("Error requesting certificate: %s", err)
}

d.SetId(*resp.CertificateArn)
Expand All @@ -229,7 +229,7 @@ func resourceAwsAcmCertificateCreateRequested(d *schema.ResourceData, meta inter
_, err := acmconn.AddTagsToCertificate(params)

if err != nil {
return fmt.Errorf("error requesting certificate: %s", err)
return fmt.Errorf("Error requesting certificate: %s", err)
}
}

Expand All @@ -243,70 +243,58 @@ func resourceAwsAcmCertificateRead(d *schema.ResourceData, meta interface{}) err
CertificateArn: aws.String(d.Id()),
}

resp, err := acmconn.DescribeCertificate(params)
if err != nil {
if !isAWSErr(err, acm.ErrCodeResourceNotFoundException, "") {
d.SetId("")
return nil
return resource.Retry(time.Duration(1)*time.Minute, func() *resource.RetryError {
resp, err := acmconn.DescribeCertificate(params)

if err != nil {
if isAWSErr(err, acm.ErrCodeResourceNotFoundException, "") {
d.SetId("")
return nil
}
return resource.NonRetryableError(fmt.Errorf("Error describing certificate: %s", err))
}
return fmt.Errorf("error describing certificate: %s", err)
}

if resp == nil {
return fmt.Errorf("empty response received when describing ACM certificate")
}
d.Set("domain_name", resp.Certificate.DomainName)
d.Set("arn", resp.Certificate.CertificateArn)
d.Set("certificate_authority_arn", resp.Certificate.CertificateAuthorityArn)
d.Set("domain_name", resp.Certificate.DomainName)
d.Set("arn", resp.Certificate.CertificateArn)
d.Set("certificate_authority_arn", resp.Certificate.CertificateAuthorityArn)

if err := d.Set("subject_alternative_names", cleanUpSubjectAlternativeNames(resp.Certificate)); err != nil {
return fmt.Errorf("error setting subject alternative nates: %s", err)
}
if err := d.Set("subject_alternative_names", cleanUpSubjectAlternativeNames(resp.Certificate)); err != nil {
return resource.NonRetryableError(err)
}

var domainValidationOptions []map[string]interface{}
var emailValidationOptions []string
err = resource.Retry(time.Duration(1)*time.Minute, func() *resource.RetryError {
var err error
domainValidationOptions, emailValidationOptions, err = convertValidationOptions(resp.Certificate)
domainValidationOptions, emailValidationOptions, err := convertValidationOptions(resp.Certificate)

if err != nil {
return resource.RetryableError(err)
}
return nil
})
if isResourceTimeoutError(err) {
domainValidationOptions, emailValidationOptions, err = convertValidationOptions(resp.Certificate)
}
if err != nil {
return fmt.Errorf("error getting validation options for ACM certificate: %s", err)
}

if err := d.Set("domain_validation_options", domainValidationOptions); err != nil {
return fmt.Errorf("error setting domain validation options: %s", err)
}
if err := d.Set("validation_emails", emailValidationOptions); err != nil {
return fmt.Errorf("error setting email validation options: %s", err)
}
if err := d.Set("domain_validation_options", domainValidationOptions); err != nil {
return resource.NonRetryableError(err)
}
if err := d.Set("validation_emails", emailValidationOptions); err != nil {
return resource.NonRetryableError(err)
}

d.Set("validation_method", resourceAwsAcmCertificateGuessValidationMethod(domainValidationOptions, emailValidationOptions))
d.Set("validation_method", resourceAwsAcmCertificateGuessValidationMethod(domainValidationOptions, emailValidationOptions))

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

input := &acm.ListTagsForCertificateInput{
CertificateArn: aws.String(d.Id()),
}
params := &acm.ListTagsForCertificateInput{
CertificateArn: aws.String(d.Id()),
}

tagResp, err := acmconn.ListTagsForCertificate(input)
if err != nil {
return fmt.Errorf("error listing tags for certificate (%s): %s", d.Id(), err)
}
if err := d.Set("tags", tagsToMapACM(tagResp.Tags)); err != nil {
return fmt.Errorf("error setting tags for certificate: %s", err)
}
tagResp, err := acmconn.ListTagsForCertificate(params)
if err != nil {
return resource.NonRetryableError(fmt.Errorf("error listing tags for certificate (%s): %s", d.Id(), err))
}
if err := d.Set("tags", tagsToMapACM(tagResp.Tags)); err != nil {
return resource.NonRetryableError(err)
}

return nil
return nil
})
}
func resourceAwsAcmCertificateGuessValidationMethod(domainValidationOptions []map[string]interface{}, emailValidationOptions []string) string {
// The DescribeCertificate Response doesn't have information on what validation method was used
Expand All @@ -326,7 +314,7 @@ func resourceAwsAcmCertificateUpdate(d *schema.ResourceData, meta interface{}) e
if d.HasChange("private_key") || d.HasChange("certificate_body") || d.HasChange("certificate_chain") {
_, err := resourceAwsAcmCertificateImport(acmconn, d, true)
if err != nil {
return fmt.Errorf("error updating certificate: %s", err)
return fmt.Errorf("Error updating certificate: %s", err)
}
}

Expand Down Expand Up @@ -359,7 +347,7 @@ func convertValidationOptions(certificate *acm.CertificateDetail) ([]map[string]
case acm.CertificateTypeAmazonIssued:
if len(certificate.DomainValidationOptions) == 0 && aws.StringValue(certificate.Status) == acm.DomainStatusPendingValidation {
log.Printf("[DEBUG] No validation options need to retry.")
return nil, nil, fmt.Errorf("no validation options need to retry.")
return nil, nil, fmt.Errorf("No validation options need to retry.")
}
for _, o := range certificate.DomainValidationOptions {
if o.ResourceRecord != nil {
Expand All @@ -376,7 +364,7 @@ func convertValidationOptions(certificate *acm.CertificateDetail) ([]map[string]
}
} 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)
return nil, nil, fmt.Errorf("No validation options need to retry: %#v", o)
}
}
case acm.CertificateTypePrivate:
Expand Down Expand Up @@ -410,11 +398,9 @@ func resourceAwsAcmCertificateDelete(d *schema.ResourceData, meta interface{}) e
}
return nil
})
if isResourceTimeoutError(err) {
_, err = acmconn.DeleteCertificate(params)
}

if err != nil && !isAWSErr(err, acm.ErrCodeResourceNotFoundException, "") {
return fmt.Errorf("error deleting certificate: %s", err)
return fmt.Errorf("Error deleting certificate: %s", err)
}

return nil
Expand Down

0 comments on commit 73663bf

Please sign in to comment.