bucket notifications - fixes for CompleteMultipartUpload etag, size. Also statusCode check (#8917)

Signed-off-by: Amit Prinz Setter <alphaprinz@gmail.com>

Author: alphaprinz

src/endpoint/s3/ops/s3_post_object_uploadId.js

@@ -43,6 +43,8 @@ async function post_object_uploadId(req, res) {
         res.setHeader('x-amz-version-id', reply.version_id);
     }
 
+    res.size_for_notif = reply.size;
+
     return {
         CompleteMultipartUploadResult: {
             Bucket: req.params.bucket,

src/endpoint/s3/s3_bucket_logging.js

@@ -8,7 +8,7 @@ const { Buffer } = require('node:buffer');
 const config = require('../../../config');
 const {compose_notification_req, check_notif_relevant, check_free_space_if_needed} = require('../../util/notifications_util');
 
-async function send_bucket_op_logs(req, res) {
+async function send_bucket_op_logs(req, res, reply) {
     if (req.params && req.params.bucket &&
         !(req.op_name === 'put_bucket' ||
           req.op_name === 'delete_bucket' ||
@@ -36,8 +36,10 @@ async function send_bucket_op_logs(req, res) {
 
         if (req.notification_logger && bucket_info.notifications) {
             for (const notif_conf of bucket_info.notifications) {
-                if (check_notif_relevant(notif_conf, req)) {
-                    const notif = compose_notification_req(req, res, bucket_info, notif_conf);
+                //write the notification log only if request is successful (ie res.statusCode < 300)
+                //and event is actually relevant to the notification conf
+                if (res.statusCode < 300 && check_notif_relevant(notif_conf, req)) {
+                    const notif = compose_notification_req(req, res, bucket_info, notif_conf, reply);
                     dbg.log1("logging notif ", notif_conf, ", notif = ", notif);
                     writes_aggregate.push({
                         file: req.notification_logger,

src/endpoint/s3/s3_rest.js

@@ -176,7 +176,7 @@ async function handle_request(req, res) {
     http_utils.send_reply(req, res, reply, options);
     collect_bucket_usage(op, req, res);
     try {
-        await s3_logging.send_bucket_op_logs(req, res); // logging again with result
+        await s3_logging.send_bucket_op_logs(req, res, reply); // logging again with result
     } catch (err) {
         dbg.error(`Could not log bucket operation (after operation ${req.op_name}):`, err);
     }

src/test/unit_tests/test_notifications.js

@@ -52,6 +52,8 @@ let http_server = null;
 let server_done = false;
 let expected_bucket;
 let expected_event_name;
+let expected_key;
+let expected_eTag;
 
 // eslint-disable-next-line max-lines-per-function
 mocha.describe('notifications', function() {
@@ -91,9 +93,10 @@ mocha.describe('notifications', function() {
                 if (notif !== "test notification") {
                     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');
+                    const expected_eTag_trimmed = expected_eTag && expected_eTag.substring(1, expected_eTag.length - 1);
+                    assert.strictEqual(notif.Records[0].s3.object.eTag, expected_eTag_trimmed, 'wrong eTag in notification');
                 }
-
                 res.writeHead(200, {'Content-Type': 'text/plain'});
                 res.end();
                 server_done = true;
@@ -120,13 +123,18 @@ mocha.describe('notifications', function() {
         });
 
         mocha.it('simple notif put', async () => {
-            await s3.putObject({
+            const res = await s3.putObject({
                 Bucket: bucket,
                 Key: 'f1',
                 Body: 'this is the body',
             });
 
-            await notify_await_result({bucket_name: bucket, event_name: 'ObjectCreated:Put'});
+            await notify_await_result({
+                bucket_name: bucket,
+                event_name: 'ObjectCreated:Put',
+                key: "f1",
+                etag: res.ETag
+            });
         });
 
 
@@ -136,7 +144,12 @@ mocha.describe('notifications', function() {
                 Key: 'f1',
             });
 
-            await notify_await_result({bucket_name: bucket, event_name: 'ObjectRemoved:Delete'});
+            await notify_await_result({
+                bucket_name: bucket,
+                event_name: 'ObjectRemoved:Delete',
+                key: "f1",
+                etag: undefined
+            });
         });
 
 
@@ -155,13 +168,18 @@ mocha.describe('notifications', function() {
 
             assert.strictEqual(set.$metadata.httpStatusCode, 200);
 
-            await s3.putObject({
+            const res = await s3.putObject({
                 Bucket: bucket,
                 Key: 'f2',
                 Body: 'this is the body',
             });
 
-            await notify_await_result({bucket_name: bucket, event_name: 'ObjectCreated:Put'});
+            await notify_await_result({
+                bucket_name: bucket,
+                event_name: 'ObjectCreated:Put',
+                key: "f2",
+                etag: res.ETag
+            });
 
             await s3.deleteObject({
                 Bucket: bucket,
@@ -171,16 +189,53 @@ mocha.describe('notifications', function() {
             //there shouldn't be a notification for the delete, wait 2 seconds to validate this
             await notify_await_result({timeout: 2000});
         });
+
+        mocha.it('multipart', async () => {
+            const res_create = await s3.createMultipartUpload({
+                Bucket: bucket,
+                Key: 'mp1'
+            });
+
+            const res_upload = await s3.uploadPart({
+                Bucket: bucket,
+                Key: 'mp1',
+                UploadId: res_create.UploadId,
+                PartNumber: 1,
+                Body: 'this is the body'
+            });
+
+            const res_complete = await s3.completeMultipartUpload({
+                Bucket: bucket,
+                Key: 'mp1',
+                UploadId: res_create.UploadId,
+                MultipartUpload: {
+                    Parts: [{
+                        ETag: res_upload.ETag,
+                        PartNumber: 1
+                    }]
+                }
+            });
+
+            await notify_await_result({
+                bucket_name: bucket,
+                event_name: 'ObjectCreated:CompleteMultipartUpload',
+                key: "mp1",
+                etag: res_complete.ETag
+            });
+        });
+
     });
 
 });
 
 const step_wait = 100;
-async function notify_await_result({bucket_name, event_name, timeout}) {
+async function notify_await_result({bucket_name, event_name, etag, key, timeout = undefined}) {
 
     //remember expected result here so server could compare it to actual result later
     expected_bucket = bucket_name;
     expected_event_name = event_name;
+    expected_eTag = etag;
+    expected_key = key;
     server_done = false;
 
     //busy-sync wait for server

src/util/notifications_util.js

@@ -418,8 +418,9 @@ function compose_notification_base(notif_conf, bucket, req) {
 }
 
 //see https://docs.aws.amazon.com/AmazonS3/latest/userguide/notification-content-structure.html
-function compose_notification_req(req, res, bucket, notif_conf) {
-    let eTag = res.getHeader('ETag');
+function compose_notification_req(req, res, bucket, notif_conf, reply) {
+    //most s3 ops put etag in header. CompleteMultipartUploadResult is an exception.
+    let eTag = res.getHeader('ETag') || reply?.CompleteMultipartUploadResult?.ETag;
     //eslint-disable-next-line
     if (eTag && eTag.startsWith('\"') && eTag.endsWith('\"')) {
         eTag = eTag.substring(1, eTag.length - 1);