Merge pull request #1855 from Chris-Peterson444/undo-os-environ-changes

Revert orig_environ fix in command controllers

subiquity/server/controllers/cmdlist.py

@@ -16,7 +16,6 @@
 import asyncio
 import os
 import shlex
-import shutil
 from typing import List, Sequence, Union
 
 import attr
@@ -26,7 +25,7 @@ from subiquity.common.types import ApplicationState
 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, orig_environ
+from subiquitycore.utils import arun_command
 
 
 @attr.s(auto_attribs=True)
@@ -79,15 +78,6 @@ class CmdListController(NonInteractiveController):
         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.
-            command = shlex.split(desc)[0]
-            path = shutil.which(command)
-            if path is not None:
-                if not path.startswith("/snap"):
-                    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

@@ -1,84 +0,0 @@
-# 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
-
-
-@mock.patch.object(cmdlist, "orig_environ", side_effect=orig_environ)
-@mock.patch.object(cmdlist, "arun_command")
-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)]
-        snap_env = {
-            "LD_LIBRARY_PATH": "/var/lib/snapd/lib/gl",
-        }
-        self.mocked_os_environ = mock.patch.dict("os.environ", snap_env)
-
-    @mock.patch("shutil.which", return_value="/usr/bin/path/to/bin")
-    async def test_no_snap_env_on_call(
-        self,
-        mocked_shutil,
-        mocked_arun,
-        mocked_orig_environ,
-    ):
-        with self.mocked_os_environ:
-            await self.controller.run()
-            args, kwargs = mocked_arun.call_args
-            call_env = kwargs["env"]
-
-            mocked_orig_environ.assert_called()
-            self.assertNotIn("LD_LIBRARY_PATH", call_env)
-
-    @mock.patch("shutil.which", return_value="/snap/path/to/bin")
-    async def test_with_snap_env_on_call(
-        self,
-        mocked_shutil,
-        mocked_arun,
-        mocked_orig_environ,
-    ):
-        with self.mocked_os_environ:
-            await self.controller.run()
-            args, kwargs = mocked_arun.call_args
-            call_env = kwargs["env"]
-
-            mocked_orig_environ.assert_not_called()
-            self.assertIn("LD_LIBRARY_PATH", 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/utils.py

@@ -19,7 +19,7 @@ import logging
 import os
 import random
 import subprocess
-from typing import Any, Dict, List, Optional, Sequence
+from typing import Any, Dict, List, Sequence
 
 log = logging.getLogger("subiquitycore.utils")
 
@@ -35,18 +35,15 @@ def _clean_env(env, *, locale=True):
     return env
 
 
-def orig_environ(env: Optional[Dict[str, str]]) -> Dict[str, str]:
+def orig_environ(env):
     """Generate an environment dict that is suitable for use for running
     programs that live outside the snap."""
-
     if env is None:
-        env: Dict[str, str] = os.environ
+        env = os.environ
-
+    ret = env.copy()
-    ret: Dict[str, str] = env.copy()
-
     for key, val in env.items():
         if key.endswith("_ORIG"):
-            key_to_restore: str = key[: -len("_ORIG")]
+            key_to_restore = key[: -len("_ORIG")]
             if val:
                 ret[key_to_restore] = val
             else: