Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

8 changes: 8 additions & 0 deletions changelogs/fragments/7790-gitlab-runner-api-pagination.yml
Original file line number Diff line number Diff line change
@@ -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).
15 changes: 15 additions & 0 deletions plugins/module_utils/gitlab.py
Original file line number Diff line number Diff line change
Expand Up @@ -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}
Comment on lines +32 to +34
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

quick note: as_list=False in older versions and iterator=True in newer versions takee precedence over all and get_all so IMO including the all / get_all arguments here is not necessary. But not a blocker.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 I'll merge for now so I can do a release this evening, if someone wants to create a follow-up PR to clean this up, feel free :)



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):
Expand Down
5 changes: 2 additions & 3 deletions plugins/modules/gitlab_deploy_key.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)


Expand Down Expand Up @@ -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

Expand Down
20 changes: 12 additions & 8 deletions plugins/modules/gitlab_group_members.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)


Expand All @@ -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):
Expand Down
12 changes: 3 additions & 9 deletions plugins/modules/gitlab_group_variable.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)


Expand All @@ -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:
Expand Down
5 changes: 2 additions & 3 deletions plugins/modules/gitlab_hook.py
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,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
)


Expand Down Expand Up @@ -264,8 +264,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

Expand Down
12 changes: 3 additions & 9 deletions plugins/modules/gitlab_instance_variable.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)


Expand All @@ -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:
Expand Down
6 changes: 3 additions & 3 deletions plugins/modules/gitlab_project_badge.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 3 additions & 9 deletions plugins/modules/gitlab_project_variable.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)


Expand All @@ -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:
Expand Down
4 changes: 2 additions & 2 deletions plugins/modules/gitlab_runner.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)


Expand Down Expand Up @@ -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
Expand Down
20 changes: 12 additions & 8 deletions plugins/modules/gitlab_user.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)


Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
Loading