On Tue, Sep 13, 2016 at 09:27:32AM +0200, Lukas Slebodnik wrote:
> On (09/08/16 10:18), Jakub Hrozek wrote:
> >On Tue, Aug 09, 2016 at 08:04:38AM +0200, Lukas Slebodnik wrote:
> >> On (09/05/16 10:07), Jakub Hrozek wrote:
> >> >On Wed, May 04, 2016 at 11:36:57PM +0200, Lukas Slebodnik wrote:
> >> >> On (27/04/16 10:51), Jakub Hrozek wrote:
> >> >> >Hi,
> >> >> >
> >> >> >the attached patches implement unit tests for the pam_sss
module using
> >> >> >pam_wrapper and libpamtest. In my testing, the coverage is
around 75%
> >> >> >with mostly the parts that require running as root being
untested.
> >> >> >
> >> >> >I worked on this patchset even though the features for 1.14 are
in full
> >> >> >swing because there are several tickets that will require us to
patch
> >> >> >pam_sss, so it's important to have the code that changes
tested. In
> >> >> >addition, when we merge Dan's patches to use TLS with
integration tests,
> >> >> >then we'll be able to also test authentication in
integration tests
> >> >> >easily using libpamtest-python.
> >> >> >
> >> >> >However, our CI fails for me constantly:
> >> >> >
http://sssd-ci.duckdns.org/logs/job/42/75/fedora_rawhide/ci.html
> >> >> >The strange thing is that running CI locally works fine and so
does make
> >> >> >check. Can anyone help point me in the right direction as to
what should
> >> >> >I check next? I suspect some of the environment variables might
not be
> >> >> >set correctly, but I don't see why..
> >> >>
> >> >> Are you sure it pass locally with valgrind?
> >> >>
> >> >> It failed because there are valgrind errors.
> >> >> I can see then on fedora 22 and fedora 23
> >> >>
http://sssd-ci.duckdns.org/logs-test/job/2/70/fedora22/ci-build-debug/src...
> >> >>
http://sssd-ci.duckdns.org/logs-test/job/2/70/fedora22/ci-build-debug/src...
> >> >>
> >> >> I cannot see them on fedora 24 or fedora rawhide
> >> >>
http://sssd-ci.duckdns.org/logs-test/job/2/70/fedora_rawhide/ci-build-deb...
> >> >> but it fails as well
> >> >>
http://sssd-ci.duckdns.org/logs-test/job/2/70/fedora_rawhide/ci-build-deb...
> >> >>
> >> >> If the problem is with missing environment variables
> >> >> then you might log all environment variables in main function
> >> >
> >> >just a quick update: the issues in tests were resolved. I hit two bugs
> >> >in pam_wrapper:
> >> >
https://github.com/jhrozek/pam_wrapper/commit/ff7ec1c5ea7ed2360cbc59bd58f...
> >> >
https://github.com/jhrozek/pam_wrapper/commit/c9b11ac8947bc0ff2ec3c4140b4...
> >> >so once Andreas approves them, I will build a new pam_wrapper RPM for
> >> >Fedora and EPEL so that we can formally review the pam_sss test
patches.
> >> Bump
> >
> >I only had time to rebase the patches:
> >
https://github.com/jhrozek/sssd/tree/pwrap
> >But now the tests fail, so I did something wrong, somewhere. I don't
> >think I will have time to look into this until next week, sorry.
>
>
http://sssd-ci.duckdns.org/logs-test/job/4/48/summary.html
>
http://sssd-ci.duckdns.org/logs-test/job/4/49/summary.html
>
> All test passed only on rhel6 :-(
> Maybe because new test was skipped.
>
> cwrap tests were not executed on debian_testing due to following failures
>
/var/lib/jenkins/workspace/ci-test/label/debian_testing/src/tests/cwrap/become_user-tests.sh:
> 3: .: Can't open /cwrap_test_setup.sh
> FAIL become_user-tests.sh (exit status: 2)
>
> @see
>
http://sssd-ci.duckdns.org/logs-test/job/4/48/debian_testing/ci-build-deb...
>
http://sssd-ci.duckdns.org/logs-test/job/4/48/debian_testing/ci-build-deb...
>
http://sssd-ci.duckdns.org/logs-test/job/4/48/debian_testing/ci-build-deb...
>
http://sssd-ci.duckdns.org/logs-test/job/4/48/debian_testing/ci-build-deb...
>
> mock build failed on epel7 (maybe missing pam_sss-tests.sh in tarball)
> make[3]: Entering directory `/builddir/build/BUILD/sssd-1.14.2/src/tests/cwrap'
> make[4]: Entering directory `/builddir/build/BUILD/sssd-1.14.2/src/tests/cwrap'
> make[4]: *** No rule to make target `pam_sss-tests.sh', needed by
> `pam_sss-tests.sh.log'. Stop.
> make[4]: *** Waiting for unfinished jobs...
>
>
> The pam_sss-tests test failed on fedora24. There are valgrind errors.
> ==28387== Invalid read of size 1
> ==28387== at 0x9CC2600: __GI_strchr (in /usr/lib64/libc-2.23.so)
> ==28387== by 0x402234: service_arg (test_wrapper_pam_sss.c:93)
> ==28387== by 0x402D73: test_pam_authenticate_root_ignore
(test_wrapper_pam_sss.c:571)
> ==28387== by 0x565B542: ??? (in /usr/lib64/libcmocka.so.0.3.1)
> ==28387== by 0x565BC7A: _cmocka_run_group_tests (in
/usr/lib64/libcmocka.so.0.3.1)
> ==28387== by 0x401AE0: main (test_wrapper_pam_sss.c:1343)
> ==28387== Address 0xc81b6b3 is 0 bytes after a block of size 227 alloc'd
> ==28387== at 0x4C2BBAD: malloc (vg_replace_malloc.c:299)
> ==28387== by 0x97FF38D: talloc_named_const (in /usr/lib64/libtalloc.so.2.1.6)
> ==28387== by 0x4021A4: service_arg (test_wrapper_pam_sss.c:67)
> ==28387== by 0x402D73: test_pam_authenticate_root_ignore
(test_wrapper_pam_sss.c:571)
> ==28387== by 0x565B542: ??? (in /usr/lib64/libcmocka.so.0.3.1)
> ==28387== by 0x565BC7A: _cmocka_run_group_tests (in
/usr/lib64/libcmocka.so.0.3.1)
> ==28387== by 0x401AE0: main (test_wrapper_pam_sss.c:1343)
>
>
http://sssd-ci.duckdns.org/logs-test/job/4/48/fedora24/ci-build-debug/src...
>
http://sssd-ci.duckdns.org/logs-test/job/4/48/fedora24/ci-build-debug/src...
>
> It would be good to merge patches. Currently we have approcimately 30% of line
> coverage and only 50% of functions are partially covered.
> (measured with unit + integration tests)
Attached patches passed CI:
http://sssd-ci.duckdns.org/logs/job/54/85/summary.html
Since the goal is to have some integration tests with authentication,
I'll also try to add a python-based integration tests. The attached
patches "just" cover pam_sss.so with test cases.
From d5ac8905458e50926f194e5e2bd0eab14b11a538 Mon Sep 17 00:00:00
2001
From: Jakub Hrozek <jhrozek(a)redhat.com>
Date: Thu, 15 Oct 2015 22:26:15 +0200
Subject: [PATCH 01/12] PAM: Move PAM packet parsing functionality to a
separate module
1.) You need to rebase due to pam_response_filter changes
+ fix mocking issues in pam-srv-tests
(maybe I just rebased patches in bad way)
2.) Fix dlopen-tests:
Running suite(s): dlopen
Unchecked library found: pam_test_sss.so
3.) make check failed under root
pam_sss.so has special case for UID 0
Ideal would be test even both cases (UID == 0 and UID != 0)
with uid_wrapper.
4.) hardcoded timezone strings in test
It fails with different timezone e.g.
export TZ=America/Havana
make check
5) Please use asserts or debug_messages in function service_arg
There might be more issues because I reviewed patches briefly
due to my own rebase of patches and failed unit tests.
LS