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

Add a flag to enable ECS exec for debugging purposes #95

Closed
sestrella opened this issue Jun 27, 2023 · 10 comments · Fixed by #127
Closed

Add a flag to enable ECS exec for debugging purposes #95

sestrella opened this issue Jun 27, 2023 · 10 comments · Fixed by #127

Comments

@sestrella
Copy link
Contributor

sestrella commented Jun 27, 2023

Is your request related to a new offering from AWS?

Is this functionality available in the AWS provider for Terraform? See CHANGELOG.md, too.

  • Yes ✅: based on the following issue it looks like enable_ecs_execute was released in version 3.34.0

Is your request related to a problem? Please describe.

Turning on ECS exec for debugging issues in a container is especially useful during the initial bootstrap of a service; however, the list of considerations for turning this feature on is big, which means there is more room for error; additionally, the error message provided by aws ecs execute-command is quite generic, making it difficult to trace problems in the configuration; the AWS team has developed a tool to pinpoint errors in the configuration. Providing a simple method to enable this feature in a container could save time.

Describe the solution you'd like.

Add two flags, one that operates at the task definition level in charge of adding the following statements to the task role:

{
   "Version": "2012-10-17",
   "Statement": [
       {
       "Effect": "Allow",
       "Action": [
            "ssmmessages:CreateControlChannel",
            "ssmmessages:CreateDataChannel",
            "ssmmessages:OpenControlChannel",
            "ssmmessages:OpenDataChannel"
       ],
      "Resource": "*"
      }
   ]
}

And another that operates at the container level to enable the following features:

"linuxParameters": {
  "initProcessEnabled": true
}
"readOnlyFilesystem": false

The interface exposed to the end user would look something like this:

module "ecs_service" {
  ...
  # Enable ECS exec for all containers
  container_definition_defaults = {
    enable_ecs_exec = true
  }

  container_definitions = {
    some_container = {
      ...
      # Enable ECS exec for a specific container
      enable_ecs_exec = true
    }
  }

  # Add the required statements to the task role and sets `enable_execute_command = true` 
  enable_ecs_exec = true
}

Design considerations:

In the event of overlapping configurations, the user's configuration takes precedence; here are a few examples:

If enable_ecs_exec is set to true at the container level and readonly_root_filesystem is set to true the final configuration would look like this:

"linuxParameters": {
  "initProcessEnabled": true
}
"readOnlyFilesystem": true

If enable_ecs_exec is set to true at the container level and linux_parameters is defined, the parameters would be merged:

linux_parameters = {
  capabilities = {
    add = [...]
  }
}

enable_ecs_exec = true

Container configuration:

"linuxParameters": {
  "capabilities": {
    "add": [...]
  }
  "initProcessEnabled": true
}
"readOnlyFilesystem": true

Describe alternatives you've considered.

Adding all of the configurations by hand or creating a wrapper around this module appears to be a workaround when setting up this feature on ECS, however, more people could benefit if this feature is added to a widely adopted module like this one.

Additional context

It is possible to add all the required configurations by hand with the features exposed by this module, however, it is less convenient and more difficult to troubleshoot errors in the configuration.

@github-actions
Copy link

This issue has been automatically marked as stale because it has been open 30 days
with no activity. Remove stale label or comment or this issue will be closed in 10 days

@github-actions github-actions bot added the stale label Jul 30, 2023
@bryantbiggs bryantbiggs added wip and removed stale labels Jul 30, 2023
@mhowell-ims
Copy link

Given that a Fargate task must be configured with 'awsvpc', and given the code here, it doesn't seem to be possible to attach any policies to the Fargate task role. If that's true, then it is additionally not possible to enable ECS exec for a Fargate task using this module. So with Fargate, this is more of a bug than an enhancement.

I also found it somewhat frustrating that there is no mention of the 'needs_iam_role' logic in the module documentation. I spent a few hours trying a variety of workarounds involving use of the various 'iam_*' module parameters with no apparent effect at all. Only after looking at the module source did I realize that all of those parameters are completely ignored when network mode is awsvpc, which is actually the default value, and unlikely to be set otherwise in most cases. Seems crazy to have a bunch of parameters that are ignored by default and not mention it anywhere.

@bryantbiggs
Copy link
Member

I also found it somewhat frustrating that there is no mention of the 'needs_iam_role' logic in the module documentation. I spent a few hours trying a variety of workarounds involving use of the various 'iam_*' module parameters with no apparent effect at all. Only after looking at the module source did I realize that all of those parameters are completely ignored when network mode is awsvpc, which is actually the default value, and unlikely to be set otherwise in most cases. Seems crazy to have a bunch of parameters that are ignored by default and not mention it anywhere.

Perhaps you should browse the ECS documentation https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task-iam-roles.html#ecs-exec-required-iam-permissions

To enable ECS Exec, the permissions should be added to the tasks IAM role

@mhowell-ims
Copy link

Yes I finally figured that out last night and managed to get exec-command working with Fargate using this module, thanks for pointing it out.

I stand by my comment on the docs though, if there's a condition where a group of parameters is ignored, that should be noted in the documentation.

@bryantbiggs
Copy link
Member

that should be noted in the documentation

Why?

@mhowell-ims
Copy link

Because otherwise it is very confusing that making a change to any of those parameters has no effect at all. It makes you think there's a bug in the module, or that there's something wrong with your syntax, etc. It's just very misleading to have a set of parameters that is ignored most of the time, with no explanation of why. A simple statement such as 'This parameter is not used if network mode is awsvpc or if the cluster is attached to a load balancer.', added to the description of each of those parameters, would clarify things.

@bryantbiggs
Copy link
Member

But if this is modeled off of the ECS service and its documentation, why the need to double document?

  • When the deployment controller type is EXTERNAL there are a number of arguments that are not supported
    for_each = { for k, v in var.capacity_provider_strategy : k => v if !local.is_external_deployment }
  • When the scheduling strategy is DAEMON, there are a number of arguments that are not supported
    deployment_maximum_percent = local.is_daemon || local.is_external_deployment ? null : var.deployment_maximum_percent
    deployment_minimum_healthy_percent = local.is_daemon || local.is_external_deployment ? null : var.deployment_minimum_healthy_percent
    desired_count = local.is_daemon || local.is_external_deployment ? null : var.desired_count

There are numerous examples of this - they are there to prevent users from configuring incorrectly. However, we cannot re-explain the ECS service and all of its facets through the module documentation here

@ksitnik-tc
Copy link

Any updates on adding this flag to the module?

@antonbabenko
Copy link
Member

This issue has been resolved in version 5.4.0 🎉

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 Nov 30, 2023
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 a pull request may close this issue.

5 participants