From 9684c2d97ed2307b44d6c182e47c3cb68759e72f Mon Sep 17 00:00:00 2001 From: Christophe Monniez Date: Wed, 17 Jun 2020 09:17:31 +0200 Subject: [PATCH] [FIX] runbot: avoid unallowed characters in docker names Docker container names are derived from the dest and step name. The dest is itself derived from the branch name. In some rare cases, it happens that a character not allowed by Docker appears in the container name computed by the runbot. With this commit, a sanitize_container_name function is used to remove unallowed characters at the container utility level. --- runbot/container.py | 12 ++++++++++++ runbot/tests/test_command.py | 26 ++++++++++++++++++++++++++ 2 files changed, 38 insertions(+) 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)