diff --git a/docs/NooBaaNonContainerized/CI&Tests.md b/docs/NooBaaNonContainerized/CI&Tests.md index 12659befbc..b98d239be3 100644 --- a/docs/NooBaaNonContainerized/CI&Tests.md +++ b/docs/NooBaaNonContainerized/CI&Tests.md @@ -103,6 +103,7 @@ The following is a list of `NC jest tests` files - 8. `test_nc_nsfs_bucket_schema_validation.test.js` - Tests NC bucket schema validation. 9. `test_nc_nsfs_account_schema_validation.test.js` - Tests NC account schema validation. 10. `test_nc_nsfs_new_buckets_path_validation.test.js` - Tests new_buckets_path RW access. +11. `test_config_fs.test.js` - Tests ConfigFS methods. #### nc_index.js File * The `nc_index.js` is a file that runs several NC and NSFS mocha related tests. diff --git a/docs/NooBaaNonContainerized/GettingStarted.md b/docs/NooBaaNonContainerized/GettingStarted.md index 767ce22365..eafc437821 100644 --- a/docs/NooBaaNonContainerized/GettingStarted.md +++ b/docs/NooBaaNonContainerized/GettingStarted.md @@ -187,7 +187,7 @@ make_bucket: s3bucket #### 3. Check that the bucket configuration file was created successfully - ```sh sudo cat /etc/noobaa.conf.d/buckets/s3bucket.json -{"_id":"65cb1efcbec92b33220112d7","name":"s3bucket","owner_account":"65cb1e7c9e6ae40d499c0ae4","system_owner":"account1","bucket_owner":"account1","versioning":"DISABLED","creation_date":"2023-09-26T05:56:16.252Z","path":"/tmp/fs1/s3bucket","should_create_underlying_storage":true} +{"_id":"65cb1efcbec92b33220112d7","name":"s3bucket","owner_account":"65cb1e7c9e6ae40d499c0ae4","bucket_owner":"account1","versioning":"DISABLED","creation_date":"2023-09-26T05:56:16.252Z","path":"/tmp/fs1/s3bucket","should_create_underlying_storage":true} ``` #### 4. Check that the underlying file system bucket directory was created successfully - diff --git a/src/cmd/manage_nsfs.js b/src/cmd/manage_nsfs.js index 66d167a712..eff4848269 100644 --- a/src/cmd/manage_nsfs.js +++ b/src/cmd/manage_nsfs.js @@ -94,7 +94,6 @@ async function fetch_bucket_data(action, user_input) { _id: undefined, name: user_input.name === undefined ? undefined : String(user_input.name), owner_account: undefined, - system_owner: user_input.owner, // GAP - needs to be the system_owner (currently it is the account name) bucket_owner: user_input.owner, tag: undefined, // if we would add the option to tag a bucket using CLI, this should be changed versioning: action === ACTIONS.ADD ? 'DISABLED' : undefined, @@ -596,8 +595,9 @@ async function list_config_files(type, config_path, wide, show_secrets, filters) let config_files_list = await P.map_with_concurrency(10, entries, async entry => { if (entry.name.endsWith(JSON_SUFFIX)) { if (wide || should_filter) { - const full_path = path.join(config_path, entry.name); - const data = await config_fs.get_config_data(full_path, options); + const data = type === TYPES.ACCOUNT ? + await config_fs.get_account_by_name(entry.name, options) : + await config_fs.get_bucket_by_name(entry.name, options); if (!data) return undefined; if (should_filter && !filter_list_item(type, data, filters)) return undefined; // remove secrets on !show_secrets && should filter diff --git a/src/endpoint/s3/s3_rest.js b/src/endpoint/s3/s3_rest.js index af31709ed9..74d9e9c44b 100755 --- a/src/endpoint/s3/s3_rest.js +++ b/src/endpoint/s3/s3_rest.js @@ -230,7 +230,7 @@ async function authorize_request_policy(req) { const account = req.object_sdk.requesting_account; const account_identifier = req.object_sdk.nsfs_config_root ? account.name.unwrap() : account.email.unwrap(); - const is_system_owner = account_identifier === system_owner.unwrap(); + const is_system_owner = Boolean(system_owner) && system_owner.unwrap() === account_identifier; // @TODO: System owner as a construct should be removed - Temporary if (is_system_owner) return; diff --git a/src/sdk/bucketspace_fs.js b/src/sdk/bucketspace_fs.js index 7e428b2e59..aba1f295d1 100644 --- a/src/sdk/bucketspace_fs.js +++ b/src/sdk/bucketspace_fs.js @@ -121,7 +121,6 @@ class BucketSpaceFS extends BucketSpaceSimpleFS { }; bucket.name = new SensitiveString(bucket.name); - bucket.system_owner = new SensitiveString(bucket.system_owner); bucket.bucket_owner = new SensitiveString(bucket.bucket_owner); bucket.owner_account = { id: bucket.owner_account, @@ -291,7 +290,6 @@ class BucketSpaceFS extends BucketSpaceSimpleFS { tag: js_utils.default_value(tag, undefined), owner_account: account._id, creator: account._id, - system_owner: new SensitiveString(account.name), bucket_owner: new SensitiveString(account.name), versioning: config.NSFS_VERSIONING_ENABLED && lock_enabled ? 'ENABLED' : 'DISABLED', object_lock_configuration: config.WORM_ENABLED ? { @@ -675,7 +673,7 @@ class BucketSpaceFS extends BucketSpaceSimpleFS { const account_identifier = account.name.unwrap(); dbg.log1('has_bucket_action_permission:', bucket.name.unwrap(), account_identifier, bucket.bucket_owner.unwrap()); - const is_system_owner = account_identifier === bucket.system_owner.unwrap(); + const is_system_owner = Boolean(bucket.system_owner) && bucket.system_owner.unwrap() === account_identifier; // If the system owner account wants to access the bucket, allow it if (is_system_owner) return true; diff --git a/src/sdk/config_fs.js b/src/sdk/config_fs.js index db26365946..b59d32a026 100644 --- a/src/sdk/config_fs.js +++ b/src/sdk/config_fs.js @@ -398,6 +398,7 @@ class ConfigFS { async get_bucket_by_name(bucket_name, options = {}) { const bucket_path = this.get_bucket_path_by_name(bucket_name); const bucket = await this.get_config_data(bucket_path, options); + this.adjust_bucket_with_schema_updates(bucket); return bucket; } @@ -440,6 +441,18 @@ class ConfigFS { const bucket_config_path = this.get_bucket_path_by_name(bucket_name); await native_fs_utils.delete_config_file(this.fs_context, this.buckets_dir_path, bucket_config_path); } + + /** + * adjust_bucket_with_schema_updates changes the bucket properties according to the schema + * @param {object} bucket + */ + adjust_bucket_with_schema_updates(bucket) { + if (!bucket) return; + // system_owner is deprecated since version 5.18 + if (bucket.system_owner !== undefined) { + delete bucket.system_owner; + } + } } // EXPORTS diff --git a/src/sdk/object_sdk.js b/src/sdk/object_sdk.js index c57efd5a8c..594c72fd33 100644 --- a/src/sdk/object_sdk.js +++ b/src/sdk/object_sdk.js @@ -194,7 +194,7 @@ class ObjectSDK { const { bucket } = await bucket_namespace_cache.get_with_cache({ sdk: this, name }); const policy_info = { s3_policy: bucket.s3_policy, - system_owner: bucket.system_owner, + system_owner: bucket.system_owner, // note that bucketspace_fs currently doesn't return system_owner bucket_owner: bucket.bucket_owner, owner_account: bucket.owner_account, // in NC NSFS this is the account id that owns the bucket }; diff --git a/src/server/system_services/schemas/nsfs_bucket_schema.js b/src/server/system_services/schemas/nsfs_bucket_schema.js index 137a663168..255b0de0f3 100644 --- a/src/server/system_services/schemas/nsfs_bucket_schema.js +++ b/src/server/system_services/schemas/nsfs_bucket_schema.js @@ -7,7 +7,6 @@ module.exports = { required: [ '_id', 'name', - 'system_owner', 'bucket_owner', 'owner_account', 'versioning', @@ -30,6 +29,10 @@ module.exports = { name: { type: 'string', }, + /** + * @deprecated system_owner is kept for backward compatibility, + * but will no longer be included in new / updated bucket json. + */ system_owner: { type: 'string', }, diff --git a/src/test/unit_tests/jest_tests/test_accountspace_fs.test.js b/src/test/unit_tests/jest_tests/test_accountspace_fs.test.js index c47be10df1..d620fb5ecf 100644 --- a/src/test/unit_tests/jest_tests/test_accountspace_fs.test.js +++ b/src/test/unit_tests/jest_tests/test_accountspace_fs.test.js @@ -1872,7 +1872,6 @@ function _new_bucket_defaults(account, bucket_name, bucket_storage_path) { _id: '65a8edc9bc5d5bbf9db71c75', name: bucket_name, owner_account: account._id, - system_owner: new SensitiveString(account.name), bucket_owner: new SensitiveString(account.name), creation_date: new Date().toISOString(), path: bucket_storage_path, diff --git a/src/test/unit_tests/jest_tests/test_config_fs.test.js b/src/test/unit_tests/jest_tests/test_config_fs.test.js new file mode 100644 index 0000000000..a595993ce3 --- /dev/null +++ b/src/test/unit_tests/jest_tests/test_config_fs.test.js @@ -0,0 +1,30 @@ +/* Copyright (C) 2024 NooBaa */ +'use strict'; + +const path = require('path'); +const config = require('../../../../config'); +const { TMP_PATH } = require('../../system_tests/test_utils'); +const { get_process_fs_context } = require('../../../util/native_fs_utils'); +const { ConfigFS } = require('../../../sdk/config_fs'); + +const tmp_fs_path = path.join(TMP_PATH, 'test_config_fs'); +const config_root = path.join(tmp_fs_path, 'config_root'); +const config_root_backend = config.NSFS_NC_CONFIG_DIR_BACKEND; +const fs_context = get_process_fs_context(config_root_backend); + +const config_fs = new ConfigFS(config_root, config_root_backend, fs_context); + +describe('adjust_bucket_with_schema_updates', () => { + it('should return undefined on undefined bucket', () => { + const bucket = undefined; + config_fs.adjust_bucket_with_schema_updates(bucket); + expect(bucket).toBeUndefined(); + }); + + it('should return bucket without deprecated properties (system_owner)', () => { + const bucket = {name: 'bucket1', system_owner: 'account1-system-owner'}; + config_fs.adjust_bucket_with_schema_updates(bucket); + expect(bucket).toBeDefined(); + expect(bucket).not.toHaveProperty('system_owner'); + }); +}); diff --git a/src/test/unit_tests/jest_tests/test_nc_nsfs_bucket_schema_validation.test.js b/src/test/unit_tests/jest_tests/test_nc_nsfs_bucket_schema_validation.test.js index eb97b6c17b..c1d7d9a7ad 100644 --- a/src/test/unit_tests/jest_tests/test_nc_nsfs_bucket_schema_validation.test.js +++ b/src/test/unit_tests/jest_tests/test_nc_nsfs_bucket_schema_validation.test.js @@ -181,24 +181,6 @@ describe('schema validation NC NSFS bucket', () => { assert_validation(bucket_data, reason, message); }); - it('bucket without system_owner', () => { - const bucket_data = get_bucket_data(); - delete bucket_data.system_owner; - const reason = 'Test should have failed because of missing required property ' + - 'system_owner'; - const message = "must have required property 'system_owner'"; - assert_validation(bucket_data, reason, message); - }); - - it('bucket with undefined system_owner', () => { - const bucket_data = get_bucket_data(); - bucket_data.system_owner = undefined; - const reason = 'Test should have failed because of missing required property ' + - 'system_owner'; - const message = "must have required property 'system_owner'"; - assert_validation(bucket_data, reason, message); - }); - it('bucket without bucket_owner', () => { const bucket_data = get_bucket_data(); delete bucket_data.bucket_owner; @@ -463,7 +445,6 @@ describe('schema validation NC NSFS bucket', () => { function get_bucket_data() { const bucket_name = 'bucket1'; const id = '65a62e22ceae5e5f1a758aa8'; - const system_owner = 'account1'; // GAP - currently account name const bucket_owner = 'account1'; // account name const owner_account = '65b3c68b59ab67b16f98c26e'; const versioning_disabled = 'DISABLED'; @@ -473,7 +454,6 @@ function get_bucket_data() { const bucket_data = { _id: id, name: bucket_name, - system_owner: system_owner, bucket_owner: bucket_owner, owner_account: owner_account, versioning: versioning_disabled, diff --git a/src/test/unit_tests/test_bucketspace_fs.js b/src/test/unit_tests/test_bucketspace_fs.js index df1a6e4cc9..fe8ee0970f 100644 --- a/src/test/unit_tests/test_bucketspace_fs.js +++ b/src/test/unit_tests/test_bucketspace_fs.js @@ -387,6 +387,38 @@ mocha.describe('bucketspace_fs', function() { }); }); + mocha.describe('read_bucket_sdk_info', function() { + mocha.before(async function() { + const param = { name: test_bucket}; + await bucketspace_fs.create_bucket(param, dummy_object_sdk); + }); + mocha.after(async function() { + await fs_utils.folder_delete(`${new_buckets_path}/${test_bucket}`); + const file_path = get_config_file_path(CONFIG_SUBDIRS.BUCKETS, test_bucket); + await fs_utils.file_delete(file_path); + }); + mocha.it('read bucket that was created - check non existing deprecated properties', async function() { + const param = { name: test_bucket}; + const bucket = await bucketspace_fs.read_bucket_sdk_info(param); + assert.ok(bucket.system_owner === undefined); + }); + mocha.it('read bucket and check modify on read', async function() { + // we want to mock a bucket that already had a deprecated property, + // we manually add system_owner in the bucket + const bucket_config_path = get_config_file_path(CONFIG_SUBDIRS.BUCKETS, test_bucket); + const data = await fs.promises.readFile(bucket_config_path); + let bucket = await JSON.parse(data.toString()); + assert.ok(bucket.system_owner === undefined); + bucket.system_owner = 'non-existing-system-owner'; + await update_config_file(process_fs_context, CONFIG_SUBDIRS.BUCKETS, bucket_config_path, JSON.stringify(bucket)); + await fs_utils.file_must_exist(bucket_config_path); + + const param = { name: test_bucket}; + bucket = await bucketspace_fs.read_bucket_sdk_info(param); + assert.ok(bucket.system_owner === undefined); + }); + }); + mocha.describe('list_buckets', async function() { mocha.before(async function() { await create_bucket(test_bucket); diff --git a/src/test/unit_tests/test_nc_nsfs_cli.js b/src/test/unit_tests/test_nc_nsfs_cli.js index 89ad47e2b5..05c4d2a783 100644 --- a/src/test/unit_tests/test_nc_nsfs_cli.js +++ b/src/test/unit_tests/test_nc_nsfs_cli.js @@ -1126,7 +1126,6 @@ function assert_response(action, type, actual_res, expected_res, show_secrets, w function assert_bucket(bucket, bucket_options) { assert.strictEqual(bucket.name, bucket_options.name); - assert.strictEqual(bucket.system_owner, bucket_options.owner); assert.strictEqual(bucket.bucket_owner, bucket_options.owner); assert.strictEqual(bucket.path, bucket_options.path); assert.strictEqual(bucket.should_create_underlying_storage, false); diff --git a/src/test/unit_tests/test_s3_bucket_policy.js b/src/test/unit_tests/test_s3_bucket_policy.js index aa02c3bec5..a39e70626b 100644 --- a/src/test/unit_tests/test_s3_bucket_policy.js +++ b/src/test/unit_tests/test_s3_bucket_policy.js @@ -467,8 +467,6 @@ mocha.describe('s3_bucket_policy', function() { }); mocha.it('should deny bucket owner access', async function() { - // test fails on NC_CORETEST because system_owner === bucket_owner, until this behavior changes skipping the test - if (process.env.NC_CORETEST) this.skip(); // eslint-disable-line no-invalid-this const policy = { Version: '2012-10-17', Statement: [{ @@ -500,6 +498,8 @@ mocha.describe('s3_bucket_policy', function() { }); mocha.it('should set and delete bucket policy when system owner', async function() { + // test fails on NC_CORETEST because we do not set system_owner in buckets + if (process.env.NC_CORETEST) this.skip(); // eslint-disable-line no-invalid-this const policy = { Version: '2012-10-17', Statement: [{