bucket notifications - add connection test to health (#8833)

alphaprinz · alphaprinz · commit 0d02b9feff3d · 2025-03-03T20:22:27.000-08:00
Signed-off-by: Amit Prinz Setter &lt;alphaprinz@gmail.com&gt;
diff --git a/docs/NooBaaNonContainerized/Health.md b/docs/NooBaaNonContainerized/Health.md
@@ -42,6 +42,8 @@ For more details about NooBaa RPM installation, see - [Getting Started](./Gettin
     - checks if there is ongoing upgrade
     - returns error if there is no ongoing upgrade, but the config directory phase is locked
     - returns message if there is no ongoing upgrade and the config directory is unlocked
+  - `Bucket event notifications connections health`
+    - Sends out a test notification for each connection.
 
 * Health CLI requires root permissions.
 
@@ -75,7 +77,12 @@ noobaa-cli diagnose health [--deployment_type][--https_port]
 - `all_bucket_details`
     - Type: Boolean
     - Default: false
-    - Description: Indicates if health output should contain valid buckets.  
+    - Description: Indicates if health output should contain valid buckets.
+
+- `all_connection_details`
+    - Type: Boolean
+    - Default: false
+    - Description: Indicates if health output should contain connection test result.
 
 - `config_root`
     - Type: String
@@ -143,6 +150,10 @@ The output of the Health CLI is a JSON object containing the following propertie
     2. The account doesn't have RW access to its `new_buckets_path` or having invalid config JSON file.
     3. The account has an invalid config JSON file.
 
+- `invalid connections`:
+  - Type: [{ "name": "connection_name", "config_path": "/connection/file/path", "code": String }]
+  - Description: Array of connections that failed test notification.
+
 - `valid_accounts`
   - Type: [{ "name": account_name, "storage_path": "/path/to/accounts/new_buckets_path" }]
   - Description: Array of all the valid accounts. If the all_account_details flag is set to true, valid_accounts will be included in the Health response.
@@ -151,6 +162,10 @@ The output of the Health CLI is a JSON object containing the following propertie
   - Type: [{ "name": bucket_name, "storage_path": "/path/to/bucket/path" }]
   - Description: Array of all the valid buckets. If the all_bucket_details flag is set to true, valid_buckets will be included in the Health response.
 
+- `valid_connections`:
+  - Type: [{ "name": "connection_name", "config_path": "/connection/file/path" }]
+  - Description: Array of all connections to which test notification was send successfully.
+
 - `error_type`
   - Type: String
   - Enum: 'PERSISTENT' | 'TEMPORARY'
@@ -239,6 +254,21 @@ Output:
       ],
       "error_type": "PERSISTENT"
     },
+    "connectoins_status": {
+      "invalid_connections": [
+        {
+          "name": "notif_invalid",
+          "config_path": "/etc/noobaa.conf.d/connections/notif_invalid.json",
+          "code": "ECONNREFUSED"
+        }
+      ],
+      "valid_connections": [
+        {
+          "name": "notif_valid",
+          "config_path": "/etc/noobaa.conf.d/connections/notif_valid.json"
+        }
+      ]
+    },
     "config_directory": {
       "phase": "CONFIG_DIR_UNLOCKED",
       "config_dir_version": "1.0.0",
diff --git a/src/manage_nsfs/health.js b/src/manage_nsfs/health.js
@@ -15,6 +15,7 @@ const { TYPES } = require('./manage_nsfs_constants');
 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;
+const notifications_util = require('../util/notifications_util');
 
 
 const HOSTNAME = 'localhost';
@@ -101,6 +102,7 @@ class NSFSHealth {
         this.https_port = options.https_port;
         this.all_account_details = options.all_account_details;
         this.all_bucket_details = options.all_bucket_details;
+        this.all_connection_details = options.all_connection_details;
         this.config_fs = options.config_fs;
     }
 
@@ -128,12 +130,14 @@ class NSFSHealth {
 
         let bucket_details;
         let account_details;
+        let connection_details;
         const endpoint_response_code = (endpoint_state && endpoint_state.response?.response_code) || 'UNKNOWN_ERROR';
         const health_check_params = { service_status, pid, endpoint_response_code, config_directory_status };
         const service_health = this._calc_health_status(health_check_params);
         const error_code = this.get_error_code(health_check_params);
         if (this.all_bucket_details) bucket_details = await this.get_bucket_status();
         if (this.all_account_details) account_details = await this.get_account_status();
+        if (this.all_connection_details) connection_details = await this.get_connection_status();
         const health = {
             service_name: NOOBAA_SERVICE,
             status: service_health,
@@ -155,11 +159,13 @@ class NSFSHealth {
                     invalid_buckets: bucket_details === undefined ? undefined : bucket_details.invalid_storages,
                     valid_buckets: bucket_details === undefined ? undefined : bucket_details.valid_storages,
                     error_type: health_errors_tyes.PERSISTENT,
-                }
+                },
+                connections_status: connection_details
             }
         };
         if (!this.all_account_details) delete health.checks.accounts_status;
         if (!this.all_bucket_details) delete health.checks.buckets_status;
