[SSSD] [PATCH] Fix faulty usn reinit

Simo Sorce simo at redhat.com
Tue Jan 8 22:54:59 UTC 2013


On Tue, 2013-01-08 at 21:52 +0100, Jakub Hrozek wrote:
> On Mon, Jan 07, 2013 at 11:13:03PM -0500, Simo Sorce wrote:
> > On Sun, 2013-01-06 at 00:37 -0500, Simo Sorce wrote:
> > > While looking at some code my eye fell on the fact that sdap_reinit.c
> > > was including sysdb_private.h
> > > 
> > > That's a no-no on its own, you don't get to use private headers
> > > liberally, or I wouldn't have marked them "private" in the first place!
> > > 
> > > However besides the abuse of the private headers I found also that the
> > > function was broken because it wasn't doing what it was trying to do
> > > (limit cleanups to users, groups and services).
> > > 
> > > Instead it would search the whole tree (3 times) and later remove all
> > > entries w/o a USN.
> > > 
> > > I think this could cause the code to remove *everything* not directly
> > > downloaded from the IPA tree (for example subdomain users) that lacks
> > > the SYSDB_USN attribute for example.
> > > 
> > > I haven't tested the patch yet tbh, but I do not have the setup right
> > > now, if someone has a 2 servers setup ready and can force sssd to
> > > reconnect to the second and step through the cleanup to make sure it
> > > runs as it should I would be grateful.
> > 
> > I am not sure how this function ever worked at all now, I found another
> > bug, state->sysdb where never assinged, so sysdb was NULl in some
> > calls ...
> > 
> > Attached rebased patch that adds this fix.
> > 
> > Simo.
> > 
> > -- 
> > Simo Sorce * Red Hat, Inc * New York
> 
> 
> I haven't tested the code yet, but can you split the patch into one that
> introduces the new sysdb function and one that uses it?

Done

> A unit test would be nice, too.

Actually, after looking at the code I went a step further, I realized
all getsvc/enumsvc functions were doing the same thing so I change them
to just use the new sysdb_search_service functions instead of direct ldb
calls. This should be more robust and implicitly tests
sysdb_search_service in all current tests.

The only annoyance is a slight mismatch in the interfaces so I had to
recreate a ldb_result structure in the getsvc calls. It's not the
prettiest solution but gets the job done w/o any side effect and w/o
requiring to change a lot of callers.

> Also, can you use new DEBUG levels in the new code?

Done.

Attached new patchset.

Simo.

-- 
Simo Sorce * Red Hat, Inc * New York
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-sysdb_search_service-helper-function.patch
Type: text/x-patch
Size: 2996 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130108/9ade1ecc/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Use-sysdb_search_service-for-all-svc-queries.patch
Type: text/x-patch
Size: 7187 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130108/9ade1ecc/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Fix-sdap-reinit.patch
Type: text/x-patch
Size: 8674 bytes
Desc: not available
URL: <https://lists.fedorahosted.org/pipermail/sssd-devel/attachments/20130108/9ade1ecc/attachment-0002.bin>


More information about the sssd-devel mailing list