fix(stepfunctions-tasks): in-flight stepfunction executions fail on update of versioned lambdas #21233

mrgrain · 2022-07-19T15:13:19Z

When the Step Function task is invoking a Lambda Version, we used to only grant invoke permissions for the specific Version. This can cause in-flight executions of the Step Function to fail due to missing permissions, since changing the referenced Version would cause the Policy to remove permissions for the previous Version - which is currently in-flight.

The fix here is to add invokeFunction permissions for all qualifiers of the Lambda Function whenever a Version is provided. While this is not ideal, it enables users to safely update Lambda Versions with LambdaInvoke. I believe this is a common enough use case to warrant this fix.

However, it's worth noting that the same issue will still occur if e.g. the Function is updated to a completely different one or an Alias is changed.

fixes #17515

All Submissions:

Have you followed the guidelines in our Contributing guide?

Adding new Unconventional Dependencies:

This PR adds new unconventional dependencies following the process described here

New Features

Have you added the new feature to an integration test?
- 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

gitpod-io · 2022-07-19T15:13:25Z

…g lambda functions via qualifiers To establish the current functionality before makes changes to this feature.

…tion should grant permissions for all qualifiers

mrgrain · 2022-07-19T16:59:12Z

packages/@aws-cdk/aws-stepfunctions-tasks/test/lambda/integ.invoke.qualifiers.ts

@@ -0,0 +1,80 @@
+import { Code, Function, Runtime } from '@aws-cdk/aws-lambda';


This file is copied from integ.invoke.ts and adjusted to use a Version and an Alias.

…voke all versions of a lambda function

…g lambda qualifiers The issue has been fixed, now the integration tests need to be updated. As expected, the PolicyDocument now lists and additional resource for the Version.

aws-cdk-automation · 2022-07-19T17:41:58Z

AWS CodeBuild CI Report

CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
Commit ID: 5263ffb
Result: SUCCEEDED
Build Logs (available for 30 days)

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

rix0rrr · 2022-07-20T17:22:56Z

packages/@aws-cdk/aws-stepfunctions-tasks/lib/lambda/invoke.ts

+   */
+  private determineResourceArnsForGrantInvoke(lambdaFunction: IFunction): string[] {
+    if (isVersion(lambdaFunction)) {
+      return lambdaFunction.lambda.resourceArnsForGrantInvoke;


Is version.resourceArnsForGrantInvoke ever correct, in the face of changing versions?

Isn't it easier to just update the behavior of Version?

Semantically, I'd expect lambdaVersion.grantInvoke(role) to grant permissions for the specified version and not all qualifiers. Otherwise I would do lambdaVersion.grantInvoke(role). That's a far reaching change to make.

The idiomatic way to solve this problem in AWS is to use an Alias. I'm on the fence, but because of this I'm more inclined to argue this "works as intended", than to change the behavior of Version.

Maybe we're going about this the wrong way and we need to solve the undeniably bad customer experience differently.

@rix0rrr @madeline-k #20177 seems to be of a similar nature. Specifically @kaizencc made the point that if users want to "[..] grant less permissible policies, grant them on the version or alias you've created, not the function itself.". Changing Version to also grant permissions to all versions, would contradict this.

You've all clearly thought about this in-depth, so I'm very happy to follow your lead.

I think Kaizen is right. Users should have created an alias, and invoked that. Invoking by currentVersion seems always incorrect, given the code that CDK generates.

In fact we should probably discourage it in the docs?

@rix0rrr So close this PR, update the docs, and then close the related issue with explanation?

@rix0rrr Can you quickly confirm if I understood you correctly regarding this PR and issue?

So close this PR, update the docs, and then close the related issue with explanation?

Agreed with @rix0rrr that this PR does not address the root cause of the issue.

rix0rrr

Please make sure that your PR title confirms to the conventional commit standard (fix, feat, chore) and that it is written in a style that will reflect correctly in the change log (See Contributing Guide, Pull Requests)

mrgrain · 2022-08-08T12:18:39Z

This change doesn't address the root cause. See comments above. Further discussions continue on #17515

github-actions bot added bug This issue is a bug. effort/small Small work item – less than a day of effort p1 labels Jul 19, 2022

aws-cdk-automation requested a review from a team July 19, 2022 15:13

mergify bot added the contribution/core This is a PR that came from AWS. label Jul 19, 2022

mrgrain force-pushed the fix/stepfunctions-tasks/lambda-versions-permissions branch from e2c5a4c to 2038466 Compare July 19, 2022 16:57

mrgrain changed the title ~~fix(stepfunction-tasks): grant step functions task permissions to invoke all versions of a lambda function~~ fix(stepfunctions-tasks): grant step functions task permissions to invoke all versions of a lambda function Jul 19, 2022

mrgrain added 2 commits July 19, 2022 17:57

test(stepfunctions-tasks): add baseline integration tests for invokin…

c0a03d6

…g lambda functions via qualifiers To establish the current functionality before makes changes to this feature.

test(stepfunctions-tasks): invoking a lambda version from a step func…

1d28826

…tion should grant permissions for all qualifiers

mrgrain force-pushed the fix/stepfunctions-tasks/lambda-versions-permissions branch from 2038466 to 83459a7 Compare July 19, 2022 16:57

mrgrain marked this pull request as ready for review July 19, 2022 16:58

mrgrain commented Jul 19, 2022

View reviewed changes

mrgrain added 2 commits July 19, 2022 18:04

fix(stepfunctions-tasks): grant step functions task permissions to in…

771cbe5

…voke all versions of a lambda function

test(stepfunctions-tasks): update integration tests after for invokin…

5263ffb

…g lambda qualifiers The issue has been fixed, now the integration tests need to be updated. As expected, the PolicyDocument now lists and additional resource for the Version.

mrgrain force-pushed the fix/stepfunctions-tasks/lambda-versions-permissions branch from 83459a7 to 5263ffb Compare July 19, 2022 17:05

aws deleted a comment from aws-cdk-automation Jul 19, 2022

rix0rrr reviewed Jul 20, 2022

View reviewed changes

mrgrain changed the title ~~fix(stepfunctions-tasks): prevent in-flight step function executions from failing with missing permissions, when the invoked lambda version is updated~~ fix(stepfunctions-tasks): in-flight stepfunction executions fail on update of versioned lambdas Jul 21, 2022

mrgrain requested a review from rix0rrr July 21, 2022 10:02

mrgrain closed this Aug 8, 2022

mrgrain deleted the fix/stepfunctions-tasks/lambda-versions-permissions branch July 26, 2023 10:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(stepfunctions-tasks): in-flight stepfunction executions fail on update of versioned lambdas #21233

fix(stepfunctions-tasks): in-flight stepfunction executions fail on update of versioned lambdas #21233

mrgrain commented Jul 19, 2022 •

edited

gitpod-io bot commented Jul 19, 2022

mrgrain Jul 19, 2022

aws-cdk-automation commented Jul 19, 2022

rix0rrr Jul 20, 2022

mrgrain Jul 20, 2022 •

edited

mrgrain Jul 21, 2022 •

edited

rix0rrr Jul 22, 2022

mrgrain Jul 22, 2022

mrgrain Jul 28, 2022

mrgrain Aug 8, 2022

rix0rrr left a comment

mrgrain commented Aug 8, 2022

		@@ -0,0 +1,80 @@
		import { Code, Function, Runtime } from '@aws-cdk/aws-lambda';

fix(stepfunctions-tasks): in-flight stepfunction executions fail on update of versioned lambdas #21233

fix(stepfunctions-tasks): in-flight stepfunction executions fail on update of versioned lambdas #21233

Conversation

mrgrain commented Jul 19, 2022 • edited

All Submissions:

Adding new Unconventional Dependencies:

New Features

gitpod-io bot commented Jul 19, 2022

mrgrain Jul 19, 2022

Choose a reason for hiding this comment

aws-cdk-automation commented Jul 19, 2022

AWS CodeBuild CI Report

rix0rrr Jul 20, 2022

Choose a reason for hiding this comment

mrgrain Jul 20, 2022 • edited

Choose a reason for hiding this comment

mrgrain Jul 21, 2022 • edited

Choose a reason for hiding this comment

rix0rrr Jul 22, 2022

Choose a reason for hiding this comment

mrgrain Jul 22, 2022

Choose a reason for hiding this comment

mrgrain Jul 28, 2022

Choose a reason for hiding this comment

mrgrain Aug 8, 2022

Choose a reason for hiding this comment

rix0rrr left a comment

Choose a reason for hiding this comment

mrgrain commented Aug 8, 2022

mrgrain commented Jul 19, 2022 •

edited

mrgrain Jul 20, 2022 •

edited

mrgrain Jul 21, 2022 •

edited