Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot extend subnet groups properly after vpc is deployed #2087

Closed
darcoli opened this issue Mar 25, 2019 · 4 comments · Fixed by #2090
Closed

Cannot extend subnet groups properly after vpc is deployed #2087

darcoli opened this issue Mar 25, 2019 · 4 comments · Fixed by #2090

Comments

@darcoli
Copy link
Contributor

darcoli commented Mar 25, 2019

Typically there are multiple subnets that must be allocated in groups. However, at the moment, it is not possible to have groups of subnets - any attempt to do so will result in configuration that is not extensible. To explain this better, consider the use-case below:

As a use-case, here is a simplification of the scenario we have in our organisation. At the moment we provide 3 business products and each of these has 2 types of subnets:

  1. An application subnet
  2. A database subnet

As the organisation grows, new business products are introduced and hence we may be asked to add other subnets in the future. When the VPC is initially created, there is no way to anticipate which products will be introduced later on. We can however assume that a maximum of 8 products is a good limit for a number of years to come.

Using the current capabilities of VpcNetwork, we define our requirements (with 3 products) as follows:

new ec2.VpcNetwork(this, 'MyVpc', {
  cidr: '10.124.0.0/16',
  maxAZs: 1,
  natGatewayPlacement: { subnetName: 'Public' },
  natGateways: 1,
  subnetConfiguration: [
    { name: 'ApplicationA', subnetType: ec2.SubnetType.Private, cidrMask: 21 },
    { name: 'ApplicationB', subnetType: ec2.SubnetType.Private, cidrMask: 21 },
    { name: 'ApplicationC', subnetType: ec2.SubnetType.Private, cidrMask: 21 },

    { name: 'DatabaseA', subnetType: ec2.SubnetType.Private, cidrMask: 24 },
    { name: 'DatabaseB', subnetType: ec2.SubnetType.Private, cidrMask: 24 },
    { name: 'DatabaseC', subnetType: ec2.SubnetType.Private, cidrMask: 24 },

    { name: 'Public', subnetType: ec2.SubnetType.Public, cidrMask: 24 },
  ],
});

Later on, when the organisation starts providing a new product, we would need to add more subnets, changing the configuration as follows:

new ec2.VpcNetwork(this, 'MyVpc', {
  cidr: '10.124.0.0/16',
  maxAZs: 1,
  natGatewayPlacement: { subnetName: 'Public' },
  natGateways: 1,
  subnetConfiguration: [
    { name: 'ApplicationA', subnetType: ec2.SubnetType.Private, cidrMask: 21 },
    { name: 'ApplicationB', subnetType: ec2.SubnetType.Private, cidrMask: 21 },
    { name: 'ApplicationC', subnetType: ec2.SubnetType.Private, cidrMask: 21 },
    { name: 'ApplicationD', subnetType: ec2.SubnetType.Private, cidrMask: 21 },  // New product

    { name: 'DatabaseA', subnetType: ec2.SubnetType.Private, cidrMask: 24 },
    { name: 'DatabaseB', subnetType: ec2.SubnetType.Private, cidrMask: 24 },
    { name: 'DatabaseC', subnetType: ec2.SubnetType.Private, cidrMask: 24 },
    { name: 'DatabaseD', subnetType: ec2.SubnetType.Private, cidrMask: 24 },   // New product

    { name: 'Public', subnetType: ec2.SubnetType.Public, cidrMask: 24 },
  ],
});

However, checking the cdk diff we get the following changes:

Resources
[+] AWS::EC2::Subnet MyVpc/ApplicationDSubnet1/Subnet MyVpcApplicationDSubnet1Subnet90047EE5 
[+] AWS::EC2::RouteTable MyVpc/ApplicationDSubnet1/RouteTable MyVpcApplicationDSubnet1RouteTableE36ABAE7 
[+] AWS::EC2::SubnetRouteTableAssociation MyVpc/ApplicationDSubnet1/RouteTableAssociation MyVpcApplicationDSubnet1RouteTableAssociation573085BC 
[+] AWS::EC2::Route MyVpc/ApplicationDSubnet1/DefaultRoute MyVpcApplicationDSubnet1DefaultRouteF080A07A 
[+] AWS::EC2::Subnet MyVpc/DatabaseDSubnet1/Subnet MyVpcDatabaseDSubnet1Subnet50AA6A28 
[+] AWS::EC2::RouteTable MyVpc/DatabaseDSubnet1/RouteTable MyVpcDatabaseDSubnet1RouteTable2276D365 
[+] AWS::EC2::SubnetRouteTableAssociation MyVpc/DatabaseDSubnet1/RouteTableAssociation MyVpcDatabaseDSubnet1RouteTableAssociation7BDDCAC3 
[+] AWS::EC2::Route MyVpc/DatabaseDSubnet1/DefaultRoute MyVpcDatabaseDSubnet1DefaultRoute01BFF404 
[~] AWS::EC2::Subnet MyVpc/DatabaseASubnet1/Subnet MyVpcDatabaseASubnet1Subnet05173CE6 replace
 └─ [~] CidrBlock (requires replacement)
     ├─ [-] 10.124.24.0/24
     └─ [+] 10.124.32.0/24
