From 1c38b7f8b054b55e866e7964582aa3b4c8012e9f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0=C4=8Dotka?= Date: Thu, 31 May 2018 16:04:40 +0200 Subject: [PATCH 01/15] add fmf metadata support to colin move fmf metadata loader outsite, to be able to use it more generic add more flexible functions to deal with default values and init values from FMF fist step with meta classes for testing via nosetest changed all core.checks to support fmf input format added fmf file for best practices and added init to all files fix defaut values, remove them from init_list cleanup of fmf init method first working version with fmf metadata for few cases added relevancy filtering to fmf scheduler add fmf rulesets via referencing shorter output when using nose added docs strings and few fixes based on reviews --- colin/.fmf/version | 1 + colin/checks/best_practices.fmf | 24 ++++++ colin/checks/best_practices.py | 28 ------- colin/checks/dockerfile.fmf | 19 +++++ colin/checks/dockerfile.py | 17 ---- colin/checks/labels.py | 10 --- colin/core/checks/abstract_check.py | 43 ++++++++-- colin/core/checks/cmd.py | 14 ++-- colin/core/checks/dockerfile.py | 39 ++++----- colin/core/checks/envs.py | 12 +-- colin/core/checks/filesystem.py | 14 +--- colin/core/checks/labels.py | 20 ++--- colin/core/fmf_metadata_loader.py | 74 +++++++++++++++++ colin/core/loader.py | 2 +- colin/default.fmf | 16 ++++ fmf_scheduler.py | 124 ++++++++++++++++++++++++++++ setup.py | 5 +- tests/unit/test_loader.py | 4 +- 18 files changed, 336 insertions(+), 130 deletions(-) create mode 100644 colin/.fmf/version create mode 100644 colin/checks/best_practices.fmf create mode 100644 colin/checks/dockerfile.fmf create mode 100644 colin/core/fmf_metadata_loader.py create mode 100644 colin/default.fmf create mode 100644 fmf_scheduler.py diff --git a/colin/.fmf/version b/colin/.fmf/version new file mode 100644 index 00000000..d00491fd --- /dev/null +++ b/colin/.fmf/version @@ -0,0 +1 @@ +1 diff --git a/colin/checks/best_practices.fmf b/colin/checks/best_practices.fmf new file mode 100644 index 00000000..0ed1bb3e --- /dev/null +++ b/colin/checks/best_practices.fmf @@ -0,0 +1,24 @@ +test: "best_practices.py" + +/cmd_or_entrypoint: + class: "CmdOrEntrypointCheck" + message: "Cmd or Entrypoint has to be specified" + description: "An ENTRYPOINT allows you to configure a container that will run as an executable. The main purpose of a CMD is to provide defaults for an executing container." + reference_url: "https://fedoraproject.org/wiki/Container:Guidelines#CMD.2FENTRYPOINT_2" + tags: ["cmd", "entrypoint"] + +/help_file_or_readme: + class: "HelpFileOrReadmeCheck" + message: "The 'helpfile' has to be provided." + description: "Just like traditional packages, containers need some 'man page' information about how they are to be used, configured, and integrated into a larger stack." + reference_url: "https://fedoraproject.org/wiki/Container:Guidelines#Help_File" + files: ['/help.1', '/README.md'] + tags: ['filesystem', 'helpfile', 'man'] + all_must_be_present: False + +/no_root: + class: "NoRootCheck" + message: "Service should not run as root by default." + description: "It can be insecure to run service as root." + reference_url: "?????" + tags: ["root", "user"] \ No newline at end of file diff --git a/colin/checks/best_practices.py b/colin/checks/best_practices.py index 7aaa39ae..8a6dd823 100644 --- a/colin/checks/best_practices.py +++ b/colin/checks/best_practices.py @@ -29,16 +29,6 @@ class CmdOrEntrypointCheck(ContainerAbstractCheck, ImageAbstractCheck): name = "cmd_or_entrypoint" - def __init__(self): - super(CmdOrEntrypointCheck, self) \ - .__init__(message="Cmd or Entrypoint has to be specified", - description="An ENTRYPOINT allows you to configure a container" - " that will run as an executable. The main purpose" - " of a CMD is to provide defaults for an executing container.", - reference_url="https://fedoraproject.org/wiki/Container:Guidelines" - "#CMD.2FENTRYPOINT_2", - tags=["cmd", "entrypoint"]) - def check(self, target): metadata = inspect_object(target.instance)["Config"] cmd_present = "Cmd" in metadata and metadata["Cmd"] @@ -62,28 +52,10 @@ def check(self, target): class HelpFileOrReadmeCheck(FileCheck): name = "help_file_or_readme" - def __init__(self): - super(HelpFileOrReadmeCheck, self) \ - .__init__(message="The 'helpfile' has to be provided.", - description="Just like traditional packages, containers need " - "some 'man page' information about how they are to be used," - " configured, and integrated into a larger stack.", - reference_url="https://fedoraproject.org/wiki/Container:Guidelines#Help_File", - files=['/help.1', '/README.md'], - tags=['filesystem', 'helpfile', 'man'], - all_must_be_present=False) - class NoRootCheck(ContainerAbstractCheck, ImageAbstractCheck): name = "no_root" - def __init__(self): - super(NoRootCheck, self) \ - .__init__(message="Service should not run as root by default.", - description="It can be insecure to run service as root.", - reference_url="?????", - tags=["root", "user"]) - def check(self, target): metadata = inspect_object(target.instance)["Config"] root_present = "User" in metadata and metadata["User"] in ["", "0", "root"] diff --git a/colin/checks/dockerfile.fmf b/colin/checks/dockerfile.fmf new file mode 100644 index 00000000..04f900da --- /dev/null +++ b/colin/checks/dockerfile.fmf @@ -0,0 +1,19 @@ +description: "Generic desription for dockerfile test if not specific description given" + +/from_tag_not_latest: + test: "dockerfile.py" + class: "FromTagNotLatestCheck" + message: "In FROM, tag has to be specified and not 'latest'." + description: "Using the 'latest' tag may cause unpredictable builds. It is recommended that a specific tag is used in the FROM." + reference_url: "https://fedoraproject.org/wiki/Container:Guidelines#FROM" + tags: ["from", "dockerfile", "baseimage", "latest"] + +/maintainer_deprecated: + test: "dockerfile.py" + class: "MaintainerDeprecatedCheck" + message: "Dockerfile instruction `MAINTAINER` is deprecated." + description: "Replace with label 'maintainer'." + reference_url: "https://docs.docker.com/engine/reference/builder/#maintainer-deprecated" + tags: ["maintainer", "dockerfile", "deprecated"] + instruction: "MAINTAINER" + max_count: 0 diff --git a/colin/checks/dockerfile.py b/colin/checks/dockerfile.py index 91e2487c..573e700a 100644 --- a/colin/checks/dockerfile.py +++ b/colin/checks/dockerfile.py @@ -22,14 +22,6 @@ class FromTagNotLatestCheck(DockerfileAbstractCheck): name = "from_tag_not_latest" - def __init__(self): - super(FromTagNotLatestCheck, self) \ - .__init__(message="In FROM, tag has to be specified and not 'latest'.", - description="Using the 'latest' tag may cause unpredictable builds." - "It is recommended that a specific tag is used in the FROM.", - reference_url="https://fedoraproject.org/wiki/Container:Guidelines#FROM", - tags=["from", "dockerfile", "baseimage", "latest"]) - def check(self, target): im = ImageName.parse(target.instance.baseimage) passed = im.tag and im.tag != "latest" @@ -43,12 +35,3 @@ def check(self, target): class MaintainerDeprecatedCheck(InstructionCountAbstractCheck): name = "maintainer_deprecated" - - def __init__(self): - super(MaintainerDeprecatedCheck, self) \ - .__init__(message="Dockerfile instruction `MAINTAINER` is deprecated.", - description="Replace with label 'maintainer'.", - reference_url="https://docs.docker.com/engine/reference/builder/#maintainer-deprecated", - tags=["maintainer", "dockerfile", "deprecated"], - instruction="MAINTAINER", - max_count=0) diff --git a/colin/checks/labels.py b/colin/checks/labels.py index 6977a552..2086c7af 100644 --- a/colin/checks/labels.py +++ b/colin/checks/labels.py @@ -231,16 +231,6 @@ def __init__(self): class MaintainerLabelCheck(LabelAbstractCheck): name = "maintainer_label" - def __init__(self): - super(MaintainerLabelCheck, self) \ - .__init__(message="Label 'maintainer' has to be specified.", - description="The name and email of the maintainer (usually the submitter).", - reference_url="https://fedoraproject.org/wiki/Container:Guidelines#LABELS", - tags=["maintainer", "label"], - labels=["maintainer"], - required=True, - value_regex=None) - class NameLabelCheck(LabelAbstractCheck): name = "name_label" diff --git a/colin/core/checks/abstract_check.py b/colin/core/checks/abstract_check.py index d03f2b1d..bc682526 100644 --- a/colin/core/checks/abstract_check.py +++ b/colin/core/checks/abstract_check.py @@ -16,16 +16,49 @@ import json from six import iteritems +from colin.core.fmf_metadata_loader import get_fmf_from_class, set_fmf_metadata class AbstractCheck(object): name = None + message = "" + description = "" + reference_url = "" + tags = [] + base_init_list = ["message", "description", "reference_url", "tags"] + init_list = [] - def __init__(self, message, description, reference_url, tags): - self.message = message - self.description = description - self.reference_url = reference_url - self.tags = tags + def _meta_init(self, *args, **kwargs): + """ + This method allows to use normal way via init parameters or alternative way of init via FMF format + Default is to use args kwargs values, in case not given it tries to search for FMF metadata + :param args: + :param kwargs: + :return: + """ + required_value_list = list(set(self.base_init_list + self.init_list)) + if len(args) > 0 or len(kwargs) > 0: + for cntr in range(len(args)): + setattr(self, required_value_list[cntr], args[cntr]) + for v in kwargs: + setattr(self, v, kwargs[v]) + for v in required_value_list: + if not getattr(self, v): + raise ValueError("Missing option: %s" % v) + else: + fmfdata = get_fmf_from_class(self) + if fmfdata: + for v in required_value_list: + if v not in fmfdata: + raise ValueError("Missing option (FMF): %s" % v) + set_fmf_metadata(self, fmfdata) + + else: + raise Exception("no FMF metadata found for item", self, required_value_list, args, kwargs, fmfdata) + + + def __init__(self, *args, **kwargs): + self._meta_init(*args, **kwargs) def check(self, target): pass diff --git a/colin/core/checks/cmd.py b/colin/core/checks/cmd.py index c3b145de..9d90d33a 100644 --- a/colin/core/checks/cmd.py +++ b/colin/core/checks/cmd.py @@ -24,15 +24,11 @@ class CmdAbstractCheck(ContainerAbstractCheck, ImageAbstractCheck): - - def __init__(self, message, description, reference_url, tags, cmd, expected_output=None, - expected_regex=None, - substring=None): - super(CmdAbstractCheck, self).__init__(message, description, reference_url, tags) - self.cmd = cmd - self.expected_output = expected_output - self.expected_regex = expected_regex - self.substring = substring + expected_output = None + expected_regex = None + substring = None + init_list = ["cmd"] + cmd = None def check(self, target): diff --git a/colin/core/checks/dockerfile.py b/colin/core/checks/dockerfile.py index 43231ce0..29ee87ce 100644 --- a/colin/core/checks/dockerfile.py +++ b/colin/core/checks/dockerfile.py @@ -40,14 +40,11 @@ class DockerfileAbstractCheck(AbstractCheck): class InstructionAbstractCheck(DockerfileAbstractCheck): - - def __init__(self, message, description, reference_url, tags, instruction, value_regex, - required): - super(InstructionAbstractCheck, self) \ - .__init__(message, description, reference_url, tags) - self.instruction = instruction - self.value_regex = value_regex - self.required = required + instruction = None + value_regex = None + required = None + init_list = ["instruction", "value_regex", + "required"] def check(self, target): instructions = get_instructions_from_dockerfile_parse(target.instance, self.instruction) @@ -72,14 +69,12 @@ def check(self, target): class InstructionCountAbstractCheck(DockerfileAbstractCheck): - - def __init__(self, message, description, reference_url, tags, instruction, min_count=None, - max_count=None): - super(InstructionCountAbstractCheck, self) \ - .__init__(message, description, reference_url, tags) - self.instruction = instruction - self.min_count = min_count - self.max_count = max_count + # Required, set as class variable + instruction = None + init_list = ["instruction"] + # default value set to these + min_count = None + max_count = None def check(self, target): count = len(get_instructions_from_dockerfile_parse(target.instance, self.instruction)) @@ -105,14 +100,10 @@ def check(self, target): class DockerfileLabelAbstractCheck(DockerfileAbstractCheck): - - def __init__(self, message, description, reference_url, tags, label, required, - value_regex=None): - super(DockerfileLabelAbstractCheck, self) \ - .__init__(message, description, reference_url, tags) - self.label = label - self.required = required - self.value_regex = value_regex + label = [] + required = None + init_list = ["label", "required"] + value_regex = None def check(self, target): labels = target.instance.labels diff --git a/colin/core/checks/envs.py b/colin/core/checks/envs.py index f60d6226..a9912887 100644 --- a/colin/core/checks/envs.py +++ b/colin/core/checks/envs.py @@ -23,14 +23,10 @@ class EnvCheck(ContainerAbstractCheck, ImageAbstractCheck): - - def __init__(self, message, description, reference_url, tags, env_var, required, - value_regex=None): - super(EnvCheck, self) \ - .__init__(message, description, reference_url, tags) - self.env_var = env_var - self.required = required - self.value_regex = value_regex + env_var = None + required = None + value_regex = None + init_list = ["env_var", "required"] def check(self, target): env_vars = inspect_object(target.instance)["Config"]["Env"] diff --git a/colin/core/checks/filesystem.py b/colin/core/checks/filesystem.py index 445359c1..cd9a0d8a 100644 --- a/colin/core/checks/filesystem.py +++ b/colin/core/checks/filesystem.py @@ -30,12 +30,7 @@ class FileCheck(ContainerAbstractCheck, ImageAbstractCheck): """ Check presence of files; w/o mounting the whole FS """ - - def __init__(self, message, description, reference_url, tags, files, all_must_be_present): - super(FileCheck, self) \ - .__init__(message, description, reference_url, tags) - self.files = files - self.all_must_be_present = all_must_be_present + init_list = ["files", "all_must_be_present"] def check(self, target): passed = self.all_must_be_present @@ -86,12 +81,7 @@ def check(self, target): class FileSystemCheck(ContainerAbstractCheck, ImageAbstractCheck): - - def __init__(self, message, description, reference_url, tags, files, all_must_be_present): - super(FileSystemCheck, self) \ - .__init__(message, description, reference_url, tags) - self.files = files - self.all_must_be_present = all_must_be_present + init_list = ["files", "all_must_be_present"] def check(self, target): try: diff --git a/colin/core/checks/labels.py b/colin/core/checks/labels.py index 795cb331..38bd45c4 100644 --- a/colin/core/checks/labels.py +++ b/colin/core/checks/labels.py @@ -21,14 +21,10 @@ class LabelAbstractCheck(ContainerAbstractCheck, ImageAbstractCheck, DockerfileAbstractCheck): - - def __init__(self, message, description, reference_url, tags, labels, required, - value_regex=None): - super(LabelAbstractCheck, self) \ - .__init__(message, description, reference_url, tags) - self.labels = labels - self.required = required - self.value_regex = value_regex + value_regex = None + init_list = ["labels", "required"] + labels = None + required = None def check(self, target): passed = check_label(labels=self.labels, @@ -47,11 +43,9 @@ def check(self, target): class DeprecatedLabelAbstractCheck(ContainerAbstractCheck, ImageAbstractCheck, DockerfileAbstractCheck): - def __init__(self, message, description, reference_url, tags, old_label, new_label): - super(DeprecatedLabelAbstractCheck, self) \ - .__init__(message, description, reference_url, tags) - self.old_label = old_label - self.new_label = new_label + old_label = None + new_label = None + init_list = ["old_label", "new_label"] def check(self, target): labels = target.labels diff --git a/colin/core/fmf_metadata_loader.py b/colin/core/fmf_metadata_loader.py new file mode 100644 index 00000000..fb9a21e1 --- /dev/null +++ b/colin/core/fmf_metadata_loader.py @@ -0,0 +1,74 @@ +# -*- coding: utf-8 -*- +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU General Public License as published by +# the Free Software Foundation, either version 3 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program. If not, see . +# + +import os +import fmf + +METADATA_LOCATION = os.path.dirname(__file__) + +def get_fmf_from_class(obj): + """ + transform class to parameteres for FMF + :param obj: you can use then self.__class__ + :return: FMF tree item + """ + if "." not in obj.__class__.__module__: + modulename = obj.__class__.__module__ + else: + modulename = obj.__class__.__module__.rsplit(".", 1)[1] + modulename += ".py" + filters = ["test:%s" % os.path.basename(modulename), "class:%s" % obj.__class__.__name__] + return get_fmf_metadata(METADATA_LOCATION, filters=filters) + + +def get_fmf_metadata(fmfpath, keys=None, names=None, filters=None, object_list=False): + """ + get fmf metadata for selected class, based on filters + :param fmfpath: + :param keys: + :param names: + :param filters: + :param object_list: + :return: + """ + output = {} + if keys is None: + keys = [] + if names is None: + names = [] + if filters is None: + filters = [] + fmf_tree = fmf.Tree(fmfpath) + items = [x for x in fmf_tree.prune(names=names, filters=filters, keys=keys) if x] + if object_list: + return items + if len(items) == 1: + output = items[0].data + elif len(items) > 1: + raise Exception("There is more FMF test metadata for item") + return output + + +def set_fmf_metadata(target_object, metadata_dict): + """ + set metadata to Object expected is any class derived from AbstractCheck + :param target_object: any AbstractCheck derived class + :param metadata_dict: values to set as instance variables + :return: None + """ + # set all attributes from FMF file as instance variables + for k in metadata_dict: + setattr(target_object, k, metadata_dict[k]) diff --git a/colin/core/loader.py b/colin/core/loader.py index 9389d8b7..1a33c953 100644 --- a/colin/core/loader.py +++ b/colin/core/loader.py @@ -118,7 +118,7 @@ def obtain_check_classes(self): return load_check_classes_from_file(self.path, self.top_py_path) for root, _, files in os.walk(self.path): for fi in files: - if fi.endswith(".pyc"): + if not fi.endswith(".py"): continue path = os.path.join(root, fi) check_classes = check_classes.union(set( diff --git a/colin/default.fmf b/colin/default.fmf new file mode 100644 index 00000000..93f12bbb --- /dev/null +++ b/colin/default.fmf @@ -0,0 +1,16 @@ +/default: + version: 1 + name: "Default ruleset for checking containers/images/dockerfiles." + description: "This set contains general checks applicable to any target." + contact_email: "user-cont-team@redhat.com" + names: + + /checks: + /check@maintainer_label: + tags+: ["required"] + /check@from_tag_not_latest: + tags+: ["required"] + usable_targets: ["dockerfile"] + /check@maintainer_deprecated: + tags+: ["required"] + usable_targets: ["dockerfile"] diff --git a/fmf_scheduler.py b/fmf_scheduler.py new file mode 100644 index 00000000..92b0ee2d --- /dev/null +++ b/fmf_scheduler.py @@ -0,0 +1,124 @@ +import os +import sys +import imp +import re +import logging +import unittest + +from fmf import Tree +from colin.core.target import Target, is_compatible + +LOG_LEVEL = 3 +TARGET = Target(os.environ.get("TARGET"), LOG_LEVEL) +CHECKS = ["colin/checks"] + +logging.basicConfig(stream=sys.stdout, level=logging.INFO) + +logger = logging.getLogger(__name__) + +class ExtendedTree(Tree): + def __init__(self, *args, **kwargs): + super(ExtendedTree, self).__init__(*args, **kwargs) + + def __remove_append_items(self, whole=False): + """ + internal method, delete all append items (ends with +) + :param whole: pass thru 'whole' param to climb + :return: None + """ + for node in self.climb(whole=whole): + for key in sorted(node.data.keys()): + if key.endswith('+'): + del node.data[key] + + def references(self, patterntree, whole=False): + """ + resolve references in names like /a/b/c/d@.x.y or /a/b/c/@y + it uses simple references schema, do not use references to another references + avoid usind / in reference because actual solution creates also this tree items + it is bug or feature, who knows :-) + :param whole: pass thru 'whole' param to climb + :param patterntree: original tree with testcases to contain parent nodes + :return: None + """ + reference_nodes = self.prune(whole=whole, names=["@"]) + for node in reference_nodes: + node.data = node.original_data + ref_item_name = node.name.rsplit("@", 1)[1] + # match item what does not contain @ before nema, otherwise it match same item + reference_node = patterntree.search("[^@]%s" % ref_item_name) + logger.debug("MERGING: {} @ {}".format(node.name, reference_node.name)) + if not reference_node: + raise ValueError("Unable to find reference for node: %s via name search: %s" % + (node.name, ref_item_name)) + node.merge(parent=reference_node) + + self.__remove_append_items(whole=whole) + + def search(self, name): + """ Search node with given name based on reqexp""" + for node in self.climb(): + if re.search(name, node.name): + return node + return None + +# COPYied from: https://eli.thegreenplace.net/2014/04/02/dynamically-generating-python-test-cases +class DynamicClassBase(unittest.TestCase): + longMessage = True + + +def make_check_function(): + """ + rename check function to test, to be able to find this function via testing frameworks + :return: function + """ + def test(self): + self.backendclass().check(TARGET) + return test + + +def class_fmf_generator(fmfpaths): + """ + generates dynamic test classes for nosetest or unittest scheduler based on FMF metadata. + :param fmfpaths: + :return: + """ + test_classes = {} + for fmfpath in fmfpaths: + metadatatree = ExtendedTree(fmfpath) + metadatatree.references(metadatatree) + for node in metadatatree.climb(): + if node.data.get("class") or node.data.get("test"): + + first_class_name = node.name.rsplit("/", 1)[1] + second_class_name = node.data.get("class") + logger.debug("searching for {}".format(node.name)) + modulepath = os.path.join(os.path.dirname(node.sources[-1]), node.data["test"]) + modulename = os.path.basename(node.sources[-1]).split(".", 1)[0] + # in case of referencing use original data tree for info + if "@" in node.name and not os.path.exists(modulepath): + modulepath = os.path.join(os.path.dirname(node.sources[-2]), node.data["test"]) + modulename = os.path.basename(node.sources[-2]).split(".", 1)[0] + test_func = make_check_function() + moduleimp = imp.load_source(modulename, modulepath) + inernalclass = getattr(moduleimp, second_class_name) + if is_compatible(target_type=TARGET.target_type, check_instance=inernalclass()): + # more verbose output + #full_class_name = '{0}_{1}'.format(first_class_name, second_class_name) + full_class_name = '{0}'.format(first_class_name) + oneclass = type(full_class_name, (DynamicClassBase,), {'test': test_func}) + oneclass.backendclass = inernalclass + test_classes[full_class_name] = oneclass + logger.info("Test added: {}".format(node.name)) + else: + logger.info("Test (not target): {}".format(node.name)) + return test_classes + + +classes = class_fmf_generator(CHECKS) +logger.info("number of test classes: {}".format(len(classes))) + +for item in classes: + globals()[item] = classes[item] + + diff --git a/setup.py b/setup.py index 15af2fa4..7bf0123f 100755 --- a/setup.py +++ b/setup.py @@ -39,7 +39,8 @@ 'Click', 'six', 'conu>=0.3.0', - 'dockerfile_parse' + 'dockerfile_parse', + 'fmf' ], entry_points=''' [console_scripts] @@ -67,4 +68,6 @@ author='Red Hat', author_email='user-cont-team@redhat.com', url='https://github.com/user-cont/colin', + package_data={'': ['*.fmf']}, + include_package_data=True, ) diff --git a/tests/unit/test_loader.py b/tests/unit/test_loader.py index 33f46fe8..cd43bf20 100644 --- a/tests/unit/test_loader.py +++ b/tests/unit/test_loader.py @@ -17,13 +17,13 @@ import os import shutil -import colin.checks +from colin import checks from colin.core.loader import CheckLoader def test_upstream_checks_can_be_loaded(): """ check whether all upstream checks can be loaded """ - colin_checks_path = colin.checks.__file__ + colin_checks_path = checks.__file__ colin_checks_dir = os.path.dirname(colin_checks_path) l = CheckLoader(colin_checks_dir) assert l.check_classes From aa8c89152f5a661a6727a9fb190ee072a747b700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0=C4=8Dotka?= Date: Fri, 29 Jun 2018 11:46:19 +0200 Subject: [PATCH 02/15] try to use another, more explicit way cleanup, use just one fmf import improve scheduler, allow schedule tests directly via this file (nosetest framework import inside) I hope that this is good enough format for codacy, grrr another codacy fixes --- colin/checks/best_practices.fmf | 2 +- colin/checks/best_practices.py | 28 +++++ colin/checks/dockerfile.py | 9 +- colin/checks/labels.fmf | 11 ++ colin/checks/labels.py | 5 +- colin/core/checks/abstract_check.py | 43 +------- colin/core/checks/cmd.py | 14 ++- colin/core/checks/dockerfile.py | 39 ++++--- colin/core/checks/envs.py | 12 ++- colin/core/checks/filesystem.py | 14 ++- colin/core/checks/fmf_check.py | 148 +++++++++++++++++++++++++++ colin/core/checks/labels.py | 20 ++-- colin/core/fmf_metadata_loader.py | 74 -------------- fmf_scheduler.py | 153 +++++++++++++++------------- tests/unit/test_loader.py | 4 +- 15 files changed, 349 insertions(+), 227 deletions(-) create mode 100644 colin/checks/labels.fmf create mode 100644 colin/core/checks/fmf_check.py delete mode 100644 colin/core/fmf_metadata_loader.py mode change 100644 => 100755 fmf_scheduler.py diff --git a/colin/checks/best_practices.fmf b/colin/checks/best_practices.fmf index 0ed1bb3e..00fe81e9 100644 --- a/colin/checks/best_practices.fmf +++ b/colin/checks/best_practices.fmf @@ -21,4 +21,4 @@ test: "best_practices.py" message: "Service should not run as root by default." description: "It can be insecure to run service as root." reference_url: "?????" - tags: ["root", "user"] \ No newline at end of file + tags: ["root", "user"] diff --git a/colin/checks/best_practices.py b/colin/checks/best_practices.py index 8a6dd823..7aaa39ae 100644 --- a/colin/checks/best_practices.py +++ b/colin/checks/best_practices.py @@ -29,6 +29,16 @@ class CmdOrEntrypointCheck(ContainerAbstractCheck, ImageAbstractCheck): name = "cmd_or_entrypoint" + def __init__(self): + super(CmdOrEntrypointCheck, self) \ + .__init__(message="Cmd or Entrypoint has to be specified", + description="An ENTRYPOINT allows you to configure a container" + " that will run as an executable. The main purpose" + " of a CMD is to provide defaults for an executing container.", + reference_url="https://fedoraproject.org/wiki/Container:Guidelines" + "#CMD.2FENTRYPOINT_2", + tags=["cmd", "entrypoint"]) + def check(self, target): metadata = inspect_object(target.instance)["Config"] cmd_present = "Cmd" in metadata and metadata["Cmd"] @@ -52,10 +62,28 @@ def check(self, target): class HelpFileOrReadmeCheck(FileCheck): name = "help_file_or_readme" + def __init__(self): + super(HelpFileOrReadmeCheck, self) \ + .__init__(message="The 'helpfile' has to be provided.", + description="Just like traditional packages, containers need " + "some 'man page' information about how they are to be used," + " configured, and integrated into a larger stack.", + reference_url="https://fedoraproject.org/wiki/Container:Guidelines#Help_File", + files=['/help.1', '/README.md'], + tags=['filesystem', 'helpfile', 'man'], + all_must_be_present=False) + class NoRootCheck(ContainerAbstractCheck, ImageAbstractCheck): name = "no_root" + def __init__(self): + super(NoRootCheck, self) \ + .__init__(message="Service should not run as root by default.", + description="It can be insecure to run service as root.", + reference_url="?????", + tags=["root", "user"]) + def check(self, target): metadata = inspect_object(target.instance)["Config"] root_present = "User" in metadata and metadata["User"] in ["", "0", "root"] diff --git a/colin/checks/dockerfile.py b/colin/checks/dockerfile.py index 573e700a..9188f72b 100644 --- a/colin/checks/dockerfile.py +++ b/colin/checks/dockerfile.py @@ -17,10 +17,11 @@ from colin.core.checks.dockerfile import DockerfileAbstractCheck, InstructionCountAbstractCheck from colin.core.result import CheckResult from colin.core.target import ImageName +from colin.core.checks.fmf_check import FMFAbstractCheck -class FromTagNotLatestCheck(DockerfileAbstractCheck): - name = "from_tag_not_latest" +class FromTagNotLatestCheck(FMFAbstractCheck, DockerfileAbstractCheck): + name, metadata = FMFAbstractCheck.get_metadata("from_tag_not_latest") def check(self, target): im = ImageName.parse(target.instance.baseimage) @@ -33,5 +34,5 @@ def check(self, target): logs=[]) -class MaintainerDeprecatedCheck(InstructionCountAbstractCheck): - name = "maintainer_deprecated" +class MaintainerDeprecatedCheck(FMFAbstractCheck, InstructionCountAbstractCheck): + name, metadata = FMFAbstractCheck.get_metadata("maintainer_deprecated") diff --git a/colin/checks/labels.fmf b/colin/checks/labels.fmf new file mode 100644 index 00000000..0540af9d --- /dev/null +++ b/colin/checks/labels.fmf @@ -0,0 +1,11 @@ +test: labels.py + +/maintainer_label: + class: "MaintainerLabelCheck" + message: "Label 'maintainer' has to be specified." + description: "The name and email of the maintainer (usually the submitter)." + reference_url: "https://fedoraproject.org/wiki/Container:Guidelines#LABELS" + tags: ["maintainer", "label"] + labels: ["maintainer"] + required: True + value_regex: None diff --git a/colin/checks/labels.py b/colin/checks/labels.py index 2086c7af..03637b34 100644 --- a/colin/checks/labels.py +++ b/colin/checks/labels.py @@ -15,6 +15,7 @@ # from colin.core.checks.labels import LabelAbstractCheck +from colin.core.checks.fmf_check import FMFAbstractCheck class ArchitectureLabelCheck(LabelAbstractCheck): @@ -228,8 +229,8 @@ def __init__(self): value_regex=None) -class MaintainerLabelCheck(LabelAbstractCheck): - name = "maintainer_label" +class MaintainerLabelCheck(FMFAbstractCheck, LabelAbstractCheck): + name, metadata = FMFAbstractCheck.get_metadata("maintainer_label") class NameLabelCheck(LabelAbstractCheck): diff --git a/colin/core/checks/abstract_check.py b/colin/core/checks/abstract_check.py index bc682526..d03f2b1d 100644 --- a/colin/core/checks/abstract_check.py +++ b/colin/core/checks/abstract_check.py @@ -16,49 +16,16 @@ import json from six import iteritems -from colin.core.fmf_metadata_loader import get_fmf_from_class, set_fmf_metadata class AbstractCheck(object): name = None - message = "" - description = "" - reference_url = "" - tags = [] - base_init_list = ["message", "description", "reference_url", "tags"] - init_list = [] - def _meta_init(self, *args, **kwargs): - """ - This method allows to use normal way via init parameters or alternative way of init via FMF format - Default is to use args kwargs values, in case not given it tries to search for FMF metadata - :param args: - :param kwargs: - :return: - """ - required_value_list = list(set(self.base_init_list + self.init_list)) - if len(args) > 0 or len(kwargs) > 0: - for cntr in range(len(args)): - setattr(self, required_value_list[cntr], args[cntr]) - for v in kwargs: - setattr(self, v, kwargs[v]) - for v in required_value_list: - if not getattr(self, v): - raise ValueError("Missing option: %s" % v) - else: - fmfdata = get_fmf_from_class(self) - if fmfdata: - for v in required_value_list: - if v not in fmfdata: - raise ValueError("Missing option (FMF): %s" % v) - set_fmf_metadata(self, fmfdata) - - else: - raise Exception("no FMF metadata found for item", self, required_value_list, args, kwargs, fmfdata) - - - def __init__(self, *args, **kwargs): - self._meta_init(*args, **kwargs) + def __init__(self, message, description, reference_url, tags): + self.message = message + self.description = description + self.reference_url = reference_url + self.tags = tags def check(self, target): pass diff --git a/colin/core/checks/cmd.py b/colin/core/checks/cmd.py index 9d90d33a..c3b145de 100644 --- a/colin/core/checks/cmd.py +++ b/colin/core/checks/cmd.py @@ -24,11 +24,15 @@ class CmdAbstractCheck(ContainerAbstractCheck, ImageAbstractCheck): - expected_output = None - expected_regex = None - substring = None - init_list = ["cmd"] - cmd = None + + def __init__(self, message, description, reference_url, tags, cmd, expected_output=None, + expected_regex=None, + substring=None): + super(CmdAbstractCheck, self).__init__(message, description, reference_url, tags) + self.cmd = cmd + self.expected_output = expected_output + self.expected_regex = expected_regex + self.substring = substring def check(self, target): diff --git a/colin/core/checks/dockerfile.py b/colin/core/checks/dockerfile.py index 29ee87ce..43231ce0 100644 --- a/colin/core/checks/dockerfile.py +++ b/colin/core/checks/dockerfile.py @@ -40,11 +40,14 @@ class DockerfileAbstractCheck(AbstractCheck): class InstructionAbstractCheck(DockerfileAbstractCheck): - instruction = None - value_regex = None - required = None - init_list = ["instruction", "value_regex", - "required"] + + def __init__(self, message, description, reference_url, tags, instruction, value_regex, + required): + super(InstructionAbstractCheck, self) \ + .__init__(message, description, reference_url, tags) + self.instruction = instruction + self.value_regex = value_regex + self.required = required def check(self, target): instructions = get_instructions_from_dockerfile_parse(target.instance, self.instruction) @@ -69,12 +72,14 @@ def check(self, target): class InstructionCountAbstractCheck(DockerfileAbstractCheck): - # Required, set as class variable - instruction = None - init_list = ["instruction"] - # default value set to these - min_count = None - max_count = None + + def __init__(self, message, description, reference_url, tags, instruction, min_count=None, + max_count=None): + super(InstructionCountAbstractCheck, self) \ + .__init__(message, description, reference_url, tags) + self.instruction = instruction + self.min_count = min_count + self.max_count = max_count def check(self, target): count = len(get_instructions_from_dockerfile_parse(target.instance, self.instruction)) @@ -100,10 +105,14 @@ def check(self, target): class DockerfileLabelAbstractCheck(DockerfileAbstractCheck): - label = [] - required = None - init_list = ["label", "required"] - value_regex = None + + def __init__(self, message, description, reference_url, tags, label, required, + value_regex=None): + super(DockerfileLabelAbstractCheck, self) \ + .__init__(message, description, reference_url, tags) + self.label = label + self.required = required + self.value_regex = value_regex def check(self, target): labels = target.instance.labels diff --git a/colin/core/checks/envs.py b/colin/core/checks/envs.py index a9912887..f60d6226 100644 --- a/colin/core/checks/envs.py +++ b/colin/core/checks/envs.py @@ -23,10 +23,14 @@ class EnvCheck(ContainerAbstractCheck, ImageAbstractCheck): - env_var = None - required = None - value_regex = None - init_list = ["env_var", "required"] + + def __init__(self, message, description, reference_url, tags, env_var, required, + value_regex=None): + super(EnvCheck, self) \ + .__init__(message, description, reference_url, tags) + self.env_var = env_var + self.required = required + self.value_regex = value_regex def check(self, target): env_vars = inspect_object(target.instance)["Config"]["Env"] diff --git a/colin/core/checks/filesystem.py b/colin/core/checks/filesystem.py index cd9a0d8a..445359c1 100644 --- a/colin/core/checks/filesystem.py +++ b/colin/core/checks/filesystem.py @@ -30,7 +30,12 @@ class FileCheck(ContainerAbstractCheck, ImageAbstractCheck): """ Check presence of files; w/o mounting the whole FS """ - init_list = ["files", "all_must_be_present"] + + def __init__(self, message, description, reference_url, tags, files, all_must_be_present): + super(FileCheck, self) \ + .__init__(message, description, reference_url, tags) + self.files = files + self.all_must_be_present = all_must_be_present def check(self, target): passed = self.all_must_be_present @@ -81,7 +86,12 @@ def check(self, target): class FileSystemCheck(ContainerAbstractCheck, ImageAbstractCheck): - init_list = ["files", "all_must_be_present"] + + def __init__(self, message, description, reference_url, tags, files, all_must_be_present): + super(FileSystemCheck, self) \ + .__init__(message, description, reference_url, tags) + self.files = files + self.all_must_be_present = all_must_be_present def check(self, target): try: diff --git a/colin/core/checks/fmf_check.py b/colin/core/checks/fmf_check.py new file mode 100644 index 00000000..47a4a099 --- /dev/null +++ b/colin/core/checks/fmf_check.py @@ -0,0 +1,148 @@ +""" +Module handling FMF stored metadata for classes +""" + +import os +import copy +import re +import logging + +from fmf import Tree +from .abstract_check import AbstractCheck + + +METADATA_LOCATION = os.path.dirname(__file__) +logger = logging.getLogger(__name__) + + +class ExtendedTree(Tree): + """ + FMF Extension. Allows to use references via @ to another items -> usefull for rulesets + """ + + def __remove_append_items(self, whole=False): + """ + internal method, delete all append items (ends with +) + :param whole: pass thru 'whole' param to climb + :return: None + """ + for node in self.climb(whole=whole): + for key in sorted(node.data.keys()): + if key.endswith('+'): + del node.data[key] + + def references(self, patterntree, whole=False): + """ + resolve references in names like /a/b/c/d@.x.y or /a/b/c/@y + it uses simple references schema, do not use references to another references + avoid usind / in reference because actual solution creates also this tree items + it is bug or feature, who knows :-) + :param whole: pass thru 'whole' param to climb + :param patterntree: original tree with testcases to contain parent nodes + :return: None + """ + reference_nodes = self.prune(whole=whole, names=["@"]) + for node in reference_nodes: + node.data = node.original_data + ref_item_name = node.name.rsplit("@", 1)[1] + # match item what does not contain @ before name, otherwise it + # match same item + reference_node = patterntree.search("[^@]%s" % ref_item_name) + logger.debug("MERGING: %s @ %s", node.name, reference_node.name) + if not reference_node: + raise ValueError("Unable to find reference for node: %s via name search: %s" % + (node.name, ref_item_name)) + node.merge(parent=reference_node) + + self.__remove_append_items(whole=whole) + + def search(self, name): + """ Search node with given name based on reqexp""" + for node in self.climb(): + if re.search(name, node.name): + return node + return None + + +class FMFCaseLoader(object): + """ + search and load FMF metadata + """ + + def __init__(self, name_id, path=None): + """ + Case loader metatada init, it try to gather metadata form FMF based on name identifier + :param name_id: + :param path: + """ + self.metadata = self.__receive_fmf_metadata( + path or METADATA_LOCATION, name=name_id) + self.name = self.get_name() + + def __receive_fmf_metadata(self, fmfpath, name=None, object_list=False): + """ + internal method, search name in metadata in fmfpath + + :param fmfpath: path to filesystem + :param name: str - name as pattern to search (substring) + :param object_list: bool, if true, return whole list of found items + :return: Tree Object or list + """ + output = {} + fmf_tree = ExtendedTree(fmfpath) + logger.debug("get FMF metadata for test (path:%s name=%s)", fmfpath, name) + items = [x for x in fmf_tree.climb( + ) if name in x.name and "@" not in x.name] + if object_list: + return items + if len(items) == 1: + output = items[0] + elif len(items) > 1: + raise Exception("There is more FMF test metadata for item by name:{}({}) {}".format( + name, len(items), [x.name for x in items])) + elif not items: + raise Exception("Unable to get FMF metadata for: {}".format(name)) + return output + + def get_name(self, full=False): + """ + return FMF Name of item + :param full: if True return full path identifier + :return: str - name identifier + """ + out = self.metadata.name + if not full: + out = self.metadata.name.rsplit("/", 1)[-1] + return out + + +class FMFAbstractCheck(AbstractCheck): + """ + Abstract class for checks and loading metadata from FMF format + """ + metadata = None + name = None + + @classmethod + def get_metadata(cls, name, path=None): + """ + Very important method what returns tuple for object initialization + COLIN tool expects to have class attribute: name + to be able to find cases + :param name: str, identifier of name in FMF + :param path: where to look for metadata + :return: tuple, name, FMF metadata Tree + """ + item = FMFCaseLoader(name_id=name, path=path) + return item.name, item.metadata + + def __init__(self): + """ + wraps parameters to COLIN format + """ + kwargs = copy.deepcopy(self.metadata.data) + if "class" in kwargs: + del kwargs["class"] + if "test" in kwargs: + del kwargs["test"] + super(FMFAbstractCheck, self).__init__(**kwargs) diff --git a/colin/core/checks/labels.py b/colin/core/checks/labels.py index 38bd45c4..795cb331 100644 --- a/colin/core/checks/labels.py +++ b/colin/core/checks/labels.py @@ -21,10 +21,14 @@ class LabelAbstractCheck(ContainerAbstractCheck, ImageAbstractCheck, DockerfileAbstractCheck): - value_regex = None - init_list = ["labels", "required"] - labels = None - required = None + + def __init__(self, message, description, reference_url, tags, labels, required, + value_regex=None): + super(LabelAbstractCheck, self) \ + .__init__(message, description, reference_url, tags) + self.labels = labels + self.required = required + self.value_regex = value_regex def check(self, target): passed = check_label(labels=self.labels, @@ -43,9 +47,11 @@ def check(self, target): class DeprecatedLabelAbstractCheck(ContainerAbstractCheck, ImageAbstractCheck, DockerfileAbstractCheck): - old_label = None - new_label = None - init_list = ["old_label", "new_label"] + def __init__(self, message, description, reference_url, tags, old_label, new_label): + super(DeprecatedLabelAbstractCheck, self) \ + .__init__(message, description, reference_url, tags) + self.old_label = old_label + self.new_label = new_label def check(self, target): labels = target.labels diff --git a/colin/core/fmf_metadata_loader.py b/colin/core/fmf_metadata_loader.py deleted file mode 100644 index fb9a21e1..00000000 --- a/colin/core/fmf_metadata_loader.py +++ /dev/null @@ -1,74 +0,0 @@ -# -*- coding: utf-8 -*- -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU General Public License as published by -# the Free Software Foundation, either version 3 of the License, or -# (at your option) any later version. -# -# This program is distributed in the hope that it will be useful, -# but WITHOUT ANY WARRANTY; without even the implied warranty of -# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the -# GNU General Public License for more details. -# -# You should have received a copy of the GNU General Public License -# along with this program. If not, see . -# - -import os -import fmf - -METADATA_LOCATION = os.path.dirname(__file__) - -def get_fmf_from_class(obj): - """ - transform class to parameteres for FMF - :param obj: you can use then self.__class__ - :return: FMF tree item - """ - if "." not in obj.__class__.__module__: - modulename = obj.__class__.__module__ - else: - modulename = obj.__class__.__module__.rsplit(".", 1)[1] - modulename += ".py" - filters = ["test:%s" % os.path.basename(modulename), "class:%s" % obj.__class__.__name__] - return get_fmf_metadata(METADATA_LOCATION, filters=filters) - - -def get_fmf_metadata(fmfpath, keys=None, names=None, filters=None, object_list=False): - """ - get fmf metadata for selected class, based on filters - :param fmfpath: - :param keys: - :param names: - :param filters: - :param object_list: - :return: - """ - output = {} - if keys is None: - keys = [] - if names is None: - names = [] - if filters is None: - filters = [] - fmf_tree = fmf.Tree(fmfpath) - items = [x for x in fmf_tree.prune(names=names, filters=filters, keys=keys) if x] - if object_list: - return items - if len(items) == 1: - output = items[0].data - elif len(items) > 1: - raise Exception("There is more FMF test metadata for item") - return output - - -def set_fmf_metadata(target_object, metadata_dict): - """ - set metadata to Object expected is any class derived from AbstractCheck - :param target_object: any AbstractCheck derived class - :param metadata_dict: values to set as instance variables - :return: None - """ - # set all attributes from FMF file as instance variables - for k in metadata_dict: - setattr(target_object, k, metadata_dict[k]) diff --git a/fmf_scheduler.py b/fmf_scheduler.py old mode 100644 new mode 100755 index 92b0ee2d..e555f671 --- a/fmf_scheduler.py +++ b/fmf_scheduler.py @@ -1,124 +1,131 @@ +#!/usr/bin/python3 +""" +FMF scheduler for nosetests +Found test via fmf files and creates dynamic testclassed based on that. +""" + + import os import sys import imp -import re import logging import unittest -from fmf import Tree from colin.core.target import Target, is_compatible +from colin.core.checks.fmf_check import ExtendedTree LOG_LEVEL = 3 -TARGET = Target(os.environ.get("TARGET"), LOG_LEVEL) CHECKS = ["colin/checks"] - -logging.basicConfig(stream=sys.stdout, level=logging.INFO) - logger = logging.getLogger(__name__) -class ExtendedTree(Tree): - def __init__(self, *args, **kwargs): - super(ExtendedTree, self).__init__(*args, **kwargs) - def __remove_append_items(self, whole=False): - """ - internal method, delete all append items (ends with +) - :param whole: pass thru 'whole' param to climb - :return: None - """ - for node in self.climb(whole=whole): - for key in sorted(node.data.keys()): - if key.endswith('+'): - del node.data[key] - - def references(self, patterntree, whole=False): - """ - resolve references in names like /a/b/c/d@.x.y or /a/b/c/@y - it uses simple references schema, do not use references to another references - avoid usind / in reference because actual solution creates also this tree items - it is bug or feature, who knows :-) - :param whole: pass thru 'whole' param to climb - :param patterntree: original tree with testcases to contain parent nodes - :return: None - """ - reference_nodes = self.prune(whole=whole, names=["@"]) - for node in reference_nodes: - node.data = node.original_data - ref_item_name = node.name.rsplit("@", 1)[1] - # match item what does not contain @ before nema, otherwise it match same item - reference_node = patterntree.search("[^@]%s" % ref_item_name) - logger.debug("MERGING: {} @ {}".format(node.name, reference_node.name)) - if not reference_node: - raise ValueError("Unable to find reference for node: %s via name search: %s" % - (node.name, ref_item_name)) - node.merge(parent=reference_node) - - self.__remove_append_items(whole=whole) - - def search(self, name): - """ Search node with given name based on reqexp""" - for node in self.climb(): - if re.search(name, node.name): - return node - return None - -# COPYied from: https://eli.thegreenplace.net/2014/04/02/dynamically-generating-python-test-cases +# COPYied from: +# https://eli.thegreenplace.net/2014/04/02/dynamically-generating-python-test-cases class DynamicClassBase(unittest.TestCase): + """ + Basic Derived test Class + """ longMessage = True -def make_check_function(): +def make_check_function(target): """ rename check function to test, to be able to find this function via testing frameworks :return: function """ + def test(self): - self.backendclass().check(TARGET) + """ + Generic test function wrapper, it calls self.check(target) + + :param self: it is function what will be incorporated to class + :return: + """ + self.backendclass().check(target) return test -def class_fmf_generator(fmfpaths): +def class_fmf_generator(fmfpaths, target_name, log_level=LOG_LEVEL): """ generates dynamic test classes for nosetest or unittest scheduler based on FMF metadata. :param fmfpaths: :return: """ + target = Target(target_name, log_level) test_classes = {} for fmfpath in fmfpaths: metadatatree = ExtendedTree(fmfpath) metadatatree.references(metadatatree) for node in metadatatree.climb(): + logger.debug("look at node: %s ", node) if node.data.get("class") or node.data.get("test"): - - first_class_name = node.name.rsplit("/", 1)[1] + logger.debug("node (%s) contains test and class item", node) + first_class_name = node.name.rsplit("/", 1)[-1] second_class_name = node.data.get("class") - logger.debug("searching for {}".format(node.name)) - modulepath = os.path.join(os.path.dirname(node.sources[-1]), node.data["test"]) - modulename = os.path.basename(node.sources[-1]).split(".", 1)[0] + logger.debug("searching for %s", first_class_name) + modulepath = os.path.join(os.path.dirname( + node.sources[-1]), node.data["test"]) + modulename = os.path.basename( + node.sources[-1]).split(".", 1)[0] # in case of referencing use original data tree for info if "@" in node.name and not os.path.exists(modulepath): - modulepath = os.path.join(os.path.dirname(node.sources[-2]), node.data["test"]) - modulename = os.path.basename(node.sources[-2]).split(".", 1)[0] - test_func = make_check_function() + modulepath = os.path.join(os.path.dirname( + node.sources[-2]), node.data["test"]) + modulename = os.path.basename( + node.sources[-2]).split(".", 1)[0] + test_func = make_check_function(target=target) + logger.info("Try to import: %s from path %s", modulename, modulepath) moduleimp = imp.load_source(modulename, modulepath) inernalclass = getattr(moduleimp, second_class_name) - if is_compatible(target_type=TARGET.target_type, check_instance=inernalclass()): + if is_compatible(target_type=target.target_type, check_instance=inernalclass()): # more verbose output #full_class_name = '{0}_{1}'.format(first_class_name, second_class_name) full_class_name = '{0}'.format(first_class_name) oneclass = type(full_class_name, (DynamicClassBase,), {'test': test_func}) oneclass.backendclass = inernalclass test_classes[full_class_name] = oneclass - logger.info("Test added: {}".format(node.name)) + logger.info("Test added: %s", node.name) else: - logger.info("Test (not target): {}".format(node.name)) + logger.info("Test (not target): %s", node.name) return test_classes +def scheduler_opts(target_name=None, checks=None): + """ + gather all options what have to be set for function class_fmf_generator + now it is able set via ENVVARS -classes = class_fmf_generator(CHECKS) -logger.info("number of test classes: {}".format(len(classes))) - -for item in classes: - globals()[item] = classes[item] - - + :param target_name: override envvar TARGET + :param checks: override envvar CHECKS + :return: dict of test classes + """ + if not target_name: + target_name = os.environ.get("TARGET") + if not target_name: + raise EnvironmentError("TARGET envvar is not set.") + if not checks: + if os.environ.get("CHECKS"): + checks = os.environ.get("CHECKS").split(":") + else: + checks = CHECKS + output = class_fmf_generator(checks, target_name) + return output + + + +if __name__ == "__main__": + logging.basicConfig(stream=sys.stdout, level=LOG_LEVEL) + classes = scheduler_opts() + for item in classes: + globals()[item] = classes[item] + + # try to schedule it via nosetests in case of direct schedule + import nose + logger.info("number of test classes: %s", len(classes)) + module_name = sys.modules[__name__].__file__ + logging.debug("running nose for package: %s", module_name) + result = nose.run(argv=[sys.argv[0], module_name, '-v']) + logging.info("all tests ok: %s", result) +else: + classes = scheduler_opts() + for item in classes: + globals()[item] = classes[item] diff --git a/tests/unit/test_loader.py b/tests/unit/test_loader.py index cd43bf20..33f46fe8 100644 --- a/tests/unit/test_loader.py +++ b/tests/unit/test_loader.py @@ -17,13 +17,13 @@ import os import shutil -from colin import checks +import colin.checks from colin.core.loader import CheckLoader def test_upstream_checks_can_be_loaded(): """ check whether all upstream checks can be loaded """ - colin_checks_path = checks.__file__ + colin_checks_path = colin.checks.__file__ colin_checks_dir = os.path.dirname(colin_checks_path) l = CheckLoader(colin_checks_dir) assert l.check_classes From a5f4deeebc045890948e467c7a063bcbdda9b76b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0=C4=8Dotka?= Date: Wed, 4 Jul 2018 14:32:40 +0200 Subject: [PATCH 03/15] improve rulesets handling --- colin/{ => checks}/.fmf/version | 0 colin/core/checks/fmf_check.py | 5 +- fmf_scheduler.py | 95 ++++++++++++++++----------------- rulesets/.fmf/version | 1 + {colin => rulesets}/default.fmf | 0 5 files changed, 51 insertions(+), 50 deletions(-) rename colin/{ => checks}/.fmf/version (100%) create mode 100644 rulesets/.fmf/version rename {colin => rulesets}/default.fmf (100%) diff --git a/colin/.fmf/version b/colin/checks/.fmf/version similarity index 100% rename from colin/.fmf/version rename to colin/checks/.fmf/version diff --git a/colin/core/checks/fmf_check.py b/colin/core/checks/fmf_check.py index 47a4a099..94101724 100644 --- a/colin/core/checks/fmf_check.py +++ b/colin/core/checks/fmf_check.py @@ -9,9 +9,10 @@ from fmf import Tree from .abstract_check import AbstractCheck +from ..ruleset.ruleset import get_checks_path +CHECK_DIRECTORY = os.environ.get("CHECK") or get_checks_path() -METADATA_LOCATION = os.path.dirname(__file__) logger = logging.getLogger(__name__) @@ -76,7 +77,7 @@ def __init__(self, name_id, path=None): :param path: """ self.metadata = self.__receive_fmf_metadata( - path or METADATA_LOCATION, name=name_id) + path or CHECK_DIRECTORY, name=name_id) self.name = self.get_name() def __receive_fmf_metadata(self, fmfpath, name=None, object_list=False): diff --git a/fmf_scheduler.py b/fmf_scheduler.py index e555f671..6129ffd7 100755 --- a/fmf_scheduler.py +++ b/fmf_scheduler.py @@ -12,10 +12,9 @@ import unittest from colin.core.target import Target, is_compatible -from colin.core.checks.fmf_check import ExtendedTree +from colin.core.checks.fmf_check import ExtendedTree, CHECK_DIRECTORY LOG_LEVEL = 3 -CHECKS = ["colin/checks"] logger = logging.getLogger(__name__) @@ -35,61 +34,62 @@ def make_check_function(target): """ def test(self): - """ - Generic test function wrapper, it calls self.check(target) - - :param self: it is function what will be incorporated to class - :return: - """ self.backendclass().check(target) return test -def class_fmf_generator(fmfpaths, target_name, log_level=LOG_LEVEL): +def class_fmf_generator(fmfpath, target_name, log_level=LOG_LEVEL, ruleset_tree_path=None): """ generates dynamic test classes for nosetest or unittest scheduler based on FMF metadata. - :param fmfpaths: + + :param fmfpath: path to checks + :param target_name: what is the target object + :param log_level: + :param ruleset_tree_path: :return: """ target = Target(target_name, log_level) test_classes = {} - for fmfpath in fmfpaths: - metadatatree = ExtendedTree(fmfpath) - metadatatree.references(metadatatree) - for node in metadatatree.climb(): - logger.debug("look at node: %s ", node) - if node.data.get("class") or node.data.get("test"): - logger.debug("node (%s) contains test and class item", node) - first_class_name = node.name.rsplit("/", 1)[-1] - second_class_name = node.data.get("class") - logger.debug("searching for %s", first_class_name) + if not ruleset_tree_path: + ruleset_tree_path = fmfpath + ruleset_metadatatree = ExtendedTree(ruleset_tree_path) + metadatatree = ExtendedTree(fmfpath) + #metadatatree.references(metadatatree) + ruleset_metadatatree.references(metadatatree) + for node in ruleset_metadatatree.climb(): + logger.debug("look at node: %s ", node) + if node.data.get("class") or node.data.get("test"): + logger.debug("node (%s) contains test and class item", node) + first_class_name = node.name.rsplit("/", 1)[-1] + second_class_name = node.data.get("class") + logger.debug("searching for %s", first_class_name) + modulepath = os.path.join(os.path.dirname( + node.sources[-1]), node.data["test"]) + modulename = os.path.basename( + node.sources[-1]).split(".", 1)[0] + # in case of referencing use original data tree for info + if "@" in node.name and not os.path.exists(modulepath): modulepath = os.path.join(os.path.dirname( - node.sources[-1]), node.data["test"]) + node.sources[-2]), node.data["test"]) modulename = os.path.basename( - node.sources[-1]).split(".", 1)[0] - # in case of referencing use original data tree for info - if "@" in node.name and not os.path.exists(modulepath): - modulepath = os.path.join(os.path.dirname( - node.sources[-2]), node.data["test"]) - modulename = os.path.basename( - node.sources[-2]).split(".", 1)[0] - test_func = make_check_function(target=target) - logger.info("Try to import: %s from path %s", modulename, modulepath) - moduleimp = imp.load_source(modulename, modulepath) - inernalclass = getattr(moduleimp, second_class_name) - if is_compatible(target_type=target.target_type, check_instance=inernalclass()): - # more verbose output - #full_class_name = '{0}_{1}'.format(first_class_name, second_class_name) - full_class_name = '{0}'.format(first_class_name) - oneclass = type(full_class_name, (DynamicClassBase,), {'test': test_func}) - oneclass.backendclass = inernalclass - test_classes[full_class_name] = oneclass - logger.info("Test added: %s", node.name) - else: - logger.info("Test (not target): %s", node.name) + node.sources[-2]).split(".", 1)[0] + test_func = make_check_function(target=target) + logger.info("Try to import: %s from path %s", modulename, modulepath) + moduleimp = imp.load_source(modulename, modulepath) + inernalclass = getattr(moduleimp, second_class_name) + if is_compatible(target_type=target.target_type, check_instance=inernalclass()): + # more verbose output + #full_class_name = '{0}_{1}'.format(first_class_name, second_class_name) + full_class_name = '{0}'.format(first_class_name) + oneclass = type(full_class_name, (DynamicClassBase,), {'test': test_func}) + oneclass.backendclass = inernalclass + test_classes[full_class_name] = oneclass + logger.info("Test added: %s", node.name) + else: + logger.info("Test (not target): %s", node.name) return test_classes -def scheduler_opts(target_name=None, checks=None): +def scheduler_opts(target_name=None, checks=None, ruleset_path=None): """ gather all options what have to be set for function class_fmf_generator now it is able set via ENVVARS @@ -103,11 +103,10 @@ def scheduler_opts(target_name=None, checks=None): if not target_name: raise EnvironmentError("TARGET envvar is not set.") if not checks: - if os.environ.get("CHECKS"): - checks = os.environ.get("CHECKS").split(":") - else: - checks = CHECKS - output = class_fmf_generator(checks, target_name) + checks = CHECK_DIRECTORY + if not ruleset_path: + ruleset_path = os.environ.get("RULESETPATH") + output = class_fmf_generator(checks, target_name, ruleset_tree_path=ruleset_path) return output diff --git a/rulesets/.fmf/version b/rulesets/.fmf/version new file mode 100644 index 00000000..d00491fd --- /dev/null +++ b/rulesets/.fmf/version @@ -0,0 +1 @@ +1 diff --git a/colin/default.fmf b/rulesets/default.fmf similarity index 100% rename from colin/default.fmf rename to rulesets/default.fmf From c363e84cca640a3778a6bc527c854ea268da8be3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0=C4=8Dotka?= Date: Mon, 9 Jul 2018 08:13:53 +0200 Subject: [PATCH 04/15] changed to Null from None, should be tranformed to pyton None --- colin/checks/labels.fmf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/colin/checks/labels.fmf b/colin/checks/labels.fmf index 0540af9d..85c7f2a2 100644 --- a/colin/checks/labels.fmf +++ b/colin/checks/labels.fmf @@ -8,4 +8,4 @@ test: labels.py tags: ["maintainer", "label"] labels: ["maintainer"] required: True - value_regex: None + value_regex: Null From aba58db4976b8d310b393657f36764d16684734e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0=C4=8Dotka?= Date: Mon, 9 Jul 2018 08:32:14 +0200 Subject: [PATCH 05/15] reverted typo back --- colin/checks/dockerfile.fmf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/colin/checks/dockerfile.fmf b/colin/checks/dockerfile.fmf index 04f900da..8ee8e6b3 100644 --- a/colin/checks/dockerfile.fmf +++ b/colin/checks/dockerfile.fmf @@ -4,7 +4,7 @@ description: "Generic desription for dockerfile test if not specific description test: "dockerfile.py" class: "FromTagNotLatestCheck" message: "In FROM, tag has to be specified and not 'latest'." - description: "Using the 'latest' tag may cause unpredictable builds. It is recommended that a specific tag is used in the FROM." + description: "Using the 'latest' tag may cause unpredictable builds.It is recommended that a specific tag is used in the FROM." reference_url: "https://fedoraproject.org/wiki/Container:Guidelines#FROM" tags: ["from", "dockerfile", "baseimage", "latest"] From f6728d2e1ed8ab276d344e36a35c2e433d1faf1f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0=C4=8Dotka?= Date: Mon, 9 Jul 2018 09:43:21 +0200 Subject: [PATCH 06/15] add filter possibilities via envvar values, possible to select some ruleset or select tests based on names or advanced filters --- fmf_scheduler.py | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/fmf_scheduler.py b/fmf_scheduler.py index 6129ffd7..2692ab00 100755 --- a/fmf_scheduler.py +++ b/fmf_scheduler.py @@ -38,7 +38,7 @@ def test(self): return test -def class_fmf_generator(fmfpath, target_name, log_level=LOG_LEVEL, ruleset_tree_path=None): +def class_fmf_generator(fmfpath, target_name, log_level=LOG_LEVEL, ruleset_tree_path=None, filter_names=None, filters=None): """ generates dynamic test classes for nosetest or unittest scheduler based on FMF metadata. @@ -54,9 +54,8 @@ def class_fmf_generator(fmfpath, target_name, log_level=LOG_LEVEL, ruleset_tree_ ruleset_tree_path = fmfpath ruleset_metadatatree = ExtendedTree(ruleset_tree_path) metadatatree = ExtendedTree(fmfpath) - #metadatatree.references(metadatatree) ruleset_metadatatree.references(metadatatree) - for node in ruleset_metadatatree.climb(): + for node in ruleset_metadatatree.prune(names=filter_names, filters=filters): logger.debug("look at node: %s ", node) if node.data.get("class") or node.data.get("test"): logger.debug("node (%s) contains test and class item", node) @@ -89,13 +88,17 @@ def class_fmf_generator(fmfpath, target_name, log_level=LOG_LEVEL, ruleset_tree_ logger.info("Test (not target): %s", node.name) return test_classes -def scheduler_opts(target_name=None, checks=None, ruleset_path=None): +def scheduler_opts(target_name=None, checks=None, ruleset_path=None, filter_names=None, filters=None): """ gather all options what have to be set for function class_fmf_generator now it is able set via ENVVARS :param target_name: override envvar TARGET :param checks: override envvar CHECKS + :param ruleset_path: path to directory, where are fmf rulesets + :param filters: dict of filters, filter out just selected cases, use FMF filter format, + via FILTER envvar use ";" as filter separator + :param filter_names: dict of item names for filtering user ";" as separator for NAMES envvar :return: dict of test classes """ if not target_name: @@ -106,11 +109,14 @@ def scheduler_opts(target_name=None, checks=None, ruleset_path=None): checks = CHECK_DIRECTORY if not ruleset_path: ruleset_path = os.environ.get("RULESETPATH") - output = class_fmf_generator(checks, target_name, ruleset_tree_path=ruleset_path) + if not filters: + filters = os.environ.get("FILTERS", "").split(";") + if not filter_names: + filter_names = os.environ.get("NAMES", "").split(";") + output = class_fmf_generator(checks, target_name, ruleset_tree_path=ruleset_path, filter_names=filter_names, filters=filters) return output - if __name__ == "__main__": logging.basicConfig(stream=sys.stdout, level=LOG_LEVEL) classes = scheduler_opts() From 0e58bcf1ebc391d3468dda3d2700cabad369772d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0=C4=8Dotka?= Date: Mon, 9 Jul 2018 12:30:39 +0200 Subject: [PATCH 07/15] missing important part, what raises exception in case, it fails --- fmf_scheduler.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/fmf_scheduler.py b/fmf_scheduler.py index 2692ab00..341cbc6a 100755 --- a/fmf_scheduler.py +++ b/fmf_scheduler.py @@ -13,6 +13,7 @@ from colin.core.target import Target, is_compatible from colin.core.checks.fmf_check import ExtendedTree, CHECK_DIRECTORY +from colin.core.constant import PASSED LOG_LEVEL = 3 logger = logging.getLogger(__name__) @@ -34,7 +35,12 @@ def make_check_function(target): """ def test(self): - self.backendclass().check(target) + out = self.backendclass().check(target) + if out.status is not PASSED: + raise AssertionError("test:{} -> {}".format( + self.backendclass.name, + str(out)) + ) return test From 9b6cbc171866e6daacb5621d74cdbd5f59313337 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0=C4=8Dotka?= Date: Mon, 9 Jul 2018 12:54:14 +0200 Subject: [PATCH 08/15] consolidate logging --- fmf_scheduler.py | 30 ++++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/fmf_scheduler.py b/fmf_scheduler.py index 341cbc6a..e823c95d 100755 --- a/fmf_scheduler.py +++ b/fmf_scheduler.py @@ -15,9 +15,10 @@ from colin.core.checks.fmf_check import ExtendedTree, CHECK_DIRECTORY from colin.core.constant import PASSED -LOG_LEVEL = 3 logger = logging.getLogger(__name__) +def get_log_level(): + return int(os.environ.get(("DEBUG")) or logging.INFO) # COPYied from: # https://eli.thegreenplace.net/2014/04/02/dynamically-generating-python-test-cases @@ -44,7 +45,7 @@ def test(self): return test -def class_fmf_generator(fmfpath, target_name, log_level=LOG_LEVEL, ruleset_tree_path=None, filter_names=None, filters=None): +def class_fmf_generator(fmfpath, target_name, log_level, ruleset_tree_path=None, filter_names=None, filters=None): """ generates dynamic test classes for nosetest or unittest scheduler based on FMF metadata. @@ -62,9 +63,8 @@ def class_fmf_generator(fmfpath, target_name, log_level=LOG_LEVEL, ruleset_tree_ metadatatree = ExtendedTree(fmfpath) ruleset_metadatatree.references(metadatatree) for node in ruleset_metadatatree.prune(names=filter_names, filters=filters): - logger.debug("look at node: %s ", node) if node.data.get("class") or node.data.get("test"): - logger.debug("node (%s) contains test and class item", node) + logger.debug("node (%s) contains test and class item", node.name) first_class_name = node.name.rsplit("/", 1)[-1] second_class_name = node.data.get("class") logger.debug("searching for %s", first_class_name) @@ -79,7 +79,7 @@ def class_fmf_generator(fmfpath, target_name, log_level=LOG_LEVEL, ruleset_tree_ modulename = os.path.basename( node.sources[-2]).split(".", 1)[0] test_func = make_check_function(target=target) - logger.info("Try to import: %s from path %s", modulename, modulepath) + logger.debug("Try to import: %s from path %s", modulename, modulepath) moduleimp = imp.load_source(modulename, modulepath) inernalclass = getattr(moduleimp, second_class_name) if is_compatible(target_type=target.target_type, check_instance=inernalclass()): @@ -89,12 +89,16 @@ def class_fmf_generator(fmfpath, target_name, log_level=LOG_LEVEL, ruleset_tree_ oneclass = type(full_class_name, (DynamicClassBase,), {'test': test_func}) oneclass.backendclass = inernalclass test_classes[full_class_name] = oneclass - logger.info("Test added: %s", node.name) + logger.debug("Test added: %s", node.name) else: - logger.info("Test (not target): %s", node.name) + logger.debug("Test (not target): %s", node.name) + else: + if "__pycache__" not in node.name: + logger.warning("error in fmf config for node (missing test and class items): %s (data: %s) ", node.name, node.data) return test_classes -def scheduler_opts(target_name=None, checks=None, ruleset_path=None, filter_names=None, filters=None): +def scheduler_opts(target_name=None, checks=None, ruleset_path=None, + filter_names=None, filters=None, log_level=None): """ gather all options what have to be set for function class_fmf_generator now it is able set via ENVVARS @@ -119,12 +123,18 @@ def scheduler_opts(target_name=None, checks=None, ruleset_path=None, filter_name filters = os.environ.get("FILTERS", "").split(";") if not filter_names: filter_names = os.environ.get("NAMES", "").split(";") - output = class_fmf_generator(checks, target_name, ruleset_tree_path=ruleset_path, filter_names=filter_names, filters=filters) + if not log_level: + log_level = get_log_level() + output = class_fmf_generator(checks, target_name, + ruleset_tree_path=ruleset_path, + filter_names=filter_names, + filters=filters, + log_level=log_level) return output if __name__ == "__main__": - logging.basicConfig(stream=sys.stdout, level=LOG_LEVEL) + logging.basicConfig(stream=sys.stdout, level=get_log_level()) classes = scheduler_opts() for item in classes: globals()[item] = classes[item] From e1dcf4c76a514998fb250d8e7baaf37a4955f632 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0=C4=8Dotka?= Date: Tue, 10 Jul 2018 12:31:39 +0200 Subject: [PATCH 09/15] add support for pure fmf classes, without any python file, not compatible with colin --- colin/checks/dockerfile.fmf | 12 ++++- fmf_scheduler.py | 97 +++++++++++++++++++++++++------------ 2 files changed, 76 insertions(+), 33 deletions(-) diff --git a/colin/checks/dockerfile.fmf b/colin/checks/dockerfile.fmf index 8ee8e6b3..d4bb6c00 100644 --- a/colin/checks/dockerfile.fmf +++ b/colin/checks/dockerfile.fmf @@ -1,7 +1,7 @@ description: "Generic desription for dockerfile test if not specific description given" +test: "dockerfile.py" /from_tag_not_latest: - test: "dockerfile.py" class: "FromTagNotLatestCheck" message: "In FROM, tag has to be specified and not 'latest'." description: "Using the 'latest' tag may cause unpredictable builds.It is recommended that a specific tag is used in the FROM." @@ -9,7 +9,6 @@ description: "Generic desription for dockerfile test if not specific description tags: ["from", "dockerfile", "baseimage", "latest"] /maintainer_deprecated: - test: "dockerfile.py" class: "MaintainerDeprecatedCheck" message: "Dockerfile instruction `MAINTAINER` is deprecated." description: "Replace with label 'maintainer'." @@ -17,3 +16,12 @@ description: "Generic desription for dockerfile test if not specific description tags: ["maintainer", "dockerfile", "deprecated"] instruction: "MAINTAINER" max_count: 0 + +/test_maintainer_pure_deprecated: + class: "InstructionCountAbstractCheck" + message: "Dockerfile instruction `MAINTAINER` is deprecated." + description: "TEST: Replace with label 'maintainer'." + reference_url: "https://docs.docker.com/engine/reference/builder/#maintainer-deprecated" + tags: ["maintainer", "dockerfile", "deprecated"] + instruction: "MAINTAINER" + max_count: 0 diff --git a/fmf_scheduler.py b/fmf_scheduler.py index e823c95d..5ab28ad5 100755 --- a/fmf_scheduler.py +++ b/fmf_scheduler.py @@ -12,20 +12,31 @@ import unittest from colin.core.target import Target, is_compatible -from colin.core.checks.fmf_check import ExtendedTree, CHECK_DIRECTORY +from colin.core.checks.fmf_check import ExtendedTree, CHECK_DIRECTORY, FMFAbstractCheck from colin.core.constant import PASSED +from colin.core.checks.dockerfile import DockerfileAbstractCheck, DockerfileLabelAbstractCheck,\ + InstructionCountAbstractCheck, InstructionAbstractCheck + +CLASS_MAPPING = {"DockerfileAbstractCheck": DockerfileAbstractCheck, + "DockerfileLabelAbstractCheck": DockerfileLabelAbstractCheck, + "InstructionCountAbstractCheck": InstructionCountAbstractCheck, + "InstructionAbstractCheck": InstructionAbstractCheck + } logger = logging.getLogger(__name__) + def get_log_level(): return int(os.environ.get(("DEBUG")) or logging.INFO) + # COPYied from: # https://eli.thegreenplace.net/2014/04/02/dynamically-generating-python-test-cases class DynamicClassBase(unittest.TestCase): """ Basic Derived test Class """ + backendclass = None longMessage = True @@ -45,7 +56,45 @@ def test(self): return test -def class_fmf_generator(fmfpath, target_name, log_level, ruleset_tree_path=None, filter_names=None, filters=None): +def make_base_fmf_class_abstract(node, target, base_class=DynamicClassBase, fmf_class=FMFAbstractCheck): + class_name = node.data["class"] + out_class_name = node.name.rsplit("/", 1)[-1] + outclass = type(out_class_name, (base_class,), { + 'test': make_check_function(target=target), + 'backendclass': type(class_name, (fmf_class, CLASS_MAPPING[class_name],), { + '__doc__': node.data.get("description"), + 'name': out_class_name, + 'metadata': node, + }) + }) + return outclass + + +def make_wrapped_fmf_class(node, target): + first_class_name = node.name.rsplit("/", 1)[-1] + second_class_name = node.data.get("class") + modulepath = os.path.join(os.path.dirname( + node.sources[-1]), node.data["test"]) + modulename = os.path.basename( + node.sources[-1]).split(".", 1)[0] + # in case of referencing use original data tree for info + if "@" in node.name and not os.path.exists(modulepath): + modulepath = os.path.join(os.path.dirname( + node.sources[-2]), node.data["test"]) + modulename = os.path.basename( + node.sources[-2]).split(".", 1)[0] + test_func = make_check_function(target=target) + logger.debug("Try to import: %s from path %s", modulename, modulepath) + moduleimp = imp.load_source(modulename, modulepath) + inernalclass = getattr(moduleimp, second_class_name) + # more verbose output + # full_class_name = '{0}_{1}'.format(first_class_name, second_class_name) + full_class_name = '{0}'.format(first_class_name) + return type(full_class_name, (DynamicClassBase,), {'test': test_func, + 'backendclass': inernalclass + }) + +def nosetests_class_fmf_generator(fmfpath, target_name, log_level, ruleset_tree_path=None, filter_names=None, filters=None): """ generates dynamic test classes for nosetest or unittest scheduler based on FMF metadata. @@ -65,30 +114,15 @@ def class_fmf_generator(fmfpath, target_name, log_level, ruleset_tree_path=None, for node in ruleset_metadatatree.prune(names=filter_names, filters=filters): if node.data.get("class") or node.data.get("test"): logger.debug("node (%s) contains test and class item", node.name) - first_class_name = node.name.rsplit("/", 1)[-1] - second_class_name = node.data.get("class") - logger.debug("searching for %s", first_class_name) - modulepath = os.path.join(os.path.dirname( - node.sources[-1]), node.data["test"]) - modulename = os.path.basename( - node.sources[-1]).split(".", 1)[0] - # in case of referencing use original data tree for info - if "@" in node.name and not os.path.exists(modulepath): - modulepath = os.path.join(os.path.dirname( - node.sources[-2]), node.data["test"]) - modulename = os.path.basename( - node.sources[-2]).split(".", 1)[0] - test_func = make_check_function(target=target) - logger.debug("Try to import: %s from path %s", modulename, modulepath) - moduleimp = imp.load_source(modulename, modulepath) - inernalclass = getattr(moduleimp, second_class_name) - if is_compatible(target_type=target.target_type, check_instance=inernalclass()): - # more verbose output - #full_class_name = '{0}_{1}'.format(first_class_name, second_class_name) - full_class_name = '{0}'.format(first_class_name) - oneclass = type(full_class_name, (DynamicClassBase,), {'test': test_func}) - oneclass.backendclass = inernalclass - test_classes[full_class_name] = oneclass + + if node.data.get("class") in CLASS_MAPPING: + logger.debug("Using pure FMF metadata for %s (class %s)", node.name, node.data.get("class")) + test_class = make_base_fmf_class_abstract(node=node, target=target) + else: + logger.debug("searching for %s", node.name) + test_class = make_wrapped_fmf_class(node=node, target=target) + if is_compatible(target_type=target.target_type, check_instance=test_class.backendclass()): + test_classes[test_class.__name__] = test_class logger.debug("Test added: %s", node.name) else: logger.debug("Test (not target): %s", node.name) @@ -97,6 +131,7 @@ def class_fmf_generator(fmfpath, target_name, log_level, ruleset_tree_path=None, logger.warning("error in fmf config for node (missing test and class items): %s (data: %s) ", node.name, node.data) return test_classes + def scheduler_opts(target_name=None, checks=None, ruleset_path=None, filter_names=None, filters=None, log_level=None): """ @@ -125,11 +160,11 @@ def scheduler_opts(target_name=None, checks=None, ruleset_path=None, filter_names = os.environ.get("NAMES", "").split(";") if not log_level: log_level = get_log_level() - output = class_fmf_generator(checks, target_name, - ruleset_tree_path=ruleset_path, - filter_names=filter_names, - filters=filters, - log_level=log_level) + output = nosetests_class_fmf_generator(checks, target_name, + ruleset_tree_path=ruleset_path, + filter_names=filter_names, + filters=filters, + log_level=log_level) return output From 68a98f614349658bdea62e625aeb4edf801cb2a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0=C4=8Dotka?= Date: Tue, 10 Jul 2018 12:43:46 +0200 Subject: [PATCH 10/15] move readinf CHECK envvar to get_check_path, to be compatible with colin --- colin/core/checks/fmf_check.py | 3 +-- colin/core/ruleset/ruleset.py | 2 +- fmf_scheduler.py | 5 +++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/colin/core/checks/fmf_check.py b/colin/core/checks/fmf_check.py index 94101724..cf3e394c 100644 --- a/colin/core/checks/fmf_check.py +++ b/colin/core/checks/fmf_check.py @@ -11,7 +11,6 @@ from .abstract_check import AbstractCheck from ..ruleset.ruleset import get_checks_path -CHECK_DIRECTORY = os.environ.get("CHECK") or get_checks_path() logger = logging.getLogger(__name__) @@ -77,7 +76,7 @@ def __init__(self, name_id, path=None): :param path: """ self.metadata = self.__receive_fmf_metadata( - path or CHECK_DIRECTORY, name=name_id) + path or get_checks_path(), name=name_id) self.name = self.get_name() def __receive_fmf_metadata(self, fmfpath, name=None, object_list=False): diff --git a/colin/core/ruleset/ruleset.py b/colin/core/ruleset/ruleset.py index 96031b81..51f0ebdc 100644 --- a/colin/core/ruleset/ruleset.py +++ b/colin/core/ruleset/ruleset.py @@ -118,7 +118,7 @@ def get_checks_path(): :return: str (absolute path of directory with checks) """ - rel_path = os.path.join(os.pardir, os.pardir, os.pardir, "checks") + rel_path = os.environ.get("CHECK") or os.path.join(os.pardir, os.pardir, os.pardir, "checks") return os.path.abspath(os.path.join(__file__, rel_path)) diff --git a/fmf_scheduler.py b/fmf_scheduler.py index 5ab28ad5..2d7f2170 100755 --- a/fmf_scheduler.py +++ b/fmf_scheduler.py @@ -12,8 +12,9 @@ import unittest from colin.core.target import Target, is_compatible -from colin.core.checks.fmf_check import ExtendedTree, CHECK_DIRECTORY, FMFAbstractCheck +from colin.core.checks.fmf_check import ExtendedTree, FMFAbstractCheck from colin.core.constant import PASSED +from colin.core.ruleset.ruleset import get_checks_path from colin.core.checks.dockerfile import DockerfileAbstractCheck, DockerfileLabelAbstractCheck,\ InstructionCountAbstractCheck, InstructionAbstractCheck @@ -151,7 +152,7 @@ def scheduler_opts(target_name=None, checks=None, ruleset_path=None, if not target_name: raise EnvironmentError("TARGET envvar is not set.") if not checks: - checks = CHECK_DIRECTORY + checks = get_checks_path() if not ruleset_path: ruleset_path = os.environ.get("RULESETPATH") if not filters: From 71f20433cb66fc1bee989c1117aac5b7190001e8 Mon Sep 17 00:00:00 2001 From: Tomas Tomecek Date: Thu, 12 Jul 2018 10:25:39 +0200 Subject: [PATCH 11/15] fix instruction checks when *_count is 0 Signed-off-by: Tomas Tomecek --- colin/core/checks/dockerfile.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/colin/core/checks/dockerfile.py b/colin/core/checks/dockerfile.py index 43231ce0..8ddc09f6 100644 --- a/colin/core/checks/dockerfile.py +++ b/colin/core/checks/dockerfile.py @@ -91,9 +91,9 @@ def check(self, target): self.max_count) logger.debug(log) passed = True - if self.min_count: + if self.min_count is not None: passed = passed and self.min_count <= count - if self.max_count: + if self.max_count is not None: passed = passed and count <= self.max_count return CheckResult(ok=passed, From 962b8450f1b01562ceb1abe6c701953db86e579a Mon Sep 17 00:00:00 2001 From: Tomas Tomecek Date: Thu, 12 Jul 2018 10:26:51 +0200 Subject: [PATCH 12/15] enable all tests by default Signed-off-by: Tomas Tomecek --- Makefile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Makefile b/Makefile index 00858558..4689a363 100644 --- a/Makefile +++ b/Makefile @@ -2,7 +2,7 @@ TEST_IMAGE_NAME := colin-test TEST_IMAGE_LABELS_NAME := colin-labels -TEST_TARGET = ./tests/integration/ +TEST_TARGET = ./tests check: build-test-image build-labels-image test-in-container From 3229fe5f1e1761a8fc1119b400c0d2e7544c40f5 Mon Sep 17 00:00:00 2001 From: Tomas Tomecek Date: Thu, 12 Jul 2018 10:27:17 +0200 Subject: [PATCH 13/15] tests: more verbose logs easier to trace issues Signed-off-by: Tomas Tomecek --- tests/integration/test_labels.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/integration/test_labels.py b/tests/integration/test_labels.py index bce99f0e..1a35af50 100644 --- a/tests/integration/test_labels.py +++ b/tests/integration/test_labels.py @@ -13,6 +13,8 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . # +import logging + import pytest import colin @@ -31,7 +33,7 @@ def empty_ruleset(): def get_results_from_colin_labels_image(): - return colin.run("colin-labels", ruleset_name="fedora") + return colin.run("colin-labels", ruleset_name="fedora", logging_level=logging.DEBUG) def test_colin_image(): From b29582714a52546232e4b54393cfd251251b6605 Mon Sep 17 00:00:00 2001 From: Tomas Tomecek Date: Thu, 12 Jul 2018 10:33:40 +0200 Subject: [PATCH 14/15] travis: require sudo b/c of sudo apt-... Signed-off-by: Tomas Tomecek --- .travis.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.travis.yml b/.travis.yml index f085fd0a..9b58a362 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,5 +1,5 @@ language: python -sudo: false +sudo: required notifications: email: false From a50310361092f9871b27c6c19472026ebe30f727 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan=20=C5=A0=C4=8Dotka?= Date: Mon, 16 Jul 2018 11:32:03 +0200 Subject: [PATCH 15/15] fix checks location --- colin/checks/dockerfile.fmf | 2 +- colin/core/ruleset/ruleset.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/colin/checks/dockerfile.fmf b/colin/checks/dockerfile.fmf index d4bb6c00..7495f975 100644 --- a/colin/checks/dockerfile.fmf +++ b/colin/checks/dockerfile.fmf @@ -17,7 +17,7 @@ test: "dockerfile.py" instruction: "MAINTAINER" max_count: 0 -/test_maintainer_pure_deprecated: +/test_maintainer_pure: class: "InstructionCountAbstractCheck" message: "Dockerfile instruction `MAINTAINER` is deprecated." description: "TEST: Replace with label 'maintainer'." diff --git a/colin/core/ruleset/ruleset.py b/colin/core/ruleset/ruleset.py index 51f0ebdc..5b1c5f93 100644 --- a/colin/core/ruleset/ruleset.py +++ b/colin/core/ruleset/ruleset.py @@ -118,8 +118,8 @@ def get_checks_path(): :return: str (absolute path of directory with checks) """ - rel_path = os.environ.get("CHECK") or os.path.join(os.pardir, os.pardir, os.pardir, "checks") - return os.path.abspath(os.path.join(__file__, rel_path)) + out_path = os.environ.get("CHECKS") or os.path.join(__file__, os.pardir, os.pardir, os.pardir, "checks") + return os.path.abspath(out_path) def get_ruleset_file(ruleset=None):