From ba2dfd10504504d78e55bc242a4747024a504389 Mon Sep 17 00:00:00 2001 From: Xia Zhao <78883180+xazhao@users.noreply.github.com> Date: Tue, 4 Mar 2025 17:35:00 -0800 Subject: [PATCH] fix(eks): cluster deployment issue when the authentication mode is not changing (#33680) ### Reason for this change The issue happens in a very small edge case: 1. create a eks.Cluster like this ``` new eks.Cluster(this, 'Cluster', { version: eks.KubernetesVersion.V1_32, kubectlLayer: new KubectlV32Layer(this, 'KubectlLayer'), }); ``` 2. In EKS console, modify the Auth model from CONFIG_MAP to API_AND_CONFIG_MAP, wait a few minutes until it completes. 3. Again, update from API_AND_CONFIG_MAP to API from console, wait until it completes 4. Now in CDK, add ``` authenticationMode: eks.AuthenticationMode.API, ``` 5. When we re-deploy, CDK would have a validation error: ``` Received response status [FAILED] from custom resource. Message returned: Cannot update from undefined(CONFIG_MAP) to API ``` It is because in local template, the auth mode is `Config_Map` while the actual resource is using `API` mode. In this case, cdk deployment should ignore the update instead of throwing an error. ### Description of changes Move the code order a little bit. Basically check if the updated auth mode is the same as existing mode first then do some validations. ### Description of how you validated changes Existing unit tests/integration tests passed. I removed 2 unit tests which are not applicable because `DescribeCluster` api call will always return auth mode. Manually tested the change in the edge case. ### Checklist - [ ] My code adheres to the [CONTRIBUTING GUIDE](https://github.com/aws/aws-cdk/blob/main/CONTRIBUTING.md) and [DESIGN GUIDELINES](https://github.com/aws/aws-cdk/blob/main/docs/DESIGN_GUIDELINES.md) ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license* --- .../cluster-resource-handler/cluster.ts | 23 +++++++------- .../aws-eks/cluster-resource-handler-mocks.ts | 1 + .../aws-eks/cluster-resource-provider.test.ts | 30 ------------------- 3 files changed, 13 insertions(+), 41 deletions(-) diff --git a/packages/@aws-cdk/custom-resource-handlers/lib/aws-eks/cluster-resource-handler/cluster.ts b/packages/@aws-cdk/custom-resource-handlers/lib/aws-eks/cluster-resource-handler/cluster.ts index e802e4e770e50..7fe12005051dc 100644 --- a/packages/@aws-cdk/custom-resource-handlers/lib/aws-eks/cluster-resource-handler/cluster.ts +++ b/packages/@aws-cdk/custom-resource-handlers/lib/aws-eks/cluster-resource-handler/cluster.ts @@ -218,6 +218,18 @@ export class ClusterResourceHandler extends ResourceHandler { } if (updates.updateAuthMode) { + // update-authmode will fail if we try to update to the same mode, + // so skip in this case. + try { + const cluster = (await this.eks.describeCluster({ name: this.clusterName })).cluster; + if (cluster?.accessConfig?.authenticationMode === this.newProps.accessConfig?.authenticationMode) { + console.log(`cluster already at ${cluster?.accessConfig?.authenticationMode}, skipping authMode update`); + return; + } + } catch (e: any) { + throw e; + } + // the update path must be // `undefined or CONFIG_MAP` -> `API_AND_CONFIG_MAP` -> `API` // and it's one way path. @@ -247,17 +259,6 @@ export class ClusterResourceHandler extends ResourceHandler { this.newProps.accessConfig?.authenticationMode === 'API') { throw new Error('Cannot update from CONFIG_MAP to API'); } - // update-authmode will fail if we try to update to the same mode, - // so skip in this case. - try { - const cluster = (await this.eks.describeCluster({ name: this.clusterName })).cluster; - if (cluster?.accessConfig?.authenticationMode === this.newProps.accessConfig?.authenticationMode) { - console.log(`cluster already at ${cluster?.accessConfig?.authenticationMode}, skipping authMode update`); - return; - } - } catch (e: any) { - throw e; - } config.accessConfig = this.newProps.accessConfig; } diff --git a/packages/@aws-cdk/custom-resource-handlers/test/aws-eks/cluster-resource-handler-mocks.ts b/packages/@aws-cdk/custom-resource-handlers/test/aws-eks/cluster-resource-handler-mocks.ts index 2c76acfb415bf..5c65a471f87a9 100644 --- a/packages/@aws-cdk/custom-resource-handlers/test/aws-eks/cluster-resource-handler-mocks.ts +++ b/packages/@aws-cdk/custom-resource-handlers/test/aws-eks/cluster-resource-handler-mocks.ts @@ -90,6 +90,7 @@ export const client: EksClient = { arn: 'arn:cluster-arn', certificateAuthority: { data: 'certificateAuthority-data' }, endpoint: 'http://endpoint', + accessConfig: { authenticationMode: 'CONFIG_MAP' }, status: simulateResponse.describeClusterResponseMockStatus || 'ACTIVE', }, }; diff --git a/packages/@aws-cdk/custom-resource-handlers/test/aws-eks/cluster-resource-provider.test.ts b/packages/@aws-cdk/custom-resource-handlers/test/aws-eks/cluster-resource-provider.test.ts index a7e49575ee0d4..8b012cec1e03d 100644 --- a/packages/@aws-cdk/custom-resource-handlers/test/aws-eks/cluster-resource-provider.test.ts +++ b/packages/@aws-cdk/custom-resource-handlers/test/aws-eks/cluster-resource-provider.test.ts @@ -590,21 +590,6 @@ describe('cluster resource provider', () => { expect(error.message).toEqual('Cannot fallback authenticationMode from defined to undefined'); }); - test('fails from API_AND_CONFIG_MAP to CONFIG_MAP', async () => { - const handler = new ClusterResourceHandler(mocks.client, mocks.newRequest('Update', { - accessConfig: { authenticationMode: 'CONFIG_MAP' }, - }, { - accessConfig: { authenticationMode: 'API_AND_CONFIG_MAP' }, - })); - let error: any; - try { - await handler.onEvent(); - } catch (e) { - error = e; - } - - expect(error.message).toEqual('Cannot fallback authenticationMode from API_AND_CONFIG_MAP to CONFIG_MAP'); - }); test('fails from API to undefined', async () => { const handler = new ClusterResourceHandler(mocks.client, mocks.newRequest('Update', { accessConfig: { authenticationMode: undefined }, @@ -635,21 +620,6 @@ describe('cluster resource provider', () => { expect(error.message).toEqual('Cannot fallback authenticationMode from API to API_AND_CONFIG_MAP'); }); - test('fails from API to CONFIG_MAP', async () => { - const handler = new ClusterResourceHandler(mocks.client, mocks.newRequest('Update', { - accessConfig: { authenticationMode: 'CONFIG_MAP' }, - }, { - accessConfig: { authenticationMode: 'API' }, - })); - let error: any; - try { - await handler.onEvent(); - } catch (e) { - error = e; - } - - expect(error.message).toEqual('Cannot fallback authenticationMode from API to CONFIG_MAP'); - }); test('fails from undefined to API', async () => { const handler = new ClusterResourceHandler(mocks.client, mocks.newRequest('Update', { accessConfig: { authenticationMode: 'API' },