NC | NSFS | Stop setting the system_owner as the account's name in bucket config

1. Remove the property system_owner as required property in nsfs_bucket_schema.
2. On the newly created bucket, do not set the system_owner value (on manage_nsfs and bucketspace_fs).
3. Remove the system_owner (with the value of the account name) from the current docs in NC NSFS.
4. Update unit test tests and remove the property system_owner.
5. Add the function modify_bucket_on_read in class ConfigFS, and use it from get_bucket_by_name and inside the bucket list in noobaa-cli.

Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>

Author: shirady

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.  

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 -

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

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;

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;

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

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
         };

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',
         },

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,

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');
+    });
+});