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 now readonky
- a version cannot be created by the compute version if the version name
  does not match the above mentioned regular expression
  • Loading branch information
d-fence committed Feb 21, 2024
1 parent f26065c commit e07fcd6
Show file tree
Hide file tree
Showing 5 changed files with 48 additions and 7 deletions.
7 changes: 2 additions & 5 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 @@ -30,7 +27,7 @@ class Bundle(models.Model):
last_done_batch = fields.Many2many('runbot.batch', 'Last batchs', compute='_compute_last_done_batch')

sticky = fields.Boolean('Sticky', compute='_compute_sticky', store=True, index=True)
is_base = fields.Boolean('Is base', index=True)
is_base = fields.Boolean('Is base', index=True, readonly=True)
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
17 changes: 16 additions & 1 deletion runbot/models/version.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import logging
import re
from odoo import models, fields, api, tools

from odoo.exceptions import UserError

_logger = logging.getLogger(__name__)

Expand All @@ -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 All @@ -42,13 +46,24 @@ def create(self, vals_list):
model.env.registry.clear_cache()
return super().create(vals_list)

def write(self, values):
if 'name' in values:
icp = self.env['ir.config_parameter'].sudo()
regex = icp.get_param('runbot.runbot_is_base_regex', False)
if regex and not re.match(regex, values['name']):
raise UserError(f"Version name {values['name']} does not match a valid base name.")

def _get(self, name):
return self.browse(self._get_id(name))

@tools.ormcache('name')
def _get_id(self, name):
version = self.search([('name', '=', name)])
if not version:
icp = self.env['ir.config_parameter'].sudo()
regex = icp.get_param('runbot.runbot_is_base_regex', False)
if regex and not re.match(regex, name):
return False
version = self.create({
'name': name,
})
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
28 changes: 28 additions & 0 deletions runbot/tests/test_bundle.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
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
})
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.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_name = 'saas-27.2-brol-zzz'
dev_bundle = self.Bundle.create({
'name': dev_name,
'project_id': self.project.id,
'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.id, False)
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

0 comments on commit e07fcd6

Please sign in to comment.