diff --git a/src/acrcssc/azext_acrcssc/_validators.py b/src/acrcssc/azext_acrcssc/_validators.py index b19a9ae51aa..e5846b1d27d 100644 --- a/src/acrcssc/azext_acrcssc/_validators.py +++ b/src/acrcssc/azext_acrcssc/_validators.py @@ -78,11 +78,27 @@ def _validate_continuouspatch_config(config): raise InvalidArgumentValueError(f"Configuration error: Version {config.get('version', '')} is not supported. Supported versions are {CONTINUOUSPATCH_CONFIG_SUPPORTED_VERSIONS}") +# to save on API calls, we the list of tasks found in the registry def check_continuous_task_exists(cmd, registry): - exists = False - for task_name in CONTINUOUSPATCH_ALL_TASK_NAMES: - exists = exists or _check_task_exists(cmd, registry, task_name) - return exists + task_list = [] + missing_tasks = [] + try: + acrtask_client = cf_acr_tasks(cmd.cli_ctx) + for task_name in CONTINUOUSPATCH_ALL_TASK_NAMES: + task = get_task(cmd, registry, task_name, acrtask_client) + if task is None: + missing_tasks.append(task_name) + else: + task_list.append(task) + + if len(missing_tasks) > 0: + logger.debug(f"Failed to find tasks {', '.join(missing_tasks)} from registry {registry.name}") + return False, task_list + + return True, task_list + except Exception as exception: + logger.debug(f"Failed to find tasks from registry {registry.name} : {exception}") + return False, task_list def check_continuous_task_config_exists(cmd, registry): @@ -106,20 +122,6 @@ def check_continuous_task_config_exists(cmd, registry): return True -def _check_task_exists(cmd, registry, task_name=""): - acrtask_client = cf_acr_tasks(cmd.cli_ctx) - - try: - task = get_task(cmd, registry, task_name, acrtask_client) - except Exception as exception: - logger.debug(f"Failed to find task {task_name} from registry {registry.name} : {exception}") - return False - - if task is not None: - return True - return False - - def _validate_schedule(schedule): # during update, schedule can be null if we are only updating the config if schedule is None: @@ -148,4 +150,4 @@ def validate_task_type(task_type): def validate_cssc_optional_inputs(cssc_config_path, schedule): if cssc_config_path is None and schedule is None: - raise InvalidArgumentValueError(error_msg="Provide at least one parameter to update: --schedule or --config") + raise InvalidArgumentValueError(error_msg="Provide at least one parameter to update: --schedule, --config") diff --git a/src/acrcssc/azext_acrcssc/helper/_deployment.py b/src/acrcssc/azext_acrcssc/helper/_deployment.py index e92cb7f553d..e263822ced8 100644 --- a/src/acrcssc/azext_acrcssc/helper/_deployment.py +++ b/src/acrcssc/azext_acrcssc/helper/_deployment.py @@ -21,8 +21,13 @@ logger = get_logger(__name__) -def validate_and_deploy_template(cmd_ctx, registry, resource_group: str, deployment_name: str, - template_file_name: str, parameters: dict, dryrun: Optional[bool] = False): +def validate_and_deploy_template(cmd_ctx, + registry, + resource_group: str, + deployment_name: str, + template_file_name: str, + parameters: dict, + dryrun: Optional[bool] = False): logger.debug(f'Working with resource group {resource_group}, registry {registry} template {template_file_name}') deployment_path = os.path.dirname( @@ -110,7 +115,7 @@ def deploy_template(cmd_ctx, resource_group, deployment_name, template): deployment = Deployment( properties=template, # tags = { "test": CSSC_TAGS }, - # we need to know if tagging is something that will help ust, + # we need to know if tagging is something that will help us, # tasks are proxy resources, so not sure how that would work ) @@ -122,8 +127,7 @@ def deploy_template(cmd_ctx, resource_group, deployment_name, template): # Wait for the deployment to complete and get the outputs deployment: DeploymentExtended = LongRunningOperation( - cmd_ctx, - "Deploying ARM template" + cmd_ctx )(poller) logger.debug("Finished deploying") diff --git a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py index 6e6dc1d7c52..3ec2d73e3a6 100644 --- a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py +++ b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py @@ -43,14 +43,24 @@ logger = get_logger(__name__) -def create_update_continuous_patch_v1(cmd, registry, cssc_config_file, schedule, dryrun, run_immediately, is_create_workflow=True): +def create_update_continuous_patch_v1(cmd, + registry, + cssc_config_file, + schedule, + dryrun, + run_immediately, + is_create_workflow=True): + logger.debug(f"Entering continuousPatchV1_creation {cssc_config_file} {dryrun} {run_immediately}") + resource_group = parse_resource_id(registry.id)[RESOURCE_GROUP] schedule_cron_expression = None if schedule is not None: schedule_cron_expression = convert_timespan_to_cron(schedule) + logger.debug(f"converted schedule to cron expression: {schedule_cron_expression}") - cssc_tasks_exists = check_continuous_task_exists(cmd, registry) + + cssc_tasks_exists, task_list = check_continuous_task_exists(cmd, registry) if is_create_workflow: if cssc_tasks_exists: raise AzCLIError(f"{CONTINUOUS_PATCHING_WORKFLOW_NAME} workflow task already exists. Use 'az acr supply-chain workflow update' command to perform updates.") @@ -58,26 +68,19 @@ def create_update_continuous_patch_v1(cmd, registry, cssc_config_file, schedule, else: if not cssc_tasks_exists: raise AzCLIError(f"{CONTINUOUS_PATCHING_WORKFLOW_NAME} workflow task does not exist. Use 'az acr supply-chain workflow create' command to create {CONTINUOUS_PATCHING_WORKFLOW_NAME} workflow.") - _update_cssc_workflow(cmd, registry, schedule_cron_expression, resource_group, dryrun) + + _update_cssc_workflow(cmd, registry, schedule_cron_expression, resource_group, dryrun, task_list) if cssc_config_file is not None: create_oci_artifact_continuous_patch(registry, cssc_config_file, dryrun) logger.debug(f"Uploading of {cssc_config_file} completed successfully.") _eval_trigger_run(cmd, registry, resource_group, run_immediately) - - # on 'update' schedule is optional - if schedule is None: - task = get_task(cmd, registry, CONTINUOUSPATCH_TASK_SCANREGISTRY_NAME) - trigger = task.trigger - if trigger and trigger.timer_triggers: - schedule_cron_expression = trigger.timer_triggers[0].schedule - next_date = get_next_date(schedule_cron_expression) print(f"Continuous Patching workflow scheduled to run next at: {next_date} UTC") -def _create_cssc_workflow(cmd, registry, schedule_cron_expression, resource_group, dry_run): +def _create_cssc_workflow(cmd, registry, schedule_cron_expression, resource_group, dry_run, silent_execution=False): parameters = { "AcrName": {"value": registry.name}, "AcrLocation": {"value": registry.location}, @@ -99,10 +102,35 @@ def _create_cssc_workflow(cmd, registry, schedule_cron_expression, resource_grou dry_run ) - logger.warning(f"Deployment of {CONTINUOUS_PATCHING_WORKFLOW_NAME} tasks completed successfully.") + if not silent_execution: + print(f"Deployment of {CONTINUOUS_PATCHING_WORKFLOW_NAME} tasks completed successfully.") + + +def _update_cssc_workflow(cmd, registry, schedule_cron_expression, resource_group, dry_run, task_list): + # compare the task definition to the existing tasks, if there is a difference, we need to update the tasks + # if we need to update the tasks, we will update the cron expression from it + # if not we just update the cron expression from the given parameter + for task in task_list: + deployed_task = task.step.encoded_task_content + extension_task = _create_encoded_task(CONTINUOUSPATCH_TASK_DEFINITION[task.name]["template_file"]) + if deployed_task != extension_task: + logger.debug(f"Task {task.name} is different from the extension task, updating the task") + + if schedule_cron_expression is None: + trigger_task = next((t for t in task_list if t.name == CONTINUOUSPATCH_TASK_SCANREGISTRY_NAME), None) + if trigger_task is None: + raise AzCLIError(f"Task {CONTINUOUSPATCH_TASK_SCANREGISTRY_NAME} not found in the registry") + if not trigger_task.trigger.timer_triggers: + raise AzCLIError(f"No timer triggers found for task {CONTINUOUSPATCH_TASK_SCANREGISTRY_NAME}") + schedule_cron_expression = trigger_task.trigger.timer_triggers[0].schedule + _create_cssc_workflow(cmd, registry, schedule_cron_expression, resource_group, dry_run, silent_execution=True) + + # the deployment will also update the schedule if it was set, we no longer need to manually set it + return + + logger.debug("No difference found between the existing tasks and the extension tasks") -def _update_cssc_workflow(cmd, registry, schedule_cron_expression, resource_group, dry_run): if schedule_cron_expression is not None: _update_task_schedule(cmd, registry, schedule_cron_expression, resource_group, dry_run) @@ -118,7 +146,7 @@ def _eval_trigger_run(cmd, registry, resource_group, run_immediately): def delete_continuous_patch_v1(cmd, registry, dryrun): logger.debug("Entering delete_continuous_patch_v1") - cssc_tasks_exists = check_continuous_task_exists(cmd, registry) + cssc_tasks_exists, _ = check_continuous_task_exists(cmd, registry) cssc_config_exists = check_continuous_task_config_exists(cmd, registry) if not dryrun and (cssc_tasks_exists or cssc_config_exists): cssc_tasks = ', '.join(CONTINUOUSPATCH_ALL_TASK_NAMES) @@ -136,8 +164,8 @@ def delete_continuous_patch_v1(cmd, registry, dryrun): def list_continuous_patch_v1(cmd, registry): logger.debug("Entering list_continuous_patch_v1") - - if not check_continuous_task_exists(cmd, registry): + cssc_tasks_exists, _ = check_continuous_task_exists(cmd, registry) + if not cssc_tasks_exists: logger.warning(f"{CONTINUOUS_PATCHING_WORKFLOW_NAME} workflow task does not exist. Run 'az acr supply-chain workflow create' to create workflow tasks") return @@ -154,7 +182,9 @@ def acr_cssc_dry_run(cmd, registry, config_file_path, is_create=True): if config_file_path is None: logger.error("--config parameter is needed to perform dry-run check.") return - if is_create and check_continuous_task_exists(cmd, registry): + + cssc_tasks_exists, _ = check_continuous_task_exists(cmd, registry) + if is_create and cssc_tasks_exists: raise AzCLIError(f"{CONTINUOUS_PATCHING_WORKFLOW_NAME} workflow task already exists. Use 'az acr supply-chain workflow update' command to perform updates.") try: file_name = os.path.basename(config_file_path) @@ -484,15 +514,3 @@ def get_next_date(cron_expression): cron = croniter(cron_expression, now, expand_from_start_time=False) next_date = cron.get_next(datetime) return str(next_date) - - -def get_task(cmd, registry, task_name=""): - acrtask_client = cf_acr_tasks(cmd.cli_ctx) - resourceid = parse_resource_id(registry.id) - resource_group = resourceid[RESOURCE_GROUP] - - try: - return acrtask_client.get(resource_group, registry.name, task_name) - except Exception as exception: - logger.debug(f"Failed to find task {task_name} from registry {registry.name} : {exception}") - return None 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 9b6cad69620..fd73f8431e0 100644 --- a/src/acrcssc/azext_acrcssc/tests/latest/test_helper_taskoperations.py +++ b/src/acrcssc/azext_acrcssc/tests/latest/test_helper_taskoperations.py @@ -22,7 +22,7 @@ def test_create_continuous_patch_v1(self, mock_trigger_task_run, mock_validate_a # Mock the necessary dependencies with tempfile.NamedTemporaryFile(delete=False) as temp_file: temp_file_path = temp_file.name - mock_check_continuoustask_exists.return_value = False + mock_check_continuoustask_exists.return_value = False, [] mock_convert_timespan_to_cron.return_value = "0 0 * * *" mock_parse_resource_id.return_value = {"resource_group": "test_rg"} cmd = self._setup_cmd() @@ -48,7 +48,7 @@ def test_create_continuous_patch_v1_create_run_immediately_triggers_task(self, m # Mock the necessary dependencies with tempfile.NamedTemporaryFile(delete=False) as temp_file: temp_file_path = temp_file.name - mock_check_continuoustask_exists.return_value = False + mock_check_continuoustask_exists.return_value = False, [] mock_convert_timespan_to_cron.return_value = "0 0 * * *" mock_parse_resource_id.return_value = {"resource_group": "test_rg"} cmd = self._setup_cmd() @@ -75,7 +75,7 @@ def test_update_continuous_patch_v1_schedule_update_should_not_update_config(sel # Mock the necessary dependencies with tempfile.NamedTemporaryFile(delete=False) as temp_file: temp_file_path = temp_file.name - mock_check_continuoustask_exists.return_value = True + mock_check_continuoustask_exists.return_value = True, [] mock_convert_timespan_to_cron.return_value = "0 0 * * *" mock_parse_resource_id.return_value = {"resource_group": "test_rg"} cmd = self._setup_cmd() @@ -100,7 +100,7 @@ def test_update_continuous_patch_v1__update_without_tasks_workflow_should_fail(s # Mock the necessary dependencies with tempfile.NamedTemporaryFile(delete=False) as temp_file: temp_file_path = temp_file.name - mock_check_continuoustask_exists.return_value = False + mock_check_continuoustask_exists.return_value = False, [] mock_convert_timespan_to_cron.return_value = "0 0 * * *" mock_parse_resource_id.return_value = {"resource_group": "test_rg"} cmd = self._setup_cmd() @@ -125,7 +125,7 @@ def test_update_continuous_patch_v1_schedule_update_run_immediately_triggers_tas # Mock the necessary dependencies with tempfile.NamedTemporaryFile(delete=False) as temp_file: temp_file_path = temp_file.name - mock_check_continuoustask_exists.return_value = True + mock_check_continuoustask_exists.return_value = True, [] mock_convert_timespan_to_cron.return_value = "0 0 * * *" mock_parse_resource_id.return_value = {"resource_group": "test_rg"} cmd = self._setup_cmd() @@ -134,7 +134,7 @@ def test_update_continuous_patch_v1_schedule_update_run_immediately_triggers_tas # Call the function create_update_continuous_patch_v1(cmd, registry, None, "2d", False, True, False) - + # Assert that the dependencies were called with the correct arguments mock_convert_timespan_to_cron.assert_called_once_with("2d") mock_create_oci_artifact_continuous_patch.assert_not_called() @@ -153,7 +153,7 @@ def test_delete_continuous_patch_v1(self, mock_cf_authorization, mock_cf_acr_tas cmd = self._setup_cmd() mock_registry = mock.MagicMock() mock_dryrun = False - mock_check_continuoustask_exists.return_value = True + mock_check_continuoustask_exists.return_value = True, [] mock_check_continuous_task_config_exists.return_value = True mock_registry.id = 'registry_id' mock_resource_group = mock.MagicMock() diff --git a/src/acrcssc/azext_acrcssc/tests/latest/test_validators.py b/src/acrcssc/azext_acrcssc/tests/latest/test_validators.py index c7ea5433a13..f911bee73ff 100644 --- a/src/acrcssc/azext_acrcssc/tests/latest/test_validators.py +++ b/src/acrcssc/azext_acrcssc/tests/latest/test_validators.py @@ -7,7 +7,6 @@ import tempfile import unittest from unittest import mock -from datetime import ( datetime,timezone) from ..._validators import ( _validate_schedule, check_continuous_task_exists, validate_continuouspatch_config_v1 ) @@ -28,15 +27,14 @@ def test_validate_schedule_valid(self): for timespan in test_cases: with self.subTest(timespan=timespan): - _validate_schedule(timespan) - + _validate_schedule(timespan) + def test_validate_schedule_invalid(self): test_cases = [('df'),('12'),('dd'),('41d'), ('21dd')] for timespan in test_cases: self.assertRaises(InvalidArgumentValueError, _validate_schedule, timespan) - @patch('azext_acrcssc._validators.cf_acr_tasks') def test_check_continuoustask_exists(self, mock_cf_acr_tasks): cmd = self._setup_cmd() @@ -46,7 +44,7 @@ def test_check_continuoustask_exists(self, mock_cf_acr_tasks): mock_cf_acr_tasks.return_value = cf_acr_tasks_mock cf_acr_tasks_mock.get.return_value = {"name": "my_task"} - exists = check_continuous_task_exists(cmd, registry) + exists, _ = check_continuous_task_exists(cmd, registry) self.assertTrue(exists) @patch('azext_acrcssc._validators.cf_acr_tasks') @@ -59,15 +57,15 @@ def test_task_does_not_exist(self, mock_cf_acr_tasks): mock_cf_acr_tasks.return_value = cf_acr_tasks_mock cf_acr_tasks_mock.get.return_value = None - exists = check_continuous_task_exists(cmd, registry) + exists, _ = check_continuous_task_exists(cmd, registry) self.assertFalse(exists) - + def test_validate_continuouspatch_file(self): # Create a temporary file for testing with tempfile.NamedTemporaryFile(delete=False) as temp_file: temp_file_path = temp_file.name - # Test when the file does not exist + # Test when the file does not exist with patch('os.path.exists', return_value=False): self.assertRaises(AzCLIError, validate_continuouspatch_config_v1, temp_file_path) @@ -118,9 +116,9 @@ def test_validate_continuouspatch_json_valid_json_should_parse(self, mock_load): patch('os.path.isfile', return_value=True), \ patch('os.path.getsize', return_value=100), \ patch('os.access', return_value=True): - validate_continuouspatch_config_v1(temp_file_path) - mock_load.assert_called_once_with(mock.ANY) - + validate_continuouspatch_config_v1(temp_file_path) + mock_load.assert_called_once_with(mock.ANY) + @patch('azext_acrcssc._validators.json.load') def test_validate_continuouspatch_json_invalid_json_should_fail(self, mock_load): with tempfile.NamedTemporaryFile(delete=False) as temp_file: @@ -131,14 +129,14 @@ def test_validate_continuouspatch_json_invalid_json_should_fail(self, mock_load) { "repository": "docker-local", "tags": ["v1"], - }], - } + }] + } mock_load.return_value = mock_invalid_config with patch('os.path.exists', return_value=True), \ - patch('os.path.isfile', return_value=True), \ - patch('os.path.getsize', return_value=100), \ - patch('os.access', return_value=True): + patch('os.path.isfile', return_value=True), \ + patch('os.path.getsize', return_value=100), \ + patch('os.access', return_value=True): self.assertRaises(AzCLIError, validate_continuouspatch_config_v1, temp_file_path) @patch('azext_acrcssc._validators.json.load') @@ -151,14 +149,14 @@ def test_validate_continuouspatch_json_invalid_tags_should_fail(self, mock_load) { "repository": "docker-local", "tags": ["v1-patched"], - }], - } + }] + } mock_load.return_value = mock_invalid_config with patch('os.path.exists', return_value=True), \ - patch('os.path.isfile', return_value=True), \ - patch('os.path.getsize', return_value=100), \ - patch('os.access', return_value=True): + patch('os.path.isfile', return_value=True), \ + patch('os.path.getsize', return_value=100), \ + patch('os.access', return_value=True): self.assertRaises(AzCLIError, validate_continuouspatch_config_v1, temp_file_path) mock_invalid_config = { @@ -171,12 +169,12 @@ def test_validate_continuouspatch_json_invalid_tags_should_fail(self, mock_load) mock_load.return_value = mock_invalid_config with patch('os.path.exists', return_value=True), \ - patch('os.path.isfile', return_value=True), \ - patch('os.path.getsize', return_value=100), \ - patch('os.access', return_value=True): + patch('os.path.isfile', return_value=True), \ + patch('os.path.getsize', return_value=100), \ + patch('os.access', return_value=True): self.assertRaises(AzCLIError, validate_continuouspatch_config_v1, temp_file_path) def _setup_cmd(self): cmd = mock.MagicMock() cmd.cli_ctx = DummyCli() - return cmd \ No newline at end of file + return cmd