aws-eks: Recent change breaks update of existing clusters

[resnikb]

Describe the bug

The recent change made in PR #21185 to support logging changes, breaks updates of existing clusters.

Our EKS cluster was created with v2.27 of aws-cdk, and has the following in CF template:

 "logging": {
      "clusterLogging": [
       {
        "enabled": true,
        "types": [
         "api",
         "audit",
         "authenticator",
         "controllerManager",
         "scheduler"
        ]
       }

The PR assumes that there are two elements inside clusterLogging array.

Expected Behavior

The cluster update works without failing.

Current Behavior

Upgrading to CDK 2.35 fails with an error:

Received response status [FAILED] from custom resource. Message returned: Cannot read property 'enabled' of undefined 
Logs: /aws/lambda/XXXX-OnEventHandler42BEBAE0-vzZCvYNs29A1 
    at parseProps (/var/task/cluster.js:1:6085) 
    at new ClusterResourceHandler (/var/task/cluster.js:1:368) 
    at createResourceHandler (/var/task/index.js:1:1642) 
    at Runtime.onEvent [as handler] (/var/task/index.js:1:1361) 
    at Runtime.handleOnceNonStreaming (/var/runtime/Runtime.js:73:25)

Reproduction Steps

To reproduce the bug, create the following cluster with 2.27, deploy, and upgrade CDK to 2.35:

   new Cluster(stack, 'Cluster', {
      clusterName: 'MyCluster',
      version: KubernetesVersion.V1_21,
      vpc,
      clusterLogging: [
        ClusterLoggingTypes.API,
        ClusterLoggingTypes.AUDIT,
        ClusterLoggingTypes.AUTHENTICATOR,
        ClusterLoggingTypes.CONTROLLER_MANAGER,
        ClusterLoggingTypes.SCHEDULER,
      ],
    });

Possible Solution

The resource handler code should check the size of the clusterLogging array before accessing the new element.

Additionally, although this may be a separate bug, the line 294 from the PR looks suspicious:

parsed.logging.clusterLogging[1].enabled = parsed.logging.clusterLogging[1].enabled === 'false';

It seems that enabled will be set to true only if it is "false" in the template?

This should also possible change to

parsed.logging.clusterLogging[1].enabled = parsed.logging.clusterLogging[1].enabled === 'true';

Additional Information/Context

No response

CDK CLI Version

2.35.0 (build 5c23578)

Framework Version

No response

Node.js Version

v16.16.0

OS

Linux

Language

Typescript

Language Version

No response

Other information

No response

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