autoinstall: Don't use snap env when invoking early and late commands

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:

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 <http://www.gnu.org/licenses/>.
+
+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()

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

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