From 950d6cc4ed0cf481857478dabad3373aef7198fa Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Wed, 10 May 2017 21:43:56 -0400 Subject: [PATCH 01/11] first stab at inverted scm rules --- koji/daemon.py | 57 +++++++++++++++++++++++++++++++++----------------- 1 file changed, 38 insertions(+), 19 deletions(-) diff --git a/koji/daemon.py b/koji/daemon.py index d479de62..aecb7c1e 100644 --- a/koji/daemon.py +++ b/koji/daemon.py @@ -284,8 +284,17 @@ class SCM(object): Verify that the host and repository of this SCM is in the provided list of allowed repositories. - allowed is a space-separated list of host:repository[:use_common[:source_cmd]] tuples. Incorrectly-formatted - tuples will be ignored. + allowed is a space-separated list entries in one of the following forms: + + host:repository[:use_common[:source_cmd]] + !host:repository + + Incorrectly-formatted entries will be ignored. + + The first form allows a specific host:repository and optionally sets a + few options for it. + + The second form explicitly blocks a specific host:repository TODO If use_common is not present, kojid will attempt to checkout a common/ directory from the repository. If use_common is set to no, off, false, or 0, it will not attempt to checkout a common/ @@ -295,26 +304,36 @@ class SCM(object): It is generally used to retrieve source files from a remote location. If no source_cmd is specified, "make sources" is run by default. """ + is_allowed = False for allowed_scm in allowed.split(): scm_tuple = allowed_scm.split(':') - if len(scm_tuple) >= 2: - if fnmatch(self.host, scm_tuple[0]) and fnmatch(self.repository, scm_tuple[1]): - # SCM host:repository is in the allowed list - # check if we specify a value for use_common - if len(scm_tuple) >= 3: - if scm_tuple[2].lower() in ('no', 'off', 'false', '0'): - self.use_common = False - # check if we specify a custom source_cmd - if len(scm_tuple) >= 4: - if scm_tuple[3]: - self.source_cmd = scm_tuple[3].split(',') - else: - # there was nothing after the trailing :, so they don't want to run a source_cmd at all - self.source_cmd = None - break - else: + if len(scm_tuple) < 2: self.logger.warn('Ignoring incorrectly formatted SCM host:repository: %s' % allowed_scm) - else: + continue + host_pat = scm_tuple[0] + repo_pat = scm_tuple[1] + invert = False + if host_pat.startwith('!'): + invert = True + host_pat = host_pat[1:] + if fnmatch(self.host, host_pat) and fnmatch(self.repository, repo_pat): + # match + if invert: + break + is_allowed = True + # check if we specify a value for use_common + if len(scm_tuple) >= 3: + if scm_tuple[2].lower() in ('no', 'off', 'false', '0'): + self.use_common = False + # check if we specify a custom source_cmd + if len(scm_tuple) >= 4: + if scm_tuple[3]: + self.source_cmd = scm_tuple[3].split(',') + else: + # there was nothing after the trailing :, so they don't want to run a source_cmd at all + self.source_cmd = None + break + if not allowed: raise koji.BuildError('%s:%s is not in the list of allowed SCMs' % (self.host, self.repository)) def checkout(self, scmdir, session=None, uploadpath=None, logfile=None): From dd87bd39e399f409ebb78e0b60744c270c459e7e Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Thu, 11 May 2017 12:57:57 -0400 Subject: [PATCH 02/11] scm unit test, partial --- koji/daemon.py | 2 +- tests/test_scm.py | 66 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 67 insertions(+), 1 deletion(-) create mode 100644 tests/test_scm.py diff --git a/koji/daemon.py b/koji/daemon.py index aecb7c1e..991e9ca2 100644 --- a/koji/daemon.py +++ b/koji/daemon.py @@ -225,7 +225,7 @@ class SCM(object): if self.scheme in schemes: self.scmtype = scmtype break - else: + else: # pragma: no cover # should never happen raise koji.GenericError('Invalid SCM URL: %s' % url) diff --git a/tests/test_scm.py b/tests/test_scm.py new file mode 100644 index 00000000..bf36a0ce --- /dev/null +++ b/tests/test_scm.py @@ -0,0 +1,66 @@ +import mock +import unittest + +import logging +import koji + +from koji.daemon import SCM + + +class TestSCM(unittest.TestCase): + + def test_urlcheck(self): + good = [ + "git://server/foo.git#bab0c73900241ef5c465d7e873e9d8b34c948e67", + "git+ssh://server2/other/path#bab0c73900241ef5c465d7e873e9d8b34c948e67", + "svn://server/path/to/code#bab0c73900241ef5c465d7e873e9d8b34c948e67", + "svn+ssh://server/some/path#bab0c73900241ef5c465d7e873e9d8b34c948e67", + "cvs://server/some/path#bab0c73900241ef5c465d7e873e9d8b34c948e67", + "cvs+ssh://server/some/path#bab0c73900241ef5c465d7e873e9d8b34c948e67", + ] + bad = [ + "http://localhost/foo.html", + "foo-1.1-1.src.rpm", + "https://server/foo-1.1-1.src.rpm", + ] + for url in good: + self.assertTrue(SCM.is_scm_url(url)) + for url in bad: + self.assertFalse(SCM.is_scm_url(url)) + + @mock.patch('logging.getLogger') + def test_init(self, getLogger): + bad = [ + "git://user@@server/foo.git#bab0c73900241ef5c465d7e873e9d8b34c948e67", + "git://user:pass@server/foo.git#bab0c73900241ef5c465d7e873e9d8b34c948e67", + "git://server/foo.git?params=not_allowed", + "git://server#asdasd", # no path + "git://server/foo.git", # no fragment + "http://localhost/foo.html", + "foo-1.1-1.src.rpm", + "https://server/foo-1.1-1.src.rpm", + ] + for url in bad: + print url + try: + #with self.assertRaises(koji.GenericError): + scm = SCM(url) + except koji.GenericError, e: + print e + else: + raise Exception("fucked") + + url = "git://user@server/foo.git#bab0c73900241ef5c465d7e873e9d8b34c948e67" + scm = SCM(url) + self.assertEqual(scm.scheme, 'git://') + self.assertEqual(scm.user, 'user') + self.assertEqual(scm.host, 'server') + self.assertEqual(scm.repository, '/foo.git') + self.assertEqual(scm.module, '') + self.assertEqual(scm.revision, 'bab0c73900241ef5c465d7e873e9d8b34c948e67') + self.assertEqual(scm.use_common, True) + self.assertEqual(scm.source_cmd, ['make', 'sources']) + self.assertEqual(scm.scmtype, 'GIT') + + + From 04b42c83203dfd64ff815c2c21562fec15d03b12 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Thu, 11 May 2017 17:50:14 -0400 Subject: [PATCH 03/11] tests/fixes for assert_allowed --- koji/daemon.py | 7 ++-- tests/test_scm.py | 99 +++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 95 insertions(+), 11 deletions(-) diff --git a/koji/daemon.py b/koji/daemon.py index 991e9ca2..faf629cb 100644 --- a/koji/daemon.py +++ b/koji/daemon.py @@ -267,7 +267,8 @@ class SCM(object): # check for validity: params should be empty, query may be empty, everything else should be populated if params: raise koji.GenericError('Unable to parse SCM URL: %s . Params element %s should be empty.' % (self.url, params)) - if not scheme: + if not scheme: #pragma: no cover + # should not happen because of is_scm_url check earlier raise koji.GenericError('Unable to parse SCM URL: %s . Could not find the scheme element.' % self.url) if not netloc: raise koji.GenericError('Unable to parse SCM URL: %s . Could not find the netloc element.' % self.url) @@ -313,7 +314,7 @@ class SCM(object): host_pat = scm_tuple[0] repo_pat = scm_tuple[1] invert = False - if host_pat.startwith('!'): + if host_pat.startswith('!'): invert = True host_pat = host_pat[1:] if fnmatch(self.host, host_pat) and fnmatch(self.repository, repo_pat): @@ -333,7 +334,7 @@ class SCM(object): # there was nothing after the trailing :, so they don't want to run a source_cmd at all self.source_cmd = None break - if not allowed: + if not is_allowed: raise koji.BuildError('%s:%s is not in the list of allowed SCMs' % (self.host, self.repository)) def checkout(self, scmdir, session=None, uploadpath=None, logfile=None): diff --git a/tests/test_scm.py b/tests/test_scm.py index bf36a0ce..4a8d7f00 100644 --- a/tests/test_scm.py +++ b/tests/test_scm.py @@ -33,22 +33,19 @@ class TestSCM(unittest.TestCase): bad = [ "git://user@@server/foo.git#bab0c73900241ef5c465d7e873e9d8b34c948e67", "git://user:pass@server/foo.git#bab0c73900241ef5c465d7e873e9d8b34c948e67", - "git://server/foo.git?params=not_allowed", + "git://server/foo.git;params=not_allowed", "git://server#asdasd", # no path "git://server/foo.git", # no fragment "http://localhost/foo.html", + "git://@localhost/foo/?a=bar/", + "http://localhost/foo.html?a=foo/", "foo-1.1-1.src.rpm", + "git://", "https://server/foo-1.1-1.src.rpm", ] for url in bad: - print url - try: - #with self.assertRaises(koji.GenericError): + with self.assertRaises(koji.GenericError): scm = SCM(url) - except koji.GenericError, e: - print e - else: - raise Exception("fucked") url = "git://user@server/foo.git#bab0c73900241ef5c465d7e873e9d8b34c948e67" scm = SCM(url) @@ -63,4 +60,90 @@ class TestSCM(unittest.TestCase): self.assertEqual(scm.scmtype, 'GIT') + @mock.patch('logging.getLogger') + def test_allowed(self, getLogger): + allowed = ''' + goodserver:*:no + !badserver:* + !maybeserver:/badpath/* + maybeserver:*:no + ''' + good = [ + "git://goodserver/path1#1234", + "git+ssh://maybeserver/path1#1234", + ] + bad = [ + "cvs://badserver/projects/42#ref", + "svn://badserver/projects/42#ref", + ] + for url in good: + scm = SCM(url) + scm.assert_allowed(allowed) + for url in bad: + scm = SCM(url) + with self.assertRaises(koji.BuildError): + scm.assert_allowed(allowed) + + @mock.patch('logging.getLogger') + def test_badrule(self, getLogger): + allowed = ''' + bogus-entry-should-be-ignored + goodserver:*:no + !badserver:* + ''' + url = "git://goodserver/path1#1234" + scm = SCM(url) + scm.assert_allowed(allowed) + + @mock.patch('logging.getLogger') + def test_opts(self, getLogger): + allowed = ''' + default:* + nocommon:*:no + srccmd:*:no:fedpkg,sources + mixed:/foo/*:no + mixed:/bar/*:yes + mixed:/baz/*:no:fedpkg,sources + ''' + + url = "git://default/koji.git#1234" + scm = SCM(url) + scm.assert_allowed(allowed) + self.assertEqual(scm.use_common, True) + self.assertEqual(scm.source_cmd, ['make', 'sources']) + + url = "git://nocommon/koji.git#1234" + scm = SCM(url) + scm.assert_allowed(allowed) + self.assertEqual(scm.use_common, False) + self.assertEqual(scm.source_cmd, ['make', 'sources']) + + url = "git://srccmd/koji.git#1234" + scm = SCM(url) + scm.assert_allowed(allowed) + self.assertEqual(scm.use_common, False) + self.assertEqual(scm.source_cmd, ['fedpkg', 'sources']) + + url = "git://mixed/foo/koji.git#1234" + scm = SCM(url) + scm.assert_allowed(allowed) + self.assertEqual(scm.use_common, False) + self.assertEqual(scm.source_cmd, ['make', 'sources']) + + url = "git://mixed/bar/koji.git#1234" + scm = SCM(url) + scm.assert_allowed(allowed) + self.assertEqual(scm.use_common, True) + self.assertEqual(scm.source_cmd, ['make', 'sources']) + + url = "git://mixed/baz/koji.git#1234" + scm = SCM(url) + scm.assert_allowed(allowed) + self.assertEqual(scm.use_common, False) + self.assertEqual(scm.source_cmd, ['fedpkg', 'sources']) + + url = "git://mixed/koji.git#1234" + scm = SCM(url) + with self.assertRaises(koji.BuildError): + scm.assert_allowed(allowed) From b4ea7c21897e5dbd240b172bdd8d4aea922db0ea Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Thu, 11 May 2017 19:49:42 -0400 Subject: [PATCH 04/11] a checkout test --- tests/test_scm.py | 80 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 80 insertions(+) diff --git a/tests/test_scm.py b/tests/test_scm.py index 4a8d7f00..955c6d59 100644 --- a/tests/test_scm.py +++ b/tests/test_scm.py @@ -2,7 +2,13 @@ import mock import unittest import logging +import shutil +import tempfile + +from pprint import pprint + import koji +import koji.daemon from koji.daemon import SCM @@ -101,6 +107,7 @@ class TestSCM(unittest.TestCase): default:* nocommon:*:no srccmd:*:no:fedpkg,sources + nosrc:*:no: mixed:/foo/*:no mixed:/bar/*:yes mixed:/baz/*:no:fedpkg,sources @@ -124,6 +131,12 @@ class TestSCM(unittest.TestCase): self.assertEqual(scm.use_common, False) self.assertEqual(scm.source_cmd, ['fedpkg', 'sources']) + url = "git://nosrc/koji.git#1234" + scm = SCM(url) + scm.assert_allowed(allowed) + self.assertEqual(scm.use_common, False) + self.assertEqual(scm.source_cmd, None) + url = "git://mixed/foo/koji.git#1234" scm = SCM(url) scm.assert_allowed(allowed) @@ -147,3 +160,70 @@ class TestSCM(unittest.TestCase): with self.assertRaises(koji.BuildError): scm.assert_allowed(allowed) + url = "git://mixed/foo/koji.git#1234" + scm = SCM(url) + scm.assert_allowed(allowed) + self.assertEqual(scm.use_common, False) + self.assertEqual(scm.source_cmd, ['make', 'sources']) + + url = "git://mixed/bar/koji.git#1234" + scm = SCM(url) + scm.assert_allowed(allowed) + self.assertEqual(scm.use_common, True) + self.assertEqual(scm.source_cmd, ['make', 'sources']) + + url = "git://mixed/baz/koji.git#1234" + scm = SCM(url) + scm.assert_allowed(allowed) + self.assertEqual(scm.use_common, False) + self.assertEqual(scm.source_cmd, ['fedpkg', 'sources']) + + url = "git://mixed/koji.git#1234" + scm = SCM(url) + with self.assertRaises(koji.BuildError): + scm.assert_allowed(allowed) + + +class TestSCMCheckouts(unittest.TestCase): + + def setUp(self): + self.symlink = mock.patch('os.symlink').start() + self.getLogger = mock.patch('logging.getLogger').start() + self.log_output = mock.patch('koji.daemon.log_output').start() + self.log_output.return_value = None + self.tempdir = tempfile.mkdtemp() + self.session = mock.MagicMock() + self.uploadpath = mock.MagicMock() + self.logfile = mock.MagicMock() + + def tearDown(self): + mock.patch.stopall() + shutil.rmtree(self.tempdir) + + def test_checkout_nocommon(self): + allowed = ''' + default:* + nocommon:*:no + srccmd:*:no:fedpkg,sources + nosrc:*:no: + ''' + + url = "git://nocommon/koji.git#asdasd" + scm = SCM(url) + scm.assert_allowed(allowed) + scm.checkout(self.tempdir, session=self.session, + uploadpath=self.uploadpath, logfile=self.logfile) + self.assertEqual(scm.use_common, False) + self.symlink.assert_not_called() + # expected commands + cmd = ['git', 'clone', '-n', 'git://nocommon/koji.git', + self.tempdir + '/koji'] + call1 = mock.call(self.session, cmd[0], cmd, self.logfile, + self.uploadpath, cwd=self.tempdir, logerror=1, + append=False, env=None) + cmd = ['git', 'reset', '--hard', 'asdasd'] + call2 = mock.call(self.session, cmd[0], cmd, self.logfile, + self.uploadpath, cwd=self.tempdir + '/koji', + logerror=1, append=True, env=None) + self.log_output.assert_has_calls([call1, call2]) + From 49b4d0ae9fa27566533467cb77616ec709458220 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Thu, 11 May 2017 22:12:41 -0400 Subject: [PATCH 05/11] another test --- tests/test_scm.py | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/tests/test_scm.py b/tests/test_scm.py index 955c6d59..a91f1295 100644 --- a/tests/test_scm.py +++ b/tests/test_scm.py @@ -195,22 +195,22 @@ class TestSCMCheckouts(unittest.TestCase): self.session = mock.MagicMock() self.uploadpath = mock.MagicMock() self.logfile = mock.MagicMock() - - def tearDown(self): - mock.patch.stopall() - shutil.rmtree(self.tempdir) - - def test_checkout_nocommon(self): - allowed = ''' + self.allowed = ''' default:* nocommon:*:no srccmd:*:no:fedpkg,sources nosrc:*:no: ''' + def tearDown(self): + mock.patch.stopall() + shutil.rmtree(self.tempdir) + + def test_checkout_git_nocommon(self): + url = "git://nocommon/koji.git#asdasd" scm = SCM(url) - scm.assert_allowed(allowed) + scm.assert_allowed(self.allowed) scm.checkout(self.tempdir, session=self.session, uploadpath=self.uploadpath, logfile=self.logfile) self.assertEqual(scm.use_common, False) @@ -227,3 +227,23 @@ class TestSCMCheckouts(unittest.TestCase): logerror=1, append=True, env=None) self.log_output.assert_has_calls([call1, call2]) + def test_checkout_gitssh_nocommon(self): + + url = "git+ssh://user@nocommon/koji.git#asdasd" + scm = SCM(url) + scm.assert_allowed(self.allowed) + scm.checkout(self.tempdir, session=self.session, + uploadpath=self.uploadpath, logfile=self.logfile) + self.assertEqual(scm.use_common, False) + self.symlink.assert_not_called() + # expected commands + cmd = ['git', 'clone', '-n', 'git+ssh://user@nocommon/koji.git', + self.tempdir + '/koji'] + call1 = mock.call(self.session, cmd[0], cmd, self.logfile, + self.uploadpath, cwd=self.tempdir, logerror=1, + append=False, env=None) + cmd = ['git', 'reset', '--hard', 'asdasd'] + call2 = mock.call(self.session, cmd[0], cmd, self.logfile, + self.uploadpath, cwd=self.tempdir + '/koji', + logerror=1, append=True, env=None) + self.log_output.assert_has_calls([call1, call2]) From c49d8552b8ba6b1c042791ebf46c1d0a484c6add Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Thu, 11 May 2017 22:19:05 -0400 Subject: [PATCH 06/11] another test --- tests/test_scm.py | 25 +++++++++++++++++++++++++ 1 file changed, 25 insertions(+) diff --git a/tests/test_scm.py b/tests/test_scm.py index a91f1295..1655e734 100644 --- a/tests/test_scm.py +++ b/tests/test_scm.py @@ -247,3 +247,28 @@ class TestSCMCheckouts(unittest.TestCase): self.uploadpath, cwd=self.tempdir + '/koji', logerror=1, append=True, env=None) self.log_output.assert_has_calls([call1, call2]) + + def test_checkout_git_common(self): + + url = "git://default/koji.git#asdasd" + scm = SCM(url) + scm.assert_allowed(self.allowed) + scm.checkout(self.tempdir, session=self.session, + uploadpath=self.uploadpath, logfile=self.logfile) + self.assertEqual(scm.use_common, True) + self.symlink.assert_called_once() + # expected commands + cmd = ['git', 'clone', '-n', 'git://default/koji.git', + self.tempdir + '/koji'] + call1 = mock.call(self.session, cmd[0], cmd, self.logfile, + self.uploadpath, cwd=self.tempdir, logerror=1, + append=False, env=None) + cmd = ['git', 'reset', '--hard', 'asdasd'] + call2 = mock.call(self.session, cmd[0], cmd, self.logfile, + self.uploadpath, cwd=self.tempdir + '/koji', + logerror=1, append=True, env=None) + cmd = ['git', 'clone', 'git://default/common.git', 'common'] + call3 = mock.call(self.session, cmd[0], cmd, self.logfile, + self.uploadpath, cwd=self.tempdir, + logerror=1, append=True, env=None) + self.log_output.assert_has_calls([call1, call2, call3]) From 39d71efa583212f9fa5433bc2e60450ba98b0d22 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Sat, 13 May 2017 13:40:10 -0400 Subject: [PATCH 07/11] clean up docstring --- koji/daemon.py | 31 ++++++++++++++++++++----------- 1 file changed, 20 insertions(+), 11 deletions(-) diff --git a/koji/daemon.py b/koji/daemon.py index faf629cb..913a2739 100644 --- a/koji/daemon.py +++ b/koji/daemon.py @@ -282,28 +282,37 @@ class SCM(object): def assert_allowed(self, allowed): """ - Verify that the host and repository of this SCM is in the provided list of - allowed repositories. + Check this scm against allowed list and apply options - allowed is a space-separated list entries in one of the following forms: + allowed is a space-separated list of entries in one of the following + forms: host:repository[:use_common[:source_cmd]] !host:repository Incorrectly-formatted entries will be ignored. - The first form allows a specific host:repository and optionally sets a + The first form allows a host:repository pattern and optionally sets a few options for it. - The second form explicitly blocks a specific host:repository TODO + The second form explicitly blocks a host:repository pattern - If use_common is not present, kojid will attempt to checkout a common/ directory from the - repository. If use_common is set to no, off, false, or 0, it will not attempt to checkout a common/ - directory. + Both host and repository are treated as glob patterns - source_cmd is a shell command (args separated with commas instead of spaces) to run before building the srpm. - It is generally used to retrieve source files from a remote location. If no source_cmd is specified, - "make sources" is run by default. + If there is a matching entry, then the optional fields, if given, will + be applied to the instance. + + If there is no matching entry, or if the host:repository is blocked + then BuildError is raised. + + The use_common option defaults to on. If it is set to no, off, false + or 0, it will be disabled. If the option is on, then kojid will + attempt to checkout a common/ directory from the repository. + + The source_command is a shell command to be run before building the + srpm. It defaults to "make sources". This can be overridden by the + matching allowed entry. The command must be encoded with commas + instead of spaces (e.g. "make,sources"). """ is_allowed = False for allowed_scm in allowed.split(): From 5e19c06fce03aee5b6ddb8d9b86064851bf6fb28 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Sat, 13 May 2017 14:22:46 -0400 Subject: [PATCH 08/11] more tests --- tests/test_scm.py | 93 +++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/tests/test_scm.py b/tests/test_scm.py index 1655e734..4e013fd9 100644 --- a/tests/test_scm.py +++ b/tests/test_scm.py @@ -272,3 +272,96 @@ class TestSCMCheckouts(unittest.TestCase): self.uploadpath, cwd=self.tempdir, logerror=1, append=True, env=None) self.log_output.assert_has_calls([call1, call2, call3]) + + def test_checkout_error_in_command(self): + + url = "git://nocommon/koji.git#asdasd" + scm = SCM(url) + scm.assert_allowed(self.allowed) + self.log_output.return_value = 1 + with self.assertRaises(koji.BuildError): + scm.checkout(self.tempdir, session=self.session, + uploadpath=self.uploadpath, logfile=self.logfile) + self.assertEqual(scm.use_common, False) + self.symlink.assert_not_called() + # expected commands + cmd = ['git', 'clone', '-n', 'git://nocommon/koji.git', + self.tempdir + '/koji'] + call1 = mock.call(self.session, cmd[0], cmd, self.logfile, + self.uploadpath, cwd=self.tempdir, logerror=1, + append=False, env=None) + # should have errored after first command + self.log_output.assert_has_calls([call1]) + + def test_checkout_cvs_common(self): + + url = "cvs://default/cvsisdead?rpms/foo/EL3#sometag" + scm = SCM(url) + scm.assert_allowed(self.allowed) + scm.checkout(self.tempdir, session=self.session, + uploadpath=self.uploadpath, logfile=self.logfile) + self.assertEqual(scm.use_common, True) + self.symlink.assert_called_once() + # expected commands + cmd = ['cvs', '-d', ':pserver:anonymous@default:/cvsisdead', 'checkout', + '-r', 'sometag', 'rpms/foo/EL3'] + call1 = mock.call(self.session, cmd[0], cmd, self.logfile, + self.uploadpath, cwd=self.tempdir, logerror=1, + append=False, env=None) + cmd = ['cvs', '-d', ':pserver:anonymous@default:/cvsisdead', 'checkout', + 'common'] + call2 = mock.call(self.session, cmd[0], cmd, self.logfile, + self.uploadpath, cwd=self.tempdir, logerror=1, + append=True, env=None) + self.log_output.assert_has_calls([call1, call2]) + + def test_checkout_cvs_ssh(self): + + url = "cvs+ssh://user@nocommon/cvsisdead?rpms/foo/EL3#sometag" + scm = SCM(url) + scm.assert_allowed(self.allowed) + scm.checkout(self.tempdir, session=self.session, + uploadpath=self.uploadpath, logfile=self.logfile) + self.assertEqual(scm.use_common, False) + self.symlink.assert_not_called() + # expected commands + cmd = ['cvs', '-d', ':ext:user@nocommon:/cvsisdead', 'checkout', '-r', + 'sometag', 'rpms/foo/EL3'] + call1 = mock.call(self.session, cmd[0], cmd, self.logfile, + self.uploadpath, cwd=self.tempdir, logerror=1, + append=False, env={'CVS_RSH': 'ssh'}) + self.log_output.assert_has_calls([call1]) + + def test_checkout_svn(self): + + url = "svn://nocommon/dist?rpms/foo/EL3#revision" + scm = SCM(url) + scm.assert_allowed(self.allowed) + scm.checkout(self.tempdir, session=self.session, + uploadpath=self.uploadpath, logfile=self.logfile) + self.assertEqual(scm.use_common, False) + self.symlink.assert_not_called() + # expected commands + cmd = ['svn', 'checkout', '-r', 'revision', + 'svn://nocommon/dist/rpms/foo/EL3', 'rpms/foo/EL3'] + call1 = mock.call(self.session, cmd[0], cmd, self.logfile, + self.uploadpath, cwd=self.tempdir, logerror=1, + append=False, env=None) + self.log_output.assert_has_calls([call1]) + + def test_checkout_svn_ssh(self): + + url = "svn+ssh://user@nocommon/dist?rpms/foo/EL3#revision" + scm = SCM(url) + scm.assert_allowed(self.allowed) + scm.checkout(self.tempdir, session=self.session, + uploadpath=self.uploadpath, logfile=self.logfile) + self.assertEqual(scm.use_common, False) + self.symlink.assert_not_called() + # expected commands + cmd = ['svn', 'checkout', '-r', 'revision', + 'svn+ssh://user@nocommon/dist/rpms/foo/EL3', 'rpms/foo/EL3'] + call1 = mock.call(self.session, cmd[0], cmd, self.logfile, + self.uploadpath, cwd=self.tempdir, logerror=1, + append=False, env=None) + self.log_output.assert_has_calls([call1]) From 38c45ee1240e5374427800d666b21840a487cf3b Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Sat, 13 May 2017 15:38:03 -0400 Subject: [PATCH 09/11] another bad url for assert_allowed test --- tests/test_scm.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/test_scm.py b/tests/test_scm.py index 4e013fd9..bb651868 100644 --- a/tests/test_scm.py +++ b/tests/test_scm.py @@ -81,14 +81,19 @@ class TestSCM(unittest.TestCase): bad = [ "cvs://badserver/projects/42#ref", "svn://badserver/projects/42#ref", + "git://maybeserver/badpath/project#1234", ] for url in good: scm = SCM(url) scm.assert_allowed(allowed) for url in bad: scm = SCM(url) - with self.assertRaises(koji.BuildError): + try: scm.assert_allowed(allowed) + except koji.BuildError: + pass + else: + raise AssertionError("allowed bad url: %s" % url) @mock.patch('logging.getLogger') def test_badrule(self, getLogger): From 3b66a81bfc3782f78605f0410e4e4725feed7c05 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Mon, 15 May 2017 12:48:32 -0400 Subject: [PATCH 10/11] rename variable in unit test. clarify docstring --- koji/daemon.py | 2 +- tests/test_scm.py | 55 +++++++++++++++++++++++------------------------ 2 files changed, 28 insertions(+), 29 deletions(-) diff --git a/koji/daemon.py b/koji/daemon.py index 913a2739..637566f4 100644 --- a/koji/daemon.py +++ b/koji/daemon.py @@ -290,7 +290,7 @@ class SCM(object): host:repository[:use_common[:source_cmd]] !host:repository - Incorrectly-formatted entries will be ignored. + Incorrectly-formatted entries will be skipped with a warning. The first form allows a host:repository pattern and optionally sets a few options for it. diff --git a/tests/test_scm.py b/tests/test_scm.py index bb651868..69120565 100644 --- a/tests/test_scm.py +++ b/tests/test_scm.py @@ -65,10 +65,9 @@ class TestSCM(unittest.TestCase): self.assertEqual(scm.source_cmd, ['make', 'sources']) self.assertEqual(scm.scmtype, 'GIT') - @mock.patch('logging.getLogger') def test_allowed(self, getLogger): - allowed = ''' + config = ''' goodserver:*:no !badserver:* !maybeserver:/badpath/* @@ -85,11 +84,11 @@ class TestSCM(unittest.TestCase): ] for url in good: scm = SCM(url) - scm.assert_allowed(allowed) + scm.assert_allowed(config) for url in bad: scm = SCM(url) try: - scm.assert_allowed(allowed) + scm.assert_allowed(config) except koji.BuildError: pass else: @@ -97,18 +96,18 @@ class TestSCM(unittest.TestCase): @mock.patch('logging.getLogger') def test_badrule(self, getLogger): - allowed = ''' + config = ''' bogus-entry-should-be-ignored goodserver:*:no !badserver:* ''' url = "git://goodserver/path1#1234" scm = SCM(url) - scm.assert_allowed(allowed) + scm.assert_allowed(config) @mock.patch('logging.getLogger') def test_opts(self, getLogger): - allowed = ''' + config = ''' default:* nocommon:*:no srccmd:*:no:fedpkg,sources @@ -120,73 +119,73 @@ class TestSCM(unittest.TestCase): url = "git://default/koji.git#1234" scm = SCM(url) - scm.assert_allowed(allowed) + scm.assert_allowed(config) self.assertEqual(scm.use_common, True) self.assertEqual(scm.source_cmd, ['make', 'sources']) url = "git://nocommon/koji.git#1234" scm = SCM(url) - scm.assert_allowed(allowed) + scm.assert_allowed(config) self.assertEqual(scm.use_common, False) self.assertEqual(scm.source_cmd, ['make', 'sources']) url = "git://srccmd/koji.git#1234" scm = SCM(url) - scm.assert_allowed(allowed) + scm.assert_allowed(config) self.assertEqual(scm.use_common, False) self.assertEqual(scm.source_cmd, ['fedpkg', 'sources']) url = "git://nosrc/koji.git#1234" scm = SCM(url) - scm.assert_allowed(allowed) + scm.assert_allowed(config) self.assertEqual(scm.use_common, False) self.assertEqual(scm.source_cmd, None) url = "git://mixed/foo/koji.git#1234" scm = SCM(url) - scm.assert_allowed(allowed) + scm.assert_allowed(config) self.assertEqual(scm.use_common, False) self.assertEqual(scm.source_cmd, ['make', 'sources']) url = "git://mixed/bar/koji.git#1234" scm = SCM(url) - scm.assert_allowed(allowed) + scm.assert_allowed(config) self.assertEqual(scm.use_common, True) self.assertEqual(scm.source_cmd, ['make', 'sources']) url = "git://mixed/baz/koji.git#1234" scm = SCM(url) - scm.assert_allowed(allowed) + scm.assert_allowed(config) self.assertEqual(scm.use_common, False) self.assertEqual(scm.source_cmd, ['fedpkg', 'sources']) url = "git://mixed/koji.git#1234" scm = SCM(url) with self.assertRaises(koji.BuildError): - scm.assert_allowed(allowed) + scm.assert_allowed(config) url = "git://mixed/foo/koji.git#1234" scm = SCM(url) - scm.assert_allowed(allowed) + scm.assert_allowed(config) self.assertEqual(scm.use_common, False) self.assertEqual(scm.source_cmd, ['make', 'sources']) url = "git://mixed/bar/koji.git#1234" scm = SCM(url) - scm.assert_allowed(allowed) + scm.assert_allowed(config) self.assertEqual(scm.use_common, True) self.assertEqual(scm.source_cmd, ['make', 'sources']) url = "git://mixed/baz/koji.git#1234" scm = SCM(url) - scm.assert_allowed(allowed) + scm.assert_allowed(config) self.assertEqual(scm.use_common, False) self.assertEqual(scm.source_cmd, ['fedpkg', 'sources']) url = "git://mixed/koji.git#1234" scm = SCM(url) with self.assertRaises(koji.BuildError): - scm.assert_allowed(allowed) + scm.assert_allowed(config) class TestSCMCheckouts(unittest.TestCase): @@ -200,7 +199,7 @@ class TestSCMCheckouts(unittest.TestCase): self.session = mock.MagicMock() self.uploadpath = mock.MagicMock() self.logfile = mock.MagicMock() - self.allowed = ''' + self.config = ''' default:* nocommon:*:no srccmd:*:no:fedpkg,sources @@ -215,7 +214,7 @@ class TestSCMCheckouts(unittest.TestCase): url = "git://nocommon/koji.git#asdasd" scm = SCM(url) - scm.assert_allowed(self.allowed) + scm.assert_allowed(self.config) scm.checkout(self.tempdir, session=self.session, uploadpath=self.uploadpath, logfile=self.logfile) self.assertEqual(scm.use_common, False) @@ -236,7 +235,7 @@ class TestSCMCheckouts(unittest.TestCase): url = "git+ssh://user@nocommon/koji.git#asdasd" scm = SCM(url) - scm.assert_allowed(self.allowed) + scm.assert_allowed(self.config) scm.checkout(self.tempdir, session=self.session, uploadpath=self.uploadpath, logfile=self.logfile) self.assertEqual(scm.use_common, False) @@ -257,7 +256,7 @@ class TestSCMCheckouts(unittest.TestCase): url = "git://default/koji.git#asdasd" scm = SCM(url) - scm.assert_allowed(self.allowed) + scm.assert_allowed(self.config) scm.checkout(self.tempdir, session=self.session, uploadpath=self.uploadpath, logfile=self.logfile) self.assertEqual(scm.use_common, True) @@ -282,7 +281,7 @@ class TestSCMCheckouts(unittest.TestCase): url = "git://nocommon/koji.git#asdasd" scm = SCM(url) - scm.assert_allowed(self.allowed) + scm.assert_allowed(self.config) self.log_output.return_value = 1 with self.assertRaises(koji.BuildError): scm.checkout(self.tempdir, session=self.session, @@ -302,7 +301,7 @@ class TestSCMCheckouts(unittest.TestCase): url = "cvs://default/cvsisdead?rpms/foo/EL3#sometag" scm = SCM(url) - scm.assert_allowed(self.allowed) + scm.assert_allowed(self.config) scm.checkout(self.tempdir, session=self.session, uploadpath=self.uploadpath, logfile=self.logfile) self.assertEqual(scm.use_common, True) @@ -324,7 +323,7 @@ class TestSCMCheckouts(unittest.TestCase): url = "cvs+ssh://user@nocommon/cvsisdead?rpms/foo/EL3#sometag" scm = SCM(url) - scm.assert_allowed(self.allowed) + scm.assert_allowed(self.config) scm.checkout(self.tempdir, session=self.session, uploadpath=self.uploadpath, logfile=self.logfile) self.assertEqual(scm.use_common, False) @@ -341,7 +340,7 @@ class TestSCMCheckouts(unittest.TestCase): url = "svn://nocommon/dist?rpms/foo/EL3#revision" scm = SCM(url) - scm.assert_allowed(self.allowed) + scm.assert_allowed(self.config) scm.checkout(self.tempdir, session=self.session, uploadpath=self.uploadpath, logfile=self.logfile) self.assertEqual(scm.use_common, False) @@ -358,7 +357,7 @@ class TestSCMCheckouts(unittest.TestCase): url = "svn+ssh://user@nocommon/dist?rpms/foo/EL3#revision" scm = SCM(url) - scm.assert_allowed(self.allowed) + scm.assert_allowed(self.config) scm.checkout(self.tempdir, session=self.session, uploadpath=self.uploadpath, logfile=self.logfile) self.assertEqual(scm.use_common, False) From 5f27889296c64f0cd8cfdcd3bd652c1de9e6270f Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Fri, 19 May 2017 10:46:43 -0400 Subject: [PATCH 11/11] update docs for allowed_scms --- docs/source/server_howto.rst | 43 +++++++++++++++++++++++++----------- 1 file changed, 30 insertions(+), 13 deletions(-) diff --git a/docs/source/server_howto.rst b/docs/source/server_howto.rst index 89166730..42cfda9f 100644 --- a/docs/source/server_howto.rst +++ b/docs/source/server_howto.rst @@ -1104,29 +1104,46 @@ By default it will look for the Kerberos keytab in ``/etc/kojid/kojid.keytab`` Kojid will not attempt kerberos authentication to the koji-hub unless the username field is commented out -Optional Configuration SourceCodeManagement -------------------------------------------- +Source Control Configuration +---------------------------- /etc/kojid/kojid.conf ^^^^^^^^^^^^^^^^^^^^^ -The pattern is as follows: - hostname:/path/match:checkout /common?, hostname2:/path/match:checkout /common? - -``checkout /common? is 'true' unless set to false`` +The *allowed_scms* setting controls which source control systems the builder +will accept. It is a space-separated list of entries in one of the following +forms: :: - allowed_scms=scm-server.example.com:/repo/base/repos*/:false + hostname:path[:use_common[:source_cmd]] + !hostname:path + +where + + *hostname* is a glob pattern matched against SCM hosts. + + *path* is a glob pattern matched against the SCM path. + + *use_common* is boolean setting (yes/no, on/off, true/false) that indicates + whether koji should also check out /common from the SCM host. The default + is on. + + *source_cmd* is a shell command to be run before building the + srpm, with commas instead of spaces. It defaults to ``make,sources``. + +The second form (``!hostname:path``) is used to explicitly block a host:path +pattern. In particular, it provides the option to block specific subtrees of +a host, but allow from it otherwise -Once the code is checked out kojid will run the following: :: - make sources - mv *.spec /SPECS - mv * /SOURCES - rpmbuild -bs /SPECS/*.spec + allowed_scms= + !scm-server.example.com:/blocked/path/* + scm-server.example.com:/repo/base/repos*/:no + alt-server.example.com:/repo/dist/repos*/:no:fedpkg,sources + Add the host to the createrepo channel -------------------------------------- @@ -1174,7 +1191,7 @@ You can check this by pointing your web browser to the web interface and clicking on the hosts tab. This will show you a list of builders in the database and the status of each builder. -Kojira - Dnd|Yum repository creation and maintenance +Kojira - Dnf|Yum repository creation and maintenance ==================================================== Configuration Files