diff --git a/changelogs/fragments/297-docker-connection-config.yml b/changelogs/fragments/297-docker-connection-config.yml new file mode 100644 index 00000000..a0ae0e46 --- /dev/null +++ b/changelogs/fragments/297-docker-connection-config.yml @@ -0,0 +1,4 @@ +bugfixes: + - "docker connection plugin - fix option handling to be compatible with ansible-core 2.13 (https://github.com/ansible-collections/community.docker/pull/297, https://github.com/ansible-collections/community.docker/issues/307)." +minor_changes: + - "docker connection plugin - the plugin supports new ways to define the timeout. These are the ``ANSIBLE_DOCKER_TIMEOUT`` environment variable, the ``timeout`` setting in the ``docker_connection`` section of ``ansible.cfg``, and the ``ansible_docker_timeout`` variable (https://github.com/ansible-collections/community.docker/pull/297)." diff --git a/plugins/connection/docker.py b/plugins/connection/docker.py index 5ff89617..96d1b46f 100644 --- a/plugins/connection/docker.py +++ b/plugins/connection/docker.py @@ -22,23 +22,59 @@ DOCUMENTATION = ''' R(community.docker.docker_api,ansible_collections.community.docker.docker_api_connection) connection plugin. options: - remote_user: - description: - - The user to execute as inside the container - vars: - - name: ansible_user - - name: ansible_docker_user - docker_extra_args: - description: - - Extra arguments to pass to the docker command line - default: '' remote_addr: description: - The name of the container you want to access. default: inventory_hostname vars: + - name: inventory_hostname - name: ansible_host - name: ansible_docker_host + remote_user: + description: + - The user to execute as inside the container. + - If Docker is too old to allow this (< 1.7), the one set by Docker itself will be used. + vars: + - name: ansible_user + - name: ansible_docker_user + ini: + - section: defaults + key: remote_user + env: + - name: ANSIBLE_REMOTE_USER + cli: + - name: user + keyword: + - name: remote_user + docker_extra_args: + description: + - Extra arguments to pass to the docker command line. + default: '' + vars: + - name: ansible_docker_extra_args + ini: + - section: docker_connection + key: extra_cli_args + container_timeout: + default: 10 + description: + - Controls how long we can wait to access reading output from the container once execution started. + env: + - name: ANSIBLE_TIMEOUT + - name: ANSIBLE_DOCKER_TIMEOUT + version_added: 2.2.0 + ini: + - key: timeout + section: defaults + - key: timeout + section: docker_connection + version_added: 2.2.0 + vars: + - name: ansible_docker_timeout + version_added: 2.2.0 + cli: + - name: timeout + type: integer ''' import fcntl @@ -47,7 +83,6 @@ import os.path import subprocess import re -import ansible.constants as C from ansible.compat import selectors from ansible.errors import AnsibleError, AnsibleFileNotFound from ansible.module_utils.six.moves import shlex_quote @@ -79,6 +114,9 @@ class Connection(ConnectionBase): # configured to be connected to by root and they are not running as # root. + self._docker_args = [] + self._container_user_cache = {} + # Windows uses Powershell modules if getattr(self._shell, "_IS_WINDOWS", False): self.module_implementation_preferences = ('.ps1', '.exe', '') @@ -91,34 +129,12 @@ class Connection(ConnectionBase): except ValueError: raise AnsibleError("docker command not found in PATH") - docker_version = self._get_docker_version() - if docker_version == u'dev': + self.docker_version = self._get_docker_version() + if self.docker_version == u'dev': display.warning(u'Docker version number is "dev". Will assume latest version.') - if docker_version != u'dev' and LooseVersion(docker_version) < LooseVersion(u'1.3'): + if self.docker_version != u'dev' and LooseVersion(self.docker_version) < LooseVersion(u'1.3'): raise AnsibleError('docker connection type requires docker 1.3 or higher') - # The remote user we will request from docker (if supported) - self.remote_user = None - # The actual user which will execute commands in docker (if known) - self.actual_user = None - - if self._play_context.remote_user is not None: - if docker_version == u'dev' or LooseVersion(docker_version) >= LooseVersion(u'1.7'): - # Support for specifying the exec user was added in docker 1.7 - self.remote_user = self._play_context.remote_user - self.actual_user = self.remote_user - else: - self.actual_user = self._get_docker_remote_user() - - if self.actual_user != self._play_context.remote_user: - display.warning(u'docker {0} does not support remote_user, using container default: {1}' - .format(docker_version, self.actual_user or u'?')) - elif self._display.verbosity > 2: - # Since we're not setting the actual_user, look it up so we have it for logging later - # Only do this if display verbosity is high enough that we'll need the value - # This saves overhead from calling into docker when we don't need to - self.actual_user = self._get_docker_remote_user() - @staticmethod def _sanitize_version(version): version = re.sub(u'[^0-9a-zA-Z.]', u'', version) @@ -126,9 +142,7 @@ class Connection(ConnectionBase): return version def _old_docker_version(self): - cmd_args = [] - if self._play_context.docker_extra_args: - cmd_args += self._play_context.docker_extra_args.split(' ') + cmd_args = self._docker_args old_version_subcommand = ['version'] @@ -140,9 +154,7 @@ class Connection(ConnectionBase): def _new_docker_version(self): # no result yet, must be newer Docker version - cmd_args = [] - if self._play_context.docker_extra_args: - cmd_args += self._play_context.docker_extra_args.split(' ') + cmd_args = self._docker_args new_version_subcommand = ['version', '--format', "'{{.Server.Version}}'"] @@ -167,7 +179,10 @@ class Connection(ConnectionBase): def _get_docker_remote_user(self): """ Get the default user configured in the docker container """ - p = subprocess.Popen([self.docker_cmd, 'inspect', '--format', '{{.Config.User}}', self._play_context.remote_addr], + container = self.get_option('remote_addr') + if container in self._container_user_cache: + return self._container_user_cache[container] + p = subprocess.Popen([self.docker_cmd, 'inspect', '--format', '{{.Config.User}}', container], stdout=subprocess.PIPE, stderr=subprocess.PIPE) out, err = p.communicate() @@ -175,10 +190,13 @@ class Connection(ConnectionBase): if p.returncode != 0: display.warning(u'unable to retrieve default user from docker container: %s %s' % (out, to_text(err))) + self._container_user_cache[container] = None return None # The default exec user is root, unless it was changed in the Dockerfile with USER - return out.strip() or u'root' + user = out.strip() or u'root' + self._container_user_cache[container] = user + return user def _build_exec_cmd(self, cmd): """ Build the local docker exec command to run cmd on remote_host @@ -189,8 +207,8 @@ class Connection(ConnectionBase): local_cmd = [self.docker_cmd] - if self._play_context.docker_extra_args: - local_cmd += self._play_context.docker_extra_args.split(' ') + if self._docker_args: + local_cmd += self._docker_args local_cmd += [b'exec'] @@ -198,26 +216,68 @@ class Connection(ConnectionBase): local_cmd += [b'-u', self.remote_user] # -i is needed to keep stdin open which allows pipelining to work - local_cmd += [b'-i', self._play_context.remote_addr] + cmd + local_cmd += [b'-i', self.get_option('remote_addr')] + cmd return local_cmd + def _set_conn_data(self): + + ''' initialize for the connection, cannot do only in init since all data is not ready at that point ''' + + # TODO: this is mostly for backwards compatibility, play_context is used as fallback for older versions + # docker arguments + del self._docker_args[:] + extra_args = self.get_option('docker_extra_args') or self._play_context.docker_extra_args + if extra_args: + self._docker_args += extra_args.split(' ') + + self.remote_user = self.get_option('remote_user') + if self.remote_user is None and self._play_context.remote_user is not None: + self.remote_user = self._play_context.remote_user + # The actual user which will execute commands in docker (if known) + self.actual_user = None + + if self.remote_user is not None: + if self.docker_version == u'dev' or LooseVersion(self.docker_version) >= LooseVersion(u'1.7'): + # Support for specifying the exec user was added in docker 1.7 + self.actual_user = self.remote_user + else: + self.remote_user = None + self.actual_user = self._get_docker_remote_user() + if self.actual_user != self.get_option('remote_user'): + display.warning(u'docker {0} does not support remote_user, using container default: {1}' + .format(self.docker_version, self.actual_user or u'?')) + elif self._display.verbosity > 2: + # Since we're not setting the actual_user, look it up so we have it for logging later + # Only do this if display verbosity is high enough that we'll need the value + # This saves overhead from calling into docker when we don't need to + self.actual_user = self._get_docker_remote_user() + + # timeout, use unless default and pc is different, backwards compat + self.timeout = self.get_option('container_timeout') + if self.timeout == 10 and self.timeout != self._play_context.timeout: + self.timeout = self._play_context.timeout + def _connect(self, port=None): """ Connect to the container. Nothing to do """ super(Connection, self)._connect() if not self._connected: + self._set_conn_data() display.vvv(u"ESTABLISH DOCKER CONNECTION FOR USER: {0}".format( - self.actual_user or u'?'), host=self._play_context.remote_addr + self.actual_user or u'?'), host=self.get_option('remote_addr') ) self._connected = True def exec_command(self, cmd, in_data=None, sudoable=False): """ Run a command on the docker host """ + + self._set_conn_data() + super(Connection, self).exec_command(cmd, in_data=in_data, sudoable=sudoable) local_cmd = self._build_exec_cmd([self._play_context.executable, '-c', cmd]) - display.vvv(u"EXEC {0}".format(to_text(local_cmd)), host=self._play_context.remote_addr) + display.vvv(u"EXEC {0}".format(to_text(local_cmd)), host=self.get_option('remote_addr')) display.debug("opening command with Popen()") local_cmd = [to_bytes(i, errors='surrogate_or_strict') for i in local_cmd] @@ -240,7 +300,7 @@ class Connection(ConnectionBase): become_output = b'' try: while not self.become.check_success(become_output) and not self.become.check_password_prompt(become_output): - events = selector.select(self._play_context.timeout) + events = selector.select(self.timeout) if not events: stdout, stderr = p.communicate() raise AnsibleError('timeout waiting for privilege escalation password prompt:\n' + to_native(become_output)) @@ -291,8 +351,9 @@ class Connection(ConnectionBase): def put_file(self, in_path, out_path): """ Transfer a file from local to docker container """ + self._set_conn_data() super(Connection, self).put_file(in_path, out_path) - display.vvv("PUT %s TO %s" % (in_path, out_path), host=self._play_context.remote_addr) + display.vvv("PUT %s TO %s" % (in_path, out_path), host=self.get_option('remote_addr')) out_path = self._prefix_login_path(out_path) if not os.path.exists(to_bytes(in_path, errors='surrogate_or_strict')): @@ -324,15 +385,16 @@ class Connection(ConnectionBase): def fetch_file(self, in_path, out_path): """ Fetch a file from container to local. """ + self._set_conn_data() super(Connection, self).fetch_file(in_path, out_path) - display.vvv("FETCH %s TO %s" % (in_path, out_path), host=self._play_context.remote_addr) + display.vvv("FETCH %s TO %s" % (in_path, out_path), host=self.get_option('remote_addr')) in_path = self._prefix_login_path(in_path) # out_path is the final file path, but docker takes a directory, not a # file path out_dir = os.path.dirname(out_path) - args = [self.docker_cmd, "cp", "%s:%s" % (self._play_context.remote_addr, in_path), out_dir] + args = [self.docker_cmd, "cp", "%s:%s" % (self.get_option('remote_addr'), in_path), out_dir] args = [to_bytes(i, errors='surrogate_or_strict') for i in args] p = subprocess.Popen(args, stdin=subprocess.PIPE, diff --git a/tests/unit/plugins/connection/test_docker.py b/tests/unit/plugins/connection/test_docker.py index 885f676c..e21872e5 100644 --- a/tests/unit/plugins/connection/test_docker.py +++ b/tests/unit/plugins/connection/test_docker.py @@ -27,6 +27,7 @@ from ansible_collections.community.docker.tests.unit.compat import unittest from ansible.errors import AnsibleError from ansible.playbook.play_context import PlayContext from ansible_collections.community.docker.plugins.connection.docker import Connection as DockerConnection +from ansible.plugins.loader import connection_loader class TestDockerConnectionClass(unittest.TestCase): @@ -37,6 +38,16 @@ class TestDockerConnectionClass(unittest.TestCase): '[sudo via ansible, key=ouzmdnewuhucvuaabtjmweasarviygqq] password: ' ) self.in_stream = StringIO() + self.mock_get_bin_path = mock.patch( + 'ansible_collections.community.docker.plugins.connection.docker.get_bin_path', return_value='docker') + self.mock_get_bin_path.start() + with mock.patch( + 'ansible_collections.community.docker.plugins.connection.docker.Connection._old_docker_version', + return_value=('false', 'garbage', '', 1)): + with mock.patch( + 'ansible_collections.community.docker.plugins.connection.docker.Connection._new_docker_version', + return_value=(['docker', 'version'], '20.10.0', '', 0)): + dc = connection_loader.get('community.docker.docker', self.play_context, self.in_stream) def tearDown(self): pass