From ac2226948b30eaa1b7c7785b8c8f9ebc5bb5d326 Mon Sep 17 00:00:00 2001 From: Rajasi Rane Date: Wed, 13 Sep 2023 14:42:32 -0700 Subject: [PATCH] Adding EULA acceptance: Addressing PR feedback #2 --- .../src/bootstrap/ConfigurationFactory.py | 53 ------------ src/core/src/bootstrap/Constants.py | 4 +- src/core/src/core_logic/ExecutionConfig.py | 25 +++++- src/core/src/core_logic/SystemctlManager.py | 2 +- src/core/tests/Test_ConfigurationFactory.py | 83 +++++++++++-------- src/core/tests/library/ArgumentComposer.py | 2 +- src/core/tests/library/RuntimeCompositor.py | 2 +- 7 files changed, 79 insertions(+), 92 deletions(-) diff --git a/src/core/src/bootstrap/ConfigurationFactory.py b/src/core/src/bootstrap/ConfigurationFactory.py index 11d93531..0b467b40 100644 --- a/src/core/src/bootstrap/ConfigurationFactory.py +++ b/src/core/src/bootstrap/ConfigurationFactory.py @@ -102,7 +102,6 @@ def get_arguments_configuration(self, argv): 'component_args': ['env_layer', 'composite_logger'], 'component_kwargs': { 'execution_parameters': str(argv), - 'accept_package_eula': self.is_package_eula_accepted(), } } } @@ -300,56 +299,4 @@ def get_vm_cloud_type(): else: print("Failed to connect IMDS end point after 5 retries. This is expected in Arc VMs. VMCloudType is set to Arc.\n") return Constants.VMCloudType.ARC - - def is_package_eula_accepted(self): - """ Reads customer provided config on EULA acceptance from disk and returns a boolean. - NOTE: This is a temporary solution and will be deprecated no later than TBD date""" - if not os.path.exists(Constants.Paths.EULA_SETTINGS): - print("NOT accepting EULA for any patch as no corresponding EULA Settings found on the VM") - return False - - try: - eula_settings = json.loads(self.read_with_retry(Constants.Paths.EULA_SETTINGS) or 'null') - if eula_settings is not None \ - and Constants.EulaSettings.ACCEPT_EULA_FOR_ALL_PATCHES in eula_settings \ - and eula_settings[Constants.EulaSettings.ACCEPT_EULA_FOR_ALL_PATCHES] is True: - print("Accept EULA set to True in customer config") - return True - else: - print("Accept EULA not found to be set to True in customer config") - return False - except Exception as error: - print("Error occurred while reading and parsing EULA settings. Not accepting EULA for any patch. Error=[{0}]".format(repr(error))) - return False - - @staticmethod - def open(file_path, mode): - """ Provides a file handle to the file_path requested using implicit redirection where required """ - for i in range(0, Constants.MAX_FILE_OPERATION_RETRY_COUNT): - try: - return open(file_path, mode) - except Exception as error: - if i < Constants.MAX_FILE_OPERATION_RETRY_COUNT - 1: - time.sleep(i + 1) - else: - raise Exception("Unable to open {0} (retries exhausted). Error: {1}.".format(str(file_path), repr(error))) - - def __obtain_file_handle(self, file_path_or_handle, mode='a+'): - """ Pass-through for handle. For path, resolution and handle open with retry. """ - is_path = False - if isinstance(file_path_or_handle, str) or not (hasattr(file_path_or_handle, 'read') and hasattr(file_path_or_handle, 'write')): - is_path = True - file_path_or_handle = self.open(file_path_or_handle, mode) - file_handle = file_path_or_handle - return file_handle, is_path - - def read_with_retry(self, file_path_or_handle): - """ Reads all content from a given file path in a single operation """ - if isinstance(file_path_or_handle, str): - file_handle, was_path = self.__obtain_file_handle(file_path_or_handle, 'r') - value = file_handle.read() - if was_path: # what was passed in was not a file handle, so close the handle that was init here - file_handle.close() - return value - return None # endregion diff --git a/src/core/src/bootstrap/Constants.py b/src/core/src/bootstrap/Constants.py index 1384c7cc..95736cc9 100644 --- a/src/core/src/bootstrap/Constants.py +++ b/src/core/src/bootstrap/Constants.py @@ -51,8 +51,10 @@ def __iter__(self): MAX_AUTO_ASSESSMENT_LOGFILE_SIZE_IN_BYTES = 5*1024*1024 MAX_AUTO_ASSESSMENT_WAIT_FOR_MAIN_CORE_EXEC_IN_MINUTES = 3 * 60 - class Paths(EnumBackport): + class SystemPaths(EnumBackport): SYSTEMD_ROOT = "/etc/systemd/system/" + + class AzGPSPaths(EnumBackport): EULA_SETTINGS = "/var/lib/azure/linuxpatchextension/patch.eula.settings" class EnvSettings(EnumBackport): diff --git a/src/core/src/core_logic/ExecutionConfig.py b/src/core/src/core_logic/ExecutionConfig.py index 45f0a16c..7e30b2c2 100644 --- a/src/core/src/core_logic/ExecutionConfig.py +++ b/src/core/src/core_logic/ExecutionConfig.py @@ -23,7 +23,7 @@ class ExecutionConfig(object): - def __init__(self, env_layer, composite_logger, execution_parameters, accept_package_eula): + def __init__(self, env_layer, composite_logger, execution_parameters): self.env_layer = env_layer self.composite_logger = composite_logger self.execution_parameters = eval(execution_parameters) @@ -87,7 +87,7 @@ def __init__(self, env_layer, composite_logger, execution_parameters, accept_pac self.composite_logger.log_debug("Not executing in auto-assessment mode.") # EULA config - self.accept_package_eula = accept_package_eula + self.accept_package_eula = self.__is_package_eula_accepted() def __transform_execution_config_for_auto_assessment(self): self.activity_id = str(uuid.uuid4()) @@ -182,3 +182,24 @@ def __check_and_create_temp_folder_if_not_exists(self): self.composite_logger.log_debug("Temp folder does not exist, creating one from extension core. [Path={0}]".format(str(self.temp_folder))) os.mkdir(self.temp_folder) + def __is_package_eula_accepted(self): + """ Reads customer provided config on EULA acceptance from disk and returns a boolean. + NOTE: This is a temporary solution and will be deprecated no later than TBD date""" + if not os.path.exists(Constants.AzGPSPaths.EULA_SETTINGS): + print("NOT accepting EULA for any patch as no corresponding EULA Settings found on the VM") + return False + + try: + eula_settings = json.loads(self.env_layer.file_system.read_with_retry(Constants.AzGPSPaths.EULA_SETTINGS) or 'null') + if eula_settings is not None \ + and Constants.EulaSettings.ACCEPT_EULA_FOR_ALL_PATCHES in eula_settings \ + and eula_settings[Constants.EulaSettings.ACCEPT_EULA_FOR_ALL_PATCHES] is True: + print("Accept EULA set to True in customer config") + return True + else: + print("Accept EULA not found to be set to True in customer config") + return False + except Exception as error: + print("Error occurred while reading and parsing EULA settings. Not accepting EULA for any patch. Error=[{0}]".format(repr(error))) + return False + diff --git a/src/core/src/core_logic/SystemctlManager.py b/src/core/src/core_logic/SystemctlManager.py index 5d10d3eb..73e125d5 100644 --- a/src/core/src/core_logic/SystemctlManager.py +++ b/src/core/src/core_logic/SystemctlManager.py @@ -31,7 +31,7 @@ def __init__(self, env_layer, execution_config, composite_logger, telemetry_writ self.service_desc = service_info.service_desc self.service_exec_path = service_info.service_exec_path - self.__systemd_path = Constants.Paths.SYSTEMD_ROOT + self.__systemd_path = Constants.SystemPaths.SYSTEMD_ROOT self.systemctl_daemon_reload_cmd = "sudo systemctl daemon-reload" self.systemctl_version = "systemctl --version" diff --git a/src/core/tests/Test_ConfigurationFactory.py b/src/core/tests/Test_ConfigurationFactory.py index 964529cb..8a9029ad 100644 --- a/src/core/tests/Test_ConfigurationFactory.py +++ b/src/core/tests/Test_ConfigurationFactory.py @@ -64,9 +64,7 @@ def test_get_dev_config_correctly(self): self.assertEqual(config['config_env'], Constants.DEV) def test_eula_acceptance_file_read_success(self): - bootstrapper = Bootstrapper(self.argument_composer, capture_stdout=False) - config_factory = bootstrapper.configuration_factory - self.assertTrue(config_factory) + self.runtime.stop() # Accept EULA set to true eula_settings = { @@ -74,54 +72,67 @@ def test_eula_acceptance_file_read_success(self): "AcceptedBy": "TestSetup", "LastModified": "2023-08-29" } - f = open(Constants.Paths.EULA_SETTINGS, "w+") + f = open(Constants.AzGPSPaths.EULA_SETTINGS, "w+") f.write(json.dumps(eula_settings)) f.close() - config = config_factory.get_arguments_configuration(self.argument_composer) - self.assertEqual(config['execution_config']['component_kwargs']['accept_package_eula'], True) + runtime = RuntimeCompositor(self.argument_composer, True, package_manager_name=Constants.APT) + container = runtime.container + execution_config = container.get('execution_config') + self.assertEqual(execution_config.accept_package_eula, True) + runtime.stop() - # Accept EULA set to true + # Accept EULA set to false eula_settings = { "AcceptEULAForAllPatches": False, "AcceptedBy": "TestSetup", "LastModified": "2023-08-29" } - f = open(Constants.Paths.EULA_SETTINGS, "w+") + f = open(Constants.AzGPSPaths.EULA_SETTINGS, "w+") f.write(json.dumps(eula_settings)) f.close() - config = config_factory.get_arguments_configuration(self.argument_composer) - self.assertEqual(config['execution_config']['component_kwargs']['accept_package_eula'], False) + runtime = RuntimeCompositor(self.argument_composer, True, package_manager_name=Constants.APT) + container = runtime.container + execution_config = container.get('execution_config') + self.assertEqual(execution_config.accept_package_eula, False) + runtime.stop() def test_eula_acceptance_file_read_when_no_data_found(self): - bootstrapper = Bootstrapper(self.argument_composer, capture_stdout=False) - config_factory = bootstrapper.configuration_factory - self.assertTrue(config_factory) + self.runtime.stop() # EULA file does not exist - config = config_factory.get_arguments_configuration(self.argument_composer) - self.assertEqual(config['execution_config']['component_kwargs']['accept_package_eula'], False) - self.assertFalse(os.path.exists(Constants.Paths.EULA_SETTINGS)) + runtime = RuntimeCompositor(self.argument_composer, True, package_manager_name=Constants.APT) + container = runtime.container + execution_config = container.get('execution_config') + self.assertEqual(execution_config.accept_package_eula, False) + self.assertFalse(os.path.exists(Constants.AzGPSPaths.EULA_SETTINGS)) + runtime.stop() # EULA settings set to None eula_settings = None - f = open(Constants.Paths.EULA_SETTINGS, "w+") + f = open(Constants.AzGPSPaths.EULA_SETTINGS, "w+") f.write(json.dumps(eula_settings)) f.close() - config = config_factory.get_arguments_configuration(self.argument_composer) - self.assertEqual(config['execution_config']['component_kwargs']['accept_package_eula'], False) - self.assertTrue(os.path.exists(Constants.Paths.EULA_SETTINGS)) + runtime = RuntimeCompositor(self.argument_composer, True, package_manager_name=Constants.APT) + container = runtime.container + execution_config = container.get('execution_config') + self.assertEqual(execution_config.accept_package_eula, False) + self.assertTrue(os.path.exists(Constants.AzGPSPaths.EULA_SETTINGS)) + runtime.stop() # AcceptEULAForAllPatches not set in config eula_settings = { "AcceptedBy": "TestSetup", "LastModified": "2023-08-29" } - f = open(Constants.Paths.EULA_SETTINGS, "w+") + f = open(Constants.AzGPSPaths.EULA_SETTINGS, "w+") f.write(json.dumps(eula_settings)) f.close() - config = config_factory.get_arguments_configuration(self.argument_composer) - self.assertEqual(config['execution_config']['component_kwargs']['accept_package_eula'], False) - self.assertTrue(os.path.exists(Constants.Paths.EULA_SETTINGS)) + runtime = RuntimeCompositor(self.argument_composer, True, package_manager_name=Constants.APT) + container = runtime.container + execution_config = container.get('execution_config') + self.assertEqual(execution_config.accept_package_eula, False) + self.assertTrue(os.path.exists(Constants.AzGPSPaths.EULA_SETTINGS)) + runtime.stop() # AcceptEULAForAllPatches not set to a boolean eula_settings = { @@ -129,17 +140,23 @@ def test_eula_acceptance_file_read_when_no_data_found(self): "AcceptedBy": "TestSetup", "LastModified": "2023-08-29" } - f = open(Constants.Paths.EULA_SETTINGS, "w+") + f = open(Constants.AzGPSPaths.EULA_SETTINGS, "w+") f.write(json.dumps(eula_settings)) f.close() - config = config_factory.get_arguments_configuration(self.argument_composer) - self.assertEqual(config['execution_config']['component_kwargs']['accept_package_eula'], False) - self.assertTrue(os.path.exists(Constants.Paths.EULA_SETTINGS)) - - self.backup_read_with_retry = config_factory.read_with_retry - config_factory.read_with_retry = self.mock_read_with_retry_raise_exception - self.assertTrue(os.path.exists(Constants.Paths.EULA_SETTINGS)) - self.assertRaises(Exception, config_factory.get_arguments_configuration(self.argument_composer)) + runtime = RuntimeCompositor(self.argument_composer, True, package_manager_name=Constants.APT) + container = runtime.container + execution_config = container.get('execution_config') + self.assertEqual(execution_config.accept_package_eula, False) + self.assertTrue(os.path.exists(Constants.AzGPSPaths.EULA_SETTINGS)) + runtime.stop() + + runtime = RuntimeCompositor(self.argument_composer, True, package_manager_name=Constants.APT) + container = runtime.container + self.backup_read_with_retry = runtime.env_layer.file_system.read_with_retry + runtime.env_layer.file_system.read_with_retry = self.mock_read_with_retry_raise_exception + self.assertTrue(os.path.exists(Constants.AzGPSPaths.EULA_SETTINGS)) + self.assertRaises(Exception, container.get('execution_config')) + runtime.stop() if __name__ == '__main__': diff --git a/src/core/tests/library/ArgumentComposer.py b/src/core/tests/library/ArgumentComposer.py index 03af4ecd..550f225b 100644 --- a/src/core/tests/library/ArgumentComposer.py +++ b/src/core/tests/library/ArgumentComposer.py @@ -47,7 +47,7 @@ def __init__(self): self.__status_folder = self.__get_custom_folder(scratch_folder, self.__STATUS_FOLDER) self.events_folder = self.__get_custom_folder(self.__log_folder, self.__EVENTS_FOLDER) self.temp_folder = self.__get_custom_folder(scratch_folder, self.__TEMP_FOLDER) - Constants.Paths.EULA_SETTINGS = os.path.join(scratch_folder, "patch.eula.settings") + Constants.AzGPSPaths.EULA_SETTINGS = os.path.join(scratch_folder, "patch.eula.settings") # config settings self.operation = Constants.INSTALLATION diff --git a/src/core/tests/library/RuntimeCompositor.py b/src/core/tests/library/RuntimeCompositor.py index f6b7aa66..dd27ab33 100644 --- a/src/core/tests/library/RuntimeCompositor.py +++ b/src/core/tests/library/RuntimeCompositor.py @@ -47,7 +47,7 @@ def __init__(self, argv=Constants.DEFAULT_UNSPECIFIED_VALUE, legacy_mode=False, os.environ[Constants.LPE_ENV_VARIABLE] = self.current_env self.argv = argv if argv != Constants.DEFAULT_UNSPECIFIED_VALUE else ArgumentComposer().get_composed_arguments() self.vm_cloud_type = vm_cloud_type - Constants.Paths.SYSTEMD_ROOT = os.getcwd() # mocking to pass a basic systemd check in Windows + Constants.SystemPaths.SYSTEMD_ROOT = os.getcwd() # mocking to pass a basic systemd check in Windows self.is_github_runner = os.getenv('RUNNER_TEMP', None) is not None if self.is_github_runner: