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

(aws-ecs): ECS Exec Support #13618

Closed
1 of 2 tasks
mbeacom opened this issue Mar 16, 2021 · 14 comments · Fixed by #14670
Closed
1 of 2 tasks

(aws-ecs): ECS Exec Support #13618

mbeacom opened this issue Mar 16, 2021 · 14 comments · Fixed by #14670
Assignees
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container effort/medium Medium work item – several days of effort feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs feature-request A feature should be added or improved. in-progress This issue is being actively worked on. p1

Comments

@mbeacom
Copy link
Contributor

mbeacom commented Mar 16, 2021

As of March 16, ECS now supports remote execution similar to docker exec.

Use Case

As a user, I want the capability to execute commands against an AWS ECS Fargate / EC2 container (similar to docker exec) workloads managed by CDK.

Proposed Solution

CloudFormation's AWS::ECS::Service resource currently supports the boolean property: EnableExecuteCommand

Expose this to the aws-ecs's FargateService similar to enableEcsManagedTags

Other

  • 👋 I may be able to implement this feature request
  • ⚠️ This feature might incur a breaking change

This is a 🚀 Feature Request

@mbeacom mbeacom added feature-request A feature should be added or improved. needs-triage This issue or PR still needs to be triaged. labels Mar 16, 2021
@peterwoodworth peterwoodworth added the @aws-cdk/aws-ecs Related to Amazon Elastic Container label Mar 16, 2021
@SoManyHs SoManyHs added p2 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels Mar 16, 2021
@SoManyHs
Copy link
Contributor

We may also want to consider modifying the task role since "the task role will need to have IAM permissions to log the output to S3 and/or CloudWatch if the cluster is configured for these options. If these options are not configured then these IAM permissions are not required."

See: "Configuring the task role with the proper IAM policy"

@underbluewaters
Copy link

Is there any way one could use this functionality before it has higher-level support within CDK?

@SoManyHs
Copy link
Contributor

@mbeacom Hi! Thanks so much for opening this issue -- was wondering if you were actively working on this? If so I can tag it as "in-progress"!

@SoManyHs SoManyHs added p1 and removed p2 labels Mar 18, 2021
@MrArnoldPalmer
Copy link
Contributor

@underbluewaters you can use escape hatches to set the EnableExecuteCommand property on the L1 ecs.CfnService construct. This property will be available on the next CDK release as it was just pulled from the new spec yesterday #13633.

You can access the underlying L1 resource of any construct or use the L1 classes directly until we add these properties to the corresponding higher level constructs. See the above docs for details.

@SoManyHs
Copy link
Contributor

Note: CfnSpec with update on ECS Service for EnableExecuteCommand is now merged: #13633

@underbluewaters
Copy link

@MrArnoldPalmer Thanks! I was able to get it working with the following code:

// const cfnService = ecsService.node.defaultChild as CfnService;
// doesn't work, see
// https://github.com/aws/aws-cdk/issues/10666
const cfnService = ecsService.node.children[0] as CfnService;
cfnService.addOverride("Properties.EnableExecuteCommand", "True");

I had to be sure to add the following permissions to the task execution role:

const role = new iam.Role(this, "MaintenanceRole", {
  assumedBy: new ServicePrincipal("ecs-tasks.amazonaws.com"),
  inlinePolicies: {
    ecsExec: new PolicyDocument({
      statements: [
        new PolicyStatement({
          effect: iam.Effect.ALLOW,
          actions: [
            "ssmmessages:CreateControlChannel",
            "ssmmessages:CreateDataChannel",
            "ssmmessages:OpenControlChannel",
            "ssmmessages:OpenDataChannel",
          ],
          resources: ["*"],
        }),
      ],
    }),
  },
});
const taskDefinition = new ecs.FargateTaskDefinition(
  this,
  "SeaSketchMaintenanceFargateTaskDef",
  {
    cpu: 256,
    executionRole: role,
    memoryLimitMiB: 512,
  }
);

@mbeacom
Copy link
Contributor Author

mbeacom commented Mar 18, 2021

@SoManyHs Yes, I'd be more than happy to (as long as an ETA of this weekend will suffice).

@SoManyHs
Copy link
Contributor

@mbeacom awesome! I have some starter code pushed to my fork if you want to save some time and build off of that -- added the EnableExecuteCommand property, but if you could continue with adding the IAM policy to the taskRole, that'd be amazing!

@SoManyHs SoManyHs added the in-progress This issue is being actively worked on. label Mar 23, 2021
@SoManyHs
Copy link
Contributor

Hi @mbeacom, were you still planning on submitting a PR for this? Thanks!

@SoManyHs SoManyHs added the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Mar 29, 2021
@mbeacom
Copy link
Contributor Author

mbeacom commented Mar 31, 2021

@SoManyHs Yes, apologies for the delay. I can push this out shortly!

@github-actions github-actions bot removed the response-requested Waiting on additional info and feedback. Will move to "closing-soon" in 7 days. label Apr 1, 2021
@ericzbeard ericzbeard added the feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs label Apr 1, 2021
@SoManyHs
Copy link
Contributor

Hi @mbeacom,
If you're not actively working on this anymore, mind if I move this back into the backlog? Thanks!

@SoManyHs
Copy link
Contributor

Hi @mbeacom!

We're planning on prioritizing this in our next sprint -- since we haven't heard from you in awhile I'll go ahead and unassign you. Thanks for your help, and feel free to reach out if you have other input!

@upparekh
Copy link
Contributor

upparekh commented May 13, 2021

Hi, just updating the issue with the current status on the feature. The implementation involves enabling support for the enableExecuteCommand field, adding IAM permissions for the AWS Systems Manager Agent (SSM) and further adding permissions for logging using CloudWatch or S3. For more information, please refer the documentation. I'm currently working on adding and testing the logging permissions.

mergify bot pushed a commit that referenced this issue Jun 2, 2021
…e log group (#14947)

This PR adds a public method `logGroupPhysicalName()` to access the physical name of a log group which is a private property of the `Resource` class.

This change is needed to enable using KMS keys with log groups for use with ECS exec.
Related: #13618

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@mergify mergify bot closed this as completed in #14670 Jun 3, 2021
mergify bot pushed a commit that referenced this issue Jun 3, 2021
Fixes #13618

This PR adds support for ECS exec command. It adds necessary IAM permissions for the AWS Systems Manager (SSM) to enable exec and also adds IAM permissions for allowing the exec command result logs to be routed to either CloudWatch logs/ S3 Bucket or both.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
@github-actions
Copy link

github-actions bot commented Jun 3, 2021

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
…e log group (aws#14947)

This PR adds a public method `logGroupPhysicalName()` to access the physical name of a log group which is a private property of the `Resource` class.

This change is needed to enable using KMS keys with log groups for use with ECS exec.
Related: aws#13618

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
hollanddd pushed a commit to hollanddd/aws-cdk that referenced this issue Aug 26, 2021
Fixes aws#13618

This PR adds support for ECS exec command. It adds necessary IAM permissions for the AWS Systems Manager (SSM) to enable exec and also adds IAM permissions for allowing the exec command result logs to be routed to either CloudWatch logs/ S3 Bucket or both.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container effort/medium Medium work item – several days of effort feature/coverage-gap Gaps in CloudFormation coverage by L2 constructs feature-request A feature should be added or improved. in-progress This issue is being actively worked on. p1
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants