[Freeipa-devel] [PATCH] 0082/0083 Handle NotFound exception when establishing trust

Alexander Bokovoy abokovoy at redhat.com
Thu Oct 4 15:06:13 UTC 2012


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


-- 
/ Alexander Bokovoy
-------------- next part --------------
>From 9916c6cf35e93c4ad80b1b426785e908cf34d0db 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 2/3] support multi-line error messages in exceptions

part of fix for https://fedorahosted.org/freeipa/ticket/3103
---
 install/ui/ipa.css |  9 ++++++++-
 install/ui/ipa.js  | 14 +++++++++++++-
 ipalib/errors.py   | 12 +++++++++---
 3 files changed, 30 insertions(+), 5 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..11ca391861c8caa5b7f29facf36afd555dcd12d7 100644
--- a/install/ui/ipa.js
+++ b/install/ui/ipa.js
@@ -762,6 +762,18 @@ IPA.command = function(spec) {
             }
         }
 
+        function beautify_text(text) {
+            lines = text.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') {
+                    lines[i] = '<span class="error-message-hinted">' + lines[i].substr(1) + '</span>';
+                }
+            }
+            return lines.join('<br>\n');
+        }
+
         function success_handler(data, text_status, xhr) {
 
             if (!data) {
@@ -783,7 +795,7 @@ IPA.command = function(spec) {
                 error_handler.call(this, xhr, text_status,  /* error_thrown */ {
                     name: IPA.get_message('errors.ipa_error', 'IPA Error')+' '+data.error.code,
                     code: data.error.code,
-                    message: data.error.message,
+                    message: beautify_text(data.error.message),
                     data: data
                 });
 
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 9fa8b25e21b2a4f240292751ae0ea21eb6e2432f 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 3/3] 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 | 34 +++++++++++++++++++++++++++++++---
 1 file changed, 31 insertions(+), 3 deletions(-)

diff --git a/ipalib/plugins/trust.py b/ipalib/plugins/trust.py
index 9d3e9a873e8f6335c12729e9f9475e59499fb3d4..28e0a7dbb7ae330017d48d290e284d17007e5b00 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,36 @@ 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:") % 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=[DNS_SERVER 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 +365,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