Skip to content

Commit

Permalink
NSFS | Healthcheck is not reporting error for buckets with out access
Browse files Browse the repository at this point in the history
Signed-off-by: naveenpaul1 <[email protected]>
  • Loading branch information
naveenpaul1 committed Oct 30, 2024
1 parent 9ad101e commit 466faf5
Show file tree
Hide file tree
Showing 3 changed files with 153 additions and 10 deletions.
15 changes: 15 additions & 0 deletions docs/NooBaaNonContainerized/Health.md
Original file line number Diff line number Diff line change
Expand Up @@ -351,3 +351,18 @@ The following error codes will be associated with a specific Bucket or Account s
- Resolutions:
- Check for FS user on the host running the Health CLI.

#### 6. Bucket with invalid account owner
- Error code: `INVALID_ACCOUNT_OWNER`
- Error message: Bucket account owner is invalid
- Reasons:
- The bucket owner account is invalid.
- Resolutions:
- Compare bucket account owner and account ids in account dir.

#### 7. Bucket missing account owner
- Error code: `MISSING_ACCOUNT_OWNER`
- Error message: Bucket account owner not found
- Reasons:
- Bucket missing owner account.
- Resolutions:
- Check for owner_account property in bucket config file.
50 changes: 43 additions & 7 deletions src/manage_nsfs/health.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ const native_fs_utils = require('../util/native_fs_utils');
const { read_stream_join } = require('../util/buffer_utils');
const { make_https_request } = require('../util/http_utils');
const { TYPES } = require('./manage_nsfs_constants');
const { get_boolean_or_string_value, throw_cli_error, write_stdout_response } = require('./manage_nsfs_cli_utils');
const { get_boolean_or_string_value, throw_cli_error, write_stdout_response, get_bucket_owner_account_by_id } = require('./manage_nsfs_cli_utils');
const { ManageCLIResponse } = require('./manage_nsfs_cli_responses');
const ManageCLIError = require('./manage_nsfs_cli_errors').ManageCLIError;

Expand Down Expand Up @@ -52,6 +52,14 @@ const health_errors = {
error_code: 'INVALID_DISTINGUISHED_NAME',
error_message: 'Account distinguished name was not found',
},
INVALID_ACCOUNT_OWNER: {
error_code: 'INVALID_ACCOUNT_OWNER',
error_message: 'Bucket account owner is invalid',
},
MISSING_ACCOUNT_OWNER: {
error_code: 'MISSING_ACCOUNT_OWNER',
error_message: 'Bucket account owner not found',
},
UNKNOWN_ERROR: {
error_code: 'UNKNOWN_ERROR',
error_message: 'An unknown error occurred',
Expand Down Expand Up @@ -364,7 +372,7 @@ class NSFSHealth {
const config_file_path = this.config_fs.get_account_path_by_name(config_file_name);
res = await is_new_buckets_path_valid(config_file_path, config_data, storage_path);
} else if (type === TYPES.BUCKET) {
res = await is_bucket_storage_path_exists(this.config_fs.fs_context, config_data, storage_path);
res = await is_bucket_storage_and_owner_exists(this.config_fs, config_data, storage_path);
}
if (all_details && res.valid_storage) {
valid_storages.push(res.valid_storage);
Expand Down Expand Up @@ -498,17 +506,18 @@ async function is_new_buckets_path_valid(config_file_path, config_data, new_buck
}

/**
* is_bucket_storage_path_exists checks if the underlying storage path of a bucket exists
* @param {nb.NativeFSContext} fs_context
* is_bucket_storage_and_owner_exists checks if the underlying storage path of a bucket exists
* @param {import('../sdk/config_fs').ConfigFS} config_fs
* @param {object} config_data
* @param {string} storage_path
* @returns {Promise<object>}
*/
async function is_bucket_storage_path_exists(fs_context, config_data, storage_path) {
async function is_bucket_storage_and_owner_exists(config_fs, config_data, storage_path) {
let res_obj;
try {
if (!_should_skip_health_access_check()) {
await nb_native().fs.stat(fs_context, storage_path);
const account_fs_context = await get_account_owner_context(config_fs, config_data.owner_account);
await nb_native().fs.stat(account_fs_context, storage_path);
}
res_obj = get_valid_object(config_data.name, undefined, storage_path);
} catch (err) {
Expand All @@ -519,13 +528,40 @@ async function is_bucket_storage_path_exists(fs_context, config_data, storage_pa
} else if (err.code === 'EACCES' || (err.code === 'EPERM' && err.message === 'Operation not permitted')) {
dbg.log1('Error: Storage path should be accessible to account: ', storage_path);
err_code = health_errors.ACCESS_DENIED.error_code;
} else if (err.code === health_errors.INVALID_ACCOUNT_OWNER.error_code ||
err.code === health_errors.MISSING_ACCOUNT_OWNER.error_code) {
dbg.log1('Error: Bucket account owner should be valid account id ', config_data.owner_account);
err_code = err.code;
}
res_obj = get_invalid_object(config_data.name, undefined, storage_path, err_code);
}
return res_obj;
}


/**
* get_account_owner_context return bucket account owner specific context
* @param {import('../sdk/config_fs').ConfigFS} config_fs
* @param {string} owner_account
* @returns {Promise<object>}
*/
async function get_account_owner_context(config_fs, owner_account) {
if (!owner_account) {
const new_err = new Error(health_errors.MISSING_ACCOUNT_OWNER.error_message);
new_err.code = health_errors.MISSING_ACCOUNT_OWNER.error_code;
throw new_err;
}
try {
// when account owner is invalid method will throw error
const owner_account_data = await get_bucket_owner_account_by_id(config_fs, owner_account);
const account_fs_context = await native_fs_utils.get_fs_context(owner_account_data.nsfs_account_config,
owner_account_data.nsfs_account_config.fs_backend);
return account_fs_context;
} catch (err) {
const new_err = new Error(`Bucket account owner ${owner_account} is invalid`);
new_err.code = health_errors.INVALID_ACCOUNT_OWNER.error_code;
throw new_err;
}
}
/**
* get_valid_object returns an object which repersents a valid account/bucket and contains defined parameters
* @param {string} name
Expand Down
98 changes: 95 additions & 3 deletions src/test/unit_tests/test_nc_nsfs_health.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const NSFSHealth = require('../../manage_nsfs/health').NSFSHealth;
const { get_process_fs_context } = require('../../util/native_fs_utils');
const { ManageCLIError } = require('../../manage_nsfs/manage_nsfs_cli_errors');
const { TYPES, DIAGNOSE_ACTIONS, ACTIONS } = require('../../manage_nsfs/manage_nsfs_constants');
const { TMP_PATH, create_fs_user_by_platform, delete_fs_user_by_platform, exec_manage_cli } = require('../system_tests/test_utils');
const { TMP_PATH, create_fs_user_by_platform, delete_fs_user_by_platform, exec_manage_cli, set_nc_config_dir_in_config } = require('../system_tests/test_utils');

const tmp_fs_path = path.join(TMP_PATH, 'test_nc_health');
const DEFAULT_FS_CONFIG = get_process_fs_context();
Expand All @@ -30,6 +30,7 @@ mocha.describe('nsfs nc health', function() {
const config_fs = new ConfigFS(config_root);
const root_path = path.join(tmp_fs_path, 'root_path_nsfs_health/');
const config_root_invalid = path.join(tmp_fs_path, 'config_root_nsfs_health_invalid');
//const nc_conf_dir = config.NSFS_NC_CONF_DIR;
let Health;

mocha.before(async () => {
Expand Down Expand Up @@ -95,6 +96,7 @@ mocha.describe('nsfs nc health', function() {
});

mocha.describe('health check', function() {
this.timeout(10000);// eslint-disable-line no-invalid-this
const new_buckets_path = `${root_path}new_buckets_path_user1/`;
const account1_options = {
name: 'account1',
Expand All @@ -107,7 +109,14 @@ mocha.describe('nsfs nc health', function() {
path: new_buckets_path + '/bucket1',
owner: account1_options.name
};
const account2_options = {
name: 'account2',
uid: process.getuid(),
gid: process.getgid(),
new_buckets_path: new_buckets_path
};
const account_inaccessible_options = { name: 'account_inaccessible', uid: 999, gid: 999, new_buckets_path: bucket_storage_path };
const bucket_inaccessible_options = { name: 'bucket2', path: bucket_storage_path + '/bucket2', owner: account_inaccessible_options.name};
const account_inaccessible_dn_options = { name: 'account_inaccessible_dn', user: 'inaccessible_dn', new_buckets_path: bucket_storage_path };
const invalid_account_dn_options = { name: 'invalid_account_dn', user: 'invalid_account_dn', new_buckets_path: bucket_storage_path };
const fs_users = {
Expand All @@ -118,6 +127,7 @@ mocha.describe('nsfs nc health', function() {
}
};
mocha.before(async () => {
this.timeout(5000);// eslint-disable-line no-invalid-this
const https_port = 6443;
Health = new NSFSHealth({ config_root, https_port, config_fs });
await fs_utils.create_fresh_path(new_buckets_path);
Expand All @@ -126,14 +136,20 @@ mocha.describe('nsfs nc health', function() {
await fs_utils.file_must_exist(new_buckets_path + '/bucket1');
await exec_manage_cli(TYPES.ACCOUNT, ACTIONS.ADD, {config_root, ...account1_options});
await exec_manage_cli(TYPES.BUCKET, ACTIONS.ADD, {config_root, ...bucket1_options});
await fs_utils.file_must_exist(path.join(config_root, 'master_keys.json'));
const get_service_memory_usage = sinon.stub(Health, "get_service_memory_usage");
get_service_memory_usage.onFirstCall().returns(Promise.resolve(100));
for (const user of Object.values(fs_users)) {
await create_fs_user_by_platform(user.distinguished_name, user.distinguished_name, user.uid, user.gid);
}
});

mocha.beforeEach(async () => {
set_nc_config_dir_in_config(config_root);
});

mocha.after(async () => {
this.timeout(5000);// eslint-disable-line no-invalid-this
fs_utils.folder_delete(new_buckets_path);
fs_utils.folder_delete(path.join(new_buckets_path, 'bucket1'));
await exec_manage_cli(TYPES.BUCKET, ACTIONS.DELETE, {config_root, name: bucket1_options.name});
Expand Down Expand Up @@ -209,10 +225,13 @@ mocha.describe('nsfs nc health', function() {
});

mocha.it('NSFS bucket with invalid storage path', async function() {
this.timeout(5000);// eslint-disable-line no-invalid-this
Health.get_service_state.restore();
Health.get_endpoint_response.restore();
const resp = await exec_manage_cli(TYPES.ACCOUNT, ACTIONS.ADD, { config_root, ...account2_options });
const parsed_res = JSON.parse(resp).response.reply;
// create it manually because we can not skip invalid storage path check on the CLI
const bucket_invalid = { _id: mongo_utils.mongoObjectId(), name: 'bucket_invalid', path: new_buckets_path + '/bucket1/invalid', owner: account1_options.name };
const bucket_invalid = { _id: mongo_utils.mongoObjectId(), name: 'bucket_invalid', path: new_buckets_path + '/bucket1/invalid', owner_account: parsed_res._id };
await test_utils.write_manual_config_file(TYPES.BUCKET, config_fs, bucket_invalid);
const get_service_state = sinon.stub(Health, "get_service_state");
get_service_state.onFirstCall().returns(Promise.resolve({ service_status: 'active', pid: 1000 }))
Expand All @@ -223,7 +242,81 @@ mocha.describe('nsfs nc health', function() {
assert.strictEqual(health_status.status, 'OK');
assert.strictEqual(health_status.checks.buckets_status.invalid_buckets.length, 1);
assert.strictEqual(health_status.checks.buckets_status.invalid_buckets[0].name, bucket_invalid.name);
assert.strictEqual(health_status.checks.buckets_status.invalid_buckets[0].code, "STORAGE_NOT_EXIST");
await exec_manage_cli(TYPES.BUCKET, ACTIONS.DELETE, { config_root, name: bucket_invalid.name});
await exec_manage_cli(TYPES.ACCOUNT, ACTIONS.DELETE, { config_root, name: account2_options.name});
});

mocha.it('Bucket with inaccessible path - uid gid', async function() {
this.timeout(5000);// eslint-disable-line no-invalid-this
await config_fs.create_config_json_file(JSON.stringify({ NC_DISABLE_ACCESS_CHECK: true }));
await exec_manage_cli(TYPES.ACCOUNT, ACTIONS.ADD, { config_root, ...account_inaccessible_options });
await exec_manage_cli(TYPES.BUCKET, ACTIONS.ADD, {config_root, ...bucket_inaccessible_options});
await config_fs.delete_config_json_file();
Health.get_service_state.restore();
Health.get_endpoint_response.restore();
Health.all_account_details = true;
Health.all_bucket_details = true;
const get_service_state = sinon.stub(Health, "get_service_state");
get_service_state.onFirstCall().returns(Promise.resolve({ service_status: 'active', pid: 1000 }))
.onSecondCall().returns(Promise.resolve({ service_status: 'active', pid: 2000 }));
const get_endpoint_response = sinon.stub(Health, "get_endpoint_response");
get_endpoint_response.onFirstCall().returns(Promise.resolve({response: {response_code: 'RUNNING', total_fork_count: 0}}));
const health_status = await Health.nc_nsfs_health();
assert.strictEqual(health_status.checks.buckets_status.invalid_buckets.length, 1);
assert.strictEqual(health_status.checks.buckets_status.invalid_buckets[0].code, "ACCESS_DENIED");
assert.strictEqual(health_status.checks.buckets_status.invalid_buckets[0].name, bucket_inaccessible_options.name);
assert.strictEqual(health_status.checks.accounts_status.valid_accounts.length, 1);
assert.strictEqual(health_status.checks.accounts_status.invalid_accounts.length, 1);
assert.strictEqual(health_status.checks.accounts_status.invalid_accounts[0].code, "ACCESS_DENIED");
assert.strictEqual(health_status.checks.accounts_status.invalid_accounts[0].name, account_inaccessible_options.name);

await exec_manage_cli(TYPES.BUCKET, ACTIONS.DELETE, { config_root, name: bucket_inaccessible_options.name});
await exec_manage_cli(TYPES.ACCOUNT, ACTIONS.DELETE, { config_root, name: account_inaccessible_options.name});
});

mocha.it('Bucket with inaccessible owner', async function() {
this.timeout(5000);// eslint-disable-line no-invalid-this
//create bucket manually, cli wont allow bucket with invalid owner
const bucket_invalid_owner = { _id: mongo_utils.mongoObjectId(), name: 'bucket_invalid_account', path: new_buckets_path + '/bucket_account', owner_account: 'invalid_account' };
await test_utils.write_manual_config_file(TYPES.BUCKET, config_fs, bucket_invalid_owner);
Health.get_service_state.restore();
Health.get_endpoint_response.restore();
Health.all_account_details = true;
Health.all_bucket_details = true;
const get_service_state = sinon.stub(Health, "get_service_state");
get_service_state.onFirstCall().returns(Promise.resolve({ service_status: 'active', pid: 1000 }))
.onSecondCall().returns(Promise.resolve({ service_status: 'active', pid: 2000 }));
const get_endpoint_response = sinon.stub(Health, "get_endpoint_response");
get_endpoint_response.onFirstCall().returns(Promise.resolve({response: {response_code: 'RUNNING', total_fork_count: 0}}));
const health_status = await Health.nc_nsfs_health();
assert.strictEqual(health_status.checks.buckets_status.invalid_buckets.length, 1);
assert.strictEqual(health_status.checks.buckets_status.invalid_buckets[0].code, "INVALID_ACCOUNT_OWNER");
assert.strictEqual(health_status.checks.buckets_status.invalid_buckets[0].name, 'bucket_invalid_account');

await exec_manage_cli(TYPES.BUCKET, ACTIONS.DELETE, { config_root, name: 'bucket_invalid_account'});
});

mocha.it('Bucket with empty owner', async function() {
this.timeout(5000);// eslint-disable-line no-invalid-this
//create bucket manually, cli wont allow bucket with empty owner
const bucket_invalid_owner = { _id: mongo_utils.mongoObjectId(), name: 'bucket_invalid_account', path: new_buckets_path + '/bucket_account' };
await test_utils.write_manual_config_file(TYPES.BUCKET, config_fs, bucket_invalid_owner);
Health.get_service_state.restore();
Health.get_endpoint_response.restore();
Health.all_account_details = true;
Health.all_bucket_details = true;
const get_service_state = sinon.stub(Health, "get_service_state");
get_service_state.onFirstCall().returns(Promise.resolve({ service_status: 'active', pid: 1000 }))
.onSecondCall().returns(Promise.resolve({ service_status: 'active', pid: 2000 }));
const get_endpoint_response = sinon.stub(Health, "get_endpoint_response");
get_endpoint_response.onFirstCall().returns(Promise.resolve({response: {response_code: 'RUNNING', total_fork_count: 0}}));
const health_status = await Health.nc_nsfs_health();
assert.strictEqual(health_status.checks.buckets_status.invalid_buckets.length, 1);
assert.strictEqual(health_status.checks.buckets_status.invalid_buckets[0].code, "MISSING_ACCOUNT_OWNER");
assert.strictEqual(health_status.checks.buckets_status.invalid_buckets[0].name, 'bucket_invalid_account');

await exec_manage_cli(TYPES.BUCKET, ACTIONS.DELETE, { config_root, name: 'bucket_invalid_account'});
});

mocha.it('NSFS invalid bucket schema json', async function() {
Expand Down Expand Up @@ -527,4 +620,3 @@ mocha.describe('nsfs nc health', function() {
});
});
});

0 comments on commit 466faf5

Please sign in to comment.