Skip to content

Commit eea10dc

Browse files
fix(lib-injection): support non-root users [backport 1.17] (#6434)
Backport b048030 from #6047 to 1.17. The current mechanism attempts a `pip install` using the current user. Many users run their containers with a non-root user (`USER 1000` or similar in their Dockerfiles) for security purposes. The injection mechanism doesn't work with this as it requires modifying the site-packages in the application container to install the ddtrace package which most often requires super user permissions. A solution is to go back to providing an installation of the package directly via the volume which does not require and runtime writing to system directories. This solution was attempted before but was flawed in that it tried to provide all the binaries in a single directory. This was broken because `ddtrace` has varying dependency versions across Python versions which resulted in conflicts. With this approach the installation directories are split by Python version and the C-library runtime. The `sitecustomize.py` file is updated to select the appropriate installation directory at runtime. Several new test cases are added to cover this regression. A reproduction of the issue is provided with [49203e6](49203e6). The simple django test image is modified to reproduce the issue and assert that the problem is fixed. ## Risks Clashing dependencies. There is no dependency clash checking implemented. This means that if the user has a version of a dependency that ddtrace does, then the ddtrace version could take precedence (depending on the PYTHONPATH). This can be mitigated in the future by doing a run-time check on the installed versions. ## Checklist - [x] Change(s) are motivated and described in the PR description. - [x] Testing strategy is described if automated tests are not included in the PR. - [x] Risk is outlined (performance impact, potential for breakage, maintainability, etc). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] [Library release note guidelines](https://ddtrace.readthedocs.io/en/stable/contributing.html#Release-Note-Guidelines) are followed. - [x] Documentation is included (in-code, generated user docs, [public corp docs](https://github.com/DataDog/documentation/)). ## Reviewer Checklist - [x] Title is accurate. - [x] No unnecessary changes are introduced. - [x] Description motivates each change. - [x] Avoids breaking [API](https://ddtrace.readthedocs.io/en/stable/versioning.html#interfaces) changes unless absolutely necessary. - [x] Testing strategy adequately addresses listed risk(s). - [x] Change is maintainable (easy to change, telemetry, documentation). - [x] Release note makes sense to a user of the library. - [x] Reviewer has explicitly acknowledged and discussed the performance implications of this PR as reported in the benchmarks PR comment. Co-authored-by: Kyle Verhoog <[email protected]>
1 parent df5bed3 commit eea10dc

File tree

15 files changed

+346
-127
lines changed

15 files changed

+346
-127
lines changed

.github/workflows/lib-injection.yml

Lines changed: 23 additions & 43 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ jobs:
99
secrets:
1010
token: ${{ secrets.GITHUB_TOKEN }}
1111
with:
12-
ddtrace-version: v1.10
12+
ddtrace-version: v1.16.1
1313
image-tag: ${{ github.sha }}
1414

1515
test:
@@ -51,56 +51,32 @@ jobs:
5151
variant: [
5252
'dd-lib-python-init-test-django',
5353
'dd-lib-python-init-test-django-gunicorn',
54+
'dd-lib-python-init-test-django-gunicorn-alpine',
5455
'dd-lib-python-init-test-django-uvicorn',
56+
'dd-lib-python-init-test-django-no-perms',
57+
'dd-lib-python-init-test-django-pre-installed',
5558
]
5659
fail-fast: false
5760
steps:
5861
- uses: actions/checkout@v3
59-
- name: Create a docker network for the app and test agent
60-
run: |
61-
docker network create test-inject
62-
- name: Run the test agent
63-
run: |
64-
docker run \
65-
-d \
66-
--name=testagent \
67-
--network=test-inject \
68-
-p 8126:8126 \
69-
ghcr.io/datadog/dd-apm-test-agent/ddapm-test-agent:v1.7.2
70-
- name: Prepare the volume
62+
- name: Build and run the app
7163
run: |
64+
SRC="$(pwd)"
7265
cd lib-injection
73-
mkdir -p lib-injection/ddtrace_pkgs
74-
cp sitecustomize.py lib-injection/
75-
./dl_wheels.py \
76-
--ddtrace-version=v1.10 \
77-
--python-version=3.11 \
78-
--python-version=3.10 \
79-
--python-version=3.9 \
80-
--python-version=3.8 \
81-
--python-version=3.7 \
82-
--arch x86_64 \
83-
--platform manylinux2014 \
84-
--output-dir ddtrace_pkgs \
85-
--verbose
86-
- name: Build test app
87-
run: |
88-
docker build \
89-
-t ${{ matrix.variant }} \
90-
tests/lib-injection/${{ matrix.variant }}/
91-
- name: Run the app
92-
run: |
93-
docker run -d \
94-
--name ${{ matrix.variant }} \
95-
--network test-inject \
96-
-p 18080:18080 \
97-
-e PYTHONPATH=/lib-injection \
98-
-e DD_TRACE_AGENT_URL=http://testagent:8126 \
99-
-v $PWD/lib-injection:/lib-injection \
100-
${{matrix.variant}}
66+
export DDTRACE_PYTHON_VERSION="v1.16.1"
67+
export APP_CONTEXT="${SRC}/tests/lib-injection/${{matrix.variant}}"
68+
export TEMP_DIR="${SRC}/tmp/ddtrace"
69+
mkdir -p "${TEMP_DIR}"
70+
# Give the temp dir permissions, by default the docker user doesn't have permissions
71+
# to write to the filesystem.
72+
chmod 777 $TEMP_DIR
73+
# Start the lib_inject to get the files copied. This avoids a race condition with the startup of the
74+
# application.
75+
docker-compose up --build lib_inject
76+
docker-compose up --build -d
10177
# Wait for the app to start
102-
sleep 20
103-
docker logs ${{matrix.variant}}
78+
sleep 60
79+
docker-compose logs
10480
- name: Test the app
10581
run: |
10682
curl http://localhost:18080
@@ -111,3 +87,7 @@ jobs:
11187
run: |
11288
N=$(curl http://localhost:8126/test/traces | jq -r -e 'length')
11389
[[ $N == "1" ]]
90+
- name: Output app logs (LOOK HERE IF THE JOB FAILS)
91+
if: success() || failure()
92+
run: |
93+
docker-compose logs

.gitlab/build-deb-rpm.sh

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ echo -n $PYTHON_PACKAGE_VERSION > auto_inject-python.version
88

99
source common_build_functions.sh
1010

11-
# Download the megawheel
1211
mkdir -p dd-trace.dir/lib
1312

1413
# Install known compatible pip as default version shipped in Ubuntu (20.0.2)
@@ -29,7 +28,7 @@ echo `pwd`
2928
--arch aarch64 \
3029
--platform musllinux_1_1 \
3130
--platform manylinux2014 \
32-
--output-dir dd-trace.dir/lib \
31+
--output-dir dd-trace.dir/lib/ddtrace_pkgs \
3332
--verbose
3433

3534
cp ../lib-injection/sitecustomize.py dd-trace.dir/lib/

lib-injection/README.md

Lines changed: 21 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ the necessary files to a given directory.
1414

1515
The `dl_wheels.py` script provides a portable set of `ddtrace` wheels. It is
1616
responsible for downloading the published wheels of `ddtrace` and its
17-
dependencies.
17+
dependencies. It installs the wheels to a set of per Python, per platform site-packages
18+
directories. These directories can be directly added to the `PYTHONPATH` to use the
19+
`ddtrace` package.
1820

1921
The Datadog Admission Controller injects the InitContainer with a new volume
2022
mount to the application deployment. The script to copy files out of the
@@ -24,12 +26,26 @@ volume mount.
2426

2527
The files copied to the volume are:
2628

27-
- `sitecustomize.py`: Python module that gets run automatically on interpreter startup when it is detected in the `PYTHONPATH`. When executed, it updates the Python path further to include the `ddtrace_pkgs/` directory and then calls `import ddtrace.bootstrap.sitecustomize` which performs automatic instrumentation.
28-
- `ddtrace_pkgs/`: Directory containing the `ddtrace` package and its dependencies for each Python version, platform and architecture.
29+
- `sitecustomize.py`: Python module that gets run automatically on interpreter startup when it is detected in the `PYTHONPATH`. When executed, it updates the Python path further to include the compatible `site-packages` directory and then calls `import ddtrace.bootstrap.sitecustomize` which performs automatic instrumentation.
30+
- `ddtrace_pkgs/`: Directory containing the per-Python, per-runtime `site-packages` directories which contain `ddtrace` package and its dependencies.
2931

3032

3133
The `PYTHONPATH` environment variable is set to the shared volume directory
3234
which contains `sitecustomize.py` and `ddtrace_pkgs`. The environment variable
33-
is injected into the the application container. This enables the
35+
is injected into the application container. This enables the
3436
`sitecustomize.py` file to execute on any Python interpreter startup which
35-
results in the automatic instrument being applied to the application.
37+
results in the automatic instrumentation being applied to the application.
38+
39+
40+
## Testing
41+
42+
To test this feature locally use the provided `docker-compose.yml`.
43+
44+
```bash
45+
export DDTRACE_PYTHON_VERSION=v1.16.1
46+
export APP_CONTEXT=$REPO_ROOT/tests/lib-injection/dd-lib-python-init-test-django
47+
export TEMP_DIR="/tmp/ddtrace"
48+
rm -rf $TEMP_DIR && docker-compose up --build lib_inject && docker-compose up --build
49+
```
50+
51+
Note that the `lib_inject` step is separate to ensure the files are copied to the volume before the app starts up.

lib-injection/dl_wheels.py

Lines changed: 57 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -2,25 +2,23 @@
22
"""
33
Script to download all required wheels (including dependencies) of the ddtrace
44
Python package for relevant Python versions (+ abis), C library platforms and
5-
architectures and merge them into a "megawheel" directory.
5+
architectures and unpack them into Python-specific site-packages directories.
66
7-
This directory provides a portable installation of ddtrace which can be used
8-
on multiple platforms and architectures.
7+
These site-package directories provide a portable installation of ddtrace which can be
8+
used on multiple platforms and architectures.
99
10-
Currently the only OS supported is Linux.
10+
Currently, the only OS supported is Linux.
1111
12-
This script has been tested with 21.0.0 and is confirmed to not work with
12+
This script has been tested with pip 21.0.0 and is confirmed to not work with
1313
20.0.2.
1414
1515
Usage:
1616
./dl_wheels.py --help
1717
18-
19-
The downloaded wheels can then be installed locally using:
20-
pip install --no-index --find-links <dir_of_downloaded_wheels> ddtrace
2118
"""
2219
import argparse
2320
import itertools
21+
import os
2422
import subprocess
2523
import sys
2624

@@ -73,34 +71,54 @@
7371
dl_dir = args.output_dir
7472
print("saving wheels to %s" % dl_dir)
7573

76-
for python_version, arch, platform in itertools.product(args.python_version, args.arch, args.platform):
77-
print("Downloading %s %s %s wheel" % (python_version, arch, platform))
78-
abi = "cp%s" % python_version.replace(".", "")
79-
# Have to special-case these versions of Python for some reason.
80-
if python_version in ["2.7", "3.5", "3.6", "3.7"]:
81-
abi += "m"
82-
83-
# See the docs for an explanation of all the options used:
84-
# https://pip.pypa.io/en/stable/cli/pip_download/
85-
# only-binary=:all: is specified to ensure we get all the dependencies of ddtrace as well.
86-
cmd = [
87-
sys.executable,
88-
"-m",
89-
"pip",
90-
"download",
91-
"ddtrace==%s" % args.ddtrace_version,
92-
"--platform",
93-
"%s_%s" % (platform, arch),
94-
"--python-version",
95-
python_version,
96-
"--abi",
97-
abi,
98-
"--only-binary=:all:",
99-
"--dest",
100-
dl_dir,
101-
]
102-
if args.verbose:
103-
print(" ".join(cmd))
104-
105-
if not args.dry_run:
106-
subprocess.run(cmd, capture_output=not args.verbose, check=True)
74+
75+
for python_version, platform in itertools.product(args.python_version, args.platform):
76+
for arch in args.arch:
77+
print("Downloading %s %s %s wheel" % (python_version, arch, platform))
78+
abi = "cp%s" % python_version.replace(".", "")
79+
# Have to special-case these versions of Python for some reason.
80+
if python_version in ["2.7", "3.5", "3.6", "3.7"]:
81+
abi += "m"
82+
83+
# See the docs for an explanation of all the options used:
84+
# https://pip.pypa.io/en/stable/cli/pip_download/
85+
# only-binary=:all: is specified to ensure we get all the dependencies of ddtrace as well.
86+
cmd = [
87+
sys.executable,
88+
"-m",
89+
"pip",
90+
"download",
91+
"ddtrace==%s" % args.ddtrace_version,
92+
"--platform",
93+
"%s_%s" % (platform, arch),
94+
"--python-version",
95+
python_version,
96+
"--abi",
97+
abi,
98+
"--only-binary=:all:",
99+
"--dest",
100+
dl_dir,
101+
]
102+
if args.verbose:
103+
print(" ".join(cmd))
104+
105+
if not args.dry_run:
106+
subprocess.run(cmd, capture_output=not args.verbose, check=True)
107+
108+
wheel_files = [f for f in os.listdir(dl_dir) if f.endswith(".whl")]
109+
for whl in wheel_files:
110+
wheel_file = os.path.join(dl_dir, whl)
111+
print("Unpacking %s" % wheel_file)
112+
# -q for quieter output, else we get all the files being unzipped.
113+
subprocess.run(
114+
[
115+
"unzip",
116+
"-q",
117+
"-o",
118+
wheel_file,
119+
"-d",
120+
os.path.join(dl_dir, "site-packages-ddtrace-py%s-%s" % (python_version, platform)),
121+
]
122+
)
123+
# Remove the wheel as it has been unpacked
124+
os.remove(wheel_file)

lib-injection/docker-compose.yml

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
# Emulate what the Cluster Agent does as closely as possible.
2+
# The Cluster Agent creates an InitContainer with the init image and runs the copy-lib.sh script.
3+
# It then patches the pods to include the PYTHONPATH environment variable and the volume mount.
4+
version: "3"
5+
6+
services:
7+
lib_inject:
8+
build:
9+
context: .
10+
dockerfile: Dockerfile
11+
args:
12+
- DDTRACE_PYTHON_VERSION=${DDTRACE_PYTHON_VERSION}
13+
command: sh copy-lib.sh /datadog-lib
14+
volumes:
15+
# A host mount is used rather than named volumes as they run into permission issues when copying files.
16+
# The injection image is run with a non-root user which does not have permission to write to the named volume.
17+
- ${TEMP_DIR:-/tmp/ddtrace_test}:/datadog-lib
18+
19+
# testagent is used to collect data from the library to validate.
20+
testagent:
21+
image: ghcr.io/datadog/dd-apm-test-agent/ddapm-test-agent:v1.11.0
22+
ports:
23+
- "8126:8126"
24+
25+
# app is parametrized to generically run images with the library injected and submit data to the test agent.
26+
app:
27+
depends_on:
28+
- lib_inject
29+
image: ${APP_IMAGE:-python:3.10}
30+
environment:
31+
- PYTHONPATH=/datadog-lib
32+
- DD_TRACE_AGENT_URL=http://testagent:8126
33+
volumes:
34+
- ${TEMP_DIR:-/tmp/ddtrace_test}:/datadog-lib
35+
36+
# same as app but a local docker file can be used.
37+
app_local:
38+
depends_on:
39+
- lib_inject
40+
build:
41+
context: ${APP_CONTEXT}
42+
ports:
43+
- "0.0.0.0:18080:18080"
44+
environment:
45+
- PYTHONPATH=/datadog-lib
46+
- DD_TRACE_AGENT_URL=http://testagent:8126
47+
- DD_TRACE_DEBUG=1
48+
- DD_CALL_BASIC_CONFIG=1
49+
volumes:
50+
- ${TEMP_DIR:-/tmp/ddtrace_test}:/datadog-lib

0 commit comments

Comments
 (0)