From 05adf01786cd661640cf3e39019a25ecc8356ddf Mon Sep 17 00:00:00 2001 From: lachmanfrantisek Date: Thu, 21 Jun 2018 13:03:15 +0200 Subject: [PATCH 1/4] Add checks for overriden labels Signed-off-by: lachmanfrantisek --- colin/checks/labels.py | 102 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 101 insertions(+), 1 deletion(-) diff --git a/colin/checks/labels.py b/colin/checks/labels.py index 6977a552..27cece06 100644 --- a/colin/checks/labels.py +++ b/colin/checks/labels.py @@ -14,7 +14,7 @@ # along with this program. If not, see . # -from colin.core.checks.labels import LabelAbstractCheck +from colin.core.checks.labels import LabelAbstractCheck, OverriddenLabelAbstractCheck class ArchitectureLabelCheck(LabelAbstractCheck): @@ -389,3 +389,103 @@ def __init__(self): labels=["version"], required=True, value_regex=None) + + +class SummaryOverriddenLabelCheck(OverriddenLabelAbstractCheck): + name = "summary_overridden" + + def __init__(self): + super(SummaryOverriddenLabelCheck, self) \ + .__init__(message="Label 'summary' has to be overridden.", + description="A short description of the image.", + reference_url="", + tags=['summary', 'overridden', 'label'], + label='summary') + + +class DescriptionOverriddenLabelCheck(OverriddenLabelAbstractCheck): + name = "description_overridden" + + def __init__(self): + super(DescriptionOverriddenLabelCheck, self) \ + .__init__(message="Label 'description' has to be overridden.", + description="Detailed description of the image.", + reference_url="", + tags=['description', 'overridden', 'label'], + label='description') + + +class IoK8sDescriptionOverriddenLabelCheck(OverriddenLabelAbstractCheck): + name = "io.k8s.description_overridden" + + def __init__(self): + super(IoK8sDescriptionOverriddenLabelCheck, self) \ + .__init__(message="Label 'io.k8s.description' has to be overridden.", + description="Description of the container displayed in Kubernetes", + reference_url="", + tags=['description', 'io.k8s.description', 'overridden', 'label'], + label='io.k8s.description') + + +class IoK8sDisplayNameOverriddenLabelCheck(OverriddenLabelAbstractCheck): + name = "io.k8s.display-name_overridden" + + def __init__(self): + super(IoK8sDisplayNameOverriddenLabelCheck, self) \ + .__init__(message="Label 'io.k8s.display-name' has to be overridden.", + description="This label is used to display a human readable name" + " of an image inside the Image / Repo Overview page.", + reference_url="", + tags=['io.k8s.display-name', 'overridden', 'label'], + label='io.k8s.display-name') + + +class IoOpenShiftTagsOverriddenLabelCheck(OverriddenLabelAbstractCheck): + name = "io.openshift.tags_overridden" + + def __init__(self): + super(IoOpenShiftTagsOverriddenLabelCheck, self) \ + .__init__(message="Label 'io.openshift.tags' has to be overridden.", + description="The primary purpose of this label is to include" + " all relevant search terms for this image.", + reference_url="", + tags=['io.openshift.tags', 'overridden', 'label'], + label='io.openshift.tags') + + +class ComRedhatComponentOverriddenLabelCheck(OverriddenLabelAbstractCheck): + name = "com.redhat.component_overridden" + + def __init__(self): + super(ComRedhatComponentOverriddenLabelCheck, self) \ + .__init__(message="Label 'com.redhat.component' has to be overridden.", + description="The Bugzilla component name where bugs against" + " this container should be reported by users.", + reference_url="", + tags=['com.redhat.component', 'overridden', 'label'], + label='com.redhat.component') + + +class NameOverriddenLabelCheck(OverriddenLabelAbstractCheck): + name = "name_overridden" + + def __init__(self): + super(NameOverriddenLabelCheck, self) \ + .__init__(message="Label 'name' has to be overridden.", + description="This label is used to display a human readable name" + " of an image inside the Image / Repo Overview page.", + reference_url="", + tags=['name', 'overridden', 'label'], + label='name') + + +class VersionOverriddenLabelCheck(OverriddenLabelAbstractCheck): + name = "version_overridden" + + def __init__(self): + super(VersionOverriddenLabelCheck, self) \ + .__init__(message="Label 'version' has to be overridden.", + description="Version of the image.", + reference_url="", + tags=['version', 'overridden', 'label'], + label='version') From 2c9cd4ee4c3160c4dcdbb5688ecad3d3fe10b8f8 Mon Sep 17 00:00:00 2001 From: lachmanfrantisek Date: Thu, 21 Jun 2018 16:42:42 +0200 Subject: [PATCH 2/4] Add check for the overridden labels Signed-off-by: lachmanfrantisek --- colin/core/checks/labels.py | 51 +++++++++++++++++++++++- colin/core/target.py | 79 ++++++++++++++++++++++++++++--------- 2 files changed, 111 insertions(+), 19 deletions(-) diff --git a/colin/core/checks/labels.py b/colin/core/checks/labels.py index 795cb331..76017ae0 100644 --- a/colin/core/checks/labels.py +++ b/colin/core/checks/labels.py @@ -13,11 +13,12 @@ # You should have received a copy of the GNU General Public License # along with this program. If not, see . # -from ..result import CheckResult +from colin.core.target import inspect_object, TargetType from .check_utils import check_label from .containers import ContainerAbstractCheck from .dockerfile import DockerfileAbstractCheck from .images import ImageAbstractCheck +from ..result import CheckResult, FailedCheckResult class LabelAbstractCheck(ContainerAbstractCheck, ImageAbstractCheck, DockerfileAbstractCheck): @@ -65,3 +66,51 @@ def check(self, target): reference_url=self.reference_url, check_name=self.name, logs=[]) + + +class OverriddenLabelAbstractCheck(ContainerAbstractCheck, ImageAbstractCheck, + DockerfileAbstractCheck): + def __init__(self, message, description, reference_url, tags, label, layers_for_base=1): + super(OverriddenLabelAbstractCheck, self) \ + .__init__(message, description, reference_url, tags) + self.label = label + self.layers_for_base = layers_for_base + + def check(self, target): + + if target.target_type == TargetType.IMAGE: + _layer_count = len(inspect_object(target, refresh=False)["RootFS"]["Layers"]) + if _layer_count <= self.layers_for_base: + return CheckResult(ok=True, + description=self.description, + message=self.message, + reference_url=self.reference_url, + check_name=self.name, + logs=["Target is a base image."]) + + present = check_label(labels=[self.label], + required=True, + target_labels=target.labels, + value_regex=None) + + if not present: + return CheckResult(ok=True, + description=self.description, + message=self.message, + reference_url=self.reference_url, + check_name=self.name, + logs=["Label '{}' not present.".format(self.label)]) + + if not target.base_image: + return FailedCheckResult(check=self, logs=["Cannot find parent image."]) + + parent_labels = inspect_object(target.base_image, refresh=False)["Config"]["Labels"] + passed = self.label not in parent_labels or target.labels[self.label] != parent_labels[ + self.label] + + return CheckResult(ok=passed, + description=self.description, + message=self.message, + reference_url=self.reference_url, + check_name=self.name, + logs=[]) diff --git a/colin/core/target.py b/colin/core/target.py index 8494cc05..d70af714 100644 --- a/colin/core/target.py +++ b/colin/core/target.py @@ -26,10 +26,10 @@ from docker.errors import NotFound from dockerfile_parse import DockerfileParser -from ..core.exceptions import ColinException from .checks.containers import ContainerAbstractCheck from .checks.dockerfile import DockerfileAbstractCheck from .checks.images import ImageAbstractCheck +from ..core.exceptions import ColinException logger = logging.getLogger(__name__) @@ -46,12 +46,12 @@ def is_compatible(target_type, check_instance): return True return \ ( - target_type == TargetType.DOCKERFILE and - isinstance(check_instance, DockerfileAbstractCheck) + target_type == TargetType.DOCKERFILE and + isinstance(check_instance, DockerfileAbstractCheck) ) \ or ( - target_type == TargetType.CONTAINER and - isinstance(check_instance, ContainerAbstractCheck) + target_type == TargetType.CONTAINER and + isinstance(check_instance, ContainerAbstractCheck) ) \ or (target_type == TargetType.IMAGE and isinstance(check_instance, ImageAbstractCheck)) @@ -76,6 +76,8 @@ class Target(object): def __init__(self, target, logging_level): self.instance = Target._get_target_instance(target, logging_level=logging_level) + self.logging_level = logging_level + self._base_image = None @staticmethod def _get_target_instance(target, logging_level): @@ -110,23 +112,32 @@ def _get_target_instance(target, logging_level): return cont except NotFound: - image_name = ImageName.parse(target) - logger.debug("Finding image '{}' with tag '{}'.".format(image_name.name, image_name.tag)) - - if image_name.tag: - image = backend.ImageClass(repository=image_name.name, - tag=image_name.tag, - pull_policy=DockerImagePullPolicy.NEVER) - else: - image = backend.ImageClass(repository=image_name.name, - pull_policy=DockerImagePullPolicy.NEVER) - - if image.is_present(): - logger.debug("Target is an image.") + image = Target._create_image_instance(backend=backend, name=target) + if image: return image logger.error("Target is neither image nor container.") raise ColinException("Target not found.") + @staticmethod + def _create_image_instance(backend, name): + image_name = ImageName.parse(name) + logger.debug( + "Finding image '{}' with tag '{}'.".format(image_name.name, image_name.tag)) + + if image_name.tag: + image = backend.ImageClass(repository=image_name.name, + tag=image_name.tag, + pull_policy=DockerImagePullPolicy.NEVER) + else: + image = backend.ImageClass(repository=image_name.name, + pull_policy=DockerImagePullPolicy.NEVER) + + if image.is_present(): + logger.debug("Target is an image.") + return image + + return None + @property def target_type(self): """ @@ -155,6 +166,38 @@ def labels(self): # labels won't change, hence refresh=false return inspect_object(self.instance, refresh=False)["Config"]["Labels"] + @property + def base_image(self): + """ + Get a following image: + Dockerfile -> FROM + Image -> Parent + Container -> Image + + :return: Image + """ + if self._base_image: + return self._base_image + + if self.target_type == TargetType.DOCKERFILE: + base_image = self.instance.parent_images + elif self.target_type == TargetType.CONTAINER: + base_image = inspect_object(self.instance, refresh=False)["Image"] + else: + base_image = inspect_object(self.instance, refresh=False)["Parent"] + if not base_image: + # Search through all images + # Look into the /root/buildinfo + pass + + if not base_image: + return None + + with DockerBackend(logging_level=self.logging_level) as backend: + self._base_image = self._create_image_instance(backend=backend, name=base_image) + + return self._base_image + def get_output(self, cmd): if isinstance(cmd, six.string_types): cmd = [cmd] From 69b6a3cb3406a85184923b660fa23d1eabd30791 Mon Sep 17 00:00:00 2001 From: lachmanfrantisek Date: Wed, 27 Jun 2018 11:15:26 +0200 Subject: [PATCH 3/4] Try to find base image from the layers Signed-off-by: lachmanfrantisek --- colin/core/checks/labels.py | 2 +- colin/core/target.py | 17 ++++++++++++++--- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/colin/core/checks/labels.py b/colin/core/checks/labels.py index 76017ae0..7864c920 100644 --- a/colin/core/checks/labels.py +++ b/colin/core/checks/labels.py @@ -79,7 +79,7 @@ def __init__(self, message, description, reference_url, tags, label, layers_for_ def check(self, target): if target.target_type == TargetType.IMAGE: - _layer_count = len(inspect_object(target, refresh=False)["RootFS"]["Layers"]) + _layer_count = len(inspect_object(target.instance, refresh=False)["RootFS"]["Layers"]) if _layer_count <= self.layers_for_base: return CheckResult(ok=True, description=self.description, diff --git a/colin/core/target.py b/colin/core/target.py index d70af714..9a5169a1 100644 --- a/colin/core/target.py +++ b/colin/core/target.py @@ -186,9 +186,20 @@ def base_image(self): else: base_image = inspect_object(self.instance, refresh=False)["Parent"] if not base_image: - # Search through all images - # Look into the /root/buildinfo - pass + with DockerBackend(logging_level=self.logging_level) as backend: + + layers = self.instance.layers() + if len(layers) >= 2 and layers[1].get_id() != '': + self._base_image = layers[1] + return layers[1] + + instance_layers \ + = inspect_object(self.instance, refresh=False)["RootFS"]["Layers"] + for i in backend.list_images(): + i_layers = inspect_object(i, refresh=False)["RootFS"]["Layers"] + if set(i_layers) < set(instance_layers): + self._base_image = i + return i if not base_image: return None From cd53508ea33845eed0ea4bc5c8db8930991cbbfe Mon Sep 17 00:00:00 2001 From: lachmanfrantisek Date: Wed, 27 Jun 2018 14:31:58 +0200 Subject: [PATCH 4/4] Add skeleton for loading parent Dockerfile Signed-off-by: lachmanfrantisek --- colin/core/checks/labels.py | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/colin/core/checks/labels.py b/colin/core/checks/labels.py index 7864c920..247451f5 100644 --- a/colin/core/checks/labels.py +++ b/colin/core/checks/labels.py @@ -102,9 +102,14 @@ def check(self, target): logs=["Label '{}' not present.".format(self.label)]) if not target.base_image: - return FailedCheckResult(check=self, logs=["Cannot find parent image."]) + parent_labels = try_get_parent_labels_from_image(target) + + if parent_labels is None: + return FailedCheckResult(check=self, + logs=["Cannot find parent image or parent Dockerfile."]) + else: + parent_labels = inspect_object(target.base_image, refresh=False)["Config"]["Labels"] - parent_labels = inspect_object(target.base_image, refresh=False)["Config"]["Labels"] passed = self.label not in parent_labels or target.labels[self.label] != parent_labels[ self.label] @@ -114,3 +119,8 @@ def check(self, target): reference_url=self.reference_url, check_name=self.name, logs=[]) + + +def try_get_parent_labels_from_image(image): + # TODO: Get labels from the Dockerfile in /root/buildinfo directory. + return []