Re: [Freeipa-devel] [PATCH 0162] Upgrade fix: masking named service should be executed only once

2014-12-09 Thread David Kupka

On 12/05/2014 12:58 PM, Martin Basti wrote:

On 05/12/14 10:23, David Kupka wrote:

On 11/12/2014 01:43 PM, Martin Basti wrote:

Hello,

masking named service is executed more than once, following patch
fixes it.

Patch attached.



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


Works as expected but:

0) mask_named_regular() returns True, False and None. None evaluates
as False so why not use False directly?
1) missing link to ticket in commit message.



Updated patch attached


Works for me, ACK.

--
David Kupka

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


Re: [Freeipa-devel] [RANT] pytest fixture scope is braindead

2014-12-09 Thread Petr Viktorin

On 12/08/2014 03:15 PM, Nathaniel McCallum wrote:

On Mon, 2014-12-08 at 12:27 +0100, Petr Viktorin wrote:

On 12/05/2014 11:51 PM, Nathaniel McCallum wrote:

So I've been working this week on porting the OTP tests that were
manually coded to the new pytest framework. Now, I'm the first to want
better tooling to make our job easier. But, pytest? Meh. I'm having to
write just as much code (or more) to get my tests on par with pytest,
and they are riddled with subtle bugs.


Hi,
Yeah, IPA runs into specific situations that are very different from how
tests usually work in other projects.


Yeah, this I get.


And you took one of the harder cases.
IPA's existing tests also don't really follow best practices – they
pretty much assume you run the whole test suite, or at least the whole
module, so selecting individual tests is currently not reliable. On the
other hand, this assumption made some things easy to code – like
complicated ordering and setup/teardown.


I think this is pretty much a red herring. It isn't so much about setup
and teardown but about mutually exclusive parameterization. It is a poor
assumption on the part of pytest that the lifecycles of parameters
overlap. At the least, an option should be provided to control this.


One thing we want to do during the move to pytest is to have the tests
be more independent of the current state of the database, and also of
each other.


Sure, this is a fine goal. Unless of course you want to test the
functionality of the code against all possible database states. I think
pretty much every xmlrpc test will have some component of this.


If certain assertions only make sense in a specific order, either put
them in the same test*, or write additional code that explicitly handles
dependencies and ordering. (Sorry, that's the price to pay for isolated
tests – I don't see a way around it.)


In an ideal world, we define a test and all possible parameters to that
test and allow the running of that test independently by choosing from
the proscribed inputs. This all works fine so long as you specify a way
to indicate when inputs are mutually exclusive. Since inputs are single
valued anyway, this should be the default behavior. Unfortunately,
pytest fails on both of these counts.


If on the other hand the order doesn't matter – any order of any subset
of individual tests should pass, which is our goal – but pytest's
default ordering makes the suite slow, there is a hook that allows you
to override the order: pytest_collection_modifyitems [0]. Stick it in
the affected module (or conftest.py file for multiple modules, or a
plugin for the whole suite), and sort the items explicitly.


The problem isn't (I think) ordering but lifecycle. I think that if I
scope the fixture to the function but reorder the items, they will still
be constructed and torn down for every single function iteration. On the
other hand, if I scope the fixture for a longer lifecycle, the
parameters may not be mutually exclusive.


If your test depends on something not existing or being disabled, you
should try removing/disabling it before the test, or write a fixture to
do that.


That's not the problem. The problem is that I can't define parameters
for a single object which sets the same variable to different values.
Such an act means the parameters are mutually exclusive.


OK, I see the problem now. Pytest fixtures are meant for resources, not 
states of a shared object.
You ran into a fairly exotic use case (which is what I feared, so I 
wanted to look into porting this.)
I think in this case we should use using separate users for the 
enablement states, since user creation is not expensive. Or am I missing 
something else?



* might make sense, for starters?
[0]
http://pytest.org/latest/plugins.html#_pytest.hookspec.pytest_collection_modifyitems



Most painful, is pytest.fixture(). It is a cool idea, but the scoping
and lifecycle is horribly broken. Here is an example:

Here is an example. I want to create a user, enable otp in two different
ways, test with two different types of tokens under two different
authentication scenarios. This is a total of 8 tests.

@pytest.fixture(scope=function) # This scope is default.
def user(request):
user = create_user('tuser1')
request.addfinalizer(lambda: delete_user('tuser1'))
return user

@pytest.fixture(params=[per-user, global])
def enablement(request, user):
output = enable(user, request.param)
request.addfinalizer(lambda: disable(user))
return output

@pytest.fixture(params=[{'type': 'HOTP'}, {'disabled': True}])
def token(request, user, enablement):
output = create_token(request.param)
request.addfinalizer(lambda: delete_token(output.id))
return output

@pytest.mark.parametrize(opts, [foo, bar])
def test_authentication(user, token, opts):
return auth(user.uid, token.code(), opts)

Because the default scope is function, a new user is created for every
single run of token. This is *way* more work than 

Re: [Freeipa-devel] [PATCH] 795 webui: increase duration of notification messages

2014-12-09 Thread Martin Basti

On 05/12/14 16:23, Petr Vobornik wrote:

increase duration of notification messages by 66%

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

ACK

--
Martin Basti

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


Re: [Freeipa-devel] [PATCH] 790 webui: fix service unprovisioning

2014-12-09 Thread Martin Basti

On 27/11/14 15:50, Petr Vobornik wrote:
Missed part of field refactoring caused that service could not be 
unprovisioned.


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

For regression tests I've opened ticket:
https://fedorahosted.org/freeipa/ticket/4772


ACK, works for me now

--
Martin Basti

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


Re: [Freeipa-devel] [PATCH] 790 webui: fix service unprovisioning

2014-12-09 Thread Petr Vobornik

On 12/09/2014 12:48 PM, Martin Basti wrote:

On 27/11/14 15:50, Petr Vobornik wrote:

Missed part of field refactoring caused that service could not be
unprovisioned.

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

For regression tests I've opened ticket:
https://fedorahosted.org/freeipa/ticket/4772


ACK, works for me now



Pushed to:
master: edddb4fb2ebad5963275b4b1910d88878e0ad774
ipa-4-1: d1cc285adf594edd4afda033f4a0e1ed35b82f95
--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 795 webui: increase duration of notification messages

2014-12-09 Thread Petr Vobornik

On 12/09/2014 12:48 PM, Martin Basti wrote:

On 05/12/14 16:23, Petr Vobornik wrote:

increase duration of notification messages by 66%

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

ACK



Pushed to:
master: e4f014dfa01e8b37c6b196310f7cca18ca4b5400
ipa-4-1: 88ab70b053f12bee81c53c534f546890d38015e9
--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 382 Fix automatic CA cert renewal endless loop in dogtag-ipa-ca-renew-agent

2014-12-09 Thread David Kupka

On 12/04/2014 08:24 AM, Jan Cholasta wrote:

Hi,

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

Honza



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


Works for me, ACK.

--
David Kupka

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


Re: [Freeipa-devel] [PATCH] 383 Check subject name encoding in ipa-cacert-manage renew

2014-12-09 Thread David Kupka

On 12/04/2014 09:36 AM, Jan Cholasta wrote:

Hi,

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

Honza



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


Works for me, ACK.

--
David Kupka

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


Re: [Freeipa-devel] [PATCH] 384 Do not renew the IPA CA cert by serial number in dogtag-ipa-ca-renew-agent

2014-12-09 Thread David Kupka

On 12/05/2014 11:30 AM, Jan Cholasta wrote:

Hi,

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

Honza



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


Works for me, ACK.

--
David Kupka

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


Re: [Freeipa-devel] [PATCH 0162] Upgrade fix: masking named service should be executed only once

2014-12-09 Thread Petr Vobornik

On 12/09/2014 09:51 AM, David Kupka wrote:

On 12/05/2014 12:58 PM, Martin Basti wrote:

On 05/12/14 10:23, David Kupka wrote:

On 11/12/2014 01:43 PM, Martin Basti wrote:

Hello,

masking named service is executed more than once, following patch
fixes it.

Patch attached.



Works as expected but:

0) mask_named_regular() returns True, False and None. None evaluates
as False so why not use False directly?
1) missing link to ticket in commit message.



Updated patch attached


Works for me, ACK.



Pushed to:
master: 29ff2868cde9f80eda62d50c0d5fc2c22541faf1
ipa-4-1: b13f764b3c355576692e558299d17e8ea8819834
--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 382 Fix automatic CA cert renewal endless loop in dogtag-ipa-ca-renew-agent

2014-12-09 Thread Petr Vobornik

On 12/09/2014 01:03 PM, David Kupka wrote:

On 12/04/2014 08:24 AM, Jan Cholasta wrote:

Hi,

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

Honza



Works for me, ACK.



Pushed to:
master: 423c3e8f34d6ae6655c3b82c4e5a18caf1e63a49
ipa-4-1: 9bfb16c22043d714b8227567600f94345c40cad6
--
Petr Vobornik

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


Re: [Freeipa-devel] [PATCH] 384 Do not renew the IPA CA cert by serial number in dogtag-ipa-ca-renew-agent

2014-12-09 Thread Petr Vobornik

On 12/09/2014 01:04 PM, David Kupka wrote:

On 12/05/2014 11:30 AM, Jan Cholasta wrote:

Hi,

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

Honza



Works for me, ACK.



Pushed to:
master: 1f6fff2b5aea7f92e3321870ea59661b127ab50a
ipa-4-1: 7f1db9303e14fc7b3f505cf63d21544197ea6047
--
Petr Vobornik

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


Re: [Freeipa-devel] ds-migrate feature enhancements

2014-12-09 Thread Martin Kosek
On 12/05/2014 12:32 AM, Alan Evans wrote:
 Hello Martin,
 
 I have spent some time looking into how to address
 https://fedorahosted.org/freeipa/ticket/3709.  I thought I would look at
 --setattr --addattr from user-add and others to see if I could reuse code
 for --user-attr-default.  At least for parsing the commandline arguments
 anyway.  It does not look like it would be easy to re-purpose the code in
 _convert_2_dict() from ipalib/plugins/baseldap.py (without blatantly
 copying it).  What are your thoughts about creating a Dict paramater type
 in ipalib/parameters.py to to user for --setaddr --addattr --delattr and to
 use for --user-attr-default?

Ah, I did not expect any integration with the standard baseldap.py --*attr
functions at all. Though reusing _convert_2_dict sounds as a good idea!

 Also I was looking at string.Template() vs string.Formatter().  I think
 string.Template is probably sufficient.  But I don't follow PEPs so I don't
 know if that's Python3 future proof or not.  Thoughts?

You do not need to worry with Python3 at this moment too much, string.Template
is already used by FreeIPA anyway.

 # Pseudocode
 (attr, default) = args['userdefaultattr'][0].split('=',2)
 entry_attrs[attr] = default.safe_substitute(entry_attrs.toDict())
 
 In looking at ipalib/plugins/baseldap.py, I was wondering if It might make
 sense to make migrate more in line with the --*attr paramaters.
 
 Consider:
 ipa migrate-ds --user-delattr ou
 
 Instead of:
 ipa-migrate-ds --user-ignore-attribute ou

Hm, I would be afraid that you would get into a too much complexity if you try
to integrate the new functionality which was supposed to be rather simple with
*attr parameters processed process_attr_options that even do LDAP search do
process the merge the *attr options with existing LDAP entry. You do not need
to worry about that, you always work only on top of the migrated user entry
that you have available. That simplifies the processing a lot.

 Then we could also add user-{setattr,addattr,delattr,attrdefault} like:
 ipa migrate-ds --user-attrdefault mail=$u...@example.com --user-setattr
 cn=$givenname $sn --user-addattr description=Migrated $(date)
 --user-delattr ou
 Which would add a mail address if one wasn't present, replace the CN with
 the first and last name, add a description attribute and get rid of the ou
 attribute.

This functionality may make sense as an extension, eventually. Currently, we
have nothing so even having

--user-attr-default
and
--user-attr-set

would go a long way.

 Another thought I had was about the behaivor of the search on the source
 directory.  Currently one provides --{user,group}-continer and
 --{user,group}-objectclass and the scope is statically defined in the
 script.
 
 
 --{user,group}-filter
 --
 
 What about adding --{user,group}-scope ONE|SUB?

This is actually a very good idea and a valid option to add! We even have a
ticket in the FreeIPA Trac that no one brave enough took so far.

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

If you decide to contribute this one, I would recommend doing in a separate
patch from your template work as it is parallel effort and would make the
review easier.

 
 My directory for example is like:
 
 dn: uid=me,ou=ops,ou=people,dc=example,dc=com
 dn: uid=joe,ou=eng,ou=people,dc=example,dc=com
 
 So currently I have to use the --continue switch and change my
 --user-container to get all of my users.  If the scope weren't limited to
 ONELEVEL then I could get everyone in one pass.
 
 ipa migrate-ds --user-container ou=People,dc=example,dc=com --user-scope SUB

This makes a lot of sense, +1 for adding the switches (should be very easy to
implement)

 Lastly I was thinking that --user-objectclass and --exclude-users could be
 made more powerful with --user-filter
 
 ipa migrate-ds --user-container ou=People,dc=example,dc=com
 --user-objectclass posixAccount --user-filter '(!(loginShell=/bin/false))'
 or we could fire all of sales --user-filter '(!(ou=Sales))'
 
 Which gets concatenated internally in the search to
 ((objectClass=posixAccount)(!(loginShell=/bin/false)))

Another great idea. Currently we only create automated filter in

def construct_filter(template, oc_list):
oc_subfilter = ''.join([ '(objectclass=%s)' % oc for oc in oc_list])
return template % oc_subfilter


based on objectclass. Your proposal would make it much more flexible. This one
is also completely parallel so it should be in a separate patch compared to the
template work.

I am really looking forward to your patches! :-)

 On Thu, Dec 4, 2014 at 4:59 AM, Martin Kosek mko...@redhat.com wrote:
 
 On 11/26/2014 11:52 AM, Martin Kosek wrote:
 On 11/24/2014 11:24 PM, Alan Evans wrote:
 I am in the midst of preparing for a migration from OpenLDAP to FreeIPA.
 ds-migrate wasn't going to fill all of my needs so I thought I would
 use it
 for most and then make up some LDIF's and massage them to do the last
 bit
 of migration.

 I have instead 

Re: [Freeipa-devel] FreeIPA integration with external DNS services

2014-12-09 Thread Martin Kosek
On 12/03/2014 05:04 PM, Petr Spacek wrote:
 On 2.12.2014 17:21, Simo Sorce wrote:
 On Tue, 02 Dec 2014 15:56:28 +0100
 Petr Spacek pspa...@redhat.com wrote:

 On 1.12.2014 17:12, Simo Sorce wrote:
 On Mon, 01 Dec 2014 16:17:54 +0100
 Petr Spacek pspa...@redhat.com wrote:

 On 14.11.2014 17:31, Petr Spacek wrote:
 On 14.11.2014 02:22, Simo Sorce wrote:
 On Tue, 11 Nov 2014 16:29:51 +0100
 Petr Spacek pspa...@redhat.com wrote:

 Hello,

 this thread is about RFE
 IPA servers when installed should register themselves in the
 external DNS https://fedorahosted.org/freeipa/ticket/4424

 It is not a complete design, just a raw idea.


 Use case
 
 FreeIPA installation to a network with existing DNS
 infrastructure + network administrator who is not willing to
 add/maintain new DNS servers just for FreeIPA.


 High-level idea
 ===
 - Transform dns* commands from FreeIPA framework to equivalent
 nsupdate commands and send DNS updates to existing DNS
 servers.
 - Provide necessary encryption/signing keys to nsupdate.


 1) Integration to FreeIPA framework
 ===
 First of all, we need to decide if external DNS integration
 can be used at the same time with FreeIPA-integrated DNS or not.
 Side-question is what to do if a first server is installed with
 external-DNS but another replica is being installed with
 integrated-DNS and so on.

 I think being able to integrate with an external DNS is
 important, and not just at the server level, being able to
 distribute TSIG keys to client would be nice too, though a lot
 less important, than getting server integration..

 Using TSIG on many clients is very problematic. Keep in mind that
 TSIG is basically HMAC computed using symmetric key so:
 a) Every client would have to use the same key - this is a
 security nightmare. b) We would have to distribute and maintain
 many keys for many^2 clients, which is an administrative
 nightmare.

 For *clients* I would rather stay with GSS-TSIG which is much more
 manageable because we can use Kerberos trust and Keytab
 distribution is already solved by ipa-client-install.

 Alternative for clients would be to use FreeIPA server as proxy
 which encapsulates TSIG keys and issues update requests on behalf
 of clients. This would be FreeIPA-specific but any
 TSIG-distribution mechanism will be FreeIPA-specific anyway.

 TSIG standard explicitly says that there is no standardized
 distribution mechanism.

 In other words, the question is if current dns.py plugin
 shipped with FreeIPA framework should be:

 a) Extended dns.py with dnsexternal-* commands
 --
 Disadvantages:
 - It complicate FreeIPA DNS interface which is a complex beast
 even now.
 - We would have add condition to every DNS API call in
 installers which would increase horribleness of the installer
 code even more (or add another layer of abstraction...).

 I agree having a special command is undesirable.

 - I don't see a point in using integrated-DNS with external-DNS
 at the same time. To use integrated-DNS you have to get a
 proper DNS delegation from parent domain - and if you can get
 the delegation then there is no point in using external DNS ...

 I disagree on this point, it makes a lot of sense to have some
 zones maintained by IPA and ... some not.

 Advantages:
 - You can use external  integrated DNS at the same time.

 We can achieve the same w/o a new ipa level command.

 This needs to be decided by FreeIPA framework experts ...

 Petr^3 came up with clever 'proxy' idea for IPA-commands. This
 proxy would provide all ipa dns* commands and dispatch user-issued
 commands to appropriate FreeIPA-plugin (internal-dns or
 external-dns). This decision could depend on some values in LDAP.

 b) Replace dns.py with another implementation of current
 dnszone-*  dnsrecord-* API.
 -
 This seems like a cleaner approach to me. It could be shipped as
 ipa-server-dns-external package (opposed to standard
 ipa-server-dns package).

 Advantages:
 - It could seamlessly work with FreeIPA client installer because
 the dns*-nsupdate command transformation would be done on
 FreeIPA server and client doesn't need to know about it.
 - Does not require re-training/not much new documentation
 because commands are the same.

 Disadvantages:
 - You can't use integrated  external DNS at the same time (but
 I don't think it justifies the added complexity).

 I disagree.

 One of the reason to use a mix is to allow smooth migrations
 where zones are moved into (or out of) IPA one by one.

 Good point, I agree. I will focus on supporting both (internal and
 external) DNS at the same time.

 Petr^3 or anyone else, what do you propose?

 I think what I'd like to see is to be able to configure a DNS
 zone in LDAP and mark it external.
 The zone would held the TSIG keys necessary to perform DNS
 updates.

 When the regular ipa 

Re: [Freeipa-devel] [PATCH] 383 Check subject name encoding in ipa-cacert-manage renew

2014-12-09 Thread Jan Cholasta

Dne 5.12.2014 v 12:01 Jan Cholasta napsal(a):

Dne 5.12.2014 v 11:43 Martin Kosek napsal(a):

On 12/05/2014 11:34 AM, Jan Cholasta wrote:

Dne 5.12.2014 v 09:03 Martin Kosek napsal(a):

