diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 105aed43d1..1e7c578797 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -45,6 +45,7 @@ Changed * Fix misc DeprecationWarnings to prepare for python 3.10 support. #6188 (by @nzlosh) * Update st2client deps: editor and prompt-toolkit. #6189 (by @nzlosh) * Updated dependency oslo.config to prepare for python 3.10 support. #6193 (by @nzlosh) +* pass without exception if jinja parameter is not found. #6101 (by @guzzijones12) * Updated unit tests to use redis for coordination instead of the NoOp driver. This will hopefully make CI more stable. #6245 Contributed by @FileMagic, @guzzijones, and @cognifloyd diff --git a/st2api/tests/unit/controllers/v1/test_executions.py b/st2api/tests/unit/controllers/v1/test_executions.py index ea5e45b6fe..05231741d0 100644 --- a/st2api/tests/unit/controllers/v1/test_executions.py +++ b/st2api/tests/unit/controllers/v1/test_executions.py @@ -782,10 +782,8 @@ def test_post_parameter_render_failed(self): # Runner type does not expects additional properties. execution["parameters"]["hosts"] = "{{ABSENT}}" post_resp = self._do_post(execution, expect_errors=True) - self.assertEqual(post_resp.status_int, 400) - self.assertEqual( - post_resp.json["faultstring"], 'Dependency unsatisfied in variable "ABSENT"' - ) + # we no longer fail if parameter is not found + self.assertEqual(post_resp.status_int, 201) def test_post_parameter_validation_explicit_none(self): execution = copy.deepcopy(LIVE_ACTION_1) diff --git a/st2common/st2common/util/param.py b/st2common/st2common/util/param.py index 67fb83e9ac..9caaf97d0d 100644 --- a/st2common/st2common/util/param.py +++ b/st2common/st2common/util/param.py @@ -170,17 +170,81 @@ def _process_defaults(G, schemas): _process(G, name, value.get("default")) +def _check_any_bad(G, nodes, check_any_bad=None, tracked_parents=None): + """ + :param G: nx.DiGraph + :param nodes: list[str] + :param check_any_bad: list[boolean] + :param tracked_parents: list[str] + """ + if tracked_parents is None: + tracked_parents = [] + if check_any_bad is None: + check_any_bad = [] + for name in nodes: + if "value" not in G.nodes[name] and "template" not in G.nodes[name]: + # this is a string not a jinja template; embedded {{sometext}} + check_any_bad.append(False) + else: + check_any_bad.append(True) + if name not in tracked_parents: + children = [i for i in G.predecessors(name)] + tracked_parents.append(name) + _check_any_bad(G, children, check_any_bad, tracked_parents) + return check_any_bad + + +def _remove_bad(g_copy, parent, nodes, tracked_parents=None): + """ + :param g_copy: nx.DiGraph + :param parent: str + :param nodes: list[str] + :param tracked_parents: list[str] + """ + + if tracked_parents is None: + tracked_parents = [parent] + else: + tracked_parents.append(parent) + for name in nodes: + if "template" in g_copy.nodes[parent].keys(): + g_copy.nodes[parent]["value"] = g_copy.nodes[parent].pop("template") + if name in g_copy.nodes: + children = [i for i in g_copy.predecessors(name)] + if children: + if name not in tracked_parents: + _remove_bad(g_copy, name, children, tracked_parents) + # remove template for neighbors; this isn't actually a variable + # it is a value + # remove template attr if it exists on parent + # remove edges + g_copy.remove_edge(name, parent) + # remove node from graph + g_copy.remove_node(name) + + def _validate(G): """ Validates dependency graph to ensure it has no missing or cyclic dependencies """ + g_copy = G.copy() for name in G.nodes: - if "value" not in G.nodes[name] and "template" not in G.nodes[name]: - msg = 'Dependency unsatisfied in variable "%s"' % name - raise ParamException(msg) - - if not nx.is_directed_acyclic_graph(G): - graph_cycles = nx.simple_cycles(G) + children = [i for i in G.predecessors(name)] + if len(children) > 0: + # has children + check_all = _check_any_bad(G, children) + # check if all are FALSE + if not any(check_all): + # this is a string or object with embedded jinja string that + # doesn't exist as a parameter and not a jinja template; embedded {{sometext}} + _remove_bad(g_copy, name, children) + elif True in check_all and False in check_all: + msg = 'Dependency unsatisfied in variable "%s"' % name + # one of the parameters exists but not all + raise ParamException(msg) + + if not nx.is_directed_acyclic_graph(g_copy): + graph_cycles = nx.simple_cycles(g_copy) variable_names = [] for cycle in graph_cycles: @@ -197,6 +261,7 @@ def _validate(G): "referencing itself" % (variable_names) ) raise ParamException(msg) + return g_copy def _render(node, render_context): @@ -276,7 +341,6 @@ def _cast_params_from(params, context, schemas): # 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 have immutable set to true in schema if param_details.get("immutable"): continue @@ -336,8 +400,7 @@ def render_live_params( [_process(G, name, value) for name, value in six.iteritems(params)] _process_defaults(G, [action_parameters, runner_parameters]) - _validate(G) - + G = _validate(G) context = _resolve_dependencies(G) live_params = _cast_params_from( params, context, [action_parameters, runner_parameters] @@ -359,7 +422,7 @@ def render_final_params(runner_parameters, action_parameters, params, action_con # by that point, all params should already be resolved so any template should be treated value [G.add_node(name, value=value) for name, value in six.iteritems(params)] _process_defaults(G, [action_parameters, runner_parameters]) - _validate(G) + G = _validate(G) context = _resolve_dependencies(G) context = _cast_params_from( diff --git a/st2common/tests/unit/test_param_utils.py b/st2common/tests/unit/test_param_utils.py index 2aa2d05218..9b53f94543 100644 --- a/st2common/tests/unit/test_param_utils.py +++ b/st2common/tests/unit/test_param_utils.py @@ -59,7 +59,6 @@ class ParamsUtilsTest(DbTestCase): runnertype_db = FIXTURES["runners"]["testrunner1.yaml"] def test_process_jinja_exception(self): - action_context = {"api_user": "noob"} config = {} G = param_utils._create_graph(action_context, config) @@ -69,7 +68,6 @@ def test_process_jinja_exception(self): self.assertEquals(G.nodes.get(name, {}).get("value"), value) def test_process_jinja_template(self): - action_context = {"api_user": "noob"} config = {} G = param_utils._create_graph(action_context, config) @@ -521,31 +519,23 @@ def test_get_finalized_params_with_cyclic_dependency(self): self.assertTrue(test_pass) def test_get_finalized_params_with_missing_dependency(self): - params = {"r1": "{{r3}}", "r2": "{{r3}}"} + params = {"r1": "{{r3}}", "r2": "{{r3}}{{r4}}"} runner_param_info = {"r1": {}, "r2": {}} action_param_info = {} - test_pass = True - try: - param_utils.get_finalized_params( - runner_param_info, action_param_info, params, {"user": None} - ) - test_pass = False - except ParamException as e: - test_pass = six.text_type(e).find("Dependency") == 0 - self.assertTrue(test_pass) + result = param_utils.get_finalized_params( + runner_param_info, action_param_info, params, {"user": None} + ) + self.assertEquals(result[0]["r1"], params["r1"]) + self.assertEquals(result[0]["r2"], params["r2"]) params = {} runner_param_info = {"r1": {"default": "{{r3}}"}, "r2": {"default": "{{r3}}"}} action_param_info = {} - test_pass = True - try: - param_utils.get_finalized_params( - runner_param_info, action_param_info, params, {"user": None} - ) - test_pass = False - except ParamException as e: - test_pass = six.text_type(e).find("Dependency") == 0 - self.assertTrue(test_pass) + result2 = param_utils.get_finalized_params( + runner_param_info, action_param_info, params, {"user": None} + ) + self.assertEquals(result2[0]["r1"], runner_param_info["r1"]["default"]) + self.assertEquals(result2[0]["r2"], runner_param_info["r2"]["default"]) def test_get_finalized_params_no_double_rendering(self): params = {"r1": "{{ action_context.h1 }}{{ action_context.h2 }}"} @@ -788,17 +778,12 @@ def test_unsatisfied_dependency_friendly_error_message(self): } action_context = {"user": None} - expected_msg = 'Dependency unsatisfied in variable "variable_not_defined"' - self.assertRaisesRegex( - ParamException, - expected_msg, - param_utils.render_live_params, - runner_param_info, - action_param_info, - params, - action_context, + result = param_utils.render_live_params( + runner_param_info, action_param_info, params, action_context ) + self.assertEquals(result["r4"], params["r4"]) + def test_add_default_templates_to_live_params(self): """Test addition of template values in defaults to live params"""