diff --git a/changelogs/fragments/docker-py-changes-1.yml b/changelogs/fragments/docker-py-changes-1.yml new file mode 100644 index 00000000..3f9f2ba8 --- /dev/null +++ b/changelogs/fragments/docker-py-changes-1.yml @@ -0,0 +1,11 @@ +bugfixes: + - "modules and plugins communicating directly with the Docker daemon - fix parsing of IPv6 addresses with a port in ``docker_host``. This is only a change relative to older community.docker 3.0.0 pre-releases or with respect to Docker SDK for Python < 6.0.0. Docker SDK for Python 6.0.0 will also include this change (https://github.com/ansible-collections/community.docker/pull/434)." + - "modules and plugins communicating directly with the Docker daemon - fix ``ProxyCommand`` handling for SSH connections when not using ``use_ssh_client=true``. This is only a change relative to older community.docker 3.0.0 pre-releases or with respect to Docker SDK for Python < 6.0.0. Docker SDK for Python 6.0.0 will also include this change (https://github.com/ansible-collections/community.docker/pull/434)." + - "modules and plugins communicating directly with the Docker daemon - do not create a subshell for SSH connections when using ``use_ssh_client=true``. This is only a change relative to older community.docker 3.0.0 pre-releases or with respect to Docker SDK for Python < 6.0.0. Docker SDK for Python 6.0.0 will also include this change (https://github.com/ansible-collections/community.docker/pull/434)." + - "docker_image - when composing the build context, trim trailing whitespace from ``.dockerignore`` entries. This is only a change relative to older community.docker 3.0.0 pre-releases or with respect to Docker SDK for Python < 6.0.0. Docker SDK for Python 6.0.0 will also include this change (https://github.com/ansible-collections/community.docker/pull/434)." +minor_changes: + - "modules and plugins communicating directly with the Docker daemon - improve default TLS version selection for Python 3.6 and newer. This is only a change relative to older community.docker 3.0.0 pre-releases or with respect to Docker SDK for Python < 6.0.0. Docker SDK for Python 6.0.0 will also include this change (https://github.com/ansible-collections/community.docker/pull/434)." +security_fixes: + - "modules and plugins communicating directly with the Docker daemon - when connecting by SSH and not using ``use_ssh_client=true``, reject unknown host keys instead of accepting them. This is only a change relative to older community.docker 3.0.0 pre-releases or with respect to Docker SDK for Python < 6.0.0. Docker SDK for Python 6.0.0 will also include this change (https://github.com/ansible-collections/community.docker/pull/434)." +breaking_changes: + - "modules and plugins communicating directly with the Docker daemon - when connecting by SSH and not using ``use_ssh_client=true``, reject unknown host keys instead of accepting them. This is only a breaking change relative to older community.docker 3.0.0 pre-releases or with respect to Docker SDK for Python < 6.0.0. Docker SDK for Python 6.0.0 will also include this change (https://github.com/ansible-collections/community.docker/pull/434)." diff --git a/plugins/module_utils/_api/api/client.py b/plugins/module_utils/_api/api/client.py index 45ea7497..faf30cda 100644 --- a/plugins/module_utils/_api/api/client.py +++ b/plugins/module_utils/_api/api/client.py @@ -15,7 +15,7 @@ import logging import struct from functools import partial -from ansible.module_utils.six import PY3, binary_type, iteritems, string_types +from ansible.module_utils.six import PY3, binary_type, iteritems, string_types, raise_from from ansible.module_utils.six.moves.urllib.parse import quote from .. import auth @@ -258,7 +258,7 @@ class APIClient( try: response.raise_for_status() except _HTTPError as e: - raise create_api_error_from_http_exception(e) + raise_from(create_api_error_from_http_exception(e), e) def _result(self, response, json=False, binary=False): if json and binary: diff --git a/plugins/module_utils/_api/errors.py b/plugins/module_utils/_api/errors.py index 03948736..90dd5aad 100644 --- a/plugins/module_utils/_api/errors.py +++ b/plugins/module_utils/_api/errors.py @@ -12,6 +12,8 @@ __metaclass__ = type from ._import_helper import HTTPError as _HTTPError +from ansible.module_utils.six import raise_from + class DockerException(Exception): """ @@ -40,7 +42,7 @@ def create_api_error_from_http_exception(e): cls = ImageNotFound else: cls = NotFound - raise cls(e, response=response, explanation=explanation) + raise_from(cls(e, response=response, explanation=explanation), e) class APIError(_HTTPError, DockerException): diff --git a/plugins/module_utils/_api/tls.py b/plugins/module_utils/_api/tls.py index 33406b85..ed5416d8 100644 --- a/plugins/module_utils/_api/tls.py +++ b/plugins/module_utils/_api/tls.py @@ -12,6 +12,7 @@ __metaclass__ = type import os import ssl +import sys from . import errors from .transport.ssladapter import SSLHTTPAdapter @@ -49,15 +50,10 @@ class TLSConfig(object): self.assert_hostname = assert_hostname self.assert_fingerprint = assert_fingerprint - # TODO(dperny): according to the python docs, PROTOCOL_TLSvWhatever is - # depcreated, and it's recommended to use OPT_NO_TLSvWhatever instead - # to exclude versions. But I think that might require a bigger - # architectural change, so I've opted not to pursue it at this time - # If the user provides an SSL version, we should use their preference if ssl_version: self.ssl_version = ssl_version - else: + elif (sys.version_info.major, sys.version_info.minor) < (3, 6): # If the user provides no ssl version, we should default to # TLSv1_2. This option is the most secure, and will work for the # majority of users with reasonably up-to-date software. However, @@ -73,6 +69,8 @@ class TLSConfig(object): # SSLv23 fails in mysterious ways: # https://github.com/docker/docker-py/issues/963 self.ssl_version = ssl.PROTOCOL_TLSv1 + else: + self.ssl_version = ssl.PROTOCOL_TLS_CLIENT # "client_cert" must have both or neither cert/key files. In # either case, Alert the user when both are expected, but any are diff --git a/plugins/module_utils/_api/transport/sshconn.py b/plugins/module_utils/_api/transport/sshconn.py index 47dc35db..063c2882 100644 --- a/plugins/module_utils/_api/transport/sshconn.py +++ b/plugins/module_utils/_api/transport/sshconn.py @@ -78,9 +78,8 @@ class SSHSocket(socket.socket): env.pop('SSL_CERT_FILE', None) self.proc = subprocess.Popen( - ' '.join(args), + args, env=env, - shell=True, stdout=subprocess.PIPE, stdin=subprocess.PIPE, preexec_fn=preexec_func) @@ -225,7 +224,7 @@ class SSHHTTPAdapter(BaseHTTPAdapter): host_config = conf.lookup(base_url.hostname) if 'proxycommand' in host_config: self.ssh_params["sock"] = paramiko.ProxyCommand( - self.ssh_conf['proxycommand'] + host_config['proxycommand'] ) if 'hostname' in host_config: self.ssh_params['hostname'] = host_config['hostname'] @@ -237,7 +236,7 @@ class SSHHTTPAdapter(BaseHTTPAdapter): self.ssh_params['key_filename'] = host_config['identityfile'] self.ssh_client.load_system_host_keys() - self.ssh_client.set_missing_host_key_policy(paramiko.WarningPolicy()) + self.ssh_client.set_missing_host_key_policy(paramiko.RejectPolicy()) def _connect(self): if self.ssh_client: diff --git a/plugins/module_utils/_api/utils/build.py b/plugins/module_utils/_api/utils/build.py index 9b5d1874..85704f94 100644 --- a/plugins/module_utils/_api/utils/build.py +++ b/plugins/module_utils/_api/utils/build.py @@ -244,6 +244,9 @@ class Pattern(object): @classmethod def normalize(cls, p): + # Remove trailing spaces + p = p.strip() + # Leading and trailing slashes are not relevant. Yes, # "foo.py/" must exclude the "foo.py" regular file. "." # components are not relevant either, even if the whole diff --git a/plugins/module_utils/_api/utils/utils.py b/plugins/module_utils/_api/utils/utils.py index 6401fbab..910b0dc3 100644 --- a/plugins/module_utils/_api/utils/utils.py +++ b/plugins/module_utils/_api/utils/utils.py @@ -11,6 +11,7 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type import base64 +import collections import json import os import os.path @@ -22,17 +23,22 @@ from ansible_collections.community.docker.plugins.module_utils.version import St from ansible.module_utils.six import PY2, PY3, binary_type, integer_types, iteritems, string_types, text_type from .. import errors -from .. import tls from ..constants import DEFAULT_HTTP_HOST from ..constants import DEFAULT_UNIX_SOCKET from ..constants import DEFAULT_NPIPE from ..constants import BYTE_UNITS +from ..tls import TLSConfig if PY2: - from urllib import splitnport - from urlparse import urlparse + from urlparse import urlparse, urlunparse else: - from urllib.parse import splitnport, urlparse + from urllib.parse import urlparse, urlunparse + + +URLComponents = collections.namedtuple( + 'URLComponents', + 'scheme netloc url params query fragment', +) def create_ipam_pool(*args, **kwargs): @@ -220,10 +226,6 @@ def parse_repository_tag(repo_name): def parse_host(addr, is_win32=False, tls=False): - path = '' - port = None - host = None - # Sensible defaults if not addr and is_win32: return DEFAULT_NPIPE @@ -282,20 +284,20 @@ def parse_host(addr, is_win32=False, tls=False): # to be valid and equivalent to unix:///path path = '/'.join((parsed_url.hostname, path)) + netloc = parsed_url.netloc 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: + port = parsed_url.port or 0 + if port <= 0: if proto != 'ssh': raise errors.DockerException( 'Invalid bind address format: port is required:' ' {0}'.format(addr) ) port = 22 + netloc = '{0}:{1}'.format(parsed_url.netloc, port) - if not host: - host = DEFAULT_HTTP_HOST + if not parsed_url.hostname: + netloc = '{0}:{1}'.format(DEFAULT_HTTP_HOST, port) # Rewrite schemes to fit library internals (requests adapters) if proto == 'tcp': @@ -305,7 +307,14 @@ def parse_host(addr, is_win32=False, tls=False): if proto in ('http+unix', 'npipe'): return "{0}://{1}".format(proto, path).rstrip('/') - return '{0}://{1}:{2}{3}'.format(proto, host, port, path).rstrip('/') + return urlunparse(URLComponents( + scheme=proto, + netloc=netloc, + url=path, + params='', + query='', + fragment='', + )).rstrip('/') def parse_devices(devices): @@ -370,7 +379,7 @@ def kwargs_from_env(ssl_version=None, assert_hostname=None, environment=None): # so if it's not set already then set it to false. assert_hostname = False - params['tls'] = tls.TLSConfig( + params['tls'] = TLSConfig( client_cert=(os.path.join(cert_path, 'cert.pem'), os.path.join(cert_path, 'key.pem')), ca_cert=os.path.join(cert_path, 'ca.pem'), diff --git a/tests/unit/plugins/module_utils/_api/api/test_client.py b/tests/unit/plugins/module_utils/_api/api/test_client.py index c0a8ed39..ea003565 100644 --- a/tests/unit/plugins/module_utils/_api/api/test_client.py +++ b/tests/unit/plugins/module_utils/_api/api/test_client.py @@ -393,7 +393,7 @@ class UnixSocketStreamTest(unittest.TestCase): self.server_socket = self._setup_socket() self.stop_server = False server_thread = threading.Thread(target=self.run_server) - server_thread.setDaemon(True) + server_thread.daemon = True server_thread.start() self.response = None self.request_handler = None @@ -519,7 +519,7 @@ class TCPSocketStreamTest(unittest.TestCase): 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.daemon = True cls.thread.start() cls.address = 'http://{0}:{1}'.format( socket.gethostname(), cls.server.server_address[1]) diff --git a/tests/unit/plugins/module_utils/_api/utils/test_utils.py b/tests/unit/plugins/module_utils/_api/utils/test_utils.py index 7679ba43..cc0dc695 100644 --- a/tests/unit/plugins/module_utils/_api/utils/test_utils.py +++ b/tests/unit/plugins/module_utils/_api/utils/test_utils.py @@ -285,17 +285,24 @@ class ParseHostTest(unittest.TestCase): '[fd12::82d1]:2375/docker/engine': ( 'http://[fd12::82d1]:2375/docker/engine' ), + 'ssh://[fd12::82d1]': 'ssh://[fd12::82d1]:22', + 'ssh://user@[fd12::82d1]:8765': 'ssh://user@[fd12::82d1]:8765', '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: - with pytest.raises(DockerException): + msg = 'Should have failed to parse invalid host: {0}'.format(host) + with self.assertRaises(DockerException, msg=msg): parse_host(host, None) for host, expected in valid_hosts.items(): - assert parse_host(host, None) == expected + self.assertEqual( + parse_host(host, None), + expected, + msg='Failed to parse valid host: {0}'.format(host), + ) def test_parse_host_empty_value(self): unix_socket = 'http+unix:///var/run/docker.sock'