diff --git a/docs/ADRs/009-custom-configuration-file-paths.md b/docs/ADRs/009-custom-configuration-file-paths.md new file mode 100644 index 0000000..e40c202 --- /dev/null +++ b/docs/ADRs/009-custom-configuration-file-paths.md @@ -0,0 +1,119 @@ +# 9. Custom Configuration File Paths + +Date: 2025-02-27 + +## Status + +Proposed + +## Context + +Currently, the Helm Values Manager uses hardcoded file paths for its configuration and lock files: +- Configuration file: `helm-values.json` +- Lock file: `.helm-values.lock` + +These paths are defined in the `BaseCommand` class and used across all commands. This approach has several limitations: + +1. **Limited Flexibility**: Users cannot manage multiple configurations in the same directory +2. **Naming Constraints**: Organizations may have specific naming conventions that conflict with our hardcoded names +3. **Integration Challenges**: Difficult to integrate with existing systems that might have their own file organization +4. **Location Restrictions**: Configuration files must be in the current working directory + +Users have expressed interest in being able to specify custom file paths for their configuration files, which would address these limitations and provide greater flexibility. + +## Decision + +We will enhance the Helm Values Manager to support custom configuration file paths by: + +1. **Adding a `--config-file` option to all commands**: + ```python + config_file: str = typer.Option( + "helm-values.json", + "--config-file", + "-c", + help="Path to the configuration file" + ) + ``` + +2. **Updating the `BaseCommand` class to accept a config file path**: + ```python + def __init__(self, config_file: str = "helm-values.json") -> None: + """Initialize the base command. + + Args: + config_file: Path to the configuration file + """ + self.config_file = config_file + self.lock_file = self._generate_lock_file_path(config_file) + self._lock_fd: Optional[int] = None + ``` + +3. **Generating the lock file name based on the config file path**: + ```python + def _generate_lock_file_path(self, config_file: str) -> str: + """Generate the lock file path based on the config file path. + + Args: + config_file: Path to the configuration file + + Returns: + str: Path to the lock file + """ + # Expand user home directory if present (e.g., ~/configs) + expanded_path = os.path.expanduser(config_file) + config_path = Path(expanded_path) + + # Check if the path is a directory + if config_path.is_dir(): + # Use default filename in the specified directory + config_path = config_path / "helm-values.json" + + # Create lock file name based on the config file stem (name without extension) + lock_file = f".{config_path.stem}.lock" + return str(config_path.parent / lock_file) + ``` + +4. **Propagating the config file path to all command instances**: + ```python + @app.command() + def init( + release_name: str = typer.Option(..., "--release", "-r", help="Name of the Helm release"), + config_file: str = typer.Option("helm-values.json", "--config-file", "-c", help="Path to the configuration file"), + ): + """Initialize a new helm-values configuration.""" + command = InitCommand(config_file=config_file) + result = command.execute(release_name=release_name) + echo(result) + ``` + +## Consequences + +### Positive + +1. **Increased Flexibility**: Users can manage multiple configurations in the same directory +2. **Custom Naming**: Organizations can follow their own naming conventions +3. **Better Integration**: Easier to integrate with existing systems and workflows +4. **Location Freedom**: Configuration files can be placed in any directory + +### Negative + +1. **API Change**: All command functions need to be updated to accept the new parameter +2. **Complexity**: Slightly more complex code to handle different file paths +3. **Testing**: Additional test cases needed to verify the functionality +4. **Documentation**: Documentation needs to be updated to reflect the new option + +### Neutral + +1. **Backward Compatibility**: Default values ensure backward compatibility with existing usage +2. **Lock File Naming**: Lock files will be named based on the configuration file name + +## Implementation Notes + +1. The implementation should be backward compatible, using the current hardcoded paths as defaults +2. All commands should propagate the config file path to their respective command instances +3. Documentation should be updated to reflect the new option +4. Tests should be added to verify the functionality with custom file paths + +## Related Issues + +- GitHub Issue [#17](https://github.com/Zipstack/helm-values-manager/issues/17): Support for custom configuration file paths diff --git a/docs/ADRs/010-configuration-update-and-merge.md b/docs/ADRs/010-configuration-update-and-merge.md new file mode 100644 index 0000000..5a6c1e0 --- /dev/null +++ b/docs/ADRs/010-configuration-update-and-merge.md @@ -0,0 +1,86 @@ +# ADR-010: Configuration Update and Merge + +Date: 2025-02-27 + +## Status + +Proposed + +## Context + +Chart owners may update their configuration schema over time, adding new configuration paths, modifying metadata, or removing obsolete paths. Users who have already configured their deployments need a way to incorporate these updates without losing their existing configurations. + +Currently, the Helm Values Manager does not provide a mechanism to update an existing configuration with changes from a new version. This leads to several challenges: + +1. Users must manually identify and apply changes between configuration versions +2. There's a risk of missing new required configuration paths +3. Users may lose their existing values when updating to a new configuration +4. Conflicts between user customizations and chart owner updates are difficult to resolve + +A structured approach to configuration updates would improve the user experience and reduce the risk of configuration errors. + +## Decision + +We will implement a configuration update and merge feature that allows users to incorporate changes from a new configuration file while preserving their existing values and deployments. + +The feature will: + +1. Compare the current configuration with the new one to identify: + - Added configuration paths + - Removed configuration paths + - Modified metadata (description, required, sensitive flags) + - Potential conflicts + +2. Provide multiple merge strategies: + - "Smart" (default): Preserve user values while adopting new metadata and paths + - "Theirs": Prefer the new configuration but keep existing values where possible + - "Ours": Prefer the existing configuration but add new paths + +3. Validate the merged configuration to ensure it meets all requirements + - Identify missing required values + - Ensure all paths have valid values for their environments + +4. Provide clear reporting of changes and required actions + +This will be implemented as a new `update-config` command in the CLI. + +## Consequences + +### Positive + +- Users can easily update their configurations when chart owners release updates +- Existing user values are preserved during updates +- New required configurations are clearly identified +- The risk of missing important configuration changes is reduced +- The update process becomes more transparent and less error-prone + +### Negative + +- Adds complexity to the codebase +- May require additional schema versioning to handle major changes +- Conflict resolution might still require manual intervention in complex cases + +### Neutral + +- Users will need to learn a new command and its options +- May need to adjust the configuration schema to better support versioning and updates + +## Implementation Notes + +The implementation will require: + +1. A new `UpdateConfigCommand` class that extends `BaseCommand` +2. Configuration comparison logic to identify differences +3. Merge strategies to combine configurations +4. Validation to ensure the merged configuration is valid +5. Reporting to communicate changes and required actions + +The command will be exposed through the CLI as: + +``` +helm values update-config [source-file] [--strategy=smart|theirs|ours] [--report-only] +``` + +## Related Issues + +- GitHub Issue [#19](https://github.com/Zipstack/helm-values-manager/issues/19): Configuration Update and Merge Feature diff --git a/docs/ADRs/README.md b/docs/ADRs/README.md index f58da4b..11f3fd5 100644 --- a/docs/ADRs/README.md +++ b/docs/ADRs/README.md @@ -5,6 +5,10 @@ This directory contains Architecture Decision Records (ADRs) for the Helm Values ## What is an ADR? An Architecture Decision Record (ADR) is a document that captures an important architectural decision made along with its context and consequences. +## Creating New ADRs + +For new ADRs, use the [ADR template](adr-template.md) as a starting point. + ## ADR Index ### [ADR-001: Helm Values Manager as a Helm Plugin](001-helm-values-manager.md) @@ -61,20 +65,16 @@ An Architecture Decision Record (ADR) is a document that captures an important a - **Decision**: Remove registry in favor of direct command instantiation - **Impact**: Simplifies code and aligns better with Typer's design -## ADR Template -For new ADRs, use this template: -```markdown -# ADR-NNN: Title - -## Status -[Proposed | Accepted | Deprecated | Superseded] - -## Context -What is the issue that we're seeing that is motivating this decision or change? - -## Decision -What is the change that we're proposing and/or doing? +### [ADR-009: Custom Configuration File Paths](009-custom-configuration-file-paths.md) +- **Status**: Proposed +- **Context**: Limited flexibility with hardcoded configuration file paths +- **Decision**: Add support for custom configuration file paths +- **Impact**: Increases flexibility and improves integration capabilities +- **Dependencies**: ADR-001 -## Consequences -What becomes easier or more difficult to do because of this change? -``` +### [ADR-010: Configuration Update and Merge](010-configuration-update-and-merge.md) +- **Status**: Proposed +- **Context**: Need to incorporate chart owner configuration updates without losing user customizations +- **Decision**: Implement configuration comparison and smart merging with multiple strategies +- **Impact**: Simplifies configuration updates and reduces risk of missing required changes +- **Dependencies**: ADR-001 diff --git a/docs/ADRs/adr-template.md b/docs/ADRs/adr-template.md new file mode 100644 index 0000000..a897f95 --- /dev/null +++ b/docs/ADRs/adr-template.md @@ -0,0 +1,37 @@ +# ADR-NNN: Title + +Date: YYYY-MM-DD + +## Status + +[Proposed | Accepted | Deprecated | Superseded] + +## Context + +What is the issue that we're seeing that is motivating this decision or change? + +## Decision + +What is the change that we're proposing and/or doing? + +## Consequences + +### Positive + +What becomes easier because of this change? + +### Negative + +What becomes more difficult because of this change? + +### Neutral + +What other effects does this change have? + +## Implementation Notes + +Additional notes on implementation details, if applicable. + +## Related Issues + +- GitHub Issue #[TBD]: Related issue title diff --git a/docs/Development/tasks.md b/docs/Development/tasks.md index 43b84de..c5b5cff 100644 --- a/docs/Development/tasks.md +++ b/docs/Development/tasks.md @@ -89,10 +89,10 @@ - [x] Add empty config initialization - [x] Add config file creation - [x] Add schema template generation - - [ ] Implement add-value-config command - - [ ] Add basic path validation - - [ ] Add metadata validation - - [ ] Add config update + - [x] Implement add-value-config command + - [x] Add basic path validation + - [x] Add metadata validation + - [x] Add config update - [ ] Implement add-deployment command - [ ] Add basic deployment validation - [ ] Add backend validation diff --git a/helm_values_manager/cli.py b/helm_values_manager/cli.py index 7116636..b0da144 100644 --- a/helm_values_manager/cli.py +++ b/helm_values_manager/cli.py @@ -1,7 +1,10 @@ """Command line interface for the helm-values-manager plugin.""" +from typing import Optional + import typer +from helm_values_manager.commands.add_value_config_command import AddValueConfigCommand from helm_values_manager.commands.init_command import InitCommand from helm_values_manager.utils.logger import HelmLogger @@ -42,5 +45,36 @@ def init( raise typer.Exit(code=1) from e +@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" + ), + required: bool = typer.Option(False, "--required", "-r", help="Whether this configuration is required"), + sensitive: bool = typer.Option( + False, + "--sensitive", + "-s", + help="Whether this configuration contains sensitive data (coming in v0.2.0)", + hidden=True, # Hide from help text until v0.2.0 + ), +): + """Add a new value configuration with metadata.""" + try: + command = AddValueConfigCommand() + + # Add warning for sensitive flag until it's fully implemented + if sensitive: + HelmLogger.warning("Sensitive value support will be available in version 0.2.0. Flag will be ignored.") + sensitive = False # Ignore the flag for now + + result = command.execute(path=path, description=description, required=required, sensitive=sensitive) + typer.echo(result) + except Exception as e: + HelmLogger.error("Failed to add value config: %s", str(e)) + raise typer.Exit(code=1) from e + + if __name__ == "__main__": app(prog_name=COMMAND_INFO) diff --git a/helm_values_manager/commands/add_value_config_command.py b/helm_values_manager/commands/add_value_config_command.py new file mode 100644 index 0000000..7a8a928 --- /dev/null +++ b/helm_values_manager/commands/add_value_config_command.py @@ -0,0 +1,53 @@ +"""Command to add a new value configuration with metadata.""" + +from typing import Optional + +from helm_values_manager.commands.base_command import BaseCommand +from helm_values_manager.models.helm_values_config import HelmValuesConfig +from helm_values_manager.utils.logger import HelmLogger + + +class AddValueConfigCommand(BaseCommand): + """Command to add a new value configuration with metadata.""" + + def run(self, config: Optional[HelmValuesConfig] = None, **kwargs) -> str: + """ + Add a new value configuration with metadata. + + Args: + config: The loaded configuration + **kwargs: Command arguments + - path (str): The configuration path (e.g., 'app.replicas') + - description (str, optional): Description of what this configuration does + - required (bool, optional): Whether this configuration is required + - sensitive (bool, optional): Whether this configuration contains sensitive data + + Returns: + str: Success message + + Raises: + ValueError: If path is invalid or already exists + """ + if config is None: + raise ValueError("Configuration not loaded") + + path = kwargs.get("path") + if not path: + raise ValueError("Path cannot be empty") + + description = kwargs.get("description") + required = kwargs.get("required", False) + sensitive = kwargs.get("sensitive", False) + + try: + # Add the new configuration path + config.add_config_path(path=path, description=description, required=required, sensitive=sensitive) + + # Save the updated configuration + self.save_config(config) + + HelmLogger.debug("Added value config '%s' (required=%s, sensitive=%s)", path, required, sensitive) + return f"Successfully added value config '{path}'" + except ValueError as e: + HelmLogger.error("Failed to add value config: %s", str(e)) + raise diff --git a/tests/integration/test_cli_integration.py b/tests/integration/test_cli_integration.py index e3acbb4..a4b2c41 100644 --- a/tests/integration/test_cli_integration.py +++ b/tests/integration/test_cli_integration.py @@ -1,5 +1,6 @@ """Integration tests for the helm-values-manager CLI.""" +import json import os import subprocess from pathlib import Path @@ -85,3 +86,109 @@ def test_init_command(plugin_install, tmp_path): assert "Successfully initialized helm-values configuration" in stdout assert Path("helm-values.json").exists() assert Path(".helm-values.lock").exists() + + +def test_add_value_config_help_command(plugin_install): + """Test that the add-value-config help command works and shows expected output.""" + stdout, stderr, returncode = run_helm_command(["values-manager", "add-value-config", "--help"]) + + assert returncode == 0 + assert "Add a new value configuration with metadata" in stdout + assert "--path" in stdout + assert "--description" in stdout + assert "--required" in stdout + # The sensitive flag is now hidden, so it shouldn't appear in help + # assert "--sensitive" in stdout + + +def test_add_value_config_command(plugin_install, tmp_path): + """Test that the add-value-config command works correctly.""" + # Change to temp directory to avoid conflicts + os.chdir(tmp_path) + + # First initialize a configuration + init_stdout, init_stderr, init_returncode = run_helm_command(["values-manager", "init", "-r", "test-app"]) + assert init_returncode == 0 + + # Add a value configuration + path = "app.replicas" + description = "Number of application replicas" + stdout, stderr, returncode = run_helm_command( + ["values-manager", "add-value-config", "--path", path, "--description", description, "--required"] + ) + + assert returncode == 0 + assert f"Successfully added value config '{path}'" in stdout + + # Verify the configuration file was updated correctly + with open("helm-values.json", "r") as f: + config = json.load(f) + + # Check that the config contains our new value + assert "config" in config + assert len(config["config"]) == 1 + assert config["config"][0]["path"] == path + assert config["config"][0]["description"] == description + assert config["config"][0]["required"] is True + assert config["config"][0]["sensitive"] is False + assert config["config"][0]["values"] == {} + + # Add another value configuration + second_path = "app.image.tag" + second_description = "Application image tag" + stdout, stderr, returncode = run_helm_command( + [ + "values-manager", + "add-value-config", + "--path", + second_path, + "--description", + second_description, + ] + ) + + assert returncode == 0 + assert f"Successfully added value config '{second_path}'" in stdout + + # Verify the configuration file was updated correctly + with open("helm-values.json", "r") as f: + config = json.load(f) + + # Check that the config contains both values + assert "config" in config + assert len(config["config"]) == 2 + + # Find the second added config + second_config = next((c for c in config["config"] if c["path"] == second_path), None) + assert second_config is not None + assert second_config["description"] == second_description + assert second_config["required"] is False + assert second_config["sensitive"] is False + assert second_config["values"] == {} + + +def test_add_value_config_duplicate_path(plugin_install, tmp_path): + """Test that adding a duplicate path fails with the correct error message.""" + # Change to temp directory to avoid conflicts + os.chdir(tmp_path) + + # First initialize a configuration + init_stdout, init_stderr, init_returncode = run_helm_command(["values-manager", "init", "-r", "test-app"]) + assert init_returncode == 0 + + # Add a value configuration + path = "app.replicas" + description = "Number of application replicas" + stdout, stderr, returncode = run_helm_command( + ["values-manager", "add-value-config", "--path", path, "--description", description] + ) + + assert returncode == 0 + + # Try to add the same path again + stdout, stderr, returncode = run_helm_command( + ["values-manager", "add-value-config", "--path", path, "--description", "Another description"] + ) + + assert returncode != 0 + assert f"Path {path} already exists" in stderr diff --git a/tests/unit/commands/test_add_value_config_command.py b/tests/unit/commands/test_add_value_config_command.py new file mode 100644 index 0000000..2b23cca --- /dev/null +++ b/tests/unit/commands/test_add_value_config_command.py @@ -0,0 +1,89 @@ +"""Tests for the add-value-config command.""" + +import json +from unittest.mock import mock_open, patch + +import pytest + +from helm_values_manager.commands.add_value_config_command import AddValueConfigCommand +from helm_values_manager.models.helm_values_config import HelmValuesConfig + + +@pytest.fixture +def mock_config_file(): + """Create a mock configuration file.""" + config = HelmValuesConfig() + config.version = "1.0" + config.release = "test-release" + return json.dumps(config.to_dict()) + + +@pytest.fixture +def command(): + """Create an instance of the AddValueConfigCommand.""" + return AddValueConfigCommand() + + +def test_add_value_config_success(command, mock_config_file): + """Test successful addition of a value configuration.""" + path = "app.replicas" + description = "Number of application replicas" + required = True + sensitive = False + + # Mock file operations + with ( + patch("builtins.open", mock_open(read_data=mock_config_file)), + patch("os.path.exists", return_value=True), + patch("fcntl.flock"), + patch("os.open"), + patch("os.close"), + ): + + result = command.execute(path=path, description=description, required=required, sensitive=sensitive) + + assert "Successfully added value config 'app.replicas'" in result + + +def test_add_value_config_empty_path(command, mock_config_file): + """Test adding a value configuration with an empty path.""" + # Mock file operations + with ( + patch("builtins.open", mock_open(read_data=mock_config_file)), + patch("os.path.exists", return_value=True), + patch("fcntl.flock"), + patch("os.open"), + patch("os.close"), + ): + + with pytest.raises(ValueError, match="Path cannot be empty"): + command.execute(path="") + + +def test_add_value_config_duplicate_path(command, mock_config_file): + """Test adding a duplicate value configuration path.""" + path = "app.replicas" + + # Create a config with the path already added + config = HelmValuesConfig.from_dict(json.loads(mock_config_file)) + config.add_config_path(path, description="Existing config") + updated_mock_config = json.dumps(config.to_dict()) + + # Mock file operations + with ( + patch("builtins.open", mock_open(read_data=updated_mock_config)), + patch("os.path.exists", return_value=True), + patch("fcntl.flock"), + patch("os.open"), + patch("os.close"), + ): + + with pytest.raises(ValueError, match=f"Path {path} already exists"): + command.execute(path=path, description="New description") + + +def test_add_value_config_none_config(command): + """Test adding a value configuration when config is None.""" + # Test the case where config is None + with pytest.raises(ValueError, match="Configuration not loaded"): + command.run(config=None, path="app.replicas", description="Test description") diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index 9247a9f..72876d7 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -65,3 +65,77 @@ def test_init_command_already_initialized(tmp_path): assert result.exit_code == 1 assert "Failed to initialize: Configuration file" in result.stdout assert "already exists" in result.stdout + + +def test_add_value_config_command(tmp_path): + """Test add-value-config command.""" + # Change to temp directory + os.chdir(tmp_path) + + # First initialize a configuration + init_result = runner.invoke(app, ["init", "--release", "test-release"], catch_exceptions=False) + assert init_result.exit_code == 0 + + # Add a value configuration + path = "app.replicas" + description = "Number of application replicas" + result = runner.invoke( + app, ["add-value-config", "--path", path, "--description", description, "--required"], catch_exceptions=False + ) + + assert result.exit_code == 0 + assert f"Successfully added value config '{path}'" in result.stdout + + # Verify the configuration file was updated correctly + config_file = Path("helm-values.json") + with config_file.open() as file: + config_data = json.load(file) + + assert "config" in config_data + assert len(config_data["config"]) == 1 + assert config_data["config"][0]["path"] == path + assert config_data["config"][0]["description"] == description + assert config_data["config"][0]["required"] is True + assert config_data["config"][0]["sensitive"] is False + assert config_data["config"][0]["values"] == {} + + +def test_add_value_config_duplicate_path(tmp_path): + """Test add-value-config command with duplicate path.""" + # Change to temp directory + os.chdir(tmp_path) + + # First initialize a configuration + init_result = runner.invoke(app, ["init", "--release", "test-release"], catch_exceptions=False) + assert init_result.exit_code == 0 + + # Add a value configuration + path = "app.replicas" + description = "Number of application replicas" + first_result = runner.invoke( + app, ["add-value-config", "--path", path, "--description", description], catch_exceptions=False + ) + + assert first_result.exit_code == 0 + + # Try to add the same path again + second_result = runner.invoke(app, ["add-value-config", "--path", path, "--description", "Another description"]) + + assert second_result.exit_code == 1 + assert f"Failed to add value config: Path {path} already exists" in second_result.stdout + + +def test_add_value_config_empty_path(tmp_path): + """Test add-value-config command with empty path.""" + # Change to temp directory + os.chdir(tmp_path) + + # First initialize a configuration + init_result = runner.invoke(app, ["init", "--release", "test-release"], catch_exceptions=False) + assert init_result.exit_code == 0 + + # Try to add a value configuration with empty path + result = runner.invoke(app, ["add-value-config", "--path", "", "--description", "Some description"]) + + assert result.exit_code == 1 + assert "Failed to add value config: Path cannot be empty" in result.stdout