[FIX] project creation: handling of mergebot info

- don't *fail* in `_compute_identity`, it causes issues when the token
  is valid but doesn't have `user:email` access as the request is
  aborted and saving doesn't work
- make `github_name` and `github_email` required rather than ad-hoc
  requiring them in `_compute_identity` (which doesn't work correctly)
- force copy of `github_name` and `github_email`, with o2ms being
  !copy this means duplicating projects now works out of the box (or
  should...)

Currently errors in `_compute_identity` are reported via logging which
is not great as it's not UI visible, should probably get moved to
chatter eventually but that's not currently enabled on projects.

Fixes #990
This commit is contained in:
Xavier Morel 2024-11-29 09:04:06 +01:00
parent 38c2bc97f3
commit 679d556c90
11 changed files with 36 additions and 15 deletions

View File

@ -179,7 +179,8 @@ def rolemap(request, config):
r.raise_for_status()
user = rolemap[role] = r.json()
data['user'] = user['login']
n = data['user'] = user['login']
data.setdefault('name', n)
return rolemap
@pytest.fixture

View File

@ -193,6 +193,8 @@ def test_massive_conflict(env, config, make_repo):
'name': "thing",
'github_token': config['github']['token'],
'github_prefix': 'hansen',
'github_name': config['github']['name'],
'github_email': "foo@example.org",
'fp_github_token': config['github']['token'],
'fp_github_name': 'herbert',
'branch_ids': [
@ -357,6 +359,8 @@ def test_conflict_deleted_deep(env, config, make_repo):
'name': "test",
'github_token': config['github']['token'],
'github_prefix': 'hansen',
'github_name': config['github']['name'],
'github_email': "foo@example.org",
'fp_github_token': config['github']['token'],
'fp_github_name': 'herbert',
'branch_ids': [

View File

@ -543,6 +543,8 @@ def post_merge(env, config, users, make_repo, branches):
'name': prod.name,
'github_token': config['github']['token'],
'github_prefix': 'hansen',
'github_name': config['github']['name'],
'github_email': "foo@example.org",
'fp_github_token': config['github']['token'],
'fp_github_name': 'herbert',
'branch_ids': [

View File

@ -320,6 +320,8 @@ def test_duplicate_fw(env, make_repo, setreviewers, config, users):
'name': 'a project',
'github_token': config['github']['token'],
'github_prefix': 'hansen',
'github_name': config['github']['name'],
'github_email': "foo@example.org",
'fp_github_token': config['github']['token'],
'fp_github_name': 'herbert',
'branch_ids': [

View File

@ -242,6 +242,8 @@ class TestNotAllBranches:
'name': 'proj',
'github_token': config['github']['token'],
'github_prefix': 'hansen',
'github_name': config['github']['name'],
'github_email': "foo@example.org",
'fp_github_token': config['github']['token'],
'fp_github_name': 'herbert',
'branch_ids': [
@ -1089,6 +1091,8 @@ def test_disable_multitudes(env, config, make_repo, users, setreviewers):
"name": "bob",
"github_token": config['github']['token'],
"github_prefix": "hansen",
'github_name': config['github']['name'],
'github_email': "foo@example.org",
"fp_github_token": config['github']['token'],
"fp_github_name": "herbert",
"branch_ids": [

View File

@ -122,6 +122,8 @@ def make_basic(
'name': project_name,
'github_token': config['github']['token'],
'github_prefix': 'hansen',
'github_name': config['github']['name'],
'github_email': "foo@example.org",
'fp_github_token': fp_token and config['github']['token'],
'fp_github_name': 'herbert',
'branch_ids': [

View File

@ -6,7 +6,6 @@ import requests
import sentry_sdk
from odoo import models, fields, api
from odoo.exceptions import UserError
from odoo.osv import expression
from odoo.tools import reverse_order
@ -41,8 +40,8 @@ class Project(models.Model):
)
github_token = fields.Char("Github Token", required=True)
github_name = fields.Char(store=True, compute="_compute_identity")
github_email = fields.Char(store=True, compute="_compute_identity")
github_name = fields.Char(store=True, compute="_compute_identity", required=True, copy=True)
github_email = fields.Char(store=True, compute="_compute_identity", required=True, copy=True)
github_prefix = fields.Char(
required=True,
default="hanson", # mergebot du bot du bot du~
@ -74,11 +73,10 @@ class Project(models.Model):
if not project.github_token or (project.github_name and project.github_email):
continue
r0 = s.get('https://api.github.com/user', headers={
'Authorization': 'token %s' % project.github_token
})
headers = {'Authorization': f'token {project.github_token}'}
r0 = s.get('https://api.github.com/user', headers=headers)
if not r0.ok:
_logger.error("Failed to fetch merge bot information for project %s: %s", project.name, r0.text or r0.content)
_logger.warning("Failed to fetch merge bot information for project %s: %s", project.name, r0.text or r0.content)
continue
r = r0.json()
@ -88,20 +86,17 @@ class Project(models.Model):
continue
if 'user:email' not in set(re.split(r',\s*', r0.headers['x-oauth-scopes'])):
raise UserError("The merge bot github token needs the user:email scope to fetch the bot's identity.")
r1 = s.get('https://api.github.com/user/emails', headers={
'Authorization': 'token %s' % project.github_token
})
_logger.warning("Unable to fetch merge bot emails for project %s: scope missing from token", project.name)
r1 = s.get('https://api.github.com/user/emails', headers=headers)
if not r1.ok:
_logger.error("Failed to fetch merge bot emails for project %s: %s", project.name, r1.text or r1.content)
_logger.warning("Failed to fetch merge bot emails for project %s: %s", project.name, r1.text or r1.content)
continue
project.github_email = next((
entry['email']
for entry in r1.json()
if entry['primary']
), None)
if not project.github_email:
raise UserError("The merge bot needs a public or accessible primary email set up.")
# technically the email could change at any moment...
@api.depends('fp_github_token')

View File

@ -10,6 +10,8 @@ def project(env, config):
'name': 'odoo',
'github_token': config['github']['token'],
'github_prefix': 'hansen',
'github_name': config['github']['name'],
'github_email': "foo@example.org",
'branch_ids': [(0, 0, {'name': 'master'})],
})

View File

@ -939,6 +939,8 @@ class TestSubstitutions:
p = env['runbot_merge.project'].create({
'name': 'proj',
'github_token': 'wheeee',
'github_name': "yyy",
'github_email': "zzz@example.org",
'repo_ids': [(0, 0, {'name': 'xxx/xxx'})],
'branch_ids': [(0, 0, {'name': 'master'})]
})
@ -1058,6 +1060,8 @@ def test_multi_project(env, make_repo, setreviewers, users, config,
'name': 'Project 1',
'github_token': gh_token,
'github_prefix': 'hansen',
'github_name': config['github']['name'],
'github_email': "foo@example.org",
'repo_ids': [(0, 0, {
'name': r1.name,
'group_id': False,
@ -1079,6 +1083,8 @@ def test_multi_project(env, make_repo, setreviewers, users, config,
'name': "Project 2",
'github_token': gh_token,
'github_prefix': 'hansen',
'github_name': config['github']['name'],
'github_email': "foo@example.org",
'repo_ids': [(0, 0, {
'name': r2.name,
'group_id': False,

View File

@ -37,6 +37,8 @@ def test_name_search(env):
p = env['runbot_merge.project'].create({
'name': 'proj',
'github_token': 'no',
'github_name': "noo",
'github_email': "nooo@example.org",
})
b = env['runbot_merge.branch'].create({
'name': 'target',

View File

@ -21,6 +21,7 @@ def test_finding(env):
project = env['runbot_merge.project'].create({
'name': 'test',
'github_token': 'xxx', 'github_prefix': 'no',
'github_name': "xxx", 'github_email': "xxx@example.org",
})
repo_1 = env['runbot_merge.repository'].create({'project_id': project.id, 'name': 'r1'})
repo_2 = env['runbot_merge.repository'].create({'project_id': project.id, 'name': 'r2'})