properly parse grafana cloud feature toggles (#1880)

# What this PR does ## Which issue(s) this PR fixes ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) (N/A) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required)
grafana · May 4, 2023 · 2879537 · 2879537
1 parent e2901a3
commit 2879537
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 1 deletion.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Improve ical comparison when checking for imported ical updates ([1870](https://github.com/grafana/oncall/pull/1870))
 
+### Fixed
+
+- Fix issue with how OnCall determines if a cloud Grafana Instance supports RBAC by @joeyorlando ([#1880](https://github.com/grafana/oncall/pull/1880))
+
 ## v1.2.18 (2023-05-03)
 
 ### Added

diff --git a/engine/apps/grafana_plugin/helpers/client.py b/engine/apps/grafana_plugin/helpers/client.py
@@ -217,14 +217,31 @@ def get_instance_info(self, stack_id: str, include_config_query_param: bool = Fa
         data, _ = self.api_get(url)
         return data
 
+    def _feature_toggle_is_enabled(self, instance_info: GCOMInstanceInfo, feature_name: str) -> bool:
+        """
+        there are two ways that feature toggles can be enabled, this method takes into account both
+        https://grafana.com/docs/grafana/latest/setup-grafana/configure-grafana/#enable
+        """
+        instance_feature_toggles = instance_info.get("config", {}).get("feature_toggles", {})
+
+        if not instance_feature_toggles:
+            return False
+
+        # features enabled via enable key are space separated
+        features_enabled_via_enable_key = instance_feature_toggles.get("enable", "").split(" ")
+        feature_enabled_via_enable_key = feature_name in features_enabled_via_enable_key
+        feature_enabled_via_direct_key = instance_feature_toggles.get(feature_name, "false") == "true"
+
+        return feature_enabled_via_enable_key or feature_enabled_via_direct_key
+
     def is_rbac_enabled_for_stack(self, stack_id: str) -> bool:
         """
         NOTE: must use an "Admin" GCOM token when calling this method
         """
         instance_info = self.get_instance_info(stack_id, True)
         if not instance_info:
             return False
-        return instance_info.get("config", {}).get("feature_toggles", {}).get("accessControlOnCall", "false") == "true"
+        return self._feature_toggle_is_enabled(instance_info, "accessControlOnCall")
 
     def get_instances(self, query: str):
         return self.api_get(query)

diff --git a/engine/apps/grafana_plugin/tests/test_gcom_api_client.py b/engine/apps/grafana_plugin/tests/test_gcom_api_client.py
@@ -6,6 +6,8 @@
 
 
 class TestIsRbacEnabledForStack:
+    TEST_FEATURE_TOGGLE = "helloWorld"
+
     @pytest.mark.parametrize(
         "gcom_api_response,expected",
         [
@@ -27,3 +29,36 @@ def test_it_returns_based_on_feature_toggle_value(
         api_client = GcomAPIClient("someFakeApiToken")
         assert api_client.is_rbac_enabled_for_stack(stack_id) == expected
         assert mocked_gcom_api_client_api_get.called_once_with(f"instances/{stack_id}?config=true")
+
+    @pytest.mark.parametrize(
+        "instance_info,expected",
+        [
+            ({}, False),
+            ({"config": {}}, False),
+            ({"config": {"feature_toggles": {}}}, False),
+            ({"config": {"feature_toggles": {"enable": "foo bar baz"}}}, False),
+            ({"config": {"feature_toggles": {TEST_FEATURE_TOGGLE: "false"}}}, False),
+            # must be space separated
+            ({"config": {"feature_toggles": {"enable": f"foo bar {TEST_FEATURE_TOGGLE}baz"}}}, False),
+            # these cases will probably never happen, but lets account for them anyways
+            (
+                {
+                    "config": {
+                        "feature_toggles": {
+                            "enable": f"foo bar baz {TEST_FEATURE_TOGGLE}",
+                            TEST_FEATURE_TOGGLE: "false",
+                        }
+                    }
+                },
+                True,
+            ),
+            ({"config": {"feature_toggles": {"enable": f"foo bar baz", TEST_FEATURE_TOGGLE: "true"}}}, True),
+            ({"config": {"feature_toggles": {"enable": f"foo bar {TEST_FEATURE_TOGGLE} baz"}}}, True),
+            ({"config": {"feature_toggles": {TEST_FEATURE_TOGGLE: "true"}}}, True),
+        ],
+    )
+    def test_feature_toggle_is_enabled(self, instance_info, expected) -> None:
+        assert (
+            GcomAPIClient("someFakeApiToken")._feature_toggle_is_enabled(instance_info, self.TEST_FEATURE_TOGGLE)
+            == expected
+        )