From 8a9b98273d5a92f1d30a9b4449cc87c80ec0b1ea Mon Sep 17 00:00:00 2001 From: Jonathan Kamens Date: Sun, 17 Sep 2023 07:22:22 -0400 Subject: [PATCH] Add `ignore_spaces` option to `ini_file` to ignore spacing changes (#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 #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 #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 --- .../fragments/7273-ini_file_ignore_spaces.yml | 2 + plugins/modules/ini_file.py | 41 ++++-- .../targets/ini_file/tasks/main.yml | 6 + .../targets/ini_file/tasks/tests/00-basic.yml | 2 +- .../ini_file/tasks/tests/04-symlink.yml | 14 +- .../ini_file/tasks/tests/05-ignore_spaces.yml | 123 ++++++++++++++++++ tests/unit/plugins/modules/test_ini_file.py | 51 ++++++++ 7 files changed, 220 insertions(+), 19 deletions(-) create mode 100644 changelogs/fragments/7273-ini_file_ignore_spaces.yml create mode 100644 tests/integration/targets/ini_file/tasks/tests/05-ignore_spaces.yml create mode 100644 tests/unit/plugins/modules/test_ini_file.py diff --git a/changelogs/fragments/7273-ini_file_ignore_spaces.yml b/changelogs/fragments/7273-ini_file_ignore_spaces.yml new file mode 100644 index 0000000000..4f5723777c --- /dev/null +++ b/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). diff --git a/plugins/modules/ini_file.py b/plugins/modules/ini_file.py index f084ce6b03..8e67a832ec 100644 --- a/plugins/modules/ini_file.py +++ b/plugins/modules/ini_file.py @@ -4,6 +4,7 @@ # Copyright (c) 2012, Jan-Piet Mens # Copyright (c) 2015, Ales Nosek # 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 @@ options: - 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,7 +185,7 @@ from ansible.module_utils.common.text.converters import to_bytes, to_text 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): @@ -186,19 +193,27 @@ def match_active_opt(option, line): 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,8 +321,8 @@ 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 @@ -315,12 +330,12 @@ def do_ini(module, filename, section=None, option=None, values=None, 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) diff --git a/tests/integration/targets/ini_file/tasks/main.yml b/tests/integration/targets/ini_file/tasks/main.yml index 11c5bf3b23..2fa54cd98c 100644 --- a/tests/integration/targets/ini_file/tasks/main.yml +++ b/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 diff --git a/tests/integration/targets/ini_file/tasks/tests/00-basic.yml b/tests/integration/targets/ini_file/tasks/tests/00-basic.yml index c619e937a5..2d0ab3ac4c 100644 --- a/tests/integration/targets/ini_file/tasks/tests/00-basic.yml +++ b/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: diff --git a/tests/integration/targets/ini_file/tasks/tests/04-symlink.yml b/tests/integration/targets/ini_file/tasks/tests/04-symlink.yml index de677bffad..7e83a010d0 100644 --- a/tests/integration/targets/ini_file/tasks/tests/04-symlink.yml +++ b/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,7 +41,7 @@ - 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' @@ -49,7 +49,7 @@ 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' diff --git a/tests/integration/targets/ini_file/tasks/tests/05-ignore_spaces.yml b/tests/integration/targets/ini_file/tasks/tests/05-ignore_spaces.yml new file mode 100644 index 0000000000..3c4b068fb7 --- /dev/null +++ b/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" diff --git a/tests/unit/plugins/modules/test_ini_file.py b/tests/unit/plugins/modules/test_ini_file.py new file mode 100644 index 0000000000..a65a9c3261 --- /dev/null +++ b/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)