On Fri, Jun 24, 2016 at 05:53:27PM +0200, Martin Basti wrote: > > > On 24.06.2016 15:09, Martin Basti wrote: > > > > > > On 24.06.2016 14:59, Sumit Bose wrote: > > > 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 > > Thank you for explanation, updated patch attached. > > > > Martin^2 > > Requested 'sss_nss_idmap >= 1.14.0' but version of sss_nss_idmap is 1.13.90 > You may find new versions of sss_nss_idmap at http://fedorahosted.org/sssd/ > > libsss_nss_idmap-devel-1.14.0-1.fc24.alpha.x86_64 > > Is it possible that you forgot to increment this version on SSSD side, or it > is my failure?
ah sorry, since 1.14.0 is not release yet we use 1.13.9x to track the alpha and beta releases and still have incrementing version numbers. So, it might be better to use '>= 1.13.90' in the spec file instead of '1.14.0'. bye, Sumit > > Martin^2 > > > > > 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