Skip to content
This repository has been archived by the owner on Sep 30, 2020. It is now read-only.

Proposal: Switch CloudFormation to YAML #96

Closed
cknowles opened this issue Nov 25, 2016 · 18 comments
Closed

Proposal: Switch CloudFormation to YAML #96

cknowles opened this issue Nov 25, 2016 · 18 comments

Comments

@cknowles
Copy link
Contributor

Quite self explanatory, is there any interest in switching to the YAML form of CloudFormation? If there is interest of at least POC, I can submit a pull request for it. If we go ahead, we'll need to pick the right timing for a merge as some other pulls will likely need updating.

@pieterlange
Copy link
Contributor

pieterlange commented Nov 25, 2016

I'm quite in favor of using yaml for the cloudformation template. I'm postponing my PR's so @mumoshu can more easily work on his big nodepool refactor (#46).

With regards to timing: If we can make the transition to YAML shortly after the nodepool feature it would be great.

@cknowles
Copy link
Contributor Author

I think it would cut it down quite significantly and may also potentially parse better in an editor. Happy to wait until the node pools are merged.

@mumoshu
Copy link
Contributor

mumoshu commented Nov 30, 2016

I prefer YAML!
But is everyone ok when JSON support is immediately dropped?
Is there anyone building your own tooling/script to run kube-aws render and/or kube-aws up --export, then tinker resulting stack-template.json or <clustername>.stack-template.json to match your requirements?
If so, we'd better support both JSON and YAML.

Let me be clear that I'm completely ok with transforming our "internal" representation of stack template template(currently config/templates/stack-template.json and nodepool/config/templates/stack-template.json) to YAML.
The point is what we provide to our users.

@cknowles
Copy link
Contributor Author

I'm doing the same but I tend to just use a diff tool and do I manually. The two are convertible automatically but it's a bit involved for kube-aws probably. Maybe we could support those cases via some sort of plugin system but I haven't thought it through entirely. For example kube-aws could provide some hooks for users to add their own pieces, e.g. They specify a CF snippet and we read it plus substitute config in and then merge with the stack definition.

@mumoshu
Copy link
Contributor

mumoshu commented Dec 1, 2016

Would you mind sharing your idea a bit deeper?

So:

  • Are you ok with stack-template.json(currently golang text/template + json) generated via kube-aws render completely becoming stack-template.yaml(golang text/template + yaml)?
    • This mean that users are forced to work with YAML templates since a certain release of kube-aws
  • Are you ok with kube-aws up --export to have a new option to specify which output format to be used: json or yaml?
    • This means that users are not forced to change their automation script which parses/updates json files automatically, if any
  • And after that, we can introduce hooks you've mentioned to help our users scripting around kube-aws?

@cknowles
Copy link
Contributor Author

cknowles commented Dec 1, 2016

Sure, to expand a bit more:

  1. All internal representations to change to YAML. One potentially interesting part of this is that it's possible to get the templates to parse correctly as both YAML and Go templates if the template code is inside YAML comments. Not sure if that's the best way but it's certainly nice to have it parse both ways.
  2. If we want to support both JSON and YAML export, I don't think it would be too tricky to write a simple converter and could include it in this. I've found at least one in Python which is circa 15 lines of code and we could replicate a similar thing.
  3. I think plugins would be a really powerful feature but a little hard to test properly. If we need to do 2 anyway then we can leave this out of scope for now.
  4. While a separate task, several of the template variables could live as CloudFormation parameters with defaults to allow users to easily change them from the console as well as the kube-aws way. A good example is the ASG min/max.

@mumoshu
Copy link
Contributor

mumoshu commented Dec 2, 2016

For 1., I'll be happy if templates become valid as both YAML and Go templates. I lack imagination for that, though! How would an ideal template which is both valid as YAML and Go template look like?

For 2., could you share the python script which does that?

