Consul Template 0.26.0 issues with command substitution

[evandam]

After upgrading from 0.25.1 -> 0.26.0, commands are no longer parsed the same way. Commands that use command substitution like sh -c 'echo $(which consul)' fail. I noticed escaping the $ with \\$ works, but as far as I can tell this was an undocumented breaking change.

Consul Template version

v0.26.0 (3b7f233)

Configuration

{
  "template": {
    "source": "/tmp/test.tmpl",
    "destination": "/tmp/test.out",
    "command": "sh -c 'echo $(which consul)'"
  }
}

(note the template is arbitrary here, it's the command hook that's the issue)

{{range services}}# {{.Name}}{{range service .Name}}
{{.Address}}{{end}}

{{end}}

Command

consul-template -once -config test.json --log-level trace

Debug output

https://gist.github.com/evandam/9f9aeaa444cc9761086b6ee09b073583

Expected behavior

Command should process sh -c 'echo $(which consul)' as expected, returning something like /usr/local/bin/consul

Actual behavior

2021-06-14T23:35:29.670Z [INFO] (runner) executing command "sh -c 'echo $(which consul)'" from "/root/foo.tmpl" => "/root/foo.out"
2021-06-14T23:35:29.670Z [INFO] (child) spawning: sh -c echo (which consul)which consul)
sh: 1: Syntax error: word unexpected (expecting ")")
2021-06-14T23:35:29.672Z [ERR] (cli) 1 error occurred:
	* failed to execute command "sh -c 'echo $(which consul)'" from "/root/foo.tmpl" => "/root/foo.out": child: command exited with a non-zero exit status:

    sh -c echo (which consul)which consul)

This is assumed to be a failure. Please ensure the command
exits with a zero exit status.

Steps to reproduce

1. Create template file /tmp/foo.tmpl
2. Create config file test.json
3. consul-template -once -config test.json

[eikenb]

Hey @evandam, thanks for filing the report.

The shell parsing library we used had a bug (reported in #1456 and #1463, updated in #1477) that was addressed in the last release, but it seems like that fix probably introduced this issue as it was fixing a different issue in sub-shell calling.

I've reproduced this and it is a break in the upstream handling. I've added a test and will see what I can do to get it fixed and merged upstream.

[eikenb]

Just pushed an upstream PR that fixes this... and hopefully doesn't break anything else.

mattn/go-shellwords#50

Once accepted (in current form or some other after upstream feedback) I'll open a local PR to update the dependency.

[eikenb]

Note that I'm planning on a pretty fast turnaround on 0.26.1. I had already planned a 0.26.1 for the Docker image updates in the next week or so. Thanks again.

[evandam]

Sounds good, thanks a ton for jumping on this so quickly @eikenb!

[eikenb]

I was out for a few days due to the heat here but am back and while reviewing current pending things I re-checked the upstream PR and realized it had issues (probably why it hadn't been merged).

I've withdrawn that PR and am re-thinking it. Sorry for the delay, I'll update here again once I have a new PR ready (or equiv).

[eikenb]

I started basically re-writing the quote handling code in go-shellwords, but that is going to be quite the task and so I'm considering my alternatives.

The primary option I'm thinking of at the moment is just ditching all shell parsing and passing the provided commands directly to an (OS appropriate) /bin/sh -c 'your-command-here'. This just passes the parsing buck to the shell itself and mainly relies on having a system shell to work with.

Thoughts on this approach?

edit: more likely something like /bin/sh -c "your-command-here_with-double-quotes-escaped" so that you can have single-quotes in your command.

[eikenb]

Was asking around with the shell experts here and one came up with this gem that I'm going to try...

awk -v cmd='your-command-here' 'BEGIN { while( (cmd | getline) == 1 ) {print $0}; close(cmd) }'

This has the advantage of eliminating the need to escape the command string. I haven't had a chance to try it yet, and I plan on adding a lot more tests around it (going to dig through old issues for test cases).

Working on an emergency issue right now, but will be back to this soon (probably next week).

[evandam]

Hey @eikenb no worries, I'm still happily on 0.25.1 that doesn't have any of these problems 😅

I think the important thing from my perspective is I don't want to rely on changing the commands across templates while upgrading, worrying about backwards compatibility, etc.

My commands are also already wrapped in a sh -c '...' so I'm not sure if your first suggestion will work?

Going to awk feels a bit complicated, from a usability perspective I'd be hesitant to switch over.

[eikenb]

Thanks for replying @evandam. I'm glad 0.25.1 is working for you and that this isn't a blocker. Makes me feel less bad about the delays.

Having 'sh -c' embedded in another 'sh -c' has no real effect other than the overhead of the extra shell call (which is very minor here). So that shouldn't matter.

I'm not 100% on the awk bit and it may be that handling the escaping in Go will be simple and the better way to go. It is just a nice alternative and benefits from the multiple decades of use (it is well vetted). We'll see how things play out.

Thanks again for the feedback.