Skip to content

Commit 73744b4

Browse files
shikha372shikha372
authored
fix(eks): looked up vpc causing premature validation errors for private subnets (#33786)
### Issue # (if applicable) Related to one of the related issue for EKS and VPC in #22025 . ### Reason for this change Currently EKS cluster with private endpoint access fails during `cdk synth` operation for a lookedup VPC. investigating further into lookup implementation, the VPC id is first populated through some dummy values including the one for private subnets : ``` { name: 'Private', type: cxapi.VpcSubnetGroupType.PRIVATE, subnets: [ { availabilityZone: 'dummy1a', subnetId: 'p-12345', routeTableId: 'rtb-12345p', cidr: '1.2.3.4/5', }, ``` But there are no dummy values defined for the case of privatesubnetIds : ``` privateSubnetIds: undefined, ``` which results in return of filtering by IDs option as a null object until the values are fully resolved. Reference code: https://github.com/aws/aws-cdk/blob/main/packages/aws-cdk-lib/aws-ec2/lib/vpc.ts#L2705 ### Description of changes using existing field `isPendingLookup` in the SubnetSelection which is being set on the basis of this.incompleteSubnetDefinition = isIncomplete; where `isIncomplete` is set to false during first pass of cdk synth. So during first synth operation, validation will be skipped. ### Describe any new or updated permissions being added NA ### Description of how you validated changes Added unit test and integration test ### 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*
1 parent 8b23b5d commit 73744b4

File tree

2 files changed

+260
-9
lines changed

2 files changed

+260
-9
lines changed

‎packages/aws-cdk-lib/aws-eks/lib/cluster.ts‎

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1656,22 +1656,30 @@ export class Cluster extends ClusterBase {
16561656
&& this.endpointAccess._config.publicCidrs
16571657
&& this.endpointAccess._config.publicCidrs.length !== 0;
16581658

1659-
// validate endpoint access configuration
1659+
// Check if any subnet selection is pending lookup
1660+
const hasPendingLookup = this.vpcSubnets.some(placement =>
1661+
this.vpc.selectSubnets(placement).isPendingLookup,
1662+
);
16601663

1661-
if (privateSubnets.length === 0 && publicAccessDisabled) {
1662-
// no private subnets and no public access at all, no good.
1663-
throw new Error('Vpc must contain private subnets when public endpoint access is disabled');
1664-
}
1664+
// validate endpoint access configuration
1665+
if (!hasPendingLookup) {
1666+
if (privateSubnets.length === 0 && publicAccessDisabled) {
1667+
// no private subnets and no public access at all, no good.
1668+
throw new Error('Vpc must contain private subnets when public endpoint access is disabled');
1669+
}
16651670

1666-
if (privateSubnets.length === 0 && publicAccessRestricted) {
1671+
if (privateSubnets.length === 0 && publicAccessRestricted) {
16671672
// no private subnets and public access is restricted, no good.
1668-
throw new Error('Vpc must contain private subnets when public endpoint access is restricted');
1673+
throw new Error('Vpc must contain private subnets when public endpoint access is restricted');
1674+
}
16691675
}
16701676

16711677
const placeClusterHandlerInVpc = props.placeClusterHandlerInVpc ?? false;
16721678

1673-
if (placeClusterHandlerInVpc && privateSubnets.length === 0) {
1674-
throw new Error('Cannot place cluster handler in the VPC since no private subnets could be selected');
1679+
if (!hasPendingLookup) {
1680+
if (placeClusterHandlerInVpc && privateSubnets.length === 0) {
1681+
throw new Error('Cannot place cluster handler in the VPC since no private subnets could be selected');
1682+
}
16751683
}
16761684

16771685
if (props.clusterHandlerSecurityGroup && !placeClusterHandlerInVpc) {

‎packages/aws-cdk-lib/aws-eks/test/cluster.test.ts‎

Lines changed: 243 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,96 @@ describe('cluster', () => {
245245
});
246246
});
247247

248+
test('should not throw when using vpc lookup with placeClusterHandlerInVpc and subnet filtering by ID', () => {
249+
const vpcId = 'vpc-12345';
250+
// can't use the regular fixture because it also adds a VPC to the stack, which prevents
251+
// us from setting context.
252+
const stack = new cdk.Stack(new cdk.App(), 'Stack', {
253+
env: {
254+
account: '11112222',
255+
region: 'us-east-1',
256+
},
257+
});
258+
stack.node.setContext(`vpc-provider:account=${stack.account}:filter.vpc-id=${vpcId}:region=${stack.region}:returnAsymmetricSubnets=true`, {
259+
vpcId: vpcId,
260+
vpcCidrBlock: '10.0.0.0/16',
261+
subnetGroups: [
262+
{
263+
name: 'Private',
264+
type: 'Private',
265+
subnets: [
266+
{
267+
subnetId: 'subnet-private-1',
268+
cidr: '10.0.1.0/24',
269+
availabilityZone: 'us-east-1a',
270+
routeTableId: 'rtb-123',
271+
},
272+
{
273+
subnetId: 'subnet-private-2',
274+
cidr: '10.0.2.0/24',
275+
availabilityZone: 'us-east-1b',
276+
routeTableId: 'rtb-456',
277+
},
278+
],
279+
},
280+
{
281+
name: 'Public',
282+
type: 'Public',
283+
subnets: [
284+
{
285+
subnetId: 'subnet-public-1',
286+
cidr: '10.0.3.0/24',
287+
availabilityZone: 'us-east-1a',
288+
routeTableId: 'rtb-789',
289+
},
290+
],
291+
},
292+
],
293+
});
294+
295+
const vpc = ec2.Vpc.fromLookup(stack, 'Vpc', {
296+
vpcId: vpcId,
297+
});
298+
const securityGroup = new ec2.SecurityGroup(stack, 'ProxyInstanceSG', {
299+
vpc,
300+
allowAllOutbound: false,
301+
});
302+
303+
// This should not throw
304+
new eks.Cluster(stack, 'Cluster', {
305+
version: CLUSTER_VERSION,
306+
vpc,
307+
placeClusterHandlerInVpc: true,
308+
clusterHandlerSecurityGroup: securityGroup,
309+
vpcSubnets: [{
310+
subnetFilters: [
311+
ec2.SubnetFilter.byIds(['subnet-private-1', 'subnet-private-2']),
312+
],
313+
}],
314+
kubectlLayer: new KubectlV31Layer(stack, 'KubectlLayer'),
315+
});
316+
317+
const nested = stack.node.tryFindChild('@aws-cdk/aws-eks.ClusterResourceProvider') as cdk.NestedStack;
318+
319+
// verify that security group id is configured properly
320+
Template.fromStack(nested).hasResourceProperties('AWS::Lambda::Function', {
321+
VpcConfig: {
322+
SecurityGroupIds: [{ Ref: 'referencetoStackProxyInstanceSG80B79D87GroupId' }],
323+
},
324+
});
325+
326+
// Verify the cluster is created with the correct subnets
327+
Template.fromStack(stack).hasResourceProperties('Custom::AWSCDK-EKS-Cluster', {
328+
Config: Match.objectLike({
329+
roleArn: { 'Fn::GetAtt': ['ClusterRoleFA261979', 'Arn'] },
330+
version: CLUSTER_VERSION.version,
331+
resourcesVpcConfig: {
332+
subnetIds: ['subnet-private-1', 'subnet-private-2'],
333+
},
334+
}),
335+
});
336+
});
337+
248338
test('security group of self-managed asg is not tagged with owned', () => {
249339
// GIVEN
250340
const { stack, vpc } = testFixture();
@@ -2891,6 +2981,159 @@ describe('cluster', () => {
28912981
});
28922982
});
28932983

2984+
test('private endpoint access selects private subnets from looked up vpc for filtering by IDs with given context', () => {
2985+
const vpcId = 'vpc-12345';
2986+
// can't use the regular fixture because it also adds a VPC to the stack, which prevents
2987+
// us from setting context.
2988+
const stack = new cdk.Stack(new cdk.App(), 'Stack', {
2989+
env: {
2990+
account: '11112222',
2991+
region: 'us-east-1',
2992+
},
2993+
});
2994+
2995+
stack.node.setContext(`vpc-provider:account=${stack.account}:filter.vpc-id=${vpcId}:region=${stack.region}:returnAsymmetricSubnets=true`, {
2996+
vpcId: vpcId,
2997+
vpcCidrBlock: '10.0.0.0/16',
2998+
subnetGroups: [
2999+
{
3000+
name: 'Private',
3001+
type: 'Private',
3002+
subnets: [
3003+
{
3004+
subnetId: 'subnet-private-in-us-east-1a',
3005+
cidr: '10.0.1.0/24',
3006+
availabilityZone: 'us-east-1a',
3007+
routeTableId: 'rtb-06068e4c4049921ef',
3008+
},
3009+
],
3010+
},
3011+
{
3012+
name: 'Public',
3013+
type: 'Public',
3014+
subnets: [
3015+
{
3016+
subnetId: 'subnet-public-in-us-east-1c',
3017+
cidr: '10.0.0.0/24',
3018+
availabilityZone: 'us-east-1c',
3019+
routeTableId: 'rtb-0ff08e62195198dbb',
3020+
},
3021+
],
3022+
},
3023+
],
3024+
});
3025+
3026+
const vpc = ec2.Vpc.fromLookup(stack, 'Vpc', {
3027+
vpcId: vpcId,
3028+
});
3029+
3030+
new eks.Cluster(stack, 'Cluster', {
3031+
vpc,
3032+
version: CLUSTER_VERSION,
3033+
prune: false,
3034+
endpointAccess: eks.EndpointAccess.PRIVATE,
3035+
vpcSubnets: [{
3036+
subnetFilters: [
3037+
ec2.SubnetFilter.byIds(['subnet-private-in-us-east-1a']),
3038+
],
3039+
}],
3040+
kubectlLayer: new KubectlV31Layer(stack, 'KubectlLayer'),
3041+
});
3042+
3043+
const nested = stack.node.tryFindChild('@aws-cdk/aws-eks.KubectlProvider') as cdk.NestedStack;
3044+
Template.fromStack(nested).hasResourceProperties('AWS::Lambda::Function', {
3045+
VpcConfig: { SubnetIds: ['subnet-private-in-us-east-1a'] },
3046+
});
3047+
});
3048+
3049+
test('private endpoint access skips validation for private subnets from looked up vpc for filtering by IDs with no context', () => {
3050+
const vpcId = 'vpc-12345';
3051+
const stack = new cdk.Stack(new cdk.App(), 'Stack', {
3052+
env: {
3053+
account: '11112222',
3054+
region: 'us-east-1',
3055+
},
3056+
});
3057+
3058+
const vpc = ec2.Vpc.fromLookup(stack, 'Vpc', {
3059+
vpcId: vpcId,
3060+
});
3061+
3062+
new eks.Cluster(stack, 'Cluster', {
3063+
vpc,
3064+
version: CLUSTER_VERSION,
3065+
prune: false,
3066+
endpointAccess: eks.EndpointAccess.PRIVATE,
3067+
vpcSubnets: [{
3068+
subnetFilters: [
3069+
ec2.SubnetFilter.byIds(['subnet-private-in-us-east-1a']),
3070+
],
3071+
}],
3072+
kubectlLayer: new KubectlV31Layer(stack, 'KubectlLayer'),
3073+
});
3074+
});
3075+
3076+
test('private endpoint access validates private subnets from looked up vpc for other select subnet options', () => {
3077+
const vpcId = 'vpc-12345';
3078+
const stack = new cdk.Stack(new cdk.App(), 'Stack', {
3079+
env: {
3080+
account: '11112222',
3081+
region: 'us-east-1',
3082+
},
3083+
});
3084+
3085+
stack.node.setContext(`vpc-provider:account=${stack.account}:filter.vpc-id=${vpcId}:region=${stack.region}:returnAsymmetricSubnets=true`, {
3086+
vpcId: vpcId,
3087+
vpcCidrBlock: '10.0.0.0/16',
3088+
subnetGroups: [
3089+
{
3090+
name: 'Public',
3091+
type: 'Public',
3092+
subnets: [
3093+
{
3094+
subnetId: 'subnet-public-in-us-east-1c',
3095+
cidr: '10.0.0.0/24',
3096+
availabilityZone: 'us-east-1c',
3097+
routeTableId: 'rtb-0ff08e62195198dbb',
3098+
},
3099+
],
3100+
},
3101+
{
3102+
name: 'Private',
3103+
type: 'Private',
3104+
subnets: [
3105+
{
3106+
subnetId: 'subnet-private-in-us-east-1a',
3107+
cidr: '10.0.1.0/24',
3108+
availabilityZone: 'us-east-1a',
3109+
routeTableId: 'rtb-06068e4c4049921ef',
3110+
},
3111+
],
3112+
},
3113+
],
3114+
});
3115+
3116+
const vpc = ec2.Vpc.fromLookup(stack, 'Vpc', {
3117+
vpcId: vpcId,
3118+
});
3119+
3120+
new eks.Cluster(stack, 'Cluster', {
3121+
vpc,
3122+
version: CLUSTER_VERSION,
3123+
prune: false,
3124+
endpointAccess: eks.EndpointAccess.PRIVATE,
3125+
vpcSubnets: [{
3126+
subnetType: ec2.SubnetType.PRIVATE_WITH_EGRESS,
3127+
}],
3128+
kubectlLayer: new KubectlV31Layer(stack, 'KubectlLayer'),
3129+
});
3130+
3131+
const nested = stack.node.tryFindChild('@aws-cdk/aws-eks.KubectlProvider') as cdk.NestedStack;
3132+
Template.fromStack(nested).hasResourceProperties('AWS::Lambda::Function', {
3133+
VpcConfig: { SubnetIds: ['subnet-private-in-us-east-1a'] },
3134+
});
3135+
});
3136+
28943137
test('private endpoint access selects only private subnets from managed vpc with concrete subnet selection', () => {
28953138
const { stack } = testFixture();
28963139

0 commit comments

Comments
 (0)