aws-eks: eks.Cluster - Changing the subnets or securityGroupIds order causes an error

[AviorSchreiber]

Describe the bug

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 the change as an unsupported update, although the list has the same items (just in a different order)

Expected Behavior

If the subnetIds or securityGroupIds does not change and only the order has been changed, the analyzeUpdate function should return replaceVpc: false

Current Behavior

The update failed with an exception:

Received response status [FAILED] from custom resource. Message returned: Cannot replace cluster "XXXXXXX" since it has an explicit physical name. Either rename the cluster or remove the "name" configuration Logs: /aws/lambda/eks-prd-Clus-OnEventHandler42BEBAE0-aIAWgDBSENKr at ClusterResourceHandler.onUpdate (/var/task/cluster.js:1:2296) at ClusterResourceHandler.onEvent (/var/task/common.js:1:680) at Runtime.onEvent [as handler] (/var/task/index.js:1:1434) at Runtime.handleOnceNonStreaming (/var/runtime/Runtime.js:74:25) (RequestId: 28e7ed6c-d32f-4cfb-91cc-86305fee567e)

Reproduction Steps

Step 1:
Deploy the following CDK app

import { KubectlV22Layer } from "@aws-cdk/lambda-layer-kubectl-v22";
import { App, Stack, StackProps } from "aws-cdk-lib";
import * as ec2 from "aws-cdk-lib/aws-ec2";
import * as eks from "aws-cdk-lib/aws-eks";
import { Construct } from "constructs";

class EksClusterStack extends Stack {
  constructor(scope: Construct, id: string, props: StackProps) {
    super(scope, id, props);

    const clusterSubnets = {
      privateSubnetIDs: [
        "subnet-xxxxxxxxxxxxx",
        "subnet-yyyyyyyyyyyyy",
        "subnet-zzzzzzzzzzzzz",
      ],
    };

    const clusterVPC = ec2.Vpc.fromLookup(this, "ClusterVPC", {
      vpcId: "vpc-xxxxxxx",
    });

    const eksControlPlaneSubnets: ec2.SubnetSelection = {
      subnets: clusterSubnets.privateSubnetIDs.map((subnetID) => {
        return ec2.Subnet.fromSubnetId(this, `Subnet-${subnetID}`, subnetID);
      }),
    };

    new eks.Cluster(this, "EKSCluster", {
      version: eks.KubernetesVersion.V1_23,
      clusterName: "test",
      endpointAccess: eks.EndpointAccess.PRIVATE,
      kubectlLayer: new KubectlV22Layer(this, `KubectlLayer1.22`),
      defaultCapacity: 0,
      vpc: clusterVPC,
      vpcSubnets: [eksControlPlaneSubnets],
      placeClusterHandlerInVpc: true,
    });
  }
}

const app = new App();
new EksClusterStack(app, "DeployKubernetesCluster", {
  env: {
    account: "123456789012",
    region: "us-east-1",
  },
});

Step 2

Change the order of subnets in the clusterSubnets variable

const clusterSubnets = {
      privateSubnetIDs: [
        "subnet-yyyyyyyyyyyyy",
        "subnet-xxxxxxxxxxxxx",
        "subnet-zzzzzzzzzzzzz",
      ],
    };

Step 3
Update the stack with the changes.

Possible Solution

The change is analyzed by a lambda function that returns for each component if it changed or not.
The bug occurs due to the comparison mechanism, which takes into account the order of the list items and not just the list contents.

function analyzeUpdate(oldProps: Partial<aws.EKS.CreateClusterRequest>, newProps: aws.EKS.CreateClusterRequest): UpdateMap {
  ...

  return {
    ...
    replaceVpc:
      JSON.stringify(newVpcProps.subnetIds) !== JSON.stringify(oldVpcProps.subnetIds) ||
      JSON.stringify(newVpcProps.securityGroupIds) !== JSON.stringify(oldVpcProps.securityGroupIds),
    ...
  };
}

The following fix to the analyzeUpdate function in file packages/@aws-cdk/aws-eks/lib/cluster-resource-handler/cluster.ts can solve the issue

function analyzeUpdate(oldProps: Partial<aws.EKS.CreateClusterRequest>, newProps: aws.EKS.CreateClusterRequest): UpdateMap {
  ...

  return {
    ...
    replaceVpc:
      JSON.stringify(newVpcProps.subnetIds?.sort()) !== JSON.stringify(oldVpcProps.subnetIds?.sort()) ||
      JSON.stringify(newVpcProps.securityGroupIds?.sort()) !== JSON.stringify(oldVpcProps.securityGroupIds?.sort()),
    ...
  };
}

Additional Information/Context

No response

CDK CLI Version

2.53.0 (build 7690f43)

Framework Version

No response

Node.js Version

v14.19.0

OS

Mac

Language

Typescript

Language Version

TypeScript: 4.7.3

Other information

No response

[peterwoodworth]

Thanks for submitting this bug, this description makes sense that this is occurring. And thanks for the PR as well

I'm curious what the use case is where the just the order changes?

[AviorSchreiber]

@peterwoodworth

We are using CDK to create sort of products, where the end-user fill out information such as vpc/subnets/etc.
As it is an external parameter we cannot control how the user will send the subnet list.

[pahud]

Yes I think the sort() method should fix this. Are you interested to submit a PR for that? @AviorSchreiber ?

[AviorSchreiber]

Yes I think the sort() method should fix this. Are you interested to submit a PR for that? @AviorSchreiber ?

There is an open PR for that,
Waiting for approval
@pahud can you assist with that?

[pahud]

@AviorSchreiber Seems @Naumel has been working on the PR with you. Feel free to discuss with @Naumel directly there. :)

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.