Repository: ansible/ansible
Pull request: Add state option to git_config module
State present/absent option works like set/unset option for 'git config'.

##### SUMMARY
Adding a state option to git_config module. State can be 'present' or 'absent'.
Works like 'git config' when 'present' and like 'git config --unset' when 'absent'.

Fixes #28567

##### ISSUE TYPE
- Feature Pull Request

##### COMPONENT NAME
git_config


Problem statement:
git_config module has no way to remove config settings
<!---
Verify first that your issue/request is not already reported on GitHub.
Also test if the latest release, and master branch are affected too.
-->

##### ISSUE TYPE
<!--- Pick one below and delete the rest: -->
 - Feature Idea

##### COMPONENT NAME
<!--- Name of the module/plugin/task/feature -->
git_config

##### ANSIBLE VERSION
<!--- Paste verbatim output from “ansible --version” between quotes below -->
```
ansible 2.3.1.0
  config file = /Users/cherdt/ansible/ansible.cfg
  configured module search path = Default w/o overrides
  python version = 2.7.13 (default, Jul 18 2017, 09:17:00) [GCC 4.2.1 Compatible Apple LLVM 8.1.0 (clang-802.0.42)]
```

##### CONFIGURATION
<!---
Mention any settings you have changed/added/removed in ansible.cfg
(or using the ANSIBLE_* environment variables).
-->
N/A

##### OS / ENVIRONMENT
<!---
Mention the OS you are running Ansible from, and the OS you are
managing, or say “N/A” for anything that is not platform-specific.
Also mention the specific version of what you are trying to control,
e.g. if this is a network bug the version of firmware on the network device.
-->
N/A

##### SUMMARY
<!--- Explain the problem briefly -->
You can add git configuration settings via the git_config module, but there is currently no way to remove settings via the git_config module.

In general I think it is desirable to have the ability to undo any state changes made with a module via the same module.

##### STEPS TO REPRODUCE
<!---
For bugs, show exactly how to reproduce the problem, using a minimal test-case.
For new features, show how the feature would be used.
-->
N/A
<!--- Paste example playbooks or commands between quotes below -->
```yaml

```

<!--- You can also paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
<!--- What did you expect to happen when running the steps above? -->
N/A

##### ACTUAL RESULTS
<!--- What actually happened? If possible run with extra verbosity (-vvvv) -->
N/A

<!--- Paste verbatim command output between quotes below -->
```

```

Hints from issue discussion:
cc @djmattyg007
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
I completely agree! Unfortunately I won't have time to create a patch anytime soon, but I'll happily review any pull requests that are put forward.
I've opened pull request #28861 to add this feature to git_config.
Any news about that?
The PR in its current state still requires some additional changes based on feedback. Chief among those changes is proper check mode support.
cc @mgedmin
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->

Commit message: Add state option to git_config module

State present/absent option works like --set/--unset option for 'git config'.

Patch to review:
diff --git a/lib/ansible/modules/source_control/git_config.py b/lib/ansible/modules/source_control/git_config.py
index 620a885b60a11c..13a4851c66b151 100644
--- a/lib/ansible/modules/source_control/git_config.py
+++ b/lib/ansible/modules/source_control/git_config.py
@@ -51,6 +51,12 @@
         also specify the repo parameter. It defaults to system only when
         not using I(list_all)=yes.
     choices: [ "local", "global", "system" ]
+  state:
+    description:
+      - Indicates the setting should be set/unset.
+    choices: [ 'present', 'absent' ]
+    default: 'present'
+    version_added: '2.8'
   value:
     description:
       - When specifying the name of a single setting, supply a value to
@@ -69,6 +75,12 @@
     scope: global
     value: status
 
+# Unset some settings in ~/.gitconfig
+- git_config:
+    name: alias.ci
+    scope: global
+    state: absent
+
 # Or system-wide:
 - git_config:
     name: alias.remotev
@@ -149,9 +161,10 @@ def main():
             name=dict(type='str'),
             repo=dict(type='path'),
             scope=dict(required=False, type='str', choices=['local', 'global', 'system']),
+            state=dict(required=False, type='str', default='present', choices=['present', 'absent']),
             value=dict(required=False)
         ),
-        mutually_exclusive=[['list_all', 'name'], ['list_all', 'value']],
+        mutually_exclusive=[['list_all', 'name'], ['list_all', 'value'], ['list_all', 'state']],
         required_if=[('scope', 'local', ['repo'])],
         required_one_of=[['list_all', 'name']],
         supports_check_mode=True,
@@ -175,6 +188,11 @@ def main():
     else:
         scope = 'system'
 
+    if params['state'] == 'absent':
+        unset = 'unset'
+    else:
+        unset = None
+
     if params['value']:
         new_value = params['value']
     else:
@@ -212,13 +230,19 @@ def main():
             k, v = value.split('=', 1)
             config_values[k] = v
         module.exit_json(changed=False, msg='', config_values=config_values)
-    elif not new_value:
+    elif not new_value and not unset:
         module.exit_json(changed=False, msg='', config_value=out.rstrip())
+    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:
             module.exit_json(changed=False, msg="")
 
+    if unset:
+        args.insert(len(args) - 1, "--" + unset)
+        new_value = ''  # No new value when unset
+
     if not module.check_mode:
         new_value_quoted = shlex_quote(new_value)
         cmd = ' '.join(args + [new_value_quoted])


Code review comments:
diff_hunk: @@ -212,13 +230,19 @@ def main():
             k, v = value.split('=', 1)
             config_values[k] = v
         module.exit_json(changed=False, msg='', config_values=config_values)
-    elif not new_value:
+    elif not new_value and not unset:
         module.exit_json(changed=False, msg='', config_value=out.rstrip())
+    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:
             module.exit_json(changed=False, msg="")
 
+    if unset:
+        args.insert(len(args) - 1, "--" + unset)
+        new_value = ''  # No new value when unset
path: lib/ansible/modules/source_control/git_config.py
text: @user1:
This will still result in a command like `git config --unset foo ''`.  According to the git config man page, that extra argument is a "value_regex" and its presence means only those values matching this regex will be unset.

Luckily the empty string is a regex that matches everything, so it all works out fine in the end.

@author:
You're right. I didn't see it.
Taken into account.