Skip to content

Add feature flip for build endpoints #3205

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 26 commits into from
Oct 31, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
59cccbf
Add feature flipping models to Projects, plus example feature flip
agjohnson Oct 5, 2017
d251bbd
Refactoring the make_api_* calls into the API models
agjohnson Oct 27, 2017
21bab41
Add migrations for proxy models
agjohnson Oct 27, 2017
954f632
Add some admin features
agjohnson Oct 27, 2017
ae8ec3d
Bump setuptools version.
bhearsum Oct 17, 2017
59f681f
Add feature flip for setuptools latest version
agjohnson Oct 27, 2017
41d0e72
Add feature flip for build endpoints
agjohnson Oct 27, 2017
e6d2bdc
Rework feature relationship, add default true value based on date
agjohnson Oct 27, 2017
adbe72a
Fix some issues, more tests
agjohnson Oct 27, 2017
2776764
Merge branch 'project-features' into project-feature-setuptools
agjohnson Oct 27, 2017
8c74d60
Merge branch 'project-features' into project-feature-build-api-endpoint
agjohnson Oct 28, 2017
1dbe354
Add data migration adding feature to database
agjohnson Oct 28, 2017
e8c3816
Fix migration
agjohnson Oct 28, 2017
ff9be05
Fix tests
agjohnson Oct 28, 2017
dbdf59f
Rename Feature.feature -> Feature.feature_id, other cleanup
agjohnson Oct 30, 2017
f48c702
Merge branch 'project-features' into project-feature-setuptools
agjohnson Oct 30, 2017
c4b0cac
Drop setuptools pinning
agjohnson Oct 30, 2017
4d92bd9
Merge branch 'project-feature-setuptools' into project-feature-build-…
agjohnson Oct 30, 2017
cee6464
Update field name
agjohnson Oct 30, 2017
ca2e0b1
Fix migration as well
agjohnson Oct 30, 2017
7ccbe97
Merge branch 'master' into project-feature-setuptools
agjohnson Oct 31, 2017
83b0a2f
Use semver for setuptools version
agjohnson Oct 31, 2017
5b185cc
Missed merge conflict
agjohnson Oct 31, 2017
bc4fdca
Merge branch 'project-feature-setuptools' into project-feature-build-…
agjohnson Oct 31, 2017
0a5527c
Rename function, also go private function
agjohnson Oct 31, 2017
2b79019
Merge branch 'master' into project-feature-build-api-endpoint
agjohnson Oct 31, 2017
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
18 changes: 17 additions & 1 deletion readthedocs/core/views/hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from readthedocs.core.utils import trigger_build
from readthedocs.builds.constants import LATEST
from readthedocs.projects import constants
from readthedocs.projects.models import Project
from readthedocs.projects.models import Project, Feature
from readthedocs.projects.tasks import update_imported_docs

import logging
Expand All @@ -23,6 +23,10 @@ class NoProjectException(Exception):
pass


def _allow_deprecated_webhook(project):
return project.has_feature(Feature.ALLOW_DEPRECATED_WEBHOOKS)


def _build_version(project, slug, already_built=()):
"""
Where we actually trigger builds for a project and slug.
Expand Down Expand Up @@ -108,6 +112,13 @@ def _build_url(url, projects, branches):
ret = ""
all_built = {}
all_not_building = {}

# This endpoint doesn't require authorization, we shouldn't allow builds to
# be triggered from this any longer. Deprecation plan is to selectively
# allow access to this endpoint for now.
if not any(_allow_deprecated_webhook(project) for project in projects):
return HttpResponse('This API endpoint is deprecated', status=403)

for project in projects:
(built, not_building) = build_branches(project, branches)
if not built:
Expand Down Expand Up @@ -314,6 +325,11 @@ def generic_build(request, project_id_or_slug=None):
project_id_or_slug)
return HttpResponseNotFound(
'Repo not found: %s' % project_id_or_slug)
# This endpoint doesn't require authorization, we shouldn't allow builds to
# be triggered from this any longer. Deprecation plan is to selectively
# allow access to this endpoint for now.
if not _allow_deprecated_webhook(project):
return HttpResponse('This API endpoint is deprecated', status=403)
if request.method == 'POST':
slug = request.POST.get('version_slug', project.default_version)
log.info(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# -*- coding: utf-8 -*-

"""Add feature for allowing access to deprecated webhook endpoints"""

from __future__ import unicode_literals

from django.db import migrations


FEATURE_ID = 'allow_deprecated_webhooks'


def forward_add_feature(apps, schema_editor):
Feature = apps.get_model('projects', 'Feature')
Feature.objects.create(
feature_id=FEATURE_ID,
default_true=True,
)


def reverse_add_feature(apps, schema_editor):
Feature = apps.get_model('projects', 'Feature')
Feature.objects.filter(feature_id=FEATURE_ID).delete()


class Migration(migrations.Migration):

dependencies = [
('projects', '0020_add-api-project-proxy'),
]

operations = [
migrations.RunPython(forward_add_feature, reverse_add_feature)
]
2 changes: 2 additions & 0 deletions readthedocs/projects/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -1003,11 +1003,13 @@ def add_features(sender, **kwargs):
# may be added by other packages
USE_SPHINX_LATEST = 'use_sphinx_latest'
USE_SETUPTOOLS_LATEST = 'use_setuptools_latest'
ALLOW_DEPRECATED_WEBHOOKS = 'allow_deprecated_webhooks'
PIP_ALWAYS_UPGRADE = 'pip_always_upgrade'

FEATURES = (
(USE_SPHINX_LATEST, _('Use latest version of Sphinx')),
(USE_SETUPTOOLS_LATEST, _('Use latest version of setuptools')),
(ALLOW_DEPRECATED_WEBHOOKS, _('Allow deprecated webhook views')),
(PIP_ALWAYS_UPGRADE, _('Always run pip install --upgrade')),
)

Expand Down
65 changes: 64 additions & 1 deletion readthedocs/rtd_tests/tests/test_post_commit_hooks.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from future.backports.urllib.parse import urlencode

from readthedocs.builds.models import Version
from readthedocs.projects.models import Project
from readthedocs.projects.models import Project, Feature


log = logging.getLogger(__name__)
Expand All @@ -32,6 +32,11 @@ def _setup(self):
self.mocks = [mock.patch('readthedocs.core.views.hooks.trigger_build')]
self.patches = [m.start() for m in self.mocks]

self.feature = Feature.objects.get(feature_id=Feature.ALLOW_DEPRECATED_WEBHOOKS)
self.feature.projects.add(self.pip)
self.feature.projects.add(self.rtfd)
self.feature.projects.add(self.sphinx)

self.client.login(username='eric', password='test')


Expand Down Expand Up @@ -141,6 +146,23 @@ def test_gitlab_post_commit_knows_default_branches(self):
rtd.default_branch = old_default
rtd.save()

def test_gitlab_webhook_is_deprecated(self):
# Project is created after feature, not included in historical allowance
url = 'https://github.com/rtfd/readthedocs-build'
payload = self.payload.copy()
payload['project']['http_url'] = url
project = get(
Project,
main_language_project=None,
repo=url,
)
r = self.client.post(
'/gitlab/',
data=json.dumps(payload),
content_type='application/json'
)
self.assertEqual(r.status_code, 403)


class GitHubPostCommitTest(BasePostCommitTest):
fixtures = ["eric"]
Expand Down Expand Up @@ -297,6 +319,23 @@ def test_github_post_commit_knows_default_branches(self):
rtd.default_branch = old_default
rtd.save()

def test_github_webhook_is_deprecated(self):
# Project is created after feature, not included in historical allowance
url = 'https://github.com/rtfd/readthedocs-build'
payload = self.payload.copy()
payload['repository']['url'] = url
project = get(
Project,
main_language_project=None,
repo=url,
)
r = self.client.post(
'/github/',
data=json.dumps(payload),
content_type='application/json'
)
self.assertEqual(r.status_code, 403)


class CorePostCommitTest(BasePostCommitTest):
fixtures = ["eric"]
Expand All @@ -320,6 +359,12 @@ def test_hook_state_tracking(self):
# Need to re-query to get updated DB entry
self.assertEqual(Project.objects.get(slug='read-the-docs').has_valid_webhook, True)

def test_generic_webhook_is_deprecated(self):
# Project is created after feature, not included in historical allowance
project = get(Project, main_language_project=None)
r = self.client.post('/build/%s' % project.pk, {'version_slug': 'master'})
self.assertEqual(r.status_code, 403)


class BitBucketHookTests(BasePostCommitTest):

Expand Down Expand Up @@ -476,6 +521,7 @@ def test_bitbucket_default_branch(self):
Project, repo='HTTPS://bitbucket.org/test/project', slug='test-project',
default_branch='integration', repo_type='git',
)
self.feature.projects.add(self.test_project)

self.git_payload['commits'] = [{
"branch": "integration",
Expand All @@ -487,3 +533,20 @@ def test_bitbucket_default_branch(self):
r = self.client.post('/bitbucket/', data=json.dumps(self.git_payload),
content_type='application/json')
self.assertContains(r, '(URL Build) Build Started: bitbucket.org/test/project [latest]')

def test_bitbucket_webhook_is_deprecated(self):
# Project is created after feature, not included in historical allowance
url = 'https://bitbucket.org/rtfd/readthedocs-build'
payload = self.git_payload.copy()
payload['repository']['absolute_url'] = '/rtfd/readthedocs-build'
project = get(
Project,
main_language_project=None,
repo=url,
)
r = self.client.post(
'/bitbucket/',
data=json.dumps(payload),
content_type='application/json'
)
self.assertEqual(r.status_code, 403)