From 71f1b22dda40437d80d07faf00189bf8ed83eb55 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 21 2016 17:37:49 +0000 Subject: [PATCH 1/13] provide some lower-level functions sans access check --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 304c106..124502b 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -1461,16 +1461,22 @@ def _tag_build(tag, build, user_id=None, force=False): else: # use the user associated with the current session user = get_user(context.session.user_id, strict=True) + #access check + assert_tag_access(tag['id'], user_id=user_id, force=force) + return _direct_tag_build(tag, build, user, force) + + +def _direct_tag_build(tag, build, user, force=False): + """Directly tag a build. No access check or user lookup.""" koji.plugin.run_callbacks('preTag', tag=tag, build=build, user=user, force=force) tag_id = tag['id'] build_id = build['id'] + user_id = user['id'] nvr = "%(name)s-%(version)s-%(release)s" % build if build['state'] != koji.BUILD_STATES['COMPLETE']: # incomplete builds may not be tagged, not even when forced state = koji.BUILD_STATES[build['state']] raise koji.TagError, "build %s not complete: state %s" % (nvr, state) - #access check - assert_tag_access(tag['id'], user_id=user_id, force=force) # see if it's already tagged retag = False table = 'tag_listing' @@ -2740,10 +2746,16 @@ def lookup_build_target(info, strict=False, create=False): """Get the id,name for build target""" return lookup_name('build_target', info, strict, create) + def create_tag(name, parent=None, arches=None, perm=None, locked=False, maven_support=False, maven_include_all=False, extra=None): """Create a new tag""" - context.session.assertPerm('admin') + return _create_tag(name, parent, arches, perm, locked, maven_support, maven_include_all, extra) + + +def _create_tag(name, parent=None, arches=None, perm=None, locked=False, maven_support=False, maven_include_all=False, extra=None): + """Create a new tag, without access check""" + if not context.opts.get('EnableMaven') and (maven_support or maven_include_all): raise koji.GenericError, "Maven support not enabled" From 06692a23cc1aebb8193b11abba151d6cbba6cd53 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 21 2016 17:37:49 +0000 Subject: [PATCH 2/13] also pkglist_add --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 124502b..9c46fee 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -856,6 +856,10 @@ def pkglist_add(taginfo, pkginfo, owner=None, block=None, extra_arches=None, for #don't check policy for admins using force if not (force and context.session.hasPerm('admin')): assert_policy('package_list', policy_data) + return _direct_pkglist_add(taginfo, pkginfo, owner, block, extra_arches, force, update) + + +def _direct_pkglist_add(taginfo, pkginfo, owner, block, extra_arches, force, update): if not pkg: pkg = lookup_package(pkginfo, create=True) koji.plugin.run_callbacks('prePackageListChange', action=action, tag=tag, package=pkg, owner=owner, From 12d339d58497a24183f8667268f4987770b7e337 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 21 2016 17:37:49 +0000 Subject: [PATCH 3/13] more pkglist... --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 9c46fee..e6886cc 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -836,6 +836,13 @@ def _pkglist_add(tag_id, pkg_id, owner, block, extra_arches): def pkglist_add(taginfo, pkginfo, owner=None, block=None, extra_arches=None, force=False, update=False): """Add to (or update) package list for tag""" + return _direct_pkglist_add(taginfo, pkginfo, owner, block, extra_arches, + force, update, policy=True) + + +def _direct_pkglist_add(taginfo, pkginfo, owner, block, extra_arches, force, + update, policy=False): + """Like pkglist_add, but without policy or access check""" #access control comes a little later (via an assert_policy) #should not make any changes until after policy is checked tag = get_tag(taginfo, strict=True) @@ -851,15 +858,12 @@ def pkglist_add(taginfo, pkginfo, owner=None, block=None, extra_arches=None, for action = 'update' elif bool(block): action = 'block' - context.session.assertLogin() - policy_data = {'tag' : tag_id, 'action' : action, 'package' : pkginfo, 'force' : force} - #don't check policy for admins using force - if not (force and context.session.hasPerm('admin')): - assert_policy('package_list', policy_data) - return _direct_pkglist_add(taginfo, pkginfo, owner, block, extra_arches, force, update) - - -def _direct_pkglist_add(taginfo, pkginfo, owner, block, extra_arches, force, update): + if policy: + context.session.assertLogin() + policy_data = {'tag' : tag_id, 'action' : action, 'package' : pkginfo, 'force' : force} + #don't check policy for admins using force + if not (force and context.session.hasPerm('admin')): + assert_policy('package_list', policy_data) if not pkg: pkg = lookup_package(pkginfo, create=True) koji.plugin.run_callbacks('prePackageListChange', action=action, tag=tag, package=pkg, owner=owner, From 4c6992a090f4cb4b2e430ca8dc0ee48570d83343 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 21 2016 17:37:49 +0000 Subject: [PATCH 4/13] unit test for create_tag() --- diff --git a/tests/test_hub/test_create_tag.py b/tests/test_hub/test_create_tag.py new file mode 100644 index 0000000..91dea25 --- /dev/null +++ b/tests/test_hub/test_create_tag.py @@ -0,0 +1,66 @@ +import copy +import mock +import shutil +import tempfile +import unittest + +import koji +import kojihub + +IP = kojihub.InsertProcessor + + +class TestCreateTag(unittest.TestCase): + + def getInsert(self, *args, **kwargs): + insert = IP(*args, **kwargs) + insert.execute = mock.MagicMock() + self.inserts.append(insert) + return insert + + def setUp(self): + self.InsertProcessor = mock.patch('kojihub.InsertProcessor', + side_effect=self.getInsert).start() + self.inserts = [] + self._dml = mock.patch('kojihub._dml').start() + self.get_tag = mock.patch('kojihub.get_tag').start() + self.get_tag_id = mock.patch('kojihub.get_tag_id').start() + self.writeInheritanceData = mock.patch('kojihub.writeInheritanceData').start() + self.context = mock.patch('kojihub.context').start() + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.context.session.assertPerm = mock.MagicMock() + self.context.session.assertLogin = mock.MagicMock() + + def tearDown(self): + mock.patch.stopall() + + def test_duplicate(self): + self.get_tag.return_value = {'name': 'duptag'} + with self.assertRaises(koji.GenericError): + kojihub.create_tag('duptag') + + def test_simple_create(self): + self.get_tag.return_value = None + self.get_tag_id.return_value = 99 + self.context.event_id = 42 + self.context.session.user_id = 23 + kojihub.create_tag('newtag') + + # check the insert + self.assertEqual(len(self.inserts), 1) + insert = self.inserts[0] + self.assertEqual(insert.table, 'tag_config') + values = { + 'arches': None, + 'create_event': 42, + 'creator_id': 23, + 'locked': False, + 'maven_include_all': False, + 'maven_support': False, + 'perm_id': None, + 'tag_id': 99, + } + self.assertEqual(insert.data, values) + self.assertEqual(insert.rawdata, {}) + insert = self.inserts[0] From 38a42e999756f37204f1c26b05a55f9d30f7c1bc Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 21 2016 17:37:49 +0000 Subject: [PATCH 5/13] stub test for _tag_build --- diff --git a/tests/test_hub/test_tag_build.py b/tests/test_hub/test_tag_build.py new file mode 100644 index 0000000..1245936 --- /dev/null +++ b/tests/test_hub/test_tag_build.py @@ -0,0 +1,89 @@ +import copy +import mock +import shutil +import tempfile +import unittest + +import koji +import kojihub + +QP = kojihub.QueryProcessor +UP = kojihub.UpdateProcessor +IP = kojihub.InsertProcessor + + +class TestTagBuild(unittest.TestCase): + + def getInsert(self, *args, **kwargs): + insert = IP(*args, **kwargs) + insert.execute = mock.MagicMock() + self.inserts.append(insert) + return insert + + def getUpdate(self, *args, **kwargs): + update = UP(*args, **kwargs) + update.execute = mock.MagicMock() + self.updates.append(updates) + return update + + def getQuery(self, *args, **kwargs): + query = QP(*args, **kwargs) + query.execute = mock.MagicMock() + query.executeOne = self.query_executeOne + query.iterate = mock.MagicMock() + self.queries.append(query) + return query + + def setUp(self): + self.InsertProcessor = mock.patch('kojihub.InsertProcessor', + side_effect=self.getInsert).start() + self.inserts = [] + self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor', + side_effect=self.getUpdate).start() + self.updates = [] + self.query_executeOne = mock.MagicMock() + self.QueryProcessor = mock.patch('kojihub.QueryProcessor', + side_effect=self.getQuery).start() + self.queries = [] + self._dml = mock.patch('kojihub._dml').start() + self.get_tag = mock.patch('kojihub.get_tag').start() + self.get_build = mock.patch('kojihub.get_build').start() + self.get_user = mock.patch('kojihub.get_user').start() + self.get_tag_id = mock.patch('kojihub.get_tag_id').start() + self.check_tag_access = mock.patch('kojihub.check_tag_access').start() + self.writeInheritanceData = mock.patch('kojihub.writeInheritanceData').start() + self.context = mock.patch('kojihub.context').start() + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.context.session.assertPerm = mock.MagicMock() + self.context.session.assertLogin = mock.MagicMock() + + def tearDown(self): + mock.patch.stopall() + + def test_simple_tag(self): + self.check_tag_access.return_value = (True, False, "") + self.get_build.return_value = { + 'id': 1, + 'name': 'name', + 'version': 'version', + 'release': 'release', + 'state': koji.BUILD_STATES['COMPLETE'], + } + # set return for the already tagged check + self.query_executeOne.return_value = None + + # call it + kojihub._tag_build('sometag', 'name-version-release') + + self.get_tag.called_once_with('sometag', strict=True) + + # check the insert + self.assertEqual(len(self.inserts), 1) + insert = self.inserts[0] + self.assertEqual(insert.table, 'tag_listing') + values = { + } + self.assertEqual(insert.data, values) + self.assertEqual(insert.rawdata, {}) + insert = self.inserts[0] From 3d0791ff1068abbd076ce15a6a55045e26de5cc9 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 21 2016 17:37:49 +0000 Subject: [PATCH 6/13] _direct_untag_build for consistency --- diff --git a/hub/kojihub.py b/hub/kojihub.py index e6886cc..02d2f35 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -1475,7 +1475,7 @@ def _tag_build(tag, build, user_id=None, force=False): def _direct_tag_build(tag, build, user, force=False): - """Directly tag a build. No access check or user lookup.""" + """Directly tag a build. No access check or value lookup.""" koji.plugin.run_callbacks('preTag', tag=tag, build=build, user=user, force=force) tag_id = tag['id'] build_id = build['id'] @@ -1527,10 +1527,15 @@ def _untag_build(tag, build, user_id=None, strict=True, force=False): else: # use the user associated with the current session user = get_user(context.session.user_id, strict=True) - koji.plugin.run_callbacks('preUntag', tag=tag, build=build, user=user, force=force, strict=strict) tag_id = tag['id'] build_id = build['id'] assert_tag_access(tag_id, user_id=user_id, force=force) + return _direct_untag_build(tag, build, user, strict, force) + + +def _direct_untag_build(tag, build, user, strict=True, force=False): + """Directly untag a build. No access check or value lookup.""" + koji.plugin.run_callbacks('preUntag', tag=tag, build=build, user=user, force=force, strict=strict) update = UpdateProcessor('tag_listing', values=locals(), clauses=['tag_id=%(tag_id)i', 'build_id=%(build_id)i']) update.make_revoke(user_id=user_id) From 63168e4a417c2f73ae117d290cbb802836d63b0a Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 21 2016 17:37:49 +0000 Subject: [PATCH 7/13] _direct_pkglist_remove --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 02d2f35..91876b2 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -922,17 +922,24 @@ def pkglist_remove(taginfo, pkginfo, force=False): The main reason to remove an entry like this is to remove an override so that the package data can be inherited from elsewhere. """ + _direct_pkglist_remove(taginfo, pkginfo, force, policy=True) + + +def _direct_pkglist_remove(taginfo, pkginfo, force=False, policy=False): + """Like pkglist_remove, but without policy check""" tag = get_tag(taginfo, strict=True) pkg = lookup_package(pkginfo, strict=True) - context.session.assertLogin() - policy_data = {'tag' : tag['id'], 'action' : 'remove', 'package' : pkg['id'], 'force' : force} - #don't check policy for admins using force - if not (force and context.session.hasPerm('admin')): - assert_policy('package_list', policy_data) + if policy: + context.session.assertLogin() + policy_data = {'tag' : tag['id'], 'action' : 'remove', 'package' : pkg['id'], 'force' : force} + #don't check policy for admins using force + if not (force and context.session.hasPerm('admin')): + assert_policy('package_list', policy_data) koji.plugin.run_callbacks('prePackageListChange', action='remove', tag=tag, package=pkg) _pkglist_remove(tag['id'], pkg['id']) koji.plugin.run_callbacks('postPackageListChange', action='remove', tag=tag, package=pkg) + def pkglist_block(taginfo, pkginfo): """Block the package in tag""" pkglist_add(taginfo, pkginfo, block=True) From 434bbb9c46fa9b471da70e9b735f9a94b97bc8bd Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 21 2016 17:37:49 +0000 Subject: [PATCH 8/13] more direct tag calls, also a fix --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 91876b2..8b684ca 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -1545,13 +1545,14 @@ def _direct_untag_build(tag, build, user, strict=True, force=False): koji.plugin.run_callbacks('preUntag', tag=tag, build=build, user=user, force=force, strict=strict) update = UpdateProcessor('tag_listing', values=locals(), clauses=['tag_id=%(tag_id)i', 'build_id=%(build_id)i']) - update.make_revoke(user_id=user_id) + update.make_revoke(user_id=user['id']) count = update.execute() if count == 0 and strict: nvr = "%(name)s-%(version)s-%(release)s" % build raise koji.TagError, "build %s not in tag %s" % (nvr, tag['name']) koji.plugin.run_callbacks('postUntag', tag=tag, build=build, user=user, force=force, strict=strict) + # tag-group operations # add # remove @@ -1559,10 +1560,16 @@ def _direct_untag_build(tag, build, user, strict=True, force=False): # unblock # list (readTagGroups) + def grplist_add(taginfo, grpinfo, block=False, force=False, **opts): """Add to (or update) group list for tag""" #only admins.... context.session.assertPerm('admin') + _grplist_add(taginfo, grpinfo, block, force, **opts) + + +def _grplist_add(taginfo, grpinfo, block, force, **opts): + """grplist_add without permission check""" tag = get_tag(taginfo) group = lookup_group(grpinfo, create=True) block = bool(block) @@ -1608,6 +1615,7 @@ def grplist_add(taginfo, grpinfo, block=False, force=False, **opts): insert.make_create() insert.execute() + def grplist_remove(taginfo, grpinfo, force=False): """Remove group from the list for tag @@ -1616,6 +1624,11 @@ def grplist_remove(taginfo, grpinfo, force=False): """ #only admins.... context.session.assertPerm('admin') + _grplist_remove(taginfo, grpinfo, force) + + +def _grplist_remove(taginfo, grpinfo, force): + """grplist_remove without permssion check""" tag = get_tag(taginfo) group = lookup_group(grpinfo, strict=True) tag_id = tag['id'] @@ -1625,10 +1638,12 @@ def grplist_remove(taginfo, grpinfo, force=False): update.make_revoke() update.execute() + def grplist_block(taginfo, grpinfo): """Block the group in tag""" grplist_add(taginfo, grpinfo, block=True) + def grplist_unblock(taginfo, grpinfo): """Unblock the group in tag @@ -1637,6 +1652,11 @@ def grplist_unblock(taginfo, grpinfo): """ # only admins... context.session.assertPerm('admin') + _grplist_unblock(taginfo, grpinfo) + + +def _grplist_unblock(taginfo, grpinfo): + """grplist_unblock without permssion check""" tag = lookup_tag(taginfo, strict=True) group = lookup_group(grpinfo, strict=True) tag_id = tag['id'] @@ -1661,10 +1681,16 @@ def grplist_unblock(taginfo, grpinfo): # unblock # list (readTagGroups) + def grp_pkg_add(taginfo, grpinfo, pkg_name, block=False, force=False, **opts): """Add package to group for tag""" #only admins.... context.session.assertPerm('admin') + _grp_pkg_add(taginfo, grpinfo, pkg_name, block, force, **opts) + + +def _grp_pkg_add(taginfo, grpinfo, pkg_name, block, force, **opts): + """grp_pkg_add without permssion checks""" tag = lookup_tag(taginfo, strict=True) group = lookup_group(grpinfo, strict=True) block = bool(block) @@ -1715,6 +1741,7 @@ def grp_pkg_add(taginfo, grpinfo, pkg_name, block=False, force=False, **opts): insert.make_create() insert.execute() + def grp_pkg_remove(taginfo, grpinfo, pkg_name, force=False): """Remove package from the list for group-tag @@ -1723,6 +1750,11 @@ def grp_pkg_remove(taginfo, grpinfo, pkg_name, force=False): """ #only admins.... context.session.assertPerm('admin') + _grp_pkg_remove(taginfo, grpinfo, pkg_name, force) + + +def _grp_pkg_remove(taginfo, grpinfo, pkg_name, force): + """grp_pkg_remove without permssion checks""" tag_id = get_tag_id(taginfo, strict=True) grp_id = get_group_id(grpinfo, strict=True) update = UpdateProcessor('group_package_listing', values=locals(), @@ -1730,10 +1762,12 @@ def grp_pkg_remove(taginfo, grpinfo, pkg_name, force=False): update.make_revoke() update.execute() + def grp_pkg_block(taginfo, grpinfo, pkg_name): """Block the package in group-tag""" grp_pkg_add(taginfo, grpinfo, pkg_name, block=True) + def grp_pkg_unblock(taginfo, grpinfo, pkg_name): """Unblock the package in group-tag @@ -1742,6 +1776,11 @@ def grp_pkg_unblock(taginfo, grpinfo, pkg_name): """ # only admins... context.session.assertPerm('admin') + _grp_pkg_unblock(taginfo, grpinfo, pkg_name) + + +def _grp_pkg_unblock(taginfo, grpinfo, pkg_name): + """grp_pkg_unblock without permission checks""" table = 'group_package_listing' tag_id = get_tag_id(taginfo, strict=True) grp_id = get_group_id(grpinfo, strict=True) @@ -1757,6 +1796,7 @@ def grp_pkg_unblock(taginfo, grpinfo, pkg_name): update.make_revoke() update.execute() + # tag-group-req operations # add # remove @@ -1764,10 +1804,16 @@ def grp_pkg_unblock(taginfo, grpinfo, pkg_name): # unblock # list (readTagGroups) + def grp_req_add(taginfo, grpinfo, reqinfo, block=False, force=False, **opts): """Add group requirement to group for tag""" #only admins.... context.session.assertPerm('admin') + _grp_req_add(taginfo, grpinfo, reqinfo, block, force, **opts) + + +def _grp_req_add(taginfo, grpinfo, reqinfo, block, force, **opts): + """grp_req_add without permssion checks""" tag = lookup_tag(taginfo, strict=True) group = lookup_group(grpinfo, strict=True, create=False) req = lookup_group(reqinfo, strict=True, create=False) @@ -1819,6 +1865,7 @@ def grp_req_add(taginfo, grpinfo, reqinfo, block=False, force=False, **opts): insert.make_create() insert.execute() + def grp_req_remove(taginfo, grpinfo, reqinfo, force=False): """Remove group requirement from the list for group-tag @@ -1827,6 +1874,11 @@ def grp_req_remove(taginfo, grpinfo, reqinfo, force=False): """ #only admins.... context.session.assertPerm('admin') + _grp_req_remove(taginfo, grpinfo, reqinfo, force) + + +def _grp_req_remove(taginfo, grpinfo, reqinfo, force): + """grp_req_remove without permission checks""" tag_id = get_tag_id(taginfo, strict=True) grp_id = get_group_id(grpinfo, strict=True) req_id = get_group_id(reqinfo, strict=True) @@ -1835,10 +1887,12 @@ def grp_req_remove(taginfo, grpinfo, reqinfo, force=False): update.make_revoke() update.execute() + def grp_req_block(taginfo, grpinfo, reqinfo): """Block the group requirement in group-tag""" grp_req_add(taginfo, grpinfo, reqinfo, block=True) + def grp_req_unblock(taginfo, grpinfo, reqinfo): """Unblock the group requirement in group-tag @@ -1847,6 +1901,11 @@ def grp_req_unblock(taginfo, grpinfo, reqinfo): """ # only admins... context.session.assertPerm('admin') + _grp_req_unblock(taginfo, grpinfo, reqinfo) + + +def _grp_req_unblock(taginfo, grpinfo, reqinfo): + """grp_req_unblock without permssion checks""" tag_id = get_tag_id(taginfo, strict=True) grp_id = get_group_id(grpinfo, strict=True) req_id = get_group_id(reqinfo, strict=True) @@ -1864,6 +1923,7 @@ def grp_req_unblock(taginfo, grpinfo, reqinfo): update.make_revoke() update.execute() + def get_tag_groups(tag, event=None, inherit=True, incl_pkgs=True, incl_reqs=True): """Return group data for the tag From 6f0685c07f26ecbfbd62c1e767ee65a2d3c0e2d4 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 21 2016 17:37:49 +0000 Subject: [PATCH 9/13] fix up test case --- diff --git a/tests/test_hub/test_tag_build.py b/tests/test_hub/test_tag_build.py index 1245936..721624a 100644 --- a/tests/test_hub/test_tag_build.py +++ b/tests/test_hub/test_tag_build.py @@ -70,6 +70,15 @@ class TestTagBuild(unittest.TestCase): 'release': 'release', 'state': koji.BUILD_STATES['COMPLETE'], } + self.get_tag.return_value = { + 'id': 777, + 'name': 'tag', + } + self.get_user.return_value = { + 'id': 999, + 'name': 'user', + } + self.context.event_id = 42 # set return for the already tagged check self.query_executeOne.return_value = None @@ -83,6 +92,10 @@ class TestTagBuild(unittest.TestCase): insert = self.inserts[0] self.assertEqual(insert.table, 'tag_listing') values = { + 'build_id': 1, + 'create_event': 42, + 'creator_id': 999, + 'tag_id': 777 } self.assertEqual(insert.data, values) self.assertEqual(insert.rawdata, {}) From ce59d83dc8592bfdb92789a82bf7da31ac9568a6 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 21 2016 17:37:49 +0000 Subject: [PATCH 10/13] test for _untag_build --- diff --git a/hub/kojihub.py b/hub/kojihub.py index 8b684ca..c17b1d4 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -1543,7 +1543,8 @@ def _untag_build(tag, build, user_id=None, strict=True, force=False): def _direct_untag_build(tag, build, user, strict=True, force=False): """Directly untag a build. No access check or value lookup.""" koji.plugin.run_callbacks('preUntag', tag=tag, build=build, user=user, force=force, strict=strict) - update = UpdateProcessor('tag_listing', values=locals(), + values = {'tag_id': tag['id'], 'build_id': build['id']} + update = UpdateProcessor('tag_listing', values=values, clauses=['tag_id=%(tag_id)i', 'build_id=%(build_id)i']) update.make_revoke(user_id=user['id']) count = update.execute() diff --git a/tests/test_hub/test_tag_build.py b/tests/test_hub/test_tag_build.py index 721624a..2f0d10c 100644 --- a/tests/test_hub/test_tag_build.py +++ b/tests/test_hub/test_tag_build.py @@ -23,7 +23,7 @@ class TestTagBuild(unittest.TestCase): def getUpdate(self, *args, **kwargs): update = UP(*args, **kwargs) update.execute = mock.MagicMock() - self.updates.append(updates) + self.updates.append(update) return update def getQuery(self, *args, **kwargs): @@ -86,6 +86,8 @@ class TestTagBuild(unittest.TestCase): kojihub._tag_build('sometag', 'name-version-release') self.get_tag.called_once_with('sometag', strict=True) + self.get_build.called_once_with('name-version-release', strict=True) + self.context.session.assertPerm.called_with('admin') # check the insert self.assertEqual(len(self.inserts), 1) @@ -100,3 +102,50 @@ class TestTagBuild(unittest.TestCase): self.assertEqual(insert.data, values) self.assertEqual(insert.rawdata, {}) insert = self.inserts[0] + + + def test_simple_untag(self): + self.check_tag_access.return_value = (True, False, "") + self.get_build.return_value = { + 'id': 1, + 'name': 'name', + 'version': 'version', + 'release': 'release', + 'state': koji.BUILD_STATES['COMPLETE'], + } + self.get_tag.return_value = { + 'id': 777, + 'name': 'tag', + } + self.get_user.return_value = { + 'id': 999, + 'name': 'user', + } + self.context.event_id = 42 + # set return for the already tagged check + self.query_executeOne.return_value = None + + # call it + kojihub._untag_build('sometag', 'name-version-release') + + self.get_tag.called_once_with('sometag', strict=True) + self.get_build.called_once_with('name-version-release', strict=True) + self.context.session.assertPerm.called_with('admin') + self.assertEqual(len(self.inserts), 0) + + # check the update + self.assertEqual(len(self.updates), 1) + update = self.updates[0] + self.assertEqual(update.table, 'tag_listing') + values = { + 'build_id': 1, + 'tag_id': 777 + } + data = { + 'revoke_event': 42, + 'revoker_id': 999, + } + self.assertEqual(update.rawdata, {'active': 'NULL'}) + self.assertEqual(update.data, data) + self.assertEqual(update.values, values) + update = self.updates[0] From caf17b6a3ebd3a7def022c1f9aff77b764142d2f Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 21 2016 17:37:49 +0000 Subject: [PATCH 11/13] rename test file --- diff --git a/tests/test_hub/test_tag_build.py b/tests/test_hub/test_tag_build.py deleted file mode 100644 index 2f0d10c..0000000 --- a/tests/test_hub/test_tag_build.py +++ /dev/null @@ -1,151 +0,0 @@ -import copy -import mock -import shutil -import tempfile -import unittest - -import koji -import kojihub - -QP = kojihub.QueryProcessor -UP = kojihub.UpdateProcessor -IP = kojihub.InsertProcessor - - -class TestTagBuild(unittest.TestCase): - - def getInsert(self, *args, **kwargs): - insert = IP(*args, **kwargs) - insert.execute = mock.MagicMock() - self.inserts.append(insert) - return insert - - def getUpdate(self, *args, **kwargs): - update = UP(*args, **kwargs) - update.execute = mock.MagicMock() - self.updates.append(update) - return update - - def getQuery(self, *args, **kwargs): - query = QP(*args, **kwargs) - query.execute = mock.MagicMock() - query.executeOne = self.query_executeOne - query.iterate = mock.MagicMock() - self.queries.append(query) - return query - - def setUp(self): - self.InsertProcessor = mock.patch('kojihub.InsertProcessor', - side_effect=self.getInsert).start() - self.inserts = [] - self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor', - side_effect=self.getUpdate).start() - self.updates = [] - self.query_executeOne = mock.MagicMock() - self.QueryProcessor = mock.patch('kojihub.QueryProcessor', - side_effect=self.getQuery).start() - self.queries = [] - self._dml = mock.patch('kojihub._dml').start() - self.get_tag = mock.patch('kojihub.get_tag').start() - self.get_build = mock.patch('kojihub.get_build').start() - self.get_user = mock.patch('kojihub.get_user').start() - self.get_tag_id = mock.patch('kojihub.get_tag_id').start() - self.check_tag_access = mock.patch('kojihub.check_tag_access').start() - self.writeInheritanceData = mock.patch('kojihub.writeInheritanceData').start() - self.context = mock.patch('kojihub.context').start() - # It seems MagicMock will not automatically handle attributes that - # start with "assert" - self.context.session.assertPerm = mock.MagicMock() - self.context.session.assertLogin = mock.MagicMock() - - def tearDown(self): - mock.patch.stopall() - - def test_simple_tag(self): - self.check_tag_access.return_value = (True, False, "") - self.get_build.return_value = { - 'id': 1, - 'name': 'name', - 'version': 'version', - 'release': 'release', - 'state': koji.BUILD_STATES['COMPLETE'], - } - self.get_tag.return_value = { - 'id': 777, - 'name': 'tag', - } - self.get_user.return_value = { - 'id': 999, - 'name': 'user', - } - self.context.event_id = 42 - # set return for the already tagged check - self.query_executeOne.return_value = None - - # call it - kojihub._tag_build('sometag', 'name-version-release') - - self.get_tag.called_once_with('sometag', strict=True) - self.get_build.called_once_with('name-version-release', strict=True) - self.context.session.assertPerm.called_with('admin') - - # check the insert - self.assertEqual(len(self.inserts), 1) - insert = self.inserts[0] - self.assertEqual(insert.table, 'tag_listing') - values = { - 'build_id': 1, - 'create_event': 42, - 'creator_id': 999, - 'tag_id': 777 - } - self.assertEqual(insert.data, values) - self.assertEqual(insert.rawdata, {}) - insert = self.inserts[0] - - - def test_simple_untag(self): - self.check_tag_access.return_value = (True, False, "") - self.get_build.return_value = { - 'id': 1, - 'name': 'name', - 'version': 'version', - 'release': 'release', - 'state': koji.BUILD_STATES['COMPLETE'], - } - self.get_tag.return_value = { - 'id': 777, - 'name': 'tag', - } - self.get_user.return_value = { - 'id': 999, - 'name': 'user', - } - self.context.event_id = 42 - # set return for the already tagged check - self.query_executeOne.return_value = None - - # call it - kojihub._untag_build('sometag', 'name-version-release') - - self.get_tag.called_once_with('sometag', strict=True) - self.get_build.called_once_with('name-version-release', strict=True) - self.context.session.assertPerm.called_with('admin') - self.assertEqual(len(self.inserts), 0) - - # check the update - self.assertEqual(len(self.updates), 1) - update = self.updates[0] - self.assertEqual(update.table, 'tag_listing') - values = { - 'build_id': 1, - 'tag_id': 777 - } - data = { - 'revoke_event': 42, - 'revoker_id': 999, - } - self.assertEqual(update.rawdata, {'active': 'NULL'}) - self.assertEqual(update.data, data) - self.assertEqual(update.values, values) - update = self.updates[0] diff --git a/tests/test_hub/test_tag_operations.py b/tests/test_hub/test_tag_operations.py new file mode 100644 index 0000000..2f0d10c --- /dev/null +++ b/tests/test_hub/test_tag_operations.py @@ -0,0 +1,151 @@ +import copy +import mock +import shutil +import tempfile +import unittest + +import koji +import kojihub + +QP = kojihub.QueryProcessor +UP = kojihub.UpdateProcessor +IP = kojihub.InsertProcessor + + +class TestTagBuild(unittest.TestCase): + + def getInsert(self, *args, **kwargs): + insert = IP(*args, **kwargs) + insert.execute = mock.MagicMock() + self.inserts.append(insert) + return insert + + def getUpdate(self, *args, **kwargs): + update = UP(*args, **kwargs) + update.execute = mock.MagicMock() + self.updates.append(update) + return update + + def getQuery(self, *args, **kwargs): + query = QP(*args, **kwargs) + query.execute = mock.MagicMock() + query.executeOne = self.query_executeOne + query.iterate = mock.MagicMock() + self.queries.append(query) + return query + + def setUp(self): + self.InsertProcessor = mock.patch('kojihub.InsertProcessor', + side_effect=self.getInsert).start() + self.inserts = [] + self.UpdateProcessor = mock.patch('kojihub.UpdateProcessor', + side_effect=self.getUpdate).start() + self.updates = [] + self.query_executeOne = mock.MagicMock() + self.QueryProcessor = mock.patch('kojihub.QueryProcessor', + side_effect=self.getQuery).start() + self.queries = [] + self._dml = mock.patch('kojihub._dml').start() + self.get_tag = mock.patch('kojihub.get_tag').start() + self.get_build = mock.patch('kojihub.get_build').start() + self.get_user = mock.patch('kojihub.get_user').start() + self.get_tag_id = mock.patch('kojihub.get_tag_id').start() + self.check_tag_access = mock.patch('kojihub.check_tag_access').start() + self.writeInheritanceData = mock.patch('kojihub.writeInheritanceData').start() + self.context = mock.patch('kojihub.context').start() + # It seems MagicMock will not automatically handle attributes that + # start with "assert" + self.context.session.assertPerm = mock.MagicMock() + self.context.session.assertLogin = mock.MagicMock() + + def tearDown(self): + mock.patch.stopall() + + def test_simple_tag(self): + self.check_tag_access.return_value = (True, False, "") + self.get_build.return_value = { + 'id': 1, + 'name': 'name', + 'version': 'version', + 'release': 'release', + 'state': koji.BUILD_STATES['COMPLETE'], + } + self.get_tag.return_value = { + 'id': 777, + 'name': 'tag', + } + self.get_user.return_value = { + 'id': 999, + 'name': 'user', + } + self.context.event_id = 42 + # set return for the already tagged check + self.query_executeOne.return_value = None + + # call it + kojihub._tag_build('sometag', 'name-version-release') + + self.get_tag.called_once_with('sometag', strict=True) + self.get_build.called_once_with('name-version-release', strict=True) + self.context.session.assertPerm.called_with('admin') + + # check the insert + self.assertEqual(len(self.inserts), 1) + insert = self.inserts[0] + self.assertEqual(insert.table, 'tag_listing') + values = { + 'build_id': 1, + 'create_event': 42, + 'creator_id': 999, + 'tag_id': 777 + } + self.assertEqual(insert.data, values) + self.assertEqual(insert.rawdata, {}) + insert = self.inserts[0] + + + def test_simple_untag(self): + self.check_tag_access.return_value = (True, False, "") + self.get_build.return_value = { + 'id': 1, + 'name': 'name', + 'version': 'version', + 'release': 'release', + 'state': koji.BUILD_STATES['COMPLETE'], + } + self.get_tag.return_value = { + 'id': 777, + 'name': 'tag', + } + self.get_user.return_value = { + 'id': 999, + 'name': 'user', + } + self.context.event_id = 42 + # set return for the already tagged check + self.query_executeOne.return_value = None + + # call it + kojihub._untag_build('sometag', 'name-version-release') + + self.get_tag.called_once_with('sometag', strict=True) + self.get_build.called_once_with('name-version-release', strict=True) + self.context.session.assertPerm.called_with('admin') + self.assertEqual(len(self.inserts), 0) + + # check the update + self.assertEqual(len(self.updates), 1) + update = self.updates[0] + self.assertEqual(update.table, 'tag_listing') + values = { + 'build_id': 1, + 'tag_id': 777 + } + data = { + 'revoke_event': 42, + 'revoker_id': 999, + } + self.assertEqual(update.rawdata, {'active': 'NULL'}) + self.assertEqual(update.data, data) + self.assertEqual(update.values, values) + update = self.updates[0] From b48e3f23a4377ee8c61fbb76201e23fa93a22157 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 21 2016 17:37:49 +0000 Subject: [PATCH 12/13] handle long type for tag ids --- diff --git a/hub/kojihub.py b/hub/kojihub.py index c17b1d4..a1785ab 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -2921,7 +2921,7 @@ def get_tag(tagInfo, strict=False, event=None): 'tag_config.maven_include_all': 'maven_include_all' } clauses = [eventCondition(event, table='tag_config')] - if isinstance(tagInfo, int): + if isinstance(tagInfo, (int, long)): clauses.append("tag.id = %(tagInfo)i") elif isinstance(tagInfo, basestring): clauses.append("tag.name = %(tagInfo)s") @@ -9312,7 +9312,7 @@ class RootExports(object): def listTagged(self, tag, event=None, inherit=False, prefix=None, latest=False, package=None, owner=None, type=None): """List builds tagged with tag""" - if not isinstance(tag, int): + if not isinstance(tag, (int, long)): #lookup tag id tag = get_tag_id(tag, strict=True) results = readTaggedBuilds(tag, event, inherit=inherit, latest=latest, package=package, owner=owner, type=type) @@ -9323,14 +9323,14 @@ class RootExports(object): def listTaggedRPMS(self, tag, event=None, inherit=False, latest=False, package=None, arch=None, rpmsigs=False, owner=None, type=None): """List rpms and builds within tag""" - if not isinstance(tag, int): + if not isinstance(tag, (int, long)): #lookup tag id tag = get_tag_id(tag, strict=True) return readTaggedRPMS(tag, event=event, inherit=inherit, latest=latest, package=package, arch=arch, rpmsigs=rpmsigs, owner=owner, type=type) def listTaggedArchives(self, tag, event=None, inherit=False, latest=False, package=None, type=None): """List archives and builds within a tag""" - if not isinstance(tag, int): + if not isinstance(tag, (int, long)): tag = get_tag_id(tag, strict=True) return readTaggedArchives(tag, event=event, inherit=inherit, latest=latest, package=package, type=type) @@ -9487,14 +9487,14 @@ class RootExports(object): def getLatestBuilds(self, tag, event=None, package=None, type=None): """List latest builds for tag (inheritance enabled)""" - if not isinstance(tag, int): + if not isinstance(tag, (int, long)): #lookup tag id tag = get_tag_id(tag, strict=True) return readTaggedBuilds(tag, event, inherit=True, latest=True, package=package, type=type) def getLatestRPMS(self, tag, package=None, arch=None, event=None, rpmsigs=False, type=None): """List latest RPMS for tag (inheritance enabled)""" - if not isinstance(tag, int): + if not isinstance(tag, (int, long)): #lookup tag id tag = get_tag_id(tag, strict=True) return readTaggedRPMS(tag, package=package, arch=arch, event=event, inherit=True, latest=True, rpmsigs=rpmsigs, type=type) @@ -9554,13 +9554,13 @@ class RootExports(object): def getInheritanceData(self, tag, event=None): """Return inheritance data for tag""" - if not isinstance(tag, int): + if not isinstance(tag, (int, long)): #lookup tag id tag = get_tag_id(tag, strict=True) return readInheritanceData(tag, event) def setInheritanceData(self, tag, data, clear=False): - if not isinstance(tag, int): + if not isinstance(tag, (int, long)): #lookup tag id tag = get_tag_id(tag, strict=True) context.session.assertPerm('admin') @@ -9571,7 +9571,7 @@ class RootExports(object): stops = {} if jumps is None: jumps = {} - if not isinstance(tag, int): + if not isinstance(tag, (int, long)): #lookup tag id tag = get_tag_id(tag, strict=True) for mapping in [stops, jumps]: @@ -9598,7 +9598,7 @@ class RootExports(object): - buildroot_id If no build has the given ID, or the build generated no RPMs, an empty list is returned.""" - if not isinstance(build, int): + if not isinstance(build, (int, long)): #lookup build id build = self.findBuildID(build, strict=True) return self.listRPMs(buildID=build) @@ -9963,7 +9963,7 @@ class RootExports(object): return taginfo def getRepo(self, tag, state=None, event=None): - if isinstance(tag, int): + if isinstance(tag, (int, long)): id = tag else: id = get_tag_id(tag, strict=True) From 8e1e72bf152e2a42e9de3c4716557b825809e8bf Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Nov 21 2016 17:37:49 +0000 Subject: [PATCH 13/13] include tag_extra data when deleting a tag --- diff --git a/hub/kojihub.py b/hub/kojihub.py index a1785ab..3ad9e19 100644 --- a/hub/kojihub.py +++ b/hub/kojihub.py @@ -3067,6 +3067,7 @@ def delete_tag(tagInfo): _tagDelete('tag_config', tagID) #technically, to 'delete' the tag we only have to revoke the tag_config entry #these remaining revocations are more for cleanup. + _tagDelete('tag_extra', tagID) _tagDelete('tag_inheritance', tagID) _tagDelete('tag_inheritance', tagID, 'parent_id') _tagDelete('build_target_config', tagID, 'build_tag')