From b3c563c784789364a5da41d804f6dd7cddefbc1d Mon Sep 17 00:00:00 2001 From: Bill Kronholm Date: Fri, 2 Aug 2024 16:54:36 -0700 Subject: [PATCH 1/3] feat: have `landscape-config --silent` only send a registration message when required and add a new `--force-registration` flag --- landscape/client/configuration.py | 84 ++++++++++++++------ landscape/client/tests/test_configuration.py | 71 +++++++++++++++-- 2 files changed, 122 insertions(+), 33 deletions(-) diff --git a/landscape/client/configuration.py b/landscape/client/configuration.py index 7f330b61..373f16a8 100644 --- a/landscape/client/configuration.py +++ b/landscape/client/configuration.py @@ -113,6 +113,7 @@ class LandscapeSetupConfiguration(BrokerConfiguration): "ok_no_register", "import_from", "skip_registration", + "force_registration", ) encoding = "utf-8" @@ -254,6 +255,11 @@ def make_parser(self): action="store_true", help="Don't send a new registration request", ) + parser.add_option( + "--force-registration", + action="store_true", + help="Force sending a new registration request", + ) return parser @@ -481,8 +487,11 @@ def query_landscape_edition(self): "Landscape Domain: ", True, ).strip("/") - self.landscape_domain = re.sub(r"^https?://", "", - self.landscape_domain) + self.landscape_domain = re.sub( + r"^https?://", + "", + self.landscape_domain, + ) self.config.ping_url = f"http://{self.landscape_domain}/ping" self.config.url = f"https://{self.landscape_domain}/message-system" else: @@ -643,7 +652,9 @@ def setup(config): if not config.no_start: try: - set_secure_id(config, "registering") + secure_id = get_secure_id(config) + if (not secure_id) or config.force_registration: + set_secure_id(config, "registering") ServiceConfig.restart_landscape() except ServiceConfigException as exc: print_text(str(exc), error=True) @@ -732,7 +743,7 @@ def done(ignored_result, connector, reactor): def got_connection(add_result, connector, reactor, remote): - """Handle becomming connected to a broker.""" + """Handle becoming connected to a broker.""" handlers = { "registration-done": partial(success, add_result), "registration-failed": partial(failure, add_result), @@ -817,6 +828,8 @@ def register( if isinstance(result, SystemExit): raise result + set_secure_id(config, "registering") + return result @@ -825,6 +838,7 @@ def report_registration_outcome(what_happened, print=print): human-readable form. """ messages = { + "registration-skipped": "Registration skipped.", "success": "Registration request sent successfully.", "unknown-account": "Invalid account name or registration key.", "max-pending-computers": ( @@ -847,8 +861,9 @@ def report_registration_outcome(what_happened, print=print): ), } message = messages.get(what_happened) + use_std_out = what_happened in {"success", "registration-skipped"} if message: - fd = sys.stdout if what_happened == "success" else sys.stderr + fd = sys.stdout if use_std_out else sys.stderr print(message, file=fd) @@ -856,7 +871,7 @@ def determine_exit_code(what_happened): """Return what the application's exit code should be depending on the registration result. """ - if what_happened == "success": + if what_happened in {"success", "registration-skipped"}: return 0 else: return 2 # An error happened @@ -912,6 +927,17 @@ def set_secure_id(config, new_id): persist.save() +def get_secure_id(config): + persist = Persist( + filename=os.path.join( + config.data_path, + f"{BrokerService.service_name}.bpickle", + ), + ) + identity = Identity(config, persist) + return identity.secure_id + + def main(args, print=print): """Interact with the user and the server to set up client configuration.""" @@ -922,9 +948,17 @@ def main(args, print=print): print_text(str(error), error=True) sys.exit(1) + if config.skip_registration and config.force_registration: + sys.exit( + "Do not set both skip registration " + "and force registration together.", + ) + + already_registered = is_registered(config) + if config.is_registered: - registration_status = is_registered(config) + registration_status = already_registered info_text = registration_info_text(config, registration_status) print(info_text) @@ -953,31 +987,31 @@ def main(args, print=print): print_text(str(e)) sys.exit("Aborting Landscape configuration") + if config.skip_registration: + return + # Attempt to register the client. reactor = LandscapeReactor() - if config.skip_registration: - return + should_register = False - if config.silent: + if config.silent and (not already_registered): + should_register = True + elif config.force_registration: + should_register = True + elif not config.silent: + default_answer = not already_registered + should_register = prompt_yes_no( + "\nRequest a new registration for this computer now?", + default=default_answer, + ) + if should_register: result = register( config, reactor, on_error=lambda _: set_secure_id(config, None), ) - report_registration_outcome(result, print=print) - sys.exit(determine_exit_code(result)) else: - default_answer = not is_registered(config) - answer = prompt_yes_no( - "\nRequest a new registration for this computer now?", - default=default_answer, - ) - if answer: - result = register( - config, - reactor, - on_error=lambda _: set_secure_id(config, None), - ) - report_registration_outcome(result, print=print) - sys.exit(determine_exit_code(result)) + result = "registration-skipped" + report_registration_outcome(result, print=print) + sys.exit(determine_exit_code(result)) diff --git a/landscape/client/tests/test_configuration.py b/landscape/client/tests/test_configuration.py index 5d3503fc..7153648f 100644 --- a/landscape/client/tests/test_configuration.py +++ b/landscape/client/tests/test_configuration.py @@ -1266,16 +1266,23 @@ def test_setup_prefers_proxies_from_config_over_environment( setup(config) mock_setup_script().run.assert_called_once_with() - # Reload it to enusre it was written down. + # Reload it to ensure it was written down. config.reload() self.assertEqual(config.http_proxy, "http://config") self.assertEqual(config.https_proxy, "https://config") + @mock.patch("landscape.client.configuration.sys.exit") @mock.patch("landscape.client.configuration.input", return_value="n") @mock.patch("landscape.client.configuration.register") @mock.patch("landscape.client.configuration.setup") - def test_main_no_registration(self, mock_setup, mock_register, mock_input): + def test_main_no_registration( + self, + mock_setup, + mock_register, + mock_input, + mock_sys_exit, + ): main(["-c", self.make_working_config()], print=noop_print) mock_register.assert_not_called() mock_input.assert_called_once_with( @@ -1290,8 +1297,10 @@ def test_skip_registration(self, mock_setup, mock_register, mock_input): Registration and input asking user to register is not called when flag on """ - main(["-c", self.make_working_config(), "--skip-registration"], - print=noop_print) + main( + ["-c", self.make_working_config(), "--skip-registration"], + print=noop_print, + ) mock_register.assert_not_called() mock_input.assert_not_called() @@ -1299,10 +1308,41 @@ def test_skip_registration(self, mock_setup, mock_register, mock_input): @mock.patch("landscape.client.configuration.setup") def test_main_no_registration_silent(self, mock_setup, mock_register): """Skip registration works in silent mode""" - main(["-c", self.make_working_config(), "--skip-registration", - "--silent"], print=noop_print) + main( + [ + "-c", + self.make_working_config(), + "--skip-registration", + "--silent", + ], + print=noop_print, + ) mock_register.assert_not_called() + @mock.patch("landscape.client.configuration.sys.exit") + @mock.patch("landscape.client.configuration.input") + @mock.patch("landscape.client.configuration.register") + @mock.patch("landscape.client.configuration.setup") + def test_main_force_registration_silent( + self, + mock_setup, + mock_register, + mock_input, + mock_sys_exit, + ): + """Force registration works in silent mode""" + main( + [ + "-c", + self.make_working_config(), + "--force-registration", + "--silent", + ], + print=noop_print, + ) + mock_register.assert_called_once() + mock_input.assert_not_called() + @mock.patch( "landscape.client.configuration.register", return_value="success", @@ -2691,6 +2731,14 @@ def test_success_case(self): self.assertIn("Registration request sent successfully.", self.result) self.assertIn(sys.stdout.name, self.output) + def test_registration_skipped_case(self): + report_registration_outcome( + "registration-skipped", + print=self.record_result, + ) + self.assertIn("Registration skipped.", self.result) + self.assertIn(sys.stdout.name, self.output) + def test_unknown_account_case(self): """ If the unknown-account error is found, an appropriate message is @@ -2756,6 +2804,13 @@ def test_success_means_exit_code_0(self): result = determine_exit_code("success") self.assertEqual(0, result) + def test_registration_skipped_means_exit_code_0(self): + """ + When passed "success" the determine_exit_code function returns 0. + """ + result = determine_exit_code("registration-skipped") + self.assertEqual(0, result) + def test_a_failure_means_exit_code_2(self): """ When passed a failure result, the determine_exit_code function returns @@ -2783,13 +2838,13 @@ def setUp(self): def test_is_registered_false(self): """ - If the client hasn't previouly registered, is_registered returns False. + If the client hasn't previously registered, is_registered returns False """ self.assertFalse(is_registered(self.config)) def test_is_registered_true(self): """ - If the client has previouly registered, is_registered returns True. + If the client has previously registered, is_registered returns True. """ self.persist.set("registration.secure-id", "super-secure") self.persist.save() From 383a3ca62cdd2f2dca7270e4bb4bb43e14cfcf7a Mon Sep 17 00:00:00 2001 From: Bill Kronholm Date: Tue, 6 Aug 2024 14:36:52 -0700 Subject: [PATCH 2/3] refactor --- landscape/client/configuration.py | 34 +++- landscape/client/tests/test_configuration.py | 198 ++++++++++++------- 2 files changed, 154 insertions(+), 78 deletions(-) diff --git a/landscape/client/configuration.py b/landscape/client/configuration.py index 373f16a8..6d49e762 100644 --- a/landscape/client/configuration.py +++ b/landscape/client/configuration.py @@ -114,6 +114,7 @@ class LandscapeSetupConfiguration(BrokerConfiguration): "import_from", "skip_registration", "force_registration", + "register_if_needed", ) encoding = "utf-8" @@ -260,6 +261,13 @@ def make_parser(self): action="store_true", help="Force sending a new registration request", ) + parser.add_option( + "--register-if-needed", + action="store_true", + help=( + "Send a new registration request only if one has not been sent" + ), + ) return parser @@ -648,13 +656,13 @@ def setup(config): script.run() decode_base64_ssl_public_certificate(config) config.write() - # Restart the client to ensure that it's using the new configuration. + +def restart_client(config): + """Restart the client to ensure that it's using the new configuration.""" if not config.no_start: try: - secure_id = get_secure_id(config) - if (not secure_id) or config.force_registration: - set_secure_id(config, "registering") + set_secure_id(config, "registering") ServiceConfig.restart_landscape() except ServiceConfigException as exc: print_text(str(exc), error=True) @@ -988,24 +996,30 @@ def main(args, print=print): sys.exit("Aborting Landscape configuration") if config.skip_registration: + result = "registration-skipped" + report_registration_outcome(result, print=print) return - # Attempt to register the client. - reactor = LandscapeReactor() - should_register = False - if config.silent and (not already_registered): + if config.force_registration: should_register = True - elif config.force_registration: + elif config.silent and not config.register_if_needed: should_register = True - elif not config.silent: + elif config.register_if_needed: + should_register = not already_registered + else: default_answer = not already_registered should_register = prompt_yes_no( "\nRequest a new registration for this computer now?", default=default_answer, ) + + if should_register or config.silent: + restart_client(config) if should_register: + # Attempt to register the client. + reactor = LandscapeReactor() result = register( config, reactor, diff --git a/landscape/client/tests/test_configuration.py b/landscape/client/tests/test_configuration.py index 7153648f..d2f56651 100644 --- a/landscape/client/tests/test_configuration.py +++ b/landscape/client/tests/test_configuration.py @@ -33,6 +33,7 @@ from landscape.client.configuration import register from landscape.client.configuration import registration_info_text from landscape.client.configuration import report_registration_outcome +from landscape.client.configuration import restart_client from landscape.client.configuration import set_secure_id from landscape.client.configuration import setup from landscape.client.configuration import show_help @@ -952,13 +953,11 @@ def side_effect_getpass(prompt): @mock.patch("landscape.client.configuration.ServiceConfig") def test_silent_setup(self, mock_serviceconfig): """ - Only command-line options are used in silent mode and registration is - attempted. + Only command-line options are used in silent mode. """ config = self.get_config(["--silent", "-a", "account", "-t", "rex"]) setup(config) mock_serviceconfig.set_start_on_boot.assert_called_once_with(True) - mock_serviceconfig.restart_landscape.assert_called_once_with() self.assertConfigEqual( self.get_content(config), f"""\ @@ -1050,7 +1049,6 @@ def test_silent_setup_unicode_computer_title(self, mock_serviceconfig): config = self.get_config(["--silent", "-a", "account", "-t", "mélody"]) setup(config) mock_serviceconfig.set_start_on_boot.assert_called_once_with(True) - mock_serviceconfig.restart_landscape.assert_called_once_with() self.assertConfigEqual( self.get_content(config), f"""\ @@ -1110,7 +1108,6 @@ def test_silent_script_users_imply_script_execution_plugin( mock_serviceconfig.restart_landscape.return_value = True setup(config) mock_serviceconfig.set_start_on_boot.assert_called_once_with(True) - mock_serviceconfig.restart_landscape.assert_called_once_with() mock_input.assert_not_called() parser = ConfigParser() parser.read(filename) @@ -1176,7 +1173,6 @@ def test_silent_setup_with_ping_url(self, mock_serviceconfig): ) setup(config) mock_serviceconfig.set_start_on_boot.assert_called_once_with(True) - mock_serviceconfig.restart_landscape.assert_called_once_with() parser = ConfigParser() parser.read(filename) @@ -1216,8 +1212,7 @@ def test_silent_setup_with_proxies_from_environment( mock_serviceconfig, ): """ - Only command-line options are used in silent mode and registration is - attempted. + Only command-line options are used in silent mode. """ os.environ["http_proxy"] = "http://environ" os.environ["https_proxy"] = "https://environ" @@ -1233,7 +1228,6 @@ def test_silent_setup_with_proxies_from_environment( ) setup(config) mock_serviceconfig.set_start_on_boot.assert_called_once_with(True) - mock_serviceconfig.restart_landscape.assert_called_once_with() parser = ConfigParser() parser.read(filename) self.assertEqual( @@ -1289,10 +1283,17 @@ def test_main_no_registration( "\nRequest a new registration for this computer now? [Y/n]: ", ) + @mock.patch("landscape.client.configuration.sys.exit") @mock.patch("landscape.client.configuration.input") @mock.patch("landscape.client.configuration.register") @mock.patch("landscape.client.configuration.setup") - def test_skip_registration(self, mock_setup, mock_register, mock_input): + def test_skip_registration( + self, + mock_setup, + mock_register, + mock_input, + mock_sys_exit, + ): """ Registration and input asking user to register is not called when flag on @@ -1319,6 +1320,7 @@ def test_main_no_registration_silent(self, mock_setup, mock_register): ) mock_register.assert_not_called() + @mock.patch("landscape.client.configuration.restart_client") @mock.patch("landscape.client.configuration.sys.exit") @mock.patch("landscape.client.configuration.input") @mock.patch("landscape.client.configuration.register") @@ -1329,6 +1331,7 @@ def test_main_force_registration_silent( mock_register, mock_input, mock_sys_exit, + mock_restart_client, ): """Force registration works in silent mode""" main( @@ -1340,15 +1343,81 @@ def test_main_force_registration_silent( ], print=noop_print, ) + mock_restart_client.assert_called_once() + mock_register.assert_called_once() + mock_input.assert_not_called() + + @mock.patch("landscape.client.configuration.restart_client") + @mock.patch("landscape.client.configuration.input") + @mock.patch( + "landscape.client.configuration.register", + return_value="success", + ) + @mock.patch("landscape.client.configuration.setup") + def test_main_register_if_needed_silent( + self, + mock_setup, + mock_register, + mock_input, + mock_restart_client, + ): + """Conditional registration works in silent mode""" + system_exit = self.assertRaises( + SystemExit, + main, + [ + "-c", + self.make_working_config(), + "--register-if-needed", + "--silent", + ], + print=noop_print, + ) + self.assertEqual(0, system_exit.code) + mock_restart_client.assert_called_once() mock_register.assert_called_once() mock_input.assert_not_called() + @mock.patch( + "landscape.client.configuration.is_registered", + return_value=True, + ) + @mock.patch("landscape.client.configuration.restart_client") + @mock.patch("landscape.client.configuration.input") + @mock.patch("landscape.client.configuration.register") + @mock.patch("landscape.client.configuration.setup") + def test_main_do_not_register_if_not_needed_silent( + self, + mock_setup, + mock_register, + mock_input, + mock_restart_client, + mock_is_registered, + ): + """Conditional registration works in silent mode""" + system_exit = self.assertRaises( + SystemExit, + main, + [ + "-c", + self.make_working_config(), + "--register-if-needed", + "--silent", + ], + print=noop_print, + ) + self.assertEqual(0, system_exit.code) + mock_restart_client.assert_called_once() + mock_register.assert_not_called() + mock_input.assert_not_called() + + @mock.patch("landscape.client.configuration.restart_client") @mock.patch( "landscape.client.configuration.register", return_value="success", ) @mock.patch("landscape.client.configuration.setup") - def test_main_silent(self, mock_setup, mock_register): + def test_main_silent(self, mock_setup, mock_register, mock_restart_client): """ In silent mode, the client should register when the registration details are changed/set. @@ -1367,13 +1436,11 @@ def test_main_silent(self, mock_setup, mock_register): print=noop_print, ) self.assertEqual(0, exception.code) - mock_setup.assert_called_once_with(mock.ANY) - mock_register.assert_called_once_with( - mock.ANY, - mock.ANY, - on_error=mock.ANY, - ) + mock_setup.assert_called_once() + mock_restart_client.assert_called_once() + mock_register.assert_called_once() + @mock.patch("landscape.client.configuration.restart_client") @mock.patch("landscape.client.configuration.input", return_value="y") @mock.patch( "landscape.client.configuration.register", @@ -1385,6 +1452,7 @@ def test_main_user_interaction_success( mock_setup, mock_register, mock_input, + mock_restart_client, ): """The successful result of register() is communicated to the user.""" printed = [] @@ -1399,12 +1467,9 @@ def faux_print(string, file=sys.stdout): print=faux_print, ) self.assertEqual(0, exception.code) - mock_setup.assert_called_once_with(mock.ANY) - mock_register.assert_called_once_with( - mock.ANY, - mock.ANY, - on_error=mock.ANY, - ) + mock_setup.assert_called_once() + mock_restart_client.assert_called_once() + mock_register.assert_called_once() mock_input.assert_called_once_with( "\nRequest a new registration for this computer now? [Y/n]: ", ) @@ -1415,6 +1480,7 @@ def faux_print(string, file=sys.stdout): printed, ) + @mock.patch("landscape.client.configuration.restart_client") @mock.patch("landscape.client.configuration.input", return_value="y") @mock.patch( "landscape.client.configuration.register", @@ -1426,6 +1492,7 @@ def test_main_user_interaction_failure( mock_setup, mock_register, mock_input, + mock_restart_client, ): """The failed result of register() is communicated to the user.""" printed = [] @@ -1440,12 +1507,9 @@ def faux_print(string, file=sys.stdout): print=faux_print, ) self.assertEqual(2, exception.code) - mock_setup.assert_called_once_with(mock.ANY) - mock_register.assert_called_once_with( - mock.ANY, - mock.ANY, - on_error=mock.ANY, - ) + mock_setup.assert_called_once() + mock_restart_client.assert_called_once() + mock_register.assert_called_once() mock_input.assert_called_once_with( "\nRequest a new registration for this computer now? [Y/n]: ", ) @@ -1458,6 +1522,7 @@ def faux_print(string, file=sys.stdout): printed, ) + @mock.patch("landscape.client.configuration.restart_client") @mock.patch("landscape.client.configuration.input") @mock.patch( "landscape.client.configuration.register", @@ -1469,6 +1534,7 @@ def test_main_user_interaction_success_silent( mock_setup, mock_register, mock_input, + mock_restart_client, ): """Successful result is communicated to the user even with --silent.""" printed = [] @@ -1483,12 +1549,9 @@ def faux_print(string, file=sys.stdout): print=faux_print, ) self.assertEqual(0, exception.code) - mock_setup.assert_called_once_with(mock.ANY) - mock_register.assert_called_once_with( - mock.ANY, - mock.ANY, - on_error=mock.ANY, - ) + mock_setup.assert_called_once() + mock_restart_client.assert_called_once() + mock_register.assert_called_once() mock_input.assert_not_called() self.assertEqual( @@ -1498,6 +1561,7 @@ def faux_print(string, file=sys.stdout): printed, ) + @mock.patch("landscape.client.configuration.restart_client") @mock.patch("landscape.client.configuration.input") @mock.patch( "landscape.client.configuration.register", @@ -1509,6 +1573,7 @@ def test_main_user_interaction_failure_silent( mock_setup, mock_register, mock_input, + mock_restart_client, ): """ A failure result is communicated to the user even with --silent. @@ -1525,12 +1590,9 @@ def faux_print(string, file=sys.stdout): print=faux_print, ) self.assertEqual(2, exception.code) - mock_setup.assert_called_once_with(mock.ANY) - mock_register.assert_called_once_with( - mock.ANY, - mock.ANY, - on_error=mock.ANY, - ) + mock_setup.assert_called_once() + mock_restart_client.assert_called_once() + mock_register.assert_called_once() mock_input.assert_not_called() # Note that the error is output via sys.stderr. self.assertEqual( @@ -1575,11 +1637,7 @@ def test_register_system_exit( mock_serviceconfig.set_start_on_boot.assert_called_once_with(True) mock_serviceconfig.restart_landscape.assert_called_once_with() mock_setup_script().run.assert_called_once_with() - mock_register.assert_called_once_with( - mock.ANY, - mock.ANY, - on_error=mock.ANY, - ) + mock_register.assert_called_once() mock_input.assert_called_with( "\nRequest a new registration for this computer now? [Y/n]: ", ) @@ -1601,7 +1659,8 @@ def test_errors_from_restart_landscape( ) config = self.get_config(["--silent", "-a", "account", "-t", "rex"]) - system_exit = self.assertRaises(SystemExit, setup, config) + setup(config) + system_exit = self.assertRaises(SystemExit, restart_client, config) self.assertEqual(system_exit.code, 2) mock_serviceconfig.set_start_on_boot.assert_called_once_with(True) mock_serviceconfig.restart_landscape.assert_called_once_with() @@ -1633,7 +1692,8 @@ def test_errors_from_restart_landscape_ok_no_register( config = self.get_config( ["--silent", "-a", "account", "-t", "rex", "--ok-no-register"], ) - system_exit = self.assertRaises(SystemExit, setup, config) + setup(config) + system_exit = self.assertRaises(SystemExit, restart_client, config) self.assertEqual(system_exit.code, 0) mock_serviceconfig.set_start_on_boot.assert_called_once_with(True) mock_serviceconfig.restart_landscape.assert_called_once_with() @@ -1647,30 +1707,41 @@ def test_errors_from_restart_landscape_ok_no_register( error=True, ) + @mock.patch("landscape.client.configuration.restart_client") @mock.patch("landscape.client.configuration.input", return_value="") @mock.patch("landscape.client.configuration.register") @mock.patch("landscape.client.configuration.setup") - def test_main_with_register(self, mock_setup, mock_register, mock_input): + def test_main_with_register( + self, + mock_setup, + mock_register, + mock_input, + mock_restart_client, + ): self.assertRaises( SystemExit, main, ["-c", self.make_working_config()], print=noop_print, ) - mock_setup.assert_called_once_with(mock.ANY) - mock_register.assert_called_once_with( - mock.ANY, - mock.ANY, - on_error=mock.ANY, - ) + mock_setup.assert_called_once() + mock_restart_client.assert_called_once() + mock_register.assert_called_once() mock_input.assert_called_once_with( "\nRequest a new registration for this computer now? [Y/n]: ", ) + @mock.patch("landscape.client.configuration.restart_client") @mock.patch("landscape.client.configuration.input") @mock.patch("landscape.client.configuration.register") @mock.patch("landscape.client.configuration.setup") - def test_register_silent(self, mock_setup, mock_register, mock_input): + def test_register_silent( + self, + mock_setup, + mock_register, + mock_input, + mock_restart_client, + ): """ Silent registration uses specified configuration to attempt a registration with the server. @@ -1681,12 +1752,9 @@ def test_register_silent(self, mock_setup, mock_register, mock_input): ["--silent", "-c", self.make_working_config()], print=noop_print, ) - mock_setup.assert_called_once_with(mock.ANY) - mock_register.assert_called_once_with( - mock.ANY, - mock.ANY, - on_error=mock.ANY, - ) + mock_setup.assert_called_once() + mock_restart_client.assert_called_once() + mock_register.assert_called_once() mock_input.assert_not_called() @mock.patch("landscape.client.configuration.input") @@ -1766,7 +1834,6 @@ def test_import_from_file(self, mock_serviceconfig): setup(config) mock_serviceconfig.set_start_on_boot.assert_called_once_with(True) - mock_serviceconfig.restart_landscape.assert_called_once_with() options = ConfigParser() options.read(config_filename) @@ -1948,7 +2015,6 @@ def test_import_from_file_preserves_old_options(self, mock_serviceconfig): setup(config) mock_serviceconfig.set_start_on_boot.assert_called_once_with(True) - mock_serviceconfig.restart_landscape.assert_called_once_with() options = ConfigParser() options.read(config_filename) @@ -2001,7 +2067,6 @@ def test_import_from_file_may_reset_old_options(self, mock_serviceconfig): ) setup(config) mock_serviceconfig.set_start_on_boot.assert_called_once_with(True) - mock_serviceconfig.restart_landscape.assert_called_once_with() options = ConfigParser() options.read(config_filename) @@ -2055,7 +2120,6 @@ def test_import_from_url( "Fetching configuration from https://config.url...", ) mock_serviceconfig.set_start_on_boot.assert_called_once_with(True) - mock_serviceconfig.restart_landscape.assert_called_once_with() options = ConfigParser() options.read(config_filename) @@ -2247,7 +2311,6 @@ def test_base64_ssl_public_key_is_exported_to_file( setup(config) mock_serviceconfig.set_start_on_boot.assert_called_once_with(True) - mock_serviceconfig.restart_landscape.assert_called_once_with() mock_print_text.assert_called_once_with( f"Writing SSL CA certificate to {key_filename}...", ) @@ -2284,7 +2347,6 @@ def test_normal_ssl_public_key_is_not_exported_to_file( setup(config) mock_serviceconfig.set_start_on_boot.assert_called_once_with(True) - mock_serviceconfig.restart_landscape.assert_called_once_with() key_filename = config_filename + ".ssl_public_key" self.assertFalse(os.path.isfile(key_filename)) From b0a0fb5876ad1980b2474ad201fe610277071f39 Mon Sep 17 00:00:00 2001 From: Bill Kronholm Date: Tue, 13 Aug 2024 11:15:42 -0700 Subject: [PATCH 3/3] Only set secure_id to registering if it has not yet been set. --- landscape/client/broker/exchange.py | 8 ++++---- landscape/client/broker/server.py | 2 +- landscape/client/configuration.py | 4 +++- landscape/client/tests/test_configuration.py | 17 ++++++++++++++++- 4 files changed, 24 insertions(+), 7 deletions(-) diff --git a/landscape/client/broker/exchange.py b/landscape/client/broker/exchange.py index 31f32ff7..b2234729 100644 --- a/landscape/client/broker/exchange.py +++ b/landscape/client/broker/exchange.py @@ -49,8 +49,8 @@ - C{CLIENT_ACCEPTED_TYPES}: Optionally, a list of message types that the client accepts. The server is supposed to send the client only messages of - this type. It will be inlcuded in the payload only if the hash that the - server sends us is out-of-date. This behavior is simmetric with respect to + this type. It will be included in the payload only if the hash that the + server sends us is out-of-date. This behavior is symmetric with respect to the C{SERVER_ACCEPTED_TYPES_DIGEST} field described above. Server->Client Payload @@ -91,7 +91,7 @@ - C{EXPECTED_EXCHANGE_TOKEN}: The token (UUID string) that the server expects to receive back the next time the client performs an exchange. Since the client receives a new token at each exchange, this can be used by the - server to detect cloned clients (either the orignal client or the cloned + server to detect cloned clients (either the original client or the cloned client will eventually send an expired token). The token is sent by the client as a special HTTP header (see L{landscape.broker.transport}). @@ -163,7 +163,7 @@ next-expected-sequence in the prior connection, or 0 if there was no previous connection. - - Get back a next-expected-sequence from the server. If that value is is not + - Get back a next-expected-sequence from the server. If that value is not len(messages) + previous-next-expected, then resynchronize. It does the following when acting as Receiver: diff --git a/landscape/client/broker/server.py b/landscape/client/broker/server.py index 766c6a03..db0e7f6c 100644 --- a/landscape/client/broker/server.py +++ b/landscape/client/broker/server.py @@ -408,7 +408,7 @@ def _message_delivered(self, results, message): @remote def stop_exchanger(self): """ - Stop exchaging messages with the message server. + Stop exchanging messages with the message server. Eventually, it is required by the plugin that no more message exchanges are performed. diff --git a/landscape/client/configuration.py b/landscape/client/configuration.py index 6d49e762..3ddb80b3 100644 --- a/landscape/client/configuration.py +++ b/landscape/client/configuration.py @@ -662,7 +662,9 @@ def restart_client(config): """Restart the client to ensure that it's using the new configuration.""" if not config.no_start: try: - set_secure_id(config, "registering") + secure_id = get_secure_id(config) + if not secure_id: + set_secure_id(config, "registering") ServiceConfig.restart_landscape() except ServiceConfigException as exc: print_text(str(exc), error=True) diff --git a/landscape/client/tests/test_configuration.py b/landscape/client/tests/test_configuration.py index d2f56651..3ca37493 100644 --- a/landscape/client/tests/test_configuration.py +++ b/landscape/client/tests/test_configuration.py @@ -20,6 +20,7 @@ from landscape.client.configuration import exchange_failure from landscape.client.configuration import EXIT_NOT_REGISTERED from landscape.client.configuration import failure +from landscape.client.configuration import get_secure_id from landscape.client.configuration import got_connection from landscape.client.configuration import got_error from landscape.client.configuration import handle_registration_errors @@ -2868,7 +2869,8 @@ def test_success_means_exit_code_0(self): def test_registration_skipped_means_exit_code_0(self): """ - When passed "success" the determine_exit_code function returns 0. + When passed "registration-skipped" the determine_exit_code + function returns 0. """ result = determine_exit_code("registration-skipped") self.assertEqual(0, result) @@ -3009,3 +3011,16 @@ def test_function(self, Identity, Persist): Persist().save.assert_called_once_with() Identity.assert_called_once_with(config, Persist()) self.assertEqual(Identity().secure_id, "fancysecureid") + + +class GetSecureIdTest(LandscapeTest): + @mock.patch("landscape.client.configuration.Persist") + @mock.patch("landscape.client.configuration.Identity") + def test_function(self, Identity, Persist): + config = mock.Mock(data_path="/tmp/landscape") + + set_secure_id(config, "fancysecureid") + + secure_id = get_secure_id(config) + + self.assertEqual(secure_id, "fancysecureid")