From e0275abe6f5192e68b7f57acf37b01aaa89003ea Mon Sep 17 00:00:00 2001 From: Jan Cholasta Date: May 25 2016 14:06:26 +0000 Subject: rpc: include structured error information in responses Include keyword arguments of exceptions in RPC responses. This is limited to JSON-RPC, as XML-RPC does not support additional data in error responses. Include keyword arguments of messages in RPC responses. Include keyword arguments of exceptions in batch command result. https://fedorahosted.org/freeipa/ticket/4739 Reviewed-By: David Kupka --- diff --git a/ipalib/messages.py b/ipalib/messages.py index c37cceb..c760e9d 100644 --- a/ipalib/messages.py +++ b/ipalib/messages.py @@ -49,6 +49,12 @@ def add_message(version, result, message): def process_message_arguments(obj, format=None, message=None, **kw): + for key, value in kw.items(): + if not isinstance(value, six.integer_types): + try: + kw[key] = unicode(value) + except UnicodeError: + pass obj.kw = kw name = obj.__class__.__name__ if obj.format is not None and format is not None: @@ -120,6 +126,7 @@ class PublicMessage(UserWarning): name=unicode(type(self).__name__), message=self.strerror, code=self.errno, + data=self.kw, ) if six.PY3: diff --git a/ipalib/plugins/batch.py b/ipalib/plugins/batch.py index 42edba2..6a5b383 100644 --- a/ipalib/plugins/batch.py +++ b/ipalib/plugins/batch.py @@ -136,6 +136,7 @@ class batch(Command): error=reported_error.strerror, error_code=reported_error.errno, error_name=unicode(type(reported_error).__name__), + error_kw=reported_error.kw, ) results.append(result) return dict(count=len(results) , results=results) diff --git a/ipalib/rpc.py b/ipalib/rpc.py index 55b8abb..16db331 100644 --- a/ipalib/rpc.py +++ b/ipalib/rpc.py @@ -1097,7 +1097,9 @@ class JSONServerProxy(object): server=self.__host, ) else: - raise error_class(message=error['message']) + kw = error.get('data', {}) + kw['message'] = error['message'] + raise error_class(**kw) return response['result'] diff --git a/ipaserver/rpcserver.py b/ipaserver/rpcserver.py index df64736..59135f2 100644 --- a/ipaserver/rpcserver.py +++ b/ipaserver/rpcserver.py @@ -457,6 +457,7 @@ class jsonserver(WSGIExecutioner, HTTP_Status): error = dict( code=error.errno, message=error.strerror, + data=error.kw, name=unicode(error.__class__.__name__), ) principal = getattr(context, 'principal', 'UNKNOWN') diff --git a/ipatests/test_xmlrpc/test_batch_plugin.py b/ipatests/test_xmlrpc/test_batch_plugin.py index 92b4af5..c2d108f 100644 --- a/ipatests/test_xmlrpc/test_batch_plugin.py +++ b/ipatests/test_xmlrpc/test_batch_plugin.py @@ -128,11 +128,17 @@ class test_batch(Declarative): error=u'%s: group not found' % group1, error_name=u'NotFound', error_code=4001, + error_kw=dict( + reason=u'%s: group not found' % group1, + ), ), dict( error=u'%s: group not found' % group1, error_name=u'NotFound', error_code=4001, + error_kw=dict( + reason=u'%s: group not found' % group1, + ), ), ], ), @@ -152,6 +158,9 @@ class test_batch(Declarative): error=u'%s: group not found' % group1, error_name=u'NotFound', error_code=4001, + error_kw=dict( + reason=u'%s: group not found' % group1, + ), ), dict( value=group1, @@ -198,36 +207,58 @@ class test_batch(Declarative): error=u"unknown command 'nonexistent_ipa_command'", error_name=u'CommandError', error_code=905, + error_kw=dict( + name=u'nonexistent_ipa_command', + ), ), dict( error=u"unknown command 'user-del'", error_name=u'CommandError', error_code=905, + error_kw=dict( + name=u'user-del', + ), ), dict( error=u"'method' is required", error_name=u'RequirementError', error_code=3007, + error_kw=dict( + name=u'method', + ), ), dict( error=u"'params' is required", error_name=u'RequirementError', error_code=3007, + error_kw=dict( + name=u'params', + ), ), dict( error=u"'givenname' is required", error_name=u'RequirementError', error_code=3007, + error_kw=dict( + name=u'givenname', + ), ), dict( error=u"'sn' is required", error_name=u'RequirementError', error_code=3007, + error_kw=dict( + name=u'sn', + ), ), dict( error=Fuzzy(u"invalid 'gid'.*"), error_name=u'ConversionError', error_code=3008, + error_kw=dict( + name=u'gid', + error=Fuzzy(u'.*'), + ), ), ), ), diff --git a/ipatests/test_xmlrpc/test_dns_plugin.py b/ipatests/test_xmlrpc/test_dns_plugin.py index eed8160..fb98d0e 100644 --- a/ipatests/test_xmlrpc/test_dns_plugin.py +++ b/ipatests/test_xmlrpc/test_dns_plugin.py @@ -592,7 +592,14 @@ class test_dns(Declarative): u"apex - '@'. ", 'code': 13005, 'type': u'warning', - 'name': u'OptionSemanticChangedWarning'}, + 'name': u'OptionSemanticChangedWarning', + 'data': { + 'current_behavior': u"It is used only for setting the " + u"SOA MNAME attribute.", + 'hint': u"NS record(s) can be edited in zone apex - " + u"'@'. ", + 'label': u"setting Authoritative nameserver" + }}, ) }, ), @@ -1759,7 +1766,11 @@ class test_dns(Declarative): u"DNS server %s: query '. SOA':" % fwd_ip), u'code': 13006, u'type':u'warning', - u'name': u'DNSServerValidationWarning'}, + u'name': u'DNSServerValidationWarning', + u'data': { + u'error': lambda x: x.startswith(u"query '. SOA':"), + u'server': u"%s" % fwd_ip + }}, ), 'result': { 'idnsforwarders': [fwd_ip], @@ -2898,7 +2909,15 @@ class test_dns(Declarative): u"on a randomly chosen IPA server.", 'code': 13015, 'type': u'warning', - 'name': u'CommandDeprecatedWarning' + 'name': u'CommandDeprecatedWarning', + 'data': { + 'command': u"dns-resolve", + 'additional_info': u"The command may return an " + u"unexpected result, the " + u"resolution of the DNS domain " + u"is done on a randomly chosen " + u"IPA server." + } },) }, ), @@ -2918,7 +2937,15 @@ class test_dns(Declarative): u"on a randomly chosen IPA server.", 'code': 13015, 'type': u'warning', - 'name': u'CommandDeprecatedWarning' + 'name': u'CommandDeprecatedWarning', + 'data': { + 'command': u"dns-resolve", + 'additional_info': u"The command may return an " + u"unexpected result, the " + u"resolution of the DNS domain " + u"is done on a randomly chosen " + u"IPA server." + } },) }, ), @@ -3415,7 +3442,12 @@ class test_forward_zones(Declarative): (forwarder1, fwzone2)), u'code': 13006, u'type':u'warning', - u'name': u'DNSServerValidationWarning'}, + u'name': u'DNSServerValidationWarning', + u'data': { + u'error': lambda x: x.startswith( + u"query '%s SOA':" % forwarder1), + u'server': u"%s" % fwzone2 + }}, ), 'result': { 'dn': fwzone2_dn, @@ -4871,7 +4903,12 @@ class test_forwardzone_delegation_warnings(Declarative): u'"fw.sub" to parent zone "dnszone.test.".', 'code': 13008, 'type': u'warning', - 'name': u'ForwardzoneIsNotEffectiveWarning'}, + 'name': u'ForwardzoneIsNotEffectiveWarning', + 'data': { + 'authzone': zone1_absolute, + 'fwzone': zone1_sub_fw, + 'ns_rec': zone1_sub_fw[:-len(zone1_absolute) - 1] + }}, ), }, ), @@ -4912,7 +4949,12 @@ class test_forwardzone_delegation_warnings(Declarative): u'"fw" to parent zone "sub.dnszone.test.".', 'code': 13008, 'type': u'warning', - 'name': u'ForwardzoneIsNotEffectiveWarning'}, + 'name': u'ForwardzoneIsNotEffectiveWarning', + 'data': { + 'authzone': zone1_sub, + 'fwzone': zone1_sub_fw, + 'ns_rec': zone1_sub_fw[:-len(zone1_sub) - 1] + }}, ), }, ), @@ -4936,7 +4978,12 @@ class test_forwardzone_delegation_warnings(Declarative): u'"fw.sub" to parent zone "dnszone.test.".', 'code': 13008, 'type': u'warning', - 'name': u'ForwardzoneIsNotEffectiveWarning'}, + 'name': u'ForwardzoneIsNotEffectiveWarning', + 'data': { + 'authzone': zone1_absolute, + 'fwzone': zone1_sub_fw, + 'ns_rec': zone1_sub_fw[:-len(zone1_absolute) - 1] + }}, ), }, ), @@ -4960,7 +5007,12 @@ class test_forwardzone_delegation_warnings(Declarative): u'"fw" to parent zone "sub.dnszone.test.".', 'code': 13008, 'type': u'warning', - 'name': u'ForwardzoneIsNotEffectiveWarning'}, + 'name': u'ForwardzoneIsNotEffectiveWarning', + 'data': { + 'authzone': zone1_sub, + 'fwzone': zone1_sub_fw, + 'ns_rec': zone1_sub_fw[:-len(zone1_sub) - 1] + }}, ), }, ), @@ -4997,7 +5049,12 @@ class test_forwardzone_delegation_warnings(Declarative): u'"fw" to parent zone "sub.dnszone.test.".', 'code': 13008, 'type': u'warning', - 'name': u'ForwardzoneIsNotEffectiveWarning'}, + 'name': u'ForwardzoneIsNotEffectiveWarning', + 'data': { + 'authzone': zone1_sub, + 'fwzone': zone1_sub_fw, + 'ns_rec': zone1_sub_fw[:-len(zone1_sub) - 1] + }}, ), }, ), @@ -5039,7 +5096,12 @@ class test_forwardzone_delegation_warnings(Declarative): u'"fw.sub" to parent zone "dnszone.test.".', 'code': 13008, 'type': u'warning', - 'name': u'ForwardzoneIsNotEffectiveWarning'}, + 'name': u'ForwardzoneIsNotEffectiveWarning', + 'data': { + 'authzone': zone1_absolute, + 'fwzone': zone1_sub_fw, + 'ns_rec': zone1_sub_fw[:-len(zone1_absolute) - 1] + }}, ), }, ), @@ -5078,7 +5140,12 @@ class test_forwardzone_delegation_warnings(Declarative): u'"fw" to parent zone "sub.dnszone.test.".', 'code': 13008, 'type': u'warning', - 'name': u'ForwardzoneIsNotEffectiveWarning'}, + 'name': u'ForwardzoneIsNotEffectiveWarning', + 'data': { + 'authzone': zone1_sub, + 'fwzone': zone1_sub_fw, + 'ns_rec': zone1_sub_fw[:-len(zone1_sub) - 1] + }}, ), }, ), @@ -5110,7 +5177,12 @@ class test_forwardzone_delegation_warnings(Declarative): u'"sub.dnszone.test.".', 'code': 13008, 'type': u'warning', - 'name': u'ForwardzoneIsNotEffectiveWarning'}, + 'name': u'ForwardzoneIsNotEffectiveWarning', + 'data': { + 'authzone': zone1_sub, + 'fwzone': zone1_sub2_fw, + 'ns_rec': zone1_sub2_fw[:-len(zone1_sub) - 1] + }}, ), }, ), @@ -5176,7 +5248,12 @@ class test_forwardzone_delegation_warnings(Declarative): u'"fw.sub" to parent zone "dnszone.test.".', 'code': 13008, 'type': u'warning', - 'name': u'ForwardzoneIsNotEffectiveWarning'}, + 'name': u'ForwardzoneIsNotEffectiveWarning', + 'data': { + 'authzone': zone1_absolute, + 'fwzone': zone1_sub_fw, + 'ns_rec': zone1_sub_fw[:-len(zone1_absolute) - 1] + }}, {'message': u'forward zone "fw.sub2.sub.dnszone.test." ' u'is not effective because of missing proper ' u'NS delegation in authoritative zone ' @@ -5185,7 +5262,12 @@ class test_forwardzone_delegation_warnings(Declarative): u'"dnszone.test.".', 'code': 13008, 'type': u'warning', - 'name': u'ForwardzoneIsNotEffectiveWarning'} + 'name': u'ForwardzoneIsNotEffectiveWarning', + 'data': { + 'authzone': zone1_absolute, + 'fwzone': zone1_sub2_fw, + 'ns_rec': zone1_sub2_fw[:-len(zone1_absolute) - 1] + }} ), }, ), @@ -5230,7 +5312,12 @@ class test_forwardzone_delegation_warnings(Declarative): u'"dnszone.test.".', 'code': 13008, 'type': u'warning', - 'name': u'ForwardzoneIsNotEffectiveWarning'}, + 'name': u'ForwardzoneIsNotEffectiveWarning', + 'data': { + 'authzone': zone1_absolute, + 'fwzone': zone1_sub2_fw, + 'ns_rec': zone1_sub2_fw[:-len(zone1_absolute) - 1] + }}, ), }, ), @@ -5478,6 +5565,13 @@ class test_dns_soa(Declarative): 'code': 13005, 'type': u'warning', 'name': u'OptionSemanticChangedWarning', + 'data': { + 'current_behavior': u"It is used only for setting the " + u"SOA MNAME attribute.", + 'hint': u"NS record(s) can be edited in zone apex - " + u"'@'. ", + 'label': u"setting Authoritative nameserver", + }, }], }, ), @@ -5533,6 +5627,13 @@ class test_dns_soa(Declarative): 'code': 13005, 'type': u'warning', 'name': u'OptionSemanticChangedWarning', + 'data': { + 'current_behavior': u"It is used only for setting the " + u"SOA MNAME attribute.", + 'hint': u"NS record(s) can be edited in zone apex - " + u"'@'. ", + 'label': u"setting Authoritative nameserver", + }, }], }, ), @@ -5574,6 +5675,13 @@ class test_dns_soa(Declarative): 'code': 13005, 'type': u'warning', 'name': u'OptionSemanticChangedWarning', + 'data': { + 'current_behavior': u"It is used only for setting the " + u"SOA MNAME attribute.", + 'hint': u"NS record(s) can be edited in zone apex - " + u"'@'. ", + 'label': u"setting Authoritative nameserver", + }, }], }, ), @@ -5615,6 +5723,13 @@ class test_dns_soa(Declarative): 'code': 13005, 'type': u'warning', 'name': u'OptionSemanticChangedWarning', + 'data': { + 'current_behavior': u"It is used only for setting the " + u"SOA MNAME attribute.", + 'hint': u"NS record(s) can be edited in zone apex - " + u"'@'. ", + 'label': u"setting Authoritative nameserver", + }, }], }, ), @@ -5818,6 +5933,13 @@ class test_dns_soa(Declarative): 'code': 13005, 'type': u'warning', 'name': u'OptionSemanticChangedWarning', + 'data': { + 'current_behavior': u"It is used only for setting the " + u"SOA MNAME attribute.", + 'hint': u"NS record(s) can be edited in zone apex - " + u"'@'. ", + 'label': u"setting Authoritative nameserver", + }, }], }, ), dict( @@ -5874,6 +5996,13 @@ class test_dns_soa(Declarative): 'code': 13005, 'type': u'warning', 'name': u'OptionSemanticChangedWarning', + 'data': { + 'current_behavior': u"It is used only for setting the " + u"SOA MNAME attribute.", + 'hint': u"NS record(s) can be edited in zone apex - " + u"'@'. ", + 'label': u"setting Authoritative nameserver", + }, }], }, ), diff --git a/ipatests/test_xmlrpc/test_dns_realmdomains_integration.py b/ipatests/test_xmlrpc/test_dns_realmdomains_integration.py index f284e13..9482ef6 100644 --- a/ipatests/test_xmlrpc/test_dns_realmdomains_integration.py +++ b/ipatests/test_xmlrpc/test_dns_realmdomains_integration.py @@ -154,7 +154,8 @@ class test_dns_realmdomains_integration(Declarative): u'the docs.', u'code': 13002, u'type': u'warning', - u'name': u'ForwardersWarning' + u'name': u'ForwardersWarning', + u'data': {} },), 'result': { 'dn': dnszone_2_dn, diff --git a/ipatests/test_xmlrpc/test_old_permission_plugin.py b/ipatests/test_xmlrpc/test_old_permission_plugin.py index 1af76f6..3e086b5 100644 --- a/ipatests/test_xmlrpc/test_old_permission_plugin.py +++ b/ipatests/test_xmlrpc/test_old_permission_plugin.py @@ -529,7 +529,10 @@ class test_old_permission(Declarative): u'Configured size limit exceeded'), 'code': 13017, 'type': u'warning', - 'name': u'SearchResultTruncated' + 'name': u'SearchResultTruncated', + 'data': { + 'reason': u"Configured size limit exceeded" + } },), ), ), @@ -589,7 +592,10 @@ class test_old_permission(Declarative): u'Configured size limit exceeded'), 'code': 13017, 'type': u'warning', - 'name': u'SearchResultTruncated' + 'name': u'SearchResultTruncated', + 'data': { + 'reason': u"Configured size limit exceeded" + } },), ), ), diff --git a/ipatests/test_xmlrpc/test_permission_plugin.py b/ipatests/test_xmlrpc/test_permission_plugin.py index df5498a..243ea44 100644 --- a/ipatests/test_xmlrpc/test_permission_plugin.py +++ b/ipatests/test_xmlrpc/test_permission_plugin.py @@ -822,7 +822,10 @@ class test_permission(Declarative): u'Configured size limit exceeded'), 'code': 13017, 'type': u'warning', - 'name': u'SearchResultTruncated' + 'name': u'SearchResultTruncated', + 'data': { + 'reason': u"Configured size limit exceeded" + } }, ), ), @@ -886,7 +889,10 @@ class test_permission(Declarative): u'Configured size limit exceeded'), 'code': 13017, 'type': u'warning', - 'name': u'SearchResultTruncated' + 'name': u'SearchResultTruncated', + 'data': { + 'reason': u"Configured size limit exceeded" + } }, ), ), diff --git a/ipatests/test_xmlrpc/test_realmdomains_plugin.py b/ipatests/test_xmlrpc/test_realmdomains_plugin.py index cd35729..dfc3351 100644 --- a/ipatests/test_xmlrpc/test_realmdomains_plugin.py +++ b/ipatests/test_xmlrpc/test_realmdomains_plugin.py @@ -106,7 +106,13 @@ class test_realmdomains(Declarative): "'%s'" % (new_domain_1, api.env.realm), u'code': 13011, u'type': u'warning', - u'name': u'KerberosTXTRecordCreationFailure'}, + u'name': u'KerberosTXTRecordCreationFailure', + u'data': { + u'domain': new_domain_1, + u'realm': api.env.realm, + u'error': (u"%s.: DNS zone not found" % + new_domain_1), + }}, ), result=dict( associateddomain=[our_domain, new_domain_1], @@ -131,7 +137,13 @@ class test_realmdomains(Declarative): realm=api.env.realm), u'code': 13011, u'type': u'warning', - u'name': u'KerberosTXTRecordCreationFailure'}, + u'name': u'KerberosTXTRecordCreationFailure', + u'data': { + u'domain': new_domain_2, + u'realm': api.env.realm, + u'error': (u"%s.: DNS zone not found" % + new_domain_2), + }}, ), ), ), @@ -151,7 +163,12 @@ class test_realmdomains(Declarative): "manually." % dict(domain=new_domain_2), u'code': 13012, u'type': u'warning', - u'name': u'KerberosTXTRecordDeletionFailure'}, + u'name': u'KerberosTXTRecordDeletionFailure', + u'data': { + u'domain': new_domain_2, + u'error': (u"%s.: DNS zone not found" % + new_domain_2), + }}, ), ), ), @@ -173,7 +190,13 @@ class test_realmdomains(Declarative): realm=api.env.realm), u'code': 13011, u'type': u'warning', - u'name': u'KerberosTXTRecordCreationFailure'}, + u'name': u'KerberosTXTRecordCreationFailure', + u'data': { + u'domain': new_domain_2, + u'realm': api.env.realm, + u'error': (u"%s.: DNS zone not found" % + new_domain_2), + }}, {u'message': u"The _kerberos TXT record from domain " "%(domain)s could not be removed (%(domain)s.: " "DNS zone not found).\nThis can happen if the zone " @@ -181,7 +204,12 @@ class test_realmdomains(Declarative): "manually." % dict(domain=new_domain_1), u'code': 13012, u'type': u'warning', - u'name': u'KerberosTXTRecordDeletionFailure'}, + u'name': u'KerberosTXTRecordDeletionFailure', + u'data': { + u'domain': new_domain_1, + u'error': (u"%s.: DNS zone not found" % + new_domain_1), + }}, ), ), ), @@ -242,7 +270,13 @@ class test_realmdomains(Declarative): realm=api.env.realm), u'code': 13011, u'type': u'warning', - u'name': u'KerberosTXTRecordCreationFailure'}, + u'name': u'KerberosTXTRecordCreationFailure', + u'data': { + u'domain': bad_domain, + u'realm': api.env.realm, + u'error': (u"%s.: DNS zone not found" % + bad_domain), + }}, ), ), ),