From cdec4329a3569311381cba3eadd4de423f16ef59 Mon Sep 17 00:00:00 2001 From: Manuel Kaufmann Date: Fri, 11 Jan 2019 02:51:40 +0100 Subject: [PATCH] Notify users about the usage of deprecated webhooks (#4898) * Notify users about the usage of deprecated webhooks Each time a deprecated webhook is hit, a notification is created (without duplicating it) to be sent. * Extend notification to support de-dup and delayed email sent * Improve decorator to support generic and specific VCS webhook views * Remove no necessary settings * DeprecatedWebhookEndpointNotification tests and improvements * Better docstring * Lint * Update copy on notifications for github services deprecation (#5067) * Updated copy on webhooks * Drop "deprecated webhook endpoint" copy, this is core team nomenclature, not user nomenclature. * Add small amount of docs to point to * Update docs and point to docs in notification message * Add year * Split up deprecated view notification to GitHub and other webhook endpoints (#5083) * Updated copy on webhooks * Drop "deprecated webhook endpoint" copy, this is core team nomenclature, not user nomenclature. * Add small amount of docs to point to * Update docs and point to docs in notification message * Split up deprecated view notification to GitHub and other webhook endpoints This sets a date for deprecated of these endpoints as Mar 1st 2019. Too soon? * Reduce complexity and drop decorator pattern for Notification classmethod pattern used in other notifications * Add notifications for non-GitHub incoming webhooks * Add docs as well * More renaming and slight refactor Found out 2x messages are being generated, so this stops the automated mechanism for triggering these messages. * Update dates * Also update docs * Typo on date * Back out some more of the changes to notifications to make them operable without automation * Add admin method for notification * Add admin filter for project features --- docs/webhooks.rst | 76 +++++++++++++++++++ readthedocs/core/views/hooks.py | 5 +- readthedocs/notifications/backends.py | 13 ++-- readthedocs/notifications/notification.py | 1 + readthedocs/projects/admin.py | 14 +++- readthedocs/projects/notifications.py | 42 ++++++++++ .../rtd_tests/tests/test_notifications.py | 48 ++++++++++++ .../deprecated_build_webhook_email.html | 6 ++ .../deprecated_build_webhook_site.html | 1 + .../deprecated_github_webhook_email.html | 8 ++ .../deprecated_github_webhook_site.html | 1 + 11 files changed, 205 insertions(+), 10 deletions(-) create mode 100644 readthedocs/templates/projects/notifications/deprecated_build_webhook_email.html create mode 100644 readthedocs/templates/projects/notifications/deprecated_build_webhook_site.html create mode 100644 readthedocs/templates/projects/notifications/deprecated_github_webhook_email.html create mode 100644 readthedocs/templates/projects/notifications/deprecated_github_webhook_site.html diff --git a/docs/webhooks.rst b/docs/webhooks.rst index ecf18611b50..7e6e109bf3e 100644 --- a/docs/webhooks.rst +++ b/docs/webhooks.rst @@ -20,6 +20,8 @@ details and a list of HTTP exchanges that have taken place for the integration. You need this information for the URL, webhook, or Payload URL needed by the repository provider such as GitHub, GitLab, or Bitbucket. +.. _webhook-creation: + Webhook Creation ---------------- @@ -36,6 +38,8 @@ As an example, the URL pattern looks like this: *readthedocs.org/api/v2/webhook/ Use this URL when setting up a new webhook with your provider -- these steps vary depending on the provider: +.. _webhook-integration-github: + GitHub ~~~~~~ @@ -53,6 +57,8 @@ For a 403 error, it's likely that the Payload URL is incorrect. .. note:: The webhook token, intended for the GitHub **Secret** field, is not yet implemented. +.. _webhook-integration-bitbucket: + Bitbucket ~~~~~~~~~ @@ -63,6 +69,8 @@ Bitbucket * Under **Triggers**, **Repository push** should be selected * Finish by clicking **Save** +.. _webhook-integration-gitlab: + GitLab ~~~~~~ @@ -73,6 +81,8 @@ GitLab * Leave the default **Push events** selected and mark **Tag push events** also * Finish by clicking **Add Webhook** +.. _webhook-integration-generic: + Using the generic API integration --------------------------------- @@ -136,3 +146,69 @@ Resyncing webhooks It might be necessary to re-establish a webhook if you are noticing problems. To resync a webhook from Read the Docs, visit the integration detail page and follow the directions for re-syncing your repository webhook. + +Troubleshooting +--------------- + +My project isn't automatically building +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +If your project isn't automatically building, you can check your integration on +Read the Docs to see the payload sent to our servers. If there is no recent +activity on your Read the Docs project webhook integration, then it's likely +that your VCS provider is not configured correctly. If there is payload +information on your Read the Docs project, you might need to verify that your +versions are configured to build correctly. + +Either way, it may help to either resync your webhook intergration (see +`Resyncing webhooks`_ for information on this process), or set up an entirely +new webhook intergration. + +.. _webhook-github-services: + +I was warned I shouldn't use GitHub Services +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +Last year, GitHub announced that effective Jan 31st, 2019, GitHub Services will stop +working [1]_. This means GitHub will stop sending notifications to Read the Docs +for projects configured with the ``ReadTheDocs`` GitHub Service. If your project +has been configured on Read the Docs for a long time, you are most likely still +using this service to automatically build your project on Read the Docs. + +In order for your project to continue automatically building, you will need to +configure your GitHub repository with a new webhook. You can use either a +connected GitHub account and a :ref:`GitHub webhook integration ` +on your Read the Docs project, or you can use a +:ref:`generic webhook integraiton ` without a connected +account. + +.. [1] https://developer.github.com/changes/2018-04-25-github-services-deprecation/ + +.. _webhook-deprecated-endpoints: + +I was warned that my project won't automatically build after April 1st +~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ + +In addition to :ref:`no longer supporting GitHub Services `, +we have decided to no longer support several other legacy incoming webhook +endpoints that were used before we introduced project webhook integrations. When +we introduced our webhook integrations, we added several features and improved +security for incoming webhooks and these features were not added to our leagcy +incoming webhooks. New projects have not been able to use our legacy incoming +webhooks since, however if you have a project that has been established for a +while, you may still be using these endpoints. + +After March 1st, 2019, we will stop accepting incoming webhook notifications for +these legacy incoming webhooks. Your project will need to be reconfigured and +have a webhook integration configured, pointing to a new webhook with your VCS +provider. + +In particular, the incoming webhook URLs that will be removed are: + +* ``https://readthedocs.org/build`` +* ``https://readthedocs.org/bitbucket`` +* ``https://readthedocs.org/github`` (as noted :ref:`above `) +* ``https://readthedocs.org/gitlab`` + +In order to establish a new project webhook integration, :ref:`follow +the directions for your VCS provider ` diff --git a/readthedocs/core/views/hooks.py b/readthedocs/core/views/hooks.py index 6b00b2d77ef..c6d4bc91188 100644 --- a/readthedocs/core/views/hooks.py +++ b/readthedocs/core/views/hooks.py @@ -370,7 +370,10 @@ def generic_build(request, project_id_or_slug=None): if request.method == 'POST': slug = request.POST.get('version_slug', project.default_version) log.info( - "(Incoming Generic Build) %s [%s]", project.slug, slug) + "(Incoming Generic Build) %s [%s]", + project.slug, + slug, + ) _build_version(project, slug) else: return HttpResponse("You must POST to this resource.") diff --git a/readthedocs/notifications/backends.py b/readthedocs/notifications/backends.py index 28529f1f1a6..6cc794ddbbf 100644 --- a/readthedocs/notifications/backends.py +++ b/readthedocs/notifications/backends.py @@ -32,11 +32,7 @@ def send_notification(request, notification): backends = getattr(settings, 'NOTIFICATION_BACKENDS', []) for cls_name in backends: backend = import_string(cls_name)(request) - # Do not send email notification if defined explicitly - if backend.name == EmailBackend.name and not notification.send_email: - pass - else: - backend.send(notification) + backend.send(notification) class Backend(object): @@ -55,11 +51,16 @@ class EmailBackend(Backend): The content body is first rendered from an on-disk template, then passed into the standard email templates as a string. + + If the notification is set to ``send_email=False``, this backend will exit + early from :py:meth:`send`. """ name = 'email' def send(self, notification): + if not notification.send_email: + return # FIXME: if the level is an ERROR an email is received and sometimes # it's not necessary. This behavior should be clearly documented in the # code @@ -114,6 +115,6 @@ def send(self, notification): backend_name=self.name, source_format=HTML, ), - extra_tags='', + extra_tags=notification.extra_tags, user=notification.user, ) diff --git a/readthedocs/notifications/notification.py b/readthedocs/notifications/notification.py index d2300e1f4f0..c4d1ca4de77 100644 --- a/readthedocs/notifications/notification.py +++ b/readthedocs/notifications/notification.py @@ -35,6 +35,7 @@ class Notification(object): subject = None user = None send_email = True + extra_tags = '' def __init__(self, context_object, request, user=None): self.object = context_object diff --git a/readthedocs/projects/admin.py b/readthedocs/projects/admin.py index d55954fdbcf..4640a7abc73 100644 --- a/readthedocs/projects/admin.py +++ b/readthedocs/projects/admin.py @@ -30,12 +30,20 @@ ProjectRelationship, WebHook, ) -from .notifications import ResourceUsageNotification +from .notifications import ( + ResourceUsageNotification, + DeprecatedBuildWebhookNotification, + DeprecatedGitHubWebhookNotification, +) from .tasks import remove_dir class ProjectSendNotificationView(SendNotificationView): - notification_classes = [ResourceUsageNotification] + notification_classes = [ + ResourceUsageNotification, + DeprecatedBuildWebhookNotification, + DeprecatedGitHubWebhookNotification, + ] def get_object_recipients(self, obj): for owner in obj.users.all(): @@ -119,7 +127,7 @@ class ProjectAdmin(GuardedModelAdmin): list_display = ('name', 'slug', 'repo', 'repo_type', 'featured') list_filter = ('repo_type', 'featured', 'privacy_level', 'documentation_type', 'programming_language', - ProjectOwnerBannedFilter) + 'feature__feature_id', ProjectOwnerBannedFilter) list_editable = ('featured',) search_fields = ('slug', 'repo') inlines = [ProjectRelationshipInline, RedirectInline, diff --git a/readthedocs/projects/notifications.py b/readthedocs/projects/notifications.py index eafbecf66c3..db7838bc80e 100644 --- a/readthedocs/projects/notifications.py +++ b/readthedocs/projects/notifications.py @@ -1,6 +1,11 @@ +# -*- coding: utf-8 -*- """Project notifications""" from __future__ import absolute_import +from datetime import timedelta +from django.utils import timezone +from django.http import HttpRequest +from messages_extends.models import Message from readthedocs.notifications import Notification from readthedocs.notifications.constants import REQUIREMENT @@ -11,3 +16,40 @@ class ResourceUsageNotification(Notification): context_object_name = 'project' subject = 'Builds for {{ project.name }} are using too many resources' level = REQUIREMENT + + +class DeprecatedViewNotification(Notification): + + """Notification to alert user of a view that is going away.""" + + context_object_name = 'project' + subject = '{{ project.name }} project webhook needs to be updated' + level = REQUIREMENT + + @classmethod + def notify_project_users(cls, projects): + """ + Notify project users of deprecated view. + + :param projects: List of project instances + :type projects: [:py:class:`Project`] + """ + for project in projects: + # Send one notification to each owner of the project + for user in project.users.all(): + notification = cls( + context_object=project, + request=HttpRequest(), + user=user, + ) + notification.send() + + +class DeprecatedGitHubWebhookNotification(DeprecatedViewNotification): + + name = 'deprecated_github_webhook' + + +class DeprecatedBuildWebhookNotification(DeprecatedViewNotification): + + name = 'deprecated_build_webhook' diff --git a/readthedocs/rtd_tests/tests/test_notifications.py b/readthedocs/rtd_tests/tests/test_notifications.py index 2213b0d8c7c..1fa16323df4 100644 --- a/readthedocs/rtd_tests/tests/test_notifications.py +++ b/readthedocs/rtd_tests/tests/test_notifications.py @@ -2,16 +2,24 @@ """Notification tests""" from __future__ import absolute_import +from datetime import timedelta import mock import django_dynamic_fixture as fixture +from django.http import HttpRequest from django.test import TestCase from django.test.utils import override_settings from django.contrib.auth.models import User, AnonymousUser +from django.utils import timezone from messages_extends.models import Message as PersistentMessage from readthedocs.notifications import Notification, SiteNotification from readthedocs.notifications.backends import EmailBackend, SiteBackend from readthedocs.notifications.constants import ERROR, INFO_NON_PERSISTENT, WARNING_NON_PERSISTENT +from readthedocs.projects.models import Project +from readthedocs.projects.notifications import ( + DeprecatedGitHubWebhookNotification, + DeprecatedBuildWebhookNotification, +) from readthedocs.builds.models import Build @@ -222,3 +230,43 @@ def test_message(self): with mock.patch('readthedocs.notifications.notification.log') as mock_log: self.assertEqual(self.n.get_message(False), '') mock_log.error.assert_called_once() + + +class DeprecatedWebhookEndpointNotificationTests(TestCase): + + def setUp(self): + PersistentMessage.objects.all().delete() + + self.user = fixture.get(User) + self.project = fixture.get(Project, users=[self.user]) + self.request = HttpRequest() + + self.notification = DeprecatedBuildWebhookNotification( + self.project, + self.request, + self.user, + ) + + @mock.patch('readthedocs.notifications.backends.send_email') + def test_dedupliation(self, send_email): + user = fixture.get(User) + project = fixture.get(Project, main_language_project=None) + project.users.add(user) + project.refresh_from_db() + self.assertEqual(project.users.count(), 1) + + self.assertEqual(PersistentMessage.objects.filter(user=user).count(), 0) + DeprecatedGitHubWebhookNotification.notify_project_users([project]) + + # Site and email notification will go out, site message doesn't have + # any reason to deduplicate yet + self.assertEqual(PersistentMessage.objects.filter(user=user).count(), 1) + self.assertTrue(send_email.called) + send_email.reset_mock() + self.assertFalse(send_email.called) + + # Expect the site message to deduplicate, the email won't + DeprecatedGitHubWebhookNotification.notify_project_users([project]) + self.assertEqual(PersistentMessage.objects.filter(user=user).count(), 1) + self.assertTrue(send_email.called) + send_email.reset_mock() diff --git a/readthedocs/templates/projects/notifications/deprecated_build_webhook_email.html b/readthedocs/templates/projects/notifications/deprecated_build_webhook_email.html new file mode 100644 index 00000000000..fb19f176532 --- /dev/null +++ b/readthedocs/templates/projects/notifications/deprecated_build_webhook_email.html @@ -0,0 +1,6 @@ +

Your project, {{ project.name }}, is currently using a legacy incoming webhook to trigger builds on Read the Docs. Effective April 1st, 2019, Read the Docs will no longer accept incoming webhooks through these endpoints.

+ +

To continue building your Read the Docs project on changes to your repository, you will need to configure a new webhook with your VCS provider. You can find more information on how to configure a new webhook in our documentation, at:

+ +{% comment %}Plain text link because of text version of email{% endcomment %} +

https://docs.readthedocs.io/en/latest/webhooks.html#webhook-deprecated-endpoints

diff --git a/readthedocs/templates/projects/notifications/deprecated_build_webhook_site.html b/readthedocs/templates/projects/notifications/deprecated_build_webhook_site.html new file mode 100644 index 00000000000..33c5e27e616 --- /dev/null +++ b/readthedocs/templates/projects/notifications/deprecated_build_webhook_site.html @@ -0,0 +1 @@ +Your project, {{ project.name }}, needs to be reconfigured in order to continue building automatically after April 1st, 2019. For more information, see our documentation on webhook integrations. diff --git a/readthedocs/templates/projects/notifications/deprecated_github_webhook_email.html b/readthedocs/templates/projects/notifications/deprecated_github_webhook_email.html new file mode 100644 index 00000000000..7d352390d42 --- /dev/null +++ b/readthedocs/templates/projects/notifications/deprecated_github_webhook_email.html @@ -0,0 +1,8 @@ +

Your project, {{ project.name }}, is currently using GitHub Services to trigger builds on Read the Docs. Effective January 31, 2019, GitHub will no longer process requests using the Services feature, and so Read the Docs will not receive notifications on updates to your repository.

+ +

To continue building your Read the Docs project on changes to your repository, you will need to add a new webhook on your GitHub repository. You can either connect your GitHub account and configure a GitHub webhook integration, or you can add a generic webhook integration.

+ +

You can find more information on our webhook intergrations in our documentation, at:

+ +{% comment %}Plain text link because of text version of email{% endcomment %} +

https://docs.readthedocs.io/en/latest/webhooks.html#webhook-github-services

diff --git a/readthedocs/templates/projects/notifications/deprecated_github_webhook_site.html b/readthedocs/templates/projects/notifications/deprecated_github_webhook_site.html new file mode 100644 index 00000000000..0832efaf793 --- /dev/null +++ b/readthedocs/templates/projects/notifications/deprecated_github_webhook_site.html @@ -0,0 +1 @@ +Your project, {{ project.name }}, needs to be reconfigured in order to continue building automatically after January 31st, 2019. For more information, see our documentation on webhook integrations.