Skip to content

Commit

Permalink
Merge pull request #6124 from saadmk11/sync-bug-fix
Browse files Browse the repository at this point in the history 
Integration Re-sync Bug Fix
  • Loading branch information
@ericholscher
ericholscher committed Sep 10, 2019
2 parents bcafa57 + b5cd50a commit 72972b5
Show file tree
Hide file tree
Showing 9 changed files with 969 additions and 61 deletions.
10 changes: 7 additions & 3 deletions docs/webhooks.rst
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,13 @@ After you have added the integration, you'll see a link to information about the

As an example, the URL pattern looks like this: *https://readthedocs.org/api/v2/webhook/<project-name>/<id>/*.

Use this URL when setting up a new webhook with your provider -- these steps vary depending on the provider:
Use this URL when setting up a new webhook with your provider -- these steps vary depending on the provider.

.. note::

If your account is connected to the provider,
we'll try to setup the webhook automatically.
If something fails, you can still setup the webhook manually.

.. _webhook-integration-github:

Expand Down Expand Up @@ -161,8 +167,6 @@ we create a secret for every integration that offers a way to verify that a webh
Currently, `GitHub <https://developer.github.com/webhooks/securing/>`__ and `GitLab <https://docs.gitlab.com/ee/user/project/integrations/webhooks.html#secret-token>`__
offer a way to check this.

When :ref:`resyncing the webhook <webhooks:Resyncing webhooks>`, the secret is changed too.

Troubleshooting
---------------

Expand Down
4 changes: 4 additions & 0 deletions readthedocs/integrations/models.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,10 @@ def recreate_secret(self):
self.secret = get_secret()
self.save(update_fields=['secret'])

def remove_secret(self):
self.secret = None
self.save(update_fields=['secret'])

def __str__(self):
return (
_('{0} for {1}')
Expand Down
13 changes: 13 additions & 0 deletions readthedocs/oauth/services/base.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,19 @@ def get_paginated_results(self, response):
"""
raise NotImplementedError

def get_provider_data(self, project, integration):
"""
Gets provider data from Git Providers Webhooks API.
:param project: project
:type project: Project
:param integration: Integration for the project
:type integration: Integration
:returns: Dictionary containing provider data from the API or None
:rtype: dict
"""
raise NotImplementedError

def setup_webhook(self, project, integration=None):
"""
Setup webhook for project.
Expand Down
86 changes: 80 additions & 6 deletions readthedocs/oauth/services/bitbucket.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,71 @@ def get_webhook_data(self, project, integration):
'events': ['repo:push'],
})

def get_provider_data(self, project, integration):
"""
Gets provider data from BitBucket Webhooks API.
:param project: project
:type project: Project
:param integration: Integration for the project
:type integration: Integration
:returns: Dictionary containing provider data from the API or None
:rtype: dict
"""

if integration.provider_data:
return integration.provider_data

session = self.get_session()
owner, repo = build_utils.get_bitbucket_username_repo(url=project.repo)

rtd_webhook_url = 'https://{domain}{path}'.format(
domain=settings.PRODUCTION_DOMAIN,
path=reverse(
'api_webhook',
kwargs={
'project_slug': project.slug,
'integration_pk': integration.pk,
},
),
)

try:
resp = session.get(
(
'https://api.bitbucket.org/2.0/repositories/{owner}/{repo}/hooks'
.format(owner=owner, repo=repo)
),
)

if resp.status_code == 200:
recv_data = resp.json()

for webhook_data in recv_data["values"]:
if webhook_data["url"] == rtd_webhook_url:
integration.provider_data = webhook_data
integration.save()

log.info(
'Bitbucket integration updated with provider data for project: %s',
project,
)
break
else:
log.info(
'Bitbucket project does not exist or user does not have '
'permissions: project=%s',
project,
)

except Exception:
log.exception(
'Bitbucket webhook Listing failed for project: %s',
project,
)

return integration.provider_data

def setup_webhook(self, project, integration=None):
"""
Set up Bitbucket project webhook for project.
Expand All @@ -219,6 +284,7 @@ def setup_webhook(self, project, integration=None):
"""
session = self.get_session()
owner, repo = build_utils.get_bitbucket_username_repo(url=project.repo)

if not integration:
integration, _ = Integration.objects.get_or_create(
project=project,
Expand Down Expand Up @@ -251,7 +317,6 @@ def setup_webhook(self, project, integration=None):
'permissions: project=%s',
project,
)
return (False, resp)

# Catch exceptions with request or deserializing JSON
except (RequestException, ValueError):
Expand All @@ -271,7 +336,8 @@ def setup_webhook(self, project, integration=None):
)
except ValueError:
pass
return (False, resp)

return (False, resp)

def update_webhook(self, project, integration):
"""
Expand All @@ -284,17 +350,25 @@ def update_webhook(self, project, integration):
:returns: boolean based on webhook set up success, and requests Response object
:rtype: (Bool, Response)
"""
provider_data = self.get_provider_data(project, integration)

# Handle the case where we don't have a proper provider_data set
# This happens with a user-managed webhook previously
if not provider_data:
return self.setup_webhook(project, integration)

session = self.get_session()
data = self.get_webhook_data(project, integration)
resp = None
try:
# Expect to throw KeyError here if provider_data is invalid
url = integration.provider_data['links']['self']['href']
url = provider_data['links']['self']['href']
resp = session.put(
url,
data=data,
headers={'content-type': 'application/json'},
)

if resp.status_code == 200:
recv_data = resp.json()
integration.provider_data = recv_data
Expand All @@ -308,15 +382,14 @@ def update_webhook(self, project, integration):
# Bitbucket returns 404 when the webhook doesn't exist. In this
# case, we call ``setup_webhook`` to re-configure it from scratch
if resp.status_code == 404:
return self.setup_webhook(project)
return self.setup_webhook(project, integration)

# Catch exceptions with request or deserializing JSON
except (KeyError, RequestException, TypeError, ValueError):
log.exception(
'Bitbucket webhook update failed for project: %s',
project,
)
return (False, resp)
else:
log.error(
'Bitbucket webhook update failed for project: %s',
Expand All @@ -331,4 +404,5 @@ def update_webhook(self, project, integration):
'Bitbucket webhook update failure response: %s',
debug_data,
)
return (False, resp)

return (False, resp)
113 changes: 98 additions & 15 deletions readthedocs/oauth/services/github.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,71 @@ def get_webhook_data(self, project, integration):
'events': ['push', 'pull_request', 'create', 'delete'],
})

def get_provider_data(self, project, integration):
"""
Gets provider data from GitHub Webhooks API.
:param project: project
:type project: Project
:param integration: Integration for the project
:type integration: Integration
:returns: Dictionary containing provider data from the API or None
:rtype: dict
"""

if integration.provider_data:
return integration.provider_data

session = self.get_session()
owner, repo = build_utils.get_github_username_repo(url=project.repo)

rtd_webhook_url = 'https://{domain}{path}'.format(
domain=settings.PRODUCTION_DOMAIN,
path=reverse(
'api_webhook',
kwargs={
'project_slug': project.slug,
'integration_pk': integration.pk,
},
)
)

try:
resp = session.get(
(
'https://api.github.com/repos/{owner}/{repo}/hooks'
.format(owner=owner, repo=repo)
),
)

if resp.status_code == 200:
recv_data = resp.json()

for webhook_data in recv_data:
if webhook_data["config"]["url"] == rtd_webhook_url:
integration.provider_data = webhook_data
integration.save()

log.info(
'GitHub integration updated with provider data for project: %s',
project,
)
break
else:
log.info(
'GitHub project does not exist or user does not have '
'permissions: project=%s',
project,
)

except Exception:
log.exception(
'GitHub webhook Listing failed for project: %s',
project,
)

return integration.provider_data

def setup_webhook(self, project, integration=None):
"""
Set up GitHub project webhook for project.
Expand All @@ -203,13 +268,16 @@ def setup_webhook(self, project, integration=None):
"""
session = self.get_session()
owner, repo = build_utils.get_github_username_repo(url=project.repo)
if integration:
integration.recreate_secret()
else:

if not integration:
integration, _ = Integration.objects.get_or_create(
project=project,
integration_type=Integration.GITHUB_WEBHOOK,
)

if not integration.secret:
integration.recreate_secret()

data = self.get_webhook_data(project, integration)
resp = None
try:
Expand All @@ -221,6 +289,7 @@ def setup_webhook(self, project, integration=None):
data=data,
headers={'content-type': 'application/json'},
)

# GitHub will return 200 if already synced
if resp.status_code in [200, 201]:
recv_data = resp.json()
Expand All @@ -238,10 +307,9 @@ def setup_webhook(self, project, integration=None):
'permissions: project=%s',
project,
)
# Set the secret to None so that the integration can be used manually.
integration.secret = None
integration.save()
return (False, resp)

# All other status codes will flow to the `else` clause below

# Catch exceptions with request or deserializing JSON
except (RequestException, ValueError):
log.exception(
Expand All @@ -263,7 +331,10 @@ def setup_webhook(self, project, integration=None):
'GitHub webhook creation failure response: %s',
debug_data,
)
return (False, resp)

# Always remove the secret and return False if we don't return True above
integration.remove_secret()
return (False, resp)

def update_webhook(self, project, integration):
"""
Expand All @@ -277,39 +348,49 @@ def update_webhook(self, project, integration):
:rtype: (Bool, Response)
"""
session = self.get_session()
integration.recreate_secret()
if not integration.secret:
integration.recreate_secret()
data = self.get_webhook_data(project, integration)
resp = None

provider_data = self.get_provider_data(project, integration)

# Handle the case where we don't have a proper provider_data set
# This happens with a user-managed webhook previously
if not provider_data:
return self.setup_webhook(project, integration)

try:
url = integration.provider_data.get('url')
url = provider_data.get('url')

resp = session.patch(
url,
data=data,
headers={'content-type': 'application/json'},
)

# GitHub will return 200 if already synced
if resp.status_code in [200, 201]:
recv_data = resp.json()
integration.provider_data = recv_data
integration.save()
log.info(
'GitHub webhook creation successful for project: %s',
'GitHub webhook update successful for project: %s',
project,
)
return (True, resp)

# GitHub returns 404 when the webhook doesn't exist. In this case,
# we call ``setup_webhook`` to re-configure it from scratch
if resp.status_code == 404:
return self.setup_webhook(project)
return self.setup_webhook(project, integration)

# Catch exceptions with request or deserializing JSON
except (AttributeError, RequestException, ValueError):
log.exception(
'GitHub webhook update failed for project: %s',
project,
)
return (False, resp)
else:
log.error(
'GitHub webhook update failed for project: %s',
Expand All @@ -320,10 +401,12 @@ def update_webhook(self, project, integration):
except ValueError:
debug_data = resp.content
log.debug(
'GitHub webhook creation failure response: %s',
'GitHub webhook update failure response: %s',
debug_data,
)
return (False, resp)

integration.remove_secret()
return (False, resp)

def send_build_status(self, build, commit, state):
"""
Expand Down
Loading

0 comments on commit 72972b5

Please sign in to comment.