search

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.55k stars 3.87k forks source link

chore(kinesisfirehose-alpha): refactor encryption property to combine encryptionKey #31430

Closed paulhcsun closed 2 weeks ago

paulhcsun commented 2 weeks ago

Reason for this change

The previous encryption and encryptionKey properties required error handling to enforce when an encryptionKey could be specified and when it was invalid (only valid when using CUSTOMER_MANAGED_KEY).

The properties should be combined to make this user experience more straightforward and only allow a KMS key to be passed in when using a customer-managed key.

Description of changes

BREAKING CHANGE: encryptionKey property is removed and encryption property type has changed from the StreamEncryption enum to the StreamEncryption class.

To pass in a KMS key for the customer managed key case, use StreamEncryption.customerManagedKey(key)

Details

Replaced encryption and encryptionKey properties with a single property encryption of type StreamEncryption and is used by calling one of the 3 methods:

SreamEncryption.unencrypted()
StreamEncryption.awsOwnedKey()
StreamEncryption.customerManagedKey(key?: IKey)

This makes it so it's not longer possible to pass in a key when the encryption type is AWS owned or unencrypted. The key is an optional parameter in StreamEncryption.customerManagedKey(key?: IKey) so following the previous behaviour, if a key is provided it will be used, otherwise a key will be created for the user.

Description of how you validated changes

Generated templates do not change so behaviour remains the same.

Updated integ/unit tests.

Checklist

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

paulhcsun commented 2 weeks ago

@GavinZZ the main issue with the current usage is that it requires the user to know when it is appropriate to pass in an encryptionKey depending on which encryptionType they used.

i.e. passing in an encryptionKey is only valid when the encryptionType === CUSTOMER_MANAGED_KEY and would throw an error if the key were passed in with encryptionType set to AWS_OWNED or UNENCRYPTED. This change makes it so that then the encryptionType is AWS_OWNED or UNENCRYPTED the user does not have an option to pass in a key which will remove the need for that error handling and remove the need to the user to know the restrictions.

mergify[bot] commented 2 weeks ago

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify[bot] commented 2 weeks ago

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

aws-cdk-automation commented 2 weeks ago

AWS CodeBuild CI Report

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

mergify[bot] commented 2 weeks ago

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

github-actions[bot] commented 2 weeks ago

Comments on closed issues and PRs are hard for our team to see. If you need help, please open a new issue that references this one.