From 2cdde907c8ff4d56c1b38870911fd5b6e2cdfc7f Mon Sep 17 00:00:00 2001 From: Koshy John Date: Wed, 12 Mar 2025 17:00:44 -0700 Subject: [PATCH 1/7] Apt - fix for non-sec updates getting misclassified --- src/core/src/package_managers/AptitudePackageManager.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/core/src/package_managers/AptitudePackageManager.py b/src/core/src/package_managers/AptitudePackageManager.py index 98c39b11..97dc7173 100644 --- a/src/core/src/package_managers/AptitudePackageManager.py +++ b/src/core/src/package_managers/AptitudePackageManager.py @@ -321,7 +321,7 @@ def get_all_updates(self, cached=False): return all_updates, all_updates_versions # when cached is False, query both default way and using Ubuntu Pro Client. - source_parts, source_list = self.__get_custom_sources_to_spec(self.max_patch_publish_date, base_classification=str()) + source_parts, source_list = self.__get_custom_sources_to_spec(self.max_patch_publish_date, base_classification=Constants.PackageClassification.SECURITY) cmd = self.__generate_command_with_custom_sources(command_template=self.cmd_dist_upgrade_simulation_template, source_parts=source_parts, source_list=source_list) out = self.invoke_package_manager(cmd) self.all_updates_cached, self.all_update_versions_cached = self.extract_packages_and_versions(out) @@ -466,7 +466,7 @@ def install_updates_fail_safe(self, excluded_packages): return def install_security_updates_azgps_coordinated(self): - source_parts, source_list = self.__get_custom_sources_to_spec(self.max_patch_publish_date, base_classification=str()) + source_parts, source_list = self.__get_custom_sources_to_spec(self.max_patch_publish_date, base_classification=Constants.PackageClassification.SECURITY) command = self.__generate_command_with_custom_sources(self.install_security_updates_azgps_coordinated_cmd, source_parts=source_parts, source_list=source_list) out, code = self.invoke_package_manager_advanced(command, raise_on_exception=False) return code, out From 0122f2703ac336e7a0079e7d161c04e0c49a515c Mon Sep 17 00:00:00 2001 From: Koshy John Date: Wed, 12 Mar 2025 20:14:38 -0700 Subject: [PATCH 2/7] Ensured additional temp cleanup --- src/core/src/CoreMain.py | 6 ++--- src/core/src/bootstrap/Constants.py | 2 +- src/core/src/bootstrap/EnvLayer.py | 21 +++++++++--------- .../AptitudePackageManager.py | 6 ++--- ...est_AptitudePackageManagerCustomSources.py | 20 +++++++++++++++++ src/core/tests/Test_CoreMain.py | 22 +++++++++++++------ src/core/tests/Test_StatusHandler.py | 8 +++---- src/extension/src/EnvLayer.py | 19 ++++++++++------ .../src/file_handlers/ExtEnvHandler.py | 2 +- 9 files changed, 70 insertions(+), 36 deletions(-) diff --git a/src/core/src/CoreMain.py b/src/core/src/CoreMain.py index a98160c7..e2e4c3d6 100644 --- a/src/core/src/CoreMain.py +++ b/src/core/src/CoreMain.py @@ -65,7 +65,7 @@ def __init__(self, argv): if os.path.exists(execution_config.temp_folder): composite_logger.log_debug("Deleting all files of certain format from temp folder [FileFormat={0}][TempFolderLocation={1}]" .format(Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST, str(execution_config.temp_folder))) - bootstrapper.env_layer.file_system.delete_files_from_dir(execution_config.temp_folder, Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST) + bootstrapper.env_layer.file_system.delete_from_dir(execution_config.temp_folder, Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST) patch_assessor = container.get('patch_assessor') package_manager = container.get('package_manager') @@ -139,9 +139,9 @@ def __init__(self, argv): # clean up temp folder of files created by Core after execution completes if self.is_temp_folder_available(bootstrapper.env_layer, execution_config): - composite_logger.log_debug("Deleting all files of certain format from temp folder [FileFormat={0}][TempFolderLocation={1}]" + composite_logger.log_debug("Deleting format-matching items from temp folder [FormatList={0}][TempFolderLocation={1}]" .format(Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST, str(execution_config.temp_folder))) - bootstrapper.env_layer.file_system.delete_files_from_dir(execution_config.temp_folder, Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST) + bootstrapper.env_layer.file_system.delete_from_dir(execution_config.temp_folder, Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST) if lifecycle_manager is not None: lifecycle_manager.update_core_sequence(completed=True) diff --git a/src/core/src/bootstrap/Constants.py b/src/core/src/bootstrap/Constants.py index b98fe1c6..e7e71d69 100644 --- a/src/core/src/bootstrap/Constants.py +++ b/src/core/src/bootstrap/Constants.py @@ -87,7 +87,7 @@ class EulaSettings(EnumBackport): LAST_MODIFIED = 'LastModified' TEMP_FOLDER_DIR_NAME = "tmp" - TEMP_FOLDER_CLEANUP_ARTIFACT_LIST = ["*.list"] + TEMP_FOLDER_CLEANUP_ARTIFACT_LIST = ["*.list", "azgps*"] # File to save default settings for auto OS updates IMAGE_DEFAULT_PATCH_CONFIGURATION_BACKUP_PATH = "ImageDefaultPatchConfiguration.bak" diff --git a/src/core/src/bootstrap/EnvLayer.py b/src/core/src/bootstrap/EnvLayer.py index ab2e1f6a..7686195e 100644 --- a/src/core/src/bootstrap/EnvLayer.py +++ b/src/core/src/bootstrap/EnvLayer.py @@ -430,20 +430,21 @@ def write_with_retry_using_temp_file(file_path, data, mode='w'): raise Exception("Unable to write to {0} (retries exhausted). Error: {1}.".format(str(file_path), repr(error))) @staticmethod - def delete_files_from_dir(dir_name, file_identifier_list, raise_if_delete_failed=False): - """ Clears all files from given dir. NOTE: Uses file_identifier_list to determine the content to delete """ - for file_identifier in file_identifier_list: - files_to_delete = glob.glob(str(dir_name) + "/" + str(file_identifier)) + def delete_from_dir(dir_name, identifier_list, raise_if_delete_failed=False, include_subdirs=True): + """ Clears all files/dirs from given dir. NOTE: Uses identifier_list to determine the content to delete """ + for identifier in identifier_list: + items_to_delete = glob.glob(str(dir_name) + "/" + str(identifier)) - for file_to_delete in files_to_delete: + for item_to_delete in items_to_delete: try: - os.remove(file_to_delete) + if os.path.isdir(item_to_delete): + if not include_subdirs: + continue + shutil.rmtree(item_to_delete) + os.remove(item_to_delete) except Exception as error: error_message = "Unable to delete files from directory [Dir={0}][File={1}][Error={2}][RaiseIfDeleteFailed={3}].".format( - str(dir_name), - str(file_to_delete), - repr(error), - str(raise_if_delete_failed)) + str(dir_name), str(item_to_delete), repr(error), str(raise_if_delete_failed)) if raise_if_delete_failed: raise Exception(error_message) diff --git a/src/core/src/package_managers/AptitudePackageManager.py b/src/core/src/package_managers/AptitudePackageManager.py index 97dc7173..78aee06b 100644 --- a/src/core/src/package_managers/AptitudePackageManager.py +++ b/src/core/src/package_managers/AptitudePackageManager.py @@ -266,7 +266,7 @@ def invoke_package_manager_advanced(self, command, raise_on_exception=True): if code != self.apt_exitcode_ok and self.STR_DPKG_WAS_INTERRUPTED in out: self.composite_logger.log_error('[ERROR] YOU NEED TO TAKE ACTION TO PROCEED. The package manager on this machine is not in a healthy state, and ' - 'Patch Management cannot proceed successfully. Before the next Pa-oDir::Etc::SourceParts=tch Operation, please run the following ' + 'Patch Management cannot proceed successfully. Before the next Patch Operation, please run the following ' 'command and perform any configuration steps necessary on the machine to return it to a healthy state: ' 'sudo dpkg --configure -a') self.telemetry_writer.write_execution_error(command, code, out) @@ -321,7 +321,7 @@ def get_all_updates(self, cached=False): return all_updates, all_updates_versions # when cached is False, query both default way and using Ubuntu Pro Client. - source_parts, source_list = self.__get_custom_sources_to_spec(self.max_patch_publish_date, base_classification=Constants.PackageClassification.SECURITY) + source_parts, source_list = self.__get_custom_sources_to_spec(self.max_patch_publish_date, base_classification=str()) cmd = self.__generate_command_with_custom_sources(command_template=self.cmd_dist_upgrade_simulation_template, source_parts=source_parts, source_list=source_list) out = self.invoke_package_manager(cmd) self.all_updates_cached, self.all_update_versions_cached = self.extract_packages_and_versions(out) @@ -349,7 +349,7 @@ def get_security_updates(self): ubuntu_pro_client_security_package_versions = [] self.composite_logger.log_verbose("[APM] Discovering 'security' packages...") - source_parts, source_list = self.__get_custom_sources_to_spec(self.max_patch_publish_date, base_classification=str()) + source_parts, source_list = self.__get_custom_sources_to_spec(self.max_patch_publish_date, base_classification=Constants.PackageClassification.SECURITY) cmd = self.__generate_command_with_custom_sources(self.cmd_dist_upgrade_simulation_template, source_parts=source_parts, source_list=source_list) out = self.invoke_package_manager(cmd) security_packages, security_package_versions = self.extract_packages_and_versions(out) diff --git a/src/core/tests/Test_AptitudePackageManagerCustomSources.py b/src/core/tests/Test_AptitudePackageManagerCustomSources.py index e8b1c300..afb3873b 100644 --- a/src/core/tests/Test_AptitudePackageManagerCustomSources.py +++ b/src/core/tests/Test_AptitudePackageManagerCustomSources.py @@ -33,6 +33,26 @@ def setUp(self): def tearDown(self): self.runtime.stop() + def test_custom_source_invocation_for_security(self): + def get_custom_sources_to_spec_for_sec(max_patch_published_date=str(), base_classification=str()): + assert max_patch_published_date is str() + assert base_classification == Constants.PackageClassification.SECURITY + return str(), str() + + def get_custom_sources_to_spec_for_sec_date(max_patch_published_date=str(), base_classification=str()): + assert max_patch_published_date == "2025-03-12T00:00:00Z" + assert base_classification == Constants.PackageClassification.SECURITY + return str(), str() + + package_manager = AptitudePackageManager.AptitudePackageManager(self.runtime.env_layer, self.runtime.execution_config, self.runtime.composite_logger, self.runtime.telemetry_writer, self.runtime.status_handler) + package_manager._AptitudePackageManager__get_custom_sources_to_spec = get_custom_sources_to_spec_for_sec + package_manager.get_security_updates() + package_manager.install_security_updates_azgps_coordinated() + + package_manager._AptitudePackageManager__get_custom_sources_to_spec = get_custom_sources_to_spec_for_sec_date + package_manager.set_max_patch_publish_date("2025-03-12T00:00:00Z") + package_manager.install_security_updates_azgps_coordinated() + def test_bad_custom_sources_to_spec_invocation(self): package_manager = AptitudePackageManager.AptitudePackageManager(self.runtime.env_layer, self.runtime.execution_config, self.runtime.composite_logger, self.runtime.telemetry_writer, self.runtime.status_handler) sources_dir, sources_list = package_manager._AptitudePackageManager__get_custom_sources_to_spec(base_classification="other") # invalid call diff --git a/src/core/tests/Test_CoreMain.py b/src/core/tests/Test_CoreMain.py index 06d14965..1d6b1123 100644 --- a/src/core/tests/Test_CoreMain.py +++ b/src/core/tests/Test_CoreMain.py @@ -1164,6 +1164,12 @@ def test_delete_temp_folder_contents_success(self): self.assertTrue(argument_composer.temp_folder is not None) self.assertEqual(argument_composer.temp_folder, os.path.abspath(os.path.join(os.path.curdir, "scratch", "tmp"))) + # add some more content + os.mkdir(os.path.join(argument_composer.temp_folder, "azgps-src-123.d")) + for identifier in Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST: + files_matched = glob.glob(str(argument_composer.temp_folder) + "/" + str(identifier)) + self.assertTrue(len(files_matched) == (1 if "azgps" in identifier else 0)) + # delete temp content argument_composer.operation = Constants.ASSESSMENT runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.APT) @@ -1172,8 +1178,9 @@ def test_delete_temp_folder_contents_success(self): # validate files are deleted self.assertTrue(argument_composer.temp_folder is not None) - files_matched = glob.glob(str(argument_composer.temp_folder) + "/" + str(Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST)) - self.assertTrue(len(files_matched) == 0) + for identifier in Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST: + files_matched = glob.glob(str(argument_composer.temp_folder) + "/" + str(identifier)) + self.assertTrue(len(files_matched) == 0) runtime.stop() def test_delete_temp_folder_contents_when_none_exists(self): @@ -1183,12 +1190,13 @@ def test_delete_temp_folder_contents_when_none_exists(self): shutil.rmtree(runtime.execution_config.temp_folder) # attempt to delete temp content - runtime.env_layer.file_system.delete_files_from_dir(runtime.execution_config.temp_folder, Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST) + runtime.env_layer.file_system.delete_from_dir(runtime.execution_config.temp_folder, Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST) # validate files are deleted self.assertTrue(runtime.execution_config.temp_folder is not None) - files_matched = glob.glob(str(runtime.execution_config.temp_folder) + "/" + str(Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST)) - self.assertTrue(len(files_matched) == 0) + for identifier in Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST: + files_matched = glob.glob(str(argument_composer.temp_folder) + "/" + str(identifier)) + self.assertTrue(len(files_matched) == 0) runtime.stop() def test_delete_temp_folder_contents_failure(self): @@ -1204,11 +1212,11 @@ def test_delete_temp_folder_contents_failure(self): runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.APT) # delete temp content attempt #1, throws exception - self.assertRaises(Exception, lambda: runtime.env_layer.file_system.delete_files_from_dir(runtime.execution_config.temp_folder, Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST, raise_if_delete_failed=True)) + self.assertRaises(Exception, lambda: runtime.env_layer.file_system.delete_from_dir(runtime.execution_config.temp_folder, Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST, raise_if_delete_failed=True)) self.assertTrue(os.path.isfile(os.path.join(runtime.execution_config.temp_folder, "temp1.list"))) # delete temp content attempt #2, does not throws exception - runtime.env_layer.file_system.delete_files_from_dir(runtime.execution_config.temp_folder, Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST) + runtime.env_layer.file_system.delete_from_dir(runtime.execution_config.temp_folder, Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST) self.assertTrue(os.path.isfile(os.path.join(runtime.execution_config.temp_folder, "temp1.list"))) # reset os.remove() mock diff --git a/src/core/tests/Test_StatusHandler.py b/src/core/tests/Test_StatusHandler.py index 4d51b83d..bca4e2d5 100644 --- a/src/core/tests/Test_StatusHandler.py +++ b/src/core/tests/Test_StatusHandler.py @@ -440,7 +440,7 @@ def test_if_status_file_resets_on_load_if_malformed(self): self.assertEqual(substatus_file_data["status"]["operation"], "Installation") self.assertIsNotNone(substatus_file_data["status"]["substatus"]) self.assertEqual(len(substatus_file_data["status"]["substatus"]), 0) - self.runtime.env_layer.file_system.delete_files_from_dir(example_file1, "*.complete.status") + self.runtime.env_layer.file_system.delete_from_dir(example_file1, "*.complete.status") def test_if_complete_and_status_path_is_dir(self): self.old_complete_status_path = self.runtime.execution_config.complete_status_file_path @@ -554,7 +554,7 @@ def test_load_status_and_set_package_install_status(self): self.assertEqual('python-samba0_2:4.4.5+dfsg-2ubuntu5.4_Ubuntu_16.04', str(json.loads(substatus_file_data["formattedMessage"]["message"])["patches"][0]["patchId"])) self.assertTrue('Critical' in str(json.loads(substatus_file_data["formattedMessage"]["message"])["patches"][2]["classifications"])) - self.runtime.env_layer.file_system.delete_files_from_dir(self.runtime.status_handler.status_file_path, '*.complete.status') + self.runtime.env_layer.file_system.delete_from_dir(self.runtime.status_handler.status_file_path, '*.complete.status') def test_remove_old_complete_status_files(self): """ Create dummy files in status folder and check if the complete_status_file_path is the latest file and delete those dummy files """ @@ -571,7 +571,7 @@ def test_remove_old_complete_status_files(self): count_status_files = glob.glob(os.path.join(file_path, '*.complete.status')) self.assertEqual(10, len(count_status_files)) self.assertTrue(os.path.isfile(self.runtime.execution_config.complete_status_file_path)) - self.runtime.env_layer.file_system.delete_files_from_dir(file_path, '*.complete.status') + self.runtime.env_layer.file_system.delete_from_dir(file_path, '*.complete.status') self.assertFalse(os.path.isfile(os.path.join(file_path, '1.complete_status'))) def test_remove_old_complete_status_files_throws_exception(self): @@ -586,7 +586,7 @@ def test_remove_old_complete_status_files_throws_exception(self): # reset os.remove() mock and remove *complete.status files os.remove = self.backup_os_remove - self.runtime.env_layer.file_system.delete_files_from_dir(file_path, '*.complete.status') + self.runtime.env_layer.file_system.delete_from_dir(file_path, '*.complete.status') self.assertFalse(os.path.isfile(os.path.join(file_path, '1.complete_status'))) def __assert_sequence_num_changed_termination(self, config, summary, status): diff --git a/src/extension/src/EnvLayer.py b/src/extension/src/EnvLayer.py index bcae0e2b..94564fcf 100644 --- a/src/extension/src/EnvLayer.py +++ b/src/extension/src/EnvLayer.py @@ -247,17 +247,22 @@ def write_with_retry(self, file_path_or_handle, data, mode='a+'): file_handle.close() @staticmethod - def delete_files_from_dir(dir_name, file_identifier_list, raise_if_delete_failed=False): - """ Clears all files from given dir. NOTE: Uses file_identifier_list to determine the content to delete """ - for file_identifier in file_identifier_list: - files_to_delete = glob.glob(str(dir_name) + "/" + str(file_identifier)) + def delete_from_dir(dir_name, identifier_list, raise_if_delete_failed=False, include_subdirs=True): + """ Clears all files/dirs from given dir. NOTE: Uses identifier_list to determine the content to delete """ + for identifier in identifier_list: + items_to_delete = glob.glob(str(dir_name) + "/" + str(identifier)) - for file_to_delete in files_to_delete: + for item_to_delete in items_to_delete: try: - os.remove(file_to_delete) + if os.path.isdir(item_to_delete): + if not include_subdirs: + continue + shutil.rmtree(item_to_delete) + os.remove(item_to_delete) except Exception as error: error_message = "Unable to delete files from directory [Dir={0}][File={1}][Error={2}][RaiseIfDeleteFailed={3}].".format( - str(dir_name), str(file_to_delete), repr(error), str(raise_if_delete_failed)) + str(dir_name), str(item_to_delete), repr(error), str(raise_if_delete_failed)) + if raise_if_delete_failed: raise Exception(error_message) else: diff --git a/src/extension/src/file_handlers/ExtEnvHandler.py b/src/extension/src/file_handlers/ExtEnvHandler.py index 9cbec33c..b3d64a5a 100644 --- a/src/extension/src/file_handlers/ExtEnvHandler.py +++ b/src/extension/src/file_handlers/ExtEnvHandler.py @@ -89,7 +89,7 @@ def delete_temp_folder_contents(self, raise_if_delete_failed=False): and self.temp_folder is not None \ and os.path.exists(self.temp_folder): self.logger.log_debug("Deleting all files of certain format from temp folder [FileFormat={0}][TempFolderLocation={1}]".format("*", str(self.temp_folder))) - self.env_layer.file_system.delete_files_from_dir(self.temp_folder, ["*"], raise_if_delete_failed=raise_if_delete_failed) + self.env_layer.file_system.delete_from_dir(self.temp_folder, ["*"], raise_if_delete_failed=raise_if_delete_failed) else: self.logger.log_debug("Temp folder not found") From d8729454a4620c9e5cdf0652aa2d1711543cb285 Mon Sep 17 00:00:00 2001 From: Koshy John Date: Wed, 12 Mar 2025 21:30:56 -0700 Subject: [PATCH 3/7] Additional fixes --- src/core/src/bootstrap/EnvLayer.py | 9 +++++---- src/extension/src/EnvLayer.py | 9 +++++---- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/core/src/bootstrap/EnvLayer.py b/src/core/src/bootstrap/EnvLayer.py index 7686195e..c15938b7 100644 --- a/src/core/src/bootstrap/EnvLayer.py +++ b/src/core/src/bootstrap/EnvLayer.py @@ -433,7 +433,7 @@ def write_with_retry_using_temp_file(file_path, data, mode='w'): def delete_from_dir(dir_name, identifier_list, raise_if_delete_failed=False, include_subdirs=True): """ Clears all files/dirs from given dir. NOTE: Uses identifier_list to determine the content to delete """ for identifier in identifier_list: - items_to_delete = glob.glob(str(dir_name) + "/" + str(identifier)) + items_to_delete = glob.glob(os.path.join(str(dir_name), str(identifier))) for item_to_delete in items_to_delete: try: @@ -441,16 +441,17 @@ def delete_from_dir(dir_name, identifier_list, raise_if_delete_failed=False, inc if not include_subdirs: continue shutil.rmtree(item_to_delete) - os.remove(item_to_delete) + else: + os.remove(item_to_delete) except Exception as error: - error_message = "Unable to delete files from directory [Dir={0}][File={1}][Error={2}][RaiseIfDeleteFailed={3}].".format( + error_message = "Unable to delete item from directory [Dir={0}][Item={1}][Error={2}][RaiseIfDeleteFailed={3}].".format( str(dir_name), str(item_to_delete), repr(error), str(raise_if_delete_failed)) if raise_if_delete_failed: raise Exception(error_message) else: print(error_message) - return None + continue # endregion - File system emulation and extensions # region - DateTime emulation and extensions diff --git a/src/extension/src/EnvLayer.py b/src/extension/src/EnvLayer.py index 94564fcf..4c1ba7b1 100644 --- a/src/extension/src/EnvLayer.py +++ b/src/extension/src/EnvLayer.py @@ -250,7 +250,7 @@ def write_with_retry(self, file_path_or_handle, data, mode='a+'): def delete_from_dir(dir_name, identifier_list, raise_if_delete_failed=False, include_subdirs=True): """ Clears all files/dirs from given dir. NOTE: Uses identifier_list to determine the content to delete """ for identifier in identifier_list: - items_to_delete = glob.glob(str(dir_name) + "/" + str(identifier)) + items_to_delete = glob.glob(os.path.join(str(dir_name), str(identifier))) for item_to_delete in items_to_delete: try: @@ -258,16 +258,17 @@ def delete_from_dir(dir_name, identifier_list, raise_if_delete_failed=False, inc if not include_subdirs: continue shutil.rmtree(item_to_delete) - os.remove(item_to_delete) + else: + os.remove(item_to_delete) except Exception as error: - error_message = "Unable to delete files from directory [Dir={0}][File={1}][Error={2}][RaiseIfDeleteFailed={3}].".format( + error_message = "Unable to delete item from directory [Dir={0}][Item={1}][Error={2}][RaiseIfDeleteFailed={3}].".format( str(dir_name), str(item_to_delete), repr(error), str(raise_if_delete_failed)) if raise_if_delete_failed: raise Exception(error_message) else: print(error_message) - return None + continue @staticmethod def remove_dir(dir_name, raise_if_delete_failed=False): From 99d5c74f8be8b6464d95dc63153fcb5b0fce292b Mon Sep 17 00:00:00 2001 From: Koshy John Date: Wed, 12 Mar 2025 21:34:18 -0700 Subject: [PATCH 4/7] Removing currently unnecessary code --- src/core/src/bootstrap/EnvLayer.py | 4 +--- src/extension/src/EnvLayer.py | 4 +--- 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/core/src/bootstrap/EnvLayer.py b/src/core/src/bootstrap/EnvLayer.py index c15938b7..6de1bd77 100644 --- a/src/core/src/bootstrap/EnvLayer.py +++ b/src/core/src/bootstrap/EnvLayer.py @@ -430,7 +430,7 @@ def write_with_retry_using_temp_file(file_path, data, mode='w'): raise Exception("Unable to write to {0} (retries exhausted). Error: {1}.".format(str(file_path), repr(error))) @staticmethod - def delete_from_dir(dir_name, identifier_list, raise_if_delete_failed=False, include_subdirs=True): + def delete_from_dir(dir_name, identifier_list, raise_if_delete_failed=False): """ Clears all files/dirs from given dir. NOTE: Uses identifier_list to determine the content to delete """ for identifier in identifier_list: items_to_delete = glob.glob(os.path.join(str(dir_name), str(identifier))) @@ -438,8 +438,6 @@ def delete_from_dir(dir_name, identifier_list, raise_if_delete_failed=False, inc for item_to_delete in items_to_delete: try: if os.path.isdir(item_to_delete): - if not include_subdirs: - continue shutil.rmtree(item_to_delete) else: os.remove(item_to_delete) diff --git a/src/extension/src/EnvLayer.py b/src/extension/src/EnvLayer.py index 4c1ba7b1..e4db7d3c 100644 --- a/src/extension/src/EnvLayer.py +++ b/src/extension/src/EnvLayer.py @@ -247,7 +247,7 @@ def write_with_retry(self, file_path_or_handle, data, mode='a+'): file_handle.close() @staticmethod - def delete_from_dir(dir_name, identifier_list, raise_if_delete_failed=False, include_subdirs=True): + def delete_from_dir(dir_name, identifier_list, raise_if_delete_failed=False): """ Clears all files/dirs from given dir. NOTE: Uses identifier_list to determine the content to delete """ for identifier in identifier_list: items_to_delete = glob.glob(os.path.join(str(dir_name), str(identifier))) @@ -255,8 +255,6 @@ def delete_from_dir(dir_name, identifier_list, raise_if_delete_failed=False, inc for item_to_delete in items_to_delete: try: if os.path.isdir(item_to_delete): - if not include_subdirs: - continue shutil.rmtree(item_to_delete) else: os.remove(item_to_delete) From 774e67f3a1d09891cdbc8591df3e1b31ff5f949a Mon Sep 17 00:00:00 2001 From: Koshy John Date: Thu, 13 Mar 2025 09:06:59 -0700 Subject: [PATCH 5/7] Minor code cleanup --- src/core/tests/Test_CoreMain.py | 26 +++++++++---------- .../src/file_handlers/ExtEnvHandler.py | 7 +++-- 2 files changed, 16 insertions(+), 17 deletions(-) diff --git a/src/core/tests/Test_CoreMain.py b/src/core/tests/Test_CoreMain.py index 1d6b1123..326dcf30 100644 --- a/src/core/tests/Test_CoreMain.py +++ b/src/core/tests/Test_CoreMain.py @@ -401,7 +401,7 @@ def test_installation_operation_fail_on_arc_due_to_no_telemetry(self): argument_composer = ArgumentComposer() argument_composer.maintenance_run_id = str(datetime.datetime.utcnow().strftime("%Y-%m-%dT%H:%M:%S.%fZ")) argument_composer.events_folder = None - runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.ZYPPER,Constants.VMCloudType.ARC) + runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.ZYPPER, Constants.VMCloudType.ARC) runtime.set_legacy_test_type('SuccessInstallPath') CoreMain(argument_composer.get_composed_arguments()) @@ -701,7 +701,7 @@ def test_auto_assessment_success_with_configure_patching_in_prev_operation_on_sa def test_auto_assessment_success_on_arc_with_configure_patching_in_prev_operation_on_same_sequence(self): """Unit test for auto assessment request with configure patching completed on the sequence before. Result: should retain prev substatus and update only PatchAssessmentSummary""" # operation #1: ConfigurePatching - # Here it should skip agent compatibility check as operation is configure patching [ not assessment or installation] + # Here it should skip agent compatibility check as operation is ConfigurePatching [ not assessment or installation] argument_composer = ArgumentComposer() argument_composer.operation = Constants.CONFIGURE_PATCHING argument_composer.patch_mode = Constants.PatchModes.AUTOMATIC_BY_PLATFORM @@ -1060,7 +1060,7 @@ def test_assessment_operation_success_after_package_manager_reboot(self): runtime.stop() def test_assessment_superseded(self): - """Unit test for an assessment request that gets superseded by a newer operation.. + """Unit test for an assessment request that gets superseded by a newer operation. Result: Assessment should terminate with a superseded error message.""" # Step 1: Run assessment normally to generate 0.status and ExtState.json argument_composer = ArgumentComposer() @@ -1095,14 +1095,14 @@ def test_assessment_superseded(self): # Step 3: Update sequence number in ExtState.json to mock a new incoming request runtime.write_ext_state_file(runtime.lifecycle_manager.ext_state_file_path, "2", - datetime.datetime.utcnow().strftime("%Y-%m-%dT%H:%M:%S.%fZ"), - runtime.execution_config.operation) + datetime.datetime.utcnow().strftime("%Y-%m-%dT%H:%M:%S.%fZ"), + runtime.execution_config.operation) raised_exit_exception = False # Step 4: Run Assessment again with sequence number 1 to mock an older request that should automatically terminate and report operation superseded try: CoreMain(argument_composer.get_composed_arguments()) - except SystemExit as error: + except SystemExit: # Should raise a SystemExit exception raised_exit_exception = True @@ -1147,7 +1147,7 @@ def test_temp_folder_created_during_execution_config_init(self): # temp_folder is set to None in ExecutionConfig with an invalid config_folder location, throws exception argument_composer = ArgumentComposer() - shutil.rmtree(argument_composer.temp_folder) + shutil.rmtree(str(argument_composer.temp_folder)) argument_composer.temp_folder = None argument_composer.operation = Constants.ASSESSMENT # mock path exists check to return False on config_folder exists check @@ -1155,8 +1155,8 @@ def test_temp_folder_created_during_execution_config_init(self): os.path.exists = self.mock_os_path_exists self.assertRaises(Exception, lambda: RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.APT)) # validate temp_folder is not created - self.assertFalse(os.path.exists(os.path.join(os.path.curdir, "scratch", "tmp"))) os.path.exists = backup_os_path_exists + self.assertFalse(os.path.exists(os.path.join(os.path.curdir, "scratch", "tmp"))) runtime.stop() def test_delete_temp_folder_contents_success(self): @@ -1167,8 +1167,8 @@ def test_delete_temp_folder_contents_success(self): # add some more content os.mkdir(os.path.join(argument_composer.temp_folder, "azgps-src-123.d")) for identifier in Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST: - files_matched = glob.glob(str(argument_composer.temp_folder) + "/" + str(identifier)) - self.assertTrue(len(files_matched) == (1 if "azgps" in identifier else 0)) + items_matched = glob.glob(os.path.join(str(argument_composer.temp_folder), str(identifier))) + self.assertTrue(len(items_matched) == (1 if "azgps" in identifier else 0)) # delete temp content argument_composer.operation = Constants.ASSESSMENT @@ -1179,8 +1179,8 @@ def test_delete_temp_folder_contents_success(self): # validate files are deleted self.assertTrue(argument_composer.temp_folder is not None) for identifier in Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST: - files_matched = glob.glob(str(argument_composer.temp_folder) + "/" + str(identifier)) - self.assertTrue(len(files_matched) == 0) + items_matched = glob.glob(os.path.join(str(argument_composer.temp_folder), str(identifier))) + self.assertTrue(len(items_matched) == 0) runtime.stop() def test_delete_temp_folder_contents_when_none_exists(self): @@ -1235,4 +1235,4 @@ def __check_telemetry_events(self, runtime): if __name__ == '__main__': - unittest.main() \ No newline at end of file + unittest.main() diff --git a/src/extension/src/file_handlers/ExtEnvHandler.py b/src/extension/src/file_handlers/ExtEnvHandler.py index b3d64a5a..dbd27ec9 100644 --- a/src/extension/src/file_handlers/ExtEnvHandler.py +++ b/src/extension/src/file_handlers/ExtEnvHandler.py @@ -13,7 +13,6 @@ # limitations under the License. # # Requires Python 2.7+ -import glob import os from extension.src.Constants import Constants @@ -57,7 +56,7 @@ def __init__(self, logger, env_layer, json_file_handler, handler_env_file=Consta self.telemetry_supported = False def get_ext_env_config_value_safely(self, key, raise_if_not_found=True): - """ Allows a update deployment configuration value to be queried safely with a fall-back default (optional). + """ Allows an update deployment configuration value to be queried safely with a fall-back default (optional). An exception will be raised if default_value is not explicitly set when called (considered by-design). """ config_type = self.env_settings_all_keys.settings_parent_key if self.handler_environment_json is not None and len(self.handler_environment_json) != 0: @@ -88,7 +87,7 @@ def delete_temp_folder_contents(self, raise_if_delete_failed=False): if self.env_layer is not None \ and self.temp_folder is not None \ and os.path.exists(self.temp_folder): - self.logger.log_debug("Deleting all files of certain format from temp folder [FileFormat={0}][TempFolderLocation={1}]".format("*", str(self.temp_folder))) + self.logger.log_debug("Deleting format-matching items from temp folder. [FormatList={0}][TempFolderLocation={1}]".format("[*]", str(self.temp_folder))) self.env_layer.file_system.delete_from_dir(self.temp_folder, ["*"], raise_if_delete_failed=raise_if_delete_failed) else: self.logger.log_debug("Temp folder not found") @@ -98,7 +97,7 @@ def delete_temp_folder(self, raise_if_delete_failed=False): if self.env_layer is not None \ and self.temp_folder is not None \ and os.path.exists(self.temp_folder): - self.logger.log_debug("Deleting all files of certain format from temp folder [FileFormat={0}][TempFolderLocation={1}]".format("*", str(self.temp_folder))) + self.logger.log_debug("Deleting the temp folder. [TempFolderLocation={0}]".format(str(self.temp_folder))) self.env_layer.file_system.remove_dir(self.temp_folder, raise_if_delete_failed=raise_if_delete_failed) else: self.logger.log_debug("Temp folder not found") From 9084ce306fc64302750d28bd85eb6a1f7c2c2171 Mon Sep 17 00:00:00 2001 From: Koshy John Date: Thu, 13 Mar 2025 09:15:36 -0700 Subject: [PATCH 6/7] Additional coverage --- src/extension/tests/Test_ExtEnvHandler.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/extension/tests/Test_ExtEnvHandler.py b/src/extension/tests/Test_ExtEnvHandler.py index bead1170..d650240d 100644 --- a/src/extension/tests/Test_ExtEnvHandler.py +++ b/src/extension/tests/Test_ExtEnvHandler.py @@ -70,7 +70,9 @@ def __create_ext_env_handler_and_validate_tmp_folder(self, test_dir): self.assertTrue(ext_env_handler.temp_folder is not None) self.assertEqual(ext_env_handler.temp_folder, os.path.join(test_dir, "tmp")) - # add files to tmp folder + # add content to tmp folder + os.mkdir(os.path.join(ext_env_handler.temp_folder, "azgps-src-123.d")) + self.assertTrue(os.path.isdir(os.path.join(ext_env_handler.temp_folder, "azgps-src-123.d"))) self.runtime.create_temp_file(ext_env_handler.temp_folder, "Test1.list", content='') self.runtime.create_temp_file(ext_env_handler.temp_folder, "Test2.list", content='') self.assertTrue(os.path.isfile(os.path.join(ext_env_handler.temp_folder, "Test1.list"))) From e1f4b9a3f46bc813b2142a8c0d4ad07fc443ce41 Mon Sep 17 00:00:00 2001 From: Koshy John Date: Thu, 13 Mar 2025 09:22:17 -0700 Subject: [PATCH 7/7] Another missing os.path.join --- src/core/tests/Test_CoreMain.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/core/tests/Test_CoreMain.py b/src/core/tests/Test_CoreMain.py index 326dcf30..c815b420 100644 --- a/src/core/tests/Test_CoreMain.py +++ b/src/core/tests/Test_CoreMain.py @@ -1195,7 +1195,7 @@ def test_delete_temp_folder_contents_when_none_exists(self): # validate files are deleted self.assertTrue(runtime.execution_config.temp_folder is not None) for identifier in Constants.TEMP_FOLDER_CLEANUP_ARTIFACT_LIST: - files_matched = glob.glob(str(argument_composer.temp_folder) + "/" + str(identifier)) + files_matched = glob.glob(os.path.join(str(argument_composer.temp_folder), str(identifier))) self.assertTrue(len(files_matched) == 0) runtime.stop()