From 64d46584a9bb8d9dc7e3e21c101250586465bdf5 Mon Sep 17 00:00:00 2001 From: Himawan Winarto Date: Thu, 7 May 2026 09:24:38 -0400 Subject: [PATCH] fix: grant access for observability model to other nodes Closes-Bug: #2041610 Signed-off-by: Himawan Winarto --- .../sunbeam/features/observability/feature.py | 21 ++- .../sunbeam/provider/local/commands.py | 12 +- .../sunbeam/features/test_observability.py | 54 +++++++ .../sunbeam/provider/local/test_commands.py | 139 ++++++++++++++++++ 4 files changed, 223 insertions(+), 3 deletions(-) create mode 100644 sunbeam-python/tests/unit/sunbeam/provider/local/test_commands.py diff --git a/sunbeam-python/sunbeam/features/observability/feature.py b/sunbeam-python/sunbeam/features/observability/feature.py index 4d5c8a4e5..4ed150476 100644 --- a/sunbeam-python/sunbeam/features/observability/feature.py +++ b/sunbeam-python/sunbeam/features/observability/feature.py @@ -79,7 +79,10 @@ TerraformPlanLocation, ) from sunbeam.steps import openstack -from sunbeam.steps.juju import RemoveSaasApplicationsStep +from sunbeam.steps.juju import ( + JujuGrantModelAccessStep, + RemoveSaasApplicationsStep, +) from sunbeam.steps.k8s import CREDENTIAL_SUFFIX from sunbeam.utils import click_option_show_hints, pass_method_obj from sunbeam.versions import TRAEFIK_CHANNEL @@ -880,6 +883,22 @@ def post_enable( } update_config(deployment.get_client(), OBSERVABILITY_FEATURE_KEY, provider) + # Grant all existing Juju users access to the observability model + client = deployment.get_client() + jhelper = JujuHelper(deployment.juju_controller) + + for node in client.cluster.list_nodes(): + node_name = node["name"] + try: + plan = [ + JujuGrantModelAccessStep(jhelper, node_name, OBSERVABILITY_MODEL) + ] + run_plan(plan, console, show_hints) + except Exception as e: + LOG.warning( + "Failed to grant %s access to observability model: %s", node_name, e + ) + def pre_disable(self, deployment: Deployment, show_hints: bool) -> None: """Handler to perform tasks before disabling the feature.""" super().pre_disable(deployment, show_hints) diff --git a/sunbeam-python/sunbeam/provider/local/commands.py b/sunbeam-python/sunbeam/provider/local/commands.py index 348524ae9..89c5a6a84 100644 --- a/sunbeam-python/sunbeam/provider/local/commands.py +++ b/sunbeam-python/sunbeam/provider/local/commands.py @@ -1196,11 +1196,19 @@ def add( JujuLoginStep(deployment.juju_account), ClusterAddNodeStep(client, name), CreateJujuUserStep(name), - JujuGrantModelAccessStep(jhelper, name, deployment.openstack_machines_model), - JujuGrantModelAccessStep(jhelper, name, OPENSTACK_MODEL), ] + plan1_results = run_plan(plan1, console, show_hints) + # Grant the new node access to all Juju models + plan_access = [ + JujuGrantModelAccessStep(jhelper, name, model["short-name"]) + for model in jhelper.models() + if model["short-name"] + ] + if plan_access: + run_plan(plan_access, console, show_hints) + add_node_step_result = get_step_result(plan1_results, ClusterAddNodeStep) create_juju_user_step_result = get_step_result(plan1_results, CreateJujuUserStep) diff --git a/sunbeam-python/tests/unit/sunbeam/features/test_observability.py b/sunbeam-python/tests/unit/sunbeam/features/test_observability.py index d1055a616..aaf4efaf5 100644 --- a/sunbeam-python/tests/unit/sunbeam/features/test_observability.py +++ b/sunbeam-python/tests/unit/sunbeam/features/test_observability.py @@ -43,6 +43,18 @@ def k8shelper(): yield p +@pytest.fixture() +def run_plan_obs(): + with patch("sunbeam.features.observability.feature.run_plan") as p: + yield p + + +@pytest.fixture() +def juju_helper_obs(): + with patch("sunbeam.features.observability.feature.JujuHelper") as p: + yield p + + class TestDeployObservabilityStackStep: def test_run( self, @@ -640,3 +652,45 @@ def test_storage_dict_deep_merges(self, read_config_obs): "active-index-directory": "4G", "loki-chunks": "8G", } + + +class TestObservabilityFeaturePostEnable: + """Test the post_enable grant access logic.""" + + def test_post_enable_grants_access_to_all_nodes( + self, deployment, update_config, run_plan_obs, juju_helper_obs + ): + """All nodes get JujuGrantModelAccessStep called via run_plan.""" + deployment.get_client.return_value.cluster.list_nodes.return_value = [ + {"name": "node-1"}, + {"name": "node-2"}, + {"name": "node-3"}, + ] + feature = observability_feature.EmbeddedObservabilityFeature() + + feature.post_enable(deployment, MagicMock(), show_hints=False) + + assert run_plan_obs.call_count == 3 + for i, call in enumerate(run_plan_obs.call_args_list, start=1): + plan = call[0][0] + assert len(plan) == 1 + step = plan[0] + assert isinstance(step, observability_feature.JujuGrantModelAccessStep) + assert step.username == f"node-{i}" + assert step.model == observability_feature.OBSERVABILITY_MODEL + + def test_post_enable_handles_grant_failure_gracefully( + self, deployment, update_config, run_plan_obs, juju_helper_obs + ): + """If granting access fails for one node, others are still processed.""" + deployment.get_client.return_value.cluster.list_nodes.return_value = [ + {"name": "node-1"}, + {"name": "node-2"}, + {"name": "node-3"}, + ] + run_plan_obs.side_effect = [None, Exception("grant failed"), None] + feature = observability_feature.EmbeddedObservabilityFeature() + + feature.post_enable(deployment, MagicMock(), show_hints=False) + + assert run_plan_obs.call_count == 3 diff --git a/sunbeam-python/tests/unit/sunbeam/provider/local/test_commands.py b/sunbeam-python/tests/unit/sunbeam/provider/local/test_commands.py new file mode 100644 index 000000000..4a0aba08f --- /dev/null +++ b/sunbeam-python/tests/unit/sunbeam/provider/local/test_commands.py @@ -0,0 +1,139 @@ +# SPDX-FileCopyrightText: 2023 - Canonical Ltd +# SPDX-License-Identifier: Apache-2.0 + +from unittest.mock import Mock, patch + +import pytest +from click.testing import CliRunner + +from sunbeam.core.common import ResultType +from sunbeam.provider.local.commands import add +from sunbeam.steps.juju import JujuGrantModelAccessStep + + +@pytest.fixture() +def run_preflight(): + with patch("sunbeam.provider.local.commands.run_preflight_checks") as p: + yield p + + +@pytest.fixture() +def daemon_group_check(): + with patch("sunbeam.provider.local.commands.DaemonGroupCheck") as p: + yield p + + +@pytest.fixture() +def verify_fqdn_check(): + with patch("sunbeam.provider.local.commands.VerifyFQDNCheck") as p: + yield p + + +@pytest.fixture() +def run_plan_cmd(): + with patch("sunbeam.provider.local.commands.run_plan") as p: + yield p + + +@pytest.fixture() +def juju_helper_cmd(): + with patch("sunbeam.provider.local.commands.JujuHelper") as p: + yield p + + +@pytest.fixture() +def get_step_result_cmd(): + with patch("sunbeam.provider.local.commands.get_step_result") as p: + yield p + + +@pytest.fixture() +def get_step_message_cmd(): + with patch("sunbeam.provider.local.commands.get_step_message") as p: + yield p + + +class TestAddNodeGrantModels: + """Test that adding a node grants access to all Juju models dynamically.""" + + def test_add_grants_access_to_all_models( + self, + daemon_group_check, + verify_fqdn_check, + run_preflight, + run_plan_cmd, + juju_helper_cmd, + get_step_result_cmd, + get_step_message_cmd, + ): + """run_plan receives JujuGrantModelAccessStep for every model.""" + jhelper_instance = juju_helper_cmd.return_value + jhelper_instance.models.return_value = [ + {"short-name": "openstack-machines"}, + {"short-name": "openstack"}, + {"short-name": "observability"}, + ] + + add_node_result = Mock(result_type=ResultType.COMPLETED, message="test-token") + create_user_result = Mock(result_type=ResultType.COMPLETED) + get_step_result_cmd.side_effect = [add_node_result, create_user_result] + get_step_message_cmd.return_value = "user-token" + + deployment = Mock() + runner = CliRunner() + result = runner.invoke(add, ["new-node.domain"], obj=deployment) + + assert result.exit_code == 0, result.output + + # Second call to run_plan is plan_access + plan_access = run_plan_cmd.call_args_list[1][0][0] + grant_steps = [ + s for s in plan_access if isinstance(s, JujuGrantModelAccessStep) + ] + assert len(grant_steps) == 3 + assert [s.model for s in grant_steps] == [ + "openstack-machines", + "openstack", + "observability", + ] + for step in grant_steps: + assert step.username == "new-node.domain" + + def test_add_skips_models_with_empty_short_name( + self, + daemon_group_check, + verify_fqdn_check, + run_preflight, + run_plan_cmd, + juju_helper_cmd, + get_step_result_cmd, + get_step_message_cmd, + ): + """Models with empty short-name are excluded from grant steps.""" + jhelper_instance = juju_helper_cmd.return_value + jhelper_instance.models.return_value = [ + {"short-name": "openstack"}, + {"short-name": "observability"}, + {"short-name": ""}, + ] + + add_node_result = Mock(result_type=ResultType.COMPLETED, message="test-token") + create_user_result = Mock(result_type=ResultType.COMPLETED) + get_step_result_cmd.side_effect = [add_node_result, create_user_result] + get_step_message_cmd.return_value = "user-token" + + deployment = Mock() + runner = CliRunner() + result = runner.invoke(add, ["new-node.domain"], obj=deployment) + + assert result.exit_code == 0, result.output + + plan_access = run_plan_cmd.call_args_list[1][0][0] + grant_steps = [ + s for s in plan_access if isinstance(s, JujuGrantModelAccessStep) + ] + assert len(grant_steps) == 2 + model_names = [s.model for s in grant_steps] + assert "openstack" in model_names + assert "observability" in model_names + assert "" not in model_names