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..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,6 +173,21 @@ describe('bucketDelete API', () => { }); }); + it('should count the non-localized versions when checking the bucket is empty', async () => { + const listObject = sinon.spy(metadata, 'listObject'); + 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 => createMPU(testRequest, initiateRequest, false, done));