From 1c8217936de84ef6dbc121573a58c6b2f8f2de7b Mon Sep 17 00:00:00 2001 From: AlanCoding Date: Thu, 7 Dec 2017 13:37:28 -0500 Subject: [PATCH] Bug fixes from integration ran on launchtime branch Make error message for muti-vault validation more consistent with historical message --- awx/api/serializers.py | 12 ++++++---- awx/api/views.py | 4 ++-- awx/main/models/credential.py | 2 +- awx/main/models/jobs.py | 6 ++--- awx/main/models/mixins.py | 2 +- awx/main/models/schedules.py | 5 ++-- awx/main/models/unified_jobs.py | 23 +++++++++++-------- awx/main/models/workflow.py | 4 ++-- awx/main/tasks.py | 10 ++------ .../test_deprecated_credential_assignment.py | 4 ++-- .../tests/unit/models/test_workflow_unit.py | 2 +- 11 files changed, 38 insertions(+), 36 deletions(-) diff --git a/awx/api/serializers.py b/awx/api/serializers.py index 6cb5501943..0b24514165 100644 --- a/awx/api/serializers.py +++ b/awx/api/serializers.py @@ -3106,15 +3106,15 @@ class WorkflowJobTemplateNodeSerializer(LaunchConfigurationBaseSerializer): }) if 'unified_job_template' in attrs: ujt_obj = attrs['unified_job_template'] - elif self.instance: + ujt_obj = None + if self.instance: ujt_obj = self.instance.unified_job_template - else: - raise serializers.ValidationError({ - "unified_job_template": _("Node needs to have a template attached.")}) if isinstance(ujt_obj, (WorkflowJobTemplate, SystemJobTemplate)): raise serializers.ValidationError({ "unified_job_template": _("Cannot nest a %s inside a WorkflowJobTemplate") % ujt_obj.__class__.__name__}) attrs = super(WorkflowJobTemplateNodeSerializer, self).validate(attrs) + if ujt_obj is None: + ujt_obj = attrs.get('unified_job_template') accepted, rejected, errors = ujt_obj._accept_or_ignore_job_kwargs(**self._build_mock_obj(attrs).prompts_dict()) # Do not raise survey validation errors errors.pop('variables_needed_to_start', None) @@ -3703,8 +3703,10 @@ class ScheduleSerializer(LaunchConfigurationBaseSerializer): elif self.instance: ujt = self.instance.unified_job_template accepted, rejected, errors = ujt.accept_or_ignore_variables(extra_data) + if 'extra_vars' in errors: + errors['extra_data'] = errors.pop('extra_vars') if errors: - raise serializers.ValidationError({'extra_data': errors['extra_vars']}) + raise serializers.ValidationError(errors) return super(ScheduleSerializer, self).validate(attrs) # We reject rrules if: diff --git a/awx/api/views.py b/awx/api/views.py index 28178322d1..41f9d6a3aa 100644 --- a/awx/api/views.py +++ b/awx/api/views.py @@ -2820,7 +2820,7 @@ class JobTemplateLaunch(RetrieveAPIView): prompted_value = modern_data.pop(key) # add the deprecated credential specified in the request - if not isinstance(prompted_value, Iterable): + if not isinstance(prompted_value, Iterable) or isinstance(prompted_value, basestring): prompted_value = [prompted_value] # If user gave extra_credentials, special case to use exactly @@ -3063,7 +3063,7 @@ class JobTemplateCredentialsList(SubListCreateAttachDetachAPIView): def is_valid_relation(self, parent, sub, created=False): if sub.unique_hash() in [cred.unique_hash() for cred in parent.credentials.all()]: - return {"msg": _("Cannot assign multiple {credential_type} credentials.".format( + return {"error": _("Cannot assign multiple {credential_type} credentials.".format( credential_type=sub.unique_hash(display=True)))} return super(JobTemplateCredentialsList, self).is_valid_relation(parent, sub, created) diff --git a/awx/main/models/credential.py b/awx/main/models/credential.py index 786405e81e..9d7363c095 100644 --- a/awx/main/models/credential.py +++ b/awx/main/models/credential.py @@ -381,7 +381,7 @@ class Credential(PasswordFieldsModel, CommonModelNameNotUnique, ResourceMixin): that can be used to evaluate exclusivity ''' if display: - type_alias = self.kind + type_alias = self.credential_type.name else: type_alias = self.credential_type_id if self.kind == 'vault' and self.inputs.get('vault_id', None): diff --git a/awx/main/models/jobs.py b/awx/main/models/jobs.py index 8685278da4..9b3d198533 100644 --- a/awx/main/models/jobs.py +++ b/awx/main/models/jobs.py @@ -876,7 +876,7 @@ class LaunchTimeConfig(BaseModel): data[prompt_name] = self.display_extra_data() else: data[prompt_name] = self.extra_data - if self.survey_passwords: + if self.survey_passwords and not display: data['survey_passwords'] = self.survey_passwords else: prompt_val = getattr(self, prompt_name) @@ -889,11 +889,11 @@ class LaunchTimeConfig(BaseModel): Hides fields marked as passwords in survey. ''' if self.survey_passwords: - extra_data = json.loads(self.extra_data) + extra_data = parse_yaml_or_json(self.extra_data) for key, value in self.survey_passwords.items(): if key in extra_data: extra_data[key] = value - return json.dumps(extra_data) + return extra_data else: return self.extra_data diff --git a/awx/main/models/mixins.py b/awx/main/models/mixins.py index 08f5c4ac5e..95077ee699 100644 --- a/awx/main/models/mixins.py +++ b/awx/main/models/mixins.py @@ -252,7 +252,7 @@ class SurveyJobTemplateMixin(models.Model): survey_errors += element_errors if key is not None and key in extra_vars: rejected[key] = extra_vars.pop(key) - else: + elif key in extra_vars: accepted[key] = extra_vars.pop(key) if survey_errors: errors['variables_needed_to_start'] = survey_errors diff --git a/awx/main/models/schedules.py b/awx/main/models/schedules.py index ecdd1cb6fc..20e6923a1c 100644 --- a/awx/main/models/schedules.py +++ b/awx/main/models/schedules.py @@ -99,10 +99,11 @@ class Schedule(CommonModel, LaunchTimeConfig): def get_job_kwargs(self): config_data = self.prompts_dict() - prompts, rejected, errors = self.unified_job_template._accept_or_ignore_job_kwargs(**config_data) + job_kwargs, rejected, errors = self.unified_job_template._accept_or_ignore_job_kwargs(**config_data) if errors: logger.info('Errors creating scheduled job: {}'.format(errors)) - return prompts + job_kwargs['_eager_fields'] = {'launch_type': 'scheduled', 'schedule': self} + return job_kwargs def update_computed_fields(self): future_rs = dateutil.rrule.rrulestr(self.rrule, forceset=True) diff --git a/awx/main/models/unified_jobs.py b/awx/main/models/unified_jobs.py index 4261a87f8a..ecd39c217a 100644 --- a/awx/main/models/unified_jobs.py +++ b/awx/main/models/unified_jobs.py @@ -343,7 +343,7 @@ class UnifiedJobTemplate(PolymorphicModel, CommonModelNameNotUnique, Notificatio ''' Create a new unified job based on this unified job template. ''' - original_passwords = kwargs.pop('survey_passwords', {}) + new_job_passwords = kwargs.pop('survey_passwords', {}) eager_fields = kwargs.pop('_eager_fields', None) unified_job_class = self._get_unified_job_class() fields = self._get_unified_job_field_names() @@ -363,12 +363,11 @@ class UnifiedJobTemplate(PolymorphicModel, CommonModelNameNotUnique, Notificatio # For JobTemplate-based jobs with surveys, add passwords to list for perma-redaction if hasattr(self, 'survey_spec') and getattr(self, 'survey_enabled', False): - password_list = self.survey_password_variables() - hide_password_dict = getattr(unified_job, 'survey_passwords', {}) - hide_password_dict.update(original_passwords) - for password in password_list: - hide_password_dict[password] = REPLACE_STR - unified_job.survey_passwords = hide_password_dict + for password in self.survey_password_variables(): + new_job_passwords[password] = REPLACE_STR + if new_job_passwords: + unified_job.survey_passwords = new_job_passwords + kwargs['survey_passwords'] = new_job_passwords # saved in config object for relaunch unified_job.save() @@ -430,7 +429,10 @@ class UnifiedJobTemplate(PolymorphicModel, CommonModelNameNotUnique, Notificatio ''' Override in subclass if template accepts _any_ prompted params ''' - return ({}, kwargs, {"all": ["Fields {} are not allowed on launch.".format(kwargs.keys())]}) + errors = {} + if kwargs: + errors['all'] = [_("Fields {} are not allowed on launch.").format(kwargs.keys())] + return ({}, kwargs, errors) def accept_or_ignore_variables(self, data, errors=None): ''' @@ -859,8 +861,11 @@ class UnifiedJob(PolymorphicModel, PasswordFieldsModel, CommonModelNameNotUnique return None JobLaunchConfig = self._meta.get_field('launch_config').related_model config = JobLaunchConfig(job=self) + valid_fields = self.unified_job_template.get_ask_mapping().keys() + if hasattr(self, 'extra_vars'): + valid_fields.extend(['survey_passwords', 'extra_vars']) for field_name, value in kwargs.items(): - if (field_name not in self.unified_job_template.get_ask_mapping() and field_name != 'survey_passwords'): + if field_name not in valid_fields: raise Exception('Unrecognized launch config field {}.'.format(field_name)) if field_name == 'credentials': continue diff --git a/awx/main/models/workflow.py b/awx/main/models/workflow.py index 539b4fd9d5..1a7f1b93eb 100644 --- a/awx/main/models/workflow.py +++ b/awx/main/models/workflow.py @@ -230,7 +230,7 @@ class WorkflowJobNode(WorkflowNodeBase): if extra_vars: data['extra_vars'] = extra_vars # ensure that unified jobs created by WorkflowJobs are marked - data['launch_type'] = 'workflow' + data['_eager_fields'] = {'launch_type': 'workflow'} return data @@ -366,7 +366,7 @@ class WorkflowJobTemplate(UnifiedJobTemplate, WorkflowJobOptions, SurveyJobTempl # WFJTs do not behave like JTs, it can not accept inventory, credential, etc. bad_kwargs = kwargs.copy() - bad_kwargs.pop('extra_vars') + bad_kwargs.pop('extra_vars', None) if bad_kwargs: rejected_fields.update(bad_kwargs) for field in bad_kwargs: diff --git a/awx/main/tasks.py b/awx/main/tasks.py index 8a47f2cd92..7fbbe6bc46 100644 --- a/awx/main/tasks.py +++ b/awx/main/tasks.py @@ -307,14 +307,8 @@ def awx_periodic_scheduler(self): logger.warn("Cache timeout is in the future, bypassing schedule for template %s" % str(template.id)) continue try: - prompts = schedule.get_job_kwargs() - new_unified_job = schedule.unified_job_template.create_unified_job( - _eager_fields=dict( - launch_type='scheduled', - schedule=schedule - ), - **prompts - ) + job_kwargs = schedule.get_job_kwargs() + new_unified_job = schedule.unified_job_template.create_unified_job(**job_kwargs) can_start = new_unified_job.signal_start() except Exception: logger.exception('Error spawning scheduled job.') diff --git a/awx/main/tests/functional/api/test_deprecated_credential_assignment.py b/awx/main/tests/functional/api/test_deprecated_credential_assignment.py index b84865d052..e9090630e2 100644 --- a/awx/main/tests/functional/api/test_deprecated_credential_assignment.py +++ b/awx/main/tests/functional/api/test_deprecated_credential_assignment.py @@ -131,7 +131,7 @@ def test_prevent_multiple_machine_creds(get, post, job_template, admin, machine_ assert get(url, admin).data['count'] == 1 resp = post(url, _new_cred('Second Cred'), admin, expect=400) - assert 'Cannot assign multiple ssh credentials.' in resp.content + assert 'Cannot assign multiple Machine credentials.' in resp.content @pytest.mark.django_db @@ -167,7 +167,7 @@ def test_extra_credentials_unique_by_kind(get, post, job_template, admin, assert get(url, admin).data['count'] == 1 resp = post(url, _new_cred('Second Cred'), admin, expect=400) - assert 'Cannot assign multiple aws credentials.' in resp.content + assert 'Cannot assign multiple Amazon Web Services credentials.' in resp.content @pytest.mark.django_db diff --git a/awx/main/tests/unit/models/test_workflow_unit.py b/awx/main/tests/unit/models/test_workflow_unit.py index bfd69a90f2..af7be73f54 100644 --- a/awx/main/tests/unit/models/test_workflow_unit.py +++ b/awx/main/tests/unit/models/test_workflow_unit.py @@ -198,7 +198,7 @@ class TestWorkflowJobNodeJobKWARGS: Tests for building the keyword arguments that go into creating and launching a new job that corresponds to a workflow node. """ - kwargs_base = {'launch_type': 'workflow'} + kwargs_base = {'_eager_fields': {'launch_type': 'workflow'}} def test_null_kwargs(self, job_node_no_prompts): assert job_node_no_prompts.get_job_kwargs() == self.kwargs_base