From bbe443e1f97b44af0ba317d768c67e0aab98f690 Mon Sep 17 00:00:00 2001 From: shipperizer Date: Thu, 20 Jun 2024 16:20:56 +0100 Subject: [PATCH] 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 fea05c5f..a445d0dc 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