diff --git a/engine/apps/grafana_plugin/helpers/client.py b/engine/apps/grafana_plugin/helpers/client.py index ea7488b4..b792994e 100644 --- a/engine/apps/grafana_plugin/helpers/client.py +++ b/engine/apps/grafana_plugin/helpers/client.py @@ -34,7 +34,7 @@ UserPermissionsDict = typing.Dict[str, typing.List[GrafanaAPIPermission]] class GCOMInstanceInfoConfigFeatureToggles(typing.TypedDict): - accessControlOnCall: str + accessControlOnCall: typing.NotRequired[str] class GCOMInstanceInfoConfig(typing.TypedDict): @@ -211,7 +211,7 @@ class GrafanaAPIClient(APIClient): def check_token(self) -> APIClientResponse: return self.api_head("api/org") - def get_users_permissions(self, rbac_is_enabled_for_org: bool) -> UserPermissionsDict: + def get_users_permissions(self) -> typing.Optional[UserPermissionsDict]: """ It is possible that this endpoint may not be available for certain Grafana orgs. Ex: for Grafana Cloud orgs whom have pinned their Grafana version to an earlier version @@ -229,13 +229,9 @@ class GrafanaAPIClient(APIClient): } } """ - if not rbac_is_enabled_for_org: - return {} response, _ = self.api_get(self.USER_PERMISSION_ENDPOINT) - if response is None: - return {} - elif isinstance(response, list): - return {} + if response is None or isinstance(response, list): + return None data: typing.Dict[str, typing.Dict[str, typing.List[str]]] = response @@ -259,7 +255,13 @@ class GrafanaAPIClient(APIClient): users: GrafanaUsersWithPermissions = users_response - user_permissions = self.get_users_permissions(rbac_is_enabled_for_org) + user_permissions = {} + if rbac_is_enabled_for_org: + user_permissions = self.get_users_permissions(rbac_is_enabled_for_org) + if user_permissions is None: + # If we cannot fetch permissions when RBAC is enabled (ex. HTTP 500), we should not return any users + # to avoid potentially wiping-out OnCall's copy of permissions for all users + return [] # merge the users permissions response into the org users response for user in users: @@ -349,51 +351,6 @@ class GcomAPIClient(APIClient): 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 - https://grafana.com/docs/grafana/latest/setup-grafana/configure-grafana/#enable - """ - instance_info_config = instance_info.get("config", {}) - if not instance_info_config: - return False - - instance_feature_toggles = instance_info_config.get("feature_toggles", {}) - - if not instance_feature_toggles: - return False - - # 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_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: - """ - 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 self._feature_toggle_is_enabled(instance_info, "accessControlOnCall") - def get_instances(self, query: str, page_size=None): if not page_size: page, _ = self.api_get(query) @@ -409,11 +366,20 @@ class GcomAPIClient(APIClient): yield page cursor = page["nextCursor"] + def _is_stack_in_certain_state(self, stack_id: str, state: str) -> bool: + instance_info = self.get_instance_info(stack_id) + if not instance_info: + return False + return instance_info.get("status") == state + def is_stack_deleted(self, stack_id: str) -> bool: url = f"instances?includeDeleted=true&id={stack_id}" instance_infos, _ = self.api_get(url) return instance_infos["items"] and instance_infos["items"][0].get("status") == self.STACK_STATUS_DELETED + def is_stack_active(self, stack_id: str) -> bool: + return self._is_stack_in_certain_state(stack_id, self.STACK_STATUS_ACTIVE) + def post_active_users(self, body) -> APIClientResponse: return self.api_post("app-active-users", body) 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 4b93437a..219396a2 100644 --- a/engine/apps/grafana_plugin/tests/test_gcom_api_client.py +++ b/engine/apps/grafana_plugin/tests/test_gcom_api_client.py @@ -8,85 +8,6 @@ from apps.grafana_plugin.helpers.gcom import get_instance_ids from settings.base import CLOUD_LICENSE_NAME -class TestIsRbacEnabledForStack: - TEST_FEATURE_TOGGLE = "helloWorld" - - @pytest.mark.parametrize( - "gcom_api_response,expected", - [ - (None, False), - ({}, False), - ({"config": {}}, False), - ({"config": {"feature_toggles": {}}}, False), - ({"config": {"feature_toggles": {"accessControlOnCall": "false"}}}, False), - ({"config": {"feature_toggles": {"accessControlOnCall": "true"}}}, True), - ], - ) - @patch("apps.grafana_plugin.helpers.client.GcomAPIClient.api_get") - def test_it_returns_based_on_feature_toggle_value( - self, mocked_gcom_api_client_api_get, gcom_api_response, expected - ): - stack_id = 5 - mocked_gcom_api_client_api_get.return_value = (gcom_api_response, {"status_code": 200}) - - 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_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", - [ - ({}, False), - ({"config": {}}, False), - ({"config": {"feature_toggles": {}}}, False), - ({"config": {"feature_toggles": {"enable": "foo,bar,baz"}}}, False), - ({"config": {"feature_toggles": {TEST_FEATURE_TOGGLE: "false"}}}, False), - ({"config": {"feature_toggles": {"enable": f"foo,bar,{TEST_FEATURE_TOGGLE}baz"}}}, False), - ({"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": "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": { - "feature_toggles": { - "enable": f"foo,bar,baz,{TEST_FEATURE_TOGGLE}", - TEST_FEATURE_TOGGLE: "false", - } - } - }, - 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 - ) - - def build_paged_responses(page_size, pages, total_items): response = [] remaining = total_items diff --git a/engine/apps/grafana_plugin/tests/test_grafana_api_client.py b/engine/apps/grafana_plugin/tests/test_grafana_api_client.py index 4dcb70c1..65e83504 100644 --- a/engine/apps/grafana_plugin/tests/test_grafana_api_client.py +++ b/engine/apps/grafana_plugin/tests/test_grafana_api_client.py @@ -9,19 +9,12 @@ API_TOKEN = "dfjkfdjkfd" class TestGetUsersPermissions: - def test_rbac_is_not_enabled_for_org(self): - api_client = GrafanaAPIClient(API_URL, API_TOKEN) - permissions = api_client.get_users_permissions(False) - assert len(permissions.keys()) == 0 - + @pytest.mark.parametrize("api_response_data", [None, []]) @patch("apps.grafana_plugin.helpers.client.GrafanaAPIClient.api_get") - def test_api_call_returns_none(self, mocked_grafana_api_client_api_get): - mocked_grafana_api_client_api_get.return_value = (None, "dfkjfdkj") - + def test_api_call_returns_none_or_list(self, mocked_grafana_api_client_api_get, api_response_data): + mocked_grafana_api_client_api_get.return_value = (api_response_data, "dfkjfdkj") api_client = GrafanaAPIClient(API_URL, API_TOKEN) - - permissions = api_client.get_users_permissions(True) - assert len(permissions.keys()) == 0 + assert api_client.get_users_permissions() is None @patch("apps.grafana_plugin.helpers.client.GrafanaAPIClient.api_get") def test_it_properly_transforms_the_data(self, mocked_grafana_api_client_api_get): @@ -32,7 +25,7 @@ class TestGetUsersPermissions: api_client = GrafanaAPIClient(API_URL, API_TOKEN) - permissions = api_client.get_users_permissions(True) + permissions = api_client.get_users_permissions() assert permissions == { "1": [ {"action": "grafana-oncall-app.alert-groups:read"}, @@ -41,6 +34,85 @@ class TestGetUsersPermissions: } +class TestGetUsers: + @pytest.mark.parametrize( + "rbac_is_enabled,api_get_return_value,get_users_permissions_return_value,expected", + [ + # RBAC is enabled - permissions are returned + ( + True, + [ + {"userId": 1, "foo": "bar"}, + {"userId": 2, "foo": "baz"}, + ], + { + "1": [ + {"action": "grafana-oncall-app.alert-groups:read"}, + {"action": "grafana-oncall-app.alert-groups:write"}, + ], + }, + [ + { + "userId": 1, + "foo": "bar", + "permissions": [ + {"action": "grafana-oncall-app.alert-groups:read"}, + {"action": "grafana-oncall-app.alert-groups:write"}, + ], + }, + { + "userId": 2, + "foo": "baz", + "permissions": [], + }, + ], + ), + # RBAC is enabled - permissions endpoint returns no permissions (ex. HTTP 500) + ( + True, + [ + {"userId": 1, "foo": "bar"}, + {"userId": 2, "foo": "baz"}, + ], + None, + [], + ), + # RBAC is not enabled - we don't fetch permissions (hence don't care about its response) + ( + False, + [ + {"userId": 1, "foo": "bar"}, + {"userId": 2, "foo": "baz"}, + ], + None, + [ + {"userId": 1, "foo": "bar", "permissions": []}, + { + "userId": 2, + "foo": "baz", + "permissions": [], + }, + ], + ), + ], + ) + @patch("apps.grafana_plugin.helpers.client.GrafanaAPIClient.api_get") + @patch("apps.grafana_plugin.helpers.client.GrafanaAPIClient.get_users_permissions") + def test_it_returns_none_if_permissions_call_returns_none( + self, + mocked_grafana_api_client_get_users_permissions, + mocked_grafana_api_client_api_get, + rbac_is_enabled, + api_get_return_value, + get_users_permissions_return_value, + expected, + ): + mocked_grafana_api_client_api_get.return_value = (api_get_return_value, "dfjkfdjkfd") + mocked_grafana_api_client_get_users_permissions.return_value = get_users_permissions_return_value + api_client = GrafanaAPIClient(API_URL, API_TOKEN) + assert api_client.get_users(rbac_is_enabled) == expected + + class TestIsRbacEnabledForOrganization: @pytest.mark.parametrize( "api_response_connected,expected", diff --git a/engine/apps/user_management/models/organization.py b/engine/apps/user_management/models/organization.py index 8564a195..5db1cd0c 100644 --- a/engine/apps/user_management/models/organization.py +++ b/engine/apps/user_management/models/organization.py @@ -1,4 +1,5 @@ import logging +import math import typing import uuid from urllib.parse import urljoin @@ -344,6 +345,20 @@ class Organization(MaintainableObject): .distinct() ) + def should_be_considered_for_rbac_permissioning(self) -> bool: + """ + this is sort of a hacky workaround to address a cloud issue we introduced with the accessControlOncall + feature flag. The flag is technically enabled for all stacks, but the way in which OnCall used to be + reading it (via GCOM config.feature_flags for the stack) made it such that RBAC wasn't actually being + enabled for most stacks from the oncall backend perspective. Once we change things to start HEADing + the permissions search endpoint, this will effectively turn on RBAC for all orgs.. soo instead lets + slowly turn it on via the logic here + """ + # if rbac permissions are already enabled for the org, they're "grandfathered" in + if self.is_rbac_permissions_enabled: + return True + return self.id <= math.floor(Organization.objects.last().id * settings.CLOUD_RBAC_ROLLOUT_PERCENTAGE) + @property def web_link(self): return urljoin(self.grafana_url, "a/grafana-oncall-app/") diff --git a/engine/apps/user_management/sync.py b/engine/apps/user_management/sync.py index a106a193..e6c6008d 100644 --- a/engine/apps/user_management/sync.py +++ b/engine/apps/user_management/sync.py @@ -28,16 +28,28 @@ def sync_organization(organization: Organization) -> None: def _sync_organization(organization: Organization) -> None: grafana_api_client = GrafanaAPIClient(api_url=organization.grafana_url, api_token=organization.api_token) + rbac_is_enabled = organization.is_rbac_permissions_enabled # NOTE: checking whether or not RBAC is enabled depends on whether we are dealing with an open-source or cloud - # stack. For Cloud we should make a call to the GCOM API, using an admin API token, and get the list of - # feature_toggles enabled for the stack. For open-source, simply make a HEAD request to the grafana instance's API - # and consider RBAC enabled if the list RBAC permissions endpoint returns 200. We cannot simply rely on the HEAD - # call in cloud because if an instance is not active, the grafana gateway will still return 200 for the - # HEAD request. + # stack. For open-source, simply make a HEAD request to the grafana instance's API and consider RBAC enabled if + # the list RBAC permissions endpoint returns 200. + # + # For cloud, we need to check the stack's status first. If the stack is active, we can make the same HEAD request + # to the grafana instance's API. If the stack is not active, we will simply rely on the org's previous state of + # org.is_rbac_permissions_enabled if settings.LICENSE == settings.CLOUD_LICENSE_NAME: + # We cannot simply rely on the HEAD call in cloud because if an instance is not active + # the grafana gateway will still return 200 for the HEAD request. + stack_id = organization.stack_id gcom_client = GcomAPIClient(settings.GRAFANA_COM_ADMIN_API_TOKEN) - rbac_is_enabled = gcom_client.is_rbac_enabled_for_stack(organization.stack_id) + + if not organization.should_be_considered_for_rbac_permissioning(): + rbac_is_enabled = False + elif gcom_client.is_stack_active(stack_id): + # the stack MUST be active for this check.. if it is in any other state + # the Grafana API risks returning an HTTP 200 but the actual permissions data that is + # synced later on will be empty (and we'd erase all RBAC permissions stored in OnCall) + rbac_is_enabled = grafana_api_client.is_rbac_enabled_for_organization() else: rbac_is_enabled = grafana_api_client.is_rbac_enabled_for_organization() diff --git a/engine/apps/user_management/tests/test_organization.py b/engine/apps/user_management/tests/test_organization.py index 58636f88..90fb73a6 100644 --- a/engine/apps/user_management/tests/test_organization.py +++ b/engine/apps/user_management/tests/test_organization.py @@ -278,3 +278,41 @@ def test_get_notifiable_direct_paging_integrations( make_channel_filter(arc, is_default=False) notifiable_direct_paging_integrations = _assert(org, arc) assert notifiable_direct_paging_integrations.count() == 1 + + +@pytest.mark.parametrize( + "is_rbac_permissions_enabled_initially,rollout_percentage,expected", + [ + # env var is not set, no orgs have is_rbac_permissions_enabled set + (False, 0.0, False), + # env var is not set but is_rbac_permissions_enabled is already set for all orgs + (True, 0.0, True), + # env var is set, only some orgs should be considered + (False, 0.5, "partial"), + (False, 1.0, True), + ], +) +@pytest.mark.django_db +def test_should_be_considered_for_rbac_permissioning( + make_organization, + settings, + is_rbac_permissions_enabled_initially, + rollout_percentage, + expected, +): + NUM_ORGS = 5 + settings.CLOUD_RBAC_ROLLOUT_PERCENTAGE = rollout_percentage + + orgs = [ + make_organization(is_rbac_permissions_enabled=is_rbac_permissions_enabled_initially) for _ in range(NUM_ORGS) + ] + + assert all(org.is_rbac_permissions_enabled is is_rbac_permissions_enabled_initially for org in orgs) + + if expected == "partial": + assert all( + org.should_be_considered_for_rbac_permissioning() == (org.id <= NUM_ORGS * rollout_percentage) + for org in orgs + ) + else: + assert all(org.should_be_considered_for_rbac_permissioning() is expected for org in orgs) diff --git a/engine/apps/user_management/tests/test_sync.py b/engine/apps/user_management/tests/test_sync.py index f24edaac..131893ab 100644 --- a/engine/apps/user_management/tests/test_sync.py +++ b/engine/apps/user_management/tests/test_sync.py @@ -1,3 +1,4 @@ +from contextlib import contextmanager from dataclasses import dataclass from typing import Optional from unittest.mock import patch @@ -20,6 +21,64 @@ from apps.user_management.sync import ( MOCK_GRAFANA_INCIDENT_BACKEND_URL = "https://grafana-incident.test" +@contextmanager +def patched_grafana_api_client(organization, is_rbac_enabled_for_organization=False): + GRAFANA_INCIDENT_PLUGIN_BACKEND_URL_KEY = "backendUrl" + + with patch("apps.user_management.sync.GrafanaAPIClient") as mock_grafana_api_client: + mock_grafana_api_client.GRAFANA_INCIDENT_PLUGIN_BACKEND_URL_KEY = GRAFANA_INCIDENT_PLUGIN_BACKEND_URL_KEY + + mock_client_instance = mock_grafana_api_client.return_value + + mock_client_instance.get_users.return_value = [ + { + "userId": 1, + "email": "test@test.test", + "name": "Test", + "login": "test", + "role": "admin", + "avatarUrl": "test.test/test", + "permissions": [], + }, + ] + mock_client_instance.get_teams.return_value = ( + { + "totalCount": 1, + "teams": ( + { + "id": 1, + "name": "Test", + "email": "test@test.test", + "avatarUrl": "test.test/test", + }, + ), + }, + None, + ) + mock_client_instance.get_team_members.return_value = ( + [ + { + "orgId": organization.org_id, + "teamId": 1, + "userId": 1, + }, + ], + None, + ) + mock_client_instance.get_grafana_incident_plugin_settings.return_value = ( + {"enabled": True, "jsonData": {GRAFANA_INCIDENT_PLUGIN_BACKEND_URL_KEY: MOCK_GRAFANA_INCIDENT_BACKEND_URL}}, + None, + ) + mock_client_instance.get_grafana_labels_plugin_settings.return_value = ( + {"enabled": True, "jsonData": {}}, + None, + ) + mock_client_instance.check_token.return_value = (None, {"connected": True}) + mock_client_instance.is_rbac_enabled_for_organization.return_value = is_rbac_enabled_for_organization + + yield mock_grafana_api_client + + @pytest.mark.django_db def test_sync_users_for_organization(make_organization, make_user_for_organization): organization = make_organization(grafana_url="https://test.test") @@ -184,79 +243,11 @@ def test_sync_users_for_team(make_organization, make_user_for_organization, make @pytest.mark.django_db -@patch.object(GrafanaAPIClient, "is_rbac_enabled_for_organization", return_value=False) -@patch.object( - GrafanaAPIClient, - "get_users", - return_value=[ - { - "userId": 1, - "email": "test@test.test", - "name": "Test", - "login": "test", - "role": "admin", - "avatarUrl": "test.test/test", - "permissions": [], - } - ], -) -@patch.object( - GrafanaAPIClient, - "get_teams", - return_value=( - { - "totalCount": 1, - "teams": ( - { - "id": 1, - "name": "Test", - "email": "test@test.test", - "avatarUrl": "test.test/test", - }, - ), - }, - None, - ), -) -@patch.object(GrafanaAPIClient, "check_token", return_value=(None, {"connected": True})) -@patch.object(GrafanaAPIClient, "get_grafana_incident_plugin_settings") -@patch.object(GrafanaAPIClient, "get_grafana_labels_plugin_settings") @patch("apps.user_management.sync.org_sync_signal") -def test_sync_organization( - mocked_org_sync_signal, - mock_get_grafana_labels_plugin_settings, - mock_get_grafana_incident_plugin_settings, - _mock_check_token, - _mock_get_teams, - _mock_get_users, - _mock_is_rbac_enabled_for_organization, - make_organization, -): - # Set optimistic responses from grafana api. - # All cases are tested properly in test_sync_grafana_incident_plugin/test_sync_grafana_labels_plugin - mock_get_grafana_incident_plugin_settings.return_value = ( - { - "enabled": True, - "jsonData": {"backendUrl": MOCK_GRAFANA_INCIDENT_BACKEND_URL}, - }, - None, - ) - mock_get_grafana_labels_plugin_settings.return_value = ({"enabled": True, "jsonData": {}}, None) - +def test_sync_organization(mocked_org_sync_signal, make_organization): organization = make_organization() - api_members_response = ( - [ - { - "orgId": organization.org_id, - "teamId": 1, - "userId": 1, - } - ], - None, - ) - - with patch.object(GrafanaAPIClient, "get_team_members", return_value=api_members_response): + with patched_grafana_api_client(organization): sync_organization(organization) # check that users are populated @@ -283,154 +274,69 @@ def test_sync_organization( mocked_org_sync_signal.send.assert_called_once_with(sender=None, organization=organization) -@pytest.mark.parametrize("grafana_api_response", [False, True]) +@pytest.mark.parametrize("is_rbac_enabled_for_organization", [False, True]) @override_settings(LICENSE=settings.OPEN_SOURCE_LICENSE_NAME) @pytest.mark.django_db -def test_sync_organization_is_rbac_permissions_enabled_open_source(make_organization, grafana_api_response): +def test_sync_organization_is_rbac_permissions_enabled_open_source(make_organization, is_rbac_enabled_for_organization): organization = make_organization() - api_users_response = ( - { - "userId": 1, - "email": "test@test.test", - "name": "Test", - "login": "test", - "role": "admin", - "avatarUrl": "test.test/test", - "permissions": [], - }, - ) - - api_teams_response = { - "totalCount": 1, - "teams": ( - { - "id": 1, - "name": "Test", - "email": "test@test.test", - "avatarUrl": "test.test/test", - }, - ), - } - - api_members_response = ( - { - "orgId": organization.org_id, - "teamId": 1, - "userId": 1, - }, - ) - api_check_token_call_status = {"connected": True} - - with patch.object(GrafanaAPIClient, "is_rbac_enabled_for_organization", return_value=grafana_api_response): - with patch.object(GrafanaAPIClient, "get_users", return_value=api_users_response): - with patch.object(GrafanaAPIClient, "get_teams", return_value=(api_teams_response, None)): - with patch.object(GrafanaAPIClient, "get_team_members", return_value=(api_members_response, None)): - with patch.object( - GrafanaAPIClient, "check_token", return_value=(None, api_check_token_call_status) - ): - with patch.object( - GrafanaAPIClient, - "get_grafana_incident_plugin_settings", - return_value=( - {"enabled": True, "jsonData": {"backendUrl": MOCK_GRAFANA_INCIDENT_BACKEND_URL}}, - None, - ), - ): - with patch.object( - GrafanaAPIClient, - "get_grafana_labels_plugin_settings", - return_value=( - {"enabled": True, "jsonData": {}}, - None, - ), - ): - sync_organization(organization) + with patched_grafana_api_client(organization, is_rbac_enabled_for_organization): + sync_organization(organization) organization.refresh_from_db() - assert organization.is_rbac_permissions_enabled == grafana_api_response + assert organization.is_rbac_permissions_enabled == is_rbac_enabled_for_organization -@pytest.mark.parametrize("gcom_api_response", [False, True]) +@pytest.mark.parametrize( + "should_be_considered_for_rbac_permissioning,gcom_api_response,grafana_api_response,org_initial_value,org_is_rbac_permissions_enabled_expected_value", + [ + # org shouldn't be considered for RBAC permissioning + (False, True, True, True, False), + # org should be considered for RBAC permissioning + # + # stack is in an inactive state, rely on org's previous state of is_rbac_permissions_enabled + (True, False, False, False, False), + (True, False, False, True, True), + # stack is active, Grafana API tells us RBAC is not enabled + (True, True, False, True, False), + # stack is active, Grafana API tells us RBAC is enabled + (True, True, True, False, True), + ], +) +@patch("apps.user_management.models.Organization.should_be_considered_for_rbac_permissioning") @patch("apps.user_management.sync.GcomAPIClient") -@patch("common.utils.cache") @override_settings(LICENSE=settings.CLOUD_LICENSE_NAME) -@override_settings(GRAFANA_COM_ADMIN_API_TOKEN="mockedToken") @pytest.mark.django_db def test_sync_organization_is_rbac_permissions_enabled_cloud( - mock_cache, mocked_gcom_client, make_organization, gcom_api_response + mock_gcom_client, + mock_should_be_considered_for_rbac_permissioning, + make_organization, + should_be_considered_for_rbac_permissioning, + gcom_api_response, + grafana_api_response, + org_initial_value, + org_is_rbac_permissions_enabled_expected_value, ): + mock_should_be_considered_for_rbac_permissioning.return_value = should_be_considered_for_rbac_permissioning + stack_id = 5 - organization = make_organization(stack_id=stack_id) + organization = make_organization(stack_id=stack_id, is_rbac_permissions_enabled=org_initial_value) + mock_gcom_client.return_value.is_stack_active.return_value = gcom_api_response - api_check_token_call_status = {"connected": True} + assert organization.is_rbac_permissions_enabled == org_initial_value - mocked_gcom_client.return_value.is_rbac_enabled_for_stack.return_value = gcom_api_response - - api_users_response = ( - { - "userId": 1, - "email": "test@test.test", - "name": "Test", - "login": "test", - "role": "admin", - "avatarUrl": "test.test/test", - "permissions": [], - }, - ) - - api_teams_response = { - "totalCount": 1, - "teams": ( - { - "id": 1, - "name": "Test", - "email": "test@test.test", - "avatarUrl": "test.test/test", - }, - ), - } - - api_members_response = ( - { - "orgId": organization.org_id, - "teamId": 1, - "userId": 1, - }, - ) - - random_uuid = "random" - with patch("apps.user_management.sync.uuid.uuid4", return_value=random_uuid): - with patch.object(GrafanaAPIClient, "check_token", return_value=(None, api_check_token_call_status)): - with patch.object(GrafanaAPIClient, "get_users", return_value=api_users_response): - with patch.object(GrafanaAPIClient, "get_teams", return_value=(api_teams_response, None)): - with patch.object(GrafanaAPIClient, "get_team_members", return_value=(api_members_response, None)): - with patch.object( - GrafanaAPIClient, - "get_grafana_incident_plugin_settings", - return_value=( - {"enabled": True, "jsonData": {"backendUrl": MOCK_GRAFANA_INCIDENT_BACKEND_URL}}, - None, - ), - ): - with patch.object( - GrafanaAPIClient, - "get_grafana_labels_plugin_settings", - return_value=( - {"enabled": True, "jsonData": {}}, - None, - ), - ): - sync_organization(organization) + with patched_grafana_api_client(organization, grafana_api_response) as mock_grafana_api_client: + sync_organization(organization) organization.refresh_from_db() - # lock is set and released - mock_cache.add.assert_called_once_with(f"sync-organization-lock-{organization.id}", random_uuid, 60 * 10) - mock_cache.delete.assert_called_once_with(f"sync-organization-lock-{organization.id}") - assert mocked_gcom_client.return_value.called_once_with("mockedToken") - assert mocked_gcom_client.return_value.is_rbac_enabled_for_stack.called_once_with(stack_id) - assert organization.is_rbac_permissions_enabled == gcom_api_response + assert organization.is_rbac_permissions_enabled == org_is_rbac_permissions_enabled_expected_value + + if should_be_considered_for_rbac_permissioning: + mock_gcom_client.return_value.is_stack_active.assert_called_once_with(stack_id) + + if gcom_api_response: + mock_grafana_api_client.return_value.is_rbac_enabled_for_organization.assert_called_once_with() @pytest.mark.django_db @@ -468,13 +374,7 @@ def test_duplicate_user_ids(make_organization, make_user_for_organization): @pytest.mark.django_db -@pytest.mark.parametrize( - "is_deleted", - [ - True, - False, - ], -) +@pytest.mark.parametrize("is_deleted", [True, False]) def test_cleanup_organization_deleted(make_organization, is_deleted): organization = make_organization(gcom_token="TEST_GCOM_TOKEN") @@ -501,19 +401,30 @@ def test_organization_not_deleted(make_organization): @pytest.mark.django_db -def test_sync_organization_lock(make_organization): +@pytest.mark.parametrize("task_lock_acquired", [True, False]) +@patch("apps.user_management.sync.task_lock") +@patch("apps.user_management.sync.uuid.uuid4", return_value="random") +def test_sync_organization_lock( + _mock_uuid4, + mock_task_lock, + make_organization, + task_lock_acquired, +): organization = make_organization() - random_uuid = "random" - with patch("apps.user_management.sync.GrafanaAPIClient") as mock_client: - with patch("apps.user_management.sync.uuid.uuid4", return_value=random_uuid): - with patch("apps.user_management.sync.task_lock") as mock_task_lock: - # lock couldn't be acquired - mock_task_lock.return_value.__enter__.return_value = False - sync_organization(organization) + lock_cache_key = f"sync-organization-lock-{organization.id}" + mock_task_lock.return_value.__enter__.return_value = task_lock_acquired - mock_task_lock.assert_called_once_with(f"sync-organization-lock-{organization.id}", random_uuid) - assert not mock_client.called + with patched_grafana_api_client(organization) as mock_grafana_api_client: + sync_organization(organization) + + mock_task_lock.assert_called_once_with(lock_cache_key, "random") + + if task_lock_acquired: + mock_grafana_api_client.assert_called_once() + else: + # task lock could not be acquired + mock_grafana_api_client.assert_not_called() @dataclass diff --git a/engine/common/utils.py b/engine/common/utils.py index 05141120..b902fe21 100644 --- a/engine/common/utils.py +++ b/engine/common/utils.py @@ -139,6 +139,16 @@ def getenv_integer(variable_name: str, default: int) -> int: return default +def getenv_float(variable_name: str, default: float) -> float: + value = os.environ.get(variable_name) + if value is None: + return default + try: + return float(value) + except ValueError: + return default + + def getenv_list(variable_name: str, default: list) -> list: value = os.environ.get(variable_name) if value is None: diff --git a/engine/conftest.py b/engine/conftest.py index 9bd2e4eb..899679fd 100644 --- a/engine/conftest.py +++ b/engine/conftest.py @@ -230,9 +230,9 @@ def mock_is_labels_feature_enabled_for_org(settings): @pytest.fixture def make_organization(): def _make_organization(**kwargs): - return OrganizationFactory( - **kwargs, is_rbac_permissions_enabled=IS_RBAC_ENABLED, is_grafana_labels_enabled=True - ) + if "is_rbac_permissions_enabled" not in kwargs: + kwargs["is_rbac_permissions_enabled"] = IS_RBAC_ENABLED + return OrganizationFactory(**kwargs, is_grafana_labels_enabled=True) return _make_organization diff --git a/engine/settings/base.py b/engine/settings/base.py index ecdac1a1..48c9e16b 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -7,7 +7,7 @@ from random import randrange from celery.schedules import crontab from firebase_admin import credentials, initialize_app -from common.utils import getenv_boolean, getenv_integer, getenv_list +from common.utils import getenv_boolean, getenv_float, getenv_integer, getenv_list VERSION = "dev-oss" SEND_ANONYMOUS_USAGE_STATS = getenv_boolean("SEND_ANONYMOUS_USAGE_STATS", default=True) @@ -914,3 +914,5 @@ ZVONOK_VERIFICATION_TEMPLATE = os.getenv("ZVONOK_VERIFICATION_TEMPLATE", None) DETACHED_INTEGRATIONS_SERVER = getenv_boolean("DETACHED_INTEGRATIONS_SERVER", default=False) ACKNOWLEDGE_REMINDER_TASK_EXPIRY_DAYS = os.environ.get("ACKNOWLEDGE_REMINDER_TASK_EXPIRY_DAYS", default=14) + +CLOUD_RBAC_ROLLOUT_PERCENTAGE = getenv_float("CLOUD_RBAC_ROLLOUT_PERCENTAGE", default=0.0)