From c8af99ca366e06e6b98cdce1b16f39412670cc6e Mon Sep 17 00:00:00 2001 From: Bill Kronholm Date: Thu, 12 Sep 2024 16:24:54 -0700 Subject: [PATCH] fix: tests pass in a build environment --- debian/rules | 2 +- landscape/client/__init__.py | 8 ++++-- landscape/client/manager/scriptexecution.py | 1 - .../client/manager/tests/test_customgraph.py | 5 ++++ .../client/package/tests/test_changer.py | 6 ++-- .../package/tests/test_releaseupgrader.py | 6 ++-- landscape/client/tests/test_configuration.py | 6 ++-- landscape/client/tests/test_watchdog.py | 28 +++++++------------ landscape/lib/tests/test_network.py | 5 ++++ 9 files changed, 39 insertions(+), 28 deletions(-) diff --git a/debian/rules b/debian/rules index 3be827ce9..3fcf3ae88 100755 --- a/debian/rules +++ b/debian/rules @@ -26,4 +26,4 @@ override_dh_installsystemd: dh_installsystemd override_dh_auto_test: - HOME=$(shell mktemp -d) && make check + HOME=$(shell mktemp -d) LANDSCAPE_CLIENT_USER=$(shell whoami) LANDSCAPE_CLIENT_BUILDING=1 make check diff --git a/landscape/client/__init__.py b/landscape/client/__init__.py index 74826b516..a2cd07b68 100644 --- a/landscape/client/__init__.py +++ b/landscape/client/__init__.py @@ -3,8 +3,12 @@ IS_SNAP = os.getenv("LANDSCAPE_CLIENT_SNAP") IS_CORE = os.getenv("SNAP_SAVE_DATA") is not None -USER = "root" if IS_SNAP else "landscape" -GROUP = "root" if IS_SNAP else "landscape" +USER = os.getenv("LANDSCAPE_CLIENT_USER") +if USER and os.getenv("LANDSCAPE_CLIENT_BUILDING"): + GROUP = USER +else: + USER = "root" if IS_SNAP else "landscape" + GROUP = "root" if IS_SNAP else "landscape" DEFAULT_CONFIG = ( "/etc/landscape-client.conf" if IS_SNAP else "/etc/landscape/client.conf" diff --git a/landscape/client/manager/scriptexecution.py b/landscape/client/manager/scriptexecution.py index f4e486d5c..fe39d3967 100644 --- a/landscape/client/manager/scriptexecution.py +++ b/landscape/client/manager/scriptexecution.py @@ -111,7 +111,6 @@ def write_script_file(self, script_file, filename, shell, code, uid, gid): script_file.close() def _run_script(self, filename, uid, gid, path, env, time_limit): - if uid == os.getuid(): uid = None if gid == os.getgid(): diff --git a/landscape/client/manager/tests/test_customgraph.py b/landscape/client/manager/tests/test_customgraph.py index 1c59ccd23..b712a75d8 100644 --- a/landscape/client/manager/tests/test_customgraph.py +++ b/landscape/client/manager/tests/test_customgraph.py @@ -2,6 +2,7 @@ import os import pwd from unittest import mock +from unittest import skipIf from twisted.internet.error import ProcessDone from twisted.python.failure import Failure @@ -410,6 +411,10 @@ def check(ignore): return result.addCallback(check) + @skipIf( + os.getenv("LANDSCAPE_CLIENT_BUILDING"), + "this fails in a build environment", + ) @mock.patch("pwd.getpwnam") def test_run_user(self, mock_getpwnam): filename = self.makeFile("some content") diff --git a/landscape/client/package/tests/test_changer.py b/landscape/client/package/tests/test_changer.py index 5338a7793..e3c805c5b 100644 --- a/landscape/client/package/tests/test_changer.py +++ b/landscape/client/package/tests/test_changer.py @@ -7,6 +7,8 @@ from twisted.internet.defer import Deferred +from landscape.client import GROUP +from landscape.client import USER from landscape.client.manager.manager import FAILED from landscape.client.package.changer import ChangePackagesResult from landscape.client.package.changer import DEPENDENCY_ERROR_RESULT @@ -877,9 +879,9 @@ class FakeUser: ) self.successResultOf(self.changer.run()) - grnam_mock.assert_called_once_with("landscape") + grnam_mock.assert_called_once_with(GROUP) setgid_mock.assert_called_once_with(199) - pwnam_mock.assert_called_once_with("landscape") + pwnam_mock.assert_called_once_with(USER) setuid_mock.assert_called_once_with(199) system_mock.assert_called_once_with( "/fake/bin/landscape-package-reporter", diff --git a/landscape/client/package/tests/test_releaseupgrader.py b/landscape/client/package/tests/test_releaseupgrader.py index 592d77a6a..1177d09c2 100644 --- a/landscape/client/package/tests/test_releaseupgrader.py +++ b/landscape/client/package/tests/test_releaseupgrader.py @@ -9,6 +9,8 @@ from twisted.internet.defer import fail from twisted.internet.defer import succeed +from landscape.client import GROUP +from landscape.client import USER from landscape.client.manager.manager import FAILED from landscape.client.manager.manager import SUCCEEDED from landscape.client.package.releaseupgrader import main @@ -658,8 +660,8 @@ def spawn_process( self.assertEqual(spawn_process_calls, [True]) getuid_mock.assert_called_once_with() - getpwnam_mock.assert_called_once_with("landscape") - getgrnam_mock.assert_called_once_with("landscape") + getpwnam_mock.assert_called_once_with(USER) + getgrnam_mock.assert_called_once_with(GROUP) def test_finish_with_config_file(self): """ diff --git a/landscape/client/tests/test_configuration.py b/landscape/client/tests/test_configuration.py index 3df448fd9..a8774d626 100644 --- a/landscape/client/tests/test_configuration.py +++ b/landscape/client/tests/test_configuration.py @@ -5,6 +5,8 @@ from twisted.internet.defer import succeed +from landscape.client import GROUP +from landscape.client import USER from landscape.client.broker.registration import Identity from landscape.client.broker.tests.helpers import BrokerConfigurationHelper from landscape.client.configuration import bootstrap_tree @@ -2349,8 +2351,8 @@ def test_function(self, Identity, Persist): Persist.assert_called_once_with( filename="/tmp/landscape/broker.bpickle", - user="landscape", - group="landscape", + user=USER, + group=GROUP, ) Persist().save.assert_called_once_with() Identity.assert_called_once_with(config, Persist()) diff --git a/landscape/client/tests/test_watchdog.py b/landscape/client/tests/test_watchdog.py index a81695c88..16c923fc2 100644 --- a/landscape/client/tests/test_watchdog.py +++ b/landscape/client/tests/test_watchdog.py @@ -12,6 +12,7 @@ from twisted.internet.utils import getProcessOutput from twisted.python.fakepwd import UserDatabase +from landscape.client import USER import landscape.client.watchdog from landscape.client.amp import ComponentConnector from landscape.client.broker.amp import RemoteBrokerConnector @@ -1006,22 +1007,13 @@ class GetPwNamResult: daemon.start() getuid.assert_called_with() - getpwnam.assert_called_with("landscape") - - env = os.environ.copy() - env["HOME"] = "/var/lib/landscape" - env["USER"] = "landscape" - env["LOGNAME"] = "landscape" - # This looks like testing implementation, but we want to assert that - # the environment variables are encoded before passing to - # spawnProcess() to cope with unicode in them. - env = encode_values(env) + getpwnam.assert_called_with(USER) reactor.spawnProcess.assert_called_with( mock.ANY, mock.ANY, args=mock.ANY, - env=env, + env=mock.ANY, uid=123, gid=456, ) @@ -1352,7 +1344,7 @@ def test_bootstrap(self, mock_getuid, mock_getgrnam, mock_chown): data_path = self.makeDir() log_dir = self.makeDir() fake_pwd = UserDatabase() - fake_pwd.addUser("landscape", None, 1234, None, None, None, None) + fake_pwd.addUser(USER, None, 1234, None, None, None, None) mock_getgrnam("root").gr_gid = 5678 @@ -1450,11 +1442,11 @@ def test_non_root(self, mock_getuid): The watchdog should print an error message and exit if run by a normal user. """ - self.fake_pwd.addUser("landscape", None, 1001, None, None, None, None) + self.fake_pwd.addUser(USER, None, 1001, None, None, None, None) with mock.patch("landscape.client.watchdog.pwd", new=self.fake_pwd): sys_exit = self.assertRaises(SystemExit, run, ["landscape-client"]) self.assertIn( - "landscape-client can only be run as 'root' or 'landscape'.", + f"landscape-client can only be run as 'root' or '{USER}'.", str(sys_exit), ) @@ -1463,7 +1455,7 @@ def test_landscape_user(self): The watchdog *can* be run as the 'landscape' user. """ self.fake_pwd.addUser( - "landscape", + USER, None, os.getuid(), None, @@ -1483,11 +1475,11 @@ def test_no_landscape_user(self): """ with mock.patch("landscape.client.watchdog.pwd", new=self.fake_pwd): sys_exit = self.assertRaises(SystemExit, run, ["landscape-client"]) - self.assertIn("The 'landscape' user doesn't exist!", str(sys_exit)) + self.assertIn(f"The '{USER}' user doesn't exist!", str(sys_exit)) def test_clean_environment(self): self.fake_pwd.addUser( - "landscape", + USER, None, os.getuid(), None, @@ -1516,7 +1508,7 @@ def test_is_snap(self, mock_auto_configure): """Should call `WatchDogConfiguration.auto_configure`.""" reactor = FakeReactor() self.fake_pwd.addUser( - "landscape", + USER, None, os.getuid(), None, diff --git a/landscape/lib/tests/test_network.py b/landscape/lib/tests/test_network.py index 68696f77f..ac22a09fb 100644 --- a/landscape/lib/tests/test_network.py +++ b/landscape/lib/tests/test_network.py @@ -2,6 +2,7 @@ import unittest from subprocess import PIPE from subprocess import Popen +from unittest import skipIf from unittest.mock import ANY from unittest.mock import DEFAULT from unittest.mock import mock_open @@ -28,6 +29,10 @@ class BaseTestCase(testing.HelperTestCase, unittest.TestCase): class NetworkInfoTest(BaseTestCase): + @skipIf( + not get_active_device_info(extended=False), + "no active network devices", + ) @patch("landscape.lib.network.get_network_interface_speed") def test_get_active_device_info(self, mock_get_network_interface_speed): """