From fde6b13a7f9716356591d11d3053145c3db862da Mon Sep 17 00:00:00 2001 From: AsifAd <47986843+AsifAd@users.noreply.github.com> Date: Tue, 19 May 2026 16:04:56 +0530 Subject: [PATCH 1/3] ini_file: do not delete comment-only lines containing the option name When state=present and exclusive=true, the cleanup loops in do_ini() treated any line that contained the option name as a match, including pure documentation comments such as "; output_buffering" that have no "=" sign. As a result those lines were either rewritten in place or deleted, even though they are not configuration entries. Skip lines where match.group(2) is a comment character ("#" or ";") and match.group(6) is empty (no "=" present). This keeps the existing behaviour of replacing commented config lines like ";output_buffering = 4096" when modify_inactive_option=true, while leaving doc comments untouched. Adds two regression tests covering the exact scenario from the issue and the simpler commented-config replacement case. Fixes: https://github.com/ansible-collections/community.general/issues/11919 --- ...-file-do-not-delete-comment-only-lines.yml | 2 + plugins/modules/ini_file.py | 9 +- tests/unit/plugins/modules/test_ini_file.py | 112 ++++++++++++++++++ 3 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 changelogs/fragments/11919-ini-file-do-not-delete-comment-only-lines.yml diff --git a/changelogs/fragments/11919-ini-file-do-not-delete-comment-only-lines.yml b/changelogs/fragments/11919-ini-file-do-not-delete-comment-only-lines.yml new file mode 100644 index 0000000000..bace7fd7c0 --- /dev/null +++ b/changelogs/fragments/11919-ini-file-do-not-delete-comment-only-lines.yml @@ -0,0 +1,2 @@ +bugfixes: + - ini_file - do not delete comment-only lines that contain the option name (for example, ``; output_buffering``) when ``state=present`` and ``exclusive=true``; only lines that are actual configuration entries (with ``=``) are now considered for replacement or deletion (https://github.com/ansible-collections/community.general/issues/11919). diff --git a/plugins/modules/ini_file.py b/plugins/modules/ini_file.py index a8aa9d5f69..c27890db5f 100644 --- a/plugins/modules/ini_file.py +++ b/plugins/modules/ini_file.py @@ -468,7 +468,10 @@ def do_ini( # override option with no value to option with value if not allow_no_value if len(values) > 0: for index, line in enumerate(section_lines): - if not changed_lines[index] and match_function(option, line): + match = not changed_lines[index] and match_function(option, line) + # skip comment-only lines (e.g. "; output_buffering" with no "="): + # group(2) is the comment char, group(6) is "=" or "" (end-of-line match) + if match and not (match.group(2) and not match.group(6)): newline = f"{option}{sep}{values.pop(0)}\n" (changed, msg) = update_section_line( option, changed, section_lines, index, changed_lines, ignore_spaces, newline, msg @@ -477,7 +480,9 @@ def do_ini( break # remove all remaining option occurrences from the rest of the section for index in range(len(section_lines) - 1, 0, -1): - if not changed_lines[index] and match_function(option, section_lines[index]): + match = not changed_lines[index] and match_function(option, section_lines[index]) + # skip comment-only lines (no "=") — only remove active or commented config lines + if match and not (match.group(2) and not match.group(6)): del section_lines[index] del changed_lines[index] changed = True diff --git a/tests/unit/plugins/modules/test_ini_file.py b/tests/unit/plugins/modules/test_ini_file.py index 821b13a4b0..0ee0aeb0ed 100644 --- a/tests/unit/plugins/modules/test_ini_file.py +++ b/tests/unit/plugins/modules/test_ini_file.py @@ -5,6 +5,11 @@ from __future__ import annotations +import shutil +import tempfile + +import pytest + from ansible_collections.community.general.plugins.modules import ini_file @@ -47,3 +52,110 @@ def test_no_ignore_spaces_changed(): def test_no_ignore_spaces_unchanged(): newline = "foobar=baz" do_test("foobar", False, newline, newline, newline, False, None) + + +class FakeModule: + """Minimal AnsibleModule stub for do_ini unit tests.""" + _diff = False + check_mode = False + tmpdir = tempfile.gettempdir() + + def fail_json(self, **kw): + raise AssertionError(kw) + + def backup_local(self, f): + return f + ".bak" + + def atomic_move(self, src, dst): + shutil.move(src, dst) + + +@pytest.fixture() +def ini_file_path(tmp_path): + """Return a factory that writes content to a temp ini file and yields the path.""" + def _make(content): + p = tmp_path / "test.ini" + p.write_text(content) + return str(p) + return _make + + +def test_ini_file_doc_comment_lines_not_deleted_gh11919(ini_file_path): + """Regression test for https://github.com/ansible-collections/community.general/issues/11919. + + Pure doc comment lines containing the option name (e.g. '; output_buffering') + must not be deleted when state=present and exclusive=True. + Only the actual commented config line (';output_buffering = 4096') should be + replaced by the active config line. + """ + content = ( + "[PHP]\n" + "; Integer = Enables the buffer and sets its maximum size in bytes.\n" + "; Note: This directive is hardcoded to Off for the CLI SAPI\n" + "; Default Value: Off\n" + "; output_buffering\n" + "; Development Value: 4096\n" + "; Production Value: 4096\n" + "; https://php.net/output-buffering\n" + ";output_buffering = 4096\n" + ) + path = ini_file_path(content) + + ini_file.do_ini( + module=FakeModule(), + filename=path, + section="PHP", + option="output_buffering", + values=["123"], + state="present", + exclusive=True, + backup=False, + no_extra_spaces=False, + ignore_spaces=False, + create=True, + allow_no_value=False, + modify_inactive_option=True, + ) + + result = open(path).read() + + assert "; output_buffering\n" in result, ( + "Doc comment '; output_buffering' must not be deleted" + ) + assert "output_buffering = 123\n" in result, ( + "Active config 'output_buffering = 123' must be present" + ) + assert ";output_buffering = 4096\n" not in result, ( + "Old commented config ';output_buffering = 4096' must be replaced" + ) + + +def test_ini_file_commented_config_replaced_not_duplicated_gh11919(ini_file_path): + """Companion to gh11919: the commented config line must be replaced in-place, + not left behind alongside the new active line.""" + content = ( + "[PHP]\n" + ";output_buffering = 4096\n" + ) + path = ini_file_path(content) + + ini_file.do_ini( + module=FakeModule(), + filename=path, + section="PHP", + option="output_buffering", + values=["123"], + state="present", + exclusive=True, + backup=False, + no_extra_spaces=False, + ignore_spaces=False, + create=True, + allow_no_value=False, + modify_inactive_option=True, + ) + + result = open(path).read() + + assert "output_buffering = 123\n" in result + assert ";output_buffering = 4096\n" not in result From 099f91519d9de9cd2334502b6166fa099726f883 Mon Sep 17 00:00:00 2001 From: Asif Draxi Date: Wed, 20 May 2026 02:24:05 +0530 Subject: [PATCH 2/3] tests: let ruff tidy test_ini_file.py (CI was being picky) --- tests/unit/plugins/modules/test_ini_file.py | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/tests/unit/plugins/modules/test_ini_file.py b/tests/unit/plugins/modules/test_ini_file.py index 0ee0aeb0ed..1f97170ee6 100644 --- a/tests/unit/plugins/modules/test_ini_file.py +++ b/tests/unit/plugins/modules/test_ini_file.py @@ -56,6 +56,7 @@ def test_no_ignore_spaces_unchanged(): class FakeModule: """Minimal AnsibleModule stub for do_ini unit tests.""" + _diff = False check_mode = False tmpdir = tempfile.gettempdir() @@ -73,10 +74,12 @@ class FakeModule: @pytest.fixture() def ini_file_path(tmp_path): """Return a factory that writes content to a temp ini file and yields the path.""" + def _make(content): p = tmp_path / "test.ini" p.write_text(content) return str(p) + return _make @@ -119,12 +122,8 @@ def test_ini_file_doc_comment_lines_not_deleted_gh11919(ini_file_path): result = open(path).read() - assert "; output_buffering\n" in result, ( - "Doc comment '; output_buffering' must not be deleted" - ) - assert "output_buffering = 123\n" in result, ( - "Active config 'output_buffering = 123' must be present" - ) + assert "; output_buffering\n" in result, "Doc comment '; output_buffering' must not be deleted" + assert "output_buffering = 123\n" in result, "Active config 'output_buffering = 123' must be present" assert ";output_buffering = 4096\n" not in result, ( "Old commented config ';output_buffering = 4096' must be replaced" ) @@ -133,10 +132,7 @@ def test_ini_file_doc_comment_lines_not_deleted_gh11919(ini_file_path): def test_ini_file_commented_config_replaced_not_duplicated_gh11919(ini_file_path): """Companion to gh11919: the commented config line must be replaced in-place, not left behind alongside the new active line.""" - content = ( - "[PHP]\n" - ";output_buffering = 4096\n" - ) + content = "[PHP]\n;output_buffering = 4096\n" path = ini_file_path(content) ini_file.do_ini( From b75d59f18f9d4943a30774ef9def88238b02c136 Mon Sep 17 00:00:00 2001 From: Asif Draxi Date: Wed, 20 May 2026 02:34:50 +0530 Subject: [PATCH 3/3] ci: retrigger Azure pipeline after RHEL 9.7 flake