diff --git a/.gitignore b/.gitignore index c893026..2bcd885 100644 --- a/.gitignore +++ b/.gitignore @@ -121,6 +121,9 @@ ipython_config.py # PEP 582; used by e.g. github.com/David-OConnor/pyflow and github.com/pdm-project/pdm __pypackages__/ +# Helm Values Manager specific +.helm-values.lock + # Celery stuff celerybeat-schedule celerybeat.pid diff --git a/docs/Development/tasks.md b/docs/Development/tasks.md index c6fd83e..5eddb41 100644 --- a/docs/Development/tasks.md +++ b/docs/Development/tasks.md @@ -74,18 +74,90 @@ - [ ] Add better validation ### Command System -- [ ] Implement BaseCommand improvements - - [ ] Add file locking mechanism - - [ ] Implement backup strategy - - [ ] Add better error handling -- [ ] Add new commands + +#### Phase 1: Core Infrastructure & Essential Commands +- [x] Basic Command Framework + - [x] Create commands directory structure + - [x] Implement BaseCommand class with basic flow + - [x] Add configuration loading/saving + - [x] Add error handling and logging + - [ ] Add command registration in CLI + - [ ] Add basic command discovery + +- [ ] Configuration Setup Commands + - [ ] Implement init command + - [ ] Add empty config initialization + - [ ] Add config file creation + - [ ] Add schema template generation - [ ] Implement add-value-config command + - [ ] Add basic path validation + - [ ] Add metadata validation + - [ ] Add config update + - [ ] Implement add-deployment command + - [ ] Add basic deployment validation + - [ ] Add backend validation + - [ ] 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 + +#### Phase 2: Enhanced Safety & Management +- [ ] Enhanced Command Infrastructure + - [ ] Add file locking mechanism + - [ ] Add atomic writes + - [ ] Add basic backup strategy + +- [ ] Configuration Management - [ ] Implement remove-value-config command - - [ ] Update existing commands for new structure -- [ ] Update command validation - - [ ] Add input validation - - [ ] Improve error messages - - [ ] Add command-specific validation + - [ ] Add path validation + - [ ] Add basic cleanup + - [ ] Enhance add-value-config + - [ ] Add conflict detection + - [ ] Add dependency validation + +- [ ] Basic Validation System + - [ ] Add PathValidator class + - [ ] Add path format validation + - [ ] Add existence checks + +#### Phase 3: Advanced Features +- [ ] Enhanced Security & Recovery + - [ ] Add comprehensive backup strategy + - [ ] Add rollback support + - [ ] Improve error handling + +- [ ] Deployment Management + - [ ] Add DeploymentValidator class + +- [ ] Advanced Validation + - [ ] Add ValueValidator class + - [ ] Add conflict detection + - [ ] Add dependency checking + +#### Phase 4: Polish & Documentation +- [ ] Command Documentation + - [ ] Add command documentation generation + - [ ] Add help text improvements + - [ ] Add usage examples + +- [ ] Testing Infrastructure + - [ ] Add command test fixtures + - [ ] Add mock file system + - [ ] Add mock backend + - [ ] Add integration tests + +- [ ] Final Touches + - [ ] Add command output formatting + - [ ] Add progress indicators + - [ ] Add interactive mode support ### Testing Infrastructure - [x] Set up test infrastructure diff --git a/helm_values_manager/commands/__init__.py b/helm_values_manager/commands/__init__.py new file mode 100644 index 0000000..5472834 --- /dev/null +++ b/helm_values_manager/commands/__init__.py @@ -0,0 +1 @@ +"""Commands package containing command implementations for the helm-values-manager.""" diff --git a/helm_values_manager/commands/base_command.py b/helm_values_manager/commands/base_command.py new file mode 100644 index 0000000..73efc34 --- /dev/null +++ b/helm_values_manager/commands/base_command.py @@ -0,0 +1,150 @@ +"""Base command class for helm-values plugin.""" + +import fcntl +import json +import os +from typing import Any, Optional + +from jsonschema.exceptions import ValidationError + +from helm_values_manager.models.helm_values_config import HelmValuesConfig +from helm_values_manager.utils.logger import HelmLogger + + +class BaseCommand: + """Base class for all helm-values commands. + + This class provides common functionality for all commands including: + - Configuration loading and saving + - Error handling and logging + - Lock management for concurrent access + """ + + def __init__(self) -> None: + """Initialize the base command.""" + self.config_file = "helm-values.json" + self.lock_file = ".helm-values.lock" + self._lock_fd: Optional[int] = None + + def _load_config_file(self) -> dict: + """Load and parse the configuration file. + + Returns: + dict: The parsed configuration data + + Raises: + FileNotFoundError: If the config file doesn't exist + json.JSONDecodeError: If the file contains invalid JSON + """ + if not os.path.exists(self.config_file): + HelmLogger.error("Configuration file %s not found", self.config_file) + raise FileNotFoundError(f"Configuration file {self.config_file} not found") + + try: + with open(self.config_file, "r", encoding="utf-8") as f: + return json.load(f) + except json.JSONDecodeError as e: + HelmLogger.error("Failed to parse configuration file: %s", e) + raise + + def load_config(self) -> HelmValuesConfig: + """Load the helm-values configuration from disk. + + Returns: + HelmValuesConfig: The loaded configuration. + + Raises: + FileNotFoundError: If the config file doesn't exist. + json.JSONDecodeError: If the file contains invalid JSON. + ValidationError: If the configuration format is invalid. + """ + data = self._load_config_file() + try: + return HelmValuesConfig.from_dict(data) + except ValidationError as e: + HelmLogger.error("Invalid configuration format: %s", e) + raise + + def save_config(self, config: HelmValuesConfig) -> None: + """Save the helm-values configuration to disk. + + Args: + config: The configuration to save. + + Raises: + IOError: If unable to write to the file. + """ + try: + with open(self.config_file, "w", encoding="utf-8") as f: + json.dump(config.to_dict(), f, indent=2) + except IOError as e: + HelmLogger.error("Failed to save configuration: %s", e) + raise + + def acquire_lock(self) -> None: + """Acquire an exclusive lock for file operations. + + This ensures thread-safe operations when multiple commands + are trying to modify the configuration. + + Raises: + IOError: If unable to acquire the lock. + """ + self._lock_fd = os.open(self.lock_file, os.O_CREAT | os.O_RDWR) + try: + fcntl.flock(self._lock_fd, fcntl.LOCK_EX | fcntl.LOCK_NB) + HelmLogger.debug("Acquired lock on file %s", self.lock_file) + except IOError: + os.close(self._lock_fd) + self._lock_fd = None + HelmLogger.error("Unable to acquire lock. Another command may be running.") + raise IOError("Unable to acquire lock. Another command may be running.") + + def release_lock(self) -> None: + """Release the exclusive lock.""" + if self._lock_fd is not None: + fcntl.flock(self._lock_fd, fcntl.LOCK_UN) + os.close(self._lock_fd) + self._lock_fd = None + HelmLogger.debug("Released lock on file %s", self.lock_file) + + def execute(self) -> Any: + """Execute the command. + + This is the main entry point for running a command. + It handles: + 1. Lock acquisition + 2. Configuration loading + 3. Command execution via run() + 4. Lock release + + Returns: + Any: The result of the command execution. + + Raises: + Exception: If any error occurs during command execution. + """ + try: + self.acquire_lock() + config = self.load_config() + result = self.run(config) + return result + finally: + self.release_lock() + + def run(self, config: HelmValuesConfig) -> Any: + """Run the command-specific logic. + + This method should be implemented by each specific command subclass + to perform its unique functionality. + + Args: + config: The loaded configuration. + + Returns: + Any: The result of running the command. + + Raises: + NotImplementedError: This method must be implemented by subclasses. + """ + raise NotImplementedError("Subclasses must implement run()") diff --git a/tests/unit/commands/__init__.py b/tests/unit/commands/__init__.py new file mode 100644 index 0000000..974f66f --- /dev/null +++ b/tests/unit/commands/__init__.py @@ -0,0 +1 @@ +"""Unit tests for the commands package.""" diff --git a/tests/unit/commands/test_base_command.py b/tests/unit/commands/test_base_command.py new file mode 100644 index 0000000..09d25e2 --- /dev/null +++ b/tests/unit/commands/test_base_command.py @@ -0,0 +1,170 @@ +"""Unit tests for the BaseCommand class.""" + +import json +import os +from unittest.mock import MagicMock, mock_open, patch + +import pytest +from jsonschema.exceptions import ValidationError + +from helm_values_manager.commands.base_command import BaseCommand +from helm_values_manager.models.helm_values_config import HelmValuesConfig + + +class TestBaseCommand: + """Test cases for BaseCommand class.""" + + @pytest.fixture + def temp_lock_file(self, tmp_path): + """Fixture for temporary lock file.""" + lock_file = tmp_path / ".helm-values.lock" + return str(lock_file) + + @pytest.fixture + def base_command(self, temp_lock_file): + """Fixture for BaseCommand instance with mocked lock file.""" + command = BaseCommand() + command.lock_file = temp_lock_file + return command + + @pytest.fixture + def mock_config(self): + """Fixture for mocked HelmValuesConfig.""" + return MagicMock(spec=HelmValuesConfig) + + @pytest.fixture + def valid_config(self): + """Fixture for valid configuration data.""" + return {"version": "1.0", "release": "test", "deployments": {}, "config": []} + + def test_initialization(self, base_command, temp_lock_file): + """Test BaseCommand initialization.""" + assert base_command.config_file == "helm-values.json" + assert base_command.lock_file == temp_lock_file + assert base_command._lock_fd is None + + def test_load_config_file_not_found(self, base_command): + """Test load_config when file doesn't exist.""" + with patch("os.path.exists", return_value=False): + with pytest.raises(FileNotFoundError, match="Configuration file .* not found"): + base_command._load_config_file() + + def test_load_config_file_invalid_json(self, base_command): + """Test load_config with invalid JSON.""" + with patch("os.path.exists", return_value=True), patch("builtins.open", mock_open(read_data="invalid json")): + with pytest.raises(json.JSONDecodeError): + base_command._load_config_file() + + def test_load_config_file_success(self, base_command, valid_config): + """Test successful config file loading.""" + with ( + patch("os.path.exists", return_value=True), + patch("builtins.open", mock_open(read_data=json.dumps(valid_config))), + ): + data = base_command._load_config_file() + assert data == valid_config + + def test_load_config_invalid_format(self, base_command): + """Test load_config with invalid configuration format.""" + invalid_config = {"version": "1.0"} # Missing required fields + with patch.object(base_command, "_load_config_file", return_value=invalid_config): + with pytest.raises(ValidationError, match="'release' is a required property"): + base_command.load_config() + + def test_load_config_success(self, base_command, valid_config): + """Test successful config loading.""" + with patch.object(base_command, "_load_config_file", return_value=valid_config): + config = base_command.load_config() + assert isinstance(config, HelmValuesConfig) + assert config.version == valid_config["version"] + assert config.release == valid_config["release"] + + def test_run_not_implemented(self, base_command): + """Test that run raises NotImplementedError.""" + with pytest.raises(NotImplementedError, match="Subclasses must implement run()"): + base_command.run(MagicMock()) + + def test_acquire_and_release_lock(self, base_command): + """Test lock acquisition and release.""" + with patch("helm_values_manager.commands.base_command.fcntl") as mock_fcntl: + base_command.acquire_lock() + mock_fcntl.flock.assert_called_once() + + base_command.release_lock() + assert mock_fcntl.flock.call_count == 2 + + def test_acquire_lock_failure(self, base_command): + """Test lock acquisition when another process holds the lock.""" + with ( + patch("helm_values_manager.commands.base_command.fcntl") as mock_fcntl, + patch("os.open", return_value=123), + patch("os.close"), + ): + mock_fcntl.flock.side_effect = IOError("Resource temporarily unavailable") + + with pytest.raises(IOError, match="Unable to acquire lock. Another command may be running."): + base_command.acquire_lock() + + # Verify the file descriptor was closed + os.close.assert_called_once_with(123) + assert base_command._lock_fd is None + + def test_release_lock_without_lock(self, base_command): + """Test release_lock when no lock is held.""" + # Release without acquiring - should not raise any error + base_command.release_lock() + assert base_command._lock_fd is None + + def test_execute_success(self, base_command, valid_config): + """Test successful command execution flow.""" + expected_result = "success" + base_command.run = MagicMock(return_value=expected_result) + + with ( + patch.object(base_command, "_load_config_file", return_value=valid_config), + patch("helm_values_manager.commands.base_command.fcntl"), + ): + result = base_command.execute() + + assert isinstance(base_command.run.call_args[0][0], HelmValuesConfig) + assert result == expected_result + + def test_execute_ensures_lock_release_on_error(self, base_command, valid_config): + """Test that lock is released even if an error occurs.""" + base_command.run = MagicMock(side_effect=ValueError("Test error")) + + with ( + patch.object(base_command, "_load_config_file", return_value=valid_config), + patch("helm_values_manager.commands.base_command.fcntl") as mock_fcntl, + ): + with pytest.raises(ValueError, match="Test error"): + base_command.execute() + + assert mock_fcntl.flock.call_count == 2 # Lock acquired and released + + def test_save_config_success(self, base_command): + """Test successful config saving.""" + config = HelmValuesConfig() + config.version = "1.0" + config.release = "test" + + with patch("builtins.open", mock_open()) as mock_file: + base_command.save_config(config) + + mock_file.assert_called_once_with(base_command.config_file, "w", encoding="utf-8") + handle = mock_file() + + written_json = "".join(call.args[0] for call in handle.write.call_args_list) + written_data = json.loads(written_json) + assert written_data["version"] == config.version + assert written_data["release"] == config.release + + def test_save_config_io_error(self, base_command): + """Test save_config when IO error occurs.""" + config = HelmValuesConfig() + error_message = "Test error" + + with patch("builtins.open", mock_open()) as mock_file: + mock_file.return_value.write.side_effect = IOError(error_message) + with pytest.raises(IOError, match=error_message): + base_command.save_config(config)