Skip to content

Commit ab255a7

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

File tree

6 files changed

+271
-224
lines changed

6 files changed

+271
-224
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: 56 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ const os_utils = require('../util/os_utils');
99
const nb_native = require('../util/nb_native');
1010
const native_fs_utils = require('../util/native_fs_utils');
1111
const nc_mkm = require('../manage_nsfs/nc_master_key_manager').get_instance();
12+
const nsfs_schema_utils = require('../manage_nsfs/nsfs_schema_utils');
1213

1314
/* Config directory sub directory comments -
1415
On 5.18 -
@@ -518,7 +519,7 @@ class ConfigFS {
518519
*/
519520
async create_account_config_file(account_data) {
520521
const { _id, name, owner = undefined } = account_data;
521-
const data_string = JSON.stringify(account_data);
522+
const data_string = await this._prepare_for_account_schema(account_data);
522523
const account_path = this.get_identity_path_by_id(_id);
523524
const account_dir_path = this.get_identity_dir_path_by_id(_id);
524525

@@ -544,8 +545,8 @@ class ConfigFS {
544545
* @returns {Promise<void>}
545546
*/
546547
async update_account_config_file(account_new_data, options = {}) {
547-
const { _id, name, owner = undefined } = account_new_data;
548-
const data_string = JSON.stringify(account_new_data);
548+
const { _id, name, owner = undefined } = account_new_data;
549+
const data_string = await this._prepare_for_account_schema(account_new_data);
549550
const account_path = this.get_identity_path_by_id(_id);
550551
const account_dir_path = this.get_identity_dir_path_by_id(_id);
551552
await native_fs_utils.update_config_file(this.fs_context, account_dir_path, account_path, data_string);
@@ -742,24 +743,63 @@ class ConfigFS {
742743

743744
/**
744745
* create_bucket_config_file creates bucket config file
745-
* @param {string} bucket_name
746-
* @param {*} data
747-
* @returns {Promise<void>}
746+
* @param {Object} bucket_data
747+
* @returns {Promise<String>}
748748
*/
749-
async create_bucket_config_file(bucket_name, data) {
750-
const bucket_path = this.get_bucket_path_by_name(bucket_name);
751-
await native_fs_utils.create_config_file(this.fs_context, this.buckets_dir_path, bucket_path, data);
749+
async create_bucket_config_file(bucket_data) {
750+
const bucket_string_data = this._prepare_for_bucket_schema(bucket_data);
751+
const bucket_path = this.get_bucket_path_by_name(bucket_data.name);
752+
await native_fs_utils.create_config_file(this.fs_context, this.buckets_dir_path, bucket_path, bucket_string_data);
753+
return bucket_string_data;
752754
}
753755

754756
/**
755-
* update_bucket_config_file updates bucket config file
756-
* @param {string} bucket_name
757-
* @param {*} data
758-
* @returns {Promise<void>}
757+
* _prepare_for_bucket_schema takes bucket data -
758+
* 1. removes API bucket properties
759+
* 2. removes undefined properties, unwrap sensitive_strings and creation_data to string
760+
* 3. checks bucket schema validation
761+
* 4. and returns stringified data ready to be written to the config directory
762+
* @param {Object} bucket_data
763+
* @returns {String}
759764
*/
760-
async update_bucket_config_file(bucket_name, data) {
761-
const bucket_config_path = this.get_bucket_path_by_name(bucket_name);
762-
await native_fs_utils.update_config_file(this.fs_context, this.buckets_dir_path, bucket_config_path, data);
765+
_prepare_for_bucket_schema(bucket_data) {
766+
const api_bucket_properties_to_remove = ['new_name'];
767+
const bucket_data_api_props_omitted = _.omit(bucket_data, api_bucket_properties_to_remove);
768+
const bucket_string_data = JSON.stringify(bucket_data_api_props_omitted);
769+
nsfs_schema_utils.validate_bucket_schema(JSON.parse(bucket_string_data));
770+
return bucket_string_data;
771+
}
772+
773+
/**
774+
* _prepare_for_account_schema takes account data -
775+
* 1. encrypts its access keys
776+
* 2. sets the used master key on the account
777+
* 3. removes API account properties
778+
* 4. removes undefined properties, unwrap sensitive_strings and creation_data to string
779+
* 5. checks accpimt schema validation
780+
* 6. and returns stringified data ready to be written to the config directory
781+
* @param {Object} account_data
782+
* @returns {Promise<String>}
783+
*/
784+
async _prepare_for_account_schema(account_data) {
785+
const encrypted_account = await nc_mkm.encrypt_access_keys(account_data);
786+
const api_account_properties_to_remove = ['new_name', 'new_access_key'];
787+
const account_data_api_props_omitted = _.omit(encrypted_account, api_account_properties_to_remove);
788+
const account_string_data = JSON.stringify(account_data_api_props_omitted);
789+
nsfs_schema_utils.validate_account_schema(JSON.parse(account_string_data));
790+
return account_string_data;
791+
}
792+
793+
/**
794+
* update_bucket_config_file updates bucket config file
795+
* @param {Object} bucket_data
796+
* @returns {Promise<String>}
797+
*/
798+
async update_bucket_config_file(bucket_data) {
799+
const bucket_string_data = this._prepare_for_bucket_schema(bucket_data);
800+
const bucket_config_path = this.get_bucket_path_by_name(bucket_data.name);
801+
await native_fs_utils.update_config_file(this.fs_context, this.buckets_dir_path, bucket_config_path, bucket_string_data);
802+
return bucket_string_data;
763803
}
764804

765805
/**

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)