Improvement/s3 c 6207/investigate ghost buckets

ghost-bucket/go.mod

@@ -0,0 +1,28 @@
+module ghost-bucket
+
+go 1.21.4
+
+require (
+	github.com/aws/aws-sdk-go-v2/config v1.28.0
+	github.com/aws/aws-sdk-go-v2/service/s3 v1.66.0
+	golang.org/x/sync v0.8.0
+)
+
+require (
+	github.com/aws/aws-sdk-go-v2 v1.32.2 // indirect
+	github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.6 // indirect
+	github.com/aws/aws-sdk-go-v2/credentials v1.17.41 // indirect
+	github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.17 // indirect
+	github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.21 // indirect
+	github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.21 // indirect
+	github.com/aws/aws-sdk-go-v2/internal/ini v1.8.1 // indirect
+	github.com/aws/aws-sdk-go-v2/internal/v4a v1.3.21 // indirect
+	github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.0 // indirect
+	github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.4.2 // indirect
+	github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.2 // indirect
+	github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.18.2 // indirect
+	github.com/aws/aws-sdk-go-v2/service/sso v1.24.2 // indirect
+	github.com/aws/aws-sdk-go-v2/service/ssooidc v1.28.2 // indirect
+	github.com/aws/aws-sdk-go-v2/service/sts v1.32.2 // indirect
+	github.com/aws/smithy-go v1.22.0 // indirect
+)

ghost-bucket/go.sum

@@ -0,0 +1,38 @@
+github.com/aws/aws-sdk-go-v2 v1.32.2 h1:AkNLZEyYMLnx/Q/mSKkcMqwNFXMAvFto9bNsHqcTduI=
+github.com/aws/aws-sdk-go-v2 v1.32.2/go.mod h1:2SK5n0a2karNTv5tbP1SjsX0uhttou00v/HpXKM1ZUo=
+github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.6 h1:pT3hpW0cOHRJx8Y0DfJUEQuqPild8jRGmSFmBgvydr0=
+github.com/aws/aws-sdk-go-v2/aws/protocol/eventstream v1.6.6/go.mod h1:j/I2++U0xX+cr44QjHay4Cvxj6FUbnxrgmqN3H1jTZA=
+github.com/aws/aws-sdk-go-v2/config v1.28.0 h1:FosVYWcqEtWNxHn8gB/Vs6jOlNwSoyOCA/g/sxyySOQ=
+github.com/aws/aws-sdk-go-v2/config v1.28.0/go.mod h1:pYhbtvg1siOOg8h5an77rXle9tVG8T+BWLWAo7cOukc=
+github.com/aws/aws-sdk-go-v2/credentials v1.17.41 h1:7gXo+Axmp+R4Z+AK8YFQO0ZV3L0gizGINCOWxSLY9W8=
+github.com/aws/aws-sdk-go-v2/credentials v1.17.41/go.mod h1:u4Eb8d3394YLubphT4jLEwN1rLNq2wFOlT6OuxFwPzU=
+github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.17 h1:TMH3f/SCAWdNtXXVPPu5D6wrr4G5hI1rAxbcocKfC7Q=
+github.com/aws/aws-sdk-go-v2/feature/ec2/imds v1.16.17/go.mod h1:1ZRXLdTpzdJb9fwTMXiLipENRxkGMTn1sfKexGllQCw=
+github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.21 h1:UAsR3xA31QGf79WzpG/ixT9FZvQlh5HY1NRqSHBNOCk=
+github.com/aws/aws-sdk-go-v2/internal/configsources v1.3.21/go.mod h1:JNr43NFf5L9YaG3eKTm7HQzls9J+A9YYcGI5Quh1r2Y=
+github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.21 h1:6jZVETqmYCadGFvrYEQfC5fAQmlo80CeL5psbno6r0s=
+github.com/aws/aws-sdk-go-v2/internal/endpoints/v2 v2.6.21/go.mod h1:1SR0GbLlnN3QUmYaflZNiH1ql+1qrSiB2vwcJ+4UM60=
+github.com/aws/aws-sdk-go-v2/internal/ini v1.8.1 h1:VaRN3TlFdd6KxX1x3ILT5ynH6HvKgqdiXoTxAF4HQcQ=
+github.com/aws/aws-sdk-go-v2/internal/ini v1.8.1/go.mod h1:FbtygfRFze9usAadmnGJNc8KsP346kEe+y2/oyhGAGc=
+github.com/aws/aws-sdk-go-v2/internal/v4a v1.3.21 h1:7edmS3VOBDhK00b/MwGtGglCm7hhwNYnjJs/PgFdMQE=
+github.com/aws/aws-sdk-go-v2/internal/v4a v1.3.21/go.mod h1:Q9o5h4HoIWG8XfzxqiuK/CGUbepCJ8uTlaE3bAbxytQ=
+github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.0 h1:TToQNkvGguu209puTojY/ozlqy2d/SFNcoLIqTFi42g=
+github.com/aws/aws-sdk-go-v2/service/internal/accept-encoding v1.12.0/go.mod h1:0jp+ltwkf+SwG2fm/PKo8t4y8pJSgOCO4D8Lz3k0aHQ=
+github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.4.2 h1:4FMHqLfk0efmTqhXVRL5xYRqlEBNBiRI7N6w4jsEdd4=
+github.com/aws/aws-sdk-go-v2/service/internal/checksum v1.4.2/go.mod h1:LWoqeWlK9OZeJxsROW2RqrSPvQHKTpp69r/iDjwsSaw=
+github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.2 h1:s7NA1SOw8q/5c0wr8477yOPp0z+uBaXBnLE0XYb0POA=
+github.com/aws/aws-sdk-go-v2/service/internal/presigned-url v1.12.2/go.mod h1:fnjjWyAW/Pj5HYOxl9LJqWtEwS7W2qgcRLWP+uWbss0=
+github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.18.2 h1:t7iUP9+4wdc5lt3E41huP+GvQZJD38WLsgVp4iOtAjg=
+github.com/aws/aws-sdk-go-v2/service/internal/s3shared v1.18.2/go.mod h1:/niFCtmuQNxqx9v8WAPq5qh7EH25U4BF6tjoyq9bObM=
+github.com/aws/aws-sdk-go-v2/service/s3 v1.66.0 h1:xA6XhTF7PE89BCNHJbQi8VvPzcgMtmGC5dr8S8N7lHk=
+github.com/aws/aws-sdk-go-v2/service/s3 v1.66.0/go.mod h1:cB6oAuus7YXRZhWCc1wIwPywwZ1XwweNp2TVAEGYeB8=
+github.com/aws/aws-sdk-go-v2/service/sso v1.24.2 h1:bSYXVyUzoTHoKalBmwaZxs97HU9DWWI3ehHSAMa7xOk=
+github.com/aws/aws-sdk-go-v2/service/sso v1.24.2/go.mod h1:skMqY7JElusiOUjMJMOv1jJsP7YUg7DrhgqZZWuzu1U=
+github.com/aws/aws-sdk-go-v2/service/ssooidc v1.28.2 h1:AhmO1fHINP9vFYUE0LHzCWg/LfUWUF+zFPEcY9QXb7o=
+github.com/aws/aws-sdk-go-v2/service/ssooidc v1.28.2/go.mod h1:o8aQygT2+MVP0NaV6kbdE1YnnIM8RRVQzoeUH45GOdI=
+github.com/aws/aws-sdk-go-v2/service/sts v1.32.2 h1:CiS7i0+FUe+/YY1GvIBLLrR/XNGZ4CtM1Ll0XavNuVo=
+github.com/aws/aws-sdk-go-v2/service/sts v1.32.2/go.mod h1:HtaiBI8CjYoNVde8arShXb94UbQQi9L4EMr6D+xGBwo=
+github.com/aws/smithy-go v1.22.0 h1:uunKnWlcoL3zO7q+gG2Pk53joueEOsnNB28QdMsmiMM=
+github.com/aws/smithy-go v1.22.0/go.mod h1:irrKGvNn1InZwb2d7fkIRNucdfwR8R+Ts3wxYa/cJHg=
+golang.org/x/sync v0.8.0 h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ=
+golang.org/x/sync v0.8.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk=

ghost-bucket/main.go

