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

feat(ec2): well-known port aliases #29793

Merged
merged 4 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 10 additions & 9 deletions packages/aws-cdk-lib/aws-ec2/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ const provider = ec2.NatProvider.instanceV2({
new ec2.Vpc(this, 'TheVPC', {
natGatewayProvider: provider,
});
provider.connections.allowFrom(ec2.Peer.ipv4('1.2.3.4/8'), ec2.Port.tcp(80));
provider.connections.allowFrom(ec2.Peer.ipv4('1.2.3.4/8'), ec2.Port.HTTP);
```

You can also customize the characteristics of your NAT instances, including their security group,
Expand Down Expand Up @@ -266,7 +266,7 @@ const provider = ec2.NatProvider.instance({
new ec2.Vpc(this, 'TheVPC', {
natGatewayProvider: provider,
});
provider.connections.allowFrom(ec2.Peer.ipv4('1.2.3.4/8'), ec2.Port.tcp(80));
provider.connections.allowFrom(ec2.Peer.ipv4('1.2.3.4/8'), ec2.Port.HTTP);
```

### Ip Address Management
Expand Down Expand Up @@ -724,13 +724,13 @@ declare const appFleet: autoscaling.AutoScalingGroup;
declare const dbFleet: autoscaling.AutoScalingGroup;

// Allow connections from anywhere
loadBalancer.connections.allowFromAnyIpv4(ec2.Port.tcp(443), 'Allow inbound HTTPS');
loadBalancer.connections.allowFromAnyIpv4(ec2.Port.HTTPS, 'Allow inbound HTTPS');

// The same, but an explicit IP address
loadBalancer.connections.allowFrom(ec2.Peer.ipv4('1.2.3.4/32'), ec2.Port.tcp(443), 'Allow inbound HTTPS');
loadBalancer.connections.allowFrom(ec2.Peer.ipv4('1.2.3.4/32'), ec2.Port.HTTPS, 'Allow inbound HTTPS');

// Allow connection between AutoScalingGroups
appFleet.connections.allowTo(dbFleet, ec2.Port.tcp(443), 'App can call database');
appFleet.connections.allowTo(dbFleet, ec2.Port.HTTPS, 'App can call database');
```

### Connection Peers
Expand All @@ -747,7 +747,7 @@ peer = ec2.Peer.anyIpv4();
peer = ec2.Peer.ipv6('::0/0');
peer = ec2.Peer.anyIpv6();
peer = ec2.Peer.prefixList('pl-12345');
appFleet.connections.allowTo(peer, ec2.Port.tcp(443), 'Allow outbound HTTPS');
appFleet.connections.allowTo(peer, ec2.Port.HTTPS, 'Allow outbound HTTPS');
```

Any object that has a security group can itself be used as a connection peer:
Expand All @@ -758,9 +758,9 @@ declare const fleet2: autoscaling.AutoScalingGroup;
declare const appFleet: autoscaling.AutoScalingGroup;

// These automatically create appropriate ingress and egress rules in both security groups
fleet1.connections.allowTo(fleet2, ec2.Port.tcp(80), 'Allow between fleets');
fleet1.connections.allowTo(fleet2, ec2.Port.HTTP, 'Allow between fleets');

appFleet.connections.allowFromAnyIpv4(ec2.Port.tcp(80), 'Allow from load balancer');
appFleet.connections.allowFromAnyIpv4(ec2.Port.HTTP, 'Allow from load balancer');
```

### Port Ranges
Expand All @@ -770,6 +770,7 @@ the connection specifier:

```ts
ec2.Port.tcp(80)
ec2.Port.HTTPS
ec2.Port.tcpRange(60000, 65535)
ec2.Port.allTcp()
ec2.Port.allIcmp()
Expand Down Expand Up @@ -823,7 +824,7 @@ const mySecurityGroupWithoutInlineRules = new ec2.SecurityGroup(this, 'SecurityG
disableInlineRules: true
});
//This will add the rule as an external cloud formation construct
mySecurityGroupWithoutInlineRules.addIngressRule(ec2.Peer.anyIpv4(), ec2.Port.tcp(22), 'allow ssh access from the world');
mySecurityGroupWithoutInlineRules.addIngressRule(ec2.Peer.anyIpv4(), ec2.Port.SSH, 'allow ssh access from the world');
```

### Importing an existing security group
Expand Down
35 changes: 35 additions & 0 deletions packages/aws-cdk-lib/aws-ec2/lib/port.ts
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,41 @@ export interface PortProps {
* Interface for classes that provide the connection-specification parts of a security group rule
*/
export class Port {
/** Well-known SSH port (TCP 22) */
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

only a nit: do we need "Well-known" in all these? Cleaner to just have SSH port (TCP 22) ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the explicit mention that this is not an arbitrary port and protocol assignment. It's not really needed but I don't feel like it's unwarranted. We could also mention that it comes from the IANA, either replacing or complementing the "well-known"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would have added these at the top of the class/enum JSDoc if they weren't declared alongside the rest of the Port class, but I don't think separating them is worth avoiding the repetition

public static readonly SSH = Port.tcp(22);
/** Well-known SMTP port (TCP 25) */
public static readonly SMTP = Port.tcp(25);
/** Well-known DNS port (UDP 53) */
public static readonly DNS_UDP = Port.udp(53);
/** Well-known DNS port (TCP 53) */
public static readonly DNS_TCP = Port.tcp(53);
/** Well-known HTTP port (TCP 80) */
public static readonly HTTP = Port.tcp(80);
/** Well-known POP3 port (TCP 110) */
public static readonly POP3 = Port.tcp(110);
/** Well-known IMAP port (TCP 143) */
public static readonly IMAP = Port.tcp(143);
/** Well-known LDAP port (TCP 389) */
public static readonly LDAP = Port.tcp(389);
/** Well-known HTTPS port (TCP 443) */
public static readonly HTTPS = Port.tcp(443);
/** Well-known SMB port (TCP 445) */
public static readonly SMB = Port.tcp(445);
/** Well-known IMAPS port (TCP 993) */
public static readonly IMAPS = Port.tcp(993);
/** Well-known POP3S port (TCP 995) */
public static readonly POP3S = Port.tcp(995);
/** Well-known Microsoft SQL Server port (TCP 1433) */
public static readonly MSSQL = Port.tcp(1433);
/** Well-known NFS port (TCP 2049) */
public static readonly NFS = Port.tcp(2049);
/** Well-known MySQL and Aurora port (TCP 3306) */
public static readonly MYSQL_AURORA = Port.tcp(3306);
/** Well-known Microsoft Remote Desktop Protocol port (TCP 3389) */
public static readonly RDP = Port.tcp(3389);
/** Well-known PostgreSQL port (TCP 5432) */
public static readonly POSTGRES = Port.tcp(5432);
Copy link
Contributor Author

@nmussy nmussy Apr 11, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me know if you would like to add more, but I think we should stick to the officially IANA assigned port, even if the AWS web console lists additional ones


/**
* A single TCP port
*/
Expand Down
6 changes: 6 additions & 0 deletions packages/aws-cdk-lib/aws-ec2/test/security-group.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,12 @@ describe('security group', () => {
}],
});
});

test('Static well-known ports are well-defined', () => {
// THEN
expect(Port.SSH).toEqual(Port.tcp(22));
expect(Port.DNS_UDP).toEqual(Port.udp(53));
});
});
});

Expand Down