Merge pull request #386 from canonical/dnplas-cherry-pick-6ad839d

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

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