Skip to content

Build: pass PATH environment variable to Docker container #10133

New issue

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

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

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Mar 16, 2023
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 44 additions & 28 deletions readthedocs/doc_builder/environments.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from docker.errors import APIError as DockerAPIError
from docker.errors import DockerException
from docker.errors import NotFound as DockerNotFoundError
from requests.exceptions import ConnectionError, ReadTimeout
from requests.exceptions import ConnectionError, ReadTimeout # noqa
from requests_toolbelt.multipart.encoder import MultipartEncoder

from readthedocs.api.v2.client import api as api_v2
Expand Down Expand Up @@ -73,7 +73,7 @@ def __init__(
bin_path=None,
record_as_success=False,
demux=False,
**kwargs,
**kwargs, # pylint: disable=unused-argument
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like we have linting configured too strongly, if so much of this PR is linting hints... Maybe that's making the code better, but I'm not convinced this check is useful?

):
self.command = command
self.shell = shell
Expand Down Expand Up @@ -252,8 +252,8 @@ def save(self):
{key: str(value) for key, value in data.items()}
)
resource = api_v2.command
resp = resource._store['session'].post(
resource._store['base_url'] + '/',
resp = resource._store["session"].post( # pylint: disable=protected-access
resource._store["base_url"] + "/", # pylint: disable=protected-access
data=encoder,
headers={
'Content-Type': encoder.content_type,
Expand Down Expand Up @@ -301,11 +301,40 @@ def run(self):

self.start_time = datetime.utcnow()
client = self.build_env.get_client()

# Create a copy of the environment to update PATH variable
environment = self._environment.copy()
# Default PATH variable
# This default comes from our Docker image:
#
# $ docker run --user docs -it --rm readthedocs/build:ubuntu-22.04 /bin/bash
# docs@bfe702e31cdd:~$ echo $PATH
# /home/docs/.asdf/shims:/home/docs/.asdf/bin
# :/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin
# docs@bfe702e31cdd:~$
asdf_paths = (
f"/home/{settings.RTD_DOCKER_USER}/.asdf/shims"
f":/home/{settings.RTD_DOCKER_USER}/.asdf/bin"
)
if settings.RTD_DOCKER_COMPOSE:
asdf_paths += ":/root/.asdf/shims:/root/.asdf/bin"

default_paths = "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin"
environment["PATH"] = f"{asdf_paths}:{default_paths}"

# Prepend the BIN_PATH if it's defined
if self.bin_path:
original_path = environment.get("PATH")
escaped_bin_path = self._escape_command(self.bin_path)
environment["PATH"] = escaped_bin_path
if original_path:
environment["PATH"] = f"{escaped_bin_path}:{original_path}"

try:
exec_cmd = client.exec_create(
container=self.build_env.container_id,
cmd=self.get_wrapped_command(),
environment=self._environment,
environment=environment,
user=self.user,
workdir=self.cwd,
stdout=True,
Expand Down Expand Up @@ -357,31 +386,18 @@ def get_wrapped_command(self):
"""
Wrap command in a shell and optionally escape special bash characters.

In order to set the current working path inside a docker container, we
need to wrap the command in a shell call manually.

Some characters will be interpreted as shell characters without
escaping, such as: ``pip install requests<0.8``. When passing
``escape_command=True`` in the init method this escapes a good majority
of those characters.
"""
prefix = ''
if self.bin_path:
bin_path = self._escape_command(self.bin_path)
prefix += f'PATH={bin_path}:$PATH '

command = (
' '.join(
self._escape_command(part) if self.escape_command else part
for part in self.command
)
)
return (
"/bin/sh -c '{prefix}{cmd}'".format(
prefix=prefix,
cmd=command,
)
)
return f"/bin/bash -c '{command}'"

def _escape_command(self, cmd):
r"""Escape the command by prefixing suspicious chars with `\`."""
Expand Down Expand Up @@ -524,14 +540,14 @@ class BuildEnvironment(BaseEnvironment):
"""

def __init__(
self,
project=None,
version=None,
build=None,
config=None,
environment=None,
record=True,
**kwargs,
self,
project=None,
version=None,
build=None,
config=None,
environment=None,
record=True,
**kwargs, # pylint: disable=unused-argument
):
super().__init__(project, environment)
self.version = version
Expand All @@ -557,7 +573,7 @@ def run(self, *cmd, **kwargs):
})
return super().run(*cmd, **kwargs)

def run_command_class(self, *cmd, **kwargs): # pylint: disable=arguments-differ
def run_command_class(self, *cmd, **kwargs): # pylint: disable=signature-differs
kwargs.update({
'build_env': self,
})
Expand Down