From 725a2feba467c8122d6ab49a0d1f91c164d5ae6d Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Tue, 4 Mar 2025 16:45:25 -0800 Subject: [PATCH 01/10] move task filtering by status to WorkflowTaskStatus class for correct selection of status --- src/acrcssc/azext_acrcssc/helper/_taskoperations.py | 11 +++++++---- src/acrcssc/azext_acrcssc/helper/_workflow_status.py | 6 +++++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py index ad4277c82c2..7e628685093 100644 --- a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py +++ b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py @@ -350,10 +350,13 @@ def _retrieve_logs_for_image(cmd, registry, resource_group_name, schedule, workf progress_indicator = IndeterminateProgressBar(cmd.cli_ctx, message="Retrieving logs for images") progress_indicator.begin() - image_status = WorkflowTaskStatus.from_taskrun(cmd, acr_task_run_client, registry, scan_taskruns, patch_taskruns, progress_indicator=progress_indicator) - if workflow_status: - filtered_image_status = [image for image in image_status if image["patch_status"] == workflow_status or image["scan_status"] == workflow_status] - image_status = filtered_image_status + image_status = WorkflowTaskStatus.from_taskrun(cmd, + acr_task_run_client, + registry, + scan_taskruns, + patch_taskruns, + progress_indicator=progress_indicator, + workflow_status_filter=workflow_status) end_time = time.time() execution_time = end_time - start_time diff --git a/src/acrcssc/azext_acrcssc/helper/_workflow_status.py b/src/acrcssc/azext_acrcssc/helper/_workflow_status.py index 7bb0d701767..fb37f95558c 100644 --- a/src/acrcssc/azext_acrcssc/helper/_workflow_status.py +++ b/src/acrcssc/azext_acrcssc/helper/_workflow_status.py @@ -255,7 +255,7 @@ def process_taskrun(taskrun): concurrent.futures.wait(futures) @staticmethod - def from_taskrun(cmd, taskrun_client, registry, scan_taskruns, patch_taskruns, progress_indicator=None): + def from_taskrun(cmd, taskrun_client, registry, scan_taskruns, patch_taskruns, progress_indicator=None, workflow_status_filter=None): WorkflowTaskStatus._retrieve_all_tasklogs(cmd, taskrun_client, registry, scan_taskruns, progress_indicator) all_status = {} @@ -313,6 +313,10 @@ def from_taskrun(cmd, taskrun_client, registry, scan_taskruns, patch_taskruns, p for workflow_status in failed_patch_tasklog_retrieval: workflow_status.patch_logs = workflow_status.patch_task.task_log_result + if workflow_status_filter: + filtered_workflow = {key: workflow for key, workflow in all_status.items() if workflow.status() == workflow_status_filter} + all_status = filtered_workflow + return [status.get_status() for status in all_status.values()] def get_status(self): From 718ba60f038744261eecda028ccb463b93f2bf24 Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Wed, 5 Mar 2025 10:18:05 -0800 Subject: [PATCH 02/10] stashing changes, there is an issue on filtering skipped as a task marked as skipped counts as successful, but just filtering skipped return nothing --- src/acrcssc/azext_acrcssc/helper/_workflow_status.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/acrcssc/azext_acrcssc/helper/_workflow_status.py b/src/acrcssc/azext_acrcssc/helper/_workflow_status.py index fb37f95558c..8ae2289756f 100644 --- a/src/acrcssc/azext_acrcssc/helper/_workflow_status.py +++ b/src/acrcssc/azext_acrcssc/helper/_workflow_status.py @@ -116,6 +116,13 @@ def status(self): return self.scan_status() return self.patch_status() + + def workflow_status(self, filter_status): + # this is the workflow status, which is the status of the scan task and the patch task + # if both exist, we return the patch status, otherwise we return the scan status + if self.patch_task is None: # use _workflow_status_to_task_status to filter the status on the tasks + return any() self.scan_task + return self.patch_status() # this extracts the image from the copacetic task logs, using this when we only have a repository # name and a wildcard tag From 18b6aa36c5bfffee56eb15196e1e8b4d7a92ed6b Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Thu, 20 Mar 2025 11:58:46 -0700 Subject: [PATCH 03/10] add --no-exitfirst to azdev test, force it to run all tests regarless of errors --- src/acrcssc/azext_acrcssc/tests/latest/test_cssc.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/acrcssc/azext_acrcssc/tests/latest/test_cssc.py b/src/acrcssc/azext_acrcssc/tests/latest/test_cssc.py index 482c9dd7829..789069305d7 100644 --- a/src/acrcssc/azext_acrcssc/tests/latest/test_cssc.py +++ b/src/acrcssc/azext_acrcssc/tests/latest/test_cssc.py @@ -22,6 +22,7 @@ def __init__(self, method_name): def test_create_acrcssc(self, mock_perform_continuous_patch_operation): create_acrcssc(self.cmd, "mockrg", self.registry.name, "continuouspatchv1", "mockconfig", "1d", False, False) mock_perform_continuous_patch_operation.assert_called_once_with(self.cmd, "mockrg", self.registry.name, "mockconfig", "1d", False, False, is_create=True) + self.assertTrue(False) @mock.patch("azext_acrcssc.cssc._perform_continuous_patch_operation") def test_update_acrcssc(self, mock_perform_continuous_patch_operation): From 19d278be1555f451e58672e459c74764b69248b0 Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Thu, 20 Mar 2025 16:31:31 -0700 Subject: [PATCH 04/10] remove extra method, run the filtering as part of the log retrieval --- src/acrcssc/azext_acrcssc/helper/_workflow_status.py | 11 +++-------- src/acrcssc/azext_acrcssc/tests/latest/test_cssc.py | 1 - 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/src/acrcssc/azext_acrcssc/helper/_workflow_status.py b/src/acrcssc/azext_acrcssc/helper/_workflow_status.py index 8ae2289756f..e235313bff6 100644 --- a/src/acrcssc/azext_acrcssc/helper/_workflow_status.py +++ b/src/acrcssc/azext_acrcssc/helper/_workflow_status.py @@ -116,13 +116,6 @@ def status(self): return self.scan_status() return self.patch_status() - - def workflow_status(self, filter_status): - # this is the workflow status, which is the status of the scan task and the patch task - # if both exist, we return the patch status, otherwise we return the scan status - if self.patch_task is None: # use _workflow_status_to_task_status to filter the status on the tasks - return any() self.scan_task - return self.patch_status() # this extracts the image from the copacetic task logs, using this when we only have a repository # name and a wildcard tag @@ -321,7 +314,9 @@ def from_taskrun(cmd, taskrun_client, registry, scan_taskruns, patch_taskruns, p workflow_status.patch_logs = workflow_status.patch_task.task_log_result if workflow_status_filter: - filtered_workflow = {key: workflow for key, workflow in all_status.items() if workflow.status() == workflow_status_filter} + filtered_workflow = {key: workflow + for key, workflow in all_status.items() + if workflow.status() == workflow_status_filter} all_status = filtered_workflow return [status.get_status() for status in all_status.values()] diff --git a/src/acrcssc/azext_acrcssc/tests/latest/test_cssc.py b/src/acrcssc/azext_acrcssc/tests/latest/test_cssc.py index 789069305d7..482c9dd7829 100644 --- a/src/acrcssc/azext_acrcssc/tests/latest/test_cssc.py +++ b/src/acrcssc/azext_acrcssc/tests/latest/test_cssc.py @@ -22,7 +22,6 @@ def __init__(self, method_name): def test_create_acrcssc(self, mock_perform_continuous_patch_operation): create_acrcssc(self.cmd, "mockrg", self.registry.name, "continuouspatchv1", "mockconfig", "1d", False, False) mock_perform_continuous_patch_operation.assert_called_once_with(self.cmd, "mockrg", self.registry.name, "mockconfig", "1d", False, False, is_create=True) - self.assertTrue(False) @mock.patch("azext_acrcssc.cssc._perform_continuous_patch_operation") def test_update_acrcssc(self, mock_perform_continuous_patch_operation): From a32af0560bbaec0e6f165b02dfd4fbe9098db1d0 Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Thu, 20 Mar 2025 16:35:12 -0700 Subject: [PATCH 05/10] fix style issues --- src/acrcssc/azext_acrcssc/helper/_workflow_status.py | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/acrcssc/azext_acrcssc/helper/_workflow_status.py b/src/acrcssc/azext_acrcssc/helper/_workflow_status.py index e235313bff6..cfb850977d1 100644 --- a/src/acrcssc/azext_acrcssc/helper/_workflow_status.py +++ b/src/acrcssc/azext_acrcssc/helper/_workflow_status.py @@ -255,7 +255,13 @@ def process_taskrun(taskrun): concurrent.futures.wait(futures) @staticmethod - def from_taskrun(cmd, taskrun_client, registry, scan_taskruns, patch_taskruns, progress_indicator=None, workflow_status_filter=None): + def from_taskrun(cmd, + taskrun_client, + registry, + scan_taskruns, + patch_taskruns, + progress_indicator=None, + workflow_status_filter=None): WorkflowTaskStatus._retrieve_all_tasklogs(cmd, taskrun_client, registry, scan_taskruns, progress_indicator) all_status = {} @@ -314,8 +320,8 @@ def from_taskrun(cmd, taskrun_client, registry, scan_taskruns, patch_taskruns, p workflow_status.patch_logs = workflow_status.patch_task.task_log_result if workflow_status_filter: - filtered_workflow = {key: workflow - for key, workflow in all_status.items() + filtered_workflow = {key: workflow + for key, workflow in all_status.items() if workflow.status() == workflow_status_filter} all_status = filtered_workflow From 10a5fa6aaacbfc380d8e66fb14de7548d0a986f3 Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Thu, 20 Mar 2025 16:58:03 -0700 Subject: [PATCH 06/10] add testcase for WorkflowTaskStatus.from_taskrun using filtering --- ...flow_Status.py => test_workflow_status.py} | 88 ++++++++++++++++++- 1 file changed, 87 insertions(+), 1 deletion(-) rename src/acrcssc/azext_acrcssc/tests/latest/{test_workflow_Status.py => test_workflow_status.py} (72%) diff --git a/src/acrcssc/azext_acrcssc/tests/latest/test_workflow_Status.py b/src/acrcssc/azext_acrcssc/tests/latest/test_workflow_status.py similarity index 72% rename from src/acrcssc/azext_acrcssc/tests/latest/test_workflow_Status.py rename to src/acrcssc/azext_acrcssc/tests/latest/test_workflow_status.py index 9206d0becac..1a88ce6c9fb 100644 --- a/src/acrcssc/azext_acrcssc/tests/latest/test_workflow_Status.py +++ b/src/acrcssc/azext_acrcssc/tests/latest/test_workflow_status.py @@ -28,7 +28,7 @@ def test_init(self): def test_image(self): self.assertEqual(self.workflow_task_status.image(), "repository:tag") - def test__task_status_to_workflow_status(self): + def test_task_status_to_workflow_status(self): from azext_acrcssc.helper._constants import TaskRunStatus task = mock.MagicMock() task.status = TaskRunStatus.Succeeded.value @@ -193,6 +193,92 @@ def test_from_taskrun(self, mock_retrieve_all_tasklogs, mock_get_missing_taskrun # test that a successful patch has a patched image reference self.assertTrue(all(True if workflow["patch_status"] != TaskRunStatus.Succeeded.value or not workflow["last_patched_image"].startswith("---") else False for workflow in result)) + + @mock.patch('azext_acrcssc.helper._workflow_status.WorkflowTaskStatus._get_missing_taskrun') + @mock.patch('azext_acrcssc.helper._workflow_status.WorkflowTaskStatus._retrieve_all_tasklogs') + def test_from_taskrun_with_filter(self, mock_retrieve_all_tasklogs, mock_get_missing_taskrun): + cmd = mock.MagicMock() + cmd.cli_ctx = DummyCli() + taskrun_client = MagicMock() + registry = MagicMock() + scan_taskruns = [self._generate_test_taskrun(True, repository="mock1"), self._generate_test_taskrun(True, repository="mock2"), self._generate_test_taskrun(True, repository="mock3")] + patch_taskruns = [] + + mock_retrieve_all_tasklogs.return_value = scan_taskruns + mock_get_missing_taskrun.return_value = None + + # Test with mixed scan and patch tasks status + patch_taskruns = [self._generate_test_taskrun(True, status=TaskRunStatus.Succeeded.value, tag="tag0"), + self._generate_test_taskrun(True, status=TaskRunStatus.Canceled.value, tag="tag1"), + self._generate_test_taskrun(True, status=TaskRunStatus.Queued.value, tag="tag2"), + self._generate_test_taskrun(True, status=TaskRunStatus.Running.value, tag="tag3"), + self._generate_test_taskrun(True, status=TaskRunStatus.Failed.value, tag="tag4")] + + scan_taskruns = [self._generate_test_taskrun(True, patch_taskid_in_scan=patch_taskruns[0].run_id, tag="tag0"), + self._generate_test_taskrun(True, patch_taskid_in_scan=patch_taskruns[1].run_id, tag="tag1"), + self._generate_test_taskrun(True, patch_taskid_in_scan=patch_taskruns[2].run_id, tag="tag2"), + self._generate_test_taskrun(True, patch_taskid_in_scan=patch_taskruns[3].run_id, tag="tag3"), + self._generate_test_taskrun(True, patch_taskid_in_scan=patch_taskruns[4].run_id, tag="tag4"), + self._generate_test_taskrun(True, status=TaskRunStatus.Failed.value, tag="tag5"), + self._generate_test_taskrun(True, status=TaskRunStatus.Canceled.value, tag="tag6"), + self._generate_test_taskrun(True, status=TaskRunStatus.Queued.value, tag="tag7"), + self._generate_test_taskrun(True, status=TaskRunStatus.Running.value, tag="tag8"),] + + result = WorkflowTaskStatus.from_taskrun(cmd, + taskrun_client, + registry, + scan_taskruns, + patch_taskruns, + workflow_status_filter=WorkflowTaskState.SUCCEEDED.value) + self.assertTrue(all(workflow["scan_status"] == WorkflowTaskState.SUCCEEDED.value + for workflow in result)) + self.assertTrue(all(workflow["patch_status"] == WorkflowTaskState.SUCCEEDED.value or + workflow["patch_status"] == WorkflowTaskState.SKIPPED.value + for workflow in result)) + + result = WorkflowTaskStatus.from_taskrun(cmd, + taskrun_client, + registry, + scan_taskruns, + patch_taskruns, + workflow_status_filter=WorkflowTaskState.FAILED.value) + self.assertTrue(all(workflow["scan_status"] == WorkflowTaskState.FAILED.value or + workflow["patch_status"] == WorkflowTaskState.FAILED.value + for workflow in result)) + + result = WorkflowTaskStatus.from_taskrun(cmd, + taskrun_client, + registry, + scan_taskruns, + patch_taskruns, + workflow_status_filter=WorkflowTaskState.RUNNING.value) + self.assertTrue(all(workflow["scan_status"] == WorkflowTaskState.RUNNING.value or + workflow["patch_status"] == WorkflowTaskState.RUNNING.value or + workflow["scan_status"] == WorkflowTaskState.QUEUED.value or + workflow["patch_status"] == WorkflowTaskState.QUEUED.value + for workflow in result)) + + result = WorkflowTaskStatus.from_taskrun(cmd, + taskrun_client, + registry, + scan_taskruns, + patch_taskruns, + workflow_status_filter=WorkflowTaskState.CANCELED.value) + self.assertTrue(all(workflow["scan_status"] == WorkflowTaskState.CANCELED.value or + workflow["patch_status"] == WorkflowTaskState.CANCELED.value + for workflow in result)) + + result = WorkflowTaskStatus.from_taskrun(cmd, + taskrun_client, + registry, + scan_taskruns, + patch_taskruns, + workflow_status_filter=WorkflowTaskState.SKIPPED.value) + self.assertTrue(all(workflow["scan_status"] == WorkflowTaskState.SUCCEEDED.value and + workflow["patch_status"] == WorkflowTaskState.SKIPPED.value + for workflow in result)) + + # generate a random scan or patch taskrun with the desired properties def _generate_test_taskrun(self, scan_task=True, status=TaskRunStatus.Succeeded.value, repository="mock-repo", tag="mock-tag", patch_taskid_in_scan=""): import random From 915b41b6c6892dba61a850c8209ee9fdd185e265 Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Thu, 20 Mar 2025 17:01:16 -0700 Subject: [PATCH 07/10] fix issue on test_from_taskrun generating wrong mock data --- .../tests/latest/test_workflow_status.py | 25 ++++++++++--------- 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/src/acrcssc/azext_acrcssc/tests/latest/test_workflow_status.py b/src/acrcssc/azext_acrcssc/tests/latest/test_workflow_status.py index 1a88ce6c9fb..0a36eebc025 100644 --- a/src/acrcssc/azext_acrcssc/tests/latest/test_workflow_status.py +++ b/src/acrcssc/azext_acrcssc/tests/latest/test_workflow_status.py @@ -159,11 +159,11 @@ def test_from_taskrun(self, mock_retrieve_all_tasklogs, mock_get_missing_taskrun self.assertTrue(result[0]["patch_status"] == WorkflowTaskState.SUCCEEDED.value) # Test with mixed scan and patch tasks status - patch_taskruns = [self._generate_test_taskrun(True, status=TaskRunStatus.Succeeded.value, tag="tag0"), - self._generate_test_taskrun(True, status=TaskRunStatus.Canceled.value, tag="tag1"), - self._generate_test_taskrun(True, status=TaskRunStatus.Queued.value, tag="tag2"), - self._generate_test_taskrun(True, status=TaskRunStatus.Running.value, tag="tag3"), - self._generate_test_taskrun(True, status=TaskRunStatus.Failed.value, tag="tag4")] + patch_taskruns = [self._generate_test_taskrun(False, status=TaskRunStatus.Succeeded.value, tag="tag0"), + self._generate_test_taskrun(False, status=TaskRunStatus.Canceled.value, tag="tag1"), + self._generate_test_taskrun(False, status=TaskRunStatus.Queued.value, tag="tag2"), + self._generate_test_taskrun(False, status=TaskRunStatus.Running.value, tag="tag3"), + self._generate_test_taskrun(False, status=TaskRunStatus.Failed.value, tag="tag4")] scan_taskruns = [self._generate_test_taskrun(True, patch_taskid_in_scan=patch_taskruns[0].run_id, tag="tag0"), self._generate_test_taskrun(True, patch_taskid_in_scan=patch_taskruns[1].run_id, tag="tag1"), @@ -201,18 +201,18 @@ def test_from_taskrun_with_filter(self, mock_retrieve_all_tasklogs, mock_get_mis cmd.cli_ctx = DummyCli() taskrun_client = MagicMock() registry = MagicMock() - scan_taskruns = [self._generate_test_taskrun(True, repository="mock1"), self._generate_test_taskrun(True, repository="mock2"), self._generate_test_taskrun(True, repository="mock3")] + scan_taskruns = [] patch_taskruns = [] mock_retrieve_all_tasklogs.return_value = scan_taskruns mock_get_missing_taskrun.return_value = None # Test with mixed scan and patch tasks status - patch_taskruns = [self._generate_test_taskrun(True, status=TaskRunStatus.Succeeded.value, tag="tag0"), - self._generate_test_taskrun(True, status=TaskRunStatus.Canceled.value, tag="tag1"), - self._generate_test_taskrun(True, status=TaskRunStatus.Queued.value, tag="tag2"), - self._generate_test_taskrun(True, status=TaskRunStatus.Running.value, tag="tag3"), - self._generate_test_taskrun(True, status=TaskRunStatus.Failed.value, tag="tag4")] + patch_taskruns = [self._generate_test_taskrun(False, status=TaskRunStatus.Succeeded.value, tag="tag0"), + self._generate_test_taskrun(False, status=TaskRunStatus.Canceled.value, tag="tag1"), + self._generate_test_taskrun(False, status=TaskRunStatus.Queued.value, tag="tag2"), + self._generate_test_taskrun(False, status=TaskRunStatus.Running.value, tag="tag3"), + self._generate_test_taskrun(False, status=TaskRunStatus.Failed.value, tag="tag4")] scan_taskruns = [self._generate_test_taskrun(True, patch_taskid_in_scan=patch_taskruns[0].run_id, tag="tag0"), self._generate_test_taskrun(True, patch_taskid_in_scan=patch_taskruns[1].run_id, tag="tag1"), @@ -222,7 +222,8 @@ def test_from_taskrun_with_filter(self, mock_retrieve_all_tasklogs, mock_get_mis self._generate_test_taskrun(True, status=TaskRunStatus.Failed.value, tag="tag5"), self._generate_test_taskrun(True, status=TaskRunStatus.Canceled.value, tag="tag6"), self._generate_test_taskrun(True, status=TaskRunStatus.Queued.value, tag="tag7"), - self._generate_test_taskrun(True, status=TaskRunStatus.Running.value, tag="tag8"),] + self._generate_test_taskrun(True, status=TaskRunStatus.Running.value, tag="tag8"), + self._generate_test_taskrun(True, status=TaskRunStatus.Succeeded.value, tag="tag9")] result = WorkflowTaskStatus.from_taskrun(cmd, taskrun_client, From b9f3d478e77a73a84c3c9502868db529a8612e12 Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Thu, 20 Mar 2025 17:14:41 -0700 Subject: [PATCH 08/10] fix taskoperations test cases to mock client usage --- .../latest/test_helper_taskoperations.py | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/src/acrcssc/azext_acrcssc/tests/latest/test_helper_taskoperations.py b/src/acrcssc/azext_acrcssc/tests/latest/test_helper_taskoperations.py index 396b2db24e7..3674ed4e848 100644 --- a/src/acrcssc/azext_acrcssc/tests/latest/test_helper_taskoperations.py +++ b/src/acrcssc/azext_acrcssc/tests/latest/test_helper_taskoperations.py @@ -67,8 +67,11 @@ def test_create_continuous_patch_v1_create_run_immediately_triggers_task(self, m @mock.patch("azext_acrcssc.helper._taskoperations.create_oci_artifact_continuous_patch") @mock.patch("azext_acrcssc.helper._taskoperations.validate_and_deploy_template") @mock.patch("azext_acrcssc.helper._taskoperations._eval_trigger_run") - def test_update_continuous_patch_v1_schedule_update_should_not_update_config(self, mock_eval_trigger_run, mock_validate_and_deploy_template, mock_create_oci_artifact_continuous_patch, mock_convert_timespan_to_cron, mock_check_continuoustask_exists, mock_update_task_schedule): + @mock.patch('azext_acrcssc.helper._taskoperations.cf_acr_tasks') + def test_update_continuous_patch_v1_schedule_update_should_not_update_config(self, mock_cf_acr_tasks, mock_eval_trigger_run, mock_validate_and_deploy_template, mock_create_oci_artifact_continuous_patch, mock_convert_timespan_to_cron, mock_check_continuoustask_exists, mock_update_task_schedule): # Mock the necessary dependencies + mock_acr_tasks_client = mock.MagicMock() + mock_cf_acr_tasks.return_value = mock_acr_tasks_client with tempfile.NamedTemporaryFile(delete=False) as temp_file: temp_file_path = temp_file.name mock_check_continuoustask_exists.return_value = True, [] @@ -107,8 +110,14 @@ def test_update_continuous_patch_v1__update_without_tasks_workflow_should_fail(s @mock.patch("azext_acrcssc.helper._taskoperations.create_oci_artifact_continuous_patch") @mock.patch("azext_acrcssc.helper._taskoperations.validate_and_deploy_template") @mock.patch("azext_acrcssc.helper._taskoperations._eval_trigger_run") - def test_update_continuous_patch_v1_schedule_update_run_immediately_triggers_task(self, mock_eval_trigger_run, mock_validate_and_deploy_template, mock_create_oci_artifact_continuous_patch, mock_convert_timespan_to_cron, mock_check_continuoustask_exists, mock_update_task_schedule): + @mock.patch('azext_acrcssc.helper._taskoperations.cf_acr_tasks') + @mock.patch('azext_acrcssc.helper._taskoperations.cf_authorization') + def test_update_continuous_patch_v1_schedule_update_run_immediately_triggers_task(self, mock_cf_authorization, mock_cf_acr_tasks, mock_eval_trigger_run, mock_validate_and_deploy_template, mock_create_oci_artifact_continuous_patch, mock_convert_timespan_to_cron, mock_check_continuoustask_exists, mock_update_task_schedule): # Mock the necessary dependencies + mock_acr_tasks_client = mock.MagicMock() + mock_cf_acr_tasks.return_value = mock_acr_tasks_client + mock_role_client = mock.MagicMock() + mock_cf_authorization.return_value = mock_role_client with tempfile.NamedTemporaryFile(delete=False) as temp_file: temp_file_path = temp_file.name mock_check_continuoustask_exists.return_value = True, [] @@ -129,9 +138,10 @@ def test_update_continuous_patch_v1_schedule_update_run_immediately_triggers_tas @mock.patch("azext_acrcssc.helper._taskoperations.check_continuous_task_config_exists") @mock.patch('azext_acrcssc.helper._taskoperations.delete_oci_artifact_continuous_patch') @mock.patch("azext_acrcssc.helper._taskoperations.check_continuous_task_exists") + @mock.patch("azext_acrcssc.helper._taskoperations.cf_acr_runs") @mock.patch('azext_acrcssc.helper._taskoperations.cf_acr_tasks') @mock.patch('azext_acrcssc.helper._taskoperations.cf_authorization') - def test_delete_continuous_patch_v1(self, mock_cf_authorization, mock_cf_acr_tasks, mock_check_continuoustask_exists, mock_delete_oci_artifact_continuous_patch, mock_check_continuous_task_config_exists, mock_get_taskruns_with_filter, mock_cancel_task_runs): + def test_delete_continuous_patch_v1(self, mock_cf_authorization, mock_cf_acr_tasks, mock_cf_acr_runs, mock_check_continuoustask_exists, mock_delete_oci_artifact_continuous_patch, mock_check_continuous_task_config_exists, mock_get_taskruns_with_filter, mock_cancel_task_runs): # Mock the necessary dependencies mock_check_continuoustask_exists.return_value = True, [] mock_check_continuous_task_config_exists.return_value = True @@ -141,6 +151,8 @@ def test_delete_continuous_patch_v1(self, mock_cf_authorization, mock_cf_acr_tas mock_cf_acr_tasks.return_value = mock_acr_tasks_client mock_role_client = mock.MagicMock() mock_cf_authorization.return_value = mock_role_client + mock_acr_run_client = mock.MagicMock() + mock_cf_acr_runs.return_value = mock_acr_run_client mock_task = mock.MagicMock() mock_task.identity = mock.MagicMock()(principal_id='principal_id') mock_acr_tasks_client.get.return_value = mock_task @@ -200,14 +212,17 @@ def test_list_continuous_patch_v1(self, mock_transform_task_list, mock_cf_acr_ta @mock.patch("azext_acrcssc.helper._taskoperations.prepare_source_location") @mock.patch("azext_acrcssc.helper._taskoperations.cf_acr_registries_tasks") @mock.patch("azext_acrcssc.helper._taskoperations.cf_acr_runs") + @mock.patch('azext_acrcssc.helper._taskoperations.cf_acr_tasks') @mock.patch("azext_acrcssc.helper._taskoperations.LongRunningOperation") - def test_acr_cssc_dry_run(self, mock_LongRunningOperation, mock_cf_acr_runs, mock_cf_acr_registries_tasks, mock_prepare_source_location, mock_delete_temporary_dry_run_file, mock_create_temporary_dry_run_file, mock_generate_logs): + def test_acr_cssc_dry_run(self, mock_LongRunningOperation, mock_cf_acr_tasks, mock_cf_acr_runs, mock_cf_acr_registries_tasks, mock_prepare_source_location, mock_delete_temporary_dry_run_file, mock_create_temporary_dry_run_file, mock_generate_logs): # Mock the necessary dependencies config_file_path = "test_config_file_path" mock_acr_registries_task_client = mock.MagicMock() mock_cf_acr_registries_tasks.return_value = mock_acr_registries_task_client mock_acr_run_client = mock.MagicMock() mock_cf_acr_runs.return_value = mock_acr_run_client + mock_acr_task_client = mock.MagicMock() + mock_cf_acr_tasks.return_value = mock_acr_task_client mock_LongRunningOperation.return_value.return_value.run_id = "test_run_id" mock_generate_logs.return_value = "mock_logs" From 4947b3552fb5bdfe1d0229870fd40afe068eac25 Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Thu, 20 Mar 2025 17:56:52 -0700 Subject: [PATCH 09/10] add comments and fix final issue with filtering --- .../azext_acrcssc/helper/_workflow_status.py | 30 +++++++++++++++---- .../tests/latest/test_workflow_status.py | 5 ++++ 2 files changed, 29 insertions(+), 6 deletions(-) diff --git a/src/acrcssc/azext_acrcssc/helper/_workflow_status.py b/src/acrcssc/azext_acrcssc/helper/_workflow_status.py index cfb850977d1..ef4523d7bf6 100644 --- a/src/acrcssc/azext_acrcssc/helper/_workflow_status.py +++ b/src/acrcssc/azext_acrcssc/helper/_workflow_status.py @@ -319,13 +319,31 @@ def from_taskrun(cmd, for workflow_status in failed_patch_tasklog_retrieval: workflow_status.patch_logs = workflow_status.patch_task.task_log_result - if workflow_status_filter: - filtered_workflow = {key: workflow - for key, workflow in all_status.items() - if workflow.status() == workflow_status_filter} - all_status = filtered_workflow + return [status.get_status() + for status in WorkflowTaskStatus._filter_taskruns(all_status, workflow_status_filter).values()] + + @staticmethod + def _filter_taskruns(workflows, workflow_status_filter=None): + if not workflows: + return {} - return [status.get_status() for status in all_status.values()] + if not workflow_status_filter: + return workflows + + # SKIPPED is a special case, because it means that the patch task does not exist, but the scan task + # succeeded. Another special case that is not explicit here is SUCCEEDED, which will include both + # scan and patch tasks that succeeded, or the scan task succeeded and the patch task is skipped + if workflow_status_filter == WorkflowTaskState.SKIPPED.value: + filtered_workflow = {key: workflow + for key, workflow in workflows.items() + if workflow.scan_status() == WorkflowTaskState.SUCCEEDED.value and + workflow.patch_status() == WorkflowTaskState.SKIPPED.value} + return filtered_workflow + + filtered_workflow = {key: workflow + for key, workflow in workflows.items() + if workflow.status() == workflow_status_filter} + return filtered_workflow def get_status(self): scan_status = self.scan_status() diff --git a/src/acrcssc/azext_acrcssc/tests/latest/test_workflow_status.py b/src/acrcssc/azext_acrcssc/tests/latest/test_workflow_status.py index 0a36eebc025..c1efcbae4ad 100644 --- a/src/acrcssc/azext_acrcssc/tests/latest/test_workflow_status.py +++ b/src/acrcssc/azext_acrcssc/tests/latest/test_workflow_status.py @@ -231,6 +231,7 @@ def test_from_taskrun_with_filter(self, mock_retrieve_all_tasklogs, mock_get_mis scan_taskruns, patch_taskruns, workflow_status_filter=WorkflowTaskState.SUCCEEDED.value) + self.assertTrue(len(result) > 0) self.assertTrue(all(workflow["scan_status"] == WorkflowTaskState.SUCCEEDED.value for workflow in result)) self.assertTrue(all(workflow["patch_status"] == WorkflowTaskState.SUCCEEDED.value or @@ -243,6 +244,7 @@ def test_from_taskrun_with_filter(self, mock_retrieve_all_tasklogs, mock_get_mis scan_taskruns, patch_taskruns, workflow_status_filter=WorkflowTaskState.FAILED.value) + self.assertTrue(len(result) > 0) self.assertTrue(all(workflow["scan_status"] == WorkflowTaskState.FAILED.value or workflow["patch_status"] == WorkflowTaskState.FAILED.value for workflow in result)) @@ -253,6 +255,7 @@ def test_from_taskrun_with_filter(self, mock_retrieve_all_tasklogs, mock_get_mis scan_taskruns, patch_taskruns, workflow_status_filter=WorkflowTaskState.RUNNING.value) + self.assertTrue(len(result) > 0) self.assertTrue(all(workflow["scan_status"] == WorkflowTaskState.RUNNING.value or workflow["patch_status"] == WorkflowTaskState.RUNNING.value or workflow["scan_status"] == WorkflowTaskState.QUEUED.value or @@ -265,6 +268,7 @@ def test_from_taskrun_with_filter(self, mock_retrieve_all_tasklogs, mock_get_mis scan_taskruns, patch_taskruns, workflow_status_filter=WorkflowTaskState.CANCELED.value) + self.assertTrue(len(result) > 0) self.assertTrue(all(workflow["scan_status"] == WorkflowTaskState.CANCELED.value or workflow["patch_status"] == WorkflowTaskState.CANCELED.value for workflow in result)) @@ -275,6 +279,7 @@ def test_from_taskrun_with_filter(self, mock_retrieve_all_tasklogs, mock_get_mis scan_taskruns, patch_taskruns, workflow_status_filter=WorkflowTaskState.SKIPPED.value) + self.assertTrue(len(result) > 0) self.assertTrue(all(workflow["scan_status"] == WorkflowTaskState.SUCCEEDED.value and workflow["patch_status"] == WorkflowTaskState.SKIPPED.value for workflow in result)) From 1e2d041d60a789eb5feb3d32c6aae4bb72de9326 Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Fri, 21 Mar 2025 09:52:09 -0700 Subject: [PATCH 10/10] fix style issue --- .../azext_acrcssc/helper/_workflow_status.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/acrcssc/azext_acrcssc/helper/_workflow_status.py b/src/acrcssc/azext_acrcssc/helper/_workflow_status.py index ef4523d7bf6..bbf68cc2c95 100644 --- a/src/acrcssc/azext_acrcssc/helper/_workflow_status.py +++ b/src/acrcssc/azext_acrcssc/helper/_workflow_status.py @@ -330,9 +330,10 @@ def _filter_taskruns(workflows, workflow_status_filter=None): if not workflow_status_filter: return workflows - # SKIPPED is a special case, because it means that the patch task does not exist, but the scan task - # succeeded. Another special case that is not explicit here is SUCCEEDED, which will include both - # scan and patch tasks that succeeded, or the scan task succeeded and the patch task is skipped + # SKIPPED is a special case, because it means that the patch task does not exist, + # but the scan task succeeded. Another special case that is not explicit here is SUCCEEDED, + # which will include both scan and patch tasks that succeeded, or the scan task succeeded + # and the patch task is skipped if workflow_status_filter == WorkflowTaskState.SKIPPED.value: filtered_workflow = {key: workflow for key, workflow in workflows.items() @@ -340,9 +341,10 @@ def _filter_taskruns(workflows, workflow_status_filter=None): workflow.patch_status() == WorkflowTaskState.SKIPPED.value} return filtered_workflow - filtered_workflow = {key: workflow - for key, workflow in workflows.items() - if workflow.status() == workflow_status_filter} + filtered_workflow = { + key: workflow + for key, workflow in workflows.items() + if workflow.status() == workflow_status_filter} return filtered_workflow def get_status(self):