Merge pull request #8183 from shirady/nsfs-nc-check-deletion

NSFS | NC | Handle Concurrency When Reading Entries and They Deleted by Another Process

src/cmd/manage_nsfs.js

@@ -20,7 +20,7 @@ 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_config_file_path, get_symlink_config_file_path,
-    get_config_data, get_boolean_or_string_value, has_access_keys, set_debug_level} = require('../manage_nsfs/manage_nsfs_cli_utils');
+    get_config_data, get_boolean_or_string_value, has_access_keys, set_debug_level, get_config_data_if_exists } = 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();
 
@@ -642,7 +642,8 @@ async function list_config_files(type, config_path, wide, show_secrets, filters)
         if (entry.name.endsWith('.json')) {
             if (wide || should_filter) {
                 const full_path = path.join(config_path, entry.name);
-                const data = await get_config_data(config_root_backend, full_path, show_secrets || should_filter);
+                const data = await get_config_data_if_exists(config_root_backend, full_path, show_secrets || should_filter);
+                if (!data) return undefined;
                 // decryption causing mkm initalization
                 // decrypt only if data has access_keys and show_secrets = true (no need to decrypt if show_secrets = false but should_filter = true)
                 if (data.access_keys && show_secrets) data.access_keys = await nc_mkm.decrypt_access_keys(data);
@@ -655,6 +656,7 @@ async function list_config_files(type, config_path, wide, show_secrets, filters)
         }
     });
     // it inserts undefined for the entry '.noobaa-config-nsfs' and we wish to remove it
+    // in case the entry was deleted during the list it also inserts undefined
     config_files_list = config_files_list.filter(item => item);
 
     return config_files_list;

src/manage_nsfs/manage_nsfs_cli_utils.js

@@ -55,6 +55,23 @@ async function get_config_data(config_root_backend, config_file_path, show_secre
     return config_data;
 }
 
+/**
+ * get_config_data_if_exists will read a config file and return its content 
+ * while omitting secrets if show_secrets flag was not provided
+ * if the config file was deleted (encounter ENOENT error) - continue (returns undefined)
+ * @param {string} config_file_path
+ * @param {boolean} [show_secrets]
+ */
+async function get_config_data_if_exists(config_root_backend, config_file_path, show_secrets = false) {
+    try {
+        const config_data = await get_config_data(config_root_backend, config_file_path, show_secrets);
+        return config_data;
+    } catch (err) {
+        dbg.warn('get_config_data_if_exists: with config_file_path', config_file_path, 'got an error', err);
+        if (err.code !== 'ENOENT') throw err;
+    }
+}
+
 /**
  * get_bucket_owner_account will return the account of the bucket_owner
  * otherwise it would throw an error
@@ -166,3 +183,4 @@ exports.has_access_keys = has_access_keys;
 exports.generate_id = generate_id;
 exports.set_debug_level = set_debug_level;
 exports.check_root_account_owns_user = check_root_account_owns_user;
+exports.get_config_data_if_exists = get_config_data_if_exists;

src/manage_nsfs/manage_nsfs_validations.js

@@ -14,7 +14,7 @@ const ManageCLIError = require('../manage_nsfs/manage_nsfs_cli_errors').ManageCL
 const bucket_policy_utils = require('../endpoint/s3/s3_bucket_policy_utils');
 const { throw_cli_error, get_config_file_path, get_bucket_owner_account,
     get_config_data, get_options_from_file, get_boolean_or_string_value,
-    check_root_account_owns_user } = require('../manage_nsfs/manage_nsfs_cli_utils');
+    check_root_account_owns_user, get_config_data_if_exists } = 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 } = require('../manage_nsfs/manage_nsfs_constants');
 
@@ -427,11 +427,12 @@ function _validate_access_keys(access_key, secret_key) {
 async function validate_delete_account(config_root_backend, buckets_dir_path, account_name) {
     const fs_context = native_fs_utils.get_process_fs_context(config_root_backend);
     const entries = await nb_native().fs.readdir(fs_context, buckets_dir_path);
+    let data;
     await P.map_with_concurrency(10, entries, async entry => {
         if (entry.name.endsWith('.json')) {
             const full_path = path.join(buckets_dir_path, entry.name);
-            const data = await get_config_data(config_root_backend, full_path);
-            if (data.bucket_owner === account_name) {
+            data = await get_config_data_if_exists(config_root_backend, full_path);
+            if (data && data.bucket_owner === account_name) {
                 const detail_msg = `Account ${account_name} has bucket ${data.name}`;
                 throw_cli_error(ManageCLIError.AccountDeleteForbiddenHasBuckets, detail_msg);
             }

src/sdk/accountspace_fs.js

@@ -597,6 +597,7 @@ class AccountSpaceFS {
         const entries = await nb_native().fs.readdir(this.fs_context, this.accounts_dir);
         const should_filter_by_prefix = check_iam_path_was_set(iam_path_prefix);
 
+        // TODO - add silent get config to handle config deletion during list (concurrency)
         const config_files_list = await P.map_with_concurrency(10, entries, async entry => {
             if (entry.name.endsWith('.json')) {
                 const full_path = path.join(this.accounts_dir, entry.name);

src/sdk/bucketspace_fs.js

@@ -231,7 +231,15 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
                 return;
             }
             const bucket_name = this.get_bucket_name(entry.name);
-            const bucket = await object_sdk.read_bucket_sdk_config_info(bucket_name);
+            let bucket;
+            try {
+                bucket = await object_sdk.read_bucket_sdk_config_info(bucket_name);
+            } catch (err) {
+                dbg.warn('list_buckets: read_bucket_sdk_config_info of bucket', bucket_name, 'got an error', err);
+                // in case the config file was deleted during the bucket list - we will continue
+                if (err.rpc_code !== 'NO_SUCH_BUCKET') throw err;
+            }
+            if (!bucket) return;
             const bucket_policy_accessible = await this.has_bucket_action_permission(bucket, account, 's3:ListBucket');
             if (!bucket_policy_accessible) return;
             const fs_accessible = await this.validate_fs_bucket_access(bucket, object_sdk);

src/test/unit_tests/jest_tests/test_nc_nsfs_account_cli.test.js

@@ -1194,6 +1194,14 @@ describe('manage nsfs cli account flow', () => {
                 .toEqual([]);
         });
 
+        it('cli list filter by access key (non existing) and name (of account3) - (none)', async () => {
+            const account_options = { config_root, name: 'account3', access_key: 'non-existing-access-key' };
+            const action = ACTIONS.LIST;
+            const res = await exec_manage_cli(TYPES.ACCOUNT, action, account_options);
+            expect(JSON.parse(res).response.reply.map(item => item.name))
+                .toEqual([]);
+        });
+
         it('should fail - cli account list invalid flags combination (show_secrets without wide)', async () => {
             const account_options = { config_root, show_secrets: true}; // without wide it's invalid
             const action = ACTIONS.LIST;