Skip to content

Commit a25d44d

Browse files
authored
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] <!-- *Note*: If you want the issue to be auto-closed once the PR is merged, change "Related to" to "Closes" in the line above. If you have more than one GitHub issue that this PR closes, be sure to preface each issue link with a [closing keyword](https://docs.github.com/en/get-started/writing-on-github/working-with-advanced-formatting/using-keywords-in-issues-and-pull-requests#linking-a-pull-request-to-an-issue). This ensures that the issue(s) are auto-closed once the PR has been merged. --> ## 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.
1 parent a577030 commit a25d44d

File tree

4 files changed

+16
-14
lines changed

4 files changed

+16
-14
lines changed

engine/apps/grafana_plugin/helpers/client.py

+10
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,8 @@ class GrafanaAPIClient(APIClient):
166166

167167
USER_PERMISSION_ENDPOINT = f"api/access-control/users/permissions/search?actionPrefix={ACTION_PREFIX}"
168168

169+
MIN_GRAFANA_TOKEN_LENGTH = 16
170+
169171
class Types:
170172
class _BaseGrafanaAPIResponse(typing.TypedDict):
171173
totalCount: int
@@ -330,6 +332,14 @@ def get_service_account_token_permissions(self) -> APIClientResponse[typing.Dict
330332
def sync(self) -> APIClientResponse:
331333
return self.api_post("api/plugins/grafana-oncall-app/resources/plugin/sync")
332334

335+
@staticmethod
336+
def validate_grafana_token_format(grafana_token: str) -> bool:
337+
if not grafana_token or not isinstance(grafana_token, str):
338+
return False
339+
if len(grafana_token) < GrafanaAPIClient.MIN_GRAFANA_TOKEN_LENGTH:
340+
return False
341+
return True
342+
333343

334344
class GcomAPIClient(APIClient):
335345
ACTIVE_INSTANCE_QUERY = "instances?status=active"

engine/apps/grafana_plugin/helpers/gcom.py

+2-11
Original file line numberDiff line numberDiff line change
@@ -6,28 +6,19 @@
66

77
from apps.auth_token.exceptions import InvalidToken
88
from apps.auth_token.models import PluginAuthToken
9-
from apps.grafana_plugin.helpers import GcomAPIClient
9+
from apps.grafana_plugin.helpers import GcomAPIClient, GrafanaAPIClient
1010
from apps.user_management.models import Organization
1111

1212
logger = logging.getLogger(__name__)
1313
logger.setLevel(logging.DEBUG)
1414
GCOM_TOKEN_CHECK_PERIOD = timezone.timedelta(minutes=60)
15-
MIN_GRAFANA_TOKEN_LENGTH = 16
1615

1716

1817
class GcomToken:
1918
def __init__(self, organization):
2019
self.organization = organization
2120

2221

23-
def _validate_grafana_token_format(grafana_token: str) -> bool:
24-
if not grafana_token or not isinstance(grafana_token, str):
25-
return False
26-
if len(grafana_token) < MIN_GRAFANA_TOKEN_LENGTH:
27-
return False
28-
return True
29-
30-
3122
def check_gcom_permission(token_string: str, context) -> GcomToken:
3223
"""
3324
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:
5445
if not instance_info or str(instance_info["orgId"]) != org_id:
5546
raise InvalidToken
5647

57-
grafana_token_format_is_valid = _validate_grafana_token_format(grafana_token)
48+
grafana_token_format_is_valid = GrafanaAPIClient.validate_grafana_token_format(grafana_token)
5849

5950
if not organization:
6051
from apps.base.models import DynamicSetting

engine/apps/grafana_plugin/tasks/sync_v2.py

+2-2
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ def sync_organizations_v2(org_ids=None):
4141
orgs_per_second = math.ceil(len(organization_qs) / SYNC_PERIOD.seconds)
4242
logger.info(f"Syncing {len(organization_qs)} organizations @ {orgs_per_second} per 1s pause")
4343
for idx, org in enumerate(organization_qs):
44-
if org.api_token:
44+
if GrafanaAPIClient.validate_grafana_token_format(org.api_token):
4545
client = GrafanaAPIClient(api_url=org.grafana_url, api_token=org.api_token)
4646
_, status = client.sync()
4747
if status["status_code"] != 200:
@@ -52,6 +52,6 @@ def sync_organizations_v2(org_ids=None):
5252
logger.info(f"Sleep 1s after {idx + 1} organizations processed")
5353
sleep(1)
5454
else:
55-
logger.info(f"Skipping stack_slug={org.stack_slug}, api_token is not set")
55+
logger.info(f"Skipping stack_slug={org.stack_slug}, api_token format is invalid or not set")
5656
else:
5757
logger.info(f"Issuing sync requests already in progress lock_id={lock_id}, check slow outgoing requests")

engine/apps/grafana_plugin/tests/test_sync_v2.py

+2-1
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,8 @@ def test_invalid_auth(make_organization_and_user_with_plugin_token, make_user_au
5151
"api_token, sync_called",
5252
[
5353
("", False),
54-
("abc", True),
54+
("abc", False),
55+
("glsa_abcdefghijklmnopqrstuvwxyz", True),
5556
],
5657
)
5758
@pytest.mark.django_db

0 commit comments

Comments
 (0)