Skip to content

Commit

Permalink
nested jinja fix
Browse files Browse the repository at this point in the history
* pattern matching logic
* rule was added to docs
* unit test modification
* related docs fix
  • Loading branch information
europ committed Jun 16, 2020
1 parent f5e97c9 commit 7ed5c23
Show file tree
Hide file tree
Showing 3 changed files with 159 additions and 129 deletions.
7 changes: 7 additions & 0 deletions docs/docsite/rst/rules/default_rules.rst
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,13 @@ Playbooks should have the ".yml" or ".yaml" extension

Variables should have spaces before and after: ``{{ var_name }}``

.. _207:

207: Nested jinja pattern
*************************

There should not be any nested jinja pattern. Example (bad): ``{{ list_one + {{ list_two | max }} }}``, example (good): ``{{ list_one + max(list_two) }}``

Command-Shell Rules (3xx)
-------------------------

Expand Down
20 changes: 12 additions & 8 deletions lib/ansiblelint/rules/NestedJinjaRule.py
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,23 @@ class NestedJinjaRule(AnsibleLintRule):
shortdesc = 'Nested jinja pattern'
description = (
"There should not be any nested jinja pattern. "
"Example (bad): '{{ list_one + {{ list_two | max }} }}', "
"example (good): '{{ list_one + max(list_two) }}'"
"Example (bad): ``{{ list_one + {{ list_two | max }} }}``, "
"example (good): ``{{ list_one + max(list_two) }}``"
)
severity = 'VERY_HIGH'
tags = ['formatting']
version_added = 'v4.3.0'

nested_jinja_pattern = re.compile(r"{{(?:[^{}]*)?{{")
pattern = re.compile(r"{{(?:[^{}]*)?{{")

def matchtask(self, file, task):
strings = (
val for key, val in task['action'].items()
if isinstance(val, str) and not key.startswith('__')
)

return any(self.nested_jinja_pattern.search(item) for item in strings)
command = "".join([
str(value)
# task properties are stored in the 'action' key
for key, value in task['action'].items()
# exclude useless values of '__file__', '__ansible_module__', '__*__', etc.
if not key.startswith('__') and not key.endswith('__')
])

return bool(self.pattern.search(command))
261 changes: 140 additions & 121 deletions test/TestNestedJinjaRule.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,128 +21,148 @@
# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
# THE SOFTWARE.

import os
import pytest
from collections import namedtuple
from ansiblelint import Runner, RulesCollection

import pytest

from ansiblelint.runner import Runner


PlayFile = namedtuple('PlayFile', ['name', 'content'])

FAIL_TASK_1LN = PlayFile('playbook.yml', u'''
- name: one-level nesting
set_fact:
var_one: "2*(1+2) is {{ 2 * {{ 1 + 2 }} }}"
''')

FAIL_TASK_1LN_M = PlayFile('playbook.yml', u'''
- name: one-level multiline nesting
set_fact:
var_one_ml: >
2*(1+2) is {{ 2 *
{{ 1 + 2 }}
}}
FAIL_TASK_1LN = PlayFile('playbook.yml', '''
- hosts: all
tasks:
- name: one-level nesting
set_fact:
var_one: "2*(1+2) is {{ 2 * {{ 1 + 2 }} }}"
''')

FAIL_TASK_2LN = PlayFile('playbook.yml', u'''
- name: two-level nesting
set_fact:
var_two: "2*(1+(3-1)) is {{ 2 * {{ 1 + {{ 3 - 1 }} }} }}"
FAIL_TASK_1LN_M = PlayFile('playbook.yml', '''
- hosts: all
tasks:
- name: one-level multiline nesting
set_fact:
var_one_ml: >
2*(1+2) is {{ 2 *
{{ 1 + 2 }}
}}
''')

FAIL_TASK_2LN_M = PlayFile('playbook.yml', u'''
- name: two-level multiline nesting
set_fact:
var_two_ml: >
2*(1+(3-1)) is {{ 2 *
{{ 1 +
{{ 3 - 1 }}
}} }}
FAIL_TASK_2LN = PlayFile('playbook.yml', '''
- hosts: all
tasks:
- name: two-level nesting
set_fact:
var_two: "2*(1+(3-1)) is {{ 2 * {{ 1 + {{ 3 - 1 }} }} }}"
''')

FAIL_TASK_W_5LN = PlayFile('playbook.yml', u'''
- name: five-level wild nesting
set_fact:
var_three_wld: "{{ {{ {{ {{ {{ 234 }} }} }} }} }}"
FAIL_TASK_2LN_M = PlayFile('playbook.yml', '''
- hosts: all
tasks:
- name: two-level multiline nesting
set_fact:
var_two_ml: >
2*(1+(3-1)) is {{ 2 *
{{ 1 +
{{ 3 - 1 }}
}} }}
''')

FAIL_TASK_W_5LN_M = PlayFile('playbook.yml', u'''
- name: five-level wild multiline nesting
set_fact:
var_three_wld_ml: >
{{
{{
{{
{{
{{ 234 }}
}}
}}
}}
}}
FAIL_TASK_W_5LN = PlayFile('playbook.yml', '''
- hosts: all
tasks:
- name: five-level wild nesting
set_fact:
var_three_wld: "{{ {{ {{ {{ {{ 234 }} }} }} }} }}"
''')

SUCCESS_TASK_P = PlayFile('playbook.yml', u'''
- name: proper non-nested example
set_fact:
var_one: "number for 'one' is {{ 2 * 1 }}"
FAIL_TASK_W_5LN_M = PlayFile('playbook.yml', '''
- hosts: all
tasks:
- name: five-level wild multiline nesting
set_fact:
var_three_wld_ml: >
{{
{{
{{
{{
{{ 234 }}
}}
}}
}}
}}
''')

SUCCESS_TASK_P_M = PlayFile('playbook.yml', u'''
- name: proper multiline non-nested example
set_fact:
var_one_ml: >
number for 'one' is {{
2 * 1 }}
SUCCESS_TASK_P = PlayFile('playbook.yml', '''
- hosts: all
tasks:
- name: non-nested example
set_fact:
var_one: "number for 'one' is {{ 2 * 1 }}"
''')

SUCCESS_TASK_2P = PlayFile('playbook.yml', u'''
- name: proper nesting far from each other
set_fact:
var_two: "number for 'two' is {{ 2 * 1 }} and number for 'three' is {{ 4 - 1 }}"
SUCCESS_TASK_P_M = PlayFile('playbook.yml', '''
- hosts: all
tasks:
- name: multiline non-nested example
set_fact:
var_one_ml: >
number for 'one' is {{
2 * 1 }}
''')

SUCCESS_TASK_2P_M = PlayFile('playbook.yml', u'''
- name: proper multiline nesting far from each other
set_fact:
var_two_ml: >
number for 'two' is {{ 2 * 1
}} and number for 'three' is {{
4 - 1 }}
SUCCESS_TASK_2P = PlayFile('playbook.yml', '''
- hosts: all
tasks:
- name: nesting far from each other
set_fact:
var_two: "number for 'two' is {{ 2 * 1 }} and number for 'three' is {{ 4 - 1 }}"
''')

SUCCESS_TASK_C_2P = PlayFile('playbook.yml', u'''
- name: proper nesting close to each other
set_fact:
var_three: "number for 'ten' is {{ 2 - 1 }}{{ 3 - 3 }}"
SUCCESS_TASK_2P_M = PlayFile('playbook.yml', '''
- hosts: all
tasks:
- name: multiline nesting far from each other
set_fact:
var_two_ml: >
number for 'two' is {{ 2 * 1
}} and number for 'three' is {{
4 - 1 }}
''')

SUCCESS_TASK_C_2P_M = PlayFile('playbook.yml', u'''
- name: proper multiline nesting close to each other
set_fact:
var_three_ml: >
number for 'ten' is {{
2 - 1
}}{{ 3 - 3 }}
SUCCESS_TASK_C_2P = PlayFile('playbook.yml', '''
- hosts: all
tasks:
- name: nesting close to each other
set_fact:
var_three: "number for 'ten' is {{ 2 - 1 }}{{ 3 - 3 }}"
''')

@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])
SUCCESS_TASK_C_2P_M = PlayFile('playbook.yml', '''
- hosts: all
tasks:
- name: multiline nesting close to each other
set_fact:
var_three_ml: >
number for 'ten' is {{
2 - 1
}}{{ 3 - 3 }}
''')


@pytest.fixture
def runner(play_file_path, rules):
return Runner(rules, play_file_path, [], [], [])
def runner(tmp_path, default_rules_collection):
return Runner(
default_rules_collection,
str(tmp_path / 'playbook.yml'),
[], [], [],
)


@pytest.fixture
def play_files(tmp_path, request):
def _playbook_file(tmp_path, request):
if request.param is None:
return
for play_file in request.param:
Expand All @@ -151,40 +171,39 @@ def play_files(tmp_path, request):


@pytest.mark.parametrize(
'play_files',
[pytest.param(
[
FAIL_TASK_1LN,
FAIL_TASK_1LN_M,
FAIL_TASK_2LN,
FAIL_TASK_2LN_M,
FAIL_TASK_W_5LN,
FAIL_TASK_W_5LN_M
],
id='file includes nested jinja pattern'
)],
indirect=['play_files']
'_playbook_file',
(
pytest.param([FAIL_TASK_1LN], id='file includes one-level nesting'),
pytest.param([FAIL_TASK_1LN_M], id='file includes one-level multiline nesting'),
pytest.param([FAIL_TASK_2LN], id='file includes two-level nesting'),
pytest.param([FAIL_TASK_2LN_M], id='file includes two-level multiline nesting'),
pytest.param([FAIL_TASK_W_5LN], id='file includes five-level wild nesting'),
pytest.param([FAIL_TASK_W_5LN_M], id='file includes five-level wild multiline nesting'),
),
indirect=['_playbook_file'],
)
def test_including_nested_jinja(runner, play_files):
results = str(runner.run())
assert 'nested jinja' in results
@pytest.mark.usefixtures('_playbook_file')
def test_including_wrong_nested_jinja(runner):
rule_violations = runner.run()
assert rule_violations[0].rule.id == '207'


@pytest.mark.parametrize(
'play_files',
[pytest.param(
[
SUCCESS_TASK_P,
SUCCESS_TASK_P_M,
SUCCESS_TASK_2P,
SUCCESS_TASK_2P_M,
SUCCESS_TASK_C_2P,
SUCCESS_TASK_C_2P_M
],
id='file does not include nested jinja pattern'
)],
indirect=['play_files']
'_playbook_file',
(
pytest.param([SUCCESS_TASK_P], id='file includes non-nested example'),
pytest.param([SUCCESS_TASK_P_M], id='file includes multiline non-nested example'),
pytest.param([SUCCESS_TASK_2P], id='file includes nesting far from each other'),
pytest.param([SUCCESS_TASK_2P_M], id='file includes multiline nesting far from each other'),
pytest.param([SUCCESS_TASK_C_2P], id='file includes nesting close to each other'),
pytest.param(
[SUCCESS_TASK_C_2P_M],
id='file includes multiline nesting close to each other',
),
),
indirect=['_playbook_file'],
)
def test_not_including_nested_jinja(runner, play_files):
results = str(runner.run())
assert 'nested jinja' not in results
@pytest.mark.usefixtures('_playbook_file')
def test_including_proper_nested_jinja(runner):
rule_violations = runner.run()
assert not rule_violations

0 comments on commit 7ed5c23

Please sign in to comment.