From 5595480d3a0be004be12193c1625491257668fa1 Mon Sep 17 00:00:00 2001 From: Rares Mardare <40542072+teodosii@users.noreply.github.com> Date: Mon, 19 Dec 2022 13:15:32 +0200 Subject: [PATCH 01/10] add qr code class with white background (#1012) # What this PR does - Adds white border around the QR code + small margin below --- .../MobileAppVerification/MobileAppVerification.module.scss | 5 +++++ .../MobileAppVerification/MobileAppVerification.tsx | 2 +- .../__snapshots__/MobileAppVerification.test.tsx.snap | 2 +- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.module.scss b/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.module.scss index cd4af8f4..94dbd47f 100644 --- a/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.module.scss +++ b/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.module.scss @@ -39,6 +39,11 @@ opacity: 0.2; } +.qr-code { + background-color: #fff; + margin-bottom: 12px; +} + .qr-loader { position: absolute; z-index: 10; diff --git a/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.tsx b/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.tsx index ed634a9f..ed8baee3 100644 --- a/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.tsx +++ b/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.tsx @@ -133,7 +133,7 @@ const MobileAppVerification = observer(({ userPk }: Props) => { Open Grafana IRM mobile application and scan this code to sync it with your account.
- + {isQRBlurry && }
diff --git a/grafana-plugin/src/containers/MobileAppVerification/__snapshots__/MobileAppVerification.test.tsx.snap b/grafana-plugin/src/containers/MobileAppVerification/__snapshots__/MobileAppVerification.test.tsx.snap index e471869c..ac791478 100644 --- a/grafana-plugin/src/containers/MobileAppVerification/__snapshots__/MobileAppVerification.test.tsx.snap +++ b/grafana-plugin/src/containers/MobileAppVerification/__snapshots__/MobileAppVerification.test.tsx.snap @@ -109,7 +109,7 @@ exports[`MobileAppVerification if we disconnect the app, it disconnects and fetc class="u-width-100 u-flex u-flex-center u-position-relative" >
Date: Tue, 20 Dec 2022 12:41:34 +0100 Subject: [PATCH 02/10] modify push notification settings + use fcm-django library (#998) - swaps out `django-push-notifications` for [`fcm-django`](https://github.com/grafana/fcm-django). Again.. this is a fork of the parent repo for exactly the same reason.. the migrations point to `auth_user` without letting us use our own user model, this has been patched in the `grafana` fork. The reason why we are using `fcm-django` vs `django-push-notifications` is that the latter does not support the new FCM API, only the "legacy" API. The legacy FCM API does not support certain push notification settings that we would like to use. - modifies the iOS/Android specific push notification settings - adds a `flower` pod in the `docker-compose-developer.yml`, useful for debugging tasks locally - sets the mobile app verification token TTL to 5 minutes when developing locally. The default of 1 minute makes working with device emulators really tricky.. This PR also swaps out the base image in `engine/Dockerfile` from `python:3.9-alpine3.16` to `python:3.9-slim-buster`. As to why.. in short, with the introduction of the `fcm-django` library there is now a peer-dependency on [`grpcio`](https://github.com/grpc/grpc) (which is used by `firebase_admin`.. which I am using in this PR to interact directly with Firebase Cloud Messaging (FCM)). `grpcio` does not publish wheels (read: compiled binaries) for the Alpine distro. It does publish wheels for Debian and hence `pip install -r requirements.txt` does not need to build this library from the source distribution. This is a [known "issue"](https://github.com/grpc/grpc/issues/22815#issuecomment-1107874367) and the recommended solution in the community is to.. not use alpine. These were the numbers, when building the image locally, in terms of image size and build time: | | Local image size (uncompressed | Build time (may differ based on your network speed) | | ------------------------- | -------------------------------------- | ---------- | | `python:3.9-alpine3.16` | 785MB | 320s | | `python:3.9-slim-buster` | 1.05GB | 90s | Co-authored-by: Salvatore Giordano --- .github/workflows/helm_tests.yml | 4 +- Makefile | 2 +- dev/README.md | 29 +++++++ docker-compose-developer.yml | 17 +++++ engine/.gitignore | 1 + engine/Dockerfile | 12 ++- engine/apps/mobile_app/backend.py | 4 +- engine/apps/mobile_app/fcm_relay.py | 17 +++-- engine/apps/mobile_app/models.py | 3 +- engine/apps/mobile_app/tasks.py | 76 ++++++++++++------- engine/apps/mobile_app/views.py | 14 +--- engine/requirements.txt | 2 +- engine/settings/base.py | 22 ++++-- .../MobileAppVerification.tsx | 4 +- grafana-plugin/webpack.config.js | 1 + 15 files changed, 143 insertions(+), 65 deletions(-) diff --git a/.github/workflows/helm_tests.yml b/.github/workflows/helm_tests.yml index a95b4389..658a9faf 100644 --- a/.github/workflows/helm_tests.yml +++ b/.github/workflows/helm_tests.yml @@ -10,10 +10,10 @@ jobs: - name: Checkout uses: actions/checkout@v3 - - name: Set up Docker Buildx # We need this step for docker caching + - name: Set up Docker Buildx # We need this step for docker caching uses: docker/setup-buildx-action@v2 - - name: Build docker image locally # using github actions docker cache + - name: Build docker image locally # using github actions docker cache uses: docker/build-push-action@v2 with: context: ./engine diff --git a/Makefile b/Makefile index 4ada9540..711e3e3d 100644 --- a/Makefile +++ b/Makefile @@ -86,7 +86,7 @@ restart: $(call run_docker_compose_command,restart) build: - $(call run_docker_compose_command,build --no-cache) + $(call run_docker_compose_command,build) cleanup: stop docker system prune --filter label="$(DOCKER_COMPOSE_DEV_LABEL)" --all --volumes diff --git a/dev/README.md b/dev/README.md index 4ffbfebc..0ff81544 100644 --- a/dev/README.md +++ b/dev/README.md @@ -13,6 +13,7 @@ - [Could not build wheels for cryptography which use PEP 517 and cannot be installed directly](#could-not-build-wheels-for-cryptography-which-use-pep-517-and-cannot-be-installed-directly) - [django.db.utils.OperationalError: (1366, "Incorrect string value")](#djangodbutilsoperationalerror-1366-incorrect-string-value) - [/bin/sh: line 0: cd: grafana-plugin: No such file or directory](#binsh-line-0-cd-grafana-plugin-no-such-file-or-directory) + - [Encountered error while trying to install package - grpcio](#encountered-error-while-trying-to-install-package---grpcio) - [IDE Specific Instructions](#ide-specific-instructions) - [PyCharm](#pycharm) @@ -280,6 +281,34 @@ clear everything in docker by resetting or: make cleanup ``` +### Encountered error while trying to install package - grpcio + +**Problem:** + +We are currently using a library, `fcm-django`, which has a dependency on `grpcio`. Google does not provide `grpcio` +wheels built for Apple Silicon Macs. The best solution so far has been to use a `conda` virtualenv. There's apparently +a lot of community work put into making packages play well with M1/arm64 architecture. + +```bash +pip install -r requirements.txt +... + note: This error originates from a subprocess, and is likely not a problem with pip. +error: legacy-install-failure + +× Encountered error while trying to install package. +╰─> grpcio +... +``` + +**Solution:** + +Use a `conda` virtualenv, and then run the following when installing the engine dependencies/ +[See here for more details](https://stackoverflow.com/a/74307636/3902555) + +```bash +GRPC_PYTHON_BUILD_SYSTEM_OPENSSL=1 GRPC_PYTHON_BUILD_SYSTEM_ZLIB=1 pip install -r requirements.txt +``` + ## IDE Specific Instructions ### PyCharm diff --git a/docker-compose-developer.yml b/docker-compose-developer.yml index 55884d05..8627802b 100644 --- a/docker-compose-developer.yml +++ b/docker-compose-developer.yml @@ -24,6 +24,7 @@ x-env-files: &oncall-env-files x-env-vars: &oncall-env-vars BROKER_TYPE: ${BROKER_TYPE} GRAFANA_API_URL: http://localhost:3000 + GOOGLE_APPLICATION_CREDENTIALS: /etc/app/gcp_service_account.json # basically this is needed because the oncall backend containers have been configured to communicate w/ grafana via # http://localhost:3000 (GRAFANA_API_URL). This URL is used in two scenarios: @@ -47,6 +48,7 @@ services: labels: *oncall-labels environment: ONCALL_API_URL: http://host.docker.internal:8080 + MOBILE_APP_QR_INTERVAL_QUEUE: 290000 # 4 minutes and 50 seconds volumes: - ./grafana-plugin:/etc/app - /etc/app/node_modules @@ -105,6 +107,21 @@ services: profiles: - engine + flower: + container_name: flower + labels: *oncall-labels + image: mher/flower:1.2.0 + environment: + # TODO: make this work properly w/ BROKER_TYPE env var + CELERY_BROKER_URL: "redis://redis:6379/0" + ports: + - "5555:5555" + depends_on: + oncall_celery: + condition: service_started + profiles: + - engine + oncall_db_migration: container_name: oncall_db_migration labels: *oncall-labels diff --git a/engine/.gitignore b/engine/.gitignore index 04b9e080..43962828 100644 --- a/engine/.gitignore +++ b/engine/.gitignore @@ -3,3 +3,4 @@ extensions/ uwsgi-local.ini celerybeat-schedule *.db +gcp_service_account.json diff --git a/engine/Dockerfile b/engine/Dockerfile index 4c84a01c..5a3a7290 100644 --- a/engine/Dockerfile +++ b/engine/Dockerfile @@ -1,8 +1,14 @@ -FROM python:3.9-alpine3.16 AS base -RUN apk add bash python3-dev build-base linux-headers pcre-dev mariadb-connector-c-dev openssl-dev libffi-dev git +FROM python:3.9-slim-buster AS base +RUN apt-get update && apt-get install -y \ + python3-dev \ + gcc \ + libpq-dev \ + libmariadb-dev \ + netcat WORKDIR /etc/app COPY ./requirements.txt ./ +RUN pip install --upgrade pip RUN pip install -r requirements.txt # we intentionally have two COPY commands, this is to have the requirements.txt in a separate build step @@ -16,7 +22,7 @@ RUN DJANGO_SETTINGS_MODULE=settings.prod_without_db DATABASE_TYPE=sqlite3 DATABA RUN chown -R 1000:2000 /var/lib/oncall FROM base AS dev -RUN apk add sqlite mysql-client postgresql-client +RUN apt-get install -y sqlite3 default-mysql-client postgresql-client FROM dev AS dev-enterprise RUN pip install -r requirements-enterprise-docker.txt diff --git a/engine/apps/mobile_app/backend.py b/engine/apps/mobile_app/backend.py index db518de0..43f257c3 100644 --- a/engine/apps/mobile_app/backend.py +++ b/engine/apps/mobile_app/backend.py @@ -1,7 +1,7 @@ import json from django.conf import settings -from push_notifications.models import GCMDevice +from fcm_django.models import FCMDevice from apps.base.messaging import BaseMessagingBackend from apps.mobile_app.tasks import notify_user_async @@ -35,7 +35,7 @@ class MobileAppBackend(BaseMessagingBackend): token.delete() # delete push notification related info for user - GCMDevice.objects.filter(user=user).delete() + FCMDevice.objects.filter(user=user).delete() def serialize_user(self, user): from apps.mobile_app.models import MobileAppAuthToken diff --git a/engine/apps/mobile_app/fcm_relay.py b/engine/apps/mobile_app/fcm_relay.py index 33bebda4..f50f2434 100644 --- a/engine/apps/mobile_app/fcm_relay.py +++ b/engine/apps/mobile_app/fcm_relay.py @@ -1,4 +1,5 @@ -from push_notifications.gcm import send_message +# from firebase_admin.messaging import Message +# from fcm_django.models import FCMDevice from rest_framework import status from rest_framework.response import Response from rest_framework.views import APIView @@ -6,6 +7,7 @@ from rest_framework.views import APIView REQUIRED_FIELDS = {"registration_ids", "notification", "data"} +# TODO: update thie class FCMRelayView(APIView): def post(self, request): """ @@ -17,10 +19,11 @@ class FCMRelayView(APIView): if not REQUIRED_FIELDS.issubset(request.data.keys()): return Response(status=status.HTTP_400_BAD_REQUEST) - registration_ids = request.data["registration_ids"] - data = { - **request.data["data"], - **request.data["notification"], - } + # registration_ids = request.data["registration_ids"] + # data = { + # **request.data["data"], + # **request.data["notification"], + # } - return send_message(registration_ids=registration_ids, data=data, cloud_type="FCM") + # return FCMDevice.objects.send_message(Message(), False, ["registration_ids"]) + return "TODO:" diff --git a/engine/apps/mobile_app/models.py b/engine/apps/mobile_app/models.py index e42ade25..7ad107dd 100644 --- a/engine/apps/mobile_app/models.py +++ b/engine/apps/mobile_app/models.py @@ -1,5 +1,6 @@ from typing import Tuple +from django.conf import settings from django.db import models from django.utils import timezone @@ -7,7 +8,7 @@ from apps.auth_token import constants, crypto from apps.auth_token.models import BaseAuthToken from apps.user_management.models import Organization, User -MOBILE_APP_AUTH_VERIFICATION_TOKEN_TIMEOUT_SECONDS = 60 +MOBILE_APP_AUTH_VERIFICATION_TOKEN_TIMEOUT_SECONDS = 60 * (5 if settings.DEBUG else 1) def get_expire_date(): diff --git a/engine/apps/mobile_app/tasks.py b/engine/apps/mobile_app/tasks.py index fc9341d9..88d56a17 100644 --- a/engine/apps/mobile_app/tasks.py +++ b/engine/apps/mobile_app/tasks.py @@ -1,6 +1,7 @@ from celery.utils.log import get_task_logger from django.conf import settings -from push_notifications.models import GCMDevice +from fcm_django.models import FCMDevice +from firebase_admin.messaging import APNSConfig, APNSPayload, Aps, ApsAlert, CriticalSound, Message from apps.alerts.models import AlertGroup from apps.mobile_app.alert_rendering import get_push_notification_message @@ -34,10 +35,10 @@ def notify_user_async(user_pk, alert_group_pk, notification_policy_pk, critical) logger.warning(f"User notification policy {notification_policy_pk} does not exist") return - gcm_devices_to_notify = GCMDevice.objects.filter(user=user) + device_to_notify = FCMDevice.objects.filter(user=user).first() # create an error log in case user has no devices set up - if not gcm_devices_to_notify.exists(): + if not device_to_notify: UserNotificationPolicyLogRecord.objects.create( author=user, type=UserNotificationPolicyLogRecord.TYPE_PERSONAL_NOTIFICATION_FAILED, @@ -47,39 +48,58 @@ def notify_user_async(user_pk, alert_group_pk, notification_policy_pk, critical) notification_step=notification_policy.step, notification_channel=notification_policy.notify_by, ) - logger.info(f"Error while sending a mobile push notification: user {user_pk} has no devices set up") + logger.info(f"Error while sending a mobile push notification: user {user_pk} has no device set up") return message = get_push_notification_message(alert_group) thread_id = f"{alert_group.channel.organization.public_primary_key}:{alert_group.public_primary_key}" + alert_title = f"Critical page: {message}" if critical else message + number_of_alerts = alert_group.alerts.count() - if critical: - aps = { - "alert": f"Critical page: {message}", - "interruption-level": "critical", - "sound": "ambulance.aiff", - } - else: - aps = { - "alert": message, - "sound": "bingbong.aiff", - } + # TODO: we should update this to check if FCM_RELAY is set and conditionally make a call here.. - extra = { - "orgId": alert_group.channel.organization.public_primary_key, - "orgName": alert_group.channel.organization.stack_slug, - "alertGroupId": alert_group.public_primary_key, - "status": alert_group.status, - "aps": aps, - } - - logger.info(f"Sending push notification with message: {message}; thread-id: {thread_id}; extra: {extra}") - - # TODO: rename category to USER_NEW_ALERT_GROUP - fcm_response = gcm_devices_to_notify.send_message( - message, thread_id=thread_id, category="USER_NEW_INCIDENT", extra=extra + message = Message( + token=device_to_notify.registration_id, + data={ + # from the docs.. + # A dictionary of data fields (optional). All keys and values in the dictionary must be strings + # + # alert_group.status is an int so it must be casted... + "orgId": alert_group.channel.organization.public_primary_key, + "orgName": alert_group.channel.organization.stack_slug, + "alertGroupId": alert_group.public_primary_key, + "status": str(alert_group.status), + "type": "oncall.critical_message" if critical else "oncall.message", + "title": alert_title, + "thread_id": thread_id, + }, + apns=APNSConfig( + payload=APNSPayload( + aps=Aps( + thread_id=thread_id, + badge=number_of_alerts, + alert=ApsAlert( + title=alert_title, + subtitle="yooo this is a subtitle", + body="hello this is the body", + ), + sound=CriticalSound( + critical=1 if critical else 0, + name="ambulance.aiff" if critical else "bingbong.aiff", + volume=1, + ), + custom_data={ + "interruption-level": "critical" if critical else "time-sensitive", + }, + ), + ), + ), ) + logger.info(f"Sending push notification with message: {message}; thread-id: {thread_id};") + + fcm_response = device_to_notify.send_message(message) + # NOTE: we may want to further handle the response from FCM, but for now lets simply log it out # https://firebase.google.com/docs/cloud-messaging/http-server-ref#interpret-downstream logger.info(f"FCM response was: {fcm_response}") diff --git a/engine/apps/mobile_app/views.py b/engine/apps/mobile_app/views.py index b0188b3e..16257440 100644 --- a/engine/apps/mobile_app/views.py +++ b/engine/apps/mobile_app/views.py @@ -1,25 +1,15 @@ -from push_notifications.api.rest_framework import GCMDeviceAuthorizedViewSet, GCMDeviceSerializer +from fcm_django.api.rest_framework import FCMDeviceAuthorizedViewSet as BaseFCMDeviceAuthorizedViewSet from rest_framework import status from rest_framework.exceptions import NotFound from rest_framework.response import Response -from rest_framework.serializers import HiddenField from rest_framework.views import APIView from apps.mobile_app.auth import MobileAppAuthTokenAuthentication, MobileAppVerificationTokenAuthentication from apps.mobile_app.models import MobileAppAuthToken -class FCMDeviceAuthorizedViewSet(GCMDeviceAuthorizedViewSet): - class FCMDeviceSerializer(GCMDeviceSerializer): - """ - GCMDevice has cloud_message_type equal to "GCM" by default, in this serializer cloud_message_type is always set - to "FCM" no matter what was provided in the request. - """ - - cloud_message_type = HiddenField(default="FCM") - +class FCMDeviceAuthorizedViewSet(BaseFCMDeviceAuthorizedViewSet): authentication_classes = (MobileAppAuthTokenAuthentication,) - serializer_class = FCMDeviceSerializer class MobileAppAuthTokenAPIView(APIView): diff --git a/engine/requirements.txt b/engine/requirements.txt index 3e45d9a4..17b49561 100644 --- a/engine/requirements.txt +++ b/engine/requirements.txt @@ -33,7 +33,7 @@ django-log-request-id==1.6.0 django-polymorphic==3.0.0 django-rest-polymorphic==0.1.9 pre-commit==2.15.0 -https://github.com/grafana/django-push-notifications/archive/refs/tags/3.0.0-fix-migration.tar.gz +https://github.com/grafana/fcm-django/archive/refs/tags/v1.0.12r1.tar.gz django-mirage-field==1.3.0 django-mysql==4.6.0 PyMySQL==1.0.2 diff --git a/engine/settings/base.py b/engine/settings/base.py index e5aac426..f8d522d0 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -216,7 +216,7 @@ INSTALLED_APPS = [ "debug_toolbar", "social_django", "polymorphic", - "push_notifications", + "fcm_django", ] REST_FRAMEWORK = { @@ -546,18 +546,26 @@ GRAFANA_API_KEY_NAME = "Grafana OnCall" EXTRA_MESSAGING_BACKENDS = [] if FEATURE_MOBILE_APP_INTEGRATION_ENABLED: + from firebase_admin import initialize_app + EXTRA_MESSAGING_BACKENDS += [ ("apps.mobile_app.backend.MobileAppBackend", 5), ("apps.mobile_app.backend.MobileAppCriticalBackend", 6), ] -PUSH_NOTIFICATIONS_SETTINGS = { - "FCM_API_KEY": os.getenv("FCM_API_KEY"), - "FCM_POST_URL": os.getenv("FCM_POST_URL", default="https://fcm.googleapis.com/fcm/send"), - "USER_MODEL": "user_management.User", - "UPDATE_ON_DUPLICATE_REG_ID": True, -} + FIREBASE_APP = initialize_app() + FCM_RELAY_ENABLED = getenv_boolean("FCM_RELAY_ENABLED", default=False) +FCM_DJANGO_SETTINGS = { + # an instance of firebase_admin.App to be used as default for all fcm-django requests + # default: None (the default Firebase app) + "DEFAULT_FIREBASE_APP": None, + "APP_VERBOSE_NAME": "OnCall", + "ONE_DEVICE_PER_USER": True, + "DELETE_INACTIVE_DEVICES": False, + "UPDATE_ON_DUPLICATE_REG_ID": True, + "USER_MODEL": "user_management.User", +} SELF_HOSTED_SETTINGS = { "STACK_ID": 5, diff --git a/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.tsx b/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.tsx index ed8baee3..c31604a5 100644 --- a/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.tsx +++ b/grafana-plugin/src/containers/MobileAppVerification/MobileAppVerification.tsx @@ -22,7 +22,9 @@ type Props = { }; const INTERVAL_MIN_THROTTLING = 500; -const INTERVAL_QUEUE_QR = 50000; +const INTERVAL_QUEUE_QR = process.env.MOBILE_APP_QR_INTERVAL_QUEUE + ? parseInt(process.env.MOBILE_APP_QR_INTERVAL_QUEUE, 10) + : 50000; const INTERVAL_POLLING = 5000; const BACKEND = 'MOBILE_APP'; diff --git a/grafana-plugin/webpack.config.js b/grafana-plugin/webpack.config.js index b733cbb3..934c1717 100644 --- a/grafana-plugin/webpack.config.js +++ b/grafana-plugin/webpack.config.js @@ -142,6 +142,7 @@ module.exports.getWebpackConfig = (config, options) => { */ new webpack.EnvironmentPlugin({ ONCALL_API_URL: null, + MOBILE_APP_QR_INTERVAL_QUEUE: null, }), ], From 327b7121ac8e78fe5edc81d85d15ed147d0a093f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juris=20Pav=C4=BCu=C4=8Denkovs?= Date: Tue, 20 Dec 2022 12:59:19 +0000 Subject: [PATCH 03/10] Helm: define engine update strategy in values.yaml (#985) # What this PR does Now it is possible to change engine deployment update strategy in values.yaml. ## Which issue(s) this PR fixes This is due to #334 and #316, as with rolling update, race conditions might happen when there is still an old engine pod running. Co-authored-by: Joey Orlando --- CHANGELOG.md | 1 + helm/oncall/Chart.yaml | 2 +- helm/oncall/templates/engine/deployment.yaml | 5 +---- helm/oncall/values.yaml | 8 ++++++++ 4 files changed, 11 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3120bf88..c6d7642b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- Added ability to change engine deployment update strategy via values in helm chart. - removed APNS support - changed the `django-push-notification` library from the `iskhakov` fork to the [`grafana` fork](https://github.com/grafana/django-push-notifications). This new fork basically patches an issue which affected the database migrations of this django app (previously the diff --git a/helm/oncall/Chart.yaml b/helm/oncall/Chart.yaml index 70f7df3c..76ffef88 100644 --- a/helm/oncall/Chart.yaml +++ b/helm/oncall/Chart.yaml @@ -7,7 +7,7 @@ type: application # This is the chart version. This version number should be incremented each time you make changes # to the chart and its templates, including the app version. # Versions are expected to follow Semantic Versioning (https://semver.org/) -version: 1.1.0 +version: 1.1.1 # This is the version number of the application being deployed. This version number should be # incremented each time you make changes to the application. Versions are not expected to diff --git a/helm/oncall/templates/engine/deployment.yaml b/helm/oncall/templates/engine/deployment.yaml index 640bb84b..f0848c76 100644 --- a/helm/oncall/templates/engine/deployment.yaml +++ b/helm/oncall/templates/engine/deployment.yaml @@ -10,10 +10,7 @@ spec: matchLabels: {{- include "oncall.engine.selectorLabels" . | nindent 6 }} strategy: - rollingUpdate: - maxSurge: 25% - maxUnavailable: 0 - type: RollingUpdate + {{- toYaml .Values.engine.updateStrategy | nindent 4 }} template: metadata: {{- with .Values.podAnnotations }} diff --git a/helm/oncall/values.yaml b/helm/oncall/values.yaml index a21ce82c..9abb14fc 100644 --- a/helm/oncall/values.yaml +++ b/helm/oncall/values.yaml @@ -29,6 +29,14 @@ engine: # 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: {} From e1b9a75ce7591992a485a8a3965829460b297e41 Mon Sep 17 00:00:00 2001 From: Ieva Date: Tue, 20 Dec 2022 17:15:18 +0000 Subject: [PATCH 04/10] RBAC: update RBAC permissions scopes for `plugins.app:access` action (#1028) Change RBAC permission scope to `plugins:id:grafana-oncall-app` from `plugins.app:id:grafana-oncall-app` for permissions with `plugins.app:access` action. This is needed because [Grafana is expecting](https://github.com/grafana/grafana/blob/main/pkg/services/accesscontrol/pluginutils/utils.go#L20-L21) `plugins:` not `plugins.app` prefix for permissions with `plugins.app:access` action. --- grafana-plugin/src/plugin.json | 54 +++++++++++++++++----------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/grafana-plugin/src/plugin.json b/grafana-plugin/src/plugin.json index 22d3f50b..51055b7b 100644 --- a/grafana-plugin/src/plugin.json +++ b/grafana-plugin/src/plugin.json @@ -189,7 +189,7 @@ "name": "Admin", "description": "Read/write access to everything in OnCall", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.alert-groups:read" }, { "action": "grafana-oncall-app.alert-groups:write" }, @@ -239,7 +239,7 @@ "name": "Editor", "description": "Similar to the Admin role, minus the abilities to: create Integrations, create Escalation Chains, create Schedules, create Outgoing Webhooks, update ChatOps settings, update other user's settings, and update general OnCall setings.", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.alert-groups:read" }, { "action": "grafana-oncall-app.alert-groups:write" }, @@ -279,7 +279,7 @@ "name": "Reader", "description": "Read-only access to everything in OnCall", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.alert-groups:read" }, { "action": "grafana-oncall-app.integrations:read" }, @@ -300,7 +300,7 @@ "name": "OnCaller", "description": "Grants read access to everything in OnCall. In addition, grants edit access to Alert Groups and Schedules", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.alert-groups:read" }, { "action": "grafana-oncall-app.alert-groups:write" }, @@ -326,7 +326,7 @@ "name": "Alert Groups Reader", "description": "Read-only access to OnCall Alert Groups", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.alert-groups:read" } ] }, @@ -337,7 +337,7 @@ "name": "Alert Groups Editor", "description": "Read/write access to OnCall Alert Groups", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.alert-groups:read" }, { "action": "grafana-oncall-app.alert-groups:write" } ] @@ -349,7 +349,7 @@ "name": "Integrations Reader", "description": "Read-only access to OnCall Integrations", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.integrations:read" } ] }, @@ -360,7 +360,7 @@ "name": "Integrations Editor", "description": "Read/write access to OnCall Integrations", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.integrations:read" }, { "action": "grafana-oncall-app.integrations:write" }, { "action": "grafana-oncall-app.integrations:test" } @@ -373,7 +373,7 @@ "name": "Escalation Chains Reader", "description": "Read-only access to OnCall Escalation Chains", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.escalation-chains:read" } ] }, @@ -384,7 +384,7 @@ "name": "Escalation Chains Editor", "description": "Read/write access to OnCall Escalation Chains", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.escalation-chains:read" }, { "action": "grafana-oncall-app.escalation-chains:write" } ] @@ -396,7 +396,7 @@ "name": "Schedules Reader", "description": "Read-only access to OnCall Schedules", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.schedules:read" } ] }, @@ -407,7 +407,7 @@ "name": "Schedules Editor", "description": "Read/write access to OnCall Schedules", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.schedules:read" }, { "action": "grafana-oncall-app.schedules:write" }, { "action": "grafana-oncall-app.schedules:export" } @@ -420,7 +420,7 @@ "name": "ChatOps Reader", "description": "Read-only access to OnCall ChatOps", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.chatops:read" } ] }, @@ -431,7 +431,7 @@ "name": "ChatOps Editor", "description": "Read/write access to OnCall ChatOps", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.chatops:read" }, { "action": "grafana-oncall-app.chatops:write" }, { "action": "grafana-oncall-app.chatops:update-settings" } @@ -444,7 +444,7 @@ "name": "Outgoing Webhooks Reader", "description": "Read-only access to OnCall Outgoing Webhooks", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.outgoing-webhooks:read" } ] }, @@ -455,7 +455,7 @@ "name": "Outgoing Webhooks Editor", "description": "Read/write access to OnCall Outgoing Webhooks", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.outgoing-webhooks:read" }, { "action": "grafana-oncall-app.outgoing-webhooks:write" } ] @@ -467,7 +467,7 @@ "name": "Maintenance Reader", "description": "Read-only access to OnCall Maintenance", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.maintenance:read" } ] }, @@ -478,7 +478,7 @@ "name": "Maintenance Editor", "description": "Read/write access to OnCall Maintenance", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.maintenance:read" }, { "action": "grafana-oncall-app.maintenance:write" } ] @@ -490,7 +490,7 @@ "name": "API Keys Reader", "description": "Read-only access to OnCall API Keys", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.api-keys:read" } ] }, @@ -501,7 +501,7 @@ "name": "API Keys Editor", "description": "Read/write access to OnCall API Keys. Also grants access to be able to consume the API.", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.api-keys:read" }, { "action": "grafana-oncall-app.api-keys:write" } ] @@ -513,7 +513,7 @@ "name": "Notification Settings Reader", "description": "Read-only access to OnCall Notification Settings", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.notification-settings:read" } ] }, @@ -524,7 +524,7 @@ "name": "Notification Settings Editor", "description": "Read/write access to OnCall Notification Settings", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.notification-settings:read" }, { "action": "grafana-oncall-app.notification-settings:write" } ] @@ -536,7 +536,7 @@ "name": "User Settings Reader", "description": "Read-only access to OnCall User Settings", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.user-settings:read" } ] }, @@ -547,7 +547,7 @@ "name": "User Settings Editor", "description": "Read/write access to own OnCall User Settings", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.user-settings:read" }, { "action": "grafana-oncall-app.user-settings:write" } ] @@ -559,7 +559,7 @@ "name": "User Settings Admin", "description": "Read/write access to your own, plus other's OnCall User Settings", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.user-settings:read" }, { "action": "grafana-oncall-app.user-settings:write" }, { "action": "grafana-oncall-app.user-settings:admin" } @@ -572,7 +572,7 @@ "name": "Settings Reader", "description": "Read-only access to OnCall Settings", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.other-settings:read" } ] }, @@ -583,7 +583,7 @@ "name": "Settings Editor", "description": "Read/write access to OnCall Settings", "permissions": [ - { "action": "plugins.app:access", "scope": "plugins.app:id:grafana-oncall-app" }, + { "action": "plugins.app:access", "scope": "plugins:id:grafana-oncall-app" }, { "action": "grafana-oncall-app.other-settings:read" }, { "action": "grafana-oncall-app.other-settings:write" } ] From e1b798d5864dd7f2dd0f80868bff2b10e3419128 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Wed, 21 Dec 2022 18:23:16 +0100 Subject: [PATCH 05/10] add curl and bash to oncall engine docker image (#1034) Currently unable to exec into a k8s pod. I believe this is because `bash` is missing from the Docker image after switching from alpine to debian buster. Debugging this by adding in `bash`. --- engine/Dockerfile | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/engine/Dockerfile b/engine/Dockerfile index 5a3a7290..72046a3c 100644 --- a/engine/Dockerfile +++ b/engine/Dockerfile @@ -2,9 +2,10 @@ FROM python:3.9-slim-buster AS base RUN apt-get update && apt-get install -y \ python3-dev \ gcc \ - libpq-dev \ libmariadb-dev \ - netcat + netcat \ + curl \ + bash WORKDIR /etc/app COPY ./requirements.txt ./ From d1a43bdf1b03214aa572c91dbf371c60b8aabe6e Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Thu, 22 Dec 2022 21:44:53 +0100 Subject: [PATCH 06/10] specify Firebase GCP project id (#1042) Modifies the Firebase app initialization to explicitly specify the GCP project ID where the Firebase app is. Previously it would use the project associated with the service account being used. --- engine/settings/base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/settings/base.py b/engine/settings/base.py index f8d522d0..a5ab1195 100644 --- a/engine/settings/base.py +++ b/engine/settings/base.py @@ -553,7 +553,7 @@ if FEATURE_MOBILE_APP_INTEGRATION_ENABLED: ("apps.mobile_app.backend.MobileAppCriticalBackend", 6), ] - FIREBASE_APP = initialize_app() + FIREBASE_APP = initialize_app(options={"projectId": os.environ.get("FCM_PROJECT_ID", None)}) FCM_RELAY_ENABLED = getenv_boolean("FCM_RELAY_ENABLED", default=False) FCM_DJANGO_SETTINGS = { From 282e58db7b0e5728e3daa8be6e774b8c4972efd7 Mon Sep 17 00:00:00 2001 From: Ildar Iskhakov Date: Thu, 29 Dec 2022 21:22:15 +0800 Subject: [PATCH 07/10] Don't render logs for too big telegram dm (#1051) # What this PR does ## Which issue(s) this PR fixes ## Checklist - [ ] Tests updated - [ ] Documentation added - [ ] `CHANGELOG.md` updated --- engine/apps/telegram/renderers/message.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/engine/apps/telegram/renderers/message.py b/engine/apps/telegram/renderers/message.py index 1e989fe4..8140c3e5 100644 --- a/engine/apps/telegram/renderers/message.py +++ b/engine/apps/telegram/renderers/message.py @@ -47,6 +47,8 @@ class TelegramMessageRenderer: message_trimmed_text = MESSAGE_TRIMMED_TEXT.format(link=self.alert_group.web_link) max_log_lines_length = max_message_length - len(start_line_text) - len(message_trimmed_text) + if max_log_lines_length < 0: + return "" is_message_trimmed = len("\n".join(log_lines)) > max_log_lines_length while len("\n".join(log_lines)) > max_log_lines_length: log_lines.pop() From 1b67a8ec68bd9113187058f3cd68726fb3e4875c Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Fri, 30 Dec 2022 16:03:39 +0000 Subject: [PATCH 08/10] Add PD migrator lint & test steps to CI (#1053) # What this PR does - Add PD migrator related hooks to `.pre-commit-config.yaml` - Add Github Actions step for running PD migrator tests --- .github/workflows/ci.yml | 11 +++++++++++ .pre-commit-config.yaml | 13 +++++++++++++ tools/pagerduty-migrator/migrator/report.py | 8 ++++---- tools/pagerduty-migrator/requirements.txt | 2 -- 4 files changed, 28 insertions(+), 6 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 908f4d48..bb574046 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -151,6 +151,17 @@ jobs: pip install -r requirements.txt ONCALL_TESTING_RBAC_ENABLED=${{ matrix.rbac_enabled }} pytest -x + unit-test-pd-migrator: + runs-on: ubuntu-latest + container: python:3.9 + steps: + - uses: actions/checkout@v2 + - name: Unit Test PD Migrator + run: | + cd tools/pagerduty-migrator + pip install -r requirements.txt + pytest -x + docker-build: runs-on: ubuntu-latest steps: diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 9bc86f5a..5f384934 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -5,6 +5,10 @@ repos: - id: isort files: ^engine args: [--settings-file=engine/pyproject.toml, --filter-files] + - id: isort + name: isort - pd-migrator + files: ^tools/pagerduty-migrator + args: [--settings-file=tools/pagerduty-migrator/.isort.cfg, --filter-files] - repo: https://github.com/psf/black rev: 22.3.0 @@ -12,6 +16,9 @@ repos: - id: black files: ^engine args: [--config=engine/pyproject.toml] + - id: black + name: black - pd-migrator + files: ^tools/pagerduty-migrator - repo: https://github.com/pycqa/flake8 rev: 3.9.2 @@ -21,6 +28,12 @@ repos: args: [--config=engine/tox.ini] additional_dependencies: - flake8-tidy-imports + - id: flake8 + name: flake8 - pd-migrator + files: ^tools/pagerduty-migrator + # Make sure config is compatible with black + # https://black.readthedocs.io/en/stable/the_black_code_style/current_style.html#line-length + args: [--max-line-length=88, "--select=C,E,F,W,B,B950", "--extend-ignore=E203,E501"] - repo: https://github.com/pre-commit/mirrors-eslint rev: v8.25.0 diff --git a/tools/pagerduty-migrator/migrator/report.py b/tools/pagerduty-migrator/migrator/report.py index 39c769ca..4947a44f 100644 --- a/tools/pagerduty-migrator/migrator/report.py +++ b/tools/pagerduty-migrator/migrator/report.py @@ -105,13 +105,13 @@ def escalation_policy_report(policies: list[dict]) -> str: for policy in sorted( policies, key=lambda p: bool(p["unmatched_users"] or p["flawed_schedules"]) ): - result += f"\n" + TAB + format_escalation_policy(policy) + result += "\n" + TAB + format_escalation_policy(policy) for user in policy["unmatched_users"]: - result += f"\n" + TAB * 2 + format_user(user) + result += "\n" + TAB * 2 + format_user(user) for schedule in policy["flawed_schedules"]: - result += f"\n" + TAB * 2 + format_schedule(schedule) + result += "\n" + TAB * 2 + format_schedule(schedule) if ( not policy["unmatched_users"] @@ -135,7 +135,7 @@ def integration_report(integrations: list[dict]) -> str: key=lambda i: bool(i["oncall_type"] and not i["is_escalation_policy_flawed"]), reverse=True, ): - result += f"\n" + TAB + format_integration(integration) + result += "\n" + TAB + format_integration(integration) if ( integration["oncall_type"] and not integration["is_escalation_policy_flawed"] diff --git a/tools/pagerduty-migrator/requirements.txt b/tools/pagerduty-migrator/requirements.txt index 339e1095..8e7340c8 100644 --- a/tools/pagerduty-migrator/requirements.txt +++ b/tools/pagerduty-migrator/requirements.txt @@ -1,6 +1,4 @@ requests==2.27.1 pdpyras==4.5.0 -isort==5.10.1 -black==22.3.0 pytest==7.1.2 pytest-env==0.6.2 \ No newline at end of file From 9ebf20c488ddd22fff0435c18d5bcf8635081e3e Mon Sep 17 00:00:00 2001 From: Vadim Stepanov Date: Fri, 30 Dec 2022 16:11:35 +0000 Subject: [PATCH 09/10] Make user matching by email case-insensitive for PD migrator (#1056) https://github.com/grafana/oncall/issues/1025 --- .../migrator/resources/users.py | 2 +- .../migrator/tests/test_matching.py | 15 ++++----------- .../migrator/tests/test_user_matching.py | 17 +++++++++++++++++ 3 files changed, 22 insertions(+), 12 deletions(-) create mode 100644 tools/pagerduty-migrator/migrator/tests/test_user_matching.py diff --git a/tools/pagerduty-migrator/migrator/resources/users.py b/tools/pagerduty-migrator/migrator/resources/users.py index e616d52c..ba9008f2 100644 --- a/tools/pagerduty-migrator/migrator/resources/users.py +++ b/tools/pagerduty-migrator/migrator/resources/users.py @@ -4,7 +4,7 @@ from migrator.utils import find_by_id def match_user(user: dict, oncall_users: list[dict]) -> None: oncall_user = None for candidate_user in oncall_users: - if user["email"] == candidate_user["email"]: + if user["email"].lower() == candidate_user["email"].lower(): oncall_user = candidate_user break diff --git a/tools/pagerduty-migrator/migrator/tests/test_matching.py b/tools/pagerduty-migrator/migrator/tests/test_matching.py index 4f74e003..556b3782 100644 --- a/tools/pagerduty-migrator/migrator/tests/test_matching.py +++ b/tools/pagerduty-migrator/migrator/tests/test_matching.py @@ -1553,21 +1553,14 @@ expected_integrations_result = [ ] -def test_matching(): - match_users_test() - match_schedule_test() - match_escalation_policies_test() - match_integrations_test() - - -def match_users_test(): +def test_match_user(): for user in pd_users_payload: match_user(user, oncall_users_payload) assert pd_users_payload == expected_users_match_result -def match_schedule_test(): +def test_match_schedule(): for schedule in pd_schedules_payload: match_schedule(schedule, oncall_schedules_payload) match_users_for_schedule(schedule, pd_users_payload) @@ -1575,7 +1568,7 @@ def match_schedule_test(): assert pd_schedules_payload == expected_schedules_result -def match_escalation_policies_test(): +def test_match_escalation_policy(): for policy in pd_escalation_policies_payload: match_escalation_policy(policy, oncall_escalation_chains) match_users_and_schedules_for_escalation_policy( @@ -1585,7 +1578,7 @@ def match_escalation_policies_test(): assert pd_escalation_policies_payload == expected_escalation_policies_result -def match_integrations_test(): +def test_match_integration(): integrations = [] for service in pd_services_payload: service_integrations = service.pop("integrations") diff --git a/tools/pagerduty-migrator/migrator/tests/test_user_matching.py b/tools/pagerduty-migrator/migrator/tests/test_user_matching.py new file mode 100644 index 00000000..539ab693 --- /dev/null +++ b/tools/pagerduty-migrator/migrator/tests/test_user_matching.py @@ -0,0 +1,17 @@ +from migrator.resources.users import match_user + + +def test_match_user_not_found(): + pd_user = {"email": "test@test.com"} + oncall_users = [{"email": "test1@test.com"}] + + match_user(pd_user, oncall_users) + assert pd_user["oncall_user"] is None + + +def test_match_user_email_case_insensitive(): + pd_user = {"email": "test@test.com"} + oncall_users = [{"email": "TEST@TEST.COM"}] + + match_user(pd_user, oncall_users) + assert pd_user["oncall_user"] == oncall_users[0] From 5e297847ae4e2b22bf50b3dbad414b00dddad041 Mon Sep 17 00:00:00 2001 From: Innokentii Konstantinov Date: Tue, 3 Jan 2023 11:04:16 +0800 Subject: [PATCH 10/10] Speedup alert group search --- engine/apps/alerts/models/alert_group.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/engine/apps/alerts/models/alert_group.py b/engine/apps/alerts/models/alert_group.py index 8a00fb83..e201d660 100644 --- a/engine/apps/alerts/models/alert_group.py +++ b/engine/apps/alerts/models/alert_group.py @@ -93,7 +93,7 @@ class AlertGroupQuerySet(models.QuerySet): ) except IntegrityError: try: - return self.get(**search_params, is_open_for_grouping=True), False + return self.get(**search_params, is_open_for_grouping__isnull=False), False except self.model.DoesNotExist: pass raise