diff --git a/internal/service/s3/bucket.go b/internal/service/s3/bucket.go index bc53a63aad6..7be25ae35fe 100644 --- a/internal/service/s3/bucket.go +++ b/internal/service/s3/bucket.go @@ -244,26 +244,22 @@ func ResourceBucket() *schema.Resource { "logging": { Type: schema.TypeSet, - Optional: true, + Computed: true, Elem: &schema.Resource{ Schema: map[string]*schema.Schema{ "target_bucket": { - Type: schema.TypeString, - Required: true, + Type: schema.TypeString, + Computed: true, + Deprecated: "Use the aws_s3_bucket_logging resource instead", }, "target_prefix": { - Type: schema.TypeString, - Optional: true, + Type: schema.TypeString, + Computed: true, + Deprecated: "Use the aws_s3_bucket_logging resource instead", }, }, }, - Set: func(v interface{}) int { - var buf bytes.Buffer - m := v.(map[string]interface{}) - buf.WriteString(fmt.Sprintf("%s-", m["target_bucket"])) - buf.WriteString(fmt.Sprintf("%s-", m["target_prefix"])) - return create.StringHashcode(buf.String()) - }, + Deprecated: "Use the aws_s3_bucket_logging resource instead", }, "lifecycle_rule": { @@ -805,12 +801,6 @@ func resourceBucketUpdate(d *schema.ResourceData, meta interface{}) error { } } - if d.HasChange("logging") { - if err := resourceBucketLoggingUpdate(conn, d); err != nil { - return err - } - } - if d.HasChange("lifecycle_rule") { if err := resourceBucketLifecycleUpdate(conn, d); err != nil { return err @@ -1109,7 +1099,7 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { d.Set("request_payer", payer.Payer) } - // Read the logging configuration + // Read the logging configuration if configured outside this resource loggingResponse, err := verify.RetryOnAWSCode(s3.ErrCodeNoSuchBucket, func() (interface{}, error) { return conn.GetBucketLogging(&s3.GetBucketLoggingInput{ Bucket: aws.String(d.Id()), @@ -1120,20 +1110,10 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { return fmt.Errorf("error getting S3 Bucket logging: %s", err) } - lcl := make([]map[string]interface{}, 0, 1) - if logging, ok := loggingResponse.(*s3.GetBucketLoggingOutput); ok && logging.LoggingEnabled != nil { - v := logging.LoggingEnabled - lc := make(map[string]interface{}) - if aws.StringValue(v.TargetBucket) != "" { - lc["target_bucket"] = aws.StringValue(v.TargetBucket) + if logging, ok := loggingResponse.(*s3.GetBucketLoggingOutput); ok { + if err := d.Set("logging", flattenBucketLoggingEnabled(logging.LoggingEnabled)); err != nil { + return fmt.Errorf("error setting logging: %s", err) } - if aws.StringValue(v.TargetPrefix) != "" { - lc["target_prefix"] = aws.StringValue(v.TargetPrefix) - } - lcl = append(lcl, lc) - } - if err := d.Set("logging", lcl); err != nil { - return fmt.Errorf("error setting logging: %s", err) } // Read the lifecycle configuration @@ -1868,41 +1848,6 @@ func resourceBucketVersioningUpdate(conn *s3.S3, bucket string, versioningConfig return nil } -func resourceBucketLoggingUpdate(conn *s3.S3, d *schema.ResourceData) error { - logging := d.Get("logging").(*schema.Set).List() - bucket := d.Get("bucket").(string) - loggingStatus := &s3.BucketLoggingStatus{} - - if len(logging) > 0 { - c := logging[0].(map[string]interface{}) - - loggingEnabled := &s3.LoggingEnabled{} - if val, ok := c["target_bucket"]; ok { - loggingEnabled.TargetBucket = aws.String(val.(string)) - } - if val, ok := c["target_prefix"]; ok { - loggingEnabled.TargetPrefix = aws.String(val.(string)) - } - - loggingStatus.LoggingEnabled = loggingEnabled - } - - i := &s3.PutBucketLoggingInput{ - Bucket: aws.String(bucket), - BucketLoggingStatus: loggingStatus, - } - log.Printf("[DEBUG] S3 put bucket logging: %#v", i) - - _, err := verify.RetryOnAWSCode(s3.ErrCodeNoSuchBucket, func() (interface{}, error) { - return conn.PutBucketLogging(i) - }) - if err != nil { - return fmt.Errorf("Error putting S3 logging: %s", err) - } - - return nil -} - func resourceBucketAccelerationUpdate(conn *s3.S3, d *schema.ResourceData) error { bucket := d.Get("bucket").(string) enableAcceleration := d.Get("acceleration_status").(string) @@ -2371,6 +2316,23 @@ func resourceBucketLifecycleUpdate(conn *s3.S3, d *schema.ResourceData) error { return nil } +func flattenBucketLoggingEnabled(loggingEnabled *s3.LoggingEnabled) []interface{} { + if loggingEnabled == nil { + return []interface{}{} + } + + m := make(map[string]interface{}) + + if loggingEnabled.TargetBucket != nil { + m["target_bucket"] = aws.StringValue(loggingEnabled.TargetBucket) + } + if loggingEnabled.TargetPrefix != nil { + m["target_prefix"] = aws.StringValue(loggingEnabled.TargetPrefix) + } + + return []interface{}{m} +} + func flattenServerSideEncryptionConfiguration(c *s3.ServerSideEncryptionConfiguration) []map[string]interface{} { var encryptionConfiguration []map[string]interface{} rules := make([]interface{}, 0, len(c.Rules)) diff --git a/internal/service/s3/bucket_test.go b/internal/service/s3/bucket_test.go index 227c16afbd3..4be71bd0ac2 100644 --- a/internal/service/s3/bucket_test.go +++ b/internal/service/s3/bucket_test.go @@ -1225,33 +1225,6 @@ func TestAccS3Bucket_Security_corsEmptyOrigin(t *testing.T) { }) } -func TestAccS3Bucket_Security_logging(t *testing.T) { - bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") - resourceName := "aws_s3_bucket.bucket" - - resource.ParallelTest(t, resource.TestCase{ - PreCheck: func() { acctest.PreCheck(t) }, - ErrorCheck: acctest.ErrorCheck(t, s3.EndpointsID), - Providers: acctest.Providers, - CheckDestroy: testAccCheckBucketDestroy, - Steps: []resource.TestStep{ - { - Config: testAccBucketWithLoggingConfig(bucketName), - Check: resource.ComposeTestCheckFunc( - testAccCheckBucketExists(resourceName), - testAccCheckBucketLogging(resourceName, "aws_s3_bucket.log_bucket", "log/"), - ), - }, - { - ResourceName: resourceName, - ImportState: true, - ImportStateVerify: true, - ImportStateVerifyIgnore: []string{"force_destroy", "acl"}, - }, - }, - }) -} - func TestAccS3Bucket_Manage_lifecycleBasic(t *testing.T) { bucketName := sdkacctest.RandomWithPrefix("tf-test-bucket") resourceName := "aws_s3_bucket.bucket" @@ -3278,49 +3251,6 @@ func testAccCheckRequestPayer(n, expectedPayer string) resource.TestCheckFunc { } } -func testAccCheckBucketLogging(n, b, p string) resource.TestCheckFunc { - return func(s *terraform.State) error { - rs := s.RootModule().Resources[n] - conn := acctest.Provider.Meta().(*conns.AWSClient).S3Conn - - out, err := conn.GetBucketLogging(&s3.GetBucketLoggingInput{ - Bucket: aws.String(rs.Primary.ID), - }) - - if err != nil { - return fmt.Errorf("GetBucketLogging error: %v", err) - } - - if out.LoggingEnabled == nil { - return fmt.Errorf("logging not enabled for bucket: %s", rs.Primary.ID) - } - - tb := s.RootModule().Resources[b] - - if v := out.LoggingEnabled.TargetBucket; v == nil { - if tb.Primary.ID != "" { - return fmt.Errorf("bad target bucket, found nil, expected: %s", tb.Primary.ID) - } - } else { - if *v != tb.Primary.ID { - return fmt.Errorf("bad target bucket, expected: %s, got %s", tb.Primary.ID, *v) - } - } - - if v := out.LoggingEnabled.TargetPrefix; v == nil { - if p != "" { - return fmt.Errorf("bad target prefix, found nil, expected: %s", p) - } - } else { - if *v != p { - return fmt.Errorf("bad target prefix, expected: %s, got %s", p, *v) - } - } - - return nil - } -} - func testAccCheckBucketReplicationRules(n string, rules []*s3.ReplicationRule) resource.TestCheckFunc { return func(s *terraform.State) error { rs := s.RootModule().Resources[n] @@ -3931,25 +3861,6 @@ resource "aws_s3_bucket" "bucket" { `, bucketName) } -func testAccBucketWithLoggingConfig(bucketName string) string { - return fmt.Sprintf(` -resource "aws_s3_bucket" "log_bucket" { - bucket = "%[1]s-log" - acl = "log-delivery-write" -} - -resource "aws_s3_bucket" "bucket" { - bucket = %[1]q - acl = "private" - - logging { - target_bucket = aws_s3_bucket.log_bucket.id - target_prefix = "log/" - } -} -`, bucketName) -} - func testAccBucketWithLifecycleConfig(bucketName string) string { return fmt.Sprintf(` resource "aws_s3_bucket" "bucket" { diff --git a/website/docs/r/s3_bucket.html.markdown b/website/docs/r/s3_bucket.html.markdown index 6e269c5c9b9..84ee04c6632 100644 --- a/website/docs/r/s3_bucket.html.markdown +++ b/website/docs/r/s3_bucket.html.markdown @@ -84,25 +84,6 @@ resource "aws_s3_bucket" "b" { } ``` -### Enable Logging - -```terraform -resource "aws_s3_bucket" "log_bucket" { - bucket = "my-tf-log-bucket" - acl = "log-delivery-write" -} - -resource "aws_s3_bucket" "b" { - bucket = "my-tf-test-bucket" - acl = "private" - - logging { - target_bucket = aws_s3_bucket.log_bucket.id - target_prefix = "log/" - } -} -``` - ### Using object lifecycle ```terraform @@ -362,7 +343,6 @@ The following arguments are supported: * `website` - (Optional) A website object (documented below). * `cors_rule` - (Optional) A rule of [Cross-Origin Resource Sharing](https://docs.aws.amazon.com/AmazonS3/latest/dev/cors.html) (documented below). * `versioning` - (Optional) A state of [versioning](https://docs.aws.amazon.com/AmazonS3/latest/dev/Versioning.html) (documented below) -* `logging` - (Optional) A settings of [bucket logging](https://docs.aws.amazon.com/AmazonS3/latest/UG/ManagingBucketLogging.html) (documented below). * `lifecycle_rule` - (Optional) A configuration of [object lifecycle management](http://docs.aws.amazon.com/AmazonS3/latest/dev/object-lifecycle-mgmt.html) (documented below). * `acceleration_status` - (Optional) Sets the accelerate configuration of an existing bucket. Can be `Enabled` or `Suspended`. * `request_payer` - (Optional) Specifies who should bear the cost of Amazon S3 data transfer. @@ -396,11 +376,6 @@ The `versioning` object supports the following: * `enabled` - (Optional) Enable versioning. Once you version-enable a bucket, it can never return to an unversioned state. You can, however, suspend versioning on that bucket. * `mfa_delete` - (Optional) Enable MFA delete for either `Change the versioning state of your bucket` or `Permanently delete an object version`. Default is `false`. This cannot be used to toggle this setting but is available to allow managed buckets to reflect the state in AWS -The `logging` object supports the following: - -* `target_bucket` - (Required) The name of the bucket that will receive the log objects. -* `target_prefix` - (Optional) To specify a key prefix for log objects. - The `lifecycle_rule` object supports the following: * `id` - (Optional) Unique identifier for the rule. Must be less than or equal to 255 characters in length. @@ -559,6 +534,9 @@ In addition to all arguments above, the following attributes are exported: * `bucket_domain_name` - The bucket domain name. Will be of format `bucketname.s3.amazonaws.com`. * `bucket_regional_domain_name` - The bucket region-specific domain name. The bucket domain name including the region name, please refer [here](https://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region) for format. Note: The AWS CloudFront allows specifying S3 region-specific endpoint when creating S3 origin, it will prevent [redirect issues](https://forums.aws.amazon.com/thread.jspa?threadID=216814) from CloudFront to S3 Origin URL. * `hosted_zone_id` - The [Route 53 Hosted Zone ID](https://docs.aws.amazon.com/general/latest/gr/rande.html#s3_website_region_endpoints) for this bucket's region. +* `logging` - The [logging parameters](https://docs.aws.amazon.com/AmazonS3/latest/UG/ManagingBucketLogging.html) for the bucket. + * `target_bucket` - The name of the bucket that receives the log objects. + * `target_prefix` - The prefix for all log object keys/ * `region` - The AWS region this bucket resides in. * `tags_all` - A map of tags assigned to the resource, including those inherited from the provider [`default_tags` configuration block](/docs/providers/aws/index.html#default_tags-configuration-block). * `website_endpoint` - The website endpoint, if the bucket is configured with a website. If not, this will be an empty string.