From 1d85771a912ffe6edebc1e778dcc9fd29aaaeca1 Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Wed, 9 Apr 2025 16:56:58 -0700 Subject: [PATCH 01/11] perform validation on sku, do not allow create/update operation on Basic registry SKU --- src/acrcssc/azext_acrcssc/_validators.py | 5 ++++- src/acrcssc/azext_acrcssc/cssc.py | 2 +- src/acrcssc/azext_acrcssc/helper/_constants.py | 1 + 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/acrcssc/azext_acrcssc/_validators.py b/src/acrcssc/azext_acrcssc/_validators.py index 38bf4bf4a4c..55047a3ebe9 100644 --- a/src/acrcssc/azext_acrcssc/_validators.py +++ b/src/acrcssc/azext_acrcssc/_validators.py @@ -22,6 +22,7 @@ CONTINUOUSPATCH_SCHEDULE_MAX_DAYS, ERROR_MESSAGE_INVALID_TIMESPAN_FORMAT, ERROR_MESSAGE_INVALID_TIMESPAN_VALUE, + REGISTRY_BASIC_SKU, SUBSCRIPTION) from .helper._constants import CSSCTaskTypes, ERROR_MESSAGE_INVALID_TASK, RECOMMENDATION_SCHEDULE from .helper._ociartifactoperations import _get_acr_token @@ -134,7 +135,9 @@ def _validate_schedule(schedule): raise InvalidArgumentValueError(error_msg=ERROR_MESSAGE_INVALID_TIMESPAN_VALUE, recommendation=RECOMMENDATION_SCHEDULE) -def validate_inputs(schedule, config_file_path=None, dryrun=False, run_immediately=False): +def validate_inputs(registry, schedule, config_file_path=None, dryrun=False, run_immediately=False): + if registry is None or registry.sku.name.lower() == REGISTRY_BASIC_SKU: + raise InvalidArgumentValueError(error_msg="This operation is not supported for registries in Basic SKU. Please use Standard or Premium SKU.") _validate_schedule(schedule) if config_file_path is not None: validate_continuouspatch_config_v1(config_file_path) diff --git a/src/acrcssc/azext_acrcssc/cssc.py b/src/acrcssc/azext_acrcssc/cssc.py index 7636ebc32b1..874d780cd74 100644 --- a/src/acrcssc/azext_acrcssc/cssc.py +++ b/src/acrcssc/azext_acrcssc/cssc.py @@ -37,7 +37,7 @@ def _perform_continuous_patch_operation(cmd, acr_client_registries = cf_acr_registries(cmd.cli_ctx, None) registry = acr_client_registries.get(resource_group_name, registry_name) - validate_inputs(schedule, config, dryrun, run_immediately) + validate_inputs(registry, schedule, config, dryrun, run_immediately) if not is_create: validate_cssc_optional_inputs(config, schedule) diff --git a/src/acrcssc/azext_acrcssc/helper/_constants.py b/src/acrcssc/azext_acrcssc/helper/_constants.py index 9e203698718..fa982bd15fe 100644 --- a/src/acrcssc/azext_acrcssc/helper/_constants.py +++ b/src/acrcssc/azext_acrcssc/helper/_constants.py @@ -36,6 +36,7 @@ class TaskRunStatus(Enum): RESOURCE_GROUP = "resource_group" SUBSCRIPTION = "subscription" TMP_DRY_RUN_FILE_NAME = "tmp_dry_run_template.yaml" +REGISTRY_BASIC_SKU = "basic" # Continuous Patch Constants From 7986e2d5c88f79d84863ab1e8ecb200210b3cd8a Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Fri, 11 Apr 2025 10:47:33 -0700 Subject: [PATCH 02/11] remove incorrect validation from RG, add space on help text --- src/acrcssc/azext_acrcssc/_params.py | 2 +- .../azext_acrcssc/templates/acrcli/artifact.json | 12 ------------ 2 files changed, 1 insertion(+), 13 deletions(-) delete mode 100644 src/acrcssc/azext_acrcssc/templates/acrcli/artifact.json diff --git a/src/acrcssc/azext_acrcssc/_params.py b/src/acrcssc/azext_acrcssc/_params.py index f4c06123dbd..c3e88a5394b 100644 --- a/src/acrcssc/azext_acrcssc/_params.py +++ b/src/acrcssc/azext_acrcssc/_params.py @@ -15,7 +15,7 @@ def load_arguments(self: AzCommandsLoader, _): from .helper._workflow_status import WorkflowTaskState with self.argument_context("acr supply-chain workflow") as c: - c.argument('resource_group', options_list=['--resource-group', '-g'], help='Name of resource group.You can configure the default group using `az configure --defaults group=`', completer=get_resource_name_completion_list(REGISTRY_RESOURCE_TYPE), configured_default='acr', validator=validate_registry_name) + c.argument('resource_group', options_list=['--resource-group', '-g'], help='Name of resource group. You can configure the default group using `az configure --defaults group=`', completer=get_resource_name_completion_list(REGISTRY_RESOURCE_TYPE)) c.argument('registry_name', options_list=['--registry', '-r'], help='The name of the container registry. It should be specified in lower case. You can configure the default registry name using `az configure --defaults acr=`', completer=get_resource_name_completion_list(REGISTRY_RESOURCE_TYPE), configured_default='acr', validator=validate_registry_name) c.argument("workflow_type", arg_type=get_enum_type(CSSCTaskTypes), options_list=['--type', '-t'], help="Type of workflow task.", required=True) diff --git a/src/acrcssc/azext_acrcssc/templates/acrcli/artifact.json b/src/acrcssc/azext_acrcssc/templates/acrcli/artifact.json deleted file mode 100644 index 57b2dbb85c4..00000000000 --- a/src/acrcssc/azext_acrcssc/templates/acrcli/artifact.json +++ /dev/null @@ -1,12 +0,0 @@ -{ - "repositories": [ - { - "enabled": true, - "repository": "python", - "tags": [ - "*" - ] - } - ], - "version": "v1" -} \ No newline at end of file From 4846be48e0722fc9b9088c05ae3e482c4b44fa9c Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Fri, 11 Apr 2025 10:48:01 -0700 Subject: [PATCH 03/11] remove comments --- src/acrcssc/azext_acrcssc/helper/_constants.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/acrcssc/azext_acrcssc/helper/_constants.py b/src/acrcssc/azext_acrcssc/helper/_constants.py index fa982bd15fe..364410bf83d 100644 --- a/src/acrcssc/azext_acrcssc/helper/_constants.py +++ b/src/acrcssc/azext_acrcssc/helper/_constants.py @@ -11,8 +11,6 @@ class CSSCTaskTypes(Enum): """Enum for the task type.""" ContinuousPatchV1 = 'continuouspatchv1' - # CopaV1 = "CopaV1" - # TrivyV1 = "TrivyV1" class TaskRunStatus(Enum): From 0762e77d8cef58f2fc7733b3225273b243215fc8 Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Fri, 11 Apr 2025 10:48:41 -0700 Subject: [PATCH 04/11] fix task check validation to attempt to get all tasks --- src/acrcssc/azext_acrcssc/_validators.py | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/acrcssc/azext_acrcssc/_validators.py b/src/acrcssc/azext_acrcssc/_validators.py index 55047a3ebe9..41dd094614f 100644 --- a/src/acrcssc/azext_acrcssc/_validators.py +++ b/src/acrcssc/azext_acrcssc/_validators.py @@ -80,24 +80,25 @@ def _validate_continuouspatch_config(config): 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: + + acrtask_client = cf_acr_tasks(cmd.cli_ctx) + for task_name in CONTINUOUSPATCH_ALL_TASK_NAMES: + try: task = get_task(cmd, registry, task_name, acrtask_client) if task is None: missing_tasks.append(task_name) else: task_list.append(task) + except Exception as exception: + logger.debug(f"Failed to find tasks from registry {registry.name} : {exception}") + missing_tasks.append(task_name) - 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}") + 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 + def check_continuous_task_config_exists(cmd, registry): # A client cannot be used in this situation because the 'show registry/image' From c947f5ed3bd01fab4cbf2a7f84ec898d9ccf37c1 Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Fri, 11 Apr 2025 13:23:47 -0700 Subject: [PATCH 05/11] move workflow status properties to the dictionary initialization --- .../azext_acrcssc/helper/_workflow_status.py | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/src/acrcssc/azext_acrcssc/helper/_workflow_status.py b/src/acrcssc/azext_acrcssc/helper/_workflow_status.py index bbf68cc2c95..cbd69711ed7 100644 --- a/src/acrcssc/azext_acrcssc/helper/_workflow_status.py +++ b/src/acrcssc/azext_acrcssc/helper/_workflow_status.py @@ -356,19 +356,15 @@ def get_status(self): patch_task_id = WORKFLOW_STATUS_NOT_AVAILABLE if self.patch_task is None else self.patch_task.run_id patched_image = self._get_patched_image_name_from_tasklog() workflow_type = CSSCTaskTypes.ContinuousPatchV1.value - patch_skipped_reason = "" - scan_error_reason = "" - patch_error_reason = "" + # Initialize reasons only if needed + patch_skipped_reason = self._get_patch_skip_reason_from_tasklog() \ + if self.patch_status() == WorkflowTaskState.SKIPPED.value else "" - # this situation means that we don't have a patched image - if self.patch_status() == WorkflowTaskState.SKIPPED.value: - patch_skipped_reason = self._get_patch_skip_reason_from_tasklog() + scan_error_reason = self._get_scan_error_reason_from_tasklog() \ + if self.scan_status() == WorkflowTaskState.FAILED.value else "" - if self.scan_status() == WorkflowTaskState.FAILED.value: - scan_error_reason = self._get_scan_error_reason_from_tasklog() - - if self.patch_status() == WorkflowTaskState.FAILED.value: - patch_error_reason = self._get_patch_error_reason_from_tasklog() + patch_error_reason = self._get_patch_error_reason_from_tasklog() \ + if self.patch_status() == WorkflowTaskState.FAILED.value else "" if patched_image == self.image(): patched_image = WORKFLOW_STATUS_PATCH_NOT_AVAILABLE @@ -378,7 +374,11 @@ def get_status(self): "scan_status": scan_status, "scan_date": scan_date, "scan_task_ID": scan_task_id, - "patch_status": patch_status + "patch_status": patch_status, + "patch_date": patch_date, + "patch_task_ID": patch_task_id, + "last_patched_image": patched_image, + "workflow_type": workflow_type } if patch_skipped_reason != "": @@ -390,11 +390,6 @@ def get_status(self): if patch_error_reason != "": result["patch_error_reason"] = patch_error_reason - result["patch_date"] = patch_date - result["patch_task_ID"] = patch_task_id - result["last_patched_image"] = patched_image - result["workflow_type"] = workflow_type - return result def __str__(self) -> str: From 7aadd77ec5ef8c0b1552dd880b3998a113aa0b8b Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Mon, 14 Apr 2025 16:35:54 -0700 Subject: [PATCH 06/11] bump min az cli version to 2.60 --- src/acrcssc/azext_acrcssc/azext_metadata.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/acrcssc/azext_acrcssc/azext_metadata.json b/src/acrcssc/azext_acrcssc/azext_metadata.json index 30fdaf614ee..b165ea04b95 100644 --- a/src/acrcssc/azext_acrcssc/azext_metadata.json +++ b/src/acrcssc/azext_acrcssc/azext_metadata.json @@ -1,4 +1,4 @@ { "azext.isPreview": true, - "azext.minCliCoreVersion": "2.15.0" + "azext.minCliCoreVersion": "2.60.0" } \ No newline at end of file From 96ceafb728d716ed1e7f3d063e680bde14626ccf Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Mon, 14 Apr 2025 16:36:30 -0700 Subject: [PATCH 07/11] first wave of official PR comments --- src/acrcssc/azext_acrcssc/_validators.py | 4 +-- .../azext_acrcssc/helper/_deployment.py | 8 ++--- .../azext_acrcssc/helper/_taskoperations.py | 19 +++-------- .../azext_acrcssc/helper/_workflow_status.py | 32 ++++++++++--------- 4 files changed, 25 insertions(+), 38 deletions(-) diff --git a/src/acrcssc/azext_acrcssc/_validators.py b/src/acrcssc/azext_acrcssc/_validators.py index 41dd094614f..06b0400e73c 100644 --- a/src/acrcssc/azext_acrcssc/_validators.py +++ b/src/acrcssc/azext_acrcssc/_validators.py @@ -76,7 +76,7 @@ def _validate_continuouspatch_config(config): raise InvalidArgumentValueError("Configuration error: Tag '*' is not allowed with other tags in the same repository. Use '*' as the only tag in the repository to avoid overlaps.") -# to save on API calls, we the list of tasks found in the registry +# to save on API calls, we use task_list to return a list of CSSC tasks found in the registry def check_continuous_task_exists(cmd, registry): task_list = [] missing_tasks = [] @@ -140,7 +140,7 @@ def validate_inputs(registry, schedule, config_file_path=None, dryrun=False, run if registry is None or registry.sku.name.lower() == REGISTRY_BASIC_SKU: raise InvalidArgumentValueError(error_msg="This operation is not supported for registries in Basic SKU. Please use Standard or Premium SKU.") _validate_schedule(schedule) - if config_file_path is not None: + if config_file_path: validate_continuouspatch_config_v1(config_file_path) validate_run_type(dryrun, run_immediately) diff --git a/src/acrcssc/azext_acrcssc/helper/_deployment.py b/src/acrcssc/azext_acrcssc/helper/_deployment.py index 7058e35532d..6e2125a2542 100644 --- a/src/acrcssc/azext_acrcssc/helper/_deployment.py +++ b/src/acrcssc/azext_acrcssc/helper/_deployment.py @@ -44,13 +44,12 @@ def validate_and_deploy_template(cmd_ctx, mode=DeploymentMode.incremental) try: validate_template(cmd_ctx, resource_group, deployment_name, template) - if (dryrun): + if dryrun: logger.debug("Dry run, skipping deployment") return None 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}') @@ -61,10 +60,7 @@ def validate_template(cmd_ctx, resource_group, deployment_name, template): api_clients = cf_resources(cmd_ctx) validation_res = None deployment = Deployment( - properties=template, - # tags = { "test": CSSC_TAGS }, #we need to know if tagging - # is something that will help ust, tasks are proxy resources, - # so not sure how that would work + properties=template ) for validation_attempt in range(2): diff --git a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py index a81c4298f2d..feb5e5949cf 100644 --- a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py +++ b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py @@ -63,7 +63,7 @@ def create_update_continuous_patch_v1(cmd, schedule_cron_expression = None cssc_tasks_exists, task_list = check_continuous_task_exists(cmd, registry) - if schedule is not None: + if schedule: schedule_cron_expression = convert_timespan_to_cron(schedule) logger.debug(f"converted schedule to cron expression: {schedule_cron_expression}") @@ -71,7 +71,7 @@ def create_update_continuous_patch_v1(cmd, if cssc_tasks_exists: raise AzCLIError(f"{ERROR_MESSAGE_WORKFLOW_TASKS_ALREADY_EXISTS}") - if cssc_config_file is not None: + if cssc_config_file: create_oci_artifact_continuous_patch(registry, cssc_config_file, dryrun) logger.debug(f"Uploading of {cssc_config_file} for create completed successfully.") @@ -80,7 +80,7 @@ def create_update_continuous_patch_v1(cmd, if not cssc_tasks_exists: raise AzCLIError(f"{ERROR_MESSAGE_WORKFLOW_TASKS_DOES_NOT_EXIST}") - if cssc_config_file is not None: + if cssc_config_file: create_oci_artifact_continuous_patch(registry, cssc_config_file, dryrun) logger.debug(f"Uploading of {cssc_config_file} for update completed successfully.") @@ -136,7 +136,7 @@ def _update_cssc_workflow(cmd, registry, schedule_cron_expression, resource_grou logger.debug(f"Task {task.name} is different from the extension task, updating the task") _update_task_yaml(acr_task_client, registry, resource_group, task, extension_task) - if schedule_cron_expression is not None: + if schedule_cron_expression: _update_task_schedule(acr_task_client, registry, resource_group, schedule_cron_expression, dry_run) @@ -531,17 +531,6 @@ def _get_custom_registry_credentials(cmd): ) -def _is_vault_secret(cmd, credential): - keyvault_dns = None - try: - keyvault_dns = cmd.cli_ctx.cloud.suffixes.keyvault_dns - except Exception: - return False - if credential is not None: - return keyvault_dns.upper() in credential.upper() - return False - - def get_next_date(cron_expression): from croniter import croniter now = datetime.now(timezone.utc) diff --git a/src/acrcssc/azext_acrcssc/helper/_workflow_status.py b/src/acrcssc/azext_acrcssc/helper/_workflow_status.py index cbd69711ed7..d99ead186a4 100644 --- a/src/acrcssc/azext_acrcssc/helper/_workflow_status.py +++ b/src/acrcssc/azext_acrcssc/helper/_workflow_status.py @@ -204,15 +204,13 @@ def _get_patched_image_name_from_tasklog(self): return None repository, patched_tag, _ = self._get_scanning_repo_from_scan_task() - if repository is not None and patched_tag is not None: + if repository and patched_tag: return f"{repository}:{patched_tag}" - if self.patch_task is None: - return None - - match = re.search(r'Patched image pushed to (\S+)', self.patch_logs) - if match: - return match.group(1) + if self.patch_task: + match = re.search(r'Patched image pushed to (\S+)', self.patch_logs) + if match: + return match.group(1) return None @staticmethod @@ -301,7 +299,7 @@ def from_taskrun(cmd, # missing the patch task id means that the scan either failed, or succeeded and patching is not needed. # this is important, because patching status depends on both the patching task status (if it exists) # and the scan task status - if patch_task_id is not None: + if patch_task_id: # it is possible for the patch task to be mentioned in the logs, but the API has not returned the # taskrun for it yet, attempt to retrieve it from client try: @@ -441,6 +439,7 @@ def generate_logs(cmd, logger.debug("log file not found for run_id: %s, registry: %s, " "resource_group: %s -- exception: %s", run_id, registry_name, resource_group_name, e) + return "" if await_task_run: try: @@ -475,24 +474,27 @@ def get_run_status_local(client, resource_group_name, registry_name, run_id): @staticmethod def _download_logs(blob_service): - blob = blob_service.download_blob() - blob_text = blob.readall().decode('utf-8') - - return blob_text + try: + blob = blob_service.download_blob() + blob_text = blob.readall().decode('utf-8') + return blob_text + except AzCLIError as e: + logger.debug("Failed to download logs from blob: %s", e) + return "" @staticmethod def remove_internal_acr_statements(blob_content): logger.debug("Removing internal ACR statements from logs, blob content size: %s", len(blob_content)) - lines = blob_content.split("\n") + lines = blob_content.splitlines() starting_identifier = "DRY RUN mode enabled" terminating_identifier = "Total matches found" print_line = False output = "" for line in lines: - if line.startswith(starting_identifier): + if line.strip().startswith(starting_identifier): print_line = True - elif line.startswith(terminating_identifier): + elif line.strip().startswith(terminating_identifier): output += "\n" + line print_line = False From 9d237b97138fe259fe5af2e48c8ff5fe8d323b02 Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Tue, 15 Apr 2025 09:52:45 -0700 Subject: [PATCH 08/11] save work from second wave of comments --- src/acrcssc/README.rst | 1 + src/acrcssc/azext_acrcssc/_validators.py | 4 +--- src/acrcssc/azext_acrcssc/cssc.py | 2 +- .../azext_acrcssc/helper/_deployment.py | 5 +++-- .../helper/_ociartifactoperations.py | 20 +++++++++++++------ .../azext_acrcssc/helper/_taskoperations.py | 5 ++--- 6 files changed, 22 insertions(+), 15 deletions(-) diff --git a/src/acrcssc/README.rst b/src/acrcssc/README.rst index aa95d2474dd..4b378e42a1f 100644 --- a/src/acrcssc/README.rst +++ b/src/acrcssc/README.rst @@ -15,6 +15,7 @@ Continuous Patching is currently in preview. The following limitations apply: - Windows-based container images aren’t supported - Only "OS-level" vulnerabilities will be patched. This includes packages in the image managed by a package manager such as “apt” and “yum”. Vulnerabilities at the “application level” are unable to be patched, such as compiled languages like Go, Python, NodeJS - Patching is only supported in Public regions, not in Sovereign regions +- CSSC patching is not supported for registries or in regions where Tasks are unavailable. Features ======== diff --git a/src/acrcssc/azext_acrcssc/_validators.py b/src/acrcssc/azext_acrcssc/_validators.py index 06b0400e73c..5d0d8543edc 100644 --- a/src/acrcssc/azext_acrcssc/_validators.py +++ b/src/acrcssc/azext_acrcssc/_validators.py @@ -136,9 +136,7 @@ def _validate_schedule(schedule): raise InvalidArgumentValueError(error_msg=ERROR_MESSAGE_INVALID_TIMESPAN_VALUE, recommendation=RECOMMENDATION_SCHEDULE) -def validate_inputs(registry, schedule, config_file_path=None, dryrun=False, run_immediately=False): - if registry is None or registry.sku.name.lower() == REGISTRY_BASIC_SKU: - raise InvalidArgumentValueError(error_msg="This operation is not supported for registries in Basic SKU. Please use Standard or Premium SKU.") +def validate_inputs(schedule, config_file_path=None, dryrun=False, run_immediately=False): _validate_schedule(schedule) if config_file_path: validate_continuouspatch_config_v1(config_file_path) diff --git a/src/acrcssc/azext_acrcssc/cssc.py b/src/acrcssc/azext_acrcssc/cssc.py index 874d780cd74..7636ebc32b1 100644 --- a/src/acrcssc/azext_acrcssc/cssc.py +++ b/src/acrcssc/azext_acrcssc/cssc.py @@ -37,7 +37,7 @@ def _perform_continuous_patch_operation(cmd, acr_client_registries = cf_acr_registries(cmd.cli_ctx, None) registry = acr_client_registries.get(resource_group_name, registry_name) - validate_inputs(registry, schedule, config, dryrun, run_immediately) + validate_inputs(schedule, config, dryrun, run_immediately) if not is_create: validate_cssc_optional_inputs(config, schedule) diff --git a/src/acrcssc/azext_acrcssc/helper/_deployment.py b/src/acrcssc/azext_acrcssc/helper/_deployment.py index 6e2125a2542..abc6457dbf1 100644 --- a/src/acrcssc/azext_acrcssc/helper/_deployment.py +++ b/src/acrcssc/azext_acrcssc/helper/_deployment.py @@ -76,7 +76,8 @@ def validate_template(cmd_ctx, resource_group, deployment_name, template): cmd_ctx, "Validating ARM template..." )(validation) break - except Exception: # pylint: disable=broad-except + except Exception as exception: # pylint: disable=broad-except + logger.debug(f"Validation attempt {validation_attempt + 1} failed for template {template}, exception: {exception}") if validation_attempt == 1: raise @@ -134,7 +135,7 @@ def deploy_template(cmd_ctx, resource_group, deployment_name, template): logger.debug(f"Deployed: {deployment.name} {deployment.id} {depl_props}") if depl_props.provisioning_state != "Succeeded": - logger.debug(f"Failed to provision: {depl_props}") + logger.error(f"Failed to provision: {depl_props}") raise RuntimeError( "Deploy of template to resource group" f" {resource_group} proceeded but the provisioning" diff --git a/src/acrcssc/azext_acrcssc/helper/_ociartifactoperations.py b/src/acrcssc/azext_acrcssc/helper/_ociartifactoperations.py index 652cf289032..c1105294ff5 100644 --- a/src/acrcssc/azext_acrcssc/helper/_ociartifactoperations.py +++ b/src/acrcssc/azext_acrcssc/helper/_ociartifactoperations.py @@ -58,6 +58,7 @@ def create_oci_artifact_continuous_patch(registry, cssc_config_file, dryrun): else: oci_target_name = f"{CSSC_WORKFLOW_POLICY_REPOSITORY}/{CONTINUOUSPATCH_OCI_ARTIFACT_CONFIG}:{CONTINUOUSPATCH_OCI_ARTIFACT_CONFIG_TAG_V1}" + logger.debug(f"Publish OCI artifact to: {oci_target_name}") oras_client.push( target=oci_target_name, files=[temp_artifact_name]) @@ -77,8 +78,13 @@ def get_oci_artifact_continuous_patch(cmd, registry): oci_target_name = f"{CSSC_WORKFLOW_POLICY_REPOSITORY}/{CONTINUOUSPATCH_OCI_ARTIFACT_CONFIG}:{CONTINUOUSPATCH_OCI_ARTIFACT_CONFIG_TAG_V1}" + logger.debug(f"Pull OCI artifact from: {oci_target_name}") oci_artifacts = oras_client.pull(target=oci_target_name, overwrite=True) + + if not oci_artifacts: + raise AzCLIError(f"Failed to pull OCI artifact from ACR: {oci_target_name}") + trigger_task = get_task(cmd, registry, CONTINUOUSPATCH_TASK_SCANREGISTRY_NAME) file_name = oci_artifacts[0] logger.debug(f"OCI artifact file name: {file_name}, trigger task: {trigger_task}") @@ -86,7 +92,8 @@ def get_oci_artifact_continuous_patch(cmd, registry): except Exception as exception: raise AzCLIError(f"Failed to get OCI artifact from ACR: {exception}") finally: - oras_client.logout(hostname=str.lower(registry.login_server)) + if oras_client: + oras_client.logout(hostname=str.lower(registry.login_server)) return config, file_name @@ -98,19 +105,19 @@ def delete_oci_artifact_continuous_patch(cmd, registry): try: token = _get_acr_token(registry.name, subscription) - + oci_target_name = f"{CSSC_WORKFLOW_POLICY_REPOSITORY}/{CONTINUOUSPATCH_OCI_ARTIFACT_CONFIG}" # Delete repository, removing only image isn't deleting the repository always (Bug) acr_repository_delete( cmd=cmd, registry_name=registry.name, - repository=f"{CSSC_WORKFLOW_POLICY_REPOSITORY}/{CONTINUOUSPATCH_OCI_ARTIFACT_CONFIG}", + repository=oci_target_name, username=BEARER_TOKEN_USERNAME, password=token, yes=True) logger.debug("Call to acr_repository_delete completed successfully") except Exception as exception: logger.debug(exception) - logger.error(f"{CSSC_WORKFLOW_POLICY_REPOSITORY}/{CONTINUOUSPATCH_OCI_ARTIFACT_CONFIG}:{CONTINUOUSPATCH_OCI_ARTIFACT_CONFIG_TAG_V1} might not exist or attempt to delete failed.") + logger.error(f"{oci_target_name}:{CONTINUOUSPATCH_OCI_ARTIFACT_CONFIG_TAG_V1} might not exist or attempt to delete failed.") raise @@ -191,9 +198,10 @@ def from_json(self, json_str, trigger_task=None): try: json_config = json.loads(json_str) validate(json_config, CONTINUOUSPATCH_CONFIG_SCHEMA_V1) + except json.JSONDecodeError as e: + raise AzCLIError(f"Failed to parse JSON: {e}", e) except ValidationError as e: - logger.error("Error validating the continuous patch config file: %s", e) - return None + raise AzCLIError(f"Error validating the continuous patch config file: {e}", e) self.version = json_config.get("version", "") repositories = json_config.get("repositories", []) diff --git a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py index feb5e5949cf..af21bd499cf 100644 --- a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py +++ b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py @@ -71,9 +71,8 @@ def create_update_continuous_patch_v1(cmd, if cssc_tasks_exists: raise AzCLIError(f"{ERROR_MESSAGE_WORKFLOW_TASKS_ALREADY_EXISTS}") - if cssc_config_file: - create_oci_artifact_continuous_patch(registry, cssc_config_file, dryrun) - logger.debug(f"Uploading of {cssc_config_file} for create completed successfully.") + create_oci_artifact_continuous_patch(registry, cssc_config_file, dryrun) + logger.debug(f"Uploading of {cssc_config_file} for create completed successfully.") _create_cssc_workflow(cmd, registry, schedule_cron_expression, resource_group, dryrun) else: From ee44f4c6e1a33166180f380f2cee105b5d7e6f05 Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Tue, 15 Apr 2025 11:43:05 -0700 Subject: [PATCH 09/11] remove basic sku check that sneaked in again (old commit/rebase) --- src/acrcssc/azext_acrcssc/_validators.py | 1 - 1 file changed, 1 deletion(-) diff --git a/src/acrcssc/azext_acrcssc/_validators.py b/src/acrcssc/azext_acrcssc/_validators.py index 5d0d8543edc..afb7a91c3eb 100644 --- a/src/acrcssc/azext_acrcssc/_validators.py +++ b/src/acrcssc/azext_acrcssc/_validators.py @@ -22,7 +22,6 @@ CONTINUOUSPATCH_SCHEDULE_MAX_DAYS, ERROR_MESSAGE_INVALID_TIMESPAN_FORMAT, ERROR_MESSAGE_INVALID_TIMESPAN_VALUE, - REGISTRY_BASIC_SKU, SUBSCRIPTION) from .helper._constants import CSSCTaskTypes, ERROR_MESSAGE_INVALID_TASK, RECOMMENDATION_SCHEDULE from .helper._ociartifactoperations import _get_acr_token From 61cc4fa2a30502c445cb7120a855e5ff6b8ff632 Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Tue, 15 Apr 2025 11:43:29 -0700 Subject: [PATCH 10/11] additional wave of PR comments --- .../azext_acrcssc/helper/_taskoperations.py | 35 ++++++++++--------- .../azext_acrcssc/helper/_workflow_status.py | 29 +++++++-------- 2 files changed, 34 insertions(+), 30 deletions(-) diff --git a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py index af21bd499cf..864f2b895a3 100644 --- a/src/acrcssc/azext_acrcssc/helper/_taskoperations.py +++ b/src/acrcssc/azext_acrcssc/helper/_taskoperations.py @@ -475,23 +475,26 @@ def _delete_task_role_assignment(cli_ctx, acrtask_client, registry, resource_gro identity = task.identity - if identity: - assigned_roles = role_client.role_assignments.list_for_scope( - registry.id, - filter=f"principalId eq '{identity.principal_id}'" - ) + if not identity or not identity.principal_id: + logger.debug(f"Task {task_name} has no associated managed identity. Skipping role assignment deletion.") + return None + + assigned_roles = role_client.role_assignments.list_for_scope( + registry.id, + filter=f"principalId eq '{identity.principal_id}'" + ) - for role in assigned_roles: - try: - logger.debug(f"Deleting role assignments of task {task_name} from the registry") - role_client.role_assignments.delete( - scope=registry.id, - role_assignment_name=role.name - ) - except ResourceNotFoundError: - logger.debug(f"Role assignment {role.name} does not exist in registry {registry.name}") - except AzCLIError as exception: - logger.error(f"Failed to delete role assignment {role.name} from registry {registry.name} : {exception}") + for role in assigned_roles: + try: + logger.debug(f"Deleting role assignments of task {task_name} from the registry") + role_client.role_assignments.delete( + scope=registry.id, + role_assignment_name=role.name + ) + except ResourceNotFoundError: + logger.debug(f"Role assignment {role.name} does not exist in registry {registry.name}") + except AzCLIError as exception: + logger.error(f"Failed to delete role assignment {role.name} from registry {registry.name} : {exception}") def _transform_task_list(tasks): diff --git a/src/acrcssc/azext_acrcssc/helper/_workflow_status.py b/src/acrcssc/azext_acrcssc/helper/_workflow_status.py index d99ead186a4..323aabb158f 100644 --- a/src/acrcssc/azext_acrcssc/helper/_workflow_status.py +++ b/src/acrcssc/azext_acrcssc/helper/_workflow_status.py @@ -284,17 +284,17 @@ def from_taskrun(cmd, continue # need to check if we have the latest scan task - task = scan - logs = scan.task_log_result - if image in all_status: - task, logs = WorkflowTaskStatus._latest_task(all_status[image].scan_task, - all_status[image].scan_logs, - scan, scan.task_log_result) + all_status[image].scan_task, all_status[image].scan_logs = WorkflowTaskStatus._latest_task( + all_status[image].scan_task, + all_status[image].scan_logs, + scan, + scan.task_log_result) else: all_status[image] = WorkflowTaskStatus(image) - all_status[image].scan_task = task - all_status[image].scan_logs = logs + all_status[image].scan_task = scan + all_status[image].scan_logs = scan.task_log_result + patch_task_id = all_status[image].get_patch_task_from_scan_tasklog() # missing the patch task id means that the scan either failed, or succeeded and patching is not needed. # this is important, because patching status depends on both the patching task status (if it exists) @@ -302,17 +302,18 @@ def from_taskrun(cmd, if patch_task_id: # it is possible for the patch task to be mentioned in the logs, but the API has not returned the # taskrun for it yet, attempt to retrieve it from client - try: - patch_task = next((task for task in patch_taskruns if task.run_id == patch_task_id)) - except StopIteration: + patch_task = next((task for task in patch_taskruns if task.run_id == patch_task_id), None) + if patch_task is None: patch_task = WorkflowTaskStatus._get_missing_taskrun(taskrun_client, registry, patch_task_id) all_status[image].patch_task = patch_task - if WorkflowTaskStatus._task_status_to_workflow_status(patch_task) == WorkflowTaskState.FAILED.value: + if patch_task and WorkflowTaskStatus._task_status_to_workflow_status( + patch_task + ) == WorkflowTaskState.FAILED.value: failed_patch_tasklog_retrieval.append(all_status[image]) - if len(failed_patch_tasklog_retrieval) > 0: - taskrunList = [task.patch_task for task in failed_patch_tasklog_retrieval] + if failed_patch_tasklog_retrieval: + taskrunList = [task.patch_task for task in failed_patch_tasklog_retrieval if task.patch_task] WorkflowTaskStatus._retrieve_all_tasklogs(cmd, taskrun_client, registry, taskrunList, progress_indicator) for workflow_status in failed_patch_tasklog_retrieval: workflow_status.patch_logs = workflow_status.patch_task.task_log_result From 4817c3773354579a898b8192d017c7b9ababea1f Mon Sep 17 00:00:00 2001 From: Cesar Gray Blanco Date: Tue, 15 Apr 2025 11:50:59 -0700 Subject: [PATCH 11/11] initialize oras_client to None --- src/acrcssc/azext_acrcssc/helper/_ociartifactoperations.py | 1 + 1 file changed, 1 insertion(+) diff --git a/src/acrcssc/azext_acrcssc/helper/_ociartifactoperations.py b/src/acrcssc/azext_acrcssc/helper/_ociartifactoperations.py index c1105294ff5..2c1afbcc5b0 100644 --- a/src/acrcssc/azext_acrcssc/helper/_ociartifactoperations.py +++ b/src/acrcssc/azext_acrcssc/helper/_ociartifactoperations.py @@ -73,6 +73,7 @@ def get_oci_artifact_continuous_patch(cmd, registry): logger.debug("Entering get_oci_artifact_continuous_patch with parameter: %s", registry.login_server) config = None file_name = None + oras_client = None try: oras_client = _oras_client(registry)