Skip to content

(@aws-cdk/aws-ec2-alpha): Optional locale on IPAM pool is not optional #34179

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

Closed
1 task
pemattr opened this issue Apr 17, 2025 · 2 comments · Fixed by #34245
Closed
1 task

(@aws-cdk/aws-ec2-alpha): Optional locale on IPAM pool is not optional #34179

pemattr opened this issue Apr 17, 2025 · 2 comments · Fixed by #34245
Assignees
Labels
bug This issue is a bug. p2 package/tools Related to AWS CDK Tools or CLI

Comments

@pemattr
Copy link

pemattr commented Apr 17, 2025

Describe the bug

https://docs.aws.amazon.com/cdk/api/v2/docs/@aws-cdk_aws-ec2-alpha.PoolOptions.html

The above defines locale as an optional string but when trying to synth it fails with the below error.

Regression Issue

  • Select this option if this issue appears to be a regression.

Last Known Working CDK Version

No response

Expected Behavior

Undefined is accepted and a default is used? But considering I'm providing multiple operatingRegions maybe that isn't possible

Current Behavior

/home/..../node_modules/@aws-cdk/aws-ec2-alpha/lib/ipam.ts:582
    throw new Error(`The provided locale '${poolOptions.locale}' is not in the operating regions.`);
          ^
Error: The provided locale 'undefined' is not in the operating regions.
    at createIpamPool (/home/..../node_modules/@aws-cdk/aws-ec2-alpha/lib/ipam.ts:582:11)
    at IpamScopeBase.addPool (/home/..../node_modules/@aws-cdk/aws-ec2-alpha/lib/ipam.ts:466:12)
    at Object.<anonymous> (/home/..../src/index.ts:19:19)
    at Module._compile (node:internal/modules/cjs/loader:1554:14)
    at Module.m._compile (/home/..../node_modules/ts-node/src/index.ts:1618:23)
    at node:internal/modules/cjs/loader:1706:10
    at Object.require.extensions.<computed> [as .ts] (/home/..../node_modules/ts-node/src/index.ts:1621:12)
    at Module.load (node:internal/modules/cjs/loader:1289:32)
    at Function._load (node:internal/modules/cjs/loader:1108:12)
    at TracingChannel.traceSync (node:diagnostics_channel:322:14)
Subprocess exited with error 1

Reproduction Steps

import { App, Stack } from 'aws-cdk-lib';
import { AddressFamily, Ipam } from '@aws-cdk/aws-ec2-alpha';
const app = new App();

const stack = new Stack(app, 'Stack');

const ipam = new Ipam(stack, 'Ipam');

ipam.privateScope.addPool('Pool', {
  addressFamily: AddressFamily.IP_V4,
});

Possible Solution

locale probably needs to be made non-optional

Additional Information/Context

No response

CDK CLI Version

2.1010.0 (build 6b421db)

Framework Version

No response

Node.js Version

v22.14.0

OS

Windows 11 WSL2 Ubuntu 24.04.2 LTS

Language

TypeScript

Language Version

No response

Other information

No response

@pemattr pemattr added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Apr 17, 2025
@github-actions github-actions bot added the package/tools Related to AWS CDK Tools or CLI label Apr 17, 2025
@ykethan
Copy link
Contributor

ykethan commented Apr 17, 2025

Hey @pemattr, thank you for reporting this issue. I was indeed to able to run into this error message when utilizing the following

import { AddressFamily, Ipam } from "@aws-cdk/aws-ec2-alpha";
import { App, Stack } from "aws-cdk-lib";

export class Repro34179Stack extends Stack {
  constructor(scope: App, id: string) {
    super(scope, id);

    const ipam = new Ipam(this, "Ipam");
    ipam.privateScope.addPool("Pool", {
      addressFamily: AddressFamily.IP_V4,
    });
  }
}

The locale does appear to be an optional property as well on CloudFormation documentation](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-ec2-ipampool.html#cfn-ec2-ipampool-locale) but the operation does appear to determine the operating region from locale.

function createIpamPool(
scope: Construct,
id: string,
scopeOptions: IpamScopeProps,
poolOptions: PoolOptions,
scopeId: string,
): IpamPool {
const isLocaleInOperatingRegions = scopeOptions.ipamOperatingRegions
? scopeOptions.ipamOperatingRegions.map(region => ({ regionName: region }))
.some(region => region.regionName === poolOptions.locale)
: false;
if (!isLocaleInOperatingRegions) {
throw new Error(`The provided locale '${poolOptions.locale}' is not in the operating regions.`);
}

similar to #34178, we will need to update the logic and feature gaps to provide a better experience with this construct.

From the top of the my mind we may need to make the locale as required with possibly a enum with the supported operating regions.

on additional, testing noticed passing in supported Local Zones as locale did not mitigate the issue.

import { AddressFamily, Ipam } from "@aws-cdk/aws-ec2-alpha";
import { App, Stack } from "aws-cdk-lib";

export class Repro34179Stack extends Stack {
  constructor(scope: App, id: string) {
    super(scope, id);

    const ipam = new Ipam(this, "Ipam");
    ipam.privateScope.addPool("Pool", {
      addressFamily: AddressFamily.IP_V4,
      locale: "us-east-1-msp-1",
    });
  }
}

@ykethan ykethan added p2 and removed needs-triage This issue or PR still needs to be triaged. labels Apr 17, 2025
@github-actions github-actions bot added potential-regression Marking this issue as a potential regression to be checked by team member and removed potential-regression Marking this issue as a potential regression to be checked by team member labels Apr 23, 2025
@mergify mergify bot closed this as completed in #34245 Apr 24, 2025
mergify bot pushed a commit that referenced this issue Apr 24, 2025
…pecified (#34245)

### Issue # (if applicable)

Closes #34179 

### Reason for this change

The current implementation throws an error when an IPAM pool's locale is not in the operating regions, even though locale is an optional field in CloudFormation. However, according to AWS documentation, while locale is optional, if it is specified it must be one of the operating regions.

### Description of changes

- Kept the locale validation check conditional on `poolOptions.locale` being provided, since locale is an optional field
- Changed the error message to be more descriptive about the requirement that locale must be configured as an operating region when specified
- Maintained strict validation when locale is provided to match AWS CloudFormation's behavior where it will error if the locale is not in operating regions
- Updated code comments to better reflect AWS's requirements

The key design decision was to keep the error throw (rather than changing to a warning) because:
1. CloudFormation will fail if locale isn't in operating regions
2. It's better to catch this at the CDK level with a clear error message
3. This matches AWS's documented behaviour

### Describe any new or updated permissions being added

No new or updated IAM permissions are required for this change.

### Description of how you validated changes

The changes maintain the existing behavior when locale is provided but improve the error message clarity. No new test cases were needed as this is a refinement of error handling rather than a functional change.

### Checklist
- [x] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md)

----

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

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 24, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug This issue is a bug. p2 package/tools Related to AWS CDK Tools or CLI
Projects
None yet
3 participants