Use default reverse zone consistently

When a new reverse zone is to be generated based on an IP address without
a network prefix length, we need to use some default value. While netaddr
library default ones (32b for IPv4 and 128b for IPv6) are not very sensible
we should use the defaults already applied in installers. That is 24b for
IPv6 and 64 for IPv6.

Test case has been added to cover the new default.

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

7 files changed, 78 insertions, 31 deletions

diff --git a/install/tools/ipa-dns-install b/install/tools/ipa-dns-install
index d4795f72e..84d1bdc2e 100755
--- a/install/tools/ipa-dns-install
+++ b/install/tools/ipa-dns-install
@@ -213,7 +213,7 @@ def main():
     else:
         reverse_zone = bindinstance.find_reverse_zone(ip)
         if reverse_zone is None and not options.no_reverse:
-            reverse_zone = bindinstance.get_reverse_zone_default(ip)
+            reverse_zone = util.get_reverse_zone_default(ip)
             if not options.unattended and bindinstance.create_reverse():
                 reverse_zone = bindinstance.read_reverse_zone(reverse_zone, ip)
 
diff --git a/install/tools/ipa-replica-install b/install/tools/ipa-replica-install
index 0378827d5..2c392754b 100755
--- a/install/tools/ipa-replica-install
+++ b/install/tools/ipa-replica-install
@@ -223,7 +223,7 @@ def install_bind(config, options):
     else:
         reverse_zone = bindinstance.find_reverse_zone(config.ip)
         if reverse_zone is None and not options.no_reverse:
-            reverse_zone = bindinstance.get_reverse_zone_default(config.ip)
+            reverse_zone = util.get_reverse_zone_default(config.ip)
             if not options.unattended and bindinstance.create_reverse():
                 reverse_zone = bindinstance.read_reverse_zone(reverse_zone, config.ip)
 
diff --git a/install/tools/ipa-server-install b/install/tools/ipa-server-install
index 201e2fb18..37d37b81a 100755
--- a/install/tools/ipa-server-install
+++ b/install/tools/ipa-server-install
@@ -814,7 +814,7 @@ def main():
         if options.reverse_zone:
             reverse_zone = bindinstance.normalize_zone(options.reverse_zone)
         elif not options.no_reverse:
