Retrieve live params first, include template-driven defaults

CHANGELOG.rst

@@ -50,6 +50,9 @@ Added
 * Added flag `--auto-dict` to `st2 run` and `st2 execution re-run` commands. This flag must now
   be specified in order to automatically convert list items to dicts based on presence of colon
   (`:`) in all of the list items (new feature) #3909 
+* Jinja templates in default parameter values now render as live parameters, if no "real" live
+  parameter was provided. This allows the template to render pre-schema validation, resulting
+  in the intended value type. (improvement) #3892
 
 Changed
 ~~~~~~~

st2api/st2api/controllers/v1/actionexecutions.py

@@ -174,8 +174,6 @@ def _schedule_execution(self,
 
         # Schedule the action execution.
         liveaction_db = LiveActionAPI.to_model(liveaction)
-        liveaction_db, actionexecution_db = action_service.create_request(liveaction_db)
-
         action_db = action_utils.get_action_by_ref(liveaction_db.action)
         runnertype_db = action_utils.get_runnertype_by_name(action_db.runner_type['name'])
 
@@ -184,16 +182,23 @@ def _schedule_execution(self,
                 runnertype_db.runner_parameters, action_db.parameters, liveaction_db.parameters,
                 liveaction_db.context)
         except param_exc.ParamException:
+
+            # We still need to create a request, so liveaction_db is assigned an ID
+            liveaction_db, actionexecution_db = action_service.create_request(liveaction_db)
+
             # By this point the execution is already in the DB therefore need to mark it failed.
             _, e, tb = sys.exc_info()
             action_service.update_status(
                 liveaction=liveaction_db,
                 new_status=action_constants.LIVEACTION_STATUS_FAILED,
                 result={'error': str(e), 'traceback': ''.join(traceback.format_tb(tb, 20))})
             # Might be a good idea to return the actual ActionExecution rather than bubble up
-            # the execption.
+            # the exception.
             raise validation_exc.ValueValidationException(str(e))
 
+        # The request should be created after the above call to render_live_params
+        # so any templates in live parameters have a chance to render.
+        liveaction_db, actionexecution_db = action_service.create_request(liveaction_db)
         liveaction_db = LiveAction.add_or_update(liveaction_db, publish=False)
 
         _, actionexecution_db = action_service.publish_request(liveaction_db, actionexecution_db)

st2api/tests/unit/controllers/v1/test_executions.py

@@ -142,6 +142,20 @@
     'runner_type': 'inquirer',
 }
 
+ACTION_DEFAULT_TEMPLATE = {
+    'name': 'st2.dummy.default_template',
+    'description': 'An action that uses a jinja template as a default value for a parameter',
+    'enabled': True,
+    'pack': 'starterpack',
+    'runner_type': 'local-shell-cmd',
+    'parameters': {
+        'intparam': {
+            'type': 'integer',
+            'default': '{{ st2kv.system.test_int | int }}'
+        }
+    }
+}
+
 LIVE_ACTION_1 = {
     'action': 'sixpack.st2.dummy.action1',
     'parameters': {
@@ -211,6 +225,12 @@
     }
 }
 
