Add a basic validation of the branch value

Signed-off-by: Zack Cerza <zack.cerza@inktank.com>

diff --git a/teuthology/repo_utils.py b/teuthology/repo_utils.py
index 7c5bd43cb802940aec5a7cb6f16a04014e16e0d3..bf5a28cbbb97d380e81c16837f2deb89bce92fd8 100644 (file)
--- a/teuthology/repo_utils.py
+++ b/teuthology/repo_utils.py
@@ -18,6 +18,7 @@ def enforce_repo_state(repo_url, dest_path, branch):
     :raises:          BranchNotFoundError if the branch is not found;
                       RuntimeError for other errors
     """
+    validate_branch(branch)
     try:
         if not os.path.isdir(dest_path):
             clone_repo(repo_url, dest_path, branch)
@@ -46,6 +47,7 @@ def clone_repo(repo_url, dest_path, branch):
     :raises:          BranchNotFoundError if the branch is not found;
                       RuntimeError for other errors
     """
+    validate_branch(branch)
     log.info("Cloning %s %s from upstream", repo_url, branch)
     proc = subprocess.Popen(
         ('git', 'clone', '--branch', branch, repo_url, dest_path),
@@ -71,6 +73,7 @@ def fetch_branch(dest_path, branch):
     :raises:          BranchNotFoundError if the branch is not found;
                       RuntimeError for other errors
     """
+    validate_branch(branch)
     log.info("Fetching %s from upstream", branch)
     proc = subprocess.Popen(
         ('git', 'fetch', '-p', 'origin', branch),
@@ -96,6 +99,7 @@ def reset_repo(repo_url, dest_path, branch):
     :raises:          BranchNotFoundError if the branch is not found;
                       RuntimeError for other errors
     """
+    validate_branch(branch)
     # This try/except block will notice if the requested branch doesn't
     # exist, whether it was cloned or fetched.
     try:
@@ -119,3 +123,8 @@ class BranchNotFoundError(ValueError):
             repo_str = ""
         return "Branch {branch} not found{repo_str}!".format(
             branch=self.branch, repo_str=repo_str)
+
+
+def validate_branch(branch):
+    if ' ' in branch:
+        raise ValueError("Illegal branch name: '%s'" % branch)

diff --git a/teuthology/test/test_repo_utils.py b/teuthology/test/test_repo_utils.py
index e03831292ca0c6e506c726b88edf409fc3e19159..0fc685e86e668c92e5ea490d8b0f39a7b955eb10 100644 (file)
--- a/teuthology/test/test_repo_utils.py
+++ b/teuthology/test/test_repo_utils.py
@@ -13,20 +13,22 @@ class TestRepoUtils(object):
     repo_url = 'file://' + src_path
     dest_path = '/tmp/empty_dest'
 
-    def setup(self):
+    def setup_method(self, method):
         assert not os.path.exists(self.dest_path)
         proc = subprocess.Popen(
             ('git', 'init', self.src_path),
+            stdout=subprocess.PIPE,
         )
         assert proc.wait() == 0
         proc = subprocess.Popen(
             ('git', 'commit', '--allow-empty', '--allow-empty-message',
              '--no-edit'),
             cwd=self.src_path,
+            stdout=subprocess.PIPE,
         )
         assert proc.wait() == 0
 
-    def teardown(self):
+    def teardown_method(self, method):
         shutil.rmtree(self.dest_path, ignore_errors=True)
 
     def test_existing_branch(self):
@@ -69,3 +71,7 @@ class TestRepoUtils(object):
         repo_utils.enforce_repo_state(self.repo_url, self.dest_path,
                                       'master')
         assert os.path.exists(self.dest_path)
+
+    def test_invalid_branch(self):
+        with raises(ValueError):
+            repo_utils.enforce_repo_state(self.repo_url, self.dest_path, 'a b')