fix: grant access for observability model to other nodes

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)

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)
 

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

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