fix(stepfunctions-tasks): missing permissions for running tasks on ecs #27891

msambol · 2023-11-08T15:17:36Z

While working on #27803, I noticed the integration tests for aws-stepfunctions-tasks/ecs were not fully working (they deployed but the state machines did not run successfully). This PR addresses two issues:

Missing permissions for ecs:RunTask on the task definition version.

The sample container was from a Lambda image. This resulted in the following error: entrypoint requires the handler name to be the first argument. I changed the image to docker/library/python:3.12.

These changes result in the successful execution of all four state machines in aws-stepfunctions-tasks/ecs.

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

msambol · 2023-11-08T15:17:58Z

...-testing/framework-integ/test/aws-stepfunctions-tasks/test/ecs/eventhandler-image/Dockerfile

-CMD python3 index.py
+FROM --platform=x86-64 public.ecr.aws/docker/library/python:3.12
+ADD index.py .
+CMD [ "python3", "./index.py" ]


I needed --platform=x86-64 since I am building on a Mac M2.

Instead of changing the image have you tried changing CMD:

CMD [ "index.handler" ]

and defining a handler function in index.py:

import os import print def handler(event, context): print('Hello from ECS!') pprint.pprint(dict(os.environ))

?

I did try that but didn't spend a bunch of time on it. I can revisit.. but my thinking is, why is it a Lambda image when this is running on ECS and not in Lambda?

Good point, thanks for clarifying.

msambol · 2023-11-08T15:19:02Z

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/ecs/run-task.ts

-        resources: [this.getTaskDefinitionFamilyArn()],
+        resources: [
+          this.getTaskDefinitionFamilyArn(),
+          `${this.getTaskDefinitionFamilyArn()}:*`,


Drawing attention to the bits that add permission to run versions of the task definition.

lpizzinidev

Thanks!
Some minor comments and a note on the Docker image change.

...@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/ecs/integ.ec2-run-task.ts

...ges/@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/ecs/integ.ec2-task.ts

...-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/ecs/integ.fargate-run-task.ts

...@aws-cdk-testing/framework-integ/test/aws-stepfunctions-tasks/test/ecs/integ.fargate-task.ts

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/ecs/run-task.ts

lpizzinidev · 2023-11-09T10:09:38Z

...-testing/framework-integ/test/aws-stepfunctions-tasks/test/ecs/eventhandler-image/Dockerfile

-CMD python3 index.py
+FROM --platform=x86-64 public.ecr.aws/docker/library/python:3.12
+ADD index.py .
+CMD [ "python3", "./index.py" ]


Instead of changing the image have you tried changing CMD:

CMD [ "index.handler" ]

and defining a handler function in index.py:

import os import print def handler(event, context): print('Hello from ECS!') pprint.pprint(dict(os.environ))

?

lpizzinidev · 2023-11-10T07:30:36Z

...-testing/framework-integ/test/aws-stepfunctions-tasks/test/ecs/eventhandler-image/Dockerfile

-CMD python3 index.py
+FROM --platform=x86-64 public.ecr.aws/docker/library/python:3.12
+ADD index.py .
+CMD [ "python3", "./index.py" ]


Good point, thanks for clarifying.

vinayak-kukreja

Hey @msambol , this is great. Just a clarification comment.

vinayak-kukreja · 2023-12-07T20:44:38Z

packages/aws-cdk-lib/aws-stepfunctions-tasks/test/ecs/eventhandler-image/Dockerfile

Could you tell me why some of these were removed?

And if we can remove --platform=x86-64?

We don't need some of these commands in the Dockerfile.

I needed --platform=x86-64 because I am building on an M2 and got an architecture mismatch. Unless I'm doing something wrong?

I'm not too familiar with Docker but I'm also concerned with this. What about other users who are not on an M2 but on a computer incompatible with this platform? Maybe @mrgrain can weigh in further because I'm not well-versed myself in Docker stuff.

@mrgrain Have some time to look at this?

@msambol What is the exact error you are getting?

@mrgrain I'm not sure if I was doing something wrong or something changed, but it's working now without "platform" 😅

@mrgrain wait... I think this is still an issue. I get exec /usr/local/bin/python3: exec format error when I run the task without --platform=linux/amd64. Let me keep testing..

@mrgrain If I don't use --platform=linux/amd64, I get the following error when the task runs: exec /usr/local/bin/python3: exec format error. Similar here: #28898.

msambol · 2023-12-16T19:00:29Z

@vinayak-kukreja Any additional feedback on this?

msambol · 2024-01-24T15:39:06Z

@mrgrain Have time for another look at this one ?

msambol · 2024-02-11T01:14:59Z

@mrgrain Mind taking another look here?

kaizencc

Thanks and sorry for the delay @msambol

mergify · 2024-02-12T15:36:50Z

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

aws-cdk-automation · 2024-02-12T16:04:15Z

AWS CodeBuild CI Report

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

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

mergify · 2024-02-12T16:06:00Z

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

#27891) While working on [#27803](#27803), I noticed the integration tests for `aws-stepfunctions-tasks/ecs` were not fully working (they deployed but the state machines did not run successfully). This PR addresses two issues: 1. Missing permissions for `ecs:RunTask` on the task definition version. <img width="1587" alt="sfn-role" src="https://github.com/aws/aws-cdk/assets/3310356/13a0d402-8cbb-4852-9708-290f3a3b6711"> 2. The sample container was from a Lambda image. This resulted in the following error: `entrypoint requires the handler name to be the first argument`. I changed the image to `docker/library/python:3.12`. These changes result in the successful execution of all four state machines in `aws-stepfunctions-tasks/ecs`. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*

Updates for the changes introduced in AWS CDK v2.128.0. See: - https://github.com/aws/aws-cdk/releases/tag/v2.128.0 - aws/aws-cdk#27891

github-actions bot added admired-contributor [Pilot] contributed between 13-24 PRs to the CDK p2 labels Nov 8, 2023

aws-cdk-automation requested a review from a team November 8, 2023 15:17

msambol commented Nov 8, 2023

View reviewed changes

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Nov 9, 2023

lpizzinidev suggested changes Nov 9, 2023

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Nov 9, 2023

lpizzinidev approved these changes Nov 10, 2023

View reviewed changes

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Nov 10, 2023

github-actions bot mentioned this pull request Dec 1, 2023

Monthly PR metrics report - November 2023 #28212

Closed

vinayak-kukreja self-assigned this Dec 7, 2023

vinayak-kukreja reviewed Dec 7, 2023

View reviewed changes

msambol added 3 commits January 9, 2024 13:17

fix(stepfunction-tasks): missing permissions for running tasks on ecs

5212117

fix snaps

ee67f9e

add feedback from Luca

885f0c9

msambol force-pushed the sfn-tasks-tests branch from 8478c8a to 9306e0f Compare January 9, 2024 21:05

I guess I don't need platform?

fb37482

msambol force-pushed the sfn-tasks-tests branch from 9306e0f to fb37482 Compare January 9, 2024 23:31

msambol added 4 commits January 9, 2024 18:23

new snaps

b3c9cdb

Merge branch 'main' into sfn-tasks-tests

148fbf4

Merge branch 'main' into sfn-tasks-tests

1370621

Merge branch 'main' into sfn-tasks-tests

b8ad777

Merge branch 'main' into sfn-tasks-tests

b170d7b

kaizencc approved these changes Feb 12, 2024

View reviewed changes

Merge branch 'main' into sfn-tasks-tests

a9bc264

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Feb 12, 2024

mergify bot merged commit 683d595 into aws:main Feb 12, 2024
12 checks passed

akash1810 added a commit to guardian/cdk that referenced this pull request Mar 14, 2024

chore: Update snapshot

ab1123f

Updates for the changes introduced in AWS CDK v2.128.0. See: - https://github.com/aws/aws-cdk/releases/tag/v2.128.0 - aws/aws-cdk#27891

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(stepfunctions-tasks): missing permissions for running tasks on ecs #27891

fix(stepfunctions-tasks): missing permissions for running tasks on ecs #27891

msambol commented Nov 8, 2023

msambol Nov 8, 2023

lpizzinidev Nov 9, 2023

msambol Nov 9, 2023

lpizzinidev Nov 10, 2023

msambol Nov 8, 2023

lpizzinidev left a comment

lpizzinidev Nov 9, 2023

lpizzinidev Nov 10, 2023

vinayak-kukreja left a comment

vinayak-kukreja Dec 7, 2023

msambol Dec 7, 2023

kaizencc Dec 22, 2023

msambol Jan 8, 2024

mrgrain Jan 9, 2024

msambol Jan 10, 2024

msambol Jan 26, 2024

msambol Jan 27, 2024

msambol commented Dec 16, 2023

msambol commented Jan 24, 2024

msambol commented Feb 11, 2024

kaizencc left a comment

mergify bot commented Feb 12, 2024

aws-cdk-automation commented Feb 12, 2024

mergify bot commented Feb 12, 2024

fix(stepfunctions-tasks): missing permissions for running tasks on ecs #27891

fix(stepfunctions-tasks): missing permissions for running tasks on ecs #27891

Conversation

msambol commented Nov 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lpizzinidev left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vinayak-kukreja left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

msambol commented Dec 16, 2023

msambol commented Jan 24, 2024

msambol commented Feb 11, 2024

kaizencc left a comment

Choose a reason for hiding this comment

mergify bot commented Feb 12, 2024

aws-cdk-automation commented Feb 12, 2024

AWS CodeBuild CI Report

mergify bot commented Feb 12, 2024