-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
fix(ecs): default ecsmanagedtags and propagatetags to be undefined #3887
Conversation
Pull Request Checklist
|
Pull Request Checklist
|
Codebuild (Continuous Integration) build failed for current commits. Please check log and resolve before PR is merged. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM pending build + tests pass
Pull request has been modified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM pending build + tests pass
Pull request has been modified.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for making this change! I think these defaults are more user friendly at this time.
Have you considered maybe adding a warning as a metadata entry to the ConstructNode
to guide customers towards opting into using longer arns? That might be a friendlier way to guide them and make them aware that an enforcement date is looming.
I definitely agree, but I'm leaning towards doing this in a separate PR once we have a date. Want to unblock users who are having this issue and get this into the next release. |
Thank you for contributing! Your pull request is now being automatically merged. |
Thanks so much for taking the time to contribute to the AWS CDK ❤️ We will shortly assign someone to review this pull request and help get it
|
EcsManagedTags require customers to have opted in to using long arns, but for customer who do not have long arns, it will fail their build.
PR: #3420 set enabledECSManagedTags to true by default and set propagateTags to the Service. Updated the defaults to be undefined for propagateTags and false for enableECSManagedTags. If a user wishes to enable either, they can manually override the properties.
Addresses: #3844
Testing:
Outputs:
aws-ecs-integ.L3LoadBalancerDNS####### = #########.us-west-2.elb.amazonaws.com
Stack ARN:
arn:aws:cloudformation:us-west-2:############:stack/aws-ecs-integ/#########-####-####-####-#########
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license