Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: consolidate backend secret custom resources #2011

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
5 changes: 5 additions & 0 deletions .changeset/purple-otters-poke.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@aws-amplify/backend': patch
---

Backend Secrets now use a single custom resource to reduce concurrent lambda executions.
2 changes: 2 additions & 0 deletions .changeset/spicy-rules-speak.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
---
---
12 changes: 9 additions & 3 deletions .github/workflows/health_checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -411,7 +411,9 @@ jobs:
- uses: ./.github/actions/setup_node
- uses: ./.github/actions/restore_install_cache
- run: git fetch origin
- run: npm run diff:check ${{ github.event.pull_request.base.sha }}
- run: npm run diff:check "$BASE_SHA"
env:
BASE_SHA: ${{ github.event.pull_request.base.sha }}
check_pr_changesets:
if: github.event_name == 'pull_request' && github.event.pull_request.user.login != 'github-actions[bot]'
runs-on: ubuntu-latest
Expand All @@ -425,9 +427,13 @@ jobs:
- uses: ./.github/actions/setup_node
- uses: ./.github/actions/restore_install_cache
- name: Validate that PR has changeset
run: npx changeset status --since origin/${{ github.event.pull_request.base.ref }}
run: npx changeset status --since origin/"$BASE_REF"
env:
BASE_REF: ${{ github.event.pull_request.base.ref }}
- name: Validate changeset is not missing packages
run: npx tsx scripts/check_changeset_completeness.ts ${{ github.event.pull_request.base.sha }}
run: npx tsx scripts/check_changeset_completeness.ts "$BASE_SHA"
env:
BASE_SHA: ${{ github.event.pull_request.base.sha }}
- name: Validate that changeset has necessary dependency updates
run: |
npx changeset version
Expand Down
3 changes: 3 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 5 additions & 5 deletions packages/backend/src/engine/backend-secret/backend_secret.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
) {}
/**
Expand All @@ -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.
};

Expand All @@ -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
),
};
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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);
Expand All @@ -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);

Expand All @@ -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);
});
});
Original file line number Diff line number Diff line change
@@ -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.
Expand All @@ -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,
Expand All @@ -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
);
};
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ const customResourceEventCommon = {
ResourceType: 'AWS::CloudFormation::CustomResource',
ResourceProperties: {
...testBackendIdentifier,
secretName: testSecretName,
secretNames: [testSecretName],
ServiceToken: 'token',
},
OldResourceProperties: {},
Expand Down Expand Up @@ -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, [
Expand Down Expand Up @@ -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, [
Expand All @@ -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, [
Expand Down
Loading
Loading