🐛 use SUR date instead of the infostore one to have an update LastMod… by DarkIsDude · Pull Request #6121 · scality/cloudserver

DarkIsDude · 2026-03-27T13:34:31Z

Pull request template

Description

https://scality.atlassian.net/browse/CLDSRV-631 this PR introduce a regression. The associated cronjob was removed https://github.com/scality/zenko-operator/pull/540/changes#diff-c76a6b4555478f74e4c871dbf370d4e6e5ae54c4b8590b0581b1cc0894e83ae5R55 but the date used is still the one associated with it. The goal of this PR is to use the date from SUR.

curl -v --insecure https://10.160.99.90:8443/zenko/s3/test2/.system-d26a9498-cb7c-4a87-a44a-8ae204f5ba6c/capacity.xml?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=FRFIWTEZRPYUSHVY6WEP%2F20260327%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20260327T155955Z&X-Amz-Expires=900&X-Amz-Security-Token=eyJzYWx0IjoiQ0Z2bzd2Y29BY1ZZRHBZTXJ4Y0dpM2ExZ1NYcVBGaGpWdis5QThhU0NSMD0iLCJ0YWciOiJXaHNFWDUyMWlQWEFkWnV1WnRCOXNnPT0iLCJjaXBoZXJ0ZXh0IjoienBFRmpzeHMvYmRORzZZbU5mWFpJbGlHS01yWFliR3RaZW9NcUx0N3JXVjVJR1JnR0F1RExlZVdGSkUwdHVBNjBPbWpqeUhINkszWkp1Q1hkVWt5VGhrcUVsWStYTUpCdE5haWRjOHZBek9uUjJnc2p1L1pKRUdVUnZnMmVHMjRXS2JPZUQra1h0Q25nQWpFTjFWTWRsd25OcTBUMmw4WWNMaVBrY2owa1BaWnB2M2I0MEZCaVFsbFowOThnN2JSanhTZ2daVEtQNnNpY1hrYU0rK2tZbjMrMHhlS2NjcEFxWENWR3l6NjNxZWFuclFqNW1Ta2FDVHF1QVlCQlVGT0hnVnFWNFIzTjdiRUpCUTNSb1JGRGIzei9BaW85WERVcDE4RVNaZHVTaGxEN0ZaaktMVEJWL1hCNkRWNHd3bnRsMWRGOVRzZHlHOFZiWHFZSVN6elJ2SEpDeXlYTnNHNTBlUjNGQVNCSWZWTlc1QXlpOHk4cVVkOTFCMVBBUGxZaVJ1T284Z0twOWtvb0NFS2hCNUprSGc1Tzl2RFhRVEdoUkxvdnplaUIzbkRYTnIrL1FhZ0dmZ0tzSEMzZVIyT2hCYnhTUU50NVBrb012bGVodnh1dlNPZ01ucFFFczVudzZTeHBtMDlZVHRwOUJZazk4Q1IxcUZnVlJBMEZDTVA4a2dwU3VmbHh3dFNMZkhIVmNkN2lCWHh5MEJDaGVMLzE4NkZkMXhDaFVYT1A0d3o0Yjd4OHZKV1JYZm82WStWZlU2a1Q5cW52WGhOSmlJeGtmalU0aUgwT2k4bTAyU2lUeVRIdGJjMlBFK1dwaTVZL0xpbFpudjMwNVJlbWR1SDNLZStKTlpBS3FSdVYvM3ZxMnczdWlER1djMVh4SWJHNytxTWhlaWZVcnRENWsvMjg2ekY5UWxSTmpwYVZHdGZ6R2tVS3VQbk82NENubkt4UFcxc05wQyt4WEljUVV0WVF4MjlFYTlDSndZZEtlazJtYXVneXdUOE1zZFA5SkI5WVRTZHRqMzRWcXNuZ2tQMVdlZWZBMTZFYjdWNU5EZm5RS0pobjFpdUVhaGpVY1ViVll4L0ZOTEVBYW1wSlltN0plNjdDeEs0dmluOTI5QXpUMHJJand5a3lNam5Fa0x3VjVrdDV5WEJ1TmNPa1lUc3o5bzd3Tnp6Zlp3MnI4cHY5cVc0YXBhcnpxNHVOcTFXdjdZeG1VY2dLRExwYlFRc1VQWW5SREdkZDNpcDNzeUR3ZXcxMzZ5V2tMOGJYU0JFUXIxdHZmL1BRQTFRdUR6Q0s1TmtzWGd6cW0vdXorZ0FzQVVub1R2cTh1RzNjaGJnR0hzalFsa29MUjFsWTJGQ2t1bHF3NTlCUjJkVzJaRmptdFQ3aTJkWnk5T2pWZFRDUFc2QXM5N1Q4T3d5SXlKUytVZ3Q2YzA2a1JVZ2dNVHhETFBpQ2FUbDFJbS9rZ3YxTmZzVWg0aFhabFBvdnkrYlJHTWFrRW5sMXdQYXVzSzlIZHMxR3FvcGVkSjlNK0svSXhqeHpkS3B4YU1ZZUcwSDBVbGVnSkRQSjl0TmRhVE15MXZqZ0F4VzlXWm9qSkpMSXNXWHhDQzlIai95MGoySFN5WGFLWGdqYnZtMlFOcWFLckFHN1lXRUVSQkdQeG1HWktwRHlhcmFLdGVaQzVNWXU4ckNVdVdTc1U3ekQrMFlHMU9CVWlMTTJyUXdWQmdSWFZ2QlRRUlVUZTJPbFplUVNZY1dZNUg2aWNON1dYZ3pEdndOUm5SeWIvVzM5R2hUMGFCL0diYTlYRVI0K0NLT1A5UXgifQ%3D%3D&X-Amz-Signature=4ca11d205c394debb177a26b8e292493a1478a91364ffd96b306628db0e0ddc2&X-Amz-SignedHeaders=host

