Skip to content

Commit 138b236

Browse files
authored
bucket notification - align test notif structure (#8951)
Signed-off-by: Amit Prinz Setter <[email protected]>
1 parent 2ad34e2 commit 138b236

File tree

6 files changed

+49
-13
lines changed

6 files changed

+49
-13
lines changed

docs/bucket-notifications.md

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,14 @@ A notification json has these fields:
1010
If not specified, the notification is relevant for all events.
1111
- TopicArn: The connection file (see below). (To specify a Kafka target topic, see "Kafka Connection Fields" below).
1212

13+
Note that in alignment with AWS notification configuration, a test notification is sent to the external server during put-bucket-notification-configuration execution.
14+
A successful test notification is a prequisite for the put-bucket-notification-configuration op.
15+
1316
Example for a bucket's notification configuration on containerized environment, in a file:
1417
{
1518
"TopicConfigurations": [
1619
{
17-
"Id": "created_from_s3op",
20+
"Id": "created_from_s3op",
1821
"TopicArn": "secret-name/connect.json",
1922
"Events": [
2023
"s3:ObjectCreated:*"
@@ -23,12 +26,12 @@ Example for a bucket's notification configuration on containerized environment,
2326
]
2427
}
2528

26-
Example for a bucket's notification configuration on Non-containerized environment, in a file:
29+
Example for a bucket's notification configuration on a non-containerized environment, in a file:
2730
{
2831
"TopicConfigurations": [
2932
{
30-
"Id": "created_from_s3op",
31-
"TopicArn": "secret-name/connect",
33+
"Id": "created_from_s3op",
34+
"TopicArn": "connect",
3235
"Events": [
3336
"s3:ObjectCreated:*"
3437
]

src/cmd/manage_nsfs.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -353,7 +353,7 @@ async function list_bucket_config_files(wide, filters = {}) {
353353
async function bucket_management(action, user_input) {
354354
const data = action === ACTIONS.LIST ? undefined : await fetch_bucket_data(action, user_input);
355355
await manage_nsfs_validations.validate_bucket_args(config_fs, data, action);
356-
await manage_nsfs_validations.validate_bucket_notifications(config_fs, user_input.notifications);
356+
await manage_nsfs_validations.validate_bucket_notifications(config_fs, user_input);
357357

358358
let response = {};
359359
if (action === ACTIONS.ADD) {

src/endpoint/s3/ops/s3_put_bucket_notification.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ async function put_bucket_notification(req) {
2828
//test new notifications.
2929
//if test notification fails, fail the put op
3030
const err = await notif_util.test_notifications(topic_configuration,
31-
req.object_sdk.nsfs_config_root);
31+
req.object_sdk.nsfs_config_root, req);
3232

3333
if (err) {
3434
let message = "Test notification failed: " + (err.message || err.code);

src/manage_nsfs/manage_nsfs_validations.js

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ const { check_root_account_owns_user } = require('../nc/nc_utils');
1818
const { validate_username } = require('../util/validation_utils');
1919
const notifications_util = require('../util/notifications_util');
2020
const version_utils = require('../util/versions_utils');
21+
const crypto = require('crypto');
2122

2223
/////////////////////////////
2324
//// GENERAL VALIDATIONS ////
@@ -495,15 +496,21 @@ async function validate_bucket_args(config_fs, data, action) {
495496
* When setting notifications, we are supposed to send a test notification.
496497
* If this test fails, we fail the user's request.
497498
* @param {object} config_fs
498-
* @param {object} notifications bucket's notification conf
499+
* @param {object} user_input user's input, including new notifications, if any
499500
* @returns {Promise} Error encountered during notification test, if any
500501
*/
501-
async function validate_bucket_notifications(config_fs, notifications) {
502-
if (!notifications) {
502+
async function validate_bucket_notifications(config_fs, user_input) {
503+
if (!user_input.notifications) {
503504
return;
504505
}
505506
//test_notification returns an error if notification fails for the given config
506-
const test_notif_err = await notifications_util.test_notifications(notifications, config_fs.config_root);
507+
const test_notif_err = await notifications_util.test_notifications(
508+
user_input.notifications,
509+
config_fs.config_root,
510+
{
511+
params: {bucket: user_input.name},
512+
request_id: crypto.randomUUID().toString()
513+
});
507514
if (test_notif_err) {
508515
throw_cli_error(ManageCLIError.InvalidArgument, "Failed to update notifications", test_notif_err);
509516
}

src/test/unit_tests/test_notifications.js

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ let expected_bucket;
5454
let expected_event_name;
5555
let expected_key;
5656
let expected_eTag;
57+
let expect_test;
5758

5859
// eslint-disable-next-line max-lines-per-function
5960
mocha.describe('notifications', function() {
@@ -90,7 +91,10 @@ mocha.describe('notifications', function() {
9091
const input = Buffer.concat(chunks);
9192
const notif = JSON.parse(input.toString());
9293

93-
if (notif !== "test notification") {
94+
if (expect_test) {
95+
assert.strictEqual(notif.Records[0].Event, "s3:TestEvent", 'wrong event name in notification');
96+
expect_test = false;
97+
} else {
9498
assert.strictEqual(notif.Records[0].s3.bucket.name, expected_bucket, 'wrong bucket name in notification');
9599
assert.strictEqual(notif.Records[0].eventName, expected_event_name, 'wrong event name in notification');
96100
assert.strictEqual(notif.Records[0].s3.object.key, expected_key, 'wrong key in notification');
@@ -108,6 +112,10 @@ mocha.describe('notifications', function() {
108112
});
109113

110114
mocha.it('set/get notif conf s3ops', async () => {
115+
116+
server_done = false;
117+
expect_test = true;
118+
111119
await s3.putBucketNotificationConfiguration({
112120
Bucket: bucket,
113121
NotificationConfiguration: {
@@ -118,6 +126,8 @@ mocha.describe('notifications', function() {
118126
},
119127
});
120128

129+
assert(server_done);
130+
121131
const get = await s3.getBucketNotificationConfiguration({Bucket: bucket});
122132
assert.strictEqual(get.TopicConfigurations[0].Id, 'system_test_http_no_event');
123133
});
@@ -155,6 +165,7 @@ mocha.describe('notifications', function() {
155165

156166
mocha.it('notifications with event filtering', async () => {
157167

168+
expect_test = true;
158169
const set = await s3.putBucketNotificationConfiguration({
159170
Bucket: bucket,
160171
NotificationConfiguration: {

src/util/notifications_util.js

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -343,7 +343,7 @@ function get_connection(connect) {
343343
* @returns {Promise} Error while testing, if any.
344344
*/
345345

346-
async function test_notifications(notifs, nc_config_dir) {
346+
async function test_notifications(notifs, nc_config_dir, req) {
347347
//notifs can be empty in case we're removing the notification from the bucket
348348
if (!notifs) {
349349
return;
@@ -364,7 +364,7 @@ async function test_notifications(notifs, nc_config_dir) {
364364
connect = await notificator.parse_connect_file(notif.topic[0]);
365365
connection = get_connection(connect);
366366
await connection.connect();
367-
await connection.promise_notify({notif: "test notification"}, async (notif_cb, err_cb, err) => {
367+
await connection.promise_notify(compose_notification_test(req), async (notif_cb, err_cb, err) => {
368368
failure = true;
369369
notif_failure = err;
370370
});
@@ -496,6 +496,21 @@ function compose_meta(record, notif_conf, bucket) {
496496
};
497497
}
498498

499+
function compose_notification_test(req) {
500+
return {
501+
notif: {
502+
Records: [{
503+
Service: "NooBaa",
504+
Event: "s3:TestEvent",
505+
Time: new Date().toISOString(),
506+
Bucket: req.params.bucket,
507+
RequestId: req.request_id,
508+
HostId: process.env.NODE_NAME || os.hostname()
509+
}]
510+
}
511+
};
512+
}
513+
499514
function _get_system_name(req) {
500515

501516
if (req && req.object_sdk && req.object_sdk.nsfs_system) {

0 commit comments

Comments
 (0)