From 5812a14bfb9cb604c16445980dd8be678d7a6f6b Mon Sep 17 00:00:00 2001 From: Rico Huijbers Date: Thu, 16 Aug 2018 17:10:30 +0200 Subject: [PATCH] BREAKING(aws-ec2): SecurityGroup can be used in Connections (#582) Refactoring of the object model for connection/security groups so that a SecurityGroup object can be used as the target of an .allowTo() statement: cluster.connections.allowTo(securityGroup) SecurityGroupRef is now abstract and needs to be constructed using SecurityGroupRef.import(). This is also the mechanism for importing a non-constructed SecurityGroup into the construct tree. As part of the refactoring: - Get rid of IDefaultConnectable, the functionality has been folded into IConnectable/Connections. - Get rid of ISecurityGroup. - Rename IConnectionPeer => ISecurityGroupRule. - Drastically simplify implementation, get rid of recursion and classes to enable the recursion to terminate. All complex logic is now nicely contained within Connections. This change is BREAKING to connections-enabled construct writers, but transparent to application builders. Fixes #579. --- .../aws-ec2/lib/auto-scaling-group.ts | 11 +- packages/@aws-cdk/aws-ec2/lib/connections.ts | 154 ++++++++---------- packages/@aws-cdk/aws-ec2/lib/index.ts | 2 +- .../@aws-cdk/aws-ec2/lib/load-balancer.ts | 31 ++-- .../{connection.ts => security-group-rule.ts} | 28 ++-- .../@aws-cdk/aws-ec2/lib/security-group.ts | 76 ++++++--- .../@aws-cdk/aws-ec2/test/test.connections.ts | 46 +++++- .../@aws-cdk/aws-quickstarts/lib/database.ts | 10 +- packages/@aws-cdk/aws-quickstarts/lib/rdgw.ts | 9 +- packages/@aws-cdk/aws-rds/lib/cluster-ref.ts | 12 +- packages/@aws-cdk/aws-rds/lib/cluster.ts | 9 +- 11 files changed, 211 insertions(+), 177 deletions(-) rename packages/@aws-cdk/aws-ec2/lib/{connection.ts => security-group-rule.ts} (84%) diff --git a/packages/@aws-cdk/aws-ec2/lib/auto-scaling-group.ts b/packages/@aws-cdk/aws-ec2/lib/auto-scaling-group.ts index e59c706822cfa..2da8662dff415 100644 --- a/packages/@aws-cdk/aws-ec2/lib/auto-scaling-group.ts +++ b/packages/@aws-cdk/aws-ec2/lib/auto-scaling-group.ts @@ -2,12 +2,12 @@ import autoscaling = require('@aws-cdk/aws-autoscaling'); import iam = require('@aws-cdk/aws-iam'); import sns = require('@aws-cdk/aws-sns'); import cdk = require('@aws-cdk/cdk'); -import { AllConnections, AnyIPv4, IConnectionPeer } from './connection'; -import { Connections } from './connections'; +import { Connections, IConnectable } from './connections'; import { InstanceType } from './instance-types'; import { ClassicLoadBalancer, IClassicLoadBalancerTarget } from './load-balancer'; import { IMachineImageSource, OperatingSystemType } from './machine-image'; import { SecurityGroup } from './security-group'; +import { AllConnections, AnyIPv4 } from './security-group-rule'; import { VpcNetworkRef, VpcPlacementStrategy } from './vpc-ref'; /** @@ -83,9 +83,7 @@ export interface AutoScalingGroupProps { * * The ASG spans all availability zones. */ -export class AutoScalingGroup extends cdk.Construct implements IClassicLoadBalancerTarget { - public readonly connectionPeer: IConnectionPeer; - +export class AutoScalingGroup extends cdk.Construct implements IClassicLoadBalancerTarget, IConnectable { /** * The type of OS instances of this fleet are running. */ @@ -110,8 +108,7 @@ export class AutoScalingGroup extends cdk.Construct implements IClassicLoadBalan super(parent, name); this.securityGroup = new SecurityGroup(this, 'InstanceSecurityGroup', { vpc: props.vpc }); - this.connections = new Connections(this.securityGroup); - this.connectionPeer = this.securityGroup; + this.connections = new Connections({ securityGroup: this.securityGroup }); if (props.allowAllOutbound !== false) { this.connections.allowTo(new AnyIPv4(), new AllConnections(), 'Outbound traffic allowed by default'); diff --git a/packages/@aws-cdk/aws-ec2/lib/connections.ts b/packages/@aws-cdk/aws-ec2/lib/connections.ts index d7eaca75ae605..6bcdd5fe2975e 100644 --- a/packages/@aws-cdk/aws-ec2/lib/connections.ts +++ b/packages/@aws-cdk/aws-ec2/lib/connections.ts @@ -1,6 +1,5 @@ -import { AnyIPv4, IConnectionPeer, IPortRange } from "./connection"; -import { SecurityGroupId } from "./ec2.generated"; -import { ISecurityGroup } from "./security-group"; +import { SecurityGroupRef } from "./security-group"; +import { AnyIPv4, IPortRange, ISecurityGroupRule } from "./security-group-rule"; /** * The goal of this module is to make possible to write statements like this: @@ -24,33 +23,56 @@ export interface IConnectable { } /** - * An object that has a Connections object as well as a default port range. + * Properties to intialize a new Connections object */ -export interface IDefaultConnectable extends IConnectable { - readonly defaultPortRange: IPortRange; +export interface ConnectionsProps { + /** + * Class that represents the rule by which others can connect to this connectable + * + * This object is required, but will be derived from securityGroup if that is passed. + * + * @default Derived from securityGroup if set. + */ + securityGroupRule?: ISecurityGroupRule; + + /** + * What securityGroup this object is managing connections for + * + * @default No security + */ + securityGroup?: SecurityGroupRef; + + /** + * Default port range for initiating connections to and from this object + * + * @default No default port range + */ + defaultPortRange?: IPortRange; } /** - * Manage the security group (firewall) for a connectable resource. + * Manage the allowed network connections for constructs with Security Groups. * - * This object contains method to allow connections between objects - * that can allow connections. + * Security Groups can be thought of as a firewall for network-connected + * devices. This class makes it easy to allow network connections to and + * from security groups, and between security groups individually. When + * establishing connectivity between security groups, it will automatically + * add rules in both security groups * - * The .allowDefaultPortXxx() methods are only available if the resource - * this object was created for has the concept of a default port range. */ export class Connections { - public readonly connectionPeer: IConnectionPeer; + private readonly securityGroupRule: ISecurityGroupRule; + private readonly securityGroup?: SecurityGroupRef; + private readonly defaultPortRange?: IPortRange; - constructor(private readonly securityGroup: ISecurityGroup, private readonly defaultPortRange?: IPortRange) { - this.connectionPeer = securityGroup; - } + constructor(props: ConnectionsProps) { + if (!props.securityGroupRule && !props.securityGroup) { + throw new Error('Connections: require one of securityGroupRule or securityGroup'); + } - /** - * Allow connections to the peer on their default port - */ - public allowToDefaultPort(other: IDefaultConnectable, description: string) { - this.allowTo(other, other.defaultPortRange, description); + this.securityGroupRule = props.securityGroupRule || props.securityGroup!; + this.securityGroup = props.securityGroup; + this.defaultPortRange = props.defaultPortRange; } /** @@ -58,12 +80,12 @@ export class Connections { */ public allowTo(other: IConnectable, portRange: IPortRange, description: string) { if (this.securityGroup) { - this.securityGroup.addEgressRule(other.connections.connectionPeer, portRange, description); + this.securityGroup.addEgressRule(other.connections.securityGroupRule, portRange, description); + } + if (other.connections.securityGroup) { + other.connections.securityGroup.addIngressRule(this.securityGroupRule, portRange, description); + } - other.connections.allowFrom( - new NullConnectable(this.connectionPeer), - portRange, - description); } /** @@ -71,12 +93,11 @@ export class Connections { */ public allowFrom(other: IConnectable, portRange: IPortRange, description: string) { if (this.securityGroup) { - this.securityGroup.addIngressRule(other.connections.connectionPeer, portRange, description); + this.securityGroup.addIngressRule(other.connections.securityGroupRule, portRange, description); + } + if (other.connections.securityGroup) { + other.connections.securityGroup.addEgressRule(this.securityGroupRule, portRange, description); } - other.connections.allowTo( - new NullConnectable(this.connectionPeer), - portRange, - description); } /** @@ -84,21 +105,21 @@ export class Connections { */ public allowInternally(portRange: IPortRange, description: string) { if (this.securityGroup) { - this.securityGroup.addIngressRule(this.securityGroup, portRange, description); + this.securityGroup.addIngressRule(this.securityGroupRule, portRange, description); } } /** * Allow to all IPv4 ranges */ - public allowToAnyIpv4(portRange: IPortRange, description: string) { + public allowToAnyIPv4(portRange: IPortRange, description: string) { this.allowTo(new AnyIPv4(), portRange, description); } /** * Allow from any IPv4 ranges */ - public allowFromAnyIpv4(portRange: IPortRange, description: string) { + public allowFromAnyIPv4(portRange: IPortRange, description: string) { this.allowFrom(new AnyIPv4(), portRange, description); } @@ -109,7 +130,7 @@ export class Connections { */ public allowDefaultPortFrom(other: IConnectable, description: string) { if (!this.defaultPortRange) { - throw new Error('Cannot call allowDefaultPortFrom(): resource has no default port'); + throw new Error('Cannot call allowDefaultPortFrom(): this resource has no default port'); } this.allowFrom(other, this.defaultPortRange, description); } @@ -119,7 +140,7 @@ export class Connections { */ public allowDefaultPortInternally(description: string) { if (!this.defaultPortRange) { - throw new Error('Cannot call allowDefaultPortInternally(): resource has no default port'); + throw new Error('Cannot call allowDefaultPortInternally(): this resource has no default port'); } this.allowInternally(this.defaultPortRange, description); } @@ -129,62 +150,19 @@ export class Connections { */ public allowDefaultPortFromAnyIpv4(description: string) { if (!this.defaultPortRange) { - throw new Error('Cannot call allowDefaultPortFromAnyIpv4(): resource has no default port'); + throw new Error('Cannot call allowDefaultPortFromAnyIpv4(): this resource has no default port'); } - this.allowFromAnyIpv4(this.defaultPortRange, description); - } -} - -/** - * Connectable that represents a peer but doesn't modify any security groups - */ -class NullConnectable implements IConnectable { - public readonly connections: Connections; - - constructor(connectionPeer: IConnectionPeer) { - this.connections = new SecurityGrouplessConnections(connectionPeer); - } -} - -/** - * This object is used by peers who don't allow reverse connections. - */ -export class SecurityGrouplessConnections extends Connections { - constructor(public readonly connectionPeer: IConnectionPeer) { - // Because Connections is no longer an interface but a concrete class, - // we must inherit from it and create it with an instance of ISecurityGroup. - super(new NullSecurityGroup()); - } - - public allowTo(_other: IConnectable, _connection: IPortRange, _description: string): void { - // Nothing to do + this.allowFromAnyIPv4(this.defaultPortRange, description); } - public allowFrom(_other: IConnectable, _connection: IPortRange, _description: string): void { - // Nothing to do - } -} - -/** - * Instance of ISecurityGroup that's only there for show. - */ -class NullSecurityGroup implements ISecurityGroup { - public securityGroupId: SecurityGroupId = new SecurityGroupId(); - public canInlineRule: boolean = false; - - public addIngressRule(_peer: IConnectionPeer, _connection: IPortRange, _description: string): void { - // Nothing - } - - public addEgressRule(_peer: IConnectionPeer, _connection: IPortRange, _description: string): void { - // Nothing - } - - public toIngressRuleJSON() { - return {}; - } + /** + * Allow connections to the security group on their default port + */ + public allowToDefaultPort(other: IConnectable, description: string) { + if (other.connections.defaultPortRange === undefined) { + throw new Error('Cannot call alloToDefaultPort(): other resource has no default port'); + } - public toEgressRuleJSON() { - return {}; + this.allowTo(other, other.connections.defaultPortRange, description); } } \ No newline at end of file diff --git a/packages/@aws-cdk/aws-ec2/lib/index.ts b/packages/@aws-cdk/aws-ec2/lib/index.ts index 5008448df605c..c293d73b5da50 100644 --- a/packages/@aws-cdk/aws-ec2/lib/index.ts +++ b/packages/@aws-cdk/aws-ec2/lib/index.ts @@ -1,10 +1,10 @@ export * from './auto-scaling-group'; -export * from './connection'; export * from './connections'; export * from './instance-types'; export * from './load-balancer'; export * from './machine-image'; export * from './security-group'; +export * from './security-group-rule'; export * from './vpc'; export * from './vpc-ref'; diff --git a/packages/@aws-cdk/aws-ec2/lib/load-balancer.ts b/packages/@aws-cdk/aws-ec2/lib/load-balancer.ts index 2a84245202b74..f4436294e69fa 100644 --- a/packages/@aws-cdk/aws-ec2/lib/load-balancer.ts +++ b/packages/@aws-cdk/aws-ec2/lib/load-balancer.ts @@ -1,8 +1,8 @@ import elasticloadbalancing = require('@aws-cdk/aws-elasticloadbalancing'); import cdk = require('@aws-cdk/cdk'); -import { AnyIPv4, IConnectionPeer, IPortRange, TcpPort } from './connection'; -import { Connections, IConnectable, IDefaultConnectable } from './connections'; -import { ISecurityGroup, SecurityGroup } from './security-group'; +import { Connections, IConnectable } from './connections'; +import { SecurityGroup, SecurityGroupRef } from './security-group'; +import { AnyIPv4, IPortRange, TcpPort } from './security-group-rule'; import { VpcNetworkRef, VpcSubnetRef } from './vpc-ref'; /** @@ -193,8 +193,6 @@ export class ClassicLoadBalancer extends cdk.Construct implements IConnectable { */ public readonly connections: Connections; - public readonly connectionPeer: IConnectionPeer; - /** * An object controlling specifically the connections for each listener added to this load balancer */ @@ -211,8 +209,7 @@ export class ClassicLoadBalancer extends cdk.Construct implements IConnectable { super(parent, name); this.securityGroup = new SecurityGroup(this, 'SecurityGroup', { vpc: props.vpc }); - this.connections = new Connections(this.securityGroup); - this.connectionPeer = this.securityGroup; + this.connections = new Connections({ securityGroup: this.securityGroup }); // Depending on whether the ELB has public or internal IPs, pick the right backend subnets const subnets: VpcSubnetRef[] = props.internetFacing ? props.vpc.publicSubnets : props.vpc.privateSubnets; @@ -323,17 +320,23 @@ export class ClassicLoadBalancer extends cdk.Construct implements IConnectable { } /** - * Reference to a listener's port just created + * Reference to a listener's port just created. + * + * This implements IConnectable with a default port (the port that an ELB + * listener was just created on) for a given security group so that it can be + * conveniently used just like any Connectable. E.g: + * + * const listener = elb.addListener(...); * - * This class exists to make it convenient to add port ranges to the load - * balancer's security group just for the port ranges that are involved in the - * listener. + * listener.connections.allowDefaultPortFromAnyIPv4(); + * // or + * instance.connections.allowToDefaultPort(listener); */ -export class ClassicListenerPort implements IDefaultConnectable { +export class ClassicListenerPort implements IConnectable { public readonly connections: Connections; - constructor(securityGroup: ISecurityGroup, public readonly defaultPortRange: IPortRange) { - this.connections = new Connections(securityGroup, defaultPortRange); + constructor(securityGroup: SecurityGroupRef, defaultPortRange: IPortRange) { + this.connections = new Connections({ securityGroup, defaultPortRange }); } } diff --git a/packages/@aws-cdk/aws-ec2/lib/connection.ts b/packages/@aws-cdk/aws-ec2/lib/security-group-rule.ts similarity index 84% rename from packages/@aws-cdk/aws-ec2/lib/connection.ts rename to packages/@aws-cdk/aws-ec2/lib/security-group-rule.ts index 2b6c4b69b3ebf..013f30b6d446a 100644 --- a/packages/@aws-cdk/aws-ec2/lib/connection.ts +++ b/packages/@aws-cdk/aws-ec2/lib/security-group-rule.ts @@ -1,10 +1,10 @@ import { Token } from "@aws-cdk/cdk"; -import { Connections, IConnectable, SecurityGrouplessConnections } from "./connections"; +import { Connections, IConnectable } from "./connections"; /** * Interface for classes that provide the peer-specification parts of a security group rule */ -export interface IConnectionPeer { +export interface ISecurityGroupRule { /** * Whether the rule can be inlined into a SecurityGroup or not */ @@ -24,12 +24,11 @@ export interface IConnectionPeer { /** * A connection to and from a given IP range */ -export class CidrIp implements IConnectionPeer, IConnectable { +export class CidrIPv4 implements ISecurityGroupRule, IConnectable { public readonly canInlineRule = true; - public readonly connections: Connections; + public readonly connections: Connections = new Connections({ securityGroupRule: this }); constructor(private readonly cidrIp: string) { - this.connections = new SecurityGrouplessConnections(this); } /** @@ -49,7 +48,7 @@ export class CidrIp implements IConnectionPeer, IConnectable { /** * Any IPv4 address */ -export class AnyIPv4 extends CidrIp { +export class AnyIPv4 extends CidrIPv4 { constructor() { super("0.0.0.0/0"); } @@ -58,12 +57,11 @@ export class AnyIPv4 extends CidrIp { /** * A connection to a from a given IPv6 range */ -export class CidrIpv6 implements IConnectionPeer, IConnectable { +export class CidrIPv6 implements ISecurityGroupRule, IConnectable { public readonly canInlineRule = true; - public readonly connections: Connections; + public readonly connections: Connections = new Connections({ securityGroupRule: this }); constructor(private readonly cidrIpv6: string) { - this.connections = new SecurityGrouplessConnections(this); } /** @@ -83,7 +81,7 @@ export class CidrIpv6 implements IConnectionPeer, IConnectable { /** * Any IPv6 address */ -export class AnyIPv6 extends CidrIpv6 { +export class AnyIPv6 extends CidrIPv6 { constructor() { super("::0/0"); } @@ -98,12 +96,11 @@ export class AnyIPv6 extends CidrIpv6 { * * https://docs.aws.amazon.com/AmazonVPC/latest/UserGuide/vpc-endpoints.html */ -export class PrefixList implements IConnectionPeer, IConnectable { +export class PrefixList implements ISecurityGroupRule, IConnectable { public readonly canInlineRule = true; - public readonly connections: Connections; + public readonly connections: Connections = new Connections({ securityGroupRule: this }); constructor(private readonly prefixListId: string) { - this.connections = new SecurityGrouplessConnections(this); } public toIngressRuleJSON(): any { @@ -119,6 +116,9 @@ export class PrefixList implements IConnectionPeer, IConnectable { * Interface for classes that provide the connection-specification parts of a security group rule */ export interface IPortRange { + /** + * Whether the rule containing this port range can be inlined into a securitygroup or not. + */ readonly canInlineRule: boolean; /** @@ -220,4 +220,4 @@ export class AllConnections implements IPortRange { toPort: -1, }; } -} +} \ No newline at end of file diff --git a/packages/@aws-cdk/aws-ec2/lib/security-group.ts b/packages/@aws-cdk/aws-ec2/lib/security-group.ts index 4d4f67dccd581..b506df17aae46 100644 --- a/packages/@aws-cdk/aws-ec2/lib/security-group.ts +++ b/packages/@aws-cdk/aws-ec2/lib/security-group.ts @@ -1,6 +1,7 @@ -import { Construct, Token } from '@aws-cdk/cdk'; -import { IConnectionPeer, IPortRange } from './connection'; +import { Construct, Output, Token } from '@aws-cdk/cdk'; +import { Connections, IConnectable } from './connections'; import { cloudformation, SecurityGroupId, SecurityGroupVpcId } from './ec2.generated'; +import { IPortRange, ISecurityGroupRule } from './security-group-rule'; import { slugify } from './util'; import { VpcNetworkRef } from './vpc-ref'; @@ -11,30 +12,27 @@ export interface SecurityGroupRefProps { securityGroupId: SecurityGroupId; } -/** - * Basic interface for security groups - */ -export interface ISecurityGroup extends IConnectionPeer { - readonly securityGroupId: SecurityGroupId; - - addIngressRule(peer: IConnectionPeer, connection: IPortRange, description: string): void; - addEgressRule(peer: IConnectionPeer, connection: IPortRange, description: string): void; -} - /** * A SecurityGroup that is not created in this template */ -export class SecurityGroupRef extends Construct implements ISecurityGroup { - public readonly securityGroupId: SecurityGroupId; - public readonly canInlineRule = false; +export abstract class SecurityGroupRef extends Construct implements ISecurityGroupRule, IConnectable { + /** + * Import an existing SecurityGroup + */ + public static import(parent: Construct, id: string, props: SecurityGroupRefProps): SecurityGroupRef { + return new ImportedSecurityGroup(parent, id, props); + } - constructor(parent: Construct, name: string, props: SecurityGroupRefProps) { - super(parent, name); + public abstract readonly securityGroupId: SecurityGroupId; + public readonly canInlineRule = false; + public readonly connections = new Connections({ securityGroup: this }); - this.securityGroupId = props.securityGroupId; - } + /** + * FIXME: Where to place this?? + */ + public readonly defaultPortRange?: IPortRange; - public addIngressRule(peer: IConnectionPeer, connection: IPortRange, description: string) { + public addIngressRule(peer: ISecurityGroupRule, connection: IPortRange, description: string) { new cloudformation.SecurityGroupIngressResource(this, slugify(description), { groupId: this.securityGroupId, ...peer.toIngressRuleJSON(), @@ -43,7 +41,7 @@ export class SecurityGroupRef extends Construct implements ISecurityGroup { }); } - public addEgressRule(peer: IConnectionPeer, connection: IPortRange, description: string) { + public addEgressRule(peer: ISecurityGroupRule, connection: IPortRange, description: string) { new cloudformation.SecurityGroupEgressResource(this, slugify(description), { groupId: this.securityGroupId, ...peer.toEgressRuleJSON(), @@ -59,6 +57,16 @@ export class SecurityGroupRef extends Construct implements ISecurityGroup { public toEgressRuleJSON(): any { return { destinationSecurityGroupId: this.securityGroupId }; } + + /** + * Export this SecurityGroup for use in a different Stack + */ + public export(): SecurityGroupRefProps { + return { + securityGroupId: new Output(this, 'SecurityGroupId', { value: this.securityGroupId }).makeImportValue() + }; + } + } export interface SecurityGroupProps { @@ -105,15 +113,19 @@ export class SecurityGroup extends SecurityGroupRef { */ public readonly vpcId: SecurityGroupVpcId; + /** + * The ID of the security group + */ + public readonly securityGroupId: SecurityGroupId; + private readonly securityGroup: cloudformation.SecurityGroupResource; private readonly directIngressRules: cloudformation.SecurityGroupResource.IngressProperty[] = []; private readonly directEgressRules: cloudformation.SecurityGroupResource.EgressProperty[] = []; constructor(parent: Construct, name: string, props: SecurityGroupProps) { - super(parent, name, { securityGroupId: new Token(() => this.securityGroup.securityGroupId) }); + super(parent, name); const groupDescription = props.description || this.path; - this.securityGroup = new cloudformation.SecurityGroupResource(this, 'Resource', { groupName: props.groupName, groupDescription, @@ -122,11 +134,12 @@ export class SecurityGroup extends SecurityGroupRef { vpcId: props.vpc.vpcId, }); + this.securityGroupId = this.securityGroup.securityGroupId; this.groupName = this.securityGroup.ref; this.vpcId = this.securityGroup.securityGroupVpcId; } - public addIngressRule(peer: IConnectionPeer, connection: IPortRange, description: string) { + public addIngressRule(peer: ISecurityGroupRule, connection: IPortRange, description: string) { if (!peer.canInlineRule || !connection.canInlineRule) { super.addIngressRule(peer, connection, description); return; @@ -139,7 +152,7 @@ export class SecurityGroup extends SecurityGroupRef { }); } - public addEgressRule(peer: IConnectionPeer, connection: IPortRange, description: string) { + public addEgressRule(peer: ISecurityGroupRule, connection: IPortRange, description: string) { if (!peer.canInlineRule || !connection.canInlineRule) { super.addEgressRule(peer, connection, description); return; @@ -228,6 +241,19 @@ export interface ConnectionRule { description?: string; } +/** + * A SecurityGroup that hasn't been created here + */ +class ImportedSecurityGroup extends SecurityGroupRef { + public readonly securityGroupId: SecurityGroupId; + + constructor(parent: Construct, name: string, props: SecurityGroupRefProps) { + super(parent, name); + + this.securityGroupId = props.securityGroupId; + } +} + /** * Compare two ingress rules for equality the same way CloudFormation would (discarding description) */ diff --git a/packages/@aws-cdk/aws-ec2/test/test.connections.ts b/packages/@aws-cdk/aws-ec2/test/test.connections.ts index 50e9d82729a2d..5f9e3d0e36045 100644 --- a/packages/@aws-cdk/aws-ec2/test/test.connections.ts +++ b/packages/@aws-cdk/aws-ec2/test/test.connections.ts @@ -1,6 +1,8 @@ +import { expect, haveResource } from '@aws-cdk/assert'; import { Stack } from '@aws-cdk/cdk'; import { Test } from 'nodeunit'; -import { Connections, IConnectable, SecurityGroup, TcpPort, VpcNetwork } from '../lib'; +import { AmazonLinuxImage, AutoScalingGroup, Connections, IConnectable, InstanceType, SecurityGroup, + SecurityGroupId, SecurityGroupRef, TcpAllPorts, TcpPort, VpcNetwork } from '../lib'; export = { 'peering between two security groups does not recursive infinitely'(test: Test) { @@ -11,13 +13,51 @@ export = { const sg1 = new SecurityGroup(stack, 'SG1', { vpc }); const sg2 = new SecurityGroup(stack, 'SG2', { vpc }); - const conn1 = new ConnectionsHolder(new Connections(sg1)); - const conn2 = new ConnectionsHolder(new Connections(sg2)); + const conn1 = new ConnectionsHolder(new Connections({ securityGroup: sg1 })); + const conn2 = new ConnectionsHolder(new Connections({ securityGroup: sg2 })); // WHEN conn1.connections.allowTo(conn2, new TcpPort(80), 'Test'); // THEN + test.done(); + }, + + '(imported) SecurityGroup can be used as target of .allowTo()'(test: Test) { + // GIVEN + const stack = new Stack(); + const vpc = new VpcNetwork(stack, 'VPC'); + const asg = new AutoScalingGroup(stack, 'ASG', { + instanceType: new InstanceType('t-1000'), + machineImage: new AmazonLinuxImage(), + vpc + }); + + const securityGroup = SecurityGroupRef.import(stack, 'ImportedSG', { securityGroupId: new SecurityGroupId('sg-12345') }); + + // WHEN + asg.connections.allowTo(securityGroup, new TcpAllPorts(), 'Connect there'); + + // THEN: rule to generated security group to connect to imported + expect(stack).to(haveResource("AWS::EC2::SecurityGroupEgress", { + GroupId: { "Fn::GetAtt": [ "ASGInstanceSecurityGroup0525485D", "GroupId" ] }, + IpProtocol: "tcp", + Description: "Connect there", + DestinationSecurityGroupId: "sg-12345", + FromPort: 0, + ToPort: 65535 + })); + + // THEN: rule to imported security group to allow connections from generated + expect(stack).to(haveResource("AWS::EC2::SecurityGroupIngress", { + IpProtocol: "tcp", + Description: "Connect there", + FromPort: 0, + GroupId: "sg-12345", + SourceSecurityGroupId: { "Fn::GetAtt": [ "ASGInstanceSecurityGroup0525485D", "GroupId" ] }, + ToPort: 65535 + })); + test.done(); } }; diff --git a/packages/@aws-cdk/aws-quickstarts/lib/database.ts b/packages/@aws-cdk/aws-quickstarts/lib/database.ts index 96c4f911afa6b..f57d135d9ef1f 100644 --- a/packages/@aws-cdk/aws-quickstarts/lib/database.ts +++ b/packages/@aws-cdk/aws-quickstarts/lib/database.ts @@ -20,13 +20,11 @@ export interface SqlServerProps { export class SqlServer extends cdk.Construct implements ec2.IConnectable { private static readonly PORT = 1433; public readonly connections: ec2.Connections; - public readonly defaultPortRange: ec2.IPortRange; - private readonly securityGroup: ec2.SecurityGroup; constructor(parent: cdk.Construct, name: string, props: SqlServerProps) { super(parent, name); - this.securityGroup = new ec2.SecurityGroup(this, 'SecurityGroup', { + const securityGroup = new ec2.SecurityGroup(this, 'SecurityGroup', { vpc: props.vpc, description: 'Database security group', }); @@ -54,10 +52,10 @@ export class SqlServer extends cdk.Construct implements ec2.IConnectable { masterUserPassword: p.masterPassword, port: SqlServer.PORT.toString(), dbSubnetGroupName: subnetGroup.ref, - vpcSecurityGroups: [ this.securityGroup.securityGroupId ] + vpcSecurityGroups: [ securityGroup.securityGroupId ] }); - this.defaultPortRange = new ec2.TcpPort(SqlServer.PORT); - this.connections = new ec2.Connections(this.securityGroup, this.defaultPortRange); + const defaultPortRange = new ec2.TcpPort(SqlServer.PORT); + this.connections = new ec2.Connections({ securityGroup, defaultPortRange }); } } diff --git a/packages/@aws-cdk/aws-quickstarts/lib/rdgw.ts b/packages/@aws-cdk/aws-quickstarts/lib/rdgw.ts index 004feca506547..d7bca3737dbe1 100644 --- a/packages/@aws-cdk/aws-quickstarts/lib/rdgw.ts +++ b/packages/@aws-cdk/aws-quickstarts/lib/rdgw.ts @@ -20,10 +20,9 @@ export interface RemoteDesktopGatewayProps { /** * Embed the Remote Desktop Gateway AWS QuickStart */ -export class RemoteDesktopGateway extends cdk.Construct implements ec2.IDefaultConnectable { +export class RemoteDesktopGateway extends cdk.Construct implements ec2.IConnectable { private static readonly PORT = 3389; public readonly connections: ec2.Connections; - public readonly defaultPortRange: ec2.IPortRange; constructor(parent: cdk.Construct, name: string, props: RemoteDesktopGatewayProps) { super(parent, name); @@ -48,11 +47,11 @@ export class RemoteDesktopGateway extends cdk.Construct implements ec2.IDefaultC parameters: params }); - const securityGroup = new ec2.SecurityGroupRef(this, 'SecurityGroup', { + const securityGroup = ec2.SecurityGroupRef.import(this, 'SecurityGroup', { securityGroupId: nestedStack.getAtt('Outputs.RemoteDesktopGatewaySGID') }); - this.defaultPortRange = new ec2.TcpPort(RemoteDesktopGateway.PORT); - this.connections = new ec2.Connections(securityGroup, this.defaultPortRange); + const defaultPortRange = new ec2.TcpPort(RemoteDesktopGateway.PORT); + this.connections = new ec2.Connections({ securityGroup, defaultPortRange }); } } diff --git a/packages/@aws-cdk/aws-rds/lib/cluster-ref.ts b/packages/@aws-cdk/aws-rds/lib/cluster-ref.ts index 747afffba2776..3ad5fe24036cb 100644 --- a/packages/@aws-cdk/aws-rds/lib/cluster-ref.ts +++ b/packages/@aws-cdk/aws-rds/lib/cluster-ref.ts @@ -5,7 +5,7 @@ import { DBClusterEndpointAddress } from './rds.generated'; /** * Create a clustered database with a given number of instances. */ -export abstract class DatabaseClusterRef extends cdk.Construct implements ec2.IDefaultConnectable { +export abstract class DatabaseClusterRef extends cdk.Construct implements ec2.IConnectable { /** * Import an existing DatabaseCluster from properties */ @@ -13,11 +13,6 @@ export abstract class DatabaseClusterRef extends cdk.Construct implements ec2.ID return new ImportedDatabaseCluster(parent, name, props); } - /** - * Default port to connect to this database - */ - public abstract readonly defaultPortRange: ec2.IPortRange; - /** * Access to the network connections */ @@ -159,7 +154,10 @@ class ImportedDatabaseCluster extends DatabaseClusterRef { this.securityGroupId = props.securityGroupId; this.defaultPortRange = new ec2.TcpPortFromAttribute(props.port); - this.connections = new ec2.Connections(new ec2.SecurityGroupRef(this, 'SecurityGroup', props), this.defaultPortRange); + this.connections = new ec2.Connections({ + securityGroup: ec2.SecurityGroupRef.import(this, 'SecurityGroup', props), + defaultPortRange: this.defaultPortRange + }); this.clusterIdentifier = props.clusterIdentifier; this.clusterEndpoint = new Endpoint(props.clusterEndpointAddress, props.port); this.readerEndpoint = new Endpoint(props.readerEndpointAddress, props.port); diff --git a/packages/@aws-cdk/aws-rds/lib/cluster.ts b/packages/@aws-cdk/aws-rds/lib/cluster.ts index 80ef230dbd255..c85c3238df141 100644 --- a/packages/@aws-cdk/aws-rds/lib/cluster.ts +++ b/packages/@aws-cdk/aws-rds/lib/cluster.ts @@ -117,11 +117,6 @@ export class DatabaseCluster extends DatabaseClusterRef { */ public readonly instanceEndpoints: Endpoint[] = []; - /** - * Default port to connect to this database - */ - public readonly defaultPortRange: ec2.IPortRange; - /** * Access to the network connections */ @@ -214,8 +209,8 @@ export class DatabaseCluster extends DatabaseClusterRef { this.instanceEndpoints.push(new Endpoint(instance.dbInstanceEndpointAddress, instance.dbInstanceEndpointPort)); } - this.defaultPortRange = new ec2.TcpPortFromAttribute(this.clusterEndpoint.port); - this.connections = new ec2.Connections(securityGroup, this.defaultPortRange); + const defaultPortRange = new ec2.TcpPortFromAttribute(this.clusterEndpoint.port); + this.connections = new ec2.Connections({ securityGroup, defaultPortRange }); } }