Skip to content

✨(CLDSRV-813) update CloudServer XML ListObjectsV2 to support optional attributes #6043

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

Open
DarkIsDude wants to merge 6 commits into
base: feature/CLDSRV-812/list-objects-v2-optional-permissions
Choose a base branch
from
+198 −47
Open

✨(CLDSRV-813) update CloudServer XML ListObjectsV2 to support optional attributes #6043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5cbb868
✨ update the XML response to support Optional attributes
DarkIsDude Jan 8, 2026
c39ff31
✅ add new tests for the new response logic
DarkIsDude Jan 8, 2026
29fee54
⬆️ bump arsenal
DarkIsDude Jan 8, 2026
4531299
✨ update Veeam route to pass the new params
DarkIsDude Jan 8, 2026
1a213d7
✨ update metadataSearch to pass the right arguments
DarkIsDude Jan 9, 2026
a8eba02
✅ ignore restoreStatus in tests
DarkIsDude Jan 9, 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
38 changes: 38 additions & 0 deletions lib/api/bucketGet.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ function processVersions(bucketName, listParams, list) {
const objectKey = escapeXmlFn(item.key);
const isLatest = lastKey !== objectKey;
lastKey = objectKey;

xml.push(
v.IsDeleteMarker ? '<DeleteMarker>' : '<Version>',
`<Key>${objectKey}</Key>`,
Expand All @@ -153,14 +154,17 @@ function processVersions(bucketName, listParams, list) {
`<ID>${v.Owner.ID}</ID>`,
`<DisplayName>${v.Owner.DisplayName}</DisplayName>`,
'</Owner>',
...processOptionalAttributes(v, listParams.optionalAttributes),
`<StorageClass>${v.StorageClass}</StorageClass>`,
v.IsDeleteMarker ? '</DeleteMarker>' : '</Version>'
);
});

list.CommonPrefixes.forEach(item => {
const val = escapeXmlFn(item);
xml.push(`<CommonPrefixes><Prefix>${val}</Prefix></CommonPrefixes>`);
});

