Repository: ansible/ansible
Pull request: Fix Gitlab module deprecation warning
##### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
Fix and enchance deprecation warning for gitlab modules.
Also fix `required` parameters that should not be required.

<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
Fixes #59976 

##### ISSUE TYPE
<!--- Pick one below and delete the rest -->
- Bugfix Pull Request

##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below -->
gitlab_runner
gitlab_user
gitlab_project
gitlab_group
gitlab_hook
gitlab_deploy_key

##### ADDITIONAL INFORMATION
<!--- Include additional information to help people understand the change here -->
<!--- A step-by-step reproduction of the problem is helpful if there is no related issue -->

<!--- Paste verbatim command output below, e.g. before and after your change -->
```paste below

```


Problem statement:
gitlab_runner module: Misleading deprecation warnings in Ansible 2.8
<!--- Verify first that your issue is not already reported on GitHub -->
<!--- Also test if the latest release and devel branch are affected too -->
<!--- Complete *all* sections as described, this form is processed automatically -->

##### SUMMARY
The gitlab_runner module throws deprecation warnings for an option I don't use and another option that is not actually deprecated

##### ISSUE TYPE
- Bug Report

##### COMPONENT NAME
<!--- Write the short name of the module, plugin, task or feature below, use your best guess if unsure -->
'gitlab_runner' module

##### ANSIBLE VERSION
<!--- Paste verbatim output from "ansible --version" between quotes -->
```
ansible 2.8.3
  config file = /etc/ansible/ansible.cfg
  configured module search path = ['/home/m/.ansible/plugins/modules', '/usr/share/ansible/plugins/modules']
  ansible python module location = /home/m/.local/lib/python3.6/site-packages/ansible
  executable location = /home/m/.local/bin/ansible
  python version = 3.6.8 (default, Jan 14 2019, 11:02:34) [GCC 8.0.1 20180414 (experimental) [trunk revision 259383]]
```

##### CONFIGURATION
<!--- Paste verbatim output from "ansible-config dump --only-changed" between quotes -->
```paste below
DEFAULT_REMOTE_USER(/etc/ansible/ansible.cfg) = root
HOST_KEY_CHECKING(/etc/ansible/ansible.cfg) = False
```

##### OS / ENVIRONMENT
<!--- Provide all relevant information below, e.g. target OS versions, network device firmware, etc. -->
Target: Debian Buster, freshly installed. Kernel 4.19.0-5-amd64, Python versions 2.7.16 and 3.7.3

##### STEPS TO REPRODUCE
<!--- Describe exactly how to reproduce the problem, using a minimal test-case -->
Create a task using the gitlab_runner module. Here is the task I use:
<!--- Paste example playbooks or commands between quotes below -->
```
m@void:~/work/ansible-default-deployment$ cat roles/gitlab-runner/tasks/main.yml 
---
- name: Install python-gitlab via pip
  pip:
      name: python-gitlab
      state: present

- name: Register runner
  gitlab_runner:
      url: <redacted>
      api_token: "{{ gitlab_api_token }}"
      registration_token: "{{ gitlab_runner_registration_token }}"
      description: "Auto-created runner for project {{ project_name }}"
      state: present
      active: False
      tag_list: ['docker', '{{project_name}}']
      run_untagged: False
      locked: False
```

<!--- HINT: You can paste gist.github.com links for larger files -->

##### EXPECTED RESULTS
<!--- Describe what you expected to happen when running the steps above -->
- I expect no deprecation warning for the `url` parameter because it is described as a required parameter with no possible alternative, and it is not marked as deprecated even in the 'devel' docs
- I expect no deprecation warning for the `login_token`  parameter because I don't use it

##### ACTUAL RESULTS
<!--- Describe what actually happened. If possible run with extra verbosity (-vvvv) -->
This is the output of running the aforenmentioned task:
<!--- Paste verbatim command output between quotes -->
```
TASK [gitlab-runner : Register runner] *************************************************************************************************************************************************************************************************************
[DEPRECATION WARNING]: Param 'url' is deprecated. See the module docs for more information. This feature will be removed in version 2.10. Deprecation warnings can be disabled by setting deprecation_warnings=False in ansible.cfg.
[DEPRECATION WARNING]: Aliases 'login_token' are deprecated. This feature will be removed in version 2.10. Deprecation warnings can be disabled by setting deprecation_warnings=False in ansible.cfg.
changed: [redacted]
```

Hints from issue discussion:
Files identified in the description:
* [`lib/ansible/modules/source_control/gitlab_runner.py`](https://github.com/ansible/ansible/blob/devel/lib/ansible/modules/source_control/gitlab_runner.py)

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

[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: components_banner --->
cc @Lunik @SamyCoenen @Shaps @dj-wasabi @jlozadad @marwatk @waheedi
[click here for bot help](https://github.com/ansible/ansibullbot/blob/master/ISSUE_HELP.md)
<!--- boilerplate: notify --->
`url` and `login_token` will be removed in 2.10

We should fix :

- `url` shouldn't be required because you should use `api_url`.
- `login_token` alias deprecation should not show if the parameter is not used.
- see if this issue could show up In other GitLab modules

Commit message: gitlab modules : Fix deprecation warnings and parameters

Also preparing for 2.10 deprecation

Patch to review:
diff --git a/lib/ansible/modules/source_control/gitlab_deploy_key.py b/lib/ansible/modules/source_control/gitlab_deploy_key.py
index d7bc52d9ab95f8..9a02836843c448 100644
--- a/lib/ansible/modules/source_control/gitlab_deploy_key.py
+++ b/lib/ansible/modules/source_control/gitlab_deploy_key.py
@@ -241,7 +241,9 @@ def deleteDeployKey(self):
 def deprecation_warning(module):
     deprecated_aliases = ['private_token', 'access_token']
 
-    module.deprecate("Aliases \'{aliases}\' are deprecated".format(aliases='\', \''.join(deprecated_aliases)), "2.10")
+    for aliase in deprecated_aliases:
+        if aliase in module.params:
+            module.deprecate("Aliases \'{aliase}\' is deprecated".format(aliase=aliase), "2.10")
 
 
 def main():
diff --git a/lib/ansible/modules/source_control/gitlab_group.py b/lib/ansible/modules/source_control/gitlab_group.py
index 8203db7a338355..78ab024a683bd4 100644
--- a/lib/ansible/modules/source_control/gitlab_group.py
+++ b/lib/ansible/modules/source_control/gitlab_group.py
@@ -32,7 +32,6 @@
   server_url:
     description:
       - The URL of the GitLab server, with protocol (i.e. http or https).
-    required: true
     type: str
   login_user:
     description:
@@ -270,7 +269,9 @@ def existsGroup(self, project_identifier):
 def deprecation_warning(module):
     deprecated_aliases = ['login_token']
 
-    module.deprecate("Aliases \'{aliases}\' are deprecated".format(aliases='\', \''.join(deprecated_aliases)), "2.10")
+    for aliase in deprecated_aliases:
+        if aliase in module.params:
+            module.deprecate("Aliases \'{aliase}\' is deprecated".format(aliase=aliase), "2.10")
 
 
 def main():
@@ -304,7 +305,8 @@ def main():
             ['login_user', 'login_password'],
         ],
         required_one_of=[
-            ['api_username', 'api_token', 'login_user', 'login_token']
+            ['api_username', 'api_token', 'login_user', 'login_token'],
+            ['server_url', 'api_url']
         ],
         supports_check_mode=True,
     )
diff --git a/lib/ansible/modules/source_control/gitlab_hook.py b/lib/ansible/modules/source_control/gitlab_hook.py
index f254580deff8fd..20a5ef377ad234 100644
--- a/lib/ansible/modules/source_control/gitlab_hook.py
+++ b/lib/ansible/modules/source_control/gitlab_hook.py
@@ -297,9 +297,11 @@ def deleteHook(self):
 
 
 def deprecation_warning(module):
-    deprecated_aliases = ['private_token', 'access_token']
+    deprecated_aliases = ['private_token', 'access_token', 'enable_ssl_verification']
 
-    module.deprecate("Aliases \'{aliases}\' are deprecated".format(aliases='\', \''.join(deprecated_aliases)), "2.10")
+    for aliase in deprecated_aliases:
+        if aliase in module.params:
+            module.deprecate("Aliases \'{aliase}\' is deprecated".format(aliase=aliase), "2.10")
 
 
 def main():
diff --git a/lib/ansible/modules/source_control/gitlab_project.py b/lib/ansible/modules/source_control/gitlab_project.py
index f867a9d2c8e5a1..08bebe7f7402f1 100644
--- a/lib/ansible/modules/source_control/gitlab_project.py
+++ b/lib/ansible/modules/source_control/gitlab_project.py
@@ -33,7 +33,6 @@
   server_url:
     description:
       - The URL of the GitLab server, with protocol (i.e. http or https).
-    required: true
     type: str
   login_user:
     description:
@@ -289,7 +288,9 @@ def existsProject(self, namespace, path):
 def deprecation_warning(module):
     deprecated_aliases = ['login_token']
 
-    module.deprecate("Aliases \'{aliases}\' are deprecated".format(aliases='\', \''.join(deprecated_aliases)), "2.10")
+    for aliase in deprecated_aliases:
+        if aliase in module.params:
+            module.deprecate("Aliases \'{aliase}\' is deprecated".format(aliase=aliase), "2.10")
 
 
 def main():
@@ -328,7 +329,8 @@ def main():
             ['login_user', 'login_password'],
         ],
         required_one_of=[
-            ['api_username', 'api_token', 'login_user', 'login_token']
+            ['api_username', 'api_token', 'login_user', 'login_token'],
+            ['server_url', 'api_url']
         ],
         supports_check_mode=True,
     )
