Merge ~pelpsi/lpci:jobs-cannot-run-as-root into lpci:main

[Simone Pelosi]

Add a root flag

New root flag to choose whether to run commands as root or as default user (_lpci).
Lpci runs jobs as root inside the container.
However, there are some reasonable jobs that call code that refuses to run as root.

LP: #1982954

[Colin Watson (cjwatson)]

Thanks! I have a collection of nitpicks, but they're mostly just that.

review: Approve

[Simone Pelosi (pelpsi)]

> Thanks! I have a collection of nitpicks, but they're mostly just that.

Thank you Colin! I make the necessary fixes and then I merge it.

diff --git a/NEWS.rst b/NEWS.rst
index ba2012b..45f0dde 100644
--- a/NEWS.rst
+++ b/NEWS.rst
@@ -9,6 +9,8 @@ Version history
 - Fix ``policy-rc.d`` issue preventing services
   from running into the container.
 
+- Add a ``root`` flag.
+
 0.1.0 (2023-04-18)
 ==================
 
diff --git a/docs/configuration.rst b/docs/configuration.rst
index 65b0b5d..3a71bcd 100644
--- a/docs/configuration.rst
+++ b/docs/configuration.rst
@@ -72,6 +72,12 @@ Job definitions
 ``plugin`` (optional)
     A plugin which will be used for this job. See :doc:`../plugins`
 
+``root`` (optional)
+    If ``true``, run shell commands declared by ``run-before``,
+    ``run``, and ``run-after`` as root;
+    if ``false``, run them as a non-root user (``_lpci``).
+    Default value: ``false``.
+
 ``run-before`` (optional)
     A string (possibly multi-line) containing shell commands to run for this
     job prior to the main ``run`` section.
diff --git a/lpci/commands/run.py b/lpci/commands/run.py
index d6c65b2..c543ed1 100644
--- a/lpci/commands/run.py
+++ b/lpci/commands/run.py
@@ -433,8 +433,12 @@ def _run_instance_command(
     host_architecture: str,
     remote_cwd: Path,
     environment: Optional[Dict[str, Optional[str]]],
+    root: bool = True,
 ) -> None:
     full_run_cmd = ["bash", "--noprofile", "--norc", "-ec", command]
+    if not root:
+        full_run_cmd[:0] = ["runuser", "-u", env.get_non_root_user(), "--"]
+
     emit.progress("Running command for the job...")
     original_mode = emit.get_mode()
     if original_mode == EmitterMode.BRIEF:
@@ -475,6 +479,11 @@ def _run_job(
     # XXX jugmac00 2022-04-27: we should create a configuration object to be
     # passed in and not so many arguments
     job = config.jobs[job_name][job_index]
+    root = job.root
+
+    # workaround necessary to please coverage
+    assert isinstance(root, bool)
+
     host_architecture = get_host_architecture()
     if host_architecture not in job.architectures:
         return
@@ -540,6 +549,7 @@ def _run_job(
         series=job.series,
         architecture=host_architecture,
         gpu_nvidia=gpu_nvidia,
+        root=root,
     ) as instance:
         snaps = list(itertools.chain(*pm.hook.lpci_install_snaps()))
         for snap in snaps:
@@ -583,6 +593,7 @@ def _run_job(
                     host_architecture=host_architecture,
                     remote_cwd=remote_cwd,
                     environment=environment,
+                    root=root,
                 )
         if config.license:
             if not job.output:
diff --git a/lpci/commands/tests/test_run.py b/lpci/commands/tests/test_run.py
index 07ae61b..c47f1e7 100644
--- a/lpci/commands/tests/test_run.py
+++ b/lpci/commands/tests/test_run.py
@@ -3537,6 +3537,55 @@ class TestRun(RunBaseTestCase):
             remote="test-remote",
         )
 
+    @patch("lpci.commands.run.get_provider")
+    @patch("lpci.commands.run.get_host_architecture", return_value="amd64")
+    def test_root_field(self, mock_get_host_architecture, mock_get_provider):
+        launcher = Mock(spec=launch)
+        provider = makeLXDProvider(lxd_launcher=launcher)
+        mock_get_provider.return_value = provider
+        execute_run = launcher.return_value.execute_run
+        execute_run.return_value = subprocess.CompletedProcess("_lpci", 0)
+        config = dedent(
+            """
+            pipeline:
+                - build
+
+            jobs:
+                build:
+                    root: False
+                    series: focal
+                    architectures: amd64
+                    run: whoami
+            """
+        )
+        Path(".launchpad.yaml").write_text(config)
+
+        result = self.run_command("run")
+
+        self.assertEqual(0, result.exit_code)
+        self.assertEqual(
+            [
+                call(
+                    [
+                        "runuser",
+                        "-u",
+                        "_lpci",
+                        "--",
+                        "bash",
+                        "--noprofile",
+                        "--norc",
+                        "-ec",
+                        "whoami",
+                    ],
+                    cwd=Path("/build/lpci/project"),
+                    env={},
+                    stdout=ANY,
+                    stderr=ANY,
+                )
+            ],
+            execute_run.call_args_list,
+        )
+
 
 class TestRunOne(RunBaseTestCase):
     def test_config_file_not_under_project_directory(self):
diff --git a/lpci/config.py b/lpci/config.py
index 8e3f77a..4af17e5 100644
--- a/lpci/config.py
+++ b/lpci/config.py
@@ -290,6 +290,7 @@ class Job(ModelConfigDefaults):
     package_repositories: List[PackageRepository] = []
     plugin: Optional[StrictStr]
     plugin_config: Optional[BaseConfig]
+    root: Optional[bool] = True
 
     @pydantic.validator("architectures", pre=True)
     def validate_architectures(
@@ -299,6 +300,16 @@ class Job(ModelConfigDefaults):
             v = [v]
         return v
 
+    @pydantic.validator("root", pre=True)
+    def validate_root(cls, v: Any) -> Any:
+        if type(v) is not bool or v is None:
+            raise ValueError(
+                "You configured `root` parameter, "
+                + "but you did not specify a valid value. "
+                + "Valid values would either be `true` or `false`."
+            )
+        return v
+
     @pydantic.validator("snaps", pre=True)
     def validate_snaps(cls, v: List[Any]) -> Any:
         clean_values = []
diff --git a/lpci/env.py b/lpci/env.py
index cfc7331..0aa9b96 100644
--- a/lpci/env.py
+++ b/lpci/env.py
@@ -6,6 +6,10 @@
 from pathlib import Path
 
 
+def get_non_root_user() -> str:
+    return "_lpci"
+
+
 def get_managed_environment_home_path() -> Path:
     """Path for home when running in managed environment."""
     return Path("/root")
diff --git a/lpci/providers/_base.py b/lpci/providers/_base.py
index 15d14f7..388891a 100644
--- a/lpci/providers/_base.py
+++ b/lpci/providers/_base.py
@@ -109,6 +109,7 @@ class Provider(ABC):
         series: str,
         architecture: str,
         gpu_nvidia: bool = False,
+        root: bool = False,
     ) -> Generator[lxd.LXDInstance, None, None]:
         """Launch environment for specified series and architecture.
 
diff --git a/lpci/providers/_lxd.py b/lpci/providers/_lxd.py
index 1a20c92..c673502 100644
--- a/lpci/providers/_lxd.py
+++ b/lpci/providers/_lxd.py
@@ -20,6 +20,7 @@ from pydantic import StrictStr
 from lpci.env import (
     get_managed_environment_home_path,
     get_managed_environment_project_path,
+    get_non_root_user,
 )
 from lpci.errors import CommandError
 from lpci.providers._base import Provider, sanitize_lxd_instance_name
@@ -238,6 +239,34 @@ class LXDProvider(Provider):
             **kwargs,
         )
 
+    def _set_up_non_root_user(
+        self,
+        instance: lxd.LXDInstance,
+        instance_name: str,
+    ) -> None:
+
+        default_user = get_non_root_user()
+        create_cmd = "getent passwd " + default_user + " >/dev/null"
+        create_cmd += " || useradd -m -U " + default_user
+
+        # Create default user if doesn't exist.
+        self._internal_execute_run(
+            instance, instance_name, ["sh", "-c", create_cmd], check=True
+        )
+
+        # Give ownership of the project directory to _lpci
+        self._internal_execute_run(
+            instance,
+            instance_name,
+            [
+                "chown",
+                "-R",
+                f"{default_user}:{default_user}",
+                str(get_managed_environment_project_path()),
+            ],
+            check=True,
+        )
+
     @contextmanager
     def launched_environment(
         self,
@@ -247,6 +276,7 @@ class LXDProvider(Provider):
         series: str,
         architecture: str,
         gpu_nvidia: bool = False,
+        root: bool = True,
     ) -> Generator[lxd.LXDInstance, None, None]:
         """Launch environment for specified series and architecture.
 
