From 5a1ee4e3ee9c38a06a19c4ad11584d749bcdc1a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?L=C3=A9o=20GATELLIER?= <26511053+lgatellier@users.noreply.github.com> Date: Mon, 6 May 2024 19:45:53 +0200 Subject: [PATCH] [PR #7790/787fa462 backport][stable-7] fix(modules/gitlab_runner): Use correct argument to list all runners (#8311) [PR #7790/787fa462 backport][stable-8] fix(modules/gitlab_runner): Use correct argument to list all runners (#8032) fix(modules/gitlab_runner): Use correct argument to list all runners (#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 (#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 (#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 (#7790) (cherry picked from commit 787fa4621763879dae26c57aaf0028941dcaaefc) Co-authored-by: patchback[bot] <45432694+patchback[bot]@users.noreply.github.com> Co-authored-by: Mike Wadsten --- .../7790-gitlab-runner-api-pagination.yml | 8 ++++++++ plugins/module_utils/gitlab.py | 15 ++++++++++++++ plugins/modules/gitlab_deploy_key.py | 5 ++--- plugins/modules/gitlab_group_members.py | 20 +++++++++++-------- plugins/modules/gitlab_group_variable.py | 12 +++-------- plugins/modules/gitlab_hook.py | 5 ++--- plugins/modules/gitlab_instance_variable.py | 11 ++-------- plugins/modules/gitlab_project_badge.py | 6 +++--- plugins/modules/gitlab_project_variable.py | 11 ++-------- plugins/modules/gitlab_runner.py | 4 ++-- plugins/modules/gitlab_user.py | 20 +++++++++++-------- 11 files changed, 63 insertions(+), 54 deletions(-) create mode 100644 changelogs/fragments/7790-gitlab-runner-api-pagination.yml diff --git a/changelogs/fragments/7790-gitlab-runner-api-pagination.yml b/changelogs/fragments/7790-gitlab-runner-api-pagination.yml new file mode 100644 index 0000000000..59a65ea8ef --- /dev/null +++ b/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). diff --git a/plugins/module_utils/gitlab.py b/plugins/module_utils/gitlab.py index 8c8aab420a..afd83864db 100644 --- a/plugins/module_utils/gitlab.py +++ b/plugins/module_utils/gitlab.py @@ -21,15 +21,30 @@ except ImportError: 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): diff --git a/plugins/modules/gitlab_deploy_key.py b/plugins/modules/gitlab_deploy_key.py index 4d12fcc9f8..604973f77e 100644 --- a/plugins/modules/gitlab_deploy_key.py +++ b/plugins/modules/gitlab_deploy_key.py @@ -121,7 +121,7 @@ from ansible.module_utils.basic import AnsibleModule 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, ensure_gitlab_package + auth_argument_spec, find_project, gitlab_authentication, gitlab, ensure_gitlab_package, list_all_kwargs ) @@ -209,8 +209,7 @@ class GitLabDeployKey(object): @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 diff --git a/plugins/modules/gitlab_group_members.py b/plugins/modules/gitlab_group_members.py index ee44c62747..07c2fbad19 100644 --- a/plugins/modules/gitlab_group_members.py +++ b/plugins/modules/gitlab_group_members.py @@ -160,7 +160,7 @@ from ansible.module_utils.api import basic_auth_argument_spec from ansible.module_utils.basic import AnsibleModule from ansible_collections.community.general.plugins.module_utils.gitlab import ( - auth_argument_spec, gitlab_authentication, gitlab, ensure_gitlab_package + auth_argument_spec, gitlab_authentication, gitlab, ensure_gitlab_package, list_all_kwargs ) @@ -171,16 +171,20 @@ class GitLabGroup(object): # 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): diff --git a/plugins/modules/gitlab_group_variable.py b/plugins/modules/gitlab_group_variable.py index 1e0cab7d1e..9c5eebcbb0 100644 --- a/plugins/modules/gitlab_group_variable.py +++ b/plugins/modules/gitlab_group_variable.py @@ -207,7 +207,8 @@ group_variable: 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, ensure_gitlab_package, filter_returned_variables, vars_to_variables + auth_argument_spec, gitlab_authentication, ensure_gitlab_package, filter_returned_variables, vars_to_variables, + list_all_kwargs ) @@ -222,14 +223,7 @@ class GitlabGroupVariables(object): 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: diff --git a/plugins/modules/gitlab_hook.py b/plugins/modules/gitlab_hook.py index 8cd0916398..23a1c1b6ad 100644 --- a/plugins/modules/gitlab_hook.py +++ b/plugins/modules/gitlab_hook.py @@ -171,7 +171,7 @@ from ansible.module_utils.api import basic_auth_argument_spec from ansible.module_utils.basic import AnsibleModule from ansible_collections.community.general.plugins.module_utils.gitlab import ( - auth_argument_spec, find_project, gitlab_authentication, ensure_gitlab_package + auth_argument_spec, find_project, gitlab_authentication, ensure_gitlab_package, list_all_kwargs ) @@ -266,8 +266,7 @@ class GitLabHook(object): @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 diff --git a/plugins/modules/gitlab_instance_variable.py b/plugins/modules/gitlab_instance_variable.py index ee2022483e..91760afec3 100644 --- a/plugins/modules/gitlab_instance_variable.py +++ b/plugins/modules/gitlab_instance_variable.py @@ -139,7 +139,7 @@ instance_variable: 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, ensure_gitlab_package, filter_returned_variables + auth_argument_spec, gitlab_authentication, ensure_gitlab_package, filter_returned_variables, list_all_kwargs ) @@ -150,14 +150,7 @@ class GitlabInstanceVariables(object): 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: diff --git a/plugins/modules/gitlab_project_badge.py b/plugins/modules/gitlab_project_badge.py index 917534a9b8..1f2253e22f 100644 --- a/plugins/modules/gitlab_project_badge.py +++ b/plugins/modules/gitlab_project_badge.py @@ -97,7 +97,7 @@ from ansible.module_utils.api import basic_auth_argument_spec from ansible.module_utils.basic import AnsibleModule from ansible_collections.community.general.plugins.module_utils.gitlab import ( - auth_argument_spec, gitlab_authentication, find_project, ensure_gitlab_package + auth_argument_spec, gitlab_authentication, find_project, ensure_gitlab_package, list_all_kwargs ) @@ -105,7 +105,7 @@ 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 diff --git a/plugins/modules/gitlab_project_variable.py b/plugins/modules/gitlab_project_variable.py index b02b7133c3..a38c9eccbc 100644 --- a/plugins/modules/gitlab_project_variable.py +++ b/plugins/modules/gitlab_project_variable.py @@ -227,7 +227,7 @@ 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, ensure_gitlab_package, filter_returned_variables, vars_to_variables, - HAS_GITLAB_PACKAGE, GITLAB_IMP_ERR + list_all_kwargs, HAS_GITLAB_PACKAGE, GITLAB_IMP_ERR ) @@ -242,14 +242,7 @@ class GitlabProjectVariables(object): 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: diff --git a/plugins/modules/gitlab_runner.py b/plugins/modules/gitlab_runner.py index 4bcbedda11..58d56e3c5c 100644 --- a/plugins/modules/gitlab_runner.py +++ b/plugins/modules/gitlab_runner.py @@ -206,7 +206,7 @@ from ansible.module_utils.basic import AnsibleModule 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, ensure_gitlab_package + auth_argument_spec, gitlab_authentication, gitlab, ensure_gitlab_package, list_all_kwargs ) @@ -309,7 +309,7 @@ class GitLabRunner(object): @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 diff --git a/plugins/modules/gitlab_user.py b/plugins/modules/gitlab_user.py index 77365fe19f..0065a089d0 100644 --- a/plugins/modules/gitlab_user.py +++ b/plugins/modules/gitlab_user.py @@ -234,7 +234,7 @@ from ansible.module_utils.basic import AnsibleModule 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, ensure_gitlab_package + auth_argument_spec, find_group, gitlab_authentication, gitlab, ensure_gitlab_package, list_all_kwargs ) @@ -349,9 +349,10 @@ class GitLabUser(object): @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 @@ -519,10 +520,13 @@ class GitLabUser(object): @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