Skip to content

Conversation

@rtpascual
Copy link
Contributor

@rtpascual rtpascual commented Dec 19, 2023

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.

Corresponding docs PR, if applicable:

Validation

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.

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
Copy link

changeset-bot bot commented Dec 19, 2023

🦋 Changeset detected

Latest commit: 6c9e653

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

@rtpascual rtpascual marked this pull request as ready for review December 21, 2023 22:40
Copy link
Contributor

@edwardfoyle edwardfoyle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good, just left some small comments.

Only somewhat big thing is I think this change warrants some e2e coverage. I'd recommend adding it to the existing data_storage_auth_with_triggers.ts test which already sets up some secrets to use for auth. You'd need to update the "specialTestFunction" definition to reference one of those secrets and then verify that it was able to fetch it at runtime. You could verify by adding the resolved secret value to the response payload here and then verify you got the right thing here

@edwardfoyle
Copy link
Contributor

One other thing that we handle with auth secrets and need to handle here as well is falling back to "shared" secrets if a branch-specific secret doesn't exist. You can see where we do this in the secret resolver custom resource here

I'm thinking we should figure out some way to refactor that code such that it can be used in the custom resource lambda and also injected into the banner of customer lambdas.

@rtpascual
Copy link
Contributor Author

Closing this PR for #845 in order to change the source branch for e2e test checks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants