From 97f6517715aa91572d2ab8dd03e70d9e0e90cfac Mon Sep 17 00:00:00 2001
From: Darko Poljak <foss@ungleich.com>
Date: Thu, 7 Sep 2017 16:13:46 +0200
Subject: [PATCH] Configuration fixes.

---
 cdist/argparse.py                    |  9 ++++++---
 cdist/configuration.py               | 18 +++++++++++++++++-
 cdist/test/configuration/__init__.py | 20 ++++++++++++++++++--
 3 files changed, 41 insertions(+), 6 deletions(-)

diff --git a/cdist/argparse.py b/cdist/argparse.py
index 28d3840e..391541b0 100644
--- a/cdist/argparse.py
+++ b/cdist/argparse.py
@@ -106,7 +106,7 @@ def get_parsers():
                   'The levels, in order from the lowest to the highest, are: '
                   'ERROR (-1), WARNING (0), INFO (1), VERBOSE (2), DEBUG (3) '
                   'TRACE (4 or higher).'),
-            action='count', default=0)
+            action='count', default=None)
 
     parser['beta'] = argparse.ArgumentParser(add_help=False)
     parser['beta'].add_argument(
@@ -417,8 +417,11 @@ def handle_loglevel(args):
 def parse_and_configure(argv, singleton=True):
     parser = get_parsers()
     parser_args = parser['main'].parse_args(argv)
-    cfg = cdist.configuration.Configuration(parser_args, singleton=singleton)
-    args = cfg.get_args()
+    try:
+        cfg = cdist.configuration.Configuration(parser_args, singleton=singleton)
+        args = cfg.get_args()
+    except ValueError as e:
+        raise cdist.Error(str(e))
     # Loglevels are handled globally in here
     handle_loglevel(args)
 
diff --git a/cdist/configuration.py b/cdist/configuration.py
index fe7fdf1b..8fc5c473 100644
--- a/cdist/configuration.py
+++ b/cdist/configuration.py
@@ -285,6 +285,11 @@ class Configuration(metaclass=Singleton):
     ADJUST_ARG_OPTION_MAPPING = {
        _ARG_OPTION_MAPPING[key]: key for key in _ARG_OPTION_MAPPING
     }
+    REQUIRED_DEFAULT_CONFIG_VALUES = {
+        'GLOBAL': {
+            'verbosity': 0,
+        },
+    }
 
     def _convert_args(self, args):
         if args:
@@ -372,7 +377,8 @@ class Configuration(metaclass=Singleton):
         for option in self.ARG_OPTION_MAPPING:
             if option in args:
                 dst_opt = self.ARG_OPTION_MAPPING[option]
-                d[dst_opt] = args[option]
+                if args[option]:
+                    d[dst_opt] = args[option]
         return d
 
     def _update_config_dict(self, config, newconfig, update_appends=False):
@@ -394,6 +400,15 @@ class Configuration(metaclass=Singleton):
             config[section][option] = option_object.update_value(
                 currval, newval, update_appends)
 
+    def _update_defaults_for_unset(self, config):
+        defaults = self.REQUIRED_DEFAULT_CONFIG_VALUES
+
+        for section in defaults:
+            section_values = defaults[section]
+            for option in section_values:
+                if option not in config[section]:
+                    config[section][option] = section_values[option]
+
     def _get_config(self):
         # global config file
         # local config file
@@ -421,4 +436,5 @@ class Configuration(metaclass=Singleton):
             for section in config:
                 self._update_config_dict_section(section, config, newconfig,
                                                  update_appends=True)
+        self._update_defaults_for_unset(config)
         return config
diff --git a/cdist/test/configuration/__init__.py b/cdist/test/configuration/__init__.py
index acd6c0bb..0e324e0b 100644
--- a/cdist/test/configuration/__init__.py
+++ b/cdist/test/configuration/__init__.py
@@ -286,7 +286,6 @@ class ConfigurationTestCase(test.CdistTestCase):
         args.tag = 'test'
 
         expected = {
-            'beta': False,
             'conf_dir': ['/usr/local/cdist1', ],
             'verbosity': 3,
         }
@@ -373,12 +372,13 @@ class ConfigurationTestCase(test.CdistTestCase):
         args = argparse.Namespace()
         expected_config_dict = {
             'GLOBAL': {
+                'verbosity': 0,
             },
         }
 
         # bypass singleton so we can test further
         cc.Configuration.instance = None
-        configuration = cc.Configuration(args, env=env)
+        configuration = cc.Configuration(args, env=env, config_files=('cdist.cfg'))
         self.assertIsNotNone(configuration.args)
         self.assertIsNotNone(configuration.env)
         self.assertIsNotNone(configuration.config_files)
@@ -668,6 +668,20 @@ class ConfigurationTestCase(test.CdistTestCase):
                                          config_files=config_files)
         self.assertEqual(configuration.config, expected_config_dict)
 
+    def test_update_defaults_for_unset(self):
+        config = {
+            'GLOBAL': {
+            },
+        }
+        expected_config = {
+            'GLOBAL': {
+                'verbosity': 0,
+            },
+        }
+        cfg = cc.Configuration(None, env={}, config_files=())
+        cfg._update_defaults_for_unset(config)
+        self.assertEqual(config, expected_config)
+
     def test_configuration6(self):
         env = {
             'PATH': '/usr/local/bin:/usr/bin:/bin',
@@ -725,6 +739,7 @@ class ConfigurationTestCase(test.CdistTestCase):
         args.conf_dir = ['/opt/sysadmin/cdist/conf', ]
         args.manifest = '/opt/sysadmin/cdist/conf/manifest/init'
         args.jobs = 10
+        args.verbose = None
 
         expected_config_dict = {
             'GLOBAL': {
@@ -1092,6 +1107,7 @@ class ConfigurationTestCase(test.CdistTestCase):
             'GLOBAL': {
                 'inventory_dir': None,
                 'conf_dir': None,
+                'verbosity': 0,
             },
         }