support extra_vars in syntax check rule #1342
Conversation
| @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.
I am not happy with this fixture, probably the better idea will be to create some ConfigWrapper like for instance pytest-django do in the settings fixture (reference).
Do you have any other ideas for a more elegant solution?
There was a problem hiding this comment.
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.
.ansible-lint
Outdated
|
|
||
| # Define required Ansible's variables to satisfy syntax check | ||
| extra_vars: | ||
| - foo: bar |
There was a problem hiding this comment.
This is not how vars are defined in ansible, you mush use a dictionary, not a list of dictionaries for variables, somethng like
extra_vars:
foo: bar
zoo: moo
| cmd = ['ansible-playbook', '--syntax-check', str(lintable.path)] | ||
| extra_vars_cmd = [] | ||
| if options.extra_vars: | ||
| extra_vars_cmd = ['--extra-vars', json.dumps(options.extra_vars)] |
There was a problem hiding this comment.
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.
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.
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:
extra_vars:
# sample values used for testing
foo: |
line1
line2
bar: ":{;\t$()"
Adds ability to define a set of extra vars to be passed to ansible when we call
--syntax-check, so we can avoid failures caused by them.Closes: #1299