Skip to content

Server side encryption migrations should not be reported as an object… #6292

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.

Sign up for GitHub

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
bert-e merged 2 commits into from
Sep 21, 2026
+4 −5
Merged

Server side encryption migrations should not be reported as an object… #6292

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
9ad829b
Server side encryption migrations should not be reported as an object…
SylvainSenechal Sep 14, 2026
1bf89fa
Reformat with prettier
SylvainSenechal Sep 14, 2026
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
9 changes: 4 additions & 5 deletions lib/api/apiUtils/bucket/updateEncryption.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,18 +77,17 @@ function updateObjectEncryption(bucket, objMD, objectKey, log, keyArnPrefix, opt
dataLocator.masterKeyId = `${keyArnPrefix}${dataLocator.masterKeyId}`;
}
}
// Reformating the key arn is not an operation that should trigger any

@claude claude Bot Sep 21, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Typo: "Reformating" → "Reformatting" (double t).

Suggested change
// Reformating the key arn is not an operation that should trigger any
// Reformatting the key arn is not an operation that should trigger any

@SylvainSenechal SylvainSenechal Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Spot it faster next time its at least the third time that I push and I just approved and created the waterfall branch -_-

// oplog driven logic (CRR, bucket notifications).
// eslint-disable-next-line no-param-reassign
objMD.originOp = 's3:ObjectCreated:Copy';
// Copy should be tested for 9.5 in INTGR-1038

@SylvainSenechal SylvainSenechal Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BourgoisMickael On this old ticket : https://scality.atlassian.net/browse/INTGR-1038

you wrote a comment "It will trigger new bucket notification, but it will not trigger a replication for CRR"

With this change, it will not trigger bucket notif anymore. But I think you have integration tests that may expect it, so I'm letting you know you may have to update your tests

@BourgoisMickael BourgoisMickael Sep 17, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did that to follow the AWS way of updating the encryption: you do a copy of the object on itself while changing the encryption in metadata.

This is an internal operation but follows the way a user would do.

I can't find that actionUpdateObjectEncryption in the bucket notification documentation.

@SylvainSenechal SylvainSenechal Sep 18, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well that's the point 🤔 actionUpdateObjectEncryption is not in the list of notifications, so it wont trigger a notif unlike the existing ObjectCreated

@BourgoisMickael BourgoisMickael Sep 18, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I though we wanted to actually receive a notification (even if it's internal change) as the metadata changes.

Is it how it's done in AWS as well ?

Otherwise I'm fine with it. Should this fix ship in development/9.4 ?

@SylvainSenechal SylvainSenechal Sep 21, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im gonna put it on 9.4

I don't know about AWS, but I guess there is no concept of "sse migration" in aws

// to make sure it does not impact backbeat CRR / bucket notif
objMD.originOp = 's3:UpdateObjectEncryption';
const params = getVersionSpecificMetadataOptions(objMD, config.nullVersionCompatMode);

log.info('reformating object encryption key', { oldKey: key, newKey, skipUpdate: opts.skipObjectUpdate });
if (opts.skipObjectUpdate) {
return cb(null, bucket, objMD);
}
return metadata.putObjectMD(bucket.getName(), objectKey, objMD, params,
log, err => cb(err, bucket, objMD));
return metadata.putObjectMD(bucket.getName(), objectKey, objMD, params, log, err => cb(err, bucket, objMD));
}

/**
Expand Down
Loading