Repository: ansible/ansible
Pull request: ansible-config dedupe ini plugin entries
fixes #82481
##### ISSUE TYPE
- Bugfix Pull Request


Problem statement:
ansible-config init -t all generate many multiple duplicate option
### Summary

I use `ansible-config init -t all` to gernerate ansible.cfg

there have many duplicate option inside

### Issue Type

Bug Report

### Component Name

pip

### Ansible Version

```console
$ ansible --version
ansible [core 2.16.2]
  config file = None
  configured module search path = ['/home/owan/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/owan/.pyenv/versions/3.11.7/envs/venv_nebula-aio_17.30/lib/python3.11/site-packages/ansible
  ansible collection location = /home/owan/.ansible/collections:/usr/share/ansible/collections
  executable location = /home/owan/.pyenv/versions/venv_nebula-aio_17.30/bin/ansible
  python version = 3.11.7 (main, Jan  4 2024, 10:54:58) [GCC 11.4.0] (/home/owan/.pyenv/versions/3.11.7/envs/venv_nebula-aio_17.30/bin/python3.11)
  jinja version = 3.1.2
  libyaml = True


$ ansible-community --version
Ansible community version 9.1.0
```


### Configuration

```console
no
```


### OS / Environment

ubuntu 22.04

### Steps to Reproduce

ansible-config init -t all | sort | uniq -dc
output
```
      2 async_dir=%USERPROFILE%\.ansible_async
      2 # Before Ansible 2.8, this was set to C(remote_tmp + "\.ansible_async").
      2 # (bool) Configure the result format to be more readable
      4 # (boolean) Pipelining reduces the number of connection operations required to execute a module on the remote server, by executing many Ansible modules without actual file transfers.
      2 # (boolean) Set this to "False" if you want to avoid host key checking by the underlying tools Ansible uses to connect to the host
      2 # (boolean) TODO: write it
      2 callback_format_pretty=
      2 callback_result_format=json
      2 display_skipped_hosts=True
      2 fact_caching_connection=
      2 fact_caching_timeout=86400
      2 flags=
      3 host_key_checking=True
      4 # However this can conflict with privilege escalation (become). For example, when using sudo operations you must first disable 'requiretty' in the sudoers file for the target hosts, which is why this feature is disabled by default.
      3 # If executable, it will be run and the resulting stdout will be used as the password.
      2 # (integer) Expiration timeout for the cache plugin data
      2 # (int) Remote port to connect to.
      2 # (list) Check all of these extensions when looking for 'variable' files which should be YAML or JSON or vaulted versions of these.
      2 look_for_keys=True
      3 password=
      5 pipelining=False
      3 private_key_file=
      2 remote_port=
      2 remote_tmp=%TEMP%
      3 remote_user=
      2 # Setting this option to V(True) will force V(json) and V(yaml) results to always be pretty printed regardless of verbosity.
      2 show_custom_stats=False
      2 ssh_common_args=
      2 ssh_extra_args=
      2 # (str) Define the task result format used in the callback output.
      2 # (string) Directory in which ansible will keep async job information.
      3 # (string) Only used in parsing ProxyCommand for use in this plugin.
      2 # (string) Path to private key file to use for authentication.
      2 # (string) Temporary directory to use on targets when copying files to the host.
      3 # (string) User you 'become' to execute the task
      4 # The existing configuration settings are still accepted with the inventory plugin adding additional options from inventory and fact cache configuration.
      2 # The output contains these formats interspersed with other non-machine parsable data.
      2 # These formats do not cause the callback to emit valid JSON or YAML formats.
      4 # These warnings can be silenced by adjusting this setting to False.
      2 # This affects vars_files, include_vars, inventory and vars plugins among others.
      4 # This can result in a very significant performance improvement when enabled.
      5 # This message will be removed in 2.16.
      5 # This setting has been moved to the individual inventory plugins as a plugin option :ref:`inventory_plugins`.
      4 timeout=10
      2 use_persistent_connections=False
      2 user=root
      2 # When O(result_format) is set to V(yaml) this option defaults to V(True), and defaults to V(False) when configured to V(json).
      2 # When set to V(True) and used with the V(yaml) result format, this option will modify module responses in an attempt to produce a more human friendly output at the expense of correctness, and should not be relied upon to aid in writing variable manipulations or conditionals. For correctness, set this option to V(False) or set O(result_format) to V(json).
      2 yaml_valid_extensions=.yml, .yaml, .json
```

