refactor: remove string boolean coercion and rely on JSON schema validation

Author: ritwik-g

helm_values_manager/models/helm_values_config.py

@@ -1,6 +1,7 @@
 """HelmValuesConfig class for managing Helm values and secrets."""
 
 import json
+import logging
 import os
 from dataclasses import dataclass, field
 from typing import Any, Dict, Optional
@@ -12,6 +13,8 @@
 from helm_values_manager.models.path_data import PathData
 from helm_values_manager.models.value import Value
 
+logger = logging.getLogger(__name__)
+
 
 @dataclass
 class Deployment:
@@ -45,6 +48,24 @@ def _load_schema(cls) -> Dict[str, Any]:
         with open(schema_path, "r", encoding="utf-8") as f:
             return json.load(f)
 
+    @classmethod
+    def _validate_schema(cls, data: dict) -> None:
+        """
+        Validate data against JSON schema.
+
+        Args:
+            data: Dictionary to validate against schema
+
+        Raises:
+            ValidationError: If the data does not match the schema
+        """
+        schema = cls._load_schema()
+        try:
+            jsonschema.validate(instance=data, schema=schema)
+        except ValidationError as e:
+            logger.error("JSON schema validation failed: %s", e)
+            raise
+
     def add_config_path(
         self, path: str, description: Optional[str] = None, required: bool = False, sensitive: bool = False
     ) -> None:
@@ -110,6 +131,10 @@ def set_value(self, path: str, environment: str, value: str) -> None:
 
     def validate(self) -> None:
         """Validate the configuration."""
+        # Validate against JSON schema first
+        self._validate_schema(self.to_dict())
+
+        # Then validate each path_data
         for path_data in self._path_map.values():
             path_data.validate()
 
@@ -136,19 +161,9 @@ def from_dict(cls, data: dict) -> "HelmValuesConfig":
         Raises:
             ValidationError: If the configuration data is invalid
         """
-        # Convert string boolean values to actual booleans for backward compatibility
-        data = data.copy()  # Don't modify the input
-        for config_item in data.get("config", []):
-            for boolean_field in ["required", "sensitive"]:
-                if boolean_field in config_item and isinstance(config_item[boolean_field], str):
-                    config_item[boolean_field] = config_item[boolean_field].lower() == "true"
-
         # Validate against schema
-        schema = cls._load_schema()
-        try:
-            jsonschema.validate(instance=data, schema=schema)
-        except ValidationError as e:
-            raise e
+        data = data.copy()  # Don't modify the input
+        cls._validate_schema(data)
 
         config = cls()
         config.version = data["version"]

tests/unit/models/test_helm_values_config.py

@@ -1,5 +1,6 @@
 """Test suite for HelmValuesConfig class."""
 
+import jsonschema
 import pytest
 
 from helm_values_manager.models.helm_values_config import HelmValuesConfig
@@ -157,3 +158,18 @@ def test_validate():
 
     # Should not raise error since path_data.validate() handles validation
     config.validate()
+
+
+def test_validate_with_schema():
+    """Test validation including schema validation."""
+    # Create config with invalid version
+    config = HelmValuesConfig()
+    config.version = "invalid"  # Version must match pattern in schema
+
+    # Should raise ValidationError due to schema validation
+    with pytest.raises(jsonschema.exceptions.ValidationError, match=r"'invalid' is not one of"):
+        config.validate()
+
+    # Fix version and test valid case
+    config.version = "1.0"
+    config.validate()  # Should not raise error

tests/unit/models/test_schema_validation.py

@@ -86,8 +86,8 @@ def test_type_coercion():
         "config": [
             {
                 "path": "app.config.key1",
-                "required": "true",  # String instead of boolean
-                "sensitive": "false",  # String instead of boolean
+                "required": True,  # Proper boolean
+                "sensitive": False,  # Proper boolean
                 "values": {},
             }
         ],