On 12/04/2014 09:36 AM, Jan Cholasta wrote:

+if x509.get_der_subject(cert, x509.DER) != der_subject:
+raise admintool.ScriptError(Subject name encoding
mismatch)


I think we can expect this to be a pretty common error, given this is
the default behavior of Microsoft Certificate Services. I would thus
like to make the error message more juicy.

We need to make sure we offer some pointers for these users or they
will
just blame IPA for screwing up. So, the information I wrote

https://bugzilla.redhat.com/show_bug.cgi?id=1129558#c11

need to somehow get to the error message as a potential/likely root
cause of the problem. Whether you write it in the error message itself
or update the design page and just insert a link is up to you.

Martin


I would rather document this and have users read the documentation,
which they
should do anyway when something goes wrong. There are many errors in
IPA which
are common and users may blame IPA for them and I don't see what makes
this one
so special that it should require a special treatment.


I saw several reasons:
- Certificateinstallation error are more common than the others and
users are usually quite lost in what to do with them.
- In this case, we know by 90% probability what is the root cause
- It will block one of the main use cases for the new CA renewal tool
and people will likely hit it as MS CAs is one of the most common CAs
and this is it's default behavior.

Giving more details in this case will not hurt us, but benefit users. So
I still do not see the harm.


I do not see a harm either, my point is that we should probably point
the user to documentation when *anything* in *any* script goes wrong,
not just when some arbitrarily cherry-picked error occurs.




Anyway, I have created
http://www.freeipa.org/page/Troubleshooting#External_CA_renewal_with_ipa-cacert-manage_fails.




Good. Do you plan to reference the section or enhance the error message?


I plan to reference http://www.freeipa.org/page/Troubleshooting.


See the attached patch (385).





Martin





--
Jan Cholasta
From b5a4da2119eb6a57750fdb55bec5a5ad1c6db669 Mon Sep 17 00:00:00 2001
From: Jan Cholasta jchol...@redhat.com
Date: Tue, 9 Dec 2014 12:47:58 +
Subject: [PATCH] Refer the user to freeipa.org when something goes wrong in
 ipa-cacert-manage

https://fedorahosted.org/freeipa/ticket/4781
---
 ipaserver/install/ipa_cacert_manage.py | 8 
 1 file changed, 8 insertions(+)

diff --git a/ipaserver/install/ipa_cacert_manage.py b/ipaserver/install/ipa_cacert_manage.py
index 8fda6a2..f73c7fe 100644
--- a/ipaserver/install/ipa_cacert_manage.py
+++ b/ipaserver/install/ipa_cacert_manage.py
@@ -120,6 +120,14 @@ class CACertManage(admintool.AdminTool):
 
 return rc
 
+def log_failure(self, error_message, return_value, exception, backtrace):
+super(CACertManage, self).log_failure(
+error_message, return_value, exception, backtrace)
+
+if isinstance(exception, admintool.ScriptError):
+print(\nVisit http://www.freeipa.org/page/Troubleshooting for 
+  troubleshooting guide)
+
 def ldap_connect(self):
 conn = ldap2()
 
-- 
2.1.0

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

Re: [Freeipa-devel] [PATCH] 380 Improve validation of --instance and --backend options in ipa-restore

2014-12-09 Thread Jan Cholasta

Dne 9.12.2014 v 14:27 David Kupka napsal(a):

On 12/01/2014 01:16 PM, Jan Cholasta wrote:

Hi,

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

Honza



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


Works for me, ACK.



Thanks for the review.

Pushed to:
master: 7b0149f32b95b42598dd30acde4d2c589ffcfce1
ipa-4-1: f92d0efca693ddcf4de0bc3413ab26c76bad6963

--
Jan Cholasta

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


Re: [Freeipa-devel] topology management question

2014-12-09 Thread Rob Crittenden
Simo Sorce wrote:
 On Sun, 07 Dec 2014 10:25:06 -0500
 Rob Crittenden rcrit...@redhat.com wrote:
 
 Simo Sorce wrote:
 On Thu, 04 Dec 2014 14:33:09 +0100
 Ludwig Krispenz lkris...@redhat.com wrote:

 hi,

 I just have another (hopefully this will end soon) issue I want to
 get your input. (please read to teh end first)

 To recapture the conditions:
 -  the topology plugin manages the connections between servers as 
 segments in the shared tree
 - it is authoritative for managed servers, eg it controls all 
 connections between servers listed under cn=masters,
it is permissive for connection to other servers
 - it rejects any removal of a segment, which would disconnect the
 topology.
 - a change in topology can be applied to any server in the
 topology, it will reach the respective servers and the plugin will
 act upon it

 Now there is a special case, causing a bit of trouble. If a replica
 is to be removed from the topology, this means that
 the replication agreements from and to this replica should be
 removed, the server should be removed from the manages servers.
 The problem is that:
 - if you remove the server first, the server becomes unmanaged and 
 removal of the segment will not trigger a removal of the
 replication agreement

 I had another, sort of unrelated thought about this, thinking about
 deleting servers.

 What happens if a replication conflict entry gets added?
 
 This would happen in case a provisioning system tries to instantiate 2
 servers with the same name at the same time talking to different
 existing masters.

Sure and quite possible if there is some link down and two admins doing
the same thing.

 
 While both exist I imagine that the actual agreement would reflect
 whichever entry is processed last. Probably not the end of the world.

 But how do you remove the conflict entry without also potentially
 deleting that master?
  
 It should probably delete both, the domain would be pretty messed up
 anyone and we have no easy way to know which of the 2 is part of the
 domain and which one has all replication severed due to their keys
 being overwritten by the other server ones.

Yes, this is what I was leaning towards as well, but the plugin may need
to specifically do this. It all depends on the search filters Ludwig
uses to find the master(s) to operate on. And should this be
automatically detected?

So in reality the fact that there is a duplicate agreement in topology
probably won't hurt anything at all since we don't really define much
there. The actual agreement(s) will still be created just fine, but this
extra topology record could be confusing depending on whether it gets
returned by anything.

 I guess the only thing we can reasonably do is to make recommendations
 on how to deal with replicas deployments to avoid this case and
 instructions on how to remove remnants entries if any.

I brought this up in case deleting the conflict entry would create an
orphan, for example.

It occurs to me that perhaps the conflict entry may not actually contain
anything different than the real entry. It isn't like we store a ton of
data. So I wonder if one deletes a conflict entry it shouldn't trigger
topology changes. But that is likely going to require extra logic.

rob

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


Re: [Freeipa-devel] [RANT] pytest fixture scope is braindead

