-
Notifications
You must be signed in to change notification settings - Fork 4.1k
(@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
Comments
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. aws-cdk/packages/@aws-cdk/aws-ec2-alpha/lib/ipam.ts Lines 569 to 583 in 0438773
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",
});
}
} |
…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*
Comments on closed issues and PRs are hard for our team to see. |
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
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 possibleCurrent Behavior
Reproduction Steps
Possible Solution
locale
probably needs to be made non-optionalAdditional 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
The text was updated successfully, but these errors were encountered: