On Thu, Jan 28, 2016 at 04:19:25PM +0100, Lukas Slebodnik wrote:
> On (27/10/15 22:35), Lukas Slebodnik wrote:
> >On (27/10/15 17:57), Jakub Hrozek wrote:
> >>On Tue, Oct 27, 2015 at 05:42:29PM +0100, Jakub Hrozek wrote:
> >>> On Fri, Oct 23, 2015 at 01:06:36PM +0200, Lukas Slebodnik wrote:
> >>> > On (21/10/15 09:20), Sumit Bose wrote:
> >>> > >On Tue, Oct 20, 2015 at 10:15:06PM +0200, Jakub Hrozek wrote:
> >>> > >> On Tue, Oct 20, 2015 at 05:17:49PM +0300, Nikolai
Kondrashov wrote:
> >>> > >> > Hi Jakub,
> >>> > >> >
> >>> > >> > On 10/19/2015 09:43 PM, Jakub Hrozek wrote:
> >>> > >> > >I'm working on pam_sss.so tests[1] and I ran
into a problem that I don't
> >>> > >> > >know how to solve best.
> >>> > >> > >
> >>> > >> > >tl;dr, I would like to set different environment
variables for different
> >>> > >> > >tests in order to set up cwrap libraries
differently per-test.
> >>> > >> > >
> >>> > >> > >I can't use setenv() in the test itself,
because that's too late, I need
> >>> > >> > >the variables to be set when
__attribute__(constructor) is run, so normally
> >>> > >> > >at program startup, when the libraries are
loaded.
> >>> > >> > >
> >>> > >> > >With cmake it's easy, use
set(TEST_ENVIRONMENT). But with autotools, I
> >>> > >> > >only found two ways:
> >>> > >> > > - TESTS_ENVIRONMENT - this is fine, but
it's per Makefile.am. So I
> >>> > >> > > would have to split the tests more, into
pam_wrapper tests that also
> >>> > >> > > require uid_wrapper, tests that only
require pam_wrapper, ...
> >>> > >> > > - LOG_COMPILER - this allows to run a
wrapper script before a test
> >>> > >> > > that receives the test name as argv. So
this is pretty much what I
> >>> > >> > > want except this is a feature new to
automake 1.12, which would
> >>> > >> > > rule out both RHEL-6 and Ubuntu Trusty
(which is used by Travis)
> >>> > >> > >
> >>> > >> > >So I'm really leaning towards creating a
src/tests/cwrap/pwrap/Makefile.am
> >>> > >> > >and src/tests/cwrap/pwrap_root/Makefile.am. The
downside of multiple
> >>> > >> > >Makefile.am files is that there is some code
duplication and the build
> >>> > >> > >takes longer. But I still think there is enough
interest (from us and from
> >>> > >> > >our users) to support git master on old
platforms. I can file a ticket to
> >>> > >> > >remove this and use LOG_COMPILER when we drop
support for RHEL-6 and old
> >>> > >> > >Ubuntu versions...
> >>> > >> > >
> >>> > >> > >If you disagree, please reply, otherwise I'm
going to send a patch with
> >>> > >> > >per-test Makefile...
> >>> > >> >
> >>> > >> > Ah, so these are unit tests, not integration tests?
> >>> > >>
> >>> > >> I'm working on both, actually. The first part is more
or less an
> >>> > >> isolated unit test of all the options that pam_sss
supports. The reason
> >>> > >> is that some options (2FA, smart cards, ...) are not
really easily
> >>> > >> testable without a mock back end, at the moment we only
have openldap in
> >>> > >> the integration tests.
> >>> > >>
> >>> > >> The next step I will start right after I finish this part
is integration
> >>> > >> tests that will exercise LDAP authentication, password
change and maybe
> >>> > >> authorisation if there's time left.
> >>> > >>
> >>> > >> >
> >>> > >> > I'm not sure I understood everything right,
sorry, but perhaps you can find
> >>> > >> > something useful in contrib/ci/run,
contrib/ci/make-check-wrap and
> >>> > >> > contrib/ci/valgrind-condense where CI matches and
handles particular tests
> >>> > >> > differently regardless of whether LOG_COMPILER is
supported or not. Ping me
> >>> > >> > if you need help figuring out what's going on
there.
> >>> > >>
> >>> > >> So more or less I wanted to have two tests and wanted to
run the first
> >>> > >> as (simplified):
> >>> > >> PAM_WRAPPER=1
./src/tests/cwrap/pam_sss_wrapper-tests
> >>> > >> and other as:
> >>> > >> PAM_WRAPPER=1 UID_WRAPPER=1
./src/tests/cwrap/pam_sss_wrapper-root--tests
> >>> > >>
> >>> > >> but it occured to me that I can always start with UID
wrapper, just drop
> >>> > >> privileges if I need a strictly non-root test. It's a
bit of a hack :-)
> >>> > >> but since the root is fake anyway, I think it's
acceptable.
> >>> > >>
> >>> > >> >
> >>> > >> > Granted this is from outside the build, but maybe
you can concoct something
> >>> > >> > from inside as well.
> >>> > >>
> >>> > >> I think this might work as well; thank you!
> >>> > >
> >>> > >I had a short look at libtool. Since we use it during
'make check' not
> >>> > >the actual binaries are called but a libtool generated wrapper
script.
> >>> > >If it would be possible to set the environment variables here
they would
> >>> > >be visible for the binary at startup.
> >>> > >
> >>> > >Libtool has the concept of 'executable wrappers' to
support cygwin and
> >>> > >similar environments but I didn't found an easy way to add
own wrapper
> >>> > >here.
> >>> > >
> >>> > >Adding the variables directly in the generated wrapper scripts
would be
> >>> > >quite a hack. But maybe it would be possible if we add our own
version
> >>> > >of build/ltmain.sh where the wrapper scripts are generated in
> >>> > >func_emit_wrapper()?
> >>> > >
> >>> > >So, I'm afraid this is not a direct answer to your
question but maybe
> >>> > >libtool might be useful here.
> >>> > >
> >>> >
> >>> > I read this thread after are came up with almost similar solution
> >>> > as Sumit. With a small difference. I did not decide to inject env
> >>> > variables to generated script but I decided to write yet another
> >>> > wrapper on top of and set env variable there.
> >>> >
> >>> > Here is a POC version. What do you think about such solution.
> >>> >
> >>> > LS
> >>>
> >>> > From 79e8f0e1cdd09ca791755bb03c1e8f38b8078dc3 Mon Sep 17 00:00:00
2001
> >>> > From: Lukas Slebodnik <lslebodn(a)redhat.com>
> >>> > Date: Fri, 23 Oct 2015 13:06:00 +0200
> >>> > Subject: [PATCH] temp
> >>>
> >>> Thank you, this works like a charm!
> >>>
> >>> ACK (but I had to apply with patch(1) in my pwrap branch, not sure if
> >>> the patch is applicable atop master...)
> >>
> >>Oh and of course please come up with a better commit message :-)
> >>
> >It was a "POC version" therefore such commit message.
> >I think tham make distcheck would not pass with my patch.
> >
> >>if you prefer, I can resend along with the pam_wrapper patches...
> >go ahead
> >
> I was expecting that pam_wrapper patches will lend sooner.
> Do you have a solution with env in WIP patches?
> or should we do it before pam tests?
>
> I just wnat to know what to do with this mail thread in patchwork.
The branch is here:
https://github.com/jhrozek/sssd/commits/pwrap
It contains your patch, Pavel's refactoring patches and the pam_wrapper
pam_sss tests. But the tests don't pass after some latest changes to
pam_wrapper, I need to change them.
I guess it would be best to keep the thread around in patchwork until I
send the patches and then just review them as part of the thread.
This thread is deprecated in favor of "[PATCH] Unit tests for pam_sss using
pam_wrapper"