Ec2.VPC: incorrect subnet calculation

[nickpowpow]

If you put a slightly incorrect (but valid) VPC CIDR in, subnets are calculated in an odd way

Reproduction Steps

self.vpc = ec2.Vpc(self,
            f"{kwargs['env']['region']}-vpc",
            max_azs=1,
            cidr=cidr_range,
            # configuration will create 1 subnet in a single AZ.
            subnet_configuration=[ec2.SubnetConfiguration(
                    subnet_type=ec2.SubnetType.ISOLATED,
                    name="Isolated",
                    cidr_mask=24
                    )
            ]
        )

If you use "10.100.0.0/16" for cidr_range, it generates the correct 10.100.0.0/24 subnet.
If you use "10.101.1.0/16" for cidr_range, it incorrectly also generates 10.100.0.0/24 subnet which errors in Cfn on deploy.

What did you expect to happen?

I would expect CDK to parse the first 16 bits of 10.101 and store that, and generate the next set of valid subnets.

What actually happened?

CDK does something weird and generates 10.100 from a 10.101.1/16 definition.

Environment

• CDK CLI Version : latest
• Framework Version: python3.9
• OS : mac

---

This is 🐛 Bug Report

[njlynch]

Thanks for the report @nickpowpow .

This looks like documented behavior for the underlying CidrBlock class that has existed almost since day 1 of the CDK (introduced in #250).

aws-cdk/packages/@aws-cdk/aws-ec2/lib/network-util.ts

Lines 221 to 223 in 774b9ed

	* If the given `cidr` or `ipAddress` is not the beginning of the block,
	* then the next available block will be returned. For example, if
	* `10.0.3.1/28` is given the returned block will represent `10.0.3.16/28`.

Given the central nature of this class, I don't think we would likely opt to change this behavior without good reason and likely a feature-flag opt-in.

Is this a blocker of any kind for you, or just an oddity/annoyance?

[nickpowpow]

I gave 10.101.1/16 and CDK calculated 10.100/16. If it was following the idea it would go to the next block, it would have proceeded to 10.102, and not backwards to 10.100. I'm mostly submitting because I spent three hours trying to figure out why my code was reusing classes (I was also instantiating another 10.100 VPC) when it was really a typo caused this bug for me. (10.101.1.0/16 instead of 10.101.0.0/16).

…

On Fri, Jul 23, 2021, 10:18 AM Nick Lynch ***@***.***> wrote: Thanks for the report @nickpowpow <https://github.com/nickpowpow> . This looks like documented behavior for the underlying CidrBlock class that has existed almost since day 1 of the CDK (introduced in #250 <#250>). https://github.com/aws/aws-cdk/blob/e133bca61/packages/%40aws-cdk/aws-ec2/lib/network-util.ts#L221-L223 If the given cidr or ipAddress is not the beginning of the block, then the next available block will be returned. For example, if 10.0.3.1/28 is given the returned block will represent 10.0.3.16/28. Given the central nature of this class, I don't think we would likely opt to change this behavior without good reason and likely a feature-flag opt-in. Is this a blocker of any kind for you, or just an oddity/annoyance? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#15682 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABEWJGMGR6JTPVHMRRDQHH3TZGI4JANCNFSM5AXIE74Q> .

[bruuuuuuuce]

I am facing a similar issue when using a non-standard cidr range. I am creating a vpc with the cidr 10.23.73.0/18, and am telling cdk to generate a private subnet in my vpc with a mask of 20. The deploy is failing because its trying to set the subnet cidr to 10.23.144.0/20 which is not contained by 10.23.73.0/18.

My :twocents: on how to help developers with this

• Allow developers to define explicit cidrs for their subnets. Its a little strange to define a vpc using a cidr, but define a subnet using a mask.

ec2.SubnetConfiguration(
     subnet_type=ec2.SubnetType.ISOLATED,
     name="Isolated",
     cidr="10.0.0.0/24"
)

• Verify that the subnet cidrs are contained within the vpc's cidr when building the template. Would be nice for this to fail at synth rather than on deploy.

[github-actions]

This issue has not received any attention in 1 year. If you want to keep this issue open, please leave a comment below and auto-close will be canceled.