-
Notifications
You must be signed in to change notification settings - Fork 4k
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
chore(ec2): warn if egress rules are ignored #9127
Conversation
@@ -404,6 +404,7 @@ export class SecurityGroup extends SecurityGroupBase { | |||
// In the case of "allowAllOutbound", we don't add any more rules. There | |||
// is only one rule which allows all traffic and that subsumes any other | |||
// rule. | |||
this.node.addWarning('Ignoring Egress rule since \'allowAllOutbound\' is set to true; To add customize rules, set allowAllOutbound=false on the SecurityGroup'); |
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.
Is it possible to include some info about which egress rule is ignored?
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.
we can add the CIDR
but Im not sure it will make a different since all rules are ignored when allowAllOutbound
is set to true.
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork). |
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
Hi @NetaNir, I run my CDK commands with I recently just upgraded to 1.54.0. This change has now caused all commands to fail in
Is this intended? |
@rrrix, can you share the code that is producing the error, I tested: const cluster = new ecs.Cluster(this, 'EcsCluster');
cluster.addCapacity('DefaultAutoScalingGroup', {
instanceType: new ec2.InstanceType('t2.micro'),
}); and couldn't reproduce. I have added the warning since by default we create the securityGroup with |
If `allowAllOutbound` is set to true, any call to `addEgressRule` will be ignored, this PR adds a warning. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
If
allowAllOutbound
is set to true, any call toaddEgressRule
will be ignored, this PR adds a warning.By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license