Skip to content

Commit

Permalink
build: Enable pyright to check if type hints are missing
Browse files Browse the repository at this point in the history
This change adds a pyright check to ensure that we add type hints to our code
  • Loading branch information
DanielArndt committed Dec 13, 2024
1 parent 8801446 commit a696c2f
Show file tree
Hide file tree
Showing 7 changed files with 68 additions and 67 deletions.
1 change: 1 addition & 0 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -79,3 +79,4 @@ skip = "build,lib,venv,icon.svg,.tox,.git,.mypy_cache,.ruff_cache,.coverage"

[tool.pyright]
include = ["src/**.py"]
reportMissingParameterType = true
6 changes: 3 additions & 3 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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")
Expand Down Expand Up @@ -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:
Expand Down
16 changes: 8 additions & 8 deletions tests/integration/test_integration.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand Down
10 changes: 5 additions & 5 deletions tests/unit/test_charm_collect_status.py
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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",
Expand Down
90 changes: 45 additions & 45 deletions tests/unit/test_charm_configure.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -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
Expand Down Expand Up @@ -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",
Expand All @@ -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(
Expand All @@ -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),
Expand Down Expand Up @@ -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,
Expand All @@ -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()
Expand All @@ -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),
Expand Down Expand Up @@ -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()
Expand Down Expand Up @@ -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",
Expand All @@ -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,
)

Expand All @@ -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()
Expand Down Expand Up @@ -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,
Expand All @@ -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",
Expand Down Expand Up @@ -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(
{
Expand Down Expand Up @@ -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()
Expand All @@ -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(
{
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/test_charm_get_issued_certificates.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -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,
Expand Down
Loading

0 comments on commit a696c2f

Please sign in to comment.