[SSSD] [PATCH] Split becoming user into a separate module (with tests!)

Lukas Slebodnik lslebodn at redhat.com
Fri Oct 10 10:10:16 UTC 2014


On (09/10/14 20:12), Jakub Hrozek wrote:
>On Thu, Oct 09, 2014 at 05:07:27PM +0200, Lukas Slebodnik wrote:
>> On (08/10/14 17:46), Lukas Slebodnik wrote:
>> >On (06/10/14 15:46), Jakub Hrozek wrote:
>> >>Hi,
>> >>
>> >>attached are more patches that are related to running sssd as a
>> >>non-root. I split the functions to become a different user into a
>> >>separate module and provided unit tests using nss_wrapper and
>> >>uid_wrapper.
>> >>
>> >>There is a separate Makefile.am for the cwrap-based tests, because it's
>> >>not easy (read: possible without a wrapper script) to run tests with a
>> >>custom environment. Some build systems, like cmake, allow that, but
>> >>autotools does not.
>> >
>> >>From 565aca30c666cb696ccb5c6a9426144276c7dce9 Mon Sep 17 00:00:00 2001
>> >>From: Jakub Hrozek <jhrozek at redhat.com>
>> >>Date: Sat, 26 Jul 2014 12:46:26 +0200
>> >>Subject: [PATCH 1/3] UTIL: Move become_user outside krb5 tree
>> >>
>> >>In order for several other SSSD processes to run as a non-root user, we
>> >>need to move the functions to become another user to a shared space in
>> >>our source tree.
>> >>---
>> >> Makefile.am                                          | 20 ++++++++++++--------
>> >> src/providers/krb5/krb5_utils.h                      |  8 --------
>> >> .../krb5/krb5_become_user.c => util/become_user.c}   |  1 -
>> >> src/util/util.h                                      |  9 +++++++++
>> >> 4 files changed, 21 insertions(+), 17 deletions(-)
>> >> rename src/{providers/krb5/krb5_become_user.c => util/become_user.c} (99%)
>> >>
>> >>diff --git a/Makefile.am b/Makefile.am
>> >>index eb0e649437f465d3354a0c063a29c65203824506..ac3f26cad6e1d0a5ac8979ce13286bc896c62b62 100644
>> >>--- a/Makefile.am
>> >>+++ b/Makefile.am
>> >>@@ -1286,13 +1286,14 @@ strtonum_tests_LDADD = \
>> >> krb5_utils_tests_SOURCES = \
>> >>     src/tests/krb5_utils-tests.c \
>> >>     src/providers/krb5/krb5_utils.c \
>> >>-    src/providers/krb5/krb5_become_user.c \
>> >>     src/providers/krb5/krb5_common.c \
>> >>     src/util/sss_krb5.c \
>> >>     src/providers/data_provider_fo.c \
>> >>     src/providers/data_provider_opts.c \
>> >>     src/providers/data_provider_callbacks.c \
>> >>-    $(SSSD_FAILOVER_OBJ)
>> >>+    src/util/become_user.c \
>> >>+    $(SSSD_FAILOVER_OBJ) \
>> >>+    $(NULL)
>> >Does it make sense to add NULL at the end?
>> >You put new file before SSSD_FAILOVER_OBJ.
>> >If you think that it make sense I will not insist on change.
>> >
>
>I'm not sure what you propose as a better way. I'm not opposed to a
>change, but I'm not sure what shall I change.
>
>I added the conditional into specfile. I also squashed in your changes
>(thanks!) and new patches are attached.

I thought that idea of NULL is to avoid changing two lines if files is added to
the end. The file "src/util/become_user.c" was added before variable
SSSD_FAILOVER_OBJ, but it isn't important. We can be consistent and add NULL
also after variable SSSD_FAILOVER_OBJ.


Patches works well. make distcheck is fixed. nitpicks were addressed as well.

ACK

LS



More information about the sssd-devel mailing list