From be1f4b8e1db30c7bc1973046e5082d83e4ee0833 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Fri, 23 Oct 2026 21:06:47 +0200 Subject: [PATCH 1/5] fix: SSA finalizer add no longer re-creates a deleted primary resource A Server-Side Apply creates the object when it is missing, so adding the finalizer with SSA to a primary resource that was deleted before its first reconciliation brought it back as a stub holding only the finalizer. The finalizer apply now carries the cached resource's UID and resource version. With the UID set, the API server rejects the apply of a missing object with a conflict instead of creating it; the resource version adds optimistic locking. On conflict the resource is re-fetched and the apply retried, sharing the retry loop of the JSON Patch finalizer path; if the resource is gone, nothing is applied. Fixes #3649 --- .../en/docs/documentation/reconciler.md | 2 + .../api/reconciler/ResourceOperations.java | 90 +++++++++++++------ .../reconciler/ResourceOperationsTest.java | 74 +++++++++++++++ 3 files changed, 141 insertions(+), 25 deletions(-) diff --git a/docs/content/en/docs/documentation/reconciler.md b/docs/content/en/docs/documentation/reconciler.md index 1fa183633f..2e4ebe2b96 100644 --- a/docs/content/en/docs/documentation/reconciler.md +++ b/docs/content/en/docs/documentation/reconciler.md @@ -157,6 +157,8 @@ You can specify the name of the finalizer to use for your `Reconciler` using the annotation. If you do not specify a finalizer name, one will be automatically generated for you. From v5, by default, the finalizer is added using Server Side Apply. See also `UpdateControl` in docs. +The apply carries the resource's UID and resource version, so a resource deleted before the finalizer +was added is not re-created: the API server rejects the apply, and the framework skips adding the finalizer. ### Read-cache-after-write consistency and event filtering diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java index eb0b0cf144..ce93207b85 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java @@ -16,7 +16,9 @@ package io.javaoperatorsdk.operator.api.reconciler; import java.lang.reflect.InvocationTargetException; +import java.net.HttpURLConnection; import java.util.Optional; +import java.util.function.IntPredicate; import java.util.function.Predicate; import java.util.function.UnaryOperator; @@ -1170,9 +1172,34 @@ public P conflictRetryingPatchPrimary( * @return updated resource from the server or unchanged if the precondition does not hold * @throws OperatorException if the maximum number of retry attempts is exceeded */ - @SuppressWarnings("unchecked") public P conflictRetryingPatchPrimary( UnaryOperator

resourceChangesOperator, Predicate

