Re: [Freeipa-devel] [PATCH 0093] Enable service authentication indicator management

[Nathaniel McCallum]

On Mon, 2016-05-30 at 19:08 +0300, Alexander Bokovoy wrote:
> On Mon, 30 May 2016, Petr Vobornik wrote:
> > On 05/27/2016 06:00 PM, Nathaniel McCallum wrote:
> > > Pavel, since we made the change here from a StrEnum to a Str, we
> > > need
> > > to update the UI patch accordingly.
> > 
> > How should admin know what to write there intuitively?
> > 
> > Shouldn't Web UI or CLI advertise the indicators supported by IPA?
> > E.g.
> > CLI in doc string. Web UI might even combine checkboxes (otp,
> > radius)
> > with textbox.
> That would be better, I think. We still need to keep the API with a
> free
> text field but Web UI, of course, should provide some pre-defined
> labels.

+1

-- 
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 0110] DNS: Warn if forwarding policy conflicts with automatic empty zone

[Martin Basti]

On 27.05.2016 14:13, Petr Spacek wrote:

On 25.5.2016 12:30, Martin Basti wrote:


On 04.05.2016 10:43, Petr Spacek wrote:

Hello,

DNS: Warn if forwarding policy conflicts with automatic empty zones

Forwarding policy "first" or "none" may conflicts with some automatic empty
zones. Queries for zones specified by RFC 6303 will ignore
forwarding and recursion and always result in NXDOMAIN answers.

This is not detected and warned about. Global forwarding is equivalent
to forward zone ".".

Example:
Forward zone 1.10.in-addr.arpa with policy "first"
will not forward anything because BIND will automatically prefer
automatic empty zone "10.in-addr.arpa." which is authoritative.

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


This is last patch in the series so the ticket can be closed when all relevant
patches are pushed.





You forgot to update tests

_
test_dns.test_command[0087: dnsconfig_mod: Update global DNS settings]
__

self = , index = 87
declarative_test_definition = {'command': ('dnsconfig_mod', [],
{'idnsforwarders': ['172.16.31.80'], 'version': '2.166'}), 'desc': 'Update
global DN...arders': ['172.16.31.80']}, 'summary': None, 'value': None},
'nice': '0087: dnsconfig_mod: Update global DNS settings'}

 def test_command(self, index, declarative_test_definition):
 """Run an individual test

 The arguments are provided by the pytest plugin.
 """
 if callable(declarative_test_definition):
 declarative_test_definition(self)
 else:

   self.check(**declarative_test_definition)

test_xmlrpc/xmlrpc_test.py:313:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
test_xmlrpc/xmlrpc_test.py:325: in check
 self.check_output(nice, cmd, args, options, expected, extra_check)
test_xmlrpc/xmlrpc_test.py:368: in check_output
 assert_deepequal(expected, got, nice)
util.py:361: in assert_deepequal
 assert_deepequal(e_sub, g_sub, doc, stack + (key,))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

expected = [{'code': 13006, 'message':  at 0x7fcef426c758>,
'name': 'DNSServerValidationWarning', 'type': 'warning'}]
got = [{'code': 13021, 'message': "Forwarding policy conflicts with some
automatic empty zones. Queries for zones specified ...': The DNS operation
timed out after 10.0008428097 seconds.", 'name': 'DNSServerValidationWarning',
'type': 'warning'}]
doc = '0087: dnsconfig_mod: Update global DNS settings', stack = ('messages',)

 def assert_deepequal(expected, got, doc='', stack=tuple()):
 """
 Recursively check for type and equality.

 If a value in expected is callable then it will used as a callback to
 test for equality on the got value. The callback is passed the got
 value and returns True if equal, False otherwise.

 If the tests fails, it will raise an ``AssertionError`` with detailed
 information, including the path to the offending value.  For example:

 >>> expected = [u'Hello', dict(world=u'how are you?')]
 >>> got = [u'Hello', dict(world='how are you?')]
 >>> expected == got
 True
 >>> assert_deepequal(expected, got, doc='Testing my nested data')
 Traceback (most recent call last):
   ...
 AssertionError: assert_deepequal: type(expected) is not type(got).
   Testing my nested data
   type(expected) = 
   type(got) = 
   expected = u'how are you?'
   got = 'how are you?'
   path = (0, 'world')

 Note that lists and tuples are considered equivalent, and the order of
 their elements does not matter.
 """
 if isinstance(expected, tuple):
 expected = list(expected)
 if isinstance(got, tuple):
 got = list(got)
 if isinstance(expected, DN):
 if isinstance(got, six.string_types):
 got = DN(got)
 if not (isinstance(expected, Fuzzy) or callable(expected) or
type(expected) is type(got)):
 raise AssertionError(
 TYPE % (doc, type(expected), type(got), expected, got, stack)
 )
 if isinstance(expected, (list, tuple)):
 if len(expected) != len(got):
 raise AssertionError(

   LEN % (doc, len(expected), len(got), expected, got, stack)

 )
E   AssertionError: assert_deepequal: list length mismatch.
E 0087: dnsconfig_mod: Update global DNS settings
E 

Re: [Freeipa-devel] [PATCH 0123-132] DNS upgrade: change forwarding policy to "only" if private IPs are used

[Martin Basti]

On 30.05.2016 12:49, Petr Spacek wrote:

On 29.5.2016 14:45, Martin Basti wrote:


On 27.05.2016 14:12, Petr Spacek wrote:

On 25.5.2016 12:50, Martin Basti wrote:

On 20.05.2016 12:19, Petr Spacek wrote:

On 11.5.2016 12:08, Martin Basti wrote:

On 03.05.2016 14:59, Petr Spacek wrote:

Hello,

DNS upgrade: change forwarding policy to "only" if private IPs are used.

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

This is the upgrade part. I will add one more patch to print a warning in
dnsforwardzone* commands to avoid surprises. Please do not close the ticket
yet.




