Overhaul bind upgrade process

/etc/named.conf is now owned by IPA. The file is overwritten on
installation and all subsequent updates. All user modification will be
lost. Config file creation and update use the same code paths.

This simplifies upgrade process a lot. There is no errprone fiddling
with config settings any more.

During upgrade there is a one-time backup of named.conf to
named.conf.ipa-backup. It allows users to salvage their customization
and move them to one of two user config files which are included by
named.conf.

Signed-off-by: Christian Heimes <cheimes@redhat.com>
Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>

freeipa.spec.in

@@ -1227,8 +1227,6 @@ fi
 %{_usr}/share/ipa/*.ldif
 %{_usr}/share/ipa/*.uldif
 %{_usr}/share/ipa/*.template
-%{_usr}/share/ipa/bind.ipa-ext.conf
-%{_usr}/share/ipa/bind.ipa-options-ext.conf.template
 %dir %{_usr}/share/ipa/advise
 %dir %{_usr}/share/ipa/advise/legacy
 %{_usr}/share/ipa/advise/legacy/*.template

install/share/Makefile.am

@@ -45,7 +45,7 @@ dist_app_DATA =				\
 	domainlevel.ldif			\
 	kerberos.ldif			\
 	indices.ldif			\
-	bind.ipa-ext.conf		\
+	bind.ipa-ext.conf.template		\
 	bind.ipa-options-ext.conf.template	\
 	bind.named.conf.template	\
 	certmap.conf.template		\

install/share/bind.ipa-ext.conf

@@ -1,11 +0,0 @@
-// Custom managed file.
-// Here you can set your own options, for instance ACL for recursion access:
-//
-// acl "trusted_network" {
-//   localnets;
-//   localhost;
-//   234.234.234.0/24;
-//   2001::co:ffee:babe:1/48;
-// };
-//
-// This file will NOT be overridden during updates!

install/share/bind.ipa-ext.conf.template

@@ -0,0 +1,16 @@
+/* User customization for BIND named
+ *
+ * This file is included in $NAMED_CONF and is not modified during IPA
+ * upgrades.
+ *
+ * "options" settings must be configured in $NAMED_CUSTOM_OPTIONS_CONF.
+ *
+ * Example: ACL for recursion access:
+ *
+ * acl "trusted_network" {
+ *   localnets;
+ *   localhost;
+ *   234.234.234.0/24;
+ *   2001::co:ffee:babe:1/48;
+ * };
+ */

install/share/bind.ipa-options-ext.conf.template

@@ -1,12 +1,17 @@
-// Custom managed file.
+/* User customization for BIND named
-// Here you can set your own options included inside the options stanza:
+ *
-//
+ * This file is included in $NAMED_CONF and is not modified during IPA
-// allow-recursion { trusted_network; };
+ * upgrades.
-// allow-query-cache { trusted_network; };
+ *
-//
+ * It must only contain "options" settings. Any other setting must be
-// This file will NOT be overridden during updates!
+ * configured in $NAMED_CUSTOM_CONF.
+ *
+ * Examples:
+ * allow-recursion { trusted_network; };
+ * allow-query-cache { trusted_network; };
+ */
 
-// turns on IPv6 for port 53, IPv4 is on by default for all ifaces
+/* turns on IPv6 for port 53, IPv4 is on by default for all ifaces */
 listen-on-v6 { any; };
 
 /* dnssec-enable is obsolete and 'yes' by default */

install/share/bind.named.conf.template

@@ -1,5 +1,10 @@
-/* WARNING: This part of the config file is IPA-managed.
+/* WARNING: This config file is managed by IPA.
- * Modifications may break IPA setup or upgrades.
+ *
+ * DO NOT MODIFY! Any modification will be overwritten by upgrades.
+ *
+ *
+ * - $NAMED_CUSTOM_OPTIONS_CONF (for options)
+ * - $NAMED_CUSTOM_CONF (all other settings)
  */
 
 options {
@@ -9,19 +14,15 @@ options {
 	statistics-file		"${NAMED_DATA_DIR}named_stats.txt";
 	memstatistics-file	"${NAMED_DATA_DIR}named_mem_stats.txt";
 
-	// If not explicitly set, the ACLs for "allow-query-cache" and
-	// "allow-recursion" are set to "localnets; localhost;".
-	// If either "allow-query-cache" or "allow-recursion" is set,
-	// the other would be set the same value.
-	// Please refer to $CUSTOM_OPTIONS_CONFIG
-	// for more informations
-	include "$CUSTOM_OPTIONS_CONFIG";
-
 	tkey-gssapi-keytab "$NAMED_KEYTAB";
+
 	pid-file "$NAMED_PID";
 
 	managed-keys-directory "$MANAGED_KEYS_DIR";
 
+	/* user customizations of options */
+	include "$NAMED_CUSTOM_OPTIONS_CONF";
+
 	/* crypto policy snippet on platforms with system-wide policy. */
 	$INCLUDE_CRYPTO_POLICY
 };
@@ -46,15 +47,14 @@ ${NAMED_ZONE_COMMENT}};
 include "$RFC1912_ZONES";
 include "$ROOT_KEY";
 
-/* custom configuration snippet */
+/* user customization */
-include "$CUSTOM_CONFIG";
+include "$NAMED_CUSTOM_CONF";
 
 dyndb "ipa" "$BIND_LDAP_SO" {
 	uri "ldapi://%2fvar%2frun%2fslapd-$SERVER_ID.socket";
-	base "cn=dns, $SUFFIX";
+	base "cn=dns,$SUFFIX";
 	server_id "$FQDN";
 	auth_method "sasl";
 	sasl_mech "GSSAPI";
 	sasl_user "DNS/$FQDN";
 };
-/* End of IPA-managed part. */

ipaplatform/base/paths.py

@@ -79,11 +79,14 @@ class BasePathNamespace:
     LDAP_CONF = "/etc/ldap.conf"
     LIBNSS_LDAP_CONF = "/etc/libnss-ldap.conf"
     NAMED_CONF = "/etc/named.conf"
-    NAMED_CUSTOM_CONFIG = "/etc/named/ipa-ext.conf"
+    NAMED_CONF_BAK = "/etc/named.conf.ipa-backup"
-    NAMED_CUSTOM_OPTIONS_CONFIG = "/etc/named/ipa-options-ext.conf"
+    NAMED_CUSTOM_CONF = "/etc/named/ipa-ext.conf"
-    NAMED_CUSTOM_CFG_SRC = '/usr/share/ipa/bind.ipa-ext.conf'
+    NAMED_CUSTOM_OPTIONS_CONF = "/etc/named/ipa-options-ext.conf"
-    NAMED_CUSTOM_OPTIONS_CFG_SRC = \
+    NAMED_CONF_SRC = '/usr/share/ipa/bind.named.conf.template'
+    NAMED_CUSTOM_CONF_SRC = '/usr/share/ipa/bind.ipa-ext.conf.template'
+    NAMED_CUSTOM_OPTIONS_CONF_SRC = (
         '/usr/share/ipa/bind.ipa-options-ext.conf.template'
+    )
     NAMED_VAR_DIR = "/var/named"
     NAMED_KEYTAB = "/etc/named.keytab"
     NAMED_RFC1912_ZONES = "/etc/named.rfc1912.zones"

ipaplatform/debian/paths.py

