Merge pull request #12 from Zipstack/4-metadata-refactor

refactor: update metadata handling and improve code organization

Author: ritwik-g

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.

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://<backend-type>/<secret-path>`
+
+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

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

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"
+            }
         }
     ]
 }

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

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"]

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
 

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

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"]

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),
+        )