From 0779df3b51257d3c417199fae3cf62b57d6d70b9 Mon Sep 17 00:00:00 2001 From: natalia Date: Wed, 15 Nov 2023 13:45:44 +0100 Subject: [PATCH] refactor: address review comments --- lib/charms/oathkeeper/v0/forward_auth.py | 20 +++++------ metadata.yaml | 3 ++ src/charm.py | 44 +++++++++++------------- 3 files changed, 34 insertions(+), 33 deletions(-) diff --git a/lib/charms/oathkeeper/v0/forward_auth.py b/lib/charms/oathkeeper/v0/forward_auth.py index d29318df..e6de32b5 100644 --- a/lib/charms/oathkeeper/v0/forward_auth.py +++ b/lib/charms/oathkeeper/v0/forward_auth.py @@ -27,18 +27,18 @@ Then, to initialise the library: ```python -from charms.oathkeeper.v0.forward_auth import ForwardAuthConfigChangedEvent, ForwardAuthRequirer +from charms.oathkeeper.v0.forward_auth import AuthConfigChangedEvent, ForwardAuthRequirer class ApiGatewayCharm(CharmBase): def __init__(self, *args): # ... self.forward_auth = ForwardAuthRequirer(self) self.framework.observe( - self.forward_auth.on.forward_auth_config_changed, + self.forward_auth.on.auth_config_changed, self.some_event_function ) - def some_event_function(self, event: ForwardAuthConfigChangedEvent): + def some_event_function(self, event: AuthConfigChangedEvent): if self.forward_auth.is_ready(): # Fetch the relation info forward_auth_data = self.forward_auth.get_forward_auth_data() @@ -66,7 +66,7 @@ def some_event_function(self, event: ForwardAuthConfigChangedEvent): # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version -LIBPATCH = 1 +LIBPATCH = 2 RELATION_NAME = "forward-auth" INTERFACE_NAME = "forward_auth" @@ -198,7 +198,7 @@ def to_dict(self) -> Dict: return {k: v for k, v in asdict(self).items() if v is not None} -class ForwardAuthConfigChangedEvent(EventBase): +class AuthConfigChangedEvent(EventBase): """Event to notify the requirer charm that the forward-auth config has changed.""" def __init__( @@ -236,7 +236,7 @@ def restore(self, snapshot: Dict) -> None: self.relation_app_name = snapshot["relation_app_name"] -class ForwardAuthConfigRemovedEvent(EventBase): +class AuthConfigRemovedEvent(EventBase): """Event to notify the requirer charm that the forward-auth config was removed.""" def __init__( @@ -259,8 +259,8 @@ def restore(self, snapshot: Dict) -> None: class ForwardAuthRequirerEvents(ObjectEvents): """Event descriptor for events raised by `ForwardAuthRequirer`.""" - forward_auth_config_changed = EventSource(ForwardAuthConfigChangedEvent) - forward_auth_config_removed = EventSource(ForwardAuthConfigRemovedEvent) + auth_config_changed = EventSource(AuthConfigChangedEvent) + auth_config_removed = EventSource(AuthConfigRemovedEvent) class ForwardAuthRequirer(ForwardAuthRelation): @@ -318,13 +318,13 @@ def _on_relation_changed_event(self, event: RelationChangedEvent) -> None: relation_app_name = event.relation.app.name # Notify Traefik to update the routes - self.on.forward_auth_config_changed.emit( + self.on.auth_config_changed.emit( decisions_address, app_names, headers, relation_id, relation_app_name ) def _on_relation_departed_event(self, event: RelationDepartedEvent) -> None: """Notify the requirer that the relation has departed.""" - self.on.forward_auth_config_removed.emit(event.relation.id) + self.on.auth_config_removed.emit(event.relation.id) def update_requirer_relation_data( self, ingress_app_names: Optional[RequirerConfig], relation_id: Optional[int] = None diff --git a/metadata.yaml b/metadata.yaml index 29824202..23824eb1 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -75,6 +75,9 @@ requires: forward-auth: interface: forward_auth limit: 1 + description: | + Receives forwardAuth middleware config from Oathkeeper. + The same middleware will be applied to all charms that requested Identity and Access Proxy protection. logging: interface: loki_push_api description: | diff --git a/src/charm.py b/src/charm.py index f70abc98..e67c02f4 100755 --- a/src/charm.py +++ b/src/charm.py @@ -7,6 +7,7 @@ import enum import functools import ipaddress +import itertools import json import logging import re @@ -29,8 +30,8 @@ from charms.grafana_k8s.v0.grafana_dashboard import GrafanaDashboardProvider from charms.loki_k8s.v0.loki_push_api import LogProxyConsumer from charms.oathkeeper.v0.forward_auth import ( - ForwardAuthConfigChangedEvent, - ForwardAuthConfigRemovedEvent, + AuthConfigChangedEvent, + AuthConfigRemovedEvent, ForwardAuthRequirer, RequirerConfig, ) @@ -229,7 +230,7 @@ def __init__(self, *args): ) self.forward_auth = ForwardAuthRequirer( - self, forward_auth_requirer_config=self._forward_auth_requirer_config + self, forward_auth_requirer_config=self._forward_auth_config ) observe = self.framework.observe @@ -265,12 +266,8 @@ def __init__(self, *args): self._on_recv_ca_cert_removed, ) - observe( - self.forward_auth.on.forward_auth_config_changed, self._on_forward_auth_config_changed - ) - observe( - self.forward_auth.on.forward_auth_config_removed, self._on_forward_auth_config_removed - ) + observe(self.forward_auth.on.auth_config_changed, self._on_forward_auth_config_changed) + observe(self.forward_auth.on.auth_config_removed, self._on_forward_auth_config_removed) # observe data_provided and data_removed events for all types of ingress we offer: for ingress in (self.ingress_per_unit, self.ingress_per_appv1, self.ingress_per_appv2): @@ -285,23 +282,24 @@ def __init__(self, *args): observe(self.on.show_proxied_endpoints_action, self._on_show_proxied_endpoints) # type: ignore @property - def _forward_auth_requirer_config(self) -> RequirerConfig: - ingress_app_names = [] - for ingress_relation in ( - self.ingress_per_appv1.relations - + self.ingress_per_appv2.relations - + self.ingress_per_unit.relations - + self.traefik_route.relations - ): - ingress_app_names.append(ingress_relation.app.name) # type: ignore + def _forward_auth_config(self) -> RequirerConfig: + ingress_app_names = [ + rel.app.name # type: ignore + for rel in itertools.chain( + self.ingress_per_appv1.relations, + self.ingress_per_appv2.relations, + self.ingress_per_unit.relations, + self.traefik_route.relations, + ) + ] return RequirerConfig(ingress_app_names) - def _on_forward_auth_config_changed(self, event: ForwardAuthConfigChangedEvent): - self.forward_auth.update_requirer_relation_data(self._forward_auth_requirer_config) + def _on_forward_auth_config_changed(self, event: AuthConfigChangedEvent): + self.forward_auth.update_requirer_relation_data(self._forward_auth_config) if self.forward_auth.is_ready(): self._process_status_and_configurations() - def _on_forward_auth_config_removed(self, event: ForwardAuthConfigRemovedEvent): + def _on_forward_auth_config_removed(self, event: AuthConfigRemovedEvent): self._process_status_and_configurations() def _on_recv_ca_cert_available(self, event: CertificateTransferAvailableEvent): @@ -760,7 +758,7 @@ def _handle_ingress_data_provided(self, event: RelationEvent): self._process_status_and_configurations() if self.forward_auth.is_ready(): - self.forward_auth.update_requirer_relation_data(self._forward_auth_requirer_config) + self.forward_auth.update_requirer_relation_data(self._forward_auth_config) if isinstance(self.unit.status, MaintenanceStatus): self.unit.status = ActiveStatus() @@ -772,7 +770,7 @@ def _handle_ingress_data_removed(self, event: RelationEvent): ) if self.forward_auth.is_ready(): - self.forward_auth.update_requirer_relation_data(self._forward_auth_requirer_config) + self.forward_auth.update_requirer_relation_data(self._forward_auth_config) # FIXME? on relation broken, data is still there so cannot simply call # self._process_status_and_configurations(). For this reason, the static config in