From 229456b44f388d8ed4b4c0fe530cdb9ae5febafc Mon Sep 17 00:00:00 2001 From: Brian Flad Date: Thu, 28 Mar 2019 11:11:30 -0400 Subject: [PATCH] resource/aws_cloudfront_distribution: Add wait_for_deployment argument Reference: https://github.com/terraform-providers/terraform-provider-aws/issues/8073 Since we are adding a virtual attribute with a `Default`, we must use include a schema state migration to prevent the "" => "true" default difference on upgrade. Output from acceptance testing: ``` ``` --- aws/import_aws_cloudfront_distribution.go | 1 + aws/resource_aws_cloudfront_distribution.go | 23 +- ...rce_aws_cloudfront_distribution_migrate.go | 34 +++ ...ws_cloudfront_distribution_migrate_test.go | 52 ++++ ...source_aws_cloudfront_distribution_test.go | 254 ++++++++++++++---- 5 files changed, 310 insertions(+), 54 deletions(-) create mode 100644 aws/resource_aws_cloudfront_distribution_migrate.go create mode 100644 aws/resource_aws_cloudfront_distribution_migrate_test.go diff --git a/aws/import_aws_cloudfront_distribution.go b/aws/import_aws_cloudfront_distribution.go index acfc836dc5c0..9558deb4e216 100644 --- a/aws/import_aws_cloudfront_distribution.go +++ b/aws/import_aws_cloudfront_distribution.go @@ -10,6 +10,7 @@ func resourceAwsCloudFrontDistributionImport(d *schema.ResourceData, meta interf // This is a non API attribute // We are merely setting this to the same value as the Default setting in the schema d.Set("retain_on_delete", false) + d.Set("wait_for_deployment", true) conn := meta.(*AWSClient).cloudfrontconn id := d.Id() diff --git a/aws/resource_aws_cloudfront_distribution.go b/aws/resource_aws_cloudfront_distribution.go index 999ec2bb00d9..1ae9055d84d1 100644 --- a/aws/resource_aws_cloudfront_distribution.go +++ b/aws/resource_aws_cloudfront_distribution.go @@ -22,6 +22,8 @@ func resourceAwsCloudFrontDistribution() *schema.Resource { Importer: &schema.ResourceImporter{ State: resourceAwsCloudFrontDistributionImport, }, + MigrateState: resourceAwsCloudFrontDistributionMigrateState, + SchemaVersion: 1, Schema: map[string]*schema.Schema{ "arn": { @@ -714,6 +716,11 @@ func resourceAwsCloudFrontDistribution() *schema.Resource { Optional: true, Default: false, }, + "wait_for_deployment": { + Type: schema.TypeBool, + Optional: true, + Default: true, + }, "is_ipv6_enabled": { Type: schema.TypeBool, Optional: true, @@ -765,9 +772,11 @@ 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) + if d.Get("wait_for_deployment").(bool) { + 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) @@ -858,9 +867,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 d.Get("wait_for_deployment").(bool) { + 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 { diff --git a/aws/resource_aws_cloudfront_distribution_migrate.go b/aws/resource_aws_cloudfront_distribution_migrate.go new file mode 100644 index 000000000000..4517930bc1de --- /dev/null +++ b/aws/resource_aws_cloudfront_distribution_migrate.go @@ -0,0 +1,34 @@ +package aws + +import ( + "fmt" + "log" + + "github.com/hashicorp/terraform/terraform" +) + +func resourceAwsCloudFrontDistributionMigrateState(v int, is *terraform.InstanceState, meta interface{}) (*terraform.InstanceState, error) { + switch v { + case 0: + log.Println("[INFO] Found CloudFront Distribution state v0; migrating to v1") + return migrateCloudFrontDistributionStateV0toV1(is) + default: + return is, fmt.Errorf("Unexpected schema version: %d", v) + } +} + +func migrateCloudFrontDistributionStateV0toV1(is *terraform.InstanceState) (*terraform.InstanceState, error) { + if is.Empty() || is.Attributes == nil { + log.Println("[DEBUG] Empty CloudFront Distribution state; nothing to migrate.") + return is, nil + } + + log.Printf("[DEBUG] Attributes before migration: %#v", is.Attributes) + + // Add wait_for_deployment virtual attribute with Default + is.Attributes["wait_for_deployment"] = "true" + + log.Printf("[DEBUG] Attributes after migration: %#v", is.Attributes) + + return is, nil +} diff --git a/aws/resource_aws_cloudfront_distribution_migrate_test.go b/aws/resource_aws_cloudfront_distribution_migrate_test.go new file mode 100644 index 000000000000..a0554ff7dde8 --- /dev/null +++ b/aws/resource_aws_cloudfront_distribution_migrate_test.go @@ -0,0 +1,52 @@ +package aws + +import ( + "testing" + + "github.com/hashicorp/terraform/terraform" +) + +func TestAwsCloudFrontDistributionMigrateState(t *testing.T) { + cases := map[string]struct { + StateVersion int + Attributes map[string]string + Expected map[string]string + Meta interface{} + }{ + "v0_1": { + StateVersion: 0, + Attributes: map[string]string{ + "wait_for_deployment": "", + }, + Expected: map[string]string{ + "wait_for_deployment": "true", + }, + }, + } + + for tn, tc := range cases { + is := &terraform.InstanceState{ + ID: "some_id", + Attributes: tc.Attributes, + } + + tfResource := resourceAwsCloudFrontDistribution() + + if tfResource.MigrateState == nil { + t.Fatalf("bad: %s, err: missing MigrateState function in resource", tn) + } + + is, err := tfResource.MigrateState(tc.StateVersion, is, tc.Meta) + if err != nil { + t.Fatalf("bad: %s, err: %#v", tn, err) + } + + for k, v := range tc.Expected { + if is.Attributes[k] != v { + t.Fatalf( + "bad: %s\n\n expected: %#v -> %#v\n got: %#v -> %#v\n in: %#v", + tn, k, v, k, is.Attributes[k], is.Attributes) + } + } + } +} diff --git a/aws/resource_aws_cloudfront_distribution_test.go b/aws/resource_aws_cloudfront_distribution_test.go index beb5c30f596f..1552d2e5d71a 100644 --- a/aws/resource_aws_cloudfront_distribution_test.go +++ b/aws/resource_aws_cloudfront_distribution_test.go @@ -123,10 +123,13 @@ func TestAccAWSCloudFrontDistribution_S3Origin(t *testing.T) { ), }, { - ResourceName: "aws_cloudfront_distribution.s3_distribution", - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"retain_on_delete"}, + ResourceName: "aws_cloudfront_distribution.s3_distribution", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "retain_on_delete", + "wait_for_deployment", + }, }, }, }) @@ -156,10 +159,13 @@ func TestAccAWSCloudFrontDistribution_S3OriginWithTags(t *testing.T) { ), }, { - ResourceName: "aws_cloudfront_distribution.s3_distribution", - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"retain_on_delete"}, + ResourceName: "aws_cloudfront_distribution.s3_distribution", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "retain_on_delete", + "wait_for_deployment", + }, }, { Config: postConfig, @@ -194,10 +200,13 @@ func TestAccAWSCloudFrontDistribution_customOrigin(t *testing.T) { ), }, { - ResourceName: "aws_cloudfront_distribution.custom_distribution", - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"retain_on_delete"}, + ResourceName: "aws_cloudfront_distribution.custom_distribution", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "retain_on_delete", + "wait_for_deployment", + }, }, }, }) @@ -226,10 +235,13 @@ func TestAccAWSCloudFrontDistribution_multiOrigin(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"retain_on_delete"}, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "retain_on_delete", + "wait_for_deployment", + }, }, }, }) @@ -261,10 +273,13 @@ func TestAccAWSCloudFrontDistribution_orderedCacheBehavior(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"retain_on_delete"}, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "retain_on_delete", + "wait_for_deployment", + }, }, }, }) @@ -362,13 +377,17 @@ func TestAccAWSCloudFrontDistribution_noOptionalItemsConfig(t *testing.T) { resource.TestCheckResourceAttr(resourceName, "tags.%", "0"), resource.TestCheckResourceAttr(resourceName, "viewer_certificate.#", "1"), resource.TestCheckResourceAttr(resourceName, "viewer_certificate.0.cloudfront_default_certificate", "true"), + resource.TestCheckResourceAttr(resourceName, "wait_for_deployment", "true"), ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"retain_on_delete"}, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "retain_on_delete", + "wait_for_deployment", + }, }, }, }) @@ -394,10 +413,13 @@ func TestAccAWSCloudFrontDistribution_HTTP11Config(t *testing.T) { ), }, { - ResourceName: "aws_cloudfront_distribution.http_1_1", - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"retain_on_delete"}, + ResourceName: "aws_cloudfront_distribution.http_1_1", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "retain_on_delete", + "wait_for_deployment", + }, }, }, }) @@ -419,10 +441,13 @@ func TestAccAWSCloudFrontDistribution_IsIPV6EnabledConfig(t *testing.T) { ), }, { - ResourceName: "aws_cloudfront_distribution.is_ipv6_enabled", - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"retain_on_delete"}, + ResourceName: "aws_cloudfront_distribution.is_ipv6_enabled", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "retain_on_delete", + "wait_for_deployment", + }, }, }, }) @@ -442,10 +467,13 @@ func TestAccAWSCloudFrontDistribution_noCustomErrorResponseConfig(t *testing.T) ), }, { - ResourceName: "aws_cloudfront_distribution.no_custom_error_responses", - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"retain_on_delete"}, + ResourceName: "aws_cloudfront_distribution.no_custom_error_responses", + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "retain_on_delete", + "wait_for_deployment", + }, }, }, }) @@ -468,10 +496,13 @@ func TestAccAWSCloudFrontDistribution_Enabled(t *testing.T) { ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"retain_on_delete"}, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "retain_on_delete", + "wait_for_deployment", + }, }, { Config: testAccAWSCloudFrontDistributionConfigEnabled(true, false), @@ -535,10 +566,13 @@ func TestAccAWSCloudFrontDistribution_ViewerCertificate_AcmCertificateArn(t *tes ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"retain_on_delete"}, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "retain_on_delete", + "wait_for_deployment", + }, }, }, }) @@ -562,10 +596,60 @@ func TestAccAWSCloudFrontDistribution_ViewerCertificate_AcmCertificateArn_Confli ), }, { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"retain_on_delete"}, + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "retain_on_delete", + "wait_for_deployment", + }, + }, + }, + }) +} + +func TestAccAWSCloudFrontDistribution_WaitForDeployment(t *testing.T) { + var distribution cloudfront.Distribution + resourceName := "aws_cloudfront_distribution.test" + + resource.ParallelTest(t, resource.TestCase{ + PreCheck: func() { testAccPreCheck(t) }, + Providers: testAccProviders, + CheckDestroy: testAccCheckCloudFrontDistributionDestroy, + Steps: []resource.TestStep{ + { + Config: testAccAWSCloudFrontDistributionConfigWaitForDeployment(false, false), + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudFrontDistributionExists(resourceName, &distribution), + testAccCheckCloudFrontDistributionStatusInProgress(&distribution), + testAccCheckCloudFrontDistributionWaitForDeployment(&distribution), + resource.TestCheckResourceAttr(resourceName, "wait_for_deployment", "false"), + ), + }, + { + ResourceName: resourceName, + ImportState: true, + ImportStateVerify: true, + ImportStateVerifyIgnore: []string{ + "retain_on_delete", + "wait_for_deployment", + }, + }, + { + Config: testAccAWSCloudFrontDistributionConfigWaitForDeployment(true, false), + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudFrontDistributionExists(resourceName, &distribution), + testAccCheckCloudFrontDistributionStatusInProgress(&distribution), + resource.TestCheckResourceAttr(resourceName, "wait_for_deployment", "false"), + ), + }, + { + Config: testAccAWSCloudFrontDistributionConfigWaitForDeployment(false, true), + Check: resource.ComposeTestCheckFunc( + testAccCheckCloudFrontDistributionExists(resourceName, &distribution), + testAccCheckCloudFrontDistributionStatusDeployed(&distribution), + resource.TestCheckResourceAttr(resourceName, "wait_for_deployment", "true"), + ), }, }, }) @@ -655,6 +739,34 @@ func testAccCheckCloudFrontDistributionExistsAPIOnly(distribution *cloudfront.Di } } +func testAccCheckCloudFrontDistributionStatusDeployed(distribution *cloudfront.Distribution) resource.TestCheckFunc { + return func(s *terraform.State) error { + if distribution == nil { + return fmt.Errorf("CloudFront Distribution empty") + } + + if aws.StringValue(distribution.Status) != "Deployed" { + return fmt.Errorf("CloudFront Distribution (%s) status not Deployed: %s", aws.StringValue(distribution.Id), aws.StringValue(distribution.Status)) + } + + return nil + } +} + +func testAccCheckCloudFrontDistributionStatusInProgress(distribution *cloudfront.Distribution) resource.TestCheckFunc { + return func(s *terraform.State) error { + if distribution == nil { + return fmt.Errorf("CloudFront Distribution empty") + } + + if aws.StringValue(distribution.Status) != "InProgress" { + return fmt.Errorf("CloudFront Distribution (%s) status not InProgress: %s", aws.StringValue(distribution.Id), aws.StringValue(distribution.Status)) + } + + return nil + } +} + func testAccCheckCloudFrontDistributionDisabled(distribution *cloudfront.Distribution) resource.TestCheckFunc { return func(s *terraform.State) error { if distribution == nil || distribution.DistributionConfig == nil { @@ -1689,3 +1801,49 @@ resource "aws_cloudfront_distribution" "test" { } `, retainOnDelete) } + +func testAccAWSCloudFrontDistributionConfigWaitForDeployment(enabled, waitForDeployment bool) string { + return fmt.Sprintf(` +resource "aws_cloudfront_distribution" "test" { + enabled = %[1]t + wait_for_deployment = %[2]t + + default_cache_behavior { + allowed_methods = ["GET", "HEAD"] + cached_methods = ["GET", "HEAD"] + target_origin_id = "test" + viewer_protocol_policy = "allow-all" + + forwarded_values { + query_string = false + + cookies { + forward = "all" + } + } + } + + origin { + domain_name = "www.example.com" + origin_id = "test" + + custom_origin_config { + http_port = 80 + https_port = 443 + origin_protocol_policy = "https-only" + origin_ssl_protocols = ["TLSv1.2"] + } + } + + restrictions { + geo_restriction { + restriction_type = "none" + } + } + + viewer_certificate { + cloudfront_default_certificate = true + } +} +`, enabled, waitForDeployment) +}