Add ignore_spaces option to ini_file to ignore spacing changes (a…

…nsible-collections#7273)

* Add `ignore_spaces` option to `ini_file` to ignore spacing changes

Add a new `ignore_spaces` option to the `ini_file` module which, if
true, prevents the module from changing a line in a file if the only
thing that would change by doing so is whitespace before or after the
`=`.

Also add test cases for this new functionality. There were previously
no tests for `ini_file` at all, and this doesn't attempt to fix that,
but it does add tests to ensure that the new behavior implemented here
as well as the old behavior in the affected code are correct.

Fixes ansible-collections#7202.

* Add changelog fragment

* pep8 / pylint

* remove unused import

* fix typo in comment in integration test file

* Add symlink tests to main.yml

It appears that ansible-collections#6546 added symlink tests but neglected to add them to
main.yml so they weren't being executed.

* ini_file symlink tests; create output files in correct location

* Add integration tests for ini_file ignore_spaces

* PR feedback

changelogs/fragments/7273-ini_file_ignore_spaces.yml

@@ -0,0 +1,2 @@
+minor_changes:
+  - ini_file - add ``ignore_spaces`` option (https://github.com/ansible-collections/community.general/pull/7273).

plugins/modules/ini_file.py

@@ -4,6 +4,7 @@
 # Copyright (c) 2012, Jan-Piet Mens <jpmens () gmail.com>
 # Copyright (c) 2015, Ales Nosek <anosek.nosek () gmail.com>
 # Copyright (c) 2017, Ansible Project
+# Copyright (c) 2023, Ansible Project
 # GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)
 # SPDX-License-Identifier: GPL-3.0-or-later
 
@@ -98,6 +99,12 @@
       - Do not insert spaces before and after '=' symbol.
     type: bool
     default: false
+  ignore_spaces:
+    description:
+      - Do not change a line if doing so would only add or remove spaces before or after the V(=) symbol.
+    type: bool
+    default: false
+    version_added: 7.5.0
   create:
     description:
       - If set to V(false), the module will fail if the file does not already exist.
@@ -178,27 +185,35 @@
 
 def match_opt(option, line):
     option = re.escape(option)
-    return re.match('[#;]?( |\t)*(%s)( |\t)*(=|$)( |\t)*(.*)' % option, line)
+    return re.match('([#;]?)( |\t)*(%s)( |\t)*(=|$)( |\t)*(.*)' % option, line)
 
 
 def match_active_opt(option, line):
     option = re.escape(option)
     return re.match('( |\t)*(%s)( |\t)*(=|$)( |\t)*(.*)' % option, line)
 
 
-def update_section_line(changed, section_lines, index, changed_lines, newline, msg):
-    option_changed = section_lines[index] != newline
+def update_section_line(option, changed, section_lines, index, changed_lines, ignore_spaces, newline, msg):
+    option_changed = None
+    if ignore_spaces:
+        old_match = match_opt(option, section_lines[index])
+        if not old_match.group(1):
+            new_match = match_opt(option, newline)
+            option_changed = old_match.group(7) != new_match.group(7)
+    if option_changed is None:
+        option_changed = section_lines[index] != newline
+    if option_changed:
+        section_lines[index] = newline
     changed = changed or option_changed
     if option_changed:
         msg = 'option changed'
-    section_lines[index] = newline
     changed_lines[index] = 1
     return (changed, msg)
 
 
 def do_ini(module, filename, section=None, option=None, values=None,
            state='present', exclusive=True, backup=False, no_extra_spaces=False,
-           create=True, allow_no_value=False, follow=False):
+           ignore_spaces=False, create=True, allow_no_value=False, follow=False):
 
     if section is not None:
         section = to_text(section)
@@ -306,21 +321,21 @@ def do_ini(module, filename, section=None, option=None, values=None,
         for index, line in enumerate(section_lines):
             if match_opt(option, line):
                 match = match_opt(option, line)
-                if values and match.group(6) in values:
-                    matched_value = match.group(6)
+                if values and match.group(7) in values:
+                    matched_value = match.group(7)
                     if not matched_value and allow_no_value:
                         # replace existing option with no value line(s)
                         newline = u'%s\n' % option
                         option_no_value_present = True
                     else:
                         # replace existing option=value line(s)
                         newline = assignment_format % (option, matched_value)
-                    (changed, msg) = update_section_line(changed, section_lines, index, changed_lines, newline, msg)
+                    (changed, msg) = update_section_line(option, changed, section_lines, index, changed_lines, ignore_spaces, newline, msg)
                     values.remove(matched_value)
                 elif not values and allow_no_value:
                     # replace existing option with no value line(s)
                     newline = u'%s\n' % option
-                    (changed, msg) = update_section_line(changed, section_lines, index, changed_lines, newline, msg)
+                    (changed, msg) = update_section_line(option, changed, section_lines, index, changed_lines, ignore_spaces, newline, msg)
                     option_no_value_present = True
                     break
 
@@ -330,7 +345,7 @@ def do_ini(module, filename, section=None, option=None, values=None,
             for index, line in enumerate(section_lines):
                 if not changed_lines[index] and match_opt(option, line):
                     newline = assignment_format % (option, values.pop(0))
-                    (changed, msg) = update_section_line(changed, section_lines, index, changed_lines, newline, msg)
+                    (changed, msg) = update_section_line(option, changed, section_lines, index, changed_lines, ignore_spaces, newline, msg)
                     if len(values) == 0:
                         break
         # remove all remaining option occurrences from the rest of the section
@@ -449,6 +464,7 @@ def main():
             state=dict(type='str', default='present', choices=['absent', 'present']),
             exclusive=dict(type='bool', default=True),
             no_extra_spaces=dict(type='bool', default=False),
+            ignore_spaces=dict(type='bool', default=False),
             allow_no_value=dict(type='bool', default=False),
             create=dict(type='bool', default=True),
             follow=dict(type='bool', default=False)
@@ -469,6 +485,7 @@ def main():
     exclusive = module.params['exclusive']
     backup = module.params['backup']
     no_extra_spaces = module.params['no_extra_spaces']
+    ignore_spaces = module.params['ignore_spaces']
     allow_no_value = module.params['allow_no_value']
     create = module.params['create']
     follow = module.params['follow']
@@ -481,7 +498,9 @@ def main():
     elif values is None:
         values = []
 
-    (changed, backup_file, diff, msg) = do_ini(module, path, section, option, values, state, exclusive, backup, no_extra_spaces, create, allow_no_value, follow)
+    (changed, backup_file, diff, msg) = do_ini(
+        module, path, section, option, values, state, exclusive, backup,
+        no_extra_spaces, ignore_spaces, create, allow_no_value, follow)
 
     if not module.check_mode and os.path.exists(path):
         file_args = module.load_file_common_arguments(module.params)

tests/integration/targets/ini_file/tasks/main.yml

@@ -38,3 +38,9 @@
 
     - name: include tasks to test regressions
       include_tasks: tests/03-encoding.yml
+
+    - name: include tasks to test symlink handling
+      include_tasks: tests/04-symlink.yml
+
+    - name: include tasks to test ignore_spaces
+      include_tasks: tests/05-ignore_spaces.yml

tests/integration/targets/ini_file/tasks/tests/00-basic.yml

@@ -3,7 +3,7 @@
 # GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)
 # SPDX-License-Identifier: GPL-3.0-or-later
 
-## basiscs
+## basics
 
 - name: test-basic 1 - specify both "value" and "values" and fail
   ini_file:

tests/integration/targets/ini_file/tasks/tests/04-symlink.yml

@@ -9,26 +9,26 @@
       content: |
         [main]
         foo=BAR
-      dest: my_original_file.ini
+      dest: "{{ remote_tmp_dir }}/my_original_file.ini"
   - name: Clean up symlink.ini
     ansible.builtin.file:
-      path: symlink.ini
+      path: "{{ remote_tmp_dir }}/symlink.ini"
       state: absent
   - name: Create a symbolic link
     ansible.builtin.file:
       src: my_original_file.ini
-      dest: symlink.ini
+      dest: "{{ remote_tmp_dir }}/symlink.ini"
       state: link
 
 - name: Set the proxy key on the symlink which will be converted as a file
   community.general.ini_file:
-    path: symlink.ini
+    path: "{{ remote_tmp_dir }}/symlink.ini"
     section: main
     option: proxy
     value: 'http://proxy.myorg.org:3128'
 - name: Set the proxy key on the final file that is still unchanged
   community.general.ini_file:
-    path: my_original_file.ini
+    path: "{{ remote_tmp_dir }}/my_original_file.ini"
     section: main
     option: proxy
     value: 'http://proxy.myorg.org:3128'
@@ -41,15 +41,15 @@
 - block: *prepare
 - name: Set the proxy key on the symlink which will be preserved
   community.general.ini_file:
-    path: symlink.ini
+    path: "{{ remote_tmp_dir }}/symlink.ini"
     section: main
     option: proxy
     value: 'http://proxy.myorg.org:3128'
     follow: true
   register: result
 - name: Set the proxy key on the target directly that was changed in the previous step
   community.general.ini_file:
-    path: my_original_file.ini
+    path: "{{ remote_tmp_dir }}/my_original_file.ini"
     section: main
     option: proxy
     value: 'http://proxy.myorg.org:3128'

tests/integration/targets/ini_file/tasks/tests/05-ignore_spaces.yml

@@ -0,0 +1,123 @@
+---
+# Copyright (c) Ansible Project
+# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or https://www.gnu.org/licenses/gpl-3.0.txt)
+# SPDX-License-Identifier: GPL-3.0-or-later
+
+## testing ignore_spaces option
+
+- name: test-ignore_spaces 1 (commented line updated) - create test file
+  copy:
+    dest: "{{ output_file }}"
+    content: "[foo]\n; bar=baz\n"
+
+- name: test-ignore_spaces 1 - set new value
+  ini_file:
+    path: "{{ output_file }}"
+    section: foo
+    option: bar
+    value: frelt
+    ignore_spaces: true
+  register: result
+
+- name: test-ignore_spaces 1 - read content from output file
+  slurp:
+    src: "{{ output_file }}"
+  register: output_content
+
+- name: test-ignore_spaces 1 - verify results
+  vars:
+    actual_content: "{{ output_content.content | b64decode }}"
+    expected_content: "[foo]\nbar = frelt\n"
+  assert:
+    that:
+      - actual_content == expected_content
+      - result is changed
+      - result.msg == 'option changed'
+
+- name: test-ignore_spaces 2 (uncommented line updated) - create test file
+  copy:
+    dest: "{{ output_file }}"
+    content: "[foo]\nbar=baz\n"
+
+- name: test-ignore_spaces 2 - set new value
+  ini_file:
+    path: "{{ output_file }}"
+    section: foo
+    option: bar
+    value: frelt
+    ignore_spaces: true
+  register: result
+
+- name: test-ignore_spaces 2 - read content from output file
+  slurp:
+    src: "{{ output_file }}"
+  register: output_content
+
+- name: test-ignore_spaces 2 - verify results
+  vars:
+    actual_content: "{{ output_content.content | b64decode }}"
+    expected_content: "[foo]\nbar = frelt\n"
+  assert:
+    that:
+      - actual_content == expected_content
+      - result is changed
+      - result.msg == 'option changed'
+
+- name: test-ignore_spaces 3 (spaces on top of no spaces) - create test file
+  copy:
+    dest: "{{ output_file }}"
+    content: "[foo]\nbar=baz\n"
+
+- name: test-ignore_spaces 3 - try to set value
+  ini_file:
+    path: "{{ output_file }}"
+    section: foo
+    option: bar
+    value: baz
+    ignore_spaces: true
+  register: result
+
+- name: test-ignore_spaces 3 - read content from output file
+  slurp:
+    src: "{{ output_file }}"
+  register: output_content
+
+- name: test-ignore_spaces 3 - verify results
+  vars:
+    actual_content: "{{ output_content.content | b64decode }}"
+    expected_content: "[foo]\nbar=baz\n"
+  assert:
+    that:
+      - actual_content == expected_content
+      - result is not changed
+      - result.msg == "OK"
+
+- name: test-ignore_spaces 4 (no spaces on top of spaces) - create test file
+  copy:
+    dest: "{{ output_file }}"
+    content: "[foo]\nbar = baz\n"
+
+- name: test-ignore_spaces 4 - try to set value
+  ini_file:
+    path: "{{ output_file }}"
+    section: foo
+    option: bar
+    value: baz
+    ignore_spaces: true
+    no_extra_spaces: true
+  register: result
+
+- name: test-ignore_spaces 4 - read content from output file
+  slurp:
+    src: "{{ output_file }}"
+  register: output_content
+
+- name: test-ignore_spaces 4 - verify results
+  vars:
+    actual_content: "{{ output_content.content | b64decode }}"
+    expected_content: "[foo]\nbar = baz\n"
+  assert:
+    that:
+      - actual_content == expected_content
+      - result is not changed
+      - result.msg == "OK"

tests/unit/plugins/modules/test_ini_file.py

@@ -0,0 +1,51 @@
+# Copyright (c) 2023 Ansible Project
+# GNU General Public License v3.0+ (see LICENSES/GPL-3.0-or-later.txt or
+# https://www.gnu.org/licenses/gpl-3.0.txt)
+# SPDX-License-Identifier: GPL-3.0-or-later
+
+from __future__ import (absolute_import, division, print_function)
+__metaclass__ = type
+
+from ansible_collections.community.general.plugins.modules import ini_file
+
+
+def do_test(option, ignore_spaces, newline, before, expected_after,
+            expected_changed, expected_msg):
+    section_lines = [before]
+    changed_lines = [0]
+    changed, msg = ini_file.update_section_line(
+        option, None, section_lines, 0, changed_lines, ignore_spaces,
+        newline, None)
+    assert section_lines[0] == expected_after
+    assert changed == expected_changed
+    assert changed_lines[0] == 1
+    assert msg == expected_msg
+
+
+def test_ignore_spaces_comment():
+    oldline = ';foobar=baz'
+    newline = 'foobar = baz'
+    do_test('foobar', True, newline, oldline, newline, True, 'option changed')
+
+
+def test_ignore_spaces_changed():
+    oldline = 'foobar=baz'
+    newline = 'foobar = freeble'
+    do_test('foobar', True, newline, oldline, newline, True, 'option changed')
+
+
+def test_ignore_spaces_unchanged():
+    oldline = 'foobar=baz'
+    newline = 'foobar = baz'
+    do_test('foobar', True, newline, oldline, oldline, False, None)
+
+
+def test_no_ignore_spaces_changed():
+    oldline = 'foobar=baz'
+    newline = 'foobar = baz'
+    do_test('foobar', False, newline, oldline, newline, True, 'option changed')
+
+
+def test_no_ignore_spaces_unchanged():
+    newline = 'foobar=baz'
+    do_test('foobar', False, newline, newline, newline, False, None)