From 09e968a4ecfad9ef975fa979ffebd7570ee3e86b Mon Sep 17 00:00:00 2001 From: guzzijones12 Date: Wed, 20 Dec 2023 20:44:50 +0000 Subject: [PATCH 01/11] pass without exception if jinja parameter isn't found fix tests so they do not check for exception for rendering parameters remove debugging and clean up comments remove comments in params rendering code black fixes st2common tests remove unused var --- st2common/st2common/util/param.py | 25 ++++++++++---- st2common/tests/unit/test_param_utils.py | 42 ++++++++---------------- 2 files changed, 31 insertions(+), 36 deletions(-) diff --git a/st2common/st2common/util/param.py b/st2common/st2common/util/param.py index 67fb83e9ac..8061e3458f 100644 --- a/st2common/st2common/util/param.py +++ b/st2common/st2common/util/param.py @@ -174,13 +174,22 @@ 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) + # this is a string not a jinja template; embedded {{sometext}} + for i in G.neighbors(name): + # remove template for neighbors; this isn't actually a variable + # it is a value + # remove template attr if it exists + g_copy.nodes[i]["value"] = g_copy.nodes[i].pop("template") + # remove edges + g_copy.remove_edge(name, i) + # remove node from graph + g_copy.remove_node(name) + + 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 +206,7 @@ def _validate(G): "referencing itself" % (variable_names) ) raise ParamException(msg) + return g_copy def _render(node, render_context): @@ -336,9 +346,10 @@ 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) + LOG.debug("context: %s" % str(context)) live_params = _cast_params_from( params, context, [action_parameters, runner_parameters] ) @@ -359,7 +370,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 ddcd2cd6ff..86a8d3cd0d 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) @@ -524,28 +522,20 @@ def test_get_finalized_params_with_missing_dependency(self): params = {"r1": "{{r3}}", "r2": "{{r3}}"} 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,16 +778,10 @@ def test_unsatisfied_dependency_friendly_error_message(self): } action_context = {"user": None} - 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, + 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""" From fd74ef5e627f1b84321aebd414def8545afa432a Mon Sep 17 00:00:00 2001 From: guzzijones12 Date: Thu, 21 Dec 2023 18:55:56 +0000 Subject: [PATCH 02/11] remove a debug --- st2common/st2common/util/param.py | 1 - 1 file changed, 1 deletion(-) diff --git a/st2common/st2common/util/param.py b/st2common/st2common/util/param.py index 8061e3458f..6c607f5983 100644 --- a/st2common/st2common/util/param.py +++ b/st2common/st2common/util/param.py @@ -349,7 +349,6 @@ def render_live_params( G = _validate(G) context = _resolve_dependencies(G) - LOG.debug("context: %s" % str(context)) live_params = _cast_params_from( params, context, [action_parameters, runner_parameters] ) From 06fc936e554b8e173ea466694211e69699672e51 Mon Sep 17 00:00:00 2001 From: guzzijones12 Date: Thu, 21 Dec 2023 19:11:30 +0000 Subject: [PATCH 03/11] do not fail if parameter is not found --- st2api/tests/unit/controllers/v1/test_executions.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/st2api/tests/unit/controllers/v1/test_executions.py b/st2api/tests/unit/controllers/v1/test_executions.py index eb3face2ba..34ade4c84e 100644 --- a/st2api/tests/unit/controllers/v1/test_executions.py +++ b/st2api/tests/unit/controllers/v1/test_executions.py @@ -774,10 +774,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) From 9f4603931aa18839b689969811f05d5eacc6a441 Mon Sep 17 00:00:00 2001 From: guzzijones12 Date: Fri, 8 Mar 2024 21:06:17 +0000 Subject: [PATCH 04/11] check that all variables for a parameter do not exist --- st2common/st2common/util/param.py | 76 +++++++++++++++++++++++++------ 1 file changed, 63 insertions(+), 13 deletions(-) diff --git a/st2common/st2common/util/param.py b/st2common/st2common/util/param.py index 6c607f5983..de4aa5549d 100644 --- a/st2common/st2common/util/param.py +++ b/st2common/st2common/util/param.py @@ -170,23 +170,75 @@ 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[dict] + :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) + children = [i for i in G.predecessors(name)] + if name not in tracked_parents: + tracked_parents.extend(nodes) + _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: nx.DiGraph + :param nodes: str + :param check_any_bad: list[str] + :param tracked_parents: list[str] + """ + + if tracked_parents is None: + tracked_parents = [parent] + for i in nodes: + g_copy.nodes[parent]["value"] = g_copy.nodes[parent].pop("template") + if i in g_copy.nodes: + children = [i for i in g_copy.predecessors(i)] + if children: + if i not in tracked_parents: + _remove_bad(g_copy, i, children) + # 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(i, parent) + # remove node from graph + g_copy.remove_node(i) + + 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]: - # this is a string not a jinja template; embedded {{sometext}} - for i in G.neighbors(name): - # remove template for neighbors; this isn't actually a variable - # it is a value - # remove template attr if it exists - g_copy.nodes[i]["value"] = g_copy.nodes[i].pop("template") - # remove edges - g_copy.remove_edge(name, i) - # remove node from graph - g_copy.remove_node(name) + 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) @@ -286,7 +338,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 @@ -347,7 +398,6 @@ def render_live_params( [_process(G, name, value) for name, value in six.iteritems(params)] _process_defaults(G, [action_parameters, runner_parameters]) G = _validate(G) - context = _resolve_dependencies(G) live_params = _cast_params_from( params, context, [action_parameters, runner_parameters] From 40f7de37c545286bc4381317f96238f633d75d73 Mon Sep 17 00:00:00 2001 From: guzzijones12 Date: Fri, 8 Mar 2024 21:39:26 +0000 Subject: [PATCH 05/11] add to tracked-parent if not none --- st2common/st2common/util/param.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/st2common/st2common/util/param.py b/st2common/st2common/util/param.py index de4aa5549d..1a45079dcc 100644 --- a/st2common/st2common/util/param.py +++ b/st2common/st2common/util/param.py @@ -204,6 +204,8 @@ def _remove_bad(g_copy, parent, nodes, tracked_parents=None): if tracked_parents is None: tracked_parents = [parent] + else: + tracked_parents.append(parent) for i in nodes: g_copy.nodes[parent]["value"] = g_copy.nodes[parent].pop("template") if i in g_copy.nodes: From 8f26137bbe81e5081566d267dc14a8b8ce9cfbfe Mon Sep 17 00:00:00 2001 From: guzzijones12 Date: Mon, 18 Mar 2024 18:03:22 +0000 Subject: [PATCH 06/11] add check if template exists; this is a problem with 2 children --- st2common/st2common/util/param.py | 3 ++- st2common/tests/unit/test_param_utils.py | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/st2common/st2common/util/param.py b/st2common/st2common/util/param.py index 1a45079dcc..dcae2613ea 100644 --- a/st2common/st2common/util/param.py +++ b/st2common/st2common/util/param.py @@ -207,7 +207,8 @@ def _remove_bad(g_copy, parent, nodes, tracked_parents=None): else: tracked_parents.append(parent) for i in nodes: - g_copy.nodes[parent]["value"] = g_copy.nodes[parent].pop("template") + if "template" in g_copy.nodes[parent].keys(): + g_copy.nodes[parent]["value"] = g_copy.nodes[parent].pop("template") if i in g_copy.nodes: children = [i for i in g_copy.predecessors(i)] if children: diff --git a/st2common/tests/unit/test_param_utils.py b/st2common/tests/unit/test_param_utils.py index 86a8d3cd0d..8b3ead639b 100644 --- a/st2common/tests/unit/test_param_utils.py +++ b/st2common/tests/unit/test_param_utils.py @@ -519,7 +519,7 @@ 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 = {} result = param_utils.get_finalized_params( From 08fc039b4f81d060f9166b3d73ac7134951428c4 Mon Sep 17 00:00:00 2001 From: guzzijones12 Date: Tue, 19 Mar 2024 16:32:53 +0000 Subject: [PATCH 07/11] execution children fetch one time if not in tracked parents --- st2common/st2common/util/param.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/st2common/st2common/util/param.py b/st2common/st2common/util/param.py index dcae2613ea..2e2dbabd15 100644 --- a/st2common/st2common/util/param.py +++ b/st2common/st2common/util/param.py @@ -187,8 +187,8 @@ def _check_any_bad(G, nodes, check_any_bad=None, tracked_parents=None): check_any_bad.append(False) else: check_any_bad.append(True) - children = [i for i in G.predecessors(name)] if name not in tracked_parents: + children = [i for i in G.predecessors(name)] tracked_parents.extend(nodes) _check_any_bad(G, children, check_any_bad, tracked_parents) return check_any_bad From 313437f2851f505029dc05f1e8356e468779e9e1 Mon Sep 17 00:00:00 2001 From: AJ Date: Fri, 13 Sep 2024 16:43:06 -0400 Subject: [PATCH 08/11] Update test_param_utils.py does not raise and error any more --- st2common/tests/unit/test_param_utils.py | 9 --------- 1 file changed, 9 deletions(-) diff --git a/st2common/tests/unit/test_param_utils.py b/st2common/tests/unit/test_param_utils.py index cc2410f17b..d51bf28d56 100644 --- a/st2common/tests/unit/test_param_utils.py +++ b/st2common/tests/unit/test_param_utils.py @@ -782,15 +782,6 @@ def test_unsatisfied_dependency_friendly_error_message(self): runner_param_info, action_param_info, params, action_context) 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 - ) self.assertEquals(result["r4"], params["r4"]) def test_add_default_templates_to_live_params(self): From 65d568576b6dfb28153f6c03b2dee827c864345a Mon Sep 17 00:00:00 2001 From: guzzijones12 Date: Mon, 16 Sep 2024 09:05:26 -0400 Subject: [PATCH 09/11] black fix --- st2common/tests/unit/test_param_utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/st2common/tests/unit/test_param_utils.py b/st2common/tests/unit/test_param_utils.py index d51bf28d56..af5f8c772e 100644 --- a/st2common/tests/unit/test_param_utils.py +++ b/st2common/tests/unit/test_param_utils.py @@ -779,7 +779,8 @@ def test_unsatisfied_dependency_friendly_error_message(self): action_context = {"user": None} result = param_utils.render_live_params( - runner_param_info, action_param_info, params, action_context) + runner_param_info, action_param_info, params, action_context + ) expected_msg = 'Dependency unsatisfied in variable "variable_not_defined"' self.assertEquals(result["r4"], params["r4"]) From 6aac6ade475516d9c12c8ace509b689578fc11e4 Mon Sep 17 00:00:00 2001 From: guzzijones12 Date: Mon, 16 Sep 2024 09:57:16 -0400 Subject: [PATCH 10/11] fix lint; unused var --- st2common/tests/unit/test_param_utils.py | 1 - 1 file changed, 1 deletion(-) diff --git a/st2common/tests/unit/test_param_utils.py b/st2common/tests/unit/test_param_utils.py index af5f8c772e..9b53f94543 100644 --- a/st2common/tests/unit/test_param_utils.py +++ b/st2common/tests/unit/test_param_utils.py @@ -782,7 +782,6 @@ def test_unsatisfied_dependency_friendly_error_message(self): runner_param_info, action_param_info, params, action_context ) - expected_msg = 'Dependency unsatisfied in variable "variable_not_defined"' self.assertEquals(result["r4"], params["r4"]) def test_add_default_templates_to_live_params(self): From 3bd25ab4a4cc3876a899338dedfd435f233f73cf Mon Sep 17 00:00:00 2001 From: guzzijones12 Date: Mon, 16 Sep 2024 11:04:20 -0400 Subject: [PATCH 11/11] update changelog --- CHANGELOG.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 8fbcebd310..2c199cf423 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -27,6 +27,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) Added ~~~~~