[Freeipa-devel] [PATCH] 377 Use correct service name in cainstance.backup_config

2014-11-21 Thread Jan Cholasta 

Hi,

the attached patch fixes https://fedorahosted.org/freeipa/ticket/4754.

Honza

--
Jan Cholasta
From c1db9d92088436234d2a00c946a8e470d740745b Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Fri, 21 Nov 2014 07:52:24 +
Subject: [PATCH] Use correct service name in cainstance.backup_config

https://fedorahosted.org/freeipa/ticket/4754
---
 ipaserver/install/cainstance.py | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/cainstance.py b/ipaserver/install/cainstance.py
index 6ccbe41..6b4317f 100644
--- a/ipaserver/install/cainstance.py
+++ b/ipaserver/install/cainstance.py
@@ -1861,7 +1861,8 @@ def backup_config(dogtag_constants=None):
 if dogtag_constants is None:
 dogtag_constants = dogtag.configured_constants()
 
-if services.knownservices.dogtag.is_running():
+if services.knownservices[dogtag_constants.SERVICE_NAME].is_running(
+dogtag_constants.PKI_INSTANCE_NAME):
 raise RuntimeError(Dogtag must be stopped when creating backup of %s
% dogtag_constants.CS_CFG_PATH)
 shutil.copy(dogtag_constants.CS_CFG_PATH,
-- 
2.1.0

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0169] Fix: read_ip_address should return CheckedIPAddress instance instead of string

2014-11-21 Thread Jan Cholasta 

Hi,

Dne 20.11.2014 v 17:52 Martin Basti napsal(a):

Ticket: https://fedorahosted.org/freeipa/ticket/4747

Patch attached:


ACK!

Pushed to:
master: 7de424f42541e73ed24a95f1dd90ff4a61e111fa
ipa-4-1: 5b397dced1ef4a1eea7b3636fc71c2b7108a0b25

Honza

--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0286] baseldap: Handle missing parent objects properly in *-find

2014-11-21 Thread Tomas Babej 

On 11/20/2014 04:01 PM, Jan Cholasta wrote:
 Dne 19.11.2014 v 15:12 Tomas Babej napsal(a):

 On 11/19/2014 02:03 PM, Jan Cholasta wrote:
 Dne 19.11.2014 v 13:44 Tomas Babej napsal(a):

 On 11/19/2014 12:51 PM, Martin Kosek wrote:
 On 11/19/2014 12:41 PM, Tomas Babej wrote:
 On 11/19/2014 12:24 PM, Martin Kosek wrote:
 On 11/19/2014 12:03 PM, Tomas Babej wrote:
 Hi,

 When constructing a parent DN in LDAPSearch, we should always
 check that the parent object exists (hence use get_dn_if_exists),
 rather than search on unexistant containers (which can happen
 with get_dn).

 Replaces get_dn calls with get_dn_if_exists in *-find commands
 and makes sure proper error message is raised.

 https://fedorahosted.org/freeipa/ticket/4659
 Doesn't it produce extra LDAP search thus making all our search
 commands
 slower? Is that what we want?
 No it does not make all of our LDAP search slower. It only
 happens for
 the objects that have parent objects, such as idoverrides or
 dnsrecords.
 ... and makes them slower.

 What I was pointing out here is that this is not a issue for ALL
 *-find
 commands (e.g user-find, group-find will not suffer from it), as you
 incorrectly stated.


 Wouldn't it be better to distinguish between LDAP
 search with no results and LDAP search with missing parent DN? The
 reply looks
 different, at least in CLI:
 Up to discussion. We would probably need to introduce a new
 exception,
 like ParentObjectNotFound.

 # search result
 search: 4
 result: 0 Success

 # search result
 search: 4
 result: 32 No such object
 matchedDN: cn=accounts,dc=mkosek-f20,dc=test

 Also, I do not think you can just stop using get_dn(), some
 commands override
 this call to get more complex searches (like host-find searching
 for shortname).
 Look into the get_dn_if_exists, it just wraps around get_dn, so no
 issue
 here. Any custom behaviour is preserved.
 Ah, ok, thanks for info.

 To sum up, I think this is worth changing this behaviour by default,
 ignoring a non-matching value of the parent object is not a correct
 general approach in my opinion.
 Well, that's the question. Whether we would leave DS to validate the
 search
 itself or do all the pre-check ourselves. To me, doing just one LDAP
 search and
 processing the error correctly looks better. But I can live even
 with your
 version then, I will leave the framework guardians like Honza or
 Petr3 to decide.

 +1 on single LDAP search and proper error processing.


 I see now what you're trying to suggest. However, the reason boils
 down to ipaldap.find_entries method not differentiating between a
 LDAP search that returns error code 32 (No such object) and LDAP
 search returning error code 0 (Success), but returning no results.

 In both cases errors.NotFound is raised.

 The reason I did not go this way is that changing the find_entries
 method
 is quite more invasive as this is the method subsenqently called by
 almost
 any command.

 You can always derive the new error (ParentNotFound or whatever) on
 NotFound, so old code won't break.


 Thanks for the suggestsions.

 Attached is a new patch which hooks into find_entries method and
 differentiates between the cases.


 Why are you special casing base scope search?


Since base search is performed only on the entry specified by the DN,
there is no need to differentiate between ContainerNotFound and NotFound.

So the logic is as follows:

subtree search - ContainerNotFound is raised when container does not
exist, NotFound if search provided no results
onelevel search - the same
base search - NotFound is raised if the specified DN does not exist

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH 0287] Re-initialize NSS database after otptoken plugin tests

2014-11-21 Thread Tomas Babej 
Hi,

OTP token tests do not properly reinitialize the NSS db, thus
making subsequent xmlrpc tests fail on SSL cert validation.

Make sure NSS db is re-initalized in the teardown method.

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

Note for reviewers: Requires Petr^3's pytest patchset, which I am
pushing right now.

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 


