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

Correctly handle list items under nomad_plugins, fixes #127 #128

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bradleydwyer
Copy link

No description provided.

…se and to correctly quote with double-quotes rather than single.
@bradleydwyer bradleydwyer changed the title Correctly handle list items under nomad_plugins. Fixes #127 Correctly handle list items under nomad_plugins, fixes #127 May 18, 2021
@Rendanic
Copy link
Contributor

This fix is at a wrong place.

The following block creates the config for allow_caps

plugin "{{ key }}" {
{{ plugin_config(value) }}
}

My quick and dirty fix:
{{ plugin_config(value) | replace('\'', '\"') }}

The major problem is, that jinja2 cannot create HCL at this point.

@bradleydwyer
Copy link
Author

@Rendanic The primary issue that I was trying to fix was the casing rather than quoting. It was this casing issue that led me to put the change where I did, as that is where the lowercase operation takes place.

I'm not overly familiar with this project and I don't really mind where any potential fix is placed, just wanted to mention there is the second casing issue in addition to the quoting issue (as I mostly lent on using the example to describe the problem, rather than explaining the two different issues explicitly so it would have been easy to miss).

That said, I haven't actually tried your fix yet and it could be that it does avoid the lowercase operation. If so just ignore this comment.

@lanefu
Copy link
Contributor

lanefu commented Dec 27, 2021

would like some final vetting from someone if possible

@Rendanic
Copy link
Contributor

I created a PR (#152) with my fix.

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.

3 participants