Skip to content

Commit

Permalink
NSFS | NC | Add flag to change the value of allow_bucket_creation exp…
Browse files Browse the repository at this point in the history 
…licitly

1. Add a flag to change the value of allow_bucket_creation, and add a new error InvalidAllowBucketCreation.
2. Edit details message when BucketCreationNotAllowed is thrown.
3. Add the option for boolean flags to pass 'true' and 'false' (wide, show_secrets, regenerate), and add a new error InvalidBooleanValue.
4. Reduce the number of spaces in the help menu and change the style of the glacier to match the existing style.

Signed-off-by: shirady <57721533+shirady@users.noreply.github.com>
  • Loading branch information
@shirady
shirady committed Mar 4, 2024
1 parent c0da9b5 commit 2a3d0a9
Show file tree
Hide file tree
Showing 7 changed files with 486 additions and 76 deletions.
84 changes: 74 additions & 10 deletions src/cmd/manage_nsfs.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ const manage_nsfs_glacier = require('../manage_nsfs/manage_nsfs_glacier');
const bucket_policy_utils = require('../endpoint/s3/s3_bucket_policy_utils');
const nsfs_schema_utils = require('../manage_nsfs/nsfs_schema_utils');
const { print_usage } = require('../manage_nsfs/manage_nsfs_help_utils');
const { TYPES, ACTIONS, VALID_OPTIONS, OPTION_TYPE,
const { TYPES, ACTIONS, VALID_OPTIONS, OPTION_TYPE, BOOLEAN_STRING_VALUES,
LIST_ACCOUNT_FILTERS, LIST_BUCKET_FILTERS, GLACIER_ACTIONS } = require('../manage_nsfs/manage_nsfs_constants');
const NoobaaEvent = require('../manage_nsfs/manage_nsfs_events_utils').NoobaaEvent;

Expand Down Expand Up @@ -146,7 +146,7 @@ async function fetch_bucket_data(argv, from_file) {
owner_account: undefined,
system_owner: argv.owner, // GAP - needs to be the system_owner (currently it is the account name)
bucket_owner: argv.owner,
wide: argv.wide,
wide: _.isUndefined(argv.wide) ? undefined : get_boolean_or_string_value(argv.wide),
tag: undefined, // if we would add the option to tag a bucket using CLI, this should be changed
versioning: action === ACTIONS.ADD ? 'DISABLED' : undefined,
creation_date: action === ACTIONS.ADD ? new Date().toISOString() : undefined,
Expand Down Expand Up @@ -239,7 +239,9 @@ async function verify_bucket_owner(bucket_owner, action) {
}
// check if bucket owner has the permission to create bucket (for bucket add only)
if (action === ACTIONS.ADD && !account.allow_bucket_creation) {
throw_cli_error(ManageCLIError.BucketCreationNotAllowed, bucket_owner);
const detail_msg = `${bucket_owner} account not allowed to create new buckets. ` +
`Please make sure to have a valid new_buckets_path and enable the flag allow_bucket_creation`;
throw_cli_error(ManageCLIError.BucketCreationNotAllowed, detail_msg);
}
return account._id;
}
Expand Down Expand Up @@ -333,7 +335,7 @@ async function manage_bucket_operations(action, data, argv) {

async function account_management(argv, from_file) {
const action = argv._[1] || '';
const show_secrets = Boolean(argv.show_secrets) || false;
const show_secrets = get_boolean_or_string_value(argv.show_secrets);
const data = await fetch_account_data(argv, from_file);
await manage_account_operations(action, data, show_secrets, argv);
}
Expand Down Expand Up @@ -379,7 +381,8 @@ async function fetch_account_data(argv, from_file) {
const regenerate = action === ACTIONS.ADD;
access_keys = set_access_keys(argv, regenerate);
} else if (action === ACTIONS.UPDATE) {
access_keys = set_access_keys(argv, Boolean(argv.regenerate));
const regenerate = get_boolean_or_string_value(argv.regenerate);
access_keys = set_access_keys(argv, regenerate);
new_access_key = access_keys[0].access_key;
access_keys[0].access_key = undefined; //Setting it as undefined so we can replace the symlink
}
Expand All @@ -389,7 +392,7 @@ async function fetch_account_data(argv, from_file) {
name: _.isUndefined(argv.name) ? undefined : String(argv.name),
email: _.isUndefined(argv.name) ? undefined : String(argv.name), // temp, keep the email internally
creation_date: action === ACTIONS.ADD ? new Date().toISOString() : undefined,
wide: argv.wide,
wide: _.isUndefined(argv.wide) ? undefined : get_boolean_or_string_value(argv.wide),
new_name: _.isUndefined(argv.new_name) ? undefined : String(argv.new_name),
new_access_key,
access_keys,
Expand All @@ -414,8 +417,14 @@ async function fetch_account_data(argv, from_file) {
// secret_key as SensitiveString
data.access_keys[0].secret_key = _.isUndefined(data.access_keys[0].secret_key) ? undefined :
new SensitiveString(String(data.access_keys[0].secret_key));
// allow_bucket_creation infer from new_buckets_path
data.allow_bucket_creation = !_.isUndefined(data.nsfs_account_config.new_buckets_path);
// allow_bucket_creation either set by user or infer from new_buckets_path
if (_.isUndefined(argv.allow_bucket_creation)) {
data.allow_bucket_creation = !_.isUndefined(data.nsfs_account_config.new_buckets_path);
} else if (typeof argv.allow_bucket_creation === 'string') {
data.allow_bucket_creation = argv.allow_bucket_creation.toLowerCase() === 'true';
} else {
data.allow_bucket_creation = Boolean(argv.allow_bucket_creation);
}
// fs_backend deletion specified with empty string '' (but it is not part of the schema)
data.nsfs_account_config.fs_backend = data.nsfs_account_config.fs_backend || undefined;

Expand Down Expand Up @@ -865,6 +874,7 @@ function validate_no_extra_options(type, action, input_options) {
* @param {object} input_options_with_data object with flag (key) and value
*/
function validate_options_type_by_value(type, input_options_with_data) {
let details;
for (const [option, value] of Object.entries(input_options_with_data)) {
const type_of_option = OPTION_TYPE[option];
const type_of_value = typeof value;
Expand All @@ -873,12 +883,66 @@ function validate_options_type_by_value(type, input_options_with_data) {
if ((option === 'name' || option === 'new_name') && (type_of_value === 'number')) {
continue;
}
const err_msg = `type of flag ${option} should be ${type_of_option}`;
throw_cli_error(ManageCLIError.InvalidArgumentType, err_msg);
// special case for allow_bucket_creation
if (option === 'allow_bucket_creation' && type_of_value === 'boolean') {
continue;
}
// special case for regenerate
if (option === 'regenerate' && validate_boolean_string_value(input_options_with_data.regenerate)) {
continue;
}
// special case for wide
if (option === 'wide' && validate_boolean_string_value(input_options_with_data.wide)) {
continue;
}
// special case for show_secrets
if (option === 'show_secrets' && validate_boolean_string_value(input_options_with_data.show_secrets)) {
continue;
}
details = `type of flag ${option} should be ${type_of_option}`;
throw_cli_error(ManageCLIError.InvalidArgumentType, details);
}
if (input_options_with_data.allow_bucket_creation &&
typeof input_options_with_data.allow_bucket_creation === 'string' &&
!BOOLEAN_STRING_VALUES.includes(input_options_with_data.allow_bucket_creation.toLowerCase())) {
throw_cli_error(ManageCLIError.InvalidAllowBucketCreation);
}
}
}

/**
* validate_boolean_string_value is used when the option type is boolean
* and we wish to allow the command also to to accept 'true' and 'false' values.
* @param {boolean|string} value
*/
function validate_boolean_string_value(value) {
if (value && typeof value === 'string') {
const check_allowed_boolean_value = BOOLEAN_STRING_VALUES.includes(value.toLowerCase());
if (!check_allowed_boolean_value) {
throw_cli_error(ManageCLIError.InvalidBooleanValue);
}
return true;
}
return false;
}

/**
* get_boolean_or_string_value will check if the value
* 1. if the value is undefined - it returns false.
* 2. (the value is defined) if it a string 'true' or 'false' = then we set boolean respectively.
* 3. (the value is defined) then we set true (Boolean convert of this case will be true).
* @param {boolean|string} value
*/
function get_boolean_or_string_value(value) {
if (_.isUndefined(value)) {
return false;
} else if (typeof value === 'string' && BOOLEAN_STRING_VALUES.includes(value.toLowerCase())) {
return value.toLowerCase() === 'true';
} else { // boolean type
return Boolean(value);
}
}

///////////////////////////////
/// UTILS ///
///////////////////////////////
Expand Down
12 changes: 12 additions & 0 deletions src/manage_nsfs/manage_nsfs_cli_errors.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,18 @@ ManageCLIError.InvalidAccountNewBucketsPath = Object.freeze({
http_code: 400,
});

ManageCLIError.InvalidAllowBucketCreation = Object.freeze({
code: 'InvalidAllowBucketCreation',
message: 'allow bucket creations supported values are true and false, default is true when passing new_buckets_path',
http_code: 400,
});

ManageCLIError.InvalidBooleanValue = Object.freeze({
code: 'InvalidBooleanValue',
message: 'supported values are true and false, default is false',
http_code: 400,
});

ManageCLIError.InvalidNewNameAccountIdentifier = Object.freeze({
code: 'InvalidNewNameAccountIdentifier',
message: 'Account new_name can not be used on add command, please remove the --new_name flag',
Expand Down
8 changes: 6 additions & 2 deletions src/manage_nsfs/manage_nsfs_constants.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ const GLOBAL_CONFIG_ROOT = 'config_root';
const GLOBAL_CONFIG_OPTIONS = new Set(['from_file', GLOBAL_CONFIG_ROOT, 'config_root_backend']);

const VALID_OPTIONS_ACCOUNT = {
'add': new Set(['name', 'uid', 'gid', 'new_buckets_path', 'user', 'access_key', 'secret_key', 'fs_backend', ...GLOBAL_CONFIG_OPTIONS]),
'update': new Set(['name', 'uid', 'gid', 'new_buckets_path', 'user', 'access_key', 'secret_key', 'fs_backend', 'new_name', 'regenerate', ...GLOBAL_CONFIG_OPTIONS]),
'add': new Set(['name', 'uid', 'gid', 'new_buckets_path', 'user', 'access_key', 'secret_key', 'fs_backend', 'allow_bucket_creation', ...GLOBAL_CONFIG_OPTIONS]),
'update': new Set(['name', 'uid', 'gid', 'new_buckets_path', 'user', 'access_key', 'secret_key', 'fs_backend', 'allow_bucket_creation', 'new_name', 'regenerate', ...GLOBAL_CONFIG_OPTIONS]),
'delete': new Set(['name', GLOBAL_CONFIG_ROOT]),
'list': new Set(['wide', 'show_secrets', GLOBAL_CONFIG_ROOT, 'gid', 'uid', 'user', 'name', 'access_key']),
'status': new Set(['name', 'access_key', 'show_secrets', GLOBAL_CONFIG_ROOT]),
Expand Down Expand Up @@ -72,6 +72,7 @@ const OPTION_TYPE = {
access_key: 'string',
secret_key: 'string',
fs_backend: 'string',
allow_bucket_creation: 'string',
config_root: 'string',
from_file: 'string',
config_root_backend: 'string',
Expand All @@ -84,6 +85,8 @@ const OPTION_TYPE = {
ips: 'string',
};

const BOOLEAN_STRING_VALUES = ['true', 'false'];

const LIST_ACCOUNT_FILTERS = ['uid', 'gid', 'user', 'name', 'access_key'];
const LIST_BUCKET_FILTERS = ['name'];

Expand All @@ -94,6 +97,7 @@ exports.GLACIER_ACTIONS = GLACIER_ACTIONS;
exports.CONFIG_SUBDIRS = CONFIG_SUBDIRS;
exports.VALID_OPTIONS = VALID_OPTIONS;
exports.OPTION_TYPE = OPTION_TYPE;
exports.BOOLEAN_STRING_VALUES = BOOLEAN_STRING_VALUES;

exports.LIST_ACCOUNT_FILTERS = LIST_ACCOUNT_FILTERS;
exports.LIST_BUCKET_FILTERS = LIST_BUCKET_FILTERS;
Loading

0 comments on commit 2a3d0a9

Please sign in to comment.