mbasti-rh's pull request #78: "Fix Ip-addr validation" was synchronize

See the full pull-request at https://github.com/freeipa/freeipa/pull/78
... or pull the PR as Git branch:
git remote add ghfreeipa https://github.com/freeipa/freeipa
git fetch ghfreeipa pull/78/head:pr78
git checkout pr78
From 0ab367a20fbbfbbe9dcc72eeda72ecdc6e5f8be2 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Tue, 13 Sep 2016 16:06:11 +0200
Subject: [PATCH 1/3] Fix missing config.ips in promote_check

When replica is installed with --setup-dns config.ips is not defined.

https://fedorahosted.org/freeipa/ticket/5814
---
 ipaserver/install/server/replicainstall.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 5edc002..5b5485e 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -1324,6 +1324,7 @@ def promote_check(installer):
         if options.setup_dns:
             dns.install_check(False, remote_api, True, options,
                               config.host_name)
+            config.ips = dns.ip_addresses
         else:
             config.ips = installutils.get_server_ip_address(
                 config.host_name, not installer.interactive,

From 6cce6e07a6beffa46e3dbd00eb740bf4b4c77b2b Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Mon, 12 Sep 2016 14:38:12 +0200
Subject: [PATCH 2/3] Abstract procedures for IP address warnings

Originaly there should be only two occurencees of this warning, one for
server, one for client. But obviously is not possible with current
installers to achive this goal, so I have to extract code to not mess
with 5 times copy and paste.

https://fedorahosted.org/freeipa/ticket/5814
---
 client/ipa-client-install                  | 19 +++---------
 ipalib/util.py                             | 27 +++++++++++++++-
 ipaserver/install/server/install.py        | 21 +++++--------
 ipaserver/install/server/replicainstall.py | 49 +++++-------------------------
 4 files changed, 46 insertions(+), 70 deletions(-)

diff --git a/client/ipa-client-install b/client/ipa-client-install
index 6330f1d..535fe65 100755
--- a/client/ipa-client-install
+++ b/client/ipa-client-install
@@ -55,7 +55,9 @@ try:
     from ipalib import api, errors
     from ipalib import x509, certstore
     from ipalib.util import (
-        normalize_hostname, validate_domain_name, verify_host_resolvable)
+        normalize_hostname, validate_domain_name, verify_host_resolvable,
+        network_ip_address_warning, broadcast_ip_address_warning
+    )
     from ipalib.constants import CACERT
     from ipapython.dn import DN
     from ipapython.ssh import SSHPublicKey
@@ -1651,19 +1653,8 @@ def update_dns(server, hostname, options):
         root_logger.info("Failed to determine this machine's ip address(es).")
         return
 
-    for ip in update_ips:
-        if ip.is_network_addr():
-            root_logger.warning("IP address %s might be network address", ip)
-            # fixme: once when loggers will be fixed, we can remove this print
-            print(
-                "WARNING: IP address {} might be network address".format(ip),
-                file=sys.stderr)
-        if ip.is_broadcast_addr():
-            root_logger.warning("IP address %s might be broadcast address", ip)
-            # fixme: once when loggers will be fixed, we can remove this print
-            print(
-                "WARNING: IP address {} might be broadcast address".format(ip),
-                file=sys.stderr)
+    network_ip_address_warning(update_ips)
+    broadcast_ip_address_warning(update_ips)
 
     update_txt = "debug\n"
     update_txt += ipautil.template_str(DELETE_TEMPLATE_A,
diff --git a/ipalib/util.py b/ipalib/util.py
index 8057740..785dd5f 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -21,7 +21,10 @@
 Various utility functions.
 """
 
-from __future__ import absolute_import
+from __future__ import (
+    absolute_import,
+    print_function,
+)
 
 import os
 import socket
@@ -29,6 +32,7 @@
 import decimal
 import dns
 import encodings
+import sys
 from weakref import WeakKeyDictionary
 
 import netaddr
@@ -45,6 +49,7 @@
 from ipapython.dn import DN, RDN
 from ipapython.dnsutil import DNSName
 from ipapython.dnsutil import resolve_ip_addresses
+from ipapython.ipa_log_manager import root_logger
 
 if six.PY3:
     unicode = str
@@ -994,3 +999,23 @@ def check_principal_realm_in_trust_namespace(api_instance, *keys):
                 name='krbprincipalname',
                 error=_('realm or UPN suffix overlaps with trusted domain '
                         'namespace'))
+
+
+def network_ip_address_warning(addr_list):
+    for ip in addr_list:
+        if ip.is_network_addr():
+            root_logger.warning("IP address %s might be network address", ip)
+            # fixme: once when loggers will be fixed, we can remove this
+            # print
+            print("WARNING: IP address {} might be network address".format(ip),
+                  file=sys.stderr)
+
+
+def broadcast_ip_address_warning(addr_list):
+    for ip in addr_list:
+        if ip.is_broadcast_addr():
+            root_logger.warning("IP address %s might be broadcast address", ip)
+            # fixme: once when loggers will be fixed, we can remove this
+            # print
+            print("WARNING: IP address {} might be broadcast address".format(
+                ip), file=sys.stderr)
diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index 2ef9bdb..7733106 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -31,7 +31,11 @@
 from ipaplatform.tasks import tasks
 from ipalib import api, constants, errors, x509
 from ipalib.constants import CACERT
-from ipalib.util import validate_domain_name
+from ipalib.util import (
+    validate_domain_name,
+    network_ip_address_warning,
+    broadcast_ip_address_warning,
+)
 import ipaclient.ntpconf
 from ipaserver.install import (
     bindinstance, ca, cainstance, certs, dns, dsinstance,
@@ -609,19 +613,8 @@ def install_check(installer):
                                              not installer.interactive, False,
                                              options.ip_addresses)
 
-    for ip in ip_addresses:
-        if ip.is_network_addr():
-            root_logger.warning("IP address %s might be network address", ip)
-            # fixme: once when loggers will be fixed, we can remove this print
-            print(
-                "WARNING: IP address {} might be network address".format(ip),
-                file=sys.stderr)
-        if ip.is_broadcast_addr():
-            root_logger.warning("IP address %s might be broadcast address", ip)
-            # fixme: once when loggers will be fixed, we can remove this print
-            print(
-                "WARNING: IP address {} might be broadcast address".format(ip),
-                file=sys.stderr)
+    network_ip_address_warning(ip_addresses)
+    broadcast_ip_address_warning(ip_addresses)
 
     # installer needs to update hosts file when DNS subsystem will be
     # installed or custom addresses are used
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 5b5485e..571b860 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -13,7 +13,6 @@
 import os
 import shutil
 import socket
-import sys
 import tempfile
 
 import six
@@ -28,6 +27,10 @@
 from ipaplatform.tasks import tasks
 from ipaplatform.paths import paths
 from ipalib import api, certstore, constants, create_api, errors, rpc, x509
+from ipalib.util import (
+    network_ip_address_warning,
+    broadcast_ip_address_warning,
+)
 import ipaclient.ipachangeconf
 import ipaclient.ntpconf
 from ipaserver.install import (
@@ -736,27 +739,8 @@ def install_check(installer):
                 config.host_name, not installer.interactive, False,
                 options.ip_addresses)
 
-        for ip in config.ips:
-            if ip.is_network_addr():
-                root_logger.warning(
-                    "IP address %s might be network address", ip)
-                # fixme: once when loggers will be fixed, we can remove this
-                # print
-                print(
-                    "WARNING: IP address {} might be network address".format(
-                        ip),
-                    file=sys.stderr
-                )
-            if ip.is_broadcast_addr():
-                root_logger.warning(
-                    "IP address %s might be broadcast address", ip)
-                # fixme: once when loggers will be fixed, we can remove this
-                # print
-                print(
-                    "WARNING: IP address {} might be broadcast address".format(
-                        ip),
-                    file=sys.stderr
-                )
+        network_ip_address_warning(config.ips)
+        broadcast_ip_address_warning(config.ips)
 
     except errors.ACIError:
         raise ScriptError("\nThe password provided is incorrect for LDAP server "
@@ -1330,25 +1314,8 @@ def promote_check(installer):
                 config.host_name, not installer.interactive,
                 False, options.ip_addresses)
 
-        for ip in config.ips:
-            if ip.is_network_addr():
-                root_logger.warning(
-                    "IP address %s might be network address", ip)
-                # fixme: once when loggers will be fixed, we can remove this
-                # print
-                print(
-                    "WARNING: IP address {} might be network address".format(
-                        ip), file=sys.stderr
-                )
-            if ip.is_broadcast_addr():
-                root_logger.warning(
-                    "IP address %s might be broadcast address", ip)
-                # fixme: once when loggers will be fixed, we can remove this
-                # print
-                print(
-                    "WARNING: IP address {} might be broadcast address".format(
-                        ip), file=sys.stderr
-                )
+        network_ip_address_warning(config.ips)
+        broadcast_ip_address_warning(config.ips)
 
     except errors.ACIError:
         raise ScriptError("\nInsufficient privileges to promote the server.")

From f0f3a2a09ed7bff121036f04f8140e256cb45bb8 Mon Sep 17 00:00:00 2001
From: Martin Basti <mba...@redhat.com>
Date: Tue, 13 Sep 2016 17:52:07 +0200
Subject: [PATCH 3/3] Add check for IP addresses into DNS installer

https://fedorahosted.org/freeipa/ticket/5814
---
 ipaserver/install/dns.py                   |  3 +++
 ipaserver/install/server/install.py        |  5 +++--
 ipaserver/install/server/replicainstall.py | 10 ++++++----
 3 files changed, 12 insertions(+), 6 deletions(-)

diff --git a/ipaserver/install/dns.py b/ipaserver/install/dns.py
index fe66274..dedafec 100644
--- a/ipaserver/install/dns.py
+++ b/ipaserver/install/dns.py
@@ -260,6 +260,9 @@ def install_check(standalone, api, replica, options, hostname):
     ip_addresses = get_server_ip_address(hostname, options.unattended,
                                          True, options.ip_addresses)
 
+    util.network_ip_address_warning(ip_addresses)
+    util.broadcast_ip_address_warning(ip_addresses)
+
     if not options.forward_policy:
         # user did not specify policy, derive it: default is 'first' but
         # if any of local IP addresses belongs to private ranges use 'only'
diff --git a/ipaserver/install/server/install.py b/ipaserver/install/server/install.py
index 7733106..ff50bef 100644
--- a/ipaserver/install/server/install.py
+++ b/ipaserver/install/server/install.py
@@ -613,8 +613,9 @@ def install_check(installer):
                                              not installer.interactive, False,
                                              options.ip_addresses)
 
-    network_ip_address_warning(ip_addresses)
-    broadcast_ip_address_warning(ip_addresses)
+        # check addresses here, dns module is doing own check
+        network_ip_address_warning(ip_addresses)
+        broadcast_ip_address_warning(ip_addresses)
 
     # installer needs to update hosts file when DNS subsystem will be
     # installed or custom addresses are used
diff --git a/ipaserver/install/server/replicainstall.py b/ipaserver/install/server/replicainstall.py
index 571b860..aefe158 100644
--- a/ipaserver/install/server/replicainstall.py
+++ b/ipaserver/install/server/replicainstall.py
@@ -739,8 +739,9 @@ def install_check(installer):
                 config.host_name, not installer.interactive, False,
                 options.ip_addresses)
 
-        network_ip_address_warning(config.ips)
-        broadcast_ip_address_warning(config.ips)
+            # check addresses here, dns module is doing own check
+            network_ip_address_warning(config.ips)
+            broadcast_ip_address_warning(config.ips)
 
     except errors.ACIError:
         raise ScriptError("\nThe password provided is incorrect for LDAP server "
@@ -1314,8 +1315,9 @@ def promote_check(installer):
                 config.host_name, not installer.interactive,
                 False, options.ip_addresses)
 
-        network_ip_address_warning(config.ips)
-        broadcast_ip_address_warning(config.ips)
+            # check addresses here, dns module is doing own check
+            network_ip_address_warning(config.ips)
+            broadcast_ip_address_warning(config.ips)
 
     except errors.ACIError:
         raise ScriptError("\nInsufficient privileges to promote the server.")
-- 
Manage your subscription for the Freeipa-devel mailing list:
https://www.redhat.com/mailman/listinfo/freeipa-devel
Contribute to FreeIPA: http://www.freeipa.org/page/Contribute/Code

Reply via email to