From e953794cd985516a626512bfb8e8903aac6a2c40 Mon Sep 17 00:00:00 2001 From: Victor Moene Date: Fri, 13 Feb 2026 12:58:53 +0100 Subject: [PATCH 1/4] Added python linting in gh actions Signed-off-by: Victor Moene --- .github/workflows/ci.yml | 13 +++++++++++-- ci/linting.sh | 16 ++++++++++++++++ pyrightconfig.json | 3 +++ 3 files changed, 30 insertions(+), 2 deletions(-) create mode 100755 ci/linting.sh create mode 100644 pyrightconfig.json diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6bf8f7b..f7ce1b9 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1,19 +1,26 @@ name: Continuous Integration on: + push: + branches: [master] pull_request: - types: [opened, reopened] + branches: [master] jobs: unit_tests: name: Run Unit Tests runs-on: ubuntu-24.04 + permissions: + contents: read steps: - uses: actions/checkout@v4 with: submodules: recursive - name: Install dependencies - run: sudo apt-get update -y && sudo apt-get install -y python3 + run: | + sudo apt-get update -y && sudo apt-get install -y python3 + pip install flake8 pyright black pyflakes + if [ -f requirements.txt ]; then pip install -r requirements.txt; fi - name: Install cfbs run: pip install cfbs - name: Check the status with cfbs @@ -22,3 +29,5 @@ jobs: run: cfbs validate - name: Check the formatting run: cfbs --check pretty ./cfbs.json + - name: Linting + run: ./ci/linting.sh diff --git a/ci/linting.sh b/ci/linting.sh new file mode 100755 index 0000000..7db1eed --- /dev/null +++ b/ci/linting.sh @@ -0,0 +1,16 @@ +#!/bin/bash + +set -e + +echo "Running flake8" +flake8 . --ignore=E203,W503,E722,E731 --max-complexity=100 --max-line-length=160 + +echo "Running pyright" +pyright . + +shopt -s globstar +echo "Running black" +black --check . + +echo "Running pyflakes" +pyflakes . diff --git a/pyrightconfig.json b/pyrightconfig.json new file mode 100644 index 0000000..389015c --- /dev/null +++ b/pyrightconfig.json @@ -0,0 +1,3 @@ +{ + "reportMissingImports": "none" +} From 3b00a5dc2b5b537adf14483d0999f94a084e97ad Mon Sep 17 00:00:00 2001 From: Victor Moene Date: Fri, 13 Feb 2026 13:28:41 +0100 Subject: [PATCH 2/4] Fixed flake8 warnings Signed-off-by: Victor Moene --- examples/git-using-lib/git_using_lib.py | 2 +- examples/gpg/gpg.py | 7 +++--- examples/rss/rss.py | 13 +++++++---- examples/site-up/site_up.py | 2 +- libraries/python/cfengine_module_library.py | 2 +- promise-types/git/git.py | 2 +- promise-types/groups/groups.py | 2 +- promise-types/http/http_promise_type.py | 26 ++++++++++----------- promise-types/symlinks/symlinks.py | 2 +- promise-types/systemd/systemd.py | 5 ++-- 10 files changed, 32 insertions(+), 31 deletions(-) diff --git a/examples/git-using-lib/git_using_lib.py b/examples/git-using-lib/git_using_lib.py index 07747b1..96c742b 100644 --- a/examples/git-using-lib/git_using_lib.py +++ b/examples/git-using-lib/git_using_lib.py @@ -10,7 +10,7 @@ def validate_promise(self, promiser, attributes, metadata): if not promiser.startswith("/"): raise ValidationError(f"File path '{promiser}' must be absolute") if "repository" not in attributes: - raise ValidationError(f"Attribute 'repository' is required") + raise ValidationError("Attribute 'repository' is required") def evaluate_promise(self, promiser, attributes, metadata): url = attributes["repository"] diff --git a/examples/gpg/gpg.py b/examples/gpg/gpg.py index 333b992..abe8039 100644 --- a/examples/gpg/gpg.py +++ b/examples/gpg/gpg.py @@ -42,8 +42,7 @@ """ import json -from subprocess import Popen, PIPE -import sys +from subprocess import Popen, PIPE, TimeoutExpired from cfengine_module_library import PromiseModule, ValidationError, Result @@ -109,9 +108,9 @@ def validate_promise(self, promiser, attributes, metadata): raise ValidationError( f"Promiser '{promiser}' for 'gpg_keys' promise must be an absolute path" ) - if not "keylist" in attributes: + if "keylist" not in attributes: raise ValidationError( - f"Required attribute 'keylist' missing for 'gpg_keys' promise" + "Required attribute 'keylist' missing for 'gpg_keys' promise" ) def evaluate_promise(self, promiser, attributes, metadata): diff --git a/examples/rss/rss.py b/examples/rss/rss.py index 5de683f..bc93de1 100755 --- a/examples/rss/rss.py +++ b/examples/rss/rss.py @@ -1,4 +1,7 @@ -import requests, html, re, os, random +import requests +import re +import os +import random import xml.etree.ElementTree as ET from cfengine_module_library import PromiseModule, ValidationError, Result @@ -42,7 +45,7 @@ def validate_promise(self, promiser, attributes, metadata): # check that attribute select has a valid type if type(select) is not str: raise ValidationError( - f"Invalid type for attribute select: expected string" + "Invalid type for attribute select: expected string" ) # check that attribute select has a valid value @@ -159,10 +162,10 @@ def _write_promiser(self, item, promiser): return Result.NOT_KEPT def _is_win_file(self, path): - return re.search(r"^[a-zA-Z]:\\[\\\S|*\S]?.*$", path) != None + return re.search(r"^[a-zA-Z]:\\[\\\S|*\S]?.*$", path) is not None def _is_unix_file(self, path): - return re.search(r"^(/[^/ ]*)+/?$", path) != None + return re.search(r"^(/[^/ ]*)+/?$", path) is not None def _is_url(self, path): return ( @@ -170,7 +173,7 @@ def _is_url(self, path): r"^http[s]?://(?:[a-zA-Z]|[0-9]|[$-_@.&+]|[!*\(\),]|(?:%[0-9a-fA-F][0-9a-fA-F]))+", path, ) - != None + is not None ) diff --git a/examples/site-up/site_up.py b/examples/site-up/site_up.py index ec7775d..dc05e81 100644 --- a/examples/site-up/site_up.py +++ b/examples/site-up/site_up.py @@ -31,7 +31,7 @@ def evaluate_promise(self, promiser, attributes, metadata): error = None try: - code = urllib.request.urlopen(url, context=ssl_ctx).getcode() + urllib.request.urlopen(url, context=ssl_ctx).getcode() self.log_verbose(f"Site '{url}' is UP!") return Result.KEPT except urllib.error.HTTPError as e: diff --git a/libraries/python/cfengine_module_library.py b/libraries/python/cfengine_module_library.py index 3e8ddb4..148a974 100644 --- a/libraries/python/cfengine_module_library.py +++ b/libraries/python/cfengine_module_library.py @@ -404,7 +404,7 @@ def _handle_evaluate(self, promiser, attributes, request): assert results is not None # Most likely someone forgot to return something # evaluate_promise should return either a result or a (result, result_classes) pair - if type(results) == str: + if isinstance(results, str): self._result = results else: assert len(results) == 2 diff --git a/promise-types/git/git.py b/promise-types/git/git.py index 3f5a077..3e209d6 100644 --- a/promise-types/git/git.py +++ b/promise-types/git/git.py @@ -261,7 +261,7 @@ def _git_envvars(self, model: object): env["GIT_SSH_COMMAND"] = model.ssh_executable if model.ssh_options: env["GIT_SSH_COMMAND"] += " " + model.ssh_options - if not "HOME" in env: + if "HOME" not in env: # git should have a HOME env var to retrieve .gitconfig, .git-credentials, etc env["HOME"] = str(Path.home()) return env diff --git a/promise-types/groups/groups.py b/promise-types/groups/groups.py index 7d6bd99..5b2b556 100644 --- a/promise-types/groups/groups.py +++ b/promise-types/groups/groups.py @@ -64,7 +64,7 @@ def validate_promise(self, promiser, attributes, metadata): ) # check attribute gid value - if type(gid) == str: + if isinstance(gid, str): try: int(gid) except ValueError: diff --git a/promise-types/http/http_promise_type.py b/promise-types/http/http_promise_type.py index fe71dce..acd1bab 100644 --- a/promise-types/http/http_promise_type.py +++ b/promise-types/http/http_promise_type.py @@ -27,14 +27,14 @@ def __init__(self, name="http_promise_module", version="0.0.0", **kwargs): def validate_promise(self, promiser, attributes, metadata): if "url" in attributes: url = attributes["url"] - if type(url) != str: + if not isinstance(url, str): raise ValidationError("'url' must be a string") if not url.startswith(("https://", "http://")): raise ValidationError("Only HTTP(S) requests are supported") if "method" in attributes: method = attributes["method"] - if type(method) != str: + if not isinstance(method, str): raise ValidationError("'method' must be a string") if method not in _SUPPORTED_METHODS: raise ValidationError( @@ -44,18 +44,18 @@ def validate_promise(self, promiser, attributes, metadata): if "headers" in attributes: headers = attributes["headers"] headers_type = type(headers) - if headers_type == str: + if headers_type is str: headers_lines = headers.splitlines() if any(line.count(":") != 1 for line in headers_lines): raise ValidationError( "'headers' must be string with 'name: value' pairs on separate lines" ) - elif headers_type == list: + elif headers_type is list: if any(line.count(":") != 1 for line in headers): raise ValidationError( "'headers' must be a list of 'name: value' pairs" ) - elif headers_type == dict: + elif headers_type is dict: # nothing to check for dict? pass else: @@ -72,7 +72,7 @@ def validate_promise(self, promiser, attributes, metadata): ) if ( - type(payload) == str + isinstance(payload, str) and payload.startswith("@") and not os.path.isabs(payload[1:]) ): @@ -80,12 +80,12 @@ def validate_promise(self, promiser, attributes, metadata): if "file" in attributes: file_ = attributes["file"] - if type(file_) != str or not os.path.isabs(file_): + if not isinstance(file_, str) or not os.path.isabs(file_): raise ValidationError("'file' must be an absolute path to a file") if "insecure" in attributes: insecure = attributes["insecure"] - if type(insecure) != str or insecure not in ( + if not isinstance(insecure, str) or insecure not in ( "true", "True", "false", @@ -125,19 +125,19 @@ def evaluate_promise(self, promiser, attributes, metadata): str.maketrans({char: "_" for char in ("@", "/", ":", "?", "&", "%")}) ) - if headers and type(headers) != dict: - if type(headers) == str: + if headers and not isinstance(headers, dict): + if isinstance(headers, str): headers = { key: value for key, value in (line.split(":") for line in headers.splitlines()) } - elif type(headers) == list: + elif isinstance(headers, list): headers = { key: value for key, value in (line.split(":") for line in headers) } if payload: - if type(payload) == dict: + if isinstance(payload, dict): try: payload = json.dumps(payload) except TypeError: @@ -182,7 +182,7 @@ def evaluate_promise(self, promiser, attributes, metadata): headers["Content-Length"] = os.path.getsize(path) # must be 'None' or bytes or file object - if type(payload) == str: + if isinstance(payload, str): payload = payload.encode("utf-8") request = urllib.request.Request( diff --git a/promise-types/symlinks/symlinks.py b/promise-types/symlinks/symlinks.py index 1b05864..9dfb262 100644 --- a/promise-types/symlinks/symlinks.py +++ b/promise-types/symlinks/symlinks.py @@ -66,7 +66,7 @@ def evaluate_promise(self, promiser, attributes, metadata): "'{}' is already unlinked from its old target".format(promiser) ) return Result.NOT_KEPT - except Exception: + except Exception as e: self.log_error( "'{}' has wrong target but couldn't be unlinked: {}".format( promiser, e diff --git a/promise-types/systemd/systemd.py b/promise-types/systemd/systemd.py index 7b2821a..1f57ce3 100644 --- a/promise-types/systemd/systemd.py +++ b/promise-types/systemd/systemd.py @@ -1,11 +1,10 @@ -import json import os import subprocess from enum import Enum from typing import Dict, List, Optional, Tuple -from cfengine_module_library import PromiseModule, ValidationError, Result +from cfengine_module_library import PromiseModule, Result SYSTEMD_LIB_PATH = "/lib/systemd/system" @@ -518,7 +517,7 @@ def _render_service_template(self, model: object) -> str: value = getattr(model, attr) if value is None: continue - elif type(value) == list: + elif isinstance(value, list): for item in value: blocks[block].append("{key}={item}".format(key=key, item=item)) else: From 74be3e574802d0e01bb228ea61b3fbdce21cbe59 Mon Sep 17 00:00:00 2001 From: Victor Moene Date: Tue, 17 Feb 2026 15:17:13 +0100 Subject: [PATCH 3/4] Fixed pyright errors Signed-off-by: Victor Moene --- libraries/python/cfengine_module_library.py | 2 + promise-types/ansible/ansible_promise.py | 12 ++--- promise-types/git/git.py | 25 +++++++--- promise-types/http/http_promise_type.py | 10 ++-- promise-types/iptables/iptables.py | 3 +- promise-types/systemd/systemd.py | 54 +++++++++++++-------- 6 files changed, 67 insertions(+), 39 deletions(-) diff --git a/libraries/python/cfengine_module_library.py b/libraries/python/cfengine_module_library.py index 148a974..da6a28e 100644 --- a/libraries/python/cfengine_module_library.py +++ b/libraries/python/cfengine_module_library.py @@ -226,6 +226,8 @@ def _handle_request(self, request): "debug", ] + promiser = None + attributes = {} if operation in ["validate_promise", "evaluate_promise"]: promiser = request["promiser"] attributes = request.get("attributes", {}) diff --git a/promise-types/ansible/ansible_promise.py b/promise-types/ansible/ansible_promise.py index 5f9e39a..b51363b 100644 --- a/promise-types/ansible/ansible_promise.py +++ b/promise-types/ansible/ansible_promise.py @@ -5,7 +5,7 @@ from cfengine_module_library import PromiseModule, ValidationError, Result try: - from ansible import context + import ansible.context as context from ansible.cli import CLI from ansible.executor.playbook_executor import PlaybookExecutor from ansible.inventory.manager import InventoryManager @@ -27,7 +27,7 @@ class CallbackModule(CallbackBase): CALLBACK_TYPE = "stdout" CALLBACK_NAME = "cfengine" - def __init__(self, *args, promise=None, **kw): + def __init__(self, *args, promise, **kw): self.promise = promise self.hosts = set() self.changed = False @@ -51,7 +51,7 @@ def v2_runner_on_ok(self, result): "Task '" + result.task_name + "' didn't change" ) - def v2_runner_on_failed(self, result, **_): + 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): @@ -65,8 +65,8 @@ def v2_playbook_on_stats(self, stats): ) if summary_dict.get("unreachable"): self.promise.log_error("Host '" + host + "' is unreachable") - else: - summary and self.promise.log_verbose( + elif summary: + self.promise.log_verbose( "Summary of the tasks for '" + host + "' is: " + summary ) @@ -153,7 +153,7 @@ def evaluate_promise( passwords={}, ) callback = CallbackModule(promise=self) - pbex._tqm._stdout_callback = callback + pbex._tqm._stdout_callback = callback # type: ignore exit_code = pbex.run() if exit_code != 0: diff --git a/promise-types/git/git.py b/promise-types/git/git.py index 3e209d6..9ed4cc5 100644 --- a/promise-types/git/git.py +++ b/promise-types/git/git.py @@ -4,7 +4,12 @@ from typing import Dict, List, Optional -from cfengine_module_library import PromiseModule, ValidationError, Result +from cfengine_module_library import ( + PromiseModule, + ValidationError, + Result, + AttributeObject, +) class GitPromiseTypeModule(PromiseModule): @@ -93,7 +98,8 @@ def evaluate_promise(self, promiser: str, attributes: Dict, metadata: Dict): result = Result.REPAIRED except subprocess.CalledProcessError as e: self.log_error("Failed clone: {error}".format(error=e.output or e)) - e.stderr and self.log_error(e.stderr.strip()) + if e.stderr: + self.log_error(e.stderr.strip()) return ( Result.NOT_KEPT, [ @@ -134,7 +140,8 @@ def evaluate_promise(self, promiser: str, attributes: Dict, metadata: Dict): result = Result.REPAIRED except subprocess.CalledProcessError as e: self.log_error("Failed reset: {error}".format(error=e.output or e)) - e.stderr and self.log_error(e.stderr.strip()) + if e.stderr: + self.log_error(e.stderr.strip()) return ( Result.NOT_KEPT, [ @@ -228,7 +235,8 @@ def evaluate_promise(self, promiser: str, attributes: Dict, metadata: Dict): ) except subprocess.CalledProcessError as e: self.log_error("Failed fetch: {error}".format(error=e.output or e)) - e.stderr and self.log_error(e.stderr.strip()) + if e.stderr: + self.log_error(e.stderr.strip()) return ( Result.NOT_KEPT, [ @@ -241,7 +249,9 @@ def evaluate_promise(self, promiser: str, attributes: Dict, metadata: Dict): # everything okay return (result, classes) - def _git(self, model: object, args: List[str], cwd: Optional[str] = None) -> str: + def _git( + self, model: AttributeObject, args: List[str], cwd: Optional[str] = None + ) -> str: self.log_verbose("Run: {cmd}".format(cmd=" ".join(args))) output = ( subprocess.check_output( @@ -253,10 +263,11 @@ def _git(self, model: object, args: List[str], cwd: Optional[str] = None) -> str .strip() .decode("utf-8") ) - output != "" and self.log_verbose(output) + if output != "": + self.log_verbose(output) return output - def _git_envvars(self, model: object): + def _git_envvars(self, model: AttributeObject): env = os.environ.copy() env["GIT_SSH_COMMAND"] = model.ssh_executable if model.ssh_options: diff --git a/promise-types/http/http_promise_type.py b/promise-types/http/http_promise_type.py index acd1bab..aaa7b85 100644 --- a/promise-types/http/http_promise_type.py +++ b/promise-types/http/http_promise_type.py @@ -2,7 +2,7 @@ import filecmp import os -import urllib +import urllib.error import urllib.request import ssl import json @@ -10,7 +10,6 @@ from cfengine_module_library import PromiseModule, ValidationError, Result - _SUPPORTED_METHODS = {"GET", "POST", "PUT", "DELETE", "PATCH"} @@ -179,7 +178,7 @@ def evaluate_promise(self, promiser, attributes, metadata): ) if "Content-Length" not in headers: - headers["Content-Length"] = os.path.getsize(path) + headers["Content-Length"] = str(os.path.getsize(path)) # must be 'None' or bytes or file object if isinstance(payload, str): @@ -194,8 +193,9 @@ def evaluate_promise(self, promiser, attributes, metadata): # convert to a boolean insecure = insecure.lower() == "true" if insecure: - SSL_context = ssl.SSLContext() - SSL_context.verify_method = ssl.CERT_NONE + SSL_context = ssl.create_default_context() + SSL_context.check_hostname = False + SSL_context.verify_mode = ssl.CERT_NONE try: with urllib.request.urlopen(request, context=SSL_context) as url_req: diff --git a/promise-types/iptables/iptables.py b/promise-types/iptables/iptables.py index 8001813..2448922 100644 --- a/promise-types/iptables/iptables.py +++ b/promise-types/iptables/iptables.py @@ -222,7 +222,8 @@ def evaluate_promise(self, promiser: str, attributes: Dict, metadata: Dict): if result == Result.NOT_KEPT: classes.append("{}_{}_failed".format(safe_promiser, command)) elif result in {Result.KEPT, Result.REPAIRED}: - result == Result.REPAIRED and self.log_info(model.log_str) + if result == Result.REPAIRED: + self.log_info(model.log_str) classes.append("{}_{}_successful".format(safe_promiser, command)) else: diff --git a/promise-types/systemd/systemd.py b/promise-types/systemd/systemd.py index 1f57ce3..25f9c7d 100644 --- a/promise-types/systemd/systemd.py +++ b/promise-types/systemd/systemd.py @@ -4,8 +4,7 @@ from enum import Enum from typing import Dict, List, Optional, Tuple -from cfengine_module_library import PromiseModule, Result - +from cfengine_module_library import PromiseModule, Result, AttributeObject SYSTEMD_LIB_PATH = "/lib/systemd/system" @@ -103,7 +102,8 @@ def evaluate_promise( self.log_error( "Failed to run systemctl: {error}".format(error=e.output or e) ) - e.stderr and self.log_error(e.stderr.strip()) + if e.stderr: + self.log_error(e.stderr.strip()) return ( Result.NOT_KEPT, ["{safe_promiser}_show_failed".format(safe_promiser=safe_promiser)], @@ -115,7 +115,7 @@ def evaluate_promise( return self._service_present(model, safe_promiser, service_status) def _service_absent( - self, model: object, safe_promiser: str, service_status: dict + self, model: AttributeObject, safe_promiser: str, service_status: dict ) -> Tuple[str, List[str]]: classes = [] result = Result.KEPT @@ -130,7 +130,8 @@ def _service_absent( self.log_error( "Failed to run systemctl: {error}".format(error=e.output or e) ) - e.stderr and self.log_error(e.stderr.strip()) + if e.stderr: + self.log_error(e.stderr.strip()) return ( Result.NOT_KEPT, ["{safe_promiser}_stop_failed".format(safe_promiser=safe_promiser)], @@ -146,7 +147,8 @@ def _service_absent( self.log_error( "Failed to run systemctl: {error}".format(error=e.output or e) ) - e.stderr and self.log_error(e.stderr.strip()) + if e.stderr: + self.log_error(e.stderr.strip()) return ( Result.NOT_KEPT, [ @@ -189,7 +191,8 @@ def _service_absent( self.log_error( "Failed to run systemctl: {error}".format(error=e.output or e) ) - e.stderr and self.log_error(e.stderr.strip()) + if e.stderr: + self.log_error(e.stderr.strip()) return ( Result.NOT_KEPT, [ @@ -203,7 +206,7 @@ def _service_absent( return (result, classes) def _service_present( - self, model: object, safe_promiser: str, service_status: dict + self, model: AttributeObject, safe_promiser: str, service_status: dict ) -> Tuple[str, List[str]]: classes = [] result = Result.KEPT @@ -244,7 +247,8 @@ def _service_present( self.log_error( "Failed to run systemctl: {error}".format(error=e.output or e) ) - e.stderr and self.log_error(e.stderr.strip()) + if e.stderr: + self.log_error(e.stderr.strip()) return ( Result.NOT_KEPT, [ @@ -265,7 +269,8 @@ def _service_present( self.log_error( "Failed to run systemctl: {error}".format(error=e.output or e) ) - e.stderr and self.log_error(e.stderr.strip()) + if e.stderr: + self.log_error(e.stderr.strip()) return ( Result.NOT_KEPT, [ @@ -286,7 +291,8 @@ def _service_present( self.log_error( "Failed to run systemctl: {error}".format(error=e.output or e) ) - e.stderr and self.log_error(e.stderr.strip()) + if e.stderr: + self.log_error(e.stderr.strip()) return ( Result.NOT_KEPT, ["{safe_promiser}_mask_failed".format(safe_promiser=safe_promiser)], @@ -303,7 +309,8 @@ def _service_present( self.log_error( "Failed to run systemctl: {error}".format(error=e.output or e) ) - e.stderr and self.log_error(e.stderr.strip()) + if e.stderr: + self.log_error(e.stderr.strip()) return ( Result.NOT_KEPT, [ @@ -330,7 +337,8 @@ def _service_present( self.log_error( "Failed to run systemctl: {error}".format(error=e.output or e) ) - e.stderr and self.log_error(e.stderr.strip()) + if e.stderr: + self.log_error(e.stderr.strip()) return ( Result.NOT_KEPT, [ @@ -357,7 +365,8 @@ def _service_present( self.log_error( "Failed to run systemctl: {error}".format(error=e.output or e) ) - e.stderr and self.log_error(e.stderr.strip()) + if e.stderr: + self.log_error(e.stderr.strip()) return ( Result.NOT_KEPT, [ @@ -383,7 +392,8 @@ def _service_present( self.log_error( "Failed to run systemctl: {error}".format(error=e.output or e) ) - e.stderr and self.log_error(e.stderr.strip()) + if e.stderr: + self.log_error(e.stderr.strip()) return ( Result.NOT_KEPT, [ @@ -410,7 +420,8 @@ def _service_present( self.log_error( "Failed to run systemctl: {error}".format(error=e.output or e) ) - e.stderr and self.log_error(e.stderr.strip()) + if e.stderr: + self.log_error(e.stderr.strip()) return ( Result.NOT_KEPT, ["{safe_promiser}_stop_failed".format(safe_promiser=safe_promiser)], @@ -429,7 +440,8 @@ def _service_present( self.log_error( "Failed to run systemctl: {error}".format(error=e.output or e) ) - e.stderr and self.log_error(e.stderr.strip()) + if e.stderr: + self.log_error(e.stderr.strip()) return ( Result.NOT_KEPT, [ @@ -452,7 +464,8 @@ def _service_present( self.log_error( "Failed to run systemctl: {error}".format(error=e.output or e) ) - e.stderr and self.log_error(e.stderr.strip()) + if e.stderr: + self.log_error(e.stderr.strip()) return ( Result.NOT_KEPT, [ @@ -476,10 +489,11 @@ def _exec_command(self, args: List[str], cwd: Optional[str] = None) -> str: .strip() .decode("utf-8") ) - output != "" and self.log_verbose(output) + if output != "": + self.log_verbose(output) return output - def _render_service_template(self, model: object) -> str: + def _render_service_template(self, model: AttributeObject) -> str: blocks = { "unit": [], "service": [], From 5d054f16c7c3238574ffed4a6bbb6de2a14bdc57 Mon Sep 17 00:00:00 2001 From: Victor Moene Date: Tue, 17 Feb 2026 15:24:54 +0100 Subject: [PATCH 4/4] Fixed ansible pyright error and unknown function error Fixed the way ansible promise type handldes missing ansible dependency to be compatible with pyright. Moreover, Ansible doesn't error anymore on unknown functionn init_plugin_loader() when ansible is not installed. Signed-off-by: Victor Moene --- promise-types/ansible/ansible_promise.py | 214 ++++++++++++----------- 1 file changed, 111 insertions(+), 103 deletions(-) diff --git a/promise-types/ansible/ansible_promise.py b/promise-types/ansible/ansible_promise.py index b51363b..fea29db 100644 --- a/promise-types/ansible/ansible_promise.py +++ b/promise-types/ansible/ansible_promise.py @@ -1,7 +1,6 @@ import os from typing import Dict, Tuple, List - from cfengine_module_library import PromiseModule, ValidationError, Result try: @@ -15,13 +14,6 @@ from ansible.vars.manager import VariableManager from ansible.plugins.loader import init_plugin_loader - ANSIBLE_AVAILABLE = True -except ImportError: - ANSIBLE_AVAILABLE = False - - -if ANSIBLE_AVAILABLE: - class CallbackModule(CallbackBase): CALLBACK_VERSION = 1.0 CALLBACK_TYPE = "stdout" @@ -70,101 +62,117 @@ def v2_playbook_on_stats(self, stats): "Summary of the tasks for '" + host + "' is: " + summary ) + 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, safe_promiser: str, attributes: Dict, metadata: Dict + ) -> Tuple[str, List[str]]: + model = self.create_attribute_object(safe_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=safe_promiser) + ) + result = Result.NOT_KEPT + elif callback.changed: + result = Result.REPAIRED + + return (result, classes) + + 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 + ) -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): - if not ANSIBLE_AVAILABLE: + def validate_promise(self, promiser: str, attributes: Dict, metadata: Dict): raise ValidationError("Ansible Python module not available") - def evaluate_promise( - self, safe_promiser: str, attributes: Dict, metadata: Dict - ) -> Tuple[str, List[str]]: - model = self.create_attribute_object(safe_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=safe_promiser)) - result = Result.NOT_KEPT - elif callback.changed: - result = Result.REPAIRED - - return (result, classes) - - -if __name__ == "__main__": - init_plugin_loader() - AnsiblePromiseTypeModule().start() + if __name__ == "__main__": + UnavailableAnsiblePromiseTypeModule().start()