-            reverse_zone = bindinstance.get_reverse_zone_default(ip)
+            reverse_zone = util.get_reverse_zone_default(ip)
             if not options.unattended and bindinstance.create_reverse():
                 reverse_zone = bindinstance.read_reverse_zone(reverse_zone, ip)
 
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 562f23a01..5484119d4 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -33,7 +33,8 @@ from ipalib.plugins.baseldap import *
 from ipalib import _, ngettext
 from ipalib.util import (validate_zonemgr, normalize_zonemgr,
         validate_hostname, validate_dns_label, validate_domain_name,
-        get_dns_forward_zone_update_policy, get_dns_reverse_zone_update_policy)
+        get_dns_forward_zone_update_policy, get_dns_reverse_zone_update_policy,
+        get_reverse_zone_default)
 from ipapython.ipautil import valid_ip, CheckedIPAddress, is_host_resolvable
 
 __doc__ = _("""
@@ -254,6 +255,14 @@ def _create_zone_serial():
     return int(time.time())
 
 def _reverse_zone_name(netstr):
+    try:
+        netaddr.IPAddress(netstr)
+    except (netaddr.AddrFormatError, ValueError):
+        pass
+    else:
+        # use more sensible default prefix than netaddr default
+        return unicode(get_reverse_zone_default(netstr))
+
     net = netaddr.IPNetwork(netstr)
     items = net.ip.reverse_dns.split('.')
     if net.version == 4:
diff --git a/ipalib/util.py b/ipalib/util.py
index 1d5900924..df8791ba0 100644
--- a/ipalib/util.py
+++ b/ipalib/util.py
@@ -27,6 +27,7 @@ import time
 import socket
 import re
 import decimal
+import netaddr
 from types import NoneType
 from weakref import WeakKeyDictionary
 from dns import resolver, rdatatype
@@ -172,6 +173,12 @@ def normalize_zonemgr(zonemgr):
 
     return zonemgr
 
+def normalize_zone(zone):
+    if zone[-1] != '.':
+        return zone + '.'
+    else:
+        return zone
+
 def validate_dns_label(dns_label, allow_underscore=False):
     label_chars = r'a-z0-9'
     underscore_err_msg = ''
@@ -487,6 +494,17 @@ def get_dns_reverse_zone_update_policy(realm, reverse_zone, rrtypes=('PTR',)):
 
     return policy
 
+def get_reverse_zone_default(ip_address):
+    ip = netaddr.IPAddress(ip_address)
+    items = ip.reverse_dns.split('.')
+
+    if ip.version == 4:
+        items = items[1:]   # /24 for IPv4
+    elif ip.version == 6:
+        items = items[16:]  # /64 for IPv6
+
+    return normalize_zone('.'.join(items))
+
 def validate_rdn_param(ugettext, value):
     try:
         rdn = RDN(value)
diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py
index 8284f3eaa..c2c4a86b4 100644
--- a/ipaserver/install/bindinstance.py
+++ b/ipaserver/install/bindinstance.py
@@ -33,7 +33,8 @@ from ipapython import sysrestore
 from ipapython import ipautil
 from ipalib.parameters import IA5Str
 from ipalib.util import (validate_zonemgr, normalize_zonemgr,
-        get_dns_forward_zone_update_policy, get_dns_reverse_zone_update_policy)
+        get_dns_forward_zone_update_policy, get_dns_reverse_zone_update_policy,
+        normalize_zone, get_reverse_zone_default)
 from ipapython.ipa_log_manager import *
 from ipalib.text import _
 
@@ -72,12 +73,6 @@ def check_inst(unattended):
 
     return True
 
-def normalize_zone(zone):
-    if zone[-1] != '.':
-        return zone + '.'
-    else:
-        return zone
-
 def create_reverse():
     return ipautil.user_input("Do you want to configure the reverse zone?", True)
 
@@ -231,17 +226,6 @@ def verify_reverse_zone(zone, ip_address):
 
     return True
 
-def get_reverse_zone_default(ip_address):
-    ip = netaddr.IPAddress(ip_address)
-    items = ip.reverse_dns.split('.')
-
-    if ip.version == 4:
-        items = items[1:]   # /24 for IPv4
-    elif ip.version == 6:
-        items = items[16:]  # /64 for IPv6
-
-    return normalize_zone('.'.join(items))
-
 def find_reverse_zone(ip_address):
     ip = netaddr.IPAddress(ip_address)
     zone = normalize_zone(ip.reverse_dns)
diff --git a/tests/test_xmlrpc/test_dns_plugin.py b/tests/test_xmlrpc/test_dns_plugin.py
index e8c0b241c..6a54bcb63 100644
--- a/tests/test_xmlrpc/test_dns_plugin.py
+++ b/tests/test_xmlrpc/test_dns_plugin.py
@@ -27,7 +27,7 @@ from tests.test_xmlrpc import objectclasses
 from xmlrpc_test import Declarative, fuzzy_digits, fuzzy_uuid
 
 dnszone1 = u'dnszone.test'
-dnszone1_dn = DN(('idnsname',dnszone1),('cn','dns'),api.env.basedn)
+dnszone1_dn = DN(('idnsname',dnszone1), api.env.container_dns, api.env.basedn)
 dnszone1_mname = u'ns1.%s.' % dnszone1
 dnszone1_mname_dn = DN(('idnsname','ns1'), dnszone1_dn)
 dnszone1_rname = u'root.%s.' % dnszone1
@@ -35,12 +35,15 @@ dnszone1_permission = u'Manage DNS zone %s' % dnszone1
 dnszone1_permission_dn = DN(('cn',dnszone1_permission),
                             api.env.container_permission,api.env.basedn)
 dnszone2 = u'dnszone2.test'
-dnszone2_dn = DN(('idnsname',dnszone2),('cn','dns'),api.env.basedn)
+dnszone2_dn = DN(('idnsname', dnszone2), api.env.container_dns, api.env.basedn)
 dnszone2_mname = u'ns1.%s.' % dnszone2
 dnszone2_rname = u'root.%s.' % dnszone2
 revdnszone1 = u'15.142.80.in-addr.arpa.'
 revdnszone1_ip = u'80.142.15.0/24'
-revdnszone1_dn = DN(('idnsname',revdnszone1),('cn','dns'),api.env.basedn)
+revdnszone1_dn = DN(('idnsname', revdnszone1), api.env.container_dns, api.env.basedn)
+revdnszone2 = u'16.142.80.in-addr.arpa.'
+revdnszone2_ip = u'80.142.16.0'
+revdnszone2_dn = DN(('idnsname',revdnszone2), api.env.container_dns, api.env.basedn)
 dnsres1 = u'testdnsres'
 dnsres1_dn = DN(('idnsname',dnsres1), dnszone1_dn)
 dnsres1_renamed = u'testdnsres-renamed'
@@ -72,11 +75,8 @@ class test_dns(Declarative):
             pass
 
     cleanup_commands = [
-        ('dnszone_del', [dnszone1], {}),
-        ('dnsrecord_del', [dnszone1, dnsres1], {'del_all' : True}),
-        ('dnsrecord_del', [dnszone1, dnsres1_renamed], {'del_all' : True}),
-        ('dnszone_del', [dnszone2], {}),
-        ('dnszone_del', [revdnszone1], {}),
+        ('dnszone_del', [dnszone1, dnszone2, revdnszone1, revdnszone2],
+            {'continue': True}),
         ('dnsconfig_mod', [], {'idnsforwarders' : None,
                                'idnsforwardpolicy' : None,
                                'idnsallowsyncptr' : None,
@@ -949,7 +949,7 @@ class test_dns(Declarative):
         ),
 
         dict(
-            desc='Create reverse from IP %s zone using name_from_ip option' % revdnszone1_ip,
+            desc='Create reverse zone from IP/netmask %r using name_from_ip option' % revdnszone1_ip,
             command=(
                 'dnszone_add', [], {
                     'name_from_ip': revdnszone1_ip,
@@ -985,6 +985,42 @@ class test_dns(Declarative):
 
 
         dict(
+            desc='Create reverse zone from IP %r using name_from_ip option' % revdnszone2_ip,
+            command=(
+                'dnszone_add', [], {
+                    'name_from_ip': revdnszone2_ip,
+                    'idnssoamname': dnszone1_mname,
+                    'idnssoarname': dnszone1_rname,
+                    'ip_address' : u'1.2.3.4',
+                }
+            ),
+            expected={
+                'value': revdnszone2,
+                'summary': None,
+                'result': {
+                    'dn': revdnszone2_dn,
+                    'idnsname': [revdnszone2],
+                    'idnszoneactive': [u'TRUE'],
+                    'idnssoamname': [dnszone1_mname],
+                    'nsrecord': [dnszone1_mname],
+                    'idnssoarname': [dnszone1_rname],
+                    'idnssoaserial': [fuzzy_digits],
+                    'idnssoarefresh': [fuzzy_digits],
+                    'idnssoaretry': [fuzzy_digits],
+                    'idnssoaexpire': [fuzzy_digits],
+                    'idnssoaminimum': [fuzzy_digits],
+                    'idnsallowdynupdate': [u'FALSE'],
+                    'idnsupdatepolicy': [u'grant %(realm)s krb5-subdomain %(zone)s PTR;'
+                                         % dict(realm=api.env.realm, zone=revdnszone2)],
+                    'idnsallowtransfer': [u'none;'],
+                    'idnsallowquery': [u'any;'],
+                    'objectclass': objectclasses.dnszone,
+                },
+            },
+        ),
+
+
+        dict(
             desc='Try to add invalid PTR %r to %r using dnsrecord_add' % (dnsrev1, revdnszone1),
             command=('dnsrecord_add', [revdnszone1, dnsrev1], {'ptrrecord': u'-.example.com' }),
             expected=errors.ValidationError(name='hostname',