Merge pull request #10352 from terraform-providers/b-aws_s3_bucket_ob…

…ject-version-id-regression

resource/aws_s3_bucket_object: Fix object deletion for non-versioned objects

aws/provider_test.go

@@ -470,8 +470,7 @@ func TestAccAWSProvider_Endpoints_Deprecated(t *testing.T) {
 			{
 				Config: testAccAWSProviderConfigEndpoints(endpointsDeprecated.String()),
 				Check: resource.ComposeTestCheckFunc(
-					testAccCheckAWSProviderEndpointsDeprecated(&providers),
-				),
+					testAccCheckAWSProviderEndpointsDeprecated(&providers)),
 			},
 		},
 	})
@@ -750,3 +749,21 @@ data "aws_arn" "test" {
 }
 `, region)
 }
+
+func testAccAssumeRoleARNPreCheck(t *testing.T) {
+	v := os.Getenv("TF_ACC_ASSUME_ROLE_ARN")
+	if v == "" {
+		t.Skip("skipping tests; TF_ACC_ASSUME_ROLE_ARN must be set")
+	}
+}
+
+func testAccProviderConfigAssumeRolePolicy(policy string) string {
+	return fmt.Sprintf(`
+provider "aws" {
+	assume_role {
+		role_arn = %q
+		policy   = %q
+	}
+}
+`, os.Getenv("TF_ACC_ASSUME_ROLE_ARN"), policy)
+}

aws/resource_aws_s3_bucket_object.go

@@ -487,7 +487,14 @@ func resourceAwsS3BucketObjectDelete(d *schema.ResourceData, meta interface{}) e
 	// We are effectively ignoring any leading '/' in the key name as aws.Config.DisableRestProtocolURICleaning is false
 	key = strings.TrimPrefix(key, "/")
 
-	if err := deleteAllS3ObjectVersions(s3conn, bucket, key, d.Get("force_destroy").(bool), false); err != nil {
+	var err error
+	if _, ok := d.GetOk("version_id"); ok {
+		err = deleteAllS3ObjectVersions(s3conn, bucket, key, d.Get("force_destroy").(bool), false)
+	} else {
+		err = deleteS3ObjectVersion(s3conn, bucket, key, "", false)
+	}
+
+	if err != nil {
 		return fmt.Errorf("error deleting S3 Bucket (%s) Object (%s): %s", bucket, key, err)
 	}
 
@@ -656,10 +663,14 @@ func deleteAllS3ObjectVersions(conn *s3.S3, bucketName, key string, force, ignor
 // Set force to true to override any S3 object lock protections.
 func deleteS3ObjectVersion(conn *s3.S3, b, k, v string, force bool) error {
 	input := &s3.DeleteObjectInput{
-		Bucket:    aws.String(b),
-		Key:       aws.String(k),
-		VersionId: aws.String(v),
+		Bucket: aws.String(b),
+		Key:    aws.String(k),
 	}
+
+	if v != "" {
+		input.VersionId = aws.String(v)
+	}
+
 	if force {
 		input.BypassGovernanceRetention = aws.Bool(true)
 	}

aws/resource_aws_s3_bucket_object_test.go

@@ -262,6 +262,30 @@ func TestAccAWSS3BucketObject_withContentCharacteristics(t *testing.T) {
 	})
 }
 
+func TestAccAWSS3BucketObject_NonVersioned(t *testing.T) {
+	sourceInitial := testAccAWSS3BucketObjectCreateTempFile(t, "initial object state")
+	defer os.Remove(sourceInitial)
+
+	var originalObj s3.GetObjectOutput
+	resourceName := "aws_s3_bucket_object.object"
+
+	resource.ParallelTest(t, resource.TestCase{
+		PreCheck:     func() { testAccPreCheck(t); testAccAssumeRoleARNPreCheck(t) },
+		Providers:    testAccProviders,
+		CheckDestroy: testAccCheckAWSS3BucketObjectDestroy,
+		Steps: []resource.TestStep{
+			{
+				Config: testAccAWSS3BucketObjectConfig_NonVersioned(acctest.RandInt(), sourceInitial),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSS3BucketObjectExists(resourceName, &originalObj),
+					testAccCheckAWSS3BucketObjectBody(&originalObj, "initial object state"),
+					resource.TestCheckResourceAttr(resourceName, "version_id", ""),
+				),
+			},
+		},
+	})
+}
+
 func TestAccAWSS3BucketObject_updates(t *testing.T) {
 	var originalObj, modifiedObj s3.GetObjectOutput
 	resourceName := "aws_s3_bucket_object.object"
@@ -1447,3 +1471,38 @@ resource "aws_s3_bucket_object" "object" {
 }
 `, randInt, content, retainUntilDate)
 }
+
+func testAccAWSS3BucketObjectConfig_NonVersioned(randInt int, source string) string {
+	policy := `{
+		"Version": "2012-10-17",
+		"Statement": [
+			{
+				"Sid": "AllowYeah",
+				"Effect": "Allow",
+				"Action": "s3:*",
+				"Resource": "*"
+			},
+			{
+				"Sid":    "DenyStm1",
+				"Effect": "Deny",
+				"Action": [
+					"s3:GetObjectVersion*",
+					"s3:ListBucketVersions"
+				],
+				"Resource": "*"
+			}
+		]
+	}`
+	return testAccProviderConfigAssumeRolePolicy(policy) + fmt.Sprintf(`
+resource "aws_s3_bucket" "object_bucket_3" {
+  bucket = "tf-object-test-bucket-%d"
+}
+
+resource "aws_s3_bucket_object" "object" {
+  bucket = "${aws_s3_bucket.object_bucket_3.bucket}"
+  key    = "updateable-key"
+  source = "%s"
+  etag   = "${filemd5("%s")}"
+}
+`, randInt, source, source)
+}