From a25d44da1ae26715bc7b865a027fdc6701e36a76 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Fri, 23 Aug 2024 13:52:53 -0600 Subject: [PATCH] Move validate_grafana_token_format to common location, use in sync_v2 (#4919) # What this PR does Moves validate_grafana_token_format to GrafanaAPIClient, use it in sync_v2 to improve logging and skip requests that would not work. ## Which issue(s) this PR closes Related to [issue link here] ## Checklist - [x] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] Added the relevant release notes label (see labels prefixed w/ `release:`). These labels dictate how your PR will show up in the autogenerated release notes. --- engine/apps/grafana_plugin/helpers/client.py | 10 ++++++++++ engine/apps/grafana_plugin/helpers/gcom.py | 13 ++----------- engine/apps/grafana_plugin/tasks/sync_v2.py | 4 ++-- engine/apps/grafana_plugin/tests/test_sync_v2.py | 3 ++- 4 files changed, 16 insertions(+), 14 deletions(-) diff --git a/engine/apps/grafana_plugin/helpers/client.py b/engine/apps/grafana_plugin/helpers/client.py index 3d54cd0d..4ed9c346 100644 --- a/engine/apps/grafana_plugin/helpers/client.py +++ b/engine/apps/grafana_plugin/helpers/client.py @@ -166,6 +166,8 @@ class GrafanaAPIClient(APIClient): USER_PERMISSION_ENDPOINT = f"api/access-control/users/permissions/search?actionPrefix={ACTION_PREFIX}" + MIN_GRAFANA_TOKEN_LENGTH = 16 + class Types: class _BaseGrafanaAPIResponse(typing.TypedDict): totalCount: int @@ -330,6 +332,14 @@ class GrafanaAPIClient(APIClient): def sync(self) -> APIClientResponse: return self.api_post("api/plugins/grafana-oncall-app/resources/plugin/sync") + @staticmethod + def validate_grafana_token_format(grafana_token: str) -> bool: + if not grafana_token or not isinstance(grafana_token, str): + return False + if len(grafana_token) < GrafanaAPIClient.MIN_GRAFANA_TOKEN_LENGTH: + return False + return True + class GcomAPIClient(APIClient): ACTIVE_INSTANCE_QUERY = "instances?status=active" diff --git a/engine/apps/grafana_plugin/helpers/gcom.py b/engine/apps/grafana_plugin/helpers/gcom.py index e327c7f7..e83ab969 100644 --- a/engine/apps/grafana_plugin/helpers/gcom.py +++ b/engine/apps/grafana_plugin/helpers/gcom.py @@ -6,13 +6,12 @@ from django.utils import timezone from apps.auth_token.exceptions import InvalidToken from apps.auth_token.models import PluginAuthToken -from apps.grafana_plugin.helpers import GcomAPIClient +from apps.grafana_plugin.helpers import GcomAPIClient, GrafanaAPIClient from apps.user_management.models import Organization logger = logging.getLogger(__name__) logger.setLevel(logging.DEBUG) GCOM_TOKEN_CHECK_PERIOD = timezone.timedelta(minutes=60) -MIN_GRAFANA_TOKEN_LENGTH = 16 class GcomToken: @@ -20,14 +19,6 @@ class GcomToken: self.organization = organization -def _validate_grafana_token_format(grafana_token: str) -> bool: - if not grafana_token or not isinstance(grafana_token, str): - return False - if len(grafana_token) < MIN_GRAFANA_TOKEN_LENGTH: - return False - return True - - def check_gcom_permission(token_string: str, context) -> GcomToken: """ Verify that request from plugin is valid. Check it and synchronize the organization details @@ -54,7 +45,7 @@ def check_gcom_permission(token_string: str, context) -> GcomToken: if not instance_info or str(instance_info["orgId"]) != org_id: raise InvalidToken - grafana_token_format_is_valid = _validate_grafana_token_format(grafana_token) + grafana_token_format_is_valid = GrafanaAPIClient.validate_grafana_token_format(grafana_token) if not organization: from apps.base.models import DynamicSetting diff --git a/engine/apps/grafana_plugin/tasks/sync_v2.py b/engine/apps/grafana_plugin/tasks/sync_v2.py index 7f2d9bcf..0f351e30 100644 --- a/engine/apps/grafana_plugin/tasks/sync_v2.py +++ b/engine/apps/grafana_plugin/tasks/sync_v2.py @@ -41,7 +41,7 @@ def sync_organizations_v2(org_ids=None): orgs_per_second = math.ceil(len(organization_qs) / SYNC_PERIOD.seconds) logger.info(f"Syncing {len(organization_qs)} organizations @ {orgs_per_second} per 1s pause") for idx, org in enumerate(organization_qs): - if org.api_token: + if GrafanaAPIClient.validate_grafana_token_format(org.api_token): client = GrafanaAPIClient(api_url=org.grafana_url, api_token=org.api_token) _, status = client.sync() if status["status_code"] != 200: @@ -52,6 +52,6 @@ def sync_organizations_v2(org_ids=None): logger.info(f"Sleep 1s after {idx + 1} organizations processed") sleep(1) else: - logger.info(f"Skipping stack_slug={org.stack_slug}, api_token is not set") + logger.info(f"Skipping stack_slug={org.stack_slug}, api_token format is invalid or not set") else: logger.info(f"Issuing sync requests already in progress lock_id={lock_id}, check slow outgoing requests") diff --git a/engine/apps/grafana_plugin/tests/test_sync_v2.py b/engine/apps/grafana_plugin/tests/test_sync_v2.py index 0c118da4..5fd67fdd 100644 --- a/engine/apps/grafana_plugin/tests/test_sync_v2.py +++ b/engine/apps/grafana_plugin/tests/test_sync_v2.py @@ -51,7 +51,8 @@ def test_invalid_auth(make_organization_and_user_with_plugin_token, make_user_au "api_token, sync_called", [ ("", False), - ("abc", True), + ("abc", False), + ("glsa_abcdefghijklmnopqrstuvwxyz", True), ], ) @pytest.mark.django_db