[Freeipa-devel] [PATCHES 523-525] replica install: add remote connection check over API

[Jan Cholasta]

Hi,

the attached patches fix .

Honza

--
Jan Cholasta
From 142fd5364cf9d31d7e2c35959516ac8d9054c9da Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 9 Dec 2015 08:17:07 +0100
Subject: [PATCH 1/3] build: put oddjob scripts into separate directory

https://fedorahosted.org/freeipa/ticket/5497
---
 freeipa.spec.in  | 3 ++-
 install/oddjob/Makefile.am   | 2 +-
 install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index a60d9b6..95948e7 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -740,6 +740,7 @@ fi
 %{_libexecdir}/ipa/ipa-dnskeysync-replica
 %{_libexecdir}/ipa/ipa-ods-exporter
 %{_libexecdir}/ipa/ipa-httpd-kdcproxy
+%dir %{_libexecdir}/ipa/oddjob
 %ghost %verify(not owner group) %dir %{_sharedstatedir}/kdcproxy
 %dir %attr(0755,root,root) %{_sysconfdir}/ipa/kdcproxy
 %config(noreplace) %{_sysconfdir}/sysconfig/ipa_memcached
@@ -914,7 +915,7 @@ fi
 %ghost %{_libdir}/krb5/plugins/libkrb5/winbind_krb5_locator.so
 %{_sysconfdir}/dbus-1/system.d/oddjob-ipa-trust.conf
 %{_sysconfdir}/oddjobd.conf.d/oddjobd-ipa-trust.conf
-%%attr(755,root,root) %{_libexecdir}/ipa/com.redhat.idm.trust-fetch-domains
+%attr(755,root,root) %{_libexecdir}/ipa/oddjob/com.redhat.idm.trust-fetch-domains
 
 %endif # ONLY_CLIENT
 
diff --git a/install/oddjob/Makefile.am b/install/oddjob/Makefile.am
index 9dde10c..5cdaf2b 100644
--- a/install/oddjob/Makefile.am
+++ b/install/oddjob/Makefile.am
@@ -1,6 +1,6 @@
 NULL =
 
-oddjobdir = $(libexecdir)/ipa
+oddjobdir = $(libexecdir)/ipa/oddjob
 oddjobconfdir = $(sysconfdir)/oddjobd.conf.d
 dbusconfdir = $(sysconfdir)/dbus-1/system.d
 
diff --git a/install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf b/install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf
index 17817de..bc2e8d1 100644
--- a/install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf
+++ b/install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf
@@ -10,7 +10,7 @@
   
   
 
-  
-- 
2.4.3

From 5710e614808df26f34baec476434af6afb3a8ddb Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 9 Dec 2015 08:18:21 +0100
Subject: [PATCH 2/3] replica install: add remote connection check over API

Add server_conncheck command which calls ipa-replica-conncheck --replica
over oddjob.

https://fedorahosted.org/freeipa/ticket/5497
---
 API.txt|   8 ++
 VERSION|   4 +-
 freeipa.spec.in|   9 +-
 install/oddjob/Makefile.am |   3 +
 .../etc/dbus-1/system.d/org.freeipa.server.conf|  21 
 install/oddjob/etc/oddjobd.conf.d/ipa-server.conf  |  20 
 install/oddjob/org.freeipa.server.conncheck|   2 +
 install/tools/ipa-ca-install   |   6 +
 install/tools/ipa-replica-conncheck| 131 ++---
 install/updates/90-post_upgrade_plugins.update |   1 -
 ipalib/errors.py   |   1 +
 ipalib/messages.py |  10 ++
 ipalib/plugins/server.py   |  70 ++-
 ipaserver/install/adtrustinstance.py   |  19 ---
 ipaserver/install/ca.py|   2 +-
 ipaserver/install/httpinstance.py  |  26 
 ipaserver/install/installutils.py  |  12 --
 ipaserver/install/plugins/adtrust.py   |  21 
 ipaserver/install/replication.py   |   6 +-
 ipaserver/install/server/replicainstall.py |   5 +-
 ipaserver/install/server/upgrade.py|   1 +
 21 files changed, 300 insertions(+), 78 deletions(-)
 create mode 100644 install/oddjob/etc/dbus-1/system.d/org.freeipa.server.conf
 create mode 100644 install/oddjob/etc/oddjobd.conf.d/ipa-server.conf
 create mode 100755 install/oddjob/org.freeipa.server.conncheck

diff --git a/API.txt b/API.txt
index 60c98c3..15be32c 100644
--- a/API.txt
+++ b/API.txt
@@ -3812,6 +3812,14 @@ option: Str('version?', exclude='webui')
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (, ), None)
 output: PrimaryKey('value', None, None)
+command: server_conncheck
+args: 2,1,3
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, query=True, required=True)
+arg: Str('remote_cn', cli_name='remote_name')
+option: Str('version?', exclude='webui')
+output: Output('result', , None)
+output: Output('summary', (, ), None)
+output: PrimaryKey('value', None, None)
 command: server_del
 args: 1,2,3
 arg: Str('cn', attribute=True, cli_name='name', multivalue=True, primary_key=True, query=True, required=True)
diff --git a/VERSION b/VERSION

Re: [Freeipa-devel] [TESTS][PATCH 0006] Add comments to stageuser plugin tests

[Martin Basti]

On 09.12.2015 09:41, Lenka Doudova wrote:

Hi,

attaching fixed patches for master and ipa-4-2 branch.
Also fixes test accordingly to 
https://fedorahosted.org/freeipa/ticket/5387.


Lenka

On 11/20/2015 12:13 PM, Martin Babinsky wrote:

On 11/19/2015 10:34 AM, Petr Viktorin wrote:

On 11/19/2015 09:30 AM, Lenka Doudova wrote:

On 11/18/2015 04:51 PM, Martin Babinsky wrote:

On 11/18/2015 02:16 PM, Lenka Doudova wrote:

Hi,

here's a patch that adds a few comments to stageuser tests in 
order to

allow easier determining of a problem when tests fail.

Lenka




Hi Lenka,

Firstly a technical detail: Python indexes lists from 0, so the
comments in 'options_ok' do not correctly map to the test names 
anyway.


I am also not sure if this patch is worth reviewing and pushing as it
IMHO doesn't help in the identification of failed tests at all.

This should be solved at more fundamental level.


Ouch, good point, I didn't realize. Sorry.

Anyway, the issue is that even if tests are run in verbose mode, 
you get

output like this:

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser27] 


PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser28] 


PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser29] 


PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser210] 


PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser211] 


PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser212] 


PASSED


If some test fails, you can't really tell which command was the one
responsible for the fail. This should be solved by
https://fedorahosted.org/freeipa/ticket/5449. Until that's done, 
though,
the only way to find out which command failed is looking at the 
code and

finding which parameters were put into the command, which was not much
possible without better commenting the test case (as I realized last
week when David Kupka asked me to help him find the reason for failed
tests).
Obviously I can rewrite the tests so there's 27 separate test 
cases, one

for each parameter, instead of one method that 'unwraps' into 27 test
cases, which would entirely eliminate the confusion. So far I don't 
know

of a way to put 27 similar test cases in one method which would allow
easy recognition of the test cases.
I'll wait with fixing the patch until further discussion.


Hello,
Pytest wants you to be a bit more explicit about how to name the
parameters. (It "hides" dicts by default, because large dicts would 
make

the output even more confusing than the numbers.)

Please try the attached patch.
Docs are at 
https://pytest.org/latest/fixture.html#parametrizing-a-fixture




This looks like a better approach IMHO, you can then see which 
attribute/value was being checked.


I would very much favor more descriptive test/fixture names in the 
beginning.







Hello,

we use usually bottom posting on freeipa-devel please try to keep reply 
in this way.


Patch:

I do not like the idea of separated lists, IMO it is hard to manage and 
is easy to create mistakes.


How about this (untested, just from top of my head): 
http://fpaste.org/298994/65184014/


Martin
-- 
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 0065] ipa-replica-install prints incorrect error message when replica is already installed

[Martin Basti]

NACK

Patch contains syntax error, missing brace

ipaserver/install/server/replicainstall.py:850: [E0001(syntax-error), ] 
invalid syntax)


Martin

On 09.12.2015 07:08, Jan Cholasta wrote:

LGTM

On 8.12.2015 17:04, Gabe Alford wrote:

Updated patch attached.

On Tue, Dec 8, 2015 at 8:27 AM, Martin Basti > wrote:



On 08.12.2015 16:26, Gabe Alford wrote:

Just to confirm:

if server is installed:
 Let's stop here and not do anything else

if domain level 0:
 check if client installed and stop here

Right?

yes





On Tue, Dec 8, 2015 at 8:20 AM, Jan Cholasta > wrote:

On 8.12.2015 16:17, Martin Basti wrote:



On 08.12.2015 16:14, Jan Cholasta wrote:

On 8.12.2015 16:09, Martin Basti wrote:



On 01.12.2015 14:57, Gabe Alford wrote:

Sorry guys, I forgot to add a meaningful
subject to this message.
Ignore the previous thread start.

-- Forwarded message --
From: *Gabe Alford* 
>>
Date: Mon, Nov 30, 2015 at 7:31 PM
Subject: [PATCH 0065]
To: freeipa-devel 
>>


Hello,

Patch fix for the following tickets:

https://fedorahosted.org/freeipa/ticket/5022
https://fedorahosted.org/freeipa/ticket/5320

Thanks,

Gabe



ACK


NACK, you can't install a server over an already
installed client,
thus the original check is correct.

Ahh domain level 0, right, but this check can be added
before the client
check.


Yes.

With domain level 1, this check should stay there IMO.


Yes. It should say "IPA server is already configured" rather
than "IPA replica is already configured", though.

--
Jan Cholasta










--
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] [PATCHES 523-525] replica install: add remote connection check over API

[Jan Cholasta]

On 9.12.2015 10:34, Alexander Bokovoy wrote:

On Wed, 09 Dec 2015, Jan Cholasta wrote:

From 142fd5364cf9d31d7e2c35959516ac8d9054c9da Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 9 Dec 2015 08:17:07 +0100
Subject: [PATCH 1/3] build: put oddjob scripts into separate directory

https://fedorahosted.org/freeipa/ticket/5497
---
freeipa.spec.in  | 3 ++-
install/oddjob/Makefile.am   | 2 +-
install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf | 2 +-
3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index a60d9b6..95948e7 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -740,6 +740,7 @@ fi
%{_libexecdir}/ipa/ipa-dnskeysync-replica
%{_libexecdir}/ipa/ipa-ods-exporter
%{_libexecdir}/ipa/ipa-httpd-kdcproxy
+%dir %{_libexecdir}/ipa/oddjob
%ghost %verify(not owner group) %dir %{_sharedstatedir}/kdcproxy
%dir %attr(0755,root,root) %{_sysconfdir}/ipa/kdcproxy
%config(noreplace) %{_sysconfdir}/sysconfig/ipa_memcached
@@ -914,7 +915,7 @@ fi
%ghost %{_libdir}/krb5/plugins/libkrb5/winbind_krb5_locator.so
%{_sysconfdir}/dbus-1/system.d/oddjob-ipa-trust.conf
%{_sysconfdir}/oddjobd.conf.d/oddjobd-ipa-trust.conf
-%%attr(755,root,root)
%{_libexecdir}/ipa/com.redhat.idm.trust-fetch-domains
+%attr(755,root,root)
%{_libexecdir}/ipa/oddjob/com.redhat.idm.trust-fetch-domains

As you modified oddjobd config file, you need to restart oddjobd on
upgrade to let it re-read the config.


Right, I have accidentally put that to %pre rather than %post.

--
Jan Cholasta

--
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] First part of the replica promotion tests + testplan

[Martin Basti]

On 08.12.2015 23:48, Oleg Fayans wrote:

Substituted a hardcoded suffix name with a constant DOMAIN_SUFFIX_NAME

On 12/08/2015 02:33 PM, Oleg Fayans wrote:

Hi all,


The patches are rebased against the current master.

On 12/02/2015 05:10 PM, Martin Basti wrote:


On 02.12.2015 16:18, Oleg Fayans wrote:

Hi Martin,

On 12/01/2015 04:08 PM, Martin Basti wrote:


On 27.11.2015 16:26, Oleg Fayans wrote:

And patch N 16 passes lint too:

On 11/27/2015 04:03 PM, Oleg Fayans wrote:

Hi,

On 11/27/2015 03:26 PM, Martin Basti wrote:


On 27.11.2015 15:04, Oleg Fayans wrote:

Hi Martin,

All your suggestions were taken into account. Both patches are
updated. Thank you for your help!

On 11/26/2015 10:50 AM, Martin Basti wrote:


On 26.11.2015 10:04, Oleg Fayans wrote:

Hi Martin,

I agree to all your points but one. please, see my comment below

On 11/25/2015 07:42 PM, Martin Basti wrote:

Hi,

0) Note
Please be aware of https://fedorahosted.org/freeipa/ticket/5469
during
KRA testing

1)
Please do not use MIN and MAX_DOMAIN_LEVEL constants, this may
change
over time, use DOMAIN_LEVEL_0 and DOMAIN_LEVEL_1 for domain
level 0
and 1

2)
Why uninstall KRA then server, is not enough just uninstall
server
which
covers KRA uninstall?

+def teardown_method(self, method):
+for host in self.replicas:
+host.run_command(self.kra_uninstall,
raiseonerr=False)
+tasks.uninstall_master(host)


3)
Can be this function more generic? It should allow specify host
where
KRA should be installed not just master

+def test_kra_install_master(self):
+self.master.run_command(self.kra_install)


4)

TestLevel0(Dummy):
Can be the test name more specific, something like
TestReplicaPromotionLevel0


5)
please remove this, the patch is on review and it will be pushed
sooner
than tests
+@pytest.mark.xfail  # Ticket N 5455

and as I mentioned in ticket #5455, I cannot reproduce it with
ipa-kra-install, so please provide steps to reproduce if you
insist
that
this still does not work as expected with KRA.

6) This is completely wrong, it removes everything that we
tried to
achieve with previous patches with domain level in CI

Actually, being able to configure domain level per class is WAY
more
convenient, than to always have to think which domain level is
appropriate for which particular test during jenkins job
configuration. In fact, I should have thought about it from the
very
beginning. For example, in test_replica_promotion.py we have on
class,
which intiates with domain level = 1, while others - with domain
level
0. With config-based approach, we would have to implement a
separate
step that raises domain level. Overall, I am against the approach,
when you have to remember to set certain domain level in config
for
any particular test. The tests themselves should be aware of the
domain level they need.

I do not say that we should not have something that overrides
settings
in from config in a particular test case, I say your patch is
doing it
wrong.

I agree it is useful to have param domain_level in install_master,
and
intall_topo methods,  but is cannot be MAX_DOMAIN_LEVEL by default,
because with your current patch the domain_level in config is not
used
at all, it will be always MAX_DOMAIN_LEVEL

For example I want to achieve this goal:
test_vault.py, this test suite can run on domain level1 and on
domain
level0, so with one test we can test 2 domain levels just with
putting
domain level into config file.

I agree that with extraordinary test like replica promotion test
is, we
need something that allows override the config file.

As I said bellow, domain_level default value should be None in
install_master and install_topo plugin. If domain level was
specified
use the specified one, if not (value is None) use the domain level
from
config file.

Agreed :)


Martin

[PATCH] Enabled setting domain_level per class derived from
TestIntegration

When I configure domain level 0 in yaml config, how is this
supposed to
get into install methods when you removed that code?

-"--domain-level=%i" % host.config.domain_level
+"--domain-level=%i" % domain_level


You always use MAX_DOMAIN_LEVEL in this case or whatever is
specified in
domain_level option.
I suggest to use domain_level=None, and when it is None use
'host.config.domain_level', if it is not None, use 'domain_level'

With this we can specify domain level in config file for test
that
can
be used for both domain levels and you can manually specify
domain
level
for test that requires specific domain level.

Also this should go away

   @classmethod
   def install(cls, mh):
+if hasattr(cls, "domain_level") and cls.master:
+cls.master.config.domain_level = cls.domain_level
   if cls.topology is None:
   return

I do not see reason why test should override configuration in
config in
this case.

Martin

On 25.11.2015 16:44, Oleg Fayans wrote:

Hi,

Here is the updated version of the 

Re: [Freeipa-devel] [TESTS][PATCH 0006] Add comments to stageuser plugin tests

[Lenka Doudova]

On 12/09/2015 10:13 AM, Martin Basti wrote:



On 09.12.2015 09:41, Lenka Doudova wrote:

Hi,

attaching fixed patches for master and ipa-4-2 branch.
Also fixes test accordingly to 
https://fedorahosted.org/freeipa/ticket/5387.


Lenka

On 11/20/2015 12:13 PM, Martin Babinsky wrote:

On 11/19/2015 10:34 AM, Petr Viktorin wrote:

On 11/19/2015 09:30 AM, Lenka Doudova wrote:

On 11/18/2015 04:51 PM, Martin Babinsky wrote:

On 11/18/2015 02:16 PM, Lenka Doudova wrote:

Hi,

here's a patch that adds a few comments to stageuser tests in 
order to

allow easier determining of a problem when tests fail.

Lenka




Hi Lenka,

Firstly a technical detail: Python indexes lists from 0, so the
comments in 'options_ok' do not correctly map to the test names 
anyway.


I am also not sure if this patch is worth reviewing and pushing 
as it

IMHO doesn't help in the identification of failed tests at all.

This should be solved at more fundamental level.


Ouch, good point, I didn't realize. Sorry.

Anyway, the issue is that even if tests are run in verbose mode, 
you get

output like this:

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser27] 


PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser28] 


PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser29] 


PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser210] 


PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser211] 


PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser212] 


PASSED


If some test fails, you can't really tell which command was the one
responsible for the fail. This should be solved by
https://fedorahosted.org/freeipa/ticket/5449. Until that's done, 
though,
the only way to find out which command failed is looking at the 
code and
finding which parameters were put into the command, which was not 
much

possible without better commenting the test case (as I realized last
week when David Kupka asked me to help him find the reason for failed
tests).
Obviously I can rewrite the tests so there's 27 separate test 
cases, one

for each parameter, instead of one method that 'unwraps' into 27 test
cases, which would entirely eliminate the confusion. So far I 
don't know

of a way to put 27 similar test cases in one method which would allow
easy recognition of the test cases.
I'll wait with fixing the patch until further discussion.


Hello,
Pytest wants you to be a bit more explicit about how to name the
parameters. (It "hides" dicts by default, because large dicts would 
make

the output even more confusing than the numbers.)

Please try the attached patch.
Docs are at 
https://pytest.org/latest/fixture.html#parametrizing-a-fixture




This looks like a better approach IMHO, you can then see which 
attribute/value was being checked.


I would very much favor more descriptive test/fixture names in the 
beginning.







Hello,

we use usually bottom posting on freeipa-devel please try to keep 
reply in this way.


Patch:

I do not like the idea of separated lists, IMO it is hard to manage 
and is easy to create mistakes.


How about this (untested, just from top of my head): 
http://fpaste.org/298994/65184014/


Martin


Great idea, thanks. Fixed patches attached.

Lenka
From 646db80b7cc0912c310615d804fcb1561e19961f Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Mon, 23 Nov 2015 10:27:07 +0100
Subject: [PATCH] Adding descriptive IDs to stageuser tests

Adding descriptive IDs to parametrized stageuser test for better identification of test cases.
---
 ipatests/test_xmlrpc/test_stageuser_plugin.py| 84 ++--
 ipatests/test_xmlrpc/tracker/stageuser_plugin.py |  2 +-
 ipatests/test_xmlrpc/tracker/user_plugin.py  |  9 ++-
 3 files changed, 56 insertions(+), 39 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py
index 4eb968451f926ca0ee8fa5aeae1a96770f56eb45..42ecf046884369bd74b03194937c425215b99f6c 100644
--- a/ipatests/test_xmlrpc/test_stageuser_plugin.py
+++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py
@@ -15,6 +15,7 @@ import pytest
 
 import six
 
+from collections import OrderedDict
 from ipalib import api, errors
 
 from ipatests.test_xmlrpc import objectclasses
@@ -53,35 +54,40 @@ sshpubkey = (u'ssh-rsa B3NzaC1yc2EDAQABAAABAQDGAX3xAeLeaJggwTqMjxNwa6X'
 sshpubkeyfp = (u'13:67:6B:BF:4E:A2:05:8E:AE:25:8B:A1:31:DE:6F:1B '
'public key test (ssh-rsa)')
 
-options_ok = [
-{u'cn': u'name'},
-{u'initials': u'in'},
-{u'displayname': u'display'},
-{u'homedirectory': u'/home/homedir'},
-{u'gecos': u'gecos'},
-{u'loginshell': u'/bin/shell'},
-{u'mail': u'email@email.email'},
-{u'title': u'newbie'},
-

Re: [Freeipa-devel] [PATCH 0394] topology: Make sure the old 'realm' topology suffix is not

[Martin Basti]

On 08.12.2015 17:32, Martin Babinsky wrote:

On 12/08/2015 04:53 PM, Tomas Babej wrote:



On 12/08/2015 02:28 PM, Tomas Babej wrote:

Hi,

The old 'realm' topology suffix is no longer used, however, it was 
being
created on masters with version 4.2.3 and later. Make sure it's 
properly

removed.

Note that this is not the case for the 'ipaca' suffix, which was later
removed to 'ca'.

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



Actually, we found out with Martin that this patch deletes realm instead
of domain suffix, against all initial impressions.

Updated patch attached.

Tomas





Works for me, ACK.

I have also made some hardening in topology connectivity checks so 
that this kind of situations is handled and reported by them. I will 
send a patch in separate thread.



Pushed to master: a84b7d2117aafc5182640d0a22675b214c27dd7c

--
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] [PATCHES 523-525] replica install: add remote connection check over API

[Alexander Bokovoy]

On Wed, 09 Dec 2015, Jan Cholasta wrote:

From 142fd5364cf9d31d7e2c35959516ac8d9054c9da Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 9 Dec 2015 08:17:07 +0100
Subject: [PATCH 1/3] build: put oddjob scripts into separate directory

https://fedorahosted.org/freeipa/ticket/5497
---
freeipa.spec.in  | 3 ++-
install/oddjob/Makefile.am   | 2 +-
install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf | 2 +-
3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index a60d9b6..95948e7 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -740,6 +740,7 @@ fi
%{_libexecdir}/ipa/ipa-dnskeysync-replica
%{_libexecdir}/ipa/ipa-ods-exporter
%{_libexecdir}/ipa/ipa-httpd-kdcproxy
+%dir %{_libexecdir}/ipa/oddjob
%ghost %verify(not owner group) %dir %{_sharedstatedir}/kdcproxy
%dir %attr(0755,root,root) %{_sysconfdir}/ipa/kdcproxy
%config(noreplace) %{_sysconfdir}/sysconfig/ipa_memcached
@@ -914,7 +915,7 @@ fi
%ghost %{_libdir}/krb5/plugins/libkrb5/winbind_krb5_locator.so
%{_sysconfdir}/dbus-1/system.d/oddjob-ipa-trust.conf
%{_sysconfdir}/oddjobd.conf.d/oddjobd-ipa-trust.conf
-%%attr(755,root,root) %{_libexecdir}/ipa/com.redhat.idm.trust-fetch-domains
+%attr(755,root,root) 
%{_libexecdir}/ipa/oddjob/com.redhat.idm.trust-fetch-domains

As you modified oddjobd config file, you need to restart oddjobd on
upgrade to let it re-read the config.



%endif # ONLY_CLIENT

diff --git a/install/oddjob/Makefile.am b/install/oddjob/Makefile.am
index 9dde10c..5cdaf2b 100644
--- a/install/oddjob/Makefile.am
+++ b/install/oddjob/Makefile.am
@@ -1,6 +1,6 @@
NULL =

-oddjobdir = $(libexecdir)/ipa
+oddjobdir = $(libexecdir)/ipa/oddjob
oddjobconfdir = $(sysconfdir)/oddjobd.conf.d
dbusconfdir = $(sysconfdir)/dbus-1/system.d

diff --git a/install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf 
b/install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf
index 17817de..bc2e8d1 100644
--- a/install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf
+++ b/install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf
@@ -10,7 +10,7 @@
  
  

-  
--
2.4.3




--
/ Alexander Bokovoy

--
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 522] replica promotion: allow OTP bulk client enrollment

[Martin Basti]

On 08.12.2015 13:19, Martin Basti wrote:



On 08.12.2015 13:09, Jan Cholasta wrote:

On 8.12.2015 12:49, Martin Basti wrote:



On 08.12.2015 10:31, Martin Basti wrote:



On 08.12.2015 08:52, Jan Cholasta wrote:

On 7.12.2015 21:11, Martin Basti wrote:



On 07.12.2015 08:21, Jan Cholasta wrote:

On 2.12.2015 16:23, Jan Cholasta wrote:

Hi,

the attached patch fixes
.

Note that you still have to provide admin password in
ipa-replica-install, either using --admin-password or 
interactively,

because:

a) Admin password is required for replica promotion. This will be
fixed
with .

Patches are on the list:
. 






Pushed.




b) Admin password is required for connection check. This will be
fixed
with .


Martin Basti pointed out that admin password should not be asked
interactively during OTP replica promotion. Fixed.

Updated and rebased patch attached.





1)
[root@vm-058-138 ~]# ipa-replica-install --server
vm-058-137.abc.idm.lab.eng.brq.redhat.com --domain
abc.idm.lab.eng.brq.redhat.com --password=bubak --setup-ca
Configuring client side components
Password for ad...@abc.idm.lab.eng.brq.redhat.com:

IMO password should be asked first, before any installation 
begins (IMO

this is for conncheck)


The same thing happens without my patch. Could you file a ticket?

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





2)
When host is not in ipaservers hostgroup. Also I would expect 
different

error message
ipa-replica-install --server 
vm-058-137.abc.idm.lab.eng.brq.redhat.com

--domain abc.idm.lab.eng.brq.redhat.com --password=bubak --setup-ca
--skip-conncheck


 step()
   File 
"/usr/lib/python2.7/site-packages/ipapython/install/core.py",

line 352, in 
 step = lambda: next(self.__gen)
   File 
"/usr/lib/python2.7/site-packages/ipapython/install/util.py",

line 81, in run_generator_with_yield_from
 six.reraise(*exc_info)
   File 
"/usr/lib/python2.7/site-packages/ipapython/install/util.py",

line 59, in run_generator_with_yield_from
 value = gen.send(prev_value)
   File 
"/usr/lib/python2.7/site-packages/ipapython/install/common.py",

line 63, in _install
 for nothing in self._installer(self.parent):
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/replicainstall.py", 



line 1507, in main
 promote_check(self)
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/replicainstall.py", 



line 374, in decorated
 func(installer)
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/replicainstall.py", 



line 1002, in promote_check
 conn.connect(ccache=installer._ccache)
   File "/usr/lib/python2.7/site-packages/ipalib/backend.py", 
line 66,

in connect
 conn = self.create_connection(*args, **kw)
   File 
"/usr/lib/python2.7/site-packages/ipaserver/plugins/ldap2.py",

line 199, in create_connection
 principal = krb_utils.get_principal(ccache_name=ccache)
   File "/usr/lib/python2.7/site-packages/ipalib/krb_utils.py", line
184, in get_principal
 raise errors.CCacheError(message=unicode(e))

2015-12-07T16:23:40Z DEBUG The ipa-replica-install command failed,
exception: CCacheError: Major (851968): Unspecified GSS failure. 
Minor

code may provide more information, Minor (2529639053): No Kerberos
credentials available
2015-12-07T16:23:40Z ERROR Major (851968): Unspecified GSS failure.
Minor code may provide more information, Minor (2529639053): No
Kerberos
credentials available


Fixed.




3)
This case is not handle very well:
a) install client with OTP password
b) install replica with the same OTP password (when host is no in
ipaservers group, if host is in ipaservers group it works)

ipa.ipapython.install.cli.install_tool(Replica): ERROR Major
(851968): Unspecified GSS failure.  Minor code may provide more
information, Minor (2529639053): No Kerberos credentials available
ipa.ipapython.install.cli.install_tool(Replica): ERROR The
ipa-replica-install command failed. See 
/var/log/ipareplica-install.log

for more information


This is the same as 2).



4)
This is not user friendly
I used wrong OTP password, can we somehow propagate the actual error
from client install to stderr?

ipa.ipapython.install.cli.install_tool(Replica): ERROR 
Configuration of

client side components failed!
ipa-client-install returned: Command ''/usr/sbin/ipa-client-install'
'--unattended' '--domain' 'abc.idm.lab.eng.brq.redhat.com' 
'--server'

'vm-058-137.abc.idm.lab.eng.brq.redhat.com' '--password' 'buba''
returned non-zero exit status 1
ipa.ipapython.install.cli.install_tool(Replica): ERROR The
ipa-replica-install command failed. See 
/var/log/ipareplica-install.log

for more information


The same thing happens without my patch for any other error. Could
you file a ticket?


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

Re: [Freeipa-devel] [PATCH] First part of the replica promotion tests + testplan

[Oleg Fayans]

Hi Martin

On 12/09/2015 10:30 AM, Martin Basti wrote:
> 
> 
> On 08.12.2015 23:48, Oleg Fayans wrote:
>> Substituted a hardcoded suffix name with a constant DOMAIN_SUFFIX_NAME
>>
>> On 12/08/2015 02:33 PM, Oleg Fayans wrote:
>>> Hi all,
>>>
>>>
>>> The patches are rebased against the current master.
>>>
>>> On 12/02/2015 05:10 PM, Martin Basti wrote:

 On 02.12.2015 16:18, Oleg Fayans wrote:
> Hi Martin,
>
> On 12/01/2015 04:08 PM, Martin Basti wrote:
>>
>> On 27.11.2015 16:26, Oleg Fayans wrote:
>>> And patch N 16 passes lint too:
>>>
>>> On 11/27/2015 04:03 PM, Oleg Fayans wrote:
 Hi,

 On 11/27/2015 03:26 PM, Martin Basti wrote:
>
> On 27.11.2015 15:04, Oleg Fayans wrote:
>> Hi Martin,
>>
>> All your suggestions were taken into account. Both patches are
>> updated. Thank you for your help!
>>
>> On 11/26/2015 10:50 AM, Martin Basti wrote:
>>>
>>> On 26.11.2015 10:04, Oleg Fayans wrote:
 Hi Martin,

 I agree to all your points but one. please, see my comment
 below

 On 11/25/2015 07:42 PM, Martin Basti wrote:
> Hi,
>
> 0) Note
> Please be aware of
> https://fedorahosted.org/freeipa/ticket/5469
> during
> KRA testing
>
> 1)
> Please do not use MIN and MAX_DOMAIN_LEVEL constants, this may
> change
> over time, use DOMAIN_LEVEL_0 and DOMAIN_LEVEL_1 for domain
> level 0
> and 1
>
> 2)
> Why uninstall KRA then server, is not enough just uninstall
> server
> which
> covers KRA uninstall?
>
> +def teardown_method(self, method):
> +for host in self.replicas:
> +host.run_command(self.kra_uninstall,
> raiseonerr=False)
> +tasks.uninstall_master(host)
>
>
> 3)
> Can be this function more generic? It should allow specify
> host
> where
> KRA should be installed not just master
>
> +def test_kra_install_master(self):
> +self.master.run_command(self.kra_install)
>
>
> 4)
>
> TestLevel0(Dummy):
> Can be the test name more specific, something like
> TestReplicaPromotionLevel0
>
>
> 5)
> please remove this, the patch is on review and it will be
> pushed
> sooner
> than tests
> +@pytest.mark.xfail  # Ticket N 5455
>
> and as I mentioned in ticket #5455, I cannot reproduce it with
> ipa-kra-install, so please provide steps to reproduce if you
> insist
> that
> this still does not work as expected with KRA.
>
> 6) This is completely wrong, it removes everything that we
> tried to
> achieve with previous patches with domain level in CI
 Actually, being able to configure domain level per class is WAY
 more
 convenient, than to always have to think which domain level is
 appropriate for which particular test during jenkins job
 configuration. In fact, I should have thought about it from the
 very
 beginning. For example, in test_replica_promotion.py we have on
 class,
 which intiates with domain level = 1, while others - with
 domain
 level
 0. With config-based approach, we would have to implement a
 separate
 step that raises domain level. Overall, I am against the
 approach,
 when you have to remember to set certain domain level in config
 for
 any particular test. The tests themselves should be aware of
 the
 domain level they need.
>>> I do not say that we should not have something that overrides
>>> settings
>>> in from config in a particular test case, I say your patch is
>>> doing it
>>> wrong.
>>>
>>> I agree it is useful to have param domain_level in
>>> install_master,
>>> and
>>> intall_topo methods,  but is cannot be MAX_DOMAIN_LEVEL by
>>> default,
>>> because with your current patch the domain_level in config is
>>> not
>>> used
>>> at all, it will be always MAX_DOMAIN_LEVEL
>>>
>>> For example I want to achieve this goal:
>>> test_vault.py, this test 

Re: [Freeipa-devel] [PATCH 0112] CI tests: ignore disconnected domain level 1 topology on IPA master teardown

[Martin Basti]

On 09.12.2015 08:26, Oleg Fayans wrote:

ACK

On 12/09/2015 07:37 AM, Martin Babinsky wrote:

On 12/07/2015 01:53 PM, Martin Babinsky wrote:

On 12/07/2015 12:07 PM, Oleg Fayans wrote:

Hi Martin,

CONFIGURED_DOMAIN_LEVEL is declared, but not used. The rest looks fine
to me

On 12/07/2015 11:05 AM, Martin Babinsky wrote:

