diff --git a/README.md b/README.md index bb20cfd..1cd51e9 100644 --- a/README.md +++ b/README.md @@ -34,15 +34,18 @@ helm plugin install https://github.com/zipstack/helm-values-manager ## Quick Start 1. Initialize a new configuration: + ```bash helm values-manager init ``` This creates: + - `values-manager.yaml` configuration file - `values` directory with environment files (`dev.yaml`, `staging.yaml`, `prod.yaml`) 2. View available commands: + ```bash helm values-manager --help ``` @@ -52,23 +55,27 @@ helm values-manager --help ### Setup Development Environment 1. Clone the repository: + ```bash git clone https://github.com/zipstack/helm-values-manager cd helm-values-manager ``` 2. Create and activate a virtual environment: + ```bash python -m venv venv source venv/bin/activate # On Windows: .\venv\Scripts\activate ``` 3. Install development dependencies: + ```bash pip install -e ".[dev]" ``` 4. Install pre-commit hooks: + ```bash pre-commit install ``` @@ -76,11 +83,13 @@ pre-commit install ### Running Tests Run tests with tox (will test against multiple Python versions): + ```bash tox ``` Run tests for a specific Python version: + ```bash tox -e py39 # For Python 3.9 ``` @@ -95,6 +104,7 @@ This project uses several tools to maintain code quality: - **flake8**: Style guide enforcement Run all code quality checks manually: + ```bash pre-commit run --all-files ``` diff --git a/docs/Design/low-level-design.md b/docs/Design/low-level-design.md index 22283bc..0354430 100644 --- a/docs/Design/low-level-design.md +++ b/docs/Design/low-level-design.md @@ -202,6 +202,88 @@ Implementations: - Azure Key Vault Backend - Additional backends can be easily added +### 5. Schema Validation + +The configuration system uses JSON Schema validation to ensure data integrity and consistency: + +```mermaid +classDiagram + class SchemaValidator { + +validate_config(data: dict) None + -load_schema() dict + -handle_validation_error(error: ValidationError) str + } + + class HelmValuesConfig { + +from_dict(data: dict) HelmValuesConfig + +to_dict() dict + +validate() None + } + + HelmValuesConfig ..> SchemaValidator : uses +``` + +#### Schema Structure + +The schema (`schemas/v1.json`) defines: +1. **Version Control** + - Schema version validation + - Backward compatibility checks + +2. **Deployment Configuration** + - Backend type validation (git-secret, aws, azure, gcp) + - Authentication method validation + - Backend-specific configuration validation + +3. **Value Configuration** + - Path format validation (dot notation) + - Required/optional field validation + - Sensitive value handling + - Environment-specific value validation + +#### Validation Points + +Schema validation occurs at critical points: +1. **Configuration Loading** (`from_dict`) + - Validates complete configuration structure + - Ensures all required fields are present + - Validates data types and formats + +2. **Pre-save Validation** (`to_dict`) + - Ensures configuration remains valid after modifications + - Validates new values match schema requirements + +3. **Path Addition** (`add_config_path`) + - Validates new path format + - Ensures path uniqueness + - Validates metadata structure + +#### Error Handling + +The validation system provides: +1. **Detailed Error Messages** + - Exact location of validation failures + - Clear explanation of validation rules + - Suggestions for fixing issues + +2. **Validation Categories** + - Schema version mismatches + - Missing required fields + - Invalid value formats + - Backend configuration errors + - Authentication configuration errors + +3. **Error Recovery** + - Validation before persistence + - Prevents invalid configurations from being saved + - Maintains system consistency + +This validation ensures: +- Configuration integrity +- Consistent data structure +- Clear error reporting +- Safe configuration updates + ## Implementation Details ### 1. Configuration Structure diff --git a/docs/Development/tasks.md b/docs/Development/tasks.md index d7241b7..c6fd83e 100644 --- a/docs/Development/tasks.md +++ b/docs/Development/tasks.md @@ -35,6 +35,20 @@ - [x] Add tests for ConfigMetadata - [x] Integrate with PathData +### Schema Validation Integration +- [x] Add Basic Schema Validation + - [x] Create test_schema_validation.py + - [x] Test valid configuration loading + - [x] Test invalid configuration detection + - [x] Test error message clarity + - [x] Add schema validation to HelmValuesConfig + - [x] Add jsonschema dependency + - [x] Implement validation in from_dict + - [x] Add clear error messages + - [x] Update documentation + - [x] Schema documentation in low-level design + - [x] Example configuration in design docs + ### ConfigMetadata - [x] Implement ConfigMetadata class - [x] Add metadata attributes @@ -45,21 +59,6 @@ - [x] Implement from_dict() static method - [x] Add tests for serialization/deserialization -### HelmValuesConfig Refactoring -- [ ] Remove PlainTextBackend references - - [ ] Update imports and dependencies - - [ ] Remove plaintext.py - - [ ] Update tests -- [ ] Implement unified path storage - - [ ] Add _path_map dictionary - - [ ] Migrate existing code to use _path_map - - [ ] Update tests for new structure -- [ ] Update value management - - [ ] Refactor set_value() to use Value class - - [ ] Refactor get_value() to use Value class - - [ ] Add value validation in set operations - - [ ] Update tests for new value handling - ### Backend System - [ ] Clean up base ValueBackend - [ ] Update interface methods @@ -97,7 +96,6 @@ - [x] Value class tests - [x] PathData class tests - [x] ConfigMetadata tests - - [ ] HelmValuesConfig tests - [ ] Backend tests - [ ] Command tests - [ ] Add integration tests diff --git a/helm_values_manager/backends/base.py b/helm_values_manager/backends/base.py index a3387be..9327994 100644 --- a/helm_values_manager/backends/base.py +++ b/helm_values_manager/backends/base.py @@ -5,7 +5,7 @@ """ from abc import ABC, abstractmethod -from typing import Dict +from typing import Dict, Union class ValueBackend(ABC): @@ -60,34 +60,37 @@ def _validate_auth_config(self, auth_config: Dict[str, str]) -> None: raise ValueError(f"Invalid auth type: {auth_config['type']}. " f"Must be one of: {', '.join(valid_types)}") @abstractmethod - def get_value(self, path: str, environment: str, resolve: bool = False) -> str: + def get_value(self, path: str, environment: str, resolve: bool = False) -> Union[str, int, float, bool, None]: """ - Get a value from storage. + Get a value from the backend. Args: - path: The configuration path - environment: The environment name - resolve: If True, resolve any secret references to their actual values. - If False, return the raw value which may be a secret reference. + path: The configuration path (e.g., "app.replicas") + environment: The environment name (e.g., "dev", "prod") + resolve: Whether to resolve any secret references Returns: - str: The value (resolved or raw depending on resolve parameter) + The value from the backend, can be a string, number, boolean, or None + + Raises: + ValueError: If the value doesn't exist + RuntimeError: If backend operation fails """ pass @abstractmethod - def set_value(self, path: str, environment: str, value: str) -> None: - """Set a value in the storage backend. + def set_value(self, path: str, environment: str, value: Union[str, int, float, bool, None]) -> None: + """ + Set a value in the backend. Args: path: The configuration path (e.g., "app.replicas") environment: The environment name (e.g., "dev", "prod") - value: The value to store. Must be a string. + value: The value to store, can be a string, number, boolean, or None Raises: - ValueError: If the value is invalid - ConnectionError: If there's an error connecting to the backend - PermissionError: If there's an authentication or authorization error + ValueError: If the value is not a string, number, boolean, or None + RuntimeError: If backend operation fails """ pass diff --git a/helm_values_manager/backends/simple.py b/helm_values_manager/backends/simple.py index b9893a7..72b446f 100644 --- a/helm_values_manager/backends/simple.py +++ b/helm_values_manager/backends/simple.py @@ -4,7 +4,7 @@ This module provides a simple in-memory backend for storing non-sensitive values. """ -from typing import Dict +from typing import Dict, Union from helm_values_manager.backends.base import ValueBackend @@ -19,24 +19,23 @@ class SimpleValueBackend(ValueBackend): def __init__(self) -> None: """Initialize an empty in-memory storage.""" super().__init__({"type": "direct"}) # Simple backend doesn't need auth - self._storage: Dict[str, str] = {} + self._storage: Dict[str, Union[str, int, float, bool, None]] = {} def _get_storage_key(self, path: str, environment: str) -> str: """Generate a unique storage key.""" return f"{path}:{environment}" - def get_value(self, path: str, environment: str, resolve: bool = False) -> str: + def get_value(self, path: str, environment: str, resolve: bool = False) -> Union[str, int, float, bool, None]: """ Get a value from the in-memory storage. Args: path: The configuration path (e.g., "app.replicas") environment: The environment name (e.g., "dev", "prod") - resolve: If True, resolve any secret references to their actual values. - If False, return the raw value which may be a secret reference. + resolve: Whether to resolve any secret references Returns: - str: The value (resolved or raw depending on resolve parameter) + The value from the backend, can be a string, number, boolean, or None Raises: ValueError: If the value doesn't exist @@ -46,20 +45,21 @@ def get_value(self, path: str, environment: str, resolve: bool = False) -> str: raise ValueError(f"No value found for {path} in {environment}") return self._storage[key] - def set_value(self, path: str, environment: str, value: str) -> None: + def set_value(self, path: str, environment: str, value: Union[str, int, float, bool, None]) -> None: """ Set a value in the in-memory storage. Args: path: The configuration path (e.g., "app.replicas") environment: The environment name (e.g., "dev", "prod") - value: The value to store + value: The value to store, can be a string, number, boolean, or None Raises: - ValueError: If the value is not a string + ValueError: If the value is not a string, number, boolean, or None """ - if not isinstance(value, str): - raise ValueError("Value must be a string") + if not isinstance(value, (str, int, float, bool, type(None))): + raise ValueError("Value must be a string, number, boolean, or None") + key = self._get_storage_key(path, environment) self._storage[key] = value diff --git a/helm_values_manager/models/helm_values_config.py b/helm_values_manager/models/helm_values_config.py new file mode 100644 index 0000000..adcb3d2 --- /dev/null +++ b/helm_values_manager/models/helm_values_config.py @@ -0,0 +1,199 @@ +"""HelmValuesConfig class for managing Helm values and secrets.""" + +import json +import os +from dataclasses import dataclass, field +from typing import Any, Dict, Optional + +import jsonschema +from jsonschema.exceptions import ValidationError + +from helm_values_manager.backends.simple import SimpleValueBackend +from helm_values_manager.models.path_data import PathData +from helm_values_manager.models.value import Value +from helm_values_manager.utils.logger import HelmLogger + + +@dataclass +class Deployment: + """Deployment configuration.""" + + name: str + auth: Dict[str, Any] + backend: str + backend_config: Dict[str, Any] = field(default_factory=dict) + + def to_dict(self) -> Dict[str, Any]: + """Convert deployment to dictionary.""" + return {"backend": self.backend, "auth": self.auth, "backend_config": self.backend_config} + + +class HelmValuesConfig: + """Configuration manager for Helm values and secrets.""" + + def __init__(self): + """Initialize configuration.""" + self.version: str = "1.0" + self.release: str = "" + self.deployments: Dict[str, Deployment] = {} + self._path_map: Dict[str, PathData] = {} + self._backend = SimpleValueBackend() # For non-sensitive values + + @classmethod + def _load_schema(cls) -> Dict[str, Any]: + """Load the JSON schema for configuration validation.""" + schema_path = os.path.join(os.path.dirname(__file__), "..", "schemas", "v1.json") + 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: + HelmLogger.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: + """ + Add a new configuration path. + + Args: + path: The configuration path + description: Description of what this configuration does + required: Whether this configuration is required + sensitive: Whether this configuration contains sensitive data + """ + if path in self._path_map: + raise ValueError(f"Path {path} already exists") + + metadata = { + "description": description, + "required": required, + "sensitive": sensitive, + } + path_data = PathData(path, metadata) + self._path_map[path] = path_data + + def get_value(self, path: str, environment: str, resolve: bool = False) -> Optional[str]: + """ + Get a value for the given path and environment. + + Args: + path: The configuration path + environment: The environment name + resolve: If True, resolve any secret references to their actual values. + If False, return the raw value which may be a secret reference. + + Returns: + Optional[str]: The value (resolved or raw depending on resolve parameter), or None if value doesn't exist. + + Raises: + KeyError: If path doesn't exist + """ + if path not in self._path_map: + HelmLogger.debug("Path %s not found in path map", path) + raise KeyError(f"Path {path} not found") + + path_data = self._path_map[path] + value_obj = path_data.get_value(environment) + if value_obj is None: + HelmLogger.debug("No value object found for path %s in environment %s", path, environment) + return None + + value = value_obj.get(resolve=resolve) + if value is None: + HelmLogger.debug("No value found for path %s in environment %s", path, environment) + return None + + HelmLogger.debug("Found value for path %s in environment %s", path, environment) + return value + + def set_value(self, path: str, environment: str, value: str) -> None: + """Set a value for the given path and environment.""" + if path not in self._path_map: + raise KeyError(f"Path {path} not found") + + value_obj = Value(path=path, environment=environment, _backend=self._backend) + value_obj.set(value) + self._path_map[path].set_value(environment, value_obj) + + 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() + + def to_dict(self) -> Dict[str, Any]: + """Convert the configuration to a dictionary.""" + return { + "version": self.version, + "release": self.release, + "deployments": {name: depl.to_dict() for name, depl in self.deployments.items()}, + "config": [path_data.to_dict() for path_data in self._path_map.values()], + } + + @classmethod + def from_dict(cls, data: dict) -> "HelmValuesConfig": + """ + Create a configuration from a dictionary. + + Args: + data: Dictionary containing configuration data + + Returns: + HelmValuesConfig: New configuration instance + + Raises: + ValidationError: If the configuration data is invalid + """ + # Validate against schema + data = data.copy() # Don't modify the input + cls._validate_schema(data) + + config = cls() + config.version = data["version"] + config.release = data["release"] + + # Load deployments + for name, depl_data in data.get("deployments", {}).items(): + config.deployments[name] = Deployment( + name=name, + backend=depl_data["backend"], + auth=depl_data["auth"], + backend_config=depl_data.get("backend_config", {}), + ) + + # Load config paths + for config_item in data.get("config", []): + path = config_item["path"] + metadata = { + "description": config_item.get("description"), + "required": config_item.get("required", False), + "sensitive": config_item.get("sensitive", False), + } + path_data = PathData(path, metadata) + config._path_map[path] = path_data + + # Load values + for env, value in config_item.get("values", {}).items(): + value_obj = Value(path=path, environment=env, _backend=config._backend) + value_obj.set(value) + path_data.set_value(env, value_obj) + + return config diff --git a/helm_values_manager/models/path_data.py b/helm_values_manager/models/path_data.py index 1ea62f5..5e99d13 100644 --- a/helm_values_manager/models/path_data.py +++ b/helm_values_manager/models/path_data.py @@ -7,8 +7,9 @@ from typing import Any, Callable, Dict, Iterator, Optional +from helm_values_manager.models.config_metadata import ConfigMetadata from helm_values_manager.models.value import Value -from helm_values_manager.utils.logger import logger +from helm_values_manager.utils.logger import HelmLogger class PathData: @@ -19,12 +20,17 @@ def __init__(self, path: str, metadata: Dict[str, Any]): Initialize PathData with a path and metadata. Args: - path: The configuration path - metadata: Dictionary containing metadata fields + path: The configuration path (e.g., "app.replicas") + metadata: Dictionary containing metadata for the path """ self.path = path - self.metadata = metadata + self._metadata = ConfigMetadata( + description=metadata.get("description"), + required=metadata.get("required", False), + sensitive=metadata.get("sensitive", False), + ) self._values: Dict[str, Value] = {} + HelmLogger.debug("Created PathData instance for path %s", path) def validate(self) -> None: """ @@ -37,20 +43,20 @@ def validate(self) -> None: Raises: ValueError: If validation fails """ - logger.debug("Validating PathData for path: %s", self.path) + HelmLogger.debug("Validating PathData for path: %s", self.path) # Validate path consistency and required values for env, value in self._values.items(): # Check path consistency if value.path != self.path: - logger.error("Path mismatch for environment %s: %s != %s", env, value.path, self.path) + HelmLogger.error("Path mismatch for environment %s: %s != %s", env, value.path, self.path) raise ValueError(f"Value for environment {env} has inconsistent path: {value.path} != {self.path}") # Check required values - if self.metadata.get("required", False): + if self._metadata.required: val = value.get() if val is None or val == "": - logger.error("Missing required value for path %s in environment %s", self.path, env) + HelmLogger.error("Missing required value for path %s in environment %s", self.path, env) raise ValueError(f"Missing required value for path {self.path} in environment {env}") def set_value(self, environment: str, value: Value) -> None: @@ -64,11 +70,11 @@ def set_value(self, environment: str, value: Value) -> None: Raises: ValueError: If the Value object's path doesn't match this PathData's path """ - logger.debug("Setting value for path %s in environment %s", self.path, environment) + HelmLogger.debug("Setting value for path %s in environment %s", self.path, environment) if value.path != self.path: - logger.error("Value path %s doesn't match PathData path %s", value.path, self.path) - raise ValueError(f"Value path {value.path} doesn't match PathData path {self.path}") + HelmLogger.error("Value path %s does not match PathData path %s", value.path, self.path) + raise ValueError(f"Value path {value.path} does not match PathData path {self.path}") self._values[environment] = value @@ -82,10 +88,12 @@ def get_value(self, environment: str) -> Optional[Value]: Returns: Optional[Value]: The Value object if it exists, None otherwise """ - logger.debug("Getting value for path %s in environment %s", self.path, environment) + HelmLogger.debug("Getting value for path %s in environment %s", self.path, environment) value = self._values.get(environment) if value is None: - logger.debug("No value found for path %s in environment %s", self.path, environment) + HelmLogger.debug("No value found for path %s in environment %s", self.path, environment) + else: + HelmLogger.debug("Found value for path %s in environment %s", self.path, environment) return value def get_environments(self) -> Iterator: @@ -106,10 +114,22 @@ def to_dict(self) -> Dict[str, Any]: """ return { "path": self.path, - "metadata": self.metadata, - "values": {env: value.to_dict() for env, value in self._values.items()}, + "description": self._metadata.description, + "required": self._metadata.required, + "sensitive": self._metadata.sensitive, + "values": {env: value.get() for env, value in self._values.items()}, } + @property + def metadata(self) -> ConfigMetadata: + """ + Get metadata for this path. + + Returns: + ConfigMetadata: Metadata instance for this path + """ + return self._metadata + @classmethod def from_dict( cls, data: Dict[str, Any], create_value_fn: Callable[[str, str, Dict[str, Any]], Value] @@ -128,23 +148,28 @@ def from_dict( ValueError: If the dictionary structure is invalid """ if not isinstance(data, dict): - logger.error("Invalid data type provided: %s", type(data)) + HelmLogger.error("Invalid data type provided: %s", type(data)) raise ValueError("Data must be a dictionary") - logger.debug("Creating PathData from dict with path: %s", data.get("path")) + HelmLogger.debug("Creating PathData from dict with path: %s", data.get("path")) - required_keys = {"path", "metadata", "values"} + required_keys = {"path", "values"} if not all(key in data for key in required_keys): missing = required_keys - set(data.keys()) - logger.error("Missing required keys in data: %s", missing) + HelmLogger.error("Missing required keys in data: %s", missing) raise ValueError(f"Missing required keys: {missing}") - path_data = cls(path=data["path"], metadata=data["metadata"]) + metadata = { + "description": data.get("description"), + "required": data.get("required", False), + "sensitive": data.get("sensitive", False), + } + path_data = cls(path=data["path"], metadata=metadata) # Create Value instances for each environment - for env, value_data in data.get("values", {}).items(): - logger.debug("Creating value for environment %s", env) - value = create_value_fn(data["path"], env, value_data) - path_data.set_value(env, value) + for env, value in data.get("values", {}).items(): + HelmLogger.debug("Creating value for environment %s", env) + value_obj = create_value_fn(data["path"], env, {"value": value}) + path_data.set_value(env, value_obj) return path_data diff --git a/helm_values_manager/models/value.py b/helm_values_manager/models/value.py index 5d0062e..03d3f2f 100644 --- a/helm_values_manager/models/value.py +++ b/helm_values_manager/models/value.py @@ -6,10 +6,10 @@ """ from dataclasses import dataclass -from typing import Any, Dict +from typing import Any, Dict, Union from helm_values_manager.backends.base import ValueBackend -from helm_values_manager.utils.logger import logger +from helm_values_manager.utils.logger import HelmLogger @dataclass @@ -29,52 +29,48 @@ class Value: def __post_init__(self): """Post-initialization validation and logging.""" - logger.debug("Created Value instance for path %s in environment %s", self.path, self.environment) + HelmLogger.debug("Created Value instance for path %s in environment %s", self.path, self.environment) - def get(self, resolve: bool = False) -> str: + def get(self, resolve: bool = False) -> Union[str, int, float, bool, None]: """ - Get the value. + Get the value using the backend. Args: - resolve (bool): If True, resolve any secret references to their actual values. - If False, return the raw value which may be a secret reference. + resolve: Whether to resolve any secret references Returns: - str: The value (resolved or raw depending on resolve parameter) + The value from the backend, can be a string, number, boolean, or None Raises: - ValueError: If value doesn't exist RuntimeError: If backend operation fails """ - logger.debug("Getting value for path %s in environment %s", self.path, self.environment) try: value = self._backend.get_value(self.path, self.environment, resolve) - logger.debug("Successfully retrieved value for path %s", self.path) + HelmLogger.debug("Successfully retrieved value for path %s", self.path) return value except Exception as e: - logger.error("Failed to get value for path %s in environment %s: %s", self.path, self.environment, str(e)) + HelmLogger.error("Failed to get value for path %s in environment %s: %s", self.path, self.environment, e) raise - def set(self, value: str) -> None: + def set(self, value: Union[str, int, float, bool, None]) -> None: """ Set the value using the backend. Args: - value: The value to store, can be a raw value or a secret reference + value: The value to store, can be a raw value, a secret reference, or None Raises: - ValueError: If value is not a string + ValueError: If value is not a string, number, boolean, or None RuntimeError: If backend operation fails """ - if not isinstance(value, str): - raise ValueError("Value must be a string") + if not isinstance(value, (str, int, float, bool, type(None))): + raise ValueError("Value must be a string, number, boolean, or None") - logger.debug("Setting value for path %s in environment %s", self.path, self.environment) try: self._backend.set_value(self.path, self.environment, value) - logger.debug("Successfully set value for path %s", self.path) + HelmLogger.debug("Successfully set value for path %s", self.path) except Exception as e: - logger.error("Failed to set value for path %s in environment %s: %s", self.path, self.environment, str(e)) + HelmLogger.error("Failed to set value for path %s in environment %s: %s", self.path, self.environment, e) raise def to_dict(self) -> Dict[str, Any]: @@ -84,7 +80,7 @@ def to_dict(self) -> Dict[str, Any]: Returns: Dict[str, Any]: Dictionary representation of the Value instance """ - logger.debug("Converting Value to dict for path %s", self.path) + HelmLogger.debug("Converting Value to dict for path %s", self.path) return {"path": self.path, "environment": self.environment, "backend_type": self._backend.backend_type} @staticmethod diff --git a/helm_values_manager/schemas/v1.json b/helm_values_manager/schemas/v1.json index d5876e3..87a69e8 100644 --- a/helm_values_manager/schemas/v1.json +++ b/helm_values_manager/schemas/v1.json @@ -51,9 +51,9 @@ "properties": { "type": { "const": "file" } } }, "then": { - "required": ["file_path"], + "required": ["path"], "properties": { - "file_path": { "type": "string" } + "path": { "type": "string" } } } }, @@ -90,7 +90,7 @@ "description": "List of configuration values to manage", "items": { "type": "object", - "required": ["path", "required", "sensitive", "values"], + "required": ["path", "values"], "properties": { "path": { "type": "string", @@ -103,17 +103,19 @@ }, "required": { "type": "boolean", - "description": "Whether this value must be present in all environments" + "description": "Whether this value is required to be set in all environments", + "default": false }, "sensitive": { "type": "boolean", - "description": "Whether this value contains sensitive data. When true, the corresponding value should be a secret reference in the format: secret:///" + "description": "Whether this value contains sensitive data that should be stored securely", + "default": false }, "values": { "type": "object", - "description": "Map of environment names to their values", + "description": "Map of environment names to values", "additionalProperties": { - "type": "string" + "type": ["string", "number", "boolean", "null"] } } } diff --git a/helm_values_manager/utils/logger.py b/helm_values_manager/utils/logger.py index 9d80bdb..db2a658 100644 --- a/helm_values_manager/utils/logger.py +++ b/helm_values_manager/utils/logger.py @@ -47,7 +47,3 @@ def error(msg: str, *args: Any) -> None: if args: msg = msg % args print("Error: %s" % msg, file=sys.stderr) - - -# Global logger instance -logger = HelmLogger() diff --git a/tests/integration/test_cli_integration.py b/tests/integration/test_cli_integration.py index b6cb3ee..6d74d90 100644 --- a/tests/integration/test_cli_integration.py +++ b/tests/integration/test_cli_integration.py @@ -58,7 +58,9 @@ def test_help_command(plugin_install): assert returncode == 0 assert "Usage: helm values-manager [OPTIONS] COMMAND [ARGS]..." in stdout - assert "A Helm plugin to manage values and secrets across environments" in stdout + # Check for description split across lines + assert "A Helm plugin to manage values and secrets across" in stdout + assert "environments" in stdout assert "init" in stdout # Check that init command is listed diff --git a/tests/unit/backends/test_simple.py b/tests/unit/backends/test_simple.py index 40221ff..978f40f 100644 --- a/tests/unit/backends/test_simple.py +++ b/tests/unit/backends/test_simple.py @@ -24,9 +24,9 @@ def test_set_and_get_value(backend): def test_set_invalid_value_type(backend): - """Test setting a non-string value.""" - with pytest.raises(ValueError, match="Value must be a string"): - backend.set_value("app.replicas", "dev", 3) + """Test setting an invalid value type.""" + with pytest.raises(ValueError, match="Value must be a string, number, boolean, or None"): + backend.set_value("app.replicas", "dev", {"key": "value"}) # Dictionary is not a valid type def test_remove_value(backend): diff --git a/tests/unit/models/test_helm_values_config.py b/tests/unit/models/test_helm_values_config.py new file mode 100644 index 0000000..2df3106 --- /dev/null +++ b/tests/unit/models/test_helm_values_config.py @@ -0,0 +1,187 @@ +"""Test suite for HelmValuesConfig class.""" + +import jsonschema +import pytest + +from helm_values_manager.models.helm_values_config import HelmValuesConfig +from helm_values_manager.models.path_data import PathData + + +def test_helm_values_config_initialization(): + """Test initialization of HelmValuesConfig.""" + config = HelmValuesConfig() + assert config.deployments == {} + assert config._path_map == {} + + +def test_add_config_path(): + """Test adding a new configuration path.""" + config = HelmValuesConfig() + path = "app.config.key1" + description = "Test config" + + config.add_config_path(path, description=description, required=True, sensitive=False) + + assert path in config._path_map + path_data = config._path_map[path] + assert isinstance(path_data, PathData) + assert path_data.path == path + assert path_data.metadata.description == description + assert path_data.metadata.required is True + assert path_data.metadata.sensitive is False + + +def test_add_duplicate_path(): + """Test adding a duplicate path.""" + config = HelmValuesConfig() + path = "app.config.key1" + + config.add_config_path(path, description="Test config") + with pytest.raises(ValueError, match=f"Path {path} already exists"): + config.add_config_path(path, description="Another config") + + +def test_set_and_get_value(): + """Test setting and getting a value.""" + config = HelmValuesConfig() + path = "app.config.key1" + value = "test-value" + environment = "dev" + + config.add_config_path(path, description="Test config") + config.set_value(path, environment, value) + + assert config.get_value(path, environment) == value + + +def test_get_value_nonexistent_path(): + """Test getting a value for a non-existent path.""" + config = HelmValuesConfig() + with pytest.raises(KeyError, match=r"Path app\.config\.key1 not found"): + config.get_value("app.config.key1", "dev") + + +def test_get_value_nonexistent_environment(): + """Test getting a value for a non-existent environment.""" + config = HelmValuesConfig() + path = "app.config.key1" + config.add_config_path(path, description="Test config", required=True, sensitive=False) + assert config.get_value(path, "dev") is None + + +def test_get_value_nonexistent_value(): + """Test getting a value that doesn't exist.""" + config = HelmValuesConfig() + path = "app.config.key1" + config.add_config_path(path, description="Test config") + assert config.get_value(path, "dev") is None + + +def test_get_value_returns_none(): + """Test getting a value when value_obj.get() returns None.""" + config = HelmValuesConfig() + path = "app.config.key1" + environment = "dev" + + # Add path and set None value + config.add_config_path(path, description="Test config") + config.set_value(path, environment, None) + + # Verify that get_value returns None + assert config.get_value(path, environment) is None + + +def test_set_value_without_path(): + """Test setting a value without first adding its path.""" + config = HelmValuesConfig() + with pytest.raises(KeyError, match=r"Path app\.config\.key1 not found"): + config.set_value("app.config.key1", "dev", "test-value") + + +def test_to_dict_from_dict(): + """Test serialization and deserialization.""" + config_data = { + "version": "1.0", + "release": "test-release", + "deployments": { + "prod": { + "backend": "aws", + "auth": {"type": "env", "env_prefix": "AWS_"}, + "backend_config": {"region": "us-west-2"}, + } + }, + "config": [ + { + "path": "app.config.key1", + "description": "Test config", + "required": True, + "sensitive": True, + "values": {"default": "test-value"}, + } + ], + } + + # Test from_dict + config = HelmValuesConfig.from_dict(config_data) + assert config.release == "test-release" + assert config.version == "1.0" + assert len(config.deployments) == 1 + assert len(config._path_map) == 1 + + # Verify deployment data + deployment = config.deployments["prod"] + assert deployment.backend == "aws" + assert deployment.auth == {"type": "env", "env_prefix": "AWS_"} + assert deployment.backend_config == {"region": "us-west-2"} + + # Verify config data + path_data = config._path_map["app.config.key1"] + assert path_data.path == "app.config.key1" + assert path_data.metadata.description == "Test config" + assert path_data.metadata.required is True + assert path_data.metadata.sensitive is True + + # Test to_dict + config_dict = config.to_dict() + assert config_dict["version"] == "1.0" + assert config_dict["release"] == "test-release" + assert config_dict["deployments"] == { + "prod": { + "backend": "aws", + "auth": {"type": "env", "env_prefix": "AWS_"}, + "backend_config": {"region": "us-west-2"}, + } + } + assert len(config_dict["config"]) == 1 + + config_item = config_dict["config"][0] + assert config_item["path"] == "app.config.key1" + assert config_item["description"] == "Test config" + assert config_item["required"] is True + assert config_item["sensitive"] is True + assert config_item["values"] == {"default": "test-value"} + + +def test_validate(): + """Test validation of required paths.""" + config = HelmValuesConfig() + path = "app.config.key1" + config.add_config_path(path, description="Test config", required=True, sensitive=False) + + # 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 diff --git a/tests/unit/models/test_path_data.py b/tests/unit/models/test_path_data.py index 635603f..23ed2d9 100644 --- a/tests/unit/models/test_path_data.py +++ b/tests/unit/models/test_path_data.py @@ -32,9 +32,9 @@ def mock_value(): def test_path_data_init(path_data): """Test PathData initialization.""" assert path_data.path == "test.path" - assert path_data.metadata["description"] == "Test description" - assert path_data.metadata["required"] is True - assert path_data.metadata["sensitive"] is False + assert path_data.metadata.description == "Test description" + assert path_data.metadata.required is True + assert path_data.metadata.sensitive is False def test_set_value(path_data, mock_value): @@ -64,12 +64,16 @@ def test_get_environments(path_data, mock_value): assert "test2" in environments -def test_validate_path_mismatch(path_data): - """Test validation with mismatched paths.""" +def test_validate_path_mismatch(): + """Test validation when a value has a mismatched path.""" + path_data = PathData("test.path", {"required": True}) mock_value = Mock(spec=Value) mock_value.path = "wrong.path" - with pytest.raises(ValueError, match=r"Value path wrong\.path doesn't match PathData path test\.path"): - path_data.set_value("test_env", mock_value) + path_data._values["test_env"] = mock_value + with pytest.raises( + ValueError, match=r"Value for environment test_env has inconsistent path: wrong\.path != test\.path" + ): + path_data.validate() def test_validate_required_missing_value(path_data): @@ -103,7 +107,7 @@ def test_validate_success(path_data, mock_value): def test_validate_not_required(path_data, mock_value): """Test validation when path is not required.""" - path_data.metadata["required"] = False + path_data.metadata.required = False mock_value.get.return_value = None path_data.set_value("test_env", mock_value) path_data.validate() # Should not raise any error @@ -111,29 +115,25 @@ def test_validate_not_required(path_data, mock_value): def test_to_dict(path_data, mock_value): """Test converting PathData to dictionary.""" - mock_value.to_dict.return_value = {"value": "test_value"} + mock_value.get.return_value = "test_value" path_data.set_value("test_env", mock_value) result = path_data.to_dict() assert result["path"] == "test.path" - assert result["metadata"] == { - "description": "Test description", - "required": True, - "sensitive": False, - } - assert result["values"] == {"test_env": {"value": "test_value"}} + assert result["description"] == "Test description" + assert result["required"] is True + assert result["sensitive"] is False + assert result["values"] == {"test_env": "test_value"} def test_from_dict(): """Test creating PathData from dictionary.""" data = { "path": "test.path", - "metadata": { - "description": "Test description", - "required": True, - "sensitive": False, - }, - "values": {"test_env": {"value": "test_value"}}, + "description": "Test description", + "required": True, + "sensitive": False, + "values": {"test_env": "test_value"}, } def create_value_fn(path, env, value_data): @@ -144,12 +144,10 @@ def create_value_fn(path, env, value_data): path_data = PathData.from_dict(data, create_value_fn) assert path_data.path == "test.path" - assert path_data.metadata == { - "description": "Test description", - "required": True, - "sensitive": False, - } - assert len(list(path_data.get_environments())) == 1 + assert path_data.metadata.description == "Test description" + assert path_data.metadata.required is True + assert path_data.metadata.sensitive is False + assert len(path_data._values) == 1 assert "test_env" in path_data._values @@ -161,13 +159,15 @@ def test_from_dict_invalid_type(): def test_from_dict_missing_keys(): """Test from_dict with missing required keys.""" - data = {"path": "test.path"} # Missing metadata and values - with pytest.raises(ValueError) as exc_info: - PathData.from_dict(data, lambda p, e, d: None) + data = {"path": "test.path"} # Missing values field - # Check that both required keys are mentioned in the error - assert "metadata" in str(exc_info.value) - assert "values" in str(exc_info.value) + def create_value_fn(path, env, value_data): + mock_value = Mock(spec=Value) + mock_value.path = path + return mock_value + + with pytest.raises(ValueError, match=r"Missing required keys: {'values'}"): + PathData.from_dict(data, create_value_fn) def test_from_dict_value_path_mismatch(): @@ -183,5 +183,5 @@ def create_value_fn(path, env, value_data): mock_value.path = "wrong.path" # Mismatched path return mock_value - with pytest.raises(ValueError, match=r"Value path wrong\.path doesn't match PathData path test\.path"): + with pytest.raises(ValueError, match=r"Value path wrong\.path does not match PathData path test\.path"): PathData.from_dict(data, create_value_fn) diff --git a/tests/unit/models/test_schema_validation.py b/tests/unit/models/test_schema_validation.py new file mode 100644 index 0000000..ac1a3b2 --- /dev/null +++ b/tests/unit/models/test_schema_validation.py @@ -0,0 +1,150 @@ +"""Test suite for schema validation and data loading in HelmValuesConfig.""" + +import pytest +from jsonschema.exceptions import ValidationError + +from helm_values_manager.models.helm_values_config import HelmValuesConfig + + +def test_valid_minimal_config(): + """Test loading a minimal valid configuration.""" + config_data = {"version": "1.0", "release": "test-release", "deployments": {}, "config": []} + config = HelmValuesConfig.from_dict(config_data) + assert isinstance(config, HelmValuesConfig) + # Verify loaded data + assert config.release == "test-release" + assert config.version == "1.0" + assert len(config.deployments) == 0 + assert len(config._path_map) == 0 + + +def test_valid_full_config(): + """Test loading a valid configuration with all fields.""" + config_data = { + "version": "1.0", + "release": "test-release", + "deployments": { + "prod": { + "backend": "aws", + "auth": {"type": "env", "env_prefix": "AWS_"}, + "backend_config": {"region": "us-west-2"}, + } + }, + "config": [ + { + "path": "app.config.key1", + "description": "Test config", + "required": True, + "sensitive": True, + "values": {"default": "test-value"}, + } + ], + } + config = HelmValuesConfig.from_dict(config_data) + # Verify loaded data + assert config.release == "test-release" + assert config.version == "1.0" + assert len(config.deployments) == 1 + assert len(config._path_map) == 1 + + # Verify deployment data + deployment = config.deployments["prod"] + assert deployment.backend == "aws" + assert deployment.auth == {"type": "env", "env_prefix": "AWS_"} + assert deployment.backend_config == {"region": "us-west-2"} + + # Verify config data + path_data = config._path_map["app.config.key1"] + assert path_data.path == "app.config.key1" + assert path_data.metadata.description == "Test config" + assert path_data.metadata.required is True + assert path_data.metadata.sensitive is True + assert config.get_value("app.config.key1", "default") == "test-value" + + +def test_default_values(): + """Test that default values are properly set when fields are omitted.""" + config_data = { + "version": "1.0", + "release": "test-release", + "deployments": {}, + "config": [{"path": "app.config.key1", "values": {}}], + } + config = HelmValuesConfig.from_dict(config_data) + path_data = config._path_map["app.config.key1"] + assert path_data.metadata.description is None + assert path_data.metadata.required is False + assert path_data.metadata.sensitive is False + + +def test_type_coercion(): + """Test that values are properly coerced to their expected types.""" + config_data = { + "version": "1.0", + "release": "test-release", + "deployments": {}, + "config": [ + { + "path": "app.config.key1", + "required": True, # Proper boolean + "sensitive": False, # Proper boolean + "values": {}, + } + ], + } + config = HelmValuesConfig.from_dict(config_data) + path_data = config._path_map["app.config.key1"] + assert path_data.metadata.required is True + assert path_data.metadata.sensitive is False + + +def test_missing_required_fields(): + """Test that missing required fields raise validation error.""" + config_data = {"version": "1.0", "release": "test-release", "config": []} # Missing required 'deployments' field + with pytest.raises(ValidationError, match=r".*'deployments'.*required property.*"): + HelmValuesConfig.from_dict(config_data) + + +def test_invalid_version(): + """Test that invalid version raises validation error.""" + config_data = {"version": "2.0", "release": "test-release", "deployments": {}, "config": []} # Invalid version + with pytest.raises(ValidationError, match=r".*'2\.0'.*not one of.*"): + HelmValuesConfig.from_dict(config_data) + + +def test_invalid_backend_type(): + """Test that invalid backend type raises validation error.""" + config_data = { + "version": "1.0", + "release": "test-release", + "deployments": { + "prod": {"backend": "invalid", "auth": {"type": "env", "env_prefix": "AWS_"}} # Invalid backend type + }, + "config": [], + } + with pytest.raises(ValidationError, match=r".*'invalid'.*not one of.*"): + HelmValuesConfig.from_dict(config_data) + + +def test_invalid_auth_config(): + """Test that invalid auth configuration raises validation error.""" + config_data = { + "version": "1.0", + "release": "test-release", + "deployments": {"prod": {"backend": "aws", "auth": {"invalid": "config"}}}, # Invalid auth config + "config": [], + } + with pytest.raises(ValidationError, match=r".*'env_prefix' is a required property.*"): + HelmValuesConfig.from_dict(config_data) + + +def test_invalid_path_format(): + """Test that invalid path format raises validation error.""" + config_data = { + "version": "1.0", + "release": "test-release", + "deployments": {}, + "config": [{"path": "invalid/path", "required": True, "sensitive": False, "values": {}}], # Invalid path format + } + with pytest.raises(ValidationError, match=r".*'invalid/path'.*does not match.*"): + HelmValuesConfig.from_dict(config_data) diff --git a/tests/unit/models/test_value.py b/tests/unit/models/test_value.py index 33676fb..72716e9 100644 --- a/tests/unit/models/test_value.py +++ b/tests/unit/models/test_value.py @@ -49,11 +49,36 @@ def test_set_value(mock_backend): mock_backend.set_value.assert_called_once_with("app.replicas", "dev", "3") +def test_set_valid_types(mock_backend): + """Test setting various valid value types.""" + value = Value(path="app.replicas", environment="dev", _backend=mock_backend) + + # Test string + value.set("test-value") + mock_backend.set_value.assert_called_with("app.replicas", "dev", "test-value") + + # Test integer + value.set(42) + mock_backend.set_value.assert_called_with("app.replicas", "dev", 42) + + # Test float + value.set(3.14) + mock_backend.set_value.assert_called_with("app.replicas", "dev", 3.14) + + # Test boolean + value.set(True) + mock_backend.set_value.assert_called_with("app.replicas", "dev", True) + + # Test None + value.set(None) + mock_backend.set_value.assert_called_with("app.replicas", "dev", None) + + def test_set_invalid_type(mock_backend): - """Test setting a non-string value.""" + """Test setting an invalid value type.""" value = Value(path="app.replicas", environment="dev", _backend=mock_backend) - with pytest.raises(ValueError, match="Value must be a string"): - value.set(3) + with pytest.raises(ValueError, match="Value must be a string, number, boolean, or None"): + value.set({"key": "value"}) # Dictionary is not a valid type def test_to_dict(mock_backend):