From cfd9d5d4f18445664c11fc0119b26cbf858e8565 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 25 May 2016 13:40:57 -0400 Subject: [PATCH] Refactor of JT related field checking --- awx/api/serializers.py | 25 +-- awx/main/models/jobs.py | 47 +++- .../functional/api/test_job_runtime_params.py | 2 +- .../tests/functional/api/test_job_template.py | 201 +++++++++++++++++ .../tests/functional/api/test_jt_copy_edit.py | 202 ------------------ awx/main/tests/unit/api/test_serializers.py | 7 +- 6 files changed, 254 insertions(+), 230 deletions(-) delete mode 100644 awx/main/tests/functional/api/test_jt_copy_edit.py diff --git a/awx/api/serializers.py b/awx/api/serializers.py index fb5af611e3..5efaed8d56 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1785,24 +1785,18 @@ class JobTemplateSerializer(UnifiedJobTemplateSerializer, JobOptionsSerializer): d['survey'] = dict(title=obj.survey_spec['name'], description=obj.survey_spec['description']) request = self.context.get('request', None) - # Conditions that would create a validation error if coppied - validation_pass = True - if obj.inventory is None and not obj.ask_inventory_on_launch: - validation_pass = False - if obj.credential is None and not obj.ask_credential_on_launch: - validation_pass = False - if obj.project is None and not obj.job_type != PERM_INVENTORY_SCAN: - validation_pass = False + # Check for conditions that would create a validation error if coppied + validation_errors, resources_needed_to_start = obj.resource_validation_data() if request is None or request.user is None: d['can_copy'] = False d['can_edit'] = False elif request.user.is_superuser: - d['can_copy'] = validation_pass + d['can_copy'] = not validation_errors d['can_edit'] = True else: jt_data = model_to_dict(obj) - d['can_copy'] = validation_pass and request.user.can_access(JobTemplate, 'add', jt_data) + d['can_copy'] = (not validation_errors) and request.user.can_access(JobTemplate, 'add', jt_data) d['can_edit'] = request.user.can_access(JobTemplate, 'change', obj, jt_data) d['recent_jobs'] = self._recent_jobs(obj) @@ -2268,12 +2262,14 @@ class JobLaunchSerializer(BaseSerializer): obj = self.context.get('obj') data = self.context.get('data') + for field in obj.resources_needed_to_start: + if not (field in attrs and obj._ask_for_vars_dict().get(field, False)): + errors[field] = "Job Template '%s' is missing or undefined." % field + if (not obj.ask_credential_on_launch) or (not attrs.get('credential', None)): credential = obj.credential else: credential = attrs.get('credential', None) - if not credential: - errors['credential'] = 'Credential not provided' # fill passwords dict with request data passwords if credential and credential.passwords_needed: @@ -2304,11 +2300,6 @@ class JobLaunchSerializer(BaseSerializer): if validation_errors: errors['variables_needed_to_start'] = validation_errors - if obj.job_type != PERM_INVENTORY_SCAN and (obj.project is None): - errors['project'] = 'Job Template Project is missing or undefined.' - if (obj.inventory is None) and not attrs.get('inventory', None): - errors['inventory'] = 'Job Template Inventory is missing or undefined.' - # Special prohibited cases for scan jobs if 'job_type' in data and obj.ask_job_type_on_launch: if ((obj.job_type == PERM_INVENTORY_SCAN and not data['job_type'] == PERM_INVENTORY_SCAN) or diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index 7e5f7fc905..7094b50a22 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -242,15 +242,44 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, ResourceMixin): 'force_handlers', 'skip_tags', 'start_at_task', 'become_enabled', 'labels',] + def resource_validation_data(self): + ''' + Process consistency errors and need-for-launch related fields. + ''' + resources_needed_to_start = [] + validation_errors = {} + + # Inventory and Credential related checks + if self.inventory is None: + resources_needed_to_start.append('inventory') + if not self.ask_inventory_on_launch: + validation_errors['inventory'] = ["Job Template must provide 'inventory' or allow prompting for it.",] + if self.credential is None: + resources_needed_to_start.append('credential') + if not self.ask_credential_on_launch: + validation_errors['credential'] = ["Job Template must provide 'credential' or allow prompting for it.",] + + # Job type dependent checks + if self.job_type == 'scan': + if self.inventory is None or self.ask_inventory_on_launch: + validation_errors['inventory'] = ["Scan jobs must be assigned a fixed inventory.",] + elif self.project is None: + resources_needed_to_start.append('project') + validation_errors['project'] = ["Job types 'run' and 'check' must have assigned a project.",] + + return (validation_errors, resources_needed_to_start) + def clean(self): - if self.job_type == 'scan' and (self.inventory is None or self.ask_inventory_on_launch): - raise ValidationError({"inventory": ["Scan jobs must be assigned a fixed inventory.",]}) - if (not self.ask_inventory_on_launch) and self.inventory is None: - raise ValidationError({"inventory": ["Job Template must provide 'inventory' or allow prompting for it.",]}) - if (not self.ask_credential_on_launch) and self.credential is None: - raise ValidationError({"credential": ["Job Template must provide 'credential' or allow prompting for it.",]}) + validation_errors, resources_needed_to_start = self.resource_validation_data() + if validation_errors: + raise ValidationError(validation_errors) return super(JobTemplate, self).clean() + @property + def resources_needed_to_start(self): + validation_errors, resources_needed_to_start = self.resource_validation_data() + return resources_needed_to_start + def create_job(self, **kwargs): ''' Create a new job based on this template. @@ -265,9 +294,9 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, ResourceMixin): Return whether job template can be used to start a new job without requiring any user input. ''' - return bool(self.credential and not len(self.passwords_needed_to_start) and - not len(self.variables_needed_to_start) and - self.inventory) + return ((not self.resources_needed_to_start) and + (not self.passwords_needed_to_start) and + (not self.variables_needed_to_start)) @property def variables_needed_to_start(self): diff --git a/awx/main/tests/functional/api/test_job_runtime_params.py b/awx/main/tests/functional/api/test_job_runtime_params.py index cfda95213a..176271f714 100644 --- a/awx/main/tests/functional/api/test_job_runtime_params.py +++ b/awx/main/tests/functional/api/test_job_runtime_params.py @@ -178,7 +178,7 @@ def test_job_launch_fails_without_inventory(deploy_jobtemplate, post, user): args=[deploy_jobtemplate.pk]), {}, user('admin', True)) assert response.status_code == 400 - assert response.data['inventory'] == ['Job Template Inventory is missing or undefined.'] + assert response.data['inventory'] == ["Job Template 'inventory' is missing or undefined."] @pytest.mark.django_db @pytest.mark.job_runtime_vars diff --git a/awx/main/tests/functional/api/test_job_template.py b/awx/main/tests/functional/api/test_job_template.py index 975eb279b0..185d946cd3 100644 --- a/awx/main/tests/functional/api/test_job_template.py +++ b/awx/main/tests/functional/api/test_job_template.py @@ -1,7 +1,27 @@ import pytest +import mock +# AWX +from awx.api.serializers import JobTemplateSerializer +from awx.main.models.jobs import JobTemplate + +# Django +from django.test.client import RequestFactory from django.core.urlresolvers import reverse + +@pytest.fixture +def jt_copy_edit(project): + return JobTemplate.objects.create( + job_type='run', + project=project, + playbook='hello_world.yml', + ask_inventory_on_launch=True, + ask_credential_on_launch=True, + name='copy-edit-job-template' + ) + + @pytest.mark.django_db def test_job_template_role_user(post, organization_factory, job_template_factory): objects = organization_factory("org", @@ -16,3 +36,184 @@ def test_job_template_role_user(post, organization_factory, job_template_factory url = reverse('api:user_roles_list', args=(objects.users.test.pk,)) response = post(url, dict(id=jt_objects.job_template.execute_role.pk), objects.superusers.admin) assert response.status_code == 204 + +# Test protection against limited set of validation problems + +@pytest.mark.django_db +def test_bad_data_copy_edit(admin_user, project): + "If a required resource (inventory here) was deleted, copying not allowed" + + jt_res = JobTemplate.objects.create( + job_type='run', + project=project, + inventory=None, ask_inventory_on_launch=False, # not allowed + credential=None, ask_credential_on_launch=True, + name='deploy-job-template' + ) + serializer = JobTemplateSerializer(jt_res) + request = RequestFactory().get('/api/v1/job_templates/12/') + request.user = admin_user + serializer.context['request'] = request + response = serializer.to_representation(jt_res) + assert not response['summary_fields']['can_copy'] + assert response['summary_fields']['can_edit'] + +# Tests for correspondence between view info and actual access + +@pytest.mark.django_db +def test_admin_copy_edit(jt_copy_edit, admin_user): + "Absent a validation error, system admins can do everything" + + # Serializer can_copy/can_edit fields + serializer = JobTemplateSerializer(jt_copy_edit) + request = RequestFactory().get('/api/v1/job_templates/12/') + request.user = admin_user + serializer.context['request'] = request + response = serializer.to_representation(jt_copy_edit) + assert response['summary_fields']['can_copy'] + assert response['summary_fields']['can_edit'] + +@pytest.mark.django_db +def test_org_admin_copy_edit(jt_copy_edit, org_admin): + "Organization admins SHOULD be able to copy a JT firmly in their org" + + # Serializer can_copy/can_edit fields + serializer = JobTemplateSerializer(jt_copy_edit) + request = RequestFactory().get('/api/v1/job_templates/12/') + request.user = org_admin + serializer.context['request'] = request + response = serializer.to_representation(jt_copy_edit) + assert response['summary_fields']['can_copy'] + assert response['summary_fields']['can_edit'] + +@pytest.mark.django_db +@pytest.mark.skip(reason="Waiting on issue 1981") +def test_org_admin_foreign_cred_no_copy_edit(jt_copy_edit, org_admin, machine_credential): + """ + Organization admins SHOULD NOT be able to copy JT without resource access + but they SHOULD be able to edit that job template + """ + + # Attach credential to JT that org admin can not use + jt_copy_edit.credential = machine_credential + jt_copy_edit.save() + + # Serializer can_copy/can_edit fields + serializer = JobTemplateSerializer(jt_copy_edit) + request = RequestFactory().get('/api/v1/job_templates/12/') + request.user = org_admin + serializer.context['request'] = request + response = serializer.to_representation(jt_copy_edit) + assert not response['summary_fields']['can_copy'] + assert response['summary_fields']['can_edit'] + +@pytest.mark.django_db +def test_jt_admin_copy_edit(jt_copy_edit, rando): + "JT admins wihout access to associated resources SHOULD NOT be able to copy" + + # random user given JT admin access only + jt_copy_edit.admin_role.members.add(rando) + jt_copy_edit.save() + + # Serializer can_copy/can_edit fields + serializer = JobTemplateSerializer(jt_copy_edit) + request = RequestFactory().get('/api/v1/job_templates/12/') + request.user = rando + serializer.context['request'] = request + response = serializer.to_representation(jt_copy_edit) + assert not response['summary_fields']['can_copy'] + assert not response['summary_fields']['can_edit'] + +@pytest.mark.django_db +def test_proj_jt_admin_copy_edit(jt_copy_edit, rando): + "JT admins with access to associated resources SHOULD be able to copy" + + # random user given JT and project admin abilities + jt_copy_edit.admin_role.members.add(rando) + jt_copy_edit.save() + jt_copy_edit.project.admin_role.members.add(rando) + jt_copy_edit.project.save() + + # Serializer can_copy/can_edit fields + serializer = JobTemplateSerializer(jt_copy_edit) + request = RequestFactory().get('/api/v1/job_templates/12/') + request.user = rando + serializer.context['request'] = request + response = serializer.to_representation(jt_copy_edit) + assert response['summary_fields']['can_copy'] + assert response['summary_fields']['can_edit'] + +# Functional tests - create new JT with all returned fields, as the UI does + +@pytest.mark.django_db +def test_org_admin_copy_edit_functional(jt_copy_edit, org_admin, get, post): + get_response = get(reverse('api:job_template_detail', args=[jt_copy_edit.pk]), user=org_admin) + + post_data = get_response.data + post_data['name'] = '%s @ 12:19:47 pm' % post_data['name'] + + assert get_response.status_code == 200 + assert get_response.data['summary_fields']['can_copy'] + + with mock.patch( + 'awx.main.models.projects.ProjectOptions.playbooks', + new_callable=mock.PropertyMock(return_value=['hello_world.yml'])): + post_response = post(reverse('api:job_template_list', args=[]), user=org_admin, data=post_data) + + print '\n post_response: ' + str(post_response.data) + assert post_response.status_code == 201 + assert post_response.data['name'] == 'copy-edit-job-template @ 12:19:47 pm' + +@pytest.mark.django_db +def test_jt_admin_copy_edit_functional(jt_copy_edit, rando, get, post): + + # Grant random user JT admin access only + jt_copy_edit.admin_role.members.add(rando) + jt_copy_edit.save() + + get_response = get(reverse('api:job_template_detail', args=[jt_copy_edit.pk]), user=rando) + + assert get_response.status_code == 200 + assert not get_response.data['summary_fields']['can_copy'] + + post_data = get_response.data + post_data['name'] = '%s @ 12:19:47 pm' % post_data['name'] + + with mock.patch( + 'awx.main.models.projects.ProjectOptions.playbooks', + new_callable=mock.PropertyMock(return_value=['hello_world.yml'])): + post_response = post(reverse('api:job_template_list', args=[]), user=rando, data=post_data) + + print '\n post_response: ' + str(post_response.data) + assert post_response.status_code == 403 + +# Validation function tests +# TODO: replace these JT creations with Wayne's new awesome factories + +@pytest.mark.django_db +def test_missing_project_error(inventory, machine_credential): + obj = JobTemplate.objects.create( + job_type='run', + project=None, + inventory=inventory, + credential=machine_credential, + name='missing-project-jt' + ) + assert 'project' in obj.resources_needed_to_start + validation_errors, resources_needed_to_start = obj.resource_validation_data() + assert 'project' in validation_errors + +@pytest.mark.django_db +def test_inventory_credential_contradictions(project): + obj = JobTemplate.objects.create( + job_type='run', + project=project, + inventory=None, ask_inventory_on_launch=False, + credential=None, ask_credential_on_launch=False, + name='missing-project-jt' + ) + assert 'inventory' in obj.resources_needed_to_start + assert 'credential' in obj.resources_needed_to_start + validation_errors, resources_needed_to_start = obj.resource_validation_data() + assert 'inventory' in validation_errors + assert 'credential' in validation_errors diff --git a/awx/main/tests/functional/api/test_jt_copy_edit.py b/awx/main/tests/functional/api/test_jt_copy_edit.py deleted file mode 100644 index a46870c37d..0000000000 --- a/awx/main/tests/functional/api/test_jt_copy_edit.py +++ /dev/null @@ -1,202 +0,0 @@ -import pytest -import mock - -# AWX -from awx.api.serializers import JobTemplateSerializer -from awx.main.access import JobTemplateAccess -from awx.main.models.jobs import JobTemplate - -# Django -from django.test.client import RequestFactory -from django.forms.models import model_to_dict -from django.core.urlresolvers import reverse - - -@pytest.fixture -def jt_copy_edit(project): - return JobTemplate.objects.create( - job_type='run', - project=project, - playbook='hello_world.yml', - ask_inventory_on_launch=True, - ask_credential_on_launch=True, - name='copy-edit-job-template' - ) - -# Test protection against limited set of validation problems - -@pytest.mark.django_db -def test_bad_data_copy_edit(admin_user, project): - "If a required resource (inventory here) was deleted, copying not allowed" - - jt_res = JobTemplate.objects.create( - job_type='run', - project=project, - inventory=None, ask_inventory_on_launch=False, # not allowed - credential=None, ask_credential_on_launch=True, - name='deploy-job-template' - ) - serializer = JobTemplateSerializer(jt_res) - request = RequestFactory().get('/api/v1/job_templates/12/') - request.user = admin_user - serializer.context['request'] = request - response = serializer.to_representation(jt_res) - assert not response['summary_fields']['can_copy'] - assert response['summary_fields']['can_edit'] - -# Tests for correspondence between view info and actual access - -@pytest.mark.django_db -def test_admin_copy_edit(jt_copy_edit, admin_user): - "Absent a validation error, system admins can do everything" - - # Serializer can_copy/can_edit fields - serializer = JobTemplateSerializer(jt_copy_edit) - request = RequestFactory().get('/api/v1/job_templates/12/') - request.user = admin_user - serializer.context['request'] = request - response = serializer.to_representation(jt_copy_edit) - assert response['summary_fields']['can_copy'] - assert response['summary_fields']['can_edit'] - - # Access - jt_access = JobTemplateAccess(admin_user) - jt_dict = model_to_dict(jt_copy_edit) - assert jt_access.can_add(jt_dict) - assert jt_access.can_change(jt_copy_edit, jt_dict) - -@pytest.mark.django_db -def test_org_admin_copy_edit(jt_copy_edit, org_admin): - "Organization admins SHOULD be able to copy a JT firmly in their org" - - # Serializer can_copy/can_edit fields - serializer = JobTemplateSerializer(jt_copy_edit) - request = RequestFactory().get('/api/v1/job_templates/12/') - request.user = org_admin - serializer.context['request'] = request - response = serializer.to_representation(jt_copy_edit) - assert response['summary_fields']['can_copy'] - assert response['summary_fields']['can_edit'] - - # Access - jt_access = JobTemplateAccess(org_admin) - jt_dict = model_to_dict(jt_copy_edit) - assert jt_access.can_add(jt_dict) - assert jt_access.can_change(jt_copy_edit, jt_dict) - -@pytest.mark.django_db -@pytest.mark.skip(reason="Waiting on issue 1981") -def test_org_admin_foreign_cred_no_copy_edit(jt_copy_edit, org_admin, machine_credential): - "Organization admins SHOULD NOT be able to copy JT without resource access" - - # Attach credential to JT that org admin can not use - jt_copy_edit.credential = machine_credential - jt_copy_edit.save() - - # Serializer can_copy/can_edit fields - serializer = JobTemplateSerializer(jt_copy_edit) - request = RequestFactory().get('/api/v1/job_templates/12/') - request.user = org_admin - serializer.context['request'] = request - response = serializer.to_representation(jt_copy_edit) - assert not response['summary_fields']['can_copy'] - assert response['summary_fields']['can_edit'] - - # Access - jt_access = JobTemplateAccess(org_admin) - jt_dict = model_to_dict(jt_copy_edit) - assert not jt_access.can_add(jt_dict) - assert jt_access.can_change(jt_copy_edit, jt_dict) - -@pytest.mark.django_db -def test_jt_admin_copy_edit(jt_copy_edit, rando): - "JT admins wihout access to associated resources SHOULD NOT be able to copy" - - # random user given JT admin access only - jt_copy_edit.admin_role.members.add(rando) - jt_copy_edit.save() - - # Serializer can_copy/can_edit fields - serializer = JobTemplateSerializer(jt_copy_edit) - request = RequestFactory().get('/api/v1/job_templates/12/') - request.user = rando - serializer.context['request'] = request - response = serializer.to_representation(jt_copy_edit) - assert not response['summary_fields']['can_copy'] - assert not response['summary_fields']['can_edit'] - - # Access - jt_access = JobTemplateAccess(rando) - jt_dict = model_to_dict(jt_copy_edit) - print ' jt_dict: ' + str(jt_dict) - assert not jt_access.can_add(jt_dict) - assert not jt_access.can_change(jt_copy_edit, jt_dict) - -@pytest.mark.django_db -def test_proj_jt_admin_copy_edit(jt_copy_edit, rando): - "JT admins with access to associated resources SHOULD be able to copy" - - # random user given JT and project admin abilities - jt_copy_edit.admin_role.members.add(rando) - jt_copy_edit.save() - jt_copy_edit.project.admin_role.members.add(rando) - jt_copy_edit.project.save() - - # Serializer can_copy/can_edit fields - serializer = JobTemplateSerializer(jt_copy_edit) - request = RequestFactory().get('/api/v1/job_templates/12/') - request.user = rando - serializer.context['request'] = request - response = serializer.to_representation(jt_copy_edit) - assert response['summary_fields']['can_copy'] - assert response['summary_fields']['can_edit'] - - # Access - jt_access = JobTemplateAccess(rando) - jt_dict = model_to_dict(jt_copy_edit) - assert jt_access.can_add(jt_dict) - assert jt_access.can_change(jt_copy_edit, jt_dict) - -# Functional tests - create new JT with all returned fields, as the UI does - -@pytest.mark.django_db -def test_org_admin_copy_edit_functional(jt_copy_edit, org_admin, get, post): - get_response = get(reverse('api:job_template_detail', args=[jt_copy_edit.pk]), user=org_admin) - - post_data = get_response.data - post_data['name'] = '%s @ 12:19:47 pm' % post_data['name'] - - assert get_response.status_code == 200 - assert get_response.data['summary_fields']['can_copy'] - - with mock.patch( - 'awx.main.models.projects.ProjectOptions.playbooks', - new_callable=mock.PropertyMock(return_value=['hello_world.yml'])): - post_response = post(reverse('api:job_template_list', args=[]), user=org_admin, data=post_data) - - print '\n post_response: ' + str(post_response.data) - assert post_response.status_code == 201 - assert post_response.data['name'] == 'copy-edit-job-template @ 12:19:47 pm' - -@pytest.mark.django_db -def test_jt_admin_copy_edit_functional(jt_copy_edit, rando, get, post): - - # Grant random user JT admin access only - jt_copy_edit.admin_role.members.add(rando) - jt_copy_edit.save() - - get_response = get(reverse('api:job_template_detail', args=[jt_copy_edit.pk]), user=rando) - - assert get_response.status_code == 200 - assert not get_response.data['summary_fields']['can_copy'] - - post_data = get_response.data - post_data['name'] = '%s @ 12:19:47 pm' % post_data['name'] - - with mock.patch( - 'awx.main.models.projects.ProjectOptions.playbooks', - new_callable=mock.PropertyMock(return_value=['hello_world.yml'])): - post_response = post(reverse('api:job_template_list', args=[]), user=rando, data=post_data) - - print '\n post_response: ' + str(post_response.data) - assert post_response.status_code == 403 diff --git a/awx/main/tests/unit/api/test_serializers.py b/awx/main/tests/unit/api/test_serializers.py index 4f98892e8c..0b2f41d930 100644 --- a/awx/main/tests/unit/api/test_serializers.py +++ b/awx/main/tests/unit/api/test_serializers.py @@ -9,9 +9,14 @@ from awx.main.models import Label, Job #DRF from rest_framework import serializers +def mock_JT_resource_data(): + return ({}, []) + @pytest.fixture def job_template(mocker): - return mocker.MagicMock(pk=5) + mock_jt = mocker.MagicMock(pk=5) + mock_jt.resource_validation_data = mock_JT_resource_data + return mock_jt @pytest.fixture def job(mocker, job_template):