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

(aws-codepipelines-actions): Cannot set region on LambdaInvokeAction #18228

Closed
jhoscar1 opened this issue Dec 30, 2021 · 7 comments · Fixed by #18255
Closed

(aws-codepipelines-actions): Cannot set region on LambdaInvokeAction #18228

jhoscar1 opened this issue Dec 30, 2021 · 7 comments · Fixed by #18255
Assignees
Labels
@aws-cdk/aws-codepipeline-actions effort/small Small work item – less than a day of effort feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs feature-request A feature should be added or improved. p2

Comments

@jhoscar1
Copy link

What is the problem?

LambdaInvokeAction does not accept region as a parameter. I have a CodePipeline that needs to invoke a function in a different region. I was hitting errors due to the LambdaInvokeAction assuming the region is the same as the CodePipeline, but it worked when I manually set the region in the action configuration in the AWS console. According to the documentation this property can be defined in Cloudformation so it’s not clear why it’s not supported in CDK. I noticed that LambdaInvokeActionProps extends the CommonActionProps interface rather than the ActionProperties interface which does include the region field.

Reproduction Steps

Created a Codepipeline stack in us-west-2 with a stage containing a LambdaInvokeAction that needed to invoke a function in us-east-1.

What did you expect to happen?

I expected to be able to define the region for the action to invoke the lambda.

What actually happened?

The action had a region of us-west-2 and I could only change the region manually in the console.

CDK CLI Version

1.108.0

Framework Version

No response

Node.js Version

14.16.1

OS

Mac OS Big Sur (11.6)

Language

Typescript

Language Version

4.5.2

Other information

No response

@jhoscar1 jhoscar1 added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 30, 2021
@ryparker ryparker added feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs p2 effort/small Small work item – less than a day of effort and removed bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Dec 30, 2021
@ryparker
Copy link
Contributor

PR: #18229

@ryparker ryparker added the feature-request A feature should be added or improved. label Dec 30, 2021
@ryparker ryparker assigned ryparker and unassigned skinny85 Dec 30, 2021
@skinny85
Copy link
Contributor

Thanks for opening the issue @jhoscar1. Can you show the CDK code you are using to create your Lambda invoke CodePipeline Action?

Thanks,
Adam

@skinny85 skinny85 added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Dec 30, 2021
@github-actions
Copy link

github-actions bot commented Jan 2, 2022

This issue has not received a response in a while. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.

@github-actions github-actions bot added the closing-soon This issue will automatically close in 4 days unless further comments are made. label Jan 2, 2022
@jhoscar1
Copy link
Author

jhoscar1 commented Jan 2, 2022

@skinny85 will post when back at work on Monday. Commenting to prevent auto close

@github-actions github-actions bot removed closing-soon This issue will automatically close in 4 days unless further comments are made. response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. labels Jan 2, 2022
@jhoscar1
Copy link
Author

jhoscar1 commented Jan 3, 2022

Here's a simplified version of the code we're running to create the action:

export class CodePipelineStack extends cdk.Stack {
  public readonly devPipelineOutput: cdk.CfnOutput;constructor(scope: cdk.Construct, id: string, props: CodePipelineStackProps) {
    super(scope, id, props);const appNameLambda = lambda.Function.fromFunctionAttributes(this, "AppNameLambda", {
      functionArn: cdk.Arn.format(
        { service: "lambda", resource: "function:AppName", account: "[**DIFFERENT ACCOUNT THAN PIPELINE**]" },
        cdk.Stack.of(this)
      ),
    });const lambdaInvokeAction = new codepipeline_actions.LambdaInvokeAction({
      actionName: "AppNameInvoke",
      lambda: appNameLambda,
      userParameters: {
        param1: "value",
      },
      role: iam.Role.fromRoleArn(
        this,
        "crossAccountRole",
        "arn:aws:iam::[**SAME ACCOUNT AS LAMBDA**]:role/app-name-cross-account-invoke"
      ),
    });const devPipeline = new codepipeline.Pipeline(this, "DevPipeline", {
      stages: [
        {
          stageName: "FetchDynamicValues",
          actions: [lambdaInvokeAction],
        },
      ],
    });this.devPipelineOutput = new cdk.CfnOutput(this, "DevPipelineOutput", {
      value: devPipeline.pipelineName,
    });
  }
}

@skinny85
Copy link
Contributor

skinny85 commented Jan 3, 2022

Yep, I see the problem. We need to pass the region from the ARN here.

I'll prepare a PR fixing this.

skinny85 added a commit to skinny85/aws-cdk that referenced this issue Jan 3, 2022
skinny85 added a commit to skinny85/aws-cdk that referenced this issue Jan 4, 2022
skinny85 added a commit to skinny85/aws-cdk that referenced this issue Jan 4, 2022
skinny85 added a commit to skinny85/aws-cdk that referenced this issue Jan 4, 2022
@mergify mergify bot closed this as completed in #18255 Jan 4, 2022
mergify bot pushed a commit that referenced this issue Jan 4, 2022
…Stack, instead of its ARN (#18255)

Fixes #18228

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Jan 4, 2022

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

mergify bot pushed a commit that referenced this issue Feb 16, 2022
…ured permissions (#18979)

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

AFAIK, #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 #18781. 

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue Feb 21, 2022
…Stack, instead of its ARN (aws#18255)

Fixes aws#18228

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
TikiTDO pushed a commit to TikiTDO/aws-cdk that referenced this issue 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-codepipeline-actions effort/small Small work item – less than a day of effort feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs feature-request A feature should be added or improved. p2
Projects
None yet
3 participants