Validate service for needed NodePort and unsupported option spec.allo…

…cateLoadBalancerNodePorts=false
stackitcloud · Jul 18, 2024 · cfd6a8e · cfd6a8e
1 parent ed39268
commit cfd6a8e
Show file tree

Hide file tree

Showing 3 changed files with 102 additions and 0 deletions.
diff --git a/controllers/yawol-cloud-controller/targetcontroller/service_controller_test.go b/controllers/yawol-cloud-controller/targetcontroller/service_controller_test.go
@@ -472,6 +472,99 @@ var _ = Describe("Check loadbalancer reconcile", Serial, Ordered, func() {
 			}, time.Second*5, time.Millisecond*500).Should(Succeed())
 		})
 
+		It("create service with unsupported option", func() {
+			By("create service")
+			service := v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "service-test35",
+					Namespace: "default"},
+				Spec: v1.ServiceSpec{
+					AllocateLoadBalancerNodePorts: ptr.To(false),
+					Ports: []v1.ServicePort{
+						{
+							Name:       "port1",
+							Protocol:   v1.ProtocolSCTP,
+							Port:       65000,
+							TargetPort: intstr.IntOrString{IntVal: 12345},
+						},
+					},
+					Type: "LoadBalancer",
+				}}
+			Expect(k8sClient.Create(ctx, &service)).Should(Succeed())
+
+			By("check for protocol in ports")
+			Consistently(func() error {
+				err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test35", Namespace: "default"}, &lb)
+				if err != nil {
+					return client.IgnoreNotFound(err)
+				}
+				return helper.ErrUnsupportedServiceOption
+			}, time.Second*5, time.Millisecond*500).Should(Succeed())
+
+			By("check for event on service")
+			Eventually(func() error {
+				eventList := v1.EventList{}
+				err := k8sClient.List(ctx, &eventList)
+				if err != nil {
+					return err
+				}
+				for _, event := range eventList.Items {
+					if event.InvolvedObject.Name == "service-test35" &&
+						event.InvolvedObject.Kind == "Service" &&
+						strings.Contains(event.Message, "unsupported service option") {
+						return nil
+					}
+				}
+				return helper.ErrNoEventFound
+			}, time.Second*5, time.Millisecond*500).Should(Succeed())
+		})
+
+		It("create service with without node port", func() {
+			By("create service")
+			service := v1.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "service-test36",
+					Namespace: "default"},
+				Spec: v1.ServiceSpec{
+					Ports: []v1.ServicePort{
+						{
+							Name:       "port1",
+							Protocol:   v1.ProtocolSCTP,
+							Port:       65000,
+							TargetPort: intstr.IntOrString{IntVal: 12345},
+						},
+					},
+					Type: "LoadBalancer",
+				}}
+			Expect(k8sClient.Create(ctx, &service)).Should(Succeed())
+
+			By("check for protocol in ports")
+			Consistently(func() error {
+				err := k8sClient.Get(ctx, types.NamespacedName{Name: "default--service-test36", Namespace: "default"}, &lb)
+				if err != nil {
+					return client.IgnoreNotFound(err)
+				}
+				return helper.ErrNodePortInvalidRange
+			}, time.Second*5, time.Millisecond*500).Should(Succeed())
+
+			By("check for event on service")
+			Eventually(func() error {
+				eventList := v1.EventList{}
+				err := k8sClient.List(ctx, &eventList)
+				if err != nil {
+					return err
+				}
+				for _, event := range eventList.Items {
+					if event.InvolvedObject.Name == "service-test3" &&
+						event.InvolvedObject.Kind == "Service" &&
+						strings.Contains(event.Message, "NodePort is not between 1 and 65535") {
+						return nil
+					}
+				}
+				return helper.ErrNoEventFound
+			}, time.Second*5, time.Millisecond*500).Should(Succeed())
+		})
+
 		It("create service with wrong className in annotation", func() {
 			By("create service")
 			service := v1.Service{

diff --git a/internal/helper/errors.go b/internal/helper/errors.go
@@ -65,6 +65,7 @@ var (
 	ErrCouldNotParseSourceRange              = errors.New("could not parse LoadBalancerSourceRange")
 	ErrListingChildLBMs                      = errors.New("unable to list child loadbalancerMachines")
 	ErrUnsupportedProtocol                   = errors.New("unsupported protocol used (TCP and UDP is supported)")
+	ErrUnsupportedServiceOption              = errors.New("unsupported service option is used")
 	ErrProjectIsImmutable                    = errors.New("project id is immutable, cant be changed after initial creation")
 	ErrNoLBMFoundForScaleDown                = errors.New("no lbm found for scale down")
 )
diff --git a/internal/helper/service.go b/internal/helper/service.go
@@ -183,13 +183,21 @@ func CheckLoadBalancerClasses(service *coreV1.Service, validClasses []string) bo
 
 // ValidateService checks if the service is valid
 func ValidateService(svc *coreV1.Service) error {
+	if svc.Spec.AllocateLoadBalancerNodePorts != nil &&
+		!*svc.Spec.AllocateLoadBalancerNodePorts {
+		return fmt.Errorf("%w: %v)", ErrUnsupportedServiceOption, "spec.allocateLoadBalancerNodePorts=false is not supported")
+	}
 	for _, port := range svc.Spec.Ports {
 		switch port.Protocol {
 		case coreV1.ProtocolTCP:
 		case coreV1.ProtocolUDP:
 		default:
 			return fmt.Errorf("%w: %v)", ErrUnsupportedProtocol, port.Protocol)
 		}
+
+		if port.NodePort > 65535 || port.NodePort < 1 {
+			return ErrNodePortInvalidRange
+		}
 	}
 	return nil
 }