r/aws_s3_bucket: Speed up deletion

[ewbankkit]

Community Note

• Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
• Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Closes #12146.
Merge after #23985.

The slowdown in aws_s3_bucket deletion, especially for S3 buckets containing a "large" number of objects when force_destroy is true, that was noticed in #12146 was caused by #9942.
Instead of deleting object versions in batches (of up to 1000), each object version was deleted in turn in order to handle the usually very rare case of the object having a legal hold lock in place.
This PR changes the bucket emptying logic to assume that no legal hold is in place and object versions are deleted in batches. In the case that the batch deletion has errors, each of those errors is inspected in turn and if necessary a legal hold removal is attempted (and then the object version deletion is retried).

Output from acceptance testing:

% make testacc TESTS=TestAccS3Bucket_Basic_ PKG=s3 ACCTEST_PARALLELISM=3 
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 3 -run='TestAccS3Bucket_Basic_'  -timeout 180m
=== RUN   TestAccS3Bucket_Basic_basic
=== PAUSE TestAccS3Bucket_Basic_basic
=== RUN   TestAccS3Bucket_Basic_emptyString
=== PAUSE TestAccS3Bucket_Basic_emptyString
=== RUN   TestAccS3Bucket_Basic_namePrefix
=== PAUSE TestAccS3Bucket_Basic_namePrefix
=== RUN   TestAccS3Bucket_Basic_generatedName
=== PAUSE TestAccS3Bucket_Basic_generatedName
=== RUN   TestAccS3Bucket_Basic_shouldFailNotFound
=== PAUSE TestAccS3Bucket_Basic_shouldFailNotFound
=== RUN   TestAccS3Bucket_Basic_forceDestroy
=== PAUSE TestAccS3Bucket_Basic_forceDestroy
=== RUN   TestAccS3Bucket_Basic_forceDestroyWithEmptyPrefixes
=== PAUSE TestAccS3Bucket_Basic_forceDestroyWithEmptyPrefixes
=== RUN   TestAccS3Bucket_Basic_forceDestroyWithObjectLockEnabled
=== PAUSE TestAccS3Bucket_Basic_forceDestroyWithObjectLockEnabled
=== CONT  TestAccS3Bucket_Basic_basic
=== CONT  TestAccS3Bucket_Basic_shouldFailNotFound
=== CONT  TestAccS3Bucket_Basic_namePrefix
--- PASS: TestAccS3Bucket_Basic_shouldFailNotFound (16.96s)
=== CONT  TestAccS3Bucket_Basic_forceDestroyWithObjectLockEnabled
--- PASS: TestAccS3Bucket_Basic_basic (24.43s)
=== CONT  TestAccS3Bucket_Basic_forceDestroyWithEmptyPrefixes
--- PASS: TestAccS3Bucket_Basic_namePrefix (24.45s)
=== CONT  TestAccS3Bucket_Basic_forceDestroy
--- PASS: TestAccS3Bucket_Basic_forceDestroyWithObjectLockEnabled (22.40s)
=== CONT  TestAccS3Bucket_Basic_emptyString
--- PASS: TestAccS3Bucket_Basic_forceDestroyWithEmptyPrefixes (19.53s)
=== CONT  TestAccS3Bucket_Basic_generatedName
--- PASS: TestAccS3Bucket_Basic_forceDestroy (20.18s)
--- PASS: TestAccS3Bucket_Basic_emptyString (21.50s)
--- PASS: TestAccS3Bucket_Basic_generatedName (21.37s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/s3	68.931s

% ./internal/service/s3/test-fixtures/populate_bucket.py ewbankkit-test-empty-bucket-001 -n 50 -l        
populating ewbankkit-test-empty-bucket-001 with 50 objects...
done!
% AWS_REGION=us-west-2 go test -v ./internal/service/s3 -run=TestEmptyBucket -b ewbankkit-test-empty-bucket-001 -f
=== RUN   TestEmptyBucket
    delete_test.go:31: 4051 S3 objects deleted
--- PASS: TestEmptyBucket (69.73s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/s3	75.424s
% ./internal/service/s3/test-fixtures/populate_bucket.py ewbankkit-test-empty-bucket-001 -n 50 -l        
populating ewbankkit-test-empty-bucket-001 with 50 objects...
done!
% AWS_REGION=us-west-2 go test -v ./internal/service/s3 -run=TestDeleteAllObjectVersions -b ewbankkit-test-empty-bucket-001 -f
=== RUN   TestDeleteAllObjectVersions
    delete_test.go:50: 3877 S3 objects deleted
--- PASS: TestDeleteAllObjectVersions (569.88s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/s3	577.162s

[ewbankkit]

% make testacc TESTARGS='-run=TestAccS3Bucket_Basic_\|TestAccS3Object_' PKG=s3 ACCTEST_PARALLELISM=3
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/s3/... -v -count 1 -parallel 3  -run=TestAccS3Bucket_Basic_\|TestAccS3Object_ -timeout 180m
go: downloading github.com/aws/aws-sdk-go v1.43.32
=== RUN   TestAccS3Bucket_Basic_basic
=== PAUSE TestAccS3Bucket_Basic_basic
=== RUN   TestAccS3Bucket_Basic_emptyString
=== PAUSE TestAccS3Bucket_Basic_emptyString
=== RUN   TestAccS3Bucket_Basic_generatedName
=== PAUSE TestAccS3Bucket_Basic_generatedName
=== RUN   TestAccS3Bucket_Basic_namePrefix
=== PAUSE TestAccS3Bucket_Basic_namePrefix
=== RUN   TestAccS3Bucket_Basic_forceDestroy
=== PAUSE TestAccS3Bucket_Basic_forceDestroy
=== RUN   TestAccS3Bucket_Basic_forceDestroyWithEmptyPrefixes
=== PAUSE TestAccS3Bucket_Basic_forceDestroyWithEmptyPrefixes
=== RUN   TestAccS3Bucket_Basic_forceDestroyWithObjectLockEnabled
=== PAUSE TestAccS3Bucket_Basic_forceDestroyWithObjectLockEnabled
=== RUN   TestAccS3Bucket_Basic_acceleration
=== PAUSE TestAccS3Bucket_Basic_acceleration
=== RUN   TestAccS3Bucket_Basic_keyEnabled
=== PAUSE TestAccS3Bucket_Basic_keyEnabled
=== RUN   TestAccS3Bucket_Basic_requestPayer
=== PAUSE TestAccS3Bucket_Basic_requestPayer
=== RUN   TestAccS3Object_noNameNoKey
=== PAUSE TestAccS3Object_noNameNoKey
=== RUN   TestAccS3Object_empty
=== PAUSE TestAccS3Object_empty
=== RUN   TestAccS3Object_source
=== PAUSE TestAccS3Object_source
=== RUN   TestAccS3Object_content
=== PAUSE TestAccS3Object_content
=== RUN   TestAccS3Object_etagEncryption
=== PAUSE TestAccS3Object_etagEncryption
=== RUN   TestAccS3Object_contentBase64
=== PAUSE TestAccS3Object_contentBase64
=== RUN   TestAccS3Object_sourceHashTrigger
=== PAUSE TestAccS3Object_sourceHashTrigger
=== RUN   TestAccS3Object_withContentCharacteristics
=== PAUSE TestAccS3Object_withContentCharacteristics
=== RUN   TestAccS3Object_nonVersioned
=== PAUSE TestAccS3Object_nonVersioned
=== RUN   TestAccS3Object_updates
=== PAUSE TestAccS3Object_updates
=== RUN   TestAccS3Object_updateSameFile
=== PAUSE TestAccS3Object_updateSameFile
=== RUN   TestAccS3Object_updatesWithVersioning
=== PAUSE TestAccS3Object_updatesWithVersioning
=== RUN   TestAccS3Object_updatesWithVersioningViaAccessPoint
=== PAUSE TestAccS3Object_updatesWithVersioningViaAccessPoint
=== RUN   TestAccS3Object_kms
=== PAUSE TestAccS3Object_kms
=== RUN   TestAccS3Object_sse
=== PAUSE TestAccS3Object_sse
=== RUN   TestAccS3Object_acl
=== PAUSE TestAccS3Object_acl
=== RUN   TestAccS3Object_metadata
=== PAUSE TestAccS3Object_metadata
=== RUN   TestAccS3Object_storageClass
=== PAUSE TestAccS3Object_storageClass
=== RUN   TestAccS3Object_tags
=== PAUSE TestAccS3Object_tags
=== RUN   TestAccS3Object_tagsLeadingSingleSlash
=== PAUSE TestAccS3Object_tagsLeadingSingleSlash
=== RUN   TestAccS3Object_tagsLeadingMultipleSlashes
=== PAUSE TestAccS3Object_tagsLeadingMultipleSlashes
=== RUN   TestAccS3Object_tagsMultipleSlashes
=== PAUSE TestAccS3Object_tagsMultipleSlashes
=== RUN   TestAccS3Object_objectLockLegalHoldStartWithNone
=== PAUSE TestAccS3Object_objectLockLegalHoldStartWithNone
=== RUN   TestAccS3Object_objectLockLegalHoldStartWithOn
=== PAUSE TestAccS3Object_objectLockLegalHoldStartWithOn
=== RUN   TestAccS3Object_objectLockRetentionStartWithNone
=== PAUSE TestAccS3Object_objectLockRetentionStartWithNone
=== RUN   TestAccS3Object_objectLockRetentionStartWithSet
=== PAUSE TestAccS3Object_objectLockRetentionStartWithSet
=== RUN   TestAccS3Object_objectBucketKeyEnabled
=== PAUSE TestAccS3Object_objectBucketKeyEnabled
=== RUN   TestAccS3Object_bucketBucketKeyEnabled
=== PAUSE TestAccS3Object_bucketBucketKeyEnabled
=== RUN   TestAccS3Object_defaultBucketSSE
=== PAUSE TestAccS3Object_defaultBucketSSE
=== RUN   TestAccS3Object_ignoreTags
=== PAUSE TestAccS3Object_ignoreTags
=== CONT  TestAccS3Bucket_Basic_basic
=== CONT  TestAccS3Object_updateSameFile
=== CONT  TestAccS3Object_noNameNoKey
--- PASS: TestAccS3Object_noNameNoKey (2.68s)
=== CONT  TestAccS3Object_updates
--- PASS: TestAccS3Bucket_Basic_basic (23.71s)
=== CONT  TestAccS3Object_nonVersioned
    acctest.go:1177: skipping test; environment variable TF_ACC_ASSUME_ROLE_ARN must be set. Usage: Amazon Resource Name (ARN) of existing IAM Role to assume for testing restricted permissions
--- SKIP: TestAccS3Object_nonVersioned (0.00s)
=== CONT  TestAccS3Object_withContentCharacteristics
--- PASS: TestAccS3Object_withContentCharacteristics (19.98s)
=== CONT  TestAccS3Object_sourceHashTrigger
--- PASS: TestAccS3Object_updateSameFile (46.46s)
=== CONT  TestAccS3Object_contentBase64
--- PASS: TestAccS3Object_updates (49.24s)
=== CONT  TestAccS3Object_etagEncryption
--- PASS: TestAccS3Object_contentBase64 (17.86s)
=== CONT  TestAccS3Object_content
--- PASS: TestAccS3Object_etagEncryption (20.53s)
=== CONT  TestAccS3Object_source
--- PASS: TestAccS3Object_sourceHashTrigger (36.81s)
=== CONT  TestAccS3Object_empty
--- PASS: TestAccS3Object_content (20.96s)
=== CONT  TestAccS3Bucket_Basic_namePrefix
--- PASS: TestAccS3Object_source (20.93s)
=== CONT  TestAccS3Bucket_Basic_forceDestroy
--- PASS: TestAccS3Object_empty (22.74s)
=== CONT  TestAccS3Bucket_Basic_keyEnabled
--- PASS: TestAccS3Bucket_Basic_namePrefix (22.13s)
=== CONT  TestAccS3Bucket_Basic_requestPayer
--- PASS: TestAccS3Bucket_Basic_keyEnabled (23.12s)
=== CONT  TestAccS3Bucket_Basic_acceleration
--- PASS: TestAccS3Bucket_Basic_forceDestroy (36.01s)
=== CONT  TestAccS3Bucket_Basic_generatedName
--- PASS: TestAccS3Bucket_Basic_generatedName (20.88s)
=== CONT  TestAccS3Bucket_Basic_forceDestroyWithEmptyPrefixes
--- PASS: TestAccS3Bucket_Basic_requestPayer (54.58s)
=== CONT  TestAccS3Bucket_Basic_forceDestroyWithObjectLockEnabled
--- PASS: TestAccS3Bucket_Basic_acceleration (41.04s)
=== CONT  TestAccS3Object_tagsLeadingMultipleSlashes
--- PASS: TestAccS3Bucket_Basic_forceDestroyWithEmptyPrefixes (18.03s)
=== CONT  TestAccS3Object_ignoreTags
--- PASS: TestAccS3Bucket_Basic_forceDestroyWithObjectLockEnabled (22.28s)
=== CONT  TestAccS3Object_defaultBucketSSE
--- PASS: TestAccS3Object_defaultBucketSSE (20.16s)
=== CONT  TestAccS3Bucket_Basic_emptyString
--- PASS: TestAccS3Object_ignoreTags (43.46s)
=== CONT  TestAccS3Object_bucketBucketKeyEnabled
--- PASS: TestAccS3Bucket_Basic_emptyString (20.72s)
=== CONT  TestAccS3Object_acl
--- PASS: TestAccS3Object_bucketBucketKeyEnabled (20.22s)
=== CONT  TestAccS3Object_tagsLeadingSingleSlash
--- PASS: TestAccS3Object_tagsLeadingMultipleSlashes (80.01s)
=== CONT  TestAccS3Object_tags
--- PASS: TestAccS3Object_acl (62.85s)
=== CONT  TestAccS3Object_storageClass
--- PASS: TestAccS3Object_tagsLeadingSingleSlash (81.75s)
=== CONT  TestAccS3Object_metadata
--- PASS: TestAccS3Object_tags (84.21s)
=== CONT  TestAccS3Object_objectBucketKeyEnabled
--- PASS: TestAccS3Object_objectBucketKeyEnabled (21.57s)
=== CONT  TestAccS3Object_objectLockRetentionStartWithSet
--- PASS: TestAccS3Object_metadata (53.16s)
=== CONT  TestAccS3Object_objectLockRetentionStartWithNone
--- PASS: TestAccS3Object_storageClass (84.26s)
=== CONT  TestAccS3Object_kms
--- PASS: TestAccS3Object_kms (21.73s)
=== CONT  TestAccS3Object_objectLockLegalHoldStartWithOn
--- PASS: TestAccS3Object_objectLockRetentionStartWithNone (60.13s)
=== CONT  TestAccS3Object_sse
--- PASS: TestAccS3Object_objectLockRetentionStartWithSet (78.32s)
=== CONT  TestAccS3Object_tagsMultipleSlashes
--- PASS: TestAccS3Object_objectLockLegalHoldStartWithOn (43.81s)
=== CONT  TestAccS3Object_updatesWithVersioningViaAccessPoint
--- PASS: TestAccS3Object_sse (20.25s)
=== CONT  TestAccS3Object_updatesWithVersioning
--- PASS: TestAccS3Object_updatesWithVersioningViaAccessPoint (46.27s)
=== CONT  TestAccS3Object_objectLockLegalHoldStartWithNone
--- PASS: TestAccS3Object_updatesWithVersioning (44.23s)
--- PASS: TestAccS3Object_tagsMultipleSlashes (78.16s)
--- PASS: TestAccS3Object_objectLockLegalHoldStartWithNone (59.94s)
PASS
ok  	github.com/hashicorp/terraform-provider-aws/internal/service/s3	548.087s

[github-actions]

This functionality has been released in v4.9.0 of the Terraform AWS Provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template. Thank you!

[github-actions]

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.