From 4130406799b0b597f00d444b91478ac90a673717 Mon Sep 17 00:00:00 2001 From: sylvain senechal Date: Tue, 29 Sep 2026 18:53:23 +0200 Subject: [PATCH 1/2] Count non-localized versions when deleting a bucket Issue: CLDSRV-1014 --- lib/api/apiUtils/bucket/bucketDeletion.js | 2 +- .../backbeat/cleanReadLocalization.js | 32 +++++++++++++++++-- tests/unit/api/bucketDelete.js | 15 +++++++++ 3 files changed, 45 insertions(+), 4 deletions(-) diff --git a/lib/api/apiUtils/bucket/bucketDeletion.js b/lib/api/apiUtils/bucket/bucketDeletion.js index adc506cd38..11352fe1b9 100644 --- a/lib/api/apiUtils/bucket/bucketDeletion.js +++ b/lib/api/apiUtils/bucket/bucketDeletion.js @@ -72,7 +72,7 @@ function deleteBucket(authInfo, bucketMD, bucketName, canonicalID, request, log, return async.waterfall( [ function checkForObjectsStep(next) { - const params = { maxKeys: 1, listingType: 'DelimiterVersions' }; + const params = { maxKeys: 1, listingType: 'DelimiterVersions', hideNonLocalizedVersions: false }; // We list all the versions as we want to return BucketNotEmpty // error if there are any versions or delete markers in the bucket. // Works for non-versioned buckets as well since listing versions diff --git a/tests/functional/backbeat/cleanReadLocalization.js b/tests/functional/backbeat/cleanReadLocalization.js index 052ec88756..2490081046 100644 --- a/tests/functional/backbeat/cleanReadLocalization.js +++ b/tests/functional/backbeat/cleanReadLocalization.js @@ -5,6 +5,7 @@ const { createHash } = require('crypto'); const { v4: uuidv4 } = require('uuid'); const { CreateBucketCommand, + DeleteBucketCommand, PutBucketVersioningCommand, PutObjectCommand, GetObjectCommand, @@ -59,13 +60,13 @@ function buildMetadataBody(versionId, dataStoreName) { // the mongo-processor replicating a version, then the data mover merging it once // the data has been copied: same version id, the location rewritten to the local one -function writeVersion(key, versionId, dataStoreName) { - if (dataStoreName === SOURCE_LOCATION) { +function writeVersion(key, versionId, dataStoreName, bucket = TEST_BUCKET) { + if (dataStoreName === SOURCE_LOCATION && bucket === TEST_BUCKET) { replicatedVersions.push({ key, versionId }); } return backbeatClient.send( new PutMetadataCommand({ - Bucket: TEST_BUCKET, + Bucket: bucket, Key: key, VersionId: encodeVersionId(versionId), Body: buildMetadataBody(versionId, dataStoreName), @@ -167,4 +168,29 @@ describeIfCleanRead('clean read: localizing a replicated version', function test const md = JSON.parse(res.Body); assert.strictEqual(md.dataStoreName, SOURCE_LOCATION); }); + + it('should not delete a bucket that only holds a replicated version', async () => { + const bucket = `${TEST_BUCKET}-delete`; + const key = 'clean-read-delete-bucket'; + const replicatedVersionId = generateVersionId(`${process.pid}`, 'RG001'); + await s3.send(new CreateBucketCommand({ Bucket: bucket })); + await s3.send( + new PutBucketVersioningCommand({ + Bucket: bucket, + VersioningConfiguration: { Status: 'Enabled' }, + }), + ); + await writeVersion(key, replicatedVersionId, SOURCE_LOCATION, bucket); + + // hidden from the clients, the version still holds the bucket + await assert.rejects(s3.send(new DeleteBucketCommand({ Bucket: bucket })), err => { + assert.strictEqual(err.name, 'BucketNotEmpty'); + return true; + }); + + // localized, the version is visible to the S3 API, which empties the bucket + await writeVersion(key, replicatedVersionId, LOCAL_LOCATION, bucket); + await bucketUtil.empty(bucket); + await bucketUtil.deleteOne(bucket); + }); }); diff --git a/tests/unit/api/bucketDelete.js b/tests/unit/api/bucketDelete.js index 0848c6263d..d91e5b9a89 100644 --- a/tests/unit/api/bucketDelete.js +++ b/tests/unit/api/bucketDelete.js @@ -169,6 +169,21 @@ describe('bucketDelete API', () => { }); }); + it('should count the non-localized versions when checking the bucket is empty', done => { + const listObject = sinon.spy(metadata, 'listObject'); + bucketPut(authInfo, testRequest, log, () => { + bucketDelete(authInfo, testRequest, log, () => { + const emptinessCheck = listObject + .getCalls() + .find(call => call.args[0] === bucketName && call.args[1].listingType === 'DelimiterVersions'); + listObject.restore(); + assert(emptinessCheck, 'the bucket versions should have been listed'); + assert.strictEqual(emptinessCheck.args[1].hideNonLocalizedVersions, false); + done(); + }); + }); + }); + it('should delete a bucket even if the bucket has ongoing mpu', done => createMPU(testRequest, initiateRequest, false, done)); From a73039b9ae50f251bd8f47d6c5cede8d678a33c3 Mon Sep 17 00:00:00 2001 From: sylvain senechal Date: Wed, 30 Sep 2026 12:08:25 +0200 Subject: [PATCH 2/2] promisify bucket put and delete in test Issue: CLDSRV-1014 --- tests/unit/api/bucketDelete.js | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/tests/unit/api/bucketDelete.js b/tests/unit/api/bucketDelete.js index d91e5b9a89..e8d52bb385 100644 --- a/tests/unit/api/bucketDelete.js +++ b/tests/unit/api/bucketDelete.js @@ -1,5 +1,6 @@ const crypto = require('crypto'); const assert = require('assert'); +const { promisify } = require('util'); const async = require('async'); const { parseString } = require('xml2js'); const { errors } = require('@scality/arsenal'); @@ -19,6 +20,9 @@ const objectPutPart = require('../../../lib/api/objectPutPart'); const { cleanup, DummyRequestLogger, makeAuthInfo } = require('../helpers'); const DummyRequest = require('../DummyRequest'); +const bucketPutAsync = promisify(bucketPut); +const bucketDeleteAsync = promisify(bucketDelete); + const log = new DummyRequestLogger(); const canonicalID = 'accessKey1'; const authInfo = makeAuthInfo(canonicalID); @@ -169,19 +173,19 @@ describe('bucketDelete API', () => { }); }); - it('should count the non-localized versions when checking the bucket is empty', done => { + it('should count the non-localized versions when checking the bucket is empty', async () => { const listObject = sinon.spy(metadata, 'listObject'); - bucketPut(authInfo, testRequest, log, () => { - bucketDelete(authInfo, testRequest, log, () => { - const emptinessCheck = listObject - .getCalls() - .find(call => call.args[0] === bucketName && call.args[1].listingType === 'DelimiterVersions'); - listObject.restore(); - assert(emptinessCheck, 'the bucket versions should have been listed'); - assert.strictEqual(emptinessCheck.args[1].hideNonLocalizedVersions, false); - done(); - }); - }); + try { + await bucketPutAsync(authInfo, testRequest, log); + await bucketDeleteAsync(authInfo, testRequest, log); + const emptinessCheck = listObject + .getCalls() + .find(call => call.args[0] === bucketName && call.args[1].listingType === 'DelimiterVersions'); + assert(emptinessCheck, 'the bucket versions should have been listed'); + assert.strictEqual(emptinessCheck.args[1].hideNonLocalizedVersions, false); + } finally { + listObject.restore(); + } }); it('should delete a bucket even if the bucket has ongoing mpu', done =>