From bde1072e41d067848d9583a972180dac06291f6a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Meinnel?=
 <frederic.meinnel@scality.com>
Date: Thu, 23 Jan 2025 19:31:41 +0100
Subject: [PATCH] trying to fix delete on hard ghost bucket

---
 lib/api/apiUtils/bucket/bucketDeletion.js | 17 +++++++++++++++++
 lib/api/bucketDelete.js                   |  5 ++++-
 lib/metadata/metadataUtils.js             |  2 ++
 3 files changed, 23 insertions(+), 1 deletion(-)

diff --git a/lib/api/apiUtils/bucket/bucketDeletion.js b/lib/api/apiUtils/bucket/bucketDeletion.js
index 5b8225c136..212df8d4bf 100644
--- a/lib/api/apiUtils/bucket/bucketDeletion.js
+++ b/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') {
diff --git a/lib/api/bucketDelete.js b/lib/api/bucketDelete.js
index 0e3d32235f..620ace6a5b 100644
--- a/lib/api/bucketDelete.js
+++ b/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,
diff --git a/lib/metadata/metadataUtils.js b/lib/metadata/metadataUtils.js
index 1885aadf58..8269893715 100644
--- a/lib/metadata/metadataUtils.js
+++ b/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);
     });