Skip to content

Commit 33f95dd

Browse files
authoredDec 11, 2024
RHOAIENG-16517: chore(tests): add sandboxing so that Dockerfile builds can only access files we know they access (opendatahub-io#803)
1 parent 059a835 commit 33f95dd

File tree

7 files changed

+173
-20
lines changed

7 files changed

+173
-20
lines changed
 

‎.github/workflows/build-notebooks-TEMPLATE.yaml

+7
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,13 @@ jobs:
3939

4040
- uses: actions/checkout@v4
4141

42+
- run: mkdir -p $TMPDIR
43+
44+
# for bin/buildinputs in scripts/sandbox.py
45+
- uses: actions/setup-go@v5
46+
with:
47+
cache-dependency-path: "**/*.sum"
48+
4249
- name: Login to GitHub Container Registry
4350
uses: docker/login-action@v3
4451
with:

‎Makefile

+2-1
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,8 @@ define build_image
6969
$(eval BUILD_ARGS := --build-arg BASE_IMAGE=$(BASE_IMAGE_NAME)),
7070
$(eval BUILD_ARGS :=)
7171
)
72-
$(CONTAINER_ENGINE) build $(CONTAINER_BUILD_CACHE_ARGS) --tag $(IMAGE_NAME) --file $(2)/Dockerfile $(BUILD_ARGS) $(ROOT_DIR)/
72+
$(ROOT_DIR)/scripts/sandbox.py --dockerfile '$(2)/Dockerfile' -- \
73+
$(CONTAINER_ENGINE) build $(CONTAINER_BUILD_CACHE_ARGS) --tag $(IMAGE_NAME) --file '$(2)/Dockerfile' $(BUILD_ARGS) {}\;
7374
endef
7475

7576
# Push function for the notebok image:

‎ci/cached-builds/gha_lvm_overlay.sh

+2-1
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@ set -Eeuo pipefail
77
# This script creates file-backed volumes on /dev/root and /dev/sda1, then creates ext4 over both, and mounts it for our use
88
# https://github.com/easimon/maximize-build-space/blob/master/action.yml
99

10-
root_reserve_mb=2048
10+
# root_reserve_mb=2048 was running out of disk space building cuda images
11+
root_reserve_mb=4096
1112
temp_reserve_mb=100
1213
swap_size_mb=4096
1314

‎poetry.lock

+29-18
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎pyproject.toml

+1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ python = "~3.12"
1313
[tool.poetry.group.dev.dependencies]
1414
pytest = "^8.2.2"
1515
pytest-subtests = "^0.12.1"
16+
pyfakefs = "^5.7.2"
1617

1718
[build-system]
1819
requires = ["poetry-core"]

‎scripts/sandbox.py

+77
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
#! /usr/bin/env python3
2+
3+
import argparse
4+
import logging
5+
import pathlib
6+
import shutil
7+
import subprocess
8+
import sys
9+
import tempfile
10+
import json
11+
from typing import cast
12+
13+
ROOT_DIR = pathlib.Path(__file__).parent.parent
14+
MAKE = shutil.which("gmake") or shutil.which("make")
15+
16+
logging.basicConfig()
17+
logging.root.name = pathlib.Path(__file__).name
18+
19+
20+
class Args(argparse.Namespace):
21+
dockerfile: pathlib.Path
22+
remaining: list[str]
23+
24+
25+
def main() -> int:
26+
p = argparse.ArgumentParser(allow_abbrev=False)
27+
p.add_argument("--dockerfile", type=pathlib.Path, required=True)
28+
p.add_argument('remaining', nargs=argparse.REMAINDER)
29+
30+
args = cast(Args, p.parse_args())
31+
32+
print(f"{__file__=} started with {args=}")
33+
34+
if not args.remaining or args.remaining[0] != "--":
35+
print("must specify command to execute after double dashes at the end, such as `-- command --args ...`")
36+
return 1
37+
if not "{};" in args.remaining:
38+
print("must give a `{};` parameter that will be replaced with new build context")
39+
return 1
40+
41+
if not (ROOT_DIR / "bin/buildinputs").exists():
42+
subprocess.check_call([MAKE, "bin/buildinputs"], cwd=ROOT_DIR)
43+
stdout = subprocess.check_output([ROOT_DIR / "bin/buildinputs", str(args.dockerfile)],
44+
text=True, cwd=ROOT_DIR)
45+
prereqs = [pathlib.Path(file) for file in json.loads(stdout)] if stdout != "\n" else []
46+
print(f"{prereqs=}")
47+
48+
with tempfile.TemporaryDirectory(delete=True) as tmpdir:
49+
setup_sandbox(prereqs, pathlib.Path(tmpdir))
50+
command = [arg if arg != "{};" else tmpdir for arg in args.remaining[1:]]
51+
print(f"running {command=}")
52+
try:
53+
subprocess.check_call(command)
54+
except subprocess.CalledProcessError as err:
55+
logging.error("Failed to execute process, see errors logged above ^^^")
56+
return err.returncode
57+
return 0
58+
59+
60+
def setup_sandbox(prereqs: list[pathlib.Path], tmpdir: pathlib.Path):
61+
# always adding .gitignore
62+
gitignore = ROOT_DIR / ".gitignore"
63+
if gitignore.exists():
64+
shutil.copy(gitignore, tmpdir)
65+
66+
for dep in prereqs:
67+
if dep.is_absolute():
68+
dep = dep.relative_to(ROOT_DIR)
69+
if dep.is_dir():
70+
shutil.copytree(dep, tmpdir / dep, symlinks=False, dirs_exist_ok=True)
71+
else:
72+
(tmpdir / dep.parent).mkdir(parents=True, exist_ok=True)
73+
shutil.copy(dep, tmpdir / dep.parent)
74+
75+
76+
if __name__ == '__main__':
77+
sys.exit(main())

‎scripts/sandbox_tests.py

+55
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
#! /usr/bin/env python3
2+
3+
import glob
4+
import logging
5+
import pathlib
6+
import sys
7+
import tempfile
8+
9+
import pyfakefs.fake_filesystem
10+
11+
from scripts.sandbox import setup_sandbox
12+
13+
ROOT_DIR = pathlib.Path(__file__).parent.parent
14+
15+
logging.basicConfig()
16+
logging.root.name = pathlib.Path(__file__).name
17+
18+
class TestSandbox:
19+
def test_filesystem_file(self, fs: pyfakefs.fake_filesystem.FakeFilesystem):
20+
pathlib.Path("a").write_text("a")
21+
22+
with tempfile.TemporaryDirectory(delete=True) as tmpdir:
23+
setup_sandbox([pathlib.Path("a")], pathlib.Path(tmpdir))
24+
assert (pathlib.Path(tmpdir) / "a").is_file()
25+
26+
def test_filesystem_dir_with_file(self, fs: pyfakefs.fake_filesystem.FakeFilesystem):
27+
pathlib.Path("a/").mkdir()
28+
pathlib.Path("a/b").write_text("b")
29+
30+
with tempfile.TemporaryDirectory(delete=True) as tmpdir:
31+
setup_sandbox([pathlib.Path("a")], pathlib.Path(tmpdir))
32+
assert (pathlib.Path(tmpdir) / "a").is_dir()
33+
assert (pathlib.Path(tmpdir) / "a" / "b").is_file()
34+
35+
def test_filesystem_dir_with_dir_with_file(self, fs: pyfakefs.fake_filesystem.FakeFilesystem):
36+
pathlib.Path("a/b").mkdir(parents=True)
37+
pathlib.Path("a/b/c").write_text("c")
38+
39+
with tempfile.TemporaryDirectory(delete=True) as tmpdir:
40+
setup_sandbox([pathlib.Path("a")], pathlib.Path(tmpdir))
41+
assert (pathlib.Path(tmpdir) / "a").is_dir()
42+
assert (pathlib.Path(tmpdir) / "a" / "b").is_dir()
43+
assert (pathlib.Path(tmpdir) / "a" / "b" / "c").is_file()
44+
45+
def test_filesystem_file_in_dir_in_dir(self, fs: pyfakefs.fake_filesystem.FakeFilesystem):
46+
pathlib.Path("a/b").mkdir(parents=True)
47+
pathlib.Path("a/b/c").write_text("c")
48+
49+
with tempfile.TemporaryDirectory(delete=True) as tmpdir:
50+
setup_sandbox([pathlib.Path("a/b/c")], pathlib.Path(tmpdir))
51+
for g in glob.glob("**/*", recursive=True):
52+
logging.debug("%s", g)
53+
assert (pathlib.Path(tmpdir) / "a").is_dir()
54+
assert (pathlib.Path(tmpdir) / "a" / "b").is_dir()
55+
assert (pathlib.Path(tmpdir) / "a" / "b" / "c").is_file()

0 commit comments

Comments
 (0)
Please sign in to comment.