From 0de9f734115ede205fd10cc4bd27d4b81c8961fd Mon Sep 17 00:00:00 2001 From: mtrspringer Date: Thu, 4 Apr 2024 12:37:16 +0100 Subject: [PATCH 1/4] add support for passing helm chart values to aws-load-balancer-controller --- .../aws-cdk-lib/aws-eks/lib/alb-controller.ts | 22 +++++ .../aws-eks/test/alb-controller.test.ts | 84 +++++++++++++++++++ 2 files changed, 106 insertions(+) diff --git a/packages/aws-cdk-lib/aws-eks/lib/alb-controller.ts b/packages/aws-cdk-lib/aws-eks/lib/alb-controller.ts index 901605af7585f..02496a3c4ce08 100644 --- a/packages/aws-cdk-lib/aws-eks/lib/alb-controller.ts +++ b/packages/aws-cdk-lib/aws-eks/lib/alb-controller.ts @@ -271,6 +271,27 @@ export interface AlbControllerOptions { * @default - Corresponds to the predefined version. */ readonly policy?: any; + + /** + * Override values to be used by the chart. + * For nested values use a nested dictionary. For example: + * values: { + * autoscaling: false, + * ingressClassParams: { create: true } + * } + * + * Note that the following values are set by the controller and cannot be overridden: + * - clusterName + * - serviceAccount.create + * - serviceAccount.name + * - region + * - vpcId + * - image.repository + * - image.tag + * + * @default - No values are provided to the chart. + */ + readonly values?: {[key: string]: any}; } /** @@ -342,6 +363,7 @@ export class AlbController extends Construct { wait: true, timeout: Duration.minutes(15), values: { + ...(props.values ?? {}), clusterName: props.cluster.clusterName, serviceAccount: { create: false, diff --git a/packages/aws-cdk-lib/aws-eks/test/alb-controller.test.ts b/packages/aws-cdk-lib/aws-eks/test/alb-controller.test.ts index 170a0cf1d1783..332dd081295d3 100644 --- a/packages/aws-cdk-lib/aws-eks/test/alb-controller.test.ts +++ b/packages/aws-cdk-lib/aws-eks/test/alb-controller.test.ts @@ -153,6 +153,90 @@ test('correct helm chart version is set for selected alb controller version', () }); }); +test('can pass values to the aws-load-balancer-controller helm chart', () => { + const { stack } = testFixture(); + + const cluster = new Cluster(stack, 'Cluster', { + version: KubernetesVersion.V1_27, + }); + + AlbController.create(stack, { + cluster, + version: AlbControllerVersion.V2_6_2, + repository: 'custom', + values: { + enableWafv2: false, + }, + }); + + Template.fromStack(stack).hasResourceProperties(HelmChart.RESOURCE_TYPE, { + Version: '1.6.2', // The helm chart version associated with AlbControllerVersion.V2_6_2 + Values: { + 'Fn::Join': [ + '', + [ + '{"enableWafv2":false,"clusterName":"', + { + Ref: 'Cluster9EE0221C', + }, + '","serviceAccount":{"create":false,"name":"aws-load-balancer-controller"},"region":"us-east-1","vpcId":"', + { + Ref: 'ClusterDefaultVpcFA9F2722', + }, + '","image":{"repository":"custom","tag":"v2.6.2"}}', + ], + ], + }, + }); +}); + +test('values passed to the aws-load-balancer-controller do not override values set by construct', () => { + const { stack } = testFixture(); + + const cluster = new Cluster(stack, 'Cluster', { + version: KubernetesVersion.V1_27, + }); + + AlbController.create(stack, { + cluster, + version: AlbControllerVersion.V2_6_2, + repository: 'custom', + values: { + clusterName: 'test-cluster', + serviceAccount: { + create: true, + name: 'test-service-account', + }, + region: 'test-region', + vpcId: 'test-vpc-id', + image: { + repository: 'test-repository', + tag: 'test-tag', + }, + }, + }); + + Template.fromStack(stack).hasResourceProperties(HelmChart.RESOURCE_TYPE, { + Version: '1.6.2', // The helm chart version associated with AlbControllerVersion.V2_6_2 + Values: { + 'Fn::Join': [ + '', + [ + '{"clusterName":"', + { + Ref: 'Cluster9EE0221C', + }, + '","serviceAccount":{"create":false,"name":"aws-load-balancer-controller"},"region":"us-east-1","vpcId":"', + { + Ref: 'ClusterDefaultVpcFA9F2722', + }, + '","image":{"repository":"custom","tag":"v2.6.2"}}', + ], + ], + }, + }); +}); + describe('AlbController AwsAuth creation', () => { const setupTest = (authenticationMode?: AuthenticationMode) => { const { stack } = testFixture(); From 4bb31e2ae6dee5f3e5d9b0fb77fbfc2402562b93 Mon Sep 17 00:00:00 2001 From: mtrspringer Date: Thu, 4 Apr 2024 14:11:50 +0000 Subject: [PATCH 2/4] add readme, integration test --- .../test/aws-eks/test/integ.alb-controller.ts | 15 +++++++++++---- packages/aws-cdk-lib/aws-eks/README.md | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-eks/test/integ.alb-controller.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-eks/test/integ.alb-controller.ts index 99fba6d52104d..d94336843278d 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-eks/test/integ.alb-controller.ts +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-eks/test/integ.alb-controller.ts @@ -1,6 +1,6 @@ /// !cdk-integ pragma:disable-update-workflow import * as ec2 from 'aws-cdk-lib/aws-ec2'; -import { App, CfnOutput, Duration, Stack } from 'aws-cdk-lib'; +import { App, CfnOutput, Duration, Stack, StackProps } from 'aws-cdk-lib'; import * as integ from '@aws-cdk/integ-tests-alpha'; import * as cdk8s from 'cdk8s'; import * as kplus from 'cdk8s-plus-27'; @@ -10,10 +10,15 @@ import * as eks from 'aws-cdk-lib/aws-eks'; import { IAM_OIDC_REJECT_UNAUTHORIZED_CONNECTIONS } from 'aws-cdk-lib/cx-api'; const LATEST_VERSION: eks.AlbControllerVersion = eks.AlbControllerVersion.V2_8_2; + +interface EksClusterAlbControllerStackProps extends StackProps { + albControllerValues?: {[key:string]: any}; +} + class EksClusterAlbControllerStack extends Stack { - constructor(scope: App, id: string) { - super(scope, id); + constructor(scope: App, id: string, props: EksClusterAlbControllerStackProps = {}) { + super(scope, id, props); // just need one nat gateway to simplify the test const vpc = new ec2.Vpc(this, 'Vpc', { maxAzs: 2, natGateways: 1, restrictDefaultSecurityGroup: false }); @@ -23,6 +28,7 @@ class EksClusterAlbControllerStack extends Stack { ...getClusterVersionConfig(this, eks.KubernetesVersion.V1_30), albController: { version: LATEST_VERSION, + values: props.albControllerValues, }, }); @@ -76,8 +82,9 @@ const app = new App({ }, }); const stack = new EksClusterAlbControllerStack(app, 'aws-cdk-eks-cluster-alb-controller'); +const stackWithAlbControllerValues = new EksClusterAlbControllerStack(app, 'aws-cdk-eks-cluster-alb-controller', { albControllerValues: { enableWafv2: false } }); new integ.IntegTest(app, 'aws-cdk-cluster-alb-controller-integ', { - testCases: [stack], + testCases: [stack, stackWithAlbControllerValues], // Test includes assets that are updated weekly. If not disabled, the upgrade PR will fail. diffAssets: false, }); diff --git a/packages/aws-cdk-lib/aws-eks/README.md b/packages/aws-cdk-lib/aws-eks/README.md index a32f94c651bb4..8761726ecfbf2 100644 --- a/packages/aws-cdk-lib/aws-eks/README.md +++ b/packages/aws-cdk-lib/aws-eks/README.md @@ -691,6 +691,25 @@ if (cluster.albController) { } ``` +If you need to configure the underlying ALB Controller, you can pass optional values that will be forwarded to the underling HelmChart construct. +For example, if your organization uses AWS Firewall Manager you will need to disable aws-load-balancer-controller's waf modification behavior or it will disassociate WAFs that are not specified in the Ingress' annotations. + +Note: supported value options may vary depending on the version of the aws-load-balancer-controller helm chart you are using. You should consult the [ArtifactHub documentation](https://artifacthub.io/packages/helm/aws/aws-load-balancer-controller) for your version to ensure you are configuring the chart correctly. + +```ts +new eks.Cluster(this, 'HelloEKS', { + version: eks.KubernetesVersion.V1_29, + albController: { + version: eks.AlbControllerVersion.V2_6_2, + values: { + enableWaf: false, + enableWafv2: false, + }, + }, +}); +``` + + ### VPC Support You can specify the VPC of the cluster using the `vpc` and `vpcSubnets` properties: From fa8c3f4fcafc101eaa4cbde44afac26fb9770025 Mon Sep 17 00:00:00 2001 From: mtrspringer Date: Fri, 5 Apr 2024 14:33:35 +0000 Subject: [PATCH 3/4] fix test stack name --- .../framework-integ/test/aws-eks/test/integ.alb-controller.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-eks/test/integ.alb-controller.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-eks/test/integ.alb-controller.ts index d94336843278d..90ed822fc9573 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-eks/test/integ.alb-controller.ts +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-eks/test/integ.alb-controller.ts @@ -82,7 +82,7 @@ const app = new App({ }, }); const stack = new EksClusterAlbControllerStack(app, 'aws-cdk-eks-cluster-alb-controller'); -const stackWithAlbControllerValues = new EksClusterAlbControllerStack(app, 'aws-cdk-eks-cluster-alb-controller', { albControllerValues: { enableWafv2: false } }); +const stackWithAlbControllerValues = new EksClusterAlbControllerStack(app, 'aws-cdk-eks-cluster-alb-controller-values', { albControllerValues: { enableWafv2: false } }); new integ.IntegTest(app, 'aws-cdk-cluster-alb-controller-integ', { testCases: [stack, stackWithAlbControllerValues], // Test includes assets that are updated weekly. If not disabled, the upgrade PR will fail. From 33b54ae450a74f7568c82589a181bbdbd266eda4 Mon Sep 17 00:00:00 2001 From: mtrspringer Date: Mon, 13 May 2024 17:43:52 +0100 Subject: [PATCH 4/4] changes from code review --- .../test/aws-eks/test/integ.alb-controller.ts | 6 ++-- packages/aws-cdk-lib/aws-eks/README.md | 4 +-- .../aws-cdk-lib/aws-eks/lib/alb-controller.ts | 27 +++++++++++++++-- .../aws-eks/test/alb-controller.test.ts | 30 ++++--------------- 4 files changed, 34 insertions(+), 33 deletions(-) diff --git a/packages/@aws-cdk-testing/framework-integ/test/aws-eks/test/integ.alb-controller.ts b/packages/@aws-cdk-testing/framework-integ/test/aws-eks/test/integ.alb-controller.ts index 90ed822fc9573..a601a474a29b8 100644 --- a/packages/@aws-cdk-testing/framework-integ/test/aws-eks/test/integ.alb-controller.ts +++ b/packages/@aws-cdk-testing/framework-integ/test/aws-eks/test/integ.alb-controller.ts @@ -12,7 +12,7 @@ import { IAM_OIDC_REJECT_UNAUTHORIZED_CONNECTIONS } from 'aws-cdk-lib/cx-api'; const LATEST_VERSION: eks.AlbControllerVersion = eks.AlbControllerVersion.V2_8_2; interface EksClusterAlbControllerStackProps extends StackProps { - albControllerValues?: {[key:string]: any}; + albControllerHelmChartValues?: {[key:string]: any}; } class EksClusterAlbControllerStack extends Stack { @@ -28,7 +28,7 @@ class EksClusterAlbControllerStack extends Stack { ...getClusterVersionConfig(this, eks.KubernetesVersion.V1_30), albController: { version: LATEST_VERSION, - values: props.albControllerValues, + helmChartValues: props.albControllerHelmChartValues, }, }); @@ -82,7 +82,7 @@ const app = new App({ }, }); const stack = new EksClusterAlbControllerStack(app, 'aws-cdk-eks-cluster-alb-controller'); -const stackWithAlbControllerValues = new EksClusterAlbControllerStack(app, 'aws-cdk-eks-cluster-alb-controller-values', { albControllerValues: { enableWafv2: false } }); +const stackWithAlbControllerValues = new EksClusterAlbControllerStack(app, 'aws-cdk-eks-cluster-alb-controller-values', { albControllerHelmChartValues: { enableWafv2: false } }); new integ.IntegTest(app, 'aws-cdk-cluster-alb-controller-integ', { testCases: [stack, stackWithAlbControllerValues], // Test includes assets that are updated weekly. If not disabled, the upgrade PR will fail. diff --git a/packages/aws-cdk-lib/aws-eks/README.md b/packages/aws-cdk-lib/aws-eks/README.md index 8761726ecfbf2..4a9b610f5c880 100644 --- a/packages/aws-cdk-lib/aws-eks/README.md +++ b/packages/aws-cdk-lib/aws-eks/README.md @@ -692,7 +692,7 @@ if (cluster.albController) { ``` If you need to configure the underlying ALB Controller, you can pass optional values that will be forwarded to the underling HelmChart construct. -For example, if your organization uses AWS Firewall Manager you will need to disable aws-load-balancer-controller's waf modification behavior or it will disassociate WAFs that are not specified in the Ingress' annotations. +For example, if your organization uses AWS Firewall Manager you will need to disable aws-load-balancer-controller's waf modification behavior or it will periodically disassociate WAFs that are not specified in the Ingress' annotations. Note: supported value options may vary depending on the version of the aws-load-balancer-controller helm chart you are using. You should consult the [ArtifactHub documentation](https://artifacthub.io/packages/helm/aws/aws-load-balancer-controller) for your version to ensure you are configuring the chart correctly. @@ -701,7 +701,7 @@ new eks.Cluster(this, 'HelloEKS', { version: eks.KubernetesVersion.V1_29, albController: { version: eks.AlbControllerVersion.V2_6_2, - values: { + helmChartValues: { enableWaf: false, enableWafv2: false, }, diff --git a/packages/aws-cdk-lib/aws-eks/lib/alb-controller.ts b/packages/aws-cdk-lib/aws-eks/lib/alb-controller.ts index 02496a3c4ce08..bde180cf3657b 100644 --- a/packages/aws-cdk-lib/aws-eks/lib/alb-controller.ts +++ b/packages/aws-cdk-lib/aws-eks/lib/alb-controller.ts @@ -275,7 +275,7 @@ export interface AlbControllerOptions { /** * Override values to be used by the chart. * For nested values use a nested dictionary. For example: - * values: { + * helmChartValues: { * autoscaling: false, * ingressClassParams: { create: true } * } @@ -291,7 +291,7 @@ export interface AlbControllerOptions { * * @default - No values are provided to the chart. */ - readonly values?: {[key: string]: any}; + readonly helmChartValues?: {[key: string]: any}; } /** @@ -352,6 +352,8 @@ export class AlbController extends Construct { } // https://kubernetes-sigs.github.io/aws-load-balancer-controller/v2.2/deploy/installation/#add-controller-to-cluster + const { helmChartValues = {} } = props; + this.validateHelmChartValues(helmChartValues); const chart = new HelmChart(this, 'Resource', { cluster: props.cluster, chart: 'aws-load-balancer-controller', @@ -363,7 +365,8 @@ export class AlbController extends Construct { wait: true, timeout: Duration.minutes(15), values: { - ...(props.values ?? {}), + ...helmChartValues, + // if you modify these values, you must also update this.restrictedHelmChartValueKeys clusterName: props.cluster.clusterName, serviceAccount: { create: false, @@ -402,4 +405,22 @@ export class AlbController extends Construct { } return resources.map(rewriteResource); } + + private validateHelmChartValues(values: {[key: string]: any}) { + const valuesKeySet = new Set(Object.keys(values)); + const invalidKeys = new Set([...valuesKeySet].filter((key) => this.restrictedHelmChartValueKeys.has(key))); + if (invalidKeys.size > 0) { + throw new Error(`The following aws-load-balancer-controller HelmChart value keys are restricted and cannot be overridden: ${Array.from(this.restrictedHelmChartValueKeys).join(', ')}. (Invalid keys: ${Array.from(invalidKeys).join(', ')})`); + } + } + + private get restrictedHelmChartValueKeys(): Set { + return new Set([ + 'clusterName', + 'serviceAccount', + 'region', + 'vpcId', + 'image', + ]); + } } diff --git a/packages/aws-cdk-lib/aws-eks/test/alb-controller.test.ts b/packages/aws-cdk-lib/aws-eks/test/alb-controller.test.ts index 332dd081295d3..e962e77ac36d0 100644 --- a/packages/aws-cdk-lib/aws-eks/test/alb-controller.test.ts +++ b/packages/aws-cdk-lib/aws-eks/test/alb-controller.test.ts @@ -164,7 +164,7 @@ test('can pass values to the aws-load-balancer-controller helm chart', () => { cluster, version: AlbControllerVersion.V2_6_2, repository: 'custom', - values: { + helmChartValues: { enableWafv2: false, }, }); @@ -190,18 +190,18 @@ test('can pass values to the aws-load-balancer-controller helm chart', () => { }); }); -test('values passed to the aws-load-balancer-controller do not override values set by construct', () => { +test('values passed to the aws-load-balancer-controller result in an error if they conflict with restricted values set by the construct', () => { const { stack } = testFixture(); const cluster = new Cluster(stack, 'Cluster', { version: KubernetesVersion.V1_27, }); - AlbController.create(stack, { + expect(() => AlbController.create(stack, { cluster, version: AlbControllerVersion.V2_6_2, repository: 'custom', - values: { + helmChartValues: { clusterName: 'test-cluster', serviceAccount: { create: true, @@ -214,27 +214,7 @@ test('values passed to the aws-load-balancer-controller do not override values s tag: 'test-tag', }, }, - }); - - Template.fromStack(stack).hasResourceProperties(HelmChart.RESOURCE_TYPE, { - Version: '1.6.2', // The helm chart version associated with AlbControllerVersion.V2_6_2 - Values: { - 'Fn::Join': [ - '', - [ - '{"clusterName":"', - { - Ref: 'Cluster9EE0221C', - }, - '","serviceAccount":{"create":false,"name":"aws-load-balancer-controller"},"region":"us-east-1","vpcId":"', - { - Ref: 'ClusterDefaultVpcFA9F2722', - }, - '","image":{"repository":"custom","tag":"v2.6.2"}}', - ], - ], - }, - }); + })).toThrow(/The following aws-load-balancer-controller HelmChart value keys are restricted and cannot be overridden: .*/); }); describe('AlbController AwsAuth creation', () => {