From c0ef933e5465baa3882e7ed301f8a7a1f4f05301 Mon Sep 17 00:00:00 2001
From: Rob Crittenden <rcritten@redhat.com>
Date: Fri, 29 Apr 2022 11:42:53 -0400
Subject: [PATCH] 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 | 39 +++++++++++++++++++++++++++++++
 src/ipahealthcheck/core/output.py |  2 +-
 tests/test_config.py              | 16 ++++++++++++-
 tests/test_options.py             |  2 +-
 4 files changed, 56 insertions(+), 3 deletions(-)

diff --git a/src/ipahealthcheck/core/config.py b/src/ipahealthcheck/core/config.py
index 01c7722..a1bd2d5 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
@@ -110,5 +131,23 @@ def read_config(config_file):
             return None
         else:
             config[key] = value
+            # 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 e44990e..ca9297c 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().__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 dbfca2b..02a7e63 100644
--- a/tests/test_config.py
+++ b/tests/test_config.py
@@ -6,7 +6,7 @@ import tempfile
 
 import pytest
 
-from ipahealthcheck.core.config import read_config
+from ipahealthcheck.core.config import read_config, convert_string
 
 
 def test_config_no_section():
@@ -59,3 +59,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