Re: [Freeipa-devel] [Test][Patch-0047] Added a test for Ticket N 5964

[Milan Kubík]

On 11/09/2016 04:37 PM, Milan Kubík wrote:

On 11/09/2016 04:34 PM, Milan Kubík wrote:

On 11/03/2016 04:56 PM, Oleg Fayans wrote:

Hi Martin,

The commit message was updated with the correct ticket link
Thanks for review!

On 11/03/2016 04:22 PM, Martin Basti wrote:

almost ACK, but the ticket in commit message is closed as invalid. So
I'm quite puzzled now what to do.


On 03.11.2016 13:28, Oleg Fayans wrote:

ping for review

On 10/19/2016 04:54 PM, Oleg Fayans wrote:

Hi Martin,

Thanks for the review. Fixed both issues.

$ ipa-run-tests test_integration/test_topology.py -k 
TestCASpecificRUVs

WARNING: Couldn't write lextab module 'pycparser.lextab'. [Errno 13]
Permission denied: 'lextab.py'
WARNING: yacc table file version is out of date
WARNING: Couldn't create 'pycparser.yacctab'. [Errno 13] Permission
denied: 'yacctab.py'
 



test session starts
= 




platform linux2 -- Python 2.7.11, pytest-2.9.2, py-1.4.31, 
pluggy-0.3.1
rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: 
pytest.ini

plugins: sourceorder-0.5, multihost-1.0
collected 5 items

test_integration/test_topology.py ..

 



2 passed in 2444.84 seconds
= 






On 10/17/2016 07:05 PM, Martin Basti wrote:

1)

you don't need to disable/enable dirsrv, just stop/start. Please 
remove

disable/enable parts


2)




traceback





self = 

def test_delete_ruvs(self):
"""
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/
Test_Plan#Test_case:_clean-ruv_subcommand
"""
replica = self.replicas[0]
master = self.master
res1 = master.run_command(['ipa-replica-manage', 
'list-ruv',

'-p',
master.config.dirman_password])

assert(res1.stdout_text.count(replica.hostname) == 2 and
   "Certificate Server Replica Update Vectors" in 
res1), (

"CA-specific RUVs are not displayed")
E   TypeError: argument of type 'SSHCommand' is not iterable

test_integration/test_topology.py:215: TypeError



entering PDB






/usr/lib/python2.7/site-packages/ipatests/test_integration/test_topology.py(215)test_delete_ruvs() 






-> assert(res1.stdout_text.count(replica.hostname) == 2 and



On 14.10.2016 11:36, Oleg Fayans wrote:

Right you are! I am sorry.

On 10/13/2016 06:10 PM, Martin Basti wrote:

I think that you forgot to squash commits. Patch 47 doesn't apply


On 13.10.2016 14:01, Oleg Fayans wrote:

Hi Martin,

Thanks for the review.
With disabling directory server it works as well, thanks for the
hint.
Also I moved the cleanup logic to the test itself for the 
sake of

simplicity. Patch-0048 was not changed

On 10/12/2016 02:35 PM, Martin Basti wrote:

1)

Can you just turn off dirsrv on replica instead of doing 
iptables

magic?


2) NACK

No more eval() ever in code, use 'getattr', 'get' or 
whatever in

the
object that can be used.

+evalhost = eval("args[0].%s" % host)

Martin^2

On 12.10.2016 14:03, Oleg Fayans wrote:

Hi Martin,

After extensive discussion with Ludwig, I finally got the 
clue on

how
does this feature work. When we uninstall the replica, the 
master

cleans the replication agreements with this replica and
automatically
cleans all replica's RUVs.
If we clean replica's RUVs on master without uninstalling the
replica,
the replica's RUVs get recreated on master (replication
works!). So,
the only way to test the clean-ruv subcommand is to turn 
off the

replica, or block the traffic on it so it gets inaccessible to
updates
from master.
The testcases were updated, see [1] and [2]

The updated versions of the patches are attached

[1]
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan#Test_case:_.2A-ruv_subcommands_of_ipa-replica-manage_are_extended_to_handle_CA-specific_RUVs 








[2]
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan#Test_case:_clean-ruv_subcommand 








On 08/05/2016 06:36 PM, Martin Basti wrote:



On 03.08.2016 14:45, Oleg Fayans wrote:

Hi Martin,

Thanks for the review! Both patches were updated.

On 07/28/2016 04:11 PM, Martin Basti wrote:



On 08.07.2016 15:41, Oleg Fayans wrote:

Hi Martin,

Thanks for the review!

On 07/08/2016 02:18 PM, Martin Basti wrote:



On 27.06.2016 13:53, Oleg Fayans wrote:

Hi guys,

Is there a chance the patches NN 0047.1 and 0048.1 get
reviewed
before
4.4 release? They cover a good part of the Managed 
Topology

4.4
feature.

On 06/17/2016 11:18 AM, Oleg Fayans wrote:

One more test was added to the patch-0048

On 06/17/2016 09:43 AM, Oleg Fayans wrote:

Fixed a bug in the previous patch, automated 2 more
testcases
from

Re: [Freeipa-devel] [Test][Patch-0049, 0050] Certs in ID overrides test

[Milan Kubík]

On 10/25/2016 10:24 AM, Oleg Fayans wrote:

Integration part of the tests is ready. 2 tests:

1. Adds a cert to idoverride of a windows user
2. sssd part - looks up user by his certificate using dbus-sssd

Second and third dbus call are executed as a string insted of as array 
of strings because it just does not work otherwise. Some quote 
escaping gets screwed probably, but the system returns "Error 
org.freedesktop.DBus.Error.UnknownInterface: Unknown interface" if the 
command is executed using the standard array-based approach


The run looks like this:

bash-4.3$ ipa-run-tests test_integration/test_idviews.py --pdb
WARNING: Couldn't write lextab module 'pycparser.lextab'. [Errno 13] 
Permission denied: 'lextab.py'

WARNING: yacc table file version is out of date
WARNING: Couldn't create 'pycparser.yacctab'. [Errno 13] Permission 
denied: 'yacctab.py'
 test session starts 


platform linux2 -- Python 2.7.11, pytest-2.9.2, py-1.4.31, pluggy-0.3.1
rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: pytest.ini
plugins: sourceorder-0.5, multihost-1.0
collected 2 items

test_integration/test_idviews.py ..

 2 passed in 948.44 seconds 
=



On 10/21/2016 10:54 AM, Oleg Fayans wrote:

Added one more test, resolved the pep8 issues

On 10/19/2016 12:32 PM, Oleg Fayans wrote:

Hi Martin,

As you suggested, I've extended the
test_xmlrpc/test_add_remove_cert_cmd.py to contain basic tests for 
certs

in idoverrides.
The integration part still needs some polishing in the part related to
user lookup by cert

On 10/14/2016 03:57 PM, Martin Babinsky wrote:

On 10/14/2016 03:48 PM, Oleg Fayans wrote:

So, did I understand correctly, that there would be 2 patches: one
containing test for basic idoverrides functionality without
AD-integration, and the second one - with AD-integration and an sssd
check, correct?
I guess, the
freeipa-ofayans-0050.1-Automated-test-for-certs-in-idoverrides-feature.patch 





might be a good candidate for the first one, I only have to change 
the

filename to test_idviews.py, right?



Oleg, we already have XMLRPC tests for idoverrides:

ipatests/test_xmlrpc/test_idviews_plugin.py

Is there any particular reason why not to extend them with add
cert/remove cert operations?

Even better, you can extend
`ipatests/test_xmlrpc/test_add_remove_cert_cmd.py` suite by doing the
same set of tests on idoverrideuser objects.

Or am I missing something?


On 09/15/2016 10:32 AM, Martin Basti wrote:



On 15.09.2016 10:10, Oleg Fayans wrote:

Hi Martin,

The file was renamed. Did I understand correctly that for now we 
are

leaving the test as is and are planning to extend it later?


I would like to have there SSSD check involved, please use what 
Summit

recommends. No new test cases.

And this can be done by separate patch, I want to have API/CLI
certificate override tests for non-AD idview (extending current
tests I
posted in this thread)

Martin^2


On 09/15/2016 09:49 AM, Martin Basti wrote:



On 14.09.2016 18:53, Sumit Bose wrote:

On Wed, Sep 14, 2016 at 06:03:37PM +0200, Martin Basti wrote:


On 14.09.2016 17:53, Alexander Bokovoy wrote:

On Wed, 14 Sep 2016, Martin Basti wrote:


On 14.09.2016 17:41, Alexander Bokovoy wrote:

On Wed, 14 Sep 2016, Martin Basti wrote:

1)
I still don't see the reason why AD trust is needed. Default
trust ID view is added just by ipa-adtrust-install, adding
trust is not needed for current implementation. You don't
need AD for this, IDviews is generic feature not just for
AD. Is that user configured on AD side?
You cannot add non-AD user to 'default trust view', so you 
will

not be
able to set up certificates to ID override which does not
exist.

For non-'default trust view' you can add both IPA and AD 
users,

so using
some other view and then assign certificate for a ID
override in
that
one.


Ok then, but anyway I would like to see API/CLI tests for this
feature with proper output validation.


How can be this tested with SSSD?

You need to log into the system with a certificate...

Is this possible from test? We are logged remotely as root, is
there any
cmdline util which allows us to test certificate against AD 
user?


You can use 'sss_ssh_authorizedkeys aduser@ad.domain' which 
should
return the ssh key derived from the public key in the 
certificate.

This
should work for certificate stored in AD as well as for 
overrides.


You can also you the DBus lookup by certificate as described in
https://fedorahosted.org/sssd/wiki/DesignDocs/LookupUsersByCertificate 





.

HTH

bye,
Sumit


Thank you Alexander and Summit for hints.

Oleg I realized we don't have any other idviews integration tests

So I propose to rename test file you are adding to
test_idviews.py. We
can add more testcases for idviews there later

Martin^2

Martin^2

--
Manage your subscription for the Freeipa-devel mailing list:

Re: [Freeipa-devel] [Test][Patch-0047] Added a test for Ticket N 5964

[Milan Kubík]

On 11/03/2016 04:56 PM, Oleg Fayans wrote:

Hi Martin,

The commit message was updated with the correct ticket link
Thanks for review!

On 11/03/2016 04:22 PM, Martin Basti wrote:

almost ACK, but the ticket in commit message is closed as invalid. So
I'm quite puzzled now what to do.


On 03.11.2016 13:28, Oleg Fayans wrote:

ping for review

On 10/19/2016 04:54 PM, Oleg Fayans wrote:

Hi Martin,

Thanks for the review. Fixed both issues.

$ ipa-run-tests test_integration/test_topology.py -k 
TestCASpecificRUVs

WARNING: Couldn't write lextab module 'pycparser.lextab'. [Errno 13]
Permission denied: 'lextab.py'
WARNING: yacc table file version is out of date
WARNING: Couldn't create 'pycparser.yacctab'. [Errno 13] Permission
denied: 'yacctab.py'
 



test session starts
= 




platform linux2 -- Python 2.7.11, pytest-2.9.2, py-1.4.31, 
pluggy-0.3.1
rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: 
pytest.ini

plugins: sourceorder-0.5, multihost-1.0
collected 5 items

test_integration/test_topology.py ..

 



2 passed in 2444.84 seconds
= 






On 10/17/2016 07:05 PM, Martin Basti wrote:

1)

you don't need to disable/enable dirsrv, just stop/start. Please 
remove

disable/enable parts


2)




traceback





self = 

def test_delete_ruvs(self):
"""
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/
Test_Plan#Test_case:_clean-ruv_subcommand
"""
replica = self.replicas[0]
master = self.master
res1 = master.run_command(['ipa-replica-manage', 'list-ruv',
'-p',
master.config.dirman_password])

assert(res1.stdout_text.count(replica.hostname) == 2 and
   "Certificate Server Replica Update Vectors" in 
res1), (

"CA-specific RUVs are not displayed")
E   TypeError: argument of type 'SSHCommand' is not iterable

test_integration/test_topology.py:215: TypeError



entering PDB






/usr/lib/python2.7/site-packages/ipatests/test_integration/test_topology.py(215)test_delete_ruvs() 






-> assert(res1.stdout_text.count(replica.hostname) == 2 and



On 14.10.2016 11:36, Oleg Fayans wrote:

Right you are! I am sorry.

On 10/13/2016 06:10 PM, Martin Basti wrote:

I think that you forgot to squash commits. Patch 47 doesn't apply


On 13.10.2016 14:01, Oleg Fayans wrote:

Hi Martin,

Thanks for the review.
With disabling directory server it works as well, thanks for the
hint.
Also I moved the cleanup logic to the test itself for the sake of
simplicity. Patch-0048 was not changed

On 10/12/2016 02:35 PM, Martin Basti wrote:

1)

Can you just turn off dirsrv on replica instead of doing iptables
magic?


2) NACK

No more eval() ever in code, use 'getattr', 'get' or whatever in
the
object that can be used.

+evalhost = eval("args[0].%s" % host)

Martin^2

On 12.10.2016 14:03, Oleg Fayans wrote:

Hi Martin,

After extensive discussion with Ludwig, I finally got the 
clue on

how
does this feature work. When we uninstall the replica, the 
master

cleans the replication agreements with this replica and
automatically
cleans all replica's RUVs.
If we clean replica's RUVs on master without uninstalling the
replica,
the replica's RUVs get recreated on master (replication
works!). So,
the only way to test the clean-ruv subcommand is to turn off the
replica, or block the traffic on it so it gets inaccessible to
updates
from master.
The testcases were updated, see [1] and [2]

The updated versions of the patches are attached

[1]
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan#Test_case:_.2A-ruv_subcommands_of_ipa-replica-manage_are_extended_to_handle_CA-specific_RUVs 








[2]
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan#Test_case:_clean-ruv_subcommand 








On 08/05/2016 06:36 PM, Martin Basti wrote:



On 03.08.2016 14:45, Oleg Fayans wrote:

Hi Martin,

Thanks for the review! Both patches were updated.

On 07/28/2016 04:11 PM, Martin Basti wrote:



On 08.07.2016 15:41, Oleg Fayans wrote:

Hi Martin,

Thanks for the review!

On 07/08/2016 02:18 PM, Martin Basti wrote:



On 27.06.2016 13:53, Oleg Fayans wrote:

Hi guys,

Is there a chance the patches NN 0047.1 and 0048.1 get
reviewed
before
4.4 release? They cover a good part of the Managed 
Topology

4.4
feature.

On 06/17/2016 11:18 AM, Oleg Fayans wrote:

One more test was added to the patch-0048

On 06/17/2016 09:43 AM, Oleg Fayans wrote:

Fixed a bug in the previous patch, automated 2 more
testcases
from
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan 











On 06/16/2016 04:46 PM, Oleg Fayans wrote:










IIUC, this will turn off the 

Re: [Freeipa-devel] [Test][Patch-0047] Added a test for Ticket N 5964

[Milan Kubík]

On 11/09/2016 04:34 PM, Milan Kubík wrote:

On 11/03/2016 04:56 PM, Oleg Fayans wrote:

Hi Martin,

The commit message was updated with the correct ticket link
Thanks for review!

On 11/03/2016 04:22 PM, Martin Basti wrote:

almost ACK, but the ticket in commit message is closed as invalid. So
I'm quite puzzled now what to do.


On 03.11.2016 13:28, Oleg Fayans wrote:

ping for review

On 10/19/2016 04:54 PM, Oleg Fayans wrote:

Hi Martin,

Thanks for the review. Fixed both issues.

$ ipa-run-tests test_integration/test_topology.py -k 
TestCASpecificRUVs

WARNING: Couldn't write lextab module 'pycparser.lextab'. [Errno 13]
Permission denied: 'lextab.py'
WARNING: yacc table file version is out of date
WARNING: Couldn't create 'pycparser.yacctab'. [Errno 13] Permission
denied: 'yacctab.py'
 



test session starts
= 




platform linux2 -- Python 2.7.11, pytest-2.9.2, py-1.4.31, 
pluggy-0.3.1
rootdir: /usr/lib/python2.7/site-packages/ipatests, inifile: 
pytest.ini

plugins: sourceorder-0.5, multihost-1.0
collected 5 items

test_integration/test_topology.py ..

 



2 passed in 2444.84 seconds
= 






On 10/17/2016 07:05 PM, Martin Basti wrote:

1)

you don't need to disable/enable dirsrv, just stop/start. Please 
remove

disable/enable parts


2)




traceback





self = 

def test_delete_ruvs(self):
"""
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/
Test_Plan#Test_case:_clean-ruv_subcommand
"""
replica = self.replicas[0]
master = self.master
res1 = master.run_command(['ipa-replica-manage', 'list-ruv',
'-p',
master.config.dirman_password])

assert(res1.stdout_text.count(replica.hostname) == 2 and
   "Certificate Server Replica Update Vectors" in 
res1), (

"CA-specific RUVs are not displayed")
E   TypeError: argument of type 'SSHCommand' is not iterable

test_integration/test_topology.py:215: TypeError



entering PDB






/usr/lib/python2.7/site-packages/ipatests/test_integration/test_topology.py(215)test_delete_ruvs() 






-> assert(res1.stdout_text.count(replica.hostname) == 2 and



On 14.10.2016 11:36, Oleg Fayans wrote:

Right you are! I am sorry.

On 10/13/2016 06:10 PM, Martin Basti wrote:

I think that you forgot to squash commits. Patch 47 doesn't apply


On 13.10.2016 14:01, Oleg Fayans wrote:

Hi Martin,

Thanks for the review.
With disabling directory server it works as well, thanks for the
hint.
Also I moved the cleanup logic to the test itself for the sake of
simplicity. Patch-0048 was not changed

On 10/12/2016 02:35 PM, Martin Basti wrote:

1)

Can you just turn off dirsrv on replica instead of doing 
iptables

magic?


2) NACK

No more eval() ever in code, use 'getattr', 'get' or whatever in
the
object that can be used.

+evalhost = eval("args[0].%s" % host)

Martin^2

On 12.10.2016 14:03, Oleg Fayans wrote:

Hi Martin,

After extensive discussion with Ludwig, I finally got the 
clue on

how
does this feature work. When we uninstall the replica, the 
master

cleans the replication agreements with this replica and
automatically
cleans all replica's RUVs.
If we clean replica's RUVs on master without uninstalling the
replica,
the replica's RUVs get recreated on master (replication
works!). So,
the only way to test the clean-ruv subcommand is to turn off 
the

replica, or block the traffic on it so it gets inaccessible to
updates
from master.
The testcases were updated, see [1] and [2]

The updated versions of the patches are attached

[1]
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan#Test_case:_.2A-ruv_subcommands_of_ipa-replica-manage_are_extended_to_handle_CA-specific_RUVs 








[2]
http://www.freeipa.org/page/V4/Manage_replication_topology_4_4/Test_Plan#Test_case:_clean-ruv_subcommand 








On 08/05/2016 06:36 PM, Martin Basti wrote:



On 03.08.2016 14:45, Oleg Fayans wrote:

Hi Martin,

Thanks for the review! Both patches were updated.

On 07/28/2016 04:11 PM, Martin Basti wrote:



On 08.07.2016 15:41, Oleg Fayans wrote:

Hi Martin,

Thanks for the review!

On 07/08/2016 02:18 PM, Martin Basti wrote:



On 27.06.2016 13:53, Oleg Fayans wrote:

Hi guys,

Is there a chance the patches NN 0047.1 and 0048.1 get
reviewed
before
4.4 release? They cover a good part of the Managed 
Topology

4.4
feature.

On 06/17/2016 11:18 AM, Oleg Fayans wrote:

One more test was added to the patch-0048

On 06/17/2016 09:43 AM, Oleg Fayans wrote:

Fixed a bug in the previous patch, automated 2 more
testcases
from
http://www.freeipa.org/page/V4/Manage_replication_

Re: [Freeipa-devel] [PATCH 0035][Tests] Fix failing tests in test_ipalib/test_frontend

[Milan Kubík]

On 08/17/2016 05:08 PM, Lenka Doudova wrote:




On 08/17/2016 04:57 PM, Milan Kubík wrote:

On 08/17/2016 04:45 PM, Lenka Doudova wrote:

Hi,

attached patch provides fix for 2 out of three failing tests in 
ipatests/test_ipalib/test_frontend.py. Failures were caused by 
changes related to thin client implementation.


Fix for the third failure will be provided later (after my PTO), as 
it will be more complicated fix.


Lenka





NACK:

* Module ipatests.test_ipalib.test_frontend
ipatests/test_ipalib/test_frontend.py:944: 
[E1120(no-value-for-parameter), test_Object.test_init.FakeAPI] No 
value for argument 'base' in constructor call)


--
Milan Kubik

Oh sorry, I attached non-updated patch...
Correct one attached now.

Lenka


Thanks for the work. ACK

--
Milan Kubik

-- 
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 0035][Tests] Fix failing tests in test_ipalib/test_frontend

[Milan Kubík]

On 08/17/2016 04:45 PM, Lenka Doudova wrote:

Hi,

attached patch provides fix for 2 out of three failing tests in 
ipatests/test_ipalib/test_frontend.py. Failures were caused by changes 
related to thin client implementation.


Fix for the third failure will be provided later (after my PTO), as it 
will be more complicated fix.


Lenka





NACK:

* Module ipatests.test_ipalib.test_frontend
ipatests/test_ipalib/test_frontend.py:944: 
[E1120(no-value-for-parameter), test_Object.test_init.FakeAPI] No value 
for argument 'base' in constructor call)


--
Milan Kubik

-- 
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 0034][Tests] Fix failing tests in test_ipalib/test_parameters

[Milan Kubík]

On 08/17/2016 04:31 PM, Lenka Doudova wrote:

Hi,

attached patch fixes part of failing tests in 
ipatests/test_ipalib/test_parameters.py. Failures were caused mainly 
by thin client feature, sometimes by usage of unicode, which tests did 
not reflect. Issues were discussed with Honza.


Remaining failing tests should be fixed in scope of one of Martin3's 
future patches (namely failures described in 
https://fedorahosted.org/freeipa/ticket/6190)


Lenka




ACK


--
Milan Kubik

-- 
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] [patch 0052] ipatests: Fix wrong fixture in kerberos principal alias test

[Milan Kubík]

Fixes issue in ticket https://fedorahosted.org/freeipa/ticket/6197


--
Milan Kubik

From b0a731c2b655c331001c9cb217f66045c9c2fdb7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Mon, 15 Aug 2016 14:31:48 +0200
Subject: [PATCH] ipatests: Fix wrong fixture in kerberos principal alias test

https://fedorahosted.org/freeipa/ticket/6197
---
 ipatests/test_xmlrpc/test_kerberos_principal_aliases.py | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/ipatests/test_xmlrpc/test_kerberos_principal_aliases.py b/ipatests/test_xmlrpc/test_kerberos_principal_aliases.py
