fix(msk): allow both scram and iam auth

aws · Oct 14, 2024 · 06b733f · 06b733f
1 parent f4f8abc
commit 06b733f
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 94 deletions.
diff --git 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
@@ -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
@@ -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', () => {