hashicorp / packer

Packer is a tool for creating identical machine images for multiple platforms from a single source configuration.
http://www.packer.io
Other
15.05k stars 3.32k forks source link

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

Closed rluiten closed 11 years ago

rluiten commented 11 years ago

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 commented 11 years ago

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 commented 11 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

@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 commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

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

dpehrson commented 10 years ago

@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 commented 10 years ago

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 commented 10 years ago

@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 commented 10 years ago

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 commented 10 years ago

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 https://github.com/mitchellh/packer/issues/283#issuecomment-29246466

kalefranz commented 10 years ago

+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 commented 10 years ago

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

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

rasa commented 10 years ago

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 commented 10 years ago

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 commented 10 years ago

:+1: 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 commented 10 years ago

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 commented 10 years ago

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

sparkprime commented 10 years ago

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 commented 10 years ago

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 commented 9 years ago

+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 commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

@nchammas before you do that see #1629.

rickard-von-essen commented 9 years ago

@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 commented 9 years ago

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 commented 9 years ago

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

nchammas commented 9 years ago

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:

@sethvargo You mentioned HCL in #1768.

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

wolfhechel commented 9 years ago

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 commented 9 years ago

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

jcoutch commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 7 years ago

people obviously want YAML