Ensure AuthClient is cached (#373)

Closes #364 /cc @GregoireW @danielbankhead could you please test this? @verbanicm hold on merge until further testing.
google-github-actions · Aug 19, 2024 · e495015 · e495015
1 parent 355876e
commit e495015
Show file tree

Hide file tree

Showing 5 changed files with 53 additions and 15 deletions.
diff --git a/dist/index.js b/dist/index.js
diff --git a/src/client.ts b/src/client.ts
@@ -23,6 +23,7 @@ import {
   StorageOptions,
   UploadOptions,
 } from '@google-cloud/storage';
+import { GoogleAuth } from 'google-auth-library';
 import { errorMessage, toPlatformPath, toPosixPath } from '@google-github-actions/actions-utils';
 
 import { Metadata } from './headers';
@@ -159,8 +160,27 @@ export interface ClientComputeDestinationOptions {
 export class Client {
   readonly storage: Storage;
 
-  constructor(opts?: ClientOptions) {
+  static async build(opts?: ClientOptions): Promise<Client> {
+    const client = new Client(opts);
+
+    // We need to force the authClient to cache its internal client. Since all
+    // our calls are done in parallel, this has to be done as part of
+    // initialization.
+    //
+    // https://github.com/google-github-actions/upload-cloud-storage/issues/364
+    await client.storage.authClient.getClient();
+
+    return client;
+  }
+
+  private constructor(opts?: ClientOptions) {
+    const authClient = new GoogleAuth({
+      projectId: opts?.projectID,
+      universeDomain: opts?.universe,
+    });
+
     const options: StorageOptions = {
+      authClient: authClient,
       projectId: opts?.projectID,
       universeDomain: opts?.universe,
       userAgent: userAgent,

diff --git a/src/main.ts b/src/main.ts
@@ -163,7 +163,7 @@ export async function run(): Promise<void> {
 
     // Create the client and upload files.
     core.startGroup('Upload files');
-    const client = new Client({
+    const client = await Client.build({
       projectID: projectID,
       universe: universe,
     });

diff --git a/tests/client.int.test.ts b/tests/client.int.test.ts
@@ -66,7 +66,7 @@ test(
     });
 
     await suite.test('throws an error on a non-existent bucket', async () => {
-      const client = new Client({ projectID: projectID });
+      const client = await Client.build({ projectID: projectID });
       await assert.rejects(async () => {
         await client.upload({
           bucket: 'definitely-not-a-real-bucket',
@@ -76,7 +76,7 @@ test(
     });
 
     await suite.test('throws an error on a non-existent file', async () => {
-      const client = new Client({ projectID: projectID });
+      const client = await Client.build({ projectID: projectID });
       await assert.rejects(async () => {
         await client.upload({
           bucket: testBucket,
@@ -86,7 +86,7 @@ test(
     });
 
     await suite.test('uploads a single file', async () => {
-      const client = new Client({ projectID: projectID });
+      const client = await Client.build({ projectID: projectID });
       await client.upload({
         bucket: testBucket,
         files: [{ source: './tests/testdata/test1.txt', destination: 'test1.txt' }],
@@ -98,7 +98,7 @@ test(
     });
 
     await suite.test('uploads files with the correct mime type', async () => {
-      const client = new Client({ projectID: projectID });
+      const client = await Client.build({ projectID: projectID });
       await client.upload({
         bucket: testBucket,
         files: [
@@ -127,7 +127,7 @@ test(
     });
 
     await suite.test('uploads a single file with prefix', async () => {
-      const client = new Client({ projectID: projectID });
+      const client = await Client.build({ projectID: projectID });
       await client.upload({
         bucket: testBucket,
         files: [{ source: './tests/testdata/test1.txt', destination: 'my/prefix/test1.txt' }],
@@ -139,7 +139,7 @@ test(
     });
 
     await suite.test('uploads a single file without an extension', async () => {
-      const client = new Client({ projectID: projectID });
+      const client = await Client.build({ projectID: projectID });
       await client.upload({
         bucket: testBucket,
         files: [{ source: './tests/testdata/testfile', destination: 'testfile' }],
@@ -154,7 +154,7 @@ test(
       'uploads a file with unicode characters in the filename',
       { skip: process.platform === 'win32' },
       async () => {
-        const client = new Client({ projectID: projectID });
+        const client = await Client.build({ projectID: projectID });
         await client.upload({
           bucket: testBucket,
           files: [{ source: './tests/testdata-unicode/🚀', destination: '🚀' }],
@@ -167,7 +167,7 @@ test(
     );
 
     await suite.test('uploads a single file with metadata', async () => {
-      const client = new Client({ projectID: projectID });
+      const client = await Client.build({ projectID: projectID });
       await client.upload({
         bucket: testBucket,
         files: [{ source: './tests/testdata/test1.txt', destination: 'test1.txt' }],

diff --git a/tests/client.test.ts b/tests/client.test.ts
@@ -19,17 +19,35 @@ import assert from 'node:assert';
 
 import * as path from 'path';
 
+import { forceRemove, randomFilepath, writeSecureFile } from '@google-github-actions/actions-utils';
+
 import { Client } from '../src/client';
 import { Bucket, UploadOptions } from '@google-cloud/storage';
 
 import { mockUpload } from './helpers.test';
 
 describe('Client', { concurrency: true }, async () => {
-  test('#new', async (suite) => {
+  test('.build', async (suite) => {
+    const originalEnv = Object.assign({}, process.env);
+    const appCreds = {
+      client_email: '[email protected]',
+      private_key: 'test-private-key',
+    };
+    const appCredsJSON = await writeSecureFile(randomFilepath(), JSON.stringify(appCreds));
+
+    suite.beforeEach(async () => {
+      process.env.GOOGLE_APPLICATION_CREDENTIALS = appCredsJSON;
+    });
+
+    suite.afterEach(async () => {
+      await forceRemove(appCredsJSON);
+      process.env = originalEnv;
+    });
+
     await suite.test('initializes with ADC', async () => {
-      const client = new Client();
+      const client = await Client.build();
       const result = client?.storage?.authClient?.jsonContent;
-      assert.deepStrictEqual(result, null);
+      assert.deepStrictEqual(result, appCreds);
     });
   });
 
@@ -215,7 +233,7 @@ describe('Client', { concurrency: true }, async () => {
       const uploadMock = t.mock.method(Bucket.prototype, 'upload', mockUpload);
 
       // Do the upload
-      const client = new Client();
+      const client = await Client.build();
       await client.upload({
         bucket: 'my-bucket',
         files: [