Merge pull request #44838 from tabito-hara/b-aws_cloudfront_distribution-logging_config_include_cookies

[bugfix] aws_cloudfront_distribution: Allow setting `logging_config.include_cookies` without enabling V1 logging

Author: ewbankkit

.changelog/44838.txt

@@ -0,0 +1,11 @@
+```release-note:bug
+resource/aws_cloudfront_distribution: Fix inability to configure `logging_config.include_cookies` argument while keeping V1 logging disabled
+```
+
+```release-note:bug
+resource/aws_cloudfront_distribution: Change `logging_config.bucket` argument from `Required` to `Optional`
+```
+
+```release-note:enhancement
+resource/aws_cloudfront_distribution: Add `logging_v1_enabled` attribute
+```

internal/service/cloudfront/distribution.go

@@ -337,7 +337,7 @@ func resourceDistribution() *schema.Resource {
 					Schema: map[string]*schema.Schema{
 						names.AttrBucket: {
 							Type:     schema.TypeString,
-							Required: true,
+							Optional: true,
 						},
 						"include_cookies": {
 							Type:     schema.TypeBool,
@@ -352,6 +352,10 @@ func resourceDistribution() *schema.Resource {
 					},
 				},
 			},
+			"logging_v1_enabled": {
+				Type:     schema.TypeBool,
+				Computed: true,
+			},
 			"ordered_cache_behavior": {
 				Type:     schema.TypeList,
 				Optional: true,
@@ -995,11 +999,17 @@ func resourceDistributionRead(ctx context.Context, d *schema.ResourceData, meta
 	d.Set("in_progress_validation_batches", output.Distribution.InProgressInvalidationBatches)
 	d.Set("is_ipv6_enabled", distributionConfig.IsIPV6Enabled)
 	d.Set("last_modified_time", aws.String(output.Distribution.LastModifiedTime.String()))
-	if distributionConfig.Logging != nil && aws.ToBool(distributionConfig.Logging.Enabled) {
-		if err := d.Set("logging_config", flattenLoggingConfig(distributionConfig.Logging)); err != nil {
-			return sdkdiag.AppendErrorf(diags, "setting logging_config: %s", err)
+	if distributionConfig.Logging != nil {
+		d.Set("logging_v1_enabled", distributionConfig.Logging.Enabled)
+		if aws.ToBool(distributionConfig.Logging.Enabled) || aws.ToBool(distributionConfig.Logging.IncludeCookies) {
+			if err := d.Set("logging_config", flattenLoggingConfig(distributionConfig.Logging)); err != nil {
+				return sdkdiag.AppendErrorf(diags, "setting logging_config: %s", err)
+			}
+		} else {
+			d.Set("logging_config", []any{})
 		}
 	} else {
+		d.Set("logging_v1_enabled", false)
 		d.Set("logging_config", []any{})
 	}
 	if distributionConfig.CacheBehaviors != nil {
@@ -2651,8 +2661,13 @@ func expandLoggingConfig(tfMap map[string]any) *awstypes.LoggingConfig {
 	apiObject := &awstypes.LoggingConfig{}
 
 	if tfMap != nil {
-		apiObject.Bucket = aws.String(tfMap[names.AttrBucket].(string))
-		apiObject.Enabled = aws.Bool(true)
+		if v, ok := tfMap[names.AttrBucket]; ok && v.(string) != "" {
+			apiObject.Bucket = aws.String(v.(string))
+			apiObject.Enabled = aws.Bool(true)
+		} else {
+			apiObject.Bucket = aws.String("")
+			apiObject.Enabled = aws.Bool(false)
+		}
 		apiObject.IncludeCookies = aws.Bool(tfMap["include_cookies"].(bool))
 		apiObject.Prefix = aws.String(tfMap[names.AttrPrefix].(string))
 	} else {

internal/service/cloudfront/distribution_test.go

@@ -41,6 +41,7 @@ func TestAccCloudFrontDistribution_basic(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName, acctest.CtTagsPercent, "0"),
 					resource.TestCheckResourceAttr(resourceName, "origin.#", "1"),
 					resource.TestCheckResourceAttr(resourceName, "origin.0.response_completion_timeout", "0"),
+					resource.TestCheckResourceAttr(resourceName, "logging_v1_enabled", acctest.CtFalse),
 				),
 			},
 			{
@@ -153,6 +154,45 @@ func TestAccCloudFrontDistribution_s3Origin(t *testing.T) {
 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckDistributionExists(ctx, "aws_cloudfront_distribution.s3_distribution", &distribution),
 					resource.TestCheckResourceAttr("aws_cloudfront_distribution.s3_distribution", names.AttrHostedZoneID, "Z2FDTNDATAQYW2"),
+					resource.TestCheckResourceAttr("aws_cloudfront_distribution.s3_distribution", "logging_v1_enabled", acctest.CtTrue),
+				),
+			},
+			{
+				ResourceName:      "aws_cloudfront_distribution.s3_distribution",
+				ImportState:       true,
+				ImportStateVerify: true,
+				ImportStateVerifyIgnore: []string{
+					"retain_on_delete",
+					"wait_for_deployment",
+				},
+			},
+		},
+	})
+}
+
+func TestAccCloudFrontDistribution_includeCookieWhenV1loggingDisabled(t *testing.T) {
+	ctx := acctest.Context(t)
+	if testing.Short() {
+		t.Skip("skipping long-running test in short mode")
+	}
+
+	var distribution awstypes.Distribution
+	rName := sdkacctest.RandomWithPrefix(acctest.ResourcePrefix)
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:                 func() { acctest.PreCheck(ctx, t); acctest.PreCheckPartitionHasService(t, names.CloudFrontEndpointID) },
+		ErrorCheck:               acctest.ErrorCheck(t, names.CloudFrontServiceID),
+		ProtoV5ProviderFactories: acctest.ProtoV5ProviderFactories,
+		CheckDestroy:             testAccCheckDistributionDestroy(ctx),
+		Steps: []resource.TestStep{
+			{
+				Config: testAccDistributionConfig_includeCookiesWhenV1loggingDisabled(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckDistributionExists(ctx, "aws_cloudfront_distribution.s3_distribution", &distribution),
+					resource.TestCheckResourceAttr("aws_cloudfront_distribution.s3_distribution", names.AttrHostedZoneID, "Z2FDTNDATAQYW2"),
+					resource.TestCheckResourceAttr("aws_cloudfront_distribution.s3_distribution", "logging_config.#", "1"),
+					resource.TestCheckResourceAttr("aws_cloudfront_distribution.s3_distribution", "logging_config.0.include_cookies", acctest.CtTrue),
+					resource.TestCheckResourceAttr("aws_cloudfront_distribution.s3_distribution", "logging_v1_enabled", acctest.CtFalse),
 				),
 			},
 			{
@@ -1931,6 +1971,66 @@ resource "aws_cloudfront_distribution" "s3_distribution" {
 `, testAccDistributionRetainConfig()))
 }
 
+func testAccDistributionConfig_includeCookiesWhenV1loggingDisabled(rName string) string {
+	return acctest.ConfigCompose(
+		originBucket(rName),
+		logBucket(rName),
+		fmt.Sprintf(`
+resource "aws_cloudfront_distribution" "s3_distribution" {
+  depends_on = [
+    aws_s3_bucket_acl.s3_bucket_origin_acl,
+    aws_s3_bucket_acl.s3_bucket_logs_acl,
+  ]
+
+  origin {
+    domain_name = aws_s3_bucket.s3_bucket_origin.bucket_regional_domain_name
+    origin_id   = "myS3Origin"
+  }
+
+  enabled             = true
+  default_root_object = "index.html"
+
+  logging_config {
+    include_cookies = true
+  }
+
+  default_cache_behavior {
+    allowed_methods  = ["DELETE", "GET", "HEAD", "OPTIONS", "PATCH", "POST", "PUT"]
+    cached_methods   = ["GET", "HEAD"]
+    target_origin_id = "myS3Origin"
+
+    forwarded_values {
+      query_string = false
+
+      cookies {
+        forward = "none"
+      }
+    }
+
+    viewer_protocol_policy = "allow-all"
+    min_ttl                = 0
+    default_ttl            = 3600
+    max_ttl                = 86400
+  }
+
+  price_class = "PriceClass_200"
+
+  restrictions {
+    geo_restriction {
+      restriction_type = "whitelist"
+      locations        = ["US", "CA", "GB", "DE"]
+    }
+  }
+
+  viewer_certificate {
+    cloudfront_default_certificate = true
+  }
+
+  %[1]s
+}
+`, testAccDistributionRetainConfig()))
+}
+
 func testAccDistributionConfig_tags1(tagKey1, tagValue1 string) string {
 	return fmt.Sprintf(`
 resource "aws_cloudfront_distribution" "test" {

website/docs/r/cloudfront_distribution.html.markdown

@@ -526,9 +526,9 @@ argument should not be specified.
 
 #### Logging Config Arguments
 
-* `bucket` (Required) - Amazon S3 bucket to store the access logs in, for example, `myawslogbucket.s3.amazonaws.com`. The bucket must have correct ACL attached with "FULL_CONTROL" permission for "awslogsdelivery" account (Canonical ID: "c4c1ede66af53448b93c283ce9448c4ba468c9432aa01d700d3878632f77d2d0") for log transfer to work.
-* `include_cookies` (Optional) - Whether to include cookies in access logs (default: `false`).
-* `prefix` (Optional) - Prefix to the access log filenames for this distribution, for example, `myprefix/`.
+* `bucket` (Optional) - Amazon S3 bucket for V1 logging where access logs are stored, for example, `myawslogbucket.s3.amazonaws.com`. V1 logging is enabled when this argument is specified. The bucket must have correct ACL attached with "FULL_CONTROL" permission for "awslogsdelivery" account (Canonical ID: "c4c1ede66af53448b93c283ce9448c4ba468c9432aa01d700d3878632f77d2d0") for log transfer to work.
+* `include_cookies` (Optional) - Whether to include cookies in access logs (default: `false`). This argument applies to both V1 and V2 logging.
+* `prefix` (Optional) - Prefix added to the access log file names for V1 logging, for example, `myprefix/`. This argument is effective only when V1 logging is enabled.
 
 #### Origin Arguments
 
@@ -608,6 +608,7 @@ This resource exports the following attributes in addition to the arguments abov
 * `id` - Identifier for the distribution. For example: `EDFDVBD632BHDS5`.
 * `arn` - ARN for the distribution. For example: `arn:aws:cloudfront::123456789012:distribution/EDFDVBD632BHDS5`, where `123456789012` is your AWS account ID.
 * `caller_reference` - Internal value used by CloudFront to allow future updates to the distribution configuration.
+* `logging_v1_enabled` - Whether V1 logging is enabled for the distribution.
 * `status` - Current status of the distribution. `Deployed` if the distribution's information is fully propagated throughout the Amazon CloudFront system.
 * `tags_all` - Map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](https://registry.terraform.io/providers/hashicorp/aws/latest/docs#default_tags-configuration-block).
 * `trusted_key_groups` - List of nested attributes for active trusted key groups, if the distribution is set up to serve private content with signed URLs.