index 3bbc641f1c5ea495d6a8e37e8aff67f1c67d5f4e..a1973af2c1f29e187d0bca8038c5452b1bd0b6cc 100644
--- a/ipatests/test_xmlrpc/test_kerberos_principal_aliases.py
+++ b/ipatests/test_xmlrpc/test_kerberos_principal_aliases.py
@@ -41,7 +41,7 @@ def trusted_domain():
 are deleted from the directory.
 """
 
-trusted_dom = TRUSTED_DOMAIN_MOCK['ldif']
+trusted_dom = TRUSTED_DOMAIN_MOCK
 
 # Write the changes
 with mocked_trust_containers(), MockLDAP() as ldap:
-- 
2.9.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 42-44, 48-51][tests] RFE: Allow users to authenticate with alternative names

[Milan Kubík]

On 07/28/2016 12:51 PM, Martin Babinsky wrote:

On 07/27/2016 11:54 AM, Milan Kubík wrote:







Hi Milan,

the tests seem to work as expected except
`test_enterprise_principal_UPN_overlap_without_additional_suffix`
which crashes on #6099. I have a few comments, however:

This is a test that hits a known bug. I have added an expected fail
marker for it.


Patch 42:

1.)
+class KerberosAliasMixin:

make sure the class inherits from 'object' so that it behaves like
new-style class in Python 2.

Also, I think that 'check_for_tracker' method is slightly redundant.
If somebody would use the mixin directly, then he will still get
"NotImplementedError" exceptions and see that he probably did
something wrong.

If you really really want to treat to prevent the instantiation of the
class, then use ABC module to turn it into proper abstract class.

But in this case it should IMHO be enough that you explained the class
usage in the module docstring.

Ok. Fixed the inheritance and removed the check for Tracker. The check
for krbprincipalname attribute has been kept.


2.)
+def _make_add_alias_cmd(self):
+raise RuntimeError("The _make_add_alias_cmd method "
+   "is not implemented.")
+
+def _make_remove_alias_cmd(self):
+raise RuntimeError("The _make_remove_alias_cmd method "
+   "is not implemented."

Abstract methods should raise "NotImplementedError" exception.


Fixed.

3.)
is the 'options=None' kwarg in {add,remove}_principals methods
necessary? I didn't see it anywhere in the later commits so I guess
you can safely remove it. Better yet, you can just replace it with
'**options' since all it does is to pass options to the
`*-add/remove-principal` commands.


Fixed to **options.

4.)
a nitpick but IIRC the mixin class should be listed before the actual
hierarchy base so that MRO works as expected.

So instead

+class UserTracker(Tracker, KerberosAliasMixin):

It should say
+class UserTracker(KerberosAliasMixin, Tracker):

Fixed in all three classes.


PATCH 43: LGTM

PATCH 44:

Please do not create any more utility modules. Either put it to
existing 'ipatests/util.py' or better yet, rename the module to
something like 'mock_trust' since the scope of it is to provide mocked
trust entries.

Moved the functions from test_range_plugin.py module to a new mock_trust
module. The fix for the range plugin test introduced a new commit here.


PATCH 45:

It would be nice if you could add '-E' option in the same way to
indicate enterprise principal name resolution.

Done.


PATCH 46: LGTM
PATCH 47:

1.)
+from ipatests.test_xmlrpc.test_range_plugin import (
+get_trust_dn, get_trusted_dom_dict, encode_mockldap_value)

Since you already introduced a module grouping all trust-mocking
machinery in patch 44, you could extract these functions and put it
there to improve code reuse.

Fixed.


2.)
I am not a big fan of duplicate code in 'trusted_domain' and
'trusted_domain_with_suffix' fixtures. Use some module-level mapping
for common attributes and add more specific attributes per fixture.

Fixed


3.)
I would like to expand the test cases for AD realm namespace overlap
checks. We should reject enterprise principals with suffixes
coinciding with realm names, NETBIOS names, and UPN suffixes and I
would like to test all three cases to get a full coverage.


Extended with explicit checks fot rhe AD realm and NETBIOS name by
constructing the enterprise principal from corresponding LDAP 
attributes.


The fixed and rebased version of the commits is in my repo [1].

[1]: https://github.com/apophys/freeipa/tree/krb5-principal-aliases-test

Regards



Tests works and code is ok, however you will need to create a separate 
ticket to your patches, since it is not allowed to add fixes to 
tickets in closed milestones. Sorry that I didn't notice it earlier.


cond-ACK if you create ticket and remove the xfail from 
`test_enterprise_principal_overlap_with_AD_realm` test case as the fix 
for #6099 was pushed to master.




Hi,

thanks for the review. The xfail marker was removed. The commit messages 
now reffer to new ticket [1].
Since the changes during review introduced new commit in the sequence, I 
have abandoned the patches 45 to 47 and renumbered them (with the new 
one) from 48 onwards.


[1]: https://fedorahosted.org/freeipa/ticket/6142

Regards

--
Milan Kubik

From 0a56dc99aa1243e525e8c0fdd8ef217077d23ab1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Fri, 22 Jul 2016 17:07:56 +0200
Subject: [PATCH 1/7] ipatests: Add tracker class for kerberos principal
 aliases

The commit implements a mixin class providing capability
to track and modify kerberos principal aliases on supported
types of entries.

The class using the mixin must inherit from the Tracker class
and must provide the implementation of two methods:

* _make_add_alias_cmd
* _make

Re: [Freeipa-devel] [PATCH 42-47][tests] RFE: Allow users to authenticate with alternative names

[Milan Kubík]

Hi Milan,

the tests seem to work as expected except 
`test_enterprise_principal_UPN_overlap_without_additional_suffix` 
which crashes on #6099. I have a few comments, however:
This is a test that hits a known bug. I have added an expected fail 
marker for it.


Patch 42:

1.)
+class KerberosAliasMixin:

make sure the class inherits from 'object' so that it behaves like 
new-style class in Python 2.


Also, I think that 'check_for_tracker' method is slightly redundant. 
If somebody would use the mixin directly, then he will still get 
"NotImplementedError" exceptions and see that he probably did 
something wrong.


If you really really want to treat to prevent the instantiation of the 
class, then use ABC module to turn it into proper abstract class.


But in this case it should IMHO be enough that you explained the class 
usage in the module docstring.
Ok. Fixed the inheritance and removed the check for Tracker. The check 
for krbprincipalname attribute has been kept.


2.)
+def _make_add_alias_cmd(self):
+raise RuntimeError("The _make_add_alias_cmd method "
+   "is not implemented.")
+
+def _make_remove_alias_cmd(self):
+raise RuntimeError("The _make_remove_alias_cmd method "
+   "is not implemented."

Abstract methods should raise "NotImplementedError" exception.


Fixed.

3.)
is the 'options=None' kwarg in {add,remove}_principals methods 
necessary? I didn't see it anywhere in the later commits so I guess 
you can safely remove it. Better yet, you can just replace it with 
'**options' since all it does is to pass options to the 
`*-add/remove-principal` commands.



Fixed to **options.

4.)
a nitpick but IIRC the mixin class should be listed before the actual 
hierarchy base so that MRO works as expected.


So instead

+class UserTracker(Tracker, KerberosAliasMixin):

It should say
+class UserTracker(KerberosAliasMixin, Tracker):

Fixed in all three classes.


PATCH 43: LGTM

PATCH 44:

Please do not create any more utility modules. Either put it to 
existing 'ipatests/util.py' or better yet, rename the module to 
something like 'mock_trust' since the scope of it is to provide mocked 
trust entries.
Moved the functions from test_range_plugin.py module to a new mock_trust 
module. The fix for the range plugin test introduced a new commit here.


PATCH 45:

It would be nice if you could add '-E' option in the same way to 
indicate enterprise principal name resolution.

Done.


PATCH 46: LGTM
PATCH 47:

1.)
+from ipatests.test_xmlrpc.test_range_plugin import (
+get_trust_dn, get_trusted_dom_dict, encode_mockldap_value)

Since you already introduced a module grouping all trust-mocking 
machinery in patch 44, you could extract these functions and put it 
there to improve code reuse.

Fixed.


2.)
I am not a big fan of duplicate code in 'trusted_domain' and 
'trusted_domain_with_suffix' fixtures. Use some module-level mapping 
for common attributes and add more specific attributes per fixture.

Fixed


3.)
I would like to expand the test cases for AD realm namespace overlap 
checks. We should reject enterprise principals with suffixes 
coinciding with realm names, NETBIOS names, and UPN suffixes and I 
would like to test all three cases to get a full coverage.


Extended with explicit checks fot rhe AD realm and NETBIOS name by 
constructing the enterprise principal from corresponding LDAP attributes.


The fixed and rebased version of the commits is in my repo [1].

[1]: https://github.com/apophys/freeipa/tree/krb5-principal-aliases-test

Regards

--
Milan Kubik

--
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 42-47][tests] RFE: Allow users to authenticate with alternative names

[Milan Kubík]

On 07/25/2016 01:53 PM, Milan Kubík wrote:

Hi,

I'm sending the tests for kerberos principal aliases rfe. The tests 
are implemented according to test plan [1] sent earlier.
Some of the patches implement modifications and extensions to previous 
code to allow implement the tests themselves.


The patches can be cloned also from github [2].

[1]: http://www.freeipa.org/page/V4/Kerberos_principal_aliases/Test_Plan
[2]: https://github.com/apophys/freeipa/tree/krb5-principal-aliases-test

Cheers,





Self nack for 0047, the ldapconn fixture is not needed. New patch attached.
Git repo updated (force-push).

--
Milan Kubik

From 5dd5fe5d0ccc921949dedb2f3e2497344f87e493 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Fri, 22 Jul 2016 17:25:06 +0200
Subject: [PATCH] ipatests: Add kerberos principal alias tests

Add tests for alias manipulation, tests authentication and several
error scenarios.

https://fedorahosted.org/freeipa/ticket/3864
https://fedorahosted.org/freeipa/ticket/5413
https://fedorahosted.org/freeipa/ticket/6099
---
 .../test_xmlrpc/test_kerberos_principal_aliases.py | 261 +
 1 file changed, 261 insertions(+)
 create mode 100644 ipatests/test_xmlrpc/test_kerberos_principal_aliases.py

diff --git a/ipatests/test_xmlrpc/test_kerberos_principal_aliases.py b/ipatests/test_xmlrpc/test_kerberos_principal_aliases.py
new file mode 100644
index ..11a69e6664a219c6f6b682266ff1b75327ae0046
--- /dev/null
+++ b/ipatests/test_xmlrpc/test_kerberos_principal_aliases.py
@@ -0,0 +1,261 @@
+# coding: utf-8
+#
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+#
+import ldap
+import pytest
+
+from ipalib import errors, api
+from ipapython import ipautil
+from ipaplatform.paths import paths
+
+from ipatests.util import MockLDAP
+from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test
+from ipatests.test_xmlrpc.tracker.user_plugin import UserTracker
+from ipatests.test_xmlrpc.tracker.host_plugin import HostTracker
+from ipatests.test_xmlrpc.tracker.service_plugin import ServiceTracker
+from ipatests.test_xmlrpc.test_range_plugin import (
+get_trust_dn, get_trusted_dom_dict, encode_mockldap_value)
+from ipatests.test_xmlrpc.utils import mocked_trust_containers
+from ipatests.util import unlock_principal_password, change_principal
+
+
+@pytest.yield_fixture
+def trusted_domain():
+"""Fixture providing mocked AD trust entries
+
+The fixture yields after creating a mock of AD trust
+entries in the directory server. After the test, the entries
+are deleted from the directory.
+"""
+trusted_dom = u'trusted.domain.net'
+trusted_dom_dn = get_trust_dn(trusted_dom)
+trusted_dom_sid = u'S-1-5-21-2997650941-1802118864-3094776726'
+
+trusted_dom_add = get_trusted_dom_dict(trusted_dom, trusted_dom_sid)
+
+# Write the changes
+with mocked_trust_containers(), MockLDAP() as ldap:
+ldap.add_entry(trusted_dom_dn, trusted_dom_add)
+yield trusted_dom
+ldap.del_entry(trusted_dom_dn)
+
+
+@pytest.yield_fixture
+def trusted_domain_with_suffix():
+"""Fixture providing mocked AD trust entries
+
+The fixture yields after creating a mock of AD trust
+entries in the directory server. After the test, the entries
+are deleted from the directory.
+"""
+trusted_dom = u'trusted.domain.net'
+trusted_dom_dn = get_trust_dn(trusted_dom)
+trusted_dom_sid = u'S-1-5-21-2997650941-1802118864-3094776726'
+
+trusted_dom_add = get_trusted_dom_dict(trusted_dom, trusted_dom_sid)
+trusted_dom_add['ipaNTAdditionalSuffixes'] = (
+encode_mockldap_value(trusted_dom))
+
+# Write the changes
+with mocked_trust_containers(),  MockLDAP() as ldap:
+ldap.add_entry(trusted_dom_dn, trusted_dom_add)
+yield trusted_dom
+ldap.del_entry(trusted_dom_dn)
+
+
+@pytest.fixture(scope='function')
+def krbalias_user(request):
+tracker = UserTracker(u'krbalias_user', u'krbalias', u'test')
+
+return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='function')
+def krbalias_user_c(request):
+tracker = UserTracker(u'krbalias_user_conflict', u'krbalias', u'test')
+
+return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='function')
+def krbalias_host(request):
+tracker = HostTracker(u'testhost-krb')
+
+return tracker.make_fixture(request)
+
+
+@pytest.fixture
+def krb_service_host(request):
+tracker = HostTracker(u'krb-srv-host')
+
+return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='function')
+def krbalias_service(request, krb_service_host):
+krb_service_host.ensure_exists()
+
+tracker = ServiceTracker(name=u'SRV1', host_fqdn=krb_service_host.name)
+
+return tracker.make_fixture(request)
+
+
+@pytest.fixture
+def ldapservice(request):
+tracker = ServiceTracker(
+   

[Freeipa-devel] [PATCH 42-47][tests] RFE: Allow users to authenticate with alternative names

[Milan Kubík]

Hi,

I'm sending the tests for kerberos principal aliases rfe. The tests are 
implemented according to test plan [1] sent earlier.
Some of the patches implement modifications and extensions to previous 
code to allow implement the tests themselves.


The patches can be cloned also from github [2].

[1]: http://www.freeipa.org/page/V4/Kerberos_principal_aliases/Test_Plan
[2]: https://github.com/apophys/freeipa/tree/krb5-principal-aliases-test

Cheers,

--
Milan Kubik

From e4fdd561af48beb02748e9cb14ffcb4e5f257128 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Fri, 22 Jul 2016 17:07:56 +0200
Subject: [PATCH 1/6] ipatests: Add tracker class for kerberos principal
 aliases

The commit implements a mixin class providing capability
to track and modify kerberos principal aliases on supported
types of entries.

The class using the mixin must inherit from the Tracker class
and must provide the implementation of two methods:

* _make_add_alias_cmd
* _make_remove_alias_cmd

These are used to get the type specific command for the particular
entry class. The methods provided will not work on entries that
do not have 'krbprincipalname' attribute.

The service, host and user trackers are being extended to use this
new mixin class.

https://fedorahosted.org/freeipa/ticket/3864
---
 ipatests/test_xmlrpc/tracker/host_plugin.py  |  10 ++-
 ipatests/test_xmlrpc/tracker/kerberos_aliases.py | 110 +++
 ipatests/test_xmlrpc/tracker/service_plugin.py   |  17 +++-
 ipatests/test_xmlrpc/tracker/user_plugin.py  |  10 ++-
 4 files changed, 141 insertions(+), 6 deletions(-)
 create mode 100644 ipatests/test_xmlrpc/tracker/kerberos_aliases.py

diff --git a/ipatests/test_xmlrpc/tracker/host_plugin.py b/ipatests/test_xmlrpc/tracker/host_plugin.py
index 03113b8fe7de7bfa2d2383b1903ff05d2543a9ed..8630a7d0c96edd725f448f5a460df91dd8f37ccc 100644
--- a/ipatests/test_xmlrpc/tracker/host_plugin.py
+++ b/ipatests/test_xmlrpc/tracker/host_plugin.py
@@ -7,13 +7,14 @@ from __future__ import print_function
 
 from ipapython.dn import DN
 from ipatests.test_xmlrpc.tracker.base import Tracker
+from ipatests.test_xmlrpc.tracker.kerberos_aliases import KerberosAliasMixin
 from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid
 from ipatests.test_xmlrpc import objectclasses
 from ipatests.util import assert_deepequal
 from ipalib import errors
 
 
-class HostTracker(Tracker):
+class HostTracker(Tracker, KerberosAliasMixin):
 """Wraps and tracks modifications to a Host object
 
 Implements the helper functions for host plugin.
@@ -175,3 +176,10 @@ class HostTracker(Tracker):
 pass
 
 request.addfinalizer(cleanup)
+
+#  Kerberos aliases methods
+def _make_add_alias_cmd(self):
+return self.make_command('host_add_principal', self.name)
+
+def _make_remove_alias_cmd(self):
+return self.make_command('host_remove_principal', self.name)
diff --git a/ipatests/test_xmlrpc/tracker/kerberos_aliases.py b/ipatests/test_xmlrpc/tracker/kerberos_aliases.py
new file mode 100644
index ..cf82a7a8ce27506fd0cdbff6d297ffa5c5b6e4b9
--- /dev/null
+++ b/ipatests/test_xmlrpc/tracker/kerberos_aliases.py
@@ -0,0 +1,110 @@
+#
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+#
+"""kerberos_aliases
+
+The module implements a mixin class that provides an interface
+to the Kerberos Aliases feature of freeIPA.
+
+In order to use the class the child class must implement the
+`_make_add_alias_cmd` and `_make_remove_alias_cmd` methods that
+are different for each entity type.
+
+The KerberosAliasMixin class then provides the implementation
+of the manipulation of the kerberos alias in general.
+
+It is up to the child class or the user to validate the
+alias being added for a particular type of an entry.
+"""
+
+from ipatests.test_xmlrpc.tracker.base import Tracker
+
+
+class KerberosAliasError(Exception):
+pass
+
+
+class KerberosAliasMixin:
+"""KerberosAliasMixin"""
+
+def _make_add_alias_cmd(self):
+raise RuntimeError("The _make_add_alias_cmd method "
+   "is not implemented.")
+
+def _make_remove_alias_cmd(self):
+raise RuntimeError("The _make_remove_alias_cmd method "
+   "is not implemented.")
+
+def _check_for_tracker(self):
+if not isinstance(self, Tracker):
+raise KerberosAliasError("Class using {} must inherit from {}."
+ .format(KerberosAliasMixin, Tracker))
+
+def _check_for_krbprincipalname_attr(self):
+# Check if the tracker has a principal name
+# Each compatible entry has at least one kerberos
+# principal matching the canonical principal name
+principals = self.attrs.get('krbprincipalname')
+if self.exists:
+if not principals:
+raise KerberosAliasError(
+"{} 

Re: [Freeipa-devel] [PATCH 0014-0016][Tests] Authentication indicators

[Milan Kubík]

On 07/14/2016 11:43 AM, Lenka Doudova wrote:




On 07/14/2016 11:25 AM, Lenka Doudova wrote:




On 07/14/2016 09:20 AM, Lenka Doudova wrote:




On 07/13/2016 04:48 PM, Milan Kubík wrote:

On 07/11/2016 01:34 PM, Lenka Doudova wrote:




On 07/08/2016 02:24 PM, Milan Kubík wrote:

On 07/01/2016 05:13 PM, Lenka Doudova wrote:




On 07/01/2016 02:42 PM, Milan Kubík wrote:

On 06/16/2016 03:23 PM, Lenka Doudova wrote:

Hi,

attached are tests for authentication indicators. Please note:

1. newly created service tracker is not exactly complete, list 
of unimplemented methods is in doc. These methods can be 
filled in when existing declarative tests are refactored.


2. patch 0015 depends on 0014, so it should not be pushed 
without it.



Lenka





patch 0014:

In the update method, what happens when the updated attributes 
contain addattr? It is not clear to me. Is it necessary?



Example:
ipa service-mod SRV --addattr="authind=radius"

Result:
The way the tracker works, this adds 
/u'addattr="authind=radius"'/ to the list of expected results 
(result of /self.attrs.update(updates)/. Of course nothing like 
that appears anywhere, so in case there's the /--addattr/ 
option, it's necessary to ensure it won't get to the 
/self.attrs/ atribute.



patch 0015:

host1 and service2 do not tell anything about the purpose of 
the fixture. Please assign more descriptive names to them.
Why do the fixtures have 'function' scope? Does the service 
entry exist during the second and third test case?



Renamed.


patch 0016:

Per offline discussion, admin user has no special privileges 
here, LGTM.


--
Milan Kubik


Thanks for review, fixed patches (14.2 and 15.2) attached.
Lenka


NACK,

the update method of ServiceTracker creates the entry if it 
doesn't exist. Why? I know the base class has this problem also 
[1], though.
Given this will be addressed, the fixtures in the xmlrpc test 
will fail since the fixture scope is wrong - function instead of 
class.


[1]: https://fedorahosted.org/freeipa/ticket/6045

--
Milan Kubik

Hi,

fixed patch attached. I chose to leave the scope as it was (in 
case one test breaks and entry is not even created, the other 
tests can still be successful), but I removed the creation command 
from ServiceTracker update method and called it directly in the 
tests, so there shouldn't be hidden actions.


Lenka


Thanks for the updated patches. There are still some issues I 
haven't noticed before:


service tracker: track_create method doesn't set self.exists flag 
which is needed


In the xmlrpc test method `test_adding_second_indicator` uses 
'addattr' to set the indicator, why?


--
Milan Kubik


Hi,
fixes for comments in attached patches.
After offline discussion, I removed the usage of "addattr" and 
replaced it with test to add two indicators in one command.


Lenka


One more problem was pointed out to me offline, attaching changed 
patch 0014.


Lenka




Resending the complete patch set.
L.




Thanks, ACK.

--
Milan Kubik

-- 
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 0014-0016][Tests] Authentication indicators

[Milan Kubík]

On 07/11/2016 01:34 PM, Lenka Doudova wrote:




On 07/08/2016 02:24 PM, Milan Kubík wrote:

On 07/01/2016 05:13 PM, Lenka Doudova wrote:




On 07/01/2016 02:42 PM, Milan Kubík wrote:

On 06/16/2016 03:23 PM, Lenka Doudova wrote:

Hi,

attached are tests for authentication indicators. Please note:

1. newly created service tracker is not exactly complete, list of 
unimplemented methods is in doc. These methods can be filled in 
when existing declarative tests are refactored.


2. patch 0015 depends on 0014, so it should not be pushed without it.


Lenka





patch 0014:

In the update method, what happens when the updated attributes 
contain addattr? It is not clear to me. Is it necessary?



Example:
ipa service-mod SRV --addattr="authind=radius"

Result:
The way the tracker works, this adds 
/u'addattr="authind=radius"'/ to the list of expected results 
(result of /self.attrs.update(updates)/. Of course nothing like that 
appears anywhere, so in case there's the /--addattr/ option, it's 
necessary to ensure it won't get to the /self.attrs/ atribute.



patch 0015:

host1 and service2 do not tell anything about the purpose of the 
fixture. Please assign more descriptive names to them.
Why do the fixtures have 'function' scope? Does the service entry 
exist during the second and third test case?



Renamed.


patch 0016:

Per offline discussion, admin user has no special privileges here, 
LGTM.


--
Milan Kubik


Thanks for review, fixed patches (14.2 and 15.2) attached.
Lenka


NACK,

the update method of ServiceTracker creates the entry if it doesn't 
exist. Why? I know the base class has this problem also [1], though.
Given this will be addressed, the fixtures in the xmlrpc test will 
fail since the fixture scope is wrong - function instead of class.


[1]: https://fedorahosted.org/freeipa/ticket/6045

--
Milan Kubik

Hi,

fixed patch attached. I chose to leave the scope as it was (in case 
one test breaks and entry is not even created, the other tests can 
still be successful), but I removed the creation command from 
ServiceTracker update method and called it directly in the tests, so 
there shouldn't be hidden actions.


Lenka


Thanks for the updated patches. There are still some issues I haven't 
noticed before:


service tracker: track_create method doesn't set self.exists flag which 
is needed


In the xmlrpc test method `test_adding_second_indicator` uses 'addattr' 
to set the indicator, why?


--
Milan Kubik

-- 
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 0014-0016][Tests] Authentication indicators

[Milan Kubík]

On 07/01/2016 05:13 PM, Lenka Doudova wrote:




On 07/01/2016 02:42 PM, Milan Kubík wrote:

On 06/16/2016 03:23 PM, Lenka Doudova wrote:

Hi,

attached are tests for authentication indicators. Please note:

1. newly created service tracker is not exactly complete, list of 
unimplemented methods is in doc. These methods can be filled in when 
existing declarative tests are refactored.


2. patch 0015 depends on 0014, so it should not be pushed without it.


Lenka





patch 0014:

In the update method, what happens when the updated attributes 
contain addattr? It is not clear to me. Is it necessary?



Example:
ipa service-mod SRV --addattr="authind=radius"

Result:
The way the tracker works, this adds /u'addattr="authind=radius"'/ 
to the list of expected results (result of 
/self.attrs.update(updates)/. Of course nothing like that appears 
anywhere, so in case there's the /--addattr/ option, it's necessary to 
ensure it won't get to the /self.attrs/ atribute.



patch 0015:

host1 and service2 do not tell anything about the purpose of the 
fixture. Please assign more descriptive names to them.
Why do the fixtures have 'function' scope? Does the service entry 
exist during the second and third test case?



Renamed.


patch 0016:

Per offline discussion, admin user has no special privileges here, LGTM.

--
Milan Kubik


Thanks for review, fixed patches (14.2 and 15.2) attached.
Lenka


NACK,

the update method of ServiceTracker creates the entry if it doesn't 
exist. Why? I know the base class has this problem also [1], though.
Given this will be addressed, the fixtures in the xmlrpc test will fail 
since the fixture scope is wrong - function instead of class.


[1]: https://fedorahosted.org/freeipa/ticket/6045

--
Milan Kubik

-- 
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 0038-0040] Sub CA test patches

[Milan Kubík]

On 07/04/2016 08:57 AM, Fraser Tweedale wrote:

Hi Milan,

Yes, we can :)  Two issues, outlined below.


1)
Running the tests, I get error in
test_create_subca_with_subject_conflict cleanup::

  ERROR at teardown of 
TestCAbasicCRUD.test_create_subca_with_subject_conflict _

 def cleanup():
 created = self.exists
 try:
 del_command()

 
 E   NotFound: crud-subca-2: Certificate Authority not found


I do not know testing framework very well but it looks like
track_create() sets 'self.exists = True' before the create command
throws the (expected) DuplicateEntry error.  (These are called from
create() in the tracker 'base' class).  Later, cleanup() catches a
NotFound but re-throws it because it believes the entry should have
existed.


2)
the usercert.conf.tmpl does not like a subject base with spaces in
it, i.e. if 'openssl req' config template gets formatted like:

 [ dn ]
 commonName = "alice"
 o=IPA.LOCAL 201606201330

then 'openssl req' fails with nasty error like:

 140644791924600:error:0D06407A:asn1 encoding 
routines:a2d_ASN1_OBJECT:first num too large:a_object.c:108:
 140644791924600:error:0B083077:x509 certificate 
routines:X509_NAME_ENTRY_create_by_txt:invalid field name:x509name.c:295:name=o

and CalledProcessError gets raised and the test fails.

Simplest solution is to simply remove the '{ipacertbase}' from the
template, because AFAIK it is not needed and parsing and formatting
the certbase (which could have multiple AVAs) is more complex than
the test calls for, IMO.


Thanks,
Fraser

Hi, thanks.

I must have missed the first issue after I removed the expected fail 
marker. I have fixed it now.


As for the usercert template, this code is older than the issues at 
hand. I do not remember why exactly I used that

option in the openssl config. I have removed that in a new patch.


--
Milan Kubik

From ac2fa02ef8f4d7290d8b7dbdd12ebf84d85d71f6 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Fri, 17 Jun 2016 12:20:55 +0200
Subject: [PATCH 1/3] ipatests: Tracker implementation for Sub CA feature

The patch implements Tracker subclass for CA plugin
and the basic CRUD tests for the plugin entries.

https://fedorahosted.org/freeipa/ticket/4559
---
 ipatests/test_xmlrpc/objectclasses.py |   5 +
 ipatests/test_xmlrpc/test_ca_plugin.py| 175 ++
 ipatests/test_xmlrpc/tracker/ca_plugin.py | 126 +
 ipatests/test_xmlrpc/xmlrpc_test.py   |   3 +
 4 files changed, 309 insertions(+)
 create mode 100644 ipatests/test_xmlrpc/test_ca_plugin.py
 create mode 100644 ipatests/test_xmlrpc/tracker/ca_plugin.py

diff --git a/ipatests/test_xmlrpc/objectclasses.py b/ipatests/test_xmlrpc/objectclasses.py
index 134a08803f3abca1124c4d26274d9e3fc981b941..1ea020b18f975f717eb9d9d5399d8e9fb40264cb 100644
--- a/ipatests/test_xmlrpc/objectclasses.py
+++ b/ipatests/test_xmlrpc/objectclasses.py
@@ -222,3 +222,8 @@ caacl = [
 u'ipaassociation',
 u'ipacaacl'
 ]
+
+ca = [
+u'top',
+u'ipaca',
+]
diff --git a/ipatests/test_xmlrpc/test_ca_plugin.py b/ipatests/test_xmlrpc/test_ca_plugin.py
new file mode 100644
index ..1e0e52ff748a5dc4faeab6526783ae95237ba73b
--- /dev/null
+++ b/ipatests/test_xmlrpc/test_ca_plugin.py
@@ -0,0 +1,175 @@
+#
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+#
+
+"""
+Test the `ipalib.plugins.ca` module.
+"""
+
+import pytest
+
+from ipalib import errors
+from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test, fuzzy_issuer
+
+from ipatests.test_xmlrpc.tracker.certprofile_plugin import CertprofileTracker
+from ipatests.test_xmlrpc.tracker.caacl_plugin import CAACLTracker
+from ipatests.test_xmlrpc.tracker.ca_plugin import CATracker
+
+
+@pytest.fixture(scope='module')
+def default_profile(request):
+name = 'caIPAserviceCert'
+desc = u'Standard profile for network services'
+tracker = CertprofileTracker(name, store=True, desc=desc)
+tracker.track_create()
+return tracker
+
+
+@pytest.fixture(scope='module')
+def default_acl(request):
+name = u'hosts_services_caIPAserviceCert'
+tracker = CAACLTracker(name, service_category=u'all', host_category=u'all')
+tracker.track_create()
+tracker.attrs.update(
+{u'ipamembercertprofile_certprofile': [u'caIPAserviceCert']})
+return tracker
+
+
+@pytest.fixture(scope='module')
+def default_ca(request):
+name = u'ipa'
+desc = u'IPA CA'
+tracker = CATracker(name, fuzzy_issuer, desc=desc)
+tracker.track_create()
+return tracker
+
+
+@pytest.fixture(scope='class')
+def crud_subca(request):
+name = u'crud-subca'
+subject = u'CN=crud subca test,O=crud testing inc'
+tracker = CATracker(name, subject)
+
+return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='class')
+def subject_conflict_subca(request):
+

Re: [Freeipa-devel] [patch 0038-0040] Sub CA test patches

[Milan Kubík]

On 06/27/2016 01:31 PM, Milan Kubík wrote:

On 06/27/2016 02:57 AM, Fraser Tweedale wrote:

On Fri, Jun 24, 2016 at 12:08:24PM +0200, Milan Kubík wrote:

On 06/24/2016 03:42 AM, Fraser Tweedale wrote:

On Tue, Jun 21, 2016 at 05:01:35PM +0200, Milan Kubík wrote:

Hi Fraser and list,

I have made changes to the test plan on the wiki [1] according to the
information in "[Testplan review] Sub CAs" thread.

I also implemented the tests in the test plan:

patch 0038 - CATracker and CA CRUD test
patch 0039 - extension to CA ACL test
patch 0040 - functional test with ACLs and certificate profile, 
reusing my
previous S/MIME based tests. This patch also tests for the 
cert-request

behavior when profile ID or CA cn are ommited.

The tests ATM do not verify the Issuer name in the certificate 
itself, just

from the ipa entry of the certificate.


The approach you are using::

  assert cert_info['result']['issuer'] == 
smime_signing_ca.ipasubjectdn


is not quite as you describe (these are virtual attributes, not
attributes of an actual entry); but the approach is valid.
The issue then is in the wording? The other approach I could have 
used here

is to retrieve the two certificates and compare the fields manually.
Are these virtual attributes created from the certificate itself?


That's correct.

Fraser, could you please verify my reasoning behind the test cases 
for

cert-request in the patch 40?


The tests look OK.  With the default CA / default profiles, is there
appropriate isolation between test cases to ensure that if, e.g.
some other test case adds/modifies CA ACLs such that these
expected-to-fail tests now pass, that this does not affect the
TestCertSignMIMEwithSubCA test case?

