From 09b3f32dbb35bb159d6bb77f696c58baa20667a7 Mon Sep 17 00:00:00 2001 From: Filip Valder Date: Mon, 7 Aug 2017 12:50:23 +0200 Subject: [PATCH] Fix #619: Support for module YAML file validation within a SCM repository --- module_build_service/scm.py | 50 ++++++++++++++++++++++++++++------ module_build_service/utils.py | 6 ++-- tests/test_build/test_build.py | 14 +++++++--- tests/test_scm.py | 27 +++++++++++------- tests/test_utils/test_utils.py | 14 +++++++--- tests/test_views/test_views.py | 14 +++++++--- 6 files changed, 91 insertions(+), 34 deletions(-) diff --git a/module_build_service/scm.py b/module_build_service/scm.py index e4244477..cace0e8a 100644 --- a/module_build_service/scm.py +++ b/module_build_service/scm.py @@ -35,7 +35,8 @@ import shutil import datetime from module_build_service import log -from module_build_service.errors import Forbidden, ValidationError, UnprocessableEntity +from module_build_service.errors import ( + Forbidden, ValidationError, UnprocessableEntity, ProgrammingError) import module_build_service.utils @@ -64,6 +65,7 @@ class SCM(object): url = url.rstrip('/') self.url = url + self.sourcedir = None # once we have more than one SCM provider, we will need some more # sophisticated lookup logic @@ -90,18 +92,20 @@ class SCM(object): else: raise ValidationError("Unhandled SCM scheme: %s" % self.scheme) - def verify(self, sourcedir): + def verify(self): """ Verifies that the information provided by a user in SCM URL and branch matches the information in SCM repository. For example verifies that the commit hash really belongs to the provided branch. - :param str sourcedir: Directory with SCM repo as returned by checkout(). :raises ValidationError """ + if not self.sourcedir: + raise ProgrammingError("Do .checkout() first.") found = False - branches = SCM._run(["git", "branch", "-r", "--contains", self.commit], chdir=sourcedir)[1] + branches = SCM._run(["git", "branch", "-r", "--contains", self.commit], + chdir=self.sourcedir)[1] for branch in branches.split("\n"): branch = branch.strip() if branch[len("origin/"):] == self.branch: @@ -143,20 +147,20 @@ class SCM(object): """ # TODO: sanity check arguments if self.scheme == "git": - sourcedir = '%s/%s' % (scmdir, self.name) + self.sourcedir = '%s/%s' % (scmdir, self.name) module_clone_cmd = ['git', 'clone', '-q'] if self.commit: module_checkout_cmd = ['git', 'checkout', '-q', self.commit] else: module_clone_cmd.extend(['--depth', '1']) - module_clone_cmd.extend([self.repository, sourcedir]) + module_clone_cmd.extend([self.repository, self.sourcedir]) # perform checkouts SCM._run(module_clone_cmd, chdir=scmdir) if self.commit: try: - SCM._run(module_checkout_cmd, chdir=sourcedir) + SCM._run(module_checkout_cmd, chdir=self.sourcedir) except RuntimeError as e: if (e.message.endswith( " did not match any file(s) known to git.\\n\"") or @@ -167,12 +171,12 @@ class SCM(object): "The original message was: %s" % e.message) raise - timestamp = SCM._run(["git", "show", "-s", "--format=%ct"], chdir=sourcedir)[1] + timestamp = SCM._run(["git", "show", "-s", "--format=%ct"], chdir=self.sourcedir)[1] dt = datetime.datetime.utcfromtimestamp(int(timestamp)) self.version = dt.strftime("%Y%m%d%H%M%S") else: raise RuntimeError("checkout: Unhandled SCM scheme.") - return sourcedir + return self.sourcedir def get_latest(self, branch='master'): """Get the latest commit ID. @@ -230,6 +234,25 @@ class SCM(object): else: raise RuntimeError('get_full_commit_hash: Unhandled SCM scheme.') + def get_module_yaml(self): + """ + Get full path to the module's YAML file. + + :return: path as a string + :raises UnprocessableEntity + """ + if not self.sourcedir: + raise ProgrammingError("Do .checkout() first.") + + path_to_yaml = os.path.join(self.sourcedir, (self.name + ".yaml")) + try: + with open(path_to_yaml): + return path_to_yaml + except IOError: + raise UnprocessableEntity( + "get_module_yaml: SCM repository doesn't seem to contain a " + "module YAML file. Couldn't access: %s" % path_to_yaml) + @staticmethod def is_full_commit_hash(scheme, commit): """ @@ -288,6 +311,15 @@ class SCM(object): def scheme(self, s): self._scheme = str(s) + @property + def sourcedir(self): + """The SCM source directory.""" + return self._sourcedir + + @sourcedir.setter + def sourcedir(self, s): + self._sourcedir = str(s) + @property def repository(self): """The repository part of the scmurl.""" diff --git a/module_build_service/utils.py b/module_build_service/utils.py index d6bbba0b..4e8966da 100644 --- a/module_build_service/utils.py +++ b/module_build_service/utils.py @@ -494,9 +494,9 @@ def _fetch_mmd(url, branch=None, allow_local_url=False, whitelist_url=False): scm = module_build_service.scm.SCM(url, branch, [url], allow_local_url) else: scm = module_build_service.scm.SCM(url, branch, conf.scmurls, allow_local_url) - cod = scm.checkout(td) - scm.verify(cod) - cofn = os.path.join(cod, (scm.name + ".yaml")) + scm.checkout(td) + scm.verify() + cofn = scm.get_module_yaml() with open(cofn, "r") as mmdfile: yaml = mmdfile.read() diff --git a/tests/test_build/test_build.py b/tests/test_build/test_build.py index 802a8e42..bc0b7f10 100644 --- a/tests/test_build/test_build.py +++ b/tests/test_build/test_build.py @@ -57,6 +57,7 @@ class MockedSCM(object): self.name = name self.commit = commit self.mmd_filename = mmd_filename + self.sourcedir = None self.mocked_scm.return_value.checkout = self.checkout self.mocked_scm.return_value.name = self.name @@ -64,19 +65,24 @@ class MockedSCM(object): self.mocked_scm.return_value.get_latest = self.get_latest self.mocked_scm.return_value.commit = self.commit self.mocked_scm.return_value.repository_root = "git://pkgs.stg.fedoraproject.org/modules/" + self.mocked_scm.return_value.sourcedir = self.sourcedir + self.mocked_scm.return_value.get_module_yaml = self.get_module_yaml def checkout(self, temp_dir): - scm_dir = path.join(temp_dir, self.name) - mkdir(scm_dir) + self.sourcedir = path.join(temp_dir, self.name) + mkdir(self.sourcedir) base_dir = path.abspath(path.dirname(__file__)) copyfile(path.join(base_dir, '..', 'staged_data', self.mmd_filename), - path.join(scm_dir, self.mmd_filename)) + self.get_module_yaml()) - return scm_dir + return self.sourcedir def get_latest(self, branch='master'): return branch + def get_module_yaml(self): + return path.join(self.sourcedir, self.name + ".yaml") + class TestModuleBuilder(GenericBuilder): """ diff --git a/tests/test_scm.py b/tests/test_scm.py index afced120..2f500a2e 100644 --- a/tests/test_scm.py +++ b/tests/test_scm.py @@ -80,31 +80,38 @@ class TestSCMModule(unittest.TestCase): def test_verify(self): scm = module_build_service.scm.SCM(repo_path) - sourcedir = scm.checkout(self.tempdir) - scm.verify(sourcedir) + scm.checkout(self.tempdir) + scm.verify() @raises(UnprocessableEntity) def test_verify_unknown_branch(self): scm = module_build_service.scm.SCM(repo_path, "unknown") - sourcedir = scm.checkout(self.tempdir) - scm.verify(sourcedir) + scm.checkout(self.tempdir) + scm.verify() def test_verify_commit_in_branch(self): target = '7035bd33614972ac66559ac1fdd019ff6027ad21' scm = module_build_service.scm.SCM(repo_path + "?#" + target, "dev") - sourcedir = scm.checkout(self.tempdir) - scm.verify(sourcedir) + scm.checkout(self.tempdir) + scm.verify() @raises(ValidationError) def test_verify_commit_not_in_branch(self): target = '7035bd33614972ac66559ac1fdd019ff6027ad21' scm = module_build_service.scm.SCM(repo_path + "?#" + target, "master") - sourcedir = scm.checkout(self.tempdir) - scm.verify(sourcedir) + scm.checkout(self.tempdir) + scm.verify() @raises(UnprocessableEntity) def test_verify_unknown_hash(self): target = '7035bd33614972ac66559ac1fdd019ff6027ad22' scm = module_build_service.scm.SCM(repo_path + "?#" + target, "master") - sourcedir = scm.checkout(self.tempdir) - scm.verify(sourcedir) + scm.checkout(self.tempdir) + scm.verify() + + @raises(UnprocessableEntity) + def test_get_module_yaml(self): + scm = module_build_service.scm.SCM(repo_path) + scm.checkout(self.tempdir) + scm.verify() + scm.get_module_yaml() diff --git a/tests/test_utils/test_utils.py b/tests/test_utils/test_utils.py index 8c38104a..7d785fd6 100644 --- a/tests/test_utils/test_utils.py +++ b/tests/test_utils/test_utils.py @@ -47,6 +47,7 @@ class MockedSCM(object): self.name = name self.commit = commit self.mmd_filename = mmd_filename + self.sourcedir = None self.mocked_scm.return_value.checkout = self.checkout self.mocked_scm.return_value.name = self.name @@ -54,19 +55,24 @@ class MockedSCM(object): self.mocked_scm.return_value.get_latest = self.get_latest self.mocked_scm.return_value.commit = self.commit self.mocked_scm.return_value.repository_root = "git://pkgs.stg.fedoraproject.org/modules/" + self.mocked_scm.return_value.sourcedir = self.sourcedir + self.mocked_scm.return_value.get_module_yaml = self.get_module_yaml def checkout(self, temp_dir): - scm_dir = path.join(temp_dir, self.name) - mkdir(scm_dir) + self.sourcedir = path.join(temp_dir, self.name) + mkdir(self.sourcedir) base_dir = path.abspath(path.dirname(__file__)) copyfile(path.join(base_dir, '..', 'staged_data', self.mmd_filename), - path.join(scm_dir, self.name + ".yaml")) + self.get_module_yaml()) - return scm_dir + return self.sourcedir def get_latest(self, branch='master'): return self.commit if self.commit else branch + def get_module_yaml(self): + return path.join(self.sourcedir, self.name + ".yaml") + class TestUtils(unittest.TestCase): diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index 1056bab1..42df8c96 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -66,6 +66,7 @@ class MockedSCM(object): mmd_filenames = [mmd_filenames] self.mmd_filenames = mmd_filenames self.checkout_id = 0 + self.sourcedir = None if checkout_raise: self.mocked_scm.return_value.checkout.side_effect = \ @@ -85,6 +86,8 @@ class MockedSCM(object): self.mocked_scm.return_value.get_latest = self.get_latest self.mocked_scm.return_value.repository_root = "git://pkgs.stg.fedoraproject.org/modules/" self.mocked_scm.return_value.branch = 'master' + self.mocked_scm.return_value.sourcedir = self.sourcedir + self.mocked_scm.return_value.get_module_yaml = self.get_module_yaml def checkout(self, temp_dir): try: @@ -92,19 +95,22 @@ class MockedSCM(object): except: mmd_filename = self.mmd_filenames[0] - scm_dir = path.join(temp_dir, self.name) - mkdir(scm_dir) + self.sourcedir = path.join(temp_dir, self.name) + mkdir(self.sourcedir) base_dir = path.abspath(path.dirname(__file__)) copyfile(path.join(base_dir, '..', 'staged_data', mmd_filename), - path.join(scm_dir, self.name + ".yaml")) + self.get_module_yaml()) self.checkout_id += 1 - return scm_dir + return self.sourcedir def get_latest(self, branch='master'): return hashlib.sha1(branch).hexdigest()[:10] + def get_module_yaml(self): + return path.join(self.sourcedir, self.name + ".yaml") + class TestViews(unittest.TestCase): maxDiff = None