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, 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", 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/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`); + } } diff --git a/tests/multipleBackend/routes/routeBackbeat.js b/tests/multipleBackend/routes/routeBackbeat.js index f436a513b0..5655453b2a 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); @@ -85,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' + @@ -165,6 +169,11 @@ 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; +const itSkipS3C = process.env.S3_END_TO_END ? it.skip : it; // FIXME: does not pass for Ceph, see CLDSRV-443 describeSkipIfNotMultipleOrCeph('backbeat DELETE routes', () => { @@ -280,7 +289,7 @@ describe('backbeat routes', () => { .then(() => done()) .catch(err => { process.stdout.write(`Error creating bucket: ${err}\n`); - throw err; + done(err); }); }); @@ -766,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' } }, @@ -1657,7 +1667,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(); @@ -2165,6 +2175,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 +2223,8 @@ describe('backbeat routes', () => { objectKey: TEST_KEY, resourceType: test.resourceType, queryObj, authCredentials: { - accessKey: 'accessKey2', - secretKey: 'verySecretKey2', + accessKey: accessKeyLisa, + secretKey: secretAccessKeyLisa, }, }, err => { @@ -2242,44 +2254,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', () => { @@ -2303,6 +2330,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); @@ -2320,7 +2348,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(); }); }); @@ -2335,12 +2366,16 @@ 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(); }); }); }); - 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. @@ -2551,12 +2586,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)}`; @@ -2618,7 +2654,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 = { @@ -2653,7 +2690,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([ @@ -2696,7 +2733,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([ @@ -2741,7 +2778,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; @@ -2811,7 +2848,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([ @@ -2857,7 +2894,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;