From 8deb66150a1171159493f3e6da64003d8084a461 Mon Sep 17 00:00:00 2001 From: Madhu Kanoor Date: Tue, 28 Feb 2023 11:50:55 -0500 Subject: [PATCH 1/5] Updated to use the newer drools_jpy (#392) version 0.2.3 Fixed the following issues - selectattr operator with negation using greater/less than operators - select operator and comparing ints and floats https://issues.redhat.com/browse/AAP-9647 https://issues.redhat.com/browse/AAP-9616 --- CHANGELOG.md | 2 ++ setup.cfg | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 634e79b9..24993024 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,8 @@ ### Fixed - get_java_version, add compatibility with macs and tests for check_jvm +- selectattr operator with negation using greater/less than operators +- select operator and comparing ints and floats ### Removed diff --git a/setup.cfg b/setup.cfg index 42b6b004..b1058462 100644 --- a/setup.cfg +++ b/setup.cfg @@ -32,7 +32,7 @@ install_requires = janus ansible-runner websockets - drools_jpy == 0.2.2 + drools_jpy == 0.2.3 [options.packages.find] include = From d6b64e951da1a3c86b676be4544b66149043fba1 Mon Sep 17 00:00:00 2001 From: Madhu Kanoor Date: Tue, 28 Feb 2023 11:59:55 -0500 Subject: [PATCH 2/5] [AAP-8921] Add support for Jinja2 substitution in rule names (#391) https://issues.redhat.com/browse/AAP-8921 Allows for rule names to be substituted from vars passed into ansible-rulebook. Co-authored-by: Tom Tuffin <71447672+ttuffin@users.noreply.github.com> --- CHANGELOG.md | 1 + ansible_rulebook/app.py | 12 ++-- ansible_rulebook/rules_parser.py | 17 ++++-- docs/rules.rst | 2 +- tests/rules/rule_names_with_substitution.yml | 15 +++++ tests/test_rules.py | 63 ++++++++++++++++++++ 6 files changed, 101 insertions(+), 9 deletions(-) create mode 100644 tests/rules/rule_names_with_substitution.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index 24993024..73093276 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ - Support for graceful shutdown, timeout to allow actions to complete - Removed the echo command in favor of debug with msg - Support for null type in conditions +- Support Jinja2 substitution in rule names ### Fixed diff --git a/ansible_rulebook/app.py b/ansible_rulebook/app.py index de6d6c25..fcf4b1ee 100644 --- a/ansible_rulebook/app.py +++ b/ansible_rulebook/app.py @@ -17,7 +17,7 @@ import logging import os from asyncio.exceptions import CancelledError -from typing import Any, Dict, List, Tuple +from typing import Any, Dict, List, Optional, Tuple import yaml @@ -62,7 +62,7 @@ async def run(parsed_args: argparse.ArgumentParser) -> None: else: inventory = {} variables = load_vars(parsed_args) - rulesets = load_rulebook(parsed_args) + rulesets = load_rulebook(parsed_args, variables) if parsed_args.inventory: inventory = load_inventory(parsed_args.inventory) project_data_file = parsed_args.project_tarball @@ -139,7 +139,9 @@ def load_vars(parsed_args) -> Dict[str, str]: # TODO(cutwater): Maybe move to util.py -def load_rulebook(parsed_args) -> List[RuleSet]: +def load_rulebook( + parsed_args: argparse.ArgumentParser, variables: Optional[Dict] = None +) -> List[RuleSet]: if not parsed_args.rulebook: logger.debug("Loading no rules") return [] @@ -150,7 +152,9 @@ def load_rulebook(parsed_args) -> List[RuleSet]: with open(parsed_args.rulebook) as f: data = yaml.safe_load(f.read()) Validate.rulebook(data) - return rules_parser.parse_rule_sets(data) + if variables is None: + variables = {} + return rules_parser.parse_rule_sets(data, variables) elif has_rulebook(*split_collection_name(parsed_args.rulebook)): logger.debug( "Loading rules from a collection %s", parsed_args.rulebook diff --git a/ansible_rulebook/rules_parser.py b/ansible_rulebook/rules_parser.py index c32875c8..7e39c56a 100644 --- a/ansible_rulebook/rules_parser.py +++ b/ansible_rulebook/rules_parser.py @@ -12,13 +12,14 @@ # See the License for the specific language governing permissions and # limitations under the License. -from typing import Any, Dict, List +from typing import Any, Dict, List, Optional import ansible_rulebook.rule_types as rt from ansible_rulebook.condition_parser import ( parse_condition as parse_condition_value, ) from ansible_rulebook.job_template_runner import job_template_runner +from ansible_rulebook.util import substitute_variables from .exception import ( RulenameDuplicateException, @@ -37,7 +38,9 @@ def parse_hosts(hosts): raise Exception(f"Unsupported hosts value {hosts}") -def parse_rule_sets(rule_sets: Dict) -> List[rt.RuleSet]: +def parse_rule_sets( + rule_sets: Dict, variables: Optional[Dict] = None +) -> List[rt.RuleSet]: rule_set_list = [] ruleset_names = [] for rule_set in rule_sets: @@ -58,12 +61,15 @@ def parse_rule_sets(rule_sets: Dict) -> List[rt.RuleSet]: ruleset_names.append(name) + if variables is None: + variables = {} + rule_set_list.append( rt.RuleSet( name=name, hosts=parse_hosts(rule_set["hosts"]), sources=parse_event_sources(rule_set["sources"]), - rules=parse_rules(rule_set.get("rules", {})), + rules=parse_rules(rule_set.get("rules", {}), variables), gather_facts=rule_set.get("gather_facts", False), ) ) @@ -102,14 +108,17 @@ def parse_source_filter(source_filter: Dict) -> rt.EventSourceFilter: return rt.EventSourceFilter(source_filter_name, source_filter_args) -def parse_rules(rules: Dict) -> List[rt.Rule]: +def parse_rules(rules: Dict, variables: Dict) -> List[rt.Rule]: rule_list = [] rule_names = [] + if variables is None: + variables = {} for rule in rules: name = rule.get("name") if name is None: raise RulenameEmptyException("Rule name not provided") + name = substitute_variables(name, variables) if name == "": raise RulenameEmptyException("Rule name cannot be an empty string") diff --git a/docs/rules.rst b/docs/rules.rst index 929d8b6d..45d5069c 100644 --- a/docs/rules.rst +++ b/docs/rules.rst @@ -18,7 +18,7 @@ A rule comprises: - Description - Required * - name - - The name is a string to identify the rule. This field is mandatory. Each rule in a ruleset must have an unique name across the rulebook. + - The name is a string to identify the rule. This field is mandatory. Each rule in a ruleset must have an unique name across the rulebook. You can use Jinja2 substitution in the name. - Yes * - condition - See :doc:`conditions` diff --git a/tests/rules/rule_names_with_substitution.yml b/tests/rules/rule_names_with_substitution.yml new file mode 100644 index 00000000..3e6ae0a3 --- /dev/null +++ b/tests/rules/rule_names_with_substitution.yml @@ -0,0 +1,15 @@ +--- +- name: Rules with names being substituted + hosts: localhost + sources: + - range: + limit: 5 + rules: + - name: "{{ custom.name1 }}" + condition: event.i == 1 + action: + debug: + - name: "{{ custom.name2 }}" + condition: event.i == 2 + action: + debug: diff --git a/tests/test_rules.py b/tests/test_rules.py index fc620d29..7a0fe293 100644 --- a/tests/test_rules.py +++ b/tests/test_rules.py @@ -19,8 +19,11 @@ import pytest import yaml from drools.ruleset import assert_fact as set_fact, post +from jinja2.exceptions import UndefinedError from ansible_rulebook.exception import ( + RulenameDuplicateException, + RulenameEmptyException, RulesetNameDuplicateException, RulesetNameEmptyException, ) @@ -174,3 +177,63 @@ async def test_missing_ruleset_names(): parse_rule_sets(data) assert str(exc_info.value) == "Ruleset name not provided" + + +@pytest.mark.asyncio +async def test_rule_name_substitution_duplicates(): + os.chdir(HERE) + variables = {"custom": {"name1": "fred", "name2": "fred"}} + with open("rules/rule_names_with_substitution.yml") as f: + data = yaml.safe_load(f.read()) + + with pytest.raises(RulenameDuplicateException): + parse_rule_sets(data, variables) + + +@pytest.mark.asyncio +async def test_rule_name_substitution_empty(): + os.chdir(HERE) + variables = {"custom": {"name1": "", "name2": "fred"}} + with open("rules/rule_names_with_substitution.yml") as f: + data = yaml.safe_load(f.read()) + + with pytest.raises(RulenameEmptyException): + parse_rule_sets(data, variables) + + +@pytest.mark.asyncio +async def test_rule_name_substitution_missing(): + os.chdir(HERE) + variables = {"custom": {"name2": "fred"}} + with open("rules/rule_names_with_substitution.yml") as f: + data = yaml.safe_load(f.read()) + + with pytest.raises(UndefinedError): + parse_rule_sets(data, variables) + + +@pytest.mark.asyncio +async def test_rule_name_substitution(): + os.chdir(HERE) + variables = {"custom": {"name1": "barney", "name2": "fred"}} + with open("rules/rule_names_with_substitution.yml") as f: + data = yaml.safe_load(f.read()) + with open("playbooks/inventory.yml") as f: + inventory = yaml.safe_load(f.read()) + + rulesets = parse_rule_sets(data, variables) + ruleset_queues = [(ruleset, Queue()) for ruleset in rulesets] + durable_rulesets = generate_rulesets(ruleset_queues, dict(), inventory) + ruleset_name = durable_rulesets[0].ruleset.name + + durable_rulesets[0].plan.queue = asyncio.Queue() + post(ruleset_name, {"i": 1}) + assert durable_rulesets[0].plan.queue.qsize() == 1 + event = durable_rulesets[0].plan.queue.get_nowait() + assert event.rule == "barney" + assert event.actions[0].action == "debug" + post(ruleset_name, {"i": 2}) + assert durable_rulesets[0].plan.queue.qsize() == 1 + event = durable_rulesets[0].plan.queue.get_nowait() + assert event.rule == "fred" + assert event.actions[0].action == "debug" From de7b97cb6ceac03135927f4b2b04c1989141fc22 Mon Sep 17 00:00:00 2001 From: Tom Tuffin <71447672+ttuffin@users.noreply.github.com> Date: Tue, 28 Feb 2023 18:02:58 +0000 Subject: [PATCH 3/5] Change variables e2e test to use variable in rulename (#394) --- tests/e2e/files/rulebooks/variables/test_variables_sanity.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/e2e/files/rulebooks/variables/test_variables_sanity.yml b/tests/e2e/files/rulebooks/variables/test_variables_sanity.yml index 94113694..f694e39f 100644 --- a/tests/e2e/files/rulebooks/variables/test_variables_sanity.yml +++ b/tests/e2e/files/rulebooks/variables/test_variables_sanity.yml @@ -14,8 +14,7 @@ lockdown_zone: 5 rules: - # use location var in rule name when bug resolved: https://issues.redhat.com/browse/AAP-8921 - - name: Intruder detected in hobart + - name: "Intruder detected in {{ alarm_location }}" condition: > event.action == "intruder_detected" and event.meta.location == vars.alarm_location and From cead990f07fb8c360f5b2d2fafbd94d95e308afb Mon Sep 17 00:00:00 2001 From: Alejandro Izquierdo Date: Wed, 1 Mar 2023 12:19:32 +0100 Subject: [PATCH 4/5] Fix json schema and validate it at load time --- ansible_rulebook/schema/ruleset_schema.json | 4 ++-- ansible_rulebook/validators.py | 14 +++++++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/ansible_rulebook/schema/ruleset_schema.json b/ansible_rulebook/schema/ruleset_schema.json index e631026b..1a2d9399 100644 --- a/ansible_rulebook/schema/ruleset_schema.json +++ b/ansible_rulebook/schema/ruleset_schema.json @@ -1,6 +1,6 @@ { - "$schema": "http://json-schema.org/draft-04/schema#", - "$id": "http://ansible.com/schema/ruleset.json", + "$schema": "https://json-schema.org/draft-04/schema", + "$id": "https://raw.githubusercontent.com/ansible/ansible-rulebook/main/ansible_rulebook/schema/ruleset_schema.json", "type": "array", "items": {"$ref": "#/$defs/ruleset"}, "minItems": 1, diff --git a/ansible_rulebook/validators.py b/ansible_rulebook/validators.py index b7e5a7ae..3bda06af 100644 --- a/ansible_rulebook/validators.py +++ b/ansible_rulebook/validators.py @@ -8,8 +8,8 @@ else: import importlib_resources as resources -from jsonschema import validate -from jsonschema.exceptions import ValidationError +from jsonschema import Draft4Validator, validate +from jsonschema.exceptions import SchemaError, ValidationError DEFAULT_RULEBOOK_SCHEMA = "ruleset_schema" logger = logging.getLogger(__name__) @@ -27,7 +27,15 @@ def _get_schema(cls): f"./schema/{DEFAULT_RULEBOOK_SCHEMA}.json" ) data = path.read_text(encoding="utf-8") - cls.schema = json.loads(data) + try: + cls.schema = json.loads(data) + Draft4Validator.check_schema(cls.schema) + except json.JSONDecodeError: + logger.exception("Can not deserialize JSON schema") + raise + except SchemaError: + logger.exception("Incorrect JSON schema") + raise return cls.schema @classmethod From 30b8f5a90d5f1d25eeb31c9d0c8f4331861b04e8 Mon Sep 17 00:00:00 2001 From: Madhu Kanoor Date: Tue, 28 Feb 2023 17:17:10 -0500 Subject: [PATCH 5/5] [AAP-9186] Use NativeTemple in Jinja2 to preserve types https://jinja.palletsprojects.com/en/3.0.x/nativetypes/ https://issues.redhat.com/browse/AAP-9186 Jinja2 support native types if we use NativeTemplate instead of regular template. This helps us preserve the data type in action args and source args. --- CHANGELOG.md | 1 + ansible_rulebook/util.py | 10 ++-- .../operators/test_relational_operators.yml | 2 +- tests/examples/72_set_fact_with_type.yml | 48 +++++++++++++++++++ tests/test_examples.py | 31 ++++++++++++ 5 files changed, 88 insertions(+), 4 deletions(-) create mode 100644 tests/examples/72_set_fact_with_type.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index 73093276..fad2091b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ - get_java_version, add compatibility with macs and tests for check_jvm - selectattr operator with negation using greater/less than operators - select operator and comparing ints and floats +- preserve native types when doing jinja substitution ### Removed diff --git a/ansible_rulebook/util.py b/ansible_rulebook/util.py index 27c043c5..c3a2cfc7 100644 --- a/ansible_rulebook/util.py +++ b/ansible_rulebook/util.py @@ -26,6 +26,7 @@ import ansible_runner import jinja2 import yaml +from jinja2.nativetypes import NativeTemplate from packaging import version logger = logging.getLogger(__name__) @@ -39,9 +40,12 @@ def get_horizontal_rule(character): def render_string(value: str, context: Dict) -> str: - return jinja2.Template(value, undefined=jinja2.StrictUndefined).render( - context - ) + if "{{" in value and "}}" in value: + return NativeTemplate(value, undefined=jinja2.StrictUndefined).render( + context + ) + + return value def render_string_or_return_value(value: Any, context: Dict) -> Any: diff --git a/tests/e2e/files/rulebooks/operators/test_relational_operators.yml b/tests/e2e/files/rulebooks/operators/test_relational_operators.yml index 0d4fbbaf..0295fc6a 100644 --- a/tests/e2e/files/rulebooks/operators/test_relational_operators.yml +++ b/tests/e2e/files/rulebooks/operators/test_relational_operators.yml @@ -57,7 +57,7 @@ pi_for_engineers: 3 - id: "testcase #11" - stark_motto: | + stark_motto: |- Winter is coming diff --git a/tests/examples/72_set_fact_with_type.yml b/tests/examples/72_set_fact_with_type.yml new file mode 100644 index 00000000..a70e7f61 --- /dev/null +++ b/tests/examples/72_set_fact_with_type.yml @@ -0,0 +1,48 @@ +--- +- name: 72 set fact with type + hosts: all + sources: + - generic: + shutdown_after: 1 + payload: + - action: "go" + rules: + - name: r1 + condition: event.action == "go" + action: + set_fact: + fact: + var_bool: "{{ my_bool }}" + var_int: "{{ my_int }}" + var_float: "{{ my_float }}" + literal_int: 5 + string_int: "5" + + - name: Match the bool + condition: event.var_bool + action: + debug: + msg: "The var bool matches" + + - name: Match the int + condition: event.var_int == 2 + action: + debug: + msg: "The var int matches" + + - name: Match the float + condition: event.var_float == 3.123 + action: + debug: + msg: "The var int matches" + + - name: Match the literal int + condition: event.literal_int == 5 + action: + debug: + msg: "The literal int matches" + - name: Match the string int + condition: event.string_int == "5" + action: + debug: + msg: "The string int matches" diff --git a/tests/test_examples.py b/tests/test_examples.py index 113f7251..12141210 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -1844,3 +1844,34 @@ async def test_70_null(): ], } validate_events(event_log, **checks) + + +@pytest.mark.asyncio +async def test_72_set_fact_with_type(): + ruleset_queues, event_log = load_rulebook( + "examples/72_set_fact_with_type.yml", + ) + + queue = ruleset_queues[0][1] + rs = ruleset_queues[0][0] + with SourceTask(rs.sources[0], "sources", {}, queue): + await run_rulesets( + event_log, + ruleset_queues, + dict(my_bool=True, my_int=2, my_float=3.123), + load_inventory("playbooks/inventory.yml"), + ) + + checks = { + "max_events": 7, + "shutdown_events": 1, + "actions": [ + "72 set fact with type::r1::set_fact", + "72 set fact with type::Match the bool::debug", + "72 set fact with type::Match the int::debug", + "72 set fact with type::Match the float::debug", + "72 set fact with type::Match the literal int::debug", + "72 set fact with type::Match the string int::debug", + ], + } + validate_events(event_log, **checks)