diff --git a/.changelog/24020.txt b/.changelog/24020.txt new file mode 100644 index 000000000000..729b3532de8b --- /dev/null +++ b/.changelog/24020.txt @@ -0,0 +1,3 @@ +```release-note:enhancement +resource/aws_s3_bucket: Speed up resource deletion, especially when the S3 buckets contains a large number of objects and `force_destroy` is `true` +``` diff --git a/internal/service/s3/bucket.go b/internal/service/s3/bucket.go index 1859f37df157..a6038389e360 100644 --- a/internal/service/s3/bucket.go +++ b/internal/service/s3/bucket.go @@ -19,6 +19,7 @@ import ( "github.com/aws/aws-sdk-go/service/s3" "github.com/aws/aws-sdk-go/service/s3/s3manager" "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" + "github.com/hashicorp/terraform-plugin-sdk/v2/diag" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/schema" "github.com/hashicorp/terraform-plugin-sdk/v2/helper/structure" @@ -32,10 +33,11 @@ import ( func ResourceBucket() *schema.Resource { return &schema.Resource{ - Create: resourceBucketCreate, - Read: resourceBucketRead, - Update: resourceBucketUpdate, - Delete: resourceBucketDelete, + Create: resourceBucketCreate, + Read: resourceBucketRead, + Update: resourceBucketUpdate, + DeleteWithoutTimeout: resourceBucketDelete, + Importer: &schema.ResourceImporter{ State: schema.ImportStatePassthrough, }, @@ -1404,11 +1406,11 @@ func resourceBucketRead(d *schema.ResourceData, meta interface{}) error { return nil } -func resourceBucketDelete(d *schema.ResourceData, meta interface{}) error { +func resourceBucketDelete(ctx context.Context, d *schema.ResourceData, meta interface{}) diag.Diagnostics { conn := meta.(*conns.AWSClient).S3Conn - log.Printf("[DEBUG] S3 Delete Bucket: %s", d.Id()) - _, err := conn.DeleteBucket(&s3.DeleteBucketInput{ + log.Printf("[DEBUG] Deleting S3 Bucket: %s", d.Id()) + _, err := conn.DeleteBucketWithContext(ctx, &s3.DeleteBucketInput{ Bucket: aws.String(d.Id()), }) @@ -1416,7 +1418,7 @@ func resourceBucketDelete(d *schema.ResourceData, meta interface{}) error { return nil } - if tfawserr.ErrCodeEquals(err, "BucketNotEmpty") { + if tfawserr.ErrCodeEquals(err, ErrCodeBucketNotEmpty) { if d.Get("force_destroy").(bool) { // Use a S3 service client that can handle multiple slashes in URIs. // While aws_s3_object resources cannot create these object @@ -1424,7 +1426,7 @@ func resourceBucketDelete(d *schema.ResourceData, meta interface{}) error { conn = meta.(*conns.AWSClient).S3ConnURICleaningDisabled // bucket may have things delete them - log.Printf("[DEBUG] S3 Bucket attempting to forceDestroy %+v", err) + log.Printf("[DEBUG] S3 Bucket attempting to forceDestroy %s", err) // Delete everything including locked objects. // Don't ignore any object errors or we could recurse infinitely. @@ -1433,19 +1435,20 @@ func resourceBucketDelete(d *schema.ResourceData, meta interface{}) error { if objectLockConfiguration != nil { objectLockEnabled = aws.StringValue(objectLockConfiguration.ObjectLockEnabled) == s3.ObjectLockEnabledEnabled } - err = DeleteAllObjectVersions(conn, d.Id(), "", objectLockEnabled, false) - if err != nil { - return fmt.Errorf("error S3 Bucket force_destroy: %s", err) + if n, err := EmptyBucket(ctx, conn, d.Id(), objectLockEnabled); err != nil { + return diag.Errorf("emptying S3 Bucket (%s): %s", d.Id(), err) + } else { + log.Printf("[DEBUG] Deleted %d S3 objects", n) } // this line recurses until all objects are deleted or an error is returned - return resourceBucketDelete(d, meta) + return resourceBucketDelete(ctx, d, meta) } } if err != nil { - return fmt.Errorf("error deleting S3 Bucket (%s): %s", d.Id(), err) + return diag.Errorf("deleting S3 Bucket (%s): %s", d.Id(), err) } return nil diff --git a/internal/service/s3/bucket_object.go b/internal/service/s3/bucket_object.go index c307c89c6a81..94317b7fc418 100644 --- a/internal/service/s3/bucket_object.go +++ b/internal/service/s3/bucket_object.go @@ -390,7 +390,7 @@ func resourceBucketObjectDelete(d *schema.ResourceData, meta interface{}) error var err error if _, ok := d.GetOk("version_id"); ok { - err = DeleteAllObjectVersions(conn, bucket, key, d.Get("force_destroy").(bool), false) + _, err = DeleteAllObjectVersions(conn, bucket, key, d.Get("force_destroy").(bool), false) } else { err = deleteS3ObjectVersion(conn, bucket, key, "", false) } diff --git a/internal/service/s3/delete.go b/internal/service/s3/delete.go new file mode 100644 index 000000000000..45a64269652e --- /dev/null +++ b/internal/service/s3/delete.go @@ -0,0 +1,240 @@ +package s3 + +import ( + "context" + "fmt" + + "github.com/aws/aws-sdk-go/aws" + "github.com/aws/aws-sdk-go/aws/awserr" + "github.com/aws/aws-sdk-go/service/s3" + "github.com/hashicorp/aws-sdk-go-base/v2/awsv1shim/v2/tfawserr" + multierror "github.com/hashicorp/go-multierror" +) + +// EmptyBucket empties the specified S3 bucket by deleting all object versions and delete markers. +// If `force` is `true` then S3 Object Lock governance mode restrictions are bypassed and +// an attempt is made to remove any S3 Object Lock legal holds. +// Returns the number of objects deleted. +func EmptyBucket(ctx context.Context, conn *s3.S3, bucket string, force bool) (int64, error) { + nObjects, err := forEachObjectVersionsPage(ctx, conn, bucket, func(ctx context.Context, conn *s3.S3, bucket string, page *s3.ListObjectVersionsOutput) (int64, error) { + return deletePageOfObjectVersions(ctx, conn, bucket, force, page) + }) + + if err != nil { + return nObjects, err + } + + n, err := forEachObjectVersionsPage(ctx, conn, bucket, deletePageOfDeleteMarkers) + nObjects += n + + if err != nil { + return nObjects, err + } + + return nObjects, nil +} + +// forEachObjectVersionsPage calls the specified function for each page returned from the S3 ListObjectVersionsPages API. +func forEachObjectVersionsPage(ctx context.Context, conn *s3.S3, bucket string, fn func(ctx context.Context, conn *s3.S3, bucket string, page *s3.ListObjectVersionsOutput) (int64, error)) (int64, error) { + var nObjects int64 + + input := &s3.ListObjectVersionsInput{ + Bucket: aws.String(bucket), + } + var lastErr error + + err := conn.ListObjectVersionsPagesWithContext(ctx, input, func(page *s3.ListObjectVersionsOutput, lastPage bool) bool { + if page == nil { + return !lastPage + } + + n, err := fn(ctx, conn, bucket, page) + nObjects += n + + if err != nil { + lastErr = err + + return false + } + + return !lastPage + }) + + if err != nil { + return nObjects, fmt.Errorf("listing S3 Bucket (%s) object versions: %w", bucket, err) + } + + if lastErr != nil { + return nObjects, lastErr + } + + return nObjects, nil +} + +// deletePageOfObjectVersions deletes a page (<= 1000) of S3 object versions. +// If `force` is `true` then S3 Object Lock governance mode restrictions are bypassed and +// an attempt is made to remove any S3 Object Lock legal holds. +// Returns the number of objects deleted. +func deletePageOfObjectVersions(ctx context.Context, conn *s3.S3, bucket string, force bool, page *s3.ListObjectVersionsOutput) (int64, error) { + var nObjects int64 + + toDelete := make([]*s3.ObjectIdentifier, 0, len(page.Versions)) + for _, v := range page.Versions { + toDelete = append(toDelete, &s3.ObjectIdentifier{ + Key: v.Key, + VersionId: v.VersionId, + }) + } + + if nObjects = int64(len(toDelete)); nObjects == 0 { + return nObjects, nil + } + + input := &s3.DeleteObjectsInput{ + Bucket: aws.String(bucket), + Delete: &s3.Delete{ + Objects: toDelete, + Quiet: aws.Bool(true), // Only report errors. + }, + } + + if force { + input.BypassGovernanceRetention = aws.Bool(true) + } + + output, err := conn.DeleteObjectsWithContext(ctx, input) + + if tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + return nObjects, nil + } + + if err != nil { + return nObjects, fmt.Errorf("deleting S3 Bucket (%s) objects: %w", bucket, err) + } + + nObjects -= int64(len(output.Errors)) + + var deleteErrs *multierror.Error + + for _, v := range output.Errors { + code := aws.StringValue(v.Code) + + if code == s3.ErrCodeNoSuchKey { + continue + } + + // Attempt to remove any legal hold on the object. + if force && code == ErrCodeAccessDenied { + key := aws.StringValue(v.Key) + versionID := aws.StringValue(v.VersionId) + + _, err := conn.PutObjectLegalHoldWithContext(ctx, &s3.PutObjectLegalHoldInput{ + Bucket: aws.String(bucket), + Key: aws.String(key), + VersionId: aws.String(versionID), + LegalHold: &s3.ObjectLockLegalHold{ + Status: aws.String(s3.ObjectLockLegalHoldStatusOff), + }, + }) + + if err != nil { + // Add the original error and the new error. + deleteErrs = multierror.Append(deleteErrs, newDeleteS3ObjectVersionError(v)) + deleteErrs = multierror.Append(deleteErrs, fmt.Errorf("removing legal hold: %w", newS3ObjectVersionError(key, versionID, err))) + } else { + // Attempt to delete the object once the legal hold has been removed. + _, err := conn.DeleteObjectWithContext(ctx, &s3.DeleteObjectInput{ + Bucket: aws.String(bucket), + Key: aws.String(key), + VersionId: aws.String(versionID), + }) + + if err != nil { + deleteErrs = multierror.Append(deleteErrs, fmt.Errorf("deleting: %w", newS3ObjectVersionError(key, versionID, err))) + } else { + nObjects++ + } + } + } else { + deleteErrs = multierror.Append(deleteErrs, newDeleteS3ObjectVersionError(v)) + } + } + + if err := deleteErrs.ErrorOrNil(); err != nil { + return nObjects, fmt.Errorf("deleting S3 Bucket (%s) objects: %w", bucket, err) + } + + return nObjects, nil +} + +// deletePageOfDeleteMarkers deletes a page (<= 1000) of S3 object delete markers. +// Returns the number of delete markers deleted. +func deletePageOfDeleteMarkers(ctx context.Context, conn *s3.S3, bucket string, page *s3.ListObjectVersionsOutput) (int64, error) { + var nObjects int64 + + toDelete := make([]*s3.ObjectIdentifier, 0, len(page.Versions)) + for _, v := range page.DeleteMarkers { + toDelete = append(toDelete, &s3.ObjectIdentifier{ + Key: v.Key, + VersionId: v.VersionId, + }) + } + + if nObjects = int64(len(toDelete)); nObjects == 0 { + return nObjects, nil + } + + input := &s3.DeleteObjectsInput{ + Bucket: aws.String(bucket), + Delete: &s3.Delete{ + Objects: toDelete, + Quiet: aws.Bool(true), // Only report errors. + }, + } + + output, err := conn.DeleteObjectsWithContext(ctx, input) + + if tfawserr.ErrCodeEquals(err, s3.ErrCodeNoSuchBucket) { + return nObjects, nil + } + + if err != nil { + return nObjects, fmt.Errorf("deleting S3 Bucket (%s) delete markers: %w", bucket, err) + } + + nObjects -= int64(len(output.Errors)) + + var deleteErrs *multierror.Error + + for _, v := range output.Errors { + deleteErrs = multierror.Append(deleteErrs, newDeleteS3ObjectVersionError(v)) + } + + if err := deleteErrs.ErrorOrNil(); err != nil { + return nObjects, fmt.Errorf("deleting S3 Bucket (%s) delete markers: %w", bucket, err) + } + + return nObjects, nil +} + +func newS3ObjectVersionError(key, versionID string, err error) error { + if err == nil { + return nil + } + + if versionID == "" { + return fmt.Errorf("S3 object (%s): %w", key, err) + } + + return fmt.Errorf("S3 object (%s) version (%s): %w", key, versionID, err) +} + +func newDeleteS3ObjectVersionError(err *s3.Error) error { + if err == nil { + return nil + } + + awsErr := awserr.New(aws.StringValue(err.Code), aws.StringValue(err.Message), nil) + + return fmt.Errorf("deleting: %w", newS3ObjectVersionError(aws.StringValue(err.Key), aws.StringValue(err.VersionId), awsErr)) +} diff --git a/internal/service/s3/delete_test.go b/internal/service/s3/delete_test.go new file mode 100644 index 000000000000..4b6f0d26d727 --- /dev/null +++ b/internal/service/s3/delete_test.go @@ -0,0 +1,51 @@ +package s3_test + +import ( + "context" + "flag" + "testing" + + "github.com/aws/aws-sdk-go/aws/session" + "github.com/aws/aws-sdk-go/service/s3" + tfs3 "github.com/hashicorp/terraform-provider-aws/internal/service/s3" +) + +// AWS_REGION=us-west-2 go test -v ./internal/service/s3 -run=TestEmptyBucket -b ewbankkit-test-empty-bucket-001 -f + +var bucket = flag.String("b", "", "bucket") +var force = flag.Bool("f", false, "force") + +func TestEmptyBucket(t *testing.T) { + if *bucket == "" { + t.Skip("bucket not specified") + } + + sess := session.Must(session.NewSession()) + svc := s3.New(sess) + ctx := context.Background() + + n, err := tfs3.EmptyBucket(ctx, svc, *bucket, *force) + + if err != nil { + t.Fatalf("error emptying S3 bucket (%s): %s", *bucket, err) + } + + t.Logf("%d S3 objects deleted", n) +} + +func TestDeleteAllObjectVersions(t *testing.T) { + if *bucket == "" { + t.Skip("bucket not specified") + } + + sess := session.Must(session.NewSession()) + svc := s3.New(sess) + + n, err := tfs3.DeleteAllObjectVersions(svc, *bucket, "", *force, false) + + if err != nil { + t.Fatalf("error emptying S3 bucket (%s): %s", *bucket, err) + } + + t.Logf("%d S3 objects deleted", n) +} diff --git a/internal/service/s3/errors.go b/internal/service/s3/errors.go index 40d27fd05bd3..482285b469d5 100644 --- a/internal/service/s3/errors.go +++ b/internal/service/s3/errors.go @@ -4,6 +4,8 @@ package s3 // https://docs.aws.amazon.com/sdk-for-go/api/service/s3/#pkg-constants const ( + ErrCodeAccessDenied = "AccessDenied" + ErrCodeBucketNotEmpty = "BucketNotEmpty" ErrCodeInvalidBucketState = "InvalidBucketState" ErrCodeInvalidRequest = "InvalidRequest" ErrCodeMalformedPolicy = "MalformedPolicy" diff --git a/internal/service/s3/object.go b/internal/service/s3/object.go index e7b99a81f755..284c8606a5d4 100644 --- a/internal/service/s3/object.go +++ b/internal/service/s3/object.go @@ -387,7 +387,7 @@ func resourceObjectDelete(d *schema.ResourceData, meta interface{}) error { var err error if _, ok := d.GetOk("version_id"); ok { - err = DeleteAllObjectVersions(conn, bucket, key, d.Get("force_destroy").(bool), false) + _, err = DeleteAllObjectVersions(conn, bucket, key, d.Get("force_destroy").(bool), false) } else { err = deleteS3ObjectVersion(conn, bucket, key, "", false) } @@ -614,7 +614,10 @@ func hasS3ObjectContentChanges(d verify.ResourceDiffer) bool { // DeleteAllObjectVersions deletes all versions of a specified key from an S3 bucket. // If key is empty then all versions of all objects are deleted. // Set force to true to override any S3 object lock protections on object lock enabled buckets. -func DeleteAllObjectVersions(conn *s3.S3, bucketName, key string, force, ignoreObjectErrors bool) error { +// Returns the number of objects deleted. +func DeleteAllObjectVersions(conn *s3.S3, bucketName, key string, force, ignoreObjectErrors bool) (int64, error) { + var nObjects int64 + input := &s3.ListObjectVersionsInput{ Bucket: aws.String(bucketName), } @@ -637,6 +640,11 @@ func DeleteAllObjectVersions(conn *s3.S3, bucketName, key string, force, ignoreO } err := deleteS3ObjectVersion(conn, bucketName, objectKey, objectVersionID, force) + + if err == nil { + nObjects++ + } + if tfawserr.ErrCodeEquals(err, "AccessDenied") && force { // Remove any legal hold. resp, err := conn.HeadObject(&s3.HeadObjectInput{ @@ -672,6 +680,8 @@ func DeleteAllObjectVersions(conn *s3.S3, bucketName, key string, force, ignoreO if err != nil { lastErr = err + } else { + nObjects++ } continue @@ -695,12 +705,12 @@ func DeleteAllObjectVersions(conn *s3.S3, bucketName, key string, force, ignoreO } if err != nil { - return err + return nObjects, err } if lastErr != nil { if !ignoreObjectErrors { - return fmt.Errorf("error deleting at least one object version, last error: %s", lastErr) + return nObjects, fmt.Errorf("error deleting at least one object version, last error: %s", lastErr) } lastErr = nil @@ -724,6 +734,8 @@ func DeleteAllObjectVersions(conn *s3.S3, bucketName, key string, force, ignoreO if err != nil { lastErr = err + } else { + nObjects++ } } @@ -735,18 +747,18 @@ func DeleteAllObjectVersions(conn *s3.S3, bucketName, key string, force, ignoreO } if err != nil { - return err + return nObjects, err } if lastErr != nil { if !ignoreObjectErrors { - return fmt.Errorf("error deleting at least one object delete marker, last error: %s", lastErr) + return nObjects, fmt.Errorf("error deleting at least one object delete marker, last error: %s", lastErr) } lastErr = nil } - return nil + return nObjects, nil } // deleteS3ObjectVersion deletes a specific object version. diff --git a/internal/service/s3/object_copy.go b/internal/service/s3/object_copy.go index 1fa6faf1164d..0a0d4107c686 100644 --- a/internal/service/s3/object_copy.go +++ b/internal/service/s3/object_copy.go @@ -462,7 +462,7 @@ func resourceObjectCopyDelete(d *schema.ResourceData, meta interface{}) error { var err error if _, ok := d.GetOk("version_id"); ok { - err = DeleteAllObjectVersions(conn, bucket, key, d.Get("force_destroy").(bool), false) + _, err = DeleteAllObjectVersions(conn, bucket, key, d.Get("force_destroy").(bool), false) } else { err = deleteS3ObjectVersion(conn, bucket, key, "", false) } diff --git a/internal/service/s3/sweep.go b/internal/service/s3/sweep.go index b6e711e55521..f044c89bed96 100644 --- a/internal/service/s3/sweep.go +++ b/internal/service/s3/sweep.go @@ -101,10 +101,10 @@ func sweepObjects(region string) error { } // Delete everything including locked objects. Ignore any object errors. - err = DeleteAllObjectVersions(conn, bucketName, "", objectLockEnabled, true) + _, err = DeleteAllObjectVersions(conn, bucketName, "", objectLockEnabled, true) if err != nil { - return fmt.Errorf("error listing S3 Bucket (%s) Objects: %s", bucketName, err) + return fmt.Errorf("error deleting S3 Bucket (%s) Objects: %s", bucketName, err) } } diff --git a/internal/service/s3/test-fixtures/populate_bucket.py b/internal/service/s3/test-fixtures/populate_bucket.py new file mode 100755 index 000000000000..b642284f1d2b --- /dev/null +++ b/internal/service/s3/test-fixtures/populate_bucket.py @@ -0,0 +1,64 @@ +#!/usr/bin/env python3 + +import argparse +import random +import boto3 + +from datetime import date, datetime, timedelta + +def main(argv): + """ + Simple Python script to populate an S3 bucket with a number of objects. + The S3 bucket must already exist and have versioning enabled. + AWS credentials must be set in the environment. + """ + p = argparse.ArgumentParser(description="script to populate an existing S3 Bucket with objects") + p.add_argument("bucket") + p.add_argument("-n", "--number-of-objects", action="store", default=100) + p.add_argument("-l", "--object-lock-enabled", action="store_true", default=False) + + args = p.parse_args(args=argv) + + populate_bucket(args.bucket, int(args.number_of_objects), bool(args.object_lock_enabled)) + +def populate_bucket(bucket, num_objects, obj_lock): + client = boto3.client('s3') + + print("populating %s with %d objects..." % (bucket, num_objects)) + + in_10_days = date.today() + timedelta(days=10) + retail_until = datetime(in_10_days.year, in_10_days.month, in_10_days.day) + + for i in range(num_objects): + key = "object-%d" % (i) + + # Each object has between 50 and 100 versions. + for j in range(random.randint(50, 100)): + contents = "data.%d" % (j) + args = { + "Bucket": bucket, + "Key": key, + "Body": contents.encode('utf-8'), + } + + if obj_lock: + # 5% chance of the object being locked in Governance mode. + chance = random.random() + if chance < 0.05: + args["ObjectLockMode"] = "GOVERNANCE" + args["ObjectLockRetainUntilDate"] = retail_until + # Or a 5% chance of having a legal hold in place. + elif chance < 0.10: + args["ObjectLockLegalHoldStatus"] = "ON" + + client.put_object(**args) + + # 10% chance of the object then being deleted. + if random.random() < 0.10: + client.delete_object(Bucket=bucket, Key=key) + + print("done!") + +if __name__ == '__main__': + import sys + main(sys.argv[1:]) \ No newline at end of file