From 883ef5b965bb7b02a701037748436f8daa9d0643 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcritten@redhat.com>
Date: Fri Apr 29 11:42:53 2022 -0400
Subject: Convert configuration option strings into native data
 types

When loading options from the configuration file they will all
be strings. This breaks existing boolean checks (if <something>)
and some assumptions about integer types (e.g. timeout, indent).

So try to detect the data type, defaulting to remain as a string.

Also hardcode some type validation for known keys to prevent
things like debug=foo (which would evaluate to True).

https://bugzilla.redhat.com/show_bug.cgi?id=2079861

Signed-off-by: Rob Crittenden <rcritten@redhat.com>
---
 src/ipahealthcheck/core/config.py | 47 ++++++++++++++++++++++++++++++-
 src/ipahealthcheck/core/output.py |  2 +-
 tests/test_config.py              | 16 ++++++++++-
 tests/test_options.py             |  2 +-
 4 files changed, 63 insertions(+), 4 deletions(-)

diff --git a/src/ipahealthcheck/core/config.py b/src/ipahealthcheck/core/config.py
index 14ae27f..3cd4e8e 100644
--- a/src/ipahealthcheck/core/config.py
+++ b/src/ipahealthcheck/core/config.py
@@ -71,6 +71,27 @@ class Config:
             self.__d[key] = d[key]
 
 
+def convert_string(value):
+    """
+    Reading options from the configuration file will leave them as
+    strings. This breaks boolean values so attempt to convert them.
+    """
+    if not isinstance(value, str):
+        return value
+
+    if value.lower() in (
+        "true",
+        "false",
+    ):
+        return value.lower() == 'true'
+    else:
+        try:
+            value = int(value)
+        except ValueError:
+            pass
+    return value
+
+
 def read_config(config_file):
     """
     Simple configuration file reader
@@ -100,6 +121,30 @@ def read_config(config_file):
     items = parser.items(CONFIG_SECTION)
 
     for (key, value) in items:
-        config[key] = value
+        if len(value) == 0 or value is None:
+            logging.error(
+                "Empty value for %s in %s [%s]",
+                key, config_file, CONFIG_SECTION
+            )
+            return None
+        else:
+            # Try to do some basic validation. This is unfortunately
+            # hardcoded.
+            if key in ('all', 'debug', 'failures_only', 'verbose'):
+                if value.lower() not in ('true', 'false'):
+                    logging.error(
+                        "%s is not a valid boolean in %s [%s]",
+                        key, config_file, CONFIG_SECTION
+                    )
+                    return None
+            elif key in ('indent',):
+                if not isinstance(convert_string(value), int):
+                    logging.error(
+                        "%s is not a valid integer in %s [%s]",
+                        key, config_file, CONFIG_SECTION
+                    )
+                    return None
+            # Some rough type translation from strings
+            config[key] = convert_string(value)
 
     return config
diff --git a/src/ipahealthcheck/core/output.py b/src/ipahealthcheck/core/output.py
index 61dffbe..945969f 100644
--- a/src/ipahealthcheck/core/output.py
+++ b/src/ipahealthcheck/core/output.py
@@ -110,7 +110,7 @@ class JSON(Output):
 
     def __init__(self, options):
         super(JSON, self).__init__(options)
-        self.indent = int(options.indent)
+        self.indent = options.indent
 
     def generate(self, data):
         output = json.dumps(data, indent=self.indent)
diff --git a/tests/test_config.py b/tests/test_config.py
index 09c2188..655233c 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -2,7 +2,7 @@
 # Copyright (C) 2019 FreeIPA Contributors see COPYING for license
 #
 
-from ipahealthcheck.core.config import read_config
+from ipahealthcheck.core.config import read_config, convert_string
 import tempfile
 
 
@@ -60,3 +60,17 @@ def test_config_recursion():
         config._Config__d['_Config__d']
     except KeyError:
         pass
+
+
+def test_convert_string():
+    for value in ("s", "string", "BiggerString"):
+        assert convert_string(value) == value
+
+    for value in ("True", "true", True):
+        assert convert_string(value) is True
+
+    for value in ("False", "false", False):
+        assert convert_string(value) is False
+
+    for value in ("10", "99999", 807):
+        assert convert_string(value) == int(value)
diff --git a/tests/test_options.py b/tests/test_options.py
index da1866f..00cdb7c 100644
--- a/tests/test_options.py
+++ b/tests/test_options.py
@@ -40,6 +40,6 @@ def test_options_merge(mock_parse, mock_run, mock_service):
 
         # verify two valus that have defaults with our overriden values
         assert run.options.output_type == 'human'
-        assert run.options.indent == '5'
+        assert run.options.indent == 5
     finally:
         os.remove(config_path)
-- 
2.31.1