From 6a849eec3e4382a29a900a0a93b78cf16552f449 Mon Sep 17 00:00:00 2001 From: Adrian Turjak Date: Wed, 23 Jan 2019 15:29:51 +1300 Subject: [PATCH] Fix an issue with tasks being approved when not yet valid Our internal auto-approval logic would approve a task before it was even actually valid. While it would still exit and not run, the fact that it was 'approved' still caused some minor edge cases we want to avoid. Change-Id: I078a56bb9647ccc7caa0485f0fa2a55d2da08048 --- adjutant/api/v1/tasks.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/adjutant/api/v1/tasks.py b/adjutant/api/v1/tasks.py index d6d8fb7..23718ef 100644 --- a/adjutant/api/v1/tasks.py +++ b/adjutant/api/v1/tasks.py @@ -195,8 +195,7 @@ class TaskView(APIViewWithLogger): email_conf = class_conf.get('emails', {}).get('initial', None) send_stage_email(task, email_conf) - action_models = task.actions - approve_list = [act.get_action().auto_approve for act in action_models] + approve_list = [act.auto_approve for act in action_instances] # TODO(amelia): It would be nice to explicitly test this, however # currently we don't have the right combinations of @@ -241,6 +240,13 @@ class TaskView(APIViewWithLogger): Will create a token if required, otherwise will run the submit steps. """ + # cannot approve an invalid task + action_models = task.actions + actions = [act.get_action() for act in action_models] + valid = all([act.valid for act in actions]) + if not valid: + return {'errors': ['actions invalid']}, 400 + # TODO(amelia): get action invalidation reasons # We approve the task before running actions, # that way if something goes wrong we know if it was approved, @@ -250,15 +256,8 @@ class TaskView(APIViewWithLogger): task.approved_by = request.keystone_user task.save() - action_models = task.actions - actions = [act.get_action() for act in action_models] need_token = False - valid = all([act.valid for act in actions]) - if not valid: - return {'errors': ['actions invalid']}, 400 - # TODO(amelia): get action invalidation reasons - # post_approve all actions for action in actions: try: