From b482e23977e109cf18334b1dfaa76028d44f7c7d Mon Sep 17 00:00:00 2001 From: Chris Peterson Date: Tue, 26 Sep 2023 14:50:59 -0700 Subject: [PATCH] autoinstall: Don't use snap env when invoking early and late commands --- subiquity/server/controllers/cmdlist.py | 13 +++- .../server/controllers/tests/test_cmdlist.py | 77 +++++++++++++++++++ subiquitycore/tests/mocks.py | 5 ++ subiquitycore/utils.py | 15 ++-- 4 files changed, 103 insertions(+), 7 deletions(-) create mode 100644 subiquity/server/controllers/tests/test_cmdlist.py diff --git a/subiquity/server/controllers/cmdlist.py b/subiquity/server/controllers/cmdlist.py index b363e5e52..3f5343763 100644 --- a/subiquity/server/controllers/cmdlist.py +++ b/subiquity/server/controllers/cmdlist.py @@ -16,6 +16,7 @@ import asyncio import os import shlex +import shutil from typing import List, Sequence, Union import attr @@ -25,7 +26,7 @@ from subiquity.server.controller import NonInteractiveController from subiquitycore.async_helpers import run_bg_task from subiquitycore.context import with_context -from subiquitycore.utils import arun_command +from subiquitycore.utils import arun_command, orig_environ @attr.s(auto_attribs=True) @@ -78,6 +79,16 @@ async def run(self, context): env = self.env() for i, cmd in enumerate(tuple(self.builtin_cmds) + tuple(self.cmds)): desc = cmd.desc() + + # If the path to the command isn't found on the snap we should + # drop the snap specific environment variables. If the command + # is `env`, we should let the user manage the environment + command = shlex.split(desc)[0] + path = shutil.which(command) + if path is not None: + if not path.startswith("/snap") and command != "env": + env = orig_environ(env) + with context.child("command_{}".format(i), desc): args = cmd.as_args_list() if self.syslog_id: diff --git a/subiquity/server/controllers/tests/test_cmdlist.py b/subiquity/server/controllers/tests/test_cmdlist.py new file mode 100644 index 000000000..837372fe1 --- /dev/null +++ b/subiquity/server/controllers/tests/test_cmdlist.py @@ -0,0 +1,77 @@ +# Copyright 2023 Canonical, Ltd. +# +# This program is free software: you can redistribute it and/or modify +# it under the terms of the GNU Affero General Public License as +# published by the Free Software Foundation, either version 3 of the +# License, or (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU Affero General Public License for more details. +# +# You should have received a copy of the GNU Affero General Public License +# along with this program. If not, see . + +from unittest import IsolatedAsyncioTestCase, mock + +from subiquity.server.controllers import cmdlist +from subiquity.server.controllers.cmdlist import ( + CmdListController, + Command, + EarlyController, + LateController, +) +from subiquitycore.tests.mocks import make_app +from subiquitycore.utils import orig_environ + + +class TestCmdListController(IsolatedAsyncioTestCase): + controller_type = CmdListController + + def setUp(self): + self.controller = self.controller_type(make_app()) + self.controller.cmds = [Command(args="some-command", check=False)] + + self.mocked_arun = mock.patch.object(cmdlist, "arun_command") + self.mocked_orig_environ = mock.patch.object( + cmdlist, "orig_environ", side_effect=orig_environ + ) + snap_env_vars = { + "LD_LIBRARY_PATH": "/var/lib/snapd/lib/gl", + } + self.mocked_os_environ = mock.patch.dict("os.environ", snap_env_vars) + + @mock.patch("shutil.which", return_value="/usr/bin/path/to/bin") + async def test_no_snap_env_on_call(self, mocked_shutil): + with self.mocked_arun as faked_process_call, self.mocked_os_environ, self.mocked_orig_environ as mocked_orig_environ: + await self.controller.run() + _, kwargs = faked_process_call.call_args + call_env = kwargs["env"] + + mocked_orig_environ.assert_called() + assert "LD_LIBRARY_PATH" not in call_env + + @mock.patch("shutil.which", return_value="/snap/path/to/bin") + async def test_with_snap_env_on_call(self, mocked_shutil): + with self.mocked_arun as faked_process_call, self.mocked_os_environ, self.mocked_orig_environ as mocked_orig_environ: + await self.controller.run() + _, kwargs = faked_process_call.call_args + call_env = kwargs["env"] + + mocked_orig_environ.assert_not_called() + assert "LD_LIBRARY_PATH" in call_env + + +class TestEarlyController(TestCmdListController): + controller_type = EarlyController + + def setUp(self): + super().setUp() + + +class TestLateController(TestCmdListController): + controller_type = LateController + + def setUp(self): + super().setUp() diff --git a/subiquitycore/tests/mocks.py b/subiquitycore/tests/mocks.py index 74f2f66bf..663b02b82 100644 --- a/subiquitycore/tests/mocks.py +++ b/subiquitycore/tests/mocks.py @@ -43,4 +43,9 @@ def make_app(model=None): app.opts = mock.Mock() app.opts.dry_run = True app.scale_factor = 1000 + app.echo_syslog_id = None + app.log_syslog_id = None + app.report_start_event = mock.Mock() + app.report_finish_event = mock.Mock() + return app diff --git a/subiquitycore/utils.py b/subiquitycore/utils.py index 2a283b5f0..dfa20e2a1 100644 --- a/subiquitycore/utils.py +++ b/subiquitycore/utils.py @@ -19,7 +19,7 @@ import os import random import subprocess -from typing import Any, Dict, List, Sequence +from typing import Any, Dict, List, Optional, Sequence log = logging.getLogger("subiquitycore.utils") @@ -35,21 +35,24 @@ def _clean_env(env, *, locale=True): return env -def orig_environ(env): +def orig_environ(env: Optional[Dict[str, str]]) -> Dict[str, str]: """Generate an environment dict that is suitable for use for running programs that live outside the snap.""" + if env is None: - env = os.environ - ret = env.copy() + env: Dict[str, str] = os.environ + + ret: Dict[str, str] = env.copy() + for key, val in env.items(): if key.endswith("_ORIG"): - key_to_restore = key[: -len("_ORIG")] + key_to_restore: str = key[: -len("_ORIG")] if val: ret[key_to_restore] = val else: del ret[key_to_restore] del ret[key] - ret.pop("LD_LIBRARY_PATH", None) + _ = ret.pop("LD_LIBRARY_PATH", None) return ret