@@ -33,8 +33,9 @@ class DebianPathNamespace(BasePathNamespace):
     OLD_IPA_KEYTAB = "/etc/apache2/ipa.keytab"
     HTTPD_PASSWORD_CONF = "/etc/apache2/password.conf"
     NAMED_CONF = "/etc/bind/named.conf"
-    NAMED_CUSTOM_CONFIG = "/etc/bind/ipa-ext.conf"
+    NAMED_CONF_BAK = "/etc/bind/named.conf.ipa-backup"
-    NAMED_CUSTOM_OPTIONS_CONFIG = "/etc/bind/ipa-options-ext.conf"
+    NAMED_CUSTOM_CONF = "/etc/bind/ipa-ext.conf"
+    NAMED_CUSTOM_OPTIONS_CONF = "/etc/bind/ipa-options-ext.conf"
     NAMED_VAR_DIR = "/var/cache/bind"
     NAMED_KEYTAB = "/etc/bind/named.keytab"
     NAMED_RFC1912_ZONES = "/etc/bind/named.conf.default-zones"

ipaplatform/suse/paths.py

@@ -25,7 +25,8 @@ class SusePathNamespace(BasePathNamespace):
     HTTPD_SSL_CONF = "/etc/apache2/conf.d/ssl.conf"
     HTTPD_SSL_SITE_CONF = "/etc/apache2/conf.d/ssl.conf"
     HTTPD_PASSWORD_CONF = "/etc/apache2/ipa/password.conf"
-    NAMED_CUSTOM_CONFIG = "/etc/named.d/ipa-ext.conf"
+    NAMED_CUSTOM_CONF = "/etc/named.d/ipa-ext.conf"
+    NAMED_CUSTOM_OPTIONS_CONF = "/etc/named.d/ipa-options-ext.conf"
     NAMED_VAR_DIR = "/var/lib/named"
     NAMED_MANAGED_KEYS_DIR = "/var/lib/named/dyn"
     IPA_P11_KIT = "/etc/pki/trust/ipa.p11-kit"

ipaserver/install/bindinstance.py

@@ -26,6 +26,7 @@ import os
 import pwd
 import netaddr
 import re
+import shutil
 import sys
 import time
 
@@ -295,29 +296,6 @@ def find_reverse_zone(ip_address, api=api):
     return None
 
 
-def named_add_ext_conf_file(src, dest, t_params=None):
-    """
-    Ensure included file is present, but don't override it.
-
-    :param src: String. Absolute path to source template
-    :param dest: String. Absolute path to destination
-    :param t_params: Dict. Parameters for source template
-    """
-    if t_params is None:
-        t_params = {}
-
-    if not os.path.exists(dest):
-        ipa_ext_txt = ipautil.template_file(src, t_params)
-        gid = pwd.getpwnam(constants.NAMED_USER).pw_gid
-
-        with open(dest, 'w') as ipa_ext:
-            os.fchmod(ipa_ext.fileno(), 0o640)
-            os.fchown(ipa_ext.fileno(), 0, gid)
-            ipa_ext.write(ipa_ext_txt)
-        return True
-    return False
-
-
 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)
@@ -672,36 +650,50 @@ class BindInstance(service.Service):
         self.dns_backup = DnsBackup(self)
         self.domain = None
         self.host = None
-        self.ip_addresses = []
+        self.ip_addresses = ()
-        self.forwarders = None
+        self.forwarders = ()
+        self.forward_policy = None
+        self.zonemgr = None
+        self.no_dnssec_validation = False
         self.sub_dict = None
-        self.reverse_zones = []
+        self.reverse_zones = ()
         self.named_regular = services.service('named-regular', api)
 
     suffix = ipautil.dn_attribute_property('_suffix')
 
     def setup(self, fqdn, ip_addresses, realm_name, domain_name, forwarders,
-              forward_policy, reverse_zones,
+              forward_policy, reverse_zones, zonemgr=None,
-              named_user=constants.NAMED_USER, zonemgr=None,
               no_dnssec_validation=False):
-        self.service_user = named_user
+        """Setup bindinstance for installation
-        self.fqdn = fqdn
+        """
+        self.setup_templating(
+            fqdn=fqdn,
+            realm_name=realm_name,
+            domain_name=domain_name,
+            no_dnssec_validation=no_dnssec_validation
+        )
         self.ip_addresses = ip_addresses
-        self.realm = realm_name
-        self.domain = domain_name
         self.forwarders = forwarders
         self.forward_policy = forward_policy
-        self.host = fqdn.split(".")[0]
-        self.suffix = ipautil.realm_to_suffix(self.realm)
         self.reverse_zones = reverse_zones
-        self.no_dnssec_validation=no_dnssec_validation
 
-        if not zonemgr:
+        if zonemgr is not None:
             self.zonemgr = 'hostmaster.%s' % normalize_zone(self.domain)
         else:
             self.zonemgr = normalize_zonemgr(zonemgr)
 
-        self.__setup_sub_dict()
+    def setup_templating(
+        self, fqdn, realm_name, domain_name, no_dnssec_validation=None
+    ):
+        """Setup bindinstance for templating
+        """
+        self.fqdn = fqdn
+        self.realm = realm_name
+        self.domain = domain_name
+        self.host = fqdn.split(".")[0]
+        self.suffix = ipautil.realm_to_suffix(self.realm)
+        self.no_dnssec_validation = no_dnssec_validation
+        self._setup_sub_dict()
 
     @property
     def host_domain(self):
@@ -765,7 +757,7 @@ class BindInstance(service.Service):
         self.step("adding NS record to the zones", self.__add_self_ns)
 
         self.step("setting up kerberos principal", self.__setup_principal)
-        self.step("setting up named.conf", self.__setup_named_conf)
+        self.step("setting up named.conf", self.setup_named_conf)
         self.step("setting up server configuration",
             self.__setup_server_configuration)
 
@@ -820,7 +812,52 @@ class BindInstance(service.Service):
         except Exception as e:
             logger.debug("Unable to mask named (%s)", e)
 
-    def __setup_sub_dict(self):
+    def _get_dnssec_validation(self):
+        """get dnssec-validation value
+
+        1) command line overwrite --no-dnssec-validation
+        2) setting dnssec-enabled or dnssec-validation from named.conf
+        3) "yes" by default
+
+        Note: The dnssec-enabled is deprecated and defaults to "yes". If the
+        setting is "no", then it is migrated as "dnssec-validation no".
+        """
+        dnssec_validation = "yes"
+        if self.no_dnssec_validation:
+            # command line overwrite
+            logger.debug(
+                "dnssec-validation 'no' command line overwrite"
+            )
+            dnssec_validation = "no"
+        elif os.path.isfile(paths.NAMED_CONF):
+            # get prev_ value from /etc/named.conf
+            prev_dnssec_validation = named_conf_get_directive(
+                "dnssec-validation",
+                NAMED_SECTION_OPTIONS,
+                str_val=False
+            )
+            prev_dnssec_enable = named_conf_get_directive(
+                "dnssec-enable",
+                NAMED_SECTION_OPTIONS,
+                str_val=False
+            )
+            if prev_dnssec_validation == "no" or prev_dnssec_enable == "no":
+                logger.debug(
+                    "Setting dnssec-validation 'no' from existing %s",
+                    paths.NAMED_CONF
+                )
+                logger.debug(
+                    "dnssec-enabled was %s (None is yes)", prev_dnssec_enable
+                )
+                logger.debug(
+                    "dnssec-validation was %s", prev_dnssec_validation
+                )
+                dnssec_validation = "no"
+        assert dnssec_validation in {"yes", "no"}
+        logger.info("dnssec-validation %s", dnssec_validation)
+        return dnssec_validation
+
+    def _setup_sub_dict(self):
         if paths.NAMED_CRYPTO_POLICY_FILE is not None:
             crypto_policy = 'include "{}";'.format(
                 paths.NAMED_CRYPTO_POLICY_FILE
@@ -840,10 +877,12 @@ class BindInstance(service.Service):
             NAMED_VAR_DIR=paths.NAMED_VAR_DIR,
             BIND_LDAP_SO=paths.BIND_LDAP_SO,
             INCLUDE_CRYPTO_POLICY=crypto_policy,
-            CUSTOM_CONFIG=paths.NAMED_CUSTOM_CONFIG,
+            NAMED_CONF=paths.NAMED_CONF,
-            CUSTOM_OPTIONS_CONFIG=paths.NAMED_CUSTOM_OPTIONS_CONFIG,
+            NAMED_CUSTOM_CONF=paths.NAMED_CUSTOM_CONF,
+            NAMED_CUSTOM_OPTIONS_CONF=paths.NAMED_CUSTOM_OPTIONS_CONF,
             NAMED_DATA_DIR=constants.NAMED_DATA_DIR,
             NAMED_ZONE_COMMENT=constants.NAMED_ZONE_COMMENT,
+            NAMED_DNSSEC_VALIDATION=self._get_dnssec_validation(),
         )
 
     def __setup_dns_container(self):
@@ -986,37 +1025,75 @@ class BindInstance(service.Service):
                             dns_principal, str(e))
             raise
 
-    def __setup_named_conf(self):
+    def setup_named_conf(self, backup=False):
+        """Create, update, or migrate named configuration files
+
+        The method is used by installer and upgrade process. The named.conf
+        is backed up the first time and overwritten every time. The user
+        specific config files are created once and not modified in subsequent
+        calls.
+
+        The "dnssec-validation" option is migrated
+
+        :returns: True if any config file was modified, else False
+        """
+        # files are owned by root:named and are readable by user and group
+        uid = 0
+        gid = pwd.getpwnam(constants.NAMED_USER).pw_gid
+        mode = 0o640
+
+        changed = False
+
         if not self.fstore.has_file(paths.NAMED_CONF):
             self.fstore.backup_file(paths.NAMED_CONF)
 
-        named_txt = ipautil.template_file(
+        # named.conf
-            os.path.join(paths.USR_SHARE_IPA_DIR, "bind.named.conf.template"),
+        txt = ipautil.template_file(
-            self.sub_dict)
+            os.path.join(paths.NAMED_CONF_SRC), self.sub_dict
-
-        gid = pwd.getpwnam(constants.NAMED_USER).pw_gid
-        with open(paths.NAMED_CONF, 'w') as named_conf:
-            os.fchmod(named_conf.fileno(), 0o640)
-            os.fchown(named_conf.fileno(), 0, gid)
-            named_conf.write(named_txt)
-
-        named_add_ext_conf_file(
-            paths.NAMED_CUSTOM_CFG_SRC,
-            paths.NAMED_CUSTOM_CONFIG
         )
+        with open(paths.NAMED_CONF) as f:
+            old_txt = f.read()
+        if txt == old_txt:
+            logger.debug("%s is unmodified", paths.NAMED_CONF)
+        else:
+            if backup:
+                if not os.path.isfile(paths.NAMED_CONF_BAK):
+                    shutil.copyfile(paths.NAMED_CONF, paths.NAMED_CONF_BAK)
+                    logger.info("created backup %s", paths.NAMED_CONF_BAK)
+                else:
+                    logger.warning(
+                        "backup %s already exists", paths.NAMED_CONF_BAK
+                    )
 
-        dnssec_validation = 'no' if self.no_dnssec_validation else 'yes'
+            with open(paths.NAMED_CONF, "w") as f:
-        named_add_ext_conf_file(
+                os.fchmod(f.fileno(), mode)
-            paths.NAMED_CUSTOM_OPTIONS_CFG_SRC,
+                os.fchown(f.fileno(), uid, gid)
-            paths.NAMED_CUSTOM_OPTIONS_CONFIG,
+                f.write(txt)
-            {'NAMED_DNSSEC_VALIDATION': dnssec_validation}
-        )
 
-        # prevent repeated upgrade on new installs
+            logger.info("created new %s", paths.NAMED_CONF)
-        sysupgrade.set_upgrade_state(
+            changed = True
-            'named.conf',
+
-            'forward_policy_conflict_with_empty_zones_handled', True
+        # user configurations
+        user_configs = (
+            (paths.NAMED_CUSTOM_CONF_SRC, paths.NAMED_CUSTOM_CONF),
+            (
+                paths.NAMED_CUSTOM_OPTIONS_CONF_SRC,
+                paths.NAMED_CUSTOM_OPTIONS_CONF
+            )
         )
+        for src, dest in user_configs:
+            if not os.path.exists(dest):
+                txt = ipautil.template_file(src, self.sub_dict)
+                with open(dest, "w") as f:
+                    os.fchmod(f.fileno(), mode)
+                    os.fchown(f.fileno(), uid, gid)
+                    f.write(txt)
+                logger.info("created named user config '%s'", dest)
+                changed = True
+            else:
+                logger.info("named user config '%s' already exists", dest)
+
+        return changed
 
     def __setup_server_configuration(self):
         ensure_dnsserver_container_exists(api.Backend.ldap2, self.api)
@@ -1267,7 +1344,8 @@ class BindInstance(service.Service):
         if named_regular_running:
             self.named_regular.start()
 
-        ipautil.remove_file(paths.NAMED_CUSTOM_CONFIG)
+        ipautil.remove_file(paths.NAMED_CONF_BAK)
-        ipautil.remove_file(paths.NAMED_CUSTOM_OPTIONS_CONFIG)
+        ipautil.remove_file(paths.NAMED_CUSTOM_CONF)
+        ipautil.remove_file(paths.NAMED_CUSTOM_OPTIONS_CONF)
         ipautil.remove_keytab(self.keytab)
         ipautil.remove_ccache(run_as=self.service_user)

ipaserver/install/ipa_backup.py

@@ -124,8 +124,8 @@ class Backup(admintool.AdminTool):
 
     files = (
         paths.NAMED_CONF,
-        paths.NAMED_CUSTOM_CONFIG,
+        paths.NAMED_CUSTOM_CONF,
-        paths.NAMED_CUSTOM_OPTIONS_CONFIG,
+        paths.NAMED_CUSTOM_OPTIONS_CONF,
         paths.NAMED_KEYTAB,
         paths.RESOLV_CONF,
         paths.SYSCONFIG_PKI_TOMCAT,
@@ -186,9 +186,6 @@ class Backup(admintool.AdminTool):
         paths.OPENDNSSEC_KASP_DB,
         paths.DNSSEC_SOFTHSM2_CONF,
         paths.DNSSEC_SOFTHSM_PIN_SO,
-        paths.NAMED_CONF,
-        paths.NAMED_CUSTOM_CONFIG,
-        paths.NAMED_CUSTOM_OPTIONS_CONFIG,
         paths.IPA_ODS_EXPORTER_KEYTAB,
         paths.IPA_DNSKEYSYNCD_KEYTAB,
         paths.IPA_CUSTODIA_KEYS,

ipaserver/install/server/upgrade.py

@@ -19,7 +19,6 @@ import sys
 import tempfile
 from contextlib import contextmanager
 from augeas import Augeas
-import dns.exception
 
 from ipalib import api, x509
 from ipalib.constants import RENEWAL_CA_NAME, RA_AGENT_PROFILE, IPA_CA_RECORD
@@ -33,7 +32,7 @@ from ipaplatform import services
 from ipaplatform.tasks import tasks
 from ipapython import ipautil, version
 from ipapython import ipaldap
-from ipapython import dnsutil, directivesetter
+from ipapython import directivesetter
 from ipapython.dn import DN
 from ipaplatform.constants import constants
 from ipaplatform.paths import paths
@@ -526,565 +525,6 @@ def ca_initialize_hsm_state(ca):
         ca.set_hsm_state(config)
 
 
-def named_remove_deprecated_options():
-    """
-    From IPA 3.3, persistent search is a default mechanism for new DNS zone
-    detection.
-
-    Remove psearch, zone_refresh cache_ttl and serial_autoincrement options,
-    as they have been deprecated in bind-dyndb-ldap configuration file.
-
-    When some change in named.conf is done, this functions returns True.
-    """
-
-    logger.info('[Removing deprecated DNS configuration options]')
-
-    if not bindinstance.named_conf_exists():
-        # DNS service may not be configured
-        logger.info('DNS is not configured')
-        return False
-
-    deprecated_options = ['zone_refresh', 'psearch', 'cache_ttl',
-                          'serial_autoincrement']
-    removed_options = []
-
-    try:
-        # Remove all the deprecated options
-        for option in deprecated_options:
-            value = bindinstance.named_conf_get_directive(option)
-
-            if value is not None:
-                bindinstance.named_conf_set_directive(option, None)
-                removed_options.append(option)
-
-    except IOError as e:
-        logger.error('Cannot modify DNS configuration in %s: %s',
-                     paths.NAMED_CONF, e)
-
-    # Log only the changed options
-    if not removed_options:
-        logger.debug('No changes made')
-        return False
-
-    logger.debug('The following configuration options have been removed: %s',
-                 ', '.join(removed_options))
-    return True
-
-
-def named_add_ipa_ext_conf_include():
-    """
-    Ensures named.conf does include the ipa-ext.conf file
-    """
-    if not bindinstance.named_conf_exists():
-        logger.info('DNS is not configured.')
-        return False
-
-    if not bindinstance.named_conf_include_exists(paths.NAMED_CUSTOM_CONFIG):
-        bindinstance.named_conf_add_include(paths.NAMED_CUSTOM_CONFIG)
-        return True
-    return False
-
-
-def named_add_ipa_ext_conf_file():
-    """
-    Wrapper around bindinstance.named_add_ext_conf_file().
-    Ensures named is configured before pushing the file.
-    """
-    if not bindinstance.named_conf_exists():
-        logger.info('DNS is not configured.')
-        return False
-
-    # migrate value from named.conf
-    dnssec_validation = bindinstance.named_conf_get_directive(
-        "dnssec-validation",
-        bindinstance.NAMED_SECTION_OPTIONS,
-        str_val=False
-    )
-    if dnssec_validation is None:
-        dnssec_validation = "yes"
-
-    tasks = [
-        bindinstance.named_add_ext_conf_file(
-            paths.NAMED_CUSTOM_CFG_SRC,
-            paths.NAMED_CUSTOM_CONFIG
-        ),
-        bindinstance.named_add_ext_conf_file(
-            paths.NAMED_CUSTOM_OPTIONS_CFG_SRC,
-            paths.NAMED_CUSTOM_OPTIONS_CONFIG,
-            dict(
-                NAMED_DNSSEC_VALIDATION=dnssec_validation
-            )
-        )
-    ]
-
-    return any(tasks)
-
-
-def named_set_minimum_connections():
-    """
-    Sets the minimal number of connections.
-
-    When some change in named.conf is done, this functions returns True.
-    """
-
-    changed = False
-
-    logger.info('[Ensuring minimal number of connections]')
-
-    if not bindinstance.named_conf_exists():
-        # DNS service may not be configured
-        logger.info('DNS is not configured')
-        return changed
-
-    # make sure number of connections is right
-    minimum_connections = 4
-
-    try:
-        connections = bindinstance.named_conf_get_directive('connections')
-    except IOError as e:
-        logger.debug('Cannot retrieve connections option from %s: %s',
-                     paths.NAMED_CONF, e)
-        return changed
-
-    try:
-        if connections is not None:
-            connections = int(connections)
-    except ValueError:
-        # this should not happend, but there is some bad value in
-        # "connections" option, bail out
-        pass
-    else:
-        if connections is not None and connections < minimum_connections:
-            try:
-                bindinstance.named_conf_set_directive('connections',
-                                                       minimum_connections)
-                logger.debug('Connections set to %d', minimum_connections)
-            except IOError as e:
-                logger.error('Cannot update connections in %s: %s',
-                             paths.NAMED_CONF, e)
-            else:
-                changed = True
-
-    if not changed:
-        logger.debug('No changes made')
-
-    return changed
-
-
-def named_update_gssapi_configuration():
-    """
-    Update GSSAPI configuration in named.conf to a recent API.
-    tkey-gssapi-credential and tkey-domain is replaced with tkey-gssapi-keytab.
-    Details can be found in https://fedorahosted.org/freeipa/ticket/3429.
-
-    When some change in named.conf is done, this functions returns True
-    """
-
-    logger.info('[Updating GSSAPI configuration in DNS]')
-
-    if not bindinstance.named_conf_exists():
-        # DNS service may not be configured
-        logger.info('DNS is not configured')
-        return False
-
-    if sysupgrade.get_upgrade_state('named.conf', 'gssapi_updated'):
-        logger.debug('Skip GSSAPI configuration check')
-        return False
-
-    try:
-        gssapi_keytab = bindinstance.named_conf_get_directive(
-            'tkey-gssapi-keytab', bindinstance.NAMED_SECTION_OPTIONS)
-    except IOError as e:
-        logger.error('Cannot retrieve tkey-gssapi-keytab option from %s: %s',
-                     paths.NAMED_CONF, e)
-        return False
-    else:
-        if gssapi_keytab:
-            logger.debug('GSSAPI configuration already updated')
-            sysupgrade.set_upgrade_state('named.conf', 'gssapi_updated', True)
-            return False
-
-    try:
-        tkey_credential = bindinstance.named_conf_get_directive('tkey-gssapi-credential',
-                bindinstance.NAMED_SECTION_OPTIONS)
-        tkey_domain = bindinstance.named_conf_get_directive('tkey-domain',
-                bindinstance.NAMED_SECTION_OPTIONS)
-    except IOError as e:
-        logger.error('Cannot retrieve tkey-gssapi-credential option from %s: '
-                     '%s',
-                     paths.NAMED_CONF, e)
-        return False
-
-    if not tkey_credential or not tkey_domain:
-        logger.error('Either tkey-gssapi-credential or tkey-domain is missing '
-                     'in %s. Skip update.', paths.NAMED_CONF)
-        return False
-
-    try:
-        bindinstance.named_conf_set_directive(
-            'tkey-gssapi-credential', None,
-            bindinstance.NAMED_SECTION_OPTIONS)
-        bindinstance.named_conf_set_directive(
-            'tkey-domain', None,
-            bindinstance.NAMED_SECTION_OPTIONS)
-        bindinstance.named_conf_set_directive(
-            'tkey-gssapi-keytab', paths.NAMED_KEYTAB,
-            bindinstance.NAMED_SECTION_OPTIONS)
-    except IOError as e:
-        logger.error('Cannot update GSSAPI configuration in %s: %s',
-                     paths.NAMED_CONF, e)
-        return False
-    else:
-        logger.debug('GSSAPI configuration updated')
-
-    sysupgrade.set_upgrade_state('named.conf', 'gssapi_updated', True)
-    return True
-
-
-def named_update_pid_file():
-    """
-    Make sure that named reads the pid file from the right file
-    """
-    logger.info('[Updating pid-file configuration in DNS]')
-
-    if not bindinstance.named_conf_exists():
-        # DNS service may not be configured
-        logger.info('DNS is not configured')
-        return False
-
-    if sysupgrade.get_upgrade_state('named.conf', 'pid-file_updated'):
-        logger.debug('Skip pid-file configuration check')
-        return False
-
-    try:
-        pid_file = bindinstance.named_conf_get_directive(
-            'pid-file', bindinstance.NAMED_SECTION_OPTIONS)
-    except IOError as e:
-        logger.error('Cannot retrieve pid-file option from %s: %s',
-                     paths.NAMED_CONF, e)
-        return False
-    else:
-        if pid_file:
-            logger.debug('pid-file configuration already updated')
-            sysupgrade.set_upgrade_state('named.conf', 'pid-file_updated', True)
-            return False
-
-    try:
-        bindinstance.named_conf_set_directive(
-            'pid-file', paths.NAMED_PID, bindinstance.NAMED_SECTION_OPTIONS)
-    except IOError as e:
-        logger.error('Cannot update pid-file configuration in %s: %s',
-                     paths.NAMED_CONF, e)
-        return False
-    else:
-        logger.debug('pid-file configuration updated')
-
-    sysupgrade.set_upgrade_state('named.conf', 'pid-file_updated', True)
-    return True
-
-
-def named_dnssec_enable():
-    """Remove obsolete dnssec-enable option from named.conf
-    """
-    if not bindinstance.named_conf_exists():
-        # DNS service may not be configured
-        logger.info('DNS is not configured')
-        return False
-
-    # old upgrade state when "dnssec-enabled yes" was added
-    sysupgrade.remove_upgrade_state("named.conf", "dnssec_enabled")
-
-    if sysupgrade.get_upgrade_state('named.conf', 'dnssec-enabled_remove'):
-        return False
-
-    # only remove when dnssec-enable is yes or not set.
-    # Official documentation says that "dnssec-enable yes;" is required and
-    # "dnssec-validation no;" should be used to disable validation.
-    # At least Bind 9.11+ has DNSSEC enabled by default.
-    enabled = bindinstance.named_conf_get_directive(
-        "dnssec-enable", bindinstance.NAMED_SECTION_OPTIONS, str_val=False
-    )
-    if enabled is not None and enabled != "yes":
-        logger.warning(
-            "[WARNING] Unable to remove obsolete 'dnssec-enable' option "
-            "from '%s' (dnssec-enabled %s;). Please remove the option "
-            "manually and set 'dnssec-validate no;' if you wish to disable "
-            "DNSSEC validation.",
-            paths.NAMED_CONF, enabled
-        )
-        return False
-
-    logger.info('[Removing obsolete "dnssec-enable" configuration]')
-    try:
-        bindinstance.named_conf_set_directive(
-            "dnssec-enable",
-            None,
-            bindinstance.NAMED_SECTION_OPTIONS,
-            str_val=False
-        )
-    except IOError as e:
-        logger.error(
-            'Cannot update dnssec-enable configuration in %s: %s',
-            paths.NAMED_CONF, e
-        )
-        return False
-    else:
-        logger.debug('Removed dnssec-enabled from %s', paths.NAMED_CONF)
-        sysupgrade.set_upgrade_state(
-            'named.conf', 'dnssec-enabled_remove', True
-        )
-    return True
-
-
-def named_validate_dnssec():
-    """dnssec-validation upgrade
-
-    The upgrade step used to add "dnssec-validation no" to named.conf IFF
-    named.conf did not contain "dnssec-validation" option at all. The
-    option has been moved to 'ipa-options-ext.conf' in IPA 4.8.7. Only remove
-    upgrade state.
-    """
-    if bindinstance.named_conf_exists():
-        sysupgrade.remove_upgrade_state(
-            'named.conf', 'dnssec_validation_upgraded'
-        )
-    return False
-
-
-def named_bindkey_file_option():
-    """Remove options bindkey_file to named.conf (4.8.7)
-
-    DNSSEC Lookaside Validation is deprecated and dlv.isc.org is shutting
-    down.
-
-    See: RFC 8749
-    See: https://pagure.io/freeipa/issue/8350
-    """
-    if not bindinstance.named_conf_exists():
-        # DNS service may not be configured
-        logger.info('DNS is not configured')
-        return False
-
-    # old upgrade state for "bindkey-file"
-    sysupgrade.remove_upgrade_state("named.conf", "bindkey-file_updated")
-
-    if sysupgrade.get_upgrade_state('named.conf', 'bindkey-file_removed'):
-        logger.debug('Skip bindkey-file configuration check')
-        return False
-
-    try:
-        bindkey_file = bindinstance.named_conf_get_directive(
-            'bindkey-file', bindinstance.NAMED_SECTION_OPTIONS)
-    except IOError as e:
-        logger.error('Cannot retrieve bindkey-file option from %s: %s',
-                     paths.NAMED_CONF, e)
-        return False
-    else:
-        if not bindkey_file:
-            logger.debug('bindkey-file configuration already removed')
-            sysupgrade.set_upgrade_state(
-                'named.conf', 'bindkey-file_removed', True
-            )
-            return False
-
-    logger.info('[Remove "bindkeys-file" option from named.conf]')
-    try:
-        bindinstance.named_conf_set_directive(
-            'bindkeys-file', None,
-            section=bindinstance.NAMED_SECTION_OPTIONS
-        )
-    except IOError as e:
-        logger.error('Cannot update bindkeys-file configuration in %s: %s',
-                     paths.NAMED_CONF, e)
-        return False
-    else:
-        sysupgrade.set_upgrade_state(
-            'named.conf', 'bindkey-file_removed', True
-        )
-    return True
-
-def named_managed_keys_dir_option():
-    """
-    Add options managed_keys_directory to named.conf
-    """
-    if not bindinstance.named_conf_exists():
-        # DNS service may not be configured
-        logger.info('DNS is not configured')
-        return False
-
-    if sysupgrade.get_upgrade_state('named.conf', 'managed-keys-directory_updated'):
-        logger.debug('Skip managed-keys-directory configuration check')
-        return False
-
-    try:
-        managed_keys = bindinstance.named_conf_get_directive('managed-keys-directory',
-                bindinstance.NAMED_SECTION_OPTIONS)
-    except IOError as e:
-        logger.error('Cannot retrieve managed-keys-directory option from %s: '
-                     '%s',
-                     paths.NAMED_CONF, e)
-        return False
-    else:
-        if managed_keys:
-            logger.debug('managed_keys_directory configuration already '
-                         'updated')
-            sysupgrade.set_upgrade_state('named.conf', 'managed-keys-directory_updated', True)
-            return False
-
-    logger.info('[Setting "managed-keys-directory" option in named.conf]')
-    try:
-        bindinstance.named_conf_set_directive('managed-keys-directory',
-                                              paths.NAMED_MANAGED_KEYS_DIR,
-                                              bindinstance.NAMED_SECTION_OPTIONS)
-    except IOError as e:
-        logger.error('Cannot update managed-keys-directory configuration in '
-                     '%s: %s',
-                     paths.NAMED_CONF, e)
-        return False
-
-
-    sysupgrade.set_upgrade_state('named.conf', 'managed-keys-directory_updated', True)
-    return True
-
-def named_root_key_include():
-    """
-    Add options managed_keys_directory to named.conf
-    """
-    if not bindinstance.named_conf_exists():
-        # DNS service may not be configured
-        logger.info('DNS is not configured')
-        return False
-
-    if sysupgrade.get_upgrade_state('named.conf', 'root_key_updated'):
-        logger.debug('Skip root key configuration check')
-        return False
-
-    try:
-        root_key = bindinstance.named_conf_include_exists(paths.NAMED_ROOT_KEY)
-    except IOError as e:
-        logger.error('Cannot check root key include in %s: %s',
-                     paths.NAMED_CONF, e)
-        return False
-    else:
-        if root_key:
-            logger.debug('root keys configuration already updated')
-            sysupgrade.set_upgrade_state('named.conf', 'root_key_updated', True)
-            return False
-
-    logger.info('[Including named root key in named.conf]')
-    try:
-        bindinstance.named_conf_add_include(paths.NAMED_ROOT_KEY)
-    except IOError as e:
-        logger.error('Cannot update named root key include in %s: %s',
-                     paths.NAMED_CONF, e)
-        return False
-
-
-    sysupgrade.set_upgrade_state('named.conf', 'root_key_updated', True)
-    return True
-
-
-def named_update_global_forwarder_policy():
-    bind = bindinstance.BindInstance()
-    if not bindinstance.named_conf_exists() or not bind.is_configured():
-        # DNS service may not be configured
-        logger.info('DNS is not configured')
-        return False
-
-    logger.info('[Checking global forwarding policy in named.conf '
-                'to avoid conflicts with automatic empty zones]')
-    if sysupgrade.get_upgrade_state(
-        'named.conf', 'forward_policy_conflict_with_empty_zones_handled'
-    ):
-        # upgrade was done already
-        return False
-
-    sysupgrade.set_upgrade_state(
-        'named.conf',
-        'forward_policy_conflict_with_empty_zones_handled',
-        True
-    )
-    try:
-        if not dnsutil.has_empty_zone_addresses(api.env.host):
-            # guess: local server does not have IP addresses from private
-            # ranges so hopefully automatic empty zones are not a problem
-            return False
-    except dns.exception.DNSException as ex:
-        logger.error(
-            'Skipping update of global DNS forwarder in named.conf: '
-            'Unable to determine if local server is using an '
-            'IP address belonging to an automatic empty zone. '
-            'Consider changing forwarding policy to "only". '
-            'DNS exception: %s', ex)
-        return False
-
-    if bindinstance.named_conf_get_directive(
-            'forward',
-            section=bindinstance.NAMED_SECTION_OPTIONS,
-            str_val=False
-    ) == 'only':
-        return False
-
-    logger.info('Global forward policy in named.conf will '
-                'be changed to "only" to avoid conflicts with '
-                'automatic empty zones')
-    bindinstance.named_conf_set_directive(
-        'forward',
-        'only',
-        section=bindinstance.NAMED_SECTION_OPTIONS,
-        str_val=False
-    )
-    return True
-
-
-def named_add_server_id():
-    """
-    DNS Locations feature requires to have configured server_id in IPA section
-    of named.conf
-    :return: if named.conf has been changed
-    """
-    bind = bindinstance.BindInstance()
-    if not bindinstance.named_conf_exists() or not bind.is_configured():
-        # DNS service may not be configured
-        logger.info('DNS is not configured')
-        return False
-
-    if sysupgrade.get_upgrade_state('named.conf', 'add_server_id'):
-        # upgrade was done already
-        return False
-
-    logger.info('[Adding server_id to named.conf]')
-    bindinstance.named_conf_set_directive('server_id', api.env.host)
-    sysupgrade.set_upgrade_state('named.conf', 'add_server_id', True)
-    return True
-
-
-def named_add_crypto_policy():
-    """Add crypto policy include
-    """
-    if not bindinstance.named_conf_exists():
-        logger.info('DNS is not configured')
-        return False
-
-    if sysupgrade.get_upgrade_state('named.conf', 'add_crypto_policy'):
-        # upgrade was done already
-        return False
-    policy_file = paths.NAMED_CRYPTO_POLICY_FILE
-    if policy_file is None:
-        # no crypto policy
-        return False
-
-    if bindinstance.named_conf_include_exists(policy_file):
-        sysupgrade.set_upgrade_state('named.conf', 'add_crypto_policy', True)
-        return False
-
-    logger.info('[Adding crypto policy include to named.conf]')
-    bindinstance.named_conf_set_directive(
-        'include', policy_file, section=bindinstance.NAMED_SECTION_OPTIONS
-    )
-    sysupgrade.set_upgrade_state('named.conf', 'add_crypto_policy', True)
-    return True
-
 
 def certificate_renewal_update(ca, kra, ds, http):
     """
@@ -1382,26 +822,24 @@ def ca_enable_pkix(ca):
     return True
 
 
-def add_ca_dns_records():
+def add_ca_dns_records(bind):
     logger.info('[Add missing CA DNS records]')
 
     if sysupgrade.get_upgrade_state('dns', 'ipa_ca_records'):
         logger.info('IPA CA DNS records already processed')
-        return
+        return False
 
     ret = api.Command['dns_is_enabled']()
     if not ret['result']:
         logger.info('DNS is not configured')
         sysupgrade.set_upgrade_state('dns', 'ipa_ca_records', True)
-        return
+        return False
-
-    bind = bindinstance.BindInstance()
 
     bind.remove_ipa_ca_cnames(api.env.domain)
-
     bind.update_system_records()
 
     sysupgrade.set_upgrade_state('dns', 'ipa_ca_records', True)
+    return True
 
 
 def find_subject_base():
@@ -1508,45 +946,6 @@ def uninstall_dogtag_9(ds, http):
     http.restart()
 
 
-def mask_named_regular():
-    """Disable named, we need to run only named-pkcs11, running both named and
-    named-pkcs can cause unexpected errors"""
-    if sysupgrade.get_upgrade_state('dns', 'regular_named_masked'):
-        return False
-
-    sysupgrade.set_upgrade_state('dns', 'regular_named_masked', True)
-
-    if bindinstance.named_conf_exists():
-        logger.info('[Masking named]')
-        named = services.service('named-regular', api)
-        try:
-            named.stop()
-        except Exception as e:
-            logger.warning('Unable to stop named service (%s)', e)
-
-        try:
-            named.mask()
-        except Exception as e:
-            logger.warning('Unable to mask named service (%s)', e)
-
-        return True
-
-    return False
-
-
-def fix_dyndb_ldap_workdir_permissions():
-    """Fix dyndb-ldap working dir permissions. DNSSEC daemons requires it"""
-    if sysupgrade.get_upgrade_state('dns', 'dyndb_ipa_workdir_perm'):
-        return
-
-    if bindinstance.named_conf_exists():
-        logger.info('[Fix bind-dyndb-ldap IPA working directory]')
-        dnskeysync = dnskeysyncinstance.DNSKeySyncInstance()
-        dnskeysync.set_dyndb_ldap_workdir_permissions()
-
-    sysupgrade.set_upgrade_state('dns', 'dyndb_ipa_workdir_perm', True)
-
-
 def fix_schema_file_syntax():
     """Fix syntax errors in schema files
 
@@ -1974,6 +1373,74 @@ def fix_permissions():
             os.chmod(filename, mode)
 
 
+def upgrade_bind(fstore):
+    """Update BIND named DNS server instance
+    """
+    bind = bindinstance.BindInstance(fstore, api=api)
+    bind.setup_templating(
+        fqdn=api.env.host,
+        realm_name=api.env.realm,
+        domain_name=api.env.domain
+    )
+
+    # always executed
+    add_ca_dns_records(bind)
+
+    if not bindinstance.named_conf_exists():
+        logger.info("DNS service is not configured")
+        return False
+
+    # get rid of old upgrade states
+    bind_old_upgrade_states()
+
+    if bind.is_configured() and not bind.is_running():
+        # some upgrade steps may require bind running
+        bind_started = True
+        bind.start()
+    else:
+        bind_started = False
+
+    try:
+        changed = bind.setup_named_conf(backup=True)
+        if changed:
+            logger.info("named.conf has been modified, restarting named")
+        try:
+            if bind.is_running():
+                bind.restart()
+        except ipautil.CalledProcessError as e:
+            logger.error("Failed to restart %s: %s", bind.service_name, e)
+    finally:
+        if bind_started:
+            bind.stop()
+
+    return changed
+
+
+def bind_old_upgrade_states():
+    """Remove old upgrade states
+    """
+    named_conf_states = (
+        # old states before 4.8.7
+        "gssapi_updated",
+        "pid-file_updated",
+        "dnssec-enabled_remove",
+        "bindkey-file_removed",
+        "managed-keys-directory_updated",
+        "root_key_updated",
+        "forward_policy_conflict_with_empty_zones_handled",
+        "add_server_id",
+        "add_crypto_policy",
+    )
+    dns_states = (
+        "regular_named_masked",
+        "dyndb_ipa_workdir_perm"
+    )
+    for state in named_conf_states:
+        sysupgrade.remove_upgrade_state("named.conf", state)
+    for state in dns_states:
+        sysupgrade.remove_upgrade_state("dns", state)
+
+
 def upgrade_configuration():
     """
     Execute configuration upgrade of the IPA services
@@ -2193,49 +1660,7 @@ def upgrade_configuration():
     cleanup_dogtag()
     upgrade_adtrust_config()
 
-    bind = bindinstance.BindInstance(fstore)
+    upgrade_bind(fstore)
-    if bind.is_configured() and not bind.is_running():
-        # some upgrade steps may require bind running
-        bind_started = True
-        bind.start()
-    else:
-        bind_started = False
-
-    add_ca_dns_records()
-
-    # Any of the following functions returns True iff the named.conf file
-    # has been altered
-    named_conf_changes = (
-                          named_remove_deprecated_options(),
-                          named_add_ipa_ext_conf_file(),
-                          named_add_ipa_ext_conf_include(),
-                          named_set_minimum_connections(),
-                          named_update_gssapi_configuration(),
-                          named_update_pid_file(),
-                          named_dnssec_enable(),
-                          named_validate_dnssec(),
-                          named_bindkey_file_option(),
-                          named_managed_keys_dir_option(),
-                          named_root_key_include(),
-                          named_update_global_forwarder_policy(),
-                          mask_named_regular(),
-                          fix_dyndb_ldap_workdir_permissions(),
-                          named_add_server_id(),
-                          named_add_crypto_policy(),
-                         )
-
-    if any(named_conf_changes):
-        # configuration has changed, restart the name server
-        logger.info('Changes to named.conf have been made, restart named')
-        bind = bindinstance.BindInstance(fstore)
-        try:
-            if bind.is_running():
-                bind.restart()
-        except ipautil.CalledProcessError as e:
-            logger.error("Failed to restart %s: %s", bind.service_name, e)
-
-    if bind_started:
-        bind.stop()
 
     custodia = custodiainstance.CustodiaInstance(api.env.host, api.env.realm)
     custodia.upgrade_instance()

ipatests/test_integration/test_installation.py

@@ -878,7 +878,7 @@ class TestInstallMasterDNS(IntegrationTest):
         related : https://pagure.io/freeipa/issue/8079
         """
         # check of /etc/named/ipa-ext.conf exist
-        assert self.master.transport.file_exists(paths.NAMED_CUSTOM_CONFIG)
+        assert self.master.transport.file_exists(paths.NAMED_CUSTOM_CONF)
 
         # check if /etc/named.conf does not contain 'allow-recursion { any; };'
         string_to_check = 'allow-recursion { any; };'
@@ -888,7 +888,7 @@ class TestInstallMasterDNS(IntegrationTest):
 
         # check if ipa-backup command backups the /etc/named/ipa-ext.conf
         result = self.master.run_command(['ipa-backup', '-v'])
-        assert paths.NAMED_CUSTOM_CONFIG in result.stderr_text
+        assert paths.NAMED_CUSTOM_CONF in result.stderr_text
 
     def test_install_kra(self):
         tasks.install_kra(self.master, first_instance=True)

ipatests/test_integration/test_upgrade.py

@@ -11,6 +11,7 @@ import os
 import io
 
 from cryptography.hazmat.primitives import serialization
+import pytest
 
 from ipaplatform.paths import paths
 from ipapython.dn import DN
@@ -54,7 +55,7 @@ include "$ROOT_KEY";
  */
 dyndb "ipa" "$BIND_LDAP_SO" {
         uri "ldapi://%2fvar%2frun%2fslapd-$SERVER_ID.socket";
-        base "cn=dns, $SUFFIX";
+        base "cn=dns,$SUFFIX";
         server_id "$FQDN";
         auth_method "sasl";
         sasl_mech "GSSAPI";
@@ -67,15 +68,10 @@ dyndb "ipa" "$BIND_LDAP_SO" {
 def named_test_template(host):
     # create bind instance to get a substitution dict
     bind = bindinstance.BindInstance()
-    bind.setup(
+    bind.setup_templating(
         fqdn=host.hostname,
-        ip_addresses=[host.ip],
         realm_name=host.domain.realm,
         domain_name=host.domain.name,
-        # not relevant
-        forwarders=[],
-        forward_policy=None,
-        reverse_zones=[]
     )
     sub_dict = bind.sub_dict.copy()
     sub_dict.update(BINDKEYS_FILE="/etc/named.iscdlv.key")
@@ -160,21 +156,30 @@ class TestUpgrade(IntegrationTest):
         )
         print(named_conf)
         custom_conf = self.master.get_file_contents(
-            paths.NAMED_CUSTOM_CONFIG, encoding="utf-8"
+            paths.NAMED_CUSTOM_CONF, encoding="utf-8"
         )
         print(custom_conf)
         opt_conf = self.master.get_file_contents(
-            paths.NAMED_CUSTOM_OPTIONS_CONFIG, encoding="utf-8"
+            paths.NAMED_CUSTOM_OPTIONS_CONF, encoding="utf-8"
         )
         print(opt_conf)
         return named_conf, custom_conf, opt_conf
 
+    @pytest.mark.skip_if_platform(
+        "debian", reason="Debian does not use crypto policy"
+    )
+    def test_named_conf_crypto_policy(self):
+        named_conf = self.master.get_file_contents(
+            paths.NAMED_CONF, encoding="utf-8"
+        )
+        assert paths.NAMED_CRYPTO_POLICY_FILE in named_conf
+
     def test_current_named_conf(self):
         named_conf, custom_conf, opt_conf = self.get_named_confs()
         # verify that both includes are present exactly one time
-        inc_opt_conf = f'include "{paths.NAMED_CUSTOM_OPTIONS_CONFIG}";'
+        inc_opt_conf = f'include "{paths.NAMED_CUSTOM_OPTIONS_CONF}";'
         assert named_conf.count(inc_opt_conf) == 1
-        inc_custom_conf = f'include "{paths.NAMED_CUSTOM_CONFIG}";'
+        inc_custom_conf = f'include "{paths.NAMED_CUSTOM_CONF}";'
         assert named_conf.count(inc_custom_conf) == 1
 
         assert "dnssec-validation yes;" in opt_conf
@@ -188,8 +193,8 @@ class TestUpgrade(IntegrationTest):
             [
                 "rm",
                 "-f",
-                paths.NAMED_CUSTOM_CONFIG,
+                paths.NAMED_CUSTOM_CONF,
-                paths.NAMED_CUSTOM_OPTIONS_CONFIG,
+                paths.NAMED_CUSTOM_OPTIONS_CONF,
             ]
         )
         self.master.run_command(['ipa-server-upgrade'])
@@ -202,9 +207,9 @@ class TestUpgrade(IntegrationTest):
         assert "dnssec-validation" not in named_conf
 
         # verify that both includes are present exactly one time
-        inc_opt_conf = f'include "{paths.NAMED_CUSTOM_OPTIONS_CONFIG}";'
+        inc_opt_conf = f'include "{paths.NAMED_CUSTOM_OPTIONS_CONF}";'
         assert named_conf.count(inc_opt_conf) == 1
-        inc_custom_conf = f'include "{paths.NAMED_CUSTOM_CONFIG}";'
+        inc_custom_conf = f'include "{paths.NAMED_CUSTOM_CONF}";'
         assert named_conf.count(inc_custom_conf) == 1
 
     def test_update_named_conf_old(self):
@@ -213,8 +218,8 @@ class TestUpgrade(IntegrationTest):
             [
                 "rm",
                 "-f",
-                paths.NAMED_CUSTOM_CONFIG,
+                paths.NAMED_CUSTOM_CONF,
-                paths.NAMED_CUSTOM_OPTIONS_CONFIG,
+                paths.NAMED_CUSTOM_OPTIONS_CONF,
             ]
         )
         # dump an old named conf to verify migration
@@ -236,7 +241,7 @@ class TestUpgrade(IntegrationTest):
         assert "dnssec-validation" not in named_conf
 
         # verify that both includes are present exactly one time
-        inc_opt_conf = f'include "{paths.NAMED_CUSTOM_OPTIONS_CONFIG}";'
+        inc_opt_conf = f'include "{paths.NAMED_CUSTOM_OPTIONS_CONF}";'
         assert named_conf.count(inc_opt_conf) == 1
-        inc_custom_conf = f'include "{paths.NAMED_CUSTOM_CONFIG}";'
+        inc_custom_conf = f'include "{paths.NAMED_CUSTOM_CONF}";'
         assert named_conf.count(inc_custom_conf) == 1

ipatests/test_ipaserver/test_install/test_bindinstance.py

@@ -1,96 +0,0 @@
-#
-# Copyright (C) 2018  FreeIPA Contributors.  See COPYING for license
-#
-
-from __future__ import absolute_import
-
-import tempfile
-
-import pytest
-
-from ipaplatform.paths import paths
-from ipaserver.install.server.upgrade import named_add_crypto_policy
-
-try:
-    from unittest.mock import patch  # pylint: disable=import-error
-except ImportError:
-    from mock import patch  # pylint: disable=import-error
-
-
-TEST_CONFIG = """
-options {
-\tdnssec-enable yes;
-\tdnssec-validation yes;
-};
-
-include "random/file";
-"""
-
-EXPECTED_CONFIG = """
-options {
-\tdnssec-enable yes;
-\tdnssec-validation yes;
-\tinclude "/etc/crypto-policies/back-ends/bind.config";
-};
-
-include "random/file";
-"""
-
-# bindinstance.named_conf_exists() looks for a section like this
-IPA_DYNDB_CONFIG = """
-dyndb "ipa" "/usr/lib/bind/ldap.so" {
-};
-"""
-
-POLICY_FILE = "/etc/crypto-policies/back-ends/bind.config"
-
-
-@pytest.fixture
-def namedconf():
-    with tempfile.NamedTemporaryFile('w+') as f:
-        with patch.multiple(paths,
-                            NAMED_CONF=f.name,
-                            NAMED_CRYPTO_POLICY_FILE=POLICY_FILE):
-            yield f.name
-
-
-@patch('ipaserver.install.sysupgrade.get_upgrade_state')
-@patch('ipaserver.install.sysupgrade.set_upgrade_state')
-def test_add_crypto_policy(m_set, m_get, namedconf):
-    m_get.return_value = False
-    with open(namedconf, 'w') as f:
-        f.write(TEST_CONFIG)
-        f.write(IPA_DYNDB_CONFIG)
-
-    result = named_add_crypto_policy()
-    assert result
-    m_get.assert_called_with('named.conf', 'add_crypto_policy')
-    m_set.assert_called_with('named.conf', 'add_crypto_policy', True)
-
-    with open(namedconf) as f:
-        content = f.read()
-    assert content == ''.join([EXPECTED_CONFIG, IPA_DYNDB_CONFIG])
-
-    m_get.reset_mock()
-    m_set.reset_mock()
-
-    m_get.return_value = True
-    named_add_crypto_policy()
-    m_get.assert_called_with('named.conf', 'add_crypto_policy')
-    m_set.assert_not_called()
-
-
-@patch('ipaserver.install.sysupgrade.get_upgrade_state')
-@patch('ipaserver.install.sysupgrade.set_upgrade_state')
-def test_add_crypto_policy_no_ipa(m_set, m_get, namedconf):
-    # Test if the update step is skipped when named.conf doesn't contain
-    # IPA related settings.
-    m_get.return_value = False
-    with open(namedconf, 'w') as f:
-        f.write(TEST_CONFIG)
-
-    result = named_add_crypto_policy()
-    assert not result
-
-    m_get.assert_not_called()
-    m_set.assert_not_called()