addressing comments

aws · Oct 24, 2024 · f516802 · f516802
1 parent 13d1458
commit f516802
Show file tree

Hide file tree

Showing 4 changed files with 159 additions and 172 deletions.
diff --git a/packages/@aws-cdk/aws-ec2-alpha/lib/subnet-v2.ts b/packages/@aws-cdk/aws-ec2-alpha/lib/subnet-v2.ts
@@ -124,7 +124,62 @@ export class SubnetV2 extends Resource implements ISubnetV2 {
    * Import an existing subnet to the VPC
    */
   public static fromSubnetV2Attributes(scope: Construct, id: string, attrs: SubnetV2Attributes) : ISubnetV2 {
-    return new ImportedSubnetV2(scope, id, attrs);
+    /**
+    * Class to define an import for an existing subnet
+    * @resource AWS::EC2::Subnet
+    */
+    class ImportedSubnetV2 extends Resource implements ISubnetV2 {
+
+      /**
+     * The IPv6 CIDR Block assigned to this subnet
+     */
+      public readonly ipv6CidrBlock?: string = attrs.ipv6CidrBlock;
+
+      /**
+      * The type of subnet (eg. public or private) that this subnet represents.
+      */
+      public readonly subnetType?: SubnetType = attrs.subnetType;
+
+      /**
+      * The Availability Zone in which subnet is located
+      */
+      public readonly availabilityZone: string = attrs.availabilityZone;
+
+      /**
+      * The subnetId for this particular subnet
+      * Refers to the physical ID created
+      */
+      public readonly subnetId: string = attrs.subnetId;
+
+      /**
+      * Dependable that can be depended upon to force internet connectivity established on the VPC
+      */
+      public readonly internetConnectivityEstablished: IDependable = new DependencyGroup();
+
+      /**
+      * The IPv4 CIDR block assigned to this subnet
+      */
+      public readonly ipv4CidrBlock: string = attrs.ipv4CidrBlock;
+
+      /**
+      *  Current route table associated with this subnet
+      */
+      public readonly routeTable: IRouteTable = { routeTableId: attrs.routeTableId! }
+
+      /**
+      * Associate a Network ACL with this subnet
+      * Required here since it is implemented in the ISubnetV2
+      */
+      public associateNetworkAcl(aclId: string, networkAcl: INetworkAcl) {
+        const aclScope = networkAcl instanceof Construct ? networkAcl : this;
+        const other = networkAcl instanceof Construct ? this : networkAcl;
+        new SubnetNetworkAclAssociation(aclScope, aclId + Names.nodeUniqueId(other.node), {
+          networkAcl,
+          subnet: this,
+        });
+      }
+    }
+    return new ImportedSubnetV2(scope, id);
   }
 
   /**
@@ -334,84 +389,6 @@ export interface SubnetV2Attributes {
 
 }
 
-/**
- * Properties required to import a subnet
- */
-export interface ImportedSubnetV2Props extends SubnetV2Attributes {}
-
-/**
- * Class to define an import for an existing subnet
- * @resource AWS::EC2::Subnet
- */
-export class ImportedSubnetV2 extends Resource implements ISubnetV2 {
-
-  /**
-   * The IPv6 CIDR Block assigned to this subnet
-   */
-  public readonly ipv6CidrBlock?: string;
-
-  /**
-   * The type of subnet (eg. public or private) that this subnet represents.
-   */
-  public readonly subnetType?: SubnetType;
-
-  /**
-   * The Availability Zone in which subnet is located
-   */
-  public readonly availabilityZone: string;
-
-  /**
-  * The subnetId for this particular subnet
-  * Refers to the physical ID created
-  */
-  public readonly subnetId: string;
-
-  /**
-   * Dependable that can be depended upon to force internet connectivity established on the VPC
-   */
-  public readonly internetConnectivityEstablished: IDependable = new DependencyGroup();
-
-  /**
-   * The IPv4 CIDR block assigned to this subnet
-   */
-  public readonly ipv4CidrBlock: string;
-
-  /**
-  *  Current route table associated with this subnet
-  */
-  public readonly routeTable: IRouteTable;
-
-  constructor(scope: Construct, id: string, props: ImportedSubnetV2Props) {
-    super(scope, id);
-
-    if (!props.routeTableId) {
-      throw new Error('Route Table ID is required');
-    }
-
-    this.ipv4CidrBlock = props.ipv4CidrBlock;
-    this.availabilityZone = props.availabilityZone;
-    this.subnetType = props.subnetType;
-    this.ipv6CidrBlock = props.ipv6CidrBlock;
-    this.subnetId = props.subnetId;
-    this.routeTable = {
-      routeTableId: props.routeTableId!,
-    };
-  }
-
-  /**
-   * Associate a Network ACL with this subnet
-   * Required here since it is implemented in the ISubnetV2
-   */
-  public associateNetworkAcl(id: string, networkAcl: INetworkAcl) {
-    const scope = networkAcl instanceof Construct ? networkAcl : this;
-    const other = networkAcl instanceof Construct ? this : networkAcl;
-    new SubnetNetworkAclAssociation(scope, id + Names.nodeUniqueId(other.node), {
-      networkAcl,
-      subnet: this,
-    });
-  }
-}
-
 const subnetTypeMap = {
   [SubnetType.PRIVATE_ISOLATED]: (vpc: IVpcV2, subnet: SubnetV2) => vpc.isolatedSubnets.push(subnet),
   [SubnetType.PUBLIC]: (vpc: IVpcV2, subnet: SubnetV2) => vpc.publicSubnets.push(subnet),

diff --git a/packages/@aws-cdk/aws-ec2-alpha/lib/vpc-v2.ts b/packages/@aws-cdk/aws-ec2-alpha/lib/vpc-v2.ts
@@ -1,9 +1,9 @@
-import { CfnVPC, CfnVPCCidrBlock, DefaultInstanceTenancy, ISubnet } from 'aws-cdk-lib/aws-ec2';
+import { CfnVPC, CfnVPCCidrBlock, DefaultInstanceTenancy, ISubnet, SubnetType } from 'aws-cdk-lib/aws-ec2';
 import { Arn, CfnResource, Lazy, Names, Resource } from 'aws-cdk-lib/core';
 import { Construct, DependencyGroup, IDependable } from 'constructs';
 import { IpamOptions, IIpamPool } from './ipam';
 import { IVpcV2, VpcV2Base } from './vpc-v2-base';
-import { ISubnetV2, ImportedSubnetV2, SubnetV2Attributes } from './subnet-v2';
+import { ISubnetV2, SubnetV2, SubnetV2Attributes } from './subnet-v2';
 
 /**
  * Additional props needed for secondary Address
@@ -203,9 +203,10 @@ export interface VpcV2Attributes {
   readonly vpcId: string;
 
   /**
-   * Arn of the VPC
-   * will be used to set value for account and region
-   * which then later can be used for establishing VPC peering connection
+   * Arn of the VPC, required in case of cross acount or cross region VPC
+   * as given arn value will be used to set fields account and region for imported VPC,
+   * which then later can be used for establishing VPC peering connection.
+   *
    * @default - constructed with stack account and region value
    */
   readonly vpcArn?: string;
@@ -218,30 +219,21 @@ export interface VpcV2Attributes {
 
   /**
    * A VPN Gateway is attached to the VPC
+   *
    * @default - No VPN Gateway
    */
   readonly vpnGatewayId?: string;
 
   /**
-   * Public subnets associated with VPC
-   * @default -  no public subnets provided
-   */
-  readonly publicSubnets?: SubnetV2Attributes[];
-
-  /**
-   * Private subnets associated with VPC
-   * @default - no private subnets provided
-   */
-  readonly privateSubnets?: SubnetV2Attributes[];
-
-  /**
-   * Isolated subnets associated with VPC
-   * @default - no isolated subnets provided
+   * Subnets associated with imported VPC
+   *
+   * @default - no subnets provided to be imported
    */
-  readonly isolatedSubnets?: SubnetV2Attributes[];
+  readonly subnets?: SubnetV2Attributes[];
 
   /**
    * Import Secondary CIDR blocks associated with VPC
+   *
    * @default - No secondary IP address
    */
   readonly secondaryCidrBlocks?: VPCCidrBlockattributes[];
@@ -260,6 +252,76 @@ export class VpcV2 extends VpcV2Base {
    * Create a VPC from existing attributes
    */
   public static fromVpcV2Attributes(scope: Construct, id: string, attrs: VpcV2Attributes): IVpcV2 {
+    /**
+   * Internal class to allow users to import VPC
+   * @internal
+   */
+    class ImportedVpcV2 extends VpcV2Base {
+
+      public readonly vpcId: string;
+      public readonly vpcArn: string;
+      public readonly publicSubnets: ISubnetV2[] = [];
+      public readonly privateSubnets: ISubnetV2[] = [];
+      public readonly isolatedSubnets: ISubnetV2[] = [];
+      public readonly internetConnectivityEstablished: IDependable = new DependencyGroup();
+      public readonly ipv4CidrBlock: string;
+      public readonly region?: string;
+      public readonly ownerAccountId?: string;
+
+      /*
+  * Reference to all secondary blocks attached
+  */
+      public readonly secondaryCidrBlock?: IVPCCidrBlock[];
+
+      /**
+   * Refers to actual VPC Resource attribute in non-imported VPC
+   * Required to implement here due to extension from Base class
+   */
+      public readonly vpcCidrBlock: string;
+
+      // Required to do CIDR range test on imported VPCs to create new subnets
+      public readonly ipv4IpamProvisionedCidrs: string[] = [];
+
+      constructor(construct: Construct, constructId: string, props: VpcV2Attributes) {
+        super(construct, constructId);
+        this.vpcId = props.vpcId,
+        this.vpcArn = props.vpcArn ?? Arn.format({
+          service: 'ec2',
+          resource: 'vpc',
+          resourceName: this.vpcId,
+        }, this.stack);
+        // Populate region and account fields that can be used to set up peering connection
+        // sample vpc Arn - arn:aws:ec2:us-west-2:123456789012:vpc/vpc-0123456789abcdef0
+        this.region = this.vpcArn.split(':')[3];
+        this.ownerAccountId = this.vpcArn.split(':')[4];
+        // Refers to actual VPC Resource attribute in non-imported VPC
+        this.vpcCidrBlock = props.vpcCidrBlock;
+        // Required for subnet range related checks
+        this.ipv4CidrBlock = props.vpcCidrBlock;
+        this._vpnGatewayId = props.vpnGatewayId;
+
+        if (props.subnets) {
+          for (const subnet of props.subnets) {
+            if (subnet.subnetType === SubnetType.PRIVATE_WITH_EGRESS || subnet.subnetType === SubnetType.PRIVATE_WITH_NAT ||
+              subnet.subnetType === SubnetType.PRIVATE) {
+              this.privateSubnets.push(SubnetV2.fromSubnetV2Attributes(scope, subnet.subnetName?? 'ImportedPrivateSubnet', subnet));
+            } else if (subnet.subnetType === SubnetType.PUBLIC) {
+              this.publicSubnets.push(SubnetV2.fromSubnetV2Attributes(scope, subnet.subnetName?? 'ImportedPublicSubnet', subnet));
+            } else if (subnet.subnetType === SubnetType.ISOLATED || subnet.subnetType === SubnetType.PRIVATE_ISOLATED) {
+              this.isolatedSubnets.push(SubnetV2.fromSubnetV2Attributes(scope, subnet.subnetName?? 'ImportedIsolatedSubnet', subnet));
+            }
+          }
+        }
+        this.secondaryCidrBlock = props.secondaryCidrBlocks?.map(cidrBlock => VPCCidrBlock.fromVPCCidrBlockattributes(scope, cidrBlock.cidrBlockName ?? 'ImportedSecondaryCidrBlock', { ...cidrBlock }));
+        if (props.secondaryCidrBlocks) {
+          for (const cidrBlock of props.secondaryCidrBlocks) {
+            if (cidrBlock.ipv4IpamProvisionedCidrs) {
+              this.ipv4IpamProvisionedCidrs.push(...cidrBlock.ipv4IpamProvisionedCidrs);
+            }
+          }
+        }
+      }
+    }
     return new ImportedVpcV2(scope, id, attrs);
   }
 
@@ -538,73 +600,6 @@ class IpamIpv4 implements IIpAddresses {
   }
 }
 
-/**
- * Internal class to allow users to import VPC
- * @internal
- */
-class ImportedVpcV2 extends VpcV2Base {
-  public readonly vpcId: string;
-  public readonly vpcArn: string;
-  public readonly publicSubnets: ISubnetV2[] = [];
-  public readonly privateSubnets: ISubnetV2[] = [];
-  public readonly isolatedSubnets: ISubnetV2[] = [];
-  public readonly internetConnectivityEstablished: IDependable = new DependencyGroup();
-  public readonly ipv4CidrBlock: string;
-  public readonly region?: string;
-  public readonly ownerAccountId?: string;
-
-  /*
-  * Reference to all secondary blocks attached
-  */
-  public readonly secondaryCidrBlock?: IVPCCidrBlock[];
-
-  /**
-   * Refers to actual VPC Resource attribute in non-imported VPC
-   * Required to implement here due to extension from Base class
-   */
-  public readonly vpcCidrBlock: string;
-
-  // Required to do CIDR range test on imported VPCs to create new subnets
-  public readonly ipv4IpamProvisionedCidrs: string[] = [];
-
-  constructor(scope: Construct, id: string, props: VpcV2Attributes) {
-    super(scope, id);
-    this.vpcId = props.vpcId,
-    this.vpcArn = props.vpcArn ?? Arn.format({
-      service: 'ec2',
-      resource: 'vpc',
-      resourceName: this.vpcId,
-    }, this.stack);
-    // Populate region and account fields that can be used to set up peering connection
-    // sample vpc Arn - arn:aws:ec2:us-west-2:123456789012:vpc/vpc-0123456789abcdef0
-    this.region = this.vpcArn.split(':')[3];
-    this.ownerAccountId = this.vpcArn.split(':')[4];
-    // Refers to actual VPC Resource attribute in non-imported VPC
-    this.vpcCidrBlock = props.vpcCidrBlock;
-    // Required for subnet range related checks
-    this.ipv4CidrBlock = props.vpcCidrBlock;
-    this._vpnGatewayId = props.vpnGatewayId;
-
-    if (props.publicSubnets) {
-      this.publicSubnets = props.publicSubnets.map(subnet => new ImportedSubnetV2(scope, subnet.subnetName?? 'ImportedPublicSubnet', subnet));
-    }
-    if (props.privateSubnets) {
-      this.privateSubnets = props.privateSubnets.map(subnet => new ImportedSubnetV2(scope, subnet.subnetName?? 'ImportedPrivateSubnet', subnet));
-    }
-    if (props.isolatedSubnets) {
-      this.isolatedSubnets = props.isolatedSubnets.map(subnet => new ImportedSubnetV2(scope, subnet.subnetName?? 'ImportedIsolatedSubnet', subnet));
-    }
-    this.secondaryCidrBlock = props.secondaryCidrBlocks?.map(cidrBlock => VPCCidrBlock.fromVPCCidrBlockattributes(scope, cidrBlock.cidrBlockName ?? 'ImportedSecondaryCidrBlock', { ...cidrBlock }));
-    if (props.secondaryCidrBlocks) {
-      for (const cidrBlock of props.secondaryCidrBlocks) {
-        if (cidrBlock.ipv4IpamProvisionedCidrs) {
-          this.ipv4IpamProvisionedCidrs.push(...cidrBlock.ipv4IpamProvisionedCidrs);
-        }
-      }
-    }
-  }
-}
-
 /**
  * Interface to create L2 for VPC Cidr Block
  */

diff --git a/packages/@aws-cdk/aws-ec2-alpha/test/integ.test-import.ts b/packages/@aws-cdk/aws-ec2-alpha/test/integ.test-import.ts
@@ -32,15 +32,14 @@ const imported_new_vpc = VpcV2.VpcV2.fromVpcV2Attributes(stack, 'ImportedNewVPC'
   }, {
     amazonProvidedIpv6CidrBlock: true,
   }],
-  isolatedSubnets: [{
+  subnets: [{
     subnetName: 'IsolatedSubnet2',
     subnetId: 'subnet-03cd773c0fe08ed26', //Subnet Id
     subnetType: SubnetType.PRIVATE_ISOLATED,
     availabilityZone: 'us-west-2a',
     ipv4CidrBlock: '10.2.0.0/24',
     routeTableId: 'rtb-0871c310f98da2cbb', //RouteTable id
-  }],
-  publicSubnets: [{
+  }, {
     subnetId: 'subnet-0fa477e01db27d820',
     subnetType: SubnetType.PUBLIC,
     availabilityZone: 'us-west-2b',