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

Add parseYAML helper #1344

Merged
merged 1 commit into from
Apr 24, 2020
Merged

Add parseYAML helper #1344

merged 1 commit into from
Apr 24, 2020

Conversation

fernferret
Copy link
Contributor

Why

Consul-template supports parseJSON. The consul ui supports syntax highlighting for both JSON and YAML so it'd be ncie if consul-template had first-class support for both reading and writing yaml.

What

This adds a new helper parseYAML that works identically to parseJSON but accepts YAML.

YAML is much nicer to use for config files than JSON mainly because
comments can be used.

It should be noted that parseYAML | toYAML will strip comments with
the current yaml library, but I don't want to go about changing
that huge of a dependency at this stage. yaml.v3 does support comments,
but it's not super easy to use as you can't just unmarshal to a
map[string]interface{} and keep comments.

Other details

To work around this, I'd made a plugin yaml2json which was just a tiny go program but then I have to carry that around in my $PATH and I have to use the following snippet which feels a bit meh given how nicely the parseJSON parser works.

{{ with $d := key "user/info.yaml" | plugin "yaml2json" | parseJSON }}
{{ $d.name }}
{{ end }}

This fixes #1343

parseYAML works identically to parseJSON but accepts YAML.

YAML is much nicer to use for config files than JSON mainly because
comments can be used.

It should be noted that `parseYAML | toYAML` will strip comments with
the current yaml library, but I don't want to go about changing
that huge of a dependency at this stage.
@hashicorp-cla
Copy link

hashicorp-cla commented Feb 23, 2020

CLA assistant check
All committers have signed the CLA.

@fernferret
Copy link
Contributor Author

Hmm looking at the build logs, vault tried to spin up but took a little while to migrate and thus the secret push failed.

Can someone with perms try re-running the pipeline? I don't believe this change should have failed anything and I did add several tests for my new function which passed with flying colors in my test area.

@fernferret
Copy link
Contributor Author

fernferret commented Feb 23, 2020

Ah looks like I was lucky enough to trigger the test race condition :)

Ref: #1329 (comment) and https://app.circleci.com/jobs/github/hashicorp/consul-template/278

@eikenb
Copy link
Contributor

eikenb commented Apr 17, 2020

Hey @fernferret, I'm starting a review of PRs for the next release. I haven't got to this one yet but I did kick the CI run and it is now passing.

Copy link
Contributor

@eikenb eikenb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@eikenb eikenb added this to the 0.25.0 milestone Apr 21, 2020
@eikenb eikenb merged commit dc4dbf0 into hashicorp:master Apr 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add the ability to call parseYAML
3 participants