launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #29946
[Merge] ~pelpsi/lpci:jobs-cannot-run-as-root into lpci:main
Simone Pelosi has proposed merging ~pelpsi/lpci:jobs-cannot-run-as-root into lpci:main.
Commit message:
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
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #1982954 in lpci: "Some jobs cannot run as root"
https://bugs.launchpad.net/lpci/+bug/1982954
For more details, see:
https://code.launchpad.net/~pelpsi/lpci/+git/lpcraft/+merge/441539
--
Your team Launchpad code reviewers is requested to review the proposed merge of ~pelpsi/lpci:jobs-cannot-run-as-root into lpci:main.
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..db9f589 100644
--- a/docs/configuration.rst
+++ b/docs/configuration.rst
@@ -61,6 +61,10 @@ Job definitions
`/etc/apt/sources.list`.
Also see the :ref:`package-repositories` section below.
+``root`` (optional)
+ If False, allow running commands as default user (_lpci).
+ Default value: True.
+
``snaps`` (optional)
Snaps to install as dependencies of this job.
Also see the :ref:`snap-properties` section below.
diff --git a/lpci/commands/run.py b/lpci/commands/run.py
index d6c65b2..ef29525 100644
--- a/lpci/commands/run.py
+++ b/lpci/commands/run.py
@@ -433,8 +433,14 @@ 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:
+ prefix = ["runuser", "-u", env.get_default_user(), "--"]
+ prefix.extend(full_run_cmd)
+ full_run_cmd = prefix
+
emit.progress("Running command for the job...")
original_mode = emit.get_mode()
if original_mode == EmitterMode.BRIEF:
@@ -475,6 +481,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 +551,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 +595,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..6eda475 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..2490fdf 100644
--- a/lpci/env.py
+++ b/lpci/env.py
@@ -6,6 +6,10 @@
from pathlib import Path
+def get_default_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..5a0e8b2 100644
--- a/lpci/providers/_lxd.py
+++ b/lpci/providers/_lxd.py
@@ -18,6 +18,7 @@ from craft_providers import Base, bases, lxd
from pydantic import StrictStr
from lpci.env import (
+ get_default_user,
get_managed_environment_home_path,
get_managed_environment_project_path,
)
@@ -238,6 +239,34 @@ class LXDProvider(Provider):
**kwargs,
)
+ def _create_default_user(
+ self,
+ instance: lxd.LXDInstance,
+ instance_name: str,
+ ) -> None:
+
+ default_user = get_default_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
+ )
+
+ # Drop privileges to _lpci
+ self._internal_execute_run(
+ instance,
+ instance_name,
+ [
+ "chown",
+ "-R",
+ 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._create_default_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..4957410 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",
+ "/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..5b06ea3 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..8bf4c62 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_default_user(self):
+ self.assertEqual("_lpci", env.get_default_user())
+
def test_get_managed_environment_home_path(self):
self.assertEqual(
Path("/root"), env.get_managed_environment_home_path()
