Dne 19.11.2014 v 15:12 Tomas Babej napsal(a):

On 11/19/2014 02:03 PM, Jan Cholasta wrote:
Dne 19.11.2014 v 13:44 Tomas Babej napsal(a):

On 11/19/2014 12:51 PM, Martin Kosek wrote:
On 11/19/2014 12:41 PM, Tomas Babej wrote:
On 11/19/2014 12:24 PM, Martin Kosek wrote:
On 11/19/2014 12:03 PM, Tomas Babej wrote:

When constructing a parent DN in LDAPSearch, we should always
check that the parent object exists (hence use get_dn_if_exists),
rather than search on unexistant containers (which can happen
with get_dn).

Replaces get_dn calls with get_dn_if_exists in *-find commands
and makes sure proper error message is raised.

Doesn't it produce extra LDAP search thus making all our search
slower? Is that what we want?
No it does not make all of our LDAP search slower. It only happens for
the objects that have parent objects, such as idoverrides or
... and makes them slower.

What I was pointing out here is that this is not a issue for ALL *-find
commands (e.g user-find, group-find will not suffer from it), as you
incorrectly stated.

Wouldn't it be better to distinguish between LDAP
search with no results and LDAP search with missing parent DN? The
reply looks
different, at least in CLI:
Up to discussion. We would probably need to introduce a new exception,
like ParentObjectNotFound.

# search result
search: 4
result: 0 Success

# search result
search: 4
result: 32 No such object
matchedDN: cn=accounts,dc=mkosek-f20,dc=test

Also, I do not think you can just stop using get_dn(), some
commands override
this call to get more complex searches (like host-find searching
for shortname).
Look into the get_dn_if_exists, it just wraps around get_dn, so no
here. Any custom behaviour is preserved.
Ah, ok, thanks for info.

To sum up, I think this is worth changing this behaviour by default,
ignoring a non-matching value of the parent object is not a correct
general approach in my opinion.
Well, that's the question. Whether we would leave DS to validate the
itself or do all the pre-check ourselves. To me, doing just one LDAP
search and
processing the error correctly looks better. But I can live even
with your
version then, I will leave the framework guardians like Honza or
Petr3 to decide.

+1 on single LDAP search and proper error processing.

I see now what you're trying to suggest. However, the reason boils
down to ipaldap.find_entries method not differentiating between a
LDAP search that returns error code 32 (No such object) and LDAP
search returning error code 0 (Success), but returning no results.

In both cases errors.NotFound is raised.

The reason I did not go this way is that changing the find_entries
is quite more invasive as this is the method subsenqently called by
any command.

You can always derive the new error (ParentNotFound or whatever) on
NotFound, so old code won't break.

Thanks for the suggestsions.

Attached is a new patch which hooks into find_entries method and
differentiates between the cases.

Why are you special casing base scope search?

Jan Cholasta

Freeipa-devel mailing list

Reply via email to