Skip to content

Commit a57ad85

Browse files
committed
NC | Accounts ID cache addition
Signed-off-by: Romy <35330373+romayalon@users.noreply.github.com>
1 parent f318b76 commit a57ad85

File tree

8 files changed

+128
-77
lines changed

8 files changed

+128
-77
lines changed

config.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -895,6 +895,7 @@ config.NC_DISABLE_HEALTH_ACCESS_CHECK = false;
895895
config.NC_DISABLE_POSIX_MODE_ACCESS_CHECK = true;
896896
config.NC_DISABLE_SCHEMA_CHECK = false;
897897

898+
config.ACCOUNTS_ID_CACHE_EXPIRY = 3 * 60 * 1000;
898899
////////// GPFS //////////
899900
config.GPFS_DOWN_DELAY = 1000;
900901

src/cmd/manage_nsfs.js

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ const cloud_utils = require('../util/cloud_utils');
1313
const native_fs_utils = require('../util/native_fs_utils');
1414
const mongo_utils = require('../util/mongo_utils');
1515
const SensitiveString = require('../util/sensitive_string');
16+
const { account_id_cache } = require('../sdk/accountspace_fs');
1617
const ManageCLIError = require('../manage_nsfs/manage_nsfs_cli_errors').ManageCLIError;
1718
const ManageCLIResponse = require('../manage_nsfs/manage_nsfs_cli_responses').ManageCLIResponse;
1819
const manage_nsfs_glacier = require('../manage_nsfs/manage_nsfs_glacier');
@@ -21,7 +22,8 @@ const noobaa_cli_diagnose = require('../manage_nsfs/diagnose');
2122
const noobaa_cli_upgrade = require('../manage_nsfs/upgrade');
2223
const { print_usage } = require('../manage_nsfs/manage_nsfs_help_utils');
2324
const { TYPES, ACTIONS, LIST_ACCOUNT_FILTERS, LIST_BUCKET_FILTERS, GLACIER_ACTIONS } = require('../manage_nsfs/manage_nsfs_constants');
24-
const { throw_cli_error, get_bucket_owner_account, write_stdout_response, get_boolean_or_string_value, has_access_keys, set_debug_level,
25+
const { throw_cli_error, get_bucket_owner_account_by_name,
26+
write_stdout_response, get_boolean_or_string_value, has_access_keys, set_debug_level,
2527
is_name_update, is_access_key_update } = require('../manage_nsfs/manage_nsfs_cli_utils');
2628
const manage_nsfs_validations = require('../manage_nsfs/manage_nsfs_validations');
2729
const nc_mkm = require('../manage_nsfs/nc_master_key_manager').get_instance();
@@ -122,7 +124,7 @@ async function fetch_bucket_data(action, user_input) {
122124
//if we're updating the owner, needs to override owner in file with the owner from user input.
123125
//if we're adding a bucket, need to set its owner id field
124126
if ((action === ACTIONS.UPDATE && user_input.owner) || (action === ACTIONS.ADD)) {
125-
const account = await get_bucket_owner_account(config_fs, String(user_input.owner));
127+
const account = await get_bucket_owner_account_by_name(config_fs, String(user_input.owner));
126128
data.owner_account = account._id;
127129
}
128130

@@ -597,8 +599,6 @@ async function list_config_files(type, wide, show_secrets, filters = {}) {
597599
entry_names = await config_fs.list_buckets();
598600
}
599601

600-
// temporary cache for mapping bucker owner_account (id) -> bucket_owner (name)
601-
const bucket_owners_map = {};
602602
let config_files_list = await P.map_with_concurrency(10, entry_names, async entry_name => {
603603
if (wide || should_filter) {
604604
const data = type === TYPES.ACCOUNT ?
@@ -610,11 +610,7 @@ async function list_config_files(type, wide, show_secrets, filters = {}) {
610610
if (!wide) return { name: entry_name };
611611
if (type === TYPES.ACCOUNT) return _.omit(data, show_secrets ? [] : ['access_keys']);
612612
if (type === TYPES.BUCKET) {
613-
data.bucket_owner = bucket_owners_map[data.owner_account];
614-
if (!data.bucket_owner) {
615-
await set_bucker_owner(data);
616-
bucket_owners_map[data.owner_account] = data.bucket_owner;
617-
}
613+
await set_bucker_owner(data);
618614
return data;
619615
}
620616
} else {
@@ -657,7 +653,12 @@ function get_access_keys(action, user_input) {
657653
* @param {object} bucket_data
658654
*/
659655
async function set_bucker_owner(bucket_data) {
660-
const account_data = await config_fs.get_identity_by_id(bucket_data.owner_account, TYPES.ACCOUNT, { silent_if_missing: true});
656+
let account_data;
657+
try {
658+
account_data = await account_id_cache.get_with_cache({ _id: bucket_data.owner_account, config_fs });
659+
} catch (err) {
660+
dbg.warn(`set_bucker_owner.couldn't find bucket owner data by id ${bucket_data.owner_account}`);
661+
}
661662
bucket_data.bucket_owner = account_data?.name;
662663
}
663664

src/manage_nsfs/manage_nsfs_cli_utils.js

Lines changed: 27 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -3,15 +3,14 @@
33

44
const dbg = require('../util/debug_module')(__filename);
55
const nb_native = require('../util/nb_native');
6-
const { CONFIG_TYPES } = require('../sdk/config_fs');
76
const native_fs_utils = require('../util/native_fs_utils');
87
const ManageCLIError = require('../manage_nsfs/manage_nsfs_cli_errors').ManageCLIError;
98
const NSFS_CLI_ERROR_EVENT_MAP = require('../manage_nsfs/manage_nsfs_cli_errors').NSFS_CLI_ERROR_EVENT_MAP;
109
const ManageCLIResponse = require('../manage_nsfs/manage_nsfs_cli_responses').ManageCLIResponse;
1110
const NSFS_CLI_SUCCESS_EVENT_MAP = require('../manage_nsfs/manage_nsfs_cli_responses').NSFS_CLI_SUCCESS_EVENT_MAP;
1211
const { BOOLEAN_STRING_VALUES } = require('../manage_nsfs/manage_nsfs_constants');
1312
const NoobaaEvent = require('../manage_nsfs/manage_nsfs_events_utils').NoobaaEvent;
14-
const mongo_utils = require('../util/mongo_utils');
13+
const { account_id_cache } = require('../sdk/accountspace_fs');
1514

1615

1716
function throw_cli_error(error, detail, event_arg) {
@@ -35,29 +34,43 @@ function write_stdout_response(response_code, detail, event_arg) {
3534
}
3635

3736
/**
38-
* get_bucket_owner_account will return the account of the bucket_owner
37+
* get_bucket_owner_account_by_name will return the account of the bucket_owner
3938
* otherwise it would throw an error
4039
* @param {import('../sdk/config_fs').ConfigFS} config_fs
41-
* @param {string} [bucket_owner]
42-
* @param {string} [owner_account_id]
40+
* @param {string} bucket_owner
4341
*/
44-
async function get_bucket_owner_account(config_fs, bucket_owner, owner_account_id) {
42+
async function get_bucket_owner_account_by_name(config_fs, bucket_owner) {
4543
try {
46-
const account = bucket_owner ?
47-
await config_fs.get_account_by_name(bucket_owner) :
48-
await config_fs.get_identity_by_id(owner_account_id, CONFIG_TYPES.ACCOUNT);
44+
const account = await config_fs.get_account_by_name(bucket_owner);
4945
return account;
5046
} catch (err) {
5147
if (err.code === 'ENOENT') {
52-
const detail_msg = bucket_owner ?
53-
`bucket owner name ${bucket_owner} does not exists` :
54-
`bucket owner id ${owner_account_id} does not exists`;
48+
const detail_msg = `bucket owner name ${bucket_owner} does not exist`;
5549
throw_cli_error(ManageCLIError.BucketSetForbiddenBucketOwnerNotExists, detail_msg, {bucket_owner: bucket_owner});
5650
}
5751
throw err;
5852
}
5953
}
6054

55+
/**
56+
* get_bucket_owner_account_by_id will return the account of the owner_account id
57+
* otherwise it would throw an error
58+
* @param {import('../sdk/config_fs').ConfigFS} config_fs
59+
* @param {string} owner_account
60+
*/
61+
async function get_bucket_owner_account_by_id(config_fs, owner_account) {
62+
try {
63+
const account = await account_id_cache.get_with_cache({ _id: owner_account, config_fs });
64+
return account;
65+
} catch (err) {
66+
if (err.code === 'ENOENT') {
67+
const detail_msg = `bucket owner account id ${owner_account} does not exist`;
68+
throw_cli_error(ManageCLIError.BucketSetForbiddenBucketOwnerNotExists, detail_msg, { owner_account: owner_account });
69+
}
70+
throw err;
71+
}
72+
}
73+
6174
/**
6275
* get_boolean_or_string_value will check if the value
6376
* 1. if the value is undefined - it returns false.
@@ -113,28 +126,6 @@ function set_debug_level(debug) {
113126
nb_native().fs.set_debug_level(debug_level);
114127
}
115128

116-
/**
117-
* generate_id will generate an id that we use to identify entities (such as account, bucket, etc.).
118-
*/
119-
// TODO:
120-
// - reuse this function in NC NSFS where we used the mongo_utils module
121-
// - this function implantation should be db_client.new_object_id(),
122-
// but to align with manage nsfs we won't change it now
123-
function generate_id() {
124-
return mongo_utils.mongoObjectId();
125-
}
126-
127-
/**
128-
* check_root_account_owns_user checks if an account is owned by root account
129-
* @param {object} root_account
130-
* @param {object} account
131-
*/
132-
function check_root_account_owns_user(root_account, account) {
133-
if (account.owner === undefined) return false;
134-
return root_account._id === account.owner;
135-
}
136-
137-
138129
/**
139130
* is_name_update returns true if a new_name flag was provided and it's not equal to
140131
* the current name
@@ -163,11 +154,10 @@ function is_access_key_update(data) {
163154
exports.throw_cli_error = throw_cli_error;
164155
exports.write_stdout_response = write_stdout_response;
165156
exports.get_boolean_or_string_value = get_boolean_or_string_value;
166-
exports.get_bucket_owner_account = get_bucket_owner_account;
157+
exports.get_bucket_owner_account_by_name = get_bucket_owner_account_by_name;
158+
exports.get_bucket_owner_account_by_id = get_bucket_owner_account_by_id;
167159
exports.get_options_from_file = get_options_from_file;
168160
exports.has_access_keys = has_access_keys;
169-
exports.generate_id = generate_id;
170161
exports.set_debug_level = set_debug_level;
171-
exports.check_root_account_owns_user = check_root_account_owns_user;
172162
exports.is_name_update = is_name_update;
173163
exports.is_access_key_update = is_access_key_update;

src/manage_nsfs/manage_nsfs_logging.js

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
const AWS = require('aws-sdk');
55
const config = require('../../config');
66
const http_utils = require('../util/http_utils');
7-
const { CONFIG_TYPES } = require('../sdk/config_fs');
7+
const { account_id_cache } = require('../sdk/accountspace_fs');
88
const { export_logs_to_target } = require('../util/bucket_logs_utils');
99
const ManageCLIError = require('../manage_nsfs/manage_nsfs_cli_errors').ManageCLIError;
1010
const ManageCLIResponse = require('../manage_nsfs/manage_nsfs_cli_responses').ManageCLIResponse;
@@ -40,10 +40,15 @@ async function export_bucket_logging(shared_config_fs) {
4040
*/
4141
async function get_bucket_owner_keys(log_bucket_name) {
4242
const log_bucket_config_data = await config_fs.get_bucket_by_name(log_bucket_name);
43-
const log_bucket_owner = log_bucket_config_data.owner_account;
44-
const owner_config_data = await config_fs.get_identity_by_id(log_bucket_owner,
45-
CONFIG_TYPES.ACCOUNT, { show_secrets: true, decrypt_secret_key: true });
46-
return owner_config_data.access_keys;
43+
const log_bucket_owner_id = log_bucket_config_data.owner_account;
44+
try {
45+
const owner_config_data = await account_id_cache.get_with_cache({ _id: log_bucket_owner_id, config_fs });
46+
return owner_config_data.access_keys;
47+
} catch (err) {
48+
throw_cli_error(ManageCLIError.BucketSetForbiddenBucketOwnerNotExists,
49+
`could not find log bucket owner by id ${log_bucket_owner_id}, can not extract owner access keys`,
50+
{ owner_account: log_bucket_owner_id });
51+
}
4752
}
4853

4954
exports.export_bucket_logging = export_bucket_logging;

src/manage_nsfs/manage_nsfs_validations.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,12 @@ const string_utils = require('../util/string_utils');
99
const native_fs_utils = require('../util/native_fs_utils');
1010
const ManageCLIError = require('../manage_nsfs/manage_nsfs_cli_errors').ManageCLIError;
1111
const bucket_policy_utils = require('../endpoint/s3/s3_bucket_policy_utils');
12-
const { throw_cli_error, get_bucket_owner_account, get_options_from_file, get_boolean_or_string_value,
13-
check_root_account_owns_user, is_name_update, is_access_key_update } = require('../manage_nsfs/manage_nsfs_cli_utils');
12+
const { throw_cli_error, get_options_from_file, get_boolean_or_string_value, get_bucket_owner_account_by_id,
13+
is_name_update, is_access_key_update } = require('../manage_nsfs/manage_nsfs_cli_utils');
1414
const { TYPES, ACTIONS, VALID_OPTIONS, OPTION_TYPE, FROM_FILE, BOOLEAN_STRING_VALUES, BOOLEAN_STRING_OPTIONS,
1515
GLACIER_ACTIONS, LIST_UNSETABLE_OPTIONS, ANONYMOUS, DIAGNOSE_ACTIONS, UPGRADE_ACTIONS } = require('../manage_nsfs/manage_nsfs_constants');
1616
const iam_utils = require('../endpoint/iam/iam_utils');
17+
const { check_root_account_owns_user } = require('../nc/nc_utils');
1718

1819
/////////////////////////////
1920
//// GENERAL VALIDATIONS ////
@@ -369,7 +370,8 @@ async function validate_bucket_args(config_fs, data, action) {
369370
}
370371

371372
// bucket owner account validations
372-
const owner_account_data = await get_bucket_owner_account(config_fs, undefined, data.owner_account);
373+
const owner_account_data = await get_bucket_owner_account_by_id(config_fs, data.owner_account);
374+
373375
const account_fs_context = await native_fs_utils.get_fs_context(owner_account_data.nsfs_account_config,
374376
owner_account_data.nsfs_account_config.fs_backend);
375377
if (!config.NC_DISABLE_ACCESS_CHECK) {

src/nc/nc_utils.js

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
/* Copyright (C) 2024 NooBaa */
2+
'use strict';
3+
4+
const mongo_utils = require('../util/mongo_utils');
5+
6+
/**
7+
* generate_id will generate an id that we use to identify entities (such as account, bucket, etc.).
8+
*/
9+
// TODO:
10+
// - reuse this function in NC NSFS where we used the mongo_utils module
11+
// - this function implantation should be db_client.new_object_id(),
12+
// but to align with manage nsfs we won't change it now
13+
function generate_id() {
14+
return mongo_utils.mongoObjectId();
15+
}
16+
17+
/**
18+
* check_root_account_owns_user checks if an account is owned by root account
19+
* @param {object} root_account
20+
* @param {object} account
21+
*/
22+
function check_root_account_owns_user(root_account, account) {
23+
if (account.owner === undefined) return false;
24+
return root_account._id === account.owner;
25+
}
26+
27+
// EXPORTS
28+
exports.generate_id = generate_id;
29+
exports.check_root_account_owns_user = check_root_account_owns_user;
30+

src/sdk/accountspace_fs.js

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,17 +3,18 @@
33

44
const _ = require('lodash');
55
const config = require('../../config');
6+
const LRUCache = require('../util/lru_cache');
67
const dbg = require('../util/debug_module')(__filename);
78
const P = require('../util/promise');
8-
const { ConfigFS } = require('./config_fs');
9+
const { ConfigFS, CONFIG_TYPES } = require('./config_fs');
910
const native_fs_utils = require('../util/native_fs_utils');
1011
const { create_arn_for_user, create_arn_for_root, get_action_message_title, check_iam_path_was_set } = require('../endpoint/iam/iam_utils');
1112
const { IAM_ACTIONS, MAX_NUMBER_OF_ACCESS_KEYS, IAM_DEFAULT_PATH,
1213
ACCESS_KEY_STATUS_ENUM, IDENTITY_ENUM } = require('../endpoint/iam/iam_constants');
1314
const IamError = require('../endpoint/iam/iam_errors').IamError;
1415
const cloud_utils = require('../util/cloud_utils');
1516
const SensitiveString = require('../util/sensitive_string');
16-
const { generate_id } = require('../manage_nsfs/manage_nsfs_cli_utils');
17+
const { generate_id } = require('../nc/nc_utils');
1718
const nc_mkm = require('../manage_nsfs/nc_master_key_manager').get_instance();
1819
const { account_cache } = require('./object_sdk');
1920

@@ -25,6 +26,26 @@ const { account_cache } = require('./object_sdk');
2526
const dummy_region = 'us-west-2';
2627
const dummy_service_name = 's3';
2728

29+
const account_id_cache = new LRUCache({
30+
name: 'AccountIDCache',
31+
// TODO: Decide on a time that we want to invalidate
32+
expiry_ms: Number(config.ACCOUNTS_ID_CACHE_EXPIRY),
33+
/**
34+
* @param {{ _id: string, config_fs: import('./config_fs').ConfigFS }} params
35+
*/
36+
make_key: ({ _id }) => _id,
37+
load: async ({ _id, config_fs }) => config_fs.get_identity_by_id(_id, CONFIG_TYPES.ACCOUNT,
38+
{ show_secrets: true, decrypt_secret_key: true }),
39+
});
40+
41+
42+
/**
43+
* @param {Object} requested_account
44+
*/
45+
function _clean_account_id_cache(requested_account) {
46+
account_id_cache.invalidate_key(requested_account._id);
47+
}
48+
2849
/**
2950
* @implements {nb.AccountSpace}
3051
*/
@@ -205,6 +226,7 @@ class AccountSpaceFS {
205226
this._check_if_requested_is_owned_by_root_account(action, requesting_account, account_to_delete);
206227
await this._check_if_user_does_not_have_resources_before_deletion(action, account_to_delete);
207228
await this.config_fs.delete_account_config_file(account_to_delete);
229+
_clean_account_id_cache(account_to_delete);
208230
} catch (err) {
209231
dbg.error(`AccountSpaceFS.${action} error`, err);
210232
throw native_fs_utils.translate_error_codes(err, native_fs_utils.entity_enum.USER);
@@ -926,8 +948,10 @@ class AccountSpaceFS {
926948
const access_key_id = access_keys.access_key;
927949
account_cache.invalidate_key(access_key_id);
928950
}
951+
_clean_account_id_cache(requested_account);
929952
}
930953
}
931954

932955
// EXPORTS
933956
module.exports = AccountSpaceFS;
957+
module.exports.account_id_cache = account_id_cache;

0 commit comments

Comments
 (0)