From 5dfa926f075ca5b7047cf8e3e7fa6446900a68b4 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 10 Jul 2023 10:11:08 +0200 Subject: [PATCH 1/3] fix parsing of grafana feature flags that're enabled via the feature_toggles.enabled syntax --- CHANGELOG.md | 5 +++++ engine/apps/grafana_plugin/helpers/client.py | 4 ++-- engine/apps/grafana_plugin/tests/test_gcom_api_client.py | 6 ++++-- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4db19143df..f5016d1d1f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 endpoints so that the `next` and `previous` pagination links are properly set when OnCall is run behind a reverse proxy by @joeyorlando ([#TBD](https://github.com/grafana/oncall/pull/TBD)) +### Fixed + +- Address issue where we were improperly parsing Grafana feature flags that were enabled via the `feature_flags.enabled` + method by @joeyorlando ([#TBD](https://github.com/grafana/oncall/pull/TBD)) + ## v1.3.7 (2023-07-06) ### Changed diff --git a/engine/apps/grafana_plugin/helpers/client.py b/engine/apps/grafana_plugin/helpers/client.py index c02cddbc25..861cba89ba 100644 --- a/engine/apps/grafana_plugin/helpers/client.py +++ b/engine/apps/grafana_plugin/helpers/client.py @@ -284,8 +284,8 @@ def _feature_toggle_is_enabled(self, instance_info: GCOMInstanceInfo, feature_na 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(" ") + # 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 feature_enabled_via_direct_key = instance_feature_toggles.get(feature_name, "false") == "true" diff --git a/engine/apps/grafana_plugin/tests/test_gcom_api_client.py b/engine/apps/grafana_plugin/tests/test_gcom_api_client.py index 0df6ae31c0..469d80879b 100644 --- a/engine/apps/grafana_plugin/tests/test_gcom_api_client.py +++ b/engine/apps/grafana_plugin/tests/test_gcom_api_client.py @@ -36,7 +36,7 @@ def test_it_returns_based_on_feature_toggle_value( ({}, False), ({"config": {}}, False), ({"config": {"feature_toggles": {}}}, False), - ({"config": {"feature_toggles": {"enable": "foo bar baz"}}}, 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), @@ -53,8 +53,10 @@ 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": {"enable": f"foo bar {TEST_FEATURE_TOGGLE} baz"}}}, 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: From b142440710acae27a9f95d8829cddae8bd1331df Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 10 Jul 2023 10:49:01 +0200 Subject: [PATCH 2/3] update tests --- engine/apps/grafana_plugin/tests/test_gcom_api_client.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/engine/apps/grafana_plugin/tests/test_gcom_api_client.py b/engine/apps/grafana_plugin/tests/test_gcom_api_client.py index 469d80879b..395d9b8886 100644 --- a/engine/apps/grafana_plugin/tests/test_gcom_api_client.py +++ b/engine/apps/grafana_plugin/tests/test_gcom_api_client.py @@ -38,14 +38,14 @@ 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 space separated - ({"config": {"feature_toggles": {"enable": f"foo bar {TEST_FEATURE_TOGGLE}baz"}}}, 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 baz {TEST_FEATURE_TOGGLE}", + "enable": f"foo,bar,baz,{TEST_FEATURE_TOGGLE}", TEST_FEATURE_TOGGLE: "false", } } From 93aff09e97f227c3bbc1a025c8826b223698496f Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Mon, 10 Jul 2023 10:58:53 +0200 Subject: [PATCH 3/3] update changelog --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f5016d1d1f..11fa2be956 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,12 +16,12 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Modified DRF pagination class used by `GET /api/internal/v1/alert_receive_channels` and `GET /api/internal/v1/schedules` endpoints so that the `next` and `previous` pagination links are properly set when OnCall is run behind - a reverse proxy by @joeyorlando ([#TBD](https://github.com/grafana/oncall/pull/TBD)) + a reverse proxy by @joeyorlando ([#2467](https://github.com/grafana/oncall/pull/2467)) ### Fixed - Address issue where we were improperly parsing Grafana feature flags that were enabled via the `feature_flags.enabled` - method by @joeyorlando ([#TBD](https://github.com/grafana/oncall/pull/TBD)) + method by @joeyorlando ([#2477](https://github.com/grafana/oncall/pull/2477)) ## v1.3.7 (2023-07-06)