fix(modules/gitlab_runner): Use correct argument to list all runners (a…

…nsible-collections#7790)

* fix(modules/gitlab_runner): Use correct argument to list all runners

python-gitlab 4.0.0 removed support for the `as_list=False` parameter.
This functionality is now available as `iterator=True`.

Without this change, the module actually only retrieves the first
20 results, which can lead to non-idempotent behavior, such as
registering a runner again.

* Add changelog entry (ansible-collections#7790)

* gitlab_runner: Check python-gitlab version when listing runners

* gitlab: Add list_all_kwargs variable to module_utils

* refactor(gitlab modules): use list_all_kwargs where it helps (ansible-collections#7790)

I did not change every instance of all=True or all=False, only those
which could obviously benefit from simplifying:

  * Code using `all=True` but then searching for any items that match a
    condition (no need to collect the full list).
  * Code that basically reimplements `all=True` with manual pagination.
    (These could be changed to `all=True`, but `list_all_kwargs` also
    sets per_page to 100, to gather data faster.)

* gitlab_instance_variable: Use list_all_kwargs

* Add new changelog entry for gitlab module changes (ansible-collections#7790)

changelogs/fragments/7790-gitlab-runner-api-pagination.yml

@@ -0,0 +1,8 @@
+bugfixes:
+  - gitlab_runner - fix pagination when checking for existing runners (https://github.com/ansible-collections/community.general/pull/7790).
+
+minor_changes:
+  - gitlab_deploy_key, gitlab_group_members, gitlab_group_variable, gitlab_hook,
+    gitlab_instance_variable, gitlab_project_badge, gitlab_project_variable,
+    gitlab_user - improve API pagination and compatibility with different versions
+    of ``python-gitlab`` (https://github.com/ansible-collections/community.general/pull/7790).

plugins/module_utils/gitlab.py

@@ -21,15 +21,30 @@
 
 import traceback
 
+
+def _determine_list_all_kwargs(version):
+    gitlab_version = LooseVersion(version)
+    if gitlab_version >= LooseVersion('4.0.0'):
+        # 4.0.0 removed 'as_list'
+        return {'iterator': True, 'per_page': 100}
+    elif gitlab_version >= LooseVersion('3.7.0'):
+        # 3.7.0 added 'get_all'
+        return {'as_list': False, 'get_all': True, 'per_page': 100}
+    else:
+        return {'as_list': False, 'all': True, 'per_page': 100}
+
+
 GITLAB_IMP_ERR = None
 try:
     import gitlab
     import requests
     HAS_GITLAB_PACKAGE = True
+    list_all_kwargs = _determine_list_all_kwargs(gitlab.__version__)
 except Exception:
     gitlab = None
     GITLAB_IMP_ERR = traceback.format_exc()
     HAS_GITLAB_PACKAGE = False
+    list_all_kwargs = {}
 
 
 def auth_argument_spec(spec=None):

plugins/modules/gitlab_deploy_key.py

@@ -120,7 +120,7 @@
 from ansible.module_utils.common.text.converters import to_native
 
 from ansible_collections.community.general.plugins.module_utils.gitlab import (
-    auth_argument_spec, find_project, gitlab_authentication, gitlab
+    auth_argument_spec, find_project, gitlab_authentication, gitlab, list_all_kwargs
 )
 
 
@@ -208,8 +208,7 @@ def update_deploy_key(self, deploy_key, arguments):
     @param key_title Title of the key
     '''
     def find_deploy_key(self, project, key_title):
-        deploy_keys = project.keys.list(all=True)
-        for deploy_key in deploy_keys:
+        for deploy_key in project.keys.list(**list_all_kwargs):
             if (deploy_key.title == key_title):
                 return deploy_key
 

plugins/modules/gitlab_group_members.py

@@ -160,7 +160,7 @@
 from ansible.module_utils.basic import AnsibleModule
 
 from ansible_collections.community.general.plugins.module_utils.gitlab import (
-    auth_argument_spec, gitlab_authentication, gitlab
+    auth_argument_spec, gitlab_authentication, gitlab, list_all_kwargs
 )
 
 
@@ -171,16 +171,20 @@ def __init__(self, module, gl):
 
     # get user id if the user exists
     def get_user_id(self, gitlab_user):
-        user_exists = self._gitlab.users.list(username=gitlab_user, all=True)
-        if user_exists:
-            return user_exists[0].id
+        return next(
+            (u.id for u in self._gitlab.users.list(username=gitlab_user, **list_all_kwargs)),
+            None
+        )
 
     # get group id if group exists
     def get_group_id(self, gitlab_group):
-        groups = self._gitlab.groups.list(search=gitlab_group, all=True)
-        for group in groups:
-            if group.full_path == gitlab_group:
-                return group.id
+        return next(
+            (
+                g.id for g in self._gitlab.groups.list(search=gitlab_group, **list_all_kwargs)
+                if g.full_path == gitlab_group
+            ),
+            None
+        )
 
     # get all members in a group
     def get_members_in_a_group(self, gitlab_group_id):

plugins/modules/gitlab_group_variable.py

@@ -206,7 +206,8 @@
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.api import basic_auth_argument_spec
 from ansible_collections.community.general.plugins.module_utils.gitlab import (
-    auth_argument_spec, gitlab_authentication, filter_returned_variables, vars_to_variables
+    auth_argument_spec, gitlab_authentication, filter_returned_variables, vars_to_variables,
+    list_all_kwargs
 )
 
 
@@ -221,14 +222,7 @@ def get_group(self, group_name):
         return self.repo.groups.get(group_name)
 
     def list_all_group_variables(self):
-        page_nb = 1
-        variables = []
-        vars_page = self.group.variables.list(page=page_nb)
-        while len(vars_page) > 0:
-            variables += vars_page
-            page_nb += 1
-            vars_page = self.group.variables.list(page=page_nb)
-        return variables
+        return list(self.group.variables.list(**list_all_kwargs))
 
     def create_variable(self, var_obj):
         if self._module.check_mode:

plugins/modules/gitlab_hook.py

@@ -174,7 +174,7 @@
 from ansible.module_utils.basic import AnsibleModule
 
 from ansible_collections.community.general.plugins.module_utils.gitlab import (
-    auth_argument_spec, find_project, gitlab_authentication
+    auth_argument_spec, find_project, gitlab_authentication, list_all_kwargs
 )
 
 
@@ -271,8 +271,7 @@ def update_hook(self, hook, arguments):
     @param hook_url Url to call on event
     '''
     def find_hook(self, project, hook_url):
-        hooks = project.hooks.list(all=True)
-        for hook in hooks:
+        for hook in project.hooks.list(**list_all_kwargs):
             if (hook.url == hook_url):
                 return hook
 

plugins/modules/gitlab_instance_variable.py

@@ -138,7 +138,8 @@
 from ansible.module_utils.basic import AnsibleModule
 from ansible.module_utils.api import basic_auth_argument_spec
 from ansible_collections.community.general.plugins.module_utils.gitlab import (
-    auth_argument_spec, gitlab_authentication, filter_returned_variables
+    auth_argument_spec, gitlab_authentication, filter_returned_variables,
+    list_all_kwargs
 )
 
 
@@ -149,14 +150,7 @@ def __init__(self, module, gitlab_instance):
         self._module = module
 
     def list_all_instance_variables(self):
-        page_nb = 1
-        variables = []
-        gl_varibales_page = self.instance.variables.list(page=page_nb)
-        while len(gl_varibales_page) > 0:
-            variables += gl_varibales_page
-            page_nb += 1
-            gl_varibales_page = self.instance.variables.list(page=page_nb)
-        return variables
+        return list(self.instance.variables.list(**list_all_kwargs))
 
     def create_variable(self, var_obj):
         if self._module.check_mode:

plugins/modules/gitlab_project_badge.py

@@ -97,15 +97,15 @@
 from ansible.module_utils.basic import AnsibleModule
 
 from ansible_collections.community.general.plugins.module_utils.gitlab import (
-    auth_argument_spec, gitlab_authentication, find_project
+    auth_argument_spec, gitlab_authentication, find_project, list_all_kwargs
 )
 
 
 def present_strategy(module, gl, project, wished_badge):
     changed = False
 
     existing_badge = None
-    for badge in project.badges.list(iterator=True):
+    for badge in project.badges.list(**list_all_kwargs):
         if badge.image_url == wished_badge["image_url"]:
             existing_badge = badge
             break
@@ -135,7 +135,7 @@ def absent_strategy(module, gl, project, wished_badge):
     changed = False
 
     existing_badge = None
-    for badge in project.badges.list(iterator=True):
+    for badge in project.badges.list(**list_all_kwargs):
         if badge.image_url == wished_badge["image_url"]:
             existing_badge = badge
             break

plugins/modules/gitlab_project_variable.py

@@ -225,7 +225,8 @@
 
 
 from ansible_collections.community.general.plugins.module_utils.gitlab import (
-    auth_argument_spec, gitlab_authentication, filter_returned_variables, vars_to_variables
+    auth_argument_spec, gitlab_authentication, filter_returned_variables, vars_to_variables,
+    list_all_kwargs
 )
 
 
@@ -240,14 +241,7 @@ def get_project(self, project_name):
         return self.repo.projects.get(project_name)
 
     def list_all_project_variables(self):
-        page_nb = 1
-        variables = []
-        vars_page = self.project.variables.list(page=page_nb)
-        while len(vars_page) > 0:
-            variables += vars_page
-            page_nb += 1
-            vars_page = self.project.variables.list(page=page_nb)
-        return variables
+        return list(self.project.variables.list(**list_all_kwargs))
 
     def create_variable(self, var_obj):
         if self._module.check_mode:

plugins/modules/gitlab_runner.py

@@ -219,7 +219,7 @@
 from ansible.module_utils.common.text.converters import to_native
 
 from ansible_collections.community.general.plugins.module_utils.gitlab import (
-    auth_argument_spec, gitlab_authentication, gitlab
+    auth_argument_spec, gitlab_authentication, gitlab, list_all_kwargs
 )
 
 
@@ -342,7 +342,7 @@ def update_runner(self, runner, arguments):
     @param description Description of the runner
     '''
     def find_runner(self, description):
-        runners = self._runners_endpoint(as_list=False)
+        runners = self._runners_endpoint(**list_all_kwargs)
 
         for runner in runners:
             # python-gitlab 2.2 through at least 2.5 returns a list of dicts for list() instead of a Runner

plugins/modules/gitlab_user.py

@@ -230,7 +230,7 @@
 from ansible.module_utils.common.text.converters import to_native
 
 from ansible_collections.community.general.plugins.module_utils.gitlab import (
-    auth_argument_spec, find_group, gitlab_authentication, gitlab
+    auth_argument_spec, find_group, gitlab_authentication, gitlab, list_all_kwargs
 )
 
 
@@ -345,9 +345,10 @@ def get_user_id(self, user):
     @param sshkey_name Name of the ssh key
     '''
     def ssh_key_exists(self, user, sshkey_name):
-        keyList = map(lambda k: k.title, user.keys.list(all=True))
-
-        return sshkey_name in keyList
+        return any(
+            k.title == sshkey_name
+            for k in user.keys.list(**list_all_kwargs)
+        )
 
     '''
     @param user User object
@@ -515,10 +516,13 @@ def delete_identities(self, user, identities):
     @param username Username of the user
     '''
     def find_user(self, username):
-        users = self._gitlab.users.list(search=username, all=True)
-        for user in users:
-            if (user.username == username):
-                return user
+        return next(
+            (
+                user for user in self._gitlab.users.list(search=username, **list_all_kwargs)
+                if user.username == username
+            ),
+            None
+        )
 
     '''
     @param username Username of the user