Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix parsing of grafana feature flags that're enabled via the feature_toggles.enabled syntax #2477

Merged
merged 3 commits into from
Jul 10, 2023
Merged

fix parsing of grafana feature flags that're enabled via the feature_toggles.enabled syntax #2477

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5dfa926
fix parsing of grafana feature flags that're
joeyorlando Jul 10, 2023
b142440
update tests
joeyorlando Jul 10, 2023
93aff09
update changelog
joeyorlando Jul 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +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 ([#2477](https://github.com/grafana/oncall/pull/2477))

## v1.3.7 (2023-07-06)

Expand Down
4 changes: 2 additions & 2 deletions engine/apps/grafana_plugin/helpers/client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
12 changes: 7 additions & 5 deletions engine/apps/grafana_plugin/tests/test_gcom_api_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,25 +36,27 @@ 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),
# 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",
}
}
},
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:
Expand Down