On (24/06/16 20:00), Alexander Bokovoy wrote: >On Fri, 24 Jun 2016, Sumit Bose wrote: >> 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 <[email protected]> >> > > > > > > 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'. >+1, At this point '>= 1.13.90' should be safe. -1 I vote for official release. I cannot see a reason why this patch should be pushed immediately. 1.13.90 is just a sssd convention for alpha release and it can be confusing for others whyt there isn't tarball for 1.13.90
LS -- 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