From 07c02e33035d79c273f6d65a598a59114ba5b23d Mon Sep 17 00:00:00 2001
From: Tomas Babej tba...@redhat.com
Date: Thu, 20 Nov 2014 18:37:57 +0100
Subject: [PATCH] Re-initialize NSS database after otptoken plugin tests

OTP token tests do not properly reinitialize the NSS db, thus
making subsequent xmlrpc tests fail on SSL cert validation.

Make sure NSS db is re-initalized in the teardown method.

https://fedorahosted.org/freeipa/ticket/4748
---
 ipalib/x509.py  | 31 -
 ipatests/test_ipaserver/test_otptoken_import.py |  5 
 2 files changed, 25 insertions(+), 11 deletions(-)

diff --git a/ipalib/x509.py b/ipalib/x509.py
index 88ea415bf2b27760ac478d5d415356d30f6852f8..a87dbf4130c60b1b1daf8bbb2ffb81c208f2529c 100644
--- a/ipalib/x509.py
+++ b/ipalib/x509.py
@@ -89,19 +89,12 @@ def strip_header(pem):
 
 return pem
 
-def load_certificate(data, datatype=PEM, dbdir=None):
+def initialize_nss_database(dbdir=None):
 
-Given a base64-encoded certificate, with or without the
-header/footer, return a request object.
-
-Returns a nss.Certificate type
+Initializes NSS database, if not initialized yet. Uses a proper database
+directory (.ipa/alias or HTTPD_ALIAS_DIR), depending on the value of
+api.env.in_tree.
 
-if type(data) in (tuple, list):
-data = data[0]
-
-if (datatype == PEM):
-data = strip_header(data)
-data = base64.b64decode(data)
 
 if not nss.nss_is_initialized():
 if dbdir is None:
@@ -116,6 +109,22 @@ def load_certificate(data, datatype=PEM, dbdir=None):
 else:
 nss.nss_init(dbdir)
 
+def load_certificate(data, datatype=PEM, dbdir=None):
+
+Given a base64-encoded certificate, with or without the
+header/footer, return a request object.
+
+Returns a nss.Certificate type
+
+if type(data) in (tuple, list):
+data = data[0]
+
+if (datatype == PEM):
+data = strip_header(data)
+data = base64.b64decode(data)
+
+initialize_nss_database(dbdir=dbdir)
+
 return nss.Certificate(buffer(data))
 
 def load_certificate_from_file(filename, dbdir=None):
diff --git a/ipatests/test_ipaserver/test_otptoken_import.py b/ipatests/test_ipaserver/test_otptoken_import.py
index 7ee0754da567087eec2e494ce076fff32c6ae14c..84df0e2a6e5858275a279f4dc10557495e635459 100644
--- a/ipatests/test_ipaserver/test_otptoken_import.py
+++ b/ipatests/test_ipaserver/test_otptoken_import.py
@@ -21,12 +21,17 @@ import os
 import sys
 import nose
 from nss import nss
+from ipalib.x509 import initialize_nss_database
 
 from ipaserver.install.ipa_otptoken_import import PSKCDocument, ValidationError
 
 basename = os.path.join(os.path.dirname(__file__), data)
 
 class test_otptoken_import(object):
+
+def teardown(self):
+initialize_nss_database()
+
 def test_figure3(self):
 doc = PSKCDocument(os.path.join(basename, pskc-figure3.xml))
 assert doc.keyname is None
-- 
1.9.3

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCHES] 0656-0673 Switch the test suite to pytest

2014-11-21 Thread Tomas Babej 

On 11/20/2014 10:12 AM, Petr Viktorin wrote:
 On 11/19/2014 01:11 PM, Tomas Babej wrote:

 On 11/14/2014 09:55 AM, Petr Viktorin wrote:
 On 10/29/2014 04:52 PM, Petr Viktorin wrote:
 On 10/29/2014 01:22 PM, Tomas Babej wrote:

 On 10/27/2014 04:38 PM, Petr Viktorin wrote:
 On 10/15/2014 02:58 PM, Petr Viktorin wrote:
 This almost completes the switch to pytest. There are two missing
 things:
 - the details of test results (--with-xunit) are not read
 correctly by
 Jenkins. I have a theory I'm investigating here.
 - the beakerlib integration is still not ready


 I'll not be available for the rest of the week so I'm sending this
 early, in case someone wants to take a look.

 I've updated (and rearranged) the patches after some more testing.
 Both points above are fixed. Individual plugins are broken out; some
 would be nice to even release independently of IPA. (There is some
 demand for the BeakerLib plugin; for that I'd only need to break the
 dependency on ipa_log_manager.)


 These depend on my patches 0656-0660.


 Thanks for this effort!

  Patch 0656: tests: Use PEP8-compliant setup/teardown method
 names

 There are some references to the old names in the test_ipapython and
 test_install:

   [tbabej@thinkpad7 ipatests]$ git grep setUpClass
   [tbabej@thinkpad7 ipatests]$ git grep tearDownClass
   [tbabej@thinkpad7 ipatests]$ git grep setUp
   test_install/test_updates.py:def setUp(self):
   test_ipapython/test_cookie.py:def setUp(self):
   test_ipapython/test_cookie.py:def setUp(self):
   test_ipapython/test_cookie.py:def setUp(self):
   test_ipapython/test_dn.py:def setUp(self):
   test_ipapython/test_dn.py:def setUp(self):
   test_ipapython/test_dn.py:def setUp(self):
   test_ipapython/test_dn.py:def setUp(self):
   test_ipapython/test_dn.py:def setUp(self):
   [tbabej@thinkpad7 ipatests]$ git grep tearDown
   test_install/test_updates.py:def tearDown(self):

 These are in unittest.testCase. It would be nice to convert those as
 well, but that's for a larger cleanup.

  Patch 0657: tests: Add configuration for pytest

 Shouldn't we rather change the class names?

 Ideally yes, but I don't think renaming most of our test classes would
 be worth the benefit.

  Patch 0658: ipatests.util.ClassChecker: Raise AttributeError in
 get_subcls

 ACK.

  Patch 0659: test_automount_plugin: Fix test ordering

 ACK.

  PATCH 0660: Use setup_class/teardown_class in Declarative tests

   --- a/ipatests/test_ipaserver/test_changepw.py
   +++ b/ipatests/test_ipaserver/test_changepw.py
   @@ -33,8 +33,7 @@
