Merge "Use previously stored repo state on executor" into feature/zuulv3
diff --git a/doc/source/triggers.rst b/doc/source/triggers.rst
index f73ad2f..41a56a0 100644
--- a/doc/source/triggers.rst
+++ b/doc/source/triggers.rst
@@ -133,6 +133,8 @@
*push* - head reference updated (pushed to branch)
+ *status* - status set on commit
+
A ``pull_request_review`` event will
have associated action(s) to trigger from. The supported actions are:
@@ -165,6 +167,12 @@
strings each of which is matched to the review state, which can be one of
``approved``, ``comment``, or ``request_changes``.
+ **status**
+ This is only used for ``status`` actions. It accepts a list of strings each of
+ which matches the user setting the status, the status context, and the status
+ itself in the format of ``user:context:status``. For example,
+ ``zuul_github_ci_bot:check_pipeline:success``.
+
**ref**
This is only used for ``push`` events. This field is treated as a regular
expression and multiple refs may be listed. Github always sends full ref
diff --git a/etc/status/public_html/jquery.zuul.js b/etc/status/public_html/jquery.zuul.js
index c7e23b2..aec7a46 100644
--- a/etc/status/public_html/jquery.zuul.js
+++ b/etc/status/public_html/jquery.zuul.js
@@ -279,7 +279,16 @@
var $change_link = $('<small />');
if (change.url !== null) {
- if (/^[0-9a-f]{40}$/.test(change.id)) {
+ var github_id = change.id.match(/^([0-9]+),([0-9a-f]{40})$/);
+ if (github_id) {
+ $change_link.append(
+ $('<a />').attr('href', change.url).append(
+ $('<abbr />')
+ .attr('title', change.id)
+ .text('#' + github_id[1])
+ )
+ );
+ } else if (/^[0-9a-f]{40}$/.test(change.id)) {
var change_id_short = change.id.slice(0, 7);
$change_link.append(
$('<a />').attr('href', change.url).append(
diff --git a/requirements.txt b/requirements.txt
index 9f20458..746bbcb 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,6 +1,8 @@
pbr>=1.1.0
-Github3.py==1.0.0a2
+# pull from master until https://github.com/sigmavirus24/github3.py/pull/671
+# is in a release
+-e git+https://github.com/sigmavirus24/github3.py.git@develop#egg=Github3.py
PyYAML>=3.1.0
Paste
WebOb>=1.2.3
@@ -21,3 +23,6 @@
sqlalchemy
alembic
cryptography>=1.6
+cachecontrol
+pyjwt
+iso8601
diff --git a/tests/base.py b/tests/base.py
index a9bcee1..5bbf065 100755
--- a/tests/base.py
+++ b/tests/base.py
@@ -16,6 +16,7 @@
# under the License.
from six.moves import configparser as ConfigParser
+import datetime
import gc
import hashlib
import json
@@ -542,7 +543,8 @@
class FakeGithubPullRequest(object):
def __init__(self, github, number, project, branch,
- subject, upstream_root, files=[], number_of_commits=1):
+ subject, upstream_root, files=[], number_of_commits=1,
+ writers=[]):
"""Creates a new PR with several commits.
Sends an event about opened PR."""
self.github = github
@@ -557,10 +559,13 @@
self.comments = []
self.labels = []
self.statuses = {}
+ self.reviews = []
+ self.writers = []
self.updated_at = None
self.head_sha = None
self.is_merged = False
self.merge_message = None
+ self.state = 'open'
self._createPRRef()
self._addCommitToRepo(files=files)
self._updateTimeStamp()
@@ -569,13 +574,11 @@
"""Adds a commit on top of the actual PR head."""
self._addCommitToRepo(files=files)
self._updateTimeStamp()
- self._clearStatuses()
def forcePush(self, files=[]):
"""Clears actual commits and add a commit on top of the base."""
self._addCommitToRepo(files=files, reset=True)
self._updateTimeStamp()
- self._clearStatuses()
def getPullRequestOpenedEvent(self):
return self._getPullRequestEvent('opened')
@@ -695,7 +698,10 @@
}
},
'head': {
- 'sha': self.head_sha
+ 'sha': self.head_sha,
+ 'repo': {
+ 'full_name': self.project
+ }
}
},
'label': {
@@ -742,6 +748,9 @@
repo.index.add([fn])
self.head_sha = repo.index.commit(msg).hexsha
+ # Create an empty set of statuses for the given sha,
+ # each sha on a PR may have a status set on it
+ self.statuses[self.head_sha] = []
repo.head.reference = 'master'
zuul.merger.merger.reset_repo_to_head(repo)
repo.git.clean('-x', '-f', '-d')
@@ -754,15 +763,54 @@
repo = self._getRepo()
return repo.references[self._getPRReference()].commit.hexsha
- def setStatus(self, state, url, description, context):
- self.statuses[context] = {
+ def setStatus(self, sha, state, url, description, context, user='zuul'):
+ # Since we're bypassing github API, which would require a user, we
+ # hard set the user as 'zuul' here.
+ # insert the status at the top of the list, to simulate that it
+ # is the most recent set status
+ self.statuses[sha].insert(0, ({
'state': state,
'url': url,
- 'description': description
- }
+ 'description': description,
+ 'context': context,
+ 'creator': {
+ 'login': user
+ }
+ }))
- def _clearStatuses(self):
- self.statuses = {}
+ def addReview(self, user, state, granted_on=None):
+ gh_time_format = '%Y-%m-%dT%H:%M:%SZ'
+ # convert the timestamp to a str format that would be returned
+ # from github as 'submitted_at' in the API response
+
+ if granted_on:
+ granted_on = datetime.datetime.utcfromtimestamp(granted_on)
+ submitted_at = time.strftime(
+ gh_time_format, granted_on.timetuple())
+ else:
+ # github timestamps only down to the second, so we need to make
+ # sure reviews that tests add appear to be added over a period of
+ # time in the past and not all at once.
+ if not self.reviews:
+ # the first review happens 10 mins ago
+ offset = 600
+ else:
+ # subsequent reviews happen 1 minute closer to now
+ offset = 600 - (len(self.reviews) * 60)
+
+ granted_on = datetime.datetime.utcfromtimestamp(
+ time.time() - offset)
+ submitted_at = time.strftime(
+ gh_time_format, granted_on.timetuple())
+
+ self.reviews.append({
+ 'state': state,
+ 'user': {
+ 'login': user,
+ 'email': user + "@derp.com",
+ },
+ 'submitted_at': submitted_at,
+ })
def _getPRReference(self):
return '%s/head' % self.number
@@ -783,7 +831,10 @@
}
},
'head': {
- 'sha': self.head_sha
+ 'sha': self.head_sha,
+ 'repo': {
+ 'full_name': self.project
+ }
}
},
'sender': {
@@ -792,6 +843,21 @@
}
return (name, data)
+ def getCommitStatusEvent(self, context, state='success', user='zuul'):
+ name = 'status'
+ data = {
+ 'state': state,
+ 'sha': self.head_sha,
+ 'description': 'Test results for %s: %s' % (self.head_sha, state),
+ 'target_url': 'http://zuul/%s' % self.head_sha,
+ 'branches': [],
+ 'context': context,
+ 'sender': {
+ 'login': user
+ }
+ }
+ return (name, data)
+
class FakeGithubConnection(githubconnection.GithubConnection):
log = logging.getLogger("zuul.test.FakeGithubConnection")
@@ -856,16 +922,31 @@
'ref': pr.branch,
},
'mergeable': True,
+ 'state': pr.state,
'head': {
- 'sha': pr.head_sha
+ 'sha': pr.head_sha,
+ 'repo': {
+ 'full_name': pr.project
+ }
}
}
return data
+ def getPullBySha(self, sha):
+ prs = list(set([p for p in self.pull_requests if sha == p.head_sha]))
+ if len(prs) > 1:
+ raise Exception('Multiple pulls found with head sha: %s' % sha)
+ pr = prs[0]
+ return self.getPull(pr.project, pr.number)
+
def getPullFileNames(self, project, number):
pr = self.pull_requests[number - 1]
return pr.files
+ def _getPullReviews(self, owner, project, number):
+ pr = self.pull_requests[number - 1]
+ return pr.reviews
+
def getUser(self, login):
data = {
'username': login,
@@ -874,6 +955,16 @@
}
return data
+ def getRepoPermission(self, project, login):
+ owner, proj = project.split('/')
+ for pr in self.pull_requests:
+ pr_owner, pr_project = pr.project.split('/')
+ if (pr_owner == owner and proj == pr_project):
+ if login in pr.writers:
+ return 'write'
+ else:
+ return 'read'
+
def getGitUrl(self, project):
return os.path.join(self.upstream_root, str(project))
@@ -901,6 +992,18 @@
pull_request.is_merged = True
pull_request.merge_message = commit_message
+ def getCommitStatuses(self, project, sha):
+ owner, proj = project.split('/')
+ for pr in self.pull_requests:
+ pr_owner, pr_project = pr.project.split('/')
+ # This is somewhat risky, if the same commit exists in multiple
+ # PRs, we might grab the wrong one that doesn't have a status
+ # that is expected to be there. Maybe re-work this so that there
+ # is a global registry of commit statuses like with github.
+ if (pr_owner == owner and pr_project == proj and
+ sha in pr.statuses):
+ return pr.statuses[sha]
+
def setCommitStatus(self, project, sha, state,
url='', description='', context=''):
owner, proj = project.split('/')
@@ -908,7 +1011,7 @@
pr_owner, pr_project = pr.project.split('/')
if (pr_owner == owner and pr_project == proj and
pr.head_sha == sha):
- pr.setStatus(state, url, description, context)
+ pr.setStatus(sha, state, url, description, context)
def labelPull(self, project, pr_number, label):
pull_request = self.pull_requests[pr_number - 1]
@@ -1602,6 +1705,18 @@
else:
self._log_stream = sys.stdout
+ # NOTE(jeblair): this is temporary extra debugging to try to
+ # track down a possible leak.
+ orig_git_repo_init = git.Repo.__init__
+
+ def git_repo_init(myself, *args, **kw):
+ orig_git_repo_init(myself, *args, **kw)
+ self.log.debug("Created git repo 0x%x %s" %
+ (id(myself), repr(myself)))
+
+ self.useFixture(fixtures.MonkeyPatch('git.Repo.__init__',
+ git_repo_init))
+
handler = logging.StreamHandler(self._log_stream)
formatter = logging.Formatter('%(asctime)s %(name)-32s '
'%(levelname)-8s %(message)s')
@@ -1820,7 +1935,7 @@
self.sched.reconfigure(self.config)
self.sched.resume()
- def configure_connections(self):
+ def configure_connections(self, source_only=False):
# Set up gerrit related fakes
# Set a changes database so multiple FakeGerrit's can report back to
# a virtual canonical database given by the configured hostname
@@ -1863,7 +1978,7 @@
# Register connections from the config using fakes
self.connections = zuul.lib.connections.ConnectionRegistry()
- self.connections.configure(self.config)
+ self.connections.configure(self.config, source_only=source_only)
def setup_config(self):
# This creates the per-test configuration object. It can be
@@ -2028,12 +2143,19 @@
self.assertEqual({}, self.executor_server.job_workers)
# Make sure that git.Repo objects have been garbage collected.
repos = []
+ gc.disable()
gc.collect()
for obj in gc.get_objects():
if isinstance(obj, git.Repo):
- self.log.debug("Leaked git repo object: %s" % repr(obj))
+ self.log.debug("Leaked git repo object: 0x%x %s" %
+ (id(obj), repr(obj)))
+ for ref in gc.get_referrers(obj):
+ self.log.debug(" Referrer %s" % (repr(ref)))
repos.append(obj)
- self.assertEqual(len(repos), 0)
+ if repos:
+ for obj in gc.garbage:
+ self.log.debug(" Garbage %s" % (repr(obj)))
+ gc.enable()
self.assertEmptyQueues()
self.assertNodepoolState()
self.assertNoGeneratedKeys()
diff --git a/tests/encrypt_secret.py b/tests/encrypt_secret.py
index b8524a0..0b0cf19 100644
--- a/tests/encrypt_secret.py
+++ b/tests/encrypt_secret.py
@@ -30,5 +30,6 @@
ciphertext = encryption.encrypt_pkcs1_oaep(sys.argv[1], public_key)
print(ciphertext.encode('base64'))
+
if __name__ == '__main__':
main()
diff --git a/tests/fixtures/layouts/merging-github.yaml b/tests/fixtures/layouts/merging-github.yaml
index 4e13063..9f43f75 100644
--- a/tests/fixtures/layouts/merging-github.yaml
+++ b/tests/fixtures/layouts/merging-github.yaml
@@ -2,6 +2,7 @@
name: merge
description: Pipeline for merging the pull request
manager: independent
+ merge-failure-message: 'Merge failed'
trigger:
github:
- event: pull_request
diff --git a/tests/fixtures/layouts/reporting-github.yaml b/tests/fixtures/layouts/reporting-github.yaml
index bcbac1b..c939f39 100644
--- a/tests/fixtures/layouts/reporting-github.yaml
+++ b/tests/fixtures/layouts/reporting-github.yaml
@@ -28,6 +28,7 @@
success:
github:
comment: false
+ status: 'success'
failure:
github:
comment: false
diff --git a/tests/fixtures/layouts/requirements-github.yaml b/tests/fixtures/layouts/requirements-github.yaml
new file mode 100644
index 0000000..9933f27
--- /dev/null
+++ b/tests/fixtures/layouts/requirements-github.yaml
@@ -0,0 +1,245 @@
+- pipeline:
+ name: pipeline
+ manager: independent
+ require:
+ github:
+ status: "zuul:check:success"
+ trigger:
+ github:
+ - event: pull_request
+ action: comment
+ comment: 'test me'
+ success:
+ github:
+ comment: true
+
+- pipeline:
+ name: trigger_status
+ manager: independent
+ trigger:
+ github:
+ - event: pull_request
+ action: comment
+ comment: 'trigger me'
+ require-status: "zuul:check:success"
+ success:
+ github:
+ comment: true
+
+- pipeline:
+ name: trigger
+ manager: independent
+ trigger:
+ github:
+ - event: pull_request
+ action: status
+ status: 'zuul:check:success'
+ success:
+ github:
+ status: 'success'
+ failure:
+ github:
+ status: 'failure'
+
+- pipeline:
+ name: reviewusername
+ manager: independent
+ require:
+ github:
+ review:
+ - username: '^(herp|derp)$'
+ type: approved
+ trigger:
+ github:
+ - event: pull_request
+ action: comment
+ comment: 'test me'
+ success:
+ github:
+ comment: true
+
+- pipeline:
+ name: reviewreq
+ manager: independent
+ require:
+ github:
+ review:
+ - type: approved
+ permission: write
+ reject:
+ github:
+ review:
+ - type: changes_requested
+ permission: write
+ trigger:
+ github:
+ - event: pull_request
+ action: comment
+ comment: 'test me'
+ success:
+ github:
+ comment: true
+
+- pipeline:
+ name: reviewuserstate
+ manager: independent
+ require:
+ github:
+ review:
+ - username: 'derp'
+ type: approved
+ permission: write
+ reject:
+ github:
+ review:
+ - type: changes_requested
+ permission: write
+ trigger:
+ github:
+ - event: pull_request
+ action: comment
+ comment: 'test me'
+ success:
+ github:
+ comment: true
+
+- pipeline:
+ name: newer_than
+ manager: independent
+ require:
+ github:
+ review:
+ - type: approved
+ permission: write
+ newer-than: 1d
+ trigger:
+ github:
+ - event: pull_request
+ action: comment
+ comment: 'test me'
+ success:
+ github:
+ comment: true
+
+- pipeline:
+ name: older_than
+ manager: independent
+ require:
+ github:
+ review:
+ - type: approved
+ permission: write
+ older-than: 1d
+ trigger:
+ github:
+ - event: pull_request
+ action: comment
+ comment: 'test me'
+ success:
+ github:
+ comment: true
+
+- pipeline:
+ name: require_open
+ manager: independent
+ require:
+ github:
+ open: true
+ trigger:
+ github:
+ - event: pull_request
+ action: comment
+ comment: 'test me'
+ success:
+ github:
+ comment: true
+
+- pipeline:
+ name: require_current
+ manager: independent
+ require:
+ github:
+ current-patchset: true
+ trigger:
+ github:
+ - event: pull_request
+ action: changed
+ success:
+ github:
+ comment: true
+
+- job:
+ name: project1-pipeline
+- job:
+ name: project2-trigger
+- job:
+ name: project3-reviewusername
+- job:
+ name: project4-reviewreq
+- job:
+ name: project5-reviewuserstate
+- job:
+ name: project6-newerthan
+- job:
+ name: project7-olderthan
+- job:
+ name: project8-requireopen
+- job:
+ name: project9-requirecurrent
+
+- project:
+ name: org/project1
+ pipeline:
+ jobs:
+ - project1-pipeline
+ trigger_status:
+ jobs:
+ - project1-pipeline
+
+- project:
+ name: org/project2
+ trigger:
+ jobs:
+ - project2-trigger
+
+- project:
+ name: org/project3
+ reviewusername:
+ jobs:
+ - project3-reviewusername
+
+- project:
+ name: org/project4
+ reviewreq:
+ jobs:
+ - project4-reviewreq
+
+- project:
+ name: org/project5
+ reviewuserstate:
+ jobs:
+ - project5-reviewuserstate
+
+- project:
+ name: org/project6
+ newer_than:
+ jobs:
+ - project6-newerthan
+
+- project:
+ name: org/project7
+ older_than:
+ jobs:
+ - project7-olderthan
+
+- project:
+ name: org/project8
+ require_open:
+ jobs:
+ - project8-requireopen
+
+- project:
+ name: org/project9
+ require_current:
+ jobs:
+ - project9-requirecurrent
diff --git a/tests/fixtures/zuul-connections-merger.conf b/tests/fixtures/zuul-connections-merger.conf
new file mode 100644
index 0000000..7a1bc42
--- /dev/null
+++ b/tests/fixtures/zuul-connections-merger.conf
@@ -0,0 +1,35 @@
+[gearman]
+server=127.0.0.1
+
+[zuul]
+job_name_in_report=true
+status_url=http://zuul.example.com/status
+
+[merger]
+git_dir=/tmp/zuul-test/git
+git_user_email=zuul@example.com
+git_user_name=zuul
+zuul_url=http://zuul.example.com/p
+
+[executor]
+git_dir=/tmp/zuul-test/executor-git
+
+[connection github]
+driver=github
+
+[connection gerrit]
+driver=gerrit
+server=review.example.com
+user=jenkins
+sshkey=fake_id_rsa1
+
+[connection resultsdb]
+driver=sql
+dburi=$MYSQL_FIXTURE_DBURI$
+
+[connection smtp]
+driver=smtp
+server=localhost
+port=25
+default_from=zuul@example.com
+default_to=you@example.com
diff --git a/tests/unit/test_connection.py b/tests/unit/test_connection.py
index db32938..92270b7 100644
--- a/tests/unit/test_connection.py
+++ b/tests/unit/test_connection.py
@@ -265,3 +265,21 @@
self.executor_server.hold_jobs_in_build = False
self.executor_server.release()
self.waitUntilSettled()
+
+
+class TestConnectionsMerger(ZuulTestCase):
+ config_file = 'zuul-connections-merger.conf'
+ tenant_config_file = 'config/single-tenant/main.yaml'
+
+ def configure_connections(self):
+ super(TestConnectionsMerger, self).configure_connections(True)
+
+ def test_connections_merger(self):
+ "Test merger only configures source connections"
+
+ self.assertIn("gerrit", self.connections.connections)
+ self.assertIn("github", self.connections.connections)
+ self.assertNotIn("smtp", self.connections.connections)
+ self.assertNotIn("sql", self.connections.connections)
+ self.assertNotIn("timer", self.connections.connections)
+ self.assertNotIn("zuul", self.connections.connections)
diff --git a/tests/unit/test_github_driver.py b/tests/unit/test_github_driver.py
index f918218..227d659 100644
--- a/tests/unit/test_github_driver.py
+++ b/tests/unit/test_github_driver.py
@@ -253,10 +253,14 @@
A = self.fake_github.openFakePullRequest('org/project', 'master', 'A')
self.fake_github.emitEvent(A.getPullRequestOpenedEvent())
self.waitUntilSettled()
- self.assertIn('check', A.statuses)
- check_status = A.statuses['check']
+ # We should have a status container for the head sha
+ self.assertIn(A.head_sha, A.statuses.keys())
+ # We should only have one status for the head sha
+ self.assertEqual(1, len(A.statuses[A.head_sha]))
+ check_status = A.statuses[A.head_sha][0]
check_url = ('http://zuul.example.com/status/#%s,%s' %
(A.number, A.head_sha))
+ self.assertEqual('tenant-one/check', check_status['context'])
self.assertEqual('Standard check', check_status['description'])
self.assertEqual('pending', check_status['state'])
self.assertEqual(check_url, check_status['url'])
@@ -265,8 +269,12 @@
self.executor_server.hold_jobs_in_build = False
self.executor_server.release()
self.waitUntilSettled()
- check_status = A.statuses['check']
- self.assertEqual('Standard check', check_status['description'])
+ # We should only have two statuses for the head sha
+ self.assertEqual(2, len(A.statuses[A.head_sha]))
+ check_status = A.statuses[A.head_sha][0]
+ check_url = ('http://zuul.example.com/status/#%s,%s' %
+ (A.number, A.head_sha))
+ self.assertEqual('tenant-one/check', check_status['context'])
self.assertEqual('success', check_status['state'])
self.assertEqual(check_url, check_status['url'])
self.assertEqual(1, len(A.comments))
@@ -278,7 +286,7 @@
self.fake_github.emitEvent(
A.getCommentAddedEvent('reporting check'))
self.waitUntilSettled()
- self.assertNotIn('reporting', A.statuses)
+ self.assertEqual(2, len(A.statuses[A.head_sha]))
# comments increased by one for the start message
self.assertEqual(2, len(A.comments))
self.assertThat(A.comments[1],
@@ -286,7 +294,11 @@
self.executor_server.hold_jobs_in_build = False
self.executor_server.release()
self.waitUntilSettled()
- self.assertNotIn('reporting', A.statuses)
+ # pipeline reports success status
+ self.assertEqual(3, len(A.statuses[A.head_sha]))
+ report_status = A.statuses[A.head_sha][0]
+ self.assertEqual('tenant-one/reporting', report_status['context'])
+ self.assertEqual('success', report_status['state'])
self.assertEqual(2, len(A.comments))
@simple_layout('layouts/merging-github.yaml', driver='github')
@@ -323,6 +335,8 @@
self.fake_github.emitEvent(D.getCommentAddedEvent('merge me'))
self.waitUntilSettled()
self.assertFalse(D.is_merged)
+ self.assertEqual(len(D.comments), 1)
+ self.assertEqual(D.comments[0], 'Merge failed')
@simple_layout('layouts/dependent-github.yaml', driver='github')
def test_parallel_changes(self):
diff --git a/tests/unit/test_github_requirements.py b/tests/unit/test_github_requirements.py
new file mode 100644
index 0000000..5dd6e80
--- /dev/null
+++ b/tests/unit/test_github_requirements.py
@@ -0,0 +1,328 @@
+#!/usr/bin/env python
+# Copyright (c) 2017 IBM Corp.
+#
+# 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.
+
+import time
+
+from tests.base import ZuulTestCase, simple_layout
+
+
+class TestGithubRequirements(ZuulTestCase):
+ """Test pipeline and trigger requirements"""
+ config_file = 'zuul-github-driver.conf'
+
+ @simple_layout('layouts/requirements-github.yaml', driver='github')
+ def test_pipeline_require_status(self):
+ "Test pipeline requirement: status"
+ A = self.fake_github.openFakePullRequest('org/project1', 'master', 'A')
+ # A comment event that we will keep submitting to trigger
+ comment = A.getCommentAddedEvent('test me')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ # No status from zuul so should not be enqueued
+ self.assertEqual(len(self.history), 0)
+
+ # An error status should not cause it to be enqueued
+ A.setStatus(A.head_sha, 'error', 'null', 'null', 'check')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # A success status goes in
+ A.setStatus(A.head_sha, 'success', 'null', 'null', 'check')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+ self.assertEqual(self.history[0].name, 'project1-pipeline')
+
+ @simple_layout('layouts/requirements-github.yaml', driver='github')
+ def test_trigger_require_status(self):
+ "Test trigger requirement: status"
+ A = self.fake_github.openFakePullRequest('org/project1', 'master', 'A')
+ # A comment event that we will keep submitting to trigger
+ comment = A.getCommentAddedEvent('trigger me')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ # No status from zuul so should not be enqueued
+ self.assertEqual(len(self.history), 0)
+
+ # An error status should not cause it to be enqueued
+ A.setStatus(A.head_sha, 'error', 'null', 'null', 'check')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # A success status goes in
+ A.setStatus(A.head_sha, 'success', 'null', 'null', 'check')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+ self.assertEqual(self.history[0].name, 'project1-pipeline')
+
+ @simple_layout('layouts/requirements-github.yaml', driver='github')
+ def test_trigger_on_status(self):
+ "Test trigger on: status"
+ A = self.fake_github.openFakePullRequest('org/project2', 'master', 'A')
+
+ # An error status should not cause it to be enqueued
+ A.setStatus(A.head_sha, 'error', 'null', 'null', 'check')
+ self.fake_github.emitEvent(A.getCommitStatusEvent('check',
+ state='error'))
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # A success status from unknown user should not cause it to be
+ # enqueued
+ A.setStatus(A.head_sha, 'success', 'null', 'null', 'check', user='foo')
+ self.fake_github.emitEvent(A.getCommitStatusEvent('check',
+ state='success',
+ user='foo'))
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # A success status from zuul goes in
+ A.setStatus(A.head_sha, 'success', 'null', 'null', 'check')
+ self.fake_github.emitEvent(A.getCommitStatusEvent('check'))
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+ self.assertEqual(self.history[0].name, 'project2-trigger')
+
+ # An error status for a different context should not cause it to be
+ # enqueued
+ A.setStatus(A.head_sha, 'error', 'null', 'null', 'gate')
+ self.fake_github.emitEvent(A.getCommitStatusEvent('gate',
+ state='error'))
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+
+ @simple_layout('layouts/requirements-github.yaml', driver='github')
+ def test_pipeline_require_review_username(self):
+ "Test pipeline requirement: review username"
+
+ A = self.fake_github.openFakePullRequest('org/project3', 'master', 'A')
+ # A comment event that we will keep submitting to trigger
+ comment = A.getCommentAddedEvent('test me')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ # No approval from derp so should not be enqueued
+ self.assertEqual(len(self.history), 0)
+
+ # Add an approved review from derp
+ A.addReview('derp', 'APPROVED')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+ self.assertEqual(self.history[0].name, 'project3-reviewusername')
+
+ @simple_layout('layouts/requirements-github.yaml', driver='github')
+ def test_pipeline_require_review_state(self):
+ "Test pipeline requirement: review state"
+
+ A = self.fake_github.openFakePullRequest('org/project4', 'master', 'A')
+ # Add derp to writers
+ A.writers.append('derp')
+ # A comment event that we will keep submitting to trigger
+ comment = A.getCommentAddedEvent('test me')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ # No positive review from derp so should not be enqueued
+ self.assertEqual(len(self.history), 0)
+
+ # A negative review from derp should not cause it to be enqueued
+ A.addReview('derp', 'CHANGES_REQUESTED')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # A positive from nobody should not cause it to be enqueued
+ A.addReview('nobody', 'APPROVED')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # A positive review from derp should cause it to be enqueued
+ A.addReview('derp', 'APPROVED')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+ self.assertEqual(self.history[0].name, 'project4-reviewreq')
+
+ @simple_layout('layouts/requirements-github.yaml', driver='github')
+ def test_pipeline_require_review_user_state(self):
+ "Test pipeline requirement: review state from user"
+
+ A = self.fake_github.openFakePullRequest('org/project5', 'master', 'A')
+ # Add derp and herp to writers
+ A.writers.extend(('derp', 'herp'))
+ # A comment event that we will keep submitting to trigger
+ comment = A.getCommentAddedEvent('test me')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ # No positive review from derp so should not be enqueued
+ self.assertEqual(len(self.history), 0)
+
+ # A negative review from derp should not cause it to be enqueued
+ A.addReview('derp', 'CHANGES_REQUESTED')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # A positive from nobody should not cause it to be enqueued
+ A.addReview('nobody', 'APPROVED')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # A positive review from herp (a writer) should not cause it to be
+ # enqueued
+ A.addReview('herp', 'APPROVED')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # A positive review from derp should cause it to be enqueued
+ A.addReview('derp', 'APPROVED')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+ self.assertEqual(self.history[0].name, 'project5-reviewuserstate')
+
+# TODO: Implement reject on approval username/state
+
+ @simple_layout('layouts/requirements-github.yaml', driver='github')
+ def test_pipeline_require_review_latest_user_state(self):
+ "Test pipeline requirement: review state from user"
+
+ A = self.fake_github.openFakePullRequest('org/project5', 'master', 'A')
+ # Add derp and herp to writers
+ A.writers.extend(('derp', 'herp'))
+ # A comment event that we will keep submitting to trigger
+ comment = A.getCommentAddedEvent('test me')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ # No positive review from derp so should not be enqueued
+ self.assertEqual(len(self.history), 0)
+
+ # The first negative review from derp should not cause it to be
+ # enqueued
+ for i in range(1, 4):
+ submitted_at = time.time() - 72 * 60 * 60
+ A.addReview('derp', 'CHANGES_REQUESTED',
+ submitted_at)
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # A positive review from derp should cause it to be enqueued
+ A.addReview('derp', 'APPROVED')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+ self.assertEqual(self.history[0].name, 'project5-reviewuserstate')
+
+ @simple_layout('layouts/requirements-github.yaml', driver='github')
+ def test_require_review_newer_than(self):
+
+ A = self.fake_github.openFakePullRequest('org/project6', 'master', 'A')
+ # Add derp and herp to writers
+ A.writers.extend(('derp', 'herp'))
+ # A comment event that we will keep submitting to trigger
+ comment = A.getCommentAddedEvent('test me')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ # No positive review from derp so should not be enqueued
+ self.assertEqual(len(self.history), 0)
+
+ # Add a too-old positive review, should not be enqueued
+ submitted_at = time.time() - 72 * 60 * 60
+ A.addReview('derp', 'APPROVED',
+ submitted_at)
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # Add a recent positive review
+ submitted_at = time.time() - 12 * 60 * 60
+ A.addReview('derp', 'APPROVED', submitted_at)
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+ self.assertEqual(self.history[0].name, 'project6-newerthan')
+
+ @simple_layout('layouts/requirements-github.yaml', driver='github')
+ def test_require_review_older_than(self):
+
+ A = self.fake_github.openFakePullRequest('org/project7', 'master', 'A')
+ # Add derp and herp to writers
+ A.writers.extend(('derp', 'herp'))
+ # A comment event that we will keep submitting to trigger
+ comment = A.getCommentAddedEvent('test me')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ # No positive review from derp so should not be enqueued
+ self.assertEqual(len(self.history), 0)
+
+ # Add a too-new positive, should not be enqueued
+ submitted_at = time.time() - 12 * 60 * 60
+ A.addReview('derp', 'APPROVED',
+ submitted_at)
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 0)
+
+ # Add an old enough positive, should enqueue
+ submitted_at = time.time() - 72 * 60 * 60
+ A.addReview('herp', 'APPROVED', submitted_at)
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ self.assertEqual(len(self.history), 1)
+ self.assertEqual(self.history[0].name, 'project7-olderthan')
+
+ @simple_layout('layouts/requirements-github.yaml', driver='github')
+ def test_require_open(self):
+
+ A = self.fake_github.openFakePullRequest('org/project8', 'master', 'A')
+ # A comment event that we will keep submitting to trigger
+ comment = A.getCommentAddedEvent('test me')
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+
+ # PR is open, we should have enqueued
+ self.assertEqual(len(self.history), 1)
+
+ # close the PR and try again
+ A.state = 'closed'
+ self.fake_github.emitEvent(comment)
+ self.waitUntilSettled()
+ # PR is closed, should not trigger
+ self.assertEqual(len(self.history), 1)
+
+ @simple_layout('layouts/requirements-github.yaml', driver='github')
+ def test_require_current(self):
+
+ A = self.fake_github.openFakePullRequest('org/project9', 'master', 'A')
+ # A sync event that we will keep submitting to trigger
+ sync = A.getPullRequestSynchronizeEvent()
+ self.fake_github.emitEvent(sync)
+ self.waitUntilSettled()
+
+ # PR head is current should enqueue
+ self.assertEqual(len(self.history), 1)
+
+ # Add a commit to the PR, re-issue the original comment event
+ A.addCommit()
+ self.fake_github.emitEvent(sync)
+ self.waitUntilSettled()
+ # Event hash is not current, should not trigger
+ self.assertEqual(len(self.history), 1)
diff --git a/tools/trigger-job.py b/tools/trigger-job.py
index 7123afc..dd69f1b 100755
--- a/tools/trigger-job.py
+++ b/tools/trigger-job.py
@@ -73,5 +73,6 @@
while not job.complete:
time.sleep(1)
+
if __name__ == '__main__':
main()
diff --git a/tools/update-storyboard.py b/tools/update-storyboard.py
index 12e6916..51434c9 100644
--- a/tools/update-storyboard.py
+++ b/tools/update-storyboard.py
@@ -96,5 +96,6 @@
if ok_lanes and not task_found:
add_task(sync, task, lanes[ok_lanes[0]])
+
if __name__ == '__main__':
main()
diff --git a/tox.ini b/tox.ini
index 6a50c6d..9b97eca 100644
--- a/tox.ini
+++ b/tox.ini
@@ -51,6 +51,6 @@
[flake8]
# These are ignored intentionally in openstack-infra projects;
# please don't submit patches that solely correct them or enable them.
-ignore = E305,E125,E129,E402,H,W503
+ignore = E125,E129,E402,H,W503
show-source = True
exclude = .venv,.tox,dist,doc,build,*.egg
diff --git a/zuul/ansible/callback/zuul_stream.py b/zuul/ansible/callback/zuul_stream.py
index e6b3461..fd95e92 100644
--- a/zuul/ansible/callback/zuul_stream.py
+++ b/zuul/ansible/callback/zuul_stream.py
@@ -40,6 +40,32 @@
yield buff
+def zuul_filter_result(result):
+ """Remove keys from shell/command output.
+
+ Zuul streams stdout into the log above, so including stdout and stderr
+ in the result dict that ansible displays in the logs is duplicate
+ noise. We keep stdout in the result dict so that other callback plugins
+ like ARA could also have access to it. But drop them here.
+
+ Remove changed so that we don't show a bunch of "changed" titles
+ on successful shell tasks, since that doesn't make sense from a Zuul
+ POV. The super class treats missing "changed" key as False.
+
+ Remove cmd because most of the script content where people want to
+ see the script run is run with -x. It's possible we may want to revist
+ this to be smarter about when we remove it - like, only remove it
+ if it has an embedded newline - so that for normal 'simple' uses
+ of cmd it'll echo what the command was for folks.
+ """
+
+ for key in ('changed', 'cmd',
+ 'stderr', 'stderr_lines',
+ 'stdout', 'stdout_lines'):
+ result.pop(key, None)
+ return result
+
+
class CallbackModule(default.CallbackModule):
'''
@@ -103,3 +129,37 @@
target=self._read_log, args=(host, ip))
p.daemon = True
p.start()
+
+ def v2_runner_on_failed(self, result, ignore_errors=False):
+ if result._task.action in ('command', 'shell'):
+ zuul_filter_result(result._result)
+ super(CallbackModule, self).v2_runner_on_failed(
+ result, ignore_errors=ignore_errors)
+
+ def v2_runner_on_ok(self, result):
+ if result._task.action in ('command', 'shell'):
+ zuul_filter_result(result._result)
+ else:
+ return super(CallbackModule, self).v2_runner_on_ok(result)
+
+ if self._play.strategy == 'free':
+ return super(CallbackModule, self).v2_runner_on_ok(result)
+
+ delegated_vars = result._result.get('_ansible_delegated_vars', None)
+
+ if delegated_vars:
+ msg = "ok: [{host} -> {delegated_host} %s]".format(
+ host=result._host.get_name(),
+ delegated_host=delegated_vars['ansible_host'])
+ else:
+ msg = "ok: [{host}]".format(host=result._host.get_name())
+
+ if result._task.loop and 'results' in result._result:
+ self._process_items(result)
+ else:
+ msg += " Runtime: {delta} Start: {start} End: {end}".format(
+ **result._result)
+
+ self._handle_warnings(result._result)
+
+ self._display.display(msg)
diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py
index 4b945a5..27ece54 100644
--- a/zuul/driver/github/githubconnection.py
+++ b/zuul/driver/github/githubconnection.py
@@ -13,11 +13,17 @@
# under the License.
import collections
+import datetime
import logging
import hmac
import hashlib
import time
+import cachecontrol
+from cachecontrol.cache import DictCache
+import iso8601
+import jwt
+import requests
import webob
import webob.dec
import voluptuous as v
@@ -29,6 +35,25 @@
from zuul.exceptions import MergeFailure
from zuul.driver.github.githubmodel import PullRequest, GithubTriggerEvent
+ACCESS_TOKEN_URL = 'https://api.github.com/installations/%s/access_tokens'
+PREVIEW_JSON_ACCEPT = 'application/vnd.github.machine-man-preview+json'
+
+
+class UTC(datetime.tzinfo):
+ """UTC"""
+
+ def utcoffset(self, dt):
+ return datetime.timedelta(0)
+
+ def tzname(self, dt):
+ return "UTC"
+
+ def dst(self, dt):
+ return datetime.timedelta(0)
+
+
+utc = UTC()
+
class GithubWebhookListener():
@@ -66,17 +91,38 @@
raise webob.exc.HTTPBadRequest(message)
try:
- event = method(request)
+ json_body = request.json_body
+ except:
+ message = 'Exception deserializing JSON body'
+ self.log.exception(message)
+ raise webob.exc.HTTPBadRequest(message)
+
+ # If there's any installation mapping information in the body then
+ # update the project mapping before any requests are made.
+ installation_id = json_body.get('installation', {}).get('id')
+ project_name = json_body.get('repository', {}).get('full_name')
+
+ if installation_id and project_name:
+ old_id = self.connection.installation_map.get(project_name)
+
+ if old_id and old_id != installation_id:
+ msg = "Unexpected installation_id change for %s. %d -> %d."
+ self.log.warning(msg, project_name, old_id, installation_id)
+
+ self.connection.installation_map[project_name] = installation_id
+
+ try:
+ event = method(json_body)
except:
self.log.exception('Exception when handling event:')
+ event = None
if event:
event.project_hostname = self.connection.canonical_hostname
self.log.debug('Scheduling github event: {0}'.format(event.type))
self.connection.sched.addEvent(event)
- def _event_push(self, request):
- body = request.json_body
+ def _event_push(self, body):
base_repo = body.get('repository')
event = GithubTriggerEvent()
@@ -96,8 +142,7 @@
return event
- def _event_pull_request(self, request):
- body = request.json_body
+ def _event_pull_request(self, body):
action = body.get('action')
pr_body = body.get('pull_request')
@@ -124,9 +169,8 @@
return event
- def _event_issue_comment(self, request):
+ def _event_issue_comment(self, body):
"""Handles pull request comments"""
- body = request.json_body
action = body.get('action')
if action != 'created':
return
@@ -144,9 +188,8 @@
event.action = 'comment'
return event
- def _event_pull_request_review(self, request):
+ def _event_pull_request_review(self, body):
"""Handles pull request reviews"""
- body = request.json_body
pr_body = body.get('pull_request')
if pr_body is None:
return
@@ -162,6 +205,25 @@
event.action = body.get('action')
return event
+ def _event_status(self, body):
+ action = body.get('action')
+ if action == 'pending':
+ return
+ pr_body = self.connection.getPullBySha(body['sha'])
+ if pr_body is None:
+ return
+
+ event = self._pull_request_to_event(pr_body)
+ event.account = self._get_sender(body)
+ event.type = 'pull_request'
+ event.action = 'status'
+ # Github API is silly. Webhook blob sets author data in
+ # 'sender', but API call to get status puts it in 'creator'.
+ # Duplicate the data so our code can look in one place
+ body['creator'] = body['sender']
+ event.status = "%s:%s:%s" % _status_as_tuple(body)
+ return event
+
def _issue_to_pull_request(self, body):
number = body.get('issue').get('number')
project_name = body.get('repository').get('full_name')
@@ -257,20 +319,32 @@
driver_name = 'github'
log = logging.getLogger("connection.github")
payload_path = 'payload'
- git_user = 'git'
def __init__(self, driver, connection_name, connection_config):
super(GithubConnection, self).__init__(
driver, connection_name, connection_config)
- self.github = None
self._change_cache = {}
self.projects = {}
- self._git_ssh = bool(self.connection_config.get('sshkey', None))
+ self.git_ssh_key = self.connection_config.get('sshkey')
self.git_host = self.connection_config.get('git_host', 'github.com')
self.canonical_hostname = self.connection_config.get(
'canonical_hostname', self.git_host)
self.source = driver.getSource(self)
+ self._github = None
+ self.app_id = None
+ self.app_key = None
+
+ self.installation_map = {}
+ self.installation_token_cache = {}
+
+ # NOTE(jamielennox): Better here would be to cache to memcache or file
+ # or something external - but zuul already sucks at restarting so in
+ # memory probably doesn't make this much worse.
+ self.cache_adapter = cachecontrol.CacheControlAdapter(
+ DictCache(),
+ cache_etags=True)
+
def onLoad(self):
webhook_listener = GithubWebhookListener(self)
self.registerHttpHandler(self.payload_path,
@@ -280,20 +354,107 @@
def onStop(self):
self.unregisterHttpHandler(self.payload_path)
- def _authenticateGithubAPI(self):
- token = self.connection_config.get('api_token', None)
- if token is not None:
- if self.git_host != 'github.com':
- url = 'https://%s/' % self.git_host
- self.github = github3.enterprise_login(token=token, url=url)
- else:
- self.github = github3.login(token=token)
- self.log.info("Github API Authentication successful.")
+ def _createGithubClient(self):
+ if self.git_host != 'github.com':
+ url = 'https://%s/' % self.git_host
+ github = github3.GitHubEnterprise(url)
else:
- self.github = None
- self.log.info(
- "No Github credentials found in zuul configuration, cannot "
- "authenticate.")
+ github = github3.GitHub()
+
+ # anything going through requests to http/s goes through cache
+ github.session.mount('http://', self.cache_adapter)
+ github.session.mount('https://', self.cache_adapter)
+ return github
+
+ def _authenticateGithubAPI(self):
+ config = self.connection_config
+
+ api_token = config.get('api_token')
+
+ app_id = config.get('app_id')
+ app_key = None
+ app_key_file = config.get('app_key')
+
+ self._github = self._createGithubClient()
+
+ if api_token:
+ self._github.login(token=api_token)
+
+ if app_key_file:
+ try:
+ with open(app_key_file, 'r') as f:
+ app_key = f.read()
+ except IOError:
+ m = "Failed to open app key file for reading: %s"
+ self.log.error(m, app_key_file)
+
+ if (app_id or app_key) and \
+ not (app_id and app_key):
+ self.log.warning("You must provide an app_id and "
+ "app_key to use installation based "
+ "authentication")
+
+ return
+
+ if app_id:
+ self.app_id = int(app_id)
+ if app_key:
+ self.app_key = app_key
+
+ def _get_installation_key(self, project, user_id=None):
+ installation_id = self.installation_map.get(project)
+
+ if not installation_id:
+ self.log.error("No installation ID available for project %s",
+ project)
+ return ''
+
+ now = datetime.datetime.now(utc)
+ token, expiry = self.installation_token_cache.get(installation_id,
+ (None, None))
+
+ if ((not expiry) or (not token) or (now >= expiry)):
+ expiry = now + datetime.timedelta(minutes=5)
+
+ data = {'iat': now, 'exp': expiry, 'iss': self.app_id}
+ app_token = jwt.encode(data,
+ self.app_key,
+ algorithm='RS256')
+
+ url = ACCESS_TOKEN_URL % installation_id
+ headers = {'Accept': PREVIEW_JSON_ACCEPT,
+ 'Authorization': 'Bearer %s' % app_token}
+ json_data = {'user_id': user_id} if user_id else None
+
+ response = requests.post(url, headers=headers, json=json_data)
+ response.raise_for_status()
+
+ data = response.json()
+
+ expiry = iso8601.parse_date(data['expires_at'])
+ expiry -= datetime.timedelta(minutes=2)
+ token = data['token']
+
+ self.installation_token_cache[installation_id] = (token, expiry)
+
+ return token
+
+ def getGithubClient(self,
+ project=None,
+ user_id=None,
+ use_app=True):
+ # if you're authenticating for a project and you're an integration then
+ # you need to use the installation specific token. There are some
+ # operations that are not yet supported by integrations so
+ # use_app lets you use api_key auth.
+ if use_app and project and self.app_id:
+ github = self._createGithubClient()
+ github.login(token=self._get_installation_key(project, user_id))
+ return github
+
+ # if we're using api_key authentication then this is already token
+ # authenticated, if not then anonymous is the best we have.
+ return self._github
def maintainCache(self, relevant):
for key, change in self._change_cache.items():
@@ -315,7 +476,13 @@
change.patchset = event.patch_number
change.files = self.getPullFileNames(project, change.number)
change.title = event.title
+ change.status = self._get_statuses(project, event.patch_number)
+ change.reviews = self.getPullReviews(project, change.number)
change.source_event = event
+ change.open = self.getPullOpen(project, change.number)
+ change.is_current_patchset = self.getIsCurrent(project,
+ change.number,
+ event.patch_number)
elif event.ref:
change = Ref(project)
change.ref = event.ref
@@ -328,12 +495,16 @@
return change
def getGitUrl(self, project):
- if self._git_ssh:
- url = 'ssh://%s@%s/%s.git' % \
- (self.git_user, self.git_host, project)
- else:
- url = 'https://%s/%s' % (self.git_host, project)
- return url
+ if self.git_ssh_key:
+ return 'ssh://git@%s/%s.git' % (self.git_host, project)
+
+ if self.app_id:
+ installation_key = self._get_installation_key(project)
+ return 'https://x-access-token:%s@%s/%s' % (installation_key,
+ self.git_host,
+ project)
+
+ return 'https://%s/%s' % (self.git_host, project)
def getGitwebUrl(self, project, sha=None):
url = 'https://%s/%s' % (self.git_host, project)
@@ -348,19 +519,21 @@
self.projects[project.name] = project
def getProjectBranches(self, project):
+ github = self.getGithubClient()
owner, proj = project.name.split('/')
- repository = self.github.repository(owner, proj)
+ repository = github.repository(owner, proj)
branches = [branch.name for branch in repository.branches()]
- log_rate_limit(self.log, self.github)
+ log_rate_limit(self.log, github)
return branches
def getPullUrl(self, project, number):
return '%s/pull/%s' % (self.getGitwebUrl(project), number)
def getPull(self, project_name, number):
+ github = self.getGithubClient(project_name)
owner, proj = project_name.split('/')
- pr = self.github.pull_request(owner, proj, number).as_dict()
- log_rate_limit(self.log, self.github)
+ pr = github.pull_request(owner, proj, number).as_dict()
+ log_rate_limit(self.log, github)
return pr
def canMerge(self, change, allow_needs):
@@ -376,60 +549,219 @@
# For now, just send back a True value.
return True
+ def getPullBySha(self, sha):
+ query = '%s type:pr is:open' % sha
+ pulls = []
+ github = self.getGithubClient()
+ for issue in github.search_issues(query=query):
+ pr_url = issue.issue.pull_request().as_dict().get('url')
+ if not pr_url:
+ continue
+ # the issue provides no good description of the project :\
+ owner, project, _, number = pr_url.split('/')[4:]
+ github = self.getGithubClient("%s/%s" % (owner, project))
+ pr = github.pull_request(owner, project, number)
+ if pr.head.sha != sha:
+ continue
+ if pr.as_dict() in pulls:
+ continue
+ pulls.append(pr.as_dict())
+
+ log_rate_limit(self.log, github)
+ if len(pulls) > 1:
+ raise Exception('Multiple pulls found with head sha %s' % sha)
+
+ if len(pulls) == 0:
+ return None
+ return pulls.pop()
+
def getPullFileNames(self, project, number):
+ github = self.getGithubClient(project)
owner, proj = project.name.split('/')
filenames = [f.filename for f in
- self.github.pull_request(owner, proj, number).files()]
- log_rate_limit(self.log, self.github)
+ github.pull_request(owner, proj, number).files()]
+ log_rate_limit(self.log, github)
return filenames
+ def getPullReviews(self, project, number):
+ owner, proj = project.name.split('/')
+
+ revs = self._getPullReviews(owner, proj, number)
+
+ reviews = {}
+ for rev in revs:
+ user = rev.get('user').get('login')
+ review = {
+ 'by': {
+ 'username': user,
+ 'email': rev.get('user').get('email'),
+ },
+ 'grantedOn': int(time.mktime(self._ghTimestampToDate(
+ rev.get('submitted_at')))),
+ }
+
+ review['type'] = rev.get('state').lower()
+ review['submitted_at'] = rev.get('submitted_at')
+
+ # Get user's rights. A user always has read to leave a review
+ review['permission'] = 'read'
+ permission = self.getRepoPermission(project.name, user)
+ if permission == 'write':
+ review['permission'] = 'write'
+ if permission == 'admin':
+ review['permission'] = 'admin'
+
+ if user not in reviews:
+ reviews[user] = review
+ else:
+ # if there are multiple reviews per user, keep the newest
+ # note that this breaks the ability to set the 'older-than'
+ # option on a review requirement.
+ if review['grantedOn'] > reviews[user]['grantedOn']:
+ reviews[user] = review
+
+ return reviews.values()
+
+ def _getPullReviews(self, owner, project, number):
+ # make a list out of the reviews so that we complete our
+ # API transaction
+ # reviews are not yet supported by integrations, use api_key:
+ # https://platform.github.community/t/api-endpoint-for-pr-reviews/409
+ github = self.getGithubClient("%s/%s" % (owner, project),
+ use_app=False)
+ reviews = [review.as_dict() for review in
+ github.pull_request(owner, project, number).reviews()]
+
+ log_rate_limit(self.log, github)
+ return reviews
+
def getUser(self, login):
- return GithubUser(self.github, login)
+ return GithubUser(self.getGithubClient(), login)
def getUserUri(self, login):
return 'https://%s/%s' % (self.git_host, login)
- def commentPull(self, project, pr_number, message):
+ def getRepoPermission(self, project, login):
+ github = self.getGithubClient(project)
owner, proj = project.split('/')
- repository = self.github.repository(owner, proj)
+ # This gets around a missing API call
+ # need preview header
+ headers = {'Accept': 'application/vnd.github.korra-preview'}
+
+ # Create a repo object
+ repository = github.repository(owner, project)
+ # Build up a URL
+ url = repository._build_url('collaborators', login, 'permission',
+ base_url=repository._api)
+ # Get the data
+ perms = repository._get(url, headers=headers)
+
+ log_rate_limit(self.log, github)
+
+ # no known user, maybe deleted since review?
+ if perms.status_code == 404:
+ return 'none'
+
+ # get permissions from the data
+ return perms.json()['permission']
+
+ def commentPull(self, project, pr_number, message):
+ github = self.getGithubClient(project)
+ owner, proj = project.split('/')
+ repository = github.repository(owner, proj)
pull_request = repository.issue(pr_number)
pull_request.create_comment(message)
- log_rate_limit(self.log, self.github)
+ log_rate_limit(self.log, github)
def mergePull(self, project, pr_number, commit_message='', sha=None):
+ github = self.getGithubClient(project)
owner, proj = project.split('/')
- pull_request = self.github.pull_request(owner, proj, pr_number)
+ pull_request = github.pull_request(owner, proj, pr_number)
try:
result = pull_request.merge(commit_message=commit_message, sha=sha)
except MethodNotAllowed as e:
raise MergeFailure('Merge was not successful due to mergeability'
' conflict, original error is %s' % e)
- log_rate_limit(self.log, self.github)
+ log_rate_limit(self.log, github)
if not result:
raise Exception('Pull request was not merged')
+ def getCommitStatuses(self, project, sha):
+ github = self.getGithubClient(project)
+ owner, proj = project.split('/')
+ repository = github.repository(owner, proj)
+ commit = repository.commit(sha)
+ # make a list out of the statuses so that we complete our
+ # API transaction
+ statuses = [status.as_dict() for status in commit.statuses()]
+
+ log_rate_limit(self.log, github)
+ return statuses
+
def setCommitStatus(self, project, sha, state, url='', description='',
context=''):
+ github = self.getGithubClient(project)
owner, proj = project.split('/')
- repository = self.github.repository(owner, proj)
+ repository = github.repository(owner, proj)
repository.create_status(sha, state, url, description, context)
- log_rate_limit(self.log, self.github)
+ log_rate_limit(self.log, github)
def labelPull(self, project, pr_number, label):
+ github = self.getGithubClient(project)
owner, proj = project.split('/')
- pull_request = self.github.issue(owner, proj, pr_number)
+ pull_request = github.issue(owner, proj, pr_number)
pull_request.add_labels(label)
- log_rate_limit(self.log, self.github)
+ log_rate_limit(self.log, github)
def unlabelPull(self, project, pr_number, label):
+ github = self.getGithubClient(project)
owner, proj = project.split('/')
- pull_request = self.github.issue(owner, proj, pr_number)
+ pull_request = github.issue(owner, proj, pr_number)
pull_request.remove_label(label)
- log_rate_limit(self.log, self.github)
+ log_rate_limit(self.log, github)
+
+ def getPullOpen(self, project, number):
+ pr = self.getPull(project, number)
+ return pr.get('state') == 'open'
+
+ def getIsCurrent(self, project, number, sha):
+ pr = self.getPull(project, number)
+ return pr.get('head').get('sha') == sha
def _ghTimestampToDate(self, timestamp):
return time.strptime(timestamp, '%Y-%m-%dT%H:%M:%SZ')
+ def _get_statuses(self, project, sha):
+ # A ref can have more than one status from each context,
+ # however the API returns them in order, newest first.
+ # So we can keep track of which contexts we've already seen
+ # and throw out the rest. Our unique key is based on
+ # the user and the context, since context is free form and anybody
+ # can put whatever they want there. We want to ensure we track it
+ # by user, so that we can require/trigger by user too.
+ seen = []
+ statuses = []
+ for status in self.getCommitStatuses(project.name, sha):
+ stuple = _status_as_tuple(status)
+ if "%s:%s" % (stuple[0], stuple[1]) not in seen:
+ statuses.append("%s:%s:%s" % stuple)
+ seen.append("%s:%s" % (stuple[0], stuple[1]))
+
+ return statuses
+
+
+def _status_as_tuple(status):
+ """Translate a status into a tuple of user, context, state"""
+
+ creator = status.get('creator')
+ if not creator:
+ user = "Unknown"
+ else:
+ user = creator.get('login')
+ context = status.get('context')
+ state = status.get('state')
+ return (user, context, state)
+
def log_rate_limit(log, github):
try:
diff --git a/zuul/driver/github/githubmodel.py b/zuul/driver/github/githubmodel.py
index 0d77cae..3e25115 100644
--- a/zuul/driver/github/githubmodel.py
+++ b/zuul/driver/github/githubmodel.py
@@ -14,9 +14,12 @@
# License for the specific language governing permissions and limitations
# under the License.
+import copy
import re
+import time
-from zuul.model import Change, TriggerEvent, EventFilter
+from zuul.model import Change, TriggerEvent, EventFilter, RefFilter
+from zuul.driver.util import time_to_seconds
EMPTY_GIT_REF = '0' * 40 # git sha of all zeros, used during creates/deletes
@@ -27,6 +30,7 @@
super(PullRequest, self).__init__(project)
self.updated_at = None
self.title = None
+ self.reviews = []
def isUpdateOf(self, other):
if (hasattr(other, 'number') and self.number == other.number and
@@ -55,13 +59,95 @@
return False
-class GithubEventFilter(EventFilter):
+class GithubCommonFilter(object):
+ def __init__(self, required_reviews=[], required_statuses=[]):
+ self._required_reviews = copy.deepcopy(required_reviews)
+ self.required_reviews = self._tidy_reviews(required_reviews)
+ self.required_statuses = required_statuses
+
+ def _tidy_reviews(self, reviews):
+ for r in reviews:
+ for k, v in r.items():
+ if k == 'username':
+ r['username'] = re.compile(v)
+ elif k == 'email':
+ r['email'] = re.compile(v)
+ elif k == 'newer-than':
+ r[k] = time_to_seconds(v)
+ elif k == 'older-than':
+ r[k] = time_to_seconds(v)
+ return reviews
+
+ def _match_review_required_review(self, rreview, review):
+ # Check if the required review and review match
+ now = time.time()
+ by = review.get('by', {})
+ for k, v in rreview.items():
+ if k == 'username':
+ if (not v.search(by.get('username', ''))):
+ return False
+ elif k == 'email':
+ if (not v.search(by.get('email', ''))):
+ return False
+ elif k == 'newer-than':
+ t = now - v
+ if (review['grantedOn'] < t):
+ return False
+ elif k == 'older-than':
+ t = now - v
+ if (review['grantedOn'] >= t):
+ return False
+ elif k == 'type':
+ if review['type'] != v:
+ return False
+ elif k == 'permission':
+ # If permission is read, we've matched. You must have read
+ # to provide a review. Write or admin permission is different.
+ if v != 'read':
+ if review['permission'] != v:
+ return False
+ return True
+
+ def matchesReviews(self, change):
+ if self.required_reviews and not change.reviews:
+ # No reviews means no matching
+ return False
+
+ return self.matchesRequiredReviews(change)
+
+ def matchesRequiredReviews(self, change):
+ for rreview in self.required_reviews:
+ matches_review = False
+ for review in change.reviews:
+ if self._match_review_required_review(rreview, review):
+ # Consider matched if any review matches
+ matches_review = True
+ break
+ if not matches_review:
+ return False
+ return True
+
+ def matchesRequiredStatuses(self, change):
+ # statuses are ORed
+ # A PR head can have multiple statuses on it. If the change
+ # statuses and the filter statuses are a null intersection, there
+ # are no matches and we return false
+ if self.required_statuses:
+ if set(change.status).isdisjoint(set(self.required_statuses)):
+ return False
+ return True
+
+
+class GithubEventFilter(EventFilter, GithubCommonFilter):
def __init__(self, trigger, types=[], branches=[], refs=[],
comments=[], actions=[], labels=[], unlabels=[],
- states=[], ignore_deletes=True):
+ states=[], statuses=[], required_statuses=[],
+ ignore_deletes=True):
EventFilter.__init__(self, trigger)
+ GithubCommonFilter.__init__(self, required_statuses=required_statuses)
+
self._types = types
self._branches = branches
self._refs = refs
@@ -74,6 +160,8 @@
self.labels = labels
self.unlabels = unlabels
self.states = states
+ self.statuses = statuses
+ self.required_statuses = required_statuses
self.ignore_deletes = ignore_deletes
def __repr__(self):
@@ -97,6 +185,10 @@
ret += ' unlabels: %s' % ', '.join(self.unlabels)
if self.states:
ret += ' states: %s' % ', '.join(self.states)
+ if self.statuses:
+ ret += ' statuses: %s' % ', '.join(self.statuses)
+ if self.required_statuses:
+ ret += ' required_statuses: %s' % ', '.join(self.required_statuses)
ret += '>'
return ret
@@ -160,4 +252,58 @@
if self.states and event.state not in self.states:
return False
+ # statuses are ORed
+ if self.statuses and event.status not in self.statuses:
+ return False
+
+ if not self.matchesRequiredStatuses(change):
+ return False
+
+ return True
+
+
+class GithubRefFilter(RefFilter, GithubCommonFilter):
+ def __init__(self, statuses=[], required_reviews=[], open=None,
+ current_patchset=None):
+ RefFilter.__init__(self)
+
+ GithubCommonFilter.__init__(self, required_reviews=required_reviews,
+ required_statuses=statuses)
+ self.statuses = statuses
+ self.open = open
+ self.current_patchset = current_patchset
+
+ def __repr__(self):
+ ret = '<GithubRefFilter'
+
+ if self.statuses:
+ ret += ' statuses: %s' % ', '.join(self.statuses)
+ if self.required_reviews:
+ ret += (' required-reviews: %s' %
+ str(self.required_reviews))
+ if self.open:
+ ret += ' open: %s' % self.open
+ if self.current_patchset:
+ ret += ' current-patchset: %s' % self.current_patchset
+
+ ret += '>'
+
+ return ret
+
+ def matches(self, change):
+ if not self.matchesRequiredStatuses(change):
+ return False
+
+ if self.open is not None:
+ if self.open != change.open:
+ return False
+
+ if self.current_patchset is not None:
+ if self.current_patchset != change.is_current_patchset:
+ return False
+
+ # required reviews are ANDed
+ if not self.matchesReviews(change):
+ return False
+
return True
diff --git a/zuul/driver/github/githubreporter.py b/zuul/driver/github/githubreporter.py
index ffec26a..2d8e6cc 100644
--- a/zuul/driver/github/githubreporter.py
+++ b/zuul/driver/github/githubreporter.py
@@ -18,6 +18,7 @@
from zuul.reporter import BaseReporter
from zuul.exceptions import MergeFailure
+from zuul.driver.util import scalar_or_list
class GithubReporter(BaseReporter):
@@ -49,11 +50,14 @@
if (self._merge and
hasattr(item.change, 'number')):
self.mergePull(item)
+ if not item.change.is_merged:
+ msg = self._formatItemReportMergeFailure(pipeline, item)
+ self.addPullComment(pipeline, item, msg)
if self._labels or self._unlabels:
self.setLabels(item)
- def addPullComment(self, pipeline, item):
- message = self._formatItemReport(pipeline, item)
+ def addPullComment(self, pipeline, item, comment=None):
+ message = comment or self._formatItemReport(pipeline, item)
project = item.change.project.name
pr_number = item.change.number
self.log.debug(
@@ -64,7 +68,7 @@
def setPullStatus(self, pipeline, item):
project = item.change.project.name
sha = item.change.patchset
- context = pipeline.name
+ context = '%s/%s' % (pipeline.layout.tenant.name, pipeline.name)
state = self._commit_status
url = ''
if self.connection.sched.config.has_option('zuul', 'status_url'):
@@ -92,13 +96,21 @@
self.log.debug('Reporting change %s, params %s, merging via API' %
(item.change, self.config))
message = self._formatMergeMessage(item.change)
- try:
- self.connection.mergePull(project, pr_number, message, sha)
- except MergeFailure:
- time.sleep(2)
- self.log.debug('Trying to merge change %s again...' % item.change)
- self.connection.mergePull(project, pr_number, message, sha)
- item.change.is_merged = True
+
+ for i in [1, 2]:
+ try:
+ self.connection.mergePull(project, pr_number, message, sha)
+ item.change.is_merged = True
+ return
+ except MergeFailure:
+ self.log.exception(
+ 'Merge attempt of change %s %s/2 failed.' %
+ (item.change, i), exc_info=True)
+ if i == 1:
+ time.sleep(2)
+ self.log.warning(
+ 'Merge of change %s failed after 2 attempts, giving up' %
+ item.change)
def setLabels(self, item):
project = item.change.project.name
@@ -143,14 +155,11 @@
def getSchema():
- def toList(x):
- return v.Any([x], x)
-
github_reporter = v.Schema({
'status': v.Any('pending', 'success', 'failure'),
'comment': bool,
'merge': bool,
- 'label': toList(str),
- 'unlabel': toList(str)
+ 'label': scalar_or_list(str),
+ 'unlabel': scalar_or_list(str)
})
return github_reporter
diff --git a/zuul/driver/github/githubsource.py b/zuul/driver/github/githubsource.py
index e7d19ac..58ca2b9 100644
--- a/zuul/driver/github/githubsource.py
+++ b/zuul/driver/github/githubsource.py
@@ -14,9 +14,12 @@
import logging
import time
+import voluptuous as v
from zuul.source import BaseSource
from zuul.model import Project
+from zuul.driver.github.githubmodel import GithubRefFilter
+from zuul.driver.util import scalar_or_list, to_list
class GithubSource(BaseSource):
@@ -92,15 +95,35 @@
return time.strptime(timestamp, '%Y-%m-%dT%H:%M:%SZ')
def getRequireFilters(self, config):
- return []
+ f = GithubRefFilter(
+ statuses=to_list(config.get('status')),
+ required_reviews=to_list(config.get('review')),
+ open=config.get('open'),
+ current_patchset=config.get('current-patchset'),
+ )
+ return [f]
def getRejectFilters(self, config):
return []
+review = v.Schema({'username': str,
+ 'email': str,
+ 'older-than': str,
+ 'newer-than': str,
+ 'type': str,
+ 'permission': v.Any('read', 'write', 'admin'),
+ })
+
+
def getRequireSchema():
- return {}
+ require = {'status': scalar_or_list(str),
+ 'review': scalar_or_list(review),
+ 'open': bool,
+ 'current-patchset': bool}
+ return require
def getRejectSchema():
- return {}
+ reject = {'review': scalar_or_list(review)}
+ return reject
diff --git a/zuul/driver/github/githubtrigger.py b/zuul/driver/github/githubtrigger.py
index f0bd2f4..328879d 100644
--- a/zuul/driver/github/githubtrigger.py
+++ b/zuul/driver/github/githubtrigger.py
@@ -16,6 +16,7 @@
import voluptuous as v
from zuul.trigger import BaseTrigger
from zuul.driver.github.githubmodel import GithubEventFilter
+from zuul.driver.util import scalar_or_list, to_list
class GithubTrigger(BaseTrigger):
@@ -23,25 +24,20 @@
log = logging.getLogger("zuul.trigger.GithubTrigger")
def getEventFilters(self, trigger_config):
- def toList(item):
- if not item:
- return []
- if isinstance(item, list):
- return item
- return [item]
-
efilters = []
- for trigger in toList(trigger_config):
+ for trigger in to_list(trigger_config):
f = GithubEventFilter(
trigger=self,
- types=toList(trigger['event']),
- actions=toList(trigger.get('action')),
- branches=toList(trigger.get('branch')),
- refs=toList(trigger.get('ref')),
- comments=toList(trigger.get('comment')),
- labels=toList(trigger.get('label')),
- unlabels=toList(trigger.get('unlabel')),
- states=toList(trigger.get('state'))
+ types=to_list(trigger['event']),
+ actions=to_list(trigger.get('action')),
+ branches=to_list(trigger.get('branch')),
+ refs=to_list(trigger.get('ref')),
+ comments=to_list(trigger.get('comment')),
+ labels=to_list(trigger.get('label')),
+ unlabels=to_list(trigger.get('unlabel')),
+ states=to_list(trigger.get('state')),
+ statuses=to_list(trigger.get('status')),
+ required_statuses=to_list(trigger.get('require-status'))
)
efilters.append(f)
@@ -52,21 +48,20 @@
def getSchema():
- def toList(x):
- return v.Any([x], x)
-
github_trigger = {
v.Required('event'):
- toList(v.Any('pull_request',
- 'pull_request_review',
- 'push')),
- 'action': toList(str),
- 'branch': toList(str),
- 'ref': toList(str),
- 'comment': toList(str),
- 'label': toList(str),
- 'unlabel': toList(str),
- 'state': toList(str),
+ scalar_or_list(v.Any('pull_request',
+ 'pull_request_review',
+ 'push')),
+ 'action': scalar_or_list(str),
+ 'branch': scalar_or_list(str),
+ 'ref': scalar_or_list(str),
+ 'comment': scalar_or_list(str),
+ 'label': scalar_or_list(str),
+ 'unlabel': scalar_or_list(str),
+ 'state': scalar_or_list(str),
+ 'require-status': scalar_or_list(str),
+ 'status': scalar_or_list(str)
}
return github_trigger
diff --git a/zuul/driver/sql/alembic_reporter/env.py b/zuul/driver/sql/alembic_reporter/env.py
index 56a5b7e..4542a22 100644
--- a/zuul/driver/sql/alembic_reporter/env.py
+++ b/zuul/driver/sql/alembic_reporter/env.py
@@ -64,6 +64,7 @@
with context.begin_transaction():
context.run_migrations()
+
if context.is_offline_mode():
run_migrations_offline()
else:
diff --git a/zuul/lib/connections.py b/zuul/lib/connections.py
index 720299a..9908fff 100644
--- a/zuul/lib/connections.py
+++ b/zuul/lib/connections.py
@@ -105,7 +105,7 @@
# The merger and the reporter only needs source driver.
# This makes sure Reporter like the SQLDriver are only created by
# the scheduler process
- if source_only and not issubclass(driver, SourceInterface):
+ if source_only and not isinstance(driver, SourceInterface):
continue
connection = driver.getConnection(con_name, con_config)
@@ -138,10 +138,11 @@
# Create default connections for drivers which need no
# connection information (e.g., 'timer' or 'zuul').
- for driver in self.drivers.values():
- if not hasattr(driver, 'getConnection'):
- connections[driver.name] = DefaultConnection(
- driver, driver.name, {})
+ if not source_only:
+ for driver in self.drivers.values():
+ if not hasattr(driver, 'getConnection'):
+ connections[driver.name] = DefaultConnection(
+ driver, driver.name, {})
self.connections = connections