Thanks,
Fraser
The ACL, SMIME CA and S/MIME profile lifetime is constrained by the 
class

scope
enforced by pytest.
The two test cases depend on the fact documented in the designs and 
that is

what
cert-request fallbacks to when CA or profile ID are not provided.
Unless something changes caIPAserviceCert profile or affiliated ACL, 
then

the test cases
are safe.


If you have thought about possible interference from other tests, I
am happy.

Note another problematic scenario: what if a different (preceding)
test adds a CA ACL that would allow the requests that you expect to
fail?  Just something to think about :)

Thanks,
Fraser
Then the failure would be problem of the preceding test and we would 
need to fix it. We are dealing with test side effects

in other parts of the execution already...

The test is constructed in a way that isolates it (to a certain 
degree) by the mechanisms available
in pytest. Of course I cannot make the test future-proof or guarantee 
that a bug in some other test
will not affect the execution of other tests as they all run against 
one IPA instance.
I do not think, however, that potential misbehaving test case that 
will interfere

should prevent us from implementing this and similar test cases.

If you have some specific issue that is in the patch, I'm happy to fix 
them.

I will try to think more about corner cases here.

[1]: http://www.freeipa.org/page/V4/Sub-CAs/Test_Plan

Cheers

--
Milan Kubik

Attaching rebased patches and removing the expected fail from one of 
the

tests as ticket 5981 has fix posted.

--
Milan Kubik






Hi Fraser,

can we continue with the review, please?

--
Milan Kubik

--
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 0014-0016][Tests] Authentication indicators

[Milan Kubík]

On 06/16/2016 03:23 PM, Lenka Doudova wrote:

Hi,

attached are tests for authentication indicators. Please note:

1. newly created service tracker is not exactly complete, list of 
unimplemented methods is in doc. These methods can be filled in when 
existing declarative tests are refactored.


2. patch 0015 depends on 0014, so it should not be pushed without it.


Lenka





patch 0014:

In the update method, what happens when the updated attributes contain 
addattr? It is not clear to me. Is it necessary?


patch 0015:

host1 and service2 do not tell anything about the purpose of the 
fixture. Please assign more descriptive names to them.
Why do the fixtures have 'function' scope? Does the service entry exist 
during the second and third test case?


patch 0016:

Per offline discussion, admin user has no special privileges here, LGTM.

--
Milan Kubik

-- 
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 661] backup: use in-server API in ipa-backup and ipa-restore

[Milan Kubík]

On 06/29/2016 02:54 PM, Jan Cholasta wrote:

Hi,

the attached patch fixes .

Honza





The restore works with the patch. ACK.


--
Milan Kubik

-- 
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 0537] CA replica promotion: add proper CA DNS records

[Milan Kubík]

On 06/28/2016 04:59 PM, Martin Basti wrote:



On 28.06.2016 16:46, Petr Spacek wrote:

On 23.6.2016 12:44, Martin Basti wrote:

patch attached.

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

ACK


Pushed to master: 5693d195501611c6abe9dbdf1370b898ffa6b3c7
Pushed to ipa-4-3: 8502fe4883d33afab57cfc4cb4695ed8061daa7e


The patch doesn't pass lint on 4.3 branch.

Pylint is running, please wait ...
* Module ipaserver.install.cainstance
ipaserver/install/cainstance.py:1632: [E0602(undefined-variable, 
__get_profile_config)] Undefined variable 'ipalib'

Makefile:137: recipe for target 'lint' failed

--
Milan Kubik

--
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] Broken pki 10.3.3-1 packages in freeipa-master COPR

[Milan Kubík]

On 06/28/2016 01:20 PM, Alexander Bokovoy wrote:

On Tue, 28 Jun 2016, Lukas Slebodnik wrote:

On (28/06/16 10:57), Alexander Bokovoy wrote:

On Tue, 28 Jun 2016, Petr Vobornik wrote:

On 06/27/2016 08:11 PM, Lukas Slebodnik wrote:
> On (27/06/16 17:55), Milan Kubík wrote:
> > Hi all,
> >
> > the pki packages that are currently in the COPR repo [1] are 
broken. There is

> > a conflict between pki-server and pki-base:
> >
> > Error: Transaction check error:
> >  file 
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyc 
from install of pki-server-10.3.3-1.fc24.noarch conflicts with file 
from package pki-base-10.3.3-1.fc24.noarch
> >  file 
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyo 
from install of pki-server-10.3.3-1.fc24.noarch conflicts with file 
from package pki-base-10.3.3-1.fc24.noarch

> >
> > [1]: 
https://copr.fedorainfracloud.org/coprs/g/freeipa/freeipa-master/

> >
> I can see the same with pki-core in fedora 24 updates-testing.
> File a fedora bug.
>
> LS
>

Right, even though I can't reproduce, the package in freeipa-master 
copr
should be the same as the one in updates testing. It was built from 
the

same srpm:
https://kojipkgs.fedoraproject.org//packages/pki-core/10.3.3/1.fc24/src/pki-core-10.3.3-1.fc24.src.rpm 


One particular issue could be that you have pki-server installed from
one source and pki-base considered from a different one. For yum and 
dnf

there is a difference where the package comes from and all subpackages
of the same source package should be coming from the same repository to
avoid conflicts like this because after install the package keeps its
source repo mark.

It is not a "particular issue".

It is a particular issue -- there may be more issues with
multi-repository package delivery but this particular bug of dnf/yum not
allowing upgrades of packages with the same name delivered via different
repositories stands out.


It's real packaging bug and have to be fixed.

Right.

The same files are owned by two packages even though one depens on 
other.

Milan, please fiel a fedora bug.

[root@5946ca9bf02b /]# rpm -q pki-server pki-base
pki-server-10.3.3-1.fc24.noarch
pki-base-10.3.3-1.fc24.noarch

[root@5946ca9bf02b /]# rpm -ql pki-base | grep pkiparser.py
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.py
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyc
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyo

[root@5946ca9bf02b /]# rpm -ql pki-server | grep pkiparser.py
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.py
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyc
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyo

This is not a real bug:
https://fedoraproject.org/wiki/Packaging:Guidelines#File_and_Directory_Ownership 



In most cases, it should not be necessary for multiple packages to
contain identical copies of the same file. However, if it is necessary,
multiple packages may contain identical copies of the same file, as long
as the following requirements are met:

   The packages sharing ownership of the identical files are built from
a single SRPM.
OR

   The packages sharing ownership of the identical files are not in a
dependency chain (e.g. if package A requires package B, they should not
both contain identical files, either A or B must own the common files,
but not both.) 
--


The bug here is that they come from different repositories and thus from
the dnf/yum point of view are built from different source packages.



[root@5946ca9bf02b /]# rpm -q --requires pki-server | grep pki
pki-base = 10.3.3-1.fc24
pki-base-java = 10.3.3-1.fc24
pki-tools = 10.3.3-1.fc24

LS


The bug is exactly the violation of the second clause. pki-server 
requires pki-base while both own the files.


--
Milan Kubik

--
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] Broken pki 10.3.3-1 packages in freeipa-master COPR

[Milan Kubík]

On 06/28/2016 01:03 PM, Lukas Slebodnik wrote:

On (28/06/16 10:57), Alexander Bokovoy wrote:

On Tue, 28 Jun 2016, Petr Vobornik wrote:

On 06/27/2016 08:11 PM, Lukas Slebodnik wrote:

On (27/06/16 17:55), Milan Kubík wrote:

Hi all,

the pki packages that are currently in the COPR repo [1] are broken. There is
a conflict between pki-server and pki-base:

Error: Transaction check error:
  file /usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyc 
from install of pki-server-10.3.3-1.fc24.noarch conflicts with file from 
package pki-base-10.3.3-1.fc24.noarch
  file /usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyo 
from install of pki-server-10.3.3-1.fc24.noarch conflicts with file from 
package pki-base-10.3.3-1.fc24.noarch

[1]: https://copr.fedorainfracloud.org/coprs/g/freeipa/freeipa-master/


I can see the same with pki-core in fedora 24 updates-testing.
File a fedora bug.

LS


Right, even though I can't reproduce, the package in freeipa-master copr
should be the same as the one in updates testing. It was built from the
same srpm:
https://kojipkgs.fedoraproject.org//packages/pki-core/10.3.3/1.fc24/src/pki-core-10.3.3-1.fc24.src.rpm

One particular issue could be that you have pki-server installed from
one source and pki-base considered from a different one. For yum and dnf
there is a difference where the package comes from and all subpackages
of the same source package should be coming from the same repository to
avoid conflicts like this because after install the package keeps its
source repo mark.

It is not a "particular issue".
It's real packaging bug and have to be fixed.

The same files are owned by two packages even though one depens on other.
Milan, please fiel a fedora bug.

[root@5946ca9bf02b /]# rpm -q pki-server pki-base
pki-server-10.3.3-1.fc24.noarch
pki-base-10.3.3-1.fc24.noarch

[root@5946ca9bf02b /]# rpm -ql pki-base | grep pkiparser.py
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.py
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyc
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyo

[root@5946ca9bf02b /]# rpm -ql pki-server | grep pkiparser.py
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.py
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyc
/usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyo

[root@5946ca9bf02b /]# rpm -q --requires pki-server | grep pki
pki-base = 10.3.3-1.fc24
pki-base-java = 10.3.3-1.fc24
pki-tools = 10.3.3-1.fc24

LS

Thanks for the rpm output, Lukas.

https://bugzilla.redhat.com/show_bug.cgi?id=1350773

--
Milan Kubik

--
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] Broken pki 10.3.3-1 packages in freeipa-master COPR

[Milan Kubík]

Hi all,

the pki packages that are currently in the COPR repo [1] are broken. 
There is a conflict between pki-server and pki-base:


Error: Transaction check error:
  file /usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyc 
from install of pki-server-10.3.3-1.fc24.noarch conflicts with file from 
package pki-base-10.3.3-1.fc24.noarch
  file /usr/lib/python2.7/site-packages/pki/server/deployment/pkiparser.pyo 
from install of pki-server-10.3.3-1.fc24.noarch conflicts with file from 
package pki-base-10.3.3-1.fc24.noarch

[1]: https://copr.fedorainfracloud.org/coprs/g/freeipa/freeipa-master/

Regards

--
Milan Kubik

-- 
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 0038-0040] Sub CA test patches

[Milan Kubík]

On 06/27/2016 02:57 AM, Fraser Tweedale wrote:

On Fri, Jun 24, 2016 at 12:08:24PM +0200, Milan Kubík wrote:

On 06/24/2016 03:42 AM, Fraser Tweedale wrote:

On Tue, Jun 21, 2016 at 05:01:35PM +0200, Milan Kubík wrote:

Hi Fraser and list,

I have made changes to the test plan on the wiki [1] according to the
information in "[Testplan review] Sub CAs" thread.

I also implemented the tests in the test plan:

patch 0038 - CATracker and CA CRUD test
patch 0039 - extension to CA ACL test
patch 0040 - functional test with ACLs and certificate profile, reusing my
previous S/MIME based tests. This patch also tests for the cert-request
behavior when profile ID or CA cn are ommited.

The tests ATM do not verify the Issuer name in the certificate itself, just
from the ipa entry of the certificate.


The approach you are using::

  assert cert_info['result']['issuer'] == smime_signing_ca.ipasubjectdn

is not quite as you describe (these are virtual attributes, not
attributes of an actual entry); but the approach is valid.

The issue then is in the wording? The other approach I could have used here
is to retrieve the two certificates and compare the fields manually.
Are these virtual attributes created from the certificate itself?


That's correct.


Fraser, could you please verify my reasoning behind the test cases for
cert-request in the patch 40?


The tests look OK.  With the default CA / default profiles, is there
appropriate isolation between test cases to ensure that if, e.g.
some other test case adds/modifies CA ACLs such that these
expected-to-fail tests now pass, that this does not affect the
TestCertSignMIMEwithSubCA test case?

Thanks,
Fraser

The ACL, SMIME CA and S/MIME profile lifetime is constrained by the class
scope
enforced by pytest.
The two test cases depend on the fact documented in the designs and that is
what
cert-request fallbacks to when CA or profile ID are not provided.
Unless something changes caIPAserviceCert profile or affiliated ACL, then
the test cases
are safe.


If you have thought about possible interference from other tests, I
am happy.

Note another problematic scenario: what if a different (preceding)
test adds a CA ACL that would allow the requests that you expect to
fail?  Just something to think about :)

Thanks,
Fraser
Then the failure would be problem of the preceding test and we would 
need to fix it. We are dealing with test side effects

in other parts of the execution already...

The test is constructed in a way that isolates it (to a certain degree) 
by the mechanisms available
in pytest. Of course I cannot make the test future-proof or guarantee 
that a bug in some other test
will not affect the execution of other tests as they all run against one 
IPA instance.
I do not think, however, that potential misbehaving test case that will 
interfere

should prevent us from implementing this and similar test cases.

If you have some specific issue that is in the patch, I'm happy to fix them.

I will try to think more about corner cases here.

[1]: http://www.freeipa.org/page/V4/Sub-CAs/Test_Plan

Cheers

--
Milan Kubik


Attaching rebased patches and removing the expected fail from one of the
tests as ticket 5981 has fix posted.

--
Milan Kubik




--
Milan Kubik

--
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 0038-0040] Sub CA test patches

[Milan Kubík]

On 06/24/2016 03:42 AM, Fraser Tweedale wrote:

On Tue, Jun 21, 2016 at 05:01:35PM +0200, Milan Kubík wrote:

Hi Fraser and list,

I have made changes to the test plan on the wiki [1] according to the
information in "[Testplan review] Sub CAs" thread.

I also implemented the tests in the test plan:

patch 0038 - CATracker and CA CRUD test
patch 0039 - extension to CA ACL test
patch 0040 - functional test with ACLs and certificate profile, reusing my
previous S/MIME based tests. This patch also tests for the cert-request
behavior when profile ID or CA cn are ommited.

The tests ATM do not verify the Issuer name in the certificate itself, just
from the ipa entry of the certificate.


The approach you are using::

 assert cert_info['result']['issuer'] == smime_signing_ca.ipasubjectdn

is not quite as you describe (these are virtual attributes, not
attributes of an actual entry); but the approach is valid.

The issue then is in the wording? The other approach I could have used here
is to retrieve the two certificates and compare the fields manually.
Are these virtual attributes created from the certificate itself?



Fraser, could you please verify my reasoning behind the test cases for
cert-request in the patch 40?


The tests look OK.  With the default CA / default profiles, is there
appropriate isolation between test cases to ensure that if, e.g.
some other test case adds/modifies CA ACLs such that these
expected-to-fail tests now pass, that this does not affect the
TestCertSignMIMEwithSubCA test case?

Thanks,
Fraser


The ACL, SMIME CA and S/MIME profile lifetime is constrained by the 
class scope

enforced by pytest.
The two test cases depend on the fact documented in the designs and that 
is what

cert-request fallbacks to when CA or profile ID are not provided.
Unless something changes caIPAserviceCert profile or affiliated ACL, 
then the test cases

are safe.

I will try to think more about corner cases here.

[1]: http://www.freeipa.org/page/V4/Sub-CAs/Test_Plan

Cheers

--
Milan Kubik

Attaching rebased patches and removing the expected fail from one of the 
tests as ticket 5981 has fix posted.


--
Milan Kubik

From 0458db3f7ff93a35531bec123e59ac71ce46f0b8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Fri, 17 Jun 2016 12:20:55 +0200
Subject: [PATCH 1/3] ipatests: Tracker implementation for Sub CA feature

The patch implements Tracker subclass for CA plugin
and the basic CRUD tests for the plugin entries.

https://fedorahosted.org/freeipa/ticket/4559
---
 ipatests/test_xmlrpc/objectclasses.py |   5 +
 ipatests/test_xmlrpc/test_ca_plugin.py| 174 ++
 ipatests/test_xmlrpc/tracker/ca_plugin.py | 126 ++
 ipatests/test_xmlrpc/xmlrpc_test.py   |   3 +
 4 files changed, 308 insertions(+)
 create mode 100644 ipatests/test_xmlrpc/test_ca_plugin.py
 create mode 100644 ipatests/test_xmlrpc/tracker/ca_plugin.py

diff --git a/ipatests/test_xmlrpc/objectclasses.py b/ipatests/test_xmlrpc/objectclasses.py
index 7050de289760ede29d057e42658c2f68d8506249..1898714513a3d64112cf2f114ca8f7e6b98ab655 100644
--- a/ipatests/test_xmlrpc/objectclasses.py
+++ b/ipatests/test_xmlrpc/objectclasses.py
@@ -221,3 +221,8 @@ caacl = [
 u'ipaassociation',
 u'ipacaacl'
 ]
+
+ca = [
+u'top',
+u'ipaca',
+]
diff --git a/ipatests/test_xmlrpc/test_ca_plugin.py b/ipatests/test_xmlrpc/test_ca_plugin.py
new file mode 100644
index ..bc2f4098d29ce3fee0bef19ccdfc80a8bd872412
--- /dev/null
+++ b/ipatests/test_xmlrpc/test_ca_plugin.py
@@ -0,0 +1,174 @@
+#
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+#
+
+"""
+Test the `ipalib.plugins.ca` module.
+"""
+
+import pytest
+
+from ipalib import errors
+from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test, fuzzy_issuer
+
+from ipatests.test_xmlrpc.tracker.certprofile_plugin import CertprofileTracker
+from ipatests.test_xmlrpc.tracker.caacl_plugin import CAACLTracker
+from ipatests.test_xmlrpc.tracker.ca_plugin import CATracker
+
+
+@pytest.fixture(scope='module')
+def default_profile(request):
+name = 'caIPAserviceCert'
+desc = u'Standard profile for network services'
+tracker = CertprofileTracker(name, store=True, desc=desc)
+tracker.track_create()
+return tracker
+
+
+@pytest.fixture(scope='module')
+def default_acl(request):
+name = u'hosts_services_caIPAserviceCert'
+tracker = CAACLTracker(name, service_category=u'all', host_category=u'all')
+tracker.track_create()
+tracker.attrs.update(
+{u'ipamembercertprofile_certprofile': [u'caIPAserviceCert']})
+return tracker
+
+
+@pytest.fixture(scope='module')
+def default_ca(request):
+name = u'ipa'
+desc = u'IPA CA'
+tracker = CATracker(name, fuzzy_issuer, desc=desc)
+tracker.track_create()
+return tracker
+
+
+@pytest.fixture(scope='class')
+def crud_subca(re

Re: [Freeipa-devel] [PATCH] 0077 Check for CA subject name collision before attempting creation

[Milan Kubík]

On 06/24/2016 09:34 AM, Fraser Tweedale wrote:

Hi,

Attached patch fixes https://fedorahosted.org/freeipa/ticket/5981.

Cheers,
Fraser

Thanks for the patch, ACK.

--
Milan Kubik

--
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] [patch 0038-0040] Sub CA test patches

[Milan Kubík]

Hi Fraser and list,

I have made changes to the test plan on the wiki [1] according to the
information in "[Testplan review] Sub CAs" thread.

I also implemented the tests in the test plan:

patch 0038 - CATracker and CA CRUD test
patch 0039 - extension to CA ACL test
patch 0040 - functional test with ACLs and certificate profile, reusing 
my previous S/MIME based tests. This patch also tests for the 
cert-request behavior when profile ID or CA cn are ommited.


The tests ATM do not verify the Issuer name in the certificate itself, 
just from the ipa entry of the certificate.


Fraser, could you please verify my reasoning behind the test cases for 
cert-request in the patch 40?


[1]: http://www.freeipa.org/page/V4/Sub-CAs/Test_Plan

Cheers

--
Milan Kubik

From 83153e3c7c4a28ba170633054b59b2d6a70807c0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Fri, 17 Jun 2016 12:20:55 +0200
Subject: [PATCH 1/3] ipatests: Tracker implementation for Sub CA feature

The patch implements Tracker subclass for CA plugin
and the basic CRUD tests for the plugin entries.

https://fedorahosted.org/freeipa/ticket/4559
---
 ipatests/test_xmlrpc/objectclasses.py |   5 +
 ipatests/test_xmlrpc/test_ca_plugin.py| 176 ++
 ipatests/test_xmlrpc/tracker/ca_plugin.py | 126 +
 ipatests/test_xmlrpc/xmlrpc_test.py   |   3 +
 4 files changed, 310 insertions(+)
 create mode 100644 ipatests/test_xmlrpc/test_ca_plugin.py
 create mode 100644 ipatests/test_xmlrpc/tracker/ca_plugin.py

diff --git a/ipatests/test_xmlrpc/objectclasses.py b/ipatests/test_xmlrpc/objectclasses.py
index 134a08803f3abca1124c4d26274d9e3fc981b941..1ea020b18f975f717eb9d9d5399d8e9fb40264cb 100644
--- a/ipatests/test_xmlrpc/objectclasses.py
+++ b/ipatests/test_xmlrpc/objectclasses.py
@@ -222,3 +222,8 @@ caacl = [
 u'ipaassociation',
 u'ipacaacl'
 ]
+
+ca = [
+u'top',
+u'ipaca',
+]
diff --git a/ipatests/test_xmlrpc/test_ca_plugin.py b/ipatests/test_xmlrpc/test_ca_plugin.py
new file mode 100644
index ..a638a0c9a5a611687d4ae707f4b067bef2ef2229
--- /dev/null
+++ b/ipatests/test_xmlrpc/test_ca_plugin.py
@@ -0,0 +1,176 @@
+#
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+#
+
+"""
+Test the `ipalib.plugins.ca` module.
+"""
+
+import pytest
+
+from ipalib import errors
+from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test, fuzzy_issuer
+
+from ipatests.test_xmlrpc.tracker.certprofile_plugin import CertprofileTracker
+from ipatests.test_xmlrpc.tracker.caacl_plugin import CAACLTracker
+from ipatests.test_xmlrpc.tracker.ca_plugin import CATracker
+
+
+@pytest.fixture(scope='module')
+def default_profile(request):
+name = 'caIPAserviceCert'
+desc = u'Standard profile for network services'
+tracker = CertprofileTracker(name, store=True, desc=desc)
+tracker.track_create()
+return tracker
+
+
+@pytest.fixture(scope='module')
+def default_acl(request):
+name = u'hosts_services_caIPAserviceCert'
+tracker = CAACLTracker(name, service_category=u'all', host_category=u'all')
+tracker.track_create()
+tracker.attrs.update(
+{u'ipamembercertprofile_certprofile': [u'caIPAserviceCert']})
+return tracker
+
+
+@pytest.fixture(scope='module')
+def default_ca(request):
+name = u'ipa'
+desc = u'IPA CA'
+tracker = CATracker(name, fuzzy_issuer, desc=desc)
+tracker.track_create()
+return tracker
+
+
+@pytest.fixture(scope='class')
+def crud_subca(request):
+name = u'crud-subca'
+subject = u'CN=crud subca test,O=crud testing inc'
+tracker = CATracker(name, subject)
+
+return tracker.make_fixture(request)
+
+
+@pytest.fixture(scope='class')
+def subject_conflict_subca(request):
+name = u'crud-subca-2'
+subject = u'CN=crud subca test,O=crud testing inc'
+tracker = CATracker(name, subject)
+
+return tracker.make_fixture(request)
+
+
+@pytest.mark.tier0
+class TestDefaultCA(XMLRPC_test):
+def test_default_ca_present(self, default_ca):
+default_ca.retrieve()
+
+def test_default_ca_delete(self, default_ca):
+with pytest.raises(errors.ProtectedEntryError):
+default_ca.delete()
+
+
+@pytest.mark.tier1
+class TestCAbasicCRUD(XMLRPC_test):
+
+ATTR_ERROR_MSG = u'attribute is not configurable'
+
+def test_create(self, crud_subca):
+crud_subca.create()
+
+def test_retrieve(self, crud_subca):
+crud_subca.retrieve()
+
+def test_retrieve_all(self, crud_subca):
+crud_subca.retrieve(all=True)
+
+def test_delete(self, crud_subca):
+crud_subca.delete()
+
+def test_find(self, crud_subca):
+crud_subca.ensure_exists()
+crud_subca.find()
+
+def test_modify_description(self, crud_subca):
+new_desc = u'updated CA description'
+crud_subca.update(
+dict(
+description=new_desc,
+),
+

[Freeipa-devel] [Testplan review] Sub CAs

[Milan Kubík]

Hi Fraser and list,

I've wrote a (minimal) draft [1] of the test plan for the Sub CAs feature
and I also have several questions.

Could you please take a look at it?

Questions:

As described in the last (currently) test case, should it be possible to 
specify

both the CA and certificate profile in cert-request call?
This way one could use (at least) two ACLs (one affiliated with CA, one 
with a profile).

Are there such use cases?

Related to this, what happens when CA ACL has specific CA and profile 
category (all)?

Applicable to other combinations as well. The ACL category semantics is
a bit unclear for me here.

Is there any validation of the CA's DN (syntax)?

How would you approach testing of the Sub CA certificate renewal and key 
replication
(I do not know if this is covered at the respective component's level or 
not)?



[1]: http://www.freeipa.org/page/V4/Sub-CAs/Test_Plan

Thanks

--
Milan Kubik

--
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] [DESIGN] Kerberos principal alias handling

[Milan Kubík]

On 04/08/2016 05:10 PM, Martin Babinsky wrote:

Hi list,

I have put together a draft [1] outlining the effort to reimplement 
the handling of Kerberos principals in both backend and frontend 
layers of FreeIPA so that we may have multiple aliases per user, host 
or service and thus implement stuff like 
https://fedorahosted.org/freeipa/ticket/3961 and 
https://fedorahosted.org/freeipa/ticket/5413 .


Since much of the plumbing was already implemented,[2] the document 
mainly describes what the patches do. Some parts required by other use 
cases may be missing so please point these out.


I would also be happy if you could correct all factual inacurracies, I 
did research on this issue a long time ago and my knowledge turned a 
bit rusty.


[1] http://www.freeipa.org/page/V4/Kerberos_principal_aliases
[2] 
https://www.redhat.com/archives/freeipa-devel/2015-October/msg00048.html




Hi!

I went through the design document and the related email thread here on 
the list and I have few questions:


1. Is there any progress on what's to happen if MODRDN would colide with 
an existing alias on a different entry?


2. How does this RFE change the behavior of stage user plugin? Is the 
principal (as in the canonical name) assigned at this stage of user 
lifetime?


3. Will there be any constraints on what string can be used as an alias? 
(The document mentions email address as one use case)


4. Will this RFE have any impact on AD trust (possibility of cross realm 
routing, RFC 6806 section 9)


Otherwise the document looks good from my POV as QE.


Regards,

--
Milan Kubik

-- 
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 0037] spec: Add python-sssdconfig dependency for python-ipatests package

[Milan Kubík]

On 04/25/2016 09:36 AM, Martin Babinsky wrote:

On 04/23/2016 12:13 AM, Milan Kubík wrote:

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

Applies to ipa-4-3, master

--
Milan Kubik





Hi Milan,

the dependency on python-sssdconfig was introduced by my fix to 
https://fedorahosted.org/freeipa/ticket/5625 (I know, shame) which 
also landed in ipa-4-2, so you'll need to send a patch also for this 
branch (the current one obviously does not apply).


Also in ipa-4-3 and master patch you should require python3-sssdconfig 
for python3-ipatests.



Good catch, thanks.
Fixed in new patches.

--
Milan Kubik

From a93217ae0b4db941622ac28875b10d7a37cfc520 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Fri, 22 Apr 2016 23:43:47 +0200
Subject: [PATCH] spec: Add python-sssdconfig dependency for python-ipatests
 package

https://fedorahosted.org/freeipa/ticket/5843
---
 freeipa.spec.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index aaa40cc9a2246ed1d244e160edf935da216c75c5..03ebe29b56dff2f874a33f190cce5598a3b2b430 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -603,6 +603,7 @@ Requires: python2-polib
 Requires: python-pytest-multihost >= 0.5
 Requires: python-pytest-sourceorder
 Requires: ldns-utils
+Requires: python-sssdconfig
 
 Provides: %{alt_name}-tests = %{version}
 Conflicts: %{alt_name}-tests
@@ -634,6 +635,7 @@ Requires: python3-polib
 Requires: python3-pytest-multihost >= 0.5
 Requires: python3-pytest-sourceorder
 Requires: ldns-utils
+Requires: python3-sssdconfig
 
 %description -n python3-ipatests
 IPA is an integrated solution to provide centrally managed Identity (users,
-- 
2.8.0

From 3d34ca333ec17967b2ccc70726767400beb613bb Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Mon, 25 Apr 2016 09:52:00 +0200
Subject: [PATCH] spec: Add python-sssdconfig dependency for freeipa-tests
 package

https://fedorahosted.org/freeipa/ticket/5843
---
 freeipa.spec.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 78eedb86d21fb86c0d63381f89c6f694c40da7b6..52c127ed030d3b902aea2a52f699d41a9b9bafdc 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -351,6 +351,7 @@ Requires: python-coverage
 Requires: python-polib
 Requires: python-pytest-multihost >= 0.5
 Requires: python-pytest-sourceorder
+Requires: python-sssdconfig
 
 Conflicts: %{alt_name}-tests
 Obsoletes: %{alt_name}-tests < %{version}
-- 
2.8.0

-- 
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] [patch 0037] spec: Add python-sssdconfig dependency for python-ipatests package

[Milan Kubík]

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

Applies to ipa-4-3, master

--
Milan Kubik

From cb26da230ae358c1d1768d82c3be8e75bb2159d3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Fri, 22 Apr 2016 23:43:47 +0200
Subject: [PATCH] spec: Add python-sssdconfig dependency for python-ipatests
 package

https://fedorahosted.org/freeipa/ticket/5843
---
 freeipa.spec.in | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index aaa40cc9a2246ed1d244e160edf935da216c75c5..1bfe2cfedb9cfb3fd799205de032653b746a4e69 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -603,6 +603,7 @@ Requires: python2-polib
 Requires: python-pytest-multihost >= 0.5
 Requires: python-pytest-sourceorder
 Requires: ldns-utils
+Requires: python-sssdconfig
 
 Provides: %{alt_name}-tests = %{version}
 Conflicts: %{alt_name}-tests
@@ -634,6 +635,7 @@ Requires: python3-polib
 Requires: python3-pytest-multihost >= 0.5
 Requires: python3-pytest-sourceorder
 Requires: ldns-utils
+Requires: python-sssdconfig
 
 %description -n python3-ipatests
 IPA is an integrated solution to provide centrally managed Identity (users,
-- 
2.8.0

-- 
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 0035] ipatests: Add test case for requesting a certificate with full principal.

[Milan Kubík]

On 04/21/2016 03:29 PM, Martin Babinsky wrote:

On 04/21/2016 03:25 PM, Martin Babinsky wrote:

On 04/21/2016 11:24 AM, Milan Kubík wrote:

On 04/05/2016 12:07 PM, Martin Babinsky wrote:

On 04/05/2016 10:24 AM, Milan Kubík wrote:

On 04/05/2016 10:17 AM, Milan Kubík wrote:

On 04/05/2016 09:31 AM, Martin Babinsky wrote:

On 04/01/2016 12:02 PM, Milan Kubík wrote:


Patches attached.



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








Hi Milan,



I would be more happy if you could send a separate patch for the
context
manager fix, since the issue is orthogonal to the added test case
(even
if the test suite explodes without it).



Otherwise LGTM.







Done. Patch 0035 now applies to all branches, context manager fix
needs separate patch for ipa-4-2.


Updated commit message in patches 0036 to include the ticket.


Thanks, ACK.


Add freeipa-devel back to the loop & push request :)

--
Milan Kubik


Ah sorry I forgot how to mailing list.

ACK again for our push-bot (aka mbasti)



I see that the fix for the failing test was already pushed so you can 
remove the xfail mark from the test and it should be all green now.


Sorry for the confusion.



I haven't noticed, sorry. Updated patch attached.


--
Milan Kubik

From cd49ec790918c7b006afe9ee542ff055739b0b74 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Tue, 5 Apr 2016 10:04:03 +0200
Subject: [PATCH] ipatests: Add test case for requesting a certificate with
 full principal.

https://fedorahosted.org/freeipa/ticket/5733
---
 ipatests/test_xmlrpc/test_caacl_profile_enforcement.py | 7 +++
 1 file changed, 7 insertions(+)

diff --git a/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py b/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
index dca4151d614a4c2e2f5a09455426d117da4c1c80..11c040966003e2ea86149359c8aacb953e9fdd37 100644
--- a/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
+++ b/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
@@ -130,6 +130,13 @@ class TestCertSignMIME(XMLRPC_test):
 api.Command.cert_request(csr, principal=smime_user,
  profile_id=smime_profile.name)
 
+def test_sign_smime_csr_full_principal(self, smime_profile, smime_user):
+csr = generate_user_csr(smime_user)
+smime_user_principal = '@'.join((smime_user, api.env.realm))
+with change_principal(smime_user, SMIME_USER_PW):
+api.Command.cert_request(csr, principal=smime_user_principal,
+ profile_id=smime_profile.name)
+
 
 @pytest.mark.tier1
 class TestSignWithDisabledACL(XMLRPC_test):
-- 
2.8.0

-- 
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 0035] ipatests: Add test case for requesting a certificate with full principal.

[Milan Kubík]

On 04/05/2016 12:07 PM, Martin Babinsky wrote:

On 04/05/2016 10:24 AM, Milan Kubík wrote:

On 04/05/2016 10:17 AM, Milan Kubík wrote:

On 04/05/2016 09:31 AM, Martin Babinsky wrote:

On 04/01/2016 12:02 PM, Milan Kubík wrote:


Patches attached.



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








Hi Milan,



I would be more happy if you could send a separate patch for the 
context
manager fix, since the issue is orthogonal to the added test case 
(even

if the test suite explodes without it).



Otherwise LGTM.







Done. Patch 0035 now applies to all branches, context manager fix
needs separate patch for ipa-4-2.


Updated commit message in patches 0036 to include the ticket.


Thanks, ACK.


Add freeipa-devel back to the loop & push request :)

--
Milan Kubik

-- 
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 0035] ipatests: Add test case for requesting a certificate with full principal.

[Milan Kubík]

On 04/05/2016 10:17 AM, Milan Kubík wrote:

On 04/05/2016 09:31 AM, Martin Babinsky wrote:

On 04/01/2016 12:02 PM, Milan Kubík wrote:


Patches attached.



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








Hi Milan,



I would be more happy if you could send a separate patch for the context
manager fix, since the issue is orthogonal to the added test case (even
if the test suite explodes without it).



Otherwise LGTM.







Done. Patch 0035 now applies to all branches, context manager fix 
needs separate patch for ipa-4-2.



Updated commit message in patches 0036 to include the ticket.

--
Milan Kubik

From eebad5ad31107f3383b4b2755a97929a75170d6f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Tue, 5 Apr 2016 10:04:03 +0200
Subject: [PATCH 1/2] ipatests: Add test case for requesting a certificate with
 full principal.

https://fedorahosted.org/freeipa/ticket/5733
---
 ipatests/test_xmlrpc/test_caacl_profile_enforcement.py | 8 
 1 file changed, 8 insertions(+)

diff --git a/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py b/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
index dca4151d614a4c2e2f5a09455426d117da4c1c80..a0b8d614cf6dd42b18eb03100a318e4a3fbfb4e0 100644
--- a/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
+++ b/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
@@ -130,6 +130,14 @@ class TestCertSignMIME(XMLRPC_test):
 api.Command.cert_request(csr, principal=smime_user,
  profile_id=smime_profile.name)
 
+@pytest.mark.xfail(strict=True, reason='freeipa ticket 5733')
+def test_sign_smime_csr_full_principal(self, smime_profile, smime_user):
+csr = generate_user_csr(smime_user)
+smime_user_principal = '@'.join((smime_user, api.env.realm))
+with change_principal(smime_user, SMIME_USER_PW):
+api.Command.cert_request(csr, principal=smime_user_principal,
+ profile_id=smime_profile.name)
+
 
 @pytest.mark.tier1
 class TestSignWithDisabledACL(XMLRPC_test):
-- 
2.8.0

From b103f0165db3536db0c3bd02aec82961e631c08b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Tue, 5 Apr 2016 10:04:37 +0200
Subject: [PATCH] ipatests: fix for change_principal context manager

The context manager was leaving API object disconnected when
an exception was raised inside of it. This led to resource leak
in the tests.

https://fedorahosted.org/freeipa/ticket/5733
---
 ipatests/util.py | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/ipatests/util.py b/ipatests/util.py
index 6aefe74d34fd7b1bd063c4b17c98af4840d6f042..118c47a12e0d97907cb559d716989a9ca6c5f304 100644
--- a/ipatests/util.py
+++ b/ipatests/util.py
@@ -696,17 +696,18 @@ def change_principal(user, password, client=None, path=None):
 
 client.Backend.rpcclient.disconnect()
 
-with private_ccache(ccache_name):
-kinit_password(user, password, ccache_name)
+try:
+with private_ccache(ccache_name):
+kinit_password(user, password, ccache_name)
+client.Backend.rpcclient.connect()
+
+try:
+yield
+finally:
+client.Backend.rpcclient.disconnect()
+finally:
 client.Backend.rpcclient.connect()
 
-try:
-yield
-finally:
-client.Backend.rpcclient.disconnect()
-
-client.Backend.rpcclient.connect()
-
 def get_group_dn(cn):
 return DN(('cn', cn), api.env.container_group, api.env.basedn)
 
-- 
2.8.0

From b15dc58c9b810cdff02438cb78c89240c9eb5416 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Tue, 5 Apr 2016 10:04:37 +0200
Subject: [PATCH] ipatests: fix for change_principal context manager

The context manager was leaving API object disconnected when
an exception was raised inside of it. This led to resource leak
in the tests.

https://fedorahosted.org/freeipa/ticket/5733
---
 ipatests/util.py | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/ipatests/util.py b/ipatests/util.py
index 4d99ff6e0a505cd3f75053f97caca9edbc802bcf..56b731407b3544b3b922f1831df4bc59845486d1 100644
--- a/ipatests/util.py
+++ b/ipatests/util.py
@@ -687,13 +687,14 @@ def change_principal(user, password, client=None, path=None):
 
 client.Backend.rpcclient.disconnect()
 
-with private_ccache(ccache_name):
-kinit_password(user, password, ccache_name)
+try:
+with private_ccache(ccache_name):
+kinit_password(user, password, ccache_name)
+client.Backend.rpcclient.connect()
+
+try:
+yield
+finally:
+client.Backend.rpcclient.disconnect()
+finally:
 client.Backend.rpcclient.connect()
-
-try:
-yiel

Re: [Freeipa-devel] [patch 0035] ipatests: Add test case for requesting a certificate with full principal.

[Milan Kubík]

On 04/05/2016 09:31 AM, Martin Babinsky wrote:

On 04/01/2016 12:02 PM, Milan Kubík wrote:


Patches attached.



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








Hi Milan,



I would be more happy if you could send a separate patch for the context
manager fix, since the issue is orthogonal to the added test case (even
if the test suite explodes without it).



Otherwise LGTM.







Done. Patch 0035 now applies to all branches, context manager fix needs 
separate patch for ipa-4-2.


--
Milan Kubik

From eebad5ad31107f3383b4b2755a97929a75170d6f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Tue, 5 Apr 2016 10:04:03 +0200
Subject: [PATCH 1/2] ipatests: Add test case for requesting a certificate with
 full principal.

https://fedorahosted.org/freeipa/ticket/5733
---
 ipatests/test_xmlrpc/test_caacl_profile_enforcement.py | 8 
 1 file changed, 8 insertions(+)

diff --git a/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py b/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
index dca4151d614a4c2e2f5a09455426d117da4c1c80..a0b8d614cf6dd42b18eb03100a318e4a3fbfb4e0 100644
--- a/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
+++ b/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
@@ -130,6 +130,14 @@ class TestCertSignMIME(XMLRPC_test):
 api.Command.cert_request(csr, principal=smime_user,
  profile_id=smime_profile.name)
 
+@pytest.mark.xfail(strict=True, reason='freeipa ticket 5733')
+def test_sign_smime_csr_full_principal(self, smime_profile, smime_user):
+csr = generate_user_csr(smime_user)
+smime_user_principal = '@'.join((smime_user, api.env.realm))
+with change_principal(smime_user, SMIME_USER_PW):
+api.Command.cert_request(csr, principal=smime_user_principal,
+ profile_id=smime_profile.name)
+
 
 @pytest.mark.tier1
 class TestSignWithDisabledACL(XMLRPC_test):
-- 
2.8.0

From a9c7fe8b5a2f477e5dd6e70496e878c373183747 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Tue, 5 Apr 2016 10:04:37 +0200
Subject: [PATCH 2/2] ipatests: fix for change_principal context manager

The context manager was leaving API object disconnected when
an exception was raised inside of it. This led to resource leak
in the tests.
---
 ipatests/util.py | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/ipatests/util.py b/ipatests/util.py
index 6aefe74d34fd7b1bd063c4b17c98af4840d6f042..118c47a12e0d97907cb559d716989a9ca6c5f304 100644
--- a/ipatests/util.py
+++ b/ipatests/util.py
@@ -696,17 +696,18 @@ def change_principal(user, password, client=None, path=None):
 
 client.Backend.rpcclient.disconnect()
 
-with private_ccache(ccache_name):
-kinit_password(user, password, ccache_name)
+try:
+with private_ccache(ccache_name):
+kinit_password(user, password, ccache_name)
+client.Backend.rpcclient.connect()
+
+try:
+yield
+finally:
+client.Backend.rpcclient.disconnect()
+finally:
 client.Backend.rpcclient.connect()
 
-try:
-yield
-finally:
-client.Backend.rpcclient.disconnect()
-
-client.Backend.rpcclient.connect()
-
 def get_group_dn(cn):
 return DN(('cn', cn), api.env.container_group, api.env.basedn)
 
-- 
2.8.0

From 3923b51494cd5336a5a9cd9c2120d6d639f6b9ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Tue, 5 Apr 2016 10:04:37 +0200
Subject: [PATCH] ipatests: fix for change_principal context manager

The context manager was leaving API object disconnected when
an exception was raised inside of it. This led to resource leak
in the tests.
---
 ipatests/util.py | 19 ++-
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/ipatests/util.py b/ipatests/util.py
index 4d99ff6e0a505cd3f75053f97caca9edbc802bcf..56b731407b3544b3b922f1831df4bc59845486d1 100644
--- a/ipatests/util.py
+++ b/ipatests/util.py
@@ -687,13 +687,14 @@ def change_principal(user, password, client=None, path=None):
 
 client.Backend.rpcclient.disconnect()
 
-with private_ccache(ccache_name):
-kinit_password(user, password, ccache_name)
+try:
+with private_ccache(ccache_name):
+kinit_password(user, password, ccache_name)
+client.Backend.rpcclient.connect()
+
+try:
+yield
+finally:
+client.Backend.rpcclient.disconnect()
+finally:
 client.Backend.rpcclient.connect()
-
-try:
-yield
-finally:
-client.Backend.rpcclient.disconnect()
-
-client.Backend.rpcclient.connect()
-- 
2.8.0

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

[Freeipa-devel] [patch 0035] ipatests: Add test case for requesting a certificate with full principal.

[Milan Kubík]

Patches attached.

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

--
Milan Kubik

From 985814ef076a828ac59aeafd0598d87983edc809 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Fri, 1 Apr 2016 11:11:54 +0200
Subject: [PATCH] ipatests: Add test case for requesting a certificate with
 full principal.

Also fixes an issue in change_principal context manager that caused
a resource leak on test case failure.

https://fedorahosted.org/freeipa/ticket/5733
---
 .../test_xmlrpc/test_caacl_profile_enforcement.py |  8 
 ipatests/util.py  | 19 ++-
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py b/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
index dca4151d614a4c2e2f5a09455426d117da4c1c80..a0b8d614cf6dd42b18eb03100a318e4a3fbfb4e0 100644
--- a/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
+++ b/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
@@ -130,6 +130,14 @@ class TestCertSignMIME(XMLRPC_test):
 api.Command.cert_request(csr, principal=smime_user,
  profile_id=smime_profile.name)
 
+@pytest.mark.xfail(strict=True, reason='freeipa ticket 5733')
+def test_sign_smime_csr_full_principal(self, smime_profile, smime_user):
+csr = generate_user_csr(smime_user)
+smime_user_principal = '@'.join((smime_user, api.env.realm))
+with change_principal(smime_user, SMIME_USER_PW):
+api.Command.cert_request(csr, principal=smime_user_principal,
+ profile_id=smime_profile.name)
+
 
 @pytest.mark.tier1
 class TestSignWithDisabledACL(XMLRPC_test):
diff --git a/ipatests/util.py b/ipatests/util.py
index 6aefe74d34fd7b1bd063c4b17c98af4840d6f042..118c47a12e0d97907cb559d716989a9ca6c5f304 100644
--- a/ipatests/util.py
+++ b/ipatests/util.py
@@ -696,17 +696,18 @@ def change_principal(user, password, client=None, path=None):
 
 client.Backend.rpcclient.disconnect()
 
-with private_ccache(ccache_name):
-kinit_password(user, password, ccache_name)
+try:
+with private_ccache(ccache_name):
+kinit_password(user, password, ccache_name)
+client.Backend.rpcclient.connect()
+
+try:
+yield
+finally:
+client.Backend.rpcclient.disconnect()
+finally:
 client.Backend.rpcclient.connect()
 
-try:
-yield
-finally:
-client.Backend.rpcclient.disconnect()
-
-client.Backend.rpcclient.connect()
-
 def get_group_dn(cn):
 return DN(('cn', cn), api.env.container_group, api.env.basedn)
 
-- 
2.8.0

From f3cb98f26551d342968281fb01d288e10cda85de Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Fri, 1 Apr 2016 11:11:54 +0200
Subject: [PATCH] ipatests: Add test case for requesting a certificate with
 full principal.

Also fixes an issue in change_principal context manager that caused
a resource leak on test case failure.

https://fedorahosted.org/freeipa/ticket/5733
---
 .../test_xmlrpc/test_caacl_profile_enforcement.py |  8 
 ipatests/util.py  | 19 ++-
 2 files changed, 18 insertions(+), 9 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py b/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
index 78262ae8c17716633ac33fcc8114f6b549066a42..98165c4919e719e72fd7a4aec977f12dacd79249 100644
--- a/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
+++ b/ipatests/test_xmlrpc/test_caacl_profile_enforcement.py
@@ -125,6 +125,14 @@ class TestCertSignMIME(XMLRPC_test):
 api.Command.cert_request(csr, principal=smime_user,
  profile_id=smime_profile.name)
 
+@pytest.mark.xfail(strict=True, reason='freeipa ticket 5733')
+def test_sign_smime_csr_full_principal(self, smime_profile, smime_user):
+csr = generate_user_csr(smime_user)
+smime_user_principal = '@'.join((smime_user, api.env.realm))
+with change_principal(smime_user, SMIME_USER_PW):
+api.Command.cert_request(csr, principal=smime_user_principal,
+ profile_id=smime_profile.name)
+
 
 @pytest.mark.tier1
 class TestSignWithDisabledACL(XMLRPC_test):
diff --git a/ipatests/util.py b/ipatests/util.py
index 4d99ff6e0a505cd3f75053f97caca9edbc802bcf..a3c4889c6fd0026bf5caa655170f785f571e09f5 100644
--- a/ipatests/util.py
+++ b/ipatests/util.py
@@ -687,13 +687,14 @@ def change_principal(user, password, client=None, path=None):
 
 client.Backend.rpcclient.disconnect()
 
-with private_ccache(ccache_name):
-kinit_password(user, password, ccache_name)
+try:
+with private_ccache(ccache_name):
+kinit_password(user, password, ccache_name)
+

Re: [Freeipa-devel] [PATCH 0006] Refactor test_hostgroup_plugin

[Milan Kubík]

On 03/07/2016 02:53 PM, Filip Škola wrote:

Sorry, forgot to cc you, Milan.

F.

On Tue, 22 Dec 2015 05:57:50 -0500 (EST)
Filip Skola  wrote:


And also sending refactored hostgroup plugin test.

F

Sorry for long delay. ACK.

--
Milan Kubik

--
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 0005] Refactor test_nesting, create HostGroupTracker

[Milan Kubík]

On 03/11/2016 03:42 PM, Filip Skola wrote:


- Original Message -

On 01/28/2016 10:45 AM, Filip Skola wrote:

The same as with patch 0002:
* Module ipatests.test_xmlrpc.tracker.hostgroup_plugin
W:142,26: Calling a dict.iter*() method (dict-iter-method)

Please use dict.items method.

--
Milan Kubik



Hi,

attaching a fixed patch. This patch is dependent on updated group plugin
test patch 0002-7.

Filip

Hello, sorry for delay. The patch no longer applies after 0002-8.

--
Milan Kubik



Hi,

rebased. The next patch should be applicable on top of this one.

Filip

Sorry for long delays. ACK.

--
Milan Kubik

--
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 0143-0144] different errors/warnings for different LDAP limit type exceeded

[Milan Kubík]

On 03/18/2016 10:28 AM, Martin Babinsky wrote:
These patches implement behavior agreed upon during discussion of 
https://fedorahosted.org/freeipa/ticket/5677


However I'm not sure if we want to push them into 4-3 branch (the 
ticket is triaged into 4.3.2 milestone) since they modify the 
framework behavior quite a bit.


If there is no need to have it there (CC'ing Milan since he is the 
reporter), I would retriage it into 4.4 milestone.


I'm OK with the patch being only in master/ipa-4-4. Assuming the problem 
in the limits exceeded cases originates in the backend (be it directory 
server or dogtag), we can compare the problem in a run on master and an 
older branch. They will likely have the same cause.


--
Milan Kubik

--
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 0002] Refactor test_group_plugin

[Milan Kubík]

On 02/22/2016 11:09 AM, Filip Skola wrote:


- Original Message -

On 02/10/2016 09:17 AM, Milan Kubík wrote:

On 02/09/2016 04:19 PM, Milan Kubík wrote:

On 01/28/2016 10:42 AM, Filip Skola wrote:

- Original Message -

On 01/25/2016 11:11 AM, Filip Skola wrote:

- Original Message -

On 01/15/2016 03:38 PM, Filip Skola wrote:

Hi,

sending rebased patch.

F.

- Original Message -

Hello,

sorry for delays. The patch no longer applies to master. Rebase
it,
please.

Milan

- Original Message -
From: "Filip Škola" <fsk...@redhat.com>
To: "Milan Kubík" <mku...@redhat.com>
Cc: freeipa-devel@redhat.com
Sent: Wednesday, 9 December, 2015 7:01:02 PM
Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor
test_group_plugin

On Mon, 7 Dec 2015 17:49:18 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 12/03/2015 08:15 PM, Filip Škola wrote:

On Mon, 30 Nov 2015 17:18:30 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 11/23/2015 04:42 PM, Filip Škola wrote:

Sending updated patch.

F.

On Mon, 23 Nov 2015 14:59:34 +0100
Filip Škola <fsk...@redhat.com> wrote:


Found couple of issues (broke some dependencies).

NACK

F.

On Fri, 20 Nov 2015 13:56:36 +0100
Filip Škola <fsk...@redhat.com> wrote:


Another one.

F.

Hi, the tests look good. Few remarks, though.

1. Please, use the shortes copyright notice in new modules.

  #
  # Copyright (C) 2015  FreeIPA Contributors see
COPYING for
license #

2. The tests `test_group_remove_group_from_protected_group` and
`test_group_full_set_of_objectclass_not_available_post_detach`
were not ported. Please, include them in the patch.

Also, for less hassle, please rebase your patches on top of
freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch

Which changes the location of tracker implementations and
prevents
circular imports.

Thanks.


Hi,

these cases are there, in corresponding classes. They are marked
with the original comments. (However I can move them to separate
class if desirable.)

The copyright notice is changed. Also included a few changes
in the
test with user without private group.

Filip

NACK

linter:
* Module tracker.group_plugin
ipatests/test_xmlrpc/tracker/group_plugin.py:257:
[E0102(function-redefined), GroupTracker.check_remove_member]
method
already defined line 253)

Probably a leftover after the rebase made on top of my patch.
Please
fix it. You can check youch changes by make-lint script before
sending them.

Thanks


Hi,

I learned to use make-lint!

Thanks,
F.


Hello,

NACK, pylint doesn't seem to like the way the fixtures are imported
(pytest does a lot of runtime magic) [1].
One possible solution would be [2]. Though, I don't think this
would be
a good idea in our environment. I suggest to create the fixtures
on per
module basis.


[1]: http://fpaste.org/311949/53118942/
[2]:
https://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects


--
Milan Kubik



Hi,

the fixtures were copied into corresponding module. Please note
that this
patch has a dependence on my patch 0001 (user plugin).

Filip

Linter:
* Module ipatests.test_xmlrpc.tracker.group_plugin
W:100,26: Calling a dict.iter*() method (dict-iter-method)

please use dict.items

--
Milan Kubik



Hi, sorry. This has been fixed in this patch.

Filip

ACK, thanks for the patience. :)


Sorry, there are some other things I need clarified. NACK.
Mail will follow later.


What is the purpose of `make_fixture_detach` in your patches? They are
not used anywhere and the finalizer does nothing.

--
Milan Kubik



Hi,

none, I guess, probably a leftover copied from the tracker in the early days. 
Deleting the function.

Filip

Ack.

--
Milan Kubik

--
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 0005] Refactor test_nesting, create HostGroupTracker

[Milan Kubík]

On 01/28/2016 10:45 AM, Filip Skola wrote:



The same as with patch 0002:
* Module ipatests.test_xmlrpc.tracker.hostgroup_plugin
W:142,26: Calling a dict.iter*() method (dict-iter-method)

Please use dict.items method.

--
Milan Kubik



Hi,

attaching a fixed patch. This patch is dependent on updated group plugin test 
patch 0002-7.

Filip

Hello, sorry for delay. The patch no longer applies after 0002-8.

--
Milan Kubik

--
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 0422] CI: allow customized DS install test to run under different domain levels

[Milan Kubík]

On 03/02/2016 04:50 PM, Martin Basti wrote:



On 24.02.2016 19:01, Martin Basti wrote:

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

Patch attached.



Bump for review



Works for me, ACK.

--
Milan Kubik

-- 
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] [TEST][Patch 0022] small refactoring in integration tests due to BZ 1303095

[Milan Kubík]

On 02/19/2016 02:11 PM, Oleg Fayans wrote:

Hi Milan,

On 02/12/2016 04:03 PM, Milan Kubík wrote:



Agreed. The latest patch gets rid of all resolv.conf related
manipulations. The tests work (where not affected by
https://fedorahosted.org/bind-dyndb-ldap/ticket/160)



--
Milan Kubik


Works for me, tested on sudo test that requires autodiscovery. ACK

--
Milan Kubik

--
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 0033] spec file: update the python-polib dependency name to python2-polib

[Milan Kubík]

On 02/25/2016 11:07 AM, Jan Cholasta wrote:

On 25.2.2016 11:03, Milan Kubík wrote:

On 02/15/2016 05:39 PM, Lukas Slebodnik wrote:

On (15/02/16 17:00), Petr Vobornik wrote:

On 02/15/2016 04:37 PM, Milan Kubík wrote:

Reflect the updated name of the package.


Seems to me as a packaging bug in python-polib. It should use
python_provide
macro to handle the transition.

There is not a bug in python-polib

sh# rpm -q python2-polib
python2-polib-1.0.7-2.fc23.noarch

sh# rpm -q --provides python2-polib
python-polib = 1.0.7-2.fc23
python2-polib = 1.0.7-2.fc23

