Skip to content

Commit 23add37

Browse files
authored
Merge pull request #8299 from nadavMiz/delete-bucket-versioning
NSFS | NC | versioning - don't delete bucket with delete marker on top of versioned-object
2 parents 1c8a75f + 1772148 commit 23add37

File tree

3 files changed

+87
-2
lines changed

3 files changed

+87
-2
lines changed

src/sdk/bucketspace_fs.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -341,7 +341,11 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
341341
// 2. delete only bucket tmpdir
342342
let list;
343343
try {
344-
list = await ns.list_objects({ ...params, bucket: name, limit: 1 }, object_sdk);
344+
if (ns._is_versioning_disabled()) {
345+
list = await ns.list_objects({ ...params, bucket: name, limit: 1 }, object_sdk);
346+
} else {
347+
list = await ns.list_object_versions({ ...params, bucket: name, limit: 1 }, object_sdk);
348+
}
345349
} catch (err) {
346350
dbg.warn('delete_bucket: bucket name', name, 'got an error while trying to list_objects', err);
347351
// in case the ULS was deleted - we will continue

src/sdk/namespace_fs.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2479,7 +2479,12 @@ class NamespaceFS {
24792479
dbg.log0('NamespaceFS: delete_uls fs_context:', fs_context, 'to_delete_dir_path: ', params.full_path);
24802480

24812481
try {
2482-
const list = await this.list_objects({ ...params, bucket: params.name, limit: 1 }, object_sdk);
2482+
let list;
2483+
if (this._is_versioning_disabled()) {
2484+
list = await this.list_objects({ ...params, bucket: params.name, limit: 1 }, object_sdk);
2485+
} else {
2486+
list = await this.list_object_versions({ ...params, bucket: params.name, limit: 1 }, object_sdk);
2487+
}
24832488

24842489
if (list && list.objects && list.objects.length > 0) {
24852490
throw new RpcError('NOT_EMPTY', 'underlying directory has files in it');

src/test/unit_tests/test_bucketspace_fs.js

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,12 +26,16 @@ const { TMP_PATH, generate_s3_policy } = require('../system_tests/test_utils');
2626
const { CONFIG_SUBDIRS, JSON_SUFFIX } = require('../../sdk/config_fs');
2727
const nc_mkm = require('../../manage_nsfs/nc_master_key_manager').get_instance();
2828

29+
const XATTR_INTERNAL_NOOBAA_PREFIX = 'user.noobaa.';
30+
const XATTR_VERSION_ID = XATTR_INTERNAL_NOOBAA_PREFIX + 'version_id';
31+
const XATTR_DELETE_MARKER = XATTR_INTERNAL_NOOBAA_PREFIX + 'delete_marker';
2932

3033
const test_bucket = 'bucket1';
3134
const test_bucket2 = 'bucket2';
3235
const test_not_empty_bucket = 'notemptybucket';
3336
const test_bucket_temp_dir = 'buckettempdir';
3437
const test_bucket_invalid = 'bucket_invalid';
38+
const test_bucket_delete_marker = 'deletemarkerbucket';
3539
const test_bucket_iam_account = 'bucket-iam-account-can-access';
3640

3741
const tmp_fs_path = path.join(TMP_PATH, 'test_bucketspace_fs');
@@ -217,6 +221,39 @@ function make_dummy_object_sdk() {
217221
};
218222
}
219223

224+
function make_versioning_object_sdk() {
225+
const versioning_object_sdk = make_dummy_object_sdk();
226+
versioning_object_sdk.nsfs = {};
227+
versioning_object_sdk._get_bucket_namespace = name => {
228+
if (_.isUndefined(versioning_object_sdk.nsfs[name])) {
229+
const buck_path = path.join(new_buckets_path, name);
230+
versioning_object_sdk.nsfs[name] = new NamespaceFS({
231+
bucket_path: buck_path,
232+
bucket_id: '1',
233+
namespace_resource_id: undefined,
234+
access_mode: undefined,
235+
versioning: undefined,
236+
force_md5_etag: undefined,
237+
stats: undefined
238+
});
239+
}
240+
return versioning_object_sdk.nsfs[name];
241+
};
242+
versioning_object_sdk.read_bucket_full_info = async function(name) {
243+
const ns = this._get_bucket_namespace(name);
244+
const bucket = (await bucketspace_fs.read_bucket_sdk_info({ name }));
245+
if (name === test_bucket_temp_dir) {
246+
bucket.namespace.should_create_underlying_storage = false;
247+
} else {
248+
bucket.namespace.should_create_underlying_storage = true;
249+
}
250+
return {ns, bucket};
251+
};
252+
253+
return versioning_object_sdk;
254+
}
255+
256+
220257
// account_user2 (copied from the dummy sdk)
221258
// is the root account of account_iam_user1
222259
const account_iam_user1 = {
@@ -637,7 +674,34 @@ mocha.describe('bucketspace_fs', function() {
637674
await bucketspace_fs.delete_bucket(param, dummy_object_sdk);
638675
await fs_utils.file_must_not_exist(bucket_config_path);
639676
});
677+
678+
mocha.it('delete_bucket with delete marker', async function() {
679+
const versioning_sdk = make_versioning_object_sdk();
680+
const param = { name: test_bucket_delete_marker };
681+
await create_bucket(param.name);
682+
683+
await bucketspace_fs.set_bucket_versioning({ name: param.name, versioning: 'ENABLED' }, versioning_sdk);
684+
const version_dir = path.join(new_buckets_path, param.name, '.versions');
685+
await nb_native().fs.mkdir(ACCOUNT_FS_CONFIG, version_dir);
686+
687+
const versioned_path = path.join(version_dir, 'dummy_mtime-crkfjum9883k-ino-guu7');
688+
await create_versioned_object(versioned_path, Buffer.from(JSON.stringify("data")), 'mtime-crkfjum9883k-ino-guu7', false);
689+
690+
const delete_marker_path = path.join(version_dir, 'dummy_mtime-crkfjx1hui2o-ino-guu9');
691+
const delete_marker_obj = await create_versioned_object(delete_marker_path, Buffer.from(JSON.stringify("data")), 'mtime-crkfjx1hui2o-ino-guu9', true);
692+
const xattr_delete_marker = { [XATTR_DELETE_MARKER]: 'true' };
693+
delete_marker_obj.replacexattr(DEFAULT_FS_CONFIG, xattr_delete_marker);
694+
695+
try {
696+
await bucketspace_fs.delete_bucket(param, versioning_sdk);
697+
assert.fail('should have failed with NOT EMPTY');
698+
} catch (err) {
699+
assert.strictEqual(err.rpc_code, 'NOT_EMPTY');
700+
assert.equal(err.message, 'underlying directory has files in it');
701+
}
702+
});
640703
});
704+
641705
mocha.describe('set_bucket_versioning', function() {
642706
mocha.before(async function() {
643707
await create_bucket(test_bucket);
@@ -900,6 +964,18 @@ async function create_bucket(bucket_name) {
900964
assert.equal(stat1.nlink, 1);
901965
}
902966

967+
async function create_versioned_object(object_path, data, version_id, return_fd) {
968+
console.log(object_path);
969+
const target_file = await nb_native().fs.open(ACCOUNT_FS_CONFIG, object_path, 'w+');
970+
await fs.promises.writeFile(object_path, data);
971+
if (version_id !== 'null') {
972+
const xattr_version_id = { [XATTR_VERSION_ID]: `${version_id}` };
973+
await target_file.replacexattr(ACCOUNT_FS_CONFIG, xattr_version_id);
974+
}
975+
if (return_fd) return target_file;
976+
await target_file.close(ACCOUNT_FS_CONFIG);
977+
}
978+
903979

904980
function get_config_file_path(config_type_path, file_name) {
905981
return path.join(config_root, config_type_path, file_name + JSON_SUFFIX);

0 commit comments

Comments
 (0)