Merge pull request #42 from canonical/IAM-195-dev-nitpicks

Dev nitpicks

CONTRIBUTING.md

@@ -31,11 +31,35 @@ source .tox/unit/bin/activate
 ### Testing
 
 ```shell
-tox -e fmt           # update your code according to linting rules
-tox -e lint          # code style
 tox -e unit          # unit tests
 tox -e integration   # integration tests
-tox                  # runs 'lint' and 'unit' environments
+```
+
+To test this charm manually, execute the container:
+```bash
+kubectl exec -it hydra-0 -c hydra -n <model> -- sh
+```
+
+Create an exemplary client:
+```shell
+# hydra create client --endpoint http://127.0.0.1:4445/ --name example-client
+CLIENT ID	b55b6857-968e-4fb7-be77-f701ec751405
+CLIENT SECRET	b3wFYH2N_epJY6C8jCuinBRP60
+GRANT TYPES	authorization_code
+RESPONSE TYPES	code
+SCOPE		offline_access offline openid
+AUDIENCE
+REDIRECT URIS
+```
+
+List the clients:
+```shell
+# hydra list clients --endpoint http://127.0.0.1:4445/
+CLIENT ID				CLIENT SECRET	GRANT TYPES		RESPONSE TYPES	SCOPE				AUDIENCE	REDIRECT URIS
+b55b6857-968e-4fb7-be77-f701ec751405			authorization_code	code		offline_access offline openid
+
+NEXT PAGE TOKEN
+IS LAST PAGE				true
 ```
 
 ## Build charm

README.md

@@ -2,73 +2,18 @@
 
 ## Description
 
-This repository hosts the Kubernetes Python Operator for Ory Hydra - a scalable, security first OAuth 2.0 and OpenID Connect server.
-
-For more details and documentation, visit https://www.ory.sh/docs/hydra/
+Python Operator for Ory Hydra - a scalable, security first OAuth 2.0 and OpenID Connect server. For more details and documentation, visit https://www.ory.sh/docs/hydra/
 
 ## Usage
 
-The Hydra Operator may be deployed using the Juju command line as follows:
-
-Deploy the `postgresql-k8s` charm:
-
 ```bash
 juju deploy postgresql-k8s --channel edge --trust
-```
-
-Clone this repository and pack the Hydra Operator with charmcraft:
-```bash
-charmcraft pack
-```
-
-Deploy the charm:
-<!-- TODO: Update to deploy from charmhub once the charm is published -->
-```bash
-juju deploy ./hydra*.charm --resource oci-image=$(yq eval '.resources.oci-image.upstream-source' metadata.yaml)
-```
-
-Finally, add the required relation:
-```bash
+juju deploy hydra --trust
 juju relate postgresql-k8s hydra
 ```
 
 You can follow the deployment status with `watch -c juju status --color`.
 
-## Testing
-
-Unit and integration tests can be run with tox:
-```bash
-tox -e unit
-tox -e integration
-```
-
-To test this charm manually, execute the container:
-```bash
-kubectl exec -it hydra-0 -c hydra -n <model> -- sh
-```
-
-Create an exemplary client:
-```shell
-# hydra create client --endpoint http://127.0.0.1:4445/ --name example-client
-CLIENT ID	b55b6857-968e-4fb7-be77-f701ec751405
-CLIENT SECRET	b3wFYH2N_epJY6C8jCuinBRP60
-GRANT TYPES	authorization_code
-RESPONSE TYPES	code
-SCOPE		offline_access offline openid
-AUDIENCE
-REDIRECT URIS
-```
-
-List the clients:
-```shell
-# hydra list clients --endpoint http://127.0.0.1:4445/
-CLIENT ID				CLIENT SECRET	GRANT TYPES		RESPONSE TYPES	SCOPE				AUDIENCE	REDIRECT URIS
-b55b6857-968e-4fb7-be77-f701ec751405			authorization_code	code		offline_access offline openid
-
-NEXT PAGE TOKEN
-IS LAST PAGE				true
-```
-
 ## Relations
 
 ### PostgreSQL
