From 06b733ff2cb6db00747dbcddbfa59afd77f8ca8d Mon Sep 17 00:00:00 2001 From: Michael Sambol Date: Sun, 13 Oct 2024 22:07:47 -0700 Subject: [PATCH] fix(msk): allow both scram and iam auth --- .../@aws-cdk/aws-msk-alpha/lib/cluster.ts | 98 +++++-------------- .../test/__snapshots__/cluster.test.ts.snap | 2 + .../aws-msk-alpha/test/cluster.test.ts | 18 ---- 3 files changed, 24 insertions(+), 94 deletions(-) diff --git a/packages/@aws-cdk/aws-msk-alpha/lib/cluster.ts b/packages/@aws-cdk/aws-msk-alpha/lib/cluster.ts index ed8041e6751a2..0f0e3ef13207d 100644 --- a/packages/@aws-cdk/aws-msk-alpha/lib/cluster.ts +++ b/packages/@aws-cdk/aws-msk-alpha/lib/cluster.ts @@ -443,11 +443,7 @@ export class Cluster extends ClusterBase { /** * Reference an existing cluster, defined outside of the CDK code, by name. */ - public static fromClusterArn( - scope: constructs.Construct, - id: string, - clusterArn: string, - ): ICluster { + public static fromClusterArn(scope: constructs.Construct, id: string, clusterArn: string): ICluster { class Import extends ClusterBase { public readonly clusterArn = clusterArn; public readonly clusterName = core.Fn.select(1, core.Fn.split('/', clusterArn)); // ['arn:partition:kafka:region:account-id', clusterName, clusterId] @@ -464,9 +460,7 @@ export class Cluster extends ClusterBase { private _clusterBootstrapBrokers?: cr.AwsCustomResource; constructor(scope: constructs.Construct, id: string, props: ClusterProps) { - super(scope, id, { - physicalName: props.clusterName, - }); + super(scope, id, { physicalName: props.clusterName }); const subnetSelection = props.vpc.selectSubnets(props.vpcSubnets); @@ -480,37 +474,18 @@ export class Cluster extends ClusterBase { }); if (subnetSelection.subnets.length < 2) { - throw Error( - `Cluster requires at least 2 subnets, got ${subnetSelection.subnets.length}`, - ); + throw Error(`Cluster requires at least 2 subnets, got ${subnetSelection.subnets.length}`); } - if ( - !core.Token.isUnresolved(props.clusterName) && - !/^[a-zA-Z0-9]+$/.test(props.clusterName) && - props.clusterName.length > 64 - ) { + if (!core.Token.isUnresolved(props.clusterName) && !/^[a-zA-Z0-9]+$/.test(props.clusterName) && props.clusterName.length > 64) { throw Error( 'The cluster name must only contain alphanumeric characters and have a maximum length of 64 characters.' + `got: '${props.clusterName}. length: ${props.clusterName.length}'`, ); } - if ( - props.clientAuthentication?.saslProps?.iam && - props.clientAuthentication?.saslProps?.scram - ) { - throw Error('Only one client authentication method can be enabled.'); - } - - if ( - props.encryptionInTransit?.clientBroker === - ClientBrokerEncryption.PLAINTEXT && - props.clientAuthentication - ) { - throw Error( - 'To enable client authentication, you must enabled TLS-encrypted traffic between clients and brokers.', - ); + if (props.encryptionInTransit?.clientBroker === ClientBrokerEncryption.PLAINTEXT && props.clientAuthentication) { + throw Error('To enable client authentication, you must enabled TLS-encrypted traffic between clients and brokers.'); } else if ( props.encryptionInTransit?.clientBroker === ClientBrokerEncryption.TLS_PLAINTEXT && @@ -522,13 +497,10 @@ export class Cluster extends ClusterBase { ); } - const volumeSize = - props.ebsStorageInfo?.volumeSize ?? 1000; + const volumeSize = props.ebsStorageInfo?.volumeSize ?? 1000; // Minimum: 1 GiB, maximum: 16384 GiB if (volumeSize < 1 || volumeSize > 16384) { - throw Error( - 'EBS volume size should be in the range 1-16384', - ); + throw Error('EBS volume size should be in the range 1-16384'); } const instanceType = props.instanceType @@ -650,13 +622,9 @@ export class Cluster extends ClusterBase { }, }; - if ( - props.clientAuthentication?.saslProps?.scram && - props.clientAuthentication?.saslProps?.key === undefined - ) { + if (props.clientAuthentication?.saslProps?.scram && props.clientAuthentication?.saslProps?.key === undefined) { this.saslScramAuthenticationKey = new kms.Key(this, 'SASLKey', { - description: - 'Used for encrypting MSK secrets for SASL/SCRAM authentication.', + description: 'Used for encrypting MSK secrets for SASL/SCRAM authentication.', alias: `msk/${props.clusterName}/sasl/scram`, }); @@ -685,40 +653,18 @@ export class Cluster extends ClusterBase { ); } - let clientAuthentication; - if (props.clientAuthentication?.saslProps?.iam) { - clientAuthentication = { - sasl: { iam: { enabled: props.clientAuthentication.saslProps.iam } }, - }; - if (props.clientAuthentication?.tlsProps) { - clientAuthentication = { - sasl: { iam: { enabled: props.clientAuthentication.saslProps.iam } }, - tls: { - certificateAuthorityArnList: props.clientAuthentication?.tlsProps?.certificateAuthorities?.map( - (ca) => ca.certificateAuthorityArn, - ), - }, - }; - } - } else if (props.clientAuthentication?.saslProps?.scram) { - clientAuthentication = { - sasl: { - scram: { - enabled: props.clientAuthentication.saslProps.scram, - }, - }, - }; - } else if ( - props.clientAuthentication?.tlsProps?.certificateAuthorities !== undefined - ) { - clientAuthentication = { - tls: { - certificateAuthorityArnList: props.clientAuthentication?.tlsProps?.certificateAuthorities.map( - (ca) => ca.certificateAuthorityArn, - ), - }, - }; - } + const clientAuthentication = { + sasl: props.clientAuthentication?.saslProps ? { + iam: props.clientAuthentication.saslProps.iam ? { enabled: true }: undefined, + scram: props.clientAuthentication.saslProps.scram ? { enabled: true }: undefined, + } : undefined, + tls: props.clientAuthentication?.tlsProps?.certificateAuthorities ? { + certificateAuthorityArnList: props.clientAuthentication.tlsProps.certificateAuthorities?.map( + (ca) => ca.certificateAuthorityArn, + ), + enabled: true, + } : undefined, + }; const resource = new CfnCluster(this, 'Resource', { clusterName: props.clusterName, diff --git a/packages/@aws-cdk/aws-msk-alpha/test/__snapshots__/cluster.test.ts.snap b/packages/@aws-cdk/aws-msk-alpha/test/__snapshots__/cluster.test.ts.snap index 8c5770b626607..a20f6d5c2ca19 100644 --- a/packages/@aws-cdk/aws-msk-alpha/test/__snapshots__/cluster.test.ts.snap +++ b/packages/@aws-cdk/aws-msk-alpha/test/__snapshots__/cluster.test.ts.snap @@ -422,6 +422,7 @@ exports[`MSK Cluster Snapshot test with all values set 1`] = ` "CertificateAuthorityArnList": [ "arn:aws:acm-pca:us-west-2:1234567890:certificate-authority/11111111-1111-1111-1111-111111111111", ], + "Enabled": true, }, }, "ClusterName": "test-cluster", @@ -920,6 +921,7 @@ exports[`MSK Cluster created with authentication enabled with sasl/iam auth and "CertificateAuthorityArnList": [ "arn:aws:acm-pca:us-west-2:1234567890:certificate-authority/11111111-1111-1111-1111-111111111111", ], + "Enabled": true, }, }, "ClusterName": "test-cluster", diff --git a/packages/@aws-cdk/aws-msk-alpha/test/cluster.test.ts b/packages/@aws-cdk/aws-msk-alpha/test/cluster.test.ts index 7c4bd6a10f4ec..bf0410d6b7b46 100644 --- a/packages/@aws-cdk/aws-msk-alpha/test/cluster.test.ts +++ b/packages/@aws-cdk/aws-msk-alpha/test/cluster.test.ts @@ -372,24 +372,6 @@ describe('MSK Cluster', () => { }); }); }); - - test('fails if more than one authentication method is enabled', () => { - expect( - () => - new msk.Cluster(stack, 'Cluster', { - clusterName: 'cluster', - kafkaVersion: msk.KafkaVersion.V2_6_1, - vpc, - encryptionInTransit: { - clientBroker: msk.ClientBrokerEncryption.TLS, - }, - clientAuthentication: msk.ClientAuthentication.sasl({ - iam: true, - scram: true, - }), - }), - ).toThrow('Only one client authentication method can be enabled.'); - }); }); describe('created with an instance type set', () => {