From 2b6676636b70113446ffbd4b813009cac280b4e2 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Tue, 19 Jan 2021 22:58:47 +0100 Subject: [PATCH] feat: add rule check_command_instead_of_argument --- ansiblelater/data/standards.py | 16 ++++++- ansiblelater/rules/ansiblefiles.py | 62 ++++++++++++++++++++++++--- ansiblelater/utils/rulehelper.py | 11 +++++ docs/content/included_rules/_index.md | 55 ++++++++++++------------ 4 files changed, 108 insertions(+), 36 deletions(-) diff --git a/ansiblelater/data/standards.py b/ansiblelater/data/standards.py index 6d3b59d..e91364b 100644 --- a/ansiblelater/data/standards.py +++ b/ansiblelater/data/standards.py @@ -4,6 +4,7 @@ from ansiblelater.rules.ansiblefiles import check_become_user from ansiblelater.rules.ansiblefiles import check_braces_spaces from ansiblelater.rules.ansiblefiles import check_command_has_changes from ansiblelater.rules.ansiblefiles import check_command_instead_of_module +from ansiblelater.rules.ansiblefiles import check_command_instead_of_argument from ansiblelater.rules.ansiblefiles import check_compare_to_literal_bool from ansiblelater.rules.ansiblefiles import check_empty_string_compare from ansiblelater.rules.ansiblefiles import check_filter_separation @@ -178,7 +179,7 @@ literal_bool_should_be_formatted = Standard( use_become_with_become_user = Standard( dict( id="ANSIBLE0015", - name="become should be combined with become_user", + name="Become should be combined with become_user", check=check_become_user, version="0.1", types=["playbook", "task", "handler"] @@ -188,13 +189,23 @@ use_become_with_become_user = Standard( use_spaces_around_filters = Standard( dict( id="ANSIBLE0016", - name="jinja2 filters should be separated with spaces", + name="Jinja2 filters should be separated with spaces", check=check_filter_separation, version="0.1", types=["playbook", "task", "handler", "rolevars", "hostvars", "groupvars"] ) ) +commands_should_not_be_used_in_place_of_argument = Standard( + dict( + id="ANSIBLE0017", + name="Commands should not be used in place of module arguments", + check=check_command_instead_of_argument, + version="0.2", + types=["playbook", "task", "handler"] + ) +) + files_should_not_contain_unnecessarily_empty_lines = Standard( dict( id="LINT0001", @@ -306,6 +317,7 @@ standards = [ literal_bool_should_be_formatted, use_become_with_become_user, use_spaces_around_filters, + commands_should_not_be_used_in_place_of_argument, deprecated_features, # Lint files_should_not_contain_unnecessarily_empty_lines, diff --git a/ansiblelater/rules/ansiblefiles.py b/ansiblelater/rules/ansiblefiles.py index 233547e..1673465 100644 --- a/ansiblelater/rules/ansiblefiles.py +++ b/ansiblelater/rules/ansiblefiles.py @@ -10,6 +10,7 @@ from ansiblelater.command.candidates import Template from ansiblelater.utils import count_spaces from ansiblelater.utils.rulehelper import get_normalized_tasks from ansiblelater.utils.rulehelper import get_normalized_yaml +from ansiblelater.utils.rulehelper import get_first_cmd_arg def check_braces_spaces(candidate, settings): @@ -120,13 +121,7 @@ def check_command_instead_of_module(candidate, settings): if not errors: for task in tasks: if task["action"]["__ansible_module__"] in commands: - if "cmd" in task["action"]: - first_cmd_arg = task["action"]["cmd"].split()[0] - elif "argv" in task["action"]: - first_cmd_arg = task["action"]["argv"][0] - else: - first_cmd_arg = task["action"]["__ansible_arguments__"][0] - + first_cmd_arg = get_first_cmd_arg(task) executable = os.path.basename(first_cmd_arg) if ( first_cmd_arg and executable in modules and task["action"].get("warn", True) @@ -206,6 +201,59 @@ def check_command_has_changes(candidate, settings): return Result(candidate.path, errors) +def check_command_instead_of_argument(candidate, settings): + # 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. + + tasks, errors = get_normalized_tasks(candidate, settings) + commands = ["command", "shell", "raw"] + arguments = { + 'chown': 'owner', + 'chmod': 'mode', + 'chgrp': 'group', + 'ln': 'state=link', + 'mkdir': 'state=directory', + 'rmdir': 'state=absent', + 'rm': 'state=absent' + } + description = "{exec} used in place of file modules argument {arg}" + + if not errors: + for task in tasks: + if task["action"]["__ansible_module__"] in commands: + first_cmd_arg = get_first_cmd_arg(task) + executable = os.path.basename(first_cmd_arg) + + if ( + first_cmd_arg and executable in arguments and task["action"].get("warn", True) + ): + errors.append( + Error( + task["__line__"], + description.format(exec=executable, arg=arguments[executable]) + ) + ) + + return Result(candidate.path, errors) + + def check_empty_string_compare(candidate, settings): yamllines, errors = get_normalized_yaml(candidate, settings) description = "use `when: var` rather than `when: var != ""` (or " \ diff --git a/ansiblelater/utils/rulehelper.py b/ansiblelater/utils/rulehelper.py index c7df5da..23e1c7f 100644 --- a/ansiblelater/utils/rulehelper.py +++ b/ansiblelater/utils/rulehelper.py @@ -194,3 +194,14 @@ def run_yamllint(candidate, options="extends: default"): candidate.faulty = True return errors + + +def get_first_cmd_arg(task): + if "cmd" in task["action"]: + first_cmd_arg = task["action"]["cmd"].split()[0] + elif "argv" in task["action"]: + first_cmd_arg = task["action"]["argv"][0] + else: + first_cmd_arg = task["action"]["__ansible_arguments__"][0] + + return first_cmd_arg diff --git a/docs/content/included_rules/_index.md b/docs/content/included_rules/_index.md index ef4aed2..56f64d8 100644 --- a/docs/content/included_rules/_index.md +++ b/docs/content/included_rules/_index.md @@ -4,30 +4,31 @@ title: Included rules Reviews are nothing without some rules or standards against which to review. ansible-later comes with a couple of built-in checks explained in the following table. -| Rule | ID | Description | Parameter | -| ------------------------------- | ----------- | ----------------------------------------------------------------- | -------------------------------------------------------------------- | -| check_yaml_empty_lines | LINT0001 | YAML should not contain unnecessarily empty lines. | {max: 1, max-start: 0, max-end: 1} | -| check_yaml_indent | LINT0002 | YAML should be correctly indented. | {spaces: 2, check-multi-line-strings: false, indent-sequences: true} | -| check_yaml_hyphens | LINT0003 | YAML should use consitent number of spaces after hyphens (-). | {max-spaces-after: 1} | -| check_yaml_document_start | LINT0004 | YAML should contain document start marker. | {document-start: {present: true}} | -| check_yaml_colons | LINT0005 | YAML should use consitent number of spaces around colons. | {colons: {max-spaces-before: 0, max-spaces-after: 1}} | -| check_yaml_file | LINT0006 | Roles file should be in yaml format. | | -| check_yaml_has_content | LINT0007 | Files should contain useful content. | | -| check_native_yaml | LINT0008 | Use YAML format for tasks and handlers rather than key=value. | | -| check_yaml_document_end | LINT0009 | YAML should contain document end marker. | {document-end: {present: true}} | -| check_line_between_tasks | ANSIBLE0001 | Single tasks should be separated by an empty line. | | -| check_meta_main | ANSIBLE0002 | Meta file should contain a basic subset of parameters. | author, description, min_ansible_version, platforms, dependencies | -| check_unique_named_task | ANSIBLE0003 | Tasks and handlers must be uniquely named within a file. | | -| check_braces | ANSIBLE0004 | YAML should use consitent number of spaces around variables. | | -| check_scm_in_src | ANSIBLE0005 | Use scm key rather than src: scm+url in requirements file. | | -| check_named_task | ANSIBLE0006 | Tasks and handlers must be named. | excludes: meta, debug, include\_\*, import\_\*, block | -| check_name_format | ANSIBLE0007 | Name of tasks and handlers must be formatted. | formats: first letter capital | -| check_command_instead_of_module | ANSIBLE0008 | Commands should not be used in place of modules. | | -| check_install_use_latest | ANSIBLE0009 | Package managers should not install with state=latest. | | -| check_shell_instead_command | ANSIBLE0010 | Use Shell only when piping, redirecting or chaining commands. | | -| check_command_has_changes | ANSIBLE0011 | Commands should be idempotent and only used with some checks. | | -| check_empty_string_compare | ANSIBLE0012 | Don't compare to "" - use `when: var` or `when: not var`. | | -| check_compare_to_literal_bool | ANSIBLE0013 | Don't compare to True/False - use `when: var` or `when: not var`. | | -| check_literal_bool_format | ANSIBLE0014 | Literal bools should be written as `True/False` or `yes/no`. | forbidden values are `true false TRUE FALSE Yes No YES NO` | -| check_become_user | ANSIBLE0015 | `become` should be always used combined with `become_user`. | | -| check_filter_separation | ANSIBLE0016 | Jinja2 filters should be separated with spaces. | | +| Rule | ID | Description | Parameter | +| --------------------------------- | ----------- | ----------------------------------------------------------------- | -------------------------------------------------------------------- | +| check_yaml_empty_lines | LINT0001 | YAML should not contain unnecessarily empty lines. | {max: 1, max-start: 0, max-end: 1} | +| check_yaml_indent | LINT0002 | YAML should be correctly indented. | {spaces: 2, check-multi-line-strings: false, indent-sequences: true} | +| check_yaml_hyphens | LINT0003 | YAML should use consitent number of spaces after hyphens (-). | {max-spaces-after: 1} | +| check_yaml_document_start | LINT0004 | YAML should contain document start marker. | {document-start: {present: true}} | +| check_yaml_colons | LINT0005 | YAML should use consitent number of spaces around colons. | {colons: {max-spaces-before: 0, max-spaces-after: 1}} | +| check_yaml_file | LINT0006 | Roles file should be in yaml format. | | +| check_yaml_has_content | LINT0007 | Files should contain useful content. | | +| check_native_yaml | LINT0008 | Use YAML format for tasks and handlers rather than key=value. | | +| check_yaml_document_end | LINT0009 | YAML should contain document end marker. | {document-end: {present: true}} | +| check_line_between_tasks | ANSIBLE0001 | Single tasks should be separated by an empty line. | | +| check_meta_main | ANSIBLE0002 | Meta file should contain a basic subset of parameters. | author, description, min_ansible_version, platforms, dependencies | +| check_unique_named_task | ANSIBLE0003 | Tasks and handlers must be uniquely named within a file. | | +| check_braces | ANSIBLE0004 | YAML should use consitent number of spaces around variables. | | +| check_scm_in_src | ANSIBLE0005 | Use scm key rather than src: scm+url in requirements file. | | +| check_named_task | ANSIBLE0006 | Tasks and handlers must be named. | excludes: meta, debug, include\_\*, import\_\*, block | +| check_name_format | ANSIBLE0007 | Name of tasks and handlers must be formatted. | formats: first letter capital | +| check_command_instead_of_module | ANSIBLE0008 | Commands should not be used in place of modules. | | +| check_install_use_latest | ANSIBLE0009 | Package managers should not install with state=latest. | | +| check_shell_instead_command | ANSIBLE0010 | Use Shell only when piping, redirecting or chaining commands. | | +| check_command_has_changes | ANSIBLE0011 | Commands should be idempotent and only used with some checks. | | +| check_empty_string_compare | ANSIBLE0012 | Don't compare to "" - use `when: var` or `when: not var`. | | +| check_compare_to_literal_bool | ANSIBLE0013 | Don't compare to True/False - use `when: var` or `when: not var`. | | +| check_literal_bool_format | ANSIBLE0014 | Literal bools should be written as `True/False` or `yes/no`. | forbidden values are `true false TRUE FALSE Yes No YES NO` | +| check_become_user | ANSIBLE0015 | `become` should be always used combined with `become_user`. | | +| check_filter_separation | ANSIBLE0016 | Jinja2 filters should be separated with spaces. | | +| check_command_instead_of_argument | ANSIBLE0017 | Commands should not be used in place of module arguments. | |