support comma and space delimited grafana feature_toggles

CHANGELOG.md

@@ -7,6 +7,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 ## Unreleased
 
+### Fixed
+
+- Address issue when Grafana feature flags which were enabled via the `feature_flags.enabled` were only properly being
+  parsed, when they were space-delimited. This fix allows them to be _either_ space or comma-delimited.
+  by @joeyorlando ([#2623](https://github.com/grafana/oncall/pull/2623))
+
 ### Added
 
 - Added banner on the ChatOps screen for OSS to let the user know if no chatops integration is enabled

engine/apps/grafana_plugin/helpers/client.py

@@ -270,6 +270,11 @@ def get_instance_info(
         data, _ = self.api_get(url)
         return data
 
+    def _feature_is_enabled_via_enable_key(
+        self, instance_feature_toggles: GCOMInstanceInfoConfigFeatureToggles, feature_name: str, delimiter: str
+    ):
+        return feature_name in instance_feature_toggles.get("enable", "").split(delimiter)
+
     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
@@ -284,12 +289,22 @@ def _feature_toggle_is_enabled(self, instance_info: GCOMInstanceInfo, feature_na
         if not instance_feature_toggles:
             return False
 
-        # features enabled via enable key are comma separated (https://github.com/grafana/grafana/issues/36511)
-        features_enabled_via_enable_key = instance_feature_toggles.get("enable", "").split(",")
-        feature_enabled_via_enable_key = feature_name in features_enabled_via_enable_key
+        # features enabled via enable key can be either space or comma delimited
+        # https://raintank-corp.slack.com/archives/C036J5B39/p1690183217162019
+
+        feature_enabled_via_enable_key_space_delimited = self._feature_is_enabled_via_enable_key(
+            instance_feature_toggles, feature_name, " "
+        )
+        feature_enabled_via_enable_key_comma_delimited = self._feature_is_enabled_via_enable_key(
+            instance_feature_toggles, feature_name, ","
+        )
         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
+        return (
+            feature_enabled_via_direct_key
+            or feature_enabled_via_enable_key_space_delimited
+            or feature_enabled_via_enable_key_comma_delimited
+        )
 
     def is_rbac_enabled_for_stack(self, stack_id: str) -> bool:
         """

engine/apps/grafana_plugin/tests/test_gcom_api_client.py

@@ -30,6 +30,26 @@ def test_it_returns_based_on_feature_toggle_value(
         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_feature_toggles,delimiter,expected",
+        [
+            ({}, " ", False),
+            ({"enable": "foo,bar,baz"}, " ", False),
+            ({"enable": "foo,bar,baz"}, ",", False),
+            ({"enable": f"foo,bar,baz{TEST_FEATURE_TOGGLE}"}, " ", False),
+            ({"enable": f"foo,bar,baz{TEST_FEATURE_TOGGLE}"}, ",", False),
+            ({"enable": f"foo,bar,baz,{TEST_FEATURE_TOGGLE}abc"}, ",", False),
+            ({"enable": f"foo,bar,baz,{TEST_FEATURE_TOGGLE}"}, ",", True),
+        ],
+    )
+    def test_feature_is_enabled_via_enable_key(self, instance_info_feature_toggles, delimiter, expected) -> None:
+        assert (
+            GcomAPIClient("someFakeApiToken")._feature_is_enabled_via_enable_key(
+                instance_info_feature_toggles, self.TEST_FEATURE_TOGGLE, delimiter
+            )
+            == expected
+        )
+
     @pytest.mark.parametrize(
         "instance_info,expected",
         [
@@ -38,9 +58,12 @@ def test_it_returns_based_on_feature_toggle_value(
             ({"config": {"feature_toggles": {}}}, False),
             ({"config": {"feature_toggles": {"enable": "foo,bar,baz"}}}, False),
             ({"config": {"feature_toggles": {TEST_FEATURE_TOGGLE: "false"}}}, False),
-            # must be comma 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,{TEST_FEATURE_TOGGLE},baz"}}}, True),
+            ({"config": {"feature_toggles": {"enable": f"foo bar {TEST_FEATURE_TOGGLE} baz"}}}, True),
+            ({"config": {"feature_toggles": {"enable": f"foo bar baz", TEST_FEATURE_TOGGLE: "true"}}}, True),
+            ({"config": {"feature_toggles": {TEST_FEATURE_TOGGLE: "true"}}}, True),
+            # this case will probably never happen, but lets account for it anyways
             (
                 {
                     "config": {
@@ -52,11 +75,6 @@ def test_it_returns_based_on_feature_toggle_value(
                 },
                 True,
             ),
-            ({"config": {"feature_toggles": {"enable": f"foo bar baz", TEST_FEATURE_TOGGLE: "true"}}}, True),
-            ({"config": {"feature_toggles": {TEST_FEATURE_TOGGLE: "true"}}}, True),
-            # features enabled via feature_toggles.enable should be comma separated, not space separated
-            ({"config": {"feature_toggles": {"enable": f"foo,bar,{TEST_FEATURE_TOGGLE},baz"}}}, True),
-            ({"config": {"feature_toggles": {"enable": f"foo bar {TEST_FEATURE_TOGGLE} baz"}}}, False),
         ],
     )
     def test_feature_toggle_is_enabled(self, instance_info, expected) -> None: