[Freeipa-devel] [PATCH 0286] baseldap: Handle missing parent objects properly in *-find

[Tomas Babej]

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