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(lambda): unlock use case for cross-account functions w/ preconfigured permissions #18979

Merged
merged 5 commits into from Feb 16, 2022

Conversation

kaizencc
Copy link
Contributor

@kaizencc kaizencc commented Feb 15, 2022

There have been a few reports of regressions after #18255 was merged in v1.139.0: #18781, #18967.

AFAIK, #18255 fixed a bug in our environment configuring logic for fromFunctionAttributes(), so the environment is now being properly passed to the functionBase that CDK creates. The bug was being relied on by CDK customers for a particular use case: calling grantInvoke() on imported cross-account lambdas with pre-configured permissions. In general, grantInvoke() modifies the trust policy of the resource if it is not from the same environment. In the case of cross-account lambdas, this is impossible, and results in a synth-time error. CDK users know this and have modified the lambda permissions in question to have the correct permissions. These users do not need permissions added to the cross-account lambda, so it is not an error.

To support this use case, I'm introducing skipPermissions as a property for fromFunctionAttributes(). When set, we skip this synth-time check all together. Beware, users who set this property are now on their own for the permissions of their cross-account lambda.

const fn = lambda.Function.fromFunctionAttributes(stack, 'Function', {
  functionArn: 'arn:aws:lambda:us-east-1:123456789012:function:MyFn',
  skipPermissions: true,
});

Closes #18781.


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

@gitpod-io
Copy link

gitpod-io bot commented Feb 15, 2022

@github-actions github-actions bot added the @aws-cdk/aws-lambda Related to AWS Lambda label Feb 15, 2022
@mergify mergify bot added the contribution/core This is a PR that came from AWS. label Feb 15, 2022
@kaizencc kaizencc changed the title feat(lambda): unlock use case for cross-account functions w/ preconfigured permissions fix(lambda): unlock use case for cross-account functions w/ preconfigured permissions Feb 15, 2022
Copy link
Contributor

@skinny85 skinny85 left a comment

Choose a reason for hiding this comment

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

Let's maybe add an entry to the ReadMe file of the module, showing this option, and under what circumstances would you use it?

packages/@aws-cdk/aws-lambda/test/function.test.ts Outdated Show resolved Hide resolved
@skinny85 skinny85 added the pr/do-not-merge This PR should not be merged at this time. label Feb 15, 2022
@kaizencc kaizencc removed the pr/do-not-merge This PR should not be merged at this time. label Feb 16, 2022
@mergify
Copy link
Contributor

mergify bot commented Feb 16, 2022

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

@aws-cdk-automation
Copy link
Collaborator

AWS CodeBuild CI Report

  • CodeBuild project: AutoBuildProject89A8053A-LhjRyN9kxr8o
  • Commit ID: 39082c2
  • Result: SUCCEEDED
  • Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

@mergify mergify bot merged commit 023108a into master Feb 16, 2022
@mergify mergify bot deleted the conroy/skipperms branch February 16, 2022 17:14
@mergify
Copy link
Contributor

mergify bot commented Feb 16, 2022

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this pull request Feb 21, 2022
…ured permissions (aws#18979)

There have been a few reports of regressions after aws#18228 was merged in v1.139.0: aws#18781, aws#18967.

AFAIK, aws#18228 fixed a bug in our environment configuring logic for `fromFunctionAttributes()`, so the environment is now being properly passed to the `functionBase` that CDK creates. The bug was being relied on by CDK customers for a particular use case: calling `grantInvoke()` on imported cross-account lambdas with pre-configured permissions. In general, `grantInvoke()` modifies the trust policy of the resource if it is not from the same environment. In the case of cross-account lambdas, this is impossible, and results in a synth-time error. CDK users know this and have modified the lambda permissions in question to have the correct permissions. These users do not need permissions added to the cross-account lambda, so it is not an error.

To support this use case, I'm introducing `skipPermissions` as a property for `fromFunctionAttributes()`. When set, we skip this synth-time check all together. Beware, users who set this property are now on their own for the permissions of their cross-account lambda.

```ts
const fn = lambda.Function.fromFunctionAttributes(stack, 'Function', {
  functionArn: 'arn:aws:lambda:us-east-1:123456789012:function:MyFn',
  skipPermissions: true,
});
```

Closes aws#18781. 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-lambda Related to AWS Lambda contribution/core This is a PR that came from AWS.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

(lambda): cross account lambda resolvers can not be created
3 participants