Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FMF metadata support to colin #141

Closed
wants to merge 15 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
language: python
sudo: false
sudo: required
notifications:
email: false

Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
1 change: 1 addition & 0 deletions colin/checks/.fmf/version
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
1
24 changes: 24 additions & 0 deletions colin/checks/best_practices.fmf
Original file line number Diff line number Diff line change
@@ -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"]
27 changes: 27 additions & 0 deletions colin/checks/dockerfile.fmf
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
description: "Generic desription for dockerfile test if not specific description given"
test: "dockerfile.py"

/from_tag_not_latest:
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:
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

/test_maintainer_pure:
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
26 changes: 5 additions & 21 deletions colin/checks/dockerfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,18 +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"

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"])
class FromTagNotLatestCheck(FMFAbstractCheck, DockerfileAbstractCheck):
name, metadata = FMFAbstractCheck.get_metadata("from_tag_not_latest")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder whether there is a nicer way of doing this. Calling functions on class' attributes outside of methods seems sketchy.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, I had also troubles with this, but actually there is probably not better way how to do it without bigger changes in colin. Because current colin implementation does static inspection of classes, to find checks based on class attribute name. If you have some idea I'll be happy to change it to better way


def check(self, target):
im = ImageName.parse(target.instance.baseimage)
Expand All @@ -41,14 +34,5 @@ def check(self, target):
logs=[])


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)
class MaintainerDeprecatedCheck(FMFAbstractCheck, InstructionCountAbstractCheck):
name, metadata = FMFAbstractCheck.get_metadata("maintainer_deprecated")
11 changes: 11 additions & 0 deletions colin/checks/labels.fmf
Original file line number Diff line number Diff line change
@@ -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: Null
15 changes: 3 additions & 12 deletions colin/checks/labels.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#

from colin.core.checks.labels import LabelAbstractCheck
from colin.core.checks.fmf_check import FMFAbstractCheck


class ArchitectureLabelCheck(LabelAbstractCheck):
Expand Down Expand Up @@ -228,18 +229,8 @@ def __init__(self):
value_regex=None)


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 MaintainerLabelCheck(FMFAbstractCheck, LabelAbstractCheck):
name, metadata = FMFAbstractCheck.get_metadata("maintainer_label")


class NameLabelCheck(LabelAbstractCheck):
Expand Down
4 changes: 2 additions & 2 deletions colin/core/checks/dockerfile.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
148 changes: 148 additions & 0 deletions colin/core/checks/fmf_check.py
Original file line number Diff line number Diff line change
@@ -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
from ..ruleset.ruleset import get_checks_path


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/[email protected] 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 get_checks_path(), 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)
2 changes: 1 addition & 1 deletion colin/core/loader.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
4 changes: 2 additions & 2 deletions colin/core/ruleset/ruleset.py
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ def get_checks_path():

:return: str (absolute path of directory with checks)
"""
rel_path = 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):
Expand Down
Loading