fix(ec2): allow adding gateway endpoints to imported VPC (aws#3509)

* fix(ec2): allow adding gateway endpoints to imported VPC Move `addGatewayEndpoint()` to `IVpc` now that imported VPCs can have route table IDs populated (aws#3171). Closes aws#3472 * deprecate addS3Endpoint() and addDynamoDbEndpoint() * remove test on conveniance methods
jeremydeshotel · Aug 7, 2019 · b5db88d · b5db88d
1 parent 0a2540b
commit b5db88d
Show file tree

Hide file tree

Showing 3 changed files with 34 additions and 56 deletions.
diff --git a/packages/@aws-cdk/aws-ec2/lib/util.ts b/packages/@aws-cdk/aws-ec2/lib/util.ts
@@ -124,7 +124,7 @@ export function allRouteTableIds(...ssns: SelectedSubnets[]): string[] {
   const ret = new Set<string>();
   for (const ssn of ssns) {
     for (const subnet of ssn.subnets) {
-      if (subnet.routeTable) {
+      if (subnet.routeTable && subnet.routeTable.routeTableId) {
         ret.add(subnet.routeTable.routeTableId);
       }
     }

diff --git a/packages/@aws-cdk/aws-ec2/lib/vpc.ts b/packages/@aws-cdk/aws-ec2/lib/vpc.ts
@@ -94,6 +94,11 @@ export interface IVpc extends IResource {
    */
   addVpnConnection(id: string, options: VpnConnectionOptions): VpnConnection;
 
+  /**
+   * Adds a new gateway endpoint to this VPC
+   */
+  addGatewayEndpoint(id: string, options: GatewayVpcEndpointOptions): GatewayVpcEndpoint
+
   /**
    * Adds a new interface endpoint to this VPC
    */
@@ -287,6 +292,16 @@ abstract class VpcBase extends Resource implements IVpc {
     });
   }
 
+  /**
+   * Adds a new gateway endpoint to this VPC
+   */
+  public addGatewayEndpoint(id: string, options: GatewayVpcEndpointOptions): GatewayVpcEndpoint {
+    return new GatewayVpcEndpoint(this, id, {
+      vpc: this,
+      ...options
+    });
+  }
+
   /**
    * Return the subnets appropriate for the placement strategy
    */
@@ -921,18 +936,11 @@ export class Vpc extends VpcBase {
       }
     }
   }
-  /**
-   * Adds a new gateway endpoint to this VPC
-   */
-  public addGatewayEndpoint(id: string, options: GatewayVpcEndpointOptions): GatewayVpcEndpoint {
-    return new GatewayVpcEndpoint(this, id, {
-      vpc: this,
-      ...options
-    });
-  }
 
   /**
    * Adds a new S3 gateway endpoint to this VPC
+   *
+   * @deprecated use `addGatewayEndpoint()` instead
    */
   public addS3Endpoint(id: string, subnets?: SubnetSelection[]): GatewayVpcEndpoint {
     return new GatewayVpcEndpoint(this, id, {
@@ -944,6 +952,8 @@ export class Vpc extends VpcBase {
 
   /**
    * Adds a new DynamoDB gateway endpoint to this VPC
+   *
+   * @deprecated use `addGatewayEndpoint()` instead
    */
   public addDynamoDbEndpoint(id: string, subnets?: SubnetSelection[]): GatewayVpcEndpoint {
     return new GatewayVpcEndpoint(this, id, {

diff --git a/packages/@aws-cdk/aws-ec2/test/test.vpc-endpoint.ts b/packages/@aws-cdk/aws-ec2/test/test.vpc-endpoint.ts
@@ -174,71 +174,39 @@ export = {
       test.done();
     },
 
-    'conveniance methods for S3 and DynamoDB'(test: Test) {
+    'works with an imported vpc'(test: Test) {
       // GIVEN
       const stack = new Stack();
-      const vpc = new Vpc(stack, 'VpcNetwork');
-
-      // WHEN
-      vpc.addS3Endpoint('S3');
-      vpc.addDynamoDbEndpoint('DynamoDb');
+      const vpc = Vpc.fromVpcAttributes(stack, 'VPC', {
+        vpcId: 'id',
+        privateSubnetIds: ['1', '2', '3'],
+        privateSubnetRouteTableIds: ['rt1', 'rt2', 'rt3'],
+        availabilityZones: ['a', 'b', 'c']
+      });
 
       // THEN
-      expect(stack).to(haveResource('AWS::EC2::VPCEndpoint', {
-        ServiceName: {
-          'Fn::Join': [
-            '',
-            [
-              'com.amazonaws.',
-              {
-                Ref: 'AWS::Region'
-              },
-              '.s3'
-            ]
-          ]
-        },
-      }));
+      vpc.addGatewayEndpoint('Gateway', { service: GatewayVpcEndpointAwsService.S3 });
 
       expect(stack).to(haveResource('AWS::EC2::VPCEndpoint', {
-        ServiceName: {
-          'Fn::Join': [
-            '',
-            [
-              'com.amazonaws.',
-              {
-                Ref: 'AWS::Region'
-              },
-              '.dynamodb'
-            ]
-          ]
-        },
+        ServiceName: { 'Fn::Join': ['', ['com.amazonaws.', { Ref: 'AWS::Region' }, '.s3']] },
+        VpcId: 'id',
+        RouteTableIds: ['rt1', 'rt2', 'rt3'],
+        VpcEndpointType: 'Gateway',
       }));
 
       test.done();
     },
 
-    'works with an imported vpc'(test: Test) {
+    'throws with an imported vpc without route tables ids'(test: Test) {
       // GIVEN
       const stack = new Stack();
       const vpc = Vpc.fromVpcAttributes(stack, 'VPC', {
         vpcId: 'id',
         privateSubnetIds: ['1', '2', '3'],
-        privateSubnetRouteTableIds: ['rt1', 'rt2', 'rt3'],
         availabilityZones: ['a', 'b', 'c']
       });
 
-      // THEN
-      new GatewayVpcEndpoint(stack, 'Gateway', {
-        service: GatewayVpcEndpointAwsService.S3,
-        vpc
-      });
-
-      expect(stack).to(haveResource('AWS::EC2::VPCEndpoint', {
-        ServiceName: { 'Fn::Join': ['', ['com.amazonaws.', { Ref: 'AWS::Region' }, '.s3']] },
-        VpcId: 'id',
-        RouteTableIds: ['rt1', 'rt2', 'rt3'],
-        VpcEndpointType: 'Gateway',
-      }));
+      test.throws(() => vpc.addGatewayEndpoint('Gateway', { service: GatewayVpcEndpointAwsService.S3 }), /route table/);
 
       test.done();
     }