[~] AWS::EC2::SubnetRouteTableAssociation MyVpc/DatabaseASubnet1/RouteTableAssociation MyVpcDatabaseASubnet1RouteTableAssociation25CA7EE3 replace
 └─ [~] SubnetId (requires replacement)
     └─ [~] .Ref:
         ├─ [-] MyVpcDatabaseASubnet1Subnet05173CE6
         └─ [+] MyVpcDatabaseASubnet1Subnet05173CE6 (replaced)
[~] AWS::EC2::Subnet MyVpc/DatabaseBSubnet1/Subnet MyVpcDatabaseBSubnet1SubnetDDD5C38E replace
 └─ [~] CidrBlock (requires replacement)
     ├─ [-] 10.124.25.0/24
     └─ [+] 10.124.33.0/24
[~] AWS::EC2::SubnetRouteTableAssociation MyVpc/DatabaseBSubnet1/RouteTableAssociation MyVpcDatabaseBSubnet1RouteTableAssociationD1A85469 replace
 └─ [~] SubnetId (requires replacement)
     └─ [~] .Ref:
         ├─ [-] MyVpcDatabaseBSubnet1SubnetDDD5C38E
         └─ [+] MyVpcDatabaseBSubnet1SubnetDDD5C38E (replaced)
[~] AWS::EC2::Subnet MyVpc/DatabaseCSubnet1/Subnet MyVpcDatabaseCSubnet1SubnetA6A5FDF8 replace
 └─ [~] CidrBlock (requires replacement)
     ├─ [-] 10.124.26.0/24
     └─ [+] 10.124.34.0/24
[~] AWS::EC2::SubnetRouteTableAssociation MyVpc/DatabaseCSubnet1/RouteTableAssociation MyVpcDatabaseCSubnet1RouteTableAssociation6F26C38B replace
 └─ [~] SubnetId (requires replacement)
     └─ [~] .Ref:
         ├─ [-] MyVpcDatabaseCSubnet1SubnetA6A5FDF8
         └─ [+] MyVpcDatabaseCSubnet1SubnetA6A5FDF8 (replaced)
[~] AWS::EC2::Subnet MyVpc/PublicSubnet1/Subnet MyVpcPublicSubnet1SubnetF6608456 replace
 └─ [~] CidrBlock (requires replacement)
     ├─ [-] 10.124.27.0/24
     └─ [+] 10.124.36.0/24
[~] AWS::EC2::SubnetRouteTableAssociation MyVpc/PublicSubnet1/RouteTableAssociation MyVpcPublicSubnet1RouteTableAssociation2ECEE1CB replace
 └─ [~] SubnetId (requires replacement)
     └─ [~] .Ref:
         ├─ [-] MyVpcPublicSubnet1SubnetF6608456
         └─ [+] MyVpcPublicSubnet1SubnetF6608456 (replaced)
[~] AWS::EC2::NatGateway MyVpc/PublicSubnet1/NATGateway MyVpcPublicSubnet1NATGatewayAD3400C1 replace
 └─ [~] SubnetId (requires replacement)
     └─ [~] .Ref:
         ├─ [-] MyVpcPublicSubnet1SubnetF6608456
         └─ [+] MyVpcPublicSubnet1SubnetF6608456 (replaced)

This basically means that a number of subnets that are already in production will need to be renumbered. IP renumbering usually is not desirable and typically entirely not possible for applications that are already in production.

In order to make such changes possible, I propose a (non-breaking) change to subnetConfiguration in VpcNetworkProps. I propose the introduction of SubnetGroups. A SubnetGroup allows pre-allocating a block of subnets for future use. In the example given above, we can "pre-allocate" 8 subnets in each subnetGroup and this ensures that adding more subnets to each group in the future would not cause an IP renumbering.

For the same example given above, I propose the following configuration:

new ec2.VpcNetwork(this, 'MyVpc', {
  cidr: '10.124.0.0/16',
  maxAZs: 1,
  natGatewayPlacement: { subnetName: 'Public' },
  natGateways: 1,
  subnetConfiguration: [
    {
      subnetGroupName: 'Applications',
      cidrMask: 21,
      maxSubnets: 8,
      subnetConfiguration: [
        { name: 'ApplicationA', subnetType: ec2.SubnetType.Private },
        { name: 'ApplicationB', subnetType: ec2.SubnetType.Private },
        { name: 'ApplicationC', subnetType: ec2.SubnetType.Private },
      ]
    },
    {
      subnetGroupName: 'Databases',
      cidrMask: 24,
      maxSubnets: 8,
      subnetConfiguration: [
        { name: 'DatabaseA', subnetType: ec2.SubnetType.Private },
        { name: 'DatabaseB', subnetType: ec2.SubnetType.Private },
        { name: 'DatabaseC', subnetType: ec2.SubnetType.Private },
      ]
    },
    { name: 'Public', subnetType: ec2.SubnetType.Public, cidrMask: 24 },
  ],
});

Note that the Public subnet is left outside any group to point out that this is a non-breaking change. All configurations defined so far will keep working. However, the subnetConfiguration property in VpcNetworkProps now can also accept SubnetGroups as shown above. Above, we have 2 subnet groups each pre-allocating a block of 8 subnets (specified by maxSubnets).

The expected cdk diff for the same addition of 'ApplicationD' and 'DatabaseD' subnets is then given below:

Resources
[+] AWS::EC2::Subnet MyVpc/ApplicationDSubnet1/Subnet MyVpcApplicationDSubnet1Subnet90047EE5 
[+] AWS::EC2::RouteTable MyVpc/ApplicationDSubnet1/RouteTable MyVpcApplicationDSubnet1RouteTableE36ABAE7 
[+] AWS::EC2::SubnetRouteTableAssociation MyVpc/ApplicationDSubnet1/RouteTableAssociation MyVpcApplicationDSubnet1RouteTableAssociation573085BC 
[+] AWS::EC2::Route MyVpc/ApplicationDSubnet1/DefaultRoute MyVpcApplicationDSubnet1DefaultRouteF080A07A 
[+] AWS::EC2::Subnet MyVpc/DatabaseDSubnet1/Subnet MyVpcDatabaseDSubnet1Subnet50AA6A28 
[+] AWS::EC2::RouteTable MyVpc/DatabaseDSubnet1/RouteTable MyVpcDatabaseDSubnet1RouteTable2276D365 
[+] AWS::EC2::SubnetRouteTableAssociation MyVpc/DatabaseDSubnet1/RouteTableAssociation MyVpcDatabaseDSubnet1RouteTableAssociation7BDDCAC3 
[+] AWS::EC2::Route MyVpc/DatabaseDSubnet1/DefaultRoute MyVpcDatabaseDSubnet1DefaultRoute01BFF404 

I have a working patch for the changes proposed. However, I wanted to discuss this with you and gather feedback before opening a pull request ;)

@darcoli darcoli changed the title Cannot extend subnet groups properly after vpc is deploy Cannot extend subnet groups properly after vpc is deployed Mar 25, 2019
@rix0rrr
Copy link
Contributor

rix0rrr commented Mar 26, 2019

Subnet groups will be a confusing name, since we internally already refer to the current subnets as subnet "groups".

Is it not sufficient to add the following to a SubnetConfiguration options:

{
  /** @default false */
  reserved?: boolean;
}

The effect of setting this to true would lead to the subnet block being used in CIDR calculations, but the resources for it wouldn't actually be created.

@darcoli
Copy link
Contributor Author

darcoli commented Mar 26, 2019

Hmm that would be simpler to implement but the downside i see is that when adding a new application/database subnet you have to remember to remove a reserved subnet. But maybe that could be handled by the client code itself instead of by cdk...

I ll see if I find some time to implement this tonight and will update the PR ;)

@darcoli
Copy link
Contributor Author

darcoli commented Mar 26, 2019

@rix0rrr Would you prefer having another subnet type: SubnetType.Reserved instead of adding another configuration property?

@darcoli
Copy link
Contributor Author

darcoli commented Mar 29, 2019

Updated PR with requested changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants