[ADD] *: per-repository webhook secret

Currently webhook secrets are configured per *project* which is an
issue both because different repositories may have different
administrators and thus creates safety concerns, and because multiple
repositories can feed into different projects (e.g. on mergebot,
odoo-dev/odoo is both an ancillary repository to the main RD project,
and the main repository to the minor / legacy master-wowl
project). This means it can be necessary to have multiple projects
share the same secret as well, this then mandates the secret for more
repositories per (1).

This is a pain in the ass, so just detach secrets from projects and
link them *only* to repositories, it's cleaner and easier to manage
and set up progressively.

This requires a lot of changes to the tests, as they all need to
correctly configure the signaling.

For `runbot_merge` there was *some* setup sharing already via the
module-level `repo` fixtures`, those were merged into a conftest-level
fixture which could handle the signaling setup. A few tests which
unnecessarily set up repositories ad-hoc were also moved to the
fixture. But for most of the ad-hoc setup in `runbot_merge`, as well
as `forwardport` where it's all ad-hoc, events sources setup was just
appended as is. This should probably be cleaned up at one point, with
the various requirements collected and organised into a small set of
fixtures doing the job more uniformly.

Fixes #887
This commit is contained in:
Xavier Morel 2024-06-06 11:07:57 +02:00
parent cc9f239261
commit fec3d39d19
25 changed files with 154 additions and 153 deletions

View File

@ -452,6 +452,10 @@ def check(response):
# to) break the existing local tests # to) break the existing local tests
@pytest.fixture @pytest.fixture
def make_repo(capsys, request, config, tunnel, users): def make_repo(capsys, request, config, tunnel, users):
"""Fixtures which creates a repository on the github side, plugs webhooks
in, and registers the repository for deletion on cleanup (unless
``--no-delete`` is set)
"""
owner = config['github']['owner'] owner = config['github']['owner']
github = requests.Session() github = requests.Session()
github.headers['Authorization'] = 'token %s' % config['github']['token'] github.headers['Authorization'] = 'token %s' % config['github']['token']

View File

@ -421,6 +421,7 @@ def post_merge(env, config, users, make_repo, branches):
}) })
] ]
}) })
env['runbot_merge.events_sources'].create({'repository': prod.name})
env['res.partner'].search([ env['res.partner'].search([
('github_login', '=', config['role_reviewer']['user']) ('github_login', '=', config['role_reviewer']['user'])

View File

@ -35,7 +35,6 @@ def test_straightforward_flow(env, config, make_repo, users):
other_user = config['role_other'] other_user = config['role_other']
other_user_repo = prod.fork(token=other_user['token']) other_user_repo = prod.fork(token=other_user['token'])
project = env['runbot_merge.project'].search([])
b_head = prod.commit('b') b_head = prod.commit('b')
c_head = prod.commit('c') c_head = prod.commit('c')
with prod, other_user_repo: with prod, other_user_repo:

View File

@ -326,6 +326,7 @@ def test_duplicate_fw(env, make_repo, setreviewers, config, users):
] ]
}) })
setreviewers(*proj.repo_ids) setreviewers(*proj.repo_ids)
env['runbot_merge.events_sources'].create({'repository': repo.name})
# create a PR in v1, merge it, then create all 3 ports # create a PR in v1, merge it, then create all 3 ports
with repo: with repo:

View File

@ -206,6 +206,7 @@ class TestNotAllBranches:
'branch_filter': '[("name", "in", ["a", "c"])]', 'branch_filter': '[("name", "in", ["a", "c"])]',
}) })
setreviewers(repo_a, repo_b) setreviewers(repo_a, repo_b)
env['runbot_merge.events_sources'].create([{'repository': a.name}, {'repository': b.name}])
return project, a, a_dev, b, b_dev return project, a, a_dev, b, b_dev
def test_single_first(self, env, repos, config): def test_single_first(self, env, repos, config):
@ -883,6 +884,7 @@ def test_disable_branch_with_batches(env, config, make_repo, users):
"required_statuses": "default", "required_statuses": "default",
"fp_remote_target": fork2.name, "fp_remote_target": fork2.name,
}) })
env['runbot_merge.events_sources'].create({'repository': repo2.name})
env['res.partner'].search([ env['res.partner'].search([
('github_login', '=', config['role_reviewer']['user']) ('github_login', '=', config['role_reviewer']['user'])
]).write({ ]).write({
@ -978,6 +980,7 @@ def test_disable_multitudes(env, config, make_repo, users, setreviewers):
})], })],
}) })
setreviewers(project.repo_ids) setreviewers(project.repo_ids)
env['runbot_merge.events_sources'].create({'repository': repo.name})
with repo: with repo:
[a, b, c, d] = repo.make_commits( [a, b, c, d] = repo.make_commits(

View File

@ -130,6 +130,7 @@ def make_basic(
}) })
prod = make_repo(reponame) prod = make_repo(reponame)
env['runbot_merge.events_sources'].create({'repository': prod.name})
with prod: with prod:
a_0, a_1, a_2, a_3, a_4, = prod.make_commits( a_0, a_1, a_2, a_3, a_4, = prod.make_commits(
None, None,

View File

@ -1,6 +1,6 @@
{ {
'name': 'merge bot', 'name': 'merge bot',
'version': '1.13', 'version': '1.14',
'depends': ['contacts', 'mail', 'website'], 'depends': ['contacts', 'mail', 'website'],
'data': [ 'data': [
'security/security.xml', 'security/security.xml',

View File

@ -86,18 +86,18 @@ class MergebotController(Controller):
github._gh.info(self._format(req)) github._gh.info(self._format(req))
c = EVENTS.get(event) repo = request.jsonrequest.get('repository', {}).get('full_name')
if not c:
_logger.warning('Unknown event %s', event)
return 'Unknown event {}'.format(event)
repo = request.jsonrequest['repository']['full_name']
env = request.env(user=1) env = request.env(user=1)
secret = env['runbot_merge.repository'].search([ source = repo and env['runbot_merge.events_sources'].search([('repository', '=', repo)])
('name', '=', repo), if not source:
]).project_id.secret _logger.warning(
if secret and secret.strip(): "Ignored hook %s to unknown source repository %s",
req.headers.get("X-Github-Delivery"),
repo,
)
return werkzeug.exceptions.Forbidden()
elif secret := source.secret:
signature = 'sha256=' + hmac.new(secret.strip().encode(), req.get_data(), hashlib.sha256).hexdigest() signature = 'sha256=' + hmac.new(secret.strip().encode(), req.get_data(), hashlib.sha256).hexdigest()
if not hmac.compare_digest(signature, req.headers.get('X-Hub-Signature-256', '')): if not hmac.compare_digest(signature, req.headers.get('X-Hub-Signature-256', '')):
_logger.warning( _logger.warning(
@ -114,6 +114,11 @@ class MergebotController(Controller):
else: else:
_logger.info("No secret or signature for %s", repo) _logger.info("No secret or signature for %s", repo)
c = EVENTS.get(event)
if not c:
_logger.warning('Unknown event %s', event)
return 'Unknown event {}'.format(event)
sentry_sdk.set_context('webhook', request.jsonrequest) sentry_sdk.set_context('webhook', request.jsonrequest)
return c(env, request.jsonrequest) return c(env, request.jsonrequest)

View File

@ -0,0 +1,12 @@
def migrate(cr, version):
cr.execute("""
CREATE TABLE runbot_merge_events_sources (
id serial primary key,
repository varchar not null,
secret varchar
);
INSERT INTO runbot_merge_events_sources (repository, secret)
SELECT r.name, p.secret
FROM runbot_merge_repository r
JOIN runbot_merge_project p ON p.id = r.project_id;
""")

View File

@ -6,4 +6,5 @@ from . import batch
from . import project_freeze from . import project_freeze
from . import stagings_create from . import stagings_create
from . import staging_cancel from . import staging_cancel
from . import events_sources
from . import crons from . import crons

View File

@ -0,0 +1,12 @@
from odoo import models, fields
class EventsSources(models.Model):
_name = 'runbot_merge.events_sources'
_description = 'Valid Webhook Event Sources'
_order = "repository"
_rec_name = "repository"
# FIXME: unique repo? Or allow multiple secrets per repo?
repository = fields.Char(index=True, required=True)
secret = fields.Char()

View File

@ -55,12 +55,6 @@ class Project(models.Model):
batch_limit = fields.Integer( batch_limit = fields.Integer(
default=8, group_operator=None, help="Maximum number of PRs staged together") default=8, group_operator=None, help="Maximum number of PRs staged together")
secret = fields.Char(
help="Webhook secret. If set, will be checked against the signature "
"of (valid) incoming webhook signatures, failing signatures "
"will lead to webhook rejection. Should only use ASCII."
)
freeze_id = fields.Many2one('runbot_merge.project.freeze', compute='_compute_freeze') freeze_id = fields.Many2one('runbot_merge.project.freeze', compute='_compute_freeze')
freeze_reminder = fields.Text() freeze_reminder = fields.Text()

View File

@ -21,6 +21,7 @@ access_runbot_merge_fetch_job_admin,Admin access to fetch jobs,model_runbot_merg
access_runbot_merge_pull_requests_feedback_admin,Admin access to feedback,model_runbot_merge_pull_requests_feedback,runbot_merge.group_admin,1,1,1,1 access_runbot_merge_pull_requests_feedback_admin,Admin access to feedback,model_runbot_merge_pull_requests_feedback,runbot_merge.group_admin,1,1,1,1
access_runbot_merge_review_rights,Admin access to review permissions,model_res_partner_review,runbot_merge.group_admin,1,1,1,1 access_runbot_merge_review_rights,Admin access to review permissions,model_res_partner_review,runbot_merge.group_admin,1,1,1,1
access_runbot_merge_review_override,Admin access to override permissions,model_res_partner_override,runbot_merge.group_admin,1,1,1,1 access_runbot_merge_review_override,Admin access to override permissions,model_res_partner_override,runbot_merge.group_admin,1,1,1,1
access_runbot_merge_events_sources,Admin access to event sources,model_runbot_merge_events_sources,runbot_merge.group_admin,1,1,1,1
access_runbot_merge_project,User access to project,model_runbot_merge_project,base.group_user,1,0,0,0 access_runbot_merge_project,User access to project,model_runbot_merge_project,base.group_user,1,0,0,0
access_runbot_merge_repository,User access to repo,model_runbot_merge_repository,base.group_user,1,0,0,0 access_runbot_merge_repository,User access to repo,model_runbot_merge_repository,base.group_user,1,0,0,0
access_runbot_merge_branch,User access to branches,model_runbot_merge_branch,base.group_user,1,0,0,0 access_runbot_merge_branch,User access to branches,model_runbot_merge_branch,base.group_user,1,0,0,0
@ -29,3 +30,4 @@ access_runbot_merge_pull_requests_feedback,Users have no reason to access feedba
access_runbot_merge_review_rights_2,Users can see partners,model_res_partner_review,base.group_user,1,0,0,0 access_runbot_merge_review_rights_2,Users can see partners,model_res_partner_review,base.group_user,1,0,0,0
access_runbot_merge_review_override_2,Users can see partners,model_res_partner_override,base.group_user,1,0,0,0 access_runbot_merge_review_override_2,Users can see partners,model_res_partner_override,base.group_user,1,0,0,0
runbot_merge.access_runbot_merge_pull_requests_feedback_template,access_runbot_merge_pull_requests_feedback_template,runbot_merge.model_runbot_merge_pull_requests_feedback_template,base.group_system,1,1,0,0 runbot_merge.access_runbot_merge_pull_requests_feedback_template,access_runbot_merge_pull_requests_feedback_template,runbot_merge.model_runbot_merge_pull_requests_feedback_template,base.group_system,1,1,0,0

1 id name model_id:id group_id:id perm_read perm_write perm_create perm_unlink
21 access_runbot_merge_pull_requests_feedback_admin Admin access to feedback model_runbot_merge_pull_requests_feedback runbot_merge.group_admin 1 1 1 1
22 access_runbot_merge_review_rights Admin access to review permissions model_res_partner_review runbot_merge.group_admin 1 1 1 1
23 access_runbot_merge_review_override Admin access to override permissions model_res_partner_override runbot_merge.group_admin 1 1 1 1
24 access_runbot_merge_events_sources Admin access to event sources model_runbot_merge_events_sources runbot_merge.group_admin 1 1 1 1
25 access_runbot_merge_project User access to project model_runbot_merge_project base.group_user 1 0 0 0
26 access_runbot_merge_repository User access to repo model_runbot_merge_repository base.group_user 1 0 0 0
27 access_runbot_merge_branch User access to branches model_runbot_merge_branch base.group_user 1 0 0 0
30 access_runbot_merge_review_rights_2 Users can see partners model_res_partner_review base.group_user 1 0 0 0
31 access_runbot_merge_review_override_2 Users can see partners model_res_partner_override base.group_user 1 0 0 0
32 runbot_merge.access_runbot_merge_pull_requests_feedback_template access_runbot_merge_pull_requests_feedback_template runbot_merge.model_runbot_merge_pull_requests_feedback_template base.group_system 1 1 0 0
33

View File

@ -39,3 +39,30 @@ def project(env, config):
'github_prefix': 'hansen', 'github_prefix': 'hansen',
'branch_ids': [(0, 0, {'name': 'master'})], 'branch_ids': [(0, 0, {'name': 'master'})],
}) })
@pytest.fixture
def make_repo2(env, project, make_repo, users, setreviewers):
"""Layer over ``make_repo`` which also:
- adds the new repo to ``project`` (with no group and the ``'default'`` status required)
- sets the standard reviewers on the repo
- and creates an event source for the repo
"""
def mr(name):
r = make_repo(name)
rr = env['runbot_merge.repository'].create({
'project_id': project.id,
'name': r.name,
'group_id': False,
'required_statuses': 'default',
})
setreviewers(rr)
env['runbot_merge.events_sources'].create({'repository': r.name})
return r
return mr
@pytest.fixture
def repo(make_repo2):
return make_repo2('repo')

View File

@ -12,17 +12,9 @@ from lxml import html
import odoo import odoo
from utils import _simple_init, seen, matches, get_partner, Commit, pr_page, to_pr, part_of, ensure_one from utils import _simple_init, seen, matches, get_partner, Commit, pr_page, to_pr, part_of, ensure_one
@pytest.fixture(autouse=True)
@pytest.fixture def _configure_statuses(project, repo):
def repo(env, project, make_repo, users, setreviewers): project.repo_ids.required_statuses = 'legal/cla,ci/runbot'
r = make_repo('repo')
project.write({'repo_ids': [(0, 0, {
'name': r.name,
'group_id': False,
'required_statuses': 'legal/cla,ci/runbot'
})]})
setreviewers(*project.repo_ids)
return r
@pytest.fixture(autouse=True, params=["statuses", "rpc"]) @pytest.fixture(autouse=True, params=["statuses", "rpc"])
def stagings(request, env, project, repo): def stagings(request, env, project, repo):
@ -353,11 +345,15 @@ Co-authored-by: Bob <bob@example.com>""".format(
) )
class TestWebhookSecurity: class TestWebhookSecurity:
@pytest.fixture(autouse=True)
def add_secret_to_source(self, env, repo):
env['runbot_merge.events_sources'].search([
('repository', '=', repo.name),
]).secret = "a secret"
def test_no_secret(self, env, project, repo): def test_no_secret(self, env, project, repo):
""" Test 1: didn't add a secret to the repo, should be ignored """ Test 1: didn't add a secret to the repo, should be ignored
""" """
project.secret = "a secret"
with repo: with repo:
m = repo.make_commit(None, "initial", None, tree={'a': 'some content'}) m = repo.make_commit(None, "initial", None, tree={'a': 'some content'})
repo.make_ref('heads/master', m) repo.make_ref('heads/master', m)
@ -371,7 +367,6 @@ class TestWebhookSecurity:
]) ])
def test_wrong_secret(self, env, project, repo): def test_wrong_secret(self, env, project, repo):
project.secret = "a secret"
with repo: with repo:
repo.set_secret("wrong secret") repo.set_secret("wrong secret")
@ -387,7 +382,6 @@ class TestWebhookSecurity:
]) ])
def test_correct_secret(self, env, project, repo): def test_correct_secret(self, env, project, repo):
project.secret = "a secret"
with repo: with repo:
repo.set_secret("a secret") repo.set_secret("a secret")
@ -2789,7 +2783,7 @@ class TestBatching(object):
def test_batching_pressing(self, env, repo, config): def test_batching_pressing(self, env, repo, config):
""" "Pressing" PRs should be selected before normal & batched together """ "Pressing" PRs should be selected before normal & batched together
""" """
# by limiting the batch size to 3 we allow both high-priority PRs, but # by limiting the batch size to 3 we allow both high-priority PRs, but
# a single normal priority one # a single normal priority one
env['runbot_merge.project'].search([]).batch_limit = 3 env['runbot_merge.project'].search([]).batch_limit = 3
with repo: with repo:

View File

@ -4,19 +4,10 @@ without wider relevance and thus other location.
from utils import Commit, to_pr from utils import Commit, to_pr
def test_close_single(env, project, make_repo, setreviewers): def test_close_single(env, repo):
"""If a batch has a single PR and that PR gets closed, the batch should be """If a batch has a single PR and that PR gets closed, the batch should be
inactive *and* blocked. inactive *and* blocked.
""" """
repo = make_repo('wheee')
r = env['runbot_merge.repository'].create({
'project_id': project.id,
'name': repo.name,
'required_statuses': 'default',
'group_id': False,
})
setreviewers(r)
with repo: with repo:
repo.make_commits(None, Commit("a", tree={"a": "a"}), ref='heads/master') repo.make_commits(None, Commit("a", tree={"a": "a"}), ref='heads/master')
[c] = repo.make_commits('master', Commit('b', tree={"b": "b"})) [c] = repo.make_commits('master', Commit('b', tree={"b": "b"}))
@ -41,25 +32,13 @@ def test_close_single(env, project, make_repo, setreviewers):
assert Batches.search_count([]) == 0 assert Batches.search_count([]) == 0
def test_close_multiple(env, project, make_repo, setreviewers): def test_close_multiple(env, make_repo2):
"""If a batch has a single PR and that PR gets closed, the batch should be """If a batch has a single PR and that PR gets closed, the batch should be
inactive *and* blocked. inactive *and* blocked.
""" """
Batches = env['runbot_merge.batch'] Batches = env['runbot_merge.batch']
repo1 = make_repo('wheee') repo1 = make_repo2('wheee')
repo2 = make_repo('wheeee') repo2 = make_repo2('wheeee')
project.write({
'repo_ids': [(0, 0, {
'name': repo1.name,
'required_statuses': 'default',
'group_id': False,
}), (0, 0, {
'name': repo2.name,
'required_statuses': 'default',
'group_id': False,
})]
})
setreviewers(*project.repo_ids)
with repo1: with repo1:
repo1.make_commits(None, Commit("a", tree={"a": "a"}), ref='heads/master') repo1.make_commits(None, Commit("a", tree={"a": "a"}), ref='heads/master')

View File

@ -2,28 +2,21 @@ import pytest
from utils import Commit from utils import Commit
@pytest.fixture @pytest.fixture
def repo(env, project, make_repo, users, setreviewers): def _setup_statuses(project, repo):
r = make_repo('repo') project.repo_ids.status_ids = [
project.write({ (5, 0, 0),
'repo_ids': [(0, 0, { (0, 0, {'context': 'ci'}),
'name': r.name, # require the lint status on master
'status_ids': [ (0, 0, {
(0, 0, {'context': 'ci'}), 'context': 'lint',
# require the lint status on master 'branch_filter': [('id', '=', project.branch_ids.id)]
(0, 0, { }),
'context': 'lint', (0, 0, {'context': 'pr', 'stagings': False}),
'branch_filter': [('id', '=', project.branch_ids.id)] (0, 0, {'context': 'staging', 'prs': False}),
}), ]
(0, 0, {'context': 'pr', 'stagings': False}),
(0, 0, {'context': 'staging', 'prs': False}),
]
})],
})
setreviewers(*project.repo_ids)
return r
@pytest.mark.usefixtures('_setup_statuses')
def test_status_applies(env, repo, config): def test_status_applies(env, repo, config):
""" If branches are associated with a repo status, only those branch should """ If branches are associated with a repo status, only those branch should
require the status on their PRs & stagings require the status on their PRs & stagings
@ -71,6 +64,7 @@ def test_status_applies(env, repo, config):
env.run_crons('runbot_merge.process_updated_commits') env.run_crons('runbot_merge.process_updated_commits')
assert st.state == 'success' assert st.state == 'success'
@pytest.mark.usefixtures('_setup_statuses')
def test_status_skipped(env, project, repo, config): def test_status_skipped(env, project, repo, config):
""" Branches not associated with a repo status should not require the status """ Branches not associated with a repo status should not require the status
on their PRs or stagings on their PRs or stagings
@ -132,6 +126,7 @@ def test_pseudo_version_tag(env, project, make_repo, setreviewers, config):
], ],
}) })
setreviewers(*project.repo_ids) setreviewers(*project.repo_ids)
env['runbot_merge.events_sources'].create({'repository': repo.name})
with repo: with repo:
[m] = repo.make_commits(None, Commit('c1', tree={'a': '1'}), ref='heads/master') [m] = repo.make_commits(None, Commit('c1', tree={'a': '1'}), ref='heads/master')

View File

@ -23,6 +23,7 @@ def test_existing_pr_disabled_branch(env, project, make_repo, setreviewers, conf
'group_id': False, 'group_id': False,
}) })
setreviewers(*project.repo_ids) setreviewers(*project.repo_ids)
env['runbot_merge.events_sources'].create({'repository': repo.name})
with repo: with repo:
[m] = repo.make_commits(None, Commit('root', tree={'a': '1'}), ref='heads/master') [m] = repo.make_commits(None, Commit('root', tree={'a': '1'}), ref='heads/master')
@ -107,6 +108,7 @@ def test_new_pr_no_branch(env, project, make_repo, setreviewers, users):
'status_ids': [(0, 0, {'context': 'status'})] 'status_ids': [(0, 0, {'context': 'status'})]
}) })
setreviewers(*project.repo_ids) setreviewers(*project.repo_ids)
env['runbot_merge.events_sources'].create({'repository': repo.name})
with repo: with repo:
[m] = repo.make_commits(None, Commit('root', tree={'a': '1'}), ref='heads/master') [m] = repo.make_commits(None, Commit('root', tree={'a': '1'}), ref='heads/master')
@ -140,6 +142,7 @@ def test_new_pr_disabled_branch(env, project, make_repo, setreviewers, users):
'active': False, 'active': False,
}) })
setreviewers(*project.repo_ids) setreviewers(*project.repo_ids)
env['runbot_merge.events_sources'].create({'repository': repo.name})
with repo: with repo:
[m] = repo.make_commits(None, Commit('root', tree={'a': '1'}), ref='heads/master') [m] = repo.make_commits(None, Commit('root', tree={'a': '1'}), ref='heads/master')

View File

@ -19,39 +19,42 @@ from utils import seen, get_partner, pr_page, to_pr, Commit
@pytest.fixture @pytest.fixture
def repo_a(project, make_repo, setreviewers): def repo_a(env, project, make_repo, setreviewers):
repo = make_repo('a') repo = make_repo('a')
r = project.env['runbot_merge.repository'].create({ r = env['runbot_merge.repository'].create({
'project_id': project.id, 'project_id': project.id,
'name': repo.name, 'name': repo.name,
'required_statuses': 'default', 'required_statuses': 'default',
'group_id': False, 'group_id': False,
}) })
setreviewers(r) setreviewers(r)
env['runbot_merge.events_sources'].create({'repository': r.name})
return repo return repo
@pytest.fixture @pytest.fixture
def repo_b(project, make_repo, setreviewers): def repo_b(env, project, make_repo, setreviewers):
repo = make_repo('b') repo = make_repo('b')
r = project.env['runbot_merge.repository'].create({ r = env['runbot_merge.repository'].create({
'project_id': project.id, 'project_id': project.id,
'name': repo.name, 'name': repo.name,
'required_statuses': 'default', 'required_statuses': 'default',
'group_id': False, 'group_id': False,
}) })
setreviewers(r) setreviewers(r)
env['runbot_merge.events_sources'].create({'repository': r.name})
return repo return repo
@pytest.fixture @pytest.fixture
def repo_c(project, make_repo, setreviewers): def repo_c(env, project, make_repo, setreviewers):
repo = make_repo('c') repo = make_repo('c')
r = project.env['runbot_merge.repository'].create({ r = env['runbot_merge.repository'].create({
'project_id': project.id, 'project_id': project.id,
'name': repo.name, 'name': repo.name,
'required_statuses': 'default', 'required_statuses': 'default',
'group_id': False, 'group_id': False,
}) })
setreviewers(r) setreviewers(r)
env['runbot_merge.events_sources'].create({'repository': r.name})
return repo return repo
def make_pr(repo, prefix, trees, *, target='master', user, def make_pr(repo, prefix, trees, *, target='master', user,
@ -934,6 +937,7 @@ class TestSubstitutions:
'repo_ids': [(0, 0, {'name': 'xxx/xxx'})], 'repo_ids': [(0, 0, {'name': 'xxx/xxx'})],
'branch_ids': [(0, 0, {'name': 'master'})] 'branch_ids': [(0, 0, {'name': 'master'})]
}) })
env['runbot_merge.events_sources'].create({'repository': 'xxx/xxx'})
r = p.repo_ids r = p.repo_ids
# replacement pattern, pr label, stored label # replacement pattern, pr label, stored label
cases = [ cases = [
@ -1001,8 +1005,7 @@ class TestSubstitutions:
with repo_a: with repo_a:
repo_a.make_commits('master', repo_a.Commit('bop', tree={'a': '1'}), ref='heads/abranch') repo_a.make_commits('master', repo_a.Commit('bop', tree={'a': '1'}), ref='heads/abranch')
pra = repo_a.make_pr(target='master', head='abranch') pra = repo_a.make_pr(target='master', head='abranch')
b_fork = repo_b.fork() with repo_b, repo_b.fork() as b_fork:
with b_fork, repo_b:
b_fork.make_commits('master', b_fork.Commit('pob', tree={'b': '1'}), ref='heads/abranch') b_fork.make_commits('master', b_fork.Commit('pob', tree={'b': '1'}), ref='heads/abranch')
prb = repo_b.make_pr( prb = repo_b.make_pr(
title="a pr", title="a pr",
@ -1058,6 +1061,7 @@ def test_multi_project(env, make_repo, setreviewers, users, config,
'branch_ids': [(0, 0, {'name': 'default'})], 'branch_ids': [(0, 0, {'name': 'default'})],
}) })
setreviewers(*p1.repo_ids) setreviewers(*p1.repo_ids)
env['runbot_merge.events_sources'].create([{'repository': r1.name}])
r2 = make_repo('repo_b') r2 = make_repo('repo_b')
with r2: with r2:
@ -1078,6 +1082,7 @@ def test_multi_project(env, make_repo, setreviewers, users, config,
'branch_ids': [(0, 0, {'name': 'default'})], 'branch_ids': [(0, 0, {'name': 'default'})],
}) })
setreviewers(*p2.repo_ids) setreviewers(*p2.repo_ids)
env['runbot_merge.events_sources'].create([{'repository': r2.name}])
assert r1_dev.owner == r2_dev.owner assert r1_dev.owner == r2_dev.owner

View File

@ -96,7 +96,7 @@ def test_unreviewer(env, project, port):
assert p.review_rights == env['res.partner.review'] assert p.review_rights == env['res.partner.review']
def test_staging_post_update(env, project, make_repo, setreviewers, users, config): def test_staging_post_update(env, repo, users, config):
"""Because statuses come from commits, it's possible to update the commits """Because statuses come from commits, it's possible to update the commits
of a staging after that staging has completed (one way or the other), either of a staging after that staging has completed (one way or the other), either
by sending statuses directly (e.g. rebuilding, for non-deterministic errors) by sending statuses directly (e.g. rebuilding, for non-deterministic errors)
@ -105,21 +105,13 @@ def test_staging_post_update(env, project, make_repo, setreviewers, users, confi
This makes post-mortem analysis quite confusing, so stagings should This makes post-mortem analysis quite confusing, so stagings should
"lock in" their statuses once they complete. "lock in" their statuses once they complete.
""" """
repo = make_repo('repo')
project.write({'repo_ids': [(0, 0, {
'name': repo.name,
'group_id': False,
'required_statuses': 'legal/cla,ci/runbot'
})]})
setreviewers(*project.repo_ids)
with repo: with repo:
[m] = repo.make_commits(None, Commit('initial', tree={'m': 'm'}), ref='heads/master') [m] = repo.make_commits(None, Commit('initial', tree={'m': 'm'}), ref='heads/master')
repo.make_commits(m, Commit('thing', tree={'m': 'c'}), ref='heads/other') repo.make_commits(m, Commit('thing', tree={'m': 'c'}), ref='heads/other')
pr = repo.make_pr(target='master', head='other') pr = repo.make_pr(target='master', head='other')
repo.post_status(pr.head, 'success', 'ci/runbot') repo.post_status(pr.head, 'success')
repo.post_status(pr.head, 'success', 'legal/cla')
pr.post_comment('hansen r+ rebase-merge', config['role_reviewer']['token']) pr.post_comment('hansen r+ rebase-merge', config['role_reviewer']['token'])
env.run_crons() env.run_crons()
pr_id = to_pr(env, pr) pr_id = to_pr(env, pr)
@ -128,33 +120,25 @@ def test_staging_post_update(env, project, make_repo, setreviewers, users, confi
staging_head = repo.commit('staging.master') staging_head = repo.commit('staging.master')
with repo: with repo:
repo.post_status(staging_head, 'failure', 'ci/runbot') repo.post_status(staging_head, 'failure')
env.run_crons() env.run_crons()
assert pr_id.state == 'error' assert pr_id.state == 'error'
assert staging_id.state == 'failure' assert staging_id.state == 'failure'
assert staging_id.statuses == [ assert staging_id.statuses == [
[repo.name, 'ci/runbot', 'failure', ''], [repo.name, 'default', 'failure', ''],
] ]
with repo: with repo:
repo.post_status(staging_head, 'success', 'ci/runbot') repo.post_status(staging_head, 'success')
env.run_crons() env.run_crons()
assert staging_id.state == 'failure' assert staging_id.state == 'failure'
assert staging_id.statuses == [ assert staging_id.statuses == [
[repo.name, 'ci/runbot', 'failure', ''], [repo.name, 'default', 'failure', ''],
] ]
def test_merge_empty_commits(env, project, make_repo, setreviewers, users, config): def test_merge_empty_commits(env, repo, users, config):
"""The mergebot should allow merging already-empty commits. """The mergebot should allow merging already-empty commits.
""" """
repo = make_repo('repo')
project.write({'repo_ids': [(0, 0, {
'name': repo.name,
'group_id': False,
'required_statuses': 'default',
})]})
setreviewers(*project.repo_ids)
with repo: with repo:
[m] = repo.make_commits(None, Commit('initial', tree={'m': 'm'}), ref='heads/master') [m] = repo.make_commits(None, Commit('initial', tree={'m': 'm'}), ref='heads/master')
@ -187,18 +171,10 @@ def test_merge_empty_commits(env, project, make_repo, setreviewers, users, confi
assert commits[1]['commit']['message'].startswith('thing1') assert commits[1]['commit']['message'].startswith('thing1')
assert commits[2]['commit']['message'] == 'initial' assert commits[2]['commit']['message'] == 'initial'
def test_merge_emptying_commits(env, project, make_repo, setreviewers, users, config): def test_merge_emptying_commits(env, repo, users, config):
"""The mergebot should *not* allow merging non-empty commits which become """The mergebot should *not* allow merging non-empty commits which become
empty as part of the staging (rebasing) empty as part of the staging (rebasing)
""" """
repo = make_repo('repo')
project.write({'repo_ids': [(0, 0, {
'name': repo.name,
'group_id': False,
'required_statuses': 'default',
})]})
setreviewers(*project.repo_ids)
with repo: with repo:
[m, _] = repo.make_commits( [m, _] = repo.make_commits(
None, None,
@ -253,15 +229,7 @@ def test_merge_emptying_commits(env, project, make_repo, setreviewers, users, co
(users['user'], f"{ping} unable to stage: results in an empty tree when merged, might be the duplicate of a merged PR.") (users['user'], f"{ping} unable to stage: results in an empty tree when merged, might be the duplicate of a merged PR.")
] ]
def test_force_ready(env, make_repo, project, setreviewers, config): def test_force_ready(env, repo, config):
repo = make_repo('repo')
project.write({'repo_ids': [(0, 0, {
'name': repo.name,
'group_id': False,
'required_statuses': 'default',
})]})
setreviewers(*project.repo_ids)
with repo: with repo:
[m] = repo.make_commits(None, Commit('initial', tree={'m': 'm'}), ref="heads/master") [m] = repo.make_commits(None, Commit('initial', tree={'m': 'm'}), ref="heads/master")

View File

@ -6,17 +6,6 @@ import pytest
from utils import Commit, to_pr, ensure_one from utils import Commit, to_pr, ensure_one
@pytest.fixture
def repo(env, project, make_repo, users, setreviewers):
r = make_repo('repo')
project.write({'repo_ids': [(0, 0, {
'name': r.name,
'group_id': False,
'required_statuses': 'default',
})]})
setreviewers(*project.repo_ids)
return r
def test_disable_staging(env, project, repo, config): def test_disable_staging(env, project, repo, config):
"""In order to avoid issues of cron locking, as well as not disable staging """In order to avoid issues of cron locking, as well as not disable staging
for every project when trying to freeze just one of them (cough cough), a for every project when trying to freeze just one of them (cough cough), a

View File

@ -1,19 +1,6 @@
import pytest
from utils import Commit, to_pr from utils import Commit, to_pr
@pytest.fixture
def repo(env, project, make_repo, users, setreviewers):
r = make_repo('repo')
project.write({'repo_ids': [(0, 0, {
'name': r.name,
'group_id': False,
'required_statuses': 'default'
})]})
setreviewers(*project.repo_ids)
return r
def test_staging_disabled_branch(env, project, repo, config): def test_staging_disabled_branch(env, project, repo, config):
"""Check that it's possible to disable staging on a specific branch """Check that it's possible to disable staging on a specific branch
""" """

View File

@ -50,6 +50,7 @@ def test_basic(env, project, make_repo, users, setreviewers, config):
'status_ids': [(0, 0, {'context': 'l/int'})] 'status_ids': [(0, 0, {'context': 'l/int'})]
}) })
setreviewers(*project.repo_ids) setreviewers(*project.repo_ids)
env['runbot_merge.events_sources'].create({'repository': repo.name})
# "other" can override the lint # "other" can override the lint
env['res.partner'].create({ env['res.partner'].create({
'name': config['role_other'].get('name', 'Other'), 'name': config['role_other'].get('name', 'Other'),
@ -110,6 +111,7 @@ def test_multiple(env, project, make_repo, users, setreviewers, config):
'status_ids': [(0, 0, {'context': 'l/int'}), (0, 0, {'context': 'c/i'})] 'status_ids': [(0, 0, {'context': 'l/int'}), (0, 0, {'context': 'c/i'})]
}) })
setreviewers(*project.repo_ids) setreviewers(*project.repo_ids)
env['runbot_merge.events_sources'].create({'repository': repo.name})
# "other" can override the lints # "other" can override the lints
env['res.partner'].create({ env['res.partner'].create({
'name': config['role_other'].get('name', 'Other'), 'name': config['role_other'].get('name', 'Other'),
@ -174,6 +176,7 @@ def test_no_repository(env, project, make_repo, users, setreviewers, config):
'status_ids': [(0, 0, {'context': 'l/int'})] 'status_ids': [(0, 0, {'context': 'l/int'})]
}) })
setreviewers(*project.repo_ids) setreviewers(*project.repo_ids)
env['runbot_merge.events_sources'].create({'repository': repo.name})
# "other" can override the lint # "other" can override the lint
env['res.partner'].create({ env['res.partner'].create({
'name': config['role_other'].get('name', 'Other'), 'name': config['role_other'].get('name', 'Other'),

View File

@ -63,6 +63,21 @@
</field> </field>
</record> </record>
<record id="action_events_sources" model="ir.actions.act_window">
<field name="name">Events Sources</field>
<field name="res_model">runbot_merge.events_sources</field>
</record>
<record id="tree_events_sources" model="ir.ui.view">
<field name="name">Events Sources List</field>
<field name="model">runbot_merge.events_sources</field>
<field name="arch" type="xml">
<tree editable="bottom">
<field name="repository"/>
<field name="secret"/>
</tree>
</field>
</record>
<menuitem name="Configuration" id="menu_configuration" parent="runbot_merge_menu"> <menuitem name="Configuration" id="menu_configuration" parent="runbot_merge_menu">
<menuitem name="CI Overrides" id="menu_configuration_overrides" <menuitem name="CI Overrides" id="menu_configuration_overrides"
action="action_overrides"/> action="action_overrides"/>
@ -70,5 +85,7 @@
action="action_review"/> action="action_review"/>
<menuitem name="Feedback Templates" id="menu_configuration_feedback" <menuitem name="Feedback Templates" id="menu_configuration_feedback"
action="action_feedback"/> action="action_feedback"/>
<menuitem name="Events Sources" id="menu_configuration_events_sources"
action="action_events_sources"/>
</menuitem> </menuitem>
</odoo> </odoo>

View File

@ -29,7 +29,6 @@
help="Identity when creating new commits, defaults to github name, falls back to login."/> help="Identity when creating new commits, defaults to github name, falls back to login."/>
<field name="github_email" readonly="0" <field name="github_email" readonly="0"
help="Identity when creating new commits, defaults to public email, falls back to primary email."/> help="Identity when creating new commits, defaults to public email, falls back to primary email."/>
<field name="secret"/>
<span attrs="{'invisible': [ <span attrs="{'invisible': [
'|', '|',
('staging_statuses', '=', False), ('staging_statuses', '=', False),