From 047ad7ca4a465ec6552b45c79056283fe4c8fd73 Mon Sep 17 00:00:00 2001 From: Aaron Tan Date: Wed, 26 Jul 2017 18:10:33 -0400 Subject: [PATCH] Enforce extra_vars override hierachy --- awx/main/models/mixins.py | 38 +++++++++++-------- awx/main/models/projects.py | 14 +++++-- .../unit/models/test_job_template_unit.py | 19 +++++++++- .../tests/unit/models/test_survey_models.py | 6 +-- awx/main/utils/common.py | 2 +- 5 files changed, 55 insertions(+), 24 deletions(-) diff --git a/awx/main/models/mixins.py b/awx/main/models/mixins.py index 5ec4a4337d..cb0a2236d4 100644 --- a/awx/main/models/mixins.py +++ b/awx/main/models/mixins.py @@ -16,8 +16,8 @@ from awx.main.utils import parse_yaml_or_json from awx.main.fields import JSONField -__all__ = ['ResourceMixin', 'SurveyJobTemplateMixin', 'SurveyJobMixin', - 'TaskManagerUnifiedJobMixin', 'TaskManagerJobMixin', 'TaskManagerProjectUpdateMixin', +__all__ = ['ResourceMixin', 'SurveyJobTemplateMixin', 'SurveyJobMixin', + 'TaskManagerUnifiedJobMixin', 'TaskManagerJobMixin', 'TaskManagerProjectUpdateMixin', 'TaskManagerInventoryUpdateMixin',] @@ -111,20 +111,29 @@ class SurveyJobTemplateMixin(models.Model): vars.append(survey_element['variable']) return vars - def _update_unified_job_kwargs(self, **kwargs): + def _update_unified_job_kwargs(self, create_kwargs, kwargs): ''' Combine extra_vars with variable precedence order: JT extra_vars -> JT survey defaults -> runtime extra_vars + + :param create_kwargs: key-worded arguments to be updated and later used for creating unified job. + :type create_kwargs: dict + :param kwargs: request parameters used to override unified job template fields with runtime values. + :type kwargs: dict + :return: modified create_kwargs. + :rtype: dict ''' # Job Template extra_vars extra_vars = self.extra_vars_dict + survey_defaults = {} + # transform to dict if 'extra_vars' in kwargs: - kwargs_extra_vars = kwargs['extra_vars'] - kwargs_extra_vars = parse_yaml_or_json(kwargs_extra_vars) + runtime_extra_vars = kwargs['extra_vars'] + runtime_extra_vars = parse_yaml_or_json(runtime_extra_vars) else: - kwargs_extra_vars = {} + runtime_extra_vars = {} # Overwrite with job template extra vars with survey default vars if self.survey_enabled and 'spec' in self.survey_spec: @@ -133,22 +142,23 @@ class SurveyJobTemplateMixin(models.Model): variable_key = survey_element.get('variable') if survey_element.get('type') == 'password': - if variable_key in kwargs_extra_vars and default: - kw_value = kwargs_extra_vars[variable_key] + if variable_key in runtime_extra_vars and default: + kw_value = runtime_extra_vars[variable_key] if kw_value.startswith('$encrypted$') and kw_value != default: - kwargs_extra_vars[variable_key] = default + runtime_extra_vars[variable_key] = default if default is not None: data = {variable_key: default} errors = self._survey_element_validation(survey_element, data) if not errors: - extra_vars[variable_key] = default + survey_defaults[variable_key] = default + extra_vars.update(survey_defaults) # Overwrite job template extra vars with explicit job extra vars # and add on job extra vars - extra_vars.update(kwargs_extra_vars) - kwargs['extra_vars'] = json.dumps(extra_vars) - return kwargs + extra_vars.update(runtime_extra_vars) + create_kwargs['extra_vars'] = json.dumps(extra_vars) + return create_kwargs def _survey_element_validation(self, survey_element, data): errors = [] @@ -291,5 +301,3 @@ class TaskManagerProjectUpdateMixin(TaskManagerUpdateOnLaunchMixin): class TaskManagerInventoryUpdateMixin(TaskManagerUpdateOnLaunchMixin): class Meta: abstract = True - - diff --git a/awx/main/models/projects.py b/awx/main/models/projects.py index 827b131aed..9fe704018b 100644 --- a/awx/main/models/projects.py +++ b/awx/main/models/projects.py @@ -377,10 +377,18 @@ class Project(UnifiedJobTemplate, ProjectOptions, ResourceMixin): def _can_update(self): return bool(self.scm_type) - def _update_unified_job_kwargs(self, **kwargs): + def _update_unified_job_kwargs(self, create_kwargs, kwargs): + ''' + :param create_kwargs: key-worded arguments to be updated and later used for creating unified job. + :type create_kwargs: dict + :param kwargs: request parameters used to override unified job template fields with runtime values. + :type kwargs: dict + :return: modified create_kwargs. + :rtype: dict + ''' if self.scm_delete_on_next_update: - kwargs['scm_delete_on_update'] = True - return kwargs + create_kwargs['scm_delete_on_update'] = True + return create_kwargs def create_project_update(self, **kwargs): return self.create_unified_job(**kwargs) diff --git a/awx/main/tests/unit/models/test_job_template_unit.py b/awx/main/tests/unit/models/test_job_template_unit.py index a6086b7b9d..2a4d48e71d 100644 --- a/awx/main/tests/unit/models/test_job_template_unit.py +++ b/awx/main/tests/unit/models/test_job_template_unit.py @@ -97,7 +97,7 @@ def test_job_template_survey_mixin(job_template_factory): obj = objects.job_template obj.survey_enabled = True obj.survey_spec = {'spec': [{'default':'my_default', 'type':'password', 'variable':'my_variable'}]} - kwargs = obj._update_unified_job_kwargs(extra_vars={'my_variable':'$encrypted$'}) + kwargs = obj._update_unified_job_kwargs({}, {'extra_vars': {'my_variable':'$encrypted$'}}) assert kwargs['extra_vars'] == '{"my_variable": "my_default"}' @@ -113,10 +113,25 @@ def test_job_template_survey_mixin_length(job_template_factory): obj.survey_enabled = True obj.survey_spec = {'spec': [{'default':'my_default', 'type':'password', 'variable':'my_variable'}, {'type':'password', 'variable':'my_other_variable'}]} - kwargs = obj._update_unified_job_kwargs(extra_vars={'my_variable':'$encrypted$'}) + kwargs = obj._update_unified_job_kwargs({}, {'extra_vars': {'my_variable':'$encrypted$'}}) assert kwargs['extra_vars'] == '{"my_variable": "my_default"}' +def test_job_template_survey_mixin_survey_runtime_has_highest_priority(job_template_factory): + objects = job_template_factory( + 'survey_mixin_test', + organization='org1', + inventory='inventory1', + credential='cred1', + persisted=False, + ) + obj = objects.job_template + obj.survey_enabled = True + obj.survey_spec = {'spec': [{'default':'foo', 'type':'password', 'variable':'my_variable'}]} + kwargs = obj._update_unified_job_kwargs({}, {'extra_vars': {'my_variable': 'bar'}}) + assert kwargs['extra_vars'] == '{"my_variable": "bar"}' + + def test_job_template_can_start_with_callback_extra_vars_provided(job_template_factory): objects = job_template_factory( 'callback_extra_vars_test', diff --git a/awx/main/tests/unit/models/test_survey_models.py b/awx/main/tests/unit/models/test_survey_models.py index af2d0e3735..d9642a9e17 100644 --- a/awx/main/tests/unit/models/test_survey_models.py +++ b/awx/main/tests/unit/models/test_survey_models.py @@ -86,7 +86,7 @@ def test_update_kwargs_survey_invalid_default(survey_spec_factory): spec['spec'][0]['min'] = 3 spec['spec'][0]['default'] = 1 jt = JobTemplate(name="test-jt", survey_spec=spec, survey_enabled=True, extra_vars="var2: 2") - defaulted_extra_vars = jt._update_unified_job_kwargs() + defaulted_extra_vars = jt._update_unified_job_kwargs({}, {}) assert 'extra_vars' in defaulted_extra_vars # Make sure we did not set the invalid default of 1 assert json.loads(defaulted_extra_vars['extra_vars'])['var2'] == 2 @@ -115,7 +115,7 @@ def test_optional_survey_question_defaults( }, ]) jt = JobTemplate(name="test-jt", survey_spec=spec, survey_enabled=True) - defaulted_extra_vars = jt._update_unified_job_kwargs() + defaulted_extra_vars = jt._update_unified_job_kwargs({}, {}) element = spec['spec'][0] if expect_use: assert jt._survey_element_validation(element, {element['variable']: element['default']}) == [] @@ -140,7 +140,7 @@ class TestWorkflowSurveys: survey_enabled=True, extra_vars="var1: 5" ) - updated_extra_vars = wfjt._update_unified_job_kwargs() + updated_extra_vars = wfjt._update_unified_job_kwargs({}, {}) assert 'extra_vars' in updated_extra_vars assert json.loads(updated_extra_vars['extra_vars'])['var1'] == 3 assert wfjt.can_start_without_user_input() diff --git a/awx/main/utils/common.py b/awx/main/utils/common.py index 80a72c9cb2..5687e68cee 100644 --- a/awx/main/utils/common.py +++ b/awx/main/utils/common.py @@ -424,7 +424,7 @@ def copy_model_by_class(obj1, Class2, fields, kwargs): # Apply class-specific extra processing for origination of unified jobs if hasattr(obj1, '_update_unified_job_kwargs') and obj1.__class__ != Class2: - new_kwargs = obj1._update_unified_job_kwargs(**create_kwargs) + new_kwargs = obj1._update_unified_job_kwargs(create_kwargs, kwargs) else: new_kwargs = create_kwargs