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

ldap_group_links.list() not usable #2738

Open
zapp42 opened this issue Dec 4, 2023 · 5 comments
Open

ldap_group_links.list() not usable #2738

zapp42 opened this issue Dec 4, 2023 · 5 comments
Labels

Comments

@zapp42
Copy link

zapp42 commented Dec 4, 2023

Description of the problem, including code/CLI snippet

The current implementation of the list() method for the ldap_group_links api is not usable. The cli throws an exceptions and the api returns unusable data, because an id is missing.

$ gitlab group-ldap-group-link list --group-id xxxxxx
[, , , , , , ]
Traceback (most recent call last):
File "/home/rof/.local/bin/gitlab", line 8, in
sys.exit(main())
^^^^^^
File "/home/rof/.local/lib/python3.12/site-packages/gitlab/cli.py", line 389, in main
gitlab.v4.cli.run(
File "/home/rof/.local/lib/python3.12/site-packages/gitlab/v4/cli.py", line 556, in run
printer.display_list(data, fields, verbose=verbose)
File "/home/rof/.local/lib/python3.12/site-packages/gitlab/v4/cli.py", line 518, in display_list
self.display(get_dict(obj, fields), verbose=verbose, obj=obj)
File "/home/rof/.local/lib/python3.12/site-packages/gitlab/v4/cli.py", line 487, in display
id = getattr(obj, obj._id_attr)
^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/home/rof/.local/lib/python3.12/site-packages/gitlab/base.py", line 134, in getattr
raise AttributeError(message)
AttributeError: 'GroupLDAPGroupLink' object has no attribute 'id'

I think the problem is, that the ldap_group_links api has two possible (exclusive) id fields: cn and filter. I did not find a way to reflect this in python-gitlab. As we do not use ldap filters in our Gitlab instance, my "fix" was to add

_id_attr = "cn"

to the GroupLDAPGroupLink class.

Expected Behavior

The data returned shall contain either a cn or filter field.

Actual Behavior

Returned data only has the provider attribute, which is useless without the other field(s).

Specifications

  • python-gitlab version: 4.2.0
  • API version you are using (v3/v4): v4
  • Gitlab server version (or gitlab.com): 16.5.3
@zapp42
Copy link
Author

zapp42 commented Dec 4, 2023

Sorry, the output was stripped, it looks like this:

[<GroupLDAPGroupLink id:None provider:ldapmain>, <GroupLDAPGroupLink id:None provider:ldapmain>, <GroupLDAPGroupLink id:None provider:ldapmain>, <GroupLDAPGroupLink id:None provider:ldapmain>, <GroupLDAPGroupLink id:None provider:ldapmain>, <GroupLDAPGroupLink id:None provider:ldapmain>, <GroupLDAPGroupLink id:None provider:ldapmain>]

@zapp42
Copy link
Author

zapp42 commented Dec 4, 2023

The create cli method is also not usable, as it does not accept a cn or filter parameter, one of those is required.

The delete cli method expects a cn parameter, but no provider. This api is deprecated. The current one also requires a provider and either cn or filter.
https://docs.gitlab.com/ee/api/groups.html#delete-ldap-group-link

Shall I create separate issues for that or broaden the scope of this one?

@nejch nejch added the bug label Dec 12, 2023
@nejch
Copy link
Member

nejch commented Dec 12, 2023

Thanks @zapp42, I'll admit the maintainers here never had a use case for this and the new methods were just a port of the old deprecated ones.

I'd be happy to review a PR or look at this at a later stage since it's a Premium features and thus not really on our radar so much.

@Sjord
Copy link
Contributor

Sjord commented Jan 21, 2024

gitlab group-ldap-group-link list --group-id xxxxxx performs a request to /api/v4/groups/xxxxxx/ldap_group_links, and the response looks like this:

[{"cn":"foo","group_access":10,"provider":"ldap"}]

@Sjord
Copy link
Contributor

Sjord commented Jan 21, 2024

Setting _id_attr = None would at least get rid of the exception, and then --verbose can be used to also display the cn or filter fields.

Since _id_attr is a class property and not an object property, it is not really possible to change it depending on whether an object has a filter or cn property.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants