r/s3_bucket: read-only versioning argument (#22606)

* feat: deprecate 'versioning' argument

* Update CHANGELOG for #22606

* Update .changelog/22606.txt

* update s3 bucket object docs with versioning resource

* update additional s3 bucket object tests with versioning resource; update lifecycle config docs

* add instructions for breaking change introduced in #22606

* fix SSE refs in tests

.changelog/22606.txt

@@ -0,0 +1,3 @@
+```release-note:breaking-change
+resource/aws_s3_bucket: The `versioning` argument has been deprecated and is now read-only. Use the `aws_s3_bucket_versioning` resource instead.
+```

internal/service/apigateway/domain_name_test.go

@@ -641,9 +641,12 @@ resource "aws_s3_bucket" "test" {
   bucket = %[1]q
 
   force_destroy = true
+}
 
-  versioning {
-    enabled = true
+resource "aws_s3_bucket_versioning" "test" {
+  bucket = aws_s3_bucket.test.id
+  versioning_configuration {
+    status = "Enabled"
   }
 }
 

internal/service/apigatewayv2/domain_name_test.go

@@ -499,9 +499,12 @@ resource "aws_s3_bucket" "test" {
   bucket = %[1]q
 
   force_destroy = true
+}
 
-  versioning {
-    enabled = true
+resource "aws_s3_bucket_versioning" "test" {
+  bucket = aws_s3_bucket.test.id
+  versioning_configuration {
+    status = "Enabled"
   }
 }
 

internal/service/athena/database_test.go

@@ -363,18 +363,23 @@ resource "aws_kms_key" "hoge" {
 resource "aws_s3_bucket" "hoge" {
   bucket        = "tf-test-athena-db-%[1]d"
   force_destroy = true
+}
+
+resource "aws_s3_bucket_server_side_encryption_configuration" "test" {
+  bucket = aws_s3_bucket.hoge.id
 
-  server_side_encryption_configuration {
-    rule {
-      apply_server_side_encryption_by_default {
-        kms_master_key_id = aws_kms_key.hoge.arn
-        sse_algorithm     = "aws:kms"
-      }
+  rule {
+    apply_server_side_encryption_by_default {
+      kms_master_key_id = aws_kms_key.hoge.arn
+      sse_algorithm     = "aws:kms"
     }
   }
 }
 
 resource "aws_athena_database" "hoge" {
+  # Must have bucket SSE enabled first
+  depends_on = [aws_s3_bucket_server_side_encryption_configuration.test]
+
   name          = "%[2]s"
   bucket        = aws_s3_bucket.hoge.bucket
   force_destroy = %[3]t

internal/service/kafkaconnect/custom_plugin_test.go

@@ -260,13 +260,19 @@ func testAccCustomPluginConfigObjectVersion(name string) string {
 	return fmt.Sprintf(`
 resource "aws_s3_bucket" "test" {
   bucket = %[1]q
+}
 
-  versioning {
-    enabled = true
+resource "aws_s3_bucket_versioning" "test" {
+  bucket = aws_s3_bucket.test.id
+  versioning_configuration {
+    status = "Enabled"
   }
 }
 
 resource "aws_s3_object" "test" {
+  # Must have versioning enabled first
+  depends_on = [aws_s3_bucket_versioning.test]
+
   bucket = aws_s3_bucket.test.id
   key    = %[1]q
   source = "test-fixtures/mongodb-connector.jar"

internal/service/lambda/function_test.go

@@ -3416,13 +3416,19 @@ resource "aws_s3_bucket" "artifacts" {
   bucket        = "%s"
   acl           = "private"
   force_destroy = true
+}
 
-  versioning {
-    enabled = true
+resource "aws_s3_bucket_versioning" "artifacts" {
+  bucket = aws_s3_bucket.artifacts.id
+  versioning_configuration {
+    status = "Enabled"
   }
 }
 
 resource "aws_s3_object" "o" {
+  # Must have versioning enabled first
+  depends_on = [aws_s3_bucket_versioning.artifacts]
+
   bucket = aws_s3_bucket.artifacts.bucket
   key    = "%s"
   source = "%s"

internal/service/mwaa/environment_test.go

@@ -519,9 +519,12 @@ resource "aws_security_group" "test" {
 resource "aws_s3_bucket" "test" {
   bucket = %[1]q
   acl    = "private"
+}
 
-  versioning {
-    enabled = true
+resource "aws_s3_bucket_versioning" "test" {
+  bucket = aws_s3_bucket.test.id
+  versioning_configuration {
+    status = "Enabled"
   }
 }
 
@@ -533,6 +536,9 @@ resource "aws_s3_bucket_public_access_block" "test" {
 }
 
 resource "aws_s3_object" "dags" {
+  # Must have bucket versioning enabled first
+  depends_on = [aws_s3_bucket_versioning.test]
+
   bucket       = aws_s3_bucket.test.id
   acl          = "private"
   key          = "dags/"
@@ -579,6 +585,7 @@ resource "aws_iam_role_policy" "test" {
 POLICY
 }
 
+
 `, rName)
 }
 
@@ -795,6 +802,9 @@ resource "aws_mwaa_environment" "test" {
 }
 
 resource "aws_s3_object" "plugins" {
+  # Must have bucket versioning enabled first
+  depends_on = [aws_s3_bucket_versioning.test]
+
   bucket  = aws_s3_bucket.test.id
   acl     = "private"
   key     = "plugins.zip"

internal/service/s3/bucket.go

@@ -220,21 +220,20 @@ func ResourceBucket() *schema.Resource {
 			},
 
 			"versioning": {
-				Type:     schema.TypeList,
-				Optional: true,
-				Computed: true,
-				MaxItems: 1,
+				Type:       schema.TypeList,
+				Computed:   true,
+				Deprecated: "Use the aws_s3_bucket_versioning resource instead",
 				Elem: &schema.Resource{
 					Schema: map[string]*schema.Schema{
 						"enabled": {
-							Type:     schema.TypeBool,
-							Optional: true,
-							Default:  false,
+							Type:       schema.TypeBool,
+							Computed:   true,
+							Deprecated: "Use the aws_s3_bucket_versioning resource instead",
 						},
 						"mfa_delete": {
-							Type:     schema.TypeBool,
-							Optional: true,
-							Default:  false,
+							Type:       schema.TypeBool,
+							Computed:   true,
+							Deprecated: "Use the aws_s3_bucket_versioning resource instead",
 						},
 					},
 				},
@@ -751,23 +750,6 @@ func resourceBucketUpdate(d *schema.ResourceData, meta interface{}) error {
 		}
 	}
 
-	if d.HasChange("versioning") {
-		v := d.Get("versioning").([]interface{})
-
-		if d.IsNewResource() {
-			if versioning := expandVersioningWhenIsNewResource(v); versioning != nil {
-				err := resourceBucketInternalVersioningUpdate(conn, d.Id(), versioning)
-				if err != nil {
-					return err
-				}
-			}
-		} else {
-			if err := resourceBucketInternalVersioningUpdate(conn, d.Id(), expandVersioning(v)); err != nil {
-				return err
-			}
-		}
-	}
-
 	if d.HasChange("acl") && !d.IsNewResource() {
 		if err := resourceBucketACLUpdate(conn, d); err != nil {
 			return err
@@ -1374,23 +1356,6 @@ func resourceBucketACLUpdate(conn *s3.S3, d *schema.ResourceData) error {
 	return nil
 }
 
-func resourceBucketInternalVersioningUpdate(conn *s3.S3, bucket string, versioningConfig *s3.VersioningConfiguration) error {
-	input := &s3.PutBucketVersioningInput{
-		Bucket:                  aws.String(bucket),
-		VersioningConfiguration: versioningConfig,
-	}
-
-	_, err := verify.RetryOnAWSCode(s3.ErrCodeNoSuchBucket, func() (interface{}, error) {
-		return conn.PutBucketVersioning(input)
-	})
-
-	if err != nil {
-		return fmt.Errorf("error putting S3 versioning for bucket (%s): %w", bucket, err)
-	}
-
-	return nil
-}
-
 func resourceBucketInternalObjectLockConfigurationUpdate(conn *s3.S3, d *schema.ResourceData) error {
 	// S3 Object Lock configuration cannot be deleted, only updated.
 	req := &s3.PutObjectLockConfigurationInput{
@@ -2233,71 +2198,6 @@ func expandS3ObjectLockConfiguration(vConf []interface{}) *s3.ObjectLockConfigur
 
 // Versioning functions
 
-func expandVersioning(l []interface{}) *s3.VersioningConfiguration {
-	if len(l) == 0 || l[0] == nil {
-		return nil
-	}
-
-	tfMap, ok := l[0].(map[string]interface{})
-
-	if !ok {
-		return nil
-	}
-
-	output := &s3.VersioningConfiguration{}
-
-	if v, ok := tfMap["enabled"].(bool); ok {
-		if v {
-			output.Status = aws.String(s3.BucketVersioningStatusEnabled)
-		} else {
-			output.Status = aws.String(s3.BucketVersioningStatusSuspended)
-		}
-	}
-
-	if v, ok := tfMap["mfa_delete"].(bool); ok {
-		if v {
-			output.MFADelete = aws.String(s3.MFADeleteEnabled)
-		} else {
-			output.MFADelete = aws.String(s3.MFADeleteDisabled)
-		}
-	}
-
-	return output
-}
-
-func expandVersioningWhenIsNewResource(l []interface{}) *s3.VersioningConfiguration {
-	if len(l) == 0 || l[0] == nil {
-		return nil
-	}
-
-	tfMap, ok := l[0].(map[string]interface{})
-
-	if !ok {
-		return nil
-	}
-
-	output := &s3.VersioningConfiguration{}
-
-	// Only set and return a non-nil VersioningConfiguration with at least one of
-	// MFADelete or Status enabled as the PutBucketVersioning API request
-	// does not need to be made for new buckets that don't require versioning.
-	// Reference: https://github.com/hashicorp/terraform-provider-aws/issues/4494
-
-	if v, ok := tfMap["enabled"].(bool); ok && v {
-		output.Status = aws.String(s3.BucketVersioningStatusEnabled)
-	}
-
-	if v, ok := tfMap["mfa_delete"].(bool); ok && v {
-		output.MFADelete = aws.String(s3.MFADeleteEnabled)
-	}
-
-	if output.MFADelete == nil && output.Status == nil {
-		return nil
-	}
-
-	return output
-}
-
 func flattenVersioning(versioning *s3.GetBucketVersioningOutput) []interface{} {
 	if versioning == nil {
 		return []interface{}{}

internal/service/s3/bucket_object_data_source_test.go

@@ -548,13 +548,19 @@ func testAccBucketObjectDataSourceConfig_allParams(randInt int) string {
 	return fmt.Sprintf(`
 resource "aws_s3_bucket" "object_bucket" {
   bucket = "tf-object-test-bucket-%[1]d"
+}
 
-  versioning {
-    enabled = true
+resource "aws_s3_bucket_versioning" "test" {
+  bucket = aws_s3_bucket.object_bucket.id
+  versioning_configuration {
+    status = "Enabled"
   }
 }
 
 resource "aws_s3_object" "object" {
+  # Must have bucket versioning enabled first
+  depends_on = [aws_s3_bucket_versioning.test]
+
   bucket = aws_s3_bucket.object_bucket.bucket
   key    = "tf-testing-obj-%[1]d-all-params"
 
@@ -586,16 +592,22 @@ func testAccBucketObjectDataSourceConfig_objectLockLegalHoldOff(randInt int) str
 resource "aws_s3_bucket" "object_bucket" {
   bucket = "tf-object-test-bucket-%[1]d"
 
-  versioning {
-    enabled = true
-  }
-
   object_lock_configuration {
     object_lock_enabled = "Enabled"
   }
 }
 
+resource "aws_s3_bucket_versioning" "test" {
+  bucket = aws_s3_bucket.object_bucket.id
+  versioning_configuration {
+    status = "Enabled"
+  }
+}
+
 resource "aws_s3_object" "object" {
+  # Must have bucket versioning enabled first
+  depends_on = [aws_s3_bucket_versioning.test]
+
   bucket                        = aws_s3_bucket.object_bucket.bucket
   key                           = "tf-testing-obj-%[1]d"
   content                       = "Hello World"
@@ -614,16 +626,22 @@ func testAccBucketObjectDataSourceConfig_objectLockLegalHoldOn(randInt int, reta
 resource "aws_s3_bucket" "object_bucket" {
   bucket = "tf-object-test-bucket-%[1]d"
 
-  versioning {
-    enabled = true
-  }
-
   object_lock_configuration {
     object_lock_enabled = "Enabled"
   }
 }
 
+resource "aws_s3_bucket_versioning" "test" {
+  bucket = aws_s3_bucket.object_bucket.id
+  versioning_configuration {
+    status = "Enabled"
+  }
+}
+
 resource "aws_s3_object" "object" {
+  # Must have bucket versioning enabled first
+  depends_on = [aws_s3_bucket_versioning.test]
+
   bucket                        = aws_s3_bucket.object_bucket.bucket
   key                           = "tf-testing-obj-%[1]d"
   content                       = "Hello World"