This patch should fix teardown methods in replication-related CI tests
ran at non-zero domain level.




Ah that was a leftover from previous implementation. Here's updated
patch.




Patch needed a rebase. Attaching new revision.




Pushed to master: 35fae355cc4441164f149288b3d126129a82aed7

--
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] [TESTS][PATCH 0006] Add comments to stageuser plugin tests

[Lenka Doudova]

Hi,

attaching fixed patches for master and ipa-4-2 branch.
Also fixes test accordingly to https://fedorahosted.org/freeipa/ticket/5387.

Lenka

On 11/20/2015 12:13 PM, Martin Babinsky wrote:

On 11/19/2015 10:34 AM, Petr Viktorin wrote:

On 11/19/2015 09:30 AM, Lenka Doudova wrote:

On 11/18/2015 04:51 PM, Martin Babinsky wrote:

On 11/18/2015 02:16 PM, Lenka Doudova wrote:

Hi,

here's a patch that adds a few comments to stageuser tests in 
order to

allow easier determining of a problem when tests fail.

Lenka




Hi Lenka,

Firstly a technical detail: Python indexes lists from 0, so the
comments in 'options_ok' do not correctly map to the test names 
anyway.


I am also not sure if this patch is worth reviewing and pushing as it
IMHO doesn't help in the identification of failed tests at all.

This should be solved at more fundamental level.


Ouch, good point, I didn't realize. Sorry.

Anyway, the issue is that even if tests are run in verbose mode, you 
get

output like this:

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser27] 


PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser28] 


PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser29] 


PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser210] 


PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser211] 


PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser212] 


PASSED


If some test fails, you can't really tell which command was the one
responsible for the fail. This should be solved by
https://fedorahosted.org/freeipa/ticket/5449. Until that's done, 
though,
the only way to find out which command failed is looking at the code 
and

finding which parameters were put into the command, which was not much
possible without better commenting the test case (as I realized last
week when David Kupka asked me to help him find the reason for failed
tests).
Obviously I can rewrite the tests so there's 27 separate test cases, 
one

for each parameter, instead of one method that 'unwraps' into 27 test
cases, which would entirely eliminate the confusion. So far I don't 
know

of a way to put 27 similar test cases in one method which would allow
easy recognition of the test cases.
I'll wait with fixing the patch until further discussion.


Hello,
Pytest wants you to be a bit more explicit about how to name the
parameters. (It "hides" dicts by default, because large dicts would make
the output even more confusing than the numbers.)

Please try the attached patch.
Docs are at 
https://pytest.org/latest/fixture.html#parametrizing-a-fixture




This looks like a better approach IMHO, you can then see which 
attribute/value was being checked.


I would very much favor more descriptive test/fixture names in the 
beginning.




From 163bbd04cf02d278b3d4ef8d0ee91878270e4377 Mon Sep 17 00:00:00 2001
From: Lenka Doudova 
Date: Mon, 23 Nov 2015 10:27:07 +0100
Subject: [PATCH] Adding descriptive IDs to stageuser tests

Adding descriptive IDs to parametrized stageuser test for better identification of test cases.
---
 ipatests/test_xmlrpc/test_stageuser_plugin.py| 30 ++--
 ipatests/test_xmlrpc/tracker/stageuser_plugin.py |  2 +-
 ipatests/test_xmlrpc/tracker/user_plugin.py  |  9 +--
 3 files changed, 31 insertions(+), 10 deletions(-)

diff --git a/ipatests/test_xmlrpc/test_stageuser_plugin.py b/ipatests/test_xmlrpc/test_stageuser_plugin.py
index 4eb968451f926ca0ee8fa5aeae1a96770f56eb45..bda738b786f99296deaeb192e84965bb3e97cfea 100644
--- a/ipatests/test_xmlrpc/test_stageuser_plugin.py
+++ b/ipatests/test_xmlrpc/test_stageuser_plugin.py
@@ -83,6 +83,16 @@ options_ok = [
 {u'random': True},
 ]
 
+options_ids = [
+'full name', 'initials', 'display name', 'home directory', 'GECOS',
+'shell', 'email address', 'job title', 'kerberos principal',
+'uppercase kerberos principal', 'street address', 'city', 'state',
+'zip code', 'telephone number', 'fax number', 'mobile tel. number',
+'pager number', 'organizational unit', 'car license', 'SSH key',
+'manager', 'user ID number', 'group ID number', 'UID and GID numbers',
+'password', 'random password'
+]
+
 
 @pytest.fixture(scope='class')
 def stageduser(request):
@@ -90,13 +100,19 @@ def stageduser(request):
 return tracker.make_fixture(request)
 
 
-@pytest.fixture(scope='class', params=options_ok)
+@pytest.fixture(scope='class', params=options_ok, ids=options_ids)
 def stageduser2(request):
 tracker = StageUserTracker(u'suser2', u'staged', u'user', **request.param)
 return tracker.make_fixture_activate(request)
 
 
 @pytest.fixture(scope='class')
+def user_activated(request):
+tracker = UserTracker(u'suser2', u'staged', u'user')
+return 

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

[Petr Viktorin]

On 12/04/2015 12:49 PM, Jan Cholasta wrote:
> On 4.12.2015 12:43, Petr Viktorin wrote:
>> On 12/04/2015 12:15 PM, Jan Cholasta wrote:
>>> On 4.12.2015 10:53, Petr Viktorin wrote:
 On 12/04/2015 08:51 AM, Jan Cholasta wrote:
> On 3.12.2015 15:42, Petr Viktorin wrote:
>> On 12/03/2015 10:55 AM, Jan Cholasta wrote:
>> [...]
>>> If we do encode/decode in run(), I think there should be a way to
>>> override the default encoding. My idea was to either accept/return
>>> only
>>> bytes and let the caller handle encoding themselves, or to handle
>>> encoding in run(), but be able to override the defaults.
>>>
>>> Given we handle encoding in run(), I would imagine it like this:
>>>
>>>def run(args, stdin=None, raiseonerr=True, nolog=(),
>>> env=None,
>>>capture_stdout=False, skip_output=False, cwd=None,
>>>runas=None, timeout=None, suplementary_groups=[],
>>>capture_stderr=False, encode_stdout=False,
>>>encode_stderr=False, encoding=None):
>>>
>>> i.e. nothing is captured by default, when stdout or stderr are
>>> captured
>>> they are returned as bytes by default, when stdout or stderr are
>>> returned as text they are decoded using the locale encoding by
>>> default,
>>> or the encoding can be explicitly specified.
>>>
>>> Do you think this is feasible?
>>
>> Feasible, yes.
>> In the majority of cases where the output is needed, we want text
>> encoded with locale.getpreferredencoding(), so I'd rather make
>> that the
>> default rather than bytes.
>>
>> Or we could make "encode_stdout" imply "capture_stdout", so you
>> wouldn't
>> have to always specify both. (It would be better named
>> "encoded_stdout"
>> in that case.)
>
> I think it should be OK to decode by default. (IMO it would be best to
> reverse the logic and name it "raw_stdout", with False default. Do we
> need "raw_stderr" at all? I don't think we do.)

 Actually, now that I think about it: if the result was a namedtuple
 subclass, we can always set extra raw_stdout/raw_stderr attributes on
 it. (Unless skip_output=True, of course.)

  result = run(['generate_binary_data'])
  use(result.raw_stdout)

  # and for backcompat,
  rc, _out, _err = result
>>>
>>> Good idea.
>>>
>>> Perhaps we should use the same names as CalledProcessError for the
>>> attributes, i.e. "returncode", "output", and "error_output",
>>> "raw_output", "raw_error_output" for the new attributes?
> 
> (Actually, since "returncode" is not "return_code", it should probably
> be "returncode", "output", "erroroutput", "raw_output", "raw_erroroutput".)
> 
>>
>> OK. It's also good to use different names than Popen's stdout/stderr,
>> which are file-like objects rather than strings. But then,
>> capture_stdout/capture_stderr should be
>> capture_output/capture_error_output.

Here is the new patch.
I also added output_log and error_log to the result to abstract the
following common pattern:

result = run([...])
if result.returncode != 0:
stderr = result.error_output
if six.PY3:
error = stderr.encode(locale.getpreferredencoding(), 'replace')
self.log.critical('...: %s', stderr)


-- 
Petr Viktorin

From 5351a375670be5c1cad756de21428a13501077ab Mon Sep 17 00:00:00 2001
From: Petr Viktorin 
Date: Wed, 25 Nov 2015 17:17:18 +0100
Subject: [PATCH] Refactor ipautil.run

The ipautil.run function now returns an object with returncode and
output are accessible as attributes.

The stdout and stderr of all commands are logged (unless skip_output is given).

The stdout/stderr contents must be explicitly requested with a keyword
argument, otherwise they are None.
This is because in Python 3, the output needs to be decoded, and that can
fail if it's not decodable (human-readable) text.

The raw (bytes) output is always available from the result object,
as is "leniently" decoded output suitable for logging.

All calls are changed to reflect this.

A use of Popen in cainstance is changed to ipautil.run.
---
 .../certmonger/dogtag-ipa-ca-renew-agent-submit|  14 ++-
 install/certmonger/ipa-server-guard|  16 ++-
 install/oddjob/com.redhat.idm.trust-fetch-domains  |   5 +-
 install/tools/ipa-replica-conncheck|  37 +++---
 ipa-client/ipa-install/ipa-client-install  |  43 ---
 ipalib/plugins/pwpolicy.py |   6 +-
 ipaplatform/base/services.py   |  41 +++---
 ipaplatform/redhat/services.py |   4 +-
 ipaplatform/redhat/tasks.py|   7 +-
 ipapython/certdb.py|  27 ++--
 ipapython/dnssec/bindmgr.py|   7 +-
 ipapython/dnssec/odsmgr.py |   3 +-
 

Re: [Freeipa-devel] [TESTS][PATCH 0006] Add comments to stageuser plugin tests

[Martin Babinsky]

On 12/09/2015 11:29 AM, Lenka Doudova wrote:



On 12/09/2015 10:13 AM, Martin Basti wrote:



On 09.12.2015 09:41, Lenka Doudova wrote:

Hi,

attaching fixed patches for master and ipa-4-2 branch.
Also fixes test accordingly to
https://fedorahosted.org/freeipa/ticket/5387.

Lenka

On 11/20/2015 12:13 PM, Martin Babinsky wrote:

On 11/19/2015 10:34 AM, Petr Viktorin wrote:

On 11/19/2015 09:30 AM, Lenka Doudova wrote:

On 11/18/2015 04:51 PM, Martin Babinsky wrote:

On 11/18/2015 02:16 PM, Lenka Doudova wrote:

Hi,

here's a patch that adds a few comments to stageuser tests in
order to
allow easier determining of a problem when tests fail.

Lenka




Hi Lenka,

Firstly a technical detail: Python indexes lists from 0, so the
comments in 'options_ok' do not correctly map to the test names
anyway.

I am also not sure if this patch is worth reviewing and pushing
as it
IMHO doesn't help in the identification of failed tests at all.

This should be solved at more fundamental level.


Ouch, good point, I didn't realize. Sorry.

Anyway, the issue is that even if tests are run in verbose mode,
you get
output like this:

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser27]

PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser28]

PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser29]

PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser210]

PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser211]

PASSED

ipatests/test_xmlrpc/test_stageuser_plugin.py::TestStagedUser::test_create_attr[stageduser212]

