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

[paulhcsun]

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

• My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

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

[paulhcsun]

Added to deal with awslint errors:

error: [awslint:docs-public-apis:@aws-cdk/aws-kinesisfirehose-alpha.StreamEncryption.type] Public API element must have a docstring
error: [awslint:docs-public-apis:@aws-cdk/aws-kinesisfirehose-alpha.StreamEncryption.encryptionKey] Public API element must have a docstring

[GavinZZ]

What's the purpose for this change? Is there an issue about the current usage?

[GavinZZ]

This pattern feels a bit off to me. StreamEncryption usage sounds like a ENUM to me but the actual usage is calling a statis method.

[paulhcsun]

This is the same pattern that's being used for TableV2's encryption here.

[paulhcsun]

@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]

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]

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]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 86cb919
• Result: SUCCEEDED
• Build Logs (available for 30 days)

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

[mergify]

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]

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.