Skip to content

Commit

Permalink
fix: fix path mounting when running in Docker
Browse files Browse the repository at this point in the history
Currently pre-commit mounts the current directory to /src and uses
current directory name as mount base.
However this does not work when pre-commit is run inside the container
on some mounted path already, because mount points are relative to the
host, not to the container.

Fixes #1387
  • Loading branch information
Oleg Kainov committed Apr 27, 2021
1 parent 60bf370 commit 237334c
Show file tree
Hide file tree
Showing 2 changed files with 166 additions and 4 deletions.
34 changes: 33 additions & 1 deletion pre_commit/languages/docker.py
@@ -1,5 +1,7 @@
import hashlib
import json
import os
import socket
from typing import Sequence
from typing import Tuple

Expand All @@ -8,13 +10,43 @@
from pre_commit.languages import helpers
from pre_commit.prefix import Prefix
from pre_commit.util import clean_path_on_failure
from pre_commit.util import cmd_output_b

ENVIRONMENT_DIR = 'docker'
PRE_COMMIT_LABEL = 'PRE_COMMIT'
get_default_version = helpers.basic_get_default_version
healthy = helpers.basic_healthy


def _is_in_docker() -> bool:
try:
with open('/proc/1/cgroup', 'rb') as f:
return b'docker' in f.read()
except FileNotFoundError:
return False


def _get_docker_path(path: str) -> str:
if not _is_in_docker():
return path
hostname = socket.gethostname()

_, out, _ = cmd_output_b(
'docker', 'inspect', hostname,
)

docker_output = json.loads(out)
for mount_path in docker_output[0]['HostConfig']['Binds']:
src_path, to_path, *_ = mount_path.split(':')
if os.path.commonpath((path, to_path)) == to_path:
# So there is something in common,
# and we can proceed remapping it
return path.replace(to_path, src_path)
# we're in Docker, but the path is not mounted, cannot really do anything,
# so fall back to original path
return path


def md5(s: str) -> str: # pragma: win32 no cover
return hashlib.md5(s.encode()).hexdigest()

Expand Down Expand Up @@ -73,7 +105,7 @@ def docker_cmd() -> Tuple[str, ...]: # pragma: win32 no cover
# https://docs.docker.com/engine/reference/commandline/run/#mount-volumes-from-container-volumes-from
# The `Z` option tells Docker to label the content with a private
# unshared label. Only the current container can use a private volume.
'-v', f'{os.getcwd()}:/src:rw,Z',
'-v', f'{_get_docker_path(os.getcwd())}:/src:rw,Z',
'--workdir', '/src',
)

Expand Down
136 changes: 133 additions & 3 deletions tests/languages/docker_test.py
@@ -1,14 +1,144 @@
import builtins
import json
from typing import List
from unittest import mock

import pytest

from pre_commit.languages import docker


def test_docker_fallback_user():
def invalid_attribute():
raise AttributeError

with mock.patch.multiple(
'os', create=True,
getuid=invalid_attribute,
getgid=invalid_attribute,
'os', create=True,
getuid=invalid_attribute,
getgid=invalid_attribute,
):
assert docker.get_docker_user() == ()


class TestInDocker:

@pytest.fixture
def mock_file_fixture(self):
return lambda read_data: mock.patch.object(
builtins, 'open',
new_callable=mock.mock_open,
read_data=read_data,
)

def test_in_docker_no_file(self, mock_file_fixture):
with mock_file_fixture(None) as m:
m.side_effect = FileNotFoundError

assert docker._is_in_docker() is False
m.assert_called()

def test_in_docker_docker_in_file(self, mock_file_fixture):
with mock_file_fixture(b'tdockert'):
assert docker._is_in_docker() is True

def test_in_docker_docker_not_in_file(self, mock_file_fixture):
with mock_file_fixture(b'testtest'):
assert docker._is_in_docker() is False


class TestDockerPath:
@pytest.fixture
def in_docker(self):
with mock.patch.object(docker, '_is_in_docker', return_value=True):
yield

@pytest.fixture
def not_in_docker(self):
with mock.patch.object(docker, '_is_in_docker', return_value=False):
yield

def test_not_in_docker_returns_same(self, not_in_docker):
assert docker._get_docker_path('abc') == 'abc'

def test_in_docker_no_binds_same_path(self, in_docker):
binds_list: List[str] = []
output_string = json.dumps(
[{'HostConfig': {'Binds': binds_list}}],
).encode('utf-8')
with mock.patch.object(
docker, 'cmd_output_b',
return_value=(0, output_string, b''),
):
assert docker._get_docker_path('abc') == 'abc'

def test_in_docker_binds_path_equal(self, in_docker):
binds_list = [
'/opt/my_code:/project',
]
output_string = json.dumps(
[{'HostConfig': {'Binds': binds_list}}],
).encode('utf-8')
with mock.patch.object(
docker, 'cmd_output_b',
return_value=(0, output_string, b''),
):
assert docker._get_docker_path('/project') == '/opt/my_code'

def test_in_docker_binds_path_complex(self, in_docker):
binds_list = [
'/opt/my_code:/project',
]
output_string = json.dumps(
[{'HostConfig': {'Binds': binds_list}}],
).encode('utf-8')
with mock.patch.object(
docker, 'cmd_output_b',
return_value=(0, output_string, b''),
):
path = '/project/test/something'
expected = '/opt/my_code/test/something'
assert docker._get_docker_path(path) == expected

def test_in_docker_no_substring(self, in_docker):
binds_list = [
'/opt/my_code:/project',
]
output_string = json.dumps(
[{'HostConfig': {'Binds': binds_list}}],
).encode('utf-8')
with mock.patch.object(
docker, 'cmd_output_b',
return_value=(0, output_string, b''),
):
path = '/projectSUffix/test/something'
assert docker._get_docker_path(path) == path

def test_in_docker_binds_path_many_binds(self, in_docker):
binds_list = [
'/something_random:/not_related',
'/opt/my_code:/project',
'/something_random2:/not_related2',
]
output_string = json.dumps(
[{'HostConfig': {'Binds': binds_list}}],
).encode('utf-8')
with mock.patch.object(
docker, 'cmd_output_b',
return_value=(0, output_string, b''),
):
assert docker._get_docker_path('/project') == '/opt/my_code'

def test_in_docker_two_colons(self, in_docker):
binds_list = [
'/opt/my_code:/project:qqqrandom',
]
output_string = json.dumps(
[{'HostConfig': {'Binds': binds_list}}],
).encode('utf-8')
with mock.patch.object(
docker, 'cmd_output_b',
return_value=(0, output_string, b''),
):
path = '/project/test/something'
expected = '/opt/my_code/test/something'
assert docker._get_docker_path(path) == expected

0 comments on commit 237334c

Please sign in to comment.