selftest: re-enable nss_winbind via nss_wrapper in the test-envs.

[Andrew Bartlett]

On Wed, 2015-02-18 at 11:21 +0100, Michael Adam wrote:
> On 2015-02-18 at 23:00 +1300, Andrew Bartlett wrote:
> > On Wed, 2015-02-18 at 09:43 +0100, Michael Adam wrote:
> > > 
> > > I don't think I agree to revert the original patch:
> > > Reading to the bottom of what you are saying, the test
> > > was originally created by comparing what came out of
> > > the NT->POSIX acl conversion and hard-wiring that into
> > > the checks. Now that we fixed the environment to more
> > > closely reflect a real setup (add missing nss_winbind),
> > > we adapted the test the same way. My guess is that you
> > > would have come up with the exact same test in the first
> > > place had the environment been correct when you did it.
> > > 
> > > So my approach would be to take this patch now, and then
> > > possibly try to understand afterwards what was going on.
> > > Reverting back to a broken environment in order to keep
> > > a test that is broken because it reflects the broken
> > > environment is not a good option imho. :-)
> > 
> > I don't see the way were running things before as broken - many sites do
> > run without nss_winbindd on the DC, as we strongly discourage using it
> > as a general purpose file server. 
> 
> Hmmm. I don't see what this has to do with a general purpose
> file server. My understanding was that a DC without nss_winbind
> is incomplete. 

That isn't the case.  This is a working, and well-used combination. 

> My understanding is also that smbd is not
> functional without the ability to reach out into nss some time
> because it tries to do getpwnam at times. 

The only case I'm aware of is for the [homes].  The other cases are
handled because we go via pdb_samba_dsdb. 

> Maybe this is just
> not true with the way smbd is used in the DC environment, but
> I was not aware. That is the basis of my statement that I
> consider a DC setup without nss_winbind incomplete, or broken. :)

OK.  But given that isn't the case, you might now understand my concern
with your change.

> > What I don't agree with is leaving the change and hoping that we may
> > 'possibly try to understand afterwards what was going on'.  We are all
> > busy, and I fear that just means accepting the change without any
> > assurance of a proper investigation.  
> 
> Well so it was done when the test was originally added.
> Simply adapting the test expectations until they matched
> what comes out of the conversion routine, right? So I am
> not doing anything much worse (except of course ignoring that
> I already know there is a strangeness hidden...).

Indeed, but brushing that strangeness back under the carpet, and just
changing which expected values we use is not OK.  

> > There may well be something funny going on here - the purpose of the
> > test was to highlight such things, and it has done so.  We shouldn't
> > 'just fix it so it passes'.  
> > 
> > Can you please investigate and explain to me why the NSS change made a
> > difference here (it isn't meant to), fix that, and then run the tests
> > with and without nss_winbind?
> 
> Well, given that I have already spent much, much more time on
> this than I intended after doing the "obviously right thing to do"
> namely enabling nss_winbindd in our env, I am inclined to fix the
> test like this for now, after having alerted everyone of the fact
> that there is something strange. I agree that this is not
> optimal, but I am somewhat tight on time to spent more on this
> issue at this moment, and had hoped for others who know more
> about the AD/DC setup to chime in (instead of just being asked to
> fix it myself...) ;-)
> 
> If you want to keep the test as it is, I propose to do a special
> environment for this test only that does not enable nss_winbindd
> for it. But for other envs / tests we do need it, so I would not
> want go back the whole way...

Please restore the test as-was, until you get time to investigate
further what is going on.  

Indeed, our small, 'clearly correct' changes often have unforeseen
impacts.  That may be frustrating, but it is a virtue, not a defect,
because the alternative is to never know we are unexpectedly changing
things at all. 

Thanks!

Andrew Bartlett

-- 
Andrew Bartlett
http://samba.org/~abartlet/
Authentication Developer, Samba Team  http://samba.org
Samba Developer, Catalyst IT          http://catalyst.net.nz/services/samba



-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: This is a digitally signed message part
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20150219/55301493/attachment.pgp>