Skip to content

Commit

Permalink
Github to GitHub
Browse files Browse the repository at this point in the history
  • Loading branch information
midnightconman committed Feb 28, 2019
1 parent 5dc9e65 commit e745c21
Show file tree
Hide file tree
Showing 94 changed files with 411 additions and 361 deletions.
13 changes: 13 additions & 0 deletions config/jobs/kubernetes/test-infra/test-infra-presubmits.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -268,3 +268,16 @@ presubmits:
- image: gcr.io/k8s-testimages/kubekins-e2e:v20190220-615bdbe2e-experimental
command:
- ./hack/verify-file-perms.sh

- name: pull-test-infra-verify-github-spelling
branches:
- master
always_run: true
decorate: true
labels:
preset-service-account: "true"
spec:
containers:
- image: gcr.io/k8s-testimages/kubekins-e2e:v20190220-615bdbe2e-experimental
command:
- ./hack/verify-github-spelling.sh
4 changes: 2 additions & 2 deletions experiment/bootstrap/paths_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ func TestPRPaths(t *testing.T) {
if err != nil {
t.Errorf("got unexpected error parsing test repos: %v", err)
}
reposGithub, err := ParseRepos([]string{"github.com/foo/bar=master:0efa7e1b,2001:8b376c6c"})
reposGitHub, err := ParseRepos([]string{"github.com/foo/bar=master:0efa7e1b,2001:8b376c6c"})
if err != nil {
t.Errorf("got unexpected error parsing test repos: %v", err)
}
Expand Down Expand Up @@ -152,7 +152,7 @@ func TestPRPaths(t *testing.T) {
Name: "normal-github",
Base: "/base",
Job: "some-job",
Repos: reposGithub,
Repos: reposGitHub,
Build: "1337",
Expected: &Paths{
Artifacts: filepath.Join("/base", "pull", "foo_bar", "2001", "some-job", "1337", "artifacts"),
Expand Down
2 changes: 1 addition & 1 deletion experiment/tracer/trace.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ func (l linesByTimestamp) String() string {
return fmt.Sprintf("[%s]", log)
}

// ic is the prefix of a URL fragment for a Github comment.
// ic is the prefix of a URL fragment for a GitHub comment.
// The URL fragment has the following format:
//
// issuecomment-#id
Expand Down
4 changes: 2 additions & 2 deletions gubernator/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ only updates after the webhook is added will be shown on the dashboard.

# Deployment

- Visit /config on the new deployment to configure Github [OAuth logins](https://github.com/settings/applications)
- Visit /config on the new deployment to configure GitHub [OAuth logins](https://github.com/settings/applications)
and webhook secrets.

# GCS Layout
Expand Down Expand Up @@ -131,5 +131,5 @@ the different types of jobs:

# Migrations

1. 2018-01-09: Github webhook and oauth secrets must be reconfigured. Visit
1. 2018-01-09: GitHub webhook and oauth secrets must be reconfigured. Visit
/config on your deployment to enter the new values.
6 changes: 3 additions & 3 deletions gubernator/github/classifier.py
Original file line number Diff line number Diff line change
Expand Up @@ -39,10 +39,10 @@ def classify_issue(repo, number):
payload: a dict, see full description for classify below.
last_event_timestamp: the timestamp of the most recent event.
"""
ancestor = models.GithubResource.make_key(repo, number)
ancestor = models.GitHubResource.make_key(repo, number)
logging.info('finding webhooks for %s %s', repo, number)
event_keys = list(models.GithubWebhookRaw.query(ancestor=ancestor)
.order(models.GithubWebhookRaw.timestamp)
event_keys = list(models.GitHubWebhookRaw.query(ancestor=ancestor)
.order(models.GitHubWebhookRaw.timestamp)
.fetch(keys_only=True))

logging.info('classifying %s %s (%d events)', repo, number, len(event_keys))
Expand Down
26 changes: 13 additions & 13 deletions gubernator/github/handlers.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def make_signature(body):
return 'sha1=' + hmac_instance.hexdigest()


class GithubHandler(webapp2.RequestHandler):
class GitHubHandler(webapp2.RequestHandler):
"""
Handle POSTs delivered using GitHub's webhook interface. Posts are
authenticated with HMAC signatures and a shared secret.
Expand Down Expand Up @@ -76,15 +76,15 @@ def post(self):

parent = None
if number:
parent = models.GithubResource.make_key(repo, number)
parent = models.GitHubResource.make_key(repo, number)

kwargs = {}
timestamp = self.request.headers.get('x-timestamp')
if timestamp is not None:
kwargs['timestamp'] = datetime.datetime.strptime(
timestamp, '%Y-%m-%d %H:%M:%S.%f')

webhook = models.GithubWebhookRaw(
webhook = models.GitHubWebhookRaw(
parent=parent,
repo=repo,
number=number,
Expand Down Expand Up @@ -136,12 +136,12 @@ def get(self):
number = int(self.request.get('number', 0)) or None
count = int(self.request.get('count', 500))
if repo is not None and number is not None:
q = models.GithubWebhookRaw.query(
models.GithubWebhookRaw.repo == repo,
models.GithubWebhookRaw.number == number)
q = models.GitHubWebhookRaw.query(
models.GitHubWebhookRaw.repo == repo,
models.GitHubWebhookRaw.number == number)
else:
q = models.GithubWebhookRaw.query()
q = q.order(models.GithubWebhookRaw.timestamp)
q = models.GitHubWebhookRaw.query()
q = q.order(models.GitHubWebhookRaw.timestamp)
events, next_cursor, more = q.fetch_page(count, start_cursor=cursor)
out = []
for event in events:
Expand Down Expand Up @@ -189,9 +189,9 @@ def emit_classified(self, repo, number):
self.response.write('<pre>%s</pre>' % traceback.format_exc())

def emit_events(self, repo, number):
ancestor = models.GithubResource.make_key(repo, number)
events = list(models.GithubWebhookRaw.query(ancestor=ancestor)
.order(models.GithubWebhookRaw.timestamp))
ancestor = models.GitHubResource.make_key(repo, number)
events = list(models.GitHubWebhookRaw.query(ancestor=ancestor)
.order(models.GitHubWebhookRaw.timestamp))

self.response.write('<h3>Distilled Events</h3>')
self.response.write('<pre>')
Expand Down Expand Up @@ -228,8 +228,8 @@ def get(self):
repo = self.request.get('repo')
number = self.request.get('number')
if self.request.get('format') == 'json':
ancestor = models.GithubResource.make_key(repo, number)
events = list(models.GithubWebhookRaw.query(ancestor=ancestor))
ancestor = models.GitHubResource.make_key(repo, number)
events = list(models.GitHubWebhookRaw.query(ancestor=ancestor))
self.response.headers['content-type'] = 'application/json'
self.response.write(json.dumps([e.body for e in events], indent=True))
return
Expand Down
4 changes: 2 additions & 2 deletions gubernator/github/index.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ indexes:

# This index allows us to iterate through events sorted by timestamp,
# without fetching all the events into memory first.
- kind: GithubWebhookRaw
- kind: GitHubWebhookRaw
ancestor: yes
properties:
- name: timestamp
Expand All @@ -17,7 +17,7 @@ indexes:
# automatically uploaded to the admin console when you next deploy
# your application using appcfg.py.

- kind: GithubWebhookRaw
- kind: GitHubWebhookRaw
properties:
- name: event
- name: timestamp
2 changes: 1 addition & 1 deletion gubernator/github/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def get(self):

app = webapp2.WSGIApplication([
('/_ah/warmup', Warmup),
(r'/webhook', handlers.GithubHandler),
(r'/webhook', handlers.GitHubHandler),
(r'/events', handlers.Events),
(r'/status', handlers.Status),
(r'/timeline', handlers.Timeline),
Expand Down
8 changes: 4 additions & 4 deletions gubernator/github/main_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,28 +60,28 @@ def get_response(self, event, body):
body = json.dumps(body)
signature = handlers.make_signature(body)
resp = app.post('/webhook', body,
{'X-Github-Event': event,
{'X-GitHub-Event': event,
'X-Hub-Signature': signature})
for task in self.taskqueue.get_filtered_tasks():
deferred.run(task.payload)
return resp

def test_webhook(self):
self.get_response('test', {'action': 'blah'})
hooks = list(models.GithubWebhookRaw.query())
hooks = list(models.GitHubWebhookRaw.query())
self.assertEqual(len(hooks), 1)
self.assertIsNotNone(hooks[0].timestamp)

def test_webhook_bad_sig(self):
body = json.dumps({'action': 'blah'})
signature = handlers.make_signature(body + 'foo')
app.post('/webhook', body,
{'X-Github-Event': 'test',
{'X-GitHub-Event': 'test',
'X-Hub-Signature': signature}, status=400)

def test_webhook_missing_sig(self):
app.post('/webhook', '{}',
{'X-Github-Event': 'test'}, status=400)
{'X-GitHub-Event': 'test'}, status=400)

def test_webhook_unicode(self):
self.get_response('test', {'action': u'blah\u03BA'})
Expand Down
8 changes: 4 additions & 4 deletions gubernator/github/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,16 @@
import google.appengine.ext.ndb as ndb


class GithubResource(ndb.Model):
class GitHubResource(ndb.Model):
# A key holder used to define an entitygroup for
# each Issue/PR, for easy ancestor queries.
@staticmethod
def make_key(repo, number):
return ndb.Key(GithubResource, '%s %s' % (repo, number))
return ndb.Key(GitHubResource, '%s %s' % (repo, number))


def shrink(body):
"""Recursively remove Github API urls from an object to make it more human-readable."""
"""Recursively remove GitHub API urls from an object to make it more human-readable."""
toremove = []
for key, value in body.iteritems():
if isinstance(value, basestring):
Expand All @@ -47,7 +47,7 @@ def shrink(body):
return body


class GithubWebhookRaw(ndb.Model):
class GitHubWebhookRaw(ndb.Model):
repo = ndb.StringProperty()
number = ndb.IntegerProperty(indexed=False)
event = ndb.StringProperty()
Expand Down
12 changes: 6 additions & 6 deletions gubernator/github/periodic_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

"""Periodically synchronize our Datastore view of PRs with Github.
"""Periodically synchronize our Datastore view of PRs with GitHub.
Various things can cause the local status of a PR to diverge from upstream:
dropped hooks from bugs in the app, upstream GitHub bugs (webhooks aren't
Expand All @@ -26,13 +26,13 @@
To handle these, on a regular schedule we perform a reconciliation step:
- for each repository that we're tracking:
- A = all open PRs from Datastore
- B = all open PRs from Github
- B = all open PRs from GitHub
- A-B is the set of improperly open PRs. For each PR, add a synthetic
webhook event to Datastore with state=closed, and reprocess.
- B-A is the set of improperly closed or missing PRs. Again, inject a
synthetic webhook with the details received from GitHub and reprocess.
This requires a Github token set like other secrets with /config in the root.
This requires a GitHub token set like other secrets with /config in the root.
Total token usage is low: number of open PRs / 100 PRs per list call.
As of 2018-01-10, 1666 open PRs in the k8s org translates into ~56 list calls.
"""
Expand Down Expand Up @@ -79,9 +79,9 @@ def get_prs_from_github(token, repo):


def inject_event_and_reclassify(repo, number, action, body):
# this follows similar code as handlers.GithubHandler
parent = models.GithubResource.make_key(repo, number)
hook = models.GithubWebhookRaw(
# this follows similar code as handlers.GitHubHandler
parent = models.GitHubResource.make_key(repo, number)
hook = models.GitHubWebhookRaw(
parent=parent, repo=repo, number=number, event='pull_request',
body=json.dumps({'action': action, 'pull_request': body}, sort_keys=True))
hook.put()
Expand Down
2 changes: 1 addition & 1 deletion gubernator/github_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ def github_client(self):
self.abort(500,
body_template=(
'An admin must <a href="/config">'
'configure Github secrets</a> for %r first.'
'configure GitHub secrets</a> for %r first.'
% self.request.host))
client = self.app.config[client_key]
return client['id'], client['secret']
Expand Down
4 changes: 2 additions & 2 deletions gubernator/github_auth_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
VEND_URL = 'https://github.com/login/oauth/access_token'
USER_URL = 'https://api.github.com/user'

class TestGithubAuth(unittest.TestCase):
class TestGitHubAuth(unittest.TestCase):
def setUp(self):
app.reset()
self.testbed.init_app_identity_stub()
Expand Down Expand Up @@ -89,7 +89,7 @@ def test_login_works(self):
'redirect_uri': ['http://localhost/github_auth/done'],
'client_id': [CLIENT_ID]})

# 2) Github redirects back
# 2) GitHub redirects back
resp = self.do_phase2(resp)
self.assertIn('Welcome, foo', resp)

Expand Down
6 changes: 3 additions & 3 deletions gubernator/templates/config.html
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ <h1>Gubernator Admin Config Page</h1>
% endblock
% block content
<form method="post">
<p>These values come from the <a href="https://github.com/settings/developers">OAuth Apps Tab</a> on Github.
<p>These values come from the <a href="https://github.com/settings/developers">OAuth Apps Tab</a> on GitHub.
<p>New apps should be registered with "Homepage URL" set to https://{{hostname}} and "Authorization callback URL" set to https://{{hostname}}/github_auth.
<p>GitHub OAuth Client Id: <input type="text" name="github_id"><br>
GitHub OAuth Client Secret: <input type="text" name="github_secret"><br>
Expand All @@ -22,9 +22,9 @@ <h1>Gubernator Admin Config Page</h1>
<h2>OAuth secrets set!</h2>
% endif
% if webhook_set
<h2>Github Webhook secret set!</h2>
<h2>GitHub Webhook secret set!</h2>
% endif
% if token_set
<h2>Github Token set!</h2>
<h2>GitHub Token set!</h2>
% endif
% endblock
31 changes: 31 additions & 0 deletions hack/verify-github-spelling.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
#!/usr/bin/env bash
# Copyright 2019 The Kubernetes Authors.
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.

set -o errexit
set -o nounset
set -o pipefail

failed="false"
for file in $( find . \( -not -path '*vendor*' -and -not -path '*.git*' -and -not -path '*hack/verify-github-spelling.sh*' \) -type f )
do
if grep -q 'Github' "${file}"; then
echo "[ERROR] ${file}: contains mis-spelling 'Github'"
failed="true"
fi
done

if [[ "${failed}" == "true" ]]; then
exit 1
fi
16 changes: 8 additions & 8 deletions pkg/ghclient/core_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,15 +30,15 @@ func setForTest(client *Client) {
client.tokenReserve = 50
}

// fakeGithub is a fake go-github client that doubles as a test instance representation. This fake
// fakeGitHub is a fake go-github client that doubles as a test instance representation. This fake
// client keeps track of the number of API calls made in order to test retry behavior and also allows
// the number of pages of results to be configured.
type fakeGithub struct {
// name is a string representation of this fakeGithub instance.
type fakeGitHub struct {
// name is a string representation of this fakeGitHub instance.
name string
// hits is a count of the number of API calls made to fakeGithub.
// hits is a count of the number of API calls made to fakeGitHub.
hits int
// hitsBeforeResponse is the number of hits that should be received before fakeGithub responds without error.
// hitsBeforeResponse is the number of hits that should be received before fakeGitHub responds without error.
hitsBeforeResponse int
// shouldSucceed indicates if the githubutil client should get a valid response.
shouldSucceed bool
Expand All @@ -49,14 +49,14 @@ type fakeGithub struct {
}

// checkHits verifies that the githubutil client made the correct number of retries before returning.
func (f *fakeGithub) checkHits() bool {
func (f *fakeGitHub) checkHits() bool {
if f.shouldSucceed {
return f.hits-f.pages+1 == f.hitsBeforeResponse
}
return f.hitsBeforeResponse > f.hits
}

func (f *fakeGithub) call() ([]interface{}, *github.Response, error) {
func (f *fakeGitHub) call() ([]interface{}, *github.Response, error) {
f.hits++
if f.hits >= f.hitsBeforeResponse {
return []interface{}{f.listOpts.Page},
Expand All @@ -67,7 +67,7 @@ func (f *fakeGithub) call() ([]interface{}, *github.Response, error) {
}

func TestRetryAndPagination(t *testing.T) {
tests := []*fakeGithub{
tests := []*fakeGitHub{
{name: "no retries", hitsBeforeResponse: 1, shouldSucceed: true, pages: 1},
{name: "max retries", hitsBeforeResponse: 6, shouldSucceed: true, pages: 1},
{name: "1 too many retries needed", hitsBeforeResponse: 7, shouldSucceed: false, pages: 1},
Expand Down
Loading

0 comments on commit e745c21

Please sign in to comment.