[Freeipa-devel] [PATCH 0017] Improve error message in ipa-replica-manage

[Rob Crittenden]

Tomas Babej wrote:
> On 10/19/2012 09:55 AM, Petr Viktorin wrote:
>> On 10/18/2012 08:01 PM, Rob Crittenden wrote:
>>> Tomas Babej wrote:
>>>> On 10/02/2012 03:55 PM, Rob Crittenden wrote:
>>>>> Tomas Babej wrote:
>>>>>> Hi,
>>>>>>
>>>>>> When executing ipa-replica-manage connect to an unknown or irrelevant
>>>>>> master, we now print a sensible error message informing the user
>>>>>> about this possiblity as well.
>>>>>>
>>>>>> https://fedorahosted.org/freeipa/ticket/3105
>>>>>>
>>>>>> Tomas
>>>>>
>>>>> I put a whole bunch of code into a try/except and this may be catching
>>>>> errors in unexpected ways.
>>>>>
>>>>> I'm not entirely sure right now what we should do, but looking at the
>>>>> code in the try:
>>>>>
>>>>> repl1.conn.getEntry(master1_dn, ldap.SCOPE_BASE)
>>>>> repl1.conn.getEntry(master2_dn, ldap.SCOPE_BASE)
>>>>>
>>>>> We take in replica1 and replica1 as arguments (the default for
>>>>> replica1 is the current host).
>>>>>
>>>>> If either of these raise a NotFound it means there there is no master
>>>>> of that name. Does that mean that the master was deleted? Well,
>>>>> clearly not.
>>>>>
>>>>> A lot has changed since I did this, I may have been relying on a
>>>>> side-effect, or just hadn't tested well-enough.
>>>>>
>>>>> I wonder if we need that message at all. Is "foo" is not an IPA server
>>>>> good enough? It still might be confusing if someone didn't know that
>>>>> "foo" was deleted and it was still running. We could probably verify
>>>>> that it is at least an IPA server by doing similar checking in the
>>>>> client, it all depends on how far we want to take it.
>>>>>
>>>>> rob
>>>>
>>>> I modified the patch. Now if the NotFound error is encountered, we try
>>>> to investigate whether we're trying to connect to an IPA server at all.
>>>> Please see if you have any suggestions.
>>>>
>>>> Tomas
>>>
>>> Getting there, the errors are a lot better.
>>>
>>> Can you modify the 'Connection unsuccessful' message to include the
>>> server we failed to connect to?
>>>
>>> The logger isn't so nice either, I think I'd prefer it if we could
>>> exclude the severity:
>>>
>>> ipa: ERROR: LDAP Error: Connect error: TLS error -8172:Peer's
>>> certificate issuer has been marked as not trusted by the user.
>>> Connection unsuccessful.
>>>
>>> So drop the 'ipa: ERROR: ' part for consistency. TI don't believe we
>>> configure the root logger at all in this tool so it is using the
>>> defaults.
>>
>> It's not very easy to find the right call to configure the logger to
>> drop the "ipa: ERROR:" part:
>> standard_logging_setup(console_format='%(message)s')
>> The function is in ipapython.ipa_log_manager. Hopefully that helps.
>>
> Thanks!
>>> I'd also replace the test for -4 with the constant
>>> ipadiscovery.NOT_IPA_SERVER
>>>
>>> rob
>>>
>>
> I implemented your suggestions. Please have a look at the new patch
> version.
>
> Tomas

Getting a traceback:

# ipa-replica-manage connect otherinstall.example.com
LDAP Error: Connect error: TLS error -8054:You are attempting to import 
a cert with the same issuer/serial as an existing cert, but that is not 
the same cert.
Traceback (most recent call last):
   File "/usr/sbin/ipa-replica-manage", line 862, in <module>
     main()
   File "/usr/sbin/ipa-replica-manage", line 845, in main
     add_link(realm, replica1, replica2, dirman_passwd, options)
   File "/usr/sbin/ipa-replica-manage", line 732, in add_link
     sys.exit("Connection to %s unsuccessful.", replica2)
TypeError: exit expected at most 1 arguments, got 2

I was just going to fix this and push and discovered another thing to 
improve.

Trying to connect to a host not in DNS doesn't return a very nice 
message. It says that the CA wasn't found, which is true in an offbeat 
sort of way I guess.

Here is a quick mock-up to see if a host resolves. Note that we can't 
use ipapython.ipautil.is_host_resolvable() because that only uses DNS.

I whipped this up in 2 seconds just to see how it'd work. The name 
stinks, it should probably go into ipapython/ipautil and the way I'm 
checking in add_link would lead to a lot of duplication if tried in the 
other commands. Probably best to have try to centralize the message and 
sys.exit().

--- ipa-replica-manage  2012-10-23 22:08:36.426435673 -0400
+++ /usr/sbin/ipa-replica-manage        2012-10-23 22:34:36.946371449 -0400
@@ -21,6 +21,7 @@
  import os

  import ldap, re, krbV
+import socket
  import traceback
  from urllib2 import urlparse

@@ -54,6 +55,19 @@
      "list-clean-ruv":(0, 0, "", ""),
  }

+def host_exists(host):
+    """
+    Resolve the host to see if it exists.
+
+    Returns True/False
+    """
+    try:
+        socket.getaddrinfo(host, 80)
+    except socket.gaierror:
+        return False
+    else:
+        return True
+
  def convert_error(exc):
      """
      LDAP exceptions are a dictionary, make them prettier.
@@ -652,6 +666,11 @@

  def add_link(realm, replica1, replica2, dirman_passwd, options):

+    if not host_exists(replica1):
+        sys.exit("unknown host %s" % replica1)
+    if not host_exists(replica2):
+        sys.exit("unknown host %s" % replica2)
+
      if options.winsync:
          if not options.binddn or not options.bindpw or not 
options.cacert or not options.passsync:
              root_logger.error("The arguments --binddn, --bindpw, 
--passsync and --cacert are required to create a winsync agreement")
@@ -729,7 +748,7 @@
                      "but it might be unknown, foreign or previously 
deleted "
                      "one." % replica2)
              else:
-                sys.exit("Connection to %s unsuccessful.", replica2)
+                sys.exit("Connection to %s unsuccessful." % replica2)

          repl1.setup_gssapi_replication(replica2, DN(('cn', 'Directory 
Manager')), dirman_passwd)
      print "Connected '%s' to '%s'" % (replica1, replica2)