+# Do not add parameters to this. There are tests that will test first without params,
+# then make a copy with params.
+LIVE_ACTION_DEFAULT_TEMPLATE = {
+    'action': 'starterpack.st2.dummy.default_template',
+}
+
 FIXTURES_PACK = 'generic'
 TEST_FIXTURES = {
     'runners': ['testrunner1.yaml'],
@@ -247,13 +267,18 @@ def setUpClass(cls):
         post_resp = cls.app.post_json('/v1/actions', cls.action_inquiry)
         cls.action_inquiry['id'] = post_resp.json['id']
 
+        cls.action_template = copy.deepcopy(ACTION_DEFAULT_TEMPLATE)
+        post_resp = cls.app.post_json('/v1/actions', cls.action_template)
+        cls.action_template['id'] = post_resp.json['id']
+
     @classmethod
     def tearDownClass(cls):
         cls.app.delete('/v1/actions/%s' % cls.action1['id'])
         cls.app.delete('/v1/actions/%s' % cls.action2['id'])
         cls.app.delete('/v1/actions/%s' % cls.action3['id'])
         cls.app.delete('/v1/actions/%s' % cls.action4['id'])
         cls.app.delete('/v1/actions/%s' % cls.action_inquiry['id'])
+        cls.app.delete('/v1/actions/%s' % cls.action_template['id'])
         super(BaseActionExecutionControllerTestCase, cls).tearDownClass()
 
     def test_get_one(self):
@@ -439,7 +464,8 @@ def test_post_parameter_validation_failed(self):
         execution['parameters'] = {"hosts": "127.0.0.1", "cmd": 1000}
         post_resp = self._do_post(execution, expect_errors=True)
         self.assertEqual(post_resp.status_int, 400)
-        self.assertEqual(post_resp.json['faultstring'], "1000 is not of type 'string', 'null'")
+        self.assertIn('Value "1000" must either be a string or None. Got "int"',
+                      post_resp.json['faultstring'])
 
         # Runner type expects parameters "cmd" to be str.
         execution['parameters'] = {"hosts": "127.0.0.1", "cmd": "1000", "c": 1}
@@ -541,7 +567,31 @@ def test_re_run_failure_parameter_override_invalid_type(self):
         re_run_resp = self.app.post_json('/v1/executions/%s/re_run' % (execution_id),
                                          data, expect_errors=True)
         self.assertEqual(re_run_resp.status_int, 400)
-        self.assertIn('1000 is not of type \'string\'', re_run_resp.json['faultstring'])
+        self.assertIn('Value "1000" must either be a string or None. Got "int"',
+                      re_run_resp.json['faultstring'])
+
+    def test_template_param(self):
+
+        # Test with default value containing template
+        post_resp = self._do_post(LIVE_ACTION_DEFAULT_TEMPLATE)
+        self.assertEqual(post_resp.status_int, 201)
+
+        # Assert that the template in the parameter default value
+        # was rendered and st2kv was used
+        self.assertEqual(post_resp.json['parameters']['intparam'], 0)
+
+        # Test with live param
+        live_int_param = 3
+        livaction_with_params = copy.deepcopy(LIVE_ACTION_DEFAULT_TEMPLATE)
+        livaction_with_params['parameters'] = {
+            "intparam": live_int_param
+        }
+        post_resp = self._do_post(livaction_with_params)
+        self.assertEqual(post_resp.status_int, 201)
+
+        # Assert that the template in the parameter default value
+        # was not rendered, and the provided parameter was used
+        self.assertEqual(post_resp.json['parameters']['intparam'], live_int_param)
 
     def test_re_run_workflow_success(self):
         # Create a new execution

st2common/st2common/util/param.py

@@ -21,6 +21,7 @@
 from jinja2 import meta
 from st2common import log as logging
 from st2common.util.config_loader import get_config
+from st2common.util.jinja import is_jinja_expression
 from st2common.constants.action import ACTION_CONTEXT_KV_PREFIX
 from st2common.constants.pack import PACK_CONFIG_CONTEXT_KV_PREFIX
 from st2common.constants.keyvalue import DATASTORE_PARENT_SCOPE, SYSTEM_SCOPE, FULL_SYSTEM_SCOPE
@@ -220,12 +221,43 @@ def _cast_params_from(params, context, schemas):
     Pick a list of parameters from context and cast each of them according to the schemas provided
     '''
     result = {}
+
+    # First, cast only explicitly provided live parameters
     for name in params:
         param_schema = {}
         for schema in schemas:
             if name in schema:
                 param_schema = schema[name]
         result[name] = _cast(context[name], param_schema)
+
+    # Now, iterate over all parameters, and add any to the live set that satisfy ALL of the
+    # following criteria:
+    #
+    # - Have a default value that is a Jinja template
+    # - Are using the default value (i.e. not being overwritten by an actual live param)
+    #
+    # We do this because the execution API controller first determines live params before
+    # validating params against the schema. So, we want to make sure that if the default
+    # value is a template, it is rendered and added to the live params before this validation.
+    for schema in schemas:
+        for param_name, param_details in schema.items():
+
+            # Skip if the parameter doesn't have a default, or if the
+            # value in the context is identical to the default
+            if 'default' not in param_details or \
+                    param_details.get('default') == context[param_name]:
+                continue
+
+            # Skip if the default value isn't a Jinja expression
+            if not is_jinja_expression(param_details.get('default')):
+                continue
+
+            # Skip if the parameter is being overridden
+            if param_name in params:
+                continue
+
+            result[param_name] = _cast(context[param_name], param_details)
+
     return result
 
 

st2common/tests/unit/test_param_utils.py

@@ -647,6 +647,7 @@ def test_get_live_params_with_additional_context(self):
 
         expected_params = {
             'r1': 'generic',
+            'r2': 'generic',
             'r3': 'lolcathost'
         }
         self.assertEqual(live_params, expected_params)
@@ -695,3 +696,82 @@ def test_unsatisfied_dependency_friendly_error_message(self):
         expected_msg = 'Dependency unsatisfied in variable "variable_not_defined"'
         self.assertRaisesRegexp(ParamException, expected_msg, param_utils.render_live_params,
                                 runner_param_info, action_param_info, params, action_context)
+
+    def test_add_default_templates_to_live_params(self):
+        """Test addition of template values in defaults to live params
+        """
+
+        # Test with no live params, and two parameters - one should make it through because
+        # it was a template, and the other shouldn't because its default wasn't a template
+        schemas = [
+            {
+                'templateparam': {
+                    'default': '{{ 3 | int }}',
+                    'type': 'integer'
+                }
+            }
+        ]
+        context = {
+            'templateparam': '3'
+        }
+        result = param_utils._cast_params_from({}, context, schemas)
+        self.assertEquals(result, {'templateparam': 3})
+
+        # Ensure parameter is skipped if the value in context is identical to default
+        schemas = [
+            {
+                'nottemplateparam': {
+                    'default': '4',
+                    'type': 'integer'
+                }
+            }
+        ]
+        context = {
+            'nottemplateparam': '4',
+        }
+        result = param_utils._cast_params_from({}, context, schemas)
+        self.assertEquals(result, {})
+
+        # Ensure parameter is skipped if the parameter doesn't have a default
+        schemas = [
+            {
+                'nottemplateparam': {
+                    'type': 'integer'
+                }
+            }
+        ]
+        context = {
+            'nottemplateparam': '4',
+        }
+        result = param_utils._cast_params_from({}, context, schemas)
+        self.assertEquals(result, {})
+
+        # Skip if the default value isn't a Jinja expression
+        schemas = [
+            {
+                'nottemplateparam': {
+                    'default': '5',
+                    'type': 'integer'
+                }
+            }
+        ]
+        context = {
+            'nottemplateparam': '4',
+        }
+        result = param_utils._cast_params_from({}, context, schemas)
+        self.assertEquals(result, {})
+
+        # Ensure parameter is skipped if the parameter is being overridden
+        schemas = [
+            {
+                'templateparam': {
+                    'default': '{{ 3 | int }}',
+                    'type': 'integer'
+                }
+            }
+        ]
+        context = {
+            'templateparam': '4',
+        }
+        result = param_utils._cast_params_from({'templateparam': '4'}, context, schemas)
+        self.assertEquals(result, {'templateparam': 4})