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..1f97170ee6 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,106 @@ 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