Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Sep 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions packages/@aws-cdk/aws-kinesisfirehose-alpha/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -153,18 +153,18 @@ declare const destination: firehose.IDestination;

// SSE with an AWS-owned key
new firehose.DeliveryStream(this, 'Delivery Stream AWS Owned', {
encryption: firehose.StreamEncryption.AWS_OWNED,
encryption: firehose.StreamEncryption.awsOwnedKey(),
destinations: [destination],
});
// SSE with an customer-managed key that is created automatically by the CDK
new firehose.DeliveryStream(this, 'Delivery Stream Implicit Customer Managed', {
encryption: firehose.StreamEncryption.CUSTOMER_MANAGED,
encryption: firehose.StreamEncryption.customerManagedKey(),
destinations: [destination],
});
// SSE with an customer-managed key that is explicitly specified
declare const key: kms.Key;
new firehose.DeliveryStream(this, 'Delivery Stream Explicit Customer Managed', {
encryptionKey: key,
encryption: firehose.StreamEncryption.customerManagedKey(key),
destinations: [destination],
});
```
Expand Down
23 changes: 7 additions & 16 deletions packages/@aws-cdk/aws-kinesisfirehose-alpha/lib/delivery-stream.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { Construct, Node } from 'constructs';
import { IDestination } from './destination';
import { FirehoseMetrics } from 'aws-cdk-lib/aws-kinesisfirehose/lib/kinesisfirehose-canned-metrics.generated';
import { CfnDeliveryStream } from 'aws-cdk-lib/aws-kinesisfirehose';
import { StreamEncryption } from './encryption';

const PUT_RECORD_ACTIONS = [
'firehose:PutRecord',
Expand Down Expand Up @@ -162,7 +163,7 @@ abstract class DeliveryStreamBase extends cdk.Resource implements IDeliveryStrea
/**
* Options for server-side encryption of a delivery stream.
*/
export enum StreamEncryption {
export enum StreamEncryptionType {
/**
* Data in the stream is stored unencrypted.
*/
Expand Down Expand Up @@ -216,16 +217,9 @@ export interface DeliveryStreamProps {
/**
* Indicates the type of customer master key (CMK) to use for server-side encryption, if any.
*
* @default StreamEncryption.UNENCRYPTED - unless `encryptionKey` is provided, in which case this will be implicitly set to `StreamEncryption.CUSTOMER_MANAGED`
* @default StreamEncryption.unencrypted()
*/
readonly encryption?: StreamEncryption;

/**
* Customer managed key to server-side encrypt data in the stream.
*
* @default - no KMS key will be used; if `encryption` is set to `CUSTOMER_MANAGED`, a KMS key will be created for you
*/
readonly encryptionKey?: kms.IKey;
}

/**
Expand Down Expand Up @@ -334,23 +328,20 @@ export class DeliveryStream extends DeliveryStreamBase {
throw new Error(`Only one destination is allowed per delivery stream, given ${props.destinations.length}`);
}

if (props.encryptionKey || props.sourceStream) {
if (props.encryption?.encryptionKey || props.sourceStream) {
this._role = this._role ?? new iam.Role(this, 'Service Role', {
assumedBy: new iam.ServicePrincipal('firehose.amazonaws.com'),
});
}

if (
props.sourceStream &&
(props.encryption === StreamEncryption.AWS_OWNED || props.encryption === StreamEncryption.CUSTOMER_MANAGED || props.encryptionKey)
(props.encryption?.type === StreamEncryptionType.AWS_OWNED || props.encryption?.type === StreamEncryptionType.CUSTOMER_MANAGED)
) {
throw new Error('Requested server-side encryption but delivery stream source is a Kinesis data stream. Specify server-side encryption on the data stream instead.');
}
if ((props.encryption === StreamEncryption.AWS_OWNED || props.encryption === StreamEncryption.UNENCRYPTED) && props.encryptionKey) {
throw new Error(`Specified stream encryption as ${StreamEncryption[props.encryption]} but provided a customer-managed key`);
}
const encryptionKey = props.encryptionKey ?? (props.encryption === StreamEncryption.CUSTOMER_MANAGED ? new kms.Key(this, 'Key') : undefined);
const encryptionConfig = (encryptionKey || (props.encryption === StreamEncryption.AWS_OWNED)) ? {
const encryptionKey = props.encryption?.encryptionKey ?? (props.encryption?.type === StreamEncryptionType.CUSTOMER_MANAGED ? new kms.Key(this, 'Key') : undefined);
const encryptionConfig = (encryptionKey || (props.encryption?.type === StreamEncryptionType.AWS_OWNED)) ? {
keyArn: encryptionKey?.keyArn,
keyType: encryptionKey ? 'CUSTOMER_MANAGED_CMK' : 'AWS_OWNED_CMK',
} : undefined;
Expand Down
44 changes: 44 additions & 0 deletions packages/@aws-cdk/aws-kinesisfirehose-alpha/lib/encryption.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { StreamEncryptionType } from './delivery-stream';
import { IKey } from 'aws-cdk-lib/aws-kms';

/**
* Represents server-side encryption for a Kinesis Firehose Delivery Stream.
*/
export abstract class StreamEncryption {
/**
* No server-side encryption is configured.
*/
public static unencrypted(): StreamEncryption {
return new (class extends StreamEncryption {
}) (StreamEncryptionType.UNENCRYPTED);
}

/**
* Configure server-side encryption using an AWS owned key.
*/
public static awsOwnedKey(): StreamEncryption {
return new (class extends StreamEncryption {
}) (StreamEncryptionType.AWS_OWNED);
}

/**
* Configure server-side encryption using customer managed keys.
*
* @param encryptionKey the KMS key for the delivery stream.
*/
public static customerManagedKey(encryptionKey?: IKey): StreamEncryption {
return new (class extends StreamEncryption {

}) (StreamEncryptionType.CUSTOMER_MANAGED, encryptionKey);
}
GavinZZ marked this conversation as resolved.
Show resolved Hide resolved

/**
* Constructor for StreamEncryption.
*
* @param type The type of server-side encryption for the Kinesis Firehose delivery stream.
* @param encryptionKey Optional KMS key used for customer managed encryption.
*/
Comment on lines +35 to +40
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

private constructor (
public readonly type: StreamEncryptionType,
public readonly encryptionKey?: IKey) {}
}
1 change: 1 addition & 0 deletions packages/@aws-cdk/aws-kinesisfirehose-alpha/lib/index.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export * from './delivery-stream';
export * from './destination';
export * from './encryption';
export * from './lambda-function-processor';
export * from './processor';

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import * as targets from 'aws-cdk-lib/aws-events-targets';
import * as cdk from 'aws-cdk-lib';
import { Construct, Node } from 'constructs';
import * as firehose from '../lib';
import { StreamEncryption } from '../lib';

describe('delivery stream', () => {
let stack: cdk.Stack;
Expand Down Expand Up @@ -151,12 +152,12 @@ describe('delivery stream', () => {
Template.fromStack(stack).resourceCountIs('AWS::IAM::Role', 2);
});

test('not providing role but specifying encryptionKey creates two roles', () => {
test('not providing role but using customerManagedKey encryption with a key creates two roles', () => {
const key = new kms.Key(stack, 'Key');

new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
encryptionKey: key,
encryption: StreamEncryption.customerManagedKey(key),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

});

Template.fromStack(stack).hasResourceProperties('AWS::IAM::Role', {
Expand Down Expand Up @@ -215,7 +216,7 @@ describe('delivery stream', () => {
test('requesting customer-owned encryption creates key and configuration', () => {
new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
encryption: firehose.StreamEncryption.CUSTOMER_MANAGED,
encryption: firehose.StreamEncryption.customerManagedKey(),
role: deliveryStreamRole,
});

Expand Down Expand Up @@ -246,12 +247,12 @@ describe('delivery stream', () => {
});
});

test('providing encryption key creates configuration', () => {
test('using customerManagedKey encryption with provided key creates configuration', () => {
const key = new kms.Key(stack, 'Key');

new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
encryptionKey: key,
encryption: StreamEncryption.customerManagedKey(key),
role: deliveryStreamRole,
});

Expand Down Expand Up @@ -281,7 +282,7 @@ describe('delivery stream', () => {
test('requesting AWS-owned key does not create key and creates configuration', () => {
new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
encryption: firehose.StreamEncryption.AWS_OWNED,
encryption: firehose.StreamEncryption.awsOwnedKey(),
role: deliveryStreamRole,
});

Expand All @@ -299,7 +300,7 @@ describe('delivery stream', () => {
test('requesting no encryption creates no configuration', () => {
new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
encryption: firehose.StreamEncryption.UNENCRYPTED,
encryption: firehose.StreamEncryption.unencrypted(),
role: deliveryStreamRole,
});

Expand All @@ -311,42 +312,22 @@ describe('delivery stream', () => {
});
});

test('requesting AWS-owned key and providing a key throws an error', () => {
const key = new kms.Key(stack, 'Key');

expect(() => new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
encryption: firehose.StreamEncryption.AWS_OWNED,
encryptionKey: key,
})).toThrowError('Specified stream encryption as AWS_OWNED but provided a customer-managed key');
});

test('requesting no encryption and providing a key throws an error', () => {
const key = new kms.Key(stack, 'Key');

expect(() => new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
encryption: firehose.StreamEncryption.UNENCRYPTED,
encryptionKey: key,
})).toThrowError('Specified stream encryption as UNENCRYPTED but provided a customer-managed key');
});

test('requesting encryption or providing a key when source is a stream throws an error', () => {
const sourceStream = new kinesis.Stream(stack, 'Source Stream');

expect(() => new firehose.DeliveryStream(stack, 'Delivery Stream 1', {
destinations: [mockS3Destination],
encryption: firehose.StreamEncryption.AWS_OWNED,
encryption: firehose.StreamEncryption.awsOwnedKey(),
sourceStream,
})).toThrowError('Requested server-side encryption but delivery stream source is a Kinesis data stream. Specify server-side encryption on the data stream instead.');
expect(() => new firehose.DeliveryStream(stack, 'Delivery Stream 2', {
destinations: [mockS3Destination],
encryption: firehose.StreamEncryption.CUSTOMER_MANAGED,
encryption: firehose.StreamEncryption.customerManagedKey(),
sourceStream,
})).toThrowError('Requested server-side encryption but delivery stream source is a Kinesis data stream. Specify server-side encryption on the data stream instead.');
expect(() => new firehose.DeliveryStream(stack, 'Delivery Stream 3', {
destinations: [mockS3Destination],
encryptionKey: new kms.Key(stack, 'Key'),
encryption: StreamEncryption.customerManagedKey(new kms.Key(stack, 'Key')),
sourceStream,
})).toThrowError('Requested server-side encryption but delivery stream source is a Kinesis data stream. Specify server-side encryption on the data stream instead.');
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ const key = new kms.Key(stack, 'Key', {

new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
encryptionKey: key,
encryption: firehose.StreamEncryption.customerManagedKey(key),
});

new firehose.DeliveryStream(stack, 'Delivery Stream No Source Or Encryption Key', {
Expand Down