Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-07-01 Thread Martin Basti 



On 24.06.2016 20:41, Lukas Slebodnik wrote:

On (24/06/16 21:09), Alexander Bokovoy wrote:

On Fri, 24 Jun 2016, Lukas Slebodnik wrote:

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

It would allow git master to be built against sssd git master without
additional problems. It is consistency here that I'm after.


It allows it even now and without this patch.
I'm sorry I miss a logic here.

No,

That's not true.

You wrote: "It would allow git master to be built"
against sssd git master"
 ^^
One more time. This patch will not change that
because you can build freeipa git master
against "sssd git master" even now.

It's not my problem that freeipa requires git master of other project.
But it does not mean that you need to officialy requires
some weird version "1.13.90". It is really confusing.


it does not prevent you from running with the code that does not
have needed support. 1.13.4 has no extdom certificate request support
so you would need to make sure you are actually installing the correct
SSSD packages manually while changing the version to 1.13.90 would make
clear we demand a specific functionality.

Building freeipa and installing are two different things.
If you need to install freeipa git master then you need to use
extra copr anyway because:
A) you cannot install freeipa master in rawhide because there isn't
pki-core-10.3.3-1.fc25
b) you cannot install freeipa master on fedora 24 due lots of missing
dependencies (including libsss_nss_idmap-1.14.0)

If one would like to install freeipa git master rpms without copr
then he/she will not able to do it on fedora 24 without new libsss_nss_idmap
because dnf will not be able to find dependency
   "libsss_nss_idmap.so.0(SSS_NSS_IDMAP_0.2.0)(64bit)"


It is a very small thing, of
course, but helpful to those who have to deal with rebases/updates of
their distribution packages and have not been following freeipa-devel@
list in detail. At the very least the inability to find 1.13.90 in a
regular place would cause question being asked.


It's helpful but confusing. That's the reason why we should avoid using
1.13.90. I doubt that anyone will try to use alpha version of sssd or freeipa
on other distributions (debian, ubuntu) especially if they do not work reliably
on fedora. So there's no reason for a rush.

LS


Pushed to master: a635135ba3caa6359c38f305d7982925ef3de50b

Version with dependency on 1.13.90 was pushed, we can increase it when 
SSSD 1.14.0 will be released



From 514cf22027cc8784256595d57b42bea23b1a2a77 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Wed, 22 Jun 2016 10:49:39 +0200
Subject: [PATCH] Bump SSSD version in requires

This is required by commit aa734da49440c5d12c0f8d4566505adaeef254e8 for
function sss_nss_getnamebycert()

https://fedorahosted.org/freeipa/ticket/4955
---
 daemons/configure.ac | 2 +-
 freeipa.spec.in  | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/daemons/configure.ac b/daemons/configure.ac
index 2906def285a0f6ad9553fc07cbc59f7a7f7fd426..94d66d813728fe4e32f9e3c0eef920d8e2395d8f 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.13.90])
 
 dnl ---
 dnl - Check for systemd unit directory
diff --git a/freeipa.spec.in b/freeipa.spec.in
index b04f819a9ceafe9506e0d5a073ae408fe898..71c24fd7c06a84ee1535516afa4fd89ebc0831ff 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
 Requires: python-sssdconfig
 Requires: certmonger >= 0.78
 Requires: nss-tools
-- 
2.5.5

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-24 Thread Lukas Slebodnik 
On (24/06/16 21:09), Alexander Bokovoy wrote:
>On Fri, 24 Jun 2016, Lukas Slebodnik wrote:
>> > > > > 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
>> > It would allow git master to be built against sssd git master without
>> > additional problems. It is consistency here that I'm after.
>> > 
>> It allows it even now and without this patch.
>> I'm sorry I miss a logic here.
>No,
That's not true.

You wrote: "It would allow git master to be built"
against sssd git master"
^^
One more time. This patch will not change that
because you can build freeipa git master
against "sssd git master" even now.

It's not my problem that freeipa requires git master of other project.
But it does not mean that you need to officialy requires
some weird version "1.13.90". It is really confusing.

>it does not prevent you from running with the code that does not
>have needed support. 1.13.4 has no extdom certificate request support
>so you would need to make sure you are actually installing the correct
>SSSD packages manually while changing the version to 1.13.90 would make
>clear we demand a specific functionality.
Building freeipa and installing are two different things.
If you need to install freeipa git master then you need to use
extra copr anyway because:
A) you cannot install freeipa master in rawhide because there isn't
   pki-core-10.3.3-1.fc25
b) you cannot install freeipa master on fedora 24 due lots of missing
   dependencies (including libsss_nss_idmap-1.14.0)

If one would like to install freeipa git master rpms without copr
then he/she will not able to do it on fedora 24 without new libsss_nss_idmap
because dnf will not be able to find dependency
  "libsss_nss_idmap.so.0(SSS_NSS_IDMAP_0.2.0)(64bit)"

>It is a very small thing, of
>course, but helpful to those who have to deal with rebases/updates of
>their distribution packages and have not been following freeipa-devel@
>list in detail. At the very least the inability to find 1.13.90 in a
>regular place would cause question being asked.
>
It's helpful but confusing. That's the reason why we should avoid using
1.13.90. I doubt that anyone will try to use alpha version of sssd or freeipa
on other distributions (debian, ubuntu) especially if they do not work reliably
on fedora. So there's no reason for a rush.

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

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-24 Thread Alexander Bokovoy 

On Fri, 24 Jun 2016, Lukas Slebodnik wrote:

> > 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

It would allow git master to be built against sssd git master without
additional problems. It is consistency here that I'm after.


It allows it even now and without this patch.
I'm sorry I miss a logic here.

No, it does not prevent you from running with the code that does not
have needed support. 1.13.4 has no extdom certificate request support
so you would need to make sure you are actually installing the correct
SSSD packages manually while changing the version to 1.13.90 would make
clear we demand a specific functionality. It is a very small thing, of
course, but helpful to those who have to deal with rebases/updates of
their distribution packages and have not been following freeipa-devel@
list in detail. At the very least the inability to find 1.13.90 in a
regular place would cause question being asked.

Of course, 1.14.0 as a requirement wouldn't be necessarily worse too but
keeping it as 1.13.90 would make freeipa working with packages from sssd
git without any other changes.
--
/ Alexander Bokovoy

--
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

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-24 Thread Lukas Slebodnik 
On (24/06/16 20:43), Alexander Bokovoy wrote:
>On Fri, 24 Jun 2016, Lukas Slebodnik wrote:
>> 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
>> > > >

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-24 Thread Alexander Bokovoy 

On Fri, 24 Jun 2016, Lukas Slebodnik wrote:

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 
> > > > > > 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
> > > > > >

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-24 Thread Lukas Slebodnik 
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 
>> > > > > > > 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 
>> > > > > > >

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-24 Thread Alexander Bokovoy 

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 
> > > > > 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:

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-24 Thread Lukas Slebodnik 
On (24/06/16 17:53), 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 
>> > > > > 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
>> > > > >

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-24 Thread Martin Basti 



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 
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?


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 
Date: Wed, 22 Jun 2016 10:49:39 +0200
Subject: [PATCH] Bump

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-24 Thread Martin Basti 



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 
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

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 
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

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-24 Thread Sumit Bose 
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 
> > > 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

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-24 Thread Martin Basti 



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 
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

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 
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

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-22 Thread Lukas Slebodnik 
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 
>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

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.
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

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

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-22 Thread Martin Basti 



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 
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
 Requires: python-sssdconfig
 Requires: certmonger >= 0.78
 Requires: nss-tools
-- 
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

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-09 Thread Martin Basti 



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

--
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

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-09 Thread Martin Basti 



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

--
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

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-09 Thread Martin Basti 



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

--
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

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-09 Thread Lukas Slebodnik 
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

-- 
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

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-09 Thread Martin Basti 



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

--
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

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-09 Thread Alexander Bokovoy 

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.

--
/ Alexander Bokovoy

--
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

Re: [Freeipa-devel] [PATCH] 0156 extdom: add certificate request

2016-06-09 Thread Jakub Hrozek 
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?)

-- 
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