[Freeipa-devel] [PATCH] 0082/0083 Handle NotFound exception when establishing trust
Alexander Bokovoy
abokovoy at redhat.com
Fri Oct 5 13:24:25 UTC 2012
On Fri, 05 Oct 2012, Petr Vobornik wrote:
>On 10/04/2012 05:06 PM, Alexander Bokovoy wrote:
>>
>>Hi,
>>
>>two attached patches attempt to solve
>>https://fedorahosted.org/freeipa/ticket/3103
>>
>>We cannot make educated guess where trusted domain's DNS server is
>>located as we ended up with NotFound exception precisely because we were
>>unable to discover trusted domain's domain controller location.
>>
>>Thus, all we can do is to suggest ways to fix the issue. Since
>>suggestion becomes relatively long (multi-line, at least), it creates
>>few issues for current exception error message handling:
>> - root_logger does not use textui to format output
>> - textui is only printing to standard output
>> - multi-line error messages thus become non-wrapped
>> - localizing such messages would become a harder context-wise.
>>
>>Web UI is showing error message as a single paragraph (<p/>), all
>>multi-line markup would be lost.
>>
>>In the end, I decided to support list-based multi-line error messages in
>>PublicError class. Its constructor will join all list-based arguments
>>into single string per argument (first patch).
>>
>>In Web UI I've added special text processing of error messages that
>>splits message into multiple lines and wraps those which start with a
>>TAB symbol into 'error-message-hinted' span to visually separate it from
>>the rest of error message. Trust code uses this to display suggested CLI
>>command to set up DNS forwarder (second patch).
>>
>>In the end, CLI shows such error message fine (note tabulated CLI command):
>>-----------------------------------------------------------------------
>># ipa trust-add --type=ad --admin Administrator at ad.local1 --password
>>ad.local1
>>Active directory domain administrator's password: ipa: ERROR: Unable to
>>resolve domain controller for 'ad.local1' domain. IPA manages DNS,
>>please configure forwarder to 'ad.local1' domain by using following CLI
>>command. Please replace DNS_SERVER and IP_ADDRESS by name and address of
>>the domain's name server:
>> ipa dnszone-add ad.local1 --name-server=DNS_SERVER
>>--admin-email='hostmaster at ad.local1' --force --forwarder=IP_ADDRESS
>>--forward-policy=only
>>When using Web UI, please create DNS zone for domain 'ad.local1' first
>>and then set forwarder and forward policy
>>-----------------------------------------------------------------------
>>
>>Web UI looks like this: http://abbra.fedorapeople.org/.paste/ui.png
>>
>>
>
>You have undeclared identifier: lines.
>
>I recommend to run `jsl -conf jsl.conf` command in install/ui folder
>to prevent these issues.
Fixed.
>I'm not convinced that "beautify" call should be in command object. I
>would rather see it in error_dialog.
I moved everything to error_dialog and used $() selectors instead of
directly playing with text.
>I'm not sure that all tabbed error text should be red. But I don't
>recall any other usage so it's probably OK.
We don't have any, this is first one.
--
/ Alexander Bokovoy
-------------- next part --------------
>From d7a413c0f3d57bbe788951de624f2caaf7065941 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy at redhat.com>
Date: Thu, 4 Oct 2012 15:05:17 +0300
Subject: [PATCH 01/16] support multi-line error messages in exceptions
---
install/ui/ipa.css | 9 ++++++++-
install/ui/ipa.js | 30 ++++++++++++++++++++++++------
ipalib/errors.py | 12 +++++++++---
3 files changed, 41 insertions(+), 10 deletions(-)
diff --git a/install/ui/ipa.css b/install/ui/ipa.css
index c8a220e78430fd38a1781be63421babaadafc948..3146e115524a6672b80e44956710a92bba79af0a 100644
--- a/install/ui/ipa.css
+++ b/install/ui/ipa.css
@@ -1113,6 +1113,13 @@ table.kerberos-key-status {
background-color: #daa520;
}
+.error-message-hinted {
+ color: red;
+ padding-top: 0.5em;
+ padding-bottom: 0.5em;
+ font-family: monospace;
+}
+
/* ---- Table ---- */
table.scrollable thead {
@@ -1755,4 +1762,4 @@ form#login {
.choice-header {
font-weight: bold;
-}
\ No newline at end of file
+}
diff --git a/install/ui/ipa.js b/install/ui/ipa.js
index bd25aeae2b4ff53f928b6fbd7429109fa9c55f68..a511d788bd230f33382f0abbd9496f47b027f9a3 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -1419,6 +1419,24 @@ IPA.error_dialog = function(spec) {
that.visible_buttons = spec.visible_buttons || ['retry', 'cancel'];
};
+ that.beautify_message = function(container, message) {
+ var lines = message.split(/\n/g);
+ for(var i=0; i<lines.length; i++) {
+ // multi-lined text may contain TAB character as first char of the line
+ // to hint at marking the whole line differently
+ if (lines[i].charAt(0) == '\t') {
+ var line_span = $('<span />', {
+ class: 'error-message-hinted',
+ text: lines[i].substr(1)
+ }).appendTo(container);
+ } else {
+ var line_span = $('<p />', {
+ text: lines[i]
+ }).appendTo(container);
+ }
+ }
+ };
+
that.create = function() {
if (that.error_thrown.url) {
$('<p/>', {
@@ -1426,9 +1444,9 @@ IPA.error_dialog = function(spec) {
}).appendTo(that.container);
}
- $('<p/>', {
- html: that.error_thrown.message
- }).appendTo(that.container);
+ var error_message = $('<span />', {});
+ that.beautify_message(error_message, that.error_thrown.message);
+ error_message.appendTo(that.container);
if(that.errors && that.errors.length > 0) {
//render errors
@@ -1457,9 +1475,9 @@ IPA.error_dialog = function(spec) {
for(var i=0; i < that.errors.length; i++) {
var error = that.errors[i];
if(error.message) {
- var error_div = $('<li />', {
- text: error.message
- }).appendTo(errors_container);
+ var error_div = $('<li />', {});
+ that.beautify_message(error_div, error.message);
+ error_div.appendTo(errors_container);
}
}
diff --git a/ipalib/errors.py b/ipalib/errors.py
index 7bf267290b484ed9570f1c7efdb606628fb5f11d..7f1113200b6b2cd80ea9156d347211e4d978b9be 100644
--- a/ipalib/errors.py
+++ b/ipalib/errors.py
@@ -265,11 +265,17 @@ class PublicError(StandardError):
)
self.format = format
self.forwarded = False
- self.msg = self.format % kw
+ def convert_keyword(value):
+ if isinstance(value, list):
+ result=u'\n'.join(map(lambda line: unicode(line), value))
+ return result
+ return value
+ kwargs = dict(map(lambda (k,v): (k, convert_keyword(v)), kw.items()))
+ self.msg = self.format % kwargs
if isinstance(self.format, basestring):
- self.strerror = ugettext(self.format) % kw
+ self.strerror = ugettext(self.format) % kwargs
else:
- self.strerror = self.format % kw
+ self.strerror = self.format % kwargs
else:
if isinstance(message, (Gettext, NGettext)):
message = unicode(message)
--
1.7.12
-------------- next part --------------
>From 3189272314588738994b7b01f2aa3984aed169b2 Mon Sep 17 00:00:00 2001
From: Alexander Bokovoy <abokovoy at redhat.com>
Date: Thu, 4 Oct 2012 17:40:05 +0300
Subject: [PATCH 02/16] Handle NotFound exception when establishing trust
Establishing trust implies discovery of the trusted domain's domain controller via DNS.
If DNS discovery is not possible, NotFound exception is raised.
Intercept the exception and process it to help diagnose and fix actual problem:
- if IPA is managing DNS, suggest to make a forward for the domain's zone
- otherwise suggest to setup DNS forwarder at upstream DNS server
https://fedorahosted.org/freeipa/ticket/3103
---
ipalib/plugins/trust.py | 37 ++++++++++++++++++++++++++++++++++---
1 file changed, 34 insertions(+), 3 deletions(-)
diff --git a/ipalib/plugins/trust.py b/ipalib/plugins/trust.py
index 9d3e9a873e8f6335c12729e9f9475e59499fb3d4..793ad90c0f90dfc4bc3fbaa3d9184b301a44aa0f 100644
--- a/ipalib/plugins/trust.py
+++ b/ipalib/plugins/trust.py
@@ -18,6 +18,7 @@
# along with this program. If not, see <http://www.gnu.org/licenses/>.
from ipalib.plugins.baseldap import *
+from ipalib.plugins.dns import dns_container_exists
from ipalib import api, Str, StrEnum, Password, DefaultFrom, _, ngettext, Object
from ipalib.parameters import Enum
from ipalib import Command
@@ -325,10 +326,39 @@ class trust_add(LDAPCreate):
raise errors.ValidationError(name=_('AD Trust setup'), error=_('Realm administrator password should be specified'))
realm_passwd = options['realm_passwd']
- result = trustinstance.join_ad_full_credentials(keys[-1], realm_server, realm_admin, realm_passwd)
+ try:
+ result = trustinstance.join_ad_full_credentials(keys[-1], realm_server, realm_admin, realm_passwd)
+ except errors.NotFound, e:
+ error_message=[_("Unable to resolve domain controller for '%s' domain. ") % (keys[-1])]
+ if dns_container_exists(self.obj.backend):
+ try:
+ dns_zone = api.Command.dnszone_show(keys[-1])['result']
+ if ('idnsforwardpolicy' in dns_zone) and dns_zone['idnsforwardpolicy'][0] == u'only':
+ error_message.append(_("Forward policy is defined for it in IPA DNS, "
+ "perhaps forwarder points to incorrect host?"))
+ except (errors.NotFound, KeyError) as e:
+ error_message.append(_("IPA manages DNS, please configure forwarder to "
+ "'%(domain)s' domain using following CLI command. "
+ "Make sure to replace DNS_SERVER and IP_ADDRESS by "
+ "actual values corresponding to the trusted domain's "
+ "DNS server:") % dict(domain=keys[-1]))
+ # tab character at the beginning of a multiline error message will be replaced
+ # in the web UI by a colorful hint. Does not affect CLI.
+ error_message.append(_("\tipa dnszone-add %(domain)s --name-server=[DNS_SERVER] "
+ "--admin-email='hostmaster@%(domain)s' "
+ "--force --forwarder=[IP_ADDRESS] "
+ "--forward-policy=only") % dict(domain=keys[-1]))
+ error_message.append(_("When using Web UI, please create DNS zone for domain '%(domain)s' "
+ "first and then set forwarder and forward policy.") % dict(domain=keys[-1]))
+ else:
+ error_message.append(_("Since IPA does not manage DNS records, ensure DNS "
+ "is configured to resolve '%(domain)s' domain from "
+ "IPA hosts and back.") % dict(domain=keys[-1]))
+ raise errors.NotFound(reason=error_message)
if result is None:
- raise errors.ValidationError(name=_('AD Trust setup'), error=_('Unable to verify write permissions to the AD'))
+ raise errors.ValidationError(name=_('AD Trust setup'),
+ error=_('Unable to verify write permissions to the AD'))
return dict(value=trustinstance.remote_domain.info['dns_domain'], verified=result['verified'])
@@ -338,7 +368,8 @@ class trust_add(LDAPCreate):
if 'trust_secret' in options:
result = trustinstance.join_ad_ipa_half(keys[-1], realm_server, options['trust_secret'])
return dict(value=trustinstance.remote_domain.info['dns_domain'], verified=result['verified'])
- raise errors.ValidationError(name=_('AD Trust setup'), error=_('Not enough arguments specified to perform trust setup'))
+ raise errors.ValidationError(name=_('AD Trust setup'),
+ error=_('Not enough arguments specified to perform trust setup'))
class trust_del(LDAPDelete):
__doc__ = _('Delete a trust.')
--
1.7.12
More information about the Freeipa-devel
mailing list