Skip to content

[Backport into 5.18] Notif backports #8919

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Mar 30, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 16 additions & 2 deletions src/api/object_api.js
Original file line number Diff line number Diff line change
Expand Up @@ -1166,9 +1166,23 @@ module.exports = {
deleted_objects: {
type: 'array',
items: {
$ref: '#/definitions/object_info'
oneOf: [
{
$ref: '#/definitions/object_info',
},
{
type: 'object',
properties: {
err_code: {
type: 'string',
enum: ['AccessDenied', 'InternalError']
},
err_message: { type: 'string' }
}
}
]
}
}
},
}
},
auth: { system: 'admin' }
Expand Down
2 changes: 2 additions & 0 deletions src/endpoint/s3/ops/s3_post_object_uploadId.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
8 changes: 5 additions & 3 deletions src/endpoint/s3/s3_bucket_logging.js
Original file line number Diff line number Diff line change
Expand Up @@ -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' ||
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion src/endpoint/s3/s3_rest.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,7 +175,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);
}
Expand Down
4 changes: 2 additions & 2 deletions src/server/bg_services/lifecycle.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,13 +68,13 @@ async function handle_bucket_rule(system, rule, j, bucket) {
})
});

//dbg.log0("LIFECYCLE PROCESSING res =", res);

if (res.deleted_objects) {

const writes = [];

for (const deleted_obj of res.deleted_objects) {
//if deletion has failed, don't send a notification
if (deleted_obj.err_code) continue;
for (const notif of bucket.notifications) {
if (check_notif_relevant(notif, {
op_name: 'lifecycle_delete',
Expand Down
17 changes: 14 additions & 3 deletions src/server/object_services/object_server.js
Original file line number Diff line number Diff line change
Expand Up @@ -964,7 +964,7 @@ async function delete_multiple_objects_by_filter(req) {

const { objects } = await MDStore.instance().find_objects(query);

await delete_multiple_objects(_.assign(req, {
const delete_results = await delete_multiple_objects(_.assign(req, {
rpc_params: {
bucket: req.bucket.name,
objects: _.map(objects, obj => ({
Expand All @@ -978,8 +978,19 @@ async function delete_multiple_objects_by_filter(req) {
if (reply_objects) {
//reply needs to include deleted objects
//(this is used for LifecycleExpiratoin event notifications)
//so map the md into (api friendly) object info
reply.deleted_objects = _.map(objects, get_object_info);
//so map the md into (api friendly) object info,
//or incude the error if deletion failed
reply.deleted_objects = [];
for (let i = 0; i < objects.length; ++i) {
if (delete_results[i].err_code) {
reply.deleted_objects[i] = {
err_code: delete_results[i].err_code,
err_message: delete_results[i].err_message
};
} else {
reply.deleted_objects[i] = get_object_info(objects[i]);
}
}
}

return reply;
Expand Down
71 changes: 63 additions & 8 deletions src/test/unit_tests/test_notifications.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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;
Expand All @@ -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
});
});


Expand All @@ -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
});
});


Expand All @@ -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,
Expand All @@ -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
Expand Down
5 changes: 3 additions & 2 deletions src/util/notifications_util.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down