From 6e4fdd59720af1f81e76f8bc9560627ae9190144 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Apr 25 2023 08:55:34 +0000 Subject: [PATCH 1/6] Drop support for Python 2, add support for Python 3.9 to 3.11 Signed-off-by: Aurélien Bompard --- diff --git a/tox.ini b/tox.ini index c40f48a..00cc8a6 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py27,py37,py38 +envlist = py37,py38,py39,py310,py311 # If the user is missing an interpreter, don't fail skip_missing_interpreters = True From bd1a778799b26b828556883fc470c080b553f2d3 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Apr 25 2023 09:01:36 +0000 Subject: [PATCH 2/6] Run 2to3 Signed-off-by: Aurélien Bompard --- diff --git a/robosignatory/atomic.py b/robosignatory/atomic.py index a52c98c..507e177 100644 --- a/robosignatory/atomic.py +++ b/robosignatory/atomic.py @@ -1,5 +1,3 @@ -from __future__ import unicode_literals, absolute_import - import robosignatory.utils as utils import robosignatory.work diff --git a/robosignatory/cli.py b/robosignatory/cli.py index 0c50922..6c5031e 100644 --- a/robosignatory/cli.py +++ b/robosignatory/cli.py @@ -1,5 +1,3 @@ -from __future__ import unicode_literals, absolute_import - import os import click diff --git a/robosignatory/consumer.py b/robosignatory/consumer.py index e8ff2fd..89fe372 100644 --- a/robosignatory/consumer.py +++ b/robosignatory/consumer.py @@ -1,5 +1,3 @@ -from __future__ import unicode_literals, absolute_import - import logging import fedora_messaging diff --git a/robosignatory/coreos.py b/robosignatory/coreos.py index 6e903b0..3a5f1ae 100644 --- a/robosignatory/coreos.py +++ b/robosignatory/coreos.py @@ -1,5 +1,3 @@ -from __future__ import unicode_literals, absolute_import - import os import stat import logging diff --git a/robosignatory/tag.py b/robosignatory/tag.py index 7d91745..83acbb2 100644 --- a/robosignatory/tag.py +++ b/robosignatory/tag.py @@ -1,5 +1,3 @@ -from __future__ import unicode_literals, absolute_import - import logging import re diff --git a/robosignatory/utils.py b/robosignatory/utils.py index 5326f58..8a70596 100644 --- a/robosignatory/utils.py +++ b/robosignatory/utils.py @@ -71,9 +71,7 @@ def get_signing_helper(backend, *args, **kwargs): return cls(*args, **kwargs) -class BaseSigningHelper(object): - __metaclass__ = abc.ABCMeta - +class BaseSigningHelper(object, metaclass=abc.ABCMeta): @abc.abstractmethod def build_cmdline(self, *args): pass diff --git a/robosignatory/xml.py b/robosignatory/xml.py index 31b5cf6..e546861 100644 --- a/robosignatory/xml.py +++ b/robosignatory/xml.py @@ -1,5 +1,3 @@ -from __future__ import unicode_literals, absolute_import - import os import stat import logging diff --git a/tests/test_tag.py b/tests/test_tag.py index 995d2e9..12abdae 100644 --- a/tests/test_tag.py +++ b/tests/test_tag.py @@ -1,5 +1,3 @@ -from __future__ import unicode_literals - import copy import logging import mock @@ -284,7 +282,7 @@ class TestTagSigner(object): 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(): + for sidetag_conf in list(self.instance_obj['sidetags'].values()): if sidetag_conf['tags_key'] == tags_key: tag_conf['sidetags'] = sidetag_conf break @@ -493,14 +491,14 @@ class TestTagSigner(object): @mark.skipif(not six.PY2, reason="only relevant on Python 2") def test_py2_koji_client_args(self): assert self._koji_ClientSession.call_count == 1 - assert not isinstance(self._koji_ClientSession.call_args_list[0][0][0], unicode) + assert not isinstance(self._koji_ClientSession.call_args_list[0][0][0], str) for kwarg, value in self._koji_ClientSession.call_args_list[0][1].items(): - assert not isinstance(value, unicode) + assert not isinstance(value, str) @mark.skipif(not six.PY2, reason="only relevant on Python 2") def test_py2_gssapi_login_args(self): assert self.instance_obj["client"].gssapi_login.call_count == 1 assert not isinstance( self.instance_obj["client"].gssapi_login.call_args_list[0][1]["principal"], - unicode + str ) From 13801152ad78621dfe9079a091bbdb8b5b53d14a Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Apr 25 2023 09:40:57 +0000 Subject: [PATCH 3/6] Do some more linting Signed-off-by: Aurélien Bompard --- diff --git a/robosignatory/atomic.py b/robosignatory/atomic.py index 507e177..386ec09 100644 --- a/robosignatory/atomic.py +++ b/robosignatory/atomic.py @@ -5,7 +5,7 @@ import logging log = logging.getLogger("robosignatory.atomicconsumer") -class AtomicSigner(object): +class AtomicSigner: def __init__(self, config): self.config = config diff --git a/robosignatory/consumer.py b/robosignatory/consumer.py index 89fe372..ff58a3d 100644 --- a/robosignatory/consumer.py +++ b/robosignatory/consumer.py @@ -11,7 +11,7 @@ from .xml import XMLSigner log = logging.getLogger('robosignatory') -class Consumer(object): +class Consumer: """All messages are received by this class's __call__() method.""" def __init__(self): @@ -55,4 +55,4 @@ class Consumer(object): except Exception as e: error_msg = '{e}: Unable to handle message: {msg}'.format(e=e, msg=msg) log.exception(error_msg) - raise fedora_messaging.exceptions.Nack(error_msg) + raise fedora_messaging.exceptions.Nack(error_msg) from e diff --git a/robosignatory/coreos.py b/robosignatory/coreos.py index 3a5f1ae..2bd160b 100644 --- a/robosignatory/coreos.py +++ b/robosignatory/coreos.py @@ -3,18 +3,18 @@ import stat import logging import shutil import tempfile +from urllib.parse import urlparse import boto3 import robosignatory.utils as utils from botocore.exceptions import ClientError -from six.moves.urllib.parse import urlparse from fedora_messaging.api import Message, publish log = logging.getLogger(__name__) -class CoreOSSigner(object): +class CoreOSSigner: def __init__(self, config): self.config = config @@ -87,7 +87,7 @@ class SigningFailed(Exception): pass -class SignerWrapper(object): +class SignerWrapper: """ This class handles the common operations that come with signing a file in S3. """ @@ -124,7 +124,7 @@ class SignerWrapper(object): try: self.bucket.download_file(filepath, local_filepath) except ClientError as e: - raise SigningFailed("Could not download {}: {}".format(filepath, e)) + raise SigningFailed("Could not download {}: {}".format(filepath, e)) from e log.info("Checking hash for %s", filepath) if utils.get_hash(local_filepath) != checksum: diff --git a/robosignatory/tag.py b/robosignatory/tag.py index 83acbb2..8a221fb 100644 --- a/robosignatory/tag.py +++ b/robosignatory/tag.py @@ -3,8 +3,6 @@ import re import koji -import six - import robosignatory.utils as utils @@ -13,7 +11,7 @@ log = logging.getLogger("robosignatory.tagconsumer") KNOWN_TAG_TYPES = ['plain', 'modular'] -class TagSigner(object): +class TagSigner: def __init__(self, config): self.config = config @@ -24,8 +22,6 @@ class TagSigner(object): self.koji_clients = {} for instance, instance_info in self.config['koji_instances'].items(): url = instance_info['url'] - if six.PY2: - url = url.encode("utf-8") client = koji.ClientSession(url, instance_info['options']) if instance_info['options']['authmethod'] == 'ssl': @@ -37,8 +33,6 @@ class TagSigner(object): for opt in ('principal', 'keytab', 'ccache'): if opt in instance_info['options']: value = instance_info['options'][opt] - if six.PY2: - value = value.encode("utf-8") kwargs[opt] = value client.gssapi_login(**kwargs) else: @@ -98,7 +92,7 @@ class TagSigner(object): trusted_taggers = sidetags['trusted_taggers'] if ( not isinstance(trusted_taggers, list) - or not all(isinstance(x, six.text_type) for x in trusted_taggers) + or not all(isinstance(x, str) for x in trusted_taggers) ): raise TypeError("`trusted_taggers` must be a list of strings, not %r." % trusted_taggers) @@ -145,7 +139,7 @@ class TagSigner(object): 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(): + for sidetag_matched in instance['sidetags'].values(): from_tag_re = sidetag_matched['from_tag_re'] m = from_tag_re.match(tag) @@ -216,8 +210,10 @@ class TagSigner(object): 'Going to sign %s with %s (%s) and file signing key %s and move to %s', build_nvr, tag_info['key'], tag_info['keyid'], tag_info['file_signing_key'], tag_to) else: - log.info('Going to sign %s with %s (%s) and move to %s', - build_nvr, tag_info['key'], tag_info['keyid'], tag_to) + log.info( + 'Going to sign %s with %s (%s) and move to %s', + 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) diff --git a/robosignatory/utils.py b/robosignatory/utils.py index 8a70596..e30f606 100644 --- a/robosignatory/utils.py +++ b/robosignatory/utils.py @@ -71,7 +71,7 @@ def get_signing_helper(backend, *args, **kwargs): return cls(*args, **kwargs) -class BaseSigningHelper(object, metaclass=abc.ABCMeta): +class BaseSigningHelper(metaclass=abc.ABCMeta): @abc.abstractmethod def build_cmdline(self, *args): pass diff --git a/robosignatory/work.py b/robosignatory/work.py index b6a7f83..61fddad 100644 --- a/robosignatory/work.py +++ b/robosignatory/work.py @@ -43,7 +43,7 @@ def process_atomic(signer, ref, commitid, key, directory, doref=True): refpath = os.path.join(directory, 'refs', 'heads', ref) if os.path.exists(refpath): - with open(refpath, 'r') as f: + with open(refpath) as f: log.info('Previous commit for %s: %s' % (ref, f.read().replace('\n', ''))) diff --git a/robosignatory/xml.py b/robosignatory/xml.py index e546861..7d56ffe 100644 --- a/robosignatory/xml.py +++ b/robosignatory/xml.py @@ -1,5 +1,4 @@ import os -import stat import logging import shutil import tempfile @@ -10,7 +9,8 @@ from fedora_messaging.api import Message, publish log = logging.getLogger(__name__) -class XMLSigner(object): + +class XMLSigner: __slots__ = ('_signer', '_tmpdir', '_key') @@ -36,8 +36,10 @@ class XMLSigner(object): cmdline = self._signer.build_xml_cmdline(self._key, input_file, output_file) ret, stdout, stderr = utils.run_command(cmdline) if ret != 0: - raise Exception('Error signing! Signing output: %s, stdout: ' - '%r, stderr: %r' % (ret, stdout, stderr)) + raise Exception( + 'Error signing! Signing output: %s, stdout: ' + '%r, stderr: %r' % (ret, stdout, stderr) + ) with open(output_file, 'rb') as f: signature = f.read().decode('ascii', 'strict') log.info('XML file was successfully signed') @@ -50,7 +52,6 @@ class XMLSigner(object): body={'body': msg.body, 'error': str(error)} )) - def consume(self, msg): log.info('Punji and/or bodhi wants to sign an XML file') @@ -82,4 +83,3 @@ class XMLSigner(object): # respond with the same body body={'body': msg.body, 'signature': signature} )) - diff --git a/setup.py b/setup.py index eaf2297..671895b 100644 --- a/setup.py +++ b/setup.py @@ -40,7 +40,6 @@ setup( "fedora_messaging", "psutil", "boto3", - "six", "click", "setuptools", # Don't depend on koji here: https://bugzilla.redhat.com/show_bug.cgi?id=1537197 diff --git a/tests/test_atomic.py b/tests/test_atomic.py index d9250df..cab1141 100644 --- a/tests/test_atomic.py +++ b/tests/test_atomic.py @@ -1,7 +1,7 @@ -import unittest import copy +import unittest +from unittest import mock -import mock from fedora_messaging.api import Message from robosignatory.atomic import AtomicSigner @@ -71,4 +71,3 @@ class TestAtomic(unittest.TestCase): msg.body["commitid"] = None self.consumer.consume(msg) process_atomic.assert_not_called() - diff --git a/tests/test_consumers.py b/tests/test_consumers.py index e4366a0..a1b87b6 100644 --- a/tests/test_consumers.py +++ b/tests/test_consumers.py @@ -1,8 +1,8 @@ import unittest +from unittest import mock from fedora_messaging.api import Message from fedora_messaging.exceptions import Nack -import mock from robosignatory.consumer import Consumer diff --git a/tests/test_coreos.py b/tests/test_coreos.py index edac1f3..1e127b3 100644 --- a/tests/test_coreos.py +++ b/tests/test_coreos.py @@ -2,8 +2,8 @@ import os import unittest import copy from collections import namedtuple +from unittest import mock -import mock from botocore.exceptions import ClientError from fedora_messaging.api import Message from fedora_messaging.testing import mock_sends @@ -52,14 +52,17 @@ OSTREE_MESSAGE = Message( S3Object = namedtuple("S3Object", "size") + def fake_download(source, local): open(local, "w").close() + def fake_download_and_artifact_sign(source, local): fake_download(source, local) # Also create the sig file for testing open(local + ".sig", "w").close() + def fake_download_and_ostree_sign(source, local): fake_download(source, local) # Also create the sig file for testing @@ -112,7 +115,10 @@ class TestCoreOS(unittest.TestCase): assert self.consumer.bucket.download_file.call_args_list[0][0][0] == "some/path/test1" run_command.assert_called() self.consumer.bucket.upload_file.assert_called() - assert self.consumer.bucket.upload_file.call_args_list[0][0][1] == "some/path/ostree-commitmeta-object" + assert ( + self.consumer.bucket.upload_file.call_args_list[0][0][1] + == "some/path/ostree-commitmeta-object" + ) @mock.patch('robosignatory.coreos.utils.run_command') def test_wrong_checksum(self, run_command): @@ -120,8 +126,10 @@ class TestCoreOS(unittest.TestCase): new_body["artifacts"][0]["checksum"] = "sha256:wrong-checksum" msg = Message(topic=ARTIFACTS_MESSAGE.topic, body=new_body) self.consumer.bucket.download_file.side_effect = fake_download - expected_response = self._get_response_message(msg, failed=True, - failure_msg='Incorrect SHA256 for some/path/test1, not signing') + expected_response = self._get_response_message( + msg, failed=True, + failure_msg='Incorrect SHA256 for some/path/test1, not signing' + ) with mock_sends(expected_response): self.consumer.consume(msg) @@ -182,7 +190,7 @@ class TestCoreOS(unittest.TestCase): def test_key_parse_autodetect(self): # Verify that when no key is provided via the config the key # is autodetected. - # + # # Grab the config and remove the hardcoded key to enable auto detection config = copy.deepcopy(TEST_CONFIG) del config["coreos"]["key"] diff --git a/tests/test_tag.py b/tests/test_tag.py index 12abdae..bd02166 100644 --- a/tests/test_tag.py +++ b/tests/test_tag.py @@ -1,8 +1,7 @@ import copy import logging -import mock +from unittest import mock -import six from fedora_messaging.api import Message from pkg_resources import parse_version from pytest import raises, mark, __version__ as pytest_version @@ -14,7 +13,7 @@ try: except ImportError: pass else: - if not hasattr(_pytest.logging.LogCaptureFixture, 'messages'): # noqa + if not hasattr(_pytest.logging.LogCaptureFixture, 'messages'): # monkey-patch missing messages property class MyLogCaptureFixture(_pytest.logging.LogCaptureFixture): @property @@ -163,7 +162,7 @@ class MockUtils(mock.MagicMock): return 0, "", "" -class DummyContext(object): +class DummyContext: def __enter__(self): pass @@ -176,7 +175,7 @@ class DummyContext(object): 'robosignatory.consumer.fedora_messaging.config.conf', {'consumer_config': TEST_CONFIG} ) -class TestTagSigner(object): +class TestTagSigner: """Test the Koji tag signer class""" test_msg = { @@ -298,8 +297,8 @@ class TestTagSigner(object): if error == 'missing-from-history': tag_history = {'tag_listing': []} else: - tag_history = {'tag_listing': - [ + tag_history = { + 'tag_listing': [ {'active': True, 'create_ts': 1554076800, 'creator_name': tagger, @@ -427,7 +426,6 @@ class TestTagSigner(object): 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': @@ -440,13 +438,19 @@ class TestTagSigner(object): ] expected_log_msgs = [ 'Packages correctly signed, moving to f30-modular-updates-testing-pending', - 'Signing command line: [\'echo\', "build_sign_cmdline: (\'fedora-30\', [\'foo-1-1.fc31.x64_64\', \'foo-libs-1-1.fc31.x64_64\'], \'test\', \'file-sign-key\') {}"]', - 'Going to sign foo-1-1.fc31 with fedora-30 (OU812I81B4U) and file signing key file-sign-key and move to f30-modular-updates-testing-pending', + 'Signing command line: [\'echo\', "build_sign_cmdline: (\'fedora-30\', ' + '[\'foo-1-1.fc31.x64_64\', \'foo-libs-1-1.fc31.x64_64\'], \'test\', ' + '\'file-sign-key\') {}"]', + 'Going to sign foo-1-1.fc31 with fedora-30 (OU812I81B4U) and file signing ' + 'key file-sign-key and move to f30-modular-updates-testing-pending', ] else: expected_log_msgs = [ - 'Signing command line: [\'echo\', "build_sign_cmdline: (\'fedora-31\', [\'foo-1-1.fc31.x64_64\', \'foo-libs-1-1.fc31.x64_64\'], \'test\', \'file-sign-key\') {}"]', - 'Going to sign foo-1-1.fc31 with fedora-31 (deadbeef) and file signing key file-sign-key and move to f31', + 'Signing command line: [\'echo\', "build_sign_cmdline: (\'fedora-31\', ' + '[\'foo-1-1.fc31.x64_64\', \'foo-libs-1-1.fc31.x64_64\'], \'test\', ' + '\'file-sign-key\') {}"]', + 'Going to sign foo-1-1.fc31 with fedora-31 (deadbeef) and file signing key ' + 'file-sign-key and move to f31', ] msg = Message(**self.test_msg) @@ -487,18 +491,3 @@ class TestTagSigner(object): self.tag_signer.consume(msg) log.info.assert_called_with('Koji instance not known, skipping') - - @mark.skipif(not six.PY2, reason="only relevant on Python 2") - def test_py2_koji_client_args(self): - assert self._koji_ClientSession.call_count == 1 - assert not isinstance(self._koji_ClientSession.call_args_list[0][0][0], str) - for kwarg, value in self._koji_ClientSession.call_args_list[0][1].items(): - assert not isinstance(value, str) - - @mark.skipif(not six.PY2, reason="only relevant on Python 2") - def test_py2_gssapi_login_args(self): - assert self.instance_obj["client"].gssapi_login.call_count == 1 - assert not isinstance( - self.instance_obj["client"].gssapi_login.call_args_list[0][1]["principal"], - str - ) diff --git a/tests/test_xml.py b/tests/test_xml.py index 5cfaccd..c1bdd15 100644 --- a/tests/test_xml.py +++ b/tests/test_xml.py @@ -1,9 +1,6 @@ -import os import unittest -import copy -from collections import namedtuple +from unittest import mock -import mock from fedora_messaging.api import Message from fedora_messaging.testing import mock_sends @@ -29,13 +26,14 @@ INVALID_MESSAGE = Message( topic="org.fedoraproject.prod.robosignatory.xml-sign", body='\0') + class TestXML(unittest.TestCase): def setUp(self): self.consumer = XMLSigner(TEST_CONFIG) def _get_response_message(self, source_msg, failed=False, failure_msg="Signing failed", sig=""): - body= { + body = { 'body': source_msg.body, 'error': failure_msg, } if failed else { @@ -58,9 +56,11 @@ class TestXML(unittest.TestCase): @mock.patch('robosignatory.xml.utils.run_command') def test_wrong_xml(self, run_command): run_command.return_value = 0, "", "" - expected_response = self._get_response_message(INVALID_MESSAGE, - failed=True, failure_msg='Refusing to sign object that does not ' - 'start with XML declaration') + expected_response = self._get_response_message( + INVALID_MESSAGE, + failed=True, + failure_msg='Refusing to sign object that does not start with XML declaration' + ) with mock_sends(expected_response): self.consumer.consume(INVALID_MESSAGE) run_command.assert_not_called() From 6b726c42722c3d6c3387c81a1210ee54388f1b0b Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Apr 25 2023 09:40:57 +0000 Subject: [PATCH 4/6] Upgrade string formatting Signed-off-by: Aurélien Bompard --- diff --git a/robosignatory/atomic.py b/robosignatory/atomic.py index 386ec09..7789951 100644 --- a/robosignatory/atomic.py +++ b/robosignatory/atomic.py @@ -34,8 +34,8 @@ class AtomicSigner: if commitid is None: return - log.info('pungi composed %(ref)s (%(commitid)s, variant %(variant)s, ' - 'arch %(arch)s)' % msg.body) + log.info('pungi composed {ref} ({commitid}, variant {variant}, ' + 'arch {arch})'.format(**msg.body)) if ref not in self.refs: log.info('Unknown reference %s. Skipping' % ref) diff --git a/robosignatory/cli.py b/robosignatory/cli.py index 6c5031e..289a40a 100644 --- a/robosignatory/cli.py +++ b/robosignatory/cli.py @@ -20,7 +20,7 @@ from robosignatory import utils ) def cli(config): if not os.path.isfile(config): - raise click.exceptions.BadParameter("{} is not a file".format(config)) + raise click.exceptions.BadParameter(f"{config} is not a file") conf.load_config(config_path=config) conf.setup_logging() diff --git a/robosignatory/consumer.py b/robosignatory/consumer.py index ff58a3d..f5ee883 100644 --- a/robosignatory/consumer.py +++ b/robosignatory/consumer.py @@ -53,6 +53,6 @@ class Consumer: log.debug('Passing message to the Text handler') self.xml_handler.consume(msg) except Exception as e: - error_msg = '{e}: Unable to handle message: {msg}'.format(e=e, msg=msg) + error_msg = f'{e}: Unable to handle message: {msg}' log.exception(error_msg) raise fedora_messaging.exceptions.Nack(error_msg) from e diff --git a/robosignatory/coreos.py b/robosignatory/coreos.py index 2bd160b..8b3eb47 100644 --- a/robosignatory/coreos.py +++ b/robosignatory/coreos.py @@ -54,12 +54,12 @@ class CoreOSSigner: # https://github.com/coreos/fedora-coreos-tracker/issues/198#issuecomment-513944390 log.info( 'CoreOS wants to sign ' - '%(build_id)s for %(basearch)s' % msg.body + '{build_id} for {basearch}'.format(**msg.body) ) key = self.get_key(msg) response = Message( - topic="{}.finished".format(msg.topic), + topic=f"{msg.topic}.finished", # respond with the same body, but clone so we keep the original one body=dict(msg.body) ) @@ -105,11 +105,11 @@ class SignerWrapper: def sign(self, url, checksum): if ':' not in checksum: - raise SigningFailed("Missing algo prefix in {}".format(checksum)) + raise SigningFailed(f"Missing algo prefix in {checksum}") algo, checksum = checksum.split(':', 1) if algo != "sha256": # for now, we only handle sha256 - raise SigningFailed("Unknown checksum algo {}".format(algo)) + raise SigningFailed(f"Unknown checksum algo {algo}") tmpdir = tempfile.mkdtemp(prefix="/tmp/robosignatory-") try: self._sign_object(url, checksum, tmpdir) @@ -124,11 +124,11 @@ class SignerWrapper: try: self.bucket.download_file(filepath, local_filepath) except ClientError as e: - raise SigningFailed("Could not download {}: {}".format(filepath, e)) from e + raise SigningFailed(f"Could not download {filepath}: {e}") from e log.info("Checking hash for %s", filepath) if utils.get_hash(local_filepath) != checksum: - raise SigningFailed("Incorrect SHA256 for {}, not signing".format(filepath)) + raise SigningFailed(f"Incorrect SHA256 for {filepath}, not signing") log.info("Signing %s", filepath) sig_filepath = self._get_sig_filepath(local_filepath) @@ -142,7 +142,7 @@ class SignerWrapper: ) if not os.path.exists(sig_filepath): raise SigningFailed( - "Signer did not produce any signature file for {}".format(filepath) + f"Signer did not produce any signature file for {filepath}" ) log.debug('Fixing signature file permissions') # Sigul writes it as 0600, which makes a lot of sense as a general file diff --git a/robosignatory/tag.py b/robosignatory/tag.py index 8a221fb..dcddc1c 100644 --- a/robosignatory/tag.py +++ b/robosignatory/tag.py @@ -121,7 +121,7 @@ class TagSigner: # u'owner': u'sayanchowdhury', # u'release': u'1.el7'}} - build_nvr = '%(name)s-%(version)s-%(release)s' % msg.body + build_nvr = '{name}-{version}-{release}'.format(**msg.body) build_id = msg.body['build_id'] tag = msg.body['tag'] koji_instance = msg.body['instance'] @@ -241,8 +241,7 @@ class TagSigner: 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} has owner {owner_name}, which is NOT mbs_user!'.format(**build)) raise Exception('Modular content tag contains invalid owned build') self.signwrite_single_build(build['nvr'], build['build_id'], tag_info, instance, koji_instance) @@ -254,7 +253,7 @@ class TagSigner: 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']) + ['{} ({}, signed: {})'.format(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 e30f606..da41476 100644 --- a/robosignatory/utils.py +++ b/robosignatory/utils.py @@ -24,7 +24,7 @@ def get_rpms(koji_client, build_nvr, build_id, sigkey=None): sigs = koji_client.queryRPMSigs(rpm_id=rpm['id'], sigkey=sigkey) info['signed'] = len(sigs) != 0 - rpminfo['%s.%s' % (rpm['nvr'], rpm['arch'])] = info + rpminfo['{}.{}'.format(rpm['nvr'], rpm['arch'])] = info return rpminfo @@ -67,7 +67,7 @@ def get_signing_helper(backend, *args, **kwargs): classes = dict([(point.name, point.load()) for point in points]) log.debug("Found the following installed signing helpers %r" % classes) cls = classes[backend] - log.debug("Instantiating helper %r from backend key %r" % (cls, backend)) + log.debug(f"Instantiating helper {cls!r} from backend key {backend!r}") return cls(*args, **kwargs) @@ -96,7 +96,7 @@ class BaseSigningHelper(metaclass=abc.ABCMeta): class EchoHelper(BaseSigningHelper): """ A dummy "hello world" helper, used for debugging. """ def __init__(self, *args, **kwargs): - log.info("Constructing EchoHelper(%r, %r)" % (args, kwargs)) + log.info(f"Constructing EchoHelper({args!r}, {kwargs!r})") def build_cmdline(self, *args, **kwargs): result = ['echo', ' '.join(['build_cmdline:', str(args), str(kwargs)])] diff --git a/robosignatory/work.py b/robosignatory/work.py index 61fddad..79f2cb1 100644 --- a/robosignatory/work.py +++ b/robosignatory/work.py @@ -19,7 +19,7 @@ def process_atomic(signer, ref, commitid, key, directory, doref=True): log.info('Commitmeta file at %s found. Skipping' % commitmetapath) return - log.info('All checks passed, signing %s with %s' % (commitid, key)) + log.info(f'All checks passed, signing {commitid} with {key}') cmdline = signer.build_atomic_cmdline(key, commitid, commitpath, @@ -44,8 +44,7 @@ def process_atomic(signer, ref, commitid, key, directory, doref=True): refpath = os.path.join(directory, 'refs', 'heads', ref) if os.path.exists(refpath): with open(refpath) as f: - log.info('Previous commit for %s: %s' - % (ref, f.read().replace('\n', ''))) + log.info('Previous commit for {}: {}'.format(ref, f.read().replace('\n', ''))) dirname_refpath = os.path.dirname(refpath) if doref and not os.path.exists(dirname_refpath): @@ -59,7 +58,7 @@ def process_atomic(signer, ref, commitid, key, directory, doref=True): os.umask(oldumask) if doref: - log.info('Writing %s to %s' % (commitid, refpath)) + log.info(f'Writing {commitid} to {refpath}') with open(refpath, 'w') as f: f.write(commitid + '\n') diff --git a/robosignatory/xml.py b/robosignatory/xml.py index 7d56ffe..2046526 100644 --- a/robosignatory/xml.py +++ b/robosignatory/xml.py @@ -23,7 +23,7 @@ class XMLSigner: elif k == 'key' and type(v) is str: self._key = v else: - raise Exception('Bad config entry {!r}'.format((k, v))) + raise Exception(f'Bad config entry {(k, v)!r}') if not hasattr(self, '_key'): raise Exception('Key must be specified') log.info('XMLSigner ready for service') @@ -37,8 +37,8 @@ class XMLSigner: ret, stdout, stderr = utils.run_command(cmdline) if ret != 0: raise Exception( - 'Error signing! Signing output: %s, stdout: ' - '%r, stderr: %r' % (ret, stdout, stderr) + 'Error signing! Signing output: {}, stdout: ' + '{!r}, stderr: {!r}'.format(ret, stdout, stderr) ) with open(output_file, 'rb') as f: signature = f.read().decode('ascii', 'strict') @@ -47,7 +47,7 @@ class XMLSigner: def _error(self, msg, error): publish(Message( - topic="{}.finished".format(msg.topic), + topic=f"{msg.topic}.finished", # respond with the same body body={'body': msg.body, 'error': str(error)} )) @@ -79,7 +79,7 @@ class XMLSigner: finally: shutil.rmtree(tmpdir) publish(Message( - topic="{}.finished".format(msg.topic), + topic=f"{msg.topic}.finished", # respond with the same body body={'body': msg.body, 'signature': signature} )) diff --git a/tests/test_tag.py b/tests/test_tag.py index bd02166..2e10c5f 100644 --- a/tests/test_tag.py +++ b/tests/test_tag.py @@ -324,7 +324,7 @@ class TestTagSigner: if error == 'non-mbs-owner': expected_log_msgs.append( - "Build {} has owner {}, which is NOT mbs_user!".format(build_id, build_owner)) + f"Build {build_id} has owner {build_owner}, which is NOT mbs_user!") expected_exc_ctx = raises(Exception, match="Modular content tag contains invalid owned build") elif error == 'untrusted-tagger': From 9a790658078726a1d33ab56bc1608efc8d19089f Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Apr 25 2023 09:40:57 +0000 Subject: [PATCH 5/6] Some more linting Signed-off-by: Aurélien Bompard --- diff --git a/robosignatory/coreos.py b/robosignatory/coreos.py index 8b3eb47..6dca955 100644 --- a/robosignatory/coreos.py +++ b/robosignatory/coreos.py @@ -110,7 +110,7 @@ class SignerWrapper: if algo != "sha256": # for now, we only handle sha256 raise SigningFailed(f"Unknown checksum algo {algo}") - tmpdir = tempfile.mkdtemp(prefix="/tmp/robosignatory-") + tmpdir = tempfile.mkdtemp(prefix="/tmp/robosignatory-") # noqa: S108 try: self._sign_object(url, checksum, tmpdir) finally: diff --git a/robosignatory/tag.py b/robosignatory/tag.py index dcddc1c..dbb3f21 100644 --- a/robosignatory/tag.py +++ b/robosignatory/tag.py @@ -241,7 +241,11 @@ class TagSigner: 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} has owner {owner_name}, which is NOT mbs_user!'.format(**build)) + log.error( + 'Build {build_id} has owner {owner_name}, which is NOT mbs_user!'.format( + **build + ) + ) raise Exception('Modular content tag contains invalid owned build') self.signwrite_single_build(build['nvr'], build['build_id'], tag_info, instance, koji_instance) diff --git a/robosignatory/utils.py b/robosignatory/utils.py index da41476..2632b77 100644 --- a/robosignatory/utils.py +++ b/robosignatory/utils.py @@ -42,9 +42,12 @@ def get_builds_in_tag(koji_client, tag): def run_command(command): - child = subprocess.Popen(command, stdin=subprocess.PIPE, - stdout=subprocess.PIPE, - stderr=subprocess.PIPE) + child = subprocess.Popen( + command, # noqa: S603 + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + ) stdout, stderr = child.communicate() ret = child.wait() return ret, stdout, stderr diff --git a/robosignatory/xml.py b/robosignatory/xml.py index 2046526..df6d366 100644 --- a/robosignatory/xml.py +++ b/robosignatory/xml.py @@ -16,7 +16,9 @@ class XMLSigner: def __init__(self, config): self._signer = utils.get_signing_helper(**config['signing']) - self._tmpdir = '/tmp' + # Disabling S108 below is OK because self._tmpdir is a top directory + # where the actual temporary directory will be created with mkdtemp. + self._tmpdir = '/tmp' # noqa: S108 for k, v in config['xml'].items(): if k == 'tmpdir' and type(v) is str: self._tmpdir = v From 788c95406547a146b7cf251c524f76846c83bf55 Mon Sep 17 00:00:00 2001 From: Aurélien Bompard Date: Apr 25 2023 09:40:57 +0000 Subject: [PATCH 6/6] Add linting with Ruff Signed-off-by: Aurélien Bompard --- diff --git a/pyproject.toml b/pyproject.toml new file mode 100644 index 0000000..371f726 --- /dev/null +++ b/pyproject.toml @@ -0,0 +1,8 @@ +[tool.ruff] +select = ["E", "F", "W", "UP", "S", "B", "RUF"] +line-length = 100 +target-version = "py38" +allowed-confusables = ["’"] + +[tool.ruff.per-file-ignores] +"tests/*" = ["S101", "S105", "S106", "S108"] diff --git a/tox.ini b/tox.ini index 00cc8a6..03987da 100644 --- a/tox.ini +++ b/tox.ini @@ -1,5 +1,5 @@ [tox] -envlist = py37,py38,py39,py310,py311 +envlist = py37,py38,py39,py310,py311,lint # If the user is missing an interpreter, don't fail skip_missing_interpreters = True @@ -12,3 +12,8 @@ deps = koji commands = python -m pytest -v {posargs} + +[testenv:lint] +deps = ruff +commands = + ruff {posargs:robosignatory tests}