diff --git a/sahara/api/v11.py b/sahara/api/v11.py index 3e4410a9..0cc76b15 100644 --- a/sahara/api/v11.py +++ b/sahara/api/v11.py @@ -71,7 +71,8 @@ def job_executions_cancel(job_execution_id): @rest.patch('/job-executions/') @acl.enforce("data-processing:job-executions:modify") @v.check_exists(api.get_job_execution, id='job_execution_id') -@v.validate(v_j_e_schema.JOB_EXEC_UPDATE_SCHEMA) +@v.validate( + v_j_e_schema.JOB_EXEC_UPDATE_SCHEMA, v_j_e.check_job_execution_update) def job_executions_update(job_execution_id, data): return u.to_wrapped_dict(api.update_job_execution, job_execution_id, data) diff --git a/sahara/db/sqlalchemy/api.py b/sahara/db/sqlalchemy/api.py index b3241e74..519b3840 100644 --- a/sahara/db/sqlalchemy/api.py +++ b/sahara/db/sqlalchemy/api.py @@ -842,11 +842,6 @@ def job_execution_update(context, job_execution_id, values): raise ex.NotFoundException(job_execution_id, _("JobExecution id '%s' not found!")) - # Skip this check for periodic tasks - if context.tenant_id: - validate.check_tenant_for_update(context, job_ex) - validate.check_protected_from_update(job_ex, values) - job_ex.update(values) session.add(job_ex) diff --git a/sahara/service/validations/edp/job_execution.py b/sahara/service/validations/edp/job_execution.py index 96302ea5..ff1fabaf 100644 --- a/sahara/service/validations/edp/job_execution.py +++ b/sahara/service/validations/edp/job_execution.py @@ -108,3 +108,11 @@ def check_job_execution_delete(job_execution_id, **kwargs): acl.check_tenant_for_delete(ctx, je) acl.check_protected_from_delete(je) + + +def check_job_execution_update(job_execution_id, data, **kwargs): + ctx = context.current() + je = conductor.job_execution_get(ctx, job_execution_id) + + acl.check_tenant_for_update(ctx, je) + acl.check_protected_from_update(je, data) diff --git a/sahara/tests/unit/conductor/manager/test_edp.py b/sahara/tests/unit/conductor/manager/test_edp.py index 045b1132..0ec87540 100644 --- a/sahara/tests/unit/conductor/manager/test_edp.py +++ b/sahara/tests/unit/conductor/manager/test_edp.py @@ -532,59 +532,6 @@ class JobExecutionTest(test_base.ConductorManagerTestCase): lst = self.api.job_execution_get_all(ctx, **kwargs) self.assertEqual(0, len(lst)) - def test_je_update_when_protected(self): - ctx = context.ctx() - job = self.api.job_create(ctx, SAMPLE_JOB) - ds_input = self.api.data_source_create(ctx, SAMPLE_DATA_SOURCE) - SAMPLE_DATA_OUTPUT = copy.deepcopy(SAMPLE_DATA_SOURCE) - SAMPLE_DATA_OUTPUT['name'] = 'output' - ds_output = self.api.data_source_create(ctx, SAMPLE_DATA_OUTPUT) - - sample = copy.deepcopy(SAMPLE_JOB_EXECUTION) - sample['is_protected'] = True - sample['job_id'] = job['id'] - sample['input_id'] = ds_input['id'] - sample['output_id'] = ds_output['id'] - - je = self.api.job_execution_create(ctx, sample) - je_id = je["id"] - - with testtools.ExpectedException(ex.UpdateFailedException): - try: - self.api.job_execution_update(ctx, je_id, {"is_public": True}) - except ex.UpdateFailedException as e: - self.assert_protected_resource_exception(e) - raise e - - self.api.job_execution_update(ctx, je_id, {"is_protected": False, - "is_public": True}) - - def test_public_je_update_from_another_tenant(self): - ctx = context.ctx() - job = self.api.job_create(ctx, SAMPLE_JOB) - ds_input = self.api.data_source_create(ctx, SAMPLE_DATA_SOURCE) - SAMPLE_DATA_OUTPUT = copy.deepcopy(SAMPLE_DATA_SOURCE) - SAMPLE_DATA_OUTPUT['name'] = 'output' - ds_output = self.api.data_source_create(ctx, SAMPLE_DATA_OUTPUT) - - sample = copy.deepcopy(SAMPLE_JOB_EXECUTION) - sample['is_public'] = True - sample['job_id'] = job['id'] - sample['input_id'] = ds_input['id'] - sample['output_id'] = ds_output['id'] - - je = self.api.job_execution_create(ctx, sample) - je_id = je["id"] - - ctx.tenant_id = 'tenant_2' - - with testtools.ExpectedException(ex.UpdateFailedException): - try: - self.api.job_execution_update(ctx, je_id, {"is_public": True}) - except ex.UpdateFailedException as e: - self.assert_created_in_another_tenant_exception(e) - raise e - class JobTest(test_base.ConductorManagerTestCase): def __init__(self, *args, **kwargs): diff --git a/sahara/tests/unit/service/validation/edp/test_job_executor.py b/sahara/tests/unit/service/validation/edp/test_job_executor.py index b10de319..b06b237c 100644 --- a/sahara/tests/unit/service/validation/edp/test_job_executor.py +++ b/sahara/tests/unit/service/validation/edp/test_job_executor.py @@ -277,6 +277,39 @@ class TestJobExecUpdateValidation(u.ValidationTestCase): } ) + @mock.patch('sahara.conductor.api.LocalApi.job_execution_get') + def test_je_update_when_protected(self, get_je_p): + + job_exec = mock.Mock(id='123', tenant_id='tenant_1', is_protected=True) + get_je_p.return_value = job_exec + + # job execution can't be updated if it's marked as protected + with testtools.ExpectedException(ex.UpdateFailedException): + try: + je.check_job_execution_update(job_exec, {'is_public': True}) + except ex.UpdateFailedException as e: + self.assert_protected_resource_exception(e) + raise e + # job execution can be updated because is_protected flag was + # set to False + je.check_job_execution_update( + job_exec, {'is_protected': False, 'is_public': True}) + + @mock.patch('sahara.conductor.api.LocalApi.job_execution_get') + def test_public_je_cancel_delete_from_another_tenant(self, get_je_p): + + job_exec = mock.Mock(id='123', tenant_id='tenant2', is_protected=False, + is_public=True) + get_je_p.return_value = job_exec + + with testtools.ExpectedException(ex.UpdateFailedException): + try: + je.check_job_execution_update( + job_exec, data={'is_public': False}) + except ex.UpdateFailedException as e: + self.assert_created_in_another_tenant_exception(e) + raise e + class TestJobExecutionCancelDeleteValidation(u.ValidationTestCase): def setUp(self):