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

2016-07-15 Thread Petr Vobornik 
On 07/14/2016 03:11 PM, Milan Kubík wrote:
> On 07/14/2016 11:43 AM, Lenka Doudova wrote:
>>

>>>
>>>
>> Resending the complete patch set.
>> L.
>>
>>
> 
> Thanks, ACK.
> 
> -- 
> Milan Kubik
> 

master:
* 0f9a5ce6b4c533647b8894f516e34bea8184f1b8 Tests: Tracker class for services
* dcdbbb975927a24ec05f7addefd59c71823a57c2 Tests: Authentication
indicators xmlrpc tests
* aab861142d3aec503ebae4779fbfa1858e20f451 Tests: Authentication
indicators integration tests

-- 
Petr Vobornik

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

2016-07-14 Thread 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

2016-07-14 Thread Lenka Doudova 



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.
From aaf2afa849b3156603e057d99fe7f31247b08725 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Wed, 15 Jun 2016 13:40:00 +0200
Subject: [PATCH] Tests: Tracker class for services

Provides basic service tracker, so far for purposes of [1].
Tracker is not complete, some methods will need to be added in case of service test refactoring.

[1] https://fedorahosted.org/freeipa/ticket/433
---
 ipatests/test_xmlrpc/tracker/service_plugin.py | 152 +
 1 file changed, 152 insertions(+)
 create mode 100644 ipatests/test_xmlrpc/tracker/service_plugin.py

diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py
new file mode 100644
index ..89c27e82161b70a77ac4e45ffd8b60d3da657163
--- /dev/null
+++ b/ipatests/test_xmlrpc/tracker/service_plugin.py
@@ -0,0 +1,152 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+#
+
+import six
+
+from ipalib import api
+from ipatests.test_xmlrpc.tracker.base import Tracker
+from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid
+from ipatests.test_xmlrpc import objectclasses
+from ipatests.util import assert_deepequal
+from ipapython.dn import DN
+
+if six.PY3:
+unicode = str
+
+
+class ServiceTracker(Tracker):
+"""
+Tracker class for service plugin
+
+So far does not include methods for these commands:
+service-add-host
+service-remove-host
+service-allow-retrieve-keytab
+service-disallow-retrieve-keytab
+service-allow-create-keytab
+service-disallow-create-keytab
+service-disable
+service-add-cert
+service-remove-cert
+"""
+
+retrieve_keys = {
+u'dn', u'krbprincipalname', u'usercertificate', u'has_keytab',
+u'ipakrbauthzdata', u'ipaallowedtoperform', u'subject',
+u'managedby', u'serial_number', u'serial_number_hex', u'issuer',
+u'valid_not_before', u'valid_not_after', u'md5_fingerprint',
+

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

2016-07-14 Thread Lenka Doudova 



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

From aaf2afa849b3156603e057d99fe7f31247b08725 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Wed, 15 Jun 2016 13:40:00 +0200
Subject: [PATCH] Tests: Tracker class for services

Provides basic service tracker, so far for purposes of [1].
Tracker is not complete, some methods will need to be added in case of service test refactoring.

[1] https://fedorahosted.org/freeipa/ticket/433
---
 ipatests/test_xmlrpc/tracker/service_plugin.py | 152 +
 1 file changed, 152 insertions(+)
 create mode 100644 ipatests/test_xmlrpc/tracker/service_plugin.py

diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py
new file mode 100644
index ..89c27e82161b70a77ac4e45ffd8b60d3da657163
--- /dev/null
+++ b/ipatests/test_xmlrpc/tracker/service_plugin.py
@@ -0,0 +1,152 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+#
+
+import six
+
+from ipalib import api
+from ipatests.test_xmlrpc.tracker.base import Tracker
+from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid
+from ipatests.test_xmlrpc import objectclasses
+from ipatests.util import assert_deepequal
+from ipapython.dn import DN
+
+if six.PY3:
+unicode = str
+
+
+class ServiceTracker(Tracker):
+"""
+Tracker class for service plugin
+
+So far does not include methods for these commands:
+service-add-host
+service-remove-host
+service-allow-retrieve-keytab
+service-disallow-retrieve-keytab
+service-allow-create-keytab
+service-disallow-create-keytab
+service-disable
+service-add-cert
+service-remove-cert
+"""
+
+retrieve_keys = {
+u'dn', u'krbprincipalname', u'usercertificate', u'has_keytab',
+u'ipakrbauthzdata', u'ipaallowedtoperform', u'subject',
+u'managedby', u'serial_number', u'serial_number_hex', u'issuer',
+u'valid_not_before', u'valid_not_after', u'md5_fingerprint',
+u'sha1_fingerprint', u'krbprincipalauthind', u'managedby_host',
+u'krbcanonicalname'}

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

2016-07-14 Thread Lenka Doudova 



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
From 1e69ce92786668d80507a02b03a3fae40d7bbbd2 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Wed, 15 Jun 2016 13:40:00 +0200
Subject: [PATCH 1/2] Tests: Tracker class for services

Provides basic service tracker, so far for purposes of [1].
Tracker is not complete, some methods will need to be added in case of service test refactoring.

[1] https://fedorahosted.org/freeipa/ticket/433
---
 ipatests/test_xmlrpc/tracker/service_plugin.py | 173 +
 1 file changed, 173 insertions(+)
 create mode 100644 ipatests/test_xmlrpc/tracker/service_plugin.py

diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py
new file mode 100644
index ..a7eb4303241e136ec88c51642f51fe703b16de7e
--- /dev/null
+++ b/ipatests/test_xmlrpc/tracker/service_plugin.py
@@ -0,0 +1,173 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+#
+
+import six
+
+from ipalib import api
+from ipatests.test_xmlrpc.tracker.base import Tracker
+from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid
+from ipatests.test_xmlrpc import objectclasses
+from ipatests.util import assert_deepequal
+from ipapython.dn import DN
+
+if six.PY3:
+unicode = str
+
+
+class ServiceTracker(Tracker):
+"""
+Tracker class for service plugin
+
+So far does not include methods for these commands:
+service-add-host
+service-remove-host
+service-allow-retrieve-keytab
+service-disallow-retrieve-keytab
+service-allow-create-keytab
+service-disallow-create-keytab
+service-disable
+service-add-cert
+service-remove-cert
+"""
+
+retrieve_keys = {
+u'dn', u'krbprincipalname', u'usercertificate', u'has_keytab',
+u'ipakrbauthzdata', u'ipaallowedtoperform', u'subject',
+u'managedby', u'serial_number', u'serial_number_hex', u'issuer',
+u'valid_not_before', u'valid_not_after', u'md5_fingerprint',
+u'sha1_fingerprint', u'krbprincipalauthind', u'managedby_host',
+u'krbcanonicalname'}
+retrieve_all_keys = retrieve_keys | {
+u'ipaKrbPrincipalAlias', u'ipaUniqueID', u'krbExtraData',
+

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

2016-07-13 Thread 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

2016-07-11 Thread Lenka Doudova 



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
From 52c8cc1f6ebe6db519bfaaf7fa3c5863d1f4d6d8 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Wed, 15 Jun 2016 13:40:00 +0200
Subject: [PATCH 1/2] Tests: Tracker class for services

Provides basic service tracker, so far for purposes of [1].
Tracker is not complete, some methods will need to be added in case of service test refactoring.

[1] https://fedorahosted.org/freeipa/ticket/433
---
 ipatests/test_xmlrpc/tracker/service_plugin.py | 172 +
 1 file changed, 172 insertions(+)
 create mode 100644 ipatests/test_xmlrpc/tracker/service_plugin.py

diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py
new file mode 100644
index ..54c5dfcf9df009048e376ac6fb41638f9299c06c
--- /dev/null
+++ b/ipatests/test_xmlrpc/tracker/service_plugin.py
@@ -0,0 +1,172 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+#
+
+import six
+
+from ipalib import api
+from ipatests.test_xmlrpc.tracker.base import Tracker
+from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid
+from ipatests.test_xmlrpc import objectclasses
+from ipatests.util import assert_deepequal
+from ipapython.dn import DN
+
+if six.PY3:
+unicode = str
+
+
+class ServiceTracker(Tracker):
+"""
+Tracker class for service plugin
+
+So far does not include methods for these commands:
+service-add-host
+service-remove-host
+service-allow-retrieve-keytab
+service-disallow-retrieve-keytab
+service-allow-create-keytab
+service-disallow-create-keytab
+service-disable
+service-add-cert
+service-remove-cert
+"""
+
+retrieve_keys = {
+u'dn', u'krbprincipalname', u'usercertificate', u'has_keytab',
+u'ipakrbauthzdata', u'ipaallowedtoperform', u'subject',
+u'managedby', u'serial_number', u'serial_number_hex', u'issuer',
+u'valid_not_before', u'valid_not_after', u'md5_fingerprint',
+u'sha1_fingerprint', u'krbprincipalauthind', u'managedby_host',
+u'krbcanonicalname'}
+retrieve_all_keys = retrieve_keys | {
+u'ipaKrbPrincipalAlias', u'ipaUniqueID', u'krbExtraData',
+u'krbLastPwdChange', u'krbLoginFailedCount', u'memberof',
+u'objectClass', u'ipakrbrequirespreauth',
+u'ipakrbokasdelegate'}
+
+create_keys = (retrieve_keys | {u'objectclass', u'ipauniqueid'}) - {
+u'usercertificate', u'has_keytab'}
+update_keys = retrieve_keys - {u'dn'}
+
+def __init__(self, name, host_fqdn, options):
+super(ServiceTracker, self).__init__(default_version=None)
+self._name = "{0}/{1}@{2}".format(name, host_fqdn, api.env.realm)
+self.dn = DN(
+('krbprincipalname', self.name),

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

2016-07-08 Thread 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 0014-0016][Tests] Authentication indicators

2016-07-01 Thread Lenka Doudova 



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
From 7f8626d9b0cb25f9fe12fdb853f5f3af213ce3ad Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Wed, 15 Jun 2016 13:40:00 +0200
Subject: [PATCH] Tests: Tracker class for services

Provides basic service tracker, so far for purposes of [1].
Tracker is not complete, some methods will need to be added in case of service test refactoring.

[1] https://fedorahosted.org/freeipa/ticket/433
---
 ipatests/test_xmlrpc/tracker/service_plugin.py | 173 +
 1 file changed, 173 insertions(+)
 create mode 100644 ipatests/test_xmlrpc/tracker/service_plugin.py

diff --git a/ipatests/test_xmlrpc/tracker/service_plugin.py b/ipatests/test_xmlrpc/tracker/service_plugin.py
new file mode 100644
index ..f56a95fd23438108b2afd5ce3dc8db72b8ba4e95
--- /dev/null
+++ b/ipatests/test_xmlrpc/tracker/service_plugin.py
@@ -0,0 +1,173 @@
+# -*- coding: utf-8 -*-
+#
+# Copyright (C) 2016  FreeIPA Contributors see COPYING for license
+#
+
+import six
+
+from ipalib import api
+from ipatests.test_xmlrpc.tracker.base import Tracker
+from ipatests.test_xmlrpc.xmlrpc_test import fuzzy_uuid
+from ipatests.test_xmlrpc import objectclasses
+from ipatests.util import assert_deepequal
+from ipapython.dn import DN
+
+if six.PY3:
+unicode = str
+
+
+class ServiceTracker(Tracker):
+"""
+Tracker class for service plugin
+
+So far does not include methods for these commands:
+service-add-host
+service-remove-host
+service-allow-retrieve-keytab
+service-disallow-retrieve-keytab
+service-allow-create-keytab
+service-disallow-create-keytab
+service-disable
+service-add-cert
+service-remove-cert
+"""
+
+retrieve_keys = {
+u'dn', u'krbprincipalname', u'usercertificate', u'has_keytab',
+u'ipakrbauthzdata', u'ipaallowedtoperform', u'subject',
+u'managedby', u'serial_number', u'serial_number_hex', u'issuer',
+u'valid_not_before', u'valid_not_after', u'md5_fingerprint',
+u'sha1_fingerprint', u'krbprincipalauthind', u'managedby_host',
+u'krbcanonicalname'}
+retrieve_all_keys = retrieve_keys | {
+u'ipaKrbPrincipalAlias', u'ipaUniqueID', u'krbExtraData',
+u'krbLastPwdChange', u'krbLoginFailedCount', u'memberof',
+u'objectClass', u'ipakrbrequirespreauth',
+u'ipakrbokasdelegate'}
+
+create_keys = (retrieve_keys | {u'objectclass', u'ipauniqueid'}) - {
+u'usercertificate', u'has_keytab'}
+update_keys = retrieve_keys - {u'dn'}
+
+def __init__(self, name, host_fqdn, options):
+super(ServiceTracker, self).__init__(default_version=None)
+self._name = "{0}/{1}@{2}".format(name, host_fqdn, api.env.realm)
+self.dn = DN(
+('krbprincipalname', self.name), api.env.container_service,
+api.env.basedn)
+self.host_fqdn = host_fqdn
+self.options = options
+
+@property
+def name(self):
+return self._name
+
+def make_create_command(self, force=True):
+""" Make function that creates a service """
+return self.make_command('service_add', self.name,
+ force=force, **self.options)
+
+def make_delete_command(self):
+""" Make function that deletes a service """
+return self.make_command('service_del', self.name)
+
+def make_retrieve_command(self, all=False, raw=False):
+""" Make function that retrieves a service """
+return self.make_command('service_show', self.name, all=all)
+
+def

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

2016-07-01 Thread 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 0014-0016][Tests] Authentication indicators

2016-06-22 Thread Lenka Doudova 

Bump for review. Thanks.


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





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