-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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): imported Function still has region and account from its Stack, instead of its ARN #18255
Conversation
b862ef3
to
3773c09
Compare
…Stack, instead of its ARN Fixes aws#18228
3773c09
to
15a0718
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎉 thanks @skinny85
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 CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
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). |
@skinny85 I'm seeing that this was released in the 2.x version of cdk. Is it possible to get this released in 1.x or is there a plan to? |
@jhoscar1 looks like this just barely missed the release window for |
Great, thanks! |
Hello, I think this breaks some of my usages when used in conjunction with const lambdaFunction = lambda.Function.fromFunctionArn(this, 'Lambda', `arn:aws:lambda:${new cdk.ScopedAws(this).region}:${new cdk.ScopedAws(this).accountId}:function:${functionName}`);
const pipeline = new codepipeline.Pipeline(this, 'Codepipeline', {
artifactBucket: "",
pipelineName: "pipeline"
});
pipeline.onStateChange('Notify', {target: new eventsTarget.LambdaFunction(lambdaFunction)});
It used to work with cdk 1.138.0 I can try to create a clean reproducer if you want EDIT : or maybe I'm using it wrong |
@cbm-gplassard when you say this used to work, do you mean it used to |
Hello @skinny85 , I created a sample reproducer stack : https://github.com/cbm-gplassard/cdk-lambda-events-reproducer (one branch with cdk 1.138.0 and another with 1.139.0) |
@cbm-gplassard I wonder - are you experiencing the same issue as described in #18405? If so, can we consolidate the conversation there? |
Seems like the same issue, indeed! 👍 |
After aws#18255 has been merged, the account and region of the Function's imported ARN is correctly recognized. Unfortunately, this has surfaced a few cases where that causes problems, like when using an imported Function as the target of a CodePipeline invoke Action (the pipeline construct needs to verify that the target Function is in the same account and region as the pipeline). Add a `Function.fromFunctionName()` method that allows you to import a Function in the same account and region. Fixes aws#19031
After #18255 has been merged, the account and region of the Function's imported ARN is correctly recognized. Unfortunately, this has surfaced a few cases where that causes problems, like when using an imported Function as the target of a CodePipeline invoke Action (the pipeline construct needs to verify that the target Function is in the same account and region as the pipeline). Add a `Function.fromFunctionName()` method that allows you to import a Function in the same account and region. Fixes #19031 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
After aws#18255 has been merged, the account and region of the Function's imported ARN is correctly recognized. Unfortunately, this has surfaced a few cases where that causes problems, like when using an imported Function as the target of a CodePipeline invoke Action (the pipeline construct needs to verify that the target Function is in the same account and region as the pipeline). Add a `Function.fromFunctionName()` method that allows you to import a Function in the same account and region. Fixes aws#19031 ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Fixes #21464. KMS keys imported using `fromKeyArn()` currently take the environment of the stack, not the environment from the arn. This PR follows the precedent set in #19026 and #18255. It is essentially the same code change and tests. Ideally, we would have a mechanism for testing all `fromXxxArn` APIs to ensure they have the correct behavior. There are still many places where it does not. However, given the significant overhead of creating such a mechanism, I'm creating this one-off PR to unblock users in KMS. ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Fixes aws#21464. KMS keys imported using `fromKeyArn()` currently take the environment of the stack, not the environment from the arn. This PR follows the precedent set in aws#19026 and aws#18255. It is essentially the same code change and tests. Ideally, we would have a mechanism for testing all `fromXxxArn` APIs to ensure they have the correct behavior. There are still many places where it does not. However, given the significant overhead of creating such a mechanism, I'm creating this one-off PR to unblock users in KMS. ---- ### All Submissions: * [ ] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) ### Adding new Unconventional Dependencies: * [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies) ### New Features * [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)? * [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)? *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Fixes #18228
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license