@@ -0,0 +1,199 @@
+package main
+
+import (
+	"context"
+	"fmt"
+	"math/rand"
+	"os"
+	"sync/atomic"
+	"time"
+
+	"github.com/aws/aws-sdk-go-v2/aws"
+	"github.com/aws/aws-sdk-go-v2/config"
+	"github.com/aws/aws-sdk-go-v2/service/s3"
+	"golang.org/x/sync/errgroup"
+)
+
+/*
+Dirty script to generate ghost buckets. It performs a lot of CreateBucket and DeleteBucket operations in parallel, for a
+large number of buckets. After a while, it checks if the buckets are ghost buckets and prints a list. It then tries to
+recreate the ghost buckets and checks again. It prints a list of deep ghost buckets which are the buckets that remain in
+the ghost state after trying to recreate them.
+
+Exanple usage:
+AWS_ACCESS_KEY_ID=accessKey1 AWS_SECRET_ACCESS_KEY=verySecretKey1 S3_ENDPOINT_URL=127.0.0.1:8000 go run main.go
+
+This should work both locally and remotely.
+
+*/
+
+func main() {
+
+	nBucket := 500
+
+	// Create an s3 bucket with the name "my-bucket"
+	cfg, err := config.LoadDefaultConfig(context.TODO(),
+		config.WithRegion("us-east-1"),
+		config.WithBaseEndpoint("http://"+os.Getenv("S3_ENDPOINT_URL")+"/"),
+		config.WithRetryer(func() aws.Retryer {
+			return aws.NopRetryer{}
+		}),
+	)
+	if err != nil {
+		panic("configuration error, " + err.Error())
+	}
+
+	client := s3.NewFromConfig(cfg)
+	group, _ := errgroup.WithContext(context.Background())
+	group.SetLimit(400)
+
+	bucketNumber := atomic.Int32{}
+
+	// Generate random prefix
+	base := rand.Intn(4e9)
+
+	for i := 0; i < nBucket; i++ {
+		time.Sleep(time.Duration(rand.Intn(100)) * time.Millisecond)
+		group.Go(func() error {
+			bucketName := fmt.Sprintf("test-%010d-%010d", base, bucketNumber.Add(1))
+			hammerWithRequests(client, bucketName)
+			return nil
+
+		})
+	}
+
+	if err := group.Wait(); err != nil {
+		fmt.Println("error: ", err)
+	}
+
+	fmt.Println("waiting for ghost buckets to appear")
+	time.Sleep(30 * time.Second)
+	fmt.Println("checking for ghost buckets")
+	ghostBuckets := make([]bool, nBucket)
+
+	for i := 0; i < nBucket; i++ {
+		i := i
+		group.Go(func() error {
+			bucketName := fmt.Sprintf("test-%010d-%010d", base, i)
+			fmt.Println("checking bucket for ghostness: ", bucketName)
+			ghost, err := isGhostBucket(client, bucketName)
+			if err != nil {
+				return err
+			}
+			ghostBuckets[i] = ghost
+			return nil
+		})
+	}
+
+	if err := group.Wait(); err != nil {
+		fmt.Println("error: ", err)
+	}
+
+	for i, ghost := range ghostBuckets {
+		if ghost {
+			fmt.Printf("bucketName: %s  isGhostBucket: %t\n", fmt.Sprintf("test-%010d-%010d", base, i), ghost)
+		}
+	}
+
+	bucketNumber.Store(0)
+
+	group.SetLimit(100)
+	for i := 0; i < nBucket; i++ {
+		group.Go(func() error {
+			bucketN := bucketNumber.Add(1) - 1
+			if !ghostBuckets[bucketN] {
+				return nil
+			}
+			bucketName := fmt.Sprintf("test-%010d-%010d", base, bucketN)
+			fmt.Println("attempting to recreate bucket: ", bucketName)
+			for range make([]struct{}, 50) {
+				client.CreateBucket(context.TODO(), &s3.CreateBucketInput{
+					Bucket: &bucketName,
+				})
+				time.Sleep(100 * time.Millisecond)
+			}
+			return nil
+		})
+	}
+
+	group.Wait()
+
+	deepGhostBuckets := make([]bool, nBucket)
+
+	for i := 0; i < nBucket; i++ {
+		i := i
+		group.Go(func() error {
+			bucketName := fmt.Sprintf("test-%010d-%010d", base, i)
+			ghost, err := isGhostBucket(client, bucketName)
+			if err != nil {
+				return err
+			}
+			deepGhostBuckets[i] = ghost
+			return nil
+		})
+	}
+
+	if err := group.Wait(); err != nil {
+		fmt.Println("error: ", err)
+	}
+
+	for i, ghost := range deepGhostBuckets {
+		if ghost {
+			fmt.Printf("bucketName: %s  isDeepGhostBucket: %t\n", fmt.Sprintf("test-%010d-%010d", base, i), ghost)
+		}
+	}
+}
+
+func isGhostBucket(client *s3.Client, bucketName string) (bool, error) {
+
+	var existsInList, existsInHead bool
+
+	out, err := client.ListBuckets(context.Background(), &s3.ListBucketsInput{})
+	if err != nil {
+		return false, err
+	}
+	for _, b := range out.Buckets {
+		if *b.Name == bucketName {
+			existsInList = true
+		}
+	}
+	_, err = client.HeadBucket(context.Background(), &s3.HeadBucketInput{
+		Bucket: &bucketName,
+	})
+	existsInHead = err == nil
+
+	return existsInList && !existsInHead, nil
+}
+
+func hammerWithRequests(client *s3.Client, bucketName string) error {
+	client.CreateBucket(context.TODO(), &s3.CreateBucketInput{
+		Bucket: &bucketName,
+	})
+	group, _ := errgroup.WithContext(context.Background())
+	for i := 0; i < 100; i++ {
+		group.Go(func() error {
+			time.Sleep(time.Duration(rand.Intn(2000)) * time.Millisecond)
+			ctx, _ := context.WithDeadline(context.Background(), time.Now().Add(1000*time.Millisecond))
+			_, err := client.CreateBucket(ctx, &s3.CreateBucketInput{
+				Bucket: &bucketName,
+			})
+			if err != nil {
+				fmt.Println("error creating bucket", bucketName, ": ", err)
+			}
+			return nil
+		})
+		group.Go(func() error {
+			time.Sleep(time.Duration(rand.Intn(5000)) * time.Millisecond)
+			ctx, _ := context.WithDeadline(context.Background(), time.Now().Add(1000*time.Millisecond))
+			_, err := client.DeleteBucket(ctx, &s3.DeleteBucketInput{
+				Bucket: &bucketName,
+			})
+			if err != nil {
+				fmt.Println("error deleting bucket", bucketName, ": ", err)
+			}
+			return nil
+		})
+	}
+	group.Wait()
+	return nil
+}

