Merge ~mthaddon/charm-k8s-ingress/+git/charm-k8s-ingress:hostname-over-relation into charm-k8s-ingress:master

[Tom Haddon]

If service-hostname is passed via the relation, use it. Also allow other properties to be passed via the relation.

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[🤖 prod-jenkaas-is (prod-jenkaas-is)]

A CI job is currently in progress. A follow up comment will be added when it completes.

[🤖 prod-jenkaas-is (prod-jenkaas-is)]

PASSED: Continuous integration, rev:f1731b33b23bc9f1ef64a398f8324870c5bce4f2
https://jenkins.canonical.com/is/job/lp-charm-k8s-ingress-ci/13/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/is/job/lp-charm-test/43/
    None: https://jenkins.canonical.com/is/job/lp-update-mp/57088/

Click here to trigger a rebuild:
https://jenkins.canonical.com/is/job/lp-charm-k8s-ingress-ci/13//rebuild

review: Approve (continuous-integration)

[🤖 prod-jenkaas-is (prod-jenkaas-is)]

A CI job is currently in progress. A follow up comment will be added when it completes.

[🤖 prod-jenkaas-is (prod-jenkaas-is)]

PASSED: Continuous integration, rev:f11595ecfdd3f4209e1f914e55ed80ff116ea50f
https://jenkins.canonical.com/is/job/lp-charm-k8s-ingress-ci/14/
Executed test runs:
    SUCCESS: https://jenkins.canonical.com/is/job/lp-charm-test/44/
    None: https://jenkins.canonical.com/is/job/lp-update-mp/57138/

Click here to trigger a rebuild:
https://jenkins.canonical.com/is/job/lp-charm-k8s-ingress-ci/14//rebuild

review: Approve (continuous-integration)

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision ca4cc16e34018af2f1782e2b974483ba59be68da

diff --git a/src/charm.py b/src/charm.py
index a206ba7..146ddc7 100755
--- a/src/charm.py
+++ b/src/charm.py
@@ -103,6 +103,14 @@ class CharmK8SIngressCharm(CharmBase):
         self._on_config_changed(event)
 
     @property
+    def _k8s_service_name(self):
+        """Return a service name for the use creating a k8s service."""
+        # Avoid collision with service name created by Juju. Currently
+        # Juju creates a K8s service listening on port 65535/TCP so we
+        # need to create a separate one.
+        return "{}-service".format(self._service_name)
+
+    @property
     def _ingress_name(self):
         """Return an ingress name for use creating a k8s ingress."""
         # Follow the same naming convention as Juju.
@@ -111,6 +119,12 @@ class CharmK8SIngressCharm(CharmBase):
         )
 
     @property
+    def _max_body_size(self):
+        """Return the max-body-size to use for k8s ingress."""
+        max_body_size = self._stored.ingress_relation_data.get("max-body-size") or self.config["max-body-size"]
+        return "{}m".format(max_body_size)
+
+    @property
     def _namespace(self):
         """Return the namespace to operate on."""
         return (
@@ -120,12 +134,9 @@ class CharmK8SIngressCharm(CharmBase):
         )
 
     @property
-    def _k8s_service_name(self):
-        """Return a service name for the use creating a k8s service."""
-        # Avoid collision with service name created by Juju. Currently
-        # Juju creates a K8s service listening on port 65535/TCP so we
-        # need to create a separate one.
-        return "{}-service".format(self._service_name)
+    def _service_hostname(self):
+        """Return the hostname for the service we're connecting to."""
+        return self._stored.ingress_relation_data.get("service-hostname") or self.config["service-hostname"]
 
     @property
     def _service_name(self):
@@ -139,6 +150,22 @@ class CharmK8SIngressCharm(CharmBase):
             return int(self._stored.ingress_relation_data.get("service-port"))
         return self.config["service-port"]
 
+    @property
+    def _session_cookie_max_age(self):
+        """Return the session-cookie-max-age to use for k8s ingress."""
+        session_cookie_max_age = (
+            self._stored.ingress_relation_data.get("session-cookie-max-age") or self.config["session-cookie-max-age"]
+        )
+        if session_cookie_max_age:
+            return str(session_cookie_max_age)
+        # Don't return "0" which would evaluate to True.
+        return ""
+
+    @property
+    def _tls_secret_name(self):
+        """Return the tls-secret-name to use for k8s ingress (if any)."""
+        return self._stored.ingress_relation_data.get("tls-secret-name") or self.config["tls-secret-name"]
+
     def k8s_auth(self):
         """Authenticate to kubernetes."""
         if self._authed:
