[PATCH] Add winbind idmap plugin

[Sumit Bose]

Hi,

this patch adds a new plugin similar to the one for the cifs-utils which
allows winbind to use the same id-mapping as SSSD.

Currently I only added it to the dlopen test because I think it would be
best to test it directly when Samba becomes available in the CI.

bye,
Sumit

[Jakub Hrozek]

On Tue, May 10, 2016 at 11:10:05AM +0200, Sumit Bose wrote:
>  Hi,
>  
>  this patch adds a new plugin similar to the one for the cifs-utils which
>  allows winbind to use the same id-mapping as SSSD.
>  
>  Currently I only added it to the dlopen test because I think it would be
>  best to test it directly when Samba becomes available in the CI.
>  
>  bye,
>  Sumit 
>  From b16a64ccf236718a877ab83de1949ab1a8091187 Mon Sep 17 00:00:00
> 2001
>  From: Sumit Bose <sbose(a)redhat.com&gt;
>  Date: Tue, 19 Apr 2016 13:52:59 +0200
>  Subject: [PATCH] Add winbind idmap plugin
>  
>  With this plugin winbind can use the same id-mapping as SSSD which makes
>  it possible to run both together in a consistent way. 
[...]

>  @@ -3710,6 +3757,7 @@ install-data-hook:
>   	if [ ! $(krb5rcachedir) = "__LIBKRB5_DEFAULTS__" ]; then \
>           $(MKDIR_P) $(DESTDIR)/$(krb5rcachedir) ; \
>   	fi
>  +	mv $(DESTDIR)/$(winbindplugindir)/winbind_idmap_sss.so
> $(DESTDIR)/$(winbindplugindir)/sss.so 
You also need to do the same for the uninstall hook otherwise distcheck
fails.

>   
>   uninstall-hook:
>   	if [ -f $(abs_builddir)/src/config/.files2 ]; then \
>  diff --git a/configure.ac b/configure.ac
>  index b4ba366d7a32a45879e9f2e9b6e84256a3ac7235..11d3f9c7333ba814cef54651cbc8e78c610b64e9
> 100644
>  --- a/configure.ac
>  +++ b/configure.ac
>  @@ -126,6 +126,7 @@ WITH_KRB5_CONF
>   WITH_PYTHON2_BINDINGS
>   WITH_PYTHON3_BINDINGS
>   WITH_CIFS_PLUGIN_PATH
>  +WITH_WINBIND_PLUGIN_PATH
>   WITH_SELINUX
>   WITH_NSCD
>   WITH_IPA_GETKEYTAB
>  diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
>  index 2ba6a4d4c919a0697b18c4293f5e33e12b996cac..63d4b28d886259bf7f7f3ea2a6f49a43da00c249
> 100644
>  --- a/contrib/sssd.spec.in
>  +++ b/contrib/sssd.spec.in
>  @@ -1002,6 +1002,7 @@ done
>   %dir %{_libdir}/%{name}
>   %dir %{_libdir}/%{name}/modules
>   %{_libdir}/%{name}/modules/libwbclient.so.*
>  +%{_libdir}/samba/idmap/sss.so 
On a machine that doesn't have winbind installed, the
libdir/samba/idmapdir would be unowned, sssd needs to own them in the
same way it owns %{_libdir}/cifs-utils.

[Sumit Bose]

On Wed, Jun 08, 2016 at 10:50:00AM +0200, Jakub Hrozek wrote:
>  On Tue, May 10, 2016 at 11:10:05AM +0200, Sumit Bose wrote:
>  > Hi,
>  > 
>  > this patch adds a new plugin similar to the one for the cifs-utils which
>  > allows winbind to use the same id-mapping as SSSD.
>  > 
>  > Currently I only added it to the dlopen test because I think it would be
>  > best to test it directly when Samba becomes available in the CI.
>  > 
>  > bye,
>  > Sumit
>  
>  > From b16a64ccf236718a877ab83de1949ab1a8091187 Mon Sep 17 00:00:00 2001
>  > From: Sumit Bose <sbose(a)redhat.com&gt;
>  > Date: Tue, 19 Apr 2016 13:52:59 +0200
>  > Subject: [PATCH] Add winbind idmap plugin
>  > 
>  > With this plugin winbind can use the same id-mapping as SSSD which makes
>  > it possible to run both together in a consistent way.
>  
>  [...]
>  
>  > @@ -3710,6 +3757,7 @@ install-data-hook:
>  >  	if [ ! $(krb5rcachedir) = "__LIBKRB5_DEFAULTS__" ]; then \
>  >          $(MKDIR_P) $(DESTDIR)/$(krb5rcachedir) ; \
>  >  	fi
>  > +	mv $(DESTDIR)/$(winbindplugindir)/winbind_idmap_sss.so
> $(DESTDIR)/$(winbindplugindir)/sss.so
>  
>  You also need to do the same for the uninstall hook otherwise distcheck
>  fails. 
fixed

>  
>  >  
>  >  uninstall-hook:
>  >  	if [ -f $(abs_builddir)/src/config/.files2 ]; then \
>  > diff --git a/configure.ac b/configure.ac
>  > index
> b4ba366d7a32a45879e9f2e9b6e84256a3ac7235..11d3f9c7333ba814cef54651cbc8e78c610b64e9 100644
>  > --- a/configure.ac
>  > +++ b/configure.ac
>  > @@ -126,6 +126,7 @@ WITH_KRB5_CONF
>  >  WITH_PYTHON2_BINDINGS
>  >  WITH_PYTHON3_BINDINGS
>  >  WITH_CIFS_PLUGIN_PATH
>  > +WITH_WINBIND_PLUGIN_PATH
>  >  WITH_SELINUX
>  >  WITH_NSCD
>  >  WITH_IPA_GETKEYTAB
>  > diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
>  > index
> 2ba6a4d4c919a0697b18c4293f5e33e12b996cac..63d4b28d886259bf7f7f3ea2a6f49a43da00c249 100644
>  > --- a/contrib/sssd.spec.in
>  > +++ b/contrib/sssd.spec.in
>  > @@ -1002,6 +1002,7 @@ done
>  >  %dir %{_libdir}/%{name}
>  >  %dir %{_libdir}/%{name}/modules
>  >  %{_libdir}/%{name}/modules/libwbclient.so.*
>  > +%{_libdir}/samba/idmap/sss.so
>  
>  On a machine that doesn't have winbind installed, the
>  libdir/samba/idmapdir would be unowned, sssd needs to own them in the
>  same way it owns %{_libdir}/cifs-utils. 
fixed, I moved the plugin into the sssd-ad package as well. The
libwbclient package does not seem to be the right place because the
idmap plugin implies that winbind running while SSSD's libwbclient
implementation implies to opposite.

New version attached.

bye,
Sumit
>  _______________________________________________
>  sssd-devel mailing list
>  sssd-devel(a)lists.fedorahosted.org
>  https://lists.fedorahosted.org/admin/lists/[e-mail]

[Lukas Slebodnik]

On (08/06/16 13:39), Sumit Bose wrote:
> On Wed, Jun 08, 2016 at 10:50:00AM +0200, Jakub Hrozek wrote:
> > On Tue, May 10, 2016 at 11:10:05AM +0200, Sumit Bose wrote:
> > > Hi,
> > > 
> > > this patch adds a new plugin similar to the one for the cifs-utils which
> > > allows winbind to use the same id-mapping as SSSD.
> > > 
> > > Currently I only added it to the dlopen test because I think it would be
> > > best to test it directly when Samba becomes available in the CI.
> > > 
> > > bye,
> > > Sumit
> > 
> > > From b16a64ccf236718a877ab83de1949ab1a8091187 Mon Sep 17 00:00:00 2001
> > > From: Sumit Bose <sbose(a)redhat.com&gt;
> > > Date: Tue, 19 Apr 2016 13:52:59 +0200
> > > Subject: [PATCH] Add winbind idmap plugin
> > > 
> > > With this plugin winbind can use the same id-mapping as SSSD which makes
> > > it possible to run both together in a consistent way.
> > 
> > [...]
> > 
> > > @@ -3710,6 +3757,7 @@ install-data-hook:
> > >  	if [ ! $(krb5rcachedir) = "__LIBKRB5_DEFAULTS__" ]; then \
> > >          $(MKDIR_P) $(DESTDIR)/$(krb5rcachedir) ; \
> > >  	fi
> > > +	mv $(DESTDIR)/$(winbindplugindir)/winbind_idmap_sss.so
> $(DESTDIR)/$(winbindplugindir)/sss.so
> > 
> > You also need to do the same for the uninstall hook otherwise distcheck
> > fails.
>
> fixed
>
> > 
> > >  
> > >  uninstall-hook:
> > >  	if [ -f $(abs_builddir)/src/config/.files2 ]; then \
> > > diff --git a/configure.ac b/configure.ac
> > > index
> b4ba366d7a32a45879e9f2e9b6e84256a3ac7235..11d3f9c7333ba814cef54651cbc8e78c610b64e9 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -126,6 +126,7 @@ WITH_KRB5_CONF
> > >  WITH_PYTHON2_BINDINGS
> > >  WITH_PYTHON3_BINDINGS
> > >  WITH_CIFS_PLUGIN_PATH
> > > +WITH_WINBIND_PLUGIN_PATH
> > >  WITH_SELINUX
> > >  WITH_NSCD
> > >  WITH_IPA_GETKEYTAB
> > > diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
> > > index
> 2ba6a4d4c919a0697b18c4293f5e33e12b996cac..63d4b28d886259bf7f7f3ea2a6f49a43da00c249 100644
> > > --- a/contrib/sssd.spec.in
> > > +++ b/contrib/sssd.spec.in
> > > @@ -1002,6 +1002,7 @@ done
> > >  %dir %{_libdir}/%{name}
> > >  %dir %{_libdir}/%{name}/modules
> > >  %{_libdir}/%{name}/modules/libwbclient.so.*
> > > +%{_libdir}/samba/idmap/sss.so
> > 
> > On a machine that doesn't have winbind installed, the
> > libdir/samba/idmapdir would be unowned, sssd needs to own them in the
> > same way it owns %{_libdir}/cifs-utils.
>
> fixed, I moved the plugin into the sssd-ad package as well. The
> libwbclient package does not seem to be the right place because the
> idmap plugin implies that winbind running while SSSD's libwbclient
> implementation implies to opposite.
>  
If I understand it correctly this plugin will be used by winbind.
therefore it might be better to create separate sub-package for this file.

