Skip to content
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

Template file is json and hence doesnt have support for comments. #283

Closed
rluiten opened this issue Aug 14, 2013 · 44 comments
Closed

Template file is json and hence doesnt have support for comments. #283

rluiten opened this issue Aug 14, 2013 · 44 comments

Comments

@rluiten
Copy link

rluiten commented Aug 14, 2013

Comments are something I already miss.
We discussed briefly on IRC and you suggested I raise issue.
You also suggested maybe can add support for rifled "comment": for some comment support.

I am in windows 7 64 bit, using packer v0.3.1

@mitchellh
Copy link
Contributor

Thanks for opening this issue. It'll at least hopefully allow Google to pick it up. For now, however, I think I'm not going to pursue this. Closing. Thanks

@jtopper
Copy link
Contributor

jtopper commented Sep 25, 2013

That's a shame - provisioning via AWS is full of magic values like AMI and security group IDs, and it'd be nice to be able to clarify these inline, though I can't immediately see a nice way to do this in vanilla JSON and supporting config file pre-processing feels like the wrong approach too.

@jasongilman
Copy link

Agreed. There needs to be some kind of workaround at least. This feels like an issue which has to be fixed. I understand that JSON doesn't support comments but then it feels like it's the wrong choice for a config file.

@dpehrson
Copy link

I just wanted to throw my two cents in here as well. Been using packer for the past week and absolutely love it, however as I have gotten further and further into making packer mirror existing technology, the need for commenting my template has grown, mostly because I have many builders with lots of "only" provisioners.

An alternative to comments would be some sort of way of splitting packer configs into multiple files that can be included in each other.

@mwhooker
Copy link
Contributor

As with most things JSON, I find it necessary and with great value to generate it using some scripting language. That provides a much better way to structure, comment, and reuse parts of it.

On Nov 24, 2013, at 13:30, Daniel Pehrson notifications@github.com wrote:

I just wanted to throw my two cents in here as well. Been using packer for the past week and absolutely love it, however as I have gotten further and further into making packer mirror existing technology, the need for commenting my template has grown, mostly because I have many builders with lots of "only" provisioners.

An alternative to comments would be some sort of way of splitting packer configs into multiple files that can be included in each other.


Reply to this email directly or view it on GitHub.

@dpehrson
Copy link

@mwhooker That’s the technique I use with CloudFormation templates, I build them in a DSL of some sort and then serialize down to JSON. While it works well, it introduces another layer of processing which adds another step to the process.

I mostly wanted to see if there was any intent to address this issue directly before I go build another piece of machinery.

@mwhooker
Copy link
Contributor

That's the exact use case I was thinking of. See https://github.com/SimpleFinance/agi

That's the problem with json. Tries to be both human and machine readable but doesn't do either very well. (A worthy successor to XML!). At least it's simple

Plaese forgive typos -- sent via phone

On Nov 25, 2013, at 5:14, Daniel Pehrson notifications@github.com wrote:

@mwhooker That’s the technique I use with CloudFormation templates, I build them in a DSL of some sort and then serialize down to JSON. While it works well, it introduces another layer of processing which adds another step to the process.

I mostly wanted to see if there was any intent to address this issue directly before I go build another piece of machinery.


Reply to this email directly or view it on GitHub.

@mitchellh
Copy link
Contributor

I've mentioned before, I am interested in perhaps adding a meta-key such as "comment" that is ignored. But I'm still thinking about how to do this best.

@jvoorhis
Copy link
Contributor

This is a long shot, but would you be open to pull requests for alternate formats like YAML, TOML or .ini?

@dpehrson
Copy link

@mitchellh I was actually thinking that you could just allow any object node to have a comment property, such as:

{
  "builders": [
    {
      "comment": "The VMWare box we use for local development.",
      "type": "vmware",
      ...
    }
  ],
  "provisioners": [
    {
      "comment": "This provisioner is used to ensure the machine has a home .ssh directory before uploading keys.",
      "type": "shell",
      ...
    }
  ],
  "post-processors": [
    {
      "comment": "Only machines used for local development are vagranted.",
      "type": "vagrant",
      ...
    }
  ]
}

Additionally, it might not be a terrible idea to support an ignore property on any object node which has the effect of "commenting out" the item, such as:

{
  "builders": [
    {
      "comment": "We're not currently building this machine for some unknown reason.",
      "ignore": true,
      "type": "vmware",
      ...
    },
    ...
  ]
}

@jvoorhis
Copy link
Contributor

The per-node comment is a nice idea, but it prevents comments on particular attributes from being placed within close proximity to the attribute they are describing.

@dpehrson
Copy link

@jvoorhis I think to a certain extent that as long as we are limited by JSON, it's probably the best we can do.

At a higher level, while I appreciate the simplicity of JSON, I believe it makes for a very poor configuration format for anything even marginally complex, which in my opinion packer is, and then some. In the long term, as packer is more widely used and is doing more and more, an alternative to JSON config will be needed whether it is provided by packer, or a third party.

@MaggieLeber
Copy link

Well, I see I'm not the only person struggling with this issue...starting with my very first template I'd hoped I could simply put comment values in and have them be ignored, but even that fails validation. A value namespace that could be used for unprocessed comment annotations would be most welcome.

Crockford's response to people who want comments to be valid JSON is

https://plus.google.com/+DouglasCrockfordEsq/posts/RK8qyGVaGSr

Jerk.

@rclilly
Copy link

rclilly commented Jan 26, 2014

Why is this issue closed? It's a need that still needs a solution. I vote for either using something other than JSON for configuration or doing something along the lines of what @dpehrson suggested in #283 (comment)

@kalefranz
Copy link

+1 for suggesting indexed by google, and
+1 for having some type of comment capability like @dpehrson's comment and ignore properties or @mwhooker's pre-process filtering

Given the Vagrantfile and Dockerfile precedents, is a non-json Packerfile so out of the question?

@MaggieLeber
Copy link

Here's my solution for now; other folks using Scala and Play may find it handy:

https://gist.github.com/MaggieLeber/8953905

@rasa
Copy link
Collaborator

rasa commented Feb 12, 2014

How about having the validator ignore any tag that starts (or ends, or both) with an underscore? That would allow extreme flexibility:

{
  "builders": [
    {
      "type": "vmware",
      "_comment1": "The VMWare box we use for local development.",
      "_comment2": "The VMWare box we use for local development.",
      ...
    }
  ],
  "provisioners": [
    {
      "sort_me_1st_comment_": "This provisioner is used to ensure the machine has a home .ssh               directory before uploading keys.",
      "sort_me_2nd_comment_": "",
      "type": "shell",
      ...
    }
  ],
  "post-processors": [
    {
      "_comment": "Only machines used for local development are vagranted.",
      "type": "vagrant",
      ...
    }
  ]
}

@themartorana
Copy link

I've been following this issue. The Universal Configuration Language library (libucl) has been gaining some traction, and seems like an interesting and possibly useful solution to retain JSON output for config files while enabling the features people want here.

https://github.com/vstakhov/libucl

@heartpunk
Copy link

👍 for YAML. Might be able to spend some work time on a pull request to make that happen, too. It solves this, and another problem, I have with using JSON, which is that I can't break long string literals into multiple lines.

@ccope
Copy link

ccope commented Apr 29, 2014

I just ran into this problem while creating a Solaris template. There are some odd things that really should be documented in comments (like where to get the iso, since you can't automatically download it).

@timc3
Copy link

timc3 commented May 27, 2014

I think just a prefix on the key would be good enough, and then packer ignores that key value pair.

@sparkprime
Copy link
Contributor

If you just want JSON with comments, you can preprocess your JSON with Jsonnet: http://google.github.io/jsonnet/doc/

This also gives you a whole lot of other language features that you may or may not want.

The same goes for Cloud Formation templates.

@sparkprime
Copy link
Contributor

Jsonnet allows you to break string literals onto multiple lines, but you can also use the importstr construct to read the string from a file, which is a lot more appropriate for things like embedding bash scripts / config files.

@nchammas
Copy link
Contributor

+1

As a Packer n00b, I often find myself wanting to comment out/in blocks of a template during testing and development.

As a user, not being able to do this hurts.

@nchammas
Copy link
Contributor

@MaggieLeber pointed to Douglas Crockford's comments on what to do if you want to comment JSON, and I think the suggestion is actually potentially useful:

Suppose you are using JSON to keep configuration files, which you would like to annotate. Go ahead and insert all the comments you like. Then pipe it through JSMin before handing it to your JSON parser.

Instead of coming up with a home-brew implementation of JSON comments or switching to a new config format altogether, why not just take Crockford's suggestion and allow people to comment their JSON templates like they were JavaScript files?

Packer can just take the file as-is, pass it through JSMin (or something equivalent) internally, and then proceed as normal.

Wouldn't that be the easiest, lowest friction way to allow template comments? What are the drawbacks?

@heartpunk
Copy link

I kinda like @nchammas' suggestion, but also I would like to avoid another dependency, as there's already a ton of stuff for me to explain to coworkers to set up when they need to use packer with our internal tooling around it. I guess the homebrew formula (which we always use) could be updated to account for this, but I imagine it'll still be counter to what seems to be a design goal of no external dependencies.

@nchammas
Copy link
Contributor

Yeah, it would be best if we avoided having an external dependency. We also don't want to implement our own JavaScript minifier if possible; that would be annoying.

What if we found an existing JavaScript minifier with a permissive license that we could just include as part of Packer? Users wouldn't have to know about it as it would just be used internally by Packer.

For example, YUI Compressor (BSD license) and Go-Yui (MIT license) look like promising options. There are also several Python options with permissive licenses, some of which don't depend on YUI and therefore don't need Java installed.

Could we just package one of those with Packer, allowing Packer to strip JSON templates of comments, without requiring users to manually manage this dependency?

@mitchellh What do you think of this approach?

Other potential options include simply porting the original JSMin from C to Go (it's quite short), or asking Crockford if he is open to relicensing JSMin under a more permissive license evaluating whether the existing license (which seems quite permissive) allows us to package JSMin safely.

@dpehrson
Copy link

Another option is to just move away from JSON. It's appropriateness as a solution to configuration management scales very poorly past very basic needs.

It's a format for transferring data between computers, it's not really designed for human management.

@heartpunk
Copy link

I actually like JSON, at least as an option. I'm considering building a layer in front of packer that might do dynamic mixing and matching of provisioners/etc., and so a standard interchange format as an option is important.

@sparkprime
Copy link
Contributor

@tehgeekmeister I have been doing that with Jsonnet, I am planning to release something very soon and if you're interested I could prioritize that. It'd be great to consider more use-cases.

http://google.github.io/jsonnet/doc/

@sparkprime
Copy link
Contributor

In general this question is a difficult one. Even in the constraint of maintaining backwards compatibility with JSON there are many options. The most obvious extension of JSON is YAML it has been around quite a while and lots of people know it. I'm surprised by how many people do not realize that YAML is an extension of JSON. But regardless, you can write idiomatic JSON with comments by using YAML. Since packer is written by Mitchell Hashimoto, the most likely extension of JSON he would use is going to be HCL, which he wrote. UCL, which is very similar, is also an option. I would say YAML is the most powerful of those syntaxes because it does support referencing and a bunch of other things (that seem to not be used that much). But even YAML is not considered powerful enough, as evidenced by the number of systems that combine it (rather inelegantly in my opinion) with Jinja. For this reason I would be wary of adding anything to packer itself, because you then never be able to take it away again. And if you decide to go in HCL direction, you cannot change your mind later and go YAML (at least without having multiple front-ends). Another problem with directly integrating a language is it would have to be bindable from Go, preferably without a native bridge.

More to the point, you don't even need to do that. I assume most people run packer from a makefile, because then you can build multiple images in parallel and chain it into your build process. With that in mind, you can run your own pre-preprocessor / generator in front of it and that works very well. You don't even need to be backwards-compatible with JSON if you go in that direction. There are Python-ish options like Coil and Pystachio. Personally I would recommend taking a look at Jsonnet though :)

@nchammas
Copy link
Contributor

Here is a work-around I am using during development to allow comments in Packer templates. All it requires is that you have JSMin or something similar installed. On OS X with Homebrew this is as simple as:

brew install jsmin

Then, as Crockford suggests:

packer validate <(jsmin < packer-template.json)

That's it. This allows comments in your templates. For example:

<snipped>

  "provisioners": [
    {
      "type": "shell",
      "execute_command": "chmod +x {{ .Path }}; {{ .Vars }} sudo -E sh -x '{{ .Path }}'",
      "scripts": [
        "./tools-setup.sh",
        "./python-setup.sh"    // Python 3 doesn't work, so we're stuck with 2.7
        // "./python3-setup.sh"
      ]
      // "pause_before": "10s"
    },

<snipped>

@nchammas
Copy link
Contributor

After discussing this with my buddy @LumberJ, we came up with another potential approach that does not require modifying core.

We can develop a Packer command plugin that introduces a couple of new commands, say x-validate and x-build (x- for extended), that pre-process the template using JSMin before handling it as usual.

For the actual minification, there is apparently already a Go port of JSMin out there that we could use.

What do people think of this approach? If it takes off, I guess down the road it could be incorporated into Packer core.

@rickard-von-essen
Copy link
Collaborator

@nchammas before you do that see #1629.

@rickard-von-essen
Copy link
Collaborator

@nchammas I suggest use Ruby or something to convert Yaml to json and write your config in Yaml instead to have a more human friendly template format.

@sparkprime
Copy link
Contributor

Note that the following is valid YAML:

{
  "provisioners": [
    {
      "type": "shell",
      "execute_command": "chmod +x {{ .Path }}; {{ .Vars }} sudo -E sh -x '{{ .Path }}'",
      "scripts": [
        "./tools-setup.sh",
        "./python-setup.sh"    # Python 3 doesn't work, so we're stuck with 2.7
        # "./python3-setup.sh"
      ]
      # "pause_before": "10s"
    }
  ]
}

So using YAML does not necessarily mean using a meaningful whitespace syntax.

@nchammas
Copy link
Contributor

nchammas commented Nov 3, 2014

Thanks for the heads up @rickard-von-essen.

@delitescere delitescere mentioned this issue Dec 16, 2014
@nchammas
Copy link
Contributor

To add to the laundry list of suggestions here, there is also JSON5, a simple extension to JSON that allows for comments as well as other tweaks that make JSON more human-friendly.

From the project website:

JSON5 is a proposed extension to JSON that aims to make it easier for humans to write and maintain by hand. It does this by adding some minimal syntax features directly from ECMAScript 5.

Some of the interesting features:

  • Object keys can be unquoted if they're valid identifiers.
  • Objects and arrays can have trailing commas.
  • Both inline (single-line) and block (multi-line) comments are allowed.

@sethvargo You mentioned HCL in #1768.

Has it been decided that that is what Packer is moving to for configuration?

@wolfhechel
Copy link

I would argue that this is something that should not be put on packer. Pre-processing is just another step that could easily be managed by some other script or tool prior to inputting it to packer.

If anyone needs a simple way to feed template with // comments into packer, here's my approach.

sed -e 's@//.*@@g' build.json | packer build -

@mfischer-zd
Copy link

The simplest solution seems like the best solution to me: ignore any keys that start with _.

@jcoutch
Copy link

jcoutch commented Apr 30, 2015

The simplest solution would be just to pick something and implement it...rather than dragging on this discussion for 2 years. Put an underscore in, implement an enabled flag, implement HCL/YAML/etc...just pick something!

@sparkprime
Copy link
Contributor

I think the value is actually kinda low because a lot of people use Packer as part of a larger toolchain. By the time your configs are large enough to need comments, you're probably generating the JSON anyway. And even if you're not in that situation, you can use whatever language you like, as long as you're prepared to write a 10 line Python wrapper.

@knope
Copy link

knope commented Aug 25, 2015

comments. json needs them.

json sucks until it has them. something as simple as an enclosing \ \ or ## ## or ... be creative... would be awesome... testing and running and running and testing sucks when a line (key) can't be commented out and back in for science. . . .

+1 for spamming: https://tools.ietf.org/html/rfc7159 ; in order to get json comments...

@kidbrax
Copy link

kidbrax commented May 3, 2017

people obviously want YAML

@hashicorp hashicorp locked and limited conversation to collaborators May 3, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.