feat(ec2): mutable? param for imported SecurityGroups (#4493)

aws · Oct 21, 2019 · 9764996 · 9764996
1 parent bbe0380
commit 9764996
Show file tree

Hide file tree

Showing 2 changed files with 64 additions and 3 deletions.
diff --git a/packages/@aws-cdk/aws-ec2/lib/security-group.ts b/packages/@aws-cdk/aws-ec2/lib/security-group.ts
@@ -230,6 +230,17 @@ export interface SecurityGroupImportOptions {
    * @default true
    */
   readonly allowAllOutbound?: boolean;
+
+  /**
+   * If a SecurityGroup is mutable CDK can add rules to existing groups
+   *
+   * Beware that making a SecurityGroup immutable might lead to issue
+   * due to missing ingress/egress rules for new resources.
+   *
+   * @experimental
+   * @default true
+   */
+  readonly mutable?: boolean;
 }
 
 /**
@@ -245,7 +256,7 @@ export class SecurityGroup extends SecurityGroupBase {
    * Import an existing security group into this app.
    */
   public static fromSecurityGroupId(scope: Construct, id: string, securityGroupId: string, options: SecurityGroupImportOptions = {}): ISecurityGroup {
-    class Import extends SecurityGroupBase {
+    class MutableImport extends SecurityGroupBase {
       public securityGroupId = securityGroupId;
 
       public addEgressRule(peer: IPeer, connection: Port, description?: string, remoteRule?: boolean) {
@@ -256,7 +267,21 @@ export class SecurityGroup extends SecurityGroupBase {
       }
     }
 
-    return new Import(scope, id);
+    class ImmutableImport extends SecurityGroupBase {
+      public securityGroupId = securityGroupId;
+
+      public addEgressRule(_peer: IPeer, _connection: Port, _description?: string, _remoteRule?: boolean) {
+        // do nothing
+      }
+
+      public addIngressRule(_peer: IPeer, _connection: Port, _description?: string, _remoteRule?: boolean)  {
+        // do nothing
+      }
+    }
+
+    return options.mutable !== false
+    ? new MutableImport(scope, id)
+    : new ImmutableImport(scope, id);
   }
 
   /**

diff --git a/packages/@aws-cdk/aws-ec2/test/test.security-group.ts b/packages/@aws-cdk/aws-ec2/test/test.security-group.ts
@@ -1,4 +1,4 @@
-import { expect, haveResource } from '@aws-cdk/assert';
+import { expect, haveResource, not } from '@aws-cdk/assert';
 import { Intrinsic, Lazy, Stack, Token } from '@aws-cdk/core';
 import { Test } from 'nodeunit';
 import { Peer, Port, SecurityGroup, Vpc } from "../lib";
@@ -112,6 +112,42 @@ export = {
     test.done();
   },
 
+  'immutable imports do not add rules'(test: Test) {
+    // GIVEN
+    const stack = new Stack();
+
+    // WHEN
+    const sg = SecurityGroup.fromSecurityGroupId(stack, 'SG1', "test-id", {mutable: false});
+    sg.addEgressRule(Peer.anyIpv4(), Port.tcp(86), 'This rule was not added');
+    sg.addIngressRule(Peer.anyIpv4(), Port.tcp(86), 'This rule was not added');
+
+    expect(stack).to(not(haveResource('AWS::EC2::SecurityGroup', {
+      SecurityGroupEgress: [
+        {
+          CidrIp: "0.0.0.0/0",
+          Description: "This rule was not added",
+          FromPort: 86,
+          IpProtocol: "tcp",
+          ToPort: 86
+        }
+      ],
+    })));
+
+    expect(stack).to(not(haveResource('AWS::EC2::SecurityGroup', {
+      SecurityGroupIngress: [
+        {
+          CidrIp: "0.0.0.0/0",
+          Description: "This rule was not added",
+          FromPort: 86,
+          IpProtocol: "tcp",
+          ToPort: 86
+        }
+      ],
+    })));
+
+    test.done();
+  },
+
   'peer between all types of peers and port range types'(test: Test) {
     // GIVEN
     const stack = new Stack(undefined, 'TestStack', { env: { account: '12345678', region: 'dummy' }});