feat: add csr-domain-name config option (#381)

feat: enable csr-domain-name config option so istio-pilot can use it on CSRs

The istio-pilot charm already has a mechanism in place to discover the ingress gateway address from the `Service`, but it is limited to only returning IP addresses, which not all TLS certificate providers accept as a valid cert subject. Having the domain-name config option will allow users to specify the domain name they'd like to use when integrating with TLS certificate operators. This feature expands the support for integrating with TLS certificate providers that cannot issue signed certificates on a CSR that only contains an IP address (like we used to do).
This commit also adds some test coverage to test the recently added code.

Fixes #379

charms/istio-pilot/config.yaml

@@ -1,4 +1,9 @@
 options:
+  csr-domain-name:
+    default: ''
+    type: string
+    description: |
+      The domain name to be used by the charm to send a Certificate Signing Request (CSR) to a TLS certificate provider. In the absence of this configuration option, the charm will try to use the ingress gateway service hostname (if configured by a LB) or its IP address.
   default-gateway:
     type: string
     default: istio-gateway

charms/istio-pilot/src/charm.py

@@ -381,16 +381,29 @@ def _check_leader(self):
             raise ErrorWithStatus("Waiting for leadership", WaitingStatus)
 
     @property
-    def _cert_subject(self):
-        """Return the domain to be used in the CSR."""
+    def _cert_subject(self) -> Optional[str]:
+        """Return the certificate subject to be used in the CSR.
+
+        If the csr-domain-name configuration option is set, this value is used;
+        otherwise, use the IP address or hostname of the actual ingress gateway service.
+        Lastly, if for any reason the service address cannot be retrieved, None will be returned.
+        """
+        # Prioritise the csr-domain-name config option
+        if csr_domain_name := self.model.config["csr-domain-name"]:
+            return csr_domain_name
+
+        # Get the ingress gateway service address
         try:
             svc = self._get_gateway_service()
         except ApiError:
-            self.log.info("Could not retrieve the gateway service for configuring the CSR.")
+            self.log.info("Could not retrieve the gateway service address for using in the CSR.")
             return None
-        gateway_address = _get_gateway_address_from_svc(svc)
-        if gateway_address:
-            return gateway_address
+
+        svc_address = _get_gateway_address_from_svc(svc)
+
+        # NOTE: returning None here means that the CSR will have the unit name as cert subject
+        # this is an implementation detail of the cert_hanlder library.
+        return svc_address if svc_address else None
 
     @property
     def _gateway_port(self):
@@ -811,6 +824,8 @@ def _get_gateway_address_from_svc(svc):
 
     if svc.spec.type == "ClusterIP":
         gateway_address = svc.spec.clusterIP
+    elif svc.spec.type == "NodePort":
+        gateway_address = svc.spec.clusterIP
     elif svc.spec.type == "LoadBalancer":
         gateway_address = _get_address_from_loadbalancer(svc)
 

charms/istio-pilot/tests/unit/test_charm.py

@@ -445,6 +445,52 @@ def test_reconcile_not_leader(
         harness.charm.reconcile("mock event")
         assert harness.charm.model.unit.status == WaitingStatus("Waiting for leadership")
 
+    @pytest.mark.parametrize(
+        "mock_service_fixture, gateway_address",
+        [
+            # Pass fixtures by their names
+            ("mock_nodeport_service", "10.10.10.10"),
+            ("mock_clusterip_service", "10.10.10.11"),
+            ("mock_loadbalancer_hostname_service", "test.com"),
+            ("mock_loadbalancer_ip_service", "127.0.0.1"),
+        ],
+    )
+    def test_cert_subject_returns_no_config(
+        self,
+        mock_service_fixture,
+        gateway_address,
+        harness,
+        mocked_lightkube_client,
+        request,
+    ):
+        """Assert the property returns the ingress gateway address."""
+        harness.begin()
+
+        mock_get_gateway_service = MagicMock(
+            return_value=request.getfixturevalue(mock_service_fixture)
+        )
+
+        harness.charm._get_gateway_service = mock_get_gateway_service
+
+        assert harness.charm._cert_subject == gateway_address
+
+    def test_cert_subject_returns_with_config(self, harness, mocked_lightkube_client):
+        """Assert the property returns the domain name config value when set."""
+        harness.begin()
+
+        expected_domain_name = "test.com"
+        harness.update_config(
+            {
+                "csr-domain-name": expected_domain_name,
+            }
+        )
+        assert harness.charm._cert_subject == expected_domain_name
+
+    def test_cert_subject_none(self, harness, mocked_lightkube_client):
+        """Assert returns None when no csr-domain-name/gateway service address is set in place."""
+        harness.begin()
+        assert harness.charm._cert_subject is None
+
     @pytest.mark.parametrize(
         "cert_handler_enabled, ssl_cert, ssl_key, expected_port, expected_context",
         [
@@ -536,8 +582,8 @@ def test_is_gateway_service_up(
         "mock_service_fixture, gateway_address",
         [
             # Pass fixtures by their names
-            ("mock_nodeport_service", None),
-            ("mock_clusterip_service", "10.10.10.10"),
+            ("mock_nodeport_service", "10.10.10.10"),
+            ("mock_clusterip_service", "10.10.10.11"),
             ("mock_loadbalancer_hostname_service", "test.com"),
             ("mock_loadbalancer_ip_service", "127.0.0.1"),
             ("mock_loadbalancer_hostname_service_not_ready", None),
@@ -1378,7 +1424,7 @@ def mock_clusterip_service():
             "apiVersion": "v1",
             "kind": "Service",
             "status": {"loadBalancer": {"ingress": [{}]}},
-            "spec": {"type": "ClusterIP", "clusterIP": "10.10.10.10"},
+            "spec": {"type": "ClusterIP", "clusterIP": "10.10.10.11"},
         }
     )
     return mock_nodeport_service
@@ -1391,7 +1437,7 @@ def mock_loadbalancer_ip_service():
             "apiVersion": "v1",
             "kind": "Service",
             "status": {"loadBalancer": {"ingress": [{"ip": "127.0.0.1"}]}},
-            "spec": {"type": "LoadBalancer", "clusterIP": "10.10.10.10"},
+            "spec": {"type": "LoadBalancer", "clusterIP": "10.10.10.12"},
         }
     )
     return mock_nodeport_service
@@ -1404,7 +1450,7 @@ def mock_loadbalancer_hostname_service():
             "apiVersion": "v1",
             "kind": "Service",
             "status": {"loadBalancer": {"ingress": [{"hostname": "test.com"}]}},
-            "spec": {"type": "LoadBalancer", "clusterIP": "10.10.10.10"},
+            "spec": {"type": "LoadBalancer", "clusterIP": "10.10.10.12"},
         }
     )
     return mock_nodeport_service