Skip to content

CLDSRV-1009: keep object metadata when restoring from cold storage - #6315

Merged
bert-e merged 5 commits into
development/9.4from
bugfix/CLDSRV-1009-restore-system-metadata
Oct 1, 2026
Merged

bert-e merged 5 commits into
development/9.4from
bugfix/CLDSRV-1009-restore-system-metadata

Conversation

@francoisferrand

Copy link
Copy Markdown
Contributor

When the cold backend restores an object, it writes it back with a PutObject (or CompleteMPU) on the archived version. That request carries the backend's own headers, not the user's, so the restored version was losing system metadata: content-type was reset to binary/octet-stream, and cache-control, content-disposition, content-encoding and expires were dropped.

The restore now takes these fields from the archived object, along with retention mode/date.

A second commit makes sure object lock info also comes only from the archived object: the object lock headers of the restore request and the bucket default retention are ignored, so a restore can't change the retention or add one the object didn't have.

Issue: CLDSRV-1009

The cold backend writes the restored object back with a PutObject (or
CompleteMultipartUpload) carrying its own headers, so the object ended up
with the SDK default content-type and lost cache-control,
content-disposition, content-encoding and expires. Object lock retention
was dropped too, or silently recomputed from the bucket default rule.

Take all of these from the archived metadata instead of the restore
request.

Issue: CLDSRV-1009
The restored version must carry the object lock info of the archived object.
The object lock headers of the restore request and the bucket default retention
could otherwise override it, or add a retention the archived object did not
have.

Issue: CLDSRV-1009
@bert-e

bert-e commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

Hello francoisferrand,

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 Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.51%. Comparing base (0ce6bca) to head (9ab6299).
⚠️ Report is 5 commits behind head on development/9.4.
✅ All tests successful. No failed tests found.

Additional details and impacted files

Impacted file tree graph

Files with missing lines Coverage Δ
lib/api/apiUtils/object/versioning.js 97.40% <100.00%> (+0.26%) ⬆️

... and 1 file with indirect coverage changes

@@                 Coverage Diff                 @@
##           development/9.4    #6315      +/-   ##
===================================================
+ Coverage            86.49%   86.51%   +0.02%     
===================================================
  Files                  212      212              
  Lines                14567    14585      +18     
===================================================
+ Hits                 12599    12618      +19     
+ Misses                1968     1967       -1     
Flag Coverage Δ
checksums-disabled-tests 35.29% <0.00%> (-0.05%) ⬇️
file-ft-tests 69.90% <0.00%> (-0.09%) ⬇️
file-ft-tests-null-compat 70.55% <100.00%> (+0.02%) ⬆️
kmip-ft-tests 28.06% <0.00%> (-0.04%) ⬇️
mongo-v0-ft-tests 71.18% <100.00%> (+0.03%) ⬆️
mongo-v1-ft-tests 71.13% <100.00%> (-0.02%) ⬇️
multiple-backend 36.07% <0.00%> (-0.05%) ⬇️
s3c-ft-tests-v0 64.94% <0.00%> (-0.07%) ⬇️
s3c-ft-tests-v0-null-compat 64.99% <0.00%> (-0.07%) ⬇️
s3c-ft-tests-v1 64.92% <0.00%> (-0.07%) ⬇️
sur-tests 37.59% <100.00%> (+0.07%) ⬆️
sur-tests-inflights 39.54% <100.00%> (+0.07%) ⬆️
unit 74.23% <100.00%> (+0.04%) ⬆️
utapi-v2-tests 35.27% <0.00%> (-0.05%) ⬇️

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.

francoisferrand and others added 2 commits September 30, 2026 16:34
The ACL of a restored object must be the one of the archived object:
ACL headers sent by the cold backend on the restore PutObject would
otherwise override it.

Issue: CLDSRV-1009
The cold storage restore functional tests for PutObject and CompleteMPU
with x-scal-s3-version-id still asserted the old behaviour, where
content-type and content-encoding were taken from the restore request.
These are system metadata of the archived object and are now preserved
on restore, so the tests must expect the archived values.

Issue: CLDSRV-1009

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@francoisferrand
francoisferrand force-pushed the bugfix/CLDSRV-1009-restore-system-metadata branch from 722d238 to b54d5fb Compare September 30, 2026 19:50

@DarkIsDude DarkIsDude left a comment

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.

LGTM 🙏

@scality scality deleted a comment from bert-e Oct 1, 2026
@bert-e

bert-e commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Request integration branches

Waiting for integration branch creation to be requested by the user.

To request integration branches, please comment on this pull request with the following command:

/create_integration_branches

Alternatively, the /approve and /create_pull_requests commands will automatically
create the integration branches.

1 similar comment
@bert-e

bert-e commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

Request integration branches

Waiting for integration branch creation to be requested by the user.

To request integration branches, please comment on this pull request with the following command:

/create_integration_branches

Alternatively, the /approve and /create_pull_requests commands will automatically
create the integration branches.

@francoisferrand

Copy link
Copy Markdown
Contributor Author

/approve

@scality scality deleted a comment from bert-e Oct 1, 2026
Comment thread lib/api/apiUtils/object/versioning.js
@bert-e

bert-e commented Oct 1, 2026

Copy link
Copy Markdown
Contributor

I have successfully merged the changeset of this pull request
into targetted development branches:

  • ✔️ development/9.4

  • ✔️ development/9.5

The following branches have NOT changed:

  • development/7.10
  • development/7.4
  • development/7.70
  • development/8.8
  • development/9.0
  • development/9.1
  • development/9.2
  • development/9.3

This pull request did not target the following hotfix branch(es) so they
were left untouched:

  • hotfix/7.4.9
  • hotfix/9.3.13
  • hotfix/7.4.4
  • hotfix/7.4.8
  • hotfix/7.70.51
  • hotfix/7.70.21
  • hotfix/7.70.11
  • hotfix/7.4.10
  • hotfix/7.4.3
  • hotfix/7.6.0
  • hotfix/9.0.7
  • hotfix/7.10.3
  • hotfix/7.10.1
  • hotfix/7.10.27
  • hotfix/7.10.4
  • hotfix/7.10.28
  • hotfix/7.9.0
  • hotfix/7.10.15
  • hotfix/7.2.0
  • hotfix/7.4.6
  • hotfix/7.8.0
  • hotfix/7.10.0
  • hotfix/7.10.30
  • hotfix/7.10.49
  • hotfix/7.4.1
  • hotfix/7.4.5
  • hotfix/7.70.45
  • hotfix/9.2.24
  • hotfix/6.4.7
  • hotfix/8.8.45
  • hotfix/7.10.8
  • hotfix/7.4.2
  • hotfix/9.2.36
  • hotfix/9.0.32
  • hotfix/7.7.0
  • hotfix/7.70.73
  • hotfix/7.4.0
  • hotfix/7.4.7
  • hotfix/7.10.2

Please check the status of the associated issue CLDSRV-1009.

Goodbye francoisferrand.

The following options are set: approve

@bert-e
bert-e merged commit 9ab6299 into development/9.4 Oct 1, 2026
36 of 37 checks passed
@bert-e
bert-e deleted the bugfix/CLDSRV-1009-restore-system-metadata branch October 1, 2026 18:25
@francoisferrand
francoisferrand restored the bugfix/CLDSRV-1009-restore-system-metadata branch October 1, 2026 18:42
@francoisferrand
francoisferrand deleted the bugfix/CLDSRV-1009-restore-system-metadata branch October 1, 2026 19:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants