Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(cssc): 31231619 Enhance update command to check for Task yaml changes #13

Open
wants to merge 5 commits into
base: feature/cssc_ext
Choose a base branch
from
Open

feat(cssc): 31231619 Enhance update command to check for Task yaml changes #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
7eeebb5
add an option to update all tasks yamls throught an ARM redeploy
cegraybl Feb 5, 2025
d6cb7e6
switch the feature to silently check and redeploy the tasks if the de…
cegraybl Feb 14, 2025
2919af7
remove incorrect parameter from deployment's LongRunningOperation
cegraybl Feb 14, 2025
da4b689
address review comments
cegraybl Feb 20, 2025
3657b4e
Update src/acrcssc/azext_acrcssc/helper/_taskoperations.py
cegraybl Feb 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 21 additions & 19 deletions src/acrcssc/azext_acrcssc/_validators.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand All @@ -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:
Expand Down Expand Up @@ -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")
14 changes: 9 additions & 5 deletions src/acrcssc/azext_acrcssc/helper/_deployment.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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
)

Expand All @@ -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")

Expand Down
78 changes: 48 additions & 30 deletions src/acrcssc/azext_acrcssc/helper/_taskoperations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,41 +43,44 @@
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.")
_create_cssc_workflow(cmd, registry, schedule_cron_expression, resource_group, dryrun)
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},
Expand All @@ -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:
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than checking the content, shall we also check if schedule_cron_expression is null? Either change should trigger a task update.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the function gets schedule_cron_expression as None, it will retrieve the cron from the trigger task.
Do you mean that we should check if the returned task also has a null cron expression (i.e. the customer modified the task to remove the schedule?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There're two scenarios where we should update the task, either when the task content (yaml) changes or schedul_cron_expression schedule is modified, right? If schedul_cron_expression is non, it means "No update", else it might means "update". So when to decide if to update the task, it should be based on 1. whether deployed_task is equal to extension_task 2. Whether schedule_cron_expression is null, e.g.

if deployed_task != extension_task or schedule_cron_expression is None:
...

In your current code, you only check if the task is equal or not. If it is not equal, you then check the schedule_cron_expression.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, the case when the deployed task and the extension task are the same and the schedule needs to be updated is on line 137 under _update_task_schedule.

This section is meant to handle only when the tasks are different, and if we don't have a cron expression (only the configuration is being updated) we need to retrieve it from the task to keep it the same, as the ARM template includes it as as parameter.

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)

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

Expand All @@ -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)
Expand Down Expand Up @@ -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
14 changes: 7 additions & 7 deletions src/acrcssc/azext_acrcssc/tests/latest/test_helper_taskoperations.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand All @@ -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()
Expand Down
Loading