aws-cdk-lib: CfnInclude "parameters" description misleading

[jarettgross]

Describe the issue

The optional "parameters" prop states that if the prop is not provided, no parameters will be replaced. Additionally it states that if any parameter is not specified here, then it is left unmodified.

However, if the parameter has a "default" value in the template, then if that parameter is omitted from CfnInclude, the parameter is reset to the default. It is modified from what it originally was.

The wording of the documentation sounds misleading in this case. The template is unmodified, but the parameter itself is changed.

Links

https://docs.aws.amazon.com/cdk/api/v2/docs/aws-cdk-lib.cloudformation_include.CfnInclude.html#parameters

[peterwoodworth]

I'm not seeing any inconsistent behaviors here - If I import a template, and then reference its parameter that has a default value, that default value will get carried over to the CDK stack. You're finding that a parameter with a default value isn't having that default value imported, correct?

[jarettgross]

The parameter is being set to the default value provided in the stack.

The issue I am seeing is that I have a stack that already exists in Cloudformation, with, say, a parameter called "MyRoleName" and a value of "TestData".

I am using CDK to import this existing stack so that CDK owns the stack. "MyRoleName" has a default value in the template of "OldData".

I then use CDK's CfnInclude to import the stack, but I leave my parameters block empty.

When deploying my CDK stack, "MyRoleName" then takes the value of "OldData". It overwrites the "TestData" value that previously existed.

From the wording in the documentation, it sounds like "TestData" would still be the value

Any parameters in the template that aren't specified here will be left unmodified.

This feels misleading because the value of "MyRoleName" is modified to the default value.

[peterwoodworth]

The parameter definition isn't being modified here - There's no way to know what the supplied value was for a parameter for a live stack. If you import a parameter with a default value, then i think most users would expect that your new stack would need to have the parameter supplied to override the default value.

Cfn docs on parameters https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/parameters-section-structure.html#parameters-section-structure-syntax.yaml, note how the parameter definition in your live stack and new stack is exactly the same.

[jarettgross]

The parameter definition is not being modified -- that's correct, but the parameter value is being modified. I think adding that differentiation ("parameter definitions") in the docs would be helpful.

From these two lines in the docs:

(optional, default: no parameters will be replaced)
Any parameters in the template that aren't specified here will be left unmodified

To me, it reads as though the entire parameter, both definition and value, will be unmodified. I interpreted this to mean "if the parameter is not supplied then the parameter, both definition and value, are unmodified and not replaced"

[MCWoo]

Hello, speaking for myself, perhaps this is an issue of not being intimately familiar with what CDK's CloudFormation's Include does that causes this confusion about what will be left unmodified means. But even if it said something like will retain its original parameter definition, I'm still not sure I'd expect it to revert to its original value. I would hope that it'd be similar to a CloudFormation deploy, which retains any parameters' values that are not specified. So the additional clarity would be helpful and I don't think it's harmful to add an extra sentence.

We ask about this documentation because it was related to a live outage to our customers that was thankfully easy to rectify. And though this may not be the sole issue, if the documentation were clearer we may have avoided such an outage. We are relatively new to CDK, and I would hope that the documentation would be helpful to our situation as we get up to speed on everything CDK related.

Thanks!

[peterwoodworth]

I hear you guys that this is confusing to people who aren't very familiar with our services yet. Thanks for letting me know what the exact confusion is

Check out my PR, hope this provides the necessary clarity #21001 @MCWoo @jarettgross

[jarettgross]

Thanks for putting up the PR! That reads great and definitely helps clarify this situation. I appreciate you taking the time to consider and review this.

[github-actions]

⚠️COMMENT VISIBILITY WARNING⚠️

Comments on closed issues are hard for our team to see.
If you need more assistance, please either tag a team member or open a new issue that references this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.