From 7c1840b17b6934c804b5cf47fc62489eb1cb2780 Mon Sep 17 00:00:00 2001 From: Madhu Kanoor Date: Fri, 27 Sep 2024 20:02:27 +0530 Subject: [PATCH 1/8] feat: option to skip including events into extra_vars (#716) When we unconditionally send event data to the controller for a workflow template the launch of the workflow template fails. With this option in a rulebook the user can prevent event data from being sent by setting - include_events: false The default value is true so existing rulebooks will keep sending events to controller. Fixes #622 https://issues.redhat.com/browse/AAP-13456 Root Cause Analysis: In the Controller if you are running a workflow template you cannot send event data to it unconditionally like we can do with job template. We get an error from Controller **Check the Prompt on Launch setting on the Workflow Job Template to include Extra Variables.** If a survey spec is involved then there are more constraints if there are certain parameters that are marked as required in the survey spec. So we get 2 distinct items - Prompt on Launch for Extra Variables - Survey Spec The following use cases are supported for workflow job template with and without survey spec. The use cases are based on a rulebook that looks like the following with the action being updated for every case. ``` --- - name: Test run job templates hosts: all sources: - ansible.eda.generic: payload: - name: Fred age: 25 rules: - name: "Run job template" condition: event.age == 25 action: run_workflow_template: name: workflow_mk organization: Default ``` ### **Case 1:** If your Workflow Template doesn't support Prompt on Launch for Extra Vars and a Survey Spec is not enabled ``` run_workflow_template: name: workflow_mk include_events: false organization: Default ``` ### **Case 2:** If your Workflow Template supports Prompt on Launch for Extra Vars and a Survey Spec is not enabled ``` run_workflow_template: name: workflow_mk organization: Default job_args: extra_vars: name: "{{ event.name }}" ``` The payload sent to Controller ``` name: Fred ansible_eda: ruleset: Test run job templates rule: Run job template event: meta: source: name: ansible.eda.generic type: ansible.eda.generic received_at: '2024-09-26T05:29:10.983346Z' uuid: 9570c7f8-9bc9-438f-9199-803707e31ccf name: Fred age: 25 ``` ### **Case 3:** If your Workflow Template doesn't support Prompt on Launch for Extra Vars and a Survey Spec is enabled ``` run_workflow_template: name: workflow_mk include_events: false organization: Default job_args: extra_vars: name: "{{ event.name }}" ``` The payload sent to Controller ``` name: Fred ``` ### **Case 4:** If your Workflow Template supports Prompt on Launch for Extra Vars and a Survey Spec is enabled ``` action: run_workflow_template: name: workflow_mk organization: Default job_args: extra_vars: name: "{{ event.name }}" ``` The payload sent to Controller ``` name: Fred ansible_eda: ruleset: Test run job templates rule: Run job template event: meta: source: name: ansible.eda.generic type: ansible.eda.generic received_at: '2024-09-26T07:26:42.160723Z' uuid: 46169e51-ba6f-4a55-963b-b626fff7cfdc name: Fred age: 25 ``` ### **Case 5:** Survey Spec has a required parameter and its not provided ``` action: run_workflow_template: name: workflow_mk organization: Default ``` Missing args 2024-09-26 12:59:03,738 - ansible_rulebook.job_template_runner - ERROR - Error {'variables_needed_to_start': ["'name' value missing"]} ### **Case 6:** Survey Spec has no required parameter and its not provided ``` action: run_workflow_template: name: workflow_mk organization: Default ``` The Default Value is provided by the Survey Spec and it works ``` name: fred ansible_eda: ruleset: Test run job templates rule: Run job template event: meta: source: name: ansible.eda.generic type: ansible.eda.generic received_at: '2024-09-26T07:55:15.320036Z' uuid: 68a6c25d-dcf5-4a1c-8eb2-7432376d2d88 name: Fred age: 25 ``` --- CHANGELOG.md | 1 + ansible_rulebook/action/helper.py | 12 ++- ansible_rulebook/action/run_job_template.py | 4 +- .../action/run_workflow_template.py | 5 +- ansible_rulebook/job_template_runner.py | 7 -- ansible_rulebook/schema/ruleset_schema.json | 8 ++ docs/actions.rst | 6 ++ sonar-project.properties | 29 ++++++ .../84_job_template_exclude_events.yml | 20 ++++ .../85_workflow_template_exclude_events.yml | 20 ++++ .../86_job_template_include_events.yml | 19 ++++ .../87_workflow_template_include_events.yml | 19 ++++ tests/examples/88_job_template_no_args.yml | 16 ++++ tests/test_examples.py | 94 +++++++++++++++++++ 14 files changed, 248 insertions(+), 12 deletions(-) create mode 100644 sonar-project.properties create mode 100644 tests/examples/84_job_template_exclude_events.yml create mode 100644 tests/examples/85_workflow_template_exclude_events.yml create mode 100644 tests/examples/86_job_template_include_events.yml create mode 100644 tests/examples/87_workflow_template_include_events.yml create mode 100644 tests/examples/88_job_template_no_args.yml diff --git a/CHANGELOG.md b/CHANGELOG.md index a46ad426..ab032e0b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ ### Changed ### Added ### Fixed +- Allow user to optionally include matching events ## [1.1.1] - 2024-09-19 diff --git a/ansible_rulebook/action/helper.py b/ansible_rulebook/action/helper.py index 3d7142b7..3135fcac 100644 --- a/ansible_rulebook/action/helper.py +++ b/ansible_rulebook/action/helper.py @@ -125,15 +125,23 @@ def embellish_internal_event(self, event: Dict) -> Dict: def set_action(self, action) -> None: self.action = action - def collect_extra_vars(self, user_extra_vars: Dict) -> Dict: + def collect_extra_vars( + self, user_extra_vars: Dict, include_events: bool = True + ) -> Dict: """When we send information to ansible-playbook or job template - on AWX, we need the rule and event specific information to + on AWX, we need the rule and optionally event specific information to be sent to this external process the caller passes in the user_extra_vars from the action args and then we append eda specific vars and return that as a the updated dictionary that is sent to the external process + + if the caller doesn't want to include events data return the + user_extra_vars. """ + if not include_events: + return user_extra_vars + extra_vars = user_extra_vars.copy() if user_extra_vars else {} eda_vars = { diff --git a/ansible_rulebook/action/run_job_template.py b/ansible_rulebook/action/run_job_template.py index 2d0b2db7..8415bc30 100644 --- a/ansible_rulebook/action/run_job_template.py +++ b/ansible_rulebook/action/run_job_template.py @@ -72,8 +72,10 @@ async def __call__(self): ) self.job_args["extra_vars"] = self.helper.collect_extra_vars( - self.job_args.get("extra_vars", {}) + self.job_args.get("extra_vars", {}), + self.action_args.get("include_events", True), ) + await self._job_start_event() await self._run() diff --git a/ansible_rulebook/action/run_workflow_template.py b/ansible_rulebook/action/run_workflow_template.py index d0f864b1..81a94d04 100644 --- a/ansible_rulebook/action/run_workflow_template.py +++ b/ansible_rulebook/action/run_workflow_template.py @@ -70,10 +70,11 @@ async def __call__(self): self.helper.metadata.rule_set, self.helper.metadata.rule, ) - self.job_args["extra_vars"] = self.helper.collect_extra_vars( - self.job_args.get("extra_vars", {}) + self.job_args.get("extra_vars", {}), + self.action_args.get("include_events", True), ) + await self._job_start_event() await self._run() diff --git a/ansible_rulebook/job_template_runner.py b/ansible_rulebook/job_template_runner.py index 4871dc67..c923b0c6 100644 --- a/ansible_rulebook/job_template_runner.py +++ b/ansible_rulebook/job_template_runner.py @@ -209,13 +209,6 @@ async def run_workflow_job_template( "Workflow template %s does not accept limit, removing it", name ) job_params.pop("limit") - if not obj["ask_variables_on_launch"] and "extra_vars" in job_params: - logger.warning( - "Workflow template %s does not accept extra vars, " - "removing it", - name, - ) - job_params.pop("extra_vars") job = await self._launch(job_params, url) return await self._monitor_job(job["url"]) diff --git a/ansible_rulebook/schema/ruleset_schema.json b/ansible_rulebook/schema/ruleset_schema.json index 6321f089..deab980c 100644 --- a/ansible_rulebook/schema/ruleset_schema.json +++ b/ansible_rulebook/schema/ruleset_schema.json @@ -453,6 +453,10 @@ }, "delay": { "type": "integer" + }, + "include_events": { + "type": "boolean", + "default": true } }, "required": [ @@ -502,6 +506,10 @@ }, "delay": { "type": "integer" + }, + "include_events": { + "type": "boolean", + "default": true } }, "required": [ diff --git a/docs/actions.rst b/docs/actions.rst index 9480d35d..8f8a24bf 100644 --- a/docs/actions.rst +++ b/docs/actions.rst @@ -147,6 +147,9 @@ Run a job template. * - organization - The name of the organization - Yes + * - include_events + - Should we include the matching events in the payload sent to controller. Default is true + - No * - set_facts - The artifacts from the job template execution are inserted back into the rule set as facts - No @@ -203,6 +206,9 @@ Run a workflow template. * - organization - The name of the organization - Yes + * - include_events + - Should we include the matching events in the payload sent to controller. Default is true. If your workflow template does not have Prompt on Launch for Extra Variables or a Survey spec, you will have to set this to false. + - No * - set_facts - The artifacts from the workflow template execution are inserted back into the rule set as facts - No diff --git a/sonar-project.properties b/sonar-project.properties new file mode 100644 index 00000000..4b83e0fe --- /dev/null +++ b/sonar-project.properties @@ -0,0 +1,29 @@ +# Complete documentation with many more options at: +# https://docs.sonarqube.org/latest/analysis/analysis-parameters/ + +## The unique project identifier. This is mandatory. +# Do not duplicate or reuse! +# Available characters: [a-zA-Z0-9_:\.\-] +# Must have least one non-digit. +# Recommended format: : +sonar.projectKey=ansible_ansible-rulebook + +sonar.organization=ansible + +# Customize what paths to scan. Default is . +sonar.sources=. + +# Verbose name of project displayed in WUI. Default is set to the projectKey. This field is optional. +sonar.projectName=ansible-rulebook + +sonar.issue.ignore.multicriteria=e1 +# Ignore "should be a variable" +#sonar.issue.ignore.multicriteria.e1.ruleKey=python:S1192 +sonar.issue.ignore.multicriteria.e1.resourceKey=**/action/* + +# Only scan with python3 +sonar.python.version=3.9,3.10,3.11,3.12 + +# Ignore code dupe for premature code reuse recommened in PR +# https://github.com/ansible/ansible-rulebook/pull/537#issuecomment-1598883529 +sonar.cpd.exclusions=**/action/* diff --git a/tests/examples/84_job_template_exclude_events.yml b/tests/examples/84_job_template_exclude_events.yml new file mode 100644 index 00000000..155ce76e --- /dev/null +++ b/tests/examples/84_job_template_exclude_events.yml @@ -0,0 +1,20 @@ +--- +- name: Test run job templates without event payload + hosts: all + sources: + - ansible.eda.generic: + payload: + - age: 55 + name: Fred + zip: 12345 + rules: + - name: "Run job template" + condition: event.name == "Fred" + action: + run_job_template: + name: Demo Job Template + organization: Default + include_events: false + job_args: + extra_vars: + name: "{{ event.name }}" diff --git a/tests/examples/85_workflow_template_exclude_events.yml b/tests/examples/85_workflow_template_exclude_events.yml new file mode 100644 index 00000000..cc6cd2d1 --- /dev/null +++ b/tests/examples/85_workflow_template_exclude_events.yml @@ -0,0 +1,20 @@ +--- +- name: Test run workflow templates without event payload + hosts: all + sources: + - ansible.eda.generic: + payload: + - age: 55 + name: Fred + zip: "12345" + rules: + - name: "Run workflow template" + condition: event.name == "Fred" + action: + run_workflow_template: + name: Demo Workflow Template + organization: Default + include_events: false + job_args: + extra_vars: + name: "{{ event.name }}" diff --git a/tests/examples/86_job_template_include_events.yml b/tests/examples/86_job_template_include_events.yml new file mode 100644 index 00000000..39b08b6b --- /dev/null +++ b/tests/examples/86_job_template_include_events.yml @@ -0,0 +1,19 @@ +--- +- name: Test run job templates with event payload + hosts: all + sources: + - ansible.eda.generic: + payload: + - age: 55 + name: Fred + zip: 12345 + rules: + - name: "Run job template" + condition: event.name == "Fred" + action: + run_job_template: + name: Demo Job Template + organization: Default + job_args: + extra_vars: + name: "{{ event.name }}" diff --git a/tests/examples/87_workflow_template_include_events.yml b/tests/examples/87_workflow_template_include_events.yml new file mode 100644 index 00000000..20153b03 --- /dev/null +++ b/tests/examples/87_workflow_template_include_events.yml @@ -0,0 +1,19 @@ +--- +- name: Test run workflow templates with event payload + hosts: all + sources: + - ansible.eda.generic: + payload: + - age: 55 + name: Fred + zip: "12345" + rules: + - name: "Run workflow template" + condition: event.name == "Fred" + action: + run_workflow_template: + name: Demo Workflow Template + organization: Default + job_args: + extra_vars: + name: "{{ event.name }}" diff --git a/tests/examples/88_job_template_no_args.yml b/tests/examples/88_job_template_no_args.yml new file mode 100644 index 00000000..a335f948 --- /dev/null +++ b/tests/examples/88_job_template_no_args.yml @@ -0,0 +1,16 @@ +--- +- name: Test run job templates with no args + hosts: all + sources: + - ansible.eda.generic: + payload: + - age: 55 + name: Fred + zip: 12345 + rules: + - name: "Run job template" + condition: event.name == "Fred" + action: + run_job_template: + name: Demo Job Template + organization: Default diff --git a/tests/test_examples.py b/tests/test_examples.py index 88d6822a..692e6cd1 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -2434,3 +2434,97 @@ async def test_83_boolean_true(): ], } await validate_events(event_log, **checks) + + +INCLUDE_EVENTS_RUN = [ + ( + ( + "ansible_rulebook.action.run_job_template." + "job_template_runner.run_job_template" + ), + "examples/84_job_template_exclude_events.yml", + "run_job_template", + "https://examples.com/#/jobs/945/details", + ["name"], + ), + ( + ( + "ansible_rulebook.action.run_workflow_template." + "job_template_runner.run_workflow_job_template" + ), + "examples/85_workflow_template_exclude_events.yml", + "run_workflow_template", + "https://examples.com/#/jobs/workflow/945/details", + ["name"], + ), + ( + ( + "ansible_rulebook.action.run_job_template." + "job_template_runner.run_job_template" + ), + "examples/86_job_template_include_events.yml", + "run_job_template", + "https://examples.com/#/jobs/945/details", + ["name", "ansible_eda"], + ), + ( + ( + "ansible_rulebook.action.run_workflow_template." + "job_template_runner.run_workflow_job_template" + ), + "examples/87_workflow_template_include_events.yml", + "run_workflow_template", + "https://examples.com/#/jobs/workflow/945/details", + ["name", "ansible_eda"], + ), + ( + ( + "ansible_rulebook.action.run_job_template." + "job_template_runner.run_job_template" + ), + "examples/88_job_template_no_args.yml", + "run_job_template", + "https://examples.com/#/jobs/945/details", + ["ansible_eda"], + ), +] + + +@pytest.mark.asyncio +@pytest.mark.parametrize( + "location,rulebook,action_type,job_url,expected_keys", INCLUDE_EVENTS_RUN +) +async def test_include_events( + location, rulebook, action_type, job_url, expected_keys +): + ruleset_queues, event_log = load_rulebook(rulebook) + + queue = ruleset_queues[0][1] + rs = ruleset_queues[0][0] + response_obj = dict( + status="successful", id=945, created="dummy", artifacts=dict(a=1) + ) + job_template_runner.host = "https://examples.com" + with SourceTask(rs.sources[0], "sources", {}, queue): + with patch( + location, + return_value=response_obj, + ) as mocked_obj: + await run_rulesets( + event_log, + ruleset_queues, + dict(), + "playbooks/inventory.yml", + ) + + while not event_log.empty(): + event = event_log.get_nowait() + if event["type"] == "Action": + action = event + + assert action["url"] == job_url + assert action["action"] == action_type + assert ( + list(mocked_obj.call_args.args[2]["extra_vars"].keys()) + == expected_keys + ) From 2dfa6f1218327d255876a3ea8b8a4a56dba5ac2c Mon Sep 17 00:00:00 2001 From: Alex Date: Wed, 13 Nov 2024 17:01:25 +0100 Subject: [PATCH 2/8] chore: pin major version of dependencies (#721) Signed-off-by: Alex --- setup.cfg | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/setup.cfg b/setup.cfg index 0db8c376..6277542a 100644 --- a/setup.cfg +++ b/setup.cfg @@ -23,19 +23,19 @@ include_package_data = True packages = find: python_requires = >=3.9 install_requires = - aiohttp - pyparsing >= 3.0 - jsonschema - jinja2 - dpath >= 2.1.4 - janus - ansible-runner - websockets + aiohttp >=3,<4 + pyparsing >= 3.0,<4 + jsonschema >=4,<5 + jinja2 >=3,<4 + dpath >= 2.1.4,<3 + janus >=1,<2 + ansible-runner >=2,<3 + websockets >=10,<14 drools_jpy == 0.3.9 - watchdog - xxhash - pyyaml - psycopg[binary] + watchdog >=3,<7 + xxhash >=3,<4 + pyyaml >=6,<7 + psycopg[binary] >=3,<4 [options.packages.find] include = @@ -61,6 +61,6 @@ extend-ignore = [options.extras_require] production = - psycopg[c] + psycopg[c] >=3,<4 development = - psycopg[binary] + psycopg[binary] >=3,<4 From 1c0b1c87ca9ef7ce6e870d4842737406e07f7be9 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 15 Nov 2024 15:00:38 +0100 Subject: [PATCH 3/8] chore: pin minor version of aiohttp (#723) Yesterday, 3.11 version of `aiohttp` was released with [breaking changes ](https://github.com/aio-libs/aiohttp/releases/tag/v3.11.0 ) which breaks `aioresponses` library used for our mocks and as consequence some of our tests are failing. Ref: https://github.com/pnuckowski/aioresponses/pull/262 Regardless of whether this is fixed in aioresponses, aiohttp can introduce breaking changes in minor versions, so I pin it to the latest tested version as this library is critical for the stability in some of our features. Signed-off-by: Alex --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 6277542a..13dcb054 100644 --- a/setup.cfg +++ b/setup.cfg @@ -23,7 +23,7 @@ include_package_data = True packages = find: python_requires = >=3.9 install_requires = - aiohttp >=3,<4 + aiohttp >=3,<3.11 pyparsing >= 3.0,<4 jsonschema >=4,<5 jinja2 >=3,<4 From 89524b7f581f203bc4de2bcde004affbe8c137b5 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 22 Nov 2024 14:55:36 +0100 Subject: [PATCH 4/8] chore: add support for python 3.12 (#713) Adds support for python 3.12. Also updates a test with a race condition in 3.12 --------- Signed-off-by: Alex --- .github/workflows/ci.yml | 1 + .github/workflows/scheduled.yml | 1 + setup.cfg | 1 + tests/e2e/files/rulebooks/test_hot_reload.yml | 3 ++- tox.ini | 3 ++- 5 files changed, 7 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8972b4d8..83a65a7a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -57,6 +57,7 @@ jobs: - "3.9" - "3.10" - "3.11" + - "3.12" steps: - name: Checkout diff --git a/.github/workflows/scheduled.yml b/.github/workflows/scheduled.yml index 0c71e62b..4565d7fe 100644 --- a/.github/workflows/scheduled.yml +++ b/.github/workflows/scheduled.yml @@ -45,6 +45,7 @@ jobs: - "3.9" - "3.10" - "3.11" + - "3.12" steps: - name: Checkout diff --git a/setup.cfg b/setup.cfg index 13dcb054..c0b3a1cd 100644 --- a/setup.cfg +++ b/setup.cfg @@ -16,6 +16,7 @@ classifiers = Programming Language :: Python :: 3.9 Programming Language :: Python :: 3.10 Programming Language :: Python :: 3.11 + Programming Language :: Python :: 3.12 [options] zip_safe = False diff --git a/tests/e2e/files/rulebooks/test_hot_reload.yml b/tests/e2e/files/rulebooks/test_hot_reload.yml index 42ec2f8a..60b72a86 100644 --- a/tests/e2e/files/rulebooks/test_hot_reload.yml +++ b/tests/e2e/files/rulebooks/test_hot_reload.yml @@ -5,6 +5,7 @@ - generic: payload: - action: "value_a" + shutdown_after: 2 rules: - name: Matching for value_a condition: event.action == "value_a" @@ -15,4 +16,4 @@ condition: event.action == "value_b" action: debug: - msg: "Rule 2: I have now matched for value_b" \ No newline at end of file + msg: "Rule 2: I have now matched for value_b" diff --git a/tox.ini b/tox.ini index b6e0791b..85f4150a 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = flake8,py39,py310,py311 +envlist = flake8,py39,py310,py311,py312 isolated_build = True [travis] @@ -7,6 +7,7 @@ python = 3.9: py39 3.10: py310 3.11: py311 + 3.12: py312 [testenv:flake8] basepython = python From 44618f0afba51fd4892a980b66e61d0e080db2be Mon Sep 17 00:00:00 2001 From: Madhu Kanoor Date: Fri, 22 Nov 2024 10:08:38 -0500 Subject: [PATCH 5/8] feat: support receiving file contents and env vars from EDA Server (#711) AAP Credential Types support File contents via file injectors. https://docs.ansible.com/automation-controller/latest/html/userguide/credential_types.html We create temporary files with the data sent from Server The source plugin can access the temporary file names to access the data. Supports single file as well as multiple files. To access the filenames in the source rulebook you can use the following extra vars - eda.filename or - eda.filename.<> - eda.filename.<> e.g. ``` - name: Use payload file to check events hosts: all sources: - ansible.eda.generic: payload_file: "{{ eda.filename.test_payload_file }}" ``` The env variables are treated like extra_vars and added to the current processes environment https://issues.redhat.com/browse/AAP-25519 --- CHANGELOG.md | 1 + ansible_rulebook/app.py | 12 ++++++- ansible_rulebook/common.py | 1 + ansible_rulebook/websocket.py | 31 ++++++++++++++++ tests/data/test_cert.pem | 1 + tests/data/test_env.yml | 3 ++ tests/data/test_key.pem | 1 + tests/test_websocket.py | 66 +++++++++++++++++++++++++++++------ 8 files changed, 105 insertions(+), 11 deletions(-) create mode 100644 tests/data/test_cert.pem create mode 100644 tests/data/test_env.yml create mode 100644 tests/data/test_key.pem diff --git a/CHANGELOG.md b/CHANGELOG.md index ab032e0b..b3206316 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ ### Added ### Fixed - Allow user to optionally include matching events +- Allow for fetching env and file contents from EDA server ## [1.1.1] - 2024-09-19 diff --git a/ansible_rulebook/app.py b/ansible_rulebook/app.py index 1971af06..e32fd801 100644 --- a/ansible_rulebook/app.py +++ b/ansible_rulebook/app.py @@ -32,7 +32,11 @@ from ansible_rulebook.engine import run_rulesets, start_source from ansible_rulebook.job_template_runner import job_template_runner from ansible_rulebook.rule_types import RuleSet, RuleSetQueue -from ansible_rulebook.util import decryptable +from ansible_rulebook.util import ( + decryptable, + decrypted_context, + substitute_variables, +) from ansible_rulebook.validators import Validate from ansible_rulebook.vault import has_vaulted_str from ansible_rulebook.websocket import ( @@ -74,6 +78,12 @@ async def run(parsed_args: argparse.Namespace) -> None: raise WebSocketExchangeException( "Error communicating with web socket server" ) + context = decrypted_context(startup_args.variables) + startup_args.env_vars = substitute_variables( + startup_args.env_vars, context + ) + for k, v in startup_args.env_vars.items(): + os.environ[k] = str(v) else: startup_args = StartupArgs() startup_args.variables = load_vars(parsed_args) diff --git a/ansible_rulebook/common.py b/ansible_rulebook/common.py index 22d5fbbd..c4f5cafa 100644 --- a/ansible_rulebook/common.py +++ b/ansible_rulebook/common.py @@ -31,3 +31,4 @@ class StartupArgs: inventory: str = field(default="") check_controller_connection: bool = field(default=False) check_vault: bool = field(default=True) + env_vars: Dict = field(default_factory=dict) diff --git a/ansible_rulebook/websocket.py b/ansible_rulebook/websocket.py index 568102cf..ce9a42ec 100644 --- a/ansible_rulebook/websocket.py +++ b/ansible_rulebook/websocket.py @@ -180,6 +180,8 @@ async def _handle_request_workload( project_data_fh = None response = StartupArgs() + non_fq_key = False + file_template_vars = {} while True: msg = await websocket.recv() data = json.loads(msg) @@ -199,6 +201,21 @@ async def _handle_request_workload( if not data.get("data") and not data.get("more"): os.close(project_data_fh) logger.debug("wrote %s", response.project_data_file) + if data.get("type") == "FileContents": + template_key = data.get("template_key") + raw_data = base64.b64decode(data.get("data")) + keys = template_key.split(".") + if len(keys) == 1 and template_key == "template": + key = "filename" + non_fq_key = True + else: + key = keys[1] + filename = tempfile.NamedTemporaryFile().name + with open(filename, "wb") as f: + f.write(raw_data) + file_template_vars[key] = filename + os.chmod(filename, 0o400) + logger.debug(f"File Content eda.filename.{key} : {filename}") if data.get("type") == "Rulebook": raw_data = base64.b64decode(data.get("data")) response.check_vault = has_vaulted_str(raw_data) @@ -209,12 +226,26 @@ async def _handle_request_workload( response.variables = yaml.safe_load( base64.b64decode(data.get("data")) ) + if data.get("type") == "EnvVars": + response.env_vars = yaml.safe_load( + base64.b64decode(data.get("data")) + ) if data.get("type") == "ControllerInfo": response.controller_url = data.get("url") response.controller_token = data.get("token") response.controller_ssl_verify = data.get("ssl_verify") response.controller_username = data.get("username", "") response.controller_password = data.get("password", "") + + if non_fq_key and "filename" in file_template_vars: + response.variables["eda"] = { + "filename": file_template_vars["filename"] + } + else: + response.variables["eda"] = {"filename": file_template_vars} + + for key, value in response.env_vars.items(): + response.variables[key] = value return response diff --git a/tests/data/test_cert.pem b/tests/data/test_cert.pem new file mode 100644 index 00000000..600b018a --- /dev/null +++ b/tests/data/test_cert.pem @@ -0,0 +1 @@ +This is a bogus certfile diff --git a/tests/data/test_env.yml b/tests/data/test_env.yml new file mode 100644 index 00000000..f7e3f2a7 --- /dev/null +++ b/tests/data/test_env.yml @@ -0,0 +1,3 @@ +--- + ENV1: abc + ENV2: xyz diff --git a/tests/data/test_key.pem b/tests/data/test_key.pem new file mode 100644 index 00000000..13762cb4 --- /dev/null +++ b/tests/data/test_key.pem @@ -0,0 +1 @@ +This is a bogus keyfile diff --git a/tests/test_websocket.py b/tests/test_websocket.py index 7b80ec4c..baf93af6 100644 --- a/tests/test_websocket.py +++ b/tests/test_websocket.py @@ -3,7 +3,7 @@ import hashlib import json import os -from typing import Dict, List +from typing import Dict, List, Optional from unittest import mock from unittest.mock import AsyncMock, patch @@ -42,17 +42,17 @@ def load_file( data_list: List, whole_file=False, block_size: int = 1024, + additional_attributes: Optional[dict] = None, ) -> None: with open(filename, "rb") as f: if whole_file: - data_list.append( - json.dumps( - { - "type": data_type, - "data": base64.b64encode(f.read()).decode("ascii"), - } - ).encode("utf-8") - ) + payload = { + "type": data_type, + "data": base64.b64encode(f.read()).decode("ascii"), + } + if additional_attributes: + payload.update(additional_attributes) + data_list.append(json.dumps(payload).encode("utf-8")) else: while filedata := f.read(block_size): data_list.append( @@ -70,8 +70,29 @@ def load_file( HERE = os.path.dirname(os.path.abspath(__file__)) +@pytest.mark.parametrize( + ("file_contents", "checks", "single"), + [ + ( + [ + ("./data/test_cert.pem", "template.cert_file"), + ("./data/test_key.pem", "template.key_file"), + ], + [ + ("cert_file", "./data/test_cert.pem"), + ("key_file", "./data/test_key.pem"), + ], + False, + ), + ( + [("./data/test_cert.pem", "template")], + [("filename", "./data/test_cert.pem")], + True, + ), + ], +) @pytest.mark.asyncio -async def test_request_workload(): +async def test_request_workload(file_contents, checks, single): prepare_settings() os.chdir(HERE) controller_url = "https://www.example.com" @@ -94,6 +115,16 @@ async def test_request_workload(): load_file("./rules/rules.yml", "Rulebook", test_data, True) load_file("./playbooks/inventory.yml", "Inventory", test_data, True) load_file("./data/test_vars.yml", "ExtraVars", test_data, True) + load_file("./data/test_env.yml", "EnvVars", test_data, True) + for file_name, var_name in file_contents: + load_file( + file_name, + "FileContents", + test_data, + True, + 1024, + {"template_key": var_name}, + ) test_data.append(dict2json({"type": "EndOfResponse"})) with patch("ansible_rulebook.websocket.websockets.connect") as mo: @@ -108,6 +139,21 @@ async def test_request_workload(): assert response.controller_ssl_verify == controller_ssl_verify assert response.rulesets[0].name == "Demo rules" assert len(response.rulesets[0].rules) == 6 + assert response.env_vars["ENV1"] == "abc" + assert response.env_vars["ENV2"] == "xyz" + assert response.variables["ENV1"] == "abc" + assert response.variables["ENV2"] == "xyz" + for key, filename in checks: + if single: + with open(response.variables["eda"][key]) as f: + data = f.read() + else: + with open(response.variables["eda"]["filename"][key]) as f: + data = f.read() + + with open(filename) as f: + original_data = f.read() + assert data == original_data @pytest.mark.asyncio From ccecb2a9c19bda2b520fced07328070549b3f5cb Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 22 Nov 2024 18:47:59 +0100 Subject: [PATCH 6/8] fix: set correct inventory path when it is a dir (#684) Fixes https://github.com/ansible/ansible-rulebook/issues/681 --------- Signed-off-by: Alex --- ansible_rulebook/action/run_playbook.py | 6 ++-- ansible_rulebook/util.py | 7 +++- .../group_vars/customgroup.yml | 1 + .../inventory_as_dir/host_vars/localhost.yml | 1 + .../inventories/inventory_as_dir/hosts.yml | 4 +++ .../e2e/files/playbooks/print_group_vars.yml | 8 +++++ .../files/rulebooks/test_inventory_as_dir.yml | 14 ++++++++ tests/e2e/test_actions.py | 33 +++++++++++++++++++ 8 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 tests/e2e/files/inventories/inventory_as_dir/group_vars/customgroup.yml create mode 100644 tests/e2e/files/inventories/inventory_as_dir/host_vars/localhost.yml create mode 100644 tests/e2e/files/inventories/inventory_as_dir/hosts.yml create mode 100644 tests/e2e/files/playbooks/print_group_vars.yml create mode 100644 tests/e2e/files/rulebooks/test_inventory_as_dir.yml diff --git a/ansible_rulebook/action/run_playbook.py b/ansible_rulebook/action/run_playbook.py index fbab1c74..73030059 100644 --- a/ansible_rulebook/action/run_playbook.py +++ b/ansible_rulebook/action/run_playbook.py @@ -149,9 +149,8 @@ async def _pre_process(self) -> None: os.mkdir(inventory_dir) if self.helper.control.inventory: - create_inventory(inventory_dir, self.helper.control.inventory) - self.inventory = os.path.join( - inventory_dir, os.path.basename(self.helper.control.inventory) + self.inventory = create_inventory( + inventory_dir, self.helper.control.inventory ) os.mkdir(project_dir) @@ -270,7 +269,6 @@ def _get_latest_artifact(self, component: str, content: bool = True): return files[0] async def _untar_project(self, output_dir, project_data_file): - cmd = [tar, "zxvf", project_data_file] proc = await asyncio.create_subprocess_exec( *cmd, diff --git a/ansible_rulebook/util.py b/ansible_rulebook/util.py index 15df1333..7acfe1cf 100644 --- a/ansible_rulebook/util.py +++ b/ansible_rulebook/util.py @@ -270,17 +270,22 @@ async def send_session_stats(event_log: asyncio.Queue, stats: Dict): ) -def create_inventory(runner_inventory_dir: str, inventory: str) -> None: +def create_inventory(runner_inventory_dir: str, inventory: str) -> str: if os.path.isfile(inventory): shutil.copy(os.path.abspath(inventory), runner_inventory_dir) + inventory_path = os.path.join( + runner_inventory_dir, os.path.basename(inventory) + ) elif os.path.exists(inventory): shutil.copytree( os.path.abspath(inventory), runner_inventory_dir, dirs_exist_ok=True, ) + inventory_path = runner_inventory_dir else: raise InventoryNotFound(f"Inventory {inventory} not found") + return inventory_path def _builtin_filter_path(name: str) -> Tuple[bool, str]: diff --git a/tests/e2e/files/inventories/inventory_as_dir/group_vars/customgroup.yml b/tests/e2e/files/inventories/inventory_as_dir/group_vars/customgroup.yml new file mode 100644 index 00000000..c8e3085f --- /dev/null +++ b/tests/e2e/files/inventories/inventory_as_dir/group_vars/customgroup.yml @@ -0,0 +1 @@ +groupvar: groupvar_value diff --git a/tests/e2e/files/inventories/inventory_as_dir/host_vars/localhost.yml b/tests/e2e/files/inventories/inventory_as_dir/host_vars/localhost.yml new file mode 100644 index 00000000..613c670e --- /dev/null +++ b/tests/e2e/files/inventories/inventory_as_dir/host_vars/localhost.yml @@ -0,0 +1 @@ +hostvar: hostvar_value diff --git a/tests/e2e/files/inventories/inventory_as_dir/hosts.yml b/tests/e2e/files/inventories/inventory_as_dir/hosts.yml new file mode 100644 index 00000000..27265db9 --- /dev/null +++ b/tests/e2e/files/inventories/inventory_as_dir/hosts.yml @@ -0,0 +1,4 @@ +customgroup: + hosts: + localhost: + ansible_connection: local diff --git a/tests/e2e/files/playbooks/print_group_vars.yml b/tests/e2e/files/playbooks/print_group_vars.yml new file mode 100644 index 00000000..3049dbd5 --- /dev/null +++ b/tests/e2e/files/playbooks/print_group_vars.yml @@ -0,0 +1,8 @@ +- hosts: customgroup + gather_facts: false + tasks: + - name: Show vars + ansible.builtin.debug: + msg: + - "groupvar: {{ groupvar }}" + - "hostvar: {{ hostvar }}" diff --git a/tests/e2e/files/rulebooks/test_inventory_as_dir.yml b/tests/e2e/files/rulebooks/test_inventory_as_dir.yml new file mode 100644 index 00000000..c28a8a4a --- /dev/null +++ b/tests/e2e/files/rulebooks/test_inventory_as_dir.yml @@ -0,0 +1,14 @@ +- name: Test inventory as dir + hosts: all + sources: + - ansible.eda.generic: + shutdown_after: 2 + payload: + motto: winter is coming + rules: + - name: Test rule + condition: event.motto == "winter is coming" + action: + run_playbook: + name: ./playbooks/print_group_vars.yml + copy_files: true diff --git a/tests/e2e/test_actions.py b/tests/e2e/test_actions.py index a1f734a3..c7ac44a5 100644 --- a/tests/e2e/test_actions.py +++ b/tests/e2e/test_actions.py @@ -384,3 +384,36 @@ def test_shutdown_action_now(update_environment): assert ( "This condition should not fire" not in result.stdout ), "a post-shutdown condition fired when it should not have" + + +@pytest.mark.e2e +def test_inventory_as_dir(): + """ + Execute a rulebook that contains a run_playbook action with an inventory + directory instead of a file. + """ + + rulebook = utils.BASE_DATA_PATH / "rulebooks/test_inventory_as_dir.yml" + inventory = utils.BASE_DATA_PATH / "inventories/inventory_as_dir" + cmd = utils.Command( + rulebook=rulebook, + inventory=inventory, + ) + + LOGGER.info(f"Running command: {cmd}") + result = subprocess.run( + cmd, + timeout=DEFAULT_CMD_TIMEOUT, + capture_output=True, + cwd=utils.BASE_DATA_PATH, + text=True, + ) + + assert result.returncode == 0 + assert not result.stderr + assert ( + "hostvar_value" in result.stdout + ), "hostvar_value not found in stdout" + assert ( + "groupvar_value" in result.stdout + ), "groupvar_value not found in stdout" From efa9ea68e3e618fac288652fe3f2811bc86a5cd1 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 22 Nov 2024 19:19:03 +0100 Subject: [PATCH 7/8] fix: add generic error msg for unknown source errors (#645) This is a little enhancement when the source plugin fails and raises an exception without message. Also extends the source plugin development guide. Related with https://issues.redhat.com/browse/AAP-19598 --- CHANGELOG.md | 1 + ansible_rulebook/engine.py | 17 ++++++- docs/sources.rst | 3 ++ tests/examples/89_source_error_with_msg.yml | 12 +++++ .../examples/90_source_error_without_msg.yml | 12 +++++ tests/sources/source_with_exception.py | 34 ++++++++++++++ tests/test_examples.py | 44 +++++++++++++++++++ 7 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 tests/examples/89_source_error_with_msg.yml create mode 100644 tests/examples/90_source_error_without_msg.yml create mode 100644 tests/sources/source_with_exception.py diff --git a/CHANGELOG.md b/CHANGELOG.md index b3206316..a3584630 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ ## [Unreleased] ### Changed ### Added +- Add generic error message for unknown source errors ### Fixed - Allow user to optionally include matching events - Allow for fetching env and file contents from EDA server diff --git a/ansible_rulebook/engine.py b/ansible_rulebook/engine.py index 00bff598..18e2586a 100644 --- a/ansible_rulebook/engine.py +++ b/ansible_rulebook/engine.py @@ -201,9 +201,22 @@ async def start_source( ) logger.debug("Task cancelled " + shutdown_msg) except BaseException as e: - logger.error("Source error %s", str(e)) + error_msg = str(e) + # Get the name of the exception class + error_type = str(type(e)).split(".")[-1].replace("'>", "") + if not error_msg: + user_msg = ( + f"Unknown error {error_type}: " + "source plugin failed with no error message." + ) + else: + user_msg = ( + f"{error_type}: Source plugin failed with error message: " + f"'{error_msg}'" + ) + logger.error("Source error: %s", user_msg) shutdown_msg = ( - f"Shutting down source: {source.source_name} error : {e}" + f"Shutting down source: {source.source_name} error: {user_msg}" ) logger.error(shutdown_msg) raise diff --git a/docs/sources.rst b/docs/sources.rst index c6b5ed71..e40b8b7b 100644 --- a/docs/sources.rst +++ b/docs/sources.rst @@ -185,6 +185,9 @@ immediately after the ``put`` method. The rulebook can contain it's own logic to finish the process through the ``shutdown`` action. If your plugin needs to perform some cleanup before the process is terminated, you must catch the ``asyncio.CancelledError`` exception. +.. note:: + Please, pay attention when handling errors in your plugin and ensure to raise an exception with a meaningful message so that ansible-rulebook + can log it correctly. Ansible-rulebook will not log the exception itself or print stack traces; it will only log the message you provide. Distributing plugins ^^^^^^^^^^^^^^^^^^^^ diff --git a/tests/examples/89_source_error_with_msg.yml b/tests/examples/89_source_error_with_msg.yml new file mode 100644 index 00000000..de4d78b7 --- /dev/null +++ b/tests/examples/89_source_error_with_msg.yml @@ -0,0 +1,12 @@ +--- +- name: "89 source error with msg" + hosts: all + sources: + - name: range_fail_with_msg + source_with_exception: + error_msg: "range fail with msg" + rules: + - name: r1 + condition: true + action: + print_event: diff --git a/tests/examples/90_source_error_without_msg.yml b/tests/examples/90_source_error_without_msg.yml new file mode 100644 index 00000000..4183c130 --- /dev/null +++ b/tests/examples/90_source_error_without_msg.yml @@ -0,0 +1,12 @@ +--- +- name: "90 source error without msg" + hosts: all + sources: + - name: range_fail_without_msg + source_with_exception: + error_msg: "" + rules: + - name: r1 + condition: true + action: + print_event: diff --git a/tests/sources/source_with_exception.py b/tests/sources/source_with_exception.py new file mode 100644 index 00000000..96e0380b --- /dev/null +++ b/tests/sources/source_with_exception.py @@ -0,0 +1,34 @@ +# Copyright 2022 Red Hat, Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import asyncio +from typing import Any, Dict + + +class TestException(Exception): + pass + + +async def main(queue: asyncio.Queue, args: Dict[str, Any]): + error_msg = str(args.get("error_msg", "")) + raise TestException(error_msg) + + +if __name__ == "__main__": + + class MockQueue: + async def put(self, event): + print(event) + + asyncio.run(main(MockQueue(), dict(limit=5))) diff --git a/tests/test_examples.py b/tests/test_examples.py index 692e6cd1..a8ce1300 100644 --- a/tests/test_examples.py +++ b/tests/test_examples.py @@ -2528,3 +2528,47 @@ async def test_include_events( list(mocked_obj.call_args.args[2]["extra_vars"].keys()) == expected_keys ) + + +@pytest.mark.asyncio +async def test_89_source_error_with_msg(caplog): + ruleset_queues, event_log = load_rulebook( + "examples/89_source_error_with_msg.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(), + dict(), + ) + expected_msg = ( + "ERROR Source error: TestException: " + "Source plugin failed with error message: 'range fail with msg'" + ) + assert expected_msg in caplog.text + + +@pytest.mark.asyncio +async def test_90_source_error_without_msg(caplog): + ruleset_queues, event_log = load_rulebook( + "examples/90_source_error_without_msg.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(), + dict(), + ) + expected_msg = ( + "ERROR Source error: Unknown error " + "TestException: source plugin failed with no error message." + ) + assert expected_msg in caplog.text From f932f6f6dcd7332a4d6b95473d42adb7e0ff06ec Mon Sep 17 00:00:00 2001 From: Zack Kayyali Date: Mon, 9 Dec 2024 11:44:30 -0500 Subject: [PATCH 8/8] chore: update to version 1.1.2 (#725) Updates ansible-rulebook version to 1.1.2 --- .bumpversion.cfg | 2 +- ansible_rulebook/__init__.py | 2 +- setup.cfg | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.bumpversion.cfg b/.bumpversion.cfg index 11a35b1f..620e041c 100644 --- a/.bumpversion.cfg +++ b/.bumpversion.cfg @@ -1,5 +1,5 @@ [bumpversion] -current_version = 1.1.1 +current_version = 1.1.2 commit = False tag = False search = {current_version} diff --git a/ansible_rulebook/__init__.py b/ansible_rulebook/__init__.py index c8507d37..8b8624c6 100644 --- a/ansible_rulebook/__init__.py +++ b/ansible_rulebook/__init__.py @@ -16,7 +16,7 @@ import yaml -__version__ = "1.1.1" +__version__ = "1.1.2" def construct_vault_encrypted_unicode(loader, node): diff --git a/setup.cfg b/setup.cfg index c0b3a1cd..79cd4123 100644 --- a/setup.cfg +++ b/setup.cfg @@ -1,6 +1,6 @@ [metadata] name = ansible_rulebook -version = 1.1.1 +version = 1.1.2 description = Event driven automation for Ansible url = https://github.com/ansible/ansible-rulebook license = Apache-2.0