Merge branch 'juerg/buildbox-run-userchroot' into 'master'

Add CI job to test buildbox-run-userchroot

Closes #1237

See merge request BuildStream/buildstream!1751

11 files changed, 108 insertions, 17 deletions

diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
index d4a30008c..1fe25810a 100644
--- a/.gitlab-ci.yml
+++ b/.gitlab-ci.yml
@@ -24,7 +24,7 @@ variables:
   # Our own variables
   # Version of the docker images we should use for all the images.
   # This is taken from buildstream/buildstream-docker-images
-  DOCKER_IMAGE_VERSION: master-101787517
+  DOCKER_IMAGE_VERSION: master-103717922
   PYTEST_ADDOPTS: "--color=yes"
   INTEGRATION_CACHE: "${CI_PROJECT_DIR}/cache/integration-cache"
   PYTEST_ARGS: "--color=yes --integration -n 2"
@@ -124,6 +124,40 @@ tests-buildbox-run:
   variables:
     BST_FORCE_SANDBOX: "buildbox-run"
 
+tests-userchroot:
+  image: registry.gitlab.com/buildstream/buildstream-docker-images/testsuite-fedora:31-${DOCKER_IMAGE_VERSION}
+  <<: *tests
+  variables:
+    BST_FORCE_SANDBOX: "buildbox-run"
+    BST_CAS_STAGING_ROOT: "/builds/userchroot"
+
+  script:
+    - mkdir -p "${INTEGRATION_CACHE}"
+    - useradd -Um buildstream
+
+    # Use buildbox-run-userchroot and hardlinking
+    - ln -svf buildbox-run-userchroot /usr/local/bin/buildbox-run
+    - rm -vf /usr/local/bin/buildbox-fuse
+
+    # When using userchroot, buildbox-casd must run as a separate user
+    - useradd -g buildstream buildbox-casd
+    - chown buildbox-casd:buildstream /usr/local/bin/buildbox-casd
+    - chmod u+s /usr/local/bin/buildbox-casd
+
+    # Set up staging root with permissions required by userchroot,
+    # must be on same filesystem as current directory to support hardlinks
+    - mkdir -p "${BST_CAS_STAGING_ROOT}"
+    - chown -R buildbox-casd:buildstream "${BST_CAS_STAGING_ROOT}"
+    # userchroot doesn't allow group/world-writable base directory
+    - chmod go-w /builds
+    - echo buildbox-casd:${BST_CAS_STAGING_ROOT} > /etc/userchroot.conf
+
+    - chown -R buildstream:buildstream .
+
+    # Run the tests as a simple user to test for permission issues
+    - su buildstream -c "umask 002 && ${TEST_COMMAND}"
+    - su buildstream -c "umask 002 && ${EXTERNAL_TESTS_COMMAND}"
+
 tests-fedora-missing-deps:
   # Ensure that tests behave nicely while missing bwrap and ostree
   image: registry.gitlab.com/buildstream/buildstream-docker-images/testsuite-fedora:31-${DOCKER_IMAGE_VERSION}
@@ -405,6 +439,7 @@ coverage:
     - tox -e coverage
     - cp -a .coverage-reports/ ./coverage-report
   dependencies:
+  - tests-buildbox-run
   - tests-centos-7.6
   - tests-debian-9
   - tests-debian-10
@@ -415,6 +450,7 @@ coverage:
   - tests-remote-execution
   - tests-ubuntu-18.04
   - tests-unix
+  - tests-userchroot
   except:
   - schedules
   artifacts:
diff --git a/src/buildstream/testing/_sourcetests/source_determinism.py b/src/buildstream/testing/_sourcetests/source_determinism.py
index d51d0e520..ed00c71ea 100644
--- a/src/buildstream/testing/_sourcetests/source_determinism.py
+++ b/src/buildstream/testing/_sourcetests/source_determinism.py
@@ -50,6 +50,10 @@ def create_test_directory(*path, mode=0o644):
 @pytest.mark.integration
 @pytest.mark.datafiles(DATA_DIR)
 @pytest.mark.skipif(not HAVE_SANDBOX, reason="Only available with a functioning sandbox")
+@pytest.mark.skipif(
+    HAVE_SANDBOX == "buildbox-run" and CASD_SEPARATE_USER,
+    reason="Flaky due to timestamps: https://gitlab.com/BuildStream/buildstream/issues/1218",
+)
 def test_deterministic_source_umask(cli, tmpdir, datafiles, kind):
     project = str(datafiles)
     element_name = "list.bst"
diff --git a/src/buildstream/testing/_utils/site.py b/src/buildstream/testing/_utils/site.py
index f3b45dbf6..9fbddf13e 100644
--- a/src/buildstream/testing/_utils/site.py
+++ b/src/buildstream/testing/_utils/site.py
@@ -82,3 +82,12 @@ if HAVE_SANDBOX is not None:
     pass
 elif IS_LINUX and HAVE_BWRAP and (not IS_WSL):
     HAVE_SANDBOX = "bwrap"
+
+
+BUILDBOX_RUN = None
+if HAVE_SANDBOX == "buildbox-run":
+    try:
+        path = utils.get_host_tool("buildbox-run")
+        BUILDBOX_RUN = os.path.basename(os.readlink(path))
+    except (ProgramNotFoundError, OSError):
+        pass
diff --git a/src/buildstream/testing/runcli.py b/src/buildstream/testing/runcli.py
index c0e278b11..1e868609a 100644
--- a/src/buildstream/testing/runcli.py
+++ b/src/buildstream/testing/runcli.py
@@ -842,6 +842,13 @@ def configured(directory, config=None):
     if not config.get("logdir", False):
         config["logdir"] = os.path.join(directory, "logs")
 
+    cas_stage_root = os.environ.get("BST_CAS_STAGING_ROOT")
+    if cas_stage_root:
+        symlink_path = os.path.join(config["cachedir"], "cas", "staging")
+        if not os.path.lexists(symlink_path):
+            os.makedirs(os.path.join(config["cachedir"], "cas"), exist_ok=True)
+            os.symlink(cas_stage_root, symlink_path)
+
     # Dump it and yield the filename for test scripts to feed it
     # to buildstream as an artument
     filename = os.path.join(directory, "buildstream.conf")
diff --git a/src/buildstream/utils.py b/src/buildstream/utils.py
index b6716a29d..545816e89 100644
--- a/src/buildstream/utils.py
+++ b/src/buildstream/utils.py
@@ -819,20 +819,25 @@ def _remove_path_with_parents(basedir: Union[Path, str], path: Union[Path, str])
 
 # Recursively remove directories, ignoring file permissions as much as
 # possible.
-def _force_rmtree(rootpath, **kwargs):
+def _force_rmtree(rootpath):
+    def fix_permissions(function, path, info):
+        parent = os.path.dirname(path)
 
-    os.chmod(rootpath, 0o755)
-    for root, dirs, _ in os.walk(rootpath):
-        for d in dirs:
-            path = os.path.join(root, d.lstrip("/"))
-            if os.path.exists(path) and not os.path.islink(path):
-                try:
-                    os.chmod(path, 0o755)
-                except OSError as e:
-                    raise UtilError("Failed to ensure write permission on file '{}': {}".format(path, e))
+        try:
+            os.chmod(parent, 0o755)
+        except OSError as e:
+            raise UtilError("Failed to ensure write permission on directory '{}': {}".format(parent, e))
+
+        # Directories need to be removed with `rmdir`, though
+        # `os.path.isdir` will follow symlinks, so make sure it's
+        # not a symlink first
+        if not os.path.islink(path) and os.path.isdir(path):
+            os.rmdir(path)
+        else:
+            os.remove(path)
 
     try:
-        shutil.rmtree(rootpath, **kwargs)
+        shutil.rmtree(rootpath, onerror=fix_permissions)
     except OSError as e:
         raise UtilError("Failed to remove cache directory '{}': {}".format(rootpath, e))
 
diff --git a/tests/integration/compose.py b/tests/integration/compose.py
index 2f38aa66c..f08f2e808 100644
--- a/tests/integration/compose.py
+++ b/tests/integration/compose.py
@@ -8,7 +8,7 @@ from buildstream import _yaml
 
 from buildstream.testing import cli_integration as cli  # pylint: disable=unused-import
 from buildstream.testing.integration import walk_dir
-from buildstream.testing._utils.site import HAVE_SANDBOX
+from buildstream.testing._utils.site import HAVE_SANDBOX, BUILDBOX_RUN
 
 
 pytestmark = pytest.mark.integration
@@ -133,6 +133,10 @@ def test_compose_include(cli, datafiles, include_domains, exclude_domains, expec
 
 @pytest.mark.datafiles(DATA_DIR)
 @pytest.mark.skipif(not HAVE_SANDBOX, reason="Only available with a functioning sandbox")
+@pytest.mark.xfail(
+    HAVE_SANDBOX == "buildbox-run" and BUILDBOX_RUN == "buildbox-run-userchroot",
+    reason="Root directory not writable with userchroot",
+)
 def test_compose_run_integration(cli, datafiles):
     project = str(datafiles)
     checkout = os.path.join(cli.directory, "checkout")
diff --git a/tests/integration/filter.py b/tests/integration/filter.py
index 2fca8957c..12061fe7a 100644
--- a/tests/integration/filter.py
+++ b/tests/integration/filter.py
@@ -7,7 +7,7 @@ import pytest
 
 from buildstream.testing import cli  # pylint: disable=unused-import
 from buildstream.testing.integration import assert_contains
-from buildstream.testing._utils.site import HAVE_SANDBOX
+from buildstream.testing._utils.site import HAVE_SANDBOX, BUILDBOX_RUN
 
 
 pytestmark = pytest.mark.integration
@@ -18,6 +18,10 @@ DATA_DIR = os.path.join(os.path.dirname(os.path.realpath(__file__)), "project")
 
 @pytest.mark.datafiles(os.path.join(DATA_DIR))
 @pytest.mark.skipif(not HAVE_SANDBOX, reason="Only available with a functioning sandbox")
+@pytest.mark.xfail(
+    HAVE_SANDBOX == "buildbox-run" and BUILDBOX_RUN == "buildbox-run-userchroot",
+    reason="Root directory not writable with userchroot",
+)
 def test_filter_pass_integration(datafiles, cli):
     project = str(datafiles)
 
diff --git a/tests/integration/script.py b/tests/integration/script.py
index 67dd310bd..a03824ecd 100644
--- a/tests/integration/script.py
+++ b/tests/integration/script.py
@@ -6,7 +6,7 @@ import pytest
 
 from buildstream import _yaml
 from buildstream.testing import cli_integration as cli  # pylint: disable=unused-import
-from buildstream.testing._utils.site import HAVE_SANDBOX
+from buildstream.testing._utils.site import HAVE_SANDBOX, BUILDBOX_RUN
 
 
 pytestmark = pytest.mark.integration
@@ -60,6 +60,10 @@ def test_script(cli, datafiles):
 
 @pytest.mark.datafiles(DATA_DIR)
 @pytest.mark.skipif(not HAVE_SANDBOX, reason="Only available with a functioning sandbox")
+@pytest.mark.xfail(
+    HAVE_SANDBOX == "buildbox-run" and BUILDBOX_RUN == "buildbox-run-userchroot",
+    reason="Root directory not writable with userchroot",
+)
 def test_script_root(cli, datafiles):
     project = str(datafiles)
     checkout = os.path.join(cli.directory, "checkout")
@@ -92,7 +96,8 @@ def test_script_root(cli, datafiles):
 @pytest.mark.datafiles(DATA_DIR)
 @pytest.mark.skipif(not HAVE_SANDBOX, reason="Only available with a functioning sandbox")
 @pytest.mark.xfail(
-    HAVE_SANDBOX == "buildbox-run", reason="Read-only root directory not supported by buildbox-run",
+    HAVE_SANDBOX == "buildbox-run" and BUILDBOX_RUN != "buildbox-run-userchroot",
+    reason="Read-only root directory not supported by buildbox-run",
 )
 def test_script_no_root(cli, datafiles):
     project = str(datafiles)
@@ -166,6 +171,10 @@ def test_script_layout(cli, datafiles):
 
 @pytest.mark.datafiles(DATA_DIR)
 @pytest.mark.skipif(not HAVE_SANDBOX, reason="Only available with a functioning sandbox")
+@pytest.mark.xfail(
+    HAVE_SANDBOX == "buildbox-run" and BUILDBOX_RUN == "buildbox-run-userchroot",
+    reason="Root directory not writable with userchroot",
+)
 def test_regression_cache_corruption(cli, datafiles):
     project = str(datafiles)
     checkout_original = os.path.join(cli.directory, "checkout-original")
@@ -206,6 +215,10 @@ def test_regression_tmpdir(cli, datafiles):
 
 @pytest.mark.datafiles(DATA_DIR)
 @pytest.mark.skipif(not HAVE_SANDBOX, reason="Only available with a functioning sandbox")
+@pytest.mark.xfail(
+    HAVE_SANDBOX == "buildbox-run" and BUILDBOX_RUN == "buildbox-run-userchroot",
+    reason="Root directory not writable with userchroot",
+)
 def test_regression_cache_corruption_2(cli, datafiles):
     project = str(datafiles)
     checkout_original = os.path.join(cli.directory, "checkout-original")
diff --git a/tests/integration/shell.py b/tests/integration/shell.py
index cea608016..040ae53a5 100644
--- a/tests/integration/shell.py
+++ b/tests/integration/shell.py
@@ -6,7 +6,7 @@ import pytest
 
 from buildstream import _yaml
 from buildstream.testing import cli_integration as cli  # pylint: disable=unused-import
-from buildstream.testing._utils.site import HAVE_SANDBOX
+from buildstream.testing._utils.site import HAVE_SANDBOX, BUILDBOX_RUN
 from buildstream._exceptions import ErrorDomain
 from buildstream import utils
 
@@ -134,6 +134,10 @@ def test_env_assign_isolated(cli, datafiles, animal):
 # /bin/sh)
 @pytest.mark.datafiles(DATA_DIR)
 @pytest.mark.skipif(not HAVE_SANDBOX, reason="Only available with a functioning sandbox")
+@pytest.mark.xfail(
+    HAVE_SANDBOX == "buildbox-run" and BUILDBOX_RUN == "buildbox-run-userchroot",
+    reason="buildbox-run-userchroot requires a shell",
+)
 def test_no_shell(cli, datafiles):
     project = str(datafiles)
     element_path = os.path.join(project, "elements")
diff --git a/tests/integration/source-determinism.py b/tests/integration/source-determinism.py
index 14559759d..355588133 100644
--- a/tests/integration/source-determinism.py
+++ b/tests/integration/source-determinism.py
@@ -29,6 +29,10 @@ def create_test_directory(*path, mode=0o644):
 @pytest.mark.integration
 @pytest.mark.datafiles(DATA_DIR)
 @pytest.mark.skipif(not HAVE_SANDBOX, reason="Only available with a functioning sandbox")
+@pytest.mark.skipif(
+    HAVE_SANDBOX == "buildbox-run" and CASD_SEPARATE_USER,
+    reason="Flaky due to timestamps: https://gitlab.com/BuildStream/buildstream/issues/1218",
+)
 def test_deterministic_source_local(cli, tmpdir, datafiles):
     """Only user rights should be considered for local source.
     """
diff --git a/tox.ini b/tox.ini
index b037e3984..62737b0ca 100644
--- a/tox.ini
+++ b/tox.ini
@@ -39,6 +39,7 @@ deps =
     randomized: pytest-random-order
 passenv =
     ARTIFACT_CACHE_SERVICE
+    BST_CAS_STAGING_ROOT
     BST_FORCE_BACKEND
     BST_FORCE_SANDBOX
     BST_FORCE_START_METHOD