@@ -354,6 +384,10 @@ class LXDProvider(Provider):
                     ["rm", "-f", "/usr/local/sbin/policy-rc.d"],
                     check=True,
                 )
+                if not root:
+                    self._set_up_non_root_user(
+                        instance=instance, instance_name=instance_name
+                    )
             except subprocess.CalledProcessError as error:
                 raise CommandError(str(error)) from error
             finally:
diff --git a/lpci/providers/tests/test_lxd.py b/lpci/providers/tests/test_lxd.py
index 0a4f803..0e9b439 100644
--- a/lpci/providers/tests/test_lxd.py
+++ b/lpci/providers/tests/test_lxd.py
@@ -500,10 +500,151 @@ class TestLXDProvider(TestCase):
                     ),
                     call().lxc.exec(
                         instance_name=expected_instance_name,
+                        command=["rm", "-f", "/usr/local/sbin/policy-rc.d"],
+                        project="test-project",
+                        remote="test-remote",
+                        runner=subprocess.run,
+                        check=True,
+                    ),
+                    call().unmount(target=Path("/root/tmp-project")),
+                ],
+                mock_launcher.mock_calls,
+            )
+            mock_launcher.reset_mock()
+
+        self.assertEqual(
+            [
+                call().lxc.exec(
+                    instance_name=expected_instance_name,
+                    command=["rm", "-rf", "/build/lpci/project"],
+                    project="test-project",
+                    remote="test-remote",
+                    runner=subprocess.run,
+                    check=True,
+                ),
+                call().unmount_all(),
+                call().stop(),
+            ],
+            mock_launcher.mock_calls,
+        )
+
+    @patch("os.environ", {"PATH": "not-using-host-path"})
+    def test_launched_environment_default_user(self):
+        expected_instance_name = "lpci-my-project-12345-focal-amd64"
+        mock_lxc = Mock(spec=LXC)
+        mock_lxc.profile_show.return_value = {
+            "config": {"sentinel": "true"},
+            "devices": {"eth0": {}},
+        }
+        mock_lxc.project_list.return_value = []
+        mock_lxc.remote_list.return_value = {}
+        mock_launcher = Mock(spec=launch)
+        provider = makeLXDProvider(lxc=mock_lxc, lxd_launcher=mock_launcher)
+
+        with provider.launched_environment(
+            project_name="my-project",
+            project_path=self.mock_path,
+            series="focal",
+            architecture="amd64",
+            root=False,
+        ) as instance:
+            self.assertIsNotNone(instance)
+            mock_lxc.project_list.assert_called_once_with("test-remote")
+            mock_lxc.project_create.assert_called_once_with(
+                project="test-project", remote="test-remote"
+            )
+            mock_lxc.profile_show.assert_called_once_with(
+                profile="default", project="default", remote="test-remote"
+            )
+            mock_lxc.profile_edit.assert_called_once_with(
+                profile="default",
+                config={
+                    "config": {"sentinel": "true"},
+                    "devices": {"eth0": {}},
+                },
+                project="test-project",
+                remote="test-remote",
+            )
+            self.assertEqual(
+                [
+                    call(
+                        name=expected_instance_name,
+                        base_configuration=LPCIBuilddBaseConfiguration(
+                            alias=BuilddBaseAlias.FOCAL,
+                            environment={"PATH": _base_path},
+                            hostname=expected_instance_name,
+                        ),
+                        image_name="focal",
+                        image_remote="craft-com.ubuntu.cloud-buildd",
+                        auto_clean=True,
+                        auto_create_project=True,
+                        map_user_uid=True,
+                        use_base_instance=True,
+                        project="test-project",
+                        remote="test-remote",
+                        lxc=mock_lxc,
+                    ),
+                    call().mount(
+                        host_source=self.mock_path,
+                        target=Path("/root/tmp-project"),
+                    ),
+                    call().lxc.exec(
+                        instance_name=expected_instance_name,
+                        command=["rm", "-rf", "/build/lpci/project"],
+                        project="test-project",
+                        remote="test-remote",
+                        runner=subprocess.run,
+                        check=True,
+                    ),
+                    call().lxc.exec(
+                        instance_name=expected_instance_name,
+                        command=["mkdir", "-p", "/build/lpci"],
+                        project="test-project",
+                        remote="test-remote",
+                        runner=subprocess.run,
+                        check=True,
+                    ),
+                    call().lxc.exec(
+                        instance_name=expected_instance_name,
                         command=[
-                            "rm",
-                            "-f",
-                            "/usr/local/sbin/policy-rc.d",
+                            "cp",
+                            "-a",
+                            "/root/tmp-project",
+                            "/build/lpci/project",
+                        ],
+                        project="test-project",
+                        remote="test-remote",
+                        runner=subprocess.run,
+                        check=True,
+                    ),
+                    call().lxc.exec(
+                        instance_name=expected_instance_name,
+                        command=["rm", "-f", "/usr/local/sbin/policy-rc.d"],
+                        project="test-project",
+                        remote="test-remote",
+                        runner=subprocess.run,
+                        check=True,
+                    ),
+                    call().lxc.exec(
+                        instance_name=expected_instance_name,
+                        command=[
+                            "sh",
+                            "-c",
+                            "getent passwd _lpci >/dev/null"
+                            + " || useradd -m -U _lpci",
+                        ],
+                        project="test-project",
+                        remote="test-remote",
+                        runner=subprocess.run,
+                        check=True,
+                    ),
+                    call().lxc.exec(
+                        instance_name=expected_instance_name,
+                        command=[
+                            "chown",
+                            "-R",
+                            "_lpci:_lpci",
+                            "/build/lpci/project",
                         ],
                         project="test-project",
                         remote="test-remote",
diff --git a/lpci/tests/test_config.py b/lpci/tests/test_config.py
index 7bf45a5..17cc308 100644
--- a/lpci/tests/test_config.py
+++ b/lpci/tests/test_config.py
@@ -848,3 +848,46 @@ class TestConfig(TestCase):
             Config.load,
             path,
         )
+
+    def test_root_value(self):
+        path = self.create_config(
+            dedent(
+                """
+                pipeline:
+                    - test
+
+                jobs:
+                    test:
+                        root: True
+                        series: focal
+                        architectures: amd64
+                """
+            )
+        )
+        config = Config.load(path)
+        root_value = config.jobs["test"][0].root
+        self.assertEqual(True, root_value)
+
+    def test_bad_root_value(self):
+        path = self.create_config(
+            dedent(
+                """
+                pipeline:
+                    - test
+
+                jobs:
+                    test:
+                        root: bad_value
+                        series: focal
+                        architectures: amd64
+                """
+            )
+        )
+        self.assertRaisesRegex(
+            ValidationError,
+            "You configured `root` parameter, "
+            + "but you did not specify a valid value. "
+            + "Valid values would either be `true` or `false`.",
+            Config.load,
+            path,
+        )
diff --git a/lpci/tests/test_env.py b/lpci/tests/test_env.py
index 58b69f1..7079481 100644
--- a/lpci/tests/test_env.py
+++ b/lpci/tests/test_env.py
@@ -9,6 +9,9 @@ from lpci import env
 
 
 class TestEnvironment(TestCase):
+    def test_get_non_root_user(self):
+        self.assertEqual("_lpci", env.get_non_root_user())
+
     def test_get_managed_environment_home_path(self):
         self.assertEqual(
             Path("/root"), env.get_managed_environment_home_path()