From 639653ce5d328479cb37289d222ce87b150d9900 Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Thu, 30 Jul 2026 12:03:09 +0200 Subject: [PATCH 1/5] Narrow ansible ModuleNotFoundError guard to the imports Ticket: ENT-14326 Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Lars Erik Wik --- promise-types/ansible/ansible_promise.py | 292 ++++++++++++----------- 1 file changed, 147 insertions(+), 145 deletions(-) diff --git a/promise-types/ansible/ansible_promise.py b/promise-types/ansible/ansible_promise.py index 610d5ec..6ff1c52 100644 --- a/promise-types/ansible/ansible_promise.py +++ b/promise-types/ansible/ansible_promise.py @@ -1,4 +1,5 @@ import os +import sys from typing import Dict, Tuple, List from cfengine_module_library import PromiseModule, ValidationError, Result @@ -13,164 +14,165 @@ from ansible.plugins.callback import CallbackBase from ansible.vars.manager import VariableManager from ansible.plugins.loader import init_plugin_loader +except ModuleNotFoundError: - class CallbackModule(CallbackBase): - CALLBACK_VERSION = 1.0 - CALLBACK_TYPE = "stdout" - CALLBACK_NAME = "cfengine" - - def __init__(self, *args, promise, **kw): - self.promise = promise - self.hosts = set() - self.changed = False - super(CallbackModule, self).__init__(*args, **kw) - - def v2_runner_on_start(self, host, task): - self.hosts.add(str(host)) - self.promise.log_verbose( - "Task '" + task.name + "' started on '" + str(host) + "'" - ) - - def v2_runner_on_ok(self, result): - is_changed = result.is_changed() - if is_changed: - self.changed = True - self.promise.log_info( - "Task '" + result.task_name + "' successfully changed" - ) - else: - self.promise.log_verbose( - "Task '" + result.task_name + "' didn't change" - ) - - def v2_runner_on_failed(self, result, ignore_errors=False): - self.promise.log_error("Task '" + result.task_name + "' failed") - - def v2_runner_on_skipped(self, result): - self.promise.log_error("Task '" + result.task_name + "' was skipped") + class UnavailableAnsiblePromiseTypeModule(PromiseModule): - def v2_playbook_on_stats(self, stats): - for host in self.hosts: - summary_dict = stats.summarize(host) - summary = " ".join( - "%s=%s" % (k, v) for k, v in summary_dict.items() if v > 0 - ) - if summary_dict.get("unreachable"): - self.promise.log_error("Host '" + host + "' is unreachable") - elif summary: - self.promise.log_verbose( - "Summary of the tasks for '" + host + "' is: " + summary - ) - - class AnsiblePromiseTypeModule(PromiseModule): def __init__(self, **kwargs): - super(AnsiblePromiseTypeModule, self).__init__( + super(UnavailableAnsiblePromiseTypeModule, self).__init__( "ansible_promise_module", "0.0.0", **kwargs ) - def must_be_absolute(v): - if not os.path.isabs(v): - raise ValidationError( - "Must be an absolute path, not '{v}'".format(v=v) - ) - - self.add_attribute( - "playbook", str, default_to_promiser=True, validator=must_be_absolute - ) - self.add_attribute("inventory", str, validator=must_be_absolute) - self.add_attribute("limit", list, default=["localhost"]) - self.add_attribute("tags", list, default=[]) - self.add_attribute("become", bool, default=False) - self.add_attribute("become_method", str, default="sudo") - self.add_attribute("become_user", str, default="root") - self.add_attribute("connection", str, default="local") - self.add_attribute("forks", int, default=1) - self.add_attribute("private_key_file", str, validator=must_be_absolute) - self.add_attribute("remote_user", str, default="root") - - def prepare_promiser_and_attributes(self, promiser, attributes): - safe_promiser = promiser.replace(",", "_") - return (safe_promiser, attributes) - def validate_promise(self, promiser: str, attributes: Dict, metadata: Dict): - return - - def evaluate_promise( - self, promiser: str, attributes: Dict, metadata: Dict - ) -> Tuple[str, List[str]]: - model = self.create_attribute_object(promiser, attributes) - - classes = [] - result = Result.KEPT - - context.CLIARGS = ImmutableDict( - tags=model.tags, - listtags=False, - listtasks=False, - listhosts=False, - syntax=False, - connection=model.connection, - module_path=None, - remote_user=model.remote_user, - private_key_file=model.private_key_file, - ssh_common_args=None, - ssh_extra_args=None, - sftp_extra_args=None, - scp_extra_args=None, - become=model.become, - become_method=model.become_method, - become_user=model.become_user, - forks=model.forks, - verbosity=0, - check=False, - start_at_task=None, - ) + raise ValidationError("Ansible Python module not available") - loader = DataLoader() - inventory = InventoryManager( - loader=loader, - sources=(model.inventory,) if model.inventory else (), - ) + if __name__ == "__main__": + UnavailableAnsiblePromiseTypeModule().start() - variable_manager = VariableManager( - loader=loader, - inventory=inventory, - version_info=CLI.version_info(gitinfo=False), - ) - pbex = PlaybookExecutor( - playbooks=[attributes["playbook"]], - inventory=inventory, - variable_manager=variable_manager, - loader=loader, - passwords={}, + # Nothing below this point can be defined without ansible. + sys.exit(0) + + +class CallbackModule(CallbackBase): + CALLBACK_VERSION = 1.0 + CALLBACK_TYPE = "stdout" + CALLBACK_NAME = "cfengine" + + def __init__(self, *args, promise, **kw): + self.promise = promise + self.hosts = set() + self.changed = False + super(CallbackModule, self).__init__(*args, **kw) + + def v2_runner_on_start(self, host, task): + self.hosts.add(str(host)) + self.promise.log_verbose( + "Task '" + task.name + "' started on '" + str(host) + "'" + ) + + def v2_runner_on_ok(self, result): + is_changed = result.is_changed() + if is_changed: + self.changed = True + self.promise.log_info( + "Task '" + result.task_name + "' successfully changed" ) - callback = CallbackModule(promise=self) - pbex._tqm._stdout_callback = callback # type: ignore + else: + self.promise.log_verbose("Task '" + result.task_name + "' didn't change") - exit_code = pbex.run() - if exit_code != 0: - classes.append("{safe_promiser}_failed".format(safe_promiser=promiser)) - result = Result.NOT_KEPT - elif callback.changed: - result = Result.REPAIRED + def v2_runner_on_failed(self, result, ignore_errors=False): + self.promise.log_error("Task '" + result.task_name + "' failed") - return (result, classes) + def v2_runner_on_skipped(self, result): + self.promise.log_error("Task '" + result.task_name + "' was skipped") - if __name__ == "__main__": - init_plugin_loader() - AnsiblePromiseTypeModule().start() - -except ModuleNotFoundError: - - class UnavailableAnsiblePromiseTypeModule(PromiseModule): - - def __init__(self, **kwargs): - super(UnavailableAnsiblePromiseTypeModule, self).__init__( - "ansible_promise_module", "0.0.0", **kwargs + def v2_playbook_on_stats(self, stats): + for host in self.hosts: + summary_dict = stats.summarize(host) + summary = " ".join( + "%s=%s" % (k, v) for k, v in summary_dict.items() if v > 0 ) + if summary_dict.get("unreachable"): + self.promise.log_error("Host '" + host + "' is unreachable") + elif summary: + self.promise.log_verbose( + "Summary of the tasks for '" + host + "' is: " + summary + ) - def validate_promise(self, promiser: str, attributes: Dict, metadata: Dict): - raise ValidationError("Ansible Python module not available") - if __name__ == "__main__": - UnavailableAnsiblePromiseTypeModule().start() +class AnsiblePromiseTypeModule(PromiseModule): + def __init__(self, **kwargs): + super(AnsiblePromiseTypeModule, self).__init__( + "ansible_promise_module", "0.0.0", **kwargs + ) + + def must_be_absolute(v): + if not os.path.isabs(v): + raise ValidationError("Must be an absolute path, not '{v}'".format(v=v)) + + self.add_attribute( + "playbook", str, default_to_promiser=True, validator=must_be_absolute + ) + self.add_attribute("inventory", str, validator=must_be_absolute) + self.add_attribute("limit", list, default=["localhost"]) + self.add_attribute("tags", list, default=[]) + self.add_attribute("become", bool, default=False) + self.add_attribute("become_method", str, default="sudo") + self.add_attribute("become_user", str, default="root") + self.add_attribute("connection", str, default="local") + self.add_attribute("forks", int, default=1) + self.add_attribute("private_key_file", str, validator=must_be_absolute) + self.add_attribute("remote_user", str, default="root") + + def prepare_promiser_and_attributes(self, promiser, attributes): + safe_promiser = promiser.replace(",", "_") + return (safe_promiser, attributes) + + def validate_promise(self, promiser: str, attributes: Dict, metadata: Dict): + return + + def evaluate_promise( + self, promiser: str, attributes: Dict, metadata: Dict + ) -> Tuple[str, List[str]]: + model = self.create_attribute_object(promiser, attributes) + + classes = [] + result = Result.KEPT + + context.CLIARGS = ImmutableDict( + tags=model.tags, + listtags=False, + listtasks=False, + listhosts=False, + syntax=False, + connection=model.connection, + module_path=None, + remote_user=model.remote_user, + private_key_file=model.private_key_file, + ssh_common_args=None, + ssh_extra_args=None, + sftp_extra_args=None, + scp_extra_args=None, + become=model.become, + become_method=model.become_method, + become_user=model.become_user, + forks=model.forks, + verbosity=0, + check=False, + start_at_task=None, + ) + + loader = DataLoader() + inventory = InventoryManager( + loader=loader, + sources=(model.inventory,) if model.inventory else (), + ) + + variable_manager = VariableManager( + loader=loader, + inventory=inventory, + version_info=CLI.version_info(gitinfo=False), + ) + pbex = PlaybookExecutor( + playbooks=[attributes["playbook"]], + inventory=inventory, + variable_manager=variable_manager, + loader=loader, + passwords={}, + ) + callback = CallbackModule(promise=self) + pbex._tqm._stdout_callback = callback # type: ignore + + exit_code = pbex.run() + if exit_code != 0: + classes.append("{safe_promiser}_failed".format(safe_promiser=promiser)) + result = Result.NOT_KEPT + elif callback.changed: + result = Result.REPAIRED + + return (result, classes) + + +if __name__ == "__main__": + init_plugin_loader() + AnsiblePromiseTypeModule().start() From 434cc94e591e476b5b0003ea348d464fff78f8f3 Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Thu, 30 Jul 2026 12:52:00 +0200 Subject: [PATCH 2/5] Install ansible callback so it survives ansible-core 2.19 TaskQueueManager stopped accepting the stdout callback as an object, so ansible loaded its default one instead and printed the play output onto stdout, corrupting the promise module protocol. Ticket: ENT-14326 Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Lars Erik Wik --- promise-types/ansible/ansible_promise.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/promise-types/ansible/ansible_promise.py b/promise-types/ansible/ansible_promise.py index 6ff1c52..2107b9a 100644 --- a/promise-types/ansible/ansible_promise.py +++ b/promise-types/ansible/ansible_promise.py @@ -161,7 +161,11 @@ def evaluate_promise( passwords={}, ) callback = CallbackModule(promise=self) - pbex._tqm._stdout_callback = callback # type: ignore + if hasattr(callback, "_init_callback_methods"): + # Required on ansible-core >= 2.19 after https://github.com/ansible/ansible/pull/85344 + callback._init_callback_methods() + pbex._tqm._callback_plugins = [callback] + pbex._tqm._callbacks_loaded = True exit_code = pbex.run() if exit_code != 0: From bb63b14bde22ed734a96de152eb0fdd3786ebe14 Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Thu, 30 Jul 2026 13:27:47 +0200 Subject: [PATCH 3/5] Read the ansible playbook attribute from the attribute model It was read from the raw request dict, so the documented "fallback to the promiser" never applied and omitting the attribute raised a KeyError. Ticket: ENT-14326 Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Lars Erik Wik --- promise-types/ansible/ansible_promise.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/promise-types/ansible/ansible_promise.py b/promise-types/ansible/ansible_promise.py index 2107b9a..6f84210 100644 --- a/promise-types/ansible/ansible_promise.py +++ b/promise-types/ansible/ansible_promise.py @@ -154,7 +154,7 @@ def evaluate_promise( version_info=CLI.version_info(gitinfo=False), ) pbex = PlaybookExecutor( - playbooks=[attributes["playbook"]], + playbooks=[model.playbook], inventory=inventory, variable_manager=variable_manager, loader=loader, From b2ebd29d29bb755098751fe221c90d99da6e6703 Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Thu, 30 Jul 2026 13:43:00 +0200 Subject: [PATCH 4/5] Apply the ansible limit attribute It was declared and documented, but never read, so setting it did nothing. For backwards compatibility, default to no limit rather than the documented localhost, which would have restricted every existing promise to the local host. Ticket: ENT-14326 Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Lars Erik Wik --- promise-types/ansible/README.md | 2 +- promise-types/ansible/ansible_promise.py | 6 +++++- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/promise-types/ansible/README.md b/promise-types/ansible/README.md index c6e3cba..c3a375e 100644 --- a/promise-types/ansible/README.md +++ b/promise-types/ansible/README.md @@ -23,7 +23,7 @@ bundle agent main | ------------------ | --------- | ------------------------------------------------------------ | --------- | --------------- | | `playbook` | `string` | Absolute path of the Ansible playbook | No | Promiser | | `inventory` | `string` | Absolute path of the inventory file | No | - | -| `limit` | `slist` | List of host names to target | No | `{"localhost"}` | +| `limit` | `slist` | List of hosts and groups to target | No | `{}` (no limit) | | `tags` | `slist` | List of tags to play | No | `{}` | | `become` | `boolean` | Set the `become` option | No | `False` | | `become_method` | `string` | Set the `become_method` option | No | `"sudo"` | diff --git a/promise-types/ansible/ansible_promise.py b/promise-types/ansible/ansible_promise.py index 6f84210..15b4f7a 100644 --- a/promise-types/ansible/ansible_promise.py +++ b/promise-types/ansible/ansible_promise.py @@ -94,7 +94,7 @@ def must_be_absolute(v): "playbook", str, default_to_promiser=True, validator=must_be_absolute ) self.add_attribute("inventory", str, validator=must_be_absolute) - self.add_attribute("limit", list, default=["localhost"]) + self.add_attribute("limit", list, default=[]) self.add_attribute("tags", list, default=[]) self.add_attribute("become", bool, default=False) self.add_attribute("become_method", str, default="sudo") @@ -147,6 +147,10 @@ def evaluate_promise( loader=loader, sources=(model.inventory,) if model.inventory else (), ) + # An empty subset means no host matches, so leave it alone to target + # everything, the way ansible-playbook does without --limit. + if model.limit: + inventory.subset(model.limit) variable_manager = VariableManager( loader=loader, From 8dbc7e4e8df93f35a96562ac982e0eabd24584e3 Mon Sep 17 00:00:00 2001 From: Lars Erik Wik Date: Thu, 30 Jul 2026 13:45:20 +0200 Subject: [PATCH 5/5] Stop reporting skipped ansible tasks as errors A task is skipped when its condition does not match, which means nothing failed. Log it at the same level as a task that ran without changing anything. Ticket: ENT-14326 Co-Authored-By: Claude Opus 5 (1M context) Signed-off-by: Lars Erik Wik --- promise-types/ansible/ansible_promise.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/promise-types/ansible/ansible_promise.py b/promise-types/ansible/ansible_promise.py index 15b4f7a..5fcd619 100644 --- a/promise-types/ansible/ansible_promise.py +++ b/promise-types/ansible/ansible_promise.py @@ -64,7 +64,7 @@ def v2_runner_on_failed(self, result, ignore_errors=False): self.promise.log_error("Task '" + result.task_name + "' failed") def v2_runner_on_skipped(self, result): - self.promise.log_error("Task '" + result.task_name + "' was skipped") + self.promise.log_verbose("Task '" + result.task_name + "' was skipped") def v2_playbook_on_stats(self, stats): for host in self.hosts: