From cfd5a95ae6b2179df29c4285effa5d58e56665fd Mon Sep 17 00:00:00 2001 From: Sorin Sbarnea Date: Sat, 26 Dec 2020 10:41:39 +0000 Subject: [PATCH] Avoid exception with YAML files which are just strings Fixes: #1082 --- .flake8 | 3 ++- a.yml | 1 + examples/plain_string.yml | 3 +++ lib/ansiblelint/_internal/rules.py | 11 +++++++++++ lib/ansiblelint/rules/LoadingFailureRule.py | 14 -------------- lib/ansiblelint/rules/__init__.py | 12 ++++++++++-- lib/ansiblelint/runner.py | 2 +- lib/ansiblelint/utils.py | 7 ++++++- test/TestExamples.py | 7 +++++++ 9 files changed, 41 insertions(+), 19 deletions(-) create mode 100644 a.yml create mode 100644 examples/plain_string.yml delete mode 100644 lib/ansiblelint/rules/LoadingFailureRule.py diff --git a/.flake8 b/.flake8 index 8e21121222d..42eee2f9e3b 100644 --- a/.flake8 +++ b/.flake8 @@ -55,8 +55,9 @@ per-file-ignores = lib/ansiblelint/__main__.py: C901 lib/ansiblelint/cli.py: D101 D102 D103 lib/ansiblelint/formatters/__init__.py: D101 D102 - lib/ansiblelint/utils.py: D103 + lib/ansiblelint/utils.py: D103 C901 lib/ansiblelint/rules/*.py: D100 D101 D102 + lib/ansiblelint/rules/__init__.py: D100 D101 D102 C901 # FIXME: drop these once they're fixed # Ref: https://github.com/ansible-community/ansible-lint/issues/725 diff --git a/a.yml b/a.yml new file mode 100644 index 00000000000..257cc5642cb --- /dev/null +++ b/a.yml @@ -0,0 +1 @@ +foo diff --git a/examples/plain_string.yml b/examples/plain_string.yml new file mode 100644 index 00000000000..fc1c31a1367 --- /dev/null +++ b/examples/plain_string.yml @@ -0,0 +1,3 @@ +foo +# This file is valid YAML but from our point of view is an error, as is +# neither a Sequence or a Mapping. diff --git a/lib/ansiblelint/_internal/rules.py b/lib/ansiblelint/_internal/rules.py index 8640be1fbb5..85dc073bbd4 100644 --- a/lib/ansiblelint/_internal/rules.py +++ b/lib/ansiblelint/_internal/rules.py @@ -62,3 +62,14 @@ class AnsibleParserErrorRule(BaseRule): severity = 'VERY_HIGH' tags = ['core'] version_added = 'v5.0.0' + + +class LoadingFailureRule(BaseRule): + """File loading failure.""" + + id = '901' + shortdesc = 'Failed to load or parse file' + description = 'Linter failed to process a YAML file, possible not an Ansible file.' + severity = 'VERY_HIGH' + tags = ['core'] + version_added = 'v4.3.0' diff --git a/lib/ansiblelint/rules/LoadingFailureRule.py b/lib/ansiblelint/rules/LoadingFailureRule.py deleted file mode 100644 index 7c37498f05e..00000000000 --- a/lib/ansiblelint/rules/LoadingFailureRule.py +++ /dev/null @@ -1,14 +0,0 @@ -"""Rule definition for a failure to load a file.""" - -from ansiblelint.rules import AnsibleLintRule - - -class LoadingFailureRule(AnsibleLintRule): - """File loading failure.""" - - id = '901' - shortdesc = 'Failed to load or parse file' - description = 'Linter failed to process a YAML file, possible not an Ansible file.' - severity = 'VERY_HIGH' - tags = ['core'] - version_added = 'v4.3.0' diff --git a/lib/ansiblelint/rules/__init__.py b/lib/ansiblelint/rules/__init__.py index 06ea6500866..36a2c6d4ba8 100644 --- a/lib/ansiblelint/rules/__init__.py +++ b/lib/ansiblelint/rules/__init__.py @@ -10,7 +10,7 @@ from typing import List import ansiblelint.utils -from ansiblelint._internal.rules import AnsibleParserErrorRule +from ansiblelint._internal.rules import AnsibleParserErrorRule, LoadingFailureRule from ansiblelint.errors import BaseRule, MatchError, RuntimeErrorRule from ansiblelint.skip_utils import append_skipped_rules, get_rule_skips_from_line @@ -129,6 +129,14 @@ def matchyaml(self, file: dict, text: str) -> List[MatchError]: return matches yaml = ansiblelint.utils.parse_yaml_linenumbers(text, file['path']) + # yaml returned can be an AnsibleUnicode (a string) when the yaml + # file contains a single string. YAML spec allows this but we consider + # this an fatal error. + if 'get' not in dir(yaml): + return [MatchError( + filename=file['path'], + rule=LoadingFailureRule() + )] if not yaml: return matches @@ -190,7 +198,7 @@ def __init__(self, rulesdirs=None) -> None: # internal rules included in order to expose them for docs as they are # not directly loaded by our rule loader. self.rules.extend( - [RuntimeErrorRule(), AnsibleParserErrorRule()]) + [RuntimeErrorRule(), AnsibleParserErrorRule(), LoadingFailureRule()]) for rulesdir in self.rulesdirs: _logger.debug("Loading rules from %s", rulesdir) self.extend(load_plugins(rulesdir)) diff --git a/lib/ansiblelint/runner.py b/lib/ansiblelint/runner.py index 5328f651bbb..1e42b31f4a0 100644 --- a/lib/ansiblelint/runner.py +++ b/lib/ansiblelint/runner.py @@ -7,8 +7,8 @@ import ansiblelint.file_utils import ansiblelint.skip_utils import ansiblelint.utils +from ansiblelint._internal.rules import LoadingFailureRule from ansiblelint.errors import MatchError -from ansiblelint.rules.LoadingFailureRule import LoadingFailureRule if TYPE_CHECKING: from ansiblelint.rules import RulesCollection diff --git a/lib/ansiblelint/utils.py b/lib/ansiblelint/utils.py index 3c5aebe3424..875e736c01d 100644 --- a/lib/ansiblelint/utils.py +++ b/lib/ansiblelint/utils.py @@ -56,7 +56,7 @@ from yaml.composer import Composer from yaml.representer import RepresenterError -from ansiblelint._internal.rules import AnsibleParserErrorRule +from ansiblelint._internal.rules import AnsibleParserErrorRule, LoadingFailureRule from ansiblelint.constants import CUSTOM_RULESDIR_ENVVAR, DEFAULT_RULESDIR, FileType from ansiblelint.errors import MatchError from ansiblelint.file_utils import normpath @@ -187,6 +187,11 @@ def find_children(playbook: Tuple[str, str], playbook_dir: str) -> List: raise SystemExit(str(e)) results = [] basedir = os.path.dirname(playbook[0]) + # playbook_ds can be an AnsibleUnicode string, which we consider invalid + if "get" not in playbook_ds: + raise MatchError( + filename=playbook[0], + rule=LoadingFailureRule) items = _playbook_items(playbook_ds) for item in items: for child in _rebind_match_filename(playbook[0], play_children)( diff --git a/test/TestExamples.py b/test/TestExamples.py index ef7c75f2327..185a823f148 100644 --- a/test/TestExamples.py +++ b/test/TestExamples.py @@ -6,3 +6,10 @@ def test_example(default_rules_collection): """example.yml is expected to have 15 match errors inside.""" result = Runner(default_rules_collection, 'examples/example.yml', [], [], []).run() assert len(result) == 15 + + +def test_example_plain_string(default_rules_collection): + """Validates that loading valid YAML string produce error.""" + result = Runner(default_rules_collection, 'examples/plain_string.yml', [], [], []).run() + assert len(result) == 1 + assert "Failed to load or parse file" in result[0]