feat: initial implementation for function secret access

[rtpascual]

Original PR #836

Problem

defineFunction does not have access to secrets created with npx amplify sandbox secret.

Issue number, if available:

Changes

• Add support for passing in secrets through the environment parameter of defineFunction.
• Move parameter path methods from backend-secret to platform-core to be used across multiple packages.
• Add bundling options for function to add a code snippet to resolve secrets for users.
• Update e2e test to cover function secret access.

Corresponding docs PR, if applicable:

Validation

Updated data_storage_auth_with_triggers.ts e2e test to ensure secrets are properly resolved and can be accessed in the lambda function.

Used a test project with local changes to deploy a function with secrets using defineFunction and verified the following:

• Environment variables for the placeholder text and secret path are populating in the Lambda console as expected.
• Test executing the function to verify secrets are resolved to their expected values as seen in the Parameter Store.

Example code with code snippet as seen in lambda console:

/* The code in this line replaces placeholder text in environment variables for secrets with values fetched from SSM, this is a noop if there are no secrets */import { SSM } from '@aws-sdk/client-ssm';/** * The body of this function will be used to resolve secrets for Lambda functions */export const internalAmplifyFunctionBannerResolveSecrets = async (client) => {    if (!client) {        client = new SSM();    }    const envPathObj = JSON.parse(process.env.AMPLIFY_SECRET_PATHS ?? '{}');    const paths = Object.values(envPathObj);    if (paths.length === 0) {        return;    }    const response = await client.getParameters({        Names: paths,        WithDecryption: true,    });    if (response.Parameters && response.Parameters?.length > 0) {        for (const parameter of response.Parameters) {            for (const [env, path] of Object.entries(envPathObj)) {                if (parameter.Name === path) {                    process.env[env] = parameter.Value;                }            }        }    }};await internalAmplifyFunctionBannerResolveSecrets();

// test-projects/testapp/amplify/function/handler.ts
var handler = async (event) => {
  console.log(`TEST_SECRET is ${process.env.TEST_SECRET}`);
  console.log(`TEST_SECRET2 is ${process.env.TEST_SECRET2}`);
};
export {
  handler
};

Checklist

• If this PR includes a functional change to the runtime behavior of the code, I have added or updated automated test coverage for this change.
• If this PR requires a change to the Project Architecture README, I have included that update in this PR.
• If this PR requires a docs update, I have linked to that docs PR above.
• If this PR modifies E2E tests, makes changes to resource provisioning, or makes SDK calls, I have run the PR checks with the run-e2e label set.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[changeset-bot]

🦋 Changeset detected

Latest commit: 5f9f1a9

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 12 packages

Name	Type
@aws-amplify/backend-function	Minor
@aws-amplify/plugin-types	Minor
@aws-amplify/backend	Minor
@aws-amplify/backend-secret	Patch
@aws-amplify/platform-core	Patch
@aws-amplify/auth-construct-alpha	Patch
@aws-amplify/backend-auth	Patch
@aws-amplify/backend-data	Patch
@aws-amplify/backend-deployer	Patch
@aws-amplify/backend-storage	Patch
create-amplify	Patch
@aws-amplify/sandbox	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[sobolk]

Would it make sense to wrap all this code (maybe except import?) into anonymous function to avoid declaring global name, i.e. eliminate a chance that we'll collide with customer symbols?

I.e. something like:

await (async function() {
  /// body of this file goes here;
}) ();

(see https://www.javascripttutorial.net/javascript-anonymous-functions/ )

[rtpascual]

I'm not sure if we will be able to test the code if we wrap it in an anonymous function, which was why it is done this way.

[sobolk]

By looking at test it seems the output assertion is using env vars, so that part is covered, so if there's some way to mock SSM client globally this could be an option.
Other option to solve testing is to test this with live SSM (i.e. not mock SSM and add e2e test for this fragment.

But if that's not very practical then another option is to keep it as is but make sure that function name is long and hard to guess. I.e. internalAmplifyFunctionBannerResolveSecrets or something more obfuscated.

It seems that function name is the only global symbol that matters, so maybe obfuscating name is good enough.

[sobolk]

How does bundler handle this import?
The reason I'm asking is that for other files that are bundled I'm guessing bunder generates some form of import/require in right place, but I'm not sure how this works for banner (which they advertise as a place to inject comments), so I wonder if theres a chance that in some more complex lambdas this might become a problem.
Might be helpful if you could share how sample bundled code looks like in a comment.

[rtpascual]

Bundling format is ESM to allow the top level await to work. The code with an example handler in lambda console looks like this:

/* The code in this line replaces placeholder text in environment variables for secrets with values fetched from SSM, this is a noop if there are no secrets */import { SSM } from '@aws-sdk/client-ssm';/** * The body of this function will be used to resolve secrets for Lambda functions */export const internalAmplifyFunctionBannerResolveSecrets = async (client) => {    if (!client) {        client = new SSM();    }    const envPathObj = JSON.parse(process.env.AMPLIFY_SECRET_PATHS ?? '{}');    const paths = Object.values(envPathObj);    if (paths.length === 0) {        return;    }    const response = await client.getParameters({        Names: paths,        WithDecryption: true,    });    if (response.Parameters && response.Parameters?.length > 0) {        for (const parameter of response.Parameters) {            for (const [env, path] of Object.entries(envPathObj)) {                if (parameter.Name === path) {                    process.env[env] = parameter.Value;                }            }        }    }};await internalAmplifyFunctionBannerResolveSecrets();//# sourceMappingURL=data:application/json;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoicmVzb2x2ZV9zZWNyZXRfYmFubmVyLmpzIiwic291cmNlUm9vdCI6IiIsInNvdXJjZXMiOlsiLi4vc3JjL3Jlc29sdmVfc2VjcmV0X2Jhbm5lci50cyJdLCJuYW1lcyI6W10sIm1hcHBpbmdzIjoiQUFBQSxtSkFBbUo7QUFDbkosT0FBTyxFQUFFLEdBQUcsRUFBRSxNQUFNLHFCQUFxQixDQUFDO0FBRTFDOztHQUVHO0FBQ0gsTUFBTSxDQUFDLE1BQU0sMkNBQTJDLEdBQUcsS0FBSyxFQUM5RCxNQUFZLEVBQ1osRUFBRTtJQUNGLElBQUksQ0FBQyxNQUFNLEVBQUU7UUFDWCxNQUFNLEdBQUcsSUFBSSxHQUFHLEVBQUUsQ0FBQztLQUNwQjtJQUVELE1BQU0sVUFBVSxHQUEyQixJQUFJLENBQUMsS0FBSyxDQUNuRCxPQUFPLENBQUMsR0FBRyxDQUFDLG9CQUFvQixJQUFJLElBQUksQ0FDekMsQ0FBQztJQUNGLE1BQU0sS0FBSyxHQUFHLE1BQU0sQ0FBQyxNQUFNLENBQUMsVUFBVSxDQUFDLENBQUM7SUFFeEMsSUFBSSxLQUFLLENBQUMsTUFBTSxLQUFLLENBQUMsRUFBRTtRQUN0QixPQUFPO0tBQ1I7SUFFRCxNQUFNLFFBQVEsR0FBRyxNQUFNLE1BQU0sQ0FBQyxhQUFhLENBQUM7UUFDMUMsS0FBSyxFQUFFLEtBQUs7UUFDWixjQUFjLEVBQUUsSUFBSTtLQUNyQixDQUFDLENBQUM7SUFDSCxJQUFJLFFBQVEsQ0FBQyxVQUFVLElBQUksUUFBUSxDQUFDLFVBQVUsRUFBRSxNQUFNLEdBQUcsQ0FBQyxFQUFFO1FBQzFELEtBQUssTUFBTSxTQUFTLElBQUksUUFBUSxDQUFDLFVBQVUsRUFBRTtZQUMzQyxLQUFLLE1BQU0sQ0FBQyxHQUFHLEVBQUUsSUFBSSxDQUFDLElBQUksTUFBTSxDQUFDLE9BQU8sQ0FBQyxVQUFVLENBQUMsRUFBRTtnQkFDcEQsSUFBSSxTQUFTLENBQUMsSUFBSSxLQUFLLElBQUksRUFBRTtvQkFDM0IsT0FBTyxDQUFDLEdBQUcsQ0FBQyxHQUFHLENBQUMsR0FBRyxTQUFTLENBQUMsS0FBSyxDQUFDO2lCQUNwQzthQUNGO1NBQ0Y7S0FDRjtBQUNILENBQUMsQ0FBQztBQUVGLE1BQU0sMkNBQTJDLEVBQUUsQ0FBQyIsInNvdXJjZXNDb250ZW50IjpbIi8qIFRoaXMgY29kZSByZXBsYWNlcyBwbGFjZWhvbGRlciB0ZXh0IGluIGVudmlyb25tZW50IHZhcmlhYmxlcyBmb3Igc2VjcmV0cyB3aXRoIHZhbHVlcyBmZXRjaGVkIGZyb20gU1NNLCB0aGlzIGlzIGEgbm9vcCBpZiB0aGVyZSBhcmUgbm8gc2VjcmV0cyAqL1xuaW1wb3J0IHsgU1NNIH0gZnJvbSAnQGF3cy1zZGsvY2xpZW50LXNzbSc7XG5cbi8qKlxuICogVGhlIGJvZHkgb2YgdGhpcyBmdW5jdGlvbiB3aWxsIGJlIHVzZWQgdG8gcmVzb2x2ZSBzZWNyZXRzIGZvciBMYW1iZGEgZnVuY3Rpb25zXG4gKi9cbmV4cG9ydCBjb25zdCBpbnRlcm5hbEFtcGxpZnlGdW5jdGlvbkJhbm5lclJlc29sdmVTZWNyZXRzID0gYXN5bmMgKFxuICBjbGllbnQ/OiBTU01cbikgPT4ge1xuICBpZiAoIWNsaWVudCkge1xuICAgIGNsaWVudCA9IG5ldyBTU00oKTtcbiAgfVxuXG4gIGNvbnN0IGVudlBhdGhPYmo6IFJlY29yZDxzdHJpbmcsIHN0cmluZz4gPSBKU09OLnBhcnNlKFxuICAgIHByb2Nlc3MuZW52LkFNUExJRllfU0VDUkVUX1BBVEhTID8/ICd7fSdcbiAgKTtcbiAgY29uc3QgcGF0aHMgPSBPYmplY3QudmFsdWVzKGVudlBhdGhPYmopO1xuXG4gIGlmIChwYXRocy5sZW5ndGggPT09IDApIHtcbiAgICByZXR1cm47XG4gIH1cblxuICBjb25zdCByZXNwb25zZSA9IGF3YWl0IGNsaWVudC5nZXRQYXJhbWV0ZXJzKHtcbiAgICBOYW1lczogcGF0aHMsXG4gICAgV2l0aERlY3J5cHRpb246IHRydWUsXG4gIH0pO1xuICBpZiAocmVzcG9uc2UuUGFyYW1ldGVycyAmJiByZXNwb25zZS5QYXJhbWV0ZXJzPy5sZW5ndGggPiAwKSB7XG4gICAgZm9yIChjb25zdCBwYXJhbWV0ZXIgb2YgcmVzcG9uc2UuUGFyYW1ldGVycykge1xuICAgICAgZm9yIChjb25zdCBbZW52LCBwYXRoXSBvZiBPYmplY3QuZW50cmllcyhlbnZQYXRoT2JqKSkge1xuICAgICAgICBpZiAocGFyYW1ldGVyLk5hbWUgPT09IHBhdGgpIHtcbiAgICAgICAgICBwcm9jZXNzLmVudltlbnZdID0gcGFyYW1ldGVyLlZhbHVlO1xuICAgICAgICB9XG4gICAgICB9XG4gICAgfVxuICB9XG59O1xuXG5hd2FpdCBpbnRlcm5hbEFtcGxpZnlGdW5jdGlvbkJhbm5lclJlc29sdmVTZWNyZXRzKCk7XG4iXX0=

// test-projects/testapp/amplify/function/handler.ts
var handler = async (event) => {
  console.log(`TEST_SECRET is ${process.env.TEST_SECRET}`);
  console.log(`TEST_SECRET2 is ${process.env.TEST_SECRET2}`);
};
export {
  handler
};

[edwardfoyle]

If we're going to minify like this, we should remove all whitespace. We should also remove the source map

[sobolk]

is this working fine if test-projects/testapp/amplify/function/handler.ts has its own imports as well?

[rtpascual]

Not sure of a good way to remove all the meaningless whitespace (we would need whitespace between await and internalAmplifyFunctionBannerResolveSecrets(); for example)

[rtpascual]

is this working fine if test-projects/testapp/amplify/function/handler.ts has its own imports as well?

Yes, all the contents of handler.ts will be under the comment // test-projects/testapp/amplify/function/handler.ts and still work as expected from what I've tested.

[edwardfoyle]

There are some interesting gotchas for this file. For example, an inline comment // some comment would break everything after that after it gets collapsed into a single line. I think imports of 3p modules or other files in resolve_secret_banner would also break as we are not following those imports and bundling them as well.

The "real" fix for all of this would be to run resolve_secret_banner through esbuild, then add that bundled output as the banner for esbuild of the customer lambda. But I'm okay if we just want to capture this thread as a GH issue and revisit if/when we need to do more complicated stuff here.

[edwardfoyle]

It sounds like there was some windows / unix issue related with keeping the newlines, but if we kept newlines in, that would remove the inline comment gotcha and make the output more readable.

[rtpascual]

Yup which is why I opted for /* */ for comments. Also it could be a problem with encoding but newlines were just kept as \n in the console and caused errors which was why I removed them.

[edwardfoyle]

Approving as long as we capture the concerns from this thread in a GH issue. However, if there are any quick wins from that thread we can include here we should do that.