On 11/20/2014 04:01 PM, Jan Cholasta wrote: > 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: >>>>>>>> Hi, >>>>>>>> >>>>>>>> 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. >>>>>>>> >>>>>>>> https://fedorahosted.org/freeipa/ticket/4659 >>>>>>> Doesn't it produce extra LDAP search thus making all our search >>>>>>> commands >>>>>>> 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 >>>>>> dnsrecords. >>>>> ... 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 >>>>>> issue >>>>>> 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 >>>>> search >>>>> 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 >>>> method >>>> is quite more invasive as this is the method subsenqently called by >>>> almost >>>> 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? >
Since base search is performed only on the entry specified by the DN, there is no need to differentiate between ContainerNotFound and NotFound. So the logic is as follows: subtree search - ContainerNotFound is raised when container does not exist, NotFound if search provided no results onelevel search - the same base search - NotFound is raised if the specified DN does not exist -- Tomas Babej Associate Software Engineer | Red Hat | Identity Management RHCE | Brno Site | IRC: tbabej | freeipa.org _______________________________________________ Freeipa-devel mailing list Freeipa-devel@redhat.com https://www.redhat.com/mailman/listinfo/freeipa-devel