On Fri, Jun 24, 2016 at 02:00:24PM +0200, Martin Basti wrote:
> 
> 
> On 22.06.2016 23:20, Lukas Slebodnik wrote:
> > On (22/06/16 11:57), Martin Basti wrote:
> > > 
> > > On 09.06.2016 21:02, Martin Basti wrote:
> > > > 
> > > > On 09.06.2016 14:45, Martin Basti wrote:
> > > > > 
> > > > > On 09.06.2016 14:42, Martin Basti wrote:
> > > > > > 
> > > > > > On 09.06.2016 14:38, Lukas Slebodnik wrote:
> > > > > > > On (09/06/16 14:29), Martin Basti wrote:
> > > > > > > > On 09.06.2016 14:22, Alexander Bokovoy wrote:
> > > > > > > > > On Thu, 09 Jun 2016, Jakub Hrozek wrote:
> > > > > > > > > > On Fri, May 20, 2016 at 09:23:46PM +0200, Sumit Bose wrote:
> > > > > > > > > > > Hi,
> > > > > > > > > > > 
> > > > > > > > > > > this patch allows the extom plugin to lookup
> > > > > > > > > > > users by certificate which
> > > > > > > > > > > is needed in the case where a IPA client
> > > > > > > > > > > wants to lookup an AD user who
> > > > > > > > > > > has the certificate stored in AD. To make
> > > > > > > > > > > this work the related patches
> > > > > > > > > > > I just send to sssd-devel are needed as well.
> > > > > > > > > > > 
> > > > > > > > > > > Currently the patches miss the change in the
> > > > > > > > > > > required version of SSSD.
> > > > > > > > > > > since the SSSD patches are not committed. But
> > > > > > > > > > > the patches are needed to
> > > > > > > > > > > fully test the SSSD patches. I will send a
> > > > > > > > > > > new version with the needed
> > > > > > > > > > > changes to the minimal SSSD version when the SSSD patches 
> > > > > > > > > > > are
> > > > > > > > > > > committed.
> > > > > > > > > > > 
> > > > > > > > > > > bye,
> > > > > > > > > > > Sumit
> > > > > > > > > > The patch works fine (tested together with the 
> > > > > > > > > > corresponding SSSD
> > > > > > > > > > patches), so ACK from me. The code also looks
> > > > > > > > > > good to me, but I'm not
> > > > > > > > > > sure if reviewing an IPA patch requires something
> > > > > > > > > > more (CI? Coverity?)
> > > > > > > > > ACK from me as well, I forgot to send email about it,
> > > > > > > > > though I reviewed
> > > > > > > > > this patch a week ago.
> > > > > > > > > 
> > > > > > > > Pushed to master: aa734da49440c5d12c0f8d4566505adaeef254e8
> > > > > > > > 
> > > > > > > It's very likey that this commit will break build of
> > > > > > > freeipa-master. I didn't try.
> > > > > > > 
> > > > > > > Because it uses new function sss_nss_getnamebycert
> > > > > > > from the library libsss_nss_idmap which is not in fedora.
> > > > > > > It was pushed to sssd master just today.
> > > > > > > 
> > > > > > > LS
> > > > > > If this is true, can you/somebody provide the SRPM of SSSD with
> > > > > > the required functionality please? We may need to add it to
> > > > > > @freeipa/freeipa-master copr and bump required version of SSSD.
> > > > > > 
> > > > > > Martin^2
> > > > > > 
> > > > > Yes, you were right, master build is broken.
> > > > > Martin^2
> > > > > 
> > > > SSSD master build has been added to @freeipa/freeipa-master copr as a
> > > > workaround (to unblock automatic testing an developers)
> > > > 
> > > > Please bump version in specfile accordingly (I don't know in which
> > > > version of SSSD will be required function)
> > > > 
> > > > Martin^2
> > > > 
> > > Bumping SSSD version in requires and buildrequires
> > > Patch attached
> > >From f2b394085157954768bc93a73b854778c65bfdcd Mon Sep 17 00:00:00 2001
> > > From: Martin Basti <mba...@redhat.com>
> > > Date: Wed, 22 Jun 2016 10:49:39 +0200
> > > Subject: [PATCH] Bump SSSD requires
> > > 
> > > https://fedorahosted.org/freeipa/ticket/4955
> > > ---
> > > freeipa.spec.in | 4 ++--
> > > 1 file changed, 2 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/freeipa.spec.in b/freeipa.spec.in
> > > index 
> > > 0d5c745d5306cd7141c573454bd1c1e6a78c7e7f..befc7af9ee2ceefa41b1b999df4bdb1c6607bea8
> > >  100644
> > > --- a/freeipa.spec.in
> > > +++ b/freeipa.spec.in
> > > @@ -85,7 +85,7 @@ BuildRequires:  python-pyasn1 >= 0.0.9a
> > > BuildRequires:  python-qrcode-core >= 5.0.0
> > > BuildRequires:  python-dns >= 1.11.1
> > > BuildRequires:  libsss_idmap-devel
> > > -BuildRequires:  libsss_nss_idmap-devel >= 1.12.2
> > > +BuildRequires:  libsss_nss_idmap-devel >= 1.14.0
> > > BuildRequires:  java-headless
> > > BuildRequires:  rhino
> > > BuildRequires:  libverto-devel
> > > @@ -327,7 +327,7 @@ Requires: pam_krb5
> > > Requires: curl
> > > Requires: libcurl >= 7.21.7-2
> > > Requires: xmlrpc-c >= 1.27.4
> > > -Requires: sssd >= 1.13.3-5
> > > +Requires: sssd >= 1.14.0
> > NACK
> Thank you.
> > 
> > A) It's not explained in commit message why you need to bump Requires for 
> > sssd.
> >     IIRC, you need just new libsss_nss_idmap-devel.
> I don't know actually, would be nice if author of the original patch can
> confirm if newer SSSD is required or not

Currently both are required. 'BuildRequires:  libsss_nss_idmap-devel >=
1.14.0' is needed for the build because the new call
sss_nss_getnamebycert() is needed to look up trusted users by
certificate.

At runtime 'Requires: sssd >= 1.14.0' is needed because currently
libsss_nss_idmap does not have a dependency to sssd. If only the
libsss_nss_idmap would be updated and not SSSD the
sss_nss_getnamebycert() would just return a not implemented error code
because the older versions of SSSD cannot handle the request.

HTH

bye,
Sumit

> > B) You forgot add detection for newer version of libsss_nss_idmap at 
> > configure
> >     time
> > Hint: * daemons/configure.ac
> >        * 
> > https://autotools.io/pkgconfig/pkg_check_modules.html#pkgconfig.pkg_check_modules.specification
> Fixed
> > 
> > LS
> 
> Updated patch attached.

> From 34c63f8ba5c478cd95a62bc3dffc6bfc7be3384b Mon Sep 17 00:00:00 2001
> From: Martin Basti <mba...@redhat.com>
> Date: Wed, 22 Jun 2016 10:49:39 +0200
> Subject: [PATCH] Bump libsss_nss_idmap-devel
> 
> This is required by commit aa734da49440c5d12c0f8d4566505adaeef254e8
> 
> https://fedorahosted.org/freeipa/ticket/4955
> ---
>  daemons/configure.ac | 2 +-
>  freeipa.spec.in      | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/daemons/configure.ac b/daemons/configure.ac
> index 
> 2906def285a0f6ad9553fc07cbc59f7a7f7fd426..fa5eab829cad6718f21ec3d5569ffe1b0168e518
>  100644
> --- a/daemons/configure.ac
> +++ b/daemons/configure.ac
> @@ -253,7 +253,7 @@ dnl -- dirsrv is needed for the extdom unit tests --
>  PKG_CHECK_MODULES([DIRSRV], [dirsrv  >= 1.3.0])
>  dnl -- sss_idmap is needed by the extdom exop --
>  PKG_CHECK_MODULES([SSSIDMAP], [sss_idmap])
> -PKG_CHECK_MODULES([SSSNSSIDMAP], [sss_nss_idmap])
> +PKG_CHECK_MODULES([SSSNSSIDMAP], [sss_nss_idmap >= 1.14.0])
>  
>  dnl 
> ---------------------------------------------------------------------------
>  dnl - Check for systemd unit directory
> diff --git a/freeipa.spec.in b/freeipa.spec.in
> index 
> d31ddfaf78a455f4e4d65724bbbe23461e1336e0..e82950d7f82fb5018d893a0644dd1a5931656e2d
>  100644
> --- a/freeipa.spec.in
> +++ b/freeipa.spec.in
> @@ -85,7 +85,7 @@ BuildRequires:  python-pyasn1 >= 0.0.9a
>  BuildRequires:  python-qrcode-core >= 5.0.0
>  BuildRequires:  python-dns >= 1.11.1
>  BuildRequires:  libsss_idmap-devel
> -BuildRequires:  libsss_nss_idmap-devel >= 1.12.2
> +BuildRequires:  libsss_nss_idmap-devel >= 1.14.0
>  BuildRequires:  java-headless
>  BuildRequires:  rhino
>  BuildRequires:  libverto-devel
> -- 
> 2.5.5
> 

> -- 
> Manage your subscription for the Freeipa-devel mailing list:
> https://www.redhat.com/mailman/listinfo/freeipa-devel
> Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to