### Expected Results

no dupliacate 

### Actual Results

```console
many option duplicate
```


### Code of Conduct

- [X] I agree to follow the Ansible Code of Conduct

Hints from issue discussion:
Files identified in the description:

* [`lib/ansible/modules/pip.py`](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/pip.py)

If these files are incorrect, please update the `component name` section of the description or use the [component](https://github.com/ansible/ansibotmini#commands) bot command.

<!--- boilerplate: components_banner --->

An `ini` file has  `[section]`s so you can have 'duplicate keys' but they still mean different things if under different sections.

Most of those will be a 'general' setting under `[defaults]` and plugin specific ones under the `[plugin_specific_section]`.
most duplicate keys is in same section `defaults`
![image](https://github.com/ansible/ansible/assets/145188628/dced1edd-7ce9-49cc-bfa5-2c225c1d549e)

That one I could verify, it seems that we missed de-duping on plugins adding to common sections plugins.

Commit message: ansible-config dedupe ini plugin entries

 added test for ini file integrity, also ensuring no dupes

Patch to review:
diff --git a/changelogs/fragments/dedupe_config_init.yml b/changelogs/fragments/dedupe_config_init.yml
new file mode 100644
index 00000000000000..16306fc13fad5d
--- /dev/null
+++ b/changelogs/fragments/dedupe_config_init.yml
@@ -0,0 +1,2 @@
+bugfixes:
+  - ansible-config init will now dedupe ini entries from plugins.
diff --git a/lib/ansible/cli/config.py b/lib/ansible/cli/config.py
index c2124e210aeab2..e7457c18f434a9 100755
--- a/lib/ansible/cli/config.py
+++ b/lib/ansible/cli/config.py
@@ -313,7 +313,7 @@ def _get_settings_vars(self, settings, subkey):
 
         return data
 
-    def _get_settings_ini(self, settings):
+    def _get_settings_ini(self, settings, seen):
 
         sections = {}
         for o in sorted(settings.keys()):
@@ -326,7 +326,7 @@ def _get_settings_ini(self, settings):
 
             if not opt.get('description'):
                 # its a plugin
-                new_sections = self._get_settings_ini(opt)
+                new_sections = self._get_settings_ini(opt, seen)
                 for s in new_sections:
                     if s in sections:
                         sections[s].extend(new_sections[s])
@@ -342,37 +342,45 @@ def _get_settings_ini(self, settings):
 
             if 'ini' in opt and opt['ini']:
                 entry = opt['ini'][-1]
+                if entry['section'] not in seen:
+                    seen[entry['section']] = []
                 if entry['section'] not in sections:
                     sections[entry['section']] = []
 
-                default = opt.get('default', '')
-                if opt.get('type', '') == 'list' and not isinstance(default, string_types):
-                    # python lists are not valid ini ones
-                    default = ', '.join(default)
-                elif default is None:
-                    default = ''
+                # avoid dupes
+                if entry['key'] not in seen[entry['section']]:
+                    seen[entry['section']].append(entry['key'])
+
+                    default = opt.get('default', '')
+                    if opt.get('type', '') == 'list' and not isinstance(default, string_types):
+                        # python lists are not valid ini ones
+                        default = ', '.join(default)
+                    elif default is None:
+                        default = ''
+
+                    if context.CLIARGS['commented']:
+                        entry['key'] = ';%s' % entry['key']
 
-                if context.CLIARGS['commented']:
-                    entry['key'] = ';%s' % entry['key']
+                    key = desc + '\n%s=%s' % (entry['key'], default)
 
-                key = desc + '\n%s=%s' % (entry['key'], default)
-                sections[entry['section']].append(key)
+                    sections[entry['section']].append(key)
 
         return sections
 
     def execute_init(self):
         """Create initial configuration"""
 
+        seen = {}
         data = []
         config_entries = self._list_entries_from_args()
         plugin_types = config_entries.pop('PLUGINS', None)
 
         if context.CLIARGS['format'] == 'ini':
-            sections = self._get_settings_ini(config_entries)
+            sections = self._get_settings_ini(config_entries, seen)
 
             if plugin_types:
                 for ptype in plugin_types:
-                    plugin_sections = self._get_settings_ini(plugin_types[ptype])
+                    plugin_sections = self._get_settings_ini(plugin_types[ptype], seen)
                     for s in plugin_sections:
                         if s in sections:
                             sections[s].extend(plugin_sections[s])
diff --git a/test/integration/targets/ansible-config/aliases b/test/integration/targets/ansible-config/aliases
new file mode 100644
index 00000000000000..1d28bdb2aa3a36
--- /dev/null
+++ b/test/integration/targets/ansible-config/aliases
@@ -0,0 +1,2 @@
+shippable/posix/group5
+context/controller
diff --git a/test/integration/targets/ansible-config/files/ini_dupes.py b/test/integration/targets/ansible-config/files/ini_dupes.py
new file mode 100755
index 00000000000000..5f7337b197d081
--- /dev/null
+++ b/test/integration/targets/ansible-config/files/ini_dupes.py
@@ -0,0 +1,10 @@
+#!/usr/bin/env python
+
+import configparser
+import sys
+
+
+ini_file = sys.argv[1]
+print(ini_file)
+c = configparser.ConfigParser(strict=True, inline_comment_prefixes=(';',))
+c.read_file(open(ini_file))
diff --git a/test/integration/targets/ansible-config/tasks/main.yml b/test/integration/targets/ansible-config/tasks/main.yml
new file mode 100644
index 00000000000000..be9e87e539071f
--- /dev/null
+++ b/test/integration/targets/ansible-config/tasks/main.yml
@@ -0,0 +1,21 @@
+- name: test ansible-config for valid output and no dupes
+  block:
+    - name: Create temporary file
+      tempfile:
+        path: '{{output_dir}}'
+        state: file
+        suffix: temp.ini
+      register: ini_tempfile
+
+    - name: run config full dump
+      shell: ansible-config init -t all > {{ini_tempfile.path}}
+
+    - name: run ini tester
+      shell: "{{ansible_playbook_python}} '{{role_path}}/files/ini_dupes.py' '{{ini_tempfile.path}}'"
+      ignore_errors: true
+      register: test_ini
+
+    - name: check results
+      assert:
+        that:
+            - test_ini is success
diff --git a/test/sanity/ignore.txt b/test/sanity/ignore.txt
index c4339bcdd54822..047d05611f3608 100644
--- a/test/sanity/ignore.txt
+++ b/test/sanity/ignore.txt
@@ -89,6 +89,7 @@ lib/ansible/plugins/inventory/advanced_host_list.py pylint:arguments-renamed
 lib/ansible/plugins/inventory/host_list.py pylint:arguments-renamed
 lib/ansible/utils/collection_loader/_collection_finder.py pylint:deprecated-class
 lib/ansible/utils/collection_loader/_collection_meta.py pylint:deprecated-class
+test/integration/targets/ansible-config/files/ini_dupes.py boilerplate
 test/integration/targets/ansible-test-sanity/ansible_collections/ns/col/tests/integration/targets/hello/files/bad.py pylint:ansible-bad-function # ignore, required for testing
 test/integration/targets/ansible-test-sanity/ansible_collections/ns/col/tests/integration/targets/hello/files/bad.py pylint:ansible-bad-import-from # ignore, required for testing
 test/integration/targets/ansible-test-sanity/ansible_collections/ns/col/tests/integration/targets/hello/files/bad.py pylint:ansible-bad-import # ignore, required for testing


Code review comments:
diff_hunk: @@ -89,6 +89,7 @@ lib/ansible/plugins/inventory/advanced_host_list.py pylint:arguments-renamed
 lib/ansible/plugins/inventory/host_list.py pylint:arguments-renamed
 lib/ansible/utils/collection_loader/_collection_finder.py pylint:deprecated-class
 lib/ansible/utils/collection_loader/_collection_meta.py pylint:deprecated-class
+test/integration/targets/ansible-config/files/ini_dupes.py boilerplate
path: test/sanity/ignore.txt
text: @user1:
Why not just add the boilerplate?

diff_hunk: @@ -0,0 +1,21 @@
+- name: test ansible-config for valid output and no dupes
+  block:
+    - name: Create temporary file
+      tempfile:
+        path: '{{output_dir}}'
+        state: file
+        suffix: temp.ini
+      register: ini_tempfile
+
+    - name: run config full dump
+      shell: ansible-config init -t all > {{ini_tempfile.path}}
+
+    - name: run ini tester
+      shell: "{{ansible_playbook_python}} '{{role_path}}/files/ini_dupes.py' '{{ini_tempfile.path}}'"
+      ignore_errors: true
path: test/integration/targets/ansible-config/tasks/main.yml
text: @user1:
Unless we are asserting the output we should be able to remove the assert check and this line