From 8fec05f76703b2075afa4d20068273d7fb642f9e Mon Sep 17 00:00:00 2001 From: shipperizer Date: Thu, 20 Jun 2024 12:09:52 +0100 Subject: [PATCH 1/2] feat: introduce internal ingress --- .../traefik_route_k8s/v0/traefik_route.py | 359 ++++++++++++++++++ metadata.yaml | 6 + requirements.txt | 2 +- src/charm.py | 134 +++++++ src/constants.py | 6 + tests/unit/test_charm.py | 133 +++++++ 6 files changed, 639 insertions(+), 1 deletion(-) create mode 100644 lib/charms/traefik_route_k8s/v0/traefik_route.py create mode 100644 src/constants.py diff --git a/lib/charms/traefik_route_k8s/v0/traefik_route.py b/lib/charms/traefik_route_k8s/v0/traefik_route.py new file mode 100644 index 00000000..48bedf38 --- /dev/null +++ b/lib/charms/traefik_route_k8s/v0/traefik_route.py @@ -0,0 +1,359 @@ +#!/usr/bin/env python3 +# Copyright 2022 Canonical Ltd. +# See LICENSE file for licensing details. + +r"""# Interface Library for traefik_route. + +This library wraps relation endpoints for traefik_route. The requirer of this +relation is the traefik-route-k8s charm, or any charm capable of providing +Traefik configuration files. The provider is the traefik-k8s charm, or another +charm willing to consume Traefik configuration files. + +## Getting Started + +To get started using the library, you just need to fetch the library using `charmcraft`. + +```shell +cd some-charm +charmcraft fetch-lib charms.traefik_route_k8s.v0.traefik_route +``` + +To use the library from the provider side (Traefik): + +```yaml +requires: + traefik_route: + interface: traefik_route + limit: 1 +``` + +```python +from charms.traefik_route_k8s.v0.traefik_route import TraefikRouteProvider + +class TraefikCharm(CharmBase): + def __init__(self, *args): + # ... + self.traefik_route = TraefikRouteProvider(self) + + self.framework.observe( + self.traefik_route.on.ready, self._handle_traefik_route_ready + ) + + def _handle_traefik_route_ready(self, event): + config: str = self.traefik_route.get_config(event.relation) # yaml + # use config to configure Traefik +``` + +To use the library from the requirer side (TraefikRoute): + +```yaml +requires: + traefik-route: + interface: traefik_route + limit: 1 + optional: false +``` + +```python +# ... +from charms.traefik_route_k8s.v0.traefik_route import TraefikRouteRequirer + +class TraefikRouteCharm(CharmBase): + def __init__(self, *args): + # ... + traefik_route = TraefikRouteRequirer( + self, self.model.relations.get("traefik-route"), + "traefik-route" + ) + if traefik_route.is_ready(): + traefik_route.submit_to_traefik( + config={'my': {'traefik': 'configuration'}} + ) + +``` +""" +import logging +from typing import Optional + +import yaml +from ops.charm import CharmBase, CharmEvents, RelationEvent +from ops.framework import EventSource, Object, StoredState +from ops.model import Relation + +# The unique Charmhub library identifier, never change it +LIBID = "fe2ac43a373949f2bf61383b9f35c83c" + +# Increment this major API version when introducing breaking changes +LIBAPI = 0 + +# Increment this PATCH version before using `charmcraft publish-lib` or reset +# to 0 if you are raising the major API version +LIBPATCH = 9 + +log = logging.getLogger(__name__) + + +class TraefikRouteException(RuntimeError): + """Base class for exceptions raised by TraefikRoute.""" + + +class UnauthorizedError(TraefikRouteException): + """Raised when the unit needs leadership to perform some action.""" + + +class TraefikRouteProviderReadyEvent(RelationEvent): + """Event emitted when Traefik is ready to provide ingress for a routed unit.""" + + +class TraefikRouteProviderDataRemovedEvent(RelationEvent): + """Event emitted when a routed ingress relation is removed.""" + + +class TraefikRouteRequirerReadyEvent(RelationEvent): + """Event emitted when a unit requesting ingress has provided all data Traefik needs.""" + + +class TraefikRouteRequirerEvents(CharmEvents): + """Container for TraefikRouteRequirer events.""" + + ready = EventSource(TraefikRouteRequirerReadyEvent) + + +class TraefikRouteProviderEvents(CharmEvents): + """Container for TraefikRouteProvider events.""" + + ready = EventSource(TraefikRouteProviderReadyEvent) # TODO rename to data_provided in v1 + data_removed = EventSource(TraefikRouteProviderDataRemovedEvent) + + +class TraefikRouteProvider(Object): + """Implementation of the provider of traefik_route. + + This will presumably be owned by a Traefik charm. + The main idea is that Traefik will observe the `ready` event and, upon + receiving it, will fetch the config from the TraefikRoute's application databag, + apply it, and update its own app databag to let Route know that the ingress + is there. + The TraefikRouteProvider provides api to do this easily. + """ + + on = TraefikRouteProviderEvents() # pyright: ignore + _stored = StoredState() + + def __init__( + self, + charm: CharmBase, + relation_name: str = "traefik-route", + external_host: str = "", + *, + scheme: str = "http", + ): + """Constructor for TraefikRouteProvider. + + Args: + charm: The charm that is instantiating the instance. + relation_name: The name of the relation relation_name to bind to + (defaults to "traefik-route"). + external_host: The external host. + scheme: The scheme. + """ + super().__init__(charm, relation_name) + self._stored.set_default(external_host=None, scheme=None) + + self._charm = charm + self._relation_name = relation_name + + if ( + self._stored.external_host != external_host # pyright: ignore + or self._stored.scheme != scheme # pyright: ignore + ): + # If traefik endpoint details changed, update + self.update_traefik_address(external_host=external_host, scheme=scheme) + + self.framework.observe( + self._charm.on[relation_name].relation_changed, self._on_relation_changed + ) + self.framework.observe( + self._charm.on[relation_name].relation_broken, self._on_relation_broken + ) + + @property + def external_host(self) -> str: + """Return the external host set by Traefik, if any.""" + self._update_stored() + return self._stored.external_host or "" # type: ignore + + @property + def scheme(self) -> str: + """Return the scheme set by Traefik, if any.""" + self._update_stored() + return self._stored.scheme or "" # type: ignore + + @property + def relations(self): + """The list of Relation instances associated with this endpoint.""" + return list(self._charm.model.relations[self._relation_name]) + + def _update_stored(self) -> None: + """Ensure that the stored data is up-to-date. + + This is split out into a separate method since, in the case of multi-unit deployments, + removal of a `TraefikRouteRequirer` will not cause a `RelationEvent`, but the guard on + app data ensures that only the previous leader will know what it is. Separating it + allows for reuse both when the property is called and if the relation changes, so a + leader change where the new leader checks the property will do the right thing. + """ + if not self._charm.unit.is_leader(): + return + + for relation in self._charm.model.relations[self._relation_name]: + if not relation.app: + self._stored.external_host = "" + self._stored.scheme = "" + return + external_host = relation.data[relation.app].get("external_host", "") + self._stored.external_host = ( + external_host or self._stored.external_host # pyright: ignore + ) + scheme = relation.data[relation.app].get("scheme", "") + self._stored.scheme = scheme or self._stored.scheme # pyright: ignore + + def _on_relation_changed(self, event: RelationEvent): + if self.is_ready(event.relation): + # todo check data is valid here? + self.update_traefik_address() + self.on.ready.emit(event.relation) + + def _on_relation_broken(self, event: RelationEvent): + self.on.data_removed.emit(event.relation) + + def update_traefik_address( + self, *, external_host: Optional[str] = None, scheme: Optional[str] = None + ): + """Ensure that requirers know the external host for Traefik.""" + if not self._charm.unit.is_leader(): + return + + for relation in self._charm.model.relations[self._relation_name]: + relation.data[self._charm.app]["external_host"] = external_host or self.external_host + relation.data[self._charm.app]["scheme"] = scheme or self.scheme + + # We first attempt to write relation data (which may raise) and only then update stored + # state. + self._stored.external_host = external_host + self._stored.scheme = scheme + + @staticmethod + def is_ready(relation: Relation) -> bool: + """Whether TraefikRoute is ready on this relation. + + Returns True when the remote app shared the config; False otherwise. + """ + assert relation.app is not None # not currently handled anyway + return "config" in relation.data[relation.app] + + @staticmethod + def get_config(relation: Relation) -> Optional[str]: + """Retrieve the config published by the remote application.""" + # TODO: validate this config + assert relation.app is not None # not currently handled anyway + return relation.data[relation.app].get("config") + + +class TraefikRouteRequirer(Object): + """Wrapper for the requirer side of traefik-route. + + The traefik_route requirer will publish to the application databag an object like: + { + 'config': + } + + NB: TraefikRouteRequirer does no validation; it assumes that the + traefik-route-k8s charm will provide valid yaml-encoded config. + The TraefikRouteRequirer provides api to store this config in the + application databag. + """ + + on = TraefikRouteRequirerEvents() # pyright: ignore + _stored = StoredState() + + def __init__(self, charm: CharmBase, relation: Relation, relation_name: str = "traefik-route"): + super(TraefikRouteRequirer, self).__init__(charm, relation_name) + self._stored.set_default(external_host=None, scheme=None) + + self._charm = charm + self._relation = relation + + self.framework.observe( + self._charm.on[relation_name].relation_changed, self._on_relation_changed + ) + self.framework.observe( + self._charm.on[relation_name].relation_broken, self._on_relation_broken + ) + + @property + def external_host(self) -> str: + """Return the external host set by Traefik, if any.""" + self._update_stored() + return self._stored.external_host or "" # type: ignore + + @property + def scheme(self) -> str: + """Return the scheme set by Traefik, if any.""" + self._update_stored() + return self._stored.scheme or "" # type: ignore + + def _update_stored(self) -> None: + """Ensure that the stored host is up-to-date. + + This is split out into a separate method since, in the case of multi-unit deployments, + removal of a `TraefikRouteRequirer` will not cause a `RelationEvent`, but the guard on + app data ensures that only the previous leader will know what it is. Separating it + allows for reuse both when the property is called and if the relation changes, so a + leader change where the new leader checks the property will do the right thing. + """ + if not self._charm.unit.is_leader(): + return + + if self._relation: + for relation in self._charm.model.relations[self._relation.name]: + if not relation.app: + self._stored.external_host = "" + self._stored.scheme = "" + return + external_host = relation.data[relation.app].get("external_host", "") + self._stored.external_host = ( + external_host or self._stored.external_host # pyright: ignore + ) + scheme = relation.data[relation.app].get("scheme", "") + self._stored.scheme = scheme or self._stored.scheme # pyright: ignore + + def _on_relation_changed(self, event: RelationEvent) -> None: + """Update StoredState with external_host and other information from Traefik.""" + self._update_stored() + if self._charm.unit.is_leader(): + self.on.ready.emit(event.relation) + + def _on_relation_broken(self, event: RelationEvent) -> None: + """On RelationBroken, clear the stored data if set and emit an event.""" + self._stored.external_host = "" + if self._charm.unit.is_leader(): + self.on.ready.emit(event.relation) + + def is_ready(self) -> bool: + """Is the TraefikRouteRequirer ready to submit data to Traefik?""" + return self._relation is not None + + def submit_to_traefik(self, config): + """Relay an ingress configuration data structure to traefik. + + This will publish to TraefikRoute's traefik-route relation databag + the config traefik needs to route the units behind this charm. + """ + if not self._charm.unit.is_leader(): + raise UnauthorizedError() + + app_databag = self._relation.data[self._charm.app] + + # Traefik thrives on yaml, feels pointless to talk json to Route + app_databag["config"] = yaml.safe_dump(config) diff --git a/metadata.yaml b/metadata.yaml index e85c4c2b..293fa300 100644 --- a/metadata.yaml +++ b/metadata.yaml @@ -38,6 +38,12 @@ requires: limit: 1 description: | Provides traces to COS Tempo instance + internal-ingress: + interface: traefik_route + limit: 1 + description: | + Ingress used for cross-cluster communication where network topology is more complex + than just one k8s cluster peers: kratos-peers: interface: kratos-peers diff --git a/requirements.txt b/requirements.txt index 3352d282..ec514372 100644 --- a/requirements.txt +++ b/requirements.txt @@ -1,7 +1,7 @@ # TODO: Remove when pr https://github.com/go-macaroon-bakery/py-macaroon-bakery/pull/95 gets merged macaroonbakery==1.3.2 cosl==0.0.7 -ops==2.10.0 +ops==2.14.0 lightkube===0.15.0 lightkube-models==1.29.0.6 Jinja2==3.1.3 diff --git a/src/charm.py b/src/charm.py index 43aafd67..ca371856 100755 --- a/src/charm.py +++ b/src/charm.py @@ -49,6 +49,7 @@ IngressPerAppRequirer, IngressPerAppRevokedEvent, ) +from charms.traefik_route_k8s.v0.traefik_route import TraefikRouteRequirer from jinja2 import Template from lightkube import Client from lightkube.resources.apps_v1 import StatefulSet @@ -62,6 +63,7 @@ PebbleReadyEvent, RelationDepartedEvent, RelationEvent, + RelationJoinedEvent, RemoveEvent, UpgradeCharmEvent, ) @@ -81,6 +83,7 @@ import config_map from config_map import IdentitySchemaConfigMap, KratosConfigMap, ProvidersConfigMap +from constants import INTERNAL_INGRESS_RELATION_NAME from kratos import KratosAPI from utils import dict_to_action_output, normalise_url @@ -151,6 +154,15 @@ def __init__(self, *args: Any) -> None: redirect_https=False, ) + # -- ingress via raw traefik_route + # TraefikRouteRequirer expects an existing relation to be passed as part of the constructor, + # so this may be none. Rely on `self.ingress.is_ready` later to check + self.internal_ingress = TraefikRouteRequirer( + self, + self.model.get_relation(INTERNAL_INGRESS_RELATION_NAME), + INTERNAL_INGRESS_RELATION_NAME, + ) # type: ignore + self.database = DatabaseRequires( self, relation_name=self._db_relation_name, @@ -251,6 +263,19 @@ def __init__(self, *args: Any) -> None: self.framework.observe(self.tracing.on.endpoint_changed, self._on_config_changed) self.framework.observe(self.tracing.on.endpoint_removed, self._on_config_changed) + self.framework.observe( + self.on[INTERNAL_INGRESS_RELATION_NAME].relation_joined, self._configure_ingress + ) + self.framework.observe( + self.on[INTERNAL_INGRESS_RELATION_NAME].relation_changed, self._configure_ingress + ) + self.framework.observe( + self.on[INTERNAL_INGRESS_RELATION_NAME].relation_broken, + self._configure_internal_ingress, + ) + self.framework.observe(self.on.leader_elected, self._configure_internal_ingress) + self.framework.observe(self.on.config_changed, self._configure_internal_ingress) + @property def _http_proxy(self) -> str: return self.config["http_proxy"] @@ -343,6 +368,93 @@ def _admin_url(self) -> Optional[str]: url = self.admin_ingress.url return normalise_url(url) if url else None + @property + def _internal_url(self) -> Optional[str]: + host = self.internal_ingress.external_host + return ( + f"{self.internal_ingress.scheme}://{host}/{self.model.name}-{self.model.app.name}" + if host + else None + ) + + @property + def _internal_ingress_config(self) -> dict: + """Build a raw ingress configuration for Traefik.""" + # The path prefix is the same as in ingress per app + external_path = f"{self.model.name}-{self.model.app.name}" + + middlewares = { + f"juju-sidecar-noprefix-{self.model.name}-{self.model.app.name}": { + "stripPrefix": {"forceSlash": False, "prefixes": [f"/{external_path}"]}, + }, + } + + routers = { + f"juju-{self.model.name}-{self.model.app.name}-admin-api-router": { + "entryPoints": ["web"], + "rule": f"PathPrefix(`/{external_path}/admin`)", + "middlewares": list(middlewares.keys()), + "service": f"juju-{self.model.name}-{self.app.name}-admin-api-service", + }, + f"juju-{self.model.name}-{self.model.app.name}-admin-api-router-tls": { + "entryPoints": ["websecure"], + "rule": f"PathPrefix(`/{external_path}/admin`)", + "middlewares": list(middlewares.keys()), + "service": f"juju-{self.model.name}-{self.app.name}-admin-api-service", + "tls": { + "domains": [ + { + "main": self.internal_ingress.external_host, + "sans": [f"*.{self.internal_ingress.external_host}"], + }, + ], + }, + }, + f"juju-{self.model.name}-{self.model.app.name}-public-api-router": { + "entryPoints": ["web"], + "rule": f"PathPrefix(`/{external_path}`)", + "middlewares": list(middlewares.keys()), + "service": f"juju-{self.model.name}-{self.app.name}-public-api-service", + }, + f"juju-{self.model.name}-{self.model.app.name}-public-api-router-tls": { + "entryPoints": ["websecure"], + "rule": f"PathPrefix(`/{external_path}`)", + "middlewares": list(middlewares.keys()), + "service": f"juju-{self.model.name}-{self.app.name}-public-api-service", + "tls": { + "domains": [ + { + "main": self.internal_ingress.external_host, + "sans": [f"*.{self.internal_ingress.external_host}"], + }, + ], + }, + }, + } + + services = { + f"juju-{self.model.name}-{self.app.name}-admin-api-service": { + "loadBalancer": { + "servers": [ + { + "url": f"http://{self.app.name}.{self.model.name}.svc.cluster.local:{KRATOS_ADMIN_PORT}" + } + ] + } + }, + f"juju-{self.model.name}-{self.app.name}-public-api-service": { + "loadBalancer": { + "servers": [ + { + "url": f"http://{self.app.name}.{self.model.name}.svc.cluster.local:{KRATOS_PUBLIC_PORT}" + } + ] + } + }, + } + + return {"http": {"routers": routers, "services": services, "middlewares": middlewares}} + @property def _kratos_endpoints(self) -> Tuple[str, str]: admin_endpoint = ( @@ -1085,6 +1197,28 @@ def _on_run_migration_action(self, event: ActionEvent) -> None: def _promtail_error(self, event: PromtailDigestError) -> None: logger.error(event.message) + def _configure_ingress(self, event: HookEvent) -> None: + """Since :class:`TraefikRouteRequirer` may not have been constructed with an existing + relation if a :class:`RelationJoinedEvent` comes through during the charm lifecycle, if we + get one here, we should recreate it, but OF will give us grief about "two objects claiming + to be ...", so manipulate its private `_relation` variable instead. + + Args: + event: a :class:`HookEvent` to signal a change we may need to respond to. + """ + if not self.unit.is_leader(): + return + + # If it's a RelationJoinedEvent, set it in the ingress object + if isinstance(event, RelationJoinedEvent): + self.internal_ingress._relation = event.relation + + # No matter what, check readiness -- this blindly checks whether `ingress._relation` is not + # None, so it overlaps a little with the above, but works as expected on leader elections + # and config-change + if self.internal_ingress.is_ready(): + self.internal_ingress.submit_to_traefik(self._internal_ingress_config) + if __name__ == "__main__": main(KratosCharm) diff --git a/src/constants.py b/src/constants.py new file mode 100644 index 00000000..6047cffe --- /dev/null +++ b/src/constants.py @@ -0,0 +1,6 @@ +# Copyright 2023 Canonical Ltd. +# See LICENSE file for licensing details. + +"""File contianing all constants""" + +INTERNAL_INGRESS_RELATION_NAME = "internal-ingress" diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py index 9ab87a98..b2c4e1f3 100644 --- a/tests/unit/test_charm.py +++ b/tests/unit/test_charm.py @@ -16,6 +16,8 @@ from ops.pebble import ExecError, TimeoutError from ops.testing import Harness +from constants import INTERNAL_INGRESS_RELATION_NAME + CONFIG_DIR = Path("/etc/config") CONTAINER_NAME = "kratos" ADMIN_PORT = "4434" @@ -67,6 +69,24 @@ def setup_ingress_relation(harness: Harness, type: str) -> int: return relation_id +def setup_internal_ingress_relation(harness: Harness, type: str) -> int: + relation_id = harness.add_relation( + f"{INTERNAL_INGRESS_RELATION_NAME}", + f"{type}-traefik", + ) + harness.add_relation_unit( + relation_id, + f"{type}-traefik/0", + ) + harness.update_relation_data( + relation_id, + f"{type}-traefik", + {"external_host": "test.staging.canonical.com", "scheme": "https"}, + ) + + return relation_id + + def setup_peer_relation(harness: Harness) -> None: rel_id = harness.add_relation("kratos-peers", "kratos") harness.add_relation_unit(rel_id, "kratos/1") @@ -1627,3 +1647,116 @@ def test_on_pebble_ready_correct_plan_with_proxy_flags_when_unset( assert environment["HTTP_PROXY"] == "" assert environment["HTTPS_PROXY"] == "" assert environment["NO_PROXY"] == "" + + +def test_kratos_info_updated_on_internal_ingress_relation_joined(harness: Harness) -> None: + kratos_info_relation_id = setup_kratos_info_relation(harness) + + _ = setup_internal_ingress_relation(harness, "admin") + + ingress_url = f"{harness.charm.internal_ingress.scheme}://{harness.charm.internal_ingress.external_host}/{harness.model.name}-{harness.model.app.name}" + + info_data = harness.get_relation_data(kratos_info_relation_id, harness.charm.app) + + assert info_data["admin_endpoint"] == ingress_url + assert info_data["public_endpoint"] == ingress_url + + +def test_kratos_info_updated_on_internal_ingress_relation_changed(harness: Harness) -> None: + kratos_info_relation_id = setup_kratos_info_relation(harness) + + relation_id = setup_internal_ingress_relation(harness, "admin") + + url_change = "new-test.staging.canonical.com" + harness.update_relation_data( + relation_id, + "admin-traefik", + {"external_host": url_change, "scheme": "https"}, + ) + + ingress_url = f"{harness.charm.internal_ingress.scheme}://{url_change}/{harness.model.name}-{harness.model.app.name}" + + info_data = harness.get_relation_data(kratos_info_relation_id, harness.charm.app) + + assert info_data["admin_endpoint"] == ingress_url + assert info_data["public_endpoint"] == ingress_url + + +def test_kratos_info_updated_on_internal_ingress_relation_broken(harness: Harness) -> None: + kratos_info_relation_id = setup_kratos_info_relation(harness) + + relation_id = setup_internal_ingress_relation(harness, "admin") + + harness.remove_relation(relation_id) + + info_data = harness.get_relation_data(kratos_info_relation_id, harness.charm.app) + + assert info_data["admin_endpoint"] == "http://kratos.kratos-model.svc.cluster.local:4434" + assert info_data["public_endpoint"] == "http://kratos.kratos-model.svc.cluster.local:4433" + + +def test_kratos_endpoint_info_updated_on_internal_ingress_relation_joined( + harness: Harness, +) -> None: + kratos_endpoint_info_relation_id = harness.add_relation( + "kratos-endpoint-info", "identity-platform-login-ui-operator" + ) + harness.add_relation_unit( + kratos_endpoint_info_relation_id, "identity-platform-login-ui-operator/0" + ) + + _ = setup_internal_ingress_relation(harness, "admin") + + ingress_url = f"{harness.charm.internal_ingress.scheme}://{harness.charm.internal_ingress.external_host}/{harness.model.name}-{harness.model.app.name}" + + endpoint_data = harness.get_relation_data(kratos_endpoint_info_relation_id, harness.charm.app) + + assert endpoint_data["admin_endpoint"] == ingress_url + assert endpoint_data["public_endpoint"] == ingress_url + + +def test_kratos_endpoint_info_updated_on_internal_ingress_relation_changed( + harness: Harness, +) -> None: + kratos_endpoint_info_relation_id = harness.add_relation( + "kratos-endpoint-info", "identity-platform-login-ui-operator" + ) + harness.add_relation_unit( + kratos_endpoint_info_relation_id, "identity-platform-login-ui-operator/0" + ) + + relation_id = setup_internal_ingress_relation(harness, "admin") + + url_change = "new-test.staging.canonical.com" + harness.update_relation_data( + relation_id, + "admin-traefik", + {"external_host": url_change, "scheme": "https"}, + ) + + ingress_url = f"{harness.charm.internal_ingress.scheme}://{url_change}/{harness.model.name}-{harness.model.app.name}" + + endpoint_data = harness.get_relation_data(kratos_endpoint_info_relation_id, harness.charm.app) + + assert endpoint_data["admin_endpoint"] == ingress_url + assert endpoint_data["public_endpoint"] == ingress_url + + +def test_kratos_endpoint_info_updated_on_internal_ingress_relation_broken( + harness: Harness, +) -> None: + kratos_endpoint_info_relation_id = harness.add_relation( + "kratos-endpoint-info", "identity-platform-login-ui-operator" + ) + harness.add_relation_unit( + kratos_endpoint_info_relation_id, "identity-platform-login-ui-operator/0" + ) + + relation_id = setup_internal_ingress_relation(harness, "admin") + + harness.remove_relation(relation_id) + + endpoint_data = harness.get_relation_data(kratos_endpoint_info_relation_id, harness.charm.app) + + assert endpoint_data["admin_endpoint"] == "http://kratos.kratos-model.svc.cluster.local:4434" + assert endpoint_data["public_endpoint"] == "http://kratos.kratos-model.svc.cluster.local:4433" From e44716aa8522d1b99a76836b2d612fb15a0eaff8 Mon Sep 17 00:00:00 2001 From: shipperizer Date: Thu, 20 Jun 2024 16:20:56 +0100 Subject: [PATCH 2/2] fix: use internal ingress if set, otherwise stick with k8s networking --- src/charm.py | 27 ++++++++++++--------------- src/constants.py | 2 +- src/utils.py | 3 +++ tests/integration/test_charm.py | 25 +++++++++++++++++-------- 4 files changed, 33 insertions(+), 24 deletions(-) diff --git a/src/charm.py b/src/charm.py index ca371856..f0f5bd92 100755 --- a/src/charm.py +++ b/src/charm.py @@ -264,10 +264,12 @@ def __init__(self, *args: Any) -> None: self.framework.observe(self.tracing.on.endpoint_removed, self._on_config_changed) self.framework.observe( - self.on[INTERNAL_INGRESS_RELATION_NAME].relation_joined, self._configure_ingress + self.on[INTERNAL_INGRESS_RELATION_NAME].relation_joined, + self._configure_internal_ingress, ) self.framework.observe( - self.on[INTERNAL_INGRESS_RELATION_NAME].relation_changed, self._configure_ingress + self.on[INTERNAL_INGRESS_RELATION_NAME].relation_changed, + self._configure_internal_ingress, ) self.framework.observe( self.on[INTERNAL_INGRESS_RELATION_NAME].relation_broken, @@ -363,11 +365,6 @@ def _public_url(self) -> Optional[str]: url = self.public_ingress.url return normalise_url(url) if url else None - @property - def _admin_url(self) -> Optional[str]: - url = self.admin_ingress.url - return normalise_url(url) if url else None - @property def _internal_url(self) -> Optional[str]: host = self.internal_ingress.external_host @@ -458,18 +455,14 @@ def _internal_ingress_config(self) -> dict: @property def _kratos_endpoints(self) -> Tuple[str, str]: admin_endpoint = ( - self._admin_url + self._internal_url or f"http://{self.app.name}.{self.model.name}.svc.cluster.local:{KRATOS_ADMIN_PORT}" ) public_endpoint = ( - self._public_url + self._internal_url or f"http://{self.app.name}.{self.model.name}.svc.cluster.local:{KRATOS_PUBLIC_PORT}" ) - admin_endpoint, public_endpoint = ( - admin_endpoint.replace("https", "http"), - public_endpoint.replace("https", "http"), - ) return admin_endpoint, public_endpoint @property @@ -1197,8 +1190,10 @@ def _on_run_migration_action(self, event: ActionEvent) -> None: def _promtail_error(self, event: PromtailDigestError) -> None: logger.error(event.message) - def _configure_ingress(self, event: HookEvent) -> None: - """Since :class:`TraefikRouteRequirer` may not have been constructed with an existing + def _configure_internal_ingress(self, event: HookEvent) -> None: + """Method setting up the internal networking. + + Since :class:`TraefikRouteRequirer` may not have been constructed with an existing relation if a :class:`RelationJoinedEvent` comes through during the charm lifecycle, if we get one here, we should recreate it, but OF will give us grief about "two objects claiming to be ...", so manipulate its private `_relation` variable instead. @@ -1218,6 +1213,8 @@ def _configure_ingress(self, event: HookEvent) -> None: # and config-change if self.internal_ingress.is_ready(): self.internal_ingress.submit_to_traefik(self._internal_ingress_config) + self._update_kratos_endpoints_relation_data(event) + self._update_kratos_info_relation_data(event) if __name__ == "__main__": diff --git a/src/constants.py b/src/constants.py index 6047cffe..df3a34b7 100644 --- a/src/constants.py +++ b/src/constants.py @@ -1,6 +1,6 @@ # Copyright 2023 Canonical Ltd. # See LICENSE file for licensing details. -"""File contianing all constants""" +"""File containing all constants.""" INTERNAL_INGRESS_RELATION_NAME = "internal-ingress" diff --git a/src/utils.py b/src/utils.py index fc0bb445..155b2231 100644 --- a/src/utils.py +++ b/src/utils.py @@ -51,6 +51,9 @@ def normalise_url(url: str) -> str: """ parsed_url = urlparse(url) + # latest versions of traefik automatically redirect to https if ceritficate relation is + # set, this would void the changes below as even a request to the http url would be redirected + # make sure to disable the certificate relation for the internal ingress or trust that certificate parsed_url = parsed_url._replace(scheme="https") parsed_url = parsed_url._replace(netloc=parsed_url.netloc.rsplit(":", 1)[0]) diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index 26b39c95..78810c4a 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -96,7 +96,7 @@ async def test_ingress_relation(ops_test: OpsTest) -> None: channel="latest/edge", config={"external_hostname": "some_hostname"}, ) - await ops_test.model.integrate(f"{KRATOS_APP}:admin-ingress", TRAEFIK_ADMIN_APP) + await ops_test.model.integrate(f"{KRATOS_APP}:internal-ingress", TRAEFIK_ADMIN_APP) await ops_test.model.integrate(f"{KRATOS_APP}:public-ingress", TRAEFIK_PUBLIC_APP) await ops_test.model.wait_for_idle( @@ -118,15 +118,24 @@ async def test_has_public_ingress(ops_test: OpsTest) -> None: assert resp.status_code == 200 -async def test_has_admin_ingress(ops_test: OpsTest) -> None: +async def test_has_internal_ingress(ops_test: OpsTest) -> None: # Get the traefik address and try to reach kratos - admin_address = await get_unit_address(ops_test, TRAEFIK_ADMIN_APP, 0) - - resp = requests.get( - f"http://{admin_address}/{ops_test.model.name}-{KRATOS_APP}/admin/identities" + internal_address = await get_unit_address(ops_test, TRAEFIK_ADMIN_APP, 0) + + # test admin endpoint + assert ( + requests.get( + f"http://{internal_address}/{ops_test.model.name}-{KRATOS_APP}/admin/identities" + ).status_code + == 200 + ) + # test public endpoint + assert ( + requests.get( + f"http://{internal_address}/{ops_test.model.name}-{KRATOS_APP}/sessions/whoami" + ).status_code + == 401 ) - - assert resp.status_code == 200 @pytest.mark.abort_on_fail