From 86a03364edafeeb06f6037a7fa7c4ef0dd8cd478 Mon Sep 17 00:00:00 2001 From: Jana Cupova Date: Jan 25 2023 11:15:41 +0000 Subject: [PATCH 1/10] Add checksum API Fixes: https://pagure.io/koji/issue/3627 --- diff --git a/docs/schema-upgrade-1.31-1.32.sql b/docs/schema-upgrade-1.31-1.32.sql index 2c25886..78f0f5d 100644 --- a/docs/schema-upgrade-1.31-1.32.sql +++ b/docs/schema-upgrade-1.31-1.32.sql @@ -6,4 +6,13 @@ BEGIN; -- 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, + UNIQUE(rpm_id, sigkey, checksum_type) + ) WITHOUT OIDS; + CREATE INDEX rpm_checksum_rpm_id ON rpm_checksum(rpm_id); COMMIT; diff --git a/docs/schema.sql b/docs/schema.sql index c2ea695..9d42aa1 100644 --- a/docs/schema.sql +++ b/docs/schema.sql @@ -965,5 +965,14 @@ CREATE TABLE proton_queue ( 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, + UNIQUE(rpm_id, sigkey, checksum_type) +) WITHOUT OIDS; +CREATE INDEX rpm_checksum_rpm_id ON rpm_checksum(rpm_id); COMMIT WORK; diff --git a/docs/source/hub_conf.rst b/docs/source/hub_conf.rst index 0870a98..7738fbc 100644 --- a/docs/source/hub_conf.rst +++ b/docs/source/hub_conf.rst @@ -542,3 +542,15 @@ Host names are listed in both groups because hosts always have an associated use 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``. diff --git a/kojihub/app/hub.conf b/kojihub/app/hub.conf index deca03e..9ea7aa6 100644 --- a/kojihub/app/hub.conf +++ b/kojihub/app/hub.conf @@ -141,3 +141,6 @@ NotifyOnSuccess = True # MaxNameLengthInternal = 256 # RegexNameInternal = ^[A-Za-z0-9/_.+-]+$ # RegexUserName = ^[A-Za-z0-9/_.@-]+$ + +## Determines default checksums +# RPMDefaultChecksums = md5 sha256 diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index ade8904..adf7cd6 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -7968,7 +7968,7 @@ def query_rpm_sigs(rpm_id=None, sigkey=None, queryOpts=None): return query.execute() -def write_signed_rpm(an_rpm, sigkey, force=False): +def write_signed_rpm(an_rpm, sigkey, force=False, checksum_types=None): """Write a signed copy of the rpm""" sigkey = sigkey.lower() rinfo = get_rpm(an_rpm, strict=True) @@ -8003,6 +8003,9 @@ def write_signed_rpm(an_rpm, sigkey, force=False): sighdr = fo.read() koji.ensuredir(os.path.dirname(signedpath)) koji.splice_rpm_sighdr(sighdr, rpm_path, signedpath) + if not checksum_types: + checksum_types = context.opts.get('RPMDefaultChecksums').split() + create_rpm_checksum(rinfo['id'], sigkey, checksum_types=checksum_types) def query_history(tables=None, **kwargs): @@ -8638,6 +8641,9 @@ def reset_build(build): 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() @@ -13649,6 +13655,56 @@ class RootExports(object): values=locals(), opts=queryOpts) return query.iterate() + 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 checsum 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') + 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: + raise koji.GenericError(f'No cached signature for rpm ID {rpm_id}.') + 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 = list_checksums_sigkeys.copy() + 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']]) + + for sigkey, chsums in missing_chsum_sigkeys.items(): + write_signed_rpm(rpm_id, sigkey, force=True, checksum_types=list(chsums)) + query_result = query_checksum.execute() + return create_rpm_checksums_output(query_result, list_checksums_sigkeys) + class BuildRoot(object): @@ -15434,3 +15490,77 @@ def verify_name_user(name=None, krb=None): 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, checksum_types=None): + """Creates RPM checksum. + + :param int rpm_id: RPM id + :param string sigkey: Sigkey for specific RPM + :param list checksum_type: List of checksum types. + """ + if checksum_types is None: + checksum_types = context.opts.get('RPMDefaultChecksums').split() + else: + if not isinstance(checksum_types, (list, tuple)): + raise koji.ParameterError(f'Invalid type of checksum_types: {type(checksum_types)}') + 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") + rinfo = get_rpm(rpm_id) + nvra = "%(name)s-%(version)s-%(release)s.%(arch)s" % rinfo + if rinfo['external_repo_id']: + raise koji.GenericError(f"Not an internal rpm: {nvra} " + f"(from {rinfo['external_repo_name']})") + query = QueryProcessor(tables=['rpmsigs'], clauses=['rpm_id=%(rpm_id)i', 'sigkey=%(sigkey)s'], + values={'rpm_id': rpm_id, 'sigkey': sigkey}, opts={'countOnly': True}) + sighash = query.singleValue(strict=False) + if not sighash: + raise koji.GenericError(f"There is no rpm {nvra} signed with {sigkey}") + checksum_type_int = [koji.CHECKSUM_TYPES[chsum] for chsum in checksum_types] + query = QueryProcessor(tables=['rpm_checksum'], columns=['checksum_type'], + 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: + checksum_types.remove(koji.CHECKSUM_TYPES[r['checksum_type']]) + buildinfo = get_build(rinfo['build_id']) + rpm_path = joinpath(koji.pathinfo.build(buildinfo), koji.pathinfo.signed(rinfo, sigkey)) + chsum_list = {chsum: getattr(hashlib, chsum)() for chsum in checksum_types} + + try: + with open(rpm_path, 'rb') as f: + while 1: + chunk = f.read(1024**2) + if not chunk: + break + for func, chsum in chsum_list.items(): + chsum.update(chunk) + except IOError: + raise koji.GenericError(f"RPM path {rpm_path} cannot be open.") + + if chsum_list: + insert = BulkInsertProcessor(table='rpm_checksum') + for func, chsum in chsum_list.items(): + insert.add_record(rpm_id=rpm_id, sigkey=sigkey, checksum=chsum.hexdigest(), + checksum_type=koji.CHECKSUM_TYPES[func]) + insert.execute() diff --git a/kojihub/kojixmlrpc.py b/kojihub/kojixmlrpc.py index f598d37..30ca70c 100644 --- a/kojihub/kojixmlrpc.py +++ b/kojihub/kojixmlrpc.py @@ -497,7 +497,9 @@ def load_config(environ): ['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 @@ def load_config(environ): opts['RegexNameInternal.compiled'] = re.compile(opts['RegexNameInternal']) if opts['RegexUserName'] != '': opts['RegexUserName.compiled'] = re.compile(opts['RegexUserName']) + return opts diff --git a/tests/test_hub/test_create_rpm_checksum.py b/tests/test_hub/test_create_rpm_checksum.py new file mode 100644 index 0000000..b146b12 --- /dev/null +++ b/tests/test_hub/test_create_rpm_checksum.py @@ -0,0 +1,132 @@ +import unittest +import mock +import six + +import koji +import kojihub + +QP = kojihub.QueryProcessor + + +def mock_open(): + """Return the right patch decorator for open""" + if six.PY2: + return mock.patch('__builtin__.open') + else: + return mock.patch('builtins.open') + + +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.get_rpm = mock.patch('kojihub.kojihub.get_rpm').start() + self.get_build = mock.patch('kojihub.kojihub.get_build').start() + self. rpm_info = {'id': 123, 'name': 'test-rpm', 'external_repo_id': 0, 'version': '11', + 'release': '222', 'arch': 'noarch', 'build_id': 222} + self.rpm_id = 123 + self.sigkey = 'test-sigkey' + self.query_execute = mock.MagicMock() + self.single_value = mock.MagicMock() + self.path_signed = mock.patch('koji.pathinfo.signed').start() + self.path_build = mock.patch('koji.pathinfo.build').start() + self.context = mock.patch('kojihub.kojihub.context').start() + self.context.session.assertPerm = mock.MagicMock() + self.context.opts = {'RPMDefaultChecksums': 'md5 sha256'} + + def tearDown(self): + mock.patch.stopall() + + def getQuery(self, *args, **kwargs): + query = QP(*args, **kwargs) + query.execute = self.query_execute + query.singleValue = self.single_value + self.queries.append(query) + return query + + def test_checksum_type_not_string(self): + checksum_types = 'type-1 type-2' + with self.assertRaises(koji.GenericError) as ex: + kojihub.create_rpm_checksum(self.rpm_id, self.sigkey, checksum_types=checksum_types) + self.assertEqual(f'Invalid type of checksum_types: {type(checksum_types)}', + str(ex.exception)) + + def test_checksum_type_bad_type(self): + checksum_types = ['md5', 'type-1'] + with self.assertRaises(koji.GenericError) as ex: + kojihub.create_rpm_checksum(self.rpm_id, self.sigkey, checksum_types=checksum_types) + self.assertEqual("Checksum_type type-1 isn't supported", str(ex.exception)) + + def test_external_rpm(self): + ext_rpm_info = {'id': 123, 'name': 'test-rpm', 'external_repo_id': 125, + 'external_repo_name': 'test-ext-rpm', 'version': '11', + 'release': '222', 'arch': 'noarch'} + self.get_rpm.return_value = ext_rpm_info + nvra = "%(name)s-%(version)s-%(release)s.%(arch)s" % ext_rpm_info + with self.assertRaises(koji.GenericError) as ex: + kojihub.create_rpm_checksum(self.rpm_id, self.sigkey) + self.assertEqual(f'Not an internal rpm: {nvra} (from test-ext-rpm)', str(ex.exception)) + + def test_not_sigkey_related_to_rpm(self): + self.get_rpm.return_value = self.rpm_info + checksum_types = ['md5', 'sha256'] + nvra = "%(name)s-%(version)s-%(release)s.%(arch)s" % self.rpm_info + expected_err = f'There is no rpm {nvra} signed with {self.sigkey}' + self.single_value.return_value = None + with self.assertRaises(koji.GenericError) as ex: + kojihub.create_rpm_checksum(self.rpm_id, self.sigkey, checksum_types=checksum_types) + self.assertEqual(expected_err % self.rpm_info, str(ex.exception)) + + 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', 'sigkey=%(sigkey)s']) + + def test_checksum_exists(self): + self.get_rpm.return_value = self.rpm_info + self.single_value.return_value = 'test-sighash' + self.query_execute.return_value = [{'checksum_type': 'md5'}, {'checksum_type': 'sha256'}] + result = kojihub.create_rpm_checksum(self.rpm_id, self.sigkey) + self.assertIsNone(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', 'sigkey=%(sigkey)s']) + + query = self.queries[1] + self.assertEqual(query.tables, ['rpm_checksum']) + self.assertEqual(query.joins, None) + self.assertEqual(query.clauses, + ["checksum_type IN %(checksum_types)s", "sigkey=%(sigkey)s"]) + + @mock_open() + def test_cannot_open_file(self, m_open): + self.get_rpm.return_value = self.rpm_info + self.get_build.return_value = {'build_id': 222} + self.single_value.return_value = 'test-sighash' + self.query_execute.return_value = [{'checksum_type': 'md5'}] + self.path_build.return_value = 'fakebuildpath' + self.path_signed.return_value = 'fakesignedpath' + m_open.side_effect = IOError() + + with self.assertRaises(koji.GenericError) as ex: + kojihub.create_rpm_checksum(self.rpm_id, self.sigkey) + self.assertEqual("RPM path fakebuildpath/fakesignedpath cannot be open.", + str(ex.exception)) + + 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', 'sigkey=%(sigkey)s']) + + query = self.queries[1] + self.assertEqual(query.tables, ['rpm_checksum']) + self.assertEqual(query.joins, None) + self.assertEqual(query.clauses, + ["checksum_type IN %(checksum_types)s", "sigkey=%(sigkey)s"]) diff --git a/tests/test_hub/test_create_rpm_checksums_output.py b/tests/test_hub/test_create_rpm_checksums_output.py new file mode 100644 index 0000000..17bc557 --- /dev/null +++ b/tests/test_hub/test_create_rpm_checksums_output.py @@ -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) diff --git a/tests/test_hub/test_get_rpm_checksums.py b/tests/test_hub/test_get_rpm_checksums.py new file mode 100644 index 0000000..60f30f2 --- /dev/null +++ b/tests/test_hub/test_get_rpm_checksums.py @@ -0,0 +1,167 @@ +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.create_rpm_checksum_output = mock.patch( + 'kojihub.kojihub.create_rpm_checksums_output').start() + self.write_signed_rpm = mock.patch('kojihub.kojihub.write_signed_rpm').start() + 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_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)) + + def test_checksum_types_not_list(self): + rpm_id = 123 + 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)) + + 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)) + + def test_all_checksum_exists(self): + rpm_id = 123 + checksum_types = ['md5', 'sha256'] + expected_result = {'sigkey1': {'md5': 'checksum-md5', 'sha256': 'checksum-sha256'}} + self.query_execute.side_effect = [ + [{'sigkey': 'sigkey-1'}], + [{'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'test-sigkey'}, + {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'test-sigkey'}]] + 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) + + def test_missing_checksum_not_sigkey(self): + rpm_id = 123 + checksum_types = ['md5'] + self.query_execute.side_effect = [[], []] + with self.assertRaises(koji.GenericError) as ex: + self.exports.getRPMChecksums(rpm_id, checksum_types=checksum_types) + self.assertEqual(f'No cached signature for rpm ID {rpm_id}.', str(ex.exception)) + + 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']) + + def test_missing_valid_checksum_generated(self): + rpm_id = 123 + checksum_types = ['md5'] + expected_result = {'sigkey1': {'md5': 'checksum-md5'}} + self.query_execute.side_effect = [ + [{'sigkey': 'sigkey-1'}], + [], + [{'checksum': 'checksum-md5', 'checksum_type': 0}]] + self.write_signed_rpm.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']) + + def test_missing_valid_more_checksum_generated_and_exists(self): + rpm_id = 123 + checksum_types = ['md5', 'sha256'] + expected_result = {'sigkey1': {'md5': 'checksum-md5', 'sha256': 'checksum-sha256'}} + self.query_execute.side_effect = [ + [{'sigkey': 'sigkey-1'}], + [{'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'test-sigkey'}], + [{'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'test-sigkey'}, + {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'test-sigkey'}]] + self.write_signed_rpm.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']) + + def test_missing_valid_more_checksum_generated_and_exists_more_sigkeys(self): + rpm_id = 123 + checksum_types = ['md5', 'sha256'] + expected_result = {'sigkey1': {'md5': 'checksum-md5', 'sha256': 'checksum-sha256'}, + 'sigkey2': {'md5': 'checksum-md5', 'sha256': 'checksum-sha256'}} + self.query_execute.side_effect = [ + [{'sigkey': 'sigkey-1'}, {'sigkey': 'sigkey-2'}], + [{'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey-1'}, + {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'sigkey-2'}], + [{'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey-1'}, + {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'sigkey-1'}, + {'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey-2'}, + {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'sigkey-2'}]] + self.write_signed_rpm.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']) From 2116349ce656671abf4463bb079c2bbabb95a265 Mon Sep 17 00:00:00 2001 From: Jana Cupova Date: Jan 25 2023 11:19:54 +0000 Subject: [PATCH 2/10] Fix review --- diff --git a/docs/schema-upgrade-1.31-1.32.sql b/docs/schema-upgrade-1.31-1.32.sql index 78f0f5d..3a86f95 100644 --- a/docs/schema-upgrade-1.31-1.32.sql +++ b/docs/schema-upgrade-1.31-1.32.sql @@ -12,7 +12,7 @@ BEGIN; sigkey TEXT NOT NULL, checksum TEXT NOT NULL UNIQUE, checksum_type SMALLINT NOT NULL, - UNIQUE(rpm_id, sigkey, checksum_type) + PRIMARY KEY (rpm_id, sigkey, checksum_type) ) WITHOUT OIDS; CREATE INDEX rpm_checksum_rpm_id ON rpm_checksum(rpm_id); COMMIT; diff --git a/docs/schema.sql b/docs/schema.sql index 9d42aa1..39d0c95 100644 --- a/docs/schema.sql +++ b/docs/schema.sql @@ -971,7 +971,7 @@ CREATE TABLE rpm_checksum ( sigkey TEXT NOT NULL, checksum TEXT NOT NULL UNIQUE, checksum_type SMALLINT NOT NULL, - UNIQUE(rpm_id, sigkey, checksum_type) + PRIMARY KEY (rpm_id, sigkey, checksum_type) ) WITHOUT OIDS; CREATE INDEX rpm_checksum_rpm_id ON rpm_checksum(rpm_id); diff --git a/koji/__init__.py b/koji/__init__.py index 50162f8..321b9ac 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -943,7 +943,8 @@ def get_sighdr_key(sighdr): return get_sigpacket_key_id(sig) -def splice_rpm_sighdr(sighdr, src, dst=None, bufsize=8192): +def splice_rpm_sighdr(sighdr, src, dst=None, bufsize=8192, rpm_id=None, sigkey=None, + checksum_types=None, callback=None): """Write a copy of an rpm with signature header spliced in""" (start, size) = find_rpm_sighdr(src) if dst is not None: @@ -953,6 +954,7 @@ def splice_rpm_sighdr(sighdr, src, dst=None, bufsize=8192): else: (fd, dst_temp) = tempfile.mkstemp() os.close(fd) + chsum_list = {chsum: getattr(hashlib, chsum)() for chsum in checksum_types} 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) @@ -962,6 +964,8 @@ def splice_rpm_sighdr(sighdr, src, dst=None, bufsize=8192): if not buf: break dst_fo.write(buf) + for func, chsum in chsum_list.items(): + chsum.update(buf) if dst is not None: src_stats = os.stat(src) dst_temp_stats = os.stat(dst_temp) @@ -970,6 +974,9 @@ def splice_rpm_sighdr(sighdr, src, dst=None, bufsize=8192): os.rename(dst_temp, dst) else: dst = dst_temp + if callback: + callback(src=src, dst=dst, sighdr=sighdr, rpm_id=rpm_id, sigkey=sigkey, + chsum_list=chsum_list) return dst diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index adf7cd6..4cc5072 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -7817,6 +7817,8 @@ def delete_rpm_sig(rpminfo, sigkey=None, all_sigs=False): 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 = [] @@ -7968,8 +7970,32 @@ def query_rpm_sigs(rpm_id=None, sigkey=None, queryOpts=None): return query.execute() +def calculate_chsum(signed_path, checksum_types): + chsum_list = {chsum: getattr(hashlib, chsum)() for chsum in checksum_types} + try: + with open(signed_path, 'rb') as f: + while 1: + chunk = f.read(1024 ** 2) + if not chunk: + break + for func, chsum in chsum_list.items(): + chsum.update(chunk) + except IOError: + raise koji.GenericError(f"RPM path {signed_path} cannot be open.") + return chsum_list + + def write_signed_rpm(an_rpm, sigkey, force=False, checksum_types=None): """Write a signed copy of the rpm""" + if checksum_types is None: + checksum_types = context.opts.get('RPMDefaultChecksums').split() + else: + if not isinstance(checksum_types, (list, tuple)): + raise koji.ParameterError(f'Invalid type of checksum_types: {type(checksum_types)}') + 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 +8021,8 @@ def write_signed_rpm(an_rpm, sigkey, force=False, checksum_types=None): if os.path.exists(signedpath): if not force: # already present + chsum_list = calculate_chsum(signedpath, checksum_types) + create_rpm_checksum(rpm_id=rpm_id, sigkey=sigkey, chsum_list=chsum_list) return else: os.unlink(signedpath) @@ -8002,10 +8030,8 @@ def write_signed_rpm(an_rpm, sigkey, force=False, checksum_types=None): with open(sigpath, 'rb') as fo: sighdr = fo.read() koji.ensuredir(os.path.dirname(signedpath)) - koji.splice_rpm_sighdr(sighdr, rpm_path, signedpath) - if not checksum_types: - checksum_types = context.opts.get('RPMDefaultChecksums').split() - create_rpm_checksum(rinfo['id'], sigkey, checksum_types=checksum_types) + koji.splice_rpm_sighdr(sighdr, rpm_path, signedpath, rpm_id=rpm_id, sigkey=sigkey, + checksum_types=checksum_types, callback=create_rpm_checksum) def query_history(tables=None, **kwargs): @@ -8592,6 +8618,9 @@ def _delete_build(binfo): 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() @@ -13655,7 +13684,7 @@ class RootExports(object): values=locals(), opts=queryOpts) return query.iterate() - def getRPMChecksums(self, rpm_id, checksum_types=None, cacheonly=False): + def getRPMChecksums(self, rpm_id, checksum_types=None, cacheonly=False, strict=False): """Returns RPM checksums for specific rpm. :param int rpm_id: RPM id @@ -13680,7 +13709,10 @@ class RootExports(object): clauses=['rpm_id=%(rpm_id)i'], values={'rpm_id': rpm_id}) sigkeys = [r['sigkey'] for r in query.execute()] if not sigkeys: - raise koji.GenericError(f'No cached signature for rpm ID {rpm_id}.') + if strict: + raise koji.GenericError(f'No cached signature for rpm ID {rpm_id}.') + else: + return {} list_checksums_sigkeys = {s: set(checksum_types) for s in sigkeys} checksum_type_int = [koji.CHECKSUM_TYPES[chsum] for chsum in checksum_types] @@ -13701,7 +13733,7 @@ class RootExports(object): koji.CHECKSUM_TYPES[r['checksum_type']]) for sigkey, chsums in missing_chsum_sigkeys.items(): - write_signed_rpm(rpm_id, sigkey, force=True, checksum_types=list(chsums)) + write_signed_rpm(rpm_id, sigkey, checksum_types=list(chsums)) query_result = query_checksum.execute() return create_rpm_checksums_output(query_result, list_checksums_sigkeys) @@ -15507,32 +15539,18 @@ def create_rpm_checksums_output(query_result, list_chsum_sigkeys): return result -def create_rpm_checksum(rpm_id, sigkey, checksum_types=None): +def create_rpm_checksum(**kwargs): """Creates RPM checksum. :param int rpm_id: RPM id :param string sigkey: Sigkey for specific RPM :param list checksum_type: List of checksum types. """ - if checksum_types is None: - checksum_types = context.opts.get('RPMDefaultChecksums').split() - else: - if not isinstance(checksum_types, (list, tuple)): - raise koji.ParameterError(f'Invalid type of checksum_types: {type(checksum_types)}') - 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") - rinfo = get_rpm(rpm_id) - nvra = "%(name)s-%(version)s-%(release)s.%(arch)s" % rinfo - if rinfo['external_repo_id']: - raise koji.GenericError(f"Not an internal rpm: {nvra} " - f"(from {rinfo['external_repo_name']})") - query = QueryProcessor(tables=['rpmsigs'], clauses=['rpm_id=%(rpm_id)i', 'sigkey=%(sigkey)s'], - values={'rpm_id': rpm_id, 'sigkey': sigkey}, opts={'countOnly': True}) - sighash = query.singleValue(strict=False) - if not sighash: - raise koji.GenericError(f"There is no rpm {nvra} signed with {sigkey}") - checksum_type_int = [koji.CHECKSUM_TYPES[chsum] for chsum in checksum_types] + chsum_list = kwargs.get('chsum_list') + sigkey = kwargs.get('sigkey') + rpm_id = kwargs.get('rpm_id') + + checksum_type_int = [koji.CHECKSUM_TYPES[func] for func, _ in chsum_list.items()] query = QueryProcessor(tables=['rpm_checksum'], columns=['checksum_type'], clauses=["checksum_type IN %(checksum_types)s", 'sigkey=%(sigkey)s'], values={'checksum_types': checksum_type_int, 'sigkey': sigkey}) @@ -15542,22 +15560,7 @@ def create_rpm_checksum(rpm_id, sigkey, checksum_types=None): else: for r in rows: if r['checksum_type'] in checksum_type_int: - checksum_types.remove(koji.CHECKSUM_TYPES[r['checksum_type']]) - buildinfo = get_build(rinfo['build_id']) - rpm_path = joinpath(koji.pathinfo.build(buildinfo), koji.pathinfo.signed(rinfo, sigkey)) - chsum_list = {chsum: getattr(hashlib, chsum)() for chsum in checksum_types} - - try: - with open(rpm_path, 'rb') as f: - while 1: - chunk = f.read(1024**2) - if not chunk: - break - for func, chsum in chsum_list.items(): - chsum.update(chunk) - except IOError: - raise koji.GenericError(f"RPM path {rpm_path} cannot be open.") - + del chsum_list[koji.CHECKSUM_TYPES[r['checksum_type']]] if chsum_list: insert = BulkInsertProcessor(table='rpm_checksum') for func, chsum in chsum_list.items(): diff --git a/tests/test_hub/test_create_rpm_checksum.py b/tests/test_hub/test_create_rpm_checksum.py index b146b12..1af67a5 100644 --- a/tests/test_hub/test_create_rpm_checksum.py +++ b/tests/test_hub/test_create_rpm_checksum.py @@ -2,7 +2,6 @@ import unittest import mock import six -import koji import kojihub QP = kojihub.QueryProcessor @@ -22,19 +21,7 @@ class TestCreateRPMChecksum(unittest.TestCase): self.QueryProcessor = mock.patch('kojihub.kojihub.QueryProcessor', side_effect=self.getQuery).start() self.queries = [] - self.get_rpm = mock.patch('kojihub.kojihub.get_rpm').start() - self.get_build = mock.patch('kojihub.kojihub.get_build').start() - self. rpm_info = {'id': 123, 'name': 'test-rpm', 'external_repo_id': 0, 'version': '11', - 'release': '222', 'arch': 'noarch', 'build_id': 222} - self.rpm_id = 123 - self.sigkey = 'test-sigkey' self.query_execute = mock.MagicMock() - self.single_value = mock.MagicMock() - self.path_signed = mock.patch('koji.pathinfo.signed').start() - self.path_build = mock.patch('koji.pathinfo.build').start() - self.context = mock.patch('kojihub.kojihub.context').start() - self.context.session.assertPerm = mock.MagicMock() - self.context.opts = {'RPMDefaultChecksums': 'md5 sha256'} def tearDown(self): mock.patch.stopall() @@ -42,90 +29,21 @@ class TestCreateRPMChecksum(unittest.TestCase): def getQuery(self, *args, **kwargs): query = QP(*args, **kwargs) query.execute = self.query_execute - query.singleValue = self.single_value self.queries.append(query) return query - def test_checksum_type_not_string(self): - checksum_types = 'type-1 type-2' - with self.assertRaises(koji.GenericError) as ex: - kojihub.create_rpm_checksum(self.rpm_id, self.sigkey, checksum_types=checksum_types) - self.assertEqual(f'Invalid type of checksum_types: {type(checksum_types)}', - str(ex.exception)) - - def test_checksum_type_bad_type(self): - checksum_types = ['md5', 'type-1'] - with self.assertRaises(koji.GenericError) as ex: - kojihub.create_rpm_checksum(self.rpm_id, self.sigkey, checksum_types=checksum_types) - self.assertEqual("Checksum_type type-1 isn't supported", str(ex.exception)) - - def test_external_rpm(self): - ext_rpm_info = {'id': 123, 'name': 'test-rpm', 'external_repo_id': 125, - 'external_repo_name': 'test-ext-rpm', 'version': '11', - 'release': '222', 'arch': 'noarch'} - self.get_rpm.return_value = ext_rpm_info - nvra = "%(name)s-%(version)s-%(release)s.%(arch)s" % ext_rpm_info - with self.assertRaises(koji.GenericError) as ex: - kojihub.create_rpm_checksum(self.rpm_id, self.sigkey) - self.assertEqual(f'Not an internal rpm: {nvra} (from test-ext-rpm)', str(ex.exception)) - - def test_not_sigkey_related_to_rpm(self): - self.get_rpm.return_value = self.rpm_info - checksum_types = ['md5', 'sha256'] - nvra = "%(name)s-%(version)s-%(release)s.%(arch)s" % self.rpm_info - expected_err = f'There is no rpm {nvra} signed with {self.sigkey}' - self.single_value.return_value = None - with self.assertRaises(koji.GenericError) as ex: - kojihub.create_rpm_checksum(self.rpm_id, self.sigkey, checksum_types=checksum_types) - self.assertEqual(expected_err % self.rpm_info, str(ex.exception)) - - 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', 'sigkey=%(sigkey)s']) - def test_checksum_exists(self): - self.get_rpm.return_value = self.rpm_info - self.single_value.return_value = 'test-sighash' + src = 'test-src' + dst = 'test-dst' + rpm_id = 123 + chsum_list = {'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(self.rpm_id, self.sigkey) + result = kojihub.create_rpm_checksum(src, dst, rpm_id, sigkey, chsum_list) self.assertIsNone(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', 'sigkey=%(sigkey)s']) - - query = self.queries[1] - self.assertEqual(query.tables, ['rpm_checksum']) - self.assertEqual(query.joins, None) - self.assertEqual(query.clauses, - ["checksum_type IN %(checksum_types)s", "sigkey=%(sigkey)s"]) - - @mock_open() - def test_cannot_open_file(self, m_open): - self.get_rpm.return_value = self.rpm_info - self.get_build.return_value = {'build_id': 222} - self.single_value.return_value = 'test-sighash' - self.query_execute.return_value = [{'checksum_type': 'md5'}] - self.path_build.return_value = 'fakebuildpath' - self.path_signed.return_value = 'fakesignedpath' - m_open.side_effect = IOError() - - with self.assertRaises(koji.GenericError) as ex: - kojihub.create_rpm_checksum(self.rpm_id, self.sigkey) - self.assertEqual("RPM path fakebuildpath/fakesignedpath cannot be open.", - str(ex.exception)) - - self.assertEqual(len(self.queries), 2) + 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', 'sigkey=%(sigkey)s']) - - query = self.queries[1] self.assertEqual(query.tables, ['rpm_checksum']) self.assertEqual(query.joins, None) self.assertEqual(query.clauses, diff --git a/tests/test_hub/test_delete_build.py b/tests/test_hub/test_delete_build.py index ea35083..5714a50 100644 --- a/tests/test_hub/test_delete_build.py +++ b/tests/test_hub/test_delete_build.py @@ -6,13 +6,58 @@ from collections import defaultdict 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 @@ class TestDeleteBuild(unittest.TestCase): 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 @@ class TestDeleteBuild(unittest.TestCase): 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 @@ class TestDeleteBuild(unittest.TestCase): 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']) + diff --git a/tests/test_hub/test_delete_rpm_sig.py b/tests/test_hub/test_delete_rpm_sig.py index 3004720..5840f56 100644 --- a/tests/test_hub/test_delete_rpm_sig.py +++ b/tests/test_hub/test_delete_rpm_sig.py @@ -119,9 +119,15 @@ class TestDeleteRPMSig(unittest.TestCase): 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 @@ class TestDeleteRPMSig(unittest.TestCase): 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 @@ class TestDeleteRPMSig(unittest.TestCase): 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']) diff --git a/tests/test_hub/test_get_rpm_checksums.py b/tests/test_hub/test_get_rpm_checksums.py index 60f30f2..8024c8b 100644 --- a/tests/test_hub/test_get_rpm_checksums.py +++ b/tests/test_hub/test_get_rpm_checksums.py @@ -71,12 +71,25 @@ class TestGetRpmChecksums(unittest.TestCase): ['checksum_type IN %(checksum_type)s', 'rpm_id=%(rpm_id)i']) self.assertEqual(expected_result, result) - def test_missing_checksum_not_sigkey(self): + def test_missing_checksum_not_sigkey_without_strict(self): + rpm_id = 123 + 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']) + + def test_missing_checksum_not_sigkey_with_strict(self): rpm_id = 123 checksum_types = ['md5'] self.query_execute.side_effect = [[], []] with self.assertRaises(koji.GenericError) as ex: - self.exports.getRPMChecksums(rpm_id, checksum_types=checksum_types) + self.exports.getRPMChecksums(rpm_id, checksum_types=checksum_types, strict=True) self.assertEqual(f'No cached signature for rpm ID {rpm_id}.', str(ex.exception)) self.assertEqual(len(self.queries), 1) diff --git a/tests/test_hub/test_reset_build.py b/tests/test_hub/test_reset_build.py index 00d84b0..a51c945 100644 --- a/tests/test_hub/test_reset_build.py +++ b/tests/test_hub/test_reset_build.py @@ -87,7 +87,7 @@ class TestResetBuild(unittest.TestCase): 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']) @@ -109,66 +109,71 @@ class TestResetBuild(unittest.TestCase): self.assertEqual(delete.values, {'id': self.binfo['build_id']}) delete = self.deletes[4] + 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[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']}) @@ -182,4 +187,4 @@ class TestResetBuild(unittest.TestCase): self.assertEqual(rv, None) self.assertEqual(len(self.queries), 0) self.assertEqual(len(self.deletes), 0) - self.assertEqual(len(self.updates), 0) + self.assertEqual(len(self.updates), 0) \ No newline at end of file From d96426f222f76a2b2a888f55928c98d2df634864 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jan 25 2023 11:19:56 +0000 Subject: [PATCH 3/10] simplify splice_rpm_sighdr changes --- diff --git a/koji/__init__.py b/koji/__init__.py index 321b9ac..eab6078 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -943,8 +943,7 @@ def get_sighdr_key(sighdr): return get_sigpacket_key_id(sig) -def splice_rpm_sighdr(sighdr, src, dst=None, bufsize=8192, rpm_id=None, sigkey=None, - checksum_types=None, callback=None): +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) if dst is not None: @@ -954,18 +953,19 @@ def splice_rpm_sighdr(sighdr, src, dst=None, bufsize=8192, rpm_id=None, sigkey=N else: (fd, dst_temp) = tempfile.mkstemp() os.close(fd) - chsum_list = {chsum: getattr(hashlib, chsum)() for chsum in checksum_types} 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) + def do_write(buf): + dst_fo.write(buf) + if callback: + callback(buf) + do_write(src_fo.read(start)) + do_write(sighdr) src_fo.seek(size, 1) while True: buf = src_fo.read(bufsize) if not buf: break - dst_fo.write(buf) - for func, chsum in chsum_list.items(): - chsum.update(buf) + do_write(buf) if dst is not None: src_stats = os.stat(src) dst_temp_stats = os.stat(dst_temp) @@ -974,9 +974,6 @@ def splice_rpm_sighdr(sighdr, src, dst=None, bufsize=8192, rpm_id=None, sigkey=N os.rename(dst_temp, dst) else: dst = dst_temp - if callback: - callback(src=src, dst=dst, sighdr=sighdr, rpm_id=rpm_id, sigkey=sigkey, - chsum_list=chsum_list) return dst diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index 4cc5072..6e9b3f9 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -7911,7 +7911,7 @@ def check_rpm_sig(an_rpm, sigkey, sighdr): 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: @@ -7970,19 +7970,28 @@ def query_rpm_sigs(rpm_id=None, sigkey=None, queryOpts=None): return query.execute() -def calculate_chsum(signed_path, checksum_types): - chsum_list = {chsum: getattr(hashlib, chsum)() for chsum in checksum_types} +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 calculate_chsum(path, checksum_types): + msum = MultiSum(checksum_types) try: - with open(signed_path, 'rb') as f: + with open(path, 'rb') as f: while 1: chunk = f.read(1024 ** 2) if not chunk: break - for func, chsum in chsum_list.items(): - chsum.update(chunk) - except IOError: - raise koji.GenericError(f"RPM path {signed_path} cannot be open.") - return chsum_list + msum.update(chunk) + except IOError as e: + raise koji.GenericError(f"File {path} cannot be read -- {e}") + return msum.checksums def write_signed_rpm(an_rpm, sigkey, force=False, checksum_types=None): @@ -8030,8 +8039,9 @@ def write_signed_rpm(an_rpm, sigkey, force=False, checksum_types=None): with open(sigpath, 'rb') as fo: sighdr = fo.read() koji.ensuredir(os.path.dirname(signedpath)) - koji.splice_rpm_sighdr(sighdr, rpm_path, signedpath, rpm_id=rpm_id, sigkey=sigkey, - checksum_types=checksum_types, callback=create_rpm_checksum) + msum = MultiSum(checksum_types) + koji.splice_rpm_sighdr(sighdr, rpm_path, dst=signedpath, callback=msum.update) + create_rpm_checksum(rpm_id=rpm_id, sigkey=sigkey, chsum_list=msum.checksums) def query_history(tables=None, **kwargs): From 1d35f93ce711d4d9b993c112d4108ab8897ab7ca Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jan 25 2023 11:19:56 +0000 Subject: [PATCH 4/10] generator for reading spliced rpm signatures --- diff --git a/koji/__init__.py b/koji/__init__.py index eab6078..8cf8ef8 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -943,9 +943,30 @@ def get_sighdr_key(sighdr): return get_sigpacket_key_id(sig) +def spliced_sig_reader(path, sighdr, bufsize=8192): + """A generator that yields the contents of an rpm with signature spliced in""" + (start, size) = find_rpm_sighdr(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 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,19 +974,11 @@ def splice_rpm_sighdr(sighdr, src, dst=None, bufsize=8192, callback=None): else: (fd, dst_temp) = tempfile.mkstemp() os.close(fd) - with open(src, 'rb') as src_fo, open(dst_temp, 'wb') as dst_fo: - def do_write(buf): + with open(dst_temp, 'wb') as dst_fo: + for buf in reader: dst_fo.write(buf) if callback: callback(buf) - do_write(src_fo.read(start)) - do_write(sighdr) - src_fo.seek(size, 1) - while True: - buf = src_fo.read(bufsize) - if not buf: - break - do_write(buf) if dst is not None: src_stats = os.stat(src) dst_temp_stats = os.stat(dst_temp) From a89364d58f94f25f9b587a74ba98a85c87ce293f Mon Sep 17 00:00:00 2001 From: Jana Cupova Date: Jan 25 2023 11:20:52 +0000 Subject: [PATCH 5/10] Use strict for rpm without signed copies or checksums + small review fixes --- diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index 6e9b3f9..c6e9b25 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -8030,8 +8030,8 @@ def write_signed_rpm(an_rpm, sigkey, force=False, checksum_types=None): if os.path.exists(signedpath): if not force: # already present - chsum_list = calculate_chsum(signedpath, checksum_types) - create_rpm_checksum(rpm_id=rpm_id, sigkey=sigkey, chsum_list=chsum_list) + chsum_dict = calculate_chsum(signedpath, checksum_types) + create_rpm_checksum(rpm_id, sigkey, chsum_dict) return else: os.unlink(signedpath) @@ -8041,7 +8041,7 @@ def write_signed_rpm(an_rpm, sigkey, force=False, checksum_types=None): koji.ensuredir(os.path.dirname(signedpath)) msum = MultiSum(checksum_types) koji.splice_rpm_sighdr(sighdr, rpm_path, dst=signedpath, callback=msum.update) - create_rpm_checksum(rpm_id=rpm_id, sigkey=sigkey, chsum_list=msum.checksums) + create_rpm_checksum(rpm_id, sigkey, msum.checksums) def query_history(tables=None, **kwargs): @@ -12315,6 +12315,71 @@ class RootExports(object): queryRPMSigs = staticmethod(query_rpm_sigs) + def getRPMChecksums(self, rpm_id, checksum_types=None, cacheonly=False, strict=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 checsum 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()] + 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 not query_result or not sigkeys: + if strict: + nvra = "%(name)s-%(version)s-%(release)s.%(arch)s" % rpm_info + raise koji.GenericError( + f"Rpm {nvra} doesn't have cached checksums or signed copies.") + else: + return {} + 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 = list_checksums_sigkeys.copy() + 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']]) + + for sigkey, chsums in missing_chsum_sigkeys.items(): + builddir = koji.pathinfo.build(rpm_info) + rpm_path = os.path.join(builddir, koji.pathinfo.rpm(rpm_info)) + sig_path = os.path.join(builddir, koji.pathinfo.sighdr(rpm_info, sigkey)) + with open(sig_path, 'rb') as fo: + sighdr = fo.read() + msum = MultiSum(checksum_types) + for buf in koji.spliced_sig_reader(rpm_path, sighdr): + msum.update(buf) + create_rpm_checksum(rpm_id, sigkey, msum.checksums) + 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') @@ -13694,59 +13759,6 @@ class RootExports(object): values=locals(), opts=queryOpts) return query.iterate() - def getRPMChecksums(self, rpm_id, checksum_types=None, cacheonly=False, strict=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 checsum 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') - 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: - if strict: - raise koji.GenericError(f'No cached signature for rpm ID {rpm_id}.') - else: - 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 = list_checksums_sigkeys.copy() - 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']]) - - for sigkey, chsums in missing_chsum_sigkeys.items(): - write_signed_rpm(rpm_id, sigkey, checksum_types=list(chsums)) - query_result = query_checksum.execute() - return create_rpm_checksums_output(query_result, list_checksums_sigkeys) - class BuildRoot(object): @@ -15549,18 +15561,16 @@ def create_rpm_checksums_output(query_result, list_chsum_sigkeys): return result -def create_rpm_checksum(**kwargs): +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 list checksum_type: List of checksum types. """ - chsum_list = kwargs.get('chsum_list') - sigkey = kwargs.get('sigkey') - rpm_id = kwargs.get('rpm_id') + chsum_dict = chsum_dict.copy() - checksum_type_int = [koji.CHECKSUM_TYPES[func] for func, _ in chsum_list.items()] + checksum_type_int = [koji.CHECKSUM_TYPES[func] for func, _ in chsum_dict.items()] query = QueryProcessor(tables=['rpm_checksum'], columns=['checksum_type'], clauses=["checksum_type IN %(checksum_types)s", 'sigkey=%(sigkey)s'], values={'checksum_types': checksum_type_int, 'sigkey': sigkey}) @@ -15570,10 +15580,10 @@ def create_rpm_checksum(**kwargs): else: for r in rows: if r['checksum_type'] in checksum_type_int: - del chsum_list[koji.CHECKSUM_TYPES[r['checksum_type']]] - if chsum_list: + del chsum_dict[koji.CHECKSUM_TYPES[r['checksum_type']]] + if chsum_dict: insert = BulkInsertProcessor(table='rpm_checksum') - for func, chsum in chsum_list.items(): + for func, chsum in chsum_dict.items(): insert.add_record(rpm_id=rpm_id, sigkey=sigkey, checksum=chsum.hexdigest(), checksum_type=koji.CHECKSUM_TYPES[func]) insert.execute() diff --git a/tests/test_hub/test_create_rpm_checksum.py b/tests/test_hub/test_create_rpm_checksum.py index 1af67a5..3f4bef4 100644 --- a/tests/test_hub/test_create_rpm_checksum.py +++ b/tests/test_hub/test_create_rpm_checksum.py @@ -33,13 +33,11 @@ class TestCreateRPMChecksum(unittest.TestCase): return query def test_checksum_exists(self): - src = 'test-src' - dst = 'test-dst' rpm_id = 123 - chsum_list = {'md5': 'chsum-1', 'sha256': 'chsum-2'} + 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(src, dst, rpm_id, sigkey, chsum_list) + result = kojihub.create_rpm_checksum(rpm_id, sigkey, chsum_dict) self.assertIsNone(result) self.assertEqual(len(self.queries), 1) diff --git a/tests/test_hub/test_get_rpm_checksums.py b/tests/test_hub/test_get_rpm_checksums.py index 8024c8b..b32e23f 100644 --- a/tests/test_hub/test_get_rpm_checksums.py +++ b/tests/test_hub/test_get_rpm_checksums.py @@ -14,10 +14,14 @@ class TestGetRpmChecksums(unittest.TestCase): self.create_rpm_checksum_output = mock.patch( 'kojihub.kojihub.create_rpm_checksums_output').start() self.write_signed_rpm = mock.patch('kojihub.kojihub.write_signed_rpm').start() + self.get_rpm = mock.patch('kojihub.kojihub.get_rpm').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'} + self.nvra = "%(name)s-%(version)s-%(release)s.%(arch)s" % self.rpm_info def tearDown(self): mock.patch.stopall() @@ -36,6 +40,7 @@ class TestGetRpmChecksums(unittest.TestCase): 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) @@ -50,6 +55,7 @@ class TestGetRpmChecksums(unittest.TestCase): def test_all_checksum_exists(self): rpm_id = 123 + self.get_rpm.return_value = self.rpm_info checksum_types = ['md5', 'sha256'] expected_result = {'sigkey1': {'md5': 'checksum-md5', 'sha256': 'checksum-sha256'}} self.query_execute.side_effect = [ @@ -73,41 +79,59 @@ class TestGetRpmChecksums(unittest.TestCase): def test_missing_checksum_not_sigkey_without_strict(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) + 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.write_signed_rpm.assert_not_called() + self.create_rpm_checksum_output.assert_not_called() + def test_missing_checksum_not_sigkey_with_strict(self): rpm_id = 123 + self.get_rpm.return_value = self.rpm_info checksum_types = ['md5'] self.query_execute.side_effect = [[], []] with self.assertRaises(koji.GenericError) as ex: self.exports.getRPMChecksums(rpm_id, checksum_types=checksum_types, strict=True) - self.assertEqual(f'No cached signature for rpm ID {rpm_id}.', str(ex.exception)) + self.assertEqual(f"Rpm {self.nvra} doesn't have cached checksums or signed copies.", + str(ex.exception)) - self.assertEqual(len(self.queries), 1) + 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.write_signed_rpm.assert_not_called() + self.create_rpm_checksum_output.assert_not_called() + def test_missing_valid_checksum_generated(self): rpm_id = 123 checksum_types = ['md5'] - expected_result = {'sigkey1': {'md5': 'checksum-md5'}} + self.get_rpm.return_value = self.rpm_info + expected_result = {} self.query_execute.side_effect = [ [{'sigkey': 'sigkey-1'}], [], [{'checksum': 'checksum-md5', 'checksum_type': 0}]] - self.write_signed_rpm.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) @@ -122,9 +146,40 @@ class TestGetRpmChecksums(unittest.TestCase): self.assertEqual(query.joins, None) self.assertEqual(query.clauses, ['checksum_type IN %(checksum_type)s', 'rpm_id=%(rpm_id)i']) + self.write_signed_rpm.assert_not_called() + self.create_rpm_checksum_output.assert_not_called() + + def test_missing_valid_checksum_generated_with_strict(self): + rpm_id = 123 + checksum_types = ['md5'] + self.get_rpm.return_value = self.rpm_info + self.query_execute.side_effect = [ + [{'sigkey': 'sigkey-1'}], + [], + [{'checksum': 'checksum-md5', 'checksum_type': 0}]] + with self.assertRaises(koji.GenericError) as ex: + self.exports.getRPMChecksums(rpm_id, checksum_types=checksum_types, strict=True) + self.assertEqual(f"Rpm {self.nvra} doesn't have cached checksums or signed copies.", + str(ex.exception)) + + 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.write_signed_rpm.assert_not_called() + self.create_rpm_checksum_output.assert_not_called() def test_missing_valid_more_checksum_generated_and_exists(self): rpm_id = 123 + self.get_rpm.return_value = self.rpm_info checksum_types = ['md5', 'sha256'] expected_result = {'sigkey1': {'md5': 'checksum-md5', 'sha256': 'checksum-sha256'}} self.query_execute.side_effect = [ @@ -151,6 +206,7 @@ class TestGetRpmChecksums(unittest.TestCase): def test_missing_valid_more_checksum_generated_and_exists_more_sigkeys(self): rpm_id = 123 + self.get_rpm.return_value = self.rpm_info checksum_types = ['md5', 'sha256'] expected_result = {'sigkey1': {'md5': 'checksum-md5', 'sha256': 'checksum-sha256'}, 'sigkey2': {'md5': 'checksum-md5', 'sha256': 'checksum-sha256'}} diff --git a/tests/test_hub/test_reset_build.py b/tests/test_hub/test_reset_build.py index a51c945..7ea681c 100644 --- a/tests/test_hub/test_reset_build.py +++ b/tests/test_hub/test_reset_build.py @@ -187,4 +187,4 @@ class TestResetBuild(unittest.TestCase): self.assertEqual(rv, None) self.assertEqual(len(self.queries), 0) self.assertEqual(len(self.deletes), 0) - self.assertEqual(len(self.updates), 0) \ No newline at end of file + self.assertEqual(len(self.updates), 0) From a591741d173fb126057528ed456ad74b969dfb7d Mon Sep 17 00:00:00 2001 From: Jana Cupova Date: Jan 25 2023 11:24:47 +0000 Subject: [PATCH 6/10] Rewrite generator to IOStream --- diff --git a/koji/__init__.py b/koji/__init__.py index 8cf8ef8..a0f2d7f 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -57,6 +57,7 @@ except ImportError: # pragma: no cover from fnmatch import fnmatch import dateutil.parser +import io import requests import six import six.moves.configparser @@ -945,23 +946,47 @@ def get_sighdr_key(sighdr): def spliced_sig_reader(path, sighdr, bufsize=8192): """A generator that yields the contents of an rpm with signature spliced in""" - (start, size) = find_rpm_sighdr(path) - with open(path, 'rb') as fo: - # the part before the signature - yield fo.read(start) - - # the spliced signature - yield sighdr + 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 - # skip original signature - fo.seek(size, 1) + def readable(self): + return True - # the part after the signature - while True: - buf = fo.read(bufsize) - if not buf: - break - yield buf + 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) def splice_rpm_sighdr(sighdr, src, dst=None, bufsize=8192, callback=None): diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index c6e9b25..6423c76 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -27,6 +27,7 @@ from __future__ import absolute_import import base64 import builtins import calendar +import copy import datetime import fcntl import fnmatch @@ -7981,16 +7982,26 @@ class MultiSum(object): 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) - try: - with open(path, 'rb') as f: - while 1: - chunk = f.read(1024 ** 2) - if not chunk: - break - msum.update(chunk) - except IOError as e: - raise koji.GenericError(f"File {path} cannot be read -- {e}") + 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.checksums @@ -12323,6 +12334,7 @@ class RootExports(object): :param bool cacheonly: when False, checksum is created for missing checksum type when True, checksum is returned as None when checsum is missing for specific checksum type + :param bool strict: if rpm checksum or signed copies not found for an rpm, raise error :returns: A dict of specific checksum types and checksums """ if not isinstance(rpm_id, int): @@ -12340,6 +12352,23 @@ class RootExports(object): 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 {} + builddir = koji.pathinfo.build(rpm_info) + for s in sigkeys: + signedpath = "%s/%s" % (builddir, koji.pathinfo.signed(rpm_info, s)) + sig_path = os.path.join(builddir, koji.pathinfo.sighdr(rpm_info, s)) + if not os.path.exists(signedpath) or not os.path.exists(sig_path): + if strict: + nvra = "%(name)s-%(version)s-%(release)s.%(arch)s" % rpm_info + raise koji.GenericError(f"Rpm {nvra} doesn't have cached signed copies.") + else: + chsum_dict = {} + for sigkey in sigkeys: + chsum_dict.setdefault( + sigkey, dict(zip(checksum_types, [None] * len(checksum_types)))) + return chsum_dict + list_checksums_sigkeys = {s: set(checksum_types) for s in sigkeys} checksum_type_int = [koji.CHECKSUM_TYPES[chsum] for chsum in checksum_types] @@ -12350,33 +12379,23 @@ class RootExports(object): values={'rpm_id': rpm_id, 'checksum_type': checksum_type_int}) query_result = query_checksum.execute() - if not query_result or not sigkeys: - if strict: - nvra = "%(name)s-%(version)s-%(release)s.%(arch)s" % rpm_info - raise koji.GenericError( - f"Rpm {nvra} doesn't have cached checksums or signed copies.") - else: - return {} - query_result = query_checksum.execute() - if len(query_result) == (len(checksum_type_int) * len(sigkeys)) or cacheonly: + 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 = list_checksums_sigkeys.copy() + 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']]) + rpm_path = os.path.join(builddir, koji.pathinfo.rpm(rpm_info)) for sigkey, chsums in missing_chsum_sigkeys.items(): - builddir = koji.pathinfo.build(rpm_info) - rpm_path = os.path.join(builddir, koji.pathinfo.rpm(rpm_info)) sig_path = os.path.join(builddir, koji.pathinfo.sighdr(rpm_info, sigkey)) with open(sig_path, 'rb') as fo: sighdr = fo.read() - msum = MultiSum(checksum_types) - for buf in koji.spliced_sig_reader(rpm_path, sighdr): - msum.update(buf) - create_rpm_checksum(rpm_id, sigkey, msum.checksums) + with koji.spliced_sig_reader(rpm_path, sighdr) as f: + chsums_dict = calculate_chsum(f, chsums) + create_rpm_checksum(rpm_id, sigkey, chsums_dict) query_result = query_checksum.execute() return create_rpm_checksums_output(query_result, list_checksums_sigkeys) @@ -15566,9 +15585,9 @@ def create_rpm_checksum(rpm_id, sigkey, chsum_dict): :param int rpm_id: RPM id :param string sigkey: Sigkey for specific RPM - :param list checksum_type: List of checksum types. + :param dict chsum_dict: Dict of checksum type and hash. """ - chsum_dict = chsum_dict.copy() + chsum_dict = copy.deepcopy(chsum_dict) checksum_type_int = [koji.CHECKSUM_TYPES[func] for func, _ in chsum_dict.items()] query = QueryProcessor(tables=['rpm_checksum'], columns=['checksum_type'], diff --git a/tests/test_hub/test_create_rpm_checksum.py b/tests/test_hub/test_create_rpm_checksum.py index 3f4bef4..9fd5524 100644 --- a/tests/test_hub/test_create_rpm_checksum.py +++ b/tests/test_hub/test_create_rpm_checksum.py @@ -1,20 +1,11 @@ import unittest import mock -import six import kojihub QP = kojihub.QueryProcessor -def mock_open(): - """Return the right patch decorator for open""" - if six.PY2: - return mock.patch('__builtin__.open') - else: - return mock.patch('builtins.open') - - class TestCreateRPMChecksum(unittest.TestCase): def setUp(self): self.maxDiff = None diff --git a/tests/test_hub/test_get_rpm_checksums.py b/tests/test_hub/test_get_rpm_checksums.py index b32e23f..592bf35 100644 --- a/tests/test_hub/test_get_rpm_checksums.py +++ b/tests/test_hub/test_get_rpm_checksums.py @@ -11,9 +11,11 @@ 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.write_signed_rpm = mock.patch('kojihub.kojihub.write_signed_rpm').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.QueryProcessor = mock.patch('kojihub.kojihub.QueryProcessor', side_effect=self.getQuery).start() @@ -37,6 +39,10 @@ class TestGetRpmChecksums(unittest.TestCase): 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 @@ -45,6 +51,10 @@ class TestGetRpmChecksums(unittest.TestCase): 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 @@ -52,16 +62,21 @@ class TestGetRpmChecksums(unittest.TestCase): 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 = {'sigkey1': {'md5': 'checksum-md5', 'sha256': 'checksum-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': 'test-sigkey'}, - {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'test-sigkey'}]] + [{'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) @@ -76,8 +91,16 @@ class TestGetRpmChecksums(unittest.TestCase): 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_without_strict(self): + def test_missing_checksum_not_sigkey(self): rpm_id = 123 self.get_rpm.return_value = self.rpm_info checksum_types = ['md5'] @@ -85,53 +108,32 @@ class TestGetRpmChecksums(unittest.TestCase): result = self.exports.getRPMChecksums(rpm_id, checksum_types=checksum_types) self.assertEqual({}, result) - self.assertEqual(len(self.queries), 2) + 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']) - 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.write_signed_rpm.assert_not_called() + 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_missing_checksum_not_sigkey_with_strict(self): - rpm_id = 123 - self.get_rpm.return_value = self.rpm_info - checksum_types = ['md5'] - self.query_execute.side_effect = [[], []] - with self.assertRaises(koji.GenericError) as ex: - self.exports.getRPMChecksums(rpm_id, checksum_types=checksum_types, strict=True) - self.assertEqual(f"Rpm {self.nvra} doesn't have cached checksums or signed copies.", - str(ex.exception)) - - 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.write_signed_rpm.assert_not_called() - self.create_rpm_checksum_output.assert_not_called() - - def test_missing_valid_checksum_generated(self): + @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 - expected_result = {} + 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) @@ -146,49 +148,51 @@ class TestGetRpmChecksums(unittest.TestCase): self.assertEqual(query.joins, None) self.assertEqual(query.clauses, ['checksum_type IN %(checksum_type)s', 'rpm_id=%(rpm_id)i']) - self.write_signed_rpm.assert_not_called() - self.create_rpm_checksum_output.assert_not_called() + 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'}} + ) def test_missing_valid_checksum_generated_with_strict(self): + self.os_path.return_value = False rpm_id = 123 checksum_types = ['md5'] self.get_rpm.return_value = self.rpm_info - self.query_execute.side_effect = [ - [{'sigkey': 'sigkey-1'}], - [], - [{'checksum': 'checksum-md5', 'checksum_type': 0}]] + self.query_execute.return_value = [{'sigkey': 'sigkey-1'}] with self.assertRaises(koji.GenericError) as ex: self.exports.getRPMChecksums(rpm_id, checksum_types=checksum_types, strict=True) - self.assertEqual(f"Rpm {self.nvra} doesn't have cached checksums or signed copies.", + self.assertEqual(f"Rpm {self.nvra} doesn't have cached signed copies.", str(ex.exception)) - self.assertEqual(len(self.queries), 2) + 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']) - 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.write_signed_rpm.assert_not_called() + 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_missing_valid_more_checksum_generated_and_exists(self): + @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 checksum_types = ['md5', 'sha256'] - expected_result = {'sigkey1': {'md5': 'checksum-md5', 'sha256': 'checksum-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': 'test-sigkey'}], - [{'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'test-sigkey'}, - {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'test-sigkey'}]] - self.write_signed_rpm.return_value = None + [{'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) @@ -204,22 +208,38 @@ class TestGetRpmChecksums(unittest.TestCase): self.assertEqual(query.clauses, ['checksum_type IN %(checksum_type)s', 'rpm_id=%(rpm_id)i']) - def test_missing_valid_more_checksum_generated_and_exists_more_sigkeys(self): + 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 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': 'sigkey-1'}, {'sigkey': 'sigkey-2'}], - [{'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey-1'}, - {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'sigkey-2'}], - [{'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey-1'}, - {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'sigkey-1'}, - {'checksum': 'checksum-md5', 'checksum_type': 0, 'sigkey': 'sigkey-2'}, - {'checksum': 'checksum-sha256', 'checksum_type': 2, 'sigkey': 'sigkey-2'}]] - self.write_signed_rpm.return_value = None + [{'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) @@ -234,3 +254,16 @@ class TestGetRpmChecksums(unittest.TestCase): 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'}} + ) diff --git a/tests/test_hub/test_reset_build.py b/tests/test_hub/test_reset_build.py index 7ea681c..13d31b2 100644 --- a/tests/test_hub/test_reset_build.py +++ b/tests/test_hub/test_reset_build.py @@ -104,15 +104,15 @@ class TestResetBuild(unittest.TestCase): self.assertEqual(delete.values, {'rpm_id': 123}) delete = self.deletes[3] - 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] 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[5] self.assertEqual(delete.table, 'maven_archives') self.assertEqual(delete.clauses, ['archive_id=%(archive_id)i']) From c3be0c4a1ba65f0f9bcfcee85e8cc97c86bf6513 Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jan 30 2023 06:48:19 +0000 Subject: [PATCH 7/10] simple unit tests for splicing code --- diff --git a/tests/test_lib/data/rpms/test-deps-1-1.fc24.x86_64.rpm.signed b/tests/test_lib/data/rpms/test-deps-1-1.fc24.x86_64.rpm.signed new file mode 100644 index 0000000..eaf322e Binary files /dev/null and b/tests/test_lib/data/rpms/test-deps-1-1.fc24.x86_64.rpm.signed differ diff --git a/tests/test_lib/test_spliced_sig.py b/tests/test_lib/test_spliced_sig.py new file mode 100644 index 0000000..107bcd8 --- /dev/null +++ b/tests/test_lib/test_spliced_sig.py @@ -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) From 46a83b6d3afa651cb09355bfd7ab066d0d45ea1e Mon Sep 17 00:00:00 2001 From: Mike McLean Date: Jan 30 2023 06:48:27 +0000 Subject: [PATCH 8/10] use signed copy for checksum if avail, drop strict option --- diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index 6423c76..7294dbc 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -12326,15 +12326,14 @@ class RootExports(object): queryRPMSigs = staticmethod(query_rpm_sigs) - def getRPMChecksums(self, rpm_id, checksum_types=None, cacheonly=False, strict=False): + 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 checsum is missing + when True, checksum is returned as None when checksum is missing for specific checksum type - :param bool strict: if rpm checksum or signed copies not found for an rpm, raise error :returns: A dict of specific checksum types and checksums """ if not isinstance(rpm_id, int): @@ -12354,20 +12353,6 @@ class RootExports(object): sigkeys = [r['sigkey'] for r in query.execute()] if not sigkeys: return {} - builddir = koji.pathinfo.build(rpm_info) - for s in sigkeys: - signedpath = "%s/%s" % (builddir, koji.pathinfo.signed(rpm_info, s)) - sig_path = os.path.join(builddir, koji.pathinfo.sighdr(rpm_info, s)) - if not os.path.exists(signedpath) or not os.path.exists(sig_path): - if strict: - nvra = "%(name)s-%(version)s-%(release)s.%(arch)s" % rpm_info - raise koji.GenericError(f"Rpm {nvra} doesn't have cached signed copies.") - else: - chsum_dict = {} - for sigkey in sigkeys: - chsum_dict.setdefault( - sigkey, dict(zip(checksum_types, [None] * len(checksum_types)))) - return chsum_dict list_checksums_sigkeys = {s: set(checksum_types) for s in sigkeys} @@ -12388,13 +12373,21 @@ class RootExports(object): missing_chsum_sigkeys[r['sigkey']].remove( koji.CHECKSUM_TYPES[r['checksum_type']]) - rpm_path = os.path.join(builddir, koji.pathinfo.rpm(rpm_info)) + if missing_chsum_sigkeys: + binfo = get_build(rpm_info['build_id']) + builddir = koji.pathinfo.build(binfo) + rpm_path = koji.joinpath(builddir, koji.pathinfo.rpm(rpm_info)) for sigkey, chsums in missing_chsum_sigkeys.items(): - sig_path = os.path.join(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 f: - chsums_dict = calculate_chsum(f, chsums) + signedpath = koji.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 = koji.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) From a3166c89a2a025b8da81027f5b51ddf4bf24c7b3 Mon Sep 17 00:00:00 2001 From: Jana Cupova Date: Jan 30 2023 17:36:28 +0000 Subject: [PATCH 9/10] Move class out of function and create to_hexdigest function --- diff --git a/koji/__init__.py b/koji/__init__.py index a0f2d7f..2eee932 100644 --- a/koji/__init__.py +++ b/koji/__init__.py @@ -944,49 +944,51 @@ def get_sighdr_key(sighdr): return get_sigpacket_key_id(sig) -def spliced_sig_reader(path, sighdr, bufsize=8192): - """A generator that yields the contents of an rpm with signature spliced in""" - 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 +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 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 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) + +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): diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index 7294dbc..469b839 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -7980,6 +7980,12 @@ class MultiSum(object): 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 @@ -8002,7 +8008,7 @@ def calculate_chsum(path, checksum_types): break msum.update(chunk) f.close() - return msum.checksums + return msum.to_hexdigest() def write_signed_rpm(an_rpm, sigkey, force=False, checksum_types=None): @@ -8052,7 +8058,7 @@ def write_signed_rpm(an_rpm, sigkey, force=False, checksum_types=None): koji.ensuredir(os.path.dirname(signedpath)) msum = MultiSum(checksum_types) koji.splice_rpm_sighdr(sighdr, rpm_path, dst=signedpath, callback=msum.update) - create_rpm_checksum(rpm_id, sigkey, msum.checksums) + create_rpm_checksum(rpm_id, sigkey, msum.to_hexdigest()) def query_history(tables=None, **kwargs): @@ -12376,14 +12382,14 @@ class RootExports(object): if missing_chsum_sigkeys: binfo = get_build(rpm_info['build_id']) builddir = koji.pathinfo.build(binfo) - rpm_path = koji.joinpath(builddir, koji.pathinfo.rpm(rpm_info)) + rpm_path = joinpath(builddir, koji.pathinfo.rpm(rpm_info)) for sigkey, chsums in missing_chsum_sigkeys.items(): - signedpath = koji.joinpath(builddir, koji.pathinfo.signed(rpm_info, sigkey)) + 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 = koji.joinpath(builddir, koji.pathinfo.sighdr(rpm_info, sigkey)) + 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: @@ -15596,6 +15602,6 @@ def create_rpm_checksum(rpm_id, sigkey, chsum_dict): 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.hexdigest(), + insert.add_record(rpm_id=rpm_id, sigkey=sigkey, checksum=chsum, checksum_type=koji.CHECKSUM_TYPES[func]) insert.execute() diff --git a/tests/test_hub/test_get_rpm_checksums.py b/tests/test_hub/test_get_rpm_checksums.py index 592bf35..3fec3d3 100644 --- a/tests/test_hub/test_get_rpm_checksums.py +++ b/tests/test_hub/test_get_rpm_checksums.py @@ -17,13 +17,15 @@ class TestGetRpmChecksums(unittest.TestCase): 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'} + '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() @@ -125,6 +127,7 @@ class TestGetRpmChecksums(unittest.TestCase): 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 = [ @@ -155,33 +158,12 @@ class TestGetRpmChecksums(unittest.TestCase): [{'checksum': 'checksum-md5', 'checksum_type': 0}], {'sigkey-1': {'md5'}} ) - def test_missing_valid_checksum_generated_with_strict(self): - self.os_path.return_value = False - rpm_id = 123 - checksum_types = ['md5'] - self.get_rpm.return_value = self.rpm_info - self.query_execute.return_value = [{'sigkey': 'sigkey-1'}] - with self.assertRaises(koji.GenericError) as ex: - self.exports.getRPMChecksums(rpm_id, checksum_types=checksum_types, strict=True) - self.assertEqual(f"Rpm {self.nvra} doesn't have cached signed copies.", - str(ex.exception)) - - 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_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'}} @@ -222,6 +204,7 @@ class TestGetRpmChecksums(unittest.TestCase): 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'}} From 1461c5f00b998de873ac5656055c3adbd4227e20 Mon Sep 17 00:00:00 2001 From: Jana Cupova Date: Jan 30 2023 20:15:02 +0000 Subject: [PATCH 10/10] Add comparison between checksum from DB and caltulated checksum --- diff --git a/kojihub/kojihub.py b/kojihub/kojihub.py index 469b839..7ebd57d 100644 --- a/kojihub/kojihub.py +++ b/kojihub/kojihub.py @@ -8011,13 +8011,9 @@ def calculate_chsum(path, checksum_types): return msum.to_hexdigest() -def write_signed_rpm(an_rpm, sigkey, force=False, checksum_types=None): +def write_signed_rpm(an_rpm, sigkey, force=False): """Write a signed copy of the rpm""" - if checksum_types is None: - checksum_types = context.opts.get('RPMDefaultChecksums').split() - else: - if not isinstance(checksum_types, (list, tuple)): - raise koji.ParameterError(f'Invalid type of checksum_types: {type(checksum_types)}') + 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") @@ -15586,10 +15582,11 @@ def create_rpm_checksum(rpm_id, sigkey, chsum_dict): :param string sigkey: Sigkey for specific RPM :param dict chsum_dict: Dict of checksum type and hash. """ - chsum_dict = copy.deepcopy(chsum_dict) + 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'], + 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() @@ -15598,7 +15595,13 @@ def create_rpm_checksum(rpm_id, sigkey, chsum_dict): else: for r in rows: if r['checksum_type'] in checksum_type_int: - del chsum_dict[koji.CHECKSUM_TYPES[r['checksum_type']]] + 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():