Skip to content

Commit

Permalink
ci: force /lgtm v2-freeze on any v2 API changes. (envoyproxy#11092)
Browse files Browse the repository at this point in the history
To assist the API shepherds in ensuring that no unintentional v2 freezes creep in, this PR extends
our forked ownerscheck.star to force a "/lgtm v2-freeze" to be issued in order for v2 API changes to
merge.

The changes made to ownerscheck.star are:

* Replace path prefix matching with regex matching.
* Allow global approvers to be opted out of; we don't want a PR "approve" stamp to allow merges without an explicit v2 related LGTM.
* Support custom GitHub status labels for each spec.

Risk level: Low (CI only)
Testing: Manual interactions with RK in envoyproxy#11092

Signed-off-by: Harvey Tuch <[email protected]>
  • Loading branch information
htuch authored May 8, 2020
1 parent 6aaad26 commit 9eeaa46
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 15 deletions.
45 changes: 30 additions & 15 deletions ci/repokitteh/modules/ownerscheck.star
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,28 @@
# "owner": "envoyproxy/api-shepherds!",
# "path": "api/",
# "label": "api",
# "allow_global_approval": True,
# "github_status_label" = "any API change",
# },
# ],
# )
#
# This module will maintain a commit status per specified path (also aka as spec).
# This module will maintain a commit status per specified path regex (also aka as spec).
#
# Two types of approvals:
# 1. Global approvals, done by approving the PR using Github's review approval feature.
# 2. Partial approval, done by commenting "/lgtm [label]" where label is the label
# associated with the path. This does not affect GitHub's PR approve status, only
# this module's maintained commit status. This approval is automatically revoked
# if any further changes are done to the relevant files in this spec.
#
# By default, 'allow_global_approval' is true and either (1) or (2) above can unblock
# merges. If 'allow_global_approval' is set false, then only (2) will unblock a merge.
#
# 'label' refers to a GitHub label applied to any matching PR. The GitHub check status
# can be customized with `github_status_label`.

load("text", "match")
load("github.com/repokitteh/modules/lib/utils.star", "react")

def _store_partial_approval(who, files):
Expand All @@ -44,11 +53,16 @@ def _get_relevant_specs(specs, changed_files):
relevant = []

for spec in specs:
prefix = spec["path"]
path_match = spec["path"]

files = [f for f in changed_files if f['filename'].startswith(prefix)]
files = [f for f in changed_files if match(path_match, f['filename'])]
allow_global_approval = spec.get("allow_global_approval", True)
status_label = spec.get("github_status_label", "")
if files:
relevant.append(struct(files=files, prefix=prefix, **spec))
relevant.append(struct(files=files,
path_match=path_match,
allow_global_approval=allow_global_approval,
status_label=status_label))

print("specs: %s" % relevant)

Expand Down Expand Up @@ -81,7 +95,7 @@ def _is_approved(spec, approvers):
print("team %s(%d) = %s" % (team_name, team_id, required))

for r in required:
if any([a for a in approvers if a == r]):
if spec.allow_global_approval and any([a for a in approvers if a == r]):
print("global approver: %s" % r)
return True

Expand All @@ -92,11 +106,12 @@ def _is_approved(spec, approvers):
return False


def _update_status(owner, prefix, approved):
def _update_status(owner, status_label, path_match, approved):
changes_to = path_match or '/'
github.create_status(
state=approved and 'success' or 'pending',
context='%s must approve' % owner,
description='changes to %s' % (prefix or '/'),
context='%s must approve for %s' % (owner, status_label),
description='changes to %s' % changes_to,
)

def _get_specs(config):
Expand All @@ -122,7 +137,7 @@ def _reconcile(config, specs=None):
results.append((spec, approved))

if spec.owner[-1] == '!':
_update_status(spec.owner[:-1], spec.prefix, approved)
_update_status(spec.owner[:-1], spec.status_label, spec.path_match, approved)

if hasattr(spec, 'label'):
if approved:
Expand Down Expand Up @@ -150,23 +165,23 @@ def _comment(config, results, force=False):
if mention[-1] == '!':
mention = mention[:-1]

prefix = spec.prefix
if prefix:
prefix = ' for changes made to `' + prefix + '`'
match_description = spec.path_match
if match_description:
match_description = ' for changes made to `' + match_description + '`'

mode = spec.owner[-1] == '!' and 'approval' or 'fyi'

key = "ownerscheck/%s/%s" % (spec.owner, spec.prefix)
key = "ownerscheck/%s/%s" % (spec.owner, spec.path_match)

if (not force) and (store_get(key) == mode):
mode = 'skip'
else:
store_put(key, mode)

if mode == 'approval':
lines.append('CC %s: Your approval is needed%s.' % (mention, prefix))
lines.append('CC %s: Your approval is needed%s.' % (mention, match_description))
elif mode == 'fyi':
lines.append('CC %s: FYI only%s.' % (mention, prefix))
lines.append('CC %s: FYI only%s.' % (mention, match_description))

if lines:
github.issue_create_comment('\n'.join(lines))
Expand Down
9 changes: 9 additions & 0 deletions repokitteh.star
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,19 @@ use("github.com/repokitteh/modules/circleci.star", secret_token=get_secret('circ
use(
"github.com/envoyproxy/envoy/ci/repokitteh/modules/ownerscheck.star",
paths=[
{
"owner": "envoyproxy/api-shepherds!",
"path":
"(api/envoy[\w/]*/(v1alpha\d?|v1|v2alpha\d?|v2))|(api/envoy/type/(matcher/)?\w+.proto)",
"label": "v2-freeze",
"allow_global_approval": False,
"github_status_label": "v2 freeze violations",
},
{
"owner": "envoyproxy/api-shepherds!",
"path": "api/",
"label": "api",
"github_status_label": "any API change",
},
{
"owner": "envoyproxy/api-watchers",
Expand Down

0 comments on commit 9eeaa46

Please sign in to comment.