From fe587f8d1a4aeff8ca778b75bb88c3435648aeb4 Mon Sep 17 00:00:00 2001
From: Sergei Petrosian <spetrosi@redhat.com>
Date: Mon, 3 Aug 2026 15:50:18 +0200
Subject: [PATCH 1/6] feat: Write roles fingerprints to /var/log/sysroles.jsonl

* Extend the sr_fingerprint module to write syslog to
/var/log/sysroles.jsonl in addition to writing them to syslog.

* Add unit test for sr_fingerprint.py

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
---
 inventory/group_vars/active_roles.yml         |   1 +
 playbooks/files/library/sr_fingerprint.py     | 202 ++++++++++++++++--
 .../files/tests/unit/test_sr_fingerprint.py   | 167 +++++++++++++++
 3 files changed, 357 insertions(+), 13 deletions(-)
 create mode 100644 playbooks/files/tests/unit/test_sr_fingerprint.py

diff --git a/inventory/group_vars/active_roles.yml b/inventory/group_vars/active_roles.yml
index 136d761..f94e9a2 100644
--- a/inventory/group_vars/active_roles.yml
+++ b/inventory/group_vars/active_roles.yml
@@ -9,6 +9,7 @@ present_files:
   - .ostree/README.md
   - README-ostree.md
   - library/sr_fingerprint.py
+  - tests/unit/test_sr_fingerprint.yml
 present_templates:
   - .ansible-lint
   - .coderabbit.yaml
diff --git a/playbooks/files/library/sr_fingerprint.py b/playbooks/files/library/sr_fingerprint.py
index 593dd39..6b44b23 100644
--- a/playbooks/files/library/sr_fingerprint.py
+++ b/playbooks/files/library/sr_fingerprint.py
@@ -7,22 +7,78 @@
 DOCUMENTATION = """
 ---
 module: sr_fingerprint
-short_description: Write a message string to syslog using Ansible C(module.log) function.
+short_description: Write role fingerprint data to syslog and optionally to a JSONL log file.
 description:
-    - Writes the given string to the system log using Ansible C(module.log) function.
+    - Collects role fingerprint data into a canonical record and writes it to
+      syslog using Ansible C(module.log) as C(key=value) pairs.
+    - Optionally appends the same record as a JSON line to a log file
+      (one JSON object per line, JSONL format), by default
+      C(/var/log/sysroles.jsonl).
+    - Playbook variables are not available inside modules automatically. Roles
+      pass C(role_name), C(role_path), C(ansible_play_hosts_all), and
+      C(ansible_facts) from the task.
+    - C(ansible_check_mode) is collected from the module execution context.
     - Intended for role-internal or diagnostic use.
 author: Rich Megginson (@richm)
 options:
-    sr_message:
-        description: Text to record in syslog.
+    status:
+        description: Role execution status.
         type: str
         required: true
+        choices:
+            - begin
+            - success
+    write_log_file:
+        description: >-
+            If C(true), append fingerprint data to the JSONL log file.
+            Defaults to C(false).
+        type: bool
+        default: false
+    log_file:
+        description: Path to the JSONL log file.
+        type: path
+        default: /var/log/sysroles.jsonl
+    role_name:
+        description: Name of the role, typically C({{ role_name }}).
+        type: str
+        required: true
+    role_path:
+        description: Path to the role, typically C({{ role_path }}).
+        type: path
+        required: true
+    ansible_play_hosts_all:
+        description: >-
+            All hosts in the play, typically C({{ ansible_play_hosts_all }}).
+            Used to derive C(play_hosts_number).
+        type: list
+        elements: str
+        required: true
+    ansible_facts:
+        description: >-
+            Facts from the playbook for the current managed host, typically
+            C({{ ansible_facts }}).
+        type: dict
+        required: true
 """
 
 EXAMPLES = """
-- name: Record a fingerprint message in syslog
+- name: Record role begin fingerprint to syslog only (not log file)
   sr_fingerprint:
-    sr_message: "system_role:ROLENAME"
+    status: begin
+    role_name: bootloader
+    role_path: "{{ role_path }}"
+    ansible_play_hosts_all: "{{ ansible_play_hosts_all }}"
+    ansible_facts: "{{ ansible_facts }}"
+    write_log_file: false
+
+- name: Record role success fingerprint
+  sr_fingerprint:
+    status: success
+    role_name: bootloader
+    role_path: "{{ role_path }}"
+    ansible_play_hosts_all: "{{ ansible_play_hosts_all }}"
+    ansible_facts: "{{ ansible_facts }}"
+    write_log_file: true
 """
 
 RETURN = r""" # """
@@ -30,6 +86,24 @@
 from ansible.module_utils.basic import AnsibleModule
 
 import datetime
+import errno
+import json
+import os
+
+DEFAULT_LOG_FILE = "/var/log/sysroles.jsonl"
+
+FINGERPRINT_FIELDS = (
+    "date",
+    "role_name",
+    "role_path",
+    "status",
+    "ansible_version",
+    "managed_node_distro",
+    "play_hosts_number",
+    "ansible_check_mode",
+)
+
+FINGERPRINT_SYSLOG_SEPARATOR = " "
 
 
 def _local_iso8601_no_microseconds():
@@ -51,9 +125,98 @@ def _local_iso8601_no_microseconds():
     return datetime.datetime.now(utc).astimezone().replace(microsecond=0).isoformat()
 
 
+def _ensure_parent_dir(path):
+    parent = os.path.dirname(path)
+    if not parent:
+        return
+    if os.path.isdir(parent):
+        return
+    try:
+        os.makedirs(parent)
+    except OSError as exc:
+        if exc.errno != errno.EEXIST or not os.path.isdir(parent):
+            raise
+
+
+def _format_fingerprint_jsonl(record):
+    """Format the canonical fingerprint record as a single JSON line."""
+    return json.dumps(record, separators=(",", ":"), sort_keys=False)
+
+
+def _write_jsonl_log(log_file, record):
+    _ensure_parent_dir(log_file)
+    with open(log_file, "a") as log_fd:
+        log_fd.write(_format_fingerprint_jsonl(record) + "\n")
+
+
+def _get_managed_node_distro(facts):
+    distribution = facts.get("distribution")
+    distribution_version = facts.get("distribution_version")
+    if distribution and distribution_version:
+        return "%s-%s" % (distribution, distribution_version)
+    return "unknown"
+
+
+def _get_play_hosts_number(play_hosts_all):
+    return len(play_hosts_all)
+
+
+def _get_ansible_version(module):
+    version = getattr(module, "ansible_version", None)
+    if version:
+        return version
+    return "unknown"
+
+
+def _get_check_mode(module):
+    return bool(getattr(module, "check_mode", False))
+
+
+def _collect_fingerprint_record(module, status):
+    """Build the canonical fingerprint record used by all output formatters."""
+    return {
+        "date": _local_iso8601_no_microseconds(),
+        "role_name": module.params["role_name"],
+        "role_path": module.params["role_path"],
+        "status": status,
+        "ansible_version": _get_ansible_version(module),
+        "managed_node_distro": _get_managed_node_distro(module.params["ansible_facts"]),
+        "play_hosts_number": _get_play_hosts_number(
+            module.params["ansible_play_hosts_all"]
+        ),
+        "ansible_check_mode": _get_check_mode(module),
+    }
+
+
+def _fingerprint_record_items(record):
+    return [(field, record[field]) for field in FINGERPRINT_FIELDS]
+
+
+def _format_fingerprint_key_value(field, value):
+    text = "" if value is None else str(value)
+    if any(char in text for char in ' "='):
+        return '%s="%s"' % (field, text.replace('"', '""'))
+    return "%s=%s" % (field, text)
+
+
+def _format_fingerprint_syslog(record):
+    """Format the canonical fingerprint record as key=value syslog text."""
+    pairs = [
+        _format_fingerprint_key_value(field, value)
+        for field, value in _fingerprint_record_items(record)
+    ]
+    return FINGERPRINT_SYSLOG_SEPARATOR.join(pairs)
+
+
 def run_module():
     module_args = dict(
-        sr_message=dict(type="str", required=True),
+        status=dict(type="str", required=True, choices=["begin", "success"]),
+        write_log_file=dict(type="bool", default=False),
+        log_file=dict(type="path", default=DEFAULT_LOG_FILE),
+        role_name=dict(type="str", required=True),
+        role_path=dict(type="path", required=True),
+        ansible_play_hosts_all=dict(type="list", elements="str", required=True),
+        ansible_facts=dict(type="dict", required=True, no_log=True),
     )
 
     module = AnsibleModule(
@@ -61,23 +224,36 @@ def run_module():
         supports_check_mode=True,
     )
 
-    log_message = "%s %s" % (
-        module.params["sr_message"],
-        _local_iso8601_no_microseconds(),
-    )
+    fingerprint_record = _collect_fingerprint_record(module, module.params["status"])
+    log_message = _format_fingerprint_syslog(fingerprint_record)
 
     if module.check_mode:
-        module.exit_json(
+        result = dict(
             changed=False,
             message="Check mode: message not logged - [%s]" % log_message,
+            fingerprint=fingerprint_record,
         )
+        if module.params["write_log_file"]:
+            result["jsonl_row"] = _format_fingerprint_jsonl(fingerprint_record)
+            result["log_file"] = module.params["log_file"]
+        module.exit_json(**result)
 
     module.log(log_message)
 
+    if module.params["write_log_file"]:
+        log_file = module.params["log_file"]
+        try:
+            _write_jsonl_log(log_file, fingerprint_record)
+        except (IOError, OSError) as exc:
+            module.fail_json(
+                msg="Failed to write fingerprint log file %s: %s"
+                % (log_file, exc)
+            )
+
     # we don't actually change anything, so we're not changed - writing a log message
     # is not considered a change
     # also, we don't want to report changed every time the role runs
-    module.exit_json(changed=False)
+    module.exit_json(changed=False, fingerprint=fingerprint_record)
 
 
 def main():
diff --git a/playbooks/files/tests/unit/test_sr_fingerprint.py b/playbooks/files/tests/unit/test_sr_fingerprint.py
new file mode 100644
index 0000000..e6d3fd1
--- /dev/null
+++ b/playbooks/files/tests/unit/test_sr_fingerprint.py
@@ -0,0 +1,167 @@
+# -*- coding: utf-8 -*-
+
+# Copyright: (c) 2026, Red Hat, Inc.
+# SPDX-License-Identifier: MIT
+"""Unit tests for sr_fingerprint module helpers."""
+
+from __future__ import absolute_import, division, print_function
+
+__metaclass__ = type
+
+import json
+import os
+import tempfile
+import unittest
+
+import sr_fingerprint
+
+
+class _FakeModule(object):
+    ansible_version = "2.16.3"
+
+    def __init__(self, params=None, check_mode=False):
+        self.params = params or {}
+        self.check_mode = check_mode
+
+
+def _sample_fingerprint_record():
+    return {
+        "date": "2026-06-10T12:00:00+00:00",
+        "role_name": "systemd",
+        "role_path": "/usr/share/ansible/roles/systemd",
+        "status": "begin",
+        "ansible_version": "2.16.3",
+        "managed_node_distro": "RedHat-9.4",
+        "play_hosts_number": 3,
+        "ansible_check_mode": False,
+    }
+
+
+class TestSrFingerprint(unittest.TestCase):
+    def test_fingerprint_fields_match_record_keys(self):
+        record = _sample_fingerprint_record()
+        self.assertEqual(set(sr_fingerprint.FINGERPRINT_FIELDS), set(record.keys()))
+
+    def test_format_fingerprint_syslog(self):
+        record = _sample_fingerprint_record()
+        message = sr_fingerprint._format_fingerprint_syslog(record)
+        self.assertEqual(
+            message,
+            "date=2026-06-10T12:00:00+00:00 role_name=systemd "
+            "role_path=/usr/share/ansible/roles/systemd status=begin "
+            "ansible_version=2.16.3 managed_node_distro=RedHat-9.4 "
+            "play_hosts_number=3 ansible_check_mode=False",
+        )
+        for field in sr_fingerprint.FINGERPRINT_FIELDS:
+            self.assertIn("%s=" % field, message)
+
+    def test_format_fingerprint_jsonl(self):
+        record = _sample_fingerprint_record()
+        line = sr_fingerprint._format_fingerprint_jsonl(record)
+        parsed = json.loads(line)
+        self.assertEqual(parsed, record)
+
+    def test_collect_fingerprint_record_from_passed_inputs(self):
+        module = _FakeModule(
+            {
+                "role_name": "systemd",
+                "role_path": "/usr/share/ansible/roles/systemd",
+                "ansible_play_hosts_all": ["host1", "host2", "host3"],
+                "ansible_facts": {
+                    "distribution": "RedHat",
+                    "distribution_version": "9.4",
+                },
+            },
+            check_mode=True,
+        )
+        record = sr_fingerprint._collect_fingerprint_record(module, "begin")
+        self.assertEqual(record["role_name"], "systemd")
+        self.assertEqual(record["role_path"], "/usr/share/ansible/roles/systemd")
+        self.assertEqual(record["managed_node_distro"], "RedHat-9.4")
+        self.assertEqual(record["play_hosts_number"], 3)
+        self.assertTrue(record["ansible_check_mode"])
+        self.assertEqual(
+            set(record.keys()),
+            set(sr_fingerprint.FINGERPRINT_FIELDS),
+        )
+
+    def test_get_managed_node_distro_from_facts(self):
+        distro = sr_fingerprint._get_managed_node_distro(
+            {"distribution": "Fedora", "distribution_version": "42"}
+        )
+        self.assertEqual(distro, "Fedora-42")
+
+    def test_get_managed_node_distro_missing(self):
+        self.assertEqual(sr_fingerprint._get_managed_node_distro({}), "unknown")
+
+    def test_get_play_hosts_number(self):
+        self.assertEqual(
+            sr_fingerprint._get_play_hosts_number(["a", "b"]),
+            2,
+        )
+        self.assertEqual(sr_fingerprint._get_play_hosts_number([]), 0)
+
+    def test_format_fingerprint_syslog_quotes_values_with_spaces(self):
+        record = _sample_fingerprint_record()
+        record["role_path"] = "/usr/share/ansible/roles/systemd extra"
+        message = sr_fingerprint._format_fingerprint_syslog(record)
+        self.assertIn('role_path="/usr/share/ansible/roles/systemd extra"', message)
+
+    def test_write_jsonl_log_appends_valid_json_lines(self):
+        with tempfile.NamedTemporaryFile(delete=False, suffix=".jsonl") as tmp:
+            log_file = tmp.name
+
+        try:
+            record = _sample_fingerprint_record()
+            sr_fingerprint._write_jsonl_log(log_file, record)
+            sr_fingerprint._write_jsonl_log(log_file, record)
+
+            with open(log_file, "r") as log_fd:
+                lines = log_fd.read().splitlines()
+
+            self.assertEqual(len(lines), 2)
+            for line in lines:
+                parsed = json.loads(line)
+                self.assertEqual(parsed, record)
+        finally:
+            os.unlink(log_file)
+
+    def test_write_jsonl_log_creates_parent_dir(self):
+        tmpdir = tempfile.mkdtemp()
+        log_file = os.path.join(tmpdir, "subdir", "fingerprint.jsonl")
+
+        try:
+            record = _sample_fingerprint_record()
+            sr_fingerprint._write_jsonl_log(log_file, record)
+
+            with open(log_file, "r") as log_fd:
+                parsed = json.loads(log_fd.readline())
+            self.assertEqual(parsed["role_name"], "systemd")
+        finally:
+            os.unlink(log_file)
+            os.rmdir(os.path.dirname(log_file))
+            os.rmdir(tmpdir)
+
+    def test_write_jsonl_log_preserves_types(self):
+        with tempfile.NamedTemporaryFile(delete=False, suffix=".jsonl") as tmp:
+            log_file = tmp.name
+
+        try:
+            record = _sample_fingerprint_record()
+            sr_fingerprint._write_jsonl_log(log_file, record)
+
+            with open(log_file, "r") as log_fd:
+                parsed = json.loads(log_fd.readline())
+
+            self.assertIsInstance(parsed["play_hosts_number"], int)
+            self.assertIsInstance(parsed["ansible_check_mode"], bool)
+        finally:
+            os.unlink(log_file)
+
+    def test_local_iso8601_no_microseconds_has_no_fraction(self):
+        timestamp = sr_fingerprint._local_iso8601_no_microseconds()
+        self.assertNotIn(".", timestamp)
+
+
+if __name__ == "__main__":
+    unittest.main()

From c372832cbdd2ae0cc4fa531cbb7c9cf58cdd1ae3 Mon Sep 17 00:00:00 2001
From: Sergei Petrosian <spetrosi@redhat.com>
Date: Mon, 3 Aug 2026 17:14:25 +0200
Subject: [PATCH 2/6] Add lines limit to log file

* Add max_log_lines defaulting to 10000
* Apply CodeRabbit review
* Change the need for large ansible_facts to smaller distribution and
distribution_version
---
 inventory/group_vars/active_roles.yml         |   2 +-
 playbooks/files/library/sr_fingerprint.py     | 130 ++++++++++-----
 playbooks/files/tests/unit/sr_fingerprint.py  |   1 +
 .../files/tests/unit/test_sr_fingerprint.py   | 149 ++++++++++++++++--
 4 files changed, 235 insertions(+), 47 deletions(-)
 create mode 120000 playbooks/files/tests/unit/sr_fingerprint.py

diff --git a/inventory/group_vars/active_roles.yml b/inventory/group_vars/active_roles.yml
index f94e9a2..accb76b 100644
--- a/inventory/group_vars/active_roles.yml
+++ b/inventory/group_vars/active_roles.yml
@@ -9,7 +9,7 @@ present_files:
   - .ostree/README.md
   - README-ostree.md
   - library/sr_fingerprint.py
-  - tests/unit/test_sr_fingerprint.yml
+  - tests/unit/test_sr_fingerprint.py
 present_templates:
   - .ansible-lint
   - .coderabbit.yaml
diff --git a/playbooks/files/library/sr_fingerprint.py b/playbooks/files/library/sr_fingerprint.py
index 6b44b23..2aa7657 100644
--- a/playbooks/files/library/sr_fingerprint.py
+++ b/playbooks/files/library/sr_fingerprint.py
@@ -15,8 +15,8 @@
       (one JSON object per line, JSONL format), by default
       C(/var/log/sysroles.jsonl).
     - Playbook variables are not available inside modules automatically. Roles
-      pass C(role_name), C(role_path), C(ansible_play_hosts_all), and
-      C(ansible_facts) from the task.
+      pass C(role_name), C(role_path), C(ansible_play_hosts_all),
+      C(distribution), and C(distribution_version) from the task.
     - C(ansible_check_mode) is collected from the module execution context.
     - Intended for role-internal or diagnostic use.
 author: Rich Megginson (@richm)
@@ -38,6 +38,13 @@
         description: Path to the JSONL log file.
         type: path
         default: /var/log/sysroles.jsonl
+    max_log_lines:
+        description: >-
+            Maximum number of lines to keep in the log file. When the file
+            exceeds this limit after a write, the oldest lines are removed.
+            Set to C(0) to disable trimming.
+        type: int
+        default: 10000
     role_name:
         description: Name of the role, typically C({{ role_name }}).
         type: str
@@ -53,12 +60,18 @@
         type: list
         elements: str
         required: true
-    ansible_facts:
+    distribution:
         description: >-
-            Facts from the playbook for the current managed host, typically
-            C({{ ansible_facts }}).
-        type: dict
-        required: true
+            OS distribution name, typically
+            C({{ ansible_facts["distribution"] }}).
+        type: str
+        default: ""
+    distribution_version:
+        description: >-
+            OS distribution version, typically
+            C({{ ansible_facts["distribution_version"] }}).
+        type: str
+        default: ""
 """
 
 EXAMPLES = """
@@ -68,7 +81,8 @@
     role_name: bootloader
     role_path: "{{ role_path }}"
     ansible_play_hosts_all: "{{ ansible_play_hosts_all }}"
-    ansible_facts: "{{ ansible_facts }}"
+    distribution: "{{ ansible_facts['distribution'] }}"
+    distribution_version: "{{ ansible_facts['distribution_version'] }}"
     write_log_file: false
 
 - name: Record role success fingerprint
@@ -77,11 +91,39 @@
     role_name: bootloader
     role_path: "{{ role_path }}"
     ansible_play_hosts_all: "{{ ansible_play_hosts_all }}"
-    ansible_facts: "{{ ansible_facts }}"
+    distribution: "{{ ansible_facts['distribution'] }}"
+    distribution_version: "{{ ansible_facts['distribution_version'] }}"
     write_log_file: true
 """
 
-RETURN = r""" # """
+RETURN = r"""
+fingerprint:
+    description: The fingerprint record written to syslog and optionally to the log file.
+    returned: always
+    type: dict
+    sample:
+        date: "2026-08-03T10:15:00+02:00"
+        role_name: network
+        role_path: /usr/share/ansible/roles/linux-system-roles.network
+        status: success
+        ansible_version: "2.16.3"
+        managed_node_distro: RedHat-9.4
+        play_hosts_number: 3
+        ansible_check_mode: false
+message:
+    description: Informational message shown in check mode.
+    returned: check mode
+    type: str
+    sample: "Check mode: message not logged - [date=... role_name=...]"
+jsonl_row:
+    description: The JSON line that would be appended to the log file.
+    returned: check mode and O(write_log_file=true)
+    type: str
+log_file:
+    description: Path to the log file that would be written.
+    returned: check mode and O(write_log_file=true)
+    type: str
+"""
 
 from ansible.module_utils.basic import AnsibleModule
 
@@ -90,8 +132,6 @@
 import json
 import os
 
-DEFAULT_LOG_FILE = "/var/log/sysroles.jsonl"
-
 FINGERPRINT_FIELDS = (
     "date",
     "role_name",
@@ -143,15 +183,24 @@ def _format_fingerprint_jsonl(record):
     return json.dumps(record, separators=(",", ":"), sort_keys=False)
 
 
-def _write_jsonl_log(log_file, record):
+def _trim_log_file(log_file, max_lines):
+    with open(log_file, "r") as log_fd:
+        lines = log_fd.readlines()
+    if len(lines) <= max_lines:
+        return
+    with open(log_file, "w") as log_fd:
+        log_fd.writelines(lines[-max_lines:])
+
+
+def _write_jsonl_log(log_file, record, max_lines=0):
     _ensure_parent_dir(log_file)
     with open(log_file, "a") as log_fd:
         log_fd.write(_format_fingerprint_jsonl(record) + "\n")
+    if max_lines > 0:
+        _trim_log_file(log_file, max_lines)
 
 
-def _get_managed_node_distro(facts):
-    distribution = facts.get("distribution")
-    distribution_version = facts.get("distribution_version")
+def _get_managed_node_distro(distribution, distribution_version):
     if distribution and distribution_version:
         return "%s-%s" % (distribution, distribution_version)
     return "unknown"
@@ -180,7 +229,9 @@ def _collect_fingerprint_record(module, status):
         "role_path": module.params["role_path"],
         "status": status,
         "ansible_version": _get_ansible_version(module),
-        "managed_node_distro": _get_managed_node_distro(module.params["ansible_facts"]),
+        "managed_node_distro": _get_managed_node_distro(
+            module.params["distribution"], module.params["distribution_version"]
+        ),
         "play_hosts_number": _get_play_hosts_number(
             module.params["ansible_play_hosts_all"]
         ),
@@ -208,22 +259,7 @@ def _format_fingerprint_syslog(record):
     return FINGERPRINT_SYSLOG_SEPARATOR.join(pairs)
 
 
-def run_module():
-    module_args = dict(
-        status=dict(type="str", required=True, choices=["begin", "success"]),
-        write_log_file=dict(type="bool", default=False),
-        log_file=dict(type="path", default=DEFAULT_LOG_FILE),
-        role_name=dict(type="str", required=True),
-        role_path=dict(type="path", required=True),
-        ansible_play_hosts_all=dict(type="list", elements="str", required=True),
-        ansible_facts=dict(type="dict", required=True, no_log=True),
-    )
-
-    module = AnsibleModule(
-        argument_spec=module_args,
-        supports_check_mode=True,
-    )
-
+def _handle_fingerprint(module):
     fingerprint_record = _collect_fingerprint_record(module, module.params["status"])
     log_message = _format_fingerprint_syslog(fingerprint_record)
 
@@ -243,19 +279,39 @@ def run_module():
     if module.params["write_log_file"]:
         log_file = module.params["log_file"]
         try:
-            _write_jsonl_log(log_file, fingerprint_record)
+            _write_jsonl_log(
+                log_file, fingerprint_record, module.params["max_log_lines"]
+            )
         except (IOError, OSError) as exc:
             module.fail_json(
                 msg="Failed to write fingerprint log file %s: %s"
                 % (log_file, exc)
             )
 
-    # we don't actually change anything, so we're not changed - writing a log message
-    # is not considered a change
-    # also, we don't want to report changed every time the role runs
     module.exit_json(changed=False, fingerprint=fingerprint_record)
 
 
+def run_module():
+    module_args = dict(
+        status=dict(type="str", required=True, choices=["begin", "success"]),
+        write_log_file=dict(type="bool", default=False),
+        log_file=dict(type="path", default="/var/log/sysroles.jsonl"),
+        max_log_lines=dict(type="int", default=10000),
+        role_name=dict(type="str", required=True),
+        role_path=dict(type="path", required=True),
+        ansible_play_hosts_all=dict(type="list", elements="str", required=True),
+        distribution=dict(type="str", default=""),
+        distribution_version=dict(type="str", default=""),
+    )
+
+    module = AnsibleModule(
+        argument_spec=module_args,
+        supports_check_mode=True,
+    )
+
+    _handle_fingerprint(module)
+
+
 def main():
     run_module()
 
diff --git a/playbooks/files/tests/unit/sr_fingerprint.py b/playbooks/files/tests/unit/sr_fingerprint.py
new file mode 120000
index 0000000..943044c
--- /dev/null
+++ b/playbooks/files/tests/unit/sr_fingerprint.py
@@ -0,0 +1 @@
+../../library/sr_fingerprint.py
\ No newline at end of file
diff --git a/playbooks/files/tests/unit/test_sr_fingerprint.py b/playbooks/files/tests/unit/test_sr_fingerprint.py
index e6d3fd1..a10d108 100644
--- a/playbooks/files/tests/unit/test_sr_fingerprint.py
+++ b/playbooks/files/tests/unit/test_sr_fingerprint.py
@@ -16,12 +16,32 @@
 import sr_fingerprint
 
 
+class _ExitJsonException(Exception):
+    def __init__(self, kwargs):
+        self.kwargs = kwargs
+
+
+class _FailJsonException(Exception):
+    def __init__(self, kwargs):
+        self.kwargs = kwargs
+
+
 class _FakeModule(object):
     ansible_version = "2.16.3"
 
     def __init__(self, params=None, check_mode=False):
         self.params = params or {}
         self.check_mode = check_mode
+        self.logged = []
+
+    def log(self, msg):
+        self.logged.append(msg)
+
+    def exit_json(self, **kwargs):
+        raise _ExitJsonException(kwargs)
+
+    def fail_json(self, **kwargs):
+        raise _FailJsonException(kwargs)
 
 
 def _sample_fingerprint_record():
@@ -67,10 +87,8 @@ def test_collect_fingerprint_record_from_passed_inputs(self):
                 "role_name": "systemd",
                 "role_path": "/usr/share/ansible/roles/systemd",
                 "ansible_play_hosts_all": ["host1", "host2", "host3"],
-                "ansible_facts": {
-                    "distribution": "RedHat",
-                    "distribution_version": "9.4",
-                },
+                "distribution": "RedHat",
+                "distribution_version": "9.4",
             },
             check_mode=True,
         )
@@ -85,14 +103,12 @@ def test_collect_fingerprint_record_from_passed_inputs(self):
             set(sr_fingerprint.FINGERPRINT_FIELDS),
         )
 
-    def test_get_managed_node_distro_from_facts(self):
-        distro = sr_fingerprint._get_managed_node_distro(
-            {"distribution": "Fedora", "distribution_version": "42"}
-        )
+    def test_get_managed_node_distro_from_params(self):
+        distro = sr_fingerprint._get_managed_node_distro("Fedora", "42")
         self.assertEqual(distro, "Fedora-42")
 
     def test_get_managed_node_distro_missing(self):
-        self.assertEqual(sr_fingerprint._get_managed_node_distro({}), "unknown")
+        self.assertEqual(sr_fingerprint._get_managed_node_distro("", ""), "unknown")
 
     def test_get_play_hosts_number(self):
         self.assertEqual(
@@ -158,6 +174,121 @@ def test_write_jsonl_log_preserves_types(self):
         finally:
             os.unlink(log_file)
 
+    def test_trim_removes_oldest_lines(self):
+        with tempfile.NamedTemporaryFile(delete=False, suffix=".jsonl") as tmp:
+            log_file = tmp.name
+
+        try:
+            record = _sample_fingerprint_record()
+            for i in range(10):
+                record_copy = dict(record, role_name="role_%d" % i)
+                sr_fingerprint._write_jsonl_log(log_file, record_copy, max_lines=5)
+
+            with open(log_file, "r") as log_fd:
+                lines = log_fd.read().splitlines()
+
+            self.assertEqual(len(lines), 5)
+            first = json.loads(lines[0])
+            last = json.loads(lines[-1])
+            self.assertEqual(first["role_name"], "role_5")
+            self.assertEqual(last["role_name"], "role_9")
+        finally:
+            os.unlink(log_file)
+
+    def test_trim_disabled_when_zero(self):
+        with tempfile.NamedTemporaryFile(delete=False, suffix=".jsonl") as tmp:
+            log_file = tmp.name
+
+        try:
+            record = _sample_fingerprint_record()
+            for i in range(20):
+                sr_fingerprint._write_jsonl_log(log_file, record, max_lines=0)
+
+            with open(log_file, "r") as log_fd:
+                lines = log_fd.read().splitlines()
+
+            self.assertEqual(len(lines), 20)
+        finally:
+            os.unlink(log_file)
+
+    def test_trim_no_op_when_under_limit(self):
+        with tempfile.NamedTemporaryFile(delete=False, suffix=".jsonl") as tmp:
+            log_file = tmp.name
+
+        try:
+            record = _sample_fingerprint_record()
+            for i in range(3):
+                sr_fingerprint._write_jsonl_log(log_file, record, max_lines=10)
+
+            with open(log_file, "r") as log_fd:
+                lines = log_fd.read().splitlines()
+
+            self.assertEqual(len(lines), 3)
+        finally:
+            os.unlink(log_file)
+
+    def test_handle_fingerprint_check_mode_without_log_file(self):
+        module = _FakeModule(
+            {
+                "status": "begin",
+                "write_log_file": False,
+                "role_name": "systemd",
+                "role_path": "/usr/share/ansible/roles/systemd",
+                "ansible_play_hosts_all": ["host1"],
+                "distribution": "RedHat",
+                "distribution_version": "9.4",
+            },
+            check_mode=True,
+        )
+        with self.assertRaises(_ExitJsonException) as ctx:
+            sr_fingerprint._handle_fingerprint(module)
+        result = ctx.exception.kwargs
+        self.assertFalse(result["changed"])
+        self.assertIn("Check mode", result["message"])
+        self.assertIn("fingerprint", result)
+        self.assertNotIn("jsonl_row", result)
+
+    def test_handle_fingerprint_check_mode_with_log_file(self):
+        module = _FakeModule(
+            {
+                "status": "success",
+                "write_log_file": True,
+                "log_file": "/tmp/test.jsonl",
+                "role_name": "systemd",
+                "role_path": "/usr/share/ansible/roles/systemd",
+                "ansible_play_hosts_all": ["host1"],
+                "distribution": "RedHat",
+                "distribution_version": "9.4",
+            },
+            check_mode=True,
+        )
+        with self.assertRaises(_ExitJsonException) as ctx:
+            sr_fingerprint._handle_fingerprint(module)
+        result = ctx.exception.kwargs
+        self.assertIn("jsonl_row", result)
+        self.assertEqual(result["log_file"], "/tmp/test.jsonl")
+        parsed = json.loads(result["jsonl_row"])
+        self.assertEqual(parsed["role_name"], "systemd")
+
+    def test_handle_fingerprint_write_failure_calls_fail_json(self):
+        module = _FakeModule(
+            {
+                "status": "success",
+                "write_log_file": True,
+                "log_file": "/nonexistent/deep/path/test.jsonl",
+                "max_log_lines": 10000,
+                "role_name": "systemd",
+                "role_path": "/usr/share/ansible/roles/systemd",
+                "ansible_play_hosts_all": ["host1"],
+                "distribution": "RedHat",
+                "distribution_version": "9.4",
+            },
+            check_mode=False,
+        )
+        with self.assertRaises(_FailJsonException) as ctx:
+            sr_fingerprint._handle_fingerprint(module)
+        self.assertIn("Failed to write fingerprint log file", ctx.exception.kwargs["msg"])
+
     def test_local_iso8601_no_microseconds_has_no_fraction(self):
         timestamp = sr_fingerprint._local_iso8601_no_microseconds()
         self.assertNotIn(".", timestamp)

From b65fbaaf29a5bf316fa89bff35ace825861f7c40 Mon Sep 17 00:00:00 2001
From: Sergei Petrosian <spetrosi@redhat.com>
Date: Mon, 3 Aug 2026 17:25:27 +0200
Subject: [PATCH 3/6] Clean extra lines with a tempfile

---
 playbooks/files/library/sr_fingerprint.py     | 46 +++++++++++++++----
 .../files/tests/unit/test_sr_fingerprint.py   | 31 +++++++++++--
 2 files changed, 64 insertions(+), 13 deletions(-)

diff --git a/playbooks/files/library/sr_fingerprint.py b/playbooks/files/library/sr_fingerprint.py
index 2aa7657..6c1eba9 100644
--- a/playbooks/files/library/sr_fingerprint.py
+++ b/playbooks/files/library/sr_fingerprint.py
@@ -129,8 +129,10 @@
 
 import datetime
 import errno
+import fcntl
 import json
 import os
+import tempfile
 
 FINGERPRINT_FIELDS = (
     "date",
@@ -183,21 +185,43 @@ def _format_fingerprint_jsonl(record):
     return json.dumps(record, separators=(",", ":"), sort_keys=False)
 
 
-def _trim_log_file(log_file, max_lines):
-    with open(log_file, "r") as log_fd:
-        lines = log_fd.readlines()
+def _trim_log_file(log_fd, log_file, max_lines):
+    """Trim log_fd in place; caller must hold an exclusive lock."""
+    log_fd.seek(0)
+    lines = log_fd.readlines()
     if len(lines) <= max_lines:
         return
-    with open(log_file, "w") as log_fd:
-        log_fd.writelines(lines[-max_lines:])
+    kept = lines[-max_lines:]
+    dir_name = os.path.dirname(log_file) or "."
+    fd, tmp_path = tempfile.mkstemp(dir=dir_name, suffix=".tmp")
+    try:
+        with os.fdopen(fd, "w") as tmp_fd:
+            tmp_fd.writelines(kept)
+            tmp_fd.flush()
+            os.fsync(tmp_fd.fileno())
+        os.rename(tmp_path, log_file)
+    except BaseException:
+        try:
+            os.unlink(tmp_path)
+        except OSError:
+            pass
+        raise
 
 
 def _write_jsonl_log(log_file, record, max_lines=0):
     _ensure_parent_dir(log_file)
-    with open(log_file, "a") as log_fd:
-        log_fd.write(_format_fingerprint_jsonl(record) + "\n")
-    if max_lines > 0:
-        _trim_log_file(log_file, max_lines)
+    lock_path = log_file + ".lock"
+    lock_fd = open(lock_path, "w")
+    try:
+        fcntl.flock(lock_fd, fcntl.LOCK_EX)
+        with open(log_file, "a") as log_fd:
+            log_fd.write(_format_fingerprint_jsonl(record) + "\n")
+        if max_lines > 0:
+            with open(log_file, "r+") as log_fd:
+                _trim_log_file(log_fd, log_file, max_lines)
+    finally:
+        fcntl.flock(lock_fd, fcntl.LOCK_UN)
+        lock_fd.close()
 
 
 def _get_managed_node_distro(distribution, distribution_version):
@@ -260,6 +284,10 @@ def _format_fingerprint_syslog(record):
 
 
 def _handle_fingerprint(module):
+    max_log_lines = module.params.get("max_log_lines", 0)
+    if max_log_lines < 0:
+        module.fail_json(msg="max_log_lines must be 0 or a positive integer, got %d" % max_log_lines)
+
     fingerprint_record = _collect_fingerprint_record(module, module.params["status"])
     log_message = _format_fingerprint_syslog(fingerprint_record)
 
diff --git a/playbooks/files/tests/unit/test_sr_fingerprint.py b/playbooks/files/tests/unit/test_sr_fingerprint.py
index a10d108..af874b6 100644
--- a/playbooks/files/tests/unit/test_sr_fingerprint.py
+++ b/playbooks/files/tests/unit/test_sr_fingerprint.py
@@ -154,8 +154,10 @@ def test_write_jsonl_log_creates_parent_dir(self):
                 parsed = json.loads(log_fd.readline())
             self.assertEqual(parsed["role_name"], "systemd")
         finally:
-            os.unlink(log_file)
-            os.rmdir(os.path.dirname(log_file))
+            subdir = os.path.dirname(log_file)
+            for name in os.listdir(subdir):
+                os.unlink(os.path.join(subdir, name))
+            os.rmdir(subdir)
             os.rmdir(tmpdir)
 
     def test_write_jsonl_log_preserves_types(self):
@@ -232,6 +234,7 @@ def test_handle_fingerprint_check_mode_without_log_file(self):
             {
                 "status": "begin",
                 "write_log_file": False,
+                "max_log_lines": 10000,
                 "role_name": "systemd",
                 "role_path": "/usr/share/ansible/roles/systemd",
                 "ansible_play_hosts_all": ["host1"],
@@ -249,11 +252,13 @@ def test_handle_fingerprint_check_mode_without_log_file(self):
         self.assertNotIn("jsonl_row", result)
 
     def test_handle_fingerprint_check_mode_with_log_file(self):
+        log_path = os.path.join(tempfile.gettempdir(), "test_sr_fingerprint.jsonl")
         module = _FakeModule(
             {
                 "status": "success",
                 "write_log_file": True,
-                "log_file": "/tmp/test.jsonl",
+                "log_file": log_path,
+                "max_log_lines": 10000,
                 "role_name": "systemd",
                 "role_path": "/usr/share/ansible/roles/systemd",
                 "ansible_play_hosts_all": ["host1"],
@@ -266,7 +271,7 @@ def test_handle_fingerprint_check_mode_with_log_file(self):
             sr_fingerprint._handle_fingerprint(module)
         result = ctx.exception.kwargs
         self.assertIn("jsonl_row", result)
-        self.assertEqual(result["log_file"], "/tmp/test.jsonl")
+        self.assertEqual(result["log_file"], log_path)
         parsed = json.loads(result["jsonl_row"])
         self.assertEqual(parsed["role_name"], "systemd")
 
@@ -289,6 +294,24 @@ def test_handle_fingerprint_write_failure_calls_fail_json(self):
             sr_fingerprint._handle_fingerprint(module)
         self.assertIn("Failed to write fingerprint log file", ctx.exception.kwargs["msg"])
 
+    def test_handle_fingerprint_rejects_negative_max_log_lines(self):
+        module = _FakeModule(
+            {
+                "status": "begin",
+                "write_log_file": False,
+                "max_log_lines": -1,
+                "role_name": "systemd",
+                "role_path": "/usr/share/ansible/roles/systemd",
+                "ansible_play_hosts_all": ["host1"],
+                "distribution": "RedHat",
+                "distribution_version": "9.4",
+            },
+            check_mode=False,
+        )
+        with self.assertRaises(_FailJsonException) as ctx:
+            sr_fingerprint._handle_fingerprint(module)
+        self.assertIn("max_log_lines must be 0 or a positive integer", ctx.exception.kwargs["msg"])
+
     def test_local_iso8601_no_microseconds_has_no_fraction(self):
         timestamp = sr_fingerprint._local_iso8601_no_microseconds()
         self.assertNotIn(".", timestamp)

From 0f6bfb0facd0fa5223117e5e7415c9bfb52ad40d Mon Sep 17 00:00:00 2001
From: Sergei Petrosian <spetrosi@redhat.com>
Date: Mon, 3 Aug 2026 17:50:27 +0200
Subject: [PATCH 4/6] Apply black formatting

---
 playbooks/files/library/sr_fingerprint.py     | 20 ++++++--
 .../files/tests/unit/test_sr_fingerprint.py   | 51 ++++++++++++++-----
 2 files changed, 52 insertions(+), 19 deletions(-)

diff --git a/playbooks/files/library/sr_fingerprint.py b/playbooks/files/library/sr_fingerprint.py
index 6c1eba9..4349cf5 100644
--- a/playbooks/files/library/sr_fingerprint.py
+++ b/playbooks/files/library/sr_fingerprint.py
@@ -7,7 +7,7 @@
 DOCUMENTATION = """
 ---
 module: sr_fingerprint
-short_description: Write role fingerprint data to syslog and optionally to a JSONL log file.
+short_description: Write role fingerprint data to syslog and optionally to a JSONL log file
 description:
     - Collects role fingerprint data into a canonical record and writes it to
       syslog using Ansible C(module.log) as C(key=value) pairs.
@@ -35,7 +35,9 @@
         type: bool
         default: false
     log_file:
-        description: Path to the JSONL log file.
+        description: >-
+            Path to the JSONL log file. A lock sidecar (C(<log_file>.lock))
+            is created next to the log file for cross-process safety.
         type: path
         default: /var/log/sysroles.jsonl
     max_log_lines:
@@ -132,6 +134,7 @@
 import fcntl
 import json
 import os
+import stat
 import tempfile
 
 FINGERPRINT_FIELDS = (
@@ -192,9 +195,15 @@ def _trim_log_file(log_fd, log_file, max_lines):
     if len(lines) <= max_lines:
         return
     kept = lines[-max_lines:]
+    orig_stat = os.fstat(log_fd.fileno())
     dir_name = os.path.dirname(log_file) or "."
     fd, tmp_path = tempfile.mkstemp(dir=dir_name, suffix=".tmp")
     try:
+        os.fchmod(fd, stat.S_IMODE(orig_stat.st_mode))
+        try:
+            os.fchown(fd, orig_stat.st_uid, orig_stat.st_gid)
+        except OSError:
+            pass
         with os.fdopen(fd, "w") as tmp_fd:
             tmp_fd.writelines(kept)
             tmp_fd.flush()
@@ -286,7 +295,9 @@ def _format_fingerprint_syslog(record):
 def _handle_fingerprint(module):
     max_log_lines = module.params.get("max_log_lines", 0)
     if max_log_lines < 0:
-        module.fail_json(msg="max_log_lines must be 0 or a positive integer, got %d" % max_log_lines)
+        module.fail_json(
+            msg="max_log_lines must be 0 or a positive integer, got %d" % max_log_lines
+        )
 
     fingerprint_record = _collect_fingerprint_record(module, module.params["status"])
     log_message = _format_fingerprint_syslog(fingerprint_record)
@@ -312,8 +323,7 @@ def _handle_fingerprint(module):
             )
         except (IOError, OSError) as exc:
             module.fail_json(
-                msg="Failed to write fingerprint log file %s: %s"
-                % (log_file, exc)
+                msg="Failed to write fingerprint log file %s: %s" % (log_file, exc)
             )
 
     module.exit_json(changed=False, fingerprint=fingerprint_record)
diff --git a/playbooks/files/tests/unit/test_sr_fingerprint.py b/playbooks/files/tests/unit/test_sr_fingerprint.py
index af874b6..cc58e13 100644
--- a/playbooks/files/tests/unit/test_sr_fingerprint.py
+++ b/playbooks/files/tests/unit/test_sr_fingerprint.py
@@ -44,6 +44,14 @@ def fail_json(self, **kwargs):
         raise _FailJsonException(kwargs)
 
 
+def _cleanup_log(log_file):
+    for path in (log_file, log_file + ".lock"):
+        try:
+            os.unlink(path)
+        except OSError:
+            pass
+
+
 def _sample_fingerprint_record():
     return {
         "date": "2026-06-10T12:00:00+00:00",
@@ -140,7 +148,7 @@ def test_write_jsonl_log_appends_valid_json_lines(self):
                 parsed = json.loads(line)
                 self.assertEqual(parsed, record)
         finally:
-            os.unlink(log_file)
+            _cleanup_log(log_file)
 
     def test_write_jsonl_log_creates_parent_dir(self):
         tmpdir = tempfile.mkdtemp()
@@ -174,7 +182,7 @@ def test_write_jsonl_log_preserves_types(self):
             self.assertIsInstance(parsed["play_hosts_number"], int)
             self.assertIsInstance(parsed["ansible_check_mode"], bool)
         finally:
-            os.unlink(log_file)
+            _cleanup_log(log_file)
 
     def test_trim_removes_oldest_lines(self):
         with tempfile.NamedTemporaryFile(delete=False, suffix=".jsonl") as tmp:
@@ -182,8 +190,8 @@ def test_trim_removes_oldest_lines(self):
 
         try:
             record = _sample_fingerprint_record()
-            for i in range(10):
-                record_copy = dict(record, role_name="role_%d" % i)
+            for _i in range(10):
+                record_copy = dict(record, role_name="role_%d" % _i)
                 sr_fingerprint._write_jsonl_log(log_file, record_copy, max_lines=5)
 
             with open(log_file, "r") as log_fd:
@@ -195,7 +203,7 @@ def test_trim_removes_oldest_lines(self):
             self.assertEqual(first["role_name"], "role_5")
             self.assertEqual(last["role_name"], "role_9")
         finally:
-            os.unlink(log_file)
+            _cleanup_log(log_file)
 
     def test_trim_disabled_when_zero(self):
         with tempfile.NamedTemporaryFile(delete=False, suffix=".jsonl") as tmp:
@@ -203,7 +211,7 @@ def test_trim_disabled_when_zero(self):
 
         try:
             record = _sample_fingerprint_record()
-            for i in range(20):
+            for _i in range(20):
                 sr_fingerprint._write_jsonl_log(log_file, record, max_lines=0)
 
             with open(log_file, "r") as log_fd:
@@ -211,7 +219,7 @@ def test_trim_disabled_when_zero(self):
 
             self.assertEqual(len(lines), 20)
         finally:
-            os.unlink(log_file)
+            _cleanup_log(log_file)
 
     def test_trim_no_op_when_under_limit(self):
         with tempfile.NamedTemporaryFile(delete=False, suffix=".jsonl") as tmp:
@@ -219,7 +227,7 @@ def test_trim_no_op_when_under_limit(self):
 
         try:
             record = _sample_fingerprint_record()
-            for i in range(3):
+            for _i in range(3):
                 sr_fingerprint._write_jsonl_log(log_file, record, max_lines=10)
 
             with open(log_file, "r") as log_fd:
@@ -227,7 +235,7 @@ def test_trim_no_op_when_under_limit(self):
 
             self.assertEqual(len(lines), 3)
         finally:
-            os.unlink(log_file)
+            _cleanup_log(log_file)
 
     def test_handle_fingerprint_check_mode_without_log_file(self):
         module = _FakeModule(
@@ -276,11 +284,12 @@ def test_handle_fingerprint_check_mode_with_log_file(self):
         self.assertEqual(parsed["role_name"], "systemd")
 
     def test_handle_fingerprint_write_failure_calls_fail_json(self):
+        log_path = os.path.join(tempfile.gettempdir(), "test_write_fail.jsonl")
         module = _FakeModule(
             {
                 "status": "success",
                 "write_log_file": True,
-                "log_file": "/nonexistent/deep/path/test.jsonl",
+                "log_file": log_path,
                 "max_log_lines": 10000,
                 "role_name": "systemd",
                 "role_path": "/usr/share/ansible/roles/systemd",
@@ -290,9 +299,20 @@ def test_handle_fingerprint_write_failure_calls_fail_json(self):
             },
             check_mode=False,
         )
-        with self.assertRaises(_FailJsonException) as ctx:
-            sr_fingerprint._handle_fingerprint(module)
-        self.assertIn("Failed to write fingerprint log file", ctx.exception.kwargs["msg"])
+        original = sr_fingerprint._write_jsonl_log
+
+        def _raise_ioerror(*args, **kwargs):
+            raise IOError("disk full")
+
+        sr_fingerprint._write_jsonl_log = _raise_ioerror
+        try:
+            with self.assertRaises(_FailJsonException) as ctx:
+                sr_fingerprint._handle_fingerprint(module)
+            self.assertIn(
+                "Failed to write fingerprint log file", ctx.exception.kwargs["msg"]
+            )
+        finally:
+            sr_fingerprint._write_jsonl_log = original
 
     def test_handle_fingerprint_rejects_negative_max_log_lines(self):
         module = _FakeModule(
@@ -310,7 +330,10 @@ def test_handle_fingerprint_rejects_negative_max_log_lines(self):
         )
         with self.assertRaises(_FailJsonException) as ctx:
             sr_fingerprint._handle_fingerprint(module)
-        self.assertIn("max_log_lines must be 0 or a positive integer", ctx.exception.kwargs["msg"])
+        self.assertIn(
+            "max_log_lines must be 0 or a positive integer",
+            ctx.exception.kwargs["msg"],
+        )
 
     def test_local_iso8601_no_microseconds_has_no_fraction(self):
         timestamp = sr_fingerprint._local_iso8601_no_microseconds()

From 20117deea82f7e183e423f3d72b037171416dca4 Mon Sep 17 00:00:00 2001
From: Sergei Petrosian <spetrosi@redhat.com>
Date: Tue, 4 Aug 2026 18:51:27 +0200
Subject: [PATCH 5/6] Apply Rich's suggestions

---
 playbooks/files/library/sr_fingerprint.py     | 49 ++++++++++---------
 .../files/tests/unit/test_sr_fingerprint.py   | 46 ++++++++++-------
 2 files changed, 53 insertions(+), 42 deletions(-)

diff --git a/playbooks/files/library/sr_fingerprint.py b/playbooks/files/library/sr_fingerprint.py
index 4349cf5..a0ca90c 100644
--- a/playbooks/files/library/sr_fingerprint.py
+++ b/playbooks/files/library/sr_fingerprint.py
@@ -40,13 +40,13 @@
             is created next to the log file for cross-process safety.
         type: path
         default: /var/log/sysroles.jsonl
-    max_log_lines:
+    max_log_size:
         description: >-
-            Maximum number of lines to keep in the log file. When the file
-            exceeds this limit after a write, the oldest lines are removed.
+            Maximum log file size in bytes. When appending a new record
+            would exceed this limit, the oldest records are removed first.
             Set to C(0) to disable trimming.
         type: int
-        default: 10000
+        default: 2000000
     role_name:
         description: Name of the role, typically C({{ role_name }}).
         type: str
@@ -188,14 +188,13 @@ def _format_fingerprint_jsonl(record):
     return json.dumps(record, separators=(",", ":"), sort_keys=False)
 
 
-def _trim_log_file(log_fd, log_file, max_lines):
-    """Trim log_fd in place; caller must hold an exclusive lock."""
-    log_fd.seek(0)
-    lines = log_fd.readlines()
-    if len(lines) <= max_lines:
-        return
-    kept = lines[-max_lines:]
-    orig_stat = os.fstat(log_fd.fileno())
+def _trim_log_file(log_file, target_size):
+    """Remove oldest records until the file fits in target_size bytes."""
+    with open(log_file, "r") as log_fd:
+        lines = log_fd.readlines()
+    while lines and sum(len(l) for l in lines) > target_size:
+        lines.pop(0)
+    orig_stat = os.stat(log_file)
     dir_name = os.path.dirname(log_file) or "."
     fd, tmp_path = tempfile.mkstemp(dir=dir_name, suffix=".tmp")
     try:
@@ -205,7 +204,7 @@ def _trim_log_file(log_fd, log_file, max_lines):
         except OSError:
             pass
         with os.fdopen(fd, "w") as tmp_fd:
-            tmp_fd.writelines(kept)
+            tmp_fd.writelines(lines)
             tmp_fd.flush()
             os.fsync(tmp_fd.fileno())
         os.rename(tmp_path, log_file)
@@ -217,17 +216,21 @@ def _trim_log_file(log_fd, log_file, max_lines):
         raise
 
 
-def _write_jsonl_log(log_file, record, max_lines=0):
+def _write_jsonl_log(log_file, record, max_size=0):
     _ensure_parent_dir(log_file)
+    new_line = _format_fingerprint_jsonl(record) + "\n"
     lock_path = log_file + ".lock"
     lock_fd = open(lock_path, "w")
     try:
         fcntl.flock(lock_fd, fcntl.LOCK_EX)
+        try:
+            cur_size = os.path.getsize(log_file)
+        except OSError:
+            cur_size = 0
+        if max_size > 0 and cur_size + len(new_line) > max_size:
+            _trim_log_file(log_file, max_size - len(new_line))
         with open(log_file, "a") as log_fd:
-            log_fd.write(_format_fingerprint_jsonl(record) + "\n")
-        if max_lines > 0:
-            with open(log_file, "r+") as log_fd:
-                _trim_log_file(log_fd, log_file, max_lines)
+            log_fd.write(new_line)
     finally:
         fcntl.flock(lock_fd, fcntl.LOCK_UN)
         lock_fd.close()
@@ -293,10 +296,10 @@ def _format_fingerprint_syslog(record):
 
 
 def _handle_fingerprint(module):
-    max_log_lines = module.params.get("max_log_lines", 0)
-    if max_log_lines < 0:
+    max_log_size = module.params.get("max_log_size", 0)
+    if max_log_size < 0:
         module.fail_json(
-            msg="max_log_lines must be 0 or a positive integer, got %d" % max_log_lines
+            msg="max_log_size must be 0 or a positive integer, got %d" % max_log_size
         )
 
     fingerprint_record = _collect_fingerprint_record(module, module.params["status"])
@@ -319,7 +322,7 @@ def _handle_fingerprint(module):
         log_file = module.params["log_file"]
         try:
             _write_jsonl_log(
-                log_file, fingerprint_record, module.params["max_log_lines"]
+                log_file, fingerprint_record, module.params["max_log_size"]
             )
         except (IOError, OSError) as exc:
             module.fail_json(
@@ -334,7 +337,7 @@ def run_module():
         status=dict(type="str", required=True, choices=["begin", "success"]),
         write_log_file=dict(type="bool", default=False),
         log_file=dict(type="path", default="/var/log/sysroles.jsonl"),
-        max_log_lines=dict(type="int", default=10000),
+        max_log_size=dict(type="int", default=2000000),
         role_name=dict(type="str", required=True),
         role_path=dict(type="path", required=True),
         ansible_play_hosts_all=dict(type="list", elements="str", required=True),
diff --git a/playbooks/files/tests/unit/test_sr_fingerprint.py b/playbooks/files/tests/unit/test_sr_fingerprint.py
index cc58e13..10cf587 100644
--- a/playbooks/files/tests/unit/test_sr_fingerprint.py
+++ b/playbooks/files/tests/unit/test_sr_fingerprint.py
@@ -56,7 +56,7 @@ def _sample_fingerprint_record():
     return {
         "date": "2026-06-10T12:00:00+00:00",
         "role_name": "systemd",
-        "role_path": "/usr/share/ansible/roles/systemd",
+        "role_path": "/usr/share/ansible/roles/linux-system-roles.systemd",
         "status": "begin",
         "ansible_version": "2.16.3",
         "managed_node_distro": "RedHat-9.4",
@@ -76,7 +76,7 @@ def test_format_fingerprint_syslog(self):
         self.assertEqual(
             message,
             "date=2026-06-10T12:00:00+00:00 role_name=systemd "
-            "role_path=/usr/share/ansible/roles/systemd status=begin "
+            "role_path=/usr/share/ansible/roles/linux-system-roles.systemd status=begin "
             "ansible_version=2.16.3 managed_node_distro=RedHat-9.4 "
             "play_hosts_number=3 ansible_check_mode=False",
         )
@@ -93,7 +93,7 @@ def test_collect_fingerprint_record_from_passed_inputs(self):
         module = _FakeModule(
             {
                 "role_name": "systemd",
-                "role_path": "/usr/share/ansible/roles/systemd",
+                "role_path": "/usr/share/ansible/roles/linux-system-roles.systemd",
                 "ansible_play_hosts_all": ["host1", "host2", "host3"],
                 "distribution": "RedHat",
                 "distribution_version": "9.4",
@@ -102,7 +102,9 @@ def test_collect_fingerprint_record_from_passed_inputs(self):
         )
         record = sr_fingerprint._collect_fingerprint_record(module, "begin")
         self.assertEqual(record["role_name"], "systemd")
-        self.assertEqual(record["role_path"], "/usr/share/ansible/roles/systemd")
+        self.assertEqual(
+            record["role_path"], "/usr/share/ansible/roles/linux-system-roles.systemd"
+        )
         self.assertEqual(record["managed_node_distro"], "RedHat-9.4")
         self.assertEqual(record["play_hosts_number"], 3)
         self.assertTrue(record["ansible_check_mode"])
@@ -127,9 +129,11 @@ def test_get_play_hosts_number(self):
 
     def test_format_fingerprint_syslog_quotes_values_with_spaces(self):
         record = _sample_fingerprint_record()
-        record["role_path"] = "/usr/share/ansible/roles/systemd extra"
+        record["role_path"] = "/usr/share/ansible/roles/linux-system-roles.systemd extra"
         message = sr_fingerprint._format_fingerprint_syslog(record)
-        self.assertIn('role_path="/usr/share/ansible/roles/systemd extra"', message)
+        self.assertIn(
+            'role_path="/usr/share/ansible/roles/linux-system-roles.systemd extra"', message
+        )
 
     def test_write_jsonl_log_appends_valid_json_lines(self):
         with tempfile.NamedTemporaryFile(delete=False, suffix=".jsonl") as tmp:
@@ -190,9 +194,13 @@ def test_trim_removes_oldest_lines(self):
 
         try:
             record = _sample_fingerprint_record()
+            # Each record with role_N name is 246 bytes; allow ~5 lines
+            max_size = 246 * 5
             for _i in range(10):
                 record_copy = dict(record, role_name="role_%d" % _i)
-                sr_fingerprint._write_jsonl_log(log_file, record_copy, max_lines=5)
+                sr_fingerprint._write_jsonl_log(
+                    log_file, record_copy, max_size=max_size
+                )
 
             with open(log_file, "r") as log_fd:
                 lines = log_fd.read().splitlines()
@@ -212,7 +220,7 @@ def test_trim_disabled_when_zero(self):
         try:
             record = _sample_fingerprint_record()
             for _i in range(20):
-                sr_fingerprint._write_jsonl_log(log_file, record, max_lines=0)
+                sr_fingerprint._write_jsonl_log(log_file, record, max_size=0)
 
             with open(log_file, "r") as log_fd:
                 lines = log_fd.read().splitlines()
@@ -228,7 +236,7 @@ def test_trim_no_op_when_under_limit(self):
         try:
             record = _sample_fingerprint_record()
             for _i in range(3):
-                sr_fingerprint._write_jsonl_log(log_file, record, max_lines=10)
+                sr_fingerprint._write_jsonl_log(log_file, record, max_size=2000000)
 
             with open(log_file, "r") as log_fd:
                 lines = log_fd.read().splitlines()
@@ -242,9 +250,9 @@ def test_handle_fingerprint_check_mode_without_log_file(self):
             {
                 "status": "begin",
                 "write_log_file": False,
-                "max_log_lines": 10000,
+                "max_log_size": 2000000,
                 "role_name": "systemd",
-                "role_path": "/usr/share/ansible/roles/systemd",
+                "role_path": "/usr/share/ansible/roles/linux-system-roles.systemd",
                 "ansible_play_hosts_all": ["host1"],
                 "distribution": "RedHat",
                 "distribution_version": "9.4",
@@ -266,9 +274,9 @@ def test_handle_fingerprint_check_mode_with_log_file(self):
                 "status": "success",
                 "write_log_file": True,
                 "log_file": log_path,
-                "max_log_lines": 10000,
+                "max_log_size": 2000000,
                 "role_name": "systemd",
-                "role_path": "/usr/share/ansible/roles/systemd",
+                "role_path": "/usr/share/ansible/roles/linux-system-roles.systemd",
                 "ansible_play_hosts_all": ["host1"],
                 "distribution": "RedHat",
                 "distribution_version": "9.4",
@@ -290,9 +298,9 @@ def test_handle_fingerprint_write_failure_calls_fail_json(self):
                 "status": "success",
                 "write_log_file": True,
                 "log_file": log_path,
-                "max_log_lines": 10000,
+                "max_log_size": 2000000,
                 "role_name": "systemd",
-                "role_path": "/usr/share/ansible/roles/systemd",
+                "role_path": "/usr/share/ansible/roles/linux-system-roles.systemd",
                 "ansible_play_hosts_all": ["host1"],
                 "distribution": "RedHat",
                 "distribution_version": "9.4",
@@ -314,14 +322,14 @@ def _raise_ioerror(*args, **kwargs):
         finally:
             sr_fingerprint._write_jsonl_log = original
 
-    def test_handle_fingerprint_rejects_negative_max_log_lines(self):
+    def test_handle_fingerprint_rejects_negative_max_log_size(self):
         module = _FakeModule(
             {
                 "status": "begin",
                 "write_log_file": False,
-                "max_log_lines": -1,
+                "max_log_size": -1,
                 "role_name": "systemd",
-                "role_path": "/usr/share/ansible/roles/systemd",
+                "role_path": "/usr/share/ansible/roles/linux-system-roles.systemd",
                 "ansible_play_hosts_all": ["host1"],
                 "distribution": "RedHat",
                 "distribution_version": "9.4",
@@ -331,7 +339,7 @@ def test_handle_fingerprint_rejects_negative_max_log_lines(self):
         with self.assertRaises(_FailJsonException) as ctx:
             sr_fingerprint._handle_fingerprint(module)
         self.assertIn(
-            "max_log_lines must be 0 or a positive integer",
+            "max_log_size must be 0 or a positive integer",
             ctx.exception.kwargs["msg"],
         )
 

From 3dbc81164529648c9efe07ebb024db0c4a313dba Mon Sep 17 00:00:00 2001
From: Sergei Petrosian <spetrosi@redhat.com>
Date: Tue, 4 Aug 2026 19:35:28 +0200
Subject: [PATCH 6/6] Fix review comments from CodeRabbit and
 github-advanced-security bot

---
 playbooks/files/library/sr_fingerprint.py       |  4 ++--
 .../files/tests/unit/test_sr_fingerprint.py     | 17 ++++++++++++-----
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/playbooks/files/library/sr_fingerprint.py b/playbooks/files/library/sr_fingerprint.py
index a0ca90c..96bfe3a 100644
--- a/playbooks/files/library/sr_fingerprint.py
+++ b/playbooks/files/library/sr_fingerprint.py
@@ -192,7 +192,7 @@ def _trim_log_file(log_file, target_size):
     """Remove oldest records until the file fits in target_size bytes."""
     with open(log_file, "r") as log_fd:
         lines = log_fd.readlines()
-    while lines and sum(len(l) for l in lines) > target_size:
+    while lines and sum(len(line) for line in lines) > target_size:
         lines.pop(0)
     orig_stat = os.stat(log_file)
     dir_name = os.path.dirname(log_file) or "."
@@ -227,7 +227,7 @@ def _write_jsonl_log(log_file, record, max_size=0):
             cur_size = os.path.getsize(log_file)
         except OSError:
             cur_size = 0
-        if max_size > 0 and cur_size + len(new_line) > max_size:
+        if max_size > 0 and cur_size + len(new_line) > max_size and cur_size > 0:
             _trim_log_file(log_file, max_size - len(new_line))
         with open(log_file, "a") as log_fd:
             log_fd.write(new_line)
diff --git a/playbooks/files/tests/unit/test_sr_fingerprint.py b/playbooks/files/tests/unit/test_sr_fingerprint.py
index 10cf587..466df90 100644
--- a/playbooks/files/tests/unit/test_sr_fingerprint.py
+++ b/playbooks/files/tests/unit/test_sr_fingerprint.py
@@ -129,10 +129,13 @@ def test_get_play_hosts_number(self):
 
     def test_format_fingerprint_syslog_quotes_values_with_spaces(self):
         record = _sample_fingerprint_record()
-        record["role_path"] = "/usr/share/ansible/roles/linux-system-roles.systemd extra"
+        record["role_path"] = (
+            "/usr/share/ansible/roles/linux-system-roles.systemd extra"
+        )
         message = sr_fingerprint._format_fingerprint_syslog(record)
         self.assertIn(
-            'role_path="/usr/share/ansible/roles/linux-system-roles.systemd extra"', message
+            'role_path="/usr/share/ansible/roles/linux-system-roles.systemd extra"',
+            message,
         )
 
     def test_write_jsonl_log_appends_valid_json_lines(self):
@@ -194,8 +197,9 @@ def test_trim_removes_oldest_lines(self):
 
         try:
             record = _sample_fingerprint_record()
-            # Each record with role_N name is 246 bytes; allow ~5 lines
-            max_size = 246 * 5
+            sample = dict(record, role_name="role_0")
+            line_size = len(sr_fingerprint._format_fingerprint_jsonl(sample) + "\n")
+            max_size = line_size * 5
             for _i in range(10):
                 record_copy = dict(record, role_name="role_%d" % _i)
                 sr_fingerprint._write_jsonl_log(
@@ -345,7 +349,10 @@ def test_handle_fingerprint_rejects_negative_max_log_size(self):
 
     def test_local_iso8601_no_microseconds_has_no_fraction(self):
         timestamp = sr_fingerprint._local_iso8601_no_microseconds()
-        self.assertNotIn(".", timestamp)
+        self.assertRegex(
+            timestamp,
+            r"^\d{4}-\d{2}-\d{2}T\d{2}:\d{2}:\d{2}[+-]\d{2}:?\d{2}$",
+        )
 
 
 if __name__ == "__main__":