Fix for reading config files when contains UTF-8 chars
Fixes: https://pagure.io/koji/issue/4191
This commit is contained in:
Jana Librova
parent
ad221feb8b
commit
729c954f3d
3 changed files with 96 additions and 82 deletions
|
|
@ -272,7 +272,6 @@ class ConfigFileTestCase(unittest.TestCase):
|
|||
mock.patch.stopall()
|
||||
|
||||
def test_read_config_files(self):
|
||||
|
||||
# bad config_files
|
||||
for files in [0,
|
||||
False,
|
||||
|
|
@ -298,7 +297,7 @@ class ConfigFileTestCase(unittest.TestCase):
|
|||
self.assertTrue(isinstance(conf,
|
||||
six.moves.configparser.ConfigParser.__class__))
|
||||
self.real_parser_clz.assert_called_once()
|
||||
self.real_parser_clz.return_value.read.assert_called_once_with([files])
|
||||
self.real_parser_clz.return_value.read.assert_called_once_with([files], encoding='utf8')
|
||||
|
||||
# list as config_files
|
||||
self.reset_mock()
|
||||
|
|
@ -306,7 +305,7 @@ class ConfigFileTestCase(unittest.TestCase):
|
|||
koji.read_config_files(files)
|
||||
|
||||
self.real_parser_clz.assert_called_once()
|
||||
self.real_parser_clz.return_value.read.assert_called_once_with(files)
|
||||
self.real_parser_clz.return_value.read.assert_called_once_with(files, encoding='utf8')
|
||||
|
||||
# tuple as config_files
|
||||
self.reset_mock()
|
||||
|
|
@ -369,7 +368,8 @@ class ConfigFileTestCase(unittest.TestCase):
|
|||
['test1.conf',
|
||||
'gooddir/test1-1.conf',
|
||||
'gooddir/test1-2.conf',
|
||||
'test2.conf'])
|
||||
'test2.conf'],
|
||||
encoding='utf8')
|
||||
self.assertEqual(self.manager.isdir.call_count, 5)
|
||||
self.assertEqual(self.manager.isfile.call_count, 6)
|
||||
self.assertEqual(self.manager.access.call_count, 4)
|
||||
|
|
@ -1282,7 +1282,8 @@ class TestRmtree(unittest.TestCase):
|
|||
|
||||
@patch('koji.util._rmtree')
|
||||
def test_rmtree_directory(self, _rmtree):
|
||||
""" Tests that the koji.util._rmtree_nofork function returns nothing when the path is a directory.
|
||||
""" Tests that the koji.util._rmtree_nofork function returns nothing
|
||||
when the path is a directory.
|
||||
"""
|
||||
stat = mock.MagicMock()
|
||||
stat.st_dev = 'dev'
|
||||
|
|
@ -1552,7 +1553,8 @@ class TestRmtree(unittest.TestCase):
|
|||
@mock.patch('os.unlink')
|
||||
@mock.patch('os.waitpid')
|
||||
@mock.patch('os._exit')
|
||||
def test_rmtree_parent_logfail(self, _exit, waitpid, unlink, fork, rmtree_nofork, logsend, mkstemp):
|
||||
def test_rmtree_parent_logfail(self, _exit, waitpid, unlink, fork, rmtree_nofork, logsend,
|
||||
mkstemp):
|
||||
log = self.tempdir + '/rmtree-log.jsonl'
|
||||
fd = os.open(log, os.O_RDWR | os.O_CREAT)
|
||||
mkstemp.return_value = fd, log
|
||||
|
|
@ -1844,6 +1846,7 @@ class TestRmtree2(unittest.TestCase):
|
|||
os.makedirs('%s/a/%s/c/d/%s/e/f/%s/g/h' % (dirname, i, j, k))
|
||||
|
||||
sync = threading.Event()
|
||||
|
||||
def do_rmtree(dirname):
|
||||
sync.wait()
|
||||
koji.util.rmtree(dirname)
|
||||
|
|
@ -1870,6 +1873,7 @@ class TestRmtree2(unittest.TestCase):
|
|||
os.makedirs('%s/a/%s/c/d/%s/e/f/%s/g/h' % (dirname, i, j, k))
|
||||
|
||||
sync = threading.Event()
|
||||
|
||||
def do_rmtree(dirname):
|
||||
sync.wait()
|
||||
koji.util.rmtree(dirname)
|
||||
|
|
@ -1895,6 +1899,7 @@ class TestRmtree2(unittest.TestCase):
|
|||
os.makedirs('%s/a/%s/c/d/%s/e/f/%s/g/h' % (dirname, i, j, k))
|
||||
|
||||
sync = multiprocessing.Event()
|
||||
|
||||
def do_rmtree(dirname):
|
||||
sync.wait()
|
||||
koji.util.rmtree(dirname)
|
||||
|
|
@ -2048,6 +2053,7 @@ class TestFormatShellCmd(unittest.TestCase):
|
|||
for inp, out in cases:
|
||||
self.assertEqual(koji.util.format_shell_cmd(inp, text_width=40), out)
|
||||
|
||||
|
||||
class TestExtractBuildTask(unittest.TestCase):
|
||||
def test_valid_binfos(self):
|
||||
binfos = [
|
||||
|
|
|
|||
|
|
@ -24,47 +24,47 @@ else:
|
|||
|
||||
|
||||
CONFIG1 = {
|
||||
'paths': {
|
||||
'default_mounts': '/mnt/archive,/mnt/workdir',
|
||||
'safe_roots': '/mnt/workdir/tmp',
|
||||
'path_subs':
|
||||
'/mnt/archive/prehistory/,/mnt/prehistoric_disk/archive/prehistory',
|
||||
},
|
||||
'path0': {
|
||||
'mountpoint': '/mnt/archive',
|
||||
'path': 'archive.org:/vol/archive',
|
||||
'fstype': 'nfs',
|
||||
'options': 'ro,hard,intr,nosuid,nodev,noatime,tcp',
|
||||
}}
|
||||
'paths': {
|
||||
'default_mounts': '/mnt/archive,/mnt/workdir',
|
||||
'safe_roots': '/mnt/workdir/tmp',
|
||||
'path_subs':
|
||||
'/mnt/archive/prehistory/,/mnt/prehistoric_disk/archive/prehistory',
|
||||
},
|
||||
'path0': {
|
||||
'mountpoint': '/mnt/archive',
|
||||
'path': 'archive.org:/vol/archive',
|
||||
'fstype': 'nfs',
|
||||
'options': 'ro,hard,intr,nosuid,nodev,noatime,tcp',
|
||||
}}
|
||||
|
||||
|
||||
CONFIG2 = {
|
||||
'paths': {
|
||||
'default_mounts': '/mnt/archive,/mnt/workdir',
|
||||
'safe_roots': '/mnt/workdir/tmp',
|
||||
'path_subs':
|
||||
'\n'
|
||||
'/mnt/archive/prehistory/,/mnt/prehistoric_disk/archive/prehistory\n'
|
||||
'/mnt/archve/workdir,/mnt/workdir\n',
|
||||
},
|
||||
'path0': {
|
||||
'mountpoint': '/mnt/archive',
|
||||
'path': 'archive.org:/vol/archive',
|
||||
'fstype': 'nfs',
|
||||
'options': 'ro,hard,intr,nosuid,nodev,noatime,tcp',
|
||||
},
|
||||
'path1': {
|
||||
'mountpoint': '/mnt/workdir',
|
||||
'path': 'archive.org:/vol/workdir',
|
||||
'fstype': 'nfs',
|
||||
'options': 'ro,hard,intr,nosuid,nodev,noatime,tcp',
|
||||
},
|
||||
'path2': {
|
||||
'mountpoint': '/mnt/prehistoric_disk',
|
||||
'path': 'archive.org:/vol/prehistoric_disk',
|
||||
'fstype': 'nfs',
|
||||
'options': 'ro,hard,intr,nosuid,nodev,noatime,tcp',
|
||||
}}
|
||||
'paths': {
|
||||
'default_mounts': '/mnt/archive,/mnt/workdir',
|
||||
'safe_roots': '/mnt/workdir/tmp',
|
||||
'path_subs':
|
||||
'\n'
|
||||
'/mnt/archive/prehistory/,/mnt/prehistoric_disk/archive/prehistory\n'
|
||||
'/mnt/archve/workdir,/mnt/workdir\n',
|
||||
},
|
||||
'path0': {
|
||||
'mountpoint': '/mnt/archive',
|
||||
'path': 'archive.org:/vol/archive',
|
||||
'fstype': 'nfs',
|
||||
'options': 'ro,hard,intr,nosuid,nodev,noatime,tcp',
|
||||
},
|
||||
'path1': {
|
||||
'mountpoint': '/mnt/workdir',
|
||||
'path': 'archive.org:/vol/workdir',
|
||||
'fstype': 'nfs',
|
||||
'options': 'ro,hard,intr,nosuid,nodev,noatime,tcp',
|
||||
},
|
||||
'path2': {
|
||||
'mountpoint': '/mnt/prehistoric_disk',
|
||||
'path': 'archive.org:/vol/prehistoric_disk',
|
||||
'fstype': 'nfs',
|
||||
'options': 'ro,hard,intr,nosuid,nodev,noatime,tcp',
|
||||
}}
|
||||
|
||||
|
||||
class FakeConfigParser(object):
|
||||
|
|
@ -75,7 +75,7 @@ class FakeConfigParser(object):
|
|||
else:
|
||||
self.CONFIG = copy.deepcopy(config)
|
||||
|
||||
def read(self, path):
|
||||
def read(self, path, encoding):
|
||||
return
|
||||
|
||||
def sections(self):
|
||||
|
|
@ -105,8 +105,7 @@ class TestRunrootConfig(unittest.TestCase):
|
|||
options.workdir = '/tmp/nonexistentdirectory'
|
||||
with self.assertRaises(koji.GenericError) as cm:
|
||||
runroot.RunRootTask(123, 'runroot', {}, session, options)
|
||||
self.assertEqual(cm.exception.args[0],
|
||||
"bad config: missing options in path0 section")
|
||||
self.assertEqual(cm.exception.args[0], "bad config: missing options in path0 section")
|
||||
|
||||
@mock.patch(CONFIG_PARSER)
|
||||
def test_bad_config_absolute_path(self, config_parser):
|
||||
|
|
@ -119,7 +118,8 @@ class TestRunrootConfig(unittest.TestCase):
|
|||
with self.assertRaises(koji.GenericError) as cm:
|
||||
runroot.RunRootTask(123, 'runroot', {}, session, options)
|
||||
self.assertEqual(cm.exception.args[0],
|
||||
"bad config: all paths (default_mounts, safe_roots, path_subs) needs to be absolute: ")
|
||||
"bad config: all paths (default_mounts, safe_roots, path_subs) "
|
||||
"needs to be absolute: ")
|
||||
|
||||
@mock.patch(CONFIG_PARSER)
|
||||
def test_valid_config(self, config_parser):
|
||||
|
|
@ -186,13 +186,14 @@ class TestMounts(unittest.TestCase):
|
|||
|
||||
# valid item
|
||||
self.assertEqual(self.t._get_path_params('/mnt/archive', 'rw'),
|
||||
('archive.org:/vol/archive/', '/mnt/archive', 'nfs', 'rw,hard,intr,nosuid,nodev,noatime,tcp'))
|
||||
('archive.org:/vol/archive/', '/mnt/archive', 'nfs',
|
||||
'rw,hard,intr,nosuid,nodev,noatime,tcp'))
|
||||
|
||||
@mock.patch('koji._open_text_file')
|
||||
@mock.patch('os.path.isdir')
|
||||
@mock.patch('runroot.log_output')
|
||||
def test_do_mounts(self, log_output, is_dir, open_mock):
|
||||
log_output.return_value = 0 # successful mount
|
||||
log_output.return_value = 0 # successful mount
|
||||
|
||||
# no mounts, don't do anything
|
||||
self.t.logger = mock.MagicMock()
|
||||
|
|
@ -202,8 +203,7 @@ class TestMounts(unittest.TestCase):
|
|||
# mountpoint has no absolute_path
|
||||
with self.assertRaises(koji.GenericError) as cm:
|
||||
self.t.do_mounts('rootdir', [('nfs:nfs', 'relative_path', 'nfs', '')])
|
||||
self.assertEqual(cm.exception.args[0],
|
||||
"invalid mount point: relative_path")
|
||||
self.assertEqual(cm.exception.args[0], "invalid mount point: relative_path")
|
||||
|
||||
# cover missing opts
|
||||
self.t.do_mounts('rootdir', [('nfs:nfs', '/mnt/archive', 'nfs', None)])
|
||||
|
|
@ -212,23 +212,24 @@ class TestMounts(unittest.TestCase):
|
|||
log_output.reset_mock()
|
||||
mounts = [self.t._get_path_params('/mnt/archive')]
|
||||
self.t.do_mounts('rootdir', mounts)
|
||||
log_output.assert_called_once_with(self.session, 'mount',
|
||||
['mount', '-t', 'nfs', '-o', 'ro,hard,intr,nosuid,nodev,noatime,tcp',
|
||||
'archive.org:/vol/archive/', 'rootdir/mnt/archive'],
|
||||
'/tmp/nonexistentdirectory/tasks/123/123/do_mounts.log',
|
||||
'tasks/123/123', append=True, logerror=True)
|
||||
log_output.assert_called_once_with(
|
||||
self.session, 'mount',
|
||||
['mount', '-t', 'nfs', '-o', 'ro,hard,intr,nosuid,nodev,noatime,tcp',
|
||||
'archive.org:/vol/archive/', 'rootdir/mnt/archive'],
|
||||
'/tmp/nonexistentdirectory/tasks/123/123/do_mounts.log', 'tasks/123/123',
|
||||
append=True, logerror=True)
|
||||
|
||||
# mount command failed
|
||||
log_output.reset_mock()
|
||||
log_output.return_value = 1
|
||||
#self.t.undo_mounts = mock.MagicMock()
|
||||
# self.t.undo_mounts = mock.MagicMock()
|
||||
mounts = [self.t._get_path_params('/mnt/archive')]
|
||||
with self.assertRaises(koji.GenericError) as cm:
|
||||
self.t.do_mounts('rootdir', mounts)
|
||||
self.assertEqual(cm.exception.args[0],
|
||||
'Unable to mount rootdir/mnt/archive: mount -t nfs -o'
|
||||
' ro,hard,intr,nosuid,nodev,noatime,tcp archive.org:/vol/archive/'
|
||||
' rootdir/mnt/archive was killed by signal 1')
|
||||
'Unable to mount rootdir/mnt/archive: mount -t nfs -o'
|
||||
' ro,hard,intr,nosuid,nodev,noatime,tcp archive.org:/vol/archive/'
|
||||
' rootdir/mnt/archive was killed by signal 1')
|
||||
|
||||
# bind ok
|
||||
log_output.return_value = 0
|
||||
|
|
@ -237,11 +238,12 @@ class TestMounts(unittest.TestCase):
|
|||
mount[3] += ',bind'
|
||||
is_dir.return_value = True
|
||||
self.t.do_mounts('rootdir', [mount])
|
||||
log_output.assert_called_once_with(self.session, 'mount',
|
||||
['mount', '-t', 'none', '-o', 'ro,hard,intr,nosuid,nodev,noatime,tcp,bind',
|
||||
'archive.org:/vol/archive/', 'rootdir/mnt/archive'],
|
||||
'/tmp/nonexistentdirectory/tasks/123/123/do_mounts.log',
|
||||
'tasks/123/123', append=True, logerror=True)
|
||||
log_output.assert_called_once_with(
|
||||
self.session, 'mount',
|
||||
['mount', '-t', 'none', '-o', 'ro,hard,intr,nosuid,nodev,noatime,tcp,bind',
|
||||
'archive.org:/vol/archive/', 'rootdir/mnt/archive'],
|
||||
'/tmp/nonexistentdirectory/tasks/123/123/do_mounts.log', 'tasks/123/123',
|
||||
append=True, logerror=True)
|
||||
|
||||
# bind - target doesn't exist
|
||||
mount = list(self.t._get_path_params('/mnt/archive'))
|
||||
|
|
@ -250,7 +252,7 @@ class TestMounts(unittest.TestCase):
|
|||
with self.assertRaises(koji.GenericError) as cm:
|
||||
self.t.do_mounts('rootdir', [mount])
|
||||
self.assertEqual(cm.exception.args[0],
|
||||
"No such directory or mount: archive.org:/vol/archive/")
|
||||
"No such directory or mount: archive.org:/vol/archive/")
|
||||
|
||||
# bg option forbidden
|
||||
log_output.reset_mock()
|
||||
|
|
@ -259,8 +261,7 @@ class TestMounts(unittest.TestCase):
|
|||
is_dir.return_value = False
|
||||
with self.assertRaises(koji.GenericError) as cm:
|
||||
self.t.do_mounts('rootdir', [mount])
|
||||
self.assertEqual(cm.exception.args[0],
|
||||
"bad config: background mount not allowed")
|
||||
self.assertEqual(cm.exception.args[0], "bad config: background mount not allowed")
|
||||
|
||||
def test_do_extra_mounts(self):
|
||||
self.t.do_mounts = mock.MagicMock()
|
||||
|
|
@ -288,7 +289,6 @@ class TestMounts(unittest.TestCase):
|
|||
self.t.do_extra_mounts('rootdir', ['/mnt/workdir/tmp/../xyz'])
|
||||
self.t.do_mounts.assert_not_called()
|
||||
|
||||
|
||||
@mock.patch('koji._open_text_file')
|
||||
@mock.patch('runroot.scan_mounts')
|
||||
@mock.patch('os.unlink')
|
||||
|
|
@ -318,10 +318,11 @@ class TestMounts(unittest.TestCase):
|
|||
with self.assertRaises(koji.GenericError) as cm:
|
||||
self.t.undo_mounts('rootdir')
|
||||
self.assertEqual(cm.exception.args[0],
|
||||
'Unable to unmount: mountpoint: error, mount_2: error, mount_1: error')
|
||||
'Unable to unmount: mountpoint: error, mount_2: error, mount_1: error')
|
||||
|
||||
os_unlink.assert_not_called()
|
||||
|
||||
|
||||
class TestHandler(unittest.TestCase):
|
||||
@mock.patch(CONFIG_PARSER)
|
||||
def setUp(self, config_parser):
|
||||
|
|
@ -335,7 +336,7 @@ class TestHandler(unittest.TestCase):
|
|||
|
||||
options = mock.MagicMock()
|
||||
options.workdir = '/tmp/nonexistentdirectory'
|
||||
#options.topurl = 'http://topurl'
|
||||
# options.topurl = 'http://topurl'
|
||||
options.topurls = None
|
||||
self.t = runroot.RunRootTask(123, 'runroot', {}, self.session, options)
|
||||
self.t.config['default_mounts'] = ['default_mount']
|
||||
|
|
@ -371,10 +372,9 @@ class TestHandler(unittest.TestCase):
|
|||
'id': 1,
|
||||
}
|
||||
self.session.host.getHost.return_value = {'arches': 'x86_64'}
|
||||
self.t.handler('tag_name', 'noarch', 'command', weight=10.0,
|
||||
repo_id=1, packages=['rpm_a', 'rpm_b'], new_chroot=True,
|
||||
mounts=['/mnt/a'], skip_setarch=True,
|
||||
upload_logs=['log_1', 'log_2'])
|
||||
self.t.handler('tag_name', 'noarch', 'command', weight=10.0, repo_id=1,
|
||||
packages=['rpm_a', 'rpm_b'], new_chroot=True, mounts=['/mnt/a'],
|
||||
skip_setarch=True, upload_logs=['log_1', 'log_2'])
|
||||
|
||||
# calls
|
||||
self.session.host.setTaskWeight.assert_called_once_with(self.t.id, 10.0)
|
||||
|
|
@ -382,15 +382,18 @@ class TestHandler(unittest.TestCase):
|
|||
self.session.getBuildConfig.assert_called_once_with('tag_name')
|
||||
self.session.repoInfo.assert_called_once_with(1, strict=True)
|
||||
self.session.host.subtask.assert_not_called()
|
||||
runroot.BuildRoot.assert_called_once_with(self.session, self.t.options,
|
||||
'tag_name', 'x86_64', self.t.id, repo_id=1, setup_dns=True,
|
||||
internal_dev_setup=None)
|
||||
runroot.BuildRoot.assert_called_once_with(
|
||||
self.session, self.t.options, 'tag_name', 'x86_64', self.t.id, repo_id=1,
|
||||
setup_dns=True, internal_dev_setup=None)
|
||||
self.session.host.setBuildRootState.assert_called_once_with(678, 'BUILDING')
|
||||
self.br.mock.assert_has_calls([
|
||||
mock.call(['--install', 'rpm_a', 'rpm_b']),
|
||||
mock.call(['--new-chroot', '--arch', 'arch', '--chroot', '--', '/bin/sh', '-c', '{ command; } < /dev/null 2>&1 | /usr/bin/tee /builddir/runroot.log; exit ${PIPESTATUS[0]}']),
|
||||
mock.call(['--new-chroot', '--arch', 'arch', '--chroot', '--', '/bin/sh', '-c',
|
||||
'{ command; } < /dev/null 2>&1 | /usr/bin/tee /builddir/runroot.log; '
|
||||
'exit ${PIPESTATUS[0]}']),
|
||||
])
|
||||
self.session.host.updateBuildRootList.assert_called_once_with(678, self.br.getPackageList())
|
||||
self.session.host.updateBuildRootList.assert_called_once_with(
|
||||
678, self.br.getPackageList())
|
||||
self.t.do_mounts.assert_called_once_with('/rootdir', ['default_mount'])
|
||||
self.t.do_extra_mounts.assert_called_once_with('/rootdir', ['/mnt/a'])
|
||||
self.t.uploadFile.assert_has_calls([
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue