fix(ec2): wrap L1 CfnSubnet in selectSubnetObjects so SubnetIds resolve in templates - #37896
Open
Zelys-DFKH wants to merge 4 commits into
Open
fix(ec2): wrap L1 CfnSubnet in selectSubnetObjects so SubnetIds resolve in templates#37896Zelys-DFKH wants to merge 4 commits into
Zelys-DFKH wants to merge 4 commits into
Conversation
aws-cdk-automation
previously requested changes
May 16, 2026
aws-cdk-automation
dismissed
their stale review
May 16, 2026 16:49
✅ Updated pull request passes all PRLinter validations. Dismissing previous PRLinter review.
This was referenced Jun 24, 2026
…ve in templates When CfnSubnet (L1) objects are passed via SubnetSelection.subnets, the synthesized template produces empty SubnetIds because CfnSubnet does not implement ISubnet.subnetId. selectSubnetObjects() now detects L1 objects via CfnSubnet.isCfnSubnet() and wraps them with Subnet.fromSubnetAttributes(), backed by the L1 ref token. A singleton guard keyed on Node.of(subnet).addr prevents duplicate construct errors on repeated selectSubnets() calls with the same CfnSubnet. Closes aws#37144
Zelys-DFKH
force-pushed
the
fix/ec2-cfnsubnet-selectsubnetobjects-37144
branch
from
July 16, 2026 17:21
af70076 to
febd905
Compare
Contributor
Author
|
CI is red because the integration test added here is missing part of its snapshot directory. The error is ENOENT on test/aws-ec2/test/integ.vpc-endpoint-cfn-subnet.js.snapshot/aws-cdk-ec2-vpc-endpoint-cfn-subnet.metadata.json, and neither stack in that snapshot has a .metadata.json file. I have not run cdk-integ with --dry-run or written the snapshot by hand, since CONTRIBUTING asks contributors not to. Could a maintainer run the integration test so it gets generated from a real deployment? |
The snapshot added with this fix was generated against an incomplete local build, so it was missing the default-security-group restriction custom resource and carried a stale latest-node-runtime mapping. Regenerated against a full build so it matches what current sources synthesize.
abidhasan-aws
temporarily deployed
to
automation
August 7, 2026 11:03 — with
GitHub Actions
Inactive
abidhasan-aws
temporarily deployed
to
automation
August 7, 2026 11:04 — with
GitHub Actions
Inactive
aws-cdk-automation
temporarily deployed
to
automation
August 7, 2026 12:01 — with
GitHub Actions
Inactive
aws-cdk-automation
temporarily deployed
to
automation
August 7, 2026 12:01 — with
GitHub Actions
Inactive
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #37144
Exemption Request: This fix changes internal behavior of
VpcBase.selectSubnetObjects(). No new CloudFormation resource types, properties, or cross-service integrations. Per AGENTS.md, integration tests are required for "new CFN resource types, new CFN properties, cross-service integrations, new versions, Custom Resources". This fix is none of those. Two unit tests withTemplate.fromStackassertions cover the regression and the singleton guard. An integration test file is included (integ.vpc-endpoint-cfn-subnet.ts) but generating the.js.snapshotrequires runninginteg-runner, which needs a full local build ofaws-cdk-lib. I can generate it if a reviewer can point me to a way to run the runner or confirm the expected template output.Credit to @pahud, whose diagnosis in the issue thread got the root cause and fix direction right. And to @Aaxhirrr, who implemented the same approach in #37179. This PR picks up where that one left off with a couple of additions.
Why this breaks
When
CfnSubnet(L1) objects are passed viaSubnetSelection.subnets, the synthesized template produces emptySubnetIds.VpcBase.selectSubnetObjects()returns the array unchanged, and downstream code calls.subnetIdon each entry, a property from theISubnetinterface thatCfnSubnetdoesn't have. CDK drops theundefinedentries without any warning.What changed
selectSubnetObjects()now usesCfnSubnet.isCfnSubnet()to identify L1 objects and wraps them viaSubnet.fromSubnetAttributes(), producing a realISubnetbacked by the L1's{ Ref }token. The wrapper construct ID is keyed onNode.of(subnet).addr(a unique hash of the full construct path) so repeated calls toselectSubnets()with the sameCfnSubnetreuse the existing wrapper instead of creating duplicates.SubnetSelection.subnetsJSDoc is updated to document thatCfnSubnetobjects are accepted and auto-wrapped.Tests
Two regression tests in
vpc-endpoint.test.ts:CfnSubnetpassed toaddInterfaceEndpoint: assertsSubnetIds: [{ Ref: "<logicalId>" }]. Fails on unfixed code, passes on the fix.CfnSubnetpassed to two endpoints: confirms the singleton guard doesn't throw a duplicate-construct error on repeated calls.Checklist