From 1889602388ff67fa724b3d5fffb29b5a1587b029 Mon Sep 17 00:00:00 2001 From: Daniel Arndt Date: Mon, 16 Dec 2024 16:31:42 -0400 Subject: [PATCH] build: Enable pyright to check if type hints are missing (#279) --- pyproject.toml | 1 + src/charm.py | 6 +- tests/integration/test_integration.py | 16 ++-- tests/unit/test_charm_collect_status.py | 10 +-- tests/unit/test_charm_configure.py | 90 +++++++++---------- .../test_charm_get_issued_certificates.py | 6 +- tests/unit/test_charm_rotate_private_key.py | 6 +- 7 files changed, 68 insertions(+), 67 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index ed5d5d9..c2383e5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -79,3 +79,4 @@ skip = "build,lib,venv,icon.svg,.tox,.git,.mypy_cache,.ruff_cache,.coverage" [tool.pyright] include = ["src/**.py"] +reportMissingParameterType = true diff --git a/src/charm.py b/src/charm.py index 2150d83..339df55 100755 --- a/src/charm.py +++ b/src/charm.py @@ -7,7 +7,7 @@ import logging import typing from datetime import datetime, timedelta -from typing import Iterator, Optional, cast +from typing import Any, Iterator, Optional, cast from charms.certificate_transfer_interface.v0.certificate_transfer import ( CertificateTransferProvides, @@ -48,7 +48,7 @@ class SelfSignedCertificatesCharm(CharmBase): """Main class to handle Juju events.""" - def __init__(self, *args): + def __init__(self, *args: Any): """Observe config change and certificate request events.""" super().__init__(*args) self.tls_certificates = TLSCertificatesProvidesV4(self, "certificates") @@ -515,7 +515,7 @@ def _on_get_ca_certificate(self, event: ActionEvent): ca_certificate_secret_content = ca_certificate_secret.get_content(refresh=True) event.set_results({"ca-certificate": ca_certificate_secret_content["ca-certificate"]}) - def _send_ca_cert(self, *, rel_id=None): + def _send_ca_cert(self, *, rel_id: Optional[int] = None): """There is one (and only one) CA cert that we need to forward to multiple apps. Args: diff --git a/tests/integration/test_integration.py b/tests/integration/test_integration.py index 88dccbd..8654467 100644 --- a/tests/integration/test_integration.py +++ b/tests/integration/test_integration.py @@ -60,10 +60,10 @@ async def wait_for_requirer_certificates(ops_test: OpsTest, ca_common_name: str) @pytest.fixture(scope="module") @pytest.mark.abort_on_fail -async def deploy(ops_test: OpsTest, request): +async def deploy(ops_test: OpsTest, request: pytest.FixtureRequest) -> None: """Build the charm-under-test and deploy it.""" assert ops_test.model - charm = Path(request.config.getoption("--charm_path")).resolve() + charm = Path(str(request.config.getoption("--charm_path"))).resolve() logger.info("Deploying charms for architecture: %s", ARCH) await ops_test.model.set_constraints({"arch": ARCH}) await ops_test.model.deploy( @@ -96,7 +96,7 @@ async def deploy(ops_test: OpsTest, request): @pytest.mark.abort_on_fail async def test_given_charm_is_built_when_deployed_then_status_is_active( ops_test: OpsTest, - deploy, + deploy: None, ): assert ops_test.model await ops_test.model.wait_for_idle( @@ -108,7 +108,7 @@ async def test_given_charm_is_built_when_deployed_then_status_is_active( async def test_given_tls_requirer_is_deployed_when_integrated_then_certificate_is_provided( ops_test: OpsTest, - deploy, + deploy: None, ): assert ops_test.model await ops_test.model.integrate( @@ -124,7 +124,7 @@ async def test_given_tls_requirer_is_deployed_when_integrated_then_certificate_i async def test_given_tls_requirer_is_integrated_when_ca_common_name_config_changed_then_new_certificate_is_provided( # noqa: E501 ops_test: OpsTest, - deploy, + deploy: None, ): new_common_name = "newexample.org" assert ops_test.model @@ -142,7 +142,7 @@ async def test_given_tls_requirer_is_integrated_when_ca_common_name_config_chang async def test_given_tls_requirer_is_integrated_when_certificates_expires_then_new_certificate_is_provided( # noqa: E501 ops_test: OpsTest, - deploy, + deploy: None, ): new_common_name = "newexample.org" assert ops_test.model @@ -193,7 +193,7 @@ async def test_given_tls_requirer_is_integrated_when_certificates_expires_then_n async def test_given_charm_scaled_then_charm_does_not_crash( ops_test: OpsTest, - deploy, + deploy: None, ): assert ops_test.model await ops_test.model.applications[APP_NAME].scale(2) # type: ignore @@ -202,7 +202,7 @@ async def test_given_charm_scaled_then_charm_does_not_crash( await ops_test.model.wait_for_idle(apps=[APP_NAME], timeout=1000, wait_for_exact_units=1) -async def run_get_certificate_action(ops_test) -> Dict[str, str]: +async def run_get_certificate_action(ops_test: OpsTest) -> Dict[str, str]: """Run `get-certificate` on the `tls-requirer-requirer/0` unit. Args: diff --git a/tests/unit/test_charm_collect_status.py b/tests/unit/test_charm_collect_status.py index 90126ab..8db6fff 100644 --- a/tests/unit/test_charm_collect_status.py +++ b/tests/unit/test_charm_collect_status.py @@ -1,7 +1,7 @@ # Copyright 2024 Canonical Ltd. # See LICENSE file for licensing details. -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest import scenario @@ -69,11 +69,11 @@ def test_given_root_ca_validity_config_not_twice_the_certificate_validity_when_c @patch("charm.generate_ca") def test_given_valid_config_when_collect_unit_status_then_status_is_active( self, - patch_generate_ca, - patch_generate_private_key, + mock_generate_ca: MagicMock, + mock_generate_private_key: MagicMock, ): - patch_generate_ca.return_value = "whatever CA certificate" - patch_generate_private_key.return_value = "whatever private key" + mock_generate_ca.return_value = "whatever CA certificate" + mock_generate_private_key.return_value = "whatever private key" state_in = scenario.State( config={ "ca-common-name": "pizza.example.com", diff --git a/tests/unit/test_charm_configure.py b/tests/unit/test_charm_configure.py index 02a7433..bf142f6 100644 --- a/tests/unit/test_charm_configure.py +++ b/tests/unit/test_charm_configure.py @@ -2,7 +2,7 @@ # See LICENSE file for licensing details. from datetime import datetime, timedelta -from unittest.mock import call, mock_open, patch +from unittest.mock import MagicMock, call, mock_open, patch import pytest import scenario @@ -40,13 +40,13 @@ def context(self): @patch("charm.generate_ca") def test_given_valid_config_when_config_changed_then_ca_certificate_is_pushed_to_charm_container( # noqa: E501 self, - patch_generate_ca, - patch_generate_private_key, + mock_generate_ca: MagicMock, + mock_generate_private_key: MagicMock, ): ca_certificate_string = "whatever CA certificate" private_key_string = "whatever private key" - patch_generate_ca.return_value = ca_certificate_string - patch_generate_private_key.return_value = private_key_string + mock_generate_ca.return_value = ca_certificate_string + mock_generate_private_key.return_value = private_key_string state_in = scenario.State( config={ "ca-common-name": "pizza.example.com", @@ -64,13 +64,13 @@ def test_given_valid_config_when_config_changed_then_ca_certificate_is_pushed_to @patch("charm.generate_ca") def test_given_valid_config_when_config_changed_then_ca_certificate_is_stored_in_juju_secret( self, - patch_generate_ca, - patch_generate_private_key, + mock_generate_ca: MagicMock, + mock_generate_private_key: MagicMock, ): ca_certificate_string = "whatever CA certificate" private_key_string = "whatever private key" - patch_generate_ca.return_value = ca_certificate_string - patch_generate_private_key.return_value = private_key_string + mock_generate_ca.return_value = ca_certificate_string + mock_generate_private_key.return_value = private_key_string certificate_validity = 100 root_ca_validity = 200 @@ -102,12 +102,12 @@ def test_given_valid_config_when_config_changed_then_ca_certificate_is_stored_in @patch("charm.generate_ca") def test_given_root_certificate_not_stored_when_config_changed_then_existing_certificates_are_revoked( # noqa: E501 self, - patch_generate_ca, - patch_generate_private_key, - patch_revoke_all_certificates, + mock_generate_ca: MagicMock, + mock_generate_private_key: MagicMock, + mock_revoke_all_certificates: MagicMock, ): - patch_generate_ca.return_value = "whatever CA certificate" - patch_generate_private_key.return_value = "whatever private key" + mock_generate_ca.return_value = "whatever CA certificate" + mock_generate_private_key.return_value = "whatever private key" state_in = scenario.State( config={ "ca-common-name": "pizza.example.com", @@ -119,14 +119,14 @@ def test_given_root_certificate_not_stored_when_config_changed_then_existing_cer self.ctx.run(self.ctx.on.config_changed(), state=state_in) - assert patch_revoke_all_certificates.called + assert mock_revoke_all_certificates.called @patch("charm.generate_private_key") @patch("charm.generate_ca") def test_given_new_root_ca_config_when_config_changed_then_new_root_ca_is_replaced( self, - patch_generate_ca, - patch_generate_private_key, + mock_generate_ca: MagicMock, + mock_generate_private_key: MagicMock, ): ca_private_key = generate_private_key() initial_ca_certificate = generate_ca( @@ -139,8 +139,8 @@ def test_given_new_root_ca_config_when_config_changed_then_new_root_ca_is_replac common_name="new.example.com", validity=timedelta(days=100), ) - patch_generate_ca.return_value = new_ca - patch_generate_private_key.return_value = ca_private_key + mock_generate_ca.return_value = new_ca + mock_generate_private_key.return_value = ca_private_key ca_certificate_secret = scenario.Secret( { "ca-certificate": str(initial_ca_certificate), @@ -169,7 +169,7 @@ def test_given_new_root_ca_config_when_config_changed_then_new_root_ca_is_replac secret_content = ca_certificates_secret.latest_content assert secret_content is not None assert secret_content["ca-certificate"] == str(new_ca) - patch_generate_ca.assert_called_with( + mock_generate_ca.assert_called_with( private_key=ca_private_key, common_name="pizza.example.com", organization=None, @@ -185,8 +185,8 @@ def test_given_new_root_ca_config_when_config_changed_then_new_root_ca_is_replac @patch("charm.generate_ca") def test_given_root_ca_about_to_expire_then_root_ca_is_marked_expiring_and_new_one_is_generated( # noqa: E501 self, - patch_generate_ca, - patch_generate_private_key, + mock_generate_ca: MagicMock, + mock_generate_private_key: MagicMock, ): initial_ca_private_key = generate_private_key() new_ca_private_key = generate_private_key() @@ -200,8 +200,8 @@ def test_given_root_ca_about_to_expire_then_root_ca_is_marked_expiring_and_new_o common_name="example.com", validity=timedelta(minutes=2), ) - patch_generate_ca.return_value = new_ca_certificate - patch_generate_private_key.return_value = new_ca_private_key + mock_generate_ca.return_value = new_ca_certificate + mock_generate_private_key.return_value = new_ca_private_key ca_certificate_secret = scenario.Secret( { "ca-certificate": str(initial_ca_certificate), @@ -248,9 +248,9 @@ def test_given_root_ca_about_to_expire_then_root_ca_is_marked_expiring_and_new_o @patch("charm.generate_certificate") def test_given_outstanding_certificate_requests_when_config_changed_then_certificates_are_generated( # noqa: E501 self, - patch_generate_certificate, - patch_get_outstanding_certificate_requests, - patch_set_relation_certificate, + mock_generate_certificate: MagicMock, + mock_get_outstanding_certificate_requests: MagicMock, + mock_set_relation_certificate: MagicMock, ): requirer_private_key = generate_private_key() provider_private_key = generate_private_key() @@ -279,14 +279,14 @@ def test_given_outstanding_certificate_requests_when_config_changed_then_certifi owner="app", expire=datetime.now() + timedelta(days=100), ) - patch_get_outstanding_certificate_requests.return_value = [ + mock_get_outstanding_certificate_requests.return_value = [ RequirerCertificateRequest( relation_id=tls_relation.id, certificate_signing_request=requirer_csr, is_ca=False, ), ] - patch_generate_certificate.return_value = certificate + mock_generate_certificate.return_value = certificate state_in = scenario.State( config={ "ca-common-name": "example.com", @@ -308,7 +308,7 @@ def test_given_outstanding_certificate_requests_when_config_changed_then_certifi ca=provider_ca, chain=[provider_ca, certificate], ) - patch_set_relation_certificate.assert_called_with( + mock_set_relation_certificate.assert_called_with( provider_certificate=expected_provider_certificate, ) @@ -317,9 +317,9 @@ def test_given_outstanding_certificate_requests_when_config_changed_then_certifi @patch("charm.generate_certificate") def test_given_outstanding_certificate_requests_and_limit_reached_when_config_changed_then_certificate_number_is_limited( # noqa: E501 self, - patch_generate_certificate, - patch_get_outstanding_certificate_requests, - patch_set_relation_certificate, + mock_generate_certificate: MagicMock, + mock_get_outstanding_certificate_requests: MagicMock, + mock_set_relation_certificate: MagicMock, ): requirer_private_key = generate_private_key() provider_private_key = generate_private_key() @@ -362,7 +362,7 @@ def test_given_outstanding_certificate_requests_and_limit_reached_when_config_ch owner="app", expire=datetime.now() + timedelta(days=100), ) - patch_get_outstanding_certificate_requests.return_value = [ + mock_get_outstanding_certificate_requests.return_value = [ RequirerCertificateRequest( relation_id=tls_relation.id, certificate_signing_request=requirer_csr_1, @@ -379,7 +379,7 @@ def test_given_outstanding_certificate_requests_and_limit_reached_when_config_ch is_ca=False, ), ] - patch_generate_certificate.side_effect = [certificate_1, certificate_2, certificate_3] + mock_generate_certificate.side_effect = [certificate_1, certificate_2, certificate_3] state_in = scenario.State( config={ "ca-common-name": "example.com", @@ -408,26 +408,26 @@ def test_given_outstanding_certificate_requests_and_limit_reached_when_config_ch ca=provider_ca, chain=[provider_ca, certificate_2], ) - patch_set_relation_certificate.assert_has_calls( + mock_set_relation_certificate.assert_has_calls( [ call(provider_certificate=expected_provider_certificate_1), call(provider_certificate=expected_provider_certificate_2), ] ) - assert patch_generate_certificate.call_count == 2 + assert mock_generate_certificate.call_count == 2 @pytest.mark.skip(reason="https://github.com/canonical/operator/issues/1316") @patch("charm.generate_private_key") @patch("charm.generate_ca") def test_given_valid_config_and_unit_is_leader_when_secret_expired_then_new_ca_certificate_is_stored_in_juju_secret( # noqa: E501 self, - patch_generate_ca, - patch_generate_private_key, + mock_generate_ca: MagicMock, + mock_generate_private_key: MagicMock, ): ca_certificate_string = "whatever CA certificate" private_key_string = "whatever private key" - patch_generate_ca.return_value = ca_certificate_string - patch_generate_private_key.return_value = private_key_string + mock_generate_ca.return_value = ca_certificate_string + mock_generate_private_key.return_value = private_key_string ca_certificates_secret = scenario.Secret( { @@ -463,8 +463,8 @@ def test_given_valid_config_and_unit_is_leader_when_secret_expired_then_new_ca_c @patch("charm.generate_ca") def test_given_initial_config_when_config_changed_then_stored_ca_common_name_uses_new_config( self, - patch_generate_ca, - patch_generate_private_key, + mock_generate_ca: MagicMock, + mock_private_key_generator: MagicMock, ): initial_ca_private_key = generate_private_key() new_ca_private_key = generate_private_key() @@ -478,8 +478,8 @@ def test_given_initial_config_when_config_changed_then_stored_ca_common_name_use common_name="common-name-new.example.com", validity=timedelta(days=100), ) - patch_generate_ca.return_value = new_ca_certificate - patch_generate_private_key.return_value = new_ca_private_key + mock_generate_ca.return_value = new_ca_certificate + mock_private_key_generator.return_value = new_ca_private_key ca_certificates_secret = scenario.Secret( { diff --git a/tests/unit/test_charm_get_issued_certificates.py b/tests/unit/test_charm_get_issued_certificates.py index 54b61f4..57e240d 100644 --- a/tests/unit/test_charm_get_issued_certificates.py +++ b/tests/unit/test_charm_get_issued_certificates.py @@ -3,7 +3,7 @@ import json from datetime import timedelta -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest import scenario @@ -40,7 +40,7 @@ def test_given_no_certificates_issued_when_get_issued_certificates_action_then_a @patch(f"{TLS_LIB_PATH}.TLSCertificatesProvidesV4.get_issued_certificates") def test_given_certificates_issued_when_get_issued_certificates_action_then_action_returns_certificates( # noqa: E501 self, - patch_get_issued_certificates, + mock_get_issued_certificates: MagicMock, ): ca_private_key = generate_private_key() ca_certificate = generate_ca( @@ -58,7 +58,7 @@ def test_given_certificates_issued_when_get_issued_certificates_action_then_acti ) chain = [ca_certificate, certificate] revoked = False - patch_get_issued_certificates.return_value = [ + mock_get_issued_certificates.return_value = [ ProviderCertificate( relation_id=1, certificate_signing_request=csr, diff --git a/tests/unit/test_charm_rotate_private_key.py b/tests/unit/test_charm_rotate_private_key.py index b02a7c5..fde7457 100644 --- a/tests/unit/test_charm_rotate_private_key.py +++ b/tests/unit/test_charm_rotate_private_key.py @@ -1,7 +1,7 @@ # Copyright 2024 Canonical Ltd. # See LICENSE file for licensing details. -from unittest.mock import patch +from unittest.mock import MagicMock, patch import pytest import scenario @@ -31,7 +31,7 @@ def test_given_not_leader_when_rotate_private_key_action_then_action_fails(self) @patch(f"{TLS_LIB_PATH}.TLSCertificatesProvidesV4.revoke_all_certificates") def test_given_rotate_private_key_action_when_certificates_revoked_and_root_certificate_generated_then_action_succeeds( # noqa: E501 self, - patch_revoke_all_certificates, + patch_revoke_all_certificates: MagicMock, ): state_in = scenario.State( leader=True, @@ -46,7 +46,7 @@ def test_given_rotate_private_key_action_when_certificates_revoked_and_root_cert @patch(f"{TLS_LIB_PATH}.TLSCertificatesProvidesV4.revoke_all_certificates") def test_given_rotate_private_key_action_when_config_is_invalid_then_action_fails( # noqa: E501 self, - patch_revoke_all_certificates, + mock_revoke_all_certificates: MagicMock, ): state_in = scenario.State( leader=True,