Apt - fix for non-sec updates getting misclassified & related tmp cleanup

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)

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"

src/core/src/bootstrap/EnvLayer.py

@@ -430,26 +430,26 @@ 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):
+            """ 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)))
 
-                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):
+                            shutil.rmtree(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(
-                            str(dir_name),
-                            str(file_to_delete),
-                            repr(error),
-                            str(raise_if_delete_failed))
+                        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

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)
@@ -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)
@@ -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

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

src/core/tests/Test_CoreMain.py

@@ -401,7 +401,7 @@
         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_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 @@
         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 @@
 
         # 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,23 +1147,29 @@
 
         # 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
         backup_os_path_exists = os.path.exists
         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):
         argument_composer = ArgumentComposer()
         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:
+            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
         runtime = RuntimeCompositor(argument_composer.get_composed_arguments(), True, Constants.APT)
@@ -1172,8 +1178,9 @@
 
         # 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:
+            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):
@@ -1183,12 +1190,13 @@
         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(os.path.join(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 @@
         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
@@ -1227,4 +1235,4 @@
 
 
 if __name__ == '__main__':
-    unittest.main()
+    unittest.main()

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):