fix(services): use JSON merge patch for managed service spec reconciliation (cloudnative-pg#10311)

Previously, the patch strategy reconciler compared service specs by
starting from the proposed spec and explicitly preserving a hard-coded
list of Kubernetes-managed fields. Any field missing from that list —
such as LoadBalancerClass set by a cloud provider — would be zeroed out,
causing API errors on immutable fields and silent spec corruption on
mutable ones.

Replace this deny-list approach by computing an RFC 7386 JSON Merge
Patch between the last-applied and proposed specs, then applying it
onto the living service. A last-applied spec annotation
(cnpg.io/lastAppliedSpec) tracks what was previously applied so that
intentional field removals produce null entries in the patch.

This naturally preserves any field the operator doesn't control —
including fields set by cloud providers, admission webhooks, or future
Kubernetes versions — without needing to enumerate them.

Closes cloudnative-pg#10132

Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
Co-authored-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
(cherry picked from commit e20d4d5)

Author: armru

internal/controller/cluster_create.go

@@ -401,6 +401,7 @@ func (r *ClusterReconciler) serviceReconciler(
 			return nil
 		}
 		contextLogger.Info("creating service")
+		servicespec.SetLastApplied(&proposed.ObjectMeta, &proposed.Spec)
 		return r.Create(ctx, proposed)
 	}
 	if err != nil {
@@ -441,9 +442,12 @@ func (r *ClusterReconciler) serviceReconciler(
 		shouldUpdate = true
 	}
 
-	// we check if the service spec has changed, preserving Kubernetes-managed fields
-	patchedSpec := proposed.Spec.DeepCopy()
-	servicespec.PreserveKubernetesDefaults(patchedSpec, &livingService.Spec)
+	// Three-way merge: start from living spec, overlay proposed changes,
+	// and use last-applied annotation to detect intentional field removals
+	patchedSpec := livingService.Spec.DeepCopy()
+	if err := servicespec.ApplyProposedChanges(patchedSpec, &proposed.Spec, livingService.Annotations); err != nil {
+		return err
+	}
 	if !reflect.DeepEqual(*patchedSpec, livingService.Spec) {
 		livingService.Spec = *patchedSpec
 		shouldUpdate = true
@@ -454,6 +458,8 @@ func (r *ClusterReconciler) serviceReconciler(
 	}
 
 	if strategy == apiv1.ServiceUpdateStrategyPatch {
+		// Store the proposed spec for future three-way merges
+		servicespec.SetLastApplied(&livingService.ObjectMeta, &proposed.Spec)
 		contextLogger.Info("reconciling service")
 		return r.Update(ctx, &livingService)
 	}

internal/controller/cluster_create_test.go

@@ -197,10 +197,16 @@ var _ = Describe("cluster_create unit tests", func() {
 				}, &afterChangesService)
 				Expect(err).ToNot(HaveOccurred())
 
-				Expect(afterChangesService.Spec.Selector).ToNot(Equal(before.Spec.Selector))
-				Expect(afterChangesService.Spec.Selector).To(Equal(expectedLabels))
+				for k, v := range expectedLabels {
+					Expect(afterChangesService.Spec.Selector).To(HaveKeyWithValue(k, v))
+				}
 				Expect(afterChangesService.Labels).To(Equal(before.Labels))
-				Expect(afterChangesService.Annotations).To(Equal(before.Annotations))
+				// The reconciler now stores a lastAppliedServiceSpec annotation
+				// for three-way merge, so we check that original annotations are preserved
+				for k, v := range before.Annotations {
+					Expect(afterChangesService.Annotations).To(HaveKeyWithValue(k, v))
+				}
+				Expect(afterChangesService.Annotations).To(HaveKey(utils.LastAppliedSpecAnnotationName))
 			}
 
 			var readOnlyService, readWriteService, readService, anyService *corev1.Service

internal/controller/pooler_update.go

@@ -140,6 +140,7 @@ func (r *PoolerReconciler) reconcileService(
 
 	if resources.Service == nil {
 		contextLog.Info("Creating the service")
+		servicespec.SetLastApplied(&expectedService.ObjectMeta, &expectedService.Spec)
 		err := r.Create(ctx, expectedService)
 		if err != nil && !apierrs.IsAlreadyExists(err) {
 			return err
@@ -149,15 +150,23 @@ func (r *PoolerReconciler) reconcileService(
 	}
 
 	patchedService := resources.Service.DeepCopy()
-	patchedService.Spec = expectedService.Spec
-	servicespec.PreserveKubernetesDefaults(&patchedService.Spec, &resources.Service.Spec)
+	if err := servicespec.ApplyProposedChanges(
+		&patchedService.Spec,
+		&expectedService.Spec,
+		resources.Service.Annotations,
+	); err != nil {
+		return err
+	}
 	utils.MergeObjectsMetadata(patchedService, expectedService)
 
 	if reflect.DeepEqual(patchedService.ObjectMeta, resources.Service.ObjectMeta) &&
 		reflect.DeepEqual(patchedService.Spec, resources.Service.Spec) {
 		return nil
 	}
 
+	// Store the proposed spec for future three-way merges
+	servicespec.SetLastApplied(&patchedService.ObjectMeta, &expectedService.Spec)
+
 	contextLog.Info("Updating the service metadata")
 
 	return r.Patch(ctx, patchedService, client.MergeFrom(resources.Service))