raise error on duplicate ports passed to patch

lib/charms/observability_libs/v1/kubernetes_service_patch.py

@@ -146,11 +146,15 @@ def setUp(self, *unused):
 
 # Increment this PATCH version before using `charmcraft publish-lib` or reset
 # to 0 if you are raising the major API version
-LIBPATCH = 9
+LIBPATCH = 10
 
 ServiceType = Literal["ClusterIP", "LoadBalancer"]
 
 
+class DuplicatePortsError(Exception):
+    """Raised if the user attempts to pass to the patch duplicate ports."""
+
+
 class KubernetesServicePatch(Object):
     """A utility for patching the Kubernetes service set up by Juju."""
 
@@ -165,6 +169,7 @@ def __init__(
         additional_annotations: Optional[dict] = None,
         *,
         refresh_event: Optional[Union[BoundEvent, List[BoundEvent]]] = None,
+        key: str = "kubernetes-service-patch"
     ):
         """Constructor for KubernetesServicePatch.
 
@@ -184,7 +189,7 @@ def __init__(
                 will be observed to re-apply the patch (e.g. on port change).
                 The `install` and `upgrade-charm` events would be observed regardless.
         """
-        super().__init__(charm, "kubernetes-service-patch")
+        super().__init__(charm, key)
         self.charm = charm
         self.service_name = service_name if service_name else self._app
         self.service = self._service_object(
@@ -237,6 +242,15 @@ def _service_object(
         Returns:
             Service: A valid representation of a Kubernetes Service with the correct ports.
         """
+        # check for duplicate ports
+        ports_with_dupes = list((p.port, p.protocol) for p in ports)
+        ports_dedup = set((p.port, p.protocol) for p in ports)
+        for port in ports_dedup:
+            ports_with_dupes.remove(port)
+
+        if ports_with_dupes:
+            raise DuplicatePortsError(f"duplicate ports: {ports_with_dupes}")
+
         if not service_name:
             service_name = self._app
         labels = {"app.kubernetes.io/name": self._app}

tests/unit/test_kubernetes_service.py

@@ -5,7 +5,7 @@
 from unittest import mock
 from unittest.mock import Mock, mock_open, patch
 
-from charms.observability_libs.v1.kubernetes_service_patch import KubernetesServicePatch
+from charms.observability_libs.v1.kubernetes_service_patch import KubernetesServicePatch, DuplicatePortsError
 from lightkube import ApiError
 from lightkube.models.core_v1 import ServicePort, ServiceSpec
 from lightkube.models.meta_v1 import ObjectMeta
@@ -361,6 +361,18 @@ def test_protocols(self):
 
         self.assertEqual(service_patch.service, expected_service)
 
+    def test_patch_raises_on_duplicate_ports(self):
+        """Check that patch raises if duplicate ports are requested."""
+        with self.assertRaises(DuplicatePortsError):
+            KubernetesServicePatch(
+                self.harness.charm,
+                ports=[
+                    ServicePort(name="svc1", port=1234, protocol="TCP"),
+                    ServicePort(name="svc2", port=1234, protocol="TCP"),
+                ],
+                key="foo"
+            )
+
     def test_custom_event_is_fired(self):
         """Check that events provided via refresh_event are called."""
         charm = self.harness.charm
@@ -500,3 +512,4 @@ def test_is_patched_k8s_service_api_error_not_404(self, client):
             with self.assertRaises(_FakeApiError):
                 charm.custom_service_name_service_patch.is_patched()
             self.assertIn("Kubernetes service get failed: broken", ";".join(logs.output))
+