trying to fix delete on hard ghost bucket

scality · Jan 24, 2025 · bde1072 · bde1072
1 parent a6cb9bd
commit bde1072
Show file tree

Hide file tree

Showing 3 changed files with 23 additions and 1 deletion.
diff --git 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
@@ -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
@@ -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);
     });