diff --git a/lib/ansible/modules/source_control/gitlab_runner.py b/lib/ansible/modules/source_control/gitlab_runner.py
index ed350c0d82bfda..a419fa2c9c5d5d 100644
--- a/lib/ansible/modules/source_control/gitlab_runner.py
+++ b/lib/ansible/modules/source_control/gitlab_runner.py
@@ -41,7 +41,6 @@
   url:
     description:
       - The URL of the GitLab server, with protocol (i.e. http or https).
-    required: true
     type: str
   api_token:
     description:
@@ -286,15 +285,17 @@ def deleteRunner(self):
 
 
 def deprecation_warning(module):
-    deprecated_aliases = ['login_token']
+    deprecated_aliases = ['private_token']
 
-    module.deprecate("Aliases \'{aliases}\' are deprecated".format(aliases='\', \''.join(deprecated_aliases)), "2.10")
+    for aliase in deprecated_aliases:
+        if aliase in module.params:
+            module.deprecate("Aliases \'{aliase}\' is deprecated".format(aliase=aliase), "2.10")
 
 
 def main():
     argument_spec = basic_auth_argument_spec()
     argument_spec.update(dict(
-        url=dict(type='str', required=True, removed_in_version="2.10"),
+        url=dict(type='str', removed_in_version="2.10"),
         api_token=dict(type='str', no_log=True, aliases=["private_token"]),
         description=dict(type='str', required=True, aliases=["name"]),
         active=dict(type='bool', default=True),
@@ -319,14 +320,16 @@ def main():
             ['login_user', 'login_password'],
         ],
         required_one_of=[
-            ['api_username', 'api_token']
+            ['api_username', 'api_token'],
+            ['api_url', 'url']
         ],
         supports_check_mode=True,
     )
 
     deprecation_warning(module)
 
-    url = re.sub('/api.*', '', module.params['url'])
+    if module.params['url'] is not None:
+        url = re.sub('/api.*', '', module.params['url'])
 
     api_url = module.params['api_url']
     validate_certs = module.params['validate_certs']
diff --git a/lib/ansible/modules/source_control/gitlab_user.py b/lib/ansible/modules/source_control/gitlab_user.py
index 9aca86923cf8d1..270c3989aa33f7 100644
--- a/lib/ansible/modules/source_control/gitlab_user.py
+++ b/lib/ansible/modules/source_control/gitlab_user.py
@@ -34,7 +34,6 @@
   server_url:
     description:
       - The URL of the GitLab server, with protocol (i.e. http or https).
-    required: true
     type: str
   login_user:
     description:
@@ -411,13 +410,15 @@ def deleteUser(self):
 def deprecation_warning(module):
     deprecated_aliases = ['login_token']
 
-    module.deprecate("Aliases \'{aliases}\' are deprecated".format(aliases='\', \''.join(deprecated_aliases)), "2.10")
+    for aliase in deprecated_aliases:
+        if aliase in module.params:
+            module.deprecate("Aliases \'{aliase}\' is deprecated".format(aliase=aliase), "2.10")
 
 
 def main():
     argument_spec = basic_auth_argument_spec()
     argument_spec.update(dict(
-        server_url=dict(type='str', required=True, removed_in_version="2.10"),
+        server_url=dict(type='str', removed_in_version="2.10"),
         login_user=dict(type='str', no_log=True, removed_in_version="2.10"),
         login_password=dict(type='str', no_log=True, removed_in_version="2.10"),
         api_token=dict(type='str', no_log=True, aliases=["login_token"]),
@@ -451,7 +452,8 @@ def main():
             ['login_user', 'login_password'],
         ],
         required_one_of=[
-            ['api_username', 'api_token', 'login_user', 'login_token']
+            ['api_username', 'api_token', 'login_user', 'login_token'],
+            ['server_url', 'api_url']
         ],
         supports_check_mode=True,
     )


Code review comments:
diff_hunk: @@ -241,7 +241,9 @@ def deleteDeployKey(self):
 def deprecation_warning(module):
     deprecated_aliases = ['private_token', 'access_token']
 
-    module.deprecate("Aliases \'{aliases}\' are deprecated".format(aliases='\', \''.join(deprecated_aliases)), "2.10")
+    for aliase in deprecated_aliases:
+        if aliase in module.params:
+            module.deprecate("Aliases \'{aliase}\' is deprecated".format(aliase=aliase), "2.10")
path: lib/ansible/modules/source_control/gitlab_deploy_key.py
text: @user1:
```suggestion
            module.deprecate("Alias \'{aliase}\' is deprecated".format(aliase=aliase), "2.10")
```

diff_hunk: @@ -411,13 +410,15 @@ def deleteUser(self):
 def deprecation_warning(module):
     deprecated_aliases = ['login_token']
 
-    module.deprecate("Aliases \'{aliases}\' are deprecated".format(aliases='\', \''.join(deprecated_aliases)), "2.10")
+    for aliase in deprecated_aliases:
+        if aliase in module.params:
+            module.deprecate("Aliases \'{aliase}\' is deprecated".format(aliase=aliase), "2.10")
path: lib/ansible/modules/source_control/gitlab_user.py
text: @user1:
```suggestion
            module.deprecate("Alias \'{aliase}\' is deprecated".format(aliase=aliase), "2.10")
```

diff_hunk: @@ -286,15 +285,17 @@ def deleteRunner(self):
 
 
 def deprecation_warning(module):
-    deprecated_aliases = ['login_token']
+    deprecated_aliases = ['private_token']
 
-    module.deprecate("Aliases \'{aliases}\' are deprecated".format(aliases='\', \''.join(deprecated_aliases)), "2.10")
+    for aliase in deprecated_aliases:
+        if aliase in module.params:
+            module.deprecate("Aliases \'{aliase}\' is deprecated".format(aliase=aliase), "2.10")
path: lib/ansible/modules/source_control/gitlab_runner.py
text: @user1:
```suggestion
            module.deprecate("Alias \'{aliase}\' is deprecated".format(aliase=aliase), "2.10")
```

diff_hunk: @@ -270,7 +269,9 @@ def existsGroup(self, project_identifier):
 def deprecation_warning(module):
     deprecated_aliases = ['login_token']
 
-    module.deprecate("Aliases \'{aliases}\' are deprecated".format(aliases='\', \''.join(deprecated_aliases)), "2.10")
+    for aliase in deprecated_aliases:
+        if aliase in module.params:
+            module.deprecate("Aliases \'{aliase}\' is deprecated".format(aliase=aliase), "2.10")
path: lib/ansible/modules/source_control/gitlab_group.py
text: @user1:
```suggestion
            module.deprecate("Alias \'{aliase}\' is deprecated".format(aliase=aliase), "2.10")
```

diff_hunk: @@ -289,7 +288,9 @@ def existsProject(self, namespace, path):
 def deprecation_warning(module):
     deprecated_aliases = ['login_token']
 
-    module.deprecate("Aliases \'{aliases}\' are deprecated".format(aliases='\', \''.join(deprecated_aliases)), "2.10")
+    for aliase in deprecated_aliases:
+        if aliase in module.params:
+            module.deprecate("Aliases \'{aliase}\' is deprecated".format(aliase=aliase), "2.10")
path: lib/ansible/modules/source_control/gitlab_project.py
text: @user1:
```suggestion
            module.deprecate("Alias \'{aliase}\' is deprecated".format(aliase=aliase), "2.10")
```

diff_hunk: @@ -297,9 +297,11 @@ def deleteHook(self):
 
 
 def deprecation_warning(module):
-    deprecated_aliases = ['private_token', 'access_token']
+    deprecated_aliases = ['private_token', 'access_token', 'enable_ssl_verification']
 
-    module.deprecate("Aliases \'{aliases}\' are deprecated".format(aliases='\', \''.join(deprecated_aliases)), "2.10")
+    for aliase in deprecated_aliases:
+        if aliase in module.params:
+            module.deprecate("Aliases \'{aliase}\' is deprecated".format(aliase=aliase), "2.10")
path: lib/ansible/modules/source_control/gitlab_hook.py
text: @user1:
```suggestion
            module.deprecate("Alias \'{aliase}\' is deprecated".format(aliase=aliase), "2.10")
```