+        if (!this.all_connection_details) delete health.checks.connections_status;
         return health;
     }
 
@@ -333,6 +339,17 @@ class NSFSHealth {
         };
     }
 
+    async validate_config_dir_exists(path, type) {
+        const config_root_type_exists = await this.config_fs.validate_config_dir_exists(path);
+        if (!config_root_type_exists) {
+            dbg.log1(`Config directory type - ${type} is missing, ${path}`);
+            return {
+                invalid_storages: [],
+                valid_storages: []
+            };
+        }
+    }
+
     async get_bucket_status() {
         const bucket_details = await this.get_storage_status(TYPES.BUCKET, this.all_bucket_details);
         return bucket_details;
@@ -343,34 +360,38 @@ class NSFSHealth {
         return account_details;
     }
 
+    async get_connection_status() {
+        const connection_details = await this.get_storage_status(TYPES.CONNECTION, this.all_connection_details);
+        //if there were in failed test notifications, mark error as temp
+        if (connection_details.invalid_storages && connection_details.invalid_storages.length > 0) {
+            connection_details.error_type = health_errors_tyes.TEMPORARY;
+        }
+        return connection_details;
+    }
+
     async get_storage_status(type, all_details) {
         const invalid_storages = [];
         const valid_storages = [];
         //check for account and buckets dir paths
-        let config_root_type_exists;
         let config_dir_path;
         if (type === TYPES.BUCKET) {
             config_dir_path = this.config_fs.buckets_dir_path;
-            config_root_type_exists = await this.config_fs.validate_config_dir_exists(config_dir_path);
         } else if (type === TYPES.ACCOUNT) {
             // TODO - handle iam accounts when directory structure changes - read_account_by_id
             config_dir_path = this.config_fs.accounts_by_name_dir_path;
-            config_root_type_exists = await this.config_fs.validate_config_dir_exists(config_dir_path);
-        }
-        // TODO - this is not a good handling for that - we need to take it to an upper level
-        if (!config_root_type_exists) {
-            dbg.log1(`Config directory type - ${type} is missing, ${config_dir_path}`);
-            return {
-                invalid_storages: invalid_storages,
-                valid_storages: valid_storages
-            };
+        } else {
+            config_dir_path = this.config_fs.connections_dir_path;
         }
+        const missing = await this.validate_config_dir_exists(config_dir_path, type);
+        if (missing) return missing;
 
         let config_files_names;
         if (type === TYPES.BUCKET) {
             config_files_names = await this.config_fs.list_buckets();
-        } else {
+        } else if (type === TYPES.ACCOUNT) {
             config_files_names = await this.config_fs.list_accounts();
+        } else {
+            config_files_names = await this.config_fs.list_connections();
         }
         for (const config_file_name of config_files_names) {
             // config_file get data or push error
@@ -386,13 +407,24 @@ class NSFSHealth {
             let res;
             const storage_path = type === TYPES.BUCKET ?
                 config_data.path :
-                config_data.nsfs_account_config.new_buckets_path;
+                config_data.nsfs_account_config?.new_buckets_path;
 
             if (type === TYPES.ACCOUNT) {
                 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_and_owner_exists(this.config_fs, config_data, storage_path);
+            } else {
+                const connection_file_path = this.config_fs.get_connection_path_by_name(config_file_name);
+                const test_notif_err = await notifications_util.test_notifications([{
+                    name: config_data.name,
+                    topic: [this.config_fs.json(config_file_name)]
+                }], this.config_fs.config_root);
+                if (test_notif_err) {
+                    res = get_invalid_object(config_data.name, connection_file_path, undefined, test_notif_err.code);
+                } else {
+                    res = get_valid_object(config_data.name, connection_file_path, undefined);
+                }
             }
             if (all_details && res.valid_storage) {
                 valid_storages.push(res.valid_storage);
@@ -416,16 +448,41 @@ class NSFSHealth {
         let config_data;
         let err_obj;
         try {
-            config_data = type === TYPES.BUCKET ?
-                await this.config_fs.get_bucket_by_name(config_file_name) :
-                // TODO - should be changed to id when moving to new structure for supporting iam accounts
-                await this.config_fs.get_account_by_name(config_file_name);
+            switch (type) {
+                case TYPES.BUCKET: {
+                    config_data = await this.config_fs.get_bucket_by_name(config_file_name);
+                    break;
+                }
+                case TYPES.ACCOUNT: {
+                    // TODO - should be changed to id when moving to new structure for supporting iam accounts
+                    config_data = await this.config_fs.get_account_by_name(config_file_name);
+                    break;
+                }
+                case TYPES.CONNECTION: {
+                    config_data = await this.config_fs.get_connection_by_name(config_file_name);
+                    break;
+                }
+                default: //shouldn't happen, for js-lint
+            }
         } catch (err) {
             let err_code;
-            const config_file_path = type === TYPES.BUCKET ?
-                this.config_fs.get_bucket_path_by_name(config_file_name) :
-                // TODO - should be changed to id when moving to new structure for supporting iam accounts
-                this.config_fs.get_account_path_by_name(config_file_name);
+            let config_file_path;
+            switch (type) {
+                case TYPES.BUCKET: {
+                    config_file_path = await this.config_fs.get_bucket_path_by_name(config_file_name);
+                    break;
+                }
+                case TYPES.ACCOUNT: {
+                    // TODO - should be changed to id when moving to new structure for supporting iam accounts
+                    config_file_path = await this.config_fs.get_account_path_by_name(config_file_name);
+                    break;
+                }
+                case TYPES.CONNECTION: {
+                    config_file_path = await this.config_fs.get_connection_path_by_name(config_file_name);
+                    break;
+                }
+                default: //shouldn't happen, for js-lint
+            }
 
             if (err.code === 'ENOENT') {
                 dbg.log1(`Error: Config file path should be a valid path`, config_file_path, err);
@@ -546,9 +603,10 @@ async function get_health_status(argv, config_fs) {
         const deployment_type = argv.deployment_type || 'nc';
         const all_account_details = get_boolean_or_string_value(argv.all_account_details);
         const all_bucket_details = get_boolean_or_string_value(argv.all_bucket_details);
+        const all_connection_details = get_boolean_or_string_value(argv.all_connection_details);
 
         if (deployment_type === 'nc') {
-            const health = new NSFSHealth({ https_port, all_account_details, all_bucket_details, config_fs });
+            const health = new NSFSHealth({ https_port, all_account_details, all_bucket_details, all_connection_details, config_fs });
             const health_status = await health.nc_nsfs_health();
             write_stdout_response(ManageCLIResponse.HealthStatus, health_status);
         } else {
diff --git a/src/manage_nsfs/manage_nsfs_constants.js b/src/manage_nsfs/manage_nsfs_constants.js
@@ -74,7 +74,7 @@ const VALID_OPTIONS_GLACIER = {
 };
 
 const VALID_OPTIONS_DIAGNOSE = {
-    'health': new Set([ 'https_port', 'deployment_type', 'all_account_details', 'all_bucket_details', ...CLI_MUTUAL_OPTIONS]),
+    'health': new Set([ 'https_port', 'deployment_type', 'all_account_details', 'all_bucket_details', 'all_connection_details', ...CLI_MUTUAL_OPTIONS]),
     'gather-logs': new Set([ CONFIG_ROOT_FLAG]),
     'metrics': new Set([CONFIG_ROOT_FLAG])
 };
@@ -143,6 +143,7 @@ const OPTION_TYPE = {
     deployment_type: 'string',
     all_account_details: 'boolean',
     all_bucket_details: 'boolean',
+    all_connection_details: 'boolean',
     https_port: 'number',
     debug: 'number',
     // upgrade options
diff --git a/src/manage_nsfs/manage_nsfs_help_utils.js b/src/manage_nsfs/manage_nsfs_help_utils.js
@@ -366,12 +366,13 @@ Usage:
 
 Flags:
 
-    --deployment_type       <string>        (optional)          Set the nsfs type for heath check.(default nc; Non Containerized)
-    --https_port            <number>        (optional)          Set the S3 endpoint listening HTTPS port to serve. (default config.ENDPOINT_SSL_PORT)
-    --all_account_details   <boolean>       (optional)          Set a flag for returning all account details.
-    --all_bucket_details    <boolean>       (optional)          Set a flag for returning all bucket details.
-    --debug                 <number>        (optional)          Use for increasing the log verbosity of health cli commands.
-    --config_root           <string>        (optional)          Set Configuration files path for Noobaa standalon NSFS. (default config.NSFS_NC_DEFAULT_CONF_DIR)
+    --deployment_type        <string>        (optional)          Set the nsfs type for heath check.(default nc; Non Containerized)
+    --https_port             <number>        (optional)          Set the S3 endpoint listening HTTPS port to serve. (default config.ENDPOINT_SSL_PORT)
+    --all_account_details    <boolean>       (optional)          Set a flag for returning all account details.
+    --all_bucket_details     <boolean>       (optional)          Set a flag for returning all bucket details.
+    --all_connection_details <boolean>       (optional)          Set a flag for returning all connection details.
+    --debug                  <number>        (optional)          Use for increasing the log verbosity of health cli commands.
+    --config_root            <string>        (optional)          Set Configuration files path for Noobaa standalon NSFS. (default config.NSFS_NC_DEFAULT_CONF_DIR)
 
 `;
 
diff --git a/src/test/unit_tests/test_nc_health.js b/src/test/unit_tests/test_nc_health.js
@@ -9,6 +9,7 @@ const sinon = require('sinon');
 const assert = require('assert');
 const config = require('../../../config');
 const pkg = require('../../../package.json');
+const fs = require('fs');
 const fs_utils = require('../../util/fs_utils');
 const nb_native = require('../../util/nb_native');
 const { ConfigFS } = require('../../sdk/config_fs');
@@ -44,6 +45,17 @@ const get_system_config_mock_default_response = [{
 } }];
 const default_mock_upgrade_status = { message: 'there is no in-progress upgrade' };
 
+const connection_from_file_path = path.join(tmp_fs_path, 'connection_from_file');
+const connection_from_file = {
+  agent_request_object: {
+    host: "localhost",
+    port: 9999,
+    timeout: 100
+  },
+  notification_protocol: "http",
+  name: "http_notif",
+};
+
 mocha.describe('nsfs nc health', function() {
 
     const config_root = path.join(tmp_fs_path, 'config_root_nsfs_health');
@@ -210,6 +222,17 @@ mocha.describe('nsfs nc health', function() {
             assert_config_dir_status(health_status, valid_system_json.config_directory);
         });
 
+        mocha.it('health should report on invalid connections', async function() {
+            fs.writeFileSync(connection_from_file_path, JSON.stringify(connection_from_file));
+            await exec_manage_cli(TYPES.CONNECTION, ACTIONS.ADD, { config_root, from_file: connection_from_file_path });
+
+            Health.all_connection_details = true;
+            const health_status = await Health.nc_nsfs_health();
+            Health.all_connection_details = false;
+
+            assert.strictEqual(health_status.checks.connections_status.invalid_storages[0].name, connection_from_file.name);
+        });
+
         mocha.it('NooBaa service is inactive', async function() {
             set_mock_functions(Health, {
                 get_service_state: [{ service_status: 'inactive', pid: 0 }],
diff --git a/src/util/notifications_util.js b/src/util/notifications_util.js
@@ -351,10 +351,12 @@ async function test_notifications(notifs, nc_config_dir) {
         return;
     }
     let connect_files_dir = config.NOTIFICATION_CONNECT_DIR;
+    let config_fs;
     if (nc_config_dir) {
-        connect_files_dir = new ConfigFS(nc_config_dir).connections_dir_path;
+        config_fs = new ConfigFS(nc_config_dir);
+        connect_files_dir = config_fs.connections_dir_path;
     }
-    const notificator = new Notificator({connect_files_dir});
+    const notificator = new Notificator({connect_files_dir, nc_config_fs: config_fs});
     for (const notif of notifs) {
         let connect;
         let connection;
