-
Notifications
You must be signed in to change notification settings - Fork 4k
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.
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
feat(bootstrap): customizable bootstrap template #9886
Conversation
There are many requests for customization of the built-in bootstrapping template. Rather than implementing each and every request, it's more productive to allow users to help themselves. This change introduces two new flags to `cdk bootstrap`: * `cdk bootstrap --show-template`: prints the current template to stdout, which people can pipe to a file. * `cdk bootstrap --template FILE`: reads the template from a file instead of using the built-in template. This can be used to arbitrarily customize the bootstrapping template for use in any organization. I know that the documentation changes in this PR are pretty light, but really a Developer Guide topic should be written on bootstrapping, which is next on my TODO list.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've not looked through the code but simply through the commit description and the README.
There are a couple of things that don't feel right about this change.
- There are some minimum requirements for a bootstrap stack to be 'valid'.
While I agree that supporting all of the customizations on the CLI is ugly, allowing any yaml as a bootstrap stack feels wrong as well. It can give the perception that the account/region is bootstrapped while it actually isn't.
Can we find some middle ground where customers can customize their stack but still meet some minimum CDK requirements?
- Allowing customers to provide an arbitrary template that is not checked in or codified anywhere seems anti to the behaviour we have been encouraging.
This leaves questions open around where the customized bootstrap template will be stored and how it can be guaranteed to be consistent across all of their accounts/regions. One can argue that this is not our responsibility but I think it is. It should be our responsibility to encourage customers do the right thing in terms of how they interact with the CDK and with their AWS accounts via the CDK.
It feels like we should explore better options here. Maybe let customers define the bootstrap stack as a CDK app/stack? If that's not feasible, other options?
I have a feeling you're letting perfect be the enemy of good here. Let's establish first that there is a need for this. We have a slew of requests for juuuuust this one customization of the bootstrap stack, so that my organization is happy. And just one more for this other person. Oh and we have our resources slightly differently. Oh and you're going to maintain and test all these combinations going forward, right? Furthermore, we always had the plan of providing the template to ops engineers at $BIG_CO, so that they can bootstrap a large number of accounts at the same time using--what's the service again, Control Tower? They will want to take the template in a form that's [currently] familiar to them, inspect it and then roll it out in their organization.
Sure -- we can add validations on the template. Is that more important than adding the base capability first? And honestly, many things of the template are customizable on purpose (you just have to tell the Stack Synthesizer about the changes you made). Wouldn't it be great to have a UI or a tool that warned you exactly about the non-standard changes you were making and gave you a piece of code to copy/paste into your code to configure the StackSynthesizer exactly how you wanted it? Sure! Is that really a prerequisite for adding the capability first?
I'm not really sure what you're implying here. Do we need to force people to only refer to templates stored in git repositories? Do you want to tie the bootstrap stack to a particular CDK app (and force it to be stored in its repository?). The bootstrap stack can be reused between applications so that wouldn't be a correct modeling.
Yeah, that'd be a good idea. Doesn't REALLY solve the customization issue though. Let's say the bootstrap stack was available as a construct in I guess we could think about vending a "Bootstrapping Stack Kit", we could have constructs in there to Build Your Own Bootstrap Stack. When comfortable with the CDK, it would be easier to customize then. It would also:
I'm not opposed against it, it would be nicely meta, and the features we would build for this would be pretty sweet (insta-deploy a CDK app from NPM into your account would be a cool feature to have!), but I'm not sure it's the most important thing to work on right now. |
I think you are expecting too much all at once
Looks like you've made up your mind that this change needs to go through, and anything I say is going to dead end at "Do you have a better solution that can be shipped to customers straight away?". However, I still think this is going to introduce more ways for customers to shoot themselves in the foot by bootstrapping 'invalid' templates. Sure, they can adjust the KMS key permissions or bring their own, but should they be able to delete the staging bucket entirely; or ECR repo? If this does occur, IIUC, the error is going to occur at suggestion: a 'validate bootstrap' command on the CLI that ensures that the bootstrap stack meets a minimum bar? |
We can totally do that. Unfortunately, be aware this is an infeasible problem to solve in generality. We cannot statically check that the user running That may catch some things, but it will not catch everything. I am okay with that, with the understanding that the custom bootstrap template is a power user feature and so I am more likely to trust the people that reach for it. Who knows, I might come to regret this position :). |
If customers do shoot themselves in the foot here, how might it manifest, and are there any changes in messaging/etc. that we could invest in to make detection or troubleshooting easier? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mostly nitpicks in the code itself; not approving otherwise to ensure the "meta" conversation is resolved first.
Co-authored-by: Nick Lynch <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving, but marking do-not-merge just in case @nija-at or anyone else wants to make any final comments on the approach itself.
Hi @rix0rrr , thanks for this one, much appreciated. I actually had to bootstrap personal account to get the template to customize, as in our corporate accounts, the default bootstrap template fails to instantiate due to SCP restrictions. The However, I was thinking if, in it's current implementation, the More general thought: I would be a bit worried about the "stability" of the interface. What if a new version of CDK will need extra resources in the bootstrap stack... I will need to maintain my template consistency manually. For this reason, I would slightly prefer some way of providing a "patch" to be applied to the template, before instantiating. Random idea: http://jsonpatch.com/ |
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). |
I'm sensitive to all of your responses, but I'm going to push this through anyway. Motivation as follows:
|
AWS CodeBuild CI Report
Powered by github-codebuild-logs, available on the AWS Serverless Application Repository |
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). |
Template format error: unsupported structure. |
@tysoncung any luck on the validationError of bootstrap template |
There are many requests for customization of the built-in bootstrapping
template. Rather than implementing each and every request, it's more
productive to allow users to help themselves.
This change introduces two new flags to
cdk bootstrap
:cdk bootstrap --show-template
: prints the current template tostdout, which people can pipe to a file.
cdk bootstrap --template FILE
: reads the template from a fileinstead of using the built-in template.
This can be used to arbitrarily customize the bootstrapping template
for use in any organization.
I know that the documentation changes in this PR are pretty light,
but really a Developer Guide topic should be written on bootstrapping,
which is next on my TODO list.
Resolves #9256, resolves #8724, resolves #3684, resolves #1528, necessary for #9681.
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license