lib/api/apiUtils/bucket/bucketDeletion.js

@@ -70,6 +70,10 @@ function deleteBucket(authInfo, bucketMD, bucketName, canonicalID, request, log,
             return metadata.listObject(bucketName, params, log,
                 (err, list) => {
                     if (err) {
+                        if (err.NoSuchBucket) {
+                            log.debug('metadata returned NoSuchBucket error: carrying on with the deletion');
+                            return next();
+                        }
                         log.error('error from metadata', { error: err });
                         return next(err);
                     }
@@ -115,6 +119,11 @@ function deleteBucket(authInfo, bucketMD, bucketName, canonicalID, request, log,
                 });
         },
         function addDeleteFlagStep(next) {
+            // If we originally got a NoSuchBucket error from metadata, we
+            // don't need to update the bucket metadata since they don't exist
+            if (bucketMD === undefined) {
+                return next();
+            }
             log.trace('adding deleted attribute to bucket attributes');
             // Remove transient flag if any so never have both transient
             // and deleted flags.
@@ -134,9 +143,17 @@ function deleteBucket(authInfo, bucketMD, bucketName, canonicalID, request, log,
         }
         return metadata.deleteBucket(bucketName, log, err => {
             log.trace('deleting bucket from metadata');
+            if (err && err.NoSuchBucket) {
+                log.debug('metadata returned NoSuchBucket error: deletion is effectively a success', err);
+                return cb();
+            }
             if (err) {
                 return cb(err);
             }
+
+            // TODO: For the NoSuchBucket bypass, we are not attempting to delete the KMS master key.
+            // Shoudn't this be another function in the waterfall?
+            // Will we need to handle the possibility that the key does not exist?
             const serverSideEncryption = bucketMD.getServerSideEncryption();
             if (serverSideEncryption &&
                 serverSideEncryption.algorithm === 'AES256') {

lib/api/bucketDelete.js

@@ -35,11 +35,14 @@ function bucketDelete(authInfo, request, log, cb) {
         (err, bucketMD) => {
             const corsHeaders = collectCorsHeaders(request.headers.origin,
                 request.method, bucketMD);
-            if (err) {
+            if (err && !err.NoSuchBucket) {
                 log.debug('error processing request',
                     { method: 'metadataValidateBucket', error: err });
                 return cb(err, corsHeaders);
             }
+            if (err && err.NoSuchBucket) {
+                log.debug('bucket not found in metadata: carrying on with deletion', err);
+            }
             log.trace('passed checks',
                 { method: 'metadataValidateBucket' });
             return deleteBucket(authInfo, bucketMD, bucketName,

lib/metadata/metadataUtils.js

@@ -260,6 +260,8 @@ function standardMetadataValidateBucket(params, actionImplicitDenies, log, callb
         if (err) {
             return callback(err);
         }
+        // In the NoSuchBucket case, we will not evaluate this, is this an issue?
+        // Don't we already evaluate IAM policies anyway?
         const validationError = validateBucket(bucket, params, log, actionImplicitDenies);
         return callback(validationError, bucket);
     });