mirror of
https://github.com/ansible-collections/community.general.git
synced 2024-09-14 20:13:21 +02:00
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)
This commit is contained in:
parent
f6d0b35bb7
commit
787fa46217
11 changed files with 65 additions and 54 deletions
|
@ -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).
|
|
@ -21,15 +21,30 @@ except ImportError:
|
||||||
|
|
||||||
import traceback
|
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
|
GITLAB_IMP_ERR = None
|
||||||
try:
|
try:
|
||||||
import gitlab
|
import gitlab
|
||||||
import requests
|
import requests
|
||||||
HAS_GITLAB_PACKAGE = True
|
HAS_GITLAB_PACKAGE = True
|
||||||
|
list_all_kwargs = _determine_list_all_kwargs(gitlab.__version__)
|
||||||
except Exception:
|
except Exception:
|
||||||
gitlab = None
|
gitlab = None
|
||||||
GITLAB_IMP_ERR = traceback.format_exc()
|
GITLAB_IMP_ERR = traceback.format_exc()
|
||||||
HAS_GITLAB_PACKAGE = False
|
HAS_GITLAB_PACKAGE = False
|
||||||
|
list_all_kwargs = {}
|
||||||
|
|
||||||
|
|
||||||
def auth_argument_spec(spec=None):
|
def auth_argument_spec(spec=None):
|
||||||
|
|
|
@ -120,7 +120,7 @@ from ansible.module_utils.basic import AnsibleModule
|
||||||
from ansible.module_utils.common.text.converters import to_native
|
from ansible.module_utils.common.text.converters import to_native
|
||||||
|
|
||||||
from ansible_collections.community.general.plugins.module_utils.gitlab import (
|
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 @@ class GitLabDeployKey(object):
|
||||||
@param key_title Title of the key
|
@param key_title Title of the key
|
||||||
'''
|
'''
|
||||||
def find_deploy_key(self, project, key_title):
|
def find_deploy_key(self, project, key_title):
|
||||||
deploy_keys = project.keys.list(all=True)
|
for deploy_key in project.keys.list(**list_all_kwargs):
|
||||||
for deploy_key in deploy_keys:
|
|
||||||
if (deploy_key.title == key_title):
|
if (deploy_key.title == key_title):
|
||||||
return deploy_key
|
return deploy_key
|
||||||
|
|
||||||
|
|
|
@ -160,7 +160,7 @@ from ansible.module_utils.api import basic_auth_argument_spec
|
||||||
from ansible.module_utils.basic import AnsibleModule
|
from ansible.module_utils.basic import AnsibleModule
|
||||||
|
|
||||||
from ansible_collections.community.general.plugins.module_utils.gitlab import (
|
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 @@ class GitLabGroup(object):
|
||||||
|
|
||||||
# get user id if the user exists
|
# get user id if the user exists
|
||||||
def get_user_id(self, gitlab_user):
|
def get_user_id(self, gitlab_user):
|
||||||
user_exists = self._gitlab.users.list(username=gitlab_user, all=True)
|
return next(
|
||||||
if user_exists:
|
(u.id for u in self._gitlab.users.list(username=gitlab_user, **list_all_kwargs)),
|
||||||
return user_exists[0].id
|
None
|
||||||
|
)
|
||||||
|
|
||||||
# get group id if group exists
|
# get group id if group exists
|
||||||
def get_group_id(self, gitlab_group):
|
def get_group_id(self, gitlab_group):
|
||||||
groups = self._gitlab.groups.list(search=gitlab_group, all=True)
|
return next(
|
||||||
for group in groups:
|
(
|
||||||
if group.full_path == gitlab_group:
|
g.id for g in self._gitlab.groups.list(search=gitlab_group, **list_all_kwargs)
|
||||||
return group.id
|
if g.full_path == gitlab_group
|
||||||
|
),
|
||||||
|
None
|
||||||
|
)
|
||||||
|
|
||||||
# get all members in a group
|
# get all members in a group
|
||||||
def get_members_in_a_group(self, gitlab_group_id):
|
def get_members_in_a_group(self, gitlab_group_id):
|
||||||
|
|
|
@ -206,7 +206,8 @@ group_variable:
|
||||||
from ansible.module_utils.basic import AnsibleModule
|
from ansible.module_utils.basic import AnsibleModule
|
||||||
from ansible.module_utils.api import basic_auth_argument_spec
|
from ansible.module_utils.api import basic_auth_argument_spec
|
||||||
from ansible_collections.community.general.plugins.module_utils.gitlab import (
|
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 @@ class GitlabGroupVariables(object):
|
||||||
return self.repo.groups.get(group_name)
|
return self.repo.groups.get(group_name)
|
||||||
|
|
||||||
def list_all_group_variables(self):
|
def list_all_group_variables(self):
|
||||||
page_nb = 1
|
return list(self.group.variables.list(**list_all_kwargs))
|
||||||
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
|
|
||||||
|
|
||||||
def create_variable(self, var_obj):
|
def create_variable(self, var_obj):
|
||||||
if self._module.check_mode:
|
if self._module.check_mode:
|
||||||
|
|
|
@ -174,7 +174,7 @@ from ansible.module_utils.api import basic_auth_argument_spec
|
||||||
from ansible.module_utils.basic import AnsibleModule
|
from ansible.module_utils.basic import AnsibleModule
|
||||||
|
|
||||||
from ansible_collections.community.general.plugins.module_utils.gitlab import (
|
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 @@ class GitLabHook(object):
|
||||||
@param hook_url Url to call on event
|
@param hook_url Url to call on event
|
||||||
'''
|
'''
|
||||||
def find_hook(self, project, hook_url):
|
def find_hook(self, project, hook_url):
|
||||||
hooks = project.hooks.list(all=True)
|
for hook in project.hooks.list(**list_all_kwargs):
|
||||||
for hook in hooks:
|
|
||||||
if (hook.url == hook_url):
|
if (hook.url == hook_url):
|
||||||
return hook
|
return hook
|
||||||
|
|
||||||
|
|
|
@ -138,7 +138,8 @@ instance_variable:
|
||||||
from ansible.module_utils.basic import AnsibleModule
|
from ansible.module_utils.basic import AnsibleModule
|
||||||
from ansible.module_utils.api import basic_auth_argument_spec
|
from ansible.module_utils.api import basic_auth_argument_spec
|
||||||
from ansible_collections.community.general.plugins.module_utils.gitlab import (
|
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 @@ class GitlabInstanceVariables(object):
|
||||||
self._module = module
|
self._module = module
|
||||||
|
|
||||||
def list_all_instance_variables(self):
|
def list_all_instance_variables(self):
|
||||||
page_nb = 1
|
return list(self.instance.variables.list(**list_all_kwargs))
|
||||||
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
|
|
||||||
|
|
||||||
def create_variable(self, var_obj):
|
def create_variable(self, var_obj):
|
||||||
if self._module.check_mode:
|
if self._module.check_mode:
|
||||||
|
|
|
@ -97,7 +97,7 @@ from ansible.module_utils.api import basic_auth_argument_spec
|
||||||
from ansible.module_utils.basic import AnsibleModule
|
from ansible.module_utils.basic import AnsibleModule
|
||||||
|
|
||||||
from ansible_collections.community.general.plugins.module_utils.gitlab import (
|
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
|
||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@ -105,7 +105,7 @@ def present_strategy(module, gl, project, wished_badge):
|
||||||
changed = False
|
changed = False
|
||||||
|
|
||||||
existing_badge = None
|
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"]:
|
if badge.image_url == wished_badge["image_url"]:
|
||||||
existing_badge = badge
|
existing_badge = badge
|
||||||
break
|
break
|
||||||
|
@ -135,7 +135,7 @@ def absent_strategy(module, gl, project, wished_badge):
|
||||||
changed = False
|
changed = False
|
||||||
|
|
||||||
existing_badge = None
|
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"]:
|
if badge.image_url == wished_badge["image_url"]:
|
||||||
existing_badge = badge
|
existing_badge = badge
|
||||||
break
|
break
|
||||||
|
|
|
@ -225,7 +225,8 @@ from ansible.module_utils.api import basic_auth_argument_spec
|
||||||
|
|
||||||
|
|
||||||
from ansible_collections.community.general.plugins.module_utils.gitlab import (
|
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 @@ class GitlabProjectVariables(object):
|
||||||
return self.repo.projects.get(project_name)
|
return self.repo.projects.get(project_name)
|
||||||
|
|
||||||
def list_all_project_variables(self):
|
def list_all_project_variables(self):
|
||||||
page_nb = 1
|
return list(self.project.variables.list(**list_all_kwargs))
|
||||||
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
|
|
||||||
|
|
||||||
def create_variable(self, var_obj):
|
def create_variable(self, var_obj):
|
||||||
if self._module.check_mode:
|
if self._module.check_mode:
|
||||||
|
|
|
@ -219,7 +219,7 @@ from ansible.module_utils.basic import AnsibleModule
|
||||||
from ansible.module_utils.common.text.converters import to_native
|
from ansible.module_utils.common.text.converters import to_native
|
||||||
|
|
||||||
from ansible_collections.community.general.plugins.module_utils.gitlab import (
|
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 @@ class GitLabRunner(object):
|
||||||
@param description Description of the runner
|
@param description Description of the runner
|
||||||
'''
|
'''
|
||||||
def find_runner(self, description):
|
def find_runner(self, description):
|
||||||
runners = self._runners_endpoint(as_list=False)
|
runners = self._runners_endpoint(**list_all_kwargs)
|
||||||
|
|
||||||
for runner in runners:
|
for runner in runners:
|
||||||
# python-gitlab 2.2 through at least 2.5 returns a list of dicts for list() instead of a Runner
|
# python-gitlab 2.2 through at least 2.5 returns a list of dicts for list() instead of a Runner
|
||||||
|
|
|
@ -230,7 +230,7 @@ from ansible.module_utils.basic import AnsibleModule
|
||||||
from ansible.module_utils.common.text.converters import to_native
|
from ansible.module_utils.common.text.converters import to_native
|
||||||
|
|
||||||
from ansible_collections.community.general.plugins.module_utils.gitlab import (
|
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 @@ class GitLabUser(object):
|
||||||
@param sshkey_name Name of the ssh key
|
@param sshkey_name Name of the ssh key
|
||||||
'''
|
'''
|
||||||
def ssh_key_exists(self, user, sshkey_name):
|
def ssh_key_exists(self, user, sshkey_name):
|
||||||
keyList = map(lambda k: k.title, user.keys.list(all=True))
|
return any(
|
||||||
|
k.title == sshkey_name
|
||||||
return sshkey_name in keyList
|
for k in user.keys.list(**list_all_kwargs)
|
||||||
|
)
|
||||||
|
|
||||||
'''
|
'''
|
||||||
@param user User object
|
@param user User object
|
||||||
|
@ -515,10 +516,13 @@ class GitLabUser(object):
|
||||||
@param username Username of the user
|
@param username Username of the user
|
||||||
'''
|
'''
|
||||||
def find_user(self, username):
|
def find_user(self, username):
|
||||||
users = self._gitlab.users.list(search=username, all=True)
|
return next(
|
||||||
for user in users:
|
(
|
||||||
if (user.username == username):
|
user for user in self._gitlab.users.list(search=username, **list_all_kwargs)
|
||||||
return user
|
if user.username == username
|
||||||
|
),
|
||||||
|
None
|
||||||
|
)
|
||||||
|
|
||||||
'''
|
'''
|
||||||
@param username Username of the user
|
@param username Username of the user
|
||||||
|
|
Loading…
Reference in a new issue