From e07fcd6dffe567ec50358eecebf79eb477d6e39c Mon Sep 17 00:00:00 2001 From: Christophe Monniez Date: Tue, 20 Feb 2024 16:22:13 +0100 Subject: [PATCH] [IMP] runbot: reinforce version 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 --- runbot/models/bundle.py | 7 ++----- runbot/models/version.py | 17 ++++++++++++++++- runbot/tests/__init__.py | 1 + runbot/tests/test_bundle.py | 28 ++++++++++++++++++++++++++++ runbot/tests/test_version.py | 2 +- 5 files changed, 48 insertions(+), 7 deletions(-) create mode 100644 runbot/tests/test_bundle.py diff --git a/runbot/models/bundle.py b/runbot/models/bundle.py index 8f6282f7b..cb7c87f70 100644 --- a/runbot/models/bundle.py +++ b/runbot/models/bundle.py @@ -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 @@ -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) diff --git a/runbot/models/version.py b/runbot/models/version.py index cd5cde55a..8333e03b6 100644 --- a/runbot/models/version.py +++ b/runbot/models/version.py @@ -1,7 +1,7 @@ import logging import re from odoo import models, fields, api, tools - +from odoo.exceptions import UserError _logger = logging.getLogger(__name__) @@ -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: @@ -42,6 +46,13 @@ 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)) @@ -49,6 +60,10 @@ def _get(self, 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, }) diff --git a/runbot/tests/__init__.py b/runbot/tests/__init__.py index 188387ca1..9e89dddc2 100644 --- a/runbot/tests/__init__.py +++ b/runbot/tests/__init__.py @@ -15,3 +15,4 @@ from . import test_upgrade from . import test_dockerfile from . import test_host +from . import test_bundle diff --git a/runbot/tests/test_bundle.py b/runbot/tests/test_bundle.py new file mode 100644 index 000000000..e734805a3 --- /dev/null +++ b/runbot/tests/test_bundle.py @@ -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) diff --git a/runbot/tests/test_version.py b/runbot/tests/test_version.py index f95b8d8c2..7db2541de 100644 --- a/runbot/tests/test_version.py +++ b/runbot/tests/test_version.py @@ -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)