On 04/30/2013 05:24 PM, Petr Viktorin wrote:
On 04/30/2013 02:32 PM, Tomas Babej wrote:
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

Thanks for the review,
The nolookup argument is never passed to get_ruv() when it is called by list_ruv, get_rid_by_host, clean_ruv, abort_clean_ruv. Some of these don't take the argument but are called with it. Lint error: install/tools/ipa-replica-manage:1188: [E1121, main] Too many positional arguments for function call

Fixed.
nolookup is also not passed to list_clean_ruv(), re_initialize(), force_sync(), show_DNA_ranges() etc.

Fixed.
Git complains about some extra whitespace:
Applying: Enforce host existence only where needed in ipa-replica-manage
/home/pviktori/freeipa/.git/rebase-apply/patch:234: new blank line at EOF.
+
warning: 1 line adds whitespace errors.

Fixed


I added the ticket to the Minor Enhacements page: http://freeipa.org/page/V3_Minor_Enhancements

Tomas
From 43d0cb94eb1848c9dc1a8360ee7431e63b0cab35 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 aad704cd867396e6397d768bffd2cc7009285a7a..f42baf04ed5db8948a9909b157f5df88be50552c 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
@@ -741,9 +742,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" %
@@ -1233,6 +1242,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 21801ebc9d86a2b16cd303a8f9b3ab190c84df74 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       | 118 +++++++++++++++++++--------------
 install/tools/man/ipa-replica-manage.1 |   3 +
 2 files changed, 73 insertions(+), 48 deletions(-)

diff --git a/install/tools/ipa-replica-manage b/install/tools/ipa-replica-manage
index 636529caaeca222c09d94d3e5539002fcd3139a9..aad704cd867396e6397d768bffd2cc7009285a7a 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="nolookup", 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)
@@ -364,23 +368,21 @@ def get_ruv(realm, host, dirman_passwd):
 
     return servers
 
-def list_ruv(realm, host, dirman_passwd, verbose):
+def list_ruv(realm, host, dirman_passwd, verbose, nolookup=False):
     """
     List the Replica Update Vectors on this host to get the available
     replica IDs.
     """
 
-    enforce_host_existence(host)
-
-    servers = get_ruv(realm, host, dirman_passwd)
+    servers = get_ruv(realm, host, dirman_passwd, nolookup)
     for (netloc, rid) in servers:
         print "%s: %s" % (netloc, rid)
 
-def get_rid_by_host(realm, sourcehost, host, dirman_passwd):
+def get_rid_by_host(realm, sourcehost, host, dirman_passwd, nolookup=False):
     """
     Try to determine the RID by host name.
     """
-    servers = get_ruv(realm, sourcehost, dirman_passwd)
+    servers = get_ruv(realm, sourcehost, dirman_passwd, nolookup)
     for (netloc, rid) in servers:
         if '%s:389' % host == netloc:
             return int(rid)
@@ -394,7 +396,8 @@ def clean_ruv(realm, ruv, options):
     except ValueError:
         sys.exit("Replica ID must be an integer: %s" % ruv)
 
-    servers = get_ruv(realm, options.host, options.dirman_passwd)
+    servers = get_ruv(realm, options.host, options.dirman_passwd,
+                      options.nolookup)
     found = False
     for (netloc, rid) in servers:
         if ruv == int(rid):
@@ -427,7 +430,8 @@ def abort_clean_ruv(realm, ruv, options):
     except ValueError:
         sys.exit("Replica ID must be an integer: %s" % ruv)
 
-    servers = get_ruv(realm, options.host, options.dirman_passwd)
+    servers = get_ruv(realm, options.host, options.dirman_passwd,
+                      options.nolookup)
     found = False
     for (netloc, rid) in servers:
         if ruv == int(rid):
@@ -438,7 +442,8 @@ def abort_clean_ruv(realm, ruv, options):
     if not found:
         sys.exit("Replica ID %s not found" % ruv)
 
-    servers = get_ruv(realm, options.host, options.dirman_passwd)
+    servers = get_ruv(realm, options.host, options.dirman_passwd,
+                      options.nolookup)
     found = False
     for (netloc, rid) in servers:
         if ruv == int(rid):
@@ -457,12 +462,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 +547,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 +555,6 @@ def enforce_host_existence(host, message=None):
 
 def del_master(realm, hostname, options):
 
-    enforce_host_existence(hostname)
-
     force_del = False
     delrepl = None
 
@@ -679,7 +683,8 @@ def del_master(realm, hostname, options):
 
     # Save the RID value before we start deleting
     if repltype == replication.IPA_REPLICA:
-        rid = get_rid_by_host(realm, options.host, hostname, options.dirman_passwd)
+        rid = get_rid_by_host(realm, options.host, hostname,
+                              options.dirman_passwd, options.nolookup)
 
     # 4. Remove each agreement
 
@@ -724,8 +729,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.nolookup:
+        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 +815,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 +845,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 +864,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 +877,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)
@@ -915,7 +927,7 @@ def show_DNA_ranges(hostname, master, realm, dirman_passwd, nextrange=False):
 
 
 def store_DNA_range(repl, range_start, range_max, deleted_master, realm,
-                 dirman_passwd):
+                    dirman_passwd):
     """
     Given a DNA range try to save it in a remaining master in the
     on-deck (dnaNextRange) value.
@@ -956,7 +968,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 +1017,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 +1151,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.nolookup):
             dirman_passwd = installutils.read_password("Directory Manager",
                 confirm=False, validate=False, retry=False)
             if dirman_passwd is None:
@@ -1149,21 +1163,24 @@ 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.nolookup)
     elif args[0] == "list-ruv":
-        list_ruv(realm, host, dirman_passwd, options.verbose)
+        list_ruv(realm, host, dirman_passwd, options.verbose, options.nolookup)
     elif args[0] == "del":
         del_master(realm, args[1], options)
     elif args[0] == "re-initialize":
         if not options.fromhost:
             print "re-initialize requires the option --from <host name>"
             sys.exit(1)
-        re_initialize(realm, host, options.fromhost, dirman_passwd)
+        re_initialize(realm, host, options.fromhost, dirman_passwd,
+                      options.nolookup)
     elif args[0] == "force-sync":
         if not options.fromhost:
             print "force-sync requires the option --from <host name>"
             sys.exit(1)
-        force_sync(realm, host, options.fromhost, options.dirman_passwd)
+        force_sync(realm, host, options.fromhost, options.dirman_passwd,
+                   options.nolookup)
     elif args[0] == "connect":
         if len(args) == 3:
             replica1 = args[1]
@@ -1185,23 +1202,28 @@ def main():
     elif args[0] == "abort-clean-ruv":
         abort_clean_ruv(realm, args[1], options)
     elif args[0] == "list-clean-ruv":
-        list_clean_ruv(realm, host, dirman_passwd, options.verbose)
+        list_clean_ruv(realm, host, dirman_passwd, options.verbose,
+                       options.nolookup)
     elif args[0] == "dnarange-show":
         if len(args) == 2:
             master = args[1]
         else:
             master = None
-        show_DNA_ranges(host, master, realm, dirman_passwd, False)
+        show_DNA_ranges(host, master, realm, dirman_passwd, False,
+                        options.nolookup)
     elif args[0] == "dnanextrange-show":
         if len(args) == 2:
             master = args[1]
         else:
             master = None
-        show_DNA_ranges(host, master, realm, dirman_passwd, True)
+        show_DNA_ranges(host, master, realm, dirman_passwd, True,
+                        options.nolookup)
     elif args[0] == "dnarange-set":
-        set_DNA_range(args[1], args[2], realm, dirman_passwd, next_range=False)
+        set_DNA_range(args[1], args[2], realm, dirman_passwd, next_range=False,
+                      nolookup=options.nolookup)
     elif args[0] == "dnanextrange-set":
-        set_DNA_range(args[1], args[2], realm, dirman_passwd, next_range=True)
+        set_DNA_range(args[1], args[2], realm, dirman_passwd, next_range=True,
+                      nolookup=options.nolookup)
 
 try:
     main()
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