[SSSD] Support for subdomains - stage 1

Jakub Hrozek jhrozek at redhat.com
Tue Apr 10 15:54:56 UTC 2012


On Tue, Apr 10, 2012 at 12:38:31AM -0400, Jakub Hrozek wrote:
> The mails grow huge. I'm going to trim the response a little and omit patches
> that were already acked elsewhere.
> 
> Patch 0001: Ack
> Patch 0002: Ack
> Patch 0003: Ack
> Patch 0004: Ack
> Patch 0005: Ack
> 
> On Fri, Apr 06, 2012 at 01:52:36PM +0200, Jan Zelený wrote:
> > > > > Patch 0006: Retrieve subdomains if there is a request for fully
> > > > > qualified user
> > > > > Looks OK, will just need amendment when sss_dp_get_domains_recv() is
> > > > > implemented. Currently the patch supports users and netgroups, are
> > > > > there plans to also support services and the SSH responder? I think we
> > > > > can leave out the sudo responder for now, sudo doesn't support domains
> > > > > (yet?). Autofs in theory can send a request for a fully-qualified map,
> > > > > but it doesn't so the work can be done later, although I would prefer
> > > > > a ticket to show that we are aware of the issue.
> > > > 
> > > > The amendment has been done.
> > > 
> > > No, sss_dp_get_domains_recv() is not used anywhere.
> > 
> > Fixed
> > 
> > > > About support of another responders - I honestly
> > > > don't know. I wasn't aware that a request for fully qualified service/ssh
> > > > entity can be given to SSSD. Perhaps Sumit will know more.
> > > 
> > > Replied elsewhere.
> > 
> > Fixed
> > 
> 
> Please also remove "subreq" from setnetgrent_send() and the declaration
> of setnetgrent_cb().
> 
> Patch 0007: Ack
> Patch 0008: Ack
> Patch 0009: Ack
> Patch 0010: Ack
> Patch 0011: Ack
> 
> > > > > Patch 0012: New config options for subdomains
> > > > > See my reply in the other part of this thread. If you are goig forward
> > > > > with these options, add them to the configAPI.
> > > > 
> > > > Based on previous discussion I removed the part which is setting default
> > > > shell. I intend to keep the homedir for two reasons:
> > > > 
> > > > I recalled why I created the config option in the first place. It's
> > > > because override_homedir can't be set per domain. After your comment
> > > > yesterday, I checked to be sure and IMO the man page is incorrect and
> > > > the override_homedir only works for responder.
> > > 
> > > What was your testcase and what exactly wasn't working for you?
> > > 
> > > The option works fine for me in a subdomain and the code seems to be quite
> > > clear (get_homedir_override())
> > 
> > I know it is straightforward, but it didn't work for me.
> 
> Can you file a bug if the override is not working in a domain? It
> should and if something is not clear, then it's most probably a docs
> bug..
> 
> > However when looking 
> > a bit deeper I discovered a bug in completely different part of the patch set. 
> > This is now fixed and the override works as expected.
> > 
> > > > The second reason is that admins might want to separate homedirs for
> > > > native users from homedirs for trusted users on some systems.
> > > 
> > > Then let's extend the homedir template by a subdomain name or "full
> > > domain name" option.
> > 
> > I'm not sure about this. For one thing this functionality is already there. 
> > But I was considering scenario when the path to subdomain homedirs is 
> > completely different. Something like this:
> > 
> > /home/ipa/%u - the main IPA domain
> > /home/ad/%u  - AD subdomains
> > 
> > I there is a consensus that it is not necessary, I can remove it. But frankly 
> > I can see us having exactly this RFE several months later.
> > 
> 
> If the AD users are to be treated differently than native IPA users the
> new option is OK.
> 
> Patch 0013: Ack
> 
> > > > > Patch 0014: Add s2n extended operation
> > > > > Old DEBUG macros again. There is a typo in s2n_response_to_attrs, one
> > > > > of the debug messages says "go [%s]", should be "got [%s]". Otherwise
> > > > > looks fine.
> > > > 
> > > > Done
> > > 
> > > GCC emits these warnings when compiling with -O2:
> > > src/providers/ipa/ipa_s2n_exop.c: In function 'ipa_s2n_get_user_done':
> > > src/providers/ipa/ipa_s2n_exop.c:408:16: warning: 'retoid' may be used
> > > uninitialized in this function [-Wuninitialized]
> > > src/providers/ipa/ipa_s2n_exop.c:582:11: note: 'retoid' was declared
> > > here
> > > src/providers/ipa/ipa_s2n_exop.c:408:35: warning: 'retdata' may be used
> > > uninitialized in this function [-Wuninitialized]
> > > src/providers/ipa/ipa_s2n_exop.c:583:20: note: 'retdata' was declared
> > > here
> > > src/providers/ipa/ipa_s2n_exop.c: In function
> > > 'ipa_s2n_get_acct_info_send':
> > > src/providers/ipa/ipa_s2n_exop.c:84:9: warning: 'bv_req' may be used
> > > uninitialized in this function [-Wuninitialized]
> > > src/providers/ipa/ipa_s2n_exop.c:532:20: note: 'bv_req' was declared
> > > here
> > 
> > All fixed
> > 
> 
> Ack
> 
> Patch 0015: Ack
> Patch 0016: Ack
> Patch 0017: Ack
> Patch 0018: Ack
> Patch 0019: Ack
> Patch 0020: Ack
> Patch 0021: Ack
> 
> Becuse the only remaining review issue is a trivial one and I know that
> Stephen has been reviewing the patches as well, feel free to resend
> another patch set when/if Stephen sends his comments.
> 
> Great work!

There seems to be a startup error with non-IPA back ends:

(Tue Apr 10 17:54:11 2012) [sssd[be[ldap]]] [load_backend_module]
(0x0010): Unable to load init fn sssm_ldap_subdomains_init from module
ldap, error: /usr/lib64/sssd/libsss_ldap.so: undefined symbol:
sssm_ldap_subdomains_init




More information about the sssd-devel mailing list