diff --git a/docs/Design/low-level-design.md b/docs/Design/low-level-design.md index 1cd31ce..b7a642d 100644 --- a/docs/Design/low-level-design.md +++ b/docs/Design/low-level-design.md @@ -8,357 +8,268 @@ The core domain model consists of several key classes that manage configuration ```mermaid classDiagram + %% Core Configuration Manager class HelmValuesConfig { - +List~Deployment~ deployments - +List~ConfigValue~ config - -Dict~str,PathData~ _path_map - +from_dict(data: dict) HelmValuesConfig - +to_dict() dict - +validate() None - +get_value(path: str, environment: str) str - +set_value(path: str, environment: str, value: str) None - +add_config_path(path: str, description: str, required: bool, sensitive: bool) None + +str version + +str release + -Dict[str, Deployment] deployments + -Dict[str, PathData] _path_map + -SimpleValueBackend _backend + +add_config_path(path, description, required, sensitive) + +get_value(path, environment, resolve) + +set_value(path, environment, value) + +validate() + +to_dict() + +from_dict(data) } + %% Deployment Management Subsystem + class Deployment { + +str name + +Dict auth + +str backend + +Dict backend_config + +to_dict() + } + + class ValueBackend { + <> + +get_value(path, environment)* + +set_value(path, environment, value)* + +delete_value(path, environment)* + } + + class SimpleValueBackend { + -Dict _values + +get_value(path, environment) + +set_value(path, environment, value) + +delete_value(path, environment) + } + + %% Value Management Subsystem class PathData { +str path - +Dict metadata - -Dict~str,Value~ _values - +validate() None - +set_value(environment: str, value: Value) None - +get_value(environment: str) Optional[Value] - +get_environments() Iterator[str] - +to_dict() Dict - +from_dict(data: Dict, create_value_fn) PathData + -ConfigMetadata _metadata + -Dict[str, Value] _values + +get_value(environment) + +set_value(environment, value) + +validate() + +to_dict() + +from_dict(data) } class Value { +str path +str environment -ValueBackend _backend - +get() str - +set(value: str) None - +to_dict() dict - +from_dict(data: dict, backend: ValueBackend) Value + +get(resolve) + +set(value) + +delete() } - class ConfigValue { - +str path - +Optional~str~ description + class ConfigMetadata { + +str description +bool required +bool sensitive - +Dict~str,str~ values + +to_dict() + +from_dict(data) } - class Deployment { - +str name - +Dict~str,Any~ auth - +str backend - +Dict~str,Any~ backend_config - } + %% Relationships + HelmValuesConfig "1" *-- "*" Deployment : manages + HelmValuesConfig "1" *-- "*" PathData : manages + HelmValuesConfig "1" *-- "1" SimpleValueBackend : uses + PathData "1" *-- "1" ConfigMetadata : has + PathData "1" *-- "*" Value : owns + SimpleValueBackend ..|> ValueBackend : implements + Value "1" o-- "1" ValueBackend : uses +``` - class BaseCommand { - <> - +execute() Any - +load_config() HelmValuesConfig - +save_config(config: HelmValuesConfig) None - #run(config: HelmValuesConfig)* Any - } +### 2. Component Organization - class ValueBackend { - <> - +get_value(path: str, environment: str)* str - +set_value(path: str, environment: str, value: str)* None - +validate_auth_config(auth_config: dict)* None - } +1. **Core Configuration Manager** - class SimpleValueBackend { - -Dict~str,str~ values - +get_value(path: str, environment: str) str - +set_value(path: str, environment: str, value: str) None - } + - `HelmValuesConfig`: Central manager that coordinates between deployment configurations and value management. It maintains separate collections for deployments and path data, ensuring they remain decoupled while working together at runtime. - class AWSSecretsBackend { - -SecretsManagerClient client - +get_value(path: str, environment: str) str - +set_value(path: str, environment: str, value: str) None - +validate_auth_config(auth_config: dict) None - } +2. **Deployment Management Subsystem** - class AzureKeyVaultBackend { - -KeyVaultClient client - +get_value(path: str, environment: str) str - +set_value(path: str, environment: str, value: str) None - +validate_auth_config(auth_config: dict) None - } + - `Deployment`: Represents a deployment environment (e.g., dev, prod) and its associated backend configuration. Defines HOW values are stored. + - `ValueBackend`: Interface defining the contract for value storage backends. + - `SimpleValueBackend`: Default implementation for storing non-sensitive values in memory. - class HelmLogger { - +debug(msg: str, *args: Any) None - +error(msg: str, *args: Any) None - } +3. **Value Management Subsystem** + - `PathData`: Manages a configuration path's metadata and values. Contains WHAT is stored. + - `Value`: Represents a specific value for a path in an environment. + - `ConfigMetadata`: Stores metadata about a configuration path (description, required, sensitive). - HelmValuesConfig "1" *-- "*" ConfigValue - HelmValuesConfig "1" *-- "*" Deployment - HelmValuesConfig "1" *-- "*" PathData - PathData "1" *-- "*" Value - Value "1" o-- "1" ValueBackend - ValueBackend <|.. SimpleValueBackend - ValueBackend <|.. AWSSecretsBackend - ValueBackend <|.. AzureKeyVaultBackend - BaseCommand <|-- SetValueCommand - BaseCommand <|-- GetValueCommand -``` +### 3. Key Design Points -### 2. Value Management - -The system manages configuration values through a hierarchy of classes: - -1. **HelmValuesConfig** - - Top-level configuration manager - - Maintains mapping of paths to PathData instances - - Handles backend selection based on value sensitivity - - Manages deployments and their configurations - -2. **PathData** - - Represents a single configuration path and its properties - - Owns the configuration path and ensures consistency - - Stores metadata (description, required status, sensitivity) - - Manages environment-specific values through Value instances - - Validates path consistency between itself and its Values - - Delegates actual value storage to Value instances - -3. **Value** - - Handles actual value storage and retrieval - - Uses appropriate backend for storage operations - - Maintains reference to its path and environment - -This hierarchy ensures: -- Clear separation of concerns -- Consistent path handling across the system -- Proper validation at each level -- Flexible backend selection based on value sensitivity - -### 3. Command Pattern - -All CLI commands inherit from `BaseCommand` to ensure consistent behavior: - -```python -class BaseCommand: - def execute(self) -> Any: - try: - config = self.load_config() - result = self.run(config) - self.save_config(config) - return result - except Exception as e: - # Handle errors, cleanup if needed - raise - - def load_config(self) -> HelmValuesConfig: - # Implement file loading with locking - pass - - def save_config(self, config: HelmValuesConfig) -> None: - # Implement file saving with backup - pass - - @abstractmethod - def run(self, config: HelmValuesConfig) -> Any: - # Subclasses implement command-specific logic - pass -``` +1. **Separation of Concerns** -Benefits: -- Consistent file operations -- Built-in error handling -- Automatic file locking -- Configuration backup support + - Deployment configurations (HOW) and value management (WHAT) are separate subsystems + - `HelmValuesConfig` coordinates between them but they don't directly interact + - This allows independent evolution of storage backends and value management -#### Command Structure for Deployments and Backends +2. **Value Storage** -The command structure for managing deployments and backends follows a nested subcommand pattern (see [ADR-011](../ADRs/011-command-structure-for-deployments.md)): + - Values are stored in a flat structure using paths + - Each value knows its environment but isn't directly tied to a deployment + - The backend used for storage is determined at runtime based on deployment configuration -``` -helm values add-deployment [name] - -helm values add-backend [backend] --deployment=[name] [backend_options] - - helm values add-backend aws --deployment=prod --region=us-west-2 - - helm values add-backend azure --deployment=prod --vault-url=https://myvault.vault.azure.net - - helm values add-backend gcp --deployment=prod --project-id=my-project - - helm values add-backend git-secret --deployment=prod - -helm values add-auth [auth_type] --deployment=[name] [auth_options] - - helm values add-auth direct --deployment=prod --credentials='{...}' - - helm values add-auth env --deployment=prod --env-prefix=AWS_ - - helm values add-auth file --deployment=prod --auth-path=~/.aws/credentials - - helm values add-auth managed-identity --deployment=prod -``` +3. **Metadata Management** + - Each path has associated metadata managed by `PathData` + - Metadata influences validation and storage behavior but doesn't affect the deployment configuration -This structure: -- Separates the concerns of creating a deployment, configuring a backend, and setting up authentication -- Uses subcommands to provide context-specific options and help text -- Follows a natural workflow of first creating a deployment, then adding backend and auth configuration +### 4. Value Management -### 4. Storage Backends +The value management system is designed to be flexible and extensible, supporting different types of values and storage backends: -The `ValueBackend` interface defines the contract for value storage: +1. **Path-Based Value Organization** -```python -class ValueBackend(ABC): - @abstractmethod - def get_value(self, path: str, environment: str) -> str: - """Get a value from storage.""" - pass + - Values are organized by paths (e.g., "app.replicas", "db.password") + - Each path has associated metadata (description, required, sensitive) + - Values for each path can be set per environment - @abstractmethod - def set_value(self, path: str, environment: str, value: str) -> None: - """Store a value.""" - pass +2. **Value Storage and Retrieval** - @abstractmethod - def validate_auth_config(self, auth_config: dict) -> None: - """Validate backend-specific authentication configuration.""" - pass -``` + - Values are stored using backends defined by deployments + - Non-sensitive values use the SimpleValueBackend + - Sensitive values can use specialized backends (e.g., AWS Secrets Manager) + - Values can be retrieved in raw form or with secret references resolved -Implementations: -- SimpleValueBackend (for non-sensitive values) -- AWS Secrets Manager Backend -- Azure Key Vault Backend -- Additional backends can be easily added +3. **Validation Rules** -For a comprehensive overview of supported backends, authentication types, and backend configurations, see [Backends and Authentication Types](backends-and-auth.md). + - Paths must be unique across the configuration + - Required paths must have values in all environments + - Values must match their expected type (string, number, boolean) + - Secret references must be valid and resolvable -### 5. Schema Validation +4. **Security Considerations** + - Sensitive values are marked in metadata + - Different storage backends for sensitive vs non-sensitive data + - Secret references allow secure value resolution -The configuration system uses JSON Schema validation to ensure data integrity and consistency: +### 5. Configuration File Format -```mermaid -classDiagram - class SchemaValidator { - +validate_config(data: dict) None - -load_schema() dict - -handle_validation_error(error: ValidationError) str - } +The configuration file format is defined by a JSON schema that ensures consistency and validation of all configuration data. The schema is available at [`schemas/v1.json`](../helm_values_manager/schemas/v1.json). - class HelmValuesConfig { - +from_dict(data: dict) HelmValuesConfig - +to_dict() dict - +validate() None - } +Key aspects of the configuration format: - HelmValuesConfig ..> SchemaValidator : uses -``` +1. **Core Structure** + + - Version information (currently 1.0) + - Release name (for Helm release identification) + - Deployment configurations (backend and authentication settings) + - Value configurations (paths, metadata, and environment-specific values) + +2. **Validation Rules** + - Required fields (version, release, deployments, config) + - Value type constraints (string, number, boolean, null) + - Name format restrictions (lowercase alphanumeric with hyphens) + - Backend configuration requirements (backend type, auth method) + +### 6. Error Handling + +The system implements comprehensive error handling at multiple levels: + +1. **Validation Errors** + + - Schema validation using JSON Schema + - Path uniqueness checks + - Required value validation + - Type validation for values + - Deployment configuration validation + +2. **Runtime Errors** + + - Backend connection failures + - Authentication errors + - File access errors + - Concurrent access conflicts + +3. **Error Propagation** + - Low-level components raise specific exceptions + - Higher-level components catch and wrap errors with context + - Command layer translates errors to user-friendly messages + - All errors are logged with appropriate detail level + +### 7. Secret Reference Resolution -#### 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 +The system supports resolving secret references to their actual values: + +1. **Reference Format** + + - Secret references use the format: `secret:///` + - Example: `secret://aws/prod/db/password` + - References can be nested in string values + +2. **Resolution Process** + + - When `resolve=True` in get_value: + 1. Parse the value for secret references + 2. For each reference: + - Validate reference format + - Check backend availability + - Fetch actual value from backend + 3. Replace references with actual values + 4. Return resolved string + +3. **Security** + - References are stored instead of actual secrets + - Resolution happens only when explicitly requested + - Backend permissions control access to actual secrets + - Failed resolutions return None or raise error based on context ## Implementation Details ### 1. Configuration Structure -The configuration follows the v1 schema: +Below is an early example of the configuration structure. Note that the schema has evolved since then - for the latest schema definition, refer to [`schemas/v1.json`](../helm_values_manager/schemas/v1.json). This example is kept for historical context and to demonstrate the basic concepts: + ```json { - "version": "1.0", - "release": "my-app", - "deployments": { - "prod": { - "backend": "gcp", - "auth": { - "type": "managed_identity" - }, - "backend_config": { - "region": "us-central1" - } - } + "version": "1.0", + "release": "my-app", + "deployments": { + "prod": { + "backend": "gcp", + "auth": { + "type": "managed_identity" + }, + "backend_config": { + "region": "us-central1" + } + } + }, + "config": [ + { + "path": "app.replicas", + "description": "Number of application replicas", + "required": true, + "sensitive": false, + "values": { + "dev": "3", + "prod": "5" + } }, - "config": [ - { - "path": "app.replicas", - "description": "Number of application replicas", - "required": true, - "sensitive": false, - "values": { - "dev": "3", - "prod": "5" - } - }, - { - "path": "app.database.password", - "description": "Database password", - "required": true, - "sensitive": true, - "values": { - "dev": "secret://gcp-secrets/my-app/dev/db-password", - "prod": "secret://gcp-secrets/my-app/prod/db-password" - } - } - ] + { + "path": "app.database.password", + "description": "Database password", + "required": true, + "sensitive": true, + "values": { + "dev": "secret://gcp-secrets/my-app/dev/db-password", + "prod": "secret://gcp-secrets/my-app/prod/db-password" + } + } + ] } ``` ### 2. Value Resolution Process 1. Path Lookup: + - O(1) lookup in `_path_map` - Validates path existence - Retrieves value metadata @@ -371,6 +282,7 @@ The configuration follows the v1 schema: ### 3. Security Features 1. Value Protection: + - Sensitive value marking - Secure storage in remote backends - Authentication validation @@ -385,17 +297,20 @@ The configuration follows the v1 schema: The logging system follows Helm plugin conventions and provides consistent output formatting: 1. **HelmLogger Class** + - Provides debug and error logging methods - Follows Helm output conventions - Uses stderr for all output - Controls debug output via HELM_DEBUG environment variable 2. **Global Logger Instance** + - Available via `from helm_values_manager.utils.logger import logger` - Ensures consistent logging across all components - Simplifies testing with mock support 3. **Performance Features** + - Uses string formatting for efficiency - Lazy evaluation of debug messages - Minimal memory overhead @@ -408,16 +323,19 @@ The logging system follows Helm plugin conventions and provides consistent outpu ## Benefits of This Design 1. **Separation of Concerns** + - Domain logic in `HelmValuesConfig` - Storage logic in backends - Clean interface boundaries 2. **Extensibility** + - Easy to add new backends - Auth handling per backend - Consistent validation 3. **Maintainability** + - Central configuration management - Clear data flow - Type safety through domain model diff --git a/docs/Design/sequence-diagrams.md b/docs/Design/sequence-diagrams.md index 1148176..ba58088 100644 --- a/docs/Design/sequence-diagrams.md +++ b/docs/Design/sequence-diagrams.md @@ -661,19 +661,19 @@ sequenceDiagram Each diagram shows: - The exact CLI command being executed - All components involved in processing the command -- Data flow between components -- Validation steps -- File system operations -- Success/error handling - -The main CLI commands covered are: -1. `init` - Initialize new configuration -2. `add-value-config` - Define a new value configuration with metadata +- The sequence of operations and data flow +- Error handling and validation steps +- Lock management for concurrent access + +## Commands Covered + +1. `init` - Initialize a new helm-values configuration +2. `add-value-config` - Add a new value configuration with metadata 3. `add-deployment` - Add a new deployment configuration -4. `set-value` - Set a value for a specific path and environment -5. `get-value` - Retrieve a value for a specific path and environment -6. `validate` - Validate the entire configuration -7. `generate` - Generate values.yaml for a specific environment +4. `add-backend` - Add a backend to a deployment +5. `add-auth` - Add authentication to a deployment +6. `get-value` - Get a value for a specific path and environment +7. `set-value` - Set a value for a specific path and environment 8. `list-values` - List all values for a specific environment 9. `list-deployments` - List all deployments 10. `remove-deployment` - Remove a deployment configuration diff --git a/helm_values_manager/cli.py b/helm_values_manager/cli.py index 46dfe97..8ae493c 100644 --- a/helm_values_manager/cli.py +++ b/helm_values_manager/cli.py @@ -1,13 +1,12 @@ """Command line interface for the helm-values-manager plugin.""" -from typing import Optional - import typer from helm_values_manager.commands.add_deployment_command import AddDeploymentCommand from helm_values_manager.commands.add_value_config_command import AddValueConfigCommand from helm_values_manager.commands.init_command import InitCommand from helm_values_manager.commands.set_value_command import SetValueCommand +from helm_values_manager.models.config_metadata import ConfigMetadata from helm_values_manager.utils.logger import HelmLogger COMMAND_INFO = "helm values-manager" @@ -50,12 +49,14 @@ def init( @app.command("add-value-config") def add_value_config( path: str = typer.Option(..., "--path", "-p", help="Configuration path (e.g., 'app.replicas')"), - description: Optional[str] = typer.Option( - None, "--description", "-d", help="Description of what this configuration does" + description: str = typer.Option( + ConfigMetadata.DEFAULT_DESCRIPTION, "--description", "-d", help="Description of what this configuration does" + ), + required: bool = typer.Option( + ConfigMetadata.DEFAULT_REQUIRED, "--required", "-r", help="Whether this configuration is required" ), - required: bool = typer.Option(False, "--required", "-r", help="Whether this configuration is required"), sensitive: bool = typer.Option( - False, + ConfigMetadata.DEFAULT_SENSITIVE, "--sensitive", "-s", help="Whether this configuration contains sensitive data (coming in v0.2.0)", diff --git a/helm_values_manager/commands/add_value_config_command.py b/helm_values_manager/commands/add_value_config_command.py index 7a8a928..a1f2401 100644 --- a/helm_values_manager/commands/add_value_config_command.py +++ b/helm_values_manager/commands/add_value_config_command.py @@ -3,6 +3,7 @@ from typing import Optional from helm_values_manager.commands.base_command import BaseCommand +from helm_values_manager.models.config_metadata import ConfigMetadata from helm_values_manager.models.helm_values_config import HelmValuesConfig from helm_values_manager.utils.logger import HelmLogger @@ -35,9 +36,9 @@ def run(self, config: Optional[HelmValuesConfig] = None, **kwargs) -> str: if not path: raise ValueError("Path cannot be empty") - description = kwargs.get("description") - required = kwargs.get("required", False) - sensitive = kwargs.get("sensitive", False) + description = kwargs.get("description", ConfigMetadata.DEFAULT_DESCRIPTION) + required = kwargs.get("required", ConfigMetadata.DEFAULT_REQUIRED) + sensitive = kwargs.get("sensitive", ConfigMetadata.DEFAULT_SENSITIVE) try: # Add the new configuration path diff --git a/helm_values_manager/commands/base_command.py b/helm_values_manager/commands/base_command.py index 9bd3982..7228cb3 100644 --- a/helm_values_manager/commands/base_command.py +++ b/helm_values_manager/commands/base_command.py @@ -73,7 +73,12 @@ def save_config(self, config: HelmValuesConfig) -> None: Raises: IOError: If unable to write to the file. + ValueError: If the configuration is invalid (e.g., missing release name). + ValidationError: If JSON schema validation fails. """ + # Validate the config before saving + config.validate() + try: with open(self.config_file, "w", encoding="utf-8") as f: json.dump(config.to_dict(), f, indent=2) diff --git a/helm_values_manager/models/config_metadata.py b/helm_values_manager/models/config_metadata.py index 4e7906e..aefe07a 100644 --- a/helm_values_manager/models/config_metadata.py +++ b/helm_values_manager/models/config_metadata.py @@ -1,7 +1,7 @@ """Simple dataclass for configuration metadata.""" from dataclasses import asdict, dataclass -from typing import Any, Dict, Optional +from typing import Any, ClassVar, Dict @dataclass @@ -10,14 +10,19 @@ class ConfigMetadata: Represents metadata for a configuration path. Attributes: - description (Optional[str]): Description of the configuration path. + description (str): Description of the configuration path. Defaults to empty string. required (bool): Whether the configuration path is required. Defaults to False. sensitive (bool): Whether the configuration path is sensitive. Defaults to False. """ - description: Optional[str] = None - required: bool = False - sensitive: bool = False + # Default values as class variables for reference elsewhere + DEFAULT_DESCRIPTION: ClassVar[str] = "" + DEFAULT_REQUIRED: ClassVar[bool] = False + DEFAULT_SENSITIVE: ClassVar[bool] = False + + description: str = DEFAULT_DESCRIPTION + required: bool = DEFAULT_REQUIRED + sensitive: bool = DEFAULT_SENSITIVE def to_dict(self) -> Dict[str, Any]: """Convert metadata to dictionary.""" @@ -27,7 +32,7 @@ def to_dict(self) -> Dict[str, Any]: def from_dict(cls, data: Dict[str, Any]) -> "ConfigMetadata": """Create a ConfigMetadata instance from a dictionary.""" return cls( - description=data.get("description"), - required=data.get("required", False), - sensitive=data.get("sensitive", False), + description=data.get("description", cls.DEFAULT_DESCRIPTION), + required=data.get("required", cls.DEFAULT_REQUIRED), + sensitive=data.get("sensitive", cls.DEFAULT_SENSITIVE), ) diff --git a/helm_values_manager/models/helm_values_config.py b/helm_values_manager/models/helm_values_config.py index 1b5f534..15deac3 100644 --- a/helm_values_manager/models/helm_values_config.py +++ b/helm_values_manager/models/helm_values_config.py @@ -9,6 +9,7 @@ from jsonschema.exceptions import ValidationError from helm_values_manager.backends.simple import SimpleValueBackend +from helm_values_manager.models.config_metadata import ConfigMetadata 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 @@ -65,7 +66,11 @@ def _validate_schema(cls, data: dict) -> None: raise def add_config_path( - self, path: str, description: Optional[str] = None, required: bool = False, sensitive: bool = False + self, + path: str, + description: str = ConfigMetadata.DEFAULT_DESCRIPTION, + required: bool = ConfigMetadata.DEFAULT_REQUIRED, + sensitive: bool = ConfigMetadata.DEFAULT_SENSITIVE, ) -> None: """ Add a new configuration path. @@ -135,7 +140,8 @@ def validate(self) -> None: Validate the configuration. Raises: - ValueError: If validation fails. + ValueError: If validation fails (e.g., missing release name) + ValidationError: If JSON schema validation fails """ if not self.release: raise ValueError("Release name is required") @@ -191,9 +197,9 @@ def from_dict(cls, data: dict) -> "HelmValuesConfig": 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), + "description": config_item.get("description", ConfigMetadata.DEFAULT_DESCRIPTION), + "required": config_item.get("required", ConfigMetadata.DEFAULT_REQUIRED), + "sensitive": config_item.get("sensitive", ConfigMetadata.DEFAULT_SENSITIVE), } path_data = PathData(path, metadata) config._path_map[path] = path_data diff --git a/helm_values_manager/models/path_data.py b/helm_values_manager/models/path_data.py index 5e99d13..b75dfe4 100644 --- a/helm_values_manager/models/path_data.py +++ b/helm_values_manager/models/path_data.py @@ -15,7 +15,7 @@ class PathData: """Manages metadata and values for a configuration path.""" - def __init__(self, path: str, metadata: Dict[str, Any]): + def __init__(self, path: str, metadata: Optional[Dict[str, Any]] = None): """ Initialize PathData with a path and metadata. @@ -24,10 +24,12 @@ def __init__(self, path: str, metadata: Dict[str, Any]): metadata: Dictionary containing metadata for the path """ self.path = path + if metadata is None: + metadata = {} self._metadata = ConfigMetadata( - description=metadata.get("description"), - required=metadata.get("required", False), - sensitive=metadata.get("sensitive", False), + description=metadata.get("description", ConfigMetadata.DEFAULT_DESCRIPTION), + required=metadata.get("required", ConfigMetadata.DEFAULT_REQUIRED), + sensitive=metadata.get("sensitive", ConfigMetadata.DEFAULT_SENSITIVE), ) self._values: Dict[str, Value] = {} HelmLogger.debug("Created PathData instance for path %s", path) @@ -160,9 +162,9 @@ def from_dict( raise ValueError(f"Missing required keys: {missing}") metadata = { - "description": data.get("description"), - "required": data.get("required", False), - "sensitive": data.get("sensitive", False), + "description": data.get("description", ConfigMetadata.DEFAULT_DESCRIPTION), + "required": data.get("required", ConfigMetadata.DEFAULT_REQUIRED), + "sensitive": data.get("sensitive", ConfigMetadata.DEFAULT_SENSITIVE), } path_data = cls(path=data["path"], metadata=metadata) diff --git a/helm_values_manager/schemas/v1.json b/helm_values_manager/schemas/v1.json index 28a4f33..062fcb5 100644 --- a/helm_values_manager/schemas/v1.json +++ b/helm_values_manager/schemas/v1.json @@ -10,11 +10,17 @@ }, "release": { "type": "string", + "pattern": "^[a-z0-9][a-z0-9-]*[a-z0-9]$", + "maxLength": 53, "description": "Name of the Helm release" }, "deployments": { "type": "object", "description": "Map of deployment names to their configurations", + "propertyNames": { + "pattern": "^[a-z0-9][a-z0-9-]*[a-z0-9]$", + "description": "Deployment names must be lowercase alphanumeric with hyphens, cannot start/end with hyphen" + }, "additionalProperties": { "type": "object", "required": ["backend", "auth"], diff --git a/helm_values_manager/utils/logger.py b/helm_values_manager/utils/logger.py index 0c921e0..8086de3 100644 --- a/helm_values_manager/utils/logger.py +++ b/helm_values_manager/utils/logger.py @@ -15,7 +15,7 @@ class HelmLogger: Logger class that follows Helm plugin conventions. This logger: - 1. Writes debug messages only when HELM_DEBUG is set + 1. Writes debug messages only when HELM_DEBUG is set and not "0" or "false" 2. Writes all messages to stderr (Helm convention) 3. Uses string formatting for better performance 4. Provides consistent error and debug message formatting @@ -24,16 +24,18 @@ class HelmLogger: @staticmethod def debug(msg: str, *args: Any) -> None: """ - Print debug message if HELM_DEBUG is set. + Print debug message if HELM_DEBUG is set and not "0" or "false". Args: msg: Message with optional string format placeholders args: Values to substitute in the message """ - if os.environ.get("HELM_DEBUG"): - if args: - msg = msg % args - print("[debug] %s" % msg, file=sys.stderr) + debug_val = os.environ.get("HELM_DEBUG", "false").lower() + if debug_val in ("0", "false"): + return + if args: + msg = msg % args + print("[debug] %s" % msg, file=sys.stderr) @staticmethod def error(msg: str, *args: Any) -> None: diff --git a/tests/unit/commands/test_base_command.py b/tests/unit/commands/test_base_command.py index c869956..df5dc9a 100644 --- a/tests/unit/commands/test_base_command.py +++ b/tests/unit/commands/test_base_command.py @@ -159,29 +159,89 @@ def test_execute_ensures_lock_release_on_error(base_command, valid_config): def test_save_config_success(base_command): - """Test successful config saving.""" + """Test successful config save.""" config = HelmValuesConfig() config.version = "1.0" config.release = "test" + # Mock the validate method + config.validate = MagicMock() + with patch("builtins.open", mock_open()) as mock_file: base_command.save_config(config) + # Verify validate was called + config.validate.assert_called_once() + mock_file.assert_called_once_with(base_command.config_file, "w", encoding="utf-8") handle = mock_file() + # Verify the written data written_json = "".join(call.args[0] for call in handle.write.call_args_list) written_data = json.loads(written_json) - assert written_data["version"] == config.version - assert written_data["release"] == config.release + assert written_data["version"] == "1.0" + assert written_data["release"] == "test" def test_save_config_io_error(base_command): - """Test save_config when IO error occurs.""" + """Test IO error handling when saving config.""" config = HelmValuesConfig() - error_message = "Test error" + config.version = "1.0" + config.release = "test" - with patch("builtins.open", mock_open()) as mock_file: - mock_file.return_value.write.side_effect = IOError(error_message) - with pytest.raises(IOError, match=error_message): + # Mock the validate method + config.validate = MagicMock() + + # Simulate an IO error + mock_file = mock_open() + mock_file.side_effect = IOError("Test IO Error") + + with patch("builtins.open", mock_file): + with pytest.raises(IOError) as excinfo: base_command.save_config(config) + + assert "Test IO Error" in str(excinfo.value) + + # Verify validate was called + config.validate.assert_called_once() + + +def test_save_config_validates_schema(base_command): + """Test that save_config validates the schema before saving.""" + # Create a mock config + config = MagicMock(spec=HelmValuesConfig) + + # Make validate method raise an error + config.validate.side_effect = ValueError("Schema validation failed") + + # Try to save the config + with pytest.raises(ValueError, match="Schema validation failed"): + base_command.save_config(config) + + # Verify that validate was called + config.validate.assert_called_once() + + +def test_save_config_with_empty_description(base_command, tmp_path): + """Test that save_config handles empty description correctly.""" + # Create a real config + config = HelmValuesConfig() + config.release = "test" + + # Add a config path with empty description (default) + config.add_config_path("test.path") + + # Set a temporary config file + temp_file = tmp_path / "test_config.json" + base_command.config_file = str(temp_file) + + # Save the config + base_command.save_config(config) + + # Read the saved file + with open(temp_file, "r") as f: + data = json.load(f) + + # Verify that description is an empty string + assert data["config"][0]["description"] == "" + assert isinstance(data["config"][0]["description"], str) diff --git a/tests/unit/commands/test_init_command.py b/tests/unit/commands/test_init_command.py index db9184f..f45c49b 100644 --- a/tests/unit/commands/test_init_command.py +++ b/tests/unit/commands/test_init_command.py @@ -36,18 +36,20 @@ def test_initialization(init_command, tmp_path): def test_run_creates_config(init_command): """Test that run creates a new config with release name.""" - with patch("builtins.open", mock_open()) as mock_file: - result = init_command.run(release_name="test-release") - assert result == "Successfully initialized helm-values configuration." - - # Verify config was saved with correct data - handle = mock_file() - written_json = "".join(call.args[0] for call in handle.write.call_args_list) - written_data = json.loads(written_json) - assert written_data["version"] == "1.0" - assert written_data["release"] == "test-release" - assert written_data["deployments"] == {} - assert written_data["config"] == [] + # Mock the validate method + with patch("helm_values_manager.models.helm_values_config.HelmValuesConfig.validate"): + with patch("builtins.open", mock_open()) as mock_file: + result = init_command.run(release_name="test-release") + assert result == "Successfully initialized helm-values configuration." + + # Verify config was saved with correct data + handle = mock_file() + written_json = "".join(call.args[0] for call in handle.write.call_args_list) + written_data = json.loads(written_json) + assert written_data["version"] == "1.0" + assert written_data["release"] == "test-release" + assert written_data["deployments"] == {} + assert written_data["config"] == [] def test_run_with_existing_config(init_command): diff --git a/tests/unit/commands/test_set_value_command.py b/tests/unit/commands/test_set_value_command.py index ee9ae8f..57982c5 100644 --- a/tests/unit/commands/test_set_value_command.py +++ b/tests/unit/commands/test_set_value_command.py @@ -1,7 +1,7 @@ """Tests for the set-value command.""" import json -from unittest.mock import mock_open, patch +from unittest.mock import MagicMock, mock_open, patch import pytest @@ -133,3 +133,16 @@ def test_set_value_none_config(command): with patch.object(command, "load_config", return_value=None): with pytest.raises(ValueError, match="Configuration not loaded"): command.execute(path="app.replicas", environment="dev", value="3") + + +def test_set_value_general_error(): + """Test that general errors are properly handled.""" + command = SetValueCommand() + config = MagicMock() + config.deployments = {"dev": MagicMock()} # Mock deployment exists + config.set_value.side_effect = Exception("Unexpected error") + + with pytest.raises(Exception) as exc_info: + command.run(config, path="app.replicas", environment="dev", value="3") + + assert str(exc_info.value) == "Unexpected error" diff --git a/tests/unit/models/test_helm_values_config.py b/tests/unit/models/test_helm_values_config.py index 4a71aa8..ca90fc7 100644 --- a/tests/unit/models/test_helm_values_config.py +++ b/tests/unit/models/test_helm_values_config.py @@ -194,3 +194,74 @@ def test_validate_without_release(): config = HelmValuesConfig() with pytest.raises(ValueError, match="Release name is required"): config.validate() + + +def test_release_name_validation(): + """ + Test that release names are properly validated. + + Release names must match the following pattern: ^[a-z0-9-]{1,53}$. + """ + # Valid cases + valid_names = [ + "my-release", + "release123", + "a-b-c-123", + "a" * 53, # max length + ] + for name in valid_names: + config = {"version": "1.0", "release": name, "deployments": {}, "config": []} + HelmValuesConfig._validate_schema(config) # Should not raise + + # Invalid cases + invalid_names = [ + "UPPERCASE", # uppercase not allowed + "my_release", # underscore not allowed + "my.release", # dot not allowed + "-starts-with-hyphen", # cannot start with hyphen + "ends-with-hyphen-", # cannot end with hyphen + "a" * 54, # too long + "", # empty string + ] + for name in invalid_names: + config = {"version": "1.0", "release": name, "deployments": {}, "config": []} + with pytest.raises(jsonschema.exceptions.ValidationError): + HelmValuesConfig._validate_schema(config) + + +def test_deployment_name_validation(): + """ + Test that deployment names are properly validated. + + Deployment names must match the following pattern: ^[a-z0-9-]{1,53}$. + """ + # Valid cases + valid_config = { + "version": "1.0", + "release": "test-release", + "deployments": { + "my-deployment": {"backend": "no-backend", "auth": {"type": "no-auth"}}, + "deployment123": {"backend": "no-backend", "auth": {"type": "no-auth"}}, + "a-b-c-123": {"backend": "no-backend", "auth": {"type": "no-auth"}}, + }, + "config": [], + } + HelmValuesConfig._validate_schema(valid_config) # Should not raise + + # Invalid cases - test one by one to ensure proper validation + base_config = {"version": "1.0", "release": "test-release", "deployments": {}, "config": []} + + invalid_names = [ + "UPPERCASE", # uppercase not allowed + "my_deployment", # underscore not allowed + "my.deployment", # dot not allowed + "-starts-with-hyphen", # cannot start with hyphen + "ends-with-hyphen-", # cannot end with hyphen + "", # empty string + ] + + for name in invalid_names: + config = dict(base_config) + config["deployments"] = {name: {"backend": "no-backend", "auth": {"type": "no-auth"}}} + with pytest.raises(jsonschema.exceptions.ValidationError): + HelmValuesConfig._validate_schema(config) diff --git a/tests/unit/models/test_path_data.py b/tests/unit/models/test_path_data.py index 23ed2d9..916d388 100644 --- a/tests/unit/models/test_path_data.py +++ b/tests/unit/models/test_path_data.py @@ -37,6 +37,18 @@ def test_path_data_init(path_data): assert path_data.metadata.sensitive is False +def test_path_data_init_no_metadata(): + """Test PathData initialization with no metadata.""" + path = "test.path" + path_data = PathData(path) + + assert path_data.path == path + assert path_data.metadata.description == "" + assert path_data.metadata.required is False + assert path_data.metadata.sensitive is False + assert len(path_data._values) == 0 + + def test_set_value(path_data, mock_value): """Test setting a value.""" path_data.set_value("test", mock_value) diff --git a/tests/unit/models/test_schema_validation.py b/tests/unit/models/test_schema_validation.py index ac1a3b2..778ce24 100644 --- a/tests/unit/models/test_schema_validation.py +++ b/tests/unit/models/test_schema_validation.py @@ -72,7 +72,7 @@ def test_default_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.description == "" assert path_data.metadata.required is False assert path_data.metadata.sensitive is False diff --git a/tests/unit/utils/test_logger.py b/tests/unit/utils/test_logger.py index 33ad67f..7e7a72b 100644 --- a/tests/unit/utils/test_logger.py +++ b/tests/unit/utils/test_logger.py @@ -34,6 +34,28 @@ def test_debug_with_helm_debug_disabled(logger): assert stderr.getvalue() == "" +def test_debug_with_helm_debug_zero(logger): + """Test debug output when HELM_DEBUG=0.""" + stderr = StringIO() + with ( + mock.patch.dict(os.environ, {"HELM_DEBUG": "0"}), + mock.patch("helm_values_manager.utils.logger.sys.stderr", stderr), + ): + logger.debug("Test message") + assert stderr.getvalue() == "" + + +def test_debug_with_helm_debug_false(logger): + """Test debug output when HELM_DEBUG=false.""" + stderr = StringIO() + with ( + mock.patch.dict(os.environ, {"HELM_DEBUG": "false"}), + mock.patch("helm_values_manager.utils.logger.sys.stderr", stderr), + ): + logger.debug("Test message") + assert stderr.getvalue() == "" + + def test_debug_with_formatting(logger): """Test debug output with string formatting.""" stderr = StringIO() diff --git a/tools/test-plugin.sh b/tools/test-plugin.sh new file mode 100755 index 0000000..697543b --- /dev/null +++ b/tools/test-plugin.sh @@ -0,0 +1,100 @@ +#!/bin/bash +set -e # Exit on any error + +# Colors for output +GREEN='\033[0;32m' +RED='\033[0;31m' +NC='\033[0m' # No Color + +# Default values +INSTALL_SOURCE="local" +GITHUB_URL="https://github.com/Zipstack/helm-values-manager" # Correct capitalization +DEBUG_FLAG="" + +# Parse command line arguments +while [[ $# -gt 0 ]]; do + case $1 in + --source) + if [[ "$2" != "local" && "$2" != "github" ]]; then + echo "Error: --source must be either 'local' or 'github'" + exit 1 + fi + INSTALL_SOURCE="$2" + shift 2 + ;; + --github-url) + # Remove .git suffix if present + GITHUB_URL="${2%.git}" + shift 2 + ;; + --debug) + DEBUG_FLAG="--debug" + shift + ;; + *) + echo "Unknown option: $1" + echo "Usage: $0 [--source local|github] [--github-url URL] [--debug]" + echo "Example:" + echo " $0 # Install from local directory" + echo " $0 --source github # Install from default GitHub repo" + echo " $0 --source github --github-url URL # Install from custom GitHub repo" + echo " $0 --debug # Run with debug output" + exit 1 + ;; + esac +done + +# Get the absolute path to the plugin directory if installing locally +if [ "$INSTALL_SOURCE" = "local" ]; then + PLUGIN_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +fi + +echo "Installing helm-values-manager plugin..." +# Remove existing plugin if it exists +helm plugin remove values-manager 2>/dev/null || true + +# Install plugin based on source +if [ "$INSTALL_SOURCE" = "local" ]; then + echo "Installing from local directory: $PLUGIN_DIR" + helm plugin install "$PLUGIN_DIR" +else + echo "Installing from GitHub: $GITHUB_URL" + helm plugin install "$GITHUB_URL" +fi + +echo -e "\n${GREEN}Running test sequence...${NC}" + +# Clean up any existing test files +rm -f helm-values.json .helm-values.lock + +# Initialize with a valid release name +echo -e "\n${GREEN}1. Initializing helm values configuration...${NC}" +helm values-manager init -r "test-release" $DEBUG_FLAG + +# Add deployments +echo -e "\n${GREEN}2. Adding deployments...${NC}" +helm values-manager add-deployment test-deployment $DEBUG_FLAG +helm values-manager add-deployment dev $DEBUG_FLAG +helm values-manager add-deployment prod $DEBUG_FLAG + +# Add value configurations +echo -e "\n${GREEN}3. Adding value configurations...${NC}" +helm values-manager add-value-config -p "app.config.name" -d "Application name" -r $DEBUG_FLAG +helm values-manager add-value-config -p "app.config.replicas" -d "Number of replicas" $DEBUG_FLAG + +# Set values for different environments +echo -e "\n${GREEN}4. Setting values...${NC}" +helm values-manager set-value -p "app.config.name" -v "my-test-app" -d dev $DEBUG_FLAG +helm values-manager set-value -p "app.config.replicas" -v "1" -d dev $DEBUG_FLAG +helm values-manager set-value -p "app.config.name" -v "my-prod-app" -d prod $DEBUG_FLAG +helm values-manager set-value -p "app.config.replicas" -v "3" -d prod $DEBUG_FLAG + +# Verify configurations +echo -e "\n${GREEN}5. Verifying configurations...${NC}" +echo -e "\nhelm-values.json contents:" +cat helm-values.json + +echo -e "\n.helm-values.lock contents:" +cat .helm-values.lock + +echo -e "\n${GREEN}Test sequence completed successfully!${NC}"