From 0affc8c56b8920b217942d79fa31b4b4dab32d07 Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Aug 26 2019 14:47:12 +0000 Subject: [PATCH 1/2] improve PEP8 compliancy Add basic flake8 configuration and fix some things (whitespace, ordering issues, etc.) that it flagged. Signed-off-by: Nils Philippsen --- diff --git a/robosignatory/coreos.py b/robosignatory/coreos.py index 37d8466..09dc963 100644 --- a/robosignatory/coreos.py +++ b/robosignatory/coreos.py @@ -32,7 +32,8 @@ class CoreOSSigner(object): log.info('CoreOSSigner ready for service') def consume(self, msg): - # Message structure: https://github.com/coreos/fedora-coreos-tracker/issues/198#issuecomment-513944390 + # Message structure: + # https://github.com/coreos/fedora-coreos-tracker/issues/198#issuecomment-513944390 log.info( 'CoreOS wants to sign ' '%(build_id)s on %(stream)s for %(basearch)s' % msg.body diff --git a/robosignatory/tag.py b/robosignatory/tag.py index 97093a0..19369df 100644 --- a/robosignatory/tag.py +++ b/robosignatory/tag.py @@ -1,12 +1,13 @@ from __future__ import unicode_literals, absolute_import +import logging + import koji import robosignatory.utils as utils -import logging -log = logging.getLogger("robosignatory.tagconsumer") +log = logging.getLogger("robosignatory.tagconsumer") KNOWN_TAG_TYPES = ['plain', 'modular'] @@ -129,9 +130,11 @@ class TagSigner(object): log.info('Signing all module content') for build in instance['client'].listTagged(content_koji_tag): if build['owner_name'] != instance['mbs_user']: - log.error('Build %(build_id)s has owner %(owner_name)s, which is NOT mbs_user!' % build) + log.error('Build %(build_id)s has owner %(owner_name)s, which is NOT mbs_user!' + % build) raise Exception('Modular content tag contains invalid owned build') - self.signwrite_single_build(build['nvr'], build['build_id'], tag_info, instance, koji_instance) + self.signwrite_single_build(build['nvr'], build['build_id'], tag_info, instance, + koji_instance) def signwrite_single_build(self, build_nvr, build_id, tag_info, instance, koji_instance): log.info('Signing and writing build %s', build_nvr) @@ -140,9 +143,8 @@ class TagSigner(object): build_id=build_id, sigkey=tag_info['keyid']) log.info('RPMs to sign and move: %s', - ['%s (%s, signed: %s)' % - (key, rpm['id'], rpm['signed']) - for key, rpm in rpms.items()]) + ['%s (%s, signed: %s)' % (key, rpm['id'], rpm['signed']) + for key, rpm in rpms.items()]) if len(rpms) < 1: log.info('Build contains no rpms, skipping signing and writing') diff --git a/robosignatory/utils.py b/robosignatory/utils.py index 9b27889..c18f736 100644 --- a/robosignatory/utils.py +++ b/robosignatory/utils.py @@ -9,7 +9,7 @@ log = logging.getLogger('robosignatory.utils') class Struct: def __init__(self, **entries): - self.__dict__.update(entries) + self.__dict__.update(entries) def get_rpms(koji_client, build_nvr, build_id, sigkey=None): @@ -24,12 +24,13 @@ def get_rpms(koji_client, build_nvr, build_id, sigkey=None): rpminfo['%s.%s' % (rpm['nvr'], rpm['arch'])] = info return rpminfo + def get_builds_in_tag(koji_client, tag): """ Return the list of builds in Koji tag. """ try: rpms, builds = koji_client.listTaggedRPMS(tag, latest=True) - except koji.GenericError as e: + except koji.GenericError: log.exception("Failed to list rpms in tag %r" % tag) # If the tag doesn't exist.. then there are no rpms in that tag. return [] diff --git a/robosignatory/work.py b/robosignatory/work.py index da59728..60303e3 100644 --- a/robosignatory/work.py +++ b/robosignatory/work.py @@ -38,8 +38,8 @@ def process_atomic(signer, ref, commitid, key, directory, doref=True): # Sigul writes it as 0600, which makes a lot of sense as a general file # mode for it, but this is just a signature file that we want published - os.chmod(commitmetapath, (stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP | - stat.S_IROTH)) + os.chmod(commitmetapath, (stat.S_IRUSR | stat.S_IWUSR | stat.S_IRGRP + | stat.S_IROTH)) log.info('Commit was succesfully signed, writing ref %s' % ref) diff --git a/setup.cfg b/setup.cfg new file mode 100644 index 0000000..022312d --- /dev/null +++ b/setup.cfg @@ -0,0 +1,8 @@ +[flake8] +show-source = True +max-line-length = 100 +exclude = .git,.tox,dist,*egg,build,tools +ignore = E712,W503 +# Configure flake8-import-order +application-import-names = robosignatory +import-order-style = pep8 diff --git a/tests/test_utils.py b/tests/test_utils.py index c07b898..49dc415 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -4,6 +4,7 @@ from pytest import raises import robosignatory.utils + class TestUtils(unittest.TestCase): def test_no_such_helper(self): From 52a0d38eab80606d500c15cccecf9d0c169d2b1f Mon Sep 17 00:00:00 2001 From: Nils Philippsen Date: Aug 28 2019 11:44:31 +0000 Subject: [PATCH 2/2] Recognize and sign side tag builds Fixes: #28 Signed-off-by: Nils Philippsen --- diff --git a/README.rst b/README.rst index 67d5d7c..bbfe363 100644 --- a/README.rst +++ b/README.rst @@ -66,7 +66,10 @@ It is a list, with each entry being a dict with the tag that should be watched, a key name (passed to the signing module to indicate which key to use) and keyid (passed to koji to indicate which signatures need to be written out). The entry also has a "to" tag, to indicate where builds need to be moved to -after being signed. +after being signed. Each entry can have an optional dict "sidetags" which +specify how a side tag for the respective release and its corresponding "from" +and "to" tags would look like, as well as which Koji users are trusted to ask +RoboSignatory to sign such builds ("trusted_taggers", a list of strings). Note that "from" and "to" can be the same tag, in this case builds will not be moved after being signed. @@ -77,7 +80,13 @@ Example: "from": "f26-pending", "to": "f26", "key": "fedora-26", - "keyid": "64dab85d" + "keyid": "64dab85d", + "sidetags": { + "pattern": "-build-side-", + "from": "-pending-signing", + "to": "-testing", + "trusted_taggers": ["bodhi"] + } } This example would watch for any builds tagged into the f26-pending tag. @@ -91,6 +100,10 @@ If that is done and the "from" tag is different from the "to" tag, it will issue a koji moveBuild operation, moving the build from "f26-pending" to "f26". After this, it is done signing the package, and continues to the next step. +Analogously, it would work for a side tag build which was e.g. tagged into +"f26-build-side-1234-pending-signing" and move it into +"f26-build-side-1234-testing". + Testing Koji Tag Signer ----------------------- diff --git a/robosignatory.toml b/robosignatory.toml index e64a6cd..c2c4cab 100644 --- a/robosignatory.toml +++ b/robosignatory.toml @@ -97,6 +97,12 @@ handlers = ["console"] key = 'fedora26' keyid = 'xxxxxxxx' + [consumer_config.koji_instances.primary.tags.sidetags] + pattern = '-build-side-' + from = '-pending-signing' + to = '-testing' + trusted_taggers = ['bodhi'] + [[consumer_config.koji_instances.primary.tags]] from = 'rawhide-modular-signcandidate' to = 'rawhide-modular' diff --git a/robosignatory/tag.py b/robosignatory/tag.py index 19369df..4c4a0ea 100644 --- a/robosignatory/tag.py +++ b/robosignatory/tag.py @@ -1,9 +1,12 @@ from __future__ import unicode_literals, absolute_import import logging +import re import koji +import six + import robosignatory.utils as utils @@ -39,6 +42,7 @@ class TagSigner(object): raise Exception('Only SSL and kerberos authmethods supported') instance_obj = {'client': client, + 'sidetags': {}, 'tags': {}} if 'mbs_user' in instance_info: instance_obj['mbs_user'] = instance_info['mbs_user'] @@ -56,6 +60,51 @@ class TagSigner(object): raise Exception('Invalid tag type detected: %s' % tag_type) instance_obj['tags'][tag['from']]['type'] = tag_type + sidetags = tag.get('sidetags') + if not sidetags: + continue + + # Fill in from, to tags in placeholders, this will be the key + # in instance_obj['sidetags']. + pattern_filled_in = sidetags['pattern'].replace( + '', tag['from'] + ).replace( + '', tag['to'] + ) + + # Escape and construct regex to match the + # '-pending-signing' tag to which we should react. + from_tag_re = re.compile( + sidetags['from'].replace( + '', + r'(?P' + + re.escape(pattern_filled_in) + + r')' + ).replace(re.escape(''), r'(?P[1-9][0-9]*)') + ) + + # We don't want to accept '-pending-signing' tagged + # builds from just anyone. + + # The 'trusted_taggers' option is a list which defines the user + # ids from which we want to accept builds tagged into the + # '-pending-signing' tag. + trusted_taggers = sidetags['trusted_taggers'] + if ( + not isinstance(trusted_taggers, list) + or not all(isinstance(x, six.text_type) for x in trusted_taggers) + ): + raise TypeError("`trusted_taggers` must be a list of strings.") + + instance_obj['sidetags'][pattern_filled_in] = { + 'pattern': sidetags['pattern'], + 'from_tag_re': from_tag_re, + 'from': sidetags['from'], + 'to': sidetags['to'], + 'tags_key': tag['from'], + 'trusted_taggers': trusted_taggers, + } + self.koji_clients[instance] = instance_obj log.info('TagSigner ready for service') @@ -86,6 +135,54 @@ class TagSigner(object): self.dowork(build_nvr, build_id, tag, koji_instance, skip_tagging=False) + def match_sidetag(self, build_nvr, build_id, tag, koji_instance): + instance = self.koji_clients[koji_instance] + + for pattern_filled_in, sidetag_matched in instance['sidetags'].items(): + from_tag_re = sidetag_matched['from_tag_re'] + + m = from_tag_re.match(tag) + if not m: + continue + + sidetag = m.group('sidetag') + + # Verify that the build was tagged into the tag by one of the + # trusted taggers. + for hist_event in sorted(instance['client'].tagHistory(build_id), + key=lambda ev: ev['create_ts'], + reverse=True): + # We want to verify the latest matching tagging event only. + if ( + hist_event['active'] + and hist_event['tag_name'] == tag + ): + if hist_event['creator_name'] in sidetag_matched['trusted_taggers']: + # This is how it should be, break out of the loop. + break + + log.error("Side tag build not tagged into %s by trusted user (%s)" + " but by '%s'!", + tag, ", ".join(sidetag_matched['trusted_taggers']), + hist_event['creator_name']) + raise Exception('Side tag build tagged by untrusted user') + else: + # We should find the tag as active in the tagging history + # of the build. Bail out because we didn't find it above + # i.e. didn't break out of the loop. + log.error("Couldn't find tag %s in history of build %s (%s)!", + tag, build_nvr, build_id) + raise Exception("Tag not found in build history") + + tag_info = instance['tags'][sidetag_matched['tags_key']] + tag_to = sidetag_matched['to'].replace('', sidetag) + + tag_matched = True + + return tag_matched, tag_info, tag_to + + return False, None, None + def dowork(self, build_nvr, build_id, tag, koji_instance, skip_tagging=False): instance = self.koji_clients[koji_instance] @@ -93,14 +190,22 @@ class TagSigner(object): if not build_id: build_id = instance['client'].findBuildID(build_nvr) - if tag not in instance['tags']: + tag_matched = False + + if tag in instance['tags']: + tag_matched = True + tag_info = instance['tags'][tag] + tag_to = tag_info['to'] + else: + tag_matched, tag_info, tag_to = self.match_sidetag(build_nvr, build_id, tag, + koji_instance) + + if not tag_matched: log.info('Tag not autosigned, skipping') return - tag_info = instance['tags'][tag] log.info('Going to sign %s with %s (%s) and move to %s', - build_nvr, tag_info['key'], tag_info['keyid'], - tag_info['to']) + build_nvr, tag_info['key'], tag_info['keyid'], tag_to) if tag_info['type'] == 'plain': self.signwrite_single_build(build_nvr, build_id, tag_info, instance, koji_instance) @@ -112,13 +217,11 @@ class TagSigner(object): if skip_tagging: log.info('Tagging skipped, done') else: - log.info('Packages correctly signed, moving to %s' % - tag_info['to']) - if tag == tag_info['to']: + log.info('Packages correctly signed, moving to %s' % tag_to) + if tag == tag_to: log.info('Non-gated, not moving') else: - instance['client'].tagBuild(tag_info['to'], build_id, False, - tag) + instance['client'].tagBuild(tag_to, build_id, False, tag) def signwrite_module(self, build_nvr, build_id, tag_info, instance, koji_instance): log.info('Signing module build %s', build_nvr) diff --git a/tests/test_tag.py b/tests/test_tag.py index dfb6476..4d06b09 100644 --- a/tests/test_tag.py +++ b/tests/test_tag.py @@ -35,6 +35,12 @@ TEST_CONFIG = { 'to': 'f31', 'key': 'fedora-31', 'keyid': 'deadbeef', + 'sidetags': { + 'pattern': '-build-side-', + 'from': '-pending-signing', + 'to': '-testing', + 'trusted_taggers': ['bodhi'], + }, }, { 'from': 'f30-signing-pending', @@ -238,21 +244,97 @@ class TestTagSigner(object): # Test well-formed messages. + def _prep_sidetag_test(self, sidetag_pending, error): + tags_key = self.test_msg['body']['tag'] # normal tag where we find robosig configuration + tag_conf = self.instance_obj['tags'][tags_key] + + for sidetag_conf in self.instance_obj['sidetags'].values(): + if sidetag_conf['tags_key'] == tags_key: + tag_conf['sidetags'] = sidetag_conf + break + else: + raise RuntimeError("Can't find sidetag configuration for {}".format( + tag_conf['from'])) + + if error == 'untrusted-tagger': + tagger = 'hamburglar' + else: + tagger = 'bodhi' + + if error == 'missing-from-history': + tag_history = [] + else: + tag_history = [ + {'active': True, + 'create_ts': 1554076800, + 'creator_name': tagger, + 'tag_name': sidetag_pending}, + ] + self.koji_client.tagHistory.return_value = tag_history + + return tag_conf, tagger + + def _get_expected_messages_and_exceptions(self, sidetag_pending, error, + build_owner, tag_conf, tagger): + body = self.test_msg['body'] + instance = body['instance'] + build_nvr = '{name}-{version}-{release}'.format(**body) + from_tag = body['tag'] + build_id = body['build_id'] + + expected_log_msgs = [ + "Build {} ({}) tagged into {} on {}".format( + build_nvr, build_id, from_tag, instance), + ] + + if error == 'non-mbs-owner': + expected_log_msgs.append( + "Build {} has owner {}, which is NOT mbs_user!".format(build_id, build_owner)) + expected_exc_ctx = raises(Exception, + match="Modular content tag contains invalid owned build") + elif error == 'untrusted-tagger': + expected_log_msgs.append( + "Side tag build not tagged into {} by trusted user ({}) but by '{}'!".format( + sidetag_pending, ", ".join(tag_conf['sidetags']['trusted_taggers']), tagger + ) + ) + expected_exc_ctx = raises(Exception, + match="Side tag build tagged by untrusted user") + elif error == 'missing-from-history': + expected_log_msgs.append( + "Couldn't find tag {} in history of build {} ({})!".format( + sidetag_pending, build_nvr, build_id + ) + ) + expected_exc_ctx = raises(Exception, + match="Tag not found in build history") + else: + expected_exc_ctx = DummyContext() + + return expected_log_msgs, expected_exc_ctx + @mock.patch('robosignatory.tag.utils', new_callable=MockUtils()) - @mark.parametrize('type_,error', - ((None, None), ('plain', None), ('modular', None), - ('modular', 'non-mbs-owner'))) - def test_build_messages(self, utils, caplog, type_, error): + @mark.parametrize( + 'type_,sidetag_pending,error', + ((None, None, None), + ('plain', None, None), + ('plain', 'f31-build-side-1234-pending-signing', None), + ('plain', 'f31-build-side-1234-pending-signing', 'untrusted-tagger'), + ('plain', 'f31-build-side-1234-pending-signing', 'missing-from-history'), + ('modular', None, None), + ('modular', None, 'non-mbs-owner'))) + def test_build_messages(self, utils, caplog, type_, sidetag_pending, error): """Test build messages for various types.""" caplog.set_level(logging.DEBUG) body = self.test_msg['body'] - instance = body['instance'] build_nvr = '{name}-{version}-{release}'.format(**body) from_tag = body['tag'] build_id = body['build_id'] tag_conf = self.instance_obj['tags'][from_tag] to_tag = tag_conf['to'] + tagger = None + build_owner = None if type_ == 'modular': body['tag'] = from_tag = 'f30-modular-signing-pending' @@ -266,20 +348,17 @@ class TestTagSigner(object): {'owner_name': build_owner, 'nvr': build_nvr, 'build_id': build_id} ] - expected_log_msgs = [ - "Build {} ({}) tagged into {} on {}".format( - build_nvr, build_id, from_tag, instance), - ] + if sidetag_pending: + tag_conf, tagger = self._prep_sidetag_test(sidetag_pending, error) + body['tag'] = from_tag = sidetag_pending + to_tag = sidetag_pending.replace('-pending-signing', '-testing') + + expected_log_msgs, expected_exc_ctx = self._get_expected_messages_and_exceptions( + sidetag_pending, error, build_owner, tag_conf, tagger + ) - if error == 'non-mbs-owner': - expected_log_msgs.append( - "Build {} has owner {}, which is NOT mbs_user!".format(build_id, build_owner)) - exc_ctx_mgr = raises(Exception, - match="Modular content tag contains invalid owned build") - else: - exc_ctx_mgr = DummyContext() msg = Message(**self.test_msg) - with exc_ctx_mgr: + with expected_exc_ctx: self.tag_signer.consume(msg) for msg in expected_log_msgs: