Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(uptime): Enforce per-domain limits #77857

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/sentry/uptime/detectors/url_extraction.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from django.core.exceptions import ValidationError
from django.core.validators import URLValidator, validate_ipv46_address
from tldextract import TLDExtract
from tldextract.tldextract import ExtractResult

if TYPE_CHECKING:
pass
Expand Down Expand Up @@ -51,3 +52,9 @@ def extract_base_url(url: str | None) -> str | None:
extracted_url = extractor.extract_urllib(split_url)
fqdn = extracted_url.fqdn
return f"{split_url.scheme}://{fqdn}" if fqdn else None


def extract_domain_parts(url: str) -> ExtractResult:
# We enable private PSL domains so that hosting services that use
# subdomains are treated as suffixes for the purposes of monitoring.
return extractor.extract_str(url, include_psl_private_domains=True)
29 changes: 28 additions & 1 deletion src/sentry/uptime/endpoints/validators.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,27 @@
from sentry.api.fields import ActorField
from sentry.api.serializers.rest_framework import CamelSnakeSerializer
from sentry.auth.superuser import is_active_superuser
from sentry.uptime.models import ProjectUptimeSubscriptionMode
from sentry.uptime.detectors.url_extraction import extract_domain_parts
from sentry.uptime.models import ProjectUptimeSubscription, ProjectUptimeSubscriptionMode
from sentry.uptime.subscriptions.subscriptions import (
create_project_uptime_subscription,
create_uptime_subscription,
)
from sentry.utils.audit import create_audit_entry

MAX_MONITORS_PER_DOMAIN = 100
"""
The bounding upper limit on how many ProjectUptimeSubscription's can exist for
a single domain + suffix.

This takes into accunt subdomains by including them in the count. For example,
for the domain `sentry.io` both the hosts `subdomain-one.sentry.io` and
`subdomain-2.sentry.io` will both count towards the limit

Importantly domains like `vercel.dev` are considered TLDs as defined by the
public suffix list (PSL). See `extract_domain_parts` fo more details
"""


@extend_schema_serializer()
class UptimeMonitorValidator(CamelSnakeSerializer):
Expand All @@ -34,6 +48,19 @@ class UptimeMonitorValidator(CamelSnakeSerializer):
)
mode = serializers.IntegerField(required=False)

def validate_url(self, url):
url_parts = extract_domain_parts(url)
existing_count = ProjectUptimeSubscription.objects.filter(
uptime_subscription__url_domain=url_parts.domain,
uptime_subscription__url_domain_suffix=url_parts.suffix,
).count()
Comment on lines +52 to +56
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One thought here is that maybe this should happen specifically in the creation function. Since we have the de-duping, it's valid to create a monitor on an existing url even if we're at the limit, since it doesn't cause any more hits to that url.

Maybe the create function could raise an exception that we catch and translate in save?

Maybe also just an edge case and not a big deal

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh right, hmm. But you can edit a monitor to change the url as well right. Yeah I guess it could be an exception.

Let's not worry about it for now.


if existing_count >= MAX_MONITORS_PER_DOMAIN:
raise serializers.ValidationError(
f"The domain *.{url_parts.domain}.{url_parts.suffix} has already been used in {MAX_MONITORS_PER_DOMAIN} uptime monitoring alerts, which is the limit. You cannot create any additional alerts for this domain."
)
return url

def validate_mode(self, mode):
if not is_active_superuser(self.context["request"]):
raise serializers.ValidationError("Only superusers can modify `mode`")
Expand Down
6 changes: 2 additions & 4 deletions src/sentry/uptime/subscriptions/subscriptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

from sentry.models.project import Project
from sentry.types.actor import Actor
from sentry.uptime.detectors.url_extraction import extractor
from sentry.uptime.detectors.url_extraction import extract_domain_parts
from sentry.uptime.models import (
ProjectUptimeSubscription,
ProjectUptimeSubscriptionMode,
Expand Down Expand Up @@ -32,9 +32,7 @@ def create_uptime_subscription(
"""
# We extract the domain and suffix of the url here. This is used to prevent there being too many checks to a single
# domain.
# We enable private PSL domains so that hosting services that use subdomains are treated as suffixes for the
# purposes of monitoring.
result = extractor.extract_str(url, include_psl_private_domains=True)
result = extract_domain_parts(url)
subscription, created = UptimeSubscription.objects.get_or_create(
url=url,
interval_seconds=interval_seconds,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
from unittest import mock

import pytest
from rest_framework.exceptions import ErrorDetail

Expand Down Expand Up @@ -92,6 +94,32 @@ def test_not_found(self):
resp = self.get_error_response(self.organization.slug, self.project.slug, 3)
assert resp.status_code == 404

@mock.patch("sentry.uptime.endpoints.validators.MAX_MONITORS_PER_DOMAIN", 1)
def test_domain_limit(self):
# First monitor is for test-one.example.com
self.create_project_uptime_subscription(
uptime_subscription=self.create_uptime_subscription(
url="test-one.example.com",
url_domain="example",
url_domain_suffix="com",
)
)

# Update second monitor to use the same domain. This will fail with a
# validation error
uptime_subscription = self.create_project_uptime_subscription()
resp = self.get_error_response(
self.organization.slug,
uptime_subscription.project.slug,
uptime_subscription.id,
status_code=400,
url="https://test-two.example.com",
)
assert (
resp.data["url"][0]
== "The domain *.example.com has already been used in 1 uptime monitoring alerts, which is the limit. You cannot create any additional alerts for this domain."
)


class ProjectUptimeAlertDetailsDeleteEndpointTest(ProjectUptimeAlertDetailsBaseEndpointTest):
method = "delete"
Expand Down
Loading