Add ipaDNSVersion option to dnsconfig* commands and use new attribute

Ad-hoc LDAP calls in DNS upgrade code were hard to maintain and
ipaConfigString was bad idea from the very beginning as it was hard to
manipulate the number in it.

To avoid problems in future we are introducing new ipaDNSVersion
attribute which is used on cn=dns instead of ipaConfigString.
Original value of ipaConfigString is kept in the tree for now
so older upgraders see it and do not execute the upgrade procedure again.

The attribute can be changed only by installer/upgrade so it is not
exposed in dnsconfig_mod API.

Command dnsconfig_show displays it only if --all option was used.

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

Reviewed-By: Martin Basti <mbasti@redhat.com>

7 files changed, 78 insertions, 23 deletions

diff --git a/ACI.txt b/ACI.txt
index 01234e434..cea814a0c 100644
--- a/ACI.txt
+++ b/ACI.txt
@@ -51,7 +51,7 @@ aci: (targetattr = "cospriority")(targetfilter = "(objectclass=costemplate)")(ve
 dn: cn=costemplates,cn=accounts,dc=ipa,dc=example
 aci: (targetattr = "cn || cospriority || createtimestamp || entryusn || krbpwdpolicyreference || modifytimestamp || objectclass")(targetfilter = "(objectclass=costemplate)")(version 3.0;acl "permission:System: Read Group Password Policy costemplate";allow (compare,read,search) groupdn = "ldap:///cn=System: Read Group Password Policy costemplate,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: dc=ipa,dc=example
-aci: (targetattr = "createtimestamp || entryusn || idnsallowsyncptr || idnsforwarders || idnsforwardpolicy || idnspersistentsearch || idnszonerefresh || modifytimestamp || objectclass")(target = "ldap:///cn=dns,dc=ipa,dc=example")(targetfilter = "(objectclass=idnsConfigObject)")(version 3.0;acl "permission:System: Read DNS Configuration";allow (read) groupdn = "ldap:///cn=System: Read DNS Configuration,cn=permissions,cn=pbac,dc=ipa,dc=example";)
+aci: (targetattr = "createtimestamp || entryusn || idnsallowsyncptr || idnsforwarders || idnsforwardpolicy || idnspersistentsearch || idnszonerefresh || ipadnsversion || modifytimestamp || objectclass")(target = "ldap:///cn=dns,dc=ipa,dc=example")(targetfilter = "(objectclass=idnsConfigObject)")(version 3.0;acl "permission:System: Read DNS Configuration";allow (read) groupdn = "ldap:///cn=System: Read DNS Configuration,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: dc=ipa,dc=example
 aci: (targetattr = "idnsallowsyncptr || idnsforwarders || idnsforwardpolicy || idnspersistentsearch || idnszonerefresh")(target = "ldap:///cn=dns,dc=ipa,dc=example")(targetfilter = "(objectclass=idnsConfigObject)")(version 3.0;acl "permission:System: Write DNS Configuration";allow (write) groupdn = "ldap:///cn=System: Write DNS Configuration,cn=permissions,cn=pbac,dc=ipa,dc=example";)
 dn: dc=ipa,dc=example
diff --git a/install/share/60ipadns.ldif b/install/share/60ipadns.ldif
index e0ed0ab86..71b99d4d0 100644
--- a/install/share/60ipadns.ldif
+++ b/install/share/60ipadns.ldif
@@ -70,9 +70,11 @@ attributeTypes: ( 2.16.840.1.113730.3.8.5.25 NAME 'idnsSecKeyRevoke' DESC 'DNSKE
 attributeTypes: ( 2.16.840.1.113730.3.8.5.26 NAME 'idnsSecKeySep' DESC 'DNSKEY SEP flag (equivalent to bit 15): RFC 4035' EQUALITY booleanMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.7 SINGLE-VALUE X-ORIGIN 'IPA v4.1' )
 attributeTypes: ( 2.16.840.1.113730.3.8.5.27 NAME 'idnsSecAlgorithm' DESC 'DNSKEY algorithm: string used as mnemonic' EQUALITY caseIgnoreIA5Match SUBSTR caseIgnoreIA5SubstringsMatch SYNTAX 1.3.6.1.4.1.1466.115.121.1.26 SINGLE-VALUE X-ORIGIN 'IPA v4.1' )
 attributeTypes: ( 2.16.840.1.113730.3.8.5.28 NAME 'idnsSecKeyRef' DESC 'PKCS#11 URI of the key' EQUALITY caseExactMatch SINGLE-VALUE SYNTAX 1.3.6.1.4.1.1466.115.121.1.15 X-ORIGIN 'IPA v4.1' )
+attributeTypes: ( 2.16.840.1.113730.3.8.11.74 NAME 'ipaDNSVersion' DESC 'IPA DNS data version' EQUALITY integerMatch ORDERING integerOrderingMatch SINGLE-VALUE SYNTAX 1.3.6.1.4.1.1466.115.121.1.27 X-ORIGIN 'IPA v4.3' )
 objectClasses: ( 2.16.840.1.113730.3.8.6.0 NAME 'idnsRecord' DESC 'dns Record, usually a host' SUP top STRUCTURAL MUST idnsName MAY ( cn $ idnsAllowDynUpdate $ dNSTTL $ dNSClass $ aRecord $ aAAARecord $ a6Record $ nSRecord $ cNAMERecord $ pTRRecord $ sRVRecord $ tXTRecord $ mXRecord $ mDRecord $ hInfoRecord $ mInfoRecord $ aFSDBRecord $ SigRecord $ KeyRecord $ LocRecord $ nXTRecord $ nAPTRRecord $ kXRecord $ certRecord $ dNameRecord $ dSRecord $ sSHFPRecord $ rRSIGRecord $ nSECRecord $ DLVRecord $ TLSARecord $ UnknownRecord $ RPRecord $ APLRecord $ IPSECKEYRecord $ DHCIDRecord $ HIPRecord $ SPFRecord ) )
 objectClasses: ( 2.16.840.1.113730.3.8.6.1 NAME 'idnsZone' DESC 'Zone class' SUP idnsRecord STRUCTURAL MUST ( idnsZoneActive $ idnsSOAmName $ idnsSOArName $ idnsSOAserial $ idnsSOArefresh $ idnsSOAretry $ idnsSOAexpire $ idnsSOAminimum ) MAY ( idnsUpdatePolicy $ idnsAllowQuery $ idnsAllowTransfer $ idnsAllowSyncPTR $ idnsForwardPolicy $ idnsForwarders $ idnsSecInlineSigning $ nSEC3PARAMRecord ) )
 objectClasses: ( 2.16.840.1.113730.3.8.6.2 NAME 'idnsConfigObject' DESC 'DNS global config options' STRUCTURAL MAY ( idnsForwardPolicy $ idnsForwarders $ idnsAllowSyncPTR $ idnsZoneRefresh $ idnsPersistentSearch ) )
 objectClasses: ( 2.16.840.1.113730.3.8.12.18 NAME 'ipaDNSZone' SUP top AUXILIARY MUST idnsName MAY managedBy X-ORIGIN 'IPA v3' )
 objectClasses: ( 2.16.840.1.113730.3.8.6.3 NAME 'idnsForwardZone' DESC 'Forward Zone class' SUP top STRUCTURAL MUST ( idnsName $ idnsZoneActive ) MAY ( idnsForwarders $ idnsForwardPolicy ) )
 objectClasses: ( 2.16.840.1.113730.3.8.6.4 NAME 'idnsSecKey' DESC 'DNSSEC key metadata' STRUCTURAL MUST ( idnsSecKeyRef $ idnsSecKeyCreated $ idnsSecAlgorithm ) MAY ( idnsSecKeyPublish $ idnsSecKeyActivate $ idnsSecKeyInactive $ idnsSecKeyDelete $ idnsSecKeyZone $ idnsSecKeyRevoke $ idnsSecKeySep $ cn ) X-ORIGIN 'IPA v4.1' )
+objectClasses: ( 2.16.840.1.113730.3.8.12.36 NAME 'ipaDNSContainer' DESC 'IPA DNS container' AUXILIARY MUST ( ipaDNSVersion ) X-ORIGIN 'IPA v4.3' )
diff --git a/install/share/dns.ldif b/install/share/dns.ldif
index 42b41a8d7..d71e2ad7d 100644
--- a/install/share/dns.ldif
+++ b/install/share/dns.ldif
@@ -2,10 +2,10 @@ dn: cn=dns,$SUFFIX
 changetype: add
 objectClass: idnsConfigObject
 objectClass: nsContainer
-objectClass: ipaConfigObject
+objectClass: ipaDNSContainer
 objectClass: top
 cn: dns
-ipaConfigString: DNSVersion 1
+ipaDNSVersion: 1
 aci: (targetattr = "*")(version 3.0; acl "Allow read access"; allow (read,search,compare) groupdn = "ldap:///cn=Read DNS Entries,cn=permissions,cn=pbac,$SUFFIX" or userattr = "parent[0,1].managedby#GROUPDN";)
 aci: (target = "ldap:///idnsname=*,cn=dns,$SUFFIX")(version 3.0;acl "Add DNS entries in a zone";allow (add) userattr = "parent[1].managedby#GROUPDN";)
 aci: (target = "ldap:///idnsname=*,cn=dns,$SUFFIX")(version 3.0;acl "Remove DNS entries from a zone";allow (delete) userattr = "parent[1].managedby#GROUPDN";)
diff --git a/install/updates/40-dns.update b/install/updates/40-dns.update
index 9f64a2f70..4c0824b83 100644
--- a/install/updates/40-dns.update
+++ b/install/updates/40-dns.update
@@ -2,7 +2,6 @@
 # update DNS container
 dn: cn=dns, $SUFFIX
 addifexist: objectClass: idnsConfigObject
-addifexist: objectClass: ipaConfigObject
 addifexist: aci:(target = "ldap:///idnsname=*,cn=dns,$SUFFIX")(version 3.0;acl "Add DNS entries in a zone";allow (add) userattr = "parent[1].managedby#GROUPDN";)
 addifexist: aci:(target = "ldap:///idnsname=*,cn=dns,$SUFFIX")(version 3.0;acl "Remove DNS entries from a zone";allow (delete) userattr = "parent[1].managedby#GROUPDN";)
 addifexist: aci:(targetattr = "a6record || aaaarecord || afsdbrecord || aplrecord || arecord || certrecord || cn || cnamerecord || dhcidrecord || dlvrecord || dnamerecord || dnsclass || dnsttl || dsrecord || hinforecord || hiprecord || idnsallowdynupdate || idnsallowquery || idnsallowsyncptr || idnsallowtransfer || idnsforwarders || idnsforwardpolicy || idnsname || idnssecinlinesigning || idnssoaexpire || idnssoaminimum || idnssoamname || idnssoarefresh || idnssoaretry || idnssoarname || idnssoaserial || idnsupdatepolicy || idnszoneactive || ipseckeyrecord || keyrecord || kxrecord || locrecord || mdrecord || minforecord || mxrecord || naptrrecord || nsecrecord || nsec3paramrecord || nsrecord || nxtrecord || ptrrecord || rprecord || rrsigrecord || sigrecord || spfrecord || srvrecord || sshfprecord || tlsarecord || txtrecord || unknownrecord ")(target = "ldap:///idnsname=*,cn=dns,$SUFFIX")(version 3.0;acl "Update DNS entries in a zone";allow (write) userattr = "parent[0,1].managedby#GROUPDN";)
diff --git a/install/updates/90-post_upgrade_plugins.update b/install/updates/90-post_upgrade_plugins.update
index 9c9ee160f..d8498edd4 100644
--- a/install/updates/90-post_upgrade_plugins.update
+++ b/install/updates/90-post_upgrade_plugins.update
@@ -3,6 +3,7 @@
 
 # middle
 plugin: update_ca_topology
+plugin: update_ipaconfigstring_dnsversion_to_ipadnsversion
 plugin: update_dnszones
 plugin: update_dns_limits
 plugin: update_sigden_extdom_broken_config
diff --git a/ipalib/plugins/dns.py b/ipalib/plugins/dns.py
index 32319dc0f..ef0d9d819 100644
--- a/ipalib/plugins/dns.py
+++ b/ipalib/plugins/dns.py
@@ -4365,6 +4365,9 @@ class dnsconfig(LDAPObject):
             cli_name='zone_refresh',
             label=_('Zone refresh interval'),
         ),
+        Int('ipadnsversion?',  # available only in installer/upgrade
+            label=_('IPA DNS version'),
+        ),
     )
     managed_permissions = {
         'System: Write DNS Configuration': {
@@ -4391,7 +4394,7 @@ class dnsconfig(LDAPObject):
             'ipapermdefaultattr': {
                 'objectclass',
                 'idnsallowsyncptr', 'idnsforwarders', 'idnsforwardpolicy',
-                'idnspersistentsearch', 'idnszonerefresh'
+                'idnspersistentsearch', 'idnszonerefresh', 'ipadnsversion'
             },
             'default_privileges': {'DNS Administrators', 'DNS Servers'},
         },
@@ -4412,11 +4415,17 @@ class dnsconfig(LDAPObject):
             result['summary'] = unicode(_('Global DNS configuration is empty'))
 
 
-
 @register()
 class dnsconfig_mod(LDAPUpdate):
     __doc__ = _('Modify global DNS configuration.')
 
+    def get_options(self):
+        """hide ipadnsversion outside of installer/upgrade"""
+        for option in super(dnsconfig_mod, self).get_options():
+            if option.name == 'ipadnsversion':
+                option = option.clone(include=('installer', 'updates'))
+            yield option
+
     def interactive_prompt_callback(self, kw):
 
         # show informative message on client side
@@ -4480,6 +4489,7 @@ class dnsconfig_show(LDAPRetrieve):
         return result
 
 
+
 @register()
 class dnsforwardzone(DNSZoneBase):
     """
diff --git a/ipaserver/install/plugins/dns.py b/ipaserver/install/plugins/dns.py
index 7bd2b2697..cb3fad4c1 100644
--- a/ipaserver/install/plugins/dns.py
+++ b/ipaserver/install/plugins/dns.py
@@ -32,6 +32,61 @@ from ipapython.ipa_log_manager import root_logger
 register = Registry()
 
 
+class DNSUpdater(Updater):
+    def version_update_needed(self, target_version):
+        """Test if IPA DNS version is smaller than target version."""
+        assert isinstance(target_version, int)
+
+        try:
+            return int(self.api.Command['dnsconfig_show'](
+                all=True)['result']['ipadnsversion'][0]) < target_version
+        except errors.NotFound:
+            # IPA DNS is not configured
+            return False
+
+
+@register()
+class update_ipaconfigstring_dnsversion_to_ipadnsversion(Updater):
+    """
+    IPA <= 4.3.1 used ipaConfigString "DNSVersion 1" on DNS container.
+    This was hard to deal with in API so from IPA 4.3.2 we are using
+    new ipaDNSVersion attribute with integer syntax.
+    Old ipaConfigString is left there for now so if someone accidentally
+    executes upgrade on an old replica again it will not re-upgrade the data.
+    """
+    def execute(self, **options):
+        ldap = self.api.Backend.ldap2
+        dns_container_dn = DN(self.api.env.container_dns, self.api.env.basedn)
+        try:
+            container_entry = ldap.get_entry(dns_container_dn)
+        except errors.NotFound:
+            # DNS container not found, nothing to upgrade
+            return False, []
+
+        if 'ipadnscontainer' in [
+            o.lower() for o in container_entry['objectclass']
+        ]:
+            # version data are already migrated
+            return False, []
+
+        self.log.debug('Migrating DNS ipaConfigString to ipaDNSVersion')
+        container_entry['objectclass'].append('ipadnscontainer')
+        version = 0
+        for config_option in container_entry.get("ipaConfigString", []):
+            matched = re.match("^DNSVersion\s+(?P<version>\d+)$",
+                               config_option, flags=re.I)
+            if matched:
+                version = int(matched.group("version"))
+            else:
+                self.log.error(
+                    'Failed to parse DNS version from ipaConfigString, '
+                    'defaulting to version %s', version)
+        container_entry['ipadnsversion'] = version
+        ldap.update_entry(container_entry)
+        self.log.debug('ipaDNSVersion = %s', version)
+        return False, []
+
+
 @register()
 class update_dnszones(Updater):
     """
@@ -141,7 +196,7 @@ class update_dns_limits(Updater):
 
 
 @register()
-class update_master_to_dnsforwardzones(Updater):
+class update_master_to_dnsforwardzones(DNSUpdater):
     """
     Update all zones to meet requirements in the new FreeIPA versions
 
@@ -158,26 +213,14 @@ class update_master_to_dnsforwardzones(Updater):
     def execute(self, **options):
         ldap = self.api.Backend.ldap2
         # check LDAP if forwardzones already uses new semantics
-        dns_container_dn = DN(self.api.env.container_dns, self.api.env.basedn)
-        try:
-            container_entry = ldap.get_entry(dns_container_dn)
-        except errors.NotFound:
-            # DNS container not found, nothing to upgrade
+        if not self.version_update_needed(target_version=1):
+            # forwardzones already uses new semantics,
+            # no upgrade is required
             return False, []
 
-        for config_option in container_entry.get("ipaConfigString", []):
-            matched = re.match("^DNSVersion\s+(?P<version>\d+)$",
-                               config_option, flags=re.I)
-            if matched and int(matched.group("version")) >= 1:
-                # forwardzones already uses new semantics,
-                # no upgrade is required
-                return False, []
-
         self.log.debug('Updating forward zones')
         # update the DNSVersion, following upgrade can be executed only once
-        container_entry.setdefault(
-            'ipaConfigString', []).append(u"DNSVersion 1")
-        ldap.update_entry(container_entry)
+        self.api.Command['dnsconfig_mod'](ipadnsversion=1)
 
         # Updater in IPA version from 4.0 to 4.1.2 doesn't work well, this
         # should detect if update in past has been executed, and set proper