refactor: add dex-issuer-url and remove public-url config options (#209)

* refactor: add dex-issuer-url and remove public-url config options

This commit removes the public-url configuration option in favour of the dex-issuer-url one.
The way to configure the issuer value for dex-auth is now by getting it from the aforementioned
configuration option or by constructing it from dex-auths Kubernetes Service DNS name:
"http://<dex-app-name>.<namespace>.svc:5556/dex"

Closes #204

charmcraft.yaml

@@ -14,4 +14,4 @@ parts:
     charm-python-packages: [setuptools, pip]
     # Install rustc and cargo as build packages because some charm's
     # dependencies need this to be built and installed from source.
-    build-packages: [rustc, cargo]
+    build-packages: [cargo, rustc, pkg-config, libffi-dev, libssl-dev]

config.yaml

@@ -6,14 +6,22 @@ options:
     type: boolean
     default: true
     description: Allows dex to keep a list of passwords which can be used to login to dex
+  issuer-url:
+    type: string
+    default: ''
+    description: |
+      Format http(s)://<publicly-accessible-dns-name>/dex
+      (Also referred to as issuer or OIDC provider ) This is the canonical URL that OIDC clients
+      MUST use to refer to dex. If not specified, it defaults to dex-auth's local
+      endpoint constructed from dex-auth's Kubernetes Service DNS name, the
+      Service port and Dex's endpoint, that is http://<dex-auth-app-name>.<namespace>.svc:5556/dex.
+      The default is set by the charm code, not the configuration option.
+      This configuration must be set when using a Dex connector that will try to reach Dex from outside
+      the cluster, thus it should be a publicly accessible endpoint, for example https://my-instance.in-my-cloud.some-cloud.com/dex
   port:
     type: int
     default: 5556
     description: Listening port
-  public-url:
-    type: string
-    default: ''
-    description: Publicly-accessible endpoint for cluster
   connectors:
     type: string
     default: ''

metadata.yaml

@@ -81,6 +81,8 @@ provides:
     interface: prometheus_scrape
   grafana-dashboard:
     interface: grafana_dashboard
+  dex-oidc-config:
+    interface: dex-oidc-config
 containers:
   dex:
     resource: oci-image

src/charm.py

@@ -10,6 +10,7 @@
 import bcrypt
 import yaml
 from charmed_kubeflow_chisme.exceptions import ErrorWithStatus
+from charms.dex_auth.v0.dex_oidc_config import DexOidcConfigProvider
 from charms.grafana_k8s.v0.grafana_dashboard import GrafanaDashboardProvider
 from charms.loki_k8s.v1.loki_push_api import LogForwarder
 from charms.observability_libs.v1.kubernetes_service_patch import KubernetesServicePatch
@@ -33,11 +34,15 @@ def __init__(self, *args):
         super().__init__(*args)
 
         self.logger: logging.Logger = logging.getLogger(__name__)
+        self._namespace = self.model.name
 
         # Patch the service to correctly expose the ports to be used
         dex_port = ServicePort(int(self.model.config["port"]), name="dex")
         metrics_port = ServicePort(int(METRICS_PORT), name="metrics-port")
 
+        # Instantiate a DexOidcConfigProvider to share this app's issuer_url
+        self.dex_oidc_config_provider = DexOidcConfigProvider(self, issuer_url=self._issuer_url)
+
         self.service_patcher = KubernetesServicePatch(
             self,
             [dex_port, metrics_port],
@@ -58,7 +63,6 @@ def __init__(self, *args):
         self._logging = LogForwarder(charm=self)
 
         self._container_name = "dex"
-        self._namespace = self.model.name
         self._container = self.unit.get_container(self._container_name)
         self._entrypoint = "/usr/local/bin/docker-entrypoint"
         self._dex_config_path = "/etc/dex/config.docker.yaml"
@@ -95,6 +99,15 @@ def _dex_auth_layer(self) -> Layer:
         }
         return Layer(layer_config)
 
+    @property
+    def _issuer_url(self) -> str:
+        """Return issuer-url value if config option exists; otherwise default Dex's endpoint."""
+        if issuer_url := self.model.config["issuer-url"]:
+            return issuer_url
+        return (
+            f"http://{self.model.app.name}.{self._namespace}.svc:{self.model.config['port']}/dex"
+        )
+
     def _update_layer(self) -> None:
         """Updates the Pebble configuration layer if changed."""
         self._check_container_connection()
@@ -192,9 +205,6 @@ def _generate_dex_auth_config(self) -> str:
         # Load config values as convenient variables
         connectors = yaml.safe_load(self.model.config["connectors"])
         port = self.model.config["port"]
-        public_url = self.model.config["public-url"].lower()
-        if not public_url.startswith(("http://", "https://")):
-            public_url = f"http://{public_url}"
 
         enable_password_db = self.model.config["enable-password-db"]
         static_config = {
@@ -227,7 +237,7 @@ def _generate_dex_auth_config(self) -> str:
 
         config = yaml.dump(
             {
-                "issuer": f"{public_url}/dex",
+                "issuer": self._issuer_url,
                 "storage": {"type": "kubernetes", "config": {"inCluster": True}},
                 "web": {"http": f"0.0.0.0:{port}"},
                 "telemetry": {"http": "0.0.0.0:5558"},

tests/integration/test_charm.py

@@ -155,16 +155,13 @@ async def test_relations(ops_test: OpsTest):
     await ops_test.model.add_relation(KUBEFLOW_PROFILES, KUBEFLOW_DASHBOARD)
     await ops_test.model.add_relation(f"{ISTIO_PILOT}:ingress", f"{KUBEFLOW_DASHBOARD}:ingress")
 
-    # Set public-url for dex and oidc
-    # Note: This could be affected by a race condition (if service has not received
-    # an IP yet, this could fail) but probably this won't happen in practice
-    # because that IP is delivered quickly and we wait_for_idle on the istio_gateway
-    public_url = get_public_url(
-        service_name="istio-ingressgateway-workload",
-        namespace=ops_test.model_name,
-    )
-    log.info(f"got public_url of {public_url}")
-    await ops_test.model.applications[DEX_AUTH_APP_NAME].set_config({"public-url": public_url})
+    # Set public-url for oidc
+    # Leaving the default value of Dex's Kubernetes Service temporarily
+    # FIXME: remove this configuration option after canonical/oidc-gatekeeper-operator/pull/164 and
+    # canonical/oidc-gatekeeper-operator/pull/163 get merged
+    # After that, we should integrate dex-auth and oidc-gatekeeper.
+    # See canonical/oidc-gatekeeper-operator#157 for more details
+    public_url = f"http://{DEX_AUTH_APP_NAME}.{ops_test.model_name}.svc:5556"
     await ops_test.model.applications[OIDC_GATEKEEPER].set_config({"public-url": public_url})
 
     await ops_test.model.wait_for_idle(

tests/unit/test_charm.py

@@ -11,6 +11,8 @@
 
 from charm import Operator
 
+DEX_OIDC_CONFIG_RELATION = "dex-oidc-config"
+
 
 @pytest.fixture
 def harness():
@@ -69,7 +71,6 @@ def test_generate_dex_auth_config_raises(harness):
     config_updates = {
         "enable-password-db": False,
         "port": 5555,
-        "public-url": "dummy.url",
     }
 
     harness.update_config(config_updates)
@@ -89,13 +90,11 @@ def test_generate_dex_auth_config_raises(harness):
         {
             "enable-password-db": False,
             "port": 5555,
-            "public-url": "dummy.url",
             "connectors": "test-connector",
         },
         {
             "enable-password-db": True,
             "port": 5555,
-            "public-url": "dummy.url",
             "static-username": "new-user",
             "static-password": "new-pass",
         },
@@ -141,7 +140,6 @@ def test_disable_static_login_no_connector_blocked_status(harness):
     config_updates = {
         "enable-password-db": False,
         "port": 5555,
-        "public-url": "dummy.url",
     }
 
     harness.update_config(config_updates)
@@ -156,8 +154,8 @@ def test_config_changed(update, harness):
 
     config_updates = {
         "enable-password-db": False,
+        "issuer-url": "http://my-dex.io/dex",
         "port": 5555,
-        "public-url": "dummy.url",
         "connectors": "connector01",
         "static-username": "new-user",
         "static-password": "new-pass",
@@ -219,3 +217,47 @@ def test_main_ingress(update, harness):
     }
 
     assert data == yaml.safe_load(relation_data["data"])
+
+
+@patch("charm.KubernetesServicePatch", lambda *_, **__: None)
+def test_dex_oidc_config_with_data(harness):
+    """Test the relation data has values by default as the charm is broadcasting them."""
+    harness.set_leader(True)
+    harness.begin()
+
+    rel_id = harness.add_relation(DEX_OIDC_CONFIG_RELATION, "app")
+    rel_data = harness.model.get_relation(DEX_OIDC_CONFIG_RELATION, rel_id).data[harness.model.app]
+
+    # Default values are expected
+    expected = {"issuer-url": f"http://{harness.model.app.name}.{harness.model.name}.svc:5556/dex"}
+    assert rel_data["issuer-url"] == expected["issuer-url"]
+
+
+@patch("charm.KubernetesServicePatch", lambda *_, **__: None)
+def test_grpc_relation_with_data_when_data_changes(
+    harness,
+):
+    """Test the relation data on config changed events."""
+    harness.set_leader(True)
+    # Change the configuration option before starting harness so
+    # the correct values are passed to the DexOidcConfig lib
+    # FIXME: the correct behaviour should be to change the config
+    # at any point in time to trigger config events and check the
+    # value gets passed correctly when it changes.
+    harness.update_config({"issuer-url": "http://my-dex.io/dex"})
+    harness.begin()
+
+    # Initialise a dex-oidc-config requirer charm
+    #    harness.charm.leadership_gate.get_status = MagicMock(return_value=ActiveStatus())
+    #    harness.charm.kubernetes_resources.get_status = MagicMock(return_value=ActiveStatus())
+
+    # Add relation between the requirer charm and this charm (dex-auth)
+    provider_rel_id = harness.add_relation(
+        relation_name=DEX_OIDC_CONFIG_RELATION, remote_app="other-app"
+    )
+    provider_rel_data = harness.get_relation_data(
+        relation_id=provider_rel_id, app_or_unit=harness.charm.app.name
+    )
+
+    # Change the port of the service and check the value changes
+    assert provider_rel_data["issuer-url"] == harness.model.config["issuer-url"]