On 04/11/2013 09:57 PM, Rob Crittenden wrote:
Tomas Babej wrote:
Hi,

In ipa-replica-manage commands, we enforce that hostnames we work
with are resolvable. However, this caused errors while deleting
or disconnecting a ipa / winsync replica, if that replica was down
and authoritative server for itself.

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

Tomas

I'm not sure this is going to do the right thing either. A lot of these commands take the an argument as the remote master to run things on, so we'd really only be validating one of the names. Not sure how that helps us.

Actually, the patch tried to adress that. I carefully reviewed the effort, now we should be consistent in validating all the names.

What if we honor the --force flag for DNS lookup failures instead? Or, since that could override it and do other things, a --no-lookup flag perhaps?

rob

I added a --no-lookup flag for ipa-replica-manage that disables host existence check.

Sending both patches rebased.

Tomas
From b19018c53036ca556dfefbe3c6310d804b98f95f Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Wed, 10 Apr 2013 13:00:45 +0200
Subject: [PATCH 46/46] Handle connection timeout in ipa-replica-manage

When connecting to replica, ipa-replica-manage could fail with
unknown error due to connection time out. This patch properly
handles the situation
---
 install/tools/ipa-replica-manage | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 796cb5b3e69259b341d8e85ef0f0444a8edda710..d624abeef7eed1c9334841d7df2fa1118b0aef91 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -24,6 +24,7 @@ import re, krbV
 import traceback
 from urllib2 import urlparse
 import ldap
+import socket
 
 from ipapython import ipautil
 from ipaserver.install import replication, dsinstance, installutils
@@ -737,9 +738,17 @@ def add_link(realm, replica1, replica2, dirman_passwd, options):
             root_logger.error("winsync agreements need to be created as root")
             sys.exit(1)
 
-    # See if we already have an agreement with this host
     try:
         repl = replication.ReplicationManager(realm, replica1, dirman_passwd)
+    except errors.NotFound:
+        print "Cannot find replica '%s'" % replica1
+        return
+    except Exception, e:
+        print "Failed to connect to '%s': %s" % (replica1, e)
+        return
+
+    # See if we already have an agreement with this host
+    try:
         if repl.get_agreement_type(replica2) == replication.WINSYNC:
             agreement = repl.get_replication_agreement(replica2)
             sys.exit("winsync agreement already exists on subtree %s" %
@@ -1222,6 +1231,9 @@ except SystemExit, e:
     sys.exit(e)
 except RuntimeError, e:
     sys.exit(e)
+except socket.timeout:
+    print "Connection timed out."
+    sys.exit(1)
 except Exception, e:
     print "unexpected error: %s" % str(e)
     sys.exit(1)
-- 
1.8.1.4

From 520e187f2112e86781476f2fee96403b246e3272 Mon Sep 17 00:00:00 2001
From: Tomas Babej <tba...@redhat.com>
Date: Tue, 9 Apr 2013 13:45:34 +0200
Subject: [PATCH 45/46] Enforce host existence only where needed in
 ipa-replica-manage

In ipa-replica-manage commands, we enforce that hostnames we work
with are resolvable. However, this caused errors while deleting
or disconnecting a ipa / winsync replica, if that replica was down
and authoritative server for itself.

Also adds an --no-lookup flag to disable host existence checks.

https://fedorahosted.org/freeipa/ticket/3524
---
 install/tools/ipa-replica-manage       | 76 ++++++++++++++++++++--------------
 install/tools/man/ipa-replica-manage.1 |  3 ++
 2 files changed, 47 insertions(+), 32 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 636529caaeca222c09d94d3e5539002fcd3139a9..796cb5b3e69259b341d8e85ef0f0444a8edda710 100755
--- a/install/tools/ipa-replica-manage
+++ b/install/tools/ipa-replica-manage
@@ -84,6 +84,8 @@ def parse_options():
     parser.add_option("--passsync", dest="passsync", default=None,
                       help="Password for the IPA system user used by the Windows PassSync plugin to synchronize passwords")
     parser.add_option("--from", dest="fromhost", help="Host to get data from")
+    parser.add_option("--no-lookup", dest="no_lookup", action="store_true", default=False,
+                      help="do not perform DNS lookup checks")
 
     options, args = parser.parse_args()
 
@@ -111,7 +113,7 @@ def parse_options():
 
     return options, args
 
-def test_connection(realm, host):
+def test_connection(realm, host, nolookup=False):
     """
     Make a GSSAPI connection to the remote LDAP server to test out credentials.
 
@@ -120,6 +122,8 @@ def test_connection(realm, host):
     returns True if connection successful, False otherwise
     """
     try:
+        if not nolookup:
+            enforce_host_existence(host)
         replman = replication.ReplicationManager(realm, host, None)
         ents = replman.find_replication_agreements()
         del replman
@@ -134,10 +138,12 @@ def test_connection(realm, host):
         # more than likely a GSSAPI error
         return False
 
-def list_replicas(realm, host, replica, dirman_passwd, verbose):
+def list_replicas(realm, host, replica, dirman_passwd, verbose, nolookup=False):
 
-    for check_host in [host, replica]:
-        enforce_host_existence(check_host)
+    if not nolookup:
+        enforce_host_existence(host)
+        if replica is not None:
+            enforce_host_existence(replica)
 
     is_replica = False
     winsync_peer = None
@@ -232,9 +238,6 @@ def del_link(realm, replica1, replica2, dirman_passwd, force=False):
     @force: force deletion even if one server is down
     """
 
-    for check_host in [replica1, replica2]:
-        enforce_host_existence(check_host)
-
     repl2 = None
 
     try:
@@ -327,12 +330,13 @@ def del_link(realm, replica1, replica2, dirman_passwd, force=False):
 
     return True
 
-def get_ruv(realm, host, dirman_passwd):
+def get_ruv(realm, host, dirman_passwd, nolookup=False):
     """
     Return the RUV entries as a list of tuples: (hostname, rid)
     """
 
-    enforce_host_existence(host)
+    if not nolookup:
+        enforce_host_existence(host)
 
     try:
         thisrepl = replication.ReplicationManager(realm, host, dirman_passwd)
@@ -370,8 +374,6 @@ def list_ruv(realm, host, dirman_passwd, verbose):
     replica IDs.
     """
 
-    enforce_host_existence(host)
-
     servers = get_ruv(realm, host, dirman_passwd)
     for (netloc, rid) in servers:
         print "%s: %s" % (netloc, rid)
@@ -457,12 +459,13 @@ def abort_clean_ruv(realm, ruv, options):
 
     print "Cleanup task stopped"
 
-def list_clean_ruv(realm, host, dirman_passwd, verbose):
+def list_clean_ruv(realm, host, dirman_passwd, verbose, nolookup=False):
     """
     List all clean RUV tasks.
     """
 
-    enforce_host_existence(host)
+    if not nolookup:
+        enforce_host_existence(host)
 
     repl = replication.ReplicationManager(realm, host, dirman_passwd)
     dn = DN(('cn', 'cleanallruv'),('cn', 'tasks'), ('cn', 'config'))
@@ -541,7 +544,7 @@ def check_last_link(delrepl, realm, dirman_passwd, force):
         return None
 
 def enforce_host_existence(host, message=None):
-    if not ipautil.host_exists(host):
+    if host is not None and not ipautil.host_exists(host):
         if message is None:
             message = "Unknown host %s" % host
 
@@ -549,8 +552,6 @@ def enforce_host_existence(host, message=None):
 
 def del_master(realm, hostname, options):
 
-    enforce_host_existence(hostname)
-
     force_del = False
     delrepl = None
 
@@ -724,8 +725,9 @@ def del_master(realm, hostname, options):
 
 def add_link(realm, replica1, replica2, dirman_passwd, options):
 
-    for check_host in [replica1,replica2]:
-        enforce_host_existence(check_host)
+    if not options.no_lookup:
+        for check_host in [replica1,replica2]:
+            enforce_host_existence(check_host)
 
     if options.winsync:
         if not options.binddn or not options.bindpw or not options.cacert or not options.passsync:
@@ -809,10 +811,11 @@ def add_link(realm, replica1, replica2, dirman_passwd, options):
         repl1.setup_gssapi_replication(replica2, DN(('cn', 'Directory Manager')), dirman_passwd)
     print "Connected '%s' to '%s'" % (replica1, replica2)
 
-def re_initialize(realm, thishost, fromhost, dirman_passwd):
+def re_initialize(realm, thishost, fromhost, dirman_passwd, nolookup=False):
 
-    for check_host in [thishost, fromhost]:
-        enforce_host_existence(check_host)
+    if not nolookup:
+        for check_host in [thishost, fromhost]:
+            enforce_host_existence(check_host)
 
     thisrepl = replication.ReplicationManager(realm, thishost, dirman_passwd)
     agreement = thisrepl.get_replication_agreement(fromhost)
@@ -838,10 +841,11 @@ def re_initialize(realm, thishost, fromhost, dirman_passwd):
         ds = dsinstance.DsInstance(realm_name = realm, dm_password = dirman_passwd)
         ds.init_memberof()
 
-def force_sync(realm, thishost, fromhost, dirman_passwd):
+def force_sync(realm, thishost, fromhost, dirman_passwd, nolookup=False):
 
-    for check_host in [thishost, fromhost]:
-        enforce_host_existence(check_host)
+    if not nolookup:
+        for check_host in [thishost, fromhost]:
+            enforce_host_existence(check_host)
 
     thisrepl = replication.ReplicationManager(realm, thishost, dirman_passwd)
     agreement = thisrepl.get_replication_agreement(fromhost)
@@ -856,7 +860,8 @@ def force_sync(realm, thishost, fromhost, dirman_passwd):
         repl = replication.ReplicationManager(realm, fromhost, dirman_passwd)
         repl.force_sync(repl.conn, thishost)
 
-def show_DNA_ranges(hostname, master, realm, dirman_passwd, nextrange=False):
+def show_DNA_ranges(hostname, master, realm, dirman_passwd, nextrange=False,
+                    nolookup=False):
     """
     Display the DNA ranges for all current masters.
 
@@ -868,8 +873,11 @@ def show_DNA_ranges(hostname, master, realm, dirman_passwd, nextrange=False):
 
     Returns nothing
     """
-    for check_host in [hostname, master]:
-        enforce_host_existence(check_host)
+
+    if not nolookup:
+        enforce_host_existence(hostname)
+        if master is not None:
+            enforce_host_existence(master)
 
     try:
         repl = replication.ReplicationManager(realm, hostname, dirman_passwd)
@@ -956,7 +964,8 @@ def store_DNA_range(repl, range_start, range_max, deleted_master, realm,
     return False
 
 
-def set_DNA_range(hostname, range, realm, dirman_passwd, next_range=False):
+def set_DNA_range(hostname, range, realm, dirman_passwd, next_range=False,
+                  nolookup=False):
     """
     Given a DNA range try to change it on the designated master.
 
@@ -1004,7 +1013,8 @@ def set_DNA_range(hostname, range, realm, dirman_passwd, next_range=False):
     def range_intersection(s1, s2, r1, r2):
         return max(s1, r1) <= min(s2, r2)
 
-    enforce_host_existence(hostname)
+    if not nolookup:
+        enforce_host_existence(hostname)
 
     err = validate_range(range, allow_all_zero=next_range)
     if err is not None:
@@ -1137,7 +1147,7 @@ def main():
     if options.dirman_passwd:
         dirman_passwd = options.dirman_passwd
     else:
-        if not test_connection(realm, host):
+        if not test_connection(realm, host, options.no_lookup):
             dirman_passwd = installutils.read_password("Directory Manager",
                 confirm=False, validate=False, retry=False)
             if dirman_passwd is None:
@@ -1149,9 +1159,10 @@ def main():
         replica = None
         if len(args) == 2:
             replica = args[1]
-        list_replicas(realm, host, replica, dirman_passwd, options.verbose)
+        list_replicas(realm, host, replica, dirman_passwd, options.verbose,
+                      options.no_lookup)
     elif args[0] == "list-ruv":
-        list_ruv(realm, host, dirman_passwd, options.verbose)
+        list_ruv(realm, host, dirman_passwd, options.verbose, options.no_lookup)
     elif args[0] == "del":
         del_master(realm, args[1], options)
     elif args[0] == "re-initialize":
@@ -1214,3 +1225,4 @@ except RuntimeError, e:
 except Exception, e:
     print "unexpected error: %s" % str(e)
     sys.exit(1)
+
diff --git a/install/tools/man/ipa-replica-manage.1 b/install/tools/man/ipa-replica-manage.1
index d00101990d1d61c3cf81cf07574a478c005de35f..a981c72f59e23024110e0d9e8331cd50cbb22130 100644
--- a/install/tools/man/ipa-replica-manage.1
+++ b/install/tools/man/ipa-replica-manage.1
@@ -101,6 +101,9 @@ Provide additional information
 \fB\-f\fR, \fB\-\-force\fR
 Ignore some types of errors, don't prompt when deleting a master
 .TP
+\fB\-c\fR, \fB\-\-no\-lookup\fR
+Do not perform DNS lookup checks.
+.TP
 \fB\-c\fR, \fB\-\-cleanup\fR
 When deleting a master with the \-\-force flag, remove leftover references to an already deleted master.
 .TP
-- 
1.8.1.4

_______________________________________________
Freeipa-devel mailing list
Freeipa-devel@redhat.com
https://www.redhat.com/mailman/listinfo/freeipa-devel

Reply via email to