@@ -77,16 +22,17 @@ This charm requires a relation with [postgresql-k8s-operator](https://github.com
 
 ### Ingress
 
-The Hydra Operator offers integration with the [traefik-k8s-operator](https://github.com/canonical/traefik-k8s-operator) for ingress.
-Hydra has two APIs which can be exposed through ingress, the public API and the admin API.
+The Hydra Operator offers integration with the [traefik-k8s-operator](https://github.com/canonical/traefik-k8s-operator) for ingress. Hydra has two APIs which can be exposed through ingress, the public API and the admin API.
 
 If you have traefik deployed and configured in your hydra model, to provide ingress to the admin API run:
-```console
+
+```bash
 juju relate traefik-admin hydra:admin-ingress
 ```
 
 To provide ingress to the public API run:
-```console
+
+```bash
 juju relate traefik-public hydra:public-ingress
 ```
 

charmcraft.yaml

@@ -9,7 +9,3 @@ bases:
     run-on:
       - name: "ubuntu"
         channel: "22.04"
-parts:
-  charm:
-    build-packages:
-      - git

lib/charms/hydra/v0/hydra_endpoints.py

@@ -39,6 +39,7 @@ def some_event_function():
 """
 
 import logging
+from typing import Dict, Optional
 
 from ops.charm import CharmBase, RelationCreatedEvent
 from ops.framework import EventBase, EventSource, Object, ObjectEvents
@@ -85,19 +86,17 @@ def __init__(self, charm: CharmBase, relation_name: str = RELATION_NAME):
             events.relation_created, self._on_provider_endpoint_relation_created
         )
 
-    def _on_provider_endpoint_relation_created(self, event: RelationCreatedEvent):
+    def _on_provider_endpoint_relation_created(self, event: RelationCreatedEvent) -> None:
         self.on.ready.emit()
 
-    def send_endpoint_relation_data(
-        self, charm: CharmBase, admin_endpoint: str, public_endpoint: str
-    ) -> None:
+    def send_endpoint_relation_data(self, admin_endpoint: str, public_endpoint: str) -> None:
         """Updates relation with endpoints info."""
         if not self._charm.unit.is_leader():
             return
 
         relations = self.model.relations[RELATION_NAME]
         for relation in relations:
-            relation.data[charm].update(
+            relation.data[self._charm.app].update(
                 {
                     "admin_endpoint": admin_endpoint,
                     "public_endpoint": public_endpoint,
@@ -114,15 +113,15 @@ class HydraEndpointsRelationError(Exception):
 class HydraEndpointsRelationMissingError(HydraEndpointsRelationError):
     """Raised when the relation is missing."""
 
-    def __init__(self):
+    def __init__(self) -> None:
         self.message = "Missing endpoint-info relation with hydra"
         super().__init__(self.message)
 
 
 class HydraEndpointsRelationDataMissingError(HydraEndpointsRelationError):
     """Raised when information is missing from the relation."""
 
-    def __init__(self, message):
+    def __init__(self, message: str) -> None:
         self.message = message
         super().__init__(self.message)
 
@@ -135,10 +134,10 @@ def __init__(self, charm: CharmBase, relation_name: str = RELATION_NAME):
         self.charm = charm
         self.relation_name = relation_name
 
-    def get_hydra_endpoints(self) -> dict:
+    def get_hydra_endpoints(self) -> Optional[Dict]:
         """Get the hydra endpoints."""
         if not self.model.unit.is_leader():
-            return
+            return None
         endpoints = self.model.relations[self.relation_name]
         if len(endpoints) == 0:
             raise HydraEndpointsRelationMissingError()

lib/charms/hydra/v0/oauth.py

@@ -422,7 +422,7 @@ def __init__(
         grant_types: List[str],
         audience: List,
         token_endpoint_auth_method: str,
-        relation_id: str,
+        relation_id: int,
     ) -> None:
         super().__init__(handle)
         self.redirect_uri = redirect_uri

pyproject.toml

@@ -38,3 +38,26 @@ docstring-convention = "google"
 copyright-check = "True"
 copyright-author = "Canonical Ltd."
 copyright-regexp = "Copyright\\s\\d{4}([-,]\\d{4})*\\s+%(author)s"
+
+
+[tool.mypy]
+pretty = true
+mypy_path = "./src:./lib/:./tests"
+# Exclude non-hydra libraries
+exclude = 'lib/charms/((?!hydra).)'
+follow_imports = "silent"
+warn_redundant_casts = true
+warn_unused_configs = true
+show_traceback = true
+show_error_codes = true
+namespace_packages = true
+explicit_package_bases = true
+check_untyped_defs = true
+allow_redefinition = true
+disallow_incomplete_defs = true
+disallow_untyped_defs = true
+
+# Ignore libraries that do not have type hint nor stubs
+[[tool.mypy.overrides]]
+module = ["ops.*", "pytest.*", "pytest_operator.*", "urllib3.*", "jinja2.*", "lightkube.*", "pytest_mock.*"]
+ignore_missing_imports = true

src/charm.py

@@ -8,6 +8,7 @@
 
 import logging
 from os.path import join
+from typing import Any
 
 from charms.data_platform_libs.v0.data_interfaces import (
     DatabaseCreatedEvent,
@@ -50,7 +51,7 @@
 class HydraCharm(CharmBase):
     """Charmed Ory Hydra."""
 
-    def __init__(self, *args):
+    def __init__(self, *args: Any) -> None:
         super().__init__(*args)
 
         self._container_name = "hydra"
@@ -156,16 +157,16 @@ def _hydra_service_is_running(self) -> bool:
             return False
         return service.is_running()
 
-    def _render_conf_file(self) -> None:
+    def _render_conf_file(self) -> str:
         """Render the Hydra configuration file."""
         with open("templates/hydra.yaml.j2", "r") as file:
             template = Template(file.read())
 
         rendered = template.render(
             db_info=self._get_database_relation_info(),
-            consent_url=join(self.config.get("login_ui_url"), "consent"),
-            error_url=join(self.config.get("login_ui_url"), "oidc_error"),
-            login_url=join(self.config.get("login_ui_url"), "login"),
+            consent_url=join(self.config.get("login_ui_url", ""), "consent"),
+            error_url=join(self.config.get("login_ui_url", ""), "oidc_error"),
+            login_url=join(self.config.get("login_ui_url", ""), "login"),
             hydra_public_url=self.public_ingress.url
             if self.public_ingress.is_ready()
             else f"http://127.0.0.1:{HYDRA_PUBLIC_PORT}/",
@@ -249,9 +250,7 @@ def _update_hydra_endpoints_relation_data(self, event: RelationEvent) -> None:
             f"Sending endpoints info: public - {public_endpoint[0]} admin - {admin_endpoint[0]}"
         )
 
-        self.endpoints_provider.send_endpoint_relation_data(
-            self.app, admin_endpoint[0], public_endpoint[0]
-        )
+        self.endpoints_provider.send_endpoint_relation_data(admin_endpoint[0], public_endpoint[0])
 
     def _on_hydra_pebble_ready(self, event: WorkloadEvent) -> None:
         """Event Handler for pebble ready event."""

src/hydra_cli.py

@@ -55,9 +55,7 @@ def _client_cmd_prefix(self, action: str) -> List[str]:
             "json",
         ]
 
-    def create_client(
-        self, client_config: ClientConfig, metadata: Optional[Union[Dict, str]] = None
-    ) -> Dict:
+    def create_client(self, client_config: ClientConfig, metadata: Optional[Dict] = None) -> Dict:
         """Create an oauth2 client."""
         cmd = self._client_cmd_prefix("create") + self._client_config_to_cmd(
             client_config, metadata
@@ -68,9 +66,7 @@ def create_client(
         logger.info(f"Successfully created client: {json_stdout.get('client_id')}")
         return json_stdout
 
-    def update_client(
-        self, client_config: ClientConfig, metadata: Optional[Union[Dict, str]] = None
-    ) -> Dict:
+    def update_client(self, client_config: ClientConfig, metadata: Optional[Dict] = None) -> Dict:
         """Update an oauth2 client."""
         cmd = self._client_cmd_prefix("update") + self._client_config_to_cmd(
             client_config, metadata

tests/integration/test_charm.py

@@ -26,7 +26,7 @@ async def get_unit_address(ops_test: OpsTest, app_name: str, unit_num: int) -> s
 
 
 @pytest.mark.abort_on_fail
-async def test_build_and_deploy(ops_test: OpsTest):
+async def test_build_and_deploy(ops_test: OpsTest) -> None:
     """Build hydra and deploy it with required charms and relations."""
     charm = await ops_test.build_charm(".")
     hydra_image_path = METADATA["resources"]["oci-image"]["upstream-source"]
@@ -60,7 +60,7 @@ async def test_build_and_deploy(ops_test: OpsTest):
         assert ops_test.model.applications[APP_NAME].units[0].workload_status == "active"
 
 
-async def test_ingress_relation(ops_test: OpsTest):
+async def test_ingress_relation(ops_test: OpsTest) -> None:
     await ops_test.model.deploy(
         TRAEFIK,
         application_name=TRAEFIK_PUBLIC_APP,
@@ -84,7 +84,7 @@ async def test_ingress_relation(ops_test: OpsTest):
     )
 
 
-async def test_has_public_ingress(ops_test: OpsTest):
+async def test_has_public_ingress(ops_test: OpsTest) -> None:
     # Get the traefik address and try to reach hydra
     public_address = await get_unit_address(ops_test, TRAEFIK_PUBLIC_APP, 0)
 
@@ -95,7 +95,7 @@ async def test_has_public_ingress(ops_test: OpsTest):
     assert resp.status_code == 200
 
 
-async def test_has_admin_ingress(ops_test: OpsTest):
+async def test_has_admin_ingress(ops_test: OpsTest) -> None:
     # Get the traefik address and try to reach hydra
     admin_address = await get_unit_address(ops_test, TRAEFIK_ADMIN_APP, 0)