NC | bucket owner removal

Signed-off-by: Romy <35330373+romayalon@users.noreply.github.com>

Author: romayalon

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","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","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

@@ -21,7 +21,7 @@ const noobaa_cli_diagnose = require('../manage_nsfs/diagnose');
 const nsfs_schema_utils = require('../manage_nsfs/nsfs_schema_utils');
 const { print_usage } = require('../manage_nsfs/manage_nsfs_help_utils');
 const { TYPES, ACTIONS, LIST_ACCOUNT_FILTERS, LIST_BUCKET_FILTERS, GLACIER_ACTIONS } = require('../manage_nsfs/manage_nsfs_constants');
-const { throw_cli_error, write_stdout_response, get_boolean_or_string_value, has_access_keys, set_debug_level } = require('../manage_nsfs/manage_nsfs_cli_utils');
+const { throw_cli_error, get_bucket_owner_account, write_stdout_response, get_boolean_or_string_value, has_access_keys, set_debug_level } = require('../manage_nsfs/manage_nsfs_cli_utils');
 const manage_nsfs_validations = require('../manage_nsfs/manage_nsfs_validations');
 const nc_mkm = require('../manage_nsfs/nc_master_key_manager').get_instance();
 
@@ -97,7 +97,6 @@ async function fetch_bucket_data(action, user_input) {
         _id: undefined,
         name: user_input.name === undefined ? undefined : String(user_input.name),
         owner_account: undefined,
-        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,
         creation_date: action === ACTIONS.ADD ? new Date().toISOString() : undefined,
@@ -126,6 +125,13 @@ async function fetch_bucket_data(action, user_input) {
         data = await fetch_existing_bucket_data(data);
     }
 
+    //if we're updating the owner, needs to override owner in file with the owner from user input.
+    //if we're adding a bucket, need to set its owner id field
+    if ((action === ACTIONS.UPDATE && user_input.owner) || (action === ACTIONS.ADD)) {
+        const account = await get_bucket_owner_account(config_fs, String(user_input.owner));
+        data.owner_account = account._id;
+    }
+
     // override values
     // fs_backend deletion specified with empty string '' (but it is not part of the schema)
     data.fs_backend = data.fs_backend || undefined;
@@ -159,15 +165,17 @@ async function add_bucket(data) {
     // for validating against the schema we need an object, hence we parse it back to object
     nsfs_schema_utils.validate_bucket_schema(JSON.parse(data_json));
     await config_fs.create_bucket_config_file(data.name, data_json);
-    write_stdout_response(ManageCLIResponse.BucketCreated, data_json, { bucket: data.name });
+    await set_bucker_owner(data);
+    write_stdout_response(ManageCLIResponse.BucketCreated, data, { bucket: data.name });
 }
 
 async function get_bucket_status(data) {
     await manage_nsfs_validations.validate_bucket_args(config_fs, data, ACTIONS.STATUS);
 
     try {
-        const config_data = await config_fs.get_bucket_by_name(data.name);
-        write_stdout_response(ManageCLIResponse.BucketStatus, config_data);
+        const bucket_data = await config_fs.get_bucket_by_name(data.name);
+        await set_bucker_owner(bucket_data);
+        write_stdout_response(ManageCLIResponse.BucketStatus, bucket_data);
     } catch (err) {
         const err_code = err.code === 'EACCES' ? ManageCLIError.AccessDenied : ManageCLIError.NoSuchBucket;
         throw_cli_error(err_code, data.name);
@@ -185,9 +193,11 @@ async function update_bucket(data) {
         // We take an object that was stringify
         // (it unwraps ths sensitive strings, creation_date to string and removes undefined parameters)
         // for validating against the schema we need an object, hence we parse it back to object
-        nsfs_schema_utils.validate_bucket_schema(JSON.parse(data));
+        const parsed_data = JSON.parse(data);
+        nsfs_schema_utils.validate_bucket_schema(parsed_data);
         await config_fs.update_bucket_config_file(cur_name, data);
-        write_stdout_response(ManageCLIResponse.BucketUpdated, data);
+        await set_bucker_owner(parsed_data);
+        write_stdout_response(ManageCLIResponse.BucketUpdated, parsed_data);
         return;
     }
 
@@ -203,6 +213,8 @@ async function update_bucket(data) {
     nsfs_schema_utils.validate_bucket_schema(JSON.parse(data));
     await config_fs.create_bucket_config_file(new_name, data);
     await config_fs.delete_bucket_config_file(cur_name);
+    data = JSON.parse(data);
+    await set_bucker_owner(data);
     write_stdout_response(ManageCLIResponse.BucketUpdated, data);
 }
 
@@ -577,7 +589,7 @@ function filter_bucket(bucket, filters) {
  * @param {object} [filters]
  */
 async function list_config_files(type, wide, show_secrets, filters = {}) {
-    let entries = [];
+    let entry_names = [];
     const should_filter = Object.keys(filters).length > 0;
     const is_filter_by_name = filters.name !== undefined;
 
@@ -592,24 +604,35 @@ async function list_config_files(type, wide, show_secrets, filters = {}) {
     // in case we have a filter by name, we don't need to read all the entries and iterate them
     // instead we "mock" the entries array to have one entry and it is the name by the filter (we add it for performance)
     if (is_filter_by_name) {
-        entries = [filters.name];
+        entry_names = [filters.name];
     } else if (type === TYPES.ACCOUNT) {
-        entries = await config_fs.list_accounts();
+        entry_names = await config_fs.list_accounts();
     } else if (type === TYPES.BUCKET) {
-        entries = await config_fs.list_buckets();
+        entry_names = await config_fs.list_buckets();
     }
 
-    let config_files_list = await P.map_with_concurrency(10, entries, async entry => {
+    // temporary cache for mapping bucker owner_account (id) -> bucket_owner (name)
+    const bucket_owners_map = {};
+    let config_files_list = await P.map_with_concurrency(10, entry_names, async entry_name => {
         if (wide || should_filter) {
             const data = type === TYPES.ACCOUNT ?
-                await config_fs.get_account_by_name(entry, options) :
-                await config_fs.get_bucket_by_name(entry, options);
+                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
-            return wide ? _.omit(data, show_secrets ? [] : ['access_keys']) : { name: entry };
+            if (!wide) return { name: entry_name };
+            if (type === TYPES.ACCOUNT) return _.omit(data, show_secrets ? [] : ['access_keys']);
+            if (type === TYPES.BUCKET) {
+                data.bucket_owner = bucket_owners_map[data.owner_account];
+                if (!data.bucket_owner) {
+                    await set_bucker_owner(data);
+                    bucket_owners_map[data.owner_account] = data.bucket_owner;
+                }
+                return data;
+            }
         } else {
-            return { name: entry };
+            return { name: entry_name };
         }
     });
     // it inserts undefined for the entry '.noobaa-config-nsfs' and we wish to remove it
@@ -646,6 +669,15 @@ function get_access_keys(action, user_input) {
     return { access_keys, new_access_key };
 }
 
+/**
+ * set_bucker_owner gets bucket owner from cache by its id and sets bucket_owner name on the bucket data
+ * @param {object} bucket_data 
+ */
+async function set_bucker_owner(bucket_data) {
+    const account_data = await config_fs.get_identity_by_id(bucket_data.owner_account, TYPES.ACCOUNT, { silent_if_missing: true});
+    bucket_data.bucket_owner = account_data?.name;
+}
+
 async function whitelist_ips_management(args) {
     const ips = args.ips;
     manage_nsfs_validations.validate_whitelist_arg(ips);

src/cmd/nsfs.js

@@ -207,7 +207,6 @@ class NsfsObjectSDK extends ObjectSDK {
                     Principal: [new SensitiveString('*')],
                 }]
             },
-            system_owner: new SensitiveString('nsfs'),
             bucket_owner: new SensitiveString('nsfs'),
             owner_account: new SensitiveString('nsfs-id'), // temp
         };

src/manage_nsfs/manage_nsfs_cli_utils.js

@@ -37,15 +37,20 @@ function write_stdout_response(response_code, detail, event_arg) {
  * get_bucket_owner_account will return the account of the bucket_owner
  * otherwise it would throw an error
  * @param {import('../sdk/config_fs').ConfigFS} config_fs
- * @param {string} bucket_owner
+ * @param {string} [bucket_owner]
+ * @param {string} [owner_account_id]
  */
-async function get_bucket_owner_account(config_fs, bucket_owner) {
+async function get_bucket_owner_account(config_fs, bucket_owner, owner_account_id) {
     try {
-        const account = await config_fs.get_account_by_name(bucket_owner);
+        const account = bucket_owner ?
+            await config_fs.get_account_by_name(bucket_owner) :
+            await config_fs.get_identity_by_id(owner_account_id);
         return account;
     } catch (err) {
         if (err.code === 'ENOENT') {
-            const detail_msg = `bucket owner ${bucket_owner} does not exists`;
+            const detail_msg = bucket_owner ?
+                `bucket owner name ${bucket_owner} does not exists` :
+                `bucket owner id ${owner_account_id} does not exists`;
             throw_cli_error(ManageCLIError.BucketSetForbiddenBucketOwnerNotExists, detail_msg, {bucket_owner: bucket_owner});
         }
         throw err;

src/manage_nsfs/manage_nsfs_logging.js

@@ -1,13 +1,14 @@
 /* Copyright (C) 2024 NooBaa */
 'use strict';
 
+const AWS = require('aws-sdk');
 const config = require('../../config');
-const { throw_cli_error, write_stdout_response} = require('../manage_nsfs/manage_nsfs_cli_utils');
+const http_utils = require('../util/http_utils');
+const { CONFIG_TYPES } = require('../sdk/config_fs');
+const { export_logs_to_target } = require('../util/bucket_logs_utils');
 const ManageCLIError = require('../manage_nsfs/manage_nsfs_cli_errors').ManageCLIError;
 const ManageCLIResponse = require('../manage_nsfs/manage_nsfs_cli_responses').ManageCLIResponse;
-const { export_logs_to_target } = require('../util/bucket_logs_utils');
-const http_utils = require('../util/http_utils');
-const AWS = require('aws-sdk');
+const { throw_cli_error, write_stdout_response} = require('../manage_nsfs/manage_nsfs_cli_utils');
 
 let config_fs;
 /** This command goes over the logs in the persistent log and move the entries to log objects in the target buckets 
@@ -39,8 +40,9 @@ async function export_bucket_logging(shared_config_fs) {
  */
 async function get_bucket_owner_keys(log_bucket_name) {
     const log_bucket_config_data = await config_fs.get_bucket_by_name(log_bucket_name);
-    const log_bucket_owner = log_bucket_config_data.bucket_owner;
-    const owner_config_data = await config_fs.get_account_by_name(log_bucket_owner, { show_secrets: true, decrypt_secret_key: true });
+    const log_bucket_owner = log_bucket_config_data.owner_account;
+    const owner_config_data = await config_fs.get_identity_by_id(log_bucket_owner,
+        CONFIG_TYPES.ACCOUNT, { show_secrets: true, decrypt_secret_key: true });
     return owner_config_data.access_keys;
 }
 

src/manage_nsfs/manage_nsfs_validations.js

@@ -9,8 +9,8 @@ const string_utils = require('../util/string_utils');
 const native_fs_utils = require('../util/native_fs_utils');
 const ManageCLIError = require('../manage_nsfs/manage_nsfs_cli_errors').ManageCLIError;
 const bucket_policy_utils = require('../endpoint/s3/s3_bucket_policy_utils');
-const { throw_cli_error, get_bucket_owner_account, get_options_from_file, get_boolean_or_string_value,
-    check_root_account_owns_user } = require('../manage_nsfs/manage_nsfs_cli_utils');
+const { throw_cli_error, get_options_from_file, get_boolean_or_string_value,
+    check_root_account_owns_user, get_bucket_owner_account} = require('../manage_nsfs/manage_nsfs_cli_utils');
 const { TYPES, ACTIONS, VALID_OPTIONS, OPTION_TYPE, FROM_FILE, BOOLEAN_STRING_VALUES, BOOLEAN_STRING_OPTIONS,
     GLACIER_ACTIONS, LIST_UNSETABLE_OPTIONS, ANONYMOUS, DIAGNOSE_ACTIONS } = require('../manage_nsfs/manage_nsfs_constants');
 const iam_utils = require('../endpoint/iam/iam_utils');
@@ -316,7 +316,7 @@ async function validate_bucket_args(config_fs, data, action) {
     if (action === ACTIONS.ADD || action === ACTIONS.UPDATE) {
         if (action === ACTIONS.ADD) native_fs_utils.validate_bucket_creation({ name: data.name });
         if ((action === ACTIONS.UPDATE) && (data.new_name !== undefined)) native_fs_utils.validate_bucket_creation({ name: data.new_name });
-        if ((action === ACTIONS.ADD) && (data.bucket_owner === undefined)) throw_cli_error(ManageCLIError.MissingBucketOwnerFlag);
+        if ((action === ACTIONS.ADD) && (data.owner_account === undefined)) throw_cli_error(ManageCLIError.MissingBucketOwnerFlag);
         if (!data.path) throw_cli_error(ManageCLIError.MissingBucketPathFlag);
         // fs_backend='' used for deletion of the fs_backend property
         if (data.fs_backend !== undefined && !['GPFS', 'CEPH_FS', 'NFSv4'].includes(data.fs_backend)) {
@@ -328,30 +328,33 @@ async function validate_bucket_args(config_fs, data, action) {
         if (!exists) {
             throw_cli_error(ManageCLIError.InvalidStoragePath, data.path);
         }
-        const account = await get_bucket_owner_account(config_fs, data.bucket_owner);
-        const account_fs_context = await native_fs_utils.get_fs_context(account.nsfs_account_config, data.fs_backend);
+
+        // bucket owner account validations 
+        const owner_account_data = await get_bucket_owner_account(config_fs, undefined, data.owner_account);
+        const account_fs_context = await native_fs_utils.get_fs_context(owner_account_data.nsfs_account_config,
+            owner_account_data.nsfs_account_config.fs_backend);
         if (!config.NC_DISABLE_ACCESS_CHECK) {
             const accessible = await native_fs_utils.is_dir_rw_accessible(account_fs_context, data.path);
             if (!accessible) {
                 throw_cli_error(ManageCLIError.InaccessibleStoragePath, data.path);
             }
         }
         if (action === ACTIONS.ADD) {
-            if (!account.allow_bucket_creation) {
-                const detail_msg = `${data.bucket_owner} account not allowed to create new buckets. ` +
+            if (!owner_account_data.allow_bucket_creation) {
+                const detail_msg = `${owner_account_data.name} account not allowed to create new buckets. ` +
                 `Please make sure to have a valid new_buckets_path and enable the flag allow_bucket_creation`;
                 throw_cli_error(ManageCLIError.BucketCreationNotAllowed, detail_msg);
+            }
         }
-            data.owner_account = account._id; // TODO move this assignment to better place
-        }
-        if (account.owner) {
-            const detail_msg = `account ${data.bucket_owner} is IAM account`;
-            throw_cli_error(ManageCLIError.BucketSetForbiddenBucketOwnerIsIAMAccount, detail_msg, {bucket_owner: data.bucket_owner});
+        if (owner_account_data.owner) {
+            const detail_msg = `account ${owner_account_data.name} is IAM account`;
+            throw_cli_error(ManageCLIError.BucketSetForbiddenBucketOwnerIsIAMAccount, detail_msg,
+                { bucket_owner: owner_account_data.name });
         }
         if (data.s3_policy) {
             try {
                 await bucket_policy_utils.validate_s3_policy(data.s3_policy, data.name,
-                    async principal => config_fs.is_account_exists_by_name(principal, account.owner)
+                    async principal => config_fs.is_account_exists_by_name(principal, owner_account_data.owner)
                 );
             } catch (err) {
                 dbg.error('validate_bucket_args invalid bucket policy err:', err);
@@ -440,7 +443,7 @@ async function validate_account_args(config_fs, data, action, is_flag_iam_operat
  * @param {object} data
  */
 async function validate_account_resources_before_deletion(config_fs, data) {
-    await validate_account_not_owns_buckets(config_fs, data.name);
+    await validate_account_not_owns_buckets(config_fs, data);
     // If it is root account (not owned by other account) then we check that it doesn't owns IAM accounts
     if (data.owner === undefined) {
         await check_if_root_account_does_not_have_IAM_users(config_fs, data, ACTIONS.DELETE);
@@ -476,14 +479,14 @@ function _validate_access_keys(access_key, secret_key) {
  * validate_delete_account will check if the account has at least one bucket
  * in case it finds one, it would throw an error
  * @param {import('../sdk/config_fs').ConfigFS} config_fs
- * @param {string} account_name
+ * @param {Object} account_data
  */
-async function validate_account_not_owns_buckets(config_fs, account_name) {
+async function validate_account_not_owns_buckets(config_fs, account_data) {
     const bucket_names = await config_fs.list_buckets();
     await P.map_with_concurrency(10, bucket_names, async bucket_name => {
         const data = await config_fs.get_bucket_by_name(bucket_name, { silent_if_missing: true });
-        if (data && data.bucket_owner === account_name) {
-            const detail_msg = `Account ${account_name} has bucket ${data.name}`;
+        if (data && data.owner_account === account_data._id) {
+            const detail_msg = `Account ${account_data.name} has bucket ${data.name}`;
             throw_cli_error(ManageCLIError.AccountDeleteForbiddenHasBuckets, detail_msg);
         }
         return data;

src/sdk/accountspace_fs.js

@@ -672,7 +672,7 @@ class AccountSpaceFS {
         const bucket_names = await this.config_fs.list_buckets();
         await P.map_with_concurrency(10, bucket_names, async bucket_name => {
             const bucket_data = await this.config_fs.get_bucket_by_name(bucket_name, { silent_if_missing: true});
-            if (bucket_data && bucket_data.bucket_owner === account_to_delete.name) {
+            if (bucket_data && bucket_data.owner_account === account_to_delete._id) {
                 this._throw_error_delete_conflict(action, account_to_delete, resource_name);
             }
             return bucket_data;