diff --git a/docs/ADRs/005-unified-backend-approach.md b/docs/ADRs/005-unified-backend-approach.md index 1305cce..bdb2155 100644 --- a/docs/ADRs/005-unified-backend-approach.md +++ b/docs/ADRs/005-unified-backend-approach.md @@ -1,7 +1,7 @@ # ADR-005: Unified Backend Approach for Value Storage ## Status -Proposed +Accepted ## Context Currently, the Value class handles two distinct storage types: local and remote. This creates a split in logic within the Value class, requiring different code paths and validation rules based on the storage type. This complexity makes the code harder to maintain and test. diff --git a/docs/ADRs/007-sensitive-value-storage.md b/docs/ADRs/007-sensitive-value-storage.md new file mode 100644 index 0000000..56c1800 --- /dev/null +++ b/docs/ADRs/007-sensitive-value-storage.md @@ -0,0 +1,95 @@ +# ADR 007: Sensitive Value Storage + +## Status +Accepted + +## Context +The helm-values-manager needs to handle both sensitive and non-sensitive configuration values. While non-sensitive values can be stored directly in the configuration files, sensitive values require special handling to ensure security. + +## Decision +We will store sensitive values using the existing configuration structure, with sensitive values using a special reference format. +The existing schema already supports this through its `sensitive` flag and flexible string values. + +1. When a value is marked as sensitive (`sensitive: true`): + - The actual value will be stored in a secure backend (AWS Secrets Manager, Azure Key Vault, etc.) + - Only a reference to the secret will be stored in our configuration files + - The reference will use a URI-like format: `secret:///` + +2. Example configuration showing both sensitive and non-sensitive values: +```json +{ + "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" + } + }, + { + "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" + } + } + ] +} +``` + +This approach: +1. Leverages the existing schema without modifications: + - Uses the `sensitive` flag to identify sensitive values + - Uses the flexible string type in `values` to store references + - Maintains backward compatibility +2. Provides a clear and consistent format for secret references +3. Supports versioning through the secret path (e.g., `secret://gcp-secrets/my-app/prod/db-password/v1`) + +The validation and resolution of secret references will be handled by: +1. The backend implementation (parsing and resolving references) +2. The `Value` class (determining whether to treat a value as a reference based on the `sensitive` flag) + +## Implementation Notes +1. Secret references will be parsed and validated by the backend implementation +2. The `Value` class will check the `sensitive` flag to determine how to handle the value: + - If `sensitive: false`, use the value as-is + - If `sensitive: true`, parse the value as a secret reference and resolve it +3. Each secure backend will implement its own reference resolution logic +4. Future enhancement: Add commands to manage secrets directly through the tool + +## Consequences + +### Positive +- Security: Sensitive values never leave the secure backend +- Traceability: Easy to track which secrets are used where +- Versioning: Support for secret rotation via version references +- Flexibility: Different backends can implement their own reference formats +- Auditability: References are human-readable for easier debugging + +### Negative +- Additional Setup: Users need to create secrets separately (until we add direct creation support) +- Complexity: Need to manage both direct values and secret references +- Dependencies: Requires access to secure backends + +## Related +- ADR 0001 (if it exists, about general value storage) +- Future ADR about secret management commands diff --git a/docs/ADRs/README.md b/docs/ADRs/README.md index e9bb760..74031d6 100644 --- a/docs/ADRs/README.md +++ b/docs/ADRs/README.md @@ -35,7 +35,7 @@ An Architecture Decision Record (ADR) is a document that captures an important a - **Dependencies**: ADR-003 ### [ADR-005: Unified Backend Approach](005-unified-backend-approach.md) -- **Status**: Proposed +- **Status**: Accepted - **Context**: Split logic in Value class for different storage types - **Decision**: Remove storage type distinction, use SimpleValueBackend - **Impact**: Simplifies Value class and unifies storage interface @@ -48,6 +48,13 @@ An Architecture Decision Record (ADR) is a document that captures an important a - **Impact**: Ensures consistent user experience and debugging - **Dependencies**: ADR-001 +### [ADR-007: Sensitive Value Storage](007-sensitive-value-storage.md) +- **Status**: Accepted +- **Context**: Need for secure handling of sensitive configuration values +- **Decision**: Store sensitive values using reference-based approach in secure backends +- **Impact**: Ensures security while maintaining flexibility and traceability +- **Dependencies**: ADR-005 + ## ADR Template For new ADRs, use this template: ```markdown diff --git a/docs/Design/low-level-design.md b/docs/Design/low-level-design.md index a85804f..22283bc 100644 --- a/docs/Design/low-level-design.md +++ b/docs/Design/low-level-design.md @@ -23,10 +23,11 @@ classDiagram class PathData { +str path +Dict metadata - +Dict~str,Value~ values + -Dict~str,Value~ _values +validate() None - +add_value(environment: str, value: Value) 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 } @@ -212,12 +213,12 @@ The configuration follows the v1 schema: "release": "my-app", "deployments": { "prod": { - "backend": "aws", + "backend": "gcp", "auth": { "type": "managed_identity" }, "backend_config": { - "region": "us-west-2" + "region": "us-central1" } } }, @@ -231,6 +232,16 @@ The configuration follows the v1 schema: "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" + } } ] } diff --git a/docs/Development/tasks.md b/docs/Development/tasks.md index 642f87e..d7241b7 100644 --- a/docs/Development/tasks.md +++ b/docs/Development/tasks.md @@ -30,6 +30,20 @@ - [x] Implement to_dict() method - [x] Implement from_dict() method - [x] Add tests for serialization +- [x] Add metadata handling + - [x] Create ConfigMetadata class + - [x] Add tests for ConfigMetadata + - [x] Integrate with PathData + +### ConfigMetadata +- [x] Implement ConfigMetadata class + - [x] Add metadata attributes + - [x] Implement constructor with type hints + - [x] Add basic validation for attributes +- [x] Add serialization support + - [x] Implement to_dict() method + - [x] Implement from_dict() static method + - [x] Add tests for serialization/deserialization ### HelmValuesConfig Refactoring - [ ] Remove PlainTextBackend references @@ -82,6 +96,7 @@ - [x] Add unit tests - [x] Value class tests - [x] PathData class tests + - [x] ConfigMetadata tests - [ ] HelmValuesConfig tests - [ ] Backend tests - [ ] Command tests @@ -110,6 +125,7 @@ - [ ] Update API documentation - [ ] Document Value class - [ ] Document PathData class + - [ ] Document ConfigMetadata class - [ ] Update HelmValuesConfig docs - [ ] Add usage examples - [ ] Basic usage examples diff --git a/helm_values_manager/backends/__init__.py b/helm_values_manager/backends/__init__.py index 6f2a7d4..891ee62 100644 --- a/helm_values_manager/backends/__init__.py +++ b/helm_values_manager/backends/__init__.py @@ -5,6 +5,6 @@ different storage systems like AWS Secrets Manager or Azure Key Vault. """ -from .base import ValueBackend +from helm_values_manager.backends.base import ValueBackend __all__ = ["ValueBackend"] diff --git a/helm_values_manager/backends/base.py b/helm_values_manager/backends/base.py index 9a8d6a0..a3387be 100644 --- a/helm_values_manager/backends/base.py +++ b/helm_values_manager/backends/base.py @@ -35,6 +35,7 @@ def __init__(self, auth_config: Dict[str, str]) -> None: ValueError: If the auth_config is invalid """ self._validate_auth_config(auth_config) + self.backend_type = self.__class__.__name__.lower().replace("backend", "") def _validate_auth_config(self, auth_config: Dict[str, str]) -> None: """Validate the authentication configuration. @@ -59,20 +60,18 @@ 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) -> str: - """Get a value from the storage backend. + def get_value(self, path: str, environment: str, resolve: bool = False) -> str: + """ + Get a value from storage. Args: - path: The configuration path (e.g., "app.replicas") - environment: The environment name (e.g., "dev", "prod") + 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: - str: The value stored in the backend. - - Raises: - ValueError: If the value doesn't exist - ConnectionError: If there's an error connecting to the backend - PermissionError: If there's an authentication or authorization error + str: The value (resolved or raw depending on resolve parameter) """ pass diff --git a/helm_values_manager/backends/simple.py b/helm_values_manager/backends/simple.py index 48d201a..b9893a7 100644 --- a/helm_values_manager/backends/simple.py +++ b/helm_values_manager/backends/simple.py @@ -6,7 +6,7 @@ from typing import Dict -from .base import ValueBackend +from helm_values_manager.backends.base import ValueBackend class SimpleValueBackend(ValueBackend): @@ -25,16 +25,18 @@ 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) -> str: + def get_value(self, path: str, environment: str, resolve: bool = False) -> str: """ 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. Returns: - str: The stored value + str: The value (resolved or raw depending on resolve parameter) Raises: ValueError: If the value doesn't exist diff --git a/helm_values_manager/models/__init__.py b/helm_values_manager/models/__init__.py index ccedfb9..ef94c04 100644 --- a/helm_values_manager/models/__init__.py +++ b/helm_values_manager/models/__init__.py @@ -1,5 +1,5 @@ """Models package for helm-values-manager.""" -from .value import Value +from helm_values_manager.models.value import Value __all__ = ["Value"] diff --git a/helm_values_manager/models/config_metadata.py b/helm_values_manager/models/config_metadata.py new file mode 100644 index 0000000..4e7906e --- /dev/null +++ b/helm_values_manager/models/config_metadata.py @@ -0,0 +1,33 @@ +"""Simple dataclass for configuration metadata.""" + +from dataclasses import asdict, dataclass +from typing import Any, Dict, Optional + + +@dataclass +class ConfigMetadata: + """ + Represents metadata for a configuration path. + + Attributes: + description (Optional[str]): Description of the configuration path. + 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 + + def to_dict(self) -> Dict[str, Any]: + """Convert metadata to dictionary.""" + return asdict(self) + + @classmethod + 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), + ) diff --git a/helm_values_manager/models/path_data.py b/helm_values_manager/models/path_data.py index cce318c..1ea62f5 100644 --- a/helm_values_manager/models/path_data.py +++ b/helm_values_manager/models/path_data.py @@ -1,39 +1,27 @@ """ -PathData class for managing path-specific metadata and values. +Module for managing path data and associated metadata. This module contains the PathData class which is responsible for managing metadata and values associated with a specific configuration path. """ -from typing import Any, Dict, Iterator, Optional +from typing import Any, Callable, Dict, Iterator, Optional -from ..utils.logger import logger -from .value import Value +from helm_values_manager.models.value import Value +from helm_values_manager.utils.logger import logger class PathData: - """ - Class representing metadata and values for a specific configuration path. + """Manages metadata and values for a configuration path.""" - This class manages both the metadata (like description, required status, - sensitivity) and the actual values (as Value objects) for a configuration path. - - Attributes: - path (str): The configuration path (e.g., "app.replicas") - metadata (Dict[str, Any]): Dictionary containing path metadata like - description, required status, and sensitivity. - """ - - def __init__(self, path: str, metadata: Dict[str, Any]) -> None: + def __init__(self, path: str, metadata: Dict[str, Any]): """ - Initialize a new PathData instance. + Initialize PathData with a path and metadata. Args: - path (str): The configuration path - metadata (Dict[str, Any]): Dictionary containing path metadata. - Expected keys: description (str), required (bool), sensitive (bool) + path: The configuration path + metadata: Dictionary containing metadata fields """ - logger.debug("Creating new PathData instance for path: %s", path) self.path = path self.metadata = metadata self._values: Dict[str, Value] = {} @@ -43,55 +31,35 @@ def validate(self) -> None: Validate the PathData instance. Ensures that: - 1. Required metadata fields are present - 2. Metadata fields have correct types - 3. If path is marked as required, all environments have values - 4. All Value instances use the same path as this PathData + 1. All Value instances use the same path as this PathData + 2. If path is marked as required, each environment has a non-empty value Raises: ValueError: If validation fails """ logger.debug("Validating PathData for path: %s", self.path) - # Validate metadata structure - required_fields = {"description", "required", "sensitive"} - missing_fields = required_fields - set(self.metadata.keys()) - if missing_fields: - logger.error("Missing required metadata fields for path %s: %s", self.path, missing_fields) - raise ValueError(f"Missing required metadata fields: {missing_fields}") - - # Validate metadata types - if not isinstance(self.metadata["description"], (str, type(None))): - logger.error("Invalid description type for path %s", self.path) - raise ValueError("Description must be a string or None") - if not isinstance(self.metadata["required"], bool): - logger.error("Invalid required flag type for path %s", self.path) - raise ValueError("Required flag must be a boolean") - if not isinstance(self.metadata["sensitive"], bool): - logger.error("Invalid sensitive flag type for path %s", self.path) - raise ValueError("Sensitive flag must be a boolean") - - # Validate path consistency + # 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) raise ValueError(f"Value for environment {env} has inconsistent path: {value.path} != {self.path}") - # If path is required, ensure all environments have values - if self.metadata["required"]: - logger.debug("Path %s is marked as required", self.path) - # Note: The actual environment validation would need to be done - # at a higher level since PathData doesn't know about all environments + # Check required values + if self.metadata.get("required", False): + val = value.get() + if val is None or val == "": + logger.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: """ Set a Value object for a specific environment. - If a value already exists for the environment, it will be replaced. - Args: - environment (str): The environment name - value (Value): The Value object to associate with the environment + environment (str): The environment to set the value for + value (Value): The Value object to set Raises: ValueError: If the Value object's path doesn't match this PathData's path @@ -102,11 +70,7 @@ def set_value(self, environment: str, value: Value) -> None: 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}") - was_update = environment in self._values self._values[environment] = value - logger.debug( - "%s value for path %s in environment %s", "Updated" if was_update else "Added", self.path, environment - ) def get_value(self, environment: str) -> Optional[Value]: """ @@ -124,24 +88,22 @@ def get_value(self, environment: str) -> Optional[Value]: logger.debug("No value found for path %s in environment %s", self.path, environment) return value - def get_environments(self) -> Iterator[str]: + def get_environments(self) -> Iterator: """ - Get an iterator over all environments that have values. + Get all environment names that have values. Returns: - Iterator[str]: Iterator of environment names + Iterator: Iterator of environment names """ - logger.debug("Getting environments for path %s", self.path) return iter(self._values.keys()) def to_dict(self) -> Dict[str, Any]: """ - Convert the PathData instance to a dictionary. + Convert PathData to a dictionary. Returns: - Dict[str, Any]: Dictionary representation of the PathData instance + Dict[str, Any]: Dictionary representation of PathData """ - logger.debug("Converting PathData to dict for path %s", self.path) return { "path": self.path, "metadata": self.metadata, @@ -149,14 +111,15 @@ def to_dict(self) -> Dict[str, Any]: } @classmethod - def from_dict(cls, data: Dict[str, Any], create_value_fn) -> "PathData": + def from_dict( + cls, data: Dict[str, Any], create_value_fn: Callable[[str, str, Dict[str, Any]], Value] + ) -> "PathData": """ Create a PathData instance from a dictionary. Args: data (Dict[str, Any]): Dictionary containing PathData data - create_value_fn: Function to create Value objects from dict data. - Should accept (path: str, environment: str, data: Dict) as arguments. + create_value_fn: Function to create Value instances. Takes (path, env, value_data) and returns Value Returns: PathData: New PathData instance @@ -176,12 +139,12 @@ def from_dict(cls, data: Dict[str, Any], create_value_fn) -> "PathData": logger.error("Missing required keys in data: %s", missing) raise ValueError(f"Missing required keys: {missing}") - instance = cls(data["path"], data["metadata"]) + path_data = cls(path=data["path"], metadata=data["metadata"]) - # Reconstruct values - for env, value_data in data["values"].items(): + # 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) - instance.set_value(env, value) + path_data.set_value(env, value) - return instance + return path_data diff --git a/helm_values_manager/models/value.py b/helm_values_manager/models/value.py index 0aa5206..5d0062e 100644 --- a/helm_values_manager/models/value.py +++ b/helm_values_manager/models/value.py @@ -8,8 +8,8 @@ from dataclasses import dataclass from typing import Any, Dict -from ..backends.base import ValueBackend -from ..utils.logger import logger +from helm_values_manager.backends.base import ValueBackend +from helm_values_manager.utils.logger import logger @dataclass @@ -31,12 +31,16 @@ def __post_init__(self): """Post-initialization validation and logging.""" logger.debug("Created Value instance for path %s in environment %s", self.path, self.environment) - def get(self) -> str: + def get(self, resolve: bool = False) -> str: """ - Get the resolved value. + Get the value. + + 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. Returns: - str: The resolved value + str: The value (resolved or raw depending on resolve parameter) Raises: ValueError: If value doesn't exist @@ -44,7 +48,7 @@ def get(self) -> str: """ logger.debug("Getting value for path %s in environment %s", self.path, self.environment) try: - value = self._backend.get_value(self.path, self.environment) + value = self._backend.get_value(self.path, self.environment, resolve) logger.debug("Successfully retrieved value for path %s", self.path) return value except Exception as e: @@ -56,18 +60,16 @@ def set(self, value: str) -> None: Set the value using the backend. Args: - value: The value to store + value: The value to store, can be a raw value or a secret reference Raises: ValueError: If value is not a string RuntimeError: If backend operation fails """ - logger.debug("Setting value for path %s in environment %s", self.path, self.environment) - if not isinstance(value, str): - logger.error("Invalid value type for path %s: expected str, got %s", self.path, type(value)) raise ValueError("Value must be a string") + 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) diff --git a/helm_values_manager/schemas/v1.json b/helm_values_manager/schemas/v1.json index 2f7665e..d5876e3 100644 --- a/helm_values_manager/schemas/v1.json +++ b/helm_values_manager/schemas/v1.json @@ -107,7 +107,7 @@ }, "sensitive": { "type": "boolean", - "description": "Whether this value contains sensitive data" + "description": "Whether this value contains sensitive data. When true, the corresponding value should be a secret reference in the format: secret:///" }, "values": { "type": "object", diff --git a/tests/unit/models/test_metadata.py b/tests/unit/models/test_metadata.py new file mode 100644 index 0000000..35a96bf --- /dev/null +++ b/tests/unit/models/test_metadata.py @@ -0,0 +1,38 @@ +"""Tests for the config_metadata module.""" + +from helm_values_manager.models.config_metadata import ConfigMetadata + + +def test_from_dict(): + """Test creating ConfigMetadata from dictionary.""" + data = { + "description": "Test description", + "required": True, + "sensitive": True, + } + metadata = ConfigMetadata.from_dict(data) + assert metadata.description == "Test description" + assert metadata.required is True + assert metadata.sensitive is True + + +def test_from_dict_defaults(): + """Test default values when creating from dictionary.""" + data = {"description": "Test description"} + metadata = ConfigMetadata.from_dict(data) + assert metadata.description == "Test description" + assert metadata.required is False + assert metadata.sensitive is False + + +def test_to_dict(): + """Test converting ConfigMetadata to dictionary.""" + metadata = ConfigMetadata( + description="Test description", + required=True, + sensitive=True, + ) + data = metadata.to_dict() + assert data["description"] == "Test description" + assert data["required"] is True + assert data["sensitive"] is True diff --git a/tests/unit/models/test_path_data.py b/tests/unit/models/test_path_data.py index b3f2d13..635603f 100644 --- a/tests/unit/models/test_path_data.py +++ b/tests/unit/models/test_path_data.py @@ -1,4 +1,4 @@ -"""Tests for the PathData class.""" +"""Tests for the path_data module.""" from unittest.mock import Mock @@ -9,153 +9,179 @@ @pytest.fixture -def mock_value(): - """Create a mock Value instance.""" - mock_backend = Mock() - mock_backend.get_value.return_value = "test_value" - return Value(path="test.path", environment="test", _backend=mock_backend) +def path_data(): + """Create a PathData instance for testing.""" + return PathData( + "test.path", + { + "description": "Test description", + "required": True, + "sensitive": False, + }, + ) @pytest.fixture -def valid_metadata(): - """Create valid metadata for testing.""" - return {"description": "Test description", "required": True, "sensitive": False} +def mock_value(): + """Create a mock Value instance.""" + value = Mock(spec=Value) + value.path = "test.path" + return value -@pytest.fixture -def path_data(valid_metadata): - """Create a PathData instance with valid metadata.""" - return PathData("test.path", valid_metadata) +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 -def test_init_with_valid_metadata(valid_metadata): - """Test PathData initialization with valid metadata.""" - path_data = PathData("test.path", valid_metadata) - assert path_data.path == "test.path" - assert path_data.metadata == valid_metadata - assert list(path_data.get_environments()) == [] +def test_set_value(path_data, mock_value): + """Test setting a value.""" + path_data.set_value("test", mock_value) + assert path_data._values["test"] == mock_value -def test_init_with_none_description(): - """Test PathData initialization with None description.""" - metadata = {"description": None, "required": True, "sensitive": False} - path_data = PathData("test.path", metadata) - assert path_data.metadata["description"] is None +def test_get_value_nonexistent_environment(path_data): + """Test getting a value for a non-existent environment.""" + assert path_data.get_value("test") is None -def test_validate_missing_metadata(): - """Test validate() with missing metadata fields.""" - metadata = {"description": "Test"} # Missing required and sensitive - path_data = PathData("test.path", metadata) - with pytest.raises(ValueError, match="Missing required metadata fields"): - path_data.validate() +def test_get_value(path_data, mock_value): + """Test getting a value.""" + path_data.set_value("test", mock_value) + assert path_data.get_value("test") == mock_value -def test_validate_invalid_metadata_types(): - """Test validate() with invalid metadata types.""" - test_cases = [ - { - "metadata": {"description": 123, "required": True, "sensitive": False}, - "match": "Description must be a string or None", - }, - { - "metadata": {"description": "Test", "required": "True", "sensitive": False}, - "match": "Required flag must be a boolean", - }, - { - "metadata": {"description": "Test", "required": True, "sensitive": "False"}, - "match": "Sensitive flag must be a boolean", - }, - ] +def test_get_environments(path_data, mock_value): + """Test getting environment names.""" + path_data.set_value("test1", mock_value) + path_data.set_value("test2", mock_value) + environments = list(path_data.get_environments()) + assert len(environments) == 2 + assert "test1" in environments + assert "test2" in environments - for case in test_cases: - path_data = PathData("test.path", case["metadata"]) - with pytest.raises(ValueError, match=case["match"]): - path_data.validate() +def test_validate_path_mismatch(path_data): + """Test validation with mismatched paths.""" + 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) -def test_validate_path_consistency(path_data): - """Test validate() with inconsistent paths.""" - mock_backend = Mock() - inconsistent_value = Value(path="different.path", environment="test", _backend=mock_backend) - # Access private attribute for testing - path_data._values["test"] = inconsistent_value - with pytest.raises(ValueError, match="Value for environment test has inconsistent path"): - path_data.validate() +def test_validate_required_missing_value(path_data): + """Test validation with missing required value.""" + mock_value = Mock(spec=Value) + mock_value.path = "test.path" + mock_value.get.return_value = None + path_data.set_value("test_env", mock_value) -def test_set_value(path_data, mock_value): - """Test setting a Value object.""" - # Test adding a new value - path_data.set_value("test", mock_value) - assert path_data.get_value("test") == mock_value - assert list(path_data.get_environments()) == ["test"] + with pytest.raises(ValueError, match=r"Missing required value for path test\.path in environment test_env"): + path_data.validate() - # Test updating an existing value - new_mock_value = Value(path="test.path", environment="test", _backend=Mock()) - path_data.set_value("test", new_mock_value) - assert path_data.get_value("test") == new_mock_value +def test_validate_required_empty_value(path_data): + """Test validation with empty required value.""" + mock_value = Mock(spec=Value) + mock_value.path = "test.path" + mock_value.get.return_value = "" + path_data.set_value("test_env", mock_value) -def test_set_value_with_wrong_path(path_data): - """Test setting a Value object with wrong path.""" - mock_backend = Mock() - wrong_path_value = Value(path="wrong.path", environment="test", _backend=mock_backend) - with pytest.raises(ValueError, match="Value path wrong.path doesn't match PathData path test.path"): - path_data.set_value("test", wrong_path_value) + with pytest.raises(ValueError, match=r"Missing required value for path test\.path in environment test_env"): + path_data.validate() -def test_get_value(path_data, mock_value): - """Test getting a Value object.""" - path_data.set_value("test", mock_value) - assert path_data.get_value("test") == mock_value - assert path_data.get_value("nonexistent") is None +def test_validate_success(path_data, mock_value): + """Test successful validation.""" + mock_value.get.return_value = "test_value" + path_data.set_value("test_env", mock_value) + path_data.validate() # Should not raise any error -def test_get_environments(path_data, mock_value): - """Test getting environment names.""" - path_data.set_value("test1", mock_value) - path_data.set_value("test2", mock_value) - assert set(path_data.get_environments()) == {"test1", "test2"} +def test_validate_not_required(path_data, mock_value): + """Test validation when path is not required.""" + 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 def test_to_dict(path_data, mock_value): """Test converting PathData to dictionary.""" - path_data.set_value("test", mock_value) - result = path_data.to_dict() + mock_value.to_dict.return_value = {"value": "test_value"} + path_data.set_value("test_env", mock_value) + result = path_data.to_dict() assert result["path"] == "test.path" - assert result["metadata"] == path_data.metadata - assert "test" in result["values"] - assert result["values"]["test"] == mock_value.to_dict() + assert result["metadata"] == { + "description": "Test description", + "required": True, + "sensitive": False, + } + assert result["values"] == {"test_env": {"value": "test_value"}} -def test_from_dict_valid_data(valid_metadata, mock_value): - """Test creating PathData from valid dictionary data.""" +def test_from_dict(): + """Test creating PathData from dictionary.""" data = { "path": "test.path", - "metadata": valid_metadata, - "values": {"test": {"path": "test.path", "environment": "test", "backend_type": "mock"}}, + "metadata": { + "description": "Test description", + "required": True, + "sensitive": False, + }, + "values": {"test_env": {"value": "test_value"}}, } - def create_value_fn(path, env, data): + def create_value_fn(path, env, value_data): + mock_value = Mock(spec=Value) + mock_value.path = path + mock_value.environment = env return mock_value path_data = PathData.from_dict(data, create_value_fn) assert path_data.path == "test.path" - assert path_data.metadata == valid_metadata - assert path_data.get_value("test") == mock_value + assert path_data.metadata == { + "description": "Test description", + "required": True, + "sensitive": False, + } + assert len(list(path_data.get_environments())) == 1 + assert "test_env" in path_data._values + + +def test_from_dict_invalid_type(): + """Test from_dict with invalid data type.""" + with pytest.raises(ValueError, match="Data must be a dictionary"): + PathData.from_dict(["not a dict"], lambda p, e, d: None) + + +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) + # 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 test_from_dict_invalid_data(): - """Test from_dict() with invalid data.""" - test_cases = [ - ("not_a_dict", "Data must be a dictionary"), - ({"metadata": {}, "values": {}}, "Missing required keys"), # Missing path - ({"path": "test.path", "values": {}}, "Missing required keys"), # Missing metadata - ({"path": "test.path", "metadata": {}}, "Missing required keys"), # Missing values - ] - for data, error_match in test_cases: - with pytest.raises(ValueError, match=error_match): - PathData.from_dict(data, lambda p, e, d: None) +def test_from_dict_value_path_mismatch(): + """Test from_dict when create_value_fn returns value with wrong path.""" + data = { + "path": "test.path", + "metadata": {"required": True}, + "values": {"test_env": {"value": "test"}}, + } + + def create_value_fn(path, env, value_data): + mock_value = Mock(spec=Value) + 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"): + PathData.from_dict(data, create_value_fn) diff --git a/tests/unit/models/test_value.py b/tests/unit/models/test_value.py index 701010b..33676fb 100644 --- a/tests/unit/models/test_value.py +++ b/tests/unit/models/test_value.py @@ -12,7 +12,11 @@ def mock_backend(): """Create a mock backend for testing.""" backend = Mock(spec=ValueBackend) - backend.get_value.return_value = "mock_value" + # Return different values based on resolve parameter + backend.get_value.side_effect = lambda path, env, resolve: ( + "secret://gcp-secrets/my-app/dev/value" if not resolve else "resolved_value" + ) + backend.backend_type = "mock" return backend @@ -24,11 +28,18 @@ def test_value_init(mock_backend): assert value._backend == mock_backend -def test_get_value(mock_backend): - """Test getting a value.""" +def test_get_value_without_resolve(mock_backend): + """Test getting a value without resolving.""" value = Value(path="app.replicas", environment="dev", _backend=mock_backend) - assert value.get() == "mock_value" - mock_backend.get_value.assert_called_once_with("app.replicas", "dev") + assert value.get(resolve=False) == "secret://gcp-secrets/my-app/dev/value" + mock_backend.get_value.assert_called_once_with("app.replicas", "dev", False) + + +def test_get_value_with_resolve(mock_backend): + """Test getting a value with resolving.""" + value = Value(path="app.replicas", environment="dev", _backend=mock_backend) + assert value.get(resolve=True) == "resolved_value" + mock_backend.get_value.assert_called_once_with("app.replicas", "dev", True) def test_set_value(mock_backend): @@ -47,7 +58,6 @@ def test_set_invalid_type(mock_backend): def test_to_dict(mock_backend): """Test serializing a value.""" - mock_backend.backend_type = "mock" value = Value(path="app.replicas", environment="dev", _backend=mock_backend) data = value.to_dict() assert data == {"path": "app.replicas", "environment": "dev", "backend_type": "mock"}