PASSED


If some test fails, you can't really tell which command was the one
responsible for the fail. This should be solved by
https://fedorahosted.org/freeipa/ticket/5449. Until that's done,
though,
the only way to find out which command failed is looking at the
code and
finding which parameters were put into the command, which was not
much
possible without better commenting the test case (as I realized last
week when David Kupka asked me to help him find the reason for failed
tests).
Obviously I can rewrite the tests so there's 27 separate test
cases, one
for each parameter, instead of one method that 'unwraps' into 27 test
cases, which would entirely eliminate the confusion. So far I
don't know
of a way to put 27 similar test cases in one method which would allow
easy recognition of the test cases.
I'll wait with fixing the patch until further discussion.


Hello,
Pytest wants you to be a bit more explicit about how to name the
parameters. (It "hides" dicts by default, because large dicts would
make
the output even more confusing than the numbers.)

Please try the attached patch.
Docs are at
https://pytest.org/latest/fixture.html#parametrizing-a-fixture




This looks like a better approach IMHO, you can then see which
attribute/value was being checked.

I would very much favor more descriptive test/fixture names in the
beginning.






Hello,

we use usually bottom posting on freeipa-devel please try to keep
reply in this way.

Patch:

I do not like the idea of separated lists, IMO it is hard to manage
and is easy to create mistakes.

How about this (untested, just from top of my head):
http://fpaste.org/298994/65184014/

Martin


Great idea, thanks. Fixed patches attached.

Lenka




Tests pass and code looks good, 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

Re: [Freeipa-devel] [PATCH 0098-0099] domain level 1 topology checks during IPA server uninstall

[Jan Cholasta]

On 2.12.2015 14:19, Martin Basti wrote:



On 02.12.2015 14:10, Martin Basti wrote:



On 02.12.2015 14:08, Martin Babinsky wrote:

On 12/02/2015 10:45 AM, Martin Babinsky wrote:

On 12/01/2015 02:40 PM, Martin Babinsky wrote:

On 11/30/2015 08:34 PM, Martin Basti wrote:



On 30.11.2015 18:41, Martin Babinsky wrote:

On 11/30/2015 06:15 PM, Martin Basti wrote:



On 30.11.2015 16:43, Martin Babinsky wrote:

On 11/30/2015 12:31 PM, Jan Cholasta wrote:

Hi,

On 27.11.2015 14:58, Martin Babinsky wrote:

On 11/19/2015 06:19 PM, Martin Babinsky wrote:

These two patches fix the following tickets:

https://fedorahosted.org/freeipa/ticket/5377
https://fedorahosted.org/freeipa/ticket/5409

I have added a new option '--ignore-disconnected-topology'
which
forces
IPA master uninstall despite reported errors in topology.
I'm not
quite
sure if we want to flood ipa-server-install with
uninstall-specific
options, maybe it is better to skip the check in unattended
mode
and
just print a warning about disconnected topology and what to do
about
it.

I would like to hear your opinions about this.




Attaching rebased and updated patches.


Patch 0098: LGTM


Patch 0099:

a) This check should be done in Server.__init__() rather than
install_check():

+if options.ignore_disconnected_topology:
+print("'--ignore-disconnected-topology' is used only
during "
+  "uninstallation")
+sys.exit(1)


b)
s/--ignore-disconnected-topology/--ignore-topology-disconnect/,
for
consistency with other options, e.g. --no-ui-redirect.

Maybe even shorten it to --ignore-topology? But we probably don't
want
people to use this option much, so it might be better to keep it
long?


I would rather leave it with the long option name, it is more
apparent
what this switch should be around.


c) I'm fine with uninstall options, you can remove the TODO:

+# TODO: ask jcholast about uninstallation options


Honza



Attaching updated patches.


NACK

ipa-server-install --uninstall

2015-11-30T17:14:30Z DEBUG Destroyed connection
context.ldap2_140081152041808
2015-11-30T17:14:30Z DEBUG Traceback (most recent call last):
   File
"/usr/lib/python2.7/site-packages/ipapython/install/common.py",
line 91, in _handle_exception
 super(Continuous, self)._handle_exception(exc_info)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 387, in _handle_exception
 six.reraise(*exc_info)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 439, in _handle_exception
 super(ComponentBase, self)._handle_exception(exc_info)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 387, in _handle_exception
 six.reraise(*exc_info)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 355, in __runner
 step()
   File
"/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 352, in 
 step = lambda: next(self.__gen)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/util.py",
line 81, in run_generator_with_yield_from
 six.reraise(*exc_info)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/util.py",
line 59, in run_generator_with_yield_from
 value = gen.send(prev_value)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/common.py",
line 71, in _uninstall
 for nothing in self._uninstaller(self.parent):
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",


line 1409, in main
 uninstall_check(self)
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",


line 265, in decorated
 func(installer)
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",


line 1140, in uninstall_check
 api, masters, options.ignore_disconnected_topology)
AttributeError: 'uninstaller(Server)' object has no attribute
'ignore_disconnected_topology'

2015-11-30T17:14:30Z ERROR 'uninstaller(Server)' object has no
attribute
'ignore_disconnected_topology'
2015-11-30T17:14:30Z INFO The ipa-server-install command was
successful



Sorry I have failed horribly during option rename. Attaching patch
that should actually work.


functional ACK

Attaching rebased patches reflecting the recent changes in the
handling
of managed topology suffixes handling.






Jan had some more suggestions to the patches. Attaching updated
version.




Attaching updated patch 99 with fixed error message.


Pushed to master: b8c619a7139bd7b65caa03b68431e22791ff19bf


ACK :-)


I was doing some unrelated testing with domain level 0 and forgot to 
remove --ignore-topology disconnect from my command line before 
uninstalling server, which gave me an error that the option cannot be 
used in domain level 0 and I had to re-run ipa-server-install 
--uninstall with the option removed.


IMO it would be better for UX if the option was ignored in domain level 
0, since topology disconnects are *always* ignored in domain level 0. 
(Right?)


The attached patch fixes that.

Re: [Freeipa-devel] [PATCHES 523-525] replica install: add remote connection check over API

[Jan Cholasta]

On 9.12.2015 10:02, Jan Cholasta wrote:

Hi,

the attached patches fix .


Note that this needs selinux-policy fix to work, so put SELinux into 
permissive mode for testing: 
.


--
Jan Cholasta

--
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 0065] ipa-replica-install prints incorrect error message when replica is already installed

[Gabe Alford]

Fixed. Updated patch attached.

On Wed, Dec 9, 2015 at 2:37 AM, Martin Basti  wrote:

> NACK
>
> Patch contains syntax error, missing brace
>
> ipaserver/install/server/replicainstall.py:850: [E0001(syntax-error), ]
> invalid syntax)
>
> Martin
>
>
> On 09.12.2015 07:08, Jan Cholasta wrote:
>
>> LGTM
>>
>> On 8.12.2015 17:04, Gabe Alford wrote:
>>
>>> Updated patch attached.
>>>
>>> On Tue, Dec 8, 2015 at 8:27 AM, Martin Basti >> > wrote:
>>>
>>>
>>>
>>> On 08.12.2015 16:26, Gabe Alford wrote:
>>>
 Just to confirm:

 if server is installed:
  Let's stop here and not do anything else

 if domain level 0:
  check if client installed and stop here

 Right?

>>> yes
>>>
>>>
>>>

 On Tue, Dec 8, 2015 at 8:20 AM, Jan Cholasta > wrote:

 On 8.12.2015 16:17, Martin Basti wrote:



 On 08.12.2015 16:14, Jan Cholasta wrote:

 On 8.12.2015 16:09, Martin Basti wrote:



 On 01.12.2015 14:57, Gabe Alford wrote:

 Sorry guys, I forgot to add a meaningful
 subject to this message.
 Ignore the previous thread start.

 -- Forwarded message --
 From: *Gabe Alford* 
 >>
 Date: Mon, Nov 30, 2015 at 7:31 PM
 Subject: [PATCH 0065]
 To: freeipa-devel 
 >>


 Hello,

 Patch fix for the following tickets:

 https://fedorahosted.org/freeipa/ticket/5022
 https://fedorahosted.org/freeipa/ticket/5320

 Thanks,

 Gabe



 ACK


 NACK, you can't install a server over an already
 installed client,
 thus the original check is correct.

 Ahh domain level 0, right, but this check can be added
 before the client
 check.


 Yes.

 With domain level 1, this check should stay there IMO.


 Yes. It should say "IPA server is already configured" rather
 than "IPA replica is already configured", though.

 --
 Jan Cholasta



>>>
>>>
>>
>>
>
From 41af20d4ef76186f4099858e12e6e954d282f70f Mon Sep 17 00:00:00 2001
From: Gabe 
Date: Wed, 9 Dec 2015 06:41:30 -0700
Subject: [PATCH] ipa-replica-install prints incorrect error message when
 replica is already installed

https://fedorahosted.org/freeipa/ticket/5022
https://fedorahosted.org/freeipa/ticket/5320
---
 ipaserver/install/server/replicainstall.py | 15 ---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 4554166752ce4e5db2a98a8f495aa061aec963e9..1f4b133e1a11c915b229514456c8624148a741f1 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -31,9 +31,8 @@ from ipaserver.install import (
 bindinstance, ca, cainstance, certs, dns, dsinstance, httpinstance,
 installutils, kra, krainstance, krbinstance, memcacheinstance,
 ntpinstance, otpdinstance, custodiainstance, service)
-from ipaserver.install.installutils import create_replica_config
-from ipaserver.install.installutils import ReplicaConfig
-from ipaserver.install.installutils import load_pkcs12
+from ipaserver.install.installutils import (
+create_replica_config, ReplicaConfig, load_pkcs12, is_ipa_configured)
 from ipaserver.install.replication import (
 ReplicationManager, replica_conn_check)
 import SSSDConfig
@@ -423,6 +422,11 @@ def install_check(installer):
 
 tasks.check_selinux_status()
 
+if is_ipa_configured():
+sys.exit("IPA server is already configured on this system.\n"
+ "If you want to reinstall the IPA server, please uninstall "
+ "it first using 'ipa-server-install --uninstall'.")
+
 client_fstore = sysrestore.FileStore(paths.IPA_CLIENT_SYSRESTORE)
 if client_fstore.has_files():
 sys.exit("IPA client is already configured on this system.\n"
@@ -828,6 +832,11 @@ def 

Re: [Freeipa-devel] [PATCH 0098-0099] domain level 1 topology checks during IPA server uninstall

[Martin Babinsky]

On 12/09/2015 03:48 PM, Jan Cholasta wrote:

On 2.12.2015 14:19, Martin Basti wrote:



On 02.12.2015 14:10, Martin Basti wrote:



On 02.12.2015 14:08, Martin Babinsky wrote:

On 12/02/2015 10:45 AM, Martin Babinsky wrote:

On 12/01/2015 02:40 PM, Martin Babinsky wrote:

On 11/30/2015 08:34 PM, Martin Basti wrote:



On 30.11.2015 18:41, Martin Babinsky wrote:

On 11/30/2015 06:15 PM, Martin Basti wrote:



On 30.11.2015 16:43, Martin Babinsky wrote:

On 11/30/2015 12:31 PM, Jan Cholasta wrote:

Hi,

On 27.11.2015 14:58, Martin Babinsky wrote:

On 11/19/2015 06:19 PM, Martin Babinsky wrote:

These two patches fix the following tickets:

https://fedorahosted.org/freeipa/ticket/5377
https://fedorahosted.org/freeipa/ticket/5409

I have added a new option '--ignore-disconnected-topology'
which
forces
IPA master uninstall despite reported errors in topology.
I'm not
quite
sure if we want to flood ipa-server-install with
uninstall-specific
options, maybe it is better to skip the check in unattended
mode
and
just print a warning about disconnected topology and what
to do
about
it.

I would like to hear your opinions about this.




Attaching rebased and updated patches.


Patch 0098: LGTM


Patch 0099:

a) This check should be done in Server.__init__() rather than
install_check():

+if options.ignore_disconnected_topology:
+print("'--ignore-disconnected-topology' is used only
during "
+  "uninstallation")
+sys.exit(1)


b)
s/--ignore-disconnected-topology/--ignore-topology-disconnect/,
for
consistency with other options, e.g. --no-ui-redirect.

Maybe even shorten it to --ignore-topology? But we probably
don't
want
people to use this option much, so it might be better to keep it
long?


I would rather leave it with the long option name, it is more
apparent
what this switch should be around.


c) I'm fine with uninstall options, you can remove the TODO:

+# TODO: ask jcholast about uninstallation options


Honza



Attaching updated patches.


NACK

ipa-server-install --uninstall

2015-11-30T17:14:30Z DEBUG Destroyed connection
context.ldap2_140081152041808
2015-11-30T17:14:30Z DEBUG Traceback (most recent call last):
   File
"/usr/lib/python2.7/site-packages/ipapython/install/common.py",
line 91, in _handle_exception
 super(Continuous, self)._handle_exception(exc_info)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 387, in _handle_exception
 six.reraise(*exc_info)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 439, in _handle_exception
 super(ComponentBase, self)._handle_exception(exc_info)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 387, in _handle_exception
 six.reraise(*exc_info)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 355, in __runner
 step()
   File
"/usr/lib/python2.7/site-packages/ipapython/install/core.py",
line 352, in 
 step = lambda: next(self.__gen)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/util.py",
line 81, in run_generator_with_yield_from
 six.reraise(*exc_info)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/util.py",
line 59, in run_generator_with_yield_from
 value = gen.send(prev_value)
   File
"/usr/lib/python2.7/site-packages/ipapython/install/common.py",
line 71, in _uninstall
 for nothing in self._uninstaller(self.parent):
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",



line 1409, in main
 uninstall_check(self)
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",



line 265, in decorated
 func(installer)
   File
"/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",



line 1140, in uninstall_check
 api, masters, options.ignore_disconnected_topology)
AttributeError: 'uninstaller(Server)' object has no attribute
'ignore_disconnected_topology'

2015-11-30T17:14:30Z ERROR 'uninstaller(Server)' object has no
attribute
'ignore_disconnected_topology'
2015-11-30T17:14:30Z INFO The ipa-server-install command was
successful



Sorry I have failed horribly during option rename. Attaching patch
that should actually work.


functional ACK

Attaching rebased patches reflecting the recent changes in the
handling
of managed topology suffixes handling.






Jan had some more suggestions to the patches. Attaching updated
version.




Attaching updated patch 99 with fixed error message.


Pushed to master: b8c619a7139bd7b65caa03b68431e22791ff19bf


ACK :-)


I was doing some unrelated testing with domain level 0 and forgot to
remove --ignore-topology disconnect from my command line before
uninstalling server, which gave me an error that the option cannot be
used in domain level 0 and I had to re-run ipa-server-install
--uninstall with the option removed.

IMO it would be better for UX if the option was ignored in domain level
0, since topology disconnects are *always* ignored in domain level 0.

Re: [Freeipa-devel] [PATCHES 523-525] replica install: add remote connection check over API

[Jan Cholasta]

On 9.12.2015 14:52, Jan Cholasta wrote:

On 9.12.2015 10:02, Jan Cholasta wrote:

Hi,

the attached patches fix .


Note that this needs selinux-policy fix to work, so put SELinux into
permissive mode for testing:
.


Updated patches attached.

--
Jan Cholasta
From 4355c6043c1c6415d4242e9d49b3f2c84d0f9f39 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 9 Dec 2015 08:17:07 +0100
Subject: [PATCH 1/3] build: put oddjob scripts into separate directory

https://fedorahosted.org/freeipa/ticket/5497
---
 freeipa.spec.in  | 3 ++-
 install/oddjob/Makefile.am   | 2 +-
 install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf | 2 +-
 3 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index a60d9b6..95948e7 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -740,6 +740,7 @@ fi
 %{_libexecdir}/ipa/ipa-dnskeysync-replica
 %{_libexecdir}/ipa/ipa-ods-exporter
 %{_libexecdir}/ipa/ipa-httpd-kdcproxy
+%dir %{_libexecdir}/ipa/oddjob
 %ghost %verify(not owner group) %dir %{_sharedstatedir}/kdcproxy
 %dir %attr(0755,root,root) %{_sysconfdir}/ipa/kdcproxy
 %config(noreplace) %{_sysconfdir}/sysconfig/ipa_memcached
@@ -914,7 +915,7 @@ fi
 %ghost %{_libdir}/krb5/plugins/libkrb5/winbind_krb5_locator.so
 %{_sysconfdir}/dbus-1/system.d/oddjob-ipa-trust.conf
 %{_sysconfdir}/oddjobd.conf.d/oddjobd-ipa-trust.conf
-%%attr(755,root,root) %{_libexecdir}/ipa/com.redhat.idm.trust-fetch-domains
+%attr(755,root,root) %{_libexecdir}/ipa/oddjob/com.redhat.idm.trust-fetch-domains
 
 %endif # ONLY_CLIENT
 
diff --git a/install/oddjob/Makefile.am b/install/oddjob/Makefile.am
index 9dde10c..5cdaf2b 100644
--- a/install/oddjob/Makefile.am
+++ b/install/oddjob/Makefile.am
@@ -1,6 +1,6 @@
 NULL =
 
-oddjobdir = $(libexecdir)/ipa
+oddjobdir = $(libexecdir)/ipa/oddjob
 oddjobconfdir = $(sysconfdir)/oddjobd.conf.d
 dbusconfdir = $(sysconfdir)/dbus-1/system.d
 
diff --git a/install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf b/install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf
index 17817de..bc2e8d1 100644
--- a/install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf
+++ b/install/oddjob/etc/oddjobd.conf.d/oddjobd-ipa-trust.conf
@@ -10,7 +10,7 @@
   
   
 
-  
-- 
2.4.3

From c0118edf52732d2b514b206448f05f1f43ac8ea6 Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 9 Dec 2015 08:18:21 +0100
Subject: [PATCH 2/3] replica install: add remote connection check over API

Add server_conncheck command which calls ipa-replica-conncheck --replica
over oddjob.

https://fedorahosted.org/freeipa/ticket/5497
---
 API.txt|   8 ++
 VERSION|   4 +-
 freeipa.spec.in|   9 +-
 install/oddjob/Makefile.am |   3 +
 .../etc/dbus-1/system.d/org.freeipa.server.conf|  21 
 install/oddjob/etc/oddjobd.conf.d/ipa-server.conf  |  20 
 install/oddjob/org.freeipa.server.conncheck|   2 +
 install/tools/ipa-ca-install   |   6 +
 install/tools/ipa-replica-conncheck| 131 ++---
 install/updates/90-post_upgrade_plugins.update |   1 -
 ipalib/errors.py   |   1 +
 ipalib/messages.py |  10 ++
 ipalib/plugins/server.py   |  70 ++-
 ipaserver/install/adtrustinstance.py   |  19 ---
 ipaserver/install/ca.py|   2 +-
 ipaserver/install/httpinstance.py  |  26 
 ipaserver/install/installutils.py  |  12 --
 ipaserver/install/plugins/adtrust.py   |  21 
 ipaserver/install/replication.py   |   6 +-
 ipaserver/install/server/replicainstall.py |   5 +-
 ipaserver/install/server/upgrade.py|   1 +
 21 files changed, 300 insertions(+), 78 deletions(-)
 create mode 100644 install/oddjob/etc/dbus-1/system.d/org.freeipa.server.conf
 create mode 100644 install/oddjob/etc/oddjobd.conf.d/ipa-server.conf
 create mode 100755 install/oddjob/org.freeipa.server.conncheck

diff --git a/API.txt b/API.txt
index 60c98c3..15be32c 100644
--- a/API.txt
+++ b/API.txt
@@ -3812,6 +3812,14 @@ option: Str('version?', exclude='webui')
 output: Entry('result', , Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (, ), None)
 output: PrimaryKey('value', None, None)
+command: server_conncheck
+args: 2,1,3
+arg: Str('cn', attribute=True, cli_name='name', multivalue=False, primary_key=True, query=True, required=True)
+arg: Str('remote_cn', cli_name='remote_name')
+option: Str('version?', exclude='webui')
+output: Output('result', , None)

Re: [Freeipa-devel] [PATCHES 509-514] replica promotion: use host credentials when setting up replication

[Jan Cholasta]

On 7.12.2015 08:14, Jan Cholasta wrote:

On 6.12.2015 21:32, Martin Basti wrote:



On 04.12.2015 16:58, Simo Sorce wrote:

On Fri, 2015-12-04 at 15:39 +0100, Jan Cholasta wrote:

On 4.12.2015 15:16, Jan Cholasta wrote:

On 4.12.2015 15:12, Jan Cholasta wrote:

On 4.12.2015 11:15, Petr Vobornik wrote:

On 12/03/2015 03:11 PM, Martin Basti wrote:


On 01.12.2015 12:19, Jan Cholasta wrote:

On 23.11.2015 15:47, Simo Sorce wrote:

On Mon, 2015-11-23 at 15:37 +0100, Jan Cholasta wrote:

Ad alternative is to add the host to ipaservers before the
checks
are
done and remove it again if any of them fail.

Too error prone, I am ok with the current way in your patches
until/unless I can think of a fail safe way. :-)

Updated patches attached. Note that 520 should be applied
between 509
and 510.




Functional ACK


Simo, do you want to review the ACIs or other things it the
patches? Or
can the patches be pushed?

There were no changes in the ACIs since last time.

Actually, memberPrincipal was removed from the "IPA server hosts can
manage own Custodia secrets" ACI, as per Simo's request.


Rebased patches attached.

Note that 520 should still be applied between 509 and 510.


LGTM


ACK


Thanks.

Pushed to master: 01ddf51df76f3298499973355c5461727e46ab5b


Martin Babinsky found out that ipaservers is not created early enough 
when installing a replica of a 4.2 or older server which causes a crash.


The attached patch fixes that.

--
Jan Cholasta
From eb887cf4291857b5fb5ce1bd991d460e7df4990b Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 9 Dec 2015 15:56:24 +0100
Subject: [PATCH] replica install: add ipaservers before the server's host
 entry is created

This prevents crash when adding the host entry to ipaservers when
installing replica of a 4.2 or older server.

https://fedorahosted.org/freeipa/ticket/3416
---
 ipaserver/install/dsinstance.py  | 4 
 ipaserver/install/krbinstance.py | 5 -
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/ipaserver/install/dsinstance.py b/ipaserver/install/dsinstance.py
index a58b0f7..1b82e56 100644
--- a/ipaserver/install/dsinstance.py
+++ b/ipaserver/install/dsinstance.py
@@ -417,6 +417,10 @@ class DsInstance(service.Service):
r_bindpw=self.dm_password)
 self.run_init_memberof = repl.needs_memberof_fixup()
 
+ld = ldapupdate.LDAPUpdate(ldapi=True)
+ld.update([os.path.join(paths.UPDATES_DIR,
+'20-ipaservers_hostgroup.update')])
+
 # Now that the server is up make sure all changes happen against
 # the local server (as repica pomotion does not have the DM password.
 if self.admin_conn:
diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py
index f928e50..1a7b65a 100644
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -122,7 +122,10 @@ class KrbInstance(service.Service):
   ('cn', 'accounts'), self.suffix)
 hostgroup_entry = self.admin_conn.get_entry(hostgroup_dn, ['member'])
 hostgroup_entry.setdefault('member', []).append(host_dn)
-self.admin_conn.update_entry(hostgroup_entry)
+try:
+self.admin_conn.update_entry(hostgroup_entry)
+except errors.EmptyModlist:
+pass
 
 def __common_setup(self, realm_name, host_name, domain_name, admin_password):
 self.fqdn = host_name
-- 
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 0394] topology: Make sure the old 'realm' topology suffix is not

[Martin Basti]

On 09.12.2015 09:43, Martin Basti wrote:



On 08.12.2015 17:32, Martin Babinsky wrote:

On 12/08/2015 04:53 PM, Tomas Babej wrote:



On 12/08/2015 02:28 PM, Tomas Babej wrote:

Hi,

The old 'realm' topology suffix is no longer used, however, it was 
being
created on masters with version 4.2.3 and later. Make sure it's 
properly

removed.

Note that this is not the case for the 'ipaca' suffix, which was later
removed to 'ca'.

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



Actually, we found out with Martin that this patch deletes realm 
instead

of domain suffix, against all initial impressions.

Updated patch attached.

Tomas





Works for me, ACK.

I have also made some hardening in topology connectivity checks so 
that this kind of situations is handled and reported by them. I will 
send a patch in separate thread.



Pushed to master: a84b7d2117aafc5182640d0a22675b214c27dd7c

I accidentally pushed first revision of the patch, fix pushed to master: 
dcb5c2a5200a797b0eec9bb809c570f9ed80f7bb


--
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 560] Allow to set allowed krb authz data type per user

[Simo Sorce]

- Original Message -
> From: "Alexander Bokovoy" 
> To: "Simo Sorce" 
> Cc: "Jan Cholasta" , "freeipa-devel" 
> 
> Sent: Tuesday, December 1, 2015 3:07:32 AM
> Subject: Re: [Freeipa-devel] [PATCH 560] Allow to set allowed krb authz data 
> type per user
> 
> On Wed, 25 Nov 2015, Simo Sorce wrote:
> >On Wed, 2015-11-25 at 08:09 +0100, Jan Cholasta wrote:
> >> On 25.11.2015 00:09, Simo Sorce wrote:
> >> > This patch is untested and mostly an RFC.
> >> >
> >> > I think it is all we need to allow to specify authz data types per user
> >> > and by setting the attribute to NONE preventing a user from getting
> >> > MS-PAC data in their ticket.
> >> >
> >> > Alexander you changed quite a bit the code around here so I'd like to
> >> > know if you think the change I made in the KDC will cause any issue with
> >> > the special PACs we generate for master's principals. As far as I can
> >> > tell it shouldn't.
> >> >
> >> > Any opinion is welcome.
> >>
> >> Before your change, the server entry was checked for AS requests, now
> >> only the client entry is checked for AS requests. I'm not very familiar
> >> with ipa-kdb, but shouldn't the server entry still be checked as a
> >> fallback when there is no authorization data in the client entry?
> >
> >This is partly why I CCed Alexander, the way the get function works is
> >that it will get policy on the entry itself and if nothing is there it
> >will try with the global policy, so in both cases the global policy is
> >sourced as fallback.
> >
> >For AS requests though you are generally asking for a TGT so the
> >"server" is the krbtgt entry that has no policy. It is through though
> >that a client *can* ask for a ticket directly via an AS request, that is
> >uncommon and it is unclear to me what we should do in that case if
> >client and server have incompatible options.
> >
> >Well this is why it is a RFC after all :)
> Can we source global policy for the direct AS request as well?

I think I would do this in a separate patch.

> >> The attribute is exposed in the service plugin, shouldn't it be exposed
> >> in the user plugin as well?
> >
> >I didn't do it on purpose yet but eventually we may want to expose it,
> >indeed. The reason I didn't is that we may want to use something like
> >CoS to populate the attribute based on group membership and I am not
> >sure we want to expose it per user, up top debate.
> I don't want to expose it in the config too.

Agreed.

attached find an updated patch as I found a crash bug with the older one in 
some situations.

Simo.
From f4a27d66e5197f8dfe66d6e63b2fd6c17212d497 Mon Sep 17 00:00:00 2001
From: Simo Sorce 
Date: Tue, 24 Nov 2015 18:01:52 -0500
Subject: [PATCH] Allow to specify Kerberos authz data type per user

Like for services setting the ipaKrbAuthzData attribute on a user object will
allow us to control exactly what authz data is allowed for that user.
Setting NONE would allow no authz data, while setting MS-PAC would allow only
Active Directory compatible data.

Signed-off-by: Simo Sorce 

Ticket: https://fedorahosted.org/freeipa/ticket/2579
---
 daemons/ipa-kdb/ipa_kdb_mspac.c | 16 +---
 install/share/60basev3.ldif |  2 +-
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 8594309dbd27b45abda68de5f7ebf0c31e16904d..2687803b769e54b597b8cb14554c6e7e177b3cad 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -2139,11 +2139,13 @@ krb5_error_code ipadb_sign_authdata(krb5_context context,
 ks_client_princ = client->princ;
 }
 
