diff --git a/runbot/container.py b/runbot/container.py index 3de78ca9..c765f760 100644 --- a/runbot/container.py +++ b/runbot/container.py @@ -15,6 +15,7 @@ import io import json import logging import os +import re import shutil import subprocess import time @@ -121,6 +122,7 @@ def docker_run(run_cmd, log_path, build_dir, container_name, exposed_ports=None, :params ro_volumes: dict of dest:source volumes to mount readonly in builddir :params env_variables: list of environment variables """ + container_name = sanitize_container_name(container_name) if isinstance(run_cmd, Command): cmd_object = run_cmd run_cmd = cmd_object.build() @@ -170,6 +172,7 @@ def docker_run(run_cmd, log_path, build_dir, container_name, exposed_ports=None, def docker_stop(container_name, build_dir=None): """Stops the container named container_name""" + container_name = sanitize_container_name(container_name) _logger.info('Stopping container %s', container_name) if build_dir: end_file = os.path.join(build_dir, 'end-%s' % container_name) @@ -179,10 +182,12 @@ def docker_stop(container_name, build_dir=None): subprocess.run(['docker', 'stop', container_name]) def docker_is_running(container_name): + container_name = sanitize_container_name(container_name) dinspect = subprocess.run(['docker', 'container', 'inspect', container_name], stderr=subprocess.DEVNULL, stdout=subprocess.DEVNULL) return True if dinspect.returncode == 0 else False def docker_state(container_name, build_dir): + container_name = sanitize_container_name(container_name) started = os.path.exists(os.path.join(build_dir, 'start-%s' % container_name)) ended = os.path.exists(os.path.join(build_dir, 'end-%s' % container_name)) if ended: @@ -198,6 +203,7 @@ def docker_state(container_name, build_dir): def docker_clear_state(container_name, build_dir): """Return True if container is still running""" + container_name = sanitize_container_name(container_name) if os.path.exists(os.path.join(build_dir, 'start-%s' % container_name)): os.remove(os.path.join(build_dir, 'start-%s' % container_name)) if os.path.exists(os.path.join(build_dir, 'end-%s' % container_name)): @@ -235,6 +241,12 @@ def build(args): docker_build(logfile, args.build_dir) _logger.info('Finished building the base image container') + +def sanitize_container_name(name): + """Returns a container name with unallowed characters removed""" + name = re.sub('^[^a-zA-Z0-9]+', '', name) + return re.sub('[^a-zA-Z0-9_.-]', '', name) + def tests(args): _logger.info('Start container tests') os.makedirs(os.path.join(args.build_dir, 'logs'), exist_ok=True) diff --git a/runbot/tests/test_command.py b/runbot/tests/test_command.py index 8fe3416a..6583a4ab 100644 --- a/runbot/tests/test_command.py +++ b/runbot/tests/test_command.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- from odoo.tests import common from ..container import Command +from ..container import sanitize_container_name CONFIG = """[options] @@ -35,3 +36,28 @@ class Test_Command(common.TransactionCase): with self.assertRaises(AssertionError): cmd.add_config_tuple('http-interface', '127.0.0.1') + + +class TestSanitizeContainerName(common.TransactionCase): + + def test_sanitize_container_name(self): + + # 1. test that a valid name remains unchanged + valid_name = '3155889-saas-13.4-container-all_at_install' + self.assertEqual(sanitize_container_name(valid_name), valid_name) + + # 2. test a name starting with an invalid character + invalid_name = '#3155889-saas-13.4-container-all_at_install' + self.assertEqual(sanitize_container_name(invalid_name), valid_name) + + # 3. test a name with an invalid character somewhere + invalid_name = '3155889-saas-13.4-container#-all_at_install' + self.assertEqual(sanitize_container_name(invalid_name), valid_name) + + # 4. test a name starting with multiple invalid characters + invalid_name = '#/.3155889-saas-13.4-container-all_at_install' + self.assertEqual(sanitize_container_name(invalid_name), valid_name) + + # 5. test both + invalid_name = '_.3155889-saas-13.4-##container/-all_at_install' + self.assertEqual(sanitize_container_name(invalid_name), valid_name)