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

Move from service hooks to webhooks #818

Merged
merged 12 commits into from Sep 26, 2018

Conversation

Projects
None yet
2 participants
@joecorcoran
Contributor

joecorcoran commented Sep 12, 2018

This makes the following changes.

When a user activates a repo:

  • Set old service hook to active: false if found
  • Create or update webhook with active: true

When a user deactivates a repo:

  • Set old service hook to active: false if found
  • Create or update webhook with active: false

The class Travis::API::V3::GitHub is used both for API V3 and for older versions too, to avoid the repeated logic we used to have.

@joecorcoran joecorcoran changed the title from Start moving to webhooks to Move from service hooks to webhooks Sep 13, 2018

@joecorcoran joecorcoran force-pushed the jc-hooks branch from 9846247 to 5ca2d89 Sep 14, 2018

def hook_url?(repo, type)
hooks_url = HOOKS_URL % [repo.slug]
if hook = gh[hooks_url].detect { |hook| hook['name'.freeze] == type }

This comment has been minimized.

@joecorcoran

joecorcoran Sep 15, 2018

Contributor

Extract the hooks GET request in order to cache the response.

@joecorcoran joecorcoran deployed to org-staging Sep 17, 2018 Active

@joecorcoran joecorcoran deployed to org-staging Sep 17, 2018 Active

@joecorcoran

This comment has been minimized.

Contributor

joecorcoran commented Sep 17, 2018

Currently trying to work out why this 404 is happening. I can see that the hook exists when I use my own token, so I'm assuming it's an access issue of some kind.

curl -I https://api.github.com/repos/joecorcoran/countries/hooks/8210553?access_token=redacted
HTTP/1.1 200 OK
...

It seems deleting a hook is allowed, so I'm not sure.

@joecorcoran

This comment has been minimized.

Contributor

joecorcoran commented Sep 17, 2018

Weird, so I have admin permissions on that repo, but I can't delete the old hook. Here's me playing in the staging console.

g.gh['repos/joecorcoran/countries/hooks/8210553']
=> {"type"=>"Repository",
 "id"=>8210553,
 "name"=>"travis",
 ...
g.gh.delete('repos/joecorcoran/countries/hooks/8210553')
GH::Error: GH request failed
verb:               :delete
url:                /repos/joecorcoran/countries/hooks/8210553?client_id=[removed]&client_secret=[removed]
...
@joecorcoran

This comment has been minimized.

Contributor

joecorcoran commented Sep 17, 2018

OK, found the issue. We can only write to the hook because we only ask for write:repo_hook and not admin:repo_hook. Which is not so good. I guess I have to deactivate the old hooks.

@joecorcoran joecorcoran deployed to org-staging Sep 19, 2018 Active

@joecorcoran joecorcoran deployed to org-staging Sep 19, 2018 Active

@joecorcoran

This comment has been minimized.

Contributor

joecorcoran commented Sep 19, 2018

This is live now on org staging. I've just tested it by:

  1. Deactivating this repo: https://staging.travis-ci.org/joecorcoran/test2
  2. Reactivating the same repo
  3. Observing the old and new hooks at curl https://api.github.com/repos/joecorcoran/test2/hooks?access_token=redacted
  4. Creating a new push event via the new webhook curl -I -X POST https://api.github.com/repos/joecorcoran/test2/hooks/51641091/tests?access_token=redacted (note that this URL is now tests rather than test which is incorrect in GitHub API V3 responses)
  5. Observing a new entry in "Recent Deliveries" at https://github.com/joecorcoran/test2/settings/hooks/51641091
  6. Observing that this build was created: https://staging.travis-ci.org/joecorcoran/test2/builds/727723

@joecorcoran joecorcoran deployed to org-staging Sep 20, 2018 Active

joecorcoran added some commits Sep 12, 2018

Fix load order issue
The Travis.config method is not available until after the setup method
is called, but it's needed now in the github/services classes,
which are required before setup. Ugly.
Check webhook config.url
This is to avoid attempting to patch a webhook from another provider.

Also changes the specs to reflect the actual responses from the GitHub
API (url instead of _links.self.href, which the GH client normalizes
to).

@joecorcoran joecorcoran force-pushed the jc-hooks branch from 5223fa4 to ca66ec5 Sep 20, 2018

@porras

porras approved these changes Sep 20, 2018

joecorcoran and others added some commits Sep 21, 2018

@porras porras merged commit 994e65f into master Sep 26, 2018

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@porras porras deleted the jc-hooks branch Sep 26, 2018

danishkhan pushed a commit that referenced this pull request Oct 24, 2018

Merge pull request #818 from travis-ci/jc-hooks
Move from service hooks to webhooks

danishkhan added a commit that referenced this pull request Oct 29, 2018

Merge pull request #845 from travis-ci/dk-backport-818
Merge pull request #818 from travis-ci/jc-hooks into enterprise 2.2 branch
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment