From c13884c866a0d20a4333c0e7e9608172c4f957d6 Mon Sep 17 00:00:00 2001 From: Tim Waugh <twaugh@redhat.com> Date: Wed, 19 Nov 2014 11:12:10 +0000 Subject: [PATCH 1/7] New TaskLint model as part of Task. This is to hold the output from running dockerfile_lint. --- dbs/admin.py | 3 ++- dbs/models.py | 7 +++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/dbs/admin.py b/dbs/admin.py index a18edef..756e31f 100644 --- a/dbs/admin.py +++ b/dbs/admin.py @@ -1,8 +1,9 @@ from __future__ import absolute_import, division, generators, nested_scopes, print_function, unicode_literals, with_statement from django.contrib import admin -from .models import TaskData, Task, Rpm, Registry, YumRepo, Image, ImageRegistryRelation +from .models import TaskData, TaskLint, Task, Rpm, Registry, YumRepo, Image, ImageRegistryRelation +admin.site.register(TaskLint) admin.site.register(TaskData) admin.site.register(Task) admin.site.register(Rpm) diff --git a/dbs/models.py b/dbs/models.py index cb8a473..38b0050 100644 --- a/dbs/models.py +++ b/dbs/models.py @@ -20,6 +20,12 @@ def __unicode__(self): return json.dumps(json.loads(self.json), indent=4) +class TaskLint(models.Model): + lint = models.TextField() + + def __unicode__(self): + return self.lint + class Task(models.Model): STATUS_PENDING = 1 @@ -48,6 +54,7 @@ class Task(models.Model): type = models.IntegerField(choices=_TYPE_NAMES.items()) owner = models.CharField(max_length=38) task_data = models.ForeignKey(TaskData) + task_lint = models.ForeignKey(TaskLint, null=True, blank=True) log = models.TextField(blank=True, null=True) class Meta: From caa380ddb72adfa657e08ef4451760a75eff771a Mon Sep 17 00:00:00 2001 From: Tim Waugh <twaugh@redhat.com> Date: Wed, 19 Nov 2014 11:17:29 +0000 Subject: [PATCH 2/7] New 'linter' task for running dockerfile_lint and marking up the Dockerfile. --- dbs/lint.py | 163 +++++++++++++++++++++++++++++++++++++++++++++++++++ dbs/tasks.py | 30 ++++++++++ 2 files changed, 193 insertions(+) create mode 100644 dbs/lint.py diff --git a/dbs/lint.py b/dbs/lint.py new file mode 100644 index 0000000..4fde34c --- /dev/null +++ b/dbs/lint.py @@ -0,0 +1,163 @@ +from __future__ import absolute_import, division, generators, nested_scopes, print_function, unicode_literals, with_statement + +import git +import shutil +import subprocess +import tempfile +import os +import json + +def html_escape(s): + # In python3 we can use html.escape(s, quote=False) + return s.replace("&", "&").replace("<", "<").replace(">", ">") + +class DockerfileLint: + rules = "/usr/lib/node_modules/dockerfile_lint/sample_rules.yaml" + + PF_CLASSES = { 'error': + { 'alert': '<div class="alert alert-danger">', + 'icon': """ + <span class="pficon-layered"> + <span class="pficon pficon-error-octagon"></span> + <span class="pficon pficon-error-exclamation"></span> + </span>""" }, + + 'warn': + { 'alert': '<div class="alert alert-warning">', + 'icon': """ + <span class="pficon-layered"> + <span class="pficon pficon-warning-triangle"></span> + <span class="pficon pficon-warning-exclamation"></span> + </span>""" }, + + 'info': + { 'alert': '<div class="alert alert-info">', + 'icon': """ + <span class="pficon pficon-info"></span>"""} + } + + def __init__ (self, git_url, git_path=None, git_commit=None): + self._git_url = git_url + self._git_path = git_path + self._git_commit = git_commit + self._temp_dir = None + + def __del__ (self): + if self._temp_dir: + try: + shutil.rmtree(self._temp_dir) + except (IOError, OSError) as exc: + pass + + def _get_dockerfile (self): + self._temp_dir = tempfile.mkdtemp () + git.Repo.clone_from (self._git_url, self._temp_dir) + if self._git_path: + if self._git_path.endswith('Dockerfile'): + git_df_dir = os.path.dirname(self._git_path) + df_path = os.path.abspath(os.path.join(self._temp_dir, + git_df_dir)) + else: + df_path = os.path.abspath(os.path.join(self._temp_dir, + self._git_path)) + else: + df_path = self._temp_dir + + self._Dockerfile = os.path.join(df_path, "Dockerfile") + + def _run_dockerfile_lint (self): + with open ("/dev/null", "rw") as devnull: + dfl = subprocess.Popen (["dockerfile_lint", + "-j", + "-r", self.rules, + "-f", self._Dockerfile], + stdin=devnull, + stdout=subprocess.PIPE, + stderr=devnull, + close_fds=True) + + (stdout, stderr) = dfl.communicate () + jsonstr = stdout.decode () + self._lint = json.loads (jsonstr) + + def _mark_dockerfile (self): + out = "" + with open(self._Dockerfile, "r") as df: + dflines = df.readlines () + dflines.append ("\n") # Extra line to bind 'absent' messages to + lastline = len (dflines) + msgs_by_linenum = {} + for severity in ["error", "warn", "info"]: + for msg in self._lint[severity]["data"]: + if "line" in msg: + linenum = msg["line"] + else: + linenum = lastline + + msgs = msgs_by_linenum.get (linenum, []) + msgs.append (msg) + msgs_by_linenum[linenum] = msgs + + linenum = 1 + for line in dflines: + msgs = msgs_by_linenum.get (linenum, []) + linenum += 1 + if not msgs: + continue + + display_line = False + msgout = "" + for msg in msgs: + if "line" in msg: + display_line = True + + level = msg["level"] + classes = self.PF_CLASSES.get (level) + if not classes: + continue + + msgout += classes["alert"] + classes["icon"] + "\n" + msgout += (" <strong>" + + html_escape (msg["message"]) + + "</strong> ") + description = msg.get ("description", "None") + if description != "None": + msgout += html_escape (description) + url = msg.get ("reference_url", "None") + if url != "None": + if type (url) == list: + url = reduce (lambda a, b: a + b, url) + + msgout += (' <a href="%s\" class="alert-link">' + '(more info)</a>' % url) + msgout += "\n</div>\n" + + if display_line: + out += (("<pre>%d:" % linenum) + + html_escape (line).rstrip () + "</pre>\n") + + out += msgout + + if out == "": + out = """ +<div class='alert alert-success'> + <span class='pficon pficon-ok'></span> + <strong>Looks good!</strong> This Dockerfile has no output from dockerfile_lint. +</div> +""" + + self._html_markup = out + + def run (self): + self._get_dockerfile () + self._run_dockerfile_lint () + self._mark_dockerfile () + return self._html_markup + + def get_json (self): + return self._lint + +if __name__ == "__main__": + git_url = "https://github.com/TomasTomecek/docker-hello-world.git" + lint = DockerfileLint (git_url) + print (lint.run ()) diff --git a/dbs/tasks.py b/dbs/tasks.py index e565c31..2ce2f20 100644 --- a/dbs/tasks.py +++ b/dbs/tasks.py @@ -3,7 +3,37 @@ from celery import shared_task from dock.core import DockerBuilder, DockerTasker from dock.outer import PrivilegedDockerBuilder +from dbs.lint import DockerfileLint +class LintErrors(Exception): + """ + This exception indicates the build was not attempted due to + lint errors. + """ + +@shared_task +def linter(git_url, git_path=None, git_commit=None): + """ + run dockerfile_lint on the Dockerfile we want to build + + :param git_url: url to git repo + :param git_path: path to dockerfile within git repo (default is ./Dockerfile) + :param git_commit: which commit to checkout (master by default) + :return: HTML markup of Dockerfile with dockerfile_lint messages + """ + json = None + try: + lint = DockerfileLint (git_url, git_path, git_commit) + html_markup = lint.run () + json = lint.get_json () + except OSError as exc: + # Perhaps dockerfile_lint is not installed + html_markup = "Executing dockerfile_lint: %s" % exc.strerror + except ValueError as exc: + # Perhaps there was a problem parsing the JSON output + html_markup = "Internal error: %s" % exc.message + finally: + return { "json": json, "html_markup": html_markup } @shared_task def build_image_hostdocker( From fa4d1782cc83a9f1abc477717eb460c2610b9929 Mon Sep 17 00:00:00 2001 From: Tim Waugh <twaugh@redhat.com> Date: Wed, 19 Nov 2014 11:20:07 +0000 Subject: [PATCH 3/7] Handle callbacks correctly when tasks fail. --- dbs/api/core.py | 1 + dbs/task_api.py | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/dbs/api/core.py b/dbs/api/core.py index 772746b..61eb38d 100644 --- a/dbs/api/core.py +++ b/dbs/api/core.py @@ -59,6 +59,7 @@ def new_image_callback(task_id, response_tuple): t.status = Task.STATUS_SUCCESS else: + logger.debug("task failed: %s" % repr (response_tuple)) t.status = Task.STATUS_FAILED t.save() diff --git a/dbs/task_api.py b/dbs/task_api.py index b68cc76..75da762 100644 --- a/dbs/task_api.py +++ b/dbs/task_api.py @@ -21,7 +21,11 @@ def watch_task(task, callback, kwargs=None): :return: None """ - response = task.wait() + try: + response = task.wait() + except Exception as exc: + response = exc + if kwargs: callback(response, **kwargs) else: From e8edfcba9b24f4887f700e5012234c5803eefcf6 Mon Sep 17 00:00:00 2001 From: Tim Waugh <twaugh@redhat.com> Date: Wed, 19 Nov 2014 11:21:03 +0000 Subject: [PATCH 4/7] Chain 'linter' task together with build tasks. The API call for building an image now starts a celery task chain and returns the celery task ID for that chain. It also gives a callback when the 'linter' task has finished. --- dbs/api/core.py | 13 +++++++++++-- dbs/task_api.py | 47 ++++++++++++++++++++++++++++++++++++++--------- dbs/tasks.py | 15 ++++++++++++--- 3 files changed, 61 insertions(+), 14 deletions(-) diff --git a/dbs/api/core.py b/dbs/api/core.py index 61eb38d..2697694 100644 --- a/dbs/api/core.py +++ b/dbs/api/core.py @@ -8,7 +8,7 @@ from django.core.exceptions import ObjectDoesNotExist from django.shortcuts import get_object_or_404 -from dbs.models import Task, TaskData, Dockerfile, Image +from dbs.models import Task, TaskData, TaskLint, Dockerfile, Image from dbs.task_api import TaskApi from dbs.utils import chain_dict_get @@ -21,6 +21,13 @@ class ErrorDuringRequest(Exception): """ indicate that there was an error during processing request; e.g. 404, invalid sth... """ +def lint_output_callback(task_id, lint, **kwargs): + t = Task.objects.get(id=task_id) + tl = TaskLint(lint=lint["html_markup"]) + tl.save() + t.task_lint = tl + t.save() + def new_image_callback(task_id, response_tuple): try: response_hash, df, build_log = response_tuple @@ -77,10 +84,12 @@ def build(post_args, **kwargs): type=Task.TYPE_BUILD, owner=owner, task_data=td) t.save() + lint_callback = partial(lint_output_callback, t.id) callback = partial(new_image_callback, t.id) post_args.update({'build_image': "buildroot-fedora", 'local_tag': local_tag, - 'callback': callback}) + 'callback': callback, + 'lint_callback': lint_callback}) task_id = builder_api.build_docker_image(**post_args) t.celery_id = task_id t.save() diff --git a/dbs/task_api.py b/dbs/task_api.py index 75da762..f13827e 100644 --- a/dbs/task_api.py +++ b/dbs/task_api.py @@ -37,7 +37,7 @@ class TaskApi(object): def build_docker_image(self, build_image, git_url, local_tag, git_dockerfile_path=None, git_commit=None, parent_registry=None, target_registries=None, tag=None, repos=None, - callback=None, kwargs=None): + callback=None, lint_callback=None, kwargs=None): """ build docker image from supplied git repo @@ -67,14 +67,43 @@ def build_docker_image(self, build_image, git_url, local_tag, git_dockerfile_pat 'git_commit': git_commit, 'git_dockerfile_path': git_dockerfile_path, 'repos': repos} - task_info = tasks.build_image.apply_async(args=args, kwargs=task_kwargs, - link=tasks.submit_results.s()) - task_id = task_info.task_id - if callback: - t = Thread(target=watch_task, args=(task_info, callback, kwargs)) - #w.daemon = True - t.start() - return task_id + + # The linter task, which runs dockerfile_lint + linter_task = tasks.linter.s(git_url, + git_dockerfile_path, + git_commit) + + # This task builds the image + build_image_task = tasks.build_image.subtask((build_image, + git_url, + local_tag), + **task_kwargs) + + # This task submits the results + submit_results_task = tasks.submit_results.s() + + # Chain the tasks together in the right order and start them + task_chain = (linter_task | + build_image_task | + submit_results_task).apply_async() + + # Call lint_callback when the linter task is done + linter = task_chain.parent.parent # 3rd from last task + lint_watcher = Thread(target=watch_task, + args=(linter, + lint_callback, + kwargs)) + lint_watcher.start() + + # Call callback when the entire chain is done + chain_watcher = Thread(target=watch_task, + args=(task_chain, + callback, + kwargs)) + chain_watcher.start() + + # Return the celery task ID of the chain + return task_chain.task_id def find_dockerfiles_in_git(self): raise NotImplemented() diff --git a/dbs/tasks.py b/dbs/tasks.py index 2ce2f20..9d20c8c 100644 --- a/dbs/tasks.py +++ b/dbs/tasks.py @@ -4,6 +4,7 @@ from dock.core import DockerBuilder, DockerTasker from dock.outer import PrivilegedDockerBuilder from dbs.lint import DockerfileLint +import time class LintErrors(Exception): """ @@ -73,13 +74,14 @@ def build_image_hostdocker( # TODO: postbuild_data = run_postbuild_plugins(d, private_tag) return inspect_data -@shared_task -def build_image(build_image, git_url, local_tag, git_dockerfile_path=None, +@shared_task(throws=(LintErrors,)) +def build_image(lint, build_image, git_url, local_tag, git_dockerfile_path=None, git_commit=None, parent_registry=None, target_registries=None, tag=None, repos=None, store_results=True): """ build docker image from provided arguments inside privileged container + :param lint: output from linter task :param build_image: name of the build image (supplied docker image is built inside this image) :param git_url: url to git repo :param local_tag: image is known within the service with this tag @@ -93,6 +95,13 @@ def build_image(build_image, git_url, local_tag, git_dockerfile_path=None, in local docker registry :return: dict with data from docker inspect """ + if lint and lint["json"]: + count = lint["json"]["error"]["count"] + if count > 0: + time.sleep (1) # Shouldn't be needed but seems to be + raise LintErrors("Build aborted: %d dockerfile_lint errors" % + count) + db = PrivilegedDockerBuilder(build_image, { "git_url": git_url, "local_tag": local_tag, @@ -138,4 +147,4 @@ def submit_results(result): """ # 2 requests, one for 'finished', other for data print(result) - + return result From 4aab11c1ba82b8cc16b610a7bd92f9594904db0a Mon Sep 17 00:00:00 2001 From: Tim Waugh <twaugh@redhat.com> Date: Wed, 19 Nov 2014 11:22:34 +0000 Subject: [PATCH 5/7] Display task's lint output in the Task Detail page. --- dbs/web/templates/dbs/task_detail.html | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/dbs/web/templates/dbs/task_detail.html b/dbs/web/templates/dbs/task_detail.html index 6b5f2af..5ccf892 100644 --- a/dbs/web/templates/dbs/task_detail.html +++ b/dbs/web/templates/dbs/task_detail.html @@ -49,6 +49,10 @@ <h1>Task Detail</h1> <li><h4>Logs</h4></li> <li><pre>{{ task.log|linebreaks }}</pre></li> {% endif %} + {% if task.task_lint != nil %} + <li><h4>Dockerfile lint</h4> + {{ task.task_lint|safe }}</li> + {% endif %} </ul> {% endblock %} From 877aa2b6379cbb97c21181e87b5d654187fbf55e Mon Sep 17 00:00:00 2001 From: Tim Waugh <twaugh@redhat.com> Date: Wed, 19 Nov 2014 11:23:12 +0000 Subject: [PATCH 6/7] Updated README.md for dockerfile_lint install instructions. --- README.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/README.md b/README.md index 560545e..8418e5d 100644 --- a/README.md +++ b/README.md @@ -65,6 +65,10 @@ in Firewall. For Fedora 21 this can be done so: firewall-cmd --permanent --add-port=8000/tcp ./manage.py runserver 0.0.0.0:8000 +For dockerfile_lint support, you'll need to install it: + + sudo npm install -g git+https://github.com/redhataccess/dockerfile_lint + Manipulate with the data ------------------------ From b985977f9d0d5562a3fb89c01fab94d914b3c483 Mon Sep 17 00:00:00 2001 From: Tim Waugh <twaugh@redhat.com> Date: Wed, 19 Nov 2014 13:16:12 +0000 Subject: [PATCH 7/7] lint: Remove tempdir eagerly, not lazily. --- dbs/lint.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/dbs/lint.py b/dbs/lint.py index 4fde34c..81598fa 100644 --- a/dbs/lint.py +++ b/dbs/lint.py @@ -46,7 +46,7 @@ def __del__ (self): if self._temp_dir: try: shutil.rmtree(self._temp_dir) - except (IOError, OSError) as exc: + except (IOError, OSError, AttributeError) as exc: pass def _get_dockerfile (self): @@ -152,6 +152,14 @@ def run (self): self._get_dockerfile () self._run_dockerfile_lint () self._mark_dockerfile () + + if self._temp_dir: + try: + shutil.rmtree(self._temp_dir) + self._temp_dir = None + except (IOError, OSError) as exc: + pass + return self._html_markup def get_json (self):