From 7ebc9cbbf759ea994ed30c7fb91e3e11ec677800 Mon Sep 17 00:00:00 2001 From: Joey Orlando Date: Tue, 20 Dec 2022 12:41:34 +0100 Subject: [PATCH] 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, }), ],