-
Notifications
You must be signed in to change notification settings - Fork 654
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Add rules for verifying the existence of imported and included files
PR #691 by @jlusiardi This new rule checks if the files referenced in import and include actually exist and emits an error if not. This can be switched off with a noqa comment (e.g. on files that depend on a variable). Signed-off-by: Joachim Lusiardi <joachim@lusiardi.de> Signed-off-by: Sviatoslav Sydorenko <webknjaz@redhat.com> Co-authored-by: Joachim Lusiardi <joachim@lusiardi.de> Co-authored-by: Sviatoslav Sydorenko <webknjaz@redhat.com>
- Loading branch information
Showing
4 changed files
with
156 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,66 @@ | ||
| # Copyright (c) 2020, Joachim Lusiardi | ||
| # Copyright (c) 2020, Ansible Project | ||
|
|
||
| from __future__ import print_function | ||
|
|
||
| import os.path | ||
|
|
||
| import ansible.parsing.yaml.objects | ||
|
|
||
| from ansiblelint import AnsibleLintRule | ||
|
|
||
|
|
||
| class IncludeMissingFileRule(AnsibleLintRule): | ||
| id = '505' | ||
| shortdesc = 'referenced files must exist' | ||
| description = ( | ||
| 'All files referenced by by include / import tasks ' | ||
| 'must exist. The check excludes files with jinja2 ' | ||
| 'templates in the filename.' | ||
| ) | ||
| severity = 'MEDIUM' | ||
| tags = ['task', 'bug'] | ||
| version_added = 'v4.3.0' | ||
|
|
||
| def matchplay(self, file, data): | ||
| absolute_directory = file.get('absolute_directory', None) | ||
| results = [] | ||
| for task in data.get('tasks', []): | ||
|
|
||
| # check if the id of the current rule is not in list of skipped rules for this play | ||
| if self.id in task['skipped_rules']: | ||
| continue | ||
|
|
||
| # collect information which file was referenced for include / import | ||
| referenced_file = None | ||
| for key, val in task.items(): | ||
| if not (key.startswith('include_') or | ||
| key.startswith('import_') or | ||
| key == 'include'): | ||
| continue | ||
| if isinstance(val, ansible.parsing.yaml.objects.AnsibleMapping): | ||
| referenced_file = val.get('file', None) | ||
| else: | ||
| referenced_file = val | ||
| # take the file and skip the remaining keys | ||
| if referenced_file: | ||
| break | ||
|
|
||
| if referenced_file is None: | ||
| continue | ||
|
|
||
| # make sure we have a absolute path here and check if it is a file | ||
| referenced_file = os.path.join(absolute_directory, referenced_file) | ||
|
|
||
| # skip if this is a jinja2 templated reference | ||
| if '{{' in referenced_file: | ||
| continue | ||
|
|
||
| # existing files do not produce any error | ||
| if os.path.isfile(referenced_file): | ||
| continue | ||
|
|
||
| results.append(({'referenced_file': referenced_file}, | ||
| 'referenced missing file in %s:%i' | ||
| % (task['__file__'], task['__line__']))) | ||
| return results |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,84 @@ | ||
| from collections import namedtuple | ||
| import os | ||
|
|
||
| import pytest | ||
|
|
||
| from ansiblelint import Runner, RulesCollection | ||
|
|
||
|
|
||
| PlayFile = namedtuple('PlayFile', ['name', 'content']) | ||
|
|
||
|
|
||
| PLAY_INCLUDING_PLAIN = PlayFile('playbook.yml', u''' | ||
| - hosts: all | ||
| tasks: | ||
| - include: some_file.yml | ||
| ''') | ||
|
|
||
| PLAY_INCLUDING_JINJA2 = PlayFile('playbook.yml', u''' | ||
| - hosts: all | ||
| tasks: | ||
| - include: "{{ some_path }}/some_file.yml" | ||
| ''') | ||
|
|
||
| PLAY_INCLUDING_NOQA = PlayFile('playbook.yml', u''' | ||
| - hosts: all | ||
| tasks: | ||
| - include: some_file.yml # noqa 505 | ||
| ''') | ||
|
|
||
| PLAY_INCLUDED = PlayFile('some_file.yml', u''' | ||
| - debug: | ||
| msg: 'was found & included' | ||
| ''') | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def play_file_path(tmp_path): | ||
| p = tmp_path / 'playbook.yml' | ||
| return str(p) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def rules(): | ||
| rulesdir = os.path.join('lib', 'ansiblelint', 'rules') | ||
| return RulesCollection([rulesdir]) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def runner(play_file_path, rules): | ||
| return Runner(rules, play_file_path, [], [], []) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def play_files(tmp_path, request): | ||
| if request.param is None: | ||
| return | ||
| for play_file in request.param: | ||
| p = tmp_path / play_file.name | ||
| p.write_text(play_file.content) | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| 'play_files', [pytest.param([PLAY_INCLUDING_PLAIN], id='referenced file missing')], | ||
| indirect=['play_files'] | ||
| ) | ||
| def test_include_file_missing(runner, play_files): | ||
| results = str(runner.run()) | ||
| assert 'referenced missing file in' in results | ||
| assert 'playbook.yml' in results | ||
| assert 'some_file.yml' in results | ||
|
|
||
|
|
||
| @pytest.mark.parametrize( | ||
| 'play_files', | ||
| [ | ||
| pytest.param([PLAY_INCLUDING_PLAIN, PLAY_INCLUDED], id='File Exists'), | ||
| pytest.param([PLAY_INCLUDING_JINJA2], id='JINJA2 in reference'), | ||
| pytest.param([PLAY_INCLUDING_NOQA], id='NOQA was used') | ||
| ], | ||
| indirect=['play_files'] | ||
| ) | ||
| def test_cases_that_do_not_report(runner, play_files): | ||
| results = str(runner.run()) | ||
| assert 'referenced missing file in' not in results |