Skip to content

Commit

Permalink
resource/aws_cloudfront_distribution: Remove viewer_certificate con…
Browse files Browse the repository at this point in the history
…figuration block argument `ConflictsWith` usage and fix various related issues with deployment timing

References:
* #7773
* #3077
* #1074
* #260

Here we remove the problematic `viewer_certificate` argument `ConflictsWith` schema configuration as it interferes with Terraform Module usage until Terraform 0.12 is more prevalent. More details: #7773 (comment)

When writing acceptance testing to cover setting both the `viewer_certificate` configuration block `acm_certificate_arn` and `cloudfront_default_certificate` arguments being defined, the below error was consistently happening when the test configuration included `enabled = false`:

```
--- FAIL: TestAccAWSCloudFrontDistribution_ViewerCertificate_AcmCertificateArn (1935.57s)
    testing.go:599: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.

        Error: Error applying: 1 error occurred:
          * aws_cloudfront_distribution.test (destroy): 1 error occurred:
          * aws_cloudfront_distribution.test: CloudFront Distribution E3GDAPNU6UPO0O cannot be deleted: PreconditionFailed: The request failed because it didn't meet the preconditions in one or more request-header fields.
          status code: 412, request id: 4e73a086-3c33-11e9-832f-7732257f45e8
```

While debugging this the following further issues were encountered:

* The resource did not wait for deployment to complete on creation and updates so in the acceptance testing the deletion function was always handling `InProgress` operations.
* Disabled distributions would always update the distribution on deletion to disable them without checking if it was necessary, causing unnecessary delays.
* The `PreconditionFailed` error seemed to be related to some eventual consistency issue within CloudFront right after disabling the distribution, which was always done. Retrying was a sufficient workaround for the error.
* The deletion process did not ignore `NoSuchDistribution` errors such as the below:

```
--- FAIL: TestAccAWSCloudFrontDistribution_ViewerCertificate_AcmCertificateArn (1835.97s)
    testing.go:599: Error destroying resource! WARNING: Dangling resources
        may exist. The full state and error is shown below.

        Error: Error applying: 1 error occurred:
          * aws_cloudfront_distribution.test (destroy): 1 error occurred:
          * aws_cloudfront_distribution.test: CloudFront Distribution E2HQM77NFHV9T cannot be deleted: NoSuchDistribution: The specified distribution does not exist.
```

This changeset bundles all these fixes together as they are related.

Previous output for `ConflictsWith` acceptance testing:

```
--- FAIL: TestAccAWSCloudFrontDistribution_ViewerCertificate_AcmCertificateArn_ConflictsWithCloudFrontDefaultCertificate (1.64s)
    testing.go:538: Step 0 error: config is invalid: 2 problems:

        - aws_cloudfront_distribution.test: "viewer_certificate.0.acm_certificate_arn": conflicts with viewer_certificate.0.cloudfront_default_certificate
        - aws_cloudfront_distribution.test: "viewer_certificate.0.cloudfront_default_certificate": conflicts with viewer_certificate.0.acm_certificate_arn
```

Output from acceptance testing:

```
--- PASS: TestAccAWSCloudFrontDistribution_Origin_EmptyDomainName (2.15s)
--- PASS: TestAccAWSCloudFrontDistribution_Origin_EmptyOriginID (2.19s)
--- PASS: TestAccAWSCloudFrontDistribution_ViewerCertificate_AcmCertificateArn_ConflictsWithCloudFrontDefaultCertificate (1915.48s)
--- PASS: TestAccAWSCloudFrontDistribution_ViewerCertificate_AcmCertificateArn (1958.08s)
--- PASS: TestAccAWSCloudFrontDistribution_noCustomErrorResponseConfig (2121.58s)
--- PASS: TestAccAWSCloudFrontDistribution_HTTP11Config (2123.60s)
--- PASS: TestAccAWSCloudFrontDistribution_orderedCacheBehavior (2126.73s)
--- PASS: TestAccAWSCloudFrontDistribution_noOptionalItemsConfig (2126.82s)
--- PASS: TestAccAWSCloudFrontDistribution_IsIPV6EnabledConfig (2176.09s)
--- PASS: TestAccAWSCloudFrontDistribution_customOrigin (2178.92s)
--- PASS: TestAccAWSCloudFrontDistribution_S3Origin (2178.98s)
--- PASS: TestAccAWSCloudFrontDistribution_multiOrigin (2179.08s)
--- PASS: TestAccAWSCloudFrontDistribution_S3OriginWithTags (3251.14s)
```
  • Loading branch information
bflad committed Mar 3, 2019
1 parent 9ee64b5 commit 286f294
Show file tree
Hide file tree
Showing 2 changed files with 313 additions and 35 deletions.
159 changes: 126 additions & 33 deletions aws/resource_aws_cloudfront_distribution.go
Original file line number Diff line number Diff line change
Expand Up @@ -606,19 +606,16 @@ func resourceAwsCloudFrontDistribution() *schema.Resource {
Elem: &schema.Resource{
Schema: map[string]*schema.Schema{
"acm_certificate_arn": {
Type: schema.TypeString,
Optional: true,
ConflictsWith: []string{"viewer_certificate.0.cloudfront_default_certificate", "viewer_certificate.0.iam_certificate_id"},
Type: schema.TypeString,
Optional: true,
},
"cloudfront_default_certificate": {
Type: schema.TypeBool,
Optional: true,
ConflictsWith: []string{"viewer_certificate.0.acm_certificate_arn", "viewer_certificate.0.iam_certificate_id"},
Type: schema.TypeBool,
Optional: true,
},
"iam_certificate_id": {
Type: schema.TypeString,
Optional: true,
ConflictsWith: []string{"viewer_certificate.0.acm_certificate_arn", "viewer_certificate.0.cloudfront_default_certificate"},
Type: schema.TypeString,
Optional: true,
},
"minimum_protocol_version": {
Type: schema.TypeString,
Expand Down Expand Up @@ -717,6 +714,12 @@ func resourceAwsCloudFrontDistributionCreate(d *schema.ResourceData, meta interf
}

d.SetId(*resp.Distribution.Id)

log.Printf("[DEBUG] Waiting until CloudFront Distribution (%s) is deployed", d.Id())
if err := resourceAwsCloudFrontDistributionWaitUntilDeployed(d.Id(), meta); err != nil {
return fmt.Errorf("error waiting until CloudFront Distribution (%s) is deployed: %s", d.Id(), err)
}

return resourceAwsCloudFrontDistributionRead(d, meta)
}

Expand Down Expand Up @@ -796,6 +799,11 @@ func resourceAwsCloudFrontDistributionUpdate(d *schema.ResourceData, meta interf
return fmt.Errorf("error updating CloudFront Distribution (%s): %s", d.Id(), err)
}

log.Printf("[DEBUG] Waiting until CloudFront Distribution (%s) is deployed", d.Id())
if err := resourceAwsCloudFrontDistributionWaitUntilDeployed(d.Id(), meta); err != nil {
return fmt.Errorf("error waiting until CloudFront Distribution (%s) is deployed: %s", d.Id(), err)
}

if err := setTagsCloudFront(conn, d, d.Get("arn").(string)); err != nil {
return err
}
Expand All @@ -806,42 +814,127 @@ func resourceAwsCloudFrontDistributionUpdate(d *schema.ResourceData, meta interf
func resourceAwsCloudFrontDistributionDelete(d *schema.ResourceData, meta interface{}) error {
conn := meta.(*AWSClient).cloudfrontconn

// manually disable the distribution first
d.Set("enabled", false)
err := resourceAwsCloudFrontDistributionUpdate(d, meta)
if err != nil {
return err
}

// skip delete if retain_on_delete is enabled
if d.Get("retain_on_delete").(bool) {
log.Printf("[WARN] Removing CloudFront Distribution ID %q with `retain_on_delete` set. Please delete this distribution manually.", d.Id())
return nil
}

// Distribution needs to be in deployed state again before it can be deleted.
err = resourceAwsCloudFrontDistributionWaitUntilDeployed(d.Id(), meta)
if err != nil {
return err
}

// now delete
params := &cloudfront.DeleteDistributionInput{
deleteDistributionInput := &cloudfront.DeleteDistributionInput{
Id: aws.String(d.Id()),
IfMatch: aws.String(d.Get("etag").(string)),
}

// Eventual consistency for "deployed" state
err = resource.Retry(1*time.Minute, func() *resource.RetryError {
_, err := conn.DeleteDistribution(params)
log.Printf("[DEBUG] Deleting CloudFront Distribution: %s", d.Id())
_, err := conn.DeleteDistribution(deleteDistributionInput)

if err == nil || isAWSErr(err, cloudfront.ErrCodeNoSuchDistribution, "") {
return nil
}

// Refresh our ETag if it is out of date and attempt deletion again.
if isAWSErr(err, cloudfront.ErrCodeInvalidIfMatchVersion, "") {
getDistributionInput := &cloudfront.GetDistributionInput{
Id: aws.String(d.Id()),
}
var getDistributionOutput *cloudfront.GetDistributionOutput

log.Printf("[DEBUG] Refreshing CloudFront Distribution (%s) ETag", d.Id())
getDistributionOutput, err = conn.GetDistribution(getDistributionInput)

if err != nil {
if isAWSErr(err, cloudfront.ErrCodeDistributionNotDisabled, "The distribution you are trying to delete has not been disabled.") {
return resource.RetryableError(err)
return fmt.Errorf("error refreshing CloudFront Distribution (%s) ETag: %s", d.Id(), err)
}

if getDistributionOutput == nil {
return fmt.Errorf("error refreshing CloudFront Distribution (%s) ETag: empty response", d.Id())
}

deleteDistributionInput.IfMatch = getDistributionOutput.ETag

_, err = conn.DeleteDistribution(deleteDistributionInput)
}

// Disable distribution if it is not yet disabled and attempt deletion again.
// Here we update via the deployed configuration to ensure we are not submitting an out of date
// configuration from the Terraform configuration, should other changes have occurred manually.
if isAWSErr(err, cloudfront.ErrCodeDistributionNotDisabled, "") {
getDistributionInput := &cloudfront.GetDistributionInput{
Id: aws.String(d.Id()),
}
var getDistributionOutput *cloudfront.GetDistributionOutput

log.Printf("[DEBUG] Refreshing CloudFront Distribution (%s) to disable", d.Id())
getDistributionOutput, err = conn.GetDistribution(getDistributionInput)

if err != nil {
return fmt.Errorf("error refreshing CloudFront Distribution (%s) to disable: %s", d.Id(), err)
}

if getDistributionOutput == nil || getDistributionOutput.Distribution == nil {
return fmt.Errorf("error refreshing CloudFront Distribution (%s) to disable: empty response", d.Id())
}

updateDistributionInput := &cloudfront.UpdateDistributionInput{
DistributionConfig: getDistributionOutput.Distribution.DistributionConfig,
Id: deleteDistributionInput.Id,
IfMatch: getDistributionOutput.ETag,
}
updateDistributionInput.DistributionConfig.Enabled = aws.Bool(false)
var updateDistributionOutput *cloudfront.UpdateDistributionOutput

log.Printf("[DEBUG] Disabling CloudFront Distribution: %s", d.Id())
updateDistributionOutput, err = conn.UpdateDistribution(updateDistributionInput)

if err != nil {
return fmt.Errorf("error disabling CloudFront Distribution (%s): %s", d.Id(), err)
}

log.Printf("[DEBUG] Waiting until CloudFront Distribution (%s) is deployed", d.Id())
if err := resourceAwsCloudFrontDistributionWaitUntilDeployed(d.Id(), meta); err != nil {
return fmt.Errorf("error waiting until CloudFront Distribution (%s) is deployed: %s", d.Id(), err)
}

deleteDistributionInput.IfMatch = updateDistributionOutput.ETag

_, err = conn.DeleteDistribution(deleteDistributionInput)

// CloudFront has eventual consistency issues even for "deployed" state.
// Occasionally the DeleteDistribution call will return this error as well, in which retries will succeed:
// * PreconditionFailed: The request failed because it didn't meet the preconditions in one or more request-header fields
if isAWSErr(err, cloudfront.ErrCodeDistributionNotDisabled, "") || isAWSErr(err, cloudfront.ErrCodePreconditionFailed, "") {
err = resource.Retry(2*time.Minute, func() *resource.RetryError {
_, err := conn.DeleteDistribution(deleteDistributionInput)

if isAWSErr(err, cloudfront.ErrCodeDistributionNotDisabled, "") {
return resource.RetryableError(err)
}

if isAWSErr(err, cloudfront.ErrCodeNoSuchDistribution, "") {
return nil
}

if isAWSErr(err, cloudfront.ErrCodePreconditionFailed, "") {
return resource.RetryableError(err)
}

if err != nil {
return resource.NonRetryableError(err)
}

return nil
})

// Propagate AWS Go SDK retried error, if any
if isResourceTimeoutError(err) {
_, err = conn.DeleteDistribution(deleteDistributionInput)
}
return resource.NonRetryableError(err)
}
}

if isAWSErr(err, cloudfront.ErrCodeNoSuchDistribution, "") {
return nil
})
}

if err != nil {
return fmt.Errorf("CloudFront Distribution %s cannot be deleted: %s", d.Id(), err)
}
Expand All @@ -859,7 +952,7 @@ func resourceAwsCloudFrontDistributionWaitUntilDeployed(id string, meta interfac
Refresh: resourceAwsCloudFrontWebDistributionStateRefreshFunc(id, meta),
Timeout: 70 * time.Minute,
MinTimeout: 15 * time.Second,
Delay: 10 * time.Minute,
Delay: 1 * time.Minute,
}

_, err := stateConf.WaitForState()
Expand Down
Loading

0 comments on commit 286f294

Please sign in to comment.