Merge branch 'improvement/CLDSRV-717-unify-test-backbeat-route' into q/9.0

Author: bert-e

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,

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",

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,
 };

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`);
+    }
 }
 
 

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;