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

fix(aws-ec2): fix SecurityGroup's all egress traffic rule #998

Merged
merged 10 commits into from
Oct 25, 2018
Prev Previous commit
Next Next commit
WIP
Rico Huijbers committed Oct 25, 2018
commit 3fc6c75650477c274f3c5107e469ed795c5bba60
10 changes: 5 additions & 5 deletions packages/@aws-cdk/aws-ec2/lib/security-group.ts
Original file line number Diff line number Diff line change
@@ -123,7 +123,7 @@ export interface SecurityGroupProps {
* outbound traffic. If this is set to false, no outbound traffic will be allowed by
* default and all egress traffic must be explicitly authorized.
*
* @default false
* @default true
*/
allowAllOutbound?: boolean;
}
@@ -168,7 +168,7 @@ export class SecurityGroup extends SecurityGroupRef implements ITaggable {
this.tags = new TagManager(this, { initialTags: props.tags});
const groupDescription = props.description || this.path;

this.allowAllOutbound = props.allowAllOutbound || false;
this.allowAllOutbound = props.allowAllOutbound !== false;

this.securityGroup = new cloudformation.SecurityGroupResource(this, 'Resource', {
groupName: props.groupName,
@@ -282,15 +282,15 @@ export class SecurityGroup extends SecurityGroupRef implements ITaggable {
if (this.allowAllOutbound) {
this.directEgressRules.push(ALLOW_ALL_RULE);
} else {
this.directEgressRules.push(BOGUS_RULE);
this.directEgressRules.push(MATCH_NO_TRAFFIC);
}
}

/**
* Remove the bogus rule if it exists
*/
private removeBogusRule() {
const i = this.directEgressRules.findIndex(r => egressRulesEqual(r, BOGUS_RULE));
const i = this.directEgressRules.findIndex(r => egressRulesEqual(r, MATCH_NO_TRAFFIC));
if (i > -1) {
this.directEgressRules.splice(i, 1);
}
@@ -306,7 +306,7 @@ export class SecurityGroup extends SecurityGroupRef implements ITaggable {
* in order to lock it down even more we'll restrict to a nonexistent
* ICMP traffic type.
*/
const BOGUS_RULE = {
const MATCH_NO_TRAFFIC = {
cidrIp: '255.255.255.255/32',
description: 'Disallow all traffic',
ipProtocol: 'icmp',
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-ec2/test/test.connections.ts
Original file line number Diff line number Diff line change
@@ -100,7 +100,7 @@ export = {
const vpc = new VpcNetwork(stack, 'VPC');

// WHEN
const sg = new SecurityGroup(stack, 'SG1', { vpc });
const sg = new SecurityGroup(stack, 'SG1', { vpc, allowAllOutbound: false });
sg.addEgressRule(new AnyIPv4(), new TcpPort(86), 'This replaces the other one');

// THEN