post reviews fixups

Author: benzekrimaha

tests/functional/sse-kms-migration/arnPrefix.js

@@ -602,12 +602,12 @@ describe('KMS error', () => {
         },
         {
             action: 'getObject', kmsAction: 'Decrypt',
-            fct: async () => helpers.s3.getObject({ Bucket, Key }) ,
+            fct: async () => helpers.s3.getObject({ Bucket, Key }),
         },
         {
             action: 'copyObject', detail: ' when getting from source', kmsAction: 'Decrypt',
             fct: async () =>
-                helpers.s3.copyObject({ Bucket, Key: 'copy', CopySource: `${Bucket}/${Key}` }) ,
+                helpers.s3.copyObject({ Bucket, Key: 'copy', CopySource: `${Bucket}/${Key}` }),
         },
         {
             action: 'copyObject', detail: ' when putting to destination', kmsAction: 'Encrypt',
@@ -617,7 +617,7 @@ describe('KMS error', () => {
                 CopySource: `${Bucket}/plaintext`,
                 ServerSideEncryption: 'aws:kms',
                 SSEKMSKeyId: masterKeyArn,
-            }) ,
+            }),
         },
         {
             action: 'createMPU', kmsAction: 'Encrypt',
@@ -632,7 +632,7 @@ describe('KMS error', () => {
                 Key: 'mpuPlaintext',
                 PartNumber: 1,
                 CopySource: `${Bucket}/${Key}`,
-            }) ,
+            }),
         },
         {
             action: 'mpu uploadPart', detail: ' when putting to destination', kmsAction: 'Encrypt',

tests/functional/sse-kms-migration/beforeMigration.js

@@ -326,8 +326,7 @@ describe('SSE KMS before migration', () => {
             it(`should PutObject versioned with SSE ${obj.name}`, async () => {
                 // ensure versioned bucket is empty
                 await helpers.bucketUtil.empty(bkt.vname);
-                let { Versions } = await helpers.s3.listObjectVersions({ Bucket: bkt.vname }) || [];
-                // regularly count versioned objects
+                let { Versions } = await helpers.s3.listObjectVersions({ Bucket: bkt.vname }) || { Versions: [] };
                 assert.strictEqual(Versions.length, 0);
 
                 const bodyBase = `BODY(${obj.name})-base`;

tests/functional/sse-kms-migration/helpers.js

@@ -20,7 +20,6 @@ const { S3Client,
     HeadBucketCommand,
 } = require('@aws-sdk/client-s3');
 const { NodeHttpHandler } = require('@smithy/node-http-handler');
-const { StandardRetryStrategy } = require('@aws-sdk/middleware-retry');
 const { Agent: HttpAgent } = require('http');
 const { Agent: HttpsAgent } = require('https');
 const kms = require('../../../lib/kms/wrapper');
@@ -40,7 +39,6 @@ function getKey(key) {
 // For Integration use default profile, in cloudserver use vault profile
 const credsProfile = process.env.S3_END_TO_END === 'true' ? 'default' : 'vault';
 
-// Create custom agents with specific pooling settings
 const httpAgent = new HttpAgent({
     keepAlive: true,
     keepAliveMsecs: 30000,
@@ -65,36 +63,21 @@ const s3config = {
         httpAgent,
         httpsAgent,
     }),
-    maxAttempts: 8,
-    retryStrategy: new StandardRetryStrategy({
-        maxAttempts: 8,
-        retryDecider: error => 
-             (
-                error.code === 'ECONNREFUSED' ||
-                error.code === 'ECONNRESET' ||
-                error.name === 'TimeoutError' ||
-                error.message?.includes('socket hang up') ||
-                error.code === 'ThrottlingException' ||
-                error.code === 'RequestTimeout'
-            )
-        ,
-        delayDecider: attempts => Math.min(1000 * Math.pow(2, attempts), 30000), // Exponential backoff
-    }),
+    maxAttempts: 3,
 };
 
 const s3Client = new S3Client(s3config);
 
-// Remove logger middleware if present
 if (s3Client.middlewareStack.identify().includes('loggerMiddleware')) {
     s3Client.middlewareStack.remove('loggerMiddleware');
 }
 
 const bucketUtil = new BucketUtility(credsProfile);
 
-// Wrapper for SDK v3 commands to return promises directly
 const wrap = exec => exec();
 const s3 = {
     createBucket: params => wrap(() => s3Client.send(new CreateBucketCommand(params))),
+    deleteBucket: params => wrap(() => s3Client.send(new DeleteBucketCommand(params))),
     putBucketEncryption: params => wrap(() => s3Client.send(new PutBucketEncryptionCommand(params))),
     getBucketEncryption: params => wrap(() => s3Client.send(new GetBucketEncryptionCommand(params))),
     putObject: params => wrap(() => s3Client.send(new PutObjectCommand(params))),
@@ -176,7 +159,7 @@ const MD = {
 
 async function getBucketSSE(Bucket) {
     try {
-        const sse = await s3Client.send(new GetBucketEncryptionCommand({ Bucket }));
+        const sse = await s3.getBucketEncryption({ Bucket });
         return sse.ServerSideEncryptionConfiguration.Rules[0].ApplyServerSideEncryptionByDefault;
     } catch (error) {
         if (error.name === 'ServerSideEncryptionConfigurationNotFoundError') {
@@ -187,10 +170,10 @@ async function getBucketSSE(Bucket) {
 }
 
 async function putEncryptedObject(Bucket, Key, sseConfig, kmsKeyId, Body) {
-    return s3Client.send(new PutObjectCommand({
+    return s3.putObject({
         ...putObjParams(Bucket, Key, sseConfig, kmsKeyId),
         Body,
-    }));
+    });
 }
 
 async function getObjectMDSSE(Bucket, Key) {
@@ -221,7 +204,7 @@ const destroyKmsKey = promisify(kms.destroyBucketKey);
 
 async function cleanup(Bucket) {
     await bucketUtil.empty(Bucket);
-    await s3Client.send(new DeleteBucketCommand({ Bucket }));
+    await s3.deleteBucket({ Bucket });
 }
 
 module.exports = {

tests/functional/sse-kms-migration/scenarios.js

@@ -235,10 +235,6 @@ async function copyObjectAndSSE(
 // check MPU headers against the MPU overview MD
 // because there is no migration for ongoing MPU
 function assertMPUSSEHeaders(actual, expected, algo) {
-    // eslint-disable-next-line no-console
-    console.log('actual', actual);
-    // eslint-disable-next-line no-console
-    console.log('expected', expected);
     if (algo) {
         assert.strictEqual(actual.ServerSideEncryption, algo);
     }
@@ -284,16 +280,9 @@ async function mpuUploadPartCopy(
 // before has no headers to assert
 async function mpuComplete({ UploadId, Bucket, Key }, { existingParts, newParts }, mpuOverviewMDSSE, algo, testCase) {
     const extractETag = part => {
-        // For uploadPartCopy responses, ETag is in CopyPartResult
-        if (part.CopyPartResult && part.CopyPartResult.ETag) {
-            return part.CopyPartResult.ETag;
-        }
-        // For regular uploadPart responses, ETag is at the top level
-        if (part.ETag) {
-            return part.ETag;
-        }
-        // If neither exists, throw an error for debugging
-        throw new Error(`Could not find ETag in part: ${JSON.stringify(part)}`);
+        const eTag = part.CopyPartResult?.ETag || part.ETag;
+        assert(eTag, `Could not find ETag in part: ${JSON.stringify(part)}`);
+        return eTag;
     };
 
     // Build the parts array with proper ETag extraction