For 4, I guess static values in the CloudFormation template could be converted in that ways! Indeed it would be a separate task but sounds useful anyways.

@cknowles
Copy link
Contributor Author

cknowles commented Dec 3, 2016

  1. I think it would be something like this in the template:
# {{ if .VpcId }} Custom VPC {{ .VpcId}} defined
# {{ else }} Default kube-aws VPC
vpc:
  name: {{ ... }}
# {{ end }}

When rendered without a VpcId defined, this should output something like:

# Default kube-aws VPC
vpc:
  name: stack-name-vpc
#

When rendered with a VpcId defined, it would be:

# Custom VPC vpc-uniqueID defined
#

Note, I've only experimented with this briefly so there's additional unneeded #s but perhaps those could be improved somehow. We may have to use the whitespace trim support in go templates. This also improves the inline documentation if we go this way or the output could strip all YAML comments which would tidy it up somewhat.

  1. Script is here, it's JSON to YAML but the same principle applies.

@iameli
Copy link
Contributor

iameli commented Dec 9, 2016

Related documentation here on how Helm handles whitespace management. Seems like they needed to do a few different things to properly template YAML files. Potentially y'all could use the same templating engine?

@sdouche
Copy link

sdouche commented Dec 9, 2016

@mumoshu @c-knowles FYI, this is a tool from AWS Labs for converting AWS CloudFormation templates between JSON and YAML formats:
https://github.com/awslabs/aws-cfn-template-flip

@icereval
Copy link
Contributor

icereval commented Jan 3, 2017

I have no qualm with the change, it would make template modifications easier.

Having significant PR changes on the templates I'm certainly willing to help w/ the refactor or update the PR's so they are not a blocker.

@redbaron
Copy link
Contributor

redbaron commented Jan 12, 2017

As mush as I like YAML, JSON works better when templated as there are no whitespaces hiding around to make life miserable :)

@icereval
Copy link
Contributor

For the discussion, I've written a couple helm charts and I do agree it takes a bit of getting used to/debugging to get right. e.g. https://github.com/kubernetes/charts/blob/master/stable/kube2iam/templates/daemonset.yaml#L41

@cknowles
Copy link
Contributor Author

@redbaron, I agree we'd have to be careful about that sort of thing as that's one of the drawbacks of YAML. My view is we aim to make it easier to use for the end user even if a contributor's job is slightly trickier. Ultimately we could probably support both without much additional effort. The advantages of better readability, reference anchors and comment embedding might outweigh whitespace issues, what do you think?

@icereval, that's interesting. I'm curious about the reason to have a template and then reference what seems to be a partial template in values.yaml? I can see a lot of other helm charts doing it so I'm wondering why the added complexity for four parameters? Side note - you've reminded me about helm for a new piece of work I'm currently doing, thanks!

@redbaron
Copy link
Contributor

@c-knowles , I was commenting as an end user :) We change what kube-aws renders before running up. Switch to YAML I expect to introduce more errors in the process :(

As for advantages, I don't see how reference anchors can be useful in CF template, and we already have comments thanks to go template.

@cknowles cknowles mentioned this issue Apr 8, 2017
@camilb
Copy link
Contributor

camilb commented Jan 16, 2018

@mumoshu @c-knowles Anyone noticed this project? https://github.com/awslabs/goformation

@mumoshu
Copy link
Contributor

mumoshu commented Jan 16, 2018

@camilb Thanks for sharing! I have been never aware of it.

Seemed like a promising project but.. at glance it looks like it doesn't "properly" support Refs?

https://github.com/awslabs/goformation#resolving-references-ref

For example I'd imagine Ref("AWS::AccountId") to be resolved as {"Ref":"AWS::AccountId"}, whereas the goformation resolves it to the default value of 123456789012.

@cknowles
Copy link
Contributor Author

cknowles commented May 5, 2018

Going to close this as stale.

@cknowles cknowles closed this as completed May 5, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

8 participants