More upgrade tests

Reviewed-By: Alexander Bokovoy <abokovoy@redhat.com>

ipaserver/install/bindinstance.py

@@ -701,14 +701,16 @@ class BindInstance(service.Service):
         else:
             self.zonemgr = normalize_zonemgr(zonemgr)
 
-        self.first_instance = not dns_container_exists(self.suffix)
-
         self.__setup_sub_dict()
 
     @property
     def host_domain(self):
         return self.fqdn.split(".", 1)[1]
 
+    @property
+    def first_instance(self):
+        return not dns_container_exists(self.suffix)
+
     @property
     def host_in_rr(self):
         # when a host is not in a default domain, it needs to be referred
@@ -736,7 +738,6 @@ class BindInstance(service.Service):
                   f.name)
 
     def create_instance(self):
-
         try:
             self.stop()
         except Exception:
@@ -1072,7 +1073,6 @@ class BindInstance(service.Service):
         self.host = fqdn.split(".")[0]
         self.suffix = ipautil.realm_to_suffix(self.realm)
         self.reverse_zones = reverse_zones
-        self.first_instance = False
         self.zonemgr = 'hostmaster.%s' % self.domain
 
         self.__add_self()

ipatests/test_integration/test_upgrade.py

@@ -5,15 +5,100 @@
 """
 Module provides tests to verify that the upgrade script works.
 """
-
 import base64
+import configparser
+import os
+import io
+
 from cryptography.hazmat.primitives import serialization
 
 from ipaplatform.paths import paths
 from ipapython.dn import DN
+from ipapython.ipautil import template_str
+from ipaserver.install import bindinstance
+from ipaserver.install.sysupgrade import STATEFILE_FILE
 from ipatests.test_integration.base import IntegrationTest
 from ipatests.pytest_ipa.integration import tasks
 
+# old template without comments for testing
+# and "dnssec-validation no"
+OLD_NAMED_TEMPLATE = """
+options {
+        listen-on-v6 {any;};
+        directory "$NAMED_VAR_DIR"; // the default
+        dump-file               "${NAMED_DATA_DIR}cache_dump.db";
+        statistics-file         "${NAMED_DATA_DIR}named_stats.txt";
+        memstatistics-file      "${NAMED_DATA_DIR}named_mem_stats.txt";
+        tkey-gssapi-keytab "$NAMED_KEYTAB";
+        pid-file "$NAMED_PID";
+        dnssec-enable yes;
+        dnssec-validation no;
+        bindkeys-file "$BINDKEYS_FILE";
+        managed-keys-directory "$MANAGED_KEYS_DIR";
+        $INCLUDE_CRYPTO_POLICY
+};
+
+logging {
+        channel default_debug {
+                file "${NAMED_DATA_DIR}named.run";
+                severity dynamic;
+                print-time yes;
+        };
+};
+
+include "$RFC1912_ZONES";
+include "$ROOT_KEY";
+
+/* WARNING: This part of the config file is IPA-managed.
+ * Modifications may break IPA setup or upgrades.
+ */
+dyndb "ipa" "$BIND_LDAP_SO" {
+        uri "ldapi://%2fvar%2frun%2fslapd-$SERVER_ID.socket";
+        base "cn=dns, $SUFFIX";
+        server_id "$FQDN";
+        auth_method "sasl";
+        sasl_mech "GSSAPI";
+        sasl_user "DNS/$FQDN";
+};
+/* End of IPA-managed part. */
+"""
+
+
+def named_test_template(host):
+    # create bind instance to get a substitution dict
+    bind = bindinstance.BindInstance()
+    bind.setup(
+        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")
+    return template_str(OLD_NAMED_TEMPLATE, sub_dict)
+
+
+def clear_sysupgrade(host, *sections):
+    # get state file
+    statefile = os.path.join(paths.STATEFILE_DIR, STATEFILE_FILE)
+    state = host.get_file_contents(statefile, encoding="utf-8")
+    # parse it
+    parser = configparser.ConfigParser()
+    parser.optionxform = str
+    parser.read_string(state)
+    # remove sections
+    for section in sections:
+        parser.remove_section(section)
+    # dump the modified config
+    out = io.StringIO()
+    parser.write(out)
+    # upload it
+    host.put_file_contents(statefile, out.getvalue())
+
 
 class TestUpgrade(IntegrationTest):
     """
@@ -25,7 +110,8 @@ class TestUpgrade(IntegrationTest):
     """
     @classmethod
     def install(cls, mh):
-        tasks.install_master(cls.master, setup_dns=False)
+        tasks.install_master(cls.master)
+        tasks.install_dns(cls.master)
 
     def test_invoke_upgrader(self):
         cmd = self.master.run_command(['ipa-server-upgrade'],
@@ -68,8 +154,35 @@ class TestUpgrade(IntegrationTest):
             raise AssertionError('%s contains a double-encoded cert'
                                  % entry.dn)
 
-    def test_update_named_conf(self):
-        tasks.install_dns(self.master)
+    def get_named_confs(self):
+        named_conf = self.master.get_file_contents(
+            paths.NAMED_CONF, encoding="utf-8"
+        )
+        print(named_conf)
+        custom_conf = self.master.get_file_contents(
+            paths.NAMED_CUSTOM_CONFIG, encoding="utf-8"
+        )
+        print(custom_conf)
+        opt_conf = self.master.get_file_contents(
+            paths.NAMED_CUSTOM_OPTIONS_CONFIG, encoding="utf-8"
+        )
+        print(opt_conf)
+        return named_conf, custom_conf, opt_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}";'
+        assert named_conf.count(inc_opt_conf) == 1
+        inc_custom_conf = f'include "{paths.NAMED_CUSTOM_CONFIG}";'
+        assert named_conf.count(inc_custom_conf) == 1
+
+        assert "dnssec-validation yes;" in opt_conf
+        assert "dnssec-validation" not in named_conf
+
+        assert custom_conf
+
+    def test_update_named_conf_simple(self):
         # remove files to force a migration
         self.master.run_command(
             [
@@ -80,7 +193,50 @@ class TestUpgrade(IntegrationTest):
             ]
         )
         self.master.run_command(['ipa-server-upgrade'])
-        txt = self.master.get_file_contents(
-            paths.NAMED_CUSTOM_OPTIONS_CONFIG, encoding="utf-8"
+        named_conf, custom_conf, opt_conf = self.get_named_confs()
+
+        # not empty
+        assert custom_conf.strip()
+        # has dnssec-validation enabled in option config
+        assert "dnssec-validation yes;" in opt_conf
+        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}";'
+        assert named_conf.count(inc_opt_conf) == 1
+        inc_custom_conf = f'include "{paths.NAMED_CUSTOM_CONFIG}";'
+        assert named_conf.count(inc_custom_conf) == 1
+
+    def test_update_named_conf_old(self):
+        # remove files to force a migration
+        self.master.run_command(
+            [
+                "rm",
+                "-f",
+                paths.NAMED_CUSTOM_CONFIG,
+                paths.NAMED_CUSTOM_OPTIONS_CONFIG,
+            ]
         )
-        assert "dnssec-validation yes;" in txt
+        # dump an old named conf to verify migration
+        old_contents = named_test_template(self.master)
+        self.master.put_file_contents(paths.NAMED_CONF, old_contents)
+        clear_sysupgrade(self.master, "dns", "named.conf")
+        # check
+        self.master.run_command(["named-checkconf", paths.NAMED_CONF])
+
+        # upgrade
+        self.master.run_command(['ipa-server-upgrade'])
+
+        named_conf, custom_conf, opt_conf = self.get_named_confs()
+
+        # not empty
+        assert custom_conf.strip()
+        # dnssec-validation is migrated as "disabled" from named.conf
+        assert "dnssec-validation no;" in opt_conf
+        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}";'
+        assert named_conf.count(inc_opt_conf) == 1
+        inc_custom_conf = f'include "{paths.NAMED_CUSTOM_CONFIG}";'
+        assert named_conf.count(inc_custom_conf) == 1