Merge pull request #212 from grafana/integration-url-path-prefix

Fix integrations giving the wrong URL when using a path prefix

engine/apps/alerts/models/alert_receive_channel.py

@@ -32,6 +32,7 @@ from apps.slack.constants import SLACK_RATE_LIMIT_DELAY, SLACK_RATE_LIMIT_TIMEOU
 from apps.slack.tasks import post_slack_rate_limit_message
 from apps.slack.utils import post_message_to_channel
 from apps.user_management.organization_log_creator import OrganizationLogType, create_organization_log
+from common.api_helpers.utils import create_engine_url
 from common.exceptions import TeamCanNotBeChangedError, UnableToSendDemoAlert
 from common.public_primary_keys import generate_public_primary_key, increase_public_primary_key_length
 
@@ -497,10 +498,7 @@ class AlertReceiveChannel(IntegrationOptionsMixin, MaintainableObject):
             AlertReceiveChannel.INTEGRATION_MAINTENANCE,
         ]:
             return None
-        return urljoin(
-            settings.BASE_URL,
-            f"/integrations/v1/{self.config.slug}/{self.token}/",
-        )
+        return create_engine_url(f"integrations/v1/{self.config.slug}/{self.token}/")
 
     @property
     def inbound_email(self):

engine/apps/api/views/schedule.py

@@ -1,8 +1,6 @@
 import datetime
-from urllib.parse import urljoin
 
 import pytz
-from django.conf import settings
 from django.core.exceptions import ObjectDoesNotExist
 from django.db.models import OuterRef, Subquery
 from django.db.utils import IntegrityError
@@ -38,6 +36,7 @@ from common.api_helpers.mixins import (
     ShortSerializerMixin,
     UpdateSerializerMixin,
 )
+from common.api_helpers.utils import create_engine_url
 
 
 class ScheduleView(
@@ -328,10 +327,9 @@ class ScheduleView(
             except IntegrityError:
                 raise Conflict("Schedule export token for user already exists")
 
-            export_url = urljoin(
-                settings.BASE_URL,
+            export_url = create_engine_url(
                 reverse("api-public:schedules-export", kwargs={"pk": schedule.public_primary_key})
-                + f"?{SCHEDULE_EXPORT_TOKEN_NAME}={token}",
+                + f"?{SCHEDULE_EXPORT_TOKEN_NAME}={token}"
             )
 
             data = {"token": token, "created_at": instance.created_at, "export_url": export_url}

engine/apps/api/views/user.py

@@ -1,5 +1,4 @@
 import logging
-from urllib.parse import urljoin
 
 import pytz
 from django.apps import apps
@@ -45,6 +44,7 @@ from apps.user_management.organization_log_creator import OrganizationLogType, c
 from common.api_helpers.exceptions import Conflict
 from common.api_helpers.mixins import FilterSerializerMixin, PublicPrimaryKeyMixin
 from common.api_helpers.paginators import HundredPageSizePaginator
+from common.api_helpers.utils import create_engine_url
 from common.constants.role import Role
 
 logger = logging.getLogger(__name__)
@@ -411,10 +411,9 @@ class UserView(
             except IntegrityError:
                 raise Conflict("Schedule export token for user already exists")
 
-            export_url = urljoin(
-                settings.BASE_URL,
+            export_url = create_engine_url(
                 reverse("api-public:users-schedule-export", kwargs={"pk": user.public_primary_key})
-                + f"?{SCHEDULE_EXPORT_TOKEN_NAME}={token}",
+                + f"?{SCHEDULE_EXPORT_TOKEN_NAME}={token}"
             )
 
             data = {"token": token, "created_at": instance.created_at, "export_url": export_url}

engine/apps/oss_installation/cloud_heartbeat.py

@@ -23,7 +23,7 @@ def setup_heartbeat_integration(name=None):
     # don't specify a team in the data, so heartbeat integration will be created in the General.
     name = name or f"OnCall Cloud Heartbeat {settings.BASE_URL}"
     data = {"type": "formatted_webhook", "name": name}
-    url = urljoin(settings.GRAFANA_CLOUD_ONCALL_API_URL, "/api/v1/integrations/")
+    url = urljoin(settings.GRAFANA_CLOUD_ONCALL_API_URL, "api/v1/integrations/")
     try:
         headers = {"Authorization": api_token}
         r = requests.post(url=url, data=data, headers=headers, timeout=5)

engine/apps/slack/scenarios/resolution_note.py

@@ -1,14 +1,13 @@
 import json
 import logging
-from urllib.parse import urljoin
 
 from django.apps import apps
-from django.conf import settings
 from django.db.models import Q
 from django.utils import timezone
 
 from apps.slack.scenarios import scenario_step
 from apps.slack.slack_client.exceptions import SlackAPIException
+from common.api_helpers.utils import create_engine_url
 
 from .step_mixins import CheckAlertIsUnarchivedMixin
 
@@ -607,7 +606,7 @@ class ResolutionNoteModalStep(CheckAlertIsUnarchivedMixin, scenario_step.Scenari
 
         if not blocks:
             # there aren't any resolution notes yet, display a hint instead
-            link_to_instruction = urljoin(settings.BASE_URL, "static/images/postmortem.gif")
+            link_to_instruction = create_engine_url("static/images/postmortem.gif")
             blocks = [
                 {
                     "type": "divider",
@@ -633,7 +632,7 @@ class ResolutionNoteModalStep(CheckAlertIsUnarchivedMixin, scenario_step.Scenari
         return blocks
 
     def get_invite_bot_tip_blocks(self, channel):
-        link_to_instruction = urljoin(settings.BASE_URL, "static/images/postmortem.gif")
+        link_to_instruction = create_engine_url("static/images/postmortem.gif")
         blocks = [
             {
                 "type": "divider",

engine/apps/slack/tests/test_scenario_steps/test_resolution_note.py

@@ -1,10 +1,9 @@
 import json
-from urllib.parse import urljoin
 
 import pytest
-from django.conf import settings
 
 from apps.slack.scenarios.scenario_step import ScenarioStep
+from common.api_helpers.utils import create_engine_url
 
 
 @pytest.mark.django_db
@@ -22,7 +21,7 @@ def test_get_resolution_notes_blocks_default_if_empty(
 
     blocks = step.get_resolution_notes_blocks(alert_group, "", False)
 
-    link_to_instruction = urljoin(settings.BASE_URL, "static/images/postmortem.gif")
+    link_to_instruction = create_engine_url("static/images/postmortem.gif")
     expected_blocks = [
         {
             "type": "divider",

engine/apps/twilioapp/twilio_client.py

@@ -2,7 +2,6 @@ import logging
 import urllib.parse
 
 from django.apps import apps
-from django.conf import settings
 from django.urls import reverse
 from twilio.base.exceptions import TwilioRestException
 from twilio.rest import Client
@@ -10,6 +9,7 @@ from twilio.rest import Client
 from apps.base.utils import live_settings
 from apps.twilioapp.constants import TEST_CALL_TEXT, TwilioLogRecordStatus, TwilioLogRecordType
 from apps.twilioapp.utils import get_calling_code, get_gather_message, get_gather_url, parse_phone_number
+from common.api_helpers.utils import create_engine_url
 
 logger = logging.getLogger(__name__)
 
@@ -24,7 +24,7 @@ class TwilioClient:
         return live_settings.TWILIO_NUMBER
 
     def send_message(self, body, to):
-        status_callback = settings.BASE_URL + reverse("twilioapp:sms_status_events")
+        status_callback = create_engine_url(reverse("twilioapp:sms_status_events"))
         try:
             return self.twilio_api_client.messages.create(
                 body=body, to=to, from_=self.twilio_number, status_callback=status_callback
@@ -143,7 +143,7 @@ class TwilioClient:
             )
 
             url = "http://twimlets.com/echo?Twiml=" + twiml_query
-            status_callback = settings.BASE_URL + reverse("twilioapp:call_status_events")
+            status_callback = create_engine_url(reverse("twilioapp:call_status_events"))
 
             status_callback_events = ["initiated", "ringing", "answered", "completed"]
 

engine/apps/twilioapp/utils.py

@@ -3,11 +3,12 @@ import re
 from string import digits
 
 from django.apps import apps
-from django.conf import settings
 from django.urls import reverse
 from phonenumbers import COUNTRY_CODE_TO_REGION_CODE
 from twilio.twiml.voice_response import Gather, VoiceResponse
 
+from common.api_helpers.utils import create_engine_url
+
 logger = logging.getLogger(__name__)
 
 
@@ -19,7 +20,7 @@ def get_calling_code(iso):
 
 
 def get_gather_url():
-    gather_url = settings.BASE_URL + reverse("twilioapp:gather")
+    gather_url = create_engine_url(reverse("twilioapp:gather"))
     return gather_url
 
 

engine/common/api_helpers/utils.py

@@ -1,3 +1,5 @@
+from urllib.parse import urljoin
+
 import requests
 from django.conf import settings
 from icalendar import Calendar
@@ -50,3 +52,11 @@ def validate_ical_url(url):
             raise serializers.ValidationError("Ical parse failed")
         return url
     return None
+
+
+def create_engine_url(path):
+    base = settings.BASE_URL
+    if not base.endswith("/"):
+        base += "/"
+    trimmed_path = path.lstrip("/")
+    return urljoin(base, trimmed_path)

engine/common/tests/test_create_engine_url.py

@@ -0,0 +1,35 @@
+from django.test.utils import override_settings
+
+from common.api_helpers.utils import create_engine_url
+
+
+@override_settings(BASE_URL="http://localhost:8000")
+def test_create_engine_url_no_slash():
+    assert create_engine_url("destination") == "http://localhost:8000/destination"
+    assert create_engine_url("/destination") == "http://localhost:8000/destination"
+    assert create_engine_url("destination/") == "http://localhost:8000/destination/"
+    assert create_engine_url("/destination/") == "http://localhost:8000/destination/"
+
+
+@override_settings(BASE_URL="http://localhost:8000/")
+def test_create_engine_url_slash():
+    assert create_engine_url("destination") == "http://localhost:8000/destination"
+    assert create_engine_url("/destination") == "http://localhost:8000/destination"
+    assert create_engine_url("destination/") == "http://localhost:8000/destination/"
+    assert create_engine_url("/destination/") == "http://localhost:8000/destination/"
+
+
+@override_settings(BASE_URL="http://localhost:8000/test123")
+def test_create_engine_url_prefix_no_slash():
+    assert create_engine_url("destination") == "http://localhost:8000/test123/destination"
+    assert create_engine_url("/destination") == "http://localhost:8000/test123/destination"
+    assert create_engine_url("destination/") == "http://localhost:8000/test123/destination/"
+    assert create_engine_url("/destination/") == "http://localhost:8000/test123/destination/"
+
+
+@override_settings(BASE_URL="http://localhost:8000/test123/")
+def test_create_engine_url_prefix_slash():
+    assert create_engine_url("destination") == "http://localhost:8000/test123/destination"
+    assert create_engine_url("/destination") == "http://localhost:8000/test123/destination"
+    assert create_engine_url("destination/") == "http://localhost:8000/test123/destination/"
+    assert create_engine_url("/destination/") == "http://localhost:8000/test123/destination/"