Dne 21.11.2014 v 11:28 Tomas Babej napsal(a):
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
There is a difference between a search on a non-existent entry and a
search on an existent entry with a non-matching filter. This difference
exists on LDAP level and applies to all search scopes, not just the base
scope.
I don't think hiding this difference is useful at all. Remember that
this bug exists because we were hiding the difference in the first place.
Also, after giving this some thought, I think it would be better to
create a new error for the case where there is no match instead of the
case where the entry does not exist. NotFound pretty much corresponds to
LDAP's NO_SUCH_OBJECT, something like NoMatchingEntry or EmptyResult
would fit the no-match result better.
--
Jan Cholasta
_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel
