Awesome. Looks sane to me! (Might be worth linking to https://github.com/coreos/fedora-coreos-tracker/issues/296 in the commit message?)

(Might be worth linking to https://github.com/coreos/fedora-coreos-tracker/issues/296 in the commit message?)

Done

Hmm, maybe we should sanity-check here that:

1. the third character is a . (or otherwise, split on dots and use the first field)
2. that the field is an integer; e.g. something like:

major = int(msg.body...)
return 'fedora-' + str(major)

Seems roundabout, but that way we get some basic enforcement on what we expect the version string to look like.

Hmm, maybe we should sanity-check here that:

the third character is a . (or otherwise, split on dots and use the first field)
that the field is an integer; e.g. something like:

major = int(msg.body...)
return 'fedora-' + str(major)

Seems roundabout, but that way we get some basic enforcement on what we expect the version string to look like.

sounds good. fixed

If the key becomes optional in the configuration then this may raise a KeyError. The .get("key") accessor could be used to avoid that.
It would also be nice to have a unit test for that situation.

Any reason for moving this into the try..except block?

I was mostly thinking of handling a situation where an exception could occur and then making sure a message got back to the requestor about the failure. I've moved it back up for now.

fixed.

could you maybe help me write a unit test in a followup?

Sure, sorry for the delay. Here's how you could write tests. Edit tests/test_coreos.py and add methods for tests you want to perform. It's a basic unittest.TestCase class so methods have to start with test_. Those methods could be modeled after this template:

    def test_key_parse(self):
        config = copy.deepcopy(TEST_CONFIG)
        del config["coreos"]["key"]  # Remove the hardcoded key to enable parsing
        consumer = CoreOSSigner(config)
        new_body = copy.deepcopy(ARTIFACTS_MESSAGE.body)
        new_body["build_id"] = "the-build-id-string-you-want-to-test"
        msg = Message(topic=ARTIFACTS_MESSAGE.topic, body=new_body)
        self.assertEqual(consumer.get_key(msg), "the-key-you-expect-for-that-build-id")

Does this help?

It would also be nice to have a test that makes sure the key is properly returned when hardcoded, as it was before.

now with 100% more tests!

tests/test_coreos.py::TestCoreOS::test_artifacts_sign PASSED           [ 23%]
tests/test_coreos.py::TestCoreOS::test_download_failed PASSED          [ 26%]
tests/test_coreos.py::TestCoreOS::test_key_parse_autodetect PASSED     [ 29%]
tests/test_coreos.py::TestCoreOS::test_key_parse_config PASSED         [ 32%]
tests/test_coreos.py::TestCoreOS::test_no_signature PASSED             [ 35%]
tests/test_coreos.py::TestCoreOS::test_ostree_sign PASSED              [ 38%]
tests/test_coreos.py::TestCoreOS::test_signing_failed PASSED           [ 41%]
tests/test_coreos.py::TestCoreOS::test_wrong_checksum PASSED

Looks great, thanks!

can we get a new release of robosignatory? See https://pagure.io/fedora-infrastructure/issue/8962 for context

I don't have the permissions to make releases on PyPI, maybe @puiterwijk can do it?

you now have permissions on PyPI!