fix(eks): fail to update cluster by disabling logging props (#24688)

* fix

* update path

* delete

Author: pahud

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

@@ -5,6 +5,8 @@ import { IsCompleteResponse, OnEventResponse } from '@aws-cdk/custom-resources/l
 // eslint-disable-next-line import/no-extraneous-dependencies
 import * as aws from 'aws-sdk';
 import { EksClient, ResourceEvent, ResourceHandler } from './common';
+import { compareLoggingProps } from './compareLogging';
+
 
 const MAX_CLUSTER_NAME_LEN = 100;
 
@@ -25,6 +27,9 @@ export class ClusterResourceHandler extends ResourceHandler {
 
     this.newProps = parseProps(this.event.ResourceProperties);
     this.oldProps = event.RequestType === 'Update' ? parseProps(event.OldResourceProperties) : {};
+    // compare newProps and oldProps and update the newProps by appending disabled LogSetup if any
+    const compared: Partial<aws.EKS.CreateClusterRequest> = compareLoggingProps(this.oldProps, this.newProps);
+    this.newProps.logging = compared.logging;
   }
 
   // ------

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

@@ -0,0 +1,45 @@
+/**
+ * This function compares the logging configuration from oldProps and newProps and returns
+ * the result that contains LogSetup with enabled:false if any.
+ *
+ * @param oldProps old properties
+ * @param newProps new properties
+ * @returns result with LogSet with enabled:false if any
+ */
+
+export function compareLoggingProps(oldProps: Partial<AWS.EKS.CreateClusterRequest>,
+  newProps: Partial<AWS.EKS.CreateClusterRequest>): Partial<AWS.EKS.CreateClusterRequest> {
+  const result: Partial<AWS.EKS.CreateClusterRequest> = { logging: {} };
+  let enabledTypes: AWS.EKS.LogType[] = [];
+  let disabledTypes: AWS.EKS.LogType[] = [];
+
+  if (newProps.logging?.clusterLogging === undefined && oldProps.logging?.clusterLogging === undefined) {
+    return newProps;
+  }
+  // if newProps containes LogSetup
+  if (newProps.logging && newProps.logging.clusterLogging && newProps.logging.clusterLogging.length > 0) {
+    enabledTypes = newProps.logging.clusterLogging[0].types!;
+    // if oldProps contains LogSetup with enabled:true
+    if (oldProps.logging && oldProps.logging.clusterLogging && oldProps.logging.clusterLogging.length > 0) {
+      // LogType in oldProp but not in newProp should be considered disabled(enabled:false)
+      disabledTypes = oldProps.logging!.clusterLogging![0].types!.filter(t => !newProps.logging!.clusterLogging![0].types!.includes(t));
+    }
+  } else {
+    // all enabled:true in oldProps will be enabled:false
+    disabledTypes = oldProps.logging!.clusterLogging![0].types!;
+  }
+
+  if (enabledTypes.length > 0 || disabledTypes.length > 0) {
+    result.logging = { clusterLogging: [] };
+  }
+
+  // append the enabled:false LogSetup to the result
+  if (enabledTypes.length > 0) {
+    result.logging!.clusterLogging!.push({ types: enabledTypes, enabled: true });
+  }
+  // append the enabled:false LogSetup to the result
+  if (disabledTypes.length > 0) {
+    result.logging!.clusterLogging!.push({ types: disabledTypes, enabled: false });
+  }
+  return result;
+}

packages/@aws-cdk/aws-eks/test/compareLog.test.ts

@@ -0,0 +1,89 @@
+import * as aws from 'aws-sdk';
+import * as eks from '../lib';
+import { compareLoggingProps } from '../lib/cluster-resource-handler/compareLogging';
+
+describe('compareLoggingProps', () => {
+
+  type Props = Partial<aws.EKS.CreateClusterRequest>;
+  const oldEnabledTypes: aws.EKS.LogTypes = [eks.ClusterLoggingTypes.API, eks.ClusterLoggingTypes.AUDIT];
+
+  test('when newProps.logging.clusterLogging is undefined, should disable all types with enabled:true in oldProps', () => {
+    const oldProps: Props = {
+      logging: {
+        clusterLogging: [{ types: oldEnabledTypes, enabled: true }],
+      },
+    };
+
+    const newProps: Props = {
+      logging: {},
+    };
+
+    const result = compareLoggingProps(oldProps, newProps);
+
+    expect(result.logging?.clusterLogging).toEqual([{ types: oldEnabledTypes, enabled: false }]);
+  });
+
+  test('when newProps.logging is undefined, should disable all types with enabled:true in oldProps', () => {
+    const oldProps: Props = {
+      logging: {
+        clusterLogging: [{ types: oldEnabledTypes, enabled: true }],
+      },
+    };
+
+    const newProps: Props = {};
+
+    const result = compareLoggingProps(oldProps, newProps);
+
+    expect(result.logging?.clusterLogging).toEqual([{ types: oldEnabledTypes, enabled: false }]);
+  });
+
+  test('should disable types with enabled:true but not defined as enabled:true in newProps', () => {
+    const oldProps: Props = {
+      logging: {
+        clusterLogging: [{ types: oldEnabledTypes, enabled: true }],
+      },
+    };
+
+    const newProps: Props = {
+      logging: {
+        clusterLogging: [{ types: [eks.ClusterLoggingTypes.AUDIT], enabled: true }],
+      },
+    };
+
+    const result = compareLoggingProps(oldProps, newProps);
+
+    expect(result.logging?.clusterLogging).toEqual([{ types: [eks.ClusterLoggingTypes.AUDIT], enabled: true },
+      { types: [eks.ClusterLoggingTypes.API], enabled: false }]);
+  });
+
+  test('when oldProps.logging.clusterLogging is undefined and newProps.logging.clusterLogging is undefined, result should be newProps', () => {
+    const oldProps: Props = {
+      logging: {},
+    };
+
+    const newProps: Props = {
+      logging: {},
+    };
+
+    const result = compareLoggingProps(oldProps, newProps);
+
+    expect(result).toEqual(newProps);
+  });
+
+  test('multiple enabled:true types in oldProps with clusterLogging undefined in newProps should all be disabled', () => {
+    const oldProps: Props = {
+      logging: {
+        clusterLogging: [{ types: oldEnabledTypes, enabled: true }],
+      },
+    };
+
+    const newProps: Props = {
+      logging: {},
+    };
+
+    const result = compareLoggingProps(oldProps, newProps);
+
+    expect(result.logging?.clusterLogging).toEqual([{ types: oldEnabledTypes, enabled: false }]);
+  });
+
+});