Skip to content

Commit 09c2c19

Browse files
fix(eks): changing the subnets or securityGroupIds order causes an error (#24163)
When the subnet list is passed to the EKS Cluster construct in a different order, an update is triggered to the EKS cluster. The update process fails as it falsely identifies a change for an unsupported update, although the list has the same items. The solution is to change the analyzeUpdate function to return `updateVpc: false` if only the securityGroups/subnetsId order has been changed. Fixes: [#24162](#24162) --- By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license
1 parent 1e1131c commit 09c2c19

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

42 files changed

+449
-482
lines changed

Diff for: packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/cluster.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -326,8 +326,8 @@ function analyzeUpdate(oldProps: Partial<aws.EKS.CreateClusterRequest>, newProps
326326
return {
327327
replaceName: newProps.name !== oldProps.name,
328328
replaceVpc:
329-
JSON.stringify(newVpcProps.subnetIds) !== JSON.stringify(oldVpcProps.subnetIds) ||
330-
JSON.stringify(newVpcProps.securityGroupIds) !== JSON.stringify(oldVpcProps.securityGroupIds),
329+
JSON.stringify(newVpcProps.subnetIds?.sort()) !== JSON.stringify(oldVpcProps.subnetIds?.sort()) ||
330+
JSON.stringify(newVpcProps.securityGroupIds?.sort()) !== JSON.stringify(oldVpcProps.securityGroupIds?.sort()),
331331
updateAccess:
332332
newVpcProps.endpointPrivateAccess !== oldVpcProps.endpointPrivateAccess ||
333333
newVpcProps.endpointPublicAccess !== oldVpcProps.endpointPublicAccess ||

Diff for: packages/@aws-cdk/aws-eks/test/cluster-resource-provider.test.ts

+19
Original file line numberDiff line numberDiff line change
@@ -280,6 +280,25 @@ describe('cluster resource provider', () => {
280280
});
281281
});
282282

283+
test('change subnets or security groups order should not trigger an update', async () => {
284+
const handler = new ClusterResourceHandler(mocks.client, mocks.newRequest('Update', {
285+
...mocks.MOCK_PROPS,
286+
resourcesVpcConfig: {
287+
subnetIds: ['subnet1', 'subnet2'],
288+
securityGroupIds: ['sg1', 'sg2'],
289+
},
290+
}, {
291+
...mocks.MOCK_PROPS,
292+
resourcesVpcConfig: {
293+
subnetIds: ['subnet2', 'subnet1'],
294+
securityGroupIds: ['sg2', 'sg1'],
295+
},
296+
}));
297+
const resp = await handler.onEvent();
298+
299+
expect(resp).toEqual(undefined);
300+
});
301+
283302
test('"roleArn" requires a replacement', async () => {
284303
const handler = new ClusterResourceHandler(mocks.client, mocks.newRequest('Update', {
285304
roleArn: 'new-arn',

Diff for: packages/@aws-cdk/aws-eks/test/integ.eks-cluster.js.snapshot/asset.42ee7a787eab7842c3d815365d104cacb9168fb9beb8a1bc019b0a6d7e969bee/cluster.js

+273
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
+10-4
Original file line numberDiff line numberDiff line change
@@ -136,10 +136,16 @@ export class ClusterResourceHandler extends ResourceHandler {
136136
return this.updateClusterVersion(this.newProps.version);
137137
}
138138

139+
if (updates.updateLogging && updates.updateAccess) {
140+
throw new Error('Cannot update logging and access at the same time');
141+
}
142+
139143
if (updates.updateLogging || updates.updateAccess) {
140144
const config: aws.EKS.UpdateClusterConfigRequest = {
141145
name: this.clusterName,
142-
logging: this.newProps.logging,
146+
};
147+
if (updates.updateLogging) {
148+
config.logging = this.newProps.logging;
143149
};
144150
if (updates.updateAccess) {
145151
// Updating the cluster with securityGroupIds and subnetIds (as specified in the warning here:
@@ -320,8 +326,8 @@ function analyzeUpdate(oldProps: Partial<aws.EKS.CreateClusterRequest>, newProps
320326
return {
321327
replaceName: newProps.name !== oldProps.name,
322328
replaceVpc:
323-
JSON.stringify(newVpcProps.subnetIds) !== JSON.stringify(oldVpcProps.subnetIds) ||
324-
JSON.stringify(newVpcProps.securityGroupIds) !== JSON.stringify(oldVpcProps.securityGroupIds),
329+
JSON.stringify(newVpcProps.subnetIds?.sort()) !== JSON.stringify(oldVpcProps.subnetIds?.sort()) ||
330+
JSON.stringify(newVpcProps.securityGroupIds?.sort()) !== JSON.stringify(oldVpcProps.securityGroupIds?.sort()),
325331
updateAccess:
326332
newVpcProps.endpointPrivateAccess !== oldVpcProps.endpointPrivateAccess ||
327333
newVpcProps.endpointPublicAccess !== oldVpcProps.endpointPublicAccess ||
@@ -334,5 +340,5 @@ function analyzeUpdate(oldProps: Partial<aws.EKS.CreateClusterRequest>, newProps
334340
}
335341

336342
function setsEqual(first: Set<string>, second: Set<string>) {
337-
return first.size === second.size || [...first].every((e: string) => second.has(e));
343+
return first.size === second.size && [...first].every((e: string) => second.has(e));
338344
}

Diff for: packages/@aws-cdk/aws-eks/test/integ.eks-cluster.js.snapshot/asset.7215c88dd3e638d28329d4538b36cdbfb54233a4d972181795814f8b904d1037/outbound.js

-45
This file was deleted.

0 commit comments

Comments
 (0)