From 9a9bf377025650140810ba425944c19ae80eca7c Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Wed, 26 Apr 2023 08:58:34 +0200 Subject: [PATCH] feat: add role for deprecated loop bare vars (#586) --- ansiblelater/rules/CheckChangedInWhen.py | 1 + ansiblelater/rules/CheckDeprecatedBareVars.py | 87 +++++++++++++++++++ .../rules/CheckFilePermissionOctal.py | 1 + ansiblelater/rules/CheckGitHasVersion.py | 1 + ansiblelater/utils/__init__.py | 13 +++ docs/content/included_rules/_index.md | 1 + 6 files changed, 104 insertions(+) create mode 100644 ansiblelater/rules/CheckDeprecatedBareVars.py diff --git a/ansiblelater/rules/CheckChangedInWhen.py b/ansiblelater/rules/CheckChangedInWhen.py index 3b55c84..09e63fd 100644 --- a/ansiblelater/rules/CheckChangedInWhen.py +++ b/ansiblelater/rules/CheckChangedInWhen.py @@ -17,6 +17,7 @@ # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN # THE SOFTWARE. + from ansiblelater.standard import StandardBase diff --git a/ansiblelater/rules/CheckDeprecatedBareVars.py b/ansiblelater/rules/CheckDeprecatedBareVars.py new file mode 100644 index 0000000..f3049e8 --- /dev/null +++ b/ansiblelater/rules/CheckDeprecatedBareVars.py @@ -0,0 +1,87 @@ +# Copyright (c) 2013-2014 Will Thames +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +# THE SOFTWARE. + +import os + +from ansiblelater.standard import StandardBase +from ansiblelater.utils import has_glob, has_jinja + + +class CheckDeprecatedBareVars(StandardBase): + + sid = "ANSIBLE0027" + description = "Deprecated bare variables in loops must not be used" + helptext = ( + "bare var '{barevar}' in '{loop_type}' must use full var syntax ('{{{{ {barevar} }}}}') " + "or be converted to a list" + ) + version = "0.3" + types = ["playbook", "task", "handler"] + + def check(self, candidate, settings): + tasks, self.errors = self.get_normalized_tasks(candidate, settings) + + if not self.errors: + for task in tasks: + loop_type = next((key for key in task if key.startswith("with_")), None) + + if loop_type in [ + "with_items", + "with_nested", + "with_together", + "with_flattened", + "with_filetree", + "with_community.general.filetree", + ]: + # These loops can either take a list defined directly in the task + # or a variable that is a list itself. When a single variable is used + # we just need to check that one variable, and not iterate over it like + # it's a list. Otherwise, loop through and check all items. + items = task[loop_type] + + if not isinstance(items, (list, tuple)): + items = [items] + for var in items: + self._matchvar(var, task, loop_type) + elif loop_type == "with_subelements": + self._matchvar(task[loop_type][0], task, loop_type) + elif loop_type in ["with_sequence", "with_ini", "with_inventory_hostnames"]: + pass + else: + self._matchvar(task[loop_type], task, loop_type) + + return self.Result(candidate.path, self.errors) + + def _matchvar(self, varstring, task, loop_type): + if isinstance(varstring, str) and not has_jinja(varstring): + valid = loop_type == "with_fileglob" and bool( + has_jinja(varstring) or has_glob(varstring), + ) + + valid |= loop_type == "with_filetree" and bool( + has_jinja(varstring) or varstring.endswith(os.sep), + ) + if not valid: + self.errors.append( + self.Error( + task["__line__"], + self.helptext.format(barevar=task[loop_type], loop_type=loop_type) + ) + ) diff --git a/ansiblelater/rules/CheckFilePermissionOctal.py b/ansiblelater/rules/CheckFilePermissionOctal.py index 1e25c4e..61f9738 100644 --- a/ansiblelater/rules/CheckFilePermissionOctal.py +++ b/ansiblelater/rules/CheckFilePermissionOctal.py @@ -17,6 +17,7 @@ # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN # THE SOFTWARE. + from ansiblelater.standard import StandardBase diff --git a/ansiblelater/rules/CheckGitHasVersion.py b/ansiblelater/rules/CheckGitHasVersion.py index 75273c2..bdd45c2 100644 --- a/ansiblelater/rules/CheckGitHasVersion.py +++ b/ansiblelater/rules/CheckGitHasVersion.py @@ -17,6 +17,7 @@ # LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, # OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN # THE SOFTWARE. + from ansiblelater.standard import StandardBase diff --git a/ansiblelater/utils/__init__.py b/ansiblelater/utils/__init__.py index 4cdc337..dac16f7 100644 --- a/ansiblelater/utils/__init__.py +++ b/ansiblelater/utils/__init__.py @@ -1,6 +1,7 @@ """Global utils collection.""" import contextlib +import re import sys from contextlib import suppress from distutils.version import LooseVersion @@ -89,6 +90,18 @@ def add_dict_branch(tree, vector, value): return tree +def has_jinja(value): + """Return true if a string seems to contain jinja templating.""" + re_has_jinja = re.compile(r"{[{%#].*[%#}]}", re.DOTALL) + return bool(isinstance(value, str) and re_has_jinja.search(value)) + + +def has_glob(value): + """Return true if a string looks like having a glob pattern.""" + re_has_glob = re.compile("[][*?]") + return bool(isinstance(value, str) and re_has_glob.search(value)) + + def sysexit(code=1): sys.exit(code) diff --git a/docs/content/included_rules/_index.md b/docs/content/included_rules/_index.md index ffaf110..a7cc1a9 100644 --- a/docs/content/included_rules/_index.md +++ b/docs/content/included_rules/_index.md @@ -41,5 +41,6 @@ Reviews are useless without some rules or standards to check against. ansible-la | CheckLocalAction | ANSIBLE0024 | Don't use local_action. | | | CheckRelativeRolePaths | ANSIBLE0025 | Don't use a relative path in a role. | | | CheckChangedInWhen | ANSIBLE0026 | Use handlers instead of `when: changed`. | | +| CheckChangedInWhen | ANSIBLE0027 | Deprecated bare variables in loops must not be used. | | | CheckVersion | ANSIBLE9998 | Standards version should be pinned. | | | CheckDeprecated | ANSIBLE9999 | Deprecated features of `ansible-later` should not be used. | |