From 9d0d2bae6cbb32be9478f8ea6f9eb09ce8ac1285 Mon Sep 17 00:00:00 2001
From: Aleksei Sviridkin <f@lex.la>
Date: Tue, 29 Sep 2026 15:16:15 +0300
Subject: [PATCH] fix(finalizers): keep other controllers' finalizers

Adding the robotlb finalizer used a JSON merge patch that set
metadata.finalizers to a one-element list. A merge patch replaces a
list as a whole, so every finalizer another controller had put on the
Service was dropped. Removal wrote back the list read from the cache,
so a finalizer added or removed in between was lost or came back.

metadata.finalizers is merged by value in a strategic merge patch, so
adding now appends only robotlb's entry and removal deletes only that
entry with a deleteFromPrimitiveList directive.

Assisted-by: LLM
Signed-off-by: Aleksei Sviridkin <f@lex.la>
---
 src/finalizers.rs | 68 ++++++++++++++++++++++++++++++++++-------------
 1 file changed, 50 insertions(+), 18 deletions(-)

diff --git a/src/finalizers.rs b/src/finalizers.rs
index f45d4fc..2f65e5b 100644
--- a/src/finalizers.rs
+++ b/src/finalizers.rs
@@ -1,4 +1,7 @@
-use k8s_openapi::{api::core::v1::Service, serde_json::json};
+use k8s_openapi::{
+    api::core::v1::Service,
+    serde_json::{json, Value},
+};
 use kube::{
     api::{Patch, PatchParams},
     Api, Client, ResourceExt,
@@ -16,15 +19,10 @@ pub async fn add(client: Client, svc: &Service) -> RobotLBResult<()> {
         client,
         svc.namespace().ok_or(RobotLBError::SkipService)?.as_str(),
     );
-    let patch = json!({
-        "metadata": {
-            "finalizers": [consts::FINALIZER_NAME]
-        }
-    });
     api.patch(
         svc.name_any().as_str(),
         &PatchParams::default(),
-        &Patch::Merge(patch),
+        &add_patch(),
     )
     .await?;
     Ok(())
@@ -51,21 +49,55 @@ pub async fn remove(client: Client, svc: &Service) -> RobotLBResult<()> {
         client,
         svc.namespace().ok_or(RobotLBError::SkipService)?.as_str(),
     );
-    let finalizers = svc
-        .finalizers()
-        .iter()
-        .filter(|item| item.as_str() != consts::FINALIZER_NAME)
-        .collect::<Vec<_>>();
-    let patch = json!({
-        "metadata": {
-            "finalizers": finalizers
-        }
-    });
     api.patch(
         svc.name_any().as_str(),
         &PatchParams::default(),
-        &Patch::Merge(patch),
+        &remove_patch(),
     )
     .await?;
     Ok(())
 }
+
+// `metadata.finalizers` is merged by value in a strategic merge patch, so these
+// touch only robotlb's own entry, whatever other controllers add or remove meanwhile.
+fn add_patch() -> Patch<Value> {
+    Patch::Strategic(json!({
+        "metadata": {
+            "finalizers": [consts::FINALIZER_NAME]
+        }
+    }))
+}
+
+fn remove_patch() -> Patch<Value> {
+    Patch::Strategic(json!({
+        "metadata": {
+            "$deleteFromPrimitiveList/finalizers": [consts::FINALIZER_NAME]
+        }
+    }))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::{add_patch, remove_patch};
+    use crate::consts;
+    use k8s_openapi::serde_json::json;
+    use kube::api::Patch;
+
+    #[test]
+    fn adding_merges_into_the_existing_list() {
+        assert_eq!(
+            add_patch(),
+            Patch::Strategic(json!({"metadata": {"finalizers": [consts::FINALIZER_NAME]}}))
+        );
+    }
+
+    #[test]
+    fn removing_deletes_only_our_finalizer() {
+        assert_eq!(
+            remove_patch(),
+            Patch::Strategic(json!({
+                "metadata": {"$deleteFromPrimitiveList/finalizers": [consts::FINALIZER_NAME]}
+            }))
+        );
+    }
+}