fix(eks): changing the subnets or securityGroupIds order causes an er…

…ror (aws#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: [aws#24162](aws#24162)

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

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

@@ -326,8 +326,8 @@ function analyzeUpdate(oldProps: Partial<aws.EKS.CreateClusterRequest>, newProps
   return {
     replaceName: newProps.name !== oldProps.name,
     replaceVpc:
-      JSON.stringify(newVpcProps.subnetIds) !== JSON.stringify(oldVpcProps.subnetIds) ||
-      JSON.stringify(newVpcProps.securityGroupIds) !== JSON.stringify(oldVpcProps.securityGroupIds),
+      JSON.stringify(newVpcProps.subnetIds?.sort()) !== JSON.stringify(oldVpcProps.subnetIds?.sort()) ||
+      JSON.stringify(newVpcProps.securityGroupIds?.sort()) !== JSON.stringify(oldVpcProps.securityGroupIds?.sort()),
     updateAccess:
       newVpcProps.endpointPrivateAccess !== oldVpcProps.endpointPrivateAccess ||
       newVpcProps.endpointPublicAccess !== oldVpcProps.endpointPublicAccess ||

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

@@ -280,6 +280,25 @@ describe('cluster resource provider', () => {
         });
       });
 
+      test('change subnets or security groups order should not trigger an update', async () => {
+        const handler = new ClusterResourceHandler(mocks.client, mocks.newRequest('Update', {
+          ...mocks.MOCK_PROPS,
+          resourcesVpcConfig: {
+            subnetIds: ['subnet1', 'subnet2'],
+            securityGroupIds: ['sg1', 'sg2'],
+          },
+        }, {
+          ...mocks.MOCK_PROPS,
+          resourcesVpcConfig: {
+            subnetIds: ['subnet2', 'subnet1'],
+            securityGroupIds: ['sg2', 'sg1'],
+          },
+        }));
+        const resp = await handler.onEvent();
+
+        expect(resp).toEqual(undefined);
+      });
+
       test('"roleArn" requires a replacement', async () => {
         const handler = new ClusterResourceHandler(mocks.client, mocks.newRequest('Update', {
           roleArn: 'new-arn',

packages/@aws-cdk/aws-eks/test/integ.eks-cluster.js.snapshot/asset.73edfa4462023915a2f13bf570acae05c5111817c606f9837f832152920ba517/cluster.ts → packages/@aws-cdk/aws-eks/test/integ.eks-cluster.js.snapshot/asset.42ee7a787eab7842c3d815365d104cacb9168fb9beb8a1bc019b0a6d7e969bee/cluster.ts

@@ -136,10 +136,16 @@ export class ClusterResourceHandler extends ResourceHandler {
       return this.updateClusterVersion(this.newProps.version);
     }
 
+    if (updates.updateLogging && updates.updateAccess) {
+      throw new Error('Cannot update logging and access at the same time');
+    }
+
     if (updates.updateLogging || updates.updateAccess) {
       const config: aws.EKS.UpdateClusterConfigRequest = {
         name: this.clusterName,
-        logging: this.newProps.logging,
+      };
+      if (updates.updateLogging) {
+        config.logging = this.newProps.logging;
       };
       if (updates.updateAccess) {
         // 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
   return {
     replaceName: newProps.name !== oldProps.name,
     replaceVpc:
-      JSON.stringify(newVpcProps.subnetIds) !== JSON.stringify(oldVpcProps.subnetIds) ||
-      JSON.stringify(newVpcProps.securityGroupIds) !== JSON.stringify(oldVpcProps.securityGroupIds),
+      JSON.stringify(newVpcProps.subnetIds?.sort()) !== JSON.stringify(oldVpcProps.subnetIds?.sort()) ||
+      JSON.stringify(newVpcProps.securityGroupIds?.sort()) !== JSON.stringify(oldVpcProps.securityGroupIds?.sort()),
     updateAccess:
       newVpcProps.endpointPrivateAccess !== oldVpcProps.endpointPrivateAccess ||
       newVpcProps.endpointPublicAccess !== oldVpcProps.endpointPublicAccess ||
@@ -334,5 +340,5 @@ function analyzeUpdate(oldProps: Partial<aws.EKS.CreateClusterRequest>, newProps
 }
 
 function setsEqual(first: Set<string>, second: Set<string>) {
-  return first.size === second.size || [...first].every((e: string) => second.has(e));
+  return first.size === second.size && [...first].every((e: string) => second.has(e));
 }