1)
Upgrade failed with 'BindInstance' object has no attribute
'named_conf_get_directive'
IPA server upgrade failed: Inspect /var/log/ipaupgrade.log and run command
ipa-server-upgrade manually.
('IPA upgrade failed.', 1)
The ipa-server-upgrade command failed. See /var/log/ipaupgrade.log for more
information

2016-05-11T08:26:20Z ERROR Upgrade failed with 'BindInstance' object has no
attribute 'named_conf_get_directive'
2016-05-11T08:26:20Z DEBUG Traceback (most recent call last):
 File
"/usr/lib/python2.7/site-packages/ipaserver/install/upgradeinstance.py",
line
213, in __upgrade
   self.modified = (ld.update(self.files) or self.modified)
 File "/usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py",
line 917, in update
   self._run_updates(all_updates)
 File "/usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py",
line 889, in _run_updates
   self._run_update_plugin(update['plugin'])
 File "/usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py",
line 862, in _run_update_plugin
   restart_ds, updates = self.api.Updater[plugin_name]()
 File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line
1418, in
__call__
   return self.execute(**options)
 File
"/usr/lib/python2.7/site-packages/ipaserver/install/plugins/dns.py",
line 547, in execute
   self.update_global_named_conf_forwarder(bind)
 File
"/usr/lib/python2.7/site-packages/ipaserver/install/plugins/dns.py",
line 508, in update_global_named_conf_forwarder
   if bind.named_conf_get_directive(
AttributeError: 'BindInstance' object has no attribute
'named_conf_get_directive'

2016-05-11T08:26:20Z DEBUG Traceback (most recent call last):
 File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py",
line
447, in start_creation
   run_step(full_msg, method)
 File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py",
line
437, in run_step
   method()
 File
"/usr/lib/python2.7/site-packages/ipaserver/install/upgradeinstance.py",
line
221, in __upgrade
   raise RuntimeError(e)
RuntimeError: 'BindInstance' object has no attribute
'named_conf_get_directive'

PATCH * Add ipaDNSVersion option to dnsconfig* commands and use new
attribute *
2)
+Int('ipadnsversion?',
+label=_('IPA DNS version'),
+),

Shouldn't be this part of System: Read DNS Configuration permission?

3)
-def postprocess_result(self, result):
+def postprocess_result(self, result, show_version):
if not any(param in result['result'] for param in self.params):
result['summary'] = unicode(_('Global DNS configuration is
empty'))

show_version param was added but I don't see it used in this patch.

4)
+Int('ipadnsversion?',
+label=_('IPA DNS version'),
+),

Could we add comment here that this option is accessible only from
installers
and upgrade?

5)
+for config_option in container_entry.get("ipaConfigString", []):
+matched = re.match("^DNSVersion\s+(?P\d+)$",
+   config_option, flags=re.I)
+if matched:
+version = int(matched.group("version"))

Shouldn't we print error if version cannot be parsed?

PATCH  * DNS upgrade: separate backup logic to make it reusable *

LGTM

PATCH * Add function ipapython.dnsutil.related_to_auto_empty_zone() *

7)
I'm curious why do you need to check superdomains?

PATCH * DNS upgrade: change forwarding policy to = only for conflicting
forward zones*

8)
+self.log.debug('Zone %s was sucessfully modified to use '
+   'forward policy "only"', zone['idnsname'][0])
<---missing empty line>
+def execute(self, **options):

PATCH * DNS upgrade: change global forwarding policy in LDAP to "only" if
private IPs are used *
9)
- dnsutil.related_to_auto_empty_zone(zone.get('idnsname')[0])
+dnsutil.related_to_auto_empty_zone(
+dnsutil.DNSName(zone.get('idnsname')[0]))

Should be in previous commit

10)
-return
+return False, []
This should be fixed in the previous commit

PATCH * DNS upgrade: change global forwarding policy in named.conf to "only"
if private IPs are used *
11)
IMO this is an upgrade of configuration and this should be in
ipaserver/install/server/upgrade.py, upgrade plugins are used only for
updating of LDAP 

[Freeipa-devel] [PATCH] 0004 Report missing certificate in external trust chain

[Florence Blanc-Renaud]

Hi all,

this patch adds in the error message the missing certificate that caused 
i/pa-server-install --external-cert-file=.../ to fail.


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

--
Florence Blanc-Renaud
Identity Management Team, Red Hat

From 5500fa36a7e61d3d86ee4550029c09fef917ce95 Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud 
Date: Mon, 30 May 2016 14:27:01 +0200
Subject: [PATCH] Report missing certificate in external trust chain

When ipa-server-install is called with an external CA, but the cert chain is
incomplete, the command exits with the following error:
ERROR CA certificate chain in  is incomplete

The fix adds in the log the name of the missing certificate:
ERRORCA certificate chain in  is incomplete: missing cert for issuer ''

https://fedorahosted.org/freeipa/ticket/5792
---
 ipaserver/install/installutils.py | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/ipaserver/install/installutils.py b/ipaserver/install/installutils.py
index 179909543e8791ff2a85b6bf4ce57dee8d00508b..a7199e083e8281747506a0bf4e5cfe1b6d115d9c 100644
--- a/ipaserver/install/installutils.py
+++ b/ipaserver/install/installutils.py
@@ -1031,8 +1031,9 @@ def load_external_cert(files, subject_base):
 break
 else:
 raise ScriptError(
-"CA certificate chain in %s is incomplete" %
-(", ".join(files)))
+"CA certificate chain in %s is incomplete: "
+"missing cert for issuer '%s'" %
+(", ".join(files), issuer))
 
 for nickname in trust_chain:
 try:
-- 
2.5.5

-- 
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 0149-0152] Server Roles: user-facing part

[Martin Babinsky]

These patches implement the usable part of Server Roles design. There 
might be some discrepancies between the design and actual 
implementation, I was head first in hacking and was not very disciplined 
in keeping the design up to date.


I will fix this ASAP.

