Only add namespace ansible_eda to extra_vars sent to playbooks (#372)

Namespace ansible_eda in rulebook is implied. It should never be
explictly referenced.

Also process var_root before Jinja substitution.

Resolves AAP-8967

Co-authored-by: Tom Tuffin <[email protected]>

ansible_rulebook/builtin.py

@@ -29,7 +29,6 @@
 from typing import Callable, Dict, List, Optional, Union
 
 import ansible_runner
-import dpath
 import janus
 import yaml
 from drools import ruleset as lang
@@ -103,21 +102,15 @@ async def print_event(
     source_rule_name: str,
     ruleset: str,
     name: Optional[str] = None,
-    var_root: Union[str, Dict, None] = None,
     pretty: Optional[str] = None,
 ):
     print_fn: Callable = print
     if pretty:
         print_fn = pprint
 
-    if var_root:
-        update_variables(variables, var_root)
+    var_name = "events" if "events" in variables else "event"
 
-    var_name = "event"
-    if "events" in variables[KEY_EDA_VARS]:
-        var_name = "events"
-
-    print_fn(variables[KEY_EDA_VARS][var_name])
+    print_fn(variables[var_name])
     sys.stdout.flush()
     await event_log.put(
         dict(
@@ -143,11 +136,8 @@ async def echo(
     source_rule_name: str,
     ruleset: str,
     name: Optional[str] = None,
-    var_root: Union[str, Dict, None] = None,
     message: Optional[str] = None,
 ):
-    if var_root:
-        update_variables(variables, var_root)
 
     print(str(datetime.now()) + " : " + message)
     sys.stdout.flush()
@@ -259,7 +249,6 @@ async def run_playbook(
     set_facts: Optional[bool] = None,
     post_events: Optional[bool] = None,
     verbosity: int = 0,
-    var_root: Union[str, Dict, None] = None,
     copy_files: Optional[bool] = False,
     json_mode: Optional[bool] = False,
     retries: Optional[int] = 0,
@@ -274,9 +263,9 @@ async def run_playbook(
         event_log,
         inventory,
         variables,
+        ruleset,
         name,
         "run_playbook",
-        var_root,
         copy_files,
         True,
         project_data_file,
@@ -356,7 +345,6 @@ async def run_module(
     set_facts: Optional[bool] = None,
     post_events: Optional[bool] = None,
     verbosity: int = 0,
-    var_root: Union[str, Dict, None] = None,
     copy_files: Optional[bool] = False,
     json_mode: Optional[bool] = False,
     module_args: Union[Dict, None] = None,
@@ -371,9 +359,9 @@ async def run_module(
         event_log,
         inventory,
         variables,
+        ruleset,
         name,
         "run_module",
-        var_root,
         copy_files,
         False,
         project_data_file,
@@ -531,9 +519,9 @@ async def pre_process_runner(
     event_log,
     inventory: Dict,
     variables: Dict,
+    ruleset: str,
     name: str,
     action: str,
-    var_root: Union[str, Dict, None] = None,
     copy_files: Optional[bool] = False,
     check_files: Optional[bool] = True,
     project_data_file: Optional[str] = None,
@@ -543,15 +531,10 @@ async def pre_process_runner(
 
     private_data_dir = tempfile.mkdtemp(prefix=action)
     logger.debug("private data dir %s", private_data_dir)
-    logger.debug("variables %s", variables)
-
-    for k, v in kwargs.items():
-        variables[k] = v
-
-    if var_root:
-        update_variables(variables, var_root)
 
-    playbook_extra_vars = _collect_extra_vars(variables, extra_vars)
+    playbook_extra_vars = _collect_extra_vars(
+        variables, extra_vars, ruleset, name
+    )
 
     env_dir = os.path.join(private_data_dir, "env")
     inventory_dir = os.path.join(private_data_dir, "inventory")
@@ -660,7 +643,6 @@ async def run_job_template(
     set_facts: Optional[bool] = None,
     post_events: Optional[bool] = None,
     verbosity: int = 0,
-    var_root: Union[str, Dict, None] = None,
     copy_files: Optional[bool] = False,
     json_mode: Optional[bool] = False,
     retries: Optional[int] = 0,
@@ -679,10 +661,8 @@ async def run_job_template(
         job_args = {}
     job_args["limit"] = hosts_limit
 
-    if var_root:
-        update_variables(variables, var_root)
     job_args["extra_vars"] = _collect_extra_vars(
-        variables, job_args.get("extra_vars", {})
+        variables, job_args.get("extra_vars", {}), ruleset, name
     )
 
     job_id = str(uuid.uuid4())
@@ -815,26 +795,6 @@ async def shutdown(
 )
 
 
-def update_variables(variables: Dict, var_root: Union[str, Dict]):
-    var_roots = {var_root: var_root} if isinstance(var_root, str) else var_root
-    variables_eda = variables[KEY_EDA_VARS]
-    if "event" in variables_eda:
-        for key, _new_key in var_roots.items():
-            new_value = dpath.get(
-                variables_eda["event"], key, separator=".", default=None
-            )
-            if new_value:
-                variables_eda["event"] = new_value
-                break
-    elif "events" in variables_eda:
-        for _k, v in variables_eda["events"].items():
-            for old_key, new_key in var_roots.items():
-                new_value = dpath.get(v, old_key, separator=".", default=None)
-                if new_value:
-                    variables_eda["events"][new_key] = new_value
-                    break
-
-
 def _get_latest_artifact(data_dir: str, artifact: str, content: bool = True):
     files = glob.glob(os.path.join(data_dir, "artifacts", "*", artifact))
     files.sort(key=os.path.getmtime, reverse=True)
@@ -846,15 +806,25 @@ def _get_latest_artifact(data_dir: str, artifact: str, content: bool = True):
 
 
 def _get_events(variables: Dict):
-    variables_eda = variables[KEY_EDA_VARS]
-    if "event" in variables_eda:
-        return {"m": variables_eda["event"]}
-    elif "events" in variables_eda:
-        return variables_eda["events"]
+    if "event" in variables:
+        return {"m": variables["event"]}
+    elif "events" in variables:
+        return variables["events"]
     return {}
 
 
-def _collect_extra_vars(variables: Dict, user_extra_vars: Dict):
+def _collect_extra_vars(
+    variables: Dict, user_extra_vars: Dict, ruleset: str, rule: str
+):
     extra_vars = user_extra_vars.copy() if user_extra_vars else {}
-    extra_vars[KEY_EDA_VARS] = variables[KEY_EDA_VARS]
+    eda_vars = dict(ruleset=ruleset, rule=rule)
+    if "events" in variables:
+        eda_vars["events"] = variables["events"]
+    if "event" in variables:
+        eda_vars["event"] = variables["event"]
+    if "facts" in variables:
+        eda_vars["facts"] = variables["events"]
+    if "fact" in variables:
+        eda_vars["fact"] = variables["event"]
+    extra_vars[KEY_EDA_VARS] = eda_vars
     return extra_vars

ansible_rulebook/engine.py

@@ -19,7 +19,7 @@
 import runpy
 from datetime import datetime
 from pprint import PrettyPrinter, pformat
-from typing import Any, Dict, List, Optional, cast
+from typing import Any, Dict, List, Optional, Union, cast
 
 import dpath
 from drools import ruleset as lang
@@ -30,7 +30,7 @@
 )
 
 import ansible_rulebook.rule_generator as rule_generator
-from ansible_rulebook.builtin import KEY_EDA_VARS, actions as builtin_actions
+from ansible_rulebook.builtin import actions as builtin_actions
 from ansible_rulebook.collection import (
     find_source,
     find_source_filter,
@@ -383,20 +383,18 @@ async def _call_action(
                 else:
                     multi_match = rules_engine_result.data
                 variables_copy = variables.copy()
-                variables_eda = {}
-                variables_copy[KEY_EDA_VARS] = variables_eda
                 if single_match is not None:
-                    variables_eda["event"] = single_match
-                    variables_eda["fact"] = single_match
+                    variables_copy["event"] = single_match
+                    variables_copy["fact"] = single_match
                     event = single_match
                     if "meta" in event:
                         if "hosts" in event["meta"]:
                             hosts = parse_hosts(event["meta"]["hosts"])
                 else:
-                    variables_eda["events"] = multi_match
-                    variables_eda["facts"] = multi_match
+                    variables_copy["events"] = multi_match
+                    variables_copy["facts"] = multi_match
                     new_hosts = []
-                    for event in variables_eda["events"].values():
+                    for event in variables_copy["events"].values():
                         if "meta" in event:
                             if "hosts" in event["meta"]:
                                 new_hosts.extend(
@@ -405,6 +403,15 @@ async def _call_action(
                     if new_hosts:
                         hosts = new_hosts
 
+                if "var_root" in action_args:
+                    var_root = action_args.pop("var_root")
+                    logger.info(
+                        "Update variables [%s] with new root [%s]",
+                        variables_copy,
+                        var_root,
+                    )
+                    _update_variables(variables_copy, var_root)
+
                 logger.info(
                     "substitute_variables [%s] [%s]",
                     action_args,
@@ -551,3 +558,22 @@ def prime_facts(name: str, hosts_facts: List[Dict]):
             lang.assert_fact(name, data)
         except MessageNotHandledException:
             pass
+
+
+def _update_variables(variables: Dict, var_root: Union[str, Dict]):
+    var_roots = {var_root: var_root} if isinstance(var_root, str) else var_root
+    if "event" in variables:
+        for key, _new_key in var_roots.items():
+            new_value = dpath.get(
+                variables["event"], key, separator=".", default=None
+            )
+            if new_value:
+                variables["event"] = new_value
+                break
+    elif "events" in variables:
+        for _k, v in variables["events"].items():
+            for old_key, new_key in var_roots.items():
+                new_value = dpath.get(v, old_key, separator=".", default=None)
+                if new_value:
+                    variables["events"][new_key] = new_value
+                    break

docs/events_and_facts.rst

@@ -44,3 +44,7 @@ Instead, the ``all`` operator must be used:
 
 
 You can combine `set_fact <actions.html#set-fact>`_ and `retract_fact <actions.html#retract-fact>`_ actions to manage the global state during the lifecycle of your rulebook.
+
+The text above describes how to use ``events`` or ``facts`` in a rulebook. A single matched ``event`` or multiple matched ``events`` are also
+sent to a playbook through extra_vars under namespace ``ansible_eda`` when a run_playbook or run_job_template action is executed. So in a playbook
+you should reference them as ``ansible_eda.event`` or ``ansible_eda.events``.

tests/e2e/files/rulebooks/actions/test_actions_sanity.yml

@@ -35,7 +35,7 @@
         run_module:
           name: ansible.builtin.debug
           module_args:
-            msg: "Event matched: {{ ansible_eda.event }}"
+            msg: "Event matched: {{ event }}"
 
     - name: print_event
       condition: event.action == "print_event"
@@ -60,7 +60,7 @@
         run_module:
           name: ansible.builtin.debug
           module_args:
-            msg: "Event matched in same ruleset: {{ ansible_eda.event.post_event_state }}"
+            msg: "Event matched in same ruleset: {{ event.post_event_state }}"
 
     - name: post_event different ruleset
       condition: event.action == "post_event" and event.rulebook_scope == "different_ruleset"
@@ -83,7 +83,7 @@
         run_module:
           name: ansible.builtin.debug
           module_args:
-            msg: "Fact matched in same ruleset: {{ ansible_eda.event.my_fact }}"
+            msg: "Fact matched in same ruleset: {{ event.my_fact }}"
 
     - name: set_fact different ruleset
       condition: event.action == "set_fact" and event.rulebook_scope == "different_ruleset"
@@ -130,12 +130,12 @@
         run_module:
           name: ansible.builtin.debug
           module_args:
-            msg: "Event matched in different ruleset: {{ ansible_eda.event.post_event_state }}"
+            msg: "Event matched in different ruleset: {{ event.post_event_state }}"
 
     - name: print set_fact different ruleset
       condition: event.my_fact == "sent"
       action:
         run_module:
           name: ansible.builtin.debug
           module_args:
-            msg: "Fact matched in different ruleset: {{ ansible_eda.event.my_fact }}"
+            msg: "Fact matched in different ruleset: {{ event.my_fact }}"

tests/e2e/files/rulebooks/actions/test_run_playbook.yml

@@ -18,7 +18,7 @@
           name: ./playbooks/run_playbook_test_playbook.yml
           post_events: true
           extra_vars:
-            target_host: "{{ ansible_eda.event.meta.hostname }}"
+            target_host: "{{ event.meta.hostname }}"
             remediation_required: true
           retry: true
           retries: 1
@@ -35,8 +35,7 @@
           var_root:
             results.remediation: results
           extra_vars:
-            # use var_root when bug resolved: https://issues.redhat.com/browse/AAP-8967
-            processed_host: "{{ ansible_eda.event.results.remediation.remediated_host }}"
+            processed_host: "{{ event.remediated_host }}"
             remediation_required: false
           json_mode: true
           verbosity: 4
@@ -53,4 +52,4 @@
             event.post_processing_host == "simba"
       action:
         echo:
-          message: "Post-processing complete on {{ ansible_eda.event.post_processing_host }}"
+          message: "Post-processing complete on {{ event.post_processing_host }}"

tests/e2e/test_actions.py

@@ -64,8 +64,7 @@ def test_actions_sanity():
  'ruleset': 'Test actions sanity',
  'source_rule_name': 'debug',
  'source_ruleset_name': 'Test actions sanity',
- 'variables': {'ansible_eda': {'event': {'action': 'debug'},
-                               'fact': {'action': 'debug'}}}}"""  # noqa: E501
+ 'variables': {'event': {'action': 'debug'}, 'fact': {'action': 'debug'}}}"""  # noqa: E501
 
     assert event_debug_expected_output in result.stdout, "debug action failed"
 
@@ -93,7 +92,7 @@ def test_actions_sanity():
     assert "Echo action executed" in result.stdout
 
     assert (
-        len(result.stdout.splitlines()) == 44
+        len(result.stdout.splitlines()) == 43
     ), "unexpected output from the rulebook"