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.

Sign up for GitHub

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

Jump to bottom

chore(kinesisfirehose-alpha): replacedestinations property with destination and change type from array to single IDestination #31630

Merged
merged 10 commits into from
Oct 4, 2024
Merged

chore(kinesisfirehose-alpha): replacedestinations property with destination and change type from array to single IDestination #31630

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
5c4c153
chore(kinesis-firehose-alpha): change Stream's destination property t…
paulhcsun Oct 2, 2024
b9db409
update property name
paulhcsun Oct 3, 2024
78b093b
Merge branch 'main' into firehose-destination-property
paulhcsun Oct 3, 2024
7eb4b71
Merge branch 'main' into firehose-destination-property
paulhcsun Oct 3, 2024
e55454f
update tests in destinations module
paulhcsun Oct 3, 2024
9c86fb1
ujpdate test
paulhcsun Oct 3, 2024
ac6537d
Merge branch 'main' into firehose-destination-property
paulhcsun Oct 3, 2024
b42656f
update readmes
paulhcsun Oct 4, 2024
3f3a031
surely this is the last
paulhcsun Oct 4, 2024
13ae67f
Merge branch 'main' into firehose-destination-property
mergify[bot] Oct 4, 2024
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
8 changes: 2 additions & 6 deletions packages/@aws-cdk/aws-kinesisfirehose-alpha/lib/delivery-stream.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ export interface DeliveryStreamProps {
*
* Only a singleton array is supported at this time.
*/
readonly destinations: IDestination[];
readonly destinations: IDestination;

/**
* A name for the delivery stream.
Expand Down Expand Up @@ -324,10 +324,6 @@ export class DeliveryStream extends DeliveryStreamBase {

this._role = props.role;

if (props.destinations.length !== 1) {
throw new Error(`Only one destination is allowed per delivery stream, given ${props.destinations.length}`);
}

if (props.encryption?.encryptionKey || props.sourceStream) {
this._role = this._role ?? new iam.Role(this, 'Service Role', {
assumedBy: new iam.ServicePrincipal('firehose.amazonaws.com'),
Expand Down Expand Up @@ -369,7 +365,7 @@ export class DeliveryStream extends DeliveryStreamBase {
readStreamGrant = props.sourceStream.grantRead(this._role);
}

const destinationConfig = props.destinations[0].bind(this, {});
const destinationConfig = props.destinations.bind(this, {});

const resource = new CfnDeliveryStream(this, 'Resource', {
deliveryStreamEncryptionConfigurationInput: encryptionConfig,
Expand Down
57 changes: 24 additions & 33 deletions packages/@aws-cdk/aws-kinesisfirehose-alpha/test/delivery-stream.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ describe('delivery stream', () => {

test('creates stream with default values', () => {
new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
});

Template.fromStack(stack).hasResourceProperties('AWS::KinesisFirehose::DeliveryStream', {
Expand All @@ -63,7 +63,7 @@ describe('delivery stream', () => {

test('creates stream with events target V2 class', () => {
const stream = new firehose.DeliveryStream(stack, 'DeliveryStream', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
});

new events.Rule(stack, 'rule', {
Expand Down Expand Up @@ -102,7 +102,7 @@ describe('delivery stream', () => {
});

const deliveryStream = new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
role: role,
});

Expand All @@ -111,7 +111,7 @@ describe('delivery stream', () => {

test('not providing sourceStream or encryptionKey creates only one role (used for S3 destination)', () => {
new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
});

Template.fromStack(stack).hasResourceProperties('AWS::IAM::Role', {
Expand All @@ -133,7 +133,7 @@ describe('delivery stream', () => {
const sourceStream = new kinesis.Stream(stack, 'Source Stream');

new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
sourceStream: sourceStream,
});

Expand All @@ -156,7 +156,7 @@ describe('delivery stream', () => {
const key = new kms.Key(stack, 'Key');

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

Expand All @@ -179,7 +179,7 @@ describe('delivery stream', () => {
const sourceStream = new kinesis.Stream(stack, 'Source Stream');

new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
sourceStream: sourceStream,
role: deliveryStreamRole,
});
Expand Down Expand Up @@ -215,7 +215,7 @@ describe('delivery stream', () => {

test('requesting customer-owned encryption creates key and configuration', () => {
new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
encryption: firehose.StreamEncryption.customerManagedKey(),
role: deliveryStreamRole,
});
Expand Down Expand Up @@ -251,7 +251,7 @@ describe('delivery stream', () => {
const key = new kms.Key(stack, 'Key');

new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
encryption: StreamEncryption.customerManagedKey(key),
role: deliveryStreamRole,
});
Expand Down Expand Up @@ -281,7 +281,7 @@ describe('delivery stream', () => {

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

test('requesting no encryption creates no configuration', () => {
new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
encryption: firehose.StreamEncryption.unencrypted(),
role: deliveryStreamRole,
});
Expand All @@ -316,17 +316,17 @@ describe('delivery stream', () => {
const sourceStream = new kinesis.Stream(stack, 'Source Stream');

expect(() => new firehose.DeliveryStream(stack, 'Delivery Stream 1', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
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],
destinations: mockS3Destination,
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],
destinations: mockS3Destination,
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 All @@ -337,7 +337,7 @@ describe('delivery stream', () => {
assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'),
});
const deliveryStream = new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
});

deliveryStream.grant(role, 'firehose:PutRecord');
Expand All @@ -360,7 +360,7 @@ describe('delivery stream', () => {
assumedBy: new iam.ServicePrincipal('lambda.amazonaws.com'),
});
const deliveryStream = new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
});

deliveryStream.grantPutRecords(role);
Expand All @@ -385,7 +385,7 @@ describe('delivery stream', () => {
const dependableId = stack.resolve((Node.of(dependable).defaultChild as cdk.CfnResource).logicalId);

new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
});

Template.fromStack(stack).hasResource('AWS::KinesisFirehose::DeliveryStream', {
Expand All @@ -396,18 +396,9 @@ describe('delivery stream', () => {
});
});

test('supplying 0 or multiple destinations throws', () => {
expect(() => new firehose.DeliveryStream(stack, 'No Destinations', {
destinations: [],
})).toThrowError(/Only one destination is allowed per delivery stream/);
expect(() => new firehose.DeliveryStream(stack, 'Too Many Destinations', {
destinations: [mockS3Destination, mockS3Destination],
})).toThrowError(/Only one destination is allowed per delivery stream/);
});

test('creating new stream should return IAM role when calling getter for grantPrincipal (backwards compatibility)', () => {
const deliveryStream = new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
});
expect(deliveryStream.grantPrincipal).toBeInstanceOf(iam.Role);
});
Expand All @@ -418,7 +409,7 @@ describe('delivery stream', () => {
beforeEach(() => {
stack = new cdk.Stack(undefined, undefined, { env: { account: '000000000000', region: 'us-west-1' } });
deliveryStream = new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
});
});

Expand Down Expand Up @@ -516,7 +507,7 @@ describe('delivery stream', () => {
const vpc = new ec2.Vpc(stack, 'VPC');
const securityGroup = new ec2.SecurityGroup(stack, 'Security Group', { vpc });
const deliveryStream = new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
});

securityGroup.connections.allowFrom(deliveryStream, ec2.Port.allTcp());
Expand All @@ -542,7 +533,7 @@ describe('delivery stream', () => {
const vpc = new ec2.Vpc(stack, 'VPC');
const securityGroup = new ec2.SecurityGroup(stack, 'Security Group', { vpc });
const deliveryStream = new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
});

securityGroup.connections.allowFrom(deliveryStream, ec2.Port.allTcp());
Expand All @@ -558,10 +549,10 @@ describe('delivery stream', () => {

test('only adds one Firehose IP address mapping to stack even if multiple delivery streams defined', () => {
new firehose.DeliveryStream(stack, 'Delivery Stream 1', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
});
new firehose.DeliveryStream(stack, 'Delivery Stream 2', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
});

Template.fromStack(stack).hasMapping('*', {
Expand All @@ -573,7 +564,7 @@ describe('delivery stream', () => {

test('can add tags', () => {
const deliveryStream = new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
});

cdk.Tags.of(deliveryStream).add('tagKey', 'tagValue');
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-kinesisfirehose-alpha/test/integ.delivery-stream.source-stream.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,12 @@ const mockS3Destination: firehose.IDestination = {
const sourceStream = new kinesis.Stream(stack, 'Source Stream');

new firehose.DeliveryStream(stack, 'Delivery Stream', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
sourceStream,
});

new firehose.DeliveryStream(stack, 'Delivery Stream No Source Or Encryption Key', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
});

app.synth();
4 changes: 2 additions & 2 deletions packages/@aws-cdk/aws-kinesisfirehose-alpha/test/integ.delivery-stream.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,12 +36,12 @@ const key = new kms.Key(stack, 'Key', {
});

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

new firehose.DeliveryStream(stack, 'Delivery Stream No Source Or Encryption Key', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
});

app.synth();
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-kinesisfirehose-alpha/test/integ.kinesis-stream-events-target.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ const mockS3Destination: firehose.IDestination = {
};

const stream = new firehose.DeliveryStream(stack, 'Delivery Stream No Source Or Encryption Key', {
destinations: [mockS3Destination],
destinations: mockS3Destination,
});

new events.Rule(stack, 'rule', {
Expand Down
Loading