http://www.freeipa.org/page/V4/Server_Roles
https://fedorahosted.org/freeipa/ticket/5181
https://fedorahosted.org/freeipa/ticket/5689

--
Martin^3 Babinsky
From 7dd703ba60ec3697a04d48b5bafb50de75399115 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Mon, 30 May 2016 18:18:38 +0200
Subject: [PATCH 1/4] Server Roles: public API for server roles

This patch implements the `serverroles` API plugin which introduces the
following commands:

* server-role-show SERVER ROLE: show status of a single role on a server
* server-role-find [--server SERVER [--role SUBSTRING]]:
  find role(s) matching the given SUBSTRING and return their status on IPA
  masters. If --server option is given, the query is limited to this
  server

https://fedorahosted.org/freeipa/ticket/5181
http://www.freeipa.org/page/V4/Server_Roles
---
 API.txt  |  20 ++
 VERSION  |   4 +-
 ipalib/plugins/serverrole.py | 142 +++
 3 files changed, 164 insertions(+), 2 deletions(-)
 create mode 100644 ipalib/plugins/serverrole.py

diff --git a/API.txt b/API.txt
index dbc6f1adc614607fab106ab0de7163961e7ecedc..15cf6d7d72e5ad945b238f0cca45862805a634fe 100644
--- a/API.txt
+++ b/API.txt
@@ -3889,6 +3889,26 @@ output: Output('count', type=[])
 output: ListOfEntries('result')
 output: Output('summary', type=[, ])
 output: Output('truncated', type=[])
+command: server_role_find
+args: 1,7,4
+arg: Str('criteria?')
+option: Flag('all', autofill=True, cli_name='all', default=False)
+option: Flag('raw', autofill=True, cli_name='raw', default=False)
+option: Str('server?', autofill=False, cli_name='server')
+option: Str('servrole?', autofill=False, cli_name='role')
+option: Int('sizelimit?', autofill=False)
+option: Int('timelimit?', autofill=False)
+option: Str('version?')
+output: Output('count', type=[])
+output: ListOfEntries('result')
+output: Output('summary', type=[, ])
+output: Output('truncated', type=[])
+command: server_role_show
+args: 2,1,1
+arg: Str('server', cli_name='server')
+arg: Str('servrole', cli_name='role')
+option: Str('version?')
+output: Output('result')
 command: server_show
 args: 1,5,3
 arg: Str('cn', cli_name='name')
