From e8a2de1b3657a1404bcd35524f23c3563d47eb44 Mon Sep 17 00:00:00 2001
From: Leon <leon@apecloud.com>
Date: Mon, 20 Jan 2025 15:18:47 +0800
Subject: [PATCH] fix: NPE when adding a new service port (#8820)

---
 controllers/apps/cluster/utils.go   | 32 ++++++++++++++---------------
 controllers/apps/component/utils.go | 32 ++++++++++++++---------------
 2 files changed, 30 insertions(+), 34 deletions(-)

diff --git a/controllers/apps/cluster/utils.go b/controllers/apps/cluster/utils.go
index 9491387e2ad..ccce65f5291 100644
--- a/controllers/apps/cluster/utils.go
+++ b/controllers/apps/cluster/utils.go
@@ -96,28 +96,26 @@ func clientOption(v *model.ObjectVertex) *multicluster.ClientOption {
 }
 
 func resolveServiceDefaultFields(oldSpec, newSpec *corev1.ServiceSpec) {
-	var exist *corev1.ServicePort
+	servicePorts := make(map[int32]corev1.ServicePort)
+	for i, port := range oldSpec.Ports {
+		servicePorts[port.Port] = oldSpec.Ports[i]
+	}
 	for i, port := range newSpec.Ports {
-		for _, oldPort := range oldSpec.Ports {
-			// assume that port.Name is user specified, if it is not changed, we need to keep the old NodePort and TargetPort if they are not set
-			if port.Name != "" && port.Name == oldPort.Name {
-				exist = &oldPort
-				break
-			}
-		}
-		if exist == nil {
-			continue
+		servicePort, ok := servicePorts[port.Port]
+		if !ok {
+			continue // new port added
 		}
 		// if the service type is NodePort or LoadBalancer, and the nodeport is not set, we should use the nodeport of the exist service
-		if shouldAllocateNodePorts(newSpec) && port.NodePort == 0 && exist.NodePort != 0 {
-			newSpec.Ports[i].NodePort = exist.NodePort
-			port.NodePort = exist.NodePort
+		if shouldAllocateNodePorts(newSpec) && port.NodePort == 0 && servicePort.NodePort != 0 {
+			port.NodePort = servicePort.NodePort
+			newSpec.Ports[i].NodePort = servicePort.NodePort
 		}
-		if port.TargetPort.IntVal == 0 && port.TargetPort.StrVal == "" {
-			port.TargetPort = exist.TargetPort
+		if port.TargetPort.IntVal != 0 {
+			continue
 		}
-		if reflect.DeepEqual(port, *exist) {
-			newSpec.Ports[i].TargetPort = exist.TargetPort
+		port.TargetPort = servicePort.TargetPort
+		if reflect.DeepEqual(port, servicePort) {
+			newSpec.Ports[i].TargetPort = servicePort.TargetPort
 		}
 	}
 	if len(newSpec.ClusterIP) == 0 {
diff --git a/controllers/apps/component/utils.go b/controllers/apps/component/utils.go
index 7b5ec0746d4..f06bc66cd1c 100644
--- a/controllers/apps/component/utils.go
+++ b/controllers/apps/component/utils.go
@@ -103,28 +103,26 @@ func clientOption(v *model.ObjectVertex) *multicluster.ClientOption {
 }
 
 func resolveServiceDefaultFields(oldSpec, newSpec *corev1.ServiceSpec) {
-	var exist *corev1.ServicePort
+	servicePorts := make(map[int32]corev1.ServicePort)
+	for i, port := range oldSpec.Ports {
+		servicePorts[port.Port] = oldSpec.Ports[i]
+	}
 	for i, port := range newSpec.Ports {
-		for _, oldPort := range oldSpec.Ports {
-			// assume that port.Name is user specified, if it is not changed, we need to keep the old NodePort and TargetPort if they are not set
-			if port.Name != "" && port.Name == oldPort.Name {
-				exist = &oldPort
-				break
-			}
-		}
-		if exist == nil {
-			continue
+		servicePort, ok := servicePorts[port.Port]
+		if !ok {
+			continue // new port added
 		}
 		// if the service type is NodePort or LoadBalancer, and the nodeport is not set, we should use the nodeport of the exist service
-		if shouldAllocateNodePorts(newSpec) && port.NodePort == 0 && exist.NodePort != 0 {
-			newSpec.Ports[i].NodePort = exist.NodePort
-			port.NodePort = exist.NodePort
+		if shouldAllocateNodePorts(newSpec) && port.NodePort == 0 && servicePort.NodePort != 0 {
+			port.NodePort = servicePort.NodePort
+			newSpec.Ports[i].NodePort = servicePort.NodePort
 		}
-		if port.TargetPort.IntVal == 0 && port.TargetPort.StrVal == "" {
-			port.TargetPort = exist.TargetPort
+		if port.TargetPort.IntVal != 0 {
+			continue
 		}
-		if reflect.DeepEqual(port, *exist) {
-			newSpec.Ports[i].TargetPort = exist.TargetPort
+		port.TargetPort = servicePort.TargetPort
+		if reflect.DeepEqual(port, servicePort) {
+			newSpec.Ports[i].TargetPort = servicePort.TargetPort
 		}
 	}
 	if len(newSpec.ClusterIP) == 0 {