-/* We only need to check the server entry here, because even if the client
- * is a service with a valid authorization data it will result to NONE
- * because ipadb_get_pac() can only generate a pac for 'real' IPA users.
- * (I assume this will be the same for PAD.) */
-get_authz_data_types(context, server, _pac, _pad);
+if (client_entry == NULL) client_entry = client;
+
+if (is_as_req) {
+get_authz_data_types(context, client_entry, _pac, _pad);
+} else {
+get_authz_data_types(context, server, _pac, _pad);
+}
 
 if (with_pad) {
 krb5_klog_syslog(LOG_ERR, "PAD authorization data is requested but " \
@@ -2185,7 +2187,7 @@ krb5_error_code ipadb_sign_authdata(krb5_context context,
 /* check or generate pac data */
 if ((pac_auth_data == NULL) || (pac_auth_data[0] == NULL)) {
 if (flags & KRB5_KDB_FLAG_CONSTRAINED_DELEGATION) {
-kerr = ipadb_get_pac(context, client_entry ? client_entry : client, );
+kerr = ipadb_get_pac(context, client_entry : client, );
 if (kerr != 0 && kerr != ENOENT) {
 goto done;
 }
@@ -2238,7 +2240,7 @@ krb5_error_code 

[Freeipa-devel] [PATCH 0373] Upgrade: Fix IPA version comparison

[Martin Basti]

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

Patch attached.
From 8ef93485d61e8732166fb0c5b6c4559209740f3e Mon Sep 17 00:00:00 2001
From: Martin Basti 
Date: Wed, 9 Dec 2015 18:53:35 +0100
Subject: [PATCH] Fix version comparison

Use RPM library to compare vendor versions of IPA for redhat platform

https://fedorahosted.org/freeipa/ticket/5535
---
 freeipa.spec.in |  2 ++
 ipaplatform/redhat/tasks.py | 19 +++
 2 files changed, 21 insertions(+)

diff --git a/freeipa.spec.in b/freeipa.spec.in
index 9f82b3695fb10c4db65cc31278364b3b34e26098..09feba7b8324f5e645da3e8010de86b6c3ee5ab9 100644
--- a/freeipa.spec.in
+++ b/freeipa.spec.in
@@ -166,6 +166,8 @@ Requires: %{etc_systemd_dir}
 Requires: gzip
 Requires: python-gssapi >= 1.1.0
 Requires: custodia
+Requires: rpm-python
+Requires: rpmdevtools
 
 Provides: %{alt_name}-server = %{version}
 Conflicts: %{alt_name}-server
diff --git a/ipaplatform/redhat/tasks.py b/ipaplatform/redhat/tasks.py
index 94d2cb4e906965a20bcfdd55f38854005091c26f..0debae1f39924b608190ef7a7f9ba5ebe1b13dfc 100644
--- a/ipaplatform/redhat/tasks.py
+++ b/ipaplatform/redhat/tasks.py
@@ -30,11 +30,13 @@ import stat
 import socket
 import sys
 import base64
+import rpm
 
 from subprocess import CalledProcessError
 from nss.error import NSPRError
 from pyasn1.error import PyAsn1Error
 from six.moves import urllib
+from rpmUtils.miscutils import stringToVersion
 
 from ipapython.ipa_log_manager import root_logger, log_mgr
 from ipapython import ipautil
@@ -66,6 +68,16 @@ def selinux_enabled():
 return False
 
 
+class IPAVersion(object):
+
+def __init__(self, version):
+self.version_tuple = stringToVersion(version)
+
+def __cmp__(self, other):
+assert isinstance(other, IPAVersion)
+return rpm.labelCompare(self.version_tuple, other.version_tuple)
+
+
 class RedHatTaskNamespace(BaseTaskNamespace):
 
 def restore_context(self, filepath, restorecon=paths.SBIN_RESTORECON):
@@ -423,5 +435,12 @@ class RedHatTaskNamespace(BaseTaskNamespace):
 super(RedHatTaskNamespace, self).create_system_user(name, group,
 homedir, shell, uid, gid, comment, create_homedir)
 
+def parse_ipa_version(self, version):
+"""
+:param version: textual version
+:return: object implementing proper __cmp__ method for version compare
+"""
+return IPAVersion(version)
+
 
 tasks = RedHatTaskNamespace()
-- 
2.5.0

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

Re: [Freeipa-devel] [PATCHES 509-514] replica promotion: use host credentials when setting up replication

[Jan Cholasta]

On 9.12.2015 16:39, Jan Cholasta wrote:

On 7.12.2015 08:14, Jan Cholasta wrote:

On 6.12.2015 21:32, Martin Basti wrote:



On 04.12.2015 16:58, Simo Sorce wrote:

On Fri, 2015-12-04 at 15:39 +0100, Jan Cholasta wrote:

On 4.12.2015 15:16, Jan Cholasta wrote:

On 4.12.2015 15:12, Jan Cholasta wrote:

On 4.12.2015 11:15, Petr Vobornik wrote:

On 12/03/2015 03:11 PM, Martin Basti wrote:


On 01.12.2015 12:19, Jan Cholasta wrote:

On 23.11.2015 15:47, Simo Sorce wrote:

On Mon, 2015-11-23 at 15:37 +0100, Jan Cholasta wrote:

Ad alternative is to add the host to ipaservers before the
checks
are
done and remove it again if any of them fail.

Too error prone, I am ok with the current way in your patches
until/unless I can think of a fail safe way. :-)

Updated patches attached. Note that 520 should be applied
between 509
and 510.




Functional ACK


Simo, do you want to review the ACIs or other things it the
patches? Or
can the patches be pushed?

There were no changes in the ACIs since last time.

Actually, memberPrincipal was removed from the "IPA server hosts can
manage own Custodia secrets" ACI, as per Simo's request.


Rebased patches attached.

Note that 520 should still be applied between 509 and 510.


LGTM


ACK


Thanks.

Pushed to master: 01ddf51df76f3298499973355c5461727e46ab5b


Martin Babinsky found out that ipaservers is not created early enough
when installing a replica of a 4.2 or older server which causes a crash.

The attached patch fixes that.


Actually I don't like how I fixed that, here's an updated patch.

Also, I noticed that replica promotion fails too late in domain level 0. 
Fixed as well.


--
Jan Cholasta
From 00db51a7a3c3b38fc8e2680bbb0304d74ebabcfa Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Wed, 9 Dec 2015 15:56:24 +0100
Subject: [PATCH] replica install: add ipaservers if it does not exist

This prevents crash when adding the host entry to ipaservers when
installing replica of a 4.2 or older server.

https://fedorahosted.org/freeipa/ticket/3416
---
 ipaserver/install/krbinstance.py | 9 -
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/ipaserver/install/krbinstance.py b/ipaserver/install/krbinstance.py
index f928e50..cd803b0 100644
--- a/ipaserver/install/krbinstance.py
+++ b/ipaserver/install/krbinstance.py
@@ -41,6 +41,7 @@ from ipapython.dn import DN
 
 from ipaserver.install import replication
 from ipaserver.install import dsinstance
+from ipaserver.install import ldapupdate
 
 import pyasn1.codec.ber.decoder
 import struct
@@ -118,11 +119,9 @@ class KrbInstance(service.Service):
 self.admin_conn.add_entry(host_entry)
 
 # Add the host to the ipaserver host group
-hostgroup_dn = DN(('cn', 'ipaservers'), ('cn', 'hostgroups'),
-  ('cn', 'accounts'), self.suffix)
-hostgroup_entry = self.admin_conn.get_entry(hostgroup_dn, ['member'])
-hostgroup_entry.setdefault('member', []).append(host_dn)
-self.admin_conn.update_entry(hostgroup_entry)
+ld = ldapupdate.LDAPUpdate(ldapi=True)
+ld.update([os.path.join(paths.UPDATES_DIR,
+'20-ipaservers_hostgroup.update')])
 
 def __common_setup(self, realm_name, host_name, domain_name, admin_password):
 self.fqdn = host_name
-- 
2.4.3

From cba6aa7404fb9475f34d189f7ed97ca63c7a2c0e Mon Sep 17 00:00:00 2001
From: Jan Cholasta 
Date: Thu, 10 Dec 2015 07:23:18 +0100
Subject: [PATCH] replica promotion: check domain level before ipaservers
 membership

Check domain level before checking ipaservers membership to prevent
"not found" error when attempting replica promotion in domain level 0.

https://fedorahosted.org/freeipa/ticket/5401
---
 ipaserver/install/server/replicainstall.py | 28 ++--
 1 file changed, 14 insertions(+), 14 deletions(-)

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index a42ed7e..d10dfd3 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -973,6 +973,20 @@ def promote_check(installer):
 replman = ReplicationManager(config.realm_name,
  config.master_host_name, None)
 
+# Detect the current domain level
+try:
+current = remote_api.Command['domainlevel_get']()['result']
+except errors.NotFound:
+# If we're joining an older master, domain entry is not
+# available
+current = constants.DOMAIN_LEVEL_0
+
+if current == constants.DOMAIN_LEVEL_0:
+raise RuntimeError(
+"You must provide a file generated by ipa-replica-prepare to "
+"create a replica when the domain is at level 0."
+)
+
 # Check authorization
 result = remote_api.Command['hostgroup_find'](
 cn=u'ipaservers',
@@ -1027,20 +1041,6 @@ def 

Re: [Freeipa-devel] [PATCH 0071] replica: Fix ipa-replica-install with replica file (domain, level 0).

[Jan Cholasta]

On 9.12.2015 08:31, David Kupka wrote:

On 08/12/15 16:33, Tomas Babej wrote:



On 12/08/2015 04:20 PM, Oleg Fayans wrote:

ACK. The initial issue is fixed.

On 12/08/2015 03:03 PM, David Kupka wrote:

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






Can we get some more love for the patch and provide at least a sentence
worth of commit message before pushing?

It's not obvious from the title what the patch does, other than it fixes
things.

Tomas


I believe it's pretty obvious from linked ticket and attached patch
changing just 5 lines.
But you're right verbosity in commit message could save time later.
Patch with changed commit message attached.


ACK.

Pushed to master: b7953cda4fc02637f6e3db574b3d7163efc78a98

--
Jan Cholasta

--
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] [PATCHES 516-517] spec file: put Python modules into standalone packages

[Petr Vobornik]

On 12/07/2015 04:21 PM, Jan Cholasta wrote:

Hi,

the attached patches partially fix
. This is done to allow
the addition of Python 3 packages, see
.
See commit messages for more information.

In order to test:
1. make rpms
2.


3. Test with both dnf and yum-deprecated.

Beware that when you run "yum-deprecated clean all", it does not remove
cache for the on-disk repository created in step 2, you have to remove
the /var/cache/yum/$basearch/$releasever/$reponame directory manually.

Honza



Shouldn't freeipa-server-dns and freeipa-server-trust-add depend on 
freeipa-server? They do not in this patch. IMO they should.


following updates work (all on f23, update from 4.2.3):
  dnf update
  dnf update freeipa-*
  yum-depracated update freeipa-*

for both client or server with all packages.

but when I tried to install only client "dnf install freeipa-client" and 
then following failed:

  dnf update freeipa-client

The difference was:
Installing:
 freeipa-client-common  noarch
 freeipa-common noarch
 python2-ipaclient  noarch
 python2-ipalib x86_64
Upgrading:
 freeipa-client

Works:
 Installing:
 freeipa-client-common  noarch
 freeipa-common noarch
 freeipa-python-compat  noarch
 replacing  freeipa-python.x86_64 4.2.3-1.1.fc23
 python2-ipaclient  noarch
 python2-ipalib x86_64
Upgrading:
 freeipa-client


not sure if it is a problem, otherwise the patch looks OK.
--
Petr Vobornik

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

Re: [Freeipa-devel] [PATCH 560] Allow to set allowed krb authz data type per user

[Simo Sorce]

Sent the wrong patch, attached the one that actually compiles.

- Original Message -
> From: "Simo Sorce" 
> To: "Alexander Bokovoy" 
> Cc: "Simo Sorce" , "Jan Cholasta" , 
> "freeipa-devel" 
> Sent: Wednesday, December 9, 2015 2:18:23 PM
> Subject: Re: [Freeipa-devel] [PATCH 560] Allow to set allowed krb authz data 
> type per user
> 
> - Original Message -
> > From: "Alexander Bokovoy" 
> > To: "Simo Sorce" 
> > Cc: "Jan Cholasta" , "freeipa-devel"
> > 
> > Sent: Tuesday, December 1, 2015 3:07:32 AM
> > Subject: Re: [Freeipa-devel] [PATCH 560] Allow to set allowed krb authz
> > data type per user
> > 
> > On Wed, 25 Nov 2015, Simo Sorce wrote:
> > >On Wed, 2015-11-25 at 08:09 +0100, Jan Cholasta wrote:
> > >> On 25.11.2015 00:09, Simo Sorce wrote:
> > >> > This patch is untested and mostly an RFC.
> > >> >
> > >> > I think it is all we need to allow to specify authz data types per
> > >> > user
> > >> > and by setting the attribute to NONE preventing a user from getting
> > >> > MS-PAC data in their ticket.
> > >> >
> > >> > Alexander you changed quite a bit the code around here so I'd like to
> > >> > know if you think the change I made in the KDC will cause any issue
> > >> > with
> > >> > the special PACs we generate for master's principals. As far as I can
> > >> > tell it shouldn't.
> > >> >
> > >> > Any opinion is welcome.
> > >>
> > >> Before your change, the server entry was checked for AS requests, now
> > >> only the client entry is checked for AS requests. I'm not very familiar
> > >> with ipa-kdb, but shouldn't the server entry still be checked as a
> > >> fallback when there is no authorization data in the client entry?
> > >
> > >This is partly why I CCed Alexander, the way the get function works is
> > >that it will get policy on the entry itself and if nothing is there it
> > >will try with the global policy, so in both cases the global policy is
> > >sourced as fallback.
> > >
> > >For AS requests though you are generally asking for a TGT so the
> > >"server" is the krbtgt entry that has no policy. It is through though
> > >that a client *can* ask for a ticket directly via an AS request, that is
> > >uncommon and it is unclear to me what we should do in that case if
> > >client and server have incompatible options.
> > >
> > >Well this is why it is a RFC after all :)
> > Can we source global policy for the direct AS request as well?
> 
> I think I would do this in a separate patch.
> 
> > >> The attribute is exposed in the service plugin, shouldn't it be exposed
> > >> in the user plugin as well?
> > >
> > >I didn't do it on purpose yet but eventually we may want to expose it,
> > >indeed. The reason I didn't is that we may want to use something like
> > >CoS to populate the attribute based on group membership and I am not
> > >sure we want to expose it per user, up top debate.
> > I don't want to expose it in the config too.
> 
> Agreed.
> 
> attached find an updated patch as I found a crash bug with the older one in
> some situations.
> 
> Simo.
> 
From f21c88b9f74453c6d6e16fb17d94efa469eed564 Mon Sep 17 00:00:00 2001
From: Simo Sorce 
Date: Tue, 24 Nov 2015 18:01:52 -0500
Subject: [PATCH] Allow to specify Kerberos authz data type per user

Like for services setting the ipaKrbAuthzData attribute on a user object will
allow us to control exactly what authz data is allowed for that user.
Setting NONE would allow no authz data, while setting MS-PAC would allow only
Active Directory compatible data.

Signed-off-by: Simo Sorce 

Ticket: https://fedorahosted.org/freeipa/ticket/2579
---
 daemons/ipa-kdb/ipa_kdb_mspac.c | 16 +---
 install/share/60basev3.ldif |  2 +-
 2 files changed, 10 insertions(+), 8 deletions(-)

diff --git a/daemons/ipa-kdb/ipa_kdb_mspac.c b/daemons/ipa-kdb/ipa_kdb_mspac.c
index 8594309dbd27b45abda68de5f7ebf0c31e16904d..5d11f3c375dd1d0715cb1813cef4de2e5b4744fd 100644
--- a/daemons/ipa-kdb/ipa_kdb_mspac.c
+++ b/daemons/ipa-kdb/ipa_kdb_mspac.c
@@ -2139,11 +2139,13 @@ krb5_error_code ipadb_sign_authdata(krb5_context context,
 ks_client_princ = client->princ;
 }
 
-/* We only need to check the server entry here, because even if the client
- * is a service with a valid authorization data it will result to NONE
- * because ipadb_get_pac() can only generate a pac for 'real' IPA users.
- * (I assume this will be the same for PAD.) */
-get_authz_data_types(context, server, _pac, _pad);
+if (client_entry == NULL) client_entry = client;
+
+if (is_as_req) {
+get_authz_data_types(context, client_entry, _pac, _pad);
+} else {
+get_authz_data_types(context, server, _pac, _pad);
+}
 
 if (with_pad) {
 krb5_klog_syslog(LOG_ERR, "PAD authorization data is requested