From 592da84e8565e228f26148d1744c6c453cf2b55b Mon Sep 17 00:00:00 2001
From: Robert Kaussow <robert.kaussow@mail.schwarz>
Date: Mon, 1 Apr 2019 15:07:22 +0200
Subject: [PATCH] complete rewrite for settings and logging

---
 ansiblelater/__init__.py                      | 17 ++--
 ansiblelater/__main__.py                      | 34 +++----
 {tests => ansiblelater/command}/__init__.py   |  0
 ansiblelater/command/base.py                  |  9 ++
 ansiblelater/{examples => data}/standards.py  |  0
 ansiblelater/{utils => }/exceptions.py        | 15 ++-
 ansiblelater/logger.py                        | 68 +++++++++-----
 ansiblelater/settings.py                      | 94 ++++++++++++-------
 ansiblelater/tests/__init__.py                |  0
 .../tests}/config/config.ini                  |  0
 .../tests}/config/standards.py                |  0
 .../tests}/data/yaml_fail.yml                 |  0
 .../tests}/data/yaml_success.yml              |  0
 .../tests}/unit/test_logging.py               |  0
 ansiblelater/utils/rulehelper.py              |  2 +-
 ansiblelater/utils/yamlhelper.py              |  2 +-
 {ansiblelater/bin => bin}/ansible-later       |  1 +
 setup.py                                      |  4 +-
 test-requirements.txt                         |  2 +-
 19 files changed, 143 insertions(+), 105 deletions(-)
 rename {tests => ansiblelater/command}/__init__.py (100%)
 create mode 100644 ansiblelater/command/base.py
 rename ansiblelater/{examples => data}/standards.py (100%)
 rename ansiblelater/{utils => }/exceptions.py (61%)
 create mode 100644 ansiblelater/tests/__init__.py
 rename {tests => ansiblelater/tests}/config/config.ini (100%)
 rename {tests => ansiblelater/tests}/config/standards.py (100%)
 rename {tests => ansiblelater/tests}/data/yaml_fail.yml (100%)
 rename {tests => ansiblelater/tests}/data/yaml_success.yml (100%)
 rename {tests => ansiblelater/tests}/unit/test_logging.py (100%)
 rename {ansiblelater/bin => bin}/ansible-later (99%)

diff --git a/ansiblelater/__init__.py b/ansiblelater/__init__.py
index bc98a53..09236aa 100644
--- a/ansiblelater/__init__.py
+++ b/ansiblelater/__init__.py
@@ -14,13 +14,12 @@ import re
 from distutils.version import LooseVersion
 
 import ansible
-from appdirs import AppDirs
 
-from . import logger
-from .settings import Settings
-from ansiblelater.utils import (get_property,
-                                is_line_in_ranges, lines_ranges,
+from ansiblelater.utils import (get_property, is_line_in_ranges, lines_ranges,
                                 read_standards, standards_latest)
+from ansiblelater.exceptions import (  # noqa
+    LaterError, LaterAnsibleError
+)
 
 try:
     # Ansible 2.4 import of module loader
@@ -31,16 +30,13 @@ except ImportError:
     except ImportError:
         from ansible.utils import module_finder as module_loader
 
-settings = Settings()
-logger = logger.get_logger(__name__, settings.config["logging"]["level"])
-
 
 class Standard(object):
     """
     Standard definition for all defined rules.
 
     Later lookup the config file for a path to a rules directory
-    or fallback to default `ansiblelater/examples/*`.
+    or fallback to default `ansiblelater/data/*`.
     """
 
     def __init__(self, standard_dict):
@@ -48,6 +44,7 @@ class Standard(object):
         Initialize a new standard object and returns None.
 
         :param standard_dict: Dictionary object containing all neseccary attributes
+
         """
         if "id" not in standard_dict:
             standard_dict.update(id="")
@@ -108,6 +105,7 @@ class Error(object):
 
         :param lineno: Line number where the error from de rule occures
         :param message: Detailed error description provided by the rule
+
         """
         self.lineno = lineno
         self.message = message
@@ -208,7 +206,6 @@ class Doc(Unversioned):
     pass
 
 
-# For ease of checking files for tabs
 class Makefile(Unversioned):
     pass
 
diff --git a/ansiblelater/__main__.py b/ansiblelater/__main__.py
index 6de6bdb..87be626 100755
--- a/ansiblelater/__main__.py
+++ b/ansiblelater/__main__.py
@@ -4,42 +4,30 @@ import argparse
 import json
 import logging
 import os
-import sys
 
-from ansiblelater import __version__, settings, logger
-from ansiblelater.utils import get_property
-
-# from .settings import Settings
+from ansiblelater import __version__
+from ansiblelater.command import base
 
 
 def main():
     parser = argparse.ArgumentParser(
         description="Validate ansible files against best pratice guideline")
-    parser.add_argument('-c', dest='config_file',
-                        help="Location of configuration file: [%s]" % settings.config_file)
-    parser.add_argument('-d', dest='rules.standards',
+    parser.add_argument("-c", dest="config_file",
+                        help="Location of configuration file")
+    parser.add_argument("-d", dest="rules.standards",
                         help="Location of standards rules")
-    parser.add_argument('-q', dest='logging.level', action="store_const",
+    parser.add_argument("-q", dest="logging.level", action="store_const",
                         const=logging.ERROR, help="Only output errors")
-    parser.add_argument('-s', dest='rules.filter', action='append',
+    parser.add_argument("-s", dest="rules.filter", action="append",
                         help="limit standards to specific names")
-    parser.add_argument('-v', '--verbose', dest='logging.level', action="count",
+    parser.add_argument("-v", "--verbose", dest="logging.level", action="count",
                         help="Show more verbose output")
-    parser.add_argument('--version', action='version', version='%(prog)s {}'.format(__version__))
+    parser.add_argument("--version", action="version", version="%(prog)s {}".format(__version__))
 
     args = parser.parse_args().__dict__
 
-    # Override correct log level from argparse
-    levels = [logging.WARNING, logging.INFO, logging.DEBUG]
-    if args.get("logging.level"):
-        args["logging.level"] = levels[min(len(levels) - 1, args["logging.level"] - 1)]
-
-    settings.set_args(args)
-
-    # print(json.dumps(settings.config, indent=4, sort_keys=True))
-    # print(settings.config["logging"]["level"])
-
-
+    settings = base.get_settings(args)
+    print(json.dumps(settings.config, indent=4, sort_keys=True))
 
     # if len(args) == 0:
     #     candidates = []
diff --git a/tests/__init__.py b/ansiblelater/command/__init__.py
similarity index 100%
rename from tests/__init__.py
rename to ansiblelater/command/__init__.py
diff --git a/ansiblelater/command/base.py b/ansiblelater/command/base.py
new file mode 100644
index 0000000..351d884
--- /dev/null
+++ b/ansiblelater/command/base.py
@@ -0,0 +1,9 @@
+from ansiblelater import settings
+
+
+def get_settings(args):
+    config = settings.Settings(
+        args=args,
+    )
+
+    return config
diff --git a/ansiblelater/examples/standards.py b/ansiblelater/data/standards.py
similarity index 100%
rename from ansiblelater/examples/standards.py
rename to ansiblelater/data/standards.py
diff --git a/ansiblelater/utils/exceptions.py b/ansiblelater/exceptions.py
similarity index 61%
rename from ansiblelater/utils/exceptions.py
rename to ansiblelater/exceptions.py
index a99902d..4d13f19 100644
--- a/ansiblelater/utils/exceptions.py
+++ b/ansiblelater/exceptions.py
@@ -1,23 +1,28 @@
+"""Custom exceptions."""
+
 import re
 
 
-# Custom exceptions
 class LaterError(Exception):
-    """Generic exception for later"""
+    """Generic exception for later."""
 
     def __init__(self, msg, original):
+        """
+        Initialize new exception.
+
+        """
         super(LaterError, self).__init__(msg + (": %s" % original))
         self.original = original
 
 
 class LaterAnsibleError(Exception):
-    """Wrapper for ansible syntax errors"""
+    """Wrapper for ansible syntax errors."""
 
     def __init__(self, msg, original):
         lines = original.message.splitlines()
 
-        line_no = re.search('line(.*?),', lines[2])
-        column_no = re.search('column(.*?),', lines[2])
+        line_no = re.search("line(.*?),", lines[2])
+        column_no = re.search("column(.*?),", lines[2])
 
         self.message = lines[0]
         self.line = line_no.group(1).strip()
diff --git a/ansiblelater/logger.py b/ansiblelater/logger.py
index a609ac0..0a9b705 100644
--- a/ansiblelater/logger.py
+++ b/ansiblelater/logger.py
@@ -1,33 +1,40 @@
+"""Global logging helpers."""
+
 import logging
 import os
 import sys
 
 import colorama
-from pythonjsonlogger import jsonlogger
 from ansible.module_utils.parsing.convert_bool import boolean as to_bool
+from pythonjsonlogger import jsonlogger
+
 
+def _should_do_markup():
 
-def should_do_markup():
-    py_colors = os.environ.get('PY_COLORS', None)
+    py_colors = os.environ.get("PY_COLORS", None)
     if py_colors is not None:
         return to_bool(py_colors, strict=False)
 
-    return sys.stdout.isatty() and os.environ.get('TERM') != 'dumb'
+    return sys.stdout.isatty() and os.environ.get("TERM") != "dumb"
 
 
-colorama.init(autoreset=True, strip=not should_do_markup())
+colorama.init(autoreset=True, strip=not _should_do_markup())
 
 
 class LogFilter(object):
-    """
-    A custom log filter which excludes log messages above the logged
-    level.
-    """
+    """A custom log filter which excludes log messages above the logged level."""
 
     def __init__(self, level):
+        """
+        Initialize a new custom log filter.
+
+        :param level: Log level limit
+        :returns: None
+
+        """
         self.__level = level
 
-    def filter(self, logRecord):  # pragma: no cover
+    def filter(self, logRecord):  # noqa
         # https://docs.python.org/3/library/logging.html#logrecord-attributes
         return logRecord.levelno <= self.__level
 
@@ -35,17 +42,15 @@ class LogFilter(object):
 def get_logger(name=None, level=logging.DEBUG, json=False):
     """
     Build a logger with the given name and returns the logger.
-    :param name: The name for the logger. This is usually the module
-                 name, ``__name__``.
+
+    :param name: The name for the logger. This is usually the module name, `__name__`.
+    :param level: Initialize the new logger with given log level.
+    :param json: Boolean flag to enable json formatted log output.
     :return: logger object
-    """
 
+    """
     logger = logging.getLogger(name)
     logger.setLevel(level)
-    #handler = logging.StreamHandler()
-    #formatter = jsonlogger.JsonFormatter("%(asctime)s - %(name)s - %(levelname)s - %(message)s")
-    #formatter = logging.Formatter("%(asctime)s - %(name)s - %(levelname)s - %(message)s")
-    #handler.setFormatter(formatter)
     logger.addHandler(_get_error_handler(json=json))
     logger.addHandler(_get_warn_handler(json=json))
     logger.addHandler(_get_info_handler(json=json))
@@ -58,10 +63,10 @@ def _get_error_handler(json=False):
     handler = logging.StreamHandler(sys.stderr)
     handler.setLevel(logging.ERROR)
     handler.addFilter(LogFilter(logging.ERROR))
-    handler.setFormatter(logging.Formatter(error('%(message)s')))
+    handler.setFormatter(logging.Formatter(error("%(message)s")))
 
     if json:
-        handler.setFormatter(jsonlogger.JsonFormatter('%(message)s'))
+        handler.setFormatter(jsonlogger.JsonFormatter("%(message)s"))
 
     return handler
 
@@ -70,10 +75,10 @@ def _get_warn_handler(json=False):
     handler = logging.StreamHandler(sys.stdout)
     handler.setLevel(logging.WARN)
     handler.addFilter(LogFilter(logging.WARN))
-    handler.setFormatter(logging.Formatter(warn('%(message)s')))
+    handler.setFormatter(logging.Formatter(warn("%(message)s")))
 
     if json:
-        handler.setFormatter(jsonlogger.JsonFormatter('%(message)s'))
+        handler.setFormatter(jsonlogger.JsonFormatter("%(message)s"))
 
     return handler
 
@@ -82,30 +87,41 @@ def _get_info_handler(json=False):
     handler = logging.StreamHandler(sys.stderr)
     handler.setLevel(logging.INFO)
     handler.addFilter(LogFilter(logging.INFO))
-    handler.setFormatter(logging.Formatter(info('%(message)s')))
+    handler.setFormatter(logging.Formatter(info("%(message)s")))
 
     if json:
-        handler.setFormatter(jsonlogger.JsonFormatter('%(message)s'))
+        handler.setFormatter(jsonlogger.JsonFormatter("%(message)s"))
 
     return handler
 
 
-def abort(message, file=sys.stderr):
+def abort(message):
+    """Format abort messages and return string."""
     return color_text(colorama.Fore.RED, "FATAL: {}".format(message))
-    sys.exit(1)
 
 
 def error(message):
+    """Format error messages and return string."""
     return color_text(colorama.Fore.RED, "ERROR: {}".format(message))
 
 
 def warn(message):
+    """Format warn messages and return string."""
     return color_text(colorama.Fore.YELLOW, "WARN: {}".format(message))
 
 
 def info(message):
+    """Format info messages and return string."""
     return color_text(colorama.Fore.BLUE, "INFO: {}".format(message))
 
 
 def color_text(color, msg):
-    return '{}{}{}'.format(color, msg, colorama.Style.RESET_ALL)
+    """
+    Colorize strings.
+
+    :param color: colorama color settings
+    :param msg: string to colorize
+    :returns: string
+
+    """
+    return "{}{}{}".format(color, msg, colorama.Style.RESET_ALL)
diff --git a/ansiblelater/settings.py b/ansiblelater/settings.py
index ece6e67..7eba4a5 100644
--- a/ansiblelater/settings.py
+++ b/ansiblelater/settings.py
@@ -1,12 +1,14 @@
+"""Global settings object definition."""
+
 import logging
 import os
-import six
 
 import anyconfig
 from appdirs import AppDirs
+from jsonschema._utils import format_as_index
 from pkg_resources import resource_filename
 
-from ansiblelater import utils, logger
+from ansiblelater import logger, utils
 
 config_dir = AppDirs("ansible-later").user_config_dir
 default_config_file = os.path.join(config_dir, "config.yml")
@@ -14,33 +16,47 @@ default_config_file = os.path.join(config_dir, "config.yml")
 logger = logger.get_logger(__name__)
 
 
-class NewInitCaller(type):
-    def __call__(cls, *args, **kwargs):
-        obj = type.__call__(cls, *args, **kwargs)
-        obj.after_init()
-        return obj
+class Settings(object):
+    """
+    Create an object with all necessary settings.
 
+    Settings are loade from multiple locations in defined order (last wins):
+    - default settings defined by `self._get_defaults()`
+    - yaml config file, defaults to OS specific user config dir (https://pypi.org/project/appdirs/)
+    - provides cli parameters
+    """
 
-@six.add_metaclass(NewInitCaller)
-class Settings(object):
     def __init__(self, args={}, config_file=default_config_file):
-        self.args = args
+        """
+        Initialize a new settings class.
+
+        :param args: An optional dict of options, arguments and commands from the CLI.
+        :param config_file: An optional path to a yaml config file.
+        :returns: None
+
+        """
         self.config_file = config_file
+        self.args = self._set_args(args)
         self.config = self._get_config()
+        self.schema = None
 
-    def set_args(self, args={}):
+    def _set_args(self, args):
         self.config_file = args.get("config_file") or default_config_file
 
         args.pop("config_file", None)
-        args = dict(filter(lambda item: item[1] is not None, args.items()))
+        tmp_args = dict(filter(lambda item: item[1] is not None, args.items()))
 
-        args_dict = {}
-        for key, value in args.items():
-            args_dict = utils.add_dict_branch(args_dict, key.split("."), value)
+        tmp_dict = {}
+        for key, value in tmp_args.items():
+            tmp_dict = utils.add_dict_branch(tmp_dict, key.split("."), value)
 
-        self.args = args_dict
-        self.config = self._get_config()
-        self._validate()
+        # Override correct log level from argparse
+        levels = [logging.WARNING, logging.INFO, logging.DEBUG]
+        if tmp_dict.get("logging"):
+            tmp_dict["logging"]["level"] = levels[
+                min(len(levels) - 1, tmp_dict["logging"]["level"] - 1)]
+
+        return tmp_dict
 
     def _get_config(self):
         defaults = self._get_defaults()
@@ -50,33 +66,39 @@ class Settings(object):
         if config_file and os.path.exists(config_file):
             with utils.open_file(config_file) as stream:
                 s = stream.read()
-                anyconfig.merge(defaults, utils.safe_load(s), ac_merge=anyconfig.MS_DICTS)
+                if self._validate(utils.safe_load(s)):
+                    anyconfig.merge(defaults, utils.safe_load(s), ac_merge=anyconfig.MS_DICTS)
 
-        if cli_options:
+        if cli_options and self._validate(cli_options):
             anyconfig.merge(defaults, cli_options, ac_merge=anyconfig.MS_DICTS)
 
         return defaults
 
     def _get_defaults(self):
-        rules_dir = os.path.join(resource_filename('ansiblelater', 'examples'))
-
-        return {
-            'rules': {
-                'standards': rules_dir,
-                'filter': [],
+        rules_dir = os.path.join(resource_filename("ansiblelater", "data"))
+        defaults = {
+            "rules": {
+                "standards": rules_dir,
+                "filter": [],
             },
-            'logging': {
-                'level': logging.WARN,
+            "logging": {
+                "level": logging.WARN,
             },
-            'ansible': {
-                'custom_modules': [],
+            "ansible": {
+                "custom_modules": [],
             }
         }
+        self.schema = anyconfig.gen_schema(defaults)
 
-    def after_init(self):
-        self.config = self._get_config()
-        self._validate()
-
-    def _validate(self):
-        logger.setLevel(self.config["logging"]["level"])
+        return defaults
 
+    def _validate(self, config):
+        try:
+            anyconfig.validate(config, self.schema, ac_schema_safe=False)
+            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])
+            )
+            logger.error("{schema}: {msg}".format(schema=schema_error, msg=e.message))
diff --git a/ansiblelater/tests/__init__.py b/ansiblelater/tests/__init__.py
new file mode 100644
index 0000000..e69de29
diff --git a/tests/config/config.ini b/ansiblelater/tests/config/config.ini
similarity index 100%
rename from tests/config/config.ini
rename to ansiblelater/tests/config/config.ini
diff --git a/tests/config/standards.py b/ansiblelater/tests/config/standards.py
similarity index 100%
rename from tests/config/standards.py
rename to ansiblelater/tests/config/standards.py
diff --git a/tests/data/yaml_fail.yml b/ansiblelater/tests/data/yaml_fail.yml
similarity index 100%
rename from tests/data/yaml_fail.yml
rename to ansiblelater/tests/data/yaml_fail.yml
diff --git a/tests/data/yaml_success.yml b/ansiblelater/tests/data/yaml_success.yml
similarity index 100%
rename from tests/data/yaml_success.yml
rename to ansiblelater/tests/data/yaml_success.yml
diff --git a/tests/unit/test_logging.py b/ansiblelater/tests/unit/test_logging.py
similarity index 100%
rename from tests/unit/test_logging.py
rename to ansiblelater/tests/unit/test_logging.py
diff --git a/ansiblelater/utils/rulehelper.py b/ansiblelater/utils/rulehelper.py
index 024068c..14cb0cd 100644
--- a/ansiblelater/utils/rulehelper.py
+++ b/ansiblelater/utils/rulehelper.py
@@ -11,7 +11,7 @@ from .yamlhelper import normalize_task
 from .yamlhelper import action_tasks
 from .yamlhelper import parse_yaml_linenumbers
 from .yamlhelper import normalized_yaml
-from .exceptions import LaterError, LaterAnsibleError
+from ansiblelater import LaterError, LaterAnsibleError
 
 
 def get_tasks(candidate, settings):
diff --git a/ansiblelater/utils/yamlhelper.py b/ansiblelater/utils/yamlhelper.py
index e541674..9e9def0 100644
--- a/ansiblelater/utils/yamlhelper.py
+++ b/ansiblelater/utils/yamlhelper.py
@@ -28,7 +28,7 @@ import six
 import ansible.parsing.mod_args
 from ansible import constants
 from ansible.errors import AnsibleError
-from .exceptions import LaterError, LaterAnsibleError
+from ansiblelater import LaterAnsibleError, LaterError
 
 try:
     # Try to import the Ansible 2 module first, it's the future-proof one
diff --git a/ansiblelater/bin/ansible-later b/bin/ansible-later
similarity index 99%
rename from ansiblelater/bin/ansible-later
rename to bin/ansible-later
index 5af873b..623e95b 100755
--- a/ansiblelater/bin/ansible-later
+++ b/bin/ansible-later
@@ -1,6 +1,7 @@
 #!/usr/bin/env python
 
 import sys
+
 import ansiblelater.__main__
 
 sys.exit(ansiblelater.__main__.main())
diff --git a/setup.py b/setup.py
index b59b112..a128b48 100644
--- a/setup.py
+++ b/setup.py
@@ -36,7 +36,7 @@ setup(
     license=get_property("__license__", PACKAGE_NAME),
     long_description=get_readme(),
     long_description_content_type='text/markdown',
-    packages=find_packages(exclude=["test", "test.*"]),
+    packages=find_packages(exclude=["tests", "tests.*"]),
     python_requires='>=2.7,!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*,,!=3.4.*',
     classifiers=[
         'Development Status :: 5 - Production/Stable',
@@ -74,5 +74,5 @@ setup(
             'ansible-later = ansiblelater.__main__:main'
         ]
     },
-    test_suite="test"
+    test_suite="tests"
 )
diff --git a/test-requirements.txt b/test-requirements.txt
index 9c19226..5caab59 100644
--- a/test-requirements.txt
+++ b/test-requirements.txt
@@ -3,7 +3,7 @@ flake8-colors
 flake8-blind-except
 flake8-builtins
 flake8-colors
-flake8-docstrings
+flake8-docstrings<=3.0.0
 flake8-isort
 flake8-logging-format
 flake8-polyfill