On Tue, 2012-01-31 at 10:58 +0100, Jakub Hrozek wrote:
> On Mon, Jan 30, 2012 at 09:26:21AM -0500, Stephen Gallagher wrote:
> > On Mon, 2012-01-30 at 15:04 +0100, Jakub Hrozek wrote:
> > > On Sat, Jan 28, 2012 at 02:10:19PM -0500, Stephen Gallagher wrote:
> > > > On Sat, 2012-01-28 at 13:52 -0500, Stephen Gallagher wrote:
> > > > > On Sat, 2012-01-28 at 12:08 -0500, Stephen Gallagher wrote:
> > > > > > I'm still working on the enumeration support, but I wanted to get
> > > > > > the
> > > > > > direct lookup on the list early.
> > > > > >
> > > > > > Patch 0001: extend sysdb_store_service() to accept additional
> > > > > > attributes
> > > > > > This is necessary to be able to store provider-specific data in the
> > > > > > entry (such as the entryUSN).
> > >
> > > Can you move the NULL check of msg->dn outside the switch so that there
> > > is only one and all cases are checked? Currently the service case does
> > > not have a NULL check.
> > >
> >
> > Fixed. Thanks, I meant to do that in the first place and then... didn't.
> >
> > > You should also pass primary_name, not name into the call of
> > > sysdb_remove_attrs() because name points to ldb_message structure that
> > > was already freed.
> > >
> >
> > Good catch, thanks!
> >
> > > > > >
> > > > > > Patch 0002: Add a helper function to get an integer value from a
> > > > > > sysdb_attrs object directly as a uint16_t.
> > > > > >
> > >
> > > Ack
> > >
> > > > > > Patch 0003: The big one. This patch adds support for looking up
> > > > > > services
> > > > > > in LDAP. There shouldn't be any big "gotchas" here. It's very
> > > > > > straightforward (if long).
> > > > > >
> > >
> > > Setting errno to 0 before strtouint16() is not needed, strtouint16()
> > > does it internally.
> > >
> >
> > Removed.
> >
> > > Otherwise Ack.
> > >
> > > Patch 0004: LDAP: Add enumeration support for services
> > > Ack
> > >
> > > > > > Patch 0004: Extend this support to the IPA Provider (even though IPA
> > > > > > itself does not yet support the services map natively).
> > > > >
> > >
> > > (This is actually patch 0005 in the most recent set)
> > > Ack
> > >
> > > > > I over-estimated the additional effort necessary to add the
> > > > > enumeration
> > > > > support. New patch set includes the enumeration work (before the IPA
> > > > > provider) as well as fixing a minor typo in patch 0003 (bad copy-paste
> > > > > in a comment).
> > > >
> > > > One more set. I forgot to include updates to the SSSDConfig API and the
> > > > manpages in the previous set. I added it as a new, sixth patch. This
> > > > should now be complete.
> > >
> > > Patch 0006: I don't understand this line:
> > > #replaced by ldap_entry_usn# 'ldap_service_entry_usn' : _('Service
> > > entryUSN attribute'),
> > >
> > > It looks like SDAP_AT_SERVICE_USN is used in the sdap services code?
> >
> > If you look at the rest of the SSSDConfig.py list, we make the same
> > comment elsewhere for users, groups, etc. The reason is that there's
> > never actually a reason to use the specific ones, because no LDAP server
> > is going to have different entryUSN attributes for different maps.
> >
> > Furthermore, we generally prefer that these options are unset, since we
> > have very good auto-detection for this attribute.
> >
> > So I'm leaving that as-is.
> >
> > >
> > > Also "ldap_service_search_base" is missing from the manual page.
> >
> > Fixed.
> >
> >
> > New patches attached.
>
> Ack to all the patches.
>
> I noticed one more bug in the responder - for a service that has both
> "tcp" and "udp" values for the "ipServiceProtocol" attribute, the first
> one is always returned unconditionally even if the user specifically
> asked for a protocol.
>
> So getservbyname("genericservice", "udp"); would always retun "tcp" if
> it's the first service in the cache even if "udp" exists.
>
> We can obviously fix this post-beta, though.Pushed to master. Opened ticket https://fedorahosted.org/sssd/ticket/1160 to track the above issue.
signature.asc
Description: This is a digitally signed message part
_______________________________________________ sssd-devel mailing list [email protected] https://fedorahosted.org/mailman/listinfo/sssd-devel
