From 07bac1777f5b1c43dd0f8d6689c734e084f200e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?G=C3=BCnther=20Grill?= Date: Sun, 3 Dec 2023 09:41:34 +0100 Subject: [PATCH] git_config: support multiple values for same name (#7260) * Refactor the existing git_config.py * Support multiple values for same name --- ...lti-values-for-same-name-in-git-config.yml | 4 + plugins/modules/git_config.py | 152 +++++++++++------- .../targets/git_config/files/gitconfig | 5 + .../tasks/get_set_state_present_file.yml | 1 + .../targets/git_config/tasks/main.yml | 3 + .../git_config/tasks/set_multi_value.yml | 79 +++++++++ .../targets/git_config/tasks/set_value.yml | 39 +++++ .../git_config/tasks/set_value_with_tilde.yml | 4 +- .../git_config/tasks/unset_multi_value.yml | 28 ++++ 9 files changed, 259 insertions(+), 56 deletions(-) create mode 100644 changelogs/fragments/7242-multi-values-for-same-name-in-git-config.yml create mode 100644 tests/integration/targets/git_config/tasks/set_multi_value.yml create mode 100644 tests/integration/targets/git_config/tasks/set_value.yml create mode 100644 tests/integration/targets/git_config/tasks/unset_multi_value.yml diff --git a/changelogs/fragments/7242-multi-values-for-same-name-in-git-config.yml b/changelogs/fragments/7242-multi-values-for-same-name-in-git-config.yml new file mode 100644 index 0000000000..be3dfdcac9 --- /dev/null +++ b/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)." diff --git a/plugins/modules/git_config.py b/plugins/modules/git_config.py index 46ea7d4f59..a8d2ebe979 100644 --- a/plugins/modules/git_config.py +++ b/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') + elif new_value in old_values and (len(old_values) == 1 or add_mode == "add"): + module.exit_json(changed=False, msg="") + + # 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: + set_args.append("--unset-all") + set_args.append(name) else: - old_value = out.rstrip() - if old_value == new_value: - module.exit_json(changed=False, msg="") + set_args.append("--" + add_mode) + set_args.append(name) + set_args.append(new_value) if not module.check_mode: - if unset: - args.insert(len(args) - 1, "--" + unset) - cmd = args - else: - cmd = args + [new_value] - (rc, out, err) = module.run_command(cmd, cwd=dir, ignore_invalid_cwd=False, expand_user_and_vars=False) + (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() diff --git a/tests/integration/targets/git_config/files/gitconfig b/tests/integration/targets/git_config/files/gitconfig index 92eeb7eb9f..29d3e8a0ef 100644 --- a/tests/integration/targets/git_config/files/gitconfig +++ b/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 diff --git a/tests/integration/targets/git_config/tasks/get_set_state_present_file.yml b/tests/integration/targets/git_config/tasks/get_set_state_present_file.yml index a61ffcc68c..c410bfe189 100644 --- a/tests/integration/targets/git_config/tasks/get_set_state_present_file.yml +++ b/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 +... \ No newline at end of file diff --git a/tests/integration/targets/git_config/tasks/main.yml b/tests/integration/targets/git_config/tasks/main.yml index 4dc72824c8..5fddaf7649 100644 --- a/tests/integration/targets/git_config/tasks/main.yml +++ b/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, ">=") ... diff --git a/tests/integration/targets/git_config/tasks/set_multi_value.yml b/tests/integration/targets/git_config/tasks/set_multi_value.yml new file mode 100644 index 0000000000..8d2710b761 --- /dev/null +++ b/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" +... diff --git a/tests/integration/targets/git_config/tasks/set_value.yml b/tests/integration/targets/git_config/tasks/set_value.yml new file mode 100644 index 0000000000..774e3136a5 --- /dev/null +++ b/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" +... diff --git a/tests/integration/targets/git_config/tasks/set_value_with_tilde.yml b/tests/integration/targets/git_config/tasks/set_value_with_tilde.yml index f78e709bde..3ca9023aad 100644 --- a/tests/integration/targets/git_config/tasks/set_value_with_tilde.yml +++ b/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' diff --git a/tests/integration/targets/git_config/tasks/unset_multi_value.yml b/tests/integration/targets/git_config/tasks/unset_multi_value.yml new file mode 100644 index 0000000000..4cb9dee497 --- /dev/null +++ b/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" +...