fix(ecs): outdated linux commands for `canContainersAccessInstanceRole=false` and also deprecate property #32763

samson-keung · 2025-01-06T23:12:14Z

Issue # (if applicable)

Reason for this change

When canContainersAccessInstanceRole=false, wrong commands are added to the ASG UserData, as described in the issue linked above.

Reason for deprecating the canContainersAccessInstanceRole option is detailed in #32609.

Description of changes

Added deprecation tag to all canContainersAccessInstanceRole options.
Created two feature flags to control the canContainersAccessInstanceRole behaviour
Added new commands if customer opted to use them via setting the right feature flags

Describe any new or updated permissions being added

None

Description of how you validated changes

A new integ test is added to ensure the commands in UserData executes without throwing errors.

The existing integ tests are updated to have the default feature flag values and they are passing. This should prove that there will not be any change to existing CDK apps.

Unit tests are added for each platform, combination of possible values for canContainersAccessInstanceRole + possible values for the feature flags (3 platforms * 3 possible values for canContainersAccessInstanceRole * 2 possible values for @aws-cdk/aws-ecs:disableEcsImdsBlocking * 2 possible values for @aws-cdk/aws-ecs:enableImdsBlockingDeprecatedFeature = 36 unit tests).

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

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 · 2025-01-06T23:32:24Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.52%. Comparing base (3d56efa) to head (6690e25).
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #32763   +/-   ##
=======================================
  Coverage   81.52%   81.52%           
=======================================
  Files         222      222           
  Lines       13715    13715           
  Branches     2417     2417           
=======================================
  Hits        11181    11181           
  Misses       2254     2254           
  Partials      280      280

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

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

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

godwingrs22

Thanks @samson-keung for the contribution. LGTM. Just left a nit comment.

godwingrs22 · 2025-01-07T00:53:56Z

packages/aws-cdk-lib/aws-ecs/lib/cluster.ts

+        // new commands
+        autoScalingGroup.addUserData('sudo yum install -y iptables-services; sudo iptables --insert DOCKER-USER 1 --in-interface docker+ --destination 169.254.169.254/32 --jump DROP');
+        autoScalingGroup.addUserData('sudo iptables-save | sudo tee /etc/sysconfig/iptables && sudo systemctl enable --now iptables');


nit: May be it would be nice to add the official AWS link doc as a comment above this line from where these commands were taken.

https://docs.aws.amazon.com/AmazonECS/latest/developerguide/task-iam-roles.html#task-iam-role-considerations

packages/aws-cdk-lib/aws-ecs/lib/cluster.ts

mergify · 2025-01-07T21:09:31Z

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-01-07T21:10:07Z

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

mergify · 2025-01-07T21:12:54Z

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-01-07T22:18: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).

aws-cdk-automation · 2025-01-07T22:49:56Z

AWS CodeBuild CI Report

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

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

mergify · 2025-01-07T22:50:19Z

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-01-07T22:50:41Z

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.

ecs canContainersAccessInstanceRole fix

ae11687

github-actions bot added bug This issue is a bug. effort/medium Medium work item – several days of effort p1 labels Jan 6, 2025

aws-cdk-automation requested a review from a team January 6, 2025 23:12

mergify bot added the contribution/core This is a PR that came from AWS. label Jan 6, 2025

godwingrs22 changed the title ~~fix(ecs): canContainersAccessInstanceRole=false adds wrong commands for AL2023~~ fix(ecs): Outdated Linux commands for canContainersAccessInstanceRole=false and also deprecate property Jan 6, 2025

aws-cdk-automation previously requested changes Jan 6, 2025

View reviewed changes

godwingrs22 self-requested a review January 6, 2025 23:26

godwingrs22 changed the title ~~fix(ecs): Outdated Linux commands for canContainersAccessInstanceRole=false and also deprecate property~~ fix(ecs): uutdated linux commands for canContainersAccessInstanceRole=false and also deprecate property Jan 6, 2025

godwingrs22 changed the title ~~fix(ecs): uutdated linux commands for canContainersAccessInstanceRole=false and also deprecate property~~ fix(ecs): outdated linux commands for canContainersAccessInstanceRole=false and also deprecate property Jan 6, 2025

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 6, 2025

godwingrs22 reviewed Jan 7, 2025

View reviewed changes

samson-keung commented Jan 7, 2025

View reviewed changes

packages/aws-cdk-lib/aws-ecs/lib/cluster.ts Outdated Show resolved Hide resolved

Add link to ECS doc in code comment

3b14355

godwingrs22 approved these changes Jan 7, 2025

View reviewed changes

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Jan 7, 2025

Merge branch 'main' into ecs-container-access

55460ad

Merge branch 'main' into ecs-container-access

6690e25

mergify bot merged commit bbdd42c into aws:main Jan 7, 2025
19 checks passed

github-actions bot locked as resolved and limited conversation to collaborators Jan 7, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(ecs): outdated linux commands for `canContainersAccessInstanceRole=false` and also deprecate property #32763

fix(ecs): outdated linux commands for `canContainersAccessInstanceRole=false` and also deprecate property #32763

samson-keung commented Jan 6, 2025

aws-cdk-automation left a comment

codecov bot commented Jan 6, 2025 •

edited

Loading

godwingrs22 left a comment

godwingrs22 Jan 7, 2025

mergify bot commented Jan 7, 2025

mergify bot commented Jan 7, 2025

mergify bot commented Jan 7, 2025

mergify bot commented Jan 7, 2025

aws-cdk-automation commented Jan 7, 2025

mergify bot commented Jan 7, 2025

github-actions bot commented Jan 7, 2025

fix(ecs): outdated linux commands for canContainersAccessInstanceRole=false and also deprecate property #32763

fix(ecs): outdated linux commands for canContainersAccessInstanceRole=false and also deprecate property #32763

Conversation

samson-keung commented Jan 6, 2025

Issue # (if applicable)

Reason for this change

Description of changes

Describe any new or updated permissions being added

Description of how you validated changes

Checklist

aws-cdk-automation left a comment

Choose a reason for hiding this comment

codecov bot commented Jan 6, 2025 • edited Loading

Codecov Report

godwingrs22 left a comment

Choose a reason for hiding this comment

godwingrs22 Jan 7, 2025

Choose a reason for hiding this comment

mergify bot commented Jan 7, 2025

mergify bot commented Jan 7, 2025

mergify bot commented Jan 7, 2025

mergify bot commented Jan 7, 2025

aws-cdk-automation commented Jan 7, 2025

AWS CodeBuild CI Report

mergify bot commented Jan 7, 2025

github-actions bot commented Jan 7, 2025

fix(ecs): outdated linux commands for `canContainersAccessInstanceRole=false` and also deprecate property #32763

fix(ecs): outdated linux commands for `canContainersAccessInstanceRole=false` and also deprecate property #32763

codecov bot commented Jan 6, 2025 •

edited

Loading