diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index de4fd3b9..7e89cab5 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -40,6 +40,7 @@ env: GIT_AUTHOR_EMAIL: ci-user@github.local GIT_AUTHOR_NAME: CI User + jobs: test: # Don't run scheduled tests on forks @@ -64,6 +65,7 @@ jobs: - unit - venv - contentproviders + - norun # Playwright test - ui include: @@ -74,6 +76,7 @@ jobs: steps: - uses: actions/checkout@v4 + - uses: actions/setup-python@v5 with: python-version: "${{ matrix.python_version }}" diff --git a/dev-requirements.txt b/dev-requirements.txt index 22ba622e..1157007e 100644 --- a/dev-requirements.txt +++ b/dev-requirements.txt @@ -5,3 +5,4 @@ pytest-cov pytest>=7 pyyaml requests_mock +bcrypt \ No newline at end of file diff --git a/docs/source/architecture.md b/docs/source/architecture.md index c145010c..2d14daf3 100644 --- a/docs/source/architecture.md +++ b/docs/source/architecture.md @@ -98,10 +98,8 @@ At the end of the assemble step, the docker image is ready to be used in various ### Push -Optionally, repo2docker can **push** a built image to a [docker registry](https://docs.docker.com/registry/). -This is done as a convenience only (since you can do the same with a `docker push` after using repo2docker -only to build), and implemented in `Repo2Docker.push` method. It is only activated if using the -`--push` commandline flag. +Optionally, repo2docker can **push** a built image to a [docker registry](https://docs.docker.com/registry/), +if you specify the `--push` flag. ### Run diff --git a/repo2docker/app.py b/repo2docker/app.py index 0d817452..b9684c11 100755 --- a/repo2docker/app.py +++ b/repo2docker/app.py @@ -37,7 +37,7 @@ RBuildPack, ) from .engine import BuildError, ContainerEngineException, ImageLoadError -from .utils import ByteSpecification, R2dState, chdir, get_platform +from .utils import ByteSpecification, R2dState, chdir, get_free_port, get_platform class Repo2Docker(Application): @@ -572,56 +572,6 @@ def initialize(self, *args, **kwargs): if self.volumes and not self.run: raise ValueError("Cannot mount volumes if container is not run") - def push_image(self): - """Push docker image to registry""" - client = self.get_engine() - # Build a progress setup for each layer, and only emit per-layer - # info every 1.5s - progress_layers = {} - layers = {} - last_emit_time = time.time() - for chunk in client.push(self.output_image_spec): - if client.string_output: - self.log.info(chunk, extra=dict(phase=R2dState.PUSHING)) - continue - # else this is Docker output - - # each chunk can be one or more lines of json events - # split lines here in case multiple are delivered at once - for line in chunk.splitlines(): - line = line.decode("utf-8", errors="replace") - try: - progress = json.loads(line) - except Exception as e: - self.log.warning("Not a JSON progress line: %r", line) - continue - if "error" in progress: - self.log.error(progress["error"], extra=dict(phase=R2dState.FAILED)) - raise ImageLoadError(progress["error"]) - if "id" not in progress: - continue - # deprecated truncated-progress data - if "progressDetail" in progress and progress["progressDetail"]: - progress_layers[progress["id"]] = progress["progressDetail"] - else: - progress_layers[progress["id"]] = progress["status"] - # include full progress data for each layer in 'layers' data - layers[progress["id"]] = progress - if time.time() - last_emit_time > 1.5: - self.log.info( - "Pushing image\n", - extra=dict( - progress=progress_layers, - layers=layers, - phase=R2dState.PUSHING, - ), - ) - last_emit_time = time.time() - self.log.info( - f"Successfully pushed {self.output_image_spec}", - extra=dict(phase=R2dState.PUSHING), - ) - def run_image(self): """Run docker container from built image @@ -660,7 +610,7 @@ def start_container(self): container_port = int(container_port_proto.split("/", 1)[0]) else: # no port specified, pick a random one - container_port = host_port = str(self._get_free_port()) + container_port = host_port = str(get_free_port()) self.ports = {f"{container_port}/tcp": host_port} self.port = host_port # To use the option --NotebookApp.custom_display_url @@ -744,30 +694,14 @@ def wait_for_container(self, container): if exit_code: sys.exit(exit_code) - def _get_free_port(self): - """ - Hacky method to get a free random port on local host - """ - import socket - - s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - s.bind(("", 0)) - port = s.getsockname()[1] - s.close() - return port - def find_image(self): # if this is a dry run it is Ok for dockerd to be unreachable so we # always return False for dry runs. 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): """ @@ -863,6 +797,12 @@ def build(self): extra=dict(phase=R2dState.BUILDING), ) + extra_build_kwargs = self.extra_build_kwargs.copy() + # Set "push" and "load" parameters in a backwards compat way, without + # having to change the signature of every buildpack + extra_build_kwargs["push"] = self.push + extra_build_kwargs["load"] = self.run + for l in picked_buildpack.build( docker_client, self.output_image_spec, @@ -870,7 +810,7 @@ def build(self): self.build_memory_limit, build_args, self.cache_from, - self.extra_build_kwargs, + extra_build_kwargs, platform=self.platform, ): if docker_client.string_output: @@ -902,8 +842,5 @@ def build(self): def start(self): self.build() - if self.push: - self.push_image() - if self.run: self.run_image() diff --git a/repo2docker/buildpacks/docker.py b/repo2docker/buildpacks/docker.py index 9185307a..b0e58778 100644 --- a/repo2docker/buildpacks/docker.py +++ b/repo2docker/buildpacks/docker.py @@ -3,8 +3,6 @@ import os -import docker - from .base import BuildPack diff --git a/repo2docker/docker.py b/repo2docker/docker.py index b92f807d..e6d3958c 100644 --- a/repo2docker/docker.py +++ b/repo2docker/docker.py @@ -2,16 +2,22 @@ Docker container engine for repo2docker """ +import json +import os import shutil +import subprocess import tarfile import tempfile +from argparse import ArgumentError +from contextlib import ExitStack, contextmanager +from pathlib import Path 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 @@ -58,7 +64,7 @@ class DockerEngine(ContainerEngine): https://docker-py.readthedocs.io/en/4.2.0/api.html#module-docker.api.build """ - string_output = False + string_output = True extra_init_args = Dict( {}, @@ -82,19 +88,11 @@ 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, *, + push=False, + load=False, buildargs=None, cache_from=None, container_limits=None, @@ -109,7 +107,17 @@ def build( ): if not shutil.which("docker"): raise RuntimeError("The docker commandline client must be installed") - args = ["docker", "buildx", "build", "--progress", "plain", "--load"] + args = ["docker", "buildx", "build", "--progress", "plain"] + if load: + if push: + raise ValueError( + "Setting push=True and load=True is currently not supported" + ) + args.append("--load") + + if push: + args.append("--push") + if buildargs: for k, v in buildargs.items(): args += ["--build-arg", f"{k}={v}"] @@ -134,38 +142,73 @@ def build( # place extra args right *before* the path args += self.extra_buildx_build_args - if fileobj: - with tempfile.TemporaryDirectory() as d: - tarf = tarfile.open(fileobj=fileobj) - tarf.extractall(d) + with ExitStack() as stack: + if self.registry_credentials: + stack.enter_context(self.docker_login(**self.registry_credentials)) + if fileobj: + with tempfile.TemporaryDirectory() as d: + tarf = tarfile.open(fileobj=fileobj) + tarf.extractall(d) - args += [d] - - for line in execute_cmd(args, True): - # Simulate structured JSON output from buildx build, since we - # do get structured json output from pushing and running - yield {"stream": line} - else: - # Assume 'path' is passed in - args += [path] + args += [d] - for line in execute_cmd(args, True): - # Simulate structured JSON output from buildx build, since we - # do get structured json output from pushing and running - yield {"stream": line} + yield from execute_cmd(args, True) + else: + # Assume 'path' is passed in + args += [path] - def images(self): - images = self._apiclient.images() - return [Image(tags=image["RepoTags"]) for image in images] + yield from execute_cmd(args, True) 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 - def push(self, image_spec): - if self.registry_credentials: - self._apiclient.login(**self.registry_credentials) - return self._apiclient.push(image_spec, stream=True) + config = json.loads(proc.stdout.decode())[0] + return Image(tags=config["RepoTags"], config=config["Config"]) + + @contextmanager + def docker_login(self, username, password, registry): + # Determine existing DOCKER_CONFIG + old_dc_path = os.environ.get("DOCKER_CONFIG") + if old_dc_path is None: + dc_path = Path("~/.docker/config.json").expanduser() + else: + dc_path = Path(old_dc_path) + + with tempfile.TemporaryDirectory() as d: + new_dc_path = Path(d) / "config.json" + if dc_path.exists(): + # If there is an existing DOCKER_CONFIG, copy it to new location so we inherit + # whatever configuration the user has already set + shutil.copy2(dc_path, new_dc_path) + + os.environ["DOCKER_CONFIG"] = d + try: + subprocess.run( + [ + "docker", + "login", + "--username", + username, + "--password-stdin", + registry, + ], + input=password.encode(), + check=True, + ) + yield + finally: + if old_dc_path: + os.environ["DOCKER_CONFIG"] = old_dc_path + else: + del os.environ["DOCKER_CONFIG"] def run( self, diff --git a/repo2docker/engine.py b/repo2docker/engine.py index a8897a64..310bddbb 100644 --- a/repo2docker/engine.py +++ b/repo2docker/engine.py @@ -6,7 +6,7 @@ import os from abc import ABC, abstractmethod -from traitlets import Dict, default +from traitlets import Dict, TraitError, default, validate from traitlets.config import LoggingConfigurable # Based on https://docker-py.readthedocs.io/en/4.2.0/containers.html @@ -176,6 +176,17 @@ def _registry_credentials_default(self): raise return {} + @validate("registry_credentials") + def _registry_credentials_validate(self, proposal): + """ + Validate form of registry credentials + """ + new = proposal["value"] + if len({"registry", "username", "password"} & new.keys()) != 3: + raise TraitError( + "registry_credentials must have keys 'registry', 'username' and 'password'" + ) + string_output = True """ Whether progress events should be strings or an object. @@ -202,6 +213,8 @@ def __init__(self, *, parent): def build( self, *, + push=False, + load=False, buildargs={}, cache_from=[], container_limits={}, @@ -219,6 +232,10 @@ def build( Parameters ---------- + push: bool + Push the resulting image to a registry + load: bool + Load the resulting image into the container store ready to be run buildargs : dict Dictionary of build arguments cache_from : list[str] @@ -254,19 +271,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? @@ -281,27 +288,6 @@ def inspect_image(self, image): """ raise NotImplementedError("inspect_image not implemented") - def push(self, image_spec): - """ - Push image to a registry - - If the registry_credentials traitlets is set it should be used to - authenticate with the registry before pushing. - - Parameters - ---------- - image_spec : str - The repository spec to push to - - Returns - ------- - A generator of strings. If an error occurs an exception must be thrown. - - If `string_output=True` this should instead be whatever Docker returns: - https://github.com/jupyter/repo2docker/blob/0.11.0/repo2docker/app.py#L469-L495 - """ - raise NotImplementedError("push not implemented") - # Note this is different from the Docker client which has Client.containers.run def run( self, diff --git a/repo2docker/utils.py b/repo2docker/utils.py index 9c2769e1..e71dcf94 100644 --- a/repo2docker/utils.py +++ b/repo2docker/utils.py @@ -1,6 +1,7 @@ import os import platform import re +import socket import subprocess import warnings from contextlib import contextmanager @@ -545,3 +546,14 @@ def get_platform(): else: warnings.warn(f"Unexpected platform '{m}', defaulting to linux/amd64") return "linux/amd64" + + +def get_free_port(): + """ + Hacky method to get a free random port on local host + """ + s = socket.socket(socket.AF_INET, socket.SOCK_STREAM) + s.bind(("", 0)) + port = s.getsockname()[1] + s.close() + return port diff --git a/tests/norun/.gitignore b/tests/norun/.gitignore new file mode 100644 index 00000000..6cbb595e --- /dev/null +++ b/tests/norun/.gitignore @@ -0,0 +1 @@ +tmp-certs-* diff --git a/tests/norun/Dockerfile b/tests/norun/Dockerfile new file mode 100644 index 00000000..bd083ea7 --- /dev/null +++ b/tests/norun/Dockerfile @@ -0,0 +1,2 @@ +# Smallest possible dockerfile, used only for building images to be tested +FROM scratch \ No newline at end of file diff --git a/tests/norun/test_find.py b/tests/norun/test_find.py new file mode 100644 index 00000000..4b8d2bd1 --- /dev/null +++ b/tests/norun/test_find.py @@ -0,0 +1,23 @@ +import secrets +from pathlib import Path + +from repo2docker.__main__ import make_r2d + +HERE = Path(__file__).parent + + +def test_find_image(): + image_name = f"{secrets.token_hex(8)}:latest" + r2d = make_r2d(["--image", image_name, str(HERE)]) + + r2d.build() + + assert r2d.find_image() + + +def test_dont_find_image(): + image_name = f"{secrets.token_hex(8)}:latest" + r2d = make_r2d(["--image", image_name, str(HERE)]) + + # Just don't actually start the build, so image won't be found + assert not r2d.find_image() diff --git a/tests/norun/test_registry.py b/tests/norun/test_registry.py new file mode 100644 index 00000000..2b332a8e --- /dev/null +++ b/tests/norun/test_registry.py @@ -0,0 +1,244 @@ +import json +import os +import secrets +import shutil +import socket +import subprocess +import time +from base64 import b64encode +from pathlib import Path +from tempfile import TemporaryDirectory + +import bcrypt +import pytest +import requests + +from repo2docker.__main__ import make_r2d +from repo2docker.utils import get_free_port + +HERE = Path(__file__).parent + + +@pytest.fixture(scope="session") +def dind(registry): + port = get_free_port() + registry_host, _, _ = registry + + # docker daemon will generate certs here, that we can then use to connect to it. + # put it in current dir than in /tmp because on macos, current dir is likely to + # shared with docker VM so it can be mounted, unlike /tmp + cert_dir = HERE / f"tmp-certs-{secrets.token_hex(8)}" + cert_dir.mkdir() + + dind_image = "docker:dind" + subprocess.check_call(["docker", "pull", dind_image]) + + cmd = [ + "docker", + "run", + "-e", + "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_host, + ] + proc = subprocess.Popen(cmd) + time.sleep(5) + + try: + yield f"tcp://127.0.0.1:{port}", cert_dir + finally: + try: + shutil.rmtree(cert_dir) + except PermissionError: + # Sometimes this is owned by root in CI. is ok, let's let it go + pass + proc.terminate() + proc.wait() + + +@pytest.fixture(scope="session") +def host_ip(): + # Get the IP of the current machine, as we need to use the same IP + # for all our docker commands, *and* the dind we run needs to reach it + # in the same way. + # Thanks to https://stackoverflow.com/a/28950776 + s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) + s.settimeout(0) + try: + # doesn't even have to be reachable + s.connect(("10.254.254.254", 1)) + host_ip = s.getsockname()[0] + finally: + s.close() + + return host_ip + + +@pytest.fixture(scope="session") +def registry(host_ip): + port = get_free_port() + username = "user" + password = secrets.token_hex(16) + bcrypted_pw = bcrypt.hashpw( + password.encode("utf-8"), bcrypt.gensalt(rounds=12) + ).decode("utf-8") + + # We put our password here, and mount it into the container. + # put it in current dir than in /tmp because on macos, current dir is likely to + # shared with docker VM so it can be mounted, unlike /tmp + htpasswd_dir = HERE / f"tmp-certs-{secrets.token_hex(8)}" + htpasswd_dir.mkdir() + (htpasswd_dir / "htpasswd.conf").write_text(f"{username}:{bcrypted_pw}") + + # Explicitly pull the image first so it runs on time + registry_image = "registry:3.0.0-rc.3" + subprocess.check_call(["docker", "pull", registry_image]) + + cmd = [ + "docker", + "run", + "--rm", + "-e", + "REGISTRY_AUTH=htpasswd", + "-e", + "REGISTRY_AUTH_HTPASSWD_REALM=basic", + "-e", + "REGISTRY_AUTH_HTPASSWD_PATH=/opt/htpasswd/htpasswd.conf", + "--mount", + f"type=bind,src={htpasswd_dir},dst=/opt/htpasswd", + "-p", + f"{port}:5000", + registry_image, + ] + proc = subprocess.Popen(cmd) + health_url = f"http://{host_ip}:{port}/v2" + # Wait for the registry to actually come up + for i in range(10): + try: + resp = requests.get(health_url) + if resp.status_code in (401, 200): + break + except requests.ConnectionError: + # The service is not up yet + pass + time.sleep(i) + else: + raise TimeoutError("Test registry did not come up in time") + + try: + yield f"{host_ip}:{port}", username, password + finally: + proc.terminate() + proc.wait() + + +def test_registry_explicit_creds(registry, dind): + """ + Test that we can push to registry when given explicit credentials + """ + registry_host, username, password = registry + image_name = f"{registry_host}/{secrets.token_hex(8)}:latest" + r2d = make_r2d(["--image", image_name, "--push", "--no-run", str(HERE)]) + + docker_host, cert_dir = dind + + old_environ = os.environ.copy() + + try: + os.environ["DOCKER_HOST"] = docker_host + os.environ["DOCKER_CERT_PATH"] = str(cert_dir / "client") + os.environ["DOCKER_TLS_VERIFY"] = "1" + os.environ["CONTAINER_ENGINE_REGISTRY_CREDENTIALS"] = json.dumps( + { + "registry": f"http://{registry_host}", + "username": username, + "password": password, + } + ) + r2d.start() + + # CONTAINER_ENGINE_REGISTRY_CREDENTIALS unfortunately doesn't propagate to docker manifest, so + # let's explicitly set up a docker_config here so we can check if the image exists + with TemporaryDirectory() as d: + (Path(d) / "config.json").write_text( + json.dumps( + { + "auths": { + f"http://{registry_host}": { + "auth": b64encode( + f"{username}:{password}".encode() + ).decode() + } + } + } + ) + ) + env = os.environ.copy() + env["DOCKER_CONFIG"] = d + proc = subprocess.run( + ["docker", "manifest", "inspect", "--insecure", image_name], env=env + ) + assert proc.returncode == 0 + + # Validate that we didn't leak our registry creds into existing docker config + docker_config_path = Path( + os.environ.get("DOCKER_CONFIG", "~/.docker/config.json") + ).expanduser() + if docker_config_path.exists(): + # Just check that our randomly generated password is not in this file + # Can this cause a conflict? Sure, if there's a different randomly generated password in here + # that matches our own randomly generated password. But if you're that unlucky, take cover from the asteroid. + assert password not in docker_config_path.read_text() + finally: + os.environ.clear() + os.environ.update(old_environ) + + +def test_registry_no_explicit_creds(registry, dind): + """ + Test that we can push to registry *without* explicit credentials but reading from a DOCKER_CONFIG + """ + registry_host, username, password = registry + image_name = f"{registry_host}/{secrets.token_hex(8)}:latest" + r2d = make_r2d(["--image", image_name, "--push", "--no-run", str(HERE)]) + + docker_host, cert_dir = dind + + old_environ = os.environ.copy() + + try: + os.environ["DOCKER_HOST"] = docker_host + os.environ["DOCKER_CERT_PATH"] = str(cert_dir / "client") + os.environ["DOCKER_TLS_VERIFY"] = "1" + with TemporaryDirectory() as d: + (Path(d) / "config.json").write_text( + json.dumps( + { + "auths": { + f"http://{registry_host}": { + "auth": b64encode( + f"{username}:{password}".encode() + ).decode() + } + } + } + ) + ) + os.environ["DOCKER_CONFIG"] = d + r2d.start() + + proc = subprocess.run( + ["docker", "manifest", "inspect", "--insecure", image_name] + ) + assert proc.returncode == 0 + finally: + os.environ.clear() + os.environ.update(old_environ) diff --git a/tests/unit/test_app.py b/tests/unit/test_app.py index 0ee4d0b3..04ec9086 100644 --- a/tests/unit/test_app.py +++ b/tests/unit/test_app.py @@ -1,9 +1,7 @@ -import errno from tempfile import TemporaryDirectory from unittest.mock import patch import escapism -import pytest import docker from repo2docker.__main__ import make_r2d @@ -11,34 +9,6 @@ from repo2docker.utils import chdir -def test_find_image(): - images = [{"RepoTags": ["some-org/some-repo:latest"]}] - - with patch("repo2docker.docker.docker.APIClient") as FakeDockerClient: - instance = FakeDockerClient.return_value - instance.images.return_value = images - - r2d = Repo2Docker() - r2d.output_image_spec = "some-org/some-repo" - assert r2d.find_image() - - instance.images.assert_called_with() - - -def test_dont_find_image(): - images = [{"RepoTags": ["some-org/some-image-name:latest"]}] - - with patch("repo2docker.docker.docker.APIClient") as FakeDockerClient: - instance = FakeDockerClient.return_value - instance.images.return_value = images - - r2d = Repo2Docker() - r2d.output_image_spec = "some-org/some-other-image-name" - assert not r2d.find_image() - - instance.images.assert_called_with() - - def test_image_name_remains_unchanged(): # if we specify an image name, it should remain unmodified with TemporaryDirectory() as src: @@ -104,25 +74,3 @@ def test_run_kwargs(repo_with_content): args, kwargs = containers.run.call_args assert "somekey" in kwargs assert kwargs["somekey"] == "somevalue" - - -def test_dryrun_works_without_docker(tmpdir, capsys): - with chdir(tmpdir): - with patch.object(docker, "APIClient") as client: - client.side_effect = docker.errors.DockerException("Error: no Docker") - app = Repo2Docker(dry_run=True) - app.build() - captured = capsys.readouterr() - assert "Error: no Docker" not in captured.err - - -def test_error_log_without_docker(tmpdir, capsys): - with chdir(tmpdir): - with patch.object(docker, "APIClient") as client: - client.side_effect = docker.errors.DockerException("Error: no Docker") - app = Repo2Docker() - - with pytest.raises(SystemExit): - app.build() - captured = capsys.readouterr() - assert "Error: no Docker" in captured.err diff --git a/tests/unit/test_argumentvalidation.py b/tests/unit/test_argumentvalidation.py index 78b96464..1439a653 100644 --- a/tests/unit/test_argumentvalidation.py +++ b/tests/unit/test_argumentvalidation.py @@ -212,34 +212,6 @@ def test_invalid_container_port_protocol_mapping_fail(temp_cwd): assert not validate_arguments(builddir, args_list, "Port specification") -def test_docker_handle_fail(temp_cwd): - """ - Test to check if r2d fails with minimal error message on not being able to connect to docker daemon - """ - args_list = [] - - assert not validate_arguments( - builddir, - args_list, - "Check if docker is running on the host.", - disable_dockerd=True, - ) - - -def test_docker_handle_debug_fail(temp_cwd): - """ - Test to check if r2d fails with helpful error message on not being able to connect to docker daemon and debug enabled - """ - args_list = ["--debug"] - - assert not validate_arguments( - builddir, - args_list, - "Check if docker is running on the host.", - disable_dockerd=True, - ) - - def test_docker_no_build_success(temp_cwd): """ Test to check if r2d succeeds with --no-build argument with not being able to connect to docker daemon diff --git a/tests/unit/test_docker.py b/tests/unit/test_docker.py index c47aa4fa..2fe0b6f5 100644 --- a/tests/unit/test_docker.py +++ b/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) @@ -22,43 +19,3 @@ def test_git_credential_env(): .strip() ) assert out == credential_env - - -class MockDockerEngine(DockerEngine): - def __init__(self, *args, **kwargs): - self._apiclient = Mock() - - -def test_docker_push_no_credentials(): - engine = MockDockerEngine() - - engine.push("image") - - assert len(engine._apiclient.method_calls) == 1 - engine._apiclient.push.assert_called_once_with("image", stream=True) - - -def test_docker_push_dict_credentials(): - engine = MockDockerEngine() - engine.registry_credentials = {"username": "abc", "password": "def"} - - engine.push("image") - - assert len(engine._apiclient.method_calls) == 2 - engine._apiclient.login.assert_called_once_with(username="abc", password="def") - engine._apiclient.push.assert_called_once_with("image", stream=True) - - -def test_docker_push_env_credentials(): - engine = MockDockerEngine() - with patch.dict( - "os.environ", - { - "CONTAINER_ENGINE_REGISTRY_CREDENTIALS": '{"username": "abc", "password": "def"}' - }, - ): - engine.push("image") - - assert len(engine._apiclient.method_calls) == 2 - engine._apiclient.login.assert_called_once_with(username="abc", password="def") - engine._apiclient.push.assert_called_once_with("image", stream=True) diff --git a/tests/unit/test_engine.py b/tests/unit/test_engine.py new file mode 100644 index 00000000..1ccf1dc2 --- /dev/null +++ b/tests/unit/test_engine.py @@ -0,0 +1,18 @@ +import pytest +from traitlets import TraitError + +from repo2docker.engine import ContainerEngine + + +def test_registry_credentials(): + e = ContainerEngine(parent=None) + + # This should be fine + e.registry_credentials = { + "registry": "something", + "username": "something", + "password": "something", + } + + with pytest.raises(TraitError): + e.registry_credentials = {"hi": "bye"} diff --git a/tests/unit/test_ports.py b/tests/unit/test_ports.py index aaebcdab..43fd990b 100644 --- a/tests/unit/test_ports.py +++ b/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)