Merge pull request #8488 from naveenpaul1/bucket_owner_check

naveenpaul1 · web-flow · commit 6437ba14b189 · 2024-11-05T16:12:18.000+05:30
NSFS | Healthcheck is not reporting error for buckets with out access
diff --git a/docs/NooBaaNonContainerized/Health.md b/docs/NooBaaNonContainerized/Health.md
@@ -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.
diff --git a/src/manage_nsfs/health.js b/src/manage_nsfs/health.js
@@ -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;
 
@@ -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',
@@ -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);
@@ -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) {
@@ -519,12 +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 existing and valid account', 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 returns bucket account owner specific FS 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, false, false);
+        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
diff --git a/src/manage_nsfs/manage_nsfs_cli_utils.js b/src/manage_nsfs/manage_nsfs_cli_utils.js
@@ -57,10 +57,12 @@ async function get_bucket_owner_account_by_name(config_fs, bucket_owner) {
  * otherwise it would throw an error
  * @param {import('../sdk/config_fs').ConfigFS} config_fs
  * @param {string} owner_account
+ * @param {boolean} show_secrets
+ * @param {boolean} decrypt_secret_key
  */
-async function get_bucket_owner_account_by_id(config_fs, owner_account) {
+async function get_bucket_owner_account_by_id(config_fs, owner_account, show_secrets = true, decrypt_secret_key = true) {
     try {
-        const account = await account_id_cache.get_with_cache({ _id: owner_account, config_fs });
+        const account = await account_id_cache.get_with_cache({ _id: owner_account, show_secrets, decrypt_secret_key, config_fs });
         return account;
     } catch (err) {
         if (err.code === 'ENOENT') {
diff --git a/src/sdk/accountspace_fs.js b/src/sdk/accountspace_fs.js
@@ -30,12 +30,18 @@ const account_id_cache = new LRUCache({
     name: 'AccountIDCache',
     // TODO: Decide on a time that we want to invalidate
     expiry_ms: Number(config.ACCOUNTS_ID_CACHE_EXPIRY),
-    /**
-     * @param {{ _id: string, config_fs: import('./config_fs').ConfigFS }} params
-     */
     make_key: ({ _id }) => _id,
-    load: async ({ _id, config_fs }) => config_fs.get_identity_by_id(_id, CONFIG_TYPES.ACCOUNT,
-        { show_secrets: true, decrypt_secret_key: true }),
+    /**
+     * Accounts are added to the cache based on id, Default value for show_secrets and decrypt_secret_key will be true, 
+     * and show_secrets and decrypt_secret_key `false` only when we load cache from the health script, 
+     * health script doesn't need to fetch or decrypt the secret.
+     * @param {{ _id: string, 
+     * show_secrets?: boolean,
+     * decrypt_secret_key?: boolean, 
+     * config_fs: import('./config_fs').ConfigFS }} params
+    */
+    load: async ({ _id, show_secrets = true, decrypt_secret_key = true, config_fs}) =>
+        config_fs.get_identity_by_id(_id, CONFIG_TYPES.ACCOUNT, { show_secrets: show_secrets, decrypt_secret_key: decrypt_secret_key }),
 });
 
 
diff --git a/src/test/unit_tests/nc_coretest.js b/src/test/unit_tests/nc_coretest.js
@@ -51,7 +51,7 @@ const http_address = `http://localhost:${http_port}`;
 const https_address = `https://localhost:${https_port}`;
 
 const FIRST_BUCKET = 'first.bucket';
-const NC_CORETEST_STORAGE_PATH = p.join(TMP_PATH, '/nc_coretest_storage_root_path/');
+const NC_CORETEST_STORAGE_PATH = p.join(TMP_PATH, 'nc_coretest_storage_root_path/');
 const FIRST_BUCKET_PATH = p.join(NC_CORETEST_STORAGE_PATH, FIRST_BUCKET, '/');
 const CONFIG_FILE_PATH = p.join(NC_CORETEST_CONFIG_DIR_PATH, 'config.json');
 const NC_CORETEST_CONFIG_FS = new ConfigFS(NC_CORETEST_CONFIG_DIR_PATH);
diff --git a/src/test/unit_tests/test_nc_nsfs_cli.js b/src/test/unit_tests/test_nc_nsfs_cli.js
@@ -80,6 +80,7 @@ mocha.describe('manage_nsfs cli', function() {
         };
 
         mocha.before(async () => {
+            config.NSFS_NC_CONF_DIR = config_root;
             await fs_utils.create_fresh_path(root_path);
         });
         mocha.after(async () => {
diff --git a/src/test/unit_tests/test_nc_nsfs_health.js b/src/test/unit_tests/test_nc_nsfs_health.js
@@ -95,6 +95,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',
@@ -107,7 +108,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 = {
@@ -118,6 +126,8 @@ mocha.describe('nsfs nc health', function() {
             }
         };
         mocha.before(async () => {
+            this.timeout(5000);// eslint-disable-line no-invalid-this
+            config.NSFS_NC_CONF_DIR = config_root;
             const https_port = 6443;
             Health = new NSFSHealth({ config_root, https_port, config_fs });
             await fs_utils.create_fresh_path(new_buckets_path);
@@ -126,6 +136,7 @@ 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)) {
@@ -134,13 +145,15 @@ mocha.describe('nsfs nc health', function() {
         });
 
         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});
             await exec_manage_cli(TYPES.ACCOUNT, ACTIONS.DELETE, {config_root, name: account1_options.name});
             for (const user of Object.values(fs_users)) {
                 await delete_fs_user_by_platform(user.distinguished_name);
             }
+            await fs_utils.folder_delete(config_root);
         });
 
         mocha.afterEach(async () => {
@@ -209,10 +222,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 }))
@@ -223,7 +239,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() {
@@ -527,4 +617,3 @@ mocha.describe('nsfs nc health', function() {
         });
     });
 });
-