2014-12-09 Thread Nathaniel McCallum
On Tue, 2014-12-09 at 10:39 +0100, Petr Viktorin wrote:
 On 12/08/2014 03:15 PM, Nathaniel McCallum wrote:
  On Mon, 2014-12-08 at 12:27 +0100, Petr Viktorin wrote:
  On 12/05/2014 11:51 PM, Nathaniel McCallum wrote:
  So I've been working this week on porting the OTP tests that were
  manually coded to the new pytest framework. Now, I'm the first to want
  better tooling to make our job easier. But, pytest? Meh. I'm having to
  write just as much code (or more) to get my tests on par with pytest,
  and they are riddled with subtle bugs.
 
  Hi,
  Yeah, IPA runs into specific situations that are very different from how
  tests usually work in other projects.
 
  Yeah, this I get.
 
  And you took one of the harder cases.
  IPA's existing tests also don't really follow best practices – they
  pretty much assume you run the whole test suite, or at least the whole
  module, so selecting individual tests is currently not reliable. On the
  other hand, this assumption made some things easy to code – like
  complicated ordering and setup/teardown.
 
  I think this is pretty much a red herring. It isn't so much about setup
  and teardown but about mutually exclusive parameterization. It is a poor
  assumption on the part of pytest that the lifecycles of parameters
  overlap. At the least, an option should be provided to control this.
 
  One thing we want to do during the move to pytest is to have the tests
  be more independent of the current state of the database, and also of
  each other.
 
  Sure, this is a fine goal. Unless of course you want to test the
  functionality of the code against all possible database states. I think
  pretty much every xmlrpc test will have some component of this.
 
  If certain assertions only make sense in a specific order, either put
  them in the same test*, or write additional code that explicitly handles
  dependencies and ordering. (Sorry, that's the price to pay for isolated
  tests – I don't see a way around it.)
 
  In an ideal world, we define a test and all possible parameters to that
  test and allow the running of that test independently by choosing from
  the proscribed inputs. This all works fine so long as you specify a way
  to indicate when inputs are mutually exclusive. Since inputs are single
  valued anyway, this should be the default behavior. Unfortunately,
  pytest fails on both of these counts.
 
  If on the other hand the order doesn't matter – any order of any subset
  of individual tests should pass, which is our goal – but pytest's
  default ordering makes the suite slow, there is a hook that allows you
  to override the order: pytest_collection_modifyitems [0]. Stick it in
  the affected module (or conftest.py file for multiple modules, or a
  plugin for the whole suite), and sort the items explicitly.
 
  The problem isn't (I think) ordering but lifecycle. I think that if I
  scope the fixture to the function but reorder the items, they will still
  be constructed and torn down for every single function iteration. On the
  other hand, if I scope the fixture for a longer lifecycle, the
  parameters may not be mutually exclusive.
 
  If your test depends on something not existing or being disabled, you
  should try removing/disabling it before the test, or write a fixture to
  do that.
 
  That's not the problem. The problem is that I can't define parameters
  for a single object which sets the same variable to different values.
  Such an act means the parameters are mutually exclusive.
 
 OK, I see the problem now. Pytest fixtures are meant for resources, not 
 states of a shared object.
 You ran into a fairly exotic use case (which is what I feared, so I 
 wanted to look into porting this.)
 I think in this case we should use using separate users for the 
 enablement states, since user creation is not expensive. Or am I missing 
 something else?

Yes, two things.

First, while a single user creation isn't expensive (~5 seconds w/ IPA
running on tmpfs), 360 user creations *are* (~30 minutes). This figure
does not include user deletions which have a similar run-time. So I
estimate creating a user for every test adds about 45 minutes to a full
test run. This is entirely unnecessary.

Second, there is a much worse case: global otp enablement. We can't
guarantee the changes take effect without sleeping for 60 seconds on
every modification (see 8b2f4443dcf61e1edf59ef0812ed05e1fa93f8fc). This
means 2 minutes for every enablement/disablement cycle. This isn't a big
deal if we do it once. If we do it 360 times, however... You can do the
math. It isn't pretty.

However, if we put this in a fixture which has module/class scope and
allow the parameters to be mutually exclusive (i.e. the finalizer for
the previous parameter is called immediately before construction of the
next parameter) this problem goes away.

  * might make sense, for starters?
  [0]
  http://pytest.org/latest/plugins.html#_pytest.hookspec.pytest_collection_modifyitems
 
 
  Most painful, is 

Re: [Freeipa-devel] topology management question

2014-12-09 Thread Simo Sorce
On Tue, 09 Dec 2014 09:21:43 -0500
Rob Crittenden rcrit...@redhat.com wrote:

 Simo Sorce wrote:
  On Sun, 07 Dec 2014 10:25:06 -0500
  Rob Crittenden rcrit...@redhat.com wrote:
  
  Simo Sorce wrote:
  On Thu, 04 Dec 2014 14:33:09 +0100
  Ludwig Krispenz lkris...@redhat.com wrote:
 
  hi,
 
  I just have another (hopefully this will end soon) issue I want
  to get your input. (please read to teh end first)
 
  To recapture the conditions:
  -  the topology plugin manages the connections between servers
  as segments in the shared tree
  - it is authoritative for managed servers, eg it controls all 
  connections between servers listed under cn=masters,
 it is permissive for connection to other servers
  - it rejects any removal of a segment, which would disconnect the
  topology.
  - a change in topology can be applied to any server in the
  topology, it will reach the respective servers and the plugin
  will act upon it
 
  Now there is a special case, causing a bit of trouble. If a
  replica is to be removed from the topology, this means that
  the replication agreements from and to this replica should be
  removed, the server should be removed from the manages servers.
  The problem is that:
  - if you remove the server first, the server becomes unmanaged
  and removal of the segment will not trigger a removal of the
  replication agreement
 
  I had another, sort of unrelated thought about this, thinking about
  deleting servers.
 
  What happens if a replication conflict entry gets added?
  
  This would happen in case a provisioning system tries to
  instantiate 2 servers with the same name at the same time talking
  to different existing masters.
 
 Sure and quite possible if there is some link down and two admins
 doing the same thing.
 
  
  While both exist I imagine that the actual agreement would reflect
  whichever entry is processed last. Probably not the end of the
  world.
 
  But how do you remove the conflict entry without also potentially
  deleting that master?
   
  It should probably delete both, the domain would be pretty messed up
  anyone and we have no easy way to know which of the 2 is part of the
  domain and which one has all replication severed due to their keys
  being overwritten by the other server ones.
 
 Yes, this is what I was leaning towards as well, but the plugin may
 need to specifically do this. It all depends on the search filters
 Ludwig uses to find the master(s) to operate on. And should this be
 automatically detected?
 
 So in reality the fact that there is a duplicate agreement in topology
 probably won't hurt anything at all since we don't really define much
 there. The actual agreement(s) will still be created just fine, but
 this extra topology record could be confusing depending on whether it
 gets returned by anything.
 
  I guess the only thing we can reasonably do is to make
  recommendations on how to deal with replicas deployments to avoid
  this case and instructions on how to remove remnants entries if any.
 
 I brought this up in case deleting the conflict entry would create an
 orphan, for example.
 
 It occurs to me that perhaps the conflict entry may not actually
 contain anything different than the real entry. It isn't like we
 store a ton of data. So I wonder if one deletes a conflict entry it
 shouldn't trigger topology changes. But that is likely going to
 require extra logic.

I think we can avoid this situation completely going forward by
introducing the concept of ephemeral replication master (what a
mouthful eh? :).

The idea is that when creating a new replica the installer always
determines deterministically a server among all to contact and creates
the master entry there with an add operation.

So if two masters try to be installed they will conflict early and one
will fail.
If we are in split-brain the one on the wrong side will fail to contact
the master and fail.

If the agreed master is down, no replicas can be installed until it is
restored or removed (or maybe a force flag is used to work around it
for whatever reason).

I think this could work, but I am not sure how well it will mesh with
the new install procedure work that promotes a normal client because
then the basic host keytab will be already broken possibly.

So perhaps we keep this possible plan in mind (open a ticket) ? But
wait a bit to understand what we will end up with on the promotion
changes side.

Simo.

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

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


Re: [Freeipa-devel] [PATCH 0036] Add missing python files to Makefile

2014-12-09 Thread Martin Basti

On 04/12/14 14:18, Gabe Alford wrote:

Thanks for the assistance Lukas! I have an updated patch attached.

Thanks,

Gabe

On Wed, Dec 3, 2014 at 9:30 AM, Lukas Slebodnik lsleb...@redhat.com 
mailto:lsleb...@redhat.com wrote:


On (02/12/14 21:13), Gabe Alford wrote:
This patch removes the changelog and Makefile.am for ipaclient as
well.

Thanks,

Gabe

On Mon, Dec 1, 2014 at 8:28 AM, Martin Kosek mko...@redhat.com
mailto:mko...@redhat.com wrote:

 On 12/01/2014 04:25 PM, Rob Crittenden wrote:
  Gabe Alford wrote:
 
  On Mon, Dec 1, 2014 at 6:05 AM, Martin Kosek
mko...@redhat.com mailto:mko...@redhat.com
  mailto:mko...@redhat.com mailto:mko...@redhat.com wrote:
 
  On 11/30/2014 03:28 AM, Gabe Alford wrote:
   Ignore the last patch. Updated patch attached.
  
   On Sat, Nov 29, 2014 at 6:03 PM, Gabe Alford
  redhatri...@gmail.com mailto:redhatri...@gmail.com
mailto:redhatri...@gmail.com mailto:redhatri...@gmail.com wrote:
  
   This patch removes the app_PYTHON usage.
  
   Thanks,
  
   Gabe
  
   On Thu, Nov 27, 2014 at 9:40 AM, Martin Kosek
mko...@redhat.com mailto:mko...@redhat.com
  mailto:mko...@redhat.com mailto:mko...@redhat.com
wrote:
  
   Exactly, this was the message from Martin :-) I did
not test it
  myself,
   but
   removing all app_PYTHON should be benign given we
use Python
  setup.py
   packaging.
  
   On 11/27/2014 04:27 PM, Gabe Alford wrote:
   Thanks guys. Sounds like it would be better to
submit a patch
 that
   removes
   app_PYTHON if it is considered dead code.
  
   Gabe
  
   On Thursday, November 27, 2014, Petr Spacek 
 pspa...@redhat.com mailto:pspa...@redhat.com
  mailto:pspa...@redhat.com mailto:pspa...@redhat.com
wrote:
  
   On 27.11.2014 11:00, Martin Basti wrote:
   On 27/11/14 00:50, Gabe Alford wrote:
   Hello,
  
   Wondering if I could get a review. Updated patch
  attached.
  
   Thanks,
   Gabe
  
   On Tue, Nov 11, 2014 at 7:21 AM, Gabe Alford
  redhatri...@gmail.com mailto:redhatri...@gmail.com
mailto:redhatri...@gmail.com mailto:redhatri...@gmail.com
   javascript:;
   mailto:redhatri...@gmail.com
mailto:redhatri...@gmail.com mailto:redhatri...@gmail.com
mailto:redhatri...@gmail.com
 
  javascript:; wrote:
  
Hello,
  
Fix for
https://fedorahosted.org/freeipa/ticket/4700
  
Thanks,
  
Gabe
  
  
  
   Hello,
  
   sorry for late response.
  
   We push this ticket to backlog, as it would be
part of build
  system
   refactoring.
   The app_PYTHON statement is not used anymore in
IPA, the
 better
   solution is
   remove it, instead of keeping dead code up-to-date.
  
   Just to clarify:
   It can be pushed if it works, there is no need to
postpone
  accepting
   patch
   if
   the patch seems okay and doesn't break anything.
  
   Martin, please keep in mind that contributions are
welcome at
  any time.
  
   Milestones in Trac reflect our view of priorities
but it
 doesn't
   prevent us
   from accepting correct patches from contributions
at any
 time, no
   matter
   which
   priority is stated in Trac (or even if there is no
ticket for
  it ...).
  
   --
   Petr^2 Spacek
 
  Worked in my tests, I did not see any breakage. I guess
we can also
  remove the
   ipa-client/ipaclient/Makefile.am while we are at it.
 
  Martin
 
 
  It looks like the ipaclient/Makefile.am is still being used.
I tried
  removing it and there were errors in the build, but maybe I
am wrong?
 
  It is needed to build ipa-join, ipa-getkeytab and ipa-rmkeytab.
 
  Feel free to rip out the outdated hg ChangeLog stuff though.
 
  rob

 I think Gabe was asking about ipa-client/ipaclient/Makefile.am
and not
 about
 ipa-client/Makefile.am - we still need this one as Rob
correctly said.

 The failure that Gabe hit in build probably comes from the the
SUBDIR
 reference
 in ipa-client/Makefile.am file. I assume that if the reference
is removed,
 the
 removal should work.

 And yes, you can remove the Changelog too if you are OK with it :)

 Martin


[Freeipa-devel] [PATCH 0177] Fix adding (warning) messages on client side

2014-12-09 Thread Martin Basti

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

I'm able to reproduce it only in one nose test.

Patch attached.

--
Martin Basti

From a50874ff0d32b004545b76f987fc402548bd2de8 Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Tue, 9 Dec 2014 12:27:42 +0100
Subject: [PATCH] Fix adding messages on client side

https://fedorahosted.org/freeipa/ticket/4793
---
 ipalib/messages.py | 12 +++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/ipalib/messages.py b/ipalib/messages.py
index b44beca729f5483a7241e4c98a9f724ed663e70f..3076879009312fd342c652c57dbb6555dc3a905d 100644
--- a/ipalib/messages.py
+++ b/ipalib/messages.py
@@ -39,8 +39,18 @@ from ipalib.capabilities import client_has_capability
 
 
 def add_message(version, result, message):
+
+Add new message to result.
+Result can contain messages in tuple on client and in list on server.
+
 if client_has_capability(version, 'messages'):
-result.setdefault('messages', []).append(message.to_dict())
+messages = result.get('messages')
+if isinstance(messages, tuple):
+# messages are in tuple, concatenate it with new message
+result['messages'] = messages + (message.to_dict(),)
+else:
+# None, list
+result.setdefault('messages', []).append(message.to_dict())
 
 
 def process_message_arguments(obj, format=None, message=None, **kw):
-- 
1.8.3.1

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

Re: [Freeipa-devel] [PATCH 0170] Detect and warn about invalid forwardzone configuration

2014-12-09 Thread Martin Basti

On 01/12/14 17:44, Petr Spacek wrote:

On 1.12.2014 14:39, Martin Basti wrote:

On 24/11/14 17:24, Petr Spacek wrote:

Hello!

Thank you for the patch. It is not ready yet but the overall direction seems
good. Please see my comments in-line.

On 24.11.2014 14:35, Martin Basti wrote:

Ticket: https://fedorahosted.org/freeipa/ticket/4721
Patch attached

--
Martin Basti


freeipa-mbasti-0170-Detect-and-warn-about-invalid-DNS-forward-zone-confi.patch


  From a5a19137e3ddf2d2d48cfbdb2968b6f68ac8f772 Mon Sep 17 00:00:00 2001
From: Martin Basti mba...@redhat.com
Date: Fri, 21 Nov 2014 16:54:09 +0100
Subject: [PATCH] Detect and warn about invalid DNS forward zone configuration

Shows warning if forward and parent authoritative zone do not have
proper NS record delegation, which can cause the forward zone will be
ineffective and forwarding will not work.

The chande in the test was required due python changes order of elemnts
in dict (python doesnt guarantee an order in dict)

Ticket: https://fedorahosted.org/freeipa/ticket/4721
---
   ipalib/messages.py  |  12 +++
   ipalib/plugins/dns.py   | 147
+---
   ipatests/test_xmlrpc/test_dns_plugin.py |   2 +-
   3 files changed, 149 insertions(+), 12 deletions(-)

diff --git a/ipalib/messages.py b/ipalib/messages.py
index
102e35275dbe37328c84ecb3cd5b2a8d8578056f..cc9bae3d6e5c7d3a7401dab89fafb1b60dc1e15f
100644
--- a/ipalib/messages.py
+++ b/ipalib/messages.py
@@ -200,6 +200,18 @@ class
DNSServerDoesNotSupportDNSSECWarning(PublicMessage):
  uIf DNSSEC validation is enabled on IPA server(s), 
  uplease disable it.)
   +class ForwardzoneIsNotEffectiveWarning(PublicMessage):
+
+**13008** Forwardzone is not effective, forwarding will not work because
+there is authoritative parent zone, without proper NS delegation
+
+
+errno = 13008
+type = warning
+format = _(uforward zone \%(fwzone)s\ is not effective (missing
proper 
+   uNS delegation in authoritative zone \%(authzone)s\). 
+   uForwarding may not work.)

I think that the error message could be made mode useful:

Forwarding will not work. Please add NS record fwdzone.relativize(authzone)
to parent zone %(authzone)s (or something like that).


+
 def iter_messages(variables, base):
   Return a tuple with all subclasses
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index
c5d96a8c4fcdf101254ecefb60cb83d63bee6310..4f92da645e0faf784c7deb4b8ce386c1440f4229
100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -1725,6 +1725,79 @@ def _normalize_zone(zone):
   return zone
 +def _find_zone_which_makes_fw_zone_ineffective(fwzonename):

Generally, this method finds an authoritative zone for given fwzonename.
What about method name _find_auth_zone_ldap(name) ?


+
+Check if forward zone is effective.
+
+If parent zone exists as authoritative zone, forward zone will not
+forward queries. It is necessary to delegate authority of forward zone

I would clarify it: forward queries by default.



+to another server (non IPA DNS server).

I would personally omit (non IPA DNS server) because this mechanism is not
related to IPA domain at all.


+Example:
+
+Forward zone: sub.example.com
+Zone: example.com
+
+Forwarding will not work, because server thinks it is authoritative
+and returns NXDOMAIN
+
+Adding record: sub.example.com NS nameserver.out.of.ipa.domain.

Again, this is not related to IPA domain at all. I propose this text:
Adding record: sub.example.com NS ns.sub.example.com.


+will delegate authority to another server, and IPA DNS server will
+forward DNS queries.
+
+:param fwzonename: forwardzone
+:return: None if effective, name of authoritative zone otherwise
+
+assert isinstance(fwzonename, DNSName)
+
+# get all zones
+zones = api.Command.dnszone_find(pkey_only=True,
+sizelimit=0)['result']

Ooooh, are you serious? :-) I don't like this approach, I would rather chop
left-most labels from name and so dnszone_find() one by one. What if you
have many DNS zones?


This could be thrown away if you iterate only over relevant zones.

+zonenames = [zone['idnsname'][0].make_absolute() for zone in zones]
+zonenames.sort(reverse=True, key=len)  # sort, we need longest match
+
+# check if is there a zone which can cause the forward zone will
+# be ineffective
+sub_of_auth_zone = None
+for name in zonenames:
+if fwzonename.is_subdomain(name):
+sub_of_auth_zone = name
+break
+
+if sub_of_auth_zone is None:
+return None
+
+# check if forwardzone is delegated in authoritative zone
+# test if exists and get NS record
+try:
+ns_records = api.Command.dnsrecord_show(
+sub_of_auth_zone, fwzonename, raw=True)['result']['nsrecord']
+except 

Re: [Freeipa-devel] [PATCH] 0677 test_integration: Use python-pytest-multihost