class test_changepw(XMLRPC_test, Unauthorized_HTTP_test):
app_uri = '/ipa/session/change_password'

   -def setup(self):
   -super(test_changepw, self).setup()
   +def setup(cls):
try:
api.Command['user_add'](uid=testuser,
 givenname=u'Test', sn=u'User')
api.Command['passwd'](testuser,
 password=u'old_password')
   @@ -43,12 +42,11 @@ def setup(self):
'Cannot set up test user: %s' % e
)

   -def teardown(self):
   +def teardown(cls):
try:
api.Command['user_del']([testuser])
except errors.NotFound:
pass
   -super(test_changepw, self).teardown()

 The setup/teardown methods are not classmethods, so the name of the
 first argument should remain self.

 Oops, thanks for the catch!

  PATCH 0661: dogtag plugin: Don't use doctest syntax for
 non-doctest
 examples

 ACK.

  PATCH 0662: test_webui: Don't use __init__ for test classes

 I don't think the following change will work:

   -def __init__(self, driver=None, config=None):
   +def setup(self, driver=None, config=None):
self.request_timeout = 30
self.driver = driver
self.config = config
if not config:
self.load_config()
   +self.get_driver().maximize_window()
   +
   +def teardown(self):
   +self.driver.quit()

def load_config(self):

   @@ -161,20 +165,6 @@ def load_config(self):
if 'type' not in c:
c['type'] = DEFAULT_TYPE

   -def setup(self):
   -
   -Test setup
   -
   -if not self.driver:
   -self.driver = self.get_driver()
   -self.driver.maximize_window()
   -
   -def teardown(self):
   -
   -Test clean up
   -
   -self.driver.quit()


 The setup(self) method used to set the self.driver attribute on the
 class instance, now nothing sets it. The setup method should do:

   def setup(self, driver=None, config=None):

Re: [Freeipa-devel] [PATCH] 377 Use correct service name in cainstance.backup_config

2014-11-21 Thread Martin Kosek 
On 11/21/2014 09:11 AM, Jan Cholasta wrote:
 Hi,
 
 the attached patch fixes https://fedorahosted.org/freeipa/ticket/4754.
 
 Honza

ACK, works fine.

Pushed to:
master: 71c4d3e979584e9f841bc8565e69b53fc111c8c6
ipa-4-1: 1b5cd5b2271240526177810f7340400db695aafc
ipa-4-0: 1ef2d080c4d8a0edc20ab96591b03aad5372ae76

Martin

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

[Freeipa-devel] [PATCH] 0031 ipa-restore: Check if directory is provided + better errors.

2014-11-21 Thread David Kupka 

https://fedorahosted.org/freeipa/ticket/4683
--
David Kupka
From f5f5ae55999b2057549306331b07a3c41c0cabeb Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Fri, 21 Nov 2014 06:30:17 -0500
Subject: [PATCH] ipa-restore: Check if directory is provided + better errors.

https://fedorahosted.org/freeipa/ticket/4683
---
 ipaserver/install/ipa_restore.py | 8 +++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index 93f176d302a49319940555a0be3037620143e1f3..c634588b5bc5a87896244166a9f8f7a571b5911f 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -152,6 +152,9 @@ class Restore(admintool.AdminTool):
 else:
 self.backup_dir = dirname
 
+if not os.path.isdir(dirname):
+raise self.option_parser.error(must provide path to backup directory)
+
 if options.gpg_keyring:
 if (not os.path.exists(options.gpg_keyring + '.pub') or
not os.path.exists(options.gpg_keyring + '.sec')):
@@ -213,7 +216,10 @@ class Restore(admintool.AdminTool):
 try:
 dirsrv = services.knownservices.dirsrv
 
-self.read_header()
+try:
+self.read_header()
+except:
+raise admintool.ScriptError('Cannot read backup metadata')
 # These two checks would normally be in the validate method but
 # we need to know the type of backup we're dealing with.
 if (self.backup_type != 'FULL' and not options.data_only and
-- 
2.1.0

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0031 ipa-restore: Check if directory is provided + better errors.

2014-11-21 Thread Tomas Babej 

On 11/21/2014 01:33 PM, David Kupka wrote:
 https://fedorahosted.org/freeipa/ticket/4683


 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel

-self.read_header()
+try:
+self.read_header()
+except:
+raise admintool.ScriptError('Cannot read backup metadata')


Is the bare except clause really necessary?

https://docs.python.org/2.7/howto/doanddont.html#except


-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0286] baseldap: Handle missing parent objects properly in *-find

2014-11-21 Thread Jan Cholasta 

Dne 21.11.2014 v 11:28 Tomas Babej napsal(a):


On 11/20/2014 04:01 PM, Jan Cholasta wrote:

Dne 19.11.2014 v 15:12 Tomas Babej napsal(a):


On 11/19/2014 02:03 PM, Jan Cholasta wrote:

Dne 19.11.2014 v 13:44 Tomas Babej napsal(a):


On 11/19/2014 12:51 PM, Martin Kosek wrote:

On 11/19/2014 12:41 PM, Tomas Babej wrote:

On 11/19/2014 12:24 PM, Martin Kosek wrote:

On 11/19/2014 12:03 PM, Tomas Babej wrote:

Hi,

When constructing a parent DN in LDAPSearch, we should always
check that the parent object exists (hence use get_dn_if_exists),
rather than search on unexistant containers (which can happen
with get_dn).

Replaces get_dn calls with get_dn_if_exists in *-find commands
and makes sure proper error message is raised.

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

Doesn't it produce extra LDAP search thus making all our search
commands
slower? Is that what we want?

No it does not make all of our LDAP search slower. It only
happens for
the objects that have parent objects, such as idoverrides or
dnsrecords.

... and makes them slower.


What I was pointing out here is that this is not a issue for ALL
*-find
commands (e.g user-find, group-find will not suffer from it), as you
incorrectly stated.




Wouldn't it be better to distinguish between LDAP
search with no results and LDAP search with missing parent DN? The
reply looks
different, at least in CLI:

Up to discussion. We would probably need to introduce a new
exception,
like ParentObjectNotFound.


# search result
search: 4
result: 0 Success

# search result
search: 4
result: 32 No such object
matchedDN: cn=accounts,dc=mkosek-f20,dc=test

Also, I do not think you can just stop using get_dn(), some
commands override
this call to get more complex searches (like host-find searching
for shortname).

Look into the get_dn_if_exists, it just wraps around get_dn, so no
issue
here. Any custom behaviour is preserved.

Ah, ok, thanks for info.


To sum up, I think this is worth changing this behaviour by default,
ignoring a non-matching value of the parent object is not a correct
general approach in my opinion.

Well, that's the question. Whether we would leave DS to validate the
search
itself or do all the pre-check ourselves. To me, doing just one LDAP
search and
processing the error correctly looks better. But I can live even
with your
version then, I will leave the framework guardians like Honza or
Petr3 to decide.


+1 on single LDAP search and proper error processing.



I see now what you're trying to suggest. However, the reason boils
down to ipaldap.find_entries method not differentiating between a
LDAP search that returns error code 32 (No such object) and LDAP
search returning error code 0 (Success), but returning no results.

In both cases errors.NotFound is raised.

The reason I did not go this way is that changing the find_entries
method
is quite more invasive as this is the method subsenqently called by
almost
any command.


You can always derive the new error (ParentNotFound or whatever) on
NotFound, so old code won't break.



Thanks for the suggestsions.

Attached is a new patch which hooks into find_entries method and
differentiates between the cases.



Why are you special casing base scope search?



Since base search is performed only on the entry specified by the DN,
there is no need to differentiate between ContainerNotFound and NotFound.

So the logic is as follows:

subtree search - ContainerNotFound is raised when container does not
exist, NotFound if search provided no results
onelevel search - the same
base search - NotFound is raised if the specified DN does not exist



There is a difference between a search on a non-existent entry and a 
search on an existent entry with a non-matching filter. This difference 
exists on LDAP level and applies to all search scopes, not just the base 
scope.


I don't think hiding this difference is useful at all. Remember that 
this bug exists because we were hiding the difference in the first place.


Also, after giving this some thought, I think it would be better to 
create a new error for the case where there is no match instead of the 
case where the entry does not exist. NotFound pretty much corresponds to 
LDAP's NO_SUCH_OBJECT, something like NoMatchingEntry or EmptyResult 
would fit the no-match result better.


--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0031 ipa-restore: Check if directory is provided + better errors.

2014-11-21 Thread Tomas Babej 

On 11/21/2014 01:56 PM, David Kupka wrote:
 On 11/21/2014 01:42 PM, Tomas Babej wrote:

 On 11/21/2014 01:33 PM, David Kupka wrote:
 https://fedorahosted.org/freeipa/ticket/4683


 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel

 -self.read_header()
 +try:
 +self.read_header()
 +except:
 +raise admintool.ScriptError('Cannot read backup
 metadata')


 Is the bare except clause really necessary?

 https://docs.python.org/2.7/howto/doanddont.html#except


 Not really. I can't imagine other reaction to any exception raised in
 read_header() than complain and exit.
 But you're right that it can hide code errors and make debugging more
 complicated.
 Fixed patch attached.

On another note, I also noticed that read_header leaves leaking file
descriptor fd.

Can you convert that part to use the with statement? This is a perfect
opportunity to fix this as you're touching related code.

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0031 ipa-restore: Check if directory is provided + better errors.

2014-11-21 Thread David Kupka 

On 11/21/2014 02:12 PM, Tomas Babej wrote:


On 11/21/2014 01:56 PM, David Kupka wrote:

On 11/21/2014 01:42 PM, Tomas Babej wrote:


On 11/21/2014 01:33 PM, David Kupka wrote:

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


___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel


-self.read_header()
+try:
+self.read_header()
+except:
+raise admintool.ScriptError('Cannot read backup
metadata')


Is the bare except clause really necessary?

https://docs.python.org/2.7/howto/doanddont.html#except



Not really. I can't imagine other reaction to any exception raised in
read_header() than complain and exit.
But you're right that it can hide code errors and make debugging more
complicated.
Fixed patch attached.


On another note, I also noticed that read_header leaves leaking file
descriptor fd.

Can you convert that part to use the with statement? This is a perfect
opportunity to fix this as you're touching related code.


I thought that python takes care of it. Thanks.
Fixed patch attached.

--
David Kupka
From 026a4eecd2c516d0ae1d579337432c43e189bd7d Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Fri, 21 Nov 2014 06:30:17 -0500
Subject: [PATCH] ipa-restore: Check if directory is provided + better errors.

https://fedorahosted.org/freeipa/ticket/4683
---
 ipaserver/install/ipa_restore.py | 14 ++
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/ipaserver/install/ipa_restore.py b/ipaserver/install/ipa_restore.py
index 93f176d302a49319940555a0be3037620143e1f3..f290bae4dc6455bb22c4e726e72efe98205d970e 100644
--- a/ipaserver/install/ipa_restore.py
+++ b/ipaserver/install/ipa_restore.py
@@ -152,6 +152,9 @@ class Restore(admintool.AdminTool):
 else:
 self.backup_dir = dirname
 
+if not os.path.isdir(dirname):
+raise self.option_parser.error(must provide path to backup directory)
+
 if options.gpg_keyring:
 if (not os.path.exists(options.gpg_keyring + '.pub') or
not os.path.exists(options.gpg_keyring + '.sec')):
@@ -213,7 +216,10 @@ class Restore(admintool.AdminTool):
 try:
 dirsrv = services.knownservices.dirsrv
 
-self.read_header()
+try:
+self.read_header()
+except IOError as e:
+raise admintool.ScriptError('Cannot read backup metadata: %s' % e)
 # These two checks would normally be in the validate method but
 # we need to know the type of backup we're dealing with.
 if (self.backup_type != 'FULL' and not options.data_only and
@@ -546,9 +552,9 @@ class Restore(admintool.AdminTool):
 Read the backup file header that contains the meta data about
 this particular backup.
 '''
-fd = open(self.header)
-config = SafeConfigParser()
-config.readfp(fd)
+with open(self.header) as fd:
+config = SafeConfigParser()
+config.readfp(fd)
 
 self.backup_type = config.get('ipa', 'type')
 self.backup_time = config.get('ipa', 'time')
-- 
2.1.0

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 1111 Use NSS protocol range setter

2014-11-21 Thread Jan Cholasta 

Hi,

Dne 20.11.2014 v 23:26 Rob Crittenden napsal(a):

Use new capability in python-nss-0.16 to use the NSS protocol range
setter. This lets us enable TLSv1.1 and TLSv1.2 for client connections.

I made this configurable via tls_protocol_range in case somebody wants
to override it.

There isn't a whole ton of error handling on bad input but there is
enough, I think, to point the user in the the right direction.

Added a couple more lines of debug output to include the negotiated
protocol and cipher.

rob


1) The patch needs a rebase on top of ipa-4-1 (applies fine on master)


2) Could you split the option into two options, say tls_version_min 
and tls_version_max? IMO it would be easier to manage the version 
range that way, when for example you have to lower just the minimal 
version on a client to make it able to connect to a SSL3-only server.



3) Would it make sense to print a warning when the configured minimal 
TLS version is not safe and the connection uses a safe TLS version? This 
is for the case when you have to lower the minimal version on the client 
because of an old server, then the server gets updated, then you 
probably no longer want to have unsafe minimal version configured on the 
client.



Functionally the patch is OK.

Honza

--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] Releasing FreeIPA 4.1.2

2014-11-21 Thread Petr Vobornik 

On 20.11.2014 15:49, Alexander Bokovoy wrote:

On Thu, 20 Nov 2014, Alexander Bokovoy wrote:

On Thu, 20 Nov 2014, Martin Kosek wrote:

Hello,

We seem to have enough content to release 4.1.2 that will be required
to fix
some of the Fedora 21 blockers:

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

and Freeze exception:
https://bugzilla.redhat.com/show_bug.cgi?id=1165674

Current plan is to release stabilization release 4.1.2 with what we
have as it
contains the important installation and upgrade fixes.

The only problem there is that some patches require components that
are not in
Fedora stable repos, more specifically commits:

7aa855a3  - requiring new certmonger
4e119311  - requiring new pki-core

We can release 4.1.2 with them, but they would need to be removed
from Fedora
21 downstream release and added back in normal 0day Fedora 21 update.
Alternative way would be to just include selected patches for
existing Fedora
21 build.

Any comments, what else are missing in 4.1.2? I only know about
https://fedorahosted.org/freeipa/ticket/4718
https://fedorahosted.org/freeipa/ticket/4728
which is ACKed at the moment, AFAIK.


certmonger and pki-core parts can be backed out in the F21 build, they
don't create issues for install time -- by definition we are not
renewing certs at that point and we are not supporting becoming MS CA
subordinate in the FreeIPA Server Role.

Thus, we can do the release without them to Fedora 21 (and with them to
rawhide) and have 0day update.

It looks like we'll have to pick up required fixes and do a build with
just them for Fedora 21 final to make sure we don't violate release
rules.

We can do the full release after it and push it to updates-testing once
stable updates consumed the fixed build for the Fedora 21 release.


Fedora 21 build and update with fixes for blockers created:

https://admin.fedoraproject.org/updates/freeipa-4.1.1-2.fc21
--
Petr Vobornik

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0031 ipa-restore: Check if directory is provided + better errors.

2014-11-21 Thread Tomas Babej 

On 11/21/2014 02:28 PM, David Kupka wrote:
 On 11/21/2014 02:12 PM, Tomas Babej wrote:

 On 11/21/2014 01:56 PM, David Kupka wrote:
 On 11/21/2014 01:42 PM, Tomas Babej wrote:

 On 11/21/2014 01:33 PM, David Kupka wrote:
 https://fedorahosted.org/freeipa/ticket/4683


 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel

 -self.read_header()
 +try:
 +self.read_header()
 +except:
 +raise admintool.ScriptError('Cannot read backup
 metadata')


 Is the bare except clause really necessary?

 https://docs.python.org/2.7/howto/doanddont.html#except


 Not really. I can't imagine other reaction to any exception raised in
 read_header() than complain and exit.
 But you're right that it can hide code errors and make debugging more
 complicated.
 Fixed patch attached.

 On another note, I also noticed that read_header leaves leaking file
 descriptor fd.

 Can you convert that part to use the with statement? This is a perfect
 opportunity to fix this as you're touching related code.

 I thought that python takes care of it. Thanks.
 Fixed patch attached.


Works quite nicely.

Thanks, ACK.

Pushed to:
master: 373bbee4e3c25fd6fb41a75b62b09d60da1a5d82
ipa-4-1: b40cf4b283c9df7d960ec80124b45d5361c75320

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH 0081] Add initial tests for OTP

2014-11-21 Thread Nathaniel McCallum 
On Fri, 2014-11-21 at 08:49 +0100, Martin Kosek wrote:
 On 11/20/2014 05:13 PM, Nathaniel McCallum wrote:
  This tests the general workflow for OTP including most possible
  token combinations. This includes 5872 tests. Further optimization
  is possible to reduce the number of duplicate tests run.
 
 Good job! Yup, heavy optimization will be needed later.
 
  Things not yet tested:
  * ipa-kdb
 
 Here you would just call kinit instead if LDAP BIND, right?

just :)

Mostly yes. Special care will be needed around FAST, password changes
and not stomping on the admin ticket for running the tests.

  * ipa-otpd
 
 How would ipa-otpd tested? Wouldn't it be tested if simple kinit is made
 instead of direct LDAP BIND?

Yes. If desired, it could also be tested directly by sending RADIUS
packets.

  * otptoken-sync
 
 Petr1 can help to provide a Web UI test for this area. Or alternatively for 
 the
 test we could use the LDAP extended operation directly, right?

If we can't just call api.Command['otptoken_sync'], we can just do the
special bind. It isn't too hard, I just wanted to get patches public.

  * RADIUS proxy
  * token self-management
  * type specific attributes
 
 +1
 
 What about password changes with OTP, can it be also covered?

That is included in ipa-kdb, but yes. :)

 Also, note that the freeipa-tests would suddenly grow a python-pyotp
 dependency, this should be considered.

I'm probably going to change this to python-cryptography since we now
have it in Fedora. I hear IPA will grow a python-cryptography dependency
anyway.

Nathaniel

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 1111 Use NSS protocol range setter

2014-11-21 Thread Rob Crittenden 
Jan Cholasta wrote:
 Hi,
 
 Dne 20.11.2014 v 23:26 Rob Crittenden napsal(a):
 Use new capability in python-nss-0.16 to use the NSS protocol range
 setter. This lets us enable TLSv1.1 and TLSv1.2 for client connections.

 I made this configurable via tls_protocol_range in case somebody wants
 to override it.

 There isn't a whole ton of error handling on bad input but there is
 enough, I think, to point the user in the the right direction.

 Added a couple more lines of debug output to include the negotiated
 protocol and cipher.

 rob
 
 1) The patch needs a rebase on top of ipa-4-1 (applies fine on master)

Attached.

 2) Could you split the option into two options, say tls_version_min
 and tls_version_max? IMO it would be easier to manage the version
 range that way, when for example you have to lower just the minimal
 version on a client to make it able to connect to a SSL3-only server.

Sure. I waffled back and forth before deciding on a single value.
Separate values are probably less error-prone.

 3) Would it make sense to print a warning when the configured minimal
 TLS version is not safe and the connection uses a safe TLS version? This
 is for the case when you have to lower the minimal version on the client
 because of an old server, then the server gets updated, then you
 probably no longer want to have unsafe minimal version configured on the
 client.

I see what you're saying but I think it could end up being just spam
that user's get used to. That and given that I'd probably want to set it
up to require tls1.1 as a minimum but we can't do that because dogtag
only supports through tls1.0 right now AFAICT. That'd be a lot of warnings.

 Functionally the patch is OK.

rob

From 3cdf7b21d3472d0710bee26a8fcabbc159739554 Mon Sep 17 00:00:00 2001
From: Rob Crittenden rcrit...@redhat.com
Date: Thu, 30 Oct 2014 11:52:14 -0400
Subject: [PATCH] Use NSS protocol range API to set available TLS protocols

Protocols are configured as an inclusive range from SSLv3 through
TLSv1.2. The allowed values in the range are ssl3, tls1.0,
tls1.1 and tls1.2. If only a single value is provided then it
represents both the min and max.

This is overridable per client by setting tls_protocol_range.

https://fedorahosted.org/freeipa/ticket/4653
---
 freeipa.spec.in |  2 +-
 ipalib/constants.py |  4 
 ipalib/rpc.py   |  5 -
 ipapython/dogtag.py |  4 +++-
 ipapython/nsslib.py | 17 +++--
 5 files changed, 27 insertions(+), 5 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index a417ab56f843b202787c6112553f8c16f2c1dde0..95ec6210a157fd158d81d97efbd46f3d35facbc6 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -284,7 +284,7 @@ Requires: gnupg
 Requires: iproute
 Requires: keyutils
 Requires: pyOpenSSL
-Requires: python-nss = 0.15
+Requires: python-nss = 0.16
 Requires: python-lxml
 Requires: python-netaddr
 Requires: libipa_hbac-python
diff --git a/ipalib/constants.py b/ipalib/constants.py
index 1eed7ca6ad0e5920318dadc68ed36fff6cf889f2..111bafe5ed0c3d2df58a1b6839feedc58a14fcc4 100644
--- a/ipalib/constants.py
+++ b/ipalib/constants.py
@@ -122,6 +122,10 @@ DEFAULT_CONFIG = (
 
 ('rpc_protocol', 'jsonrpc'),
 
+# Define an inclusive range of SSL/TLS version support
+('tls_version_min', 'tls1.0'),
+('tls_version_max', 'tls1.2'),
+
 # Time to wait for a service to start, in seconds
 ('startup_timeout', 300),
 
diff --git a/ipalib/rpc.py b/ipalib/rpc.py
index 5934f0c26e4b7c0a44adbab978c1f9b319d72e9f..806f6bb9adf004660c9cb285cf31b09a988afa93 100644
--- a/ipalib/rpc.py
+++ b/ipalib/rpc.py
@@ -68,6 +68,7 @@ from ipalib.krb_utils import KRB5KDC_ERR_S_PRINCIPAL_UNKNOWN, KRB5KRB_AP_ERR_TKT
  KRB5_FCC_PERM, KRB5_FCC_NOFILE, KRB5_CC_FORMAT, KRB5_REALM_CANT_RESOLVE
 from ipapython.dn import DN
 from ipalib.capabilities import VERSION_WITHOUT_CAPABILITIES
+from ipalib import api
 
 COOKIE_NAME = 'ipa_session'
 KEYRING_COOKIE_NAME = '%s_cookie:%%s' % COOKIE_NAME
@@ -488,7 +489,9 @@ class SSLTransport(LanguageAwareTransport):
 if sys.version_info  (2, 7):
 conn = NSSHTTPS(host, 443, dbdir=dbdir, no_init=no_init)
 else:
-conn = NSSConnection(host, 443, dbdir=dbdir, no_init=no_init)
+conn = NSSConnection(host, 443, dbdir=dbdir, no_init=no_init,
+ tls_version_min=api.env.tls_version_min,
+ tls_version_max=api.env.tls_version_max)
 self.dbdir=dbdir
 
 conn.connect()
diff --git a/ipapython/dogtag.py b/ipapython/dogtag.py
index 14824b99431e85dd73613befd72e500d370cfe2c..0e0aacca798377517244075ed6b07dff63e87358 100644
--- a/ipapython/dogtag.py
+++ b/ipapython/dogtag.py
@@ -234,7 +234,9 @@ def https_request(host, port, url, secdir, password, nickname, **kw):
 
 
 def connection_factory(host, port):
-conn = nsslib.NSSConnection(host, port, dbdir=secdir)
+conn = nsslib.NSSConnection(host, port,

[Freeipa-devel] [PATCH] 0032 Fix error message for nonexistent members and add tests.

2014-11-21 Thread David Kupka 

https://fedorahosted.org/freeipa/ticket/4643
--
David Kupka
From 3403f21a15d1e1dcb05187708ff16a2750052f7a Mon Sep 17 00:00:00 2001
From: David Kupka dku...@redhat.com
Date: Tue, 21 Oct 2014 18:12:23 -0400
Subject: [PATCH] Fix error message for nonexistent members and add tests.

https://fedorahosted.org/freeipa/ticket/4643
---
 ipalib/plugins/automember.py   |  5 ++-
 ipatests/test_xmlrpc/test_automember_plugin.py | 45 ++
 2 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/ipalib/plugins/automember.py b/ipalib/plugins/automember.py
index 0f47393166003e04ec53c16dcd1629038127509a..e45be9d3b4a692d7145b27a0c8d0a51a12e5bae3 100644
--- a/ipalib/plugins/automember.py
+++ b/ipalib/plugins/automember.py
@@ -735,7 +735,10 @@ class automember_rebuild(Command):
 names = options.get(opt_name)
 if names:
 for name in names:
-obj.get_dn_if_exists(name)
+try:
+obj.get_dn(name)
+except errors.NotFound:
+obj.handle_not_found(*keys)
 search_filter = ldap.make_filter_from_attr(
 obj.primary_key.name,
 names,
diff --git a/ipatests/test_xmlrpc/test_automember_plugin.py b/ipatests/test_xmlrpc/test_automember_plugin.py
index 6618ac60531ddce3e651eff16d0ce0031b9f189d..8e3777e00411a405ae16b3ffe56ee9f4cd6e88bf 100644
--- a/ipatests/test_xmlrpc/test_automember_plugin.py
+++ b/ipatests/test_xmlrpc/test_automember_plugin.py
@@ -30,6 +30,7 @@ from ipatests.test_xmlrpc.test_user_plugin import get_user_result
 
 
 user1 = u'tuser1'
+user_does_not_exist = u'does_not_exist'
 manager1 = u'mscott'
 fqdn1 = u'web1.%s' % api.env.domain
 short1 = u'web1'
@@ -41,6 +42,7 @@ fqdn4 = u'www5.%s' % api.env.domain
 short4 = u'www5'
 fqdn5 = u'webserver5.%s' % api.env.domain
 short5 = u'webserver5'
+fqdn_does_not_exist = u'does_not_exist.%s' % api.env.domain
 
 group1 = u'group1'
 group1_dn = DN(('cn', group1), ('cn', 'groups'),
@@ -1625,4 +1627,47 @@ class test_automember(Declarative):
 ),
 ),
 
+dict(
+desc='Rebuild membership with type hostgroup and --hosts',
+command=('automember_rebuild', [], {u'type': u'hostgroup', u'hosts': fqdn1}),
+expected=dict(
+value=None,
+summary=u'Automember rebuild task finished. Processed (1) entries.',
+result={
+}
+),
+),
+
+dict(
+desc='Rebuild membership with type group and --users',
+command=('automember_rebuild', [], {u'type': u'group', u'users': user1}),
+expected=dict(
+value=None,
+summary=u'Automember rebuild task finished. Processed (1) entries.',
+result={
+}
+),
+),
+
+dict(
+desc='Try to rebuild membership with invalid host in --hosts',
+command=('automember_rebuild', [], {u'type': u'hostgroup', u'hosts': fqdn_does_not_exist}),
+expected=dict(
+value=None,
+summary=u'Automember rebuild task finished. Processed (0) entries.',
+result={
+}
+),
+),
+
+dict(
+desc='Try to rebuild membership with invalid user in --users',
+command=('automember_rebuild', [], {u'type': u'group', u'users': user_does_not_exist}),
+expected=dict(
+value=None,
+summary=u'Automember rebuild task finished. Processed (0) entries.',
+result={
+}
+),
+),
 ]
-- 
2.1.0

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0032 Fix error message for nonexistent members and add tests.

2014-11-21 Thread Jan Cholasta 

Hi,

Dne 21.11.2014 v 16:11 David Kupka napsal(a):

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


You probably don't want to change get_dn_if_exists to get_dn, as the 
latter does not usually raise NotFound when the entry does not exist.


Honza

--
Jan Cholasta

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 0032 Fix error message for nonexistent members and add tests.

2014-11-21 Thread Tomas Babej 

On 11/21/2014 04:11 PM, David Kupka wrote:
 https://fedorahosted.org/freeipa/ticket/4643


 ___
 Freeipa-devel mailing list
 Freeipa-devel@redhat.com
 https://www.redhat.com/mailman/listinfo/freeipa-devel

-obj.get_dn_if_exists(name)
+try:
+obj.get_dn(name)
+except errors.NotFound:
+obj.handle_not_found(*keys)

You switched from get_dn_if_exists to get_dn, why? The get_dn method
never raises the NotFound error, it just constructs the dn regardless of
the fact whether the actual entry at that dn exists.

You need to use get_dn_if_exists here. Some negative tests for this
would be welcome :).

Indeed, by testing the patch:

[tbabej@vm-218 ~]$ ipa automember_rebuild --users=doesnotexist

Automember rebuild task finished. Processed (0) entries.


Where the old behaviour was:

[tbabej@vm-218 ~]$ ipa automember_rebuild --users=test
ipa: ERROR: no such entry

-- 
Tomas Babej
Associate Software Engineer | Red Hat | Identity Management
RHCE | Brno Site | IRC: tbabej | freeipa.org 

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCH] 376 Stop tracking certificates before restoring them in ipa-restore

2014-11-21 Thread Petr Vobornik 

On 20.11.2014 15:21, Jan Cholasta wrote:

Hi,

the attached patch fixes https://fedorahosted.org/freeipa/ticket/4727.

Honza



ACK

Pushed to master: aa9ecb253a60d9d15cd41c5c38695fe64058669a
Pushed to ipa-4-1: 66db7b910ff0dcc6f2bbce01c3c2b1ce6442
--
Petr Vobornik

___
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Re: [Freeipa-devel] [PATCHES] 0656-0673 Switch the test suite to pytest

2014-11-21 Thread Rob Crittenden 
Tomas Babej wrote:
 
 On 11/20/2014 10:12 AM, Petr Viktorin wrote:
 On 11/19/2014 01:11 PM, Tomas Babej wrote:

 On 11/14/2014 09:55 AM, Petr Viktorin wrote:
 On 10/29/2014 04:52 PM, Petr Viktorin wrote:
 On 10/29/2014 01:22 PM, Tomas Babej wrote:

 On 10/27/2014 04:38 PM, Petr Viktorin wrote:
 On 10/15/2014 02:58 PM, Petr Viktorin wrote:
 This almost completes the switch to pytest. There are two missing
 things:
 - the details of test results (--with-xunit) are not read
 correctly by
 Jenkins. I have a theory I'm investigating here.
 - the beakerlib integration is still not ready


 I'll not be available for the rest of the week so I'm sending this
 early, in case someone wants to take a look.

 I've updated (and rearranged) the patches after some more testing.
 Both points above are fixed. Individual plugins are broken out; some
 would be nice to even release independently of IPA. (There is some
 demand for the BeakerLib plugin; for that I'd only need to break the
 dependency on ipa_log_manager.)


 These depend on my patches 0656-0660.


 Thanks for this effort!

  Patch 0656: tests: Use PEP8-compliant setup/teardown method
 names

 There are some references to the old names in the test_ipapython and
 test_install:

   [tbabej@thinkpad7 ipatests]$ git grep setUpClass
   [tbabej@thinkpad7 ipatests]$ git grep tearDownClass
   [tbabej@thinkpad7 ipatests]$ git grep setUp
   test_install/test_updates.py:def setUp(self):
   test_ipapython/test_cookie.py:def setUp(self):
   test_ipapython/test_cookie.py:def setUp(self):
   test_ipapython/test_cookie.py:def setUp(self):
   test_ipapython/test_dn.py:def setUp(self):
   test_ipapython/test_dn.py:def setUp(self):
   test_ipapython/test_dn.py:def setUp(self):
   test_ipapython/test_dn.py:def setUp(self):
   test_ipapython/test_dn.py:def setUp(self):
   [tbabej@thinkpad7 ipatests]$ git grep tearDown
   test_install/test_updates.py:def tearDown(self):

 These are in unittest.testCase. It would be nice to convert those as
 well, but that's for a larger cleanup.

  Patch 0657: tests: Add configuration for pytest

 Shouldn't we rather change the class names?

 Ideally yes, but I don't think renaming most of our test classes would
 be worth the benefit.

  Patch 0658: ipatests.util.ClassChecker: Raise AttributeError in
 get_subcls

 ACK.

  Patch 0659: test_automount_plugin: Fix test ordering

 ACK.

  PATCH 0660: Use setup_class/teardown_class in Declarative tests

   --- a/ipatests/test_ipaserver/test_changepw.py
   +++ b/ipatests/test_ipaserver/test_changepw.py
   @@ -33,8 +33,7 @@
class test_changepw(XMLRPC_test, Unauthorized_HTTP_test):
app_uri = '/ipa/session/change_password'

   -def setup(self):
   -super(test_changepw, self).setup()
   +def setup(cls):
try:
api.Command['user_add'](uid=testuser,
 givenname=u'Test', sn=u'User')
api.Command['passwd'](testuser,
 password=u'old_password')
   @@ -43,12 +42,11 @@ def setup(self):
'Cannot set up test user: %s' % e
)

   -def teardown(self):
   +def teardown(cls):
try:
api.Command['user_del']([testuser])
except errors.NotFound:
pass
   -super(test_changepw, self).teardown()

 The setup/teardown methods are not classmethods, so the name of the
 first argument should remain self.

 Oops, thanks for the catch!

  PATCH 0661: dogtag plugin: Don't use doctest syntax for
 non-doctest
 examples

 ACK.

  PATCH 0662: test_webui: Don't use __init__ for test classes

 I don't think the following change will work:

   -def __init__(self, driver=None, config=None):
   +def setup(self, driver=None, config=None):
self.request_timeout = 30
self.driver = driver
self.config = config
if not config:
self.load_config()
   +self.get_driver().maximize_window()
   +
   +def teardown(self):
   +self.driver.quit()

def load_config(self):

   @@ -161,20 +165,6 @@ def load_config(self):
if 'type' not in c:
c['type'] = DEFAULT_TYPE

   -def setup(self):
   -
   -Test setup
   -
   -if not self.driver:
   -self.driver = self.get_driver()
   -self.driver.maximize_window()
   -
   -def teardown(self):
   -
   -Test clean up
   -
   -self.driver.quit()


 The setup(self) method used to set the self.driver attribute on the
 class instance, now nothing sets it. The setup method should do:

   def setup(self,