From 343dc7f8a8bf6472d65b80ed055c8776bc85b6a5 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sun, 1 Jan 2023 23:20:36 +0100 Subject: [PATCH 01/27] Move copying functionality to module_utils. --- plugins/connection/docker_api.py | 136 ++++++-------------- plugins/module_utils/copy.py | 210 +++++++++++++++++++++++++++++++ 2 files changed, 246 insertions(+), 100 deletions(-) create mode 100644 plugins/module_utils/copy.py diff --git a/plugins/connection/docker_api.py b/plugins/connection/docker_api.py index 1a48f0842..e911dc04c 100644 --- a/plugins/connection/docker_api.py +++ b/plugins/connection/docker_api.py @@ -68,11 +68,8 @@ type: integer ''' -import io import os import os.path -import shutil -import tarfile from ansible.errors import AnsibleFileNotFound, AnsibleConnectionFailure from ansible.module_utils.common.text.converters import to_bytes, to_native, to_text @@ -82,6 +79,13 @@ from ansible_collections.community.docker.plugins.module_utils.common_api import ( RequestException, ) +from ansible_collections.community.docker.plugins.module_utils.copy import ( + DockerFileCopyError, + DockerFileNotFound, + fetch_file, + put_file, +) + from ansible_collections.community.docker.plugins.plugin_utils.socket_handler import ( DockerSocketHandler, ) @@ -89,7 +93,6 @@ AnsibleDockerClient, ) -from ansible_collections.community.docker.plugins.module_utils._api.constants import DEFAULT_DATA_CHUNK_SIZE from ansible_collections.community.docker.plugins.module_utils._api.errors import APIError, DockerException, NotFound MIN_DOCKER_API = None @@ -260,24 +263,12 @@ def _prefix_login_path(self, remote_path): remote_path = os.path.join(os.path.sep, remote_path) return os.path.normpath(remote_path) - def _put_archive(self, container, path, data): - # data can also be file object for streaming. This is because _put uses requests's put(). - # See https://2.python-requests.org/en/master/user/advanced/#streaming-uploads - # WARNING: might not work with all transports! - url = self.client._url('/containers/{0}/archive', container) - res = self.client._put(url, params={'path': path}, data=data) - self.client._raise_for_status(res) - return res.status_code == 200 - def put_file(self, in_path, out_path): """ Transfer a file from local to docker container """ super(Connection, self).put_file(in_path, out_path) 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')): - raise AnsibleFileNotFound( - "file or module does not exist: %s" % to_native(in_path)) if self.actual_user not in self.ids: dummy, ids, dummy = self.exec_command(b'id -u && id -g') @@ -294,43 +285,23 @@ def put_file(self, in_path, out_path): .format(e, self.get_option('remote_addr'), ids) ) - b_in_path = to_bytes(in_path, errors='surrogate_or_strict') - - out_dir, out_file = os.path.split(out_path) - - # TODO: stream tar file, instead of creating it in-memory into a BytesIO - - bio = io.BytesIO() - with tarfile.open(fileobj=bio, mode='w|', dereference=True, encoding='utf-8') as tar: - # Note that without both name (bytes) and arcname (unicode), this either fails for - # Python 2.7, Python 3.5/3.6, or Python 3.7+. Only when passing both (in this - # form) it works with Python 2.7, 3.5, 3.6, and 3.7 up to 3.11 - tarinfo = tar.gettarinfo(b_in_path, arcname=to_text(out_file)) - user_id, group_id = self.ids[self.actual_user] - tarinfo.uid = user_id - tarinfo.uname = '' - if self.actual_user: - tarinfo.uname = self.actual_user - tarinfo.gid = group_id - tarinfo.gname = '' - tarinfo.mode &= 0o700 - with open(b_in_path, 'rb') as f: - tar.addfile(tarinfo, fileobj=f) - data = bio.getvalue() - - ok = self._call_client( - lambda: self._put_archive( - self.get_option('remote_addr'), - out_dir, - data, - ), - not_found_can_be_resource=True, - ) - if not ok: - raise AnsibleConnectionFailure( - 'Unknown error while creating file "{0}" in container "{1}".' - .format(out_path, self.get_option('remote_addr')) + user_id, group_id = self.ids[self.actual_user] + try: + put_file( + call_client=lambda callback: self._call_client(lambda: callback(self.client), not_found_can_be_resource=True), + container=self.get_option('remote_addr'), + in_path=in_path, + out_path=out_path, + user_id=user_id, + group_id=group_id, + user_name=self.actual_user, + follow_links=True, + log=lambda msg: display.vvvv(msg, host=self.get_option('remote_addr')), ) + except DockerFileNotFound as exc: + raise AnsibleFileNotFound(to_native(exc)) + except DockerFileCopyError as exc: + raise AnsibleConnectionFailure(to_native(exc)) def fetch_file(self, in_path, out_path): """ Fetch a file from container to local. """ @@ -338,55 +309,20 @@ def fetch_file(self, in_path, out_path): display.vvv("FETCH %s TO %s" % (in_path, out_path), host=self.get_option('remote_addr')) in_path = self._prefix_login_path(in_path) - b_out_path = to_bytes(out_path, errors='surrogate_or_strict') - - considered_in_paths = set() - - while True: - if in_path in considered_in_paths: - raise AnsibleConnectionFailure('Found infinite symbolic link loop when trying to fetch "{0}"'.format(in_path)) - considered_in_paths.add(in_path) - - display.vvvv('FETCH: Fetching "%s"' % in_path, host=self.get_option('remote_addr')) - stream = self._call_client( - lambda: self.client.get_raw_stream( - '/containers/{0}/archive', self.get_option('remote_addr'), - params={'path': in_path}, - headers={'Accept-Encoding': 'identity'}, - ), - not_found_can_be_resource=True, - ) - # TODO: stream tar file instead of downloading it into a BytesIO - - bio = io.BytesIO() - for chunk in stream: - bio.write(chunk) - bio.seek(0) - - with tarfile.open(fileobj=bio, mode='r|') as tar: - symlink_member = None - first = True - for member in tar: - if not first: - raise AnsibleConnectionFailure('Received tarfile contains more than one file!') - first = False - if member.issym(): - symlink_member = member - continue - if not member.isfile(): - raise AnsibleConnectionFailure('Remote file "%s" is not a regular file or a symbolic link' % in_path) - in_f = tar.extractfile(member) # in Python 2, this *cannot* be used in `with`... - with open(b_out_path, 'wb') as out_f: - shutil.copyfileobj(in_f, out_f, member.size) - if first: - raise AnsibleConnectionFailure('Received tarfile is empty!') - # If the only member was a file, it's already extracted. If it is a symlink, process it now. - if symlink_member is not None: - in_path = os.path.join(os.path.split(in_path)[0], symlink_member.linkname) - display.vvvv('FETCH: Following symbolic link to "%s"' % in_path, host=self.get_option('remote_addr')) - continue - return + try: + fetch_file( + call_client=lambda callback: self._call_client(lambda: callback(self.client), not_found_can_be_resource=True), + container=self.get_option('remote_addr'), + in_path=in_path, + out_path=out_path, + follow_links=True, + log=lambda msg: display.vvvv(msg, host=self.get_option('remote_addr')), + ) + except DockerFileNotFound as exc: + raise AnsibleFileNotFound(to_native(exc)) + except DockerFileCopyError as exc: + raise AnsibleConnectionFailure(to_native(exc)) def close(self): """ Terminate the connection. Nothing to do for Docker""" diff --git a/plugins/module_utils/copy.py b/plugins/module_utils/copy.py new file mode 100644 index 000000000..12e29586a --- /dev/null +++ b/plugins/module_utils/copy.py @@ -0,0 +1,210 @@ +# Copyright 2016 Red Hat | Ansible +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import io +import os +import os.path +import shutil +import tarfile + +from ansible.module_utils.common.text.converters import to_bytes, to_native, to_text +from ansible.module_utils.six import raise_from + +from ansible_collections.community.docker.plugins.module_utils.common_api import ( + RequestException, +) + +from ansible_collections.community.docker.plugins.module_utils._api.errors import APIError, DockerException, NotFound + + +class DockerFileCopyError(Exception): + pass + + +class DockerUnexpectedError(DockerFileCopyError): + pass + + +class DockerFileNotFound(DockerFileCopyError): + pass + + +def _put_archive(client, container, path, data): + # data can also be file object for streaming. This is because _put uses requests's put(). + # See https://2.python-requests.org/en/master/user/advanced/#streaming-uploads + # WARNING: might not work with all transports! + url = client._url('/containers/{0}/archive', container) + res = client._put(url, params={'path': path}, data=data) + client._raise_for_status(res) + return res.status_code == 200 + + +def put_file(call_client, container, in_path, out_path, user_id, group_id, mode=None, user_name=None, follow_links=False, log=None): + """Transfer a file from local to Docker container.""" + if not os.path.exists(to_bytes(in_path, errors='surrogate_or_strict')): + raise DockerFileNotFound( + "file or module does not exist: %s" % to_native(in_path)) + + b_in_path = to_bytes(in_path, errors='surrogate_or_strict') + + out_dir, out_file = os.path.split(out_path) + + # TODO: stream tar file, instead of creating it in-memory into a BytesIO + + bio = io.BytesIO() + with tarfile.open(fileobj=bio, mode='w|', dereference=follow_links, encoding='utf-8') as tar: + # Note that without both name (bytes) and arcname (unicode), this either fails for + # Python 2.7, Python 3.5/3.6, or Python 3.7+. Only when passing both (in this + # form) it works with Python 2.7, 3.5, 3.6, and 3.7 up to 3.11 + tarinfo = tar.gettarinfo(b_in_path, arcname=to_text(out_file)) + tarinfo.uid = user_id + tarinfo.uname = '' + if user_name: + tarinfo.uname = user_name + tarinfo.gid = group_id + tarinfo.gname = '' + tarinfo.mode &= 0o700 + if mode is not None: + tarinfo.mode = mode + if os.path.isfile(b_in_path) or follow_links: + with open(b_in_path, 'rb') as f: + tar.addfile(tarinfo, fileobj=f) + else: + tar.addfile(tarinfo) + data = bio.getvalue() + + ok = call_client( + lambda client: _put_archive( + client, + container, + out_dir, + data, + ) + ) + if not ok: + raise DockerFileCopyError('Unknown error while creating file "{0}" in container "{1}".'.format(out_path, container)) + + +def fetch_file_ex(call_client, container, in_path, process_none, process_regular, process_symlink, follow_links=False, log=None): + """Fetch a file (as a tar file entry) from a Docker container to local.""" + considered_in_paths = set() + + while True: + if in_path in considered_in_paths: + raise DockerFileCopyError('Found infinite symbolic link loop when trying to fetch "{0}"'.format(in_path)) + considered_in_paths.add(in_path) + + if log: + log('FETCH: Fetching "%s"' % in_path) + try: + stream = call_client( + lambda client: client.get_raw_stream( + '/containers/{0}/archive', container, + params={'path': in_path}, + headers={'Accept-Encoding': 'identity'}, + ) + ) + except DockerFileNotFound: + return process_none(in_path) + + # TODO: stream tar file instead of downloading it into a BytesIO + + bio = io.BytesIO() + for chunk in stream: + bio.write(chunk) + bio.seek(0) + + with tarfile.open(fileobj=bio, mode='r|') as tar: + file_member = None + symlink_member = None + result = None + found = False + for member in tar: + if found: + raise DockerFileCopyError('Received tarfile contains more than one file!') + found = True + if member.issym(): + symlink_member = member + continue + if member.isfile(): + result = process_regular(in_path, tar, member) + continue + raise DockerFileCopyError('Remote file "%s" is not a regular file or a symbolic link' % in_path) + if symlink_member: + if not follow_links: + return process_symlink(in_path, symlink_member) + in_path = os.path.join(os.path.split(in_path)[0], symlink_member.linkname) + if log: + log('FETCH: Following symbolic link to "%s"' % in_path) + continue + if found: + return result + raise DockerUnexpectedError('Received tarfile is empty!') + + +def fetch_file(call_client, container, in_path, out_path, follow_links=False, log=None): + b_out_path = to_bytes(out_path, errors='surrogate_or_strict') + + def process_none(in_path): + raise DockerFileNotFound( + 'File {in_path} does not exist in container {container}' + .format(in_path=in_path, container=container) + ) + + def process_regular(in_path, tar, member): + if not follow_links and os.path.exists(b_out_path): + os.unlink(b_out_path) + + in_f = tar.extractfile(member) # in Python 2, this *cannot* be used in `with`... + with open(b_out_path, 'wb') as out_f: + shutil.copyfileobj(in_f, out_f, member.size) + return in_path + + def process_symlink(in_path, member): + if os.path.exists(b_out_path): + os.unlink(b_out_path) + + os.symlink(member.linkname, b_out_path) + return in_path + + return fetch_file_ex(call_client, container, in_path, process_none, process_regular, process_symlink, follow_links=follow_links, log=log) + + +def call_client(client, container, use_file_not_found_exception=False): + def f(callback): + try: + return callback(client) + except NotFound as e: + if use_file_not_found_exception: + raise_from(DockerFileNotFound(to_native(e)), e) + raise_from( + DockerFileCopyError('Could not find container "{1}" or resource in it ({0})'.format(e, container)), + e, + ) + except APIError as e: + if e.response and e.response.status_code == 409: + raise_from( + DockerFileCopyError('The container "{1}" has been paused ({0})'.format(e, container)), + e, + ) + raise_from( + DockerUnexpectedError('An unexpected Docker error occurred for container "{1}": {0}'.format(e, container)), + e, + ) + except DockerException as e: + raise_from( + DockerUnexpectedError('An unexpected Docker error occurred for container "{1}": {0}'.format(e, container)), + e, + ) + except RequestException as e: + raise_from( + DockerUnexpectedError( + 'An unexpected requests error occurred for container "{1}" when trying to talk to the Docker daemon: {0}'.format(e, container)), + e, + ) + + return f From d320d0eb21b4762c7c173434c899a74f43d5f13a Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Thu, 8 Apr 2021 08:01:34 +0200 Subject: [PATCH 02/27] Add docker_container_copy_into module. --- README.md | 1 + meta/runtime.yml | 1 + plugins/module_utils/_api/api/client.py | 4 + plugins/module_utils/copy.py | 109 +++++ plugins/modules/docker_container_copy_into.py | 290 ++++++++++++ .../docker_container_copy_into/aliases | 6 + .../docker_container_copy_into/meta/main.yml | 8 + .../docker_container_copy_into/tasks/main.yml | 45 ++ .../tasks/run-test.yml | 7 + .../tasks/tests/simple.yml | 441 ++++++++++++++++++ 10 files changed, 912 insertions(+) create mode 100644 plugins/modules/docker_container_copy_into.py create mode 100644 tests/integration/targets/docker_container_copy_into/aliases create mode 100644 tests/integration/targets/docker_container_copy_into/meta/main.yml create mode 100644 tests/integration/targets/docker_container_copy_into/tasks/main.yml create mode 100644 tests/integration/targets/docker_container_copy_into/tasks/run-test.yml create mode 100644 tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml diff --git a/README.md b/README.md index b8cfbd20e..e82e0a8ed 100644 --- a/README.md +++ b/README.md @@ -57,6 +57,7 @@ If you use the Ansible package and do not update collections independently, use * Modules: * Docker: - community.docker.docker_container: manage Docker containers + - community.docker.docker_container_copy_into: copy a file into a Docker container - community.docker.docker_container_exec: run commands in Docker containers - community.docker.docker_container_info: retrieve information on Docker containers - community.docker.docker_host_info: retrieve information on the Docker daemon diff --git a/meta/runtime.yml b/meta/runtime.yml index c7d94dda5..7616e6fea 100644 --- a/meta/runtime.yml +++ b/meta/runtime.yml @@ -9,6 +9,7 @@ action_groups: - docker_compose - docker_config - docker_container + - docker_container_copy_into - docker_container_exec - docker_container_info - docker_host_info diff --git a/plugins/module_utils/_api/api/client.py b/plugins/module_utils/_api/api/client.py index faf30cda2..fa99e069b 100644 --- a/plugins/module_utils/_api/api/client.py +++ b/plugins/module_utils/_api/api/client.py @@ -227,6 +227,10 @@ def _post(self, url, **kwargs): def _get(self, url, **kwargs): return self.get(url, **self._set_request_timeout(kwargs)) + @update_headers + def _head(self, url, **kwargs): + return self.head(url, **self._set_request_timeout(kwargs)) + @update_headers def _put(self, url, **kwargs): return self.put(url, **self._set_request_timeout(kwargs)) diff --git a/plugins/module_utils/copy.py b/plugins/module_utils/copy.py index 12e29586a..0af5cde59 100644 --- a/plugins/module_utils/copy.py +++ b/plugins/module_utils/copy.py @@ -5,7 +5,9 @@ from __future__ import (absolute_import, division, print_function) __metaclass__ = type +import base64 import io +import json import os import os.path import shutil @@ -89,6 +91,48 @@ def put_file(call_client, container, in_path, out_path, user_id, group_id, mode= raise DockerFileCopyError('Unknown error while creating file "{0}" in container "{1}".'.format(out_path, container)) +def stat_file(call_client, container, in_path, follow_links=False, log=None): + considered_in_paths = set() + + while True: + if in_path in considered_in_paths: + raise DockerFileCopyError('Found infinite symbolic link loop when trying to stating "{0}"'.format(in_path)) + considered_in_paths.add(in_path) + + if log: + log('FETCH: Stating "%s"' % in_path) + + def f(client): + response = client._head( + client._url('/containers/{0}/archive', container), + params={'path': in_path}, + ) + if response.status_code == 404: + return None + client._raise_for_status(response) + header = response.headers.get('x-docker-container-path-stat') + try: + return json.loads(base64.b64decode(header)) + except Exception as exc: + raise DockerUnexpectedError( + 'When retrieving information for {in_path} from {container}, obtained header {header!r} that cannot be loaded as JSON: {exc}' + .format(in_path=in_path, container=container, header=header, exc=exc) + ) + + stat_data = call_client(f) + if stat_data is None: + return in_path, None, None + + link_target = stat_data.pop('linkTarget', '') + if link_target: + if not follow_links: + return in_path, None, link_target + in_path = os.path.join(os.path.split(in_path)[0], link_target) + continue + + return in_path, stat_data, None + + def fetch_file_ex(call_client, container, in_path, process_none, process_regular, process_symlink, follow_links=False, log=None): """Fetch a file (as a tar file entry) from a Docker container to local.""" considered_in_paths = set() @@ -208,3 +252,68 @@ def f(callback): ) return f + + +def _execute_command(client, container, command, log=None, check_rc=False): + if log: + log('Executing {command} in {container}'.format(command=command, container=container)) + + data = { + 'Container': container, + 'User': '', + 'Privileged': False, + 'Tty': False, + 'AttachStdin': False, + 'AttachStdout': True, + 'AttachStderr': True, + 'Cmd': command, + } + + if 'detachKeys' in client._general_configs: + data['detachKeys'] = client._general_configs['detachKeys'] + + exec_data = client.post_json_to_json('/containers/{0}/exec', container, data=data) + exec_id = exec_data['Id'] + + data = { + 'Tty': False, + 'Detach': False + } + stdout, stderr = client.post_json_to_stream('/exec/{0}/start', exec_id, stream=False, demux=True, tty=False) + + result = client.get_json('/exec/{0}/json', exec_id) + + rc = result.get('ExitCode') or 0 + stdout = stdout or b'' + stderr = stderr or b'' + + if log: + log('Exit code {rc}, stdout {stdout!r}, stderr {stderr!r}'.format(rc=rc, stdout=stdout, stderr=stderr)) + + if check_rc and rc != 0: + raise DockerFileCopyError( + 'Obtained unexpected exit code {rc} when running "{command}" in {container}.\nSTDOUT: {stdout}\nSTDERR: {stderr}' + .format(command=' '.join(command), container=container, rc=rc, stdout=stdout, stderr=stderr) + ) + + return rc, stdout, stderr + + +def determine_user_group(client, container, log=None): + dummy, stdout, stderr = _execute_command(client, container, ['/bin/sh', '-c', 'id -u && id -g'], check_rc=True, log=log) + + stdout_lines = stdout.splitlines() + if len(stdout_lines) != 2: + raise DockerFileCopyError( + 'Expected two-line output to obtain user and group ID for container {container}, but got {lc} lines:\n{stdout}' + .format(container=container, lc=len(stdout_lines), stdout=stdout) + ) + + user_id, group_id = stdout_lines + try: + return int(user_id), int(group_id) + except ValueError: + raise DockerFileCopyError( + 'Expected two-line output with numeric IDs to obtain user and group ID for container {container}, but got "{l1}" and "{l2}" instead' + .format(container=container, l1=user_id, l2=group_id) + ) diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py new file mode 100644 index 000000000..6e5c91a9e --- /dev/null +++ b/plugins/modules/docker_container_copy_into.py @@ -0,0 +1,290 @@ +#!/usr/bin/python +# +# Copyright 2016 Red Hat | Ansible +# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) + +from __future__ import absolute_import, division, print_function +__metaclass__ = type + + +DOCUMENTATION = ''' +--- +module: docker_container_copy_into + +short_description: Copy a file into a Docker container + +description: + - Copy a file into a Docker container. + - Similar to C(docker cp). + - The file is processed in-memory. Do not use for large files! + +options: + container: + description: + - The name of the container to copy files to/from. + type: str + required: true + path: + description: + - Path to a file on the managed node. + type: path + required: true + container_path: + description: + - Path to a file inside the Docker container. + - Must be an absolute path. + type: str + required: true + follow: + description: + - This flag indicates that filesystem links in the Docker container, if they exist, should be followed. + type: bool + default: false + local_follow: + description: + - This flag indicates that filesystem links in the source tree (where the module is executed), if they exist, should be followed. + type: bool + default: true + owner_id: + description: + - The owner ID to use when writing the file to disk. + - If provided, I(group_id) must also be provided. + - If not provided, the module will try to determine the user and group ID for the current user in the container. + This will only work if C(/bin/sh) is present in the container and the C(id) binary or shell builtin is available. + type: int + group_id: + description: + - The group ID to use when writing the file to disk. + - If provided, I(owner_id) must also be provided. + - If not provided, the module will try to determine the user and group ID for the current user in the container. + This will only work if C(/bin/sh) is present in the container and the C(id) binary or shell builtin is available. + type: int + mode: + description: + - The file mode to use when writing the file to disk. + - Will use the file's mode from the source system if this option is not provided. + type: int + +extends_documentation_fragment: + - community.docker.docker.api_documentation + - community.docker.attributes + - community.docker.attributes.actiongroup_docker + +author: + - "Felix Fontein (@felixfontein)" + +requirements: + - "Docker API >= 1.25" +''' + +EXAMPLES = ''' +- name: Copy a file into the container + community.docker.docker_container_copy_into: + container: mydata + path: /home/user/data.txt + container_path: /data/input.txt +''' + +RETURN = ''' +container_path: + description: + - The actual path in the container. + - Can only be different from I(container_path) when I(follow=true). + type: str + returned: success +''' + +import os +import stat +import traceback + +from ansible.module_utils._text import to_bytes, to_text, to_native + +from ansible_collections.community.docker.plugins.module_utils.common_api import ( + AnsibleDockerClient, + RequestException, +) + +from ansible_collections.community.docker.plugins.module_utils.copy import ( + DockerFileCopyError, + DockerFileNotFound, + DockerUnexpectedError, + call_client, + determine_user_group, + fetch_file_ex, + put_file, + stat_file, +) + + +def is_idempotent(client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode): + # Retrieve information of local file + try: + file_stat = os.stat(managed_path) if local_follow_links else os.lstat(managed_path) + except OSError as exc: + if exc.errno == 2: + raise DockerFileNotFound('Cannot find local file {managed_path}'.format(managed_path=managed_path)) + raise + if mode is None: + mode = stat.S_IMODE(file_stat.st_mode) + if not stat.S_ISLNK(file_stat.st_mode) and not stat.S_ISREG(file_stat.st_mode): + raise DockerFileCopyError('Local path {managed_path} is not a symbolic link or file') + + real_container_path, regular_stat, link_target = stat_file( + call_client(client, container), + container, + in_path=container_path, + follow_links=follow_links, + ) + + # Follow links in the Docker container? + if follow_links: + container_path = real_container_path + + # If the file wasn't found, continue + if regular_stat is None and link_target is None: + return container_path, mode, False + + # Basic idempotency checks + if stat.S_ISLNK(file_stat.st_mode): + if link_target is None: + return container_path, mode, False + local_link_target = os.readlink(managed_path) + return container_path, mode, local_link_target == link_target + if not stat.S_ISREG(file_stat.st_mode): + raise DockerFileCopyError('Local path {managed_path} is not a symbolic link or file') + if regular_stat is None: + return container_path, mode, False + if file_stat.st_size != regular_stat['size']: + return container_path, mode, False + if mode != regular_stat['mode'] & 0xFFF: + return container_path, mode, False + + # Fetch file from container + def process_none(in_path): + return container_path, mode, False + + def process_regular(in_path, tar, member): + # Check things like user/group ID and mode + if member.mode & 0xFFF != mode: + return container_path, mode, False + if member.uid != owner_id: + return container_path, mode, False + if member.gid != group_id: + return container_path, mode, False + + if not stat.S_ISREG(file_stat.st_mode): + return container_path, mode, False + if member.size != file_stat.st_size: + return container_path, mode, False + + tar_f = tar.extractfile(member) # in Python 2, this *cannot* be used in `with`... + with open(managed_path, 'rb') as local_f: + return container_path, mode, tar_f.read() == local_f.read() + + def process_symlink(in_path, member): + # Check things like user/group ID and mode + if member.mode & 0xFFF != mode: + return container_path, mode, False + if member.uid != owner_id: + return container_path, mode, False + if member.gid != group_id: + return container_path, mode, False + + if not stat.S_ISLNK(file_stat.st_mode): + return container_path, mode, False + + local_link_target = os.readlink(managed_path) + return container_path, mode, member.linkname == local_link_target + + return fetch_file_ex( + call_client(client, container, use_file_not_found_exception=True), + container, + in_path=container_path, + process_none=process_none, + process_regular=process_regular, + process_symlink=process_symlink, + follow_links=follow_links, + ) + + +def copy_into_container(client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode): + container_path, mode, idempotent = is_idempotent( + client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode) + changed = not idempotent + + if changed and not client.module.check_mode: + put_file( + call_client(client, container), + container, + in_path=managed_path, + out_path=container_path, + user_id=owner_id, + group_id=group_id, + mode=mode, + follow_links=local_follow_links, + ) + + client.module.exit_json( + container_path=container_path, + changed=changed, + ) + + +def main(): + argument_spec = dict( + container=dict(type='str', required=True), + path=dict(type='path', required=True), + container_path=dict(type='str', required=True), + follow=dict(type='bool', default=False), + local_follow=dict(type='bool', default=True), + owner_id=dict(type='int'), + group_id=dict(type='int'), + mode=dict(type='int'), + ) + + client = AnsibleDockerClient( + argument_spec=argument_spec, + min_docker_api_version='1.20', + supports_check_mode=True, + required_together=[('owner_id', 'group_id')], + ) + + container = client.module.params['container'] + managed_path = client.module.params['path'] + container_path = client.module.params['container_path'] + follow = client.module.params['follow'] + local_follow = client.module.params['local_follow'] + owner_id = client.module.params['owner_id'] + group_id = client.module.params['group_id'] + mode = client.module.params['mode'] + + if not container_path.startswith(os.path.sep): + container_path = os.path.join(os.path.sep, container_path) + container_path = os.path.normpath(container_path) + + try: + if owner_id is None or group_id is None: + owner_id, group_id = call_client(client, container)(lambda c: determine_user_group(c, container)) + + copy_into_container( + client, + container, + managed_path, + container_path, + follow_links=follow, + local_follow_links=local_follow, + owner_id=owner_id, + group_id=group_id, + mode=mode, + ) + except DockerUnexpectedError as exc: + client.fail('Unexpected error: {exc}'.format(exc=to_native(exc)), exception=traceback.format_exc()) + except DockerFileCopyError as exc: + client.fail(to_native(exc)) + except OSError as exc: + client.fail('Unexpected error: {exc}'.format(exc=to_native(exc)), exception=traceback.format_exc()) + + +if __name__ == '__main__': + main() diff --git a/tests/integration/targets/docker_container_copy_into/aliases b/tests/integration/targets/docker_container_copy_into/aliases new file mode 100644 index 000000000..2e1acc0ad --- /dev/null +++ b/tests/integration/targets/docker_container_copy_into/aliases @@ -0,0 +1,6 @@ +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +azp/4 +destructive diff --git a/tests/integration/targets/docker_container_copy_into/meta/main.yml b/tests/integration/targets/docker_container_copy_into/meta/main.yml new file mode 100644 index 000000000..2650229d8 --- /dev/null +++ b/tests/integration/targets/docker_container_copy_into/meta/main.yml @@ -0,0 +1,8 @@ +--- +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +dependencies: + - setup_docker + - setup_remote_tmp_dir diff --git a/tests/integration/targets/docker_container_copy_into/tasks/main.yml b/tests/integration/targets/docker_container_copy_into/tasks/main.yml new file mode 100644 index 000000000..20f9a2681 --- /dev/null +++ b/tests/integration/targets/docker_container_copy_into/tasks/main.yml @@ -0,0 +1,45 @@ +--- +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +#################################################################### +# WARNING: These are designed specifically for Ansible tests # +# and should not be used as examples of how to write Ansible roles # +#################################################################### + +- name: Gather facts on controller + setup: + gather_subset: '!all' + delegate_to: localhost + delegate_facts: true + run_once: true + +# Create random name prefix (for containers) +- name: Create random container name prefix + set_fact: + cname_prefix: "{{ 'ansible-docker-test-%0x' % ((2**32) | random) }}" + cnames: [] + +- debug: + msg: "Using container name prefix {{ cname_prefix }}" + +# Run the tests +- block: + - include_tasks: run-test.yml + with_fileglob: + - "tests/*.yml" + + always: + - name: "Make sure all containers are removed" + docker_container: + name: "{{ item }}" + state: absent + force_kill: true + with_items: "{{ cnames }}" + diff: false + + when: docker_api_version is version('1.25', '>=') + +- fail: msg="Too old Docker API version to run all docker_container_copy_into tests!" + when: not(docker_api_version is version('1.25', '>=')) and (ansible_distribution != 'CentOS' or ansible_distribution_major_version|int > 6) diff --git a/tests/integration/targets/docker_container_copy_into/tasks/run-test.yml b/tests/integration/targets/docker_container_copy_into/tasks/run-test.yml new file mode 100644 index 000000000..65853ddd8 --- /dev/null +++ b/tests/integration/targets/docker_container_copy_into/tasks/run-test.yml @@ -0,0 +1,7 @@ +--- +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +- name: "Loading tasks from {{ item }}" + include_tasks: "{{ item }}" diff --git a/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml b/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml new file mode 100644 index 000000000..a919d95bb --- /dev/null +++ b/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml @@ -0,0 +1,441 @@ +--- +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +- name: Registering container name + set_fact: + cname: "{{ cname_prefix ~ '-hi' }}" +- name: Registering container name + set_fact: + cnames: "{{ cnames + [cname] }}" + +# Create container + +- name: Create container + docker_container: + image: "{{ docker_test_image_alpine }}" + command: + - /bin/sh + - "-c" + - >- + mkdir /dir; + ln -s file /lnk; + ln -s lnk3 /lnk2; + ln -s lnk2 /lnk1; + sleep 10m; + name: "{{ cname }}" + state: started + +# Create files + +- name: Create file 1 + copy: + dest: '{{ remote_tmp_dir }}/file_1' + content: | + Content 1 + mode: 0644 + +- name: Create file 2 + copy: + dest: '{{ remote_tmp_dir }}/file_2' + content: |- + Content 2 + Extra line + mode: 0644 + +- name: Create link 1 + file: + dest: '{{ remote_tmp_dir }}/link_1' + state: link + src: file_1 + follow: false + mode: 0644 + +- name: Create link 2 + file: + dest: '{{ remote_tmp_dir }}/link_2' + state: link + src: dead + force: true + follow: false + mode: 0644 + +################################################################################################ +# Do tests + +######################### Copy + +- name: Copy file (check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/file' + check_mode: true + register: result_1 + +- name: Copy file (check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/file' + register: result_2 + +- name: Copy file (idempotent, check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/file' + check_mode: true + register: result_3 + +- name: Copy file (idempotent) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/file' + register: result_4 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /file | base64; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_5 + +- name: Check results + assert: + that: + - result_1 is changed + - result_2 is changed + - result_3 is not changed + - result_4 is not changed + - result_5.stdout | b64decode == 'Content 1\n' + - result_5.stderr == '10 644 regular file 0 0 /file' + +######################### Follow link - idempotence + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /lnk | base64; + stat -c '%s %a %F %u %g %N' /lnk > /dev/stderr; + chdir: /root + register: result_0 + +- name: Copy file following link (check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/lnk' + follow: true + check_mode: true + register: result_1 + +- name: Copy file following link (check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/lnk' + follow: true + register: result_2 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /lnk | base64; + stat -c '%s %a %F %u %g %N' /lnk > /dev/stderr; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_3 + +- name: Check results + assert: + that: + - result_0.stdout | b64decode == 'Content 1\n' + - result_0.stderr == "4 777 symbolic link 0 0 '/lnk' -> 'file'" + - result_1 is not changed + - result_1.container_path == '/file' + - result_2 is not changed + - result_2.container_path == '/file' + - result_3.stdout | b64decode == 'Content 1\n' + - result_3.stderr_lines[0] == "4 777 symbolic link 0 0 '/lnk' -> 'file'" + - result_3.stderr_lines[1] == '10 644 regular file 0 0 /file' + +######################### Do not follow link - replace by file + +- name: Copy file not following link (check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/lnk' + follow: false + check_mode: true + register: result_1 + +- name: Copy file not following link (check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/lnk' + follow: false + register: result_2 + +- name: Copy file not following link (idempotent, check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/lnk' + check_mode: true + register: result_3 + +- name: Copy file not following link (idempotent) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/lnk' + register: result_4 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /lnk | base64; + stat -c '%s %a %F %u %g %N' /lnk > /dev/stderr + chdir: /root + register: result_5 + +- name: Check results + assert: + that: + - result_1 is changed + - result_1.container_path == '/lnk' + - result_2 is changed + - result_2.container_path == '/lnk' + - result_3 is not changed + - result_4 is not changed + - result_5.stdout | b64decode == 'Content 1\n' + - result_5.stderr == '10 644 regular file 0 0 /lnk' + +######################### Modify + +- name: Copy file (changed, check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + check_mode: true + register: result_1 + +- name: Copy file (changed) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + register: result_2 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /file | base64; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_3 + +- name: Check results + assert: + that: + - result_1 is changed + - result_2 is changed + - result_3.stdout | b64decode == 'Content 2\nExtra line' + - result_3.stderr == '20 644 regular file 0 0 /file' + +######################### Change mode + +- name: Copy file (mode changed, check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + mode: 0707 + check_mode: true + register: result_1 + +- name: Copy file (mode changed) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + mode: 0707 + register: result_2 + +- name: Copy file (idempotent, check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + mode: 0707 + check_mode: true + register: result_3 + +- name: Copy file (idempotent) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + mode: 0707 + register: result_4 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /file | base64; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_5 + +- name: Check results + assert: + that: + - result_1 is changed + - result_2 is changed + - result_3 is not changed + - result_4 is not changed + - result_5.stdout | b64decode == 'Content 2\nExtra line' + - result_5.stderr == '20 707 regular file 0 0 /file' + +######################### Change owner and group + +- name: Copy file (owner/group changed, check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + check_mode: true + register: result_1 + +- name: Copy file (owner/group changed) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + register: result_2 + +- name: Copy file (idempotent, check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + check_mode: true + register: result_3 + +- name: Copy file (idempotent) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + register: result_4 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /file | base64; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_5 + +- name: Copy file (owner/group changed again, check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + mode: 0707 + owner_id: 13 + group_id: 13 + check_mode: true + register: result_6 + +- name: Copy file (owner/group changed again) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + mode: 0707 + owner_id: 13 + group_id: 13 + register: result_7 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /file | base64; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_8 + +- name: Check results + assert: + that: + - result_1 is changed + - result_2 is changed + - result_3 is not changed + - result_4 is not changed + - result_5.stdout | b64decode == 'Content 2\nExtra line' + - result_5.stderr == '20 707 regular file 12 910 /file' + - result_6 is changed + - result_7 is changed + - result_8.stdout | b64decode == 'Content 2\nExtra line' + - result_8.stderr == '20 707 regular file 13 13 /file' + +################################################################################################ +# Cleanup + +- name: Remove container + docker_container: + name: "{{ cname }}" + state: absent + force_kill: yes From b890be77d49b88d8de1f406a2622dd1239b3d803 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sun, 1 Jan 2023 23:40:51 +0100 Subject: [PATCH 03/27] Use new module in other tests. --- .../targets/generic_connection_tests/tasks/main.yml | 10 ++++++++-- .../setup_docker_registry/tasks/setup-frontend.yml | 10 ++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/tests/integration/targets/generic_connection_tests/tasks/main.yml b/tests/integration/targets/generic_connection_tests/tasks/main.yml index 22aa534b7..e0bd9c8b8 100644 --- a/tests/integration/targets/generic_connection_tests/tasks/main.yml +++ b/tests/integration/targets/generic_connection_tests/tasks/main.yml @@ -43,7 +43,10 @@ - nginx.conf - name: Copy static files into volume - command: docker cp {{ remote_tmp_dir }}/{{ item }} {{ daemon_nginx_frontend }}:/etc/nginx/{{ item }} + docker_container_copy_into: + container: '{{ daemon_nginx_frontend }}' + path: '{{ remote_tmp_dir }}/{{ item }}' + container_path: '/etc/nginx/{{ item }}' loop: - nginx.conf register: can_copy_files @@ -94,7 +97,10 @@ provider: ownca - name: Copy dynamic files into volume - command: docker cp {{ remote_tmp_dir }}/{{ item }} {{ daemon_nginx_frontend }}:/etc/nginx/{{ item }} + docker_container_copy_into: + container: '{{ daemon_nginx_frontend }}' + path: '{{ remote_tmp_dir }}/{{ item }}' + container_path: '/etc/nginx/{{ item }}' loop: - ca.pem - cert.pem diff --git a/tests/integration/targets/setup_docker_registry/tasks/setup-frontend.yml b/tests/integration/targets/setup_docker_registry/tasks/setup-frontend.yml index d7b55e1ce..8fa2e322e 100644 --- a/tests/integration/targets/setup_docker_registry/tasks/setup-frontend.yml +++ b/tests/integration/targets/setup_docker_registry/tasks/setup-frontend.yml @@ -39,7 +39,10 @@ - nginx.htpasswd - name: Copy static files into volume - command: docker cp {{ remote_tmp_dir }}/{{ item }} {{ docker_registry_container_name_frontend }}:/etc/nginx/{{ item }} + docker_container_copy_into: + container: '{{ docker_registry_container_name_frontend }}' + path: '{{ remote_tmp_dir }}/{{ item }}' + container_path: '/etc/nginx/{{ item }}' loop: - nginx.conf - nginx.htpasswd @@ -71,7 +74,10 @@ provider: selfsigned - name: Copy dynamic files into volume - command: docker cp {{ remote_tmp_dir }}/{{ item }} {{ docker_registry_container_name_frontend }}:/etc/nginx/{{ item }} + docker_container_copy_into: + container: '{{ docker_registry_container_name_frontend }}' + path: '{{ remote_tmp_dir }}/{{ item }}' + container_path: '/etc/nginx/{{ item }}' loop: - cert.pem - cert.key From a9941fc49a8399095b71658cc641a4cd7a93cf72 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Mon, 2 Jan 2023 17:30:12 +0100 Subject: [PATCH 04/27] Fix copyright and attributes. --- plugins/modules/docker_container_copy_into.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py index 6e5c91a9e..24410d7fd 100644 --- a/plugins/modules/docker_container_copy_into.py +++ b/plugins/modules/docker_container_copy_into.py @@ -1,7 +1,8 @@ #!/usr/bin/python # -# Copyright 2016 Red Hat | Ansible -# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) +# Copyright (c) 2022, Felix Fontein +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later from __future__ import absolute_import, division, print_function __metaclass__ = type @@ -18,6 +19,12 @@ - Similar to C(docker cp). - The file is processed in-memory. Do not use for large files! +attributes: + check_mode: + support: full + diff_mode: + support: none + options: container: description: From 26f855e7342aa1c61235a3e8038f60451190a8a8 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Wed, 4 Jan 2023 23:26:16 +0100 Subject: [PATCH 05/27] Improve idempotency, improve stat code. --- plugins/module_utils/copy.py | 18 +++++- plugins/modules/docker_container_copy_into.py | 37 ++++++++++-- .../tasks/tests/simple.yml | 58 +++++++++++++++++++ 3 files changed, 105 insertions(+), 8 deletions(-) diff --git a/plugins/module_utils/copy.py b/plugins/module_utils/copy.py index 0af5cde59..ee12c35cd 100644 --- a/plugins/module_utils/copy.py +++ b/plugins/module_utils/copy.py @@ -92,6 +92,17 @@ def put_file(call_client, container, in_path, out_path, user_id, group_id, mode= def stat_file(call_client, container, in_path, follow_links=False, log=None): + """Fetch information on a file from a Docker container to local. + + Return a tuple ``(path, stat_data, link_target)`` where: + + :path: is the resolved path in case ``follow_links=True``; + :stat_data: is ``None`` if the file does not exist, or a dictionary with fields + ``name`` (string), ``size`` (integer), ``mode`` (integer, see https://pkg.go.dev/io/fs#FileMode), + ``mtime`` (string), and ``linkTarget`` (string); + :link_target: is ``None`` if the file is not a symlink or when ``follow_links=False``, + and a string with the symlink target otherwise. + """ considered_in_paths = set() while True: @@ -123,10 +134,11 @@ def f(client): if stat_data is None: return in_path, None, None - link_target = stat_data.pop('linkTarget', '') - if link_target: + # https://pkg.go.dev/io/fs#FileMode: bit 32 - 5 means ModeSymlink + if stat_data['mode'] & (1 << (32 - 5)) != 0: + link_target = stat_data['linkTarget'] if not follow_links: - return in_path, None, link_target + return in_path, stat_data, link_target in_path = os.path.join(os.path.split(in_path)[0], link_target) continue diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py index 24410d7fd..4d6684473 100644 --- a/plugins/modules/docker_container_copy_into.py +++ b/plugins/modules/docker_container_copy_into.py @@ -90,6 +90,15 @@ container: mydata path: /home/user/data.txt container_path: /data/input.txt + +- name: Copy a file into the container with owner, group, and mode set + community.docker.docker_container_copy_into: + container: mydata + path: /home/user/bin/runme.o + container_path: /bin/runme + owner: 0 # root + group: 0 # root + mode: 0o755 # readable and executable by all users, writable by root ''' RETURN = ''' @@ -149,7 +158,7 @@ def is_idempotent(client, container, managed_path, container_path, follow_links, container_path = real_container_path # If the file wasn't found, continue - if regular_stat is None and link_target is None: + if regular_stat is None: return container_path, mode, False # Basic idempotency checks @@ -158,13 +167,31 @@ def is_idempotent(client, container, managed_path, container_path, follow_links, return container_path, mode, False local_link_target = os.readlink(managed_path) return container_path, mode, local_link_target == link_target - if not stat.S_ISREG(file_stat.st_mode): - raise DockerFileCopyError('Local path {managed_path} is not a symbolic link or file') - if regular_stat is None: + if link_target is not None: return container_path, mode, False + for bit in ( + # https://pkg.go.dev/io/fs#FileMode + 32 - 1, # ModeDir + 32 - 4, # ModeTemporary + 32 - 5, # ModeSymlink + 32 - 6, # ModeDevice + 32 - 7, # ModeNamedPipe + 32 - 8, # ModeSocket + 32 - 11, # ModeCharDevice + 32 - 13, # ModeIrregular + ): + if regular_stat['mode'] & (1 << bit) != 0: + return container_path, mode, False if file_stat.st_size != regular_stat['size']: return container_path, mode, False - if mode != regular_stat['mode'] & 0xFFF: + container_file_mode = regular_stat['mode'] & 0xFFF + if regular_stat['mode'] & (1 << (32 - 9)) != 0: # ModeSetuid + container_file_mode |= stat.S_ISUID # set UID bit + if regular_stat['mode'] & (1 << (32 - 10)) != 0: # ModeSetgid + container_file_mode |= stat.S_ISGID # set GID bit + if regular_stat['mode'] & (1 << (32 - 12)) != 0: # ModeSticky + container_file_mode |= stat.S_ISVTX # sticky bit + if mode != container_file_mode: return container_path, mode, False # Fetch file from container diff --git a/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml b/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml index a919d95bb..d8f8fb1c2 100644 --- a/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml +++ b/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml @@ -233,6 +233,64 @@ - result_5.stdout | b64decode == 'Content 1\n' - result_5.stderr == '10 644 regular file 0 0 /lnk' +######################### Replace directory by file + +- name: Copy file to replace directory (check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/dir' + follow: false + check_mode: true + register: result_1 + +- name: Copy file to replace directory (check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/dir' + follow: false + register: result_2 + +- name: Copy file to replace directory (idempotent, check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/dir' + check_mode: true + register: result_3 + +- name: Copy file to replace directory (idempotent) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/dir' + register: result_4 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /dir | base64; + stat -c '%s %a %F %u %g %N' /dir > /dev/stderr + chdir: /root + register: result_5 + +- name: Check results + assert: + that: + - result_1 is changed + - result_1.container_path == '/dir' + - result_2 is changed + - result_2.container_path == '/dir' + - result_3 is not changed + - result_4 is not changed + - result_5.stdout | b64decode == 'Content 1\n' + - result_5.stderr == '10 644 regular file 0 0 /dir' + ######################### Modify - name: Copy file (changed, check mode) From 10bfd7bec01762d860ea5211b96b45851e5cfd25 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Wed, 4 Jan 2023 23:35:26 +0100 Subject: [PATCH 06/27] Document and test when a stopped container works. --- plugins/modules/docker_container_copy_into.py | 3 + .../tasks/tests/simple.yml | 77 +++++++++++++++++++ 2 files changed, 80 insertions(+) diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py index 4d6684473..0231d7c8f 100644 --- a/plugins/modules/docker_container_copy_into.py +++ b/plugins/modules/docker_container_copy_into.py @@ -18,6 +18,7 @@ - Copy a file into a Docker container. - Similar to C(docker cp). - The file is processed in-memory. Do not use for large files! + - To copy files in a non-running container, you must provide the I(owner_id) and I(group_id) options. attributes: check_mode: @@ -58,6 +59,7 @@ - If provided, I(group_id) must also be provided. - If not provided, the module will try to determine the user and group ID for the current user in the container. This will only work if C(/bin/sh) is present in the container and the C(id) binary or shell builtin is available. + Also the container must be running. type: int group_id: description: @@ -65,6 +67,7 @@ - If provided, I(owner_id) must also be provided. - If not provided, the module will try to determine the user and group ID for the current user in the container. This will only work if C(/bin/sh) is present in the container and the C(id) binary or shell builtin is available. + Also the container must be running. type: int mode: description: diff --git a/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml b/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml index d8f8fb1c2..d64cfb70b 100644 --- a/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml +++ b/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml @@ -489,6 +489,83 @@ - result_8.stdout | b64decode == 'Content 2\nExtra line' - result_8.stderr == '20 707 regular file 13 13 /file' +######################### Operate with stopped container + +- name: Stop container + docker_container: + name: "{{ cname }}" + state: stopped + stop_timeout: 1 + +- name: Copy file (stopped container, check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + check_mode: true + register: result_1 + +- name: Copy file (stopped container) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + register: result_2 + +- name: Copy file (stopped container, idempotent, check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + check_mode: true + register: result_3 + +- name: Copy file (stopped container, idempotent) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + register: result_4 + +- name: Start container + docker_container: + name: "{{ cname }}" + state: started + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /file | base64; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_5 + +- name: Check results + assert: + that: + - result_1 is changed + - result_2 is changed + - result_3 is not changed + - result_4 is not changed + - result_5.stdout | b64decode == 'Content 1\n' + - result_5.stderr == '10 707 regular file 12 910 /file' + ################################################################################################ # Cleanup From a66351c9832d73549d60e74b52f88dec4273a695 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Wed, 4 Jan 2023 23:54:30 +0100 Subject: [PATCH 07/27] Improve owner/group detection error handling when container is stopped. --- plugins/module_utils/copy.py | 12 ++++++++++-- .../tasks/tests/simple.yml | 17 ++++++++++++++--- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/plugins/module_utils/copy.py b/plugins/module_utils/copy.py index ee12c35cd..764620700 100644 --- a/plugins/module_utils/copy.py +++ b/plugins/module_utils/copy.py @@ -242,7 +242,7 @@ def f(callback): e, ) except APIError as e: - if e.response and e.response.status_code == 409: + if e.response is not None and e.response.status_code == 409: raise_from( DockerFileCopyError('The container "{1}" has been paused ({0})'.format(e, container)), e, @@ -284,7 +284,15 @@ def _execute_command(client, container, command, log=None, check_rc=False): if 'detachKeys' in client._general_configs: data['detachKeys'] = client._general_configs['detachKeys'] - exec_data = client.post_json_to_json('/containers/{0}/exec', container, data=data) + try: + exec_data = client.post_json_to_json('/containers/{0}/exec', container, data=data) + except APIError as e: + if e.response is not None and e.response.status_code == 409: + raise_from( + DockerFileCopyError('Cannot execute command in paused container "{0}"'.format(container)), + e, + ) + raise exec_id = exec_data['Id'] data = { diff --git a/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml b/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml index d64cfb70b..c8e138e58 100644 --- a/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml +++ b/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml @@ -539,6 +539,15 @@ group_id: 910 register: result_4 +- name: Copy file (stopped container, no owner/group provided, should fail) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/file' + mode: 0707 + register: result_5 + ignore_errors: true + - name: Start container docker_container: name: "{{ cname }}" @@ -554,7 +563,7 @@ cat /file | base64; stat -c '%s %a %F %u %g %N' /file > /dev/stderr chdir: /root - register: result_5 + register: result_6 - name: Check results assert: @@ -563,8 +572,10 @@ - result_2 is changed - result_3 is not changed - result_4 is not changed - - result_5.stdout | b64decode == 'Content 1\n' - - result_5.stderr == '10 707 regular file 12 910 /file' + - result_5 is failed + - result_5.msg == ('Cannot execute command in paused container "' ~ cname ~ '"') + - result_6.stdout | b64decode == 'Content 1\n' + - result_6.stderr == '10 707 regular file 12 910 /file' ################################################################################################ # Cleanup From e452c9d4bf0a51a9a0b10fbc7ebddb182465e95d Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Thu, 5 Jan 2023 21:16:22 +0100 Subject: [PATCH 08/27] Fix formulation. Co-authored-by: Brian Scholer <1260690+briantist@users.noreply.github.com> --- plugins/modules/docker_container_copy_into.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py index 0231d7c8f..8af0b8043 100644 --- a/plugins/modules/docker_container_copy_into.py +++ b/plugins/modules/docker_container_copy_into.py @@ -29,7 +29,7 @@ options: container: description: - - The name of the container to copy files to/from. + - The name of the container to copy files to. type: str required: true path: From 3114fb79d7978a2a481bb3e11c66a2fb79e1d2c5 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Thu, 5 Jan 2023 22:44:30 +0100 Subject: [PATCH 09/27] Improve file comparison. --- plugins/modules/docker_container_copy_into.py | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py index 8af0b8043..4368354aa 100644 --- a/plugins/modules/docker_container_copy_into.py +++ b/plugins/modules/docker_container_copy_into.py @@ -136,6 +136,37 @@ ) +def are_fileobjs_equal(f1, f2): + '''Given two (buffered) file objects, compare their contents.''' + blocksize = 65536 + b1buf = b'' + b2buf = b'' + while True: + if f1 and len(b1buf) < blocksize: + f1b = f1.read1(blocksize) + if not f1b: + # f1 is EOF, so stop reading from it + f1 = None + b1buf += f1b + if f2 and len(b2buf) < blocksize: + f2b = f2.read1(blocksize) + if not f2b: + # f2 is EOF, so stop reading from it + f2 = None + b2buf += f2b + if not b1buf or not b2buf: + # At least one of f1 and f2 is EOF and all its data has + # been processed. If both are EOF and their data has been + # processed, the files are equal, otherwise not. + return not b1buf and not b2buf + # Compare the next chunk of data, and remove it from the buffers + buflen = min(len(b1buf), len(b2buf)) + if b1buf[:buflen] != b2buf[:buflen]: + return False + b1buf = b1buf[buflen:] + b2buf = b2buf[buflen:] + + def is_idempotent(client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode): # Retrieve information of local file try: @@ -217,7 +248,7 @@ def process_regular(in_path, tar, member): tar_f = tar.extractfile(member) # in Python 2, this *cannot* be used in `with`... with open(managed_path, 'rb') as local_f: - return container_path, mode, tar_f.read() == local_f.read() + return container_path, mode, are_fileobjs_equal(tar_f, local_f) def process_symlink(in_path, member): # Check things like user/group ID and mode From d5ac786e456d756a855f7f1a55c389c43f47d006 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Thu, 5 Jan 2023 22:47:26 +0100 Subject: [PATCH 10/27] Avoid reading whole file at once. --- plugins/module_utils/copy.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/module_utils/copy.py b/plugins/module_utils/copy.py index 764620700..f04945d3d 100644 --- a/plugins/module_utils/copy.py +++ b/plugins/module_utils/copy.py @@ -217,7 +217,7 @@ def process_regular(in_path, tar, member): in_f = tar.extractfile(member) # in Python 2, this *cannot* be used in `with`... with open(b_out_path, 'wb') as out_f: - shutil.copyfileobj(in_f, out_f, member.size) + shutil.copyfileobj(in_f, out_f) return in_path def process_symlink(in_path, member): From e06bd43ca481d743c021355d5e0a523cefe5a90d Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Thu, 5 Jan 2023 23:31:33 +0100 Subject: [PATCH 11/27] Stream when fetching files from daemon. --- plugins/module_utils/copy.py | 47 ++++++++++-- tests/unit/plugins/module_utils/test_copy.py | 78 ++++++++++++++++++++ 2 files changed, 117 insertions(+), 8 deletions(-) create mode 100644 tests/unit/plugins/module_utils/test_copy.py diff --git a/plugins/module_utils/copy.py b/plugins/module_utils/copy.py index f04945d3d..9d155a776 100644 --- a/plugins/module_utils/copy.py +++ b/plugins/module_utils/copy.py @@ -145,6 +145,44 @@ def f(client): return in_path, stat_data, None +class _RawGeneratorFileobj(io.RawIOBase): + def __init__(self, stream): + self._stream = stream + self._buf = b'' + + def readable(self): + return True + + def _readinto_from_buf(self, b, index, length): + cpy = min(length - index, len(self._buf)) + if cpy: + b[index:index + cpy] = self._buf[:cpy] + self._buf = self._buf[cpy:] + index += cpy + return index + + def readinto(self, b): + index = 0 + length = len(b) + + index = self._readinto_from_buf(b, index, length) + if index == length: + return index + + try: + self._buf += next(self._stream) + except StopIteration: + return index + + return self._readinto_from_buf(b, index, length) + + +def _stream_generator_to_fileobj(stream): + '''Given a generator that generates chunks of bytes, create a readable buffered stream.''' + raw = _RawGeneratorFileobj(stream) + return io.BufferedReader(raw) + + def fetch_file_ex(call_client, container, in_path, process_none, process_regular, process_symlink, follow_links=False, log=None): """Fetch a file (as a tar file entry) from a Docker container to local.""" considered_in_paths = set() @@ -167,14 +205,7 @@ def fetch_file_ex(call_client, container, in_path, process_none, process_regular except DockerFileNotFound: return process_none(in_path) - # TODO: stream tar file instead of downloading it into a BytesIO - - bio = io.BytesIO() - for chunk in stream: - bio.write(chunk) - bio.seek(0) - - with tarfile.open(fileobj=bio, mode='r|') as tar: + with tarfile.open(fileobj=_stream_generator_to_fileobj(stream), mode='r|') as tar: file_member = None symlink_member = None result = None diff --git a/tests/unit/plugins/module_utils/test_copy.py b/tests/unit/plugins/module_utils/test_copy.py new file mode 100644 index 000000000..4962cf733 --- /dev/null +++ b/tests/unit/plugins/module_utils/test_copy.py @@ -0,0 +1,78 @@ +# Copyright 2022 Red Hat | Ansible +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import pytest +import tarfile + +from ansible_collections.community.docker.plugins.module_utils.copy import ( + _stream_generator_to_fileobj, +) + + +def _simple_generator(sequence): + for elt in sequence: + yield elt + + +@pytest.mark.parametrize('chunks, read_sizes', [ + ( + [ + (1, b'1'), + (1, b'2'), + (1, b'3'), + (1, b'4'), + ], + [ + 1, + 2, + 3, + ] + ), + ( + [ + (1, b'123'), + (1, b'456'), + (1, b'789'), + ], + [ + 1, + 4, + 2, + 2, + 2, + ] + ), + ( + [ + (10 * 1024 * 1024, b'0'), + (10 * 1024 * 1024, b'1'), + ], + [ + 1024 * 1024 - 5, + 5 * 1024 * 1024 - 3, + 10 * 1024 * 1024 - 2, + 2 * 1024 * 1024 - 1, + 2 * 1024 * 1024 + 5 + 3 + 2 + 1, + ] + ), +]) +def test__stream_generator_to_fileobj(chunks, read_sizes): + chunks = [count * data for count, data in chunks] + stream = _simple_generator(chunks) + expected = b''.join(chunks) + + buffer = b'' + totally_read = 0 + f = _stream_generator_to_fileobj(stream) + for read_size in read_sizes: + chunk = f.read(read_size) + assert len(chunk) == min(read_size, len(expected) - len(buffer)) + buffer += chunk + totally_read += read_size + + assert buffer == expected[:len(buffer)] + assert min(totally_read, len(expected)) == len(buffer) From ace7c6107a0fa98c64bbd930ae4fecb3ea02aad9 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Fri, 6 Jan 2023 00:00:29 +0100 Subject: [PATCH 12/27] Fix comment. --- plugins/module_utils/copy.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/plugins/module_utils/copy.py b/plugins/module_utils/copy.py index 9d155a776..20846af56 100644 --- a/plugins/module_utils/copy.py +++ b/plugins/module_utils/copy.py @@ -37,8 +37,7 @@ class DockerFileNotFound(DockerFileCopyError): def _put_archive(client, container, path, data): # data can also be file object for streaming. This is because _put uses requests's put(). - # See https://2.python-requests.org/en/master/user/advanced/#streaming-uploads - # WARNING: might not work with all transports! + # See https://requests.readthedocs.io/en/latest/user/advanced/#streaming-uploads url = client._url('/containers/{0}/archive', container) res = client._put(url, params={'path': path}, data=data) client._raise_for_status(res) From ce4644850b005debdb611826c4a740207d949d36 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Fri, 6 Jan 2023 00:36:16 +0100 Subject: [PATCH 13/27] Use read() instead of read1(). --- plugins/modules/docker_container_copy_into.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py index 4368354aa..804976256 100644 --- a/plugins/modules/docker_container_copy_into.py +++ b/plugins/modules/docker_container_copy_into.py @@ -143,13 +143,13 @@ def are_fileobjs_equal(f1, f2): b2buf = b'' while True: if f1 and len(b1buf) < blocksize: - f1b = f1.read1(blocksize) + f1b = f1.read(blocksize) if not f1b: # f1 is EOF, so stop reading from it f1 = None b1buf += f1b if f2 and len(b2buf) < blocksize: - f2b = f2.read1(blocksize) + f2b = f2.read(blocksize) if not f2b: # f2 is EOF, so stop reading from it f2 = None From 052c7f6214a619768ffe289fe106246918cc5eff Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Fri, 6 Jan 2023 14:46:15 +0100 Subject: [PATCH 14/27] Stream files when copying into container. --- changelogs/fragments/545-docker_api.yml | 2 + plugins/module_utils/copy.py | 113 +++++++++++++----- plugins/modules/docker_container_copy_into.py | 1 - 3 files changed, 87 insertions(+), 29 deletions(-) create mode 100644 changelogs/fragments/545-docker_api.yml diff --git a/changelogs/fragments/545-docker_api.yml b/changelogs/fragments/545-docker_api.yml new file mode 100644 index 000000000..65b9f5401 --- /dev/null +++ b/changelogs/fragments/545-docker_api.yml @@ -0,0 +1,2 @@ +minor_changes: + - "docker_api connection plugin - when copying files to/from a container, stream the file contents instead of first reading them to memory (https://github.com/ansible-collections/community.docker/pull/545)." diff --git a/plugins/module_utils/copy.py b/plugins/module_utils/copy.py index 20846af56..586168733 100644 --- a/plugins/module_utils/copy.py +++ b/plugins/module_utils/copy.py @@ -11,6 +11,7 @@ import os import os.path import shutil +import stat import tarfile from ansible.module_utils.common.text.converters import to_bytes, to_native, to_text @@ -44,20 +45,11 @@ def _put_archive(client, container, path, data): return res.status_code == 200 -def put_file(call_client, container, in_path, out_path, user_id, group_id, mode=None, user_name=None, follow_links=False, log=None): - """Transfer a file from local to Docker container.""" - if not os.path.exists(to_bytes(in_path, errors='surrogate_or_strict')): - raise DockerFileNotFound( - "file or module does not exist: %s" % to_native(in_path)) - - b_in_path = to_bytes(in_path, errors='surrogate_or_strict') - - out_dir, out_file = os.path.split(out_path) - - # TODO: stream tar file, instead of creating it in-memory into a BytesIO - +def _symlink_tar_creator(b_in_path, file_stat, out_file, user_id, group_id, mode=None, user_name=None): + if not stat.S_ISLNK(file_stat.st_mode): + raise DockerUnexpectedError('stat information is not for a symlink') bio = io.BytesIO() - with tarfile.open(fileobj=bio, mode='w|', dereference=follow_links, encoding='utf-8') as tar: + with tarfile.open(fileobj=bio, mode='w|', dereference=False, encoding='utf-8') as tar: # Note that without both name (bytes) and arcname (unicode), this either fails for # Python 2.7, Python 3.5/3.6, or Python 3.7+. Only when passing both (in this # form) it works with Python 2.7, 3.5, 3.6, and 3.7 up to 3.11 @@ -71,21 +63,86 @@ def put_file(call_client, container, in_path, out_path, user_id, group_id, mode= tarinfo.mode &= 0o700 if mode is not None: tarinfo.mode = mode - if os.path.isfile(b_in_path) or follow_links: - with open(b_in_path, 'rb') as f: - tar.addfile(tarinfo, fileobj=f) - else: - tar.addfile(tarinfo) - data = bio.getvalue() - - ok = call_client( - lambda client: _put_archive( - client, - container, - out_dir, - data, - ) - ) + if not tarinfo.issym(): + raise DockerUnexpectedError('stat information is not for a symlink') + tar.addfile(tarinfo) + return bio.getvalue() + + +def _symlink_tar_generator(b_in_path, file_stat, out_file, user_id, group_id, mode=None, user_name=None): + yield _symlink_tar_creator(b_in_path, file_stat, out_file, user_id, group_id, mode, user_name) + + +def _regular_file_tar_generator(b_in_path, file_stat, out_file, user_id, group_id, mode=None, user_name=None): + if not stat.S_ISREG(file_stat.st_mode): + raise DockerUnexpectedError('stat information is not for a regular file') + tarinfo = tarfile.TarInfo() + tarinfo.name = os.path.splitdrive(to_text(out_file))[1].replace(os.sep, '/').lstrip('/') + tarinfo.mode = (file_stat.st_mode & 0o700) if mode is None else mode + tarinfo.uid = user_id + tarinfo.gid = group_id + tarinfo.size = file_stat.st_size + tarinfo.mtime = file_stat.st_mtime + tarinfo.type = tarfile.REGTYPE + tarinfo.linkname = '' + + tarinfo_buf = tarinfo.tobuf() + total_size = len(tarinfo_buf) + yield tarinfo_buf + + remainder = tarinfo.size % tarfile.BLOCKSIZE + size = tarinfo.size + total_size += size + with open(b_in_path, 'rb') as f: + while size > 0: + to_read = min(size, 65536) + buf = f.read(to_read) + if not buf: + break + size -= len(buf) + yield buf + + if size: + # If for some reason the file shrunk, fill up to the announced size with zeros. + # (If it enlarged, ignore the remainder.) + yield tarfile.NUL * size + if remainder: + # We need to write a multiple of 512 bytes. Fill up with zeros. + yield tarfile.NUL * (tarfile.BLOCKSIZE - remainder) + total_size += tarfile.BLOCKSIZE - remainder + + # End with two zeroed blocks + yield tarfile.NUL * (2 * tarfile.BLOCKSIZE) + total_size += 2 * tarfile.BLOCKSIZE + + remainder = total_size % tarfile.RECORDSIZE + if remainder > 0: + yield tarfile.NUL * (tarfile.RECORDSIZE - remainder) + + +def put_file(call_client, container, in_path, out_path, user_id, group_id, mode=None, user_name=None, follow_links=False, log=None): + """Transfer a file from local to Docker container.""" + if not os.path.exists(to_bytes(in_path, errors='surrogate_or_strict')): + raise DockerFileNotFound( + "file or module does not exist: %s" % to_native(in_path)) + + b_in_path = to_bytes(in_path, errors='surrogate_or_strict') + + out_dir, out_file = os.path.split(out_path) + + if follow_links: + file_stat = os.stat(b_in_path) + else: + file_stat = os.lstat(b_in_path) + + if stat.S_ISREG(file_stat.st_mode): + stream = _regular_file_tar_generator(b_in_path, file_stat, out_file, user_id, group_id, mode=mode, user_name=user_name) + elif stat.S_ISLNK(file_stat.st_mode): + stream = _symlink_tar_generator(b_in_path, file_stat, out_file, user_id, group_id, mode=mode, user_name=user_name) + else: + raise DockerFileCopyError('File{0} {1} is neither a regular file nor a symlink (stat mode {2}).'.format(' referenced by' if follow_links else '', in_path, oct(file_stat.st_mode))) + + ok = call_client(lambda client: _put_archive(client, container, out_dir, stream)) if not ok: raise DockerFileCopyError('Unknown error while creating file "{0}" in container "{1}".'.format(out_path, container)) diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py index 804976256..df5431b2e 100644 --- a/plugins/modules/docker_container_copy_into.py +++ b/plugins/modules/docker_container_copy_into.py @@ -17,7 +17,6 @@ description: - Copy a file into a Docker container. - Similar to C(docker cp). - - The file is processed in-memory. Do not use for large files! - To copy files in a non-running container, you must provide the I(owner_id) and I(group_id) options. attributes: From 1280703e2cc35ee3ddcd2a60b6294d58a1e18a86 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Fri, 6 Jan 2023 14:50:46 +0100 Subject: [PATCH 15/27] Linting. --- plugins/module_utils/copy.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/plugins/module_utils/copy.py b/plugins/module_utils/copy.py index 586168733..050c3013c 100644 --- a/plugins/module_utils/copy.py +++ b/plugins/module_utils/copy.py @@ -85,7 +85,7 @@ def _regular_file_tar_generator(b_in_path, file_stat, out_file, user_id, group_i tarinfo.mtime = file_stat.st_mtime tarinfo.type = tarfile.REGTYPE tarinfo.linkname = '' - + tarinfo_buf = tarinfo.tobuf() total_size = len(tarinfo_buf) yield tarinfo_buf @@ -140,7 +140,9 @@ def put_file(call_client, container, in_path, out_path, user_id, group_id, mode= elif stat.S_ISLNK(file_stat.st_mode): stream = _symlink_tar_generator(b_in_path, file_stat, out_file, user_id, group_id, mode=mode, user_name=user_name) else: - raise DockerFileCopyError('File{0} {1} is neither a regular file nor a symlink (stat mode {2}).'.format(' referenced by' if follow_links else '', in_path, oct(file_stat.st_mode))) + raise DockerFileCopyError( + 'File{0} {1} is neither a regular file nor a symlink (stat mode {2}).'.format( + ' referenced by' if follow_links else '', in_path, oct(file_stat.st_mode))) ok = call_client(lambda client: _put_archive(client, container, out_dir, stream)) if not ok: From a702fa7845826d0af5379c0c79d63acccd0ae516 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sat, 7 Jan 2023 17:31:36 +0100 Subject: [PATCH 16/27] Add force parameter. --- plugins/modules/docker_container_copy_into.py | 23 +++- .../tasks/tests/simple.yml | 111 +++++++++++++++++- 2 files changed, 129 insertions(+), 5 deletions(-) diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py index df5431b2e..3759c7ba8 100644 --- a/plugins/modules/docker_container_copy_into.py +++ b/plugins/modules/docker_container_copy_into.py @@ -73,6 +73,11 @@ - The file mode to use when writing the file to disk. - Will use the file's mode from the source system if this option is not provided. type: int + force: + description: + - Force writing the file (without performing any idempotency checks). + type: bool + default: false extends_documentation_fragment: - community.docker.docker.api_documentation @@ -166,7 +171,7 @@ def are_fileobjs_equal(f1, f2): b2buf = b2buf[buflen:] -def is_idempotent(client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode): +def is_idempotent(client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode, force=False): # Retrieve information of local file try: file_stat = os.stat(managed_path) if local_follow_links else os.lstat(managed_path) @@ -179,6 +184,11 @@ def is_idempotent(client, container, managed_path, container_path, follow_links, if not stat.S_ISLNK(file_stat.st_mode) and not stat.S_ISREG(file_stat.st_mode): raise DockerFileCopyError('Local path {managed_path} is not a symbolic link or file') + # When forcing and we're not following links in the container, go! + if force and not follow_links: + return container_path, mode, False + + # Resolve symlinks in the container (if requested), and get information on container's file real_container_path, regular_stat, link_target = stat_file( call_client(client, container), container, @@ -190,6 +200,10 @@ def is_idempotent(client, container, managed_path, container_path, follow_links, if follow_links: container_path = real_container_path + # When forcing, go! + if force: + return container_path, mode, False + # If the file wasn't found, continue if regular_stat is None: return container_path, mode, False @@ -275,9 +289,9 @@ def process_symlink(in_path, member): ) -def copy_into_container(client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode): +def copy_into_container(client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode, force=False): container_path, mode, idempotent = is_idempotent( - client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode) + client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode, force=force) changed = not idempotent if changed and not client.module.check_mode: @@ -308,6 +322,7 @@ def main(): owner_id=dict(type='int'), group_id=dict(type='int'), mode=dict(type='int'), + force=dict(type='bool', default=False), ) client = AnsibleDockerClient( @@ -325,6 +340,7 @@ def main(): owner_id = client.module.params['owner_id'] group_id = client.module.params['group_id'] mode = client.module.params['mode'] + force = client.module.params['force'] if not container_path.startswith(os.path.sep): container_path = os.path.join(os.path.sep, container_path) @@ -344,6 +360,7 @@ def main(): owner_id=owner_id, group_id=group_id, mode=mode, + force=force, ) except DockerUnexpectedError as exc: client.fail('Unexpected error: {exc}'.format(exc=to_native(exc)), exception=traceback.format_exc()) diff --git a/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml b/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml index c8e138e58..dc5af2af2 100644 --- a/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml +++ b/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml @@ -108,6 +108,35 @@ chdir: /root register: result_5 +- name: Copy file (force, check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/file' + force: true + check_mode: true + register: result_6 + +- name: Copy file (force) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/file' + force: true + register: result_7 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /file | base64; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_8 + - name: Check results assert: that: @@ -117,6 +146,10 @@ - result_4 is not changed - result_5.stdout | b64decode == 'Content 1\n' - result_5.stderr == '10 644 regular file 0 0 /file' + - result_6 is changed + - result_7 is changed + - result_8.stdout | b64decode == 'Content 1\n' + - result_8.stderr == '10 644 regular file 0 0 /file' ######################### Follow link - idempotence @@ -141,7 +174,7 @@ check_mode: true register: result_1 -- name: Copy file following link (check mode) +- name: Copy file following link docker_container_copy_into: container: '{{ cname }}' path: '{{ remote_tmp_dir }}/file_1' @@ -162,6 +195,38 @@ chdir: /root register: result_3 +- name: Copy file following link (force, check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/lnk' + follow: true + force: true + check_mode: true + register: result_4 + +- name: Copy file following link (force) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/lnk' + follow: true + force: true + register: result_5 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /lnk | base64; + stat -c '%s %a %F %u %g %N' /lnk > /dev/stderr; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_6 + - name: Check results assert: that: @@ -174,6 +239,13 @@ - result_3.stdout | b64decode == 'Content 1\n' - result_3.stderr_lines[0] == "4 777 symbolic link 0 0 '/lnk' -> 'file'" - result_3.stderr_lines[1] == '10 644 regular file 0 0 /file' + - result_4 is changed + - result_4.container_path == '/file' + - result_5 is changed + - result_5.container_path == '/file' + - result_6.stdout | b64decode == 'Content 1\n' + - result_6.stderr_lines[0] == "4 777 symbolic link 0 0 '/lnk' -> 'file'" + - result_6.stderr_lines[1] == '10 644 regular file 0 0 /file' ######################### Do not follow link - replace by file @@ -186,7 +258,7 @@ check_mode: true register: result_1 -- name: Copy file not following link (check mode) +- name: Copy file not following link docker_container_copy_into: container: '{{ cname }}' path: '{{ remote_tmp_dir }}/file_1' @@ -221,6 +293,35 @@ chdir: /root register: result_5 +- name: Copy file not following link (force, check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/lnk' + force: true + check_mode: true + register: result_6 + +- name: Copy file not following link (force) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/lnk' + force: true + register: result_7 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /lnk | base64; + stat -c '%s %a %F %u %g %N' /lnk > /dev/stderr + chdir: /root + register: result_8 + - name: Check results assert: that: @@ -232,6 +333,12 @@ - result_4 is not changed - result_5.stdout | b64decode == 'Content 1\n' - result_5.stderr == '10 644 regular file 0 0 /lnk' + - result_6 is changed + - result_6.container_path == '/lnk' + - result_7 is changed + - result_7.container_path == '/lnk' + - result_8.stdout | b64decode == 'Content 1\n' + - result_8.stderr == '10 644 regular file 0 0 /lnk' ######################### Replace directory by file From bed4d807772b24347705e4c910712e47ec04f63f Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sun, 8 Jan 2023 09:53:48 +0100 Subject: [PATCH 17/27] Simplify library code. --- plugins/connection/docker_api.py | 39 +++--- plugins/module_utils/copy.py | 116 ++++++------------ plugins/modules/docker_container_copy_into.py | 21 +++- 3 files changed, 72 insertions(+), 104 deletions(-) diff --git a/plugins/connection/docker_api.py b/plugins/connection/docker_api.py index e911dc04c..14d9d190f 100644 --- a/plugins/connection/docker_api.py +++ b/plugins/connection/docker_api.py @@ -287,16 +287,18 @@ def put_file(self, in_path, out_path): user_id, group_id = self.ids[self.actual_user] try: - put_file( - call_client=lambda callback: self._call_client(lambda: callback(self.client), not_found_can_be_resource=True), - container=self.get_option('remote_addr'), - in_path=in_path, - out_path=out_path, - user_id=user_id, - group_id=group_id, - user_name=self.actual_user, - follow_links=True, - log=lambda msg: display.vvvv(msg, host=self.get_option('remote_addr')), + self._call_client( + lambda: put_file( + self.client, + container=self.get_option('remote_addr'), + in_path=in_path, + out_path=out_path, + user_id=user_id, + group_id=group_id, + user_name=self.actual_user, + follow_links=True, + ), + not_found_can_be_resource=True, ) except DockerFileNotFound as exc: raise AnsibleFileNotFound(to_native(exc)) @@ -311,13 +313,16 @@ def fetch_file(self, in_path, out_path): in_path = self._prefix_login_path(in_path) try: - fetch_file( - call_client=lambda callback: self._call_client(lambda: callback(self.client), not_found_can_be_resource=True), - container=self.get_option('remote_addr'), - in_path=in_path, - out_path=out_path, - follow_links=True, - log=lambda msg: display.vvvv(msg, host=self.get_option('remote_addr')), + self._call_client( + lambda: fetch_file( + self.client, + container=self.get_option('remote_addr'), + in_path=in_path, + out_path=out_path, + follow_links=True, + log=lambda msg: display.vvvv(msg, host=self.get_option('remote_addr')), + ), + not_found_can_be_resource=True, ) except DockerFileNotFound as exc: raise AnsibleFileNotFound(to_native(exc)) diff --git a/plugins/module_utils/copy.py b/plugins/module_utils/copy.py index 050c3013c..4e47a1383 100644 --- a/plugins/module_utils/copy.py +++ b/plugins/module_utils/copy.py @@ -17,11 +17,7 @@ from ansible.module_utils.common.text.converters import to_bytes, to_native, to_text from ansible.module_utils.six import raise_from -from ansible_collections.community.docker.plugins.module_utils.common_api import ( - RequestException, -) - -from ansible_collections.community.docker.plugins.module_utils._api.errors import APIError, DockerException, NotFound +from ansible_collections.community.docker.plugins.module_utils._api.errors import APIError, NotFound class DockerFileCopyError(Exception): @@ -85,12 +81,13 @@ def _regular_file_tar_generator(b_in_path, file_stat, out_file, user_id, group_i tarinfo.mtime = file_stat.st_mtime tarinfo.type = tarfile.REGTYPE tarinfo.linkname = '' + if user_name: + tarinfo.uname = user_name tarinfo_buf = tarinfo.tobuf() total_size = len(tarinfo_buf) yield tarinfo_buf - remainder = tarinfo.size % tarfile.BLOCKSIZE size = tarinfo.size total_size += size with open(b_in_path, 'rb') as f: @@ -101,11 +98,12 @@ def _regular_file_tar_generator(b_in_path, file_stat, out_file, user_id, group_i break size -= len(buf) yield buf - if size: # If for some reason the file shrunk, fill up to the announced size with zeros. # (If it enlarged, ignore the remainder.) yield tarfile.NUL * size + + remainder = tarinfo.size % tarfile.BLOCKSIZE if remainder: # We need to write a multiple of 512 bytes. Fill up with zeros. yield tarfile.NUL * (tarfile.BLOCKSIZE - remainder) @@ -120,7 +118,7 @@ def _regular_file_tar_generator(b_in_path, file_stat, out_file, user_id, group_i yield tarfile.NUL * (tarfile.RECORDSIZE - remainder) -def put_file(call_client, container, in_path, out_path, user_id, group_id, mode=None, user_name=None, follow_links=False, log=None): +def put_file(client, container, in_path, out_path, user_id, group_id, mode=None, user_name=None, follow_links=False): """Transfer a file from local to Docker container.""" if not os.path.exists(to_bytes(in_path, errors='surrogate_or_strict')): raise DockerFileNotFound( @@ -144,12 +142,12 @@ def put_file(call_client, container, in_path, out_path, user_id, group_id, mode= 'File{0} {1} is neither a regular file nor a symlink (stat mode {2}).'.format( ' referenced by' if follow_links else '', in_path, oct(file_stat.st_mode))) - ok = call_client(lambda client: _put_archive(client, container, out_dir, stream)) + ok = _put_archive(client, container, out_dir, stream) if not ok: - raise DockerFileCopyError('Unknown error while creating file "{0}" in container "{1}".'.format(out_path, container)) + raise DockerUnexpectedError('Unknown error while creating file "{0}" in container "{1}".'.format(out_path, container)) -def stat_file(call_client, container, in_path, follow_links=False, log=None): +def stat_file(client, container, in_path, follow_links=False, log=None): """Fetch information on a file from a Docker container to local. Return a tuple ``(path, stat_data, link_target)`` where: @@ -171,26 +169,21 @@ def stat_file(call_client, container, in_path, follow_links=False, log=None): if log: log('FETCH: Stating "%s"' % in_path) - def f(client): - response = client._head( - client._url('/containers/{0}/archive', container), - params={'path': in_path}, - ) - if response.status_code == 404: - return None - client._raise_for_status(response) - header = response.headers.get('x-docker-container-path-stat') - try: - return json.loads(base64.b64decode(header)) - except Exception as exc: - raise DockerUnexpectedError( - 'When retrieving information for {in_path} from {container}, obtained header {header!r} that cannot be loaded as JSON: {exc}' - .format(in_path=in_path, container=container, header=header, exc=exc) - ) - - stat_data = call_client(f) - if stat_data is None: + response = client._head( + client._url('/containers/{0}/archive', container), + params={'path': in_path}, + ) + if response.status_code == 404: return in_path, None, None + client._raise_for_status(response) + header = response.headers.get('x-docker-container-path-stat') + try: + stat_data = json.loads(base64.b64decode(header)) + except Exception as exc: + raise DockerUnexpectedError( + 'When retrieving information for {in_path} from {container}, obtained header {header!r} that cannot be loaded as JSON: {exc}' + .format(in_path=in_path, container=container, header=header, exc=exc) + ) # https://pkg.go.dev/io/fs#FileMode: bit 32 - 5 means ModeSymlink if stat_data['mode'] & (1 << (32 - 5)) != 0: @@ -241,7 +234,7 @@ def _stream_generator_to_fileobj(stream): return io.BufferedReader(raw) -def fetch_file_ex(call_client, container, in_path, process_none, process_regular, process_symlink, follow_links=False, log=None): +def fetch_file_ex(client, container, in_path, process_none, process_regular, process_symlink, follow_links=False, log=None): """Fetch a file (as a tar file entry) from a Docker container to local.""" considered_in_paths = set() @@ -253,24 +246,21 @@ def fetch_file_ex(call_client, container, in_path, process_none, process_regular if log: log('FETCH: Fetching "%s"' % in_path) try: - stream = call_client( - lambda client: client.get_raw_stream( - '/containers/{0}/archive', container, - params={'path': in_path}, - headers={'Accept-Encoding': 'identity'}, - ) + stream = client.get_raw_stream( + '/containers/{0}/archive', container, + params={'path': in_path}, + headers={'Accept-Encoding': 'identity'}, ) - except DockerFileNotFound: + except NotFound as dummy: return process_none(in_path) with tarfile.open(fileobj=_stream_generator_to_fileobj(stream), mode='r|') as tar: - file_member = None symlink_member = None result = None found = False for member in tar: if found: - raise DockerFileCopyError('Received tarfile contains more than one file!') + raise DockerUnexpectedError('Received tarfile contains more than one file!') found = True if member.issym(): symlink_member = member @@ -291,7 +281,7 @@ def fetch_file_ex(call_client, container, in_path, process_none, process_regular raise DockerUnexpectedError('Received tarfile is empty!') -def fetch_file(call_client, container, in_path, out_path, follow_links=False, log=None): +def fetch_file(client, container, in_path, out_path, follow_links=False, log=None): b_out_path = to_bytes(out_path, errors='surrogate_or_strict') def process_none(in_path): @@ -316,43 +306,7 @@ def process_symlink(in_path, member): os.symlink(member.linkname, b_out_path) return in_path - return fetch_file_ex(call_client, container, in_path, process_none, process_regular, process_symlink, follow_links=follow_links, log=log) - - -def call_client(client, container, use_file_not_found_exception=False): - def f(callback): - try: - return callback(client) - except NotFound as e: - if use_file_not_found_exception: - raise_from(DockerFileNotFound(to_native(e)), e) - raise_from( - DockerFileCopyError('Could not find container "{1}" or resource in it ({0})'.format(e, container)), - e, - ) - except APIError as e: - if e.response is not None and e.response.status_code == 409: - raise_from( - DockerFileCopyError('The container "{1}" has been paused ({0})'.format(e, container)), - e, - ) - raise_from( - DockerUnexpectedError('An unexpected Docker error occurred for container "{1}": {0}'.format(e, container)), - e, - ) - except DockerException as e: - raise_from( - DockerUnexpectedError('An unexpected Docker error occurred for container "{1}": {0}'.format(e, container)), - e, - ) - except RequestException as e: - raise_from( - DockerUnexpectedError( - 'An unexpected requests error occurred for container "{1}" when trying to talk to the Docker daemon: {0}'.format(e, container)), - e, - ) - - return f + return fetch_file_ex(client, container, in_path, process_none, process_regular, process_symlink, follow_links=follow_links, log=log) def _execute_command(client, container, command, log=None, check_rc=False): @@ -400,7 +354,7 @@ def _execute_command(client, container, command, log=None, check_rc=False): log('Exit code {rc}, stdout {stdout!r}, stderr {stderr!r}'.format(rc=rc, stdout=stdout, stderr=stderr)) if check_rc and rc != 0: - raise DockerFileCopyError( + raise DockerUnexpectedError( 'Obtained unexpected exit code {rc} when running "{command}" in {container}.\nSTDOUT: {stdout}\nSTDERR: {stderr}' .format(command=' '.join(command), container=container, rc=rc, stdout=stdout, stderr=stderr) ) @@ -413,7 +367,7 @@ def determine_user_group(client, container, log=None): stdout_lines = stdout.splitlines() if len(stdout_lines) != 2: - raise DockerFileCopyError( + raise DockerUnexpectedError( 'Expected two-line output to obtain user and group ID for container {container}, but got {lc} lines:\n{stdout}' .format(container=container, lc=len(stdout_lines), stdout=stdout) ) @@ -422,7 +376,7 @@ def determine_user_group(client, container, log=None): try: return int(user_id), int(group_id) except ValueError: - raise DockerFileCopyError( + raise DockerUnexpectedError( 'Expected two-line output with numeric IDs to obtain user and group ID for container {container}, but got "{l1}" and "{l2}" instead' .format(container=container, l1=user_id, l2=group_id) ) diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py index 3759c7ba8..04085f849 100644 --- a/plugins/modules/docker_container_copy_into.py +++ b/plugins/modules/docker_container_copy_into.py @@ -121,7 +121,9 @@ import stat import traceback -from ansible.module_utils._text import to_bytes, to_text, to_native +from ansible.module_utils._text import to_bytes, to_native + +from ansible_collections.community.docker.plugins.module_utils._api.errors import APIError, DockerException, NotFound from ansible_collections.community.docker.plugins.module_utils.common_api import ( AnsibleDockerClient, @@ -132,7 +134,6 @@ DockerFileCopyError, DockerFileNotFound, DockerUnexpectedError, - call_client, determine_user_group, fetch_file_ex, put_file, @@ -190,7 +191,7 @@ def is_idempotent(client, container, managed_path, container_path, follow_links, # Resolve symlinks in the container (if requested), and get information on container's file real_container_path, regular_stat, link_target = stat_file( - call_client(client, container), + client, container, in_path=container_path, follow_links=follow_links, @@ -279,7 +280,7 @@ def process_symlink(in_path, member): return container_path, mode, member.linkname == local_link_target return fetch_file_ex( - call_client(client, container, use_file_not_found_exception=True), + client, container, in_path=container_path, process_none=process_none, @@ -296,7 +297,7 @@ def copy_into_container(client, container, managed_path, container_path, follow_ if changed and not client.module.check_mode: put_file( - call_client(client, container), + client, container, in_path=managed_path, out_path=container_path, @@ -348,7 +349,7 @@ def main(): try: if owner_id is None or group_id is None: - owner_id, group_id = call_client(client, container)(lambda c: determine_user_group(c, container)) + owner_id, group_id = determine_user_group(client, container) copy_into_container( client, @@ -362,6 +363,14 @@ def main(): mode=mode, force=force, ) + except NotFound as exc: + client.fail('Could not find container "{1}" or resource in it ({0})'.format(exc, container), exception=traceback.format_exc()) + except APIError as exc: + client.fail('An unexpected Docker error occurred for container "{1}": {0}'.format(exc, container), exception=traceback.format_exc()) + except DockerException as exc: + client.fail('An unexpected Docker error occurred for container "{1}": {0}'.format(exc, container), exception=traceback.format_exc()) + except RequestException as exc: + client.fail('An unexpected requests error occurred for container "{1}" when trying to talk to the Docker daemon: {0}'.format(exc, container), exception=traceback.format_exc()) except DockerUnexpectedError as exc: client.fail('Unexpected error: {exc}'.format(exc=to_native(exc)), exception=traceback.format_exc()) except DockerFileCopyError as exc: From e66292cb0a521c6cf9f42416f0b0fe76dea30813 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sun, 8 Jan 2023 10:02:51 +0100 Subject: [PATCH 18/27] Linting. --- plugins/modules/docker_container_copy_into.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py index 04085f849..b51d7f20d 100644 --- a/plugins/modules/docker_container_copy_into.py +++ b/plugins/modules/docker_container_copy_into.py @@ -370,7 +370,9 @@ def main(): except DockerException as exc: client.fail('An unexpected Docker error occurred for container "{1}": {0}'.format(exc, container), exception=traceback.format_exc()) except RequestException as exc: - client.fail('An unexpected requests error occurred for container "{1}" when trying to talk to the Docker daemon: {0}'.format(exc, container), exception=traceback.format_exc()) + client.fail( + 'An unexpected requests error occurred for container "{1}" when trying to talk to the Docker daemon: {0}'.format(exc, container), + exception=traceback.format_exc()) except DockerUnexpectedError as exc: client.fail('Unexpected error: {exc}'.format(exc=to_native(exc)), exception=traceback.format_exc()) except DockerFileCopyError as exc: From f3a2bcb20d332f84ff7ef7c9ecbba3e2088f620d Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sun, 8 Jan 2023 11:14:46 +0100 Subject: [PATCH 19/27] Add content and content_is_b64 options. --- plugins/module_utils/copy.py | 52 ++ plugins/modules/docker_container_copy_into.py | 226 ++++- .../tasks/tests/content.yml | 770 ++++++++++++++++++ .../tasks/tests/{simple.yml => file.yml} | 0 4 files changed, 1010 insertions(+), 38 deletions(-) create mode 100644 tests/integration/targets/docker_container_copy_into/tasks/tests/content.yml rename tests/integration/targets/docker_container_copy_into/tasks/tests/{simple.yml => file.yml} (100%) diff --git a/plugins/module_utils/copy.py b/plugins/module_utils/copy.py index 4e47a1383..4f06c3b42 100644 --- a/plugins/module_utils/copy.py +++ b/plugins/module_utils/copy.py @@ -6,6 +6,7 @@ __metaclass__ = type import base64 +import datetime import io import json import os @@ -118,6 +119,46 @@ def _regular_file_tar_generator(b_in_path, file_stat, out_file, user_id, group_i yield tarfile.NUL * (tarfile.RECORDSIZE - remainder) +def _regular_content_tar_generator(content, out_file, user_id, group_id, mode, user_name=None): + tarinfo = tarfile.TarInfo() + tarinfo.name = os.path.splitdrive(to_text(out_file))[1].replace(os.sep, '/').lstrip('/') + tarinfo.mode = mode + tarinfo.uid = user_id + tarinfo.gid = group_id + tarinfo.size = len(content) + try: + tarinfo.mtime = int(datetime.datetime.now().timestamp()) + except AttributeError: + # Python 2 (or more precisely: Python < 3.3) has no timestamp(). Use the following + # expression for Python 2: + tarinfo.mtime = int((datetime.datetime.utcnow() - datetime.datetime(1970, 1, 1)).total_seconds()) + tarinfo.type = tarfile.REGTYPE + tarinfo.linkname = '' + if user_name: + tarinfo.uname = user_name + + tarinfo_buf = tarinfo.tobuf() + total_size = len(tarinfo_buf) + yield tarinfo_buf + + total_size += len(content) + yield content + + remainder = tarinfo.size % tarfile.BLOCKSIZE + if remainder: + # We need to write a multiple of 512 bytes. Fill up with zeros. + yield tarfile.NUL * (tarfile.BLOCKSIZE - remainder) + total_size += tarfile.BLOCKSIZE - remainder + + # End with two zeroed blocks + yield tarfile.NUL * (2 * tarfile.BLOCKSIZE) + total_size += 2 * tarfile.BLOCKSIZE + + remainder = total_size % tarfile.RECORDSIZE + if remainder > 0: + yield tarfile.NUL * (tarfile.RECORDSIZE - remainder) + + def put_file(client, container, in_path, out_path, user_id, group_id, mode=None, user_name=None, follow_links=False): """Transfer a file from local to Docker container.""" if not os.path.exists(to_bytes(in_path, errors='surrogate_or_strict')): @@ -147,6 +188,17 @@ def put_file(client, container, in_path, out_path, user_id, group_id, mode=None, raise DockerUnexpectedError('Unknown error while creating file "{0}" in container "{1}".'.format(out_path, container)) +def put_file_content(client, container, content, out_path, user_id, group_id, mode, user_name=None): + """Transfer a file from local to Docker container.""" + out_dir, out_file = os.path.split(out_path) + + stream = _regular_content_tar_generator(content, out_file, user_id, group_id, mode, user_name=user_name) + + ok = _put_archive(client, container, out_dir, stream) + if not ok: + raise DockerUnexpectedError('Unknown error while creating file "{0}" in container "{1}".'.format(out_path, container)) + + def stat_file(client, container, in_path, follow_links=False, log=None): """Fetch information on a file from a Docker container to local. diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py index b51d7f20d..1c9f8d8a0 100644 --- a/plugins/modules/docker_container_copy_into.py +++ b/plugins/modules/docker_container_copy_into.py @@ -34,8 +34,23 @@ path: description: - Path to a file on the managed node. + - Mutually exclusive with I(content). One of I(content) and I(path) is required. type: path - required: true + content: + description: + - The file's content. + - If you plan to provide binary data, use the I(content_is_b64) option. + - Mutually exclusive with I(path). One of I(content) and I(path) is required. + type: str + content_is_b64: + description: + - If set to C(true), the content in I(content) is assumed to be Base64 encoded and + will be decoded before being used. + - To use binary I(content), it is better to keep it Base64 encoded and let it + be decoded by this option. Otherwise you risk the data to be interpreted as + UTF-8 and corrupted. + type: bool + default: false container_path: description: - Path to a file inside the Docker container. @@ -117,6 +132,8 @@ returned: success ''' +import base64 +import io import os import stat import traceback @@ -137,6 +154,7 @@ determine_user_group, fetch_file_ex, put_file, + put_file_content, stat_file, ) @@ -172,7 +190,35 @@ def are_fileobjs_equal(f1, f2): b2buf = b2buf[buflen:] -def is_idempotent(client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode, force=False): +def is_container_file_not_regular_file(container_stat): + for bit in ( + # https://pkg.go.dev/io/fs#FileMode + 32 - 1, # ModeDir + 32 - 4, # ModeTemporary + 32 - 5, # ModeSymlink + 32 - 6, # ModeDevice + 32 - 7, # ModeNamedPipe + 32 - 8, # ModeSocket + 32 - 11, # ModeCharDevice + 32 - 13, # ModeIrregular + ): + if container_stat['mode'] & (1 << bit) != 0: + return True + return False + + +def get_container_file_mode(container_stat): + mode = container_stat['mode'] & 0xFFF + if container_stat['mode'] & (1 << (32 - 9)) != 0: # ModeSetuid + mode |= stat.S_ISUID # set UID bit + if container_stat['mode'] & (1 << (32 - 10)) != 0: # ModeSetgid + mode |= stat.S_ISGID # set GID bit + if container_stat['mode'] & (1 << (32 - 12)) != 0: # ModeSticky + mode |= stat.S_ISVTX # sticky bit + return mode + + +def is_file_idempotent(client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode, force=False): # Retrieve information of local file try: file_stat = os.stat(managed_path) if local_follow_links else os.lstat(managed_path) @@ -217,29 +263,11 @@ def is_idempotent(client, container, managed_path, container_path, follow_links, return container_path, mode, local_link_target == link_target if link_target is not None: return container_path, mode, False - for bit in ( - # https://pkg.go.dev/io/fs#FileMode - 32 - 1, # ModeDir - 32 - 4, # ModeTemporary - 32 - 5, # ModeSymlink - 32 - 6, # ModeDevice - 32 - 7, # ModeNamedPipe - 32 - 8, # ModeSocket - 32 - 11, # ModeCharDevice - 32 - 13, # ModeIrregular - ): - if regular_stat['mode'] & (1 << bit) != 0: - return container_path, mode, False + if is_container_file_not_regular_file(regular_stat): + return container_path, mode, False if file_stat.st_size != regular_stat['size']: return container_path, mode, False - container_file_mode = regular_stat['mode'] & 0xFFF - if regular_stat['mode'] & (1 << (32 - 9)) != 0: # ModeSetuid - container_file_mode |= stat.S_ISUID # set UID bit - if regular_stat['mode'] & (1 << (32 - 10)) != 0: # ModeSetgid - container_file_mode |= stat.S_ISGID # set GID bit - if regular_stat['mode'] & (1 << (32 - 12)) != 0: # ModeSticky - container_file_mode |= stat.S_ISVTX # sticky bit - if mode != container_file_mode: + if mode != get_container_file_mode(regular_stat): return container_path, mode, False # Fetch file from container @@ -290,8 +318,8 @@ def process_symlink(in_path, member): ) -def copy_into_container(client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode, force=False): - container_path, mode, idempotent = is_idempotent( +def copy_file_into_container(client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode, force=False): + container_path, mode, idempotent = is_file_idempotent( client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode, force=force) changed = not idempotent @@ -313,10 +341,100 @@ def copy_into_container(client, container, managed_path, container_path, follow_ ) +def is_content_idempotent(client, container, content, container_path, follow_links, owner_id, group_id, mode, force=False): + # When forcing and we're not following links in the container, go! + if force and not follow_links: + return container_path, mode, False + + # Resolve symlinks in the container (if requested), and get information on container's file + real_container_path, regular_stat, link_target = stat_file( + client, + container, + in_path=container_path, + follow_links=follow_links, + ) + + # Follow links in the Docker container? + if follow_links: + container_path = real_container_path + + # When forcing, go! + if force: + return container_path, mode, False + + # If the file wasn't found, continue + if regular_stat is None: + return container_path, mode, False + + # Basic idempotency checks + if link_target is not None: + return container_path, mode, False + if is_container_file_not_regular_file(regular_stat): + return container_path, mode, False + if len(content) != regular_stat['size']: + return container_path, mode, False + if mode != get_container_file_mode(regular_stat): + return container_path, mode, False + + # Fetch file from container + def process_none(in_path): + return container_path, mode, False + + def process_regular(in_path, tar, member): + # Check things like user/group ID and mode + if member.mode & 0xFFF != mode: + return container_path, mode, False + if member.uid != owner_id: + return container_path, mode, False + if member.gid != group_id: + return container_path, mode, False + + if member.size != len(content): + return container_path, mode, False + + tar_f = tar.extractfile(member) # in Python 2, this *cannot* be used in `with`... + return container_path, mode, are_fileobjs_equal(tar_f, io.BytesIO(content)) + + def process_symlink(in_path, member): + return container_path, mode, False + + return fetch_file_ex( + client, + container, + in_path=container_path, + process_none=process_none, + process_regular=process_regular, + process_symlink=process_symlink, + follow_links=follow_links, + ) + + +def copy_content_into_container(client, container, content, container_path, follow_links, owner_id, group_id, mode, force=False): + container_path, mode, idempotent = is_content_idempotent( + client, container, content, container_path, follow_links, owner_id, group_id, mode, force=force) + changed = not idempotent + + if changed and not client.module.check_mode: + put_file_content( + client, + container, + content=content, + out_path=container_path, + user_id=owner_id, + group_id=group_id, + mode=mode, + ) + + client.module.exit_json( + container_path=container_path, + changed=changed, + ) + + def main(): argument_spec = dict( container=dict(type='str', required=True), - path=dict(type='path', required=True), + path=dict(type='path'), container_path=dict(type='str', required=True), follow=dict(type='bool', default=False), local_follow=dict(type='bool', default=True), @@ -324,13 +442,19 @@ def main(): group_id=dict(type='int'), mode=dict(type='int'), force=dict(type='bool', default=False), + content=dict(type='str'), + content_is_b64=dict(type='bool', default=False), ) client = AnsibleDockerClient( argument_spec=argument_spec, min_docker_api_version='1.20', supports_check_mode=True, + mutually_exclusive=[('path', 'content')], required_together=[('owner_id', 'group_id')], + required_by={ + 'content': ['mode'], + }, ) container = client.module.params['container'] @@ -342,6 +466,16 @@ def main(): group_id = client.module.params['group_id'] mode = client.module.params['mode'] force = client.module.params['force'] + content = client.module.params['content'] + + if content is not None: + if client.module.params['content_is_b64']: + try: + content = base64.b64decode(content) + except Exception as e: # depending on Python version and error, multiple different exceptions can be raised + client.fail('Cannot Base64 decode the content option: {0}'.format(e)) + else: + content = to_bytes(content) if not container_path.startswith(os.path.sep): container_path = os.path.join(os.path.sep, container_path) @@ -351,18 +485,34 @@ def main(): if owner_id is None or group_id is None: owner_id, group_id = determine_user_group(client, container) - copy_into_container( - client, - container, - managed_path, - container_path, - follow_links=follow, - local_follow_links=local_follow, - owner_id=owner_id, - group_id=group_id, - mode=mode, - force=force, - ) + if content is not None: + copy_content_into_container( + client, + container, + content, + container_path, + follow_links=follow, + owner_id=owner_id, + group_id=group_id, + mode=mode, + force=force, + ) + elif managed_path is not None: + copy_file_into_container( + client, + container, + managed_path, + container_path, + follow_links=follow, + local_follow_links=local_follow, + owner_id=owner_id, + group_id=group_id, + mode=mode, + force=force, + ) + else: + # Can happen if a user explicitly passes `content: null` or `path: null`... + client.fail('One of path and content must be supplied') except NotFound as exc: client.fail('Could not find container "{1}" or resource in it ({0})'.format(exc, container), exception=traceback.format_exc()) except APIError as exc: diff --git a/tests/integration/targets/docker_container_copy_into/tasks/tests/content.yml b/tests/integration/targets/docker_container_copy_into/tasks/tests/content.yml new file mode 100644 index 000000000..ee7f654bc --- /dev/null +++ b/tests/integration/targets/docker_container_copy_into/tasks/tests/content.yml @@ -0,0 +1,770 @@ +--- +# Copyright (c) Ansible Project +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +- name: Registering container name + set_fact: + cname: "{{ cname_prefix ~ '-c' }}" +- name: Registering container name + set_fact: + cnames: "{{ cnames + [cname] }}" + +# Create container + +- name: Create container + docker_container: + image: "{{ docker_test_image_alpine }}" + command: + - /bin/sh + - "-c" + - >- + mkdir /dir; + ln -s file /lnk; + ln -s lnk3 /lnk2; + ln -s lnk2 /lnk1; + sleep 10m; + name: "{{ cname }}" + state: started + +################################################################################################ +# Do tests + +- name: Copy content without mode + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/file' + register: result + ignore_errors: true + +- name: Check results + assert: + that: + - result is failed + - |- + result.msg in [ + "missing parameter(s) required by 'content': mode", + ] + +######################### Copy + +- name: Copy content (check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/file' + mode: 0644 + check_mode: true + register: result_1 + +- name: Copy content (check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/file' + mode: 0644 + register: result_2 + +- name: Copy content (idempotent, check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/file' + mode: 0644 + check_mode: true + register: result_3 + +- name: Copy content (idempotent) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/file' + mode: 0644 + register: result_4 + +- name: Copy content (idempotent, check mode, base 64) + docker_container_copy_into: + container: '{{ cname }}' + content: "{{ 'Content 1\n' | b64encode }}" + content_is_b64: true + container_path: '/file' + mode: 0644 + check_mode: true + register: result_3b64 + +- name: Copy content (idempotent, base 64) + docker_container_copy_into: + container: '{{ cname }}' + content: "{{ 'Content 1\n' | b64encode }}" + content_is_b64: true + container_path: '/file' + mode: 0644 + register: result_4b64 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /file | base64; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_5 + +- name: Copy content (force, check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/file' + mode: 0644 + force: true + check_mode: true + register: result_6 + +- name: Copy content (force) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/file' + mode: 0644 + force: true + register: result_7 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /file | base64; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_8 + +- name: Check results + assert: + that: + - result_1 is changed + - result_2 is changed + - result_3 is not changed + - result_4 is not changed + - result_3b64 is not changed + - result_4b64 is not changed + - result_5.stdout | b64decode == 'Content 1\n' + - result_5.stderr == '10 644 regular file 0 0 /file' + - result_6 is changed + - result_7 is changed + - result_8.stdout | b64decode == 'Content 1\n' + - result_8.stderr == '10 644 regular file 0 0 /file' + +######################### Follow link - idempotence + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /lnk | base64; + stat -c '%s %a %F %u %g %N' /lnk > /dev/stderr; + chdir: /root + register: result_0 + +- name: Copy content following link (check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/lnk' + mode: 0644 + follow: true + check_mode: true + register: result_1 + +- name: Copy content following link + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/lnk' + mode: 0644 + follow: true + register: result_2 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /lnk | base64; + stat -c '%s %a %F %u %g %N' /lnk > /dev/stderr; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_3 + +- name: Copy content following link (force, check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/lnk' + mode: 0644 + follow: true + force: true + check_mode: true + register: result_4 + +- name: Copy content following link (force) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/lnk' + mode: 0644 + follow: true + force: true + register: result_5 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /lnk | base64; + stat -c '%s %a %F %u %g %N' /lnk > /dev/stderr; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_6 + +- name: Check results + assert: + that: + - result_0.stdout | b64decode == 'Content 1\n' + - result_0.stderr == "4 777 symbolic link 0 0 '/lnk' -> 'file'" + - result_1 is not changed + - result_1.container_path == '/file' + - result_2 is not changed + - result_2.container_path == '/file' + - result_3.stdout | b64decode == 'Content 1\n' + - result_3.stderr_lines[0] == "4 777 symbolic link 0 0 '/lnk' -> 'file'" + - result_3.stderr_lines[1] == '10 644 regular file 0 0 /file' + - result_4 is changed + - result_4.container_path == '/file' + - result_5 is changed + - result_5.container_path == '/file' + - result_6.stdout | b64decode == 'Content 1\n' + - result_6.stderr_lines[0] == "4 777 symbolic link 0 0 '/lnk' -> 'file'" + - result_6.stderr_lines[1] == '10 644 regular file 0 0 /file' + +######################### Do not follow link - replace by file + +- name: Copy content not following link (check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/lnk' + mode: 0644 + follow: false + check_mode: true + register: result_1 + +- name: Copy content not following link + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/lnk' + mode: 0644 + follow: false + register: result_2 + +- name: Copy content not following link (idempotent, check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/lnk' + mode: 0644 + check_mode: true + register: result_3 + +- name: Copy content not following link (idempotent) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/lnk' + mode: 0644 + register: result_4 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /lnk | base64; + stat -c '%s %a %F %u %g %N' /lnk > /dev/stderr + chdir: /root + register: result_5 + +- name: Copy content not following link (force, check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/lnk' + mode: 0644 + force: true + check_mode: true + register: result_6 + +- name: Copy content not following link (force) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/lnk' + mode: 0644 + force: true + register: result_7 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /lnk | base64; + stat -c '%s %a %F %u %g %N' /lnk > /dev/stderr + chdir: /root + register: result_8 + +- name: Check results + assert: + that: + - result_1 is changed + - result_1.container_path == '/lnk' + - result_2 is changed + - result_2.container_path == '/lnk' + - result_3 is not changed + - result_4 is not changed + - result_5.stdout | b64decode == 'Content 1\n' + - result_5.stderr == '10 644 regular file 0 0 /lnk' + - result_6 is changed + - result_6.container_path == '/lnk' + - result_7 is changed + - result_7.container_path == '/lnk' + - result_8.stdout | b64decode == 'Content 1\n' + - result_8.stderr == '10 644 regular file 0 0 /lnk' + +######################### Replace directory by file + +- name: Copy content to replace directory (check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/dir' + mode: 0644 + follow: false + check_mode: true + register: result_1 + +- name: Copy content to replace directory (check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/dir' + mode: 0644 + follow: false + register: result_2 + +- name: Copy content to replace directory (idempotent, check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/dir' + mode: 0644 + check_mode: true + register: result_3 + +- name: Copy content to replace directory (idempotent) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/dir' + mode: 0644 + register: result_4 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /dir | base64; + stat -c '%s %a %F %u %g %N' /dir > /dev/stderr + chdir: /root + register: result_5 + +- name: Check results + assert: + that: + - result_1 is changed + - result_1.container_path == '/dir' + - result_2 is changed + - result_2.container_path == '/dir' + - result_3 is not changed + - result_4 is not changed + - result_5.stdout | b64decode == 'Content 1\n' + - result_5.stderr == '10 644 regular file 0 0 /dir' + +######################### Modify + +- name: Copy content (changed, check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: |- + Content 2 + Extra line + container_path: '/file' + mode: 0644 + check_mode: true + register: result_1 + +- name: Copy content (changed) + docker_container_copy_into: + container: '{{ cname }}' + content: |- + Content 2 + Extra line + container_path: '/file' + mode: 0644 + register: result_2 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /file | base64; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_3 + +- name: Check results + assert: + that: + - result_1 is changed + - result_2 is changed + - result_3.stdout | b64decode == 'Content 2\nExtra line' + - result_3.stderr == '20 644 regular file 0 0 /file' + +######################### Change mode + +- name: Copy content (mode changed, check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: |- + Content 2 + Extra line + container_path: '/file' + mode: 0707 + check_mode: true + register: result_1 + +- name: Copy content (mode changed) + docker_container_copy_into: + container: '{{ cname }}' + content: |- + Content 2 + Extra line + container_path: '/file' + mode: 0707 + register: result_2 + +- name: Copy content (idempotent, check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: |- + Content 2 + Extra line + container_path: '/file' + mode: 0707 + check_mode: true + register: result_3 + +- name: Copy content (idempotent) + docker_container_copy_into: + container: '{{ cname }}' + content: |- + Content 2 + Extra line + container_path: '/file' + mode: 0707 + register: result_4 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /file | base64; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_5 + +- name: Check results + assert: + that: + - result_1 is changed + - result_2 is changed + - result_3 is not changed + - result_4 is not changed + - result_5.stdout | b64decode == 'Content 2\nExtra line' + - result_5.stderr == '20 707 regular file 0 0 /file' + +######################### Change owner and group + +- name: Copy content (owner/group changed, check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: |- + Content 2 + Extra line + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + check_mode: true + register: result_1 + +- name: Copy content (owner/group changed) + docker_container_copy_into: + container: '{{ cname }}' + content: |- + Content 2 + Extra line + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + register: result_2 + +- name: Copy content (idempotent, check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: |- + Content 2 + Extra line + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + check_mode: true + register: result_3 + +- name: Copy content (idempotent) + docker_container_copy_into: + container: '{{ cname }}' + content: |- + Content 2 + Extra line + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + register: result_4 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /file | base64; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_5 + +- name: Copy content (owner/group changed again, check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: |- + Content 2 + Extra line + container_path: '/file' + mode: 0707 + owner_id: 13 + group_id: 13 + check_mode: true + register: result_6 + +- name: Copy content (owner/group changed again) + docker_container_copy_into: + container: '{{ cname }}' + content: |- + Content 2 + Extra line + container_path: '/file' + mode: 0707 + owner_id: 13 + group_id: 13 + register: result_7 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /file | base64; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_8 + +- name: Check results + assert: + that: + - result_1 is changed + - result_2 is changed + - result_3 is not changed + - result_4 is not changed + - result_5.stdout | b64decode == 'Content 2\nExtra line' + - result_5.stderr == '20 707 regular file 12 910 /file' + - result_6 is changed + - result_7 is changed + - result_8.stdout | b64decode == 'Content 2\nExtra line' + - result_8.stderr == '20 707 regular file 13 13 /file' + +######################### Operate with stopped container + +- name: Stop container + docker_container: + name: "{{ cname }}" + state: stopped + stop_timeout: 1 + +- name: Copy content (stopped container, check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + check_mode: true + register: result_1 + +- name: Copy content (stopped container) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + register: result_2 + +- name: Copy content (stopped container, idempotent, check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + check_mode: true + register: result_3 + +- name: Copy content (stopped container, idempotent) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + register: result_4 + +- name: Copy content (stopped container, no owner/group provided, should fail) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/file' + mode: 0707 + register: result_5 + ignore_errors: true + +- name: Start container + docker_container: + name: "{{ cname }}" + state: started + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /file | base64; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_6 + +- name: Check results + assert: + that: + - result_1 is changed + - result_2 is changed + - result_3 is not changed + - result_4 is not changed + - result_5 is failed + - result_5.msg == ('Cannot execute command in paused container "' ~ cname ~ '"') + - result_6.stdout | b64decode == 'Content 1\n' + - result_6.stderr == '10 707 regular file 12 910 /file' + +################################################################################################ +# Cleanup + +- name: Remove container + docker_container: + name: "{{ cname }}" + state: absent + force_kill: yes diff --git a/tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml b/tests/integration/targets/docker_container_copy_into/tasks/tests/file.yml similarity index 100% rename from tests/integration/targets/docker_container_copy_into/tasks/tests/simple.yml rename to tests/integration/targets/docker_container_copy_into/tasks/tests/file.yml From 0bc8a6043f0ff91aa9f3780035e133ed667a7769 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sun, 8 Jan 2023 11:27:47 +0100 Subject: [PATCH 20/27] Make force=false work as for copy module: only copy if the destination does not exist. --- plugins/modules/docker_container_copy_into.py | 18 ++++++-- .../tasks/tests/content.yml | 41 +++++++++++++++++++ .../tasks/tests/file.yml | 39 ++++++++++++++++++ 3 files changed, 95 insertions(+), 3 deletions(-) diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py index 1c9f8d8a0..781d9ae07 100644 --- a/plugins/modules/docker_container_copy_into.py +++ b/plugins/modules/docker_container_copy_into.py @@ -90,9 +90,13 @@ type: int force: description: - - Force writing the file (without performing any idempotency checks). + - If set to C(true), force writing the file (without performing any idempotency checks). + - If set to C(false), only write the file if it does not exist on the target. If a filesystem object exists at + the destination, the module will not do any change. + - If this option is not specified, the module will be idempotent. To verify idempotency, it will try to get information + on the filesystem object in the container, and if everything seems to match will download the file from the container + to compare it to the file to upload. type: bool - default: false extends_documentation_fragment: - community.docker.docker.api_documentation @@ -255,6 +259,10 @@ def is_file_idempotent(client, container, managed_path, container_path, follow_l if regular_stat is None: return container_path, mode, False + # If force is set to False, and the destination exists, assume there's nothing to do + if force is False: + return container_path, mode, True + # Basic idempotency checks if stat.S_ISLNK(file_stat.st_mode): if link_target is None: @@ -366,6 +374,10 @@ def is_content_idempotent(client, container, content, container_path, follow_lin if regular_stat is None: return container_path, mode, False + # If force is set to False, and the destination exists, assume there's nothing to do + if force is False: + return container_path, mode, True + # Basic idempotency checks if link_target is not None: return container_path, mode, False @@ -441,7 +453,7 @@ def main(): owner_id=dict(type='int'), group_id=dict(type='int'), mode=dict(type='int'), - force=dict(type='bool', default=False), + force=dict(type='bool'), content=dict(type='str'), content_is_b64=dict(type='bool', default=False), ) diff --git a/tests/integration/targets/docker_container_copy_into/tasks/tests/content.yml b/tests/integration/targets/docker_container_copy_into/tasks/tests/content.yml index ee7f654bc..f6c930768 100644 --- a/tests/integration/targets/docker_container_copy_into/tasks/tests/content.yml +++ b/tests/integration/targets/docker_container_copy_into/tasks/tests/content.yml @@ -152,6 +152,43 @@ chdir: /root register: result_8 +- name: Copy content (force=false, check mode) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Some other content + container_path: '/file' + mode: 0777 + owner_id: 123 + group_id: 321 + force: false + check_mode: true + register: result_9 + +- name: Copy content (force=false) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Some other content + container_path: '/file' + mode: 0777 + owner_id: 123 + group_id: 321 + force: false + register: result_10 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /file | base64; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_11 + - name: Check results assert: that: @@ -167,6 +204,10 @@ - result_7 is changed - result_8.stdout | b64decode == 'Content 1\n' - result_8.stderr == '10 644 regular file 0 0 /file' + - result_9 is not changed + - result_10 is not changed + - result_11.stdout | b64decode == 'Content 1\n' + - result_11.stderr == '10 644 regular file 0 0 /file' ######################### Follow link - idempotence diff --git a/tests/integration/targets/docker_container_copy_into/tasks/tests/file.yml b/tests/integration/targets/docker_container_copy_into/tasks/tests/file.yml index dc5af2af2..4d02cffcb 100644 --- a/tests/integration/targets/docker_container_copy_into/tasks/tests/file.yml +++ b/tests/integration/targets/docker_container_copy_into/tasks/tests/file.yml @@ -137,6 +137,41 @@ chdir: /root register: result_8 +- name: Copy file (force=false, check mode) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + mode: 0777 + owner_id: 123 + group_id: 321 + force: false + check_mode: true + register: result_9 + +- name: Copy file (force=false) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + mode: 0777 + owner_id: 123 + group_id: 321 + force: false + register: result_10 + +- name: Dump file + docker_container_exec: + container: '{{ cname }}' + argv: + - /bin/sh + - "-c" + - >- + cat /file | base64; + stat -c '%s %a %F %u %g %N' /file > /dev/stderr + chdir: /root + register: result_11 + - name: Check results assert: that: @@ -150,6 +185,10 @@ - result_7 is changed - result_8.stdout | b64decode == 'Content 1\n' - result_8.stderr == '10 644 regular file 0 0 /file' + - result_9 is not changed + - result_10 is not changed + - result_11.stdout | b64decode == 'Content 1\n' + - result_11.stderr == '10 644 regular file 0 0 /file' ######################### Follow link - idempotence From a9cb8eea4c3543490c023446ab0ae4641d34bc71 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sun, 8 Jan 2023 11:40:40 +0100 Subject: [PATCH 21/27] Improve docs. --- plugins/modules/docker_container_copy_into.py | 1 + 1 file changed, 1 insertion(+) diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py index 781d9ae07..7eb70897f 100644 --- a/plugins/modules/docker_container_copy_into.py +++ b/plugins/modules/docker_container_copy_into.py @@ -18,6 +18,7 @@ - Copy a file into a Docker container. - Similar to C(docker cp). - To copy files in a non-running container, you must provide the I(owner_id) and I(group_id) options. + This is also necessary if the container does not contain a C(/bin/sh) shell with an C(id) tool. attributes: check_mode: From 9f44a4624573d208016d1676ccee8c636d5cca02 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sun, 8 Jan 2023 11:56:47 +0100 Subject: [PATCH 22/27] content should be no_log. --- plugins/modules/docker_container_copy_into.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py index 7eb70897f..6ff201fae 100644 --- a/plugins/modules/docker_container_copy_into.py +++ b/plugins/modules/docker_container_copy_into.py @@ -455,7 +455,7 @@ def main(): group_id=dict(type='int'), mode=dict(type='int'), force=dict(type='bool'), - content=dict(type='str'), + content=dict(type='str', no_log=True), content_is_b64=dict(type='bool', default=False), ) From f630d38a906d1db74af0f2878720dfe422d99be5 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sun, 8 Jan 2023 20:53:43 +0100 Subject: [PATCH 23/27] Implement diff mode. --- plugins/action/docker_container_copy_into.py | 41 ++ plugins/module_utils/_scramble.py | 39 ++ plugins/module_utils/copy.py | 9 +- plugins/modules/docker_container_copy_into.py | 401 ++++++++++++++++-- .../tasks/tests/content.yml | 386 +++++++++++++++++ .../tasks/tests/file.yml | 334 ++++++++++++++- tests/sanity/ignore-2.10.txt | 1 + tests/sanity/ignore-2.11.txt | 1 + tests/sanity/ignore-2.12.txt | 1 + tests/sanity/ignore-2.13.txt | 1 + tests/sanity/ignore-2.14.txt | 1 + tests/sanity/ignore-2.15.txt | 1 + tests/sanity/ignore-2.9.txt | 1 + .../plugins/module_utils/test__scramble.py | 29 ++ 14 files changed, 1203 insertions(+), 43 deletions(-) create mode 100644 plugins/action/docker_container_copy_into.py create mode 100644 plugins/module_utils/_scramble.py create mode 100644 tests/unit/plugins/module_utils/test__scramble.py diff --git a/plugins/action/docker_container_copy_into.py b/plugins/action/docker_container_copy_into.py new file mode 100644 index 000000000..b6f255b75 --- /dev/null +++ b/plugins/action/docker_container_copy_into.py @@ -0,0 +1,41 @@ +# Copyright (c) 2022, Felix Fontein +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +from __future__ import absolute_import, division, print_function +__metaclass__ = type + +import base64 + +from ansible import constants as C +from ansible.errors import AnsibleError +from ansible.plugins.action import ActionBase +from ansible.utils.vars import merge_hash + +from ansible_collections.community.docker.plugins.module_utils._scramble import unscramble + + +class ActionModule(ActionBase): + # Set to True when transfering files to the remote + TRANSFERS_FILES = False + + def run(self, tmp=None, task_vars=None): + self._supports_check_mode = True + self._supports_async = True + + result = super(ActionModule, self).run(tmp, task_vars) + del tmp # tmp no longer has any effect + + self._task.args['_max_file_size_for_diff'] = C.MAX_FILE_SIZE_FOR_DIFF + + result = merge_hash(result, self._execute_module(task_vars=task_vars, wrap_async=self._task.async_val)) + + if u'diff' in result and result[u'diff'].get(u'scrambled_diff'): + # Scrambling is not done for security, but to avoid no_log screwing up the diff + diff = result[u'diff'] + key = base64.b64decode(diff.pop(u'scrambled_diff')) + for k in (u'before', u'after'): + if k in diff: + diff[k] = unscramble(diff[k], key) + + return result diff --git a/plugins/module_utils/_scramble.py b/plugins/module_utils/_scramble.py new file mode 100644 index 000000000..47cfb8d9d --- /dev/null +++ b/plugins/module_utils/_scramble.py @@ -0,0 +1,39 @@ +# Copyright 2016 Red Hat | Ansible +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import base64 + +from ansible.module_utils.common.text.converters import to_bytes, to_native, to_text +from ansible.module_utils.six import PY2 + + +def scramble(value, key): + if len(key) < 1: + raise ValueError('Key must be at least one byte') + value = to_bytes(value) + if PY2: + k = ord(key[0]) + value = b''.join([chr(k ^ ord(b)) for b in value]) + else: + k = key[0] + value = bytes([k ^ b for b in value]) + return '=S=' + to_native(base64.b64encode(value)) + + +def unscramble(value, key): + if len(key) < 1: + raise ValueError('Key must be at least one byte') + if not value.startswith(u'=S='): + raise ValueError('Value does not start with indicator') + value = base64.b64decode(value[3:]) + if PY2: + k = ord(key[0]) + value = b''.join([chr(k ^ ord(b)) for b in value]) + else: + k = key[0] + value = bytes([k ^ b for b in value]) + return to_text(value) diff --git a/plugins/module_utils/copy.py b/plugins/module_utils/copy.py index 4f06c3b42..8926db383 100644 --- a/plugins/module_utils/copy.py +++ b/plugins/module_utils/copy.py @@ -286,7 +286,7 @@ def _stream_generator_to_fileobj(stream): return io.BufferedReader(raw) -def fetch_file_ex(client, container, in_path, process_none, process_regular, process_symlink, follow_links=False, log=None): +def fetch_file_ex(client, container, in_path, process_none, process_regular, process_symlink, process_other, follow_links=False, log=None): """Fetch a file (as a tar file entry) from a Docker container to local.""" considered_in_paths = set() @@ -320,7 +320,7 @@ def fetch_file_ex(client, container, in_path, process_none, process_regular, pro if member.isfile(): result = process_regular(in_path, tar, member) continue - raise DockerFileCopyError('Remote file "%s" is not a regular file or a symbolic link' % in_path) + result = process_other(in_path, member) if symlink_member: if not follow_links: return process_symlink(in_path, symlink_member) @@ -358,7 +358,10 @@ def process_symlink(in_path, member): os.symlink(member.linkname, b_out_path) return in_path - return fetch_file_ex(client, container, in_path, process_none, process_regular, process_symlink, follow_links=follow_links, log=log) + def process_other(in_path, member): + raise DockerFileCopyError('Remote file "%s" is not a regular file or a symbolic link' % in_path) + + return fetch_file_ex(client, container, in_path, process_none, process_regular, process_symlink, process_other, follow_links=follow_links, log=log) def _execute_command(client, container, command, log=None, check_rc=False): diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py index 6ff201fae..c4e7367a6 100644 --- a/plugins/modules/docker_container_copy_into.py +++ b/plugins/modules/docker_container_copy_into.py @@ -24,7 +24,11 @@ check_mode: support: full diff_mode: - support: none + support: full + details: + - Additional data will need to be transferred to compute diffs. + - The module uses R(the MAX_FILE_SIZE_FOR_DIFF ansible-core configuration,MAX_FILE_SIZE_FOR_DIFF) + to determine for how large files diffs should be computed. options: container: @@ -140,10 +144,11 @@ import base64 import io import os +import random import stat import traceback -from ansible.module_utils._text import to_bytes, to_native +from ansible.module_utils._text import to_bytes, to_native, to_text from ansible_collections.community.docker.plugins.module_utils._api.errors import APIError, DockerException, NotFound @@ -163,6 +168,8 @@ stat_file, ) +from ansible_collections.community.docker.plugins.module_utils._scramble import scramble + def are_fileobjs_equal(f1, f2): '''Given two (buffered) file objects, compare their contents.''' @@ -195,6 +202,52 @@ def are_fileobjs_equal(f1, f2): b2buf = b2buf[buflen:] +def are_fileobjs_equal_read_first(f1, f2): + '''Given two (buffered) file objects, compare their contents. + + Returns a tuple (is_equal, content_of_f1), where the first element indicates + whether the two file objects have the same content, and the second element is + the content of the first file object.''' + blocksize = 65536 + b1buf = b'' + b2buf = b'' + is_equal = True + content = [] + while True: + if f1 and len(b1buf) < blocksize: + f1b = f1.read(blocksize) + if not f1b: + # f1 is EOF, so stop reading from it + f1 = None + b1buf += f1b + if f2 and len(b2buf) < blocksize: + f2b = f2.read(blocksize) + if not f2b: + # f2 is EOF, so stop reading from it + f2 = None + b2buf += f2b + if not b1buf or not b2buf: + # At least one of f1 and f2 is EOF and all its data has + # been processed. If both are EOF and their data has been + # processed, the files are equal, otherwise not. + is_equal = not b1buf and not b2buf + break + # Compare the next chunk of data, and remove it from the buffers + buflen = min(len(b1buf), len(b2buf)) + if b1buf[:buflen] != b2buf[:buflen]: + is_equal = False + break + content.append(b1buf[:buflen]) + b1buf = b1buf[buflen:] + b2buf = b2buf[buflen:] + + content.append(b1buf) + if f1: + content.append(f1.read()) + + return is_equal, b''.join(content) + + def is_container_file_not_regular_file(container_stat): for bit in ( # https://pkg.go.dev/io/fs#FileMode @@ -223,7 +276,152 @@ def get_container_file_mode(container_stat): return mode -def is_file_idempotent(client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode, force=False): +def add_other_diff(diff, in_path, member): + if diff is None: + return + diff['before_header'] = in_path + if member.isdir(): + diff['before'] = '(directory)' + elif member.issym() or member.islnk(): + diff['before'] = member.linkname + elif member.ischr(): + diff['before'] = '(character device)' + elif member.isblk(): + diff['before'] = '(block device)' + elif member.isfifo(): + diff['before'] = '(fifo)' + elif member.isdev(): + diff['before'] = '(device)' + elif member.isfile(): + raise DockerUnexpectedError('should not be a regular file') + else: + diff['before'] = '(unknown filesystem object)' + + +def retrieve_diff(client, container, container_path, follow_links, diff, max_file_size_for_diff, regular_stat=None, link_target=None): + if diff is None: + return + if regular_stat is not None: + # First handle all filesystem object types that are not regular files + if regular_stat['mode'] & (1 << (32 - 1)) != 0: + diff['before_header'] = container_path + diff['before'] = '(directory)' + return + elif regular_stat['mode'] & (1 << (32 - 4)) != 0: + diff['before_header'] = container_path + diff['before'] = '(temporary file)' + return + elif regular_stat['mode'] & (1 << (32 - 5)) != 0: + diff['before_header'] = container_path + diff['before'] = link_target + return + elif regular_stat['mode'] & (1 << (32 - 6)) != 0: + diff['before_header'] = container_path + diff['before'] = '(device)' + return + elif regular_stat['mode'] & (1 << (32 - 7)) != 0: + diff['before_header'] = container_path + diff['before'] = '(named pipe)' + return + elif regular_stat['mode'] & (1 << (32 - 8)) != 0: + diff['before_header'] = container_path + diff['before'] = '(socket)' + return + elif regular_stat['mode'] & (1 << (32 - 11)) != 0: + diff['before_header'] = container_path + diff['before'] = '(character device)' + return + elif regular_stat['mode'] & (1 << (32 - 13)) != 0: + diff['before_header'] = container_path + diff['before'] = '(unknown filesystem object)' + return + # Check whether file is too large + if regular_stat['size'] > max_file_size_for_diff > 0: + diff['dst_larger'] = max_file_size_for_diff + return + + # We need to get hold of the content + def process_none(in_path): + diff['before'] = '' + + def process_regular(in_path, tar, member): + add_diff_dst_from_regular_member(diff, max_file_size_for_diff, in_path, tar, member) + + def process_symlink(in_path, member): + diff['before_header'] = in_path + diff['before'] = member.linkname + + def process_other(in_path, member): + add_other_diff(diff, in_path, member) + + fetch_file_ex( + client, + container, + in_path=container_path, + process_none=process_none, + process_regular=process_regular, + process_symlink=process_symlink, + process_other=process_other, + follow_links=follow_links, + ) + + +def is_binary(content): + if b'\x00' in content: + return True + # TODO: better detection + # (ansible-core also just checks for 0x00, and even just sticks to the first 8k, so this isn't too bad...) + return False + + +def are_fileobjs_equal_with_diff_of_first(f1, f2, size, diff, max_file_size_for_diff, container_path): + if diff is None: + return are_fileobjs_equal(f1, f2) + if size > max_file_size_for_diff > 0: + diff['dst_larger'] = max_file_size_for_diff + return are_fileobjs_equal(f1, f2) + is_equal, content = are_fileobjs_equal_read_first(f1, f2) + if is_binary(content): + diff['dst_binary'] = 1 + else: + diff['before_header'] = container_path + diff['before'] = to_text(content) + return is_equal + + +def add_diff_dst_from_regular_member(diff, max_file_size_for_diff, container_path, tar, member): + if diff is None: + return + if member.size > max_file_size_for_diff > 0: + diff['dst_larger'] = max_file_size_for_diff + return + + tar_f = tar.extractfile(member) # in Python 2, this *cannot* be used in `with`... + content = tar_f.read() + if is_binary(content): + diff['dst_binary'] = 1 + else: + diff['before_header'] = container_path + diff['before'] = to_text(content) + + +def copy_dst_to_src(diff): + if diff is None: + return + for f, t in [ + ('dst_size', 'src_size'), + ('dst_binary', 'src_binary'), + ('before_header', 'after_header'), + ('before', 'after'), + ]: + if f in diff: + diff[t] = diff[f] + elif t in diff: + diff.pop(t) + + +def is_file_idempotent(client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode, + force=False, diff=None, max_file_size_for_diff=1): # Retrieve information of local file try: file_stat = os.stat(managed_path) if local_follow_links else os.lstat(managed_path) @@ -236,8 +434,24 @@ def is_file_idempotent(client, container, managed_path, container_path, follow_l if not stat.S_ISLNK(file_stat.st_mode) and not stat.S_ISREG(file_stat.st_mode): raise DockerFileCopyError('Local path {managed_path} is not a symbolic link or file') + if diff is not None: + if file_stat.st_size > max_file_size_for_diff > 0: + diff['src_larger'] = max_file_size_for_diff + elif stat.S_ISLNK(file_stat.st_mode): + diff['after_header'] = managed_path + diff['after'] = os.readlink(managed_path) + else: + with open(managed_path, 'rb') as f: + content = f.read() + if is_binary(content): + diff['src_binary'] = 1 + else: + diff['after_header'] = managed_path + diff['after'] = to_text(content) + # When forcing and we're not following links in the container, go! if force and not follow_links: + retrieve_diff(client, container, container_path, follow_links, diff, max_file_size_for_diff) return container_path, mode, False # Resolve symlinks in the container (if requested), and get information on container's file @@ -252,31 +466,43 @@ def is_file_idempotent(client, container, managed_path, container_path, follow_l if follow_links: container_path = real_container_path - # When forcing, go! - if force: - return container_path, mode, False - # If the file wasn't found, continue if regular_stat is None: + if diff is not None: + diff['before_header'] = container_path + diff['before'] = '' + return container_path, mode, False + + # When forcing, go! + if force: + retrieve_diff(client, container, container_path, follow_links, diff, max_file_size_for_diff, regular_stat, link_target) return container_path, mode, False # If force is set to False, and the destination exists, assume there's nothing to do if force is False: + retrieve_diff(client, container, container_path, follow_links, diff, max_file_size_for_diff, regular_stat, link_target) + copy_dst_to_src(diff) return container_path, mode, True # Basic idempotency checks if stat.S_ISLNK(file_stat.st_mode): if link_target is None: + retrieve_diff(client, container, container_path, follow_links, diff, max_file_size_for_diff, regular_stat, link_target) return container_path, mode, False local_link_target = os.readlink(managed_path) + retrieve_diff(client, container, container_path, follow_links, diff, max_file_size_for_diff, regular_stat, link_target) return container_path, mode, local_link_target == link_target if link_target is not None: + retrieve_diff(client, container, container_path, follow_links, diff, max_file_size_for_diff, regular_stat, link_target) return container_path, mode, False if is_container_file_not_regular_file(regular_stat): + retrieve_diff(client, container, container_path, follow_links, diff, max_file_size_for_diff, regular_stat, link_target) return container_path, mode, False if file_stat.st_size != regular_stat['size']: + retrieve_diff(client, container, container_path, follow_links, diff, max_file_size_for_diff, regular_stat, link_target) return container_path, mode, False if mode != get_container_file_mode(regular_stat): + retrieve_diff(client, container, container_path, follow_links, diff, max_file_size_for_diff, regular_stat, link_target) return container_path, mode, False # Fetch file from container @@ -285,23 +511,26 @@ def process_none(in_path): def process_regular(in_path, tar, member): # Check things like user/group ID and mode - if member.mode & 0xFFF != mode: - return container_path, mode, False - if member.uid != owner_id: - return container_path, mode, False - if member.gid != group_id: - return container_path, mode, False - - if not stat.S_ISREG(file_stat.st_mode): - return container_path, mode, False - if member.size != file_stat.st_size: + if any([ + member.mode & 0xFFF != mode, + member.uid != owner_id, + member.gid != group_id, + not stat.S_ISREG(file_stat.st_mode), + member.size != file_stat.st_size, + ]): + add_diff_dst_from_regular_member(diff, max_file_size_for_diff, in_path, tar, member) return container_path, mode, False tar_f = tar.extractfile(member) # in Python 2, this *cannot* be used in `with`... with open(managed_path, 'rb') as local_f: - return container_path, mode, are_fileobjs_equal(tar_f, local_f) + is_equal = are_fileobjs_equal_with_diff_of_first(tar_f, local_f, member.size, diff, max_file_size_for_diff, in_path) + return container_path, mode, is_equal def process_symlink(in_path, member): + if diff is not None: + diff['before_header'] = in_path + diff['before'] = member.linkname + # Check things like user/group ID and mode if member.mode & 0xFFF != mode: return container_path, mode, False @@ -316,6 +545,10 @@ def process_symlink(in_path, member): local_link_target = os.readlink(managed_path) return container_path, mode, member.linkname == local_link_target + def process_other(in_path, member): + add_other_diff(diff, in_path, member) + return container_path, mode, False + return fetch_file_ex( client, container, @@ -323,13 +556,32 @@ def process_symlink(in_path, member): process_none=process_none, process_regular=process_regular, process_symlink=process_symlink, + process_other=process_other, follow_links=follow_links, ) -def copy_file_into_container(client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode, force=False): +def copy_file_into_container(client, container, managed_path, container_path, follow_links, local_follow_links, + owner_id, group_id, mode, force=False, diff=False, max_file_size_for_diff=1): + if diff: + diff = {} + else: + diff = None + container_path, mode, idempotent = is_file_idempotent( - client, container, managed_path, container_path, follow_links, local_follow_links, owner_id, group_id, mode, force=force) + client, + container, + managed_path, + container_path, + follow_links, + local_follow_links, + owner_id, + group_id, + mode, + force=force, + diff=diff, + max_file_size_for_diff=max_file_size_for_diff, + ) changed = not idempotent if changed and not client.module.check_mode: @@ -344,15 +596,29 @@ def copy_file_into_container(client, container, managed_path, container_path, fo follow_links=local_follow_links, ) - client.module.exit_json( + result = dict( container_path=container_path, changed=changed, ) + if diff: + result['diff'] = diff + client.module.exit_json(**result) + + +def is_content_idempotent(client, container, content, container_path, follow_links, owner_id, group_id, mode, + force=False, diff=None, max_file_size_for_diff=1): + if diff is not None: + if len(content) > max_file_size_for_diff > 0: + diff['src_larger'] = max_file_size_for_diff + elif is_binary(content): + diff['src_binary'] = 1 + else: + diff['after_header'] = 'dynamically generated' + diff['after'] = to_text(content) - -def is_content_idempotent(client, container, content, container_path, follow_links, owner_id, group_id, mode, force=False): # When forcing and we're not following links in the container, go! if force and not follow_links: + retrieve_diff(client, container, container_path, follow_links, diff, max_file_size_for_diff) return container_path, mode, False # Resolve symlinks in the container (if requested), and get information on container's file @@ -367,48 +633,68 @@ def is_content_idempotent(client, container, content, container_path, follow_lin if follow_links: container_path = real_container_path - # When forcing, go! - if force: - return container_path, mode, False - # If the file wasn't found, continue if regular_stat is None: + if diff is not None: + diff['before_header'] = container_path + diff['before'] = '' + return container_path, mode, False + + # When forcing, go! + if force: + retrieve_diff(client, container, container_path, follow_links, diff, max_file_size_for_diff, regular_stat, link_target) return container_path, mode, False # If force is set to False, and the destination exists, assume there's nothing to do if force is False: + retrieve_diff(client, container, container_path, follow_links, diff, max_file_size_for_diff, regular_stat, link_target) + copy_dst_to_src(diff) return container_path, mode, True # Basic idempotency checks if link_target is not None: + retrieve_diff(client, container, container_path, follow_links, diff, max_file_size_for_diff, regular_stat, link_target) return container_path, mode, False if is_container_file_not_regular_file(regular_stat): + retrieve_diff(client, container, container_path, follow_links, diff, max_file_size_for_diff, regular_stat, link_target) return container_path, mode, False if len(content) != regular_stat['size']: + retrieve_diff(client, container, container_path, follow_links, diff, max_file_size_for_diff, regular_stat, link_target) return container_path, mode, False if mode != get_container_file_mode(regular_stat): + retrieve_diff(client, container, container_path, follow_links, diff, max_file_size_for_diff, regular_stat, link_target) return container_path, mode, False # Fetch file from container def process_none(in_path): + if diff is not None: + diff['before'] = '' return container_path, mode, False def process_regular(in_path, tar, member): # Check things like user/group ID and mode - if member.mode & 0xFFF != mode: - return container_path, mode, False - if member.uid != owner_id: - return container_path, mode, False - if member.gid != group_id: - return container_path, mode, False - - if member.size != len(content): + if any([ + member.mode & 0xFFF != mode, + member.uid != owner_id, + member.gid != group_id, + member.size != len(content), + ]): + add_diff_dst_from_regular_member(diff, max_file_size_for_diff, in_path, tar, member) return container_path, mode, False tar_f = tar.extractfile(member) # in Python 2, this *cannot* be used in `with`... - return container_path, mode, are_fileobjs_equal(tar_f, io.BytesIO(content)) + is_equal = are_fileobjs_equal_with_diff_of_first(tar_f, io.BytesIO(content), member.size, diff, max_file_size_for_diff, in_path) + return container_path, mode, is_equal def process_symlink(in_path, member): + if diff is not None: + diff['before_header'] = in_path + diff['before'] = member.linkname + + return container_path, mode, False + + def process_other(in_path, member): + add_other_diff(diff, in_path, member) return container_path, mode, False return fetch_file_ex( @@ -418,13 +704,31 @@ def process_symlink(in_path, member): process_none=process_none, process_regular=process_regular, process_symlink=process_symlink, + process_other=process_other, follow_links=follow_links, ) -def copy_content_into_container(client, container, content, container_path, follow_links, owner_id, group_id, mode, force=False): +def copy_content_into_container(client, container, content, container_path, follow_links, + owner_id, group_id, mode, force=False, diff=False, max_file_size_for_diff=1): + if diff: + diff = {} + else: + diff = None + container_path, mode, idempotent = is_content_idempotent( - client, container, content, container_path, follow_links, owner_id, group_id, mode, force=force) + client, + container, + content, + container_path, + follow_links, + owner_id, + group_id, + mode, + force=force, + diff=diff, + max_file_size_for_diff=max_file_size_for_diff, + ) changed = not idempotent if changed and not client.module.check_mode: @@ -438,10 +742,21 @@ def copy_content_into_container(client, container, content, container_path, foll mode=mode, ) - client.module.exit_json( + result = dict( container_path=container_path, changed=changed, ) + if diff: + # Since the content is no_log, make sure that the before/after strings look sufficiently different + key = b'\x00' + while key[0] == 0: + key = random.randbytes(1) + diff['scrambled_diff'] = base64.b64encode(key) + for k in ('before', 'after'): + if k in diff: + diff[k] = scramble(diff[k], key) + result['diff'] = diff + client.module.exit_json(**result) def main(): @@ -457,6 +772,9 @@ def main(): force=dict(type='bool'), content=dict(type='str', no_log=True), content_is_b64=dict(type='bool', default=False), + + # Undocumented parameters for use by the action plugin + _max_file_size_for_diff=dict(type='int', required=True), ) client = AnsibleDockerClient( @@ -480,6 +798,7 @@ def main(): mode = client.module.params['mode'] force = client.module.params['force'] content = client.module.params['content'] + max_file_size_for_diff = client.module.params['_max_file_size_for_diff'] or 1 if content is not None: if client.module.params['content_is_b64']: @@ -509,6 +828,8 @@ def main(): group_id=group_id, mode=mode, force=force, + diff=client.module._diff, + max_file_size_for_diff=max_file_size_for_diff, ) elif managed_path is not None: copy_file_into_container( @@ -522,6 +843,8 @@ def main(): group_id=group_id, mode=mode, force=force, + diff=client.module._diff, + max_file_size_for_diff=max_file_size_for_diff, ) else: # Can happen if a user explicitly passes `content: null` or `path: null`... diff --git a/tests/integration/targets/docker_container_copy_into/tasks/tests/content.yml b/tests/integration/targets/docker_container_copy_into/tasks/tests/content.yml index f6c930768..ae22af001 100644 --- a/tests/integration/targets/docker_container_copy_into/tasks/tests/content.yml +++ b/tests/integration/targets/docker_container_copy_into/tasks/tests/content.yml @@ -58,8 +58,20 @@ container_path: '/file' mode: 0644 check_mode: true + diff: false register: result_1 +- name: Copy content (check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/file' + mode: 0644 + check_mode: true + diff: true + register: result_1_diff + - name: Copy content (check mode) docker_container_copy_into: container: '{{ cname }}' @@ -77,8 +89,20 @@ container_path: '/file' mode: 0644 check_mode: true + diff: false register: result_3 +- name: Copy content (idempotent, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/file' + mode: 0644 + check_mode: true + diff: true + register: result_3_diff + - name: Copy content (idempotent) docker_container_copy_into: container: '{{ cname }}' @@ -96,8 +120,20 @@ container_path: '/file' mode: 0644 check_mode: true + diff: false register: result_3b64 +- name: Copy content (idempotent, check mode, base 64, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: "{{ 'Content 1\n' | b64encode }}" + content_is_b64: true + container_path: '/file' + mode: 0644 + check_mode: true + diff: true + register: result_3b64_diff + - name: Copy content (idempotent, base 64) docker_container_copy_into: container: '{{ cname }}' @@ -128,8 +164,21 @@ mode: 0644 force: true check_mode: true + diff: false register: result_6 +- name: Copy content (force, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/file' + mode: 0644 + force: true + check_mode: true + diff: true + register: result_6_diff + - name: Copy content (force) docker_container_copy_into: container: '{{ cname }}' @@ -163,8 +212,23 @@ group_id: 321 force: false check_mode: true + diff: false register: result_9 +- name: Copy content (force=false, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Some other content + container_path: '/file' + mode: 0777 + owner_id: 123 + group_id: 321 + force: false + check_mode: true + diff: true + register: result_9_diff + - name: Copy content (force=false) docker_container_copy_into: container: '{{ cname }}' @@ -193,18 +257,48 @@ assert: that: - result_1 is changed + - "'diff' not in result_1" + - result_1_diff.diff.before == '' + - result_1_diff.diff.before_header == '/file' + - result_1_diff.diff.after == 'Content 1\n' + - result_1_diff.diff.after_header == 'dynamically generated' + - result_1 == (result_1_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_2 is changed - result_3 is not changed + - "'diff' not in result_3" + - result_3_diff.diff.before == 'Content 1\n' + - result_3_diff.diff.before_header == '/file' + - result_3_diff.diff.after == 'Content 1\n' + - result_3_diff.diff.after_header == 'dynamically generated' + - result_3 == (result_3_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_4 is not changed - result_3b64 is not changed + - "'diff' not in result_3b64" + - result_3b64_diff.diff.before == 'Content 1\n' + - result_3b64_diff.diff.before_header == '/file' + - result_3b64_diff.diff.after == 'Content 1\n' + - result_3b64_diff.diff.after_header == 'dynamically generated' + - result_3b64 == (result_3b64_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_4b64 is not changed - result_5.stdout | b64decode == 'Content 1\n' - result_5.stderr == '10 644 regular file 0 0 /file' - result_6 is changed + - "'diff' not in result_6" + - result_6_diff.diff.before == 'Content 1\n' + - result_6_diff.diff.before_header == '/file' + - result_6_diff.diff.after == 'Content 1\n' + - result_6_diff.diff.after_header == 'dynamically generated' + - result_6 == (result_6_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_7 is changed - result_8.stdout | b64decode == 'Content 1\n' - result_8.stderr == '10 644 regular file 0 0 /file' - result_9 is not changed + - "'diff' not in result_9" + - result_9_diff.diff.before == 'Content 1\n' + - result_9_diff.diff.before_header == '/file' + - result_9_diff.diff.after == 'Content 1\n' # note that force=false + - result_9_diff.diff.after_header == '/file' # note that force=false + - result_9 == (result_9_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_10 is not changed - result_11.stdout | b64decode == 'Content 1\n' - result_11.stderr == '10 644 regular file 0 0 /file' @@ -232,8 +326,21 @@ mode: 0644 follow: true check_mode: true + diff: false register: result_1 +- name: Copy content following link (check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/lnk' + mode: 0644 + follow: true + check_mode: true + diff: true + register: result_1_diff + - name: Copy content following link docker_container_copy_into: container: '{{ cname }}' @@ -267,8 +374,22 @@ follow: true force: true check_mode: true + diff: false register: result_4 +- name: Copy content following link (force, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/lnk' + mode: 0644 + follow: true + force: true + check_mode: true + diff: true + register: result_4_diff + - name: Copy content following link (force) docker_container_copy_into: container: '{{ cname }}' @@ -300,6 +421,12 @@ - result_0.stderr == "4 777 symbolic link 0 0 '/lnk' -> 'file'" - result_1 is not changed - result_1.container_path == '/file' + - "'diff' not in result_1" + - result_1_diff.diff.before == 'Content 1\n' + - result_1_diff.diff.before_header == '/file' + - result_1_diff.diff.after == 'Content 1\n' + - result_1_diff.diff.after_header == 'dynamically generated' + - result_1 == (result_1_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_2 is not changed - result_2.container_path == '/file' - result_3.stdout | b64decode == 'Content 1\n' @@ -307,6 +434,12 @@ - result_3.stderr_lines[1] == '10 644 regular file 0 0 /file' - result_4 is changed - result_4.container_path == '/file' + - "'diff' not in result_4" + - result_4_diff.diff.before == 'Content 1\n' + - result_4_diff.diff.before_header == '/file' + - result_4_diff.diff.after == 'Content 1\n' + - result_4_diff.diff.after_header == 'dynamically generated' + - result_4 == (result_4_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_5 is changed - result_5.container_path == '/file' - result_6.stdout | b64decode == 'Content 1\n' @@ -324,8 +457,21 @@ mode: 0644 follow: false check_mode: true + diff: false register: result_1 +- name: Copy content not following link (check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/lnk' + mode: 0644 + follow: false + check_mode: true + diff: true + register: result_1_diff + - name: Copy content not following link docker_container_copy_into: container: '{{ cname }}' @@ -344,8 +490,20 @@ container_path: '/lnk' mode: 0644 check_mode: true + diff: false register: result_3 +- name: Copy content not following link (idempotent, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/lnk' + mode: 0644 + check_mode: true + diff: true + register: result_3_diff + - name: Copy content not following link (idempotent) docker_container_copy_into: container: '{{ cname }}' @@ -376,8 +534,21 @@ mode: 0644 force: true check_mode: true + diff: false register: result_6 +- name: Copy content not following link (force, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/lnk' + mode: 0644 + force: true + check_mode: true + diff: true + register: result_6_diff + - name: Copy content not following link (force) docker_container_copy_into: container: '{{ cname }}' @@ -405,14 +576,32 @@ that: - result_1 is changed - result_1.container_path == '/lnk' + - "'diff' not in result_1" + - result_1_diff.diff.before == '/file' + - result_1_diff.diff.before_header == '/lnk' + - result_1_diff.diff.after == 'Content 1\n' + - result_1_diff.diff.after_header == 'dynamically generated' + - result_1 == (result_1_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_2 is changed - result_2.container_path == '/lnk' - result_3 is not changed + - "'diff' not in result_3" + - result_3_diff.diff.before == 'Content 1\n' + - result_3_diff.diff.before_header == '/lnk' + - result_3_diff.diff.after == 'Content 1\n' + - result_3_diff.diff.after_header == 'dynamically generated' + - result_3 == (result_3_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_4 is not changed - result_5.stdout | b64decode == 'Content 1\n' - result_5.stderr == '10 644 regular file 0 0 /lnk' - result_6 is changed - result_6.container_path == '/lnk' + - "'diff' not in result_6" + - result_6_diff.diff.before == 'Content 1\n' + - result_6_diff.diff.before_header == '/lnk' + - result_6_diff.diff.after == 'Content 1\n' + - result_6_diff.diff.after_header == 'dynamically generated' + - result_6 == (result_6_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_7 is changed - result_7.container_path == '/lnk' - result_8.stdout | b64decode == 'Content 1\n' @@ -429,8 +618,21 @@ mode: 0644 follow: false check_mode: true + diff: false register: result_1 +- name: Copy content to replace directory (check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/dir' + mode: 0644 + follow: false + check_mode: true + diff: true + register: result_1_diff + - name: Copy content to replace directory (check mode) docker_container_copy_into: container: '{{ cname }}' @@ -449,8 +651,20 @@ container_path: '/dir' mode: 0644 check_mode: true + diff: false register: result_3 +- name: Copy content to replace directory (idempotent, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/dir' + mode: 0644 + check_mode: true + diff: true + register: result_3_diff + - name: Copy content to replace directory (idempotent) docker_container_copy_into: container: '{{ cname }}' @@ -477,9 +691,21 @@ that: - result_1 is changed - result_1.container_path == '/dir' + - "'diff' not in result_1" + - result_1_diff.diff.before == '(directory)' + - result_1_diff.diff.before_header == '/dir' + - result_1_diff.diff.after == 'Content 1\n' + - result_1_diff.diff.after_header == 'dynamically generated' + - result_1 == (result_1_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_2 is changed - result_2.container_path == '/dir' - result_3 is not changed + - "'diff' not in result_3" + - result_3_diff.diff.before == 'Content 1\n' + - result_3_diff.diff.before_header == '/dir' + - result_3_diff.diff.after == 'Content 1\n' + - result_3_diff.diff.after_header == 'dynamically generated' + - result_3 == (result_3_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_4 is not changed - result_5.stdout | b64decode == 'Content 1\n' - result_5.stderr == '10 644 regular file 0 0 /dir' @@ -495,8 +721,21 @@ container_path: '/file' mode: 0644 check_mode: true + diff: false register: result_1 +- name: Copy content (changed, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: |- + Content 2 + Extra line + container_path: '/file' + mode: 0644 + check_mode: true + diff: true + register: result_1_diff + - name: Copy content (changed) docker_container_copy_into: container: '{{ cname }}' @@ -523,6 +762,12 @@ assert: that: - result_1 is changed + - "'diff' not in result_1" + - result_1_diff.diff.before == 'Content 1\n' + - result_1_diff.diff.before_header == '/file' + - result_1_diff.diff.after == 'Content 2\nExtra line' + - result_1_diff.diff.after_header == 'dynamically generated' + - result_1 == (result_1_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_2 is changed - result_3.stdout | b64decode == 'Content 2\nExtra line' - result_3.stderr == '20 644 regular file 0 0 /file' @@ -538,8 +783,21 @@ container_path: '/file' mode: 0707 check_mode: true + diff: false register: result_1 +- name: Copy content (mode changed, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: |- + Content 2 + Extra line + container_path: '/file' + mode: 0707 + check_mode: true + diff: true + register: result_1_diff + - name: Copy content (mode changed) docker_container_copy_into: container: '{{ cname }}' @@ -559,8 +817,21 @@ container_path: '/file' mode: 0707 check_mode: true + diff: false register: result_3 +- name: Copy content (idempotent, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: |- + Content 2 + Extra line + container_path: '/file' + mode: 0707 + check_mode: true + diff: true + register: result_3_diff + - name: Copy content (idempotent) docker_container_copy_into: container: '{{ cname }}' @@ -587,8 +858,20 @@ assert: that: - result_1 is changed + - "'diff' not in result_1" + - result_1_diff.diff.before == 'Content 2\nExtra line' + - result_1_diff.diff.before_header == '/file' + - result_1_diff.diff.after == 'Content 2\nExtra line' + - result_1_diff.diff.after_header == 'dynamically generated' + - result_1 == (result_1_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_2 is changed - result_3 is not changed + - "'diff' not in result_3" + - result_3_diff.diff.before == 'Content 2\nExtra line' + - result_3_diff.diff.before_header == '/file' + - result_3_diff.diff.after == 'Content 2\nExtra line' + - result_3_diff.diff.after_header == 'dynamically generated' + - result_3 == (result_3_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_4 is not changed - result_5.stdout | b64decode == 'Content 2\nExtra line' - result_5.stderr == '20 707 regular file 0 0 /file' @@ -606,8 +889,23 @@ owner_id: 12 group_id: 910 check_mode: true + diff: false register: result_1 +- name: Copy content (owner/group changed, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: |- + Content 2 + Extra line + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + check_mode: true + diff: true + register: result_1_diff + - name: Copy content (owner/group changed) docker_container_copy_into: container: '{{ cname }}' @@ -631,8 +929,23 @@ owner_id: 12 group_id: 910 check_mode: true + diff: false register: result_3 +- name: Copy content (idempotent, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: |- + Content 2 + Extra line + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + check_mode: true + diff: true + register: result_3_diff + - name: Copy content (idempotent) docker_container_copy_into: container: '{{ cname }}' @@ -668,8 +981,23 @@ owner_id: 13 group_id: 13 check_mode: true + diff: false register: result_6 +- name: Copy content (owner/group changed again, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: |- + Content 2 + Extra line + container_path: '/file' + mode: 0707 + owner_id: 13 + group_id: 13 + check_mode: true + diff: true + register: result_6_diff + - name: Copy content (owner/group changed again) docker_container_copy_into: container: '{{ cname }}' @@ -698,12 +1026,30 @@ assert: that: - result_1 is changed + - "'diff' not in result_1" + - result_1_diff.diff.before == 'Content 2\nExtra line' + - result_1_diff.diff.before_header == '/file' + - result_1_diff.diff.after == 'Content 2\nExtra line' + - result_1_diff.diff.after_header == 'dynamically generated' + - result_1 == (result_1_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_2 is changed - result_3 is not changed + - "'diff' not in result_3" + - result_3_diff.diff.before == 'Content 2\nExtra line' + - result_3_diff.diff.before_header == '/file' + - result_3_diff.diff.after == 'Content 2\nExtra line' + - result_3_diff.diff.after_header == 'dynamically generated' + - result_3 == (result_3_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_4 is not changed - result_5.stdout | b64decode == 'Content 2\nExtra line' - result_5.stderr == '20 707 regular file 12 910 /file' - result_6 is changed + - "'diff' not in result_6" + - result_6_diff.diff.before == 'Content 2\nExtra line' + - result_6_diff.diff.before_header == '/file' + - result_6_diff.diff.after == 'Content 2\nExtra line' + - result_6_diff.diff.after_header == 'dynamically generated' + - result_6 == (result_6_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_7 is changed - result_8.stdout | b64decode == 'Content 2\nExtra line' - result_8.stderr == '20 707 regular file 13 13 /file' @@ -726,8 +1072,22 @@ owner_id: 12 group_id: 910 check_mode: true + diff: false register: result_1 +- name: Copy content (stopped container, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + check_mode: true + diff: true + register: result_1_diff + - name: Copy content (stopped container) docker_container_copy_into: container: '{{ cname }}' @@ -749,8 +1109,22 @@ owner_id: 12 group_id: 910 check_mode: true + diff: false register: result_3 +- name: Copy content (stopped container, idempotent, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + content: | + Content 1 + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + check_mode: true + diff: true + register: result_3_diff + - name: Copy content (stopped container, idempotent) docker_container_copy_into: container: '{{ cname }}' @@ -793,8 +1167,20 @@ assert: that: - result_1 is changed + - "'diff' not in result_1" + - result_1_diff.diff.before == 'Content 2\nExtra line' + - result_1_diff.diff.before_header == '/file' + - result_1_diff.diff.after == 'Content 1\n' + - result_1_diff.diff.after_header == 'dynamically generated' + - result_1 == (result_1_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_2 is changed - result_3 is not changed + - "'diff' not in result_3" + - result_3_diff.diff.before == 'Content 1\n' + - result_3_diff.diff.before_header == '/file' + - result_3_diff.diff.after == 'Content 1\n' + - result_3_diff.diff.after_header == 'dynamically generated' + - result_3 == (result_3_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_4 is not changed - result_5 is failed - result_5.msg == ('Cannot execute command in paused container "' ~ cname ~ '"') diff --git a/tests/integration/targets/docker_container_copy_into/tasks/tests/file.yml b/tests/integration/targets/docker_container_copy_into/tasks/tests/file.yml index 4d02cffcb..8181d125d 100644 --- a/tests/integration/targets/docker_container_copy_into/tasks/tests/file.yml +++ b/tests/integration/targets/docker_container_copy_into/tasks/tests/file.yml @@ -5,7 +5,7 @@ - name: Registering container name set_fact: - cname: "{{ cname_prefix ~ '-hi' }}" + cname: "{{ cname_prefix ~ '-f' }}" - name: Registering container name set_fact: cnames: "{{ cnames + [cname] }}" @@ -72,8 +72,18 @@ path: '{{ remote_tmp_dir }}/file_1' container_path: '/file' check_mode: true + diff: false register: result_1 +- name: Copy file (check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/file' + check_mode: true + diff: true + register: result_1_diff + - name: Copy file (check mode) docker_container_copy_into: container: '{{ cname }}' @@ -87,8 +97,18 @@ path: '{{ remote_tmp_dir }}/file_1' container_path: '/file' check_mode: true + diff: false register: result_3 +- name: Copy file (idempotent, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/file' + check_mode: true + diff: true + register: result_3_diff + - name: Copy file (idempotent) docker_container_copy_into: container: '{{ cname }}' @@ -115,8 +135,19 @@ container_path: '/file' force: true check_mode: true + diff: false register: result_6 +- name: Copy file (force, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/file' + force: true + check_mode: true + diff: true + register: result_6_diff + - name: Copy file (force) docker_container_copy_into: container: '{{ cname }}' @@ -147,8 +178,22 @@ group_id: 321 force: false check_mode: true + diff: false register: result_9 +- name: Copy file (force=false, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + mode: 0777 + owner_id: 123 + group_id: 321 + force: false + check_mode: true + diff: true + register: result_9_diff + - name: Copy file (force=false) docker_container_copy_into: container: '{{ cname }}' @@ -176,16 +221,40 @@ assert: that: - result_1 is changed + - "'diff' not in result_1" + - result_1_diff.diff.before == '' + - result_1_diff.diff.before_header == '/file' + - result_1_diff.diff.after == 'Content 1\n' + - result_1_diff.diff.after_header == remote_tmp_dir ~ '/file_1' + - result_1 == (result_1_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_2 is changed - result_3 is not changed + - "'diff' not in result_3" + - result_3_diff.diff.before == 'Content 1\n' + - result_3_diff.diff.before_header == '/file' + - result_3_diff.diff.after == 'Content 1\n' + - result_3_diff.diff.after_header == remote_tmp_dir ~ '/file_1' + - result_3 == (result_3_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_4 is not changed - result_5.stdout | b64decode == 'Content 1\n' - result_5.stderr == '10 644 regular file 0 0 /file' - result_6 is changed + - "'diff' not in result_6" + - result_6_diff.diff.before == 'Content 1\n' + - result_6_diff.diff.before_header == '/file' + - result_6_diff.diff.after == 'Content 1\n' + - result_6_diff.diff.after_header == remote_tmp_dir ~ '/file_1' + - result_6 == (result_6_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_7 is changed - result_8.stdout | b64decode == 'Content 1\n' - result_8.stderr == '10 644 regular file 0 0 /file' - result_9 is not changed + - "'diff' not in result_9" + - result_9_diff.diff.before == 'Content 1\n' + - result_9_diff.diff.before_header == '/file' + - result_9_diff.diff.after == 'Content 1\n' # note that force=false + - result_9_diff.diff.after_header == '/file' # note that force=false + - result_9 == (result_9_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_10 is not changed - result_11.stdout | b64decode == 'Content 1\n' - result_11.stderr == '10 644 regular file 0 0 /file' @@ -211,8 +280,19 @@ container_path: '/lnk' follow: true check_mode: true + diff: false register: result_1 +- name: Copy file following link (check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/lnk' + follow: true + check_mode: true + diff: true + register: result_1_diff + - name: Copy file following link docker_container_copy_into: container: '{{ cname }}' @@ -242,8 +322,20 @@ follow: true force: true check_mode: true + diff: false register: result_4 +- name: Copy file following link (force, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/lnk' + follow: true + force: true + check_mode: true + diff: true + register: result_4_diff + - name: Copy file following link (force) docker_container_copy_into: container: '{{ cname }}' @@ -273,6 +365,12 @@ - result_0.stderr == "4 777 symbolic link 0 0 '/lnk' -> 'file'" - result_1 is not changed - result_1.container_path == '/file' + - "'diff' not in result_1" + - result_1_diff.diff.before == 'Content 1\n' + - result_1_diff.diff.before_header == '/file' + - result_1_diff.diff.after == 'Content 1\n' + - result_1_diff.diff.after_header == remote_tmp_dir ~ '/file_1' + - result_1 == (result_1_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_2 is not changed - result_2.container_path == '/file' - result_3.stdout | b64decode == 'Content 1\n' @@ -280,6 +378,12 @@ - result_3.stderr_lines[1] == '10 644 regular file 0 0 /file' - result_4 is changed - result_4.container_path == '/file' + - "'diff' not in result_4" + - result_4_diff.diff.before == 'Content 1\n' + - result_4_diff.diff.before_header == '/file' + - result_4_diff.diff.after == 'Content 1\n' + - result_4_diff.diff.after_header == remote_tmp_dir ~ '/file_1' + - result_4 == (result_4_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_5 is changed - result_5.container_path == '/file' - result_6.stdout | b64decode == 'Content 1\n' @@ -295,8 +399,19 @@ container_path: '/lnk' follow: false check_mode: true + diff: false register: result_1 +- name: Copy file not following link (check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/lnk' + follow: false + check_mode: true + diff: true + register: result_1_diff + - name: Copy file not following link docker_container_copy_into: container: '{{ cname }}' @@ -311,8 +426,18 @@ path: '{{ remote_tmp_dir }}/file_1' container_path: '/lnk' check_mode: true + diff: false register: result_3 +- name: Copy file not following link (idempotent, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/lnk' + check_mode: true + diff: true + register: result_3_diff + - name: Copy file not following link (idempotent) docker_container_copy_into: container: '{{ cname }}' @@ -339,8 +464,19 @@ container_path: '/lnk' force: true check_mode: true + diff: false register: result_6 +- name: Copy file not following link (force, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/lnk' + force: true + check_mode: true + diff: true + register: result_6_diff + - name: Copy file not following link (force) docker_container_copy_into: container: '{{ cname }}' @@ -366,14 +502,32 @@ that: - result_1 is changed - result_1.container_path == '/lnk' + - "'diff' not in result_1" + - result_1_diff.diff.before == '/file' + - result_1_diff.diff.before_header == '/lnk' + - result_1_diff.diff.after == 'Content 1\n' + - result_1_diff.diff.after_header == remote_tmp_dir ~ '/file_1' + - result_1 == (result_1_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_2 is changed - result_2.container_path == '/lnk' - result_3 is not changed + - "'diff' not in result_3" + - result_3_diff.diff.before == 'Content 1\n' + - result_3_diff.diff.before_header == '/lnk' + - result_3_diff.diff.after == 'Content 1\n' + - result_3_diff.diff.after_header == remote_tmp_dir ~ '/file_1' + - result_3 == (result_3_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_4 is not changed - result_5.stdout | b64decode == 'Content 1\n' - result_5.stderr == '10 644 regular file 0 0 /lnk' - result_6 is changed - result_6.container_path == '/lnk' + - "'diff' not in result_6" + - result_6_diff.diff.before == 'Content 1\n' + - result_6_diff.diff.before_header == '/lnk' + - result_6_diff.diff.after == 'Content 1\n' + - result_6_diff.diff.after_header == remote_tmp_dir ~ '/file_1' + - result_6 == (result_6_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_7 is changed - result_7.container_path == '/lnk' - result_8.stdout | b64decode == 'Content 1\n' @@ -388,8 +542,19 @@ container_path: '/dir' follow: false check_mode: true + diff: false register: result_1 +- name: Copy file to replace directory (check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/dir' + follow: false + check_mode: true + diff: true + register: result_1_diff + - name: Copy file to replace directory (check mode) docker_container_copy_into: container: '{{ cname }}' @@ -404,8 +569,18 @@ path: '{{ remote_tmp_dir }}/file_1' container_path: '/dir' check_mode: true + diff: false register: result_3 +- name: Copy file to replace directory (idempotent, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/dir' + check_mode: true + diff: true + register: result_3_diff + - name: Copy file to replace directory (idempotent) docker_container_copy_into: container: '{{ cname }}' @@ -430,9 +605,21 @@ that: - result_1 is changed - result_1.container_path == '/dir' + - "'diff' not in result_1" + - result_1_diff.diff.before == '(directory)' + - result_1_diff.diff.before_header == '/dir' + - result_1_diff.diff.after == 'Content 1\n' + - result_1_diff.diff.after_header == remote_tmp_dir ~ '/file_1' + - result_1 == (result_1_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_2 is changed - result_2.container_path == '/dir' - result_3 is not changed + - "'diff' not in result_3" + - result_3_diff.diff.before == 'Content 1\n' + - result_3_diff.diff.before_header == '/dir' + - result_3_diff.diff.after == 'Content 1\n' + - result_3_diff.diff.after_header == remote_tmp_dir ~ '/file_1' + - result_3 == (result_3_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_4 is not changed - result_5.stdout | b64decode == 'Content 1\n' - result_5.stderr == '10 644 regular file 0 0 /dir' @@ -445,8 +632,18 @@ path: '{{ remote_tmp_dir }}/file_2' container_path: '/file' check_mode: true + diff: false register: result_1 +- name: Copy file (changed, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + check_mode: true + diff: true + register: result_1_diff + - name: Copy file (changed) docker_container_copy_into: container: '{{ cname }}' @@ -470,6 +667,12 @@ assert: that: - result_1 is changed + - "'diff' not in result_1" + - result_1_diff.diff.before == 'Content 1\n' + - result_1_diff.diff.before_header == '/file' + - result_1_diff.diff.after == 'Content 2\nExtra line' + - result_1_diff.diff.after_header == remote_tmp_dir ~ '/file_2' + - result_1 == (result_1_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_2 is changed - result_3.stdout | b64decode == 'Content 2\nExtra line' - result_3.stderr == '20 644 regular file 0 0 /file' @@ -483,8 +686,19 @@ container_path: '/file' mode: 0707 check_mode: true + diff: false register: result_1 +- name: Copy file (mode changed, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + mode: 0707 + check_mode: true + diff: true + register: result_1_diff + - name: Copy file (mode changed) docker_container_copy_into: container: '{{ cname }}' @@ -500,8 +714,19 @@ container_path: '/file' mode: 0707 check_mode: true + diff: false register: result_3 +- name: Copy file (idempotent, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + mode: 0707 + check_mode: true + diff: true + register: result_3_diff + - name: Copy file (idempotent) docker_container_copy_into: container: '{{ cname }}' @@ -526,8 +751,20 @@ assert: that: - result_1 is changed + - "'diff' not in result_1" + - result_1_diff.diff.before == 'Content 2\nExtra line' + - result_1_diff.diff.before_header == '/file' + - result_1_diff.diff.after == 'Content 2\nExtra line' + - result_1_diff.diff.after_header == remote_tmp_dir ~ '/file_2' + - result_1 == (result_1_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_2 is changed - result_3 is not changed + - "'diff' not in result_3" + - result_3_diff.diff.before == 'Content 2\nExtra line' + - result_3_diff.diff.before_header == '/file' + - result_3_diff.diff.after == 'Content 2\nExtra line' + - result_3_diff.diff.after_header == remote_tmp_dir ~ '/file_2' + - result_3 == (result_3_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_4 is not changed - result_5.stdout | b64decode == 'Content 2\nExtra line' - result_5.stderr == '20 707 regular file 0 0 /file' @@ -543,8 +780,21 @@ owner_id: 12 group_id: 910 check_mode: true + diff: false register: result_1 +- name: Copy file (owner/group changed, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + check_mode: true + diff: true + register: result_1_diff + - name: Copy file (owner/group changed) docker_container_copy_into: container: '{{ cname }}' @@ -564,8 +814,21 @@ owner_id: 12 group_id: 910 check_mode: true + diff: false register: result_3 +- name: Copy file (idempotent, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + check_mode: true + diff: true + register: result_3_diff + - name: Copy file (idempotent) docker_container_copy_into: container: '{{ cname }}' @@ -597,8 +860,21 @@ owner_id: 13 group_id: 13 check_mode: true + diff: false register: result_6 +- name: Copy file (owner/group changed again, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_2' + container_path: '/file' + mode: 0707 + owner_id: 13 + group_id: 13 + check_mode: true + diff: true + register: result_6_diff + - name: Copy file (owner/group changed again) docker_container_copy_into: container: '{{ cname }}' @@ -625,12 +901,30 @@ assert: that: - result_1 is changed + - "'diff' not in result_1" + - result_1_diff.diff.before == 'Content 2\nExtra line' + - result_1_diff.diff.before_header == '/file' + - result_1_diff.diff.after == 'Content 2\nExtra line' + - result_1_diff.diff.after_header == remote_tmp_dir ~ '/file_2' + - result_1 == (result_1_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_2 is changed - result_3 is not changed + - "'diff' not in result_3" + - result_3_diff.diff.before == 'Content 2\nExtra line' + - result_3_diff.diff.before_header == '/file' + - result_3_diff.diff.after == 'Content 2\nExtra line' + - result_3_diff.diff.after_header == remote_tmp_dir ~ '/file_2' + - result_3 == (result_3_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_4 is not changed - result_5.stdout | b64decode == 'Content 2\nExtra line' - result_5.stderr == '20 707 regular file 12 910 /file' - result_6 is changed + - "'diff' not in result_6" + - result_6_diff.diff.before == 'Content 2\nExtra line' + - result_6_diff.diff.before_header == '/file' + - result_6_diff.diff.after == 'Content 2\nExtra line' + - result_6_diff.diff.after_header == remote_tmp_dir ~ '/file_2' + - result_6 == (result_6_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_7 is changed - result_8.stdout | b64decode == 'Content 2\nExtra line' - result_8.stderr == '20 707 regular file 13 13 /file' @@ -652,8 +946,21 @@ owner_id: 12 group_id: 910 check_mode: true + diff: false register: result_1 +- name: Copy file (stopped container, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + check_mode: true + diff: true + register: result_1_diff + - name: Copy file (stopped container) docker_container_copy_into: container: '{{ cname }}' @@ -673,8 +980,21 @@ owner_id: 12 group_id: 910 check_mode: true + diff: false register: result_3 +- name: Copy file (stopped container, idempotent, check mode, diff) + docker_container_copy_into: + container: '{{ cname }}' + path: '{{ remote_tmp_dir }}/file_1' + container_path: '/file' + mode: 0707 + owner_id: 12 + group_id: 910 + check_mode: true + diff: true + register: result_3_diff + - name: Copy file (stopped container, idempotent) docker_container_copy_into: container: '{{ cname }}' @@ -715,8 +1035,20 @@ assert: that: - result_1 is changed + - "'diff' not in result_1" + - result_1_diff.diff.before == 'Content 2\nExtra line' + - result_1_diff.diff.before_header == '/file' + - result_1_diff.diff.after == 'Content 1\n' + - result_1_diff.diff.after_header == remote_tmp_dir ~ '/file_1' + - result_1 == (result_1_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_2 is changed - result_3 is not changed + - "'diff' not in result_3" + - result_3_diff.diff.before == 'Content 1\n' + - result_3_diff.diff.before_header == '/file' + - result_3_diff.diff.after == 'Content 1\n' + - result_3_diff.diff.after_header == remote_tmp_dir ~ '/file_1' + - result_3 == (result_3_diff | dict2items | rejectattr('key', 'eq', 'diff') | items2dict) - result_4 is not changed - result_5 is failed - result_5.msg == ('Cannot execute command in paused container "' ~ cname ~ '"') diff --git a/tests/sanity/ignore-2.10.txt b/tests/sanity/ignore-2.10.txt index f1a910e3d..1a9f48884 100644 --- a/tests/sanity/ignore-2.10.txt +++ b/tests/sanity/ignore-2.10.txt @@ -8,3 +8,4 @@ plugins/modules/current_container_facts.py validate-modules:return-syntax-error plugins/module_utils/module_container/module.py compile-2.6!skip # Uses Python 2.7+ syntax plugins/module_utils/module_container/module.py import-2.6!skip # Uses Python 2.7+ syntax plugins/modules/docker_container.py import-2.6!skip # Import uses Python 2.7+ syntax +plugins/modules/docker_container_copy_into.py validate-modules:undocumented-parameter # _max_file_size_for_diff is used by the action plugin diff --git a/tests/sanity/ignore-2.11.txt b/tests/sanity/ignore-2.11.txt index f1a910e3d..1a9f48884 100644 --- a/tests/sanity/ignore-2.11.txt +++ b/tests/sanity/ignore-2.11.txt @@ -8,3 +8,4 @@ plugins/modules/current_container_facts.py validate-modules:return-syntax-error plugins/module_utils/module_container/module.py compile-2.6!skip # Uses Python 2.7+ syntax plugins/module_utils/module_container/module.py import-2.6!skip # Uses Python 2.7+ syntax plugins/modules/docker_container.py import-2.6!skip # Import uses Python 2.7+ syntax +plugins/modules/docker_container_copy_into.py validate-modules:undocumented-parameter # _max_file_size_for_diff is used by the action plugin diff --git a/tests/sanity/ignore-2.12.txt b/tests/sanity/ignore-2.12.txt index 5d5b2fd4c..3d71834db 100644 --- a/tests/sanity/ignore-2.12.txt +++ b/tests/sanity/ignore-2.12.txt @@ -1,2 +1,3 @@ .azure-pipelines/scripts/publish-codecov.py replace-urlopen plugins/modules/current_container_facts.py validate-modules:return-syntax-error +plugins/modules/docker_container_copy_into.py validate-modules:undocumented-parameter # _max_file_size_for_diff is used by the action plugin diff --git a/tests/sanity/ignore-2.13.txt b/tests/sanity/ignore-2.13.txt index 2dc9aec2e..2a06013da 100644 --- a/tests/sanity/ignore-2.13.txt +++ b/tests/sanity/ignore-2.13.txt @@ -1 +1,2 @@ .azure-pipelines/scripts/publish-codecov.py replace-urlopen +plugins/modules/docker_container_copy_into.py validate-modules:undocumented-parameter # _max_file_size_for_diff is used by the action plugin diff --git a/tests/sanity/ignore-2.14.txt b/tests/sanity/ignore-2.14.txt index 2dc9aec2e..2a06013da 100644 --- a/tests/sanity/ignore-2.14.txt +++ b/tests/sanity/ignore-2.14.txt @@ -1 +1,2 @@ .azure-pipelines/scripts/publish-codecov.py replace-urlopen +plugins/modules/docker_container_copy_into.py validate-modules:undocumented-parameter # _max_file_size_for_diff is used by the action plugin diff --git a/tests/sanity/ignore-2.15.txt b/tests/sanity/ignore-2.15.txt index 2dc9aec2e..2a06013da 100644 --- a/tests/sanity/ignore-2.15.txt +++ b/tests/sanity/ignore-2.15.txt @@ -1 +1,2 @@ .azure-pipelines/scripts/publish-codecov.py replace-urlopen +plugins/modules/docker_container_copy_into.py validate-modules:undocumented-parameter # _max_file_size_for_diff is used by the action plugin diff --git a/tests/sanity/ignore-2.9.txt b/tests/sanity/ignore-2.9.txt index 4d39d8bb6..81b68cbd8 100644 --- a/tests/sanity/ignore-2.9.txt +++ b/tests/sanity/ignore-2.9.txt @@ -7,3 +7,4 @@ plugins/module_utils/module_container/module.py compile-2.6!skip # Uses Python 2.7+ syntax plugins/module_utils/module_container/module.py import-2.6!skip # Uses Python 2.7+ syntax plugins/modules/docker_container.py import-2.6!skip # Import uses Python 2.7+ syntax +plugins/modules/docker_container_copy_into.py validate-modules:undocumented-parameter # _max_file_size_for_diff is used by the action plugin diff --git a/tests/unit/plugins/module_utils/test__scramble.py b/tests/unit/plugins/module_utils/test__scramble.py new file mode 100644 index 000000000..5718fe340 --- /dev/null +++ b/tests/unit/plugins/module_utils/test__scramble.py @@ -0,0 +1,29 @@ +# Copyright 2022 Red Hat | Ansible +# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt) +# SPDX-License-Identifier: GPL-3.0-or-later + +from __future__ import (absolute_import, division, print_function) +__metaclass__ = type + +import pytest +import tarfile + +from ansible_collections.community.docker.plugins.module_utils._scramble import ( + scramble, + unscramble, +) + + +@pytest.mark.parametrize('plaintext, key, scrambled', [ + (u'', b'0', '=S='), + (u'hello', b'\x00', '=S=aGVsbG8='), + (u'hello', b'\x01', '=S=aWRtbW4='), +]) +def test_scramble_unscramble(plaintext, key, scrambled): + scrambled_ = scramble(plaintext, key) + print('{0!r} == {1!r}'.format(scrambled_, scrambled)) + assert scrambled_ == scrambled + + plaintext_ = unscramble(scrambled, key) + print('{0!r} == {1!r}'.format(plaintext_, plaintext)) + assert plaintext_ == plaintext From 748f57d7c2dd7f1007f64999d232036a88324062 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sun, 8 Jan 2023 20:53:53 +0100 Subject: [PATCH 24/27] Improve error handling. --- plugins/module_utils/copy.py | 7 ++++++- plugins/modules/docker_container_copy_into.py | 2 +- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/plugins/module_utils/copy.py b/plugins/module_utils/copy.py index 8926db383..a7409a11a 100644 --- a/plugins/module_utils/copy.py +++ b/plugins/module_utils/copy.py @@ -384,10 +384,15 @@ def _execute_command(client, container, command, log=None, check_rc=False): try: exec_data = client.post_json_to_json('/containers/{0}/exec', container, data=data) + except NotFound as e: + raise_from( + DockerFileCopyError('Could not find container "{container}"'.format(container=container)), + e, + ) except APIError as e: if e.response is not None and e.response.status_code == 409: raise_from( - DockerFileCopyError('Cannot execute command in paused container "{0}"'.format(container)), + DockerFileCopyError('Cannot execute command in paused container "{container}"'.format(container=container)), e, ) raise diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py index c4e7367a6..fe445567e 100644 --- a/plugins/modules/docker_container_copy_into.py +++ b/plugins/modules/docker_container_copy_into.py @@ -850,7 +850,7 @@ def main(): # Can happen if a user explicitly passes `content: null` or `path: null`... client.fail('One of path and content must be supplied') except NotFound as exc: - client.fail('Could not find container "{1}" or resource in it ({0})'.format(exc, container), exception=traceback.format_exc()) + client.fail('Could not find container "{1}" or resource in it ({0})'.format(exc, container)) except APIError as exc: client.fail('An unexpected Docker error occurred for container "{1}": {0}'.format(exc, container), exception=traceback.format_exc()) except DockerException as exc: From 0fc727ffcaf41c042bc2d19082f17f27475a7a49 Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sun, 8 Jan 2023 21:13:42 +0100 Subject: [PATCH 25/27] Lint and improve. --- plugins/module_utils/_scramble.py | 17 +++++++++++++++++ plugins/modules/docker_container_copy_into.py | 8 +++----- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/plugins/module_utils/_scramble.py b/plugins/module_utils/_scramble.py index 47cfb8d9d..103257311 100644 --- a/plugins/module_utils/_scramble.py +++ b/plugins/module_utils/_scramble.py @@ -6,12 +6,28 @@ __metaclass__ = type import base64 +import random from ansible.module_utils.common.text.converters import to_bytes, to_native, to_text from ansible.module_utils.six import PY2 +def generate_insecure_key(): + '''Do NOT use this for cryptographic purposes!''' + while True: + # Generate a one-byte key. Right now the functions below do not use more + # than one byte, so this is sufficient. + if PY2: + key = chr(random.randint(0, 255)) + else: + key = bytes([random.randint(0, 255)]) + # Return anything that is not zero + if key != b'\x00': + return key + + def scramble(value, key): + '''Do NOT use this for cryptographic purposes!''' if len(key) < 1: raise ValueError('Key must be at least one byte') value = to_bytes(value) @@ -25,6 +41,7 @@ def scramble(value, key): def unscramble(value, key): + '''Do NOT use this for cryptographic purposes!''' if len(key) < 1: raise ValueError('Key must be at least one byte') if not value.startswith(u'=S='): diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py index fe445567e..16972ccdc 100644 --- a/plugins/modules/docker_container_copy_into.py +++ b/plugins/modules/docker_container_copy_into.py @@ -168,7 +168,7 @@ stat_file, ) -from ansible_collections.community.docker.plugins.module_utils._scramble import scramble +from ansible_collections.community.docker.plugins.module_utils._scramble import generate_insecure_key, scramble def are_fileobjs_equal(f1, f2): @@ -748,9 +748,7 @@ def copy_content_into_container(client, container, content, container_path, foll ) if diff: # Since the content is no_log, make sure that the before/after strings look sufficiently different - key = b'\x00' - while key[0] == 0: - key = random.randbytes(1) + key = generate_insecure_key() diff['scrambled_diff'] = base64.b64encode(key) for k in ('before', 'after'): if k in diff: @@ -774,7 +772,7 @@ def main(): content_is_b64=dict(type='bool', default=False), # Undocumented parameters for use by the action plugin - _max_file_size_for_diff=dict(type='int', required=True), + _max_file_size_for_diff=dict(type='int'), ) client = AnsibleDockerClient( From f644f0a7dbc0ac3786250c378eea4903b2f8a7cd Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Sun, 8 Jan 2023 21:32:14 +0100 Subject: [PATCH 26/27] Set owner/group ID to avoid ID lookup (which fails in paused containers). --- .../targets/generic_connection_tests/tasks/main.yml | 4 ++++ .../targets/setup_docker_registry/tasks/setup-frontend.yml | 4 ++++ 2 files changed, 8 insertions(+) diff --git a/tests/integration/targets/generic_connection_tests/tasks/main.yml b/tests/integration/targets/generic_connection_tests/tasks/main.yml index e0bd9c8b8..4a0c63361 100644 --- a/tests/integration/targets/generic_connection_tests/tasks/main.yml +++ b/tests/integration/targets/generic_connection_tests/tasks/main.yml @@ -47,6 +47,8 @@ container: '{{ daemon_nginx_frontend }}' path: '{{ remote_tmp_dir }}/{{ item }}' container_path: '/etc/nginx/{{ item }}' + owner_id: 0 + group_id: 0 loop: - nginx.conf register: can_copy_files @@ -101,6 +103,8 @@ container: '{{ daemon_nginx_frontend }}' path: '{{ remote_tmp_dir }}/{{ item }}' container_path: '/etc/nginx/{{ item }}' + owner_id: 0 + group_id: 0 loop: - ca.pem - cert.pem diff --git a/tests/integration/targets/setup_docker_registry/tasks/setup-frontend.yml b/tests/integration/targets/setup_docker_registry/tasks/setup-frontend.yml index 8fa2e322e..66bc1daee 100644 --- a/tests/integration/targets/setup_docker_registry/tasks/setup-frontend.yml +++ b/tests/integration/targets/setup_docker_registry/tasks/setup-frontend.yml @@ -43,6 +43,8 @@ container: '{{ docker_registry_container_name_frontend }}' path: '{{ remote_tmp_dir }}/{{ item }}' container_path: '/etc/nginx/{{ item }}' + owner_id: 0 + group_id: 0 loop: - nginx.conf - nginx.htpasswd @@ -78,6 +80,8 @@ container: '{{ docker_registry_container_name_frontend }}' path: '{{ remote_tmp_dir }}/{{ item }}' container_path: '/etc/nginx/{{ item }}' + owner_id: 0 + group_id: 0 loop: - cert.pem - cert.key From d83becf8149807a3d62300420e24d915c207227c Mon Sep 17 00:00:00 2001 From: Felix Fontein Date: Mon, 9 Jan 2023 06:09:46 +0100 Subject: [PATCH 27/27] Apply suggestions from code review Co-authored-by: Brian Scholer <1260690+briantist@users.noreply.github.com> --- plugins/module_utils/copy.py | 2 +- plugins/modules/docker_container_copy_into.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/module_utils/copy.py b/plugins/module_utils/copy.py index a7409a11a..6df84598a 100644 --- a/plugins/module_utils/copy.py +++ b/plugins/module_utils/copy.py @@ -303,7 +303,7 @@ def fetch_file_ex(client, container, in_path, process_none, process_regular, pro params={'path': in_path}, headers={'Accept-Encoding': 'identity'}, ) - except NotFound as dummy: + except NotFound: return process_none(in_path) with tarfile.open(fileobj=_stream_generator_to_fileobj(stream), mode='r|') as tar: diff --git a/plugins/modules/docker_container_copy_into.py b/plugins/modules/docker_container_copy_into.py index 16972ccdc..8c89ce3c4 100644 --- a/plugins/modules/docker_container_copy_into.py +++ b/plugins/modules/docker_container_copy_into.py @@ -44,7 +44,7 @@ content: description: - The file's content. - - If you plan to provide binary data, use the I(content_is_b64) option. + - If you plan to provide binary data, provide it pre-encoded to base64, and set I(content_is_b64=true). - Mutually exclusive with I(path). One of I(content) and I(path) is required. type: str content_is_b64: