feat(ecs,lambda,rds): specify allowAllOutbound when importing security groups (#3833)

* fix(lambda/rds): expose allowAllOutbound prop when importing

* fix and test ecs

* test lambda

* test elbv2

* test rds

* deprecate in rds

* deprecate in lambda

* deprecate in elbv2

* JSDoc

* JSDoc

* remove securityGroupId in rds

BREAKING CHANGE: `securityGroupId: string` replaced by `securityGroup: ISecurityGroup` when
importing a cluster/instance in `@aws-cdk/aws-rds`

Author: jogold

packages/@aws-cdk/aws-ecs/lib/cluster.ts

@@ -581,11 +581,9 @@ class ImportedCluster extends Resource implements ICluster {
       resourceName: props.clusterName
     });
 
-    let i = 1;
-    for (const sgProps of props.securityGroups) {
-      this.connections.addSecurityGroup(ec2.SecurityGroup.fromSecurityGroupId(this, `SecurityGroup${i}`, sgProps.securityGroupId));
-      i++;
-    }
+    this.connections = new ec2.Connections({
+      securityGroups: props.securityGroups
+    });
   }
 
   public get defaultCloudMapNamespace(): cloudmap.INamespace | undefined {

packages/@aws-cdk/aws-ecs/test/test.ecs-cluster.ts

@@ -1,4 +1,4 @@
-import { expect, haveResource } from '@aws-cdk/assert';
+import { countResources, expect, haveResource } from '@aws-cdk/assert';
 import ec2 = require('@aws-cdk/aws-ec2');
 import { InstanceType } from '@aws-cdk/aws-ec2';
 import cloudmap = require('@aws-cdk/aws-servicediscovery');
@@ -1099,6 +1099,33 @@ export = {
     test.done();
   },
 
+  'imported cluster with imported security groups honors allowAllOutbound'(test: Test) {
+    // GIVEN
+    const stack = new cdk.Stack();
+    const vpc = new ec2.Vpc(stack, 'Vpc');
+
+    const importedSg1 = ec2.SecurityGroup.fromSecurityGroupId(stack, 'SG1', 'sg-1', { allowAllOutbound: false });
+    const importedSg2 = ec2.SecurityGroup.fromSecurityGroupId(stack, 'SG2', 'sg-2');
+
+    const cluster = ecs.Cluster.fromClusterAttributes(stack, 'Cluster', {
+      clusterName: 'cluster-name',
+      securityGroups: [importedSg1, importedSg2],
+      vpc,
+    });
+
+    // WHEN
+    cluster.connections.allowToAnyIpv4(ec2.Port.tcp(443));
+
+    // THEN
+    expect(stack).to(haveResource('AWS::EC2::SecurityGroupEgress', {
+      GroupId: 'sg-1'
+    }));
+
+    expect(stack).to(countResources('AWS::EC2::SecurityGroupEgress', 1));
+
+    test.done();
+  },
+
   "Metric"(test: Test) {
     // GIVEN
     const stack = new cdk.Stack();

packages/@aws-cdk/aws-elasticloadbalancingv2/lib/alb/application-listener.ts

@@ -327,20 +327,30 @@ export interface ApplicationListenerAttributes {
 
   /**
    * Security group ID of the load balancer this listener is associated with
+   *
+   * @deprecated use `securityGroup` instead
+   */
+  readonly securityGroupId?: string;
+
+  /**
+   * Security group of the load balancer this listener is associated with
    */
-  readonly securityGroupId: string;
+  readonly securityGroup?: ec2.ISecurityGroup;
 
   /**
    * The default port on which this listener is listening
    */
   readonly defaultPort?: number;
 
   /**
-   * Whether the security group allows all outbound traffic or not
+   * Whether the imported security group allows all outbound traffic or not when
+   * imported using `securityGroupId`
    *
    * Unless set to `false`, no egress rules will be added to the security group.
    *
    * @default true
+   *
+   * @deprecated use `securityGroup` instead
    */
   readonly securityGroupAllowsAllOutbound?: boolean;
 }
@@ -360,10 +370,19 @@ class ImportedApplicationListener extends Resource implements IApplicationListen
 
     const defaultPort = props.defaultPort !== undefined ? ec2.Port.tcp(props.defaultPort) : undefined;
 
-    this.connections = new ec2.Connections({
-      securityGroups: [ec2.SecurityGroup.fromSecurityGroupId(this, 'SecurityGroup', props.securityGroupId, {
+    let securityGroup: ec2.ISecurityGroup;
+    if (props.securityGroup) {
+      securityGroup = props.securityGroup;
+    } else if (props.securityGroupId) {
+      securityGroup = ec2.SecurityGroup.fromSecurityGroupId(scope, 'SecurityGroup', props.securityGroupId, {
         allowAllOutbound: props.securityGroupAllowsAllOutbound
-      })],
+      });
+    } else {
+      throw new Error('Either `securityGroup` or `securityGroupId` must be specified to import an application listener.');
+    }
+
+    this.connections = new ec2.Connections({
+      securityGroups: [securityGroup],
       defaultPort,
     });
   }

packages/@aws-cdk/aws-elasticloadbalancingv2/test/alb/test.listener.ts

@@ -624,7 +624,29 @@ export = {
     }), /`targetGroups`.*`fixedResponse`/);
 
     test.done();
-  }
+  },
+
+  'Imported listener with imported security group and allowAllOutbound set to false'(test: Test) {
+    // GIVEN
+    const stack = new cdk.Stack();
+    const listener = elbv2.ApplicationListener.fromApplicationListenerAttributes(stack, 'Listener', {
+      listenerArn: 'listener-arn',
+      defaultPort: 443,
+      securityGroup: ec2.SecurityGroup.fromSecurityGroupId(stack, 'SG', 'security-group-id', {
+        allowAllOutbound: false,
+      }),
+    });
+
+    // WHEN
+    listener.connections.allowToAnyIpv4(ec2.Port.tcp(443));
+
+    // THEN
+    expect(stack).to(haveResource('AWS::EC2::SecurityGroupEgress', {
+      GroupId: 'security-group-id'
+    }));
+
+    test.done();
+  },
 };
 
 class ResourceWithLBDependency extends cdk.CfnResource {

packages/@aws-cdk/aws-lambda/lib/function-base.ts

@@ -114,12 +114,22 @@ export interface FunctionAttributes {
   readonly role?: iam.IRole;
 
   /**
-   * Id of the securityGroup for this Lambda, if in a VPC.
+   * Id of the security group of this Lambda, if in a VPC.
    *
    * This needs to be given in order to support allowing connections
    * to this Lambda.
+   *
+   * @deprecated use `securityGroup` instead
    */
   readonly securityGroupId?: string;
+
+  /**
+   * The security group of this Lambda, if in a VPC.
+   *
+   * This needs to be given in order to support allowing connections
+   * to this Lambda.
+   */
+  readonly securityGroup?: ec2.ISecurityGroup;
 }
 
 export abstract class FunctionBase extends Resource implements IFunction {

packages/@aws-cdk/aws-lambda/lib/function.ts

@@ -275,11 +275,13 @@ export class Function extends FunctionBase {
 
         this.grantPrincipal = role || new iam.UnknownPrincipal({ resource: this } );
 
-        if (attrs.securityGroupId) {
+        if (attrs.securityGroup) {
           this._connections = new ec2.Connections({
-            securityGroups: [
-              ec2.SecurityGroup.fromSecurityGroupId(this, 'SecurityGroup', attrs.securityGroupId)
-            ]
+            securityGroups: [attrs.securityGroup]
+          });
+        } else if (attrs.securityGroupId) {
+          this._connections = new ec2.Connections({
+            securityGroups: [ec2.SecurityGroup.fromSecurityGroupId(scope, 'SecurityGroup', attrs.securityGroupId)]
           });
         }
       }

packages/@aws-cdk/aws-lambda/test/test.lambda.ts

@@ -1,4 +1,5 @@
 import { expect, haveResource, MatchStyle, ResourcePart } from '@aws-cdk/assert';
+import ec2 = require('@aws-cdk/aws-ec2');
 import iam = require('@aws-cdk/aws-iam');
 import logs = require('@aws-cdk/aws-logs');
 import sqs = require('@aws-cdk/aws-sqs');
@@ -1372,7 +1373,29 @@ export = {
     }));
 
     test.done();
-   }
+   },
+
+   'imported lambda with imported security group and allowAllOutbound set to false'(test: Test) {
+    // GIVEN
+    const stack = new cdk.Stack();
+
+    const fn = lambda.Function.fromFunctionAttributes(stack, 'fn', {
+      functionArn: 'arn:aws:lambda:us-east-1:123456789012:function:my-function',
+      securityGroup: ec2.SecurityGroup.fromSecurityGroupId(stack, 'SG', 'sg-123456789', {
+        allowAllOutbound: false,
+      }),
+    });
+
+    // WHEN
+    fn.connections.allowToAnyIpv4(ec2.Port.tcp(443));
+
+    // THEN
+    expect(stack).to(haveResource('AWS::EC2::SecurityGroupEgress', {
+      GroupId: 'sg-123456789',
+    }));
+
+    test.done();
+  }
 };
 
 function newTestLambda(scope: cdk.Construct) {

packages/@aws-cdk/aws-rds/lib/cluster-ref.ts

@@ -50,9 +50,9 @@ export interface DatabaseClusterAttributes {
   readonly port: number;
 
   /**
-   * The security group for this database cluster
+   * The security group of the database cluster
    */
-  readonly securityGroupId: string;
+  readonly securityGroup: ec2.ISecurityGroup;
 
   /**
    * Identifier for the cluster

packages/@aws-cdk/aws-rds/lib/cluster.ts

@@ -192,15 +192,15 @@ export class DatabaseCluster extends DatabaseClusterBase {
     class Import extends DatabaseClusterBase implements IDatabaseCluster {
       public readonly defaultPort = ec2.Port.tcp(attrs.port);
       public readonly connections = new ec2.Connections({
-        securityGroups: [ec2.SecurityGroup.fromSecurityGroupId(this, 'SecurityGroup', attrs.securityGroupId)],
+        securityGroups: [attrs.securityGroup],
         defaultPort: this.defaultPort
       });
       public readonly clusterIdentifier = attrs.clusterIdentifier;
       public readonly instanceIdentifiers: string[] = [];
       public readonly clusterEndpoint = new Endpoint(attrs.clusterEndpointAddress, attrs.port);
       public readonly clusterReadEndpoint = new Endpoint(attrs.readerEndpointAddress, attrs.port);
       public readonly instanceEndpoints = attrs.instanceEndpointAddresses.map(a => new Endpoint(a, attrs.port));
-      public readonly securityGroupId = attrs.securityGroupId;
+      public readonly securityGroupId = attrs.securityGroup.securityGroupId;
     }
 
     return new Import(scope, id);

packages/@aws-cdk/aws-rds/lib/instance.ts

@@ -76,9 +76,9 @@ export interface DatabaseInstanceAttributes {
   readonly port: number;
 
   /**
-   * The security group identifier of the instance.
+   * The security group of the instance.
    */
-  readonly securityGroupId: string;
+  readonly securityGroup: ec2.ISecurityGroup;
 }
 
 /**
@@ -92,14 +92,14 @@ export abstract class DatabaseInstanceBase extends Resource implements IDatabase
     class Import extends DatabaseInstanceBase implements IDatabaseInstance {
       public readonly defaultPort = ec2.Port.tcp(attrs.port);
       public readonly connections = new ec2.Connections({
-        securityGroups: [ec2.SecurityGroup.fromSecurityGroupId(this, 'SecurityGroup', attrs.securityGroupId)],
+        securityGroups: [attrs.securityGroup],
         defaultPort: this.defaultPort
       });
       public readonly instanceIdentifier = attrs.instanceIdentifier;
       public readonly dbInstanceEndpointAddress = attrs.instanceEndpointAddress;
       public readonly dbInstanceEndpointPort = attrs.port.toString();
       public readonly instanceEndpoint = new Endpoint(attrs.instanceEndpointAddress, attrs.port);
-      public readonly securityGroupId = attrs.securityGroupId;
+      public readonly securityGroupId = attrs.securityGroup.securityGroupId;
     }
 
     return new Import(scope, id);