From 7f6716dbfd01822bfb80dcfe3c8483e54c820b9e Mon Sep 17 00:00:00 2001 From: Mickael Bourgois Date: Thu, 14 Aug 2025 14:58:25 +0200 Subject: [PATCH 1/9] CLDSRV-717: Fix backbeat route parsedContentLength Because it doesn't go through arsenal's normalizeRequest. Do the same as for veeam route, to have the log field bytesReceived --- lib/routes/routeBackbeat.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/lib/routes/routeBackbeat.js b/lib/routes/routeBackbeat.js index 3d69912bd3..89b671e17a 100644 --- a/lib/routes/routeBackbeat.js +++ b/lib/routes/routeBackbeat.js @@ -1463,6 +1463,9 @@ function routeBackbeat(clientIP, request, response, log) { // Attach the apiMethod method to the request, so it can used by monitoring in the server // eslint-disable-next-line no-param-reassign request.apiMethod = 'routeBackbeat'; + const contentLength = request.headers['x-amz-decoded-content-length'] || request.headers['content-length']; + // eslint-disable-next-line no-param-reassign + request.parsedContentLength = Number.parseInt(contentLength?.toString() ?? '', 10); log.addDefaultFields({ clientIP, url: request.url, From 9661e86f217cc3aa30076ec9322b0d640546c774 Mon Sep 17 00:00:00 2001 From: Mickael Bourgois Date: Thu, 14 Aug 2025 15:01:33 +0200 Subject: [PATCH 2/9] CLDSRV-717: Fix tests with location This file is imported in tests used by S3C that have no external location configured --- .../test/multipleBackend/utils.js | 28 +++++++++++++------ 1 file changed, 19 insertions(+), 9 deletions(-) diff --git a/tests/functional/aws-node-sdk/test/multipleBackend/utils.js b/tests/functional/aws-node-sdk/test/multipleBackend/utils.js index 1774387d52..3a755c85b7 100644 --- a/tests/functional/aws-node-sdk/test/multipleBackend/utils.js +++ b/tests/functional/aws-node-sdk/test/multipleBackend/utils.js @@ -47,15 +47,25 @@ let gcpBucket; let gcpBucketMPU; if (config.backends.data === 'multiple') { - const awsConfig = getRealAwsConfig(awsLocation); - awsS3 = new AWS.S3(awsConfig); - awsBucket = config.locationConstraints[awsLocation].details.bucketName; - - const gcpConfig = getRealAwsConfig(gcpLocation); - gcpClient = new GCP(gcpConfig); - gcpBucket = config.locationConstraints[gcpLocation].details.bucketName; - gcpBucketMPU = - config.locationConstraints[gcpLocation].details.mpuBucketName; + if (config.locationConstraints[awsLocation]) { + const awsConfig = getRealAwsConfig(awsLocation); + awsS3 = new AWS.S3(awsConfig); + awsBucket = config.locationConstraints[awsLocation].details.bucketName; + } else { + process.stdout.write(`LocationConstraint for aws '${awsLocation}' not found in ${ + Object.keys(config.locationConstraints)}\n`); + } + + if (config.locationConstraints[gcpLocation]) { + const gcpConfig = getRealAwsConfig(gcpLocation); + gcpClient = new GCP(gcpConfig); + gcpBucket = config.locationConstraints[gcpLocation].details.bucketName; + gcpBucketMPU = + config.locationConstraints[gcpLocation].details.mpuBucketName; + } else { + process.stdout.write(`LocationConstraint for gcp '${gcpLocation}' not found in ${ + Object.keys(config.locationConstraints)}\n`); + } } From 4ce7ecbfc272fca32468be3c66a3a9131847215b Mon Sep 17 00:00:00 2001 From: Mickael Bourgois Date: Thu, 7 Aug 2025 18:31:27 +0200 Subject: [PATCH 3/9] CLDSRV-717: Fix script ft_route_backbeat --- package.json | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/package.json b/package.json index b488b65700..f3bfb69331 100644 --- a/package.json +++ b/package.json @@ -93,7 +93,7 @@ "ft_backbeat": "cd tests/functional/backbeat && mocha --reporter mocha-multi-reporters --reporter-options configFile=$INIT_CWD/tests/reporter-config.json -t 40000 *.js --exit", "ft_node": "cd tests/functional/raw-node && yarn test", "ft_node_routes": "cd tests/functional/raw-node && yarn run test-routes", - "ft_node_route_backbeat": "cd tests/functional/raw-node && mocha --reporter mocha-multi-reporters --reporter-options configFile=$INIT_CWD/tests/reporter-config.json -t 40000 test/routes/routeBackbeat.js --exit", + "ft_route_backbeat": "cd tests/multipleBackend/routes && mocha --reporter mocha-multi-reporters --reporter-options configFile=$INIT_CWD/tests/reporter-config.json -t 40000 routeBackbeat.js routeBackbeatForReplication.js --exit", "ft_gcp": "cd tests/functional/raw-node && yarn run test-gcp", "ft_healthchecks": "cd tests/functional/healthchecks && yarn test", "ft_s3cmd": "cd tests/functional/s3cmd && mocha --reporter mocha-multi-reporters --reporter-options configFile=$INIT_CWD/tests/reporter-config.json -t 40000 *.js --exit", @@ -107,7 +107,6 @@ "ft_sse_before_migration": "cd tests/functional/sse-kms-migration && mocha --reporter mocha-multi-reporters --reporter-options configFile=$INIT_CWD/tests/reporter-config.json -t 10000 cleanup.js beforeMigration.js --exit", "ft_sse_migration": "cd tests/functional/sse-kms-migration && mocha --reporter mocha-multi-reporters --reporter-options configFile=$INIT_CWD/tests/reporter-config.json -t 10000 migration.js --exit", "ft_sse_arn": "cd tests/functional/sse-kms-migration && mocha --reporter mocha-multi-reporters --reporter-options configFile=$INIT_CWD/tests/reporter-config.json -t 10000 cleanup.js arnPrefix.js --exit", - "install_ft_deps": "yarn install aws-sdk@2.28.0 bluebird@3.3.1 mocha@2.3.4 mocha-junit-reporter@1.23.1 tv4@1.2.7", "lint": "eslint $(git ls-files '*.js')", "lint_md": "mdlint $(git ls-files '*.md')", "mem_backend": "S3BACKEND=mem node index.js", From 3164f7678cf6ce26d40af7dd18c423119e552edd Mon Sep 17 00:00:00 2001 From: Mickael Bourgois Date: Wed, 27 Aug 2025 12:05:10 +0200 Subject: [PATCH 4/9] CLDSRV-717: Replace hardcoded credentials --- tests/multipleBackend/routes/routeBackbeat.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/multipleBackend/routes/routeBackbeat.js b/tests/multipleBackend/routes/routeBackbeat.js index f436a513b0..cfac3814b6 100644 --- a/tests/multipleBackend/routes/routeBackbeat.js +++ b/tests/multipleBackend/routes/routeBackbeat.js @@ -2165,6 +2165,8 @@ describe('backbeat routes', () => { }); }); describe('backbeat authorization checks', () => { + const { accessKeyId: accessKeyLisa, secretAccessKey: secretAccessKeyLisa } = getCredentials('lisa'); + [{ method: 'PUT', resourceType: 'metadata' }, { method: 'PUT', resourceType: 'data' }].forEach(test => { const queryObj = test.resourceType === 'data' ? { v2: '' } : {}; @@ -2211,8 +2213,8 @@ describe('backbeat routes', () => { objectKey: TEST_KEY, resourceType: test.resourceType, queryObj, authCredentials: { - accessKey: 'accessKey2', - secretKey: 'verySecretKey2', + accessKey: accessKeyLisa, + secretKey: secretAccessKeyLisa, }, }, err => { From 5370181d767b091196a8543d9f180eda460ad6a5 Mon Sep 17 00:00:00 2001 From: Mickael Bourgois Date: Wed, 27 Aug 2025 12:06:33 +0200 Subject: [PATCH 5/9] CLDSRV-717: Unify backbeat api proxy test --- tests/multipleBackend/routes/routeBackbeat.js | 91 +++++++++++-------- 1 file changed, 53 insertions(+), 38 deletions(-) diff --git a/tests/multipleBackend/routes/routeBackbeat.js b/tests/multipleBackend/routes/routeBackbeat.js index cfac3814b6..be16b5235a 100644 --- a/tests/multipleBackend/routes/routeBackbeat.js +++ b/tests/multipleBackend/routes/routeBackbeat.js @@ -2244,44 +2244,59 @@ describe('backbeat routes', () => { }); }); }); - it('GET /_/backbeat/api/... should respond with ' + - '503 on authenticated requests (API server down)', - done => { - const options = { - authCredentials: { - accessKey: 'accessKey2', - secretKey: 'verySecretKey2', - }, - hostname: ipAddress, - port: 8000, - method: 'GET', - path: '/_/backbeat/api/crr/failed', - jsonResponse: true, - }; - makeRequest(options, err => { - assert(err); - assert.strictEqual(err.statusCode, 503); - assert.strictEqual(err.code, 'ServiceUnavailable'); - done(); - }); - }); - it('GET /_/backbeat/api/... should respond with ' + - '403 Forbidden if the request is unauthenticated', - done => { - const options = { - hostname: ipAddress, - port: 8000, - method: 'GET', - path: '/_/backbeat/api/crr/failed', - jsonResponse: true, - }; - makeRequest(options, err => { - assert(err); - assert.strictEqual(err.statusCode, 403); - assert.strictEqual(err.code, 'AccessDenied'); - done(); - }); - }); + + const apiProxy = !!config.backbeat; + describe(`when api proxy is ${apiProxy ? '' : 'NOT '}configured`, () => { + const errors = { + 403: 'AccessDenied', + 405: 'MethodNotAllowed', + 503: 'ServiceUnavailable', + }; + + it(`GET /_/backbeat/api/... should respond with ${ + apiProxy ? 503 : 405 + } on authenticated requests (API server down)`, + done => { + const options = { + authCredentials: { + accessKey: accessKeyLisa, + secretKey: secretAccessKeyLisa, + }, + hostname: ipAddress, + port: 8000, + method: 'GET', + path: '/_/backbeat/api/crr/failed', + jsonResponse: true, + }; + makeRequest(options, err => { + assert(err); + const expected = apiProxy ? 503 : 405; + assert.strictEqual(err.statusCode, expected); + assert.strictEqual(err.code, errors[expected]); + done(); + }); + }); + + it(`GET /_/backbeat/api/... should respond with ${ + apiProxy ? 403 : 405 + } if the request is unauthenticated`, + done => { + const options = { + hostname: ipAddress, + port: 8000, + method: 'GET', + path: '/_/backbeat/api/crr/failed', + jsonResponse: true, + }; + makeRequest(options, err => { + assert(err); + const expected = apiProxy ? 403 : 405; + assert.strictEqual(err.statusCode, expected); + assert.strictEqual(err.code, errors[expected]); + done(); + }); + }); + }); }); describe('GET Metadata route', () => { From d0f3bcc1788ec78b40bfc83d8a16d5ec9639f960 Mon Sep 17 00:00:00 2001 From: Mickael Bourgois Date: Wed, 27 Aug 2025 12:09:26 +0200 Subject: [PATCH 6/9] CLDSRV-717: Update backbeat GET metadata error tst --- tests/multipleBackend/routes/routeBackbeat.js | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/tests/multipleBackend/routes/routeBackbeat.js b/tests/multipleBackend/routes/routeBackbeat.js index be16b5235a..deffd4816a 100644 --- a/tests/multipleBackend/routes/routeBackbeat.js +++ b/tests/multipleBackend/routes/routeBackbeat.js @@ -280,7 +280,7 @@ describe('backbeat routes', () => { .then(() => done()) .catch(err => { process.stdout.write(`Error creating bucket: ${err}\n`); - throw err; + done(err); }); }); @@ -2320,6 +2320,7 @@ describe('backbeat routes', () => { versionId: versionIdUtils.encode(testMd.versionId), }, }, (err, data) => { + assert.ifError(err); const parsedBody = JSON.parse(JSON.parse(data.body).Body); assert.strictEqual(data.statusCode, 200); assert.deepStrictEqual(parsedBody, testMd); @@ -2337,7 +2338,10 @@ describe('backbeat routes', () => { }, }, (err, data) => { assert.strictEqual(data.statusCode, 404); - assert.strictEqual(JSON.parse(data.body).code, 'NoSuchBucket'); + const body = JSON.parse(data.body); + assert.strictEqual(body.code, 'NoSuchBucket'); + // err is parsed data.body + statusCode + assert.deepStrictEqual(err, { ...body, statusCode: data.statusCode }); done(); }); }); @@ -2352,7 +2356,10 @@ describe('backbeat routes', () => { }, }, (err, data) => { assert.strictEqual(data.statusCode, 404); - assert.strictEqual(JSON.parse(data.body).code, 'ObjNotFound'); + const body = JSON.parse(data.body); + assert.strictEqual(body.code, 'ObjNotFound'); + // err is parsed data.body + statusCode + assert.deepStrictEqual(err, { ...body, statusCode: data.statusCode }); done(); }); }); From 288606a7de0773121c7518cabfe4b124c7897aae Mon Sep 17 00:00:00 2001 From: Mickael Bourgois Date: Wed, 27 Aug 2025 12:11:32 +0200 Subject: [PATCH 7/9] CLDSRV-717: Skip based on configured location To unify with S3C --- .../aws-node-sdk/lib/utility/test-utils.js | 5 +++ tests/multipleBackend/routes/routeBackbeat.js | 39 +++++++++++-------- 2 files changed, 27 insertions(+), 17 deletions(-) diff --git a/tests/functional/aws-node-sdk/lib/utility/test-utils.js b/tests/functional/aws-node-sdk/lib/utility/test-utils.js index a9a909d7db..d0a1191804 100644 --- a/tests/functional/aws-node-sdk/lib/utility/test-utils.js +++ b/tests/functional/aws-node-sdk/lib/utility/test-utils.js @@ -11,10 +11,15 @@ if (config.backends.data === 'multiple') { describeSkipIfNotMultipleOrCeph = isCEPH ? describe.skip : describe.skip; // always skip } +function hasLocation(lc) { + return config.locationConstraints[lc] !== undefined; +} + module.exports = { isCEPH, itSkipCeph, describeSkipIfCeph, describeSkipIfNotMultiple, describeSkipIfNotMultipleOrCeph, + hasLocation, }; diff --git a/tests/multipleBackend/routes/routeBackbeat.js b/tests/multipleBackend/routes/routeBackbeat.js index deffd4816a..4375985389 100644 --- a/tests/multipleBackend/routes/routeBackbeat.js +++ b/tests/multipleBackend/routes/routeBackbeat.js @@ -1,5 +1,4 @@ const assert = require('assert'); -const AWS = require('aws-sdk'); const async = require('async'); const crypto = require('crypto'); const { v4: uuidv4 } = require('uuid'); @@ -8,23 +7,23 @@ const versionIdUtils = versioning.VersionID; const { makeid } = require('../../unit/helpers'); const { makeRequest, makeBackbeatRequest } = require('../../functional/raw-node/utils/makeRequest'); -const BucketUtility = - require('../../functional/aws-node-sdk/lib/utility/bucket-util'); -const { describeSkipIfNotMultipleOrCeph, itSkipCeph } = require('../../functional/aws-node-sdk/lib/utility/test-utils'); +const BucketUtility = require('../../functional/aws-node-sdk/lib/utility/bucket-util'); +const { + describeSkipIfNotMultipleOrCeph, + itSkipCeph, + hasLocation +} = require('../../functional/aws-node-sdk/lib/utility/test-utils'); const { awsLocation, + awsS3: awsClient, + awsBucket, azureLocation, getAzureContainerName, getAzureClient, } = require('../../functional/aws-node-sdk/test/multipleBackend/utils'); -const { getRealAwsConfig } = - require('../../functional/aws-node-sdk/test/support/awsConfig'); const { getCredentials } = require('../../functional/aws-node-sdk/test/support/credentials'); const { config } = require('../../../lib/Config'); -const awsConfig = getRealAwsConfig(awsLocation); -const awsClient = new AWS.S3(awsConfig); -const awsBucket = config.locationConstraints[awsLocation].details.bucketName; const azureClient = getAzureClient(); const containerName = getAzureContainerName(azureLocation); @@ -165,6 +164,10 @@ function updateStorageClass(data, storageClass) { function generateUniqueBucketName(prefix, suffix = uuidv4()) { return `${prefix}-${suffix.substring(0, 8)}`.substring(0, 63); } +const describeIfLocationAws = hasLocation(awsLocation) ? describe : describe.skip; +const itIfLocationAwsSkipCeph = hasLocation(awsLocation) ? itSkipCeph : it.skip; +const itIfLocationAws = hasLocation(awsLocation) ? it : it.skip; +const itIfLocationAzure = hasLocation(azureLocation) ? it : it.skip; // FIXME: does not pass for Ceph, see CLDSRV-443 describeSkipIfNotMultipleOrCeph('backbeat DELETE routes', () => { @@ -1657,7 +1660,7 @@ describe('backbeat routes', () => { }); }); - itSkipCeph('should PUT tags for a non-versioned bucket', function test(done) { + itIfLocationAwsSkipCeph('should PUT tags for a non-versioned bucket (awslocation)', function test(done) { this.timeout(10000); const bucket = NONVERSIONED_BUCKET; const awsKey = uuidv4(); @@ -2364,7 +2367,8 @@ describe('backbeat routes', () => { }); }); }); - describe('backbeat multipart upload operations', function test() { + + describeIfLocationAws('backbeat multipart upload operations (external location)', function test() { this.timeout(10000); // The ceph image does not support putting tags during initiate MPU. @@ -2575,12 +2579,13 @@ describe('backbeat routes', () => { }, err => { // should error out as location shall no longer exist assert(err); + assert.strictEqual(err.statusCode, 503); done(); }), ], done); }); - itSkipCeph('should batch delete a versioned AWS location', done => { + itIfLocationAwsSkipCeph('should batch delete a versioned AWS location', done => { let versionId; const awsKey = `${TEST_BUCKET}/batch-delete-test-key-${makeid(8)}`; @@ -2677,7 +2682,7 @@ describe('backbeat routes', () => { ], done); }); - it('should not put delete tags if the source is not Azure and ' + + itIfLocationAws('should not put delete tags if the source is not Azure and ' + 'if-unmodified-since header is not provided', done => { const awsKey = uuidv4(); async.series([ @@ -2720,7 +2725,7 @@ describe('backbeat routes', () => { ], done); }); - itSkipCeph('should not put tags if the source is not Azure and ' + + itIfLocationAwsSkipCeph('should not put tags if the source is not Azure and ' + 'if-unmodified-since condition is not met', done => { const awsKey = uuidv4(); async.series([ @@ -2765,7 +2770,7 @@ describe('backbeat routes', () => { ], done); }); - itSkipCeph('should put tags if the source is not Azure and ' + + itIfLocationAwsSkipCeph('should put tags if the source is not Azure and ' + 'if-unmodified-since condition is met', done => { const awsKey = uuidv4(); let lastModified; @@ -2835,7 +2840,7 @@ describe('backbeat routes', () => { ], done); }); - it('should not delete the object if the source is Azure and ' + + itIfLocationAzure('should not delete the object if the source is Azure and ' + 'if-unmodified-since condition is not met', done => { const blob = uuidv4(); async.series([ @@ -2881,7 +2886,7 @@ describe('backbeat routes', () => { ], done); }); - it('should delete the object if the source is Azure and ' + + itIfLocationAzure('should delete the object if the source is Azure and ' + 'if-unmodified-since condition is met', done => { const blob = uuidv4(); let lastModified; From fdacd95f2f97b42da77212a53004271ae5ed5c9b Mon Sep 17 00:00:00 2001 From: Mickael Bourgois Date: Wed, 27 Aug 2025 12:15:48 +0200 Subject: [PATCH 8/9] CLDSRV-717: Reinclude S3C specifics from 7.70 --- tests/multipleBackend/routes/routeBackbeat.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/tests/multipleBackend/routes/routeBackbeat.js b/tests/multipleBackend/routes/routeBackbeat.js index 4375985389..74b3956c47 100644 --- a/tests/multipleBackend/routes/routeBackbeat.js +++ b/tests/multipleBackend/routes/routeBackbeat.js @@ -84,6 +84,11 @@ const testMd = { }, }; +// S3_TESTVAL_OWNERCANONICALID variable is used by Integration that runs E2E tests with real Vault account. +if (process.env.S3_TESTVAL_OWNERCANONICALID) { + testMd['owner-id'] = process.env.S3_TESTVAL_OWNERCANONICALID; +} + const nonVersionedTestMd = { 'owner-display-name': 'Bart', 'owner-id': ('79a59df900b949e55d96a1e698fbaced' + @@ -168,6 +173,7 @@ const describeIfLocationAws = hasLocation(awsLocation) ? describe : describe.ski const itIfLocationAwsSkipCeph = hasLocation(awsLocation) ? itSkipCeph : it.skip; const itIfLocationAws = hasLocation(awsLocation) ? it : it.skip; const itIfLocationAzure = hasLocation(azureLocation) ? it : it.skip; +const itSkipS3C = process.env.S3_END_TO_END ? it.skip : it; // FIXME: does not pass for Ceph, see CLDSRV-443 describeSkipIfNotMultipleOrCeph('backbeat DELETE routes', () => { @@ -2647,7 +2653,8 @@ describe('backbeat routes', () => { done(); }); }); - it('should skip batch delete of a non-existent location', done => { + // TODO: unskip test when S3C-9123 is fixed + itSkipS3C('should skip batch delete of a non-existent location', done => { async.series([ done => { const options = { From 4e4f7ffde042f3003ebe50cbea4d6d74744ee035 Mon Sep 17 00:00:00 2001 From: Mickael Bourgois Date: Wed, 27 Aug 2025 12:16:31 +0200 Subject: [PATCH 9/9] CLDSRV-717: Skip S3C broken null version + delete --- tests/multipleBackend/routes/routeBackbeat.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/multipleBackend/routes/routeBackbeat.js b/tests/multipleBackend/routes/routeBackbeat.js index 74b3956c47..5655453b2a 100644 --- a/tests/multipleBackend/routes/routeBackbeat.js +++ b/tests/multipleBackend/routes/routeBackbeat.js @@ -775,7 +775,8 @@ describe('backbeat routes', () => { }); }); - it('should create a new null version if versioning suspended and delete marker null version', done => { + // TODO fix broken on S3C with metadata backend,create 2 null Versions + itSkipS3C('should create a new null version if versioning suspended and delete marker null version', done => { let objMD; return async.series([ next => s3.putBucketVersioning({ Bucket: bucket, VersioningConfiguration: { Status: 'Suspended' } },