[Freeipa-devel] [PATCH 0342] Use domain level constants in topology plugin

[Martin Basti]

Patch attached.
From 16aa96f0446a7b351763fc8061497d82fdeedbdf Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Tue, 3 Nov 2015 10:37:15 +0100
Subject: [PATCH] Use domain level constants in topology plugin

---
 ipalib/plugins/topology.py | 7 +++
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/ipalib/plugins/topology.py b/ipalib/plugins/topology.py
index 2b82215e273d959fdb207801ed146b843460bae5..2c43758c3136a546a50472cf97d79129d1d6c738 100644
--- a/ipalib/plugins/topology.py
+++ b/ipalib/plugins/topology.py
@@ -12,6 +12,7 @@ from ipalib.plugins.baseldap import (
 LDAPRetrieve)
 from ipalib import _, ngettext
 from ipalib import output
+from ipalib.constants import DOMAIN_LEVEL_1
 from ipalib.util import create_topology_graph, get_topology_connection_errors
 from ipapython.dn import DN
 
@@ -28,15 +29,13 @@ Requires minimum domain level 1.
 
 register = Registry()
 
-MINIMUM_DOMAIN_LEVEL = 1
-
 
 def validate_domain_level(api):
 current = int(api.Command.domainlevel_get()['result'])
-if current < MINIMUM_DOMAIN_LEVEL:
+if current < DOMAIN_LEVEL_1:
 raise errors.InvalidDomainLevelError(
 reason=_('Topology management requires minimum domain level {0} '
-   .format(MINIMUM_DOMAIN_LEVEL))
+   .format(DOMAIN_LEVEL_1))
 )
 
 
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Re: [Freeipa-devel] [PATCH 0011, 0012, 0015] Replica promotion related changes in integration tests

[Martin Basti]

On 02.11.2015 16:05, Oleg Fayans wrote:



On 11/02/2015 03:45 PM, Martin Basti wrote:



On 02.11.2015 14:45, Oleg Fayans wrote:

Hi Martin,

On 11/02/2015 12:52 PM, Martin Basti wrote:



On 02.11.2015 11:54, Oleg Fayans wrote:

Hi Martin,

On 11/02/2015 10:39 AM, Martin Basti wrote:



On 29.10.2015 18:32, Martin Basti wrote:



On 29.10.2015 18:31, Martin Basti wrote:

NACK

1)
DO NOT use tabs in code to indent

Fixed


2)
Replica uninstallation does not work, uninstallation works 
different

with domain level 0 and 1 (currently uninstallation with domain 1
level will not work, it is known issue, but still the patch should
solve the uninstallation)

This is not valid, my bad, I was confused with new behaviour of
replica
uninstallation, but it is bug not a feature.
So replica uninstallation is the same for level 0 and 1
Sorry.


3)
apply_common_fixes(host)
Method for domain_level 1 is called twice, first time in replica
install, second time in client install

Fixed


4)
during testing this patch I used test_simple_replication and I 
found

4 bugs:

3 bugs -^^^

#5419, #5420, #5421

Bug #5419 fixed, see patch N 15


patch 0015 NACK

1)
You fixed just half of  the issue, there is also wrong dnsrecord-add

2)
I do not think that your solution is the right, it can result in
failures when the domain 'my.ipa.domain' is used.

I prefer to add '.' to record name and use it as absolute name,
dnsrecord-* command will handle it.


Done


That's a good point. The problem though, is that dnsrecord-add handles
it correctly, while dnsrecord-find - does not.

$ ipa dnsrecord-add idm.lab.eng.brq.redhat.com.
vm-002.idm.lab.eng.brq.redhat.com. --a-rec 192.168.254.2
  Record name: vm-002
  A record: 192.168.254.2

$ ipa dnsrecord-find idm.lab.eng.brq.redhat.com.
vm-001.idm.lab.eng.brq.redhat.com.

Number of entries returned 0


$ ipa dnsrecord-find idm.lab.eng.brq.redhat.com. vm-001
  Record name: vm-001
  A record: 192.168.254.1

Number of entries returned 1


Should I file a ticket against it?


I do not think that this is a bug, dnsrecord-find does search in
multiple attributes not only in record names

