Commit 88637d42 authored by Peter Hutterer's avatar Peter Hutterer
Browse files

ci-fairy: add a command to check merge requests

Right now, the only supported feature is a check for the allow_collaboration
option. It's annoying to have to remind users to check that checkbox and
gitlab does not provide an option to enable this by default.
https://gitlab.com/gitlab-org/gitlab/-/issues/23308



Let's work around this by providing a simple command that can check for this
and fails. It can optionally write a junit.xml file which can be collected by
a CI template to provide a nice-looking error report.
Signed-off-by: Peter Hutterer's avatarPeter Hutterer <peter.hutterer@who-t.net>
parent 1df78c3b
......@@ -113,6 +113,24 @@ sanity check:
- exit 1
check merge request:
extends: .pip_install
script:
- pip3 install .
- ci-fairy check-merge-request --require-allow-collaboration --junit-xml=check-merge-request.xml
artifacts:
expire_in: 1 week
when: on_failure
paths:
- check-merge-request.xml
reports:
junit: check-merge-request.xml
variables:
FDO_UPSTREAM_REPO: freedesktop/ci-templates
# We allow this to fail because no MR may have been filed yet
allow_failure: true
check commits:
extends: .pip_install
script:
......
......@@ -230,3 +230,95 @@ variable to be set to the upstream project's full name. An example
otherwise it defaults to the ``master`` branch of the ``$FDO_UPSTREAM_REPO``.
For the above snippet, the commit range checked will thus be
``upstream/master..HEAD``.
Checking merge requests
-----------------------
``ci-fairy`` can be used to check merge requests for common requirements: ::
$ ci-fairy check-merge-request --require-allow-collaboration
The ``--require-allow-collaboration`` flag checks for the merge request to
`allow edits from maintainers
<https://docs.gitlab.com/ce/user/project/merge_requests/allow_collaboration.html>`__.
This can be used in the CI to fail a merge request pipeline if that checkbox
is not set. There are two ways to invoke this through the CI: either through
a `detached merge pipeline
<https://docs.gitlab.com/ce/ci/merge_request_pipelines/index.html>`__ or by
searching for a merge request with the same commit sha as our branch.
The detached merge pipeline has side-effects you should familiarize yourself
with.
Checking merge requests within a pipeline
.........................................
Here is an example for utilising ``FDO_UPSTREAM_REPO``:
.. code-block:: yaml
variables:
FDO_UPSTREAM_REPO: 'project/name'
check-merge-request:
image: golang:alpine
before_script:
- apk add python3 git
- pip3 install git+http://gitlab.freedesktop.org/freedesktop/ci-templates
script:
- ci-fairy check-merge-request --require-allow-collaboration --junit-xml=check-merge-request.xml
artifacts:
when: on_failure
reports:
junit: check-merge-request.xml
When pushing to a branch, this snippet will search ``project/name`` for an
**open** merge requests that has the same git sha as this pipeline. That
merge request is the one the checks are performed on. If no suitable merge
request is found, ``ci-fairy``'s exits code is 2. Otherwise, ``ci-fairy``
exits with status 0 on success or 1 in case of failure.
The advantag of this approach is that the job is run as part of the normal
CI pipeline and no detached merge pipeline needs to be run.
The disadvantages of this approach is that the pipeline may complete before a
merge request is filed and the job may fail despite the (future) merge
request having the checkbox set.
Checking merge requests within a detached pipeline
..................................................
Here is an example for utilizing detached merge pipelines:
.. code-block:: yaml
# work around the detached merge request pipeline issues by defining
# rules for all jobs
workflow:
rules:
- when: always
check-merge-request:
image: golang:alpine
before_script:
- apk add python3 git
- pip3 install git+http://gitlab.freedesktop.org/freedesktop/ci-templates
script:
- ci-fairy check-merge-request --require-allow-collaboration --junit-xml=check-merge-request.xml
rules:
- if: '$CI_MERGE_REQUEST_TARGET_BRANCH_NAME == "master"'
when: always
- when: never # override the workflow's always rule
artifacts:
when: on_failure
reports:
junit: check-merge-request.xml
Note that defining ``rules:`` for a single job has the side-effect of
running `detached merge pipelines
<https://docs.gitlab.com/ce/ci/merge_request_pipelines/index.html>`__. This
is almost never what you really want. To work around this, the above example
defines global rules in ``workflow``, effectively providing a ``rules``
definition for all jobs. Ensure that this is compatible with your project's
CI.
......@@ -57,6 +57,24 @@ sanity check:
- exit 1
check merge request:
extends: .pip_install
script:
- pip3 install .
- ci-fairy check-merge-request --require-allow-collaboration --junit-xml=check-merge-request.xml
artifacts:
expire_in: 1 week
when: on_failure
paths:
- check-merge-request.xml
reports:
junit: check-merge-request.xml
variables:
FDO_UPSTREAM_REPO: freedesktop/ci-templates
# We allow this to fail because no MR may have been filed yet
allow_failure: true
check commits:
extends: .pip_install
script:
......
......@@ -10,17 +10,24 @@ import fnmatch
import logging
import os
import sys
import urllib.parse
import yaml
from pathlib import Path
def gitlab_project(gitlab, project=None):
def gitlab_project(gitlab, project=None, fallback='__default__'):
'''
Returns the Gitlab Project object for the given project name. Where name
is None, it is taken from the CI environment.
'''
if project is None:
project_id = os.getenv('CI_PROJECT_ID')
# we can't default fallback to os.getenv() without breaking the
# tests (which rely on putenv), so let's use a special value here.
if fallback == '__default__':
fallback = os.getenv('CI_PROJECT_ID')
if fallback is None:
return None
project_id = urllib.parse.quote(fallback, safe='')
if project_id is None:
return None
p = gitlab.projects.get(project_id)
......@@ -516,6 +523,114 @@ def check_commits(ctx, commit_range, branch, signed_off_by, textwidth, junit_xml
sys.exit(1 if failures else 0)
@ci_fairy.command()
@click.option('--project', help='Project the merge request is filed against, e.g. freedesktop/ci-templates')
@click.option('--merge-request-iid', help='The merge request IID to check')
@click.option('--junit-xml', help='junit output file to write to')
@click.option('--require-allow-collaboration', is_flag=True, help='Check that allow_collaboration is set')
@click.pass_context
def check_merge_request(ctx, project, merge_request_iid, junit_xml, require_allow_collaboration):
'''
Checks the given merge request in the project for various settings.
Currently supported:
--require-allow-calibration: checks that the ``allow_collaboration``
boolean flag is set (the one that allows maintainers to rebase an
MR).
If the requirements are not set, this commands exit with status code 1.
If a junit output is given, this command writes a junit XML file with an
appropriate error message.
This command defaults to CI_MERGE_REQUEST_PROJECT_ID and CI_MERGE_REQUEST_IID,
see https://docs.gitlab.com/ce/ci/merge_request_pipelines/index.html
Where not present, it uses FDO_UPSTREAM_REPO to find the upstream
repository and finds the merge request associated with this commit. This
relies on CI_COMMIT_SHA.
This command exits with status 2 if no suitable merge request could be
found.
'''
fallback = None
if not project:
fallback = \
os.getenv('CI_MERGE_REQUEST_PROJECT_ID') or \
os.getenv('FDO_UPSTREAM_REPO') or \
os.getenv('CI_PROJECT_ID')
p = gitlab_project(ctx.obj.gitlab, project, fallback=fallback)
if not p:
logger.error('Unable to find or identify project')
sys.exit(1)
if not require_allow_collaboration:
logger.error('At least one check must be specified')
sys.exit(1)
failures = []
exit_status = 0
# First check: CI_MERGE_REQUEST_IID. If we have this one we can use it
# directly but since that requires a detached pipeline (and rules: in
# the gitlab-ci.yml file) most projects won't bother, so...
if merge_request_iid is None:
merge_request_iid = os.getenv('CI_MERGE_REQUEST_IID')
if merge_request_iid is not None:
mr = p.mergerequests.get(merge_request_iid)
if not mr:
logger.error(f'Merge request IID {merge_request_iid} does not exist')
sys.exit(1)
# ... the fallback is to check FDO_UPSTREAM_REPO for all open merge requests
# and compare their sha with the sha of our current pipeline (or HEAD if
# run locally). But a branch may not have an MR, so we exit with a
# different exit code in that case.
else:
logger.debug('This is not a merge pipeline, searching for MR')
sha = os.getenv('CI_COMMIT_SHA')
if not sha:
try:
repo = git.Repo('.')
sha = repo.commit('HEAD').hexsha
except git.exc.InvalidGitRepositoryError:
pass
if not sha:
logger.error('Cannot find git sha, unable to search for merge request')
mr = next(iter([m for m in p.mergerequests.list(state='opened', per_page=100) if m.sha == sha]), None)
if not mr:
# Not having a merge request *may* be fine so let's use a
# special exit code.
exit_status = 2
tcaseid = 'mr_is_filed'
tcasename = 'Check a merge request is filed'
message = f'No open merge request against {p.path_with_namespace} with sha {sha}'
failures.append((tcaseid, tcasename, message))
logger.error(message)
if exit_status == 0:
if require_allow_collaboration:
if not mr.allow_collaboration:
exit_status = 1
tcaseid = 'allow_collaboration'
tcasename = 'Check allow_collaboration checkbox is set'
message = '''
Error: This merge request does not allow edits from maintainers.
Please edit the merge request and set the checkbox to
"Allow commits from members who can merge to the target branch"
See https://docs.gitlab.com/ce/user/project/merge_requests/allow_collaboration.html'''
failures.append((tcaseid, tcasename, message))
logger.error(message)
if failures:
if junit_xml is not None:
suite_id = '{}.merge_request.{}'.format(p.name, merge_request_iid)
suite_name = 'Merge request check ({} !{})'.format(p.name, merge_request_iid)
make_junit_xml(junit_xml, suite_id, suite_name, failures)
sys.exit(exit_status)
def main():
ci_fairy()
......
......@@ -58,6 +58,24 @@ def mock_gitlab(gitlab):
project.id = GITLAB_TEST_PROJECT_ID
project.repositories.list = MagicMock(return_value=repos)
mrs = []
for i in range(4):
mr = MagicMock()
mr.state = 'opened' if i < 2 else 'merged'
mr.id = i
mr.sha = f'dead{i:08x}'
mr.allow_collaboration = (i % 2) == 0
mrs.append(mr)
def mrget(mid):
return [m for m in mrs if m.id == int(mid)][0]
def mrlist(state=None, per_page=None):
return [m for m in mrs if state == None or m.state == state]
project.mergerequests.list = MagicMock(side_effect=mrlist)
project.mergerequests.get = MagicMock(side_effect=mrget)
ctx = gitlab(GITLAB_TEST_URL)
ctx.projects.list = MagicMock(return_value=[project])
# This always returns our project, even where the ID doesn't match. Good
......@@ -94,27 +112,32 @@ def test_project_lookup(gitlab, caplog, gitlab_default_env, monkeypatch):
ctx.projects.list = MagicMock(side_effect=plist)
# default is CI_PROJECT_ID
p = ci_fairy.gitlab_project(ctx, None)
p = ci_fairy.gitlab_project(ctx)
assert p is not None
assert p.name == 'foo/foo'
assert p.id == 3
# exact match
p = ci_fairy.gitlab_project(ctx, 'user/foo')
assert p is not None
assert p.name == 'user/foo'
assert p.id == 1
# require exact match over substring match
p = ci_fairy.gitlab_project(ctx, 'foo')
assert p is not None
assert p.name == 'foo'
assert p.id == 0
# exact match
p = ci_fairy.gitlab_project(ctx, 'foobar')
assert p is not None
assert p.name == 'foobar'
assert p.id == 2
# exact match
p = ci_fairy.gitlab_project(ctx, 'foo/foo')
assert p is not None
assert p.name == 'foo/foo'
assert p.id == 3
......@@ -672,3 +695,58 @@ def test_commits_gitlabemail(caplog, gitlab_default_env):
result = runner.invoke(ci_fairy.ci_fairy, args)
assert result.exit_code == 1
assert 'git author email invalid' in ''.join(caplog.messages)
@patch('ci_fairy.Gitlab')
def test_merge_request_missing_arg(gitlab, caplog, gitlab_default_env):
args = ['check-merge-request']
runner = CliRunner(env=gitlab_default_env)
result = runner.invoke(ci_fairy.ci_fairy, args)
assert result.exit_code == 1
assert 'At least one check must be specified' in caplog.text
@patch('ci_fairy.Gitlab')
def test_merge_request_detached_pipeline(gitlab, caplog, gitlab_default_env):
args = ['check-merge-request', '--require-allow-collaboration']
env = gitlab_default_env
env['CI_MERGE_REQUEST_PROJECT_ID'] = GITLAB_TEST_PROJECT_ID
env['CI_MERGE_REQUEST_IID'] = '1'
# mock_gitlab sets allow_collaboration off for every second MR
gitlab, project, _ = mock_gitlab(gitlab)
runner = CliRunner(env=env)
result = runner.invoke(ci_fairy.ci_fairy, args)
assert result.exit_code == 1
assert 'This merge request does not allow edits from maintainers' in caplog.text
env['CI_MERGE_REQUEST_IID'] = '0'
gitlab, project, _ = mock_gitlab(gitlab)
runner = CliRunner(env=env)
result = runner.invoke(ci_fairy.ci_fairy, args)
assert result.exit_code == 0
@patch('ci_fairy.Gitlab')
def test_merge_request_fdo_upstream_repo(gitlab, caplog, gitlab_default_env):
args = ['-vv', 'check-merge-request', '--require-allow-collaboration']
env = gitlab_default_env
env['FDO_UPSTREAM_REPO'] = GITLAB_TEST_PROJECT_ID
env['CI_PROJECT_ID'] = '9999'
env['CI_COMMIT_SHA'] = 'dead00000001'
# mock_gitlab sets allow_collaboration off for every second MR
gitlab, project, _ = mock_gitlab(gitlab)
runner = CliRunner(env=env)
result = runner.invoke(ci_fairy.ci_fairy, args)
assert result.exit_code == 1
assert 'This merge request does not allow edits from maintainers' in caplog.text
env['CI_COMMIT_SHA'] = 'dead00000000'
gitlab, project, _ = mock_gitlab(gitlab)
runner = CliRunner(env=env)
result = runner.invoke(ci_fairy.ci_fairy, args)
assert result.exit_code == 0
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment