fix(aws-eks): fail to update both logging and access at the same time (…

…#22957)

This PR addresses the following known issues:

1. When updating the cluster endpoint access type only with logging predefined yet unchanged, the cluster-resource-handler updates both the logging and access, which is not allowed and throws the SDK error. This PR fixed this and will update access type only, which is allowed.
2. When updating the cluster endpoint public cidr with exactly the same size of cidr, the `setsEqual` function should return correctly.
3. When updating the cluster endpoint public access from one cidr to multiple cidr with logging predefined yet unchanged, the update should return correctly.
4. Updating both access and logging now throws an error from CDK custom resource.

This PR is just a temporary fix that does not implement multiple operations in the cluster-resource-handler custom resource provider(i.e. update both logging and access). 

Fixes: #21439

----

### All Submissions:

* [x] Have you followed the guidelines in our [Contributing guide?](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md)

### Adding new Unconventional Dependencies:

* [ ] This PR adds new unconventional dependencies following the process described [here](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md/#adding-new-unconventional-dependencies)

### New Features

* [ ] Have you added the new feature to an [integration test](https://github.com/aws/aws-cdk/blob/main/INTEGRATION_TESTS.md)?
	* [ ] Did you use `yarn integ` to deploy the infrastructure and generate the snapshot (i.e. `yarn integ` without `--dry-run`)?

*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

@@ -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:
@@ -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));
 }

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

@@ -588,11 +588,16 @@ describe('cluster resource provider', () => {
             logging: undefined,
             resourcesVpcConfig: undefined,
           }));
-
-          const resp = await handler.onEvent();
-          expect(resp).toEqual({ EksUpdateId: mocks.MOCK_UPDATE_STATUS_ID });
-          expect(mocks.actualRequest.updateClusterConfigRequest!).toEqual({
-            name: 'physical-resource-id',
+          let error: any;
+          try {
+            await handler.onEvent();
+          } catch (e) {
+            error = e;
+          }
+          expect(error.message).toEqual('Cannot update logging and access at the same time');
+        });
+        test('both logging and access defined and modify both of them', async () => {
+          const handler = new ClusterResourceHandler(mocks.client, mocks.newRequest('Update', {
             logging: {
               clusterLogging: [
                 {
@@ -601,13 +606,99 @@ describe('cluster resource provider', () => {
                 },
               ],
             },
+            resourcesVpcConfig: {
+              endpointPrivateAccess: true,
+              endpointPublicAccess: false,
+              publicAccessCidrs: ['0.0.0.0/0'],
+            },
+          }, {
+            logging: {
+              clusterLogging: [
+                {
+                  types: ['api', 'audit', 'authenticator', 'controllerManager'],
+                  enabled: true,
+                },
+              ],
+            },
             resourcesVpcConfig: {
               endpointPrivateAccess: true,
               endpointPublicAccess: true,
               publicAccessCidrs: ['0.0.0.0/0'],
             },
-          });
-          expect(mocks.actualRequest.createClusterRequest).toEqual(undefined);
+          }));
+          let error: any;
+          try {
+            await handler.onEvent();
+          } catch (e) {
+            error = e;
+          }
+          expect(error.message).toEqual('Cannot update logging and access at the same time');
+        });
+        test('Given logging enabled and unchanged, updating the only publicAccessCidrs is allowed ', async () => {
+          const handler = new ClusterResourceHandler(mocks.client, mocks.newRequest('Update', {
+            logging: {
+              clusterLogging: [
+                {
+                  types: ['api', 'audit', 'authenticator', 'controllerManager', 'scheduler'],
+                  enabled: true,
+                },
+              ],
+            },
+            resourcesVpcConfig: {
+              endpointPrivateAccess: true,
+              endpointPublicAccess: true,
+              publicAccessCidrs: ['1.2.3.4/32'],
+            },
+          }, {
+            logging: {
+              clusterLogging: [
+                {
+                  types: ['api', 'audit', 'authenticator', 'controllerManager', 'scheduler'],
+                  enabled: true,
+                },
+              ],
+            },
+            resourcesVpcConfig: {
+              endpointPrivateAccess: true,
+              endpointPublicAccess: true,
+              publicAccessCidrs: ['0.0.0.0/0'],
+            },
+          }));
+          const resp = await handler.onEvent();
+          expect(resp).toEqual({ EksUpdateId: 'MockEksUpdateStatusId' });
+        });
+        test('Given logging enabled and unchanged, updating publicAccessCidrs from one to multiple entries is allowed ', async () => {
+          const handler = new ClusterResourceHandler(mocks.client, mocks.newRequest('Update', {
+            logging: {
+              clusterLogging: [
+                {
+                  types: ['api', 'audit', 'authenticator', 'controllerManager', 'scheduler'],
+                  enabled: true,
+                },
+              ],
+            },
+            resourcesVpcConfig: {
+              endpointPrivateAccess: true,
+              endpointPublicAccess: true,
+              publicAccessCidrs: ['2.4.6.0/24', '1.2.3.4/32', '3.3.3.3/32'],
+            },
+          }, {
+            logging: {
+              clusterLogging: [
+                {
+                  types: ['api', 'audit', 'authenticator', 'controllerManager', 'scheduler'],
+                  enabled: true,
+                },
+              ],
+            },
+            resourcesVpcConfig: {
+              endpointPrivateAccess: true,
+              endpointPublicAccess: true,
+              publicAccessCidrs: ['2.4.6.0/24'],
+            },
+          }));
+          const resp = await handler.onEvent();
+          expect(resp).toEqual({ EksUpdateId: 'MockEksUpdateStatusId' });
         });
       });
     });