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

aws_ec2: limitation with allocateSubnetsCidr method of AwsIpam class #25537

Closed
fradetjulien opened this issue May 11, 2023 · 6 comments · Fixed by #28027
Closed

aws_ec2: limitation with allocateSubnetsCidr method of AwsIpam class #25537

fradetjulien opened this issue May 11, 2023 · 6 comments · Fixed by #28027
Assignees
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. effort/medium Medium work item – several days of effort investigating This issue is being investigated and/or work is in progress to resolve the issue. p2

Comments

@fradetjulien
Copy link

Describe the bug

I want to deploy the following ec2.Vpc with a CIDR allocation done with IPAM.

ec2.Vpc(
    self,
    "VPC",
    vpc_name=f"vpc-{uuid4()}",
    ip_addresses=ec2.IpAddresses.aws_ipam_allocation(
        ipv4_ipam_pool_id=ipam_pool_id,
        ipv4_netmask_length=16,
        default_subnet_ipv4_netmask_length=21,
    ),
    nat_gateway_provider=ec2.NatProvider.gateway(),
    nat_gateways=1,
    max_azs=3,
    subnet_configuration=[
        ec2.SubnetConfiguration(
            name="public1",
            subnet_type=ec2.SubnetType.PUBLIC,
            cidr_mask=24,
        ),
        ec2.SubnetConfiguration(
            name="private1",
            subnet_type=ec2.SubnetType.PRIVATE_WITH_EGRESS,
            cidr_mask=24,
        ),
        ec2.SubnetConfiguration(
            name="transit1",
            subnet_type=ec2.SubnetType.PRIVATE_ISOLATED,
            cidr_mask=28,
        ),
    ],
)

However, it is impossible for me to have a successful deployment if the ipv4_netmask_length is < 20.

I can successfully deploy the vpc using an ec2.IpAddresses.aws_ipam_allocation resource with an ipv4_netmask_length >= 20

It seems that the count variable computed here is greater than the maximum allowed value 256 defined here. Causing an error when calling the Fn.cidr method here.

This looks normal to me because :

const count = Math.pow(2, netmask - rootNetmask);
math.pow(2, 28 - 16) = 4096
math.pow(2, 24 - 16) = 256
math.pow(2, 28 - 20) = 256

In my case, I think I need to generate the following CIDRs :

- !Cidr 10.0.0.0/16, 256, 24
- !Cidr 10.0.0.0/16, 4096, 28

But the range limitation from the Fn.cidr method forbids it. I don't understand why..

When I use the ec2.IpAddresses.cidr resource, I have no problem because the allocateSubnetsCidr it is not using the Fn.cidr method. See here.

Expected Behavior

I expect the ec2.IpAddresses.aws_ipam_allocation resource to behave like the ec2.IpAddresses.cidr resource. It should not limit the count value to a maximum value of 256.

Current Behavior

I get the following error :

Error: Fn::Cidr's count attribute must be between 1 and 256, 4096 was provided.
      at new FnCidr (/private/var/folders/s8/t9pzpf1s2b19qg675763jwgc0000gn/T/jsii-kernel-1CU08O/node_modules/aws-cdk-lib/core/lib/cfn-fn.js:618:19)
      at Fn.cidr (/private/var/folders/s8/t9pzpf1s2b19qg675763jwgc0000gn/T/jsii-kernel-1CU08O/node_modules/aws-cdk-lib/core/lib/cfn-fn.js:166:37)
      at /private/var/folders/s8/t9pzpf1s2b19qg675763jwgc0000gn/T/jsii-kernel-1CU08O/node_modules/aws-cdk-lib/aws-ec2/lib/ip-addresses.js:95:64
      at Array.map (<anonymous>)
      at AwsIpam.allocateSubnetsCidr (/private/var/folders/s8/t9pzpf1s2b19qg675763jwgc0000gn/T/jsii-kernel-1CU08O/node_modules/aws-cdk-lib/aws-ec2/lib/ip-addresses.js:93:44)
      at Vpc.createSubnets (/private/var/folders/s8/t9pzpf1s2b19qg675763jwgc0000gn/T/jsii-kernel-1CU08O/node_modules/aws-cdk-lib/aws-ec2/lib/vpc.js:674:55)
      at new Vpc (/private/var/folders/s8/t9pzpf1s2b19qg675763jwgc0000gn/T/jsii-kernel-1CU08O/node_modules/aws-cdk-lib/aws-ec2/lib/vpc.js:570:14)
      at Kernel._create (/private/var/folders/s8/t9pzpf1s2b19qg675763jwgc0000gn/T/tmpx0t0nvxv/lib/program.js:9964:29)
      at Kernel.create (/private/var/folders/s8/t9pzpf1s2b19qg675763jwgc0000gn/T/tmpx0t0nvxv/lib/program.js:9693:29)
      at KernelHost.processRequest (/private/var/folders/s8/t9pzpf1s2b19qg675763jwgc0000gn/T/tmpx0t0nvxv/lib/program.js:11544:36)

Reproduction Steps

ec2.Vpc(
    self,
    "VPC",
    vpc_name=f"vpc-{uuid4()}",
    ip_addresses=ec2.IpAddresses.aws_ipam_allocation(
        ipv4_ipam_pool_id=ipam_pool_id,
        ipv4_netmask_length=16,
        default_subnet_ipv4_netmask_length=21,
    ),
    nat_gateway_provider=ec2.NatProvider.gateway(),
    nat_gateways=1,
    max_azs=3,
    subnet_configuration=[
        ec2.SubnetConfiguration(
            name="public1",
            subnet_type=ec2.SubnetType.PUBLIC,
            cidr_mask=24,
        ),
        ec2.SubnetConfiguration(
            name="private1",
            subnet_type=ec2.SubnetType.PRIVATE_WITH_EGRESS,
            cidr_mask=24,
        ),
        ec2.SubnetConfiguration(
            name="transit1",
            subnet_type=ec2.SubnetType.PRIVATE_ISOLATED,
            cidr_mask=28,
        ),
    ],
)

Possible Solution

Avoid using the Fn.cidr method inside the allocateSubnetsCidr method from the AwsIpam class.

Additional Information/Context

No response

CDK CLI Version

2.77.0 (build 06a0b19)

Framework Version

2.77.0

Node.js Version

v18.16.0

OS

Amazon Linux 2

Language

Python

Language Version

3.11.0

Other information

No response

@fradetjulien fradetjulien added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels May 11, 2023
@github-actions github-actions bot added the @aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud label May 11, 2023
@pahud pahud self-assigned this May 12, 2023
@pahud pahud added investigating This issue is being investigated and/or work is in progress to resolve the issue. needs-review p1 effort/medium Medium work item – several days of effort and removed needs-triage This issue or PR still needs to be triaged. labels May 12, 2023
@pahud
Copy link
Contributor

pahud commented May 17, 2023

Thanks for your report. We will look into this issue.

@rix0rrr
Copy link
Contributor

rix0rrr commented Nov 16, 2023

Unfortunately this is a limitation of CloudFormation's Fn::Cidr function: https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/intrinsic-function-reference-cidr.html

There is not much we can do about this.

Given that from an IPAM allocation, we can't know the CIDR range at synth time, we can't precalculate the ranges at synth time either using the logic that just parses any CIDR. Instead, we have to defer the CIDR calculation to deployment time (where CloudFormation does it), which is causing this error.

You can report the issue to CloudFormation: https://github.com/aws-cloudformation/cloudformation-coverage-roadmap

Someone could also go and implement a patch to Fn.cidr() where we do a nested CIDR calculation if the we need to split more than 256 times:

// I think this is correct but it needs double checking, this math requires a lot of thinking
const split1 = Fn.cidr('10.0.0.0/16', 256, 24);
const split2 = Fn.cidr(Fn.select(0, split1), 16, 28);

@mergify mergify bot closed this as completed in #28027 Nov 17, 2023
mergify bot pushed a commit that referenced this issue Nov 17, 2023
…8027)

Because of IPAM allocation, we can't know the parent CIDR at synth time, so we cannot calculate the CIDR split at synth time either.

This forces us to rely on the `{ Fn::Cidr }` function provided by CloudFormation. For resource consumption reasons, this function is limited to splitting any range into at most 256 subranges, which means the IPAM allocated VPC cannot split into more subranges either.

This PR adds a recursive split feature: if we need to split an CIDR range more than 256 times, we will do multiple splits:

```ts
Fn.select(300, Fn.cidr(range, 4096, 4)) // <-- illegal

// ==

Fn.select(44, Fn.cidr(Fn.select(1, Fn.cidr(range, 4, 12)), 256, 4))
```

Fixes #25537.

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
Copy link

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

@jonahweissman
Copy link

@fradetjulien IPAM service team member here. The intended way to get an IPAM allocated cidr block for your VPC is to provide an Ipv4IpamPoolId in the VPC properties. The IPAM allocation resource should only be used for custom allocations.

@delenamalan
Copy link

@jonahweissman how would you do this using CDK since the Vpc class doesn't have a Ipv4IpamPoolId parameter?

@jonahweissman
Copy link

Looks like we haven't updated the VPC L2 construct to include this field, but it should be available in the CfnVPC L1 construct. It's not ideal, but it should work

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ec2 Related to Amazon Elastic Compute Cloud bug This issue is a bug. effort/medium Medium work item – several days of effort investigating This issue is being investigated and/or work is in progress to resolve the issue. p2
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants