Re: [Freeipa-devel] [PATCH] Workaround for trac N 5348

2015-10-08 Thread Oleg Fayans 

Hi Martin

On 10/07/2015 04:30 PM, Martin Basti wrote:



On 10/07/2015 04:13 PM, Oleg Fayans wrote:

subj




Workaround looks good, but I prefer not to push it in upstream tests,
because it is not test failure.
I agree, we should rather fix the original issue. But as a temporary 
solution, to satisfy downstream, it could do.


Why is there this sleep, this might be useful in upstream tests too, but
what is the reason to add sleep there?


Without it I kept getting this error:
E   CalledProcessError: Command '['drill', '@localhost', '-k', 
'/etc/trusted-key.key', '-S', 'example.test.', 'SOA']' returned non-zero 
exit status 29


with --pdb option, though, my attempts to re-run the command succeeded, 
so I assumed it was a timing issue, and indeed, this 1 second sleep helped.




  # verify signatures
+time.sleep(1)
  args = [




Attached is an updated version of the patch with Martin's remarks taken 
into account


--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From 18c7fe38fcc2e064a77c257837775cfb6f5efe53 Mon Sep 17 00:00:00 2001
From: Oleg Fayans 
Date: Thu, 8 Oct 2015 09:10:52 +0200
Subject: [PATCH] Fixed failure in requesting signed root zone info

After creating signed root zone, the server requires named.service restart for dig
requests to this zone to start displaying the key.

https://fedorahosted.org/freeipa/ticket/5348
---
 ipatests/test_integration/test_dnssec.py | 12 +---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/ipatests/test_integration/test_dnssec.py b/ipatests/test_integration/test_dnssec.py
index 098b227f6543fa221ed6c75d1e98e9f056761977..afcbcf130a614aa580feca4ae4a61c4d1e667243 100644
--- a/ipatests/test_integration/test_dnssec.py
+++ b/ipatests/test_integration/test_dnssec.py
@@ -280,7 +280,10 @@ class TestInstallDNSSECFirst(IntegrationTest):
 "--ns-rec=" + self.master.hostname
 ]
 self.master.run_command(args)
-
+# A workaround for https://fedorahosted.org/freeipa/ticket/5348
+time.sleep(20)
+self.master.run_command(["systemctl", "restart", "named-pkcs11.service"])
+# End of workaround
 # test master
 assert wait_until_record_is_signed(
 self.master.ip, root_zone, self.log, timeout=100
@@ -303,8 +306,10 @@ class TestInstallDNSSECFirst(IntegrationTest):
 ]
 
 self.master.run_command(args)
-
-# wait until zone is signed
+# A workaround for https://fedorahosted.org/freeipa/ticket/5348
+time.sleep(20)
+self.master.run_command(["systemctl", "restart", "named-pkcs11.service"])
+# End of workaround
 assert wait_until_record_is_signed(
 self.master.ip, example_test_zone, self.log, timeout=100
 ), "Zone %s is not signed (master)" % example_test_zone
@@ -382,6 +387,7 @@ class TestInstallDNSSECFirst(IntegrationTest):
root_keys_rrset.to_text() + '\n')
 
 # verify signatures
+time.sleep(1)
 args = [
 "drill", "@localhost", "-k",
 paths.DNSSEC_TRUSTED_KEY, "-S",
-- 
2.4.3

-- 
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] Workaround for trac N 5348

2015-10-08 Thread Martin Basti 



On 10/08/2015 09:13 AM, Oleg Fayans wrote:

Hi Martin

On 10/07/2015 04:30 PM, Martin Basti wrote:



On 10/07/2015 04:13 PM, Oleg Fayans wrote:

subj




Workaround looks good, but I prefer not to push it in upstream tests,
because it is not test failure.
I agree, we should rather fix the original issue. But as a temporary 
solution, to satisfy downstream, it could do.


Why is there this sleep, this might be useful in upstream tests too, but
what is the reason to add sleep there?


Without it I kept getting this error:
E   CalledProcessError: Command '['drill', '@localhost', '-k', 
'/etc/trusted-key.key', '-S', 'example.test.', 'SOA']' returned 
non-zero exit status 29


with --pdb option, though, my attempts to re-run the command 
succeeded, so I assumed it was a timing issue, and indeed, this 1 
second sleep helped.




  # verify signatures
+time.sleep(1)
  args = [




Attached is an updated version of the patch with Martin's remarks 
taken into account



Can you please send this as separate patch? I would like to push this one.

--
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] Workaround for trac N 5348

2015-10-08 Thread Jan Pazdziora 
On Thu, Oct 08, 2015 at 11:12:37AM +0200, Oleg Fayans wrote:
> >
> >When the ticket is addressed and these workarounds are no longer
> >needed -- what is our process for finding these workarounds and
> >reverting them, so that the tests test the real, expected behaviour?
> 
> As per discussion with Martin Basti, it was decided that this workaround
> will only be applied to the current 4-2 branch, not to the upstream. In

That sounds like a reasonable plan for this issue.

> upstream the issue itself will (supposedly) be solved

Except currently it's not, so (IIUIC) you will keep having
nondeterministic failures in master.

I was mostly interested in the general approach that we have to
workarounds -- how do we track them, how do we make sure they don't
stick in tests forever, even after the issue was already properly
addressed.

-- 
Jan Pazdziora
Senior Principal Software Engineer, Identity Management Engineering, Red Hat

-- 
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] Workaround for trac N 5348

2015-10-08 Thread Oleg Fayans 

Done
On 10/08/2015 10:37 AM, Martin Basti wrote:



On 10/08/2015 09:13 AM, Oleg Fayans wrote:

Hi Martin

On 10/07/2015 04:30 PM, Martin Basti wrote:



On 10/07/2015 04:13 PM, Oleg Fayans wrote:

subj




Workaround looks good, but I prefer not to push it in upstream tests,
because it is not test failure.

I agree, we should rather fix the original issue. But as a temporary
solution, to satisfy downstream, it could do.


Why is there this sleep, this might be useful in upstream tests too, but
what is the reason to add sleep there?


Without it I kept getting this error:
E   CalledProcessError: Command '['drill', '@localhost', '-k',
'/etc/trusted-key.key', '-S', 'example.test.', 'SOA']' returned
non-zero exit status 29

with --pdb option, though, my attempts to re-run the command
succeeded, so I assumed it was a timing issue, and indeed, this 1
second sleep helped.



  # verify signatures
+time.sleep(1)
  args = [




Attached is an updated version of the patch with Martin's remarks
taken into account


Can you please send this as separate patch? I would like to push this one.


--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.
From ad6341499d25833986f097eeac1ae89b0ea2450b Mon Sep 17 00:00:00 2001
From: Oleg Fayans 
Date: Thu, 8 Oct 2015 11:14:15 +0200
Subject: [PATCH] Fixed a timing issue with drill returning non-zero exitcode

---
 ipatests/test_integration/test_dnssec.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ipatests/test_integration/test_dnssec.py b/ipatests/test_integration/test_dnssec.py
index 098b227f6543fa221ed6c75d1e98e9f056761977..66e67a6efbe1db767f8b7102d2928be775e723af 100644
--- a/ipatests/test_integration/test_dnssec.py
+++ b/ipatests/test_integration/test_dnssec.py
@@ -382,6 +382,7 @@ class TestInstallDNSSECFirst(IntegrationTest):
root_keys_rrset.to_text() + '\n')
 
 # verify signatures
+time.sleep(1)
 args = [
 "drill", "@localhost", "-k",
 paths.DNSSEC_TRUSTED_KEY, "-S",
-- 
2.4.3

-- 
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] Workaround for trac N 5348

2015-10-08 Thread Oleg Fayans 

Hi Jan,

On 10/08/2015 10:44 AM, Jan Pazdziora wrote:

On Wed, Oct 07, 2015 at 04:13:25PM +0200, Oleg Fayans wrote:

subj

--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.



 From 7ab1afe5e9a8f6b28be2d5b92423eccec61248a0 Mon Sep 17 00:00:00 2001
From: Oleg Fayans 
Date: Wed, 7 Oct 2015 16:08:30 +0200
Subject: [PATCH] Added a workaround for ticket N 5348

After creating signed root zone, the server requires named.service restart for 
dig
requests to this zone to start displaying the key.
---
  ipatests/test_integration/test_dnssec.py | 12 +---
  1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/ipatests/test_integration/test_dnssec.py 
b/ipatests/test_integration/test_dnssec.py
index 
098b227f6543fa221ed6c75d1e98e9f056761977..b63c6ce4795c53c5c2dd604783c321835d8a689b
 100644
--- a/ipatests/test_integration/test_dnssec.py
+++ b/ipatests/test_integration/test_dnssec.py
@@ -280,7 +280,10 @@ class TestInstallDNSSECFirst(IntegrationTest):
  "--ns-rec=" + self.master.hostname
  ]
  self.master.run_command(args)
-
+# A workaround for ticket N 5348
+time.sleep(20)
+self.master.run_command(["systemctl", "restart", 
"named-pkcs11.service"])


When the ticket is addressed and these workarounds are no longer
needed -- what is our process for finding these workarounds and
reverting them, so that the tests test the real, expected behaviour?


As per discussion with Martin Basti, it was decided that this workaround 
will only be applied to the current 4-2 branch, not to the upstream. In 
upstream the issue itself will (supposedly) be solved




Also, instead of blind sleeps, wouldn't it be better to have some
polling for status of the services we are waiting for?


Since we still do not know what exactly causes the issue, it is really 
hard to figure out what is it that we should be polling. Otherwise I am 
really anti-blind-sleeps myself.






--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

--
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] [PATCHES 0069-0077] support for proper Kerberos principal canonicalization

2015-10-08 Thread David Kupka 

On 07/10/15 17:32, thierry bordaz wrote:

On 10/07/2015 05:29 PM, Simo Sorce wrote:

On 07/10/15 11:06, thierry bordaz wrote:

On 10/07/2015 03:10 PM, David Kupka wrote:

On 06/10/15 17:52, Jakub Hrozek wrote:

On Tue, Oct 06, 2015 at 08:32:29AM -0400, Simo Sorce wrote:

On 06/10/15 08:04, David Kupka wrote:

On 06/10/15 13:35, Simo Sorce wrote:

On 06/10/15 03:51, thierry bordaz wrote:

On 10/06/2015 07:19 AM, David Kupka wrote:

On 05/10/15 16:12, Simo Sorce wrote:

On 05/10/15 09:00, Martin Babinsky wrote:

These patches implement the plumbing required to properly
support
canonicalization of Kerberos principals (
https://fedorahosted.org/freeipa/ticket/3864).

Setting multiple principal aliases on hosts/services is beyond
the
scope
of this patchset and should be done after these patches are
pushed.

I will try to send some tests for the patches later this week.

Please review the hell out of them.


LGTM, I do not see any issue at quick visual inspection.
What about the performance regression with the indexes ? Is
that bug
fixed in 389ds ?

Simo.




The issue is still there. Thierry investigated this in 389 DS
and IIUC
he is not sure if it's bug or completely missing feature.
Therefore we
still don't know how much time is needed there.


Hi,
that is correct.
I can reproduce the problem. Although the matching rule (in my
test
caseIgnoreIA5Match) is found, it has no registered indexing
function, so
the setting (nsMatchingRule) is ignored.
I do not know if the indexing function is missing or there is a
bug so
that the matching rule "forget" to register it.
This feature is documented but I can not find any QA test around
it, so
I do not know yet if it is a regression or if it was not enabled
at all.

I do not expect rapid progress on it. How urgent is it ? 7.3 ?
For the moment I can think to only two workarounds:

  * use filtered matching rule (preferred)
  * change the attribute syntax/matching rule, in the schema (I
would
discourage this one because changing the schema is risky)


We can't change the syntax at this point.

Well this patchset is blocked until the 389 ds bug is fixed (the
performance regression is too big to just put it in and hope) so I
guess
we'll have to negotiate a time for the fix.

Simo.



I agree that we really shouldn't change schema.

But I don't think the patches're necessary blocked by this issue.
Canonicalization was never supported in FreeIPA and when it is not
requested the performance is not effected at all. We could merge
patches
as soon as they're carefully reviewed and tested to avoid tedious
rebasing and start using the new functionality when 389 DS gets
fixed.


The fact we didn't do canonicalization this way doesn't mean clients
aren't
asking for it.

I think Windows clients ask for canonicalization by default, and in
SSSD I
see we turn on by default krb5_canonicalize in the IPA nd LDAP case
(oddly
enough not in the AD case ?)

So SSSD's authentication requests would end up hitting this case all
the
time if I am reading the code correctly (CCed Jakub to
confirm/dispel this).


We ask for canonicalization always in IPA and LDAP, but also whenever
enterprise principals are used, which is true for AD provider.



Then SSSD will hit this every time it requests ticket on behalf of
user.
But to be sure what the impact would be I've once again set up FreeIPA
server with 10K users and run some tests.

1) 3 LDAP searches (caseIgnoreIA5Match, caseExactIA5Match, without
specifying the matching rule).
Results (http://fpaste.org/275847/44221770/raw/) shows that unindexed
search takes ~100 times longer than indexed.

2) kinit with and without requested canonicalization.

As we use kinit to get the ticket it makes sense to check what will
the performance hit be when we run kinit as a whole and not just an
isolated LDAP search.
The results (http://fpaste.org/275848/21793144/raw/) shows that with
canonicalization it takes ~2 times longer than without it.
While this is nothing to be happy about it's certainly better than I
would expect.


Clearly we need to make the search indexed.
In your deployment you defined:

dn: uid=user198,cn=users,cn=accounts,dc=example,dc=test
uid: user198
givenName: Test
sn: User198
cn: Test User198
initials: TU
homeDirectory: /home/user198
gecos: Test User198
loginShell: /bin/sh
mail: user1000...@example.test
uidNumber: 761100198
gidNumber: 761100198
displayName: Test User198
*krbPrincipalName: user1000...@example.test*
*krbCanonicalName: user1000...@example.test*
memberOf: cn=ipausers,cn=groups,cn=accounts,dc=example,dc=test
objectClass: ipaobject
objectClass: person
objectClass: top
objectClass: ipasshuser
objectClass: inetorgperson
objectClass: organizationalperson
objectClass: krbticketpolicyaux
objectClass: krbprincipalaux
objectClass: inetuser
objectClass: posixaccount
objectClass: ipaSshGroupOfPubKeys

Re: [Freeipa-devel] [PATCH] Workaround for trac N 5348

2015-10-08 Thread Jan Pazdziora 
On Wed, Oct 07, 2015 at 04:13:25PM +0200, Oleg Fayans wrote:
> subj
> 
> -- 
> Oleg Fayans
> Quality Engineer
> FreeIPA team
> RedHat.

> From 7ab1afe5e9a8f6b28be2d5b92423eccec61248a0 Mon Sep 17 00:00:00 2001
> From: Oleg Fayans 
> Date: Wed, 7 Oct 2015 16:08:30 +0200
> Subject: [PATCH] Added a workaround for ticket N 5348
> 
> After creating signed root zone, the server requires named.service restart 
> for dig
> requests to this zone to start displaying the key.
> ---
>  ipatests/test_integration/test_dnssec.py | 12 +---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/ipatests/test_integration/test_dnssec.py 
> b/ipatests/test_integration/test_dnssec.py
> index 
> 098b227f6543fa221ed6c75d1e98e9f056761977..b63c6ce4795c53c5c2dd604783c321835d8a689b
>  100644
> --- a/ipatests/test_integration/test_dnssec.py
> +++ b/ipatests/test_integration/test_dnssec.py
> @@ -280,7 +280,10 @@ class TestInstallDNSSECFirst(IntegrationTest):
>  "--ns-rec=" + self.master.hostname
>  ]
>  self.master.run_command(args)
> -
> +# A workaround for ticket N 5348
> +time.sleep(20)
> +self.master.run_command(["systemctl", "restart", 
> "named-pkcs11.service"])

When the ticket is addressed and these workarounds are no longer
needed -- what is our process for finding these workarounds and
reverting them, so that the tests test the real, expected behaviour?

Also, instead of blind sleeps, wouldn't it be better to have some
polling for status of the services we are waiting for?

-- 
Jan Pazdziora
Senior Principal Software Engineer, Identity Management Engineering, Red Hat

-- 
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] [PATCHES 0069-0077] support for proper Kerberos principal canonicalization

2015-10-08 Thread thierry bordaz 

On 10/08/2015 11:03 AM, David Kupka wrote:

On 07/10/15 17:32, thierry bordaz wrote:

On 10/07/2015 05:29 PM, Simo Sorce wrote:

On 07/10/15 11:06, thierry bordaz wrote:

On 10/07/2015 03:10 PM, David Kupka wrote:

On 06/10/15 17:52, Jakub Hrozek wrote:

On Tue, Oct 06, 2015 at 08:32:29AM -0400, Simo Sorce wrote:

On 06/10/15 08:04, David Kupka wrote:

On 06/10/15 13:35, Simo Sorce wrote:

On 06/10/15 03:51, thierry bordaz wrote:

On 10/06/2015 07:19 AM, David Kupka wrote:

On 05/10/15 16:12, Simo Sorce wrote:

On 05/10/15 09:00, Martin Babinsky wrote:

These patches implement the plumbing required to properly
support
canonicalization of Kerberos principals (
https://fedorahosted.org/freeipa/ticket/3864).

Setting multiple principal aliases on hosts/services is 
beyond

the
scope
of this patchset and should be done after these patches are
pushed.

I will try to send some tests for the patches later this 
week.


Please review the hell out of them.


LGTM, I do not see any issue at quick visual inspection.
What about the performance regression with the indexes ? Is
that bug
fixed in 389ds ?

Simo.




The issue is still there. Thierry investigated this in 389 DS
and IIUC
he is not sure if it's bug or completely missing feature.
Therefore we
still don't know how much time is needed there.


Hi,
that is correct.
I can reproduce the problem. Although the matching rule (in my
test
caseIgnoreIA5Match) is found, it has no registered indexing
function, so
the setting (nsMatchingRule) is ignored.
I do not know if the indexing function is missing or there is a
bug so
that the matching rule "forget" to register it.
This feature is documented but I can not find any QA test around
it, so
I do not know yet if it is a regression or if it was not enabled
at all.

I do not expect rapid progress on it. How urgent is it ? 7.3 ?
For the moment I can think to only two workarounds:

  * use filtered matching rule (preferred)
  * change the attribute syntax/matching rule, in the schema (I
would
discourage this one because changing the schema is risky)


We can't change the syntax at this point.

Well this patchset is blocked until the 389 ds bug is fixed (the
performance regression is too big to just put it in and hope) 
so I

guess
we'll have to negotiate a time for the fix.

Simo.



I agree that we really shouldn't change schema.

But I don't think the patches're necessary blocked by this issue.
Canonicalization was never supported in FreeIPA and when it is not
requested the performance is not effected at all. We could merge
patches
as soon as they're carefully reviewed and tested to avoid tedious
rebasing and start using the new functionality when 389 DS gets
fixed.


The fact we didn't do canonicalization this way doesn't mean 
clients

aren't
asking for it.

I think Windows clients ask for canonicalization by default, and in
SSSD I
see we turn on by default krb5_canonicalize in the IPA nd LDAP case
(oddly
enough not in the AD case ?)

So SSSD's authentication requests would end up hitting this case 
all

the
time if I am reading the code correctly (CCed Jakub to
confirm/dispel this).


We ask for canonicalization always in IPA and LDAP, but also 
whenever

enterprise principals are used, which is true for AD provider.



Then SSSD will hit this every time it requests ticket on behalf of
user.
But to be sure what the impact would be I've once again set up 
FreeIPA

server with 10K users and run some tests.

1) 3 LDAP searches (caseIgnoreIA5Match, caseExactIA5Match, without
specifying the matching rule).
Results (http://fpaste.org/275847/44221770/raw/) shows that unindexed
search takes ~100 times longer than indexed.

2) kinit with and without requested canonicalization.

As we use kinit to get the ticket it makes sense to check what will
the performance hit be when we run kinit as a whole and not just an
isolated LDAP search.
The results (http://fpaste.org/275848/21793144/raw/) shows that with
canonicalization it takes ~2 times longer than without it.
While this is nothing to be happy about it's certainly better than I
would expect.


Clearly we need to make the search indexed.
In your deployment you defined:

dn: uid=user198,cn=users,cn=accounts,dc=example,dc=test
uid: user198
givenName: Test
sn: User198
cn: Test User198
initials: TU
homeDirectory: /home/user198
gecos: Test User198
loginShell: /bin/sh
mail: user1000...@example.test
uidNumber: 761100198
gidNumber: 761100198
displayName: Test User198
*krbPrincipalName: user1000...@example.test*
*krbCanonicalName: user1000...@example.test*
memberOf: cn=ipausers,cn=groups,cn=accounts,dc=example,dc=test
objectClass: ipaobject
objectClass: person
objectClass: top
objectClass: ipasshuser
objectClass: inetorgperson
objectClass: organizationalperson
objectClass: krbticketpolicyaux
objectClass: krbprincipalaux
objectClass: inetuser

Re: [Freeipa-devel] [PATCH] Workaround for trac N 5348

2015-10-08 Thread Oleg Fayans 

Hi,

On 10/08/2015 11:18 AM, Jan Pazdziora wrote:

On Thu, Oct 08, 2015 at 11:12:37AM +0200, Oleg Fayans wrote:


When the ticket is addressed and these workarounds are no longer
needed -- what is our process for finding these workarounds and
reverting them, so that the tests test the real, expected behaviour?


As per discussion with Martin Basti, it was decided that this workaround
will only be applied to the current 4-2 branch, not to the upstream. In


That sounds like a reasonable plan for this issue.


upstream the issue itself will (supposedly) be solved


Except currently it's not, so (IIUIC) you will keep having
nondeterministic failures in master.

I was mostly interested in the general approach that we have to
workarounds -- how do we track them, how do we make sure they don't
stick in tests forever, even after the issue was already properly
addressed.

That's actually a great point. I personally would like tickets to have 
one more field: "workaround" containing the address of a workaround in 
the format "path_to_the_file:line_number" or better even - a commit id 
of this workaround, so that once the ticket is resolved, we could easily 
find what to reverse.


--
Oleg Fayans
Quality Engineer
FreeIPA team
RedHat.

--
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] Workaround for trac N 5348

2015-10-08 Thread Martin Basti 



On 10/08/2015 11:18 AM, Jan Pazdziora wrote:

On Thu, Oct 08, 2015 at 11:12:37AM +0200, Oleg Fayans wrote:

When the ticket is addressed and these workarounds are no longer
needed -- what is our process for finding these workarounds and
reverting them, so that the tests test the real, expected behaviour?

As per discussion with Martin Basti, it was decided that this workaround
will only be applied to the current 4-2 branch, not to the upstream. In

That sounds like a reasonable plan for this issue.


upstream the issue itself will (supposedly) be solved

Except currently it's not, so (IIUIC) you will keep having
nondeterministic failures in master.

I was mostly interested in the general approach that we have to
workarounds -- how do we track them, how do we make sure they don't
stick in tests forever, even after the issue was already properly
addressed.


I'm not sure if there is a formal process how to work with workarounds.

Usually, we open ticket, push workaround there, and leave ticket opened 
until the issue is fixed.
If there is no time to do proper fix and workaround works well we close 
ticket and open new ticket in further milestones.


I do not remember if we have a similar issue with test workaround in past.

Martin

--
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] Workaround for trac N 5348

2015-10-08 Thread Martin Basti 



On 10/08/2015 02:41 PM, Martin Kosek wrote:

On 10/08/2015 02:06 PM, Martin Basti wrote:


On 10/08/2015 11:18 AM, Jan Pazdziora wrote:

On Thu, Oct 08, 2015 at 11:12:37AM +0200, Oleg Fayans wrote:

When the ticket is addressed and these workarounds are no longer
needed -- what is our process for finding these workarounds and
reverting them, so that the tests test the real, expected behaviour?

As per discussion with Martin Basti, it was decided that this workaround
will only be applied to the current 4-2 branch, not to the upstream. In

That sounds like a reasonable plan for this issue.


upstream the issue itself will (supposedly) be solved

Except currently it's not, so (IIUIC) you will keep having
nondeterministic failures in master.

I was mostly interested in the general approach that we have to
workarounds -- how do we track them, how do we make sure they don't
stick in tests forever, even after the issue was already properly
addressed.


I'm not sure if there is a formal process how to work with workarounds.

Usually, we open ticket, push workaround there, and leave ticket opened until
the issue is fixed.
If there is no time to do proper fix and workaround works well we close ticket
and open new ticket in further milestones.

I do not remember if we have a similar issue with test workaround in past.

Can we anyhow utilize "wait_for_dns" knob we added for making DNS tests 
reliable?

No,

I already do that when records in CI test are created, there is polling.

The first part of oleg's workaround has nothing common with timing 
issue, only restart of named process will help


The second part, I do not know why there is 1sec delay needed, because 
presence of signed records was detected in step before, so DNSKEY record 
must be available, and probably this is caused by named internals, that 
DNSKEY record is available later than signed records of zone.
I don think so that extending testing for all types of DNSSEC records is 
worth it and 1sec is enough for bind to be ready.


Martin

--
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] Workaround for trac N 5348

2015-10-08 Thread Martin Kosek 
On 10/08/2015 02:08 PM, Oleg Fayans wrote:
> Hi,
> 
> On 10/08/2015 11:18 AM, Jan Pazdziora wrote:
>> On Thu, Oct 08, 2015 at 11:12:37AM +0200, Oleg Fayans wrote:

 When the ticket is addressed and these workarounds are no longer
 needed -- what is our process for finding these workarounds and
 reverting them, so that the tests test the real, expected behaviour?
>>>
>>> As per discussion with Martin Basti, it was decided that this workaround
>>> will only be applied to the current 4-2 branch, not to the upstream. In
>>
>> That sounds like a reasonable plan for this issue.
>>
>>> upstream the issue itself will (supposedly) be solved
>>
>> Except currently it's not, so (IIUIC) you will keep having
>> nondeterministic failures in master.
>>
>> I was mostly interested in the general approach that we have to
>> workarounds -- how do we track them, how do we make sure they don't
>> stick in tests forever, even after the issue was already properly
>> addressed.
>>
> That's actually a great point. I personally would like tickets to have one 
> more
> field: "workaround" containing the address of a workaround in the format
> "path_to_the_file:line_number" or better even - a commit id of this 
> workaround,
> so that once the ticket is resolved, we could easily find what to reverse.
> 

Please don't add any more trac fields, there is too many of them already :-)
Keyword may serve better for now...

-- 
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] Workaround for trac N 5348

2015-10-08 Thread Martin Basti 



On 10/08/2015 02:39 PM, Martin Kosek wrote:

On 10/08/2015 02:08 PM, Oleg Fayans wrote:

Hi,

On 10/08/2015 11:18 AM, Jan Pazdziora wrote:

On Thu, Oct 08, 2015 at 11:12:37AM +0200, Oleg Fayans wrote:

When the ticket is addressed and these workarounds are no longer
needed -- what is our process for finding these workarounds and
reverting them, so that the tests test the real, expected behaviour?

As per discussion with Martin Basti, it was decided that this workaround
will only be applied to the current 4-2 branch, not to the upstream. In

That sounds like a reasonable plan for this issue.


upstream the issue itself will (supposedly) be solved

Except currently it's not, so (IIUIC) you will keep having
nondeterministic failures in master.

I was mostly interested in the general approach that we have to
workarounds -- how do we track them, how do we make sure they don't
stick in tests forever, even after the issue was already properly
addressed.


That's actually a great point. I personally would like tickets to have one more
field: "workaround" containing the address of a workaround in the format
"path_to_the_file:line_number" or better even - a commit id of this workaround,
so that once the ticket is resolved, we could easily find what to reverse.


Please don't add any more trac fields, there is too many of them already :-)
Keyword may serve better for now...


+1

new trac field for a few workarounds per year is not worth it.

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

[Freeipa-devel] terminology: "main/primary/? DNS domain" for FreeIPA

2015-10-08 Thread Petr Spacek 
Hello list,

I'm in process of reviewing and fixing some of our docs and it seems that we
do not have established term for The Domain user specified during
ipa-server-install.

Term "DNS domain" is not specific enough because FreeIPA can manage multiple
DNS domains but only one of them contains SRV records. Term "IdM domain"
sounds too vague for the same reasons.

What about "primary DNS domain"? Or "primary IdM domain"?

What term is used in AD world? My google-fu is weak on this.

Ideas are more than welcome!

-- 
Petr^2 Spacek

-- 
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] Workaround for trac N 5348

2015-10-08 Thread Martin Kosek 
On 10/08/2015 02:06 PM, Martin Basti wrote:
> 
> 
> On 10/08/2015 11:18 AM, Jan Pazdziora wrote:
>> On Thu, Oct 08, 2015 at 11:12:37AM +0200, Oleg Fayans wrote:
 When the ticket is addressed and these workarounds are no longer
 needed -- what is our process for finding these workarounds and
 reverting them, so that the tests test the real, expected behaviour?
>>> As per discussion with Martin Basti, it was decided that this workaround
>>> will only be applied to the current 4-2 branch, not to the upstream. In
>> That sounds like a reasonable plan for this issue.
>>
>>> upstream the issue itself will (supposedly) be solved
>> Except currently it's not, so (IIUIC) you will keep having
>> nondeterministic failures in master.
>>
>> I was mostly interested in the general approach that we have to
>> workarounds -- how do we track them, how do we make sure they don't
>> stick in tests forever, even after the issue was already properly
>> addressed.
>>
> I'm not sure if there is a formal process how to work with workarounds.
> 
> Usually, we open ticket, push workaround there, and leave ticket opened until
> the issue is fixed.
> If there is no time to do proper fix and workaround works well we close ticket
> and open new ticket in further milestones.
> 
> I do not remember if we have a similar issue with test workaround in past.

Can we anyhow utilize "wait_for_dns" knob we added for making DNS tests 
reliable?

-- 
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] terminology: "main/primary/? DNS domain" for FreeIPA

2015-10-08 Thread Stephen Gallagher 
-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 10/08/2015 09:34 AM, Petr Spacek wrote:
> Hello list,
> 
> I'm in process of reviewing and fixing some of our docs and it
> seems that we do not have established term for The Domain user
> specified during ipa-server-install.
> 
> Term "DNS domain" is not specific enough because FreeIPA can manage
> multiple DNS domains but only one of them contains SRV records.
> Term "IdM domain" sounds too vague for the same reasons.
> 
> What about "primary DNS domain"? Or "primary IdM domain"?
> 
> What term is used in AD world? My google-fu is weak on this.
> 
> Ideas are more than welcome!
> 


In active directory, they use the term "Forest" to describe a
top-level domain. The first domain created always has the same name as
the forest (so: e.g. example.com). When adding new servers, you are
given the option of adding a new domain to an existing forest (which
then becomes subdomain.example.com) or creating a new forest. (Or
creating a replica server of an existing domain, of course).
-BEGIN PGP SIGNATURE-
Version: GnuPG v2

iEYEARECAAYFAlYWhYgACgkQeiVVYja6o6PVsACfV+AUb7TiBFiEaQBI4M0pS8cD
4KYAn2eajHx9K8JsMDc8R4Yjv8s4jHrp
=Pil0
-END PGP SIGNATURE-

-- 
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] [PATCHES] More Python 3 porting

2015-10-08 Thread Petr Viktorin 
On 10/07/2015 10:27 AM, Jan Cholasta wrote:
> On 6.10.2015 12:04, Petr Viktorin wrote:
>> On 10/05/2015 07:56 AM, Jan Cholasta wrote:
>>> On 2.10.2015 13:09, Petr Viktorin wrote:
 On 10/01/2015 03:15 PM, Jan Cholasta wrote:
> Hi,
>
> On 1.10.2015 13:01, Martin Basti wrote:
>>
>>
>> On 09/30/2015 10:25 AM, Petr Viktorin wrote:
>>> On 09/23/2015 04:46 PM, Petr Viktorin wrote:
 On 09/22/2015 02:59 PM, David Kupka wrote:
> On 18/09/15 17:00, Petr Viktorin wrote:
>> Hello,
>> Here are more patches that bring IPA closer to Python 3
>> compatibility.
 [...]
>>>
>> LGTM
>>
>> I ran xmlrpc tests, DNSSEC ci tests, backup and restore CI test and
>> everything works
>
> Patches 713-719: ACK
>
>
> Patch 720:
>
> You missed:
>
> ipa-client/ipa-install/ipa-client-install:32:from ConfigParser
> import RawConfigParser


 Thanks, fixed.

> Patches 721-722: ACK
>
>
> Patch 723:
>
> Why the "NoneType = type(None)" in parameters.py? It is used only at:
>
> ipalib/parameters.py:388:type = NoneType  # Ouch, this wont be
> very
> useful in the real world!

 I believe this is less confusing than `type = type(None)`, but I can
 change that if needed.
>>>
>>> I don't care which one is used TBH, just that it is done consistently
>>> accross the whole patch, and this seemed like the simpler thing to do.
>>
>> OK, changed.
>>
> Patch 724:
>
> The SSHPublicKey class was written with the assumption that "str"
> means
> binary data, so unless I'm missing something, you only need to replace
> "str" with "bytes".

 It specifically did take non-binary data as str:

 -if isinstance(key, str) and key[:3] != '\0\0\0':
 -key = key.decode(encoding)
>>>
>>> I don't follow, this is quite obviously binary data. It reads: "If key
>>> is binary and does not start with 3 null bytes, decode it to text using
>>> the specified encoding."
>>
>> Right, it's text (non-binary) data encoded in str (bytes), so it needs
>> to be encoded.
>>
 I've removed this for Python 3, where text data shouldn't be in bytes.

 Since this means the '\0\0\0' check is skipped in __init__ under Python
 3, I've added it also to _parse_raw.
>>>
>>> When the SSH integration feature was first introduced, SSH public keys
>>> were stored in the raw binary form in LDAP, i.e. not text data. We still
>>> need to support that, so support for binary data and the 3 null check
>>> must remain in SSHPublicKey.
>>
>> Changed, updated patches attached.
> 
> Thanks, ACK.
> 
> I took the liberty of amending patch 718 to silence this pylint false
> positive I was getting on F22:
> 
> ipalib/plugins/otptoken.py:496: [E1101(no-member),
> HTTPSHandler.https_open] Instance of 'HTTPSHandler' has no 'do_open'
> member)

Thanks!

> Pushed to master: f82d3da1e8e5dc1d0716201af5abb724a8e78fde
> 
> BTW, in patch 724, binascii.Error is handled in addition to TypeError
> with base64.b64decode(). There are multiple places where
> base64.b64decode() is used in IPA where only TypeError is handled. Are
> you planning on fixing this as well?

I'll go through them to make sure we're handling them correctly.


-- 
Petr Viktorin

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

[Freeipa-devel] Announcing FreeIPA 4.2.2

2015-10-08 Thread Petr Vobornik 

The FreeIPA team would like to announce FreeIPA v4.2.2 security release!

It can be downloaded from http://www.freeipa.org/page/Downloads. The 
builds are available for Fedora 23 
 and 
rawhide. Builds for Fedora 22 are available in the official COPR 
repository .


This announcement is also available at 
.


== Highlights in 4.2.2 ==
FreeIPA 4.2.0 introduced Key Archival Agent (KRA) support. This release 
fixes CVE-2015-5284. The CVE affects IPA servers where ipa-kra-install 
was run. Read manual instructions if your system was affected 
.


=== Bug fixes ===
* CVE-2015-5284: ipa-kra-install included certificate and private key in 
world readable file.

* Firefox configuration steps were adjusted to new extension signing policy.
* ipa-restore does not overwrite files with local users/groups
* ipa-restore now works with KRA installed
* Fixed an integer underflow bug in libotp which prevented from syncing 
TOTP tokens under certain circumstances.
* winsync-migrate properly handles collisions in the names of external 
group

* Fixed regression where installation of CA failed on CA-less server #5288.
* Vault operations now works on a replica without KRA installed 
(assuming that at least one replica has KRA installed). #5302


=== Enhancements ===
* Improved performance of search in Web UI's dialog for adding e.g. 
users to e.g. sudo rules.
* Modified vault access control and  added commands for managing vault 
containers. #5250
* Added support for generating client referrals for trusted domain 
principals. Note that bug 
https://bugzilla.redhat.com/show_bug.cgi?id=1259844 has to be fixed in 
MIT Kerberos packages to allow client referrals from FreeIPA KDC to be 
processed.


== Upgrading ==
Upgrade instructions are available on upgrade page 
.


== Feedback ==
Please provide comments, bugs and other feedback via the freeipa-users 
mailing list (http://www.redhat.com/mailman/listinfo/freeipa-users) or 
#freeipa channel on Freenode.


== Detailed Changelog since 4.2.1 ==

=== Abhijeet Kasurde (1) ===
* Updated number of legacy permission in ipatests

=== Alexander Bokovoy (1) ===
* client referral support for trusted domain principals

=== Christian Heimes (1) ===
* Handle timeout error in ipa-httpd-kdcproxy

=== Gabe Alford (4) ===
* Add Chromium configuration note to ssbrowser
* Standardize minvalue for ipasearchrecordlimit and ipasesarchsizelimit 
for unlimited minvalue

* dnssec option missing in ipa-dns-install man page
* Update FreeIPA package description

=== Jan Cholasta (16) ===
* config: allow user/host attributes with tagging options
* baseldap: make subtree deletion optional in LDAPDelete
* vault: set owner to current user on container creation
* vault: update access control
* vault: add permissions and administrator privilege
* install: support KRA update
* install: Support overriding knobs in subclasses
* install: Add common base class for server and replica install
* install: Move unattended option to the general help section
* install: create kdcproxy user during server install
* platform: add option to create home directory when adding user
* install: fix kdcproxy user home directory
* install: fix ipa-server-install fail on missing --forwarder
* install: fix KRA agent PEM file permissions
* install: always export KRA agent PEM file
* vault: select a server with KRA for vault operations

=== Martin Babinsky (5) ===
* load RA backend plugins during standalone CA install on CA-less IPA master
* destroy httpd ccache after stopping the service
* ipa-server-install: mark master_password Knob as deprecated
* re-kinit after ipa-restore in backup/restore CI tests
* do not overwrite files with local users/groups when restoring authconfig

=== Martin Bašti (11) ===
* FIX vault tests
* Server Upgrade: backup CS.cfg when dogtag is turned off
* IPA Restore: allows to specify files that should be removed
* DNSSEC: improve CI test
* DNSSEC CI: test master migration
* backup CI: test DNS/DNSSEC after backup and restore
* Limit max age of replication changelog
* Server Upgrade: addifnew should not create entry
* CI: backup and restore with KRA
* Replica inst. fix: do not require -r, -a, -p options in unattended mode
* Fix import get_reverse_zone_default in tasks

=== Milan Kubík (4) ===
* ipatests: Add Certprofile tracker class implementation
* ipatests: Add basic tests for certificate profile plugin
* ipatests: configure Network Manager not to manage resolv.conf
* Include ipatests/test_xmlrpc/data directory into distribution.

=== Nathaniel McCallum (1) ===
* Fix an integer underflow bug in libotp

=== Oleg Fayans (1) ===
* Added a proper workaround for dnssec test failures in Beaker environment

=== Petr Voborník (4) ===
* vault: add vault

[Freeipa-devel] [PATCHES 0318 - 0320] installer: allow to modify dse.ldif during installation

2015-10-08 Thread Martin Basti 

The attached patches fix following tickets:
https://fedorahosted.org/freeipa/ticket/4949
https://fedorahosted.org/freeipa/ticket/4048
https://fedorahosted.org/freeipa/ticket/1930

With these patches, an administrator can specify LDIF file that contains 
modifications to be applied to dse.ldif right after creation of DS instance.
From 3d0b62913a2611004c52804ae9cf34d7f4b4b55a Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Mon, 5 Oct 2015 14:37:05 +0200
Subject: [PATCH 1/3] Make offline LDIF modify more robust

* move code to installutils
* add replace_value method
* use lists instead of single values for add_value, remove_value methods

https://fedorahosted.org/freeipa/ticket/4949

Also fixes:
https://fedorahosted.org/freeipa/ticket/4048
https://fedorahosted.org/freeipa/ticket/1930
---
 ipaserver/install/installutils.py|  86 +++
 ipaserver/install/upgradeinstance.py | 112 ---
 2 files changed, 97 insertions(+), 101 deletions(-)

diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 58be9f23384f0c1734d1ba7a14182f60817a32a8..325856e29166fe25df5405cb3f010c3a4f2a0cc8 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -22,6 +22,7 @@ from __future__ import print_function
 
 import socket
 import getpass
+import ldif
 import os
 import re
 import fileinput
@@ -1107,3 +1108,88 @@ def enable_and_start_oddjobd(sstore):
 oddjobd.start()
 except Exception as e:
 root_logger.critical("Unable to start oddjobd: {0}".format(str(e)))
+
+
+class ModifyLDIF(ldif.LDIFParser):
+"""
+Allows to modify LDIF file.
+
+Operations keep the order in whihc were specified per DN.
+Warning: only modifications of existing DNs are supported
+"""
+def __init__(self, input_file, output_file):
+"""
+:param input_file: an LDIF
+:param output_file: an LDIF file
+"""
+ldif.LDIFParser.__init__(self, input_file)
+self.writer = ldif.LDIFWriter(output_file)
+
+self.modifications = {}  # keep modify operations in original order
+
+def add_value(self, dn, attr, values):
+"""
+Add value to LDIF.
+:param dn: DN of entry (must exists)
+:param attr: attribute name
+:param value: value to be added
+"""
+assert isinstance(values, list)
+self.modifications.setdefault(dn, []).append(
+dict(
+op="add",
+attr=attr,
+values=values,
+)
+)
+
+def remove_value(self, dn, attr, values=None):
+"""
+Remove value from LDIF.
+:param dn: DN of entry
+:param attr: attribute name
+:param value: value to be removed, if value is None, attribute will
+be removed
+"""
+assert values is None or isinstance(values, list)
+self.modifications.setdefault(dn, []).append(
+dict(
+op="del",
+attr=attr,
+values=values,
+)
+)
+
+def replace_value(self, dn, attr, values):
+"""
+Replace values in LDIF with new value.
+:param dn: DN of entry
+:param attr: attribute name
+:param value: new value for atribute
+"""
+assert isinstance(values, list)
+self.remove_value(dn, attr)
+self.add_value(dn, attr, values)
+
+def handle(self, dn, entry):
+for mod in self.modifications.get(dn, []):
+attr_name = mod["attr"]
+values = mod["values"]
+
+if mod["op"] == "del":
+# delete
+attribute = entry.setdefault(attr_name, [])
+if values is None:
+attribute = []
+else:
+attribute = [v for v in attribute if v not in values]
+if not attribute:  # empty
+del entry[attr_name]
+elif mod["op"] == "add":
+# add
+attribute = entry.setdefault(attr_name, [])
+attribute.extend([v for v in values if v not in attribute])
+else:
+assert False, "Unknown operation: %r" % mod["op"]
+
+self.writer.unparse(dn, entry)
diff --git a/ipaserver/install/upgradeinstance.py b/ipaserver/install/upgradeinstance.py
index 684a3dd99e2215c86b92dcb7ba9d00ee9e17b8fb..602e6ec4930cd9d2b9e686a5ec2ed3de10cb082f 100644
--- a/ipaserver/install/upgradeinstance.py
+++ b/ipaserver/install/upgradeinstance.py
@@ -66,85 +66,6 @@ class GetEntryFromLDIF(ldif.LDIFParser):
 self.results[dn] = entry
 
 
-class ModifyLDIF(ldif.LDIFParser):
-"""
-Allows to modify LDIF file.
-
-Remove operations are executed before add operations
-"""
-def __init__(self, input_file, writer):
-"""
-:param input_file: an

[Freeipa-devel] [PATCHES] More Python3 porting

2015-10-08 Thread Petr Viktorin 
Hello,
Here is another batch of Python 3 porting patches.


-- 
Petr Viktorin
From 53770dbbee76728517d7ae0b5ebce3446bb6692b Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Fri, 18 Sep 2015 11:30:15 +0200
Subject: [PATCH] Do not compare types that are not comparable in Python 3

In Python 3, different types are generally not comparable (except for equality),
and None can't be compared to None.
Fix cases of these comparisons.

In ipatest.util, give up on sorting lists if the sorting raises a TypeError.
---
 ipalib/parameters.py|  6 +++---
 ipatests/test_ipapython/test_ipautil.py |  2 +-
 ipatests/util.py| 10 --
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/ipalib/parameters.py b/ipalib/parameters.py
index 34cd65d2980514729d58427443ee1b2296a37cb7..c7468627133aa862988634bb677eb24ccf5dc261 100644
--- a/ipalib/parameters.py
+++ b/ipalib/parameters.py
@@ -1157,9 +1157,9 @@ def __init__(self, name, *rules, **kw):
 
 super(Decimal, self).__init__(name, *rules, **kw)
 
-if (self.minvalue > self.maxvalue) \
-and (self.minvalue is not None and \
- self.maxvalue is not None):
+if (self.minvalue is not None and
+self.maxvalue is not None and
+self.minvalue > self.maxvalue):
 raise ValueError(
 '%s: minvalue > maxvalue (minvalue=%s, maxvalue=%s)' % (
 self.nice, self.minvalue, self.maxvalue)
diff --git a/ipatests/test_ipapython/test_ipautil.py b/ipatests/test_ipapython/test_ipautil.py
index 609e1f0e5be29278391551d2dafccbdbe36e36a5..e19cd2cb677d0508685ccb8f82c2656e078e2069 100644
--- a/ipatests/test_ipapython/test_ipautil.py
+++ b/ipatests/test_ipapython/test_ipautil.py
@@ -321,7 +321,7 @@ def test_popitem(self):
 def test_fromkeys(self):
 dct = ipautil.CIDict.fromkeys(('A', 'b', 'C'))
 assert sorted(dct.keys()) == sorted(['A', 'b', 'C'])
-assert sorted(dct.values()) == [None] * 3
+assert list(dct.values()) == [None] * 3
 
 
 class TestTimeParser(object):
diff --git a/ipatests/util.py b/ipatests/util.py
index d180c91b77b0bafd6bff2f01b9dfd7740519c1bd..85b5dbc5e6f8d4a7d484fe1a05a3e9be8a09335a 100644
--- a/ipatests/util.py
+++ b/ipatests/util.py
@@ -331,8 +331,14 @@ def assert_deepequal(expected, got, doc='', stack=tuple()):
 s_got = got
 s_expected = expected
 else:
-s_got = sorted(got)
-s_expected = sorted(expected)
+try:
+s_got = sorted(got)
+except TypeError:
+s_got = got
+try:
+s_expected = sorted(expected)
+except TypeError:
+s_expected = expected
 for (i, e_sub) in enumerate(s_expected):
 g_sub = s_got[i]
 assert_deepequal(e_sub, g_sub, doc, stack + (i,))
-- 
2.1.0

From d0cf54888a1249cb9a40f5e1e8044ba9dcf611c8 Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Fri, 18 Sep 2015 17:20:08 +0200
Subject: [PATCH] x509: Port to Python 3

In python 3 , `bytes` has the buffer interface, and `buffer` was removed.

Also, invalid padding in base64-encoded data raises a ValueError rather
than TypeError.

In tests, use pytest.assert_raises for more correct exception assertions.
Also, get rid of unused imports in the tests
---
 ipalib/x509.py|  9 -
 ipatests/test_ipalib/test_x509.py | 21 ++---
 2 files changed, 14 insertions(+), 16 deletions(-)

diff --git a/ipalib/x509.py b/ipalib/x509.py
index e48d3edf78ed701482cdb3b1998a8f3afe708b5c..037d6785c85552a7335d848f7b21fb5eba26ceda 100644
--- a/ipalib/x509.py
+++ b/ipalib/x509.py
@@ -37,10 +37,13 @@
 import sys
 import base64
 import re
+
 import nss.nss as nss
 from nss.error import NSPRError
 from pyasn1.type import univ, namedtype, tag
 from pyasn1.codec.der import decoder, encoder
+import six
+
 from ipapython import ipautil
 from ipalib import api
 from ipalib import _
@@ -127,7 +130,11 @@ def load_certificate(data, datatype=PEM, dbdir=None):
 
 initialize_nss_database(dbdir=dbdir)
 
-return nss.Certificate(buffer(data))
+if six.PY2:
+return nss.Certificate(buffer(data))
+else:
+# In python 3 , `bytes` has the buffer interface
+return nss.Certificate(data)
 
 def load_certificate_from_file(filename, dbdir=None):
 """
diff --git a/ipatests/test_ipalib/test_x509.py b/ipatests/test_ipalib/test_x509.py
index c7fafbbd95f38e28dfa57b6080c8a9c511921cb9..d8004c4a0b4d37130b71b2026956d04c44aa6db3 100644
--- a/ipatests/test_ipalib/test_x509.py
+++ b/ipatests/test_ipalib/test_x509.py
@@ -21,17 +21,12 @@
 Test the `ipalib.x509` module.
 """
 
-import os
-from os import path
-import sys
-from ipatests.util import raises, setitem, delitem, ClassChecker
-from ipatests.util import getitem, setitem, delitem
-from ipatests.util import

[Freeipa-devel] [PATCHES 0321 - 0322] CI: vault CI test

2015-10-08 Thread Martin Basti 

Patches attached.

Tests for https://fedorahosted.org/freeipa/ticket/5302
From 9c87a6c66a72fc5f1a35c39529c0e518b4736a18 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Tue, 6 Oct 2015 20:28:18 +0200
Subject: [PATCH 1/2] CI Test: add setup_kra options into install scripts

https://fedorahosted.org/freeipa/ticket/5302
---
 ipatests/test_integration/tasks.py | 30 +++---
 .../test_integration/test_backup_and_restore.py|  8 +-
 2 files changed, 27 insertions(+), 11 deletions(-)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 7bfd12dce4ce0330ad18180948efec41f8f82fe2..918f78cde23d65a20fdab1a9484ea29ecceb4d10 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -246,7 +246,7 @@ def enable_replication_debugging(host):
  stdin_text=logging_ldif)
 
 
-def install_master(host, setup_dns=True):
+def install_master(host, setup_dns=True, setup_kra=False):
 host.collect_log(paths.IPASERVER_INSTALL_LOG)
 host.collect_log(paths.IPACLIENT_INSTALL_LOG)
 inst = host.domain.realm.replace('.', '-')
@@ -273,10 +273,23 @@ def install_master(host, setup_dns=True):
 enable_replication_debugging(host)
 setup_sssd_debugging(host)
 
+if setup_kra:
+args = [
+"ipa-kra-install",
+"-p", host.config.dirman_password,
+"-U",
+]
+host.run_command(args)
+
 kinit_admin(host)
 
 
-def install_replica(master, replica, setup_ca=True, setup_dns=False):
+def get_replica_filename(replica):
+return os.path.join(replica.config.test_dir, 'replica-info.gpg')
+
+
+def install_replica(master, replica, setup_ca=True, setup_dns=False,
+setup_kra=False):
 replica.collect_log(paths.IPAREPLICA_INSTALL_LOG)
 replica.collect_log(paths.IPAREPLICA_CONNCHECK_LOG)
 
@@ -289,8 +302,7 @@ def install_replica(master, replica, setup_ca=True, setup_dns=False):
 replica.hostname])
 replica_bundle = master.get_file_contents(
 paths.REPLICA_INFO_GPG_TEMPLATE % replica.hostname)
-replica_filename = os.path.join(replica.config.test_dir,
-'replica-info.gpg')
+replica_filename = get_replica_filename(replica)
 replica.put_file_contents(replica_filename, replica_bundle)
 args = ['ipa-replica-install', '-U',
 '-p', replica.config.dirman_password,
@@ -309,6 +321,16 @@ def install_replica(master, replica, setup_ca=True, setup_dns=False):
 enable_replication_debugging(replica)
 setup_sssd_debugging(replica)
 
+if setup_kra:
+assert setup_ca, "CA must be installed on replica with KRA"
+args = [
+"ipa-kra-install",
+replica_filename,
+"-p", replica.config.dirman_password,
+"-U",
+]
+replica.run_command(args)
+
 kinit_admin(replica)
 
 
diff --git a/ipatests/test_integration/test_backup_and_restore.py b/ipatests/test_integration/test_backup_and_restore.py
index c0d30fc8188384c0886f65d68e73e07436bcc897..8b9cd2dc4af146b2c00e6d2224625c4288854be8 100644
--- a/ipatests/test_integration/test_backup_and_restore.py
+++ b/ipatests/test_integration/test_backup_and_restore.py
@@ -354,13 +354,7 @@ class BaseBackupAndRestoreWithKRA(IntegrationTest):
 
 @classmethod
 def install(cls, mh):
-tasks.install_master(cls.master, setup_dns=True)
-args = [
-"ipa-kra-install",
-"-p", cls.master.config.dirman_password,
-"-U",
-]
-cls.master.run_command(args)
+tasks.install_master(cls.master, setup_dns=True, setup_kra=True)
 
 def _full_backup_restore_with_vault(self, reinstall=False):
 with restore_checker(self.master):
-- 
2.4.3

From 0a8614835e12b960a30fb9f52380a9de5ebe3d68 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Tue, 6 Oct 2015 20:26:01 +0200
Subject: [PATCH 2/2] CI TEST: Vault

Simple CI test for vault feature, including testing with replica

Covers https://fedorahosted.org/freeipa/ticket/5302
---
 ipatests/test_integration/test_vault.py | 205 
 1 file changed, 205 insertions(+)
 create mode 100644 ipatests/test_integration/test_vault.py

diff --git a/ipatests/test_integration/test_vault.py b/ipatests/test_integration/test_vault.py
new file mode 100644
index ..ba472af7152b508fb8a6fd92ebf18879518d2246
--- /dev/null
+++ b/ipatests/test_integration/test_vault.py
@@ -0,0 +1,205 @@
+#
+# Copyright (C) 2015  FreeIPA Contributors see COPYING for license
+#
+
+import time
+
+from ipatests.test_integration.base import IntegrationTest
+from ipatests.test_integration import tasks
+
+WAIT_AFTER_ARCHIVE = 30  # give some time to replication
+
+
+class TestInstallKRA(IntegrationTest):
+"""
+Test if vault feature behaves as expected, when KRA is

[Freeipa-devel] [PATCH 0078-0081] ipa-client-install autodiscovery code improvements

2015-10-08 Thread Martin Babinsky 

These patches fix https://fedorahosted.org/freeipa/ticket/4305

Actually only the last patch does the work itself (suppress 
autodiscovery when installing client on master), but when I saw the 
state of autodiscovery code I have taken the liberty to clean it up a bit.


Patch #78 has separate versions for master and 4-2 branch, other patches 
should apply on top of it in both branches.


--
Martin^3 Babinsky
From 86918274dc583278b331783e51d9713ef170f8e6 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Wed, 7 Oct 2015 15:21:12 +0200
Subject: [PATCH 4/4] do not perform autodiscovery when installing client-side
 components on master

the IPA master FQDN, realm, and domain name will be taken directly from CLI
options and no DNS discovery (apart of fetching LDAP suffix) will be performed
when ipa-client-install is run with '-on-master' option.

https://fedorahosted.org/freeipa/ticket/4305
---
 ipa-client/ipa-install/ipa-client-install | 31 ++-
 1 file changed, 30 insertions(+), 1 deletion(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index a1b1783f3ffda1e7625f872499d45eb7761207af..44c9e58887074833f877c82869dce1c0796753ff 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -2394,6 +2394,29 @@ def perform_autodiscovery(ds, hostname, options):
 return dnsok
 
 
+def set_ipa_domain_params(ds, servers, domain_name, realm_name,
+  ca_cert_path=None):
+"""
+set IPA server, domain, realm name and other parameters directly without
+DNS discovery
+:param ds: IPADiscovery instance
+:param servers: IPA server FQDN
+:param domain_name: name of IPA domain
+:param realm_name: IPA realm name
+"""
+ds.servers = servers
+ds.server = servers[0]
+
+ds.domain = domain_name
+ds.realm = realm_name
+ds.kdc = servers[0]
+
+# use ipacheckldap to get basedn from IPA master
+if ds.ipacheckldap(ds.server, ds.realm, ca_cert_path=ca_cert_path)[0]:
+raise RuntimeError("Failed to get basedn from IPA master %s"
+   % ds.server)
+
+
 def install(options, env, fstore, statestore):
 dnsok=False
 
@@ -2463,7 +2486,13 @@ def install(options, env, fstore, statestore):
 ds = ipadiscovery.IPADiscovery()
 
 try:
-dnsok = perform_autodiscovery(ds, hostname, options)
+if options.on_master:
+set_ipa_domain_params(ds, options.server, options.domain,
+  options.realm_name, CACERT)
+ds.server_source = ds.domain_source = ds.realm_source = (
+"set by IPA master")
+else:
+dnsok = perform_autodiscovery(ds, hostname, options)
 except RuntimeError as e:
 root_logger.error("Error running IPA discovery: %s", e)
 return CLIENT_INSTALL_ERROR
-- 
2.4.3

From fed560b03169d73376deb590777f618b82c6f5a0 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Wed, 7 Oct 2015 15:16:25 +0200
Subject: [PATCH 3/4] ipa-client-install: store server/domain/realm info in
 IPADiscovery object

https://fedorahosted.org/freeipa/ticket/4305
---
 ipa-client/ipa-install/ipa-client-install | 107 +++---
 1 file changed, 52 insertions(+), 55 deletions(-)

diff --git a/ipa-client/ipa-install/ipa-client-install b/ipa-client/ipa-install/ipa-client-install
index 0c62875553d2c6577e3b71aaa439f52096161475..a1b1783f3ffda1e7625f872499d45eb7761207af 100755
--- a/ipa-client/ipa-install/ipa-client-install
+++ b/ipa-client/ipa-install/ipa-client-install
@@ -2215,14 +2215,16 @@ def configure_firefox(options, statestore, domain):
 
 
 def perform_autodiscovery(ds, hostname, options):
+"""
+Perform automated DNS discovery of domain, realm, IPA servers and KDCs
+
+:param ds: IPADiscovery instance
+:param hostname: machine FQDN
+:param options: options passed to installer
+:return: True if DNS discovery works in IPA domain, False otherwise
+"""
 dnsok = False
 
-cli_domain = None
-cli_server = None
-
-cli_domain_source = 'Unknown source'
-cli_server_source = 'Unknown source'
-
 ret = ds.search(domain=options.domain, servers=options.server,
 realm=options.realm_name, hostname=hostname,
 ca_cert_path=get_cert_path(options.ca_cert_file))
@@ -2259,47 +2261,44 @@ def perform_autodiscovery(ds, hostname, options):
 else:
 root_logger.debug("Domain not found")
 if options.domain:
-cli_domain = options.domain
-cli_domain_source = 'Provided as option'
+ds.domain = options.domain
+ds.domain_source = 'Provided as option'
 elif options.unattended:
 raise RuntimeError("Unable to discover domain, not provided on "
"command line")
 else:

Re: [Freeipa-devel] terminology: "main/primary/? DNS domain" for FreeIPA

2015-10-08 Thread Simo Sorce 

On 08/10/15 09:34, Petr Spacek wrote:

Hello list,

I'm in process of reviewing and fixing some of our docs and it seems that we
do not have established term for The Domain user specified during
ipa-server-install.

Term "DNS domain" is not specific enough because FreeIPA can manage multiple
DNS domains but only one of them contains SRV records. Term "IdM domain"
sounds too vague for the same reasons.


We should make it easy to publish SRV records on any domain, if a client 
is in a different DNS domain it should still be able to discover the IPA 
servers.



What about "primary DNS domain"? Or "primary IdM domain"?


What's "primary" about it ? The fact that the Realm is named after it ?
I guess that could work, but it wouldn't necessarily be accurate if you 
decide to move tto use poredominantly another different DNS domain 
during the lifetime of an install.



What term is used in AD world? My google-fu is weak on this.


There is no such term because, mostly, in the AD world there is only one 
DNS domain that matters, the one that corresponds 1-1 to the Realm name. 
So there is only The AD Domain Name.



Ideas are more than welcome!


I guess primary is ok, I draw a blank on what else could be used right now.

Simo.


--
Simo Sorce * Red Hat, Inc * New York

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