From b0bbeb2ca8e9c6bcc9faf0ccc7f7fd62bf00e60b Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Mon, 11 Apr 2016 15:39:06 -0400 Subject: [PATCH] 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')