(ec2): allowAllOutbound on default security group should mimic allowAllTraffic on NAT instance

[f0rr0]

Setting allowAllTraffic to false in ec2.NatProvider.instance should set allowAllOutbound on the default security group created for the NAT instance.

Reproduction Steps

ec2.NatProvider.instance({
      instanceType: new ec2.InstanceType('t2.micro'),
      keyName: key.name,
      allowAllTraffic: false,
})

What did you expect to happen?

Default security group associated with NAT instance disallows all outbound connections.

What actually happened?

All outbound connections are allowed on the default security group.

---

This is 🐛 Bug Report

[hedrall]

This is a comment of allowAllTraffic.

aws-cdk/packages/@aws-cdk/aws-ec2/lib/nat.ts

Lines 150 to 162 in 31b1b32

	/**
	* Allow all traffic through the NAT instance
	*
	* If you set this to false, you must configure the NAT instance's security
	* groups in another way, either by passing in a fully configured Security
	* Group using the `securityGroup` property, or by configuring it using the
	* `.securityGroup` or `.connections` members after passing the NAT Instance
	* Provider to a Vpc.
	*
	* @default true
	*/
	readonly allowAllTraffic?: boolean;
	}

According to this,
I also feel that this may be a bit misleading, because it looks like the behavior when allowAllTraffic is false is practically not implemented.

[f0rr0]

The way I (and potentially others) use this option is to let CDK generate a default security group and then configure it manually. However this default security group should not have all outbound connections allowed if the flag is explicitly passed. Pre configuring a security group before creating a NAT instance seems impossible since the preconfigured security group would depend on a VPC which would in turn depend on the NAT instance.

…

________________________________ From: Masaharu Komuro <notifications@github.com> Sent: Saturday, January 23, 2021 1:19:55 PM To: aws/aws-cdk <aws-cdk@noreply.github.com> Cc: Sid <sid_26@outlook.com>; Author <author@noreply.github.com> Subject: Re: [aws/aws-cdk] (ec2): allowAllOutbound on default security group should mimic allowAllTraffic on NAT instance (#12673) This is a comment of allowAllTraffic. https://github.com/aws/aws-cdk/blob/31b1b3289b99c4ec4ed19c51705d66f0e83783dd/packages/%40aws-cdk/aws-ec2/lib/nat.ts#L150-L162 /** * Allow all traffic through the NAT instance * * If you set this to false, you must configure the NAT instance's security * groups in another way, either by passing in a fully configured Security * Group using the `securityGroup` property, or by configuring it using the * `.securityGroup` or `.connections` members after passing the NAT Instance * Provider to a Vpc. * * @default true */ I also feel that this may be a bit misleading, because it looks like the behavior when allowAllTraffic is false is practically not implemented. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub<#12673 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACBNKC3I3AOCAOXD7DZLHN3S3J5SHANCNFSM4WPNHTAA>.

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.