Skip to content

Can't deploy step functions with ECS integration #11

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

Closed
jakubigla opened this issue Apr 6, 2021 · 10 comments
Closed

Can't deploy step functions with ECS integration #11

jakubigla opened this issue Apr 6, 2021 · 10 comments

Comments

@jakubigla
Copy link

jakubigla commented Apr 6, 2021

I'm trying to deploy step functions with arn:aws:states:::ecs:runTask.sync Task.

However for some reason I can't deploy it. This is the error code:

Error: AccessDeniedException: 'arn:aws:iam::xxxx:role/step-functions-role' is not authorized to create managed-rule.
        status code: 400, request id: xxxx

My service_integration block:

service_integrations = {
    lambda = {
      lambda = [module.data_transform.open_charge_map_ref_lambda_arn]
    }
    ecs_Sync = {
      ecs = [module.pipeline_ecs_cluster.this_ecs_cluster_arn]
      events = ["*"]
    }
}

My feeling is, that the above block is wrong, but there's no clear example.
The lambda stuff works correctly, the ecs task is my next iteration.

@antonbabenko
Copy link
Member

HI,

Your code looks good.

I see there were some similar issues reported before:

One of the potential solutions can be to wait for the IAM role creation. Ideally, Terraform AWS provider should do this.

@jakubigla
Copy link
Author

jakubigla commented Apr 6, 2021

I was able to make that working with the following snippet:

service_integrations = {
    ecs_Sync = {
      ecs = [module.data_transform_crawler_trigger_task.arn]
      events = ["arn:aws:events:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:rule/StepFunctionsGetEventsForECSTaskRule"]
    }
  }

It feels that this could be something we could add to this awesome module, so the consumer doesn't need to compose this by themselves as it looks like a static-ish value.

This allowed me to create a step functions state machine, but I now get failures during execution.

User: arn:aws:sts::xxxx:assumed-role/step-functions-role/xxxxxxxxx is not authorized to perform: iam:PassRole on resource: arn:aws:iam::xxxx:role/crawler-task-exec-role (Service: AmazonECS; Status Code: 400; Error Code: AccessDeniedException; Request ID: xxxxxxxx; Proxy: null)

I've looked into locals.tf file and I can see, that some service integrations support this, but not the ECS one. Is it also something that could be added to the module? For now I think I will need to pass a custom policy/role.

@antonbabenko
Copy link
Member

Good that the first part works for you.

We need to change locals.tf inside events (line 253) and add:

default_resources = ["arn:aws:events:${local.aws_region}:${data.aws_caller_identity.current.account_id}:rule/StepFunctionsGetEventsForECSTaskRule"]

A similar change has to be done for ecs section (line 239) according to the doc.

Also add data-source "aws_caller_identity" "current" into main.tf.

Will you be able to make it?


I am reading the official doc and I don't see a mention of iam:PassRole anywhere though it is certainly required (for eg, https://cloudonaut.io/resilient-task-scheduling-with-ecs-fargate-cron-scheduled-task/ ).

I am not sure whether we should add it somewhere? Or just describe this case via custom policy? What do you think?

@jakubigla
Copy link
Author

I can certainly do a pull request this week, which will enhance this.

Regarding the iam:PassRole.
Fargate tasks require a task execution role, therefore if the task is scheduled by some other AWS service, which is invoked by a user, this permission is required. I suggest we should just add a note about this.
This has worked:

module "step_functions_pipeline" {
  source  = "terraform-aws-modules/step-functions/aws"
  version = "1.2.0"

  ....

  service_integrations = {
    ecs_Sync = {
      ecs = [module.data_transform.crawler_trigger_task_definition_arn]
      events = ["arn:aws:events:${data.aws_region.current.name}:${data.aws_caller_identity.current.account_id}:rule/StepFunctionsGetEventsForECSTaskRule"]
    }
  }

  attach_policy_json = true
  policy_json        = data.aws_iam_policy_document.ecs_task_pass_role.json
}

data "aws_iam_policy_document" "ecs_task_pass_role" {
  statement {
    actions = ["iam:PassRole"]
    resources = [
      module.data_transform.crawler_trigger_task_execution_role_arn
    ]
  }
}

@antonbabenko
Copy link
Member

One option is like you say but I think it is rather common case and worth adding to the module as another item in local.aws_service_policies which will allow users to write this:

  service_integrations = {
    ecs_Sync = {
      ecs = [module.data_transform.crawler_trigger_task_definition_arn]
      events = true  # <= `true` means that value from `default_resources` will be used
    }
    iam = {
      iam_PassRole = [module.data_transform.crawler_trigger_task_execution_role_arn]
    }
  }

Please make the required changes in a PR.

@kalosisz
Copy link
Contributor

Hey, I was gonna create an issue for the very same matter. The PR #19 was initially adding the same default. Why has it been pushed out? Based on AWS docs, the resource never changes there. It could easily be part of the default. What do you guys think?

@antonbabenko
Copy link
Member

So many questions :)

@kalosisz Do you say that #19 fixes the issue? Could you please validate it, so that I can merge it faster?

@kalosisz
Copy link
Contributor

I would add the

default_resources = ["arn:aws:events:${local.aws_region}:${data.aws_caller_identity.current.account_id}:rule/StepFunctionsGetEventsForECSTaskRule"]

for ECS. Maybe the caller identity could be added to locals as it corresponds to the account of interest.

I have created a #24 for adding iam:PassRole for ECS tasks as this is needed if one runs them on Fargate.

I'm also running another SF from the parent SF and it would need

default_resources = ["arn:aws:events:${local.aws_region}:${data.aws_caller_identity.current.account_id}:rule/StepFunctionsGetEventsForStepFunctionsExecutionRule"]

but the logic is the same. This doesn't need iam:PassRole though.

@antonbabenko
Copy link
Member

Fixed in #24

v2.5.0 has been just released.

@github-actions
Copy link

I'm going to lock this issue 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 similar to this, 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

No branches or pull requests

3 participants