However it is a change in behaviour in dnf/yum.
You can see more details in BZ1291850 or better BZ1096506.

This a readon why "dnf builddep" will try to remove package.
(it's not downgrade from dnf point of view)

sh# dnf builddep freeipa.spec
Last metadata expiration check performed 0:17:37 ago on Mon Feb 15
16:19:14
2016.
Package python-setuptools-18.0.1-2.fc23.noarch is already installed,
skipping.
Package systemd-222-10.fc23.x86_64 is already installed, skipping.
Package systemd-222-10.fc23.x86_64 is already installed, skipping.
Error: installed package python2-polib-1.0.7-2.fc23.noarch obsoletes
python-polib < 1.0.7-2.fc23 provided by 
python-polib-1.0.3-6.fc23.noarch

(try to add '--allowerasing' to command line to replace conflicting
packages)


You might try to file a dnf BZ but mine 1291850 was two tiles closed
as not a
but and then closed as a duplicate of another bug.

IMHO the simplest solution would to push the patch with better
explanation
in's a workaround.

LSommit message becuase it's a workaround.

LS

Updated patch with reworded commit message.


Please also add "workaround for 
https://bugzilla.redhat.com/show_bug.cgi?id=1096506; comment above the 
changed requires.



Done.

--
Milan Kubik

From 591eb0a92f79f234307cb3b7f4407bb1aa857ff5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Mon, 15 Feb 2016 15:54:40 +0100
Subject: [PATCH] spec file: rename the python-polib dependency name to
 python2-polib

Trying to install the package depending on python-polib breaks
when the system has newer (and renamed) version python2-polib.

*This patch is an workaround* for the issue described in [1].
If a renamed package's provides is equal to an older package's
name, dnf tries to install the older package.
When the newer package is in the system, this leads to a conflict.

[1]: https://bugzilla.redhat.com/show_bug.cgi?id=1096506
---
 freeipa.spec.in | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 48fec974246dbc2933dd172318157f3e0e050a3b..c8fd9db8fcdc14313eab724514b4f4a7f6095a37 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -77,7 +77,8 @@ BuildRequires:  python-gssapi >= 1.1.2
 BuildRequires:  python-rhsm
 BuildRequires:  pyOpenSSL
 BuildRequires:  pylint >= 1.0
-BuildRequires:  python-polib
+# workaround for https://bugzilla.redhat.com/show_bug.cgi?id=1096506
+BuildRequires:  python2-polib
 BuildRequires:  python-libipa_hbac
 BuildRequires:  python-memcached
 BuildRequires:  python-lxml
@@ -562,7 +563,8 @@ Requires: python-nose
 Requires: pytest >= 2.6
 Requires: python-paste
 Requires: python-coverage
-Requires: python-polib
+# workaround for https://bugzilla.redhat.com/show_bug.cgi?id=1096506
+Requires: python2-polib
 Requires: python-pytest-multihost >= 0.5
 Requires: python-pytest-sourceorder
 Requires: ldns-utils
-- 
2.7.1

-- 
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 0033] spec file: update the python-polib dependency name to python2-polib

[Milan Kubík]

On 02/15/2016 05:39 PM, Lukas Slebodnik wrote:

On (15/02/16 17:00), Petr Vobornik wrote:

On 02/15/2016 04:37 PM, Milan Kubík wrote:

Reflect the updated name of the package.


Seems to me as a packaging bug in python-polib. It should use python_provide
macro to handle the transition.

There is not a bug in python-polib

sh# rpm -q python2-polib
python2-polib-1.0.7-2.fc23.noarch

sh# rpm -q --provides python2-polib
python-polib = 1.0.7-2.fc23
python2-polib = 1.0.7-2.fc23

However it is a change in behaviour in dnf/yum.
You can see more details in BZ1291850 or better BZ1096506.

This a readon why "dnf builddep" will try to remove package.
(it's not downgrade from dnf point of view)

sh# dnf builddep freeipa.spec
Last metadata expiration check performed 0:17:37 ago on Mon Feb 15 16:19:14
2016.
Package python-setuptools-18.0.1-2.fc23.noarch is already installed, skipping.
Package systemd-222-10.fc23.x86_64 is already installed, skipping.
Package systemd-222-10.fc23.x86_64 is already installed, skipping.
Error: installed package python2-polib-1.0.7-2.fc23.noarch obsoletes
python-polib < 1.0.7-2.fc23 provided by python-polib-1.0.3-6.fc23.noarch
(try to add '--allowerasing' to command line to replace conflicting packages)


You might try to file a dnf BZ but mine 1291850 was two tiles closed as not a
but and then closed as a duplicate of another bug.

IMHO the simplest solution would to push the patch with better explanation
in's a workaround.

LSommit message becuase it's a workaround.

LS

Updated patch with reworded commit message.

--
Milan Kubik

From d55965f8662f335b7319a1868ab072075ed03077 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Mon, 15 Feb 2016 15:54:40 +0100
Subject: [PATCH] spec file: rename the python-polib dependency name to
 python2-polib

Trying to install the package depending on python-polib breaks
when the system has newer (and renamed) version python2-polib.

*This patch is an workaround* for the issue described in [1].
If a renamed package's provides is equal to an older package's
name, dnf tries to install the older package.
When the newer package is in the system, this leads to a conflict.

[1]: https://bugzilla.redhat.com/show_bug.cgi?id=1096506
---
 freeipa.spec.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 48fec974246dbc2933dd172318157f3e0e050a3b..caaef8803dd8625f24fa40005f7d83be7e6a6c66 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -77,7 +77,7 @@ BuildRequires:  python-gssapi >= 1.1.2
 BuildRequires:  python-rhsm
 BuildRequires:  pyOpenSSL
 BuildRequires:  pylint >= 1.0
-BuildRequires:  python-polib
+BuildRequires:  python2-polib
 BuildRequires:  python-libipa_hbac
 BuildRequires:  python-memcached
 BuildRequires:  python-lxml
@@ -562,7 +562,7 @@ Requires: python-nose
 Requires: pytest >= 2.6
 Requires: python-paste
 Requires: python-coverage
-Requires: python-polib
+Requires: python2-polib
 Requires: python-pytest-multihost >= 0.5
 Requires: python-pytest-sourceorder
 Requires: ldns-utils
-- 
2.7.1

-- 
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 0034] ipatests: extend permission plugin test with new expected output

[Milan Kubík]

On 02/24/2016 07:05 PM, Martin Basti wrote:



On 24.02.2016 08:34, Milan Kubík wrote:

On 02/18/2016 03:52 PM, Milan Kubík wrote:

On 02/15/2016 04:59 PM, Milan Kubík wrote:

Patch attached. Applies on ipa-4-3 as well.




Updated version of patch fixes test_old_permission_plugin as well.

--
Milan Kubik



Review bump.

--
Milan Kubik



NACK

[mbasti@dhcp129-96 freeipa-devel]$ git show -U0 | pep8 --diff
./ipatests/test_xmlrpc/test_old_permission_plugin.py:528:80: E501 line 
too long (95 > 79 characters)
./ipatests/test_xmlrpc/test_old_permission_plugin.py:586:1: E101 
indentation contains mixed spaces and tabs
./ipatests/test_xmlrpc/test_old_permission_plugin.py:586:1: W191 
indentation contains tabs
./ipatests/test_xmlrpc/test_old_permission_plugin.py:587:1: E101 
indentation contains mixed spaces and tabs
./ipatests/test_xmlrpc/test_old_permission_plugin.py:587:80: E501 line 
too long (95 > 79 characters)
./ipatests/test_xmlrpc/test_old_permission_plugin.py:591:1: E101 
indentation contains mixed spaces and tabs
./ipatests/test_xmlrpc/test_old_permission_plugin.py:591:1: W191 
indentation contains tabs
./ipatests/test_xmlrpc/test_permission_plugin.py:821:80: E501 line too 
long (99 > 79 characters)
./ipatests/test_xmlrpc/test_permission_plugin.py:884:80: E501 line too 
long (99 > 79 characters)



Sorry for that. Updated patch attached.

--
Milan Kubik

From ecaa2d4ef7a2e4a5d5b953c56e6e5bd760f35012 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Mon, 15 Feb 2016 16:54:34 +0100
Subject: [PATCH] ipatests: extend permission plugin test with new expected
 output

---
 ipatests/test_xmlrpc/test_old_permission_plugin.py | 14 ++
 ipatests/test_xmlrpc/test_permission_plugin.py | 18 ++
 2 files changed, 32 insertions(+)

diff --git a/ipatests/test_xmlrpc/test_old_permission_plugin.py b/ipatests/test_xmlrpc/test_old_permission_plugin.py
index 9e4b561a6f8ff4d6eac767f7f24e35ee4a910eff..09f43fee8c60e4e97d233d256f046fef44d31acf 100644
--- a/ipatests/test_xmlrpc/test_old_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_old_permission_plugin.py
@@ -524,6 +524,13 @@ class test_old_permission(Declarative):
 'subtree': u'ldap:///%s' % users_dn,
 },
 ],
+messages=({
+'message': (u'Search result has been truncated to '
+'configured search limit.'),
+'code': 13017,
+'type': u'warning',
+'name': u'SearchResultTruncated'
+},),
 ),
 ),
 
@@ -577,6 +584,13 @@ class test_old_permission(Declarative):
 DN(res['dn']).endswith(DN(api.env.container_permission,
   api.env.basedn)) and
 'ipapermission' in res['objectclass']],
+messages=({
+'message': (u'Search result has been truncated to '
+'configured search limit.'),
+'code': 13017,
+'type': u'warning',
+'name': u'SearchResultTruncated'
+},),
 ),
 ),
 
diff --git a/ipatests/test_xmlrpc/test_permission_plugin.py b/ipatests/test_xmlrpc/test_permission_plugin.py
index 01294665814fc667f932272ee8bc3077583b67df..8026e84366e3b9056ed6e2ff6d76c58bdc95140e 100644
--- a/ipatests/test_xmlrpc/test_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_permission_plugin.py
@@ -816,6 +816,15 @@ class test_permission(Declarative):
 'ipapermlocation': [users_dn],
 },
 ],
+messages=(
+{
+'message': (u'Search result has been truncated to '
+'configured search limit.'),
+'code': 13017,
+'type': u'warning',
+'name': u'SearchResultTruncated'
+},
+),
 ),
 ),
 
@@ -871,6 +880,15 @@ class test_permission(Declarative):
 DN(res['dn']).endswith(DN(api.env.container_permission,
   api.env.basedn)) and
 'ipapermission' in res['objectclass']],
+messages=(
+{
+'message': (u'Search result has been truncated to '
+'configured search limit.'),
+'code': 13017,
+'type': u'warning',
+'name': u'SearchResultTruncated'
+},
+),
 ),
 ),
 
-- 
2.7.1

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

Re: [Freeipa-devel] [patch 0034] ipatests: extend permission plugin test with new expected output

[Milan Kubík]

On 02/18/2016 03:52 PM, Milan Kubík wrote:

On 02/15/2016 04:59 PM, Milan Kubík wrote:

Patch attached. Applies on ipa-4-3 as well.




Updated version of patch fixes test_old_permission_plugin as well.

--
Milan Kubik



Review bump.

--
Milan Kubik

-- 
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 0034] ipatests: extend permission plugin test with new expected output

[Milan Kubík]

On 02/15/2016 04:59 PM, Milan Kubík wrote:

Patch attached. Applies on ipa-4-3 as well.




Updated version of patch fixes test_old_permission_plugin as well.

--
Milan Kubik

From 518f85240a14f256b5cc8542f66d3766a66b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Mon, 15 Feb 2016 16:54:34 +0100
Subject: [PATCH] ipatests: extend permission plugin test with new expected
 output

---
 ipatests/test_xmlrpc/test_old_permission_plugin.py | 12 
 ipatests/test_xmlrpc/test_permission_plugin.py | 16 
 2 files changed, 28 insertions(+)

diff --git a/ipatests/test_xmlrpc/test_old_permission_plugin.py b/ipatests/test_xmlrpc/test_old_permission_plugin.py
index 9e4b561a6f8ff4d6eac767f7f24e35ee4a910eff..c225d784d440751c09e7b16909fc8ad89f55c344 100644
--- a/ipatests/test_xmlrpc/test_old_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_old_permission_plugin.py
@@ -524,6 +524,12 @@ class test_old_permission(Declarative):
 'subtree': u'ldap:///%s' % users_dn,
 },
 ],
+messages=({
+'message': u'Search result has been truncated to configured search limit.',
+'code': 13017,
+'type': u'warning',
+'name': u'SearchResultTruncated'
+},),
 ),
 ),
 
@@ -577,6 +583,12 @@ class test_old_permission(Declarative):
 DN(res['dn']).endswith(DN(api.env.container_permission,
   api.env.basedn)) and
 'ipapermission' in res['objectclass']],
+		messages=({
+'message': u'Search result has been truncated to configured search limit.',
+'code': 13017,
+'type': u'warning',
+'name': u'SearchResultTruncated'
+		},),
 ),
 ),
 
diff --git a/ipatests/test_xmlrpc/test_permission_plugin.py b/ipatests/test_xmlrpc/test_permission_plugin.py
index 01294665814fc667f932272ee8bc3077583b67df..d641a2d078e0f275a44bbc539aef1352c023ae9b 100644
--- a/ipatests/test_xmlrpc/test_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_permission_plugin.py
@@ -816,6 +816,14 @@ class test_permission(Declarative):
 'ipapermlocation': [users_dn],
 },
 ],
+messages=(
+{
+'message': u'Search result has been truncated to configured search limit.',
+'code': 13017,
+'type': u'warning',
+'name': u'SearchResultTruncated'
+},
+),
 ),
 ),
 
@@ -871,6 +879,14 @@ class test_permission(Declarative):
 DN(res['dn']).endswith(DN(api.env.container_permission,
   api.env.basedn)) and
 'ipapermission' in res['objectclass']],
+messages=(
+{
+'message': u'Search result has been truncated to configured search limit.',
+'code': 13017,
+'type': u'warning',
+'name': u'SearchResultTruncated'
+},
+),
 ),
 ),
 
-- 
2.7.1

-- 
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 0033] spec file: update the python-polib dependency name to python2-polib

[Milan Kubík]

On 02/15/2016 05:00 PM, Petr Vobornik wrote:

On 02/15/2016 04:37 PM, Milan Kubík wrote:

Reflect the updated name of the package.



Seems to me as a packaging bug in python-polib. It should use 
python_provide macro to handle the transition.

I will open a bug against it, then.

--
Milan Kubik

--
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] [patch 0034] ipatests: extend permission plugin test with new expected output

[Milan Kubík]

Patch attached. Applies on ipa-4-3 as well.

--
Milan Kubik

From 0f0433b360f65ffd4431948d1efed4428c39feae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Mon, 15 Feb 2016 16:54:34 +0100
Subject: [PATCH] ipatests: extend permission plugin test with new expected
 output

---
 ipatests/test_xmlrpc/test_permission_plugin.py | 16 
 1 file changed, 16 insertions(+)

diff --git a/ipatests/test_xmlrpc/test_permission_plugin.py b/ipatests/test_xmlrpc/test_permission_plugin.py
index 01294665814fc667f932272ee8bc3077583b67df..d641a2d078e0f275a44bbc539aef1352c023ae9b 100644
--- a/ipatests/test_xmlrpc/test_permission_plugin.py
+++ b/ipatests/test_xmlrpc/test_permission_plugin.py
@@ -816,6 +816,14 @@ class test_permission(Declarative):
 'ipapermlocation': [users_dn],
 },
 ],
+messages=(
+{
+'message': u'Search result has been truncated to configured search limit.',
+'code': 13017,
+'type': u'warning',
+'name': u'SearchResultTruncated'
+},
+),
 ),
 ),
 
@@ -871,6 +879,14 @@ class test_permission(Declarative):
 DN(res['dn']).endswith(DN(api.env.container_permission,
   api.env.basedn)) and
 'ipapermission' in res['objectclass']],
+messages=(
+{
+'message': u'Search result has been truncated to configured search limit.',
+'code': 13017,
+'type': u'warning',
+'name': u'SearchResultTruncated'
+},
+),
 ),
 ),
 
-- 
2.7.1

-- 
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] [patch 0033] spec file: update the python-polib dependency name to python2-polib

[Milan Kubík]

Reflect the updated name of the package.

--
Milan Kubik

From 43d532107a150538246591b1999afc41bd43315e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Mon, 15 Feb 2016 15:54:40 +0100
Subject: [PATCH] spec file: update the python-polib dependency name to
 python2-polib

Trying to install the package depending on python-polib breaks
when the system has newer (and renamed) version python2-polib.
This patch removes the conflict.
---
 freeipa.spec.in | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 54a11bfc8cced643c19c29c5ada70bacf7540395..b775eca8cb31e28660e057436b8aed438bdf8c87 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -77,7 +77,7 @@ BuildRequires:  python-gssapi >= 1.1.2
 BuildRequires:  python-rhsm
 BuildRequires:  pyOpenSSL
 BuildRequires:  pylint >= 1.0
-BuildRequires:  python-polib
+BuildRequires:  python2-polib
 BuildRequires:  python-libipa_hbac
 BuildRequires:  python-memcached
 BuildRequires:  python-lxml
@@ -562,7 +562,7 @@ Requires: python-nose
 Requires: pytest >= 2.6
 Requires: python-paste
 Requires: python-coverage
-Requires: python-polib
+Requires: python2-polib
 Requires: python-pytest-multihost >= 0.5
 Requires: python-pytest-sourceorder
 Requires: ldns-utils
-- 
2.7.1

-- 
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 0084-0086] CI: Add double circle topology

[Milan Kubík]

On 02/12/2016 10:59 AM, David Kupka wrote:
Sending one more topology test. This one creates a M groups consisting 
N (N>=2) servers.
First two servers in each group are used to connect with nearest four 
groups and also with the other servers inside the group (when N>2). 
Servers inside the group (not connecting to other groups) are 
connected with each other.


The patch set needs freeipa-dkupka-8{1,2,3} applied.


ACK

--
Milan Kubik

--
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] CI: Add simple replication test in 2-connected topology.

[Milan Kubík]

On 02/12/2016 10:50 AM, David Kupka wrote:

On 10/02/16 08:51, David Kupka wrote:

This topology should be closer to the ones in real world than our
current ones. But it is still impractical and (hopefully) no one has
such deployment.
If some user could share his/her deployment topology I will be happy to
create generator based on it.


Updated patches attached.


ACK. Thanks for providing more complicated topologies.

--
Milan Kubik

--
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] [TEST][Patch 0022] small refactoring in integration tests due to BZ 1303095

[Milan Kubík]

On 02/04/2016 08:49 AM, Oleg Fayans wrote:

Hi Petr,

On 02/03/2016 02:19 PM, Petr Spacek wrote:

On 3.2.2016 10:22, Oleg Fayans wrote:

Guys, can anyone take a look at this?

The commit message does not explain why you are setting search path.

Fixed.


I have to say that I do not like touching resolv.conf, as stated many times
earlier. Why the test has to reconfigure the host and cannot use values
provided by the provisioning system?

This patch exactly removes this messing around with nameservers in
resolv.conf
It introduces the possibility to put ipa domain in the search directive
of resolv.conf so that we could test service autodiscovery during client
installation.



I just verified that the tampering with resolv.conf is not needed 
(libvirt and ovirt/rhev). I think this is an artifact from the whole 
issue of "let's use improvised domain names, what can go wrong" approach 
that was uncovered by the enforced DNS checks. I think we can defer the 
networking configuration to provisioning system.


--
Milan Kubik

-- 
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 0032] ipatests: add missing certprofile fixture

[Milan Kubík]

On 02/11/2016 10:00 AM, Martin Babinsky wrote:

On 02/09/2016 04:06 PM, Milan Kubík wrote:

On 02/09/2016 02:37 PM, Milan Kubík wrote:

Fixes the CA ACL tests broken by removed import. This patch doesn't
rely on undocumented behavior of pytest.

The patch invalidates patch 133 by Martin Babinsky.




Patch updated with trac link

--
Milan Kubik




ACK I guess, although copypasta makes me very sad.

Why do you think the conftest.py approach is not practical for us?

I think introducing new file to hold shared fixtures is not worth 
managing it and it also hides (by the virtue of pytest's working) from 
where the fixture comes. I think it is better in our case to have all 
fixtures the test uses in its module, even though it looks like 
copy-pasting. If I had renamed the entry for the test only, would it 
still been considered copy-paste?


Anyway, patch for ipa-4-3 attached.


--
Milan Kubik

From 7615c5ca6bed0392d1483840c828119eff4306f1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Tue, 2 Feb 2016 11:34:26 +0100
Subject: [PATCH] ipatests: Add missing certificate profile fixture

https://fedorahosted.org/freeipa/ticket/5630
---
 ipatests/test_xmlrpc/test_caacl_plugin.py | 12 ++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_caacl_plugin.py b/ipatests/test_xmlrpc/test_caacl_plugin.py
index d5ded19516ec917bcb2e90034b4fac8a6324b2e9..f20b02b295024313008be7b75bdcae2ade70b4ff 100644
--- a/ipatests/test_xmlrpc/test_caacl_plugin.py
+++ b/ipatests/test_xmlrpc/test_caacl_plugin.py
@@ -11,13 +11,21 @@ import pytest
 from ipalib import errors
 from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test
 
-# reuse the fixture
-from ipatests.test_xmlrpc.test_certprofile_plugin import default_profile
+from ipatests.test_xmlrpc.tracker.certprofile_plugin import CertprofileTracker
 from ipatests.test_xmlrpc.tracker.caacl_plugin import CAACLTracker
 from ipatests.test_xmlrpc.tracker.stageuser_plugin import StageUserTracker
 
 
 @pytest.fixture(scope='class')
+def default_profile(request):
+name = 'caIPAserviceCert'
+desc = u'Standard profile for network services'
+tracker = CertprofileTracker(name, store=True, desc=desc)
+tracker.track_create()
+return tracker
+
+
+@pytest.fixture(scope='class')
 def default_acl(request):
 name = u'hosts_services_caIPAserviceCert'
 tracker = CAACLTracker(name, service_category=u'all', host_category=u'all')
-- 
2.7.1

-- 
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 0002] Refactor test_group_plugin

[Milan Kubík]

On 02/09/2016 04:19 PM, Milan Kubík wrote:

On 01/28/2016 10:42 AM, Filip Skola wrote:


- Original Message -

On 01/25/2016 11:11 AM, Filip Skola wrote:

- Original Message -

On 01/15/2016 03:38 PM, Filip Skola wrote:

Hi,

sending rebased patch.

F.

- Original Message -

Hello,

sorry for delays. The patch no longer applies to master. Rebase it,
please.

Milan

- Original Message -
From: "Filip Škola" <fsk...@redhat.com>
To: "Milan Kubík" <mku...@redhat.com>
Cc: freeipa-devel@redhat.com
Sent: Wednesday, 9 December, 2015 7:01:02 PM
Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor 
test_group_plugin


On Mon, 7 Dec 2015 17:49:18 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 12/03/2015 08:15 PM, Filip Škola wrote:

On Mon, 30 Nov 2015 17:18:30 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 11/23/2015 04:42 PM, Filip Škola wrote:

Sending updated patch.

F.

On Mon, 23 Nov 2015 14:59:34 +0100
Filip Škola <fsk...@redhat.com> wrote:


Found couple of issues (broke some dependencies).

NACK

F.

On Fri, 20 Nov 2015 13:56:36 +0100
Filip Škola <fsk...@redhat.com> wrote:


Another one.

F.

Hi, the tests look good. Few remarks, though.

1. Please, use the shortes copyright notice in new modules.

 #
 # Copyright (C) 2015  FreeIPA Contributors see 
COPYING for

license #

2. The tests `test_group_remove_group_from_protected_group` and
`test_group_full_set_of_objectclass_not_available_post_detach`
were not ported. Please, include them in the patch.

Also, for less hassle, please rebase your patches on top of
freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch 

Which changes the location of tracker implementations and 
prevents

circular imports.

Thanks.


Hi,

these cases are there, in corresponding classes. They are marked
with the original comments. (However I can move them to separate
class if desirable.)

The copyright notice is changed. Also included a few changes 
in the

test with user without private group.

Filip

NACK

linter:
* Module tracker.group_plugin
ipatests/test_xmlrpc/tracker/group_plugin.py:257:
[E0102(function-redefined), GroupTracker.check_remove_member] 
method

already defined line 253)

Probably a leftover after the rebase made on top of my patch. 
Please

fix it. You can check youch changes by make-lint script before
sending them.

Thanks


Hi,

I learned to use make-lint!

Thanks,
F.


Hello,

NACK, pylint doesn't seem to like the way the fixtures are imported
(pytest does a lot of runtime magic) [1].
One possible solution would be [2]. Though, I don't think this 
would be
a good idea in our environment. I suggest to create the fixtures 
on per

module basis.


[1]: http://fpaste.org/311949/53118942/
[2]:
https://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects 



--
Milan Kubik



Hi,

the fixtures were copied into corresponding module. Please note 
that this

patch has a dependence on my patch 0001 (user plugin).

Filip

Linter:
* Module ipatests.test_xmlrpc.tracker.group_plugin
W:100,26: Calling a dict.iter*() method (dict-iter-method)

please use dict.items

--
Milan Kubik



Hi, sorry. This has been fixed in this patch.

Filip

ACK, thanks for the patience. :)


Sorry, there are some other things I need clarified. NACK.
Mail will follow later.

--
Milan Kubik

--
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 0002] Refactor test_group_plugin

[Milan Kubík]

On 02/10/2016 09:17 AM, Milan Kubík wrote:

On 02/09/2016 04:19 PM, Milan Kubík wrote:

On 01/28/2016 10:42 AM, Filip Skola wrote:


- Original Message -

On 01/25/2016 11:11 AM, Filip Skola wrote:

- Original Message -

On 01/15/2016 03:38 PM, Filip Skola wrote:

Hi,

sending rebased patch.

F.

- Original Message -

Hello,

sorry for delays. The patch no longer applies to master. Rebase 
it,

please.

Milan

- Original Message -
From: "Filip Škola" <fsk...@redhat.com>
To: "Milan Kubík" <mku...@redhat.com>
Cc: freeipa-devel@redhat.com
Sent: Wednesday, 9 December, 2015 7:01:02 PM
Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor 
test_group_plugin


On Mon, 7 Dec 2015 17:49:18 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 12/03/2015 08:15 PM, Filip Škola wrote:

On Mon, 30 Nov 2015 17:18:30 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 11/23/2015 04:42 PM, Filip Škola wrote:

Sending updated patch.

F.

On Mon, 23 Nov 2015 14:59:34 +0100
Filip Škola <fsk...@redhat.com> wrote:


Found couple of issues (broke some dependencies).

NACK

F.

On Fri, 20 Nov 2015 13:56:36 +0100
Filip Škola <fsk...@redhat.com> wrote:


Another one.

F.

Hi, the tests look good. Few remarks, though.

1. Please, use the shortes copyright notice in new modules.

 #
 # Copyright (C) 2015  FreeIPA Contributors see 
COPYING for

license #

2. The tests `test_group_remove_group_from_protected_group` and
`test_group_full_set_of_objectclass_not_available_post_detach`
were not ported. Please, include them in the patch.

Also, for less hassle, please rebase your patches on top of
freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch 

Which changes the location of tracker implementations and 
prevents

circular imports.

Thanks.


Hi,

these cases are there, in corresponding classes. They are marked
with the original comments. (However I can move them to separate
class if desirable.)

The copyright notice is changed. Also included a few changes 
in the

test with user without private group.

Filip

NACK

linter:
* Module tracker.group_plugin
ipatests/test_xmlrpc/tracker/group_plugin.py:257:
[E0102(function-redefined), GroupTracker.check_remove_member] 
method

already defined line 253)

Probably a leftover after the rebase made on top of my patch. 
Please

fix it. You can check youch changes by make-lint script before
sending them.

Thanks


Hi,

I learned to use make-lint!

Thanks,
F.


Hello,

NACK, pylint doesn't seem to like the way the fixtures are imported
(pytest does a lot of runtime magic) [1].
One possible solution would be [2]. Though, I don't think this 
would be
a good idea in our environment. I suggest to create the fixtures 
on per

module basis.


[1]: http://fpaste.org/311949/53118942/
[2]:
https://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects 



--
Milan Kubik



Hi,

the fixtures were copied into corresponding module. Please note 
that this

patch has a dependence on my patch 0001 (user plugin).

Filip

Linter:
* Module ipatests.test_xmlrpc.tracker.group_plugin
W:100,26: Calling a dict.iter*() method (dict-iter-method)

please use dict.items

--
Milan Kubik



Hi, sorry. This has been fixed in this patch.

Filip

ACK, thanks for the patience. :)


Sorry, there are some other things I need clarified. NACK.
Mail will follow later.

What is the purpose of `make_fixture_detach` in your patches? They are 
not used anywhere and the finalizer does nothing.


--
Milan Kubik

--
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 0002] Refactor test_group_plugin

[Milan Kubík]

On 01/28/2016 10:42 AM, Filip Skola wrote:


- Original Message -

On 01/25/2016 11:11 AM, Filip Skola wrote:

- Original Message -

On 01/15/2016 03:38 PM, Filip Skola wrote:

Hi,

sending rebased patch.

F.

- Original Message -

Hello,

sorry for delays. The patch no longer applies to master. Rebase it,
please.

Milan

- Original Message -
From: "Filip Škola" <fsk...@redhat.com>
To: "Milan Kubík" <mku...@redhat.com>
Cc: freeipa-devel@redhat.com
Sent: Wednesday, 9 December, 2015 7:01:02 PM
Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin

On Mon, 7 Dec 2015 17:49:18 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 12/03/2015 08:15 PM, Filip Škola wrote:

On Mon, 30 Nov 2015 17:18:30 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 11/23/2015 04:42 PM, Filip Škola wrote:

Sending updated patch.

F.

On Mon, 23 Nov 2015 14:59:34 +0100
Filip Škola <fsk...@redhat.com> wrote:


Found couple of issues (broke some dependencies).

NACK

F.

On Fri, 20 Nov 2015 13:56:36 +0100
Filip Škola <fsk...@redhat.com> wrote:


Another one.

F.

Hi, the tests look good. Few remarks, though.

1. Please, use the shortes copyright notice in new modules.

 #
 # Copyright (C) 2015  FreeIPA Contributors see COPYING for
license #

2. The tests `test_group_remove_group_from_protected_group` and
`test_group_full_set_of_objectclass_not_available_post_detach`
were not ported. Please, include them in the patch.

Also, for less hassle, please rebase your patches on top of
freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch
Which changes the location of tracker implementations and prevents
circular imports.

Thanks.


Hi,

these cases are there, in corresponding classes. They are marked
with the original comments. (However I can move them to separate
class if desirable.)

The copyright notice is changed. Also included a few changes in the
test with user without private group.

Filip

NACK

linter:
* Module tracker.group_plugin
ipatests/test_xmlrpc/tracker/group_plugin.py:257:
[E0102(function-redefined), GroupTracker.check_remove_member] method
already defined line 253)

Probably a leftover after the rebase made on top of my patch. Please
fix it. You can check youch changes by make-lint script before
sending them.

Thanks


Hi,

I learned to use make-lint!

Thanks,
F.


Hello,

NACK, pylint doesn't seem to like the way the fixtures are imported
(pytest does a lot of runtime magic) [1].
One possible solution would be [2]. Though, I don't think this would be
a good idea in our environment. I suggest to create the fixtures on per
module basis.


[1]: http://fpaste.org/311949/53118942/
[2]:
https://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects

--
Milan Kubik



Hi,

the fixtures were copied into corresponding module. Please note that this
patch has a dependence on my patch 0001 (user plugin).

Filip

Linter:
* Module ipatests.test_xmlrpc.tracker.group_plugin
W:100,26: Calling a dict.iter*() method (dict-iter-method)

please use dict.items

--
Milan Kubik



Hi, sorry. This has been fixed in this patch.

Filip

ACK, thanks for the patience. :)

--
Milan Kubik

--
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 0032] ipatests: add missing certprofile fixture

[Milan Kubík]

On 02/09/2016 02:37 PM, Milan Kubík wrote:
Fixes the CA ACL tests broken by removed import. This patch doesn't 
rely on undocumented behavior of pytest.


The patch invalidates patch 133 by Martin Babinsky.




Patch updated with trac link

--
Milan Kubik

From 0779fb587d10cb8fd8f7b7293527be623b078077 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Tue, 2 Feb 2016 11:34:26 +0100
Subject: [PATCH] ipatests: Add missing certificate profile fixture

https://fedorahosted.org/freeipa/ticket/5630
---
 ipatests/test_xmlrpc/test_caacl_plugin.py | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/ipatests/test_xmlrpc/test_caacl_plugin.py b/ipatests/test_xmlrpc/test_caacl_plugin.py
index 85c7072a0bc483822b9b5c201d61932a423dd3d8..f20b02b295024313008be7b75bdcae2ade70b4ff 100644
--- a/ipatests/test_xmlrpc/test_caacl_plugin.py
+++ b/ipatests/test_xmlrpc/test_caacl_plugin.py
@@ -11,12 +11,21 @@ import pytest
 from ipalib import errors
 from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test
 
-# reuse the fixture
+from ipatests.test_xmlrpc.tracker.certprofile_plugin import CertprofileTracker
 from ipatests.test_xmlrpc.tracker.caacl_plugin import CAACLTracker
 from ipatests.test_xmlrpc.tracker.stageuser_plugin import StageUserTracker
 
 
 @pytest.fixture(scope='class')
+def default_profile(request):
+name = 'caIPAserviceCert'
+desc = u'Standard profile for network services'
+tracker = CertprofileTracker(name, store=True, desc=desc)
+tracker.track_create()
+return tracker
+
+
+@pytest.fixture(scope='class')
 def default_acl(request):
 name = u'hosts_services_caIPAserviceCert'
 tracker = CAACLTracker(name, service_category=u'all', host_category=u'all')
-- 
2.7.1

-- 
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] [patch 0032] ipatests: add missing certprofile fixture

[Milan Kubík]

Fixes the CA ACL tests broken by removed import. This patch doesn't rely 
on undocumented behavior of pytest.


The patch invalidates patch 133 by Martin Babinsky.

--
Milan Kubik

From 73dc9e91605c9299e48cdf62ddc0eb4927471a57 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Tue, 2 Feb 2016 11:34:26 +0100
Subject: [PATCH] ipatests: Add missing certificate profile fixture

---
 ipatests/test_xmlrpc/test_caacl_plugin.py | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/ipatests/test_xmlrpc/test_caacl_plugin.py b/ipatests/test_xmlrpc/test_caacl_plugin.py
index 85c7072a0bc483822b9b5c201d61932a423dd3d8..f20b02b295024313008be7b75bdcae2ade70b4ff 100644
--- a/ipatests/test_xmlrpc/test_caacl_plugin.py
+++ b/ipatests/test_xmlrpc/test_caacl_plugin.py
@@ -11,12 +11,21 @@ import pytest
 from ipalib import errors
 from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test
 
-# reuse the fixture
+from ipatests.test_xmlrpc.tracker.certprofile_plugin import CertprofileTracker
 from ipatests.test_xmlrpc.tracker.caacl_plugin import CAACLTracker
 from ipatests.test_xmlrpc.tracker.stageuser_plugin import StageUserTracker
 
 
 @pytest.fixture(scope='class')
+def default_profile(request):
+name = 'caIPAserviceCert'
+desc = u'Standard profile for network services'
+tracker = CertprofileTracker(name, store=True, desc=desc)
+tracker.track_create()
+return tracker
+
+
+@pytest.fixture(scope='class')
 def default_acl(request):
 name = u'hosts_services_caIPAserviceCert'
 tracker = CAACLTracker(name, service_category=u'all', host_category=u'all')
-- 
2.7.0

-- 
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 0133] re-add missing fixture import to the CA ACL plugin test

[Milan Kubík]

On 02/02/2016 03:36 PM, Martin Babinsky wrote:

This was causing some of the CA ACL plugin tests to fail/error.

Possible NACK. This is not an officially supported way how to reuse 
fixtures. [1]
I was working on a fix, that would contain all of the needed fixtures in 
the module that requires them.
See patch attached. However, this way kind of breaks DRY principle. The 
official conftest.py way is not practical for us, though.



[1]: 
http://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects


--
Milan Kubik

From 73dc9e91605c9299e48cdf62ddc0eb4927471a57 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Tue, 2 Feb 2016 11:34:26 +0100
Subject: [PATCH] ipatests: Add missing certificate profile fixture

---
 ipatests/test_xmlrpc/test_caacl_plugin.py | 11 ++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/ipatests/test_xmlrpc/test_caacl_plugin.py b/ipatests/test_xmlrpc/test_caacl_plugin.py
index 85c7072a0bc483822b9b5c201d61932a423dd3d8..f20b02b295024313008be7b75bdcae2ade70b4ff 100644
--- a/ipatests/test_xmlrpc/test_caacl_plugin.py
+++ b/ipatests/test_xmlrpc/test_caacl_plugin.py
@@ -11,12 +11,21 @@ import pytest
 from ipalib import errors
 from ipatests.test_xmlrpc.xmlrpc_test import XMLRPC_test
 
-# reuse the fixture
+from ipatests.test_xmlrpc.tracker.certprofile_plugin import CertprofileTracker
 from ipatests.test_xmlrpc.tracker.caacl_plugin import CAACLTracker
 from ipatests.test_xmlrpc.tracker.stageuser_plugin import StageUserTracker
 
 
 @pytest.fixture(scope='class')
+def default_profile(request):
+name = 'caIPAserviceCert'
+desc = u'Standard profile for network services'
+tracker = CertprofileTracker(name, store=True, desc=desc)
+tracker.track_create()
+return tracker
+
+
+@pytest.fixture(scope='class')
 def default_acl(request):
 name = u'hosts_services_caIPAserviceCert'
 tracker = CAACLTracker(name, service_category=u'all', host_category=u'all')
-- 
2.7.0

-- 
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 0003] Refactor test_replace

[Milan Kubík]

On 01/12/2016 03:04 PM, Milan Kubík wrote:

On 12/04/2015 11:29 AM, Filip Škola wrote:

On Fri, 4 Dec 2015 10:08:40 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 12/04/2015 10:04 AM, Filip Škola wrote:

Hi,

sending rather short one this time.

F.

NACK, UserTracker is implemented in
ipatests.test_xmlrpc.tracker.user_plugin.


Ah, sorry for this.


F.

Hi,
the tests do not work. Similar problems to test_attr. There are some 
problems with the expected and actual results. NACK.


Problems were caused by missing dependency (patch). The code looks good. 
ACK.


--
Milan Kubik

--
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 0004] Refactor test_attr

[Milan Kubík]

On 01/12/2016 03:03 PM, Milan Kubík wrote:

On 12/07/2015 01:25 PM, Filip Škola wrote:

Now the tier marker have lost somewhere on the way... which is
corrected in this patch.

/me apologizes for the noise

F.

On Mon, 7 Dec 2015 13:00:41 +0100
Filip Škola <fsk...@redhat.com> wrote:


Self-NACK, resubmitting with the last commit which includes
UserTracker from the right location...

F.

On Fri, 4 Dec 2015 16:24:16 +0100
Filip Škola <fsk...@redhat.com> wrote:


Hi,

sending a new version of test_attr.

F.
Hello, tha patch doesn't work. The tests fail on mismatches in 
expected and actual result.

NACK.


My mistake, the code just needed earlier patch. ACK.

--
Milan Kubik

--
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 0002] Refactor test_group_plugin

[Milan Kubík]

On 01/25/2016 11:11 AM, Filip Skola wrote:


- Original Message -

On 01/15/2016 03:38 PM, Filip Skola wrote:

Hi,

sending rebased patch.

F.

- Original Message -

Hello,

sorry for delays. The patch no longer applies to master. Rebase it,
please.

Milan

- Original Message -
From: "Filip Škola" <fsk...@redhat.com>
To: "Milan Kubík" <mku...@redhat.com>
Cc: freeipa-devel@redhat.com
Sent: Wednesday, 9 December, 2015 7:01:02 PM
Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin

On Mon, 7 Dec 2015 17:49:18 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 12/03/2015 08:15 PM, Filip Škola wrote:

On Mon, 30 Nov 2015 17:18:30 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 11/23/2015 04:42 PM, Filip Škola wrote:

Sending updated patch.

F.

On Mon, 23 Nov 2015 14:59:34 +0100
Filip Škola <fsk...@redhat.com> wrote:


Found couple of issues (broke some dependencies).

NACK

F.

On Fri, 20 Nov 2015 13:56:36 +0100
Filip Škola <fsk...@redhat.com> wrote:


Another one.

F.

Hi, the tests look good. Few remarks, though.

1. Please, use the shortes copyright notice in new modules.

#
# Copyright (C) 2015  FreeIPA Contributors see COPYING for
license #

2. The tests `test_group_remove_group_from_protected_group` and
`test_group_full_set_of_objectclass_not_available_post_detach`
were not ported. Please, include them in the patch.

Also, for less hassle, please rebase your patches on top of
freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch
Which changes the location of tracker implementations and prevents
circular imports.

Thanks.


Hi,

these cases are there, in corresponding classes. They are marked
with the original comments. (However I can move them to separate
class if desirable.)

The copyright notice is changed. Also included a few changes in the
test with user without private group.

Filip

NACK

linter:
* Module tracker.group_plugin
ipatests/test_xmlrpc/tracker/group_plugin.py:257:
[E0102(function-redefined), GroupTracker.check_remove_member] method
already defined line 253)

Probably a leftover after the rebase made on top of my patch. Please
fix it. You can check youch changes by make-lint script before
sending them.

Thanks


Hi,

I learned to use make-lint!

Thanks,
F.


Hello,

NACK, pylint doesn't seem to like the way the fixtures are imported
(pytest does a lot of runtime magic) [1].
One possible solution would be [2]. Though, I don't think this would be
a good idea in our environment. I suggest to create the fixtures on per
module basis.


[1]: http://fpaste.org/311949/53118942/
[2]:
https://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects

--
Milan Kubik



Hi,

the fixtures were copied into corresponding module. Please note that this patch 
has a dependence on my patch 0001 (user plugin).

Filip

Linter:
* Module ipatests.test_xmlrpc.tracker.group_plugin
W:100,26: Calling a dict.iter*() method (dict-iter-method)

please use dict.items

--
Milan Kubik

--
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 0005] Refactor test_nesting, create HostGroupTracker

[Milan Kubík]

On 01/18/2016 02:26 PM, Filip Skola wrote:

Hi,

this should be fixed in this patch.

F.

- Original Message -

On 01/15/2016 03:37 PM, Filip Skola wrote:

Hi,

sending rebased patch.

F.

- Original Message -

Hi,

the patch no longer applies to master. Please rebase it.

Thanks,
Milan

- Original Message -
From: "Filip Skola" <fsk...@redhat.com>
To: freeipa-devel@redhat.com
Cc: "Milan Kubík" <mku...@redhat.com>, "Aleš Mareček"
<amare...@redhat.com>
Sent: Tuesday, 22 December, 2015 11:56:15 AM
Subject: [PATCH 0005] Refactor test_nesting, create HostGroupTracker

Hi,

another patch from refactoring-test_xmlrpc series.

Filip


NACK, something seems to be missing in the patch


* Module ipatests.test_xmlrpc.tracker.hostgroup_plugin
ipatests/test_xmlrpc/tracker/hostgroup_plugin.py:222: [E1101(no-member),
HostGroupTracker.check_add_member_negative] Instance of
'HostGroupTracker' has no 'adds' member)

--
Milan Kubik



The same as with patch 0002:
* Module ipatests.test_xmlrpc.tracker.hostgroup_plugin
W:142,26: Calling a dict.iter*() method (dict-iter-method)

Please use dict.items method.

--
Milan Kubik

--
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 0031] ipatests: fix the install of external ca

[Milan Kubík]

On 01/22/2016 02:22 PM, Martin Babinsky wrote:

On 01/19/2016 05:56 PM, Milan Kubík wrote:

On 01/19/2016 05:31 PM, Milan Kubík wrote:

Patch attached.




This actually has a ticket opened. Patch with fixed commit message. ;)

--
Milan Kubik





Hi Milan,

for the step 1 installation I would rather reuse the 
tasks:install_master function which already does (nearly) all CLI 
option-related magic. You can extend its signature by adding a 
parameter to pass on additional options like this:


--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -258,7 +258,7 @@ def enable_replication_debugging(host):
  stdin_text=logging_ldif)


-def install_master(host, setup_dns=True, setup_kra=False):
+def install_master(host, setup_dns=True, setup_kra=False, 
extra_args=()):

 host.collect_log(paths.IPASERVER_INSTALL_LOG)
 host.collect_log(paths.IPACLIENT_INSTALL_LOG)
 inst = host.domain.realm.replace('.', '-')
@@ -284,6 +284,8 @@ def install_master(host, setup_dns=True, 
setup_kra=False):

 '--auto-reverse'
 ])

+args.extend(extra_args)
+
 host.run_command(args)
 enable_replication_debugging(host)
 setup_sssd_debugging(host)

Thanks for the suggestion. Though, this is not possible without larger 
changes to tasks.install_master. The external ca test needs to skip 
several steps that occur in the general install task. In this case, I'd 
remain with customized install in the test itself.


--
Milan Kubik

--
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] 0001 Refactor test_user_plugin

[Milan Kubík]

On 01/15/2016 03:41 PM, Filip Skola wrote:

Hi,

sending rebased patch on top of 58c42ddac0964a8cce7c1e1faa7516da53f028ad.

Includes a "fix" for the rename-to-invalid-username issue for the new version.

F.

- Original Message -

Hi,

I don't know what is causing the \r\n issue. I use vim and than send each
email with claws-mail. Didn't spot this issue when trying emailing the patch
to my other address. I'm trying to send it from zimbra now, let me know if
that helped pls.

Fix for the stageuser plugin issues caused by this patch should have been
included in the last update; I think the remaining issue is not caused by
UserTracker changes. Please correct me, if I'm wrong.


There is some issue with "test_rename_to_too_long_login" test. It fails but
actually this is false positive because it is possible to create login upto
255 characters. I don't know why test mentions 32 characters without any
other modified setup.
NACK for now.
  - alich -

This has been changed. This test still fails, though.

Filip



- Original Message -

From: "Aleš Mareček" <amare...@redhat.com>
To: "Filip Škola" <fsk...@redhat.com>
Cc: freeipa-devel@redhat.com, "Milan Kubík" <mku...@redhat.com>
Sent: Thursday, December 10, 2015 4:11:47 PM
Subject: Re: [Freeipa-devel] [PATCH] 0001 Refactor test_user_plugin

Ah, sorry, haven't realized there had been devel list attached.
Ok, there is some problem with \r\n in the patch.
Filip, please take a look at it...
Thanks...
  - alich -

- Original Message -

From: "Filip Škola" <fsk...@redhat.com>
To: "Aleš Mareček" <amare...@redhat.com>
Cc: freeipa-devel@redhat.com, "Milan Kubík" <mku...@redhat.com>
Sent: Thursday, December 10, 2015 11:29:52 AM
Subject: Re: [Freeipa-devel] [PATCH] 0001 Refactor test_user_plugin

Hi,

this if fixed. Also issues with test_stageuser_plugin caused by
UserTracker changes should be fixed here.

Filip


On Mon, 7 Dec 2015 09:29:31 -0500 (EST)
Aleš Mareček <amare...@redhat.com> wrote:


NACK.

$ ./make-lint
* Module ipatests.test_xmlrpc.test_user_plugin
ipatests/test_xmlrpc/test_user_plugin.py:42:
[E0611(no-name-in-module), ] No name 'ldaptracker' in module
'ipatests.test_xmlrpc')

$ grep ldaptracker ipatests/test_xmlrpc/test_user_plugin.py
from ipatests.test_xmlrpc.ldaptracker import Tracker
$ ls ipatests/test_xmlrpc/ldaptracker*
ls: cannot access ipatests/test_xmlrpc/ldaptracker*: No such file or
directory


- Original Message -

From: "Filip Škola" <fsk...@redhat.com>
To: "Milan Kubík" <mku...@redhat.com>
Cc: freeipa-devel@redhat.com
Sent: Thursday, December 3, 2015 5:38:43 PM
Subject: Re: [Freeipa-devel] [PATCH] 0001 Refactor test_user_plugin

Hi,

sending corrected version.

F.

On Thu, 12 Nov 2015 14:03:19 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 11/10/2015 12:13 PM, Filip Škola wrote:

Hi,

fixed.

F.

On Tue, 10 Nov 2015 10:52:45 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 11/09/2015 04:35 PM, Filip Škola wrote:

Another patch was applied in the meantime.

Attaching an updated version.

F.

On Mon, 9 Nov 2015 13:35:02 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 11/06/2015 11:32 AM, Filip Škola wrote:
Hi,
the patch doesn't apply.


Please fix this.

   ipatests/test_xmlrpc/test_user_plugin.py:1419:
[E0602(undefined-variable),
TestDeniedBindWithExpiredPrincipal.teardown_class] Undefined
variable 'user1')

Also, use the version numbers for your changed patches.




Thanks for the patch. Several issues:

1. Use dict.items instead of dict.iteritems, for python3
compatibility

2. What is the purpose of TestPrepare class? The 'purge' methods
do not call any ipa commands.
Tracker.make_fixture should be used to make the Tracked resources
clean themselves up when they're out of scope.

3. Why reference the resources by hardcoded name if they have a
fixture representation?

4. Rewrite {create,delete}_test_group to a fixture. You may want
to use different scope (or not).

5. In `def atest_rename_to_invalid_login(self, user):` - use
pytest.skipif decorator and provide a reason if you must,
do not obfuscate method name in order not to run 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



--
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
NACK, there are errors occuring that do not appear in the respective 
test cases in the declarative test.
In the original module the ` Test a login name that is too long` and 
`Try to rename to a username that is too long` do not use {add,set}attr. 
Why do you use them?


I'm also postponing the review of your other patche

[Freeipa-devel] [PATCH 0031] ipatests: fix the install of external ca

[Milan Kubík]

Patch attached.

--
Milan Kubik

From 8f343a755dfdf9feb1ab9a3b7c91797f80b0261e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Tue, 19 Jan 2016 15:06:38 +0100
Subject: [PATCH] ipatests: fix the install of external ca

Fixes the install invocation in the test to use domain and
realm correctly. Also makes the test aware of domain levels.
---
 ipatests/test_integration/test_external_ca.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ipatests/test_integration/test_external_ca.py b/ipatests/test_integration/test_external_ca.py
index fbffdf14bd28d8b990d7a30746af172f67c5faa2..1228c9d6537fe542e7a0521b50a0a8f17112eab6 100644
--- a/ipatests/test_integration/test_external_ca.py
+++ b/ipatests/test_integration/test_external_ca.py
@@ -33,7 +33,9 @@ class TestExternalCA(IntegrationTest):
 '-a', self.master.config.admin_password,
 '-p', self.master.config.dirman_password,
 '--setup-dns', '--no-forwarders',
-'-r', self.master.domain.name,
+'-n', self.master.domain.name,
+'-r', self.master.domain.realm,
+'--domain-level=%i' % self.master.config.domain_level,
 '--external-ca'
 ])
 
-- 
2.7.0

-- 
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 0031] ipatests: fix the install of external ca

[Milan Kubík]

On 01/19/2016 05:31 PM, Milan Kubík wrote:

Patch attached.




This actually has a ticket opened. Patch with fixed commit message. ;)

--
Milan Kubik

From d3e43f982d246a9d82807aeff1320e8c1af58e0e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Tue, 19 Jan 2016 15:06:38 +0100
Subject: [PATCH] ipatests: fix the install of external ca

Fixes the install invocation in the test to use domain and
realm correctly. Also makes the test aware of domain levels.

https://fedorahosted.org/freeipa/ticket/5605
---
 ipatests/test_integration/test_external_ca.py | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/ipatests/test_integration/test_external_ca.py b/ipatests/test_integration/test_external_ca.py
index fbffdf14bd28d8b990d7a30746af172f67c5faa2..1228c9d6537fe542e7a0521b50a0a8f17112eab6 100644
--- a/ipatests/test_integration/test_external_ca.py
+++ b/ipatests/test_integration/test_external_ca.py
@@ -33,7 +33,9 @@ class TestExternalCA(IntegrationTest):
 '-a', self.master.config.admin_password,
 '-p', self.master.config.dirman_password,
 '--setup-dns', '--no-forwarders',
-'-r', self.master.domain.name,
+'-n', self.master.domain.name,
+'-r', self.master.domain.realm,
+'--domain-level=%i' % self.master.config.domain_level,
 '--external-ca'
 ])
 
-- 
2.7.0

-- 
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 0005] Refactor test_nesting, create HostGroupTracker

[Milan Kubík]

On 01/15/2016 03:37 PM, Filip Skola wrote:

Hi,

sending rebased patch.

F.

- Original Message -

Hi,

the patch no longer applies to master. Please rebase it.

Thanks,
Milan

- Original Message -
From: "Filip Skola" <fsk...@redhat.com>
To: freeipa-devel@redhat.com
Cc: "Milan Kubík" <mku...@redhat.com>, "Aleš Mareček" <amare...@redhat.com>
Sent: Tuesday, 22 December, 2015 11:56:15 AM
Subject: [PATCH 0005] Refactor test_nesting, create HostGroupTracker

Hi,

another patch from refactoring-test_xmlrpc series.

Filip


NACK, something seems to be missing in the patch


* Module ipatests.test_xmlrpc.tracker.hostgroup_plugin
ipatests/test_xmlrpc/tracker/hostgroup_plugin.py:222: [E1101(no-member), 
HostGroupTracker.check_add_member_negative] Instance of 
'HostGroupTracker' has no 'adds' member)


--
Milan Kubik

--
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 0002] Refactor test_group_plugin

[Milan Kubík]

On 01/15/2016 03:38 PM, Filip Skola wrote:

Hi,

sending rebased patch.

F.

- Original Message -

Hello,

sorry for delays. The patch no longer applies to master. Rebase it, please.

Milan

- Original Message -
From: "Filip Škola" <fsk...@redhat.com>
To: "Milan Kubík" <mku...@redhat.com>
Cc: freeipa-devel@redhat.com
Sent: Wednesday, 9 December, 2015 7:01:02 PM
Subject: Re: [Freeipa-devel] [PATCH 0002] Refactor test_group_plugin

On Mon, 7 Dec 2015 17:49:18 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 12/03/2015 08:15 PM, Filip Škola wrote:

On Mon, 30 Nov 2015 17:18:30 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 11/23/2015 04:42 PM, Filip Škola wrote:

Sending updated patch.

F.

On Mon, 23 Nov 2015 14:59:34 +0100
Filip Škola <fsk...@redhat.com> wrote:


Found couple of issues (broke some dependencies).

NACK

F.

On Fri, 20 Nov 2015 13:56:36 +0100
Filip Škola <fsk...@redhat.com> wrote:


Another one.

F.

Hi, the tests look good. Few remarks, though.

1. Please, use the shortes copyright notice in new modules.

   #
   # Copyright (C) 2015  FreeIPA Contributors see COPYING for
license #

2. The tests `test_group_remove_group_from_protected_group` and
`test_group_full_set_of_objectclass_not_available_post_detach`
were not ported. Please, include them in the patch.

Also, for less hassle, please rebase your patches on top of
freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch
Which changes the location of tracker implementations and prevents
circular imports.

Thanks.



Hi,

these cases are there, in corresponding classes. They are marked
with the original comments. (However I can move them to separate
class if desirable.)

The copyright notice is changed. Also included a few changes in the
test with user without private group.

Filip

NACK

linter:
* Module tracker.group_plugin
ipatests/test_xmlrpc/tracker/group_plugin.py:257:
[E0102(function-redefined), GroupTracker.check_remove_member] method
already defined line 253)

Probably a leftover after the rebase made on top of my patch. Please
fix it. You can check youch changes by make-lint script before
sending them.

Thanks



Hi,

I learned to use make-lint!

Thanks,
F.


Hello,

NACK, pylint doesn't seem to like the way the fixtures are imported 
(pytest does a lot of runtime magic) [1].
One possible solution would be [2]. Though, I don't think this would be 
a good idea in our environment. I suggest to create the fixtures on per 
module basis.



[1]: http://fpaste.org/311949/53118942/
[2]: 
https://pytest.org/latest/fixture.html#using-fixtures-from-classes-modules-or-projects


--
Milan Kubik

--
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 0003] Refactor test_replace

[Milan Kubík]

On 12/04/2015 11:29 AM, Filip Škola wrote:

On Fri, 4 Dec 2015 10:08:40 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 12/04/2015 10:04 AM, Filip Škola wrote:

Hi,

sending rather short one this time.

F.

NACK, UserTracker is implemented in
ipatests.test_xmlrpc.tracker.user_plugin.


Ah, sorry for this.


F.

Hi,
the tests do not work. Similar problems to test_attr. There are some 
problems with the expected and actual results. NACK.


--
Milan Kubik

--
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 0029, 0030] fixes for install tasks in integration tests

[Milan Kubík]

On 01/07/2016 09:36 AM, Milan Kubík wrote:

0029: Add 10.in-addr.arpa. zone to ipa
0030: If the IP addresses in the topology are resolvable, do not add 
them to master.





Hi. I'm dropping 0029 for now. 0030 gets an update.

--
Milan Kubik

From df95013b33fa1954cc92425fb7e6ea8dc6ac7e15 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Wed, 6 Jan 2016 16:18:17 +0100
Subject: [PATCH 1/2] ipatests: Make the A record for hosts in topology
 conditional

---
 ipatests/test_integration/tasks.py | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index e7984f35fc6b3d3fec93f303b1022136c325db71..346c96ea31185251209f471d1a626b9958925698 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -37,7 +37,8 @@ from ipapython.ipa_log_manager import log_mgr
 from ipatests.test_integration import util
 from ipatests.test_integration.env_config import env_to_script
 from ipatests.test_integration.host import Host
-from ipalib.util import get_reverse_zone_default
+from ipalib import errors
+from ipalib.util import get_reverse_zone_default, verify_host_resolvable
 from ipalib.constants import DOMAIN_SUFFIX_NAME
 from ipalib.constants import DOMAIN_LEVEL_0
 
@@ -892,7 +893,13 @@ def add_a_records_for_hosts_in_master_domain(master):
 for host in master.domain.hosts:
 # We don't need to take care of the zone creation since it is master
 # domain
-add_a_record(master, host)
+try:
+verify_host_resolvable(host.hostname, log)
+log.debug("The host (%s) is resolvable." % host.domain.name)
+except errors.DNSNotARecordError:
+log.debug("Hostname (%s) does not have A/ record. Adding new one.",
+ master.hostname)
+add_a_record(master, host)
 
 
 def add_a_record(master, host):
-- 
2.7.0

-- 
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 0029, 0030] fixes for install tasks in integration tests

[Milan Kubík]

On 01/07/2016 10:12 AM, Martin Basti wrote:



On 07.01.2016 10:03, Oleg Fayans wrote:

Hi Milan,

There is already a more generic method in tasks.py, that does the same:
prepare_reverse_zone
You can just call it from apply_common_fixes. That way, it will work not
only in 10.0.0.0/16 networks.


I'll use this one then.

On 01/07/2016 09:36 AM, Milan Kubík wrote:

0029: Add 10.in-addr.arpa. zone to ipa
0030: If the IP addresses in the topology are resolvable, do not add
them to master.



How about other private subnets? What if I run test under 
172.16.1.0/24 range, or 192.168.1.0/24?



Good point. Would introducing a new option into config be a good idea?

--
Milan Kubik

--
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] [patch 0027] ipatests: Roll back the forwarder config after a test case

[Milan Kubík]

Patch attached.

--
Milan Kubik

From 66db35cf2a315f0a58e002b06f0fcc5bd915a5c5 Mon Sep 17 00:00:00 2001
From: Milan Kubik 
Date: Thu, 17 Dec 2015 14:17:22 +0100
Subject: [PATCH] ipatests: Roll back the forwarder config after a test case

---
 ipatests/test_xmlrpc/test_dns_plugin.py | 9 +
 1 file changed, 9 insertions(+)

diff --git a/ipatests/test_xmlrpc/test_dns_plugin.py b/ipatests/test_xmlrpc/test_dns_plugin.py
index 5f692528eb9a5ae0dc488c663ab43cc47ffd29b3..cba6b001053e6a5c708e9d4359119ad071d70280 100644
--- a/ipatests/test_xmlrpc/test_dns_plugin.py
+++ b/ipatests/test_xmlrpc/test_dns_plugin.py
@@ -1768,6 +1768,15 @@ class test_dns(Declarative):
 },
 ),
 
+dict(
+desc='Update global DNS settings - rollback',
+command=('dnsconfig_mod', [], {'idnsforwarders' : None,}),
+expected={
+'value': None,
+'summary': u'Global DNS configuration is empty',
+'result': {},
+},
+),
 
 dict(
 desc='Try to add invalid allow-query to zone %r' % zone1,
-- 
2.5.0

-- 
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] [patch 0026] ipatests: replace the test-example.com domain in tests

[Milan Kubík]

Applies to ipa-4-3 and master. Reason in commit message.

--
Milan Kubik

From 912ecd6c48149696eb8ca3c60644e5b24af6e534 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Wed, 16 Dec 2015 16:27:34 +0100
Subject: [PATCH] ipatests: replace the test-example.com domain in tests

Latest DNS patches introduced checks for the added zones.
If a zone exists, the add fails if not forced. The domain
test-example.com is resolvable thus causing errors in the test.

Also adds missing __init__.py to the ipatests.test_cmdline package.
---
 ipatests/test_cmdline/__init__.py |  3 +++
 ipatests/test_cmdline/test_cli.py | 57 ---
 2 files changed, 33 insertions(+), 27 deletions(-)
 create mode 100644 ipatests/test_cmdline/__init__.py

diff --git a/ipatests/test_cmdline/__init__.py b/ipatests/test_cmdline/__init__.py
new file mode 100644
index ..9da42e7b4d782ef596e8fda080b6c1994b901866
--- /dev/null
+++ b/ipatests/test_cmdline/__init__.py
@@ -0,0 +1,3 @@
+#
+# Copyright (C) 2015  FreeIPA Contributors see COPYING for license
+#
diff --git a/ipatests/test_cmdline/test_cli.py b/ipatests/test_cmdline/test_cli.py
index c2203e68f1f90505a457de5cdf17f2716cbcaa0d..ddc4c71a89a245bf2066f6ec0831cecdc3255d35 100644
--- a/ipatests/test_cmdline/test_cli.py
+++ b/ipatests/test_cmdline/test_cli.py
@@ -14,6 +14,8 @@ import pytest
 if six.PY3:
 unicode = str
 
+TEST_ZONE = u'zoneadd.%(domain)s' % api.env
+
 
 @pytest.mark.tier0
 class TestCLIParsing(object):
@@ -123,9 +125,9 @@ class TestCLIParsing(object):
 all=False)
 
 def test_dnsrecord_add(self):
-self.check_command('dnsrecord-add test-example.com ns --a-rec=1.2.3.4',
+self.check_command('dnsrecord-add %s ns --a-rec=1.2.3.4' % TEST_ZONE,
 'dnsrecord_add',
-dnszoneidnsname=u'test-example.com',
+dnszoneidnsname=TEST_ZONE,
 idnsname=u'ns',
 arecord=u'1.2.3.4',
 structured=False,
@@ -135,33 +137,33 @@ class TestCLIParsing(object):
 
 def test_dnsrecord_del_all(self):
 try:
-self.run_command('dnszone_add', idnsname=u'test-example.com')
+self.run_command('dnszone_add', idnsname=TEST_ZONE)
 except errors.NotFound:
 raise nose.SkipTest('DNS is not configured')
 try:
 self.run_command('dnsrecord_add',
-dnszoneidnsname=u'test-example.com',
+dnszoneidnsname=TEST_ZONE,
 idnsname=u'ns', arecord=u'1.2.3.4', force=True)
 with self.fake_stdin('yes\n'):
-self.check_command('dnsrecord_del test-example.com ns',
+self.check_command('dnsrecord_del %s ns' % TEST_ZONE,
 'dnsrecord_del',
-dnszoneidnsname=u'test-example.com',
+dnszoneidnsname=TEST_ZONE,
 idnsname=u'ns',
 del_all=True,
 structured=False)
 with self.fake_stdin('YeS\n'):
-self.check_command('dnsrecord_del test-example.com ns',
+self.check_command('dnsrecord_del %s ns' % TEST_ZONE,
 'dnsrecord_del',
-dnszoneidnsname=u'test-example.com',
+dnszoneidnsname=TEST_ZONE,
 idnsname=u'ns',
 del_all=True,
 structured=False)
 finally:
-self.run_command('dnszone_del', idnsname=u'test-example.com')
+self.run_command('dnszone_del', idnsname=TEST_ZONE)
 
 def test_dnsrecord_del_one_by_one(self):
 try:
-self.run_command('dnszone_add', idnsname=u'test-example.com')
+self.run_command('dnszone_add', idnsname=TEST_ZONE)
 except errors.NotFound:
 raise nose.SkipTest('DNS is not configured')
 try:
@@ -169,26 +171,26 @@ class TestCLIParsing(object):
u'2 1 FD2693C1EFFC11A8D2BE57229212A04B45663791')
 for record in records:
 self.run_command('dnsrecord_add',
-dnszoneidnsname=u'test-example.com', idnsname=u'ns',
+dnszoneidnsname=TEST_ZONE, idnsname=u'ns',
 sshfprecord=record)
 with self.fake_stdin('no\nyes\nyes\n'):
-self.check_command('dnsrecord_del test-example.com ns',
+self.check_command('dnsrecord_del %s ns' % TEST_ZONE,
 'dnsrecord_del',
-dnszoneidnsname=u'test-example.com',
+dnszoneidnsname=TEST_ZONE,
 idnsname=u'ns',
 del_all=False,
 sshfprecord=records,
 structured=False)
 finally:
-self.run_command('dnszone_del', idnsname=u'test-example.com')
+self.run_command('dnszone_del', 

Re: [Freeipa-devel] Testing FreeIPA 4.3 for GA

[Milan Kubík]

On 12/15/2015 05:04 PM, Aleš Mareček wrote:


- Original Message -

From: "Milan Kubík" <mku...@redhat.com>
To: "Petr Vobornik" <pvobo...@redhat.com>
Cc: "freeipa-devel" <freeipa-devel@redhat.com>, "Ales Marecek" 
<amare...@redhat.com>
Sent: Tuesday, December 15, 2015 4:53:18 PM
Subject: Re: [Freeipa-devel] Testing FreeIPA 4.3 for GA

On 12/15/2015 04:35 PM, Petr Vobornik wrote:

On 12/15/2015 01:05 AM, Petr Vobornik wrote:

Blocking patches for FreeIPA 4.3 were pushed, ipa-4-3 branch was
created. Master branch is ready for 4.4 development.

A build is available for testing in my pvoborni/freeipa-4-3 COPR repo
[1] until the official 4.3 repo is created. The repo contains only this
build. The build is not pure upstream ipa-4-3 branch but rather a build
which will go to Fedora rawhide and official 4.3 copr repo - Fedora
downstream spec with the SELinux workaround applied [2][3].

Known issues:
* https://fedorahosted.org/freeipa/ticket/5469 - requires fix in PKI
* https://bugzilla.redhat.com/show_bug.cgi?id=1289930 - SELinux Policy
update needed for connection check

I have started drafting release page [4].

[1] https://copr.fedoraproject.org/coprs/pvoborni/freeipa-4-3/
[2] https://fedorahosted.org/freeipa/ticket/5533
[3]
http://www.redhat.com/archives/freeipa-devel/2015-December/msg00234.html
[4] http://www.freeipa.org/page/Releases/4.3.0

Copr contains a rebuild with a patch for ticket:
   https://fedorahosted.org/freeipa/ticket/5551

https://copr.fedoraproject.org/coprs/pvoborni/freeipa-4-3/build/147975/

The build has exactly the same version as the one before. Ales, Milan,
do we want to differentiate that somehow?


The job uses fresh install of the rpms. The same version and build
shouldn't be a problem.

+1, I don't see any issue to use the "same" (not-yet-released) version.


--
Milan Kubik


On a second thought, increasing the build number may be useful. I just 
ran into the cyclic import issue again. Until I looked into the rpm I 
wasn't sure if I got the newer packages.


--
Milan Kubik

--
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] Testing FreeIPA 4.3 for GA

[Milan Kubík]

On 12/15/2015 04:35 PM, Petr Vobornik wrote:

On 12/15/2015 01:05 AM, Petr Vobornik wrote:

Blocking patches for FreeIPA 4.3 were pushed, ipa-4-3 branch was
created. Master branch is ready for 4.4 development.

A build is available for testing in my pvoborni/freeipa-4-3 COPR repo
[1] until the official 4.3 repo is created. The repo contains only this
build. The build is not pure upstream ipa-4-3 branch but rather a build
which will go to Fedora rawhide and official 4.3 copr repo - Fedora
downstream spec with the SELinux workaround applied [2][3].

Known issues:
* https://fedorahosted.org/freeipa/ticket/5469 - requires fix in PKI
* https://bugzilla.redhat.com/show_bug.cgi?id=1289930 - SELinux Policy
update needed for connection check

I have started drafting release page [4].

[1] https://copr.fedoraproject.org/coprs/pvoborni/freeipa-4-3/
[2] https://fedorahosted.org/freeipa/ticket/5533
[3]
http://www.redhat.com/archives/freeipa-devel/2015-December/msg00234.html
[4] http://www.freeipa.org/page/Releases/4.3.0


Copr contains a rebuild with a patch for ticket:
  https://fedorahosted.org/freeipa/ticket/5551

https://copr.fedoraproject.org/coprs/pvoborni/freeipa-4-3/build/147975/

The build has exactly the same version as the one before. Ales, Milan, 
do we want to differentiate that somehow?


The job uses fresh install of the rpms. The same version and build 
shouldn't be a problem.


--
Milan Kubik

--
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 0116] CI tests: remove '-p' option from ipa-dns-install calls

[Milan Kubík]

On 12/10/2015 04:35 PM, Martin Babinsky wrote:

See commit message.


Works for me. ACK.


--
Milan Kubik

--
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 0002] Refactor test_group_plugin

[Milan Kubík]

On 12/03/2015 08:15 PM, Filip Škola wrote:

On Mon, 30 Nov 2015 17:18:30 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 11/23/2015 04:42 PM, Filip Škola wrote:

Sending updated patch.

F.

On Mon, 23 Nov 2015 14:59:34 +0100
Filip Škola <fsk...@redhat.com> wrote:


Found couple of issues (broke some dependencies).

NACK

F.

On Fri, 20 Nov 2015 13:56:36 +0100
Filip Škola <fsk...@redhat.com> wrote:


Another one.

F.



Hi, the tests look good. Few remarks, though.

1. Please, use the shortes copyright notice in new modules.

  #
  # Copyright (C) 2015  FreeIPA Contributors see COPYING for
license #

2. The tests `test_group_remove_group_from_protected_group` and
`test_group_full_set_of_objectclass_not_available_post_detach`
were not ported. Please, include them in the patch.

Also, for less hassle, please rebase your patches on top of
freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch
Which changes the location of tracker implementations and prevents
circular imports.

Thanks.




Hi,

these cases are there, in corresponding classes. They are marked with
the original comments. (However I can move them to separate class if
desirable.)

The copyright notice is changed. Also included a few changes in the
test with user without private group.

Filip

NACK

linter:
* Module tracker.group_plugin
ipatests/test_xmlrpc/tracker/group_plugin.py:257: 
[E0102(function-redefined), GroupTracker.check_remove_member] method 
already defined line 253)


Probably a leftover after the rebase made on top of my patch. Please fix 
it. You can check youch changes by make-lint script before sending them.


Thanks

--
Milan Kubik

--
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 0003] Refactor test_replace

[Milan Kubík]

On 12/04/2015 10:04 AM, Filip Škola wrote:

Hi,

sending rather short one this time.

F.
NACK, UserTracker is implemented in 
ipatests.test_xmlrpc.tracker.user_plugin.


--
Milan Kubik

--
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 0002] Refactor test_group_plugin

[Milan Kubík]

On 11/23/2015 04:42 PM, Filip Škola wrote:

Sending updated patch.

F.

On Mon, 23 Nov 2015 14:59:34 +0100
Filip Škola  wrote:


Found couple of issues (broke some dependencies).

NACK

F.

On Fri, 20 Nov 2015 13:56:36 +0100
Filip Škola  wrote:


Another one.

F.







Hi, the tests look good. Few remarks, though.

1. Please, use the shortes copyright notice in new modules.

#
# Copyright (C) 2015  FreeIPA Contributors see COPYING for license
#

2. The tests `test_group_remove_group_from_protected_group` and 
`test_group_full_set_of_objectclass_not_available_post_detach`

were not ported. Please, include them in the patch.

Also, for less hassle, please rebase your patches on top of 
freeipa-mkubik-0025-3-Separated-Tracker-implementations-into-standalone-pa.patch
Which changes the location of tracker implementations and prevents 
circular imports.


Thanks.

--
Milan Kubik

--
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 0025] Separated Tracker implementations into standalone package

[Milan Kubík]

On 11/27/2015 03:31 PM, Milan Kubík wrote:

On 11/23/2015 10:43 AM, Lenka Doudova wrote:

NACK - there's a "typo" in /tracker/user_plugin.py, line 17-18:

def get_user_dn(cn):

return DN(('cn', cn), api.env.container_user, api.env.basedn)


should be

def get_user_dn(uid):

return DN(('uid', uid), api.env.container_user, api.env.basedn)


Some tests may fail because of that.
Lenka


On 11/20/2015 08:54 PM, Aleš Mareček wrote:

Looks good. ACK.

- Original Message -

From: "Milan Kubík" <mku...@redhat.com>
To: "freeipa-devel" <freeipa-devel@redhat.com>
Cc: "Filip Skola" <fsk...@redhat.com>, "Ales Marecek" 
<amare...@redhat.com>

Sent: Friday, November 20, 2015 3:44:30 PM
Subject: [patch 0025] Separated Tracker implementations into 
standalone package


Fixes https://fedorahosted.org/freeipa/ticket/5467
Patches attached.

--
Milan Kubik





Fixed the function and moved it into different module.
Updated patches attached.




Self nack, some imports missing

--
Milan Kubik

-- 
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 0102] update idrange tests to reflect disabled modification of local ID ranges

[Milan Kubík]

On 11/20/2015 04:06 PM, Martin Babinsky wrote:
When I fixed https://fedorahosted.org/freeipa/ticket/4826 I forgot to 
fix the corresponding xmlrpc tests.


This oversight bit me today when I ran in-tree tests on my VM.

Here is the patch that makes idrange tests green and shiny again.


Tests are now passing, ACK

--
Milan Kubik

--
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] [patch 0024] Fix missed module import in ipaserver tests

[Milan Kubík]

--
Milan Kubik

From 7dabe3f38005e1553b28b98a32578390f2bc629b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Thu, 12 Nov 2015 10:39:48 +0100
Subject: [PATCH] ipatests: Fix missed module import in ipaserver tests

---
 ipatests/test_ipaserver/test_ldap.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ipatests/test_ipaserver/test_ldap.py b/ipatests/test_ipaserver/test_ldap.py
index 85e578b12d3d7ae5c98e221bf4b0fdadfd0d..06c1eb4bf88d95032bde35f959b479cf462f62a9 100644
--- a/ipatests/test_ipaserver/test_ldap.py
+++ b/ipatests/test_ipaserver/test_ldap.py
@@ -28,6 +28,7 @@
 import os
 
 import nose
+import pytest
 from nose.tools import assert_raises  # pylint: disable=E0611
 import nss.nss as nss
 
-- 
2.6.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] ipa-4-2 branch is broken !!!

[Milan Kubík]

On 11/12/2015 10:35 AM, Martin Basti wrote:

Lint failed in current ipa-4-2 branch.

=== 


Errors were found during the static code check.

If you are certain that any of the reported errors are false 
positives, please

mark them in the source code according to the pylint documentation.
=== 



* Module ipatests.test_ipaserver.test_ldap
ipatests/test_ipaserver/test_ldap.py:43: [E0602(undefined-variable), 
test_ldap] Undefined variable 'pytest')
ipatests/test_ipaserver/test_ldap.py:154: [E0602(undefined-variable), 
test_LDAPEntry] Undefined variable 'pytest')

Makefile:119: recipe for target 'lint' failed

Fixed by patch 0024.

--
Milan Kubik

--
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] 0001 Refactor test_user_plugin

[Milan Kubík]

On 11/10/2015 12:13 PM, Filip Škola wrote:

Hi,

fixed.

F.

On Tue, 10 Nov 2015 10:52:45 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 11/09/2015 04:35 PM, Filip Škola wrote:

Another patch was applied in the meantime.

Attaching an updated version.

F.

On Mon, 9 Nov 2015 13:35:02 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 11/06/2015 11:32 AM, Filip Škola wrote:
Hi,
the patch doesn't apply.


Please fix this.

  ipatests/test_xmlrpc/test_user_plugin.py:1419:
[E0602(undefined-variable),
TestDeniedBindWithExpiredPrincipal.teardown_class] Undefined variable
'user1')

Also, use the version numbers for your changed patches.





Thanks for the patch. Several issues:

1. Use dict.items instead of dict.iteritems, for python3 compatibility

2. What is the purpose of TestPrepare class? The 'purge' methods do not 
call any ipa commands.
Tracker.make_fixture should be used to make the Tracked resources clean 
themselves up when they're out of scope.


3. Why reference the resources by hardcoded name if they have a fixture 
representation?


4. Rewrite {create,delete}_test_group to a fixture. You may want to use 
different scope (or not).


5. In `def atest_rename_to_invalid_login(self, user):` - use 
pytest.skipif decorator and provide a reason if you must,

do not obfuscate method name in order not to run it.


--
Milan Kubik

--
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] 0001 Refactor test_user_plugin

[Milan Kubík]

On 11/09/2015 04:35 PM, Filip Škola wrote:

Another patch was applied in the meantime.

Attaching an updated version.

F.

On Mon, 9 Nov 2015 13:35:02 +0100
Milan Kubík <mku...@redhat.com> wrote:


On 11/06/2015 11:32 AM, Filip Škola wrote:



Hi,
the patch doesn't apply.


Please fix this.

ipatests/test_xmlrpc/test_user_plugin.py:1419: 
[E0602(undefined-variable), 
TestDeniedBindWithExpiredPrincipal.teardown_class] Undefined variable 
'user1')


Also, use the version numbers for your changed patches.

--
Milan Kubik

--
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 0012-0019] CA ACL tracker and functional test

[Milan Kubík]

On 10/20/2015 02:19 PM, Martin Basti wrote:


NACK



1)

I still see many hardcoded passwords in the code

with change_principal(smime_user, "Secret123"):


For now changed to module variable.



2)

Also the 'alice' username can be extracted to module variable
instead hardcoding


The fixture should take the place of module variables in the tests. 
Changed u'alice' into local variable.
Once we fix the problems with UserTracker, we should store the password 
here as well.


3)

File alice.conf.tmpl can be generalized to be used for more users,
replace alice in template to {username} and in code replace this
variable with alice, also do not forgot rename template to something
more general




Done.










Updated patch set attached.

--
Milan Kubik

From 42df41de0b621392e113140602c8c9fd10c7d719 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Fri, 17 Jul 2015 14:42:23 +0200
Subject: [PATCH 1/6] ipatests: add fuzzy instances for CA ACL DN and RDN

https://fedorahosted.org/freeipa/ticket/57
---
 ipatests/test_xmlrpc/xmlrpc_test.py | 8 
 1 file changed, 8 insertions(+)

diff --git a/ipatests/test_xmlrpc/xmlrpc_test.py b/ipatests/test_xmlrpc/xmlrpc_test.py
index 80638e2efdd9d7ff07fd89688397acb7d44654cd..5880a4c6f2d9bae23f296aa6386f5705f18eea29 100644
--- a/ipatests/test_xmlrpc/xmlrpc_test.py
+++ b/ipatests/test_xmlrpc/xmlrpc_test.py
@@ -77,6 +77,14 @@ fuzzy_sudocmddn = Fuzzy(
 '(?i)ipauniqueid=%s,cn=sudocmds,cn=sudo,%s' % (uuid_re, api.env.basedn)
 )
 
+# Matches caacl dn
+fuzzy_caacldn = Fuzzy(
+'(?i)ipauniqueid=%s,cn=caacls,cn=ca,%s' % (uuid_re, api.env.basedn)
+)
+
+# Matches fuzzy ipaUniqueID DN group (RDN)
+fuzzy_ipauniqueid = Fuzzy('(?i)ipauniqueid=%s' % uuid_re)
+
 # Matches a hash signature, not enforcing length
 fuzzy_hash = Fuzzy('^([a-f0-9][a-f0-9]:)+[a-f0-9][a-f0-9]$', type=six.string_types)
 
-- 
2.6.1

From 57fcb2f7e5e4a30963c698cf41aae0a6bab45d26 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Tue, 30 Jun 2015 17:00:18 +0200
Subject: [PATCH 2/6] ipatests: Add initial CAACLTracker implementation

The patch implements the tracker for CA ACL feature.
The basic CRUD checkers has been implemented. The methods
for adding and removing the association of the resources
with the ACL do not have the check methods. These will be provided
as a separate test suite.

https://fedorahosted.org/freeipa/ticket/57
---
 ipatests/test_xmlrpc/objectclasses.py |   5 +
 ipatests/test_xmlrpc/test_caacl_plugin.py | 378 ++
 2 files changed, 383 insertions(+)
 create mode 100644 ipatests/test_xmlrpc/test_caacl_plugin.py

diff --git a/ipatests/test_xmlrpc/objectclasses.py b/ipatests/test_xmlrpc/objectclasses.py
index 1cd77c7f885fe408d0d9d48fc6d8284900c91b7f..134a08803f3abca1124c4d26274d9e3fc981b941 100644
--- a/ipatests/test_xmlrpc/objectclasses.py
+++ b/ipatests/test_xmlrpc/objectclasses.py
@@ -217,3 +217,8 @@ certprofile = [
 u'top',
 u'ipacertprofile',
 ]
