diff options
| -rw-r--r-- | requirements.txt | 1 | ||||
| -rw-r--r-- | tests/fake_graphql.py | 20 | ||||
| -rw-r--r-- | zuul/driver/github/githubconnection.py | 3 | ||||
| -rw-r--r-- | zuul/driver/github/graphql/__init__.py | 29 | ||||
| -rw-r--r-- | zuul/driver/github/graphql/canmerge-legacy.graphql | 5 | ||||
| -rw-r--r-- | zuul/driver/github/graphql/canmerge.graphql | 5 |
6 files changed, 51 insertions, 12 deletions
diff --git a/requirements.txt b/requirements.txt index 019bd352a..20778a1fd 100644 --- a/requirements.txt +++ b/requirements.txt @@ -32,7 +32,6 @@ paho-mqtt cherrypy ws4py routes -pathspec jsonpath-rw urllib3!=1.25.4,!=1.25.5 # https://github.com/urllib3/urllib3/pull/1684 cheroot!=8.1.*,!=8.2.*,!=8.3.0 # https://github.com/cherrypy/cheroot/issues/263 diff --git a/tests/fake_graphql.py b/tests/fake_graphql.py index 6eaa965ba..9e86f81ad 100644 --- a/tests/fake_graphql.py +++ b/tests/fake_graphql.py @@ -26,11 +26,28 @@ class FakePageInfo(ObjectType): return False +class FakeMatchingRef(ObjectType): + name = String() + + def resolve_name(parent, info): + return parent + + +class FakeMatchingRefs(ObjectType): + nodes = List(FakeMatchingRef) + + def resolve_nodes(parent, info): + # To simplify tests just return the pattern and a bogus ref that should + # not disturb zuul. + return [parent.pattern, 'bogus-ref'] + + class FakeBranchProtectionRule(ObjectType): pattern = String() requiredStatusCheckContexts = List(String) requiresApprovingReviews = Boolean() requiresCodeOwnerReviews = Boolean() + matchingRefs = Field(FakeMatchingRefs, first=Int()) def resolve_pattern(parent, info): return parent.pattern @@ -44,6 +61,9 @@ class FakeBranchProtectionRule(ObjectType): def resolve_requiresCodeOwnerReviews(parent, info): return parent.require_codeowners_review + def resolve_matchingRefs(parent, info, first=None): + return parent + class FakeBranchProtectionRules(ObjectType): nodes = List(FakeBranchProtectionRule) diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py index df4c013d4..4290f7337 100644 --- a/zuul/driver/github/githubconnection.py +++ b/zuul/driver/github/githubconnection.py @@ -1635,7 +1635,8 @@ class GithubConnection(BaseConnection): # For performance reasons fetch all needed data for canMerge upfront # using a single graphql call. - canmerge_data = self.graphql_client.fetch_canmerge(github, change) + canmerge_data = self.graphql_client.fetch_canmerge( + github, change, zuul_event_id=event) # If the PR is a draft it cannot be merged. if canmerge_data.get('isDraft', False): diff --git a/zuul/driver/github/graphql/__init__.py b/zuul/driver/github/graphql/__init__.py index 191c8e1e7..1704d7efc 100644 --- a/zuul/driver/github/graphql/__init__.py +++ b/zuul/driver/github/graphql/__init__.py @@ -13,9 +13,10 @@ # under the License. import logging -import pathspec from pkg_resources import resource_string +from zuul.lib.logutil import get_annotated_logger + def nested_get(d, *keys, default=None): temp = d @@ -70,7 +71,8 @@ class GraphQLClient: response = github.session.post(self.url, json=query) return response.json() - def fetch_canmerge(self, github, change): + def fetch_canmerge(self, github, change, zuul_event_id=None): + log = get_annotated_logger(self.log, zuul_event_id) owner, repo = change.project.name.split('/') data = self._fetch_canmerge(github, owner, repo, change.number, @@ -81,14 +83,21 @@ class GraphQLClient: # Find corresponding rule to our branch rules = nested_get(repository, 'branchProtectionRules', 'nodes', default=[]) - matching_rule = None - for rule in rules: - pattern = pathspec.patterns.GitWildMatchPattern( - rule.get('pattern')) - match = pathspec.match_files([pattern], [change.branch]) - if match: - matching_rule = rule - break + + # Filter branch protection rules for the one matching the change. + matching_rules = [ + rule for rule in rules + for ref in nested_get(rule, 'matchingRefs', 'nodes', default=[]) + if ref.get('name') == change.branch + ] + if len(matching_rules) > 1: + log.warn('More than one branch protection rules match change %s', + change) + return result + elif len(matching_rules) == 1: + matching_rule = matching_rules[0] + else: + matching_rule = None # If there is a matching rule, get required status checks if matching_rule: diff --git a/zuul/driver/github/graphql/canmerge-legacy.graphql b/zuul/driver/github/graphql/canmerge-legacy.graphql index 4bddd73dc..edc1b0fcd 100644 --- a/zuul/driver/github/graphql/canmerge-legacy.graphql +++ b/zuul/driver/github/graphql/canmerge-legacy.graphql @@ -11,6 +11,11 @@ query canMergeData( requiredStatusCheckContexts requiresApprovingReviews requiresCodeOwnerReviews + matchingRefs(first: 100) { + nodes { + name + } + } } } pullRequest(number: $pull) { diff --git a/zuul/driver/github/graphql/canmerge.graphql b/zuul/driver/github/graphql/canmerge.graphql index ee3cd8fd3..baa4a9f81 100644 --- a/zuul/driver/github/graphql/canmerge.graphql +++ b/zuul/driver/github/graphql/canmerge.graphql @@ -11,6 +11,11 @@ query canMergeData( requiredStatusCheckContexts requiresApprovingReviews requiresCodeOwnerReviews + matchingRefs(first: 100) { + nodes { + name + } + } } } pullRequest(number: $pull) { |