Skip to content

Commit

Permalink
[IMP] runbot: reinforce version
Browse files Browse the repository at this point in the history
When updating a bundle and changing the is_base field to True, it
creates a new version based on the bundle name. This can potentially
breaks builds and moreover, it can breaks the whole runbot when a
duplicate version name is created.

With this commit:
- a constraint on the version name uniqueness is added
- the is_base field is hidden in the interface when the bundle name does
  not match the regular expression defined in the settings
- a version cannot be created by the compute version if the bundle name
  does not match the above mentioned regular expression
  • Loading branch information
d-fence committed Feb 20, 2024
1 parent d661fd9 commit fca01cf
Show file tree
Hide file tree
Showing 6 changed files with 47 additions and 9 deletions.
18 changes: 12 additions & 6 deletions runbot/models/bundle.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
import time
import logging
import datetime
import subprocess
import re

from collections import defaultdict
from odoo import models, fields, api, tools
Expand Down Expand Up @@ -31,6 +28,7 @@ class Bundle(models.Model):

sticky = fields.Boolean('Sticky', compute='_compute_sticky', store=True, index=True)
is_base = fields.Boolean('Is base', index=True)
match_base = fields.Boolean('Can be base', compute='_compute_match_base')
defined_base_id = fields.Many2one('runbot.bundle', 'Forced base bundle', domain="[('project_id', '=', project_id), ('is_base', '=', True)]")
base_id = fields.Many2one('runbot.bundle', 'Base bundle', compute='_compute_base_id', store=True)
to_upgrade = fields.Boolean('To upgrade', compute='_compute_to_upgrade', store=True, index=False)
Expand Down Expand Up @@ -121,10 +119,10 @@ def _compute_has_pr(self):
def _get_base_ids(self, project_id):
return [(b.id, b.name) for b in self.search([('is_base', '=', True), ('project_id', '=', project_id)])]

@api.depends('is_base', 'base_id.version_id')
@api.depends('is_base', 'base_id.version_id', 'match_base')
def _compute_version_id(self):
for bundle in self.sorted(key='is_base', reverse=True):
if not bundle.is_base:
if not bundle.is_base or not bundle.match_base:
bundle.version_id = bundle.base_id.version_id
continue
bundle.version_id = self.env['runbot.version']._get(bundle.name)
Expand Down Expand Up @@ -198,6 +196,14 @@ def _compute_last_done_batch(self):
for batch in batchs:
batch.bundle_id.last_done_batch = batch

@api.depends('name')
def _compute_match_base(self):
icp = self.env['ir.config_parameter'].sudo()
regex = icp.get_param('runbot.runbot_is_base_regex', False)
if regex:
for bundle in self:
bundle.match_base = re.match(regex, bundle.name)

def _url(self):
self.ensure_one()
return "/runbot/bundle/%s" % self.id
Expand Down
4 changes: 4 additions & 0 deletions runbot/models/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ class Version(models.Model):

dockerfile_id = fields.Many2one('runbot.dockerfile', default=lambda self: self.env.ref('runbot.docker_default', raise_if_not_found=False))

_sql_constraints = [
('unique_name', 'unique (name)', 'avoid duplicate versions'),
]

@api.depends('name')
def _compute_version_number(self):
for version in self:
Expand Down
1 change: 1 addition & 0 deletions runbot/tests/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,3 +15,4 @@
from . import test_upgrade
from . import test_dockerfile
from . import test_host
from . import test_bundle
25 changes: 25 additions & 0 deletions runbot/tests/test_bundle.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
from .common import RunbotCase

class TestBundleCreation(RunbotCase):
def test_version_at_bundle_creation(self):
saas_name = 'saas-27.2'
saas_bundle = self.Bundle.create({
'name': saas_name,
'project_id': self.project.id
})
self.assertTrue(saas_bundle.match_base)
saas_bundle.is_base = True
self.assertEqual(saas_bundle.version_id.name, saas_name, 'The bundle version_id should create base version')

dev_name = 'saas-27.2-brol-bro'
dev_bundle = self.Bundle.create({
'name': dev_name,
'project_id': self.project.id
})
self.assertFalse(dev_bundle.match_base)
self.assertEqual(dev_bundle.version_id.name, saas_name)

self.assertFalse(self.Version.search([('name', '=', dev_name)]), 'A dev bundle should not summon a new version')
dev_bundle.is_base = True
self.assertFalse(self.Version.search([('name', '=', dev_name)]), 'A dev bundle should not summon a new version, even if is_base is True')
self.assertEqual(dev_bundle.version_id.name, saas_name)
2 changes: 1 addition & 1 deletion runbot/tests/test_version.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ def test_basic_version(self):

self.assertGreater(saas_version.number, major_version.number)

master_version = self.Version.create({'name': 'master'})
master_version = self.Version.search([('name', '=', 'master')])
self.assertEqual(master_version.number, '~')
self.assertGreater(master_version.number, saas_version.number)

Expand Down
6 changes: 4 additions & 2 deletions runbot/views/bundle_views.xml
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,8 @@
<field name="project_id"/>
<field name="sticky" readonly="0"/>
<field name="to_upgrade" readonly="0"/>
<field name="is_base"/>
<field name="match_base" invisible="1"/>
<field name="is_base" invisible="not match_base"/>
<field name="base_id"/>
<field name="defined_base_id"/>
<field name="version_id"/>
Expand Down Expand Up @@ -147,7 +148,8 @@
<field name="project_id"/>
<field name="name" widget="char_frontend_url"/>
<field name="version_number"/>
<field name="is_base"/>
<field name="match_base" column_invisible="True"/>
<field name="is_base" invisible="not match_base"/>
<field name="sticky"/>
<field name="to_upgrade"/>
<field name="no_build"/>
Expand Down

0 comments on commit fca01cf

Please sign in to comment.