Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NC | NSFS | Add stat to account_id_cache #8642

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
9 changes: 8 additions & 1 deletion config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 //
Expand Down Expand Up @@ -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;

Expand Down
55 changes: 47 additions & 8 deletions src/sdk/accountspace_fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
75 changes: 74 additions & 1 deletion src/sdk/config_fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
shirady marked this conversation as resolved.
Show resolved Hide resolved
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
Expand Down Expand Up @@ -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
Expand All @@ -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)
Expand Down Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion src/sdk/object_sdk.js
Original file line number Diff line number Diff line change
Expand Up @@ -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().
Expand Down Expand Up @@ -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;
}
}
Expand Down
82 changes: 79 additions & 3 deletions src/test/unit_tests/test_nc_with_a_couple_of_forks.js
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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 });
});
});
Loading