Breaking change in ini_file removes comments

[rightaway]

Summary

Setting option: opt and value: value now removes a line matching it with a comment.

If the file has a description like this the whole # opt: val line will be removed. It's new breaking behavior. Not good because the removed line can be useful documentation.

# Set this value
# opt: val

Issue Type

Bug Report

Component Name

ini_file

Ansible Version

$ ansible --version
ansible [core 2.15.4]
  config file = /etc/ansible/ansible.cfg
  ansible python module location = /usr/lib/python3.11/site-packages/ansible
  executable location = /usr/bin/ansible
  python version = 3.11.5 (main, Sep  2 2023, 14:16:33) [GCC 13.2.1 20230801] (/usr/bin/python)
  jinja version = 3.1.2
  libyaml = True

Community.general Version

$ ansible-galaxy collection list community.general

Configuration

$ ansible-config dump --only-changed

OS / Environment

No response

Steps to Reproduce

Expected Results

Doesn't remove the line.

Actual Results

Code of Conduct

• I agree to follow the Ansible Code of Conduct

[ansibullbot]

Files identified in the description:

• plugins/modules/ini_file.py

If these files are incorrect, please update the component name section of the description or use the !component bot command.

click here for bot help

[ansibullbot]

cc @jpmens @noseka1
click here for bot help

[felixfontein]

This is a result of #6575. As you can see in the discussion of that PR, depending on how the module was used it was already doing that before, and that behavior got broken at some point, so this change was in fact reverting an earlier breaking change.

If someone wants to implement that, it would probably be good to be able to configure the behavior, since both behaviors (using 'commented-out' options, or keeping them) have both good reasons for and against.

[njutn95]

@felixfontein I've made a PR (#7401) to make this a configurable option. @rightaway please confirm if this is what you were looking for.

[rightaway]

Yes but please make modify_inactive_option false as default. It worked that way for years and most people have probably not noticed yet that it changed because it was so recent and are expecting old behavior. The recent change was breaking and it needs to go back to original to prevent destroying original configuration for thousands of files.

[felixfontein]

For the reason explained above (#7297 (comment)) the behavior it has now was intended for a long time, so it should not be disabled by default.

[njutn95]

@rightaway While I sort of agree with you that #6575 might be a breaking change (even though it was a fix of a breaking change), this has already undergone two 'breaking changes', making a third one will make things even worse IMO, especially considering that it's been half a year since then, so it's not a bug fix anymore.

[rightaway]

Can a global option be added to ansible config so it doesn't need to be updated in thousands of places and many repositories without destroying original configuration?

[felixfontein]

No, there's no way to configure something like that for modules, but you can use module defaults to set that option for a play.

[rightaway]

Default should never be something destructive that loses information. Merge was hasty. Is there RFC process for community to discuss these things for future breaking changes?

[felixfontein]

The process of proposing changes to a module is a Pull Request on GitHub. If you want to be informed for new PRs for the ini_file module, add yourself to notify: for ini_file here.