2014-12-09 Thread Petr Viktorin

On 11/26/2014 04:18 PM, Petr Viktorin wrote:

Hello,

I have split FreeIPA's multi-host testing infrastructure into a separate
project. It is temoprarily available at:
 https://github.com/encukou/pytest-multihost
and I will move it to fedorahosted as soon as it's approved:
 https://fedorahosted.org/fedora-infrastructure/ticket/4605
RPMs for Fedora 20..rawhide and EPEL 7 are available in COPR:
 https://copr.fedoraproject.org/coprs/pviktori/pytest-plugins/

This patch contains the necessary changes to FreeIPA. The tests
themselves are almost unchanged. FreeIPA specific parts (most
importantly, logging and environ-based configuration) are also left in.


Hi Tomáš! Thanks for testing the patch and finding a problem in the 
pytest-multihost library. It is now solved.


I'm also attaching two smaller patches that fix bugs in the pytest port.


--
Petr³
From aedb828ca268ad8d8e2901ebead0c1e951f4b526 Mon Sep 17 00:00:00 2001
From: Petr Viktorin pvikt...@redhat.com
Date: Thu, 13 Nov 2014 16:23:56 +0100
Subject: [PATCH] test_integration: Use python-pytest-multihost

The core integration testing functionality was split into a separate
project. Use this project, and configure it for FreeIPA.

The mh (multihost) fixture is made available for integration tests.

Configuration based on environment variables is moved into a separate
module, to ease eventual deprecation.
---
 freeipa.spec.in|   2 +-
 ipatests/ipa-test-config   |   5 +-
 ipatests/ipa-test-task |   2 +
 ipatests/pytest_plugins/integration.py | 117 +++---
 ipatests/test_integration/base.py  |  11 +-
 ipatests/test_integration/config.py| 420 +++
 ipatests/test_integration/env_config.py| 356 +
 ipatests/test_integration/host.py  | 238 +--
 ipatests/test_integration/tasks.py |   2 +-
 ipatests/test_integration/test_caless.py   |  35 +-
 .../test_forced_client_reenrollment.py |   6 +-
 ipatests/test_integration/test_testconfig.py   |   6 +-
 ipatests/test_integration/test_trust.py|   2 +-
 ipatests/test_integration/transport.py | 443 -
 ipatests/test_integration/util.py  |  10 -
 15 files changed, 525 insertions(+), 1130 deletions(-)
 create mode 100644 ipatests/test_integration/env_config.py
 delete mode 100644 ipatests/test_integration/transport.py

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 9b12c20899e729cedacdee470f8f2b13250af4e0..f4218d4098204403aa86a66070439be3724461db 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -310,7 +310,7 @@ Requires: pytest = 2.6
 Requires: python-paste
 Requires: python-coverage
 Requires: python-polib
-Requires: python-paramiko = 1.7.7
+Requires: python-pytest-multihost = 0.2
 
 Conflicts: %{alt_name}-tests
 Obsoletes: %{alt_name}-tests  %{version}
diff --git a/ipatests/ipa-test-config b/ipatests/ipa-test-config
index dc94b8afb8afd6f24f0806a2fc2c74d445f2d336..6a3101f63ee5e16f675849e3390e91c39350326e 100755
--- a/ipatests/ipa-test-config
+++ b/ipatests/ipa-test-config
@@ -25,7 +25,7 @@ import argparse
 import json
 
 from ipalib.constants import FQDN
-from ipatests.test_integration import config
+from ipatests.test_integration import config, env_config
 
 
 def main(argv):
@@ -92,7 +92,8 @@ def main(argv):
 import yaml
 return yaml.safe_dump(conf.to_dict(), default_flow_style=False)
 else:
-return config.env_to_script(get_object(conf, args).to_env(**kwargs))
+env = get_object(conf, args).to_env(**kwargs)
+return env_config.env_to_script(env)
 
 
 def get_object(conf, args):
diff --git a/ipatests/ipa-test-task b/ipatests/ipa-test-task
index 612974549363277fdfe101734cf9defc59c99ab8..d89af841de9f8558ca620989fb665e6f3e2c573c 100755
--- a/ipatests/ipa-test-task
+++ b/ipatests/ipa-test-task
@@ -248,6 +248,8 @@ class TaskRunner(object):
 
 args = self.get_parser().parse_args(argv)
 self.config = config.Config.from_env(os.environ)
+if not self.config:
+raise EnvironmentError('Multihost environment not configured')
 
 logs_to_collect = {}
 
diff --git a/ipatests/pytest_plugins/integration.py b/ipatests/pytest_plugins/integration.py
index 5329e5190a78b37b3881fb3a5b7bd6b0c5ad215b..a6c09518ff4602d31eb37a9cbc27be3ae752ea29 100644
--- a/ipatests/pytest_plugins/integration.py
+++ b/ipatests/pytest_plugins/integration.py
@@ -24,10 +24,13 @@
 import shutil
 
 import pytest
+from pytest_multihost import make_multihost_fixture
 
 from ipapython import ipautil
 from ipapython.ipa_log_manager import log_mgr
-from ipatests.test_integration.config import get_global_config
+from ipatests.test_integration import tasks
+from ipatests.test_integration.config import Config
+from ipatests.test_integration.env_config import get_global_config
 
 
 log 

Re: [Freeipa-devel] [PATCH] 793-794 Fix schema related replication issues between IPA-3-0 and IPA-4-1

2014-12-09 Thread Jan Cholasta

Hi,

Dne 5.12.2014 v 14:33 Petr Vobornik napsal(a):

Hello,
I've transformed Thierry's and Ludwig's findings of bz 1167964 [1] and
ticket  4794 [2] into patches.

I wonder if the mgrpRFC822MailMember and nsViewFilter issue(patch 794)
should be solved on 389's side rather than on FreeIPA's?

Also is the increase of nsslapd-sasl-max-buffer-size necessary? With
these two patches, replication appears to work fine for me. Tested with
F21 FreeIPA 4.1.x-GIT-something and ipa-server-3.0.0-42.el6.x86_64

[1] https://bugzilla.redhat.com/show_bug.cgi?id=1167964
[2] https://fedorahosted.org/freeipa/ticket/4794


Patch 793: Works for me, ACK.

Pushed to:
master: 489dfe64689f86f7ddc4ad0784de0636f8e6c1f8
ipa-4-1: 2fa07b1d24f61f9bcff5adb804a18c9eae72932d


Patch 794: As Thierry pointed out, this patch is not necessary, as the 
bug is fixed by patch 793 alone.


Not pushed.


Honza

--
Jan Cholasta

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