Merge pull request #9653 from BassinD/bugfix/nil-check-for-service-health-check-node-port-in-last-applied-config

Fix service restore with null healthCheckNodePort in last-applied-configuration label

Author: blackpiglet

changelogs/unreleased/9653-BassinD

@@ -0,0 +1 @@
+Fix service restore with null healthCheckNodePort in last-applied-configuration label

pkg/restore/actions/service_action.go

@@ -94,21 +94,18 @@ func deleteHealthCheckNodePort(service *corev1api.Service) error {
 
 	// Search HealthCheckNodePort from server's last-applied-configuration
 	// annotation(HealthCheckNodePort is specified by `kubectl apply` command)
-	lastAppliedConfig, ok := service.Annotations[annotationLastAppliedConfig]
-	if ok {
-		appliedServiceUnstructured := new(map[string]any)
-		if err := json.Unmarshal([]byte(lastAppliedConfig), appliedServiceUnstructured); err != nil {
-			return errors.WithStack(err)
+	if lastAppliedConfig, ok := service.Annotations[annotationLastAppliedConfig]; ok {
+		var appliedConfig struct {
+			Spec struct {
+				HealthCheckNodePort *int32 `json:"healthCheckNodePort"`
+			} `json:"spec"`
 		}
 
-		healthCheckNodePort, exist, err := unstructured.NestedFloat64(*appliedServiceUnstructured, "spec", "healthCheckNodePort")
-		if err != nil {
+		if err := json.Unmarshal([]byte(lastAppliedConfig), &appliedConfig); err != nil {
 			return errors.WithStack(err)
 		}
 
-		// Found healthCheckNodePort in lastAppliedConfig annotation,
-		// and the value is not 0. No need to delete, return.
-		if exist && healthCheckNodePort != 0 {
+		if appliedConfig.Spec.HealthCheckNodePort != nil && *appliedConfig.Spec.HealthCheckNodePort != 0 {
 			return nil
 		}
 	}

pkg/restore/actions/service_action_test.go

@@ -644,6 +644,36 @@ func TestServiceActionExecute(t *testing.T) {
 				},
 			},
 		},
+		{
+			name: "If PreserveNodePorts is false and HealthCheckNodePort is null in last-applied-configuration, it should not crash and the port should be cleared.",
+			obj: corev1api.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "svc-1",
+					Annotations: map[string]string{
+						"kubectl.kubernetes.io/last-applied-configuration": `{"spec":{"healthCheckNodePort":null}}`,
+					},
+				},
+				Spec: corev1api.ServiceSpec{
+					HealthCheckNodePort:   8080,
+					ExternalTrafficPolicy: corev1api.ServiceExternalTrafficPolicyTypeLocal,
+					Type:                  corev1api.ServiceTypeLoadBalancer,
+				},
+			},
+			restore: builder.ForRestore(api.DefaultNamespace, "").PreserveNodePorts(false).Result(),
+			expectedRes: corev1api.Service{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: "svc-1",
+					Annotations: map[string]string{
+						"kubectl.kubernetes.io/last-applied-configuration": `{"spec":{"healthCheckNodePort":null}}`,
+					},
+				},
+				Spec: corev1api.ServiceSpec{
+					HealthCheckNodePort:   0,
+					ExternalTrafficPolicy: corev1api.ServiceExternalTrafficPolicyTypeLocal,
+					Type:                  corev1api.ServiceTypeLoadBalancer,
+				},
+			},
+		},
 	}
 
 	for _, test := range tests {