You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
140 lines
4.5 KiB
140 lines
4.5 KiB
2 years ago
|
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
|
||
|
|