fix: NPE when adding a new service port (#8820)

apecloud · Jan 20, 2025 · e8a2de1 · e8a2de1
1 parent 4e537d5
commit e8a2de1
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 34 deletions.
diff --git 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
@@ -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 {