Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Vendored Docker SDK for Python updates #434

Merged
merged 10 commits into from
Jul 31, 2022
11 changes: 11 additions & 0 deletions changelogs/fragments/docker-py-changes-1.yml
Original file line number Diff line number Diff line change
@@ -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)."
4 changes: 2 additions & 2 deletions plugins/module_utils/_api/api/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
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
Expand Down Expand Up @@ -258,7 +258,7 @@ def _raise_for_status(self, response):
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:
Expand Down
4 changes: 3 additions & 1 deletion plugins/module_utils/_api/errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@

from ._import_helper import HTTPError as _HTTPError

from ansible.module_utils.six import raise_from


class DockerException(Exception):
"""
Expand Down Expand Up @@ -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):
Expand Down
10 changes: 4 additions & 6 deletions plugins/module_utils/_api/tls.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@

import os
import ssl
import sys

from . import errors
from .transport.ssladapter import SSLHTTPAdapter
Expand Down Expand Up @@ -49,15 +50,10 @@ def __init__(self, client_cert=None, ca_cert=None, verify=None,
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,
Expand All @@ -73,6 +69,8 @@ def __init__(self, client_cert=None, ca_cert=None, verify=None,
# 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
Expand Down
7 changes: 3 additions & 4 deletions plugins/module_utils/_api/transport/sshconn.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,9 +78,8 @@ def f():
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)
Expand Down Expand Up @@ -225,7 +224,7 @@ def _create_paramiko_client(self, base_url):
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']
Expand All @@ -237,7 +236,7 @@ def _create_paramiko_client(self, base_url):
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:
Expand Down
3 changes: 3 additions & 0 deletions plugins/module_utils/_api/utils/build.py
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,9 @@ def __init__(self, pattern_str):
@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
Expand Down
41 changes: 25 additions & 16 deletions plugins/module_utils/_api/utils/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
__metaclass__ = type

import base64
import collections
import json
import os
import os.path
Expand All @@ -22,17 +23,22 @@
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):
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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':
Expand All @@ -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):
Expand Down Expand Up @@ -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'),
Expand Down
4 changes: 2 additions & 2 deletions tests/unit/plugins/module_utils/_api/api/test_client.py
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ def setUp(self):
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
Expand Down Expand Up @@ -519,7 +519,7 @@ 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.daemon = True
cls.thread.start()
cls.address = 'http://{0}:{1}'.format(
socket.gethostname(), cls.server.server_address[1])
Expand Down
11 changes: 9 additions & 2 deletions tests/unit/plugins/module_utils/_api/utils/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -285,17 +285,24 @@ def test_parse_host(self):
'[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'
Expand Down