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 | CLI | add/update bucket policy of a bucket #7678

Merged
merged 1 commit into from
Jan 3, 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
205 changes: 120 additions & 85 deletions src/cmd/manage_nsfs.js

Large diffs are not rendered by default.

1 change: 1 addition & 0 deletions src/cmd/nsfs.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ class NsfsObjectSDK extends ObjectSDK {
bucketspace,
stats: endpoint_stats_collector.instance(),
});
this.nsfs_config_root = nsfs_config_root;
this.nsfs_fs_root = fs_root;
this.nsfs_fs_config = fs_config;
this.nsfs_account = account;
Expand Down
4 changes: 2 additions & 2 deletions src/endpoint/s3/s3_rest.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,9 @@ async function authorize_request_policy(req) {
if (is_owner) return;
throw new S3Error(S3Error.AccessDenied);
}

const account_identifier = req.object_sdk.nsfs_config_root ? account.name.unwrap() : account.email.unwrap();
const permission = await s3_bucket_policy_utils.has_bucket_policy_permission(
s3_policy, account.email.unwrap(), method, arn_path, req);
s3_policy, account_identifier, method, arn_path, req);
if (permission === "DENY") throw new S3Error(S3Error.AccessDenied);
if (permission === "ALLOW" || is_owner) return;

Expand Down
7 changes: 7 additions & 0 deletions src/manage_nsfs/manage_nsfs_cli_errors.js
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,13 @@ ManageCLIError.InvalidFSBackend = Object.freeze({
http_code: 400,
});

ManageCLIError.MalformedPolicy = Object.freeze({
code: 'MalformedPolicy',
message: 'Invalid bucket policy',
http_code: 400,
});


ManageCLIError.FS_ERRORS_TO_MANAGE = Object.freeze({
EACCES: ManageCLIError.AccessDenied,
EPERM: ManageCLIError.AccessDenied,
Expand Down
14 changes: 11 additions & 3 deletions src/sdk/bucketspace_fs.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const bucket_policy_utils = require('../endpoint/s3/s3_bucket_policy_utils');
const { default: Ajv } = require('ajv');
const bucket_schema = require('../server/object_services/schemas/nsfs_bucket_schema');
const account_schema = require('../server/object_services/schemas/nsfs_account_schema');
const { KEYWORDS } = require('../util/schema_keywords');
const common_api = require('../api/common_api');

const KeysSemaphore = require('../util/keys_semaphore');
Expand All @@ -27,6 +28,13 @@ const BUCKET_PATH = 'buckets';
const ACCOUNT_PATH = 'accounts';
const ACCESS_KEYS_PATH = 'access_keys';
const ajv = new Ajv({ verbose: true, allErrors: true });
ajv.addKeyword(KEYWORDS.methods);
ajv.addKeyword(KEYWORDS.doc);
ajv.addKeyword(KEYWORDS.date);
ajv.addKeyword(KEYWORDS.idate);
ajv.addKeyword(KEYWORDS.objectid);
ajv.addKeyword(KEYWORDS.binary);
ajv.addKeyword(KEYWORDS.wrapper);
ajv.addSchema(common_api);
const bucket_semaphore = new KeysSemaphore(1);

Expand Down Expand Up @@ -76,9 +84,9 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
}

async _get_account_by_name(name) {
const access_key_config_path = this._get_account_config_path(name);
const account_config_path = this._get_account_config_path(name);
try {
await nb_native().fs.stat(this.fs_context, access_key_config_path);
await nb_native().fs.stat(this.fs_context, account_config_path);
return true;
} catch (err) {
return false;
Expand Down Expand Up @@ -282,7 +290,7 @@ class BucketSpaceFS extends BucketSpaceSimpleFS {
async create_bucket(params, sdk) {
return bucket_semaphore.surround_key(String(params.name), async () => {
if (!sdk.requesting_account.allow_bucket_creation) {
throw new RpcError('UNAUTHORIZED', 'Not allowed to create new buckets')
throw new RpcError('UNAUTHORIZED', 'Not allowed to create new buckets');
}
if (!sdk.requesting_account.nsfs_account_config || !sdk.requesting_account.nsfs_account_config.new_buckets_path) {
throw new RpcError('MISSING_NSFS_ACCOUNT_CONFIGURATION');
Expand Down
3 changes: 3 additions & 0 deletions src/server/object_services/schemas/nsfs_bucket_schema.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,5 +41,8 @@ module.exports = {
fs_backend: {
$ref: 'common_api#/definitions/fs_backend'
},
s3_policy: {
$ref: 'common_api#/definitions/bucket_policy',
}
}
};
81 changes: 71 additions & 10 deletions src/test/unit_tests/test_nc_nsfs_cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ const nb_native = require('../../util/nb_native');
const config_module = require('../../../config');
const { ManageCLIError } = require('../../manage_nsfs/manage_nsfs_cli_errors');
const { ManageCLIResponse } = require('../../manage_nsfs/manage_nsfs_cli_responses');
const test_utils = require('../system_tests/test_utils');

const MAC_PLATFORM = 'darwin';
let tmp_fs_path = '/tmp/test_bucketspace_fs';
Expand Down Expand Up @@ -64,14 +65,39 @@ mocha.describe('manage_nsfs cli', function() {
const bucket_on_gpfs = 'bucketgpfs1';
const owner_email = 'user1@noobaa.io';
const bucket_path = `${root_path}${name}/`;
const bucket_with_policy = 'bucket-with-policy';
const bucket_policy = test_utils.generate_s3_policy('*', bucket_with_policy, ['s3:*']).policy;
const bucket1_policy = test_utils.generate_s3_policy('*', name, ['s3:*']).policy;
const invalid_bucket_policy = test_utils.generate_s3_policy('invalid_account', name, ['s3:*']).policy;
const empty_bucket_policy = '';

const schema_dir = 'buckets';
let bucket_options = { config_root, name, owner_email, bucket_path };
const gpfs_bucket_options = { config_root, name: bucket_on_gpfs, owner_email, bucket_path, fs_backend: 'GPFS' };
const bucket_with_policy_options = { ...bucket_options, bucket_policy: bucket_policy, name: bucket_with_policy };

mocha.it('cli bucket create with invalid bucket policy - should fail', async function() {
const action = nc_nsfs_manage_actions.ADD;
try {
await fs_utils.create_fresh_path(bucket_path);
await fs_utils.file_must_exist(bucket_path);
await exec_manage_cli(type, action, { ...bucket_options, bucket_policy: invalid_bucket_policy});
assert.fail('should have failed with invalid bucket policy');
} catch (err) {
assert_error(err, ManageCLIError.MalformedPolicy);
}
});

mocha.it('cli bucket create - bucket_with_policy', async function() {
const action = nc_nsfs_manage_actions.ADD;
await exec_manage_cli(type, action, bucket_with_policy_options);
const bucket = await read_config_file(config_root, schema_dir, bucket_with_policy);
assert_bucket(bucket, bucket_with_policy_options);
await assert_config_file_permissions(config_root, schema_dir, bucket_with_policy);
});

mocha.it('cli bucket create', async function() {
const action = nc_nsfs_manage_actions.ADD;
await fs_utils.create_fresh_path(bucket_path);
await fs_utils.file_must_exist(bucket_path);
const bucket_status = await exec_manage_cli(type, action, bucket_options);
assert_response(action, type, bucket_status, bucket_options);
const bucket = await read_config_file(config_root, schema_dir, name);
Expand Down Expand Up @@ -100,14 +126,14 @@ mocha.describe('manage_nsfs cli', function() {
mocha.it('cli bucket list', async function() {
const action = nc_nsfs_manage_actions.LIST;
const bucket_list = await exec_manage_cli(type, action, { config_root });
const expected_list = [{ name }];
const expected_list = [{ name }, { name: bucket_with_policy }];
assert_response(action, type, bucket_list, expected_list);
});

mocha.it('cli bucket list - wide', async function() {
const action = nc_nsfs_manage_actions.LIST;
const bucket_list = await exec_manage_cli(type, action, { config_root, wide: true });
const expected_list = [bucket_options];
const expected_list = [bucket_options, bucket_with_policy_options];
assert_response(action, type, bucket_list, expected_list, undefined, true);
});

Expand Down Expand Up @@ -151,6 +177,35 @@ mocha.describe('manage_nsfs cli', function() {
await assert_config_file_permissions(config_root, schema_dir, name);
});

mocha.it('cli bucket update invalid bucket policy - should fail', async function() {
const action = nc_nsfs_manage_actions.UPDATE;
const update_options = { config_root, bucket_policy: invalid_bucket_policy, name };
try {
await exec_manage_cli(type, action, update_options);
assert.fail('should have failed with invalid bucket policy');
} catch (err) {
assert_error(err, ManageCLIError.MalformedPolicy);
}
});

mocha.it('cli bucket update bucket policy', async function() {
const action = nc_nsfs_manage_actions.UPDATE;
bucket_options = { ...bucket_options, bucket_policy: bucket1_policy };
await exec_manage_cli(type, action, bucket_options);
const bucket = await read_config_file(config_root, schema_dir, bucket_options.name);
assert_bucket(bucket, bucket_options);
await assert_config_file_permissions(config_root, schema_dir, bucket_options.name);
});

mocha.it('cli bucket update bucket policy - delete bucket policy', async function() {
const action = nc_nsfs_manage_actions.UPDATE;
bucket_options = { ...bucket_options, bucket_policy: empty_bucket_policy };
await exec_manage_cli(type, action, bucket_options);
const bucket = await read_config_file(config_root, schema_dir, bucket_options.name);
assert_bucket(bucket, bucket_options);
await assert_config_file_permissions(config_root, schema_dir, bucket_options.name);
});

mocha.it('cli bucket update bucket name', async function() {
const action = nc_nsfs_manage_actions.UPDATE;
const update_options = { config_root, new_name: 'bucket2', name };
Expand Down Expand Up @@ -748,7 +803,7 @@ async function assert_config_file_permissions(config_root, schema_dir, config_fi
const config_path = path.join(config_root, schema_dir, config_file_name + (is_symlink ? '.symlink' : '.json'));
const { stat } = await nb_native().fs.readFile(DEFAULT_FS_CONFIG, config_path);
// 33152 means 600 (only owner has read and write permissions)
assert.ok(stat.mode, 33152);
assert.equal(stat.mode, 33152);
}

function assert_error(err, expect_error) {
Expand All @@ -769,12 +824,16 @@ function assert_response(action, type, actual_res, expected_res, show_secrets, w
} else if (action === nc_nsfs_manage_actions.DELETE) {
assert.equal(parsed.response.code, ManageCLIResponse.BucketDeleted.code);
} else if (action === nc_nsfs_manage_actions.LIST) {
if (wide) {
for (let i = 0; i < parsed.response.reply.length; i++) {
assert_bucket(parsed.response.reply[i], expected_res[i]);
assert.equal(parsed.response.reply.length, expected_res.length);
for (let i = 0; i < parsed.response.reply.length; i++) {
const name = parsed.response.reply[i].name;
const expected_res_by_name = expected_res.find(expected => expected.name === name);
if (wide) {
assert_bucket(parsed.response.reply[i], expected_res_by_name);
} else {
assert.deepEqual(parsed.response.reply[i], expected_res_by_name);

}
} else {
assert.deepEqual(parsed.response.reply, expected_res);
}
} else {
assert.fail(`Invalid command action - ${action}`);
Expand Down Expand Up @@ -810,6 +869,7 @@ function assert_bucket(bucket, bucket_options) {
assert.strictEqual(bucket.should_create_underlying_storage, false);
assert.strictEqual(bucket.versioning, 'DISABLED');
assert.strictEqual(bucket.fs_backend, bucket_options.fs_backend);
assert.deepStrictEqual(bucket.s3_policy, bucket_options.bucket_policy);
return true;
}

Expand Down Expand Up @@ -842,6 +902,7 @@ async function exec_manage_cli(type, action, options) {
const bucket_flags = (options.name ? `--name ${options.name}` : ``) +
(options.owner_email ? ` --email ${options.owner_email}` : ``) +
(options.fs_backend === undefined ? `` : ` --fs_backend '${options.fs_backend}'`) +
(options.bucket_policy === undefined ? `` : ` --bucket_policy '${JSON.stringify(options.bucket_policy)}'`) +
(options.bucket_path ? ` --path ${options.bucket_path}` : ``);

const account_flags = (options.name ? ` --name ${options.name}` : ``) +
Expand Down