Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NSFS | NC | Symlinks should contain a relative path #7916

Merged
merged 1 commit into from
Mar 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion docs/design/NonContainerizedNSFSDesign.md
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ High level configuration -

3.1. accounts/ - directory that contains accounts configurations, each account configuration file is called {account_name}.json and fits to the account schema.

3.2. access_keys/ - directory that contains symlinks to accounts configurations, each symlink called {access_key}.symlink and links to an account under accounts/ directory.
3.2. access_keys/ - directory that contains symlinks to accounts configurations, each symlink called {access_key}.symlink and links to an account under accounts/ directory. Access key symlink is targeted to relative account path not absolute path. eg: `../accounts/acc_symlink1.json`

3.3. buckets/ - directory that contains buckets configurations, each bucket configuration file called {bucket_name}.json and fits the bucket schema.

Expand Down
7 changes: 5 additions & 2 deletions src/cmd/manage_nsfs.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ function write_stdout_response(response_code, detail, event_arg) {
const buckets_dir_name = '/buckets';
const accounts_dir_name = '/accounts';
const access_keys_dir_name = '/access_keys';
const acounts_dir_relative_path = '../accounts/';
naveenpaul1 marked this conversation as resolved.
Show resolved Hide resolved

let config_root;
let accounts_dir_path;
Expand Down Expand Up @@ -440,6 +441,7 @@ async function add_account(data) {
const fs_context = native_fs_utils.get_process_fs_context(config_root_backend);
const access_key = data.access_keys[0].access_key;
const account_config_path = get_config_file_path(accounts_dir_path, data.name);
const account_config_relative_path = get_config_file_path(acounts_dir_relative_path, data.name);
const account_config_access_key_path = get_symlink_config_file_path(access_keys_dir_path, access_key);

const name_exists = await native_fs_utils.is_path_exists(fs_context, account_config_path);
Expand All @@ -458,7 +460,7 @@ async function add_account(data) {
nsfs_schema_utils.validate_account_schema(JSON.parse(data));
await native_fs_utils.create_config_file(fs_context, accounts_dir_path, account_config_path, data);
await native_fs_utils._create_path(access_keys_dir_path, fs_context, config.BASE_MODE_CONFIG_DIR);
await nb_native().fs.symlink(fs_context, account_config_path, account_config_access_key_path);
await nb_native().fs.symlink(fs_context, account_config_relative_path, account_config_access_key_path);
shirady marked this conversation as resolved.
Show resolved Hide resolved
write_stdout_response(ManageCLIResponse.AccountCreated, data, {account: event_arg});
}

Expand Down Expand Up @@ -489,6 +491,7 @@ async function update_account(data) {
data.access_keys[0].access_key = data.new_access_key || cur_access_key;
const cur_account_config_path = get_config_file_path(accounts_dir_path, cur_name);
const new_account_config_path = get_config_file_path(accounts_dir_path, data.name);
const new_account_relative_config_path = get_config_file_path(acounts_dir_relative_path, data.name);
const cur_access_key_config_path = get_symlink_config_file_path(access_keys_dir_path, cur_access_key);
const new_access_key_config_path = get_symlink_config_file_path(access_keys_dir_path, data.access_keys[0].access_key);
const name_exists = update_name && await native_fs_utils.is_path_exists(fs_context, new_account_config_path);
Expand All @@ -512,7 +515,7 @@ async function update_account(data) {
// need to find a better way for atomic unlinking of symbolic links
// handle atomicity for symlinks
await nb_native().fs.unlink(fs_context, cur_access_key_config_path);
await nb_native().fs.symlink(fs_context, new_account_config_path, new_access_key_config_path);
await nb_native().fs.symlink(fs_context, new_account_relative_config_path, new_access_key_config_path);
write_stdout_response(ManageCLIResponse.AccountUpdated, data);
}

Expand Down
38 changes: 37 additions & 1 deletion 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 @@ -333,6 +333,22 @@ describe('manage nsfs cli account flow', () => {
expect(JSON.parse(res.stdout).error.code).toBe(ManageCLIError.InvalidArgumentType.code);
});

it('cli create account - check for the symlink relative path, not absolute path', async () => {
const action = ACTIONS.ADD;
const { type, access_key, secret_key, name, new_buckets_path, uid, gid } = defaults;
const account_options = { config_root, access_key, secret_key, name, new_buckets_path, uid, gid };
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);
assert_account(account, account_options, false);
const account_symlink = await read_config_file(config_root, CONFIG_SUBDIRS.ACCESS_KEYS, access_key, true);
assert_account(account_symlink, account_options);
const real_path = fs.readlinkSync(path.join(config_root, CONFIG_SUBDIRS.ACCESS_KEYS, access_key + '.symlink'));
expect(real_path).toContain('../accounts/' + account_options.name + '.json');
});

});

describe('cli update account', () => {
Expand Down Expand Up @@ -576,6 +592,24 @@ describe('manage nsfs cli account flow', () => {
expect(new_account_details.allow_bucket_creation).toBe(true);
});

it('cli account update account by name, access_key and secret_key - check for the symlink relative path, not absolute path', async function() {
const { name } = defaults;
const new_name = 'account1_new_name';
const access_key = 'GIGiFAnjaaE7OEXAMPLE';
const secret_key = 'U3AYaMpU3zRDcRFWmvzgQr9MoHIAsD+3oEXAMPLE';
const account_options = { config_root, name, new_name, access_key, secret_key };
const action = ACTIONS.UPDATE;
account_options.new_name = 'account1_new_name';
await exec_manage_cli(type, action, account_options);
let new_account_details = await read_config_file(config_root, CONFIG_SUBDIRS.ACCOUNTS, new_name);
const account_symlink = await read_config_file(config_root, CONFIG_SUBDIRS.ACCESS_KEYS, access_key, true);
//fixing the new_account_details for compare.
new_account_details = { ...new_account_details, ...new_account_details.nsfs_account_config };
assert_account(account_symlink, new_account_details);
const real_path = fs.readlinkSync(path.join(config_root, CONFIG_SUBDIRS.ACCESS_KEYS, access_key + '.symlink'));
expect(real_path).toContain('../accounts/' + new_name + '.json');
});

});

describe('cli list account', () => {
Expand Down Expand Up @@ -862,9 +896,11 @@ describe('manage nsfs cli account flow', () => {
expect(res_json.response.code).toBe(ManageCLIResponse.AccountDeleted.code);
const config_path = path.join(config_root, CONFIG_SUBDIRS.ACCOUNTS, name + '.json');
await fs_utils.file_must_not_exist(config_path);
const symlink_config_path = path.join(config_root, CONFIG_SUBDIRS.ACCESS_KEYS, defaults.access_key + '.symlink');
await fs_utils.file_must_not_exist(symlink_config_path);
});

it('cli delete account - should fail, account owns a bucket', async () => {
it('should fail - cli delete account, account owns a bucket', async () => {
// cli create account - happens in the "beforeEach"

// cli create bucket
Expand Down