From e688c09d68604298bfaf0660f8e4b71d7549e52c Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Mon, 29 Oct 2018 14:46:27 -0700 Subject: [PATCH 01/68] Bump requests dependency in requirements.txt (CVE-2018-18074) Signed-off-by: Joffrey F --- requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/requirements.txt b/requirements.txt index c46a021e2c..c0ce59aeb1 100644 --- a/requirements.txt +++ b/requirements.txt @@ -14,7 +14,7 @@ pyOpenSSL==18.0.0 pyparsing==2.2.0 pypiwin32==219; sys_platform == 'win32' and python_version < '3.6' pypiwin32==223; sys_platform == 'win32' and python_version >= '3.6' -requests==2.14.2 +requests==2.20.0 six==1.10.0 websocket-client==0.40.0 urllib3==1.21.1; python_version == '3.3' \ No newline at end of file From a3111d9e00c15f957af19f5bb5e301cc606f9aeb Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Wed, 31 Oct 2018 18:05:26 -0700 Subject: [PATCH 02/68] Add xfail to ignore 18.09 beta bug Signed-off-by: Joffrey F --- tests/integration/api_build_test.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tests/integration/api_build_test.py b/tests/integration/api_build_test.py index baaf33e3d9..bad411beec 100644 --- a/tests/integration/api_build_test.py +++ b/tests/integration/api_build_test.py @@ -540,6 +540,11 @@ def test_build_in_context_abs_dockerfile(self): ) == sorted(lsdata) @requires_api_version('1.31') + @pytest.mark.xfail( + True, + reason='Currently fails on 18.09: ' + 'https://github.com/moby/moby/issues/37920' + ) def test_prune_builds(self): prune_result = self.client.prune_builds() assert 'SpaceReclaimed' in prune_result From dd7386de30c028300fd3ac9721cc0841fcd065c8 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Thu, 1 Nov 2018 15:23:21 -0700 Subject: [PATCH 03/68] Update version detection script for CI Signed-off-by: Joffrey F --- scripts/versions.py | 39 ++++++++++++++++++++++++--------------- 1 file changed, 24 insertions(+), 15 deletions(-) diff --git a/scripts/versions.py b/scripts/versions.py index 77aaf4f182..7212195dec 100644 --- a/scripts/versions.py +++ b/scripts/versions.py @@ -11,23 +11,24 @@ 'test' ] +STAGES = ['tp', 'beta', 'rc'] -class Version(namedtuple('_Version', 'major minor patch rc edition')): + +class Version(namedtuple('_Version', 'major minor patch stage edition')): @classmethod def parse(cls, version): edition = None version = version.lstrip('v') - version, _, rc = version.partition('-') - if rc: - if 'rc' not in rc: - edition = rc - rc = None - elif '-' in rc: - edition, rc = rc.split('-') - + version, _, stage = version.partition('-') + if stage: + if not any(marker in stage for marker in STAGES): + edition = stage + stage = None + elif '-' in stage: + edition, stage = stage.split('-') major, minor, patch = version.split('.', 3) - return cls(major, minor, patch, rc, edition) + return cls(major, minor, patch, stage, edition) @property def major_minor(self): @@ -38,14 +39,22 @@ def order(self): """Return a representation that allows this object to be sorted correctly with the default comparator. """ - # rc releases should appear before official releases - rc = (0, self.rc) if self.rc else (1, ) - return (int(self.major), int(self.minor), int(self.patch)) + rc + # non-GA releases should appear before GA releases + # Order: tp -> beta -> rc -> GA + if self.stage: + for st in STAGES: + if st in self.stage: + stage = (STAGES.index(st), self.stage) + break + else: + stage = (len(STAGES),) + + return (int(self.major), int(self.minor), int(self.patch)) + stage def __str__(self): - rc = '-{}'.format(self.rc) if self.rc else '' + stage = '-{}'.format(self.stage) if self.stage else '' edition = '-{}'.format(self.edition) if self.edition else '' - return '.'.join(map(str, self[:3])) + edition + rc + return '.'.join(map(str, self[:3])) + edition + stage def main(): From 479f13eff1293731c3cf32db04f191f302fca090 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Wed, 31 Oct 2018 17:04:05 -0700 Subject: [PATCH 04/68] Add paramiko requirement for SSH transport Signed-off-by: Joffrey F --- requirements.txt | 1 + setup.py | 3 +++ 2 files changed, 4 insertions(+) diff --git a/requirements.txt b/requirements.txt index c0ce59aeb1..d13e9d6cad 100644 --- a/requirements.txt +++ b/requirements.txt @@ -9,6 +9,7 @@ enum34==1.1.6 idna==2.5 ipaddress==1.0.18 packaging==16.8 +paramiko==2.4.2 pycparser==2.17 pyOpenSSL==18.0.0 pyparsing==2.2.0 diff --git a/setup.py b/setup.py index 390783d50f..3ad572b3ec 100644 --- a/setup.py +++ b/setup.py @@ -42,6 +42,9 @@ # 'requests[security] >= 2.5.2, != 2.11.0, != 2.12.2' 'tls': ['pyOpenSSL>=17.5.0', 'cryptography>=1.3.4', 'idna>=2.0.0'], + # Only required when connecting using the ssh:// protocol + 'ssh': ['paramiko>=2.4.2'], + } version = None From 338dfb00b1c82d6fbd1bdd6a8acc16f7a4c920fa Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Wed, 31 Oct 2018 17:06:23 -0700 Subject: [PATCH 05/68] Add support for SSH protocol in base_url Signed-off-by: Joffrey F --- docker/api/client.py | 19 ++++++ docker/transport/__init__.py | 5 ++ docker/transport/sshconn.py | 110 +++++++++++++++++++++++++++++++++++ docker/utils/utils.py | 16 +++-- 4 files changed, 145 insertions(+), 5 deletions(-) create mode 100644 docker/transport/sshconn.py diff --git a/docker/api/client.py b/docker/api/client.py index 91da1c893b..197846d105 100644 --- a/docker/api/client.py +++ b/docker/api/client.py @@ -39,6 +39,11 @@ except ImportError: pass +try: + from ..transport import SSHAdapter +except ImportError: + pass + class APIClient( requests.Session, @@ -141,6 +146,18 @@ def __init__(self, base_url=None, version=None, ) self.mount('http+docker://', self._custom_adapter) self.base_url = 'http+docker://localnpipe' + elif base_url.startswith('ssh://'): + try: + self._custom_adapter = SSHAdapter( + base_url, timeout, pool_connections=num_pools + ) + except NameError: + raise DockerException( + 'Install paramiko package to enable ssh:// support' + ) + self.mount('http+docker://ssh', self._custom_adapter) + self._unmount('http://', 'https://') + self.base_url = 'http+docker://ssh' else: # Use SSLAdapter for the ability to specify SSL version if isinstance(tls, TLSConfig): @@ -279,6 +296,8 @@ def _get_raw_response_socket(self, response): self._raise_for_status(response) if self.base_url == "http+docker://localnpipe": sock = response.raw._fp.fp.raw.sock + elif self.base_url.startswith('http+docker://ssh'): + sock = response.raw._fp.fp.channel elif six.PY3: sock = response.raw._fp.fp.raw if self.base_url.startswith("https://"): diff --git a/docker/transport/__init__.py b/docker/transport/__init__.py index abbee182fc..d2cf2a7af3 100644 --- a/docker/transport/__init__.py +++ b/docker/transport/__init__.py @@ -6,3 +6,8 @@ from .npipesocket import NpipeSocket except ImportError: pass + +try: + from .sshconn import SSHAdapter +except ImportError: + pass diff --git a/docker/transport/sshconn.py b/docker/transport/sshconn.py new file mode 100644 index 0000000000..6c9c1196d2 --- /dev/null +++ b/docker/transport/sshconn.py @@ -0,0 +1,110 @@ +import urllib.parse + +import paramiko +import requests.adapters +import six + + +from .. import constants + +if six.PY3: + import http.client as httplib +else: + import httplib + +try: + import requests.packages.urllib3 as urllib3 +except ImportError: + import urllib3 + +RecentlyUsedContainer = urllib3._collections.RecentlyUsedContainer + + +class SSHConnection(httplib.HTTPConnection, object): + def __init__(self, ssh_transport, timeout=60): + super(SSHConnection, self).__init__( + 'localhost', timeout=timeout + ) + self.ssh_transport = ssh_transport + self.timeout = timeout + + def connect(self): + sock = self.ssh_transport.open_session() + sock.settimeout(self.timeout) + sock.exec_command('docker system dial-stdio') + self.sock = sock + + +class SSHConnectionPool(urllib3.connectionpool.HTTPConnectionPool): + scheme = 'ssh' + + def __init__(self, ssh_client, timeout=60, maxsize=10): + super(SSHConnectionPool, self).__init__( + 'localhost', timeout=timeout, maxsize=maxsize + ) + self.ssh_transport = ssh_client.get_transport() + self.timeout = timeout + + def _new_conn(self): + return SSHConnection(self.ssh_transport, self.timeout) + + # When re-using connections, urllib3 calls fileno() on our + # SSH channel instance, quickly overloading our fd limit. To avoid this, + # we override _get_conn + def _get_conn(self, timeout): + conn = None + try: + conn = self.pool.get(block=self.block, timeout=timeout) + + except AttributeError: # self.pool is None + raise urllib3.exceptions.ClosedPoolError(self, "Pool is closed.") + + except six.moves.queue.Empty: + if self.block: + raise urllib3.exceptions.EmptyPoolError( + self, + "Pool reached maximum size and no more " + "connections are allowed." + ) + pass # Oh well, we'll create a new connection then + + return conn or self._new_conn() + + +class SSHAdapter(requests.adapters.HTTPAdapter): + + __attrs__ = requests.adapters.HTTPAdapter.__attrs__ + [ + 'pools', 'timeout', 'ssh_client', + ] + + def __init__(self, base_url, timeout=60, + pool_connections=constants.DEFAULT_NUM_POOLS): + self.ssh_client = paramiko.SSHClient() + self.ssh_client.load_system_host_keys() + + parsed = urllib.parse.urlparse(base_url) + self.ssh_client.connect( + parsed.hostname, parsed.port, parsed.username, + ) + self.timeout = timeout + self.pools = RecentlyUsedContainer( + pool_connections, dispose_func=lambda p: p.close() + ) + super(SSHAdapter, self).__init__() + + def get_connection(self, url, proxies=None): + with self.pools.lock: + pool = self.pools.get(url) + if pool: + return pool + + pool = SSHConnectionPool( + self.ssh_client, self.timeout + ) + self.pools[url] = pool + + return pool + + def close(self): + self.pools.clear() + self.ssh_client.close() diff --git a/docker/utils/utils.py b/docker/utils/utils.py index fe3b9a5767..f8f712349d 100644 --- a/docker/utils/utils.py +++ b/docker/utils/utils.py @@ -250,6 +250,9 @@ def parse_host(addr, is_win32=False, tls=False): addr = addr[8:] elif addr.startswith('fd://'): raise errors.DockerException("fd protocol is not implemented") + elif addr.startswith('ssh://'): + proto = 'ssh' + addr = addr[6:] else: if "://" in addr: raise errors.DockerException( @@ -257,17 +260,20 @@ def parse_host(addr, is_win32=False, tls=False): ) proto = "https" if tls else "http" - if proto in ("http", "https"): + if proto in ("http", "https", "ssh"): address_parts = addr.split('/', 1) host = address_parts[0] if len(address_parts) == 2: path = '/' + address_parts[1] host, port = splitnport(host) - if port is None: - raise errors.DockerException( - "Invalid port: {0}".format(addr) - ) + if port is None or port < 0: + if proto == 'ssh': + port = 22 + else: + raise errors.DockerException( + "Invalid port: {0}".format(addr) + ) if not host: host = DEFAULT_HTTP_HOST From f4e9a1dc2a5a781f5768920b7677b7b8627b5675 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Wed, 31 Oct 2018 17:07:01 -0700 Subject: [PATCH 06/68] Remove misleading fileno method from NpipeSocket class Signed-off-by: Joffrey F --- docker/transport/npipesocket.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/docker/transport/npipesocket.py b/docker/transport/npipesocket.py index c04b39dd66..ef02031640 100644 --- a/docker/transport/npipesocket.py +++ b/docker/transport/npipesocket.py @@ -87,10 +87,6 @@ def detach(self): def dup(self): return NpipeSocket(self._handle) - @check_closed - def fileno(self): - return int(self._handle) - def getpeername(self): return self._address From 1df021ee240140d4d713108822473572c24d5feb Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Wed, 31 Oct 2018 17:08:52 -0700 Subject: [PATCH 07/68] Update tests for ssh protocol compatibility Signed-off-by: Joffrey F --- tests/helpers.py | 4 ++++ tests/integration/api_container_test.py | 1 + 2 files changed, 5 insertions(+) diff --git a/tests/helpers.py b/tests/helpers.py index b36d6d786f..f912bd8d43 100644 --- a/tests/helpers.py +++ b/tests/helpers.py @@ -10,6 +10,7 @@ import socket import docker +import paramiko import pytest @@ -121,6 +122,9 @@ def assert_cat_socket_detached_with_keys(sock, inputs): if getattr(sock, 'family', -9) == getattr(socket, 'AF_UNIX', -1): with pytest.raises(socket.error): sock.sendall(b'make sure the socket is closed\n') + elif isinstance(sock, paramiko.Channel): + with pytest.raises(OSError): + sock.sendall(b'make sure the socket is closed\n') else: sock.sendall(b"make sure the socket is closed\n") data = sock.recv(128) diff --git a/tests/integration/api_container_test.py b/tests/integration/api_container_test.py index 6ce846bb20..249ef7cfcc 100644 --- a/tests/integration/api_container_test.py +++ b/tests/integration/api_container_test.py @@ -1255,6 +1255,7 @@ def test_attach_no_stream(self): assert output == 'hello\n'.encode(encoding='ascii') @pytest.mark.timeout(5) + @pytest.mark.xfail(True, reason='Cancellable events broken over SSH') def test_attach_stream_and_cancel(self): container = self.client.create_container( BUSYBOX, 'sh -c "echo hello && sleep 60"', From 94aa9a89f73e4873350349e79d79b638e101e491 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Wed, 31 Oct 2018 17:09:33 -0700 Subject: [PATCH 08/68] Update tests to properly dispose of client instances in tearDown Signed-off-by: Joffrey F --- tests/integration/api_network_test.py | 2 +- tests/integration/api_plugin_test.py | 16 +++--- tests/integration/api_swarm_test.py | 3 +- tests/integration/base.py | 73 ++++++++++++++------------- 4 files changed, 48 insertions(+), 46 deletions(-) diff --git a/tests/integration/api_network_test.py b/tests/integration/api_network_test.py index b6726d0242..db37cbd974 100644 --- a/tests/integration/api_network_test.py +++ b/tests/integration/api_network_test.py @@ -8,8 +8,8 @@ class TestNetworks(BaseAPIIntegrationTest): def tearDown(self): - super(TestNetworks, self).tearDown() self.client.leave_swarm(force=True) + super(TestNetworks, self).tearDown() def create_network(self, *args, **kwargs): net_name = random_name() diff --git a/tests/integration/api_plugin_test.py b/tests/integration/api_plugin_test.py index 1150b0957a..38f9d12dad 100644 --- a/tests/integration/api_plugin_test.py +++ b/tests/integration/api_plugin_test.py @@ -3,7 +3,7 @@ import docker import pytest -from .base import BaseAPIIntegrationTest, TEST_API_VERSION +from .base import BaseAPIIntegrationTest from ..helpers import requires_api_version SSHFS = 'vieux/sshfs:latest' @@ -13,27 +13,27 @@ class PluginTest(BaseAPIIntegrationTest): @classmethod def teardown_class(cls): - c = docker.APIClient( - version=TEST_API_VERSION, timeout=60, - **docker.utils.kwargs_from_env() - ) + client = cls.get_client_instance() try: - c.remove_plugin(SSHFS, force=True) + client.remove_plugin(SSHFS, force=True) except docker.errors.APIError: pass def teardown_method(self, method): + client = self.get_client_instance() try: - self.client.disable_plugin(SSHFS) + client.disable_plugin(SSHFS) except docker.errors.APIError: pass for p in self.tmp_plugins: try: - self.client.remove_plugin(p, force=True) + client.remove_plugin(p, force=True) except docker.errors.APIError: pass + client.close() + def ensure_plugin_installed(self, plugin_name): try: return self.client.inspect_plugin(plugin_name) diff --git a/tests/integration/api_swarm_test.py b/tests/integration/api_swarm_test.py index dbf3786eb0..b58dabc639 100644 --- a/tests/integration/api_swarm_test.py +++ b/tests/integration/api_swarm_test.py @@ -13,14 +13,13 @@ def setUp(self): self._unlock_key = None def tearDown(self): - super(SwarmTest, self).tearDown() try: if self._unlock_key: self.client.unlock_swarm(self._unlock_key) except docker.errors.APIError: pass - force_leave_swarm(self.client) + super(SwarmTest, self).tearDown() @requires_api_version('1.24') def test_init_swarm_simple(self): diff --git a/tests/integration/base.py b/tests/integration/base.py index 56c23ed4af..262769de40 100644 --- a/tests/integration/base.py +++ b/tests/integration/base.py @@ -29,41 +29,44 @@ def setUp(self): def tearDown(self): client = docker.from_env(version=TEST_API_VERSION) - for img in self.tmp_imgs: - try: - client.api.remove_image(img) - except docker.errors.APIError: - pass - for container in self.tmp_containers: - try: - client.api.remove_container(container, force=True, v=True) - except docker.errors.APIError: - pass - for network in self.tmp_networks: - try: - client.api.remove_network(network) - except docker.errors.APIError: - pass - for volume in self.tmp_volumes: - try: - client.api.remove_volume(volume) - except docker.errors.APIError: - pass - - for secret in self.tmp_secrets: - try: - client.api.remove_secret(secret) - except docker.errors.APIError: - pass - - for config in self.tmp_configs: - try: - client.api.remove_config(config) - except docker.errors.APIError: - pass - - for folder in self.tmp_folders: - shutil.rmtree(folder) + try: + for img in self.tmp_imgs: + try: + client.api.remove_image(img) + except docker.errors.APIError: + pass + for container in self.tmp_containers: + try: + client.api.remove_container(container, force=True, v=True) + except docker.errors.APIError: + pass + for network in self.tmp_networks: + try: + client.api.remove_network(network) + except docker.errors.APIError: + pass + for volume in self.tmp_volumes: + try: + client.api.remove_volume(volume) + except docker.errors.APIError: + pass + + for secret in self.tmp_secrets: + try: + client.api.remove_secret(secret) + except docker.errors.APIError: + pass + + for config in self.tmp_configs: + try: + client.api.remove_config(config) + except docker.errors.APIError: + pass + + for folder in self.tmp_folders: + shutil.rmtree(folder) + finally: + client.close() class BaseAPIIntegrationTest(BaseIntegrationTest): From 6bfe2005e0a700621c094a01b42db39e7c6408de Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Wed, 31 Oct 2018 17:59:26 -0700 Subject: [PATCH 09/68] Clear error for cancellable streams over SSH Signed-off-by: Joffrey F --- docker/types/daemon.py | 12 +++++++++++- tests/integration/api_container_test.py | 5 ++++- tests/integration/models_containers_test.py | 3 +++ 3 files changed, 18 insertions(+), 2 deletions(-) diff --git a/docker/types/daemon.py b/docker/types/daemon.py index ee8624e80a..700f9a90c4 100644 --- a/docker/types/daemon.py +++ b/docker/types/daemon.py @@ -5,6 +5,8 @@ except ImportError: import urllib3 +from ..errors import DockerException + class CancellableStream(object): """ @@ -55,9 +57,17 @@ def close(self): elif hasattr(sock_raw, '_sock'): sock = sock_raw._sock + elif hasattr(sock_fp, 'channel'): + # We're working with a paramiko (SSH) channel, which doesn't + # support cancelable streams with the current implementation + raise DockerException( + 'Cancellable streams not supported for the SSH protocol' + ) else: sock = sock_fp._sock - if isinstance(sock, urllib3.contrib.pyopenssl.WrappedSocket): + + if hasattr(urllib3.contrib, 'pyopenssl') and isinstance( + sock, urllib3.contrib.pyopenssl.WrappedSocket): sock = sock.socket sock.shutdown(socket.SHUT_RDWR) diff --git a/tests/integration/api_container_test.py b/tests/integration/api_container_test.py index 249ef7cfcc..02f3603374 100644 --- a/tests/integration/api_container_test.py +++ b/tests/integration/api_container_test.py @@ -883,6 +883,8 @@ def test_logs_streaming_and_follow(self): assert logs == (snippet + '\n').encode(encoding='ascii') @pytest.mark.timeout(5) + @pytest.mark.skipif(os.environ.get('DOCKER_HOST', '').startswith('ssh://'), + reason='No cancellable streams over SSH') def test_logs_streaming_and_follow_and_cancel(self): snippet = 'Flowering Nights (Sakuya Iyazoi)' container = self.client.create_container( @@ -1255,7 +1257,8 @@ def test_attach_no_stream(self): assert output == 'hello\n'.encode(encoding='ascii') @pytest.mark.timeout(5) - @pytest.mark.xfail(True, reason='Cancellable events broken over SSH') + @pytest.mark.skipif(os.environ.get('DOCKER_HOST', '').startswith('ssh://'), + reason='No cancellable streams over SSH') def test_attach_stream_and_cancel(self): container = self.client.create_container( BUSYBOX, 'sh -c "echo hello && sleep 60"', diff --git a/tests/integration/models_containers_test.py b/tests/integration/models_containers_test.py index ab41ea57de..b48f6fb6ce 100644 --- a/tests/integration/models_containers_test.py +++ b/tests/integration/models_containers_test.py @@ -1,3 +1,4 @@ +import os import tempfile import threading @@ -146,6 +147,8 @@ def test_run_with_streamed_logs(self): assert logs[1] == b'world\n' @pytest.mark.timeout(5) + @pytest.mark.skipif(os.environ.get('DOCKER_HOST', '').startswith('ssh://'), + reason='No cancellable streams over SSH') def test_run_with_streamed_logs_and_cancel(self): client = docker.from_env(version=TEST_API_VERSION) out = client.containers.run( From f302756599a61d6775fbdf2beab8f1de7e0022c4 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Thu, 1 Nov 2018 14:57:29 -0700 Subject: [PATCH 10/68] Rewrite utils.parse_host to detect more invalid addresses. The method now uses parsing methods from urllib to better split provided URLs. Addresses containing query strings, parameters, passwords or fragments no longer fail silently. SSH addresses containing paths are no longer accepted. Signed-off-by: Joffrey F --- docker/transport/sshconn.py | 5 +- docker/utils/utils.py | 130 ++++++++++++++++++++---------------- tests/unit/utils_test.py | 12 +++- 3 files changed, 83 insertions(+), 64 deletions(-) diff --git a/docker/transport/sshconn.py b/docker/transport/sshconn.py index 6c9c1196d2..0f6bb51fc2 100644 --- a/docker/transport/sshconn.py +++ b/docker/transport/sshconn.py @@ -1,10 +1,7 @@ -import urllib.parse - import paramiko import requests.adapters import six - from .. import constants if six.PY3: @@ -82,7 +79,7 @@ def __init__(self, base_url, timeout=60, self.ssh_client = paramiko.SSHClient() self.ssh_client.load_system_host_keys() - parsed = urllib.parse.urlparse(base_url) + parsed = six.moves.urllib_parse.urlparse(base_url) self.ssh_client.connect( parsed.hostname, parsed.port, parsed.username, ) diff --git a/docker/utils/utils.py b/docker/utils/utils.py index f8f712349d..4e04cafdb4 100644 --- a/docker/utils/utils.py +++ b/docker/utils/utils.py @@ -1,10 +1,11 @@ import base64 +import json import os import os.path -import json import shlex -from distutils.version import StrictVersion +import string from datetime import datetime +from distutils.version import StrictVersion import six @@ -13,11 +14,12 @@ if six.PY2: from urllib import splitnport + from urlparse import urlparse else: - from urllib.parse import splitnport + from urllib.parse import splitnport, urlparse DEFAULT_HTTP_HOST = "127.0.0.1" -DEFAULT_UNIX_SOCKET = "http+unix://var/run/docker.sock" +DEFAULT_UNIX_SOCKET = "http+unix:///var/run/docker.sock" DEFAULT_NPIPE = 'npipe:////./pipe/docker_engine' BYTE_UNITS = { @@ -212,81 +214,93 @@ def parse_repository_tag(repo_name): return repo_name, None -# Based on utils.go:ParseHost http://tinyurl.com/nkahcfh -# fd:// protocol unsupported (for obvious reasons) -# Added support for http and https -# Protocol translation: tcp -> http, unix -> http+unix def parse_host(addr, is_win32=False, tls=False): - proto = "http+unix" - port = None path = '' + port = None + host = None + # Sensible defaults if not addr and is_win32: - addr = DEFAULT_NPIPE - + return DEFAULT_NPIPE if not addr or addr.strip() == 'unix://': return DEFAULT_UNIX_SOCKET addr = addr.strip() - if addr.startswith('http://'): - addr = addr.replace('http://', 'tcp://') - if addr.startswith('http+unix://'): - addr = addr.replace('http+unix://', 'unix://') - if addr == 'tcp://': + parsed_url = urlparse(addr) + proto = parsed_url.scheme + if not proto or any([x not in string.ascii_letters + '+' for x in proto]): + # https://bugs.python.org/issue754016 + parsed_url = urlparse('//' + addr, 'tcp') + proto = 'tcp' + + if proto == 'fd': + raise errors.DockerException('fd protocol is not implemented') + + # These protos are valid aliases for our library but not for the + # official spec + if proto == 'http' or proto == 'https': + tls = proto == 'https' + proto = 'tcp' + elif proto == 'http+unix': + proto = 'unix' + + if proto not in ('tcp', 'unix', 'npipe', 'ssh'): raise errors.DockerException( - "Invalid bind address format: {0}".format(addr) + "Invalid bind address protocol: {}".format(addr) ) - elif addr.startswith('unix://'): - addr = addr[7:] - elif addr.startswith('tcp://'): - proto = 'http{0}'.format('s' if tls else '') - addr = addr[6:] - elif addr.startswith('https://'): - proto = "https" - addr = addr[8:] - elif addr.startswith('npipe://'): - proto = 'npipe' - addr = addr[8:] - elif addr.startswith('fd://'): - raise errors.DockerException("fd protocol is not implemented") - elif addr.startswith('ssh://'): - proto = 'ssh' - addr = addr[6:] - else: - if "://" in addr: - raise errors.DockerException( - "Invalid bind address protocol: {0}".format(addr) - ) - proto = "https" if tls else "http" - if proto in ("http", "https", "ssh"): - address_parts = addr.split('/', 1) - host = address_parts[0] - if len(address_parts) == 2: - path = '/' + address_parts[1] - host, port = splitnport(host) + if proto == 'tcp' and not parsed_url.netloc: + # "tcp://" is exceptionally disallowed by convention; + # omitting a hostname for other protocols is fine + raise errors.DockerException( + 'Invalid bind address format: {}'.format(addr) + ) + if any([ + parsed_url.params, parsed_url.query, parsed_url.fragment, + parsed_url.password + ]): + raise errors.DockerException( + 'Invalid bind address format: {}'.format(addr) + ) + + if parsed_url.path and proto == 'ssh': + raise errors.DockerException( + 'Invalid bind address format: no path allowed for this protocol:' + ' {}'.format(addr) + ) + else: + path = parsed_url.path + if proto == 'unix' and parsed_url.hostname is not None: + # For legacy reasons, we consider unix://path + # to be valid and equivalent to unix:///path + path = '/'.join((parsed_url.hostname, path)) + + if proto in ('tcp', 'ssh'): + # parsed_url.hostname strips brackets from IPv6 addresses, + # which can be problematic hence our use of splitnport() instead. + host, port = splitnport(parsed_url.netloc) if port is None or port < 0: - if proto == 'ssh': - port = 22 - else: + if proto != 'ssh': raise errors.DockerException( - "Invalid port: {0}".format(addr) + 'Invalid bind address format: port is required:' + ' {}'.format(addr) ) + port = 22 if not host: host = DEFAULT_HTTP_HOST - else: - host = addr - if proto in ("http", "https") and port == -1: - raise errors.DockerException( - "Bind address needs a port: {0}".format(addr)) + # Rewrite schemes to fit library internals (requests adapters) + if proto == 'tcp': + proto = 'http{}'.format('s' if tls else '') + elif proto == 'unix': + proto = 'http+unix' - if proto == "http+unix" or proto == 'npipe': - return "{0}://{1}".format(proto, host).rstrip('/') - return "{0}://{1}:{2}{3}".format(proto, host, port, path).rstrip('/') + if proto in ('http+unix', 'npipe'): + return "{}://{}".format(proto, path).rstrip('/') + return '{0}://{1}:{2}{3}'.format(proto, host, port, path).rstrip('/') def parse_devices(devices): diff --git a/tests/unit/utils_test.py b/tests/unit/utils_test.py index 8880cfef0f..c862a1cec9 100644 --- a/tests/unit/utils_test.py +++ b/tests/unit/utils_test.py @@ -272,6 +272,11 @@ def test_parse_host(self): 'tcp://', 'udp://127.0.0.1', 'udp://127.0.0.1:2375', + 'ssh://:22/path', + 'tcp://netloc:3333/path?q=1', + 'unix:///sock/path#fragment', + 'https://netloc:3333/path;params', + 'ssh://:clearpassword@host:22', ] valid_hosts = { @@ -281,7 +286,7 @@ def test_parse_host(self): 'http://:7777': 'http://127.0.0.1:7777', 'https://kokia.jp:2375': 'https://kokia.jp:2375', 'unix:///var/run/docker.sock': 'http+unix:///var/run/docker.sock', - 'unix://': 'http+unix://var/run/docker.sock', + 'unix://': 'http+unix:///var/run/docker.sock', '12.234.45.127:2375/docker/engine': ( 'http://12.234.45.127:2375/docker/engine' ), @@ -294,6 +299,9 @@ def test_parse_host(self): '[fd12::82d1]:2375/docker/engine': ( 'http://[fd12::82d1]:2375/docker/engine' ), + 'ssh://': 'ssh://127.0.0.1:22', + 'ssh://user@localhost:22': 'ssh://user@localhost:22', + 'ssh://user@remote': 'ssh://user@remote:22', } for host in invalid_hosts: @@ -304,7 +312,7 @@ def test_parse_host(self): assert parse_host(host, None) == expected def test_parse_host_empty_value(self): - unix_socket = 'http+unix://var/run/docker.sock' + unix_socket = 'http+unix:///var/run/docker.sock' npipe = 'npipe:////./pipe/docker_engine' for val in [None, '']: From 490b2db3ae3ed35bd4f57be08ce554bfacff508e Mon Sep 17 00:00:00 2001 From: Adam Dangoor Date: Mon, 5 Nov 2018 00:04:43 +0000 Subject: [PATCH 11/68] Add a missing space in a log message Signed-off-by: Adam Dangoor --- docker/auth.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/auth.py b/docker/auth.py index 9635f933ec..17158f4ae3 100644 --- a/docker/auth.py +++ b/docker/auth.py @@ -267,7 +267,7 @@ def load_config(config_path=None, config_dict=None): return res log.debug( - "Couldn't find auth-related section ; attempting to interpret" + "Couldn't find auth-related section ; attempting to interpret " "as auth-only file" ) return {'auths': parse_auth(config_dict)} From e237c0ea165e485f23b02b0d598c6a65fd8deb94 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Tue, 6 Nov 2018 14:46:37 -0800 Subject: [PATCH 12/68] Add named parameter to image.save to identify which repository name to use in the resulting tarball Signed-off-by: Joffrey F --- docker/models/images.py | 20 ++++++++++++++++-- tests/integration/models_images_test.py | 27 +++++++++++++++++++++++++ 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/docker/models/images.py b/docker/models/images.py index 7d9ab70b56..28b1fd3ffd 100644 --- a/docker/models/images.py +++ b/docker/models/images.py @@ -59,7 +59,7 @@ def history(self): """ return self.client.api.history(self.id) - def save(self, chunk_size=DEFAULT_DATA_CHUNK_SIZE): + def save(self, chunk_size=DEFAULT_DATA_CHUNK_SIZE, named=False): """ Get a tarball of an image. Similar to the ``docker save`` command. @@ -67,6 +67,12 @@ def save(self, chunk_size=DEFAULT_DATA_CHUNK_SIZE): chunk_size (int): The generator will return up to that much data per iteration, but may return less. If ``None``, data will be streamed as it is received. Default: 2 MB + named (str or bool): If ``False`` (default), the tarball will not + retain repository and tag information for this image. If set + to ``True``, the first tag in the :py:attr:`~tags` list will + be used to identify the image. Alternatively, any element of + the :py:attr:`~tags` list can be used as an argument to use + that specific tag as the saved identifier. Returns: (generator): A stream of raw archive data. @@ -83,7 +89,17 @@ def save(self, chunk_size=DEFAULT_DATA_CHUNK_SIZE): >>> f.write(chunk) >>> f.close() """ - return self.client.api.get_image(self.id, chunk_size) + img = self.id + if named: + img = self.tags[0] if self.tags else img + if isinstance(named, six.string_types): + if named not in self.tags: + raise InvalidArgument( + "{} is not a valid tag for this image".format(named) + ) + img = named + + return self.client.api.get_image(img, chunk_size) def tag(self, repository, tag=None, **kwargs): """ diff --git a/tests/integration/models_images_test.py b/tests/integration/models_images_test.py index ae735baafb..31fab10968 100644 --- a/tests/integration/models_images_test.py +++ b/tests/integration/models_images_test.py @@ -5,6 +5,7 @@ import pytest from .base import BaseIntegrationTest, BUSYBOX, TEST_API_VERSION +from ..helpers import random_name class ImageCollectionTest(BaseIntegrationTest): @@ -108,6 +109,32 @@ def test_save_and_load(self): assert len(result) == 1 assert result[0].id == image.id + def test_save_and_load_repo_name(self): + client = docker.from_env(version=TEST_API_VERSION) + image = client.images.get(BUSYBOX) + additional_tag = random_name() + image.tag(additional_tag) + self.tmp_imgs.append(additional_tag) + image.reload() + with tempfile.TemporaryFile() as f: + stream = image.save(named='{}:latest'.format(additional_tag)) + for chunk in stream: + f.write(chunk) + + f.seek(0) + client.images.remove(additional_tag, force=True) + result = client.images.load(f.read()) + + assert len(result) == 1 + assert result[0].id == image.id + assert '{}:latest'.format(additional_tag) in result[0].tags + + def test_save_name_error(self): + client = docker.from_env(version=TEST_API_VERSION) + image = client.images.get(BUSYBOX) + with pytest.raises(docker.errors.InvalidArgument): + image.save(named='sakuya/izayoi') + class ImageTest(BaseIntegrationTest): From 9987c1bc42bc36160bc7fbdf17849a32fcf11808 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Thu, 8 Nov 2018 15:05:22 -0800 Subject: [PATCH 13/68] Fix docs examples to work with Python 3 Signed-off-by: Joffrey F --- docker/api/daemon.py | 6 ++--- docker/api/image.py | 16 ++++++------ docker/api/mixin.py | 57 ++++++++++++++++++++++++++++++++++++++++++ docker/types/daemon.py | 2 +- 4 files changed, 69 insertions(+), 12 deletions(-) create mode 100644 docker/api/mixin.py diff --git a/docker/api/daemon.py b/docker/api/daemon.py index 76a94cf034..431e7d41cd 100644 --- a/docker/api/daemon.py +++ b/docker/api/daemon.py @@ -42,8 +42,8 @@ def events(self, since=None, until=None, filters=None, decode=None): Example: - >>> for event in client.events() - ... print event + >>> for event in client.events(decode=True) + ... print(event) {u'from': u'image/with:tag', u'id': u'container-id', u'status': u'start', @@ -54,7 +54,7 @@ def events(self, since=None, until=None, filters=None, decode=None): >>> events = client.events() >>> for event in events: - ... print event + ... print(event) >>> # and cancel from another thread >>> events.close() """ diff --git a/docker/api/image.py b/docker/api/image.py index 5f05d8877e..1c1fcde84c 100644 --- a/docker/api/image.py +++ b/docker/api/image.py @@ -352,8 +352,8 @@ def pull(self, repository, tag=None, stream=False, auth_config=None, Example: - >>> for line in cli.pull('busybox', stream=True): - ... print(json.dumps(json.loads(line), indent=4)) + >>> for line in cli.pull('busybox', stream=True, decode=True): + ... print(json.dumps(line, indent=4)) { "status": "Pulling image (latest) from busybox", "progressDetail": {}, @@ -428,12 +428,12 @@ def push(self, repository, tag=None, stream=False, auth_config=None, If the server returns an error. Example: - >>> for line in cli.push('yourname/app', stream=True): - ... print line - {"status":"Pushing repository yourname/app (1 tags)"} - {"status":"Pushing","progressDetail":{},"id":"511136ea3c5a"} - {"status":"Image already pushed, skipping","progressDetail":{}, - "id":"511136ea3c5a"} + >>> for line in cli.push('yourname/app', stream=True, decode=True): + ... print(line) + {'status': 'Pushing repository yourname/app (1 tags)'} + {'status': 'Pushing','progressDetail': {}, 'id': '511136ea3c5a'} + {'status': 'Image already pushed, skipping', 'progressDetail':{}, + 'id': '511136ea3c5a'} ... """ diff --git a/docker/api/mixin.py b/docker/api/mixin.py new file mode 100644 index 0000000000..1353568167 --- /dev/null +++ b/docker/api/mixin.py @@ -0,0 +1,57 @@ +from typing import Any, Dict, Iterable, List, Optional, Union + +import requests + +from ..constants import DEFAULT_DOCKER_API_VERSION, DEFAULT_TIMEOUT_SECONDS + +class BaseMixin(object): + base_url: str = '' + credstore_env: Optional[Dict[str, str]] = None + timeout: int = DEFAULT_TIMEOUT_SECONDS + _auth_configs: Dict[str, Dict] + _general_configs: Dict[str, Dict] + _version: str = DEFAULT_DOCKER_API_VERSION + + def _url(self, pathfmt: str, *args, **kwargs) -> str: + raise NotImplemented + + def _post(self, url: str, **kwargs) -> requests.Response: + raise NotImplemented + + def _get(self, url: str, **kwargs) -> requests.Response: + raise NotImplemented + + def _put(self, url: str, **kwargs) -> requests.Response: + raise NotImplemented + + def _delete(self, url: str, **kwargs) -> requests.Response: + raise NotImplemented + + def _post_json(self, url: str, data: Optional[Union[Dict[str, Any], List[Any]]], **kwargs) -> requests.Response: + raise NotImplemented + + def _raise_for_status(self, response: requests.Response) -> None: + raise NotImplemented + + def _result(self, response: requests.Response, json: bool=False, binary: bool=False) -> Any: + raise NotImplemented + + def _stream_helper(self, response: requests.Response, decode: bool = False) -> Iterable: + raise NotImplemented + + def _get_raw_response_socket(self, response: requests.Response) -> Iterable: + raise NotImplemented + + def _read_from_socket( + self, + response: requests.Response, + stream: bool, + tty: bool = False) -> Union[Iterable[bytes], bytes]: + raise NotImplemented + + def _stream_raw_result( + self, + response: requests.Response, + chunk_size: int = 1, + decode: bool = True) -> Iterable[bytes]: + raise NotImplemented diff --git a/docker/types/daemon.py b/docker/types/daemon.py index 700f9a90c4..af3e5bcb5e 100644 --- a/docker/types/daemon.py +++ b/docker/types/daemon.py @@ -15,7 +15,7 @@ class CancellableStream(object): Example: >>> events = client.events() >>> for event in events: - ... print event + ... print(event) >>> # and cancel from another thread >>> events.close() """ From 1d124a1262c73a85ff6601e8fdef9c7e48b33543 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Thu, 8 Nov 2018 15:32:10 -0800 Subject: [PATCH 14/68] Improve ulimits documentation Signed-off-by: Joffrey F --- docker/api/container.py | 2 +- docker/models/containers.py | 4 ++-- docker/types/containers.py | 17 +++++++++++++++++ docs/api.rst | 1 + 4 files changed, 21 insertions(+), 3 deletions(-) diff --git a/docker/api/container.py b/docker/api/container.py index c59a6d01a5..6967a13acf 100644 --- a/docker/api/container.py +++ b/docker/api/container.py @@ -543,7 +543,7 @@ def create_host_config(self, *args, **kwargs): } ulimits (:py:class:`list`): Ulimits to set inside the container, - as a list of dicts. + as a list of :py:class:`docker.types.Ulimit` instances. userns_mode (str): Sets the user namespace mode for the container when user namespace remapping option is enabled. Supported values are: ``host`` diff --git a/docker/models/containers.py b/docker/models/containers.py index f60ba6e225..98c717421f 100644 --- a/docker/models/containers.py +++ b/docker/models/containers.py @@ -691,8 +691,8 @@ def run(self, image, command=None, stdout=True, stderr=False, } tty (bool): Allocate a pseudo-TTY. - ulimits (:py:class:`list`): Ulimits to set inside the container, as - a list of dicts. + ulimits (:py:class:`list`): Ulimits to set inside the container, + as a list of :py:class:`docker.types.Ulimit` instances. user (str or int): Username or UID to run commands as inside the container. userns_mode (str): Sets the user namespace mode for the container diff --git a/docker/types/containers.py b/docker/types/containers.py index 9dfea8ceb8..13eb4ef37b 100644 --- a/docker/types/containers.py +++ b/docker/types/containers.py @@ -58,6 +58,23 @@ def unset_config(self, key): class Ulimit(DictType): + """ + Create a ulimit declaration to be used with + :py:meth:`~docker.api.container.ContainerApiMixin.create_host_config`. + + Args: + + name (str): Which ulimit will this apply to. A list of valid names can + be found `here `_. + soft (int): The soft limit for this ulimit. Optional. + hard (int): The hard limit for this ulimit. Optional. + + Example: + + nproc_limit = docker.types.Ulimit(name='nproc', soft=1024) + hc = client.create_host_config(ulimits=[nproc_limit]) + container = client.create_container('busybox', 'true', host_config=hc) + """ def __init__(self, **kwargs): name = kwargs.get('name', kwargs.get('Name')) soft = kwargs.get('soft', kwargs.get('Soft')) diff --git a/docs/api.rst b/docs/api.rst index 6931245716..2c2391a803 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -151,4 +151,5 @@ Configuration types .. autoclass:: SwarmExternalCA .. autoclass:: SwarmSpec(*args, **kwargs) .. autoclass:: TaskTemplate +.. autoclass:: Ulimit .. autoclass:: UpdateConfig From d5bc46ad456ca7f8c008baa31f6623d4f58ccdcd Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Thu, 8 Nov 2018 16:20:28 -0800 Subject: [PATCH 15/68] Improved LogConfig documentation Signed-off-by: Joffrey F --- docker/api/container.py | 8 +------ docker/models/containers.py | 8 +------ docker/types/containers.py | 45 ++++++++++++++++++++++++++++++++++--- docs/api.rst | 1 + 4 files changed, 45 insertions(+), 17 deletions(-) diff --git a/docker/api/container.py b/docker/api/container.py index 6967a13acf..39478329ae 100644 --- a/docker/api/container.py +++ b/docker/api/container.py @@ -476,13 +476,7 @@ def create_host_config(self, *args, **kwargs): isolation (str): Isolation technology to use. Default: `None`. links (dict or list of tuples): Either a dictionary mapping name to alias or as a list of ``(name, alias)`` tuples. - log_config (dict): Logging configuration, as a dictionary with - keys: - - - ``type`` The logging driver name. - - ``config`` A dictionary of configuration for the logging - driver. - + log_config (LogConfig): Logging configuration lxc_conf (dict): LXC config. mem_limit (float or str): Memory limit. Accepts float values (which represent the memory limit of the created container in diff --git a/docker/models/containers.py b/docker/models/containers.py index 98c717421f..4cd7d13f41 100644 --- a/docker/models/containers.py +++ b/docker/models/containers.py @@ -576,13 +576,7 @@ def run(self, image, command=None, stdout=True, stderr=False, ``["label1", "label2"]``) links (dict or list of tuples): Either a dictionary mapping name to alias or as a list of ``(name, alias)`` tuples. - log_config (dict): Logging configuration, as a dictionary with - keys: - - - ``type`` The logging driver name. - - ``config`` A dictionary of configuration for the logging - driver. - + log_config (LogConfig): Logging configuration. mac_address (str): MAC address to assign to the container. mem_limit (int or str): Memory limit. Accepts float values (which represent the memory limit of the created container in diff --git a/docker/types/containers.py b/docker/types/containers.py index 13eb4ef37b..d040c0fb5e 100644 --- a/docker/types/containers.py +++ b/docker/types/containers.py @@ -23,6 +23,36 @@ class LogConfigTypesEnum(object): class LogConfig(DictType): + """ + Configure logging for a container, when provided as an argument to + :py:meth:`~docker.api.container.ContainerApiMixin.create_host_config`. + You may refer to the + `official logging driver documentation `_ + for more information. + + Args: + type (str): Indicate which log driver to use. A set of valid drivers + is provided as part of the :py:attr:`LogConfig.types` + enum. Other values may be accepted depending on the engine version + and available logging plugins. + config (dict): A driver-dependent configuration dictionary. Please + refer to the driver's documentation for a list of valid config + keys. + + Example: + + >>> from docker.types import LogConfig + >>> lc = LogConfig(type=LogConfig.types.JSON, config={ + ... 'max-size': '1g', + ... 'labels': 'production_status,geo' + ... }) + >>> hc = client.create_host_config(log_config=lc) + >>> container = client.create_container('busybox', 'true', + ... host_config=hc) + >>> client.inspect_container(container)['HostConfig']['LogConfig'] + {'Type': 'json-file', 'Config': {'labels': 'production_status,geo', 'max-size': '1g'}} + + """ # flake8: noqa types = LogConfigTypesEnum def __init__(self, **kwargs): @@ -50,9 +80,13 @@ def config(self): return self['Config'] def set_config_value(self, key, value): + """ Set a the value for ``key`` to ``value`` inside the ``config`` + dict. + """ self.config[key] = value def unset_config(self, key): + """ Remove the ``key`` property from the ``config`` dict. """ if key in self.config: del self.config[key] @@ -71,9 +105,14 @@ class Ulimit(DictType): Example: - nproc_limit = docker.types.Ulimit(name='nproc', soft=1024) - hc = client.create_host_config(ulimits=[nproc_limit]) - container = client.create_container('busybox', 'true', host_config=hc) + >>> nproc_limit = docker.types.Ulimit(name='nproc', soft=1024) + >>> hc = client.create_host_config(ulimits=[nproc_limit]) + >>> container = client.create_container( + 'busybox', 'true', host_config=hc + ) + >>> client.inspect_container(container)['HostConfig']['Ulimits'] + [{'Name': 'nproc', 'Hard': 0, 'Soft': 1024}] + """ def __init__(self, **kwargs): name = kwargs.get('name', kwargs.get('Name')) diff --git a/docs/api.rst b/docs/api.rst index 2c2391a803..1682128951 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -140,6 +140,7 @@ Configuration types .. autoclass:: Healthcheck .. autoclass:: IPAMConfig .. autoclass:: IPAMPool +.. autoclass:: LogConfig .. autoclass:: Mount .. autoclass:: Placement .. autoclass:: Privileges From 6064947431dd36b8aa33a5f2eb850b03302de5ee Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Thu, 8 Nov 2018 16:52:32 -0800 Subject: [PATCH 16/68] Update links docs and fix bug in normalize_links Signed-off-by: Joffrey F --- docker/api/container.py | 17 ++++++++++------- docker/models/containers.py | 6 ++++-- docker/utils/utils.py | 2 +- 3 files changed, 15 insertions(+), 10 deletions(-) diff --git a/docker/api/container.py b/docker/api/container.py index 39478329ae..8858aa68a3 100644 --- a/docker/api/container.py +++ b/docker/api/container.py @@ -473,9 +473,11 @@ def create_host_config(self, *args, **kwargs): signals and reaps processes init_path (str): Path to the docker-init binary ipc_mode (str): Set the IPC mode for the container. - isolation (str): Isolation technology to use. Default: `None`. - links (dict or list of tuples): Either a dictionary mapping name - to alias or as a list of ``(name, alias)`` tuples. + isolation (str): Isolation technology to use. Default: ``None``. + links (dict): Mapping of links using the + ``{'container': 'alias'}`` format. The alias is optional. + Containers declared in this dict will be linked to the new + container using the provided alias. Default: ``None``. log_config (LogConfig): Logging configuration lxc_conf (dict): LXC config. mem_limit (float or str): Memory limit. Accepts float values @@ -605,9 +607,10 @@ def create_endpoint_config(self, *args, **kwargs): aliases (:py:class:`list`): A list of aliases for this endpoint. Names in that list can be used within the network to reach the container. Defaults to ``None``. - links (:py:class:`list`): A list of links for this endpoint. - Containers declared in this list will be linked to this - container. Defaults to ``None``. + links (dict): Mapping of links for this endpoint using the + ``{'container': 'alias'}`` format. The alias is optional. + Containers declared in this dict will be linked to this + container using the provided alias. Defaults to ``None``. ipv4_address (str): The IP address of this container on the network, using the IPv4 protocol. Defaults to ``None``. ipv6_address (str): The IP address of this container on the @@ -622,7 +625,7 @@ def create_endpoint_config(self, *args, **kwargs): >>> endpoint_config = client.create_endpoint_config( aliases=['web', 'app'], - links=['app_db'], + links={'app_db': 'db', 'another': None}, ipv4_address='132.65.0.123' ) diff --git a/docker/models/containers.py b/docker/models/containers.py index 4cd7d13f41..bba0395eed 100644 --- a/docker/models/containers.py +++ b/docker/models/containers.py @@ -574,8 +574,10 @@ def run(self, image, command=None, stdout=True, stderr=False, ``{"label1": "value1", "label2": "value2"}``) or a list of names of labels to set with empty values (e.g. ``["label1", "label2"]``) - links (dict or list of tuples): Either a dictionary mapping name - to alias or as a list of ``(name, alias)`` tuples. + links (dict): Mapping of links using the + ``{'container': 'alias'}`` format. The alias is optional. + Containers declared in this dict will be linked to the new + container using the provided alias. Default: ``None``. log_config (LogConfig): Logging configuration. mac_address (str): MAC address to assign to the container. mem_limit (int or str): Memory limit. Accepts float values diff --git a/docker/utils/utils.py b/docker/utils/utils.py index 4e04cafdb4..a8e814d7d5 100644 --- a/docker/utils/utils.py +++ b/docker/utils/utils.py @@ -441,7 +441,7 @@ def normalize_links(links): if isinstance(links, dict): links = six.iteritems(links) - return ['{0}:{1}'.format(k, v) for k, v in sorted(links)] + return ['{0}:{1}'.format(k, v) if v else k for k, v in sorted(links)] def parse_env_file(env_file): From 6bfe4c90906b3ff737b256825b5059c893a6a4d1 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Thu, 8 Nov 2018 17:05:42 -0800 Subject: [PATCH 17/68] Document attr caching for Container objects Signed-off-by: Joffrey F --- docker/models/containers.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/docker/models/containers.py b/docker/models/containers.py index bba0395eed..a3fd1a8edf 100644 --- a/docker/models/containers.py +++ b/docker/models/containers.py @@ -15,7 +15,12 @@ class Container(Model): - + """ Local representation of a container object. Detailed configuration may + be accessed through the :py:attr:`attrs` attribute. Note that local + attributes are cached; users may call :py:meth:`reload` to + query the Docker daemon for the current properties, causing + :py:attr:`attrs` to be refreshed. + """ @property def name(self): """ From b927a5f62cf2d66209244129ed3434575c4045f5 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Thu, 8 Nov 2018 17:08:41 -0800 Subject: [PATCH 18/68] Fix incorrect return info for inspect_service Signed-off-by: Joffrey F --- docker/api/service.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docker/api/service.py b/docker/api/service.py index 8b956b63e1..08e2591730 100644 --- a/docker/api/service.py +++ b/docker/api/service.py @@ -197,7 +197,8 @@ def inspect_service(self, service, insert_defaults=None): into the service inspect output. Returns: - ``True`` if successful. + (dict): A dictionary of the server-side representation of the + service, including all relevant properties. Raises: :py:class:`docker.errors.APIError` From 89ee08f511d8a0882d5b034fc5be670f8987a802 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Thu, 8 Nov 2018 17:13:19 -0800 Subject: [PATCH 19/68] Disallow incompatible combination stats(decode=True, stream=False) Signed-off-by: Joffrey F --- docker/api/container.py | 7 ++++++- docker/models/containers.py | 3 ++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/docker/api/container.py b/docker/api/container.py index 8858aa68a3..753e0a57fe 100644 --- a/docker/api/container.py +++ b/docker/api/container.py @@ -1071,7 +1071,8 @@ def stats(self, container, decode=None, stream=True): Args: container (str): The container to stream statistics from decode (bool): If set to true, stream will be decoded into dicts - on the fly. False by default. + on the fly. Only applicable if ``stream`` is True. + False by default. stream (bool): If set to false, only the current stats will be returned instead of a stream. True by default. @@ -1085,6 +1086,10 @@ def stats(self, container, decode=None, stream=True): return self._stream_helper(self._get(url, stream=True), decode=decode) else: + if decode: + raise errors.InvalidArgument( + "decode is only available in conjuction with stream=True" + ) return self._result(self._get(url, params={'stream': False}), json=True) diff --git a/docker/models/containers.py b/docker/models/containers.py index a3fd1a8edf..493b9fc732 100644 --- a/docker/models/containers.py +++ b/docker/models/containers.py @@ -385,7 +385,8 @@ def stats(self, **kwargs): Args: decode (bool): If set to true, stream will be decoded into dicts - on the fly. False by default. + on the fly. Only applicable if ``stream`` is True. + False by default. stream (bool): If set to false, only the current stats will be returned instead of a stream. True by default. From f83fe7c9594e72cddf1d89031603c3d246c4c101 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Thu, 8 Nov 2018 17:22:24 -0800 Subject: [PATCH 20/68] Properly convert non-string filters to expected string format Signed-off-by: Joffrey F --- docker/utils/utils.py | 5 ++++- tests/unit/utils_test.py | 4 ++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/docker/utils/utils.py b/docker/utils/utils.py index a8e814d7d5..61e307adc7 100644 --- a/docker/utils/utils.py +++ b/docker/utils/utils.py @@ -386,7 +386,10 @@ def convert_filters(filters): v = 'true' if v else 'false' if not isinstance(v, list): v = [v, ] - result[k] = v + result[k] = [ + str(item) if not isinstance(item, six.string_types) else item + for item in v + ] return json.dumps(result) diff --git a/tests/unit/utils_test.py b/tests/unit/utils_test.py index c862a1cec9..a4e9c9c53e 100644 --- a/tests/unit/utils_test.py +++ b/tests/unit/utils_test.py @@ -457,8 +457,8 @@ def test_convert_filters(self): tests = [ ({'dangling': True}, '{"dangling": ["true"]}'), ({'dangling': "true"}, '{"dangling": ["true"]}'), - ({'exited': 0}, '{"exited": [0]}'), - ({'exited': [0, 1]}, '{"exited": [0, 1]}'), + ({'exited': 0}, '{"exited": ["0"]}'), + ({'exited': [0, 1]}, '{"exited": ["0", "1"]}'), ] for filters, expected in tests: From cebdee4aefd6c17eeb3542f1c53f1c461b95ea61 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Thu, 8 Nov 2018 17:31:22 -0800 Subject: [PATCH 21/68] Add doc example for get_archive Signed-off-by: Joffrey F --- docker/api/container.py | 12 ++++++++++++ docker/models/containers.py | 11 +++++++++++ 2 files changed, 23 insertions(+) diff --git a/docker/api/container.py b/docker/api/container.py index 753e0a57fe..fce73af640 100644 --- a/docker/api/container.py +++ b/docker/api/container.py @@ -694,6 +694,18 @@ def get_archive(self, container, path, chunk_size=DEFAULT_DATA_CHUNK_SIZE): Raises: :py:class:`docker.errors.APIError` If the server returns an error. + + Example: + + >>> c = docker.APIClient() + >>> f = open('./sh_bin.tar', 'wb') + >>> bits, stat = c.get_archive(container, '/bin/sh') + >>> print(stat) + {'name': 'sh', 'size': 1075464, 'mode': 493, + 'mtime': '2018-10-01T15:37:48-07:00', 'linkTarget': ''} + >>> for chunk in bits: + ... f.write(chunk) + >>> f.close() """ params = { 'path': path diff --git a/docker/models/containers.py b/docker/models/containers.py index 493b9fc732..9d6f2cc6af 100644 --- a/docker/models/containers.py +++ b/docker/models/containers.py @@ -233,6 +233,17 @@ def get_archive(self, path, chunk_size=DEFAULT_DATA_CHUNK_SIZE): Raises: :py:class:`docker.errors.APIError` If the server returns an error. + + Example: + + >>> f = open('./sh_bin.tar', 'wb') + >>> bits, stat = container.get_archive('/bin/sh') + >>> print(stat) + {'name': 'sh', 'size': 1075464, 'mode': 493, + 'mtime': '2018-10-01T15:37:48-07:00', 'linkTarget': ''} + >>> for chunk in bits: + ... f.write(chunk) + >>> f.close() """ return self.client.api.get_archive(self.id, path, chunk_size) From 852d79b08d708ff25c1f8bee3b68ffb11ea98dc0 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Thu, 8 Nov 2018 17:32:33 -0800 Subject: [PATCH 22/68] Fix file mode in image.save examples Signed-off-by: Joffrey F --- docker/api/image.py | 2 +- docker/models/images.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docker/api/image.py b/docker/api/image.py index 1c1fcde84c..a9f801e93b 100644 --- a/docker/api/image.py +++ b/docker/api/image.py @@ -32,7 +32,7 @@ def get_image(self, image, chunk_size=DEFAULT_DATA_CHUNK_SIZE): Example: >>> image = cli.get_image("busybox:latest") - >>> f = open('/tmp/busybox-latest.tar', 'w') + >>> f = open('/tmp/busybox-latest.tar', 'wb') >>> for chunk in image: >>> f.write(chunk) >>> f.close() diff --git a/docker/models/images.py b/docker/models/images.py index 28b1fd3ffd..4578c0bd89 100644 --- a/docker/models/images.py +++ b/docker/models/images.py @@ -84,7 +84,7 @@ def save(self, chunk_size=DEFAULT_DATA_CHUNK_SIZE, named=False): Example: >>> image = cli.get_image("busybox:latest") - >>> f = open('/tmp/busybox-latest.tar', 'w') + >>> f = open('/tmp/busybox-latest.tar', 'wb') >>> for chunk in image: >>> f.write(chunk) >>> f.close() From 35b9460748d792670d815496767fab8b396d4300 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Thu, 8 Nov 2018 17:38:59 -0800 Subject: [PATCH 23/68] Remove prematurely committed file Signed-off-by: Joffrey F --- docker/api/mixin.py | 57 --------------------------------------------- 1 file changed, 57 deletions(-) delete mode 100644 docker/api/mixin.py diff --git a/docker/api/mixin.py b/docker/api/mixin.py deleted file mode 100644 index 1353568167..0000000000 --- a/docker/api/mixin.py +++ /dev/null @@ -1,57 +0,0 @@ -from typing import Any, Dict, Iterable, List, Optional, Union - -import requests - -from ..constants import DEFAULT_DOCKER_API_VERSION, DEFAULT_TIMEOUT_SECONDS - -class BaseMixin(object): - base_url: str = '' - credstore_env: Optional[Dict[str, str]] = None - timeout: int = DEFAULT_TIMEOUT_SECONDS - _auth_configs: Dict[str, Dict] - _general_configs: Dict[str, Dict] - _version: str = DEFAULT_DOCKER_API_VERSION - - def _url(self, pathfmt: str, *args, **kwargs) -> str: - raise NotImplemented - - def _post(self, url: str, **kwargs) -> requests.Response: - raise NotImplemented - - def _get(self, url: str, **kwargs) -> requests.Response: - raise NotImplemented - - def _put(self, url: str, **kwargs) -> requests.Response: - raise NotImplemented - - def _delete(self, url: str, **kwargs) -> requests.Response: - raise NotImplemented - - def _post_json(self, url: str, data: Optional[Union[Dict[str, Any], List[Any]]], **kwargs) -> requests.Response: - raise NotImplemented - - def _raise_for_status(self, response: requests.Response) -> None: - raise NotImplemented - - def _result(self, response: requests.Response, json: bool=False, binary: bool=False) -> Any: - raise NotImplemented - - def _stream_helper(self, response: requests.Response, decode: bool = False) -> Iterable: - raise NotImplemented - - def _get_raw_response_socket(self, response: requests.Response) -> Iterable: - raise NotImplemented - - def _read_from_socket( - self, - response: requests.Response, - stream: bool, - tty: bool = False) -> Union[Iterable[bytes], bytes]: - raise NotImplemented - - def _stream_raw_result( - self, - response: requests.Response, - chunk_size: int = 1, - decode: bool = True) -> Iterable[bytes]: - raise NotImplemented From f7a1052b2ba5b5ac308a9ab5c218057ab569a204 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Thu, 8 Nov 2018 18:58:06 -0800 Subject: [PATCH 24/68] Fix versions script to accept versions without -ce suffix Signed-off-by: Joffrey F --- scripts/versions.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/versions.py b/scripts/versions.py index 7212195dec..7ad1d56a61 100644 --- a/scripts/versions.py +++ b/scripts/versions.py @@ -66,7 +66,7 @@ def main(): Version.parse( v.strip('"').lstrip('docker-').rstrip('.tgz').rstrip('-x86_64') ) for v in re.findall( - r'"docker-[0-9]+\.[0-9]+\.[0-9]+-.*tgz"', content + r'"docker-[0-9]+\.[0-9]+\.[0-9]+-?.*tgz"', content ) ] sorted_versions = sorted( From 47c10aa383b87e0cb47904bd912839c13d902924 Mon Sep 17 00:00:00 2001 From: Corentin Henry Date: Wed, 21 Nov 2018 17:12:01 -0800 Subject: [PATCH 25/68] tests: fix failure due to pytest deprecation Signed-off-by: Corentin Henry --- tests/unit/utils_config_test.py | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/tests/unit/utils_config_test.py b/tests/unit/utils_config_test.py index 50ba3831db..b0934f9568 100644 --- a/tests/unit/utils_config_test.py +++ b/tests/unit/utils_config_test.py @@ -4,8 +4,8 @@ import tempfile import json -from py.test import ensuretemp -from pytest import mark +from pytest import mark, fixture + from docker.utils import config try: @@ -15,25 +15,25 @@ class FindConfigFileTest(unittest.TestCase): - def tmpdir(self, name): - tmpdir = ensuretemp(name) - self.addCleanup(tmpdir.remove) - return tmpdir + + @fixture(autouse=True) + def tmpdir(self, tmpdir): + self.mkdir = tmpdir.mkdir def test_find_config_fallback(self): - tmpdir = self.tmpdir('test_find_config_fallback') + tmpdir = self.mkdir('test_find_config_fallback') with mock.patch.dict(os.environ, {'HOME': str(tmpdir)}): assert config.find_config_file() is None def test_find_config_from_explicit_path(self): - tmpdir = self.tmpdir('test_find_config_from_explicit_path') + tmpdir = self.mkdir('test_find_config_from_explicit_path') config_path = tmpdir.ensure('my-config-file.json') assert config.find_config_file(str(config_path)) == str(config_path) def test_find_config_from_environment(self): - tmpdir = self.tmpdir('test_find_config_from_environment') + tmpdir = self.mkdir('test_find_config_from_environment') config_path = tmpdir.ensure('config.json') with mock.patch.dict(os.environ, {'DOCKER_CONFIG': str(tmpdir)}): @@ -41,7 +41,7 @@ def test_find_config_from_environment(self): @mark.skipif("sys.platform == 'win32'") def test_find_config_from_home_posix(self): - tmpdir = self.tmpdir('test_find_config_from_home_posix') + tmpdir = self.mkdir('test_find_config_from_home_posix') config_path = tmpdir.ensure('.docker', 'config.json') with mock.patch.dict(os.environ, {'HOME': str(tmpdir)}): @@ -49,7 +49,7 @@ def test_find_config_from_home_posix(self): @mark.skipif("sys.platform == 'win32'") def test_find_config_from_home_legacy_name(self): - tmpdir = self.tmpdir('test_find_config_from_home_legacy_name') + tmpdir = self.mkdir('test_find_config_from_home_legacy_name') config_path = tmpdir.ensure('.dockercfg') with mock.patch.dict(os.environ, {'HOME': str(tmpdir)}): @@ -57,7 +57,7 @@ def test_find_config_from_home_legacy_name(self): @mark.skipif("sys.platform != 'win32'") def test_find_config_from_home_windows(self): - tmpdir = self.tmpdir('test_find_config_from_home_windows') + tmpdir = self.mkdir('test_find_config_from_home_windows') config_path = tmpdir.ensure('.docker', 'config.json') with mock.patch.dict(os.environ, {'USERPROFILE': str(tmpdir)}): From 493d7f0f3041eb6f7170fa1edce2e8aa2740bd41 Mon Sep 17 00:00:00 2001 From: Corentin Henry Date: Wed, 21 Nov 2018 17:52:35 -0800 Subject: [PATCH 26/68] tests: bump pytest-timeout Signed-off-by: Corentin Henry pytest-timeout 1.2.1 seems to be incompatible with pytest 3.6.3: INTERNALERROR> Traceback (most recent call last): INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/_pytest/main.py", line 185, in wrap_session INTERNALERROR> session.exitstatus = doit(config, session) or 0 INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/_pytest/main.py", line 225, in _main INTERNALERROR> config.hook.pytest_runtestloop(session=session) INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/pluggy/hooks.py", line 284, in __call__ INTERNALERROR> return self._hookexec(self, self.get_hookimpls(), kwargs) INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/pluggy/manager.py", line 67, in _hookexec INTERNALERROR> return self._inner_hookexec(hook, methods, kwargs) INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/pluggy/manager.py", line 61, in INTERNALERROR> firstresult=hook.spec.opts.get("firstresult") if hook.spec else False, INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/pluggy/callers.py", line 208, in _multicall INTERNALERROR> return outcome.get_result() INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/pluggy/callers.py", line 81, in get_result INTERNALERROR> _reraise(*ex) # noqa INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/pluggy/callers.py", line 187, in _multicall INTERNALERROR> res = hook_impl.function(*args) INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/_pytest/main.py", line 246, in pytest_runtestloop INTERNALERROR> item.config.hook.pytest_runtest_protocol(item=item, nextitem=nextitem) INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/pluggy/hooks.py", line 284, in __call__ INTERNALERROR> return self._hookexec(self, self.get_hookimpls(), kwargs) INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/pluggy/manager.py", line 67, in _hookexec INTERNALERROR> return self._inner_hookexec(hook, methods, kwargs) INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/pluggy/manager.py", line 61, in INTERNALERROR> firstresult=hook.spec.opts.get("firstresult") if hook.spec else False, INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/pluggy/callers.py", line 208, in _multicall INTERNALERROR> return outcome.get_result() INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/pluggy/callers.py", line 81, in get_result INTERNALERROR> _reraise(*ex) # noqa INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/pluggy/callers.py", line 182, in _multicall INTERNALERROR> next(gen) # first yield INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/pytest_timeout.py", line 76, in pytest_runtest_protocol INTERNALERROR> timeout_setup(item) INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/pytest_timeout.py", line 104, in timeout_setup INTERNALERROR> timeout, method = get_params(item) INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/pytest_timeout.py", line 162, in get_params INTERNALERROR> timeout, method = _parse_marker(item.keywords['timeout']) INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/pytest_timeout.py", line 178, in _parse_marker INTERNALERROR> if not marker.args and not marker.kwargs: INTERNALERROR> File "/usr/local/lib/python2.7/site-packages/_pytest/mark/structures.py", line 25, in warned INTERNALERROR> warnings.warn(warning, stacklevel=2) INTERNALERROR> RemovedInPytest4Warning: MarkInfo objects are deprecated as they contain merged marks which are hard to deal with correctly. INTERNALERROR> Please use node.get_closest_marker(name) or node.iter_markers(name). INTERNALERROR> Docs: https://docs.pytest.org/en/latest/mark.html#updating-code --- test-requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test-requirements.txt b/test-requirements.txt index 9ad59cc664..07e1a900db 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -4,4 +4,4 @@ mock==1.0.1 pytest==2.9.1; python_version == '3.3' pytest==3.6.3; python_version > '3.3' pytest-cov==2.1.0 -pytest-timeout==1.2.1 +pytest-timeout==1.3.3 From 3fca75ffef3513a0ab8243c3d688b1dff384fd2e Mon Sep 17 00:00:00 2001 From: Frank Sachsenheim Date: Tue, 27 Nov 2018 01:06:02 +0100 Subject: [PATCH 27/68] Fixes return value models.containers.Container.exec_run.__doc__ Signed-off-by: Frank Sachsenheim --- docker/models/containers.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker/models/containers.py b/docker/models/containers.py index 9d6f2cc6af..34996cec61 100644 --- a/docker/models/containers.py +++ b/docker/models/containers.py @@ -172,10 +172,10 @@ def exec_run(self, cmd, stdout=True, stderr=True, stdin=False, tty=False, exit_code: (int): Exit code for the executed command or ``None`` if either ``stream```or ``socket`` is ``True``. - output: (generator or str): + output: (generator or bytes): If ``stream=True``, a generator yielding response chunks. If ``socket=True``, a socket object for the connection. - A string containing response data otherwise. + A bytestring containing response data otherwise. Raises: :py:class:`docker.errors.APIError` From 114630161a08b19489e464ad2c1a70ccccc9cc74 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Mon, 26 Nov 2018 17:33:30 -0800 Subject: [PATCH 28/68] Correctly handle longpath prefix in process_dockerfile when joining paths Signed-off-by: Joffrey F --- docker/api/build.py | 9 ++++- docker/constants.py | 1 + tests/unit/api_build_test.py | 64 +++++++++++++++++++++++++++++++++++- 3 files changed, 72 insertions(+), 2 deletions(-) diff --git a/docker/api/build.py b/docker/api/build.py index 0486dce62d..3a67ff8b28 100644 --- a/docker/api/build.py +++ b/docker/api/build.py @@ -339,7 +339,14 @@ def process_dockerfile(dockerfile, path): abs_dockerfile = dockerfile if not os.path.isabs(dockerfile): abs_dockerfile = os.path.join(path, dockerfile) - + if constants.IS_WINDOWS_PLATFORM and path.startswith( + constants.WINDOWS_LONGPATH_PREFIX): + abs_dockerfile = '{}{}'.format( + constants.WINDOWS_LONGPATH_PREFIX, + os.path.normpath( + abs_dockerfile[len(constants.WINDOWS_LONGPATH_PREFIX):] + ) + ) if (os.path.splitdrive(path)[0] != os.path.splitdrive(abs_dockerfile)[0] or os.path.relpath(abs_dockerfile, path).startswith('..')): # Dockerfile not in context - read data to insert into tar later diff --git a/docker/constants.py b/docker/constants.py index 7565a76889..1ab11ec051 100644 --- a/docker/constants.py +++ b/docker/constants.py @@ -14,6 +14,7 @@ 'is deprecated and non-functional. Please remove it.' IS_WINDOWS_PLATFORM = (sys.platform == 'win32') +WINDOWS_LONGPATH_PREFIX = '\\\\?\\' DEFAULT_USER_AGENT = "docker-sdk-python/{0}".format(version) DEFAULT_NUM_POOLS = 25 diff --git a/tests/unit/api_build_test.py b/tests/unit/api_build_test.py index a7f34fd3f2..59470caa5f 100644 --- a/tests/unit/api_build_test.py +++ b/tests/unit/api_build_test.py @@ -1,12 +1,16 @@ import gzip import io +import shutil import docker from docker import auth +from docker.api.build import process_dockerfile -from .api_test import BaseAPIClientTest, fake_request, url_prefix import pytest +from ..helpers import make_tree +from .api_test import BaseAPIClientTest, fake_request, url_prefix + class BuildTest(BaseAPIClientTest): def test_build_container(self): @@ -161,3 +165,61 @@ def test_set_auth_headers_with_dict_and_no_auth_configs(self): self.client._set_auth_headers(headers) assert headers == expected_headers + + @pytest.mark.skipif( + not docker.constants.IS_WINDOWS_PLATFORM, + reason='Windows-specific syntax') + def test_process_dockerfile_win_longpath_prefix(self): + dirs = [ + 'foo', 'foo/bar', 'baz', + ] + + files = [ + 'Dockerfile', 'foo/Dockerfile.foo', 'foo/bar/Dockerfile.bar', + 'baz/Dockerfile.baz', + ] + + base = make_tree(dirs, files) + self.addCleanup(shutil.rmtree, base) + + def pre(path): + return docker.constants.WINDOWS_LONGPATH_PREFIX + path + + assert process_dockerfile(None, pre(base)) == (None, None) + assert process_dockerfile('Dockerfile', pre(base)) == ( + 'Dockerfile', None + ) + assert process_dockerfile('foo/Dockerfile.foo', pre(base)) == ( + 'foo/Dockerfile.foo', None + ) + assert process_dockerfile( + '../Dockerfile', pre(base + '\\foo') + )[1] is not None + assert process_dockerfile( + '../baz/Dockerfile.baz', pre(base + '/baz') + ) == ('../baz/Dockerfile.baz', None) + + def test_process_dockerfile(self): + dirs = [ + 'foo', 'foo/bar', 'baz', + ] + + files = [ + 'Dockerfile', 'foo/Dockerfile.foo', 'foo/bar/Dockerfile.bar', + 'baz/Dockerfile.baz', + ] + + base = make_tree(dirs, files) + self.addCleanup(shutil.rmtree, base) + + assert process_dockerfile(None, base) == (None, None) + assert process_dockerfile('Dockerfile', base) == ('Dockerfile', None) + assert process_dockerfile('foo/Dockerfile.foo', base) == ( + 'foo/Dockerfile.foo', None + ) + assert process_dockerfile( + '../Dockerfile', base + '/foo' + )[1] is not None + assert process_dockerfile('../baz/Dockerfile.baz', base + '/baz') == ( + '../baz/Dockerfile.baz', None + ) From 5f157bbaca5ae62a5bb71547106beb6ef02bc485 Mon Sep 17 00:00:00 2001 From: Corentin Henry Date: Mon, 8 Oct 2018 11:02:31 -0700 Subject: [PATCH 29/68] implement stream demultiplexing for exec commands fixes https://github.com/docker/docker-py/issues/1952 Signed-off-by: Corentin Henry --- docker/api/client.py | 18 +++-- docker/api/container.py | 13 ++-- docker/api/exec_api.py | 13 ++-- docker/models/containers.py | 70 ++++++++++++++++++- docker/utils/socket.py | 89 +++++++++++++++++++++---- tests/integration/api_container_test.py | 5 +- tests/integration/api_exec_test.py | 5 +- tests/unit/api_test.py | 2 +- tests/unit/models_containers_test.py | 6 +- 9 files changed, 181 insertions(+), 40 deletions(-) diff --git a/docker/api/client.py b/docker/api/client.py index 197846d105..8a5a60b0b2 100644 --- a/docker/api/client.py +++ b/docker/api/client.py @@ -32,7 +32,7 @@ from ..tls import TLSConfig from ..transport import SSLAdapter, UnixAdapter from ..utils import utils, check_resource, update_headers, config -from ..utils.socket import frames_iter, socket_raw_iter +from ..utils.socket import frames_iter, consume_socket_output, demux_adaptor from ..utils.json_stream import json_stream try: from ..transport import NpipeAdapter @@ -381,19 +381,23 @@ def _stream_raw_result(self, response, chunk_size=1, decode=True): for out in response.iter_content(chunk_size, decode): yield out - def _read_from_socket(self, response, stream, tty=False): + def _read_from_socket(self, response, stream, tty=True, demux=False): socket = self._get_raw_response_socket(response) - gen = None - if tty is False: - gen = frames_iter(socket) + gen = frames_iter(socket, tty) + + if demux: + # The generator will output tuples (stdout, stderr) + gen = (demux_adaptor(*frame) for frame in gen) else: - gen = socket_raw_iter(socket) + # The generator will output strings + gen = (data for (_, data) in gen) if stream: return gen else: - return six.binary_type().join(gen) + # Wait for all the frames, concatenate them, and return the result + return consume_socket_output(gen, demux=demux) def _disable_socket_timeout(self, socket): """ Depending on the combination of python version and whether we're diff --git a/docker/api/container.py b/docker/api/container.py index fce73af640..ab3b1cf410 100644 --- a/docker/api/container.py +++ b/docker/api/container.py @@ -13,7 +13,7 @@ class ContainerApiMixin(object): @utils.check_resource('container') def attach(self, container, stdout=True, stderr=True, - stream=False, logs=False): + stream=False, logs=False, demux=False): """ Attach to a container. @@ -28,11 +28,15 @@ def attach(self, container, stdout=True, stderr=True, stream (bool): Return container output progressively as an iterator of strings, rather than a single string. logs (bool): Include the container's previous output. + demux (bool): Keep stdout and stderr separate. Returns: - By default, the container's output as a single string. + By default, the container's output as a single string (two if + ``demux=True``: one for stdout and one for stderr). - If ``stream=True``, an iterator of output strings. + If ``stream=True``, an iterator of output strings. If + ``demux=True``, two iterators are returned: one for stdout and one + for stderr. Raises: :py:class:`docker.errors.APIError` @@ -54,8 +58,7 @@ def attach(self, container, stdout=True, stderr=True, response = self._post(u, headers=headers, params=params, stream=True) output = self._read_from_socket( - response, stream, self._check_is_tty(container) - ) + response, stream, self._check_is_tty(container), demux=demux) if stream: return CancellableStream(output, response) diff --git a/docker/api/exec_api.py b/docker/api/exec_api.py index 986d87f21c..3950991972 100644 --- a/docker/api/exec_api.py +++ b/docker/api/exec_api.py @@ -118,7 +118,7 @@ def exec_resize(self, exec_id, height=None, width=None): @utils.check_resource('exec_id') def exec_start(self, exec_id, detach=False, tty=False, stream=False, - socket=False): + socket=False, demux=False): """ Start a previously set up exec instance. @@ -130,11 +130,14 @@ def exec_start(self, exec_id, detach=False, tty=False, stream=False, stream (bool): Stream response data. Default: False socket (bool): Return the connection socket to allow custom read/write operations. + demux (bool): Separate return stdin, stdout and stderr separately Returns: - (generator or str): If ``stream=True``, a generator yielding - response chunks. If ``socket=True``, a socket object for the - connection. A string containing response data otherwise. + + (generator or str or tuple): If ``stream=True``, a generator + yielding response chunks. If ``socket=True``, a socket object for + the connection. A string containing response data otherwise. If + ``demux=True``, stdin, stdout and stderr are separated. Raises: :py:class:`docker.errors.APIError` @@ -162,4 +165,4 @@ def exec_start(self, exec_id, detach=False, tty=False, stream=False, return self._result(res) if socket: return self._get_raw_response_socket(res) - return self._read_from_socket(res, stream, tty) + return self._read_from_socket(res, stream, tty=tty, demux=demux) diff --git a/docker/models/containers.py b/docker/models/containers.py index 9d6f2cc6af..15cc212531 100644 --- a/docker/models/containers.py +++ b/docker/models/containers.py @@ -144,7 +144,7 @@ def diff(self): def exec_run(self, cmd, stdout=True, stderr=True, stdin=False, tty=False, privileged=False, user='', detach=False, stream=False, - socket=False, environment=None, workdir=None): + socket=False, environment=None, workdir=None, demux=False): """ Run a command inside this container. Similar to ``docker exec``. @@ -166,6 +166,7 @@ def exec_run(self, cmd, stdout=True, stderr=True, stdin=False, tty=False, the following format ``["PASSWORD=xxx"]`` or ``{"PASSWORD": "xxx"}``. workdir (str): Path to working directory for this exec session + demux (bool): Return stdout and stderr separately Returns: (ExecResult): A tuple of (exit_code, output) @@ -180,6 +181,70 @@ def exec_run(self, cmd, stdout=True, stderr=True, stdin=False, tty=False, Raises: :py:class:`docker.errors.APIError` If the server returns an error. + + Example: + + Create a container that runs in the background + + >>> client = docker.from_env() + >>> container = client.containers.run( + ... 'bfirsh/reticulate-splines', detach=True) + + Prepare the command we are going to use. It prints "hello stdout" + in `stdout`, followed by "hello stderr" in `stderr`: + + >>> cmd = '/bin/sh -c "echo hello stdout ; echo hello stderr >&2"' + + We'll run this command with all four the combinations of ``stream`` + and ``demux``. + + With ``stream=False`` and ``demux=False``, the output is a string + that contains both the `stdout` and the `stderr` output: + + >>> res = container.exec_run(cmd, stream=False, demux=False) + >>> res.output + b'hello stderr\nhello stdout\n' + + With ``stream=True``, and ``demux=False``, the output is a + generator that yields strings containing the output of both + `stdout` and `stderr`: + + >>> res = container.exec_run(cmd, stream=True, demux=False) + >>> next(res.output) + b'hello stdout\n' + >>> next(res.output) + b'hello stderr\n' + >>> next(res.output) + Traceback (most recent call last): + File "", line 1, in + StopIteration + + With ``stream=True`` and ``demux=True``, the generator now + separates the streams, and yield tuples + ``(stdout, stderr)``: + + >>> res = container.exec_run(cmd, stream=True, demux=True) + >>> next(res.output) + (b'hello stdout\n', None) + >>> next(res.output) + (None, b'hello stderr\n') + >>> next(res.output) + Traceback (most recent call last): + File "", line 1, in + StopIteration + + Finally, with ``stream=False`` and ``demux=True``, the whole output + is returned, but the streams are still separated: + + >>> res = container.exec_run(cmd, stream=True, demux=True) + >>> next(res.output) + (b'hello stdout\n', None) + >>> next(res.output) + (None, b'hello stderr\n') + >>> next(res.output) + Traceback (most recent call last): + File "", line 1, in + StopIteration """ resp = self.client.api.exec_create( self.id, cmd, stdout=stdout, stderr=stderr, stdin=stdin, tty=tty, @@ -187,7 +252,8 @@ def exec_run(self, cmd, stdout=True, stderr=True, stdin=False, tty=False, workdir=workdir ) exec_output = self.client.api.exec_start( - resp['Id'], detach=detach, tty=tty, stream=stream, socket=socket + resp['Id'], detach=detach, tty=tty, stream=stream, socket=socket, + demux=demux ) if socket or stream: return ExecResult(None, exec_output) diff --git a/docker/utils/socket.py b/docker/utils/socket.py index 7b96d4fce6..fe4a33266c 100644 --- a/docker/utils/socket.py +++ b/docker/utils/socket.py @@ -12,6 +12,10 @@ NpipeSocket = type(None) +STDOUT = 1 +STDERR = 2 + + class SocketError(Exception): pass @@ -51,28 +55,43 @@ def read_exactly(socket, n): return data -def next_frame_size(socket): +def next_frame_header(socket): """ - Returns the size of the next frame of data waiting to be read from socket, - according to the protocol defined here: + Returns the stream and size of the next frame of data waiting to be read + from socket, according to the protocol defined here: - https://docs.docker.com/engine/reference/api/docker_remote_api_v1.24/#/attach-to-a-container + https://docs.docker.com/engine/api/v1.24/#attach-to-a-container """ try: data = read_exactly(socket, 8) except SocketError: - return -1 + return (-1, -1) + + stream, actual = struct.unpack('>BxxxL', data) + return (stream, actual) + - _, actual = struct.unpack('>BxxxL', data) - return actual +def frames_iter(socket, tty): + """ + Return a generator of frames read from socket. A frame is a tuple where + the first item is the stream number and the second item is a chunk of data. + + If the tty setting is enabled, the streams are multiplexed into the stdout + stream. + """ + if tty: + return ((STDOUT, frame) for frame in frames_iter_tty(socket)) + else: + return frames_iter_no_tty(socket) -def frames_iter(socket): +def frames_iter_no_tty(socket): """ - Returns a generator of frames read from socket + Returns a generator of data read from the socket when the tty setting is + not enabled. """ while True: - n = next_frame_size(socket) + (stream, n) = next_frame_header(socket) if n < 0: break while n > 0: @@ -84,13 +103,13 @@ def frames_iter(socket): # We have reached EOF return n -= data_length - yield result + yield (stream, result) -def socket_raw_iter(socket): +def frames_iter_tty(socket): """ - Returns a generator of data read from the socket. - This is used for non-multiplexed streams. + Return a generator of data read from the socket when the tty setting is + enabled. """ while True: result = read(socket) @@ -98,3 +117,45 @@ def socket_raw_iter(socket): # We have reached EOF return yield result + + +def consume_socket_output(frames, demux=False): + """ + Iterate through frames read from the socket and return the result. + + Args: + + demux (bool): + If False, stdout and stderr are multiplexed, and the result is the + concatenation of all the frames. If True, the streams are + demultiplexed, and the result is a 2-tuple where each item is the + concatenation of frames belonging to the same stream. + """ + if demux is False: + # If the streams are multiplexed, the generator returns strings, that + # we just need to concatenate. + return six.binary_type().join(frames) + + # If the streams are demultiplexed, the generator returns tuples + # (stdin, stdout, stderr) + out = [six.binary_type(), six.binary_type()] + for frame in frames: + for stream_id in [STDOUT, STDERR]: + # It is guaranteed that for each frame, one and only one stream + # is not None. + if frame[stream_id] is not None: + out[stream_id] += frame[stream_id] + return tuple(out) + + +def demux_adaptor(stream_id, data): + """ + Utility to demultiplex stdout and stderr when reading frames from the + socket. + """ + if stream_id == STDOUT: + return (data, None) + elif stream_id == STDERR: + return (None, data) + else: + raise ValueError('{0} is not a valid stream'.format(stream_id)) diff --git a/tests/integration/api_container_test.py b/tests/integration/api_container_test.py index 02f3603374..83df3424a9 100644 --- a/tests/integration/api_container_test.py +++ b/tests/integration/api_container_test.py @@ -7,7 +7,7 @@ import docker from docker.constants import IS_WINDOWS_PLATFORM -from docker.utils.socket import next_frame_size +from docker.utils.socket import next_frame_header from docker.utils.socket import read_exactly import pytest @@ -1242,7 +1242,8 @@ def test_run_container_reading_socket(self): self.client.start(container) - next_size = next_frame_size(pty_stdout) + (stream, next_size) = next_frame_header(pty_stdout) + assert stream == 1 # correspond to stdout assert next_size == len(line) data = read_exactly(pty_stdout, next_size) assert data.decode('utf-8') == line diff --git a/tests/integration/api_exec_test.py b/tests/integration/api_exec_test.py index 1a5a4e5472..ac64af7724 100644 --- a/tests/integration/api_exec_test.py +++ b/tests/integration/api_exec_test.py @@ -1,4 +1,4 @@ -from docker.utils.socket import next_frame_size +from docker.utils.socket import next_frame_header from docker.utils.socket import read_exactly from .base import BaseAPIIntegrationTest, BUSYBOX @@ -91,7 +91,8 @@ def test_exec_start_socket(self): socket = self.client.exec_start(exec_id, socket=True) self.addCleanup(socket.close) - next_size = next_frame_size(socket) + (stream, next_size) = next_frame_header(socket) + assert stream == 1 # stdout (0 = stdin, 1 = stdout, 2 = stderr) assert next_size == len(line) data = read_exactly(socket, next_size) assert data.decode('utf-8') == line diff --git a/tests/unit/api_test.py b/tests/unit/api_test.py index af2bb1c202..ccddbb16de 100644 --- a/tests/unit/api_test.py +++ b/tests/unit/api_test.py @@ -83,7 +83,7 @@ def fake_delete(self, url, *args, **kwargs): return fake_request('DELETE', url, *args, **kwargs) -def fake_read_from_socket(self, response, stream, tty=False): +def fake_read_from_socket(self, response, stream, tty=False, demux=False): return six.binary_type() diff --git a/tests/unit/models_containers_test.py b/tests/unit/models_containers_test.py index 22dd241064..24f6316acf 100644 --- a/tests/unit/models_containers_test.py +++ b/tests/unit/models_containers_test.py @@ -417,7 +417,8 @@ def test_exec_run(self): workdir=None ) client.api.exec_start.assert_called_with( - FAKE_EXEC_ID, detach=False, tty=False, stream=True, socket=False + FAKE_EXEC_ID, detach=False, tty=False, stream=True, socket=False, + demux=False, ) def test_exec_run_failure(self): @@ -430,7 +431,8 @@ def test_exec_run_failure(self): workdir=None ) client.api.exec_start.assert_called_with( - FAKE_EXEC_ID, detach=False, tty=False, stream=False, socket=False + FAKE_EXEC_ID, detach=False, tty=False, stream=False, socket=False, + demux=False, ) def test_export(self): From a74d546864b64ba03dce1e3407d3b0cd5badee9f Mon Sep 17 00:00:00 2001 From: adw1n Date: Mon, 3 Sep 2018 05:54:12 +0200 Subject: [PATCH 30/68] Fix pulling images with `stream=True` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Pulling an image with option `stream=True` like this: ``` client.api.pull('docker.io/user/repo_name', tag='latest', stream=True) ``` without consuming the generator oftentimes results in premature drop of the connection. Docker daemon tries to send progress of pulling the image to the client, but it encounters an error (broken pipe) and therefore cancells the pull action: ``` Thread 1 "dockerd-dev" received signal SIGPIPE, Broken pipe. ERRO[2018-09-03T05:12:35.746497638+02:00] Not continuing with pull after error: context canceled ``` As described in issue #2116, even though client receives response with status code 200, image is not pulled. Closes #2116 Signed-off-by: Przemysław Adamek --- docker/api/image.py | 3 ++- docker/models/images.py | 1 + tests/unit/models_containers_test.py | 3 ++- tests/unit/models_images_test.py | 6 ++++-- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/docker/api/image.py b/docker/api/image.py index a9f801e93b..d3fed5c0cc 100644 --- a/docker/api/image.py +++ b/docker/api/image.py @@ -334,7 +334,8 @@ def pull(self, repository, tag=None, stream=False, auth_config=None, Args: repository (str): The repository to pull tag (str): The tag to pull - stream (bool): Stream the output as a generator + stream (bool): Stream the output as a generator. Make sure to + consume the generator, otherwise pull might get cancelled. auth_config (dict): Override the credentials that :py:meth:`~docker.api.daemon.DaemonApiMixin.login` has set for this request. ``auth_config`` should contain the ``username`` diff --git a/docker/models/images.py b/docker/models/images.py index 4578c0bd89..f8b842a943 100644 --- a/docker/models/images.py +++ b/docker/models/images.py @@ -425,6 +425,7 @@ def pull(self, repository, tag=None, **kwargs): if not tag: repository, tag = parse_repository_tag(repository) + kwargs['stream'] = False self.client.api.pull(repository, tag=tag, **kwargs) if tag: return self.get('{0}{2}{1}'.format( diff --git a/tests/unit/models_containers_test.py b/tests/unit/models_containers_test.py index 22dd241064..957035af0a 100644 --- a/tests/unit/models_containers_test.py +++ b/tests/unit/models_containers_test.py @@ -232,7 +232,8 @@ def test_run_pull(self): container = client.containers.run('alpine', 'sleep 300', detach=True) assert container.id == FAKE_CONTAINER_ID - client.api.pull.assert_called_with('alpine', platform=None, tag=None) + client.api.pull.assert_called_with('alpine', platform=None, tag=None, + stream=False) def test_run_with_error(self): client = make_fake_client() diff --git a/tests/unit/models_images_test.py b/tests/unit/models_images_test.py index 67832795fe..ef81a1599d 100644 --- a/tests/unit/models_images_test.py +++ b/tests/unit/models_images_test.py @@ -43,7 +43,8 @@ def test_load(self): def test_pull(self): client = make_fake_client() image = client.images.pull('test_image:latest') - client.api.pull.assert_called_with('test_image', tag='latest') + client.api.pull.assert_called_with('test_image', tag='latest', + stream=False) client.api.inspect_image.assert_called_with('test_image:latest') assert isinstance(image, Image) assert image.id == FAKE_IMAGE_ID @@ -51,7 +52,8 @@ def test_pull(self): def test_pull_multiple(self): client = make_fake_client() images = client.images.pull('test_image') - client.api.pull.assert_called_with('test_image', tag=None) + client.api.pull.assert_called_with('test_image', tag=None, + stream=False) client.api.images.assert_called_with( all=False, name='test_image', filters=None ) From c53423f11851a69fa18783b5b0b801a24d8d2f82 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Wed, 28 Nov 2018 11:24:21 -0800 Subject: [PATCH 31/68] Update DockerClient.images.pull to always stream response Also raise a warning when users attempt to specify the "stream" parameter Signed-off-by: Joffrey F --- docker/models/images.py | 18 ++++++++++++++++-- tests/unit/models_containers_test.py | 5 +++-- tests/unit/models_images_test.py | 24 +++++++++++++++++++----- 3 files changed, 38 insertions(+), 9 deletions(-) diff --git a/docker/models/images.py b/docker/models/images.py index f8b842a943..30e86f109e 100644 --- a/docker/models/images.py +++ b/docker/models/images.py @@ -1,5 +1,6 @@ import itertools import re +import warnings import six @@ -425,8 +426,21 @@ def pull(self, repository, tag=None, **kwargs): if not tag: repository, tag = parse_repository_tag(repository) - kwargs['stream'] = False - self.client.api.pull(repository, tag=tag, **kwargs) + if 'stream' in kwargs: + warnings.warn( + '`stream` is not a valid parameter for this method' + ' and will be overridden' + ) + del kwargs['stream'] + + pull_log = self.client.api.pull( + repository, tag=tag, stream=True, **kwargs + ) + for _ in pull_log: + # We don't do anything with the logs, but we need + # to keep the connection alive and wait for the image + # to be pulled. + pass if tag: return self.get('{0}{2}{1}'.format( repository, tag, '@' if tag.startswith('sha256:') else ':' diff --git a/tests/unit/models_containers_test.py b/tests/unit/models_containers_test.py index 957035af0a..39e409e4bf 100644 --- a/tests/unit/models_containers_test.py +++ b/tests/unit/models_containers_test.py @@ -232,8 +232,9 @@ def test_run_pull(self): container = client.containers.run('alpine', 'sleep 300', detach=True) assert container.id == FAKE_CONTAINER_ID - client.api.pull.assert_called_with('alpine', platform=None, tag=None, - stream=False) + client.api.pull.assert_called_with( + 'alpine', platform=None, tag=None, stream=True + ) def test_run_with_error(self): client = make_fake_client() diff --git a/tests/unit/models_images_test.py b/tests/unit/models_images_test.py index ef81a1599d..fd894ab71d 100644 --- a/tests/unit/models_images_test.py +++ b/tests/unit/models_images_test.py @@ -1,6 +1,8 @@ +import unittest +import warnings + from docker.constants import DEFAULT_DATA_CHUNK_SIZE from docker.models.images import Image -import unittest from .fake_api import FAKE_IMAGE_ID from .fake_api_client import make_fake_client @@ -43,8 +45,9 @@ def test_load(self): def test_pull(self): client = make_fake_client() image = client.images.pull('test_image:latest') - client.api.pull.assert_called_with('test_image', tag='latest', - stream=False) + client.api.pull.assert_called_with( + 'test_image', tag='latest', stream=True + ) client.api.inspect_image.assert_called_with('test_image:latest') assert isinstance(image, Image) assert image.id == FAKE_IMAGE_ID @@ -52,8 +55,9 @@ def test_pull(self): def test_pull_multiple(self): client = make_fake_client() images = client.images.pull('test_image') - client.api.pull.assert_called_with('test_image', tag=None, - stream=False) + client.api.pull.assert_called_with( + 'test_image', tag=None, stream=True + ) client.api.images.assert_called_with( all=False, name='test_image', filters=None ) @@ -63,6 +67,16 @@ def test_pull_multiple(self): assert isinstance(image, Image) assert image.id == FAKE_IMAGE_ID + def test_pull_with_stream_param(self): + client = make_fake_client() + with warnings.catch_warnings(record=True) as w: + client.images.pull('test_image', stream=True) + + assert len(w) == 1 + assert str(w[0].message).startswith( + '`stream` is not a valid parameter' + ) + def test_push(self): client = make_fake_client() client.images.push('foobar', insecure_registry=True) From 6540900dae21571a60fd92037e926cd6599a52eb Mon Sep 17 00:00:00 2001 From: Corentin Henry Date: Tue, 27 Nov 2018 17:01:06 -0800 Subject: [PATCH 32/68] add tests for _read_from_socket Check that the return value against the various combination of parameters this function can take (tty, stream, and demux). This commit also fixes a bug that the tests uncovered a bug in consume_socket_output. Signed-off-by: Corentin Henry --- docker/utils/socket.py | 16 ++++--- tests/unit/api_test.py | 98 +++++++++++++++++++++++++++++++++++------- 2 files changed, 91 insertions(+), 23 deletions(-) diff --git a/docker/utils/socket.py b/docker/utils/socket.py index fe4a33266c..4b32853688 100644 --- a/docker/utils/socket.py +++ b/docker/utils/socket.py @@ -136,15 +136,17 @@ def consume_socket_output(frames, demux=False): # we just need to concatenate. return six.binary_type().join(frames) - # If the streams are demultiplexed, the generator returns tuples - # (stdin, stdout, stderr) + # If the streams are demultiplexed, the generator yields tuples + # (stdout, stderr) out = [six.binary_type(), six.binary_type()] for frame in frames: - for stream_id in [STDOUT, STDERR]: - # It is guaranteed that for each frame, one and only one stream - # is not None. - if frame[stream_id] is not None: - out[stream_id] += frame[stream_id] + # It is guaranteed that for each frame, one and only one stream + # is not None. + assert frame != (None, None) + if frame[0] is not None: + out[0] += frame[0] + else: + out[1] += frame[1] return tuple(out) diff --git a/tests/unit/api_test.py b/tests/unit/api_test.py index ccddbb16de..0f5ad7c49e 100644 --- a/tests/unit/api_test.py +++ b/tests/unit/api_test.py @@ -15,6 +15,7 @@ import requests from requests.packages import urllib3 import six +import struct from . import fake_api @@ -467,24 +468,25 @@ def test_early_stream_response(self): class TCPSocketStreamTest(unittest.TestCase): - text_data = b''' + stdout_data = b''' Now, those children out there, they're jumping through the flames in the hope that the god of the fire will make them fruitful. Really, you can't blame them. After all, what girl would not prefer the child of a god to that of some acne-scarred artisan? ''' + stderr_data = b''' + And what of the true God? To whose glory churches and monasteries have been + built on these islands for generations past? Now shall what of Him? + ''' def setUp(self): - self.server = six.moves.socketserver.ThreadingTCPServer( - ('', 0), self.get_handler_class() - ) + ('', 0), self.get_handler_class()) self.thread = threading.Thread(target=self.server.serve_forever) self.thread.setDaemon(True) self.thread.start() self.address = 'http://{}:{}'.format( - socket.gethostname(), self.server.server_address[1] - ) + socket.gethostname(), self.server.server_address[1]) def tearDown(self): self.server.shutdown() @@ -492,31 +494,95 @@ def tearDown(self): self.thread.join() def get_handler_class(self): - text_data = self.text_data + stdout_data = self.stdout_data + stderr_data = self.stderr_data class Handler(six.moves.BaseHTTPServer.BaseHTTPRequestHandler, object): def do_POST(self): + resp_data = self.get_resp_data() self.send_response(101) self.send_header( - 'Content-Type', 'application/vnd.docker.raw-stream' - ) + 'Content-Type', 'application/vnd.docker.raw-stream') self.send_header('Connection', 'Upgrade') self.send_header('Upgrade', 'tcp') self.end_headers() self.wfile.flush() time.sleep(0.2) - self.wfile.write(text_data) + self.wfile.write(resp_data) self.wfile.flush() + def get_resp_data(self): + path = self.path.split('/')[-1] + if path == 'tty': + return stdout_data + stderr_data + elif path == 'no-tty': + data = b'' + data += self.frame_header(1, stdout_data) + data += stdout_data + data += self.frame_header(2, stderr_data) + data += stderr_data + return data + else: + raise Exception('Unknown path {0}'.format(path)) + + @staticmethod + def frame_header(stream, data): + return struct.pack('>BxxxL', stream, len(data)) + return Handler - def test_read_from_socket(self): + def request(self, stream=None, tty=None, demux=None): + assert stream is not None and tty is not None and demux is not None with APIClient(base_url=self.address) as client: - resp = client._post(client._url('/dummy'), stream=True) - data = client._read_from_socket(resp, stream=True, tty=True) - results = b''.join(data) - - assert results == self.text_data + if tty: + url = client._url('/tty') + else: + url = client._url('/no-tty') + resp = client._post(url, stream=True) + return client._read_from_socket( + resp, stream=stream, tty=tty, demux=demux) + + def test_read_from_socket_1(self): + res = self.request(stream=True, tty=True, demux=False) + assert next(res) == self.stdout_data + self.stderr_data + with self.assertRaises(StopIteration): + next(res) + + def test_read_from_socket_2(self): + res = self.request(stream=True, tty=True, demux=True) + assert next(res) == (self.stdout_data + self.stderr_data, None) + with self.assertRaises(StopIteration): + next(res) + + def test_read_from_socket_3(self): + res = self.request(stream=True, tty=False, demux=False) + assert next(res) == self.stdout_data + assert next(res) == self.stderr_data + with self.assertRaises(StopIteration): + next(res) + + def test_read_from_socket_4(self): + res = self.request(stream=True, tty=False, demux=True) + assert (self.stdout_data, None) == next(res) + assert (None, self.stderr_data) == next(res) + with self.assertRaises(StopIteration): + next(res) + + def test_read_from_socket_5(self): + res = self.request(stream=False, tty=True, demux=False) + assert res == self.stdout_data + self.stderr_data + + def test_read_from_socket_6(self): + res = self.request(stream=False, tty=True, demux=True) + assert res == (self.stdout_data + self.stderr_data, b'') + + def test_read_from_socket_7(self): + res = self.request(stream=False, tty=False, demux=False) + res == self.stdout_data + self.stderr_data + + def test_read_from_socket_8(self): + res = self.request(stream=False, tty=False, demux=True) + assert res == (self.stdout_data, self.stderr_data) class UserAgentTest(unittest.TestCase): From 76447d0ca330e32811c95fe7fecdebc8cd5c71c2 Mon Sep 17 00:00:00 2001 From: Corentin Henry Date: Wed, 28 Nov 2018 13:36:28 -0800 Subject: [PATCH 33/68] tests various exec_create/exec_start combinations Test the interation of the tty, demux and stream parameters Signed-off-by: Corentin Henry --- tests/integration/api_exec_test.py | 69 ++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/tests/integration/api_exec_test.py b/tests/integration/api_exec_test.py index ac64af7724..fd2f0eae31 100644 --- a/tests/integration/api_exec_test.py +++ b/tests/integration/api_exec_test.py @@ -75,6 +75,75 @@ def test_exec_command_streaming(self): res += chunk assert res == b'hello\nworld\n' + def test_exec_command_demux(self): + container = self.client.create_container( + BUSYBOX, 'cat', detach=True, stdin_open=True) + id = container['Id'] + self.client.start(id) + self.tmp_containers.append(id) + + script = ' ; '.join([ + # Write something on stdout + 'echo hello out', + # Busybox's sleep does not handle sub-second times. + # This loops takes ~0.3 second to execute on my machine. + 'for i in $(seq 1 50000); do echo $i>/dev/null; done', + # Write something on stderr + 'echo hello err >&2']) + cmd = 'sh -c "{}"'.format(script) + + # tty=False, stream=False, demux=False + res = self.client.exec_create(id, cmd) + exec_log = self.client.exec_start(res) + assert exec_log == b'hello out\nhello err\n' + + # tty=False, stream=True, demux=False + res = self.client.exec_create(id, cmd) + exec_log = self.client.exec_start(res, stream=True) + assert next(exec_log) == b'hello out\n' + assert next(exec_log) == b'hello err\n' + with self.assertRaises(StopIteration): + next(exec_log) + + # tty=False, stream=False, demux=True + res = self.client.exec_create(id, cmd) + exec_log = self.client.exec_start(res, demux=True) + assert exec_log == (b'hello out\n', b'hello err\n') + + # tty=False, stream=True, demux=True + res = self.client.exec_create(id, cmd) + exec_log = self.client.exec_start(res, demux=True, stream=True) + assert next(exec_log) == (b'hello out\n', None) + assert next(exec_log) == (None, b'hello err\n') + with self.assertRaises(StopIteration): + next(exec_log) + + # tty=True, stream=False, demux=False + res = self.client.exec_create(id, cmd, tty=True) + exec_log = self.client.exec_start(res) + assert exec_log == b'hello out\r\nhello err\r\n' + + # tty=True, stream=True, demux=False + res = self.client.exec_create(id, cmd, tty=True) + exec_log = self.client.exec_start(res, stream=True) + assert next(exec_log) == b'hello out\r\n' + assert next(exec_log) == b'hello err\r\n' + with self.assertRaises(StopIteration): + next(exec_log) + + # tty=True, stream=False, demux=True + res = self.client.exec_create(id, cmd, tty=True) + exec_log = self.client.exec_start(res, demux=True) + assert exec_log == (b'hello out\r\nhello err\r\n', b'') + + # tty=True, stream=True, demux=True + res = self.client.exec_create(id, cmd, tty=True) + exec_log = self.client.exec_start(res, demux=True, stream=True) + assert next(exec_log) == (b'hello out\r\n', None) + assert next(exec_log) == (b'hello err\r\n', None) + with self.assertRaises(StopIteration): + next(exec_log) + def test_exec_start_socket(self): container = self.client.create_container(BUSYBOX, 'cat', detach=True, stdin_open=True) From 9a67e2032e68eea397f3bf6b727464ed39a02123 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Wed, 28 Nov 2018 14:31:28 -0800 Subject: [PATCH 34/68] Next dev version Signed-off-by: Joffrey F --- docker/version.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/version.py b/docker/version.py index 0b27a263a6..b4cf22bbb2 100644 --- a/docker/version.py +++ b/docker/version.py @@ -1,2 +1,2 @@ -version = "3.6.0" +version = "3.7.0-dev" version_info = tuple([int(d) for d in version.split("-")[0].split(".")]) From 41c0eb7e804ef428da27509714df34d26a56b9a6 Mon Sep 17 00:00:00 2001 From: Corentin Henry Date: Wed, 28 Nov 2018 13:59:19 -0800 Subject: [PATCH 35/68] fix exec_start() documentation Signed-off-by: Corentin Henry --- docker/api/exec_api.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker/api/exec_api.py b/docker/api/exec_api.py index 3950991972..d13b128998 100644 --- a/docker/api/exec_api.py +++ b/docker/api/exec_api.py @@ -130,14 +130,14 @@ def exec_start(self, exec_id, detach=False, tty=False, stream=False, stream (bool): Stream response data. Default: False socket (bool): Return the connection socket to allow custom read/write operations. - demux (bool): Separate return stdin, stdout and stderr separately + demux (bool): Return stdout and stderr separately Returns: (generator or str or tuple): If ``stream=True``, a generator yielding response chunks. If ``socket=True``, a socket object for the connection. A string containing response data otherwise. If - ``demux=True``, stdin, stdout and stderr are separated. + ``demux=True``, stdout and stderr are separated. Raises: :py:class:`docker.errors.APIError` From 7b3b83dfdbb9f4270dcf54e1449645efc045dfd3 Mon Sep 17 00:00:00 2001 From: Corentin Henry Date: Wed, 28 Nov 2018 14:32:12 -0800 Subject: [PATCH 36/68] fix exec api inconsistency Signed-off-by: Corentin Henry --- docker/utils/socket.py | 12 +++++++++--- tests/integration/api_exec_test.py | 2 +- tests/unit/api_test.py | 2 +- 3 files changed, 11 insertions(+), 5 deletions(-) diff --git a/docker/utils/socket.py b/docker/utils/socket.py index 4b32853688..7ba9505538 100644 --- a/docker/utils/socket.py +++ b/docker/utils/socket.py @@ -138,15 +138,21 @@ def consume_socket_output(frames, demux=False): # If the streams are demultiplexed, the generator yields tuples # (stdout, stderr) - out = [six.binary_type(), six.binary_type()] + out = [None, None] for frame in frames: # It is guaranteed that for each frame, one and only one stream # is not None. assert frame != (None, None) if frame[0] is not None: - out[0] += frame[0] + if out[0] is None: + out[0] = frame[0] + else: + out[0] += frame[0] else: - out[1] += frame[1] + if out[1] is None: + out[1] = frame[1] + else: + out[1] += frame[1] return tuple(out) diff --git a/tests/integration/api_exec_test.py b/tests/integration/api_exec_test.py index fd2f0eae31..857a18cb3f 100644 --- a/tests/integration/api_exec_test.py +++ b/tests/integration/api_exec_test.py @@ -134,7 +134,7 @@ def test_exec_command_demux(self): # tty=True, stream=False, demux=True res = self.client.exec_create(id, cmd, tty=True) exec_log = self.client.exec_start(res, demux=True) - assert exec_log == (b'hello out\r\nhello err\r\n', b'') + assert exec_log == (b'hello out\r\nhello err\r\n', None) # tty=True, stream=True, demux=True res = self.client.exec_create(id, cmd, tty=True) diff --git a/tests/unit/api_test.py b/tests/unit/api_test.py index 0f5ad7c49e..fac314d3d2 100644 --- a/tests/unit/api_test.py +++ b/tests/unit/api_test.py @@ -574,7 +574,7 @@ def test_read_from_socket_5(self): def test_read_from_socket_6(self): res = self.request(stream=False, tty=True, demux=True) - assert res == (self.stdout_data + self.stderr_data, b'') + assert res == (self.stdout_data + self.stderr_data, None) def test_read_from_socket_7(self): res = self.request(stream=False, tty=False, demux=False) From bc5d7c8cb676d54ad00b8cd8e731a9db049c392c Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Wed, 28 Nov 2018 19:32:01 -0800 Subject: [PATCH 37/68] Modernize auth management Signed-off-by: Joffrey F --- docker/api/build.py | 37 +--- docker/api/daemon.py | 16 +- docker/auth.py | 388 +++++++++++++++++++++-------------- requirements.txt | 2 +- setup.py | 2 +- tests/unit/api_build_test.py | 18 +- tests/unit/api_test.py | 12 +- tests/unit/auth_test.py | 56 ++--- 8 files changed, 294 insertions(+), 237 deletions(-) diff --git a/docker/api/build.py b/docker/api/build.py index 3a67ff8b28..1723083bc7 100644 --- a/docker/api/build.py +++ b/docker/api/build.py @@ -293,31 +293,11 @@ def _set_auth_headers(self, headers): # Send the full auth configuration (if any exists), since the build # could use any (or all) of the registries. if self._auth_configs: - auth_cfgs = self._auth_configs - auth_data = {} - if auth_cfgs.get('credsStore'): - # Using a credentials store, we need to retrieve the - # credentials for each registry listed in the config.json file - # Matches CLI behavior: https://github.com/docker/docker/blob/ - # 67b85f9d26f1b0b2b240f2d794748fac0f45243c/cliconfig/ - # credentials/native_store.go#L68-L83 - for registry in auth_cfgs.get('auths', {}).keys(): - auth_data[registry] = auth.resolve_authconfig( - auth_cfgs, registry, - credstore_env=self.credstore_env, - ) - else: - for registry in auth_cfgs.get('credHelpers', {}).keys(): - auth_data[registry] = auth.resolve_authconfig( - auth_cfgs, registry, - credstore_env=self.credstore_env - ) - for registry, creds in auth_cfgs.get('auths', {}).items(): - if registry not in auth_data: - auth_data[registry] = creds - # See https://github.com/docker/docker-py/issues/1683 - if auth.INDEX_NAME in auth_data: - auth_data[auth.INDEX_URL] = auth_data[auth.INDEX_NAME] + auth_data = self._auth_configs.get_all_credentials() + + # See https://github.com/docker/docker-py/issues/1683 + if auth.INDEX_URL not in auth_data and auth.INDEX_URL in auth_data: + auth_data[auth.INDEX_URL] = auth_data.get(auth.INDEX_NAME, {}) log.debug( 'Sending auth config ({0})'.format( @@ -325,9 +305,10 @@ def _set_auth_headers(self, headers): ) ) - headers['X-Registry-Config'] = auth.encode_header( - auth_data - ) + if auth_data: + headers['X-Registry-Config'] = auth.encode_header( + auth_data + ) else: log.debug('No auth config found') diff --git a/docker/api/daemon.py b/docker/api/daemon.py index 431e7d41cd..a2936f2a0e 100644 --- a/docker/api/daemon.py +++ b/docker/api/daemon.py @@ -124,13 +124,15 @@ def login(self, username, password=None, email=None, registry=None, # If dockercfg_path is passed check to see if the config file exists, # if so load that config. if dockercfg_path and os.path.exists(dockercfg_path): - self._auth_configs = auth.load_config(dockercfg_path) + self._auth_configs = auth.load_config( + dockercfg_path, credstore_env=self.credstore_env + ) elif not self._auth_configs: - self._auth_configs = auth.load_config() + self._auth_configs = auth.load_config( + credstore_env=self.credstore_env + ) - authcfg = auth.resolve_authconfig( - self._auth_configs, registry, credstore_env=self.credstore_env, - ) + authcfg = self._auth_configs.resolve_authconfig(registry) # If we found an existing auth config for this registry and username # combination, we can return it immediately unless reauth is requested. if authcfg and authcfg.get('username', None) == username \ @@ -146,9 +148,7 @@ def login(self, username, password=None, email=None, registry=None, response = self._post_json(self._url('/auth'), data=req_data) if response.status_code == 200: - if 'auths' not in self._auth_configs: - self._auth_configs['auths'] = {} - self._auth_configs['auths'][registry or auth.INDEX_NAME] = req_data + self._auth_configs.add_auth(registry or auth.INDEX_NAME, req_data) return self._result(response, json=True) def ping(self): diff --git a/docker/auth.py b/docker/auth.py index 17158f4ae3..a6c8ae16a3 100644 --- a/docker/auth.py +++ b/docker/auth.py @@ -70,81 +70,246 @@ def split_repo_name(repo_name): def get_credential_store(authconfig, registry): - if not registry or registry == INDEX_NAME: - registry = 'https://index.docker.io/v1/' + return authconfig.get_credential_store(registry) + + +class AuthConfig(object): + def __init__(self, dct, credstore_env=None): + if 'auths' not in dct: + dct['auths'] = {} + self._dct = dct + self._credstore_env = credstore_env + self._stores = {} + + @classmethod + def parse_auth(cls, entries, raise_on_error=False): + """ + Parses authentication entries + + Args: + entries: Dict of authentication entries. + raise_on_error: If set to true, an invalid format will raise + InvalidConfigFile + + Returns: + Authentication registry. + """ + + conf = {} + for registry, entry in six.iteritems(entries): + if not isinstance(entry, dict): + log.debug( + 'Config entry for key {0} is not auth config'.format( + registry + ) + ) + # We sometimes fall back to parsing the whole config as if it + # was the auth config by itself, for legacy purposes. In that + # case, we fail silently and return an empty conf if any of the + # keys is not formatted properly. + if raise_on_error: + raise errors.InvalidConfigFile( + 'Invalid configuration for registry {0}'.format( + registry + ) + ) + return {} + if 'identitytoken' in entry: + log.debug( + 'Found an IdentityToken entry for registry {0}'.format( + registry + ) + ) + conf[registry] = { + 'IdentityToken': entry['identitytoken'] + } + continue # Other values are irrelevant if we have a token + + if 'auth' not in entry: + # Starting with engine v1.11 (API 1.23), an empty dictionary is + # a valid value in the auths config. + # https://github.com/docker/compose/issues/3265 + log.debug( + 'Auth data for {0} is absent. Client might be using a ' + 'credentials store instead.'.format(registry) + ) + conf[registry] = {} + continue - return authconfig.get('credHelpers', {}).get(registry) or authconfig.get( - 'credsStore' - ) + username, password = decode_auth(entry['auth']) + log.debug( + 'Found entry (registry={0}, username={1})' + .format(repr(registry), repr(username)) + ) + conf[registry] = { + 'username': username, + 'password': password, + 'email': entry.get('email'), + 'serveraddress': registry, + } + return conf + + @classmethod + def load_config(cls, config_path, config_dict, credstore_env=None): + """ + Loads authentication data from a Docker configuration file in the given + root directory or if config_path is passed use given path. + Lookup priority: + explicit config_path parameter > DOCKER_CONFIG environment + variable > ~/.docker/config.json > ~/.dockercfg + """ + + if not config_dict: + config_file = config.find_config_file(config_path) + + if not config_file: + return cls({}, credstore_env) + try: + with open(config_file) as f: + config_dict = json.load(f) + except (IOError, KeyError, ValueError) as e: + # Likely missing new Docker config file or it's in an + # unknown format, continue to attempt to read old location + # and format. + log.debug(e) + return cls(_load_legacy_config(config_file), credstore_env) + + res = {} + if config_dict.get('auths'): + log.debug("Found 'auths' section") + res.update({ + 'auths': cls.parse_auth( + config_dict.pop('auths'), raise_on_error=True + ) + }) + if config_dict.get('credsStore'): + log.debug("Found 'credsStore' section") + res.update({'credsStore': config_dict.pop('credsStore')}) + if config_dict.get('credHelpers'): + log.debug("Found 'credHelpers' section") + res.update({'credHelpers': config_dict.pop('credHelpers')}) + if res: + return cls(res, credstore_env) -def resolve_authconfig(authconfig, registry=None, credstore_env=None): - """ - Returns the authentication data from the given auth configuration for a - specific registry. As with the Docker client, legacy entries in the config - with full URLs are stripped down to hostnames before checking for a match. - Returns None if no match was found. - """ + log.debug( + "Couldn't find auth-related section ; attempting to interpret " + "as auth-only file" + ) + return cls({'auths': cls.parse_auth(config_dict)}, credstore_env) + + @property + def auths(self): + return self._dct.get('auths', {}) + + @property + def creds_store(self): + return self._dct.get('credsStore', None) + + @property + def cred_helpers(self): + return self._dct.get('credHelpers', {}) + + def resolve_authconfig(self, registry=None): + """ + Returns the authentication data from the given auth configuration for a + specific registry. As with the Docker client, legacy entries in the + config with full URLs are stripped down to hostnames before checking + for a match. Returns None if no match was found. + """ + + if self.creds_store or self.cred_helpers: + store_name = self.get_credential_store(registry) + if store_name is not None: + log.debug( + 'Using credentials store "{0}"'.format(store_name) + ) + cfg = self._resolve_authconfig_credstore(registry, store_name) + if cfg is not None: + return cfg + log.debug('No entry in credstore - fetching from auth dict') - if 'credHelpers' in authconfig or 'credsStore' in authconfig: - store_name = get_credential_store(authconfig, registry) - if store_name is not None: - log.debug( - 'Using credentials store "{0}"'.format(store_name) - ) - cfg = _resolve_authconfig_credstore( - authconfig, registry, store_name, env=credstore_env + # Default to the public index server + registry = resolve_index_name(registry) if registry else INDEX_NAME + log.debug("Looking for auth entry for {0}".format(repr(registry))) + + if registry in self.auths: + log.debug("Found {0}".format(repr(registry))) + return self.auths[registry] + + for key, conf in six.iteritems(self.auths): + if resolve_index_name(key) == registry: + log.debug("Found {0}".format(repr(key))) + return conf + + log.debug("No entry found") + return None + + def _resolve_authconfig_credstore(self, registry, credstore_name): + if not registry or registry == INDEX_NAME: + # The ecosystem is a little schizophrenic with index.docker.io VS + # docker.io - in that case, it seems the full URL is necessary. + registry = INDEX_URL + log.debug("Looking for auth entry for {0}".format(repr(registry))) + store = self._get_store_instance(credstore_name) + try: + data = store.get(registry) + res = { + 'ServerAddress': registry, + } + if data['Username'] == TOKEN_USERNAME: + res['IdentityToken'] = data['Secret'] + else: + res.update({ + 'Username': data['Username'], + 'Password': data['Secret'], + }) + return res + except dockerpycreds.CredentialsNotFound as e: + log.debug('No entry found') + return None + except dockerpycreds.StoreError as e: + raise errors.DockerException( + 'Credentials store error: {0}'.format(repr(e)) ) - if cfg is not None: - return cfg - log.debug('No entry in credstore - fetching from auth dict') - # Default to the public index server - registry = resolve_index_name(registry) if registry else INDEX_NAME - log.debug("Looking for auth entry for {0}".format(repr(registry))) + def _get_store_instance(self, name): + if name not in self._stores: + self._stores[name] = dockerpycreds.Store( + name, environment=self._credstore_env + ) + return self._stores[name] + + def get_credential_store(self, registry): + if not registry or registry == INDEX_NAME: + registry = 'https://index.docker.io/v1/' + + return self.cred_helpers.get(registry) or self.creds_store + + def get_all_credentials(self): + auth_data = self.auths.copy() + if self.creds_store: + # Retrieve all credentials from the default store + store = self._get_store_instance(self.creds_store) + for k in store.list().keys(): + auth_data[k] = self._resolve_authconfig_credstore( + k, self.creds_store + ) - authdict = authconfig.get('auths', {}) - if registry in authdict: - log.debug("Found {0}".format(repr(registry))) - return authdict[registry] + # credHelpers entries take priority over all others + for reg, store_name in self.cred_helpers.items(): + auth_data[reg] = self._resolve_authconfig_credstore( + reg, store_name + ) - for key, conf in six.iteritems(authdict): - if resolve_index_name(key) == registry: - log.debug("Found {0}".format(repr(key))) - return conf + return auth_data - log.debug("No entry found") - return None + def add_auth(self, reg, data): + self._dct['auths'][reg] = data -def _resolve_authconfig_credstore(authconfig, registry, credstore_name, - env=None): - if not registry or registry == INDEX_NAME: - # The ecosystem is a little schizophrenic with index.docker.io VS - # docker.io - in that case, it seems the full URL is necessary. - registry = INDEX_URL - log.debug("Looking for auth entry for {0}".format(repr(registry))) - store = dockerpycreds.Store(credstore_name, environment=env) - try: - data = store.get(registry) - res = { - 'ServerAddress': registry, - } - if data['Username'] == TOKEN_USERNAME: - res['IdentityToken'] = data['Secret'] - else: - res.update({ - 'Username': data['Username'], - 'Password': data['Secret'], - }) - return res - except dockerpycreds.CredentialsNotFound as e: - log.debug('No entry found') - return None - except dockerpycreds.StoreError as e: - raise errors.DockerException( - 'Credentials store error: {0}'.format(repr(e)) - ) +def resolve_authconfig(authconfig, registry=None, credstore_env=None): + return authconfig.resolve_authconfig(registry) def convert_to_hostname(url): @@ -177,100 +342,11 @@ def parse_auth(entries, raise_on_error=False): Authentication registry. """ - conf = {} - for registry, entry in six.iteritems(entries): - if not isinstance(entry, dict): - log.debug( - 'Config entry for key {0} is not auth config'.format(registry) - ) - # We sometimes fall back to parsing the whole config as if it was - # the auth config by itself, for legacy purposes. In that case, we - # fail silently and return an empty conf if any of the keys is not - # formatted properly. - if raise_on_error: - raise errors.InvalidConfigFile( - 'Invalid configuration for registry {0}'.format(registry) - ) - return {} - if 'identitytoken' in entry: - log.debug('Found an IdentityToken entry for registry {0}'.format( - registry - )) - conf[registry] = { - 'IdentityToken': entry['identitytoken'] - } - continue # Other values are irrelevant if we have a token, skip. - - if 'auth' not in entry: - # Starting with engine v1.11 (API 1.23), an empty dictionary is - # a valid value in the auths config. - # https://github.com/docker/compose/issues/3265 - log.debug( - 'Auth data for {0} is absent. Client might be using a ' - 'credentials store instead.'.format(registry) - ) - conf[registry] = {} - continue - - username, password = decode_auth(entry['auth']) - log.debug( - 'Found entry (registry={0}, username={1})' - .format(repr(registry), repr(username)) - ) - - conf[registry] = { - 'username': username, - 'password': password, - 'email': entry.get('email'), - 'serveraddress': registry, - } - return conf - + return AuthConfig.parse_auth(entries, raise_on_error) -def load_config(config_path=None, config_dict=None): - """ - Loads authentication data from a Docker configuration file in the given - root directory or if config_path is passed use given path. - Lookup priority: - explicit config_path parameter > DOCKER_CONFIG environment variable > - ~/.docker/config.json > ~/.dockercfg - """ - if not config_dict: - config_file = config.find_config_file(config_path) - - if not config_file: - return {} - try: - with open(config_file) as f: - config_dict = json.load(f) - except (IOError, KeyError, ValueError) as e: - # Likely missing new Docker config file or it's in an - # unknown format, continue to attempt to read old location - # and format. - log.debug(e) - return _load_legacy_config(config_file) - - res = {} - if config_dict.get('auths'): - log.debug("Found 'auths' section") - res.update({ - 'auths': parse_auth(config_dict.pop('auths'), raise_on_error=True) - }) - if config_dict.get('credsStore'): - log.debug("Found 'credsStore' section") - res.update({'credsStore': config_dict.pop('credsStore')}) - if config_dict.get('credHelpers'): - log.debug("Found 'credHelpers' section") - res.update({'credHelpers': config_dict.pop('credHelpers')}) - if res: - return res - - log.debug( - "Couldn't find auth-related section ; attempting to interpret " - "as auth-only file" - ) - return {'auths': parse_auth(config_dict)} +def load_config(config_path=None, config_dict=None, credstore_env=None): + return AuthConfig.load_config(config_path, config_dict, credstore_env) def _load_legacy_config(config_file): diff --git a/requirements.txt b/requirements.txt index d13e9d6cad..f1c9bdbc76 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,7 +4,7 @@ backports.ssl-match-hostname==3.5.0.1 cffi==1.10.0 cryptography==1.9; python_version == '3.3' cryptography==2.3; python_version > '3.3' -docker-pycreds==0.3.0 +docker-pycreds==0.4.0 enum34==1.1.6 idna==2.5 ipaddress==1.0.18 diff --git a/setup.py b/setup.py index 3ad572b3ec..4ce55fe815 100644 --- a/setup.py +++ b/setup.py @@ -12,7 +12,7 @@ requirements = [ 'six >= 1.4.0', 'websocket-client >= 0.32.0', - 'docker-pycreds >= 0.3.0', + 'docker-pycreds >= 0.4.0', 'requests >= 2.14.2, != 2.18.0', ] diff --git a/tests/unit/api_build_test.py b/tests/unit/api_build_test.py index 59470caa5f..7e07a2695e 100644 --- a/tests/unit/api_build_test.py +++ b/tests/unit/api_build_test.py @@ -65,7 +65,7 @@ def test_build_container_custom_context_gzip(self): ) def test_build_remote_with_registry_auth(self): - self.client._auth_configs = { + self.client._auth_configs = auth.AuthConfig({ 'auths': { 'https://example.com': { 'user': 'example', @@ -73,7 +73,7 @@ def test_build_remote_with_registry_auth(self): 'email': 'example@example.com' } } - } + }) expected_params = {'t': None, 'q': False, 'dockerfile': None, 'rm': False, 'nocache': False, 'pull': False, @@ -81,7 +81,7 @@ def test_build_remote_with_registry_auth(self): 'remote': 'https://github.com/docker-library/mongo'} expected_headers = { 'X-Registry-Config': auth.encode_header( - self.client._auth_configs['auths'] + self.client._auth_configs.auths ) } @@ -115,7 +115,7 @@ def test_build_container_invalid_container_limits(self): }) def test_set_auth_headers_with_empty_dict_and_auth_configs(self): - self.client._auth_configs = { + self.client._auth_configs = auth.AuthConfig({ 'auths': { 'https://example.com': { 'user': 'example', @@ -123,12 +123,12 @@ def test_set_auth_headers_with_empty_dict_and_auth_configs(self): 'email': 'example@example.com' } } - } + }) headers = {} expected_headers = { 'X-Registry-Config': auth.encode_header( - self.client._auth_configs['auths'] + self.client._auth_configs.auths ) } @@ -136,7 +136,7 @@ def test_set_auth_headers_with_empty_dict_and_auth_configs(self): assert headers == expected_headers def test_set_auth_headers_with_dict_and_auth_configs(self): - self.client._auth_configs = { + self.client._auth_configs = auth.AuthConfig({ 'auths': { 'https://example.com': { 'user': 'example', @@ -144,12 +144,12 @@ def test_set_auth_headers_with_dict_and_auth_configs(self): 'email': 'example@example.com' } } - } + }) headers = {'foo': 'bar'} expected_headers = { 'X-Registry-Config': auth.encode_header( - self.client._auth_configs['auths'] + self.client._auth_configs.auths ), 'foo': 'bar' } diff --git a/tests/unit/api_test.py b/tests/unit/api_test.py index af2bb1c202..14399fe285 100644 --- a/tests/unit/api_test.py +++ b/tests/unit/api_test.py @@ -221,13 +221,11 @@ def test_login(self): 'username': 'sakuya', 'password': 'izayoi' } assert args[1]['headers'] == {'Content-Type': 'application/json'} - assert self.client._auth_configs['auths'] == { - 'docker.io': { - 'email': None, - 'password': 'izayoi', - 'username': 'sakuya', - 'serveraddress': None, - } + assert self.client._auth_configs.auths['docker.io'] == { + 'email': None, + 'password': 'izayoi', + 'username': 'sakuya', + 'serveraddress': None, } def test_events(self): diff --git a/tests/unit/auth_test.py b/tests/unit/auth_test.py index 947d680018..4ad74d5d68 100644 --- a/tests/unit/auth_test.py +++ b/tests/unit/auth_test.py @@ -106,13 +106,13 @@ class ResolveAuthTest(unittest.TestCase): private_config = {'auth': encode_auth({'username': 'privateuser'})} legacy_config = {'auth': encode_auth({'username': 'legacyauth'})} - auth_config = { + auth_config = auth.AuthConfig({ 'auths': auth.parse_auth({ 'https://index.docker.io/v1/': index_config, 'my.registry.net': private_config, 'http://legacy.registry.url/v1/': legacy_config, }) - } + }) def test_resolve_authconfig_hostname_only(self): assert auth.resolve_authconfig( @@ -211,13 +211,15 @@ def test_resolve_registry_and_auth_unauthenticated_registry(self): ) is None def test_resolve_auth_with_empty_credstore_and_auth_dict(self): - auth_config = { + auth_config = auth.AuthConfig({ 'auths': auth.parse_auth({ 'https://index.docker.io/v1/': self.index_config, }), 'credsStore': 'blackbox' - } - with mock.patch('docker.auth._resolve_authconfig_credstore') as m: + }) + with mock.patch( + 'docker.auth.AuthConfig._resolve_authconfig_credstore' + ) as m: m.return_value = None assert 'indexuser' == auth.resolve_authconfig( auth_config, None @@ -226,13 +228,13 @@ def test_resolve_auth_with_empty_credstore_and_auth_dict(self): class CredStoreTest(unittest.TestCase): def test_get_credential_store(self): - auth_config = { + auth_config = auth.AuthConfig({ 'credHelpers': { 'registry1.io': 'truesecret', 'registry2.io': 'powerlock' }, 'credsStore': 'blackbox', - } + }) assert auth.get_credential_store( auth_config, 'registry1.io' @@ -245,12 +247,12 @@ def test_get_credential_store(self): ) == 'blackbox' def test_get_credential_store_no_default(self): - auth_config = { + auth_config = auth.AuthConfig({ 'credHelpers': { 'registry1.io': 'truesecret', 'registry2.io': 'powerlock' }, - } + }) assert auth.get_credential_store( auth_config, 'registry2.io' ) == 'powerlock' @@ -259,12 +261,12 @@ def test_get_credential_store_no_default(self): ) is None def test_get_credential_store_default_index(self): - auth_config = { + auth_config = auth.AuthConfig({ 'credHelpers': { 'https://index.docker.io/v1/': 'powerlock' }, 'credsStore': 'truesecret' - } + }) assert auth.get_credential_store(auth_config, None) == 'powerlock' assert auth.get_credential_store( @@ -293,8 +295,8 @@ def test_load_legacy_config(self): cfg = auth.load_config(cfg_path) assert auth.resolve_authconfig(cfg) is not None - assert cfg['auths'][auth.INDEX_NAME] is not None - cfg = cfg['auths'][auth.INDEX_NAME] + assert cfg.auths[auth.INDEX_NAME] is not None + cfg = cfg.auths[auth.INDEX_NAME] assert cfg['username'] == 'sakuya' assert cfg['password'] == 'izayoi' assert cfg['email'] == 'sakuya@scarlet.net' @@ -312,8 +314,8 @@ def test_load_json_config(self): ) cfg = auth.load_config(cfg_path) assert auth.resolve_authconfig(cfg) is not None - assert cfg['auths'][auth.INDEX_URL] is not None - cfg = cfg['auths'][auth.INDEX_URL] + assert cfg.auths[auth.INDEX_URL] is not None + cfg = cfg.auths[auth.INDEX_URL] assert cfg['username'] == 'sakuya' assert cfg['password'] == 'izayoi' assert cfg['email'] == email @@ -335,8 +337,8 @@ def test_load_modern_json_config(self): }, f) cfg = auth.load_config(cfg_path) assert auth.resolve_authconfig(cfg) is not None - assert cfg['auths'][auth.INDEX_URL] is not None - cfg = cfg['auths'][auth.INDEX_URL] + assert cfg.auths[auth.INDEX_URL] is not None + cfg = cfg.auths[auth.INDEX_URL] assert cfg['username'] == 'sakuya' assert cfg['password'] == 'izayoi' assert cfg['email'] == email @@ -360,7 +362,7 @@ def test_load_config_with_random_name(self): with open(dockercfg_path, 'w') as f: json.dump(config, f) - cfg = auth.load_config(dockercfg_path)['auths'] + cfg = auth.load_config(dockercfg_path).auths assert registry in cfg assert cfg[registry] is not None cfg = cfg[registry] @@ -387,7 +389,7 @@ def test_load_config_custom_config_env(self): json.dump(config, f) with mock.patch.dict(os.environ, {'DOCKER_CONFIG': folder}): - cfg = auth.load_config(None)['auths'] + cfg = auth.load_config(None).auths assert registry in cfg assert cfg[registry] is not None cfg = cfg[registry] @@ -417,8 +419,8 @@ def test_load_config_custom_config_env_with_auths(self): with mock.patch.dict(os.environ, {'DOCKER_CONFIG': folder}): cfg = auth.load_config(None) - assert registry in cfg['auths'] - cfg = cfg['auths'][registry] + assert registry in cfg.auths + cfg = cfg.auths[registry] assert cfg['username'] == 'sakuya' assert cfg['password'] == 'izayoi' assert cfg['email'] == 'sakuya@scarlet.net' @@ -446,8 +448,8 @@ def test_load_config_custom_config_env_utf8(self): with mock.patch.dict(os.environ, {'DOCKER_CONFIG': folder}): cfg = auth.load_config(None) - assert registry in cfg['auths'] - cfg = cfg['auths'][registry] + assert registry in cfg.auths + cfg = cfg.auths[registry] assert cfg['username'] == b'sakuya\xc3\xa6'.decode('utf8') assert cfg['password'] == b'izayoi\xc3\xa6'.decode('utf8') assert cfg['email'] == 'sakuya@scarlet.net' @@ -464,7 +466,7 @@ def test_load_config_unknown_keys(self): json.dump(config, f) cfg = auth.load_config(dockercfg_path) - assert cfg == {'auths': {}} + assert cfg._dct == {'auths': {}} def test_load_config_invalid_auth_dict(self): folder = tempfile.mkdtemp() @@ -479,7 +481,7 @@ def test_load_config_invalid_auth_dict(self): json.dump(config, f) cfg = auth.load_config(dockercfg_path) - assert cfg == {'auths': {'scarlet.net': {}}} + assert cfg._dct == {'auths': {'scarlet.net': {}}} def test_load_config_identity_token(self): folder = tempfile.mkdtemp() @@ -500,7 +502,7 @@ def test_load_config_identity_token(self): json.dump(config, f) cfg = auth.load_config(dockercfg_path) - assert registry in cfg['auths'] - cfg = cfg['auths'][registry] + assert registry in cfg.auths + cfg = cfg.auths[registry] assert 'IdentityToken' in cfg assert cfg['IdentityToken'] == token From 01ccaa6af2106f01b9804177782622f12525b8a5 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Fri, 30 Nov 2018 13:51:01 -0800 Subject: [PATCH 38/68] Make AuthConfig a dict subclass for backward-compatibility Signed-off-by: Joffrey F --- docker/auth.py | 18 +++++++++++------- tests/unit/auth_test.py | 4 ++-- 2 files changed, 13 insertions(+), 9 deletions(-) diff --git a/docker/auth.py b/docker/auth.py index a6c8ae16a3..462390b1d1 100644 --- a/docker/auth.py +++ b/docker/auth.py @@ -43,7 +43,7 @@ def get_config_header(client, registry): log.debug( "No auth config in memory - loading from filesystem" ) - client._auth_configs = load_config() + client._auth_configs = load_config(credstore_env=client.credstore_env) authcfg = resolve_authconfig( client._auth_configs, registry, credstore_env=client.credstore_env ) @@ -70,14 +70,16 @@ def split_repo_name(repo_name): def get_credential_store(authconfig, registry): + if not isinstance(authconfig, AuthConfig): + authconfig = AuthConfig(authconfig) return authconfig.get_credential_store(registry) -class AuthConfig(object): +class AuthConfig(dict): def __init__(self, dct, credstore_env=None): if 'auths' not in dct: dct['auths'] = {} - self._dct = dct + self.update(dct) self._credstore_env = credstore_env self._stores = {} @@ -200,15 +202,15 @@ def load_config(cls, config_path, config_dict, credstore_env=None): @property def auths(self): - return self._dct.get('auths', {}) + return self.get('auths', {}) @property def creds_store(self): - return self._dct.get('credsStore', None) + return self.get('credsStore', None) @property def cred_helpers(self): - return self._dct.get('credHelpers', {}) + return self.get('credHelpers', {}) def resolve_authconfig(self, registry=None): """ @@ -305,10 +307,12 @@ def get_all_credentials(self): return auth_data def add_auth(self, reg, data): - self._dct['auths'][reg] = data + self['auths'][reg] = data def resolve_authconfig(authconfig, registry=None, credstore_env=None): + if not isinstance(authconfig, AuthConfig): + authconfig = AuthConfig(authconfig, credstore_env) return authconfig.resolve_authconfig(registry) diff --git a/tests/unit/auth_test.py b/tests/unit/auth_test.py index 4ad74d5d68..d3c8eee621 100644 --- a/tests/unit/auth_test.py +++ b/tests/unit/auth_test.py @@ -466,7 +466,7 @@ def test_load_config_unknown_keys(self): json.dump(config, f) cfg = auth.load_config(dockercfg_path) - assert cfg._dct == {'auths': {}} + assert dict(cfg) == {'auths': {}} def test_load_config_invalid_auth_dict(self): folder = tempfile.mkdtemp() @@ -481,7 +481,7 @@ def test_load_config_invalid_auth_dict(self): json.dump(config, f) cfg = auth.load_config(dockercfg_path) - assert cfg._dct == {'auths': {'scarlet.net': {}}} + assert dict(cfg) == {'auths': {'scarlet.net': {}}} def test_load_config_identity_token(self): folder = tempfile.mkdtemp() From bef10ecac1692146fd770dcd0a098f28860bce13 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Fri, 30 Nov 2018 13:51:40 -0800 Subject: [PATCH 39/68] Add credstore_env to all load_config calls Signed-off-by: Joffrey F --- docker/api/build.py | 4 +++- docker/api/client.py | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/docker/api/build.py b/docker/api/build.py index 1723083bc7..c4fc37ec98 100644 --- a/docker/api/build.py +++ b/docker/api/build.py @@ -288,7 +288,9 @@ def _set_auth_headers(self, headers): # file one more time in case anything showed up in there. if not self._auth_configs: log.debug("No auth config in memory - loading from filesystem") - self._auth_configs = auth.load_config() + self._auth_configs = auth.load_config( + credsore_env=self.credsore_env + ) # Send the full auth configuration (if any exists), since the build # could use any (or all) of the registries. diff --git a/docker/api/client.py b/docker/api/client.py index 197846d105..74c4698875 100644 --- a/docker/api/client.py +++ b/docker/api/client.py @@ -115,7 +115,7 @@ def __init__(self, base_url=None, version=None, self._general_configs = config.load_general_config() self._auth_configs = auth.load_config( - config_dict=self._general_configs + config_dict=self._general_configs, credstore_env=credstore_env, ) self.credstore_env = credstore_env @@ -476,4 +476,6 @@ def reload_config(self, dockercfg_path=None): Returns: None """ - self._auth_configs = auth.load_config(dockercfg_path) + self._auth_configs = auth.load_config( + dockercfg_path, credstore_env=self.credstore_env + ) From cc38efa68e6640933f19481b4caf5fb21c7b0564 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Fri, 30 Nov 2018 14:41:56 -0800 Subject: [PATCH 40/68] Add some credHelpers tests Signed-off-by: Joffrey F --- docker/auth.py | 2 +- tests/unit/auth_test.py | 281 ++++++++++++++++++++++++++++++++-------- 2 files changed, 231 insertions(+), 52 deletions(-) diff --git a/docker/auth.py b/docker/auth.py index 462390b1d1..c1b874f870 100644 --- a/docker/auth.py +++ b/docker/auth.py @@ -284,7 +284,7 @@ def _get_store_instance(self, name): def get_credential_store(self, registry): if not registry or registry == INDEX_NAME: - registry = 'https://index.docker.io/v1/' + registry = INDEX_URL return self.cred_helpers.get(registry) or self.creds_store diff --git a/tests/unit/auth_test.py b/tests/unit/auth_test.py index d3c8eee621..dc4d6f59ad 100644 --- a/tests/unit/auth_test.py +++ b/tests/unit/auth_test.py @@ -10,6 +10,7 @@ import unittest from docker import auth, errors +import dockerpycreds import pytest try: @@ -226,57 +227,6 @@ def test_resolve_auth_with_empty_credstore_and_auth_dict(self): )['username'] -class CredStoreTest(unittest.TestCase): - def test_get_credential_store(self): - auth_config = auth.AuthConfig({ - 'credHelpers': { - 'registry1.io': 'truesecret', - 'registry2.io': 'powerlock' - }, - 'credsStore': 'blackbox', - }) - - assert auth.get_credential_store( - auth_config, 'registry1.io' - ) == 'truesecret' - assert auth.get_credential_store( - auth_config, 'registry2.io' - ) == 'powerlock' - assert auth.get_credential_store( - auth_config, 'registry3.io' - ) == 'blackbox' - - def test_get_credential_store_no_default(self): - auth_config = auth.AuthConfig({ - 'credHelpers': { - 'registry1.io': 'truesecret', - 'registry2.io': 'powerlock' - }, - }) - assert auth.get_credential_store( - auth_config, 'registry2.io' - ) == 'powerlock' - assert auth.get_credential_store( - auth_config, 'registry3.io' - ) is None - - def test_get_credential_store_default_index(self): - auth_config = auth.AuthConfig({ - 'credHelpers': { - 'https://index.docker.io/v1/': 'powerlock' - }, - 'credsStore': 'truesecret' - }) - - assert auth.get_credential_store(auth_config, None) == 'powerlock' - assert auth.get_credential_store( - auth_config, 'docker.io' - ) == 'powerlock' - assert auth.get_credential_store( - auth_config, 'images.io' - ) == 'truesecret' - - class LoadConfigTest(unittest.TestCase): def test_load_config_no_file(self): folder = tempfile.mkdtemp() @@ -506,3 +456,232 @@ def test_load_config_identity_token(self): cfg = cfg.auths[registry] assert 'IdentityToken' in cfg assert cfg['IdentityToken'] == token + + +class CredstoreTest(unittest.TestCase): + def setUp(self): + self.authconfig = auth.AuthConfig({'credsStore': 'default'}) + self.default_store = InMemoryStore('default') + self.authconfig._stores['default'] = self.default_store + self.default_store.store( + 'https://gensokyo.jp/v2', 'sakuya', 'izayoi', + ) + self.default_store.store( + 'https://default.com/v2', 'user', 'hunter2', + ) + + def test_get_credential_store(self): + auth_config = auth.AuthConfig({ + 'credHelpers': { + 'registry1.io': 'truesecret', + 'registry2.io': 'powerlock' + }, + 'credsStore': 'blackbox', + }) + + assert auth_config.get_credential_store('registry1.io') == 'truesecret' + assert auth_config.get_credential_store('registry2.io') == 'powerlock' + assert auth_config.get_credential_store('registry3.io') == 'blackbox' + + def test_get_credential_store_no_default(self): + auth_config = auth.AuthConfig({ + 'credHelpers': { + 'registry1.io': 'truesecret', + 'registry2.io': 'powerlock' + }, + }) + assert auth_config.get_credential_store('registry2.io') == 'powerlock' + assert auth_config.get_credential_store('registry3.io') is None + + def test_get_credential_store_default_index(self): + auth_config = auth.AuthConfig({ + 'credHelpers': { + 'https://index.docker.io/v1/': 'powerlock' + }, + 'credsStore': 'truesecret' + }) + + assert auth_config.get_credential_store(None) == 'powerlock' + assert auth_config.get_credential_store('docker.io') == 'powerlock' + assert auth_config.get_credential_store('images.io') == 'truesecret' + + def test_get_credential_store_with_plain_dict(self): + auth_config = { + 'credHelpers': { + 'registry1.io': 'truesecret', + 'registry2.io': 'powerlock' + }, + 'credsStore': 'blackbox', + } + + assert auth.get_credential_store( + auth_config, 'registry1.io' + ) == 'truesecret' + assert auth.get_credential_store( + auth_config, 'registry2.io' + ) == 'powerlock' + assert auth.get_credential_store( + auth_config, 'registry3.io' + ) == 'blackbox' + + def test_get_all_credentials_credstore_only(self): + assert self.authconfig.get_all_credentials() == { + 'https://gensokyo.jp/v2': { + 'Username': 'sakuya', + 'Password': 'izayoi', + 'ServerAddress': 'https://gensokyo.jp/v2', + }, + 'https://default.com/v2': { + 'Username': 'user', + 'Password': 'hunter2', + 'ServerAddress': 'https://default.com/v2', + }, + } + + def test_get_all_credentials_with_empty_credhelper(self): + self.authconfig['credHelpers'] = { + 'registry1.io': 'truesecret', + } + self.authconfig._stores['truesecret'] = InMemoryStore() + assert self.authconfig.get_all_credentials() == { + 'https://gensokyo.jp/v2': { + 'Username': 'sakuya', + 'Password': 'izayoi', + 'ServerAddress': 'https://gensokyo.jp/v2', + }, + 'https://default.com/v2': { + 'Username': 'user', + 'Password': 'hunter2', + 'ServerAddress': 'https://default.com/v2', + }, + 'registry1.io': None, + } + + def test_get_all_credentials_with_credhelpers_only(self): + del self.authconfig['credsStore'] + assert self.authconfig.get_all_credentials() == {} + + self.authconfig['credHelpers'] = { + 'https://gensokyo.jp/v2': 'default', + 'https://default.com/v2': 'default', + } + + assert self.authconfig.get_all_credentials() == { + 'https://gensokyo.jp/v2': { + 'Username': 'sakuya', + 'Password': 'izayoi', + 'ServerAddress': 'https://gensokyo.jp/v2', + }, + 'https://default.com/v2': { + 'Username': 'user', + 'Password': 'hunter2', + 'ServerAddress': 'https://default.com/v2', + }, + } + + def test_get_all_credentials_with_auths_entries(self): + self.authconfig.add_auth('registry1.io', { + 'ServerAddress': 'registry1.io', + 'Username': 'reimu', + 'Password': 'hakurei', + }) + + assert self.authconfig.get_all_credentials() == { + 'https://gensokyo.jp/v2': { + 'Username': 'sakuya', + 'Password': 'izayoi', + 'ServerAddress': 'https://gensokyo.jp/v2', + }, + 'https://default.com/v2': { + 'Username': 'user', + 'Password': 'hunter2', + 'ServerAddress': 'https://default.com/v2', + }, + 'registry1.io': { + 'ServerAddress': 'registry1.io', + 'Username': 'reimu', + 'Password': 'hakurei', + }, + } + + def test_get_all_credentials_helpers_override_default(self): + self.authconfig['credHelpers'] = { + 'https://default.com/v2': 'truesecret', + } + truesecret = InMemoryStore('truesecret') + truesecret.store('https://default.com/v2', 'reimu', 'hakurei') + self.authconfig._stores['truesecret'] = truesecret + assert self.authconfig.get_all_credentials() == { + 'https://gensokyo.jp/v2': { + 'Username': 'sakuya', + 'Password': 'izayoi', + 'ServerAddress': 'https://gensokyo.jp/v2', + }, + 'https://default.com/v2': { + 'Username': 'reimu', + 'Password': 'hakurei', + 'ServerAddress': 'https://default.com/v2', + }, + } + + def test_get_all_credentials_3_sources(self): + self.authconfig['credHelpers'] = { + 'registry1.io': 'truesecret', + } + truesecret = InMemoryStore('truesecret') + truesecret.store('registry1.io', 'reimu', 'hakurei') + self.authconfig._stores['truesecret'] = truesecret + self.authconfig.add_auth('registry2.io', { + 'ServerAddress': 'registry2.io', + 'Username': 'reimu', + 'Password': 'hakurei', + }) + + assert self.authconfig.get_all_credentials() == { + 'https://gensokyo.jp/v2': { + 'Username': 'sakuya', + 'Password': 'izayoi', + 'ServerAddress': 'https://gensokyo.jp/v2', + }, + 'https://default.com/v2': { + 'Username': 'user', + 'Password': 'hunter2', + 'ServerAddress': 'https://default.com/v2', + }, + 'registry1.io': { + 'ServerAddress': 'registry1.io', + 'Username': 'reimu', + 'Password': 'hakurei', + }, + 'registry2.io': { + 'ServerAddress': 'registry2.io', + 'Username': 'reimu', + 'Password': 'hakurei', + } + } + + +class InMemoryStore(dockerpycreds.Store): + def __init__(self, *args, **kwargs): + self.__store = {} + + def get(self, server): + try: + return self.__store[server] + except KeyError: + raise dockerpycreds.errors.CredentialsNotFound() + + def store(self, server, username, secret): + self.__store[server] = { + 'ServerURL': server, + 'Username': username, + 'Secret': secret, + } + + def list(self): + return dict( + [(k, v['Username']) for k, v in self.__store.items()] + ) + + def erase(self, server): + del self.__store[server] From b2ad302636bc845b17ef63785738a757aef099f7 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Fri, 30 Nov 2018 14:58:18 -0800 Subject: [PATCH 41/68] Fix test names Signed-off-by: Joffrey F --- tests/unit/api_test.py | 53 ++++++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/tests/unit/api_test.py b/tests/unit/api_test.py index fac314d3d2..d0f22a81c9 100644 --- a/tests/unit/api_test.py +++ b/tests/unit/api_test.py @@ -479,23 +479,26 @@ class TCPSocketStreamTest(unittest.TestCase): built on these islands for generations past? Now shall what of Him? ''' - def setUp(self): - self.server = six.moves.socketserver.ThreadingTCPServer( - ('', 0), self.get_handler_class()) - self.thread = threading.Thread(target=self.server.serve_forever) - self.thread.setDaemon(True) - self.thread.start() - self.address = 'http://{}:{}'.format( - socket.gethostname(), self.server.server_address[1]) - - def tearDown(self): - self.server.shutdown() - self.server.server_close() - self.thread.join() - - def get_handler_class(self): - stdout_data = self.stdout_data - stderr_data = self.stderr_data + @classmethod + def setup_class(cls): + cls.server = six.moves.socketserver.ThreadingTCPServer( + ('', 0), cls.get_handler_class()) + cls.thread = threading.Thread(target=cls.server.serve_forever) + cls.thread.setDaemon(True) + cls.thread.start() + cls.address = 'http://{}:{}'.format( + socket.gethostname(), cls.server.server_address[1]) + + @classmethod + def teardown_class(cls): + cls.server.shutdown() + cls.server.server_close() + cls.thread.join() + + @classmethod + def get_handler_class(cls): + stdout_data = cls.stdout_data + stderr_data = cls.stderr_data class Handler(six.moves.BaseHTTPServer.BaseHTTPRequestHandler, object): def do_POST(self): @@ -542,45 +545,45 @@ def request(self, stream=None, tty=None, demux=None): return client._read_from_socket( resp, stream=stream, tty=tty, demux=demux) - def test_read_from_socket_1(self): + def test_read_from_socket_tty(self): res = self.request(stream=True, tty=True, demux=False) assert next(res) == self.stdout_data + self.stderr_data with self.assertRaises(StopIteration): next(res) - def test_read_from_socket_2(self): + def test_read_from_socket_tty_demux(self): res = self.request(stream=True, tty=True, demux=True) assert next(res) == (self.stdout_data + self.stderr_data, None) with self.assertRaises(StopIteration): next(res) - def test_read_from_socket_3(self): + def test_read_from_socket_no_tty(self): res = self.request(stream=True, tty=False, demux=False) assert next(res) == self.stdout_data assert next(res) == self.stderr_data with self.assertRaises(StopIteration): next(res) - def test_read_from_socket_4(self): + def test_read_from_socket_no_tty_demux(self): res = self.request(stream=True, tty=False, demux=True) assert (self.stdout_data, None) == next(res) assert (None, self.stderr_data) == next(res) with self.assertRaises(StopIteration): next(res) - def test_read_from_socket_5(self): + def test_read_from_socket_no_stream_tty(self): res = self.request(stream=False, tty=True, demux=False) assert res == self.stdout_data + self.stderr_data - def test_read_from_socket_6(self): + def test_read_from_socket_no_stream_tty_demux(self): res = self.request(stream=False, tty=True, demux=True) assert res == (self.stdout_data + self.stderr_data, None) - def test_read_from_socket_7(self): + def test_read_from_socket_no_stream_no_tty(self): res = self.request(stream=False, tty=False, demux=False) res == self.stdout_data + self.stderr_data - def test_read_from_socket_8(self): + def test_read_from_socket_no_stream_no_tty_demux(self): res = self.request(stream=False, tty=False, demux=True) assert res == (self.stdout_data, self.stderr_data) From 16c28093b965e13ef8a394799f7a9aa1bd08307c Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Fri, 30 Nov 2018 15:26:51 -0800 Subject: [PATCH 42/68] Move exec_run example to user guides section of docs Signed-off-by: Joffrey F --- docker/models/containers.py | 64 ----------------------------- docs/index.rst | 1 + docs/user_guides/index.rst | 8 ++++ docs/user_guides/multiplex.rst | 66 ++++++++++++++++++++++++++++++ docs/user_guides/swarm_services.md | 4 ++ 5 files changed, 79 insertions(+), 64 deletions(-) create mode 100644 docs/user_guides/index.rst create mode 100644 docs/user_guides/multiplex.rst diff --git a/docker/models/containers.py b/docker/models/containers.py index 75d8c2ebd7..41bc4da859 100644 --- a/docker/models/containers.py +++ b/docker/models/containers.py @@ -181,70 +181,6 @@ def exec_run(self, cmd, stdout=True, stderr=True, stdin=False, tty=False, Raises: :py:class:`docker.errors.APIError` If the server returns an error. - - Example: - - Create a container that runs in the background - - >>> client = docker.from_env() - >>> container = client.containers.run( - ... 'bfirsh/reticulate-splines', detach=True) - - Prepare the command we are going to use. It prints "hello stdout" - in `stdout`, followed by "hello stderr" in `stderr`: - - >>> cmd = '/bin/sh -c "echo hello stdout ; echo hello stderr >&2"' - - We'll run this command with all four the combinations of ``stream`` - and ``demux``. - - With ``stream=False`` and ``demux=False``, the output is a string - that contains both the `stdout` and the `stderr` output: - - >>> res = container.exec_run(cmd, stream=False, demux=False) - >>> res.output - b'hello stderr\nhello stdout\n' - - With ``stream=True``, and ``demux=False``, the output is a - generator that yields strings containing the output of both - `stdout` and `stderr`: - - >>> res = container.exec_run(cmd, stream=True, demux=False) - >>> next(res.output) - b'hello stdout\n' - >>> next(res.output) - b'hello stderr\n' - >>> next(res.output) - Traceback (most recent call last): - File "", line 1, in - StopIteration - - With ``stream=True`` and ``demux=True``, the generator now - separates the streams, and yield tuples - ``(stdout, stderr)``: - - >>> res = container.exec_run(cmd, stream=True, demux=True) - >>> next(res.output) - (b'hello stdout\n', None) - >>> next(res.output) - (None, b'hello stderr\n') - >>> next(res.output) - Traceback (most recent call last): - File "", line 1, in - StopIteration - - Finally, with ``stream=False`` and ``demux=True``, the whole output - is returned, but the streams are still separated: - - >>> res = container.exec_run(cmd, stream=True, demux=True) - >>> next(res.output) - (b'hello stdout\n', None) - >>> next(res.output) - (None, b'hello stderr\n') - >>> next(res.output) - Traceback (most recent call last): - File "", line 1, in - StopIteration """ resp = self.client.api.exec_create( self.id, cmd, stdout=stdout, stderr=stderr, stdin=stdin, tty=tty, diff --git a/docs/index.rst b/docs/index.rst index 39426b6819..63e85d3635 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -92,4 +92,5 @@ That's just a taste of what you can do with the Docker SDK for Python. For more, volumes api tls + user_guides/index change-log diff --git a/docs/user_guides/index.rst b/docs/user_guides/index.rst new file mode 100644 index 0000000000..79b3a909e3 --- /dev/null +++ b/docs/user_guides/index.rst @@ -0,0 +1,8 @@ +User guides and tutorials +========================= + +.. toctree:: + :maxdepth: 2 + + multiplex + swarm_services \ No newline at end of file diff --git a/docs/user_guides/multiplex.rst b/docs/user_guides/multiplex.rst new file mode 100644 index 0000000000..78d7e3728d --- /dev/null +++ b/docs/user_guides/multiplex.rst @@ -0,0 +1,66 @@ +Handling multiplexed streams +============================ + +.. note:: + The following instruction assume you're interested in getting output from + an ``exec`` command. These instruction are similarly applicable to the + output of ``attach``. + +First create a container that runs in the background: + +>>> client = docker.from_env() +>>> container = client.containers.run( +... 'bfirsh/reticulate-splines', detach=True) + +Prepare the command we are going to use. It prints "hello stdout" +in `stdout`, followed by "hello stderr" in `stderr`: + +>>> cmd = '/bin/sh -c "echo hello stdout ; echo hello stderr >&2"' +We'll run this command with all four the combinations of ``stream`` +and ``demux``. +With ``stream=False`` and ``demux=False``, the output is a string +that contains both the `stdout` and the `stderr` output: +>>> res = container.exec_run(cmd, stream=False, demux=False) +>>> res.output +b'hello stderr\nhello stdout\n' + +With ``stream=True``, and ``demux=False``, the output is a +generator that yields strings containing the output of both +`stdout` and `stderr`: + +>>> res = container.exec_run(cmd, stream=True, demux=False) +>>> next(res.output) +b'hello stdout\n' +>>> next(res.output) +b'hello stderr\n' +>>> next(res.output) +Traceback (most recent call last): + File "", line 1, in +StopIteration + +With ``stream=True`` and ``demux=True``, the generator now +separates the streams, and yield tuples +``(stdout, stderr)``: + +>>> res = container.exec_run(cmd, stream=True, demux=True) +>>> next(res.output) +(b'hello stdout\n', None) +>>> next(res.output) +(None, b'hello stderr\n') +>>> next(res.output) +Traceback (most recent call last): + File "", line 1, in +StopIteration + +Finally, with ``stream=False`` and ``demux=True``, the whole output +is returned, but the streams are still separated: + +>>> res = container.exec_run(cmd, stream=True, demux=True) +>>> next(res.output) +(b'hello stdout\n', None) +>>> next(res.output) +(None, b'hello stderr\n') +>>> next(res.output) +Traceback (most recent call last): + File "", line 1, in +StopIteration diff --git a/docs/user_guides/swarm_services.md b/docs/user_guides/swarm_services.md index 9bd4dca3fb..369fbed00e 100644 --- a/docs/user_guides/swarm_services.md +++ b/docs/user_guides/swarm_services.md @@ -1,5 +1,9 @@ # Swarm services +> Warning: +> This is a stale document and may contain outdated information. +> Refer to the API docs for updated classes and method signatures. + Starting with Engine version 1.12 (API 1.24), it is possible to manage services using the Docker Engine API. Note that the engine needs to be part of a [Swarm cluster](../swarm.rst) before you can use the service-related methods. From bc84ed11ec7f7e20f27139a5e44ae0862d5a1f7b Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Fri, 30 Nov 2018 15:40:14 -0800 Subject: [PATCH 43/68] Fix empty authconfig detection Signed-off-by: Joffrey F --- docker/api/build.py | 4 ++-- docker/api/daemon.py | 2 +- docker/auth.py | 8 +++++++- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/docker/api/build.py b/docker/api/build.py index c4fc37ec98..5db58382ba 100644 --- a/docker/api/build.py +++ b/docker/api/build.py @@ -286,10 +286,10 @@ def _set_auth_headers(self, headers): # If we don't have any auth data so far, try reloading the config # file one more time in case anything showed up in there. - if not self._auth_configs: + if not self._auth_configs or self._auth_configs.is_empty: log.debug("No auth config in memory - loading from filesystem") self._auth_configs = auth.load_config( - credsore_env=self.credsore_env + credstore_env=self.credstore_env ) # Send the full auth configuration (if any exists), since the build diff --git a/docker/api/daemon.py b/docker/api/daemon.py index a2936f2a0e..f715a131ad 100644 --- a/docker/api/daemon.py +++ b/docker/api/daemon.py @@ -127,7 +127,7 @@ def login(self, username, password=None, email=None, registry=None, self._auth_configs = auth.load_config( dockercfg_path, credstore_env=self.credstore_env ) - elif not self._auth_configs: + elif not self._auth_configs or self._auth_configs.is_empty: self._auth_configs = auth.load_config( credstore_env=self.credstore_env ) diff --git a/docker/auth.py b/docker/auth.py index c1b874f870..58b35eb491 100644 --- a/docker/auth.py +++ b/docker/auth.py @@ -39,7 +39,7 @@ def resolve_index_name(index_name): def get_config_header(client, registry): log.debug('Looking for auth config') - if not client._auth_configs: + if not client._auth_configs or client._auth_configs.is_empty: log.debug( "No auth config in memory - loading from filesystem" ) @@ -212,6 +212,12 @@ def creds_store(self): def cred_helpers(self): return self.get('credHelpers', {}) + @property + def is_empty(self): + return ( + not self.auths and not self.creds_store and not self.cred_helpers + ) + def resolve_authconfig(self, registry=None): """ Returns the authentication data from the given auth configuration for a From 3381f7be151400c973e26a32f194e1ef869f6db8 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Fri, 30 Nov 2018 17:26:50 -0800 Subject: [PATCH 44/68] Update setup.py for modern pypi / setuptools Signed-off-by: Joffrey F --- scripts/release.sh | 9 +-------- setup.py | 18 +++++++++++------- 2 files changed, 12 insertions(+), 15 deletions(-) diff --git a/scripts/release.sh b/scripts/release.sh index 5b37b6d083..f3ace27bdf 100755 --- a/scripts/release.sh +++ b/scripts/release.sh @@ -3,12 +3,6 @@ # Create the official release # -if [ -z "$(command -v pandoc 2> /dev/null)" ]; then - >&2 echo "$0 requires http://pandoc.org/" - >&2 echo "Please install it and make sure it is available on your \$PATH." - exit 2 -fi - VERSION=$1 REPO=docker/docker-py GITHUB_REPO=git@github.com:$REPO @@ -37,11 +31,10 @@ if [[ $2 == 'upload' ]]; then fi -pandoc -f markdown -t rst README.md -o README.rst || exit 1 echo "##> sdist & wheel" python setup.py sdist bdist_wheel if [[ $2 == 'upload' ]]; then echo '##> Uploading sdist to pypi' twine upload dist/docker-$VERSION* -fi \ No newline at end of file +fi diff --git a/setup.py b/setup.py index 4ce55fe815..f1c3c204b0 100644 --- a/setup.py +++ b/setup.py @@ -55,24 +55,27 @@ long_description = '' -try: - with codecs.open('./README.rst', encoding='utf-8') as readme_rst: - long_description = readme_rst.read() -except IOError: - # README.rst is only generated on release. Its absence should not prevent - # setup.py from working properly. - pass +with codecs.open('./README.md', encoding='utf-8') as readme_md: + long_description = readme_md.read() setup( name="docker", version=version, description="A Python library for the Docker Engine API.", long_description=long_description, + long_description_content_type='text/markdown', url='https://github.com/docker/docker-py', + project_urls={ + 'Documentation': 'https://docker-py.readthedocs.io', + 'Changelog': 'https://docker-py.readthedocs.io/en/stable/change-log.html', # flake8: noqa + 'Source': 'https://github.com/docker/docker-py', + 'Tracker': 'https://github.com/docker/docker-py/issues', + }, packages=find_packages(exclude=["tests.*", "tests"]), install_requires=requirements, tests_require=test_requirements, extras_require=extras_require, + python_requires='>=2.7, !=3.0.*, !=3.1.*, !=3.2.*', zip_safe=False, test_suite='tests', classifiers=[ @@ -89,6 +92,7 @@ 'Programming Language :: Python :: 3.5', 'Programming Language :: Python :: 3.6', 'Programming Language :: Python :: 3.7', + 'Topic :: Software Development', 'Topic :: Utilities', 'License :: OSI Approved :: Apache Software License', ], From 1bc5783a3d253021f82d21f123b00a8fe45d08e3 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Fri, 7 Dec 2018 15:30:25 -0800 Subject: [PATCH 45/68] Prevent untracked files in releases Signed-off-by: Joffrey F --- scripts/release.sh | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/scripts/release.sh b/scripts/release.sh index f3ace27bdf..d9e7a055a1 100755 --- a/scripts/release.sh +++ b/scripts/release.sh @@ -12,8 +12,9 @@ if [ -z $VERSION ]; then exit 1 fi -echo "##> Removing stale build files" -rm -rf ./build || exit 1 +echo "##> Removing stale build files and other untracked files" +git clean -x -d -i +test -z "$(git clean -x -d -n)" || exit 1 echo "##> Tagging the release as $VERSION" git tag $VERSION From e15db4cb20b22c97e5df5f4fc2fdb54c18e785e9 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Fri, 7 Dec 2018 16:56:45 -0800 Subject: [PATCH 46/68] Improve handling of placement preferences; improve docs Signed-off-by: Joffrey F --- docker/models/services.py | 10 ++++++---- docker/types/__init__.py | 5 +++-- docker/types/services.py | 41 ++++++++++++++++++++++++++++++++------- docs/api.rst | 1 + 4 files changed, 44 insertions(+), 13 deletions(-) diff --git a/docker/models/services.py b/docker/models/services.py index a2a3ed011f..5d2bd9b3ec 100644 --- a/docker/models/services.py +++ b/docker/models/services.py @@ -153,10 +153,12 @@ def create(self, image, command=None, **kwargs): image (str): The image name to use for the containers. command (list of str or str): Command to run. args (list of str): Arguments to the command. - constraints (list of str): Placement constraints. - preferences (list of str): Placement preferences. - platforms (list of tuple): A list of platforms constraints - expressed as ``(arch, os)`` tuples + constraints (list of str): :py:class:`~docker.types.Placement` + constraints. + preferences (list of tuple): :py:class:`~docker.types.Placement` + preferences. + platforms (list of tuple): A list of platform constraints + expressed as ``(arch, os)`` tuples. container_labels (dict): Labels to apply to the container. endpoint_spec (EndpointSpec): Properties that can be configured to access and load balance a service. Default: ``None``. diff --git a/docker/types/__init__.py b/docker/types/__init__.py index 64512333df..f3cac1bc17 100644 --- a/docker/types/__init__.py +++ b/docker/types/__init__.py @@ -5,7 +5,8 @@ from .networks import EndpointConfig, IPAMConfig, IPAMPool, NetworkingConfig from .services import ( ConfigReference, ContainerSpec, DNSConfig, DriverConfig, EndpointSpec, - Mount, Placement, Privileges, Resources, RestartPolicy, RollbackConfig, - SecretReference, ServiceMode, TaskTemplate, UpdateConfig + Mount, Placement, PlacementPreference, Privileges, Resources, + RestartPolicy, RollbackConfig, SecretReference, ServiceMode, TaskTemplate, + UpdateConfig ) from .swarm import SwarmSpec, SwarmExternalCA diff --git a/docker/types/services.py b/docker/types/services.py index c66d41a167..79794d7eb2 100644 --- a/docker/types/services.py +++ b/docker/types/services.py @@ -648,18 +648,24 @@ class Placement(dict): Placement constraints to be used as part of a :py:class:`TaskTemplate` Args: - constraints (:py:class:`list`): A list of constraints - preferences (:py:class:`list`): Preferences provide a way to make - the scheduler aware of factors such as topology. They are - provided in order from highest to lowest precedence. - platforms (:py:class:`list`): A list of platforms expressed as - ``(arch, os)`` tuples + constraints (:py:class:`list` of str): A list of constraints + preferences (:py:class:`list` of tuple): Preferences provide a way + to make the scheduler aware of factors such as topology. They + are provided in order from highest to lowest precedence and + are expressed as ``(strategy, descriptor)`` tuples. See + :py:class:`PlacementPreference` for details. + platforms (:py:class:`list` of tuple): A list of platforms + expressed as ``(arch, os)`` tuples """ def __init__(self, constraints=None, preferences=None, platforms=None): if constraints is not None: self['Constraints'] = constraints if preferences is not None: - self['Preferences'] = preferences + self['Preferences'] = [] + for pref in preferences: + if isinstance(pref, tuple): + pref = PlacementPreference(*pref) + self['Preferences'].append(pref) if platforms: self['Platforms'] = [] for plat in platforms: @@ -668,6 +674,27 @@ def __init__(self, constraints=None, preferences=None, platforms=None): }) +class PlacementPreference(dict): + """ + Placement preference to be used as an element in the list of + preferences for :py:class:`Placement` objects. + + Args: + strategy (string): The placement strategy to implement. Currently, + the only supported strategy is ``spread``. + descriptor (string): A label descriptor. For the spread strategy, + the scheduler will try to spread tasks evenly over groups of + nodes identified by this label. + """ + def __init__(self, strategy, descriptor): + if strategy != 'spread': + raise errors.InvalidArgument( + 'PlacementPreference strategy value is invalid ({}):' + ' must be "spread".'.format(strategy) + ) + self['SpreadOver'] = descriptor + + class DNSConfig(dict): """ Specification for DNS related configurations in resolver configuration diff --git a/docs/api.rst b/docs/api.rst index 1682128951..edb8fffadc 100644 --- a/docs/api.rst +++ b/docs/api.rst @@ -143,6 +143,7 @@ Configuration types .. autoclass:: LogConfig .. autoclass:: Mount .. autoclass:: Placement +.. autoclass:: PlacementPreference .. autoclass:: Privileges .. autoclass:: Resources .. autoclass:: RestartPolicy From b297b837df511178a69f29a9f8461aee0193e278 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Fri, 7 Dec 2018 16:57:40 -0800 Subject: [PATCH 47/68] Dynamically retrieve version information for generated docs Signed-off-by: Joffrey F --- docs/conf.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/docs/conf.py b/docs/conf.py index 3e17678a83..f46d1f76ea 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -69,10 +69,12 @@ # |version| and |release|, also used in various other places throughout the # built documents. # -# The short X.Y version. -version = u'2.0' +with open('../docker/version.py', 'r') as vfile: + exec(vfile.read()) # The full version, including alpha/beta/rc tags. -release = u'2.0' +release = version +# The short X.Y version. +version = '{}.{}'.format(version_info[0], version_info[1]) # The language for content autogenerated by Sphinx. Refer to documentation # for a list of supported languages. From a207122c0d6ce4cbd32c8887533018593dbdeae5 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Fri, 7 Dec 2018 17:04:54 -0800 Subject: [PATCH 48/68] Update Jenkinsfile version map Signed-off-by: Joffrey F --- Jenkinsfile | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 211159bc28..33a0fc3136 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -45,10 +45,13 @@ def getDockerVersions = { -> } def getAPIVersion = { engineVersion -> - def versionMap = ['17.06': '1.30', '17.12': '1.35', '18.02': '1.36', '18.03': '1.37'] + def versionMap = [ + '17.06': '1.30', '17.12': '1.35', '18.02': '1.36', '18.03': '1.37', + '18.06': '1.38', '18.09': '1.39' + ] def result = versionMap[engineVersion.substring(0, 5)] if (!result) { - return '1.37' + return '1.39' } return result } From 543d83cb094ef7b1fe9f5cd61fcc978b99b14ca3 Mon Sep 17 00:00:00 2001 From: Maximilian Bischoff Date: Fri, 14 Dec 2018 17:50:20 +0100 Subject: [PATCH 49/68] Fixed a typo in the configs api doc The documentation for id in ConfigApiMixin inspect_config was wrongly mentioning removal of a config Signed-off-by: Maximilian Bischoff --- docker/api/config.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/api/config.py b/docker/api/config.py index 767bef263a..93e5168f64 100644 --- a/docker/api/config.py +++ b/docker/api/config.py @@ -42,7 +42,7 @@ def inspect_config(self, id): Retrieve config metadata Args: - id (string): Full ID of the config to remove + id (string): Full ID of the config to inspect Returns (dict): A dictionary of metadata From 341e2580aa5c1b278d93f382517a48fc177c64bd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Schoentgen?= Date: Thu, 20 Dec 2018 14:43:00 +0100 Subject: [PATCH 50/68] Fix DeprecationWarning: invalid escape sequence in services.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Mickaël Schoentgen --- docker/types/services.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/types/services.py b/docker/types/services.py index 79794d7eb2..ac1c181a90 100644 --- a/docker/types/services.py +++ b/docker/types/services.py @@ -714,7 +714,7 @@ def __init__(self, nameservers=None, search=None, options=None): class Privileges(dict): - """ + r""" Security options for a service's containers. Part of a :py:class:`ContainerSpec` definition. From e99ce1e3593c64e6a6cae08a0af99ccb05a005e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Micka=C3=ABl=20Schoentgen?= Date: Thu, 20 Dec 2018 14:47:28 +0100 Subject: [PATCH 51/68] Fix DeprecationWarning: invalid escape sequence in ports.py MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Mickaël Schoentgen --- docker/utils/ports.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/docker/utils/ports.py b/docker/utils/ports.py index bf7d697271..cf5987c94f 100644 --- a/docker/utils/ports.py +++ b/docker/utils/ports.py @@ -3,10 +3,10 @@ PORT_SPEC = re.compile( "^" # Match full string "(" # External part - "((?P[a-fA-F\d.:]+):)?" # Address - "(?P[\d]*)(-(?P[\d]+))?:" # External range + r"((?P[a-fA-F\d.:]+):)?" # Address + r"(?P[\d]*)(-(?P[\d]+))?:" # External range ")?" - "(?P[\d]+)(-(?P[\d]+))?" # Internal range + r"(?P[\d]+)(-(?P[\d]+))?" # Internal range "(?P/(udp|tcp))?" # Protocol "$" # Match full string ) From 3cda1e8bbd20900e9eadd9b88459d16c86d93648 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Fri, 28 Dec 2018 15:45:54 -0800 Subject: [PATCH 52/68] Make swarm.init() return value match documentation Signed-off-by: Joffrey F --- docker/models/swarm.py | 1 + 1 file changed, 1 insertion(+) diff --git a/docker/models/swarm.py b/docker/models/swarm.py index 7396e730d7..3a02ae3707 100644 --- a/docker/models/swarm.py +++ b/docker/models/swarm.py @@ -112,6 +112,7 @@ def init(self, advertise_addr=None, listen_addr='0.0.0.0:2377', init_kwargs['swarm_spec'] = self.client.api.create_swarm_spec(**kwargs) self.client.api.init_swarm(**init_kwargs) self.reload() + return True def join(self, *args, **kwargs): return self.client.api.join_swarm(*args, **kwargs) From 72f4f527ad60b2a677b883dc54669ebe98f0879f Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Wed, 9 Jan 2019 11:14:08 -0800 Subject: [PATCH 53/68] Update test dependencies to latest version, fix some flake8 errors Signed-off-by: Joffrey F --- docker/auth.py | 2 +- docker/types/containers.py | 11 +++++------ setup.py | 2 +- test-requirements.txt | 10 ++++++---- tests/integration/api_client_test.py | 2 +- tests/unit/dockertypes_test.py | 2 +- 6 files changed, 15 insertions(+), 14 deletions(-) diff --git a/docker/auth.py b/docker/auth.py index 58b35eb491..638ab9b0a9 100644 --- a/docker/auth.py +++ b/docker/auth.py @@ -273,7 +273,7 @@ def _resolve_authconfig_credstore(self, registry, credstore_name): 'Password': data['Secret'], }) return res - except dockerpycreds.CredentialsNotFound as e: + except dockerpycreds.CredentialsNotFound: log.debug('No entry found') return None except dockerpycreds.StoreError as e: diff --git a/docker/types/containers.py b/docker/types/containers.py index d040c0fb5e..fd8cab4979 100644 --- a/docker/types/containers.py +++ b/docker/types/containers.py @@ -51,8 +51,7 @@ class LogConfig(DictType): ... host_config=hc) >>> client.inspect_container(container)['HostConfig']['LogConfig'] {'Type': 'json-file', 'Config': {'labels': 'production_status,geo', 'max-size': '1g'}} - - """ # flake8: noqa + """ # noqa: E501 types = LogConfigTypesEnum def __init__(self, **kwargs): @@ -320,10 +319,10 @@ def __init__(self, version, binds=None, port_bindings=None, if not isinstance(ulimits, list): raise host_config_type_error('ulimits', ulimits, 'list') self['Ulimits'] = [] - for l in ulimits: - if not isinstance(l, Ulimit): - l = Ulimit(**l) - self['Ulimits'].append(l) + for lmt in ulimits: + if not isinstance(lmt, Ulimit): + lmt = Ulimit(**lmt) + self['Ulimits'].append(lmt) if log_config is not None: if not isinstance(log_config, LogConfig): diff --git a/setup.py b/setup.py index f1c3c204b0..94fbdf444e 100644 --- a/setup.py +++ b/setup.py @@ -67,7 +67,7 @@ url='https://github.com/docker/docker-py', project_urls={ 'Documentation': 'https://docker-py.readthedocs.io', - 'Changelog': 'https://docker-py.readthedocs.io/en/stable/change-log.html', # flake8: noqa + 'Changelog': 'https://docker-py.readthedocs.io/en/stable/change-log.html', # noqa: E501 'Source': 'https://github.com/docker/docker-py', 'Tracker': 'https://github.com/docker/docker-py/issues', }, diff --git a/test-requirements.txt b/test-requirements.txt index 07e1a900db..510fa295ec 100644 --- a/test-requirements.txt +++ b/test-requirements.txt @@ -1,7 +1,9 @@ -coverage==3.7.1 -flake8==3.4.1 +coverage==4.5.2 +flake8==3.6.0; python_version != '3.3' +flake8==3.4.1; python_version == '3.3' mock==1.0.1 pytest==2.9.1; python_version == '3.3' -pytest==3.6.3; python_version > '3.3' -pytest-cov==2.1.0 +pytest==4.1.0; python_version != '3.3' +pytest-cov==2.6.1; python_version != '3.3' +pytest-cov==2.5.1; python_version == '3.3' pytest-timeout==1.3.3 diff --git a/tests/integration/api_client_test.py b/tests/integration/api_client_test.py index 905e06484d..9e348f3e3f 100644 --- a/tests/integration/api_client_test.py +++ b/tests/integration/api_client_test.py @@ -47,7 +47,7 @@ def test_timeout(self): # This call isn't supposed to complete, and it should fail fast. try: res = self.client.inspect_container('id') - except: + except: # noqa: E722 pass end = time.time() assert res is None diff --git a/tests/unit/dockertypes_test.py b/tests/unit/dockertypes_test.py index cdacf8cd5b..0689d07b32 100644 --- a/tests/unit/dockertypes_test.py +++ b/tests/unit/dockertypes_test.py @@ -14,7 +14,7 @@ try: from unittest import mock -except: +except: # noqa: E722 import mock From bfdd0a881ea10e0f09a90a5282ccb1e023e1ba75 Mon Sep 17 00:00:00 2001 From: Corentin Henry Date: Wed, 12 Dec 2018 12:29:06 +0100 Subject: [PATCH 54/68] add support for proxies Signed-off-by: Corentin Henry --- docker/api/build.py | 5 ++- docker/api/client.py | 7 +++++ docker/api/container.py | 4 +++ docker/api/exec_api.py | 1 + docker/utils/proxy.py | 69 +++++++++++++++++++++++++++++++++++++++++ 5 files changed, 85 insertions(+), 1 deletion(-) create mode 100644 docker/utils/proxy.py diff --git a/docker/api/build.py b/docker/api/build.py index 5db58382ba..0871df8c60 100644 --- a/docker/api/build.py +++ b/docker/api/build.py @@ -168,8 +168,11 @@ def build(self, path=None, tag=None, quiet=False, fileobj=None, } params.update(container_limits) + final_buildargs = self._proxy_configs.get_environment() if buildargs: - params.update({'buildargs': json.dumps(buildargs)}) + final_buildargs.update(buildargs) + if final_buildargs: + params.update({'buildargs': json.dumps(final_buildargs)}) if shmsize: if utils.version_gte(self._version, '1.22'): diff --git a/docker/api/client.py b/docker/api/client.py index 265dfdcef5..b12398e5ed 100644 --- a/docker/api/client.py +++ b/docker/api/client.py @@ -34,6 +34,7 @@ from ..utils import utils, check_resource, update_headers, config from ..utils.socket import frames_iter, consume_socket_output, demux_adaptor from ..utils.json_stream import json_stream +from ..utils.proxy import ProxyConfig try: from ..transport import NpipeAdapter except ImportError: @@ -114,6 +115,12 @@ def __init__(self, base_url=None, version=None, self.headers['User-Agent'] = user_agent self._general_configs = config.load_general_config() + try: + proxies = self._general_configs['proxies']['default'] + except KeyError: + proxies = {} + self._proxy_configs = ProxyConfig.from_dict(proxies) + self._auth_configs = auth.load_config( config_dict=self._general_configs, credstore_env=credstore_env, ) diff --git a/docker/api/container.py b/docker/api/container.py index ab3b1cf410..2a80ff7dc7 100644 --- a/docker/api/container.py +++ b/docker/api/container.py @@ -403,6 +403,10 @@ def create_container(self, image, command=None, hostname=None, user=None, if isinstance(volumes, six.string_types): volumes = [volumes, ] + if isinstance(environment, dict): + environment = utils.utils.format_environment(environment) + environment = self._proxy_configs.inject_proxy_environment(environment) + config = self.create_container_config( image, command, hostname, user, detach, stdin_open, tty, ports, environment, volumes, diff --git a/docker/api/exec_api.py b/docker/api/exec_api.py index d13b128998..0dabdd3078 100644 --- a/docker/api/exec_api.py +++ b/docker/api/exec_api.py @@ -50,6 +50,7 @@ def exec_create(self, container, cmd, stdout=True, stderr=True, if isinstance(environment, dict): environment = utils.utils.format_environment(environment) + environment = self._proxy_configs.inject_proxy_environment(environment) data = { 'Container': container, diff --git a/docker/utils/proxy.py b/docker/utils/proxy.py new file mode 100644 index 0000000000..3f55a3c13d --- /dev/null +++ b/docker/utils/proxy.py @@ -0,0 +1,69 @@ +from .utils import format_environment + + +class ProxyConfig(): + ''' + Hold the client's proxy configuration + ''' + + def __init__(self, http=None, https=None, ftp=None, no_proxy=None): + self.http = http + self.https = https + self.ftp = ftp + self.no_proxy = no_proxy + + @staticmethod + def from_dict(config): + ''' + Instantiate a new ProxyConfig from a dictionary that represents a + client configuration, as described in `the documentation`_. + + .. _the documentation: + https://docs.docker.com/network/proxy/#configure-the-docker-client + ''' + return ProxyConfig( + http=config.get('httpProxy', None), + https=config.get('httpsProxy', None), + ftp=config.get('ftpProxy', None), + no_proxy=config.get('noProxy', None)) + + def get_environment(self): + ''' + Return a dictionary representing the environment variables used to + set the proxy settings. + ''' + env = {} + if self.http: + env['http_proxy'] = env['HTTP_PROXY'] = self.http + if self.https: + env['https_proxy'] = env['HTTPS_PROXY'] = self.https + if self.ftp: + env['ftp_proxy'] = env['FTP_PROXY'] = self.ftp + if self.no_proxy: + env['no_proxy'] = env['NO_PROXY'] = self.no_proxy + return env + + def inject_proxy_environment(self, environment): + ''' + Given a list of strings representing environment variables, prepend the + environemt variables corresponding to the proxy settings. + ''' + if not self: + return environment + + proxy_env = format_environment(self.get_environment()) + if not environment: + return proxy_env + # It is important to prepend our variables, because we want the + # variables defined in "environment" to take precedence. + return proxy_env + environment + + def __bool__(self): + return bool(self.http or self.https or self.ftp or self.no_proxy) + + def __nonzero__(self): + return self.__bool__() + + def __str__(self): + return 'ProxyConfig(http={}, https={}, ftp={}, no_proxy={})'.format( + self.http, self.https, self.ftp, self.no_proxy) From 6e227895d3ed2a0d41409f7e2bdd7d82bf6a8068 Mon Sep 17 00:00:00 2001 From: Corentin Henry Date: Wed, 12 Dec 2018 15:14:49 +0100 Subject: [PATCH 55/68] tests: remove outdated code the _cfg attribute does not exist anymore Signed-off-by: Corentin Henry --- tests/unit/api_test.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/unit/api_test.py b/tests/unit/api_test.py index 203caf3fbb..f4d220a2c6 100644 --- a/tests/unit/api_test.py +++ b/tests/unit/api_test.py @@ -106,8 +106,6 @@ def setUp(self): ) self.patcher.start() self.client = APIClient() - # Force-clear authconfig to avoid tampering with the tests - self.client._cfg = {'Configs': {}} def tearDown(self): self.client.close() From 545adc2a59193cbdf4fc79bfe761828229d1dc0f Mon Sep 17 00:00:00 2001 From: Corentin Henry Date: Wed, 12 Dec 2018 15:58:00 +0100 Subject: [PATCH 56/68] add unit tests for ProxyConfig Signed-off-by: Corentin Henry --- tests/unit/utils_proxy_test.py | 84 ++++++++++++++++++++++++++++++++++ 1 file changed, 84 insertions(+) create mode 100644 tests/unit/utils_proxy_test.py diff --git a/tests/unit/utils_proxy_test.py b/tests/unit/utils_proxy_test.py new file mode 100644 index 0000000000..ff0e14ba74 --- /dev/null +++ b/tests/unit/utils_proxy_test.py @@ -0,0 +1,84 @@ +# -*- coding: utf-8 -*- + +import unittest +import six + +from docker.utils.proxy import ProxyConfig + +HTTP = 'http://test:80' +HTTPS = 'https://test:443' +FTP = 'ftp://user:password@host:23' +NO_PROXY = 'localhost,.localdomain' +CONFIG = ProxyConfig(http=HTTP, https=HTTPS, ftp=FTP, no_proxy=NO_PROXY) +ENV = { + 'http_proxy': HTTP, + 'HTTP_PROXY': HTTP, + 'https_proxy': HTTPS, + 'HTTPS_PROXY': HTTPS, + 'ftp_proxy': FTP, + 'FTP_PROXY': FTP, + 'no_proxy': NO_PROXY, + 'NO_PROXY': NO_PROXY, +} + + +class ProxyConfigTest(unittest.TestCase): + + def test_from_dict(self): + config = ProxyConfig.from_dict({ + 'httpProxy': HTTP, + 'httpsProxy': HTTPS, + 'ftpProxy': FTP, + 'noProxy': NO_PROXY + }) + self.assertEqual(CONFIG.http, config.http) + self.assertEqual(CONFIG.https, config.https) + self.assertEqual(CONFIG.ftp, config.ftp) + self.assertEqual(CONFIG.no_proxy, config.no_proxy) + + def test_new(self): + config = ProxyConfig() + self.assertIsNone(config.http) + self.assertIsNone(config.https) + self.assertIsNone(config.ftp) + self.assertIsNone(config.no_proxy) + + config = ProxyConfig(http='a', https='b', ftp='c', no_proxy='d') + self.assertEqual(config.http, 'a') + self.assertEqual(config.https, 'b') + self.assertEqual(config.ftp, 'c') + self.assertEqual(config.no_proxy, 'd') + + def test_truthiness(self): + assert not ProxyConfig() + assert ProxyConfig(http='non-zero') + assert ProxyConfig(https='non-zero') + assert ProxyConfig(ftp='non-zero') + assert ProxyConfig(no_proxy='non-zero') + + def test_environment(self): + self.assertDictEqual(CONFIG.get_environment(), ENV) + empty = ProxyConfig() + self.assertDictEqual(empty.get_environment(), {}) + + def test_inject_proxy_environment(self): + # Proxy config is non null, env is None. + self.assertSetEqual( + set(CONFIG.inject_proxy_environment(None)), + set(['{}={}'.format(k, v) for k, v in six.iteritems(ENV)])) + + # Proxy config is null, env is None. + self.assertIsNone(ProxyConfig().inject_proxy_environment(None), None) + + env = ['FOO=BAR', 'BAR=BAZ'] + + # Proxy config is non null, env is non null + actual = CONFIG.inject_proxy_environment(env) + expected = ['{}={}'.format(k, v) for k, v in six.iteritems(ENV)] + env + # It's important that the first 8 variables are the ones from the proxy + # config, and the last 2 are the ones from the input environment + self.assertSetEqual(set(actual[:8]), set(expected[:8])) + self.assertSetEqual(set(actual[-2:]), set(expected[-2:])) + + # Proxy is null, and is non null + self.assertListEqual(ProxyConfig().inject_proxy_environment(env), env) From 4f79ba160e78f5327c683b6a7797bde5dfb6dc32 Mon Sep 17 00:00:00 2001 From: Corentin Henry Date: Wed, 12 Dec 2018 17:10:04 +0100 Subject: [PATCH 57/68] make the integration tests more verbose Signed-off-by: Corentin Henry --- Jenkinsfile | 2 +- Makefile | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 33a0fc3136..387e147036 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -91,7 +91,7 @@ def runTests = { Map settings -> --network ${testNetwork} \\ --volumes-from ${dindContainerName} \\ ${testImage} \\ - py.test -v -rxs tests/integration + py.test -vv -rxs tests/integration """ } finally { sh """ diff --git a/Makefile b/Makefile index 434d40e1cc..684716af2f 100644 --- a/Makefile +++ b/Makefile @@ -35,11 +35,11 @@ unit-test-py3: build-py3 .PHONY: integration-test integration-test: build - docker run -t --rm -v /var/run/docker.sock:/var/run/docker.sock docker-sdk-python py.test -v tests/integration/${file} + docker run -t --rm -v /var/run/docker.sock:/var/run/docker.sock docker-sdk-python py.test -vv tests/integration/${file} .PHONY: integration-test-py3 integration-test-py3: build-py3 - docker run -t --rm -v /var/run/docker.sock:/var/run/docker.sock docker-sdk-python3 py.test tests/integration/${file} + docker run -t --rm -v /var/run/docker.sock:/var/run/docker.sock docker-sdk-python3 py.test -vv tests/integration/${file} TEST_API_VERSION ?= 1.35 TEST_ENGINE_VERSION ?= 17.12.0-ce @@ -57,7 +57,7 @@ integration-dind-py2: build setup-network docker run -d --network dpy-tests --name dpy-dind-py2 --privileged\ dockerswarm/dind:${TEST_ENGINE_VERSION} dockerd -H tcp://0.0.0.0:2375 --experimental docker run -t --rm --env="DOCKER_HOST=tcp://dpy-dind-py2:2375" --env="DOCKER_TEST_API_VERSION=${TEST_API_VERSION}"\ - --network dpy-tests docker-sdk-python py.test tests/integration + --network dpy-tests docker-sdk-python py.test -vv tests/integration docker rm -vf dpy-dind-py2 .PHONY: integration-dind-py3 @@ -66,7 +66,7 @@ integration-dind-py3: build-py3 setup-network docker run -d --network dpy-tests --name dpy-dind-py3 --privileged\ dockerswarm/dind:${TEST_ENGINE_VERSION} dockerd -H tcp://0.0.0.0:2375 --experimental docker run -t --rm --env="DOCKER_HOST=tcp://dpy-dind-py3:2375" --env="DOCKER_TEST_API_VERSION=${TEST_API_VERSION}"\ - --network dpy-tests docker-sdk-python3 py.test tests/integration + --network dpy-tests docker-sdk-python3 py.test -vv tests/integration docker rm -vf dpy-dind-py3 .PHONY: integration-dind-ssl @@ -81,10 +81,10 @@ integration-dind-ssl: build-dind-certs build build-py3 --tlskey=/certs/server-key.pem -H tcp://0.0.0.0:2375 --experimental docker run -t --rm --volumes-from dpy-dind-ssl --env="DOCKER_HOST=tcp://docker:2375"\ --env="DOCKER_TLS_VERIFY=1" --env="DOCKER_CERT_PATH=/certs" --env="DOCKER_TEST_API_VERSION=${TEST_API_VERSION}"\ - --network dpy-tests docker-sdk-python py.test tests/integration + --network dpy-tests docker-sdk-python py.test -vv tests/integration docker run -t --rm --volumes-from dpy-dind-ssl --env="DOCKER_HOST=tcp://docker:2375"\ --env="DOCKER_TLS_VERIFY=1" --env="DOCKER_CERT_PATH=/certs" --env="DOCKER_TEST_API_VERSION=${TEST_API_VERSION}"\ - --network dpy-tests docker-sdk-python3 py.test tests/integration + --network dpy-tests docker-sdk-python3 py.test -vv tests/integration docker rm -vf dpy-dind-ssl dpy-dind-certs .PHONY: flake8 From 0d37390c463b64b3ec3f95831921ff19eaf1a598 Mon Sep 17 00:00:00 2001 From: Corentin Henry Date: Wed, 12 Dec 2018 16:39:24 +0100 Subject: [PATCH 58/68] add integration tests for proxy support Signed-off-by: Corentin Henry --- tests/integration/api_build_test.py | 38 +++++++++++++++++++++++++++++ tests/integration/api_exec_test.py | 34 ++++++++++++++++++++++++++ 2 files changed, 72 insertions(+) diff --git a/tests/integration/api_build_test.py b/tests/integration/api_build_test.py index bad411beec..3c4e982048 100644 --- a/tests/integration/api_build_test.py +++ b/tests/integration/api_build_test.py @@ -4,6 +4,7 @@ import tempfile from docker import errors +from docker.utils.proxy import ProxyConfig import pytest import six @@ -13,6 +14,43 @@ class BuildTest(BaseAPIIntegrationTest): + def test_build_with_proxy(self): + self.client._proxy_configs = ProxyConfig( + ftp='a', http='b', https='c', no_proxy='d') + + script = io.BytesIO('\n'.join([ + 'FROM busybox', + 'RUN env | grep "FTP_PROXY=a"', + 'RUN env | grep "ftp_proxy=a"', + 'RUN env | grep "HTTP_PROXY=b"', + 'RUN env | grep "http_proxy=b"', + 'RUN env | grep "HTTPS_PROXY=c"', + 'RUN env | grep "https_proxy=c"', + 'RUN env | grep "NO_PROXY=d"', + 'RUN env | grep "no_proxy=d"', + ]).encode('ascii')) + self.client.build(fileobj=script, decode=True) + + def test_build_with_proxy_and_buildargs(self): + self.client._proxy_configs = ProxyConfig( + ftp='a', http='b', https='c', no_proxy='d') + + script = io.BytesIO('\n'.join([ + 'FROM busybox', + 'RUN env | grep "FTP_PROXY=XXX"', + 'RUN env | grep "ftp_proxy=xxx"', + 'RUN env | grep "HTTP_PROXY=b"', + 'RUN env | grep "http_proxy=b"', + 'RUN env | grep "HTTPS_PROXY=c"', + 'RUN env | grep "https_proxy=c"', + 'RUN env | grep "NO_PROXY=d"', + 'RUN env | grep "no_proxy=d"', + ]).encode('ascii')) + self.client.build( + fileobj=script, + decode=True, + buildargs={'FTP_PROXY': 'XXX', 'ftp_proxy': 'xxx'}) + def test_build_streaming(self): script = io.BytesIO('\n'.join([ 'FROM busybox', diff --git a/tests/integration/api_exec_test.py b/tests/integration/api_exec_test.py index 857a18cb3f..6d4f97daab 100644 --- a/tests/integration/api_exec_test.py +++ b/tests/integration/api_exec_test.py @@ -1,5 +1,6 @@ from docker.utils.socket import next_frame_header from docker.utils.socket import read_exactly +from docker.utils.proxy import ProxyConfig from .base import BaseAPIIntegrationTest, BUSYBOX from ..helpers import ( @@ -8,6 +9,39 @@ class ExecTest(BaseAPIIntegrationTest): + def test_execute_proxy_env(self): + # Set a custom proxy config on the client + self.client._proxy_configs = ProxyConfig( + ftp='a', https='b', http='c', no_proxy='d') + + container = self.client.create_container( + BUSYBOX, 'cat', detach=True, stdin_open=True) + id = container['Id'] + self.client.start(id) + self.tmp_containers.append(id) + + # First, just make sure the environment variables from the custom + # config are set + res = self.client.exec_create( + id, cmd='sh -c "env | grep -i proxy"') + output = self.client.exec_start(res).decode('utf-8').split('\n') + expected = [ + 'ftp_proxy=a', 'https_proxy=b', 'http_proxy=c', 'no_proxy=d', + 'FTP_PROXY=a', 'HTTPS_PROXY=b', 'HTTP_PROXY=c', 'NO_PROXY=d'] + for item in expected: + assert item in output + + # Overwrite some variables with a custom environment + env = {'https_proxy': 'xxx', 'HTTPS_PROXY': 'XXX'} + res = self.client.exec_create( + id, cmd='sh -c "env | grep -i proxy"', environment=env) + output = self.client.exec_start(res).decode('utf-8').split('\n') + expected = [ + 'ftp_proxy=a', 'https_proxy=xxx', 'http_proxy=c', 'no_proxy=d', + 'FTP_PROXY=a', 'HTTPS_PROXY=XXX', 'HTTP_PROXY=c', 'NO_PROXY=d'] + for item in expected: + assert item in output + def test_execute_command(self): container = self.client.create_container(BUSYBOX, 'cat', detach=True, stdin_open=True) From 708ef6d81ebd12d997b8bca5481541655cbfa61d Mon Sep 17 00:00:00 2001 From: Corentin Henry Date: Sat, 15 Dec 2018 14:55:43 +0100 Subject: [PATCH 59/68] Revert "make the integration tests more verbose" This reverts commit 7584e5d7f068400d33d6658ae706c79db1aab2c5. Signed-off-by: Corentin Henry --- Jenkinsfile | 2 +- Makefile | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 387e147036..33a0fc3136 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -91,7 +91,7 @@ def runTests = { Map settings -> --network ${testNetwork} \\ --volumes-from ${dindContainerName} \\ ${testImage} \\ - py.test -vv -rxs tests/integration + py.test -v -rxs tests/integration """ } finally { sh """ diff --git a/Makefile b/Makefile index 684716af2f..434d40e1cc 100644 --- a/Makefile +++ b/Makefile @@ -35,11 +35,11 @@ unit-test-py3: build-py3 .PHONY: integration-test integration-test: build - docker run -t --rm -v /var/run/docker.sock:/var/run/docker.sock docker-sdk-python py.test -vv tests/integration/${file} + docker run -t --rm -v /var/run/docker.sock:/var/run/docker.sock docker-sdk-python py.test -v tests/integration/${file} .PHONY: integration-test-py3 integration-test-py3: build-py3 - docker run -t --rm -v /var/run/docker.sock:/var/run/docker.sock docker-sdk-python3 py.test -vv tests/integration/${file} + docker run -t --rm -v /var/run/docker.sock:/var/run/docker.sock docker-sdk-python3 py.test tests/integration/${file} TEST_API_VERSION ?= 1.35 TEST_ENGINE_VERSION ?= 17.12.0-ce @@ -57,7 +57,7 @@ integration-dind-py2: build setup-network docker run -d --network dpy-tests --name dpy-dind-py2 --privileged\ dockerswarm/dind:${TEST_ENGINE_VERSION} dockerd -H tcp://0.0.0.0:2375 --experimental docker run -t --rm --env="DOCKER_HOST=tcp://dpy-dind-py2:2375" --env="DOCKER_TEST_API_VERSION=${TEST_API_VERSION}"\ - --network dpy-tests docker-sdk-python py.test -vv tests/integration + --network dpy-tests docker-sdk-python py.test tests/integration docker rm -vf dpy-dind-py2 .PHONY: integration-dind-py3 @@ -66,7 +66,7 @@ integration-dind-py3: build-py3 setup-network docker run -d --network dpy-tests --name dpy-dind-py3 --privileged\ dockerswarm/dind:${TEST_ENGINE_VERSION} dockerd -H tcp://0.0.0.0:2375 --experimental docker run -t --rm --env="DOCKER_HOST=tcp://dpy-dind-py3:2375" --env="DOCKER_TEST_API_VERSION=${TEST_API_VERSION}"\ - --network dpy-tests docker-sdk-python3 py.test -vv tests/integration + --network dpy-tests docker-sdk-python3 py.test tests/integration docker rm -vf dpy-dind-py3 .PHONY: integration-dind-ssl @@ -81,10 +81,10 @@ integration-dind-ssl: build-dind-certs build build-py3 --tlskey=/certs/server-key.pem -H tcp://0.0.0.0:2375 --experimental docker run -t --rm --volumes-from dpy-dind-ssl --env="DOCKER_HOST=tcp://docker:2375"\ --env="DOCKER_TLS_VERIFY=1" --env="DOCKER_CERT_PATH=/certs" --env="DOCKER_TEST_API_VERSION=${TEST_API_VERSION}"\ - --network dpy-tests docker-sdk-python py.test -vv tests/integration + --network dpy-tests docker-sdk-python py.test tests/integration docker run -t --rm --volumes-from dpy-dind-ssl --env="DOCKER_HOST=tcp://docker:2375"\ --env="DOCKER_TLS_VERIFY=1" --env="DOCKER_CERT_PATH=/certs" --env="DOCKER_TEST_API_VERSION=${TEST_API_VERSION}"\ - --network dpy-tests docker-sdk-python3 py.test -vv tests/integration + --network dpy-tests docker-sdk-python3 py.test tests/integration docker rm -vf dpy-dind-ssl dpy-dind-certs .PHONY: flake8 From 9146dd57d7a71f830a4845b9ecd52d561dee29ef Mon Sep 17 00:00:00 2001 From: Corentin Henry Date: Sat, 15 Dec 2018 14:16:56 +0100 Subject: [PATCH 60/68] code style improvement Signed-off-by: Corentin Henry --- docker/api/build.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docker/api/build.py b/docker/api/build.py index 0871df8c60..dab06d5d57 100644 --- a/docker/api/build.py +++ b/docker/api/build.py @@ -168,11 +168,11 @@ def build(self, path=None, tag=None, quiet=False, fileobj=None, } params.update(container_limits) - final_buildargs = self._proxy_configs.get_environment() + proxy_args = self._proxy_configs.get_environment() + for k, v in proxy_args.items(): + buildargs.setdefault(k, v) if buildargs: - final_buildargs.update(buildargs) - if final_buildargs: - params.update({'buildargs': json.dumps(final_buildargs)}) + params.update({'buildargs': json.dumps(buildargs)}) if shmsize: if utils.version_gte(self._version, '1.22'): From f97f71342ff07038ee56fac030daef6f568fa4b3 Mon Sep 17 00:00:00 2001 From: Corentin Henry Date: Sat, 15 Dec 2018 14:27:09 +0100 Subject: [PATCH 61/68] refactor ProxyConfig Signed-off-by: Corentin Henry --- docker/utils/proxy.py | 52 ++++++++++++++++++++++++++++++------------- 1 file changed, 36 insertions(+), 16 deletions(-) diff --git a/docker/utils/proxy.py b/docker/utils/proxy.py index 3f55a3c13d..943c25b02f 100644 --- a/docker/utils/proxy.py +++ b/docker/utils/proxy.py @@ -1,16 +1,41 @@ from .utils import format_environment -class ProxyConfig(): +class ProxyConfig(dict): ''' Hold the client's proxy configuration ''' + @property + def http(self): + return self['http'] - def __init__(self, http=None, https=None, ftp=None, no_proxy=None): - self.http = http - self.https = https - self.ftp = ftp - self.no_proxy = no_proxy + @http.setter + def http(self, value): + self['http'] = value + + @property + def https(self): + return self['https'] + + @https.setter + def https(self, value): + self['https'] = value + + @property + def ftp(self): + return self['ftp'] + + @ftp.setter + def ftp(self, value): + self['ftp'] = value + + @property + def no_proxy(self): + return self['no_proxy'] + + @no_proxy.setter + def no_proxy(self, value): + self['no_proxy'] = value @staticmethod def from_dict(config): @@ -22,10 +47,11 @@ def from_dict(config): https://docs.docker.com/network/proxy/#configure-the-docker-client ''' return ProxyConfig( - http=config.get('httpProxy', None), - https=config.get('httpsProxy', None), - ftp=config.get('ftpProxy', None), - no_proxy=config.get('noProxy', None)) + http=config.get('httpProxy'), + https=config.get('httpsProxy'), + ftp=config.get('ftpProxy'), + no_proxy=config.get('noProxy'), + ) def get_environment(self): ''' @@ -58,12 +84,6 @@ def inject_proxy_environment(self, environment): # variables defined in "environment" to take precedence. return proxy_env + environment - def __bool__(self): - return bool(self.http or self.https or self.ftp or self.no_proxy) - - def __nonzero__(self): - return self.__bool__() - def __str__(self): return 'ProxyConfig(http={}, https={}, ftp={}, no_proxy={})'.format( self.http, self.https, self.ftp, self.no_proxy) From 73c17f85e5844accc401dc5ca4ae879bafb7f9e2 Mon Sep 17 00:00:00 2001 From: Corentin Henry Date: Sat, 15 Dec 2018 14:28:01 +0100 Subject: [PATCH 62/68] fix typo in docstring Signed-off-by: Corentin Henry --- docker/utils/proxy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker/utils/proxy.py b/docker/utils/proxy.py index 943c25b02f..1c4fb248ef 100644 --- a/docker/utils/proxy.py +++ b/docker/utils/proxy.py @@ -72,7 +72,7 @@ def get_environment(self): def inject_proxy_environment(self, environment): ''' Given a list of strings representing environment variables, prepend the - environemt variables corresponding to the proxy settings. + environment variables corresponding to the proxy settings. ''' if not self: return environment From 2c4a8651a82016bd422e09688542872d3e236958 Mon Sep 17 00:00:00 2001 From: Corentin Henry Date: Sat, 15 Dec 2018 14:32:10 +0100 Subject: [PATCH 63/68] By default, disable proxy support This is to avoid a breaking change Signed-off-by: Corentin Henry --- docker/api/build.py | 14 ++++++++++---- docker/api/container.py | 11 +++++++++-- docker/api/exec_api.py | 11 +++++++++-- docker/models/containers.py | 9 +++++++-- docker/utils/proxy.py | 8 ++++---- tests/integration/api_exec_test.py | 7 ++++--- tests/unit/models_containers_test.py | 4 ++-- 7 files changed, 45 insertions(+), 19 deletions(-) diff --git a/docker/api/build.py b/docker/api/build.py index dab06d5d57..53c94b0dcf 100644 --- a/docker/api/build.py +++ b/docker/api/build.py @@ -19,7 +19,8 @@ def build(self, path=None, tag=None, quiet=False, fileobj=None, forcerm=False, dockerfile=None, container_limits=None, decode=False, buildargs=None, gzip=False, shmsize=None, labels=None, cache_from=None, target=None, network_mode=None, - squash=None, extra_hosts=None, platform=None, isolation=None): + squash=None, extra_hosts=None, platform=None, isolation=None, + use_config_proxy=False): """ Similar to the ``docker build`` command. Either ``path`` or ``fileobj`` needs to be set. ``path`` can be a local path (to a directory @@ -103,6 +104,10 @@ def build(self, path=None, tag=None, quiet=False, fileobj=None, platform (str): Platform in the format ``os[/arch[/variant]]`` isolation (str): Isolation technology used during build. Default: `None`. + use_config_proxy (bool): If ``True``, and if the docker client + configuration file (``~/.docker/config.json`` by default) + contains a proxy configuration, the corresponding environment + variables will be set in the container being built. Returns: A generator for the build output. @@ -168,9 +173,10 @@ def build(self, path=None, tag=None, quiet=False, fileobj=None, } params.update(container_limits) - proxy_args = self._proxy_configs.get_environment() - for k, v in proxy_args.items(): - buildargs.setdefault(k, v) + if use_config_proxy: + proxy_args = self._proxy_configs.get_environment() + for k, v in proxy_args.items(): + buildargs.setdefault(k, v) if buildargs: params.update({'buildargs': json.dumps(buildargs)}) diff --git a/docker/api/container.py b/docker/api/container.py index 2a80ff7dc7..54f9950ffd 100644 --- a/docker/api/container.py +++ b/docker/api/container.py @@ -221,7 +221,8 @@ def create_container(self, image, command=None, hostname=None, user=None, working_dir=None, domainname=None, host_config=None, mac_address=None, labels=None, stop_signal=None, networking_config=None, healthcheck=None, - stop_timeout=None, runtime=None): + stop_timeout=None, runtime=None, + use_config_proxy=False): """ Creates a container. Parameters are similar to those for the ``docker run`` command except it doesn't support the attach options (``-a``). @@ -390,6 +391,10 @@ def create_container(self, image, command=None, hostname=None, user=None, runtime (str): Runtime to use with this container. healthcheck (dict): Specify a test to perform to check that the container is healthy. + use_config_proxy (bool): If ``True``, and if the docker client + configuration file (``~/.docker/config.json`` by default) + contains a proxy configuration, the corresponding environment + variables will be set in the container being created. Returns: A dictionary with an image 'Id' key and a 'Warnings' key. @@ -405,7 +410,9 @@ def create_container(self, image, command=None, hostname=None, user=None, if isinstance(environment, dict): environment = utils.utils.format_environment(environment) - environment = self._proxy_configs.inject_proxy_environment(environment) + if use_config_proxy: + environment = \ + self._proxy_configs.inject_proxy_environment(environment) config = self.create_container_config( image, command, hostname, user, detach, stdin_open, tty, diff --git a/docker/api/exec_api.py b/docker/api/exec_api.py index 0dabdd3078..830432a72c 100644 --- a/docker/api/exec_api.py +++ b/docker/api/exec_api.py @@ -8,7 +8,8 @@ class ExecApiMixin(object): @utils.check_resource('container') def exec_create(self, container, cmd, stdout=True, stderr=True, stdin=False, tty=False, privileged=False, user='', - environment=None, workdir=None, detach_keys=None): + environment=None, workdir=None, detach_keys=None, + use_config_proxy=False): """ Sets up an exec instance in a running container. @@ -31,6 +32,10 @@ def exec_create(self, container, cmd, stdout=True, stderr=True, or `ctrl-` where `` is one of: `a-z`, `@`, `^`, `[`, `,` or `_`. ~/.docker/config.json is used by default. + use_config_proxy (bool): If ``True``, and if the docker client + configuration file (``~/.docker/config.json`` by default) + contains a proxy configuration, the corresponding environment + variables will be set in the container being created. Returns: (dict): A dictionary with an exec ``Id`` key. @@ -50,7 +55,9 @@ def exec_create(self, container, cmd, stdout=True, stderr=True, if isinstance(environment, dict): environment = utils.utils.format_environment(environment) - environment = self._proxy_configs.inject_proxy_environment(environment) + if use_config_proxy: + environment = \ + self._proxy_configs.inject_proxy_environment(environment) data = { 'Container': container, diff --git a/docker/models/containers.py b/docker/models/containers.py index 41bc4da859..817d5db5f5 100644 --- a/docker/models/containers.py +++ b/docker/models/containers.py @@ -144,7 +144,8 @@ def diff(self): def exec_run(self, cmd, stdout=True, stderr=True, stdin=False, tty=False, privileged=False, user='', detach=False, stream=False, - socket=False, environment=None, workdir=None, demux=False): + socket=False, environment=None, workdir=None, demux=False, + use_config_proxy=False): """ Run a command inside this container. Similar to ``docker exec``. @@ -167,6 +168,10 @@ def exec_run(self, cmd, stdout=True, stderr=True, stdin=False, tty=False, ``{"PASSWORD": "xxx"}``. workdir (str): Path to working directory for this exec session demux (bool): Return stdout and stderr separately + use_config_proxy (bool): If ``True``, and if the docker client + configuration file (``~/.docker/config.json`` by default) + contains a proxy configuration, the corresponding environment + variables will be set in the command's environment. Returns: (ExecResult): A tuple of (exit_code, output) @@ -185,7 +190,7 @@ def exec_run(self, cmd, stdout=True, stderr=True, stdin=False, tty=False, resp = self.client.api.exec_create( self.id, cmd, stdout=stdout, stderr=stderr, stdin=stdin, tty=tty, privileged=privileged, user=user, environment=environment, - workdir=workdir + workdir=workdir, use_config_proxy=use_config_proxy, ) exec_output = self.client.api.exec_start( resp['Id'], detach=detach, tty=tty, stream=stream, socket=socket, diff --git a/docker/utils/proxy.py b/docker/utils/proxy.py index 1c4fb248ef..c368116eab 100644 --- a/docker/utils/proxy.py +++ b/docker/utils/proxy.py @@ -7,7 +7,7 @@ class ProxyConfig(dict): ''' @property def http(self): - return self['http'] + return self.get('http') @http.setter def http(self, value): @@ -15,7 +15,7 @@ def http(self, value): @property def https(self): - return self['https'] + return self.get('https') @https.setter def https(self, value): @@ -23,7 +23,7 @@ def https(self, value): @property def ftp(self): - return self['ftp'] + return self.get('ftp') @ftp.setter def ftp(self, value): @@ -31,7 +31,7 @@ def ftp(self, value): @property def no_proxy(self): - return self['no_proxy'] + return self.get('no_proxy') @no_proxy.setter def no_proxy(self, value): diff --git a/tests/integration/api_exec_test.py b/tests/integration/api_exec_test.py index 6d4f97daab..8947b41328 100644 --- a/tests/integration/api_exec_test.py +++ b/tests/integration/api_exec_test.py @@ -20,10 +20,11 @@ def test_execute_proxy_env(self): self.client.start(id) self.tmp_containers.append(id) + cmd = 'sh -c "env | grep -i proxy"' + # First, just make sure the environment variables from the custom # config are set - res = self.client.exec_create( - id, cmd='sh -c "env | grep -i proxy"') + res = self.client.exec_create(id, cmd=cmd, use_config_proxy=True) output = self.client.exec_start(res).decode('utf-8').split('\n') expected = [ 'ftp_proxy=a', 'https_proxy=b', 'http_proxy=c', 'no_proxy=d', @@ -34,7 +35,7 @@ def test_execute_proxy_env(self): # Overwrite some variables with a custom environment env = {'https_proxy': 'xxx', 'HTTPS_PROXY': 'XXX'} res = self.client.exec_create( - id, cmd='sh -c "env | grep -i proxy"', environment=env) + id, cmd=cmd, environment=env, use_config_proxy=True) output = self.client.exec_start(res).decode('utf-8').split('\n') expected = [ 'ftp_proxy=a', 'https_proxy=xxx', 'http_proxy=c', 'no_proxy=d', diff --git a/tests/unit/models_containers_test.py b/tests/unit/models_containers_test.py index cb92c62be0..b35aeb6a38 100644 --- a/tests/unit/models_containers_test.py +++ b/tests/unit/models_containers_test.py @@ -416,7 +416,7 @@ def test_exec_run(self): client.api.exec_create.assert_called_with( FAKE_CONTAINER_ID, "echo hello world", stdout=True, stderr=True, stdin=False, tty=False, privileged=True, user='', environment=None, - workdir=None + workdir=None, use_config_proxy=False, ) client.api.exec_start.assert_called_with( FAKE_EXEC_ID, detach=False, tty=False, stream=True, socket=False, @@ -430,7 +430,7 @@ def test_exec_run_failure(self): client.api.exec_create.assert_called_with( FAKE_CONTAINER_ID, "docker ps", stdout=True, stderr=True, stdin=False, tty=False, privileged=True, user='', environment=None, - workdir=None + workdir=None, use_config_proxy=False, ) client.api.exec_start.assert_called_with( FAKE_EXEC_ID, detach=False, tty=False, stream=False, socket=False, From 6969e8becde593ee3dc1d3093d9299502ccc10ed Mon Sep 17 00:00:00 2001 From: Corentin Henry Date: Mon, 17 Dec 2018 10:44:37 +0100 Subject: [PATCH 64/68] handle url-based proxy configurations Signed-off-by: Corentin Henry --- docker/api/client.py | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/docker/api/client.py b/docker/api/client.py index b12398e5ed..668dfeef86 100644 --- a/docker/api/client.py +++ b/docker/api/client.py @@ -115,10 +115,13 @@ def __init__(self, base_url=None, version=None, self.headers['User-Agent'] = user_agent self._general_configs = config.load_general_config() + + proxy_config = self._general_configs.get('proxies', {}) try: - proxies = self._general_configs['proxies']['default'] + proxies = proxy_config[base_url] except KeyError: - proxies = {} + proxies = proxy_config.get('default', {}) + self._proxy_configs = ProxyConfig.from_dict(proxies) self._auth_configs = auth.load_config( From 65bebc085ab8bb36e5d6de44399b8480e7ae3e68 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Wed, 9 Jan 2019 10:52:50 -0800 Subject: [PATCH 65/68] Style fixes and removed some unused code Signed-off-by: Joffrey F --- docker/api/container.py | 6 ++++-- docker/utils/proxy.py | 16 ---------------- tests/integration/api_build_test.py | 11 ++++++++--- tests/integration/api_exec_test.py | 27 ++++++++++++++++----------- 4 files changed, 28 insertions(+), 32 deletions(-) diff --git a/docker/api/container.py b/docker/api/container.py index 54f9950ffd..43ae5320ff 100644 --- a/docker/api/container.py +++ b/docker/api/container.py @@ -410,9 +410,11 @@ def create_container(self, image, command=None, hostname=None, user=None, if isinstance(environment, dict): environment = utils.utils.format_environment(environment) + if use_config_proxy: - environment = \ - self._proxy_configs.inject_proxy_environment(environment) + environment = self._proxy_configs.inject_proxy_environment( + environment + ) config = self.create_container_config( image, command, hostname, user, detach, stdin_open, tty, diff --git a/docker/utils/proxy.py b/docker/utils/proxy.py index c368116eab..49e98ed912 100644 --- a/docker/utils/proxy.py +++ b/docker/utils/proxy.py @@ -9,34 +9,18 @@ class ProxyConfig(dict): def http(self): return self.get('http') - @http.setter - def http(self, value): - self['http'] = value - @property def https(self): return self.get('https') - @https.setter - def https(self, value): - self['https'] = value - @property def ftp(self): return self.get('ftp') - @ftp.setter - def ftp(self, value): - self['ftp'] = value - @property def no_proxy(self): return self.get('no_proxy') - @no_proxy.setter - def no_proxy(self, value): - self['no_proxy'] = value - @staticmethod def from_dict(config): ''' diff --git a/tests/integration/api_build_test.py b/tests/integration/api_build_test.py index 3c4e982048..8bfc7960fc 100644 --- a/tests/integration/api_build_test.py +++ b/tests/integration/api_build_test.py @@ -16,7 +16,8 @@ class BuildTest(BaseAPIIntegrationTest): def test_build_with_proxy(self): self.client._proxy_configs = ProxyConfig( - ftp='a', http='b', https='c', no_proxy='d') + ftp='a', http='b', https='c', no_proxy='d' + ) script = io.BytesIO('\n'.join([ 'FROM busybox', @@ -29,11 +30,13 @@ def test_build_with_proxy(self): 'RUN env | grep "NO_PROXY=d"', 'RUN env | grep "no_proxy=d"', ]).encode('ascii')) + self.client.build(fileobj=script, decode=True) def test_build_with_proxy_and_buildargs(self): self.client._proxy_configs = ProxyConfig( - ftp='a', http='b', https='c', no_proxy='d') + ftp='a', http='b', https='c', no_proxy='d' + ) script = io.BytesIO('\n'.join([ 'FROM busybox', @@ -46,10 +49,12 @@ def test_build_with_proxy_and_buildargs(self): 'RUN env | grep "NO_PROXY=d"', 'RUN env | grep "no_proxy=d"', ]).encode('ascii')) + self.client.build( fileobj=script, decode=True, - buildargs={'FTP_PROXY': 'XXX', 'ftp_proxy': 'xxx'}) + buildargs={'FTP_PROXY': 'XXX', 'ftp_proxy': 'xxx'} + ) def test_build_streaming(self): script = io.BytesIO('\n'.join([ diff --git a/tests/integration/api_exec_test.py b/tests/integration/api_exec_test.py index 8947b41328..e6079eb337 100644 --- a/tests/integration/api_exec_test.py +++ b/tests/integration/api_exec_test.py @@ -9,37 +9,42 @@ class ExecTest(BaseAPIIntegrationTest): - def test_execute_proxy_env(self): + def test_execute_command_with_proxy_env(self): # Set a custom proxy config on the client self.client._proxy_configs = ProxyConfig( - ftp='a', https='b', http='c', no_proxy='d') + ftp='a', https='b', http='c', no_proxy='d' + ) container = self.client.create_container( - BUSYBOX, 'cat', detach=True, stdin_open=True) - id = container['Id'] - self.client.start(id) - self.tmp_containers.append(id) + BUSYBOX, 'cat', detach=True, stdin_open=True, + use_config_proxy=True, + ) + self.client.start(container) + self.tmp_containers.append(container) cmd = 'sh -c "env | grep -i proxy"' # First, just make sure the environment variables from the custom # config are set - res = self.client.exec_create(id, cmd=cmd, use_config_proxy=True) + + res = self.client.exec_create(container, cmd=cmd) output = self.client.exec_start(res).decode('utf-8').split('\n') expected = [ 'ftp_proxy=a', 'https_proxy=b', 'http_proxy=c', 'no_proxy=d', - 'FTP_PROXY=a', 'HTTPS_PROXY=b', 'HTTP_PROXY=c', 'NO_PROXY=d'] + 'FTP_PROXY=a', 'HTTPS_PROXY=b', 'HTTP_PROXY=c', 'NO_PROXY=d' + ] for item in expected: assert item in output # Overwrite some variables with a custom environment env = {'https_proxy': 'xxx', 'HTTPS_PROXY': 'XXX'} - res = self.client.exec_create( - id, cmd=cmd, environment=env, use_config_proxy=True) + + res = self.client.exec_create(container, cmd=cmd, environment=env) output = self.client.exec_start(res).decode('utf-8').split('\n') expected = [ 'ftp_proxy=a', 'https_proxy=xxx', 'http_proxy=c', 'no_proxy=d', - 'FTP_PROXY=a', 'HTTPS_PROXY=XXX', 'HTTP_PROXY=c', 'NO_PROXY=d'] + 'FTP_PROXY=a', 'HTTPS_PROXY=XXX', 'HTTP_PROXY=c', 'NO_PROXY=d' + ] for item in expected: assert item in output From 219c52141e3cd15db3348c5420220f640323499f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 9 Jan 2019 00:35:03 +0100 Subject: [PATCH 66/68] Regression 443 test: relax status-code check This test was testing for a 500 status, but this status is actually a bug in the API (as it's due to an invalid request), and the API should actually return a 400 status. To make this test handle both situations, relax the test to accept either a 4xx or 5xx status. Signed-off-by: Sebastiaan van Stijn --- docker/errors.py | 3 +++ tests/integration/regression_test.py | 2 +- tests/unit/errors_test.py | 21 +++++++++++++++++++++ 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/docker/errors.py b/docker/errors.py index 0253695a5f..c340dcb123 100644 --- a/docker/errors.py +++ b/docker/errors.py @@ -63,6 +63,9 @@ def status_code(self): if self.response is not None: return self.response.status_code + def is_error(self): + return self.is_client_error() or self.is_server_error() + def is_client_error(self): if self.status_code is None: return False diff --git a/tests/integration/regression_test.py b/tests/integration/regression_test.py index 0fd4e43104..9aab076e30 100644 --- a/tests/integration/regression_test.py +++ b/tests/integration/regression_test.py @@ -14,7 +14,7 @@ def test_443_handle_nonchunked_response_in_stream(self): with pytest.raises(docker.errors.APIError) as exc: for line in self.client.build(fileobj=dfile, tag="a/b/c"): pass - assert exc.value.response.status_code == 500 + assert exc.value.is_error() dfile.close() def test_542_truncate_ids_client_side(self): diff --git a/tests/unit/errors_test.py b/tests/unit/errors_test.py index e27a9b1975..2134f86f04 100644 --- a/tests/unit/errors_test.py +++ b/tests/unit/errors_test.py @@ -79,6 +79,27 @@ def test_is_client_error_400(self): err = APIError('', response=resp) assert err.is_client_error() is True + def test_is_error_300(self): + """Report no error on 300 response.""" + resp = requests.Response() + resp.status_code = 300 + err = APIError('', response=resp) + assert err.is_error() is False + + def test_is_error_400(self): + """Report error on 400 response.""" + resp = requests.Response() + resp.status_code = 400 + err = APIError('', response=resp) + assert err.is_error() is True + + def test_is_error_500(self): + """Report error on 500 response.""" + resp = requests.Response() + resp.status_code = 500 + err = APIError('', response=resp) + assert err.is_error() is True + def test_create_error_from_exception(self): resp = requests.Response() resp.status_code = 500 From a579e9e20578ca9a074b28865ff594ed02fba6b3 Mon Sep 17 00:00:00 2001 From: Joffrey F Date: Wed, 9 Jan 2019 14:38:53 -0800 Subject: [PATCH 67/68] Remove use_config_proxy from exec. Add use_config_proxy docs to DockerClient Signed-off-by: Joffrey F --- Jenkinsfile | 2 +- docker/api/exec_api.py | 10 +--------- docker/models/containers.py | 16 ++++++++-------- docker/models/images.py | 4 ++++ tests/integration/models_containers_test.py | 13 +++++++++++++ tests/unit/models_containers_test.py | 4 ++-- 6 files changed, 29 insertions(+), 20 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 33a0fc3136..8724c10fb3 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -91,7 +91,7 @@ def runTests = { Map settings -> --network ${testNetwork} \\ --volumes-from ${dindContainerName} \\ ${testImage} \\ - py.test -v -rxs tests/integration + py.test -v -rxs --cov=docker tests/ """ } finally { sh """ diff --git a/docker/api/exec_api.py b/docker/api/exec_api.py index 830432a72c..d13b128998 100644 --- a/docker/api/exec_api.py +++ b/docker/api/exec_api.py @@ -8,8 +8,7 @@ class ExecApiMixin(object): @utils.check_resource('container') def exec_create(self, container, cmd, stdout=True, stderr=True, stdin=False, tty=False, privileged=False, user='', - environment=None, workdir=None, detach_keys=None, - use_config_proxy=False): + environment=None, workdir=None, detach_keys=None): """ Sets up an exec instance in a running container. @@ -32,10 +31,6 @@ def exec_create(self, container, cmd, stdout=True, stderr=True, or `ctrl-` where `` is one of: `a-z`, `@`, `^`, `[`, `,` or `_`. ~/.docker/config.json is used by default. - use_config_proxy (bool): If ``True``, and if the docker client - configuration file (``~/.docker/config.json`` by default) - contains a proxy configuration, the corresponding environment - variables will be set in the container being created. Returns: (dict): A dictionary with an exec ``Id`` key. @@ -55,9 +50,6 @@ def exec_create(self, container, cmd, stdout=True, stderr=True, if isinstance(environment, dict): environment = utils.utils.format_environment(environment) - if use_config_proxy: - environment = \ - self._proxy_configs.inject_proxy_environment(environment) data = { 'Container': container, diff --git a/docker/models/containers.py b/docker/models/containers.py index 817d5db5f5..10f667d706 100644 --- a/docker/models/containers.py +++ b/docker/models/containers.py @@ -144,8 +144,7 @@ def diff(self): def exec_run(self, cmd, stdout=True, stderr=True, stdin=False, tty=False, privileged=False, user='', detach=False, stream=False, - socket=False, environment=None, workdir=None, demux=False, - use_config_proxy=False): + socket=False, environment=None, workdir=None, demux=False): """ Run a command inside this container. Similar to ``docker exec``. @@ -168,10 +167,6 @@ def exec_run(self, cmd, stdout=True, stderr=True, stdin=False, tty=False, ``{"PASSWORD": "xxx"}``. workdir (str): Path to working directory for this exec session demux (bool): Return stdout and stderr separately - use_config_proxy (bool): If ``True``, and if the docker client - configuration file (``~/.docker/config.json`` by default) - contains a proxy configuration, the corresponding environment - variables will be set in the command's environment. Returns: (ExecResult): A tuple of (exit_code, output) @@ -190,7 +185,7 @@ def exec_run(self, cmd, stdout=True, stderr=True, stdin=False, tty=False, resp = self.client.api.exec_create( self.id, cmd, stdout=stdout, stderr=stderr, stdin=stdin, tty=tty, privileged=privileged, user=user, environment=environment, - workdir=workdir, use_config_proxy=use_config_proxy, + workdir=workdir, ) exec_output = self.client.api.exec_start( resp['Id'], detach=detach, tty=tty, stream=stream, socket=socket, @@ -682,6 +677,7 @@ def run(self, image, command=None, stdout=True, stderr=False, For example: ``{"Name": "on-failure", "MaximumRetryCount": 5}`` + runtime (str): Runtime to use with this container. security_opt (:py:class:`list`): A list of string values to customize labels for MLS systems, such as SELinux. shm_size (str or int): Size of /dev/shm (e.g. ``1G``). @@ -713,6 +709,10 @@ def run(self, image, command=None, stdout=True, stderr=False, tty (bool): Allocate a pseudo-TTY. ulimits (:py:class:`list`): Ulimits to set inside the container, as a list of :py:class:`docker.types.Ulimit` instances. + use_config_proxy (bool): If ``True``, and if the docker client + configuration file (``~/.docker/config.json`` by default) + contains a proxy configuration, the corresponding environment + variables will be set in the container being built. user (str or int): Username or UID to run commands as inside the container. userns_mode (str): Sets the user namespace mode for the container @@ -737,7 +737,6 @@ def run(self, image, command=None, stdout=True, stderr=False, volumes_from (:py:class:`list`): List of container names or IDs to get volumes from. working_dir (str): Path to the working directory. - runtime (str): Runtime to use with this container. Returns: The container logs, either ``STDOUT``, ``STDERR``, or both, @@ -952,6 +951,7 @@ def prune(self, filters=None): 'stdin_open', 'stop_signal', 'tty', + 'use_config_proxy', 'user', 'volume_driver', 'working_dir', diff --git a/docker/models/images.py b/docker/models/images.py index 30e86f109e..af94520d9b 100644 --- a/docker/models/images.py +++ b/docker/models/images.py @@ -258,6 +258,10 @@ def build(self, **kwargs): platform (str): Platform in the format ``os[/arch[/variant]]``. isolation (str): Isolation technology used during build. Default: `None`. + use_config_proxy (bool): If ``True``, and if the docker client + configuration file (``~/.docker/config.json`` by default) + contains a proxy configuration, the corresponding environment + variables will be set in the container being built. Returns: (tuple): The first item is the :py:class:`Image` object for the diff --git a/tests/integration/models_containers_test.py b/tests/integration/models_containers_test.py index b48f6fb6ce..92eca36d1a 100644 --- a/tests/integration/models_containers_test.py +++ b/tests/integration/models_containers_test.py @@ -163,6 +163,19 @@ def test_run_with_streamed_logs_and_cancel(self): assert logs[0] == b'hello\n' assert logs[1] == b'world\n' + def test_run_with_proxy_config(self): + client = docker.from_env(version=TEST_API_VERSION) + client.api._proxy_configs = docker.utils.proxy.ProxyConfig( + ftp='sakuya.jp:4967' + ) + + out = client.containers.run( + 'alpine', 'sh -c "env"', use_config_proxy=True + ) + + assert b'FTP_PROXY=sakuya.jp:4967\n' in out + assert b'ftp_proxy=sakuya.jp:4967\n' in out + def test_get(self): client = docker.from_env(version=TEST_API_VERSION) container = client.containers.run("alpine", "sleep 300", detach=True) diff --git a/tests/unit/models_containers_test.py b/tests/unit/models_containers_test.py index b35aeb6a38..f44e365851 100644 --- a/tests/unit/models_containers_test.py +++ b/tests/unit/models_containers_test.py @@ -416,7 +416,7 @@ def test_exec_run(self): client.api.exec_create.assert_called_with( FAKE_CONTAINER_ID, "echo hello world", stdout=True, stderr=True, stdin=False, tty=False, privileged=True, user='', environment=None, - workdir=None, use_config_proxy=False, + workdir=None, ) client.api.exec_start.assert_called_with( FAKE_EXEC_ID, detach=False, tty=False, stream=True, socket=False, @@ -430,7 +430,7 @@ def test_exec_run_failure(self): client.api.exec_create.assert_called_with( FAKE_CONTAINER_ID, "docker ps", stdout=True, stderr=True, stdin=False, tty=False, privileged=True, user='', environment=None, - workdir=None, use_config_proxy=False, + workdir=None, ) client.api.exec_start.assert_called_with( FAKE_EXEC_ID, detach=False, tty=False, stream=False, socket=False, From e6783d8cf33d0bd2654da01904b986a4af134daa Mon Sep 17 00:00:00 2001 From: Ulysses Souza Date: Thu, 10 Jan 2019 17:58:39 +0100 Subject: [PATCH 68/68] Bump 3.7.0 Signed-off-by: Ulysses Souza --- docker/version.py | 2 +- docs/change-log.md | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/docker/version.py b/docker/version.py index b4cf22bbb2..c3edb8a35e 100644 --- a/docker/version.py +++ b/docker/version.py @@ -1,2 +1,2 @@ -version = "3.7.0-dev" +version = "3.7.0" version_info = tuple([int(d) for d in version.split("-")[0].split(".")]) diff --git a/docs/change-log.md b/docs/change-log.md index 873db8cef5..008a2ad270 100644 --- a/docs/change-log.md +++ b/docs/change-log.md @@ -1,6 +1,28 @@ Change log ========== +3.7.0 +----- + +[List of PRs / issues for this release](https://github.com/docker/docker-py/milestone/56?closed=1) + +### Features + +* Added support for multiplexed streams (for `attach` and `exec_start`). Learn + more at https://docker-py.readthedocs.io/en/stable/user_guides/multiplex.html +* Added the `use_config_proxy` parameter to the following methods: + `APIClient.build`, `APIClient.create_container`, `DockerClient.images.build` + and `DockerClient.containers.run` (`False` by default). **This parameter** + **will become `True` by default in the 4.0.0 release.** +* Placement preferences for Swarm services are better validated on the client + and documentation has been updated accordingly + +### Bugfixes + +* Fixed a bug where credential stores weren't queried for relevant registry + credentials with certain variations of the `config.json` file. +* `DockerClient.swarm.init` now returns a boolean value as advertised. + 3.6.0 -----