From 9628bdc51f10320d54bda924b6e2c1e035788e20 Mon Sep 17 00:00:00 2001 From: Innokentii Konstantinov Date: Wed, 22 Nov 2023 19:17:41 +0800 Subject: [PATCH] Webhook labels (#3383) This PR add labels for webhooks. 1. Make webhook "labelable" with ability to filter by labels. 2. Add labels to the webhook payload. It contain new field webhook with it's name, id and labels. Field integration and alert_group has a corresponding label field as well. See example of a new payload below: ``` { "event": { "type": "escalation" }, "user": null, "alert_group": { "id": "IRFN6ZD31N31B", "integration_id": "CTWM7U4A2QG97", "route_id": "RUE7U7Z46SKGY", "alerts_count": 1, "state": "firing", "created_at": "2023-11-22T08:54:55.178243Z", "resolved_at": null, "acknowledged_at": null, "title": "Incident", "permalinks": { "slack": null, "telegram": null, "web": "http://grafana:3000/a/grafana-oncall-app/alert-groups/IRFN6ZD31N31B" }, "labels": { "severity": "critical" } }, "alert_group_id": "IRFN6ZD31N31B", "alert_payload": { "message": "This alert was sent by user for demonstration purposes" }, "integration": { "id": "CTWM7U4A2QG97", "type": "webhook", "name": "hi - Webhook", "team": null, "labels": { "hello": "world", "severity": "critical" } }, "notified_users": [], "users_to_be_notified": [], "webhook": { "id": "WHAXK4BTC7TAEQ", "name": "test", "labels": { "hello": "kesha" } } } ``` I feel that there is an opportunity to make code cleaner - remove all label logic from serializers, views and utils to models or dedicated LabelerService and introduce Labelable interface with something like label_verbal, update_labels methods. However, I don't want to tie webhook labels with a refactoring. --------- Co-authored-by: Dominik --- Tiltfile | 2 +- engine/apps/api/label_filtering.py | 15 ++ engine/apps/api/serializers/webhook.py | 20 +- .../api/tests/test_alert_receive_channel.py | 2 - engine/apps/api/tests/test_labels.py | 9 - engine/apps/api/tests/test_webhook_presets.py | 1 + engine/apps/api/tests/test_webhooks.py | 187 ++++++++++++++++++ engine/apps/api/views/alert_group.py | 17 +- .../apps/api/views/alert_receive_channel.py | 22 ++- engine/apps/api/views/labels.py | 33 +--- engine/apps/api/views/webhooks.py | 27 +++ .../migrations/0004_webhookassociatedlabel.py | 29 +++ engine/apps/labels/models.py | 21 ++ engine/apps/labels/tasks.py | 13 +- engine/apps/labels/tests/factories.py | 6 + engine/apps/labels/tests/test_labels.py | 13 +- engine/apps/labels/utils.py | 20 ++ engine/apps/webhooks/tasks/trigger_webhook.py | 2 +- .../webhooks/tests/test_trigger_webhook.py | 8 +- engine/apps/webhooks/utils.py | 9 +- engine/conftest.py | 10 + grafana-plugin/.eslintrc.js | 2 + grafana-plugin/package.json | 2 +- grafana-plugin/src/components/GForm/GForm.tsx | 11 +- .../LabelsTooltipBadge/LabelsTooltipBadge.tsx | 40 ++++ .../IntegrationForm/IntegrationForm.tsx | 11 +- .../src/containers/Labels/Labels.tsx | 16 +- .../OutgoingWebhookForm.config.tsx | 98 +++++---- .../OutgoingWebhookForm.tsx | 50 ++++- .../OutgoingWebhookForm.types.ts | 18 ++ .../RemoteFilters/RemoteFilters.helpers.ts | 5 +- .../RemoteFilters/RemoteFilters.tsx | 2 +- grafana-plugin/src/models/filters/filters.ts | 19 ++ .../outgoing_webhook.types.ts | 2 + .../src/pages/incidents/Incidents.tsx | 73 ++----- .../src/pages/integrations/Integrations.tsx | 69 +------ .../outgoing_webhooks/OutgoingWebhooks.tsx | 42 ++-- .../src/state/rootBaseStore/index.ts | 6 +- 38 files changed, 653 insertions(+), 279 deletions(-) create mode 100644 engine/apps/api/label_filtering.py create mode 100644 engine/apps/labels/migrations/0004_webhookassociatedlabel.py create mode 100644 grafana-plugin/src/components/LabelsTooltipBadge/LabelsTooltipBadge.tsx create mode 100644 grafana-plugin/src/containers/OutgoingWebhookForm/OutgoingWebhookForm.types.ts diff --git a/Tiltfile b/Tiltfile index 5a293c92..eb71dad6 100644 --- a/Tiltfile +++ b/Tiltfile @@ -58,7 +58,7 @@ local_resource( allow_parallel=True, ) -yaml = helm("helm/oncall", name=HELM_PREFIX, values=["./dev/helm-local.yml"]) +yaml = helm("helm/oncall", name=HELM_PREFIX, values=["./dev/helm-local.yml", "./dev/helm-local.dev.yml"]) k8s_yaml(yaml) diff --git a/engine/apps/api/label_filtering.py b/engine/apps/api/label_filtering.py new file mode 100644 index 00000000..10173c4f --- /dev/null +++ b/engine/apps/api/label_filtering.py @@ -0,0 +1,15 @@ +from typing import List, Tuple + + +def parse_label_query(label_query: List[str]) -> List[Tuple[str, str]]: + """ + parse_label_query returns list of key-value tuples from a list of "raw" labels – key-value pairs separated with ':'. + """ + kv_pairs = [] + for label in label_query: + label_data = label.split(":") + # Check if label_data is a valid key-value label pair]: ["key1", "value1"] + if len(label_data) != 2: + continue + kv_pairs.append((label_data[0], label_data[1])) + return kv_pairs diff --git a/engine/apps/api/serializers/webhook.py b/engine/apps/api/serializers/webhook.py index 832292ce..f49f7b8f 100644 --- a/engine/apps/api/serializers/webhook.py +++ b/engine/apps/api/serializers/webhook.py @@ -3,6 +3,7 @@ from collections import defaultdict from rest_framework import serializers from rest_framework.validators import UniqueTogetherValidator +from apps.api.serializers.labels import LabelsSerializerMixin from apps.webhooks.models import Webhook, WebhookResponse from apps.webhooks.models.webhook import PUBLIC_WEBHOOK_HTTP_METHODS, WEBHOOK_FIELD_PLACEHOLDER from apps.webhooks.presets.preset_options import WebhookPresetOptions @@ -27,7 +28,7 @@ class WebhookResponseSerializer(serializers.ModelSerializer): ] -class WebhookSerializer(serializers.ModelSerializer): +class WebhookSerializer(LabelsSerializerMixin, serializers.ModelSerializer): id = serializers.CharField(read_only=True, source="public_primary_key") organization = serializers.HiddenField(default=CurrentOrganizationDefault()) team = TeamPrimaryKeyRelatedField(allow_null=True, default=CurrentTeamDefault()) @@ -37,6 +38,8 @@ class WebhookSerializer(serializers.ModelSerializer): trigger_type = serializers.CharField(allow_null=True) trigger_type_name = serializers.SerializerMethodField() + PREFETCH_RELATED = ["labels", "labels__key", "labels__value"] + class Meta: model = Webhook fields = [ @@ -61,10 +64,25 @@ class WebhookSerializer(serializers.ModelSerializer): "last_response_log", "integration_filter", "preset", + "labels", ] validators = [UniqueTogetherValidator(queryset=Webhook.objects.all(), fields=["name", "organization"])] + def create(self, validated_data): + organization = self.context["request"].auth.organization + labels = validated_data.pop("labels", None) + + instance = super().create(validated_data) + self.update_labels_association_if_needed(labels, instance, organization) + return instance + + def update(self, instance, validated_data): + labels = validated_data.pop("labels", None) + organization = self.context["request"].auth.organization + self.update_labels_association_if_needed(labels, instance, organization) + return super().update(instance, validated_data) + def to_representation(self, instance): result = super().to_representation(instance) if instance.password: diff --git a/engine/apps/api/tests/test_alert_receive_channel.py b/engine/apps/api/tests/test_alert_receive_channel.py index e5c1ece0..989171db 100644 --- a/engine/apps/api/tests/test_alert_receive_channel.py +++ b/engine/apps/api/tests/test_alert_receive_channel.py @@ -1310,7 +1310,6 @@ def test_integration_filter_by_labels( def test_update_alert_receive_channel_labels( make_organization_and_user_with_plugin_token, make_alert_receive_channel, - make_integration_label_association, make_user_auth_headers, ): organization, user, token = make_organization_and_user_with_plugin_token() @@ -1353,7 +1352,6 @@ def test_update_alert_receive_channel_labels( def test_update_alert_receive_channel_labels_duplicate_key( make_organization_and_user_with_plugin_token, make_alert_receive_channel, - make_integration_label_association, make_user_auth_headers, ): organization, user, token = make_organization_and_user_with_plugin_token() diff --git a/engine/apps/api/tests/test_labels.py b/engine/apps/api/tests/test_labels.py index e47b7218..41e6be3f 100644 --- a/engine/apps/api/tests/test_labels.py +++ b/engine/apps/api/tests/test_labels.py @@ -43,7 +43,6 @@ def test_get_update_key_get( mocked_get_values, make_organization_and_user_with_plugin_token, make_user_auth_headers, - make_alert_receive_channel, ): organization, user, token = make_organization_and_user_with_plugin_token() client = APIClient() @@ -68,7 +67,6 @@ def test_get_update_key_put( mocked_rename_key, make_organization_and_user_with_plugin_token, make_user_auth_headers, - make_alert_receive_channel, ): organization, user, token = make_organization_and_user_with_plugin_token() client = APIClient() @@ -94,7 +92,6 @@ def test_add_value( mocked_add_value, make_organization_and_user_with_plugin_token, make_user_auth_headers, - make_alert_receive_channel, ): organization, user, token = make_organization_and_user_with_plugin_token() client = APIClient() @@ -120,7 +117,6 @@ def test_rename_value( mocked_rename_value, make_organization_and_user_with_plugin_token, make_user_auth_headers, - make_alert_receive_channel, ): organization, user, token = make_organization_and_user_with_plugin_token() client = APIClient() @@ -146,7 +142,6 @@ def test_get_value( mocked_get_value, make_organization_and_user_with_plugin_token, make_user_auth_headers, - make_alert_receive_channel, ): organization, user, token = make_organization_and_user_with_plugin_token() client = APIClient() @@ -171,7 +166,6 @@ def test_labels_create_label( mocked_create_label, make_organization_and_user_with_plugin_token, make_user_auth_headers, - make_alert_receive_channel, ): organization, user, token = make_organization_and_user_with_plugin_token() client = APIClient() @@ -189,7 +183,6 @@ def test_labels_create_label( def test_labels_feature_false( make_organization_and_user_with_plugin_token, make_user_auth_headers, - make_alert_receive_channel, settings, ): setattr(settings, "FEATURE_LABELS_ENABLED_FOR_ALL", False) @@ -239,7 +232,6 @@ def test_labels_feature_false( def test_labels_permissions_get_actions( make_organization_and_user_with_plugin_token, make_user_auth_headers, - make_alert_receive_channel, role, expected_status, ): @@ -274,7 +266,6 @@ def test_labels_permissions_get_actions( def test_labels_permissions_create_update_actions( make_organization_and_user_with_plugin_token, make_user_auth_headers, - make_alert_receive_channel, role, expected_status, ): diff --git a/engine/apps/api/tests/test_webhook_presets.py b/engine/apps/api/tests/test_webhook_presets.py index e87f7587..b2ce4df7 100644 --- a/engine/apps/api/tests/test_webhook_presets.py +++ b/engine/apps/api/tests/test_webhook_presets.py @@ -63,6 +63,7 @@ def test_create_webhook_from_preset( "http_method": "GET", "integration_filter": None, "is_webhook_enabled": True, + "labels": [], "is_legacy": False, "last_response_log": { "request_data": "", diff --git a/engine/apps/api/tests/test_webhooks.py b/engine/apps/api/tests/test_webhooks.py index f3162515..4c5bb35f 100644 --- a/engine/apps/api/tests/test_webhooks.py +++ b/engine/apps/api/tests/test_webhooks.py @@ -52,6 +52,7 @@ def test_get_list_webhooks(webhook_internal_api_setup, make_user_auth_headers): "http_method": "POST", "integration_filter": None, "is_webhook_enabled": True, + "labels": [], "is_legacy": False, "last_response_log": { "request_data": "", @@ -95,6 +96,7 @@ def test_get_detail_webhook(webhook_internal_api_setup, make_user_auth_headers): "http_method": "POST", "integration_filter": None, "is_webhook_enabled": True, + "labels": [], "is_legacy": False, "last_response_log": { "request_data": "", @@ -143,6 +145,7 @@ def test_create_webhook(webhook_internal_api_setup, make_user_auth_headers): "http_method": "POST", "integration_filter": None, "is_webhook_enabled": True, + "labels": [], "is_legacy": False, "last_response_log": { "request_data": "", @@ -203,6 +206,7 @@ def test_create_valid_templated_field(webhook_internal_api_setup, make_user_auth "http_method": "POST", "integration_filter": None, "is_webhook_enabled": True, + "labels": [], "is_legacy": False, "last_response_log": { "request_data": "", @@ -583,6 +587,7 @@ def test_webhook_field_masking(webhook_internal_api_setup, make_user_auth_header "http_method": "POST", "integration_filter": None, "is_webhook_enabled": True, + "labels": [], "is_legacy": False, "last_response_log": { "request_data": "", @@ -642,6 +647,7 @@ def test_webhook_copy(webhook_internal_api_setup, make_user_auth_headers): "http_method": "POST", "integration_filter": None, "is_webhook_enabled": True, + "labels": [], "is_legacy": False, "last_response_log": { "request_data": "", @@ -711,3 +717,184 @@ def test_create_invalid_missing_fields(webhook_internal_api_setup, make_user_aut response = client.post(url, data, format="json", **make_user_auth_headers(user, token)) assert response.status_code == status.HTTP_400_BAD_REQUEST assert response.json()["trigger_type"][0] == "This field is required." + + +@pytest.mark.django_db +def test_webhook_filter_by_labels( + make_organization_and_user_with_plugin_token, + make_custom_webhook, + make_webhook_label_association, + make_label_key_and_value, + make_user_auth_headers, +): + organization, user, token = make_organization_and_user_with_plugin_token() + webhook_with_label = make_custom_webhook(organization) + label = make_webhook_label_association(organization, webhook_with_label) + + webhook_with_another_label = make_custom_webhook(organization) + another_label = make_webhook_label_association(organization, webhook_with_another_label) + + not_attached_key, not_attached_value = make_label_key_and_value(organization) + + client = APIClient() + + # test filter by label, which is attached to only one webhook + url = reverse("api-internal:webhooks-list") + response = client.get( + f"{url}?label={label.key_id}:{label.value_id}", + content_type="application/json", + **make_user_auth_headers(user, token), + ) + + assert response.status_code == status.HTTP_200_OK + assert len(response.json()) == 1 + assert response.json()[0]["id"] == webhook_with_label.public_primary_key + + url = reverse("api-internal:webhooks-list") + response = client.get( + f"{url}?label={another_label.key_id}:{another_label.value_id}", + content_type="application/json", + **make_user_auth_headers(user, token), + ) + + assert response.status_code == status.HTTP_200_OK + assert len(response.json()) == 1 + assert response.json()[0]["id"] == webhook_with_another_label.public_primary_key + + # test filter by label which is not attached to any webhooks + response = client.get( + f"{url}?label={not_attached_key.id}:{not_attached_value.id}", + content_type="application/json", + **make_user_auth_headers(user, token), + ) + assert len(response.json()) == 0 + + +@pytest.mark.django_db +def test_update_webhook_labels( + webhook_internal_api_setup, + make_user_auth_headers, +): + user, token, webhook = webhook_internal_api_setup + client = APIClient() + + url = reverse("api-internal:webhooks-detail", kwargs={"pk": webhook.public_primary_key}) + key_id = "testkey" + value_id = "testvalue" + data = {"labels": [{"key": {"id": key_id, "name": "test"}, "value": {"id": value_id, "name": "testv"}}]} + response = client.patch( + url, + data=json.dumps(data), + content_type="application/json", + **make_user_auth_headers(user, token), + ) + + webhook.refresh_from_db() + + assert response.status_code == status.HTTP_200_OK + assert webhook.labels.count() == 1 + label = webhook.labels.first() + assert label.key_id == key_id + assert label.value_id == value_id + + response = client.patch( + url, + data=json.dumps({"labels": []}), + content_type="application/json", + **make_user_auth_headers(user, token), + ) + + webhook.refresh_from_db() + + assert response.status_code == status.HTTP_200_OK + assert webhook.labels.count() == 0 + + +@pytest.mark.django_db +def test_create_webhook_with_labels( + make_organization_and_user_with_plugin_token, + make_user_auth_headers, +): + organization, user, token = make_organization_and_user_with_plugin_token() + client = APIClient() + + url = reverse("api-internal:webhooks-list") + + key_id = "testkey" + value_id = "testvalue" + data = { + "name": "the_webhook", + "url": TEST_URL, + "trigger_type": Webhook.TRIGGER_ALERT_GROUP_CREATED, + "http_method": "POST", + "labels": [{"key": {"id": key_id, "name": "test"}, "value": {"id": value_id, "name": "testv"}}], + "team": None, + } + + response = client.post( + url, + data=json.dumps(data), + content_type="application/json", + **make_user_auth_headers(user, token), + ) + + assert response.status_code == 201 + webhook = Webhook.objects.get(public_primary_key=response.json()["id"]) + expected_response = data | { + "id": webhook.public_primary_key, + "data": None, + "username": None, + "password": None, + "authorization_header": None, + "forward_all": True, + "headers": None, + "http_method": "POST", + "integration_filter": None, + "is_webhook_enabled": True, + "is_legacy": False, + "last_response_log": { + "request_data": "", + "request_headers": "", + "timestamp": None, + "content": "", + "status_code": None, + "request_trigger": "", + "url": "", + "event_data": "", + }, + "trigger_template": None, + "trigger_type": str(data["trigger_type"]), + "trigger_type_name": "Alert Group Created", + "preset": None, + } + assert response.status_code == status.HTTP_201_CREATED + assert response.json() == expected_response + + +@pytest.mark.django_db +def test_update_webhook_labels_duplicate_key( + webhook_internal_api_setup, + make_user_auth_headers, +): + user, token, webhook = webhook_internal_api_setup + client = APIClient() + + url = reverse("api-internal:webhooks-detail", kwargs={"pk": webhook.public_primary_key}) + key_id = "testkey" + data = { + "labels": [ + {"key": {"id": key_id, "name": "test"}, "value": {"id": "testvalue1", "name": "testv1"}}, + {"key": {"id": key_id, "name": "test"}, "value": {"id": "testvalue2", "name": "testv2"}}, + ] + } + response = client.patch( + url, + data=json.dumps(data), + content_type="application/json", + **make_user_auth_headers(user, token), + ) + + webhook.refresh_from_db() + + assert response.status_code == status.HTTP_400_BAD_REQUEST + assert webhook.labels.count() == 0 diff --git a/engine/apps/api/views/alert_group.py b/engine/apps/api/views/alert_group.py index 81a26704..c5daecbd 100644 --- a/engine/apps/api/views/alert_group.py +++ b/engine/apps/api/views/alert_group.py @@ -18,6 +18,7 @@ from apps.alerts.models import Alert, AlertGroup, AlertReceiveChannel, Escalatio from apps.alerts.paging import unpage_user from apps.alerts.tasks import delete_alert_group, send_update_resolution_note_signal from apps.api.errors import AlertGroupAPIError +from apps.api.label_filtering import parse_label_query from apps.api.permissions import RBACPermission from apps.api.serializers.alert_group import AlertGroupListSerializer, AlertGroupSerializer from apps.api.serializers.team import TeamSerializer @@ -339,19 +340,15 @@ class AlertGroupView( alert_receive_channels_ids = list(alert_receive_channels_qs.values_list("id", flat=True)) queryset = AlertGroup.objects.filter(channel__in=alert_receive_channels_ids) - # filter by labels - labels = self.request.query_params.getlist("label") - for label in labels: - label_split = label.split(":") - if len(label_split) != 2: - continue - key_name, value_name = label_split - + # Filter by labels. Since alert group labels are "static" filter by names, not IDs. + label_query = self.request.query_params.getlist("label", []) + kv_pairs = parse_label_query(label_query) + for key, value in kv_pairs: # Utilize (organization, key_name, value_name, alert_group) index on AlertGroupAssociatedLabel queryset = queryset.filter( labels__organization=self.request.auth.organization, - labels__key_name=key_name, - labels__value_name=value_name, + labels__key_name=key, + labels__value_name=value, ) queryset = queryset.only("id") diff --git a/engine/apps/api/views/alert_receive_channel.py b/engine/apps/api/views/alert_receive_channel.py index aa79da6d..950af614 100644 --- a/engine/apps/api/views/alert_receive_channel.py +++ b/engine/apps/api/views/alert_receive_channel.py @@ -11,6 +11,7 @@ from rest_framework.viewsets import ModelViewSet from apps.alerts.grafana_alerting_sync_manager.grafana_alerting_sync import GrafanaAlertingSyncManager from apps.alerts.models import Alert, AlertGroup, AlertReceiveChannel from apps.alerts.models.maintainable_object import MaintainableObject +from apps.api.label_filtering import parse_label_query from apps.api.permissions import RBACPermission from apps.api.serializers.alert_receive_channel import ( AlertReceiveChannelSerializer, @@ -18,7 +19,7 @@ from apps.api.serializers.alert_receive_channel import ( FilterAlertReceiveChannelSerializer, ) from apps.api.throttlers import DemoAlertThrottler -from apps.api.views.labels import LabelsAssociatingMixin +from apps.api.views.labels import schedule_update_label_cache from apps.auth_token.auth import PluginAuthentication from apps.integrations.legacy_prefix import has_legacy_prefix, remove_legacy_prefix from apps.labels.utils import is_labels_feature_enabled @@ -76,7 +77,6 @@ class AlertReceiveChannelView( PublicPrimaryKeyMixin, FilterSerializerMixin, UpdateSerializerMixin, - LabelsAssociatingMixin, ModelViewSet, ): authentication_classes = ( @@ -159,7 +159,17 @@ class AlertReceiveChannelView( if not ignore_filtering_by_available_teams: queryset = queryset.filter(*self.available_teams_lookup_args).distinct() - queryset = self.filter_by_labels(queryset) + # filter labels + label_query = self.request.query_params.getlist("label", []) + kv_pairs = parse_label_query(label_query) + for key, value in kv_pairs: + queryset = queryset.filter( + labels__key_id=key, + labels__value_id=value, + ) + + # distinct to remove duplicates after alert_receive_channels X labels join + queryset = queryset.distinct() return queryset @@ -170,7 +180,11 @@ class AlertReceiveChannelView( """ if self.request.query_params.get("skip_pagination", "false").lower() == "true": return None - return super().paginate_queryset(queryset) + page = super().paginate_queryset(queryset) + if page is not None: + ids = [d.id for d in queryset] + schedule_update_label_cache(self.model.__name__, self.request.auth.organization, ids) + return page @action(detail=True, methods=["post"], throttle_classes=[DemoAlertThrottler]) def send_demo_alert(self, request, pk): diff --git a/engine/apps/api/views/labels.py b/engine/apps/api/views/labels.py index 9b3cd36c..dc1df6a4 100644 --- a/engine/apps/api/views/labels.py +++ b/engine/apps/api/views/labels.py @@ -6,7 +6,6 @@ from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.viewsets import ViewSet -from apps.alerts.models import AlertReceiveChannel from apps.api.permissions import BasicRolePermission, LegacyAccessControlRole from apps.api.serializers.labels import ( LabelKeySerializer, @@ -172,30 +171,8 @@ class AlertGroupLabelsViewSet(LabelsFeatureFlagViewSet): ) -class LabelsAssociatingMixin: # use for labelable objects views (ex. AlertReceiveChannelView) - def filter_by_labels(self, queryset): - """Call this method in `get_queryset()` to add filtering by labels""" - if not is_labels_feature_enabled(self.request.auth.organization): - return queryset - labels = self.request.query_params.getlist("label") # ["key1:value1", "key2:value2"] - if not labels: - return queryset - for label in labels: - label_data = label.split(":") - if len(label_data) != 2: # ["key1", "value1"] - continue - key_id, value_id = label_data - queryset &= AlertReceiveChannel.objects_with_deleted.filter( - labels__key_id=key_id, labels__value_id=value_id - ).distinct() - return queryset - - def paginate_queryset(self, queryset): - organization = self.request.auth.organization - data = super().paginate_queryset(queryset) - if not is_labels_feature_enabled(self.request.auth.organization): - return data - ids = [d.id for d in data] - logger.info(f"start update_instances_labels_cache for ids: {ids}") - update_instances_labels_cache.apply_async((organization.id, ids, self.model.__name__)) - return data +def schedule_update_label_cache(model_name, org, ids): + if not is_labels_feature_enabled(org): + return + logger.info(f"start update_instances_labels_cache for ids: {ids}") + update_instances_labels_cache.apply_async((org.id, ids, model_name)) diff --git a/engine/apps/api/views/webhooks.py b/engine/apps/api/views/webhooks.py index bd7dc8d7..2aa1258f 100644 --- a/engine/apps/api/views/webhooks.py +++ b/engine/apps/api/views/webhooks.py @@ -11,9 +11,12 @@ from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response from rest_framework.viewsets import ModelViewSet +from apps.api.label_filtering import parse_label_query from apps.api.permissions import RBACPermission from apps.api.serializers.webhook import WebhookResponseSerializer, WebhookSerializer +from apps.api.views.labels import schedule_update_label_cache from apps.auth_token.auth import PluginAuthentication +from apps.labels.utils import is_labels_feature_enabled from apps.webhooks.models import Webhook, WebhookResponse from apps.webhooks.presets.preset_options import WebhookPresetOptions from apps.webhooks.utils import apply_jinja_template_for_json @@ -94,6 +97,21 @@ class WebhooksView(TeamFilteringMixin, PublicPrimaryKeyMixin, ModelViewSet): ).prefetch_related("responses") if not ignore_filtering_by_available_teams: queryset = queryset.filter(*self.available_teams_lookup_args).distinct() + + # filter by labels + label_query = self.request.query_params.getlist("label", []) + kv_pairs = parse_label_query(label_query) + for key, value in kv_pairs: + queryset = queryset.filter( + labels__key_id=key, + labels__value_id=value, + ) + # distinct to remove duplicates after webhooks X labels join + queryset = queryset.distinct() + # schedule update of labels cache + ids = [d.id for d in queryset] + schedule_update_label_cache(self.model.__name__, self.request.auth.organization, ids) + return queryset def get_object(self): @@ -132,6 +150,15 @@ class WebhooksView(TeamFilteringMixin, PublicPrimaryKeyMixin, ModelViewSet): }, ] + if is_labels_feature_enabled(self.request.auth.organization): + filter_options.append( + { + "name": "label", + "display_name": "Label", + "type": "labels", + } + ) + if filter_name is not None: filter_options = list(filter(lambda f: filter_name in f["name"], filter_options)) diff --git a/engine/apps/labels/migrations/0004_webhookassociatedlabel.py b/engine/apps/labels/migrations/0004_webhookassociatedlabel.py new file mode 100644 index 00000000..7c7f645b --- /dev/null +++ b/engine/apps/labels/migrations/0004_webhookassociatedlabel.py @@ -0,0 +1,29 @@ +# Generated by Django 4.2.7 on 2023-11-22 06:10 + +from django.db import migrations, models +import django.db.models.deletion + + +class Migration(migrations.Migration): + + dependencies = [ + ('webhooks', '0011_auto_20230920_1813'), + ('user_management', '0017_alter_organization_maintenance_author'), + ('labels', '0003_alertreceivechannelassociatedlabel_inherit'), + ] + + operations = [ + migrations.CreateModel( + name='WebhookAssociatedLabel', + fields=[ + ('id', models.BigAutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('key', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='labels.labelkeycache')), + ('organization', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='webhook_labels', to='user_management.organization')), + ('value', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to='labels.labelvaluecache')), + ('webhook', models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='labels', to='webhooks.webhook')), + ], + options={ + 'unique_together': {('key_id', 'value_id', 'webhook_id')}, + }, + ), + ] diff --git a/engine/apps/labels/models.py b/engine/apps/labels/models.py index f9861a72..28947e75 100644 --- a/engine/apps/labels/models.py +++ b/engine/apps/labels/models.py @@ -139,3 +139,24 @@ class AlertGroupAssociatedLabel(models.Model): name="unique_alert_group_label", ) ] + + +class WebhookAssociatedLabel(AssociatedLabel): + """Keeps information about label association with outgoing webhooks instances""" + + webhook = models.ForeignKey( + "webhooks.Webhook", + on_delete=models.CASCADE, + related_name="labels", + ) + organization = models.ForeignKey( + "user_management.Organization", on_delete=models.CASCADE, related_name="webhook_labels" + ) + + class Meta: + unique_together = ["key_id", "value_id", "webhook_id"] + + @staticmethod + def get_associating_label_field_name() -> str: + """Returns ForeignKey field name for the associated model""" + return "webhook" diff --git a/engine/apps/labels/tasks.py b/engine/apps/labels/tasks.py index 65c7c550..2ac22ba1 100644 --- a/engine/apps/labels/tasks.py +++ b/engine/apps/labels/tasks.py @@ -6,7 +6,13 @@ from django.conf import settings from django.utils import timezone from apps.labels.client import LabelsAPIClient -from apps.labels.utils import LABEL_OUTDATED_TIMEOUT_MINUTES, LabelKeyData, LabelsData, get_associating_label_model +from apps.labels.utils import ( + LABEL_OUTDATED_TIMEOUT_MINUTES, + LabelKeyData, + LabelsData, + ValueData, + get_associating_label_model, +) from apps.user_management.models import Organization from common.custom_celery_tasks import shared_dedicated_queue_retry_task @@ -14,11 +20,6 @@ logger = get_task_logger(__name__) logger.setLevel(logging.DEBUG) -class ValueData(typing.TypedDict): - value_name: str - key_name: str - - def unify_labels_data(labels_data: LabelsData | LabelKeyData) -> typing.Dict[str, ValueData]: values_data: typing.Dict[str, ValueData] if isinstance(labels_data, list): # LabelsData diff --git a/engine/apps/labels/tests/factories.py b/engine/apps/labels/tests/factories.py index 5f910989..aa4052f7 100644 --- a/engine/apps/labels/tests/factories.py +++ b/engine/apps/labels/tests/factories.py @@ -5,6 +5,7 @@ from apps.labels.models import ( AlertReceiveChannelAssociatedLabel, LabelKeyCache, LabelValueCache, + WebhookAssociatedLabel, ) from common.utils import UniqueFaker @@ -33,3 +34,8 @@ class AlertReceiveChannelAssociatedLabelFactory(factory.DjangoModelFactory): class AlertGroupAssociatedLabelFactory(factory.DjangoModelFactory): class Meta: model = AlertGroupAssociatedLabel + + +class WebhookAssociatedLabelFactory(factory.DjangoModelFactory): + class Meta: + model = WebhookAssociatedLabel diff --git a/engine/apps/labels/tests/test_labels.py b/engine/apps/labels/tests/test_labels.py index 9d32583a..b4d116c2 100644 --- a/engine/apps/labels/tests/test_labels.py +++ b/engine/apps/labels/tests/test_labels.py @@ -1,8 +1,14 @@ import pytest from apps.alerts.models import AlertReceiveChannel -from apps.labels.models import AlertReceiveChannelAssociatedLabel, AssociatedLabel, LabelValueCache +from apps.labels.models import ( + AlertReceiveChannelAssociatedLabel, + AssociatedLabel, + LabelValueCache, + WebhookAssociatedLabel, +) from apps.labels.utils import get_associating_label_model, is_labels_feature_enabled +from apps.webhooks.models import Webhook @pytest.mark.django_db @@ -104,6 +110,11 @@ def test_get_associating_label_model(): result = get_associating_label_model(model_name) assert result == expected_result + model_name = Webhook.__name__ + expected_result = WebhookAssociatedLabel + result = get_associating_label_model(model_name) + assert result == expected_result + wrong_model_name = "SomeModel" with pytest.raises(LookupError): get_associating_label_model(wrong_model_name) diff --git a/engine/apps/labels/utils.py b/engine/apps/labels/utils.py index 98d1bf95..46b0583a 100644 --- a/engine/apps/labels/utils.py +++ b/engine/apps/labels/utils.py @@ -27,6 +27,11 @@ class LabelData(typing.TypedDict): value: LabelParams +class ValueData(typing.TypedDict): + value_name: str + key_name: str + + class LabelKeyData(typing.TypedDict): key: LabelParams values: typing.List[LabelParams] @@ -66,3 +71,18 @@ def assign_labels(alert_group: "AlertGroup", alert_receive_channel: "AlertReceiv for label in alert_receive_channel.labels.filter(inheritable=True).select_related("key", "value") ] AlertGroupAssociatedLabel.objects.bulk_create(alert_group_labels) + + +def get_label_verbal(labelable) -> typing.Dict[str, str]: + """ + label_verbal returns dict of labels' key and values names for the given object + """ + return {label.key.name: label.value.name for label in labelable.labels.all().select_related("key", "value")} + + +def get_alert_group_label_verbal(alert_group: "AlertGroup") -> typing.Dict[str, str]: + """ + get_alert_group_label_verbal returns dict of labels' key and values names for the given alert group. + It's different from get_label_verbal, because AlertGroupAssociated labels store key/value_name, not key/value_id + """ + return {label.key_name: label.value_name for label in alert_group.labels.all()} diff --git a/engine/apps/webhooks/tasks/trigger_webhook.py b/engine/apps/webhooks/tasks/trigger_webhook.py index f7f71f0a..0e43f82c 100644 --- a/engine/apps/webhooks/tasks/trigger_webhook.py +++ b/engine/apps/webhooks/tasks/trigger_webhook.py @@ -91,7 +91,7 @@ def _build_payload(webhook, alert_group, user): response_data = r.content responses_data[r.webhook.public_primary_key] = response_data - data = serialize_event(event, alert_group, user, responses_data) + data = serialize_event(event, alert_group, user, webhook, responses_data) return data diff --git a/engine/apps/webhooks/tests/test_trigger_webhook.py b/engine/apps/webhooks/tests/test_trigger_webhook.py index 95483224..4da2e6ed 100644 --- a/engine/apps/webhooks/tests/test_trigger_webhook.py +++ b/engine/apps/webhooks/tests/test_trigger_webhook.py @@ -302,6 +302,7 @@ def test_execute_webhook_ok_forward_all( "type": alert_receive_channel.integration, "name": alert_receive_channel.short_name, "team": None, + "labels": {}, }, "notified_users": [ { @@ -310,10 +311,15 @@ def test_execute_webhook_ok_forward_all( "email": notified_user.email, } ], - "alert_group": IncidentSerializer(alert_group).data, + "alert_group": {**IncidentSerializer(alert_group).data, "labels": {}}, "alert_group_id": alert_group.public_primary_key, "alert_payload": "", "users_to_be_notified": [], + "webhook": { + "id": webhook.public_primary_key, + "name": webhook.name, + "labels": {}, + }, } expected_call = call( "https://something/{}/".format(alert_group.public_primary_key), diff --git a/engine/apps/webhooks/utils.py b/engine/apps/webhooks/utils.py index 5b9cb92a..feafe93e 100644 --- a/engine/apps/webhooks/utils.py +++ b/engine/apps/webhooks/utils.py @@ -7,6 +7,7 @@ from urllib.parse import urlparse from django.conf import settings from apps.base.utils import live_settings +from apps.labels.utils import get_alert_group_label_verbal, get_label_verbal, is_labels_feature_enabled from apps.schedules.ical_utils import list_users_to_notify_from_ical from common.jinja_templater import apply_jinja_template @@ -150,7 +151,7 @@ def _extract_users_from_escalation_snapshot(escalation_snapshot): return list({u["id"]: u for u in users if u}.values()) -def serialize_event(event, alert_group, user, responses=None): +def serialize_event(event, alert_group, user, webhook, responses=None): from apps.public_api.serializers import IncidentSerializer alert_payload = alert_group.alerts.first() @@ -179,4 +180,10 @@ def serialize_event(event, alert_group, user, responses=None): if responses: data["responses"] = responses + # Enrich webhook data with labels payloads if labels feature is enabled + # TODO: once feature flag will be removed this code should go to the 'data' dict declaration + if is_labels_feature_enabled(alert_group.channel.organization): + data["webhook"] = {"id": webhook.public_primary_key, "name": webhook.name, "labels": get_label_verbal(webhook)} + data["integration"]["labels"] = get_label_verbal(alert_group.channel) + data["alert_group"]["labels"] = get_alert_group_label_verbal(alert_group) return data diff --git a/engine/conftest.py b/engine/conftest.py index 200f3a0d..4f30e34a 100644 --- a/engine/conftest.py +++ b/engine/conftest.py @@ -62,6 +62,7 @@ from apps.labels.tests.factories import ( AlertReceiveChannelAssociatedLabelFactory, LabelKeyFactory, LabelValueFactory, + WebhookAssociatedLabelFactory, ) from apps.mobile_app.models import MobileAppAuthToken, MobileAppVerificationToken from apps.phone_notifications.phone_backend import PhoneBackend @@ -994,3 +995,12 @@ def make_alert_group_label_association(): return AlertGroupAssociatedLabelFactory(alert_group=alert_group, organization=organization, **kwargs) return _make_alert_group_label_association + + +@pytest.fixture +def make_webhook_label_association(make_label_key_and_value): + def _make_integration_label_association(organization, webhook, **kwargs): + key, value = make_label_key_and_value(organization) + return WebhookAssociatedLabelFactory(webhook=webhook, organization=organization, key=key, value=value, **kwargs) + + return _make_integration_label_association diff --git a/grafana-plugin/.eslintrc.js b/grafana-plugin/.eslintrc.js index 23cbc3ab..a09cde51 100644 --- a/grafana-plugin/.eslintrc.js +++ b/grafana-plugin/.eslintrc.js @@ -49,6 +49,8 @@ module.exports = { ], 'no-duplicate-imports': 'error', 'no-restricted-imports': 'warn', + // https://eslint.org/docs/latest/rules/no-redeclare#handled_by_typescript + 'no-redeclare': 0, 'react/display-name': 'warn', /** * It appears as though the react/prop-types rule has a bug in it diff --git a/grafana-plugin/package.json b/grafana-plugin/package.json index 02cd81fa..3b7bda98 100644 --- a/grafana-plugin/package.json +++ b/grafana-plugin/package.json @@ -4,7 +4,7 @@ "description": "Grafana OnCall Plugin", "scripts": { "lint": "eslint --cache --ext .js,.jsx,.ts,.tsx --max-warnings=0 ./src ./e2e-tests", - "lint:fix": "eslint --fix --cache --ext .js,.jsx,.ts,.tsx --max-warnings=0 --quiet ./src ./e2e-tests", + "lint:fix": "eslint --fix --cache --ext .js,.jsx,.ts,.tsx --quiet ./src ./e2e-tests", "stylelint": "stylelint ./src/**/*.{css,scss,module.css,module.scss}", "stylelint:fix": "stylelint --fix ./src/**/*.{css,scss,module.css,module.scss}", "build": "grafana-toolkit plugin:build", diff --git a/grafana-plugin/src/components/GForm/GForm.tsx b/grafana-plugin/src/components/GForm/GForm.tsx index cbba9571..fb1c77ec 100644 --- a/grafana-plugin/src/components/GForm/GForm.tsx +++ b/grafana-plugin/src/components/GForm/GForm.tsx @@ -10,13 +10,21 @@ import { FormItem, FormItemType } from 'components/GForm/GForm.types'; import MonacoEditor from 'components/MonacoEditor/MonacoEditor'; import { MONACO_READONLY_CONFIG } from 'components/MonacoEditor/MonacoEditor.config'; import GSelect from 'containers/GSelect/GSelect'; -import { CustomFieldSectionRendererProps } from 'containers/IntegrationForm/IntegrationForm'; import RemoteSelect from 'containers/RemoteSelect/RemoteSelect'; import styles from './GForm.module.scss'; const cx = cn.bind(styles); +export interface CustomFieldSectionRendererProps { + control: any; + formItem: FormItem; + errors: any; + register: any; + setValue: (fieldName: string, fieldValue: any) => void; + getValues: (fieldName: string | string[]) => T; +} + interface GFormProps { form: { name: string; fields: FormItem[] }; data: any; @@ -211,6 +219,7 @@ class GForm extends React.Component { }} errors={errors} register={register} + getValues={getValues} /> ); } diff --git a/grafana-plugin/src/components/LabelsTooltipBadge/LabelsTooltipBadge.tsx b/grafana-plugin/src/components/LabelsTooltipBadge/LabelsTooltipBadge.tsx new file mode 100644 index 00000000..5ec9d084 --- /dev/null +++ b/grafana-plugin/src/components/LabelsTooltipBadge/LabelsTooltipBadge.tsx @@ -0,0 +1,40 @@ +import React, { FC } from 'react'; + +import { LabelTag } from '@grafana/labels'; +import { VerticalGroup, HorizontalGroup, Button } from '@grafana/ui'; + +import TooltipBadge from 'components/TooltipBadge/TooltipBadge'; +import { LabelKeyValue } from 'models/label/label.types'; + +interface LabelsTooltipBadgeProps { + labels: LabelKeyValue[]; + onClick: (label: LabelKeyValue) => void; +} + +const LabelsTooltipBadge: FC = ({ labels, onClick }) => + labels.length ? ( + + {labels.map((label) => ( + + +