Skip to content

CLDSRV-901: Add functional test coverage for ListObjectsV2 optional object attributes #6174

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 5 commits into from
May 22, 2026
Merged

CLDSRV-901: Add functional test coverage for ListObjectsV2 optional object attributes #6174

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1ed1972
CLDSRV-901: Bump CI vault to 7.89.0
dvasilas May 19, 2026
5a3985d
CLDSRV-901: Prevent duplicate optional-attributes elements
dvasilas May 19, 2026
9dfed18
CLDSRV-901: Do not include optional-attributes in delete markers
dvasilas May 19, 2026
97d0418
CLDSRV-901: Add functional tests for ListObjectsV2 optional-attribute…
dvasilas May 13, 2026
ea65b8e
CLDSRV-901: Bump version to 9.2.41
dvasilas May 20, 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
4 changes: 3 additions & 1 deletion .github/docker/docker-compose.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ services:
volumes:
- /tmp/artifacts/${JOB_NAME}/sproxyd:/logs
vault:
image: ghcr.io/scality/vault:7.76.0
image: ghcr.io/scality/vault:7.89.0
profiles: ['vault']
user: root
command: sh -c "chmod 400 tests/utils/keyfile && yarn start > /artifacts/vault.log 2> /artifacts/vault-stderr.log"
Expand All @@ -122,6 +122,8 @@ services:
- ENABLE_LOCAL_CACHE=true
- REDIS_HOST=0.0.0.0
- REDIS_PORT=6379
# TODO: drop NODE_AUTH_TOKEN when VAULT-721 is done
Copy link
Copy Markdown
Contributor

@DarkIsDude DarkIsDude May 21, 2026

Choose a reason for hiding this comment

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

We should mention that in the JIRA also to make sure the develop who will resolve the VAULT-721, will remove it from here 🙏. Why waiting to do it ?

Copy link
Copy Markdown
Contributor Author

@dvasilas dvasilas May 21, 2026

Choose a reason for hiding this comment

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

Done: https://scality.atlassian.net/browse/VAULT-721?focusedCommentId=475640

- NODE_AUTH_TOKEN=dummy
depends_on:
- redis
metadata-standalone:
Expand Down
19 changes: 12 additions & 7 deletions lib/api/bucketGet.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -154,7 +154,9 @@ function processVersions(bucketName, listParams, list) {
`<ID>${v.Owner.ID}</ID>`,
`<DisplayName>${v.Owner.DisplayName}</DisplayName>`,
'</Owner>',
...processOptionalAttributes(v, listParams.optionalAttributes),
...(v.IsDeleteMarker
? []
: processOptionalAttributes(v, listParams.optionalAttributes)),
`<StorageClass>${v.StorageClass}</StorageClass>`,
v.IsDeleteMarker ? '</DeleteMarker>' : '</Version>'
);
Expand Down Expand Up @@ -332,16 +334,19 @@ function bucketGet(authInfo, request, log, callback) {
const bucketName = request.bucketName;
const v2 = params['list-type'];

const optionalAttributes =
const optionalAttributes = new Set(
request.headers['x-amz-optional-object-attributes']
?.split(',')
.map(attr => attr.trim())
.map(attr => attr !== 'RestoreStatus' ? attr.toLowerCase() : attr)
?? [];
if (optionalAttributes.some(attr => !attr.startsWith('x-amz-meta-') && attr != 'RestoreStatus')) {
return callback(
errorInstances.InvalidArgument.customizeDescription('Invalid attribute name specified')
);
?? []
);
for (const attr of optionalAttributes) {
Copy link
Copy Markdown
Contributor

@DarkIsDude DarkIsDude May 21, 2026

Choose a reason for hiding this comment

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

Are we sure this match the AWS behaviour ? Of course no duplicate blocks, but should we reject the request ?

Copy link
Copy Markdown
Contributor Author

@dvasilas dvasilas May 21, 2026

Choose a reason for hiding this comment

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

Checked with multiple RestoreStatus. AWS does not reject the request

aws s3api list-objects-v2 --bucket  cldsrv-probe-optattr-manual --optional-object-attributes RestoreStatus RestoreStatus --no-cli-pager
{
    "Contents": [
        {
            "Key": "obj1",
            "LastModified": "2026-05-21T10:15:25+00:00",
            "ETag": "\"74e11c9bddafedf3ed7cd8bea153e7c8\"",
            "ChecksumAlgorithm": [
                "CRC64NVME"
            ],
            "ChecksumType": "FULL_OBJECT",
            "Size": 19,
            "StorageClass": "STANDARD"
        }
    ],
    "RequestCharged": null,
    "Prefix": ""
}

Side note: We have a divergence with AWS. AWS does not include RestoreStatus in the response for STANDARD objects (but I didn't want to change this in this PR because it will break existing behavior in Artesca).

if (!attr.startsWith('x-amz-meta-') && attr !== 'RestoreStatus') {
return callback(
errorInstances.InvalidArgument.customizeDescription('Invalid attribute name specified')
);
}
}

if (v2 !== undefined && Number.parseInt(v2, 10) !== 2) {
Expand Down
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@zenko/cloudserver",
"version": "9.2.40",
"version": "9.2.41",
"description": "Zenko CloudServer, an open-source Node.js implementation of a server handling the Amazon S3 protocol",
"main": "index.js",
"engines": {
Expand Down
Loading
Loading