From c9ce61efd79f677b4d70026f5391f3f37aa418e2 Mon Sep 17 00:00:00 2001 From: Yulya Artyukhina Date: Thu, 2 Nov 2023 17:40:26 +0100 Subject: [PATCH 01/12] Fix db migration for mobile app (#3260) # What this PR does Fixes bd migration ## Which issue(s) this PR fixes Closes https://github.com/grafana/oncall/issues/3222 ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [x] Documentation added (or `pr:no public docs` PR label added if not required) - [x] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- CHANGELOG.md | 4 ++++ ...sersettings_going_oncall_notification_timing.py | 14 +++++--------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0edbcb4e..fc8f47fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Fixed + +- Fix db migration for mobile app @Ferril ([#3260](https://github.com/grafana/oncall/pull/3260)) + ## v1.3.52 (2023-11-02) ### Fixed diff --git a/engine/apps/mobile_app/migrations/0011_alter_mobileappusersettings_going_oncall_notification_timing.py b/engine/apps/mobile_app/migrations/0011_alter_mobileappusersettings_going_oncall_notification_timing.py index c758fa8c..cdeac8fc 100644 --- a/engine/apps/mobile_app/migrations/0011_alter_mobileappusersettings_going_oncall_notification_timing.py +++ b/engine/apps/mobile_app/migrations/0011_alter_mobileappusersettings_going_oncall_notification_timing.py @@ -4,13 +4,6 @@ import apps.mobile_app.models import django_migration_linter as linter from django.db import migrations, models -from apps.mobile_app.models import default_notification_timing_options - - -def set_going_oncall_notification_timing_to_default(apps, schema_editor): - MobileAppUserSettings = apps.get_model("mobile_app", "MobileAppUserSettings") - default = default_notification_timing_options() - MobileAppUserSettings.objects.all().update(going_oncall_notification_timing=default) class Migration(migrations.Migration): @@ -21,10 +14,13 @@ class Migration(migrations.Migration): operations = [ linter.IgnoreMigration(), - migrations.AlterField( + migrations.RemoveField( + model_name='mobileappusersettings', + name='going_oncall_notification_timing', + ), + migrations.AddField( model_name='mobileappusersettings', name='going_oncall_notification_timing', field=models.JSONField(default=apps.mobile_app.models.default_notification_timing_options), ), - migrations.RunPython(set_going_oncall_notification_timing_to_default, migrations.RunPython.noop), ] From f23a0572a1b0822916472727a4334e8f0f3c8da3 Mon Sep 17 00:00:00 2001 From: Michael Derynck Date: Thu, 2 Nov 2023 11:56:12 -0600 Subject: [PATCH 02/12] Limit number of times webhooks retry (#3244) # What this PR does Set limit on number of times webhooks retry to 3 ## Which issue(s) this PR fixes ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- engine/apps/webhooks/tasks/trigger_webhook.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/apps/webhooks/tasks/trigger_webhook.py b/engine/apps/webhooks/tasks/trigger_webhook.py index 68bfdf22..f7f71f0a 100644 --- a/engine/apps/webhooks/tasks/trigger_webhook.py +++ b/engine/apps/webhooks/tasks/trigger_webhook.py @@ -167,7 +167,7 @@ def make_request(webhook, alert_group, data): @shared_dedicated_queue_retry_task( - autoretry_for=(Exception,), retry_backoff=True, max_retries=1 if settings.DEBUG else None + autoretry_for=(Exception,), retry_backoff=True, max_retries=1 if settings.DEBUG else 3 ) def execute_webhook(webhook_pk, alert_group_id, user_id, escalation_policy_id): from apps.webhooks.models import Webhook From 1056f9f039e0535d840892569b2fd751d197810a Mon Sep 17 00:00:00 2001 From: Ildar Iskhakov Date: Fri, 3 Nov 2023 17:09:24 +0800 Subject: [PATCH 03/12] Update docker-compose.yml (#3266) # What this PR does ## Which issue(s) this PR fixes ## Checklist - [ ] Unit, integration, and e2e (if applicable) tests updated - [ ] Documentation added (or `pr:no public docs` PR label added if not required) - [ ] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- docker-compose.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docker-compose.yml b/docker-compose.yml index 2dadfc3a..b7476986 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -9,7 +9,7 @@ x-environment: &oncall-environment PROMETHEUS_EXPORTER_SECRET: ${PROMETHEUS_EXPORTER_SECRET:-} REDIS_URI: redis://redis:6379/0 DJANGO_SETTINGS_MODULE: settings.hobby - CELERY_WORKER_QUEUE: "default,critical,long,slack,telegram,webhook,retry,celery" + CELERY_WORKER_QUEUE: "default,critical,long,slack,telegram,webhook,retry,celery,grafana" CELERY_WORKER_CONCURRENCY: "1" CELERY_WORKER_MAX_TASKS_PER_CHILD: "100" CELERY_WORKER_SHUTDOWN_INTERVAL: "65m" From 68c1b2efbac451682abd29f7e105b4e1791bac2e Mon Sep 17 00:00:00 2001 From: Yulya Artyukhina Date: Fri, 3 Nov 2023 11:22:38 +0100 Subject: [PATCH 04/12] Fix getting alert group from telegram action (#3262) # What this PR does Fix getting alert group from telegram action: try to get alert group from telegram message first instead of getting it from action data ## 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] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- .../tests/test_button_update_handler.py | 7 +++++- .../updates/update_handlers/button_press.py | 24 ++++++++++++++----- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/engine/apps/telegram/tests/test_button_update_handler.py b/engine/apps/telegram/tests/test_button_update_handler.py index 5ae49f7d..657c9e46 100644 --- a/engine/apps/telegram/tests/test_button_update_handler.py +++ b/engine/apps/telegram/tests/test_button_update_handler.py @@ -1,4 +1,4 @@ -from unittest.mock import MagicMock +from unittest.mock import MagicMock, patch import pytest @@ -6,8 +6,13 @@ from apps.telegram.renderers.keyboard import Action from apps.telegram.updates.update_handlers.button_press import ButtonPressHandler +@patch( + "apps.telegram.updates.update_handlers.button_press.ButtonPressHandler._get_alert_group_from_message", + return_value=None, +) @pytest.mark.django_db def test_get_action_context( + mocked_get_alert_group_from_message, make_organization_and_user_with_slack_identities, make_alert_receive_channel, make_alert_group, diff --git a/engine/apps/telegram/updates/update_handlers/button_press.py b/engine/apps/telegram/updates/update_handlers/button_press.py index d629462c..0c8c13f8 100644 --- a/engine/apps/telegram/updates/update_handlers/button_press.py +++ b/engine/apps/telegram/updates/update_handlers/button_press.py @@ -5,7 +5,7 @@ from typing import Callable, Optional, Tuple from apps.alerts.constants import ActionSource from apps.alerts.models import AlertGroup from apps.api.permissions import RBACPermission, user_is_authorized -from apps.telegram.models import TelegramToUserConnector +from apps.telegram.models import TelegramMessage, TelegramToUserConnector from apps.telegram.renderers.keyboard import CODE_TO_ACTION_MAP, Action from apps.telegram.updates.update_handlers import UpdateHandler from apps.telegram.utils import CallbackQueryFactory @@ -45,6 +45,15 @@ class ButtonPressHandler(UpdateHandler): self.update.callback_query.answer(PERMISSION_DENIED, show_alert=True) logger.info(f"User {user} has no permission to trigger '{fn.__name__}'") + def _get_alert_group_from_message(self) -> Optional[AlertGroup]: + alert_group = None + if self.update.message: + telegram_message = TelegramMessage.objects.get( + message_id=self.update.message.message_id, chat_id=self.update.message.chat.id + ) + alert_group = telegram_message.alert_group + return alert_group + def _get_user(self, action_context: ActionContext) -> Optional[User]: connector = TelegramToUserConnector.objects.filter( telegram_chat_id=self.update.effective_user.id, @@ -60,12 +69,15 @@ class ButtonPressHandler(UpdateHandler): has_permission = user_is_authorized(user, [RBACPermission.Permissions.CHATOPS_WRITE]) return user.organization == alert_group.channel.organization and has_permission - @classmethod - def _get_action_context(cls, data: str) -> ActionContext: + def _get_action_context(self, data: str) -> ActionContext: args = CallbackQueryFactory.decode_data(data) - alert_group_pk = args[0] - alert_group = AlertGroup.objects.get(pk=alert_group_pk) + # Try to get alert group from telegram message, because encoded data is not valid for migrated organizations + alert_group = self._get_alert_group_from_message() + + if alert_group is None: + alert_group_pk = args[0] + alert_group = AlertGroup.objects.get(pk=alert_group_pk) action_value = args[1] try: @@ -77,7 +89,7 @@ class ButtonPressHandler(UpdateHandler): action_name = action_value action = Action(action_name) - action_data = args[2] if len(args) >= 3 and not cls._is_oncall_identifier(args[2]) else None + action_data = args[2] if len(args) >= 3 and not self._is_oncall_identifier(args[2]) else None return ActionContext(alert_group=alert_group, action=action, action_data=action_data) From 24357f5ff0e6e5dd5fe1da737fc514390c87676e Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Fri, 3 Nov 2023 09:34:22 -0300 Subject: [PATCH 05/12] Update helm chart to detach integrations pod (#3204) Depends on https://github.com/grafana/oncall/pull/3203 Related to https://github.com/grafana/oncall/issues/3162 --- engine/engine/tests/test_views.py | 41 ++++++ engine/engine/views.py | 9 +- helm/README.md | 28 +++- helm/kind.yml | 2 + helm/oncall/templates/_env.tpl | 14 ++ helm/oncall/templates/_helpers.tpl | 5 + helm/oncall/templates/celery/deployment.yaml | 11 +- helm/oncall/templates/engine/deployment.yaml | 12 +- helm/oncall/templates/engine/job-migrate.yaml | 2 +- helm/oncall/templates/ingress-regular.yaml | 9 ++ .../templates/integrations/_helpers.tpl | 26 ++++ .../templates/integrations/deployment.yaml | 99 ++++++++++++++ .../integrations/service-external.yaml | 24 ++++ .../integrations/service-internal.yaml | 15 ++ .../telegram-polling/deployment.yaml | 2 +- .../integrations_deployment_test.yaml.snap | 129 ++++++++++++++++++ ...telegram_polling_deployment_test.yaml.snap | 2 + .../__snapshot__/wait_for_db_test.yaml.snap | 12 ++ .../tests/integrations_deployment_test.yaml | 52 +++++++ helm/oncall/values.yaml | 76 +++++++++++ helm/simple.yml | 7 + 21 files changed, 548 insertions(+), 29 deletions(-) create mode 100644 engine/engine/tests/test_views.py create mode 100644 helm/oncall/templates/integrations/_helpers.tpl create mode 100644 helm/oncall/templates/integrations/deployment.yaml create mode 100644 helm/oncall/templates/integrations/service-external.yaml create mode 100644 helm/oncall/templates/integrations/service-internal.yaml create mode 100644 helm/oncall/tests/__snapshot__/integrations_deployment_test.yaml.snap create mode 100644 helm/oncall/tests/integrations_deployment_test.yaml diff --git a/engine/engine/tests/test_views.py b/engine/engine/tests/test_views.py new file mode 100644 index 00000000..27eeed7a --- /dev/null +++ b/engine/engine/tests/test_views.py @@ -0,0 +1,41 @@ +import sys +from importlib import import_module, reload +from unittest.mock import patch + +import pytest +from django.conf import settings +from django.urls import clear_url_caches +from rest_framework import status +from rest_framework.test import APIClient + + +def reload_urlconf(): + clear_url_caches() + if settings.ROOT_URLCONF in sys.modules: + reload(sys.modules[settings.ROOT_URLCONF]) + return import_module(settings.ROOT_URLCONF) + + +@pytest.mark.parametrize( + "detached_integrations,urlconf,is_cache_updated", + [ + (False, None, True), + (True, None, False), + (True, "engine.integrations_urls", True), + ], +) +def test_startupprobe_populates_integrations_cache(settings, detached_integrations, urlconf, is_cache_updated): + settings.DETACHED_INTEGRATIONS_SERVER = detached_integrations + if urlconf: + settings.ROOT_URLCONF = urlconf + reload_urlconf() + + client = APIClient() + + with patch( + "apps.integrations.mixins.AlertChannelDefiningMixin.update_alert_receive_channel_cache" + ) as mock_update_cache: + response = client.get("/startupprobe/") + + assert response.status_code == status.HTTP_200_OK + assert mock_update_cache.called == is_cache_updated diff --git a/engine/engine/views.py b/engine/engine/views.py index 7f3e2f5d..0022d02f 100644 --- a/engine/engine/views.py +++ b/engine/engine/views.py @@ -1,3 +1,4 @@ +from django import urls from django.conf import settings from django.core.cache import cache from django.http import HttpResponse, JsonResponse @@ -43,7 +44,13 @@ class StartupProbeView(View): dangerously_bypass_middlewares = True def get(self, request): - if cache.get(AlertChannelDefiningMixin.CACHE_KEY_DB_FALLBACK) is None: + # enable integrations cache if current engine instance is serving them + integrations_enabled = True + if settings.DETACHED_INTEGRATIONS_SERVER: + url_resolver = urls.get_resolver(urls.get_urlconf()) + integrations_enabled = url_resolver.namespace_dict.get("integrations") + + if integrations_enabled and cache.get(AlertChannelDefiningMixin.CACHE_KEY_DB_FALLBACK) is None: AlertChannelDefiningMixin().update_alert_receive_channel_cache() cache.set("healthcheck", "healthcheck", 30) # Checking cache connectivity diff --git a/helm/README.md b/helm/README.md index 8e9a4de2..f3fce708 100644 --- a/helm/README.md +++ b/helm/README.md @@ -2,7 +2,8 @@ 1. Create the cluster with [kind](https://kind.sigs.k8s.io/docs/user/quick-start/#installation) - > Make sure ports 30001 and 30002 are free on your machine + > Make sure ports 30001, 30002 (Grafana, optional) and + > 30003 (detached integrations server, optional) are free on your machine ```bash kind create cluster --image kindest/node:v1.24.7 --config kind.yml @@ -10,12 +11,25 @@ 2. (Optional) Build oncall image locally and load it to kind cluster -3. ```bash - docker build ../engine -t oncall/engine:latest --target dev - kind load docker-image oncall/engine:latest + ```bash + docker build ../engine -t oncall/engine:latest --target dev + kind load docker-image oncall/engine:latest ``` -4. Install the helm chart + Also make sure to add the following lines to your `simple.yml` (you may also need to enable `devMode`): + + ```yaml + image: + repository: oncall/engine + tag: latest + pullPolicy: IfNotPresent + oncall: + devMode: true + ``` + + Alternatively you can also pass an extra `--values ./local_image.yml` in the command below. + +3. Install the helm chart ```bash helm install helm-testing \ @@ -24,14 +38,14 @@ ./oncall ``` -5. Get credentials +4. Get credentials ```bash echo "\n\nOpen Grafana on localhost:30002 with credentials - user: admin, password: $(kubectl get secret --namespace default helm-testing-grafana -o jsonpath="{.data.admin-password}" | base64 --decode ; echo)" echo "Open Plugins -> Grafana OnCall -> fill form: backend url: http://host.docker.internal:30001" ``` -6. Clean up +5. Clean up If you happen to `helm uninstall helm-testing` be sure to delete all the Persistent Volume Claims, as Postgres stores the auto-generated password on disk, and the next `helm install` will fail. diff --git a/helm/kind.yml b/helm/kind.yml index 0cb7db53..4b564559 100644 --- a/helm/kind.yml +++ b/helm/kind.yml @@ -8,6 +8,8 @@ nodes: hostPort: 30001 - containerPort: 30002 hostPort: 30002 + - containerPort: 30003 + hostPort: 30003 # https://stackoverflow.com/a/62695918 extraMounts: # this basically mounts our local ./grafana-plugin (frontend) directory into the kind node diff --git a/helm/oncall/templates/_env.tpl b/helm/oncall/templates/_env.tpl index 72f7d4c8..56dff3e5 100644 --- a/helm/oncall/templates/_env.tpl +++ b/helm/oncall/templates/_env.tpl @@ -19,6 +19,8 @@ value: "admin" - name: OSS value: "True" +- name: DETACHED_INTEGRATIONS_SERVER + value: {{ .Values.detached_integrations.enabled | toString | title | quote }} {{- include "snippet.oncall.uwsgi" . }} - name: BROKER_TYPE value: {{ .Values.broker.type | default "rabbitmq" }} @@ -640,3 +642,15 @@ when broker.type != rabbitmq, we do not need to include rabbitmq environment var value: {{ .Values.oncall.exporter.enabled | toString | title | quote }} {{- end }} {{- end }} + +{{- define "snippet.oncall.engine.env" -}} +{{ include "snippet.oncall.env" . }} +{{ include "snippet.oncall.slack.env" . }} +{{ include "snippet.oncall.telegram.env" . }} +{{ include "snippet.oncall.smtp.env" . }} +{{ include "snippet.oncall.twilio.env" . }} +{{ include "snippet.oncall.exporter.env" . }} +{{ include "snippet.db.env" . }} +{{ include "snippet.broker.env" . }} +{{ include "oncall.extraEnvs" . }} +{{- end }} diff --git a/helm/oncall/templates/_helpers.tpl b/helm/oncall/templates/_helpers.tpl index 8a602cc6..6486bfe5 100644 --- a/helm/oncall/templates/_helpers.tpl +++ b/helm/oncall/templates/_helpers.tpl @@ -85,6 +85,11 @@ Create the name of the service account to use {{- printf "%s-%s" .Release.Name "redis" | trunc 63 | trimSuffix "-" }} {{- end }} +{{/* Generate engine image name */}} +{{- define "oncall.engine.image" -}} +{{- printf "%s:%s" .Values.image.repository (.Values.image.tag | default .Chart.AppVersion) }} +{{- end }} + {{- define "oncall.initContainer" }} - name: wait-for-db image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}" diff --git a/helm/oncall/templates/celery/deployment.yaml b/helm/oncall/templates/celery/deployment.yaml index fcf1877e..32b4245c 100644 --- a/helm/oncall/templates/celery/deployment.yaml +++ b/helm/oncall/templates/celery/deployment.yaml @@ -51,7 +51,7 @@ spec: - name: {{ .Chart.Name }} securityContext: {{- toYaml .Values.securityContext | nindent 12 }} - image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}" + image: {{ include "oncall.engine.image" . }} {{- if .Values.oncall.devMode }} command: ["python", "manage.py", "start_celery"] {{- else }} @@ -60,14 +60,7 @@ spec: imagePullPolicy: {{ .Values.image.pullPolicy }} env: {{- include "snippet.celery.env" . | nindent 12 }} - {{- include "snippet.oncall.env" . | nindent 12 }} - {{- include "snippet.oncall.slack.env" . | nindent 12 }} - {{- include "snippet.oncall.telegram.env" . | nindent 12 }} - {{- include "snippet.oncall.smtp.env" . | nindent 12 }} - {{- include "snippet.oncall.exporter.env" . | nindent 12 }} - {{- include "snippet.db.env" . | nindent 12 }} - {{- include "snippet.broker.env" . | nindent 12 }} - {{- include "oncall.extraEnvs" . | nindent 12 }} + {{- include "snippet.oncall.engine.env" . | nindent 12 }} {{- if .Values.celery.livenessProbe.enabled }} livenessProbe: exec: diff --git a/helm/oncall/templates/engine/deployment.yaml b/helm/oncall/templates/engine/deployment.yaml index 1f80c137..e94df2ba 100644 --- a/helm/oncall/templates/engine/deployment.yaml +++ b/helm/oncall/templates/engine/deployment.yaml @@ -34,7 +34,7 @@ spec: - name: {{ .Chart.Name }} securityContext: {{- toYaml .Values.securityContext | nindent 12 }} - image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}" + image: {{ include "oncall.engine.image" . }} imagePullPolicy: {{ .Values.image.pullPolicy }} {{- if .Values.oncall.devMode }} command: ["sh", "-c", "uwsgi --disable-logging --py-autoreload 3 --ini uwsgi.ini"] @@ -44,15 +44,7 @@ spec: containerPort: 8080 protocol: TCP env: - {{- include "snippet.oncall.env" . | nindent 12 }} - {{- include "snippet.oncall.slack.env" . | nindent 12 }} - {{- include "snippet.oncall.telegram.env" . | nindent 12 }} - {{- include "snippet.oncall.smtp.env" . | nindent 12 }} - {{- include "snippet.oncall.twilio.env" . | nindent 12 }} - {{- include "snippet.oncall.exporter.env" . | nindent 12 }} - {{- include "snippet.db.env" . | nindent 12 }} - {{- include "snippet.broker.env" . | nindent 12 }} - {{- include "oncall.extraEnvs" . | nindent 12 }} + {{- include "snippet.oncall.engine.env" . | nindent 12 }} livenessProbe: httpGet: path: /health/ diff --git a/helm/oncall/templates/engine/job-migrate.yaml b/helm/oncall/templates/engine/job-migrate.yaml index 861c8308..5f1b716e 100644 --- a/helm/oncall/templates/engine/job-migrate.yaml +++ b/helm/oncall/templates/engine/job-migrate.yaml @@ -58,7 +58,7 @@ spec: - name: {{ .Chart.Name }}-migrate securityContext: {{- toYaml .Values.securityContext | nindent 12 }} - image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}" + image: {{ include "oncall.engine.image" . }} imagePullPolicy: {{ .Values.image.pullPolicy }} command: - /bin/sh diff --git a/helm/oncall/templates/ingress-regular.yaml b/helm/oncall/templates/ingress-regular.yaml index d29756aa..9a5357ff 100644 --- a/helm/oncall/templates/ingress-regular.yaml +++ b/helm/oncall/templates/ingress-regular.yaml @@ -53,4 +53,13 @@ spec: port: number: 80 {{- end }} + {{ if .Values.detached_integrations.enabled }} + - path: /integrations + pathType: Prefix + backend: + service: + name: {{ include "oncall.detached_integrations.fullname" . }} + port: + number: 8080 + {{- end }} {{- end }} diff --git a/helm/oncall/templates/integrations/_helpers.tpl b/helm/oncall/templates/integrations/_helpers.tpl new file mode 100644 index 00000000..6727ed42 --- /dev/null +++ b/helm/oncall/templates/integrations/_helpers.tpl @@ -0,0 +1,26 @@ +{{/* +Maximum of 63 chars because some Kubernetes name fields are limited to this (by the DNS naming spec). +*/}} +{{- define "oncall.detached_integrations.name" -}} +{{ include "oncall.name" . | trunc 55 }}-integrations +{{- end }} + +{{- define "oncall.detached_integrations.fullname" -}} +{{ include "oncall.fullname" . | trunc 55 }}-integrations +{{- end }} + +{{/* +Integrations common labels +*/}} +{{- define "oncall.detached_integrations.labels" -}} +{{ include "oncall.labels" . }} +app.kubernetes.io/component: integrations +{{- end }} + +{{/* +Integrations selector labels +*/}} +{{- define "oncall.detached_integrations.selectorLabels" -}} +{{ include "oncall.selectorLabels" . }} +app.kubernetes.io/component: integrations +{{- end }} diff --git a/helm/oncall/templates/integrations/deployment.yaml b/helm/oncall/templates/integrations/deployment.yaml new file mode 100644 index 00000000..5e08eaf7 --- /dev/null +++ b/helm/oncall/templates/integrations/deployment.yaml @@ -0,0 +1,99 @@ +{{- if .Values.detached_integrations.enabled -}} +apiVersion: apps/v1 +kind: Deployment +metadata: + name: {{ include "oncall.detached_integrations.fullname" . }} + labels: + {{- include "oncall.detached_integrations.labels" . | nindent 4 }} +spec: + replicas: {{ .Values.detached_integrations.replicaCount }} + selector: + matchLabels: + {{- include "oncall.detached_integrations.selectorLabels" . | nindent 6 }} + strategy: + {{- toYaml .Values.detached_integrations.updateStrategy | nindent 4 }} + template: + metadata: + {{- with .Values.podAnnotations }} + annotations: + random-annotation: {{ randAlphaNum 10 | lower }} + {{- toYaml . | nindent 8 }} + {{- end }} + labels: + {{- include "oncall.detached_integrations.selectorLabels" . | nindent 8 }} + spec: + {{- with .Values.imagePullSecrets }} + imagePullSecrets: + {{- toYaml . | nindent 8 }} + {{- end }} + serviceAccountName: {{ include "oncall.serviceAccountName" . }} + securityContext: + {{- toYaml .Values.podSecurityContext | nindent 8 }} + initContainers: + {{- include "oncall.initContainer" . | indent 8 }} + containers: + - name: {{ .Chart.Name }} + securityContext: + {{- toYaml .Values.securityContext | nindent 12 }} + image: {{ include "oncall.engine.image" . }} + imagePullPolicy: {{ .Values.image.pullPolicy }} + {{- if .Values.oncall.devMode }} + command: ["sh", "-c", "uwsgi --disable-logging --py-autoreload 3 --ini uwsgi.ini"] + {{- end }} + ports: + - name: http + containerPort: 8080 + protocol: TCP + env: + {{- include "snippet.oncall.engine.env" . | nindent 12 }} + - name: ROOT_URLCONF + value: "engine.integrations_urls" + livenessProbe: + httpGet: + path: /health/ + port: http + periodSeconds: 60 + timeoutSeconds: 3 + readinessProbe: + httpGet: + path: /ready/ + port: http + periodSeconds: 60 + timeoutSeconds: 3 + startupProbe: + httpGet: + path: /startupprobe/ + port: http + periodSeconds: 10 + timeoutSeconds: 3 + resources: + {{- toYaml .Values.detached_integrations.resources | nindent 12 }} + {{- with .Values.detached_integrations.extraVolumeMounts }} + volumeMounts: {{- . | toYaml | nindent 12 }} + {{- end }} + {{- with .Values.detached_integrations.extraContainers }} + {{- tpl . $ | nindent 8 }} + {{- end }} + {{- with .Values.detached_integrations.nodeSelector }} + nodeSelector: + {{- toYaml . | nindent 8 }} + {{- end }} + {{- with .Values.detached_integrations.affinity }} + affinity: + {{- toYaml . | nindent 8 }} + {{- end }} + {{- with .Values.detached_integrations.tolerations }} + tolerations: + {{- toYaml . | nindent 8 }} + {{- end }} + {{- with .Values.detached_integrations.topologySpreadConstraints }} + topologySpreadConstraints: + {{- toYaml . | nindent 8 }} + {{- end }} + {{- with .Values.detached_integrations.priorityClassName }} + priorityClassName: {{ . }} + {{- end }} + {{- with .Values.detached_integrations.extraVolumes }} + volumes: {{- . | toYaml | nindent 8 }} + {{- end }} +{{- end -}} diff --git a/helm/oncall/templates/integrations/service-external.yaml b/helm/oncall/templates/integrations/service-external.yaml new file mode 100644 index 00000000..455d4aa0 --- /dev/null +++ b/helm/oncall/templates/integrations/service-external.yaml @@ -0,0 +1,24 @@ +{{- if .Values.detached_integrations_service.enabled }} +apiVersion: v1 +kind: Service +metadata: + name: {{ include "oncall.detached_integrations.fullname" . }}-external + labels: + {{- include "oncall.detached_integrations.labels" . | nindent 4 }} + {{- with .Values.detached_integrations_service.annotations }} + annotations: + {{- toYaml . | nindent 4 }} + {{- end }} +spec: + type: {{ .Values.detached_integrations_service.type }} + ports: + - port: {{ .Values.detached_integrations_service.port }} + targetPort: http + protocol: TCP + name: http + {{- if and (eq .Values.detached_integrations_service.type "NodePort") (.Values.detached_integrations_service.nodePort) }} + nodePort: {{ .Values.detached_integrations_service.nodePort }} + {{- end }} + selector: + {{- include "oncall.detached_integrations.selectorLabels" . | nindent 4 }} +{{- end }} diff --git a/helm/oncall/templates/integrations/service-internal.yaml b/helm/oncall/templates/integrations/service-internal.yaml new file mode 100644 index 00000000..ed14d92f --- /dev/null +++ b/helm/oncall/templates/integrations/service-internal.yaml @@ -0,0 +1,15 @@ +apiVersion: v1 +kind: Service +metadata: + name: {{ include "oncall.detached_integrations.fullname" . }} + labels: + {{- include "oncall.detached_integrations.labels" . | nindent 4 }} +spec: + type: ClusterIP + ports: + - port: 8080 + targetPort: http + protocol: TCP + name: http + selector: + {{- include "oncall.detached_integrations.selectorLabels" . | nindent 4 }} diff --git a/helm/oncall/templates/telegram-polling/deployment.yaml b/helm/oncall/templates/telegram-polling/deployment.yaml index aba55385..acf90c97 100644 --- a/helm/oncall/templates/telegram-polling/deployment.yaml +++ b/helm/oncall/templates/telegram-polling/deployment.yaml @@ -28,7 +28,7 @@ spec: - name: telegram-polling securityContext: {{- toYaml .Values.securityContext | nindent 12 }} - image: "{{ .Values.image.repository }}:{{ .Values.image.tag | default .Chart.AppVersion }}" + image: {{ include "oncall.engine.image" . }} imagePullPolicy: {{ .Values.image.pullPolicy }} command: ['sh', '-c', 'python manage.py start_telegram_polling'] env: diff --git a/helm/oncall/tests/__snapshot__/integrations_deployment_test.yaml.snap b/helm/oncall/tests/__snapshot__/integrations_deployment_test.yaml.snap new file mode 100644 index 00000000..b2b31638 --- /dev/null +++ b/helm/oncall/tests/__snapshot__/integrations_deployment_test.yaml.snap @@ -0,0 +1,129 @@ +detached_integrations.enabled=true -> should create integrations deployment: + 1: | + - env: + - name: BASE_URL + value: https://example.com + - name: SECRET_KEY + valueFrom: + secretKeyRef: + key: SECRET_KEY + name: oncall + - name: MIRAGE_SECRET_KEY + valueFrom: + secretKeyRef: + key: MIRAGE_SECRET_KEY + name: oncall + - name: MIRAGE_CIPHER_IV + value: 1234567890abcdef + - name: DJANGO_SETTINGS_MODULE + value: settings.helm + - name: AMIXR_DJANGO_ADMIN_PATH + value: admin + - name: OSS + value: "True" + - name: DETACHED_INTEGRATIONS_SERVER + value: "True" + - name: UWSGI_LISTEN + value: "1024" + - name: BROKER_TYPE + value: rabbitmq + - name: GRAFANA_API_URL + value: http://oncall-grafana + - name: FEATURE_SLACK_INTEGRATION_ENABLED + value: "False" + - name: FEATURE_TELEGRAM_INTEGRATION_ENABLED + value: "False" + - name: FEATURE_EMAIL_INTEGRATION_ENABLED + value: "True" + - name: EMAIL_HOST + value: null + - name: EMAIL_PORT + value: "587" + - name: EMAIL_HOST_USER + value: null + - name: EMAIL_HOST_PASSWORD + valueFrom: + secretKeyRef: + key: smtp-password + name: oncall-smtp + optional: true + - name: EMAIL_USE_TLS + value: "True" + - name: EMAIL_FROM_ADDRESS + value: null + - name: EMAIL_NOTIFICATIONS_LIMIT + value: "200" + - name: FEATURE_PROMETHEUS_EXPORTER_ENABLED + value: "False" + - name: MYSQL_HOST + value: oncall-mariadb + - name: MYSQL_PORT + value: "3306" + - name: MYSQL_DB_NAME + value: oncall + - name: MYSQL_USER + value: root + - name: MYSQL_PASSWORD + valueFrom: + secretKeyRef: + key: mariadb-root-password + name: oncall-mariadb + - name: REDIS_PROTOCOL + value: redis + - name: REDIS_HOST + value: oncall-redis-master + - name: REDIS_PORT + value: "6379" + - name: REDIS_DATABASE + value: "0" + - name: REDIS_USERNAME + value: "" + - name: REDIS_PASSWORD + valueFrom: + secretKeyRef: + key: redis-password + name: oncall-redis + - name: RABBITMQ_USERNAME + value: user + - name: RABBITMQ_PASSWORD + valueFrom: + secretKeyRef: + key: rabbitmq-password + name: oncall-rabbitmq + - name: RABBITMQ_HOST + value: oncall-rabbitmq + - name: RABBITMQ_PORT + value: "5672" + - name: RABBITMQ_PROTOCOL + value: amqp + - name: RABBITMQ_VHOST + value: "" + - name: ROOT_URLCONF + value: engine.integrations_urls + image: grafana/oncall:v1.3.39 + imagePullPolicy: Always + livenessProbe: + httpGet: + path: /health/ + port: http + periodSeconds: 60 + timeoutSeconds: 3 + name: oncall + ports: + - containerPort: 8080 + name: http + protocol: TCP + readinessProbe: + httpGet: + path: /ready/ + port: http + periodSeconds: 60 + timeoutSeconds: 3 + resources: {} + securityContext: {} + startupProbe: + httpGet: + path: /startupprobe/ + port: http + periodSeconds: 10 + timeoutSeconds: 3 diff --git a/helm/oncall/tests/__snapshot__/telegram_polling_deployment_test.yaml.snap b/helm/oncall/tests/__snapshot__/telegram_polling_deployment_test.yaml.snap index c59eac17..341fa32d 100644 --- a/helm/oncall/tests/__snapshot__/telegram_polling_deployment_test.yaml.snap +++ b/helm/oncall/tests/__snapshot__/telegram_polling_deployment_test.yaml.snap @@ -25,6 +25,8 @@ telegramPolling.enabled=true -> should create telegram polling deployment: value: admin - name: OSS value: "True" + - name: DETACHED_INTEGRATIONS_SERVER + value: "False" - name: UWSGI_LISTEN value: "1024" - name: BROKER_TYPE diff --git a/helm/oncall/tests/__snapshot__/wait_for_db_test.yaml.snap b/helm/oncall/tests/__snapshot__/wait_for_db_test.yaml.snap index 7403070b..2cee70e7 100644 --- a/helm/oncall/tests/__snapshot__/wait_for_db_test.yaml.snap +++ b/helm/oncall/tests/__snapshot__/wait_for_db_test.yaml.snap @@ -25,6 +25,8 @@ database.type=mysql -> should create initContainer for MySQL database (default): value: admin - name: OSS value: "True" + - name: DETACHED_INTEGRATIONS_SERVER + value: "False" - name: UWSGI_LISTEN value: "1024" - name: BROKER_TYPE @@ -111,6 +113,8 @@ database.type=mysql -> should create initContainer for MySQL database (default): value: admin - name: OSS value: "True" + - name: DETACHED_INTEGRATIONS_SERVER + value: "False" - name: UWSGI_LISTEN value: "1024" - name: BROKER_TYPE @@ -197,6 +201,8 @@ database.type=mysql -> should create initContainer for MySQL database (default): value: admin - name: OSS value: "True" + - name: DETACHED_INTEGRATIONS_SERVER + value: "False" - name: UWSGI_LISTEN value: "1024" - name: BROKER_TYPE @@ -284,6 +290,8 @@ database.type=postgresql -> should create initContainer for PostgreSQL database: value: admin - name: OSS value: "True" + - name: DETACHED_INTEGRATIONS_SERVER + value: "False" - name: UWSGI_LISTEN value: "1024" - name: BROKER_TYPE @@ -372,6 +380,8 @@ database.type=postgresql -> should create initContainer for PostgreSQL database: value: admin - name: OSS value: "True" + - name: DETACHED_INTEGRATIONS_SERVER + value: "False" - name: UWSGI_LISTEN value: "1024" - name: BROKER_TYPE @@ -460,6 +470,8 @@ database.type=postgresql -> should create initContainer for PostgreSQL database: value: admin - name: OSS value: "True" + - name: DETACHED_INTEGRATIONS_SERVER + value: "False" - name: UWSGI_LISTEN value: "1024" - name: BROKER_TYPE diff --git a/helm/oncall/tests/integrations_deployment_test.yaml b/helm/oncall/tests/integrations_deployment_test.yaml new file mode 100644 index 00000000..51559f17 --- /dev/null +++ b/helm/oncall/tests/integrations_deployment_test.yaml @@ -0,0 +1,52 @@ +suite: test integrations deployment +templates: + - integrations/deployment.yaml +release: + name: oncall +chart: + appVersion: v1.3.39 +tests: + - it: detached_integrations.enabled=false -> should not create deployment (default) + asserts: + - hasDocuments: + count: 0 + + - it: detached_integrations.enabled=true -> should create integrations deployment + set: + detached_integrations.enabled: true + asserts: + - containsDocument: + kind: Deployment + apiVersion: apps/v1 + metadata.name: oncall-integrations + - isSubset: + path: metadata.labels + content: + app.kubernetes.io/component: integrations + app.kubernetes.io/instance: oncall + app.kubernetes.io/name: oncall + - isSubset: + path: spec.selector.matchLabels + content: + app.kubernetes.io/component: integrations + app.kubernetes.io/instance: oncall + app.kubernetes.io/name: oncall + - isSubset: + path: spec.template.metadata.labels + content: + app.kubernetes.io/component: integrations + app.kubernetes.io/instance: oncall + app.kubernetes.io/name: oncall + - equal: + path: spec.replicas + value: 1 + - equal: + path: spec.template.spec.serviceAccountName + value: oncall + - contains: + path: spec.template.spec.initContainers + content: + name: wait-for-db + any: true + - matchSnapshot: + path: spec.template.spec.containers diff --git a/helm/oncall/values.yaml b/helm/oncall/values.yaml index 7d7a5bae..29f715f1 100644 --- a/helm/oncall/values.yaml +++ b/helm/oncall/values.yaml @@ -95,6 +95,82 @@ engine: # - mountPath: /mnt/redis-tls # name: redis-tls + +detached_integrations_service: + enabled: false + type: LoadBalancer + port: 8080 + annotations: {} + +# Integrations pods configuration +detached_integrations: + enabled: false + replicaCount: 1 + resources: + {} + # limits: + # cpu: 100m + # memory: 128Mi + # requests: + # cpu: 100m + # memory: 128Mi + + ## Deployment update strategy + ## ref: https://kubernetes.io/docs/concepts/workloads/controllers/deployment/#strategy + updateStrategy: + rollingUpdate: + maxSurge: 25% + maxUnavailable: 0 + type: RollingUpdate + + ## Affinity for pod assignment + ## ref: https://kubernetes.io/docs/concepts/configuration/assign-pod-node/#affinity-and-anti-affinity + affinity: {} + + ## Node labels for pod assignment + ## ref: https://kubernetes.io/docs/user-guide/node-selection/ + nodeSelector: {} + + ## Tolerations for pod assignment + ## ref: https://kubernetes.io/docs/concepts/configuration/taint-and-toleration/ + tolerations: [] + + ## Topology spread constraints for pod assignment + ## ref: https://kubernetes.io/docs/concepts/scheduling-eviction/topology-spread-constraints/ + topologySpreadConstraints: [] + + ## Priority class for the pods + ## ref: https://kubernetes.io/docs/concepts/scheduling-eviction/pod-priority-preemption/ + priorityClassName: "" + + # Extra containers which runs as sidecar + extraContainers: "" + # extraContainers: | + # - name: cloud-sql-proxy + # image: gcr.io/cloud-sql-connectors/cloud-sql-proxy:2.1.2 + # args: + # - --private-ip + # - --port=5432 + # - example:europe-west3:grafana-oncall-db + + # Extra volume mounts for the container + extraVolumeMounts: [] + # - name: postgres-tls + # configMap: + # name: my-postgres-tls + # defaultMode: 0640 + # - name: redis-tls + # configMap: + # name: my-redis-tls + # defaultMode: 0640 + + # Extra volumes for the pod + extraVolumes: [] + # - mountPath: /mnt/postgres-tls + # name: postgres-tls + # - mountPath: /mnt/redis-tls + # name: redis-tls + # Celery workers pods configuration celery: replicaCount: 1 diff --git a/helm/simple.yml b/helm/simple.yml index 84456649..219c7a11 100644 --- a/helm/simple.yml +++ b/helm/simple.yml @@ -14,6 +14,13 @@ grafana: service: type: NodePort nodePort: 30002 +detached_integrations: + enabled: true +detached_integrations_service: + enabled: true + type: NodePort + port: 8080 + nodePort: 30003 database: # can be either mysql or postgresql type: postgresql From 929c07825223aa354dedad50b54bf783c2166f0a Mon Sep 17 00:00:00 2001 From: "dependabot[bot]" <49699333+dependabot[bot]@users.noreply.github.com> Date: Fri, 3 Nov 2023 13:22:38 +0000 Subject: [PATCH 06/12] Bump django from 4.2.6 to 4.2.7 in /engine (#3263) Bumps [django](https://github.com/django/django) from 4.2.6 to 4.2.7.
Commits
  • d254a54 [4.2.x] Bumped version for 4.2.7 release.
  • 048a9eb [4.2.x] Fixed CVE-2023-46695 -- Fixed potential DoS in UsernameField on Windows.
  • 3fae5d9 [4.2.x] Refs #30601 -- Fixed typos in docs/topics/db/transactions.txt.
  • a8aa940 [4.2.x] Refs #15578 -- Made cosmetic edits to fixtures docs.
  • 109f39a [4.2.x] Fixed #34932 -- Restored varchar_pattern_ops/text_pattern_ops index c...
  • 6161299 [4.2.x] Fixed typos in docs/ref/models/expressions.txt.
  • 696fbc3 [4.2.x] Fixed #30601 -- Doc'd the need to manually revert all app state on tr...
  • ffba631 [4.2.x] Fixed typo in docs/ref/contrib/gis/geos.txt.
  • 43a3646 [4.2.x] Fixed #15578 -- Stated the processing order of fixtures in the fixtur...
  • 0cd8b86 [4.2.x] Added stub release notes and release date for 4.2.7, 4.1.13, and 3.2.23.
  • Additional commits viewable in compare view

[![Dependabot compatibility score](https://dependabot-badges.githubapp.com/badges/compatibility_score?dependency-name=django&package-manager=pip&previous-version=4.2.6&new-version=4.2.7)](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) ---
Dependabot commands and options
You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot merge` will merge this PR after your CI passes on it - `@dependabot squash and merge` will squash and merge this PR after your CI passes on it - `@dependabot cancel merge` will cancel a previously requested merge and block automerging - `@dependabot reopen` will reopen this PR if it is closed - `@dependabot close` will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually - `@dependabot show ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/grafana/oncall/network/alerts).
Signed-off-by: dependabot[bot] Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Joey Orlando --- engine/requirements.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/requirements.txt b/engine/requirements.txt index e6ac5e73..87aabd88 100644 --- a/engine/requirements.txt +++ b/engine/requirements.txt @@ -1,4 +1,4 @@ -django==4.2.6 +django==4.2.7 djangorestframework==3.14.0 slack_sdk==3.21.3 whitenoise==5.3.0 From fb3bc0d7e575abf049c55a0e77e9b8f67b806b2f Mon Sep 17 00:00:00 2001 From: Matias Bordese Date: Fri, 3 Nov 2023 12:30:51 -0300 Subject: [PATCH 07/12] Add missing referenced local_image.yml for local helm setup (#3268) Forgot to add the `local_image.yml` file referenced [here](https://github.com/grafana/oncall/pull/3204/files#diff-ba66b531f5db27e2cb364be1b4cda073681f7447f088ac98139ca96c1609dc16R30). --- helm/local_image.yml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 helm/local_image.yml diff --git a/helm/local_image.yml b/helm/local_image.yml new file mode 100644 index 00000000..c78283f5 --- /dev/null +++ b/helm/local_image.yml @@ -0,0 +1,6 @@ +image: + repository: oncall/engine + tag: latest + pullPolicy: IfNotPresent +oncall: + devMode: true From 2cbb20601ea3534cdd5ed5f0126e8d4e165b5e4d Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Fri, 3 Nov 2023 12:40:54 -0400 Subject: [PATCH 08/12] Improve performance of GET /users and GET /teams endpoints used by add responders popup (#3241) # What this PR does - Improve performance of the specific `GET /users` and `GET /teams` calls that're made by the Add Responders dropdown in the UI - Add `GET /team/{teamId}` internal API route (needed by Grafana Incident team for their Add Responders changes) - Some UI improvements to the Add Responders popup (loading state + pre-fetch users and teams when the drawer is opened) - Re-enable django-admin only if `settings.SILK_PROFILER_ENABLED == True` (need to be able to log into django admin to auth to silk routes) Closes #3231 Closes https://github.com/grafana/oncall-private/issues/2252 ## 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] `CHANGELOG.md` updated (or `pr:no changelog` PR label added if not required) --- engine/apps/alerts/paging.py | 25 -- engine/apps/alerts/tests/test_paging.py | 65 ---- engine/apps/api/serializers/user.py | 11 +- engine/apps/api/tests/test_team.py | 34 +- engine/apps/api/tests/test_user.py | 2 +- engine/apps/api/views/team.py | 22 +- engine/apps/api/views/user.py | 19 +- engine/apps/slack/scenarios/paging.py | 41 ++- .../tests/test_scenario_steps/test_paging.py | 88 +++++ .../user_management/models/organization.py | 35 +- .../tests/test_organization.py | 99 ++++-- engine/engine/urls.py | 7 +- engine/settings/base.py | 5 +- .../AddResponders/AddResponders.tsx | 6 +- .../AddResponders/AddResponders.types.ts | 4 +- .../AddRespondersPopup.module.scss | 4 + .../AddRespondersPopup.test.tsx | 33 +- .../AddRespondersPopup/AddRespondersPopup.tsx | 127 +++++-- .../AddRespondersPopup.test.tsx.snap | 329 +----------------- .../UserResponder/UserResponder.test.tsx | 4 +- .../src/models/alertgroup/alertgroup.types.ts | 6 +- .../src/models/direct_paging/direct_paging.ts | 4 +- .../src/models/user/user.helpers.tsx | 2 +- grafana-plugin/src/models/user/user.ts | 16 +- grafana-plugin/src/models/user/user.types.ts | 25 +- 25 files changed, 428 insertions(+), 585 deletions(-) diff --git a/engine/apps/alerts/paging.py b/engine/apps/alerts/paging.py index 42a996f7..d8240940 100644 --- a/engine/apps/alerts/paging.py +++ b/engine/apps/alerts/paging.py @@ -197,28 +197,3 @@ def unpage_user(alert_group: AlertGroup, user: User, from_user: User) -> None: def user_is_oncall(user: User) -> bool: schedules_with_oncall_users = get_oncall_users_for_multiple_schedules(OnCallSchedule.objects.related_to_user(user)) return user.pk in {user.pk for _, users in schedules_with_oncall_users.items() for user in users} - - -def integration_is_notifiable(integration: AlertReceiveChannel) -> bool: - """ - Returns true if: - - the integration has more than one channel filter associated with it - - the default channel filter has at least one notification method specified or an escalation chain associated with it - """ - if integration.channel_filters.count() > 1: - return True - - default_channel_filter = integration.default_channel_filter - if not default_channel_filter: - return False - - organization = integration.organization - notify_via_slack = organization.slack_is_configured and default_channel_filter.notify_in_slack - notify_via_telegram = organization.telegram_is_configured and default_channel_filter.notify_in_telegram - - notify_via_chatops = notify_via_slack or notify_via_telegram - custom_messaging_backend_configured = default_channel_filter.notification_backends is not None - - return ( - default_channel_filter.escalation_chain is not None or notify_via_chatops or custom_messaging_backend_configured - ) diff --git a/engine/apps/alerts/tests/test_paging.py b/engine/apps/alerts/tests/test_paging.py index 96d5b746..2c0ae9f8 100644 --- a/engine/apps/alerts/tests/test_paging.py +++ b/engine/apps/alerts/tests/test_paging.py @@ -8,7 +8,6 @@ from apps.alerts.paging import ( DirectPagingUserTeamValidationError, _construct_title, direct_paging, - integration_is_notifiable, unpage_user, user_is_oncall, ) @@ -292,67 +291,3 @@ def test_construct_title(make_organization, make_team, make_user_for_organizatio assert _construct_title(from_user, team, multiple_users) == _title( f"{team.name}, {user1.username}, {user2.username} and {user3.username}" ) - - -@pytest.mark.django_db -def test_integration_is_notifiable( - make_organization, - make_alert_receive_channel, - make_channel_filter, - make_escalation_chain, - make_slack_team_identity, - make_telegram_channel, -): - organization = make_organization() - - # integration has no default channel filter - arc = make_alert_receive_channel(organization) - make_channel_filter(arc, is_default=False) - assert integration_is_notifiable(arc) is False - - # integration has more than one channel filter - arc = make_alert_receive_channel(organization) - make_channel_filter(arc, is_default=False) - make_channel_filter(arc, is_default=False) - assert integration_is_notifiable(arc) is True - - # integration's default channel filter is setup to notify via slack but Slack is not configured for the org - arc = make_alert_receive_channel(organization) - make_channel_filter(arc, is_default=True, notify_in_slack=True) - assert integration_is_notifiable(arc) is False - - # integration's default channel filter is setup to notify via slack and Slack is configured for the org - arc = make_alert_receive_channel(organization) - slack_team_identity = make_slack_team_identity() - organization.slack_team_identity = slack_team_identity - organization.save() - - make_channel_filter(arc, is_default=True, notify_in_slack=True) - assert integration_is_notifiable(arc) is True - - # integration's default channel filter is setup to notify via telegram but Telegram is not configured for the org - arc = make_alert_receive_channel(organization) - make_channel_filter(arc, is_default=True, notify_in_slack=False, notify_in_telegram=True) - assert integration_is_notifiable(arc) is False - - # integration's default channel filter is setup to notify via telegram and Telegram is configured for the org - arc = make_alert_receive_channel(organization) - make_channel_filter(arc, is_default=True, notify_in_slack=False, notify_in_telegram=True) - make_telegram_channel(organization) - assert integration_is_notifiable(arc) is True - - # integration's default channel filter is contactable via a custom messaging backend - arc = make_alert_receive_channel(organization) - make_channel_filter( - arc, - is_default=True, - notify_in_slack=False, - notification_backends={"MSTEAMS": {"channel": "test", "enabled": True}}, - ) - assert integration_is_notifiable(arc) is True - - # integration's default channel filter has an escalation chain attached to it - arc = make_alert_receive_channel(organization) - escalation_chain = make_escalation_chain(organization) - make_channel_filter(arc, is_default=True, notify_in_slack=False, escalation_chain=escalation_chain) - assert integration_is_notifiable(arc) is True diff --git a/engine/apps/api/serializers/user.py b/engine/apps/api/serializers/user.py index a0332dc7..2fd9fd91 100644 --- a/engine/apps/api/serializers/user.py +++ b/engine/apps/api/serializers/user.py @@ -270,14 +270,19 @@ class UserShortSerializer(serializers.ModelSerializer): ] -class UserLongSerializer(UserSerializer): +class UserIsCurrentlyOnCallSerializer(UserShortSerializer, EagerLoadingMixin): context: UserSerializerContext teams = FastTeamSerializer(read_only=True, many=True) is_currently_oncall = serializers.SerializerMethodField() - class Meta(UserSerializer.Meta): - fields = UserSerializer.Meta.fields + [ + SELECT_RELATED = ["organization"] + PREFETCH_RELATED = ["teams"] + + class Meta(UserShortSerializer.Meta): + fields = UserShortSerializer.Meta.fields + [ + "name", + "timezone", "teams", "is_currently_oncall", ] diff --git a/engine/apps/api/tests/test_team.py b/engine/apps/api/tests/test_team.py index 69a8afbe..f66b4436 100644 --- a/engine/apps/api/tests/test_team.py +++ b/engine/apps/api/tests/test_team.py @@ -28,6 +28,25 @@ def get_payload_from_team(team, long=False): return payload +@pytest.mark.django_db +def test_get_team(make_organization_and_user_with_plugin_token, make_team, make_user_auth_headers): + organization, user, token = make_organization_and_user_with_plugin_token() + team = make_team(organization) + + client = APIClient() + + # team exists + url = reverse("api-internal:team-detail", kwargs={"pk": team.public_primary_key}) + response = client.get(url, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_200_OK + assert response.json() == get_payload_from_team(team) + + # 404 scenario + url = reverse("api-internal:team-detail", kwargs={"pk": "asdfasdflkjlkajsdf"}) + response = client.get(url, format="json", **make_user_auth_headers(user, token)) + assert response.status_code == status.HTTP_404_NOT_FOUND + + @pytest.mark.django_db def test_list_teams( make_organization, @@ -100,7 +119,20 @@ def test_list_teams_only_include_notifiable_teams( client = APIClient() url = reverse("api-internal:team-list") - with patch("apps.api.views.team.integration_is_notifiable", side_effect=lambda obj: obj.id == arc1.id): + def mock_get_notifiable_direct_paging_integrations(): + class MockRelatedManager: + def filter(self, *args, **kwargs): + return self + + def values_list(self, *args, **kwargs): + return [arc1.team.pk] + + return MockRelatedManager() + + with patch( + "apps.user_management.models.Organization.get_notifiable_direct_paging_integrations", + side_effect=mock_get_notifiable_direct_paging_integrations, + ): response = client.get( f"{url}?only_include_notifiable_teams=true&include_no_team=false", format="json", diff --git a/engine/apps/api/tests/test_user.py b/engine/apps/api/tests/test_user.py index e918762c..8c15db34 100644 --- a/engine/apps/api/tests/test_user.py +++ b/engine/apps/api/tests/test_user.py @@ -1935,7 +1935,7 @@ def test_users_is_currently_oncall_attribute_works_properly( schedule.refresh_ical_final_schedule() client = APIClient() - url = f"{reverse('api-internal:user-list')}?short=false" + url = f"{reverse('api-internal:user-list')}?is_currently_oncall=all" response = client.get(url, format="json", **make_user_auth_headers(user1, token)) oncall_statuses = { diff --git a/engine/apps/api/views/team.py b/engine/apps/api/views/team.py index 47c10e11..72c10770 100644 --- a/engine/apps/api/views/team.py +++ b/engine/apps/api/views/team.py @@ -4,7 +4,6 @@ from rest_framework.filters import SearchFilter from rest_framework.permissions import IsAuthenticated from rest_framework.response import Response -from apps.alerts.paging import integration_is_notifiable from apps.api.permissions import RBACPermission from apps.api.serializers.team import TeamLongSerializer, TeamSerializer from apps.auth_token.auth import PluginAuthentication @@ -14,7 +13,13 @@ from apps.user_management.models import Team from common.api_helpers.mixins import PublicPrimaryKeyMixin -class TeamViewSet(PublicPrimaryKeyMixin, mixins.ListModelMixin, mixins.UpdateModelMixin, viewsets.GenericViewSet): +class TeamViewSet( + PublicPrimaryKeyMixin, + mixins.ListModelMixin, + mixins.RetrieveModelMixin, + mixins.UpdateModelMixin, + viewsets.GenericViewSet, +): authentication_classes = ( MobileAppAuthTokenAuthentication, PluginAuthentication, @@ -61,14 +66,11 @@ class TeamViewSet(PublicPrimaryKeyMixin, mixins.ListModelMixin, mixins.UpdateMod queryset = self.filter_queryset(self.get_queryset()) if self.request.query_params.get("only_include_notifiable_teams", "false") == "true": - # filters down to only teams that have a direct paging integration that is "notifiable" - orgs_direct_paging_integrations = self.request.user.organization.get_direct_paging_integrations() - notifiable_direct_paging_integrations = [ - i for i in orgs_direct_paging_integrations if integration_is_notifiable(i) - ] - team_ids = [i.team.pk for i in notifiable_direct_paging_integrations if i.team is not None] - - queryset = queryset.filter(pk__in=team_ids) + queryset = queryset.filter( + pk__in=self.request.user.organization.get_notifiable_direct_paging_integrations() + .filter(team__isnull=False) + .values_list("team__pk", flat=True) + ) queryset = queryset.order_by("name") diff --git a/engine/apps/api/views/user.py b/engine/apps/api/views/user.py index c6a51fc7..06076e44 100644 --- a/engine/apps/api/views/user.py +++ b/engine/apps/api/views/user.py @@ -29,7 +29,7 @@ from apps.api.serializers.user import ( CurrentUserSerializer, FilterUserSerializer, UserHiddenFieldsSerializer, - UserLongSerializer, + UserIsCurrentlyOnCallSerializer, UserSerializer, ) from apps.api.throttlers import ( @@ -238,20 +238,14 @@ class UserView( return self.request.query_params.get("is_currently_oncall", "").lower() def _is_currently_oncall_request(self) -> bool: - return self._get_is_currently_oncall_query_param() in ["true", "false"] - - def _is_long_request(self) -> bool: - return self.request.query_params.get("short", "true").lower() == "false" - - def _is_currently_oncall_or_long_request(self) -> bool: - return self._is_currently_oncall_request() or self._is_long_request() + return self._get_is_currently_oncall_query_param() in ["true", "false", "all"] def get_serializer_context(self): context = super().get_serializer_context() context.update( { "schedules_with_oncall_users": self.schedules_with_oncall_users - if self._is_currently_oncall_or_long_request() + if self._is_currently_oncall_request() else {} } ) @@ -268,8 +262,8 @@ class UserView( if is_list_request and is_filters_request: return self.get_filter_serializer_class() - elif is_list_request and self._is_currently_oncall_or_long_request(): - return UserLongSerializer + elif is_list_request and self._is_currently_oncall_request(): + return UserIsCurrentlyOnCallSerializer is_users_own_data = kwargs.get("pk") is not None and kwargs.get("pk") == user.public_primary_key has_admin_permission = user_is_authorized(user, [RBACPermission.Permissions.USER_SETTINGS_ADMIN]) @@ -296,8 +290,7 @@ class UserView( def _get_oncall_user_ids(): return {user.pk for _, users in self.schedules_with_oncall_users.items() for user in users} - is_currently_oncall_query_param = self._get_is_currently_oncall_query_param() - if is_currently_oncall_query_param == "true": + if (is_currently_oncall_query_param := self._get_is_currently_oncall_query_param()) == "true": # client explicitly wants to filter out users that are on-call queryset = queryset.filter(pk__in=_get_oncall_user_ids()) elif is_currently_oncall_query_param == "false": diff --git a/engine/apps/slack/scenarios/paging.py b/engine/apps/slack/scenarios/paging.py index dd40ac62..9bd54498 100644 --- a/engine/apps/slack/scenarios/paging.py +++ b/engine/apps/slack/scenarios/paging.py @@ -534,26 +534,54 @@ def _get_team_select_blocks( slack_user_identity: "SlackUserIdentity", organization: "Organization", is_selected: bool, - value: "Team", + value: typing.Optional["Team"], input_id_prefix: str, ) -> Block.AnyBlocks: + blocks: Block.AnyBlocks = [] user = slack_user_identity.get_user(organization) # TODO: handle None - teams = user.available_teams + teams = ( + user.organization.get_notifiable_direct_paging_integrations() + .filter(team__isnull=False) + .values_list("team__pk", "team__name") + ) + + direct_paging_info_msg = { + "type": "context", + "elements": [ + { + "type": "mrkdwn", + "text": ( + "*Note*: You can only page teams which have a Direct Paging integration that is configured. " + "" + ), + }, + ], + } + + if not teams: + direct_paging_info_msg["elements"][0][ + "text" + ] += ". There are currently no teams which have a Direct Paging integration that is configured." + blocks.append(direct_paging_info_msg) + return blocks team_options: typing.List[CompositionObjectOption] = [] initial_option_idx = 0 for idx, team in enumerate(teams): - if team == value: + team_pk, team_name = team + team_pk_str = str(team_pk) + + if value == team_pk_str: initial_option_idx = idx team_options.append( { "text": { "type": "plain_text", - "text": f"{team.name}", + "text": team_name, "emoji": True, }, - "value": f"{team.pk}", + "value": team_pk_str, } ) @@ -574,10 +602,11 @@ def _get_team_select_blocks( "optional": True, } - blocks: Block.AnyBlocks = [team_select] + blocks.append(team_select) # No context block if no team selected if not is_selected: + blocks.append(direct_paging_info_msg) return blocks team_select["element"]["initial_option"] = team_options[initial_option_idx] diff --git a/engine/apps/slack/tests/test_scenario_steps/test_paging.py b/engine/apps/slack/tests/test_scenario_steps/test_paging.py index 91e6cf50..2821b8e6 100644 --- a/engine/apps/slack/tests/test_scenario_steps/test_paging.py +++ b/engine/apps/slack/tests/test_scenario_steps/test_paging.py @@ -4,6 +4,7 @@ from unittest.mock import patch import pytest from django.utils import timezone +from apps.alerts.models import AlertReceiveChannel from apps.schedules.models import CustomOnCallShift, OnCallScheduleWeb from apps.slack.scenarios.paging import ( DIRECT_PAGING_MESSAGE_INPUT_ID, @@ -19,6 +20,7 @@ from apps.slack.scenarios.paging import ( Policy, StartDirectPaging, _get_organization_select, + _get_team_select_blocks, ) from apps.user_management.models import Organization @@ -243,6 +245,20 @@ def test_trigger_paging_additional_responders(make_organization_and_user_with_sl mock_direct_paging.called_once_with(organization, user, "The Message", team, [(user, True)]) +@pytest.mark.django_db +def test_page_team(make_organization_and_user_with_slack_identities, make_team): + organization, user, slack_team_identity, slack_user_identity = make_organization_and_user_with_slack_identities() + team = make_team(organization) + payload = make_slack_payload(organization=organization, team=team) + + step = FinishDirectPaging(slack_team_identity) + with patch("apps.slack.scenarios.paging.direct_paging") as mock_direct_paging: + with patch.object(step._slack_client, "api_call"): + step.process_scenario(slack_user_identity, slack_team_identity, payload) + + mock_direct_paging.called_once_with(organization, user, "The Message", team) + + @pytest.mark.django_db def test_get_organization_select(make_organization): organization = make_organization(org_title="Organization", stack_slug="stack_slug") @@ -251,3 +267,75 @@ def test_get_organization_select(make_organization): assert len(select["element"]["options"]) == 1 assert select["element"]["options"][0]["value"] == str(organization.pk) assert select["element"]["options"][0]["text"]["text"] == "Organization (stack_slug)" + + +@pytest.mark.django_db +def test_get_team_select_blocks( + make_organization_and_user_with_slack_identities, + make_team, + make_alert_receive_channel, + make_escalation_chain, + make_channel_filter, +): + info_msg = ( + "*Note*: You can only page teams which have a Direct Paging integration that is configured. " + "" + ) + + input_id_prefix = "nmxcnvmnxv" + + # no team selected - no team direct paging integrations available + organization, _, _, slack_user_identity = make_organization_and_user_with_slack_identities() + blocks = _get_team_select_blocks(slack_user_identity, organization, False, None, input_id_prefix) + + assert len(blocks) == 1 + + context_block = blocks[0] + assert context_block["type"] == "context" + assert ( + context_block["elements"][0]["text"] + == info_msg + ". There are currently no teams which have a Direct Paging integration that is configured." + ) + + # no team selected - 1 team direct paging integration available + organization, _, _, slack_user_identity = make_organization_and_user_with_slack_identities() + team = make_team(organization) + arc = make_alert_receive_channel(organization, team=team, integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING) + escalation_chain = make_escalation_chain(organization) + make_channel_filter(arc, is_default=True, escalation_chain=escalation_chain) + + blocks = _get_team_select_blocks(slack_user_identity, organization, False, None, input_id_prefix) + + assert len(blocks) == 2 + input_block, context_block = blocks + + team_option = {"text": {"emoji": True, "text": team.name, "type": "plain_text"}, "value": str(team.pk)} + + assert input_block["type"] == "input" + assert len(input_block["element"]["options"]) == 1 + assert input_block["element"]["options"] == [team_option] + assert context_block["elements"][0]["text"] == info_msg + + # team selected + organization, _, _, slack_user_identity = make_organization_and_user_with_slack_identities() + team = make_team(organization) + arc = make_alert_receive_channel(organization, team=team, integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING) + escalation_chain = make_escalation_chain(organization) + make_channel_filter(arc, is_default=True, escalation_chain=escalation_chain) + + blocks = _get_team_select_blocks(slack_user_identity, organization, True, team.pk, input_id_prefix) + + assert len(blocks) == 2 + input_block, context_block = blocks + + team_option = {"text": {"emoji": True, "text": team.name, "type": "plain_text"}, "value": str(team.pk)} + + assert input_block["type"] == "input" + assert len(input_block["element"]["options"]) == 1 + assert input_block["element"]["options"] == [team_option] + assert input_block["element"]["initial_option"] == team_option + + assert ( + context_block["elements"][0]["text"] + == f"Integration <{arc.web_link}|{arc.verbal_name}> will be used for notification." + ) diff --git a/engine/apps/user_management/models/organization.py b/engine/apps/user_management/models/organization.py index 5a74531a..70fcf8e0 100644 --- a/engine/apps/user_management/models/organization.py +++ b/engine/apps/user_management/models/organization.py @@ -6,6 +6,7 @@ from urllib.parse import urljoin from django.conf import settings from django.core.validators import MinLengthValidator from django.db import models +from django.db.models import Count, Q from django.utils import timezone from mirage import fields as mirage_fields @@ -298,10 +299,32 @@ class Organization(MaintainableObject): new_channel=channel_name, ) - def get_direct_paging_integrations(self) -> "RelatedManager['AlertReceiveChannel']": + def get_notifiable_direct_paging_integrations(self) -> "RelatedManager['AlertReceiveChannel']": + """ + in layman's terms, this filters down an organization's integrations to ones which meet the following criterias: + - the integration is a direct paging integration + + AND at-least one of the following conditions are true for the integration: + - have more than one channel filter associated with it + - OR the organization has either Slack or Telegram configured (as the direct paging integration + would automatically be configured to be notified via these channel(s)) + - OR the default channel filter associated with the integration has an escalation chain associated with it + - OR the default channel filter associated with the integration is contactable via a custom + messaging backend + """ from apps.alerts.models import AlertReceiveChannel - return self.alert_receive_channels.filter(integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING) + return self.alert_receive_channels.annotate( + num_channel_filters=Count("channel_filters"), + # used to determine if the organization has telegram configured + num_org_telegram_channels=Count("organization__telegram_channel"), + ).filter( + Q(num_channel_filters__gt=1) + | (Q(organization__slack_team_identity__isnull=False) | Q(num_org_telegram_channels__gt=0)) + | Q(channel_filters__is_default=True, channel_filters__escalation_chain__isnull=False) + | Q(channel_filters__is_default=True, channel_filters__notification_backends__isnull=False), + integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING, + ) @property def web_link(self): @@ -312,14 +335,6 @@ class Organization(MaintainableObject): # It's a workaround to pass some unique identifier to the oncall gateway while proxying telegram requests return urljoin(self.grafana_url, f"a/grafana-oncall-app/?oncall-uuid={self.uuid}") - @property - def slack_is_configured(self) -> bool: - return self.slack_team_identity is not None - - @property - def telegram_is_configured(self) -> bool: - return self.telegram_channel.count() > 0 - @classmethod def __str__(self): return f"{self.pk}: {self.org_title}" diff --git a/engine/apps/user_management/tests/test_organization.py b/engine/apps/user_management/tests/test_organization.py index 692eae90..91915500 100644 --- a/engine/apps/user_management/tests/test_organization.py +++ b/engine/apps/user_management/tests/test_organization.py @@ -198,47 +198,74 @@ def test_organization_hard_delete( @pytest.mark.django_db -def test_slack_is_configured(make_organization, make_slack_team_identity): - organization = make_organization() +def test_get_notifiable_direct_paging_integrations( + make_organization, + make_alert_receive_channel, + make_channel_filter, + make_escalation_chain, + make_slack_team_identity, + make_telegram_channel, +): + def _make_org_and_arc(**arc_kwargs): + org = make_organization() + arc = make_alert_receive_channel(org, integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING, **arc_kwargs) + return org, arc - assert organization.slack_is_configured is False + def _assert(org, arc, should_be_returned=True): + notifiable_direct_paging_integrations = org.get_notifiable_direct_paging_integrations() + if should_be_returned: + assert arc in notifiable_direct_paging_integrations + else: + assert arc not in notifiable_direct_paging_integrations + + # integration has no default channel filter + org, arc = _make_org_and_arc() + make_channel_filter(arc, is_default=False) + _assert(org, arc, should_be_returned=False) + + # integration has more than one channel filter + org, arc = _make_org_and_arc() + make_channel_filter(arc, is_default=False) + make_channel_filter(arc, is_default=False) + _assert(org, arc) + + # integration's default channel filter is setup to notify via slack but Slack is not configured for the org + org, arc = _make_org_and_arc() + make_channel_filter(arc, is_default=True, notify_in_slack=True) + _assert(org, arc, should_be_returned=False) + + # integration's default channel filter is setup to notify via slack and Slack is configured for the org + org, arc = _make_org_and_arc() slack_team_identity = make_slack_team_identity() - organization.slack_team_identity = slack_team_identity - organization.save() - assert organization.slack_is_configured is True + org.slack_team_identity = slack_team_identity + org.save() + make_channel_filter(arc, is_default=True, notify_in_slack=True) + _assert(org, arc) -@pytest.mark.django_db -def test_telegram_is_configured(make_organization, make_telegram_channel): - organization = make_organization() - assert organization.telegram_is_configured is False - make_telegram_channel(organization) - assert organization.telegram_is_configured is True + # integration's default channel filter is setup to notify via telegram but Telegram is not configured for the org + org, arc = _make_org_and_arc() + make_channel_filter(arc, is_default=True, notify_in_slack=False, notify_in_telegram=True) + _assert(org, arc, should_be_returned=False) + # integration's default channel filter is setup to notify via telegram and Telegram is configured for the org + org, arc = _make_org_and_arc() + make_channel_filter(arc, is_default=True, notify_in_slack=False, notify_in_telegram=True) + make_telegram_channel(org) + _assert(org, arc) -@pytest.mark.django_db -def test_get_direct_paging_integrations(make_organization, make_team, make_alert_receive_channel): - org1 = make_organization() - org1_team1 = make_team(org1) - org1_team2 = make_team(org1) - - org2 = make_organization() - - org1_direct_paging_integration1 = make_alert_receive_channel( - org1, integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING, team=org1_team1 - ) - org1_direct_paging_integration2 = make_alert_receive_channel( - org1, integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING, team=org1_team2 + # integration's default channel filter is contactable via a custom messaging backend + org, arc = _make_org_and_arc() + make_channel_filter( + arc, + is_default=True, + notify_in_slack=False, + notification_backends={"MSTEAMS": {"channel": "test", "enabled": True}}, ) + _assert(org, arc) - make_alert_receive_channel(org1, integration=AlertReceiveChannel.INTEGRATION_ALERTMANAGER) - make_alert_receive_channel(org2, integration=AlertReceiveChannel.INTEGRATION_DIRECT_PAGING) - - org1_direct_paging_integrations = org1.get_direct_paging_integrations() - org2_direct_paging_integrations = org2.get_direct_paging_integrations() - - assert len(org1_direct_paging_integrations) == 2 - assert len(org2_direct_paging_integrations) == 1 - - assert org1_direct_paging_integration1 in org1_direct_paging_integrations - assert org1_direct_paging_integration2 in org1_direct_paging_integrations + # integration's default channel filter has an escalation chain attached to it + org, arc = _make_org_and_arc() + escalation_chain = make_escalation_chain(org) + make_channel_filter(arc, is_default=True, notify_in_slack=False, escalation_chain=escalation_chain) + _assert(org, arc) diff --git a/engine/engine/urls.py b/engine/engine/urls.py index c86a9a74..ff2d1c6d 100644 --- a/engine/engine/urls.py +++ b/engine/engine/urls.py @@ -15,6 +15,7 @@ Including another URLconf """ from django.conf import settings from django.conf.urls.static import static +from django.contrib import admin from django.urls import URLPattern, URLResolver, include, path from .views import HealthCheckView, MaintenanceModeStatusView, ReadinessCheckView, StartupProbeView @@ -76,7 +77,11 @@ if settings.DEBUG: ] + urlpatterns if settings.SILK_PROFILER_ENABLED: - urlpatterns += [path(settings.SILK_PATH, include("silk.urls", namespace="silk"))] + urlpatterns += [ + # need django admin enabled to be able to access silk + path(settings.ONCALL_DJANGO_ADMIN_PATH, admin.site.urls), + path(settings.SILK_PATH, include("silk.urls", namespace="silk")), + ] if settings.DRF_SPECTACULAR_ENABLED: from drf_spectacular.views import SpectacularAPIView, SpectacularSwaggerView diff --git a/engine/settings/base.py b/engine/settings/base.py index 100ff125..cd58cb26 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -11,7 +11,6 @@ from common.utils import getenv_boolean, getenv_integer, getenv_list VERSION = "dev-oss" SEND_ANONYMOUS_USAGE_STATS = getenv_boolean("SEND_ANONYMOUS_USAGE_STATS", default=True) -ADMIN_ENABLED = False # disable django admin panel # License is OpenSource or Cloud OPEN_SOURCE_LICENSE_NAME = "OpenSource" @@ -590,6 +589,10 @@ SELF_IP = os.environ.get("SELF_IP") SILK_PROFILER_ENABLED = getenv_boolean("SILK_PROFILER_ENABLED", default=False) and not IS_IN_MAINTENANCE_MODE +# django admin panel is required to auth with django silk. Otherwise if silk isn't enabled, we don't need it. +ONCALL_DJANGO_ADMIN_PATH = os.environ.get("ONCALL_DJANGO_ADMIN_PATH", "django-admin") + "/" +ADMIN_ENABLED = SILK_PROFILER_ENABLED + if SILK_PROFILER_ENABLED: SILK_PATH = os.environ.get("SILK_PATH", "silk/") SILKY_INTERCEPT_PERCENT = getenv_integer("SILKY_INTERCEPT_PERCENT", 100) diff --git a/grafana-plugin/src/containers/AddResponders/AddResponders.tsx b/grafana-plugin/src/containers/AddResponders/AddResponders.tsx index b7249d4e..515bbe7a 100644 --- a/grafana-plugin/src/containers/AddResponders/AddResponders.tsx +++ b/grafana-plugin/src/containers/AddResponders/AddResponders.tsx @@ -11,7 +11,7 @@ import Text from 'components/Text/Text'; import { WithPermissionControlTooltip } from 'containers/WithPermissionControl/WithPermissionControlTooltip'; import { Alert as AlertType } from 'models/alertgroup/alertgroup.types'; import { getTimezone } from 'models/user/user.helpers'; -import { User } from 'models/user/user.types'; +import { UserCurrentlyOnCall } from 'models/user/user.types'; import { useStore } from 'state/useStore'; import { UserActions } from 'utils/authorization'; @@ -62,7 +62,7 @@ const AddResponders = observer( const currentMoment = useMemo(() => dayjs(), []); const isCreateMode = mode === 'create'; - const [currentlyConsideredUser, setCurrentlyConsideredUser] = useState(null); + const [currentlyConsideredUser, setCurrentlyConsideredUser] = useState(null); const [currentlyConsideredUserNotificationPolicy, setCurrentlyConsideredUserNotificationPolicy] = useState(NotificationPolicyValue.Default); @@ -141,7 +141,7 @@ const AddResponders = observer( disableNotificationPolicySelect handleDelete={generateRemovePreviouslyPagedUserCallback(user.pk)} important={user.important} - data={user as unknown as User} + data={user as unknown as UserCurrentlyOnCall} /> ))} {selectedUserResponders.map((responder, index) => ( diff --git a/grafana-plugin/src/containers/AddResponders/AddResponders.types.ts b/grafana-plugin/src/containers/AddResponders/AddResponders.types.ts index a53fbf5d..9fe481cf 100644 --- a/grafana-plugin/src/containers/AddResponders/AddResponders.types.ts +++ b/grafana-plugin/src/containers/AddResponders/AddResponders.types.ts @@ -1,7 +1,7 @@ import { SelectableValue } from '@grafana/data'; import { ActionMeta } from '@grafana/ui'; -import { User } from 'models/user/user.types'; +import { UserCurrentlyOnCall } from 'models/user/user.types'; export enum NotificationPolicyValue { Default = 0, @@ -9,7 +9,7 @@ export enum NotificationPolicyValue { } export type UserResponder = { - data: User; + data: UserCurrentlyOnCall; important: boolean; }; export type UserResponders = UserResponder[]; diff --git a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.module.scss b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.module.scss index fbbd2163..7b5f57ac 100644 --- a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.module.scss +++ b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.module.scss @@ -30,6 +30,10 @@ margin: 8px; } +.loading-placeholder { + margin: 8px; +} + .table { max-height: 150px; overflow: auto; diff --git a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.test.tsx b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.test.tsx index af0ec1c9..37128060 100644 --- a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.test.tsx +++ b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.test.tsx @@ -21,7 +21,7 @@ describe('AddRespondersPopup', () => { }, ]; - test('it renders teams properly', () => { + test('it shows a loading message initially', () => { const mockStoreValue = { directPagingStore: { selectedTeamResponder: null, @@ -30,36 +30,7 @@ describe('AddRespondersPopup', () => { getSearchResult: jest.fn().mockReturnValue(teams), }, userStore: { - getSearchResult: jest.fn().mockReturnValue({ results: [] }), - }, - }; - - const component = render( - - - - ); - - expect(component.container).toMatchSnapshot(); - }); - - test('if a team is selected it shows an info alert', () => { - const mockStoreValue = { - directPagingStore: { - selectedTeamResponder: teams[0], - selectedUserResponders: [], - }, - grafanaTeamStore: { - getSearchResult: jest.fn().mockReturnValue(teams), - }, - userStore: { - getSearchResult: jest.fn().mockReturnValue({ results: [] }), + search: jest.fn().mockReturnValue({ results: [] }), }, }; diff --git a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.tsx b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.tsx index 9be2456d..7a500076 100644 --- a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.tsx +++ b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/AddRespondersPopup.tsx @@ -1,6 +1,6 @@ import React, { useState, useCallback, useEffect, useRef, FC } from 'react'; -import { Alert, HorizontalGroup, Icon, Input, RadioButtonGroup } from '@grafana/ui'; +import { Alert, HorizontalGroup, Icon, Input, LoadingPlaceholder, RadioButtonGroup } from '@grafana/ui'; import cn from 'classnames/bind'; import { observer } from 'mobx-react'; import { ColumnsType } from 'rc-table/lib/interface'; @@ -10,7 +10,7 @@ import GTable from 'components/GTable/GTable'; import Text from 'components/Text/Text'; import { Alert as AlertType } from 'models/alertgroup/alertgroup.types'; import { GrafanaTeam } from 'models/grafana_team/grafana_team.types'; -import { User } from 'models/user/user.types'; +import { UserCurrentlyOnCall } from 'models/user/user.types'; import { useStore } from 'state/useStore'; import { useDebouncedCallback, useOnClickOutside } from 'utils/hooks'; @@ -21,7 +21,7 @@ type Props = { visible: boolean; setVisible: (value: boolean) => void; - setCurrentlyConsideredUser: (user: User) => void; + setCurrentlyConsideredUser: (user: UserCurrentlyOnCall) => void; setShowUserConfirmationModal: (value: boolean) => void; existingPagedUsers?: AlertType['paged_users']; @@ -34,13 +34,6 @@ enum TabOptions { Users = 'users', } -/** - * TODO: properly filter out 'No team'. Right now it shows up on first render and then shortly thereafter the component - * re-renders with 'No team' filtered out - * - * TODO: properly fetch/show loading state when fetching users. Right now it shows an empty list on the initial network - * request, we can probably have a better experience here - */ const AddRespondersPopup = observer( ({ mode, @@ -55,23 +48,13 @@ const AddRespondersPopup = observer( const isCreateMode = mode === 'create'; + const [searchLoading, setSearchLoading] = useState(true); const [activeOption, setActiveOption] = useState(isCreateMode ? TabOptions.Teams : TabOptions.Users); + const [teamSearchResults, setTeamSearchResults] = useState([]); + const [userSearchResults, setUserSearchResults] = useState([]); const [searchTerm, setSearchTerm] = useState(''); const ref = useRef(); - const teamSearchResults = grafanaTeamStore.getSearchResult(); - - let userSearchResults = userStore.getSearchResult().results || []; - - /** - * in the context where some user(s) have already been paged (ex. on a direct paging generated - * alert group detail page), we should filter out the search results to not include these users - */ - if (existingPagedUsers.length > 0) { - const existingPagedUserIds = existingPagedUsers.map(({ pk }) => pk); - userSearchResults = userSearchResults.filter(({ pk }) => !existingPagedUserIds.includes(pk)); - } - const usersCurrentlyOnCall = userSearchResults.filter(({ is_currently_oncall }) => is_currently_oncall); const usersNotCurrentlyOnCall = userSearchResults.filter(({ is_currently_oncall }) => !is_currently_oncall); @@ -87,7 +70,7 @@ const AddRespondersPopup = observer( ); const onClickUser = useCallback( - async (user: User) => { + async (user: UserCurrentlyOnCall) => { if (isCreateMode && user.is_currently_oncall) { directPagingStore.addUserToSelectedUsers(user); } else { @@ -113,18 +96,86 @@ const AddRespondersPopup = observer( [setVisible, directPagingStore, setActiveOption] ); - const handleSearchTermChange = useDebouncedCallback(() => { + const searchForUsers = useCallback(async () => { + const userResults = await userStore.search({ searchTerm, is_currently_oncall: 'all' }); + setUserSearchResults(userResults.results); + }, [searchTerm]); + + const searchForTeams = useCallback(async () => { + await grafanaTeamStore.updateItems(searchTerm, false, true, false); + setTeamSearchResults(grafanaTeamStore.getSearchResult()); + }, [searchTerm]); + + const handleSearchTermChange = useDebouncedCallback(async () => { + setSearchLoading(true); + if (isCreateMode && activeOption === TabOptions.Teams) { - grafanaTeamStore.updateItems(searchTerm, false, true, false); + await searchForTeams(); } else { - userStore.updateItems({ searchTerm, short: 'false' }); + await searchForUsers(); } + + setSearchLoading(false); }, 500); - useEffect(handleSearchTermChange, [searchTerm, activeOption]); + const onChangeTab = useCallback( + async (tab: TabOptions) => { + /** + * there's no need to trigger a new search request when the user changes tabs if they don't have a + * search term + */ + if (searchTerm) { + setSearchLoading(true); + + if (activeOption === TabOptions.Teams) { + await searchForTeams(); + } else { + await searchForUsers(); + } + + setSearchLoading(false); + } + + setActiveOption(tab); + }, + [searchTerm] + ); + + useEffect(handleSearchTermChange, [searchTerm]); + + /** + * in the context where some user(s) have already been paged (ex. on a direct paging generated + * alert group detail page), we should filter out the search results to not include these users + */ + useEffect(() => { + if (existingPagedUsers.length > 0) { + const existingPagedUserIds = existingPagedUsers.map(({ pk }) => pk); + setUserSearchResults((userSearchResults) => + userSearchResults.filter(({ pk }) => !existingPagedUserIds.includes(pk)) + ); + } + }, [existingPagedUsers]); + + /** + * pre-populate the users and teams search results so that when the user opens AddRespondersPopup it is already + * populated with data (nicer UX) + */ + useEffect(() => { + (async () => { + /** + * teams are not relevant when the component is rendered in "update" mode so we skip fetching teams here + */ + if (isCreateMode) { + await searchForTeams(); + } + + await searchForUsers(); + setSearchLoading(false); + })(); + }, []); const userIsSelected = useCallback( - (user: User) => selectedUserResponders.some((userResponder) => userResponder.data.pk === user.pk), + (user: UserCurrentlyOnCall) => selectedUserResponders.some((userResponder) => userResponder.data.pk === user.pk), [selectedUserResponders] ); @@ -155,11 +206,11 @@ const AddRespondersPopup = observer( }, ]; - const userColumns: ColumnsType = [ + const userColumns: ColumnsType = [ // TODO: how to make the rows span full width properly? { width: 300, - render: (user: User) => { + render: (user: UserCurrentlyOnCall) => { const { avatar, name, username, teams } = user; const disabled = userIsSelected(user); @@ -170,6 +221,7 @@ const AddRespondersPopup = observer( {name || username} + {/* TODO: we should add an elippsis and/or tooltip in the event that the user has a ton of teams */} {teams?.length > 0 && {teams.map(({ name }) => name).join(', ')}} @@ -179,18 +231,18 @@ const AddRespondersPopup = observer( }, { width: 40, - render: (user: User) => (userIsSelected(user) ? : null), + render: (user: UserCurrentlyOnCall) => (userIsSelected(user) ? : null), key: 'Checked', }, ]; - const UserResultsSection: FC<{ header: string; users: User[] }> = ({ header, users }) => + const UserResultsSection: FC<{ header: string; users: UserCurrentlyOnCall[] }> = ({ header, users }) => users.length > 0 && ( <> {header} - + emptyText={users ? 'No users found' : 'Loading...'} rowKey="pk" columns={userColumns} @@ -223,11 +275,12 @@ const AddRespondersPopup = observer( ]} className={cx('radio-buttons')} value={activeOption} - onChange={setActiveOption} + onChange={onChangeTab} fullWidth /> )} - {activeOption === TabOptions.Teams && ( + {searchLoading && } + {!searchLoading && activeOption === TabOptions.Teams && ( <> {selectedTeamResponder ? ( )} - {activeOption === TabOptions.Users && ( + {!searchLoading && activeOption === TabOptions.Users && ( <> diff --git a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/__snapshots__/AddRespondersPopup.test.tsx.snap b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/__snapshots__/AddRespondersPopup.test.tsx.snap index 16a3c3c9..ba89c566 100644 --- a/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/__snapshots__/AddRespondersPopup.test.tsx.snap +++ b/grafana-plugin/src/containers/AddResponders/parts/AddRespondersPopup/__snapshots__/AddRespondersPopup.test.tsx.snap @@ -1,119 +1,6 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`AddRespondersPopup if a team is selected it shows an info alert 1`] = ` -
-
-
-
- -
-
- - - -
-
-
-
-
- - - - -
-
-
-
- - - -
-
-
-
- You can add only one team per escalation. Please remove the existing team before adding a new one. -
-
-
-
-
-`; - -exports[`AddRespondersPopup it renders teams properly 1`] = ` +exports[`AddRespondersPopup it shows a loading message initially 1`] = `
+ Loading... +
-
- - - -
-
-
-
- - You can only page teams which have a Direct Paging integration that is configured. - - - -
-
- Learn more -
-
-
- - - -
-
-
- - - -
-
-
-
-
-
-
- - - - - - - - - - - - -
-
-
-
-
-
- -
-
- - my test team - -
-
-
-
- - 1 - user - - on-call - -
-
-
-
-
-
-
-
-
- -
-
- - my test team 2 - -
-
-
-
-
-
-
-
+