noobaa · nadavMiz · Apr 4, 2024 · Mar 20, 2024
diff --git a/src/cmd/manage_nsfs.js b/src/cmd/manage_nsfs.js
@@ -23,7 +23,7 @@ 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, FROM_FILE, BOOLEAN_STRING_VALUES,
-    LIST_ACCOUNT_FILTERS, LIST_BUCKET_FILTERS, GLACIER_ACTIONS } = require('../manage_nsfs/manage_nsfs_constants');
+    LIST_ACCOUNT_FILTERS, LIST_BUCKET_FILTERS, GLACIER_ACTIONS, LIST_UNSETABLE_OPTIONS } = require('../manage_nsfs/manage_nsfs_constants');
 const NoobaaEvent = require('../manage_nsfs/manage_nsfs_events_utils').NoobaaEvent;
 
 function throw_cli_error(error_code, detail, event_arg) {
@@ -143,7 +143,8 @@ async function fetch_bucket_data(action, user_input) {
         path: user_input.path,
         should_create_underlying_storage: action === ACTIONS.ADD ? false : undefined,
         new_name: _.isUndefined(user_input.new_name) ? undefined : String(user_input.new_name),
-        fs_backend: _.isUndefined(user_input.fs_backend) ? config.NSFS_NC_STORAGE_BACKEND : String(user_input.fs_backend)
+        fs_backend: _.isUndefined(user_input.fs_backend) ? config.NSFS_NC_STORAGE_BACKEND : String(user_input.fs_backend),
+        force_md5_etag: _.isUndefined(user_input.force_md5_etag) || user_input.force_md5_etag === '' ? user_input.force_md5_etag : get_boolean_or_string_value(user_input.force_md5_etag)
         };
 
     if (user_input.bucket_policy !== undefined) {
@@ -169,6 +170,8 @@ async function fetch_bucket_data(action, user_input) {
     data.fs_backend = data.fs_backend || undefined;
     // s3_policy deletion specified with empty string '' (but it is not part of the schema)
     data.s3_policy = data.s3_policy || undefined;
+    // force_md5_etag deletion specified with empty string '' checked against user_input because data.force_md5_etag is boolean
+    data.force_md5_etag = data.force_md5_etag === '' ? undefined : data.force_md5_etag;
 
     return data;
 }
@@ -376,6 +379,7 @@ async function fetch_account_data(action, user_input) {
         new_name: _.isUndefined(user_input.new_name) ? undefined : String(user_input.new_name),
         new_access_key,
         access_keys,
+        force_md5_etag: _.isUndefined(user_input.force_md5_etag) || user_input.force_md5_etag === '' ? user_input.force_md5_etag : get_boolean_or_string_value(user_input.force_md5_etag),
         nsfs_account_config: {
             distinguished_name: user_input.user,
             uid: user_input.user ? undefined : user_input.uid,
@@ -401,6 +405,8 @@ async function fetch_account_data(action, user_input) {
     data.nsfs_account_config.fs_backend = data.nsfs_account_config.fs_backend || undefined;
     // new_buckets_path deletion specified with empty string ''
     data.nsfs_account_config.new_buckets_path = data.nsfs_account_config.new_buckets_path || undefined;
+    // force_md5_etag deletion specified with empty string '' checked against user_input.force_md5_etag because data.force_md5_etag is boolean
+    data.force_md5_etag = data.force_md5_etag === '' ? undefined : data.force_md5_etag;
     // allow_bucket_creation either set by user or infer from new_buckets_path
     if (_.isUndefined(user_input.allow_bucket_creation)) {
         data.allow_bucket_creation = !_.isUndefined(data.nsfs_account_config.new_buckets_path);
@@ -937,12 +943,16 @@ function validate_options_type_by_value(input_options_with_data) {
         const type_of_option = OPTION_TYPE[option];
         const type_of_value = typeof value;
         if (type_of_value !== type_of_option) {
+            // special case for unset value (specified by '').
+            if (LIST_UNSETABLE_OPTIONS.includes(option) && value === '') {
+                continue;
+            }
             // special case for names, although the type is string we want to allow numbers as well
             if ((option === 'name' || option === 'new_name') && (type_of_value === 'number')) {
                 continue;
             }
             // special case for boolean values
-            if (['allow_bucket_creation', 'regenerate', 'wide', 'show_secrets', 'force'].includes(option) && validate_boolean_string_value(value)) {
+            if (['allow_bucket_creation', 'regenerate', 'wide', 'show_secrets', 'force', 'force_md5_etag'].includes(option) && validate_boolean_string_value(value)) {
                 continue;
             }
             // special case for bucket_policy (from_file)

diff --git a/src/manage_nsfs/manage_nsfs_constants.js b/src/manage_nsfs/manage_nsfs_constants.js
@@ -33,16 +33,16 @@ const GLOBAL_CONFIG_OPTIONS = new Set([GLOBAL_CONFIG_ROOT, 'config_root_backend'
 const FROM_FILE = 'from_file';
 
 const VALID_OPTIONS_ACCOUNT = {
-    'add': new Set(['name', 'uid', 'gid', 'new_buckets_path', 'user', 'access_key', 'secret_key', 'fs_backend', 'allow_bucket_creation', FROM_FILE, ...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]),
+    'add': new Set(['name', 'uid', 'gid', 'new_buckets_path', 'user', 'access_key', 'secret_key', 'fs_backend', 'allow_bucket_creation', 'force_md5_etag', FROM_FILE, ...GLOBAL_CONFIG_OPTIONS]),
+    'update': new Set(['name', 'uid', 'gid', 'new_buckets_path', 'user', 'access_key', 'secret_key', 'fs_backend', 'allow_bucket_creation', 'force_md5_etag', 'new_name', 'regenerate', ...GLOBAL_CONFIG_OPTIONS]),
     'delete': new Set(['name', ...GLOBAL_CONFIG_OPTIONS]),
     'list': new Set(['wide', 'show_secrets', 'gid', 'uid', 'user', 'name', 'access_key', ...GLOBAL_CONFIG_OPTIONS]),
     'status': new Set(['name', 'access_key', 'show_secrets', ...GLOBAL_CONFIG_OPTIONS]),
 };
 
 const VALID_OPTIONS_BUCKET = {
-    'add': new Set(['name', 'owner', 'path', 'bucket_policy', 'fs_backend', FROM_FILE, ...GLOBAL_CONFIG_OPTIONS]),
-    'update': new Set(['name', 'owner', 'path', 'bucket_policy', 'fs_backend', 'new_name', ...GLOBAL_CONFIG_OPTIONS]),
+    'add': new Set(['name', 'owner', 'path', 'bucket_policy', 'fs_backend', 'force_md5_etag', FROM_FILE, ...GLOBAL_CONFIG_OPTIONS]),
+    'update': new Set(['name', 'owner', 'path', 'bucket_policy', 'fs_backend', 'new_name', 'force_md5_etag', ...GLOBAL_CONFIG_OPTIONS]),
     'delete': new Set(['name', 'force', ...GLOBAL_CONFIG_OPTIONS]),
     'list': new Set(['wide', 'name', ...GLOBAL_CONFIG_OPTIONS]),
     'status': new Set(['name', ...GLOBAL_CONFIG_OPTIONS]),
@@ -77,6 +77,7 @@ const OPTION_TYPE = {
     secret_key: 'string',
     fs_backend: 'string',
     allow_bucket_creation: 'boolean',
+    force_md5_etag: 'boolean',
     config_root: 'string',
     from_file: 'string',
     config_root_backend: 'string',
@@ -92,6 +93,9 @@ const OPTION_TYPE = {
 
 const BOOLEAN_STRING_VALUES = ['true', 'false'];
 
+//options that can be unset using ''
+const LIST_UNSETABLE_OPTIONS = ['fs_backend', 's3_policy', 'force_md5_etag'];
+
 const LIST_ACCOUNT_FILTERS = ['uid', 'gid', 'user', 'name', 'access_key'];
 const LIST_BUCKET_FILTERS = ['name'];
 
@@ -104,6 +108,7 @@ exports.VALID_OPTIONS = VALID_OPTIONS;
 exports.OPTION_TYPE = OPTION_TYPE;
 exports.FROM_FILE = FROM_FILE;
 exports.BOOLEAN_STRING_VALUES = BOOLEAN_STRING_VALUES;
+exports.LIST_UNSETABLE_OPTIONS = LIST_UNSETABLE_OPTIONS;
 
 exports.LIST_ACCOUNT_FILTERS = LIST_ACCOUNT_FILTERS;
 exports.LIST_BUCKET_FILTERS = LIST_BUCKET_FILTERS;
diff --git a/src/manage_nsfs/manage_nsfs_help_utils.js b/src/manage_nsfs/manage_nsfs_help_utils.js
@@ -78,6 +78,7 @@ Flags:
 --secret_key <string>                                 (optional)        Set the secret key for the account (default is generated)
 --fs_backend <none | GPFS | CEPH_FS | NFSv4>          (optional)        Set the filesystem type of new_buckets_path (default config.NSFS_NC_STORAGE_BACKEND)
 --allow_bucket_creation <true | false>                (optional)        Set the account to explicitly allow or block bucket creation
+--force_md5_etag <true | false>                       (optional)        Set the account to force md5 etag calculation. (unset with '') (will override default config.NSFS_NC_STORAGE_BACKEND)
 --from_file <string>                                  (optional)        Use details from the JSON file, there is no need to mention all the properties individually in the CLI
 `;
 
@@ -97,6 +98,7 @@ Flags:
 --secret_key <string>                                 (optional)        Update the secret key
 --fs_backend <none | GPFS | CEPH_FS | NFSv4>          (optional)        Update the filesystem type of new_buckets_path (default config.NSFS_NC_STORAGE_BACKEND)
 --allow_bucket_creation <true | false>                (optional)        Update the account to explicitly allow or block bucket creation
+--force_md5_etag <true | false>                       (optional)        Update the account to force md5 etag calculation (unset with '') (will override default config.NSFS_NC_STORAGE_BACKEND)
 `;
 
 const ACCOUNT_FLAGS_DELETE = `
@@ -141,6 +143,7 @@ Flags:
 --path <string>                                                         Set the bucket path
 --bucket_policy <string>                              (optional)        Set the bucket policy, type is a string of valid JSON policy
 --fs_backend <none | GPFS | CEPH_FS | NFSv4>          (optional)        Set the filesystem type (default config.NSFS_NC_STORAGE_BACKEND)
+--force_md5_etag <true | false>                       (optional)        Set the bucket to force md5 etag calculation (unset with '') (will override default config.NSFS_NC_STORAGE_BACKEND)
 --from_file <string>                                  (optional)        Use details from the JSON file, there is no need to mention all the properties individually in the CLI
 `;
 
@@ -155,6 +158,7 @@ Flags:
 --path <string>                                       (optional)        Update the bucket path
 --bucket_policy <string>                              (optional)        Update the bucket policy, type is a string of valid JSON policy (unset with '')
 --fs_backend <none | GPFS | CEPH_FS | NFSv4>          (optional)        Update the filesystem type (unset with '') (default config.NSFS_NC_STORAGE_BACKEND)
+--force_md5_etag <true | false>                       (optional)        Update the bucket to force md5 etag calculation (unset with '') (will override default config.NSFS_NC_STORAGE_BACKEND)
 `;
 
 const BUCKET_FLAGS_DELETE = `

diff --git a/src/sdk/namespace_fs.js b/src/sdk/namespace_fs.js
@@ -1457,8 +1457,7 @@ class NamespaceFS {
         const { source_stream } = params;
         try {
             // Not using async iterators with ReadableStreams due to unsettled promises issues on abort/destroy
-            const md5_enabled = config.NSFS_CALCULATE_MD5 || (this.force_md5_etag ||
-                object_sdk?.requesting_account?.force_md5_etag);
+            const md5_enabled = this._is_force_md5_enabled(object_sdk);
             const chunk_fs = new ChunkFS({
                 target_file,
                 fs_context,
@@ -1649,8 +1648,7 @@ class NamespaceFS {
         await this._throw_if_low_space(fs_context);
         const open_mode = 'w*';
         try {
-            const md5_enabled = config.NSFS_CALCULATE_MD5 || (this.force_md5_etag ||
-                object_sdk?.requesting_account?.force_md5_etag);
+            const md5_enabled = this._is_force_md5_enabled(object_sdk);
             const MD5Async = md5_enabled ? new (nb_native().crypto.MD5Async)() : undefined;
             const { multiparts = [] } = params;
             multiparts.sort((a, b) => a.num - b.num);
@@ -2474,6 +2472,24 @@ class NamespaceFS {
         const is_dir_content = this._is_directory_content(file_path, params.key);
         return is_dir_content && params.size === 0;
     }
+    /**
+     * returns if should force md5 calculation for the bucket/account.
+     * first check if defined for bucket / account, if not use global default
+     * @param {nb.ObjectSDK} object_sdk
+     * @returns {boolean}
+     */
+    _is_force_md5_enabled(object_sdk) {
+        // value defined for bucket
+        if (!_.isUndefined(this.force_md5_etag)) {
+            return this.force_md5_etag;
+        }
+        // value defined for account
+        if (!_.isUndefined(object_sdk?.requesting_account?.force_md5_etag)) {
+            return object_sdk?.requesting_account?.force_md5_etag;
+        }
+        // otherwise return global default
+        return config.NSFS_CALCULATE_MD5;
+    }
 
     //////////////////////////
     //// VERSIONING UTILS ////

diff --git a/src/server/system_services/schemas/nsfs_account_schema.js b/src/server/system_services/schemas/nsfs_account_schema.js
@@ -29,6 +29,9 @@ module.exports = {
         allow_bucket_creation: {
             type: 'boolean',
         },
+        force_md5_etag: {
+            type: 'boolean',
+        },
         access_keys: {
             type: 'array',
             items: {

diff --git a/src/server/system_services/schemas/nsfs_bucket_schema.js b/src/server/system_services/schemas/nsfs_bucket_schema.js
@@ -59,6 +59,9 @@ module.exports = {
         },
         website: {
             $ref: 'common_api#/definitions/bucket_website',
+        },
+        force_md5_etag: {
+            type: 'boolean',
         }
     }
 };
diff --git a/src/test/unit_tests/jest_tests/test_nc_nsfs_account_cli.test.js b/src/test/unit_tests/jest_tests/test_nc_nsfs_account_cli.test.js
@@ -349,6 +349,19 @@ describe('manage nsfs cli account flow', () => {
             expect(real_path).toContain('../accounts/' + account_options.name + '.json');
         });
 
+        it('cli account add - use flag force_md5_etag', async function() {
+            const action = ACTIONS.ADD;
+            const { type, name, new_buckets_path, uid, gid } = defaults;
+            const force_md5_etag = true;
+            const account_options = { config_root, name, new_buckets_path, uid, gid, force_md5_etag };
+            await fs_utils.create_fresh_path(new_buckets_path);
+            await fs_utils.file_must_exist(new_buckets_path);
+            await set_path_permissions_and_owner(new_buckets_path, account_options, 0o700);
+            await exec_manage_cli(type, action, account_options);
+            const account = await read_config_file(config_root, CONFIG_SUBDIRS.ACCOUNTS, name);
+            expect(account.force_md5_etag).toBe(true);
+        });
+
     });
 
     describe('cli update account', () => {
@@ -610,6 +623,37 @@ describe('manage nsfs cli account flow', () => {
             expect(real_path).toContain('../accounts/' + new_name + '.json');
         });
 
+        it('cli update account set flag force_md5_etag', async function() {
+            const { name } = defaults;
+            const account_options = { config_root, name, force_md5_etag: 'true'};
+            const action = ACTIONS.UPDATE;
+            await exec_manage_cli(type, action, account_options);
+            let new_account_details = await read_config_file(config_root, CONFIG_SUBDIRS.ACCOUNTS, name);
+            expect(new_account_details.force_md5_etag).toBe(true);
+
+            account_options.force_md5_etag = 'false';
+            await exec_manage_cli(type, action, account_options);
+            new_account_details = await read_config_file(config_root, CONFIG_SUBDIRS.ACCOUNTS, name);
+            expect(new_account_details.force_md5_etag).toBe(false);
+        });
+
+        it('cli update account unset flag force_md5_etag', async function() {
+            // first set the value of force_md5_etag to be true
+            const { name } = defaults;
+            const account_options = { config_root, name, force_md5_etag: 'true'};
+            const action = ACTIONS.UPDATE;
+            await exec_manage_cli(type, action, account_options);
+            let new_account_details = await read_config_file(config_root, CONFIG_SUBDIRS.ACCOUNTS, name);
+            expect(new_account_details.force_md5_etag).toBe(true);
+
+            // unset force_md5_etag
+            const empty_string = '\'\'';
+            account_options.force_md5_etag = empty_string;
+            await exec_manage_cli(type, action, account_options);
+            new_account_details = await read_config_file(config_root, CONFIG_SUBDIRS.ACCOUNTS, name);
+            expect(new_account_details.force_md5_etag).toBeUndefined();
+        });
+
     });
 
     describe('cli list account', () => {

diff --git a/src/test/unit_tests/jest_tests/test_nc_nsfs_account_schema_validation.test.js b/src/test/unit_tests/jest_tests/test_nc_nsfs_account_schema_validation.test.js
@@ -44,6 +44,13 @@ describe('schema validation NC NSFS account', () => {
             nsfs_schema_utils.validate_account_schema(account_data);
         });
 
+        it('nsfs_account_config with force_md5_etag', () => {
+            const account_data = get_account_data();
+            // @ts-ignore
+            account_data.force_md5_etag = true; // added
+            nsfs_schema_utils.validate_account_schema(account_data);
+        });
+
     });
 
     describe('account with additional properties', () => {
@@ -329,6 +336,16 @@ describe('schema validation NC NSFS account', () => {
             const message = 'must be equal to one of the allowed values';
             assert_validation(account_data, reason, message);
         });
+
+        it('nsfs_account_config with force_md5_etag as a string (instead of boolean)', () => {
+            const account_data = get_account_data();
+            // @ts-ignore
+            account_data.force_md5_etag = ""; // added
+            const reason = 'Test should have failed because of wrong type for' +
+                'force_md5_etag (string instead of boolean)';
+            const message = 'must be boolean';
+            assert_validation(account_data, reason, message);
+        });
     });
 
 });