Merge pull request #8642 from shirady/nsfs-nc-account-id-cache-with-stat

NC | NSFS | Add `stat` to `account_id_cache`

Author: shirady

config.js

@@ -635,16 +635,24 @@ config.REMOTE_NOOAA_NAMESPACE = `remote-${config.KUBE_APP_LABEL}`;
 ///////////////////////////////
 config.INLINE_MAX_SIZE = 4096;
 
+///////////////////////////////
+// CACHE (ACCOUNT, BUCKET)   //
+///////////////////////////////
+
 // Object SDK bucket cache expiration time
 config.OBJECT_SDK_BUCKET_CACHE_EXPIRY_MS = 60000;
 // Object SDK account cache expiration time
 config.OBJECT_SDK_ACCOUNT_CACHE_EXPIRY_MS = Number(process.env.ACCOUNTS_CACHE_EXPIRY) || 10 * 60 * 1000; // TODO: Decide on a time that we want to invalidate
+// Accountspace_fs account id cache expiration time
+config.ACCOUNTS_ID_CACHE_EXPIRY = 3 * 60 * 1000; // TODO: Decide on a time that we want to invalidate
 
 
 // Object SDK bucket_namespace_cache allow stat of the config file
 config.NC_ENABLE_BUCKET_NS_CACHE_STAT_VALIDATION = true;
 // Object SDK account_cache allow stat of the config file
 config.NC_ENABLE_ACCOUNT_CACHE_STAT_VALIDATION = true;
+// accountspace_fs allow stat of the config file
+config.NC_ENABLE_ACCOUNT_ID_CACHE_STAT_VALIDATION = true;
 
 //////////////////////////////
 // OPERATOR RELATED         //
@@ -932,7 +940,6 @@ config.NC_DISABLE_HEALTH_ACCESS_CHECK = false;
 config.NC_DISABLE_POSIX_MODE_ACCESS_CHECK = true;
 config.NC_DISABLE_SCHEMA_CHECK = false;
 
-config.ACCOUNTS_ID_CACHE_EXPIRY = 3 * 60 * 1000;
 ////////// GPFS //////////
 config.GPFS_DOWN_DELAY = 1000;
 

src/sdk/accountspace_fs.js

@@ -28,22 +28,61 @@ const dummy_service_name = 's3';
 
 const account_id_cache = new LRUCache({
     name: 'AccountIDCache',
-    // TODO: Decide on a time that we want to invalidate
-    expiry_ms: Number(config.ACCOUNTS_ID_CACHE_EXPIRY),
+    expiry_ms: config.ACCOUNTS_ID_CACHE_EXPIRY,
     make_key: ({ _id }) => _id,
     /**
      * Accounts are added to the cache based on id, Default value for show_secrets and decrypt_secret_key will be true, 
      * and show_secrets and decrypt_secret_key `false` only when we load cache from the health script, 
      * health script doesn't need to fetch or decrypt the secret.
-     * @param {{ _id: string, 
-     * show_secrets?: boolean,
-     * decrypt_secret_key?: boolean, 
-     * config_fs: import('./config_fs').ConfigFS }} params
+     * @param {{
+        * _id: string,
+        * show_secrets?: boolean,
+        * decrypt_secret_key?: boolean,
+        * config_fs: ConfigFS
+     * }} params
     */
-    load: async ({ _id, show_secrets = true, decrypt_secret_key = true, config_fs}) =>
-        config_fs.get_identity_by_id(_id, CONFIG_TYPES.ACCOUNT, { show_secrets: show_secrets, decrypt_secret_key: decrypt_secret_key }),
+    load: async ({ _id, show_secrets = true, decrypt_secret_key = true, config_fs }) =>
+        config_fs.get_identity_by_id_and_stat_file(
+            _id, CONFIG_TYPES.ACCOUNT, { show_secrets: show_secrets, decrypt_secret_key: decrypt_secret_key }),
+    validate: (params, data) => _validate_account_id(params, data),
 });
 
+/** _validate_account_id is an additional layer (to expiry_ms)
+ * to checks the stat of the config file
+ * @param {object} data
+ * @param {object} params
+ * @returns Promise<{boolean>}
+ */
+async function _validate_account_id(params, data) {
+    if (config.NC_ENABLE_ACCOUNT_ID_CACHE_STAT_VALIDATION) {
+        const same_stat = await check_same_stat_account(params._id, params.name, params.stat, data.config_fs);
+        if (!same_stat) { // config file of account was changed
+            return false;
+        }
+    }
+    return true;
+}
+
+    /**
+     * check_same_stat_account will return true the config file was not changed
+     * in case we had any issue (for example error during stat) the returned value will be undefined
+     * @param {string} id
+     * @param {string} account_name
+     * @param {nb.NativeFSStats} account_stat
+     * @param {ConfigFS} config_fs
+     * @returns Promise<{boolean|undefined>}
+     */
+    async function check_same_stat_account(id, account_name, account_stat, config_fs) {
+        if (!config_fs) return;
+        try {
+            const current_stat = await config_fs.stat_account_config_file_by_identity(id, account_name);
+            if (current_stat) {
+                return current_stat.ino === account_stat.ino && current_stat.mtimeNsBigint === account_stat.mtimeNsBigint;
+            }
+        } catch (err) {
+            dbg.warn('check_same_stat_account: current_stat got an error', err, 'ignoring...');
+        }
+    }
 
 /**
  * @param {Object} requested_account

src/sdk/config_fs.js

@@ -378,6 +378,29 @@ class ConfigFS {
         return identity;
     }
 
+    /**
+     * get_identity_by_id_and_stat_file returns the full account/user data and stat the file:
+     * @param {string} id
+     * @param {string} [type]
+     * @param {{show_secrets?: boolean, decrypt_secret_key?: boolean, silent_if_missing?: boolean}} [options]
+     * @returns {Promise<Object>} 
+    */
+    async get_identity_by_id_and_stat_file(id, type, options = {}) {
+        let identity;
+        try {
+            identity = await this.get_identity_by_id(id, type, options);
+            if (!identity && options.silent_if_missing) {
+                return undefined; // we don't have the identity, so we can't add the property of stat to it
+            }
+            const stat = await this.stat_account_config_file_by_identity(id, identity.name);
+            identity.stat = stat;
+            return identity; // this identity object should have also a stat property
+        } catch (err) {
+            dbg.error('get_identity_by_id_and_stat_file: got an error', err);
+            throw err;
+        }
+    }
+
     /**
      * search_accounts_by_id searches old accounts directory and finds an account that its _id matches the given id param
      * @param {string} id
@@ -450,7 +473,7 @@ class ConfigFS {
     }
 
     /**
-     * stat_account_config_file will return the stat output on account config file
+     * stat_account_config_file will return the stat output on account config file by access key
      * please notice that stat might throw an error - you should wrap it with try-catch and handle the error
      * Note: access_key type of anonymous_access_key is a symbol, otherwise it is a string (not SensitiveString)
      * @param {Symbol|string} access_key
@@ -468,6 +491,38 @@ class ConfigFS {
         return nb_native().fs.stat(this.fs_context, path_for_account_or_user_config_file);
     }
 
+    /**
+     * stat_account_config_file_by_identity will return the stat output on account config file by id or by account name
+     * 1. try by identity path
+     * 2. if not found - try by account name with new accounts path
+     * 3. if not found - try by account name with old accounts path
+     * 4. else throw an error
+     * @param {string} id
+     * @param {string} account_name
+     * @returns {Promise<nb.NativeFSStats>}
+     */
+    async stat_account_config_file_by_identity(id, account_name) {
+        const options = { silent_if_missing: true };
+
+        const identity_path = this.get_identity_path_by_id(id);
+        dbg.log2('stat_account_config_file_by_identity: will try to stat by identity (identities path)');
+        let stat = await this.stat_config_file_general_use(identity_path, options);
+        if (stat) return stat;
+
+        dbg.log2('stat_account_config_file_by_identity: will try to stat by account name (new accounts path)');
+        const account_name_new_path = this.get_account_path_by_name(account_name);
+        stat = await this.stat_config_file_general_use(account_name_new_path, options);
+        if (stat) return stat;
+
+        dbg.log2('stat_account_config_file_by_identity: will try to stat by account name (old accounts path)');
+        const account_name_old_path = this._get_old_account_path_by_name(account_name);
+        stat = await this.stat_config_file_general_use(account_name_old_path, options);
+        if (stat) return stat;
+
+        dbg.error(`stat_account_config_file_by_identity: could not stat identity by id ${id} or by account name ${account_name} (new and old accounts path)`);
+        throw new Error('Could not stat account (identities path, new accounts path and old accounts path)');
+    }
+
     /**
      * is_account_exists_by_name returns true if account config path exists in config dir
      * if account does not exist and it's a regular account (not an IAM user) 
@@ -1255,6 +1310,24 @@ class ConfigFS {
     get_hosts_data(system_data) {
         return _.omit(system_data, 'config_directory');
     }
+
+    /** stat_config_file_general_use will stat a file by a path
+     * if the file is not found:
+     * - using the options with silent_if_missing true on ENOENT it would return undefined
+     * - else it would throw an error
+     * @param {string} path_to_stat
+     * @param {{ silent_if_missing: any; }} options
+     */
+    async stat_config_file_general_use(path_to_stat, options) {
+        try {
+            const stat_by_identity_path = await nb_native().fs.stat(this.fs_context, path_to_stat);
+            return stat_by_identity_path;
+        } catch (err) {
+            if (err.code === 'ENOENT' && options.silent_if_missing) return;
+            dbg.error('stat_account_config_file_by_identity: could not stat by identity ID, got an error', err);
+            throw err;
+        }
+    }
 }
 
 // EXPORTS

src/sdk/object_sdk.js

@@ -27,6 +27,7 @@ const BucketSpaceNB = require('./bucketspace_nb');
 const { RpcError } = require('../rpc');
 
 const anonymous_access_key = Symbol('anonymous_access_key');
+
 const bucket_namespace_cache = new LRUCache({
     name: 'ObjectSDK-Bucket-Namespace-Cache',
     // This is intentional. Cache entry expiration is handled by _validate_bucket_namespace().
@@ -93,7 +94,7 @@ async function _validate_account(data, params) {
     const bs_allow_stat_account = Boolean(bs.check_same_stat_account);
     if (bs_allow_stat_account && config.NC_ENABLE_ACCOUNT_CACHE_STAT_VALIDATION) {
         const same_stat = await bs.check_same_stat_account(params.access_key, data.stat);
-        if (!same_stat) { // config file of bucket was changed
+        if (!same_stat) { // config file of account was changed
             return false;
         }
     }

src/test/unit_tests/test_nc_with_a_couple_of_forks.js

@@ -9,16 +9,16 @@ const P = require('../../util/promise');
 const mocha = require('mocha');
 const assert = require('assert');
 const fs_utils = require('../../util/fs_utils');
-const { TMP_PATH, generate_nsfs_account, get_new_buckets_path_by_test_env, generate_s3_client,
-    get_coretest_path, exec_manage_cli } = require('../system_tests/test_utils');
+const { TMP_PATH, generate_nsfs_account, get_new_buckets_path_by_test_env, generate_s3_client, get_coretest_path, exec_manage_cli } = require('../system_tests/test_utils');
 const { TYPES, ACTIONS } = require('../../manage_nsfs/manage_nsfs_constants');
 const ManageCLIResponse = require('../../manage_nsfs/manage_nsfs_cli_responses').ManageCLIResponse;
 
 const coretest_path = get_coretest_path();
 const coretest = require(coretest_path);
 const setup_options = { forks: 2, debug: 5 };
 coretest.setup(setup_options);
-const { rpc_client, EMAIL, get_current_setup_options, stop_nsfs_process, start_nsfs_process, config_dir_name } = coretest;
+const { rpc_client, EMAIL, get_current_setup_options, stop_nsfs_process, start_nsfs_process,
+    config_dir_name, NC_CORETEST_CONFIG_FS, NC_CORETEST_STORAGE_PATH } = coretest;
 
 const CORETEST_ENDPOINT = coretest.get_http_address();
 
@@ -52,6 +52,7 @@ mocha.describe('operations with a couple of forks', async function() {
 
     mocha.after(async () => {
         fs_utils.folder_delete(`${config_root}`);
+        fs_utils.folder_delete(`${new_bucket_path_param}`);
     });
 
     mocha.it('versioning change with a couple of forks', async function() {
@@ -147,4 +148,79 @@ mocha.describe('operations with a couple of forks', async function() {
         // cleanup
         await s3_uid5_after_access_keys_update.deleteBucket({ Bucket: bucket_name2 });
     });
+
+    mocha.it('head a bucket after account update (change fs_backend)', async function() {
+        // create additional account
+        const account_name = 'Oliver';
+        const account_options_create = { account_name, uid: 6001, gid: 6001, config_root: config_dir_name };
+        await fs_utils.create_fresh_path(new_bucket_path_param);
+        await fs.promises.chown(new_bucket_path_param, account_options_create.uid, account_options_create.gid);
+        await fs.promises.chmod(new_bucket_path_param, 0o700);
+        const access_details = await generate_nsfs_account(rpc_client, EMAIL, new_bucket_path_param, account_options_create);
+        // check the account status
+        const account_options_status = { config_root: config_dir_name, name: account_name};
+        const res_account_status = await exec_manage_cli(TYPES.ACCOUNT, ACTIONS.STATUS, account_options_status);
+        assert.equal(JSON.parse(res_account_status).response.code, ManageCLIResponse.AccountStatus.code);
+        // generate the s3 client
+const s3_uid6001 = generate_s3_client(access_details.access_key,
+            access_details.secret_key, CORETEST_ENDPOINT);
+        // check the connection for the new account (can be any of the forks)
+        const res_list_buckets = await s3_uid6001.listBuckets({});
+        assert.equal(res_list_buckets.$metadata.httpStatusCode, 200);
+        // create a bucket
+        const bucket_name3 = 'bucket3';
+        const res_bucket_create = await s3_uid6001.createBucket({ Bucket: bucket_name3 });
+        assert.equal(res_bucket_create.$metadata.httpStatusCode, 200);
+        // head the bucket
+        const res_head_bucket1 = await s3_uid6001.headBucket({Bucket: bucket_name3});
+        assert.equal(res_head_bucket1.$metadata.httpStatusCode, 200);
+        // update the account
+        const account_options_update = { config_root: config_dir_name, name: account_name, fs_backend: 'GPFS'};
+        const res_account_update = await exec_manage_cli(TYPES.ACCOUNT, ACTIONS.UPDATE, account_options_update);
+        assert.equal(JSON.parse(res_account_update).response.code, ManageCLIResponse.AccountUpdated.code);
+        // head the bucket (again)
+        const res_head_bucket2 = await s3_uid6001.headBucket({Bucket: bucket_name3});
+        assert.equal(res_head_bucket2.$metadata.httpStatusCode, 200);
+
+        // cleanup
+        await s3_uid6001.deleteBucket({ Bucket: bucket_name3 });
+    });
+
+    mocha.it('create a bucket after account update (change buckets_path)', async function() {
+        // create an additional account
+        const account_name = 'John';
+        const account_options_create = { account_name, uid: 7001, gid: 7001, config_root: config_dir_name };
+        // reuse NC_CORETEST_STORAGE_PATH as new_buckets_path (no need for create fresh path, chmod and chown) 
+        const access_details = await generate_nsfs_account(rpc_client, EMAIL, NC_CORETEST_STORAGE_PATH, account_options_create);
+        // check the account status
+        const account_options_status = { config_root: config_dir_name, name: account_name};
+        const res_account_status = await exec_manage_cli(TYPES.ACCOUNT, ACTIONS.STATUS, account_options_status);
+        assert.equal(JSON.parse(res_account_status).response.code, ManageCLIResponse.AccountStatus.code);
+        // generate the s3 client
+        const s3_uid6001 = generate_s3_client(access_details.access_key,
+        access_details.secret_key, CORETEST_ENDPOINT);
+        // check the connection for the new account (can be any of the forks)
+        const res_list_buckets = await s3_uid6001.listBuckets({});
+        assert.equal(res_list_buckets.$metadata.httpStatusCode, 200);
+        // update the account - change its new_bucket_path
+        const new_bucket_path_param2 = path.join(TMP_PATH, 'nc_coretest_storage_root_path2/');
+        await fs_utils.create_fresh_path(new_bucket_path_param2);
+        await fs.promises.chown(new_bucket_path_param2, account_options_create.uid, account_options_create.gid);
+        await fs.promises.chmod(new_bucket_path_param2, 0o700);
+        const account_options_update = { config_root: config_dir_name, name: account_name, new_buckets_path: new_bucket_path_param2};
+        const res_account_update = await exec_manage_cli(TYPES.ACCOUNT, ACTIONS.UPDATE, account_options_update);
+        assert.equal(JSON.parse(res_account_update).response.code, ManageCLIResponse.AccountUpdated.code);
+        // create a bucket
+        const bucket_name4 = 'bucket4';
+        const res_bucket_create = await s3_uid6001.createBucket({ Bucket: bucket_name4 });
+        assert.equal(res_bucket_create.$metadata.httpStatusCode, 200);
+        // validate the bucket was created in the updated path
+        const bucket4 = await NC_CORETEST_CONFIG_FS.get_bucket_by_name(bucket_name4);
+        const expected_bucket_path = path.join(new_bucket_path_param2, bucket_name4);
+        assert.equal(bucket4.path, expected_bucket_path);
+
+        // cleanup
+        await s3_uid6001.deleteBucket({ Bucket: bucket_name4 });
+        await fs.promises.rm(new_bucket_path_param2, { recursive: true });
+    });
 });