Stop using docker-py for anything

Also properly setup TLS for docker daemon to close security hole

Author: yuvipanda

repo2docker/app.py

@@ -750,12 +750,8 @@ def find_image(self):
         if self.dry_run:
             return False
         # check if we already have an image for this content
-        client = self.get_engine()
-        for image in client.images():
-            for tag in image.tags:
-                if tag == self.output_image_spec + ":latest":
-                    return True
-        return False
+        engine = self.get_engine()
+        return engine.inspect_image(self.output_image_spec) is not None
 
     def build(self):
         """

repo2docker/buildpacks/docker.py

@@ -3,8 +3,6 @@
 
 import os
 
-import docker
-
 from .base import BuildPack
 
 

repo2docker/docker.py

@@ -2,6 +2,7 @@
 Docker container engine for repo2docker
 """
 
+import json
 import os
 import shutil
 import subprocess
@@ -13,9 +14,7 @@
 from iso8601 import parse_date
 from traitlets import Dict, List, Unicode
 
-import docker
-
-from .engine import Container, ContainerEngine, ContainerEngineException, Image
+from .engine import Container, ContainerEngine, Image
 from .utils import execute_cmd
 
 
@@ -86,16 +85,6 @@ class DockerEngine(ContainerEngine):
         config=True,
     )
 
-    def __init__(self, *, parent):
-        super().__init__(parent=parent)
-        try:
-            kwargs = docker.utils.kwargs_from_env()
-            kwargs.update(self.extra_init_args)
-            kwargs.setdefault("version", "auto")
-            self._apiclient = docker.APIClient(**kwargs)
-        except docker.errors.DockerException as e:
-            raise ContainerEngineException("Check if docker is running on the host.", e)
-
     def build(
         self,
         *,
@@ -152,13 +141,20 @@ def build(
 
             yield from execute_cmd(args, True)
 
-    def images(self):
-        images = self._apiclient.images()
-        return [Image(tags=image["RepoTags"]) for image in images]
 
     def inspect_image(self, image):
-        image = self._apiclient.inspect_image(image)
-        return Image(tags=image["RepoTags"], config=image["Config"])
+        """
+        Return image configuration if it exists, otherwise None
+        """
+        proc = subprocess.run([
+            "docker", "image", "inspect", image
+        ], capture_output=True)
+
+        if proc.returncode != 0:
+            return None
+
+        config = json.loads(proc.stdout.decode())
+        return Image(tags=config["RepoTags"], config=config["Config"])
 
     @contextmanager
     def docker_login(self, username, password, registry):

repo2docker/engine.py

@@ -254,19 +254,9 @@ def build(
         """
         raise NotImplementedError("build not implemented")
 
-    def images(self):
-        """
-        List images
-
-        Returns
-        -------
-        list[Image] : List of Image objects.
-        """
-        raise NotImplementedError("images not implemented")
-
     def inspect_image(self, image):
         """
-        Get information about an image
+        Get information about an image, or None if the image does not exist
 
         TODO: This is specific to the engine, can we convert it to a standard format?
 

setup.py

@@ -50,7 +50,6 @@ def get_identifier(json):
     version=versioneer.get_version(),
     install_requires=[
         "chardet",
-        "docker!=5.0.0",
         "entrypoints",
         "escapism",
         "iso8601",

tests/registry/test_registry.py

@@ -1,4 +1,5 @@
 import os
+import shutil
 import secrets
 import socket
 import subprocess
@@ -17,33 +18,36 @@
 @pytest.fixture(scope="session")
 def dind(registry, host_ip):
     port = get_free_port()
+
+    # Generate CA certs here so we can securely connect to the docker daemon
+    cert_dir = HERE / f"tmp-certs-{secrets.token_hex(8)}"
+    cert_dir.mkdir()
+
     dind_image = "docker:dind"
     subprocess.check_call(["docker", "pull", dind_image])
-    # This is insecure, because I'm disabling all kinds of authentication on the docker daemon.
-    # FIXME: Use TLS verification.
-    # but also docker this is your own fucking fault for making technical choices that force dockerhub
-    # to be the primary registry, so your registry handling sucks and forces these kinds of difficulties.
+
     cmd = [
         "docker",
         "run",
         "-e",
-        "DOCKER_TLS_CERTDIR=",
+        "DOCKER_TLS_CERTDIR=/opt/certs",
         "--privileged",
+        "--mount", f"type=bind,src={cert_dir},dst=/opt/certs",
         "-p",
         f"{port}:2376",
         dind_image,
         "--host",
         "0.0.0.0:2376",
         "--insecure-registry",
         registry,
-        "--tls=false",
     ]
     proc = subprocess.Popen(cmd)
     time.sleep(5)
 
     try:
-        yield f"tcp://{host_ip}:{port}"
+        yield f"tcp://127.0.0.1:{port}", cert_dir
     finally:
+        shutil.rmtree(cert_dir)
         proc.terminate()
         proc.wait()
 
@@ -100,7 +104,10 @@ def test_registry(registry, dind):
     image_name = f"{registry}/{secrets.token_hex(8)}:latest"
     r2d = make_r2d(["--image", image_name, "--push", "--no-run", str(HERE)])
 
-    os.environ["DOCKER_HOST"] = dind
+    docker_host, cert_dir = dind
+    os.environ["DOCKER_HOST"] = docker_host
+    os.environ["DOCKER_CERT_PATH"] = str(cert_dir / "client")
+    os.environ["DOCKER_TLS_VERIFY"] = "1"
     r2d.start()
 
     proc = subprocess.run(["docker", "manifest", "inspect", "--insecure", image_name])

tests/unit/test_docker.py

@@ -2,9 +2,6 @@
 
 import os
 from subprocess import check_output
-from unittest.mock import Mock, patch
-
-from repo2docker.docker import DockerEngine
 
 repo_root = os.path.abspath(
     os.path.join(os.path.dirname(__file__), os.pardir, os.pardir)

tests/unit/test_ports.py

@@ -11,7 +11,6 @@
 import pytest
 import requests
 
-import docker
 from repo2docker.__main__ import make_r2d
 from repo2docker.app import Repo2Docker
 
@@ -69,10 +68,7 @@ def _cleanup():
         container.reload()
         if container.status == "running":
             container.kill()
-        try:
             container.remove()
-        except docker.errors.NotFound:
-            pass
 
     request.addfinalizer(_cleanup)