commit 421fc376ccb8668c07692d3a3394a5869dc97296 Author: Fraser Tweedale Date: Wed Mar 28 16:05:05 2018 +1100 Fix upgrade when named.conf does not exist Commit aee0d2180c7119bef30ab7cafea81dc3df1170b7 adds an upgrade step that adds system crypto policy include to named.conf. This step omitted the named.conf existence check; upgrade fails when it does not exist. Add the existence check. Also update the test to add the IPA-related part of the named.conf config, because the "existence check" actually does more than just check that the file exists - it also check that it contains the IPA bind-dyndb-ldap configuration section. Part of: https://pagure.io/freeipa/issue/4853 Reviewed-By: Christian Heimes diff --git a/ipaserver/install/bindinstance.py b/ipaserver/install/bindinstance.py index 5cf537201..cd70cc983 100644 --- a/ipaserver/install/bindinstance.py +++ b/ipaserver/install/bindinstance.py @@ -93,6 +93,10 @@ def create_reverse(): def named_conf_exists(): + """ + Checks that named.conf exists AND that it contains IPA-related config. + + """ try: with open(paths.NAMED_CONF, 'r') as named_fd: lines = named_fd.readlines() diff --git a/ipaserver/install/server/upgrade.py b/ipaserver/install/server/upgrade.py index c192f4fff..07d783445 100644 --- a/ipaserver/install/server/upgrade.py +++ b/ipaserver/install/server/upgrade.py @@ -905,6 +905,10 @@ def named_add_server_id(): 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 diff --git a/ipatests/test_ipaserver/test_install/test_bindinstance.py b/ipatests/test_ipaserver/test_install/test_bindinstance.py index 6b072ad8a..b88b93194 100644 --- a/ipatests/test_ipaserver/test_install/test_bindinstance.py +++ b/ipatests/test_ipaserver/test_install/test_bindinstance.py @@ -24,7 +24,6 @@ options { include "random/file"; """ - EXPECTED_CONFIG = """ options { \tdnssec-enable yes; @@ -35,6 +34,12 @@ options { 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" @@ -53,14 +58,16 @@ 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) - named_add_crypto_policy() + 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 == EXPECTED_CONFIG + assert content == ''.join([EXPECTED_CONFIG, IPA_DYNDB_CONFIG]) m_get.reset_mock() m_set.reset_mock() @@ -69,3 +76,19 @@ def test_add_crypto_policy(m_set, m_get, namedconf): 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()