From f8a54b12dc1a5077af8f33853da1e6588b400d24 Mon Sep 17 00:00:00 2001 From: KillianG Date: Thu, 22 Jun 2023 12:34:16 +0000 Subject: [PATCH 1/7] Fix user MD and tags getting deleted when restoring ISSUE: CLDSRV-408 --- lib/api/apiUtils/object/versioning.js | 7 ++ tests/unit/api/apiUtils/versioning.js | 125 +++++++++++++++++++++++++- 2 files changed, 131 insertions(+), 1 deletion(-) diff --git a/lib/api/apiUtils/object/versioning.js b/lib/api/apiUtils/object/versioning.js index 6dfddc9f82..b183c655d4 100644 --- a/lib/api/apiUtils/object/versioning.js +++ b/lib/api/apiUtils/object/versioning.js @@ -453,6 +453,13 @@ function overwritingVersioning(objMD, metadataStoreParams) { // set correct originOp metadataStoreParams.originOp = 's3:ObjectRestore:Completed'; + // We need to keep user metadata and tags + for (const key of Object.keys(objMD)) { + if (key.startsWith('x-amz-meta-')) { + metadataStoreParams.metaHeaders[key] = objMD[key]; + } + } + metadataStoreParams.taggingCopy = objMD.tags; // update restore const days = objMD.archive?.restoreRequestedDays; const now = Date.now(); diff --git a/tests/unit/api/apiUtils/versioning.js b/tests/unit/api/apiUtils/versioning.js index fca9e06c72..3ff5d9b17a 100644 --- a/tests/unit/api/apiUtils/versioning.js +++ b/tests/unit/api/apiUtils/versioning.js @@ -3,9 +3,12 @@ const assert = require('assert'); const { versioning } = require('arsenal'); const { config } = require('../../../../lib/Config'); const INF_VID = versioning.VersionID.getInfVid(config.replicationGroupId); +const { scaledMsPerDay } = config.getTimeOptions(); +const sinon = require('sinon'); const { processVersioningState, getMasterState, - preprocessingVersioningDelete } = + preprocessingVersioningDelete, + overwritingVersioning } = require('../../../../lib/api/apiUtils/object/versioning'); describe('versioning helpers', () => { @@ -616,4 +619,124 @@ describe('versioning helpers', () => { assert.deepStrictEqual(options, testCase[expectedResAttr]); }))); }); + + describe('overwritingVersioning', () => { + const now = Date.now(); + sinon.useFakeTimers(now); + const days = 3; + const archiveInfo = { + 'archiveID': '126783123678', + }; + + it('Should update archive with restore infos', () => { + const metadataStoreParams = {}; + const objMD = { + 'versionId': '2345678', + 'creation-time': now, + 'last-modified': now, + 'originOp': 's3:PutObject', + archive: { + 'restoreRequestedDays': days, + 'restoreRequestedAt': now, + archiveInfo + } + }; + const options = overwritingVersioning(objMD, metadataStoreParams); + + const expectedRes = { + 'creationTime': now, + 'lastModifiedDate': now, + 'updateMicroVersionId': true, + 'originOp': 's3:ObjectRestore:Completed', + taggingCopy: undefined, + archive: { + archiveInfo, + 'restoreRequestedDays': 3, + 'restoreRequestedAt': now, + 'restoreCompletedAt': new Date(now), + 'restoreWillExpireAt': new Date(now + (days * scaledMsPerDay)), + } + }; + assert.deepStrictEqual(options.versionId, '2345678'); + assert.deepStrictEqual(metadataStoreParams, expectedRes); + }); + + it('Should keep user mds and tags', () => { + const metadataStoreParams = { + 'metaHeaders': {}, + }; + const objMD = { + 'versionId': '2345678', + 'creation-time': now, + 'last-modified': now, + 'originOp': 's3:PutObject', + 'x-amz-meta-test': 'test', + 'x-amz-meta-test2': 'test2', + 'tags': { 'testtag': 'testtag', 'testtag2': 'testtag2' }, + archive: { + 'restoreRequestedDays': days, + 'restoreRequestedAt': now, + archiveInfo + } + }; + const options = overwritingVersioning(objMD, metadataStoreParams); + + const expectedRes = { + 'creationTime': now, + 'lastModifiedDate': now, + 'updateMicroVersionId': true, + 'originOp': 's3:ObjectRestore:Completed', + 'metaHeaders': { + 'x-amz-meta-test': 'test', + 'x-amz-meta-test2': 'test2', + }, + taggingCopy: { 'testtag': 'testtag', 'testtag2': 'testtag2' }, + archive: { + archiveInfo, + 'restoreRequestedDays': 3, + 'restoreRequestedAt': now, + 'restoreCompletedAt': new Date(now), + 'restoreWillExpireAt': new Date(now + (days * scaledMsPerDay)), + } + }; + assert.deepStrictEqual(options.versionId, '2345678'); + assert.deepStrictEqual(metadataStoreParams, expectedRes); + }); + + it('Should return nullVersionId', () => { + const metadataStoreParams = {}; + const objMD = { + 'creation-time': now, + 'last-modified': now, + 'originOp': 's3:PutObject', + 'nullVersionId': 'vnull', + 'isNull': true, + archive: { + 'restoreRequestedDays': days, + 'restoreRequestedAt': now, + archiveInfo + } + }; + const options = overwritingVersioning(objMD, metadataStoreParams); + + const expectedRes = { + 'creationTime': now, + 'lastModifiedDate': now, + 'updateMicroVersionId': true, + 'originOp': 's3:ObjectRestore:Completed', + taggingCopy: undefined, + archive: { + archiveInfo, + 'restoreRequestedDays': 3, + 'restoreRequestedAt': now, + 'restoreCompletedAt': new Date(now), + 'restoreWillExpireAt': new Date(now + (days * scaledMsPerDay)), + } + }; + assert.deepStrictEqual(options.versionId, undefined); + assert.deepStrictEqual(options.extraMD.nullVersionId, 'vnull'); + assert.deepStrictEqual(options.isNull, true); + assert.deepStrictEqual(metadataStoreParams, expectedRes); + }); + }); }); From ac620b7e8d5b4dbaa8cf434b7705013327bc566f Mon Sep 17 00:00:00 2001 From: KillianG Date: Wed, 28 Jun 2023 09:20:56 +0000 Subject: [PATCH 2/7] Adding replicationinfo to overwriteVersionning to avoid loosing replication info in case of a restore Issue: CLDSRV-408 --- lib/api/apiUtils/object/versioning.js | 6 +- tests/unit/api/apiUtils/versioning.js | 277 +++++++++++++++++--------- 2 files changed, 193 insertions(+), 90 deletions(-) diff --git a/lib/api/apiUtils/object/versioning.js b/lib/api/apiUtils/object/versioning.js index b183c655d4..d386ecbe6c 100644 --- a/lib/api/apiUtils/object/versioning.js +++ b/lib/api/apiUtils/object/versioning.js @@ -453,12 +453,16 @@ function overwritingVersioning(objMD, metadataStoreParams) { // set correct originOp metadataStoreParams.originOp = 's3:ObjectRestore:Completed'; + const userMDNotToKeep = ['x-amz-meta-scal-s3-restore-attempt']; // We need to keep user metadata and tags for (const key of Object.keys(objMD)) { - if (key.startsWith('x-amz-meta-')) { + if (key.startsWith('x-amz-meta-') && !userMDNotToKeep.includes(key)) { metadataStoreParams.metaHeaders[key] = objMD[key]; } } + if (objMD.replicationInfo?.status === 'COMPLETED') { + metadataStoreParams.replicationInfo = objMD.replicationInfo; + } metadataStoreParams.taggingCopy = objMD.tags; // update restore const days = objMD.archive?.restoreRequestedDays; diff --git a/tests/unit/api/apiUtils/versioning.js b/tests/unit/api/apiUtils/versioning.js index 3ff5d9b17a..d3c5d06aaa 100644 --- a/tests/unit/api/apiUtils/versioning.js +++ b/tests/unit/api/apiUtils/versioning.js @@ -620,123 +620,222 @@ describe('versioning helpers', () => { }))); }); - describe('overwritingVersioning', () => { - const now = Date.now(); - sinon.useFakeTimers(now); + describe.only('overwritingVersioning', () => { + sinon.useFakeTimers(); const days = 3; + const now = Date.now(); const archiveInfo = { 'archiveID': '126783123678', }; - it('Should update archive with restore infos', () => { - const metadataStoreParams = {}; - const objMD = { - 'versionId': '2345678', - 'creation-time': now, - 'last-modified': now, - 'originOp': 's3:PutObject', - archive: { - 'restoreRequestedDays': days, - 'restoreRequestedAt': now, - archiveInfo - } - }; - const options = overwritingVersioning(objMD, metadataStoreParams); - - const expectedRes = { - 'creationTime': now, - 'lastModifiedDate': now, - 'updateMicroVersionId': true, - 'originOp': 's3:ObjectRestore:Completed', - taggingCopy: undefined, - archive: { - archiveInfo, - 'restoreRequestedDays': 3, - 'restoreRequestedAt': now, - 'restoreCompletedAt': new Date(now), - 'restoreWillExpireAt': new Date(now + (days * scaledMsPerDay)), - } - }; - assert.deepStrictEqual(options.versionId, '2345678'); - assert.deepStrictEqual(metadataStoreParams, expectedRes); - }); - - it('Should keep user mds and tags', () => { - const metadataStoreParams = { - 'metaHeaders': {}, - }; - const objMD = { + const array = [ + { + description: 'Should update archive with restore infos', + objMD: { 'versionId': '2345678', 'creation-time': now, 'last-modified': now, 'originOp': 's3:PutObject', - 'x-amz-meta-test': 'test', - 'x-amz-meta-test2': 'test2', - 'tags': { 'testtag': 'testtag', 'testtag2': 'testtag2' }, archive: { 'restoreRequestedDays': days, 'restoreRequestedAt': now, archiveInfo + }}, + expectedRes : { + 'creationTime': now, + 'lastModifiedDate': now, + 'updateMicroVersionId': true, + 'originOp': 's3:ObjectRestore:Completed', + taggingCopy: undefined, + archive: { + archiveInfo, + 'restoreRequestedDays': 3, + 'restoreRequestedAt': now, + 'restoreCompletedAt': new Date(now), + 'restoreWillExpireAt': new Date(now + (days * scaledMsPerDay)), + } } - }; - const options = overwritingVersioning(objMD, metadataStoreParams); - - const expectedRes = { - 'creationTime': now, - 'lastModifiedDate': now, - 'updateMicroVersionId': true, - 'originOp': 's3:ObjectRestore:Completed', - 'metaHeaders': { + }, + { + description: 'Should keep user mds and tags', + hasUserMD: true, + objMD: { + 'versionId': '2345678', + 'creation-time': now, + 'last-modified': now, + 'originOp': 's3:PutObject', 'x-amz-meta-test': 'test', 'x-amz-meta-test2': 'test2', + 'tags': { 'testtag': 'testtag', 'testtag2': 'testtag2' }, + archive: { + 'restoreRequestedDays': days, + 'restoreRequestedAt': now, + archiveInfo + } }, - taggingCopy: { 'testtag': 'testtag', 'testtag2': 'testtag2' }, - archive: { - archiveInfo, - 'restoreRequestedDays': 3, - 'restoreRequestedAt': now, - 'restoreCompletedAt': new Date(now), - 'restoreWillExpireAt': new Date(now + (days * scaledMsPerDay)), + expectedRes: { + 'creationTime': now, + 'lastModifiedDate': now, + 'updateMicroVersionId': true, + 'originOp': 's3:ObjectRestore:Completed', + 'metaHeaders': { + 'x-amz-meta-test': 'test', + 'x-amz-meta-test2': 'test2', + }, + taggingCopy: { 'testtag': 'testtag', 'testtag2': 'testtag2' }, + archive: { + archiveInfo, + 'restoreRequestedDays': 3, + 'restoreRequestedAt': now, + 'restoreCompletedAt': new Date(now), + 'restoreWillExpireAt': new Date(now + (days * scaledMsPerDay)), + } + }, + }, + { + description: 'Should return nullVersionId', + objMD: { + 'creation-time': now, + 'last-modified': now, + 'originOp': 's3:PutObject', + 'nullVersionId': 'vnull', + 'isNull': true, + archive: { + 'restoreRequestedDays': days, + 'restoreRequestedAt': now, + archiveInfo + } + }, + expectedRes: { + 'creationTime': now, + 'lastModifiedDate': now, + 'updateMicroVersionId': true, + 'originOp': 's3:ObjectRestore:Completed', + taggingCopy: undefined, + archive: { + archiveInfo, + 'restoreRequestedDays': 3, + 'restoreRequestedAt': now, + 'restoreCompletedAt': new Date(now), + 'restoreWillExpireAt': new Date(now + (days * scaledMsPerDay)), + } } - }; - assert.deepStrictEqual(options.versionId, '2345678'); - assert.deepStrictEqual(metadataStoreParams, expectedRes); - }); - - it('Should return nullVersionId', () => { - const metadataStoreParams = {}; - const objMD = { + }, + { + description: 'Should not keep x-amz-meta-scal-s3-restore-attempt user MD', + hasUserMD: true, + objMD: { + 'versionId': '2345678', + 'creation-time': now, + 'last-modified': now, + 'originOp': 's3:PutObject', + 'x-amz-meta-test': 'test', + 'x-amz-meta-scal-s3-restore-attempt': 14, + archive: { + 'restoreRequestedDays': days, + 'restoreRequestedAt': now, + archiveInfo + } + }, + expectedRes: { + 'creationTime': now, + 'lastModifiedDate': now, + 'updateMicroVersionId': true, + 'originOp': 's3:ObjectRestore:Completed', + 'metaHeaders': { + 'x-amz-meta-test': 'test', + }, + taggingCopy: undefined, + archive: { + archiveInfo, + 'restoreRequestedDays': 3, + 'restoreRequestedAt': now, + 'restoreCompletedAt': new Date(now), + 'restoreWillExpireAt': new Date(now + (days * scaledMsPerDay)), + } + } + }, + { + description: 'Should not update replication infos if COMPLETED', + objMD: { + 'versionId': '2345678', 'creation-time': now, 'last-modified': now, 'originOp': 's3:PutObject', - 'nullVersionId': 'vnull', - 'isNull': true, + 'replicationInfo': { + 'status' : 'COMPLETED', + 'backends' : [ + { + 'site' : 'azure-blob', + 'status' : 'COMPLETED', + 'dataStoreVersionId' : '' + } + ], + 'content' : [ + 'DATA', + 'METADATA' + ], + 'destination' : 'arn:aws:s3:::replicate-cold', + 'storageClass' : 'azure-blob', + 'role' : 'arn:aws:iam::root:role/s3-replication-role', + 'storageType' : 'azure', + 'dataStoreVersionId' : '', + }, archive: { 'restoreRequestedDays': days, 'restoreRequestedAt': now, archiveInfo + } + }, + expectedRes : { + 'creationTime': now, + 'lastModifiedDate': now, + 'updateMicroVersionId': true, + 'originOp': 's3:ObjectRestore:Completed', + 'replicationInfo': { + 'status' : 'COMPLETED', + 'backends' : [ + { + 'site' : 'azure-blob', + 'status' : 'COMPLETED', + 'dataStoreVersionId' : '' + } + ], + 'content' : [ + 'DATA', + 'METADATA' + ], + 'destination' : 'arn:aws:s3:::replicate-cold', + 'storageClass' : 'azure-blob', + 'role' : 'arn:aws:iam::root:role/s3-replication-role', + 'storageType' : 'azure', + 'dataStoreVersionId' : '', + }, + taggingCopy: undefined, + archive: { + archiveInfo, + 'restoreRequestedDays': 3, + 'restoreRequestedAt': now, + 'restoreCompletedAt': new Date(now), + 'restoreWillExpireAt': new Date(now + (days * scaledMsPerDay)), + } + } + }, + ].forEach(testCase => { + it(testCase.description, () => { + const metadataStoreParams = {}; + if (testCase.hasUserMD) { + metadataStoreParams.metaHeaders = {}; } - }; - const options = overwritingVersioning(objMD, metadataStoreParams); + const options = overwritingVersioning(testCase.objMD, metadataStoreParams); + assert.deepStrictEqual(options.versionId, testCase.objMD.versionId); + assert.deepStrictEqual(metadataStoreParams, testCase.expectedRes); - const expectedRes = { - 'creationTime': now, - 'lastModifiedDate': now, - 'updateMicroVersionId': true, - 'originOp': 's3:ObjectRestore:Completed', - taggingCopy: undefined, - archive: { - archiveInfo, - 'restoreRequestedDays': 3, - 'restoreRequestedAt': now, - 'restoreCompletedAt': new Date(now), - 'restoreWillExpireAt': new Date(now + (days * scaledMsPerDay)), + if (testCase.objMD.isNull) { + assert.deepStrictEqual(options.extraMD.nullVersionId, 'vnull'); + assert.deepStrictEqual(options.isNull, true); } - }; - assert.deepStrictEqual(options.versionId, undefined); - assert.deepStrictEqual(options.extraMD.nullVersionId, 'vnull'); - assert.deepStrictEqual(options.isNull, true); - assert.deepStrictEqual(metadataStoreParams, expectedRes); + }); }); }); }); From 2a17f0d4b39121578e9292eac31d45a906cd6237 Mon Sep 17 00:00:00 2001 From: KillianG Date: Wed, 28 Jun 2023 12:40:06 +0000 Subject: [PATCH 3/7] Keep ACL during a restore Issue: CLDSRV-408 --- lib/api/apiUtils/object/versioning.js | 43 ++++++++++---- lib/services.js | 8 ++- tests/unit/api/apiUtils/versioning.js | 82 ++++++++++++++++++++++++++- 3 files changed, 120 insertions(+), 13 deletions(-) diff --git a/lib/api/apiUtils/object/versioning.js b/lib/api/apiUtils/object/versioning.js index d386ecbe6c..bcca55b3d6 100644 --- a/lib/api/apiUtils/object/versioning.js +++ b/lib/api/apiUtils/object/versioning.js @@ -434,6 +434,36 @@ function preprocessingVersioningDelete(bucketName, bucketMD, objectMD, reqVersio return options; } +/** + * Keep metadatas when the object is restored from cold storage + * but remove the specific ones we don't want to keep + * @param {object} objMD - obj metadata + * @param {object} metadataStoreParams - custom built object containing resource details. + */ +function restoreMetadatas(objMD, metadataStoreParams) { + const userMDNotToKeep = ['x-amz-meta-scal-s3-restore-attempt']; + // We need to keep user metadata and tags + for (const key of Object.keys(objMD)) { + if (key.startsWith('x-amz-meta-') && !userMDNotToKeep.includes(key)) { + metadataStoreParams.metaHeaders[key] = objMD[key]; + } + } + + if (objMD.replicationInfo?.status === 'COMPLETED') { + metadataStoreParams.replicationInfo = objMD.replicationInfo; + } + + if (objMD.legalHold) { + metadataStoreParams.legalHold = objMD.legalHold; + } + + if (objMD.acl) { + metadataStoreParams.oldAcl = objMD.acl; + } + + metadataStoreParams.taggingCopy = objMD.tags; +} + /** overwritingVersioning - return versioning information for S3 to handle * storing version metadata with a specific version id. * @param {object} objMD - obj metadata @@ -453,17 +483,6 @@ function overwritingVersioning(objMD, metadataStoreParams) { // set correct originOp metadataStoreParams.originOp = 's3:ObjectRestore:Completed'; - const userMDNotToKeep = ['x-amz-meta-scal-s3-restore-attempt']; - // We need to keep user metadata and tags - for (const key of Object.keys(objMD)) { - if (key.startsWith('x-amz-meta-') && !userMDNotToKeep.includes(key)) { - metadataStoreParams.metaHeaders[key] = objMD[key]; - } - } - if (objMD.replicationInfo?.status === 'COMPLETED') { - metadataStoreParams.replicationInfo = objMD.replicationInfo; - } - metadataStoreParams.taggingCopy = objMD.tags; // update restore const days = objMD.archive?.restoreRequestedDays; const now = Date.now(); @@ -488,6 +507,8 @@ function overwritingVersioning(objMD, metadataStoreParams) { }; } + restoreMetadatas(objMD, metadataStoreParams); + return options; } diff --git a/lib/services.js b/lib/services.js index eda0528cfa..bc88fc142a 100644 --- a/lib/services.js +++ b/lib/services.js @@ -100,7 +100,7 @@ const services = { tagging, taggingCopy, replicationInfo, defaultRetention, dataStoreName, creationTime, retentionMode, retentionDate, legalHold, originOp, updateMicroVersionId, archive, oldReplayId, - deleteNullKey } = params; + deleteNullKey, oldAcl } = params; log.trace('storing object in metadata'); assert.strictEqual(typeof bucketName, 'string'); const md = new ObjectMD(); @@ -254,6 +254,12 @@ const services = { if (multipart || md.getIsDeleteMarker()) { return callback(); } + if (oldAcl) { + // In case of a restore we dont pass ACL in the headers + // but we take them from the old metadata + md.setAcl(oldAcl); + return callback(); + } const parseAclParams = { headers, resourceType: 'object', diff --git a/tests/unit/api/apiUtils/versioning.js b/tests/unit/api/apiUtils/versioning.js index d3c5d06aaa..ec19945a53 100644 --- a/tests/unit/api/apiUtils/versioning.js +++ b/tests/unit/api/apiUtils/versioning.js @@ -756,7 +756,7 @@ describe('versioning helpers', () => { } }, { - description: 'Should not update replication infos if COMPLETED', + description: 'Should keep replication infos', objMD: { 'versionId': '2345678', 'creation-time': now, @@ -821,6 +821,86 @@ describe('versioning helpers', () => { } } }, + { + description: 'Should keep legalHold', + objMD: { + 'versionId': '2345678', + 'creation-time': now, + 'last-modified': now, + 'originOp': 's3:PutObject', + 'legalHold': true, + archive: { + 'restoreRequestedDays': days, + 'restoreRequestedAt': now, + archiveInfo + } + }, + expectedRes : { + 'creationTime': now, + 'lastModifiedDate': now, + 'updateMicroVersionId': true, + 'originOp': 's3:ObjectRestore:Completed', + 'legalHold': true, + taggingCopy: undefined, + archive: { + archiveInfo, + 'restoreRequestedDays': 3, + 'restoreRequestedAt': now, + 'restoreCompletedAt': new Date(now), + 'restoreWillExpireAt': new Date(now + (days * scaledMsPerDay)), + } + } + }, + { + description: 'Should keep ACLs', + objMD: { + 'versionId': '2345678', + 'creation-time': now, + 'last-modified': now, + 'originOp': 's3:PutObject', + 'acl' : { + 'Canned' : '', + 'FULL_CONTROL' : [ + '872c04772893deae2b48365752362cd92672eb80eb3deea50d89e834a10ce185' + ], + 'WRITE_ACP' : [ ], + 'READ' : [ + 'http://acs.amazonaws.com/groups/global/AllUsers' + ], + 'READ_ACP' : [ ] + }, + archive: { + 'restoreRequestedDays': days, + 'restoreRequestedAt': now, + archiveInfo + } + }, + expectedRes : { + 'creationTime': now, + 'lastModifiedDate': now, + 'updateMicroVersionId': true, + 'originOp': 's3:ObjectRestore:Completed', + 'oldAcl' : { + 'Canned' : '', + 'FULL_CONTROL' : [ + '872c04772893deae2b48365752362cd92672eb80eb3deea50d89e834a10ce185' + ], + 'WRITE_ACP' : [ ], + 'READ' : [ + 'http://acs.amazonaws.com/groups/global/AllUsers' + ], + 'READ_ACP' : [ ] + }, + taggingCopy: undefined, + archive: { + archiveInfo, + 'restoreRequestedDays': 3, + 'restoreRequestedAt': now, + 'restoreCompletedAt': new Date(now), + 'restoreWillExpireAt': new Date(now + (days * scaledMsPerDay)), + } + } + }, ].forEach(testCase => { it(testCase.description, () => { const metadataStoreParams = {}; From 33382bf8e935ce08e0bcb6a76dac37eb8554e686 Mon Sep 17 00:00:00 2001 From: KillianG Date: Wed, 28 Jun 2023 14:06:23 +0000 Subject: [PATCH 4/7] Keep content- metadatas Issue: CLDSRV-408 --- lib/api/apiUtils/object/versioning.js | 16 ++-- tests/unit/api/apiUtils/versioning.js | 116 +++++++++++++++++--------- 2 files changed, 85 insertions(+), 47 deletions(-) diff --git a/lib/api/apiUtils/object/versioning.js b/lib/api/apiUtils/object/versioning.js index bcca55b3d6..d6ef368c72 100644 --- a/lib/api/apiUtils/object/versioning.js +++ b/lib/api/apiUtils/object/versioning.js @@ -439,15 +439,20 @@ function preprocessingVersioningDelete(bucketName, bucketMD, objectMD, reqVersio * but remove the specific ones we don't want to keep * @param {object} objMD - obj metadata * @param {object} metadataStoreParams - custom built object containing resource details. + * @return {undefined} */ function restoreMetadatas(objMD, metadataStoreParams) { + /* eslint-disable no-param-reassign */ const userMDNotToKeep = ['x-amz-meta-scal-s3-restore-attempt']; // We need to keep user metadata and tags - for (const key of Object.keys(objMD)) { - if (key.startsWith('x-amz-meta-') && !userMDNotToKeep.includes(key)) { + Object.keys(objMD).forEach(key => { + if (key.startsWith('x-amz-meta-') && !userMDNotToKeep.includes(key)) { metadataStoreParams.metaHeaders[key] = objMD[key]; } - } + if (key.startsWith('content-') && !userMDNotToKeep.includes(key)) { + metadataStoreParams[key] = objMD[key]; + } + }); if (objMD.replicationInfo?.status === 'COMPLETED') { metadataStoreParams.replicationInfo = objMD.replicationInfo; @@ -461,6 +466,8 @@ function restoreMetadatas(objMD, metadataStoreParams) { metadataStoreParams.oldAcl = objMD.acl; } + metadataStoreParams.creationTime = objMD['creation-time']; + metadataStoreParams.lastModifiedDate = objMD['last-modified']; metadataStoreParams.taggingCopy = objMD.tags; } @@ -475,9 +482,6 @@ function restoreMetadatas(objMD, metadataStoreParams) { * version id of the null version */ function overwritingVersioning(objMD, metadataStoreParams) { - /* eslint-disable no-param-reassign */ - metadataStoreParams.creationTime = objMD['creation-time']; - metadataStoreParams.lastModifiedDate = objMD['last-modified']; metadataStoreParams.updateMicroVersionId = true; // set correct originOp diff --git a/tests/unit/api/apiUtils/versioning.js b/tests/unit/api/apiUtils/versioning.js index ec19945a53..04d293ba4f 100644 --- a/tests/unit/api/apiUtils/versioning.js +++ b/tests/unit/api/apiUtils/versioning.js @@ -620,7 +620,7 @@ describe('versioning helpers', () => { }))); }); - describe.only('overwritingVersioning', () => { + describe('overwritingVersioning', () => { sinon.useFakeTimers(); const days = 3; const now = Date.now(); @@ -628,7 +628,7 @@ describe('versioning helpers', () => { 'archiveID': '126783123678', }; - const array = [ + [ { description: 'Should update archive with restore infos', objMD: { @@ -640,8 +640,8 @@ describe('versioning helpers', () => { 'restoreRequestedDays': days, 'restoreRequestedAt': now, archiveInfo - }}, - expectedRes : { + } }, + expectedRes: { 'creationTime': now, 'lastModifiedDate': now, 'updateMicroVersionId': true, @@ -763,23 +763,23 @@ describe('versioning helpers', () => { 'last-modified': now, 'originOp': 's3:PutObject', 'replicationInfo': { - 'status' : 'COMPLETED', - 'backends' : [ + 'status': 'COMPLETED', + 'backends': [ { - 'site' : 'azure-blob', - 'status' : 'COMPLETED', - 'dataStoreVersionId' : '' + 'site': 'azure-blob', + 'status': 'COMPLETED', + 'dataStoreVersionId': '' } ], - 'content' : [ + 'content': [ 'DATA', 'METADATA' ], - 'destination' : 'arn:aws:s3:::replicate-cold', - 'storageClass' : 'azure-blob', - 'role' : 'arn:aws:iam::root:role/s3-replication-role', - 'storageType' : 'azure', - 'dataStoreVersionId' : '', + 'destination': 'arn:aws:s3:::replicate-cold', + 'storageClass': 'azure-blob', + 'role': 'arn:aws:iam::root:role/s3-replication-role', + 'storageType': 'azure', + 'dataStoreVersionId': '', }, archive: { 'restoreRequestedDays': days, @@ -787,29 +787,29 @@ describe('versioning helpers', () => { archiveInfo } }, - expectedRes : { + expectedRes: { 'creationTime': now, 'lastModifiedDate': now, 'updateMicroVersionId': true, 'originOp': 's3:ObjectRestore:Completed', 'replicationInfo': { - 'status' : 'COMPLETED', - 'backends' : [ + 'status': 'COMPLETED', + 'backends': [ { - 'site' : 'azure-blob', - 'status' : 'COMPLETED', - 'dataStoreVersionId' : '' + 'site': 'azure-blob', + 'status': 'COMPLETED', + 'dataStoreVersionId': '' } ], - 'content' : [ + 'content': [ 'DATA', 'METADATA' ], - 'destination' : 'arn:aws:s3:::replicate-cold', - 'storageClass' : 'azure-blob', - 'role' : 'arn:aws:iam::root:role/s3-replication-role', - 'storageType' : 'azure', - 'dataStoreVersionId' : '', + 'destination': 'arn:aws:s3:::replicate-cold', + 'storageClass': 'azure-blob', + 'role': 'arn:aws:iam::root:role/s3-replication-role', + 'storageType': 'azure', + 'dataStoreVersionId': '', }, taggingCopy: undefined, archive: { @@ -835,7 +835,7 @@ describe('versioning helpers', () => { archiveInfo } }, - expectedRes : { + expectedRes: { 'creationTime': now, 'lastModifiedDate': now, 'updateMicroVersionId': true, @@ -858,16 +858,16 @@ describe('versioning helpers', () => { 'creation-time': now, 'last-modified': now, 'originOp': 's3:PutObject', - 'acl' : { - 'Canned' : '', - 'FULL_CONTROL' : [ + 'acl': { + 'Canned': '', + 'FULL_CONTROL': [ '872c04772893deae2b48365752362cd92672eb80eb3deea50d89e834a10ce185' ], - 'WRITE_ACP' : [ ], - 'READ' : [ + 'WRITE_ACP': [], + 'READ': [ 'http://acs.amazonaws.com/groups/global/AllUsers' ], - 'READ_ACP' : [ ] + 'READ_ACP': [] }, archive: { 'restoreRequestedDays': days, @@ -875,21 +875,21 @@ describe('versioning helpers', () => { archiveInfo } }, - expectedRes : { + expectedRes: { 'creationTime': now, 'lastModifiedDate': now, 'updateMicroVersionId': true, 'originOp': 's3:ObjectRestore:Completed', - 'oldAcl' : { - 'Canned' : '', - 'FULL_CONTROL' : [ + 'oldAcl': { + 'Canned': '', + 'FULL_CONTROL': [ '872c04772893deae2b48365752362cd92672eb80eb3deea50d89e834a10ce185' ], - 'WRITE_ACP' : [ ], - 'READ' : [ + 'WRITE_ACP': [], + 'READ': [ 'http://acs.amazonaws.com/groups/global/AllUsers' ], - 'READ_ACP' : [ ] + 'READ_ACP': [] }, taggingCopy: undefined, archive: { @@ -901,6 +901,40 @@ describe('versioning helpers', () => { } } }, + { + description: 'Should keep content-* metadata', + objMD: { + 'versionId': '2345678', + 'creation-time': now, + 'last-modified': now, + 'content-length': 123456, + 'content-md5': 'md5', + 'content-type': 'text/plain', + 'originOp': 's3:PutObject', + archive: { + 'restoreRequestedDays': days, + 'restoreRequestedAt': now, + archiveInfo + } + }, + expectedRes: { + 'creationTime': now, + 'lastModifiedDate': now, + 'updateMicroVersionId': true, + 'originOp': 's3:ObjectRestore:Completed', + 'content-length': 123456, + 'content-md5': 'md5', + 'content-type': 'text/plain', + taggingCopy: undefined, + archive: { + archiveInfo, + 'restoreRequestedDays': 3, + 'restoreRequestedAt': now, + 'restoreCompletedAt': new Date(now), + 'restoreWillExpireAt': new Date(now + (days * scaledMsPerDay)), + } + } + }, ].forEach(testCase => { it(testCase.description, () => { const metadataStoreParams = {}; From 4cdeecaa5083f97d9b459f7728083d08bce619e9 Mon Sep 17 00:00:00 2001 From: KillianG Date: Wed, 28 Jun 2023 15:45:57 +0000 Subject: [PATCH 5/7] Sinon fake timers in before each and after each to avoid collision with other tests Issue: CLDSRV-408 --- tests/unit/api/apiUtils/versioning.js | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/tests/unit/api/apiUtils/versioning.js b/tests/unit/api/apiUtils/versioning.js index 04d293ba4f..c323628a89 100644 --- a/tests/unit/api/apiUtils/versioning.js +++ b/tests/unit/api/apiUtils/versioning.js @@ -621,12 +621,20 @@ describe('versioning helpers', () => { }); describe('overwritingVersioning', () => { - sinon.useFakeTimers(); const days = 3; - const now = Date.now(); const archiveInfo = { 'archiveID': '126783123678', }; + const now = Date.now(); + let clock; + + beforeEach(() => { + clock = sinon.useFakeTimers(now); + }); + + afterEach(() => { + clock.restore(); + }); [ { From 646c3e2bb6d39c6ca8aa7ca756e87d580590ca86 Mon Sep 17 00:00:00 2001 From: KillianG Date: Wed, 5 Jul 2023 15:21:17 +0000 Subject: [PATCH 6/7] Adding functionnal testing and verfication for SSE and website redirect Issue: CLDSRV-408 --- lib/api/apiUtils/object/versioning.js | 20 +++++++++++---- lib/services.js | 5 ++++ .../aws-node-sdk/test/utils/init.js | 25 +++++++++++++++++++ tests/unit/api/apiUtils/versioning.js | 21 ++++++++-------- 4 files changed, 56 insertions(+), 15 deletions(-) diff --git a/lib/api/apiUtils/object/versioning.js b/lib/api/apiUtils/object/versioning.js index d6ef368c72..069634f684 100644 --- a/lib/api/apiUtils/object/versioning.js +++ b/lib/api/apiUtils/object/versioning.js @@ -443,18 +443,28 @@ function preprocessingVersioningDelete(bucketName, bucketMD, objectMD, reqVersio */ function restoreMetadatas(objMD, metadataStoreParams) { /* eslint-disable no-param-reassign */ - const userMDNotToKeep = ['x-amz-meta-scal-s3-restore-attempt']; + const userMDToSkip = ['x-amz-meta-scal-s3-restore-attempt']; // We need to keep user metadata and tags Object.keys(objMD).forEach(key => { - if (key.startsWith('x-amz-meta-') && !userMDNotToKeep.includes(key)) { + if (key.startsWith('x-amz-meta-') && !userMDToSkip.includes(key)) { metadataStoreParams.metaHeaders[key] = objMD[key]; - } - if (key.startsWith('content-') && !userMDNotToKeep.includes(key)) { + } else if (key.startsWith('content-')) { metadataStoreParams[key] = objMD[key]; } }); - if (objMD.replicationInfo?.status === 'COMPLETED') { + if (objMD['x-amz-server-side-encryption']) { + metadataStoreParams.cipherBundle = { + algorithm: objMD['x-amz-server-side-encryption'], + masterKeyId: objMD['x-amz-server-side-encryption-aws-kms-key-id'] || objMD['x-amz-server-side-encryption-customer-algorithm'], + } + } + + if (objMD['x-amz-website-redirect-location']) { + metadataStoreParams.headers['x-amz-website-redirect-location'] = objMD['x-amz-website-redirect-location']; + } + + if (objMD.replicationInfo) { metadataStoreParams.replicationInfo = objMD.replicationInfo; } diff --git a/lib/services.js b/lib/services.js index bc88fc142a..65d2c7ea08 100644 --- a/lib/services.js +++ b/lib/services.js @@ -128,6 +128,11 @@ const services = { if (lastModifiedDate) { md.setLastModified(lastModifiedDate); } + + if (!cipherBundle && params.cipherBundle) { + cipherBundle = params.cipherBundle; + } + if (cipherBundle) { md.setAmzServerSideEncryption(cipherBundle.algorithm); // configuredMasterKeyId takes precedence diff --git a/tests/functional/aws-node-sdk/test/utils/init.js b/tests/functional/aws-node-sdk/test/utils/init.js index 9816cbfe60..5c4c38fd91 100644 --- a/tests/functional/aws-node-sdk/test/utils/init.js +++ b/tests/functional/aws-node-sdk/test/utils/init.js @@ -79,6 +79,31 @@ function fakeMetadataArchive(bucketName, objectName, versionId, archive, cb) { /* eslint-disable no-param-reassign */ objMD.dataStoreName = 'location-dmf-v1'; objMD.archive = archive; + objMD['x-amz-meta-custom-user-md'] = 'custom-md'; + objMD.tags = { tag1: 'value1', tag2: 'value2' }; + objMD["x-amz-server-side-encryption"] = "AES256"; + objMD["x-amz-server-side-encryption-aws-kms-key-id"] = "very-secret-key"; + objMD['x-amz-website-redirect-location'] = 'https://scality.com/' + objMD.replicationInfo = { + "status" : "PENDING", + "backends" : [ + { + "site" : "azure-normal", + "status" : "PENDING", + "dataStoreVersionId" : "" + } + ], + "content" : [ + "DATA", + "METADATA" + ], + "destination" : "arn:aws:s3:::versioned", + "storageClass" : "azure-normal", + "role" : "arn:aws:iam::root:role/s3-replication-role", + "storageType" : "azure", + "dataStoreVersionId" : "", + "isNFS" : null + }; /* eslint-enable no-param-reassign */ return metadata.putObjectMD(bucketName, objectName, objMD, { versionId: decodeVersionId(versionId) }, log, err => cb(err)); diff --git a/tests/unit/api/apiUtils/versioning.js b/tests/unit/api/apiUtils/versioning.js index c323628a89..64e4746ac4 100644 --- a/tests/unit/api/apiUtils/versioning.js +++ b/tests/unit/api/apiUtils/versioning.js @@ -639,16 +639,17 @@ describe('versioning helpers', () => { [ { description: 'Should update archive with restore infos', - objMD: { - 'versionId': '2345678', - 'creation-time': now, - 'last-modified': now, - 'originOp': 's3:PutObject', - archive: { - 'restoreRequestedDays': days, - 'restoreRequestedAt': now, - archiveInfo - } }, + objMD: { + 'versionId': '2345678', + 'creation-time': now, + 'last-modified': now, + 'originOp': 's3:PutObject', + archive: { + 'restoreRequestedDays': days, + 'restoreRequestedAt': now, + archiveInfo + } + }, expectedRes: { 'creationTime': now, 'lastModifiedDate': now, From 4f4762f902f7ed9fa026375dac5f147f8ed0f0c5 Mon Sep 17 00:00:00 2001 From: KillianG Date: Wed, 5 Jul 2023 15:29:12 +0000 Subject: [PATCH 7/7] Lint and rework SSE condition Issue: CLDSRV-408 --- lib/api/apiUtils/object/versioning.js | 8 +++-- lib/services.js | 14 +++----- .../aws-node-sdk/test/object/objectCopy.js | 1 + .../aws-node-sdk/test/utils/init.js | 34 +++++++++---------- tests/unit/api/apiUtils/versioning.js | 2 +- 5 files changed, 29 insertions(+), 30 deletions(-) diff --git a/lib/api/apiUtils/object/versioning.js b/lib/api/apiUtils/object/versioning.js index 069634f684..0a0dcdd8c3 100644 --- a/lib/api/apiUtils/object/versioning.js +++ b/lib/api/apiUtils/object/versioning.js @@ -456,11 +456,15 @@ function restoreMetadatas(objMD, metadataStoreParams) { if (objMD['x-amz-server-side-encryption']) { metadataStoreParams.cipherBundle = { algorithm: objMD['x-amz-server-side-encryption'], - masterKeyId: objMD['x-amz-server-side-encryption-aws-kms-key-id'] || objMD['x-amz-server-side-encryption-customer-algorithm'], - } + masterKeyId: objMD['x-amz-server-side-encryption-aws-kms-key-id'] || + objMD['x-amz-server-side-encryption-customer-algorithm'], + }; } if (objMD['x-amz-website-redirect-location']) { + if (!metadataStoreParams.headers) { + metadataStoreParams.headers = {}; + } metadataStoreParams.headers['x-amz-website-redirect-location'] = objMD['x-amz-website-redirect-location']; } diff --git a/lib/services.js b/lib/services.js index 65d2c7ea08..4f8bbb61dd 100644 --- a/lib/services.js +++ b/lib/services.js @@ -129,16 +129,10 @@ const services = { md.setLastModified(lastModifiedDate); } - if (!cipherBundle && params.cipherBundle) { - cipherBundle = params.cipherBundle; - } - - if (cipherBundle) { - md.setAmzServerSideEncryption(cipherBundle.algorithm); - // configuredMasterKeyId takes precedence - if (cipherBundle.configuredMasterKeyId || cipherBundle.masterKeyId) { - md.setAmzEncryptionKeyId(cipherBundle.configuredMasterKeyId || cipherBundle.masterKeyId); - } + if (cipherBundle || params.cipherBundle) { + const bundle = cipherBundle || params.cipherBundle; + md.setAmzServerSideEncryption(bundle.algorithm); + md.setAmzEncryptionKeyId(bundle.configuredMasterKeyId || bundle.masterKeyId); } if (headers && headers['x-amz-website-redirect-location']) { md.setRedirectLocation(headers['x-amz-website-redirect-location']); diff --git a/tests/functional/aws-node-sdk/test/object/objectCopy.js b/tests/functional/aws-node-sdk/test/object/objectCopy.js index a1c3a1ac39..13b692bfee 100644 --- a/tests/functional/aws-node-sdk/test/object/objectCopy.js +++ b/tests/functional/aws-node-sdk/test/object/objectCopy.js @@ -1279,6 +1279,7 @@ describe('Object Copy', () => { restoreCompletedAt: new Date(10), restoreWillExpireAt: new Date(10 + (5 * 24 * 60 * 60 * 1000)), }; + originalMetadata['custom-user-md'] = 'custom-md'; fakeMetadataArchive(sourceBucketName, sourceObjName, undefined, archiveCompleted, err => { assert.ifError(err); s3.copyObject({ diff --git a/tests/functional/aws-node-sdk/test/utils/init.js b/tests/functional/aws-node-sdk/test/utils/init.js index 5c4c38fd91..60b6d93aed 100644 --- a/tests/functional/aws-node-sdk/test/utils/init.js +++ b/tests/functional/aws-node-sdk/test/utils/init.js @@ -81,28 +81,28 @@ function fakeMetadataArchive(bucketName, objectName, versionId, archive, cb) { objMD.archive = archive; objMD['x-amz-meta-custom-user-md'] = 'custom-md'; objMD.tags = { tag1: 'value1', tag2: 'value2' }; - objMD["x-amz-server-side-encryption"] = "AES256"; - objMD["x-amz-server-side-encryption-aws-kms-key-id"] = "very-secret-key"; - objMD['x-amz-website-redirect-location'] = 'https://scality.com/' + objMD['x-amz-server-side-encryption'] = 'AES256'; + objMD['x-amz-server-side-encryption-aws-kms-key-id'] = 'very-secret-key'; + objMD['x-amz-website-redirect-location'] = 'https://scality.com/'; objMD.replicationInfo = { - "status" : "PENDING", - "backends" : [ + 'status': 'PENDING', + 'backends': [ { - "site" : "azure-normal", - "status" : "PENDING", - "dataStoreVersionId" : "" + 'site': 'azure-normal', + 'status': 'PENDING', + 'dataStoreVersionId': '' } ], - "content" : [ - "DATA", - "METADATA" + 'content': [ + 'DATA', + 'METADATA' ], - "destination" : "arn:aws:s3:::versioned", - "storageClass" : "azure-normal", - "role" : "arn:aws:iam::root:role/s3-replication-role", - "storageType" : "azure", - "dataStoreVersionId" : "", - "isNFS" : null + 'destination': 'arn:aws:s3:::versioned', + 'storageClass': 'azure-normal', + 'role': 'arn:aws:iam::root:role/s3-replication-role', + 'storageType': 'azure', + 'dataStoreVersionId': '', + 'isNFS': null }; /* eslint-enable no-param-reassign */ return metadata.putObjectMD(bucketName, objectName, objMD, { versionId: decodeVersionId(versionId) }, diff --git a/tests/unit/api/apiUtils/versioning.js b/tests/unit/api/apiUtils/versioning.js index 64e4746ac4..7382f30e1d 100644 --- a/tests/unit/api/apiUtils/versioning.js +++ b/tests/unit/api/apiUtils/versioning.js @@ -648,7 +648,7 @@ describe('versioning helpers', () => { 'restoreRequestedDays': days, 'restoreRequestedAt': now, archiveInfo - } + } }, expectedRes: { 'creationTime': now,