xml.push('</ListVersionsResult>');
return xml.join('');
}
Expand Down Expand Up @@ -224,6 +228,7 @@ function processMasterVersions(bucketName, listParams, list) {
if (v.isDeleteMarker) {
return null;
}

const objectKey = escapeXmlFn(item.key);
xml.push(
'<Contents>',
Expand All @@ -232,6 +237,7 @@ function processMasterVersions(bucketName, listParams, list) {
`<ETag>&quot;${v.ETag}&quot;</ETag>`,
`<Size>${v.Size}</Size>`
);

if (!listParams.v2 || listParams.fetchOwner) {
xml.push(
'<Owner>',
Expand All @@ -240,6 +246,9 @@ function processMasterVersions(bucketName, listParams, list) {
'</Owner>'
);
}

xml.push(...processOptionalAttributes(v, listParams.optionalAttributes));

return xml.push(
`<StorageClass>${v.StorageClass}</StorageClass>`,
'</Contents>'
Expand All @@ -253,20 +262,48 @@ function processMasterVersions(bucketName, listParams, list) {
return xml.join('');
}

function processOptionalAttributes(item, optionalAttributes) {
const xml = [];

for (const key of Object.keys(item)) {
if (key.startsWith('x-amz-meta-')) {
if (optionalAttributes.includes('x-amz-meta-*') || optionalAttributes.includes(key)) {
xml.push(`<${key}>${item[key]}</${key}>`);
}
}
}
Comment on lines +268 to +274
Copy link
Contributor

@francoisferrand francoisferrand Jan 11, 2026

Choose a reason for hiding this comment

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

it would be more efficient to go the other way round:

  • e.g. go through every optionalAttributes (which are more likely not as numerous as the fields in our metadata objects)
  • in particular, this also ensure we keep the "usual" case (e.g. without optionalAttributes) optimized.
  • it avoids the repeated lookups in optionalAttributes
for(const attr  in optionalAttributes) {
   switch (attr) {
   case 'RestoreStatus':
     [...]
	 break;

   case 'x-amz-meta-*':
     for ((key, value) in item.getUserMetadata()) {
        xml.push(`<${key}>${item[key]}</${key}>`);
     }
     break;

   default:
     const v = item[attr];
     if (v !== undefined) {
         xml.push(....)
     }
     break;
   }

The x-amz-meta-* case is a bit more concerning, since

  • it needs to go through every field to get the every user metadata (but there is already the ObjectMD.getUserMetadata() function to do this)
  • there could be an issue if both x-amz-meta-* and specific fields are specified ; but this should be rejected when validating the request?

This approach is also more future-proof, as it will allow to ad more fields by simply adding them to the optionalAttributes list during validation (i.e. less coupling, only need to update one place if we add more fields later)


if (optionalAttributes.includes('RestoreStatus')) {
xml.push('<RestoreStatus>');
xml.push(`<IsRestoreInProgress>${!!item.restoreStatus?.inProgress}</IsRestoreInProgress>`);

if (item.restoreStatus?.expiryDate) {
xml.push(`<RestoreExpiryDate>${item.restoreStatus?.expiryDate}</RestoreExpiryDate>`);
}

xml.push('</RestoreStatus>');
}

return xml;
}

function handleResult(listParams, requestMaxKeys, encoding, authInfo,
bucketName, list, corsHeaders, log, callback) {
// eslint-disable-next-line no-param-reassign
listParams.maxKeys = requestMaxKeys;
// eslint-disable-next-line no-param-reassign
listParams.encoding = encoding;

let res;
if (listParams.listingType === 'DelimiterVersions') {
res = processVersions(bucketName, listParams, list);
} else {
res = processMasterVersions(bucketName, listParams, list);
}

pushMetric('listBucket', log, { authInfo, bucket: bucketName });
monitoring.promMetrics('GET', bucketName, '200', 'listBucket');

return callback(null, res, corsHeaders);
}

Expand Down Expand Up @@ -344,6 +381,7 @@ function bucketGet(authInfo, request, log, callback) {
listingType: 'DelimiterMaster',
maxKeys: actualMaxKeys,
prefix: params.prefix,
optionalAttributes,
};

if (params.delimiter) {
Expand Down
2 changes: 2 additions & 0 deletions lib/api/metadataSearch.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ function handleResult(listParams, requestMaxKeys, encoding, authInfo,
listParams.maxKeys = requestMaxKeys;
// eslint-disable-next-line no-param-reassign
listParams.encoding = encoding;
// eslint-disable-next-line no-param-reassign
listParams.optionalAttributes = [];
let res;
if (listParams.listingType === 'DelimiterVersions') {
res = processVersions(bucketName, listParams, list);
Expand Down
1 change: 1 addition & 0 deletions lib/routes/veeam/list.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ function buildXMLResponse(request, arrayOfFiles, versioned = false) {
prefix: validPath,
maxKeys: parsedQs['max-keys'] || 1000,
delimiter: '/',
optionalAttributes: [],
};
const list = {
IsTruncated: false,
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
Expand Up @@ -21,7 +21,7 @@
"dependencies": {
"@azure/storage-blob": "^12.28.0",
"@hapi/joi": "^17.1.1",
"arsenal": "git+https://github.com/scality/Arsenal#8.2.43",
"arsenal": "git+https://github.com/scality/arsenal#feature/ARSN-544/list-objects-v2-optional-attributes",
"async": "2.6.4",
"aws-sdk": "^2.1692.0",
"bucketclient": "scality/bucketclient#8.2.7",
Expand Down
47 changes: 24 additions & 23 deletions tests/functional/aws-node-sdk/test/object/mpuVersion.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,15 @@ function checkObjMdAndUpdate(objMDBefore, objMDAfter, props) {
});
}

function clearUploadIdFromVersions(versions) {
function clearUploadIdFromVersionsAndRestoreStatus(versions) {
Copy link
Contributor

@francoisferrand francoisferrand Jan 11, 2026

Choose a reason for hiding this comment

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

Suggested change
function clearUploadIdFromVersionsAndRestoreStatus(versions) {
function clearUploadIdAndRestoreStatusFromVersions(versions) {

if (!versions || versions.length === 0) {
return versions;
}

for (const version of versions) {
if (version.value) {
version.value.uploadId = undefined;
version.value.restoreStatus = undefined;
Copy link
Contributor

@francoisferrand francoisferrand Jan 11, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

why do we need to clear RestoreStatus from all of these tests?

RestoreStatus is only set by cold storage, so will not be set in most case : so seems best to keep this "as is" and handle the cold-storage tests

}
}

Expand Down Expand Up @@ -263,7 +264,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
return next(err);
}),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsBefore = clearUploadIdFromVersions(res.Versions);
versionsBefore = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
next => putMPUVersion(s3, bucketName, objectName, '', next),
Expand All @@ -272,7 +273,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
return next(err);
}),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsAfter = clearUploadIdFromVersions(res.Versions);
versionsAfter = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
], err => {
Expand Down Expand Up @@ -304,7 +305,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
return next(err);
}),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsBefore = clearUploadIdFromVersions(res.Versions);
versionsBefore = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
next => putMPUVersion(s3, bucketName, objectName, '', next),
Expand All @@ -313,7 +314,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
return next(err);
}),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsAfter = clearUploadIdFromVersions(res.Versions);
versionsAfter = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
], err => {
Expand Down Expand Up @@ -353,7 +354,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
}),
next => fakeMetadataArchive(bucketName, objectName, vId, archive, next),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsBefore = clearUploadIdFromVersions(res.Versions);
versionsBefore = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
next => getMetadata(bucketName, objectName, vId, (err, objMD) => {
Expand All @@ -366,7 +367,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
return next(err);
}),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsAfter = clearUploadIdFromVersions(res.Versions);
versionsAfter = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
], err => {
Expand Down Expand Up @@ -405,7 +406,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
}),
next => fakeMetadataArchive(bucketName, objectName, vId, archive, next),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsBefore = clearUploadIdFromVersions(res.Versions);
versionsBefore = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
next => getMetadata(bucketName, objectName, vId, (err, objMD) => {
Expand All @@ -418,7 +419,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
return next(err);
}),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsAfter = clearUploadIdFromVersions(res.Versions);
versionsAfter = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
], err => {
Expand Down Expand Up @@ -458,7 +459,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
return next(err);
}),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsBefore = clearUploadIdFromVersions(res.Versions);
versionsBefore = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
next => putMPUVersion(s3, bucketName, objectName, 'null', next),
Expand All @@ -467,7 +468,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
return next(err);
}),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsAfter = clearUploadIdFromVersions(res.Versions);
versionsAfter = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
], err => {
Expand Down Expand Up @@ -511,7 +512,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
return next(err);
}),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsBefore = clearUploadIdFromVersions(res.Versions);
versionsBefore = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
next(err);
}),
next => putMPUVersion(s3, bucketName, objectName, vId, next),
Expand All @@ -520,7 +521,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
return next(err);
}),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsAfter = clearUploadIdFromVersions(res.Versions);
versionsAfter = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
], err => {
Expand Down Expand Up @@ -567,7 +568,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
return next(err);
}),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsBefore = clearUploadIdFromVersions(res.Versions);
versionsBefore = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
next(err);
}),
next => putMPUVersion(s3, bucketName, objectName, '', next),
Expand All @@ -576,7 +577,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
return next(err);
}),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsAfter = clearUploadIdFromVersions(res.Versions);
versionsAfter = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
], err => {
Expand Down Expand Up @@ -621,7 +622,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
return next(err);
}),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsBefore = clearUploadIdFromVersions(res.Versions);
versionsBefore = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
next => putMPUVersion(s3, bucketName, objectName, vId, next),
Expand All @@ -630,7 +631,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
return next(err);
}),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsAfter = clearUploadIdFromVersions(res.Versions);
versionsAfter = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
], err => {
Expand Down Expand Up @@ -670,7 +671,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
}),
next => fakeMetadataArchive(bucketName, objectName, vId, archive, next),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsBefore = clearUploadIdFromVersions(res.Versions);
versionsBefore = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
next => getMetadata(bucketName, objectName, vId, (err, objMD) => {
Expand All @@ -683,7 +684,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
return next(err);
}),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsAfter = clearUploadIdFromVersions(res.Versions);
versionsAfter = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
], err => {
Expand Down Expand Up @@ -729,7 +730,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
}),
next => fakeMetadataArchive(bucketName, objectName, vId, archive, next),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsBefore = clearUploadIdFromVersions(res.Versions);
versionsBefore = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
next => getMetadata(bucketName, objectName, vId, (err, objMD) => {
Expand All @@ -743,7 +744,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
return next(err);
}),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsAfter = clearUploadIdFromVersions(res.Versions);
versionsAfter = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
], err => {
Expand Down Expand Up @@ -777,7 +778,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
next => s3.putObject(params, next),
next => fakeMetadataArchive(bucketName, objectName, undefined, archive, next),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsBefore = clearUploadIdFromVersions(res.Versions);
versionsBefore = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
next => getMetadata(bucketName, objectName, undefined, (err, objMD) => {
Expand All @@ -791,7 +792,7 @@ describe('MPU with x-scal-s3-version-id header', () => {
return next(err);
}),
next => metadata.listObject(bucketName, mdListingParams, log, (err, res) => {
versionsAfter = clearUploadIdFromVersions(res.Versions);
versionsAfter = clearUploadIdFromVersionsAndRestoreStatus(res.Versions);
return next(err);
}),
], err => {
Expand Down
Loading
Loading