From 2f3c20def6870faaede4ecadf6fcddff297c522f Mon Sep 17 00:00:00 2001 From: Maha Benzekri Date: Fri, 7 Feb 2025 11:45:46 +0100 Subject: [PATCH] coverage improvement and post reviews fixups --- README.md | 5 ++- eslint.config.mjs | 1 + lib/auth/Vault.ts | 5 ++- lib/auth/v4/awsURIencode.ts | 2 +- lib/network/kmsAWS/Client.ts | 12 +++---- .../mongoclient/MongoClientInterface.ts | 34 +++++++++---------- .../policyEvaluator/RequestContext.spec.js | 22 ++++++++++++ tests/unit/s3routes/routeGET.spec.js | 34 +++++++++++++++++++ 8 files changed, 87 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 69df7b038..77bd3c478 100644 --- a/README.md +++ b/README.md @@ -1,5 +1,8 @@ # Arsenal +[![Public Build Status](https://github.com/scality/Arsenal/actions/workflows/tests.yaml/badge.svg)](https://github.com/scality/Arsenal/actions/workflows/tests.yaml) +[![codecov](https://codecov.io/gh/scality/Arsenal/branch/development/8.1/graph/badge.svg)](https://codecov.io/gh/scality/Arsenal) + Common utilities for the S3 project components Within this repository, you will be able to find the shared libraries for the @@ -153,4 +156,4 @@ to the process (which results in an immediate termination, and this signal can't be caught). [badgepub]: https://github.com/scality/Arsenal/actions/workflows/tests.yaml/badge.svg -[codecov]: https://codecov.io/gh/scality/Arsenal/branch/development/8.1/graph/badge.svg?token=X0esXhJSwb +[codecov]: https://codecov.io/gh/scality/Arsenal/branch/development/8.1/graph/badge.svg diff --git a/eslint.config.mjs b/eslint.config.mjs index ccb465a51..265012537 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -28,6 +28,7 @@ export default tseslint.config( 'camelcase': 'off', 'no-param-reassign': 'off', 'new-cap': 'off', + 'quotes': 'off', '@typescript-eslint/no-unsafe-function-type':'off' } } diff --git a/lib/auth/Vault.ts b/lib/auth/Vault.ts index 34461e256..58483c3eb 100644 --- a/lib/auth/Vault.ts +++ b/lib/auth/Vault.ts @@ -37,9 +37,8 @@ function vaultSignatureCb( { errorMessage: err }); return callback(err); } - - const { ...userInfoWithoutEmail } = authInfo.message.body.userInfo; - + // eslint-disable-next-line @typescript-eslint/no-unused-vars + const { email: _, ...userInfoWithoutEmail } = authInfo.message.body.userInfo; log.debug('received info from Vault', { ...authInfo, message: { diff --git a/lib/auth/v4/awsURIencode.ts b/lib/auth/v4/awsURIencode.ts index e53bcd4f9..6c6795225 100644 --- a/lib/auth/v4/awsURIencode.ts +++ b/lib/auth/v4/awsURIencode.ts @@ -42,7 +42,7 @@ export default function awsURIencode( if (typeof input !== 'string') { return ''; } - let encoded = ''; + let encoded = ""; const slash = encodeSlash === undefined || encodeSlash ? '%2F' : '/'; const star = noEncodeStar !== undefined && noEncodeStar ? '*' : '%2A'; for (let i = 0; i < input.length; i++) { diff --git a/lib/network/kmsAWS/Client.ts b/lib/network/kmsAWS/Client.ts index bc2374ed9..2ecf3926b 100644 --- a/lib/network/kmsAWS/Client.ts +++ b/lib/network/kmsAWS/Client.ts @@ -175,8 +175,8 @@ export default class Client { } if (!data) { - const error = arsenalErrorAWSKMS('failed to generate data key: empty response'); - logger.error('AWS KMS: failed to generate data key: empty response'); + const error = arsenalErrorAWSKMS("failed to generate data key: empty response"); + logger.error("AWS KMS: failed to generate data key: empty response"); cb(error); return; } @@ -212,8 +212,8 @@ export default class Client { } if (!data) { - const error = arsenalErrorAWSKMS('failed to cipher data key: empty response'); - logger.error('AWS KMS: failed to cipher data key: empty response'); + const error = arsenalErrorAWSKMS("failed to cipher data key: empty response"); + logger.error("AWS KMS: failed to cipher data key: empty response"); cb(error); return; } @@ -247,8 +247,8 @@ export default class Client { } if (!data) { - const error = arsenalErrorAWSKMS('failed to decipher data key: empty response'); - logger.error('AWS KMS: failed to decipher data key: empty response'); + const error = arsenalErrorAWSKMS("failed to decipher data key: empty response"); + logger.error("AWS KMS: failed to decipher data key: empty response"); cb(error); return; } diff --git a/lib/storage/metadata/mongoclient/MongoClientInterface.ts b/lib/storage/metadata/mongoclient/MongoClientInterface.ts index bd74d28b1..23eded9f8 100644 --- a/lib/storage/metadata/mongoclient/MongoClientInterface.ts +++ b/lib/storage/metadata/mongoclient/MongoClientInterface.ts @@ -1686,7 +1686,6 @@ class MongoClientInterface { // a master or re-delete it in between so place an // atomic condition on the PHD flag and the mst // version: - const filter = { 'value.isPHD': true, 'value.versionId': mst.versionId, @@ -2587,7 +2586,8 @@ class MongoClientInterface { bucketCount, bucketInfos, }); - })).catch(err => { + }) + ).catch(err => { log.error('could not get list of collections', { method: 'getBucketInfos', error: err, @@ -2646,7 +2646,6 @@ class MongoClientInterface { } consolidateData(store, dataManaged) { - if (dataManaged && dataManaged.locations && dataManaged.total) { const locations = dataManaged.locations; store.dataManaged.total.curr += dataManaged.total.curr; @@ -2976,21 +2975,22 @@ class MongoClientInterface { collRes[targetData][site] = data[site]; } }); - }).then(() => { - const bucketStatus = bucketInfo.getVersioningConfiguration(); - const isVer = (bucketStatus && - (bucketStatus.Status === 'Enabled' || - bucketStatus.Status === 'Suspended')); - const retResult = this._handleResults(collRes, isVer); - retResult.stalled = stalledCount; - return callback(null, retResult); - }).catch(err => { - log.error('Error when processing mongo entries', { - method: 'getObjectMDStats', - error: err, + }) + .then(() => { + const bucketStatus = bucketInfo.getVersioningConfiguration(); + const isVer = (bucketStatus && + (bucketStatus.Status === 'Enabled' || + bucketStatus.Status === 'Suspended')); + const retResult = this._handleResults(collRes, isVer); + retResult.stalled = stalledCount; + return callback(null, retResult); + }).catch(err => { + log.error('Error when processing mongo entries', { + method: 'getObjectMDStats', + error: err, + }); + return callback(err); }); - return callback(err); - }); } getIngestionBuckets(log: werelogs.Logger, cb: ArsenalCallback) { diff --git a/tests/unit/policyEvaluator/RequestContext.spec.js b/tests/unit/policyEvaluator/RequestContext.spec.js index 0cc368484..96825bc54 100644 --- a/tests/unit/policyEvaluator/RequestContext.spec.js +++ b/tests/unit/policyEvaluator/RequestContext.spec.js @@ -172,4 +172,26 @@ describe('RequestContext', () => { const ssoRC = new RequestContext(...ssoParams); assert.strictEqual(ssoRC.getResource(), 'arn:scality:sso:::general-resource/specific-resource'); }); + + it('should return correct ARN for s3 service without specific resource', () => { + const s3Params = [...constructorParams]; + s3Params[3] = undefined; // specificResource + const s3RC = new RequestContext(...s3Params); + assert.strictEqual(s3RC.getResource(), 'arn:aws:s3:::general-resource'); + }); + + it('should return correct ARN for s3 service without general and specific resource', () => { + const s3Params = [...constructorParams]; + s3Params[2] = undefined; // generalResource + s3Params[3] = undefined; // specificResource + const s3RC = new RequestContext(...s3Params); + assert.strictEqual(s3RC.getResource(), 'arn:aws:s3:::'); + }); + + it('should return undefined for unknown service', () => { + const unknownParams = [...constructorParams]; + unknownParams[7] = 'unknown'; + const unknownRC = new RequestContext(...unknownParams); + assert.strictEqual(unknownRC.getResource(), undefined); + }); }); \ No newline at end of file diff --git a/tests/unit/s3routes/routeGET.spec.js b/tests/unit/s3routes/routeGET.spec.js index 5bf1afd56..a20e1521a 100644 --- a/tests/unit/s3routes/routeGET.spec.js +++ b/tests/unit/s3routes/routeGET.spec.js @@ -128,4 +128,38 @@ describe('routerGET', () => { new ArsenalError('InternalError'), statsClient, ); }); + + it('should call the appropriate bucket query methods based on query parameters', () => { + const queryMethods = { + acl: 'bucketGetACL', + replication: 'bucketGetReplication', + cors: 'bucketGetCors', + versioning: 'bucketGetVersioning', + website: 'bucketGetWebsite', + tagging: 'bucketGetTagging', + lifecycle: 'bucketGetLifecycle', + uploads: 'listMultipartUploads', + location: 'bucketGetLocation', + policy: 'bucketGetPolicy', + 'object-lock': 'bucketGetObjectLock', + notification: 'bucketGetNotification', + encryption: 'bucketGetEncryption', + search: 'metadataSearch', + }; + + Object.keys(queryMethods).forEach(queryParam => { + request.bucketName = 'bucketName'; + request.objectKey = undefined; + request.query = { [queryParam]: true }; + + routerGET(request, response, api, log, statsClient, dataRetrievalParams); + + expect(api.callApiMethod).toHaveBeenCalledWith( + queryMethods[queryParam], request, response, log, expect.any(Function), + ); + + api.callApiMethod.mockClear(); + }); + }); + });