From cc84ed51d6254d46f287244ffdbf1afcaaaf2f4a Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 4 Apr 2016 15:45:47 -0400 Subject: [PATCH 01/20] Add ability to prompt for several variable types on launch --- awx/api/serializers.py | 22 ++- awx/api/views.py | 57 ++++++- .../0013_v300_job_prompt_for_fields.py | 35 ++++ awx/main/models/jobs.py | 16 ++ .../functional/api/test_job_runtime_params.py | 159 ++++++++++++++++++ awx/main/tests/functional/conftest.py | 4 + awx/main/tests/old/jobs/job_launch.py | 1 + tools/docker-compose/start_development.sh | 3 +- 8 files changed, 291 insertions(+), 6 deletions(-) create mode 100644 awx/main/migrations/0013_v300_job_prompt_for_fields.py create mode 100644 awx/main/tests/functional/api/test_job_runtime_params.py diff --git a/awx/api/serializers.py b/awx/api/serializers.py index bda63004c4..2bc5c7a151 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1673,7 +1673,9 @@ class JobTemplateSerializer(UnifiedJobTemplateSerializer, JobOptionsSerializer): class Meta: model = JobTemplate - fields = ('*', 'host_config_key', 'ask_variables_on_launch', 'survey_enabled', 'become_enabled') + fields = ('*', 'host_config_key', 'ask_variables_on_launch', 'ask_limit_on_launch', + 'ask_tags_on_launch', 'ask_job_type_on_launch', 'ask_inventory_on_launch', + 'survey_enabled', 'become_enabled') def get_related(self, obj): res = super(JobTemplateSerializer, self).get_related(obj) @@ -1730,10 +1732,16 @@ class JobSerializer(UnifiedJobSerializer, JobOptionsSerializer): passwords_needed_to_start = serializers.ReadOnlyField() ask_variables_on_launch = serializers.ReadOnlyField() + ask_limit_on_launch = serializers.ReadOnlyField() + ask_tags_on_launch = serializers.ReadOnlyField() + ask_job_type_on_launch = serializers.ReadOnlyField() + ask_inventory_on_launch = serializers.ReadOnlyField() class Meta: model = Job - fields = ('*', 'job_template', 'passwords_needed_to_start', 'ask_variables_on_launch') + fields = ('*', 'job_template', 'passwords_needed_to_start', 'ask_variables_on_launch', + 'ask_limit_on_launch', 'ask_tags_on_launch', 'ask_job_type_on_launch', + 'ask_inventory_on_launch') def get_related(self, obj): res = super(JobSerializer, self).get_related(obj) @@ -2102,9 +2110,13 @@ class JobLaunchSerializer(BaseSerializer): class Meta: model = JobTemplate fields = ('can_start_without_user_input', 'passwords_needed_to_start', 'extra_vars', - 'ask_variables_on_launch', 'survey_enabled', 'variables_needed_to_start', + 'ask_variables_on_launch', 'ask_tags_on_launch', 'ask_job_type_on_launch', + 'ask_inventory_on_launch', 'ask_limit_on_launch', + 'survey_enabled', 'variables_needed_to_start', 'credential', 'credential_needed_to_start',) - read_only_fields = ('ask_variables_on_launch',) + read_only_fields = ('ask_variables_on_launch', 'ask_limit_on_launch', + 'ask_tags_on_launch', 'ask_job_type_on_launch', + 'ask_inventory_on_launch') extra_kwargs = { 'credential': { 'write_only': True, @@ -2164,7 +2176,9 @@ class JobLaunchSerializer(BaseSerializer): if errors: raise serializers.ValidationError(errors) + JT_extra_vars = obj.extra_vars attrs = super(JobLaunchSerializer, self).validate(attrs) + obj.extra_vars = JT_extra_vars return attrs class NotifierSerializer(BaseSerializer): diff --git a/awx/api/views.py b/awx/api/views.py index adb16289c1..b1243c28a7 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2115,8 +2115,60 @@ class JobTemplateLaunch(RetrieveAPIView, GenericAPIView): kv = { 'credential': serializer.instance.credential.pk, } + + # -- following code will be moved to JobTemplate model -- + # Sort the runtime fields allowed and disallowed by job template + ignored_fields = {} if 'extra_vars' in request.data: - kv['extra_vars'] = request.data['extra_vars'] + kv['extra_vars'] = {} + ignored_fields['extra_vars'] = {} + if obj.ask_variables_on_launch: + # Accept all extra_vars if the flag is set + kv['extra_vars'] = request.data['extra_vars'] + else: + if obj.survey_enabled: + # Accept vars defined in the survey and no others + survey_vars = [question['variable'] for question in obj.survey_spec['spec']] + for key in request.data['extra_vars']: + if key in survey_vars: + kv['extra_vars'][key] = request.data['extra_vars'][key] + else: + ignored_fields['extra_vars'][key] = request.data['extra_vars'][key] + else: + # No survey & prompt flag is false - ignore all + ignored_fields['extra_vars'] = request.data['extra_vars'] + + if 'limit' in request.data: + if obj.ask_limit_on_launch: + kv['limit'] = request.data['limit'] + else: + ignored_fields['limit'] = request.data['limit'] + + if 'job_tags' or 'skip_tags' in request.data: + if obj.ask_tags_on_launch: + if 'job_tags' in request.data: + kv['job_tags'] = request.data['job_tags'] + if 'skip_tags' in request.data: + kv['skip_tags'] = request.data['skip_tags'] + else: + if 'job_tags' in request.data: + ignored_fields['job_tags'] = request.data['job_tags'] + if 'skip_tags' in request.data: + ignored_fields['skip_tags'] = request.data['skip_tags'] + + if 'job_type' in request.data: + if obj.ask_job_type_on_launch: + kv['job_type'] = request.data['job_type'] + else: + ignored_fields['job_type'] = request.data['job_type'] + + if 'inventory' in request.data: + inv_id = request.data['inventory'] + if obj.ask_inventory_on_launch and Inventory.objects.get(pk=inv_id).accessible_by(self.request.user, {'write': True}): + kv['inventory'] = inv_id + else: + ignored_fields['inventory'] = inv_id + kv.update(passwords) new_job = obj.create_unified_job(**kv) @@ -2127,6 +2179,9 @@ class JobTemplateLaunch(RetrieveAPIView, GenericAPIView): return Response(data, status=status.HTTP_400_BAD_REQUEST) else: data = dict(job=new_job.id) + serializer = JobSerializer(new_job) + data['job_data'] = serializer.data + data['ignored_fields'] = ignored_fields return Response(data, status=status.HTTP_202_ACCEPTED) class JobTemplateSchedulesList(SubListCreateAttachDetachAPIView): diff --git a/awx/main/migrations/0013_v300_job_prompt_for_fields.py b/awx/main/migrations/0013_v300_job_prompt_for_fields.py new file mode 100644 index 0000000000..f9122b7cee --- /dev/null +++ b/awx/main/migrations/0013_v300_job_prompt_for_fields.py @@ -0,0 +1,35 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from django.db import migrations, models +from django.conf import settings + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0012_v300_create_labels'), + ] + + operations = [ + migrations.AddField( + model_name='jobtemplate', + name='ask_limit_on_launch', + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name='jobtemplate', + name='ask_inventory_on_launch', + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name='jobtemplate', + name='ask_job_type_on_launch', + field=models.BooleanField(default=False), + ), + migrations.AddField( + model_name='jobtemplate', + name='ask_tags_on_launch', + field=models.BooleanField(default=False), + ), + ] diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index 0314a57fb7..9a0c62f58a 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -194,6 +194,22 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, ResourceMixin): blank=True, default=False, ) + ask_limit_on_launch = models.BooleanField( + blank=True, + default=False, + ) + ask_tags_on_launch = models.BooleanField( + blank=True, + default=False, + ) + ask_job_type_on_launch = models.BooleanField( + blank=True, + default=False, + ) + ask_inventory_on_launch = models.BooleanField( + blank=True, + default=False, + ) survey_enabled = models.BooleanField( default=False, diff --git a/awx/main/tests/functional/api/test_job_runtime_params.py b/awx/main/tests/functional/api/test_job_runtime_params.py new file mode 100644 index 0000000000..e79c956084 --- /dev/null +++ b/awx/main/tests/functional/api/test_job_runtime_params.py @@ -0,0 +1,159 @@ +import pytest +import yaml + +from awx.api.serializers import JobLaunchSerializer +from awx.main.models.credential import Credential +from awx.main.models.inventory import Inventory +from awx.main.models.jobs import Job, JobTemplate + +from django.core.urlresolvers import reverse + +@pytest.fixture +def runtime_data(): + cred_obj = Credential.objects.create(name='runtime-cred', kind='ssh', username='test_user2', password='pas4word2') + return dict( + limit='test-servers', + job_type='check', + inventory=cred_obj.pk, + job_tags='["provision"]', + skip_tags='["restart"]', + extra_vars='{"job_launch_var": 4}' + ) + +@pytest.fixture +def job_template_prompts(project, inventory, machine_credential): + def rf(on_off): + return JobTemplate.objects.create( + job_type='run', project=project, inventory=inventory, + credential=machine_credential, name='deploy-job-template', + ask_variables_on_launch=on_off, + ask_tags_on_launch=on_off, + ask_job_type_on_launch=on_off, + ask_inventory_on_launch=on_off, + ask_limit_on_launch=on_off, + ) + return rf + +# Probably remove this test after development is finished +@pytest.mark.django_db +def test_job_launch_prompts_echo(job_template_prompts, get, user): + job_template = job_template_prompts(True) + assert job_template.ask_variables_on_launch + + url = reverse('api:job_template_launch', args=[job_template.pk]) + + response = get( + reverse('api:job_template_launch', args=[job_template.pk]), + user('admin', True)) + + # Just checking that the GET response has what we expect + assert response.data['ask_variables_on_launch'] + assert response.data['ask_tags_on_launch'] + assert response.data['ask_job_type_on_launch'] + assert response.data['ask_inventory_on_launch'] + assert response.data['ask_limit_on_launch'] + +@pytest.mark.django_db +def test_job_ignore_unprompted_vars(runtime_data, job_template_prompts, post, user): + job_template = job_template_prompts(False) + + response = post( + reverse('api:job_template_launch', args=[job_template.pk]), + runtime_data, user('admin', True)) + + job_id = response.data['job'] + job_obj = Job.objects.get(pk=job_id) + + # Check that job data matches job_template data + assert len(yaml.load(job_obj.extra_vars)) == 0 + assert job_obj.limit == job_template.limit + assert job_obj.job_type == job_template.job_type + assert job_obj.inventory.pk == job_template.inventory.pk + assert job_obj.job_tags == job_template.job_tags + + # Check that response tells us what things were ignored + assert 'job_launch_var' in response.data['ignored_fields']['extra_vars'] + assert 'job_type' in response.data['ignored_fields'] + assert 'limit' in response.data['ignored_fields'] + assert 'inventory' in response.data['ignored_fields'] + assert 'job_tags' in response.data['ignored_fields'] + assert 'skip_tags' in response.data['ignored_fields'] + +@pytest.mark.django_db +def test_job_accept_prompted_vars(runtime_data, job_template_prompts, post, user): + job_template = job_template_prompts(True) + + response = post( + reverse('api:job_template_launch', args=[job_template.pk]), + runtime_data, user('admin', True)) + + job_id = response.data['job'] + job_obj = Job.objects.get(pk=job_id) + + # Check that job data matches the given runtime variables + assert 'job_launch_var' in yaml.load(job_obj.extra_vars) + assert job_obj.limit == runtime_data['limit'] + assert job_obj.job_type == runtime_data['job_type'] + assert job_obj.inventory.pk == runtime_data['inventory'] + assert job_obj.job_tags == runtime_data['job_tags'] + +@pytest.mark.django_db +def test_job_launch_JT_with_validation(machine_credential, deploy_jobtemplate): + deploy_jobtemplate.extra_vars = '{"job_template_var": 3}' + deploy_jobtemplate.save() + + kv = dict(extra_vars={"job_launch_var": 4}, credential=machine_credential.id) + serializer = JobLaunchSerializer( + instance=deploy_jobtemplate, data=kv, + context={'obj': deploy_jobtemplate, 'data': kv, 'passwords': {}}) + validated = serializer.is_valid() + assert validated + + job_obj = deploy_jobtemplate.create_unified_job(**kv) + result = job_obj.signal_start(**kv) + + final_job_extra_vars = yaml.load(job_obj.extra_vars) + assert result + assert 'job_template_var' in final_job_extra_vars + assert 'job_launch_var' in final_job_extra_vars + +@pytest.mark.django_db +def test_job_launch_unprompted_vars_with_survey(job_template_prompts, post, user): + job_template = job_template_prompts(False) + job_template.survey_enabled = True + job_template.survey_spec = { + "spec": [ + { + "index": 0, + "question_name": "survey_var", + "min": 0, + "default": "", + "max": 100, + "question_description": "A survey question", + "required": True, + "variable": "survey_var", + "choices": "", + "type": "integer" + } + ], + "description": "", + "name": "" + } + job_template.save() + + response = post( + reverse('api:job_template_launch', args=[job_template.pk]), + dict(extra_vars={"job_launch_var": 3, "survey_var": 4}), + user('admin', True)) + + job_id = response.data['job'] + job_obj = Job.objects.get(pk=job_id) + + # Check that the survey variable is accept and the job variable isn't + job_extra_vars = yaml.load(job_obj.extra_vars) + assert 'job_launch_var' not in job_extra_vars + assert 'survey_var' in job_extra_vars + +# To add: +# permissions testing (can't provide inventory you can't run against) +# credentials/password test if they will be included in response format diff --git a/awx/main/tests/functional/conftest.py b/awx/main/tests/functional/conftest.py index 593f3e420b..e187d8cd03 100644 --- a/awx/main/tests/functional/conftest.py +++ b/awx/main/tests/functional/conftest.py @@ -167,6 +167,10 @@ def organization_factory(instance): def credential(): return Credential.objects.create(kind='aws', name='test-cred') +@pytest.fixture +def machine_credential(): + return Credential.objects.create(name='machine-cred', kind='ssh', username='test_user', password='pas4word') + @pytest.fixture def inventory(organization): return organization.inventories.create(name="test-inv") diff --git a/awx/main/tests/old/jobs/job_launch.py b/awx/main/tests/old/jobs/job_launch.py index c0997607ee..57c87a1b82 100644 --- a/awx/main/tests/old/jobs/job_launch.py +++ b/awx/main/tests/old/jobs/job_launch.py @@ -27,6 +27,7 @@ class JobTemplateLaunchTest(BaseJobTestMixin, django.test.TransactionTestCase): project = self.proj_dev.pk, credential = self.cred_sue.pk, playbook = self.proj_dev.playbooks[0], + ask_variables_on_launch = True, ) self.data_no_cred = dict( name = 'launched job template no credential', diff --git a/tools/docker-compose/start_development.sh b/tools/docker-compose/start_development.sh index 16d859a3ce..30557418b7 100755 --- a/tools/docker-compose/start_development.sh +++ b/tools/docker-compose/start_development.sh @@ -19,7 +19,8 @@ else echo "Failed to find tower source tree, map your development tree volume" fi -cp -nR /tmp/ansible_tower.egg-info /tower_devel/ || true +# will remove before PR lands +cp -fR /tmp/ansible_tower.egg-info /tower_devel/ || true # Check if we need to build dependencies #if [ -f "awx/lib/.deps_built" ]; then From 19b855a4d362ebc73031e0a3a11f2ca1848c9951 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Tue, 5 Apr 2016 09:56:12 -0400 Subject: [PATCH 02/20] prompt-for acceptance code in JT model --- awx/api/serializers.py | 5 ++++ awx/api/views.py | 54 ++----------------------------------- awx/main/models/jobs.py | 59 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 66 insertions(+), 52 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 2bc5c7a151..655e9f9269 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -2121,6 +2121,11 @@ class JobLaunchSerializer(BaseSerializer): 'credential': { 'write_only': True, }, + 'limit': {'write_only': True}, + 'job_tags': {'write_only': True}, + 'skip_tags': {'write_only': True}, + 'job_type': {'write_only': True}, + 'inventory': {'write_only': True}, } def get_credential_needed_to_start(self, obj): diff --git a/awx/api/views.py b/awx/api/views.py index b1243c28a7..accfabc350 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2116,59 +2116,9 @@ class JobTemplateLaunch(RetrieveAPIView, GenericAPIView): 'credential': serializer.instance.credential.pk, } - # -- following code will be moved to JobTemplate model -- - # Sort the runtime fields allowed and disallowed by job template - ignored_fields = {} - if 'extra_vars' in request.data: - kv['extra_vars'] = {} - ignored_fields['extra_vars'] = {} - if obj.ask_variables_on_launch: - # Accept all extra_vars if the flag is set - kv['extra_vars'] = request.data['extra_vars'] - else: - if obj.survey_enabled: - # Accept vars defined in the survey and no others - survey_vars = [question['variable'] for question in obj.survey_spec['spec']] - for key in request.data['extra_vars']: - if key in survey_vars: - kv['extra_vars'][key] = request.data['extra_vars'][key] - else: - ignored_fields['extra_vars'][key] = request.data['extra_vars'][key] - else: - # No survey & prompt flag is false - ignore all - ignored_fields['extra_vars'] = request.data['extra_vars'] - - if 'limit' in request.data: - if obj.ask_limit_on_launch: - kv['limit'] = request.data['limit'] - else: - ignored_fields['limit'] = request.data['limit'] - - if 'job_tags' or 'skip_tags' in request.data: - if obj.ask_tags_on_launch: - if 'job_tags' in request.data: - kv['job_tags'] = request.data['job_tags'] - if 'skip_tags' in request.data: - kv['skip_tags'] = request.data['skip_tags'] - else: - if 'job_tags' in request.data: - ignored_fields['job_tags'] = request.data['job_tags'] - if 'skip_tags' in request.data: - ignored_fields['skip_tags'] = request.data['skip_tags'] - - if 'job_type' in request.data: - if obj.ask_job_type_on_launch: - kv['job_type'] = request.data['job_type'] - else: - ignored_fields['job_type'] = request.data['job_type'] - - if 'inventory' in request.data: - inv_id = request.data['inventory'] - if obj.ask_inventory_on_launch and Inventory.objects.get(pk=inv_id).accessible_by(self.request.user, {'write': True}): - kv['inventory'] = inv_id - else: - ignored_fields['inventory'] = inv_id + prompted_fields, ignored_fields = obj._accept_or_ignore_job_kwargs(user=self.request.user, **request.data) + kv.update(prompted_fields) kv.update(passwords) new_job = obj.create_unified_job(**kv) diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index 9a0c62f58a..981c91cdd6 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -378,6 +378,65 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, ResourceMixin): kwargs['extra_vars'] = json.dumps(extra_vars) return kwargs + def _accept_or_ignore_job_kwargs(self, user, **kwargs): + # Sort the runtime fields allowed and disallowed by job template + ignored_fields = {} + prompted_fields = {} + if 'extra_vars' in kwargs: + prompted_fields['extra_vars'] = {} + ignored_fields['extra_vars'] = {} + if self.ask_variables_on_launch: + # Accept all extra_vars if the flag is set + prompted_fields['extra_vars'] = kwargs['extra_vars'] + else: + if self.survey_enabled: + # Accept vars defined in the survey and no others + survey_vars = [question['variable'] for question in self.survey_spec['spec']] + for key in kwargs['extra_vars']: + if key in survey_vars: + prompted_fields['extra_vars'][key] = kwargs['extra_vars'][key] + else: + ignored_fields['extra_vars'][key] = kwargs['extra_vars'][key] + else: + # No survey & prompt flag is false - ignore all + ignored_fields['extra_vars'] = kwargs['extra_vars'] + + if 'limit' in kwargs: + if self.ask_limit_on_launch: + prompted_fields['limit'] = kwargs['limit'] + else: + ignored_fields['limit'] = kwargs['limit'] + + if 'job_tags' or 'skip_tags' in kwargs: + if self.ask_tags_on_launch: + if 'job_tags' in kwargs: + prompted_fields['job_tags'] = kwargs['job_tags'] + if 'skip_tags' in kwargs: + prompted_fields['skip_tags'] = kwargs['skip_tags'] + else: + if 'job_tags' in kwargs: + ignored_fields['job_tags'] = kwargs['job_tags'] + if 'skip_tags' in kwargs: + ignored_fields['skip_tags'] = kwargs['skip_tags'] + + if 'job_type' in kwargs: + if self.ask_job_type_on_launch: + prompted_fields['job_type'] = kwargs['job_type'] + else: + ignored_fields['job_type'] = kwargs['job_type'] + + if 'inventory' in kwargs: + inv_id = kwargs['inventory'] + if self.ask_inventory_on_launch: + from awx.main.models.inventory import Inventory + if Inventory.objects.get(pk=inv_id).accessible_by(user, {'write': True}): + prompted_fields['inventory'] = inv_id + else: + ignored_fields['inventory'] = inv_id + else: + ignored_fields['inventory'] = inv_id + return prompted_fields, ignored_fields + @property def cache_timeout_blocked(self): if Job.objects.filter(job_template=self, status__in=['pending', 'waiting', 'running']).count() > getattr(tower_settings, 'SCHEDULE_MAX_JOBS', 10): From bea15021b32b9b612dd6c4113b0608dbe8838b7d Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 6 Apr 2016 14:19:22 -0400 Subject: [PATCH 03/20] ask_for_inventory permissions and runtime tests finished cleanup prompt-for additions update migration after rebase --- awx/api/serializers.py | 18 +- awx/api/views.py | 16 +- ....py => 0014_v300_job_prompt_for_fields.py} | 2 +- awx/main/models/jobs.py | 53 +++--- .../functional/api/test_job_runtime_params.py | 170 +++++++++++++----- tools/docker-compose/start_development.sh | 3 +- 6 files changed, 175 insertions(+), 87 deletions(-) rename awx/main/migrations/{0013_v300_job_prompt_for_fields.py => 0014_v300_job_prompt_for_fields.py} (95%) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 655e9f9269..dc0f04cfc4 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -2109,7 +2109,8 @@ class JobLaunchSerializer(BaseSerializer): class Meta: model = JobTemplate - fields = ('can_start_without_user_input', 'passwords_needed_to_start', 'extra_vars', + fields = ('can_start_without_user_input', 'passwords_needed_to_start', + 'extra_vars', 'limit', 'job_tags', 'skip_tags', 'job_type', 'inventory', 'ask_variables_on_launch', 'ask_tags_on_launch', 'ask_job_type_on_launch', 'ask_inventory_on_launch', 'ask_limit_on_launch', 'survey_enabled', 'variables_needed_to_start', @@ -2121,11 +2122,6 @@ class JobLaunchSerializer(BaseSerializer): 'credential': { 'write_only': True, }, - 'limit': {'write_only': True}, - 'job_tags': {'write_only': True}, - 'skip_tags': {'write_only': True}, - 'job_type': {'write_only': True}, - 'inventory': {'write_only': True}, } def get_credential_needed_to_start(self, obj): @@ -2182,8 +2178,18 @@ class JobLaunchSerializer(BaseSerializer): raise serializers.ValidationError(errors) JT_extra_vars = obj.extra_vars + JT_limit = obj.limit + JT_job_type = obj.job_type + JT_job_tags = obj.job_tags + JT_skip_tags = obj.skip_tags + JT_inventory = obj.inventory attrs = super(JobLaunchSerializer, self).validate(attrs) obj.extra_vars = JT_extra_vars + obj.limit = JT_limit + obj.job_type = JT_job_type + obj.skip_tags = JT_skip_tags + obj.job_tags = JT_job_tags + obj.inventory = JT_inventory return attrs class NotifierSerializer(BaseSerializer): diff --git a/awx/api/views.py b/awx/api/views.py index accfabc350..02ecf1ef4b 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2097,11 +2097,11 @@ class JobTemplateLaunch(RetrieveAPIView, GenericAPIView): def post(self, request, *args, **kwargs): obj = self.get_object() - if not request.user.can_access(self.model, 'start', obj): - raise PermissionDenied() if 'credential' not in request.data and 'credential_id' in request.data: request.data['credential'] = request.data['credential_id'] + if 'inventory' not in request.data and 'inventory_id' in request.data: + request.data['inventory'] = request.data['inventory_id'] passwords = {} serializer = self.serializer_class(instance=obj, data=request.data, context={'obj': obj, 'data': request.data, 'passwords': passwords}) @@ -2116,12 +2116,22 @@ class JobTemplateLaunch(RetrieveAPIView, GenericAPIView): 'credential': serializer.instance.credential.pk, } - prompted_fields, ignored_fields = obj._accept_or_ignore_job_kwargs(user=self.request.user, **request.data) + prompted_fields, ignored_fields = obj._accept_or_ignore_job_kwargs(**request.data) + + if 'inventory' in prompted_fields: + new_inventory = Inventory.objects.get(pk=prompted_fields['inventory']) + if not request.user.can_access(Inventory, 'read', new_inventory): + raise PermissionDenied() kv.update(prompted_fields) kv.update(passwords) new_job = obj.create_unified_job(**kv) + + if not request.user.can_access(Job, 'start', new_job): + new_job.delete() + raise PermissionDenied() + result = new_job.signal_start(**kv) if not result: data = dict(passwords_needed_to_start=new_job.passwords_needed_to_start) diff --git a/awx/main/migrations/0013_v300_job_prompt_for_fields.py b/awx/main/migrations/0014_v300_job_prompt_for_fields.py similarity index 95% rename from awx/main/migrations/0013_v300_job_prompt_for_fields.py rename to awx/main/migrations/0014_v300_job_prompt_for_fields.py index f9122b7cee..68bb315b9a 100644 --- a/awx/main/migrations/0013_v300_job_prompt_for_fields.py +++ b/awx/main/migrations/0014_v300_job_prompt_for_fields.py @@ -8,7 +8,7 @@ from django.conf import settings class Migration(migrations.Migration): dependencies = [ - ('main', '0012_v300_create_labels'), + ('main', '0013_v300_label_changes'), ] operations = [ diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index 981c91cdd6..3abdb29c8e 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -378,10 +378,11 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, ResourceMixin): kwargs['extra_vars'] = json.dumps(extra_vars) return kwargs - def _accept_or_ignore_job_kwargs(self, user, **kwargs): + def _accept_or_ignore_job_kwargs(self, **kwargs): # Sort the runtime fields allowed and disallowed by job template ignored_fields = {} prompted_fields = {} + if 'extra_vars' in kwargs: prompted_fields['extra_vars'] = {} ignored_fields['extra_vars'] = {} @@ -401,40 +402,26 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, ResourceMixin): # No survey & prompt flag is false - ignore all ignored_fields['extra_vars'] = kwargs['extra_vars'] - if 'limit' in kwargs: - if self.ask_limit_on_launch: - prompted_fields['limit'] = kwargs['limit'] - else: - ignored_fields['limit'] = kwargs['limit'] + # Fields which all follow the same pattern + ask_for_field_dict = dict( + limit=self.ask_limit_on_launch, + job_tags=self.ask_tags_on_launch, + skip_tags=self.ask_tags_on_launch, + job_type=self.ask_job_type_on_launch, + inventory=self.ask_inventory_on_launch + ) - if 'job_tags' or 'skip_tags' in kwargs: - if self.ask_tags_on_launch: - if 'job_tags' in kwargs: - prompted_fields['job_tags'] = kwargs['job_tags'] - if 'skip_tags' in kwargs: - prompted_fields['skip_tags'] = kwargs['skip_tags'] - else: - if 'job_tags' in kwargs: - ignored_fields['job_tags'] = kwargs['job_tags'] - if 'skip_tags' in kwargs: - ignored_fields['skip_tags'] = kwargs['skip_tags'] - - if 'job_type' in kwargs: - if self.ask_job_type_on_launch: - prompted_fields['job_type'] = kwargs['job_type'] - else: - ignored_fields['job_type'] = kwargs['job_type'] - - if 'inventory' in kwargs: - inv_id = kwargs['inventory'] - if self.ask_inventory_on_launch: - from awx.main.models.inventory import Inventory - if Inventory.objects.get(pk=inv_id).accessible_by(user, {'write': True}): - prompted_fields['inventory'] = inv_id + for field in ask_for_field_dict: + if field in kwargs: + if ask_for_field_dict[field]: + prompted_fields[field] = kwargs[field] else: - ignored_fields['inventory'] = inv_id - else: - ignored_fields['inventory'] = inv_id + ignored_fields[field] = kwargs[field] + + if prompted_fields.get('job_type', None) == 'scan' or self.job_type == 'scan': + if 'inventory' in prompted_fields: + ignored_fields['inventory'] = prompted_fields.pop('inventory') + return prompted_fields, ignored_fields @property 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 e79c956084..cc09c8e6b7 100644 --- a/awx/main/tests/functional/api/test_job_runtime_params.py +++ b/awx/main/tests/functional/api/test_job_runtime_params.py @@ -3,29 +3,35 @@ import yaml from awx.api.serializers import JobLaunchSerializer from awx.main.models.credential import Credential -from awx.main.models.inventory import Inventory from awx.main.models.jobs import Job, JobTemplate from django.core.urlresolvers import reverse +from copy import copy + @pytest.fixture -def runtime_data(): +def runtime_data(organization): cred_obj = Credential.objects.create(name='runtime-cred', kind='ssh', username='test_user2', password='pas4word2') + inv_obj = organization.inventories.create(name="runtime-inv") return dict( + extra_vars='{"job_launch_var": 4}', limit='test-servers', job_type='check', - inventory=cred_obj.pk, - job_tags='["provision"]', - skip_tags='["restart"]', - extra_vars='{"job_launch_var": 4}' + job_tags='provision', + skip_tags='restart', + inventory=inv_obj.pk, + credential=cred_obj.pk, ) @pytest.fixture def job_template_prompts(project, inventory, machine_credential): def rf(on_off): return JobTemplate.objects.create( - job_type='run', project=project, inventory=inventory, - credential=machine_credential, name='deploy-job-template', + job_type='run', + project=project, + inventory=inventory, + credential=machine_credential, + name='deploy-job-template', ask_variables_on_launch=on_off, ask_tags_on_launch=on_off, ask_job_type_on_launch=on_off, @@ -34,42 +40,25 @@ def job_template_prompts(project, inventory, machine_credential): ) return rf -# Probably remove this test after development is finished -@pytest.mark.django_db -def test_job_launch_prompts_echo(job_template_prompts, get, user): - job_template = job_template_prompts(True) - assert job_template.ask_variables_on_launch - - url = reverse('api:job_template_launch', args=[job_template.pk]) - - response = get( - reverse('api:job_template_launch', args=[job_template.pk]), - user('admin', True)) - - # Just checking that the GET response has what we expect - assert response.data['ask_variables_on_launch'] - assert response.data['ask_tags_on_launch'] - assert response.data['ask_job_type_on_launch'] - assert response.data['ask_inventory_on_launch'] - assert response.data['ask_limit_on_launch'] - @pytest.mark.django_db +@pytest.mark.job_runtime_vars def test_job_ignore_unprompted_vars(runtime_data, job_template_prompts, post, user): job_template = job_template_prompts(False) + job_template_saved = copy(job_template) - response = post( - reverse('api:job_template_launch', args=[job_template.pk]), - runtime_data, user('admin', True)) + response = post(reverse('api:job_template_launch', args=[job_template.pk]), + runtime_data, user('admin', True)) + assert response.status_code == 202 job_id = response.data['job'] job_obj = Job.objects.get(pk=job_id) # Check that job data matches job_template data assert len(yaml.load(job_obj.extra_vars)) == 0 - assert job_obj.limit == job_template.limit - assert job_obj.job_type == job_template.job_type - assert job_obj.inventory.pk == job_template.inventory.pk - assert job_obj.job_tags == job_template.job_tags + assert job_obj.limit == job_template_saved.limit + assert job_obj.job_type == job_template_saved.job_type + assert job_obj.inventory.pk == job_template_saved.inventory.pk + assert job_obj.job_tags == job_template_saved.job_tags # Check that response tells us what things were ignored assert 'job_launch_var' in response.data['ignored_fields']['extra_vars'] @@ -80,13 +69,18 @@ def test_job_ignore_unprompted_vars(runtime_data, job_template_prompts, post, us assert 'skip_tags' in response.data['ignored_fields'] @pytest.mark.django_db +@pytest.mark.job_runtime_vars def test_job_accept_prompted_vars(runtime_data, job_template_prompts, post, user): job_template = job_template_prompts(True) + admin_user = user('admin', True) - response = post( - reverse('api:job_template_launch', args=[job_template.pk]), - runtime_data, user('admin', True)) + job_template.inventory.executor_role.members.add(admin_user) + job_template.inventory.save() + response = post(reverse('api:job_template_launch', args=[job_template.pk]), + runtime_data, user('admin', True)) + + assert response.status_code == 202 job_id = response.data['job'] job_obj = Job.objects.get(pk=job_id) @@ -97,6 +91,101 @@ def test_job_accept_prompted_vars(runtime_data, job_template_prompts, post, user assert job_obj.inventory.pk == runtime_data['inventory'] assert job_obj.job_tags == runtime_data['job_tags'] +@pytest.mark.django_db +@pytest.mark.job_runtime_vars +def test_job_reject_invalid_prompted_vars(runtime_data, job_template_prompts, post, user): + job_template = job_template_prompts(True) + + response = post( + reverse('api:job_template_launch', args=[job_template.pk]), + dict(job_type='foobicate', # foobicate is not a valid job type + inventory=87865), user('admin', True)) + + assert response.status_code == 400 + assert response.data['job_type'] == [u'"foobicate" is not a valid choice.'] + assert response.data['inventory'] == [u'Invalid pk "87865" - object does not exist.'] + +@pytest.mark.django_db +@pytest.mark.job_runtime_vars +def test_job_reject_invalid_prompted_extra_vars(runtime_data, job_template_prompts, post, user): + job_template = job_template_prompts(True) + + response = post( + reverse('api:job_template_launch', args=[job_template.pk]), + dict(extra_vars='{"unbalanced brackets":'), user('admin', True)) + + assert response.status_code == 400 + assert response.data['extra_vars'] == ['Must be valid JSON or YAML'] + +@pytest.mark.django_db +@pytest.mark.job_runtime_vars +def test_job_launch_fails_without_inventory(deploy_jobtemplate, post, user): + deploy_jobtemplate.inventory = None + deploy_jobtemplate.save() + + response = post(reverse('api:job_template_launch', + args=[deploy_jobtemplate.pk]), {}, user('admin', True)) + + assert response.status_code == 400 + assert response.data['inventory'] == ['Job Template Inventory is missing or undefined'] + +@pytest.mark.django_db +@pytest.mark.job_runtime_vars +def test_job_launch_fails_without_inventory_access(deploy_jobtemplate, machine_credential, post, user): + deploy_jobtemplate.ask_inventory_on_launch = True + deploy_jobtemplate.credential = machine_credential + common_user = user('test-user', False) + # TODO: Change admin_role to executor_role once issue #1422 is resolved + deploy_jobtemplate.admin_role.members.add(common_user) + deploy_jobtemplate.save() + deploy_jobtemplate.inventory.executor_role.members.add(common_user) + deploy_jobtemplate.inventory.save() + deploy_jobtemplate.project.member_role.members.add(common_user) + deploy_jobtemplate.project.save() + # TODO: change owner_role to usage_role after fix + deploy_jobtemplate.credential.owner_role.members.add(common_user) + deploy_jobtemplate.credential.save() + + # Assure that the base job template can be launched to begin with + response = post(reverse('api:job_template_launch', + args=[deploy_jobtemplate.pk]), {}, common_user) + + assert response.status_code == 202 + + # Assure that giving an inventory without access to the inventory blocks the launch + new_inv = deploy_jobtemplate.project.organization.inventories.create(name="user-can-not-use") + response = post(reverse('api:job_template_launch', args=[deploy_jobtemplate.pk]), + dict(inventory=new_inv.pk), common_user) + + assert response.status_code == 403 + assert response.data['detail'] == u'You do not have permission to perform this action.' + +@pytest.mark.django_db +@pytest.mark.job_runtime_vars +def test_job_relaunch_prompted_vars(runtime_data, job_template_prompts, post, user): + job_template = job_template_prompts(True) + admin_user = user('admin', True) + + # Launch job, overwriting several JT fields + first_response = post(reverse('api:job_template_launch', args=[job_template.pk]), + runtime_data, admin_user) + + assert first_response.status_code == 202 + original_job = Job.objects.get(pk=first_response.data['job']) + + # Launch a second job as a relaunch of the first + second_response = post(reverse('api:job_relaunch', args=[original_job.pk]), + {}, admin_user) + relaunched_job = Job.objects.get(pk=second_response.data['job']) + + # Check that job data matches the original runtime variables + assert first_response.status_code == 202 + assert 'job_launch_var' in yaml.load(relaunched_job.extra_vars) + assert relaunched_job.limit == runtime_data['limit'] + assert relaunched_job.job_type == runtime_data['job_type'] + assert relaunched_job.inventory.pk == runtime_data['inventory'] + assert relaunched_job.job_tags == runtime_data['job_tags'] + @pytest.mark.django_db def test_job_launch_JT_with_validation(machine_credential, deploy_jobtemplate): deploy_jobtemplate.extra_vars = '{"job_template_var": 3}' @@ -118,6 +207,7 @@ def test_job_launch_JT_with_validation(machine_credential, deploy_jobtemplate): assert 'job_launch_var' in final_job_extra_vars @pytest.mark.django_db +@pytest.mark.job_runtime_vars def test_job_launch_unprompted_vars_with_survey(job_template_prompts, post, user): job_template = job_template_prompts(False) job_template.survey_enabled = True @@ -149,11 +239,7 @@ def test_job_launch_unprompted_vars_with_survey(job_template_prompts, post, user job_id = response.data['job'] job_obj = Job.objects.get(pk=job_id) - # Check that the survey variable is accept and the job variable isn't + # Check that the survey variable is accepted and the job variable isn't job_extra_vars = yaml.load(job_obj.extra_vars) assert 'job_launch_var' not in job_extra_vars assert 'survey_var' in job_extra_vars - -# To add: -# permissions testing (can't provide inventory you can't run against) -# credentials/password test if they will be included in response format diff --git a/tools/docker-compose/start_development.sh b/tools/docker-compose/start_development.sh index 30557418b7..16d859a3ce 100755 --- a/tools/docker-compose/start_development.sh +++ b/tools/docker-compose/start_development.sh @@ -19,8 +19,7 @@ else echo "Failed to find tower source tree, map your development tree volume" fi -# will remove before PR lands -cp -fR /tmp/ansible_tower.egg-info /tower_devel/ || true +cp -nR /tmp/ansible_tower.egg-info /tower_devel/ || true # Check if we need to build dependencies #if [ -f "awx/lib/.deps_built" ]; then From 056a7d798a0666beed4599e7ee4bff5d41c297c0 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Thu, 7 Apr 2016 17:37:33 -0400 Subject: [PATCH 04/20] prompt-for vars in API browser box smart behavior --- awx/api/serializers.py | 9 ++++++--- awx/api/views.py | 14 +++++++++++++- 2 files changed, 19 insertions(+), 4 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index dc0f04cfc4..eb9fd89e8c 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -2119,9 +2119,12 @@ class JobLaunchSerializer(BaseSerializer): 'ask_tags_on_launch', 'ask_job_type_on_launch', 'ask_inventory_on_launch') extra_kwargs = { - 'credential': { - 'write_only': True, - }, + 'credential': {'write_only': True,}, + 'limit': {'write_only': True,}, + 'job_tags': {'write_only': True,}, + 'skip_tags': {'write_only': True,}, + 'job_type': {'write_only': True,}, + 'inventory': {'write_only': True,} } def get_credential_needed_to_start(self, obj): diff --git a/awx/api/views.py b/awx/api/views.py index 02ecf1ef4b..b04454918d 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2092,7 +2092,19 @@ class JobTemplateLaunch(RetrieveAPIView, GenericAPIView): data['credential'] = None for v in obj.variables_needed_to_start: extra_vars.setdefault(v, u'') - data['extra_vars'] = extra_vars + ask_for_field_dict = dict( + extra_vars=obj.ask_variables_on_launch, + limit=obj.ask_limit_on_launch, + job_tags=obj.ask_tags_on_launch, + skip_tags=obj.ask_tags_on_launch, + job_type=obj.ask_job_type_on_launch, + inventory=obj.ask_inventory_on_launch + ) + for field in ask_for_field_dict: + if not ask_for_field_dict[field]: + data.pop(field, None) + elif field == 'extra_vars': + data[field] = extra_vars return data def post(self, request, *args, **kwargs): From 68d8d25d646dc8b8e16b53cd78ce31177eb62c92 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Fri, 8 Apr 2016 10:56:15 -0400 Subject: [PATCH 05/20] fill in prompted data from JT, use standard launch permission, and compat code --- awx/api/views.py | 35 ++++++++++++-------------- awx/main/access.py | 2 +- awx/main/models/jobs.py | 55 +++++++++++++++++++---------------------- 3 files changed, 43 insertions(+), 49 deletions(-) diff --git a/awx/api/views.py b/awx/api/views.py index b04454918d..03caa2c37b 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2092,23 +2092,26 @@ class JobTemplateLaunch(RetrieveAPIView, GenericAPIView): data['credential'] = None for v in obj.variables_needed_to_start: extra_vars.setdefault(v, u'') - ask_for_field_dict = dict( - extra_vars=obj.ask_variables_on_launch, - limit=obj.ask_limit_on_launch, - job_tags=obj.ask_tags_on_launch, - skip_tags=obj.ask_tags_on_launch, - job_type=obj.ask_job_type_on_launch, - inventory=obj.ask_inventory_on_launch - ) - for field in ask_for_field_dict: - if not ask_for_field_dict[field]: + ask_for_vars_dict = obj._ask_for_vars_dict() + for field in ask_for_vars_dict: + if not ask_for_vars_dict[field]: data.pop(field, None) elif field == 'extra_vars': data[field] = extra_vars + elif field == 'inventory': + inv_obj = getattr(obj, field) + if inv_obj in ('', None): + data[field] = None + else: + data[field] = inv_obj.id + else: + data[field] = getattr(obj, field) return data def post(self, request, *args, **kwargs): obj = self.get_object() + if not request.user.can_access(self.model, 'start', obj): + raise PermissionDenied() if 'credential' not in request.data and 'credential_id' in request.data: request.data['credential'] = request.data['credential_id'] @@ -2139,22 +2142,16 @@ class JobTemplateLaunch(RetrieveAPIView, GenericAPIView): kv.update(passwords) new_job = obj.create_unified_job(**kv) - - if not request.user.can_access(Job, 'start', new_job): - new_job.delete() - raise PermissionDenied() - result = new_job.signal_start(**kv) if not result: data = dict(passwords_needed_to_start=new_job.passwords_needed_to_start) new_job.delete() return Response(data, status=status.HTTP_400_BAD_REQUEST) else: - data = dict(job=new_job.id) - serializer = JobSerializer(new_job) - data['job_data'] = serializer.data + data = JobSerializer(new_job).data + data['job'] = new_job.id data['ignored_fields'] = ignored_fields - return Response(data, status=status.HTTP_202_ACCEPTED) + return Response(data, status=status.HTTP_201_CREATED) class JobTemplateSchedulesList(SubListCreateAttachDetachAPIView): diff --git a/awx/main/access.py b/awx/main/access.py index 565a9be47f..b250f9b40b 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -766,7 +766,7 @@ class JobTemplateAccess(BaseAccess): self.check_license() if obj.job_type == PERM_INVENTORY_SCAN: self.check_license(feature='system_tracking') - if obj.survey_enabled: + if getattr(obj, 'survey_enabled', None): self.check_license(feature='surveys') # Super users can start any job diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index 3abdb29c8e..4d4c321d5e 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -378,32 +378,9 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, ResourceMixin): kwargs['extra_vars'] = json.dumps(extra_vars) return kwargs - def _accept_or_ignore_job_kwargs(self, **kwargs): - # Sort the runtime fields allowed and disallowed by job template - ignored_fields = {} - prompted_fields = {} - - if 'extra_vars' in kwargs: - prompted_fields['extra_vars'] = {} - ignored_fields['extra_vars'] = {} - if self.ask_variables_on_launch: - # Accept all extra_vars if the flag is set - prompted_fields['extra_vars'] = kwargs['extra_vars'] - else: - if self.survey_enabled: - # Accept vars defined in the survey and no others - survey_vars = [question['variable'] for question in self.survey_spec['spec']] - for key in kwargs['extra_vars']: - if key in survey_vars: - prompted_fields['extra_vars'][key] = kwargs['extra_vars'][key] - else: - ignored_fields['extra_vars'][key] = kwargs['extra_vars'][key] - else: - # No survey & prompt flag is false - ignore all - ignored_fields['extra_vars'] = kwargs['extra_vars'] - - # Fields which all follow the same pattern - ask_for_field_dict = dict( + def _ask_for_vars_dict(self): + return dict( + extra_vars=self.ask_variables_on_launch, limit=self.ask_limit_on_launch, job_tags=self.ask_tags_on_launch, skip_tags=self.ask_tags_on_launch, @@ -411,13 +388,33 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, ResourceMixin): inventory=self.ask_inventory_on_launch ) - for field in ask_for_field_dict: + def _accept_or_ignore_job_kwargs(self, **kwargs): + # Sort the runtime fields allowed and disallowed by job template + ignored_fields = {} + prompted_fields = {} + + ask_for_vars_dict = self._ask_for_vars_dict() + + for field in ask_for_vars_dict: if field in kwargs: - if ask_for_field_dict[field]: + if field == 'extra_vars': + prompted_fields[field] = {} + ignored_fields[field] = {} + if ask_for_vars_dict[field]: prompted_fields[field] = kwargs[field] else: - ignored_fields[field] = kwargs[field] + if field == 'extra_vars' and self.survey_enabled: + # Accept vars defined in the survey and no others + survey_vars = [question['variable'] for question in self.survey_spec['spec']] + for key in kwargs[field]: + if key in survey_vars: + prompted_fields[field][key] = kwargs[field][key] + else: + ignored_fields[field][key] = kwargs[field][key] + else: + ignored_fields[field] = kwargs[field] + # Special case to ignore inventory if it is a scan job if prompted_fields.get('job_type', None) == 'scan' or self.job_type == 'scan': if 'inventory' in prompted_fields: ignored_fields['inventory'] = prompted_fields.pop('inventory') From f471f9b0720fd12db1cc6e6a5e827cf978752dbb Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Fri, 8 Apr 2016 12:30:57 -0400 Subject: [PATCH 06/20] change migration to allow blank inventory field --- .../migrations/0014_v300_job_prompt_for_fields.py | 11 +++++++++++ awx/main/models/jobs.py | 2 ++ 2 files changed, 13 insertions(+) diff --git a/awx/main/migrations/0014_v300_job_prompt_for_fields.py b/awx/main/migrations/0014_v300_job_prompt_for_fields.py index 68bb315b9a..e7e5a5b623 100644 --- a/awx/main/migrations/0014_v300_job_prompt_for_fields.py +++ b/awx/main/migrations/0014_v300_job_prompt_for_fields.py @@ -2,6 +2,7 @@ from __future__ import unicode_literals from django.db import migrations, models +import django.db.models.deletion from django.conf import settings @@ -32,4 +33,14 @@ class Migration(migrations.Migration): name='ask_tags_on_launch', field=models.BooleanField(default=False), ), + migrations.AlterField( + model_name='job', + name='inventory', + field=models.ForeignKey(related_name='jobs', on_delete=django.db.models.deletion.SET_NULL, default=None, blank=True, to='main.Inventory', null=True), + ), + migrations.AlterField( + model_name='jobtemplate', + name='inventory', + field=models.ForeignKey(related_name='jobtemplates', on_delete=django.db.models.deletion.SET_NULL, default=None, blank=True, to='main.Inventory', null=True), + ), ] diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index 4d4c321d5e..ba584a6eb6 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -53,7 +53,9 @@ class JobOptions(BaseModel): inventory = models.ForeignKey( 'Inventory', related_name='%(class)ss', + blank=True, null=True, + default=None, on_delete=models.SET_NULL, ) project = models.ForeignKey( From 86aa0136a2754ac17845f4b39040ba039625903e Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Fri, 8 Apr 2016 14:49:42 -0400 Subject: [PATCH 07/20] runtime inventory compatibility with blank, and resolve issue #1453 --- awx/api/serializers.py | 9 +++++++-- awx/main/models/jobs.py | 4 +++- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index eb9fd89e8c..c6fe2ca325 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -2140,7 +2140,12 @@ class JobLaunchSerializer(BaseSerializer): obj = self.context.get('obj') data = self.context.get('data') - credential = attrs.get('credential', obj and obj.credential or None) + if obj and obj.credential is not None: + # force job template to override runtime credential, if present + credential = obj.credential + attrs.pop('credential', None) + else: + credential = attrs.get('credential', None) if not credential: errors['credential'] = 'Credential not provided' @@ -2174,7 +2179,7 @@ class JobLaunchSerializer(BaseSerializer): 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: + if (obj.inventory is None) and not attrs.get('inventory', None): errors['inventory'] = 'Job Template Inventory is missing or undefined' if errors: diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index ba584a6eb6..fb371e3a9e 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -265,7 +265,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)) + return bool(self.credential and not len(self.passwords_needed_to_start) + and not len(self.variables_needed_to_start) + and self.inventory) @property def variables_needed_to_start(self): From 52d8993aa81ecdf8f5ea937f11d77e6f2354a39c Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Fri, 8 Apr 2016 16:15:30 -0400 Subject: [PATCH 08/20] update tests to conform to new credential behavior and launch response --- .../functional/api/test_job_runtime_params.py | 12 ++++++---- awx/main/tests/old/jobs/job_launch.py | 24 +++++++++---------- awx/main/tests/old/jobs/jobs_monolithic.py | 20 ++++++++-------- 3 files changed, 29 insertions(+), 27 deletions(-) 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 cc09c8e6b7..afdd002828 100644 --- a/awx/main/tests/functional/api/test_job_runtime_params.py +++ b/awx/main/tests/functional/api/test_job_runtime_params.py @@ -49,7 +49,7 @@ def test_job_ignore_unprompted_vars(runtime_data, job_template_prompts, post, us response = post(reverse('api:job_template_launch', args=[job_template.pk]), runtime_data, user('admin', True)) - assert response.status_code == 202 + assert response.status_code == 201 job_id = response.data['job'] job_obj = Job.objects.get(pk=job_id) @@ -80,7 +80,7 @@ def test_job_accept_prompted_vars(runtime_data, job_template_prompts, post, user response = post(reverse('api:job_template_launch', args=[job_template.pk]), runtime_data, user('admin', True)) - assert response.status_code == 202 + assert response.status_code == 201 job_id = response.data['job'] job_obj = Job.objects.get(pk=job_id) @@ -150,7 +150,7 @@ def test_job_launch_fails_without_inventory_access(deploy_jobtemplate, machine_c response = post(reverse('api:job_template_launch', args=[deploy_jobtemplate.pk]), {}, common_user) - assert response.status_code == 202 + assert response.status_code == 201 # Assure that giving an inventory without access to the inventory blocks the launch new_inv = deploy_jobtemplate.project.organization.inventories.create(name="user-can-not-use") @@ -170,7 +170,7 @@ def test_job_relaunch_prompted_vars(runtime_data, job_template_prompts, post, us first_response = post(reverse('api:job_template_launch', args=[job_template.pk]), runtime_data, admin_user) - assert first_response.status_code == 202 + assert first_response.status_code == 201 original_job = Job.objects.get(pk=first_response.data['job']) # Launch a second job as a relaunch of the first @@ -179,7 +179,7 @@ def test_job_relaunch_prompted_vars(runtime_data, job_template_prompts, post, us relaunched_job = Job.objects.get(pk=second_response.data['job']) # Check that job data matches the original runtime variables - assert first_response.status_code == 202 + assert first_response.status_code == 201 assert 'job_launch_var' in yaml.load(relaunched_job.extra_vars) assert relaunched_job.limit == runtime_data['limit'] assert relaunched_job.job_type == runtime_data['job_type'] @@ -189,6 +189,7 @@ def test_job_relaunch_prompted_vars(runtime_data, job_template_prompts, post, us @pytest.mark.django_db def test_job_launch_JT_with_validation(machine_credential, deploy_jobtemplate): deploy_jobtemplate.extra_vars = '{"job_template_var": 3}' + deploy_jobtemplate.credential = None deploy_jobtemplate.save() kv = dict(extra_vars={"job_launch_var": 4}, credential=machine_credential.id) @@ -205,6 +206,7 @@ def test_job_launch_JT_with_validation(machine_credential, deploy_jobtemplate): assert result assert 'job_template_var' in final_job_extra_vars assert 'job_launch_var' in final_job_extra_vars + assert job_obj.credential.id == machine_credential.id @pytest.mark.django_db @pytest.mark.job_runtime_vars diff --git a/awx/main/tests/old/jobs/job_launch.py b/awx/main/tests/old/jobs/job_launch.py index 57c87a1b82..27a44ddb21 100644 --- a/awx/main/tests/old/jobs/job_launch.py +++ b/awx/main/tests/old/jobs/job_launch.py @@ -68,7 +68,7 @@ class JobTemplateLaunchTest(BaseJobTestMixin, django.test.TransactionTestCase): def test_credential_implicit(self): # Implicit, attached credentials with self.current_user(self.user_sue): - response = self.post(self.launch_url, {}, expect=202) + response = self.post(self.launch_url, {}, expect=201) j = Job.objects.get(pk=response['job']) self.assertTrue(j.status == 'new') @@ -76,7 +76,7 @@ class JobTemplateLaunchTest(BaseJobTestMixin, django.test.TransactionTestCase): # Sending extra_vars as a JSON string, implicit credentials with self.current_user(self.user_sue): data = dict(extra_vars = '{\"a\":3}') - response = self.post(self.launch_url, data, expect=202) + response = self.post(self.launch_url, data, expect=201) j = Job.objects.get(pk=response['job']) ev_dict = yaml.load(j.extra_vars) self.assertIn('a', ev_dict) @@ -87,7 +87,7 @@ class JobTemplateLaunchTest(BaseJobTestMixin, django.test.TransactionTestCase): # Sending extra_vars as a JSON string, implicit credentials with self.current_user(self.user_sue): data = dict(extra_vars = 'a: 3') - response = self.post(self.launch_url, data, expect=202) + response = self.post(self.launch_url, data, expect=201) j = Job.objects.get(pk=response['job']) ev_dict = yaml.load(j.extra_vars) self.assertIn('a', ev_dict) @@ -98,7 +98,7 @@ class JobTemplateLaunchTest(BaseJobTestMixin, django.test.TransactionTestCase): # Explicit, credential with self.current_user(self.user_sue): self.cred_sue.delete() - response = self.post(self.launch_url, {'credential': self.cred_doug.pk}, expect=202) + response = self.post(self.launch_url, {'credential': self.cred_doug.pk}, expect=201) j = Job.objects.get(pk=response['job']) self.assertEqual(j.status, 'new') self.assertEqual(j.credential.pk, self.cred_doug.pk) @@ -107,26 +107,26 @@ class JobTemplateLaunchTest(BaseJobTestMixin, django.test.TransactionTestCase): # Explicit, credential with self.current_user(self.user_sue): self.cred_sue.delete() - response = self.post(self.launch_url, {'credential_id': self.cred_doug.pk}, expect=202) + response = self.post(self.launch_url, {'credential_id': self.cred_doug.pk}, expect=201) j = Job.objects.get(pk=response['job']) self.assertEqual(j.status, 'new') self.assertEqual(j.credential.pk, self.cred_doug.pk) - def test_credential_override(self): + def test_credential_override_reject(self): # Explicit, credential with self.current_user(self.user_sue): - response = self.post(self.launch_url, {'credential': self.cred_doug.pk}, expect=202) + response = self.post(self.launch_url, {'credential': self.cred_doug.pk}, expect=201) j = Job.objects.get(pk=response['job']) self.assertEqual(j.status, 'new') - self.assertEqual(j.credential.pk, self.cred_doug.pk) + self.assertEqual(j.credential.pk, self.cred_sue.pk) - def test_credential_override_via_credential_id(self): + def test_credential_override_via_credential_id_reject(self): # Explicit, credential with self.current_user(self.user_sue): - response = self.post(self.launch_url, {'credential_id': self.cred_doug.pk}, expect=202) + response = self.post(self.launch_url, {'credential_id': self.cred_doug.pk}, expect=201) j = Job.objects.get(pk=response['job']) self.assertEqual(j.status, 'new') - self.assertEqual(j.credential.pk, self.cred_doug.pk) + self.assertEqual(j.credential.pk, self.cred_sue.pk) def test_bad_credential_launch_fail(self): # Can't launch a job template without a credential defined (or if we @@ -212,7 +212,7 @@ class JobTemplateLaunchPasswordsTest(BaseJobTestMixin, django.test.TransactionTe def test_explicit_cred_with_ask_password(self): with self.current_user(self.user_sue): - response = self.post(self.launch_url, {'ssh_password': 'whatever'}, expect=202) + response = self.post(self.launch_url, {'ssh_password': 'whatever'}, expect=201) j = Job.objects.get(pk=response['job']) self.assertEqual(j.status, 'new') diff --git a/awx/main/tests/old/jobs/jobs_monolithic.py b/awx/main/tests/old/jobs/jobs_monolithic.py index e174fd55d5..6151fe15c7 100644 --- a/awx/main/tests/old/jobs/jobs_monolithic.py +++ b/awx/main/tests/old/jobs/jobs_monolithic.py @@ -1087,7 +1087,7 @@ class JobTransactionTest(BaseJobTestMixin, django.test.LiveServerTestCase): response = self.get(url) self.assertTrue(response['can_start']) self.assertFalse(response['passwords_needed_to_start']) - response = self.post(url, {}, expect=202) + response = self.post(url, {}, expect=201) job = Job.objects.get(pk=job.pk) self.assertEqual(job.status, 'successful', job.result_stdout) self.assertFalse(errors) @@ -1146,14 +1146,14 @@ class JobTemplateSurveyTest(BaseJobTestMixin, django.test.TransactionTestCase): # should return, and should be able to launch template without error. response = self.get(launch_url) self.assertFalse(response['survey_enabled']) - self.post(launch_url, {}, expect=202) + self.post(launch_url, {}, expect=201) # Now post a survey spec and check that the answer is set in the # job's extra vars. self.post(url, json.loads(TEST_SIMPLE_REQUIRED_SURVEY), expect=200) response = self.get(launch_url) self.assertTrue(response['survey_enabled']) self.assertTrue('favorite_color' in response['variables_needed_to_start']) - response = self.post(launch_url, dict(extra_vars=dict(favorite_color="green")), expect=202) + response = self.post(launch_url, dict(extra_vars=dict(favorite_color="green")), expect=201) job = Job.objects.get(pk=response["job"]) job_extra = json.loads(job.extra_vars) self.assertTrue("favorite_color" in job_extra) @@ -1187,7 +1187,7 @@ class JobTemplateSurveyTest(BaseJobTestMixin, django.test.TransactionTestCase): with self.current_user(self.user_sue): response = self.post(url, json.loads(TEST_SURVEY_REQUIREMENTS), expect=200) # Just the required answer should work - self.post(launch_url, dict(extra_vars=dict(reqd_answer="foo")), expect=202) + self.post(launch_url, dict(extra_vars=dict(reqd_answer="foo")), expect=201) # Short answer but requires a long answer self.post(launch_url, dict(extra_vars=dict(long_answer='a', reqd_answer="foo")), expect=400) # Long answer but requires a short answer @@ -1199,9 +1199,9 @@ class JobTemplateSurveyTest(BaseJobTestMixin, django.test.TransactionTestCase): # Integer that's too big self.post(launch_url, dict(extra_vars=dict(int_answer=10, reqd_answer="foo")), expect=400) # Integer that's just riiiiight - self.post(launch_url, dict(extra_vars=dict(int_answer=3, reqd_answer="foo")), expect=202) + self.post(launch_url, dict(extra_vars=dict(int_answer=3, reqd_answer="foo")), expect=201) # Integer bigger than min with no max defined - self.post(launch_url, dict(extra_vars=dict(int_answer_no_max=3, reqd_answer="foo")), expect=202) + self.post(launch_url, dict(extra_vars=dict(int_answer_no_max=3, reqd_answer="foo")), expect=201) # Integer answer that's the wrong type self.post(launch_url, dict(extra_vars=dict(int_answer="test", reqd_answer="foo")), expect=400) # Float that's too big @@ -1209,7 +1209,7 @@ class JobTemplateSurveyTest(BaseJobTestMixin, django.test.TransactionTestCase): # Float that's too small self.post(launch_url, dict(extra_vars=dict(float_answer=1.995, reqd_answer="foo")), expect=400) # float that's just riiiiight - self.post(launch_url, dict(extra_vars=dict(float_answer=2.01, reqd_answer="foo")), expect=202) + self.post(launch_url, dict(extra_vars=dict(float_answer=2.01, reqd_answer="foo")), expect=201) # float answer that's the wrong type self.post(launch_url, dict(extra_vars=dict(float_answer="test", reqd_answer="foo")), expect=400) # Wrong choice in single choice @@ -1219,11 +1219,11 @@ class JobTemplateSurveyTest(BaseJobTestMixin, django.test.TransactionTestCase): # Wrong type for multi choicen self.post(launch_url, dict(extra_vars=dict(reqd_answer="foo", multi_choice="two")), expect=400) # Right choice in single choice - self.post(launch_url, dict(extra_vars=dict(reqd_answer="foo", single_choice="two")), expect=202) + self.post(launch_url, dict(extra_vars=dict(reqd_answer="foo", single_choice="two")), expect=201) # Right choices in multi choice - self.post(launch_url, dict(extra_vars=dict(reqd_answer="foo", multi_choice=["one", "two"])), expect=202) + self.post(launch_url, dict(extra_vars=dict(reqd_answer="foo", multi_choice=["one", "two"])), expect=201) # Nested json - self.post(launch_url, dict(extra_vars=dict(json_answer=dict(test="val", num=1), reqd_answer="foo")), expect=202) + self.post(launch_url, dict(extra_vars=dict(json_answer=dict(test="val", num=1), reqd_answer="foo")), expect=201) # Bob can access and update the survey because he's an org-admin with self.current_user(self.user_bob): From 0dff851f62226da1931c8094f0ab6a765c396d7f Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Sat, 9 Apr 2016 16:31:32 -0400 Subject: [PATCH 09/20] JT validator: check for prompt_for cases to reject, and flake8 --- awx/main/models/jobs.py | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index fb371e3a9e..3fbf545f01 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -251,6 +251,13 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, ResourceMixin): 'force_handlers', 'skip_tags', 'start_at_task', 'become_enabled', 'labels',] + def clean(self): + if self.job_type == 'scan' and (self.inventory is None or self.ask_inventory_on_launch): + raise ValidationError('Scan jobs must be assigned a fixed inventory') + if (not self.ask_inventory_on_launch) and self.inventory is None: + raise ValidationError('Job Template must either have an inventory or allow prompting for inventory') + return super(JobTemplate, self).clean() + def create_job(self, **kwargs): ''' Create a new job based on this template. @@ -265,9 +272,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 bool(self.credential and not len(self.passwords_needed_to_start) and + not len(self.variables_needed_to_start) and + self.inventory) @property def variables_needed_to_start(self): From b0bbeb2ca8e9c6bcc9faf0ccc7f7fd62bf00e60b Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 11 Apr 2016 15:39:06 -0400 Subject: [PATCH 10/20] move credential to prompt-for mechanism --- awx/api/serializers.py | 20 ++++++----- awx/api/templates/api/job_template_launch.md | 28 ++++++++++++---- awx/api/views.py | 33 +++++++------------ ...elds.py => 0015_v300_prompting_changes.py} | 7 +++- .../0016_v300_prompting_migrations.py | 16 +++++++++ awx/main/migrations/_ask_for_variables.py | 9 +++++ awx/main/models/jobs.py | 9 ++++- awx/main/tests/base.py | 1 + .../functional/api/test_job_runtime_params.py | 9 +++-- awx/main/tests/job_base.py | 1 + awx/main/tests/old/jobs/job_launch.py | 12 ++++--- awx/main/tests/old/jobs/jobs_monolithic.py | 14 ++++---- 12 files changed, 108 insertions(+), 51 deletions(-) rename awx/main/migrations/{0014_v300_job_prompt_for_fields.py => 0015_v300_prompting_changes.py} (87%) create mode 100644 awx/main/migrations/0016_v300_prompting_migrations.py create mode 100644 awx/main/migrations/_ask_for_variables.py diff --git a/awx/api/serializers.py b/awx/api/serializers.py index c6fe2ca325..190036da48 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -1675,7 +1675,7 @@ class JobTemplateSerializer(UnifiedJobTemplateSerializer, JobOptionsSerializer): model = JobTemplate fields = ('*', 'host_config_key', 'ask_variables_on_launch', 'ask_limit_on_launch', 'ask_tags_on_launch', 'ask_job_type_on_launch', 'ask_inventory_on_launch', - 'survey_enabled', 'become_enabled') + 'ask_credential_on_launch', 'survey_enabled', 'become_enabled') def get_related(self, obj): res = super(JobTemplateSerializer, self).get_related(obj) @@ -2104,6 +2104,7 @@ class JobLaunchSerializer(BaseSerializer): can_start_without_user_input = serializers.BooleanField(read_only=True) variables_needed_to_start = serializers.ReadOnlyField() credential_needed_to_start = serializers.SerializerMethodField() + inventory_needed_to_start = serializers.SerializerMethodField() survey_enabled = serializers.SerializerMethodField() extra_vars = VerbatimField(required=False, write_only=True) @@ -2111,13 +2112,13 @@ class JobLaunchSerializer(BaseSerializer): model = JobTemplate fields = ('can_start_without_user_input', 'passwords_needed_to_start', 'extra_vars', 'limit', 'job_tags', 'skip_tags', 'job_type', 'inventory', - 'ask_variables_on_launch', 'ask_tags_on_launch', 'ask_job_type_on_launch', - 'ask_inventory_on_launch', 'ask_limit_on_launch', + 'credential', 'ask_variables_on_launch', 'ask_tags_on_launch', + 'ask_job_type_on_launch', 'ask_inventory_on_launch', 'ask_limit_on_launch', 'survey_enabled', 'variables_needed_to_start', - 'credential', 'credential_needed_to_start',) + 'credential_needed_to_start', 'inventory_needed_to_start',) read_only_fields = ('ask_variables_on_launch', 'ask_limit_on_launch', 'ask_tags_on_launch', 'ask_job_type_on_launch', - 'ask_inventory_on_launch') + 'ask_inventory_on_launch', 'ask_credential_on_launch') extra_kwargs = { 'credential': {'write_only': True,}, 'limit': {'write_only': True,}, @@ -2130,6 +2131,9 @@ class JobLaunchSerializer(BaseSerializer): def get_credential_needed_to_start(self, obj): return not (obj and obj.credential) + def get_inventory_needed_to_start(self, obj): + return not (obj and obj.inventory) + def get_survey_enabled(self, obj): if obj: return obj.survey_enabled and 'spec' in obj.survey_spec @@ -2140,10 +2144,8 @@ class JobLaunchSerializer(BaseSerializer): obj = self.context.get('obj') data = self.context.get('data') - if obj and obj.credential is not None: - # force job template to override runtime credential, if present + if (not obj.ask_credential_on_launch) or (not attrs.get('credential', None)): credential = obj.credential - attrs.pop('credential', None) else: credential = attrs.get('credential', None) if not credential: @@ -2191,6 +2193,7 @@ class JobLaunchSerializer(BaseSerializer): JT_job_tags = obj.job_tags JT_skip_tags = obj.skip_tags JT_inventory = obj.inventory + JT_credential = obj.credential attrs = super(JobLaunchSerializer, self).validate(attrs) obj.extra_vars = JT_extra_vars obj.limit = JT_limit @@ -2198,6 +2201,7 @@ class JobLaunchSerializer(BaseSerializer): obj.skip_tags = JT_skip_tags obj.job_tags = JT_job_tags obj.inventory = JT_inventory + obj.credential = JT_credential return attrs class NotifierSerializer(BaseSerializer): diff --git a/awx/api/templates/api/job_template_launch.md b/awx/api/templates/api/job_template_launch.md index 1dddde4210..5bbf5c507d 100644 --- a/awx/api/templates/api/job_template_launch.md +++ b/awx/api/templates/api/job_template_launch.md @@ -6,6 +6,16 @@ The response will include the following fields: * `ask_variables_on_launch`: Flag indicating whether the job_template is configured to prompt for variables upon launch (boolean, read-only) +* `ask_tags_on_launch`: Flag indicating whether the job_template is + configured to prompt for tags upon launch (boolean, read-only) +* `ask_job_type_on_launch`: Flag indicating whether the job_template is + configured to prompt for job_type upon launch (boolean, read-only) +* `ask_limit_on_launch`: Flag indicating whether the job_template is + configured to prompt for limit upon launch (boolean, read-only) +* `ask_inventory_on_launch`: Flag indicating whether the job_template is + configured to prompt for limit upon launch (boolean, read-only) +* `ask_credential_on_launch`: Flag indicating whether the job_template is + configured to prompt for limit upon launch (boolean, read-only) * `can_start_without_user_input`: Flag indicating if the job_template can be launched without user-input (boolean, read-only) * `passwords_needed_to_start`: Password names required to launch the @@ -17,13 +27,19 @@ The response will include the following fields: * `credential_needed_to_start`: Flag indicating the presence of a credential associated with the job template. If not then one should be supplied when launching the job (boolean, read-only) +* `inventory_needed_to_start`: Flag indicating the presence of an inventory + associated with the job template. If not then one should be supplied when + launching the job (boolean, read-only) Make a POST request to this resource to launch the job_template. If any -passwords or extra variables (extra_vars) are required, they must be passed -via POST data, with extra_vars given as a YAML or JSON string and escaped -parentheses. If `credential_needed_to_start` is `True` then the `credential` -field is required as well. +passwords, inventory, or extra variables (extra_vars) are required, they must +be passed via POST data, with extra_vars given as a YAML or JSON string and +escaped parentheses. If `credential_needed_to_start` is `True` then the +`credential` field is required and if the `inventory_needed_to_start` is +`True` then the `inventory` is required as well. -If successful, the response status code will be 202. If any required passwords +If successful, the response status code will be 201. If any required passwords are not provided, a 400 status code will be returned. If the job cannot be -launched, a 405 status code will be returned. +launched, a 405 status code will be returned. If the provided credential or +inventory are not allowed to be used by the user, then a 403 status code will +be returned. diff --git a/awx/api/views.py b/awx/api/views.py index 03caa2c37b..eae58b548a 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2086,10 +2086,6 @@ class JobTemplateLaunch(RetrieveAPIView, GenericAPIView): if obj: for p in obj.passwords_needed_to_start: data[p] = u'' - if obj.credential: - data.pop('credential', None) - else: - data['credential'] = None for v in obj.variables_needed_to_start: extra_vars.setdefault(v, u'') ask_for_vars_dict = obj._ask_for_vars_dict() @@ -2098,12 +2094,8 @@ class JobTemplateLaunch(RetrieveAPIView, GenericAPIView): data.pop(field, None) elif field == 'extra_vars': data[field] = extra_vars - elif field == 'inventory': - inv_obj = getattr(obj, field) - if inv_obj in ('', None): - data[field] = None - else: - data[field] = inv_obj.id + elif field == 'inventory' or field == 'credential': + data[field] = getattrd(obj, "%s.%s" % (field, 'id'), None) else: data[field] = getattr(obj, field) return data @@ -2123,22 +2115,19 @@ class JobTemplateLaunch(RetrieveAPIView, GenericAPIView): if not serializer.is_valid(): return Response(serializer.errors, status=status.HTTP_400_BAD_REQUEST) - # At this point, a credential is gauranteed to exist at serializer.instance.credential - if not request.user.can_access(Credential, 'read', serializer.instance.credential): - raise PermissionDenied() - - kv = { - 'credential': serializer.instance.credential.pk, - } - prompted_fields, ignored_fields = obj._accept_or_ignore_job_kwargs(**request.data) - if 'inventory' in prompted_fields: + if 'credential' in prompted_fields and prompted_fields['credential'] != getattrd(obj, 'credential.pk', None): + new_credential = Credential.objects.get(pk=prompted_fields['credential']) + if not request.user.can_access(Credential, 'read', new_credential): + raise PermissionDenied() + + if 'inventory' in prompted_fields and prompted_fields['inventory'] != getattrd(obj, 'inventory.pk', None): new_inventory = Inventory.objects.get(pk=prompted_fields['inventory']) if not request.user.can_access(Inventory, 'read', new_inventory): raise PermissionDenied() - kv.update(prompted_fields) + kv = prompted_fields kv.update(passwords) new_job = obj.create_unified_job(**kv) @@ -2435,7 +2424,7 @@ class JobTemplateCallback(GenericAPIView): # Return the location of the new job. headers = {'Location': job.get_absolute_url()} - return Response(status=status.HTTP_202_ACCEPTED, headers=headers) + return Response(status=status.HTTP_201_CREATED, headers=headers) class JobTemplateJobsList(SubListCreateAPIView): @@ -2483,7 +2472,7 @@ class SystemJobTemplateLaunch(GenericAPIView): new_job = obj.create_unified_job(**request.data) new_job.signal_start(**request.data) data = dict(system_job=new_job.id) - return Response(data, status=status.HTTP_202_ACCEPTED) + return Response(data, status=status.HTTP_201_CREATED) class SystemJobTemplateSchedulesList(SubListCreateAttachDetachAPIView): diff --git a/awx/main/migrations/0014_v300_job_prompt_for_fields.py b/awx/main/migrations/0015_v300_prompting_changes.py similarity index 87% rename from awx/main/migrations/0014_v300_job_prompt_for_fields.py rename to awx/main/migrations/0015_v300_prompting_changes.py index e7e5a5b623..8c8dfa4456 100644 --- a/awx/main/migrations/0014_v300_job_prompt_for_fields.py +++ b/awx/main/migrations/0015_v300_prompting_changes.py @@ -9,7 +9,7 @@ from django.conf import settings class Migration(migrations.Migration): dependencies = [ - ('main', '0013_v300_label_changes'), + ('main', '0014_v300_invsource_cred'), ] operations = [ @@ -23,6 +23,11 @@ class Migration(migrations.Migration): name='ask_inventory_on_launch', field=models.BooleanField(default=False), ), + migrations.AddField( + model_name='jobtemplate', + name='ask_credential_on_launch', + field=models.BooleanField(default=False), + ), migrations.AddField( model_name='jobtemplate', name='ask_job_type_on_launch', diff --git a/awx/main/migrations/0016_v300_prompting_migrations.py b/awx/main/migrations/0016_v300_prompting_migrations.py new file mode 100644 index 0000000000..3f02a1714d --- /dev/null +++ b/awx/main/migrations/0016_v300_prompting_migrations.py @@ -0,0 +1,16 @@ +# -*- coding: utf-8 -*- +from __future__ import unicode_literals + +from awx.main.migrations import _ask_for_variables as ask_for_variables +from django.db import migrations + + +class Migration(migrations.Migration): + + dependencies = [ + ('main', '0015_v300_prompting_changes'), + ] + + operations = [ + migrations.RunPython(ask_for_variables.migrate_credential), + ] diff --git a/awx/main/migrations/_ask_for_variables.py b/awx/main/migrations/_ask_for_variables.py new file mode 100644 index 0000000000..932849b638 --- /dev/null +++ b/awx/main/migrations/_ask_for_variables.py @@ -0,0 +1,9 @@ +def migrate_credential(apps, schema_editor): + '''If credential is not currently present, set ask_for_credential_on_launch + equal to True, and otherwise leave it as the default False value. + ''' + JobTemplate = apps.get_model('main', 'JobTemplate') + for jt in JobTemplate.objects.iterator(): + if jt.credential is None: + jt.ask_credential_on_launch = True + jt.save() diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index 3fbf545f01..725e55bbe4 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -212,6 +212,10 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, ResourceMixin): blank=True, default=False, ) + ask_credential_on_launch = models.BooleanField( + blank=True, + default=False, + ) survey_enabled = models.BooleanField( default=False, @@ -256,6 +260,8 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, ResourceMixin): raise ValidationError('Scan jobs must be assigned a fixed inventory') if (not self.ask_inventory_on_launch) and self.inventory is None: raise ValidationError('Job Template must either have an inventory or allow prompting for inventory') + if (not self.ask_credential_on_launch) and self.credential is None: + raise ValidationError('Job Template must either have a credential or allow prompting for credential') return super(JobTemplate, self).clean() def create_job(self, **kwargs): @@ -396,7 +402,8 @@ class JobTemplate(UnifiedJobTemplate, JobOptions, ResourceMixin): job_tags=self.ask_tags_on_launch, skip_tags=self.ask_tags_on_launch, job_type=self.ask_job_type_on_launch, - inventory=self.ask_inventory_on_launch + inventory=self.ask_inventory_on_launch, + credential=self.ask_credential_on_launch ) def _accept_or_ignore_job_kwargs(self, **kwargs): diff --git a/awx/main/tests/base.py b/awx/main/tests/base.py index 64467d3c8f..789bbb8c86 100644 --- a/awx/main/tests/base.py +++ b/awx/main/tests/base.py @@ -352,6 +352,7 @@ class BaseTestMixin(QueueTestMixin, MockCommonlySlowTestMixin): 'host_config_key': settings.SYSTEM_UUID, 'created_by': created_by, 'playbook': playbook, + 'ask_credential_on_launch': True, } opts.update(kwargs) return JobTemplate.objects.create(**opts) 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 afdd002828..6689825ccb 100644 --- a/awx/main/tests/functional/api/test_job_runtime_params.py +++ b/awx/main/tests/functional/api/test_job_runtime_params.py @@ -37,6 +37,7 @@ def job_template_prompts(project, inventory, machine_credential): ask_job_type_on_launch=on_off, ask_inventory_on_launch=on_off, ask_limit_on_launch=on_off, + ask_credential_on_launch=on_off, ) return rf @@ -59,12 +60,14 @@ def test_job_ignore_unprompted_vars(runtime_data, job_template_prompts, post, us assert job_obj.job_type == job_template_saved.job_type assert job_obj.inventory.pk == job_template_saved.inventory.pk assert job_obj.job_tags == job_template_saved.job_tags + assert job_obj.credential.pk == job_template_saved.credential.pk # Check that response tells us what things were ignored assert 'job_launch_var' in response.data['ignored_fields']['extra_vars'] assert 'job_type' in response.data['ignored_fields'] assert 'limit' in response.data['ignored_fields'] assert 'inventory' in response.data['ignored_fields'] + assert 'credential' in response.data['ignored_fields'] assert 'job_tags' in response.data['ignored_fields'] assert 'skip_tags' in response.data['ignored_fields'] @@ -89,6 +92,7 @@ def test_job_accept_prompted_vars(runtime_data, job_template_prompts, post, user assert job_obj.limit == runtime_data['limit'] assert job_obj.job_type == runtime_data['job_type'] assert job_obj.inventory.pk == runtime_data['inventory'] + assert job_obj.credential.pk == runtime_data['credential'] assert job_obj.job_tags == runtime_data['job_tags'] @pytest.mark.django_db @@ -99,11 +103,12 @@ def test_job_reject_invalid_prompted_vars(runtime_data, job_template_prompts, po response = post( reverse('api:job_template_launch', args=[job_template.pk]), dict(job_type='foobicate', # foobicate is not a valid job type - inventory=87865), user('admin', True)) + inventory=87865, credential=48474), user('admin', True)) assert response.status_code == 400 assert response.data['job_type'] == [u'"foobicate" is not a valid choice.'] assert response.data['inventory'] == [u'Invalid pk "87865" - object does not exist.'] + assert response.data['credential'] == [u'Invalid pk "48474" - object does not exist.'] @pytest.mark.django_db @pytest.mark.job_runtime_vars @@ -189,7 +194,7 @@ def test_job_relaunch_prompted_vars(runtime_data, job_template_prompts, post, us @pytest.mark.django_db def test_job_launch_JT_with_validation(machine_credential, deploy_jobtemplate): deploy_jobtemplate.extra_vars = '{"job_template_var": 3}' - deploy_jobtemplate.credential = None + deploy_jobtemplate.ask_credential_on_launch = True deploy_jobtemplate.save() kv = dict(extra_vars={"job_launch_var": 4}, credential=machine_credential.id) diff --git a/awx/main/tests/job_base.py b/awx/main/tests/job_base.py index 9cea21e2cd..fb88134f61 100644 --- a/awx/main/tests/job_base.py +++ b/awx/main/tests/job_base.py @@ -503,6 +503,7 @@ class BaseJobTestMixin(BaseTestMixin): playbook=self.proj_dev.playbooks[0], host_config_key=uuid.uuid4().hex, created_by=self.user_sue, + ask_credential_on_launch=True, ) # self.job_eng_run = self.jt_eng_run.create_job( # created_by=self.user_sue, diff --git a/awx/main/tests/old/jobs/job_launch.py b/awx/main/tests/old/jobs/job_launch.py index 27a44ddb21..f1f9f607eb 100644 --- a/awx/main/tests/old/jobs/job_launch.py +++ b/awx/main/tests/old/jobs/job_launch.py @@ -28,6 +28,7 @@ class JobTemplateLaunchTest(BaseJobTestMixin, django.test.TransactionTestCase): credential = self.cred_sue.pk, playbook = self.proj_dev.playbooks[0], ask_variables_on_launch = True, + ask_credential_on_launch = True, ) self.data_no_cred = dict( name = 'launched job template no credential', @@ -35,6 +36,8 @@ class JobTemplateLaunchTest(BaseJobTestMixin, django.test.TransactionTestCase): inventory = self.inv_eng.pk, project = self.proj_dev.pk, playbook = self.proj_dev.playbooks[0], + ask_credential_on_launch = True, + ask_variables_on_launch = True, ) self.data_cred_ask = dict(self.data) self.data_cred_ask['name'] = 'launched job templated with ask passwords' @@ -112,21 +115,21 @@ class JobTemplateLaunchTest(BaseJobTestMixin, django.test.TransactionTestCase): self.assertEqual(j.status, 'new') self.assertEqual(j.credential.pk, self.cred_doug.pk) - def test_credential_override_reject(self): + def test_credential_override(self): # Explicit, credential with self.current_user(self.user_sue): response = self.post(self.launch_url, {'credential': self.cred_doug.pk}, expect=201) j = Job.objects.get(pk=response['job']) self.assertEqual(j.status, 'new') - self.assertEqual(j.credential.pk, self.cred_sue.pk) + self.assertEqual(j.credential.pk, self.cred_doug.pk) - def test_credential_override_via_credential_id_reject(self): + def test_credential_override_via_credential_id(self): # Explicit, credential with self.current_user(self.user_sue): response = self.post(self.launch_url, {'credential_id': self.cred_doug.pk}, expect=201) j = Job.objects.get(pk=response['job']) self.assertEqual(j.status, 'new') - self.assertEqual(j.credential.pk, self.cred_sue.pk) + self.assertEqual(j.credential.pk, self.cred_doug.pk) def test_bad_credential_launch_fail(self): # Can't launch a job template without a credential defined (or if we @@ -192,6 +195,7 @@ class JobTemplateLaunchPasswordsTest(BaseJobTestMixin, django.test.TransactionTe project = self.proj_dev.pk, credential = self.cred_sue_ask.pk, playbook = self.proj_dev.playbooks[0], + ask_credential_on_launch = True, ) with self.current_user(self.user_sue): diff --git a/awx/main/tests/old/jobs/jobs_monolithic.py b/awx/main/tests/old/jobs/jobs_monolithic.py index 6151fe15c7..fa4123ad05 100644 --- a/awx/main/tests/old/jobs/jobs_monolithic.py +++ b/awx/main/tests/old/jobs/jobs_monolithic.py @@ -798,7 +798,7 @@ class JobTemplateCallbackTest(BaseJobTestMixin, django.test.LiveServerTestCase): self.assertEqual(jobs_qs.count(), 0) # Create the job itself. - result = self.post(url, data, expect=202, remote_addr=host_ip) + result = self.post(url, data, expect=201, remote_addr=host_ip) # Establish that we got back what we expect, and made the changes # that we expect. @@ -813,7 +813,7 @@ class JobTemplateCallbackTest(BaseJobTestMixin, django.test.LiveServerTestCase): self.assertEqual(job.hosts.all()[0], host) # Create the job itself using URL-encoded form data instead of JSON. - result = self.post(url, data, expect=202, remote_addr=host_ip, data_type='form') + result = self.post(url, data, expect=201, remote_addr=host_ip, data_type='form') # Establish that we got back what we expect, and made the changes # that we expect. @@ -829,7 +829,7 @@ class JobTemplateCallbackTest(BaseJobTestMixin, django.test.LiveServerTestCase): # Run the callback job again with extra vars and verify their presence data.update(dict(extra_vars=dict(key="value"))) - result = self.post(url, data, expect=202, remote_addr=host_ip) + result = self.post(url, data, expect=201, remote_addr=host_ip) jobs_qs = job_template.jobs.filter(launch_type='callback').order_by('-pk') job = jobs_qs[0] self.assertTrue("key" in job.extra_vars) @@ -878,7 +878,7 @@ class JobTemplateCallbackTest(BaseJobTestMixin, django.test.LiveServerTestCase): break self.assertTrue(host) self.assertEqual(jobs_qs.count(), 3) - self.post(url, data, expect=202, remote_addr=host_ip) + self.post(url, data, expect=201, remote_addr=host_ip) self.assertEqual(jobs_qs.count(), 4) job = jobs_qs[0] self.assertEqual(job.launch_type, 'callback') @@ -903,7 +903,7 @@ class JobTemplateCallbackTest(BaseJobTestMixin, django.test.LiveServerTestCase): break self.assertTrue(host) self.assertEqual(jobs_qs.count(), 4) - self.post(url, data, expect=202, remote_addr=host_ip) + self.post(url, data, expect=201, remote_addr=host_ip) self.assertEqual(jobs_qs.count(), 5) job = jobs_qs[0] self.assertEqual(job.launch_type, 'callback') @@ -917,7 +917,7 @@ class JobTemplateCallbackTest(BaseJobTestMixin, django.test.LiveServerTestCase): host = host_qs[0] host_ip = host.variables_dict['ansible_ssh_host'] self.assertEqual(jobs_qs.count(), 5) - self.post(url, data, expect=202, remote_addr=host_ip) + self.post(url, data, expect=201, remote_addr=host_ip) self.assertEqual(jobs_qs.count(), 6) job = jobs_qs[0] self.assertEqual(job.launch_type, 'callback') @@ -951,7 +951,7 @@ class JobTemplateCallbackTest(BaseJobTestMixin, django.test.LiveServerTestCase): break self.assertTrue(host) self.assertEqual(jobs_qs.count(), 6) - self.post(url, data, expect=202, remote_addr=host_ip) + self.post(url, data, expect=201, remote_addr=host_ip) self.assertEqual(jobs_qs.count(), 7) job = jobs_qs[0] self.assertEqual(job.launch_type, 'callback') From 6fc51e2d85870ff79e2214d27be4426131141559 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Tue, 12 Apr 2016 16:48:04 -0400 Subject: [PATCH 11/20] preparing for new incoming migrations --- ...v300_prompting_changes.py => 0016_v300_prompting_changes.py} | 2 +- ...rompting_migrations.py => 0017_v300_prompting_migrations.py} | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) rename awx/main/migrations/{0015_v300_prompting_changes.py => 0016_v300_prompting_changes.py} (97%) rename awx/main/migrations/{0016_v300_prompting_migrations.py => 0017_v300_prompting_migrations.py} (87%) diff --git a/awx/main/migrations/0015_v300_prompting_changes.py b/awx/main/migrations/0016_v300_prompting_changes.py similarity index 97% rename from awx/main/migrations/0015_v300_prompting_changes.py rename to awx/main/migrations/0016_v300_prompting_changes.py index 8c8dfa4456..63edda959b 100644 --- a/awx/main/migrations/0015_v300_prompting_changes.py +++ b/awx/main/migrations/0016_v300_prompting_changes.py @@ -9,7 +9,7 @@ from django.conf import settings class Migration(migrations.Migration): dependencies = [ - ('main', '0014_v300_invsource_cred'), + ('main', '0015_v300_label_changes'), ] operations = [ diff --git a/awx/main/migrations/0016_v300_prompting_migrations.py b/awx/main/migrations/0017_v300_prompting_migrations.py similarity index 87% rename from awx/main/migrations/0016_v300_prompting_migrations.py rename to awx/main/migrations/0017_v300_prompting_migrations.py index 3f02a1714d..f08d760a8c 100644 --- a/awx/main/migrations/0016_v300_prompting_migrations.py +++ b/awx/main/migrations/0017_v300_prompting_migrations.py @@ -8,7 +8,7 @@ from django.db import migrations class Migration(migrations.Migration): dependencies = [ - ('main', '0015_v300_prompting_changes'), + ('main', '0016_v300_prompting_changes'), ] operations = [ From 914918b5aec59e4973b67b29524116fc5f344570 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 13 Apr 2016 06:34:25 -0400 Subject: [PATCH 12/20] update permission logic, update job runtime tests --- awx/api/views.py | 4 +- .../functional/api/test_job_runtime_params.py | 123 ++++++++++++------ 2 files changed, 87 insertions(+), 40 deletions(-) diff --git a/awx/api/views.py b/awx/api/views.py index eae58b548a..e49f8408bd 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2119,12 +2119,12 @@ class JobTemplateLaunch(RetrieveAPIView, GenericAPIView): if 'credential' in prompted_fields and prompted_fields['credential'] != getattrd(obj, 'credential.pk', None): new_credential = Credential.objects.get(pk=prompted_fields['credential']) - if not request.user.can_access(Credential, 'read', new_credential): + if not request.user.can_access(Credential, 'use', new_credential): raise PermissionDenied() if 'inventory' in prompted_fields and prompted_fields['inventory'] != getattrd(obj, 'inventory.pk', None): new_inventory = Inventory.objects.get(pk=prompted_fields['inventory']) - if not request.user.can_access(Inventory, 'read', new_inventory): + if not request.user.can_access(Inventory, 'use', new_inventory): raise PermissionDenied() kv = prompted_fields 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 6689825ccb..cc417111db 100644 --- a/awx/main/tests/functional/api/test_job_runtime_params.py +++ b/awx/main/tests/functional/api/test_job_runtime_params.py @@ -41,6 +41,22 @@ def job_template_prompts(project, inventory, machine_credential): ) return rf +@pytest.fixture +def job_template_prompts_null(project): + return JobTemplate.objects.create( + job_type='run', + project=project, + inventory=None, + credential=None, + name='deploy-job-template', + ask_variables_on_launch=True, + ask_tags_on_launch=True, + ask_job_type_on_launch=True, + ask_inventory_on_launch=True, + ask_limit_on_launch=True, + ask_credential_on_launch=True, + ) + @pytest.mark.django_db @pytest.mark.job_runtime_vars def test_job_ignore_unprompted_vars(runtime_data, job_template_prompts, post, user): @@ -81,7 +97,38 @@ def test_job_accept_prompted_vars(runtime_data, job_template_prompts, post, user job_template.inventory.save() response = post(reverse('api:job_template_launch', args=[job_template.pk]), - runtime_data, user('admin', True)) + runtime_data, admin_user) + + assert response.status_code == 201 + job_id = response.data['job'] + job_obj = Job.objects.get(pk=job_id) + + # Check that job data matches the given runtime variables + assert 'job_launch_var' in yaml.load(job_obj.extra_vars) + assert job_obj.limit == runtime_data['limit'] + assert job_obj.job_type == runtime_data['job_type'] + assert job_obj.inventory.pk == runtime_data['inventory'] + assert job_obj.credential.pk == runtime_data['credential'] + assert job_obj.job_tags == runtime_data['job_tags'] + +@pytest.mark.django_db +@pytest.mark.skip(reason="JT can_start without inventory needs to be fixed before passing") +@pytest.mark.job_runtime_vars +def test_job_accept_prompted_vars_null(runtime_data, job_template_prompts_null, post, user): + job_template = job_template_prompts_null + common_user = user('admin', False) + + job_template.executor_role.members.add(common_user) + job_template.save() + job_template.project.member_role.members.add(common_user) + job_template.project.save() + + credential = Credential.objects.get(pk=runtime_data['credential']) + credential.usage_role.members.add(common_user) + credential.save() + + response = post(reverse('api:job_template_launch', args=[job_template.pk]), + runtime_data, common_user) assert response.status_code == 201 job_id = response.data['job'] @@ -140,15 +187,13 @@ def test_job_launch_fails_without_inventory_access(deploy_jobtemplate, machine_c deploy_jobtemplate.ask_inventory_on_launch = True deploy_jobtemplate.credential = machine_credential common_user = user('test-user', False) - # TODO: Change admin_role to executor_role once issue #1422 is resolved - deploy_jobtemplate.admin_role.members.add(common_user) + deploy_jobtemplate.executor_role.members.add(common_user) deploy_jobtemplate.save() - deploy_jobtemplate.inventory.executor_role.members.add(common_user) + deploy_jobtemplate.inventory.usage_role.members.add(common_user) deploy_jobtemplate.inventory.save() deploy_jobtemplate.project.member_role.members.add(common_user) deploy_jobtemplate.project.save() - # TODO: change owner_role to usage_role after fix - deploy_jobtemplate.credential.owner_role.members.add(common_user) + deploy_jobtemplate.credential.usage_role.members.add(common_user) deploy_jobtemplate.credential.save() # Assure that the base job template can be launched to begin with @@ -215,38 +260,40 @@ def test_job_launch_JT_with_validation(machine_credential, deploy_jobtemplate): @pytest.mark.django_db @pytest.mark.job_runtime_vars -def test_job_launch_unprompted_vars_with_survey(job_template_prompts, post, user): - job_template = job_template_prompts(False) - job_template.survey_enabled = True - job_template.survey_spec = { - "spec": [ - { - "index": 0, - "question_name": "survey_var", - "min": 0, - "default": "", - "max": 100, - "question_description": "A survey question", - "required": True, - "variable": "survey_var", - "choices": "", - "type": "integer" - } - ], - "description": "", - "name": "" - } - job_template.save() +def test_job_launch_unprompted_vars_with_survey(mocker, job_template_prompts, post, user): + with mocker.patch('awx.main.access.BaseAccess.check_license', return_value=False): + job_template = job_template_prompts(False) + job_template.survey_enabled = True + job_template.survey_spec = { + "spec": [ + { + "index": 0, + "question_name": "survey_var", + "min": 0, + "default": "", + "max": 100, + "question_description": "A survey question", + "required": True, + "variable": "survey_var", + "choices": "", + "type": "integer" + } + ], + "description": "", + "name": "" + } + job_template.save() - response = post( - reverse('api:job_template_launch', args=[job_template.pk]), - dict(extra_vars={"job_launch_var": 3, "survey_var": 4}), - user('admin', True)) + response = post( + reverse('api:job_template_launch', args=[job_template.pk]), + dict(extra_vars={"job_launch_var": 3, "survey_var": 4}), + user('admin', True)) + assert response.status_code == 201 - job_id = response.data['job'] - job_obj = Job.objects.get(pk=job_id) + job_id = response.data['job'] + job_obj = Job.objects.get(pk=job_id) - # Check that the survey variable is accepted and the job variable isn't - job_extra_vars = yaml.load(job_obj.extra_vars) - assert 'job_launch_var' not in job_extra_vars - assert 'survey_var' in job_extra_vars + # Check that the survey variable is accepted and the job variable isn't + job_extra_vars = yaml.load(job_obj.extra_vars) + assert 'job_launch_var' not in job_extra_vars + assert 'survey_var' in job_extra_vars From dc37f00d0aece3b48cd009ecedf63ff3f6cdcafc Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 13 Apr 2016 15:19:10 -0400 Subject: [PATCH 13/20] use parent serializer method to avoid API browser job access check --- awx/api/views.py | 5 +++-- awx/main/access.py | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/awx/api/views.py b/awx/api/views.py index e49f8408bd..a3ba16f9bb 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2137,9 +2137,10 @@ class JobTemplateLaunch(RetrieveAPIView, GenericAPIView): new_job.delete() return Response(data, status=status.HTTP_400_BAD_REQUEST) else: - data = JobSerializer(new_job).data - data['job'] = new_job.id + data = OrderedDict() data['ignored_fields'] = ignored_fields + data.update(JobSerializer(new_job).to_representation(new_job)) + data['job'] = new_job.id return Response(data, status=status.HTTP_201_CREATED) class JobTemplateSchedulesList(SubListCreateAttachDetachAPIView): diff --git a/awx/main/access.py b/awx/main/access.py index b250f9b40b..565a9be47f 100644 --- a/awx/main/access.py +++ b/awx/main/access.py @@ -766,7 +766,7 @@ class JobTemplateAccess(BaseAccess): self.check_license() if obj.job_type == PERM_INVENTORY_SCAN: self.check_license(feature='system_tracking') - if getattr(obj, 'survey_enabled', None): + if obj.survey_enabled: self.check_license(feature='surveys') # Super users can start any job From d1960241559368cf89adaac3dc636c6bfc7eba56 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Wed, 13 Apr 2016 15:32:03 -0400 Subject: [PATCH 14/20] fix problems with wording in template --- awx/api/templates/api/job_template_launch.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/awx/api/templates/api/job_template_launch.md b/awx/api/templates/api/job_template_launch.md index 5bbf5c507d..0c17c3d842 100644 --- a/awx/api/templates/api/job_template_launch.md +++ b/awx/api/templates/api/job_template_launch.md @@ -13,16 +13,16 @@ The response will include the following fields: * `ask_limit_on_launch`: Flag indicating whether the job_template is configured to prompt for limit upon launch (boolean, read-only) * `ask_inventory_on_launch`: Flag indicating whether the job_template is - configured to prompt for limit upon launch (boolean, read-only) + configured to prompt for inventory upon launch (boolean, read-only) * `ask_credential_on_launch`: Flag indicating whether the job_template is - configured to prompt for limit upon launch (boolean, read-only) + configured to prompt for credential upon launch (boolean, read-only) * `can_start_without_user_input`: Flag indicating if the job_template can be launched without user-input (boolean, read-only) * `passwords_needed_to_start`: Password names required to launch the job_template (array, read-only) * `variables_needed_to_start`: Required variable names required to launch the job_template (array, read-only) -* `survey_enabled`: Flag indicating if whether the job_template has an enabled +* `survey_enabled`: Flag indicating whether the job_template has an enabled survey (boolean, read-only) * `credential_needed_to_start`: Flag indicating the presence of a credential associated with the job template. If not then one should be supplied when From 8c4e4f4c0d282889eeda057bd510cfdfaad13def Mon Sep 17 00:00:00 2001 From: Matthew Jones Date: Fri, 15 Apr 2016 14:42:51 -0400 Subject: [PATCH 15/20] Creating pre-loaded demo data * Bootstrap some initial data if there is no default org * Renamed default org script appropriately --- .../management/commands/create_default_org.py | 27 ----------- .../commands/create_preload_data.py | 47 +++++++++++++++++++ 2 files changed, 47 insertions(+), 27 deletions(-) delete mode 100644 awx/main/management/commands/create_default_org.py create mode 100644 awx/main/management/commands/create_preload_data.py diff --git a/awx/main/management/commands/create_default_org.py b/awx/main/management/commands/create_default_org.py deleted file mode 100644 index dbdb64ef85..0000000000 --- a/awx/main/management/commands/create_default_org.py +++ /dev/null @@ -1,27 +0,0 @@ -# Copyright (c) 2015 Ansible, Inc. -# All Rights Reserved - -from django.core.management.base import BaseCommand -from crum import impersonate -from awx.main.models import User, Organization - - -class Command(BaseCommand): - """Creates the default organization if and only if no organizations - exist in the system. - """ - help = 'Creates a default organization iff there are none.' - - def handle(self, *args, **kwargs): - # Sanity check: Is there already an organization in the system? - if Organization.objects.count(): - return - - # Create a default organization as the first superuser found. - try: - superuser = User.objects.filter(is_superuser=True).order_by('pk')[0] - except IndexError: - superuser = None - with impersonate(superuser): - Organization.objects.create(name='Default') - print('Default organization added.') diff --git a/awx/main/management/commands/create_preload_data.py b/awx/main/management/commands/create_preload_data.py new file mode 100644 index 0000000000..37b65c3bf4 --- /dev/null +++ b/awx/main/management/commands/create_preload_data.py @@ -0,0 +1,47 @@ +# Copyright (c) 2015 Ansible, Inc. +# All Rights Reserved + +from django.core.management.base import BaseCommand +from crum import impersonate +from awx.main.models import User, Organization, Project, Inventory, Credential, Host, JobTemplate + + +class Command(BaseCommand): + """Create preloaded data, intended for new installs + """ + help = 'Creates a preload tower data iff there is none.' + + def handle(self, *args, **kwargs): + # Sanity check: Is there already an organization in the system? + if Organization.objects.count(): + return + + # Create a default organization as the first superuser found. + try: + superuser = User.objects.filter(is_superuser=True).order_by('pk')[0] + except IndexError: + superuser = None + with impersonate(superuser): + o = Organization.objects.create(name='Default') + p = Project.objects.create(name='Demo Project', + scm_type='git', + scm_url='https://github.com/ansible/ansible-tower-samples', + scm_update_on_launch=True, + scm_update_cache_timeout=0, + organization=o) + c = Credential.objects.create(name='Demo Credential', + username=superuser.username, + created_by=superuser) + c.owner_role.members.add(superuser) + i = Inventory.objects.create(name='Demo Inventory', + organization=o, + created_by=superuser) + h = Host.objects.create(name='localhost', + inventory=i, + variables="ansible_connection: local", + created_by=superuser) + jt = JobTemplate.objects.create(name='Demo Job Template', + project=p, + inventory=i, + credential=c) + print('Default organization added.') From e0bd906de885c7e138c79018c8b9f0948655b14d Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Fri, 15 Apr 2016 14:59:20 -0400 Subject: [PATCH 16/20] fill in extra_vars to content box even if ask_for_vars false and survey enabled --- awx/api/views.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/awx/api/views.py b/awx/api/views.py index a3ba16f9bb..2538035410 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2082,18 +2082,19 @@ class JobTemplateLaunch(RetrieveAPIView, GenericAPIView): def update_raw_data(self, data): obj = self.get_object() - extra_vars = data.get('extra_vars') or {} + extra_vars = data.pop('extra_vars', None) or {} if obj: for p in obj.passwords_needed_to_start: data[p] = u'' for v in obj.variables_needed_to_start: extra_vars.setdefault(v, u'') + if extra_vars: + data['extra_vars'] = extra_vars ask_for_vars_dict = obj._ask_for_vars_dict() + ask_for_vars_dict.pop('extra_vars') for field in ask_for_vars_dict: if not ask_for_vars_dict[field]: data.pop(field, None) - elif field == 'extra_vars': - data[field] = extra_vars elif field == 'inventory' or field == 'credential': data[field] = getattrd(obj, "%s.%s" % (field, 'id'), None) else: From d1801bdf9f12eaa25ee6378b230d981804a4d391 Mon Sep 17 00:00:00 2001 From: Matthew Jones Date: Fri, 15 Apr 2016 15:14:10 -0400 Subject: [PATCH 17/20] Demo data flake8 cleanup --- .../management/commands/create_preload_data.py | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/awx/main/management/commands/create_preload_data.py b/awx/main/management/commands/create_preload_data.py index 37b65c3bf4..11e4b35fe0 100644 --- a/awx/main/management/commands/create_preload_data.py +++ b/awx/main/management/commands/create_preload_data.py @@ -36,12 +36,12 @@ class Command(BaseCommand): i = Inventory.objects.create(name='Demo Inventory', organization=o, created_by=superuser) - h = Host.objects.create(name='localhost', - inventory=i, - variables="ansible_connection: local", - created_by=superuser) - jt = JobTemplate.objects.create(name='Demo Job Template', - project=p, - inventory=i, - credential=c) + Host.objects.create(name='localhost', + inventory=i, + variables="ansible_connection: local", + created_by=superuser) + JobTemplate.objects.create(name='Demo Job Template', + project=p, + inventory=i, + credential=c) print('Default organization added.') From ba7f48abf12afd026782df2667bc77f321d2ebe5 Mon Sep 17 00:00:00 2001 From: Matthew Jones Date: Mon, 18 Apr 2016 11:27:39 -0400 Subject: [PATCH 18/20] Updating tests around data preload --- .../tests/old/commands/commands_monolithic.py | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/awx/main/tests/old/commands/commands_monolithic.py b/awx/main/tests/old/commands/commands_monolithic.py index 869b530ac9..ba65385139 100644 --- a/awx/main/tests/old/commands/commands_monolithic.py +++ b/awx/main/tests/old/commands/commands_monolithic.py @@ -191,32 +191,23 @@ class CreateDefaultOrgTest(BaseCommandMixin, BaseTest): def setUp(self): super(CreateDefaultOrgTest, self).setUp() + self.setup_instances() def test_create_default_org(self): self.setup_users() self.assertEqual(Organization.objects.count(), 0) - result, stdout, stderr = self.run_command('create_default_org') + result, stdout, stderr = self.run_command('create_preload_data') self.assertEqual(result, None) self.assertTrue('Default organization added' in stdout) self.assertEqual(Organization.objects.count(), 1) org = Organization.objects.all()[0] self.assertEqual(org.created_by, self.super_django_user) self.assertEqual(org.modified_by, self.super_django_user) - result, stdout, stderr = self.run_command('create_default_org') + result, stdout, stderr = self.run_command('create_preload_data') self.assertEqual(result, None) self.assertFalse('Default organization added' in stdout) self.assertEqual(Organization.objects.count(), 1) - def test_create_default_org_when_no_superuser_exists(self): - self.assertEqual(Organization.objects.count(), 0) - result, stdout, stderr = self.run_command('create_default_org') - self.assertEqual(result, None) - self.assertTrue('Default organization added' in stdout) - self.assertEqual(Organization.objects.count(), 1) - org = Organization.objects.all()[0] - self.assertEqual(org.created_by, None) - self.assertEqual(org.modified_by, None) - class DumpDataTest(BaseCommandMixin, BaseTest): ''' Test cases for dumpdata management command. From 5abde762ae0f6ba9a6c17dd0e375bdbbf9bb5a51 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 18 Apr 2016 14:29:30 -0400 Subject: [PATCH 19/20] updates to prompt-for tests and logic for new RBAC updates --- awx/api/views.py | 4 ++-- .../functional/api/test_job_runtime_params.py | 24 +++++++++---------- 2 files changed, 14 insertions(+), 14 deletions(-) diff --git a/awx/api/views.py b/awx/api/views.py index 80edcad4a5..05d908f8e4 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2120,12 +2120,12 @@ class JobTemplateLaunch(RetrieveAPIView, GenericAPIView): if 'credential' in prompted_fields and prompted_fields['credential'] != getattrd(obj, 'credential.pk', None): new_credential = Credential.objects.get(pk=prompted_fields['credential']) - if not request.user.can_access(Credential, 'use', new_credential): + if request.user not in new_credential.use_role: raise PermissionDenied() if 'inventory' in prompted_fields and prompted_fields['inventory'] != getattrd(obj, 'inventory.pk', None): new_inventory = Inventory.objects.get(pk=prompted_fields['inventory']) - if not request.user.can_access(Inventory, 'use', new_inventory): + if request.user not in new_inventory.use_role: raise PermissionDenied() kv = prompted_fields 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 cc417111db..28bc250d8f 100644 --- a/awx/main/tests/functional/api/test_job_runtime_params.py +++ b/awx/main/tests/functional/api/test_job_runtime_params.py @@ -3,6 +3,7 @@ import yaml from awx.api.serializers import JobLaunchSerializer from awx.main.models.credential import Credential +from awx.main.models.inventory import Inventory from awx.main.models.jobs import Job, JobTemplate from django.core.urlresolvers import reverse @@ -93,7 +94,7 @@ def test_job_accept_prompted_vars(runtime_data, job_template_prompts, post, user job_template = job_template_prompts(True) admin_user = user('admin', True) - job_template.inventory.executor_role.members.add(admin_user) + job_template.inventory.execute_role.members.add(admin_user) job_template.inventory.save() response = post(reverse('api:job_template_launch', args=[job_template.pk]), @@ -112,20 +113,19 @@ def test_job_accept_prompted_vars(runtime_data, job_template_prompts, post, user assert job_obj.job_tags == runtime_data['job_tags'] @pytest.mark.django_db -@pytest.mark.skip(reason="JT can_start without inventory needs to be fixed before passing") @pytest.mark.job_runtime_vars def test_job_accept_prompted_vars_null(runtime_data, job_template_prompts_null, post, user): job_template = job_template_prompts_null - common_user = user('admin', False) + common_user = user('not-admin', False) - job_template.executor_role.members.add(common_user) - job_template.save() - job_template.project.member_role.members.add(common_user) - job_template.project.save() + # Give user permission to execute the job template + job_template.execute_role.members.add(common_user) + # Give user permission to use inventory and credential at runtime credential = Credential.objects.get(pk=runtime_data['credential']) - credential.usage_role.members.add(common_user) - credential.save() + credential.use_role.members.add(common_user) + inventory = Inventory.objects.get(pk=runtime_data['inventory']) + inventory.use_role.members.add(common_user) response = post(reverse('api:job_template_launch', args=[job_template.pk]), runtime_data, common_user) @@ -187,13 +187,13 @@ def test_job_launch_fails_without_inventory_access(deploy_jobtemplate, machine_c deploy_jobtemplate.ask_inventory_on_launch = True deploy_jobtemplate.credential = machine_credential common_user = user('test-user', False) - deploy_jobtemplate.executor_role.members.add(common_user) + deploy_jobtemplate.execute_role.members.add(common_user) deploy_jobtemplate.save() - deploy_jobtemplate.inventory.usage_role.members.add(common_user) + deploy_jobtemplate.inventory.use_role.members.add(common_user) deploy_jobtemplate.inventory.save() deploy_jobtemplate.project.member_role.members.add(common_user) deploy_jobtemplate.project.save() - deploy_jobtemplate.credential.usage_role.members.add(common_user) + deploy_jobtemplate.credential.use_role.members.add(common_user) deploy_jobtemplate.credential.save() # Assure that the base job template can be launched to begin with From 1a97773aa1fcfd0b27b3d58860a4a77c83ad4008 Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 18 Apr 2016 14:36:18 -0400 Subject: [PATCH 20/20] remove more save calls --- awx/main/tests/functional/api/test_job_runtime_params.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) 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 28bc250d8f..470e43423f 100644 --- a/awx/main/tests/functional/api/test_job_runtime_params.py +++ b/awx/main/tests/functional/api/test_job_runtime_params.py @@ -95,7 +95,6 @@ def test_job_accept_prompted_vars(runtime_data, job_template_prompts, post, user admin_user = user('admin', True) job_template.inventory.execute_role.members.add(admin_user) - job_template.inventory.save() response = post(reverse('api:job_template_launch', args=[job_template.pk]), runtime_data, admin_user) @@ -186,15 +185,12 @@ def test_job_launch_fails_without_inventory(deploy_jobtemplate, post, user): def test_job_launch_fails_without_inventory_access(deploy_jobtemplate, machine_credential, post, user): deploy_jobtemplate.ask_inventory_on_launch = True deploy_jobtemplate.credential = machine_credential + deploy_jobtemplate.save() common_user = user('test-user', False) deploy_jobtemplate.execute_role.members.add(common_user) - deploy_jobtemplate.save() deploy_jobtemplate.inventory.use_role.members.add(common_user) - deploy_jobtemplate.inventory.save() deploy_jobtemplate.project.member_role.members.add(common_user) - deploy_jobtemplate.project.save() deploy_jobtemplate.credential.use_role.members.add(common_user) - deploy_jobtemplate.credential.save() # Assure that the base job template can be launched to begin with response = post(reverse('api:job_template_launch',