Merge "Add support for requiring github pr head status" into feature/zuulv3

tests/base.py

@@ -569,13 +569,11 @@ class FakeGithubPullRequest(object):
         """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 +693,10 @@ class FakeGithubPullRequest(object):
                     }
                 },
                 'head': {
-                    'sha': self.head_sha
+                    'sha': self.head_sha,
+                    'repo': {
+                        'full_name': self.project
+                    }
                 }
             },
             'label': {
@@ -742,6 +743,9 @@ class FakeGithubPullRequest(object):
         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 +758,21 @@ class FakeGithubPullRequest(object):
         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):
+        # Since we're bypassing github API, which would require a user, we
+        # hard set the user as 'zuul' here.
+        user = 'zuul'
+        # 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
-        }
-
-    def _clearStatuses(self):
-        self.statuses = {}
+            'description': description,
+            'context': context,
+            'creator': {
+                'login': user
+            }
+        }))
 
     def _getPRReference(self):
         return '%s/head' % self.number
@@ -783,7 +793,10 @@ class FakeGithubPullRequest(object):
                     }
                 },
                 'head': {
-                    'sha': self.head_sha
+                    'sha': self.head_sha,
+                    'repo': {
+                        'full_name': self.project
+                    }
                 }
             },
             'sender': {
@@ -857,7 +870,10 @@ class FakeGithubConnection(githubconnection.GithubConnection):
             },
             'mergeable': True,
             'head': {
-                'sha': pr.head_sha
+                'sha': pr.head_sha,
+                'repo': {
+                    'full_name': pr.project
+                }
             }
         }
         return data
@@ -901,6 +917,14 @@ class FakeGithubConnection(githubconnection.GithubConnection):
         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('/')
+            if (pr_owner == owner and pr_project == proj and
+                pr.head_sha == sha):
+                return pr.statuses[sha]
+
     def setCommitStatus(self, project, sha, state,
                         url='', description='', context=''):
         owner, proj = project.split('/')
@@ -908,7 +932,7 @@ class FakeGithubConnection(githubconnection.GithubConnection):
             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]

tests/fixtures/layouts/reporting-github.yaml

@@ -28,6 +28,7 @@
     success:
       github:
         comment: false
+        status: 'success'
     failure:
       github:
         comment: false

tests/fixtures/layouts/requirements-github.yaml

@@ -0,0 +1,23 @@
+- 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
+
+- job:
+    name: project1-pipeline
+
+- project:
+    name: org/project1
+    pipeline:
+      jobs:
+        - project1-pipeline

tests/unit/test_github_driver.py

@@ -253,10 +253,14 @@ class TestGithubDriver(ZuulTestCase):
         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('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 @@ class TestGithubDriver(ZuulTestCase):
         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('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 @@ class TestGithubDriver(ZuulTestCase):
         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 @@ class TestGithubDriver(ZuulTestCase):
         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('reporting', report_status['context'])
+        self.assertEqual('success', report_status['state'])
         self.assertEqual(2, len(A.comments))
 
     @simple_layout('layouts/merging-github.yaml', driver='github')

tests/unit/test_github_requirements.py

@@ -0,0 +1,45 @@
+#!/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.
+
+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')

zuul/driver/github/githubconnection.py

@@ -315,6 +315,7 @@ class GithubConnection(BaseConnection):
             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.source_event = event
         elif event.ref:
             change = Ref(project)
@@ -408,6 +409,17 @@ class GithubConnection(BaseConnection):
         if not result:
             raise Exception('Pull request was not merged')
 
+    def getCommitStatuses(self, project, sha):
+        owner, proj = project.split('/')
+        repository = self.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, self.github)
+        return statuses
+
     def setCommitStatus(self, project, sha, state, url='', description='',
                         context=''):
         owner, proj = project.split('/')
@@ -430,6 +442,30 @@ class GithubConnection(BaseConnection):
     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):
+            # creator can be None if the user has been removed.
+            creator = status.get('creator')
+            if not creator:
+                continue
+            user = creator.get('login')
+            context = status.get('context')
+            state = status.get('state')
+            if "%s:%s" % (user, context) not in seen:
+                statuses.append("%s:%s:%s" % (user, context, state))
+                seen.append("%s:%s" % (user, context))
+
+        return statuses
+
 
 def log_rate_limit(log, github):
     try:

zuul/driver/github/githubmodel.py

@@ -16,7 +16,7 @@
 
 import re
 
-from zuul.model import Change, TriggerEvent, EventFilter
+from zuul.model import Change, TriggerEvent, EventFilter, RefFilter
 
 
 EMPTY_GIT_REF = '0' * 40  # git sha of all zeros, used during creates/deletes
@@ -161,3 +161,31 @@ class GithubEventFilter(EventFilter):
             return False
 
         return True
+
+
+class GithubRefFilter(RefFilter):
+    def __init__(self, statuses=[]):
+        RefFilter.__init__(self)
+
+        self.statuses = statuses
+
+    def __repr__(self):
+        ret = '<GithubRefFilter'
+
+        if self.statuses:
+            ret += ' statuses: %s' % ', '.join(self.statuses)
+
+        ret += '>'
+
+        return ret
+
+    def matches(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.statuses:
+            if set(change.status).isdisjoint(set(self.statuses)):
+                return False
+
+        return True

zuul/driver/github/githubsource.py

@@ -17,6 +17,8 @@ import time
 
 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,14 +94,18 @@ class GithubSource(BaseSource):
         return time.strptime(timestamp, '%Y-%m-%dT%H:%M:%SZ')
 
     def getRequireFilters(self, config):
-        return []
+        f = GithubRefFilter(
+            statuses=to_list(config.get('status')),
+        )
+        return [f]
 
     def getRejectFilters(self, config):
         return []
 
 
 def getRequireSchema():
-    return {}
+    require = {'status': scalar_or_list(str)}
+    return require
 
 
 def getRejectSchema():