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

netgroup: avoid extraneous LDAP search when retrieving primary key from DN #68

Closed
wants to merge 1 commit into from

Conversation

martbab
Copy link
Contributor

@martbab martbab commented Sep 8, 2016

Fixes https://fedorahosted.org/freeipa/ticket/5855

Please note that the parent method does not correctly handle cases when the
attribute considered as primary ked is contained in multiple RDNs:

>>> api.Object.netgroup.get_primary_key_from_dn(
...     DN('ipauniqueid=yadda-yadda,cn=ng,cn=alt,dc=ipa,dc=test'))
u'ng'

That's why I had to completely override parent method.

@stlaz stlaz self-assigned this Sep 9, 2016
@stlaz
Copy link
Contributor

stlaz commented Sep 9, 2016

Please add assert isinstance(dn, DN) as in the parent method.

Also, I think maybe it's safer to check for rdns emptiness before you try to assign from it.

…om DN

DNs for netgroup entries can contain either 'cn' or 'ipauniqueid' attribute in
their leaf RDN depending on their origin. Since 'cn' is the primary key, we
can return it in `get_primary_key_from_dn` right away and avoid any extraneous
LDAP search.

https://fedorahosted.org/freeipa/ticket/5855
@stlaz stlaz added the ack Pull Request approved, can be merged label Sep 9, 2016
@martbab martbab added the pushed Pull Request has already been pushed label Sep 9, 2016
@martbab martbab closed this Sep 9, 2016
@martbab martbab deleted the netgroups_pkey_from_dn branch September 13, 2016 14:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ack Pull Request approved, can be merged pushed Pull Request has already been pushed
Projects
None yet
2 participants