With the fix < last-modified: Fri, 27 Mar 2026 16:04:16 GMT
Without the fix < last-modified: Wed, 25 Mar 2026 14:12:11 GMT

bert-e · 2026-03-27T13:34:35Z

Hello darkisdude,

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	⭐
`/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 `TBA`
`/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.

bert-e · 2026-03-27T13:34:40Z

Incorrect fix version

The Fix Version/s in issue CLDSRV-878 contains:

9.1.14
9.2.32
9.3.6
9.4.0

Considering where you are trying to merge, I ignored possible hotfix versions and I expected to find:

9.1.14
9.2.33
9.3.6
9.4.0

Please check the Fix Version/s of CLDSRV-878, or the target
branch of this pull request.

lib/routes/veeam/get.js

lib/routes/veeam/head.js

codecov · 2026-03-27T13:42:15Z

Codecov Report

❌ Patch coverage is 95.00000% with 5 lines in your changes missing coverage. Please review.
✅ Project coverage is 83.88%. Comparing base (4aac273) to head (78fd519).
✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
lib/routes/veeam/utils.js	93.65%	4 Missing ⚠️
lib/routes/veeam/list.js	95.83%	1 Missing ⚠️

Additional details and impacted files

Files with missing lines	Coverage Δ
lib/routes/veeam/get.js	`91.66% <100.00%> (-5.64%)`	⬇️
lib/routes/veeam/head.js	`90.00% <100.00%> (ø)`
lib/routes/veeam/put.js	`91.66% <100.00%> (+0.23%)`	⬆️
lib/routes/veeam/list.js	`97.91% <95.83%> (+0.41%)`	⬆️
lib/routes/veeam/utils.js	`95.04% <93.65%> (+5.04%)`	⬆️

... and 1 file with indirect coverage changes

@@                 Coverage Diff                 @@
##           development/9.1    #6121      +/-   ##
===================================================
+ Coverage            83.79%   83.88%   +0.08%     
===================================================
  Files                  191      191              
  Lines                12350    12355       +5     
===================================================
+ Hits                 10349    10364      +15     
+ Misses                2001     1991      -10

Flag	Coverage Δ
file-ft-tests	`66.89% <14.00%> (+0.01%)`	⬆️
kmip-ft-tests	`27.05% <14.00%> (+0.02%)`	⬆️
mongo-v0-ft-tests	`68.25% <14.00%> (+0.05%)`	⬆️
mongo-v1-ft-tests	`68.26% <14.00%> (+0.06%)`	⬆️
multiple-backend	`34.30% <14.00%> (+0.02%)`	⬆️
sur-tests	`34.57% <66.00%> (-0.85%)`	⬇️
sur-tests-inflights	`36.47% <66.00%> (-0.04%)`	⬇️
unit	`69.69% <78.00%> (+0.02%)`	⬆️
utapi-v2-tests	`33.42% <14.00%> (+0.02%)`	⬆️

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.

…ified date Issue: CLDSRV-878

lib/routes/veeam/get.js

tests/unit/api/apiUtils/objectLockHelpers.js

Call UtilizationService.getUtilizationMetrics for capacity.xml on the HEAD and LIST routes, mirroring the existing pattern from GET. This ensures Last-Modified is derived from SUR metrics (not stale metadata) on all three routes. Fall back to new Date() when metrics are unavailable (404) or absent. Also use new Date() as the fallback in GET instead of the stored LastModified value. Issue: CLDSRV-878

Move the shared UtilizationService call pattern (bucket key derivation, 404 warn/fallback, error logging) into a single fetchCapacityMetrics helper in utils.js, used by GET, HEAD, and LIST routes. Issue: CLDSRV-878

Issue: CLDSRV-878

lib/routes/veeam/head.js

lib/routes/veeam/list.js

lib/routes/veeam/get.js

bert-e · 2026-04-01T09:41:42Z

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.

Issue: CLDSRV-878

francoisferrand · 2026-04-01T10:35:21Z

lib/routes/veeam/head.js

-        return responseContentHeaders(null, {}, getResponseHeader(request, data,
-            dataBuffer, modified, log), response, log);
-    });
+


nit: not really related to this PR, but I wonder why we need to make a call to getBucket even though we receive bucketMD as function parameter? do you know why or what is the difference?

To be honest I don't know. I have the feeling that nothing is different 😬. Maybe the idea was to keep the handler as independant as possible ?

lib/routes/veeam/utils.js

lib/routes/veeam/list.js

francoisferrand · 2026-04-01T10:43:14Z

lib/routes/veeam/utils.js

-        } else {
-            return callback(null, value.toString());
-        }
+    return new Promise((resolve, reject) => {


use promisify

in the guidelines, I think there was mention of never returning a promise (to ensure stack trace is correct)

Suggested change

return new Promise((resolve, reject) => {

return await utils.promisify(() => { ... })();

🤔 I'm not sure the usage of promisify here will add a value ? Finally it's the same ? Thanks for the await 🙏

lib/routes/veeam/utils.js

Issue: CLDSRV-878

DarkIsDude self-assigned this Mar 27, 2026

DarkIsDude commented Mar 27, 2026

View reviewed changes

lib/routes/veeam/get.js Outdated Show resolved Hide resolved

DarkIsDude commented Mar 27, 2026

View reviewed changes

lib/routes/veeam/head.js Show resolved Hide resolved

DarkIsDude force-pushed the bugfix/CLDSRV-878/use-sur-date branch from bb7499d to beee7a8 Compare March 27, 2026 14:03

🐛 use SUR date instead of the infostore one to have an update LastMod…

d9a5aed

…ified date Issue: CLDSRV-878

DarkIsDude force-pushed the bugfix/CLDSRV-878/use-sur-date branch from beee7a8 to d9a5aed Compare March 27, 2026 14:10

DarkIsDude requested review from a team, delthas and francoisferrand March 27, 2026 14:47

DarkIsDude marked this pull request as ready for review March 27, 2026 14:47

francoisferrand reviewed Mar 27, 2026

View reviewed changes

lib/routes/veeam/get.js Show resolved Hide resolved

francoisferrand reviewed Mar 27, 2026

View reviewed changes

tests/unit/api/apiUtils/objectLockHelpers.js Show resolved Hide resolved

DarkIsDude requested a review from francoisferrand March 30, 2026 13:27

DarkIsDude force-pushed the bugfix/CLDSRV-878/use-sur-date branch from bed3bca to 4a70cf7 Compare March 31, 2026 13:30

DarkIsDude added 2 commits March 31, 2026 15:32

♻️ extract fetchCapacityMetrics helper to veeam utils

225cf9c

Move the shared UtilizationService call pattern (bucket key derivation, 404 warn/fallback, error logging) into a single fetchCapacityMetrics helper in utils.js, used by GET, HEAD, and LIST routes. Issue: CLDSRV-878

✅ add unit tests for fetchCapacityMetrics helper

b779f1f

Issue: CLDSRV-878

francoisferrand reviewed Mar 31, 2026

View reviewed changes

lib/routes/veeam/head.js Outdated Show resolved Hide resolved

lib/routes/veeam/list.js Outdated Show resolved Hide resolved

lib/routes/veeam/list.js Outdated Show resolved Hide resolved

lib/routes/veeam/get.js Outdated Show resolved Hide resolved

DarkIsDude requested a review from francoisferrand March 31, 2026 17:42

♻️ factorise logic in get and head lib/route/veeam

63a33eb

Issue: CLDSRV-878

DarkIsDude force-pushed the bugfix/CLDSRV-878/use-sur-date branch from 48e4d3e to 63a33eb Compare April 1, 2026 09:52

♻️ migrate to async/await style

19a1ee8

Issue: CLDSRV-878

francoisferrand reviewed Apr 1, 2026

View reviewed changes

✅ migrate tests to async/await

9524559

Issue: CLDSRV-878

DarkIsDude force-pushed the bugfix/CLDSRV-878/use-sur-date branch from 24af2da to 9524559 Compare April 1, 2026 11:47

♻️ prefer promisify over manual new Promise

6f9012b

Issue: CLDSRV-878

DarkIsDude force-pushed the bugfix/CLDSRV-878/use-sur-date branch from fb85c7f to 7258a79 Compare April 1, 2026 14:32

♻️ remove useless argument to follow caller

78fd519

Issue: CLDSRV-878

DarkIsDude force-pushed the bugfix/CLDSRV-878/use-sur-date branch from 7258a79 to 78fd519 Compare April 1, 2026 14:49

DarkIsDude requested a review from francoisferrand April 1, 2026 15:15

delthas approved these changes Apr 1, 2026

View reviewed changes

	return new Promise((resolve, reject) => {
	return await utils.promisify(() => { ... })();

Conversation

DarkIsDude commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull request template

Description

Uh oh!

bert-e commented Mar 27, 2026

Hello darkisdude,

Uh oh!

bert-e commented Mar 27, 2026

Incorrect fix version

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Mar 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bert-e commented Apr 1, 2026

Request integration branches

Uh oh!

francoisferrand Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

DarkIsDude Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

francoisferrand Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

DarkIsDude Apr 1, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

DarkIsDude commented Mar 27, 2026 •

edited

Loading

codecov bot commented Mar 27, 2026 •

edited

Loading