Add support for query ldap groups links #1612

andrewlarssen · 2021-09-24T15:04:37Z

You can currently add or delete ldap links but you can't query existing links

andrewlarssen · 2021-09-24T15:06:19Z

For issue #1609

andrewlarssen · 2021-09-24T15:08:13Z

Have tested this manually on a real gitlab instance. I could not see any automated tests for gitlab -> ldap features

codecov-commenter · 2021-09-26T15:23:59Z

Codecov Report

Merging #1612 (772e79a) into master (7ea4ddc) will increase coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1612      +/-   ##
==========================================
+ Coverage   91.59%   91.61%   +0.01%     
==========================================
  Files          74       74              
  Lines        4272     4279       +7     
==========================================
+ Hits         3913     3920       +7     
  Misses        359      359

Flag	Coverage Δ
cli_func_v4	`81.63% <100.00%> (+0.03%)`	⬆️
py_func_v4	`80.64% <100.00%> (+0.03%)`	⬆️
unit	`83.40% <100.00%> (+0.02%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
gitlab/v4/objects/groups.py	`87.87% <100.00%> (+0.67%)`	⬆️

andrewlarssen · 2021-09-27T12:52:48Z

Fixed 2 linting issues. One was commit message so had to force push

nejch

Thanks a lot @andrewlarssen! Just a comment on naming consistency. If you're able to add some unit tests to this, would also be great! Here https://github.com/python-gitlab/python-gitlab/blob/master/tests/unit/objects/test_groups.py or you could create a new test_ldap_links.py if it's getting too crowded there. Unit tests mock the responses so we can test EE features there.

While reviewing this I noticed our existing method for deletion uses deprecated API endpoints: https://docs.gitlab.com/ee/api/groups.html#delete-ldap-group-link

So as a bonus if you find time, I think these methods actually could be removed and you can add the CreateMixin/DeleteMixin to the manager, and ObjectDeleteMixin to the object. Then we could remove the custom methods here:

python-gitlab/gitlab/v4/objects/groups.py

Lines 112 to 150 in 227607c

    
               @cli.register_custom_action("Group", ("cn", "group_access", "provider")) 
        
               @exc.on_http_error(exc.GitlabCreateError) 
        
               def add_ldap_group_link(self, cn, group_access, provider, **kwargs): 
        
                   """Add an LDAP group link. 
        
                   Args: 
        
                       cn (str): CN of the LDAP group 
        
                       group_access (int): Minimum access level for members of the LDAP 
        
                           group 
        
                       provider (str): LDAP provider for the LDAP group 
        
                       **kwargs: Extra options to send to the server (e.g. sudo) 
        
                   Raises: 
        
                       GitlabAuthenticationError: If authentication is not correct 
        
                       GitlabCreateError: If the server cannot perform the request 
        
                   """ 
        
                   path = "/groups/%s/ldap_group_links" % self.get_id() 
        
                   data = {"cn": cn, "group_access": group_access, "provider": provider} 
        
                   self.manager.gitlab.http_post(path, post_data=data, **kwargs) 
        
               @cli.register_custom_action("Group", ("cn",), ("provider",)) 
        
               @exc.on_http_error(exc.GitlabDeleteError) 
        
               def delete_ldap_group_link(self, cn, provider=None, **kwargs): 
        
                   """Delete an LDAP group link. 
        
                   Args: 
        
                       cn (str): CN of the LDAP group 
        
                       provider (str): LDAP provider for the LDAP group 
        
                       **kwargs: Extra options to send to the server (e.g. sudo) 
        
                   Raises: 
        
                       GitlabAuthenticationError: If authentication is not correct 
        
                       GitlabDeleteError: If the server cannot perform the request 
        
                   """ 
        
                   path = "/groups/%s/ldap_group_links" % self.get_id() 
        
                   if provider is not None: 
        
                       path += "/%s" % provider 
        
                   path += "/%s" % cn 
        
                   self.manager.gitlab.http_delete(path)

)

This would be a breaking change, but we have a major release coming up anyway. Let me know if this makes sense or you need help. Also, you'll just need to amend your message again as commitlint does not like capitalized sentences :)

nejch · 2021-09-27T19:46:31Z

gitlab/v4/objects/groups.py

+    "GroupLdapLinks",
+    "GroupLdapLinksManager",


For consistency with other objects, we should use the single (the API also supports adding a single link)

Suggested change

"GroupLdapLinks",

"GroupLdapLinksManager",

"GroupLdapLink",

"GroupLdapLinkManager",

nejch · 2021-09-27T19:46:45Z

gitlab/v4/objects/groups.py

@@ -71,6 +73,7 @@ class Group(SaveMixin, ObjectDeleteMixin, RESTObject):
    subgroups: "GroupSubgroupManager"
    variables: GroupVariableManager
    wikis: GroupWikiManager
+    ldap_group_links: "GroupLdapLinksManager"


Suggested change

ldap_group_links: "GroupLdapLinksManager"

ldap_group_links: "GroupLdapLinkManager"

nejch · 2021-09-27T19:47:02Z

gitlab/v4/objects/groups.py

@@ -320,6 +323,16 @@ class GroupSubgroupManager(ListMixin, RESTManager):
    _types = {"skip_groups": types.ListAttribute}


+class GroupLdapLinks(RESTObject):


Suggested change

class GroupLdapLinks(RESTObject):

class GroupLdapLink(RESTObject):

nejch · 2021-09-27T19:53:29Z

gitlab/v4/objects/groups.py

+class GroupLdapLinksManager(ListMixin, RESTManager):
+    _path = "/groups/%(group_id)s/ldap_group_links"
+    _obj_cls = GroupLdapLinks


Suggested change

class GroupLdapLinksManager(ListMixin, RESTManager):

_path = "/groups/%(group_id)s/ldap_group_links"

_obj_cls = GroupLdapLinks

class GroupLdapLinkManager(ListMixin, RESTManager):

_path = "/groups/%(group_id)s/ldap_group_links"

_obj_cls = GroupLdapLink

github-actions · 2022-01-08T02:19:39Z

This Pull Request (PR) was marked stale because it has been open 90 days with no activity. Please remove the stale label or comment on this PR. Otherwise, it will be closed in 15 days.

github-actions · 2022-01-24T02:13:59Z

This PR was closed because it has been marked stale for 15 days with no activity. If this PR is still valid, please re-open.

andrewlarssen mentioned this pull request Sep 24, 2021

Cant query current ldap groups #1609

Closed

feat(api): Add support for query ldap groups links

772e79a

andrewlarssen force-pushed the master branch from 2322542 to 772e79a Compare September 27, 2021 12:46

nejch requested changes Sep 27, 2021

View reviewed changes

nejch added the Waiting for response label Oct 6, 2021

nejch assigned andrewlarssen Oct 6, 2021

github-actions bot added the stale label Jan 8, 2022

github-actions bot closed this Jan 24, 2022

nejch mentioned this pull request Nov 17, 2022

feat(groups): add LDAP link manager and deprecate old API endpoints #2379

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Add support for query ldap groups links #1612

Add support for query ldap groups links #1612

Uh oh!

andrewlarssen commented Sep 24, 2021

Uh oh!

andrewlarssen commented Sep 24, 2021

Uh oh!

andrewlarssen commented Sep 24, 2021

Uh oh!

codecov-commenter commented Sep 26, 2021 •

edited

Loading

Uh oh!

andrewlarssen commented Sep 27, 2021

Uh oh!

nejch left a comment •

edited

Loading

Uh oh!

nejch Sep 27, 2021

Uh oh!

nejch Sep 27, 2021

Uh oh!

nejch Sep 27, 2021

Uh oh!

nejch Sep 27, 2021

Uh oh!

github-actions bot commented Jan 8, 2022

Uh oh!

github-actions bot commented Jan 24, 2022

Uh oh!

Uh oh!

	@cli.register_custom_action("Group", ("cn", "group_access", "provider"))
	@exc.on_http_error(exc.GitlabCreateError)
	def add_ldap_group_link(self, cn, group_access, provider, **kwargs):
	"""Add an LDAP group link.

	Args:
	cn (str): CN of the LDAP group
	group_access (int): Minimum access level for members of the LDAP
	group
	provider (str): LDAP provider for the LDAP group
	**kwargs: Extra options to send to the server (e.g. sudo)

	Raises:
	GitlabAuthenticationError: If authentication is not correct
	GitlabCreateError: If the server cannot perform the request
	"""
	path = "/groups/%s/ldap_group_links" % self.get_id()
	data = {"cn": cn, "group_access": group_access, "provider": provider}
	self.manager.gitlab.http_post(path, post_data=data, **kwargs)

	@cli.register_custom_action("Group", ("cn",), ("provider",))
	@exc.on_http_error(exc.GitlabDeleteError)
	def delete_ldap_group_link(self, cn, provider=None, **kwargs):
	"""Delete an LDAP group link.

	Args:
	cn (str): CN of the LDAP group
	provider (str): LDAP provider for the LDAP group
	**kwargs: Extra options to send to the server (e.g. sudo)

	Raises:
	GitlabAuthenticationError: If authentication is not correct
	GitlabDeleteError: If the server cannot perform the request
	"""
	path = "/groups/%s/ldap_group_links" % self.get_id()
	if provider is not None:
	path += "/%s" % provider
	path += "/%s" % cn
	self.manager.gitlab.http_delete(path)

	ldap_group_links: "GroupLdapLinksManager"
	ldap_group_links: "GroupLdapLinkManager"

		@@ -320,6 +323,16 @@ class GroupSubgroupManager(ListMixin, RESTManager):
		_types = {"skip_groups": types.ListAttribute}


		class GroupLdapLinks(RESTObject):

	class GroupLdapLinks(RESTObject):
	class GroupLdapLink(RESTObject):

Add support for query ldap groups links #1612

Add support for query ldap groups links #1612

Uh oh!

Conversation

andrewlarssen commented Sep 24, 2021

Uh oh!

andrewlarssen commented Sep 24, 2021

Uh oh!

andrewlarssen commented Sep 24, 2021

Uh oh!

codecov-commenter commented Sep 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

andrewlarssen commented Sep 27, 2021

Uh oh!

nejch left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nejch Sep 27, 2021

Choose a reason for hiding this comment

Uh oh!

nejch Sep 27, 2021

Choose a reason for hiding this comment

Uh oh!

nejch Sep 27, 2021

Choose a reason for hiding this comment

Uh oh!

nejch Sep 27, 2021

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jan 8, 2022

Uh oh!

github-actions bot commented Jan 24, 2022

Uh oh!

Uh oh!

codecov-commenter commented Sep 26, 2021 •

edited

Loading

nejch left a comment •

edited

Loading