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.51k stars 3.85k forks source link

aws_kinesis : Kinesis does not include streamMode Provisioned in CFN Template #31293

Closed Exter-dg closed 1 week ago

Exter-dg commented 1 week ago

Describe the bug

The Kinesis Stream documentation says that the default value of streamMode is Provisioned. But the cloudformation template omits this field in the latest version. It was set to Provisioned explicitly in previous versions - https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.aws_kinesis.Stream.html#streammode

This was not an issue earlier in 1.204.0 as streamMode was set to Provisioned in the CDK Code itself - https://github.com/aws/aws-cdk/blob/9752ed225c82982d8b443f7c6a47f47979458217/packages/%40aws-cdk/aws-kinesis/lib/stream.ts#L759C44-L759C67

But currently, it is set to undefined. While migrating from 1.204.0 to latest, I noticed that StreamMode was being removed from my Cloudformation template (diff). Even though the documentation states that it will be Provisioned by default.

Regression Issue

Last Known Working CDK Version

1.204.0

Expected Behavior

StreamMode should be set to Provisioned by default as mentioned in the docs

Current Behavior

StreamMode is undefined and omitted from Cloudformation Template

Reproduction Steps

const myStream = new kinesis.Stream(this, "MyFirstStream", {
      streamName: "myStreamName",
      shardCount: 1,
      retentionPeriod: cdk.Duration.hours(retentionPeriodInDays * 24),
    });

Possible Solution

Set default value of streammode to provisioned in the code

Additional Information/Context

No response

CDK CLI Version

2.108.1

Framework Version

No response

Node.js Version

v22.1.0

OS

MacOs

Language

TypeScript

Language Version

No response

Other information

No response

Exter-dg commented 1 week ago

I believe this was removed as part of this issue and the commit associated with it: https://github.com/aws/aws-cdk/issues/21829.

If that's the case, isn't the documentation incorrect to state that streamMode will be set to Provisioned by default?

ashishdhingra commented 1 week ago

@Exter-dg Good morning. Thanks for opening the issue. The documentation just indicates that the property streamMode is not optional and if the value is not specified, then CloudFormation by default considers value as StreamMode.PROVISIONED. The issue https://github.com/aws/aws-cdk/issues/21829 (and associated PR https://github.com/aws/aws-cdk/pull/24994) that you linked just removes the redundant initialization of the streamMode property. I do not see it as a breaking change since the behavior in CloudFormation is unchanged and documentation correctly reflects this behavior.

Thanks, Ashish

pahud commented 1 week ago

I believe this was previously a bug that got fixed in https://github.com/aws/aws-cdk/pull/24994

In that bug, when props.streamMode is undefined, it was previously explicit set as Provisioned under StreamModeDetails, which was not required.

The fix made it implicitly set as Provisioned by CFN with StreamModeDetails undefined. The behavior should be the same but if you npx cdk synth you should see this:

   Type: AWS::Kinesis::Stream
    Properties:
      Name: myStreamName
      RetentionPeriodHours: 24
      ShardCount: 1
      StreamEncryption:
        Fn::If:
          - AwsCdkKinesisEncryptedStreamsUnsupportedRegions
          - Ref: AWS::NoValue
          - EncryptionType: KMS
            KeyId: alias/aws/kinesis

The StreamModeDetails would be undefined as fixed in https://github.com/aws/aws-cdk/pull/24994 but the streamMode should be implicitly Provisioned.

Given StreamModeDetails is having Update requires: No interruption as described in the CFN doc. This should not cause any resource replacement.

ashishdhingra commented 1 week ago

@Exter-dg Please review guidance above and confirm if we are good to close the issue.

Thanks, Ashish

Exter-dg commented 1 week ago

Thanks @ashishdhingra @pahud, This makes things clearer for me. Closing this issue.

github-actions[bot] commented 1 week 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.