diff --git a/VERSION b/VERSION
index eb7957eb1c5ae2487975a2fae4485a43f613cb64..b9bf6316db05cf318f15099d12d7e1a3a751e523 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=2010061412
 #  #
 
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=169
-# Last change: vault: copy arguments of client commands from server counterparts
+IPA_API_VERSION_MINOR=170
+# Last change: Server Roles: public API for server roles
diff --git a/ipalib/plugins/serverrole.py b/ipalib/plugins/serverrole.py
new file mode 100644
index ..514a0e255fe7748ee5f6a45a9ab7ed62295b677b
--- /dev/null
+++ b/ipalib/plugins/serverrole.py
@@ -0,0 +1,142 @@
+#
+# Copyright (C) 2015  FreeIPA Contributors see COPYING for license
+#
+
+from ipalib.frontend import Object
+from ipalib.parameters import Str, StrEnum
+from ipalib.plugable import Registry
+from ipalib.plugins.baseldap import (
+LDAPObject,
+LDAPQuery,
+LDAPSearch,
+)
+from ipalib import _, ngettext
+
+
+__doc__ = _("""
+IPA server roles
+""") + _("""
+Get status of roles (DNS server, CA, etc. )provided by IPA masters.
+""") + _("""
+EXAMPLES:
+""") + _("""
+  Show status of 'DNS server' role on a server:
+ipa server-role-show ipa.example.com "DNS server"
+""") + _("""
+  Show status of all roles containing 'AD' on a server:
+ipa server-role-find --server ipa.example.com --role='AD'
+""") + _("""
+  Show status of all configured roles on a server:
+ipa server-role-find ipa.example.com
+""")
+
+
+register = Registry()
+
+
+@register()
+class server_role(LDAPObject):
+"""
+association between certain role (e.g. DNS server) and its status with
+an IPA master
+"""
+backend_name = 'serverroles'
+object_name = _('server role')
+object_name_plural = _('server roles')
+default_attributes = [
+'role', 'status'
+]
+label = _('IPA Server Roles')
+label_singular = _('IPA Server Role')
+
+takes_params = (
+Str(
+'server',
+cli_name='server',
+label=_('Server name'),
+doc=_('IPA server hostname'),
+),
+Str(
+'servrole',
+cli_name='role',
+label=_("Role name"),
+doc=_("IPA server role name"),
+flags=(u'virtual_attribute',)
+),
+StrEnum(
+

Re: [Freeipa-devel] [PATCH 0146-0147] Server Roles: basic infrastructure

[Martin Babinsky]

On 05/27/2016 05:30 PM, Martin Babinsky wrote:

On 05/19/2016 04:59 PM, Martin Babinsky wrote:

Patch 0146 implements lower-lever infrastructure for querying server
roles/attributes

Patch 0147 are some basic tests slapped together for the `serverroles`
backend to ensure that it works as expected.

The new/modified CLI commands specified in the design page [1] will be
coming soon.

Also coming soon will be an interface to construct DNS records for each
role requested by Petr^2 and Martin^2 which I will start implement when
we agree on the backend implementation.

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

[1] https://www.freeipa.org/page/V4/Server_Roles#CLI




Thanks Martin and Honza for review.

I have reworked the patches based on your comments. I have split patch
146 into two (role/attribute definitions and backend code) so patches
146-147 are for code and 148 hosts test suite.

I hope that I will send API patches on monday after I resolve some
framework-related questions with the local guru.





Another revision of backend patches attached.

--
Martin^3 Babinsky
From 27e5dbcab36d8c9155cbc8dab695fa31dbf715c1 Mon Sep 17 00:00:00 2001
From: Martin Babinsky 
Date: Thu, 26 May 2016 19:24:22 +0200
Subject: [PATCH 1/3] Server roles: definitions of server roles and attributes

This patch introduces classes which define the properties of server roles and
attributes and their relationship to LDAP attributes representing the
role/attribute.

A brief documentation about defining and using roles is given at the beginning
of the module.

http://www.freeipa.org/page/V4/Server_Roles
https://fedorahosted.org/freeipa/ticket/5181
---
 ipaserver/servroles.py | 612 +
 1 file changed, 612 insertions(+)
 create mode 100644 ipaserver/servroles.py

diff --git a/ipaserver/servroles.py b/ipaserver/servroles.py
new file mode 100644
index ..6f732f71a3bcaa0c08fad94c529979873fdcc429
--- /dev/null
+++ b/ipaserver/servroles.py
@@ -0,0 +1,612 @@
+#
+# Copyright (C) 2016 FreeIPA Contributors see COPYING for license
+#
+
+
+"""
+This module contains the set of classes which abstract various bits and pieces
+of information present in the LDAP tree about functionalities such as DNS
+server, Active Directory trust controller etc. These properties come in two
+distinct groups:
+
+server roles
+this group represents a genral functionality provided by one or more
+IPA servers, such as DNS server, certificate authority and such. In
+this case there is a many-to-many mapping between the roles and the
+masters which provide them.
+
+server attributes
+these represent a functionality associated with the whole topology,
+such as CA renewal master or DNSSec key master.
+
+See the corresponding design page (http://www.freeipa.org/page/V4/Server_Roles)
+for more info.
+
+Both of these groups use `LDAPBasedProperty` class as a base.
+
+Server Roles
+
+
+Server role objects are usually consuming information from the master's service
+container (cn=FQDN,cn=masters,cn=ipa,cn=etc,$SUFFIX) are represented by
+`ServiceBasedRole`class. To create an instance of such role, you only need to
+specify role name and individual services comprising the role (more systemd
+services may be enabled to provide some function):
+
+>>> example_role = ServiceBasedRole(
+... "Example Role",
+... component_services = ['SERVICE1', 'SERVICE2'])
+>>> example_role.name
+'Example Role'
+
+Each role object has an `attr_name` property which returns its name transformed
+into an LDAP-like attribute name useful in higher-level commands implemented in
+API framework
+
+>>> example_role.attr_name
+'ipaexamplerole'
+
+The role object can then be queried for the status of the role in the whole
+topology or on a single master by using its `status` method. This method
+returns a list of dictionaries akin to LDAP entries comprised from server name,
+role name and role status (enabled if role is enabled, configured if the
+service entries are present but not marked as enabled by 'enabledService'
+config string, absent if the service entries are not present).
+
+Note that 'AD trust agent' role is based on membership of the master in the
+'adtrust agents' sysaccount group and is thus an instance of different class
+(`ADTrustBasedRole`). This role also does not have 'configured' status, since
+the master is either member of the group ('enabled') or not ('absent')
+
+Server Attributes
+=
+
+Server attributes are implemented as instances of `ServerAttribute` class. The
+attribute is defined by some flag set ornf 'ipaConfigString' attribute of some
+service entry. To create your own server attribute, see the following example:
+
+>>> example_attribute = ServerAttribute("Example Attribute", example_role,
+... "SERVICE1", "roleMaster")
+>>> 

Re: [Freeipa-devel] [PATCH] 0002 Add the culprit line when a configuration file has an incorrect format

[Florence Blanc-Renaud]

Hi Martin,

thanks for the review and the suggestion. Please find the updated patch 
attached.


Flo.

On 05/30/2016 11:00 AM, Martin Basti wrote:




On 27.05.2016 11:35, Florence Blanc-Renaud wrote:


Hi all,

this patch adds information to the output of ipa-client-install when 
it fails due to invalid format in a configuration file:
ipa-client-install failing with SyntaxError: Syntax Error: Unknown 
line format


Fixes: https://fedorahosted.org/freeipa/ticket/5811

--
Florence Blanc-Renaud
Identity Management Team, Red Hat


Thank you for your patch, I have just one nitpick. Can you please 
reuse the original exception?


-curopts.append(self.parseLine(line))
+try:
+curopts.append(self.parseLine(line))
+except SyntaxError as e:
+raise SyntaxError('{error} in file {fname}: 
[{line}]'.format(

+error=e, fname=f.name, line=line))

Martin^2


--
Florence Blanc-Renaud
Identity Management Team, Red Hat

From 9fd8c3005905bb2e3eccc0b6b43cf5a5cbdfef3c Mon Sep 17 00:00:00 2001
From: Florence Blanc-Renaud 
Date: Mon, 23 May 2016 17:18:15 +0200
Subject: [PATCH] Add the culprit line when a configuration file has an
 incorrect format

For instance if /etc/nsswitch.conf contains an incorrect line
sudoers		file sss
(Note the missing : after sudoers)
ipa-client-install exits with a SyntaxError traceback but does not state
which line caused the issue.
With the fix, the filename and the line are displayed in the SyntaxError message.

https://fedorahosted.org/freeipa/ticket/5811
---
 ipaclient/ipachangeconf.py | 6 +-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/ipaclient/ipachangeconf.py b/ipaclient/ipachangeconf.py
index e73f2978cf512fdfe055f349dba80f4d3275c433..683efee5e7faaba7d2c840bb8d80b6a6863ceac9 100644
--- a/ipaclient/ipachangeconf.py
+++ b/ipaclient/ipachangeconf.py
@@ -460,7 +460,11 @@ class IPAChangeConf:
 continue
 
 # Copy anything else as is.
-curopts.append(self.parseLine(line))
+try:
+curopts.append(self.parseLine(line))
+except SyntaxError as e:
+raise SyntaxError('{error} in file {fname}: [{line}]'.format(
+error=e, fname=f.name, line=line))
 
 #Add last section if any
 if len(sectopts) is not 0:
-- 
2.5.5

-- 
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 0093] Enable service authentication indicator management

[Alexander Bokovoy]

On Mon, 30 May 2016, Petr Vobornik wrote:

On 05/27/2016 06:00 PM, Nathaniel McCallum wrote:

Pavel, since we made the change here from a StrEnum to a Str, we need
to update the UI patch accordingly.


How should admin know what to write there intuitively?

Shouldn't Web UI or CLI advertise the indicators supported by IPA? E.g.
CLI in doc string. Web UI might even combine checkboxes (otp, radius)
with textbox.

That would be better, I think. We still need to keep the API with a free
text field but Web UI, of course, should provide some pre-defined
labels.

--
/ 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 0013] Updated ipa-server-install man page for domain-level attribute

[Petr Vobornik]

On 05/20/2016 01:56 PM, Petr Spacek wrote:
> On 20.5.2016 13:21, Abhijeet Kasurde wrote:
>> Hi All,

...

> 
> Thanks but NACK.
> 
> Domain level is not about one server. It defines that all servers in one IPA
> topology behave in some particular way.
> 

There is a proposal in triage:
https://fedorahosted.org/freeipa/ticket/5907 which might and most likely
will obsolete this ticket - #5708.
-- 
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 0093] Enable service authentication indicator management

[Petr Vobornik]

On 05/27/2016 06:00 PM, Nathaniel McCallum wrote:
> Pavel, since we made the change here from a StrEnum to a Str, we need
> to update the UI patch accordingly.

How should admin know what to write there intuitively?

Shouldn't Web UI or CLI advertise the indicators supported by IPA? E.g.
CLI in doc string. Web UI might even combine checkboxes (otp, radius)
with textbox.

> 
> On Fri, 2016-05-27 at 11:55 -0400, Nathaniel McCallum wrote:
>> On Fri, 2016-05-27 at 18:35 +0300, Alexander Bokovoy wrote:
>>> On Fri, 27 May 2016, Nathaniel McCallum wrote:
 All core functionality for authentication indicators has already
 been
 merged. All that is left is the CLI and UI patches. Attached is
 the
 CLI
 patch.

 One outstanding question that I have is how to future-proof this
 patch.
 Right now, we want to only permit two possible values: otp and
 radius.
 So we are using an StrEnum. However, in the future (probably
 after
 krb5-spake) we may want to have per-token custom indicators. That
 means
 that this value will need to become a Str.
>>> PKINIT has already support for AI, so it would be good to add
>>> pkinit
>>> indicator as well. The problem here is that pkinit indicator is not
>>> fixed and can be defined in the krb5.conf.
>>
>> Okay. You've convinced me that we should just make it a string now
>> and
>> be done with it since administrators can already set custom AIs. New
>> patch attached. I think this is ready for merge.
>>
 How do I code this so that we can later do a StrEnum => Str
 transition
 without breaking API?
>>> Maybe just go to Str* right now and make a validation function that
>>> performs the actual check? Once you'd upgrade the validation code
>>> would
>>> change but method signature wouldn't.
>>
>> Since admins can already set custom AIs, there is no reason for a
>> validator. Let's just accept everything.
> 


-- 
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] [PATCHES] 0789-0796 Python 3 fixes for the server part

[Martin Basti]

On 20.05.2016 17:04, Petr Viktorin wrote:

Hello,
Here are some more Python3 patches. Most are pretty routine, but pay
special attention to the first and last patch.


With these patches, running the in-tree test suite gives me the same
errors in Python 2 and Python 3, except:
- test_install – failures in the updater that I haven't investigated yet
- test_ipaserver – test bug (relying on order of values in an LDAP
attribute) and a text/bytes issue in certificate parsing


In the next few months, I'll need to focus less on IPA and more on
Samba, which is a prerequisite for porting the IPA server. So I'll
quickly summarize the current state of the porting effort:

All of FreeIPA's dependencies except Samba are ported to Python 3 (and
packaged in Fedora).
A recent change switched the IPA client to running on Python 3. With the
patches I'm sending now, most of the "single machine" tests are passing.
The install scripts will still need some work, as will the server parts
that aren't shared with the client.


I'd like to ask the IPA team to sometimes take a look at the Python 3
tests, and try to avoid too many regressions.





ACK, pushed

master:
* 36094b2a542a9472506034dc6c86a573e95c71de ipaldap: Keep attribute names 
as text, not bytes
* 75d0a73bbc426e9b6cabc38f2e932d701b25a762 ipapython.secrets.kem: Use 
ConfigParser from six.moves
* 9477cddfeb9adc03e039155ce3b8a0b560f71098 test_topology_plugin: Don't 
rely on order of an attribute's values
* 9ca450ac436344afd3a46d9852d8329a2e9a48d2 test_rpcserver: Expect 
updated error message under Python 3
* 743828b0f47ca4934125cfb8dc57f79283b95a4d ipaplatform.redhat: Use 
bytestrings when calling rpm.so for version comparison
* 25560f0e1db0c432de35a2496d9c1f8e63dc3dfd test_ipaserver.test_ldap: Use 
bytestrings for raw LDAP values
* c192c1ae3e080597995b906d93cc7607ffc605c0 ipaldap: Convert dict items 
to list before iterating
* 037eae26d0cd8467d3a559bb4cc585c61b626734 test_ipaserver.test_ldap: 
Adjust tests to Python 3's KeyView

ipa-4-3:
* f01b5e506c720181f56e54aa89925a4720642d83 ipaldap: Keep attribute names 
as text, not bytes
* 546b1d0fe6cefd2cbaa3d56f0b1d2939fbdff291 ipapython.secrets.kem: Use 
ConfigParser from six.moves
* 937ebf43745dff7c9894954262bab2d71d9afc71 test_topology_plugin: Don't 
rely on order of an attribute's values
* 74e3fd1d4ae4067caf0d4bda9e962e7c5ceb821c test_rpcserver: Expect 
updated error message under Python 3
* 12e73c95ccd26b5d8156abfddd59ddaca0689a6b ipaplatform.redhat: Use 
bytestrings when calling rpm.so for version comparison
* 3c610bee162f6b420ae91592d90303b662249b7e test_ipaserver.test_ldap: Use 
bytestrings for raw LDAP values
* a78c350589733585b78d6cd7ece63276c2b57634 ipaldap: Convert dict items 
to list before iterating
* 1933e604fb0822bc08caa4aec499be949f731d3b test_ipaserver.test_ldap: 
Adjust tests to Python 3's KeyView


-- 
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 0036] Increased mod_wsgi socket-timeout

[Petr Spacek]

On 28.5.2016 15:59, Martin Basti wrote:
> 
> 
> On 27.05.2016 14:52, Stanislav Laznicka wrote:
>> https://fedorahosted.org/freeipa/ticket/5833
>>
>>
>>
> Is possible to remove timeout completely as it used to be before?
> 
> Even if this timeout is exceeded, command continue in execution and it just
> doesnt print result to user

I agree with Martin. The timeout is pointless, please remove it or set it to
2^31 or so.

-- 
Petr^2 Spacek

-- 
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 0033] Fix CA being presented as running even if it weren't

[Jan Cholasta]

On 30.5.2016 12:36, Martin Basti wrote:



On 26.05.2016 19:31, Stanislav Laznicka wrote:


Self NACK. I should not post patches when tired, sorry. Minor fix is
attached.


On 05/26/2016 07:21 PM, Stanislav Laznicka wrote:

Hello,

Please, see the attached patch. Fixes
https://fedorahosted.org/freeipa/ticket/5898

Standa









LGTM, if nobody is against this, I will push it in 2 days


NACK, please add `wait` argument and call self.wait_until_running(), 
same as in start() and restart().


--
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 0123-132] DNS upgrade: change forwarding policy to "only" if private IPs are used

[Petr Spacek]

On 29.5.2016 14:45, Martin Basti wrote:
> 
> 
> On 27.05.2016 14:12, Petr Spacek wrote:
>> On 25.5.2016 12:50, Martin Basti wrote:
>>>
>>> On 20.05.2016 12:19, Petr Spacek wrote:
 On 11.5.2016 12:08, Martin Basti wrote:
> On 03.05.2016 14:59, Petr Spacek wrote:
>> Hello,
>>
>> DNS upgrade: change forwarding policy to "only" if private IPs are used.
>>
>> https://fedorahosted.org/freeipa/ticket/5710
>>
>> This is the upgrade part. I will add one more patch to print a warning in
>> dnsforwardzone* commands to avoid surprises. Please do not close the 
>> ticket
>> yet.
>>
>>
>>
> 1)
> Upgrade failed with 'BindInstance' object has no attribute
> 'named_conf_get_directive'
> IPA server upgrade failed: Inspect /var/log/ipaupgrade.log and run command
> ipa-server-upgrade manually.
> ('IPA upgrade failed.', 1)
> The ipa-server-upgrade command failed. See /var/log/ipaupgrade.log for 
> more
> information
>
> 2016-05-11T08:26:20Z ERROR Upgrade failed with 'BindInstance' object has 
> no
> attribute 'named_conf_get_directive'
> 2016-05-11T08:26:20Z DEBUG Traceback (most recent call last):
> File
> "/usr/lib/python2.7/site-packages/ipaserver/install/upgradeinstance.py",
> line
> 213, in __upgrade
>   self.modified = (ld.update(self.files) or self.modified)
> File 
> "/usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py",
> line 917, in update
>   self._run_updates(all_updates)
> File 
> "/usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py",
> line 889, in _run_updates
>   self._run_update_plugin(update['plugin'])
> File 
> "/usr/lib/python2.7/site-packages/ipaserver/install/ldapupdate.py",
> line 862, in _run_update_plugin
>   restart_ds, updates = self.api.Updater[plugin_name]()
> File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line
> 1418, in
> __call__
>   return self.execute(**options)
> File
> "/usr/lib/python2.7/site-packages/ipaserver/install/plugins/dns.py",
> line 547, in execute
>   self.update_global_named_conf_forwarder(bind)
> File
> "/usr/lib/python2.7/site-packages/ipaserver/install/plugins/dns.py",
> line 508, in update_global_named_conf_forwarder
>   if bind.named_conf_get_directive(
> AttributeError: 'BindInstance' object has no attribute
> 'named_conf_get_directive'
>
> 2016-05-11T08:26:20Z DEBUG Traceback (most recent call last):
> File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py",
> line
> 447, in start_creation
>   run_step(full_msg, method)
> File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py",
> line
> 437, in run_step
>   method()
> File
> "/usr/lib/python2.7/site-packages/ipaserver/install/upgradeinstance.py",
> line
> 221, in __upgrade
>   raise RuntimeError(e)
> RuntimeError: 'BindInstance' object has no attribute
> 'named_conf_get_directive'
>
> PATCH * Add ipaDNSVersion option to dnsconfig* commands and use new
> attribute *
> 2)
> +Int('ipadnsversion?',
> +label=_('IPA DNS version'),
> +),
>
> Shouldn't be this part of System: Read DNS Configuration permission?
>
> 3)
> -def postprocess_result(self, result):
> +def postprocess_result(self, result, show_version):
>if not any(param in result['result'] for param in self.params):
>result['summary'] = unicode(_('Global DNS configuration is
> empty'))
>
> show_version param was added but I don't see it used in this patch.
>
> 4)
> +Int('ipadnsversion?',
> +label=_('IPA DNS version'),
> +),
>
> Could we add comment here that this option is accessible only from
> installers
> and upgrade?
>
> 5)
> +for config_option in container_entry.get("ipaConfigString", []):
> +matched = re.match("^DNSVersion\s+(?P\d+)$",
> +   config_option, flags=re.I)
> +if matched:
> +version = int(matched.group("version"))
>
> Shouldn't we print error if version cannot be parsed?
>
> PATCH  * DNS upgrade: separate backup logic to make it reusable *
>
> LGTM
>
> PATCH * Add function ipapython.dnsutil.related_to_auto_empty_zone() *
>
> 7)
> I'm curious why do you need to check superdomains?
>
> PATCH * DNS upgrade: change forwarding policy to = only for conflicting
> forward zones*
>
> 8)
> +self.log.debug('Zone %s was sucessfully modified to use '
> +   'forward policy "only"', zone['idnsname'][0])
> 

Re: [Freeipa-devel] [PATCH 0033] Fix CA being presented as running even if it weren't

[Martin Basti]

On 26.05.2016 19:31, Stanislav Laznicka wrote:


Self NACK. I should not post patches when tired, sorry. Minor fix is 
attached.



On 05/26/2016 07:21 PM, Stanislav Laznicka wrote:

Hello,

Please, see the attached patch. Fixes 
https://fedorahosted.org/freeipa/ticket/5898


Standa









LGTM, if nobody is against this, I will push it in 2 days
-- 
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] 0002 Add the culprit line when a configuration file has an incorrect format

[Martin Basti]

On 27.05.2016 11:35, Florence Blanc-Renaud wrote:


Hi all,

this patch adds information to the output of ipa-client-install when 
it fails due to invalid format in a configuration file:
ipa-client-install failing with SyntaxError: Syntax Error: Unknown 
line format


Fixes: https://fedorahosted.org/freeipa/ticket/5811

--
Florence Blanc-Renaud
Identity Management Team, Red Hat


Thank you for your patch, I have just one nitpick. Can you please reuse 
the original exception?


-curopts.append(self.parseLine(line))
+try:
+curopts.append(self.parseLine(line))
+except SyntaxError as e:
+raise SyntaxError('{error} in file {fname}: 
[{line}]'.format(

+error=e, fname=f.name, line=line))

Martin^2
-- 
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] [WIP] Thin client

[Pavel Vomacka]

On 05/30/2016 08:28 AM, Jan Cholasta wrote:

On 28.5.2016 18:33, Pavel Vomacka wrote:



On 05/27/2016 10:33 AM, Petr Spacek wrote:

On 27.5.2016 09:26, Martin Basti wrote:


On 27.05.2016 07:49, Jan Cholasta wrote:

On 26.5.2016 18:43, Martin Basti wrote:


On 26.05.2016 11:21, Martin Basti wrote:


On 26.05.2016 07:15, Jan Cholasta wrote:

On 25.5.2016 18:17, Martin Basti wrote:


On 25.05.2016 16:07, Jan Cholasta wrote:

On 25.5.2016 15:03, David Kupka wrote:

On 18/05/16 08:01, Jan Cholasta wrote:

On 16.5.2016 16:35, Martin Basti wrote:


On 09.05.2016 14:07, Jan Cholasta wrote:

On 6.5.2016 14:32, Martin Basti wrote:


On 28.04.2016 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for 
cross-plugin

imports" should be good for review. The rest is subject to
change
(WARNING: I will force push into this branch).

Honza


I did not test it yet, I just checked the code

* automount: do not inherit automountlocation_tofiles from
LDAPQuery *
LGTM

* dns: move all dnsrecord code called on client to a single
class *
LGTM

* dns: do not rely on server data structures in code 
called on

client *
1)
you forgot to increment VERSION
This was deliberate, as it will no longer be necessary to 
bump

VERSION
for backward compatible changes after this whole patchset is
merged.
But we're not there yet, so fixed.


How we should handle VERSION after your patches?

Otherwise LGTM

* otptoken: fix import of DN *
ACK

* otptoken_yubikey: fix otptoken_add_yubikey arguments *
1)
you forgot to increment VERSION

Fixed.


2)
Did you find out why this was issue?
-del answer['value']# Why does this cause an 
error if

omitted?
 -del answer['summary']  # Why does this cause an
error if
omitted?

The command definition was not complete, it was missing
has_output.


Otherwise LGTM

* vault: move client-side code to the module level *
LGTM

* vault: copy arguments of client commands from server
counterparts *
1)
you forgot to increment Version

Fixed.


* ipalib: use relative imports for cross-plugin imports *
1) Missing explanation for future generations why this 
change is

needed
in commit message

Fixed.


The other commits I will check later.
Martin^2


Okay. Thanks.


* frontend: remove the unused Command.soft_validate method
LGTM

* frontend: perform argument value validation only on server
LGTM

* frontend: do not forward argument defaults to server
I'm not a fan of returning  None in promt_param function, 
but I

havent
found anything better to use.

It always returned None for unset params.


LGTM

* ipalib: optimize API.txt
this contains a lot of black magic, but because this is 
mainly

copy of
current to proper places, LGTM

It's actually mostly cut-n-paste.


* makeaci: load additional plugins using API.add_module
Looks good, but I miss explanation why is this change needed

The next change would be impossible without this.

* plugable: replace API.import_plugins with new 
API.add_package

LGTM


* ipalib, ipaserver: migrate all plugins to Registry-based
registration
ACK

* ipalib, ipaserver: fix incorrect API.register calls in 
docstrings

LGTM


* plugable: remove the unused deprecated API.register method
LGTM


* plugable: switch API to Registry-based plugin discovery
LGTM

* frontend: merge baseldap.CallbackRegistry into Command
LGTM

*frontend: move the interactive_prompt callback type to 
Command

 LGTM

Martin^2





Hello,
first batch of 30 patches from Honza's trac-4739 branch
(https://github.com/jcholast/freeipa/commits/trac-4739) is 
ready

to be
pushed into master.
All upto "frontend: allow commands to have an argument named
`name`" got
over numerous test cycles in last week+ and is working as
expected
now, ACK.

Thanks for the review.


Honzo, please rebase and push them, thanks!


Attaching the patches for reference.

Pushed to master: 2b50fc617024f18a81e97f30f75ed1b9221c4055


Guys, you forgot to test it with newer pylint.

I don't see any "BuildRequires: newer pylint" in the spec file.


Patch attached.
LGTM, although the commit message is wrong - this is not 
related to
thin client at all, PublicError and PublicMessage had .kw since 
forever.



updated commit message






Can I push that fix for pylint?

Sure, ACK.

Pushed to master: aa4123d852d81b908cd18208577bb509fff08c5b



I found something suspicious in tests, did anything in IPA messages
change? I suspect that this is related to client_patches.
Yes, 'data' is a dict which contains structured message data. I 
did not see
these specific failures before, though. Just add it to expected 
results

where missing.

E   AssertionError: assert_deepequal: dict keys 
mismatch.
E 0026: permission_find: Search for u'testperm' 
with a

limit of 1 (truncated) with members
E missing keys = []
E extra keys = [u'data']
E 

Re: [Freeipa-devel] [WIP] Thin client

[Jan Cholasta]

On 28.5.2016 18:33, Pavel Vomacka wrote:



On 05/27/2016 10:33 AM, Petr Spacek wrote:

On 27.5.2016 09:26, Martin Basti wrote:


On 27.05.2016 07:49, Jan Cholasta wrote:

On 26.5.2016 18:43, Martin Basti wrote:


On 26.05.2016 11:21, Martin Basti wrote:


On 26.05.2016 07:15, Jan Cholasta wrote:

On 25.5.2016 18:17, Martin Basti wrote:


On 25.05.2016 16:07, Jan Cholasta wrote:

On 25.5.2016 15:03, David Kupka wrote:

On 18/05/16 08:01, Jan Cholasta wrote:

On 16.5.2016 16:35, Martin Basti wrote:


On 09.05.2016 14:07, Jan Cholasta wrote:

On 6.5.2016 14:32, Martin Basti wrote:


On 28.04.2016 14:45, Jan Cholasta wrote:

Hi,

I have pushed my thin client WIP branch to GitHub:
.

All commits up to "ipalib: use relative imports for cross-plugin
imports" should be good for review. The rest is subject to
change
(WARNING: I will force push into this branch).

Honza


I did not test it yet, I just checked the code

* automount: do not inherit automountlocation_tofiles from
LDAPQuery *
LGTM

* dns: move all dnsrecord code called on client to a single
class *
LGTM

* dns: do not rely on server data structures in code called on
client *
1)
you forgot to increment VERSION

This was deliberate, as it will no longer be necessary to bump
VERSION
for backward compatible changes after this whole patchset is
merged.
But we're not there yet, so fixed.


How we should handle VERSION after your patches?

Otherwise LGTM

* otptoken: fix import of DN *
ACK

* otptoken_yubikey: fix otptoken_add_yubikey arguments *
1)
you forgot to increment VERSION

Fixed.


2)
Did you find out why this was issue?
-del answer['value']# Why does this cause an error if
omitted?
 -del answer['summary']  # Why does this cause an
error if
omitted?

The command definition was not complete, it was missing
has_output.


Otherwise LGTM

* vault: move client-side code to the module level *
LGTM

* vault: copy arguments of client commands from server
counterparts *
1)
you forgot to increment Version

Fixed.


* ipalib: use relative imports for cross-plugin imports *
1) Missing explanation for future generations why this change is
needed
in commit message

Fixed.


The other commits I will check later.
Martin^2


Okay. Thanks.


* frontend: remove the unused Command.soft_validate method
LGTM

* frontend: perform argument value validation only on server
LGTM

* frontend: do not forward argument defaults to server
I'm not a fan of returning  None in promt_param function, but I
havent
found anything better to use.

It always returned None for unset params.


LGTM

* ipalib: optimize API.txt
this contains a lot of black magic, but because this is mainly
copy of
current to proper places, LGTM

It's actually mostly cut-n-paste.


* makeaci: load additional plugins using API.add_module
Looks good, but I miss explanation why is this change needed

The next change would be impossible without this.


* plugable: replace API.import_plugins with new API.add_package
LGTM


* ipalib, ipaserver: migrate all plugins to Registry-based
registration
ACK

* ipalib, ipaserver: fix incorrect API.register calls in docstrings
LGTM


* plugable: remove the unused deprecated API.register method
LGTM


* plugable: switch API to Registry-based plugin discovery
LGTM

* frontend: merge baseldap.CallbackRegistry into Command
LGTM

*frontend: move the interactive_prompt callback type to Command
 LGTM

Martin^2





Hello,
first batch of 30 patches from Honza's trac-4739 branch
(https://github.com/jcholast/freeipa/commits/trac-4739) is ready
to be
pushed into master.
All upto "frontend: allow commands to have an argument named
`name`" got
over numerous test cycles in last week+ and is working as
expected
now, ACK.

Thanks for the review.


Honzo, please rebase and push them, thanks!


Attaching the patches for reference.

Pushed to master: 2b50fc617024f18a81e97f30f75ed1b9221c4055


Guys, you forgot to test it with newer pylint.

I don't see any "BuildRequires: newer pylint" in the spec file.


Patch attached.

LGTM, although the commit message is wrong - this is not related to
thin client at all, PublicError and PublicMessage had .kw since forever.


updated commit message






Can I push that fix for pylint?

Sure, ACK.

Pushed to master: aa4123d852d81b908cd18208577bb509fff08c5b



I found something suspicious in tests, did anything in IPA messages
change? I suspect that this is related to client_patches.

Yes, 'data' is a dict which contains structured message data. I did not see
these specific failures before, though. Just add it to expected results
where missing.


E   AssertionError: assert_deepequal: dict keys mismatch.
E 0026: permission_find: Search for u'testperm' with a
limit of 1 (truncated) with members
E missing keys = []
E extra keys = [u'data']
E expected = {'message': u'Search result has been
truncated: Configured size