Skip to content

Commit 72972b5

Browse files
ericholscherericholscher
authored
Merge pull request readthedocs#6124 from saadmk11/sync-bug-fix
Integration Re-sync Bug Fix
2 parents bcafa57 + b5cd50a commit 72972b5

File tree

9 files changed

+969
-61
lines changed

9 files changed

+969
-61
lines changed

‎docs/webhooks.rst

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,13 @@ After you have added the integration, you'll see a link to information about the
3636

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

39-
Use this URL when setting up a new webhook with your provider -- these steps vary depending on the provider:
39+
Use this URL when setting up a new webhook with your provider -- these steps vary depending on the provider.
40+
41+
.. note::
42+
43+
If your account is connected to the provider,
44+
we'll try to setup the webhook automatically.
45+
If something fails, you can still setup the webhook manually.
4046

4147
.. _webhook-integration-github:
4248

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

164-
When :ref:`resyncing the webhook <webhooks:Resyncing webhooks>`, the secret is changed too.
165-
166170
Troubleshooting
167171
---------------
168172

‎readthedocs/integrations/models.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -274,6 +274,10 @@ def recreate_secret(self):
274274
self.secret = get_secret()
275275
self.save(update_fields=['secret'])
276276

277+
def remove_secret(self):
278+
self.secret = None
279+
self.save(update_fields=['secret'])
280+
277281
def __str__(self):
278282
return (
279283
_('{0} for {1}')

‎readthedocs/oauth/services/base.py

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,19 @@ def get_paginated_results(self, response):
226226
"""
227227
raise NotImplementedError
228228

229+
def get_provider_data(self, project, integration):
230+
"""
231+
Gets provider data from Git Providers Webhooks API.
232+
233+
:param project: project
234+
:type project: Project
235+
:param integration: Integration for the project
236+
:type integration: Integration
237+
:returns: Dictionary containing provider data from the API or None
238+
:rtype: dict
239+
"""
240+
raise NotImplementedError
241+
229242
def setup_webhook(self, project, integration=None):
230243
"""
231244
Setup webhook for project.

‎readthedocs/oauth/services/bitbucket.py

Lines changed: 80 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -206,6 +206,71 @@ def get_webhook_data(self, project, integration):
206206
'events': ['repo:push'],
207207
})
208208

209+
def get_provider_data(self, project, integration):
210+
"""
211+
Gets provider data from BitBucket Webhooks API.
212+
213+
:param project: project
214+
:type project: Project
215+
:param integration: Integration for the project
216+
:type integration: Integration
217+
:returns: Dictionary containing provider data from the API or None
218+
:rtype: dict
219+
"""
220+
221+
if integration.provider_data:
222+
return integration.provider_data
223+
224+
session = self.get_session()
225+
owner, repo = build_utils.get_bitbucket_username_repo(url=project.repo)
226+
227+
rtd_webhook_url = 'https://{domain}{path}'.format(
228+
domain=settings.PRODUCTION_DOMAIN,
229+
path=reverse(
230+
'api_webhook',
231+
kwargs={
232+
'project_slug': project.slug,
233+
'integration_pk': integration.pk,
234+
},
235+
),
236+
)
237+
238+
try:
239+
resp = session.get(
240+
(
241+
'https://api.bitbucket.org/2.0/repositories/{owner}/{repo}/hooks'
242+
.format(owner=owner, repo=repo)
243+
),
244+
)
245+
246+
if resp.status_code == 200:
247+
recv_data = resp.json()
248+
249+
for webhook_data in recv_data["values"]:
250+
if webhook_data["url"] == rtd_webhook_url:
251+
integration.provider_data = webhook_data
252+
integration.save()
253+
254+
log.info(
255+
'Bitbucket integration updated with provider data for project: %s',
256+
project,
257+
)
258+
break
259+
else:
260+
log.info(
261+
'Bitbucket project does not exist or user does not have '
262+
'permissions: project=%s',
263+
project,
264+
)
265+
266+
except Exception:
267+
log.exception(
268+
'Bitbucket webhook Listing failed for project: %s',
269+
project,
270+
)
271+
272+
return integration.provider_data
273+
209274
def setup_webhook(self, project, integration=None):
210275
"""
211276
Set up Bitbucket project webhook for project.
@@ -219,6 +284,7 @@ def setup_webhook(self, project, integration=None):
219284
"""
220285
session = self.get_session()
221286
owner, repo = build_utils.get_bitbucket_username_repo(url=project.repo)
287+
222288
if not integration:
223289
integration, _ = Integration.objects.get_or_create(
224290
project=project,
@@ -251,7 +317,6 @@ def setup_webhook(self, project, integration=None):
251317
'permissions: project=%s',
252318
project,
253319
)
254-
return (False, resp)
255320

256321
# Catch exceptions with request or deserializing JSON
257322
except (RequestException, ValueError):
@@ -271,7 +336,8 @@ def setup_webhook(self, project, integration=None):
271336
)
272337
except ValueError:
273338
pass
274-
return (False, resp)
339+
340+
return (False, resp)
275341

276342
def update_webhook(self, project, integration):
277343
"""
@@ -284,17 +350,25 @@ def update_webhook(self, project, integration):
284350
:returns: boolean based on webhook set up success, and requests Response object
285351
:rtype: (Bool, Response)
286352
"""
353+
provider_data = self.get_provider_data(project, integration)
354+
355+
# Handle the case where we don't have a proper provider_data set
356+
# This happens with a user-managed webhook previously
357+
if not provider_data:
358+
return self.setup_webhook(project, integration)
359+
287360
session = self.get_session()
288361
data = self.get_webhook_data(project, integration)
289362
resp = None
290363
try:
291364
# Expect to throw KeyError here if provider_data is invalid
292-
url = integration.provider_data['links']['self']['href']
365+
url = provider_data['links']['self']['href']
293366
resp = session.put(
294367
url,
295368
data=data,
296369
headers={'content-type': 'application/json'},
297370
)
371+
298372
if resp.status_code == 200:
299373
recv_data = resp.json()
300374
integration.provider_data = recv_data
@@ -308,15 +382,14 @@ def update_webhook(self, project, integration):
308382
# Bitbucket returns 404 when the webhook doesn't exist. In this
309383
# case, we call ``setup_webhook`` to re-configure it from scratch
310384
if resp.status_code == 404:
311-
return self.setup_webhook(project)
385+
return self.setup_webhook(project, integration)
312386

313387
# Catch exceptions with request or deserializing JSON
314388
except (KeyError, RequestException, TypeError, ValueError):
315389
log.exception(
316390
'Bitbucket webhook update failed for project: %s',
317391
project,
318392
)
319-
return (False, resp)
320393
else:
321394
log.error(
322395
'Bitbucket webhook update failed for project: %s',
@@ -331,4 +404,5 @@ def update_webhook(self, project, integration):
331404
'Bitbucket webhook update failure response: %s',
332405
debug_data,
333406
)
334-
return (False, resp)
407+
408+
return (False, resp)

‎readthedocs/oauth/services/github.py

Lines changed: 98 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -190,6 +190,71 @@ def get_webhook_data(self, project, integration):
190190
'events': ['push', 'pull_request', 'create', 'delete'],
191191
})
192192

193+
def get_provider_data(self, project, integration):
194+
"""
195+
Gets provider data from GitHub Webhooks API.
196+
197+
:param project: project
198+
:type project: Project
199+
:param integration: Integration for the project
200+
:type integration: Integration
201+
:returns: Dictionary containing provider data from the API or None
202+
:rtype: dict
203+
"""
204+
205+
if integration.provider_data:
206+
return integration.provider_data
207+
208+
session = self.get_session()
209+
owner, repo = build_utils.get_github_username_repo(url=project.repo)
210+
211+
rtd_webhook_url = 'https://{domain}{path}'.format(
212+
domain=settings.PRODUCTION_DOMAIN,
213+
path=reverse(
214+
'api_webhook',
215+
kwargs={
216+
'project_slug': project.slug,
217+
'integration_pk': integration.pk,
218+
},
219+
)
220+
)
221+
222+
try:
223+
resp = session.get(
224+
(
225+
'https://api.github.com/repos/{owner}/{repo}/hooks'
226+
.format(owner=owner, repo=repo)
227+
),
228+
)
229+
230+
if resp.status_code == 200:
231+
recv_data = resp.json()
232+
233+
for webhook_data in recv_data:
234+
if webhook_data["config"]["url"] == rtd_webhook_url:
235+
integration.provider_data = webhook_data
236+
integration.save()
237+
238+
log.info(
239+
'GitHub integration updated with provider data for project: %s',
240+
project,
241+
)
242+
break
243+
else:
244+
log.info(
245+
'GitHub project does not exist or user does not have '
246+
'permissions: project=%s',
247+
project,
248+
)
249+
250+
except Exception:
251+
log.exception(
252+
'GitHub webhook Listing failed for project: %s',
253+
project,
254+
)
255+
256+
return integration.provider_data
257+
193258
def setup_webhook(self, project, integration=None):
194259
"""
195260
Set up GitHub project webhook for project.
@@ -203,13 +268,16 @@ def setup_webhook(self, project, integration=None):
203268
"""
204269
session = self.get_session()
205270
owner, repo = build_utils.get_github_username_repo(url=project.repo)
206-
if integration:
207-
integration.recreate_secret()
208-
else:
271+
272+
if not integration:
209273
integration, _ = Integration.objects.get_or_create(
210274
project=project,
211275
integration_type=Integration.GITHUB_WEBHOOK,
212276
)
277+
278+
if not integration.secret:
279+
integration.recreate_secret()
280+
213281
data = self.get_webhook_data(project, integration)
214282
resp = None
215283
try:
@@ -221,6 +289,7 @@ def setup_webhook(self, project, integration=None):
221289
data=data,
222290
headers={'content-type': 'application/json'},
223291
)
292+
224293
# GitHub will return 200 if already synced
225294
if resp.status_code in [200, 201]:
226295
recv_data = resp.json()
@@ -238,10 +307,9 @@ def setup_webhook(self, project, integration=None):
238307
'permissions: project=%s',
239308
project,
240309
)
241-
# Set the secret to None so that the integration can be used manually.
242-
integration.secret = None
243-
integration.save()
244-
return (False, resp)
310+
311+
# All other status codes will flow to the `else` clause below
312+
245313
# Catch exceptions with request or deserializing JSON
246314
except (RequestException, ValueError):
247315
log.exception(
@@ -263,7 +331,10 @@ def setup_webhook(self, project, integration=None):
263331
'GitHub webhook creation failure response: %s',
264332
debug_data,
265333
)
266-
return (False, resp)
334+
335+
# Always remove the secret and return False if we don't return True above
336+
integration.remove_secret()
337+
return (False, resp)
267338

268339
def update_webhook(self, project, integration):
269340
"""
@@ -277,39 +348,49 @@ def update_webhook(self, project, integration):
277348
:rtype: (Bool, Response)
278349
"""
279350
session = self.get_session()
280-
integration.recreate_secret()
351+
if not integration.secret:
352+
integration.recreate_secret()
281353
data = self.get_webhook_data(project, integration)
282354
resp = None
355+
356+
provider_data = self.get_provider_data(project, integration)
357+
358+
# Handle the case where we don't have a proper provider_data set
359+
# This happens with a user-managed webhook previously
360+
if not provider_data:
361+
return self.setup_webhook(project, integration)
362+
283363
try:
284-
url = integration.provider_data.get('url')
364+
url = provider_data.get('url')
365+
285366
resp = session.patch(
286367
url,
287368
data=data,
288369
headers={'content-type': 'application/json'},
289370
)
371+
290372
# GitHub will return 200 if already synced
291373
if resp.status_code in [200, 201]:
292374
recv_data = resp.json()
293375
integration.provider_data = recv_data
294376
integration.save()
295377
log.info(
296-
'GitHub webhook creation successful for project: %s',
378+
'GitHub webhook update successful for project: %s',
297379
project,
298380
)
299381
return (True, resp)
300382

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

306388
# Catch exceptions with request or deserializing JSON
307389
except (AttributeError, RequestException, ValueError):
308390
log.exception(
309391
'GitHub webhook update failed for project: %s',
310392
project,
311393
)
312-
return (False, resp)
313394
else:
314395
log.error(
315396
'GitHub webhook update failed for project: %s',
@@ -320,10 +401,12 @@ def update_webhook(self, project, integration):
320401
except ValueError:
321402
debug_data = resp.content
322403
log.debug(
323-
'GitHub webhook creation failure response: %s',
404+
'GitHub webhook update failure response: %s',
324405
debug_data,
325406
)
326-
return (False, resp)
407+
408+
integration.remove_secret()
409+
return (False, resp)
327410

328411
def send_build_status(self, build, commit, state):
329412
"""

0 commit comments

Comments
 (0)