From 83795ae2e2b14f8af229ddc2c5b50947ca7cec33 Mon Sep 17 00:00:00 2001 From: Guillaume Boutry Date: Fri, 14 Aug 2026 11:07:29 +0200 Subject: [PATCH] fix(storage): handle manifest without backend config Read backend config directly so software-only storage manifests retain an empty preseed before CLI and config-file values are merged. Closes-Bug: #2161882 Assisted-By: Codex (gpt-5-6-sol) Signed-off-by: Guillaume Boutry (cherry picked from commit 4fe7e1da1d1c705d0f9ca54fa6ea1c7afe8bd689) --- sunbeam-python/sunbeam/storage/steps.py | 3 +- .../tests/unit/sunbeam/storage/test_base.py | 8 +- .../tests/unit/sunbeam/storage/test_steps.py | 93 +++++++++++++++++++ 3 files changed, 102 insertions(+), 2 deletions(-) diff --git a/sunbeam-python/sunbeam/storage/steps.py b/sunbeam-python/sunbeam/storage/steps.py index bc159bb84..7d0534350 100644 --- a/sunbeam-python/sunbeam/storage/steps.py +++ b/sunbeam-python/sunbeam/storage/steps.py @@ -303,7 +303,8 @@ def prompt( if crt := backends.root.get(self.backend_name): # Since question generation depends on field name, # do not dump by alias - preseed = crt.model_dump(by_alias=False)["config"] + if crt.config is not None: + preseed = crt.config.model_dump(by_alias=False) # Preseed from user is higher priority than manifest preseed.update(self.preseed) diff --git a/sunbeam-python/tests/unit/sunbeam/storage/test_base.py b/sunbeam-python/tests/unit/sunbeam/storage/test_base.py index 69ffe39f0..081fbe414 100644 --- a/sunbeam-python/tests/unit/sunbeam/storage/test_base.py +++ b/sunbeam-python/tests/unit/sunbeam/storage/test_base.py @@ -279,6 +279,12 @@ def test_remove_backend(self, backend, mock_deployment, mock_console, tmp_path): def test_build_terraform_vars(self, backend, mock_deployment, mock_manifest): """Test Terraform variables generation.""" backend_name = "test-backend" + charm_manifest = Mock(channel="latest/edge", revision=None) + backend_manifest = Mock() + backend_manifest.software.charms = {backend.charm_name: charm_manifest} + mock_manifest.storage.root = { + backend.backend_type: Mock(root={backend_name: backend_manifest}) + } config = backend.config_type().model_validate( { "required-field": "test", @@ -294,7 +300,7 @@ def test_build_terraform_vars(self, backend, mock_deployment, mock_manifest): assert tfvars["principal_application"] == backend.principal_application assert "charm_name" in tfvars assert tfvars["charm_name"] == backend.charm_name - assert "charm_channel" in tfvars + assert tfvars["charm_channel"] == "latest/edge" assert "charm_base" in tfvars assert "endpoint_bindings" in tfvars assert "charm_config" in tfvars diff --git a/sunbeam-python/tests/unit/sunbeam/storage/test_steps.py b/sunbeam-python/tests/unit/sunbeam/storage/test_steps.py index 73f0707df..1b563f7a7 100644 --- a/sunbeam-python/tests/unit/sunbeam/storage/test_steps.py +++ b/sunbeam-python/tests/unit/sunbeam/storage/test_steps.py @@ -12,6 +12,7 @@ from sunbeam.core.steps import DeployMachineApplicationStep from sunbeam.storage.models import SecretDictField from sunbeam.storage.steps import ( + BaseStorageBackendDeployStep, DeploySpecificCinderVolumeStep, basemodel_validator, generate_questions_from_config, @@ -103,6 +104,98 @@ def test_optional_questions_include_validation(self): optional_question.validation_function(-1) # type: ignore[arg-type] +class TestBaseStorageBackendDeployStep: + """Tests for the base storage backend deployment step.""" + + @pytest.fixture + def deploy_step( + self, + basic_deployment, + basic_client, + basic_tfhelper, + basic_jhelper, + basic_manifest, + mock_backend, + test_model, + ): + """Create a base storage backend deployment step.""" + return BaseStorageBackendDeployStep( + basic_deployment, + basic_client, + basic_tfhelper, + basic_jhelper, + basic_manifest, + { + "required_field": "cli-value", + "secret_field": "cli-secret", + }, + "test-backend", + mock_backend, + test_model, + ) + + def test_prompt_without_manifest_config( + self, deploy_step, basic_manifest, mock_backend + ): + """CLI configuration is used when manifest config is absent.""" + backend_manifest = Mock(config=None) + basic_manifest.storage.root = { + mock_backend.backend_type: Mock( + root={deploy_step.backend_name: backend_manifest} + ) + } + + with ( + patch("sunbeam.storage.steps.load_answers", return_value={}), + patch("sunbeam.storage.steps.QuestionBank") as question_bank, + patch("sunbeam.storage.steps.ConfirmQuestion") as confirm_question, + patch("sunbeam.storage.steps.write_answers"), + ): + question_bank.return_value.questions = {} + confirm_question.return_value.ask.return_value = False + + deploy_step.prompt() + + assert question_bank.call_args.kwargs["preseed"] == { + "required_field": "cli-value", + "secret_field": "cli-secret", + } + + def test_prompt_cli_config_overrides_manifest_config( + self, deploy_step, basic_manifest, mock_backend + ): + """CLI configuration takes precedence over manifest configuration.""" + manifest_config = mock_backend.config_type().model_validate( + { + "required-field": "manifest-value", + "secret-field": "manifest-secret", + "optional-field": "manifest-optional", + } + ) + backend_manifest = Mock(config=manifest_config) + basic_manifest.storage.root = { + mock_backend.backend_type: Mock( + root={deploy_step.backend_name: backend_manifest} + ) + } + + with ( + patch("sunbeam.storage.steps.load_answers", return_value={}), + patch("sunbeam.storage.steps.QuestionBank") as question_bank, + patch("sunbeam.storage.steps.ConfirmQuestion") as confirm_question, + patch("sunbeam.storage.steps.write_answers"), + ): + question_bank.return_value.questions = {} + confirm_question.return_value.ask.return_value = False + + deploy_step.prompt() + + preseed = question_bank.call_args.kwargs["preseed"] + assert preseed["required_field"] == "cli-value" + assert preseed["secret_field"] == "cli-secret" + assert preseed["optional_field"] == "manifest-optional" + + class TestDeploySpecificCinderVolumeStep: """Tests for DeploySpecificCinderVolumeStep class."""