aws / aws-cdk

The AWS Cloud Development Kit is a framework for defining cloud infrastructure in code
https://aws.amazon.com/cdk
Apache License 2.0
11.71k stars 3.93k forks source link

(aws-msk-alpha): Cannot provide my own encryption key for SASL Scram authentication #22617

Open iriskraja77 opened 2 years ago

iriskraja77 commented 2 years ago

Describe the bug

ClientAuthentication.sasl properties currently allow you to provide an encryption key for the user secret. However, the construct does not leverage that key.

Current code for authentication configuration:

  clientAuthentication: ClientAuthentication.sasl({
    scram: true,
    key: props.kmsEncryptionKeyForCredentialsSecret,
  })

Then, add user: this.cluster.addUser(user)

Expected Behavior

It will use the encryption key I provide.

Current Behavior

It does not use the encryption key, nor creates new one. It throws an error:Cannot create users if an authentication KMS key has not been created/provided.

Reproduction Steps

See above.

Possible Solution

Initialize this.saslScramAuthenticationKey with the provided key, if set.

https://github.com/aws/aws-cdk/blob/main/packages/%40aws-cdk/aws-msk/lib/cluster.ts

Additional Information/Context

No response

CDK CLI Version

2.45

Framework Version

No response

Node.js Version

16.15.1

OS

MacOS

Language

Typescript

Language Version

No response

Other information

No response

peterwoodworth commented 2 years ago

Thanks for reporting this @iriskraja77, taking a quick look through the code it looks like we aren't ever making use of the key property (instead only checking once but just to see if it's defined and creates a new key if not). We make some checks starting here in the constructor: https://github.com/aws/aws-cdk/blob/4c11af6067b35125781aa590bb33c7990078d1ed/packages/%40aws-cdk/aws-msk/lib/cluster.ts#L448

We'll have to make sure to parse the key correctly if passed in.

I am marking this issue as p2, which means that we are unable to work on this immediately.

We use +1s to help prioritize our work, and are happy to revaluate this issue based on community feedback. You can reach out to the cdk.dev community on Slack to solicit support for reprioritization.

Check out our contributing guide if you're interested in contributing yourself - there's a low chance the team will be able to address this soon but we'd be happy to review a PR 🙂

bvicinay commented 1 year ago

+1