Merge pull request #38153 from vutny/master-includes-error-tolerance

Master config includes may contain errors and be safely skipped

salt/config/__init__.py

@@ -1628,7 +1628,7 @@ def _absolute_path(path, relative_to=None):
     return path
 
 
-def load_config(path, env_var, default_path=None):
+def load_config(path, env_var, default_path=None, exit_on_config_errors=True):
     '''
     Returns configuration dict from parsing either the file described by
     ``path`` or the environment variable described by ``env_var`` as YAML.
@@ -1676,17 +1676,20 @@ def load_config(path, env_var, default_path=None):
                     ifile.readline()  # skip first line
                     out.write(ifile.read())
 
+    opts = {}
+
     if salt.utils.validate.path.is_readable(path):
         try:
             opts = _read_conf_file(path)
             opts['conf_file'] = path
-            return opts
         except salt.exceptions.SaltConfigurationError as error:
             log.error(error)
-            sys.exit(salt.defaults.exitcodes.EX_GENERIC)
+            if exit_on_config_errors:
+                sys.exit(salt.defaults.exitcodes.EX_GENERIC)
+    else:
+        log.debug('Missing configuration file: {0}'.format(path))
 
-    log.debug('Missing configuration file: {0}'.format(path))
-    return {}
+    return opts
 
 
 def include_config(include, orig_path, verbose, exit_on_config_errors=False):
@@ -1731,6 +1734,9 @@ def include_config(include, orig_path, verbose, exit_on_config_errors=False):
                 log.error(error)
                 if exit_on_config_errors:
                     sys.exit(salt.defaults.exitcodes.EX_GENERIC)
+                else:
+                    # Initialize default config if we wish to skip config errors
+                    opts = {}
 
             include = opts.get('include', [])
             if include:
@@ -3085,8 +3091,10 @@ def master_config(path, env_var='SALT_MASTER_CONFIG', defaults=None, exit_on_con
                                     defaults['default_include'])
     include = overrides.get('include', [])
 
-    overrides.update(include_config(default_include, path, verbose=False), exit_on_config_errors=exit_on_config_errors)
-    overrides.update(include_config(include, path, verbose=True), exit_on_config_errors=exit_on_config_errors)
+    overrides.update(include_config(default_include, path, verbose=False),
+                     exit_on_config_errors=exit_on_config_errors)
+    overrides.update(include_config(include, path, verbose=True),
+                     exit_on_config_errors=exit_on_config_errors)
     opts = apply_master_config(overrides, defaults)
     _validate_opts(opts)
     # If 'nodegroups:' is uncommented in the master config file, and there are

tests/unit/config/config_test.py

@@ -19,17 +19,20 @@ from contextlib import contextmanager
 from salttesting import TestCase
 from salttesting.mock import MagicMock, patch
 from salttesting.helpers import ensure_in_syspath
-from salt.exceptions import CommandExecutionError
 
 ensure_in_syspath('../')
 
-# Import salt libs
+# Import Salt libs
 import salt.minion
 import salt.utils
 import salt.utils.network
 import integration
 from salt import config as sconfig
-from salt.exceptions import SaltCloudConfigError
+from salt.exceptions import (
+    CommandExecutionError,
+    SaltConfigurationError,
+    SaltCloudConfigError
+)
 
 # Import Third-Party Libs
 import yaml
@@ -66,6 +69,13 @@ def _unhandled_mock_read(filename):
     raise CommandExecutionError('Unhandled mock read for {0}'.format(filename))
 
 
+def _salt_configuration_error(filename):
+    '''
+    Raise an error to indicate error in the Salt configuration file
+    '''
+    raise SaltConfigurationError('Configuration error in {0}'.format(filename))
+
+
 @contextmanager
 def _fopen_side_effect_etc_hostname(filename):
     '''
@@ -933,6 +943,50 @@ class ConfigTestCase(TestCase, integration.AdaptedConfigurationTestCaseMixIn):
 
 # <---- Salt Cloud Configuration Tests ---------------------------------------------
 
+    def test_include_config_without_errors(self):
+        '''
+        Tests that include_config function returns valid configuration
+        '''
+        include_file = 'minion.d/my.conf'
+        config_path = '/etc/salt/minion'
+        config_opts = {'id': 'myminion.example.com'}
+
+        with patch('glob.glob', MagicMock(return_value=include_file)):
+            with patch('salt.config._read_conf_file', MagicMock(return_value=config_opts)):
+                configuration = sconfig.include_config(include_file, config_path, verbose=False)
+
+        self.assertEqual(config_opts, configuration)
+
+    def test_include_config_with_errors(self):
+        '''
+        Tests that include_config function returns valid configuration even on errors
+        '''
+        include_file = 'minion.d/my.conf'
+        config_path = '/etc/salt/minion'
+        config_opts = {}
+
+        with patch('glob.glob', MagicMock(return_value=include_file)):
+            with patch('salt.config._read_conf_file', _salt_configuration_error):
+                configuration = sconfig.include_config(include_file, config_path, verbose=False)
+
+        self.assertEqual(config_opts, configuration)
+
+    def test_include_config_with_errors_exit(self):
+        '''
+        Tests that include_config exits on errors
+        '''
+        include_file = 'minion.d/my.conf'
+        config_path = '/etc/salt/minion'
+
+        with patch('glob.glob', MagicMock(return_value=include_file)):
+            with patch('salt.config._read_conf_file', _salt_configuration_error):
+                with self.assertRaises(SystemExit):
+                    sconfig.include_config(include_file,
+                                           config_path,
+                                           verbose=False,
+                                           exit_on_config_errors=True)
+
+
 if __name__ == '__main__':
     from integration import run_tests
     run_tests(ConfigTestCase, needs_daemon=False)