From 7c16c3e53ffffc39d792dfd984bc470b494bc48a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabrice=20Flore-Th=C3=A9bault?= Date: Tue, 2 Jun 2020 14:48:23 +0200 Subject: [PATCH 01/11] fix #1243 - the -u option is not necessary on podman --- pre_commit/languages/docker.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py index 4091492cc..b0b7727f3 100644 --- a/pre_commit/languages/docker.py +++ b/pre_commit/languages/docker.py @@ -1,5 +1,6 @@ import hashlib import os +import subprocess from typing import Sequence from typing import Tuple @@ -78,7 +79,11 @@ def install_environment( def get_docker_user() -> Tuple[str, ...]: # pragma: win32 no cover try: - return ('-u', f'{os.getuid()}:{os.getgid()}') + ver_cmd = 'docker', '--version' + if subprocess.check_output(ver_cmd).startswith(b'podman version '): + return () + else: + return ('-u', f'{os.getuid()}:{os.getgid()}') except AttributeError: return () From 5c1cd8168aa6608242c5b06190f25bd64e5e83f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabrice=20Flore-Th=C3=A9bault?= Date: Wed, 3 Jun 2020 17:40:22 +0200 Subject: [PATCH 02/11] the -u option is not necessary on a rootless environment --- pre_commit/languages/docker.py | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py index b0b7727f3..9f2e137fe 100644 --- a/pre_commit/languages/docker.py +++ b/pre_commit/languages/docker.py @@ -79,11 +79,19 @@ def install_environment( def get_docker_user() -> Tuple[str, ...]: # pragma: win32 no cover try: - ver_cmd = 'docker', '--version' - if subprocess.check_output(ver_cmd).startswith(b'podman version '): - return () - else: + rootless = False + output = subprocess.check_output(('docker', 'system', 'info'), text=True) + for line in output.splitlines(): + # rootless docker has "rootless" + # rootless podman has "rootless: true" + if line.strip().startswith('rootless'): + if not 'false' in line: + rootless = True + break + if not rootless: return ('-u', f'{os.getuid()}:{os.getgid()}') + else: + return() except AttributeError: return () From a8887d2cfe3c5a1c1fb29b5029162e575bc49093 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabrice=20Flore-Th=C3=A9bault?= Date: Wed, 3 Jun 2020 17:45:44 +0200 Subject: [PATCH 03/11] the -u option is not necessary on a rootless environment (rewrite by hroncok) --- pre_commit/languages/docker.py | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py index 9f2e137fe..e3970f49b 100644 --- a/pre_commit/languages/docker.py +++ b/pre_commit/languages/docker.py @@ -78,22 +78,15 @@ def install_environment( def get_docker_user() -> Tuple[str, ...]: # pragma: win32 no cover - try: - rootless = False - output = subprocess.check_output(('docker', 'system', 'info'), text=True) - for line in output.splitlines(): - # rootless docker has "rootless" - # rootless podman has "rootless: true" - if line.strip().startswith('rootless'): - if not 'false' in line: - rootless = True - break - if not rootless: - return ('-u', f'{os.getuid()}:{os.getgid()}') - else: - return() - except AttributeError: - return () + output = subprocess.check_output(('docker', 'system', 'info'), text=True) + for line in output.splitlines(): + # rootless docker has "rootless" + # rootless podman has "rootless: true" + if line.strip().startswith('rootless'): + if not 'false' in line: + return () # no -u for rootless + break + return ('-u', f'{os.getuid()}:{os.getgid()}') def docker_cmd() -> Tuple[str, ...]: # pragma: win32 no cover From 4416c3d51220846e9de9624735f6943744388bda Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabrice=20Flore-Th=C3=A9bault?= Date: Wed, 3 Jun 2020 18:04:13 +0200 Subject: [PATCH 04/11] add AttributeError again --- pre_commit/languages/docker.py | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py index e3970f49b..244c2341e 100644 --- a/pre_commit/languages/docker.py +++ b/pre_commit/languages/docker.py @@ -78,15 +78,21 @@ def install_environment( def get_docker_user() -> Tuple[str, ...]: # pragma: win32 no cover - output = subprocess.check_output(('docker', 'system', 'info'), text=True) - for line in output.splitlines(): - # rootless docker has "rootless" - # rootless podman has "rootless: true" - if line.strip().startswith('rootless'): - if not 'false' in line: - return () # no -u for rootless - break - return ('-u', f'{os.getuid()}:{os.getgid()}') + try: + output = subprocess.check_output( + ('docker', 'system', 'info'), + text=True, + ) + for line in output.splitlines(): + # rootless docker has "rootless" + # rootless podman has "rootless: true" + if line.strip().startswith('rootless'): + if 'false' not in line: + return () # no -u for rootless + break + return ('-u', f'{os.getuid()}:{os.getgid()}') + except AttributeError: + return () def docker_cmd() -> Tuple[str, ...]: # pragma: win32 no cover From 818a9092224e71b75c3359d9a120ceb14bd04823 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabrice=20Flore-Th=C3=A9bault?= Date: Thu, 4 Jun 2020 08:22:16 +0200 Subject: [PATCH 05/11] output before try --- pre_commit/languages/docker.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py index 244c2341e..eea1bdcd4 100644 --- a/pre_commit/languages/docker.py +++ b/pre_commit/languages/docker.py @@ -78,11 +78,11 @@ def install_environment( def get_docker_user() -> Tuple[str, ...]: # pragma: win32 no cover + output = subprocess.check_output( + ('docker', 'system', 'info'), + text=True, + ) try: - output = subprocess.check_output( - ('docker', 'system', 'info'), - text=True, - ) for line in output.splitlines(): # rootless docker has "rootless" # rootless podman has "rootless: true" From 79528ec13edabe842aa0340445affe5044756765 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabrice=20Flore-Th=C3=A9bault?= Date: Thu, 4 Jun 2020 10:11:03 +0200 Subject: [PATCH 06/11] Not just output, move this whole thing out. --- pre_commit/languages/docker.py | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py index eea1bdcd4..72a98ea56 100644 --- a/pre_commit/languages/docker.py +++ b/pre_commit/languages/docker.py @@ -82,14 +82,14 @@ def get_docker_user() -> Tuple[str, ...]: # pragma: win32 no cover ('docker', 'system', 'info'), text=True, ) + for line in output.splitlines(): + # rootless docker has "rootless" + # rootless podman has "rootless: true" + if line.strip().startswith('rootless'): + if 'false' not in line: + return () # no -u for rootless + break try: - for line in output.splitlines(): - # rootless docker has "rootless" - # rootless podman has "rootless: true" - if line.strip().startswith('rootless'): - if 'false' not in line: - return () # no -u for rootless - break return ('-u', f'{os.getuid()}:{os.getgid()}') except AttributeError: return () From b0fc24f681e885428650e77e9f48955757a971de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabrice=20Flore-Th=C3=A9bault?= Date: Mon, 15 Jun 2020 13:50:45 +0200 Subject: [PATCH 07/11] docker.py reworked, thanks @hroncok --- pre_commit/languages/docker.py | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py index 72a98ea56..c0a004889 100644 --- a/pre_commit/languages/docker.py +++ b/pre_commit/languages/docker.py @@ -1,6 +1,6 @@ +import functools import hashlib import os -import subprocess from typing import Sequence from typing import Tuple @@ -10,6 +10,7 @@ from pre_commit.prefix import Prefix from pre_commit.util import CalledProcessError from pre_commit.util import clean_path_on_failure +from pre_commit.util import cmd_output from pre_commit.util import cmd_output_b ENVIRONMENT_DIR = 'docker' @@ -77,18 +78,24 @@ def install_environment( os.mkdir(directory) -def get_docker_user() -> Tuple[str, ...]: # pragma: win32 no cover - output = subprocess.check_output( +@functools.lru_cache(maxsize=1) +def docker_is_rootless() -> bool: + returncode, stdout, stderr = cmd_output( ('docker', 'system', 'info'), - text=True, ) - for line in output.splitlines(): + for line in stdout.splitlines(): # rootless docker has "rootless" # rootless podman has "rootless: true" if line.strip().startswith('rootless'): if 'false' not in line: - return () # no -u for rootless + return True break + return False + + +def get_docker_user() -> Tuple[str, ...]: # pragma: win32 no cover + if: docker_is_rootless(): + return () try: return ('-u', f'{os.getuid()}:{os.getgid()}') except AttributeError: From da85e738c746dc27a8a524652095441e5de4eef0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabrice=20Flore-Th=C3=A9bault?= Date: Mon, 15 Jun 2020 14:14:29 +0200 Subject: [PATCH 08/11] docker.py tests, thanks @hroncok --- tests/languages/docker_test.py | 196 +++++++++++++++++++++++++++++++++ 1 file changed, 196 insertions(+) diff --git a/tests/languages/docker_test.py b/tests/languages/docker_test.py index b65b2235a..34dc5e4f8 100644 --- a/tests/languages/docker_test.py +++ b/tests/languages/docker_test.py @@ -3,7 +3,187 @@ from pre_commit.languages import docker from pre_commit.util import CalledProcessError +DOCKER_NOT_ROOTLESS_SYSTEM_INFO = ''' +Client: + Debug Mode: false +Server: + Containers: 8 + Running: 0 + Paused: 0 + Stopped: 8 + Images: 2 + Server Version: 19.03.8 + Storage Driver: overlay2 + Backing Filesystem: + Supports d_type: true + Native Overlay Diff: true + Logging Driver: journald + Cgroup Driver: systemd + Plugins: + Volume: local + Network: bridge host ipvlan macvlan null overlay + Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog + Swarm: inactive + Runtimes: runc + Default Runtime: runc + Init Binary: /usr/libexec/docker/docker-init + containerd version: + runc version: fbdbaf85ecbc0e077f336c03062710435607dbf1 + init version: + Security Options: + seccomp + Profile: default + selinux + Kernel Version: 5.6.14-300.fc32.x86_64 + Operating System: Fedora 32 (Thirty Two) + OSType: linux + Architecture: x86_64 + CPUs: 8 + Total Memory: 15.29GiB + Name: carbon + ID: IUGE:4L5B:VVTV:JGIA:IIJN:G7MG:TVGF:XBVW:YHF7:MYRK:L524:4HBK + Docker Root Dir: /var/lib/docker + Debug Mode: false + Registry: https://index.docker.io/v1/ + Labels: + Experimental: false + Insecure Registries: + 127.0.0.0/8 + Live Restore Enabled: true +''' + +DOCKER_ROOTLESS_SYSTEM_INFO = ''' +Client: + Debug Mode: false + +Server: + Containers: 0 + Running: 0 + Paused: 0 + Stopped: 0 + Images: 0 + Server Version: 19.03.11 + Storage Driver: vfs + Logging Driver: json-file + Cgroup Driver: none + Plugins: + Volume: local + Network: bridge host ipvlan macvlan null overlay + Log: awslogs fluentd gcplogs gelf journald json-file local logentries splunk syslog + Swarm: inactive + Runtimes: runc + Default Runtime: runc + Init Binary: docker-init + containerd version: 7ad184331fa3e55e52b890ea95e65ba581ae3429 + runc version: dc9208a3303feef5b3839f4323d9beb36df0a9dd + init version: fec3683 + Security Options: + seccomp + Profile: default + rootless + Kernel Version: 5.6.16-300.fc32.x86_64 + Operating System: Fedora 32 (Workstation Edition) + OSType: linux + Architecture: x86_64 + CPUs: 8 + Total Memory: 23.36GiB + Name: laptop.centsix.taz + ID: 6EMS:RSJ4:NXM3:3D56:CV4N:VEUX:LBFM:KWHK:P7XA:HTCS:GVSC:UVUE + Docker Root Dir: /home/remote/user/.local/share/docker + Debug Mode: false + Registry: https://index.docker.io/v1/ + Labels: + Experimental: true + Insecure Registries: + 127.0.0.0/8 + Live Restore Enabled: false + Product License: Community Engine +''' + +PODMAN_SYSTEM_INFO = ''' +host: + arch: amd64 + buildahVersion: 1.14.9 + cgroupVersion: v2 + conmon: + package: conmon-2.0.17-1.fc32.x86_64 + path: /usr/libexec/crio/conmon + version: 'conmon version 2.0.17, commit: bb8e273f5925c1a51737644637ef65d094a67ab1' + cpus: 8 + distribution: + distribution: fedora + version: "32" + eventLogger: file + hostname: laptop.centsix.taz + idMappings: + gidmap: + - container_id: 0 + host_id: 1001 + size: 1 + - container_id: 1 + host_id: 200000 + size: 65536 + uidmap: + - container_id: 0 + host_id: 105971 + size: 1 + - container_id: 1 + host_id: 200000 + size: 65536 + kernel: 5.6.16-300.fc32.x86_64 + memFree: 7256297472 + memTotal: 25081421824 + ociRuntime: + name: crun + package: crun-0.13-2.fc32.x86_64 + path: /usr/bin/crun + version: |- + crun version 0.13 + commit: e79e4de4ac16da0ce48777afb72c6241de870525 + spec: 1.0.0 + +SYSTEMD +SELINUX +APPARMOR +CAP +SECCOMP +EBPF +YAJL + os: linux + rootless: true + slirp4netns: + executable: /usr/bin/slirp4netns + package: slirp4netns-1.0.0-1.fc32.x86_64 + version: |- + slirp4netns version 1.0.0 + commit: a3be729152a33e692cd28b52f664defbf2e7810a + libslirp: 4.2.0 + swapFree: 20372766720 + swapTotal: 20372766720 + uptime: 4h 46m 1.38s (Approximately 0.17 days) +registries: {} +store: + configFile: /home/remote/user/.config/containers/storage.conf + containerStore: + number: 66 + paused: 0 + running: 0 + stopped: 66 + graphDriverName: overlay + graphOptions: + overlay.mount_program: + Executable: /usr/bin/fuse-overlayfs + Package: fuse-overlayfs-1.0.0-1.fc32.x86_64 + Version: |- + fusermount3 version: 3.9.1 + fuse-overlayfs: version 1.0.0 + FUSE library version 3.9.1 + using FUSE kernel interface version 7.31 + graphRoot: /home/remote/user/.local/share/containers/storage + graphStatus: + Backing Filesystem: extfs + Native Overlay Diff: "false" + Supports d_type: "true" + Using metacopy: "false" + imageStore: + number: 23 + runRoot: /tmp/105971 + volumePath: /home/remote/user/.local/share/containers/storage/volumes +''' def test_docker_is_running_process_error(): with mock.patch( 'pre_commit.languages.docker.cmd_output_b', @@ -21,3 +201,19 @@ def invalid_attribute(): getgid=invalid_attribute, ): assert docker.get_docker_user() == () + + +def test_docker_is_not_rootless(): + with mock.patch.object(docker, 'cmd_output', return_value=(0, DOCKER_NOT_ROOTLESS_SYSTEM_INFO, '')): + assert docker.docker_is_rootless() == False + + +def test_docker_is_rootless(): + with mock.patch.object(docker, 'cmd_output', return_value=(0, DOCKER_ROOTLESS_SYSTEM_INFO, '')): + assert docker.docker_is_rootless() == True + + +def test_podman_is_rootless(): + with mock.patch.object(docker, 'cmd_output', return_value=(0, PODMAN_SYSTEM_INFO, '')): + assert docker.docker_is_rootless() == True + From 12a40fdab29c6612d9b0b484ddabf18f506808cf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabrice=20Flore-Th=C3=A9bault?= Date: Mon, 15 Jun 2020 14:44:05 +0200 Subject: [PATCH 09/11] docker.py tests thanks @hroncok --- pre_commit/languages/docker.py | 13 +++++++++---- tests/languages/docker_test.py | 34 ++++++++++++++++++++++------------ 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py index c0a004889..81cfbeed4 100644 --- a/pre_commit/languages/docker.py +++ b/pre_commit/languages/docker.py @@ -78,23 +78,28 @@ def install_environment( os.mkdir(directory) -@functools.lru_cache(maxsize=1) -def docker_is_rootless() -> bool: +def _docker_is_rootless() -> bool: returncode, stdout, stderr = cmd_output( - ('docker', 'system', 'info'), + 'docker', 'system', 'info', ) for line in stdout.splitlines(): # rootless docker has "rootless" # rootless podman has "rootless: true" if line.strip().startswith('rootless'): if 'false' not in line: + print(line) return True break return False +@functools.lru_cache(maxsize=1) +def docker_is_rootless() -> bool: + return _docker_is_rootless() + + def get_docker_user() -> Tuple[str, ...]: # pragma: win32 no cover - if: docker_is_rootless(): + if docker_is_rootless(): return () try: return ('-u', f'{os.getuid()}:{os.getgid()}') diff --git a/tests/languages/docker_test.py b/tests/languages/docker_test.py index 34dc5e4f8..808bbd6d3 100644 --- a/tests/languages/docker_test.py +++ b/tests/languages/docker_test.py @@ -28,9 +28,9 @@ Runtimes: runc Default Runtime: runc Init Binary: /usr/libexec/docker/docker-init - containerd version: + containerd version: runc version: fbdbaf85ecbc0e077f336c03062710435607dbf1 - init version: + init version: Security Options: seccomp Profile: default @@ -51,7 +51,7 @@ Insecure Registries: 127.0.0.0/8 Live Restore Enabled: true -''' +''' # noqa DOCKER_ROOTLESS_SYSTEM_INFO = ''' Client: @@ -99,7 +99,7 @@ 127.0.0.0/8 Live Restore Enabled: false Product License: Community Engine -''' +''' # noqa PODMAN_SYSTEM_INFO = ''' host: @@ -183,7 +183,9 @@ number: 23 runRoot: /tmp/105971 volumePath: /home/remote/user/.local/share/containers/storage/volumes -''' +''' # noqa + + def test_docker_is_running_process_error(): with mock.patch( 'pre_commit.languages.docker.cmd_output_b', @@ -204,16 +206,24 @@ def invalid_attribute(): def test_docker_is_not_rootless(): - with mock.patch.object(docker, 'cmd_output', return_value=(0, DOCKER_NOT_ROOTLESS_SYSTEM_INFO, '')): - assert docker.docker_is_rootless() == False + with mock.patch.object( + docker, 'cmd_output', + return_value=(0, DOCKER_NOT_ROOTLESS_SYSTEM_INFO, ''), + ): + assert docker._docker_is_rootless() is False def test_docker_is_rootless(): - with mock.patch.object(docker, 'cmd_output', return_value=(0, DOCKER_ROOTLESS_SYSTEM_INFO, '')): - assert docker.docker_is_rootless() == True + with mock.patch.object( + docker, 'cmd_output', + return_value=(0, DOCKER_ROOTLESS_SYSTEM_INFO, ''), + ): + assert docker._docker_is_rootless() is True def test_podman_is_rootless(): - with mock.patch.object(docker, 'cmd_output', return_value=(0, PODMAN_SYSTEM_INFO, '')): - assert docker.docker_is_rootless() == True - + with mock.patch.object( + docker, 'cmd_output', + return_value=(0, PODMAN_SYSTEM_INFO, ''), + ): + assert docker._docker_is_rootless() is True From dff7d60255109bd759d63337aab7f1388e0afdb3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabrice=20Flore-Th=C3=A9bault?= Date: Mon, 15 Jun 2020 14:49:33 +0200 Subject: [PATCH 10/11] docker.py tests thanks @hroncok --- pre_commit/languages/docker.py | 1 - 1 file changed, 1 deletion(-) diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py index 81cfbeed4..72c1886c6 100644 --- a/pre_commit/languages/docker.py +++ b/pre_commit/languages/docker.py @@ -87,7 +87,6 @@ def _docker_is_rootless() -> bool: # rootless podman has "rootless: true" if line.strip().startswith('rootless'): if 'false' not in line: - print(line) return True break return False From 9d4fe3a00b89bdbfce5708d4433c5d7c4f7aadb2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fabrice=20Flore-Th=C3=A9bault?= Date: Mon, 15 Jun 2020 14:53:56 +0200 Subject: [PATCH 11/11] add pragma: win32 no cover --- pre_commit/languages/docker.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pre_commit/languages/docker.py b/pre_commit/languages/docker.py index 72c1886c6..e3ed98ba6 100644 --- a/pre_commit/languages/docker.py +++ b/pre_commit/languages/docker.py @@ -78,7 +78,7 @@ def install_environment( os.mkdir(directory) -def _docker_is_rootless() -> bool: +def _docker_is_rootless() -> bool: # pragma: win32 no cover returncode, stdout, stderr = cmd_output( 'docker', 'system', 'info', ) @@ -93,7 +93,7 @@ def _docker_is_rootless() -> bool: @functools.lru_cache(maxsize=1) -def docker_is_rootless() -> bool: +def docker_is_rootless() -> bool: # pragma: win32 no cover return _docker_is_rootless()