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

parseJSON crashes consul-template on corrupt json data #1289

Closed
niko opened this issue Sep 26, 2019 · 4 comments
Closed

parseJSON crashes consul-template on corrupt json data #1289

niko opened this issue Sep 26, 2019 · 4 comments

Comments

@niko
Copy link

niko commented Sep 26, 2019

Consul Template v0.22.0 (005b42e)

Configuration

Just the default configuration.

The template:

{{- with $REDIRECTS := key "/redirects" | parseJSON }}{{ range $K, $V := $REDIRECTS }}
http-request redirect location http://{{ $V }}%[capture.req.uri] if { hdr(host) -i {{ $K }} }
{{- end }}{{ end }}

In the /redirects key:

{
  "foo": "bar",
  "baz": "bazza
}

Command

consul-template -retry 1s -consul-retry-attempts 10080 --template foo.ctmpl -dry

Debug output

https://gist.github.com/niko/8dac306eed91b673a3209831941e0c7b

Expected behavior

It should just log an error to STDout (or STDerr) and do nothing else. Don't write templates, don't restart any attached processes.

Actual behavior

It crashes when parseJSON encounters corrupt json.

IMHO it should not be possible to crash a running consul-template process not matter the data in consul changes. In containers consul-template is meant to be PID 1. It should never crash.

Steps to reproduce

  1. Insert valid json into a consul key.
  2. Run consul-template with a call to parseJSON on this key.
  3. Change the content of the key to be not valid json any more.

References

I added a comment here: #1165

I'm not sure that my issue and that are actually describing the same thing.

@eikenb
Copy link
Contributor

eikenb commented Mar 10, 2020

@niko @TwitchChen

Given the current architecture we're in an all or nothing situation when it comes to errors occurring during template execution. Any error in the template bubbles up to the top and consul-template exits.

I'm thinking of an option to change enable changing this behavior. When set it would change things so that template errors would get logged instead of exiting. This option could be per-template or global.

How does this sound?

@crypt1d
Copy link

crypt1d commented Aug 14, 2020

A config switch to prevent exits on errors would be very much appreciated.

In situations where consul-template, for example, handles nginx configs, there will be edge cases that prevent an immediate nginx reload (eg, due to connection issues). With the current design, this will cause consul-template to crash, making the the problem even worse because nginx config will slowly get out of sync, especially if upstream servers keep changing (eg, in a microservices setup of any kind).
Having the ability to tell consul-template not to crash due to such issues, and to simply try again in a few minutes, would help address this.

@eikenb
Copy link
Contributor

eikenb commented Aug 17, 2020

Thanks for chiming in @crypt1d.

Please 👍 the original issue as we use that to gauge peoples interest in an issue and help to prioritize things. Thanks.

@eikenb
Copy link
Contributor

eikenb commented Feb 3, 2022

With #1420 merged I believe this is resolved. Please let me know if you think it should be handled differently and re-opened.

@eikenb eikenb closed this as completed Feb 3, 2022
@eikenb eikenb added this to the v0.28.0 milestone Feb 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants