Skip to content

Support for generic webhooks #8522

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

Merged
merged 50 commits into from
Nov 9, 2021
Merged
Show file tree
Hide file tree
Changes from 18 commits
Commits
Show all changes
50 commits
Select commit Hold shift + click to select a range
12cd38b
Support for generic webhooks
stsewd Sep 22, 2021
ed703fd
Rename unsub_url -> unsubscribe_url
stsewd Sep 29, 2021
2584afd
Merge branch 'master' into webhooks
stsewd Sep 29, 2021
224622e
Update migrations
stsewd Sep 29, 2021
c531286
Fix merge conflict
stsewd Sep 29, 2021
1a5c5b7
Add more susbtitutions
stsewd Oct 4, 2021
2e03439
Fix
stsewd Oct 4, 2021
9a961e7
Zero downtime
stsewd Oct 4, 2021
297d1a6
Test views
stsewd Oct 4, 2021
73e5efb
fix tests
stsewd Oct 4, 2021
c0689f1
Document generic webhooks
astrojuanlu Oct 21, 2021
c99df5d
Merge branch 'master' into webhooks
stsewd Oct 21, 2021
c6c499b
Whitespace
astrojuanlu Oct 25, 2021
0a031b3
Describe legacy webhooks and migration path
astrojuanlu Oct 25, 2021
9405d4b
Add examples
astrojuanlu Oct 25, 2021
2dd6b39
Fix external version disclaimer
astrojuanlu Oct 25, 2021
3e988a5
Add example to validate webhook signature
astrojuanlu Oct 25, 2021
80446dd
Merge branch 'webhooks' into webhooks-documentation
astrojuanlu Oct 25, 2021
ce31189
Fix markup for Discord webhook example
astrojuanlu Oct 25, 2021
43f9dd4
Clarify that validation is optional
astrojuanlu Oct 25, 2021
e1b426e
Upgrade to use sha256 instead of sha1
stsewd Oct 25, 2021
b6fd66e
Style improvements
astrojuanlu Oct 25, 2021
b443b4f
Fix digest comparison
astrojuanlu Oct 25, 2021
8f55e27
Webhook example improvements
astrojuanlu Oct 25, 2021
39c8ab2
Update external documentation links
astrojuanlu Oct 25, 2021
37484f1
Link to docs
stsewd Oct 25, 2021
fe45d31
Fix screenshot zoom
astrojuanlu Oct 25, 2021
5ff125c
Update test
stsewd Oct 25, 2021
342eea7
Support build.start_date
stsewd Oct 25, 2021
509b09b
Don't show timezone and microseconds
stsewd Oct 25, 2021
e4ea22e
Merge branch 'master' into webhooks
stsewd Nov 2, 2021
2b16a86
Change substitutions format again
stsewd Nov 2, 2021
33f9557
Move payload validation down
astrojuanlu Nov 8, 2021
1077d61
Update template syntax
astrojuanlu Nov 8, 2021
481380d
Merge branch 'webhooks' into webhooks-documentation
astrojuanlu Nov 8, 2021
19eec46
Complete webhook example payload
astrojuanlu Nov 8, 2021
088e629
Add screenshot for custom payload
astrojuanlu Nov 8, 2021
2a0a031
Style improvements
astrojuanlu Nov 8, 2021
a26bd54
Comment
stsewd Nov 8, 2021
a2b6a2a
Merge branch 'master' into webhooks
stsewd Nov 8, 2021
ec9b4fc
Switch to one space for template substitutions
astrojuanlu Nov 8, 2021
b049047
Add examples for URLs
astrojuanlu Nov 8, 2021
94f9fc1
Fix parameter name
astrojuanlu Nov 8, 2021
a928097
Complete remaining variables
astrojuanlu Nov 8, 2021
8f20472
Fix title length
astrojuanlu Nov 8, 2021
04e2953
Fix JSON
astrojuanlu Nov 8, 2021
d1a21f0
Merge pull request #8609 from readthedocs/webhooks-documentation
astrojuanlu Nov 8, 2021
b6dd115
Small updates to docs
stsewd Nov 8, 2021
732ccdd
Merge branch 'master' into webhooks
stsewd Nov 9, 2021
d2611b4
Explicitly test with/out organizations
stsewd Nov 9, 2021
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
2 changes: 0 additions & 2 deletions readthedocs/__init__.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
# -*- coding: utf-8 -*-

"""Read the Docs."""

import os.path
Expand Down
185 changes: 182 additions & 3 deletions readthedocs/builds/tasks.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,13 @@
from datetime import datetime, timedelta
from io import BytesIO

import requests
from celery import Task
from django.conf import settings
from django.urls import reverse
from django.utils.translation import ugettext_lazy as _

from readthedocs import __version__
from readthedocs.api.v2.serializers import BuildSerializer
from readthedocs.api.v2.utils import (
delete_versions_from_db,
Expand All @@ -25,11 +29,11 @@
from readthedocs.builds.models import Build, Version
from readthedocs.builds.utils import memcache_lock
from readthedocs.core.permissions import AdminPermission
from readthedocs.core.utils import trigger_build
from readthedocs.oauth.models import RemoteRepository
from readthedocs.core.utils import send_email, trigger_build
from readthedocs.integrations.models import HttpExchange
from readthedocs.oauth.notifications import GitBuildStatusFailureNotification
from readthedocs.projects.constants import GITHUB_BRAND, GITLAB_BRAND
from readthedocs.projects.models import Project
from readthedocs.projects.models import Project, WebHookEvent
from readthedocs.storage import build_commands_storage
from readthedocs.worker import app

Expand Down Expand Up @@ -453,3 +457,178 @@ def send_build_status(build_pk, commit, status, link_to_build=False):
build.project.slug
)
return False


@app.task(queue='web')
def send_build_notifications(version_pk, build_pk, event):
version = Version.objects.get_object_or_log(pk=version_pk)
if not version or version.type == EXTERNAL:
return

build = Build.objects.filter(pk=build_pk).first()
if not build:
return

sender = BuildNotificationSender(
version=version,
build=build,
event=event,
)
sender.send()


class BuildNotificationSender:

webhook_timeout = 2

def __init__(self, version, build, event):
self.version = version
self.build = build
self.project = version.project
Comment on lines +484 to +487
Copy link
Member

Choose a reason for hiding this comment

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

Why this requires version and build? It should be enough just with the build.

Copy link
Member Author

Choose a reason for hiding this comment

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

The old code was sending the build and version pk separately, don't know if there is a case where a build doesn't have the version attached.

Copy link
Member

Choose a reason for hiding this comment

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

Is it still required with the new code after the refactor? Can we remove it?

Copy link
Member Author

Choose a reason for hiding this comment

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

I have no idea why the old code required it, so not sure about changing that on this PR.

self.event = event

def send(self):
"""
Send email and webhook notifications for `project` about the `build`.

Email notifications are only send for build:failed events.
Webhooks choose to what events they subscribe to.
"""
if self.event == WebHookEvent.BUILD_FAILED:
email_addresses = (
self.project.emailhook_notifications.all()
.values_list('email', flat=True)
)
for email in email_addresses:
try:
self.send_email(email)
except Exception:
log.exception(
'Failed to send email notification. '
'email=%s project=%s version=%s build=%s',
email, self.project.slug, self.version.slug, self.build.pk,
)

webhooks = (
self.project.webhook_notifications
.filter(events__name=self.event)
)
for webhook in webhooks:
try:
self.send_webhook(webhook)
except Exception:
log.exception(
'Failed to send webhook. webhook=%s project=%s version=%s build=%s',
webhook.id, self.project.slug, self.version.slug, self.build.pk,
)

def send_email(self, email):
"""Send email notifications for build failures."""
# We send only what we need from the Django model objects here to avoid
# serialization problems in the ``readthedocs.core.tasks.send_email_task``
protocol = 'http' if settings.DEBUG else 'https'
context = {
'version': {
'verbose_name': self.version.verbose_name,
},
'project': {
'name': self.project.name,
},
'build': {
'pk': self.build.pk,
'error': self.build.error,
},
'build_url': '{}://{}{}'.format(
protocol,
settings.PRODUCTION_DOMAIN,
self.build.get_absolute_url(),
),
'unsubscribe_url': '{}://{}{}'.format(
protocol,
settings.PRODUCTION_DOMAIN,
reverse('projects_notifications', args=[self.project.slug]),
),
}

if self.build.commit:
title = _('Failed: {project[name]} ({commit})').format(
commit=self.build.commit[:8],
**context,
)
else:
title = _('Failed: {project[name]} ({version[verbose_name]})').format(
**context
)

log.info(
'Sending email notification. email=%s project=%s version=%s build=%s',
email, self.project.slug, self.version.slug, self.build.id,
)
send_email(
email,
title,
template='projects/email/build_failed.txt',
template_html='projects/email/build_failed.html',
context=context,
)

def send_webhook(self, webhook):
"""
Send webhook notification.

The payload is signed using HMAC-SHA256,
for users to be able to verify the authenticity of the request.

Webhooks that don't have a payload,
are from the old implementation, for those we keep sending the
old default payload.

An HttpExchange object is created for each transaction.
"""
payload = webhook.get_payload(
version=self.version,
build=self.build,
event=self.event,
)
if not payload:
# Default payload from old webhooks.
payload = json.dumps({
'name': self.project.name,
'slug': self.project.slug,
'build': {
'id': self.build.id,
'commit': self.build.commit,
'state': self.build.state,
'success': self.build.success,
'date': self.build.date.strftime('%Y-%m-%d %H:%M:%S'),
},
})

headers = {
'content-type': 'application/json',
'User-Agent': f'Read-the-Docs/{__version__} ({settings.PRODUCTION_DOMAIN})',
'X-RTD-Event': self.event,
}
if webhook.secret:
headers['X-Hub-Signature'] = webhook.sign_payload(payload)

try:
log.info(
'Sending webhook notification. webhook=%s project=%s version=%s build=%s',
webhook.pk, self.project.slug, self.version.slug, self.build.pk,
)
response = requests.post(
webhook.url,
data=payload,
headers=headers,
timeout=self.webhook_timeout,
)
HttpExchange.objects.from_requests_exchange(
response=response,
related_object=webhook,
)
Comment on lines +626 to +629
Copy link
Member Author

Choose a reason for hiding this comment

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

We could create this object in two steps, before sending the request and updating it after the request was made. This is so we save the exchange even if the request failed (like timeout).

except Exception:
log.exception(
'Failed to POST to webhook url. webhook=%s url=%s',
webhook.pk, webhook.url,
)
12 changes: 8 additions & 4 deletions readthedocs/core/utils/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,10 +56,10 @@ def prepare_build(
"""
# Avoid circular import
from readthedocs.builds.models import Build
from readthedocs.projects.models import Feature, Project
from readthedocs.builds.tasks import send_build_notifications
from readthedocs.projects.models import Feature, Project, WebHookEvent
from readthedocs.projects.tasks import (
send_external_build_status,
send_notifications,
update_docs_task,
)

Expand Down Expand Up @@ -123,8 +123,12 @@ def prepare_build(
)

if build and version.type != EXTERNAL:
# Send Webhook notification for build triggered.
send_notifications.delay(version.pk, build_pk=build.pk, email=False)
# Send notifications for build triggered.
send_build_notifications.delay(
version_pk=version.pk,
build_pk=build.pk,
event=WebHookEvent.BUILD_TRIGGERED,
)

options['priority'] = CELERY_HIGH
if project.main_language_project:
Expand Down
21 changes: 19 additions & 2 deletions readthedocs/integrations/models.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
# -*- coding: utf-8 -*-

"""Integration models for external services."""

import json
Expand Down Expand Up @@ -96,6 +94,25 @@ def from_exchange(self, req, resp, related_object, payload=None):
self.delete_limit(related_object)
return obj

def from_requests_exchange(self, response, related_object):
"""
Create an exchange object from a requests' response.

:param response: The result from calling request.post() or similar.
:param related_object: Object to use for generic relationship.
"""
request = response.request
obj = self.create(
related_object=related_object,
request_headers=request.headers or {},
request_body=request.body or '',
status_code=response.status_code,
response_headers=response.headers,
response_body=response.text,
)
self.delete_limit(related_object)
return obj

def delete_limit(self, related_object, limit=10):
if isinstance(related_object, Integration):
queryset = self.filter(integrations=related_object)
Expand Down
4 changes: 3 additions & 1 deletion readthedocs/projects/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,13 @@
EmailHook,
EnvironmentVariable,
Feature,
HTTPHeader,
HTMLFile,
HTTPHeader,
ImportedFile,
Project,
ProjectRelationship,
WebHook,
WebHookEvent,
)
from .notifications import (
DeprecatedBuildWebhookNotification,
Expand Down Expand Up @@ -410,4 +411,5 @@ class EnvironmentVariableAdmin(admin.ModelAdmin):
admin.site.register(Feature, FeatureAdmin)
admin.site.register(EmailHook)
admin.site.register(WebHook)
admin.site.register(WebHookEvent)
admin.site.register(HTMLFile, ImportedFileAdmin)
47 changes: 37 additions & 10 deletions readthedocs/projects/forms.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
"""Project forms."""
import json
from random import choice
from re import fullmatch
from urllib.parse import urlparse
Expand Down Expand Up @@ -444,23 +445,49 @@ def save(self):

class WebHookForm(forms.ModelForm):

"""Project webhook form."""
project = forms.CharField(widget=forms.HiddenInput(), required=False)

class Meta:
model = WebHook
fields = ['project', 'url', 'events', 'payload', 'secret']
widgets = {
'events': forms.CheckboxSelectMultiple,
Copy link
Member Author

Choose a reason for hiding this comment

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

I think this is better than the list, and we don't have that many events. I wanted to show something similar to what github does (the name and the description)
Screenshot 2021-09-28 at 15-07-07 Webhook · https requires io github web-hook · readthedocs readthedocs org

but looks like we would need to just manually do that, there isn't a widget that does it for us :/

}

def __init__(self, *args, **kwargs):
self.project = kwargs.pop('project', None)
super().__init__(*args, **kwargs)

def save(self, commit=True):
self.webhook = WebHook.objects.get_or_create(
url=self.cleaned_data['url'],
project=self.project,
)[0]
self.project.webhook_notifications.add(self.webhook)
if self.instance and self.instance.pk:
# Show secret in the detail form, but as readonly.
self.fields['secret'].disabled = True
else:
# Don't show the secret in the creation form.
self.fields.pop('secret')
self.fields['payload'].initial = json.dumps({
'event': '{{ event }}',
'name': '{{ project.name }}',
'slug': '{{ project.slug }}',
'version': '{{ version.slug }}',
'commit': '{{ build.commit }}',
'build': '{{ build.id }}',
'start_date': '{{ build.start_date }}',
'build_url': '{{ build.url }}',
'docs_url': '{{ build.docs_url }}',
}, indent=2)

def clean_project(self):
return self.project

class Meta:
model = WebHook
fields = ['url']
def clean_payload(self):
"""Check if the payload is a valid json object and format it."""
payload = self.cleaned_data['payload']
try:
payload = json.loads(payload)
payload = json.dumps(payload, indent=2)
except Exception:
raise forms.ValidationError(_('The payload must be a valid JSON object.'))
return payload


class TranslationBaseForm(forms.Form):
Expand Down
Loading