(aws-kinesis): fails to deploy on-demand stream

[igilham]

What is the problem?

Error from CloudFormation when using Kinesis Data Streams in on-demand mode with shardCount specified.

ShardCount is not expected when StreamMode=ON_DEMAND

Reproduction Steps

Create a new stream with a streamMode set.

new Stream(this, 'KinesisStream', {
    streamName: 'my-ingest-stream',
    shardCount: 1, // optional
    streamMode: StreamMode.ON_DEMAND,
});

What did you expect to happen?

Deploy a stream in on-demand mode, possibly informed by the shardCount for an initial scaling hint.

What actually happened?

Error from CloudFormation

ShardCount is not expected when StreamMode=ON_DEMAND

CDK CLI Version

1.137.0 (build bfbdf64)

Framework Version

1.137.0

Node.js Version

14.17.3

OS

MacOS

Language

Typescript

Language Version

4.5.2

Other information

CloudFormation's documentation does not inform the user that setting the StreamMode to ON_DEMNAD is an error. Can we raise a bug with the right team to get the documentation improved?

Removing the shardCount field from the CDK user code does not fix the error because internally, CDK always sets the shardCount to 1 if it is left undefined by the user.

The solution in CDK is to remove shardCount from the underlying CfnStream if the user did not provide the field. CloudFormation will default to 1 anyway if needed, so CDK's default is not doing anything useful here.

We should also add a validator to ensure that shardCount and StreamMode: 'ON_DEMAND' are not set at the same time when generating the template.

An end-to-end test through CloudFormation would have caught this bug before release. I'm not aware that there is any automated testing of this type at present.

[igilham]

Since I wrote the code for this feature, I can only apologise for not testing it through CloudFormation before shipping it.

[otaviomacedo]

@igilham do you think you can submit another PR with these fixes?

An end-to-end test through CloudFormation would have caught this bug before release. I'm not aware that there is any automated testing of this type at present.

There are integration tests all over the CDK code base. You can take a look and write your own. It's pretty straightforward.

[igilham]

I'd be happy to fix the bug when I have time. It'll probably be next week. I'm not familiar with all the testing layers so doing a thorough job may not be such a quick fix but I'm happy to take a look.

If anyone wants to do it sooner than I can, I won't be offended but I'll aim to pick this up soon if nobody else has.

[igilham]

@otaviomacedo I'm making a start on this. Can you help me to understand how the L1 validators are generated? Do they come directly from the CloudFormation spec or are they generated from somewhere else in the repo?

Edit: Nevermind. I found a similar solution in DynamoDB's tables so I've copied the existing pattern.

[d4r3topk]

Have you tested the code for both PROVISIONED and ON_DEMAND options to check whether it works for the following scenarios.

• PROVISIONED without a shard count - should default to 1
• PROVISIONED with a shard count
• ON_DEMAND without shard count

[igilham]

@d4r3topk I'm working through these scenarios now.

I initially thought it would be easiest to remove default values from the generated CloudFormation and allow the upstream service's defaults to take over. Now that I've been spending a bit more time with CDK's codebase, this seems not to be the preferred pattern, though. CDK likes to be explicit where possible.

I'm now following the example set by Table in the DynamoDB module. This uses guard statements in the L2 constructor to ensure the correct defaults are applied in each scenario, or an error is thrown if the user sets incompatible options.

I can't get the tests to run on my computer so I'm relying on the CodeBuild infrastructure connected to the Pull Request. I hope to finish getting it ready today.