fix: consolidate backend secret custom resources (#2011)

* fix: use a single custom resource for fetching secrets

* chore: update package-lock.json

* chore: add changeset

* chore: refactor changes

* chore: cleanup

* chore: cleanup

* chore: cleanup

* chore: cleanup

* chore: rename resource id

* feed pr base sha and ref into envs before scripts (#2168)

* feed pr base sha and ref into envs before scripts

* removing empty file

---------

Co-authored-by: Roshane Pascual <[email protected]>

Author: awsluja

.changeset/purple-otters-poke.md

@@ -0,0 +1,5 @@
+---
+'@aws-amplify/backend': patch
+---
+
+Backend Secrets now use a single custom resource to reduce concurrent lambda executions.

.changeset/spicy-rules-speak.md

@@ -0,0 +1,2 @@
+---
+---

packages/backend/src/engine/backend-secret/backend_secret.ts

@@ -16,7 +16,7 @@ export class CfnTokenBackendSecret implements BackendSecret {
    * The name of the secret to fetch.
    */
   constructor(
-    private readonly name: string,
+    private readonly secretName: string,
     private readonly secretResourceFactory: BackendSecretFetcherFactory
   ) {}
   /**
@@ -28,11 +28,11 @@ export class CfnTokenBackendSecret implements BackendSecret {
   ): SecretValue => {
     const secretResource = this.secretResourceFactory.getOrCreate(
       scope,
-      this.name,
+      this.secretName,
       backendIdentifier
     );
 
-    const val = secretResource.getAttString('secretValue');
+    const val = secretResource.getAttString(`${this.secretName}`);
     return SecretValue.unsafePlainText(val); // safe since 'val' is a cdk token.
   };
 
@@ -43,11 +43,11 @@ export class CfnTokenBackendSecret implements BackendSecret {
     return {
       branchSecretPath: ParameterPathConversions.toParameterFullPath(
         backendIdentifier,
-        this.name
+        this.secretName
       ),
       sharedSecretPath: ParameterPathConversions.toParameterFullPath(
         backendIdentifier.namespace,
-        this.name
+        this.secretName
       ),
     };
   };

packages/backend/src/engine/backend-secret/backend_secret_fetcher_factory.test.ts

@@ -9,7 +9,7 @@ import {
 } from './backend_secret_fetcher_factory.js';
 import { BackendIdentifier } from '@aws-amplify/plugin-types';
 
-const secretResourceType = 'Custom::SecretFetcherResource';
+const secretResourceType = 'Custom::AmplifySecretFetcherResource';
 const namespace = 'testId';
 const name = 'testBranch';
 const secretName1 = 'testSecretName1';
@@ -33,26 +33,18 @@ void describe('getOrCreate', () => {
     resourceFactory.getOrCreate(stack, secretName2, backendId);
 
     const template = Template.fromStack(stack);
-    template.resourceCountIs(secretResourceType, 2);
-    let customResources = template.findResources(secretResourceType, {
+    // only one custom resource is created that fetches all secrets
+    template.resourceCountIs(secretResourceType, 1);
+    const customResources = template.findResources(secretResourceType, {
       Properties: {
         namespace,
         name,
-        secretName: secretName1,
+        secretNames: [secretName1, secretName2],
         secretLastUpdated,
       },
     });
     assert.equal(Object.keys(customResources).length, 1);
 
-    customResources = template.findResources(secretResourceType, {
-      Properties: {
-        namespace,
-        name,
-        secretName: secretName2,
-      },
-    });
-    assert.equal(Object.keys(customResources).length, 1);
-
     // only 1 secret fetcher lambda and 1 resource provider lambda are created.
     const providers = template.findResources('AWS::Lambda::Function');
     const names = Object.keys(providers);
@@ -67,6 +59,8 @@ void describe('getOrCreate', () => {
   void it('does not create duplicate resource for the same secret name', () => {
     const app = new App();
     const stack = new Stack(app);
+
+    // ensure only 1 resource is created even if this is called twice
     resourceFactory.getOrCreate(stack, secretName1, backendId);
     resourceFactory.getOrCreate(stack, secretName1, backendId);
 
@@ -78,6 +72,7 @@ void describe('getOrCreate', () => {
     const body = customResources[resourceName]['Properties'];
     assert.strictEqual(body['namespace'], namespace);
     assert.strictEqual(body['name'], name);
-    assert.strictEqual(body['secretName'], secretName1);
+    assert.equal(body['secretNames'].length, 1);
+    assert.equal(body['secretNames'][0], secretName1);
   });
 });

packages/backend/src/engine/backend-secret/backend_secret_fetcher_factory.ts

@@ -1,18 +1,38 @@
 import { Construct } from 'constructs';
 import { BackendSecretFetcherProviderFactory } from './backend_secret_fetcher_provider_factory.js';
-import { CustomResource } from 'aws-cdk-lib';
+import { CustomResource, CustomResourceProps, Lazy } from 'aws-cdk-lib';
 import { BackendIdentifier } from '@aws-amplify/plugin-types';
 import { SecretResourceProps } from './lambda/backend_secret_fetcher_types.js';
 
 /**
  * Resource provider ID for the backend secret resource.
  */
-export const SECRET_RESOURCE_PROVIDER_ID = 'SecretFetcherResourceProvider';
+export const SECRET_RESOURCE_PROVIDER_ID =
+  'AmplifySecretFetcherResourceProvider';
+
+class SecretFetcherCustomResource extends CustomResource {
+  private secrets: Set<string>;
+  constructor(
+    scope: Construct,
+    id: string,
+    props: CustomResourceProps,
+    secrets: Set<string>
+  ) {
+    super(scope, id, {
+      ...props,
+    });
+    this.secrets = secrets;
+  }
+
+  public addSecret = (secretName: string) => {
+    this.secrets.add(secretName);
+  };
+}
 
 /**
  * Type of the backend custom CFN resource.
  */
-const SECRET_RESOURCE_TYPE = `Custom::SecretFetcherResource`;
+const SECRET_RESOURCE_TYPE = `Custom::AmplifySecretFetcherResource`;
 
 /**
  * The factory to create backend secret-fetcher resource.
@@ -33,15 +53,18 @@ export class BackendSecretFetcherFactory {
     scope: Construct,
     secretName: string,
     backendIdentifier: BackendIdentifier
-  ): CustomResource => {
-    const secretResourceId = `${secretName}SecretFetcherResource`;
+  ): SecretFetcherCustomResource => {
+    const secretResourceId = `AmplifySecretFetcherResource`;
     const existingResource = scope.node.tryFindChild(
       secretResourceId
-    ) as CustomResource;
+    ) as SecretFetcherCustomResource;
 
     if (existingResource) {
+      existingResource.addSecret(secretName);
       return existingResource;
     }
+    const secrets: Set<string> = new Set();
+    secrets.add(secretName);
 
     const provider = this.secretProviderFactory.getOrCreateInstance(
       scope,
@@ -59,16 +82,25 @@ export class BackendSecretFetcherFactory {
       namespace: backendIdentifier.namespace,
       name: backendIdentifier.name,
       type: backendIdentifier.type,
-      secretName: secretName,
+      secretNames: Lazy.list({
+        produce: () => {
+          return Array.from(secrets);
+        },
+      }),
     };
 
-    return new CustomResource(scope, secretResourceId, {
-      serviceToken: provider.serviceToken,
-      properties: {
-        ...customResourceProps,
-        secretLastUpdated, // this property is only to trigger resource update event.
+    return new SecretFetcherCustomResource(
+      scope,
+      secretResourceId,
+      {
+        serviceToken: provider.serviceToken,
+        properties: {
+          ...customResourceProps,
+          secretLastUpdated, // this property is only to trigger resource update event.
+        },
+        resourceType: SECRET_RESOURCE_TYPE,
       },
-      resourceType: SECRET_RESOURCE_TYPE,
-    });
+      secrets
+    );
   };
 }

packages/backend/src/engine/backend-secret/lambda/backend_secret_fetcher.test.ts

@@ -43,7 +43,7 @@ const customResourceEventCommon = {
   ResourceType: 'AWS::CloudFormation::CustomResource',
   ResourceProperties: {
     ...testBackendIdentifier,
-    secretName: testSecretName,
+    secretNames: [testSecretName],
     ServiceToken: 'token',
   },
   OldResourceProperties: {},
@@ -84,7 +84,7 @@ void describe('handleCreateUpdateEvent', () => {
       Promise.resolve(testSecret)
     );
     const val = await handleCreateUpdateEvent(secretHandler, createCfnEvent);
-    assert.equal(val, testSecretValue);
+    assert.equal(val[testSecretName], testSecretValue);
 
     assert.equal(mockGetSecret.mock.callCount(), 1);
     assert.deepStrictEqual(mockGetSecret.mock.calls[0].arguments, [
@@ -120,7 +120,7 @@ void describe('handleCreateUpdateEvent', () => {
     );
 
     const val = await handleCreateUpdateEvent(secretHandler, createCfnEvent);
-    assert.equal(val, testSecretValue);
+    assert.equal(val[testSecretName], testSecretValue);
 
     assert.equal(mockGetSecret.mock.callCount(), 2);
     assert.deepStrictEqual(mockGetSecret.mock.calls[0].arguments, [
@@ -145,7 +145,7 @@ void describe('handleCreateUpdateEvent', () => {
       }
     );
     const val = await handleCreateUpdateEvent(secretHandler, createCfnEvent);
-    assert.equal(val, testSecretValue);
+    assert.equal(val[testSecretName], testSecretValue);
 
     assert.equal(mockGetSecret.mock.callCount(), 2);
     assert.deepStrictEqual(mockGetSecret.mock.calls[0].arguments, [

packages/backend/src/engine/backend-secret/lambda/backend_secret_fetcher.ts

@@ -22,11 +22,11 @@ export const handler = async (
 
   const physicalId =
     event.RequestType === 'Create' ? randomUUID() : event.PhysicalResourceId;
-  let data: { secretValue: string } | undefined = undefined;
+  let data: Record<string, string> | undefined = undefined;
   if (event.RequestType === 'Update' || event.RequestType === 'Create') {
-    const val = await handleCreateUpdateEvent(secretClient, event);
+    const secretMap = await handleCreateUpdateEvent(secretClient, event);
     data = {
-      secretValue: val,
+      ...secretMap,
     };
   }
 
@@ -47,54 +47,59 @@ export const handler = async (
 export const handleCreateUpdateEvent = async (
   secretClient: SecretClient,
   event: CloudFormationCustomResourceEvent
-): Promise<string> => {
+): Promise<Record<string, string>> => {
   const props = event.ResourceProperties as unknown as SecretResourceProps;
-  let secret: string | undefined;
+  const secretMap: Record<string, string> = {};
+  for (const secretName of props.secretNames) {
+    let secretValue: string | undefined = undefined;
+    try {
+      const resp = await secretClient.getSecret(
+        {
+          namespace: props.namespace,
+          name: props.name,
+          type: props.type,
+        },
+        {
+          name: secretName,
+        }
+      );
+      secretValue = resp.value;
+    } catch (err) {
+      const secretErr = err as SecretError;
+      if (secretErr.httpStatusCode && secretErr.httpStatusCode >= 500) {
+        throw new Error(
+          `Failed to retrieve backend secret '${secretName}' for '${
+            props.namespace
+          }/${props.name}'. Reason: ${JSON.stringify(err)}`
+        );
+      }
+    }
 
-  try {
-    const resp = await secretClient.getSecret(
-      {
-        namespace: props.namespace,
-        name: props.name,
-        type: props.type,
-      },
-      {
-        name: props.secretName,
+    // if the secret is not available in branch path, try retrieving it at the app-level.
+    if (!secretValue) {
+      try {
+        const resp = await secretClient.getSecret(props.namespace, {
+          name: secretName,
+        });
+        secretValue = resp.value;
+      } catch (err) {
+        throw new Error(
+          `Failed to retrieve backend secret '${secretName}' for '${
+            props.namespace
+          }'. Reason: ${JSON.stringify(err)}`
+        );
       }
-    );
-    secret = resp?.value;
-  } catch (err) {
-    const secretErr = err as SecretError;
-    if (secretErr.httpStatusCode && secretErr.httpStatusCode >= 500) {
-      throw new Error(
-        `Failed to retrieve backend secret '${props.secretName}' for '${
-          props.namespace
-        }/${props.name}'. Reason: ${JSON.stringify(err)}`
-      );
     }
-  }
 
-  // if the secret is not available in branch path, try retrieving it at the app-level.
-  if (!secret) {
-    try {
-      const resp = await secretClient.getSecret(props.namespace, {
-        name: props.secretName,
-      });
-      secret = resp?.value;
-    } catch (err) {
+    if (!secretValue) {
       throw new Error(
-        `Failed to retrieve backend secret '${props.secretName}' for '${
-          props.namespace
-        }'. Reason: ${JSON.stringify(err)}`
+        `Unable to find backend secret for backend '${props.namespace}', branch '${props.name}', name '${secretName}'`
       );
     }
-  }
 
-  if (!secret) {
-    throw new Error(
-      `Unable to find backend secret for backend '${props.namespace}', branch '${props.name}', name '${props.secretName}'`
-    );
+    // store the secret->secretValue pair in the secret map
+    secretMap[secretName] = secretValue;
   }
 
-  return secret;
+  return secretMap;
 };

packages/backend/src/engine/backend-secret/lambda/backend_secret_fetcher_types.ts

@@ -1,5 +1,5 @@
 import { BackendIdentifier } from '@aws-amplify/plugin-types';
 
 export type SecretResourceProps = Omit<BackendIdentifier, 'hash'> & {
-  secretName: string;
+  secretNames: string[];
 };