From 7eeebb5fd86525bdc9f64dd52387383dd267b539 Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Wed, 5 Feb 2025 11:29:22 -0800 Subject: [PATCH 1/5] add an option to update all tasks yamls throught an ARM redeploy --- src/acrcssc/azext_acrcssc/_params.py | 1 + src/acrcssc/azext_acrcssc/_validators.py | 6 +-- src/acrcssc/azext_acrcssc/cssc.py | 17 +++++-- .../azext_acrcssc/helper/_deployment.py | 11 +++-- .../azext_acrcssc/helper/_taskoperations.py | 47 +++++++++++++------ 5 files changed, 56 insertions(+), 26 deletions(-) diff --git a/src/acrcssc/azext_acrcssc/_params.py b/src/acrcssc/azext_acrcssc/_params.py index 5305a15f1d6..ca7c83d39cf 100644 --- a/src/acrcssc/azext_acrcssc/_params.py +++ b/src/acrcssc/azext_acrcssc/_params.py @@ -29,6 +29,7 @@ def load_arguments(self: AzCommandsLoader, _): c.argument("schedule", options_list=["--schedule"], help="schedule to run the scan and patching task. E.g. `d` where n is the number of days between each run. Max value is 30d.", required=False) c.argument("run_immediately", options_list=["--run-immediately"], help="Set this flag to trigger the immediate run of the selected workflow task. Default value: false.", arg_type=get_three_state_flag(), required=False) c.argument("dryrun", options_list=["--dry-run"], help="Use this flag to see the qualifying repositories and tags that would be affected by the workflow. Default value: false. 'config' parameter is mandatory to provide with dry-run", arg_type=get_three_state_flag(), required=False) + c.argument("force_task_update", options_list=["--force-task-update"], help="Use this flag to update the Workflow's Task definition to the latest version. This should be done only when the CLI extension is updated to ensure compatibility with the latest server-side code.", arg_type=get_three_state_flag(), required=False) with self.argument_context("acr supply-chain workflow list") as c: c.argument("status", arg_type=get_enum_type(WorkflowTaskState), options_list=["--run-status"], help="Status to filter the supply-chain workflow image status.", required=False) diff --git a/src/acrcssc/azext_acrcssc/_validators.py b/src/acrcssc/azext_acrcssc/_validators.py index b19a9ae51aa..a5bcf4e0581 100644 --- a/src/acrcssc/azext_acrcssc/_validators.py +++ b/src/acrcssc/azext_acrcssc/_validators.py @@ -146,6 +146,6 @@ def validate_task_type(task_type): raise InvalidArgumentValueError(error_msg=ERROR_MESSAGE_INVALID_TASK) -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") +def validate_cssc_optional_inputs(cssc_config_path, schedule, force_task_update): + if cssc_config_path is None and schedule is None and force_task_update is False: + raise InvalidArgumentValueError(error_msg="Provide at least one parameter to update: --schedule, --config, or --force-task-update") diff --git a/src/acrcssc/azext_acrcssc/cssc.py b/src/acrcssc/azext_acrcssc/cssc.py index ba227b76434..f185d79ddc2 100644 --- a/src/acrcssc/azext_acrcssc/cssc.py +++ b/src/acrcssc/azext_acrcssc/cssc.py @@ -31,6 +31,7 @@ def _perform_continuous_patch_operation(cmd, schedule, dryrun=False, run_immediately=False, + force_task_update=False, is_create=True): acr_client_registries = cf_acr_registries(cmd.cli_ctx, None) registry = acr_client_registries.get(resource_group_name, registry_name) @@ -38,14 +39,17 @@ def _perform_continuous_patch_operation(cmd, validate_inputs(schedule, config) if not is_create: - validate_cssc_optional_inputs(config, schedule) + validate_cssc_optional_inputs(config, schedule, force_task_update) logger.debug('validations completed successfully.') if dryrun: - dryrun_output = acr_cssc_dry_run(cmd, registry=registry, config_file_path=config, is_create=is_create) + dryrun_output = acr_cssc_dry_run(cmd, + registry=registry, + config_file_path=config, + is_create=is_create) print(dryrun_output) else: - create_update_continuous_patch_v1(cmd, registry, config, schedule, dryrun, run_immediately, is_create) + create_update_continuous_patch_v1(cmd, registry, config, schedule, dryrun, run_immediately, is_create, force_task_update) def create_acrcssc(cmd, @@ -65,6 +69,7 @@ def create_acrcssc(cmd, schedule, dryrun, run_immediately, + force_task_update=False, is_create=True) @@ -75,9 +80,10 @@ def update_acrcssc(cmd, config, schedule, dryrun=False, - run_immediately=False): + run_immediately=False, + force_task_update=False): '''Update a continuous patch task in the registry.''' - logger.debug(f'Entering update_acrcssc with parameters: {registry_name} {workflow_type} {config} {schedule} {dryrun} {run_immediately}') + logger.debug(f'Entering update_acrcssc with parameters: {registry_name} {workflow_type} {config} {schedule} {dryrun} {run_immediately} {force_task_update}') _perform_continuous_patch_operation(cmd, resource_group_name, registry_name, @@ -85,6 +91,7 @@ def update_acrcssc(cmd, schedule, dryrun, run_immediately, + force_task_update, is_create=False) diff --git a/src/acrcssc/azext_acrcssc/helper/_deployment.py b/src/acrcssc/azext_acrcssc/helper/_deployment.py index e92cb7f553d..d867070566c 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 ) diff --git a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py index 6e6dc1d7c52..69b30bf5d4f 100644 --- a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py +++ b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py @@ -43,13 +43,27 @@ 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, + force_task_update=False): + 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) + else: + logger.debug("Schedule not provided, will attempt to get the current schedule from the task") + schedule_cron_expression = _get_continuous_patch_v1_trigger_schedule(cmd, registry) + logger.debug(f"converted schedule to cron expression: {schedule_cron_expression}") + cssc_tasks_exists = check_continuous_task_exists(cmd, registry) if is_create_workflow: if cssc_tasks_exists: @@ -58,25 +72,33 @@ 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) + + # if the force_task_update flag is set, we will update the task yaml via ARM deployment, + # and that will also update the schedule. If only the schedule is updated we can chage + # that through client call. The configuration will need to be updated separately + if force_task_update: + logger.debug("Force task update flag is set, updating the task definition") + _create_cssc_workflow(cmd, registry, schedule_cron_expression, resource_group, dryrun) + elif schedule is not None: + _update_task_schedule(cmd, registry, schedule_cron_expression, resource_group, dryrun) 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 _get_continuous_patch_v1_trigger_schedule(cmd, registry): + task = get_task(cmd, registry, CONTINUOUSPATCH_TASK_SCANREGISTRY_NAME) + trigger = task.trigger + if trigger and trigger.timer_triggers: + return trigger.timer_triggers[0].schedule + return None + + def _create_cssc_workflow(cmd, registry, schedule_cron_expression, resource_group, dry_run): parameters = { "AcrName": {"value": registry.name}, @@ -102,11 +124,6 @@ def _create_cssc_workflow(cmd, registry, schedule_cron_expression, resource_grou logger.warning(f"Deployment of {CONTINUOUS_PATCHING_WORKFLOW_NAME} tasks completed successfully.") -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) - - def _eval_trigger_run(cmd, registry, resource_group, run_immediately): if run_immediately: logger.warning(f'Triggering the {CONTINUOUSPATCH_TASK_SCANREGISTRY_NAME} to run immediately') From d6cb7e6f5e8a3359c06476af19516fed5975c5ea Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Fri, 14 Feb 2025 15:04:16 -0800 Subject: [PATCH 2/5] switch the feature to silently check and redeploy the tasks if the deployed version do not match the extension task definition --- src/acrcssc/azext_acrcssc/_params.py | 1 - src/acrcssc/azext_acrcssc/_validators.py | 39 +++++----- src/acrcssc/azext_acrcssc/cssc.py | 17 ++--- .../azext_acrcssc/helper/_deployment.py | 16 ++-- .../azext_acrcssc/helper/_taskoperations.py | 75 ++++++++++--------- .../latest/test_helper_taskoperations.py | 2 +- 6 files changed, 70 insertions(+), 80 deletions(-) diff --git a/src/acrcssc/azext_acrcssc/_params.py b/src/acrcssc/azext_acrcssc/_params.py index ca7c83d39cf..5305a15f1d6 100644 --- a/src/acrcssc/azext_acrcssc/_params.py +++ b/src/acrcssc/azext_acrcssc/_params.py @@ -29,7 +29,6 @@ def load_arguments(self: AzCommandsLoader, _): c.argument("schedule", options_list=["--schedule"], help="schedule to run the scan and patching task. E.g. `d` where n is the number of days between each run. Max value is 30d.", required=False) c.argument("run_immediately", options_list=["--run-immediately"], help="Set this flag to trigger the immediate run of the selected workflow task. Default value: false.", arg_type=get_three_state_flag(), required=False) c.argument("dryrun", options_list=["--dry-run"], help="Use this flag to see the qualifying repositories and tags that would be affected by the workflow. Default value: false. 'config' parameter is mandatory to provide with dry-run", arg_type=get_three_state_flag(), required=False) - c.argument("force_task_update", options_list=["--force-task-update"], help="Use this flag to update the Workflow's Task definition to the latest version. This should be done only when the CLI extension is updated to ensure compatibility with the latest server-side code.", arg_type=get_three_state_flag(), required=False) with self.argument_context("acr supply-chain workflow list") as c: c.argument("status", arg_type=get_enum_type(WorkflowTaskState), options_list=["--run-status"], help="Status to filter the supply-chain workflow image status.", required=False) diff --git a/src/acrcssc/azext_acrcssc/_validators.py b/src/acrcssc/azext_acrcssc/_validators.py index a5bcf4e0581..43014785366 100644 --- a/src/acrcssc/azext_acrcssc/_validators.py +++ b/src/acrcssc/azext_acrcssc/_validators.py @@ -78,11 +78,20 @@ def _validate_continuouspatch_config(config): raise InvalidArgumentValueError(f"Configuration error: Version {config.get('version', '')} is not supported. Supported versions are {CONTINUOUSPATCH_CONFIG_SUPPORTED_VERSIONS}") -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 +# to save on API calls, we offer the option to return the list of tasks +def check_continuous_task_exists(cmd, registry, task_list=None): + exists = True + 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) + exists = exists and task is not None + if task_list is not None and task is not None: + task_list.append(task) + return exists + except Exception as exception: + logger.debug(f"Failed to find task {task_name} from registry {registry.name} : {exception}") + return False def check_continuous_task_config_exists(cmd, registry): @@ -106,20 +115,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: @@ -146,6 +141,6 @@ def validate_task_type(task_type): raise InvalidArgumentValueError(error_msg=ERROR_MESSAGE_INVALID_TASK) -def validate_cssc_optional_inputs(cssc_config_path, schedule, force_task_update): - if cssc_config_path is None and schedule is None and force_task_update is False: - raise InvalidArgumentValueError(error_msg="Provide at least one parameter to update: --schedule, --config, or --force-task-update") +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, --config") diff --git a/src/acrcssc/azext_acrcssc/cssc.py b/src/acrcssc/azext_acrcssc/cssc.py index f185d79ddc2..ba227b76434 100644 --- a/src/acrcssc/azext_acrcssc/cssc.py +++ b/src/acrcssc/azext_acrcssc/cssc.py @@ -31,7 +31,6 @@ def _perform_continuous_patch_operation(cmd, schedule, dryrun=False, run_immediately=False, - force_task_update=False, is_create=True): acr_client_registries = cf_acr_registries(cmd.cli_ctx, None) registry = acr_client_registries.get(resource_group_name, registry_name) @@ -39,17 +38,14 @@ def _perform_continuous_patch_operation(cmd, validate_inputs(schedule, config) if not is_create: - validate_cssc_optional_inputs(config, schedule, force_task_update) + validate_cssc_optional_inputs(config, schedule) logger.debug('validations completed successfully.') if dryrun: - dryrun_output = acr_cssc_dry_run(cmd, - registry=registry, - config_file_path=config, - is_create=is_create) + dryrun_output = acr_cssc_dry_run(cmd, registry=registry, config_file_path=config, is_create=is_create) print(dryrun_output) else: - create_update_continuous_patch_v1(cmd, registry, config, schedule, dryrun, run_immediately, is_create, force_task_update) + create_update_continuous_patch_v1(cmd, registry, config, schedule, dryrun, run_immediately, is_create) def create_acrcssc(cmd, @@ -69,7 +65,6 @@ def create_acrcssc(cmd, schedule, dryrun, run_immediately, - force_task_update=False, is_create=True) @@ -80,10 +75,9 @@ def update_acrcssc(cmd, config, schedule, dryrun=False, - run_immediately=False, - force_task_update=False): + run_immediately=False): '''Update a continuous patch task in the registry.''' - logger.debug(f'Entering update_acrcssc with parameters: {registry_name} {workflow_type} {config} {schedule} {dryrun} {run_immediately} {force_task_update}') + logger.debug(f'Entering update_acrcssc with parameters: {registry_name} {workflow_type} {config} {schedule} {dryrun} {run_immediately}') _perform_continuous_patch_operation(cmd, resource_group_name, registry_name, @@ -91,7 +85,6 @@ def update_acrcssc(cmd, schedule, dryrun, run_immediately, - force_task_update, is_create=False) diff --git a/src/acrcssc/azext_acrcssc/helper/_deployment.py b/src/acrcssc/azext_acrcssc/helper/_deployment.py index d867070566c..2fd78437cf9 100644 --- a/src/acrcssc/azext_acrcssc/helper/_deployment.py +++ b/src/acrcssc/azext_acrcssc/helper/_deployment.py @@ -27,7 +27,8 @@ def validate_and_deploy_template(cmd_ctx, deployment_name: str, template_file_name: str, parameters: dict, - dryrun: Optional[bool] = False): + dryrun: Optional[bool] = False, + silent_execution: Optional[bool] = False): logger.debug(f'Working with resource group {resource_group}, registry {registry} template {template_file_name}') deployment_path = os.path.dirname( @@ -43,18 +44,18 @@ def validate_and_deploy_template(cmd_ctx, parameters=parameters, mode=DeploymentMode.incremental) try: - validate_template(cmd_ctx, resource_group, deployment_name, template) + validate_template(cmd_ctx, resource_group, deployment_name, template, silent_execution) if (dryrun): logger.debug("Dry run, skipping deployment") return None - return deploy_template(cmd_ctx, resource_group, deployment_name, template) + return deploy_template(cmd_ctx, resource_group, deployment_name, template, silent_execution) except Exception as exception: logger.debug(f'Failed to validate and deploy template: {exception}') raise AzCLIError(f'Failed to validate and deploy template: {exception}') -def validate_template(cmd_ctx, resource_group, deployment_name, template): +def validate_template(cmd_ctx, resource_group, deployment_name, template, silent_execution): # Validation is automatically re-attempted in live runs, but not in test # playback, causing them to fail. This explicitly re-attempts validation to # ensure the tests pass @@ -77,7 +78,8 @@ def validate_template(cmd_ctx, resource_group, deployment_name, template): ) ) validation_res = LongRunningOperation( - cmd_ctx, "Validating ARM template..." + cmd_ctx, "Validating ARM template...", + progress_bar=None if silent_execution else "Running validation" )(validation) break except Exception: # pylint: disable=broad-except @@ -109,7 +111,7 @@ def validate_template(cmd_ctx, resource_group, deployment_name, template): logger.debug("Successfully validated resources for {resource_group}") -def deploy_template(cmd_ctx, resource_group, deployment_name, template): +def deploy_template(cmd_ctx, resource_group, deployment_name, template, silent_execution): api_client = cf_resources(cmd_ctx) deployment = Deployment( @@ -128,7 +130,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" + progress_bar=None if silent_execution else "Deploying ARM template" )(poller) logger.debug("Finished deploying") diff --git a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py index 69b30bf5d4f..461a2a8738d 100644 --- a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py +++ b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py @@ -49,8 +49,7 @@ def create_update_continuous_patch_v1(cmd, schedule, dryrun, run_immediately, - is_create_workflow=True, - force_task_update=False): + is_create_workflow=True): logger.debug(f"Entering continuousPatchV1_creation {cssc_config_file} {dryrun} {run_immediately}") @@ -58,13 +57,11 @@ def create_update_continuous_patch_v1(cmd, schedule_cron_expression = None if schedule is not None: schedule_cron_expression = convert_timespan_to_cron(schedule) - else: - logger.debug("Schedule not provided, will attempt to get the current schedule from the task") - schedule_cron_expression = _get_continuous_patch_v1_trigger_schedule(cmd, registry) logger.debug(f"converted schedule to cron expression: {schedule_cron_expression}") - cssc_tasks_exists = check_continuous_task_exists(cmd, registry) + task_list = [] + cssc_tasks_exists = check_continuous_task_exists(cmd, registry, task_list=task_list) 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.") @@ -73,14 +70,7 @@ def create_update_continuous_patch_v1(cmd, 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.") - # if the force_task_update flag is set, we will update the task yaml via ARM deployment, - # and that will also update the schedule. If only the schedule is updated we can chage - # that through client call. The configuration will need to be updated separately - if force_task_update: - logger.debug("Force task update flag is set, updating the task definition") - _create_cssc_workflow(cmd, registry, schedule_cron_expression, resource_group, dryrun) - elif schedule is not None: - _update_task_schedule(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) @@ -91,15 +81,7 @@ def create_update_continuous_patch_v1(cmd, print(f"Continuous Patching workflow scheduled to run next at: {next_date} UTC") -def _get_continuous_patch_v1_trigger_schedule(cmd, registry): - task = get_task(cmd, registry, CONTINUOUSPATCH_TASK_SCANREGISTRY_NAME) - trigger = task.trigger - if trigger and trigger.timer_triggers: - return trigger.timer_triggers[0].schedule - return None - - -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}, @@ -118,10 +100,41 @@ def _create_cssc_workflow(cmd, registry, schedule_cron_expression, resource_grou CONTINUOUSPATCH_DEPLOYMENT_NAME, CONTINUOUSPATCH_DEPLOYMENT_TEMPLATE, parameters, - dry_run + dry_run, + silent_execution=silent_execution ) - logger.warning(f"Deployment of {CONTINUOUS_PATCHING_WORKFLOW_NAME} tasks completed successfully.") + if not silent_execution: + logger.info(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: + if task.name not in CONTINUOUSPATCH_ALL_TASK_NAMES: + logger.debug(f"Task {task.name} is not part of the continuous patching workflow, skipping update") + continue + 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") + + # TODO this is wrong, we don't know if the current taks is the trigger with the schedule + 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") + 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 + + if schedule_cron_expression is not None: + _update_task_schedule(cmd, registry, schedule_cron_expression, resource_group, dry_run) def _eval_trigger_run(cmd, registry, resource_group, run_immediately): @@ -501,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..8b81feb960b 100644 --- a/src/acrcssc/azext_acrcssc/tests/latest/test_helper_taskoperations.py +++ b/src/acrcssc/azext_acrcssc/tests/latest/test_helper_taskoperations.py @@ -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() From 2919af76838a8b292d7d1d02ea8f1009a822ec87 Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Fri, 14 Feb 2025 15:30:13 -0800 Subject: [PATCH 3/5] remove incorrect parameter from deployment's LongRunningOperation --- src/acrcssc/azext_acrcssc/helper/_deployment.py | 17 +++++++---------- .../azext_acrcssc/helper/_taskoperations.py | 7 ++++--- 2 files changed, 11 insertions(+), 13 deletions(-) diff --git a/src/acrcssc/azext_acrcssc/helper/_deployment.py b/src/acrcssc/azext_acrcssc/helper/_deployment.py index 2fd78437cf9..e263822ced8 100644 --- a/src/acrcssc/azext_acrcssc/helper/_deployment.py +++ b/src/acrcssc/azext_acrcssc/helper/_deployment.py @@ -27,8 +27,7 @@ def validate_and_deploy_template(cmd_ctx, deployment_name: str, template_file_name: str, parameters: dict, - dryrun: Optional[bool] = False, - silent_execution: Optional[bool] = False): + dryrun: Optional[bool] = False): logger.debug(f'Working with resource group {resource_group}, registry {registry} template {template_file_name}') deployment_path = os.path.dirname( @@ -44,18 +43,18 @@ def validate_and_deploy_template(cmd_ctx, parameters=parameters, mode=DeploymentMode.incremental) try: - validate_template(cmd_ctx, resource_group, deployment_name, template, silent_execution) + validate_template(cmd_ctx, resource_group, deployment_name, template) if (dryrun): logger.debug("Dry run, skipping deployment") return None - return deploy_template(cmd_ctx, resource_group, deployment_name, template, silent_execution) + return deploy_template(cmd_ctx, resource_group, deployment_name, template) except Exception as exception: logger.debug(f'Failed to validate and deploy template: {exception}') raise AzCLIError(f'Failed to validate and deploy template: {exception}') -def validate_template(cmd_ctx, resource_group, deployment_name, template, silent_execution): +def validate_template(cmd_ctx, resource_group, deployment_name, template): # Validation is automatically re-attempted in live runs, but not in test # playback, causing them to fail. This explicitly re-attempts validation to # ensure the tests pass @@ -78,8 +77,7 @@ def validate_template(cmd_ctx, resource_group, deployment_name, template, silent ) ) validation_res = LongRunningOperation( - cmd_ctx, "Validating ARM template...", - progress_bar=None if silent_execution else "Running validation" + cmd_ctx, "Validating ARM template..." )(validation) break except Exception: # pylint: disable=broad-except @@ -111,7 +109,7 @@ def validate_template(cmd_ctx, resource_group, deployment_name, template, silent logger.debug("Successfully validated resources for {resource_group}") -def deploy_template(cmd_ctx, resource_group, deployment_name, template, silent_execution): +def deploy_template(cmd_ctx, resource_group, deployment_name, template): api_client = cf_resources(cmd_ctx) deployment = Deployment( @@ -129,8 +127,7 @@ def deploy_template(cmd_ctx, resource_group, deployment_name, template, silent_e # Wait for the deployment to complete and get the outputs deployment: DeploymentExtended = LongRunningOperation( - cmd_ctx, - progress_bar=None if silent_execution else "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 461a2a8738d..d5dfa7dfb18 100644 --- a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py +++ b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py @@ -100,12 +100,11 @@ def _create_cssc_workflow(cmd, registry, schedule_cron_expression, resource_grou CONTINUOUSPATCH_DEPLOYMENT_NAME, CONTINUOUSPATCH_DEPLOYMENT_TEMPLATE, parameters, - dry_run, - silent_execution=silent_execution + dry_run ) if not silent_execution: - logger.info(f"Deployment of {CONTINUOUS_PATCHING_WORKFLOW_NAME} tasks completed successfully.") + 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): @@ -132,6 +131,8 @@ def _update_cssc_workflow(cmd, registry, schedule_cron_expression, resource_grou # 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") if schedule_cron_expression is not None: _update_task_schedule(cmd, registry, schedule_cron_expression, resource_group, dry_run) From da4b689e874a13ac7d7f64ed8d9c590450abd8a6 Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Thu, 20 Feb 2025 15:22:27 -0800 Subject: [PATCH 4/5] address review comments --- src/acrcssc/azext_acrcssc/_validators.py | 23 +++++---- .../azext_acrcssc/helper/_taskoperations.py | 19 ++++---- .../latest/test_helper_taskoperations.py | 12 ++--- .../tests/latest/test_validators.py | 48 +++++++++---------- 4 files changed, 52 insertions(+), 50 deletions(-) diff --git a/src/acrcssc/azext_acrcssc/_validators.py b/src/acrcssc/azext_acrcssc/_validators.py index 43014785366..e5846b1d27d 100644 --- a/src/acrcssc/azext_acrcssc/_validators.py +++ b/src/acrcssc/azext_acrcssc/_validators.py @@ -78,20 +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 offer the option to return the list of tasks -def check_continuous_task_exists(cmd, registry, task_list=None): - exists = True +# to save on API calls, we the list of tasks found in the registry +def check_continuous_task_exists(cmd, registry): + 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) - exists = exists and task is not None - if task_list is not None and task is not None: + if task is None: + missing_tasks.append(task_name) + else: task_list.append(task) - return exists + + 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 task {task_name} from registry {registry.name} : {exception}") - return False + logger.debug(f"Failed to find tasks from registry {registry.name} : {exception}") + return False, task_list def check_continuous_task_config_exists(cmd, registry): diff --git a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py index d5dfa7dfb18..3fda4ac6a12 100644 --- a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py +++ b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py @@ -60,8 +60,7 @@ def create_update_continuous_patch_v1(cmd, logger.debug(f"converted schedule to cron expression: {schedule_cron_expression}") - task_list = [] - cssc_tasks_exists = check_continuous_task_exists(cmd, registry, task_list=task_list) + 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.") @@ -112,15 +111,11 @@ def _update_cssc_workflow(cmd, registry, schedule_cron_expression, resource_grou # 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: - if task.name not in CONTINUOUSPATCH_ALL_TASK_NAMES: - logger.debug(f"Task {task.name} is not part of the continuous patching workflow, skipping update") - continue 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") - # TODO this is wrong, we don't know if the current taks is the trigger with the schedule 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: @@ -131,7 +126,7 @@ def _update_cssc_workflow(cmd, registry, schedule_cron_expression, resource_grou # 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") if schedule_cron_expression is not None: @@ -149,7 +144,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) @@ -167,8 +162,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 @@ -185,7 +180,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) 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 8b81feb960b..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() @@ -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 From 3657b4e5ca10338d4fdc04525f32cb6f8a6f4a6e Mon Sep 17 00:00:00 2001 From: Cesar Gray <52045802+cegraybl@users.noreply.github.com> Date: Thu, 20 Feb 2025 15:29:21 -0800 Subject: [PATCH 5/5] Update src/acrcssc/azext_acrcssc/helper/_taskoperations.py add checks for the trigger task timer before attempting to update Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/acrcssc/azext_acrcssc/helper/_taskoperations.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py index 3fda4ac6a12..3ec2d73e3a6 100644 --- a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py +++ b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py @@ -120,6 +120,8 @@ def _update_cssc_workflow(cmd, registry, schedule_cron_expression, resource_grou 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)