feat(events-targets): allow all ECS TaskOverrides #32344

blimmer · 2024-12-01T17:33:56Z

Issue #32217

Reason for this change

ECS targets can override any item in the TaskOverride structure via the input parameter, according to the docs.

However, today, only the containerOverrides option is exposed:

aws-cdk/packages/aws-cdk-lib/aws-events-targets/lib/ecs-task.ts

Lines 225 to 227 in 1b7265b

    
           const containerOverrides = this.props.containerOverrides && this.props.containerOverrides 
        
             .map(({ containerName, ...overrides }) => ({ name: containerName, ...overrides })); 
        
           const input = { containerOverrides };

Description of changes

This PR adds all TaskOverride properties to the input parameter.

Description of how you validated changes

I add unit tests. I also updated the existing integration tests to include additional overrides. I validated them in my AWS account.

Checklist

My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

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

blimmer · 2024-12-01T17:34:44Z

packages/aws-cdk-lib/aws-events-targets/lib/ecs-task-properties.ts

+ */
+export interface EphemeralStorageOverride {
+  /**
+   * The total amount, in GiB, of ephemeral storage to set for the task.


I copied these docstrings from the Task Override docs: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_TaskOverride.html

blimmer · 2024-12-01T17:35:02Z

packages/aws-cdk-lib/aws-events-targets/lib/ecs-task.ts

@@ -50,6 +50,54 @@ export interface EcsTaskProps extends TargetBaseProps {
   */
  readonly containerOverrides?: ContainerOverride[];

+  /**
+   * The CPU override for the task.


Again, copied from the upstream docs: https://docs.aws.amazon.com/AmazonECS/latest/APIReference/API_TaskOverride.html

blimmer · 2024-12-01T17:35:30Z

packages/aws-cdk-lib/aws-events-targets/lib/ecs-task.ts

    const taskCount = this.taskCount;
    const taskDefinitionArn = this.taskDefinition.taskDefinitionArn;
    const propagateTags = this.propagateTags;
    const tagList = this.tags;
    const enableExecuteCommand = this.enableExecuteCommand;
+    const input = this.createInput();


Since this logic is more complex now, I refactored it to a dedicated private method.

aws-cdk-automation

The pull request linter has failed. See the aws-cdk-automation comment below for failure reasons. If you believe this pull request should receive an exemption, please comment and provide a justification.

A comment requesting an exemption should contain the text Exemption Request. Additionally, if clarification is needed add Clarification Request to a comment.

✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.

codecov · 2024-12-01T17:53:42Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.90%. Comparing base (a0fa042) to head (fccb178).

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #32344   +/-   ##
=======================================
  Coverage   80.90%   80.90%           
=======================================
  Files         236      236           
  Lines       14261    14261           
  Branches     2493     2493           
=======================================
  Hits        11538    11538           
  Misses       2438     2438           
  Partials      285      285

Flag	Coverage Δ
suite.unit	`80.90% <ø> (ø)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Components	Coverage Δ
packages/aws-cdk	`79.74% <ø> (ø)`
packages/aws-cdk-lib/core	`82.16% <ø> (ø)`

aaythapa

Thanks for adding this! Looks good, just a few nits

packages/aws-cdk-lib/aws-events-targets/lib/ecs-task.ts

packages/aws-cdk-lib/aws-events-targets/test/ecs/event-rule-target.test.ts

...aws-cdk-testing/framework-integ/test/aws-events-targets/test/ecs/integ.event-fargate-task.ts

…sk-overrides

blimmer · 2025-02-12T21:19:21Z

Thanks for the thorough and diligent review @aaythapa . From my perspective, this is good to go. Let me know if you need any more changes from my end.

mergify · 2025-02-12T21:23:18Z

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).

aaythapa · 2025-02-12T21:23:42Z

Thanks for the thorough and diligent review @aaythapa . From my perspective, this is good to go. Let me know if you need any more changes from my end.

LGTM, thanks as always for your contribution

mergify · 2025-02-12T21:28:16Z

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).

mergify · 2025-02-12T22:16:10Z

This pull request has been removed from the queue for the following reason: pull request branch update failed.

The pull request can't be updated

You should look at the reason for the failure and decide if the pull request needs to be fixed or if you want to requeue it.

If you want to requeue this pull request, you need to post a comment with the text: @mergifyio requeue

blimmer · 2025-02-12T22:38:20Z

@Mergifyio requeue

mergify · 2025-02-12T22:38:28Z

requeue

❌ Command disallowed due to command restrictions in the Mergify configuration.

sender-permission >= write

Pull request has been modified.

blimmer · 2025-02-12T22:38:57Z

@aaythapa I think you'll need to re-queue this. It looks like main updated a workflow, so mergify rejected it.

aws-cdk-automation · 2025-02-12T23:09:52Z

AWS CodeBuild CI Report

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

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

mergify · 2025-02-12T23:48:46Z

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).

github-actions · 2025-02-12T23:49:04Z

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

blimmer added 6 commits November 30, 2024 12:46

feat: add all ecs task properties for overrides

8036fea

refactor: move input to a separate private method

c42d496

feat: add unit tests for ephemeral storage

6050489

feat: test all inputs

ddf3a13

fix: update ec2 integration test

b387072

fix: updatfargate integration test

9c657ae

github-actions bot added effort/small Small work item – less than a day of effort feature-request A feature should be added or improved. p2 labels Dec 1, 2024

aws-cdk-automation requested a review from a team December 1, 2024 17:34

github-actions bot added the admired-contributor [Pilot] contributed between 13-24 PRs to the CDK label Dec 1, 2024

blimmer commented Dec 1, 2024

View reviewed changes

Merge branch 'main' into add-all-ecs-task-overrides

7a68fed

aws-cdk-automation previously requested changes Dec 1, 2024

View reviewed changes

docs: add README

b65c237

blimmer changed the title ~~feat(events-targets): add all ECS task properties for overrides~~ feat(events-targets): allow all ECS TaskOverrides Dec 1, 2024

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

Merge branch 'main' into add-all-ecs-task-overrides

5c179b5

github-actions bot mentioned this pull request Jan 1, 2025

Monthly PR metrics report #32703

Closed

aaythapa reviewed Feb 5, 2025

View reviewed changes

blimmer and others added 3 commits February 7, 2025 08:28

Merge branch 'main' of github.com:blimmer/aws-cdk into add-all-ecs-ta…

92cd8d9

…sk-overrides

fix: test.each

7e60f87

Merge branch 'main' into add-all-ecs-task-overrides

b7f8bb4

aaythapa self-assigned this Feb 10, 2025

aaythapa previously approved these changes Feb 12, 2025

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 Feb 12, 2025

Merge branch 'main' into add-all-ecs-task-overrides

fccb178

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

aaythapa approved these changes Feb 12, 2025

View reviewed changes

mergify bot merged commit aebb331 into aws:main Feb 12, 2025
26 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Feb 12, 2025

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

blimmer deleted the add-all-ecs-task-overrides branch February 13, 2025 01:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(events-targets): allow all ECS TaskOverrides #32344

feat(events-targets): allow all ECS TaskOverrides #32344

blimmer commented Dec 1, 2024

blimmer Dec 1, 2024

blimmer Dec 1, 2024

blimmer Dec 1, 2024

aws-cdk-automation left a comment

codecov bot commented Dec 1, 2024 •

edited

Loading

aaythapa left a comment

blimmer commented Feb 12, 2025

mergify bot commented Feb 12, 2025

aaythapa commented Feb 12, 2025

mergify bot commented Feb 12, 2025

mergify bot commented Feb 12, 2025

blimmer commented Feb 12, 2025

mergify bot commented Feb 12, 2025

blimmer commented Feb 12, 2025

aws-cdk-automation commented Feb 12, 2025

mergify bot commented Feb 12, 2025

github-actions bot commented Feb 12, 2025

	const containerOverrides = this.props.containerOverrides && this.props.containerOverrides
	.map(({ containerName, ...overrides }) => ({ name: containerName, ...overrides }));
	const input = { containerOverrides };

feat(events-targets): allow all ECS TaskOverrides #32344

feat(events-targets): allow all ECS TaskOverrides #32344

Conversation

blimmer commented Dec 1, 2024

Issue #32217

Reason for this change

Description of changes

Description of how you validated changes

Checklist

blimmer Dec 1, 2024

Choose a reason for hiding this comment

blimmer Dec 1, 2024

Choose a reason for hiding this comment

blimmer Dec 1, 2024

Choose a reason for hiding this comment

aws-cdk-automation left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 1, 2024 • edited Loading

Codecov Report

aaythapa left a comment

Choose a reason for hiding this comment

blimmer commented Feb 12, 2025

mergify bot commented Feb 12, 2025

aaythapa commented Feb 12, 2025

mergify bot commented Feb 12, 2025

mergify bot commented Feb 12, 2025

blimmer commented Feb 12, 2025

mergify bot commented Feb 12, 2025

❌ Command disallowed due to command restrictions in the Mergify configuration.

blimmer commented Feb 12, 2025

aws-cdk-automation commented Feb 12, 2025

AWS CodeBuild CI Report

mergify bot commented Feb 12, 2025

github-actions bot commented Feb 12, 2025

codecov bot commented Dec 1, 2024 •

edited

Loading