[SSSD] Support for subdomains - stage 1

Jakub Hrozek jhrozek at redhat.com
Tue Apr 10 04:38:31 UTC 2012


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!



More information about the sssd-devel mailing list