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

feat(cfn-include): allow passing Parameters to the included template #9543

Merged

mergify merged 81 commits into aws:master from comcalvi:compile-time-parameters

Aug 13, 2020

Contributor

comcalvi commented Aug 8, 2020

Closes #4994

Cfn-Include can now be passed a mapping of parameters and their values. Specified parameters will have all references to them replaced with the passed value at build time, and their definitions will be removed from the template. Unspecified parameters and references to them will not be modified.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

comcalvi and others added 30 commits

June 4, 2020 17:44


          doc: added slack link to readme

db60c27


          Merge branch 'master' into master

5924f3b


          Update README.md

f0c85a3

Co-authored-by: Adam Ruka <adamruka85@gmail.com>


          Merge branch 'master' into master

f37e60e


          Merge branch 'master' of https://github.com/aws/aws-cdk

83ed743


          added support for Fn::Select, Fn::FindInMap, Fn::Cidr, Fn::GetAZs, Fn…

c6664af

…::And, Fn::Not, and Fn::Or, including unit tests.


          added support for the 'Fn::Transform' cloudformation intrinsic function.

d0f64d2


          added support for the Fn::Base64 Intrinsic Function

d69e46d


          tested more complex combinations of conditional and non-conditional i…

df1446a

…ntrinsic functions


          fixed linter issues

132cb4d


          implmented Adam's requests and fixed additional linter issues

ae7c570


          updated README to reflect the newly supported cloudformation functions

7ca707e


          removed quotes from the type of Transform's parameter argument, modif…

63c27a7

…ied the return type of transform to IResolvable, and updated transform docs


          fixed teseting issue related to Fn::Select


          Merge branch 'master' into CfnFunctions

117ae63


          fixing merge conflicts

dbb9ef8


          Merge branch 'master' of https://github.com/aws/aws-cdk

bdab761


          Merge branch 'master' of https://github.com/aws/aws-cdk into CfnFunct…

d390580

…ions


          merge conflict resolution

14a0d64


          Merge branch 'CfnFunctions' of github.com:comcalvi/aws-cdk into CfnFu…

dfb42b9

…nctions


          Merge branch 'master' of https://github.com/aws/aws-cdk into CfnFunct…

9f1b2d9

…ions


          fixed typo in condition name

4c9d1ec


          Merge branch 'CfnFunctions'

e15c187


          Merge branch 'master' of https://github.com/aws/aws-cdk

82c4317


          removed parameters from _toCloudFormation()

42c0228


          added support for parameters in templates

6a2df1e


          fixed merge conflict

43198b7


          updated documentation

cd27ff6


          updated readme

1e72465


          incorporated adam's comments

070f902

comcalvi added 2 commits

August 11, 2020 16:18


          incorporated review comments

f84bccf


          fixed linter issues

1c511f0

comcalvi requested a review from skinny85

August 11, 2020 20:47

skinny85 suggested changes

View reviewed changes

Contributor

skinny85 left a comment

Quick first look.

packages/@aws-cdk/cloudformation-include/README.md Outdated Show resolved Hide resolved

packages/@aws-cdk/cloudformation-include/README.md Outdated Show resolved Hide resolved

packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts Outdated Show resolved Hide resolved

tools/cfn2ts/lib/codegen.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/core/lib/from-cfn.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/core/lib/cfn-parse.ts Show resolved Hide resolved

packages/@aws-cdk/core/lib/cfn-parse.ts Outdated Show resolved Hide resolved

skinny85 reviewed

View reviewed changes

packages/@aws-cdk/core/lib/cfn-parse.ts Outdated Show resolved Hide resolved

skinny85 reviewed

View reviewed changes

packages/@aws-cdk/core/lib/cfn-parse.ts Outdated Show resolved Hide resolved

skinny85 suggested changes

View reviewed changes

Contributor

skinny85 left a comment

Final round of comments, now that I had some more time to look at the code.

packages/@aws-cdk/core/lib/cfn-parse.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/core/lib/cfn-parse.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-parameter-shadow.json Outdated Show resolved Hide resolved

packages/@aws-cdk/cloudformation-include/test/test-templates/fn-sub-parameters.json Outdated Show resolved Hide resolved

skinny85 reviewed

View reviewed changes

packages/@aws-cdk/core/lib/cfn-parse.ts Outdated Show resolved Hide resolved

comcalvi added 2 commits

August 12, 2020 17:34


          incorporated review comments

e49c836


          removed unneeded import

b6d443f

comcalvi requested a review from skinny85

August 12, 2020 22:30


          removed commented code

548018f

skinny85 suggested changes

View reviewed changes

Contributor

skinny85 left a comment

We're this close 👌 to merging this one. There are just a few simplifications I want to make first. But this is great!

packages/@aws-cdk/cloudformation-include/README.md Outdated Show resolved Hide resolved

packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts Show resolved Hide resolved

packages/@aws-cdk/core/lib/cfn-parse.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/core/lib/index.ts Outdated Show resolved Hide resolved

tools/cfn2ts/lib/codegen.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/cloudformation-include/test/valid-templates.test.ts Outdated Show resolved Hide resolved

comcalvi added 3 commits

August 12, 2020 19:07


          incorporated review comments

9f90801


          import order fixed

79c6ff3


          incorporated more review comments

4598c6e

comcalvi requested a review from skinny85

August 12, 2020 23:28


          fixed from-cfn issue

4923d72

skinny85 added the pr/do-not-merge label

skinny85 approved these changes

View reviewed changes

Contributor

skinny85 left a comment

Looks good! Some cosmetic comments. After you've addressed them, remove the do-not-merge label, and the PR will be automatically merged in.

packages/@aws-cdk/core/lib/cfn-parse.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/core/lib/cfn-parse.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/core/lib/cfn-parse.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/core/lib/cfn-parse.ts Show resolved Hide resolved

tools/cfn2ts/lib/codegen.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/core/lib/cfn-parse.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/core/lib/cfn-parse.ts Outdated Show resolved Hide resolved

packages/@aws-cdk/cloudformation-include/lib/cfn-include.ts Outdated Show resolved Hide resolved


          cosmetic fixes

36b731c

comcalvi removed the pr/do-not-merge label

Contributor

mergify bot commented Aug 13, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).


          Merge branch 'master' into compile-time-parameters

d281c23

Collaborator

aws-cdk-automation commented Aug 13, 2020

AWS CodeBuild CI Report

CodeBuild project: AutoBuildProject6AEA49D1-qxepHUsryhcu
Commit ID: d281c23
Result: SUCCEEDED
Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

Contributor

mergify bot commented Aug 13, 2020

Thank you for contributing! Your pull request will be updated from master and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

mergify bot merged commit cb6de0a into aws:master

joekiller commented Aug 13, 2020

Thanks for making this clear! @skinny85

joekiller commented Aug 13, 2020

also thanks @comcalvi

Contributor

skinny85 commented Aug 13, 2020

You're welcome @joekiller ! If you have any feedback about using cloudformation-include (bugs, feature requests, etc.), please, let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment