-
Notifications
You must be signed in to change notification settings - Fork 654
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
support extra_vars in syntax check rule #1342
Changes from all commits
811ab73
06d44a6
b37f0cc
4d9208f
e421f20
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,7 @@ | ||
- hosts: localhost | ||
gather_facts: false | ||
tags: | ||
- "{{ foo }}" | ||
tasks: | ||
- name: Run custom module | ||
fake_module: {} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
--- | ||
- hosts: all | ||
tags: | ||
- baz | ||
- "{{ foo }}" | ||
tasks: | ||
- name: Show `complex_variable` value loaded from `extra_vars` | ||
ansible.builtin.debug: | ||
msg: "{{ complex_variable }}" |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,10 +5,12 @@ | |
|
||
pytest_plugins = ['ansiblelint.testing'] | ||
""" | ||
import copy | ||
import os | ||
|
||
import pytest | ||
|
||
from ansiblelint.config import options # noqa: F401 | ||
from ansiblelint.constants import DEFAULT_RULESDIR | ||
from ansiblelint.rules import RulesCollection | ||
from ansiblelint.runner import Runner | ||
|
@@ -50,6 +52,15 @@ def rule_runner(request): | |
return RunFromText(collection) | ||
|
||
|
||
@pytest.fixture | ||
def config_options(): | ||
"""Return configuration options that will be restored after testrun.""" | ||
global options # pylint: disable=global-statement | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am not happy with this fixture, probably the better idea will be to create some There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that django approach is considerably better than what we have but if you want to do this, please do it as a separated pull-request after we sort the rest. |
||
original_options = copy.deepcopy(options) | ||
yield options | ||
options = original_options | ||
|
||
|
||
@pytest.fixture | ||
def _play_files(tmp_path, request): | ||
if request.param is None: | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will likely fail to work well for more complex vars, like one containing shell escapable vars, multilines and so.
IMHO, I would instead dump these vars inside
.cache/extra_vars.yml
and pass@.cache/extra_vars.yml
to ansible, so it will load them from there.Be sure to add some problematic var example to the list and see if it works. I really do not want to get bug reports about linter failing to load var files, when this is the job of ansible itself.
Maybe we should only support mentioning the extra_vars_filename inside config and force users that want this feature to add another file with them, so we can rely on ansible to process the file.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see why this should fail for more complex variables. The shell is not involved in interpreting this (see
shell=False
below), this is passed directly as-is to ansible-playbook.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will be glad to see it as working as is, lets put something like this into the example, which also works as test-file: