adding tests for externalclients and gcpservice

lib/auth/v4/streamingV4/constructChunkStringToSign.ts

@@ -24,8 +24,12 @@ export default function constructChunkStringToSign(
         currentChunkHash = constants.emptyStringHash;
     } else {
         const hash = crypto.createHash('sha256');
-        const temp = hash.update(justDataChunk);
-        currentChunkHash = temp.digest('hex');
+        if (typeof justDataChunk === 'string') {
+            hash.update(justDataChunk, 'binary');
+        } else {
+            hash.update(justDataChunk);
+        }
+        currentChunkHash = hash.digest('hex');
     }
     return `AWS4-HMAC-SHA256-PAYLOAD\n${timestamp}\n` +
         `${credentialScope}\n${lastSignature}\n` +

lib/network/kmsAWS/Client.ts

@@ -60,7 +60,7 @@ export default class Client implements KMSInterface {
 
     constructor(options: ClientOptions) {
         this._supportsDefaultKeyPerAccount = true;
-        const { providerName,tls, ak, sk, region, endpoint, noAwsArn } = options.kmsAWS;
+        const { providerName, tls, ak, sk, region, endpoint, noAwsArn } = options.kmsAWS;
 
         const requestHandler = new NodeHttpHandler({
             httpAgent: !tls ? new HttpAgent({
@@ -134,6 +134,10 @@ export default class Client implements KMSInterface {
                 // Prefer ARN, but fall back to KeyId if ARN is missing
                 keyId = keyMetadata?.Arn ?? (keyMetadata?.KeyId || '');
             }
+            // May produce double arn prefix: scality arn + aws arn
+            // arn:scality:kms:external:aws_kms:custom:key/arn:aws:kms:region:accountId:key/cbd69d33-ba8e-4b56-8cfe
+            // If this is a problem, a config flag should be used to hide the scality arn when returning the KMS KeyId
+            // or aws arn when creating the KMS Key
             const arn = `${this.backend.arnPrefix}${keyId}`;
             cb(null, keyId, arn);
         }).catch(err => {

lib/network/utils.ts

@@ -1,5 +1,7 @@
 import { ArsenalError, errorInstances } from '../errors';
 import { allowedKmsErrors } from '../errors/kmsErrors';
+import { S3ServiceException } from '@aws-sdk/client-s3';
+import { KMSServiceException } from '@aws-sdk/client-kms';
 
 /**
  * Normalize errors according to arsenal definitions with a custom prefix
@@ -31,28 +33,14 @@ export function arsenalErrorKMIP(err: string | Error) {
 
 const allowedKmsErrorCodes = Object.keys(allowedKmsErrors) as unknown as (keyof typeof allowedKmsErrors)[];
 
-// Local AWSError type for compatibility with v3 error handling
-export type AWSError = Error & {
-    name?: string;
-    $fault?: 'client' | 'server';
-    $metadata?: {
-        httpStatusCode?: number;
-        requestId?: string;
-        attempts?: number;
-        totalRetryDelay?: number;
-    };
-    $retryable?: {
-        throttling?: boolean;
-    };
-    message?: string;
-};
-
-function isAWSError(err: string | Error | AWSError): err is AWSError {
-    return (err as AWSError).name !== undefined
-        && (err as AWSError).$metadata !== undefined;
+function isAWSError(err: unknown): 
+    err is S3ServiceException | KMSServiceException | (Error & { name?: string }) {
+    return (err instanceof S3ServiceException || err instanceof KMSServiceException ||
+        (err instanceof Error && typeof err.name === 'string')
+    );
 }
 
-export function arsenalErrorAWSKMS(err: string | Error | AWSError) {
+export function arsenalErrorAWSKMS(err: string | Error | S3ServiceException) {
     if (isAWSError(err)) {
         const errorCode = err.name;
         if (allowedKmsErrorCodes.includes(errorCode as keyof typeof allowedKmsErrors)) {

lib/storage/data/external/GCP/GcpService.js

@@ -546,7 +546,7 @@ class GcpClient extends S3Client {
             delete deleteParams.VersionId;
         }
         return this.send(new DeleteObjectCommand(deleteParams))
-            .then(data => callback && callback(null, data))
+            .then(data => callback?.(null, data))
             .catch(err => callback?.(err));
     }
 

lib/storage/data/external/GcpClient.js

@@ -307,37 +307,37 @@ class GcpClient extends AwsClient {
 
     listObjects(params, callback) {
         return this.send(new ListObjectsCommand(params))
-            .then(data => callback && callback(null, data))
+            .then(data => callback?.(null, data))
             .catch(err => callback?.(err));
     }
 
     putObject(params, callback) {
         return this.send(new PutObjectCommand(params))
-            .then(data => callback && callback(null, data))
+            .then(data => callback?.(null, data))
             .catch(err => callback?.(err));
     }
 
     getObject(params, callback) {
         return this.send(new GetObjectCommand(params))
-            .then(data => callback && callback(null, data))
+            .then(data => callback?.(null, data))
             .catch(err => callback?.(err));
     }
 
     deleteObject(params, callback) {
         return this.send(new DeleteObjectCommand(params))
-            .then(data => callback && callback(null, data))
+            .then(data => callback?.(null, data))
             .catch(err => callback?.(err));
     }
 
     copyObject(params, callback) {
         return this.send(new CopyObjectCommand(params))
-            .then(data => callback && callback(null, data))
+            .then(data => callback?.(null, data))
             .catch(err => callback?.(err));
     }
 
     headObject(params, callback) {
         return this.send(new HeadObjectCommand(params))
-            .then(data => callback && callback(null, data))
+            .then(data => callback?.(null, data))
             .catch(err => callback?.(err));
     }
 }

tests/unit/storage/data/DummyService.js

@@ -88,6 +88,13 @@ class AzureDummyContainerClient {
             readableStreamBody: new DummyObjectStream(offset, length || OBJECT_SIZE),
         };
     }
+
+    async deleteIfExists() {
+        if (this.key === 'externalBackendTestBucket/externalBackendMissingKey') {
+            return { succeeded: false };
+        }
+        return { succeeded: true };
+    }
 }
 
 class DummyService {

tests/unit/storage/data/external/ExternalClients.spec.js

@@ -1,6 +1,6 @@
 const assert = require('assert');
-const async = require('async');
 const stream = require('stream');
+const sinon = require('sinon');
 const { promisify } = require('util');
 
 const AwsClient = require('../../../../../lib/storage/data/external/AwsClient');
@@ -50,11 +50,21 @@ const backendClients = [
     },
 ];
 const log = new DummyRequestLogger();
+let sandbox;
 
 describe('external backend clients', () => {
+    beforeEach(() => {
+        sandbox = sinon.createSandbox();
+    });
+
+    afterEach(() => {
+        sandbox.restore();
+    });
+
     backendClients.forEach(backend => {
         let testClient;
-        let headAsync, getAsync, objectPutTaggingAsync, objectDeleteTaggingAsync;
+        let headAsync, getAsync, deleteAsync, objectPutTaggingAsync, objectDeleteTaggingAsync,
+            createMPUAsync, uploadPartAsync, abortMPUAsync, listPartsAsync;
 
         beforeAll(() => {
             testClient = new backend.Class(backend.config);
@@ -63,10 +73,17 @@ describe('external backend clients', () => {
             // Promisify the client methods
             headAsync = promisify(testClient.head.bind(testClient));
             getAsync = promisify(testClient.get.bind(testClient));
+            deleteAsync = promisify(testClient.delete.bind(testClient));
             if (backend.config.type !== 'azure') {
+                createMPUAsync = promisify(testClient.createMPU.bind(testClient));
+                uploadPartAsync = promisify(testClient.uploadPart.bind(testClient));
+                abortMPUAsync = promisify(testClient.abortMPU.bind(testClient));
                 objectPutTaggingAsync = promisify(testClient.objectPutTagging.bind(testClient));
                 objectDeleteTaggingAsync = promisify(testClient.objectDeleteTagging.bind(testClient));
             }
+            if (backend.config.type === 'aws') {
+                listPartsAsync = promisify(testClient.listParts.bind(testClient));
+            }
         });
 
         if (backend.config.type !== 'azure') {
@@ -92,7 +109,9 @@ describe('external backend clients', () => {
                 const uploadId = 'externalBackendTestUploadId';
                 testClient.completeMPU(jsonList, null, key,
                     uploadId, bucketName, log, (err, res) => {
-                        if (err) return done(err);
+                        if (err) {
+                            return done(err);
+                        }
                         assert.strictEqual(typeof res.key, 'string');
                         assert.strictEqual(typeof res.eTag, 'string');
                         assert.strictEqual(typeof res.dataStoreVersionId, 'string');
@@ -172,6 +191,16 @@ describe('external backend clients', () => {
             assert.strictEqual(errorHandled, true);
         });
 
+        it(`${backend.name} delete() should delete the requested key without error`, async () => {
+            const key = 'externalBackendTestKey';
+            const bucketName = 'externalBackendTestBucket';
+            const objectInfo = Object.assign({
+                deleteVersion: false,
+            }, testClient.toObjectGetInfo(key, bucketName));
+            const result = await deleteAsync(objectInfo, '');
+            assert.strictEqual(result, undefined);
+        });
+
         if (backend.config.type !== 'azure') {
             it(`${backend.name} should set tags and then delete it`, async () => {
                 const key = 'externalBackendTestKey';
@@ -226,7 +255,93 @@ describe('external backend clients', () => {
                     assert(err.is.ServiceUnavailable);
                 }
             });
+
+            it(`${backend.name} uploadPart() should return sanitized data retrieval info`, async () => {
+                const key = 'externalBackendTestKey';
+                const bucketName = 'externalBackendTestBucket';
+                const result = await uploadPartAsync(null, null,
+                    stream.Readable.from(['part data']),
+                    9, key, 'uploadId-123', 1, bucketName, log);
+
+                assert.strictEqual(result.key, `${bucketName}/${key}`);
+                assert.strictEqual(result.dataStoreName, backend.config.dataStoreName);
+                assert(result.dataStoreETag);
+                assert.strictEqual(result.dataStoreETag.includes('"'), false);
+            });
+
+            it(`${backend.name} abortMPU() should resolve without error`, async () => {
+                const key = 'externalBackendTestKey';
+                const bucketName = 'externalBackendTestBucket';
+
+                const result = await abortMPUAsync(key, 'uploadId-123', bucketName, log);
+                assert.strictEqual(result, undefined);
+            });
+
+            if (backend.config.type === 'aws') {
+                it(`${backend.name} listParts() should map result parts`, async () => {
+                    const key = 'externalBackendTestKey';
+                    const bucketName = 'externalBackendTestBucket';
+
+                    const storedParts = await listPartsAsync(key, 'uploadId-123', bucketName, 0, 1000, log);
+
+                    assert(Array.isArray(storedParts.Contents));
+                    assert(storedParts.Contents.length > 0);
+                    const firstPart = storedParts.Contents[0];
+                    assert.strictEqual(typeof firstPart.partNumber, 'number');
+                    assert(firstPart.value);
+                    assert.strictEqual(firstPart.value.ETag.includes('"'), false);
+                });
+            }
+
+            it(`${backend.name} createMPU() should trim metadata and forward tagging`, async () => {
+                const key = 'externalBackendTestKey';
+                const bucketName = 'externalBackendTestBucket';
+                const metaHeaders = {
+                    'x-amz-meta-custom-key': 'customValue',
+                    'x-amz-meta-second-key': 'secondValue',
+                    ignored: 'shouldBeDropped',
+                };
+                const args = [
+                    key,
+                    metaHeaders,
+                    bucketName,
+                    'http://redirect',
+                    'text/plain',
+                    'max-age=3600',
+                    'attachment',
+                    'gzip',
+                    'k1=v1&k2=v2',
+                    log,
+                ];
+
+                if (backend.config.type === 'aws') {
+                    const sendSpy = sandbox.spy(testClient._client, 'send');
+                    const result = await createMPUAsync(...args);
+                    assert(result);
+                    assert(result.UploadId);
+                    assert(sendSpy.calledOnce);
+                    const command = sendSpy.firstCall.args[0];
+                    assert.strictEqual(command.constructor.name, 'CreateMultipartUploadCommand');
+                    assert.deepStrictEqual(command.input.Metadata, {
+                        'custom-key': 'customValue',
+                        'second-key': 'secondValue',
+                    });
+                    assert.strictEqual(command.input.Tagging, 'k1=v1&k2=v2');
+                } else {
+                    const createSpy = sandbox.spy(testClient._client, 'createMultipartUpload');
+                    const result = await createMPUAsync(...args);
+                    assert(result);
+                    assert(result.UploadId);
+                    assert(createSpy.calledOnce);
+                    const capturedParams = createSpy.firstCall.args[0];
+                    assert.strictEqual(capturedParams.Bucket, backend.config.mpuBucket);
+                    assert.deepStrictEqual(capturedParams.Metadata, {
+                        'custom-key': 'customValue',
+                        'second-key': 'secondValue',
+                    });
+                    assert.strictEqual(capturedParams.Tagging, 'k1=v1&k2=v2');
+                }
+            });
         }
-        // To-Do: test the other external client methods (delete, createMPU ...)
     });
 });