Skip to content

Commit fbcb732

Browse files
authored
fix(msk): allow both sasl/scram and iam auth (#31743)
Pointed out [here](#14647 (comment)) and verified in the Console, both `SASL/SCRAM` and `IAM` can be enabled together. Closes #32779 It's a little confusing because [CloudFormation](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-properties-msk-cluster-sasl.html) groups `Iam` and `Scram` together under `Sasl`, but the Console separates the two and allows both at the same time. I'd like to refactor this further but this change unblocks the issue where `SASL/SCRAM` and `IAM` cannot be enabled together. ![image](https://github.com/user-attachments/assets/60dd662e-4db9-4bbb-bcc0-67bd059f6870)
1 parent cf9d9e2 commit fbcb732

13 files changed

+2449
-90
lines changed

packages/@aws-cdk/aws-msk-alpha/README.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ const cluster = new msk.Cluster(this, 'cluster', {
124124
});
125125
```
126126

127-
### SASL/IAM
127+
### IAM
128128

129129
Enable client authentication with [IAM](https://docs.aws.amazon.com/msk/latest/developerguide/iam-access-control.html):
130130

packages/@aws-cdk/aws-msk-alpha/lib/cluster.ts

Lines changed: 18 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -444,11 +444,7 @@ export class Cluster extends ClusterBase {
444444
/**
445445
* Reference an existing cluster, defined outside of the CDK code, by name.
446446
*/
447-
public static fromClusterArn(
448-
scope: constructs.Construct,
449-
id: string,
450-
clusterArn: string,
451-
): ICluster {
447+
public static fromClusterArn(scope: constructs.Construct, id: string, clusterArn: string): ICluster {
452448
class Import extends ClusterBase {
453449
public readonly clusterArn = clusterArn;
454450
public readonly clusterName = core.Fn.select(1, core.Fn.split('/', clusterArn)); // ['arn:partition:kafka:region:account-id', clusterName, clusterId]
@@ -465,9 +461,7 @@ export class Cluster extends ClusterBase {
465461
private _clusterBootstrapBrokers?: cr.AwsCustomResource;
466462

467463
constructor(scope: constructs.Construct, id: string, props: ClusterProps) {
468-
super(scope, id, {
469-
physicalName: props.clusterName,
470-
});
464+
super(scope, id, { physicalName: props.clusterName });
471465
// Enhanced CDK Analytics Telemetry
472466
addConstructMetadata(this, props);
473467

@@ -483,26 +477,11 @@ export class Cluster extends ClusterBase {
483477
});
484478

485479
if (subnetSelection.subnets.length < 2) {
486-
throw Error(
487-
`Cluster requires at least 2 subnets, got ${subnetSelection.subnets.length}`,
488-
);
480+
throw Error(`Cluster requires at least 2 subnets, got ${subnetSelection.subnets.length}`);
489481
}
490482

491-
if (
492-
props.clientAuthentication?.saslProps?.iam &&
493-
props.clientAuthentication?.saslProps?.scram
494-
) {
495-
throw Error('Only one client authentication method can be enabled.');
496-
}
497-
498-
if (
499-
props.encryptionInTransit?.clientBroker ===
500-
ClientBrokerEncryption.PLAINTEXT &&
501-
props.clientAuthentication
502-
) {
503-
throw Error(
504-
'To enable client authentication, you must enabled TLS-encrypted traffic between clients and brokers.',
505-
);
483+
if (props.encryptionInTransit?.clientBroker === ClientBrokerEncryption.PLAINTEXT && props.clientAuthentication) {
484+
throw Error('To enable client authentication, you must enabled TLS-encrypted traffic between clients and brokers.');
506485
} else if (
507486
props.encryptionInTransit?.clientBroker ===
508487
ClientBrokerEncryption.TLS_PLAINTEXT &&
@@ -514,13 +493,10 @@ export class Cluster extends ClusterBase {
514493
);
515494
}
516495

517-
const volumeSize =
518-
props.ebsStorageInfo?.volumeSize ?? 1000;
496+
const volumeSize = props.ebsStorageInfo?.volumeSize ?? 1000;
519497
// Minimum: 1 GiB, maximum: 16384 GiB
520498
if (volumeSize < 1 || volumeSize > 16384) {
521-
throw Error(
522-
'EBS volume size should be in the range 1-16384',
523-
);
499+
throw Error('EBS volume size should be in the range 1-16384');
524500
}
525501

526502
const instanceType = props.instanceType
@@ -642,13 +618,9 @@ export class Cluster extends ClusterBase {
642618
},
643619
};
644620

645-
if (
646-
props.clientAuthentication?.saslProps?.scram &&
647-
props.clientAuthentication?.saslProps?.key === undefined
648-
) {
621+
if (props.clientAuthentication?.saslProps?.scram && props.clientAuthentication?.saslProps?.key === undefined) {
649622
this.saslScramAuthenticationKey = new kms.Key(this, 'SASLKey', {
650-
description:
651-
'Used for encrypting MSK secrets for SASL/SCRAM authentication.',
623+
description: 'Used for encrypting MSK secrets for SASL/SCRAM authentication.',
652624
alias: `msk/${props.clusterName}/sasl/scram`,
653625
});
654626

@@ -678,37 +650,16 @@ export class Cluster extends ClusterBase {
678650
}
679651

680652
let clientAuthentication: CfnCluster.ClientAuthenticationProperty | undefined;
681-
if (props.clientAuthentication?.saslProps?.iam) {
653+
if (props.clientAuthentication) {
654+
const { saslProps, tlsProps } = props.clientAuthentication;
682655
clientAuthentication = {
683-
sasl: { iam: { enabled: props.clientAuthentication.saslProps.iam } },
684-
};
685-
if (props.clientAuthentication?.tlsProps) {
686-
clientAuthentication = {
687-
sasl: { iam: { enabled: props.clientAuthentication.saslProps.iam } },
688-
tls: {
689-
certificateAuthorityArnList: props.clientAuthentication?.tlsProps?.certificateAuthorities?.map(
690-
(ca) => ca.certificateAuthorityArn,
691-
),
692-
},
693-
};
694-
}
695-
} else if (props.clientAuthentication?.saslProps?.scram) {
696-
clientAuthentication = {
697-
sasl: {
698-
scram: {
699-
enabled: props.clientAuthentication.saslProps.scram,
700-
},
701-
},
702-
};
703-
} else if (
704-
props.clientAuthentication?.tlsProps?.certificateAuthorities !== undefined
705-
) {
706-
clientAuthentication = {
707-
tls: {
708-
certificateAuthorityArnList: props.clientAuthentication?.tlsProps?.certificateAuthorities.map(
709-
(ca) => ca.certificateAuthorityArn,
710-
),
711-
},
656+
sasl: saslProps ? {
657+
iam: saslProps.iam ? { enabled: true }: undefined,
658+
scram: saslProps.scram ? { enabled: true }: undefined,
659+
} : undefined,
660+
tls: tlsProps?.certificateAuthorities ? {
661+
certificateAuthorityArnList: tlsProps.certificateAuthorities?.map((ca) => ca.certificateAuthorityArn),
662+
} : undefined,
712663
};
713664
}
714665

packages/@aws-cdk/aws-msk-alpha/test/__snapshots__/cluster.test.ts.snap

Lines changed: 87 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -493,7 +493,7 @@ exports[`MSK Cluster Snapshot test with all values set 1`] = `
493493
}
494494
`;
495495

496-
exports[`MSK Cluster created with authentication enabled with sasl/iam auth and tls Snapshot test with all values set (iam/sasl) 1`] = `
496+
exports[`MSK Cluster created with authentication enabled with combinations of sasl/scram, iam, and tls Snapshot test with all values set (iam/scram/tls) 1`] = `
497497
{
498498
"Resources": {
499499
"Vpc8378EB38": {
@@ -915,6 +915,9 @@ exports[`MSK Cluster created with authentication enabled with sasl/iam auth and
915915
"Iam": {
916916
"Enabled": true,
917917
},
918+
"Scram": {
919+
"Enabled": true,
920+
},
918921
},
919922
"Tls": {
920923
"CertificateAuthorityArnList": [
@@ -965,6 +968,89 @@ exports[`MSK Cluster created with authentication enabled with sasl/iam auth and
965968
"Type": "AWS::MSK::Cluster",
966969
"UpdateReplacePolicy": "Retain",
967970
},
971+
"kafkaSASLKey69FC3AFA": {
972+
"DeletionPolicy": "Retain",
973+
"Properties": {
974+
"Description": "Used for encrypting MSK secrets for SASL/SCRAM authentication.",
975+
"KeyPolicy": {
976+
"Statement": [
977+
{
978+
"Action": "kms:*",
979+
"Effect": "Allow",
980+
"Principal": {
981+
"AWS": {
982+
"Fn::Join": [
983+
"",
984+
[
985+
"arn:",
986+
{
987+
"Ref": "AWS::Partition",
988+
},
989+
":iam::",
990+
{
991+
"Ref": "AWS::AccountId",
992+
},
993+
":root",
994+
],
995+
],
996+
},
997+
},
998+
"Resource": "*",
999+
},
1000+
{
1001+
"Action": [
1002+
"kms:Encrypt",
1003+
"kms:Decrypt",
1004+
"kms:ReEncrypt*",
1005+
"kms:GenerateDataKey*",
1006+
"kms:CreateGrant",
1007+
"kms:DescribeKey",
1008+
],
1009+
"Condition": {
1010+
"StringEquals": {
1011+
"kms:CallerAccount": {
1012+
"Ref": "AWS::AccountId",
1013+
},
1014+
"kms:ViaService": {
1015+
"Fn::Join": [
1016+
"",
1017+
[
1018+
"secretsmanager.",
1019+
{
1020+
"Ref": "AWS::Region",
1021+
},
1022+
".amazonaws.com",
1023+
],
1024+
],
1025+
},
1026+
},
1027+
},
1028+
"Effect": "Allow",
1029+
"Principal": {
1030+
"AWS": "*",
1031+
},
1032+
"Resource": "*",
1033+
"Sid": "Allow access through AWS Secrets Manager for all principals in the account that are authorized to use AWS Secrets Manager",
1034+
},
1035+
],
1036+
"Version": "2012-10-17",
1037+
},
1038+
},
1039+
"Type": "AWS::KMS::Key",
1040+
"UpdateReplacePolicy": "Retain",
1041+
},
1042+
"kafkaSASLKeyAlias7A73E101": {
1043+
"Properties": {
1044+
"AliasName": "alias/msk/test-cluster/sasl/scram",
1045+
"TargetKeyId": {
1046+
"Fn::GetAtt": [
1047+
"kafkaSASLKey69FC3AFA",
1048+
"Arn",
1049+
],
1050+
},
1051+
},
1052+
"Type": "AWS::KMS::Alias",
1053+
},
9681054
"sg1fromsg32181E6F4C07E": {
9691055
"Properties": {
9701056
"Description": "from sg3:2181",

0 commit comments

Comments
 (0)