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

WIP: Allow certain, simple Jinja values to be unquoted. #36331

Open
wants to merge 3 commits into
base: devel
Choose a base branch
from

Conversation

ingydotnet
Copy link

@ingydotnet ingydotnet commented Feb 16, 2018

This allows the following to be unquoted:

foo: {{ bar }}

The above is valid YAML but semantically means something like this:

{ "foo": { { "bar": null}: null} }

This patch looks for events of that form coming from the parser, and
reconstructs the key bar into a string node of value "{{ bar }}".

SUMMARY

You can test this with the simple files here: https://gist.github.com/anonymous/48b8744dcafd8f6e6bdac3803e77aa4b

This is just a proof of concept. It only works for simple values that happen to be valid YAML:

- {{ foo }} # works
- {{ foo | bar }} # works
- {{ foo[42] }} # not valid yaml
- {{ 'foo' }} # would be same as {{ foo }} 
ISSUE TYPE
  • Feature Pull Request
COMPONENT NAME

YAML Loader

ANSIBLE VERSION

ADDITIONAL INFORMATION

This allows the following to be unquoted:
```
foo: {{ bar }}
```

The above is valid YAML but semantically means something like this:
```
{ "foo": { { "bar": null}: null} }
```

This patch looks for events of that form coming from the parser, and
reconstructs the key `bar` into a string node of value "{{ bar }}".
@ingydotnet ingydotnet changed the title Allow certain, simple jinja values to be unquoted. WIP: Allow certain, simple Jinja values to be unquoted. Feb 16, 2018
@ansibot ansibot added WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers. feature_pull_request needs_triage Needs a first human triage before being processed. new_contributor This PR is the first contribution by a new community member. support:core This issue/PR relates to code supported by the Ansible Engineering Team. labels Feb 16, 2018
@ansibot
Copy link
Contributor

ansibot commented Feb 16, 2018

The test ansible-test sanity --test pep8 [explain] failed with 2 errors:

lib/ansible/parsing/utils/yaml.py:53:13: E125 continuation line with same indent as next logical line
lib/ansible/parsing/utils/yaml.py:64:1: E302 expected 2 blank lines, found 1

click here for bot help

ingydotnet added 2 commits Feb 16, 2018
This works properly now:
```
unquoted: {{ foo }}
quoted: {{ 'foo' }}
```
The unquoted jinja feature only works when the unquoted value is valid
YAML.

Don't allow `foo: {{ 'bar' }}` when the internal value is a quoted
string. This form is dubious and probably never seen in the wild. We
just don't give it special treatment so it fails same as before.

Also added check to assert jinja key is a string.
@ingydotnet
Copy link
Author

ingydotnet commented Feb 18, 2018

I added a couple commits to solidify the logic.

The logic is:

  • If all of these are true:
    • YAML is valid
    • A node is a mapping with one pair
    • The pair's key is a mapping
    • The pair's value is null
    • The key mapping's key is an unquoted string
  • Then:
    • Load the node as that string with jinja double curlies around it
  • Else:
    • Use old/existing logic

The result is that certain unquoted Jinja values will work. If they don't work, they will simply fail in the same way they always did.

@ansibot
Copy link
Contributor

ansibot commented Feb 18, 2018

The test ansible-test sanity --test pep8 [explain] failed with 3 errors:

lib/ansible/parsing/utils/yaml.py:53:13: E125 continuation line with same indent as next logical line
lib/ansible/parsing/utils/yaml.py:60:17: E125 continuation line with same indent as next logical line
lib/ansible/parsing/utils/yaml.py:67:1: E302 expected 2 blank lines, found 1

click here for bot help

@sivel sivel removed the needs_triage Needs a first human triage before being processed. label Feb 19, 2018
@ansibot ansibot added stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. feature This issue/PR relates to a feature request. and removed feature_pull_request labels Feb 27, 2018
@ansibot ansibot added the affects_2.6 This issue/PR affects Ansible v2.6 label May 21, 2018
@gundalow gundalow added the candidate_to_close Think we can close this, though need to check with Core label Jan 11, 2019
@ansibot ansibot added pre_azp This PR was last tested before migration to Azure Pipelines. and removed stale_ci This PR has been tested by CI more than one week ago. Close and re-open this PR to get it retested. labels Dec 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affects_2.6 This issue/PR affects Ansible v2.6 candidate_to_close Think we can close this, though need to check with Core feature This issue/PR relates to a feature request. has_issue new_contributor This PR is the first contribution by a new community member. pre_azp This PR was last tested before migration to Azure Pipelines. support:core This issue/PR relates to code supported by the Ansible Engineering Team. WIP This issue/PR is a work in progress. Nevertheless it was shared for getting input from peers.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants