diff --git a/module_build_service/scm.py b/module_build_service/scm.py index 4411093d..4d25425e 100644 --- a/module_build_service/scm.py +++ b/module_build_service/scm.py @@ -47,7 +47,7 @@ class SCM(object): # Assuming git for HTTP schemas types = module_build_service.utils.scm_url_schemes() - def __init__(self, url, allowed_scm=None, allow_local = False): + def __init__(self, url, branch = None, allowed_scm=None, allow_local = False): """Initialize the SCM object using the specified scmurl. If url is not in the list of allowed_scm, an error will be raised. @@ -85,11 +85,33 @@ class SCM(object): if self.name.endswith(".git"): self.name = self.name[:-4] self.commit = match.group("commit") - self.branch = "master" + self.branch = branch if branch else "master" + if not self.commit: + self.commit = self.get_latest(self.branch) self.version = None else: raise ValidationError("Unhandled SCM scheme: %s" % self.scheme) + def verify(self, sourcedir): + """ + 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 + """ + + found = False + branches = SCM._run(["git", "branch", "-r", "--contains", self.commit], chdir=sourcedir)[1] + for branch in branches.split("\n"): + branch = branch.strip() + if branch[len("origin/"):] == self.branch: + found = True + break + if not found: + raise ValidationError("Commit %s is not in branch %s." % (self.commit, self.branch)) + def scm_url_from_name(self, name): """ Generates new SCM URL for another module defined by a name. The new URL @@ -154,7 +176,8 @@ class SCM(object): log.debug("Getting/verifying commit hash for %s" % self.repository) output = SCM._run(["git", "ls-remote", self.repository, branch])[1] if output: - return output.split("\t")[0] + self.commit = output.split("\t")[0] + return self.commit # Hopefully `branch` is really a commit hash. Code later needs to verify this. if self.is_available(True): diff --git a/module_build_service/utils.py b/module_build_service/utils.py index 1941a66e..d5072c09 100644 --- a/module_build_service/utils.py +++ b/module_build_service/utils.py @@ -311,7 +311,7 @@ def filter_module_builds(flask_request): return query.paginate(page, per_page, False) -def _fetch_mmd(url, allow_local_url = False): +def _fetch_mmd(url, branch = None, allow_local_url = False): # Import it here, because SCM uses utils methods # and fails to import them because of dep-chain. import module_build_service.scm @@ -322,8 +322,9 @@ def _fetch_mmd(url, allow_local_url = False): try: log.debug('Verifying modulemd') td = tempfile.mkdtemp() - scm = module_build_service.scm.SCM(url, conf.scmurls, allow_local_url) + 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")) with open(cofn, "r") as mmdfile: @@ -556,8 +557,9 @@ def submit_module_build_from_yaml(username, yaml, optional_params=None): return submit_module_build(username, None, mmd, None, yaml, optional_params) -def submit_module_build_from_scm(username, url, allow_local_url=False, optional_params=None): - mmd, scm, yaml = _fetch_mmd(url, allow_local_url) +def submit_module_build_from_scm(username, url, branch, allow_local_url=False, + optional_params=None): + mmd, scm, yaml = _fetch_mmd(url, branch, allow_local_url) return submit_module_build(username, url, mmd, scm, yaml, optional_params) @@ -610,7 +612,7 @@ def submit_module_build(username, url, mmd, scm, yaml, optional_params=None): def validate_optional_params(params): - forbidden_params = [k for k in params if k not in models.ModuleBuild.__table__.columns] + forbidden_params = [k for k in params if k not in models.ModuleBuild.__table__.columns and k not in ["branch"]] if forbidden_params: raise ValidationError('The request contains unspecified parameters: {}'.format(", ".join(forbidden_params))) diff --git a/module_build_service/views.py b/module_build_service/views.py index d53e63fd..9ea9a31d 100644 --- a/module_build_service/views.py +++ b/module_build_service/views.py @@ -127,9 +127,15 @@ class ModuleBuildAPI(MethodView): log.error("The submitted scmurl %r is not valid" % url) raise Unauthorized("The submitted scmurl %s is not valid" % url) + if "branch" not in r: + log.error('Missing branch') + raise ValidationError('Missing branch') + + branch = r["branch"] + validate_optional_params(r) - optional_params = {k: v for k, v in r.items() if k != "scmurl"} - return submit_module_build_from_scm(username, url, allow_local_url=False, optional_params=optional_params) + optional_params = {k: v for k, v in r.items() if k != "scmurl" and k != 'branch'} + return submit_module_build_from_scm(username, url, branch, allow_local_url=False, optional_params=optional_params) def post_file(self, username): if not conf.yaml_submit_allowed: diff --git a/tests/scm_data/testrepo/objects/17/2c8d5a1db52737ebeb50fe5058212b97ac3f82 b/tests/scm_data/testrepo/objects/17/2c8d5a1db52737ebeb50fe5058212b97ac3f82 new file mode 100644 index 00000000..e6cc0fd8 Binary files /dev/null and b/tests/scm_data/testrepo/objects/17/2c8d5a1db52737ebeb50fe5058212b97ac3f82 differ diff --git a/tests/scm_data/testrepo/objects/70/35bd33614972ac66559ac1fdd019ff6027ad21 b/tests/scm_data/testrepo/objects/70/35bd33614972ac66559ac1fdd019ff6027ad21 new file mode 100644 index 00000000..11ff6bc9 Binary files /dev/null and b/tests/scm_data/testrepo/objects/70/35bd33614972ac66559ac1fdd019ff6027ad21 differ diff --git a/tests/scm_data/testrepo/objects/b8/f5533d9ab4cbc7a7dee0c1131dfc12e4376524 b/tests/scm_data/testrepo/objects/b8/f5533d9ab4cbc7a7dee0c1131dfc12e4376524 new file mode 100644 index 00000000..abc35a1e Binary files /dev/null and b/tests/scm_data/testrepo/objects/b8/f5533d9ab4cbc7a7dee0c1131dfc12e4376524 differ diff --git a/tests/scm_data/testrepo/refs/heads/dev b/tests/scm_data/testrepo/refs/heads/dev new file mode 100644 index 00000000..b0de8524 --- /dev/null +++ b/tests/scm_data/testrepo/refs/heads/dev @@ -0,0 +1 @@ +7035bd33614972ac66559ac1fdd019ff6027ad21 diff --git a/tests/test_build/test_build.py b/tests/test_build/test_build.py index 62593a3d..8939e790 100644 --- a/tests/test_build/test_build.py +++ b/tests/test_build/test_build.py @@ -245,7 +245,7 @@ class TestBuild(unittest.TestCase): '620ec77321b2ea7b0d67d82992dda3e1d67055b4') rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( - {'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#68932c90de214d9d13feefbd35246a81b6cb8d49'})) data = json.loads(rv.data) @@ -316,7 +316,7 @@ class TestBuild(unittest.TestCase): @timed(30) @patch('module_build_service.auth.get_user', return_value=user) def test_submit_build_with_optional_params(self, mocked_get_user): - params = {'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + params = {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#68932c90de214d9d13feefbd35246a81b6cb8d49'} def submit(data): @@ -342,7 +342,7 @@ class TestBuild(unittest.TestCase): '620ec77321b2ea7b0d67d82992dda3e1d67055b4') rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( - {'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#68932c90de214d9d13feefbd35246a81b6cb8d49'})) data = json.loads(rv.data) @@ -395,7 +395,7 @@ class TestBuild(unittest.TestCase): '620ec77321b2ea7b0d67d82992dda3e1d67055b4') rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( - {'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#68932c90de214d9d13feefbd35246a81b6cb8d49'})) data = json.loads(rv.data) @@ -428,7 +428,7 @@ class TestBuild(unittest.TestCase): conf.set_item("num_consecutive_builds", 1) rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( - {'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#68932c90de214d9d13feefbd35246a81b6cb8d49'})) data = json.loads(rv.data) diff --git a/tests/test_scm.py b/tests/test_scm.py index f4c4c2cb..4b5edc59 100644 --- a/tests/test_scm.py +++ b/tests/test_scm.py @@ -25,8 +25,10 @@ import shutil import tempfile import unittest +from nose.tools import raises import module_build_service.scm +from module_build_service.errors import ValidationError repo_path = 'file://' + os.path.dirname(__file__) + "/scm_data/testrepo" @@ -75,3 +77,34 @@ class TestSCMModule(unittest.TestCase): scm = module_build_service.scm.SCM(repo_path + '/') target = 'testrepo' assert scm.name == target, '%r != %r' % (scm.name, target) + + def test_verify(self): + scm = module_build_service.scm.SCM(repo_path) + sourcedir = scm.checkout(self.tempdir) + scm.verify(sourcedir) + + @raises(RuntimeError) + def test_verify_unknown_branch(self): + scm = module_build_service.scm.SCM(repo_path, "unknown") + sourcedir = scm.checkout(self.tempdir) + scm.verify(sourcedir) + + 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) + + @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) + + @raises(RuntimeError) + 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) diff --git a/tests/test_views/test_views.py b/tests/test_views/test_views.py index 6bfd62ee..f6ff6f20 100644 --- a/tests/test_views/test_views.py +++ b/tests/test_views/test_views.py @@ -260,7 +260,7 @@ class TestViews(unittest.TestCase): '620ec77321b2ea7b0d67d82992dda3e1d67055b4') rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( - {'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49'})) data = json.loads(rv.data) @@ -299,7 +299,7 @@ class TestViews(unittest.TestCase): '3da541559918a808c2402bba5012f6c60b27661c') rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( - {'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49'})) data = json.loads(rv.data) @@ -322,7 +322,7 @@ class TestViews(unittest.TestCase): client_secrets = path.join(base_dir, "client_secrets.json") with patch.dict('module_build_service.app.config', {'OIDC_CLIENT_SECRETS': client_secrets}): rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( - {'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#48931b90de214d9d13feefbd35246a81b6cb8d49'})) data = json.loads(rv.data) self.assertEquals( @@ -335,7 +335,7 @@ class TestViews(unittest.TestCase): @patch('module_build_service.auth.get_user', return_value=user) def test_submit_build_scm_url_error(self, mocked_get_user): rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( - {'scmurl': 'git://badurl.com'})) + {'branch': 'master', 'scmurl': 'git://badurl.com'})) data = json.loads(rv.data) self.assertEquals(data['message'], 'The submitted scmurl ' 'git://badurl.com is not allowed') @@ -345,7 +345,7 @@ class TestViews(unittest.TestCase): @patch('module_build_service.auth.get_user', return_value=user) def test_submit_build_scm_url_without_hash(self, mocked_get_user): rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( - {'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git'})) data = json.loads(rv.data) self.assertEquals(data['message'], 'The submitted scmurl ' @@ -360,7 +360,7 @@ class TestViews(unittest.TestCase): mocked_scm_obj = MockedSCM(mocked_scm, "bad", "bad.yaml") rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( - {'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49'})) data = json.loads(rv.data) self.assertTrue(data['message'].startswith('Invalid modulemd:')) @@ -381,7 +381,7 @@ class TestViews(unittest.TestCase): start = time.time() rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( - {'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49'})) data = json.loads(rv.data) @@ -415,7 +415,7 @@ class TestViews(unittest.TestCase): mocked_scm.return_value.get_latest = mocked_scm_get_latest rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( - {'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49'})) data = json.loads(rv.data) @@ -432,7 +432,7 @@ class TestViews(unittest.TestCase): ["includedmodules.yaml", "testmodule.yaml"]) try: rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( - {'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49'})) except e: raise @@ -472,7 +472,7 @@ class TestViews(unittest.TestCase): mocked_scm_obj = MockedSCM(mocked_scm, "includedmodules", ["includedmodules.yaml", "testmodule.yaml"]) rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( - {'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49'})) data = json.loads(rv.data) @@ -524,7 +524,7 @@ class TestViews(unittest.TestCase): scmurl = 'unsupported://example.com/modules/' 'testmodule.git?#0000000000000000000000000000000000000000' rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( - {'scmurl': scmurl})) + {'branch': 'master', 'scmurl': scmurl})) data = json.loads(rv.data) self.assertIn( data['message'], ( @@ -542,7 +542,7 @@ class TestViews(unittest.TestCase): '620ec77321b2ea7b0d67d82992dda3e1d67055b4') rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( - {'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49'})) data = json.loads(rv.data) self.assertEquals(data['status'], 400) @@ -560,7 +560,7 @@ class TestViews(unittest.TestCase): '620ec77321b2ea7b0d67d82992dda3e1d67055b4') rv = self.client.post('/module-build-service/1/module-builds/', data=json.dumps( - {'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' + {'branch': 'master', 'scmurl': 'git://pkgs.stg.fedoraproject.org/modules/' 'testmodule.git?#68931c90de214d9d13feefbd35246a81b6cb8d49'})) data = json.loads(rv.data) self.assertEquals(data['status'], 400)