Because in container world you will need to install sssd-ad if you want
to use this plugin in "winbind container". It seems to be a overkill.

"winbind container" will need to just this plugin + dependencies
+ bind mounted unix sockets for communication libsss_*idmap.so and sssd daemon.

It seems to me the same use-case as https://fedorahosted.org/sssd/ticket/3024

LS

[Sumit Bose]

On Wed, Jun 08, 2016 at 07:24:02PM +0200, Lukas Slebodnik wrote:
>  On (08/06/16 13:39), Sumit Bose wrote:
>  >On Wed, Jun 08, 2016 at 10:50:00AM +0200, Jakub Hrozek wrote:
>  >> On Tue, May 10, 2016 at 11:10:05AM +0200, Sumit Bose wrote:
>  >> > Hi,
>  >> > 
>  >> > this patch adds a new plugin similar to the one for the cifs-utils which
>  >> > allows winbind to use the same id-mapping as SSSD.
>  >> > 
>  >> > Currently I only added it to the dlopen test because I think it would be
>  >> > best to test it directly when Samba becomes available in the CI.
>  >> > 
>  >> > bye,
>  >> > Sumit
>  >> 
>  >> > From b16a64ccf236718a877ab83de1949ab1a8091187 Mon Sep 17 00:00:00 2001
>  >> > From: Sumit Bose <sbose(a)redhat.com&gt;
>  >> > Date: Tue, 19 Apr 2016 13:52:59 +0200
>  >> > Subject: [PATCH] Add winbind idmap plugin
>  >> > 
>  >> > With this plugin winbind can use the same id-mapping as SSSD which makes
>  >> > it possible to run both together in a consistent way.
>  >> 
>  >> [...]
>  >> 
>  >> > @@ -3710,6 +3757,7 @@ install-data-hook:
>  >> >  	if [ ! $(krb5rcachedir) = "__LIBKRB5_DEFAULTS__" ]; then \
>  >> >          $(MKDIR_P) $(DESTDIR)/$(krb5rcachedir) ; \
>  >> >  	fi
>  >> > +	mv $(DESTDIR)/$(winbindplugindir)/winbind_idmap_sss.so
> $(DESTDIR)/$(winbindplugindir)/sss.so
>  >> 
>  >> You also need to do the same for the uninstall hook otherwise distcheck
>  >> fails.
>  >
>  >fixed
>  >
>  >> 
>  >> >  
>  >> >  uninstall-hook:
>  >> >  	if [ -f $(abs_builddir)/src/config/.files2 ]; then \
>  >> > diff --git a/configure.ac b/configure.ac
>  >> > index
> b4ba366d7a32a45879e9f2e9b6e84256a3ac7235..11d3f9c7333ba814cef54651cbc8e78c610b64e9 100644
>  >> > --- a/configure.ac
>  >> > +++ b/configure.ac
>  >> > @@ -126,6 +126,7 @@ WITH_KRB5_CONF
>  >> >  WITH_PYTHON2_BINDINGS
>  >> >  WITH_PYTHON3_BINDINGS
>  >> >  WITH_CIFS_PLUGIN_PATH
>  >> > +WITH_WINBIND_PLUGIN_PATH
>  >> >  WITH_SELINUX
>  >> >  WITH_NSCD
>  >> >  WITH_IPA_GETKEYTAB
>  >> > diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
>  >> > index
> 2ba6a4d4c919a0697b18c4293f5e33e12b996cac..63d4b28d886259bf7f7f3ea2a6f49a43da00c249 100644
>  >> > --- a/contrib/sssd.spec.in
>  >> > +++ b/contrib/sssd.spec.in
>  >> > @@ -1002,6 +1002,7 @@ done
>  >> >  %dir %{_libdir}/%{name}
>  >> >  %dir %{_libdir}/%{name}/modules
>  >> >  %{_libdir}/%{name}/modules/libwbclient.so.*
>  >> > +%{_libdir}/samba/idmap/sss.so
>  >> 
>  >> On a machine that doesn't have winbind installed, the
>  >> libdir/samba/idmapdir would be unowned, sssd needs to own them in the
>  >> same way it owns %{_libdir}/cifs-utils.
>  >
>  >fixed, I moved the plugin into the sssd-ad package as well. The
>  >libwbclient package does not seem to be the right place because the
>  >idmap plugin implies that winbind running while SSSD's libwbclient
>  >implementation implies to opposite.
>  >
>  If I understand it correctly this plugin will be used by winbind.
>  therefore it might be better to create separate sub-package for this file.
>  
>  Because in container world you will need to install sssd-ad if you want
>  to use this plugin in "winbind container". It seems to be a overkill.
>  
>  "winbind container" will need to just this plugin + dependencies
>  + bind mounted unix sockets for communication libsss_*idmap.so and sssd daemon. 
Makes sense. I put the plugin into a new package and to not make it feel
lonely I added a man page as well.

bye,
Sumit

>  
>  It seems to me the same use-case as https://fedorahosted.org/sssd/ticket/3024
>  
>  LS
>  _______________________________________________
>  sssd-devel mailing list
>  sssd-devel(a)lists.fedorahosted.org
>  https://lists.fedorahosted.org/admin/lists/[e-mail]

[Lukas Slebodnik]

On (13/06/16 15:44), Sumit Bose wrote:
> On Wed, Jun 08, 2016 at 07:24:02PM +0200, Lukas Slebodnik wrote:
> > On (08/06/16 13:39), Sumit Bose wrote:
> > >On Wed, Jun 08, 2016 at 10:50:00AM +0200, Jakub Hrozek wrote:
> > >> On Tue, May 10, 2016 at 11:10:05AM +0200, Sumit Bose wrote:
> > >> > Hi,
> > >> > 
> > >> > this patch adds a new plugin similar to the one for the cifs-utils
> which
> > >> > allows winbind to use the same id-mapping as SSSD.
> > >> > 
> > >> > Currently I only added it to the dlopen test because I think it would
> be
> > >> > best to test it directly when Samba becomes available in the CI.
> > >> > 
> > >> > bye,
> > >> > Sumit
> > >> 
> > >> > From b16a64ccf236718a877ab83de1949ab1a8091187 Mon Sep 17 00:00:00 2001
> > >> > From: Sumit Bose <sbose(a)redhat.com&gt;
> > >> > Date: Tue, 19 Apr 2016 13:52:59 +0200
> > >> > Subject: [PATCH] Add winbind idmap plugin
> > >> > 
> > >> > With this plugin winbind can use the same id-mapping as SSSD which
> makes
> > >> > it possible to run both together in a consistent way.
> > >> 
> > >> [...]
> > >> 
> > >> > @@ -3710,6 +3757,7 @@ install-data-hook:
> > >> >  	if [ ! $(krb5rcachedir) = "__LIBKRB5_DEFAULTS__" ]; then \
> > >> >          $(MKDIR_P) $(DESTDIR)/$(krb5rcachedir) ; \
> > >> >  	fi
> > >> > +	mv $(DESTDIR)/$(winbindplugindir)/winbind_idmap_sss.so
> $(DESTDIR)/$(winbindplugindir)/sss.so
> > >> 
> > >> You also need to do the same for the uninstall hook otherwise distcheck
> > >> fails.
> > >
> > >fixed
> > >
> > >> 
> > >> >  
> > >> >  uninstall-hook:
> > >> >  	if [ -f $(abs_builddir)/src/config/.files2 ]; then \
> > >> > diff --git a/configure.ac b/configure.ac
> > >> > index
> b4ba366d7a32a45879e9f2e9b6e84256a3ac7235..11d3f9c7333ba814cef54651cbc8e78c610b64e9 100644
> > >> > --- a/configure.ac
> > >> > +++ b/configure.ac
> > >> > @@ -126,6 +126,7 @@ WITH_KRB5_CONF
> > >> >  WITH_PYTHON2_BINDINGS
> > >> >  WITH_PYTHON3_BINDINGS
> > >> >  WITH_CIFS_PLUGIN_PATH
> > >> > +WITH_WINBIND_PLUGIN_PATH
> > >> >  WITH_SELINUX
> > >> >  WITH_NSCD
> > >> >  WITH_IPA_GETKEYTAB
> > >> > diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
> > >> > index
> 2ba6a4d4c919a0697b18c4293f5e33e12b996cac..63d4b28d886259bf7f7f3ea2a6f49a43da00c249 100644
> > >> > --- a/contrib/sssd.spec.in
> > >> > +++ b/contrib/sssd.spec.in
> > >> > @@ -1002,6 +1002,7 @@ done
> > >> >  %dir %{_libdir}/%{name}
> > >> >  %dir %{_libdir}/%{name}/modules
> > >> >  %{_libdir}/%{name}/modules/libwbclient.so.*
> > >> > +%{_libdir}/samba/idmap/sss.so
> > >> 
> > >> On a machine that doesn't have winbind installed, the
> > >> libdir/samba/idmapdir would be unowned, sssd needs to own them in the
> > >> same way it owns %{_libdir}/cifs-utils.
> > >
> > >fixed, I moved the plugin into the sssd-ad package as well. The
> > >libwbclient package does not seem to be the right place because the
> > >idmap plugin implies that winbind running while SSSD's libwbclient
> > >implementation implies to opposite.
> > >
> > If I understand it correctly this plugin will be used by winbind.
> > therefore it might be better to create separate sub-package for this file.
> > 
> > Because in container world you will need to install sssd-ad if you want
> > to use this plugin in "winbind container". It seems to be a overkill.
> > 
> > "winbind container" will need to just this plugin + dependencies
> > + bind mounted unix sockets for communication libsss_*idmap.so and sssd daemon.
>
> Makes sense. I put the plugin into a new package and to not make it feel
> lonely I added a man page as well.
>  
LGTM.

BTW we can add Requires/Recommends into pacakge sssd-ad for this sub-pacakge.
So it will be installed by default.
But I can amend patch before pushing if there are not any other comments
to this patch.

LS

[Sumit Bose]

On Mon, Jun 20, 2016 at 11:15:20AM +0200, Lukas Slebodnik wrote:
>  On (13/06/16 15:44), Sumit Bose wrote:
>  >On Wed, Jun 08, 2016 at 07:24:02PM +0200, Lukas Slebodnik wrote:
>  >> On (08/06/16 13:39), Sumit Bose wrote:
>  >> >On Wed, Jun 08, 2016 at 10:50:00AM +0200, Jakub Hrozek wrote:
>  >> >> On Tue, May 10, 2016 at 11:10:05AM +0200, Sumit Bose wrote:
>  >> >> > Hi,
>  >> >> > 
>  >> >> > this patch adds a new plugin similar to the one for the cifs-utils
> which
>  >> >> > allows winbind to use the same id-mapping as SSSD.
>  >> >> > 
>  >> >> > Currently I only added it to the dlopen test because I think it
> would be
>  >> >> > best to test it directly when Samba becomes available in the CI.
>  >> >> > 
>  >> >> > bye,
>  >> >> > Sumit
>  >> >> 
>  >> >> > From b16a64ccf236718a877ab83de1949ab1a8091187 Mon Sep 17 00:00:00
> 2001
>  >> >> > From: Sumit Bose <sbose(a)redhat.com&gt;
>  >> >> > Date: Tue, 19 Apr 2016 13:52:59 +0200
>  >> >> > Subject: [PATCH] Add winbind idmap plugin
>  >> >> > 
>  >> >> > With this plugin winbind can use the same id-mapping as SSSD which
> makes
>  >> >> > it possible to run both together in a consistent way.
>  >> >> 
>  >> >> [...]
>  >> >> 
>  >> >> > @@ -3710,6 +3757,7 @@ install-data-hook:
>  >> >> >  	if [ ! $(krb5rcachedir) = "__LIBKRB5_DEFAULTS__" ];
> then \
>  >> >> >          $(MKDIR_P) $(DESTDIR)/$(krb5rcachedir) ; \
>  >> >> >  	fi
>  >> >> > +	mv $(DESTDIR)/$(winbindplugindir)/winbind_idmap_sss.so
> $(DESTDIR)/$(winbindplugindir)/sss.so
>  >> >> 
>  >> >> You also need to do the same for the uninstall hook otherwise
> distcheck
>  >> >> fails.
>  >> >
>  >> >fixed
>  >> >
>  >> >> 
>  >> >> >  
>  >> >> >  uninstall-hook:
>  >> >> >  	if [ -f $(abs_builddir)/src/config/.files2 ]; then \
>  >> >> > diff --git a/configure.ac b/configure.ac
>  >> >> > index
> b4ba366d7a32a45879e9f2e9b6e84256a3ac7235..11d3f9c7333ba814cef54651cbc8e78c610b64e9 100644
>  >> >> > --- a/configure.ac
>  >> >> > +++ b/configure.ac
>  >> >> > @@ -126,6 +126,7 @@ WITH_KRB5_CONF
>  >> >> >  WITH_PYTHON2_BINDINGS
>  >> >> >  WITH_PYTHON3_BINDINGS
>  >> >> >  WITH_CIFS_PLUGIN_PATH
>  >> >> > +WITH_WINBIND_PLUGIN_PATH
>  >> >> >  WITH_SELINUX
>  >> >> >  WITH_NSCD
>  >> >> >  WITH_IPA_GETKEYTAB
>  >> >> > diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
>  >> >> > index
> 2ba6a4d4c919a0697b18c4293f5e33e12b996cac..63d4b28d886259bf7f7f3ea2a6f49a43da00c249 100644
>  >> >> > --- a/contrib/sssd.spec.in
>  >> >> > +++ b/contrib/sssd.spec.in
>  >> >> > @@ -1002,6 +1002,7 @@ done
>  >> >> >  %dir %{_libdir}/%{name}
>  >> >> >  %dir %{_libdir}/%{name}/modules
>  >> >> >  %{_libdir}/%{name}/modules/libwbclient.so.*
>  >> >> > +%{_libdir}/samba/idmap/sss.so
>  >> >> 
>  >> >> On a machine that doesn't have winbind installed, the
>  >> >> libdir/samba/idmapdir would be unowned, sssd needs to own them in the
>  >> >> same way it owns %{_libdir}/cifs-utils.
>  >> >
>  >> >fixed, I moved the plugin into the sssd-ad package as well. The
>  >> >libwbclient package does not seem to be the right place because the
>  >> >idmap plugin implies that winbind running while SSSD's libwbclient
>  >> >implementation implies to opposite.
>  >> >
>  >> If I understand it correctly this plugin will be used by winbind.
>  >> therefore it might be better to create separate sub-package for this file.
>  >> 
>  >> Because in container world you will need to install sssd-ad if you want
>  >> to use this plugin in "winbind container". It seems to be a overkill.
>  >> 
>  >> "winbind container" will need to just this plugin + dependencies
>  >> + bind mounted unix sockets for communication libsss_*idmap.so and sssd daemon.
>  >
>  >Makes sense. I put the plugin into a new package and to not make it feel
>  >lonely I added a man page as well.
>  >
>  LGTM.
>  
>  BTW we can add Requires/Recommends into pacakge sssd-ad for this sub-pacakge.
>  So it will be installed by default. 
I think this is not needed. It is only needed for samba, not on an
general AD client. And even with samba people might want to select
between the idmap plugin and SSSD's libwbclient implementation.

bye,
Sumit

> But I can amend patch before pushing if there are not any other comments
> to this patch.
> 
> LS
> _______________________________________________
> sssd-devel mailing list
> sssd-devel(a)lists.fedorahosted.org
> https://lists.fedorahosted.org/admin/lists/[e-mail]

[Lukas Slebodnik]

On (20/06/16 11:48), Sumit Bose wrote:
> On Mon, Jun 20, 2016 at 11:15:20AM +0200, Lukas Slebodnik wrote:
> > On (13/06/16 15:44), Sumit Bose wrote:
> > >On Wed, Jun 08, 2016 at 07:24:02PM +0200, Lukas Slebodnik wrote:
> > >> On (08/06/16 13:39), Sumit Bose wrote:
> > >> >On Wed, Jun 08, 2016 at 10:50:00AM +0200, Jakub Hrozek wrote:
> > >> >> On Tue, May 10, 2016 at 11:10:05AM +0200, Sumit Bose wrote:
> > >> >> > Hi,
> > >> >> > 
> > >> >> > this patch adds a new plugin similar to the one for the
> cifs-utils which
> > >> >> > allows winbind to use the same id-mapping as SSSD.
> > >> >> > 
> > >> >> > Currently I only added it to the dlopen test because I think
> it would be
> > >> >> > best to test it directly when Samba becomes available in the
> CI.
> > >> >> > 
> > >> >> > bye,
> > >> >> > Sumit
> > >> >> 
> > >> >> > From b16a64ccf236718a877ab83de1949ab1a8091187 Mon Sep 17
> 00:00:00 2001
> > >> >> > From: Sumit Bose <sbose(a)redhat.com&gt;
> > >> >> > Date: Tue, 19 Apr 2016 13:52:59 +0200
> > >> >> > Subject: [PATCH] Add winbind idmap plugin
> > >> >> > 
> > >> >> > With this plugin winbind can use the same id-mapping as SSSD
> which makes
> > >> >> > it possible to run both together in a consistent way.
> > >> >> 
> > >> >> [...]
> > >> >> 
> > >> >> > @@ -3710,6 +3757,7 @@ install-data-hook:
> > >> >> >  	if [ ! $(krb5rcachedir) = "__LIBKRB5_DEFAULTS__"
> ]; then \
> > >> >> >          $(MKDIR_P) $(DESTDIR)/$(krb5rcachedir) ; \
> > >> >> >  	fi
> > >> >> > +	mv $(DESTDIR)/$(winbindplugindir)/winbind_idmap_sss.so
> $(DESTDIR)/$(winbindplugindir)/sss.so
> > >> >> 
> > >> >> You also need to do the same for the uninstall hook otherwise
> distcheck
> > >> >> fails.
> > >> >
> > >> >fixed
> > >> >
> > >> >> 
> > >> >> >  
> > >> >> >  uninstall-hook:
> > >> >> >  	if [ -f $(abs_builddir)/src/config/.files2 ]; then \
> > >> >> > diff --git a/configure.ac b/configure.ac
> > >> >> > index
> b4ba366d7a32a45879e9f2e9b6e84256a3ac7235..11d3f9c7333ba814cef54651cbc8e78c610b64e9 100644
> > >> >> > --- a/configure.ac
> > >> >> > +++ b/configure.ac
> > >> >> > @@ -126,6 +126,7 @@ WITH_KRB5_CONF
> > >> >> >  WITH_PYTHON2_BINDINGS
> > >> >> >  WITH_PYTHON3_BINDINGS
> > >> >> >  WITH_CIFS_PLUGIN_PATH
> > >> >> > +WITH_WINBIND_PLUGIN_PATH
> > >> >> >  WITH_SELINUX
> > >> >> >  WITH_NSCD
> > >> >> >  WITH_IPA_GETKEYTAB
> > >> >> > diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
> > >> >> > index
> 2ba6a4d4c919a0697b18c4293f5e33e12b996cac..63d4b28d886259bf7f7f3ea2a6f49a43da00c249 100644
> > >> >> > --- a/contrib/sssd.spec.in
> > >> >> > +++ b/contrib/sssd.spec.in
> > >> >> > @@ -1002,6 +1002,7 @@ done
> > >> >> >  %dir %{_libdir}/%{name}
> > >> >> >  %dir %{_libdir}/%{name}/modules
> > >> >> >  %{_libdir}/%{name}/modules/libwbclient.so.*
> > >> >> > +%{_libdir}/samba/idmap/sss.so
> > >> >> 
> > >> >> On a machine that doesn't have winbind installed, the
> > >> >> libdir/samba/idmapdir would be unowned, sssd needs to own them in
> the
> > >> >> same way it owns %{_libdir}/cifs-utils.
> > >> >
> > >> >fixed, I moved the plugin into the sssd-ad package as well. The
> > >> >libwbclient package does not seem to be the right place because the
> > >> >idmap plugin implies that winbind running while SSSD's libwbclient
> > >> >implementation implies to opposite.
> > >> >
> > >> If I understand it correctly this plugin will be used by winbind.
> > >> therefore it might be better to create separate sub-package for this file.
> > >> 
> > >> Because in container world you will need to install sssd-ad if you want
> > >> to use this plugin in "winbind container". It seems to be a
> overkill.
> > >> 
> > >> "winbind container" will need to just this plugin + dependencies
> > >> + bind mounted unix sockets for communication libsss_*idmap.so and sssd
> daemon.
> > >
> > >Makes sense. I put the plugin into a new package and to not make it feel
> > >lonely I added a man page as well.
> > >
> > LGTM.
> > 
> > BTW we can add Requires/Recommends into pacakge sssd-ad for this sub-pacakge.
> > So it will be installed by default.
>
> I think this is not needed. It is only needed for samba, not on an
> general AD client. And even with samba people might want to select
> between the idmap plugin and SSSD's libwbclient implementation.
>  
OK,

ACK

LS

[Lukas Slebodnik]

On (21/06/16 12:11), Lukas Slebodnik wrote:
> On (20/06/16 11:48), Sumit Bose wrote:
> >On Mon, Jun 20, 2016 at 11:15:20AM +0200, Lukas Slebodnik wrote:
> >> On (13/06/16 15:44), Sumit Bose wrote:
> >> >On Wed, Jun 08, 2016 at 07:24:02PM +0200, Lukas Slebodnik wrote:
> >> >> On (08/06/16 13:39), Sumit Bose wrote:
> >> >> >On Wed, Jun 08, 2016 at 10:50:00AM +0200, Jakub Hrozek wrote:
> >> >> >> On Tue, May 10, 2016 at 11:10:05AM +0200, Sumit Bose wrote:
> >> >> >> > Hi,
> >> >> >> > 
> >> >> >> > this patch adds a new plugin similar to the one for the
> cifs-utils which
> >> >> >> > allows winbind to use the same id-mapping as SSSD.
> >> >> >> > 
> >> >> >> > Currently I only added it to the dlopen test because I
> think it would be
> >> >> >> > best to test it directly when Samba becomes available in
> the CI.
> >> >> >> > 
> >> >> >> > bye,
> >> >> >> > Sumit
> >> >> >> 
> >> >> >> > From b16a64ccf236718a877ab83de1949ab1a8091187 Mon Sep 17
> 00:00:00 2001
> >> >> >> > From: Sumit Bose <sbose(a)redhat.com&gt;
> >> >> >> > Date: Tue, 19 Apr 2016 13:52:59 +0200
> >> >> >> > Subject: [PATCH] Add winbind idmap plugin
> >> >> >> > 
> >> >> >> > With this plugin winbind can use the same id-mapping as
> SSSD which makes
> >> >> >> > it possible to run both together in a consistent way.
> >> >> >> 
> >> >> >> [...]
> >> >> >> 
> >> >> >> > @@ -3710,6 +3757,7 @@ install-data-hook:
> >> >> >> >  	if [ ! $(krb5rcachedir) =
> "__LIBKRB5_DEFAULTS__" ]; then \
> >> >> >> >          $(MKDIR_P) $(DESTDIR)/$(krb5rcachedir) ; \
> >> >> >> >  	fi
> >> >> >> > +	mv $(DESTDIR)/$(winbindplugindir)/winbind_idmap_sss.so
> $(DESTDIR)/$(winbindplugindir)/sss.so
> >> >> >> 
> >> >> >> You also need to do the same for the uninstall hook otherwise
> distcheck
> >> >> >> fails.
> >> >> >
> >> >> >fixed
> >> >> >
> >> >> >> 
> >> >> >> >  
> >> >> >> >  uninstall-hook:
> >> >> >> >  	if [ -f $(abs_builddir)/src/config/.files2 ]; then \
> >> >> >> > diff --git a/configure.ac b/configure.ac
> >> >> >> > index
> b4ba366d7a32a45879e9f2e9b6e84256a3ac7235..11d3f9c7333ba814cef54651cbc8e78c610b64e9 100644
> >> >> >> > --- a/configure.ac
> >> >> >> > +++ b/configure.ac
> >> >> >> > @@ -126,6 +126,7 @@ WITH_KRB5_CONF
> >> >> >> >  WITH_PYTHON2_BINDINGS
> >> >> >> >  WITH_PYTHON3_BINDINGS
> >> >> >> >  WITH_CIFS_PLUGIN_PATH
> >> >> >> > +WITH_WINBIND_PLUGIN_PATH
> >> >> >> >  WITH_SELINUX
> >> >> >> >  WITH_NSCD
> >> >> >> >  WITH_IPA_GETKEYTAB
> >> >> >> > diff --git a/contrib/sssd.spec.in b/contrib/sssd.spec.in
> >> >> >> > index
> 2ba6a4d4c919a0697b18c4293f5e33e12b996cac..63d4b28d886259bf7f7f3ea2a6f49a43da00c249 100644
> >> >> >> > --- a/contrib/sssd.spec.in
> >> >> >> > +++ b/contrib/sssd.spec.in
> >> >> >> > @@ -1002,6 +1002,7 @@ done
> >> >> >> >  %dir %{_libdir}/%{name}
> >> >> >> >  %dir %{_libdir}/%{name}/modules
> >> >> >> >  %{_libdir}/%{name}/modules/libwbclient.so.*
> >> >> >> > +%{_libdir}/samba/idmap/sss.so
> >> >> >> 
> >> >> >> On a machine that doesn't have winbind installed, the
> >> >> >> libdir/samba/idmapdir would be unowned, sssd needs to own them
> in the
> >> >> >> same way it owns %{_libdir}/cifs-utils.
> >> >> >
> >> >> >fixed, I moved the plugin into the sssd-ad package as well. The
> >> >> >libwbclient package does not seem to be the right place because the
> >> >> >idmap plugin implies that winbind running while SSSD's
> libwbclient
> >> >> >implementation implies to opposite.
> >> >> >
> >> >> If I understand it correctly this plugin will be used by winbind.
> >> >> therefore it might be better to create separate sub-package for this
> file.
> >> >> 
> >> >> Because in container world you will need to install sssd-ad if you want
> >> >> to use this plugin in "winbind container". It seems to be a
> overkill.
> >> >> 
> >> >> "winbind container" will need to just this plugin +
> dependencies
> >> >> + bind mounted unix sockets for communication libsss_*idmap.so and sssd
> daemon.
> >> >
> >> >Makes sense. I put the plugin into a new package and to not make it feel
> >> >lonely I added a man page as well.
> >> >
> >> LGTM.
> >> 
> >> BTW we can add Requires/Recommends into pacakge sssd-ad for this sub-pacakge.
> >> So it will be installed by default.
> >
> >I think this is not needed. It is only needed for samba, not on an
> >general AD client. And even with samba people might want to select
> >between the idmap plugin and SSSD's libwbclient implementation.
> >
> OK,
>
> ACK
>  
master:
* 1d1a0a019d8d4d9ab0f51ada03604cd2cada287e

If there is a ticket please add commit there and close it.

LS

[Stephen Gallagher]

On 06/20/2016 05:48 AM, Sumit Bose wrote:
>  On Mon, Jun 20, 2016 at 11:15:20AM +0200, Lukas Slebodnik wrote:
> > BTW we can add Requires/Recommends into pacakge sssd-ad for this sub-pacakge.
> > So it will be installed by default.
>  
>  I think this is not needed. It is only needed for samba, not on an
>  general AD client. And even with samba people might want to select
>  between the idmap plugin and SSSD's libwbclient implementation.
>   
What is the advantage of one over the other? This seems to me like one of those
situations where we have two solutions that people aren't realistically going to
know which to pick. In that case, we should probably consider using dependencies
to select the preferred one if neither is currently present.

We can do this by having both packages add a virtual `Provides:
sssd-samba-id-mapping` and have sssd-ad add

Recommends: sssd-samba-id-mapping
Suggests: sssd-libwbclient

(or Suggests: sssd-winbind-idmap whichever one is more feature-complete)


What this will do is pull in the Suggests dependency unless the other one was
specified manually (or already exists on the system) to satisfy the virtual
Provides.

[Sumit Bose]

On Tue, Jun 21, 2016 at 09:01:05AM -0400, Stephen Gallagher wrote:
>  On 06/20/2016 05:48 AM, Sumit Bose wrote:
>  > On Mon, Jun 20, 2016 at 11:15:20AM +0200, Lukas Slebodnik wrote:
>  >> BTW we can add Requires/Recommends into pacakge sssd-ad for this sub-pacakge.
>  >> So it will be installed by default.
>  > 
>  > I think this is not needed. It is only needed for samba, not on an
>  > general AD client. And even with samba people might want to select
>  > between the idmap plugin and SSSD's libwbclient implementation.
>  > 
>  
>  What is the advantage of one over the other? This seems to me like one of those
>  situations where we have two solutions that people aren't realistically going to
>  know which to pick. In that case, we should probably consider using dependencies
>  to select the preferred one if neither is currently present.
>  
>  We can do this by having both packages add a virtual `Provides:
>  sssd-samba-id-mapping` and have sssd-ad add
>  
>  Recommends: sssd-samba-id-mapping
>  Suggests: sssd-libwbclient
>  
>  (or Suggests: sssd-winbind-idmap whichever one is more feature-complete)
>  
>  
>  What this will do is pull in the Suggests dependency unless the other one was
>  specified manually (or already exists on the system) to satisfy the virtual
>  Provides.
>   
But as said the packages are not needed on a general AD client. Only if
Samba and/or Winbind are used either sssd-libwbclient (winbind not
running) or sssd-winbind-idmap (winbind is running) are needed. So for
the time being I would prefer to not pull in one of the package
automatically to keep e.g. the container images small.

Please note that we are working with some Samba developers on how winbind
and SSSD can work better together in future. We have just started so it
is currently not clear what will be the results but of course it should
be easy for a user to configure a Samba server without having to think
about which package to install in which situation.

bye,
Sumit