eks: Changing securityGroup deletes entire cluster

[gricey432]

Describe the bug

Adding a custom securityGroup to eks.Cluster causes a full replacement of the stateful k8s cluster and data loss.

Expected Behavior

securityGroup would be updated in place as it is through the console, SDK, and as indicated in cdk diff.

Current Behavior

The custom resource quietly created a new, empty EKS cluster and then deleted the existing cluster.

Reproduction Steps

Should be reproducible by

1. Create a barebones eks.Cluster with no securityGroup
2. Deploy
3. Add a custom securityGroup to the cluster
4. Deploy

Possible Solution

Looking at the onEventHandler logs for the custom resource, it looks like the issue is that changing the SG caused this update set

{
    "updates": {
        "replaceName": false,
        "replaceVpc": true,
        "updateAccess": false,
        "replaceRole": false,
        "updateVersion": false,
        "updateEncryption": false,
        "updateLogging": false
    }
}

Looks like the check for replaceVpc is very rough and way too trigger happy

https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/custom-resource-handlers/lib/aws-eks/cluster-resource-handler/cluster.ts#L330

In fact it looks like the API can replace subnets and security groups in place https://docs.aws.amazon.com/eks/latest/APIReference/API_UpdateClusterConfig.html

The eks.Cluster resource takes a vpc prop, I think the changing of that prop should be the only reason that replaceVpc needs to be true...

Additional Information/Context

No response

CDK CLI Version

2.113.0 (build ccd534a)

Framework Version

No response

Node.js Version

v18.17.1

OS

Windows 11

Language

TypeScript

Language Version

4.9.3

Other information

#25544 may have aided disaster recovery here

I would like to have a Stack Policy on this stack but since it's implemented as a custom resource, cloudformation can only tell me that the CR itself isn't going to be replaced, but can't stop / inform me that the CR is going to be destructive.

[pahud]

Yes I think it's possible to just update the cluster security groups without replacing the cluster. At this moment the cluster construct from aws-eks is using customer resource to create the cluster but if we look at SecurityGroupsIds, it's update with no Interruption.

Agree we should fix replaceVpc.

[gricey432]

I've created #28609 to capture the broader idea of removing this custom resource all together.

This smaller improvement could still be implemented though, given the scale of that other ticket.

Update, that's closed in favour of #24059

[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.