Repository: pulumi/pulumi-awsx (ts)

Issue:
AWSX wants to replace all VPC subnet cidr blocks
### What happened?

When running a preview for one of our stacks Pulumi is indicating it wants to replace all of our VPC subnets(12) due to changes with the cidr blocks. There are existing resources in these subnets.

```
Previewing update (prd):
     Type                                          Name                       Plan        Info
     pulumi:pulumi:Stack                           aws-eks-prd                            
     └─ awsx:ec2:Vpc                               app-prd-vpc                            
        └─ aws:ec2:Vpc                             app-prd-vpc                            [diff: +tagsAll-instanceTenancy~__defaults]
 +-        ├─ aws:ec2:Subnet                       app-prd-vpc-private-2      replace     [diff: ~cidrBlock]
           │  └─ aws:ec2:RouteTable                app-prd-vpc-private-2                  [diff: +tagsAll]
 +-        │     ├─ aws:ec2:RouteTableAssociation  app-prd-vpc-private-2      replace     [diff: ~subnetId]
 ~         │     └─ aws:ec2:Route                  app-prd-vpc-private-2      update      [diff: ~natGatewayId]
 +-        ├─ aws:ec2:Subnet                       app-prd-vpc-public-1       replace     [diff: ~cidrBlock]
 +-        │  ├─ aws:ec2:NatGateway                app-prd-vpc-1              replace     [diff: ~subnetId]
           │  └─ aws:ec2:RouteTable                app-prd-vpc-public-1                   [diff: +tagsAll]
 +-        │     └─ aws:ec2:RouteTableAssociation  app-prd-vpc-public-1       replace     [diff: ~subnetId]
 +-        ├─ aws:ec2:Subnet                       app-prd-vpc-private-3      replace     [diff: ~cidrBlock]
           │  └─ aws:ec2:RouteTable                app-prd-vpc-private-3                  [diff: +tagsAll]
 +-        │     ├─ aws:ec2:RouteTableAssociation  app-prd-vpc-private-3      replace     [diff: ~subnetId]
 ~         │     └─ aws:ec2:Route                  app-prd-vpc-private-3      update      [diff: ~natGatewayId]
 +-        ├─ aws:ec2:Subnet                       app-prd-vpc-elasticache-2  replace     [diff: ~cidrBlock]
           │  └─ aws:ec2:RouteTable                app-prd-vpc-elasticache-2              [diff: +tagsAll]
 +-        │     └─ aws:ec2:RouteTableAssociation  app-prd-vpc-elasticache-2  replace     [diff: ~subnetId]
 +-        ├─ aws:ec2:Subnet                       app-prd-vpc-rds-3          replace     [diff: ~cidrBlock]
           │  └─ aws:ec2:RouteTable                app-prd-vpc-rds-3                      [diff: +tagsAll]
 +-        │     └─ aws:ec2:RouteTableAssociation  app-prd-vpc-rds-3          replace     [diff: ~subnetId]
 +-        ├─ aws:ec2:Subnet                       app-prd-vpc-elasticache-3  replace     [diff: ~cidrBlock]
           │  └─ aws:ec2:RouteTable                app-prd-vpc-elasticache-3              [diff: +tagsAll]
 +-        │     └─ aws:ec2:RouteTableAssociation  app-prd-vpc-elasticache-3  replace     [diff: ~subnetId]
 +-        ├─ aws:ec2:Subnet                       app-prd-vpc-public-3       replace     [diff: ~cidrBlock]
           │  ├─ aws:ec2:RouteTable                app-prd-vpc-public-3                   [diff: +tagsAll]
 +-        │  │  └─ aws:ec2:RouteTableAssociation  app-prd-vpc-public-3       replace     [diff: ~subnetId]
 +-        │  └─ aws:ec2:NatGateway                app-prd-vpc-3              replace     [diff: ~subnetId]
 +-        ├─ aws:ec2:Subnet                       app-prd-vpc-rds-2          replace     [diff: ~cidrBlock]
           │  └─ aws:ec2:RouteTable                app-prd-vpc-rds-2                      [diff: +tagsAll]
 +-        │     └─ aws:ec2:RouteTableAssociation  app-prd-vpc-rds-2          replace     [diff: ~subnetId]
 +-        ├─ aws:ec2:Subnet                       app-prd-vpc-private-1      replace     [diff: ~cidrBlock]
           │  └─ aws:ec2:RouteTable                app-prd-vpc-private-1                  [diff: +tagsAll]
 +-        │     ├─ aws:ec2:RouteTableAssociation  app-prd-vpc-private-1      replace     [diff: ~subnetId]
 ~         │     └─ aws:ec2:Route                  app-prd-vpc-private-1      update      [diff: ~natGatewayId]
 +-        ├─ aws:ec2:Subnet                       app-prd-vpc-public-2       replace     [diff: ~cidrBlock]
           │  ├─ aws:ec2:RouteTable                app-prd-vpc-public-2                   [diff: +tagsAll]
 +-        │  │  └─ aws:ec2:RouteTableAssociation  app-prd-vpc-public-2       replace     [diff: ~subnetId]
 +-        │  └─ aws:ec2:NatGateway                app-prd-vpc-2              replace     [diff: ~subnetId]
 +-        ├─ aws:ec2:Subnet                       app-prd-vpc-elasticache-1  replace     [diff: ~cidrBlock]
           │  └─ aws:ec2:RouteTable                app-prd-vpc-elasticache-1              [diff: +tagsAll]
 +-        │     └─ aws:ec2:RouteTableAssociation  app-prd-vpc-elasticache-1  replace     [diff: ~subnetId]
 +-        └─ aws:ec2:Subnet                       app-prd-vpc-rds-1          replace     [diff: ~cidrBlock]
              └─ aws:ec2:RouteTable                app-prd-vpc-rds-1                      [diff: +tagsAll]
 +-              └─ aws:ec2:RouteTableAssociation  app-prd-vpc-rds-1          replace     [diff: ~subnetId]

```

When looking at the diff of the preview it is suggesting new cidr blocks for all the subnets.

### Example

Code that generates the VPC
```
vpc = awsx.ec2.Vpc(
        f"{cluster_config.name}-vpc",
        enable_dns_hostnames=True,
        availability_zone_names=availability_zone_names,
        cidr_block=cluster_config.vpc_cidr_block,
        subnet_strategy=awsx.ec2.SubnetAllocationStrategy("Legacy"),
        subnet_specs=[
            awsx.ec2.SubnetSpecArgs(
                type=awsx.ec2.SubnetType.PRIVATE
            ),
            awsx.ec2.SubnetSpecArgs(
                type=awsx.ec2.SubnetType.PUBLIC
            ),
            awsx.ec2.SubnetSpecArgs(
                type=awsx.ec2.SubnetType.ISOLATED,
                name="rds",
                tags={"Service": "RDS", "Cluster": cluster_config.name},
            ),
            awsx.ec2.SubnetSpecArgs(
                type=awsx.ec2.SubnetType.ISOLATED,
                name="elasticache",
                tags={"Service": "ElastiCache", "Cluster": cluster_config.name},
            ),
        ],
        tags={
            "Name": f"VPC-{cluster_config.name}",
            "Environment": cluster_config.environment,
            "Platform": "pulumi",
        },
    )
```

### Output of `pulumi about`

```
CLI          
Version      3.101.1
Go Version   go1.21.5
Go Compiler  gc

Plugins
NAME        VERSION
aws         6.13.3
aws-native  0.90.0
awsx        2.3.0
command     0.9.2
datadog     4.23.0
docker      4.5.0
eks         2.0.0
kubernetes  4.6.0
postgresql  3.10.0
python      unknown
random      4.15.0
tailscale   0.13.3

Host     
OS       ubuntu
Version  22.04
Arch     x86_64

```

### Additional context

We recently updated  from awsx v1.0.2 to v2.3.0. We didn't see this issues with our test and dev environments.

### Contributing

Vote on this issue by adding a 👍 reaction. 
To contribute a fix for this issue, leave a comment (and link to your pull request, if you've opened one already). 


Pull request:
Fix VPC legacy default subnet distribution
Fixes #1204 

- Assuming all subnets are the same size is overly cautious and breaks some existing setups.
- Maintain the new special case for single subnet layouts to use the whole of small VPCs.
- This will now fail and require manual layout for smaller VPCs with either:
   - More than 1 private subnets
   - More than 2 public subnets
   - More than 1 public subnets and more than 4 isolated subnets

Patch:
diff --git a/awsx/ec2/knownWorkingSubnets.ts b/awsx/ec2/knownWorkingSubnets.ts
index 46f2089f..6f4ae67a 100644
--- a/awsx/ec2/knownWorkingSubnets.ts
+++ b/awsx/ec2/knownWorkingSubnets.ts
@@ -1323,4 +1323,22 @@ export const knownWorkingSubnets: {
     ],
     vpcCidr: "10.0.0.0/20",
   },
+  {
+    vpcCidr: "10.1.0.0/18",
+    subnetSpecs: [
+      {
+        type: "Private",
+      },
+      {
+        type: "Public",
+      },
+      {
+        type: "Isolated",
+      },
+      {
+        type: "Isolated",
+      },
+    ],
+    result: ["10.1.0.0/19", "10.1.32.0/20", "10.1.48.0/24", "10.1.49.0/24"],
+  },
 ];
diff --git a/awsx/ec2/subnetDistributorLegacy.ts b/awsx/ec2/subnetDistributorLegacy.ts
index c0aadb69..5b396bc1 100644
--- a/awsx/ec2/subnetDistributorLegacy.ts
+++ b/awsx/ec2/subnetDistributorLegacy.ts
@@ -70,8 +70,9 @@ export function getSubnetSpecsLegacy(
 
   let subnetsOut: SubnetSpec[] = [];
 
-  // How many bits do we need if just dividing up evenly?
-  const newBitsPerSubnet = Math.log2(nextPow2(subnetInputs.length));
+  // How many bits do we need if just assuming a "normal" layout with at least a private and public subnet?
+  // Special case where there's only 1 subnet - with a small VPC, we'll let it use the whole space.
+  const newBitsPerSubnet = subnetInputs.length === 1 ? 0 : 1;
 
   for (let i = 0; i < azNames.length; i++) {
     const privateSubnetsOut: SubnetSpec[] = [];


Test patch:
diff --git a/awsx/ec2/subnetDistributorLegacy.test.ts b/awsx/ec2/subnetDistributorLegacy.test.ts
index d47cdeb1..25f18819 100644
--- a/awsx/ec2/subnetDistributorLegacy.test.ts
+++ b/awsx/ec2/subnetDistributorLegacy.test.ts
@@ -86,6 +86,44 @@ describe("default subnet layout", () => {
     );
   });
 
+  describe("default sizes private, public and isolated subnet", () => {
+    it("gives /19, /20 and /24 for /16 AZ", () => {
+      expect(getDefaultSubnetSizes(16)).toEqual([19, 20, 24]);
+    });
+    it("gives /19, /20 and /24 for /17 AZ", () => {
+      expect(getDefaultSubnetSizes(17)).toEqual([19, 20, 24]);
+    });
+    it("gives /19, /20 and /24 for /18 AZ", () => {
+      expect(getDefaultSubnetSizes(18)).toEqual([19, 20, 24]);
+    });
+    it("gives /20, /20 and /24 for /19 AZ", () => {
+      expect(getDefaultSubnetSizes(19)).toEqual([20, 20, 24]);
+    });
+    it("gives /21, /21 and /24 for /20 AZ", () => {
+      expect(getDefaultSubnetSizes(20)).toEqual([21, 21, 24]);
+    });
+    it("gives /22, /22 and /24 for /21 AZ", () => {
+      expect(getDefaultSubnetSizes(21)).toEqual([22, 22, 24]);
+    });
+    it("gives /23, /23 and /24 for /22 AZ", () => {
+      expect(getDefaultSubnetSizes(22)).toEqual([23, 23, 24]);
+    });
+    it("gives /24, /24 and /24 for /23 AZ", () => {
+      expect(getDefaultSubnetSizes(23)).toEqual([24, 24, 24]);
+    });
+
+    function getDefaultSubnetSizes(azSize: number) {
+      const vpcCidr = `10.0.0.0/${azSize}`;
+      const result = getSubnetSpecsLegacy(
+        "vpcName",
+        vpcCidr,
+        ["us-east-1a"],
+        [{ type: "Private" }, { type: "Public" }, { type: "Isolated" }],
+      );
+      return result.map((s) => getCidrMask(s.cidrBlock));
+    }
+  });
+
   it("should use default values if VPC is large", () => {
     fc.assert(
       fc.property(
diff --git a/awsx/ec2/subnetDistributorNew.test.ts b/awsx/ec2/subnetDistributorNew.test.ts
index b4970ea0..407da630 100644
--- a/awsx/ec2/subnetDistributorNew.test.ts
+++ b/awsx/ec2/subnetDistributorNew.test.ts
@@ -50,6 +50,30 @@ function subnetSpec() {
 }
 
 describe("default subnet layout", () => {
+  describe("when no layout specified", () => {
+    it.each([16, 17, 18, 19, 20, 21, 22, 23, 24])(
+      "/%i AZ creates single private & public with staggered sizes",
+      (azCidrMask) => {
+        expect(getDefaultSubnetSizes(azCidrMask)).toMatchObject([
+          {
+            type: "Private",
+            cidrMask: azCidrMask + 1,
+          },
+          {
+            type: "Public",
+            cidrMask: azCidrMask + 2,
+          },
+        ]);
+      },
+    );
+
+    function getDefaultSubnetSizes(azSize: number) {
+      const vpcCidr = `10.0.0.0/${azSize}`;
+      const result = getSubnetSpecs("vpcName", vpcCidr, ["us-east-1a"], undefined);
+      return result.map((s) => ({ type: s.type, cidrMask: getCidrMask(s.cidrBlock) }));
+    }
+  });
+
   it("should have smaller subnets than the vpc", () => {
     fc.assert(
       fc.property(
@@ -92,6 +116,36 @@ describe("default subnet layout", () => {
     );
   });
 
+  describe("default sizes for private, public and isolated subnet", () => {
+    it.each([16, 17, 18, 19, 20, 21, 22, 23, 24])(
+      "/%i AZ evenly distributes space",
+      (azCidrMask) => {
+        const vpcCidr = `10.0.0.0/${azCidrMask}`;
+        const result = getSubnetSpecs(
+          "vpcName",
+          vpcCidr,
+          ["us-east-1a"],
+          [{ type: "Private" }, { type: "Public" }, { type: "Isolated" }],
+        );
+        const masks = result.map((s) => ({ type: s.type, cidrMask: getCidrMask(s.cidrBlock) }));
+        expect(masks).toMatchObject([
+          {
+            type: "Private",
+            cidrMask: azCidrMask + 2,
+          },
+          {
+            type: "Public",
+            cidrMask: azCidrMask + 2,
+          },
+          {
+            type: "Isolated",
+            cidrMask: azCidrMask + 2,
+          },
+        ]);
+      },
+    );
+  });
+
   it("should not have overlapping ranges", () => {
     fc.assert(
       fc.property(