I suggest to use dnsrecord-show instead of dnsrecord-find




if not host.hostname.endswith('.'):
 host.hostname += '{}.'.format(host.hostname)

And I would replace dnsrecord-find with dnsrecord-show

patches 11, 12 LGTM, I will test them later

Martin^2



IMO it is related only to this one test case and to pass this test
case #5419 or #5421 must be fixed.


On 27.10.2015 16:34, Oleg Fayans wrote:

Hi Martin,

The updated patch is attached

On 10/27/2015 01:58 PM, Martin Basti wrote:



On 27.10.2015 13:56, Oleg Fayans wrote:



On 10/27/2015 01:22 PM, Martin Basti wrote:



On 27.10.2015 12:06, Oleg Fayans wrote:

Hi Martin,

On 10/27/2015 10:50 AM, Martin Basti wrote:



On 27.10.2015 10:22, Martin Basti wrote:



On 27.10.2015 10:00, Oleg Fayans wrote:

Hi Martin,

The updated version of the patch is attached. Please, 
see my

comments
below
My comments inline, I may be completely wrong in how the 
test

suite
work, so feel free to correct me.

Martin



On 10/26/2015 06:48 PM, Martin Basti wrote:



On 26.10.2015 08:59, Oleg Fayans wrote:



On 10/23/2015 03:10 PM, Martin Basti wrote:



On 23.10.2015 15:00, Oleg Fayans wrote:

Hi Martin,

Here comes the updated version.

On 10/22/2015 05:38 PM, Martin Basti wrote:



On 22.10.2015 15:23, Martin Basti wrote:


On 22.10.2015 14:13, Oleg Fayans wrote:






Hello,

thank you for the patch.

1)
please remove the added empty lines, they are
unrelated to
this
ticket


done



2)
-def install_master(host, setup_dns=True,
setup_kra=False):
+def install_master(host, setup_dns=True,
setup_kra=False,
domainlevel=1):

I suggest to use default domainlevel=None, which 
will

use the
default
domain level (specified in build)


done



3)
+domain_level = domainlevel(master)
I do not think that this meets expectations.

We have to test, both domain level 0 and 1 for IPA
4.3,
respectively
new IPA must support all older domain levels, domain
level is
independent on IPA version, only admin can raise it
up.

So you have to find out way how to pass the domain
level for
which
test will be running, we were talking about using
config
files,
but
feel free to find something new and better


Fixed. Now, we declare domain level in config.yaml 
with

the
directive
domain_level



4)
Did you resolve the pytest fixtures which specifies
which
tests
can be
run under which domain level?


In fact, we do not seem to have any tests yet that 
would

require it.
All the existing tests just use install_replica
 method, no matter how is it done.

How about topology CI test? This can be executed only
with
domain
level


That's right. The topology test was updated. Patch is
attached
together with a proper version of 11-th patch (not a 
swap

file,
sorry
about that).

Re: [Freeipa-devel] [PATCH 0329] Tests: fix user tracker

[Lenka Doudova]

On 10/26/2015 06:05 PM, Martin Basti wrote:



On 26.10.2015 09:01, Lenka Doudova wrote:



On 10/21/2015 06:53 AM, Lenka Doudova wrote:



On 10/20/2015 06:21 PM, Martin Basti wrote:



On 20.10.2015 15:53, Martin Basti wrote:



On 19.10.2015 14:16, Martin Basti wrote:



On 19.10.2015 12:30, Martin Basti wrote:

Attribute nsaccountlock has not been processed correctly

Patch attached.




Self-NACK, more fixes required



Updated patch attached, but it still needs to improve because 
tests in my patch 331 are still failing.




Eternal self-NACK for this patch

I'm not able to fix UserTracker, I need help from somebody with 
higher view of how this tracker is supposed to work.

Follow my patch 0331


Hi, I'll take a look at it today.
Lenka



Hi,

I fixed the trackers and tests, rebased patch attached.
Lenka




Thank you,

1)
* Module ipatests.test_xmlrpc.test_stageuser_plugin
ipatests/test_xmlrpc/test_stageuser_plugin.py:938: 
[E0102(function-redefined), TestMultipleManagers] class already 
defined line 913)


2)
Because the patch contains tests too, I suggest to rename patch to 
Multiple manager per user tests.

Also you should change commiter of patch to you.

Martin^2

Fixed patch attached.
Lenka
From aca3dfb072c8bb86efd7fe247157d630a332e691 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Tue, 3 Nov 2015 10:03:15 +0100
Subject: [PATCH] Multiple manager per user tests

Multiple managers per user tests
---
 ipatests/test_xmlrpc/test_stageuser_plugin.py |  52 -
 ipatests/test_xmlrpc/test_user_plugin.py  | 103 +-
 2 files changed, 134 insertions(+), 21 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py
index b09ef6e84cd95a32061b07d833c5a39f1750f80b..bd3e790fb4c318f449a9e36763245f2ea7f39924 100644
--- a/ipatests/test_xmlrpc/test_stageuser_plugin.py
+++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py
@@ -100,10 +100,10 @@ class StageUserTracker(Tracker):
 u'usercertificate', u'dn', u'has_keytab', u'has_password',
 u'street', u'postalcode', u'facsimiletelephonenumber',
 u'carlicense', u'ipasshpubkey', u'sshpubkeyfp', u'l',
-u'st', u'mobile', u'pager', }
+u'st', u'mobile', u'pager', u'manager'}
 retrieve_all_keys = retrieve_keys | {
 u'cn', u'ipauniqueid', u'objectclass', u'description',
-u'displayname', u'gecos', u'initials', u'krbprincipalname', u'manager'}
+u'displayname', u'gecos', u'initials', u'krbprincipalname'}
 
 create_keys = retrieve_all_keys | {
 u'objectclass', u'ipauniqueid', u'randompassword',
@@ -184,7 +184,12 @@ class StageUserTracker(Tracker):
 (self.kwargs[key].split('@'))[0].lower(),
 (self.kwargs[key].split('@'))[1])]
 elif key == u'manager':
-self.attrs[key] = [unicode(get_user_dn(self.kwargs[key]))]
+if isinstance(self.kwargs[key], list):
+self.attrs[key] = [
+unicode(get_user_dn(item))
+for item in self.kwargs[key]]
+else:
+self.attrs[key] = [unicode(get_user_dn(self.kwargs[key]))]
 elif key == u'ipasshpubkey':
 self.attrs[u'sshpubkeyfp'] = [sshpubkeyfp]
 self.attrs[key] = [self.kwargs[key]]
@@ -891,3 +896,44 @@ class TestGroups(XMLRPC_test):
 command = group.make_add_member_command(options={u'user': user.uid})
 result = command()
 group.check_add_member_negative(result)
+
+
+@pytest.fixture(scope='class')
+def manager1(request):
+t = UserTracker(u"manager1", u"manager", u"manager1")
+return t.make_fixture(request)
+
+
+@pytest.fixture(scope='class')
+def manager2(request):
+t = UserTracker(u"manager2", u"manager", u"manager2")
+return t.make_fixture(request)
+
+
+class TestMultipleManagers(XMLRPC_test):
+"""Tests for: https://fedorahosted.org/freeipa/ticket/5344"";
+def test_multiple_managers_per_stageduser(self, manager1, manager2,
+  stageduser):
+manager1.create()
+manager2.create()
+stageduser.create()
+
+stageduser.update(
+updates={u"manager": [manager1.name, manager2.name]},
+expected_updates={u"manager": [manager1.name, manager2.name]})
+
+def test_find_stageuser_with_multiple_managers(self, manager1, manager2,
+   stageduser):
+command = stageduser.make_find_command(
+manager=[manager1.name, manager2.name])
+result = command()
+stageduser.check_find(result)
+
+def test_create_new_stageduser_with_multiple_managers(
+self, manager1, manager2, stageduser):
+stageduser.ensure_missing()
+command = stageduser.make_create_command(
+

Re: [Freeipa-devel] [PATCH 0340 - 0341] DNSSEC restore state fixes in uninstall

[Martin Basti]

On 03.11.2015 10:19, Petr Spacek wrote:

On 2.11.2015 20:21, Martin Basti wrote:

Attached patches removes the sysrestore states that has been left in
sysrestore.state file after uninstall

ACK


Pushed to master: 0d66026d220dd675e9b017db37127b822815cf4a

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH 0343] Upgrade: enable custodia service during upgrade

[Martin Basti]

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

Patch attached.
From 5d57f6ed48260606261eb53a2b250295538c1d69 Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Tue, 3 Nov 2015 18:33:17 +0100
Subject: [PATCH] Upgrade: enable custodia service during upgrade

There was missing step in upgrade that enables the service in LDAP

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

diff --git a/ipaserver/install/custodiainstance.py b/ipaserver/install/custodiainstance.py
index eb9512bf131cf73af3c6bf69bcaecc7b57d100ad..c2ecd397063db3dfe27006232831023d865aac40 100644
--- a/ipaserver/install/custodiainstance.py
+++ b/ipaserver/install/custodiainstance.py
@@ -5,7 +5,9 @@ from ipapython.secrets.client import CustodiaClient
 from ipaplatform.paths import paths
 from service import SimpleServiceInstance
 from ipapython import ipautil
+from ipapython.ipa_log_manager import root_logger
 from ipaserver.install import installutils
+from ipaserver.install import sysupgrade
 from base64 import b64encode, b64decode
 from jwcrypto.common import json_decode
 import shutil
@@ -45,6 +47,7 @@ class CustodiaInstance(SimpleServiceInstance):
   dm_password=dm_password,
   ldap_suffix=suffix,
   realm=self.realm)
+sysupgrade.set_upgrade_state('custodia', 'installed', True)
 
 def __gen_keys(self):
 KeyStore = IPAKEMKeys({'server_keys': self.server_keys,
@@ -52,10 +55,9 @@ class CustodiaInstance(SimpleServiceInstance):
 KeyStore.generate_server_keys()
 
 def upgrade_instance(self):
-if not os.path.exists(self.config_file):
-self.__config_file()
-if not os.path.exists(self.server_keys):
-self.__gen_keys()
+if not sysupgrade.get_upgrade_state("custodia", "installed"):
+root_logger.info("Custodia service is being configured")
+self.create_instance()
 
 def create_replica(self, master_host_name):
 suffix = ipautil.realm_to_suffix(self.realm)
-- 
2.4.3

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [BLOG] FreeIPA PKI: current plans and a future vision

[Fraser Tweedale]

I have been alluding for a while about my ideas for future
FreeIPA/Dogtag PKI integration; I finally put the ideas down in a
blog post.  If you are interested in this aspect of IdM please read
it; all feedback is welcome!

http://blog-ftweedal.rhcloud.com/2015/11/freeipa-pki-current-plans-and-a-future-vision/

Thanks,
Fraser

-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

[Freeipa-devel] [PATCH] 0748 Handle encoding for ipautil.run

[Petr Viktorin]

Hello,
Python 3's strings are Unicode, so data coming to or leaving a Python
program needs to be decoded/encoded if it's to be handled as a string.
One of the boundaries where encoding is necessary is external programs,
specifically, ipautil.run.
Unfortunately there's no one set of options that would work for all
run() invocations, so I went through them all and specified the
stdin/stdout/stderr encoding where necessary. I've also updated the call
sites to make it clearer if the return values are used or not.
If an encoding is not set, run() will accept/return bytes. (This is a
fail-safe setting, since it can't raise errors, and using bytes where
strings are expected generally fails loudly in py3.)

Note that the changes are not effective under Python 2.

-- 
Petr Viktorin
From 0d586fb8b7a49dc67fb8bfff46b6a9f0e972a4f8 Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Tue, 27 Oct 2015 17:54:54 +0100
Subject: [PATCH] Handle encoding for ipautil.run

For Python 2, nothing is changed, and all data discussed below is
bytestrings (str).

Parts of the "argv" for run() may be strings or bytes, and may even be
mixed.

Stdin must be bytes (or None) by default.
If stdin_encoding is given, it may be a string, and is encoded appropriately.

Stdout and stderr are returned as bytes, unless specified otherwise.
This means there will be no decoding errors in the (very common) case
where these are ignored.
For the logs, these output streams are encoded with 'backslashreplace',
so errors can't be raised, and also binary garbage won't show up in logs.

If the stdout_encoding and/or stderr_encoding arguments are given,
the corresponding stream is decoded. This may raise an exception
if the data can't be encoded.
To suppress errors, pass {stdout,stderr}_strict=False. This will cause
invalid characters to be replaced with backslash sequences. It's appropriate
when the stream is copied to a log file, NOT when the data is further
manipulated.
---
 .../certmonger/dogtag-ipa-ca-renew-agent-submit|  9 ++-
 install/certmonger/ipa-server-guard| 11 +++-
 install/oddjob/com.redhat.idm.trust-fetch-domains  |  5 +-
 install/tools/ipa-adtrust-install  |  3 +-
 install/tools/ipa-replica-conncheck| 16 +++---
 ipa-client/ipa-install/ipa-client-install  | 31 +++
 ipalib/plugins/pwpolicy.py |  3 +-
 ipaplatform/base/services.py   | 26 -
 ipaplatform/redhat/services.py |  3 +-
 ipaplatform/redhat/tasks.py|  4 +-
 ipapython/certdb.py| 14 +++--
 ipapython/dnssec/bindmgr.py|  8 ++-
 ipapython/dnssec/odsmgr.py |  3 +-
 ipapython/ipautil.py   | 65 ++
 ipapython/kernel_keyring.py| 30 --
 ipaserver/dcerpc.py| 15 ++---
 ipaserver/install/cainstance.py| 24 +---
 ipaserver/install/certs.py |  2 +-
 ipaserver/install/httpinstance.py  |  4 +-
 ipaserver/install/ipa_backup.py| 29 +++---
 ipaserver/install/ipa_restore.py   | 24 ++--
 ipaserver/install/krbinstance.py   |  7 ++-
 ipaserver/install/opendnssecinstance.py|  7 ++-
 ipaserver/install/replication.py   |  2 +-
 ipaserver/install/server/install.py|  8 ++-
 ipatests/test_ipapython/test_keyring.py| 17 +++---
 ipatests/test_xmlrpc/test_host_plugin.py   |  2 +-
 27 files changed, 253 insertions(+), 119 deletions(-)

diff --git a/install/certmonger/dogtag-ipa-ca-renew-agent-submit b/install/certmonger/dogtag-ipa-ca-renew-agent-submit
index 44993b038a38da60a25843147e86b64deda874e1..93399845bb5213739057e56531d63d9be96b637c 100755
--- a/install/certmonger/dogtag-ipa-ca-renew-agent-submit
+++ b/install/certmonger/dogtag-ipa-ca-renew-agent-submit
@@ -156,8 +156,13 @@ def request_cert():
 args = [path] + sys.argv[1:]
 if os.environ.get('CERTMONGER_CA_PROFILE') == 'caCACert':
 args += ['-N', '-O', 'bypassCAnotafter=true']
-stdout, stderr, rc = ipautil.run(args, raiseonerr=False, env=os.environ)
-sys.stderr.write(stderr)
+stdout, stderr, rc = ipautil.run(args, raiseonerr=False, env=os.environ,
+ stdout_encoding='ascii')
+if six.PY2:
+sys.stderr.write(stderr)
+else:
+# Write bytes directly
+sys.stderr.buffer.write(stderr)
 sys.stderr.flush()
 
 syslog.syslog(syslog.LOG_NOTICE, "dogtag-ipa-renew-agent returned %d" % rc)
diff --git a/install/certmonger/ipa-server-guard b/install/certmonger/ipa-server-guard
index 7ce3e43fce16ce9974e5db10af7cf851c0411943..abd384f481e9dd32ba0d6a5c5d780d20e40a8943 100755
--- a/install/certmonger/ipa-server-guard
+++ 

Re: [Freeipa-devel] [PATCH 505] install: fix command line option validation

[Martin Babinsky]

On 11/03/2015 09:41 AM, Jan Cholasta wrote:

Hi,

the attached patch fixes:




Honza




Once more for the whole list, ACK.

--
Martin^3 Babinsky

--
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code