CR

Signed-off-by: Romy <[email protected]>

src/manage_nsfs/manage_nsfs_constants.js

@@ -164,8 +164,21 @@ const BOOLEAN_STRING_VALUES = ['true', 'false'];
 const BOOLEAN_STRING_OPTIONS = new Set(['allow_bucket_creation', 'regenerate', 'wide', 'show_secrets', 'force',
     'force_md5_etag', 'iam_operate_on_root_account', 'all_account_details', 'all_bucket_details', 'anonymous']);
 
-//options that can be unset using ''
-const LIST_UNSETTABLE_OPTIONS = ['fs_backend', 'bucket_policy', 'force_md5_etag', 'supplemental_groups', 'new_buckets_path'];
+// CLI UNSET VALUES
+const CLI_EMPTY_STRING = '';
+const CLI_EMPTY_STRING_ARRAY = '[]';
+
+const CLI_EMPTY_VALUES = new Set([CLI_EMPTY_STRING, CLI_EMPTY_STRING_ARRAY]);
+
+// options that can be unset using '' / '[]'
+const UNSETTABLE_OPTIONS_OBJ = {
+    'fs_backend': CLI_EMPTY_STRING,
+    'bucket_policy': CLI_EMPTY_STRING,
+    'force_md5_etag': CLI_EMPTY_STRING,
+    'supplemental_groups': CLI_EMPTY_STRING,
+    'new_buckets_path': CLI_EMPTY_STRING,
+    'ips': CLI_EMPTY_STRING_ARRAY,
+};
 
 const LIST_ACCOUNT_FILTERS = ['uid', 'gid', 'user', 'name', 'access_key'];
 const LIST_BUCKET_FILTERS = ['name'];
@@ -181,7 +194,10 @@ exports.OPTION_TYPE = OPTION_TYPE;
 exports.FROM_FILE = FROM_FILE;
 exports.BOOLEAN_STRING_VALUES = BOOLEAN_STRING_VALUES;
 exports.BOOLEAN_STRING_OPTIONS = BOOLEAN_STRING_OPTIONS;
-exports.LIST_UNSETTABLE_OPTIONS = LIST_UNSETTABLE_OPTIONS;
+exports.UNSETTABLE_OPTIONS_OBJ = UNSETTABLE_OPTIONS_OBJ;
+exports.CLI_EMPTY_VALUES = CLI_EMPTY_VALUES;
+exports.CLI_EMPTY_STRING = CLI_EMPTY_STRING;
+exports.CLI_EMPTY_STRING_ARRAY = CLI_EMPTY_STRING_ARRAY;
 
 exports.LIST_ACCOUNT_FILTERS = LIST_ACCOUNT_FILTERS;
 exports.LIST_BUCKET_FILTERS = LIST_BUCKET_FILTERS;

src/manage_nsfs/manage_nsfs_validations.js

@@ -12,7 +12,7 @@ const bucket_policy_utils = require('../endpoint/s3/s3_bucket_policy_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_UNSETTABLE_OPTIONS, ANONYMOUS, DIAGNOSE_ACTIONS, UPGRADE_ACTIONS } = require('../manage_nsfs/manage_nsfs_constants');
+    GLACIER_ACTIONS, UNSETTABLE_OPTIONS_OBJ, CLI_EMPTY_VALUES, ANONYMOUS, DIAGNOSE_ACTIONS, UPGRADE_ACTIONS } = require('../manage_nsfs/manage_nsfs_constants');
 const { check_root_account_owns_user } = require('../nc/nc_utils');
 const { validate_username } = require('../util/validation_utils');
 const notifications_util = require('../util/notifications_util');
@@ -159,16 +159,18 @@ function validate_no_extra_options(type, action, input_options, is_options_from_
 
 /**
  * validate_options_type_by_value checks the type of the value that match what we expect.
- * another check is for unset value (specified by '') - it'll be allowed only for flags specified in LIST_UNSETTABLE_OPTIONS
+ * another check is for unset value (specified by ''/'[]') - it'll be allowed only for flags specified in UNSETTABLE_OPTIONS_OBJ
  * @param {object} input_options_with_data object with flag (key) and value
  */
 function validate_options_type_by_value(input_options_with_data) {
     for (const [option, value] of Object.entries(input_options_with_data)) {
         const type_of_option = OPTION_TYPE[option];
         const type_of_value = typeof value;
+        const is_empty_cli_value = CLI_EMPTY_VALUES.has(value);
+        const is_unsettable_option_match_value = UNSETTABLE_OPTIONS_OBJ[option] === value;
         if (type_of_value !== type_of_option) {
             // if unset is allowed but the type is not string, we allow it
-            if (LIST_UNSETTABLE_OPTIONS.includes(option) && value === '') {
+            if (is_empty_cli_value && is_unsettable_option_match_value) {
                 continue;
             }
             // special case for names, although the type is string we want to allow numbers as well
@@ -193,9 +195,9 @@ function validate_options_type_by_value(input_options_with_data) {
             const details = `type of flag ${option} should be ${type_of_option} (and the received value is ${value})`;
             throw_cli_error(ManageCLIError.InvalidArgumentType, details);
         }
-        // special case for unset value (specified by '').
-        if (!LIST_UNSETTABLE_OPTIONS.includes(option) && value === '') {
-            const details = `flag value of ${option} is '' but this option can't be unset.`;
+        // special case for unset value (specified by ''/'[]').
+        if (is_empty_cli_value && !is_unsettable_option_match_value) {
+            const details = `flag value of ${option} is ${value} but this option can't be unset via ${value}.`;
             throw_cli_error(ManageCLIError.UnsetArgumentIsInvalid, details);
         }
     }

src/test/unit_tests/test_nc_cli.js

@@ -996,7 +996,7 @@ mocha.describe('manage_nsfs cli', function() {
                 await exec_manage_cli(type, '', { config_root, ips: '' });
                 assert.fail('should have failed with whitelist ips should not be empty.');
             } catch (err) {
-                assert_error(err, ManageCLIError.UnsetArgumentIsInvalid);
+                assert_error(err, ManageCLIError.InvalidWhiteListIPFormat);
             }
         });