+
+caacl = [
+u'ipaassociation',
+u'ipacaacl'
+]
diff --git a/ipatests/test_xmlrpc/test_caacl_plugin.py b/ipatests/test_xmlrpc/test_caacl_plugin.py
new file mode 100644
index ..6cf835b229f70797e32bcfd2309cfa7be5732f51
--- /dev/null
+++ b/ipatests/test_xmlrpc/test_caacl_plugin.py
@@ -0,0 +1,378 @@
+#
+# Copyright (C) 2015  FreeIPA Contributors see COPYING for license
+#
+
+"""
+Test the `ipalib.plugins.caacl` module.
+"""
+
+import os
+
+import pytest
+
+from ipapython import ipautil
+from ipalib import errors, x509
+from ipapython.dn import DN
+from ipatests.test_xmlrpc.ldaptracker import Tracker
+from ipatests.test_xmlrpc.xmlrpc_test import (XMLRPC_test, fuzzy_caacldn,
+  fuzzy_uuid, fuzzy_ipauniqueid,
+  raises_exact)
+from ipatests.test_xmlrpc import objectclasses
+from ipatests.util import assert_deepequal
+
+
+class CAACLTracker(Tracker):
+"""Tracker class for CA ACL LDAP object.
+
+The class implements methods required by the base class
+to help with basic CRUD operations.
+
+Methods for adding and deleting actual member entries into an ACL
+do not have check methods as these would make the class
+unnecessarily complicated. The checks are implemented as
+a standalone test suite. However, this makes the test crucial
+in debugging more complicated test cases. Since the add/remove
+operation won't be checked right away by the tracker, a problem
+in this operation may propagate into more complicated test case.
+
+It is possible to pass a list of member uids to these methods.
+
+The test uses instances of Fuzzy class to compare results as they
+are in the UUID format. The dn and rdn properties were modified
+to reflect this as well.
+"""
+
+member_keys = {
+u'memberuser_user', u'memberuser_group',
+u'memberhost_host', 

Re: [Freeipa-devel] [PATCH 0012-0019] CA ACL tracker and functional test

[Milan Kubík]

On 10/19/2015 01:38 PM, Martin Basti wrote:



On 16.10.2015 15:43, Milan Kubík wrote:

On 09/30/2015 02:47 PM, Martin Basti wrote:










On 09/24/2015 02:49 PM, Milan Kubík
wrote:



Hi
all,




an update for CA ACL tests!




I, with help from M. Babinsky, managed to find a way how to change
the identity during acceptance cest run, which allows


to test CA ACLs (and perhaps other areas with some form of access
controll).




This allowed me to write a test for CA ACLs and certificate
profiles that checks if the ACL/profile is being used and
enforced.


The first several tests are based on Fraser's blogpost using SMIME
profile [1].




The master and ipa-4-2 branches diverged a bit, so I had to change
two commits when rebasing to ipa-4-2 branch.




Commits should be applied in the order (including rebased patches
I sent in an earlier email):




master:


* 12 - 17




ipa-4-2:


* 18, 13 - 15, 19, 17




For convenience:


patches on top of master:
https://github.com/apophys/freeipa/tree/acl-profile-functional


patches on top of ipa-4-2:
https://github.com/apophys/freeipa/tree/acl-42






[1]:
https://blog-ftweedal.rhcloud.com/2015/08/user-certificates-and-custom-profiles-with-freeipa-4-2/



Cheers,


Milan











NACK



0)

rpm file does not contain test_xmlrpc/data directory, please modify
setup.py.in.



1)

Code contains to much todo for my taste.



2)

Please do not use filter function, use dict comprehension.









Hi,

updated patches and the numbering mess somehow curbed. The patches 
are rebased on top of current master and ipa-4-2.


0) fixed by 0021

1) docs for tracker extended, added more test cases

2) changed


--
Milan Kubik

I have a few comments:

1)
+# TODO: rewrite these into Tracker instances
+@pytest.fixture(scope='class')
+def smime_user(request):
+api.Command.user_add(uid=u'alice', givenname=u'Alice', sn=u'SMIME',
+ userpassword=u'Change123')
+
+unlock_principal_password('alice', 'Change123', 'Secret123')
+
+def fin():
+api.Command.user_del(u'alice')
+request.addfinalizer(fin)
+
+return u'alice'

I do not like hardcoded password value, as this password is used in 
many places in the test, I sugest to use a module variable

Done


2)
+class TestSignWithChangedProfile(XMLRPC_test):
+""" Test to verify that the updated profile is used."""
+pass  # import invalid profile, try to sign, expect fail

IMO something is missing here, a test maybe?


Done by using profile with constraint that CSR cannot meet.

3)
# noqa
Please remove "# noqa" commets from commits


Done.

--
Milan Kubik

From d3599a45ecb7d3bc8e5364fde239769291672ca2 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Fri, 7 Aug 2015 15:54:18 +0200
Subject: [PATCH 3/6] tests: add test to check the default ACL

Also includes basic ACL manipulation and adding
and removing members to/from the acl.

https://fedorahosted.org/freeipa/ticket/57
---
 ipatests/test_xmlrpc/test_caacl_plugin.py | 135 --
 1 file changed, 128 insertions(+), 7 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_caacl_plugin.py b/ipatests/test_xmlrpc/test_caacl_plugin.py
index 6cf835b229f70797e32bcfd2309cfa7be5732f51..33268d6dde115ce040e55eaae08a7fe1299ad112 100644
--- a/ipatests/test_xmlrpc/test_caacl_plugin.py
+++ b/ipatests/test_xmlrpc/test_caacl_plugin.py
@@ -6,20 +6,20 @@
 Test the `ipalib.plugins.caacl` module.
 """
 
-import os
-
 import pytest
 
-from ipapython import ipautil
-from ipalib import errors, x509
-from ipapython.dn import DN
+from ipalib import errors
 from ipatests.test_xmlrpc.ldaptracker import Tracker
 from ipatests.test_xmlrpc.xmlrpc_test import (XMLRPC_test, fuzzy_caacldn,
-  fuzzy_uuid, fuzzy_ipauniqueid,
-  raises_exact)
+  fuzzy_uuid, fuzzy_ipauniqueid)
+
 from ipatests.test_xmlrpc import objectclasses
 from ipatests.util import assert_deepequal
 
+# reuse the fixture
+from ipatests.test_xmlrpc.test_certprofile_plugin import default_profile
+from ipatests.test_xmlrpc.test_stageuser_plugin import StageUserTracker
+
 
 class CAACLTracker(Tracker):
 """Tracker class for CA ACL LDAP object.
@@ -376,3 +376,124 @@ class CAACLTracker(Tracker):
 command = self.make_command(u'caacl_disable', self.name)
 self.attrs.update({u'ipaenabledflag': [u'FALSE']})
 command()
+
+
+@pytest.fixture(scope='class')
+def default_acl(request):
+name = u'hosts_services_caIPAserviceCert'
+tracker = CAACLTracker(name, service_category=u'all', host_category=u'all')
+tracker.track_create()
+tracker.attrs.update(
+{u'ipamembercertprofile_certprofile': [u'caIPAserviceCert']})
+return tracker
+
+
+@pytest.fixture(scope='class')
+def crud_acl(reque

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

[Milan Kubík]

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

Patches attached.

Tests for https://fedorahosted.org/freeipa/ticket/5302



LGTM, ACK.

--
Milan Kubik

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

[Milan Kubík]

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



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.

Perhaps we could use pytest's expected fail (xfail) or skip marker. [1] 
It would prevent test from failing in the report and once the underlying 
issue is fixed, it will raise as an unexpected pass.


It could be used as a temporary solution, once the issue is fixed, we 
would remove the mark from the test. This would probably need some 
workflow to be defined for these cases.


[1]: https://pytest.org/latest/skipping.html

--
Milan Kubik

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

[Milan Kubík]

On 10/09/2015 09:01 AM, Milan Kubík wrote:

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



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.

Perhaps we could use pytest's expected fail (xfail) or skip marker. 
[1] It would prevent test from failing in the report and once the 
underlying issue is fixed, it will raise as an unexpected pass.


It could be used as a temporary solution, once the issue is fixed, we 
would remove the mark from the test. This would probably need some 
workflow to be defined for these cases.


[1]: https://pytest.org/latest/skipping.html

I write faster than I read. The issue at hand here is diffeerent use 
case. :)


--
Milan Kubik

--
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] [patch 0022] ipatests: remove the ipatests specific config from ipaplatform

[Milan Kubík]

To keep the test specific configuration in the ipatest package.

Patch attached.

--
Milan Kubik

From 49701f9775e59bd19bc62295af6ed332f1aa054b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Tue, 6 Oct 2015 14:55:49 +0200
Subject: [PATCH] ipatests: remove the ipatests specific config from
 ipaplatform

Move the test only configuration for Network Manager entirely
into ipatests part of the tree.
---
 ipaplatform/base/paths.py   |  1 -
 ipatests/test_integration/env_config.py |  1 +
 ipatests/test_integration/tasks.py  | 11 ---
 3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index a272143d0053451c017c0df613951cc0e6d52c54..3292cbfdcfde7c96bc4b4d241e5aa8c20534d602 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -354,6 +354,5 @@ class BasePathNamespace(object):
 DB2BAK = '/usr/sbin/db2bak'
 KDCPROXY_CONFIG = '/etc/ipa/kdcproxy/kdcproxy.conf'
 CERTMONGER = '/usr/sbin/certmonger'
-NETWORK_MANAGER_CONFIG_DIR = '/etc/NetworkManager/conf.d'
 
 path_namespace = BasePathNamespace
diff --git a/ipatests/test_integration/env_config.py b/ipatests/test_integration/env_config.py
index d16a3430d04968575583b84a945db4bc7f7b0e93..5c6fd4f7ef0366135ae5ee47e56e668513109fcf 100644
--- a/ipatests/test_integration/env_config.py
+++ b/ipatests/test_integration/env_config.py
@@ -33,6 +33,7 @@ from ipatests.test_integration.config import Config, Domain
 
 TESTHOST_PREFIX = 'TESTHOST_'
 
+IPATEST_NETWORK_MANAGER_CONFIG = '/etc/NetworkManager/conf.d/20-ipatest-unmanaged-resolv.conf'
 
 _SettingInfo = collections.namedtuple('Setting', 'name var_name default')
 _setting_infos = (
diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index c9ecf2645183d5f368694d3446ddf2853de22a2a..355b38992616e364b959ba381c03015046edbda2 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -35,14 +35,13 @@ from ipaplatform.paths import paths
 from ipapython.dn import DN
 from ipapython.ipa_log_manager import log_mgr
 from ipatests.test_integration import util
-from ipatests.test_integration.env_config import env_to_script
+from ipatests.test_integration.env_config import (
+env_to_script, IPATEST_NETWORK_MANAGER_CONFIG)
 from ipatests.test_integration.host import Host
 from ipalib.util import get_reverse_zone_default
 
 log = log_mgr.get_logger(__name__)
 
-IPATEST_NM_CONFIG = '20-ipatest-unmanaged-resolv.conf'
-
 
 def prepare_reverse_zone(host, ip):
 zone = get_reverse_zone_default(ip)
@@ -125,9 +124,8 @@ def modify_nm_resolv_conf_settings(host):
 return
 
 config = "[main]\ndns=none\n"
-path = os.path.join(paths.NETWORK_MANAGER_CONFIG_DIR, IPATEST_NM_CONFIG)
 
-host.put_file_contents(path, config)
+host.put_file_contents(IPATEST_NETWORK_MANAGER_CONFIG, config)
 host.run_command(['systemctl', 'restart', 'NetworkManager'],
  raiseonerr=False)
 
@@ -136,8 +134,7 @@ def undo_nm_resolv_conf_settings(host):
 if not host_service_active(host, 'NetworkManager'):
 return
 
-path = os.path.join(paths.NETWORK_MANAGER_CONFIG_DIR, IPATEST_NM_CONFIG)
-host.run_command(['rm', '-f', path], raiseonerr=False)
+host.run_command(['rm', '-f', IPATEST_NETWORK_MANAGER_CONFIG], raiseonerr=False)
 host.run_command(['systemctl', 'restart', 'NetworkManager'],
  raiseonerr=False)
 
-- 
2.6.1

From 52294d1ec92c4e8340273891b761bef9c22c65ed Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Tue, 6 Oct 2015 14:55:49 +0200
Subject: [PATCH] ipatests: remove the ipatests specific config from
 ipaplatform

Move the test only configuration for Network Manager entirely
into ipatests part of the tree.
---
 ipaplatform/base/paths.py   |  1 -
 ipatests/test_integration/env_config.py |  1 +
 ipatests/test_integration/tasks.py  | 11 ---
 3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index 0d2c4c17769ef643ba2d6c9991d910cf6e00858d..d2fcd8c708c08069f469beb332bfc1b793a8e903 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -355,6 +355,5 @@ class BasePathNamespace(object):
 DB2BAK = '/usr/sbin/db2bak'
 KDCPROXY_CONFIG = '/etc/ipa/kdcproxy/kdcproxy.conf'
 CERTMONGER = '/usr/sbin/certmonger'
-NETWORK_MANAGER_CONFIG_DIR = '/etc/NetworkManager/conf.d'
 
 path_namespace = BasePathNamespace
diff --git a/ipatests/test_integration/env_config.py b/ipatests/test_integration/env_config.py
index 96062bef30ec067faa644dc060af8531f5e899c9..82e412d39e41bd273ec8ae9c43481e93ba6e050e 100644
--- a/ipatests/test_integration/env_config.py
+++ b/ipatests/test_integration/env_config.py
@@ -35,6 +35,7 @@ from ipatests.test_integration.config import Config, Domain
 
 TESTHOST_PREFIX = 'TESTHOST_'
 

Re: [Freeipa-devel] [patch 0022] ipatests: remove the ipatests specific config from ipaplatform

[Milan Kubík]

On 10/06/2015 03:01 PM, Milan Kubík wrote:

To keep the test specific configuration in the ipatest package.

Patch attached.




Self NACK. This is not necessary in upstream.

--
Milan Kubik

-- 
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] [patch 0021] Include ipatests/test_xmlrpc/data directory into distribution

[Milan Kubík]

Adds ipatests/test_xmlrpc/data directory and its content into package. 
The files are needed for certprofile (and CA ACL) tests.

Patch attached.

--
Milan Kubik

From 2e7e84f27590efd7b5097551104f723e018c722f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Thu, 1 Oct 2015 15:55:19 +0200
Subject: [PATCH 1/2] Include ipatests/test_xmlrpc/data directory into
 distribution.

---
 ipatests/setup.py.in | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ipatests/setup.py.in b/ipatests/setup.py.in
index 90390c06d191cfbc85c385af9b3af6768826703e..afc77ad564eca3e7ad5f488662d32c54d11ea189 100644
--- a/ipatests/setup.py.in
+++ b/ipatests/setup.py.in
@@ -84,6 +84,7 @@ def setup_package():
 'ipatests.test_integration': ['scripts/*'],
 'ipatests.test_pkcs10': ['*.csr'],
 "ipatests.test_ipaserver": ['data/*'],
+'ipatests.test_xmlrpc': ['data/*'],
 }
 )
 finally:
-- 
2.6.0

-- 
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] Proper fix for ticket 5306

[Milan Kubík]

On 10/02/2015 04:11 PM, Martin Basti wrote:



On 10/01/2015 02:48 PM, Martin Basti wrote:



On 10/01/2015 02:43 PM, Oleg Fayans wrote:

Hi Martin,

On 10/01/2015 11:18 AM, Martin Basti wrote:



On 09/30/2015 01:24 PM, Martin Basti wrote:



On 09/30/2015 12:19 PM, Oleg Fayans wrote:



On 09/30/2015 11:46 AM, Petr Spacek wrote:

On 29.9.2015 09:12, Oleg Fayans wrote:

+def prepare_reverse_zone(host, ip):
+zone = get_reverse_zone_default(ip)
+host.run_command(["ipa",
+  "dnszone-add",
+  zone,
+  "--name-from-ip=%s" % ip], 
raiseonerr=False)


There is probably no point in specifying --name-from-ip because you
did that
already by calling get_reverse_zone_default(ip).


Agree. Fixed



Anyway, I'm not sure that this

+ prepare_reverse_zone(master, replica.ip)

will not break if the reverse zone already exists (think about case
where two
or more replicas are in the same subnet).


That's why I am using the raiseonerr=False here.



I did not test the code, I simply do not have time for it right 
now.







 LGTM, I will test it soon, but it needs rebase for ipa-4-2 branch



ACK, please send rebased version for ipa-4-2


Here it is


Pushed to ipa-4-2: c898c968d3979a0d8c2fe0db8e125dfc2268eba0
Pushed to master: 03d696f224642c1c4c4f1a434fecefd1c6270e37



In rebased patch for ipa-4-2 you removed import for function and I 
didn't noticed that.

This breaks builds of ipa-4-2.

Patch that fix this attached.




ACK

--
Milan Kubik

-- 
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] [patch 0020] ipatests: configure Network Manager not to manage resolv.conf

[Milan Kubík]

Fixes https://fedorahosted.org/freeipa/ticket/5331

Patch attached.
From 4200b386058489f8ad73ee2d2f7eed582dea70b5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= 
Date: Fri, 25 Sep 2015 21:09:24 +0200
Subject: [PATCH] ipatests: configure Network Manager not to manage resolv.conf

For the duration of the test, makes resolv.conf unmanaged.

https://fedorahosted.org/freeipa/ticket/5331
---
 ipaplatform/base/paths.py  |  2 +-
 ipatests/test_integration/tasks.py | 17 +
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index 215caf90ea1ca4e5db8f43f8f09002ce5d5cd280..a272143d0053451c017c0df613951cc0e6d52c54 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -354,6 +354,6 @@ class BasePathNamespace(object):
 DB2BAK = '/usr/sbin/db2bak'
 KDCPROXY_CONFIG = '/etc/ipa/kdcproxy/kdcproxy.conf'
 CERTMONGER = '/usr/sbin/certmonger'
-
+NETWORK_MANAGER_CONFIG_DIR = '/etc/NetworkManager/conf.d'
 
 path_namespace = BasePathNamespace
diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index 06049d4ae01332e0af4d8775b745342406fc868d..d3d46682d90e749ebf33b5f673217940debf7f1c 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -40,6 +40,7 @@ from ipatests.test_integration.host import Host
 
 log = log_mgr.get_logger(__name__)
 
+IPATEST_NM_CONFIG = '20-ipatest-unmanaged-resolv.conf'
 
 def check_arguments_are(slice, instanceof):
 """
@@ -74,6 +75,7 @@ def prepare_host(host):
 def apply_common_fixes(host):
 fix_etc_hosts(host)
 fix_hostname(host)
+modify_nm_resolv_conf_settings(host)
 fix_resolv_conf(host)
 
 
@@ -119,6 +121,20 @@ def fix_hostname(host):
 host.run_command('hostname > %s' % ipautil.shell_quote(backupname))
 
 
+def modify_nm_resolv_conf_settings(host):
+config = "[main]\ndns=none\n"
+path = os.path.join(paths.NETWORK_MANAGER_CONFIG_DIR, IPATEST_NM_CONFIG)
+
+host.put_file_contents(path, config)
+host.run_command(['systemctl', 'restart', 'NetworkManager'], raiseonerr=False)
+
+
+def undo_nm_resolv_conf_settings(host):
+path = os.path.join(paths.NETWORK_MANAGER_CONFIG_DIR, IPATEST_NM_CONFIG)
+host.run_command(['rm', '-f', path], raiseonerr=False)
+host.run_command(['systemctl', 'restart', 'NetworkManager'], raiseonerr=False)
+
+
 def fix_resolv_conf(host):
 backup_file(host, paths.RESOLV_CONF)
 lines = host.get_file_contents(paths.RESOLV_CONF).splitlines()
@@ -147,6 +163,7 @@ def fix_apache_semaphores(master):
 def unapply_fixes(host):
 restore_files(host)
 restore_hostname(host)
+undo_nm_resolv_conf_settings(host)
 
 # Clean up the test directory
 host.run_command(['rm', '-rvf', host.config.test_dir])
-- 
2.6.0

-- 
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 0020] ipatests: configure Network Manager not to manage resolv.conf

[Milan Kubík]

On 10/01/2015 10:06 AM, Milan Kubík wrote:

Fixes https://fedorahosted.org/freeipa/ticket/5331

Patch attached.



Patch for ipa-4-2 branch.

Milan
From 5d19b29474b577688910a60fbc5efdf38ff6c455 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Fri, 25 Sep 2015 21:09:24 +0200
Subject: [PATCH] ipatests: configure Network Manager not to manage resolv.conf

For the duration of the test, makes resolv.conf unmanaged.

https://fedorahosted.org/freeipa/ticket/5331
---
 ipaplatform/base/paths.py  |  2 +-
 ipatests/test_integration/tasks.py | 17 +
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index 215caf90ea1ca4e5db8f43f8f09002ce5d5cd280..a272143d0053451c017c0df613951cc0e6d52c54 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -354,6 +354,6 @@ class BasePathNamespace(object):
 DB2BAK = '/usr/sbin/db2bak'
 KDCPROXY_CONFIG = '/etc/ipa/kdcproxy/kdcproxy.conf'
 CERTMONGER = '/usr/sbin/certmonger'
-
+NETWORK_MANAGER_CONFIG_DIR = '/etc/NetworkManager/conf.d'
 
 path_namespace = BasePathNamespace
diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index f579f286826f749a8c5f8433f2a8bf7348664ba9..d188adbd4d7b3af7dd82b964917770c5afda7a96 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -40,6 +40,7 @@ from ipatests.test_integration.host import Host
 
 log = log_mgr.get_logger(__name__)
 
+IPATEST_NM_CONFIG = '20-ipatest-unmanaged-resolv.conf'
 
 def prepare_host(host):
 if isinstance(host, Host):
@@ -56,6 +57,7 @@ def prepare_host(host):
 def apply_common_fixes(host):
 fix_etc_hosts(host)
 fix_hostname(host)
+modify_nm_resolv_conf_settings(host)
 fix_resolv_conf(host)
 
 
@@ -101,6 +103,20 @@ def fix_hostname(host):
 host.run_command('hostname > %s' % ipautil.shell_quote(backupname))
 
 
+def modify_nm_resolv_conf_settings(host):
+config = "[main]\ndns=none\n"
+path = os.path.join(paths.NETWORK_MANAGER_CONFIG_DIR, IPATEST_NM_CONFIG)
+
+host.put_file_contents(path, config)
+host.run_command(['systemctl', 'restart', 'NetworkManager'], raiseonerr=False)
+
+
+def undo_nm_resolv_conf_settings(host):
+path = os.path.join(paths.NETWORK_MANAGER_CONFIG_DIR, IPATEST_NM_CONFIG)
+host.run_command(['rm', '-f', path], raiseonerr=False)
+host.run_command(['systemctl', 'restart', 'NetworkManager'], raiseonerr=False)
+
+
 def fix_resolv_conf(host):
 backup_file(host, paths.RESOLV_CONF)
 lines = host.get_file_contents(paths.RESOLV_CONF).splitlines()
@@ -128,6 +144,7 @@ def fix_apache_semaphores(master):
 def unapply_fixes(host):
 restore_files(host)
 restore_hostname(host)
+undo_nm_resolv_conf_settings(host)
 
 # Clean up the test directory
 host.run_command(['rm', '-rvf', host.config.test_dir])
-- 
2.6.0

-- 
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 0020] ipatests: configure Network Manager not to manage resolv.conf

[Milan Kubík]

On 10/01/2015 12:39 PM, Alexander Bokovoy wrote:

On Thu, 01 Oct 2015, Milan Kubík wrote:

On 10/01/2015 11:23 AM, Martin Basti wrote:



On 10/01/2015 10:18 AM, Milan Kubík wrote:

On 10/01/2015 10:06 AM, Milan Kubík wrote:

Fixes https://fedorahosted.org/freeipa/ticket/5331

Patch attached.



Patch for ipa-4-2 branch.

Milan



NACK

http://fpaste.org/273499/43691381/

I disagree. From [1]:

   Fedora now by default relies on NetworkManager for network 
configuration. This is the case also for minimal installations and 
server installations.


The cloud image, which you are probably using does not have 
NetworkManager installed. I think we can rely on the default here and 
assume

NetworkManager is present.

If you and the list disagree, I will make the fix depend on 
NetworkManager's presence.
Nitpick: there can be other services that manage (and rewrite) 
resolv.conf.

While other tools may do it, please make it so that your fix only
activated if NetworkManager is active.

We'll get to other services once we'll encounter them.

Ok. Now I only apply the config and restart NetworkManager when it is 
already running. I don't check if the service is enabled.


--
Milan Kubik

From c41e5941c7d24575495b8064f6412afcecab55ff Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Fri, 25 Sep 2015 21:09:24 +0200
Subject: [PATCH] ipatests: configure Network Manager not to manage resolv.conf

For the duration of the test, makes resolv.conf unmanaged.
If NetworkManager is not running, nothing is changed.

https://fedorahosted.org/freeipa/ticket/5331
---
 ipaplatform/base/paths.py  |  2 +-
 ipatests/test_integration/tasks.py | 36 
 2 files changed, 37 insertions(+), 1 deletion(-)

diff --git a/ipaplatform/base/paths.py b/ipaplatform/base/paths.py
index 215caf90ea1ca4e5db8f43f8f09002ce5d5cd280..a272143d0053451c017c0df613951cc0e6d52c54 100644
--- a/ipaplatform/base/paths.py
+++ b/ipaplatform/base/paths.py
@@ -354,6 +354,6 @@ class BasePathNamespace(object):
 DB2BAK = '/usr/sbin/db2bak'
 KDCPROXY_CONFIG = '/etc/ipa/kdcproxy/kdcproxy.conf'
 CERTMONGER = '/usr/sbin/certmonger'
-
+NETWORK_MANAGER_CONFIG_DIR = '/etc/NetworkManager/conf.d'
 
 path_namespace = BasePathNamespace
diff --git a/ipatests/test_integration/tasks.py b/ipatests/test_integration/tasks.py
index f579f286826f749a8c5f8433f2a8bf7348664ba9..db10a52e8a68b0104cf6f4c8b29b25a655fa742a 100644
--- a/ipatests/test_integration/tasks.py
+++ b/ipatests/test_integration/tasks.py
@@ -40,6 +40,8 @@ from ipatests.test_integration.host import Host
 
 log = log_mgr.get_logger(__name__)
 
+IPATEST_NM_CONFIG = '20-ipatest-unmanaged-resolv.conf'
+
 
 def prepare_host(host):
 if isinstance(host, Host):
@@ -56,6 +58,7 @@ def prepare_host(host):
 def apply_common_fixes(host):
 fix_etc_hosts(host)
 fix_hostname(host)
+modify_nm_resolv_conf_settings(host)
 fix_resolv_conf(host)
 
 
@@ -101,6 +104,38 @@ def fix_hostname(host):
 host.run_command('hostname > %s' % ipautil.shell_quote(backupname))
 
 
+def host_service_active(host, service):
+res = host.run_command(['systemctl', 'is-active', '--quiet', service],
+   raiseonerr=False)
+
+if res.returncode == 0:
+return True
+else:
+return False
+
+
+def modify_nm_resolv_conf_settings(host):
+if not host_service_active(host, 'NetworkManager'):
+return
+
+config = "[main]\ndns=none\n"
+path = os.path.join(paths.NETWORK_MANAGER_CONFIG_DIR, IPATEST_NM_CONFIG)
+
+host.put_file_contents(path, config)
+host.run_command(['systemctl', 'restart', 'NetworkManager'],
+ raiseonerr=False)
+
+
+def undo_nm_resolv_conf_settings(host):
+if not host_service_active(host, 'NetworkManager'):
+return
+
+path = os.path.join(paths.NETWORK_MANAGER_CONFIG_DIR, IPATEST_NM_CONFIG)
+host.run_command(['rm', '-f', path], raiseonerr=False)
+host.run_command(['systemctl', 'restart', 'NetworkManager'],
+ raiseonerr=False)
+
+
 def fix_resolv_conf(host):
 backup_file(host, paths.RESOLV_CONF)
 lines = host.get_file_contents(paths.RESOLV_CONF).splitlines()
@@ -128,6 +163,7 @@ def fix_apache_semaphores(master):
 def unapply_fixes(host):
 restore_files(host)
 restore_hostname(host)
+undo_nm_resolv_conf_settings(host)
 
 # Clean up the test directory
 host.run_command(['rm', '-rvf', host.config.test_dir])
-- 
2.6.0

From fb8ab435c71f08c6fcb9cc8f4a983a278824de44 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Milan=20Kub=C3=ADk?= <mku...@redhat.com>
Date: Fri, 25 Sep 2015 21:09:24 +0200
Subject: [PATCH] ipatests: configure Network Manager not to manage resolv.conf

For the duration of the test, makes resolv.conf unmanaged.
If NetworkManager is not running, nothing is changed.

https://fedorahosted.org/freeipa/ticket/5331
---
 ipaplatform/base/paths.py  |  2 +-