From 7f04f13609f7836491c57cb419510268e9fb6a5b Mon Sep 17 00:00:00 2001 From: Jonathan Gramain Date: Fri, 9 May 2025 16:16:55 -0700 Subject: [PATCH 1/3] CLDSRV-647 [refactor] pass S3 config to getReplicationInfo Instead of pulling the location constraints and replication endpoints from the Cloudserver config within the getReplicationInfo helper itself, pass the config object explicitly. This makes the function pure, using only its input parameters, hence more test cases can be easily added with a different configuration for location constraints and/or replication endpoints. --- .../apiUtils/object/createAndStoreObject.js | 3 +- lib/api/apiUtils/object/getReplicationInfo.js | 16 ++++--- lib/api/completeMultipartUpload.js | 4 +- lib/api/objectCopy.js | 4 +- lib/api/objectDeleteTagging.js | 5 ++- lib/api/objectPutLegalHold.js | 5 ++- lib/api/objectPutRetention.js | 5 ++- lib/api/objectPutTagging.js | 5 ++- lib/metadata/acl.js | 3 +- tests/unit/api/apiUtils/getReplicationInfo.js | 43 +++++++++++++++---- 10 files changed, 64 insertions(+), 29 deletions(-) diff --git a/lib/api/apiUtils/object/createAndStoreObject.js b/lib/api/apiUtils/object/createAndStoreObject.js index 9d7c84b02b..46c38a4b12 100644 --- a/lib/api/apiUtils/object/createAndStoreObject.js +++ b/lib/api/apiUtils/object/createAndStoreObject.js @@ -135,7 +135,8 @@ function createAndStoreObject(bucketName, bucketMD, objectKey, objMD, authInfo, size, headers, isDeleteMarker, - replicationInfo: getReplicationInfo(objectKey, bucketMD, false, size, null, null, authInfo, isDeleteMarker), + replicationInfo: getReplicationInfo(config, + objectKey, bucketMD, false, size, null, null, authInfo, isDeleteMarker), log, }; diff --git a/lib/api/apiUtils/object/getReplicationInfo.js b/lib/api/apiUtils/object/getReplicationInfo.js index cbda915851..f9a54e8c97 100644 --- a/lib/api/apiUtils/object/getReplicationInfo.js +++ b/lib/api/apiUtils/object/getReplicationInfo.js @@ -1,4 +1,3 @@ -const s3config = require('../../../Config').config; const { isLifecycleSession } = require('../authorization/permissionChecks.js'); function _getBackend(objectMD, site) { @@ -15,7 +14,7 @@ function _getBackend(objectMD, site) { }; } -function _getStorageClasses(rule) { +function _getStorageClasses(s3config, rule) { if (rule.storageClass) { return rule.storageClass.split(','); } @@ -29,11 +28,11 @@ function _getStorageClasses(rule) { return [replicationEndpoints[0].site]; } -function _getReplicationInfo(rule, replicationConfig, content, operationType, +function _getReplicationInfo(s3config, rule, replicationConfig, content, operationType, objectMD) { const storageTypes = []; const backends = []; - const storageClasses = _getStorageClasses(rule); + const storageClasses = _getStorageClasses(s3config, rule); storageClasses.forEach(storageClass => { const location = s3config.locationConstraints[storageClass]; if (location && ['aws_s3', 'azure'].includes(location.type)) { @@ -58,6 +57,9 @@ function _getReplicationInfo(rule, replicationConfig, content, operationType, /** * Get the object replicationInfo to replicate data and metadata, or only * metadata if the operation only changes metadata or the object is 0 bytes + * @param {object} s3config - Cloudserver configuration object + * @param {object} s3config.locationConstraints - Configured map of location constraints + * @param {object[]} s3config.replicationEndpoints - Configured replication endpoints * @param {string} objKey - The key of the object * @param {object} bucketMD - The bucket metadata * @param {boolean} isMD - Whether the operation is only updating metadata @@ -68,8 +70,8 @@ function _getReplicationInfo(rule, replicationConfig, content, operationType, * @param {boolean} [isDeleteMarker] - whether creating a delete marker * @return {undefined} */ -function getReplicationInfo(objKey, bucketMD, isMD, objSize, operationType, - objectMD, authInfo, isDeleteMarker) { +function getReplicationInfo(s3config, + objKey, bucketMD, isMD, objSize, operationType, objectMD, authInfo, isDeleteMarker) { const content = isMD || objSize === 0 ? ['METADATA'] : ['DATA', 'METADATA']; const config = bucketMD.getReplicationConfiguration(); // If bucket does not have a replication configuration, do not replicate. @@ -83,7 +85,7 @@ function getReplicationInfo(objKey, bucketMD, isMD, objSize, operationType, const rule = config.rules.find(rule => (objKey.startsWith(rule.prefix) && rule.enabled)); if (rule) { - return _getReplicationInfo(rule, config, content, operationType, + return _getReplicationInfo(s3config, rule, config, content, operationType, objectMD); } } diff --git a/lib/api/completeMultipartUpload.js b/lib/api/completeMultipartUpload.js index 0fe7214adc..024c6a2f35 100644 --- a/lib/api/completeMultipartUpload.js +++ b/lib/api/completeMultipartUpload.js @@ -301,8 +301,8 @@ function completeMultipartUpload(authInfo, request, log, callback) { contentMD5: aggregateETag, size: calculatedSize, multipart: true, - replicationInfo: getReplicationInfo(objectKey, destBucket, - false, calculatedSize, REPLICATION_ACTION), + replicationInfo: getReplicationInfo(config, + objectKey, destBucket, false, calculatedSize, REPLICATION_ACTION), originOp: 's3:ObjectCreated:CompleteMultipartUpload', log, }; diff --git a/lib/api/objectCopy.js b/lib/api/objectCopy.js index 44ce1558e5..8f3689fdf2 100644 --- a/lib/api/objectCopy.js +++ b/lib/api/objectCopy.js @@ -167,8 +167,8 @@ function _prepMetadata(request, sourceObjMD, headers, sourceIsDestination, lastModifiedDate: new Date().toJSON(), tagging, taggingCopy, - replicationInfo: getReplicationInfo(objectKey, destBucketMD, false, - sourceObjMD['content-length']), + replicationInfo: getReplicationInfo(config, + objectKey, destBucketMD, false, sourceObjMD['content-length']), locationMatch, originOp: 's3:ObjectCreated:Copy', }; diff --git a/lib/api/objectDeleteTagging.js b/lib/api/objectDeleteTagging.js index b21fd3f3a8..143041afff 100644 --- a/lib/api/objectDeleteTagging.js +++ b/lib/api/objectDeleteTagging.js @@ -10,6 +10,7 @@ const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const metadata = require('../metadata/wrapper'); const getReplicationInfo = require('./apiUtils/object/getReplicationInfo'); const { data } = require('../data/wrapper'); +const { config } = require('../Config'); const REPLICATION_ACTION = 'DELETE_TAGGING'; /** @@ -72,8 +73,8 @@ function objectDeleteTagging(authInfo, request, log, callback) { objectMD.tags = {}; const params = objectMD.versionId ? { versionId: objectMD.versionId } : {}; - const replicationInfo = getReplicationInfo(objectKey, bucket, true, - 0, REPLICATION_ACTION, objectMD); + const replicationInfo = getReplicationInfo(config, + objectKey, bucket, true, 0, REPLICATION_ACTION, objectMD); if (replicationInfo) { // eslint-disable-next-line no-param-reassign objectMD.replicationInfo = Object.assign({}, diff --git a/lib/api/objectPutLegalHold.js b/lib/api/objectPutLegalHold.js index b37f3b28cd..29cb14fd70 100644 --- a/lib/api/objectPutLegalHold.js +++ b/lib/api/objectPutLegalHold.js @@ -8,6 +8,7 @@ const getReplicationInfo = require('./apiUtils/object/getReplicationInfo'); const metadata = require('../metadata/wrapper'); const { standardMetadataValidateBucketAndObj } = require('../metadata/metadataUtils'); const { pushMetric } = require('../utapi/utilities'); +const { config } = require('../Config'); const { parseLegalHoldXml } = s3middleware.objectLegalHold; @@ -84,8 +85,8 @@ function objectPutLegalHold(authInfo, request, log, callback) { objectMD.legalHold = legalHold; const params = objectMD.versionId ? { versionId: objectMD.versionId } : {}; - const replicationInfo = getReplicationInfo(objectKey, bucket, true, - 0, REPLICATION_ACTION, objectMD); + const replicationInfo = getReplicationInfo(config, + objectKey, bucket, true, 0, REPLICATION_ACTION, objectMD); if (replicationInfo) { // eslint-disable-next-line no-param-reassign objectMD.replicationInfo = Object.assign({}, diff --git a/lib/api/objectPutRetention.js b/lib/api/objectPutRetention.js index c5afdcd9e5..70c9c3cbe3 100644 --- a/lib/api/objectPutRetention.js +++ b/lib/api/objectPutRetention.js @@ -10,6 +10,7 @@ const { pushMetric } = require('../utapi/utilities'); const getReplicationInfo = require('./apiUtils/object/getReplicationInfo'); const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const metadata = require('../metadata/wrapper'); +const { config } = require('../Config'); const { parseRetentionXml } = s3middleware.retention; const REPLICATION_ACTION = 'PUT_RETENTION'; @@ -124,8 +125,8 @@ function objectPutRetention(authInfo, request, log, callback) { objectMD.retentionDate = retentionInfo.date; const params = objectMD.versionId ? { versionId: objectMD.versionId } : {}; - const replicationInfo = getReplicationInfo(objectKey, bucket, true, - 0, REPLICATION_ACTION, objectMD); + const replicationInfo = getReplicationInfo(config, + objectKey, bucket, true, 0, REPLICATION_ACTION, objectMD); if (replicationInfo) { objectMD.replicationInfo = Object.assign({}, objectMD.replicationInfo, replicationInfo); diff --git a/lib/api/objectPutTagging.js b/lib/api/objectPutTagging.js index 8514b8340b..73bd9514c3 100644 --- a/lib/api/objectPutTagging.js +++ b/lib/api/objectPutTagging.js @@ -10,6 +10,7 @@ const getReplicationInfo = require('./apiUtils/object/getReplicationInfo'); const collectCorsHeaders = require('../utilities/collectCorsHeaders'); const metadata = require('../metadata/wrapper'); const { data } = require('../data/wrapper'); +const { config } = require('../Config'); const { parseTagXml } = s3middleware.tagging; const REPLICATION_ACTION = 'PUT_TAGGING'; @@ -77,8 +78,8 @@ function objectPutTagging(authInfo, request, log, callback) { objectMD.tags = tags; const params = objectMD.versionId ? { versionId: objectMD.versionId } : {}; - const replicationInfo = getReplicationInfo(objectKey, bucket, true, - 0, REPLICATION_ACTION, objectMD); + const replicationInfo = getReplicationInfo(config, + objectKey, bucket, true, 0, REPLICATION_ACTION, objectMD); if (replicationInfo) { // eslint-disable-next-line no-param-reassign objectMD.replicationInfo = Object.assign({}, diff --git a/lib/metadata/acl.js b/lib/metadata/acl.js index ae57be55ad..d12e23cc64 100644 --- a/lib/metadata/acl.js +++ b/lib/metadata/acl.js @@ -5,6 +5,7 @@ const aclUtils = require('../utilities/aclUtils'); const constants = require('../../constants'); const metadata = require('../metadata/wrapper'); const vault = require('../auth/vault'); +const { config } = require('../Config'); const acl = { addACL(bucket, addACLParams, log, cb) { @@ -19,7 +20,7 @@ const acl = { objectMD.acl = addACLParams; // eslint-disable-next-line no-param-reassign objectMD.originOp = 's3:ObjectAcl:Put'; - const replicationInfo = getReplicationInfo(objectKey, bucket, true); + const replicationInfo = getReplicationInfo(config, objectKey, bucket, true); if (replicationInfo) { // eslint-disable-next-line no-param-reassign objectMD.replicationInfo = Object.assign({}, diff --git a/tests/unit/api/apiUtils/getReplicationInfo.js b/tests/unit/api/apiUtils/getReplicationInfo.js index 75f13af158..c6749461f5 100644 --- a/tests/unit/api/apiUtils/getReplicationInfo.js +++ b/tests/unit/api/apiUtils/getReplicationInfo.js @@ -5,15 +5,39 @@ const getReplicationInfo = require('../../../../lib/api/apiUtils/object/getReplicationInfo'); const { makeAuthInfo } = require('../../helpers'); -function _getObjectReplicationInfo(replicationConfig, authInfo, isDeleteMarker) { +function _getObjectReplicationInfo(s3config, replicationConfig, authInfo, isDeleteMarker) { const bucketInfo = new BucketInfo( 'testbucket', 'someCanonicalId', 'accountDisplayName', new Date().toJSON(), null, null, null, null, null, null, null, null, null, replicationConfig); - return getReplicationInfo('fookey', bucketInfo, true, 123, null, null, authInfo, isDeleteMarker); + return getReplicationInfo(s3config, + 'fookey', bucketInfo, true, 123, null, null, authInfo, isDeleteMarker); } +const TEST_CONFIG = { + locationConstraints: { + awsbackend: { + type: 'aws_s3', + legacyAwsBehavior: true, + details: { + awsEndpoint: 's3.amazonaws.com', + bucketName: 'awsbucket', + bucketMatch: true, + credentialsProfile: 'default', + }, + }, + }, + replicationEndpoints: [{ + site: 'zenko', + servers: ['127.0.0.1:8000'], + default: true, + }, { + site: 'us-east-2', + type: 'aws_s3', + }], +}; + describe('getReplicationInfo helper', () => { it('should get replication info when rules are enabled', () => { const replicationConfig = { @@ -25,7 +49,7 @@ describe('getReplicationInfo helper', () => { }], destination: 'tosomewhere', }; - const replicationInfo = _getObjectReplicationInfo(replicationConfig); + const replicationInfo = _getObjectReplicationInfo(TEST_CONFIG, replicationConfig); assert.deepStrictEqual(replicationInfo, { status: 'PENDING', backends: [{ @@ -53,7 +77,8 @@ describe('getReplicationInfo helper', () => { }; const authInfo = makeAuthInfo('accessKey1', null, 'another-session'); - const replicationInfo = _getObjectReplicationInfo(replicationConfig, authInfo, true); + const replicationInfo = _getObjectReplicationInfo(TEST_CONFIG, + replicationConfig, authInfo, true); assert.deepStrictEqual(replicationInfo, { status: 'PENDING', @@ -83,7 +108,8 @@ describe('getReplicationInfo helper', () => { }; const authInfo = makeAuthInfo('accessKey1', null, 'backbeat-lifecycle'); - const replicationInfo = _getObjectReplicationInfo(replicationConfig, authInfo, false); + const replicationInfo = _getObjectReplicationInfo(TEST_CONFIG, + replicationConfig, authInfo, false); assert.deepStrictEqual(replicationInfo, { status: 'PENDING', @@ -110,11 +136,11 @@ describe('getReplicationInfo helper', () => { }], destination: 'tosomewhere', }; - const replicationInfo = _getObjectReplicationInfo(replicationConfig); + const replicationInfo = _getObjectReplicationInfo(TEST_CONFIG, replicationConfig); assert.deepStrictEqual(replicationInfo, undefined); }); - it('should not get replication info when action comming from lifecycle session', () => { + it('should not get replication info when action coming from lifecycle session', () => { const replicationConfig = { role: 'arn:aws:iam::root:role/s3-replication-role', rules: [{ @@ -126,7 +152,8 @@ describe('getReplicationInfo helper', () => { }; const authInfo = makeAuthInfo('accessKey1', null, 'backbeat-lifecycle'); - const replicationInfo = _getObjectReplicationInfo(replicationConfig, authInfo, true); + const replicationInfo = _getObjectReplicationInfo(TEST_CONFIG, + replicationConfig, authInfo, true); assert.deepStrictEqual(replicationInfo, undefined); }); From db8ed585441f0c62e0cf8249f27cab466a772f4f Mon Sep 17 00:00:00 2001 From: Jonathan Gramain Date: Fri, 9 May 2025 16:49:18 -0700 Subject: [PATCH 2/3] bf: CLDSRV-647 crash in getReplicationInfo with no replication endpoint Fix an exception in getReplicationInfo when: - no StorageClass is provided - no replication endpoint is configured Instead, return `undefined` so the object is not set to PENDING state for replication, even if the bucket already has a replication configuration enabled. --- lib/api/apiUtils/object/getReplicationInfo.js | 6 ++ tests/unit/api/apiUtils/getReplicationInfo.js | 74 +++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/lib/api/apiUtils/object/getReplicationInfo.js b/lib/api/apiUtils/object/getReplicationInfo.js index f9a54e8c97..a25193a0c9 100644 --- a/lib/api/apiUtils/object/getReplicationInfo.js +++ b/lib/api/apiUtils/object/getReplicationInfo.js @@ -25,6 +25,9 @@ function _getStorageClasses(s3config, rule) { replicationEndpoints.find(endpoint => endpoint.default); return [endPoint.site]; } + if (replicationEndpoints.length === 0) { + return []; + } return [replicationEndpoints[0].site]; } @@ -33,6 +36,9 @@ function _getReplicationInfo(s3config, rule, replicationConfig, content, operati const storageTypes = []; const backends = []; const storageClasses = _getStorageClasses(s3config, rule); + if (storageClasses.length === 0) { + return undefined; + } storageClasses.forEach(storageClass => { const location = s3config.locationConstraints[storageClass]; if (location && ['aws_s3', 'azure'].includes(location.type)) { diff --git a/tests/unit/api/apiUtils/getReplicationInfo.js b/tests/unit/api/apiUtils/getReplicationInfo.js index c6749461f5..c1a237268a 100644 --- a/tests/unit/api/apiUtils/getReplicationInfo.js +++ b/tests/unit/api/apiUtils/getReplicationInfo.js @@ -157,4 +157,78 @@ describe('getReplicationInfo helper', () => { assert.deepStrictEqual(replicationInfo, undefined); }); + + it('should get replication info with default StorageClass when rules are enabled', () => { + const replicationConfig = { + role: 'arn:aws:iam::root:role/s3-replication-role-1,arn:aws:iam::root:role/s3-replication-role-2', + rules: [{ + prefix: '', + enabled: true, + }], + destination: 'tosomewhere', + }; + const replicationInfo = _getObjectReplicationInfo(TEST_CONFIG, replicationConfig); + assert.deepStrictEqual(replicationInfo, { + status: 'PENDING', + backends: [{ + site: 'zenko', + status: 'PENDING', + dataStoreVersionId: '', + }], + content: ['METADATA'], + destination: 'tosomewhere', + storageClass: 'zenko', + role: 'arn:aws:iam::root:role/s3-replication-role-1,arn:aws:iam::root:role/s3-replication-role-2', + storageType: '', + }); + }); + + it('should return undefined with specified StorageClass mode if no replication endpoint is configured', () => { + const replicationConfig = { + role: 'arn:aws:iam::root:role/s3-replication-role', + rules: [{ + prefix: '', + enabled: true, + storageClass: 'awsbackend', + }], + destination: 'tosomewhere', + }; + const configWithNoReplicationEndpoint = { + locationConstraints: TEST_CONFIG.locationConstraints, + replicationEndpoints: [], + }; + const replicationInfo = _getObjectReplicationInfo(configWithNoReplicationEndpoint, + replicationConfig); + assert.deepStrictEqual(replicationInfo, { + status: 'PENDING', + backends: [{ + site: 'awsbackend', + status: 'PENDING', + dataStoreVersionId: '', + }], + content: ['METADATA'], + destination: 'tosomewhere', + storageClass: 'awsbackend', + role: 'arn:aws:iam::root:role/s3-replication-role', + storageType: 'aws_s3', + }); + }); + + it('should return undefined with default StorageClass if no replication endpoint is configured', () => { + const replicationConfig = { + role: 'arn:aws:iam::root:role/s3-replication-role-1,arn:aws:iam::root:role/s3-replication-role-2', + rules: [{ + prefix: '', + enabled: true, + }], + destination: 'tosomewhere', + }; + const configWithNoReplicationEndpoint = { + locationConstraints: TEST_CONFIG.locationConstraints, + replicationEndpoints: [], + }; + const replicationInfo = _getObjectReplicationInfo(configWithNoReplicationEndpoint, + replicationConfig); + assert.deepStrictEqual(replicationInfo, undefined); + }); }); From 71bbee6c4c0cd78427d2d5d42ca170ca0f383e7b Mon Sep 17 00:00:00 2001 From: Jonathan Gramain Date: Tue, 13 May 2025 09:22:23 -0700 Subject: [PATCH 3/3] CLDSRV-647 bump cloudserver version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 0d6c6cc466..bfa3cf9be6 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "s3", - "version": "7.10.54", + "version": "7.10.55", "description": "S3 connector", "main": "index.js", "engines": {