Consider supporting command args with JSON Array

[justenwalker]

All commands in the app.json are just strings. Currently, Containerbuddy splits these strings on space.
This could be fragile if the command requires an argument to have a space.

This leaves three options:

1. Spaces in arguments are not supported - rewrite your app please
2. Make the arg splitter more intelligent (i.e. support quoting/escaping arguments)
3. Allow the user to enter a JSON array of arguments

Proposal

Supporting a JSON array of arguments makes practical sense:

1. Option 1 makes the application less usable
2. Option 2 can be complicated to implement, and may introduce command parsing bugs
3. JSON Arrays are easy enough to add, and it uses the JSON syntax for argument splitting, which is already well-defined and implemented.

Commands & arguments

All executable fields, such as onStart and onChange, accept both a string or an array. If a string is given, the command and its arguments are separated by spaces; otherwise, the first element of the array is the command path, and the rest are its arguments.

String Command

"health": "/usr/bin/curl --fail -s http://localhost/app"

Array Command

"health": [
  "/usr/bin/curl",
  "--fail",
  "-s",
  "http://localhost/app"
]

[tgross]

Both forms can be supported at the same time:

I like where you're going with the overall concept here but I think supporting two different field names is inelegant. I'd love if we could do something like what the Docker folks did with the CMD and ENTRYPOINT Dockerfile stanzas, where they were able to parse both "/usr/bin/my/health" and ["/usr/bin/my/health", "--with", "some", "-args"].

The typical way to do this would be to deserialize the JSON into map[string][]json.RawMessage, parse the pieces we need, and then complete serialization of the nested ServiceConfig and BackendConfig structs (ref http://mattyjwilliams.blogspot.com/2013/01/using-go-to-unmarshal-json-lists-with.html, in the "option C" for an example of how this could be done). This means the code that parses the configuration will be less efficient but honestly we only parse it once so if it's mildly hairy that's fine as long as it's well-tested.

If we weren't going to do that, then I'd say we just break backwards compat (this is still marked alpha) and use a JSON array that will sometimes be a one-argument array. But if we can make the former work that'd be ideal.

[tgross]

I should probably point out that we'll probably need to do pre-processing via json.RawMessage to support #16 anyways, so it might be a good idea to get the scaffolding in place to support it sooner rather than later.

[justenwalker]

@tgross I agree. Originally I wanted to do something like you described (supporting both arrays and strings) but I wasn't sure how to. Thanks for the reference. I'll revisit this tonight and see if I come up with something.

[justenwalker]

@tgross I've implemented the enhancement as suggested. I'm not sure how you want to handle #16 though - since it kind of seems like the implementation strategy is still up in the air.

Also, it seemed like it would be good to support the string/array for interfaces as well - so if you only want to provide 1 interface, it doesn't need to be wrapped in an array.

[tgross]

I'm not sure how you want to handle #16 though - since it kind of seems like the implementation strategy is still up in the air.

Let's not worry about that for now; what you have here provides the places to hook-in for whomever is going to implement that nicely.

[tgross]

This has been merged and will be in the next release (presumably 0.0.5). Thanks @justenwalker!

[tgross]

Released in https://github.com/joyent/containerbuddy/releases/tag/0.0.5