From 81318ad346077370e8a59a9aa57ef848f8a0fd50 Mon Sep 17 00:00:00 2001 From: Daniela Plascencia Date: Fri, 26 Jul 2024 11:48:28 +0200 Subject: [PATCH 1/2] docs: add Limitations section on README (#166) Add a section describing how this charm can only be used as an OIDC client when Dex and Charmed Kubeflow are deployed in the model; otherwise it will fail, as there are multiple parts of this charm's code that are tightly coupled to those. Fixes #159 --- README.md | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 3bbb360..8023425 100644 --- a/README.md +++ b/README.md @@ -10,12 +10,18 @@ This repository hosts the Kubernetes Python Operator for OIDC Gatekeeper The OIDC Gatekeeper Operator may be deployed using the Juju command line as follows ```bash -juju deploy oidc-gatekeeper -juju config oidc-gatekeeper client-secret= public-url=http:// +juju deploy oidc-gatekeeper --trust +juju deploy dex-auth --trust +juju config oidc-gatekeeper client-secret= +juju integrate dex-auth:dex-oidc-config oidc-gatekeeper:dex-oidc-config ``` Upstream documentation can be found at https://github.com/arrikto/oidc-authservice +## Limitations + +This charm has been designed around Charmed Kubeflow and it will not work as an OIDC client outside of a model where `dex-auth` and Charmed Kubeflow are deployed. There are currently no plans to change this behaviour. + ## Looking for a fully supported platform for MLOps? Canonical [Charmed Kubeflow](https://charmed-kubeflow.io) is a state of the art, fully supported MLOps platform that helps data scientists collaborate on AI innovation on any cloud from concept to production, offered by Canonical - the publishers of [Ubuntu](https://ubuntu.com). From 2b39ad08c8e255fcfa7e1359a289d228f4a8ec2a Mon Sep 17 00:00:00 2001 From: Daniela Plascencia Date: Fri, 26 Jul 2024 12:37:19 +0200 Subject: [PATCH 2/2] feat: integrate with dex-oidc-config interface and remove `public-url` (#163) This commit integrates the `oidc-gatekeeper` charm with `dex-auth` via the `dex-oidc-config` interface. `dex-auth` (see https://github.com/canonical/dex-auth-operator/issues/203) recently introduced the aforementioned interface, which will help us remove the `public-url` configuration option as the main way of configuring oidc-gatekeeper's OIDC_AUTH_PROVIDER env variable. The new relation provides all the information that is required for setting that env variable, so this charm and its users should not change it. The changes in this PR include: * Removing the `public-url` configuration option entirely (a backwards compatible solution is not required in this case because dex-auth already has it) -> this tackles #157 * Removing all traces of the public-url from the charm code and test code * Integrating with the new interface and use the relation data to render were needed -> this tackles #156 * Adding the `dex_oidc_config` library to this charm so the interface is handled by it * Adding some extra requirements for the library to work Fixes #156 Fixes #157 --- charmcraft.yaml | 1 + config.yaml | 6 +- lib/charms/dex_auth/v0/dex_oidc_config.py | 373 ++++++++++++++++++++++ metadata.yaml | 2 + requirements-unit.txt | 14 +- requirements.in | 2 + requirements.txt | 12 +- src/charm.py | 53 ++- tests/integration/test_charm.py | 18 +- tests/unit/test_operator.py | 124 ++++--- 10 files changed, 541 insertions(+), 64 deletions(-) create mode 100644 lib/charms/dex_auth/v0/dex_oidc_config.py diff --git a/charmcraft.yaml b/charmcraft.yaml index c9fc650..56fc7ad 100644 --- a/charmcraft.yaml +++ b/charmcraft.yaml @@ -12,3 +12,4 @@ bases: parts: charm: charm-python-packages: [setuptools, pip] + build-packages: [cargo, rustc, pkg-config, libffi-dev, libssl-dev] diff --git a/config.yaml b/config.yaml index b9a8177..a0d8708 100644 --- a/config.yaml +++ b/config.yaml @@ -14,10 +14,6 @@ options: type: string default: '' description: OpenID Connect client secret - public-url: - type: string - default: '' - description: Publicly-accessible endpoint for cluster oidc-scopes: type: string default: 'profile email groups' @@ -41,4 +37,4 @@ options: userid-claim: type: string default: 'email' - description: OpenID Connect claim whose value will be used as the userid. \ No newline at end of file + description: OpenID Connect claim whose value will be used as the userid. diff --git a/lib/charms/dex_auth/v0/dex_oidc_config.py b/lib/charms/dex_auth/v0/dex_oidc_config.py new file mode 100644 index 0000000..8f77a65 --- /dev/null +++ b/lib/charms/dex_auth/v0/dex_oidc_config.py @@ -0,0 +1,373 @@ +#!/usr/bin/env python3 +# Copyright 2024 Canonical Ltd. +# See LICENSE file for licensing details. + +"""Library for sharing Dex's OIDC configuration with OIDC clients. + +This library offers a Python API for providing and requesting information about +Dex's OIDC configuration. +The default relation name is `dex-oidc-config` and it's recommended to use that name, +though if changed, you must ensure to pass the correct name when instantiating the +provider and requirer classes, as well as in `metadata.yaml`. + +## Getting Started + +### Fetching the library with charmcraft + +Using charmcraft you can: +```shell +charmcraft fetch-lib charms.dex_auth.v0.dex_oidc_config + + +## Using the library as requirer + +### Add relation to metadata.yaml +```yaml +requires: + dex-oidc-config: + interface: dex-oidc-config + limit: 1 +``` + +### Instantiate the DexOidcConfigRequirer class in charm.py + +```python +from ops.charm import CharmBase +from charms.dex_auth.v0.dex_oidc_config import DexOidcConfigRequirer, DexOidcConfigRelationError + +class RequirerCharm(CharmBase): + def __init__(self, *args): + self._dex_oidc_config_requirer = DexOidcConfigRequirer(self) + self.framework.observe(self.on.some_event_emitted, self.some_event_function) + self.framework.observe(self._dex_oidc_config_requirer.on.update, self.some_event_function) + + def some_event_function(): + # use the getter function wherever the info is needed + try: + k8s_svc_info_data = self._dex_oidc_config_requirer.get_data() + except DexOidcConfigRelationError as error: + "your error handler goes here" +``` + +## Using the library as provider + +### Add relation to metadata.yaml +```yaml +provides: + dex-oidc-config: + interface: dex-oidc-config +``` + +### Instantiate the DexOidcConfigProvider class in charm.py + +```python +from ops.charm import CharmBase +from charms.dex_auth.v0.dex_oidc_config import DexOidcConfigProvider, DexOidcConfigRelationError + +class ProviderCharm(CharmBase): + def __init__(self, *args, **kwargs): + ... + self._dex_oidc_config_provider = DexOidcConfigProvider(self) + self.observe(self.on.some_event, self._some_event_handler) + + def _some_event_handler(self, ...): + # This will update the relation data bag with the issuer URL + try: + self._dex_oidc_config_provider.send_data(issuer_url) + except DexOidcConfigRelationError as error: + "your error handler goes here" +``` + +Alternatively, if the provider is just broadcasting known data, it can be: + +```python +from ops.charm import CharmBase +from charms.dex_auth.v0.dex_oidc_config import DexOidcConfigProvider, DexOidcConfigRelationError + +class ProviderCharm(CharmBase): + def __init__(self, *args, **kwargs): + ... + self._dex_oidc_config_provider = DexOidcConfigProvider(self) +``` + +## Relation data + +The data shared by this library is: +* issuer-url: the canonical URL for the issuer, OIDC cliets use this to refer to Dex +""" +import logging +from typing import List, Optional, Union + +from ops.charm import CharmBase, RelationEvent +from ops.framework import BoundEvent, EventSource, Object, ObjectEvents +from ops.model import Relation +from pydantic import BaseModel + +# The unique Charmhub library identifier, never change it +LIBID = "eb5a471989b246e4977399bc8cf9ae6f" + +# 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 = 1 + +# Default relation and interface names. If changed, consistency must be kept +# across the provider and requirer. +DEFAULT_RELATION_NAME = "dex-oidc-config" +DEFAULT_INTERFACE_NAME = "dex-oidc-config" +REQUIRED_ATTRIBUTES = ["issuer-url"] + +logger = logging.getLogger(__name__) + + +class DexOidcConfigRelationError(Exception): + """Base exception class for any relation error handled by this library.""" + + pass + + +class DexOidcConfigRelationMissingError(DexOidcConfigRelationError): + """Exception to raise when the relation is missing on either end.""" + + def __init__(self): + self.message = "Missing relation with a Dex OIDC config provider." + super().__init__(self.message) + + +class DexOidcConfigRelationDataMissingError(DexOidcConfigRelationError): + """Exception to raise when there is missing data in the relation data bag.""" + + def __init__(self, message): + self.message = message + super().__init__(self.message) + + +class DexOidcConfigUpdatedEvent(RelationEvent): + """Indicates the Dex OIDC config data was updated.""" + + +class DexOidcConfigEvents(ObjectEvents): + """Events for the Dex OIDC config library.""" + + updated = EventSource(DexOidcConfigUpdatedEvent) + + +class DexOidcConfigObject(BaseModel): + """Representation of a Dex OIDC config object. + + Args: + issuer_url: This is the canonical URL that OIDC clients MUST use to refer to dex. + """ + + issuer_url: str + + +class DexOidcConfigRequirer(Object): + """Implement the Requirer end of the Dex OIDC config relation. + + This library emits: + * DexOidcConfigUpdatedEvent: when data received on the relation is updated. + + Args: + charm (CharmBase): the provider application + refresh_events: (list, optional): list of BoundEvents that this manager should handle. + Use this to update the data sent on this relation on demand. + relation_name (str, optional): the name of the relation + + Attributes: + charm (CharmBase): variable for storing the requirer application + relation_name (str): variable for storing the name of the relation + """ + + on = DexOidcConfigEvents() + + def __init__( + self, + charm: CharmBase, + refresh_events: Optional[List[BoundEvent]] = None, + relation_name: Optional[str] = DEFAULT_RELATION_NAME, + ): + super().__init__(charm, relation_name) + self._charm = charm + self._relation_name = relation_name + self._requirer_wrapper = DexOidcConfigRequirerWrapper(self._charm, self._relation_name) + + self.framework.observe( + self._charm.on[self._relation_name].relation_changed, self._on_relation_changed + ) + + self.framework.observe( + self._charm.on[self._relation_name].relation_broken, self._on_relation_broken + ) + + if refresh_events: + for evt in refresh_events: + self.framework.observe(evt, self._on_relation_changed) + + def get_data(self) -> DexOidcConfigObject: + """Return a DexOidcConfigObject.""" + return self._requirer_wrapper.get_data() + + def _on_relation_changed(self, event: BoundEvent) -> None: + """Handle relation-changed event for this relation.""" + self.on.updated.emit(event.relation) + + def _on_relation_broken(self, event: BoundEvent) -> None: + """Handle relation-broken event for this relation.""" + self.on.updated.emit(event.relation) + + +class DexOidcConfigRequirerWrapper(Object): + """Wrapper for the relation data getting logic. + + Args: + charm (CharmBase): the requirer application + relation_name (str, optional): the name of the relation + + Attributes: + relation_name (str): variable for storing the name of the relation + """ + + def __init__(self, charm, relation_name: Optional[str] = DEFAULT_RELATION_NAME): + super().__init__(charm, relation_name) + self.relation_name = relation_name + + @staticmethod + def _validate_relation(relation: Optional[Relation]) -> None: + """Series of checks for the relation and relation data. + + Args: + relation (optional, Relation): the relation object to run the checks on. + This object must always come from a call of get_relation, which + can either return a Relation object or None. + + Raises: + DexOidcConfigRelationDataMissingError if data is missing or incomplete + DexOidcConfigRelationMissingError: if there is no related application + """ + # Raise if there is no related application + if not relation: + raise DexOidcConfigRelationMissingError() + + # Extract remote app information from relation + remote_app = relation.app + # Get relation data from remote app + relation_data = relation.data[remote_app] + + # Raise if there is no data found in the relation data bag + if not relation_data: + raise DexOidcConfigRelationDataMissingError( + f"No data found in relation {relation.name} data bag." + ) + + def get_data(self) -> DexOidcConfigObject: + """Return a DexOidcConfigObject containing Dex's OIDC configuration. + + Raises: + DexOidcConfigRelationDataMissingError: if data is missing entirely or some attributes + DexOidcConfigRelationMissingError: if there is no related application + ops.model.TooManyRelatedAppsError: if there is more than one related application + """ + # Validate relation data + # Raises TooManyRelatedAppsError if related to more than one app + relation = self.model.get_relation(self.relation_name) + + self._validate_relation(relation=relation) + + # Get relation data from remote app + relation_data = relation.data[relation.app] + + return DexOidcConfigObject(issuer_url=relation_data["issuer-url"]) + + +class DexOidcConfigProvider(Object): + """Implement the Provider end of the Dex OIDC config relation. + + Observes relation events to send data to related applications. + + Args: + charm (CharmBase): the provider application + issuer_url (str): This is the canonical URL that OIDC clients MUST use to refer to dex. + refresh_events: (list, optional): list of BoundEvents that this manager should handle. Use this to update + the data sent on this relation on demand. + relation_name (str, optional): the name of the relation + + Attributes: + charm (CharmBase): variable for storing the provider application + relation_name (str): variable for storing the name of the relation + """ + + def __init__( + self, + charm: CharmBase, + issuer_url: str, + refresh_events: Optional[List[BoundEvent]] = None, + relation_name: Optional[str] = DEFAULT_RELATION_NAME, + ): + super().__init__(charm, relation_name) + self.charm = charm + self.relation_name = relation_name + self._provider_wrapper = DexOidcConfigProviderWrapper(self.charm, self.relation_name) + self._issuer_url = issuer_url + + self.framework.observe(self.charm.on.leader_elected, self._send_data) + + self.framework.observe(self.charm.on[self.relation_name].relation_created, self._send_data) + + if refresh_events: + for evt in refresh_events: + self.framework.observe(evt, self._send_data) + + def _send_data(self, _) -> None: + """Serve as an event handler for sending Dex's OIDC configuration.""" + self._provider_wrapper.send_data(self._issuer_url) + + +class DexOidcConfigProviderWrapper(Object): + """Wrapper for the relation data sending logic. + + Args: + charm (CharmBase): the provider application + relation_name (str, optional): the name of the relation + + Attributes: + charm (CharmBase): variable for storing the provider application + relation_name (str): variable for storing the name of the relation + """ + + def __init__(self, charm: CharmBase, relation_name: Optional[str] = DEFAULT_RELATION_NAME): + super().__init__(charm, relation_name) + self.charm = charm + self.relation_name = relation_name + + def send_data( + self, + issuer_url: str, + ) -> None: + """Update the relation data bag with data from Dex's OIDC configuration. + + This method will complete successfully even if there are no related applications. + + Args: + issuer_url (str): This is the canonical URL that OIDC clients MUST use to refer to dex. + """ + # Validate unit is leader to send data; otherwise return + if not self.charm.model.unit.is_leader(): + logger.info( + "DexOidcConfigProvider handled send_data event when it is not the leader." + "Skipping event - no data sent." + ) + return + + # Update the relation data bag with Dex's OIDC configuration + relations = self.charm.model.relations[self.relation_name] + + # Update relation data + for relation in relations: + relation.data[self.charm.app].update( + { + "issuer-url": issuer_url, + } + ) diff --git a/metadata.yaml b/metadata.yaml index 26f0eb1..af583d9 100755 --- a/metadata.yaml +++ b/metadata.yaml @@ -52,6 +52,8 @@ provides: versions: [v1] __schema_source: https://raw.githubusercontent.com/canonical/operator-schemas/oidc-schemas/oidc-client.yaml requires: + dex-oidc-config: + interface: dex-oidc-config ingress: interface: ingress schema: diff --git a/requirements-unit.txt b/requirements-unit.txt index a25071e..addb2f1 100644 --- a/requirements-unit.txt +++ b/requirements-unit.txt @@ -4,6 +4,8 @@ # # pip-compile requirements-unit.in # +annotated-types==0.7.0 + # via pydantic anyio==3.7.1 # via httpcore attrs==23.1.0 @@ -69,6 +71,10 @@ pkgutil-resolve-name==1.3.10 # via jsonschema pluggy==1.2.0 # via pytest +pydantic==2.8.2 + # via -r requirements.in +pydantic-core==2.20.1 + # via pydantic pyrsistent==0.19.3 # via jsonschema pytest==7.4.0 @@ -91,7 +97,7 @@ requests==2.31.0 # via serialized-data-interface ruamel-yaml==0.17.32 # via charmed-kubeflow-chisme -ruamel-yaml-clib==0.2.8 +ruamel-yaml-clib==0.2.7 # via ruamel-yaml serialized-data-interface==0.3.6 # via @@ -107,7 +113,11 @@ tenacity==8.2.2 tomli==2.0.1 # via pytest typing-extensions==4.12.2 - # via cosl + # via + # annotated-types + # cosl + # pydantic + # pydantic-core urllib3==2.0.4 # via requests websocket-client==1.6.1 diff --git a/requirements.in b/requirements.in index 3e565bd..7adbcb6 100644 --- a/requirements.in +++ b/requirements.in @@ -2,6 +2,8 @@ # See LICENSE file for licensing details. ops oci-image +# required by dex-oidc-config library +pydantic # oidc-gatekeeper's current implementation is not compatible # with SDI>0.3. serialized-data-interface<0.4 diff --git a/requirements.txt b/requirements.txt index 0f84f32..27ff869 100644 --- a/requirements.txt +++ b/requirements.txt @@ -4,6 +4,8 @@ # # pip-compile requirements.in # +annotated-types==0.7.0 + # via pydantic anyio==3.7.1 # via httpcore attrs==23.1.0 @@ -58,6 +60,10 @@ ordered-set==4.1.0 # via deepdiff pkgutil-resolve-name==1.3.10 # via jsonschema +pydantic==2.8.2 + # via -r requirements.in +pydantic-core==2.20.1 + # via pydantic pyrsistent==0.19.3 # via jsonschema pyyaml==6.0.1 @@ -84,7 +90,11 @@ sniffio==1.3.0 tenacity==8.2.2 # via charmed-kubeflow-chisme typing-extensions==4.12.2 - # via cosl + # via + # annotated-types + # cosl + # pydantic + # pydantic-core urllib3==2.0.4 # via requests websocket-client==1.6.1 diff --git a/src/charm.py b/src/charm.py index 0e7ee15..57fb59d 100755 --- a/src/charm.py +++ b/src/charm.py @@ -8,6 +8,11 @@ from charmed_kubeflow_chisme.exceptions import ErrorWithStatus from charmed_kubeflow_chisme.pebble import update_layer +from charms.dex_auth.v0.dex_oidc_config import ( + DexOidcConfigRelationDataMissingError, + DexOidcConfigRelationMissingError, + DexOidcConfigRequirer, +) from charms.loki_k8s.v1.loki_push_api import LogForwarder from charms.observability_libs.v1.kubernetes_service_patch import KubernetesServicePatch from lightkube.models.core_v1 import ServicePort @@ -17,6 +22,8 @@ from ops.pebble import Layer from serialized_data_interface import NoCompatibleVersions, NoVersionsListed, get_interfaces +OIDC_PROVIDER_INFO_RELATION = "dex-oidc-config" + class OIDCGatekeeperOperator(CharmBase): """Charm OIDC Gatekeeper Operator.""" @@ -30,6 +37,10 @@ def __init__(self, *args): self._container_name = "oidc-authservice" self._container = self.unit.get_container(self._container_name) self.pebble_service_name = "oidc-authservice" + self._dex_oidc_config_requirer = DexOidcConfigRequirer( + charm=self, + relation_name=OIDC_PROVIDER_INFO_RELATION, + ) http_service_port = ServicePort(self._http_port, name="http-port") self.service_patcher = KubernetesServicePatch( @@ -37,10 +48,6 @@ def __init__(self, *args): [http_service_port], ) - self.public_url = self.model.config["public-url"] - if not self.public_url.startswith(("http://", "https://")): - self.public_url = f"http://{self.public_url}" - for event in [ self.on.start, self.on.leader_elected, @@ -51,6 +58,9 @@ def __init__(self, *args): self.on["ingress-auth"].relation_changed, self.on["oidc-client"].relation_changed, self.on["client-secret"].relation_changed, + self.on[OIDC_PROVIDER_INFO_RELATION].relation_changed, + self.on[OIDC_PROVIDER_INFO_RELATION].relation_broken, + self._dex_oidc_config_requirer.on.updated, ]: self.framework.observe(event, self.main) @@ -58,8 +68,8 @@ def __init__(self, *args): def main(self, event): try: - self._check_public_url() self._check_leader() + self._check_dex_oidc_config_relation() interfaces = self._get_interfaces() secret_key = self._check_secret() self._send_info(interfaces, secret_key) @@ -72,13 +82,34 @@ def main(self, event): self.model.unit.status = ActiveStatus() + def _check_dex_oidc_config_relation(self) -> None: + """Check for exceptions from the library and raises ErrorWithStatus to set the unit status. + + Raises: + ErrorWithStatus: if the relation hasn't been established, set unit to BlockedStatus + ErrorWithStatus: if the relation has empty or missing data, set unit to WaitingStatus + """ + try: + self._dex_oidc_config_requirer.get_data() + except DexOidcConfigRelationMissingError as rel_error: + raise ErrorWithStatus( + f"{rel_error.message} Please add the missing relation.", BlockedStatus + ) + except DexOidcConfigRelationDataMissingError as data_error: + self.logger.error(f"Empty or missing data. Got: {data_error.message}") + raise ErrorWithStatus( + f"Empty or missing data in {OIDC_PROVIDER_INFO_RELATION} relation." + " This may be transient, but if it persists it is likely an error.", + WaitingStatus, + ) + @property def service_environment(self): """Return environment variables based on model configuration.""" secret_key = self._check_secret() skip_urls = self.model.config["skip-auth-urls"] or "" dex_skip_urls = "/dex/" if not skip_urls else "/dex/," + skip_urls - + oidc_provider = self._dex_oidc_config_requirer.get_data().issuer_url ret_env_vars = { "AFTER_LOGIN_URL": "/", "AFTER_LOGOUT_URL": "/", @@ -87,7 +118,7 @@ def service_environment(self): "CLIENT_SECRET": secret_key, "DISABLE_USERINFO": True, "OIDC_AUTH_URL": "/dex/auth", - "OIDC_PROVIDER": f"{self.public_url}/dex", + "OIDC_PROVIDER": oidc_provider, "OIDC_SCOPES": self.model.config["oidc-scopes"], "SERVER_PORT": self._http_port, "USERID_CLAIM": self.model.config["userid-claim"], @@ -146,11 +177,6 @@ def _get_interfaces(self): raise ErrorWithStatus(str(err), BlockedStatus) return interfaces - def _check_public_url(self): - """Check if `public-url` config is set.""" - if not self.model.config.get("public-url"): - raise ErrorWithStatus("public-url config required", BlockedStatus) - def _configure_mesh(self, interfaces): """Update ingress and ingress-auth relations with mesh info.""" if interfaces["ingress"]: @@ -179,9 +205,6 @@ def _send_info(self, interfaces, secret_key): """Send info to oidc-client relation.""" config = self.model.config - if not config.get("public-url"): - return False - if interfaces["oidc-client"]: interfaces["oidc-client"].send_data( { diff --git a/tests/integration/test_charm.py b/tests/integration/test_charm.py index f67dc28..455e5ac 100644 --- a/tests/integration/test_charm.py +++ b/tests/integration/test_charm.py @@ -57,7 +57,15 @@ async def test_deploy(self, ops_test: OpsTest): config=OIDC_CONFIG, ) - await ops_test.model.applications[APP_NAME].set_config({"public-url": PUBLIC_URL}) + # Deploying dex-auth is a hard requirement for this charm as + # a dex-oidc-config requirer; otherwise it will block + await ops_test.model.deploy(DEX_AUTH, channel=DEX_AUTH_CHANNEL, trust=DEX_AUTH_TRUST) + await ops_test.model.wait_for_idle( + apps=[DEX_AUTH], status="active", raise_on_blocked=False, timeout=60 * 10 + ) + await ops_test.model.integrate( + f"{APP_NAME}:dex-oidc-config", f"{DEX_AUTH}:dex-oidc-config" + ) await ops_test.model.wait_for_idle( apps=[APP_NAME], status="active", raise_on_blocked=False, timeout=60 * 10 @@ -81,14 +89,11 @@ async def test_relations(self, ops_test: OpsTest): channel=ISTIO_PILOT_CHANNEL, trust=ISTIO_PILOT_TRUST, ) - await ops_test.model.deploy(DEX_AUTH, channel=DEX_AUTH_CHANNEL, trust=DEX_AUTH_TRUST) await ops_test.model.integrate(ISTIO_PILOT, DEX_AUTH) await ops_test.model.integrate(f"{ISTIO_PILOT}:ingress", f"{APP_NAME}:ingress") await ops_test.model.integrate(f"{ISTIO_PILOT}:ingress-auth", f"{APP_NAME}:ingress-auth") await ops_test.model.integrate(f"{APP_NAME}:oidc-client", f"{DEX_AUTH}:oidc-client") - await ops_test.model.applications[DEX_AUTH].set_config({"public-url": PUBLIC_URL}) - # Not raising on blocked will allow istio-pilot to be deployed # without istio-gateway and provide oidc with the data it needs. await ops_test.model.wait_for_idle( @@ -125,7 +130,10 @@ async def test_upgrade(self, ops_test: OpsTest): await ops_test.model.integrate(f"{ISTIO_PILOT}:ingress", f"{APP_NAME}:ingress") await ops_test.model.integrate(f"{ISTIO_PILOT}:ingress-auth", f"{APP_NAME}:ingress-auth") await ops_test.model.integrate(f"{APP_NAME}:oidc-client", f"{DEX_AUTH}:oidc-client") - await ops_test.model.applications[APP_NAME].set_config({"public-url": PUBLIC_URL}) + + # TODO: remove after releasing ckf-1.9/stable, this has been preserved to avoid breaking + # integration tests. + await ops_test.model.applications[APP_NAME].set_config({"public-url": "http://foo.io"}) print("Stable charm is deployed, add relations") await ops_test.model.wait_for_idle( diff --git a/tests/unit/test_operator.py b/tests/unit/test_operator.py index c2c561a..eebd152 100644 --- a/tests/unit/test_operator.py +++ b/tests/unit/test_operator.py @@ -4,7 +4,13 @@ import pytest import yaml -from ops.model import ActiveStatus, WaitingStatus +from charmed_kubeflow_chisme.exceptions import ErrorWithStatus +from charms.dex_auth.v0.dex_oidc_config import ( + DexOidcConfigRelationDataMissingError, + DexOidcConfigRelationMissingError, + DexOidcConfigRequirer, +) +from ops.model import ActiveStatus, BlockedStatus, WaitingStatus from ops.testing import Harness from charm import OIDCGatekeeperOperator @@ -25,7 +31,6 @@ def test_log_forwarding(harness): @patch("charm.KubernetesServicePatch", lambda x, y: None) def test_not_leader(harness): - harness.update_config({"public-url": "10.64.140.43.nip.io"}) harness.begin_with_initial_hooks() assert harness.charm.model.unit.status == WaitingStatus("Waiting for leadership") @@ -33,7 +38,9 @@ def test_not_leader(harness): @patch("charm.KubernetesServicePatch", lambda x, y: None) def test_no_relation(harness): harness.set_leader(True) - harness.update_config({"public-url": "10.64.140.43.nip.io"}) + # Add dex-oidc-config relation by default; otherwise charm will block + harness.add_relation("dex-oidc-config", "app", app_data={"issuer-url": "http://dex.io/dex"}) + harness.add_oci_resource( "oci-image", { @@ -50,7 +57,10 @@ def test_no_relation(harness): @patch("charm.KubernetesServicePatch", lambda x, y: None) def test_with_relation(harness): harness.set_leader(True) - harness.update_config({"public-url": "10.64.140.43.nip.io"}) + + # Add dex-oidc-config relation by default; otherwise charm will block + harness.add_relation("dex-oidc-config", "app", app_data={"issuer-url": "http://dex.io/dex"}) + rel_id = harness.add_relation("ingress", "app") harness.add_relation_unit(rel_id, "app/0") @@ -65,37 +75,14 @@ def test_with_relation(harness): assert isinstance(harness.charm.model.unit.status, ActiveStatus) -@pytest.mark.parametrize( - "url_prefix,url_result", - [ - ( - "", - "http://", - ), - ("https://", "https://"), - ("http://", "http://"), - ], -) -@patch("charm.KubernetesServicePatch", lambda x, y: None) -def test_public_url(harness, url_prefix, url_result): - harness.set_leader(True) - harness.update_config({"public-url": f"{url_prefix}10.64.140.43.nip.io"}) - harness.begin_with_initial_hooks() - - plan = harness.get_container_pebble_plan("oidc-authservice") - - assert "OIDC_PROVIDER" in plan.services["oidc-authservice"].environment - assert ( - plan.services["oidc-authservice"].environment["OIDC_PROVIDER"] - == f"{url_result}10.64.140.43.nip.io/dex" - ) - - @patch("charm.KubernetesServicePatch", lambda x, y: None) def test_skip_auth_url_config_has_value(harness): harness.set_leader(True) - harness.update_config({"public-url": "10.64.140.43.nip.io"}) harness.update_config({"skip-auth-urls": "/test/,/path1/"}) + + # Add dex-oidc-config relation by default; otherwise charm will block + harness.add_relation("dex-oidc-config", "app", app_data={"issuer-url": "http://dex.io/dex"}) + harness.begin_with_initial_hooks() plan = harness.get_container_pebble_plan("oidc-authservice") @@ -108,7 +95,9 @@ def test_skip_auth_url_config_has_value(harness): @patch("charm.KubernetesServicePatch", lambda x, y: None) def test_skip_auth_url_config_is_empty(harness): harness.set_leader(True) - harness.update_config({"public-url": "10.64.140.43.nip.io"}) + # Add dex-oidc-config relation by default; otherwise charm will block + harness.add_relation("dex-oidc-config", "app", app_data={"issuer-url": "http://dex.io/dex"}) + harness.begin_with_initial_hooks() plan = harness.get_container_pebble_plan("oidc-authservice") @@ -119,8 +108,10 @@ def test_skip_auth_url_config_is_empty(harness): @patch("charm.KubernetesServicePatch", lambda x, y: None) def test_ca_bundle_config(harness): harness.set_leader(True) - harness.update_config({"public-url": "10.64.140.43.nip.io"}) harness.update_config({"ca-bundle": "aaa"}) + # Add dex-oidc-config relation by default; otherwise charm will block + harness.add_relation("dex-oidc-config", "app", app_data={"issuer-url": "http://dex.io/dex"}) + harness.begin_with_initial_hooks() plan = harness.get_container_pebble_plan("oidc-authservice") @@ -133,7 +124,9 @@ def test_ca_bundle_config(harness): @patch("charm.KubernetesServicePatch", lambda x, y: None) def test_session_store(harness): harness.set_leader(True) - harness.update_config({"public-url": "10.64.140.43.nip.io"}) + # Add dex-oidc-config relation by default; otherwise charm will block + harness.add_relation("dex-oidc-config", "app", app_data={"issuer-url": "http://dex.io/dex"}) + harness.begin_with_initial_hooks() plan = harness.get_container_pebble_plan("oidc-authservice") @@ -146,15 +139,17 @@ def test_session_store(harness): ) -@patch("charm.KubernetesServicePatch", lambda x, y: None) @patch("charm.update_layer", MagicMock()) +@patch("charm.KubernetesServicePatch", lambda x, y: None) def test_pebble_ready_hook_handled(harness: Harness): """ Test if we handle oidc_authservice_pebble_ready hook. This test fails if we don't. """ harness.set_leader(True) + # Add dex-oidc-config relation by default; otherwise charm will block + harness.add_relation("dex-oidc-config", "app", app_data={"issuer-url": "http://dex.io/dex"}) + harness.begin() - harness.charm._check_public_url = MagicMock() harness.charm._get_interfaces = MagicMock() harness.charm._check_secret = MagicMock() harness.charm._send_info = MagicMock() @@ -163,3 +158,60 @@ def test_pebble_ready_hook_handled(harness: Harness): harness.charm.on.oidc_authservice_pebble_ready.emit(harness.charm) assert isinstance(harness.charm.model.unit.status, ActiveStatus) + + +@patch("charm.KubernetesServicePatch", lambda x, y: None) +def test_charm_blocks_on_missing_dex_oidc_config_relation(harness): + """Test the charm goes into BlockedStatus when the relation is missing.""" + harness.set_leader(True) + harness.add_oci_resource( + "oci-image", + { + "registrypath": "ci-test", + "username": "", + "password": "", + }, + ) + harness.begin_with_initial_hooks() + + assert isinstance(harness.charm.model.unit.status, BlockedStatus) + assert ( + "Missing relation with a Dex OIDC config provider" + in harness.charm.model.unit.status.message + ) + + +@patch("charm.KubernetesServicePatch", lambda x, y: None) +def test_service_environment_uses_data_from_relation(harness): + """Test the service_environment property has the correct values set by the relation data.""" + harness.set_leader(True) + # Add the client-secret peer relation as it is required to render the service environment + harness.add_relation("client-secret", harness.model.app.name) + + expected_oidc_provider = "http://dex.io/dex" + harness.add_relation("dex-oidc-config", "app", app_data={"issuer-url": expected_oidc_provider}) + + harness.begin() + + service_environment = harness.charm.service_environment + assert service_environment["OIDC_PROVIDER"] == expected_oidc_provider + + +@patch("charm.KubernetesServicePatch", lambda x, y: None) +@pytest.mark.parametrize( + "expected_raise, expected_status", + ( + (DexOidcConfigRelationMissingError, BlockedStatus), + (DexOidcConfigRelationDataMissingError("Empty or missing data"), WaitingStatus), + ), +) +@patch.object(DexOidcConfigRequirer, "get_data") +def test_check_dex_oidc_config_relation(mocked_get_data, expected_raise, expected_status, harness): + """Verify the method raises ErrorWithStatus with correct status.""" + harness.begin() + mocked_get_data.side_effect = expected_raise + with pytest.raises(ErrorWithStatus) as raised_exception: + harness.charm._check_dex_oidc_config_relation() + + # We can only check what status is sent to the main handler, which is the one setting it + assert raised_exception.value.status_type == expected_status