preCondition, boolean cacheOnly) { + return conflictRetrying( + preCondition, + r -> + jsonPatchPrimary( + r, + resourceChangesOperator, + cacheOnly + ? Options.cacheOnly() + : Options.matchAndFilterWithDefaultMatcher(UpdateType.JSON_PATCH)), + // also retry on unprocessable content (422) which can happen if JSON Patch is not a valid + // request since there was a concurrent request which already removed another finalizer: + // List element removal from a list is by index in JSON Patch so if addressing a second + // finalizer but first is meanwhile removed it is a wrong request. + code -> code == HttpURLConnection.HTTP_CONFLICT || code == 422); + } + + /** + * Runs the given write against the primary resource, re-fetching the resource and retrying when + * the server responds with one of the retried HTTP codes, up to {@link + * ResourceOperations#DEFAULT_MAX_RETRY} times. The pre-condition is evaluated against the + * (possibly re-fetched) resource before each attempt; a re-fetched resource is {@code null} if it + * no longer exists. + */ + @SuppressWarnings("unchecked") + private P conflictRetrying( + Predicate

preCondition, UnaryOperator

write, IntPredicate retriedCode) { var resource = context.getPrimaryResource(); var client = context.getClient(); if (log.isDebugEnabled()) { @@ -1184,22 +1211,11 @@ public P conflictRetryingPatchPrimary( if (!preCondition.test(resource)) { return resource; } - return jsonPatchPrimary( - resource, - resourceChangesOperator, - cacheOnly - ? Options.cacheOnly() - : Options.matchAndFilterWithDefaultMatcher(UpdateType.JSON_PATCH)); + return write.apply(resource); } catch (KubernetesClientException e) { log.trace("Exception during patch for resource: {}", resource); retryIndex++; - // only retry on conflict (409) and unprocessable content (422) which - // can happen if JSON Patch is not a valid request since there was - // a concurrent request which already removed another finalizer: - // List element removal from a list is by index in JSON Patch - // so if addressing a second finalizer but first is meanwhile removed - // it is a wrong request. - if (e.getCode() != 409 && e.getCode() != 422) { + if (!retriedCode.test(e.getCode())) { throw e; } if (retryIndex >= DEFAULT_MAX_RETRY) { @@ -1235,7 +1251,8 @@ public P conflictRetryingPatchPrimary( * String)} with the configured finalizer name. Note that explicitly adding finalizer is required * only if "Trigger reconciliation on all event" mode is on. * - * @return the patched resource from the server response + * @return the patched resource from the server response, the resource unchanged if the finalizer + * does not need to be added, or {@code null} if the resource no longer exists * @see #addFinalizerWithSSA(String) */ public P addFinalizerWithSSA() { @@ -1244,28 +1261,51 @@ public P addFinalizerWithSSA() { /** * Adds finalizer using Server-Side Apply. In the background this method creates a fresh copy of - * the target resource, setting only name, namespace and finalizer. Does not use optimistic - * locking for the patch. Note that explicitly adding finalizer is required only if "Trigger - * reconciliation on all event" mode is on. + * the target resource, setting only name, namespace, UID, resource version and finalizer. It does + * not try to add the finalizer if there is already a finalizer or the resource is marked for + * deletion. Note that explicitly adding finalizer is required only if "Trigger reconciliation on + * all event" mode is on. + * + *

A Server-Side Apply creates the resource if it does not exist, so without further + * precautions the finalizer add would re-create a resource deleted after it was cached. The UID + * makes the API server reject the apply with a conflict instead, and the resource version adds + * optimistic locking. On conflict the resource is re-fetched and the apply retried, up to {@link + * ResourceOperations#DEFAULT_MAX_RETRY} times; if the resource no longer exists, nothing is + * applied and {@code null} is returned. * * @param finalizerName name of the finalizer to add - * @return the patched resource from the server response + * @return the patched resource from the server response, the resource unchanged if the finalizer + * does not need to be added, or {@code null} if the resource no longer exists */ public P addFinalizerWithSSA(String finalizerName) { - var originalResource = context.getPrimaryResource(); if (log.isDebugEnabled()) { + var originalResource = context.getPrimaryResource(); log.debug( "Adding finalizer (using SSA) for resource: {} version: {}", getUID(originalResource), getVersion(originalResource)); } + return conflictRetrying( + r -> { + if (r == null) { + log.warn("Cannot add finalizer since resource no longer exists."); + return false; + } + return !r.isMarkedForDeletion() && !r.hasFinalizer(finalizerName); + }, + r -> serverSideApplyPrimary(finalizerApplyFor(r, finalizerName), Options.cacheOnly()), + code -> code == HttpURLConnection.HTTP_CONFLICT); + } + + private P finalizerApplyFor(P actualResource, String finalizerName) { try { @SuppressWarnings("unchecked") - P resource = (P) originalResource.getClass().getConstructor().newInstance(); - resource.initNameAndNamespaceFrom(originalResource); + P resource = (P) actualResource.getClass().getConstructor().newInstance(); + resource.initNameAndNamespaceFrom(actualResource); + resource.getMetadata().setUid(actualResource.getMetadata().getUid()); + resource.getMetadata().setResourceVersion(actualResource.getMetadata().getResourceVersion()); resource.addFinalizer(finalizerName); - - return serverSideApplyPrimary(resource, Options.cacheOnly()); + return resource; } catch (InstantiationException | IllegalAccessException | InvocationTargetException @@ -1273,7 +1313,7 @@ public P addFinalizerWithSSA(String finalizerName) { throw new RuntimeException( "Issue with creating custom resource instance with reflection." + " Custom Resources must provide a no-arg constructor. Class: " - + originalResource.getClass().getName(), + + actualResource.getClass().getName(), e); } } diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java index 95c1b1815e..ca9dffe4de 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java @@ -156,6 +156,80 @@ void addsFinalizerWithSSA() { verify(controllerEventSource, times(1)).updateAndCacheResource(any(), any(UnaryOperator.class)); } + @Test + void ssaFinalizerAddSendsUidAndResourceVersion() { + var resource = TestUtils.testCustomResource1(); + resource.getMetadata().setUid("uid-1"); + resource.getMetadata().setResourceVersion("1"); + when(context.getPrimaryResource()).thenReturn(resource); + var applied = ArgumentCaptor.forClass(TestCustomResource.class); + when(controllerEventSource.updateAndCacheResource(applied.capture(), any(UnaryOperator.class))) + .thenAnswer(invocation -> invocation.getArgument(0)); + + resourceOperations.addFinalizerWithSSA(FINALIZER_NAME); + + var metadata = applied.getValue().getMetadata(); + assertThat(metadata.getName()).isEqualTo(resource.getMetadata().getName()); + assertThat(metadata.getUid()).isEqualTo("uid-1"); + assertThat(metadata.getResourceVersion()).isEqualTo("1"); + assertThat(metadata.getFinalizers()).containsExactly(FINALIZER_NAME); + } + + @Test + void ssaFinalizerAddDoesNotRecreateDeletedResource() { + var resource = TestUtils.testCustomResource1(); + resource.getMetadata().setUid("uid-1"); + resource.getMetadata().setResourceVersion("1"); + when(context.getPrimaryResource()).thenReturn(resource); + // with the UID set the API server rejects the apply of a missing resource with a conflict + when(controllerEventSource.updateAndCacheResource(any(), any(UnaryOperator.class))) + .thenThrow(new KubernetesClientException("Conflict", 409, null)); + when(resourceOp.get()).thenReturn(null); + + var result = resourceOperations.addFinalizerWithSSA(FINALIZER_NAME); + + assertThat(result).isNull(); + verify(controllerEventSource, times(1)).updateAndCacheResource(any(), any(UnaryOperator.class)); + verify(resourceOp, times(1)).get(); + } + + @Test + void ssaFinalizerAddRetriesConflictWithFreshResource() { + var resource = TestUtils.testCustomResource1(); + resource.getMetadata().setUid("uid-1"); + resource.getMetadata().setResourceVersion("1"); + when(context.getPrimaryResource()).thenReturn(resource); + var freshResource = TestUtils.testCustomResource1(); + freshResource.getMetadata().setUid("uid-1"); + freshResource.getMetadata().setResourceVersion("2"); + when(resourceOp.get()).thenReturn(freshResource); + var applied = ArgumentCaptor.forClass(TestCustomResource.class); + when(controllerEventSource.updateAndCacheResource(applied.capture(), any(UnaryOperator.class))) + .thenThrow(new KubernetesClientException("Conflict", 409, null)) + .thenAnswer(invocation -> invocation.getArgument(0)); + + var result = resourceOperations.addFinalizerWithSSA(FINALIZER_NAME); + + assertThat(result.hasFinalizer(FINALIZER_NAME)).isTrue(); + assertThat(applied.getAllValues()) + .extracting(r -> r.getMetadata().getResourceVersion()) + .containsExactly("1", "2"); + verify(resourceOp, times(1)).get(); + } + + @Test + void ssaFinalizerAddSkipsResourceThatAlreadyHasFinalizer() { + var resource = TestUtils.testCustomResource1(); + resource.getMetadata().setResourceVersion("1"); + resource.addFinalizer(FINALIZER_NAME); + when(context.getPrimaryResource()).thenReturn(resource); + + var result = resourceOperations.addFinalizerWithSSA(FINALIZER_NAME); + + assertThat(result).isSameAs(resource); + verify(controllerEventSource, never()).updateAndCacheResource(any(), any(UnaryOperator.class)); + } + @Test void removesFinalizer() { var resource = TestUtils.testCustomResource1(); From b3b4ae9b9be7884bf5cb963a60fe8c7c96e390c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Sat, 24 Oct 2026 18:06:11 +0200 Subject: [PATCH 2/5] fix: use only optimistic locking for the SSA finalizer add Drop the UID from the finalizer apply and keep only the resource version, so the apply uses optimistic locking: on conflict the resource is re-fetched and the apply retried, and nothing is applied if the re-fetched resource is gone. Without the UID, a Server-Side Apply of a resource that is already deleted can still create it, so this no longer addresses #3649 on its own. --- docs/content/en/docs/documentation/reconciler.md | 2 -- .../api/reconciler/ResourceOperations.java | 14 +++++--------- .../api/reconciler/ResourceOperationsTest.java | 10 +++------- 3 files changed, 8 insertions(+), 18 deletions(-) diff --git a/docs/content/en/docs/documentation/reconciler.md b/docs/content/en/docs/documentation/reconciler.md index 2e4ebe2b96..1fa183633f 100644 --- a/docs/content/en/docs/documentation/reconciler.md +++ b/docs/content/en/docs/documentation/reconciler.md @@ -157,8 +157,6 @@ You can specify the name of the finalizer to use for your `Reconciler` using the annotation. If you do not specify a finalizer name, one will be automatically generated for you. From v5, by default, the finalizer is added using Server Side Apply. See also `UpdateControl` in docs. -The apply carries the resource's UID and resource version, so a resource deleted before the finalizer -was added is not re-created: the API server rejects the apply, and the framework skips adding the finalizer. ### Read-cache-after-write consistency and event filtering diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java index ce93207b85..274851e5f7 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java @@ -1261,17 +1261,14 @@ public P addFinalizerWithSSA() { /** * Adds finalizer using Server-Side Apply. In the background this method creates a fresh copy of - * the target resource, setting only name, namespace, UID, resource version and finalizer. It does - * not try to add the finalizer if there is already a finalizer or the resource is marked for + * the target resource, setting only name, namespace, resource version and finalizer. It does not + * try to add the finalizer if there is already a finalizer or the resource is marked for * deletion. Note that explicitly adding finalizer is required only if "Trigger reconciliation on * all event" mode is on. * - *

A Server-Side Apply creates the resource if it does not exist, so without further - * precautions the finalizer add would re-create a resource deleted after it was cached. The UID - * makes the API server reject the apply with a conflict instead, and the resource version adds - * optimistic locking. On conflict the resource is re-fetched and the apply retried, up to {@link - * ResourceOperations#DEFAULT_MAX_RETRY} times; if the resource no longer exists, nothing is - * applied and {@code null} is returned. + *

The resource version makes the apply use optimistic locking. On conflict the resource is + * re-fetched and the apply retried, up to {@link ResourceOperations#DEFAULT_MAX_RETRY} times; if + * the re-fetched resource no longer exists, nothing is applied and {@code null} is returned. * * @param finalizerName name of the finalizer to add * @return the patched resource from the server response, the resource unchanged if the finalizer @@ -1302,7 +1299,6 @@ private P finalizerApplyFor(P actualResource, String finalizerName) { @SuppressWarnings("unchecked") P resource = (P) actualResource.getClass().getConstructor().newInstance(); resource.initNameAndNamespaceFrom(actualResource); - resource.getMetadata().setUid(actualResource.getMetadata().getUid()); resource.getMetadata().setResourceVersion(actualResource.getMetadata().getResourceVersion()); resource.addFinalizer(finalizerName); return resource; diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java index ca9dffe4de..f58b17ddb9 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java @@ -157,7 +157,7 @@ void addsFinalizerWithSSA() { } @Test - void ssaFinalizerAddSendsUidAndResourceVersion() { + void ssaFinalizerAddSendsResourceVersion() { var resource = TestUtils.testCustomResource1(); resource.getMetadata().setUid("uid-1"); resource.getMetadata().setResourceVersion("1"); @@ -170,18 +170,16 @@ void ssaFinalizerAddSendsUidAndResourceVersion() { var metadata = applied.getValue().getMetadata(); assertThat(metadata.getName()).isEqualTo(resource.getMetadata().getName()); - assertThat(metadata.getUid()).isEqualTo("uid-1"); + assertThat(metadata.getUid()).isNull(); assertThat(metadata.getResourceVersion()).isEqualTo("1"); assertThat(metadata.getFinalizers()).containsExactly(FINALIZER_NAME); } @Test - void ssaFinalizerAddDoesNotRecreateDeletedResource() { + void ssaFinalizerAddStopsWhenResourceIsGoneAfterConflict() { var resource = TestUtils.testCustomResource1(); - resource.getMetadata().setUid("uid-1"); resource.getMetadata().setResourceVersion("1"); when(context.getPrimaryResource()).thenReturn(resource); - // with the UID set the API server rejects the apply of a missing resource with a conflict when(controllerEventSource.updateAndCacheResource(any(), any(UnaryOperator.class))) .thenThrow(new KubernetesClientException("Conflict", 409, null)); when(resourceOp.get()).thenReturn(null); @@ -196,11 +194,9 @@ void ssaFinalizerAddDoesNotRecreateDeletedResource() { @Test void ssaFinalizerAddRetriesConflictWithFreshResource() { var resource = TestUtils.testCustomResource1(); - resource.getMetadata().setUid("uid-1"); resource.getMetadata().setResourceVersion("1"); when(context.getPrimaryResource()).thenReturn(resource); var freshResource = TestUtils.testCustomResource1(); - freshResource.getMetadata().setUid("uid-1"); freshResource.getMetadata().setResourceVersion("2"); when(resourceOp.get()).thenReturn(freshResource); var applied = ArgumentCaptor.forClass(TestCustomResource.class); From e08057b553b157abc8f692b9ed587640b2138708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Sun, 25 Oct 2026 08:15:05 +0100 Subject: [PATCH 3/5] fix: guard the SSA finalizer add with the resource UID instead of optimistic locking Send the cached resource's UID instead of its resource version with the finalizer apply. The API server then rejects the apply with a conflict (409) if the resource no longer exists, instead of re-creating it, and with unprocessable content (422) if it was re-created under the same name. Both are retried after re-fetching the resource; if it is gone, nothing is applied. No optimistic locking is needed: finalizers are a set for Server-Side Apply, so the apply only adds this finalizer and leaves concurrent changes intact. Fixes #3649 --- .../en/docs/documentation/reconciler.md | 2 ++ .../api/reconciler/ResourceOperations.java | 22 +++++++++------ .../reconciler/ResourceOperationsTest.java | 28 ++++++++++--------- 3 files changed, 30 insertions(+), 22 deletions(-) diff --git a/docs/content/en/docs/documentation/reconciler.md b/docs/content/en/docs/documentation/reconciler.md index 1fa183633f..86e701d801 100644 --- a/docs/content/en/docs/documentation/reconciler.md +++ b/docs/content/en/docs/documentation/reconciler.md @@ -157,6 +157,8 @@ You can specify the name of the finalizer to use for your `Reconciler` using the annotation. If you do not specify a finalizer name, one will be automatically generated for you. From v5, by default, the finalizer is added using Server Side Apply. See also `UpdateControl` in docs. +The apply carries the resource's UID, so a resource deleted before the finalizer was added is not +re-created: the API server rejects the apply, and the framework skips adding the finalizer. ### Read-cache-after-write consistency and event filtering diff --git a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java index 274851e5f7..93bbe7fa7b 100644 --- a/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java +++ b/operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java @@ -1261,14 +1261,18 @@ public P addFinalizerWithSSA() { /** * Adds finalizer using Server-Side Apply. In the background this method creates a fresh copy of - * the target resource, setting only name, namespace, resource version and finalizer. It does not - * try to add the finalizer if there is already a finalizer or the resource is marked for - * deletion. Note that explicitly adding finalizer is required only if "Trigger reconciliation on - * all event" mode is on. + * the target resource, setting only name, namespace, UID and finalizer. It does not try to add + * the finalizer if there is already a finalizer or the resource is marked for deletion. Note that + * explicitly adding finalizer is required only if "Trigger reconciliation on all event" mode is + * on. * - *

The resource version makes the apply use optimistic locking. On conflict the resource is - * re-fetched and the apply retried, up to {@link ResourceOperations#DEFAULT_MAX_RETRY} times; if - * the re-fetched resource no longer exists, nothing is applied and {@code null} is returned. + *

A Server-Side Apply creates the resource if it does not exist, so without further + * precautions the finalizer add would re-create a resource deleted after it was cached. The UID + * makes the API server reject the apply instead: with a conflict (HTTP 409) if the resource no + * longer exists, and with unprocessable content (HTTP 422) if it was re-created under the same + * name. In both cases the resource is re-fetched and the apply retried, up to {@link + * ResourceOperations#DEFAULT_MAX_RETRY} times; if the resource no longer exists, nothing is + * applied and {@code null} is returned. No optimistic locking is used. * * @param finalizerName name of the finalizer to add * @return the patched resource from the server response, the resource unchanged if the finalizer @@ -1291,7 +1295,7 @@ public P addFinalizerWithSSA(String finalizerName) { return !r.isMarkedForDeletion() && !r.hasFinalizer(finalizerName); }, r -> serverSideApplyPrimary(finalizerApplyFor(r, finalizerName), Options.cacheOnly()), - code -> code == HttpURLConnection.HTTP_CONFLICT); + code -> code == HttpURLConnection.HTTP_CONFLICT || code == 422); } private P finalizerApplyFor(P actualResource, String finalizerName) { @@ -1299,7 +1303,7 @@ private P finalizerApplyFor(P actualResource, String finalizerName) { @SuppressWarnings("unchecked") P resource = (P) actualResource.getClass().getConstructor().newInstance(); resource.initNameAndNamespaceFrom(actualResource); - resource.getMetadata().setResourceVersion(actualResource.getMetadata().getResourceVersion()); + resource.getMetadata().setUid(actualResource.getMetadata().getUid()); resource.addFinalizer(finalizerName); return resource; } catch (InstantiationException diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java index f58b17ddb9..657df5f81a 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java @@ -157,7 +157,7 @@ void addsFinalizerWithSSA() { } @Test - void ssaFinalizerAddSendsResourceVersion() { + void ssaFinalizerAddSendsUidWithoutResourceVersion() { var resource = TestUtils.testCustomResource1(); resource.getMetadata().setUid("uid-1"); resource.getMetadata().setResourceVersion("1"); @@ -170,16 +170,17 @@ void ssaFinalizerAddSendsResourceVersion() { var metadata = applied.getValue().getMetadata(); assertThat(metadata.getName()).isEqualTo(resource.getMetadata().getName()); - assertThat(metadata.getUid()).isNull(); - assertThat(metadata.getResourceVersion()).isEqualTo("1"); + assertThat(metadata.getUid()).isEqualTo("uid-1"); + assertThat(metadata.getResourceVersion()).isNull(); assertThat(metadata.getFinalizers()).containsExactly(FINALIZER_NAME); } @Test - void ssaFinalizerAddStopsWhenResourceIsGoneAfterConflict() { + void ssaFinalizerAddDoesNotRecreateDeletedResource() { var resource = TestUtils.testCustomResource1(); - resource.getMetadata().setResourceVersion("1"); + resource.getMetadata().setUid("uid-1"); when(context.getPrimaryResource()).thenReturn(resource); + // with the UID set the API server rejects the apply of a missing resource with a conflict when(controllerEventSource.updateAndCacheResource(any(), any(UnaryOperator.class))) .thenThrow(new KubernetesClientException("Conflict", 409, null)); when(resourceOp.get()).thenReturn(null); @@ -192,24 +193,25 @@ void ssaFinalizerAddStopsWhenResourceIsGoneAfterConflict() { } @Test - void ssaFinalizerAddRetriesConflictWithFreshResource() { + void ssaFinalizerAddRetriesWithUidOfRecreatedResource() { var resource = TestUtils.testCustomResource1(); - resource.getMetadata().setResourceVersion("1"); + resource.getMetadata().setUid("uid-1"); when(context.getPrimaryResource()).thenReturn(resource); - var freshResource = TestUtils.testCustomResource1(); - freshResource.getMetadata().setResourceVersion("2"); - when(resourceOp.get()).thenReturn(freshResource); + var recreatedResource = TestUtils.testCustomResource1(); + recreatedResource.getMetadata().setUid("uid-2"); + when(resourceOp.get()).thenReturn(recreatedResource); var applied = ArgumentCaptor.forClass(TestCustomResource.class); + // a UID differing from the existing resource's is rejected as an immutable field change when(controllerEventSource.updateAndCacheResource(applied.capture(), any(UnaryOperator.class))) - .thenThrow(new KubernetesClientException("Conflict", 409, null)) + .thenThrow(new KubernetesClientException("Unprocessable", 422, null)) .thenAnswer(invocation -> invocation.getArgument(0)); var result = resourceOperations.addFinalizerWithSSA(FINALIZER_NAME); assertThat(result.hasFinalizer(FINALIZER_NAME)).isTrue(); assertThat(applied.getAllValues()) - .extracting(r -> r.getMetadata().getResourceVersion()) - .containsExactly("1", "2"); + .extracting(r -> r.getMetadata().getUid()) + .containsExactly("uid-1", "uid-2"); verify(resourceOp, times(1)).get(); } From f65604645c14d2a5eb2cd685a4a77b7751397d51 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Sun, 25 Oct 2026 08:22:23 +0100 Subject: [PATCH 4/5] test: verify the SSA finalizer add against a real API server Uses kube-api-test to check that adding the finalizer with Server-Side Apply does not re-create a deleted resource, adds it to a resource re-created under the same name, and works from a stale cached copy. --- .../AddFinalizerWithSSAIntegrationTest.java | 151 ++++++++++++++++++ 1 file changed, 151 insertions(+) create mode 100644 operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/AddFinalizerWithSSAIntegrationTest.java diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/AddFinalizerWithSSAIntegrationTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/AddFinalizerWithSSAIntegrationTest.java new file mode 100644 index 0000000000..2c76af7283 --- /dev/null +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/AddFinalizerWithSSAIntegrationTest.java @@ -0,0 +1,151 @@ +/* + * Copyright Java Operator SDK Authors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.javaoperatorsdk.operator.api.reconciler; + +import java.util.Map; +import java.util.function.UnaryOperator; + +import org.junit.jupiter.api.Test; + +import io.fabric8.kubeapitest.junit.EnableKubeAPIServer; +import io.fabric8.kubernetes.api.model.ConfigMap; +import io.fabric8.kubernetes.api.model.ConfigMapBuilder; +import io.fabric8.kubernetes.client.KubernetesClient; +import io.javaoperatorsdk.operator.api.config.ControllerConfiguration; +import io.javaoperatorsdk.operator.processing.event.EventSourceRetriever; +import io.javaoperatorsdk.operator.processing.event.source.controller.ControllerEventSource; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + +/** + * Adds the finalizer with Server-Side Apply against a real API server, which is what decides + * whether the apply creates a missing resource or rejects it. + */ +@EnableKubeAPIServer +class AddFinalizerWithSSAIntegrationTest { + + private static final String NAMESPACE = "default"; + private static final String FINALIZER = "int.test/finalizer"; + + static KubernetesClient client; + + @Test + void addsFinalizerToExistingResource() { + var cached = createConfigMap("existing"); + + var result = resourceOperationsFor(cached).addFinalizerWithSSA(FINALIZER); + + var actual = get("existing"); + assertThat(result.getMetadata().getFinalizers()).containsExactly(FINALIZER); + assertThat(actual.getMetadata().getFinalizers()).containsExactly(FINALIZER); + assertThat(actual.getData()).containsEntry("key", "value"); + removeFinalizerAndDelete(actual); + } + + @Test + void doesNotRecreateDeletedResource() { + var cached = createConfigMap("deleted"); + client.resource(cached).delete(); + + var result = resourceOperationsFor(cached).addFinalizerWithSSA(FINALIZER); + + assertThat(result).isNull(); + assertThat(get("deleted")).isNull(); + } + + @Test + void addsFinalizerToResourceRecreatedWithSameName() { + var cached = createConfigMap("recreated"); + client.resource(cached).delete(); + var recreated = createConfigMap("recreated"); + + var result = resourceOperationsFor(cached).addFinalizerWithSSA(FINALIZER); + + var actual = get("recreated"); + assertThat(result.getMetadata().getUid()).isEqualTo(recreated.getMetadata().getUid()); + assertThat(actual.getMetadata().getUid()).isEqualTo(recreated.getMetadata().getUid()); + assertThat(actual.getMetadata().getFinalizers()).containsExactly(FINALIZER); + removeFinalizerAndDelete(actual); + } + + @Test + void addsFinalizerUsingStaleCachedResource() { + var cached = createConfigMap("stale"); + var updated = get("stale"); + updated.setData(Map.of("key", "updated")); + client.resource(updated).update(); + + resourceOperationsFor(cached).addFinalizerWithSSA(FINALIZER); + + var actual = get("stale"); + assertThat(actual.getMetadata().getFinalizers()).containsExactly(FINALIZER); + assertThat(actual.getData()).containsEntry("key", "updated"); + removeFinalizerAndDelete(actual); + } + + /** + * Only the framework plumbing around the API call is mocked: the controller event source just + * runs the write it is given, so the apply and the re-fetch on retry hit the real API server. + */ + @SuppressWarnings("unchecked") + private static ResourceOperations resourceOperationsFor(ConfigMap primary) { + Context context = mock(Context.class); + ControllerConfiguration controllerConfiguration = + mock(ControllerConfiguration.class); + var eventSourceRetriever = mock(EventSourceRetriever.class); + ControllerEventSource controllerEventSource = mock(ControllerEventSource.class); + + when(context.getClient()).thenReturn(client); + when(context.getPrimaryResource()).thenReturn(primary); + when(context.getControllerConfiguration()).thenReturn(controllerConfiguration); + when(controllerConfiguration.fieldManager()).thenReturn("finalizer-test"); + when(context.eventSourceRetriever()).thenReturn(eventSourceRetriever); + when(eventSourceRetriever.getControllerEventSource()).thenReturn(controllerEventSource); + when(controllerEventSource.updateAndCacheResource(any(), any(UnaryOperator.class))) + .thenAnswer( + invocation -> { + UnaryOperator write = invocation.getArgument(1); + return write.apply(invocation.getArgument(0)); + }); + return new ResourceOperations<>(context); + } + + private static ConfigMap createConfigMap(String name) { + return client + .resource( + new ConfigMapBuilder() + .withNewMetadata() + .withName(name) + .withNamespace(NAMESPACE) + .endMetadata() + .withData(Map.of("key", "value")) + .build()) + .create(); + } + + private static ConfigMap get(String name) { + return client.configMaps().inNamespace(NAMESPACE).withName(name).get(); + } + + private static void removeFinalizerAndDelete(ConfigMap configMap) { + configMap.getMetadata().getFinalizers().clear(); + client.resource(configMap).update(); + client.resource(configMap).delete(); + } +} From ea3e37f68e5961e559fc278f4c8397355dc273fb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Attila=20M=C3=A9sz=C3=A1ros?= Date: Sun, 25 Oct 2026 11:14:01 +0100 Subject: [PATCH 5/5] wip MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Attila Mészáros --- ...tegrationTest.java => AddFinalizerWithSSAIntegrationIT.java} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/{AddFinalizerWithSSAIntegrationTest.java => AddFinalizerWithSSAIntegrationIT.java} (99%) diff --git a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/AddFinalizerWithSSAIntegrationTest.java b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/AddFinalizerWithSSAIntegrationIT.java similarity index 99% rename from operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/AddFinalizerWithSSAIntegrationTest.java rename to operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/AddFinalizerWithSSAIntegrationIT.java index 2c76af7283..09db57fe45 100644 --- a/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/AddFinalizerWithSSAIntegrationTest.java +++ b/operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/AddFinalizerWithSSAIntegrationIT.java @@ -38,7 +38,7 @@ * whether the apply creates a missing resource or rejects it. */ @EnableKubeAPIServer -class AddFinalizerWithSSAIntegrationTest { +class AddFinalizerWithSSAIntegrationIT { private static final String NAMESPACE = "default"; private static final String FINALIZER = "int.test/finalizer";