@@ -179,7 +206,7 @@ class CharmK8SIngressCharm(CharmBase):
         spec = kubernetes.client.NetworkingV1beta1IngressSpec(
             rules=[
                 kubernetes.client.NetworkingV1beta1IngressRule(
-                    host=self.config["service-hostname"],
+                    host=self._service_hostname,
                     http=kubernetes.client.NetworkingV1beta1HTTPIngressRuleValue(
                         paths=[
                             kubernetes.client.NetworkingV1beta1HTTPIngressPath(
@@ -196,24 +223,21 @@ class CharmK8SIngressCharm(CharmBase):
         )
         annotations = {
             "nginx.ingress.kubernetes.io/rewrite-target": "/",
-            "nginx.ingress.kubernetes.io/proxy-body-size": "{}m".format(self.config["max-body-size"]),
+            "nginx.ingress.kubernetes.io/proxy-body-size": self._max_body_size,
         }
-        if self.config["session-cookie-max-age"]:
+        if self._session_cookie_max_age:
             annotations["nginx.ingress.kubernetes.io/affinity"] = "cookie"
             annotations["nginx.ingress.kubernetes.io/affinity-mode"] = "balanced"
             annotations["nginx.ingress.kubernetes.io/session-cookie-change-on-failure"] = "true"
-            annotations["nginx.ingress.kubernetes.io/session-cookie-max-age"] = "{}".format(
-                self.config["session-cookie-max-age"]
-            )
+            annotations["nginx.ingress.kubernetes.io/session-cookie-max-age"] = self._session_cookie_max_age
             annotations["nginx.ingress.kubernetes.io/session-cookie-name"] = "{}_AFFINITY".format(
                 self._service_name.upper()
             )
             annotations["nginx.ingress.kubernetes.io/session-cookie-samesite"] = "Lax"
-        tls_secret_name = self.config["tls-secret-name"]
-        if tls_secret_name:
+        if self._tls_secret_name:
             spec.tls = kubernetes.client.NetworkingV1beta1IngressTLS(
-                hosts=[self.config["service-hostname"]],
-                secret_name=tls_secret_name,
+                hosts=[self._service_hostname],
+                secret_name=self._tls_secret_name,
             )
         else:
             annotations["nginx.ingress.kubernetes.io/ssl-redirect"] = "false"
diff --git a/tests/unit/test_charm.py b/tests/unit/test_charm.py
index 3efb784..1496048 100644
--- a/tests/unit/test_charm.py
+++ b/tests/unit/test_charm.py
@@ -58,6 +58,16 @@ class TestCharm(unittest.TestCase):
         # Confirm status is as expected.
         self.assertEqual(self.harness.charm.unit.status, ActiveStatus())
 
+    def test_max_body_size(self):
+        """Test for the max-body-size property."""
+        # First set via config.
+        self.harness.update_config({"max-body-size": 80})
+        self.assertEqual(self.harness.charm._max_body_size, "80m")
+        # Now set via the StoredState. This will be set to a string, as all
+        # relation data must be a string.
+        self.harness.charm._stored.ingress_relation_data["max-body-size"] = "88"
+        self.assertEqual(self.harness.charm._max_body_size, "88m")
+
     def test_namespace(self):
         """Test for the namespace property."""
         # If charm config and _stored is empty, use model name.
@@ -81,6 +91,39 @@ class TestCharm(unittest.TestCase):
         self.harness.charm._stored.ingress_relation_data["service-port"] = "88"
         self.assertEqual(self.harness.charm._service_port, 88)
 
+    def test_service_hostname(self):
+        """Test the service-hostname property."""
+        # First set via config.
+        self.harness.update_config({"service-hostname": "foo.internal"})
+        self.assertEqual(self.harness.charm._service_hostname, "foo.internal")
+        # Now set via the StoredState. This will be set to a string, as all
+        # relation data must be a string.
+        self.harness.charm._stored.ingress_relation_data["service-hostname"] = "foo-bar.internal"
+        self.assertEqual(self.harness.charm._service_hostname, "foo-bar.internal")
+
+    def test_session_cookie_max_age(self):
+        """Test the session-cookie-max-age property."""
+        # First set via config.
+        self.harness.update_config({"session-cookie-max-age": 3600})
+        self.assertEqual(self.harness.charm._session_cookie_max_age, "3600")
+        # Confirm if we set this to 0 we get a False value, e.g. it doesn't
+        # return a string of "0" which would be evaluated to True.
+        self.harness.update_config({"session-cookie-max-age": 0})
+        self.assertFalse(self.harness.charm._session_cookie_max_age)
+        # Now set via the StoredState. This will be set to a string, as all
+        # relation data must be a string.
+        self.harness.charm._stored.ingress_relation_data["session-cookie-max-age"] = "3688"
+        self.assertEqual(self.harness.charm._session_cookie_max_age, "3688")
+
+    def test_tls_secret_name(self):
+        """Test the tls-secret-name property."""
+        self.harness.update_config({"tls-secret-name": "gunicorn-tls"})
+        self.assertEqual(self.harness.charm._tls_secret_name, "gunicorn-tls")
+        # Now set via the StoredState. This will be set to a string, as all
+        # relation data must be a string.
+        self.harness.charm._stored.ingress_relation_data["tls-secret-name"] = "gunicorn-tls-new"
+        self.assertEqual(self.harness.charm._tls_secret_name, "gunicorn-tls-new")
+
     @patch('charm.CharmK8SIngressCharm._on_config_changed')
     def test_on_ingress_relation_changed(self, _on_config_changed):
         """Test ingress relation changed handler."""
@@ -139,7 +182,7 @@ class TestCharm(unittest.TestCase):
     def test_get_k8s_ingress(self):
         """Test getting our definition of a k8s ingress."""
         self.harness.disable_hooks()
-        self.harness.update_config({"service-name": "gunicorn", "service-port": 80, "service-hostname": "foo.internal"})
+        self.harness.update_config({"service-hostname": "foo.internal", "service-name": "gunicorn", "service-port": 80})
         expected = kubernetes.client.NetworkingV1beta1Ingress(
             api_version="networking.k8s.io/v1beta1",
             kind="Ingress",