PR#3628: Add checksum API - koji

		`@@ -6,4 +6,13 @@`
		`-- fix duplicate extension in archivetypes`
		`UPDATE archivetypes SET extensions = 'vhdx.gz vhdx.xz' WHERE name = 'vhdx-compressed';`

		`+ -- track checksum of rpms`
		`+ CREATE TABLE rpm_checksum (`
		`+ rpm_id INTEGER NOT NULL REFERENCES rpminfo(id),`
		`+ sigkey TEXT NOT NULL,`
		`+ checksum TEXT NOT NULL UNIQUE,`
		`+ checksum_type SMALLINT NOT NULL,`
		`+ PRIMARY KEY (rpm_id, sigkey, checksum_type)`
		`+ ) WITHOUT OIDS;`
		`+ CREATE INDEX rpm_checksum_rpm_id ON rpm_checksum(rpm_id);`
		`COMMIT;`

docs/schema.sql

file modified

		`@@ -965,5 +965,14 @@`
		`body JSON NOT NULL`
		`) WITHOUT OIDS;`

		`+ -- track checksum of rpms`
		`+ CREATE TABLE rpm_checksum (`
		`+ rpm_id INTEGER NOT NULL REFERENCES rpminfo(id),`
		`+ sigkey TEXT NOT NULL,`
		`+ checksum TEXT NOT NULL UNIQUE,`
		`+ checksum_type SMALLINT NOT NULL,`
		`+ PRIMARY KEY (rpm_id, sigkey, checksum_type)`
		`+ ) WITHOUT OIDS;`
		`+ CREATE INDEX rpm_checksum_rpm_id ON rpm_checksum(rpm_id);`

		`COMMIT WORK;`

docs/source/hub_conf.rst

file modified

+12

		`@@ -542,3 +542,15 @@`
		`Set regex for verify a user name and kerberos. User name and kerberos have`
		`in default set up allowed '@' and '/' chars on top of basic name regex`
		`for internal names. When regex string is empty, verifying is disabled.`
		`+`
		`+ Default checksums types`
		`+ ^^^^^^^^^^^^^^^^^^^^^^^`
		`+ We have default checksums types for create rpm checksums.`
		`+`
		`+ .. glossary::`
		`+ RPMDefaultChecksums`
		`+ Type: string`
		`+`
		+ Default: ``md5 sha256``
		`+`
		+ Set RPM default checksums type. Default value is set upt to ``md5 sha256``.

koji/__init__.py

file modified

+54 -10

		`@@ -57,6 +57,7 @@`
		`from fnmatch import fnmatch`

		`import dateutil.parser`
		`+ import io`
		`import requests`
		`import six`
		`import six.moves.configparser`
		`@@ -943,9 +944,56 @@`
		`return get_sigpacket_key_id(sig)`


		`- def splice_rpm_sighdr(sighdr, src, dst=None, bufsize=8192):`
		`+ class SplicedSigStreamReader(io.RawIOBase):`
		`+ def __init__(self, path, sighdr, bufsize):`
		`+ self.path = path`
		`+ self.sighdr = sighdr`
		`+ self.buf = None`
		`+ self.gen = self.generator()`
		`+ self.bufsize = bufsize`
		`+`
		`+ def generator(self):`
		`+ (start, size) = find_rpm_sighdr(self.path)`
		`+ with open(self.path, 'rb') as fo:`
		`+ # the part before the signature`
		`+ yield fo.read(start)`
		`+`
		`+ # the spliced signature`
		`+ yield self.sighdr`
		`+`
		`+ # skip original signature`
		`+ fo.seek(size, 1)`
		`+`
		`+ # the part after the signature`
		`+ while True:`
		`+ buf = fo.read(self.bufsize)`
		`+ if not buf:`
		`+ break`
		`+ yield buf`
		`+`
		`+ def readable(self):`
		`+ return True`
		`+`
		`+ def readinto(self, b):`
		`+ try:`
		`+ expected_buf_size = len(b)`
		`+ data = self.buf or next(self.gen)`
		`+ output = data[:expected_buf_size]`
		`+ self.buf = data[expected_buf_size:]`
		`+ b[:len(output)] = output`
		`+ return len(output)`
		`+ except StopIteration:`
		`+ return 0 # indicate EOF`
		`+`
		`+`
		`+ def spliced_sig_reader(path, sighdr, bufsize=8192):`
		`+ """Returns a file-like object whose contents have the new signature spliced in"""`
		`+ return io.BufferedReader(SplicedSigStreamReader(path, sighdr, bufsize), buffer_size=bufsize)`
		`+`
		`+`
		`+ def splice_rpm_sighdr(sighdr, src, dst=None, bufsize=8192, callback=None):`
		`"""Write a copy of an rpm with signature header spliced in"""`
		`- (start, size) = find_rpm_sighdr(src)`
		`+ reader = spliced_sig_reader(src, sighdr, bufsize=bufsize)`
		`if dst is not None:`
		`dirname = os.path.dirname(dst)`
		`os.makedirs(dirname, exist_ok=True)`
		`@@ -953,15 +1001,11 @@`
		`else:`
		`(fd, dst_temp) = tempfile.mkstemp()`
		`os.close(fd)`
		`- with open(src, 'rb') as src_fo, open(dst_temp, 'wb') as dst_fo:`
		`- dst_fo.write(src_fo.read(start))`
		`- dst_fo.write(sighdr)`
		`- src_fo.seek(size, 1)`
		`- while True:`
		`- buf = src_fo.read(bufsize)`
		`- if not buf:`
		`- break`
		`+ with open(dst_temp, 'wb') as dst_fo:`
		`+ for buf in reader:`
		`dst_fo.write(buf)`
		`+ if callback:`
		`+ callback(buf)`
		`if dst is not None:`
		`src_stats = os.stat(src)`
		`dst_temp_stats = os.stat(dst_temp)`

kojihub/app/hub.conf

file modified

		`@@ -141,3 +141,6 @@`
		`# MaxNameLengthInternal = 256`
		`# RegexNameInternal = ^[A-Za-z0-9/_.+-]+$`
		`# RegexUserName = ^[A-Za-z0-9/_.@-]+$`
		`+`
		`+ ## Determines default checksums`
		`+ # RPMDefaultChecksums = md5 sha256`

kojihub/kojihub.py

file modified

+176 -2

		`@@ -27,6 +27,7 @@`
		`import base64`
		`import builtins`
		`import calendar`
		`+ import copy`
		`import datetime`
		`import fcntl`
		`import fnmatch`
		`@@ -7817,6 +7818,8 @@`
		`rpm_id = rinfo['id']`
		`delete = DeleteProcessor(table='rpmsigs', clauses=clauses, values=locals())`
		`delete.execute()`
		`+ delete = DeleteProcessor(table='rpm_checksum', clauses=clauses, values=locals())`
		`+ delete.execute()`
		`binfo = get_build(rinfo['build_id'])`
		`builddir = koji.pathinfo.build(binfo)`
		`list_sigcaches = []`
		`@@ -7909,7 +7912,7 @@`
		`fd, temp = tempfile.mkstemp()`
		`os.close(fd)`
		`try:`
		`- koji.splice_rpm_sighdr(sighdr, rpm_path, temp)`
		`+ koji.splice_rpm_sighdr(sighdr, rpm_path, dst=temp)`
		`ts = rpm.TransactionSet()`
		`ts.setVSFlags(0) # full verify`
		`with open(temp, 'rb') as fo:`
		`@@ -7968,8 +7971,53 @@`
		`return query.execute()`


		`+ class MultiSum(object):`
		`+`
		`+ def __init__(self, checksum_types):`
		`+ self.checksums = {name: getattr(hashlib, name)() for name in checksum_types}`
		`+`
		`+ def update(self, buf):`
		`+ for name, checksum in self.checksums.items():`
		`+ checksum.update(buf)`
		`+`
		`+ def to_hexdigest(self):`
		`+ checksums_hex = {}`
		`+ for name, checksum in self.checksums.items():`
		`+ checksums_hex[name] = checksum.hexdigest()`
		`+ return checksums_hex`
		`+`
		`+`
		`+ def calculate_chsum(path, checksum_types):`
		`+ """Calculate checksum for specific checksum_types`
		`+`
		`+ :param path: a string path to file`
		`+ a BufferedReader object`
		`+ :param list checksum_types: list of checksum types`
		`+ """`
		`+ msum = MultiSum(checksum_types)`
		`+ if isinstance(path, str):`
		`+ try:`
		`+ f = open(path, 'rb')`
		`+ except IOError as e:`
		`+ raise koji.GenericError(f"File {path} cannot be read -- {e}")`
		`+ else:`
		`+ f = path`
		`+ while 1:`
		`+ chunk = f.read(1024 ** 2)`
		`+ if not chunk:`
		`+ break`
		`+ msum.update(chunk)`
		`+ f.close()`
		`+ return msum.to_hexdigest()`
		`+`
		`+`
		`def write_signed_rpm(an_rpm, sigkey, force=False):`
		`"""Write a signed copy of the rpm"""`
		`+ checksum_types = context.opts.get('RPMDefaultChecksums').split()`
		`+ for ch_type in checksum_types:`
		`+ if ch_type not in koji.CHECKSUM_TYPES:`
		`+ raise koji.GenericError(f"Checksum_type {ch_type} isn't supported")`
		`+`
		`sigkey = sigkey.lower()`
		`rinfo = get_rpm(an_rpm, strict=True)`
		`if rinfo['external_repo_id']:`
		`@@ -7995,6 +8043,8 @@`
		`if os.path.exists(signedpath):`
		`if not force:`
		`# already present`
		`+ chsum_dict = calculate_chsum(signedpath, checksum_types)`
		`+ create_rpm_checksum(rpm_id, sigkey, chsum_dict)`
		`return`
		`else:`
		`os.unlink(signedpath)`
		`@@ -8002,7 +8052,9 @@`
		`with open(sigpath, 'rb') as fo:`
		`sighdr = fo.read()`
		`koji.ensuredir(os.path.dirname(signedpath))`
		`- koji.splice_rpm_sighdr(sighdr, rpm_path, signedpath)`
		`+ msum = MultiSum(checksum_types)`
		`+ koji.splice_rpm_sighdr(sighdr, rpm_path, dst=signedpath, callback=msum.update)`
		`+ create_rpm_checksum(rpm_id, sigkey, msum.to_hexdigest())`


		`def query_history(tables=None, **kwargs):`
		`@@ -8589,6 +8641,9 @@`
		`delete = DeleteProcessor(table='rpmsigs', clauses=['rpm_id=%(rpm_id)i'],`
		`values={'rpm_id': rpm_id})`
		`delete.execute()`
		`+ delete = DeleteProcessor(table='rpm_checksum', clauses=['rpm_id=%(rpm_id)i'],`
		`+ values={'rpm_id': rpm_id})`
		`+ delete.execute()`
		`values = {'build_id': build_id}`
		`update = UpdateProcessor('tag_listing', clauses=["build_id=%(build_id)i"], values=values)`
		`update.make_revoke()`
		`@@ -8638,6 +8693,9 @@`
		`delete = DeleteProcessor(table='archive_rpm_components', clauses=['rpm_id=%(rpm_id)i'],`
		`values={'rpm_id': rpm_id})`
		`delete.execute()`
		`+ delete = DeleteProcessor(table='rpm_checksum', clauses=['rpm_id=%(rpm_id)i'],`
		`+ values={'rpm_id': rpm_id})`
		`+ delete.execute()`
		`delete = DeleteProcessor(table='rpminfo', clauses=['build_id=%(id)i'],`
		`values={'id': binfo['build_id']})`
		`delete.execute()`
		`@@ -12270,6 +12328,72 @@`

		`queryRPMSigs = staticmethod(query_rpm_sigs)`

		`+ def getRPMChecksums(self, rpm_id, checksum_types=None, cacheonly=False):`
		`+ """Returns RPM checksums for specific rpm.`
		`+`
		`+ :param int rpm_id: RPM id`
		`+ :param list checksum_type: List of checksum types. Default sha256 checksum type`
		`+ :param bool cacheonly: when False, checksum is created for missing checksum type`
		`+ when True, checksum is returned as None when checksum is missing`
		`+ for specific checksum type`
		`+ :returns: A dict of specific checksum types and checksums`
		`+ """`
		`+ if not isinstance(rpm_id, int):`
		`+ raise koji.GenericError('rpm_id must be an integer')`
		`+ rpm_info = get_rpm(rpm_id, strict=True)`
		`+ if not checksum_types:`
		`+ checksum_types = context.opts.get('RPMDefaultChecksums').split()`
		`+ if not isinstance(checksum_types, list):`
		`+ raise koji.GenericError('checksum_type must be a list')`
		`+`
		`+ for ch_type in checksum_types:`
		`+ if ch_type not in koji.CHECKSUM_TYPES:`
		`+ raise koji.GenericError(f"Checksum_type {ch_type} isn't supported")`
		`+`
		`+ query = QueryProcessor(tables=['rpmsigs'], columns=['sigkey'],`
		`+ clauses=['rpm_id=%(rpm_id)i'], values={'rpm_id': rpm_id})`
		`+ sigkeys = [r['sigkey'] for r in query.execute()]`
		`+ if not sigkeys:`
		`+ return {}`
		`+`
		`+ list_checksums_sigkeys = {s: set(checksum_types) for s in sigkeys}`
		`+`
		`+ checksum_type_int = [koji.CHECKSUM_TYPES[chsum] for chsum in checksum_types]`
		`+ query_checksum = QueryProcessor(tables=['rpm_checksum'],`
		`+ columns=['checksum', 'checksum_type', 'sigkey'],`
		`+ clauses={'rpm_id=%(rpm_id)i',`
		`+ 'checksum_type IN %(checksum_type)s'},`
		`+ values={'rpm_id': rpm_id,`
		`+ 'checksum_type': checksum_type_int})`
		`+ query_result = query_checksum.execute()`
		`+ if (len(query_result) == (len(checksum_type_int) * len(sigkeys))) or cacheonly:`
		`+ return create_rpm_checksums_output(query_result, list_checksums_sigkeys)`
		`+ else:`
		`+ missing_chsum_sigkeys = copy.deepcopy(list_checksums_sigkeys)`
		`+ for r in query_result:`
		`+ if r['checksum_type'] in checksum_type_int and r['sigkey'] in sigkeys:`
		`+ missing_chsum_sigkeys[r['sigkey']].remove(`
		`+ koji.CHECKSUM_TYPES[r['checksum_type']])`
		`+`
		`+ if missing_chsum_sigkeys:`
		`+ binfo = get_build(rpm_info['build_id'])`
		`+ builddir = koji.pathinfo.build(binfo)`
		`+ rpm_path = joinpath(builddir, koji.pathinfo.rpm(rpm_info))`
		`+ for sigkey, chsums in missing_chsum_sigkeys.items():`
		`+ signedpath = joinpath(builddir, koji.pathinfo.signed(rpm_info, sigkey))`
		`+ if os.path.exists(signedpath):`
		`+ with open(signedpath, 'rb') as fo:`
		`+ chsums_dict = calculate_chsum(fo, chsums)`
		`+ else:`
		`+ sig_path = joinpath(builddir, koji.pathinfo.sighdr(rpm_info, sigkey))`
		`+ with open(sig_path, 'rb') as fo:`
		`+ sighdr = fo.read()`
		`+ with koji.spliced_sig_reader(rpm_path, sighdr) as fo:`
		`+ chsums_dict = calculate_chsum(fo, chsums)`
		`+ create_rpm_checksum(rpm_id, sigkey, chsums_dict)`
		`+ query_result = query_checksum.execute()`
		`+ return create_rpm_checksums_output(query_result, list_checksums_sigkeys)`
		`+`
		`def writeSignedRPM(self, an_rpm, sigkey, force=False):`
		`"""Write a signed copy of the rpm"""`
		`context.session.assertPerm('sign')`
		`@@ -15434,3 +15558,53 @@`
		`def verify_host_name(name):`
		`verify_name_internal(name)`
		`verify_name_user(name)`
		`+`
		`+`
		`+ def create_rpm_checksums_output(query_result, list_chsum_sigkeys):`
		`+ """Creates RPM checksum human-friendly dict.`
		`+`
		`+ :param dict query_result: Result of QueryProcessor`
		`+ :param list checksum_type: List of checksum types`
		`+ :return result: Human-friendly dict of checksums`
		`+ """`
		`+ result = {}`
		`+ for sigkey, chsums in list_chsum_sigkeys.items():`
		`+ result.setdefault(sigkey, dict(zip(chsums, [None] * len(chsums))))`
		`+ for r in query_result:`
		`+ result[r['sigkey']][koji.CHECKSUM_TYPES[r['checksum_type']]] = r['checksum']`
		`+ return result`
		`+`
		`+`
		`+ def create_rpm_checksum(rpm_id, sigkey, chsum_dict):`
		`+ """Creates RPM checksum.`
		`+`
		`+ :param int rpm_id: RPM id`
		`+ :param string sigkey: Sigkey for specific RPM`
		`+ :param dict chsum_dict: Dict of checksum type and hash.`
		`+ """`
		`+ chsum_dict = chsum_dict.copy()`
		`+`
		`+ checksum_type_int = [koji.CHECKSUM_TYPES[func] for func, _ in chsum_dict.items()]`
		`+ query = QueryProcessor(tables=['rpm_checksum'],`
		`+ columns=['checksum_type', 'checksum', 'sigkey', 'rpm_id'],`
		`+ clauses=["checksum_type IN %(checksum_types)s", 'sigkey=%(sigkey)s'],`
		`+ values={'checksum_types': checksum_type_int, 'sigkey': sigkey})`
		`+ rows = query.execute()`
		`+ if len(rows) == len(checksum_type_int):`
		`+ return None`
		`+ else:`
		`+ for r in rows:`
		`+ if r['checksum_type'] in checksum_type_int:`
		`+ if r['checksum'] == chsum_dict[koji.CHECKSUM_TYPES[r['checksum_type']]]:`
		`+ del chsum_dict[koji.CHECKSUM_TYPES[r['checksum_type']]]`
		`+ else:`
		`+ raise koji.GenericError(`
		`+ f"Calculate checksum is different than checksum in DB for "`
		`+ f"rpm ID {r['rpm_id']}, sigkey {r['sigkey']} and "`
		`+ f"checksum type {koji.CHECKSUM_TYPES[r['checksum_type']]}.")`
		`+ if chsum_dict:`
		`+ insert = BulkInsertProcessor(table='rpm_checksum')`
		`+ for func, chsum in chsum_dict.items():`
		`+ insert.add_record(rpm_id=rpm_id, sigkey=sigkey, checksum=chsum,`
		`+ checksum_type=koji.CHECKSUM_TYPES[func])`
		`+ insert.execute()`

kojihub/kojixmlrpc.py

file modified

+4 -1

		`@@ -497,7 +497,9 @@`

		`['MaxNameLengthInternal', 'integer', 256],`
		`['RegexNameInternal', 'string', r'^[A-Za-z0-9/_.+-]+$'],`
		`- ['RegexUserName', 'string', r'^[A-Za-z0-9/_.@-]+$']`
		`+ ['RegexUserName', 'string', r'^[A-Za-z0-9/_.@-]+$'],`
		`+`
		`+ ['RPMDefaultChecksums', 'string', 'md5 sha256']`
		`]`
		`opts = {}`
		`for name, dtype, default in cfgmap:`
		`@@ -532,6 +534,7 @@`
		`opts['RegexNameInternal.compiled'] = re.compile(opts['RegexNameInternal'])`
		`if opts['RegexUserName'] != '':`
		`opts['RegexUserName.compiled'] = re.compile(opts['RegexUserName'])`
		`+`
		`return opts`

tests/test_hub/test_create_rpm_checksum.py

file added

+39

		`@@ -0,0 +1,39 @@`
		`+ import unittest`
		`+ import mock`
		`+`
		`+ import kojihub`
		`+`
		`+ QP = kojihub.QueryProcessor`
		`+`
		`+`
		`+ class TestCreateRPMChecksum(unittest.TestCase):`
		`+ def setUp(self):`
		`+ self.maxDiff = None`
		`+ self.QueryProcessor = mock.patch('kojihub.kojihub.QueryProcessor',`
		`+ side_effect=self.getQuery).start()`
		`+ self.queries = []`
		`+ self.query_execute = mock.MagicMock()`
		`+`
		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`+ def getQuery(self, args, *kwargs):`
		`+ query = QP(args, *kwargs)`
		`+ query.execute = self.query_execute`
		`+ self.queries.append(query)`
		`+ return query`
		`+`
		`+ def test_checksum_exists(self):`
		`+ rpm_id = 123`
		`+ chsum_dict = {'md5': 'chsum-1', 'sha256': 'chsum-2'}`
		`+ sigkey = 'test-sigkey'`
		`+ self.query_execute.return_value = [{'checksum_type': 'md5'}, {'checksum_type': 'sha256'}]`
		`+ result = kojihub.create_rpm_checksum(rpm_id, sigkey, chsum_dict)`
		`+ self.assertIsNone(result)`
		`+`
		`+ self.assertEqual(len(self.queries), 1)`
		`+ query = self.queries[0]`
		`+ self.assertEqual(query.tables, ['rpm_checksum'])`
		`+ self.assertEqual(query.joins, None)`
		`+ self.assertEqual(query.clauses,`
		`+ ["checksum_type IN %(checksum_types)s", "sigkey=%(sigkey)s"])`

tests/test_hub/test_create_rpm_checksums_output.py

file added

+34

		`@@ -0,0 +1,34 @@`
		`+ import unittest`
		`+`
		`+ import kojihub`
		`+`
		`+`
		`+ class TestCreateRPMChecksumsOutput(unittest.TestCase):`
		`+ def setUp(self):`
		`+ self.maxDiff = None`
		`+ self.exports = kojihub.RootExports()`
		`+`
		`+ def test_cacheonly_all_exists(self):`
		`+ expected_result = {'sigkey1': {'md5': 'checksum-md5', 'sha256': 'checksum-sha256'},`
		`+ 'sigkey2': {'md5': 'checksum-md5', 'sha256': 'checksum-sha256'}}`
		`+ query_result = [{'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey1'},`
		`+ {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'sigkey1'},`
		`+ {'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey2'},`
		`+ {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'sigkey2'}]`
		`+ checksum_types = {'sigkey1': {'md5', 'sha256'},`
		`+ 'sigkey2': {'md5', 'sha256'}`
		`+ }`
		`+`
		`+ result = kojihub.create_rpm_checksums_output(query_result, checksum_types)`
		`+ self.assertEqual(expected_result, result)`
		`+`
		`+ def test_cacheonly_some_exists(self):`
		`+ expected_result = {'sigkey1': {'md5': 'checksum-md5', 'sha256': None},`
		`+ 'sigkey2': {'md5': None, 'sha256': 'checksum-sha256'}}`
		`+ query_result = [{'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey1'},`
		`+ {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'sigkey2'}]`
		`+ checksum_types = {'sigkey1': {'md5', 'sha256'},`
		`+ 'sigkey2': {'md5', 'sha256'}`
		`+ }`
		`+ result = kojihub.create_rpm_checksums_output(query_result, checksum_types)`
		`+ self.assertEqual(expected_result, result)`

tests/test_hub/test_delete_build.py

file modified

+93 -24

		`@@ -6,13 +6,58 @@`
		`import koji`
		`import kojihub`

		`+ DP = kojihub.DeleteProcessor`
		`+ QP = kojihub.QueryProcessor`
		`+ UP = kojihub.UpdateProcessor`
		`+`

		`class TestDeleteBuild(unittest.TestCase):`

		`- @mock.patch('kojihub.kojihub.context')`
		`- @mock.patch('kojihub.kojihub.get_build')`
		`- def test_delete_build_raise_error(self, build, context):`
		`- context.session.assertPerm = mock.MagicMock()`
		`+ def getDelete(self, args, *kwargs):`
		`+ delete = DP(args, *kwargs)`
		`+ delete.execute = mock.MagicMock()`
		`+ self.deletes.append(delete)`
		`+ return delete`
		`+`
		`+ def getQuery(self, args, *kwargs):`
		`+ query = QP(args, *kwargs)`
		`+ query.execute = self.query_execute`
		`+ self.queries.append(query)`
		`+ return query`
		`+`
		`+ def getUpdate(self, args, *kwargs):`
		`+ update = UP(args, *kwargs)`
		`+ update.execute = mock.MagicMock()`
		`+ self.updates.append(update)`
		`+ return update`
		`+`
		`+ def setUp(self):`
		`+ self.DeleteProcessor = mock.patch('kojihub.kojihub.DeleteProcessor',`
		`+ side_effect=self.getDelete).start()`
		`+ self.deletes = []`
		`+ self.QueryProcessor = mock.patch('kojihub.kojihub.QueryProcessor',`
		`+ side_effect=self.getQuery).start()`
		`+ self.queries = []`
		`+ self.query_execute = mock.MagicMock()`
		`+ self.UpdateProcessor = mock.patch('kojihub.kojihub.UpdateProcessor',`
		`+ side_effect=self.getUpdate).start()`
		`+ self.updates = []`
		`+ self.context_db = mock.patch('koji.db.context').start()`
		`+ self.context_db.session.assertLogin = mock.MagicMock()`
		`+ self.context_db.event_id = 42`
		`+ self.context_db.session.user_id = 24`
		`+ self.get_build = mock.patch('kojihub.kojihub.get_build').start()`
		`+ self._delete_build = mock.patch('kojihub.kojihub._delete_build').start()`
		`+ self.get_user = mock.patch('kojihub.kojihub.get_user').start()`
		`+ self.context = mock.patch('kojihub.kojihub.context').start()`
		`+ self.context.session.assertPerm = mock.MagicMock()`
		`+ self.binfo = {'id': 'BUILD ID', 'state': koji.BUILD_STATES['COMPLETE'], 'name': 'test_nvr',`
		`+ 'nvr': 'test_nvr-3.3-20.el8', 'version': '3.3', 'release': '20'}`
		`+`
		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`+ def test_delete_build_raise_error(self):`
		`references = ['tags', 'rpms', 'archives', 'component_of']`
		`for ref in references:`
		`context = mock.MagicMock()`
		`@@ -25,10 +70,7 @@`
		`with self.assertRaises(koji.GenericError):`
		`kojihub.delete_build(build='', strict=True)`

		`- @mock.patch('kojihub.kojihub.context')`
		`- @mock.patch('kojihub.kojihub.get_build')`
		`- def test_delete_build_return_false(self, build, context):`
		`- context.session.assertPerm = mock.MagicMock()`
		`+ def test_delete_build_return_false(self):`
		`references = ['tags', 'rpms', 'archives', 'component_of']`
		`for ref in references:`
		`context = mock.MagicMock()`
		`@@ -40,10 +82,7 @@`
		`refs.return_value = retval`
		`assert kojihub.delete_build(build='', strict=False) is False`

		`- @mock.patch('kojihub.kojihub.context')`
		`- @mock.patch('kojihub.kojihub.get_build')`
		`- def test_delete_build_check_last_used_raise_error(self, build, context):`
		`- context.session.assertPerm = mock.MagicMock()`
		`+ def test_delete_build_check_last_used_raise_error(self):`
		`references = ['tags', 'rpms', 'archives', 'component_of', 'last_used']`
		`for ref in references:`
		`context = mock.MagicMock()`
		`@@ -56,22 +95,52 @@`
		`refs.return_value = retval`
		`self.assertFalse(kojihub.delete_build(build='', strict=False))`

		`- @mock.patch('kojihub.kojihub.get_user')`
		`- @mock.patch('kojihub.kojihub._delete_build')`
		`@mock.patch('kojihub.kojihub.build_references')`
		`- @mock.patch('kojihub.kojihub.context')`
		`- @mock.patch('kojihub.kojihub.get_build')`
		`- def test_delete_build_lazy_refs(self, build, context, buildrefs, _delete, get_user):`
		`+ def test_delete_build_lazy_refs(self, buildrefs):`
		`'''Test that we can handle lazy return from build_references'''`
		`- get_user.return_value = {'authtype': 2, 'id': 1, 'krb_principal': None,`
		`+ self.get_user.return_value = {'authtype': 2, 'id': 1, 'krb_principal': None,`
		`'krb_principals': [], 'name': 'kojiadmin', 'status': 0,`
		`'usertype': 0}`
		`- context.session.assertPerm = mock.MagicMock()`
		`buildrefs.return_value = {'tags': []}`
		`- binfo = {'id': 'BUILD ID', 'state': koji.BUILD_STATES['COMPLETE'],`
		`- 'nvr': 'test_nvr-3.3-20.el8'}`
		`- build.return_value = binfo`
		`- kojihub.delete_build(build=binfo, strict=True)`
		`+ self.get_build.return_value = self.binfo`
		`+ kojihub.delete_build(build=self.binfo, strict=True)`

		`# no build refs, so we should have called _delete_build`
		`- _delete.assert_called_with(binfo)`
		`+ self._delete_build.assert_called_with(self.binfo)`
		`+`
		`+ def test_delete_build_queries(self):`
		`+ self.query_execute.return_value = [(123, )]`
		`+`
		`+ kojihub._delete_build(self.binfo)`
		`+`
		`+ self.assertEqual(len(self.queries), 1)`
		`+ query = self.queries[0]`
		`+ self.assertEqual(query.tables, ['rpminfo'])`
		`+ self.assertEqual(query.joins, None)`
		`+ self.assertEqual(query.clauses, ['build_id=%(build_id)i'])`
		`+ self.assertEqual(query.columns, ['id'])`
		`+`
		`+ self.assertEqual(len(self.deletes), 2)`
		`+ delete = self.deletes[0]`
		`+ self.assertEqual(delete.table, 'rpmsigs')`
		`+ self.assertEqual(delete.clauses, ["rpm_id=%(rpm_id)i"])`
		`+`
		`+ delete = self.deletes[1]`
		`+ self.assertEqual(delete.table, 'rpm_checksum')`
		`+ self.assertEqual(delete.clauses, ["rpm_id=%(rpm_id)i"])`
		`+`
		`+ self.assertEqual(len(self.updates), 2)`
		`+ update = self.updates[0]`
		`+ self.assertEqual(update.table, 'tag_listing')`
		`+ self.assertEqual(update.values, {'build_id': self.binfo['id']})`
		`+ self.assertEqual(update.data, {'revoke_event': 42, 'revoker_id': 24})`
		`+ self.assertEqual(update.rawdata, {'active': 'NULL'})`
		`+ self.assertEqual(update.clauses, ["build_id=%(build_id)i", 'active = TRUE'])`
		`+`
		`+ update = self.updates[1]`
		`+ self.assertEqual(update.table, 'build')`
		`+ self.assertEqual(update.values, {'build_id': self.binfo['id']})`
		`+ self.assertEqual(update.data, {'state': 2})`
		`+ self.assertEqual(update.rawdata, {})`
		`+ self.assertEqual(update.clauses, ['id=%(build_id)i'])`
		`+`

tests/test_hub/test_delete_rpm_sig.py

file modified

+18 -1

		`@@ -119,9 +119,15 @@`
		`self.query_rpm_sigs.return_value = self.queryrpmsigs`
		`r = kojihub.delete_rpm_sig(rpminfo, sigkey='testkey')`
		`self.assertEqual(r, None)`
		`+`
		`+ self.assertEqual(len(self.deletes), 2)`
		`delete = self.deletes[0]`
		`self.assertEqual(delete.table, 'rpmsigs')`
		`self.assertEqual(delete.clauses, ["rpm_id=%(rpm_id)i", "sigkey=%(sigkey)s"])`
		`+`
		`+ delete = self.deletes[1]`
		`+ self.assertEqual(delete.table, 'rpm_checksum')`
		`+ self.assertEqual(delete.clauses, ["rpm_id=%(rpm_id)i", "sigkey=%(sigkey)s"])`
		`self.get_rpm.assert_called_once_with(rpminfo, strict=True)`
		`self.query_rpm_sigs.assert_called_once_with(rpm_id=self.rinfo['id'], sigkey='testkey')`
		`self.get_build.assert_called_once_with(self.rinfo['build_id'])`
		`@@ -138,9 +144,15 @@`
		`with self.assertRaises(koji.GenericError) as ex:`
		`kojihub.delete_rpm_sig(rpminfo, all_sigs=True)`
		`self.assertEqual(ex.exception.args[0], expected_msg)`
		`+`
		`+ self.assertEqual(len(self.deletes), 2)`
		`delete = self.deletes[0]`
		`self.assertEqual(delete.table, 'rpmsigs')`
		`self.assertEqual(delete.clauses, ["rpm_id=%(rpm_id)i"])`
		`+`
		`+ delete = self.deletes[1]`
		`+ self.assertEqual(delete.table, 'rpm_checksum')`
		`+ self.assertEqual(delete.clauses, ["rpm_id=%(rpm_id)i"])`
		`self.get_rpm.assert_called_once_with(rpminfo, strict=True)`
		`self.query_rpm_sigs.assert_called_once_with(rpm_id=self.rinfo['id'], sigkey=None)`
		`self.get_build.assert_called_once_with(self.rinfo['build_id'])`
		`@@ -154,10 +166,15 @@`
		`self.get_user.return_value = self.userinfo`
		`self.query_rpm_sigs.return_value = self.queryrpmsigs`
		`kojihub.delete_rpm_sig(rpminfo, all_sigs=True)`
		`- self.assertEqual(len(self.deletes), 1)`
		`+`
		`+ self.assertEqual(len(self.deletes), 2)`
		`delete = self.deletes[0]`
		`self.assertEqual(delete.table, 'rpmsigs')`
		`self.assertEqual(delete.clauses, ["rpm_id=%(rpm_id)i"])`
		`+`
		`+ delete = self.deletes[1]`
		`+ self.assertEqual(delete.table, 'rpm_checksum')`
		`+ self.assertEqual(delete.clauses, ["rpm_id=%(rpm_id)i"])`
		`self.get_rpm.assert_called_once_with(rpminfo, strict=True)`
		`self.query_rpm_sigs.assert_called_once_with(rpm_id=self.rinfo['id'], sigkey=None)`
		`self.get_build.assert_called_once_with(self.rinfo['build_id'])`

tests/test_hub/test_get_rpm_checksums.py

file added

+252

		`@@ -0,0 +1,252 @@`
		`+ import unittest`
		`+ import mock`
		`+`
		`+ import koji`
		`+ import kojihub`
		`+`
		`+ QP = kojihub.QueryProcessor`
		`+`
		`+`
		`+ class TestGetRpmChecksums(unittest.TestCase):`
		`+ def setUp(self):`
		`+ self.maxDiff = None`
		`+ self.exports = kojihub.RootExports()`
		`+ self.os_path = mock.patch('os.path.exists').start()`
		`+ self.create_rpm_checksum_output = mock.patch(`
		`+ 'kojihub.kojihub.create_rpm_checksums_output').start()`
		`+ self.create_rpm_checksum = mock.patch('kojihub.kojihub.create_rpm_checksum').start()`
		`+ self.calculate_chsum = mock.patch('kojihub.kojihub.calculate_chsum').start()`
		`+ self.get_rpm = mock.patch('kojihub.kojihub.get_rpm').start()`
		`+ self.get_build = mock.patch('kojihub.kojihub.get_build').start()`
		`+ self.QueryProcessor = mock.patch('kojihub.kojihub.QueryProcessor',`
		`+ side_effect=self.getQuery).start()`
		`+ self.queries = []`
		`+ self.query_execute = mock.MagicMock()`
		`+ self.rpm_info = {'id': 123, 'name': 'test-name', 'version': '1.1', 'release': '123',`
		`+ 'arch': 'arch', 'build_id': 3}`
		`+ self.nvra = "%(name)s-%(version)s-%(release)s.%(arch)s" % self.rpm_info`
		`+ self.build_info = {'build_id': 3, 'name': 'test-name', 'version': '1.1', 'release': '123'}`
		`+`
		`+ def tearDown(self):`
		`+ mock.patch.stopall()`
		`+`
		`+ def getQuery(self, args, *kwargs):`
		`+ query = QP(args, *kwargs)`
		`+ query.execute = self.query_execute`
		`+ self.queries.append(query)`
		`+ return query`
		`+`
		`+ def test_rpm_id_not_str(self):`
		`+ rpm_id = ['123']`
		`+ with self.assertRaises(koji.GenericError) as ex:`
		`+ self.exports.getRPMChecksums(rpm_id)`
		`+ self.assertEqual('rpm_id must be an integer', str(ex.exception))`
		`+ self.get_rpm.assert_not_called()`
		`+ self.calculate_chsum.assert_not_called()`
		`+ self.create_rpm_checksum.assert_not_called()`
		`+ self.create_rpm_checksum_output.assert_not_called()`
		`+`
		`+ def test_checksum_types_not_list(self):`
		`+ rpm_id = 123`
		`+ self.get_rpm.return_value = self.rpm_info`
		`+ checksum_types = 'type'`
		`+ with self.assertRaises(koji.GenericError) as ex:`
		`+ self.exports.getRPMChecksums(rpm_id, checksum_types=checksum_types)`
		`+ self.assertEqual('checksum_type must be a list', str(ex.exception))`
		`+ self.get_rpm.assert_called_once_with(rpm_id, strict=True)`
		`+ self.calculate_chsum.assert_not_called()`
		`+ self.create_rpm_checksum.assert_not_called()`
		`+ self.create_rpm_checksum_output.assert_not_called()`
		`+`
		`+ def test_checksum_types_wrong_type(self):`
		`+ rpm_id = 123`
		`+ checksum_types = ['md5', 'type']`
		`+ with self.assertRaises(koji.GenericError) as ex:`
		`+ self.exports.getRPMChecksums(rpm_id, checksum_types=checksum_types)`
		`+ self.assertEqual("Checksum_type type isn't supported", str(ex.exception))`
		`+ self.get_rpm.assert_called_once_with(rpm_id, strict=True)`
		`+ self.calculate_chsum.assert_not_called()`
		`+ self.create_rpm_checksum.assert_not_called()`
		`+ self.create_rpm_checksum_output.assert_not_called()`
		`+`
		`+ def test_all_checksum_exists(self):`
		`+ self.os_path.return_value = True`
		`+ rpm_id = 123`
		`+ self.get_rpm.return_value = self.rpm_info`
		`+ checksum_types = ['md5', 'sha256']`
		`+ expected_result = {'sigkey-1': {'md5': 'checksum-md5', 'sha256': 'checksum-sha256'}}`
		`+ self.query_execute.side_effect = [`
		`+ [{'sigkey': 'sigkey-1'}],`
		`+ [{'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey-1'},`
		`+ {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'sigkey-1'}]]`
		`+ self.create_rpm_checksum_output.return_value = expected_result`
		`+ result = self.exports.getRPMChecksums(rpm_id, checksum_types=checksum_types)`
		`+ self.assertEqual(len(self.queries), 2)`
		`+ query = self.queries[0]`
		`+ self.assertEqual(query.tables, ['rpmsigs'])`
		`+ self.assertEqual(query.joins, None)`
		`+ self.assertEqual(query.clauses, ['rpm_id=%(rpm_id)i'])`
		`+`
		`+ query = self.queries[1]`
		`+ self.assertEqual(query.tables, ['rpm_checksum'])`
		`+ self.assertEqual(query.joins, None)`
		`+ self.assertEqual(query.clauses,`
		`+ ['checksum_type IN %(checksum_type)s', 'rpm_id=%(rpm_id)i'])`
		`+ self.assertEqual(expected_result, result)`
		`+ self.get_rpm.assert_called_once_with(rpm_id, strict=True)`
		`+ self.calculate_chsum.assert_not_called()`
		`+ self.create_rpm_checksum.assert_not_called()`
		`+ self.create_rpm_checksum_output.assert_called_once_with(`
		`+ [{'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey-1'},`
		`+ {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'sigkey-1'}],`
		`+ {'sigkey-1': {'sha256', 'md5'}}`
		`+ )`
		`+`
		`+ def test_missing_checksum_not_sigkey(self):`
		`+ rpm_id = 123`
		`+ self.get_rpm.return_value = self.rpm_info`
		`+ checksum_types = ['md5']`
		`+ self.query_execute.side_effect = [[], []]`
		`+ result = self.exports.getRPMChecksums(rpm_id, checksum_types=checksum_types)`
		`+ self.assertEqual({}, result)`
		`+`
		`+ self.assertEqual(len(self.queries), 1)`
		`+ query = self.queries[0]`
		`+ self.assertEqual(query.tables, ['rpmsigs'])`
		`+ self.assertEqual(query.joins, None)`
		`+ self.assertEqual(query.clauses, ['rpm_id=%(rpm_id)i'])`
		`+`
		`+ self.get_rpm.assert_called_once_with(rpm_id, strict=True)`
		`+ self.calculate_chsum.assert_not_called()`
		`+ self.create_rpm_checksum.assert_not_called()`
		`+ self.create_rpm_checksum_output.assert_not_called()`
		`+`
		`+ @mock.patch('kojihub.kojihub.open')`
		`+ def test_missing_valid_all_checksum_generated(self, open):`
		`+ self.os_path.return_value = True`
		`+ rpm_id = 123`
		`+ checksum_types = ['md5']`
		`+ self.get_rpm.return_value = self.rpm_info`
		`+ self.get_build.return_value = self.build_info`
		`+ expected_result = {'sigkey-1': {'md5': 'checksum-md5'}}`
		`+ calculate_chsum_res = {'sigkey-1': {'md5': 'checksum-md5'}}`
		`+ self.query_execute.side_effect = [`
		`+ [{'sigkey': 'sigkey-1'}],`
		`+ [],`
		`+ [{'checksum': 'checksum-md5', 'checksum_type': 0}]]`
		`+ self.calculate_chsum.return_value = calculate_chsum_res`
		`+ self.create_rpm_checksum.return_value = None`
		`+ self.create_rpm_checksum_output.return_value = expected_result`
		`+ result = self.exports.getRPMChecksums(rpm_id, checksum_types=checksum_types)`
		`+ self.assertEqual(expected_result, result)`
		`+`
		`+ self.assertEqual(len(self.queries), 2)`
		`+ query = self.queries[0]`
		`+ self.assertEqual(query.tables, ['rpmsigs'])`
		`+ self.assertEqual(query.joins, None)`
		`+ self.assertEqual(query.clauses, ['rpm_id=%(rpm_id)i'])`
		`+`
		`+ query = self.queries[1]`
		`+ self.assertEqual(query.tables, ['rpm_checksum'])`
		`+ self.assertEqual(query.joins, None)`
		`+ self.assertEqual(query.clauses,`
		`+ ['checksum_type IN %(checksum_type)s', 'rpm_id=%(rpm_id)i'])`
		`+ self.get_rpm.assert_called_once_with(rpm_id, strict=True)`
		`+ self.assertEqual(self.calculate_chsum.call_count, 1)`
		`+ self.create_rpm_checksum.assert_called_once_with(rpm_id, 'sigkey-1', calculate_chsum_res)`
		`+ self.create_rpm_checksum_output.assert_called_once_with(`
		`+ [{'checksum': 'checksum-md5', 'checksum_type': 0}], {'sigkey-1': {'md5'}}`
		`+ )`
		`+`
		`+ @mock.patch('kojihub.kojihub.open')`
		`+ def test_missing_valid_more_checksum_generated_and_exists(self, open):`
		`+ self.os_path.return_value = True`
		`+ rpm_id = 123`
		`+ self.get_rpm.return_value = self.rpm_info`
		`+ self.get_build.return_value = self.build_info`
		`+ checksum_types = ['md5', 'sha256']`
		`+ expected_result = {'sigkey-1': {'md5': 'checksum-md5', 'sha256': 'checksum-sha256'}}`
		`+ calculate_chsum_res = {'sigkey-1': {'sha256': 'checksum-sha256'}}`
		`+ self.query_execute.side_effect = [`
		`+ [{'sigkey': 'sigkey-1'}],`
		`+ [{'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey-1'}],`
		`+ [{'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey-1'},`
		`+ {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'sigkey-1'}]]`
		`+ self.calculate_chsum.return_value = calculate_chsum_res`
		`+ self.create_rpm_checksum_output.return_value = expected_result`
		`+ self.create_rpm_checksum.return_value = None`
		`+ result = self.exports.getRPMChecksums(rpm_id, checksum_types=checksum_types)`
		`+ self.assertEqual(expected_result, result)`
		`+`
		`+ self.assertEqual(len(self.queries), 2)`
		`+ query = self.queries[0]`
		`+ self.assertEqual(query.tables, ['rpmsigs'])`
		`+ self.assertEqual(query.joins, None)`
		`+ self.assertEqual(query.clauses, ['rpm_id=%(rpm_id)i'])`
		`+`
		`+ query = self.queries[1]`
		`+ self.assertEqual(query.tables, ['rpm_checksum'])`
		`+ self.assertEqual(query.joins, None)`
		`+ self.assertEqual(query.clauses,`
		`+ ['checksum_type IN %(checksum_type)s', 'rpm_id=%(rpm_id)i'])`
		`+`
		`+ self.get_rpm.assert_called_once_with(rpm_id, strict=True)`
		`+ self.assertEqual(self.calculate_chsum.call_count, 1)`
		`+ self.create_rpm_checksum.assert_called_once_with(rpm_id, 'sigkey-1', calculate_chsum_res)`
		`+ self.create_rpm_checksum_output.assert_called_once_with(`
		`+ [{'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey-1'},`
		`+ {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'sigkey-1'}],`
		`+ {'sigkey-1': {'md5', 'sha256'}}`
		`+ )`
		`+`
		`+ @mock.patch('kojihub.kojihub.open')`
		`+ def test_missing_valid_more_checksum_generated_and_exists_more_sigkeys(self, open):`
		`+ self.os_path.return_value = True`
		`+ rpm_id = 123`
		`+ self.get_rpm.return_value = self.rpm_info`
		`+ self.get_build.return_value = self.build_info`
		`+ checksum_types = ['md5', 'sha256']`
		`+ expected_result = {'sigkey1': {'md5': 'checksum-md5', 'sha256': 'checksum-sha256'},`
		`+ 'sigkey2': {'md5': 'checksum-md5', 'sha256': 'checksum-sha256'}}`
		`+ calculate_chsum_res = [`
		`+ {'sigkey1': {'md5': 'checksum-md5', 'sha256': 'checksum-sha256'}},`
		`+ {'sigkey2': {'md5': 'checksum-md5', 'sha256': 'checksum-sha256'}},`
		`+ ]`
		`+ self.query_execute.side_effect = [`
		`+ [{'sigkey': 'sigkey1'}, {'sigkey': 'sigkey2'}],`
		`+ [{'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey1'},`
		`+ {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'sigkey2'}],`
		`+ [{'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey1'},`
		`+ {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'sigkey1'},`
		`+ {'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey2'},`
		`+ {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'sigkey2'}]]`
		`+ self.calculate_chsum.side_effect = calculate_chsum_res`
		`+ self.create_rpm_checksum_output.return_value = expected_result`
		`+ self.create_rpm_checksum.side_effect = [None, None]`
		`+ result = self.exports.getRPMChecksums(rpm_id, checksum_types=checksum_types)`
		`+ self.assertEqual(expected_result, result)`
		`+`
		`+ self.assertEqual(len(self.queries), 2)`
		`+ query = self.queries[0]`
		`+ self.assertEqual(query.tables, ['rpmsigs'])`
		`+ self.assertEqual(query.joins, None)`
		`+ self.assertEqual(query.clauses, ['rpm_id=%(rpm_id)i'])`
		`+`
		`+ query = self.queries[1]`
		`+ self.assertEqual(query.tables, ['rpm_checksum'])`
		`+ self.assertEqual(query.joins, None)`
		`+ self.assertEqual(query.clauses,`
		`+ ['checksum_type IN %(checksum_type)s', 'rpm_id=%(rpm_id)i'])`
		`+`
		`+ self.get_rpm.assert_called_once_with(rpm_id, strict=True)`
		`+ self.assertEqual(self.calculate_chsum.call_count, 2)`
		`+ self.create_rpm_checksum.assert_has_calls(`
		`+ [mock.call(rpm_id, 'sigkey1', calculate_chsum_res[0]),`
		`+ mock.call(rpm_id, 'sigkey2', calculate_chsum_res[1])])`
		`+ self.create_rpm_checksum_output.assert_called_once_with(`
		`+ [{'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey1'},`
		`+ {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'sigkey1'},`
		`+ {'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey2'},`
		`+ {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'sigkey2'}],`
		`+ {'sigkey1': {'md5', 'sha256'}, 'sigkey2': {'md5', 'sha256'}}`
		`+ )`

tests/test_hub/test_reset_build.py

file modified

+19 -14

		`@@ -87,7 +87,7 @@`
		`self.assertEqual(update.rawdata, {})`
		`self.assertEqual(update.clauses, ['id=%(id)s'])`

		`- self.assertEqual(len(self.deletes), 17)`
		`+ self.assertEqual(len(self.deletes), 18)`
		`delete = self.deletes[0]`
		`self.assertEqual(delete.table, 'rpmsigs')`
		`self.assertEqual(delete.clauses, ['rpm_id=%(rpm_id)i'])`
		`@@ -104,71 +104,76 @@`
		`self.assertEqual(delete.values, {'rpm_id': 123})`

		`delete = self.deletes[3]`
		`+ self.assertEqual(delete.table, 'rpm_checksum')`
		`+ self.assertEqual(delete.clauses, ['rpm_id=%(rpm_id)i'])`
		`+ self.assertEqual(delete.values, {'rpm_id': 123})`
		`+`
		`+ delete = self.deletes[4]`
		`self.assertEqual(delete.table, 'rpminfo')`
		`self.assertEqual(delete.clauses, ['build_id=%(id)i'])`
		`self.assertEqual(delete.values, {'id': self.binfo['build_id']})`

		`- delete = self.deletes[4]`
		`+ delete = self.deletes[5]`
		`self.assertEqual(delete.table, 'maven_archives')`
		`self.assertEqual(delete.clauses, ['archive_id=%(archive_id)i'])`
		`self.assertEqual(delete.values, {'archive_id': 9999})`

		`- delete = self.deletes[5]`
		`+ delete = self.deletes[6]`
		`self.assertEqual(delete.table, 'win_archives')`
		`self.assertEqual(delete.clauses, ['archive_id=%(archive_id)i'])`
		`self.assertEqual(delete.values, {'archive_id': 9999})`

		`- delete = self.deletes[6]`
		`+ delete = self.deletes[7]`
		`self.assertEqual(delete.table, 'image_archives')`
		`self.assertEqual(delete.clauses, ['archive_id=%(archive_id)i'])`
		`self.assertEqual(delete.values, {'archive_id': 9999})`

		`- delete = self.deletes[7]`
		`+ delete = self.deletes[8]`
		`self.assertEqual(delete.table, 'buildroot_archives')`
		`self.assertEqual(delete.clauses, ['archive_id=%(archive_id)i'])`
		`self.assertEqual(delete.values, {'archive_id': 9999})`

		`- delete = self.deletes[8]`
		`+ delete = self.deletes[9]`
		`self.assertEqual(delete.table, 'archive_rpm_components')`
		`self.assertEqual(delete.clauses, ['archive_id=%(archive_id)i'])`
		`self.assertEqual(delete.values, {'archive_id': 9999})`

		`- delete = self.deletes[9]`
		`+ delete = self.deletes[10]`
		`self.assertEqual(delete.table, 'archive_components')`
		`self.assertEqual(delete.clauses, ['archive_id=%(archive_id)i'])`
		`self.assertEqual(delete.values, {'archive_id': 9999})`

		`- delete = self.deletes[10]`
		`+ delete = self.deletes[11]`
		`self.assertEqual(delete.table, 'archive_components')`
		`self.assertEqual(delete.clauses, ['component_id=%(archive_id)i'])`
		`self.assertEqual(delete.values, {'archive_id': 9999})`

		`- delete = self.deletes[11]`
		`+ delete = self.deletes[12]`
		`self.assertEqual(delete.table, 'archiveinfo')`
		`self.assertEqual(delete.clauses, ['build_id=%(id)i'])`
		`self.assertEqual(delete.values, {'id': self.binfo['build_id']})`

		`- delete = self.deletes[12]`
		`+ delete = self.deletes[13]`
		`self.assertEqual(delete.table, 'maven_builds')`
		`self.assertEqual(delete.clauses, ['build_id=%(id)i'])`
		`self.assertEqual(delete.values, {'id': self.binfo['build_id']})`

		`- delete = self.deletes[13]`
		`+ delete = self.deletes[14]`
		`self.assertEqual(delete.table, 'win_builds')`
		`self.assertEqual(delete.clauses, ['build_id=%(id)i'])`
		`self.assertEqual(delete.values, {'id': self.binfo['build_id']})`

		`- delete = self.deletes[14]`
		`+ delete = self.deletes[15]`
		`self.assertEqual(delete.table, 'image_builds')`
		`self.assertEqual(delete.clauses, ['build_id=%(id)i'])`
		`self.assertEqual(delete.values, {'id': self.binfo['build_id']})`

		`- delete = self.deletes[15]`
		`+ delete = self.deletes[16]`
		`self.assertEqual(delete.table, 'build_types')`
		`self.assertEqual(delete.clauses, ['build_id=%(id)i'])`
		`self.assertEqual(delete.values, {'id': self.binfo['build_id']})`

		`- delete = self.deletes[16]`
		`+ delete = self.deletes[17]`
		`self.assertEqual(delete.table, 'tag_listing')`
		`self.assertEqual(delete.clauses, ['build_id=%(id)i'])`
		`self.assertEqual(delete.values, {'id': self.binfo['build_id']})`

tests/test_lib/data/rpms/test-deps-1-1.fc24.x86_64.rpm.signed

file added

empty or binary file added

tests/test_lib/test_spliced_sig.py

file added

+40

		`@@ -0,0 +1,40 @@`
		`+ # coding=utf-8`
		`+ from __future__ import absolute_import`
		`+ import os.path`
		`+ import shutil`
		`+ import tempfile`
		`+ import unittest`
		`+`
		`+ import koji`
		`+`
		`+`
		`+`
		`+ class TestCheckSigMD5(unittest.TestCase):`
		`+`
		`+ def setUp(self):`
		`+ self.tempdir = tempfile.mkdtemp()`
		`+ orig_path = os.path.join(os.path.dirname(__file__), 'data/rpms/test-deps-1-1.fc24.x86_64.rpm')`
		`+ orig_signed = os.path.join(os.path.dirname(__file__), 'data/rpms/test-deps-1-1.fc24.x86_64.rpm.signed')`
		`+ self.path = '%s/test.rpm' % self.tempdir`
		`+ self.signed = '%s/test.rpm.signed' % self.tempdir`
		`+ shutil.copyfile(orig_path, self.path)`
		`+ shutil.copyfile(orig_signed, self.signed)`
		`+`
		`+ def tearDown(self):`
		`+ shutil.rmtree(self.tempdir)`
		`+`
		`+ def test_spliced_sig_reader(self):`
		`+ contents_orig = open(self.path, 'rb').read()`
		`+ contents_signed = open(self.signed, 'rb').read()`
		`+ sighdr = koji.rip_rpm_sighdr(self.signed)`
		`+ contents_spliced = koji.spliced_sig_reader(self.path, sighdr).read()`
		`+ self.assertEqual(contents_signed, contents_spliced)`
		`+ self.assertNotEqual(contents_signed, contents_orig)`
		`+`
		`+ def test_splice_rpm_sighdr(self):`
		`+ contents_signed = open(self.signed, 'rb').read()`
		`+ sighdr = koji.rip_rpm_sighdr(self.signed)`
		`+ dst = '%s/signed-copy.rpm' % self.tempdir`
		`+ koji.splice_rpm_sighdr(sighdr, self.path, dst=dst)`
		`+ contents_spliced = open(dst, 'rb').read()`
		`+ self.assertEqual(contents_signed, contents_spliced)`

jcupova commented a year ago

Fixes: https://pagure.io/koji/issue/3627

tkopecek commented on line 7 of docs/schema-update-1.31-1.32.sql a year ago

Add foreign key constraint here, please.

tkopecek commented on line 346 of kojihub/kojihub.py a year ago

mutable field shouldn't be the default value (replace with None and set it in function instead) + checksum_types would be better here if we expect list

Edited a year ago by tkopecek

tkopecek commented on line 355 of kojihub/kojihub.py a year ago

typo

tkopecek commented on line 365 of kojihub/kojihub.py a year ago

More informative would be something like "There is no rpm {nvrea} signed with {sigkey}". I think that "No cached signature" is a bit misleading.

tkopecek commented on line 371 of kojihub/kojihub.py a year ago

There is no need to error here - just return with noop?

tkopecek commented on line 373 of kojihub/kojihub.py a year ago

We want to compute checksum for signed rpm itself, not for its content (it is not even possible to get rpm file data this way)

rebased onto 8f17c8a29356f93c738617a606c567621364aa78

a year ago

rebased onto fa94a30e4f3c80ee0aef3c066b5fe059108b7fc2

a year ago

rebased onto e4a3ba5ea439c1bd7a2d57ad773c01d545a654fc

a year ago

rebased onto 2b9f77990f83e0f2254b18182bab020b546c4b32

a year ago

rebased onto 7a1c0a7a11372de51345a843b212974fea6484d8

a year ago

rebased onto 7bc8c3958b83aa07833c123573319839a83e21b8

a year ago

rebased onto 4db50efc2ac9fe3b8efc6f11e4dd48e91814b8e6

a year ago

rebased onto a06501dafae91ce210b8ee090a90d7497b9d2ecf

a year ago

rebased onto 3c4c87d9be15b19dc42d2032e673aca07e3cbb29

a year ago

tkopecek commented on line 11 of docs/schema-upgrade-1.31-1.32.sql a year ago

It works, but for consistency with other records it could be in one line rpm_id INTEGER NOT NULL REFERENCES rpminfo(id)

tkopecek commented on line 5 of kojihub/kojihub.py a year ago

These can be reverted to original state. There is no change in the method, it is just pulled out of original space (listRPMFiles), same for get_rpm_file

tkopecek commented on line 265 of kojihub/kojihub.py a year ago

Is there any reason to publish this method via API?

tkopecek commented on line 116 of kojihub/kojihub.py a year ago

This is not necessary - we want to have default types in config file, not code.

tkopecek commented on line 355 of kojihub/kojihub.py a year ago

pull it from config

tkopecek commented on line 357 of kojihub/kojihub.py a year ago

any iterable is ok, so at least (list, tuple)

tkopecek commented on line 366 of kojihub/kojihub.py a year ago

sighash is not used for anything, it could be replaced with no columns and countOnly.

tkopecek commented on line 372 of kojihub/kojihub.py a year ago

same nvra could be used in previous error message when there is only rpm_id

tkopecek commented on line 374 of kojihub/kojihub.py a year ago

checksum_types are now strings. In the db are integers, unify usage (either convert to ints now or expects ints on input. sigkey is completely missing in clauses. Furthermore, next step checks only equivalent sets. In case some of hashes already exist it will fail during the insert.

tkopecek commented on line 383 of kojihub/kojihub.py a year ago

It could be a bit easier without list encapsulation
chsum_list = {chsum: getattr(hashlib, chsum)() for chsum in checksum_type}

tkopecek commented on line 388 of kojihub/kojihub.py a year ago

chunk name is used in similar context throughout the code

rebased onto 15cbf3b9dd8e6112867886ae9a334804b25bd873

a year ago

jcupova commented a year ago

@tkopecek all fixed

tkopecek commented on line 11 of docs/schema.sql a year ago

+
CREATE INDEX rpm_checksum_rpm_id ON rpm_checksum(id);
UNIQUE(rpm_id, sigkey, checksum_type)

Edited a year ago by tkopecek

tkopecek commented on line 7 of kojihub/app/hub.conf a year ago

quotes should not be there + typo in name

Edited a year ago by tkopecek

tkopecek commented on line 154 of kojihub/kojihub.py a year ago

checksum=chsum.hexdigest()

tkopecek commented on line 14 of kojihub/kojihub.py a year ago

mutable list can't be in default values (use None) instead + config says that default is md5+sha256, not sha256

tkopecek commented on line 11 of docs/source/hub_conf.rst a year ago

typo

tkopecek commented on line 6 of kojihub/app/hub.conf a year ago

typo

tkopecek commented on line 31 of kojihub/kojihub.py a year ago

checksum_type_int = [koji.CHECKSUM_TYPES[chsum] for chsum in checksum_types]

tkopecek commented on line 40 of kojihub/kojihub.py a year ago

There could be subset available.

tkopecek commented on line 45 of kojihub/kojihub.py a year ago

you should call write_signed_rpm instead of following code. It would create signed/* data, so it could be computed. Otherwise it would be failing in most cases as GC will delete those copies.

tkopecek commented on line 8 of kojihub/kojixmlrpc.py a year ago

typo

tkopecek commented a year ago

generally "s/RPMDefaultCHecksums/RPMDefaultChecksums/g"

tkopecek commented on line 15 of kojihub/kojihub.py a year ago

s/repo/rpm

tkopecek commented on line 81 of kojihub/kojihub.py a year ago

It should be renamed everywhere as it is always list -> missing_checksum_types

tkopecek commented on line 108 of kojihub/kojihub.py a year ago

isinstance(checksum_types, (list, tuple))

tkopecek commented on line 111 of kojihub/kojihub.py a year ago

if chtype not in koji.CHECKSUM_TYPES

tkopecek commented on line 51 of kojihub/kojihub.py a year ago

it could be simplifed via set()

tkopecek commented on line 155 of kojihub/kojihub.py a year ago

koji.CHECKSUM_TYPES[func]

tkopecek commented on line 77 of kojihub/kojihub.py a year ago

s/friandly/friendly

tkopecek commented on line 149 of kojihub/kojihub.py a year ago

drop this cycle ^

rebased onto e62295c6049ecca99ff9ed2a035bdc655fb5a53e

a year ago

rebased onto 7a674468faac1d77389c26875938095e988f9038

a year ago

rebased onto f3a844e3254e63f483e6c5486a6aa98597c908b7

a year ago

rebased onto 93dbacda08054c9b199fba8af8f74e0e1ac8dab3

a year ago

rebased onto 0318ba216284d4c03a70c64f183ef03960b5119e

a year ago

rebased onto ffbcc0f77570378a867f00bc59abb3b3048f9fee

a year ago

Metadata Update from @tkopecek:
- Pull-request tagged with: testing-ready

a year ago

rebased onto df2bfa978d3900f57031c75aef1c0c503b6c8fca

a year ago

mikem commented a year ago

There are a few places we need to clear this data:

delete_build
reset_build
delete_rpm_sig

mikem commented a year ago

The rpm_checksum table doesn't currently have a primary key. It would make sense to use (rpm_id, sigkey, checksum_type)

mikem commented a year ago

The exception for unsigned rpms seems unnecessary and inconsistent. The call is not looking for a specific key, but for any key. If there are no keys, then a {} result is accurate.

The koji get* calls are generally tolerant unless they are explicitly passed a strict arg. This one should be too (not that I'm suggesting that we add a strict mode here).

mikem commented a year ago

I had hoped for more efficient use of IO in the write_signed_rpm hook. We already loop over the new files contents in koji.splice_rpm_sighdr. We could add a callback there to enable the checksum. This is admittedly more complex, but without it we're re-reading every signed copy after writing it. I thought I'd made this more explicit, but looking back I see I only alluded to a few places.

Metadata Update from @jcupova:
- Pull-request untagged with: testing-ready

a year ago

1 new commit added

Fix review

a year ago

2 new commits added

Fix review
Add checksum API

a year ago

2 new commits added

Fix review
Add checksum API

a year ago

mikem commented a year ago

Apologies for the delay. There's a bit more complexity here than I expected.

1) splice changes

The splice_rpm_sighdr callback changes are not quite what I envisioned, and I don't think we need to expose so much specific data to that function. I made some changes here that are more along the lines of what I was thinking.

https://pagure.io/fork/mikem/koji/commits/pr3628updates

The first commit there, "simplify splice_rpm_sighdr changes", is kind of what I had in mind initially. The second commit takes things a bit further and bears some explanation (see below).

Also, note that the previous callback changes were not capturing the full file contents, but only the part after the signature. This would result in incorrect sums. This is fixed in the above changes.

2) unauthenticated writes

The getRPMChecksums export is an unauthenticated call that can potentially write out signed copies to disc. I don't think we should allow this so freely. The writeSignedRPM call requires special access.

There are a couple ways we might address this. One is to calculate the checksums without writing the copy. This is what led to my second commit above ("generator for reading spliced rpm signatures"). Splitting that logic out of the original splice function should make it fairly easy to generate a checksum for a signed rpm without actually performing the writes.

Alternately, we could simply fail when we have neither cached sums or signed copies. Which leads me to...

3) strict option

Above I write that I was not suggesting adding a strict option for getRPMChecksums. There might be reason to, but I think it bears more thought. However, I do not think that the case where an rpm has no signatures should ever result in an error. In that case, there's a definitive answer to the query and that answer is {}.

That said, the situation above suggests a situation where a strict option might make sense -- whether to raise an error when we have the signature, but neither a cached checksum nor a signed copy. In the non-strict case we'd report the missing checksums as None. This would make it easier for the client to distinguish which sum is missing.

4) Some smaller things.

The variable named chsum_list is not a list, which is unnecessarily confusing.

Seems a little odd to use BulkInsert for such a small number of inserts.

The create_rpm_checksum function has signature *kwargs, which seems unnecessarily generic. We only call it twice, and with the same sort of parameters each time.

The create_rpm_checksum function modifies mutable parameters that are passed to it. This sort of hidden side effect can cause issues that are very hard to debug.

The getRPMChecksums function is appended at the end of the exports class, but it would probably make more sense to have it near the other exports that relate to rpm signatures. E.g. somewhere near writeSignedRPM.

rebased onto 5f9a68a8e365d8c741beb373d2959cd635eb9358

a year ago

3 new commits added

Use strict for rpm without signed copies or checksums + small review fixes
generator for reading spliced rpm signatures
simplify splice_rpm_sighdr changes

a year ago

5 new commits added

Use strict for rpm without signed copies or checksums + small review fixes
generator for reading spliced rpm signatures
simplify splice_rpm_sighdr changes
Fix review
Add checksum API

a year ago

5 new commits added

Use strict for rpm without signed copies or checksums + small review fixes
generator for reading spliced rpm signatures
simplify splice_rpm_sighdr changes
Fix review
Add checksum API

a year ago

tkopecek commented a year ago

With rewriting generator to IOStream it could be more usable? We can unify behaviour to deal with it as with signed copy in all places except writing the signed copy (based on https://gist.github.com/mechanical-snail/7688353, not clear what is the license there). Not tested.

def spliced_sig_reader(path, sighdr, bufsize=8192):
    class Stream(io.RawIOBase):
        def __init__(self, path, sighdr):
            self.path = path
            self.sighdr = sighdr
            self.buf = None
            self.gen = self.generator()

        def generator(self):
            (start, size) = find_rpm_sighdr(self.path)
            with open(path, 'rb') as fo:
                # the part before the signature
                yield fo.read(start)

                # the spliced signature
                yield sighdr

                # skip original signature
                fo.seek(size, 1)

                # the part after the signature
                while True:
                    buf = fo.read(bufsize)
                    if not buf:
                        break
                    yield buf

        def readable(self):
            return True

        def readinto(self, b):
            try:
                expected_buf_size = len(b)
                data = self.buf or next(self.gen)
                output = data[:expected_buf_size]
                self.buf = data[expected_buf_size:]
                b[:len(output)] = output
                return len(output)
            except StopIteration:
                return 0    # indicate EOF

    return io.BufferedReader(Stream(path, sighdr), buffer_size=bufsize)

Edited a year ago by tkopecek

1 new commit added

Rewrite generator to IOStream

a year ago

6 new commits added

Rewrite generator to IOStream
Use strict for rpm without signed copies or checksums + small review fixes
generator for reading spliced rpm signatures
simplify splice_rpm_sighdr changes
Fix review
Add checksum API

a year ago

6 new commits added

Rewrite generator to IOStream
Use strict for rpm without signed copies or checksums + small review fixes
generator for reading spliced rpm signatures
simplify splice_rpm_sighdr changes
Fix review
Add checksum API

a year ago

6 new commits added

Rewrite generator to IOStream
Use strict for rpm without signed copies or checksums + small review fixes
generator for reading spliced rpm signatures
simplify splice_rpm_sighdr changes
Fix review
Add checksum API

a year ago

rebased onto 86a0336

a year ago

mikem commented a year ago

We should avoid os.path.join in the hub code for safety reasons. We have koji.joinpath for this now.

The BufferedReader implementation is neat, but the class declaration should be moved out of the function (and given a clearer name). Embedding the class definition like this creates a new class each time the function is called.

With the code updates that automatically splice and sum when the signed copy is missing, the strict option doesn't really have much meaning separate from (anti-)cacheonly. Better and simpler to just drop the strict option.

The builddir calculation is incorrect.

        builddir = koji.pathinfo.build(rpm_info)

The rpm info cannot stand in for the build info. The NVR could be completely different.

I have addressed some of these on my branch. I've also added some unit tests for the splicing code.
https://pagure.io/fork/mikem/koji/commits/pr3628updates

3 new commits added

Move class out of function
use signed copy for checksum if avail, drop strict option
simple unit tests for splicing code

a year ago

9 new commits added

Move class out of function
use signed copy for checksum if avail, drop strict option
simple unit tests for splicing code
Rewrite generator to IOStream
Use strict for rpm without signed copies or checksums + small review fixes
generator for reading spliced rpm signatures
simplify splice_rpm_sighdr changes
Fix review
Add checksum API

a year ago

mikem commented a year ago

self.checksums[name] = checksum.hexdigest()

Having this function alter the structure of a key instance variable seems fragile and unnecessary. This could could instead return an altered copy and calling code could use the result of that call rather than accessing checksums.

def spliced_sig_reader(path, sighdr, bufsize=8192):
    """A generator that yields the contents of an rpm with signature spliced in"""

minor, but the docstring here is now inaccurate. Should be something like: "returns a file-like object whose contents have the new signature spliced in"
Sorry, meant to note that before.

Your commit message only mentions the class move, but the other changes are also notable.

9 new commits added

Move class out of function and create to_hexdigest function
use signed copy for checksum if avail, drop strict option
simple unit tests for splicing code
Rewrite generator to IOStream
Use strict for rpm without signed copies or checksums + small review fixes
generator for reading spliced rpm signatures
simplify splice_rpm_sighdr changes
Fix review
Add checksum API

a year ago

9 new commits added

Move class out of function and create to_hexdigest function
use signed copy for checksum if avail, drop strict option
simple unit tests for splicing code
Rewrite generator to IOStream
Use strict for rpm without signed copies or checksums + small review fixes
generator for reading spliced rpm signatures
simplify splice_rpm_sighdr changes
Fix review
Add checksum API

a year ago

mikem commented a year ago

Oh! One more thing.

In create_rpm_checksum we filter out the checksums that we already have in the db. We should probably take the opportunity to check that the values we're ignoring actually match what is in the db. A mismatch here is a "should not happen" situation, so we should raise an error.

1 new commit added

Add comparison between checksum from DB and caltulated checksum

a year ago

mikem commented a year ago

Sorry, I keep finding more small things :)

The deepcopy in create_rpm_checksum is not needed at this point, since the data will be a name:digest dictionary. A simple .copy() should suffice.

The error in create_rpm_checksum should indicate the rpm id, sigkey, and checksum type so that this can be debugged if it ever happens.

The checksum_types keyword arg for write_signed_rpm is never used in the current version of the PR. I think we can drop it and just use the default.

10 new commits added