diff --git a/colin/checks/abstract/dockerfile.py b/colin/checks/abstract/dockerfile.py index 84d657f6..f05014dc 100644 --- a/colin/checks/abstract/dockerfile.py +++ b/colin/checks/abstract/dockerfile.py @@ -13,27 +13,64 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . # +import logging +import re +from ..check_utils import check_label +from ..result import CheckResult from .abstract_check import AbstractCheck +logger = logging.getLogger(__name__) + + +def get_instructions_from_dockerfile_parse(dfp, instruction): + """ + Get the list of instruction dictionary for given instruction name. + (Subset of DockerfileParser.structure only for given instruction.) + + :param dfp: DockerfileParser + :param instruction: str + :return: list + """ + return [inst for inst in dfp.structure if inst["instruction"] == instruction] + class DockerfileCheck(AbstractCheck): pass -class InstructionCheck(AbstractCheck): +class InstructionCheck(DockerfileCheck): - def __init__(self, name, message, description, reference_url, tags, instruction, regex, required): + def __init__(self, name, message, description, reference_url, tags, instruction, value_regex, required): super().__init__(name, message, description, reference_url, tags) self.instruction = instruction - self.regex = regex + self.value_regex = value_regex self.required = required def check(self, target): - pass + instructions = get_instructions_from_dockerfile_parse(target.instance, self.instruction) + pattern = re.compile(self.value_regex) + logs = [] + passed = True + for inst in instructions: + match = bool(pattern.match(inst["value"])) + passed = match == self.required + log = "Value for instruction {} {}mach regex: '{}'.".format(inst["content"], + "" if match else "does not ", + self.value_regex) + logs.append(log) + logger.debug(log) + return CheckResult(ok=passed, + severity=self.severity, + description=self.description, + message=self.message, + reference_url=self.reference_url, + check_name=self.name, + logs=logs) -class InstructionCountCheck(AbstractCheck): + +class InstructionCountCheck(DockerfileCheck): def __init__(self, name, message, description, reference_url, tags, instruction, min_count=None, max_count=None): super().__init__(name, message, description, reference_url, tags) @@ -42,4 +79,47 @@ def __init__(self, name, message, description, reference_url, tags, instruction, self.max_count = max_count def check(self, target): - pass + count = len(get_instructions_from_dockerfile_parse(target.instance, self.instruction)) + + log = "Found {} occurrences of the {} instruction. Needed: min {} | max {}".format(count, + self.instruction, + self.min_count, + self.max_count) + logger.debug(log) + passed = True + if self.min_count: + passed = passed and self.min_count <= count + if self.max_count: + passed = passed and count <= self.max_count + + return CheckResult(ok=passed, + severity=self.severity, + description=self.description, + message=self.message, + reference_url=self.reference_url, + check_name=self.name, + logs=[log]) + + +class DockerfileLabelCheck(DockerfileCheck): + + def __init__(self, name, message, description, reference_url, tags, label, required, value_regex=None): + super().__init__(name, message, description, reference_url, tags) + self.label = label + self.required = required + self.value_regex = value_regex + + def check(self, target): + labels = target.instance.labels + passed = check_label(label=self.label, + required=self.required, + value_regex=self.value_regex, + labels=labels) + + return CheckResult(ok=passed, + severity=self.severity, + description=self.description, + message=self.message, + reference_url=self.reference_url, + check_name=self.name, + logs=[]) diff --git a/colin/checks/abstract/envs.py b/colin/checks/abstract/envs.py index f92c7adb..a0825670 100644 --- a/colin/checks/abstract/envs.py +++ b/colin/checks/abstract/envs.py @@ -16,9 +16,9 @@ import re +from ..result import CheckResult from .containers import ContainerCheck from .images import ImageCheck -from ..result import CheckResult class EnvCheck(ContainerCheck, ImageCheck): diff --git a/colin/checks/abstract/filesystem.py b/colin/checks/abstract/filesystem.py index 08553594..5546d80e 100644 --- a/colin/checks/abstract/filesystem.py +++ b/colin/checks/abstract/filesystem.py @@ -14,10 +14,10 @@ # along with this program. If not, see . # +from ...core.exceptions import ColinException +from ..result import CheckResult from .containers import ContainerCheck from .images import ImageCheck -from ..result import CheckResult -from ...core.exceptions import ColinException class FileSystemCheck(ContainerCheck, ImageCheck): diff --git a/colin/checks/abstract/labels.py b/colin/checks/abstract/labels.py index 5ef55cbe..921e59c5 100644 --- a/colin/checks/abstract/labels.py +++ b/colin/checks/abstract/labels.py @@ -13,15 +13,14 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . # - -import re - +from ..check_utils import check_label +from ..result import CheckResult from .containers import ContainerCheck +from .dockerfile import DockerfileCheck from .images import ImageCheck -from ..result import CheckResult -class LabelCheck(ContainerCheck, ImageCheck): +class LabelCheck(ContainerCheck, ImageCheck, DockerfileCheck): def __init__(self, name, message, description, reference_url, tags, label, required, value_regex=None): super().__init__(name, message, description, reference_url, tags) @@ -30,20 +29,10 @@ def __init__(self, name, message, description, reference_url, tags, label, requi self.value_regex = value_regex def check(self, target): - labels = target.instance.get_metadata()["Config"]["Labels"] - present = labels is not None and self.label in labels - - if present: - if self.required and not self.value_regex: - passed = True - elif self.value_regex: - pattern = re.compile(self.value_regex) - passed = bool(pattern.match(labels[self.label])) - else: - passed = False - - else: - passed = not self.required + passed = check_label(label=self.label, + required=self.required, + value_regex=self.value_regex, + labels=target.labels) return CheckResult(ok=passed, severity=self.severity, @@ -54,7 +43,7 @@ def check(self, target): logs=[]) -class DeprecatedLabelCheck(ContainerCheck, ImageCheck): +class DeprecatedLabelCheck(ContainerCheck, ImageCheck, DockerfileCheck): def __init__(self, name, message, description, reference_url, tags, old_label, new_label): super().__init__(name, message, description, reference_url, tags) @@ -62,7 +51,7 @@ def __init__(self, name, message, description, reference_url, tags, old_label, n self.new_label = new_label def check(self, target): - labels = target.instance.get_metadata()["Config"]["Labels"] + labels = target.labels old_present = labels is not None and self.old_label in labels passed = (not old_present) or (self.new_label in labels) diff --git a/colin/checks/check_utils.py b/colin/checks/check_utils.py new file mode 100644 index 00000000..3a4a520c --- /dev/null +++ b/colin/checks/check_utils.py @@ -0,0 +1,27 @@ +import re + + +def check_label(label, required, value_regex, labels): + """ + Check if the label is required and match the regex + + :param label: str + :param required: bool (if the presence means pass or not) + :param value_regex: str + :param labels: [str] + :return: bool (required==True: True if the label is present and match the regex if specified) + (required==False: True if the label is not present) + """ + present = labels is not None and label in labels + + if present: + if required and not value_regex: + return True + elif value_regex: + pattern = re.compile(value_regex) + return bool(pattern.match(labels[label])) + else: + return False + + else: + return not required diff --git a/colin/checks/dockerfile/from_tag.py b/colin/checks/dockerfile/from_tag.py index e32a2c4a..c9f4510a 100644 --- a/colin/checks/dockerfile/from_tag.py +++ b/colin/checks/dockerfile/from_tag.py @@ -1,14 +1,25 @@ -from colin.checks.abstract.dockerfile import InstructionCheck +from colin.checks.abstract.dockerfile import DockerfileCheck +from colin.checks.result import CheckResult +from colin.core.target import ImageName -class FromTagCheck(InstructionCheck): +class FromTagCheck(DockerfileCheck): def __init__(self): - super().__init__(name="is_tag_latest", - message="", - description="", - reference_url="https://docs.docker.com/engine/reference/builder/#from", - tags=["from", "dockerfile", "latest"], - instruction="FROM", - regex=".*/latest$", - required=False) + super().__init__(name="from_tag_not_latest", + 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" + return CheckResult(ok=passed, + severity=self.severity, + description=self.description, + message=self.message, + reference_url=self.reference_url, + check_name=self.name, + logs=[]) diff --git a/colin/checks/dockerfile/layered_run.py b/colin/checks/dockerfile/layered_run.py deleted file mode 100644 index 05068aed..00000000 --- a/colin/checks/dockerfile/layered_run.py +++ /dev/null @@ -1,15 +0,0 @@ -from colin.checks.abstract.dockerfile import DockerfileCheck - - -class LayeredRunCheck(DockerfileCheck): - - def __init__(self): - super().__init__(name="maintainer_deprecated", - message="", - description="", - reference_url="", - tags=["run", "dockerfile"] - ) - - def check(self, target): - pass diff --git a/colin/checks/result.py b/colin/checks/result.py index b6df2321..d675952c 100644 --- a/colin/checks/result.py +++ b/colin/checks/result.py @@ -18,7 +18,7 @@ from six import iteritems -from ..core.constant import REQUIRED, PASSED, FAILED, WARNING, OPTIONAL +from ..core.constant import FAILED, OPTIONAL, PASSED, REQUIRED, WARNING class CheckResult(object): diff --git a/colin/cli/colin.py b/colin/cli/colin.py index be9215fa..af853f8e 100644 --- a/colin/cli/colin.py +++ b/colin/cli/colin.py @@ -20,12 +20,12 @@ from six import iteritems from ..checks.abstract.abstract_check import AbstractCheck -from ..core.ruleset.ruleset import get_rulesets -from .default_group import DefaultGroup +from ..core.colin import get_checks, run from ..core.constant import COLOURS, OUTPUT_CHARS from ..core.exceptions import ColinException -from ..core.colin import run, get_checks +from ..core.ruleset.ruleset import get_rulesets from ..version import __version__ +from .default_group import DefaultGroup logger = logging.getLogger(__name__) @@ -58,7 +58,7 @@ def cli(): help="Verbose mode.") def check(target, ruleset, ruleset_file, debug, json, stat, verbose): """ - Check the image/container (default). + Check the image/container/dockerfile (default). """ if ruleset and ruleset_file: raise click.BadOptionUsage("Options '--ruleset' and '--file-ruleset' cannot be used together.") diff --git a/colin/core/colin.py b/colin/core/colin.py index 4192c739..5a6123ba 100644 --- a/colin/core/colin.py +++ b/colin/core/colin.py @@ -28,8 +28,9 @@ def run(target, group=None, severity=None, tags=None, ruleset_name=None, ruleset """ Runs the sanity checks for the target. - :param target: str or Image/Container (name of the container or image or Image/Container instance from conu, - dockerfile will be added in the future) + :param target: str + or Image/Container (name of the container/image or Image/Container instance from conu) + or path or file-like object for dockerfile :param group: str (name of the folder with group of checks, if None, all of them will be checked.) :param severity: str (if not None, only those checks will be run -- optional x required x warn ...) :param tags: list of str (if not None, the checks will be filtered by tags.) diff --git a/colin/core/exceptions.py b/colin/core/exceptions.py index a12e5c67..d791b6d9 100644 --- a/colin/core/exceptions.py +++ b/colin/core/exceptions.py @@ -14,6 +14,7 @@ # along with this program. If not, see . # + class ColinException(Exception): """ Generic exception when something goes wrong with colin. """ diff --git a/colin/core/ruleset/ruleset.py b/colin/core/ruleset/ruleset.py index 5bf71f20..8f9541a6 100644 --- a/colin/core/ruleset/ruleset.py +++ b/colin/core/ruleset/ruleset.py @@ -18,10 +18,11 @@ import logging import os +import six from six import iteritems +from ..constant import JSON, RULESET_DIRECTORY from ..exceptions import ColinRulesetException -from ..constant import RULESET_DIRECTORY, JSON from ..loader import load_check_implementation from ..target import is_compatible @@ -70,19 +71,21 @@ def get_checks(self, target_type, group=None, severity=None, tags=None): :param tags: list of str :return: list of check instances """ - check_files = self._get_check_files(group=group, - severity=severity) groups = {} - for (group, check_files) in iteritems(check_files): - checks = [] - for severity, check_file in check_files: + for g in self._get_check_groups(group): + logger.debug("Getting checks for group '{}'.".format(g)) + check_files = [] + for sev, rules in iteritems(self.ruleset_dict[g]): - check_classes = load_check_implementation(path=check_file, severity=severity) - for check_class in check_classes: - if is_compatible(target_type, check_class, severity, tags): - checks.append(check_class) + if severity and severity != sev: + continue - groups[group] = checks + check_files += Ruleset.get_checks_from_rules(rules=rules, + group=g, + target_type=target_type, + severity=sev, + tags=tags) + groups[g] = check_files return groups @staticmethod @@ -97,21 +100,49 @@ def get_check_file(group, name): return os.path.join(get_checks_path(), group, name + ".py") @staticmethod - def get_check_files(group, names, severity): + def get_checks_from_rules(rules, group, target_type, severity, tags): """ - Get the check files from given group with given names. + get check from the list of check items in the resultset file. - :param severity: str + :param rules: [str or dict] :param group: str - :param names: list of str - :return: list of str (paths) + :param target_type: TargetType enum + :param severity: str + :param tags: [str] + :return: list of filtered check instances """ - check_files = [] - for f in names: - check_file = Ruleset.get_check_file(group=group, - name=f) - check_files.append((severity, check_file)) - return check_files + rule_items = [] + for rule in rules: + + if isinstance(rule, six.string_types): + rule_items.append(rule) + elif isinstance(rule, dict): + if target_type and target_type.name not in rule["type"]: + continue + + rule_items += rule["checks"] + + check_instances = [] + for r in rule_items: + logger.debug("Loading check instance for {}".format(r)) + check_instances += load_check_implementation(path=Ruleset.get_check_file(group, r), + severity=severity) + result = [] + for check_instance in check_instances: + if not is_compatible(target_type=target_type, check_instance=check_instance): + logger.debug( + "Check {} not compatible with the target type: {}".format(check_instance.name, target_type.name)) + continue + + if tags: + for t in tags: + if t not in check_instance.tags: + logger.debug("Check not passed the tag control: {}".format(t)) + continue + result.append(check_instance) + logger.debug("Check instance {} added.".format(check_instance.name)) + + return result def _get_check_groups(self, group=None): """ @@ -131,26 +162,6 @@ def _get_check_groups(self, group=None): logger.debug("Found groups: {}.".format(check_groups)) return check_groups - def _get_check_files(self, group=None, severity=None): - """ - Get file names with checks filtered by group and severity. - - :param group: str (if None, all groups will be used) - :param severity: str (if None, all severities will be used) - :return: list of str (absolute paths) - """ - groups = {} - for g in self._get_check_groups(group): - logger.debug("Getting checks for group '{}'.".format(g)) - check_files = [] - for sev, files in iteritems(self.ruleset_dict[g]): - if (not severity) or severity == sev: - check_files += Ruleset.get_check_files(group=g, - names=files, - severity=sev) - groups[g] = check_files - return groups - def get_checks_path(): """ diff --git a/colin/core/target.py b/colin/core/target.py index e2e5f092..5a645af5 100644 --- a/colin/core/target.py +++ b/colin/core/target.py @@ -15,21 +15,39 @@ # import enum +import io import logging +import os from conu import DockerBackend, DockerImagePullPolicy -from conu.apidefs.container import Container, Image +from conu.apidefs.container import Container from conu.apidefs.image import Image from docker.errors import NotFound +from dockerfile_parse import DockerfileParser -from ..core.exceptions import ColinException from ..checks.abstract.containers import ContainerCheck from ..checks.abstract.dockerfile import DockerfileCheck from ..checks.abstract.images import ImageCheck +from ..core.exceptions import ColinException logger = logging.getLogger(__name__) +def is_compatible(target_type, check_instance): + """ + Check the target compatibility with the check instance. + + :param target_type: TargetType enum, if None, returns True + :param check_instance: instance of some Check class + :return: True if the target is None or compatible with the check instance + """ + if not target_type: + return True + return (target_type == TargetType.DOCKERFILE and isinstance(check_instance, DockerfileCheck)) \ + or (target_type == TargetType.CONTAINER and isinstance(check_instance, ContainerCheck)) \ + or (target_type == TargetType.CONTAINER_IMAGE and isinstance(check_instance, ImageCheck)) + + class Target(object): def __init__(self, target, logging_level): @@ -40,14 +58,24 @@ def _get_target_instance(target, logging_level): """ Get the Container/Image instance for the given name. (Container is the first choice.) + or DockerfileParser instance if the target is path or file-like object. - :param target: str or instance of Image/Container - :return: Container/Image + :param target: str + or instance of Image/Container + or file-like object as Dockerfile + :return: Target object """ logger.debug("Finding target '{}'.".format(target)) if isinstance(target, (Image, Container)): + logger.debug("Target is a conu object.") return target + if isinstance(target, io.IOBase): + logger.debug("Target is a dockerfile loaded from the file-like object.") + return DockerfileParser(fileobj=target) + if os.path.isfile(target): + logger.debug("Target is a dockerfile.") + return DockerfileParser(fileobj=open(target)) with DockerBackend(logging_level=logging_level) as backend: @@ -77,13 +105,31 @@ def _get_target_instance(target, logging_level): @property def target_type(self): + """ + Type of the target (image/container/dockerfile) + + :return: TargetType enum + """ if isinstance(self.instance, Image): return TargetType.CONTAINER_IMAGE elif isinstance(self.instance, Container): return TargetType.CONTAINER + elif isinstance(self.instance, DockerfileParser): + return TargetType.DOCKERFILE logger.debug("Target type not found.") raise ColinException("Target type not found.") + @property + def labels(self): + """ + Get list of labels from the target instance. + + :return: [str] + """ + if self.target_type == TargetType.DOCKERFILE: + return self.instance.labels + return self.instance.get_metadata()["Config"]["Labels"] + class TargetType(enum.Enum): DOCKERFILE = 0 @@ -91,15 +137,6 @@ class TargetType(enum.Enum): CONTAINER_IMAGE = 2 -def is_compatible(target_type, check_class, severity, tags): - if not target_type: - return True - # TODO take severity and tags into consideration - return (target_type == TargetType.DOCKERFILE and isinstance(check_class, DockerfileCheck)) \ - or (target_type == TargetType.CONTAINER and isinstance(check_class, ContainerCheck)) \ - or (target_type == TargetType.CONTAINER_IMAGE and isinstance(check_class, ImageCheck)) - - class ImageName(object): def __init__(self, registry=None, namespace=None, repository=None, tag=None, digest=None): self.registry = registry diff --git a/docs/conf.py b/docs/conf.py index ef7907c5..4f26530c 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -1,7 +1,7 @@ # -*- coding: utf-8 -*- # -import sys import os +import sys # If extensions (or modules to document with autodoc) are in another directory, # add these directories to sys.path here. If the directory is relative to the diff --git a/rulesets/default.json b/rulesets/default.json index cc6a993b..bd960355 100644 --- a/rulesets/default.json +++ b/rulesets/default.json @@ -7,10 +7,10 @@ }, "dockerfile": { "required": [ - "from_tag", - "maintainer_deprecated", - "layered_run" + "maintainer_deprecated" ], - "optional": [] + "optional": [ + "from_tag" + ] } } diff --git a/rulesets/fedora.json b/rulesets/fedora.json index 241d2dc8..fa6e3720 100644 --- a/rulesets/fedora.json +++ b/rulesets/fedora.json @@ -26,11 +26,10 @@ }, "dockerfile": { "required": [ - "from_tag", - "maintainer_deprecated" + "maintainer_deprecated", + "from_tag" ], "optional": [ - "layered_run" ] }, "best_practices": { diff --git a/rulesets/redhat.json b/rulesets/redhat.json index dba83ce5..d6eec54f 100644 --- a/rulesets/redhat.json +++ b/rulesets/redhat.json @@ -8,16 +8,6 @@ "usage", "io_k8s_display-name", "io_openshift_tags", - "architecture", - "com_redhat_build-host", - "authoritative-source-url", - "url", - "vendor", - "release", - "build-date", - "distribution-scope", - "vcs-ref", - "vcs-type", "description", "io_k8s_description", "architecture_capital_deprecated", @@ -26,7 +16,25 @@ "version_capital_deprecated", "install_capital_deprecated", "uninstall_capital_deprecated", - "release_capital_deprecated" + "release_capital_deprecated", + { + "type": [ + "Container", + "Image" + ], + "checks": [ + "architecture", + "com_redhat_build-host", + "authoritative-source-url", + "vendor", + "release", + "url", + "build-date", + "distribution-scope", + "vcs-ref", + "vcs-type" + ] + } ], "optional": [ "vcs-url", @@ -37,11 +45,10 @@ }, "dockerfile": { "required": [ - "from_tag", - "maintainer_deprecated" + "maintainer_deprecated", + "from_tag" ], "optional": [ - "layered_run" ] }, "best_practices": { diff --git a/setup.py b/setup.py index 9fd31644..6c06c841 100644 --- a/setup.py +++ b/setup.py @@ -1,6 +1,6 @@ import os -from setuptools import setup, find_packages +from setuptools import find_packages, setup BASE_PATH = os.path.dirname(__file__) @@ -21,7 +21,8 @@ install_requires=[ 'Click', 'six', - 'conu>=0.3.0rc0' + 'conu>=0.3.0rc0', + 'dockerfile_parse' ], entry_points=''' [console_scripts] diff --git a/tests/integration/colin_tests.py b/tests/integration/colin_tests.py index 6b04d589..862fb8d7 100644 --- a/tests/integration/colin_tests.py +++ b/tests/integration/colin_tests.py @@ -1,5 +1,6 @@ import colin + def get_colin_test_image(): return colin.run("colin-test") diff --git a/tests/test_image_name.py b/tests/test_image_name.py index 28e4f7b4..9cd08259 100644 --- a/tests/test_image_name.py +++ b/tests/test_image_name.py @@ -14,7 +14,6 @@ # along with this program. If not, see . # import pytest - from colin.core.target import ImageName