fix(GCP): Increase KMS call retries to 10

And increase timeout to 3 seconds. A continuation of #133 because we hit the same issue again, although it's now very rare.
relaycorp · Feb 23, 2023 · ea58fe9 · ea58fe9
1 parent b3df326
commit ea58fe9
Show file tree

Hide file tree

Showing 4 changed files with 18 additions and 17 deletions.
diff --git a/package-lock.json b/package-lock.json
diff --git a/src/lib/gcp/GCPPrivateKeyStore.spec.ts b/src/lib/gcp/GCPPrivateKeyStore.spec.ts
@@ -154,27 +154,27 @@ describe('Identity keys', () => {
         );
       });
 
-      test('Version creation call should time out after 2 seconds', async () => {
+      test('Version creation call should time out after 3 seconds', async () => {
         const kmsClient = makeKmsClient();
         const store = new GCPPrivateKeyStore(kmsClient, getDBConnection(), KMS_CONFIG);
 
         await store.generateIdentityKeyPair();
 
         expect(kmsClient.createCryptoKeyVersion).toHaveBeenCalledWith(
           expect.anything(),
-          expect.objectContaining({ timeout: 2_000 }),
+          expect.objectContaining({ timeout: 3_000 }),
         );
       });
 
-      test('Version creation call should be retried up to 8 times', async () => {
+      test('Version creation call should be retried', async () => {
         const kmsClient = makeKmsClient();
         const store = new GCPPrivateKeyStore(kmsClient, getDBConnection(), KMS_CONFIG);
 
         await store.generateIdentityKeyPair();
 
         expect(kmsClient.createCryptoKeyVersion).toHaveBeenCalledWith(
           expect.anything(),
-          expect.objectContaining({ maxRetries: 8 }),
+          expect.objectContaining({ maxRetries: 10 }),
         );
       });
 
@@ -682,7 +682,7 @@ describe('Session keys', () => {
         );
       });
 
-      test('Request should time out after 2 seconds', async () => {
+      test('Request should time out after 3 seconds', async () => {
         const kmsClient = makeKMSClient();
         const store = new GCPPrivateKeyStore(kmsClient, getDBConnection(), KMS_CONFIG);
 
@@ -694,11 +694,11 @@ describe('Session keys', () => {
 
         expect(kmsClient.encrypt).toHaveBeenCalledWith(
           expect.anything(),
-          expect.objectContaining({ timeout: 2_000 }),
+          expect.objectContaining({ timeout: 3_000 }),
         );
       });
 
-      test('Request should be retried up to 8 times', async () => {
+      test('Request should be retried', async () => {
         const kmsClient = makeKMSClient();
         const store = new GCPPrivateKeyStore(kmsClient, getDBConnection(), KMS_CONFIG);
 
@@ -710,7 +710,7 @@ describe('Session keys', () => {
 
         expect(kmsClient.encrypt).toHaveBeenCalledWith(
           expect.anything(),
-          expect.objectContaining({ maxRetries: 8 }),
+          expect.objectContaining({ maxRetries: 10 }),
         );
       });
 
@@ -918,7 +918,7 @@ describe('Session keys', () => {
         );
       });
 
-      test('Request should time out after 2 seconds', async () => {
+      test('Request should time out after 3 seconds', async () => {
         const kmsClient = makeKMSClient();
         const store = new GCPPrivateKeyStore(kmsClient, getDBConnection(), KMS_CONFIG);
         await saveKey();
@@ -927,11 +927,11 @@ describe('Session keys', () => {
 
         expect(kmsClient.decrypt).toHaveBeenCalledWith(
           expect.anything(),
-          expect.objectContaining({ timeout: 2_000 }),
+          expect.objectContaining({ timeout: 3_000 }),
         );
       });
 
-      test('Request should be retried up to 8 times', async () => {
+      test('Request should be retried', async () => {
         const kmsClient = makeKMSClient();
         const store = new GCPPrivateKeyStore(kmsClient, getDBConnection(), KMS_CONFIG);
         await saveKey();
@@ -940,7 +940,7 @@ describe('Session keys', () => {
 
         expect(kmsClient.decrypt).toHaveBeenCalledWith(
           expect.anything(),
-          expect.objectContaining({ maxRetries: 8 }),
+          expect.objectContaining({ maxRetries: 10 }),
         );
       });
 

diff --git a/src/lib/gcp/GcpKmsRsaPssProvider.spec.ts b/src/lib/gcp/GcpKmsRsaPssProvider.spec.ts
@@ -143,27 +143,27 @@ describe('onSign', () => {
     );
   });
 
-  test('Request should time out after 2 seconds', async () => {
+  test('Request should time out after 3 seconds', async () => {
     const kmsClient = makeKmsClient();
     const provider = new GcpKmsRsaPssProvider(kmsClient);
 
     await provider.sign(ALGORITHM, privateKey, PLAINTEXT);
 
     expect(kmsClient.asymmetricSign).toHaveBeenCalledWith(
       expect.anything(),
-      expect.objectContaining({ timeout: 2_000 }),
+      expect.objectContaining({ timeout: 3_000 }),
     );
   });
 
-  test('Request should be retried up to 8 times', async () => {
+  test('Request should be retried', async () => {
     const kmsClient = makeKmsClient();
     const provider = new GcpKmsRsaPssProvider(kmsClient);
 
     await provider.sign(ALGORITHM, privateKey, PLAINTEXT);
 
     expect(kmsClient.asymmetricSign).toHaveBeenCalledWith(
       expect.anything(),
-      expect.objectContaining({ maxRetries: 8 }),
+      expect.objectContaining({ maxRetries: 10 }),
     );
   });
 

diff --git a/src/lib/gcp/kmsUtils.ts b/src/lib/gcp/kmsUtils.ts
@@ -10,7 +10,7 @@ import { wrapGCPCallError } from './gcpUtils';
  * maximum number of retries before any response was received". We're working around that by
  * retrying a few times.
  */
-export const KMS_REQUEST_OPTIONS = { timeout: 2_000, maxRetries: 8 };
+export const KMS_REQUEST_OPTIONS = { timeout: 3_000, maxRetries: 10 };
 
 export async function retrieveKMSPublicKey(
   kmsKeyVersionName: string,