From 675bb6271e0dca31442d9c001abc45c86c44ccf4 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Sun, 5 Apr 2020 14:33:43 +0200 Subject: [PATCH 1/5] add yapf as formatter --- .drone.jsonnet | 4 +- ansiblelater/__init__.py | 4 +- ansiblelater/__main__.py | 41 ++- ansiblelater/command/base.py | 20 +- ansiblelater/command/candidates.py | 93 ++++-- ansiblelater/data/standards.py | 406 ++++++++++++++----------- ansiblelater/logger.py | 4 +- ansiblelater/rules/ansiblefiles.py | 70 +++-- ansiblelater/rules/yamlfiles.py | 24 +- ansiblelater/settings.py | 15 +- ansiblelater/standard.py | 6 +- ansiblelater/tests/config/standards.py | 378 ++++++++++++----------- ansiblelater/tests/unit/test_logger.py | 12 +- ansiblelater/utils/__init__.py | 10 +- ansiblelater/utils/rulehelper.py | 3 +- ansiblelater/utils/yamlhelper.py | 138 ++++++--- setup.cfg | 14 +- setup.py | 29 +- test-requirements.txt | 5 +- tox.ini | 2 +- 20 files changed, 725 insertions(+), 553 deletions(-) diff --git a/.drone.jsonnet b/.drone.jsonnet index a19701d..aafa9b6 100644 --- a/.drone.jsonnet +++ b/.drone.jsonnet @@ -59,9 +59,9 @@ local PipelineTest = { CODECOV_TOKEN: { from_secret: 'codecov_token' }, }, commands: [ - 'pip install codecov', + 'pip install codecov -qq', 'coverage combine .tox/py*/.coverage', - 'codecov --required', + 'codecov --required -X gcov', ], depends_on: [ 'python35-ansible', diff --git a/ansiblelater/__init__.py b/ansiblelater/__init__.py index 343c1df..1f64015 100644 --- a/ansiblelater/__init__.py +++ b/ansiblelater/__init__.py @@ -2,11 +2,11 @@ __author__ = "Robert Kaussow" __project__ = "ansible-later" -__version__ = "0.3.1" __license__ = "MIT" __maintainer__ = "Robert Kaussow" __email__ = "mail@geeklabor.de" -__status__ = "Production" +__url__ = "https://github.com/xoxys/ansible-later" +__version__ = "0.3.1" from ansiblelater import logger diff --git a/ansiblelater/__main__.py b/ansiblelater/__main__.py index 6b0289e..46b7f84 100755 --- a/ansiblelater/__main__.py +++ b/ansiblelater/__main__.py @@ -15,19 +15,34 @@ from ansiblelater.command import candidates def main(): """Run main program.""" parser = argparse.ArgumentParser( - description="Validate ansible files against best pratice guideline") - parser.add_argument("-c", "--config", dest="config_file", - help="location of configuration file") - parser.add_argument("-r", "--rules", dest="rules.standards", - help="location of standards rules") - parser.add_argument("-s", "--standards", dest="rules.filter", action="append", - help="limit standards to given ID's") - parser.add_argument("-x", "--exclude-standards", dest="rules.exclude_filter", action="append", - help="exclude standards by given ID's") - parser.add_argument("-v", dest="logging.level", action="append_const", const=-1, - help="increase log level") - parser.add_argument("-q", dest="logging.level", action="append_const", - const=1, help="decrease log level") + description="Validate ansible files against best pratice guideline" + ) + parser.add_argument( + "-c", "--config", dest="config_file", help="location of configuration file" + ) + parser.add_argument( + "-r", "--rules", dest="rules.standards", help="location of standards rules" + ) + parser.add_argument( + "-s", + "--standards", + dest="rules.filter", + action="append", + help="limit standards to given ID's" + ) + parser.add_argument( + "-x", + "--exclude-standards", + dest="rules.exclude_filter", + action="append", + help="exclude standards by given ID's" + ) + parser.add_argument( + "-v", dest="logging.level", action="append_const", const=-1, help="increase log level" + ) + parser.add_argument( + "-q", dest="logging.level", action="append_const", const=1, help="decrease log level" + ) parser.add_argument("rules.files", nargs="*") parser.add_argument("--version", action="version", version="%(prog)s {}".format(__version__)) diff --git a/ansiblelater/command/base.py b/ansiblelater/command/base.py index bc3bffa..589a151 100644 --- a/ansiblelater/command/base.py +++ b/ansiblelater/command/base.py @@ -20,9 +20,7 @@ def get_settings(args): :returns: Settings object """ - config = settings.Settings( - args=args, - ) + config = settings.Settings(args=args) return config @@ -33,13 +31,15 @@ def get_standards(filepath): standards = importlib.import_module("standards") except ImportError as e: utils.sysexit_with_message( - "Could not import standards from directory %s: %s" % (filepath, str(e))) + "Could not import standards from directory %s: %s" % (filepath, str(e)) + ) if getattr(standards, "ansible_min_version", None) and \ LooseVersion(standards.ansible_min_version) > LooseVersion(ansible.__version__): - utils.sysexit_with_message("Standards require ansible version %s (current version %s). " - "Please upgrade ansible." % - (standards.ansible_min_version, ansible.__version__)) + utils.sysexit_with_message( + "Standards require ansible version %s (current version %s). " + "Please upgrade ansible." % (standards.ansible_min_version, ansible.__version__) + ) if getattr(standards, "ansible_later_min_version", None) and \ LooseVersion(standards.ansible_later_min_version) > LooseVersion( @@ -47,13 +47,15 @@ def get_standards(filepath): utils.sysexit_with_message( "Standards require ansible-later version %s (current version %s). " "Please upgrade ansible-later." % - (standards.ansible_later_min_version, utils.get_property("__version__"))) + (standards.ansible_later_min_version, utils.get_property("__version__")) + ) normalized_std = (list(toolz.remove(lambda x: x.id == "", standards.standards))) unique_std = len(list(toolz.unique(normalized_std, key=lambda x: x.id))) all_std = len(normalized_std) if not all_std == unique_std: utils.sysexit_with_message( - "Detect duplicate ID's in standards definition. Please use unique ID's only.") + "Detect duplicate ID's in standards definition. Please use unique ID's only." + ) return standards.standards diff --git a/ansiblelater/command/candidates.py b/ansiblelater/command/candidates.py index aa3ba70..ed139a4 100644 --- a/ansiblelater/command/candidates.py +++ b/ansiblelater/command/candidates.py @@ -77,15 +77,18 @@ class Candidate(object): "%s %s is in a role that contains a meta/main.yml without a declared " "standards version. " "Using latest standards version %s" % - (type(self).__name__, self.path, version)) + (type(self).__name__, self.path, version) + ) else: LOG.warning( "%s %s does not present standards version. " "Using latest standards version %s" % - (type(self).__name__, self.path, version)) + (type(self).__name__, self.path, version) + ) else: - LOG.info("%s %s declares standards version %s" % - (type(self).__name__, self.path, version)) + LOG.info( + "%s %s declares standards version %s" % (type(self).__name__, self.path, version) + ) return version @@ -113,39 +116,61 @@ class Candidate(object): result = standard.check(self, settings.config) if not result: - utils.sysexit_with_message("Standard '{}' returns an empty result object.".format( - standard.check.__name__)) - - labels = {"tag": "review", "standard": standard.name, "file": self.path, "passed": True} + utils.sysexit_with_message( + "Standard '{}' returns an empty result object.".format( + standard.check.__name__ + ) + ) + + labels = { + "tag": "review", + "standard": standard.name, + "file": self.path, + "passed": True + } if standard.id and standard.id.strip(): labels["id"] = standard.id - for err in [err for err in result.errors - if not err.lineno or utils.is_line_in_ranges(err.lineno, utils.lines_ranges(lines))]: # noqa + for err in [ + err for err in result.errors if not err.lineno + or utils.is_line_in_ranges(err.lineno, utils.lines_ranges(lines)) + ]: # noqa err_labels = copy.copy(labels) err_labels["passed"] = False if isinstance(err, Error): err_labels.update(err.to_dict()) if not standard.version: - LOG.warning("{id}Best practice '{name}' not met:\n{path}:{error}".format( - id=self._format_id(standard.id), - name=standard.name, - path=self.path, - error=err), extra=flag_extra(err_labels)) + LOG.warning( + "{id}Best practice '{name}' not met:\n{path}:{error}".format( + id=self._format_id(standard.id), + name=standard.name, + path=self.path, + error=err + ), + extra=flag_extra(err_labels) + ) elif LooseVersion(standard.version) > LooseVersion(self.version): - LOG.warning("{id}Future standard '{name}' not met:\n{path}:{error}".format( - id=self._format_id(standard.id), - name=standard.name, - path=self.path, - error=err), extra=flag_extra(err_labels)) + LOG.warning( + "{id}Future standard '{name}' not met:\n{path}:{error}".format( + id=self._format_id(standard.id), + name=standard.name, + path=self.path, + error=err + ), + extra=flag_extra(err_labels) + ) else: - LOG.error("{id}Standard '{name}' not met:\n{path}:{error}".format( - id=self._format_id(standard.id), - name=standard.name, - path=self.path, - error=err), extra=flag_extra(err_labels)) + LOG.error( + "{id}Standard '{name}' not met:\n{path}:{error}".format( + id=self._format_id(standard.id), + name=standard.name, + path=self.path, + error=err + ), + extra=flag_extra(err_labels) + ) errors = errors + 1 return errors @@ -156,14 +181,15 @@ class Candidate(object): return standard_id - def __repr__(self): # noqa + def __repr__(self): # noqa return "%s (%s)" % (type(self).__name__, self.path) - def __getitem__(self, item): # noqa + def __getitem__(self, item): # noqa return self.__dict__.get(item) class RoleFile(Candidate): + def __init__(self, filename, settings={}, standards=[]): super(RoleFile, self).__init__(filename, settings, standards) @@ -181,12 +207,14 @@ class Playbook(Candidate): class Task(RoleFile): + def __init__(self, filename, settings={}, standards=[]): super(Task, self).__init__(filename, settings, standards) self.filetype = "tasks" class Handler(RoleFile): + def __init__(self, filename, settings={}, standards=[]): super(Handler, self).__init__(filename, settings, standards) self.filetype = "handlers" @@ -197,6 +225,7 @@ class Vars(Candidate): class Unversioned(Candidate): + def __init__(self, filename, settings={}, standards=[]): super(Unversioned, self).__init__(filename, settings, standards) self.expected_version = False @@ -267,7 +296,7 @@ class Error(object): for (key, value) in iteritems(kwargs): setattr(self, key, value) - def __repr__(self): # noqa + def __repr__(self): # noqa if self.lineno: return "%s: %s" % (self.lineno, self.message) else: @@ -281,13 +310,13 @@ class Error(object): class Result(object): + def __init__(self, candidate, errors=None): self.candidate = candidate self.errors = errors or [] def message(self): - return "\n".join(["{0}:{1}".format(self.candidate, error) - for error in self.errors]) + return "\n".join(["{0}:{1}".format(self.candidate, error) for error in self.errors]) def classify(filename, settings={}, standards=[]): @@ -306,8 +335,8 @@ def classify(filename, settings={}, standards=[]): return HostVars(filename, settings, standards) if parentdir in ["meta"]: return Meta(filename, settings, standards) - if parentdir in ["library", "lookup_plugins", "callback_plugins", - "filter_plugins"] or filename.endswith(".py"): + if parentdir in ["library", "lookup_plugins", "callback_plugins", "filter_plugins" + ] or filename.endswith(".py"): return Code(filename, settings, standards) if "inventory" == basename or "hosts" == basename or parentdir in ["inventories"]: return Inventory(filename, settings, standards) diff --git a/ansiblelater/data/standards.py b/ansiblelater/data/standards.py index e4fd602..6f77703 100644 --- a/ansiblelater/data/standards.py +++ b/ansiblelater/data/standards.py @@ -27,217 +27,257 @@ from ansiblelater.rules.yamlfiles import check_yaml_hyphens from ansiblelater.rules.yamlfiles import check_yaml_indent from ansiblelater.standard import Standard -tasks_should_be_separated = Standard(dict( - id="ANSIBLE0001", - name="Single tasks should be separated by empty line", - check=check_line_between_tasks, - version="0.1", - types=["playbook", "task", "handler"] -)) +tasks_should_be_separated = Standard( + dict( + id="ANSIBLE0001", + name="Single tasks should be separated by empty line", + check=check_line_between_tasks, + version="0.1", + types=["playbook", "task", "handler"] + ) +) -role_must_contain_meta_main = Standard(dict( - id="ANSIBLE0002", - name="Roles must contain suitable meta/main.yml", - check=check_meta_main, - version="0.1", - types=["meta"] -)) +role_must_contain_meta_main = Standard( + dict( + id="ANSIBLE0002", + name="Roles must contain suitable meta/main.yml", + check=check_meta_main, + version="0.1", + types=["meta"] + ) +) -tasks_are_uniquely_named = Standard(dict( - id="ANSIBLE0003", - name="Tasks and handlers must be uniquely named within a single file", - check=check_unique_named_task, - version="0.1", - types=["playbook", "task", "handler"], -)) +tasks_are_uniquely_named = Standard( + dict( + id="ANSIBLE0003", + name="Tasks and handlers must be uniquely named within a single file", + check=check_unique_named_task, + version="0.1", + types=["playbook", "task", "handler"], + ) +) -use_spaces_between_variable_braces = Standard(dict( - id="ANSIBLE0004", - name="YAML should use consistent number of spaces around variables", - check=check_braces_spaces, - version="0.1", - types=["playbook", "task", "handler", "rolevars", - "hostvars", "groupvars", "meta"] -)) +use_spaces_between_variable_braces = Standard( + dict( + id="ANSIBLE0004", + name="YAML should use consistent number of spaces around variables", + check=check_braces_spaces, + version="0.1", + types=["playbook", "task", "handler", "rolevars", "hostvars", "groupvars", "meta"] + ) +) -roles_scm_not_in_src = Standard(dict( - id="ANSIBLE0005", - name="Use scm key rather than src: scm+url", - check=check_scm_in_src, - version="0.1", - types=["rolesfile"] -)) +roles_scm_not_in_src = Standard( + dict( + id="ANSIBLE0005", + name="Use scm key rather than src: scm+url", + check=check_scm_in_src, + version="0.1", + types=["rolesfile"] + ) +) -tasks_are_named = Standard(dict( - id="ANSIBLE0006", - name="Tasks and handlers must be named", - check=check_named_task, - version="0.1", - types=["playbook", "task", "handler"], -)) +tasks_are_named = Standard( + dict( + id="ANSIBLE0006", + name="Tasks and handlers must be named", + check=check_named_task, + version="0.1", + types=["playbook", "task", "handler"], + ) +) -tasks_names_are_formatted = Standard(dict( - id="ANSIBLE0007", - name="Name of tasks and handlers must be formatted", - check=check_name_format, - version="0.1", - types=["playbook", "task", "handler"], -)) +tasks_names_are_formatted = Standard( + dict( + id="ANSIBLE0007", + name="Name of tasks and handlers must be formatted", + check=check_name_format, + version="0.1", + types=["playbook", "task", "handler"], + ) +) -commands_should_not_be_used_in_place_of_modules = Standard(dict( - id="ANSIBLE0008", - name="Commands should not be used in place of modules", - check=check_command_instead_of_module, - version="0.1", - types=["playbook", "task", "handler"] -)) +commands_should_not_be_used_in_place_of_modules = Standard( + dict( + id="ANSIBLE0008", + name="Commands should not be used in place of modules", + check=check_command_instead_of_module, + version="0.1", + types=["playbook", "task", "handler"] + ) +) -package_installs_should_not_use_latest = Standard(dict( - id="ANSIBLE0009", - name="Package installs should use present, not latest", - check=check_install_use_latest, - types=["playbook", "task", "handler"] -)) +package_installs_should_not_use_latest = Standard( + dict( + id="ANSIBLE0009", + name="Package installs should use present, not latest", + check=check_install_use_latest, + types=["playbook", "task", "handler"] + ) +) -use_shell_only_when_necessary = Standard(dict( - id="ANSIBLE0010", - name="Shell should only be used when essential", - check=check_shell_instead_command, - types=["playbook", "task", "handler"] -)) +use_shell_only_when_necessary = Standard( + dict( + id="ANSIBLE0010", + name="Shell should only be used when essential", + check=check_shell_instead_command, + types=["playbook", "task", "handler"] + ) +) -commands_should_be_idempotent = Standard(dict( - id="ANSIBLE0011", - name="Commands should be idempotent", - check=check_command_has_changes, - version="0.1", - types=["playbook", "task"] -)) +commands_should_be_idempotent = Standard( + dict( + id="ANSIBLE0011", + name="Commands should be idempotent", + check=check_command_has_changes, + version="0.1", + types=["playbook", "task"] + ) +) -dont_compare_to_empty_string = Standard(dict( - id="ANSIBLE0012", - name="Don't compare to \"\" - use `when: var` or `when: not var`", - check=check_empty_string_compare, - version="0.1", - types=["playbook", "task", "handler", "template"] -)) +dont_compare_to_empty_string = Standard( + dict( + id="ANSIBLE0012", + name="Don't compare to \"\" - use `when: var` or `when: not var`", + check=check_empty_string_compare, + version="0.1", + types=["playbook", "task", "handler", "template"] + ) +) -dont_compare_to_literal_bool = Standard(dict( - id="ANSIBLE0013", - name="Don't compare to True or False - use `when: var` or `when: not var`", - check=check_compare_to_literal_bool, - version="0.1", - types=["playbook", "task", "handler", "template"] -)) +dont_compare_to_literal_bool = Standard( + dict( + id="ANSIBLE0013", + name="Don't compare to True or False - use `when: var` or `when: not var`", + check=check_compare_to_literal_bool, + version="0.1", + types=["playbook", "task", "handler", "template"] + ) +) -literal_bool_should_be_formatted = Standard(dict( - id="ANSIBLE0014", - name="Literal bools should start with a capital letter", - check=check_literal_bool_format, - version="0.1", - types=["playbook", "task", "handler", "rolevars", - "hostvars", "groupvars"] -)) +literal_bool_should_be_formatted = Standard( + dict( + id="ANSIBLE0014", + name="Literal bools should start with a capital letter", + check=check_literal_bool_format, + version="0.1", + types=["playbook", "task", "handler", "rolevars", "hostvars", "groupvars"] + ) +) -use_become_with_become_user = Standard(dict( - id="ANSIBLE0015", - name="become should be combined with become_user", - check=check_become_user, - version="0.1", - types=["playbook", "task", "handler"] -)) +use_become_with_become_user = Standard( + dict( + id="ANSIBLE0015", + name="become should be combined with become_user", + check=check_become_user, + version="0.1", + types=["playbook", "task", "handler"] + ) +) -use_spaces_around_filters = Standard(dict( - id="ANSIBLE0016", - name="jinja2 filters should be separated with spaces", - check=check_filter_separation, - version="0.1", - types=["playbook", "task", "handler", "rolevars", - "hostvars", "groupvars"] -)) +use_spaces_around_filters = Standard( + dict( + id="ANSIBLE0016", + name="jinja2 filters should be separated with spaces", + check=check_filter_separation, + version="0.1", + types=["playbook", "task", "handler", "rolevars", "hostvars", "groupvars"] + ) +) -files_should_not_contain_unnecessarily_empty_lines = Standard(dict( - id="LINT0001", - name="YAML should not contain unnecessarily empty lines", - check=check_yaml_empty_lines, - version="0.1", - types=["playbook", "task", "handler", "rolevars", - "hostvars", "groupvars", "meta"] -)) +files_should_not_contain_unnecessarily_empty_lines = Standard( + dict( + id="LINT0001", + name="YAML should not contain unnecessarily empty lines", + check=check_yaml_empty_lines, + version="0.1", + types=["playbook", "task", "handler", "rolevars", "hostvars", "groupvars", "meta"] + ) +) -files_should_be_indented = Standard(dict( - id="LINT0002", - name="YAML should be correctly indented", - check=check_yaml_indent, - version="0.1", - types=["playbook", "task", "handler", "rolevars", - "hostvars", "groupvars", "meta"] -)) +files_should_be_indented = Standard( + dict( + id="LINT0002", + name="YAML should be correctly indented", + check=check_yaml_indent, + version="0.1", + types=["playbook", "task", "handler", "rolevars", "hostvars", "groupvars", "meta"] + ) +) -files_should_use_consistent_spaces_after_hyphens = Standard(dict( - id="LINT0003", - name="YAML should use consistent number of spaces after hyphens", - check=check_yaml_hyphens, - version="0.1", - types=["playbook", "task", "handler", "rolevars", - "hostvars", "groupvars", "meta"] -)) +files_should_use_consistent_spaces_after_hyphens = Standard( + dict( + id="LINT0003", + name="YAML should use consistent number of spaces after hyphens", + check=check_yaml_hyphens, + version="0.1", + types=["playbook", "task", "handler", "rolevars", "hostvars", "groupvars", "meta"] + ) +) -files_should_contain_document_start_marker = Standard(dict( - id="LINT0004", - name="YAML should contain document start marker", - check=check_yaml_document_start, - version="0.1", - types=["playbook", "task", "handler", "rolevars", - "hostvars", "groupvars", "meta"] -)) +files_should_contain_document_start_marker = Standard( + dict( + id="LINT0004", + name="YAML should contain document start marker", + check=check_yaml_document_start, + version="0.1", + types=["playbook", "task", "handler", "rolevars", "hostvars", "groupvars", "meta"] + ) +) -spaces_around_colons = Standard(dict( - id="LINT0005", - name="YAML should use consistent number of spaces around colons", - check=check_yaml_colons, - version="0.1", - types=["playbook", "task", "handler", "rolevars", - "hostvars", "groupvars", "meta"] -)) +spaces_around_colons = Standard( + dict( + id="LINT0005", + name="YAML should use consistent number of spaces around colons", + check=check_yaml_colons, + version="0.1", + types=["playbook", "task", "handler", "rolevars", "hostvars", "groupvars", "meta"] + ) +) -rolesfile_should_be_in_yaml = Standard(dict( - id="LINT0006", - name="Roles file should be in yaml format", - check=check_yaml_file, - version="0.1", - types=["rolesfile"] -)) +rolesfile_should_be_in_yaml = Standard( + dict( + id="LINT0006", + name="Roles file should be in yaml format", + check=check_yaml_file, + version="0.1", + types=["rolesfile"] + ) +) -files_should_not_be_purposeless = Standard(dict( - id="LINT0007", - name="Files should contain useful content", - check=check_yaml_has_content, - version="0.1", - types=["playbook", "task", "handler", "rolevars", "defaults", "meta"] -)) +files_should_not_be_purposeless = Standard( + dict( + id="LINT0007", + name="Files should contain useful content", + check=check_yaml_has_content, + version="0.1", + types=["playbook", "task", "handler", "rolevars", "defaults", "meta"] + ) +) -use_yaml_rather_than_key_value = Standard(dict( - id="LINT0008", - name="Use YAML format for tasks and handlers rather than key=value", - check=check_native_yaml, - version="0.1", - types=["playbook", "task", "handler"] -)) +use_yaml_rather_than_key_value = Standard( + dict( + id="LINT0008", + name="Use YAML format for tasks and handlers rather than key=value", + check=check_native_yaml, + version="0.1", + types=["playbook", "task", "handler"] + ) +) -files_should_contain_document_end_marker = Standard(dict( - id="LINT0009", - name="YAML should contain document end marker", - check=check_yaml_document_end, - version="0.1", - types=["playbook", "task", "handler", "rolevars", - "hostvars", "groupvars", "meta"] -)) +files_should_contain_document_end_marker = Standard( + dict( + id="LINT0009", + name="YAML should contain document end marker", + check=check_yaml_document_end, + version="0.1", + types=["playbook", "task", "handler", "rolevars", "hostvars", "groupvars", "meta"] + ) +) ansible_min_version = "2.5" ansible_later_min_version = "0.3.0" - standards = [ # Ansible tasks_should_be_separated, diff --git a/ansiblelater/logger.py b/ansiblelater/logger.py index 8393e46..c96352b 100644 --- a/ansiblelater/logger.py +++ b/ansiblelater/logger.py @@ -61,7 +61,7 @@ class LogFilter(object): class MultilineFormatter(logging.Formatter): """Logging Formatter to reset color after newline characters.""" - def format(self, record): # noqa + def format(self, record): # noqa record.msg = record.msg.replace("\n", "\n{}... ".format(colorama.Style.RESET_ALL)) return logging.Formatter.format(self, record) @@ -69,7 +69,7 @@ class MultilineFormatter(logging.Formatter): class MultilineJsonFormatter(jsonlogger.JsonFormatter): """Logging Formatter to remove newline characters.""" - def format(self, record): # noqa + def format(self, record): # noqa record.msg = record.msg.replace("\n", " ") return jsonlogger.JsonFormatter.format(self, record) diff --git a/ansiblelater/rules/ansiblefiles.py b/ansiblelater/rules/ansiblefiles.py index 0a47652..ce5a91e 100644 --- a/ansiblelater/rules/ansiblefiles.py +++ b/ansiblelater/rules/ansiblefiles.py @@ -16,7 +16,8 @@ def check_braces_spaces(candidate, settings): yamllines, errors = get_normalized_yaml(candidate, settings) conf = settings["ansible"]["double-braces"] description = "no suitable numbers of spaces (min: {min} max: {max})".format( - min=conf["min-spaces-inside"], max=conf["max-spaces-inside"]) + min=conf["min-spaces-inside"], max=conf["max-spaces-inside"] + ) matches = [] braces = re.compile("{{(.*?)}}") @@ -32,19 +33,18 @@ def check_braces_spaces(candidate, settings): [leading, trailing] = count_spaces(line) sum_spaces = leading + trailing - if ( - (sum_spaces < conf["min-spaces-inside"] * 2) - or (sum_spaces > conf["min-spaces-inside"] * 2) - ): + if ((sum_spaces < conf["min-spaces-inside"] * 2) + or (sum_spaces > conf["min-spaces-inside"] * 2)): errors.append(Error(i, description)) return Result(candidate.path, errors) def check_named_task(candidate, settings): tasks, errors = get_normalized_tasks(candidate, settings) - nameless_tasks = ["meta", "debug", "include_role", "import_role", - "include_tasks", "import_tasks", "include_vars", - "block"] + nameless_tasks = [ + "meta", "debug", "include_role", "import_role", "include_tasks", "import_tasks", + "include_vars", "block" + ] description = "module '%s' used without or empty name attribute" if not errors: @@ -93,11 +93,22 @@ def check_command_instead_of_module(candidate, settings): tasks, errors = get_normalized_tasks(candidate, settings) commands = ["command", "shell", "raw"] modules = { - "git": "git", "hg": "hg", "curl": "get_url or uri", "wget": "get_url or uri", - "svn": "subversion", "service": "service", "mount": "mount", - "rpm": "yum or rpm_key", "yum": "yum", "apt-get": "apt-get", - "unzip": "unarchive", "tar": "unarchive", "chkconfig": "service", - "rsync": "synchronize", "supervisorctl": "supervisorctl", "systemctl": "systemd", + "git": "git", + "hg": "hg", + "curl": "get_url or uri", + "wget": "get_url or uri", + "svn": "subversion", + "service": "service", + "mount": "mount", + "rpm": "yum or rpm_key", + "yum": "yum", + "apt-get": "apt-get", + "unzip": "unarchive", + "tar": "unarchive", + "chkconfig": "service", + "rsync": "synchronize", + "supervisorctl": "supervisorctl", + "systemctl": "systemd", "sed": "template or lineinfile" } description = "%s command used in place of %s module" @@ -111,26 +122,32 @@ def check_command_instead_of_module(candidate, settings): first_cmd_arg = task["action"]["__ansible_arguments__"][0] executable = os.path.basename(first_cmd_arg) - if (first_cmd_arg and executable in modules - and task["action"].get("warn", True) and "register" not in task): + if ( + first_cmd_arg and executable in modules and task["action"].get("warn", True) + and "register" not in task + ): errors.append( - Error(task["__line__"], description % (executable, modules[executable]))) + Error(task["__line__"], description % (executable, modules[executable])) + ) return Result(candidate.path, errors) def check_install_use_latest(candidate, settings): tasks, errors = get_normalized_tasks(candidate, settings) - package_managers = ["yum", "apt", "dnf", "homebrew", "pacman", "openbsd_package", "pkg5", - "portage", "pkgutil", "slackpkg", "swdepot", "zypper", "bundler", "pip", - "pear", "npm", "yarn", "gem", "easy_install", "bower", "package", "apk", - "openbsd_pkg", "pkgng", "sorcery", "xbps"] + package_managers = [ + "yum", "apt", "dnf", "homebrew", "pacman", "openbsd_package", "pkg5", "portage", "pkgutil", + "slackpkg", "swdepot", "zypper", "bundler", "pip", "pear", "npm", "yarn", "gem", + "easy_install", "bower", "package", "apk", "openbsd_pkg", "pkgng", "sorcery", "xbps" + ] description = "package installs should use state=present with or without a version" if not errors: for task in tasks: - if (task["action"]["__ansible_module__"] in package_managers - and task["action"].get("state") == "latest"): + if ( + task["action"]["__ansible_module__"] in package_managers + and task["action"].get("state") == "latest" + ): errors.append(Error(task["__line__"], description)) return Result(candidate.path, errors) @@ -165,10 +182,11 @@ def check_command_has_changes(candidate, settings): if not errors: for task in tasks: if task["action"]["__ansible_module__"] in commands: - if ("changed_when" not in task and "when" not in task - and "when" not in task["__ansible_action_meta__"] - and "creates" not in task["action"] - and "removes" not in task["action"]): + if ( + "changed_when" not in task and "when" not in task + and "when" not in task["__ansible_action_meta__"] + and "creates" not in task["action"] and "removes" not in task["action"] + ): errors.append(Error(task["__line__"], description)) return Result(candidate.path, errors) diff --git a/ansiblelater/rules/yamlfiles.py b/ansiblelater/rules/yamlfiles.py index aa72832..3f26c3a 100644 --- a/ansiblelater/rules/yamlfiles.py +++ b/ansiblelater/rules/yamlfiles.py @@ -49,43 +49,39 @@ def check_native_yaml(candidate, settings): def check_yaml_empty_lines(candidate, settings): - options = "rules: {{empty-lines: {conf}}}".format( - conf=settings["yamllint"]["empty-lines"]) + options = "rules: {{empty-lines: {conf}}}".format(conf=settings["yamllint"]["empty-lines"]) errors = run_yamllint(candidate.path, options) return Result(candidate.path, errors) def check_yaml_indent(candidate, settings): - options = "rules: {{indentation: {conf}}}".format( - conf=settings["yamllint"]["indentation"]) + options = "rules: {{indentation: {conf}}}".format(conf=settings["yamllint"]["indentation"]) errors = run_yamllint(candidate.path, options) return Result(candidate.path, errors) def check_yaml_hyphens(candidate, settings): - options = "rules: {{hyphens: {conf}}}".format( - conf=settings["yamllint"]["hyphens"]) + options = "rules: {{hyphens: {conf}}}".format(conf=settings["yamllint"]["hyphens"]) errors = run_yamllint(candidate.path, options) return Result(candidate.path, errors) def check_yaml_document_start(candidate, settings): options = "rules: {{document-start: {conf}}}".format( - conf=settings["yamllint"]["document-start"]) + conf=settings["yamllint"]["document-start"] + ) errors = run_yamllint(candidate.path, options) return Result(candidate.path, errors) def check_yaml_document_end(candidate, settings): - options = "rules: {{document-end: {conf}}}".format( - conf=settings["yamllint"]["document-end"]) + options = "rules: {{document-end: {conf}}}".format(conf=settings["yamllint"]["document-end"]) errors = run_yamllint(candidate.path, options) return Result(candidate.path, errors) def check_yaml_colons(candidate, settings): - options = "rules: {{colons: {conf}}}".format( - conf=settings["yamllint"]["colons"]) + options = "rules: {{colons: {conf}}}".format(conf=settings["yamllint"]["colons"]) errors = run_yamllint(candidate.path, options) return Result(candidate.path, errors) @@ -95,14 +91,12 @@ def check_yaml_file(candidate, settings): filename = candidate.path if os.path.isfile(filename) and os.path.splitext(filename)[1][1:] != "yml": - errors.append( - Error(None, "file does not have a .yml extension")) + errors.append(Error(None, "file does not have a .yml extension")) elif os.path.isfile(filename) and os.path.splitext(filename)[1][1:] == "yml": with codecs.open(filename, mode="rb", encoding="utf-8") as f: try: yaml.safe_load(f) except Exception as e: - errors.append( - Error(e.problem_mark.line + 1, "syntax error: %s" % (e.problem))) + errors.append(Error(e.problem_mark.line + 1, "syntax error: %s" % (e.problem))) return Result(candidate.path, errors) diff --git a/ansiblelater/settings.py b/ansiblelater/settings.py index 495c9a8..7a8f797 100644 --- a/ansiblelater/settings.py +++ b/ansiblelater/settings.py @@ -71,8 +71,9 @@ class Settings(object): defaults = self._get_defaults() source_files = [] source_files.append(self.config_file) - source_files.append(os.path.relpath( - os.path.normpath(os.path.join(os.getcwd(), ".later.yml")))) + source_files.append( + os.path.relpath(os.path.normpath(os.path.join(os.getcwd(), ".later.yml"))) + ) cli_options = self.args for config in source_files: @@ -147,10 +148,11 @@ class Settings(object): return True except Exception as e: schema_error = "Failed validating '{validator}' in schema{schema}".format( - validator=e.validator, - schema=format_as_index(list(e.relative_schema_path)[:-1]) + validator=e.validator, schema=format_as_index(list(e.relative_schema_path)[:-1]) + ) + utils.sysexit_with_message( + "{schema}: {msg}".format(schema=schema_error, msg=e.message) ) - utils.sysexit_with_message("{schema}: {msg}".format(schema=schema_error, msg=e.message)) def _update_filelist(self): includes = self.config["rules"]["files"] @@ -165,8 +167,7 @@ class Settings(object): filelist = [] for root, dirs, files in os.walk("."): for filename in files: - filelist.append( - os.path.relpath(os.path.normpath(os.path.join(root, filename)))) + filelist.append(os.path.relpath(os.path.normpath(os.path.join(root, filename)))) valid = [] includespec = pathspec.PathSpec.from_lines("gitwildmatch", includes) diff --git a/ansiblelater/standard.py b/ansiblelater/standard.py index cc2f0ea..15b0c73 100644 --- a/ansiblelater/standard.py +++ b/ansiblelater/standard.py @@ -22,7 +22,5 @@ class Standard(object): self.check = standard_dict.get("check") self.types = standard_dict.get("types") - - def __repr__(self): # noqa - return "Standard: %s (version: %s, types: %s)" % ( - self.name, self.version, self.types) + def __repr__(self): # noqa + return "Standard: %s (version: %s, types: %s)" % (self.name, self.version, self.types) diff --git a/ansiblelater/tests/config/standards.py b/ansiblelater/tests/config/standards.py index c433a39..4198f0b 100644 --- a/ansiblelater/tests/config/standards.py +++ b/ansiblelater/tests/config/standards.py @@ -21,183 +21,217 @@ from ansiblelater.rules.yamlfiles import check_yaml_has_content from ansiblelater.rules.yamlfiles import check_yaml_hyphens from ansiblelater.rules.yamlfiles import check_yaml_indent -tasks_should_be_separated = Standard(dict( - id="ANSIBLE0001", - name="Single tasks should be separated by empty line", - check=check_line_between_tasks, - version="0.1", - types=["playbook", "task", "handler"] -)) - -role_must_contain_meta_main = Standard(dict( - id="ANSIBLE0002", - name="Roles must contain suitable meta/main.yml", - check=check_meta_main, - version="0.1", - types=["meta"] -)) - -tasks_are_uniquely_named = Standard(dict( - id="ANSIBLE0003", - name="Tasks and handlers must be uniquely named within a single file", - check=check_unique_named_task, - version="0.1", - types=["playbook", "task", "handler"], -)) - -use_spaces_between_variable_braces = Standard(dict( - id="ANSIBLE0004", - name="YAML should use consistent number of spaces around variables", - check=check_braces_spaces, - version="0.1", - types=["playbook", "task", "handler", "rolevars", - "hostvars", "groupvars", "meta"] -)) - -roles_scm_not_in_src = Standard(dict( - id="ANSIBLE0005", - name="Use scm key rather than src: scm+url", - check=check_scm_in_src, - version="0.1", - types=["rolesfile"] -)) - -tasks_are_named = Standard(dict( - id="ANSIBLE0006", - name="Tasks and handlers must be named", - check=check_named_task, - version="0.1", - types=["playbook", "task", "handler"], -)) - -tasks_names_are_formatted = Standard(dict( - id="ANSIBLE0007", - name="Name of tasks and handlers must be formatted", - check=check_name_format, - version="0.1", - types=["playbook", "task", "handler"], -)) - -commands_should_not_be_used_in_place_of_modules = Standard(dict( - id="ANSIBLE0008", - name="Commands should not be used in place of modules", - check=check_command_instead_of_module, - version="0.1", - types=["playbook", "task", "handler"] -)) - -package_installs_should_not_use_latest = Standard(dict( - id="ANSIBLE0009", - name="Package installs should use present, not latest", - check=check_install_use_latest, - types=["playbook", "task", "handler"] -)) - -use_shell_only_when_necessary = Standard(dict( - id="ANSIBLE0010", - name="Shell should only be used when essential", - check=check_shell_instead_command, - types=["playbook", "task", "handler"] -)) - -commands_should_be_idempotent = Standard(dict( - id="ANSIBLE0011", - name="Commands should be idempotent", - check=check_command_has_changes, - version="0.1", - types=["playbook", "task"] -)) - -dont_compare_to_empty_string = Standard(dict( - id="ANSIBLE0012", - name="Don't compare to \"\" - use `when: var` or `when: not var`", - check=check_empty_string_compare, - version="0.1", - types=["playbook", "task", "handler", "template"] -)) - -dont_compare_to_literal_bool = Standard(dict( - id="ANSIBLE0013", - name="Don't compare to True or False - use `when: var` or `when: not var`", - check=check_compare_to_literal_bool, - version="0.1", - types=["playbook", "task", "handler", "template"] -)) - -files_should_not_contain_unnecessarily_empty_lines = Standard(dict( - id="LINT0001", - name="YAML should not contain unnecessarily empty lines", - check=check_yaml_empty_lines, - version="0.1", - types=["playbook", "task", "handler", "rolevars", - "hostvars", "groupvars", "meta"] -)) - -files_should_be_indented = Standard(dict( - id="LINT0002", - name="YAML should be correctly indented", - check=check_yaml_indent, - version="0.1", - types=["playbook", "task", "handler", "rolevars", - "hostvars", "groupvars", "meta"] -)) - -files_should_use_consistent_spaces_after_hyphens = Standard(dict( - id="LINT0003", - name="YAML should use consistent number of spaces after hyphens", - check=check_yaml_hyphens, - version="0.1", - types=["playbook", "task", "handler", "rolevars", - "hostvars", "groupvars", "meta"] -)) - -files_should_contain_document_start_marker = Standard(dict( - id="LINT0004", - name="YAML should contain document start marker", - check=check_yaml_document_start, - version="0.1", - types=["playbook", "task", "handler", "rolevars", - "hostvars", "groupvars", "meta"] -)) - -spaces_around_colons = Standard(dict( - id="LINT0005", - name="YAML should use consistent number of spaces around colons", - check=check_yaml_colons, - version="0.1", - types=["playbook", "task", "handler", "rolevars", - "hostvars", "groupvars", "meta"] -)) - -rolesfile_should_be_in_yaml = Standard(dict( - id="LINT0006", - name="Roles file should be in yaml format", - check=check_yaml_file, - version="0.1", - types=["rolesfile"] -)) - -files_should_not_be_purposeless = Standard(dict( - id="LINT0007", - name="Files should contain useful content", - check=check_yaml_has_content, - version="0.1", - types=["playbook", "task", "handler", "rolevars", "defaults", "meta"] -)) - -use_yaml_rather_than_key_value = Standard(dict( - id="LINT0008", - name="Use YAML format for tasks and handlers rather than key=value", - check=check_native_yaml, - version="0.1", - types=["playbook", "task", "handler"] -)) +tasks_should_be_separated = Standard( + dict( + id="ANSIBLE0001", + name="Single tasks should be separated by empty line", + check=check_line_between_tasks, + version="0.1", + types=["playbook", "task", "handler"] + ) +) +role_must_contain_meta_main = Standard( + dict( + id="ANSIBLE0002", + name="Roles must contain suitable meta/main.yml", + check=check_meta_main, + version="0.1", + types=["meta"] + ) +) + +tasks_are_uniquely_named = Standard( + dict( + id="ANSIBLE0003", + name="Tasks and handlers must be uniquely named within a single file", + check=check_unique_named_task, + version="0.1", + types=["playbook", "task", "handler"], + ) +) + +use_spaces_between_variable_braces = Standard( + dict( + id="ANSIBLE0004", + name="YAML should use consistent number of spaces around variables", + check=check_braces_spaces, + version="0.1", + types=["playbook", "task", "handler", "rolevars", "hostvars", "groupvars", "meta"] + ) +) + +roles_scm_not_in_src = Standard( + dict( + id="ANSIBLE0005", + name="Use scm key rather than src: scm+url", + check=check_scm_in_src, + version="0.1", + types=["rolesfile"] + ) +) + +tasks_are_named = Standard( + dict( + id="ANSIBLE0006", + name="Tasks and handlers must be named", + check=check_named_task, + version="0.1", + types=["playbook", "task", "handler"], + ) +) + +tasks_names_are_formatted = Standard( + dict( + id="ANSIBLE0007", + name="Name of tasks and handlers must be formatted", + check=check_name_format, + version="0.1", + types=["playbook", "task", "handler"], + ) +) + +commands_should_not_be_used_in_place_of_modules = Standard( + dict( + id="ANSIBLE0008", + name="Commands should not be used in place of modules", + check=check_command_instead_of_module, + version="0.1", + types=["playbook", "task", "handler"] + ) +) + +package_installs_should_not_use_latest = Standard( + dict( + id="ANSIBLE0009", + name="Package installs should use present, not latest", + check=check_install_use_latest, + types=["playbook", "task", "handler"] + ) +) + +use_shell_only_when_necessary = Standard( + dict( + id="ANSIBLE0010", + name="Shell should only be used when essential", + check=check_shell_instead_command, + types=["playbook", "task", "handler"] + ) +) + +commands_should_be_idempotent = Standard( + dict( + id="ANSIBLE0011", + name="Commands should be idempotent", + check=check_command_has_changes, + version="0.1", + types=["playbook", "task"] + ) +) + +dont_compare_to_empty_string = Standard( + dict( + id="ANSIBLE0012", + name="Don't compare to \"\" - use `when: var` or `when: not var`", + check=check_empty_string_compare, + version="0.1", + types=["playbook", "task", "handler", "template"] + ) +) + +dont_compare_to_literal_bool = Standard( + dict( + id="ANSIBLE0013", + name="Don't compare to True or False - use `when: var` or `when: not var`", + check=check_compare_to_literal_bool, + version="0.1", + types=["playbook", "task", "handler", "template"] + ) +) + +files_should_not_contain_unnecessarily_empty_lines = Standard( + dict( + id="LINT0001", + name="YAML should not contain unnecessarily empty lines", + check=check_yaml_empty_lines, + version="0.1", + types=["playbook", "task", "handler", "rolevars", "hostvars", "groupvars", "meta"] + ) +) + +files_should_be_indented = Standard( + dict( + id="LINT0002", + name="YAML should be correctly indented", + check=check_yaml_indent, + version="0.1", + types=["playbook", "task", "handler", "rolevars", "hostvars", "groupvars", "meta"] + ) +) + +files_should_use_consistent_spaces_after_hyphens = Standard( + dict( + id="LINT0003", + name="YAML should use consistent number of spaces after hyphens", + check=check_yaml_hyphens, + version="0.1", + types=["playbook", "task", "handler", "rolevars", "hostvars", "groupvars", "meta"] + ) +) + +files_should_contain_document_start_marker = Standard( + dict( + id="LINT0004", + name="YAML should contain document start marker", + check=check_yaml_document_start, + version="0.1", + types=["playbook", "task", "handler", "rolevars", "hostvars", "groupvars", "meta"] + ) +) + +spaces_around_colons = Standard( + dict( + id="LINT0005", + name="YAML should use consistent number of spaces around colons", + check=check_yaml_colons, + version="0.1", + types=["playbook", "task", "handler", "rolevars", "hostvars", "groupvars", "meta"] + ) +) + +rolesfile_should_be_in_yaml = Standard( + dict( + id="LINT0006", + name="Roles file should be in yaml format", + check=check_yaml_file, + version="0.1", + types=["rolesfile"] + ) +) + +files_should_not_be_purposeless = Standard( + dict( + id="LINT0007", + name="Files should contain useful content", + check=check_yaml_has_content, + version="0.1", + types=["playbook", "task", "handler", "rolevars", "defaults", "meta"] + ) +) + +use_yaml_rather_than_key_value = Standard( + dict( + id="LINT0008", + name="Use YAML format for tasks and handlers rather than key=value", + check=check_native_yaml, + version="0.1", + types=["playbook", "task", "handler"] + ) +) ansible_min_version = '2.1' ansible_later_min_version = '0.1.0' - standards = [ # Ansible tasks_should_be_separated, diff --git a/ansiblelater/tests/unit/test_logger.py b/ansiblelater/tests/unit/test_logger.py index 1c5a05d..befcfc0 100644 --- a/ansiblelater/tests/unit/test_logger.py +++ b/ansiblelater/tests/unit/test_logger.py @@ -21,8 +21,7 @@ def test_critical(capsys, mocker): log.critical("foo") _, stderr = capsys.readouterr() - print("{}{}{}".format(colorama.Fore.RED, "CRITICAL: foo".rstrip(), - colorama.Style.RESET_ALL)) + print("{}{}{}".format(colorama.Fore.RED, "CRITICAL: foo".rstrip(), colorama.Style.RESET_ALL)) x, _ = capsys.readouterr() assert x == stderr @@ -33,8 +32,7 @@ def test_error(capsys, mocker): log.error("foo") _, stderr = capsys.readouterr() - print("{}{}{}".format(colorama.Fore.RED, "ERROR: foo".rstrip(), - colorama.Style.RESET_ALL)) + print("{}{}{}".format(colorama.Fore.RED, "ERROR: foo".rstrip(), colorama.Style.RESET_ALL)) x, _ = capsys.readouterr() assert x == stderr @@ -45,8 +43,7 @@ def test_warn(capsys, mocker): log.warning("foo") stdout, _ = capsys.readouterr() - print("{}{}{}".format(colorama.Fore.YELLOW, "WARNING: foo".rstrip(), - colorama.Style.RESET_ALL)) + print("{}{}{}".format(colorama.Fore.YELLOW, "WARNING: foo".rstrip(), colorama.Style.RESET_ALL)) x, _ = capsys.readouterr() assert x == stdout @@ -57,8 +54,7 @@ def test_info(capsys, mocker): log.info("foo") stdout, _ = capsys.readouterr() - print("{}{}{}".format(colorama.Fore.BLUE, "INFO: foo".rstrip(), - colorama.Style.RESET_ALL)) + print("{}{}{}".format(colorama.Fore.BLUE, "INFO: foo".rstrip(), colorama.Style.RESET_ALL)) x, _ = capsys.readouterr() assert x == stdout diff --git a/ansiblelater/utils/__init__.py b/ansiblelater/utils/__init__.py index e3999a7..70e7df3 100644 --- a/ansiblelater/utils/__init__.py +++ b/ansiblelater/utils/__init__.py @@ -13,10 +13,9 @@ import yaml from ansiblelater import logger try: - import ConfigParser as configparser # noqa + import ConfigParser as configparser # noqa except ImportError: - import configparser # noqa - + import configparser # noqa LOG = logger.get_logger(__name__) @@ -35,7 +34,7 @@ def count_spaces(c_string): break trailing_spaces += 1 - return((leading_spaces, trailing_spaces)) + return ((leading_spaces, trailing_spaces)) def get_property(prop): @@ -43,7 +42,8 @@ def get_property(prop): parentdir = os.path.dirname(currentdir) result = re.search( r'{}\s*=\s*[\'"]([^\'"]*)[\'"]'.format(prop), - open(os.path.join(parentdir, "__init__.py")).read()) + open(os.path.join(parentdir, "__init__.py")).read() + ) return result.group(1) diff --git a/ansiblelater/utils/rulehelper.py b/ansiblelater/utils/rulehelper.py index bb82e71..cf65389 100644 --- a/ansiblelater/utils/rulehelper.py +++ b/ansiblelater/utils/rulehelper.py @@ -81,7 +81,8 @@ def get_normalized_tasks(candidate, settings): # No need to normalize_task if we are skipping it. continue normalized.append( - normalize_task(task, candidate.path, settings["ansible"]["custom_modules"])) + normalize_task(task, candidate.path, settings["ansible"]["custom_modules"]) + ) except LaterError as ex: e = ex.original diff --git a/ansiblelater/utils/yamlhelper.py b/ansiblelater/utils/yamlhelper.py index 7f7fc2b..35e002c 100644 --- a/ansiblelater/utils/yamlhelper.py +++ b/ansiblelater/utils/yamlhelper.py @@ -78,14 +78,46 @@ LINE_NUMBER_KEY = "__line__" FILENAME_KEY = "__file__" VALID_KEYS = [ - "name", "action", "when", "async", "poll", "notify", - "first_available_file", "include", "import_playbook", - "tags", "register", "ignore_errors", "delegate_to", - "local_action", "transport", "remote_user", "sudo", - "sudo_user", "sudo_pass", "when", "connection", "environment", "args", "always_run", - "any_errors_fatal", "changed_when", "failed_when", "check_mode", "delay", - "retries", "until", "su", "su_user", "su_pass", "no_log", "run_once", - "become", "become_user", "become_method", FILENAME_KEY, + "name", + "action", + "when", + "async", + "poll", + "notify", + "first_available_file", + "include", + "import_playbook", + "tags", + "register", + "ignore_errors", + "delegate_to", + "local_action", + "transport", + "remote_user", + "sudo", + "sudo_user", + "sudo_pass", + "when", + "connection", + "environment", + "args", + "always_run", + "any_errors_fatal", + "changed_when", + "failed_when", + "check_mode", + "delay", + "retries", + "until", + "su", + "su_user", + "su_pass", + "no_log", + "run_once", + "become", + "become_user", + "become_method", + FILENAME_KEY, ] BLOCK_NAME_TO_ACTION_TYPE_MAP = { @@ -170,17 +202,16 @@ def find_children(playbook, playbook_dir): break valid_tokens.append(token) path = " ".join(valid_tokens) - results.append({ - "path": path_dwim(basedir, path), - "type": child["type"] - }) + results.append({"path": path_dwim(basedir, path), "type": child["type"]}) return results def template(basedir, value, variables, fail_on_undefined=False, **kwargs): try: - value = ansible_template(os.path.abspath(basedir), value, variables, - **dict(kwargs, fail_on_undefined=fail_on_undefined)) + value = ansible_template( + os.path.abspath(basedir), value, variables, + **dict(kwargs, fail_on_undefined=fail_on_undefined) + ) # Hack to skip the following exception when using to_json filter on a variable. # I guess the filter doesn't like empty vars... except (AnsibleError, ValueError): @@ -207,10 +238,12 @@ def play_children(basedir, item, parent_type, playbook_dir): if k in delegate_map: if v: - v = template(os.path.abspath(basedir), - v, - dict(playbook_dir=os.path.abspath(basedir)), - fail_on_undefined=False) + v = template( + os.path.abspath(basedir), + v, + dict(playbook_dir=os.path.abspath(basedir)), + fail_on_undefined=False + ) return delegate_map[k](basedir, k, v, parent_type) return [] @@ -237,12 +270,23 @@ def _taskshandlers_children(basedir, k, v, parent_type): elif "import_tasks" in th: append_children(th["import_tasks"], basedir, k, parent_type, results) elif "import_role" in th: - results.extend(_roles_children(basedir, k, [th["import_role"].get("name")], parent_type, - main=th["import_role"].get("tasks_from", "main"))) + results.extend( + _roles_children( + basedir, + k, [th["import_role"].get("name")], + parent_type, + main=th["import_role"].get("tasks_from", "main") + ) + ) elif "include_role" in th: - results.extend(_roles_children(basedir, k, [th["include_role"].get("name")], - parent_type, - main=th["include_role"].get("tasks_from", "main"))) + results.extend( + _roles_children( + basedir, + k, [th["include_role"].get("name")], + parent_type, + main=th["include_role"].get("tasks_from", "main") + ) + ) elif "block" in th: results.extend(_taskshandlers_children(basedir, k, th["block"], parent_type)) if "rescue" in th: @@ -260,10 +304,7 @@ def append_children(taskhandler, basedir, k, parent_type, results): playbook_section = k else: playbook_section = parent_type - results.append({ - "path": path_dwim(basedir, taskhandler), - "type": playbook_section - }) + results.append({"path": path_dwim(basedir, taskhandler), "type": playbook_section}) def _roles_children(basedir, k, v, parent_type, main="main"): @@ -272,12 +313,16 @@ def _roles_children(basedir, k, v, parent_type, main="main"): if isinstance(role, dict): if "role" in role or "name" in role: if "tags" not in role or "skip_ansible_later" not in role["tags"]: - results.extend(_look_for_role_files(basedir, - role.get("role", role.get("name")), - main=main)) + results.extend( + _look_for_role_files( + basedir, role.get("role", role.get("name")), main=main + ) + ) else: - raise SystemExit("role dict {0} does not contain a 'role' " - "or 'name' key".format(role)) + raise SystemExit( + "role dict {0} does not contain a 'role' " + "or 'name' key".format(role) + ) else: results.extend(_look_for_role_files(basedir, role, main=main)) return results @@ -296,9 +341,7 @@ def _rolepath(basedir, role): path_dwim(basedir, os.path.join("roles", role)), path_dwim(basedir, role), # if included from roles/[role]/meta/main.yml - path_dwim( - basedir, os.path.join("..", "..", "..", "roles", role) - ), + path_dwim(basedir, os.path.join("..", "..", "..", "roles", role)), path_dwim(basedir, os.path.join("..", "..", role)) ] @@ -356,7 +399,7 @@ def normalize_task(task, filename, custom_modules=[]): ansible_action_type = task.get("__ansible_action_type__", "task") ansible_action_meta = task.get("__ansible_action_meta__", dict()) if "__ansible_action_type__" in task: - del(task["__ansible_action_type__"]) + del (task["__ansible_action_type__"]) normalized = dict() # TODO: Workaround for custom modules @@ -372,7 +415,7 @@ def normalize_task(task, filename, custom_modules=[]): # denormalize shell -> command conversion if "_uses_shell" in arguments: action = "shell" - del(arguments["_uses_shell"]) + del (arguments["_uses_shell"]) for (k, v) in list(task.items()): if k in ("action", "local_action", "args", "delegate_to") or k == action: @@ -386,7 +429,7 @@ def normalize_task(task, filename, custom_modules=[]): if "_raw_params" in arguments: normalized["action"]["__ansible_arguments__"] = arguments["_raw_params"].strip().split() - del(arguments["_raw_params"]) + del (arguments["_raw_params"]) else: normalized["action"]["__ansible_arguments__"] = list() normalized["action"].update(arguments) @@ -410,8 +453,11 @@ def action_tasks(yaml, file): block_rescue_always = ("block", "rescue", "always") tasks[:] = [task for task in tasks if all(k not in task for k in block_rescue_always)] - return [task for task in tasks if set( - ["include", "include_tasks", "import_playbook", "import_tasks"]).isdisjoint(task.keys())] + return [ + task for task in tasks + if set(["include", "include_tasks", "import_playbook", "import_tasks"] + ).isdisjoint(task.keys()) + ] def task_to_str(task): @@ -419,9 +465,11 @@ def task_to_str(task): if name: return name action = task.get("action") - args = " ".join([u"{0}={1}".format(k, v) for (k, v) in action.items() - if k not in ["__ansible_module__", "__ansible_arguments__"] - ] + action.get("__ansible_arguments__")) + args = " ".join([ + u"{0}={1}".format(k, v) + for (k, v) in action.items() + if k not in ["__ansible_module__", "__ansible_arguments__"] + ] + action.get("__ansible_arguments__")) return u"{0} {1}".format(action["__ansible_module__"], args) @@ -439,7 +487,8 @@ def extract_from_list(blocks, candidates): elif block[candidate] is not None: raise RuntimeError( "Key '%s' defined, but bad value: '%s'" % - (candidate, str(block[candidate]))) + (candidate, str(block[candidate])) + ) return results @@ -460,6 +509,7 @@ def parse_yaml_linenumbers(data, filename): The line numbers are stored in each node's LINE_NUMBER_KEY key. """ + def compose_node(parent, index): # the line number where the previous token has ended (plus empty lines) line = loader.line diff --git a/setup.cfg b/setup.cfg index bee1912..1f92f1c 100644 --- a/setup.cfg +++ b/setup.cfg @@ -10,11 +10,21 @@ default_section = THIRDPARTY known_first_party = ansiblelater sections = FUTURE,STDLIB,THIRDPARTY,FIRSTPARTY,LOCALFOLDER force_single_line = true -line_length = 110 -skip_glob = **/.env/*,**/docs/* +line_length = 99 +skip_glob = **/.env*,**/env/*,**/docs/* + +[yapf] +based_on_style = google +column_limit = 99 +dedent_closing_brackets = true +coalesce_brackets = true +split_before_logical_operator = true [tool:pytest] filterwarnings = ignore::FutureWarning ignore:.*collections.*:DeprecationWarning ignore:.*pep8.*:FutureWarning + +[coverage:run] +omit = **/tests/* diff --git a/setup.py b/setup.py index 5cc29b2..7c43ade 100644 --- a/setup.py +++ b/setup.py @@ -15,7 +15,8 @@ def get_property(prop, project): current_dir = os.path.dirname(os.path.realpath(__file__)) result = re.search( r'{}\s*=\s*[\'"]([^\'"]*)[\'"]'.format(prop), - open(os.path.join(current_dir, project, "__init__.py")).read()) + open(os.path.join(current_dir, project, "__init__.py")).read() + ) return result.group(1) @@ -28,12 +29,12 @@ def get_readme(filename="README.md"): setup( name=get_property("__project__", PACKAGE_NAME), - version=get_property("__version__", PACKAGE_NAME), description=("Reviews ansible playbooks, roles and inventories and suggests improvements."), keywords="ansible code review", + version=get_property("__version__", PACKAGE_NAME), author=get_property("__author__", PACKAGE_NAME), author_email=get_property("__email__", PACKAGE_NAME), - url="https://github.com/xoxys/ansible-later", + url=get_property("__url__", PACKAGE_NAME), license=get_property("__license__", PACKAGE_NAME), long_description=get_readme(), long_description_content_type="text/markdown", @@ -59,25 +60,9 @@ setup( include_package_data=True, zip_safe=False, install_requires=[ - "ansible", - "six", - "pyyaml", - "appdirs", - "unidiff", - "flake8", - "yamllint", - "nested-lookup", - "colorama", - "anyconfig", - "python-json-logger", - "jsonschema", - "pathspec", - "toolz" + "ansible", "six", "pyyaml", "appdirs", "unidiff", "flake8", "yamllint", "nested-lookup", + "colorama", "anyconfig", "python-json-logger", "jsonschema", "pathspec", "toolz" ], - entry_points={ - "console_scripts": [ - "ansible-later = ansiblelater.__main__:main" - ] - }, + entry_points={"console_scripts": ["ansible-later = ansiblelater.__main__:main"]}, test_suite="tests" ) diff --git a/test-requirements.txt b/test-requirements.txt index 25c2aaf..c75d784 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,11 +1,8 @@ -# open issue -# https://gitlab.com/pycqa/flake8-docstrings/issues/36 pydocstyle<4.0.0 flake8 flake8-colors flake8-blind-except flake8-builtins -flake8-colors flake8-docstrings<=3.0.0 flake8-isort flake8-logging-format @@ -17,3 +14,5 @@ pytest pytest-mock pytest-cov bandit +requests-mock +yapf diff --git a/tox.ini b/tox.ini index b0121f5..29054c3 100644 --- a/tox.ini +++ b/tox.ini @@ -15,4 +15,4 @@ deps = ansibledevel: git+https://github.com/ansible/ansible.git commands = ansible-later --help - pytest ansiblelater/tests/ --cov={toxinidir}/ansiblelater/ --no-cov-on-fail + pytest --cov={toxinidir}/ansiblelater --no-cov-on-fail From ccc9e5b6d7b8e8e1f54c674bd9db1798112f1359 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Sun, 5 Apr 2020 14:54:39 +0200 Subject: [PATCH 2/5] fix linting issues --- .flake8 | 18 +++++++++++--- ansiblelater/command/candidates.py | 39 ++++++++++++++++++++++++++++-- ansiblelater/rules/ansiblefiles.py | 6 +++-- ansiblelater/utils/yamlhelper.py | 9 +++---- 4 files changed, 58 insertions(+), 14 deletions(-) diff --git a/.flake8 b/.flake8 index 6e3a62c..83ef6c0 100644 --- a/.flake8 +++ b/.flake8 @@ -1,8 +1,18 @@ [flake8] -# Temp disable Docstring checks D101, D102, D103, D107 -ignore = E501, W503, F401, N813, D101, D102, D103, D107 -max-line-length = 110 +ignore = D102, D103, D107, D202, W503 +max-line-length = 99 inline-quotes = double -exclude = .git,.tox,__pycache__,build,dist,tests,*.pyc,*.egg-info,.cache,.eggs +exclude = + .git + .tox + __pycache__ + build + dist + tests + *.pyc + *.egg-info + .cache + .eggs + env* application-import-names = ansiblelater format = ${cyan}%(path)s:%(row)d:%(col)d${reset}: ${red_bold}%(code)s${reset} %(text)s diff --git a/ansiblelater/command/candidates.py b/ansiblelater/command/candidates.py index ed139a4..8ece4d1 100644 --- a/ansiblelater/command/candidates.py +++ b/ansiblelater/command/candidates.py @@ -189,6 +189,7 @@ class Candidate(object): class RoleFile(Candidate): + """Object classified as Ansible role file.""" def __init__(self, filename, settings={}, standards=[]): super(RoleFile, self).__init__(filename, settings, standards) @@ -203,10 +204,13 @@ class RoleFile(Candidate): class Playbook(Candidate): + """Object classified as Ansible playbook.""" + pass class Task(RoleFile): + """Object classified as Ansible task file.""" def __init__(self, filename, settings={}, standards=[]): super(Task, self).__init__(filename, settings, standards) @@ -214,6 +218,7 @@ class Task(RoleFile): class Handler(RoleFile): + """Object classified as Ansible handler file.""" def __init__(self, filename, settings={}, standards=[]): super(Handler, self).__init__(filename, settings, standards) @@ -221,10 +226,13 @@ class Handler(RoleFile): class Vars(Candidate): + """Object classified as Ansible vars file.""" + pass class Unversioned(Candidate): + """Object classified as unversioned file.""" def __init__(self, filename, settings={}, standards=[]): super(Unversioned, self).__init__(filename, settings, standards) @@ -232,50 +240,74 @@ class Unversioned(Candidate): class InventoryVars(Unversioned): + """Object classified as Ansible inventory vars.""" + pass class HostVars(InventoryVars): + """Object classified as Ansible host vars.""" + pass class GroupVars(InventoryVars): + """Object classified as Ansible group vars.""" + pass class RoleVars(RoleFile): + """Object classified as Ansible role vars.""" + pass class Meta(RoleFile): + """Object classified as Ansible meta file.""" + pass class Inventory(Unversioned): + """Object classified as Ansible inventory file.""" + pass class Code(Unversioned): + """Object classified as code file.""" + pass class Template(RoleFile): + """Object classified as Ansible template file.""" + pass class Doc(Unversioned): + """Object classified as documentation file.""" + pass class Makefile(Unversioned): + """Object classified as makefile.""" + pass class File(RoleFile): + """Object classified as generic file.""" + pass class Rolesfile(Unversioned): + """Object classified as Ansible roles file.""" + pass @@ -310,6 +342,7 @@ class Error(object): class Result(object): + """Generic result object.""" def __init__(self, candidate, errors=None): self.candidate = candidate @@ -335,8 +368,10 @@ def classify(filename, settings={}, standards=[]): return HostVars(filename, settings, standards) if parentdir in ["meta"]: return Meta(filename, settings, standards) - if parentdir in ["library", "lookup_plugins", "callback_plugins", "filter_plugins" - ] or filename.endswith(".py"): + if ( + parentdir in ["library", "lookup_plugins", "callback_plugins", "filter_plugins"] + or filename.endswith(".py") + ): return Code(filename, settings, standards) if "inventory" == basename or "hosts" == basename or parentdir in ["inventories"]: return Inventory(filename, settings, standards) diff --git a/ansiblelater/rules/ansiblefiles.py b/ansiblelater/rules/ansiblefiles.py index ce5a91e..569c61d 100644 --- a/ansiblelater/rules/ansiblefiles.py +++ b/ansiblelater/rules/ansiblefiles.py @@ -33,8 +33,10 @@ def check_braces_spaces(candidate, settings): [leading, trailing] = count_spaces(line) sum_spaces = leading + trailing - if ((sum_spaces < conf["min-spaces-inside"] * 2) - or (sum_spaces > conf["min-spaces-inside"] * 2)): + if ( + sum_spaces < conf["min-spaces-inside"] * 2 + or sum_spaces > conf["min-spaces-inside"] * 2 + ): errors.append(Error(i, description)) return Result(candidate.path, errors) diff --git a/ansiblelater/utils/yamlhelper.py b/ansiblelater/utils/yamlhelper.py index 35e002c..f64c3eb 100644 --- a/ansiblelater/utils/yamlhelper.py +++ b/ansiblelater/utils/yamlhelper.py @@ -23,7 +23,6 @@ import codecs import glob import imp -import inspect import os import ansible.parsing.mod_args @@ -453,11 +452,9 @@ def action_tasks(yaml, file): block_rescue_always = ("block", "rescue", "always") tasks[:] = [task for task in tasks if all(k not in task for k in block_rescue_always)] - return [ - task for task in tasks - if set(["include", "include_tasks", "import_playbook", "import_tasks"] - ).isdisjoint(task.keys()) - ] + allowed = ["include", "include_tasks", "import_playbook", "import_tasks"] + + return [task for task in tasks if set(allowed).isdisjoint(task.keys())] def task_to_str(task): From dc0af76559acaf3c06d0b4d3a2966ee03a8737b9 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Sun, 5 Apr 2020 14:57:44 +0200 Subject: [PATCH 3/5] update drone config --- .drone.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.drone.yml b/.drone.yml index 0634c50..fb3910d 100644 --- a/.drone.yml +++ b/.drone.yml @@ -74,9 +74,9 @@ steps: - name: codecov image: python:3.7 commands: - - pip install codecov + - pip install codecov -qq - coverage combine .tox/py*/.coverage - - codecov --required + - codecov --required -X gcov environment: CODECOV_TOKEN: from_secret: codecov_token @@ -470,6 +470,6 @@ depends_on: --- kind: signature -hmac: 4b5f6077a3352113853e936f62396caf68d9a7a2014245f6b24cca6ae0fa8b3b +hmac: 03c91a03dab38f62ce4e792238c87e34ac583fa570ff806bf4208df7ed579cd8 ... From e1ff6d2e73641addec59862ad0287afc8dac99a0 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Sun, 5 Apr 2020 15:24:51 +0200 Subject: [PATCH 4/5] slightly rework of the log output formatting --- CHANGELOG.md | 6 +++--- ansiblelater/logger.py | 12 +++++++----- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 98b8d13..e0abdf7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,3 @@ -* BUGFIX - * replace removed dependency `ansible.module_utils.parsing.convert_bool` - * decode task actions to prevent errors on multiline strings +* ENHANCEMENT + * improve ANSIBLE0001 logic to avoid false positive results + * improve log output readability diff --git a/ansiblelater/logger.py b/ansiblelater/logger.py index c96352b..bc01180 100644 --- a/ansiblelater/logger.py +++ b/ansiblelater/logger.py @@ -9,7 +9,7 @@ import colorama from pythonjsonlogger import jsonlogger from six import iteritems -CONSOLE_FORMAT = "%(levelname)s: %(message)s" +CONSOLE_FORMAT = "%(levelname)s: {}%(message)s" JSON_FORMAT = "(asctime) (levelname) (message)" @@ -63,6 +63,7 @@ class MultilineFormatter(logging.Formatter): def format(self, record): # noqa record.msg = record.msg.replace("\n", "\n{}... ".format(colorama.Style.RESET_ALL)) + record.msg = record.msg + "\n" return logging.Formatter.format(self, record) @@ -157,22 +158,22 @@ def _get_critical_handler(json=False): def critical(message): """Format critical messages and return string.""" - return color_text(colorama.Fore.RED, "{}".format(message)) + return color_text(colorama.Fore.RED, message) def error(message): """Format error messages and return string.""" - return color_text(colorama.Fore.RED, "{}".format(message)) + return color_text(colorama.Fore.RED, message) def warn(message): """Format warn messages and return string.""" - return color_text(colorama.Fore.YELLOW, "{}".format(message)) + return color_text(colorama.Fore.YELLOW, message) def info(message): """Format info messages and return string.""" - return color_text(colorama.Fore.BLUE, "{}".format(message)) + return color_text(colorama.Fore.BLUE, message) def color_text(color, msg): @@ -184,4 +185,5 @@ def color_text(color, msg): :returns: string """ + msg = msg.format(colorama.Style.BRIGHT) return "{}{}{}".format(color, msg, colorama.Style.RESET_ALL) From fddb65d98b558834d277780db8167ed7d88f8a71 Mon Sep 17 00:00:00 2001 From: Robert Kaussow Date: Sun, 5 Apr 2020 15:42:48 +0200 Subject: [PATCH 5/5] fix pytest --- ansiblelater/tests/unit/test_logger.py | 24 ++++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/ansiblelater/tests/unit/test_logger.py b/ansiblelater/tests/unit/test_logger.py index befcfc0..440c19a 100644 --- a/ansiblelater/tests/unit/test_logger.py +++ b/ansiblelater/tests/unit/test_logger.py @@ -21,7 +21,11 @@ def test_critical(capsys, mocker): log.critical("foo") _, stderr = capsys.readouterr() - print("{}{}{}".format(colorama.Fore.RED, "CRITICAL: foo".rstrip(), colorama.Style.RESET_ALL)) + print( + "{}CRITICAL: {}foo\n{}".format( + colorama.Fore.RED, colorama.Style.BRIGHT, colorama.Style.RESET_ALL + ) + ) x, _ = capsys.readouterr() assert x == stderr @@ -32,7 +36,11 @@ def test_error(capsys, mocker): log.error("foo") _, stderr = capsys.readouterr() - print("{}{}{}".format(colorama.Fore.RED, "ERROR: foo".rstrip(), colorama.Style.RESET_ALL)) + print( + "{}ERROR: {}foo\n{}".format( + colorama.Fore.RED, colorama.Style.BRIGHT, colorama.Style.RESET_ALL + ) + ) x, _ = capsys.readouterr() assert x == stderr @@ -43,7 +51,11 @@ def test_warn(capsys, mocker): log.warning("foo") stdout, _ = capsys.readouterr() - print("{}{}{}".format(colorama.Fore.YELLOW, "WARNING: foo".rstrip(), colorama.Style.RESET_ALL)) + print( + "{}WARNING: {}foo\n{}".format( + colorama.Fore.YELLOW, colorama.Style.BRIGHT, colorama.Style.RESET_ALL + ) + ) x, _ = capsys.readouterr() assert x == stdout @@ -54,7 +66,11 @@ def test_info(capsys, mocker): log.info("foo") stdout, _ = capsys.readouterr() - print("{}{}{}".format(colorama.Fore.BLUE, "INFO: foo".rstrip(), colorama.Style.RESET_ALL)) + print( + "{}INFO: {}foo\n{}".format( + colorama.Fore.BLUE, colorama.Style.BRIGHT, colorama.Style.RESET_ALL + ) + ) x, _ = capsys.readouterr() assert x == stdout