koji-wrapper: Run koji runroot with fresh credentials cache

If the koji profile we are using is configured to use keytab, we should
run koji executable with a fresh credentials cache. Otherwise we risk a
race condition as multiple processes will trample over the same
directory in /tmp/krbcc_0.

This is currently only implemented for calling `koji runroot`. We might
need to do it for other commands as well (currently there is a sleep to
avoid the race condition for other commands).

Fixes: https://pagure.io/releng/issue/6715
Signed-off-by: Lubomír Sedlář <lsedlar@redhat.com>
This commit is contained in:
Lubomír Sedlář 2017-04-24 18:44:04 +02:00
parent 7028399403
commit 059449e140
2 changed files with 63 additions and 4 deletions

View File

@ -19,12 +19,15 @@ import pipes
import re import re
import time import time
import threading import threading
import contextlib
import koji import koji
import rpmUtils.arch import rpmUtils.arch
from kobo.shortcuts import run from kobo.shortcuts import run
from ConfigParser import ConfigParser from ConfigParser import ConfigParser
from .. import util
class KojiWrapper(object): class KojiWrapper(object):
lock = threading.Lock() lock = threading.Lock()
@ -109,6 +112,19 @@ class KojiWrapper(object):
return cmd return cmd
@contextlib.contextmanager
def get_runroot_env(self):
"""Get environment variables for running a koji command.
If we are authenticated with a keytab, we need a fresh credentials
cache to avoid possible race condition.
"""
if getattr(self.koji_module.config, 'keytab', None):
with util.temp_dir(prefix='krb_ccache') as tempdir:
yield {'KRB5CCNAME': 'DIR:%s' % tempdir}
else:
yield {}
def run_runroot_cmd(self, command, log_file=None): def run_runroot_cmd(self, command, log_file=None):
""" """
Run koji runroot command and wait for results. Run koji runroot command and wait for results.
@ -117,7 +133,8 @@ class KojiWrapper(object):
contains the id, it will be captured and returned. contains the id, it will be captured and returned.
""" """
task_id = None task_id = None
retcode, output = run(command, can_fail=True, logfile=log_file, show_cmd=True) with self.get_runroot_env() as env:
retcode, output = run(command, can_fail=True, logfile=log_file, show_cmd=True, env=env)
if "--task-id" in command: if "--task-id" in command:
first_line = output.splitlines()[0] first_line = output.splitlines()[0]
if re.match(r'^\d+$', first_line): if re.match(r'^\d+$', first_line):

View File

@ -16,6 +16,12 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), ".."))
from pungi.wrappers.kojiwrapper import KojiWrapper, get_buildroot_rpms from pungi.wrappers.kojiwrapper import KojiWrapper, get_buildroot_rpms
class DumbMock(object):
def __init__(self, **kwargs):
for key, value in kwargs.iteritems():
setattr(self, key, value)
class KojiWrapperBaseTestCase(unittest.TestCase): class KojiWrapperBaseTestCase(unittest.TestCase):
def setUp(self): def setUp(self):
_, self.tmpfile = tempfile.mkstemp() _, self.tmpfile = tempfile.mkstemp()
@ -24,10 +30,9 @@ class KojiWrapperBaseTestCase(unittest.TestCase):
koji.krb_login = mock.Mock() koji.krb_login = mock.Mock()
koji.get_profile_module = mock.Mock( koji.get_profile_module = mock.Mock(
return_value=mock.Mock( return_value=mock.Mock(
config=mock.Mock( config=DumbMock(
server='koji.example.com',
authtype='kerberos', authtype='kerberos',
principal='testprincipal',
keytab='testkeytab',
krb_rdns=False, krb_rdns=False,
cert=''), cert=''),
pathinfo=mock.Mock( pathinfo=mock.Mock(
@ -46,6 +51,8 @@ class KojiWrapperBaseTestCase(unittest.TestCase):
class KojiWrapperTest(KojiWrapperBaseTestCase): class KojiWrapperTest(KojiWrapperBaseTestCase):
def test_krb_login_krb(self): def test_krb_login_krb(self):
self.koji.koji_module.config.keytab = 'testkeytab'
self.koji.koji_module.config.principal = 'testprincipal'
self.assertEqual(self.koji.koji_module.config.krb_rdns, False) self.assertEqual(self.koji.koji_module.config.krb_rdns, False)
self.koji.login() self.koji.login()
self.koji.koji_proxy.krb_login.assert_called_with('testprincipal', self.koji.koji_proxy.krb_login.assert_called_with('testprincipal',
@ -382,6 +389,10 @@ class RunrootKojiWrapperTest(KojiWrapperBaseTestCase):
result = self.koji.run_runroot_cmd(cmd) result = self.koji.run_runroot_cmd(cmd)
self.assertDictEqual(result, {'retcode': 0, 'output': output, 'task_id': None}) self.assertDictEqual(result, {'retcode': 0, 'output': output, 'task_id': None})
self.assertEqual(
run.call_args_list,
[mock.call(cmd, can_fail=True, env={}, logfile=None, show_cmd=True)]
)
@mock.patch('pungi.wrappers.kojiwrapper.run') @mock.patch('pungi.wrappers.kojiwrapper.run')
def test_run_runroot_cmd_with_task_id(self, run): def test_run_runroot_cmd_with_task_id(self, run):
@ -391,6 +402,10 @@ class RunrootKojiWrapperTest(KojiWrapperBaseTestCase):
result = self.koji.run_runroot_cmd(cmd) result = self.koji.run_runroot_cmd(cmd)
self.assertDictEqual(result, {'retcode': 0, 'output': output, 'task_id': 1234}) self.assertDictEqual(result, {'retcode': 0, 'output': output, 'task_id': 1234})
self.assertEqual(
run.call_args_list,
[mock.call(cmd, can_fail=True, env={}, logfile=None, show_cmd=True)]
)
@mock.patch('pungi.wrappers.kojiwrapper.run') @mock.patch('pungi.wrappers.kojiwrapper.run')
def test_run_runroot_cmd_with_task_id_and_fail(self, run): def test_run_runroot_cmd_with_task_id_and_fail(self, run):
@ -400,6 +415,10 @@ class RunrootKojiWrapperTest(KojiWrapperBaseTestCase):
result = self.koji.run_runroot_cmd(cmd) result = self.koji.run_runroot_cmd(cmd)
self.assertDictEqual(result, {'retcode': 1, 'output': output, 'task_id': None}) self.assertDictEqual(result, {'retcode': 1, 'output': output, 'task_id': None})
self.assertEqual(
run.call_args_list,
[mock.call(cmd, can_fail=True, env={}, logfile=None, show_cmd=True)]
)
@mock.patch('pungi.wrappers.kojiwrapper.run') @mock.patch('pungi.wrappers.kojiwrapper.run')
def test_run_runroot_cmd_with_task_id_and_fail_but_emit_id(self, run): def test_run_runroot_cmd_with_task_id_and_fail_but_emit_id(self, run):
@ -409,6 +428,29 @@ class RunrootKojiWrapperTest(KojiWrapperBaseTestCase):
result = self.koji.run_runroot_cmd(cmd) result = self.koji.run_runroot_cmd(cmd)
self.assertDictEqual(result, {'retcode': 1, 'output': output, 'task_id': 12345}) self.assertDictEqual(result, {'retcode': 1, 'output': output, 'task_id': 12345})
self.assertEqual(
run.call_args_list,
[mock.call(cmd, can_fail=True, env={}, logfile=None, show_cmd=True)]
)
@mock.patch('shutil.rmtree')
@mock.patch('tempfile.mkdtemp')
@mock.patch('pungi.wrappers.kojiwrapper.run')
def test_run_runroot_cmd_with_keytab(self, run, mkdtemp, rmtree):
# We mock rmtree to avoid deleing something we did not create.
mkdtemp.return_value = '/tmp/foo'
self.koji.koji_module.config.keytab = 'foo'
cmd = ['koji', 'runroot']
output = 'Output ...'
run.return_value = (0, output)
result = self.koji.run_runroot_cmd(cmd)
self.assertDictEqual(result, {'retcode': 0, 'output': output, 'task_id': None})
self.assertEqual(
run.call_args_list,
[mock.call(cmd, can_fail=True, env={'KRB5CCNAME': 'DIR:/tmp/foo'},
logfile=None, show_cmd=True)]
)
class RunBlockingCmdTest(KojiWrapperBaseTestCase): class RunBlockingCmdTest(KojiWrapperBaseTestCase):