Skip to content

Commit

Permalink
Fix jinja[invalid] false positive (#2465)
Browse files Browse the repository at this point in the history
Fixes: #2464
Fixes: #2462
Fixes: #2459
  • Loading branch information
ssbarnea committed Sep 20, 2022
1 parent c81a853 commit 5346f67
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 14 deletions.
1 change: 1 addition & 0 deletions .config/dictionary.txt
Expand Up @@ -71,6 +71,7 @@ hwcksum
idempotency
importlib
iniconfig
ipwrap
isclass
isdir
isdisjoint
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/tox.yml
Expand Up @@ -166,7 +166,7 @@ jobs:
WSLENV: FORCE_COLOR:PYTEST_REQPASS:TOXENV:TOX_PARALLEL_NO_SPINNER
# Number of expected test passes, safety measure for accidental skip of
# tests. Update value if you add/remove tests.
PYTEST_REQPASS: 697
PYTEST_REQPASS: 698

steps:
- name: Activate WSL1
Expand Down
4 changes: 3 additions & 1 deletion conftest.py
Expand Up @@ -19,9 +19,11 @@
)
sys.exit(1)

if not HAS_LIBYAML:
if not HAS_LIBYAML and sys.version_info >= (3, 9, 0):
# While presence of libyaml is not required for runtime, we keep this error
# fatal here in order to be sure that we spot libyaml errors during testing.
#
# For 3.8.x we do not do this check, as libyaml does not have an arm64 build for py38.
print(
"FATAL: For testing, we require pyyaml to be installed with its native extension, missing it would make testing 3x slower and risk missing essential bugs.",
file=sys.stderr,
Expand Down
4 changes: 2 additions & 2 deletions examples/playbooks/rule-jinja-invalid.yml
Expand Up @@ -2,8 +2,8 @@
- name: Fixture
hosts: localhost
tasks:
- name: Foo
- name: Foo # <-- this is valid jinja
ansible.builtin.debug:
msg: "{{ {{ '1' }} }}" # <-- jinja2[invalid]
msg: "{{ 'a' b }}" # <-- jinja2[invalid]
# It should be noted that even ansible --syntax-check fails to spot the jinja
# error above, but ansible will throw a runtime error when running
22 changes: 22 additions & 0 deletions examples/playbooks/rule-jinja-valid.yml
@@ -0,0 +1,22 @@
---
# https://github.com/ansible/ansible-lint/issues/2464
# https://github.com/ansible/ansible-lint/issues/2462
# https://github.com/ansible/ansible-lint/issues/2459
- name: Fixture to test various jinja parsing bugs that we should ignore
hosts: localhost
tasks:
- name: Foo {{ buildset_registry.host | ipwrap }}
ansible.builtin.debug:
msg: "{{ lookup('template', 'lookup/redis_server__env_ports.j2') | from_yaml }}"
loop: "{{ github_release_query.results | subelements('json.assets', {'skip_missing': True}) }}"
- name: Zoo
ansible.builtin.debug:
msg: "{{ lookup('ansible.builtin.ini', 'SOME_VAR', type='properties', file='/tmp/some-file') }}"

- name: Generate Dovecot main configuration file
ansible.builtin.template:
src: '{{ lookup("template_src", "etc/dovecot/dovecot.conf.j2") }}'
dest: "/etc/dovecot/dovecot.conf"
owner: "root"
group: "dovecot"
mode: "0640"
80 changes: 70 additions & 10 deletions src/ansiblelint/rules/jinja.py
Expand Up @@ -2,15 +2,16 @@
from __future__ import annotations

import logging
import re
import sys
from collections import namedtuple
from typing import TYPE_CHECKING, Any

import black
import jinja2
from ansible.errors import AnsibleError
from ansible.errors import AnsibleError, AnsibleFilterError, AnsibleParserError
from ansible.parsing.yaml.objects import AnsibleUnicode
from yaml.representer import RepresenterError
from jinja2.exceptions import TemplateSyntaxError

from ansiblelint.constants import LINE_NUMBER_KEY
from ansiblelint.file_utils import Lintable
Expand All @@ -36,6 +37,9 @@ class JinjaRule(AnsibleLintRule):
severity = "LOW"
tags = ["formatting"]
version_added = "v6.5.0"
_ansible_error_re = re.compile(
r"^(?P<error>.*): (?P<detail>.*)\. String: (?P<string>.*)$", flags=re.MULTILINE
)

env = jinja2.Environment(trim_blocks=False)
_tag2msg = {
Expand All @@ -47,7 +51,8 @@ def _msg(self, tag: str, value: str, reformatted: str) -> str:
"""Generate error message."""
return self._tag2msg[tag].format(value=value, reformatted=reformatted)

def matchtask(
# pylint: disable=too-many-branches
def matchtask( # noqa: C901
self, task: dict[str, Any], file: Lintable | None = None
) -> bool | str | MatchError:
for key, v, _ in nested_items_path(task):
Expand All @@ -58,15 +63,62 @@ def matchtask(
basedir=file.dir if file else ".",
value=v,
variables={},
fail_on_error=True,
fail_on_error=True, # we later decide which ones to ignore or not
)
except (AnsibleError, ValueError, RepresenterError) as exc:
return self.create_matcherror(
message=str(exc),
linenumber=task[LINE_NUMBER_KEY],
filename=file,
tag=f"{self.id}[invalid]",
# ValueError RepresenterError
except AnsibleError as exc:
bypass = False
orig_exc = exc.orig_exc if getattr(exc, "orig_exc", None) else exc
match = self._ansible_error_re.match(
getattr(orig_exc, "message", str(orig_exc))
)
if (
isinstance(exc, AnsibleFilterError)
or "unable to locate collection" in orig_exc.message
):
bypass = True
elif re.match(
r"^the template file (.*) could not be found for the lookup$",
orig_exc.message,
) or re.match(r"could not locate file in lookup", orig_exc.message):
bypass = True
elif isinstance(orig_exc, AnsibleParserError):
# "An unhandled exception occurred while running the lookup plugin '...'. Error was a <class 'ansible.errors.AnsibleParserError'>, original message: Invalid filename: 'None'. Invalid filename: 'None'"

# An unhandled exception occurred while running the lookup plugin 'template'. Error was a <class 'ansible.errors.AnsibleError'>, original message: the template file ... could not be found for the lookup. the template file ... could not be found for the lookup

# ansible@devel (2.14) new behavior:
# AnsibleError(TemplateSyntaxError): template error while templating string: Could not load "ipwrap": 'Invalid plugin FQCN (ansible.netcommon.ipwrap): unable to locate collection ansible.netcommon'. String: Foo {{ buildset_registry.host | ipwrap }}. Could not load "ipwrap": 'Invalid plugin FQCN (ansible.netcommon.ipwrap): unable to locate collection ansible.netcommon'
bypass = True
elif (
isinstance(orig_exc, (AnsibleError, TemplateSyntaxError))
and match
):
error = match.group("error")
detail = match.group("detail")
# string = match.group("string")
if error.startswith("template error while templating string"):
bypass = False
elif detail.startswith("unable to locate collection"):
_logger.debug("Ignored AnsibleError: %s", exc)
bypass = True
else:
# breakpoint()
bypass = False
elif re.match(r"^lookup plugin (.*) not found$", exc.message):
# lookup plugin 'template' not found
bypass = True

# AnsibleFilterError: 'obj must be a list of dicts or a nested dict'
# AnsibleError: template error while templating string: expected token ':', got '}'. String: {{ {{ '1' }} }}
# AnsibleError: template error while templating string: unable to locate collection ansible.netcommon. String: Foo {{ buildset_registry.host | ipwrap }}
if not bypass:
return self.create_matcherror(
message=str(exc),
linenumber=task[LINE_NUMBER_KEY],
filename=file,
tag=f"{self.id}[invalid]",
)

reformatted, details, tag = self.check_whitespace(v, key=key)
if reformatted != v:
Expand Down Expand Up @@ -532,3 +584,11 @@ def test_jinja_invalid() -> None:
assert len(errs) == 1
assert errs[0].tag == "jinja[invalid]"
assert errs[0].rule.id == "jinja"

def test_jinja_valid() -> None:
"""Tests our ability to parse jinja, even when variables may not be defined."""
collection = RulesCollection()
collection.register(JinjaRule())
success = "examples/playbooks/rule-jinja-valid.yml"
errs = Runner(success, rules=collection).run()
assert len(errs) == 0

0 comments on commit 5346f67

Please sign in to comment.