Skip to content

Commit

Permalink
Add force_md5_etag option to account and bucket
Browse files Browse the repository at this point in the history
Signed-off-by: nadav mizrahi <nadav.mizrahi16@gmail.com>
  • Loading branch information
nadavMiz committed Mar 25, 2024
1 parent 780a8e8 commit c6f9a48
Show file tree
Hide file tree
Showing 10 changed files with 162 additions and 11 deletions.
6 changes: 4 additions & 2 deletions src/cmd/manage_nsfs.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) ? undefined : get_boolean_or_string_value(user_input.force_md5_etag)
};

if (user_input.bucket_policy !== undefined) {
Expand Down Expand Up @@ -376,6 +377,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) ? undefined : 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,
Expand Down Expand Up @@ -942,7 +944,7 @@ function validate_options_type_by_value(input_options_with_data) {
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)
Expand Down
9 changes: 5 additions & 4 deletions src/manage_nsfs/manage_nsfs_constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]),
Expand Down Expand Up @@ -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',
Expand Down
4 changes: 4 additions & 0 deletions src/manage_nsfs/manage_nsfs_help_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
--from_file <string> (optional) Use details from the JSON file, there is no need to mention all the properties individually in the CLI
`;

Expand All @@ -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
`;

const ACCOUNT_FLAGS_DELETE = `
Expand Down Expand Up @@ -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
--from_file <string> (optional) Use details from the JSON file, there is no need to mention all the properties individually in the CLI
`;

Expand All @@ -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
`;

const BUCKET_FLAGS_DELETE = `
Expand Down
15 changes: 11 additions & 4 deletions src/sdk/namespace_fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -1426,6 +1426,15 @@ class NamespaceFS {
}
}

_is_force_md5_enabled(object_sdk) {
let md5_enabled = config.NSFS_CALCULATE_MD5;
// If exists specific Bucket/Account settings should override global settings
if (!_.isUndefined(this.force_md5_etag) || !_.isUndefined(object_sdk?.requesting_account?.force_md5_etag)) {
md5_enabled = (this.force_md5_etag || object_sdk?.requesting_account?.force_md5_etag);
}
return md5_enabled;
}

// Allocated the largest semaphore size config.NSFS_BUF_SIZE_L in Semaphore but in fact we can take up more inside
// This is due to MD5 calculation and data buffers
// Can be finetuned further on if needed and inserting the Semaphore logic inside
Expand All @@ -1434,8 +1443,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,
Expand Down Expand Up @@ -1617,8 +1625,7 @@ class NamespaceFS {
const fs_context = this.prepare_fs_context(object_sdk);
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);
Expand Down
3 changes: 3 additions & 0 deletions src/server/system_services/schemas/nsfs_account_schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ module.exports = {
allow_bucket_creation: {
type: 'boolean',
},
force_md5_etag: {
type: 'boolean',
},
access_keys: {
type: 'array',
items: {
Expand Down
3 changes: 3 additions & 0 deletions src/server/system_services/schemas/nsfs_bucket_schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,9 @@ module.exports = {
},
website: {
$ref: 'common_api#/definitions/bucket_website',
},
force_md5_etag: {
type: 'boolean',
}
}
};
27 changes: 27 additions & 0 deletions src/test/unit_tests/jest_tests/test_nc_nsfs_account_cli.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -610,6 +623,20 @@ 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);
});

});

describe('cli list account', () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down Expand Up @@ -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);
});
});

});
Expand Down
73 changes: 72 additions & 1 deletion src/test/unit_tests/jest_tests/test_nc_nsfs_bucket_cli.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ describe('manage nsfs cli bucket flow', () => {

const bucket_defaults = {
name: 'bucket1',
owner: 'account1',
owner: account_defaults.name,
path: bucket_storage_path,
};

Expand Down Expand Up @@ -78,6 +78,15 @@ describe('manage nsfs cli bucket flow', () => {
expect(JSON.parse(res.stdout).error.message).toBe(ManageCLIError.InvalidStoragePath.message);
});

it('cli create bucket use flag force_md5_etag', async () => {
const action = ACTIONS.ADD;
const force_md5_etag = 'true';
const bucket_options = { config_root, ...bucket_defaults, force_md5_etag };
await exec_manage_cli(TYPES.BUCKET, action, bucket_options);
const bucket = await read_config_file(config_root, CONFIG_SUBDIRS.BUCKETS, bucket_defaults.name);
expect(bucket.force_md5_etag).toBe(true);
});

});


Expand Down Expand Up @@ -367,6 +376,68 @@ describe('cli create bucket using from_file', () => {

});

describe('cli update bucket', () => {
const config_root = path.join(tmp_fs_path, 'config_root_manage_nsfs3');
const root_path = path.join(tmp_fs_path, 'root_path_manage_nsfs3/');
const bucket_storage_path = path.join(tmp_fs_path, 'root_path_manage_nsfs3', 'bucket1');

const account_name = 'account_test';
const account_defaults = {
name: account_name,
new_buckets_path: `${root_path}new_buckets_path_4/`,
uid: 1001,
gid: 1001,
};

const bucket_defaults = {
name: 'bucket1',
owner: account_name,
path: bucket_storage_path,
};

beforeEach(async () => {
await fs_utils.create_fresh_path(`${config_root}/${CONFIG_SUBDIRS.BUCKETS}`);
await fs_utils.create_fresh_path(root_path);
await fs_utils.create_fresh_path(bucket_storage_path);
const action = ACTIONS.ADD;
// Account add
const { new_buckets_path: account_path } = account_defaults;
const account_options = { config_root, ...account_defaults };
await fs_utils.create_fresh_path(account_path);
await fs_utils.file_must_exist(account_path);
await set_path_permissions_and_owner(account_path, account_options, 0o700);
await exec_manage_cli(TYPES.ACCOUNT, action, account_options);

//bucket add
const { path: bucket_path } = bucket_defaults;
const bucket_options = { config_root, ...bucket_defaults };
await fs_utils.create_fresh_path(bucket_path);
await fs_utils.file_must_exist(bucket_path);
const resp = await exec_manage_cli(TYPES.BUCKET, action, bucket_options);
const bucket_resp = JSON.parse(resp);
expect(bucket_resp.response.reply._id).not.toBeNull();
});

afterEach(async () => {
await fs_utils.folder_delete(`${config_root}`);
await fs_utils.folder_delete(`${root_path}`);
});

it('cli update bucket set force_md5_etag', async () => {
const action = ACTIONS.UPDATE;
const force_md5_etag = 'true';
const bucket_options = { config_root, name: bucket_defaults.name, force_md5_etag };
await exec_manage_cli(TYPES.BUCKET, action, bucket_options);
let bucket_config = await read_config_file(config_root, CONFIG_SUBDIRS.BUCKETS, bucket_defaults.name);
expect(bucket_config.force_md5_etag).toBe(true);

bucket_options.force_md5_etag = 'false';
await exec_manage_cli(TYPES.BUCKET, action, bucket_options);
bucket_config = await read_config_file(config_root, CONFIG_SUBDIRS.BUCKETS, bucket_defaults.name);
expect(bucket_config.force_md5_etag).toBe(false);
});
});

/**
* exec_manage_cli will get the flags for the cli and runs the cli with it's flags
* @param {string} type
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,12 @@ describe('schema validation NC NSFS bucket', () => {
nsfs_schema_utils.validate_bucket_schema(bucket_data);
});

it('nsfs_bucket with force_md5_etag', () => {
const bucket_data = get_bucket_data();
bucket_data.force_md5_etag = true; // added
nsfs_schema_utils.validate_bucket_schema(bucket_data);
});

});

describe('bucket with additional properties', () => {
Expand Down Expand Up @@ -387,6 +393,16 @@ describe('schema validation NC NSFS bucket', () => {
assert_validation(bucket_data, reason, message);
});

it('bucket with force_md5_etag as string (instead of boolean)', () => {
const bucket_data = get_bucket_data();
// @ts-ignore
bucket_data.force_md5_etag = ""; // number instead of string
const reason = 'Test should have failed because of wrong type ' +
'force_md5_etag with boolean (instead of string)';
const message = 'must be boolean';
assert_validation(bucket_data, reason, message);
});

});

});
Expand Down

0 comments on commit c6f9a48

Please sign in to comment.