Subdomain external hostname

src/charm.py

@@ -159,20 +159,6 @@ class TraefikIngressCharm(CharmBase):
     def __init__(self, *args):
         super().__init__(*args)
 
-        # Before doing anything, validate the charm config.  If configuration is invalid, warn the user.
-        # FIXME: Invalid configuration here SHOULD halt the charm's operation until resolution, or at least make a
-        #  persistent BlockedStatus, but this charm handles events atomically rather than holistically.
-        #  This means that skipping events will result in unexpected issues, so if we halt the charm here we must
-        #  ensure the charm processes all backlogged events on resumption in the original order.  Rather than do that
-        #  and risk losing an event, we simply warn the user and continue functioning as best as possible.  The charm
-        #  will operate correctly except that it will not publish ingress urls to related applications, instead
-        #  leaving the ingress relation data empty and logging an error.
-        #  If we refactor this charm to handle events holistically (and thus we can skip events without issue), we
-        #  should refactor this validation truly halt the charm.
-        #  If we refactor this charm to use collect_unit_status, we could raise a persistent BlockedStatus message when
-        #  this configuration is invalid.
-        self._validate_config()
-
         self._stored.set_default(
             config_hash=None,
         )
@@ -711,6 +697,13 @@ def _process_status_and_configurations(self):
             )
             return
 
+        if routing_mode == "subdomain" and self.config.get("external_hostname", None) is None:
+            self._wipe_ingress_for_all_relations()
+            self.unit.status = BlockedStatus(
+                '"external_hostname" must be set while using routing mode "subdomain"'
+            )
+            return
+
         hostname = self._external_host
 
         if not hostname:
@@ -1290,29 +1283,6 @@ def server_cert_sans_dns(self) -> List[str]:
         # If all else fails, we'd rather use the bare IP
         return [target] if target else []
 
-    def _validate_config(self):
-        """Validate the charm configuration, emitting warning messages on misconfigurations.
-
-        In scope for this validation is:
-        * validating the combination of external_hostname and routing_mode
-        """
-        # FIXME: This will false positive in cases where the LoadBalancer provides an external host rather than an IP.
-        #  The warning will occur, but the charm will function normally.  We could better validate the LoadBalancer if
-        #  we want to avoid this, but it probably isn't worth the effort until someone notices.
-        invalid_hostname_and_routing_mode_message = (
-            "Likely configuration error: When using routing_mode=='subdomain', external_hostname should be "
-            "set.  This is because when external_hostname is unset, Traefik uses the LoadBalancer's address as the "
-            "hostname for all provided URLS and that hostname is typically an IP address.  This leads to invalid urls "
-            "like `model-app.1.2.3.4`.  The charm will continue to operate as currently set, but will not provide urls"
-            " to any related applications if they would be invalid."
-        )
-
-        if self.config.get("routing_mode", "") == "subdomain":
-            # subdomain mode can only be used if an external_hostname is set and is not an IP address
-            external_hostname = self.config.get("external_hostname", "")
-            if not isinstance(external_hostname, str) or not is_valid_hostname(external_hostname):
-                logger.warning(invalid_hostname_and_routing_mode_message)
-
 
 def is_valid_hostname(hostname: str) -> bool:
     """Check if a hostname is valid.

tests/scenario/test_ingress_per_unit.py

@@ -1,8 +1,6 @@
 import pytest
 from scenario import Relation, State
 
-from tests.scenario.conftest import MOCK_LB_ADDRESS
-
 
 @pytest.mark.parametrize("leader", (True, False))
 @pytest.mark.parametrize("url", ("url.com", "http://foo.bar.baz"))
@@ -35,7 +33,7 @@ def test_ingress_unit_provider_request_response(
     state = State(
         relations=[ipu],
         leader=leader,
-        config={"routing_mode": routing_mode},
+        config={"routing_mode": routing_mode, "external_hostname": "example.com"},
         containers=[traefik_container],
     )
 
@@ -48,12 +46,12 @@ def test_ingress_unit_provider_request_response(
         assert not local_app_data
     else:
         if mode == "tcp":
-            expected_url = f"{MOCK_LB_ADDRESS}:{port}"
+            expected_url = f"example.com:{port}"
         else:
             prefix = f"{model}-{remote_unit_name.replace('/', '-')}"
             if routing_mode == "path":
-                expected_url = f"http://{MOCK_LB_ADDRESS}/{prefix}"
+                expected_url = f"http://example.com/{prefix}"
             else:
-                expected_url = f"http://{prefix}.{MOCK_LB_ADDRESS}/"
+                expected_url = f"http://{prefix}.example.com/"
 
         assert local_app_data == {"ingress": f"{remote_unit_name}:\n  url: {expected_url}\n"}

tests/scenario/test_status.py

@@ -37,6 +37,22 @@ def test_start_traefik_no_hostname(traefik_ctx, *_):
     )
 
 
+@patch("charm.TraefikIngressCharm._external_host", PropertyMock(return_value="1.1.1.1"))
+def test_start_traefik_subdomain_without_hostname(traefik_ctx, *_):
+    # GIVEN external_hostname is not set but routing_mode is set to subdomain
+    # WHEN a `start` hook fires
+    state = State(
+        config={"routing_mode": "subdomain"},
+        containers=[Container(name="traefik", can_connect=True)],
+    )
+    out = traefik_ctx.run("start", state)
+
+    # THEN unit status is `waiting`
+    assert out.unit_status == BlockedStatus(
+        '"external_hostname" must be set while using routing mode "subdomain"'
+    )
+
+
 @patch("charm.TraefikIngressCharm._external_host", PropertyMock(return_value="foo.bar"))
 @patch("traefik.Traefik.is_ready", PropertyMock(return_value=True))
 @patch("charm.TraefikIngressCharm._static_config_changed", PropertyMock(return_value=False))