Merge pull request #8304 from romayalon/romy-accounts-id-cache

NC | Accounts ID cache addition

Author: romayalon

config.js

@@ -895,6 +895,7 @@ 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/cmd/manage_nsfs.js

@@ -13,6 +13,7 @@ const cloud_utils = require('../util/cloud_utils');
 const native_fs_utils = require('../util/native_fs_utils');
 const mongo_utils = require('../util/mongo_utils');
 const SensitiveString = require('../util/sensitive_string');
+const { account_id_cache } = require('../sdk/accountspace_fs');
 const ManageCLIError = require('../manage_nsfs/manage_nsfs_cli_errors').ManageCLIError;
 const ManageCLIResponse = require('../manage_nsfs/manage_nsfs_cli_responses').ManageCLIResponse;
 const manage_nsfs_glacier = require('../manage_nsfs/manage_nsfs_glacier');
@@ -21,7 +22,8 @@ const noobaa_cli_diagnose = require('../manage_nsfs/diagnose');
 const noobaa_cli_upgrade = require('../manage_nsfs/upgrade');
 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, get_bucket_owner_account, write_stdout_response, get_boolean_or_string_value, has_access_keys, set_debug_level,
+const { throw_cli_error, get_bucket_owner_account_by_name,
+    write_stdout_response, get_boolean_or_string_value, has_access_keys, set_debug_level,
     is_name_update, is_access_key_update } = 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();
@@ -122,7 +124,7 @@ async function fetch_bucket_data(action, user_input) {
     //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));
+        const account = await get_bucket_owner_account_by_name(config_fs, String(user_input.owner));
         data.owner_account = account._id;
     }
 
@@ -597,8 +599,6 @@ async function list_config_files(type, wide, show_secrets, filters = {}) {
         entry_names = await config_fs.list_buckets();
     }
 
-    // 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 ?
@@ -610,11 +610,7 @@ async function list_config_files(type, wide, show_secrets, filters = {}) {
             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;
-                }
+                await set_bucker_owner(data);
                 return data;
             }
         } else {
@@ -657,7 +653,12 @@ function get_access_keys(action, user_input) {
  * @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});
+    let account_data;
+    try {
+        account_data = await account_id_cache.get_with_cache({ _id: bucket_data.owner_account, config_fs });
+    } catch (err) {
+        dbg.warn(`set_bucker_owner.couldn't find bucket owner data by id ${bucket_data.owner_account}`);
+    }
     bucket_data.bucket_owner = account_data?.name;
 }
 

src/manage_nsfs/manage_nsfs_cli_utils.js

@@ -3,15 +3,14 @@
 
 const dbg = require('../util/debug_module')(__filename);
 const nb_native = require('../util/nb_native');
-const { CONFIG_TYPES } = require('../sdk/config_fs');
 const native_fs_utils = require('../util/native_fs_utils');
 const ManageCLIError = require('../manage_nsfs/manage_nsfs_cli_errors').ManageCLIError;
 const NSFS_CLI_ERROR_EVENT_MAP = require('../manage_nsfs/manage_nsfs_cli_errors').NSFS_CLI_ERROR_EVENT_MAP;
 const ManageCLIResponse = require('../manage_nsfs/manage_nsfs_cli_responses').ManageCLIResponse;
 const NSFS_CLI_SUCCESS_EVENT_MAP = require('../manage_nsfs/manage_nsfs_cli_responses').NSFS_CLI_SUCCESS_EVENT_MAP;
 const { BOOLEAN_STRING_VALUES } = require('../manage_nsfs/manage_nsfs_constants');
 const NoobaaEvent = require('../manage_nsfs/manage_nsfs_events_utils').NoobaaEvent;
-const mongo_utils = require('../util/mongo_utils');
+const { account_id_cache } = require('../sdk/accountspace_fs');
 
 
 function throw_cli_error(error, detail, event_arg) {
@@ -35,29 +34,43 @@ function write_stdout_response(response_code, detail, event_arg) {
 }
 
 /**
- * get_bucket_owner_account will return the account of the bucket_owner
+ * get_bucket_owner_account_by_name 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} [owner_account_id]
+ * @param {string} bucket_owner
  */
-async function get_bucket_owner_account(config_fs, bucket_owner, owner_account_id) {
+async function get_bucket_owner_account_by_name(config_fs, bucket_owner) {
     try {
-        const account = bucket_owner ?
-            await config_fs.get_account_by_name(bucket_owner) :
-            await config_fs.get_identity_by_id(owner_account_id, CONFIG_TYPES.ACCOUNT);
+        const account = await config_fs.get_account_by_name(bucket_owner);
         return account;
     } catch (err) {
         if (err.code === 'ENOENT') {
-            const detail_msg = bucket_owner ?
-                `bucket owner name ${bucket_owner} does not exists` :
-                `bucket owner id ${owner_account_id} does not exists`;
+            const detail_msg = `bucket owner name ${bucket_owner} does not exist`;
             throw_cli_error(ManageCLIError.BucketSetForbiddenBucketOwnerNotExists, detail_msg, {bucket_owner: bucket_owner});
         }
         throw err;
     }
 }
 
+/**
+ * get_bucket_owner_account_by_id will return the account of the owner_account id
+ * otherwise it would throw an error
+ * @param {import('../sdk/config_fs').ConfigFS} config_fs
+ * @param {string} owner_account
+ */
+async function get_bucket_owner_account_by_id(config_fs, owner_account) {
+    try {
+        const account = await account_id_cache.get_with_cache({ _id: owner_account, config_fs });
+        return account;
+    } catch (err) {
+        if (err.code === 'ENOENT') {
+            const detail_msg = `bucket owner account id ${owner_account} does not exist`;
+            throw_cli_error(ManageCLIError.BucketSetForbiddenBucketOwnerNotExists, detail_msg, { owner_account: owner_account });
+        }
+        throw err;
+    }
+}
+
 /**
  * get_boolean_or_string_value will check if the value
  * 1. if the value is undefined - it returns false.
@@ -113,28 +126,6 @@ function set_debug_level(debug) {
     nb_native().fs.set_debug_level(debug_level);
 }
 
-/**
- * generate_id will generate an id that we use to identify entities (such as account, bucket, etc.). 
- */
-// TODO: 
-// - reuse this function in NC NSFS where we used the mongo_utils module
-// - this function implantation should be db_client.new_object_id(), 
-//   but to align with manage nsfs we won't change it now
-function generate_id() {
-    return mongo_utils.mongoObjectId();
-}
-
-/**
- * check_root_account_owns_user checks if an account is owned by root account
- * @param {object} root_account
- * @param {object} account
- */
-function check_root_account_owns_user(root_account, account) {
-    if (account.owner === undefined) return false;
-    return root_account._id === account.owner;
-}
-
-
 /**
  * is_name_update returns true if a new_name flag was provided and it's not equal to 
  * the current name
@@ -163,11 +154,10 @@ function is_access_key_update(data) {
 exports.throw_cli_error = throw_cli_error;
 exports.write_stdout_response = write_stdout_response;
 exports.get_boolean_or_string_value = get_boolean_or_string_value;
-exports.get_bucket_owner_account = get_bucket_owner_account;
+exports.get_bucket_owner_account_by_name = get_bucket_owner_account_by_name;
+exports.get_bucket_owner_account_by_id = get_bucket_owner_account_by_id;
 exports.get_options_from_file = get_options_from_file;
 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.is_name_update = is_name_update;
 exports.is_access_key_update = is_access_key_update;

src/manage_nsfs/manage_nsfs_logging.js

@@ -4,7 +4,7 @@
 const AWS = require('aws-sdk');
 const config = require('../../config');
 const http_utils = require('../util/http_utils');
-const { CONFIG_TYPES } = require('../sdk/config_fs');
+const { account_id_cache } = require('../sdk/accountspace_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;
@@ -40,10 +40,15 @@ 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.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;
+    const log_bucket_owner_id = log_bucket_config_data.owner_account;
+    try {
+        const owner_config_data = await account_id_cache.get_with_cache({ _id: log_bucket_owner_id, config_fs });
+        return owner_config_data.access_keys;
+    } catch (err) {
+        throw_cli_error(ManageCLIError.BucketSetForbiddenBucketOwnerNotExists,
+            `could not find log bucket owner by id ${log_bucket_owner_id}, can not extract owner access keys`,
+            { owner_account: log_bucket_owner_id });
+    }
 }
 
 exports.export_bucket_logging = export_bucket_logging;

src/manage_nsfs/manage_nsfs_validations.js

@@ -9,11 +9,12 @@ 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, is_name_update, is_access_key_update } = require('../manage_nsfs/manage_nsfs_cli_utils');
+const { throw_cli_error, get_options_from_file, get_boolean_or_string_value, get_bucket_owner_account_by_id,
+    is_name_update, is_access_key_update } = 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, UPGRADE_ACTIONS } = require('../manage_nsfs/manage_nsfs_constants');
 const iam_utils = require('../endpoint/iam/iam_utils');
+const { check_root_account_owns_user } = require('../nc/nc_utils');
 
 /////////////////////////////
 //// GENERAL VALIDATIONS ////
@@ -369,7 +370,8 @@ async function validate_bucket_args(config_fs, data, action) {
         }
 
         // bucket owner account validations 
-        const owner_account_data = await get_bucket_owner_account(config_fs, undefined, data.owner_account);
+        const owner_account_data = await get_bucket_owner_account_by_id(config_fs, 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) {

src/nc/nc_utils.js

@@ -0,0 +1,30 @@
+/* Copyright (C) 2024 NooBaa */
+'use strict';
+
+const mongo_utils = require('../util/mongo_utils');
+
+/**
+ * generate_id will generate an id that we use to identify entities (such as account, bucket, etc.). 
+ */
+// TODO: 
+// - reuse this function in NC NSFS where we used the mongo_utils module
+// - this function implantation should be db_client.new_object_id(), 
+//   but to align with manage nsfs we won't change it now
+function generate_id() {
+    return mongo_utils.mongoObjectId();
+}
+
+/**
+ * check_root_account_owns_user checks if an account is owned by root account
+ * @param {object} root_account
+ * @param {object} account
+ */
+function check_root_account_owns_user(root_account, account) {
+    if (account.owner === undefined) return false;
+    return root_account._id === account.owner;
+}
+
+// EXPORTS
+exports.generate_id = generate_id;
+exports.check_root_account_owns_user = check_root_account_owns_user;
+

src/sdk/accountspace_fs.js

@@ -3,17 +3,18 @@
 
 const _ = require('lodash');
 const config = require('../../config');
+const LRUCache = require('../util/lru_cache');
 const dbg = require('../util/debug_module')(__filename);
 const P = require('../util/promise');
-const { ConfigFS } = require('./config_fs');
+const { ConfigFS, CONFIG_TYPES } = require('./config_fs');
 const native_fs_utils = require('../util/native_fs_utils');
 const { create_arn_for_user, create_arn_for_root, get_action_message_title, check_iam_path_was_set } = require('../endpoint/iam/iam_utils');
 const { IAM_ACTIONS, MAX_NUMBER_OF_ACCESS_KEYS, IAM_DEFAULT_PATH,
     ACCESS_KEY_STATUS_ENUM, IDENTITY_ENUM } = require('../endpoint/iam/iam_constants');
 const IamError = require('../endpoint/iam/iam_errors').IamError;
 const cloud_utils = require('../util/cloud_utils');
 const SensitiveString = require('../util/sensitive_string');
-const { generate_id } = require('../manage_nsfs/manage_nsfs_cli_utils');
+const { generate_id } = require('../nc/nc_utils');
 const nc_mkm = require('../manage_nsfs/nc_master_key_manager').get_instance();
 const { account_cache } = require('./object_sdk');
 
@@ -25,6 +26,26 @@ const { account_cache } = require('./object_sdk');
 const dummy_region = 'us-west-2';
 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),
+    /**
+     * @param {{ _id: string, config_fs: import('./config_fs').ConfigFS }} params
+     */
+    make_key: ({ _id }) => _id,
+    load: async ({ _id, config_fs }) => config_fs.get_identity_by_id(_id, CONFIG_TYPES.ACCOUNT,
+        { show_secrets: true, decrypt_secret_key: true }),
+});
+
+
+/**
+ * @param {Object} requested_account
+ */
+function _clean_account_id_cache(requested_account) {
+    account_id_cache.invalidate_key(requested_account._id);
+}
+
 /**
  * @implements {nb.AccountSpace}
  */
@@ -205,6 +226,7 @@ class AccountSpaceFS {
             this._check_if_requested_is_owned_by_root_account(action, requesting_account, account_to_delete);
             await this._check_if_user_does_not_have_resources_before_deletion(action, account_to_delete);
             await this.config_fs.delete_account_config_file(account_to_delete);
+            _clean_account_id_cache(account_to_delete);
         } catch (err) {
             dbg.error(`AccountSpaceFS.${action} error`, err);
             throw native_fs_utils.translate_error_codes(err, native_fs_utils.entity_enum.USER);
@@ -926,8 +948,10 @@ class AccountSpaceFS {
             const access_key_id = access_keys.access_key;
             account_cache.invalidate_key(access_key_id);
         }
+        _clean_account_id_cache(requested_account);
     }
 }
 
 // EXPORTS
 module.exports = AccountSpaceFS;
+module.exports.account_id_cache = account_id_cache;