From 182afb6bccc637d3646ac248f9c7a8dc73f3c0d8 Mon Sep 17 00:00:00 2001 From: Pietro Pasotti Date: Mon, 12 Feb 2024 17:46:14 +0100 Subject: [PATCH] raise error on duplicate ports passed to patch --- .../v1/kubernetes_service_patch.py | 18 ++++++++++++++++-- tests/unit/test_kubernetes_service.py | 15 ++++++++++++++- 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/lib/charms/observability_libs/v1/kubernetes_service_patch.py b/lib/charms/observability_libs/v1/kubernetes_service_patch.py index 2cce729..2105f0a 100644 --- a/lib/charms/observability_libs/v1/kubernetes_service_patch.py +++ b/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} diff --git a/tests/unit/test_kubernetes_service.py b/tests/unit/test_kubernetes_service.py index 1d11990..fa2621d 100644 --- a/tests/unit/test_kubernetes_service.py +++ b/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)) +