Skip to content

Commit

Permalink
refactor: address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
natalian98 committed Nov 15, 2023
1 parent e0dd988 commit 0779df3
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 33 deletions.
20 changes: 10 additions & 10 deletions lib/charms/oathkeeper/v0/forward_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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__(
Expand Down Expand Up @@ -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__(
Expand All @@ -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):
Expand Down Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions metadata.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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: |
Expand Down
44 changes: 21 additions & 23 deletions src/charm.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
import enum
import functools
import ipaddress
import itertools
import json
import logging
import re
Expand All @@ -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,
)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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):
Expand All @@ -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):
Expand Down Expand Up @@ -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()
Expand All @@ -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
Expand Down

0 comments on commit 0779df3

Please sign in to comment.