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

[SylvainSenechal]

Issue: CLDSRV-990

Taking a look at the ticket :
We have this updateObjectEncryption function setting
objMD.originOp = 's3:ObjectCreated:Copy';
all the time which is indeed problematic as it sends bucket notifications (although this is debatable, sending a notif is not exactly a P0..), and also can trigger transition (which is more problematic and the reason why this ticket was created scality/backbeat#2830 (comment))

But, upon looking, the function is called from 5 paths :

• objectPut

• objectCopy

• initMpu

• objectGet

• objectHead

The first 3 are writes, which calls updateEncryption with a flag, so that the originOp rewrite path is not even exercised at all (because these 3 writes will have their own logic to set origin op)

Only the 2 reads get/head can make a call that would wrongly set originOp to copy. But even then, this only happens in the even of a sse migration.

So I'm making a change on the originOp to use something that will not trigger bucket notif/transition

2 remarks :

• Could consider sending this pr on a lower cloudserver version
• updateEncryption is not tested at all, neither for this particular fix, or for anytging else at all, I could maybe add a test, didnt do it its too easy to create 500 lines of ai test crap

[bert-e]

Hello sylvainsenechal,

My role is to assist you with the merge of this
pull request. Please type @bert-e help to get information
on this process, or consult the user documentation.

Available options

name	description	privileged	authored
/after_pull_request	Wait for the given pull request id to be merged before continuing with the current one.
/bypass_author_approval	Bypass the pull request author's approval	⭐
/bypass_build_status	Bypass the build and test status	⭐
/bypass_commit_size	Bypass the check on the size of the changeset TBA	⭐
/bypass_incompatible_branch	Bypass the check on the source branch prefix	⭐
/bypass_jira_check	Bypass the Jira issue check	⭐
/bypass_peer_approval	Bypass the pull request peers' approval	⭐
/bypass_leader_approval	Bypass the pull request leaders' approval	⭐
/bypass_source_branch_lineage	Bypass the cross-branch contamination check	⭐
/approve	Instruct Bert-E that the author has approved the pull request.		✍️
/create_pull_requests	Allow the creation of integration pull requests.
/create_integration_branches	Allow the creation of integration branches.
/no_octopus	Prevent Wall-E from doing any octopus merge and use multiple consecutive merge instead
/unanimity	Change review acceptance criteria from one reviewer at least to all reviewers
/wait	Instruct Bert-E not to run until further notice.

Available commands

name	description	privileged
/help	Print Bert-E's manual in the pull request.
/status	Print Bert-E's current status in the pull request.
/clear	Remove all comments from Bert-E from the history TBA
/retry	Re-start a fresh build TBA
/build	Re-start a fresh build TBA
/force_reset	Delete integration branches & pull requests, and restart merge process from the beginning.
/reset	Try to remove integration branches unless there are commits on them which do not appear on the source branch.

Status report is not available.

[codecov]

Codecov Report

❌ Patch coverage is 0% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.55%. Comparing base (ca73322) to head (1f36e5b).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
lib/api/apiUtils/bucket/updateEncryption.js	0.00%	2 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
lib/api/apiUtils/bucket/updateEncryption.js	18.18% <0.00%> (+0.32%)	⬆️

@@               Coverage Diff                @@
##           development/9.5    #6292   +/-   ##
================================================
  Coverage            86.54%   86.55%           
================================================
  Files                  213      213           
  Lines                14615    14614    -1     
================================================
  Hits                 12649    12649           
+ Misses                1966     1965    -1     

Flag	Coverage Δ
checksums-disabled-tests	35.38% <0.00%> (+<0.01%)	⬆️
file-ft-tests	70.09% <0.00%> (+0.01%)	⬆️
file-ft-tests-null-compat	70.51% <0.00%> (-0.05%)	⬇️
kmip-ft-tests	28.15% <0.00%> (+<0.01%)	⬆️
mongo-v0-ft-tests	71.13% <0.00%> (-0.07%)	⬇️
mongo-v1-ft-tests	71.13% <0.00%> (+<0.01%)	⬆️
multiple-backend	36.13% <0.00%> (+<0.01%)	⬆️
s3c-ft-tests-v0	65.05% <0.00%> (+<0.01%)	⬆️
s3c-ft-tests-v0-null-compat	65.10% <0.00%> (+<0.01%)	⬆️
s3c-ft-tests-v1	65.03% <0.00%> (+<0.01%)	⬆️
sur-tests	36.64% <0.00%> (-0.90%)	⬇️
sur-tests-inflights	39.49% <0.00%> (+0.03%)	⬆️
unit	74.29% <0.00%> (+<0.01%)	⬆️
utapi-v2-tests	35.36% <0.00%> (+<0.01%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[bert-e]

Waiting for approval

The following approvals are needed before I can proceed with the merge:

• the author

• 2 peers

[SylvainSenechal]

@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]

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]

Adding Maha as you kinda created the ticket from the discussion on the bb pr, and Mickael which may be affected on the Ring tests