Skip to content

Commit 179ba4d

Browse files
committed
NC | More Refactoring config_fs
Signed-off-by: Romy <[email protected]>
1 parent a56cb25 commit 179ba4d

File tree

6 files changed

+275
-225
lines changed

6 files changed

+275
-225
lines changed

src/cmd/manage_nsfs.js

Lines changed: 153 additions & 176 deletions
Large diffs are not rendered by default.

src/manage_nsfs/manage_nsfs_cli_utils.js

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -129,6 +129,30 @@ function check_root_account_owns_user(root_account, account) {
129129
}
130130

131131

132+
/**
133+
* _is_name_update returns true if a new_name flag was provided and it's not equal than
134+
* the current name,
135+
* @param {Object} data
136+
* @returns {Boolean}
137+
*/
138+
function _is_name_update(data) {
139+
const cur_name = data.name;
140+
const new_name = data.new_name;
141+
return new_name && cur_name && new_name !== cur_name;
142+
}
143+
144+
/**
145+
* _is_access_key_update returns true if a new_access_key flag was provided and it's not equal than
146+
* the current access_key,
147+
* @param {Object} data
148+
* @returns {Boolean}
149+
*/
150+
function _is_access_key_update(data) {
151+
const cur_access_key = has_access_keys(data.access_keys) ? data.access_keys[0].access_key.unwrap() : undefined;
152+
const new_access_key = data.new_access_key;
153+
return new_access_key && cur_access_key && new_access_key !== cur_access_key;
154+
}
155+
132156
// EXPORTS
133157
exports.throw_cli_error = throw_cli_error;
134158
exports.write_stdout_response = write_stdout_response;
@@ -139,3 +163,5 @@ exports.has_access_keys = has_access_keys;
139163
exports.generate_id = generate_id;
140164
exports.set_debug_level = set_debug_level;
141165
exports.check_root_account_owns_user = check_root_account_owns_user;
166+
exports._is_name_update = _is_name_update;
167+
exports._is_access_key_update = _is_access_key_update;

src/manage_nsfs/manage_nsfs_validations.js

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ 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');
1212
const { throw_cli_error, get_bucket_owner_account, get_options_from_file, get_boolean_or_string_value,
13-
check_root_account_owns_user } = require('../manage_nsfs/manage_nsfs_cli_utils');
13+
check_root_account_owns_user, _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 } = require('../manage_nsfs/manage_nsfs_constants');
1616
const iam_utils = require('../endpoint/iam/iam_utils');
@@ -306,6 +306,22 @@ if (action === ACTIONS.STATUS || action === ACTIONS.ADD || action === ACTIONS.UP
306306
// in list there is no identifier
307307
}
308308

309+
/**
310+
* check_new_bucket_name_exists will validate that we have the needed identifier for the command
311+
* @param {import('../sdk/config_fs').ConfigFS} config_fs
312+
* @param {string} action
313+
* @param {object} data
314+
*/
315+
async function check_new_bucket_name_exists(config_fs, action, data) {
316+
let new_bucket_name = data.name;
317+
if (action === ACTIONS.UPDATE) {
318+
if (!data.new_name) return;
319+
new_bucket_name = data.new_name;
320+
}
321+
const exists = await config_fs.is_bucket_exists(new_bucket_name);
322+
if (exists) throw_cli_error(ManageCLIError.BucketAlreadyExists, new_bucket_name, { bucket: new_bucket_name });
323+
}
324+
309325
/**
310326
* validate_bucket_args will validate the cli args of the bucket command
311327
* @param {import('../sdk/config_fs').ConfigFS} config_fs
@@ -322,6 +338,7 @@ async function validate_bucket_args(config_fs, data, action) {
322338
if (data.fs_backend !== undefined && !['GPFS', 'CEPH_FS', 'NFSv4'].includes(data.fs_backend)) {
323339
throw_cli_error(ManageCLIError.InvalidFSBackend);
324340
}
341+
await check_new_bucket_name_exists(config_fs, action, data);
325342
// in case we have the fs_backend it changes the fs_context that we use for the path
326343
const fs_context_fs_backend = native_fs_utils.get_process_fs_context(data.fs_backend);
327344
const exists = await native_fs_utils.is_path_exists(fs_context_fs_backend, data.path);
@@ -393,6 +410,15 @@ function validate_account_identifier(action, input_options) {
393410
*/
394411
async function validate_account_args(config_fs, data, action, is_flag_iam_operate_on_root_account_update_action) {
395412
if (action === ACTIONS.ADD || action === ACTIONS.UPDATE) {
413+
const update_name = _is_name_update(data);
414+
const update_access_key = _is_access_key_update(data);
415+
416+
const name_exists = update_name && await config_fs.is_account_exists_by_name(data.new_name);
417+
const access_key_exists = update_access_key && await config_fs.is_account_exists_by_access_key(data.new_access_keys);
418+
if (name_exists || access_key_exists) {
419+
const err_code = name_exists ? ManageCLIError.AccountNameAlreadyExists : ManageCLIError.AccountAccessKeyAlreadyExists;
420+
throw_cli_error(err_code);
421+
}
396422
if (data.nsfs_account_config.gid && data.nsfs_account_config.uid === undefined) {
397423
throw_cli_error(ManageCLIError.MissingAccountNSFSConfigUID, data.nsfs_account_config);
398424
}

src/sdk/accountspace_fs.js

Lines changed: 7 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -10,12 +10,10 @@ const native_fs_utils = require('../util/native_fs_utils');
1010
const { create_arn, get_action_message_title, check_iam_path_was_set } = require('../endpoint/iam/iam_utils');
1111
const { IAM_ACTIONS, MAX_NUMBER_OF_ACCESS_KEYS, IAM_DEFAULT_PATH,
1212
ACCESS_KEY_STATUS_ENUM, IDENTITY_ENUM } = require('../endpoint/iam/iam_constants');
13-
const nsfs_schema_utils = require('../manage_nsfs/nsfs_schema_utils');
1413
const IamError = require('../endpoint/iam/iam_errors').IamError;
1514
const cloud_utils = require('../util/cloud_utils');
1615
const SensitiveString = require('../util/sensitive_string');
1716
const { generate_id } = require('../manage_nsfs/manage_nsfs_cli_utils');
18-
const nc_mkm = require('../manage_nsfs/nc_master_key_manager').get_instance();
1917
const { account_cache } = require('./object_sdk');
2018

2119

@@ -150,10 +148,7 @@ class AccountSpaceFS {
150148
is_username_update);
151149
await this._update_account_config_new_username(action, params, requested_account);
152150
} else {
153-
const requested_account_encrypted = await nc_mkm.encrypt_access_keys(requested_account);
154-
const account_string = JSON.stringify(requested_account_encrypted);
155-
nsfs_schema_utils.validate_account_schema(JSON.parse(account_string));
156-
await this.config_fs.update_account_config_file(JSON.parse(account_string));
151+
await this.config_fs.update_account_config_file(requested_account);
157152
}
158153
this._clean_account_cache(requested_account);
159154
return {
@@ -265,11 +260,8 @@ class AccountSpaceFS {
265260
deactivated: false,
266261
};
267262
requested_account.access_keys.push(created_access_key_obj);
268-
const requested_account_encrypted = await nc_mkm.encrypt_access_keys(requested_account);
269-
const account_to_create_access_keys_string = JSON.stringify(requested_account_encrypted);
270-
nsfs_schema_utils.validate_account_schema(JSON.parse(account_to_create_access_keys_string));
271263
await this.config_fs.update_account_config_file(
272-
JSON.parse(account_to_create_access_keys_string),
264+
requested_account,
273265
{ new_access_keys_to_link: [created_access_key_obj] }
274266
);
275267
return {
@@ -355,10 +347,7 @@ class AccountSpaceFS {
355347
return;
356348
}
357349
access_key_obj.deactivated = this._check_access_key_is_deactivated(params.status);
358-
const requested_account_encrypted = await nc_mkm.encrypt_access_keys(requested_account);
359-
const account_string = JSON.stringify(requested_account_encrypted);
360-
nsfs_schema_utils.validate_account_schema(JSON.parse(account_string));
361-
await this.config_fs.update_account_config_file(JSON.parse(account_string));
350+
await this.config_fs.update_account_config_file(requested_account);
362351
this._clean_account_cache(requested_account);
363352
} catch (err) {
364353
dbg.error(`AccountSpaceFS.${action} error`, err);
@@ -398,11 +387,8 @@ class AccountSpaceFS {
398387
}
399388
requested_account.access_keys = requested_account.access_keys.filter(access_key_obj =>
400389
access_key_obj.access_key !== access_key_id);
401-
const requested_account_encrypted = await nc_mkm.encrypt_access_keys(requested_account);
402-
const account_string = JSON.stringify(requested_account_encrypted);
403-
nsfs_schema_utils.validate_account_schema(JSON.parse(account_string));
404390
await this.config_fs.update_account_config_file(
405-
JSON.parse(account_string),
391+
requested_account,
406392
{ access_keys_to_delete: [{ access_key: access_key_id }] }
407393
);
408394
this._clean_account_cache(requested_account);
@@ -625,12 +611,9 @@ class AccountSpaceFS {
625611
}
626612

627613
async _copy_data_from_requesting_account_to_account_config(action, requesting_account, params) {
628-
const master_key_id = await nc_mkm.get_active_master_key_id();
629-
const created_account = this._new_user_defaults(requesting_account, params, master_key_id);
614+
const created_account = this._new_user_defaults(requesting_account, params);
630615
dbg.log1(`AccountSpaceFS.${action} new_account`, created_account);
631-
const new_account_string = JSON.stringify(created_account);
632-
nsfs_schema_utils.validate_account_schema(JSON.parse(new_account_string));
633-
await this.config_fs.create_account_config_file(JSON.parse(new_account_string));
616+
await this.config_fs.create_account_config_file(created_account);
634617
return created_account;
635618
}
636619

@@ -665,8 +648,6 @@ class AccountSpaceFS {
665648
this._check_if_user_does_not_have_access_keys_before_deletion(action, account_to_delete);
666649
}
667650

668-
// TODO - when we have the structure of config we can check easily which buckets are owned by the root account
669-
// currently, partial copy from verify_account_not_owns_bucket
670651
async _check_if_root_account_does_not_have_buckets_before_deletion(action, account_to_delete) {
671652
const resource_name = 'buckets';
672653
const bucket_names = await this.config_fs.list_buckets();
@@ -710,10 +691,7 @@ class AccountSpaceFS {
710691
requested_account.name = params.new_username;
711692
requested_account.email = params.new_username; // internally saved
712693
// handle account config creation
713-
const requested_account_encrypted = await nc_mkm.encrypt_access_keys(requested_account);
714-
const account_string = JSON.stringify(requested_account_encrypted);
715-
nsfs_schema_utils.validate_account_schema(JSON.parse(account_string));
716-
await this.config_fs.update_account_config_file(JSON.parse(account_string), { old_name: params.username });
694+
await this.config_fs.update_account_config_file(requested_account, { old_name: params.username });
717695
}
718696

719697
_check_root_account_or_user(requesting_account, username) {

src/sdk/config_fs.js

Lines changed: 60 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,8 @@ const nb_native = require('../util/nb_native');
99
const native_fs_utils = require('../util/native_fs_utils');
1010
const nc_mkm = require('../manage_nsfs/nc_master_key_manager').get_instance();
1111
const { TYPES } = require('../manage_nsfs/manage_nsfs_constants');
12+
const nsfs_schema_utils = require('../manage_nsfs/nsfs_schema_utils');
13+
const { IS_MAC } = require('../util/os_utils');
1214

1315
/* Config directory sub directory comments -
1416
On 5.18 -
@@ -517,7 +519,7 @@ class ConfigFS {
517519
*/
518520
async create_account_config_file(account_data) {
519521
const { name, _id, owner = undefined } = account_data;
520-
const data_string = JSON.stringify(account_data);
522+
const data_string = await this._prepare_for_account_schema(account_data);
521523
const account_path = this.get_identity_path_by_id(_id);
522524
const account_dir_path = this.get_identity_dir_path_by_id(_id);
523525

@@ -544,7 +546,7 @@ class ConfigFS {
544546
*/
545547
async update_account_config_file(account_new_data, options = {}) {
546548
const { name, _id, owner = undefined } = account_new_data;
547-
const data_string = JSON.stringify(account_new_data);
549+
const data_string = await this._prepare_for_account_schema(account_new_data);
548550
const account_path = this.get_identity_path_by_id(_id);
549551
const account_dir_path = this.get_identity_dir_path_by_id(_id);
550552
await native_fs_utils.update_config_file(this.fs_context, account_dir_path, account_path, data_string);
@@ -607,7 +609,8 @@ class ConfigFS {
607609
async unlink_account_name_index(account_name, account_id_config_path) {
608610
const account_name_path = this.get_account_path_by_name(account_name);
609611
const full_path = await nb_native().fs.realpath(this.fs_context, account_name_path);
610-
if (full_path === account_id_config_path) {
612+
if (full_path === account_id_config_path ||
613+
(IS_MAC && full_path === path.join('/private/', account_id_config_path))) {
611614
await nb_native().fs.unlink(this.fs_context, account_name_path);
612615
}
613616
}
@@ -639,7 +642,8 @@ class ConfigFS {
639642
async unlink_access_key_index(access_key, account_id_config_path) {
640643
const access_key_path = this.get_account_or_user_path_by_access_key(access_key);
641644
const full_path = await nb_native().fs.realpath(this.fs_context, access_key_path);
642-
if (full_path === account_id_config_path) {
645+
if (full_path === account_id_config_path ||
646+
(IS_MAC && full_path === path.join('/private/', account_id_config_path))) {
643647
await nb_native().fs.unlink(this.fs_context, access_key_path);
644648
}
645649
}
@@ -709,24 +713,63 @@ class ConfigFS {
709713

710714
/**
711715
* create_bucket_config_file creates bucket config file
712-
* @param {string} bucket_name
713-
* @param {*} data
714-
* @returns {Promise<void>}
716+
* @param {Object} bucket_data
717+
* @returns {Promise<String>}
715718
*/
716-
async create_bucket_config_file(bucket_name, data) {
717-
const bucket_path = this.get_bucket_path_by_name(bucket_name);
718-
await native_fs_utils.create_config_file(this.fs_context, this.buckets_dir_path, bucket_path, data);
719+
async create_bucket_config_file(bucket_data) {
720+
const bucket_string_data = this._prepare_for_bucket_schema(bucket_data);
721+
const bucket_path = this.get_bucket_path_by_name(bucket_data.name);
722+
await native_fs_utils.create_config_file(this.fs_context, this.buckets_dir_path, bucket_path, bucket_string_data);
723+
return bucket_string_data;
719724
}
720725

721726
/**
722-
* update_bucket_config_file updates bucket config file
723-
* @param {string} bucket_name
724-
* @param {*} data
725-
* @returns {Promise<void>}
727+
* _prepare_for_bucket_schema takes bucket data -
728+
* 1. removes API bucket properties
729+
* 2. removes undefined properties, unwrap sensitive_strings and creation_data to string
730+
* 3. checks bucket schema validation
731+
* 4. and returns stringified data ready to be written to the config directory
732+
* @param {Object} bucket_data
733+
* @returns {String}
726734
*/
727-
async update_bucket_config_file(bucket_name, data) {
728-
const bucket_config_path = this.get_bucket_path_by_name(bucket_name);
729-
await native_fs_utils.update_config_file(this.fs_context, this.buckets_dir_path, bucket_config_path, data);
735+
_prepare_for_bucket_schema(bucket_data) {
736+
const api_bucket_properties_to_remove = ['new_name'];
737+
const bucket_data_api_props_omitted = _.omit(bucket_data, api_bucket_properties_to_remove);
738+
const bucket_string_data = JSON.stringify(bucket_data_api_props_omitted);
739+
nsfs_schema_utils.validate_bucket_schema(JSON.parse(bucket_string_data));
740+
return bucket_string_data;
741+
}
742+
743+
/**
744+
* _prepare_for_account_schema takes account data -
745+
* 1. encrypts its access keys
746+
* 2. sets the used master key on the account
747+
* 3. removes API account properties
748+
* 4. removes undefined properties, unwrap sensitive_strings and creation_data to string
749+
* 5. checks accpimt schema validation
750+
* 6. and returns stringified data ready to be written to the config directory
751+
* @param {Object} account_data
752+
* @returns {Promise<String>}
753+
*/
754+
async _prepare_for_account_schema(account_data) {
755+
const encrypted_account = await nc_mkm.encrypt_access_keys(account_data);
756+
const api_account_properties_to_remove = ['new_name', 'new_access_key'];
757+
const account_data_api_props_omitted = _.omit(encrypted_account, api_account_properties_to_remove);
758+
const account_string_data = JSON.stringify(account_data_api_props_omitted);
759+
nsfs_schema_utils.validate_account_schema(JSON.parse(account_string_data));
760+
return account_string_data;
761+
}
762+
763+
/**
764+
* update_bucket_config_file updates bucket config file
765+
* @param {Object} bucket_data
766+
* @returns {Promise<String>}
767+
*/
768+
async update_bucket_config_file(bucket_data) {
769+
const bucket_string_data = this._prepare_for_bucket_schema(bucket_data);
770+
const bucket_config_path = this.get_bucket_path_by_name(bucket_data.name);
771+
await native_fs_utils.update_config_file(this.fs_context, this.buckets_dir_path, bucket_config_path, bucket_string_data);
772+
return bucket_string_data;
730773
}
731774

732775
/**

src/test/unit_tests/test_nc_nsfs_health.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ mocha.describe('nsfs nc health', function() {
125125
await fs_utils.file_must_exist(new_buckets_path + '/bucket1');
126126
await config_fs.create_config_dirs_if_missing();
127127
await config_fs.create_account_config_file(account1);
128-
await config_fs.create_bucket_config_file(bucket1.name, JSON.stringify(bucket1));
128+
await config_fs.create_bucket_config_file(bucket1);
129129
const get_service_memory_usage = sinon.stub(Health, "get_service_memory_usage");
130130
get_service_memory_usage.onFirstCall().returns(Promise.resolve(100));
131131
for (const user of Object.values(fs_users)) {
@@ -224,7 +224,7 @@ mocha.describe('nsfs nc health', function() {
224224
Health.get_service_state.restore();
225225
Health.get_endpoint_response.restore();
226226
const bucket_invalid_schema = { name: 'bucket_invalid_schema', path: new_buckets_path };
227-
await config_fs.create_bucket_config_file(bucket_invalid_schema.name, JSON.stringify(bucket_invalid_schema) + 'invalid');
227+
await config_fs.create_bucket_config_file(bucket_invalid_schema);
228228
const get_service_state = sinon.stub(Health, "get_service_state");
229229
get_service_state.onFirstCall().returns(Promise.resolve({ service_status: 'active', pid: 1000 }))
230230
.onSecondCall().returns(Promise.resolve({ service_status: 'active', pid: 2000 }));

0 commit comments

Comments
 (0)