diff --git a/docs/Development/standardize-terminology.md b/docs/Development/standardize-terminology.md new file mode 100644 index 0000000..aefc76c --- /dev/null +++ b/docs/Development/standardize-terminology.md @@ -0,0 +1,79 @@ +# Standardize Terminology: Environment to Deployment + +This document outlines the tasks required to standardize the terminology in the codebase, replacing all instances of "environment" with "deployment" for consistency. + +## Background + +Currently, the codebase uses "environment" in the internal API and "deployment" in the CLI interface and user-facing messages. This inconsistency can lead to confusion for developers and users. The goal is to standardize on "deployment" throughout the codebase. + +## Tasks + +### 1. Update Core Models + +- [ ] Update `HelmValuesConfig` class: + - [ ] Change method signatures to use "deployment" instead of "environment" + - [ ] Update docstrings and comments + - [ ] Ensure backward compatibility or provide migration path + +- [ ] Update `Value` class: + - [ ] Rename `environment` parameter to `deployment` in constructor and methods + - [ ] Update docstrings and comments + +### 2. Update Command Classes + +- [ ] Update `SetValueCommand` class: + - [ ] Rename internal variables from "environment" to "deployment" + - [ ] Ensure all docstrings and comments use "deployment" + +- [ ] Review and update other command classes that might use "environment" + +### 3. Update Backend Classes + +- [ ] Update `SimpleValueBackend` class: + - [ ] Rename method parameters from "environment" to "deployment" + - [ ] Update internal storage keys if necessary + +- [ ] Update other backend implementations if present + +### 4. Update Tests + +- [ ] Update unit tests: + - [ ] Rename test variables from "environment" to "deployment" + - [ ] Update mock objects and assertions + +- [ ] Update integration tests: + - [ ] Ensure all tests use "deployment" consistently + +### 5. Update Documentation + +- [ ] Update design documentation: + - [ ] Review and update low-level-design.md + - [ ] Review and update sequence-diagrams.md + +- [ ] Update user documentation: + - [ ] Ensure all examples and explanations use "deployment" + +### 6. Create Migration Plan + +- [ ] Assess impact on existing configurations: + - [ ] Determine if existing configurations need to be migrated + - [ ] Create migration script if necessary + +## Implementation Strategy + +This change should be implemented as a single, focused PR to ensure consistency across the codebase. The PR should: + +1. Not include any functional changes beyond the terminology standardization +2. Include comprehensive tests to ensure no functionality is broken +3. Update all relevant documentation + +## Testing Strategy + +1. Run all existing tests to ensure they pass with the updated terminology +2. Add specific tests to verify that the terminology change doesn't affect functionality +3. Manually test key workflows to ensure they work as expected + +## Risks and Mitigation + +- **Breaking Changes**: This change may introduce breaking changes for users who have integrated with the internal API. Consider providing a deprecation period or backward compatibility layer. +- **Documentation**: Ensure all documentation is updated to reflect the new terminology to avoid confusion. diff --git a/docs/Development/tasks.md b/docs/Development/tasks.md index 5aaa0b8..886f30c 100644 --- a/docs/Development/tasks.md +++ b/docs/Development/tasks.md @@ -93,21 +93,21 @@ - [x] Add basic path validation - [x] Add metadata validation - [x] Add config update - - [ ] Implement add-deployment command - - [ ] Add basic deployment validation - - [ ] Add backend validation - - [ ] Add deployment registration + - [x] Implement add-deployment command + - [x] Add basic deployment validation + - [x] Add backend validation + - [x] Add deployment registration - [ ] Implement generate command - [ ] Add template generation - [ ] Add basic value substitution - [ ] Value Management Commands - - [ ] Implement get-value command - - [ ] Add basic path validation - - [ ] Add value retrieval - - [ ] Implement set-value command - - [ ] Add basic path validation - - [ ] Add value storage + - [x] Implement set-value command + - [x] Add basic path validation + - [x] Add value storage + - [ ] Implement generate command + - [ ] Add template generation + - [ ] Add basic value substitution #### Phase 2: Enhanced Safety & Management - [x] Enhanced Command Infrastructure @@ -123,6 +123,11 @@ - [ ] Add conflict detection - [ ] Add dependency validation +- [ ] Value Management Commands + - [ ] Implement get-value command + - [ ] Add basic path validation + - [ ] Add value retrieval + - [ ] Basic Validation System - [ ] Add PathValidator class - [ ] Add path format validation @@ -259,6 +264,16 @@ - [ ] Add missing tests - [ ] Improve existing tests +## Code Quality and Maintenance +- [ ] Standardize terminology (environment → deployment) + - [ ] Update core models (HelmValuesConfig, Value) + - [ ] Update command classes + - [ ] Update backend classes + - [ ] Update tests + - [ ] Update documentation +- [ ] Add more comprehensive error handling +- [ ] Improve test coverage + ## Development Guidelines 1. Follow TDD approach - Write tests first diff --git a/helm_values_manager/cli.py b/helm_values_manager/cli.py index 3388a00..46dfe97 100644 --- a/helm_values_manager/cli.py +++ b/helm_values_manager/cli.py @@ -7,6 +7,7 @@ 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.utils.logger import HelmLogger COMMAND_INFO = "helm values-manager" @@ -97,5 +98,31 @@ def add_deployment( raise typer.Exit(code=1) from e +@app.command("set-value") +def set_value( + path: str = typer.Option(..., "--path", "-p", help="Configuration path (e.g., 'app.replicas')"), + deployment: str = typer.Option( + ..., "--deployment", "-d", help="Deployment to set the value for (e.g., 'dev', 'prod')" + ), + value: str = typer.Option(..., "--value", "-v", help="Value to set"), +): + """Set a value for a specific path and deployment.""" + try: + command = SetValueCommand() + + # Create kwargs for command execution + kwargs = { + "path": path, + "environment": deployment, # Map deployment to environment for API compatibility + "value": value, + } + + result = command.execute(**kwargs) + typer.echo(result) + except Exception as e: + HelmLogger.error("Failed to set value: %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/set_value_command.py b/helm_values_manager/commands/set_value_command.py new file mode 100644 index 0000000..6cb3f3a --- /dev/null +++ b/helm_values_manager/commands/set_value_command.py @@ -0,0 +1,62 @@ +"""Command to set a value for a specific path and deployment.""" + +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 SetValueCommand(BaseCommand): + """Command to set a value for a specific path and deployment.""" + + def run(self, config: Optional[HelmValuesConfig] = None, **kwargs) -> str: + """ + Set a value for a specific path and deployment. + + Args: + config: The loaded configuration + **kwargs: Command arguments + - path (str): The configuration path (e.g., 'app.replicas') + - environment (str): The deployment to set the value for (e.g., 'dev', 'prod') + - value (str): The value to set + + Returns: + str: Success message + + Raises: + ValueError: If path or environment is empty + KeyError: If path or deployment doesn't exist in the configuration + """ + if config is None: + raise ValueError("Configuration not loaded") + + path = kwargs.get("path") + if not path: + raise ValueError("Path cannot be empty") + + environment = kwargs.get("environment") + if not environment: + raise ValueError("Deployment cannot be empty") + + value = kwargs.get("value") + + # Validate that the deployment exists + if environment not in config.deployments: + raise KeyError(f"Deployment '{environment}' not found") + + try: + # Set the value for the specified path and deployment + config.set_value(path=path, environment=environment, value=value) + + # Save the updated configuration + self.save_config(config) + + HelmLogger.debug("Set value for path '%s' in deployment '%s'", path, environment) + return f"Successfully set value for path '{path}' in deployment '{environment}'" + except KeyError as e: + HelmLogger.error("Failed to set value: %s", str(e)) + raise + except Exception as e: + HelmLogger.error("Failed to set value: %s", str(e)) + raise diff --git a/tests/integration/test_cli_integration.py b/tests/integration/test_cli_integration.py index 377cfd9..f7c46f5 100644 --- a/tests/integration/test_cli_integration.py +++ b/tests/integration/test_cli_integration.py @@ -261,3 +261,117 @@ def test_add_deployment_no_config(plugin_install, tmp_path): stdout, stderr, returncode = run_helm_command(["values-manager", "add-deployment", "dev"]) assert returncode == 1 assert "Configuration file helm-values.json not found" in stderr + + +def test_set_value_command(plugin_install, tmp_path): + """Test that the set-value command works correctly.""" + # Create a working directory + work_dir = tmp_path / "test_set_value" + work_dir.mkdir() + os.chdir(work_dir) + + # Initialize the config + init_stdout, init_stderr, init_returncode = run_helm_command( + ["values-manager", "init", "--release", "test-release"] + ) + assert init_returncode == 0 + assert Path("helm-values.json").exists() + + # Add a deployment + add_deployment_stdout, add_deployment_stderr, add_deployment_returncode = run_helm_command( + ["values-manager", "add-deployment", "dev"] + ) + assert add_deployment_returncode == 0 + + # Add a value config + add_value_config_stdout, add_value_config_stderr, add_value_config_returncode = run_helm_command( + [ + "values-manager", + "add-value-config", + "--path", + "app.replicas", + "--description", + "Number of replicas", + "--required", + ] + ) + assert add_value_config_returncode == 0 + + # Set a value + stdout, stderr, returncode = run_helm_command( + ["values-manager", "set-value", "--path", "app.replicas", "--deployment", "dev", "--value", "3"] + ) + assert returncode == 0 + assert "Successfully set value for path 'app.replicas' in deployment 'dev'" in stdout + + # Verify the value was set + with open("helm-values.json", "r") as f: + config = json.load(f) + + # The value itself is stored in the backend, not directly in the config file + # But we can verify that the path exists in the config + assert any(path["path"] == "app.replicas" for path in config["config"]) + + +def test_set_value_nonexistent_path(plugin_install, tmp_path): + """Test that setting a value for a nonexistent path fails with the correct error message.""" + # Create a working directory + work_dir = tmp_path / "test_set_value_nonexistent_path" + work_dir.mkdir() + os.chdir(work_dir) + + # Initialize the config + init_stdout, init_stderr, init_returncode = run_helm_command( + ["values-manager", "init", "--release", "test-release"] + ) + assert init_returncode == 0 + assert Path("helm-values.json").exists() + + # Add a deployment + add_deployment_stdout, add_deployment_stderr, add_deployment_returncode = run_helm_command( + ["values-manager", "add-deployment", "dev"] + ) + assert add_deployment_returncode == 0 + + # Try to set a value for a nonexistent path + stdout, stderr, returncode = run_helm_command( + ["values-manager", "set-value", "--path", "nonexistent.path", "--deployment", "dev", "--value", "3"] + ) + assert returncode == 1 + assert "Path nonexistent.path not found" in stderr + + +def test_set_value_nonexistent_deployment(plugin_install, tmp_path): + """Test that setting a value for a nonexistent deployment fails with the correct error message.""" + # Create a working directory + work_dir = tmp_path / "test_set_value_nonexistent_deployment" + work_dir.mkdir() + os.chdir(work_dir) + + # Initialize the config + init_stdout, init_stderr, init_returncode = run_helm_command( + ["values-manager", "init", "--release", "test-release"] + ) + assert init_returncode == 0 + assert Path("helm-values.json").exists() + + # Add a value config + add_value_config_stdout, add_value_config_stderr, add_value_config_returncode = run_helm_command( + [ + "values-manager", + "add-value-config", + "--path", + "app.replicas", + "--description", + "Number of replicas", + "--required", + ] + ) + assert add_value_config_returncode == 0 + + # Try to set a value for a nonexistent deployment + stdout, stderr, returncode = run_helm_command( + ["values-manager", "set-value", "--path", "app.replicas", "--deployment", "nonexistent", "--value", "3"] + ) + assert returncode == 1 + assert "Deployment 'nonexistent' not found" in stderr diff --git a/tests/unit/commands/test_set_value_command.py b/tests/unit/commands/test_set_value_command.py new file mode 100644 index 0000000..ee9ae8f --- /dev/null +++ b/tests/unit/commands/test_set_value_command.py @@ -0,0 +1,135 @@ +"""Tests for the set-value command.""" + +import json +from unittest.mock import mock_open, patch + +import pytest + +from helm_values_manager.commands.set_value_command import SetValueCommand +from helm_values_manager.models.constants import NO_AUTH, NO_BACKEND +from helm_values_manager.models.helm_values_config import Deployment, HelmValuesConfig + + +@pytest.fixture +def mock_config_file(): + """Create a mock configuration file with a path already added.""" + config = HelmValuesConfig() + config.version = "1.0" + config.release = "test-release" + config.add_config_path(path="app.replicas", description="Number of app replicas", required=True) + + # Add deployment directly to the deployments dictionary + deployment = Deployment( + name="dev", + backend=NO_BACKEND, + auth={"type": NO_AUTH}, + backend_config={}, + ) + config.deployments["dev"] = deployment + + return json.dumps(config.to_dict()) + + +@pytest.fixture +def command(): + """Create an instance of the SetValueCommand.""" + return SetValueCommand() + + +def test_set_value_success(command, mock_config_file): + """Test successful setting of a value.""" + path = "app.replicas" + deployment = "dev" + value = "3" + + # 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, environment=deployment, value=value) + + assert f"Successfully set value for path '{path}' in deployment '{deployment}'" in result + + +def test_set_value_path_not_found(command, mock_config_file): + """Test setting a value for a path that doesn't exist.""" + path = "nonexistent.path" + deployment = "dev" + value = "3" + + # 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(KeyError, match=f"Path {path} not found"): + command.execute(path=path, environment=deployment, value=value) + + +def test_set_value_deployment_not_found(command, mock_config_file): + """Test setting a value for a deployment that doesn't exist.""" + path = "app.replicas" + deployment = "nonexistent" + value = "3" + + # 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(KeyError, match=f"Deployment '{deployment}' not found"): + command.execute(path=path, environment=deployment, value=value) + + +def test_set_value_empty_path(command, mock_config_file): + """Test setting a value with an empty path.""" + path = "" + deployment = "dev" + value = "3" + + # 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=path, environment=deployment, value=value) + + +def test_set_value_empty_deployment(command, mock_config_file): + """Test setting a value with an empty deployment.""" + path = "app.replicas" + deployment = "" + value = "3" + + # 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="Deployment cannot be empty"): + command.execute(path=path, environment=deployment, value=value) + + +def test_set_value_none_config(command): + """Test setting a value when config is None.""" + # Mock the load_config method to return None + 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") diff --git a/tests/unit/test_cli.py b/tests/unit/test_cli.py index fe03bce..56e1912 100644 --- a/tests/unit/test_cli.py +++ b/tests/unit/test_cli.py @@ -223,3 +223,86 @@ def test_add_deployment_no_config(tmp_path): result = runner.invoke(app, ["add-deployment", "dev"], catch_exceptions=False) assert result.exit_code == 1 assert "Configuration file helm-values.json not found" in result.stdout + + +def test_set_value_command(tmp_path): + """Test set-value command.""" + # Change to temp directory + os.chdir(tmp_path) + + # Initialize the config + init_result = runner.invoke(app, ["init", "--release", "test-release"]) + assert init_result.exit_code == 0 + + # Add a deployment + add_deployment_result = runner.invoke(app, ["add-deployment", "dev"]) + assert add_deployment_result.exit_code == 0 + + # Add a value config + add_value_config_result = runner.invoke( + app, ["add-value-config", "--path", "app.replicas", "--description", "Number of replicas", "--required"] + ) + assert add_value_config_result.exit_code == 0 + + # Set a value + result = runner.invoke(app, ["set-value", "--path", "app.replicas", "--deployment", "dev", "--value", "3"]) + assert result.exit_code == 0 + assert "Successfully set value for path 'app.replicas' in deployment 'dev'" in result.stdout + + # Verify the value was set by checking the config file + with open("helm-values.json", "r") as f: + config = json.load(f) + + # The value itself is stored in the backend, not directly in the config file + # But we can verify that the path exists in the config + assert "app.replicas" in [path["path"] for path in config["config"]] + + +def test_set_value_nonexistent_path(tmp_path): + """Test set-value command with nonexistent path.""" + # Change to temp directory + os.chdir(tmp_path) + + # Initialize the config + init_result = runner.invoke(app, ["init", "--release", "test-release"]) + assert init_result.exit_code == 0 + + # Add a deployment + add_deployment_result = runner.invoke(app, ["add-deployment", "dev"]) + assert add_deployment_result.exit_code == 0 + + # Set a value for a nonexistent path + result = runner.invoke(app, ["set-value", "--path", "nonexistent.path", "--deployment", "dev", "--value", "3"]) + assert result.exit_code == 1 + assert "Path nonexistent.path not found" in result.stdout + + +def test_set_value_nonexistent_deployment(tmp_path): + """Test set-value command with nonexistent deployment.""" + # Change to temp directory + os.chdir(tmp_path) + + # Initialize the config + init_result = runner.invoke(app, ["init", "--release", "test-release"]) + assert init_result.exit_code == 0 + + # Add a value config + add_value_config_result = runner.invoke( + app, ["add-value-config", "--path", "app.replicas", "--description", "Number of replicas", "--required"] + ) + assert add_value_config_result.exit_code == 0 + + # Set a value for a nonexistent deployment + result = runner.invoke(app, ["set-value", "--path", "app.replicas", "--deployment", "nonexistent", "--value", "3"]) + assert result.exit_code == 1 + assert "Deployment 'nonexistent' not found" in result.stdout + + +def test_set_value_no_config(tmp_path): + """Test set-value command without initializing config first.""" + # Change to temp directory + os.chdir(tmp_path) + + result = runner.invoke(app, ["set-value", "--path", "app.replicas", "--deployment", "dev", "--value", "3"]) + assert result.exit_code == 1 + assert "Configuration file helm-values.json not found" in result.stdout