Enforce extra_vars override hierachy

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', 
+__all__ = ['ResourceMixin', 'SurveyJobTemplateMixin', 'SurveyJobMixin',
-           'TaskManagerUnifiedJobMixin', 'TaskManagerJobMixin', 'TaskManagerProjectUpdateMixin', 
+           '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']
+            runtime_extra_vars = kwargs['extra_vars']
-            kwargs_extra_vars = parse_yaml_or_json(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:
+                    if variable_key in runtime_extra_vars and default:
-                        kw_value = kwargs_extra_vars[variable_key]
+                        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)
+        extra_vars.update(runtime_extra_vars)
-        kwargs['extra_vars'] = json.dumps(extra_vars)
+        create_kwargs['extra_vars'] = json.dumps(extra_vars)
-        return kwargs
+        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
-
-

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
+            create_kwargs['scm_delete_on_update'] = True
-        return kwargs
+        return create_kwargs
 
     def create_project_update(self, **kwargs):
         return self.create_unified_job(**kwargs)

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',

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()

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