Skip to content

feat: Adding IAM PassRole for ECS tasks as it is required for Fargate #24

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

Merged
merged 2 commits into from
Sep 15, 2021

Conversation

kalosisz
Copy link
Contributor

Description

If one runs ECS tasks on Fargate, the task execution role mast be passed on. Otherwise one gets a ECS.AccessDeniedException error (more here and here).

Motivation and Context

This PR adds the possibility to reference the task execution role as any other resource in ECS tasks making it simpler for the user no to create erroneous aws_iam_policy_document documents.

Breaking Changes

No.

How Has This Been Tested?

  • I have tested and validated these changes using one or more of the provided examples/* projects
    Deployed in my own project filling out the
ecs_Sync = {
      ecs = [
        data.aws_ecs_task_definition.xxxx.id
      ]
      ecs_Wildcard = [
        data.aws_ecs_task_definition.xxxx.id
      ]
      iam_PassRole = [
        data.aws_iam_role.ecs_task_execution_role.arn
      ]
    }

which would add the statement

                  + {
                      + Action    = "iam:PassRole"
                      + Condition = {
                          + StringEquals = {
                              + iam:PassedToService = [
                                  + "ecs-tasks.amazonaws.com",
                                ]
                            }
                        }
                      + Effect    = "Allow"
                      + Resource  = "arn:aws:iam::xxxxxx:role/xxxxxxx"
                      + Sid       = "ecsSyncIamPassRole"
                    },

as well as without which results in adding statement to the policy (in that case it would not add or remove)

                  - {
                      - Action    = "iam:PassRole"
                      - Condition = {
                          - StringEquals = {
                              - iam:PassedToService = [
                                  - "ecs-tasks.amazonaws.com",
                                ]
                            }
                        }
                      - Effect    = "Allow"
                      - Resource  = "arn:aws:iam::xxxxxx:role/xxxxxxx"
                      - Sid       = "ecsSyncIamPassRole"
                    },

The results for ecs_WaitForTaskToken is identical.

Copy link
Member

@antonbabenko antonbabenko left a comment

Choose a reason for hiding this comment

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

I can't see this in the documentation for this integration here - https://docs.aws.amazon.com/step-functions/latest/dg/ecs-iam.html

For example, there is such service policy for EKS already in the documentation and in aws_service_policies (line 589).

It means that AWS documentation is wrong. Not the first time but good to know!

@antonbabenko antonbabenko changed the title feat: adding PassRole for ECS tasks as it is required for Fargate feat: Adding IAM PassRole for ECS tasks as it is required for Fargate Sep 15, 2021
@antonbabenko antonbabenko merged commit 37206c8 into terraform-aws-modules:master Sep 15, 2021
@antonbabenko
Copy link
Member

Thanks for this addition, @kalosisz !

v2.5.0 has been just released.

@github-actions
Copy link

I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants