On 09/12/15 18:55, Petr Spacek wrote:
On 9.12.2015 13:37, David Kupka wrote:
On 08/12/15 15:24, Petr Spacek wrote:
On 8.12.2015 12:19, David Kupka wrote:
On 08/12/15 08:56, Petr Spacek wrote:
On 7.12.2015 14:41, David Kupka wrote:
+def is_host_resolvable(fqdn):
+    if not isinstance(fqdn, DNSName):
+        fqdn = DNSName(fqdn)
+    for rdtype in (rdatatype.A, rdatatype.AAAA):
+        try:
+            resolver.query(fqdn.make_absolute(), rdtype)
+        except DNSException:
+            continue
+        else:
+            return True
+
+    return False


NACK, you are re-introducing duplicate function which was removed in
498471e4aed1367b72cd74d15811d0584a6ee268.

Please amend the patch ASAP to use new verify_host_resolvable() function so I
can test it and get it into 4.3.

Updated patches attached.

Hmm, my review script screams:

Detected base branch:   origin/master
Checks will be made against following base commit: 848912a add missing
/ipaplatform/constants.py to .gitignore
Writing API to API.txt
./ipalib/plugins/dns.py:2127:9: E124 closing bracket does not match visual
indentation
./ipalib/plugins/dns.py:2711:13: E128 continuation line under-indented for
visual indent
./ipalib/plugins/dns.py:2713:9: E124 closing bracket does not match visual
indentation
./ipalib/plugins/dns.py:2716:13: E128 continuation line under-indented for
visual indent
./ipapython/ipautil.py:928:1: E302 expected 2 blank lines, found 1
./ipapython/ipautil.py:948:17: E128 continuation line under-indented for
visual indent
./ipapython/ipautil.py:956:1: E302 expected 2 blank lines, found 1
./ipapython/ipautil.py:997:80: E501 line too long (83 > 79 characters)
./ipapython/ipautil.py:998:80: E501 line too long (83 > 79 characters)
./ipaserver/install/bindinstance.py:49:9: E128 continuation line
under-indented for visual indent
./ipaserver/install/bindinstance.py:292:80: E501 line too long (80 > 79
characters)
./ipaserver/install/bindinstance.py:438:19: E128 continuation line
under-indented for visual indent
./ipaserver/install/server/common.py:271:63: E261 at least two spaces before
inline comment
./ipaserver/install/server/common.py:271:80: E501 line too long (90 > 79
characters)
ERROR: PEP8 --diff failed, continuing ...
ERROR: API.txt was changed without a change in VERSION, continuing ...
Summary of detected errors:
   ERROR: PEP8 --diff failed
   ERROR: API.txt was changed without a change in VERSION

Please fix it :-)

Make make this more pleasant for you, you can use (and review) my attached
patch. It adds 'review' target to Makefile, it will do the same checks as I do.


Unfortunately your tool does not work for me (output w/ -o xtrace attached).
Anyway I have incremented API version and fixed most of the pep8 errors except
for E124 twice in ipalib/plugins/dns.py as it seems to be convention in the
file and E501 twice in ipapython/ipautil.py because IMO breaking string
constant into multiple lines does not help readability.

Updated patches also attached.

We are almost there, but NACK for now.

1) Following attempt to install IPA explodes. Please note that
dom-245.idm.lab.eng.brq.redhat.com DNS domain is delegated to the IPA server
before installation is started, so it gives 'timeout' or possibly SERVFAIL.

# ipa-server-install --ds-password=root4lab --admin-password=root4lab
--setup-dns --forwarder=10.38.5.26 --no-reverse --allow-zone-overlap
--domain=dom-245.idm.lab.eng.brq.redhat.com
--realm=DOM-245.IDM.LAB.ENG.BRQ.REDHAT.COM
[...]

Configuring DNS (named)
   [1/11]: generating rndc key file
   [2/11]: adding DNS container
   [3/11]: setting up our zone
   [error] InvocationError: DNS check for domain
dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after
30.000453949 seconds. Please make sure that the domain is properly delegated
to this IPA server.
ipa.ipapython.install.cli.install_tool(Server): ERROR    DNS check for domain
dom-245.idm.lab.eng.brq.redhat.com. failed: The DNS operation timed out after
30.000453949 seconds. Please make sure that the domain is properly delegated
to this IPA server.
ipa.ipapython.install.cli.install_tool(Server): ERROR    The
ipa-server-install command failed. See /var/log/ipaserver-install.log for more
information

2015-12-09T17:15:37Z DEBUG   File
"/usr/lib/python2.7/site-packages/ipapython/admintool.py", line 171, in execute
     return_value = self.run()
   File "/usr/lib/python2.7/site-packages/ipapython/install/cli.py", line 318,
in run
     cfgr.run()
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 310,
in run
     self.execute()
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 332,
in execute
     for nothing in self._executor():
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 372,
in __runner
     self._handle_exception(exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 394,
in _handle_exception
     six.reraise(*exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 362,
in __runner
     step()
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 359,
in <lambda>
     step = lambda: next(self.__gen)
   File "/usr/lib/python2.7/site-packages/ipapython/install/util.py", line 81,
in run_generator_with_yield_from
     six.reraise(*exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/util.py", line 59,
in run_generator_with_yield_from
     value = gen.send(prev_value)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 571,
in _configure
     next(executor)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 372,
in __runner
     self._handle_exception(exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 449,
in _handle_exception
     self.__parent._handle_exception(exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 394,
in _handle_exception
     six.reraise(*exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 446,
in _handle_exception
     super(ComponentBase, self)._handle_exception(exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 394,
in _handle_exception
     six.reraise(*exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 362,
in __runner
     step()
   File "/usr/lib/python2.7/site-packages/ipapython/install/core.py", line 359,
in <lambda>
     step = lambda: next(self.__gen)
   File "/usr/lib/python2.7/site-packages/ipapython/install/util.py", line 81,
in run_generator_with_yield_from
     six.reraise(*exc_info)
   File "/usr/lib/python2.7/site-packages/ipapython/install/util.py", line 59,
in run_generator_with_yield_from
     value = gen.send(prev_value)

   File "/usr/lib/python2.7/site-packages/ipapython/install/common.py", line
63, in _install
     for nothing in self._installer(self.parent):
   File "/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",
line 1471, in main
     install(self)
   File "/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",
line 265, in decorated
     func(installer)
   File "/usr/lib/python2.7/site-packages/ipaserver/install/server/install.py",
line 958, in install
     dns.install(False, False, options)
   File "/usr/lib/python2.7/site-packages/ipaserver/install/dns.py", line 322,
in install
     bind.create_instance()
   File "/usr/lib/python2.7/site-packages/ipaserver/install/bindinstance.py",
line 680, in create_instance
     self.start_creation()
   File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py", line
447, in start_creation
     run_step(full_msg, method)
   File "/usr/lib/python2.7/site-packages/ipaserver/install/service.py", line
437, in run_step
     method()
   File "/usr/lib/python2.7/site-packages/ipaserver/install/bindinstance.py",
line 805, in __setup_zone
     ns_hostname=self.api.env.host, force=True, api=self.api)
   File "/usr/lib/python2.7/site-packages/ipaserver/install/bindinstance.py",
line 331, in add_zone
     force=force)
   File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 447, in
__call__
     ret = self.run(*args, **options)
   File "/usr/lib/python2.7/site-packages/ipalib/frontend.py", line 764, in run
     return self.execute(*args, **options)
   File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 2781, in
execute
     result = super(dnszone_add, self).execute(*keys, **options)
   File "/usr/lib/python2.7/site-packages/ipalib/plugins/baseldap.py", line
1233, in execute
     *keys, **options)
   File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 2747, in
pre_callback
     ldap, dn, entry_attrs, attrs_list, *keys, **options)
   File "/usr/lib/python2.7/site-packages/ipalib/plugins/dns.py", line 2153, in
pre_callback
     raise errors.InvocationError(e.message)


2) Please print 'Checking DNS domain <xyz>, please wait ...' when validating
domain parameter in installer. The timeout is 30 seconds and users may be
nervous when the installed blocks for 30 seconds without a visible reason.

Precedent for this is in check_forwarders() in
ipaserver/install/bindinstance.py . Encapsulating check_zone_overlap() in
auxiliary method may be an option.


3) Timeout is a fatal error instead of warning. We have been discussing this
and it should really be just a warning.


4) Nitpicks are attached in .patch file.


5) ipa dnszone-add checks work as expected, good job!


Thank you for patience!

Updated patches attached. Added patch introducing --auto-reverse option that should generate needed reverse zones automatically.

--
David Kupka
From bda2091256f6343d8c99eda78c80cddfedf0992c Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Thu, 10 Dec 2015 12:54:08 +0100
Subject: [PATCH] dns: Add --auto-reverse option.

Introducing '--auto-reverse' option. When specified reverse records for
all server's IP addresses are checked and when record nor reverse zone
does not exist reverse zone is created.
---
 install/tools/ipa-dns-install      |  4 ++++
 ipaserver/install/bindinstance.py  |  5 ++++-
 ipaserver/install/server/common.py | 14 ++++++++++++++
 3 files changed, 22 insertions(+), 1 deletion(-)

diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index 4fd75670f044a3ccf1e22463b5f2dd77515172d3..eebc9e2ad8a6da71807b7d9d24cd41289c5b4d58 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -57,6 +57,8 @@ def parse_options():
                       help="The reverse DNS zone to use. This option can be used multiple times")
     parser.add_option("--no-reverse", dest="no_reverse", action="store_true",
                       default=False, help="Do not create new reverse DNS zone")
+    parser.add_option("--auto-reverse", dest="auto_reverse", action="store_true",
+                      default=False, help="Create necessary DNS zones")
     parser.add_option("--allow-zone-overlap", dest="allow_zone_overlap",
                       action="store_true", default=False, help="Create DNS "
                       "zone even if it already exists")
@@ -90,6 +92,8 @@ def parse_options():
         parser.error("You cannot specify a --forwarder option together with --no-forwarders")
     elif options.reverse_zones and options.no_reverse:
         parser.error("You cannot specify a --reverse-zone option together with --no-reverse")
+    elif options.auto_reverse and options.no_reverse:
+        parser.error("You cannot specify a --auto-reverse option together with --no-reverse")
 
     if options.unattended:
         if not options.forwarders and not options.no_forwarders:
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index ffdd7d5958fc748412ddc846cfd6ee7145b5e1e1..882ec4ab90b198b9f6d2f04decc98c5a2032ba09 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -479,7 +479,10 @@ def check_reverse_zones(ip_addresses, reverse_zones, options, unattended, search
 
     # create reverse zone for IP addresses that does not have one
     for (ip, rz) in get_auto_reverse_zones(ips_missing_reverse):
-        if unattended:
+        if options.auto_reverse:
+            root_logger.info("Reverse zone %s will be created" % rz)
+            checked_reverse_zones.append(rz)
+        elif unattended:
             root_logger.warning("Missing reverse record for IP address %s" % ip)
         else:
             if ipautil.user_input("Do you want to create reverse zone for IP "
diff --git a/ipaserver/install/server/common.py b/ipaserver/install/server/common.py
index 511dd6c4252967063ccf7e47f92187a7d6c84c53..55a98e1cb76e2ddd8edf10bdee7e8c4961bedd08 100644
--- a/ipaserver/install/server/common.py
+++ b/ipaserver/install/server/common.py
@@ -197,6 +197,11 @@ class BaseServerDNS(common.Installable, core.Group, core.Composite):
         description="Do not create new reverse DNS zone",
     )
 
+    auto_reverse = Knob(
+        bool, False,
+        description="Create necessary reverse zones",
+    )
+
     no_dnssec_validation = Knob(
         bool, False,
         description="Disable DNSSEC validation",
@@ -423,6 +428,10 @@ class BaseServer(common.Installable, common.Interactive, core.Composite):
                 raise RuntimeError(
                     "You cannot specify a --reverse-zone option without the "
                     "--setup-dns option")
+            if self.dns.auto_reverse:
+                raise RuntimeError(
+                    "You cannot specify a --auto-reverse option without the "
+                    "--setup-dns option")
             if self.dns.no_reverse:
                 raise RuntimeError(
                     "You cannot specify a --no-reverse option without the "
@@ -443,6 +452,10 @@ class BaseServer(common.Installable, common.Interactive, core.Composite):
             raise RuntimeError(
                 "You cannot specify a --reverse-zone option together with "
                 "--no-reverse")
+        elif self.dns.auto_reverse and self.dns.no_reverse:
+            raise RuntimeError(
+                "You cannot specify a --auto-reverse option together with "
+                "--no-reverse")
 
         # Automatically disable pkinit w/ dogtag until that is supported
         self.no_pkinit = True
@@ -469,6 +482,7 @@ class BaseServer(common.Installable, common.Interactive, core.Composite):
         self.no_forwarders = self.dns.no_forwarders
         self.reverse_zones = self.dns.reverse_zones
         self.no_reverse = self.dns.no_reverse
+        self.auto_reverse = self.dns.auto_reverse
         self.allow_zone_overlap = self.dns.allow_zone_overlap
         self.no_dnssec_validation = self.dns.no_dnssec_validation
         self.dnssec_master = self.dns.dnssec_master
-- 
2.5.0

From f0a1acfc35c4f9d12405b9aab3f9d5ef8d6e6ff4 Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Wed, 2 Dec 2015 13:17:13 +0000
Subject: [PATCH] dns: do not add (forward)zone if it is already resolvable.

Check if the zone user wants to add is already resolvable and refuse to
create it if yes. --skip-overlap-check and --force options suppress this check.

https://fedorahosted.org/freeipa/ticket/5087
---
 API.txt               |  7 ++--
 VERSION               |  2 +-
 ipalib/plugins/dns.py | 38 ++++++++++++++++++---
 ipapython/ipautil.py  | 92 +++++++++++++++++++++++++++++++++++++++++++++++++--
 4 files changed, 129 insertions(+), 10 deletions(-)

diff --git a/API.txt b/API.txt
index 60c98c31aa85d6c8879cd145f3d84188d4fea5b7..3a9fb65a386a2a6529b8cd241642446c135471f2 100644
--- a/API.txt
+++ b/API.txt
@@ -959,7 +959,7 @@ output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDA
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: PrimaryKey('value', None, None)
 command: dnsforwardzone_add
-args: 1,8,3
+args: 1,9,3
 arg: DNSNameParam('idnsname', attribute=True, cli_name='name', multivalue=False, only_absolute=True, primary_key=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -968,6 +968,7 @@ option: StrEnum('idnsforwardpolicy', attribute=True, cli_name='forward_policy',
 option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue=False, required=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Flag('skip_overlap_check', autofill=True, default=False)
 option: Str('version?', exclude='webui')
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
@@ -1366,7 +1367,7 @@ output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDA
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
 output: PrimaryKey('value', None, None)
 command: dnszone_add
-args: 1,26,3
+args: 1,28,3
 arg: DNSNameParam('idnsname', attribute=True, cli_name='name', multivalue=False, only_absolute=True, primary_key=True, required=True)
 option: Str('addattr*', cli_name='addattr', exclude='webui')
 option: Flag('all', autofill=True, cli_name='all', default=False, exclude='webui')
@@ -1393,6 +1394,8 @@ option: Str('name_from_ip', attribute=False, cli_name='name_from_ip', multivalue
 option: Str('nsec3paramrecord', attribute=True, cli_name='nsec3param_rec', multivalue=False, pattern='^\\d+ \\d+ \\d+ (([0-9a-fA-F]{2})+|-)$', required=False)
 option: Flag('raw', autofill=True, cli_name='raw', default=False, exclude='webui')
 option: Str('setattr*', cli_name='setattr', exclude='webui')
+option: Flag('skip_nameserver_check', autofill=True, default=False)
+option: Flag('skip_overlap_check', autofill=True, default=False)
 option: Str('version?', exclude='webui')
 output: Entry('result', <type 'dict'>, Gettext('A dictionary representing an LDAP entry', domain='ipa', localedir=None))
 output: Output('summary', (<type 'unicode'>, <type 'NoneType'>), None)
diff --git a/VERSION b/VERSION
index b7f261b5c5927053e2c2bfa4e3e7075c07caab5c..aeb1fea6bbeb2462752d001e96b0bf1b35011dc3 100644
--- a/VERSION
+++ b/VERSION
@@ -90,5 +90,5 @@ IPA_DATA_VERSION=20100614120000
 #                                                      #
 ########################################################
 IPA_API_VERSION_MAJOR=2
-IPA_API_VERSION_MINOR=161
+IPA_API_VERSION_MINOR=162
 # Last change: pvoborni - topologysuffix: change iparepltopomanagedsuffix type
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 67947360eb207de31ed114bb630705c409b2f9a9..a4a5ecef77eb27b5865213b8397203f69502bdfa 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -53,8 +53,7 @@ from ipalib.util import (normalize_zonemgr,
                          validate_dnssec_zone_forwarder_step1,
                          validate_dnssec_zone_forwarder_step2,
                          verify_host_resolvable)
-
-from ipapython.ipautil import CheckedIPAddress
+from ipapython.ipautil import CheckedIPAddress, check_zone_overlap
 from ipapython.dnsutil import DNSName
 
 if six.PY3:
@@ -2121,6 +2120,13 @@ class DNSZoneBase(LDAPObject):
 
 class DNSZoneBase_add(LDAPCreate):
 
+    takes_options = LDAPCreate.takes_options + (
+        Flag('skip_overlap_check',
+             doc=_('Force DNS zone creation even if it will overlap with '
+                   'an existing zone.')
+        ),
+    )
+
     has_output_params = LDAPCreate.has_output_params + dnszone_output_params
 
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
@@ -2140,6 +2146,12 @@ class DNSZoneBase_add(LDAPCreate):
 
         entry_attrs['idnszoneactive'] = 'TRUE'
 
+        if not options['skip_overlap_check']:
+            try:
+                check_zone_overlap(keys[-1])
+            except ValueError as e:
+                raise errors.InvocationError(e.message)
+
         return dn
 
 
@@ -2696,8 +2708,13 @@ class dnszone_add(DNSZoneBase_add):
 
     takes_options = DNSZoneBase_add.takes_options + (
         Flag('force',
-             label=_('Force'),
-             doc=_('Force DNS zone creation even if nameserver is not resolvable.'),
+             doc=_('Force DNS zone creation even if nameserver is not '
+                   'resolvable. (Deprecated)'),
+        ),
+
+        Flag('skip_nameserver_check',
+             doc=_('Force DNS zone creation even if nameserver is not '
+                   'resolvable.'),
         ),
 
         # Deprecated
@@ -2717,10 +2734,21 @@ class dnszone_add(DNSZoneBase_add):
                     option='ip-address',
                     additional_info=u"Value will be ignored.")
             )
+        if 'force' in options:
+            messages.add_message(
+                options['version'],
+                result,
+                messages.OptionDeprecatedWarning(
+			option='force',
+			additional_info=u"")
+            )
 
     def pre_callback(self, ldap, dn, entry_attrs, attrs_list, *keys, **options):
         assert isinstance(dn, DN)
 
+        if options.get('force'):
+            options['skip_nameserver_check'] = True
+
         dn = super(dnszone_add, self).pre_callback(
             ldap, dn, entry_attrs, attrs_list, *keys, **options)
 
@@ -2736,7 +2764,7 @@ class dnszone_add(DNSZoneBase_add):
                     error=_("Nameserver for reverse zone cannot be a relative DNS name"))
 
             # verify if user specified server is resolvable
-            if not options['force']:
+            if not options['skip_nameserver_check']:
                 check_ns_rec_resolvable(keys[0], entry_attrs['idnssoamname'],
                                         self.log)
             # show warning about --name-server option
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 89047b2e8ea16d14a6634e551c49abe240c54009..1061f61723465a1b9c8313c3b5382d53ced2ae6f 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -39,7 +39,7 @@ import grp
 from contextlib import contextmanager
 
 from dns import resolver, rdatatype
-from dns.exception import DNSException
+from dns.exception import DNSException, Timeout
 import six
 from six.moves import input
 from six.moves import urllib
@@ -50,6 +50,7 @@ from ipapython import config
 from ipaplatform.paths import paths
 from ipapython.dn import DN
 from ipapython.dnsutil import DNSName
+from ipalib.util import normalize_zone
 
 SHARE_DIR = paths.USR_SHARE_IPA_DIR
 PLUGINS_SHARE_DIR = paths.IPA_PLUGINS
@@ -240,7 +241,6 @@ def template_file(infilename, vars):
     with open(infilename) as f:
         return template_str(f.read(), vars)
 
-
 def copy_template_file(infilename, outfilename, vars):
     """Copy a file, performing template substitutions"""
     txt = template_file(infilename, vars)
@@ -925,6 +925,94 @@ def host_exists(host):
     else:
         return True
 
+
+def check_zone_overlap(zone, raise_on_timeout=True):
+    root_logger.info("Checking DNS domain %s, please wait ..." % zone)
+    if not isinstance(zone, DNSName):
+        zone = DNSName(normalize_zone(zone))
+
+    try:
+        containing_zone = resolver.zone_for_name(zone)
+    except Timeout as e:
+        msg = ("DNS check for domain %s failed: %s. Please make sure that the "
+               "domain is properly delegated to this IPA server." % (zone, e))
+        if raise_on_timeout:
+            raise ValueError(msg)
+        else:
+            root_logger.warning(msg)
+            return
+
+    if containing_zone == zone:
+        try:
+            ns = [ans.to_text() for ans in resolver.query(zone, 'NS')]
+        except DNSException as e:
+            root_logger.debug("Failed to resolve nameserver(s) for domain"
+                              " {0}: {1}".format(zone, e))
+            ns = []
+
+        msg = u"DNS zone {0} already exists in DNS".format(zone)
+        if ns:
+            msg += u" and is handled by server(s): {0}".format(', '.join(ns))
+        raise ValueError(msg)
+
+
+def check_reverse_zone_overlap(zone, raise_on_timeout=True):
+    automatic_empty_zones = [
+        # RFC 1918
+        "10.IN-ADDR.ARPA", "16.172.IN-ADDR.ARPA", "17.172.IN-ADDR.ARPA",
+        "18.172.IN-ADDR.ARPA", "19.172.IN-ADDR.ARPA", "20.172.IN-ADDR.ARPA",
+        "21.172.IN-ADDR.ARPA", "22.172.IN-ADDR.ARPA", "23.172.IN-ADDR.ARPA",
+        "24.172.IN-ADDR.ARPA", "25.172.IN-ADDR.ARPA", "26.172.IN-ADDR.ARPA",
+        "27.172.IN-ADDR.ARPA", "28.172.IN-ADDR.ARPA", "29.172.IN-ADDR.ARPA",
+        "30.172.IN-ADDR.ARPA", "31.172.IN-ADDR.ARPA", "168.192.IN-ADDR.ARPA",
+        # RFC 6598
+        "64.100.IN-ADDR.ARPA", "65.100.IN-ADDR.ARPA", "66.100.IN-ADDR.ARPA",
+        "67.100.IN-ADDR.ARPA", "68.100.IN-ADDR.ARPA", "69.100.IN-ADDR.ARPA",
+        "70.100.IN-ADDR.ARPA", "71.100.IN-ADDR.ARPA", "72.100.IN-ADDR.ARPA",
+        "73.100.IN-ADDR.ARPA", "74.100.IN-ADDR.ARPA", "75.100.IN-ADDR.ARPA",
+        "76.100.IN-ADDR.ARPA", "77.100.IN-ADDR.ARPA", "78.100.IN-ADDR.ARPA",
+        "79.100.IN-ADDR.ARPA", "80.100.IN-ADDR.ARPA", "81.100.IN-ADDR.ARPA",
+        "82.100.IN-ADDR.ARPA", "83.100.IN-ADDR.ARPA", "84.100.IN-ADDR.ARPA",
+        "85.100.IN-ADDR.ARPA", "86.100.IN-ADDR.ARPA", "87.100.IN-ADDR.ARPA",
+        "88.100.IN-ADDR.ARPA", "89.100.IN-ADDR.ARPA", "90.100.IN-ADDR.ARPA",
+        "91.100.IN-ADDR.ARPA", "92.100.IN-ADDR.ARPA", "93.100.IN-ADDR.ARPA",
+        "94.100.IN-ADDR.ARPA", "95.100.IN-ADDR.ARPA", "96.100.IN-ADDR.ARPA",
+        "97.100.IN-ADDR.ARPA", "98.100.IN-ADDR.ARPA", "99.100.IN-ADDR.ARPA",
+        "100.100.IN-ADDR.ARPA", "101.100.IN-ADDR.ARPA",
+        "102.100.IN-ADDR.ARPA", "103.100.IN-ADDR.ARPA",
+        "104.100.IN-ADDR.ARPA", "105.100.IN-ADDR.ARPA",
+        "106.100.IN-ADDR.ARPA", "107.100.IN-ADDR.ARPA",
+        "108.100.IN-ADDR.ARPA", "109.100.IN-ADDR.ARPA",
+        "110.100.IN-ADDR.ARPA", "111.100.IN-ADDR.ARPA",
+        "112.100.IN-ADDR.ARPA", "113.100.IN-ADDR.ARPA",
+        "114.100.IN-ADDR.ARPA", "115.100.IN-ADDR.ARPA",
+        "116.100.IN-ADDR.ARPA", "117.100.IN-ADDR.ARPA",
+        "118.100.IN-ADDR.ARPA", "119.100.IN-ADDR.ARPA",
+        "120.100.IN-ADDR.ARPA", "121.100.IN-ADDR.ARPA",
+        "122.100.IN-ADDR.ARPA", "123.100.IN-ADDR.ARPA",
+        "124.100.IN-ADDR.ARPA", "125.100.IN-ADDR.ARPA",
+        "126.100.IN-ADDR.ARPA", "127.100.IN-ADDR.ARPA",
+        # RFC 5735 and RFC 5737
+        "0.IN-ADDR.ARPA", "127.IN-ADDR.ARPA", "254.169.IN-ADDR.ARPA",
+        "2.0.192.IN-ADDR.ARPA", "100.51.198.IN-ADDR.ARPA",
+        "113.0.203.IN-ADDR.ARPA", "255.255.255.255.IN-ADDR.ARPA",
+        # Local IPv6 Unicast Addresses
+        "0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.IP6.ARPA",
+        "1.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.0.IP6.ARPA",
+        # LOCALLY ASSIGNED LOCAL ADDRESS SCOPE
+        "D.F.IP6.ARPA", "8.E.F.IP6.ARPA", "9.E.F.IP6.ARPA", "A.E.F.IP6.ARPA",
+        "B.E.F.IP6.ARPA",
+        # Example Prefix, RFC 3849.
+        "8.B.D.0.1.0.0.2.IP6.ARPA",
+        # RFC 7534
+        "EMPTY.AS112.ARPA",
+    ]
+    # automatic empty zones always exist so checking them is pointless,
+    # do not report them to avoid meaningless error messages
+    if zone in automatic_empty_zones:
+        return
+    check_zone_overlap(zone, raise_on_timeout=raise_on_timeout)
+
 def get_ipa_basedn(conn):
     """
     Get base DN of IPA suffix in given LDAP server.
-- 
2.5.0

From 7215548bf3b2f8c01a93493038beb6ad66adb3ad Mon Sep 17 00:00:00 2001
From: David Kupka <dku...@redhat.com>
Date: Wed, 2 Dec 2015 14:20:50 +0000
Subject: [PATCH] dns: Check if domain already exists.

Raise an error when the domain already exists. This can be overriden using
--force or --allow-zone-overlap options.

https://fedorahosted.org/freeipa/ticket/3681
---
 install/tools/ipa-dns-install      |   3 +
 ipapython/ipautil.py               |  18 ++++-
 ipaserver/install/bindinstance.py  | 159 ++++++++++++++++++++++++-------------
 ipaserver/install/dns.py           |  20 +++++
 ipaserver/install/server/common.py |  18 +++++
 5 files changed, 160 insertions(+), 58 deletions(-)

diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index bdaffd30b2554c100dc29bd18e0474165d05c024..4fd75670f044a3ccf1e22463b5f2dd77515172d3 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -57,6 +57,9 @@ def parse_options():
                       help="The reverse DNS zone to use. This option can be used multiple times")
     parser.add_option("--no-reverse", dest="no_reverse", action="store_true",
                       default=False, help="Do not create new reverse DNS zone")
+    parser.add_option("--allow-zone-overlap", dest="allow_zone_overlap",
+                      action="store_true", default=False, help="Create DNS "
+                      "zone even if it already exists")
     parser.add_option("--no-dnssec-validation", dest="no_dnssec_validation", action="store_true",
                       default=False, help="Disable DNSSEC validation")
     parser.add_option("--dnssec-master", dest="dnssec_master", action="store_true",
diff --git a/ipapython/ipautil.py b/ipapython/ipautil.py
index 1061f61723465a1b9c8313c3b5382d53ced2ae6f..9abf26b601cd102a5be718b7ab56b773b941d1b4 100644
--- a/ipapython/ipautil.py
+++ b/ipapython/ipautil.py
@@ -38,7 +38,7 @@ import pwd
 import grp
 from contextlib import contextmanager
 
-from dns import resolver, rdatatype
+from dns import resolver, rdatatype, reversename
 from dns.exception import DNSException, Timeout
 import six
 from six.moves import input
@@ -926,6 +926,22 @@ def host_exists(host):
         return True
 
 
+def reverse_record_exists(ip_address):
+    """
+    Checks if IP address have some reverse record somewhere.
+    Does not care where it points.
+
+    Returns True/False
+    """
+    reverse = reversename.from_address(ip_address)
+    try:
+        resolver.query(reverse, "PTR")
+    except DNSException:
+        # really don't care what exception, PTR is simply unresolvable
+        return False
+    return True
+
+
 def check_zone_overlap(zone, raise_on_timeout=True):
     root_logger.info("Checking DNS domain %s, please wait ..." % zone)
     if not isinstance(zone, DNSName):
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 6bfde83de600df43e3430299100565e554a80583..ffdd7d5958fc748412ddc846cfd6ee7145b5e1e1 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -43,10 +43,12 @@ from ipaplatform.constants import constants
 from ipaplatform.paths import paths
 from ipaplatform.tasks import tasks
 from ipalib.util import (validate_zonemgr_str, normalize_zonemgr,
-        get_dns_forward_zone_update_policy, get_dns_reverse_zone_update_policy,
-        normalize_zone, get_reverse_zone_default, zone_is_reverse,
-        validate_dnssec_global_forwarder, DNSSECSignatureMissingError,
-        EDNS0UnsupportedError, UnresolvableRecordError)
+                         get_dns_forward_zone_update_policy,
+                         get_dns_reverse_zone_update_policy,
+                         normalize_zone, get_reverse_zone_default,
+                         zone_is_reverse, validate_dnssec_global_forwarder,
+                         DNSSECSignatureMissingError, EDNS0UnsupportedError,
+                         UnresolvableRecordError, verify_host_resolvable)
 from ipalib.constants import CACERT
 
 if six.PY3:
@@ -278,20 +280,47 @@ def find_reverse_zone(ip_address, api=api):
     return None
 
 
-def read_reverse_zone(default, ip_address):
+def read_reverse_zone(default, ip_address, allow_zone_overlap=False):
     while True:
         zone = ipautil.user_input("Please specify the reverse zone name", default=default)
         if not zone:
             return None
-        if verify_reverse_zone(zone, ip_address):
-            break
-        else:
-            print("Invalid reverse zone %s for IP address %s" % (zone, ip_address))
+        if not verify_reverse_zone(zone, ip_address):
+            root_logger.error("Invalid reverse zone %s for IP address %s"
+                              % (zone, ip_address))
+            continue
+        if not allow_zone_overlap:
+            try:
+                ipautil.check_reverse_zone_overlap(zone,
+                                                   raise_on_timeout=False)
+            except ValueError as e:
+                root_logger.error("Reverse zone %s will not be used: %s"
+                                  % (zone, e))
+                continue
+        break
 
     return normalize_zone(zone)
 
+def get_auto_reverse_zones(ip_addresses):
+    auto_zones = []
+    for ip in ip_addresses:
+        if ipautil.reverse_record_exists(ip):
+            # PTR exist there is no reason to create reverse zone
+            root_logger.info("Reverse record for IP address %s already "
+                             "exists" % ip)
+            continue
+        default_reverse = get_reverse_zone_default(ip)
+        try:
+            ipautil.check_reverse_zone_overlap(default_reverse)
+        except ValueError:
+            root_logger.info("Reverse zone %s for IP address %s already exists"
+                             % (default_reverse, ip))
+            continue
+        auto_zones.append(default_reverse)
+    return auto_zones
+
 def add_zone(name, zonemgr=None, dns_backup=None, ns_hostname=None,
-       update_policy=None, force=False, api=api):
+       update_policy=None, force=False, skip_overlap_check=False, api=api):
 
     # always normalize zones
     name = normalize_zone(name)
@@ -317,6 +346,7 @@ def add_zone(name, zonemgr=None, dns_backup=None, ns_hostname=None,
                                 idnsupdatepolicy=unicode(update_policy),
                                 idnsallowquery=u'any',
                                 idnsallowtransfer=u'none',
+                                skip_overlap_check=skip_overlap_check,
                                 force=force)
     except (errors.DuplicateEntry, errors.EmptyModlist):
         pass
@@ -407,45 +437,57 @@ def zonemgr_callback(option, opt_str, value, parser):
     parser.values.zonemgr = value
 
 def check_reverse_zones(ip_addresses, reverse_zones, options, unattended, search_reverse_zones=False):
-    reverse_asked = False
+    checked_reverse_zones = []
 
-    ret_reverse_zones = []
-    # check that there is IP address in every reverse zone
-    if reverse_zones:
-        for rz in reverse_zones:
-            for ip in ip_addresses:
-                if verify_reverse_zone(rz, ip):
-                    ret_reverse_zones.append(normalize_zone(rz))
-                    break
-            else:
-                # no ip matching reverse zone found
-                sys.exit("There is no IP address matching reverse zone %s." % rz)
-    if not options.no_reverse:
-        # check that there is reverse zone for every IP
-        for ip in ip_addresses:
-            if search_reverse_zones and find_reverse_zone(str(ip)):
-                # reverse zone is already in LDAP
+    if not options.no_reverse and not reverse_zones:
+        if unattended:
+            options.no_reverse = True
+        else:
+            options.no_reverse = not create_reverse()
+
+    # shortcut
+    if options.no_reverse:
+        return []
+
+    # verify zones passed in options
+    for rz in reverse_zones:
+        # isn't the zone managed by someone else
+        if not options.allow_zone_overlap:
+            try:
+                ipautil.check_reverse_zone_overlap(rz)
+            except ValueError as e:
+                msg = "Reverse zone %s will not be used: %s" % (rz, e)
+                if options.unattended:
+                    sys.exit(msg)
+                else:
+                    root_logger.warning(msg)
                 continue
-            for rz in ret_reverse_zones:
-                if verify_reverse_zone(rz, ip):
-                    # reverse zone was entered by user
-                    break
-            else:
-                # no reverse zone for ip found
-                if not reverse_asked:
-                    if not unattended and not reverse_zones:
-                        # user did not specify reverse_zone nor no_reverse
-                        options.no_reverse = not create_reverse()
-                        if options.no_reverse:
-                            # user decided not to create reverse zone
-                            return []
-                    reverse_asked = True
-                rz = get_reverse_zone_default(str(ip))
-                if not unattended:
-                    rz = read_reverse_zone(rz, str(ip))
-                ret_reverse_zones.append(rz)
+        checked_reverse_zones.append(normalize_zone(rz))
 
-    return ret_reverse_zones
+    # check that there is reverse zone for every IP
+    ips_missing_reverse = []
+    for ip in ip_addresses:
+        if search_reverse_zones and find_reverse_zone(str(ip)):
+            # reverse zone is already in LDAP
+            continue
+        for rz in checked_reverse_zones:
+            if verify_reverse_zone(rz, ip):
+                # reverse zone was entered by user
+                break
+        else:
+            ips_missing_reverse.append(ip)
+
+    # create reverse zone for IP addresses that does not have one
+    for (ip, rz) in get_auto_reverse_zones(ips_missing_reverse):
+        if unattended:
+            root_logger.warning("Missing reverse record for IP address %s" % ip)
+        else:
+            if ipautil.user_input("Do you want to create reverse zone for IP "
+                                  "%s" % ip, True):
+                rz = read_reverse_zone(rz, str(ip), options.allow_zone_overlap)
+                checked_reverse_zones.append(rz)
+
+    return checked_reverse_zones
 
 def check_forwarders(dns_forwarders, logger):
     print("Checking DNS forwarders, please wait ...")
@@ -770,7 +812,8 @@ class BindInstance(service.Service):
     def __setup_zone(self):
         # Always use force=True as named is not set up yet
         add_zone(self.domain, self.zonemgr, dns_backup=self.dns_backup,
-                 ns_hostname=self.api.env.host, force=True, api=self.api)
+                 ns_hostname=self.api.env.host, force=True,
+                 skip_overlap_check=True, api=self.api)
 
         add_rr(self.domain, "_kerberos", "TXT", self.realm, api=self.api)
 
@@ -788,7 +831,8 @@ class BindInstance(service.Service):
         # Always use force=True as named is not set up yet
         for reverse_zone in self.reverse_zones:
             add_zone(reverse_zone, self.zonemgr, ns_hostname=self.api.env.host,
-                     dns_backup=self.dns_backup, force=True, api=self.api)
+                     dns_backup=self.dns_backup, force=True,
+                     skip_overlap_check=True, api=self.api)
 
     def __add_master_records(self, fqdn, addrs):
         host, zone = fqdn.split(".", 1)
@@ -817,18 +861,19 @@ class BindInstance(service.Service):
                    api=self.api)
 
         if not dns_zone_exists(zone, self.api):
-            # add DNS domain for host first
-            root_logger.debug(
-                "Host domain (%s) is different from DNS domain (%s)!" % (
-                    zone, self.domain))
-            root_logger.debug("Add DNS zone for host first.")
-
-            add_zone(zone, self.zonemgr, dns_backup=self.dns_backup,
-                     ns_hostname=self.fqdn, force=True, api=self.api)
+            # check if master hostname is resolvable
+            try:
+                verify_host_resolvable(fqdn, root_logger)
+            except errors.DNSNotARecordError:
+                root_logger.warning("Master FQDN (%s) is not resolvable.",
+                                    fqdn)
 
         # Add forward and reverse records to self
         for addr in addrs:
-            add_fwd_rr(zone, host, addr, api=self.api)
+            try:
+                add_fwd_rr(zone, host, addr, self.api)
+            except errors.NotFound as e:
+                pass
 
             reverse_zone = find_reverse_zone(addr, self.api)
             if reverse_zone:
diff --git a/ipaserver/install/dns.py b/ipaserver/install/dns.py
index 258bf5dbe46e2167e07a62127c7fd8fd4be23ee6..276dae2778eb15e01f65f3ab05b4e4fc9bdcffe4 100644
--- a/ipaserver/install/dns.py
+++ b/ipaserver/install/dns.py
@@ -13,11 +13,13 @@ from subprocess import CalledProcessError
 
 from ipalib import api
 from ipalib import errors
+from ipalib import util
 from ipaplatform.paths import paths
 from ipaplatform.constants import constants
 from ipaplatform import services
 from ipapython import ipautil
 from ipapython import sysrestore
+from ipapython import dnsutil
 from ipapython.dn import DN
 from ipapython.ipa_log_manager import root_logger
 from ipapython.ipaldap import AUTOBIND_ENABLED
@@ -106,6 +108,24 @@ def install_check(standalone, replica, options, hostname):
         raise RuntimeError("Integrated DNS requires '%s' package" %
                            constants.IPA_DNS_PACKAGE_NAME)
 
+    domain = dnsutil.DNSName(util.normalize_zone(api.env.domain))
+    try:
+        ipautil.check_zone_overlap(domain, raise_on_timeout=False)
+    except ValueError as e:
+        if options.force or options.allow_zone_overlap:
+            root_logger.warning(e.message)
+        else:
+            raise e
+
+    for reverse_zone in options.reverse_zones:
+        try:
+            ipautil.check_reverse_zone_overlap(reverse_zone)
+        except ValueError as e:
+            if options.force or options.allow_zone_overlap:
+                root_logger.warning(e.message)
+            else:
+                raise e
+
     if standalone:
         print("==============================================================================")
         print("This program will setup DNS for the FreeIPA Server.")
diff --git a/ipaserver/install/server/common.py b/ipaserver/install/server/common.py
index 1c161120bcb22d384afcd9a5c462645cfcc753a8..511dd6c4252967063ccf7e47f92187a7d6c84c53 100644
--- a/ipaserver/install/server/common.py
+++ b/ipaserver/install/server/common.py
@@ -10,6 +10,8 @@ from ipapython.install import common, core
 from ipapython.install.core import Knob
 from ipalib.util import validate_domain_name
 from ipaserver.install import bindinstance
+from ipapython.ipautil import check_reverse_zone_overlap, check_zone_overlap
+from ipapython.dnsutil import DNSName
 
 VALID_SUBJECT_ATTRS = ['st', 'o', 'ou', 'dnqualifier', 'c',
                        'serialnumber', 'l', 'title', 'sn', 'givenname',
@@ -171,6 +173,11 @@ class BaseServerDNS(common.Installable, core.Group, core.Composite):
         description="Do not add any DNS forwarders, use root servers instead",
     )
 
+    allow_zone_overlap = Knob(
+        bool, False,
+        description="Create DNS zone even if it already exists",
+    )
+
     reverse_zones = Knob(
         (list, str), [],
         description=("The reverse DNS zone to use. This option can be used "
@@ -179,6 +186,12 @@ class BaseServerDNS(common.Installable, core.Group, core.Composite):
         cli_metavar='REVERSE_ZONE',
     )
 
+    @reverse_zones.validator
+    def reverse_zones(self, values):
+        if not self.allow_zone_overlap:
+            for zone in values:
+                check_reverse_zone_overlap(zone)
+
     no_reverse = Knob(
         bool, False,
         description="Do not create new reverse DNS zone",
@@ -255,6 +268,10 @@ class BaseServer(common.Installable, common.Interactive, core.Composite):
     @domain_name.validator
     def domain_name(self, value):
         validate_domain_name(value)
+        if (self.setup_dns and
+                not self.dns.allow_zone_overlap):  # pylint: disable=no-member
+            print("Checking DNS domain %s, please wait ..." % value)
+            check_zone_overlap(value)
 
     dm_password = Knob(
         str, None,
@@ -452,6 +469,7 @@ class BaseServer(common.Installable, common.Interactive, core.Composite):
         self.no_forwarders = self.dns.no_forwarders
         self.reverse_zones = self.dns.reverse_zones
         self.no_reverse = self.dns.no_reverse
+        self.allow_zone_overlap = self.dns.allow_zone_overlap
         self.no_dnssec_validation = self.dns.no_dnssec_validation
         self.dnssec_master = self.dns.dnssec_master
         self.disable_dnssec_master = self.dns.disable_dnssec_master
-- 
2.5.0

-- 
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