git_config: support multiple values for same name (#7260)

* Refactor the existing git_config.py

* Support multiple values for same name

changelogs/fragments/7242-multi-values-for-same-name-in-git-config.yml

@@ -0,0 +1,4 @@
+minor_changes:
+    - "git_config - allow multiple git configs for the same name with the new ``add_mode`` option (https://github.com/ansible-collections/community.general/pull/7260)."
+    - "git_config - the ``after`` and ``before`` fields in the ``diff`` of the return value can be a list instead of a string in case more configs with the same key are affected (https://github.com/ansible-collections/community.general/pull/7260)."
+    - "git_config - when a value is unset, all configs with the same key are unset (https://github.com/ansible-collections/community.general/pull/7260)."

plugins/modules/git_config.py

@@ -75,6 +75,16 @@ options:
       - When specifying the name of a single setting, supply a value to
         set that setting to the given value.
     type: str
+  add_mode:
+    description:
+      - Specify if a value should replace the existing value(s) or if the new
+        value should be added alongside other values with the same name.
+      - This option is only relevant when adding/replacing values. If O(state=absent) or
+        values are just read out, this option is not considered.
+    choices: [ "add", "replace-all" ]
+    type: str
+    default: "replace-all"
+    version_added: 8.1.0
 '''
 
 EXAMPLES = '''
@@ -118,6 +128,15 @@ EXAMPLES = '''
     name: color.ui
     value: auto
 
+- name: Add several options for the same name
+  community.general.git_config:
+    name: push.pushoption
+    value: "{{ item }}"
+    add_mode: add
+  loop:
+    - merge_request.create
+    - merge_request.draft
+
 - name: Make etckeeper not complaining when it is invoked by cron
   community.general.git_config:
     name: user.email
@@ -178,6 +197,7 @@ def main():
             name=dict(type='str'),
             repo=dict(type='path'),
             file=dict(type='path'),
+            add_mode=dict(required=False, type='str', default='replace-all', choices=['add', 'replace-all']),
             scope=dict(required=False, type='str', choices=['file', 'local', 'global', 'system']),
             state=dict(required=False, type='str', default='present', choices=['present', 'absent']),
             value=dict(required=False),
@@ -197,94 +217,118 @@ def main():
     # Set the locale to C to ensure consistent messages.
     module.run_command_environ_update = dict(LANG='C', LC_ALL='C', LC_MESSAGES='C', LC_CTYPE='C')
 
-    if params['name']:
-        name = params['name']
-    else:
-        name = None
+    name = params['name'] or ''
+    unset = params['state'] == 'absent'
+    new_value = params['value'] or ''
+    add_mode = params['add_mode']
 
-    if params['scope']:
-        scope = params['scope']
-    elif params['list_all']:
-        scope = None
-    else:
-        scope = 'system'
+    scope = determine_scope(params)
+    cwd = determine_cwd(scope, params)
 
-    if params['state'] == 'absent':
-        unset = 'unset'
-        params['value'] = None
-    else:
-        unset = None
+    base_args = [git_path, "config", "--includes"]
 
-    if params['value']:
-        new_value = params['value']
-    else:
-        new_value = None
-
-    args = [git_path, "config", "--includes"]
-    if params['list_all']:
-        args.append('-l')
     if scope == 'file':
-        args.append('-f')
-        args.append(params['file'])
+        base_args.append('-f')
+        base_args.append(params['file'])
     elif scope:
-        args.append("--" + scope)
+        base_args.append("--" + scope)
+
+    list_args = list(base_args)
+
+    if params['list_all']:
+        list_args.append('-l')
+
     if name:
-        args.append(name)
+        list_args.append("--get-all")
+        list_args.append(name)
 
-    if scope == 'local':
-        dir = params['repo']
-    elif params['list_all'] and params['repo']:
-        # Include local settings from a specific repo when listing all available settings
-        dir = params['repo']
-    else:
-        # Run from root directory to avoid accidentally picking up any local config settings
-        dir = "/"
+    (rc, out, err) = module.run_command(list_args, cwd=cwd, expand_user_and_vars=False)
 
-    (rc, out, err) = module.run_command(args, cwd=dir, expand_user_and_vars=False)
     if params['list_all'] and scope and rc == 128 and 'unable to read config file' in err:
         # This just means nothing has been set at the given scope
         module.exit_json(changed=False, msg='', config_values={})
     elif rc >= 2:
         # If the return code is 1, it just means the option hasn't been set yet, which is fine.
-        module.fail_json(rc=rc, msg=err, cmd=' '.join(args))
+        module.fail_json(rc=rc, msg=err, cmd=' '.join(list_args))
+
+    old_values = out.rstrip().splitlines()
 
     if params['list_all']:
-        values = out.rstrip().splitlines()
         config_values = {}
-        for value in values:
+        for value in old_values:
             k, v = value.split('=', 1)
             config_values[k] = v
         module.exit_json(changed=False, msg='', config_values=config_values)
     elif not new_value and not unset:
-        module.exit_json(changed=False, msg='', config_value=out.rstrip())
+        module.exit_json(changed=False, msg='', config_value=old_values[0] if old_values else '')
     elif unset and not out:
         module.exit_json(changed=False, msg='no setting to unset')
-    else:
-        old_value = out.rstrip()
-        if old_value == new_value:
+    elif new_value in old_values and (len(old_values) == 1 or add_mode == "add"):
         module.exit_json(changed=False, msg="")
 
-    if not module.check_mode:
+    # Until this point, the git config was just read and in case no change is needed, the module has already exited.
+
+    set_args = list(base_args)
     if unset:
-            args.insert(len(args) - 1, "--" + unset)
-            cmd = args
+        set_args.append("--unset-all")
+        set_args.append(name)
     else:
-            cmd = args + [new_value]
-        (rc, out, err) = module.run_command(cmd, cwd=dir, ignore_invalid_cwd=False, expand_user_and_vars=False)
+        set_args.append("--" + add_mode)
+        set_args.append(name)
+        set_args.append(new_value)
+
+    if not module.check_mode:
+        (rc, out, err) = module.run_command(set_args, cwd=cwd, ignore_invalid_cwd=False, expand_user_and_vars=False)
         if err:
-            module.fail_json(rc=rc, msg=err, cmd=cmd)
+            module.fail_json(rc=rc, msg=err, cmd=set_args)
+
+    if unset:
+        after_values = []
+    elif add_mode == "add":
+        after_values = old_values + [new_value]
+    else:
+        after_values = [new_value]
 
     module.exit_json(
         msg='setting changed',
         diff=dict(
-            before_header=' '.join(args),
-            before=old_value + "\n",
-            after_header=' '.join(args),
-            after=(new_value or '') + "\n"
+            before_header=' '.join(set_args),
+            before=build_diff_value(old_values),
+            after_header=' '.join(set_args),
+            after=build_diff_value(after_values),
         ),
         changed=True
     )
 
 
+def determine_scope(params):
+    if params['scope']:
+        return params['scope']
+    elif params['list_all']:
+        return ""
+    else:
+        return 'system'
+
+
+def build_diff_value(value):
+    if not value:
+        return "\n"
+    elif len(value) == 1:
+        return value[0] + "\n"
+    else:
+        return value
+
+
+def determine_cwd(scope, params):
+    if scope == 'local':
+        return params['repo']
+    elif params['list_all'] and params['repo']:
+        # Include local settings from a specific repo when listing all available settings
+        return params['repo']
+    else:
+        # Run from root directory to avoid accidentally picking up any local config settings
+        return "/"
+
+
 if __name__ == '__main__':
     main()

tests/integration/targets/git_config/files/gitconfig

@@ -4,3 +4,8 @@
 
 [http]
         proxy = foo
+
+[push]
+pushoption = merge_request.create
+pushoption = merge_request.draft
+pushoption = merge_request.target=foobar

tests/integration/targets/git_config/tasks/get_set_state_present_file.yml

@@ -30,3 +30,4 @@
       - set_result.diff.after == option_value + "\n"
       - get_result is not changed
       - get_result.config_value == option_value
+...

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

@@ -13,6 +13,7 @@
   import_tasks: setup.yml
 
 - block:
+    - import_tasks: set_value.yml
     # testing parameters exclusion: state and list_all
     - import_tasks: exclusion_state_list-all.yml
     # testing get/set option without state
@@ -31,5 +32,7 @@
     - import_tasks: unset_check_mode.yml
     # testing for case in issue #1776
     - import_tasks: set_value_with_tilde.yml
+    - import_tasks: set_multi_value.yml
+    - import_tasks: unset_multi_value.yml
   when: git_installed is succeeded and git_version.stdout is version(git_version_supporting_includes, ">=")
 ...

tests/integration/targets/git_config/tasks/set_multi_value.yml

@@ -0,0 +1,79 @@
+---
+# 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
+
+- import_tasks: setup_no_value.yml
+
+- name: setting value
+  git_config:
+    name: push.pushoption
+    add_mode: add
+    value: "{{ item }}"
+    state: present
+    scope: global
+  loop:
+    - 'merge_request.create'
+    - 'merge_request.draft'
+    - 'merge_request.target=foobar'
+  register: set_result1
+
+- name: setting value
+  git_config:
+    name: push.pushoption
+    add_mode: add
+    value: "{{ item }}"
+    state: present
+    scope: global
+  loop:
+    - 'merge_request.create'
+    - 'merge_request.draft'
+    - 'merge_request.target=foobar'
+  register: set_result2
+
+- name: getting the multi-value
+  git_config:
+    name: push.pushoption
+    scope: global
+  register: get_single_result
+
+- name: getting all values for the single option
+  git_config_info:
+    name: push.pushoption
+    scope: global
+  register: get_all_result
+
+- name: replace-all values
+  git_config:
+    name: push.pushoption
+    add_mode: replace-all
+    value: merge_request.create
+    state: present
+    scope: global
+  register: set_result3
+
+- name: assert set changed and value is correct
+  assert:
+    that:
+      - set_result1.results[0] is changed
+      - set_result1.results[1] is changed
+      - set_result1.results[2] is changed
+      - set_result2.results[0] is not changed
+      - set_result2.results[1] is not changed
+      - set_result2.results[2] is not changed
+      - set_result3 is changed
+      - get_single_result.config_value == 'merge_request.create'
+      - 'get_all_result.config_values == {"push.pushoption": ["merge_request.create", "merge_request.draft", "merge_request.target=foobar"]}'
+
+- name: assert the diffs are also right
+  assert:
+    that:
+      - set_result1.results[0].diff.before == "\n"
+      - set_result1.results[0].diff.after == "merge_request.create\n"
+      - set_result1.results[1].diff.before == "merge_request.create\n"
+      - set_result1.results[1].diff.after == ["merge_request.create", "merge_request.draft"]
+      - set_result1.results[2].diff.before == ["merge_request.create", "merge_request.draft"]
+      - set_result1.results[2].diff.after == ["merge_request.create", "merge_request.draft", "merge_request.target=foobar"]
+      - set_result3.diff.before == ["merge_request.create", "merge_request.draft", "merge_request.target=foobar"]
+      - set_result3.diff.after == "merge_request.create\n"
+...

tests/integration/targets/git_config/tasks/set_value.yml

@@ -0,0 +1,39 @@
+---
+# 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
+
+- import_tasks: setup_no_value.yml
+
+- name: setting value
+  git_config:
+    name: core.name
+    value: foo
+    scope: global
+  register: set_result1
+
+- name: setting another value for same name
+  git_config:
+    name: core.name
+    value: bar
+    scope: global
+  register: set_result2
+
+- name: getting value
+  git_config:
+    name: core.name
+    scope: global
+  register: get_result
+
+- name: assert set changed and value is correct
+  assert:
+    that:
+      - set_result1 is changed
+      - set_result2 is changed
+      - get_result is not changed
+      - get_result.config_value == 'bar'
+      - set_result1.diff.before == "\n"
+      - set_result1.diff.after == "foo\n"
+      - set_result2.diff.before == "foo\n"
+      - set_result2.diff.after == "bar\n"
+...

tests/integration/targets/git_config/tasks/set_value_with_tilde.yml

@@ -11,7 +11,7 @@
     value: '~/foo/bar'
     state: present
     scope: global
-  register: set_result
+  register: set_result1
 
 - name: setting value again
   git_config:
@@ -30,7 +30,7 @@
 - name: assert set changed and value is correct
   assert:
     that:
-      - set_result is changed
+      - set_result1 is changed
       - set_result2 is not changed
       - get_result is not changed
       - get_result.config_value == '~/foo/bar'

tests/integration/targets/git_config/tasks/unset_multi_value.yml

@@ -0,0 +1,28 @@
+---
+# 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
+
+- import_tasks: setup_value.yml
+
+- name: unsetting "push.pushoption"
+  git_config:
+    name: push.pushoption
+    scope: global
+    state: absent
+  register: unset_result
+
+- name: getting all pushoptions values
+  git_config_info:
+    name: push.pushoption
+    scope: global
+  register: get_all_result
+
+- name: assert unsetting muti-values
+  assert:
+    that:
+      - unset_result is changed
+      - 'get_all_result.config_values == {"push.pushoption": []}'
+      - unset_result.diff.before == ["merge_request.create", "merge_request.draft", "merge_request.target=foobar"]
+      - unset_result.diff.after == "\n"
+...