Merge pull request #8989 from alphaprinz/notif_backports

[backport into 5.18] Notif backports

Author: nimrod-becker

docs/bucket-notifications.md

@@ -10,11 +10,14 @@ A notification json has these fields:
           If not specified, the notification is relevant for all events.
 - TopicArn: The connection file (see below). (To specify a Kafka target topic, see "Kafka Connection Fields" below).
 
+Note that in alignment with AWS notification configuration, a test notification is sent to the external server during put-bucket-notification-configuration execution.
+A successful test notification is a prequisite for the put-bucket-notification-configuration op.
+
 Example for a bucket's notification configuration on containerized environment, in a file:
 {
     "TopicConfigurations": [
         {
-	    "Id": "created_from_s3op",
+            "Id": "created_from_s3op",
             "TopicArn": "secret-name/connect.json",
             "Events": [
                 "s3:ObjectCreated:*"
@@ -23,12 +26,12 @@ Example for a bucket's notification configuration on containerized environment,
     ]
 }
 
-Example for a bucket's notification configuration on Non-containerized environment, in a file:
+Example for a bucket's notification configuration on a non-containerized environment, in a file:
 {
     "TopicConfigurations": [
         {
-	    "Id": "created_from_s3op",
-            "TopicArn": "secret-name/connect",
+            "Id": "created_from_s3op",
+            "TopicArn": "connect",
             "Events": [
                 "s3:ObjectCreated:*"
             ]

src/cmd/manage_nsfs.js

@@ -286,7 +286,7 @@ async function delete_bucket(data, force) {
 async function bucket_management(action, user_input) {
     const data = action === ACTIONS.LIST ? undefined : await fetch_bucket_data(action, user_input);
     await manage_nsfs_validations.validate_bucket_args(config_fs, data, action);
-    await manage_nsfs_validations.validate_bucket_notifications(config_fs, user_input.notifications);
+    await manage_nsfs_validations.validate_bucket_notifications(config_fs, user_input);
 
     let response = {};
     if (action === ACTIONS.ADD) {

src/endpoint/s3/ops/s3_put_bucket_notification.js

@@ -28,7 +28,7 @@ async function put_bucket_notification(req) {
     //test new notifications.
     //if test notification fails, fail the put op
     const err = await notif_util.test_notifications(topic_configuration,
-        req.object_sdk.nsfs_config_root);
+        req.object_sdk.nsfs_config_root, req);
 
     if (err) {
         let message = "Test notification failed: " + (err.message || err.code);

src/manage_nsfs/manage_nsfs_validations.js

@@ -16,6 +16,7 @@ const { TYPES, ACTIONS, VALID_OPTIONS, OPTION_TYPE, FROM_FILE, BOOLEAN_STRING_VA
 const { check_root_account_owns_user } = require('../nc/nc_utils');
 const { validate_username } = require('../util/validation_utils');
 const notifications_util = require('../util/notifications_util');
+const crypto = require('crypto');
 
 /////////////////////////////
 //// GENERAL VALIDATIONS ////
@@ -454,15 +455,21 @@ async function validate_bucket_args(config_fs, data, action) {
  * When setting notifications, we are supposed to send a test notification.
  * If this test fails, we fail the user's request.
  * @param {object} config_fs
- * @param {object} notifications bucket's notification conf
+ * @param {object} user_input user's input, including new notifications, if any
  * @returns {Promise} Error encountered during notification test, if any
  */
-async function validate_bucket_notifications(config_fs, notifications) {
-    if (!notifications) {
+async function validate_bucket_notifications(config_fs, user_input) {
+    if (!user_input.notifications) {
         return;
     }
     //test_notification returns an error if notification fails for the given config
-    const test_notif_err = await notifications_util.test_notifications(notifications, config_fs.config_root);
+    const test_notif_err = await notifications_util.test_notifications(
+        user_input.notifications,
+        config_fs.config_root,
+        {
+            params: {bucket: user_input.name},
+            request_id: crypto.randomUUID().toString()
+        });
     if (test_notif_err) {
         throw_cli_error(ManageCLIError.InvalidArgument, "Failed to update notifications", test_notif_err);
     }

src/test/unit_tests/test_notifications.js

@@ -54,6 +54,7 @@ let expected_bucket;
 let expected_event_name;
 let expected_key;
 let expected_eTag;
+let expect_test;
 
 // eslint-disable-next-line max-lines-per-function
 mocha.describe('notifications', function() {
@@ -90,7 +91,10 @@ mocha.describe('notifications', function() {
                 const input = Buffer.concat(chunks);
                 const notif = JSON.parse(input.toString());
 
-                if (notif !== "test notification") {
+                if (expect_test) {
+                    assert.strictEqual(notif.Records[0].Event, "s3:TestEvent", 'wrong event name in notification');
+                    expect_test = false;
+                } else {
                     assert.strictEqual(notif.Records[0].s3.bucket.name, expected_bucket, 'wrong bucket name in notification');
                     assert.strictEqual(notif.Records[0].eventName, expected_event_name, 'wrong event name in notification');
                     assert.strictEqual(notif.Records[0].s3.object.key, expected_key, 'wrong key in notification');
@@ -108,6 +112,10 @@ mocha.describe('notifications', function() {
         });
 
         mocha.it('set/get notif conf s3ops', async () => {
+
+            server_done = false;
+            expect_test = true;
+
             await s3.putBucketNotificationConfiguration({
                 Bucket: bucket,
                 NotificationConfiguration: {
@@ -118,6 +126,8 @@ mocha.describe('notifications', function() {
                 },
             });
 
+            assert(server_done);
+
             const get = await s3.getBucketNotificationConfiguration({Bucket: bucket});
             assert.strictEqual(get.TopicConfigurations[0].Id, 'system_test_http_no_event');
         });
@@ -155,6 +165,7 @@ mocha.describe('notifications', function() {
 
         mocha.it('notifications with event filtering', async () => {
 
+            expect_test = true;
             const set = await s3.putBucketNotificationConfiguration({
                 Bucket: bucket,
                 NotificationConfiguration: {

src/util/notifications_util.js

@@ -161,7 +161,7 @@ class Notificator {
 
     async handle_failed_notification(notif, failure_append, err) {
         if (this.nc_config_fs) {
-            new NoobaaEvent(NoobaaEvent.NOTIFICATION_FAILED).create_event(notif?.meta?.name, err, err.toString());
+            new NoobaaEvent(NoobaaEvent.NOTIFICATION_FAILED).create_event(notif?.meta?.name, err, err?.toString());
         }
 
         if (notif) {
@@ -343,7 +343,7 @@ function get_connection(connect) {
  * @returns {Promise} Error while testing, if any.
  */
 
-async function test_notifications(notifs, nc_config_dir) {
+async function test_notifications(notifs, nc_config_dir, req) {
     //notifs can be empty in case we're removing the notification from the bucket
     if (!notifs) {
         return;
@@ -364,7 +364,7 @@ async function test_notifications(notifs, nc_config_dir) {
             connect = await notificator.parse_connect_file(notif.topic[0]);
             connection = get_connection(connect);
             await connection.connect();
-            await connection.promise_notify({notif: "test notification"}, async (notif_cb, err_cb, err) => {
+            await connection.promise_notify(compose_notification_test(req), async (notif_cb, err_cb, err) => {
                 failure = true;
                 notif_failure = err;
             });
@@ -496,12 +496,28 @@ function compose_meta(record, notif_conf, bucket) {
     };
 }
 
+function compose_notification_test(req) {
+    return {
+        notif: {
+            Records: [{
+                Service: "NooBaa",
+                Event: "s3:TestEvent",
+                Time: new Date().toISOString(),
+                Bucket: req.params.bucket,
+                RequestId: req.request_id,
+                HostId: process.env.NODE_NAME || os.hostname()
+            }]
+        }
+    };
+}
+
 function _get_system_name(req) {
 
+    //in NC case - return node name
     if (req && req.object_sdk && req.object_sdk.nsfs_system) {
-        const name = Object.keys(req.object_sdk.nsfs_system)[0];
+        const name = process.env.NODE_NAME || os.hostname();
         return name;
-    } else {
+    } else { //in containerized - return system's name
         //see comment on Notificator._can_run() for the require here
         const system_store = require('../server/system_services/system_store').get_instance();
         return system_store.data.systems[0].name;