feat: detect dependent resource API version changes - #3536
Conversation
Add an opt-in, experimental detectApiVersionChange option on @KubernetesDependent that records the API version the operator applies in the javaoperatorsdk.io/last-applied-api-version annotation. The regular matcher then detects a mismatch when that marker differs from (or is missing relative to) the currently applied API version, causing a one-time update without triggering repeated reconciliations once the resource is up-to-date. Disabled by default, so existing behavior and matching are unaffected unless explicitly enabled. Fixes operator-framework#2644 Signed-off-by: hej090224 <fc49854985@gmail.com>
d322648 to
f0530e1
Compare
There was a problem hiding this comment.
Pull request overview
Note
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Adds an opt-in, experimental mechanism for dependent resources to detect API version changes by persisting the operator-applied API version in an annotation, so the existing matcher logic can trigger a one-time update when that marker changes.
Changes:
- Introduces
detectApiVersionChangeon@KubernetesDependent, plus config + builder wiring to propagate the flag. - Adds
javaoperatorsdk.io/last-applied-api-versionmarker annotation support inKubernetesDependentResource. - Adds focused unit tests and documentation covering both SSA and non-SSA matcher behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java | Adds marker annotation constant and writes marker when feature enabled. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.java | Adds config flag, default, constructor overload, and getter. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfigBuilder.java | Adds builder support for detectApiVersionChange. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentConverter.java | Wires annotation attribute into config creation. |
| operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.java | Adds experimental annotation attribute and documentation. |
| operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceApiVersionChangeTest.java | New tests validating marker-driven mismatch behavior for SSA and non-SSA. |
| operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentConverterTest.java | New focused test for annotation→config wiring for the new flag. |
| docs/content/en/docs/documentation/dependent-resource-and-workflows/dependent-resources.md | Documents the new experimental feature and its behavior/limitations. |
| private void addLastAppliedApiVersion(R target) { | ||
| if (kubernetesDependentResourceConfig == null | ||
| || !kubernetesDependentResourceConfig.detectApiVersionChange()) { | ||
| return; | ||
| } | ||
| var apiVersion = target.getApiVersion(); | ||
| if (apiVersion != null) { | ||
| target | ||
| .getMetadata() | ||
| .getAnnotations() | ||
| .put(LAST_APPLIED_API_VERSION_ANNOTATION_KEY, apiVersion); | ||
| } | ||
| } |
There was a problem hiding this comment.
Fixed in aacbb4b: the desired resource's annotations map is now defensively copied into a mutable LinkedHashMap (via ensureMutableAnnotations) whenever detectApiVersionChange is enabled, before this method or the pre-existing previous-annotation bookkeeping write to it. This is scoped to the opt-in feature so default behavior for everyone else is unchanged. Added a regression test (nonSSA_preservesExistingAnnotationsWhenMarkingEvenIfImmutable) that sets an immutable Map.of(...) on the desired resource and asserts both the existing entry and the new marker survive.
| private void addLastAppliedApiVersion(R target) { | ||
| if (kubernetesDependentResourceConfig == null | ||
| || !kubernetesDependentResourceConfig.detectApiVersionChange()) { | ||
| return; | ||
| } | ||
| var apiVersion = target.getApiVersion(); | ||
| if (apiVersion != null) { | ||
| target | ||
| .getMetadata() | ||
| .getAnnotations() | ||
| .put(LAST_APPLIED_API_VERSION_ANNOTATION_KEY, apiVersion); | ||
| } | ||
| } |
There was a problem hiding this comment.
Added nonSSA_preservesExistingAnnotationsWhenMarkingEvenIfImmutable in aacbb4b, which sets an immutable Map.of("user.example.com/owner", "team-a") as the desired resource's annotations, then asserts the marker is added and the pre-existing entry is preserved. This also caught that the pre-existing previous-annotation handling code had the same latent assumption, which is now fixed alongside it (see the other reply).
| private static class ConfigMapDependentResourceForTest | ||
| extends KubernetesDependentResource<GenericKubernetesResource, HasMetadata> { | ||
| public ConfigMapDependentResourceForTest() { | ||
| super(GenericKubernetesResource.class, null); | ||
| } | ||
| } |
There was a problem hiding this comment.
Renamed to WidgetDependentResourceForTest in aacbb4b — good catch, it does manage a GenericKubernetesResource (a synthetic "Widget" test resource), not a ConfigMap.
| var result = dr.match(actual, desired, primary(), context); | ||
|
|
||
| assertThat(result.matched()).isNotNull(); | ||
| assertThat(desired.getMetadata().getAnnotations()) | ||
| .doesNotContainKey(KubernetesDependentResource.LAST_APPLIED_API_VERSION_ANNOTATION_KEY); |
There was a problem hiding this comment.
Fixed in aacbb4b — replaced with assertThat(result.matched()).withFailMessage(...).isTrue(), asserting the concrete expected outcome (a null desired API version just skips marking, and matching proceeds normally) instead of the no-op null check.
| * <p>When enabled, JOSDK records the API version it applies in the {@code | ||
| * javaoperatorsdk.io/last-applied-api-version} annotation. On subsequent reconciliations, the | ||
| * resource is considered mismatched (and thus updated) if that recorded marker differs from the |
There was a problem hiding this comment.
Fixed in aacbb4b — the Javadoc now references {@value KubernetesDependentResource#LAST_APPLIED_API_VERSION_ANNOTATION_KEY} instead of duplicating the string literal.
Guard against a null or immutable annotations map (e.g. Map.of(...)) on the desired resource when detectApiVersionChange is enabled, since writing the last-applied-api-version marker (and the pre-existing previous-annotation bookkeeping that runs alongside it) requires a mutable map. Also rename a misleadingly-named test helper and replace a no-op assertion on a primitive boolean with a concrete expectation. Addresses Copilot review feedback on PR operator-framework#3536. Signed-off-by: hej090224 <fc49854985@gmail.com>
Summary
detectApiVersionChangeoption on@KubernetesDependentto detect when a dependent resource's API version has changed since the operator last applied it, and request a one-time update in that case.javaoperatorsdk.io/last-applied-api-versionannotation, following the same pattern as the existingjavaoperatorsdk.io/previousannotation.Motivation
When a dependent resource's CRD gains a new API version and the operator is upgraded to target it, comparing
actualResource.getApiVersion()with the desired resource's API version is not a reliable way to detect resources that still need to be updated: the Kubernetes API server serves a resource using the requested, served API version regardless of what it is actually stored as, so this comparison would always trivially match. This is whyKubernetesDependentResourcealready ignoresapiVersionentirely in both matchers.Instead of trying to infer the actual stored/storage version (which JOSDK cannot reliably observe, and which tools like StorageVersionMigration exist to address), this PR lets JOSDK track what the operator itself last applied, using a persistent annotation marker, discussed in #2644.
When
detectApiVersionChangeis enabled:This is explicitly not a replacement for Kubernetes' StorageVersionMigration and does not attempt to read or infer the actual stored representation of the resource.
Public API
@KubernetesDependent(detectApiVersionChange = true)annotation attribute (defaultfalse), marked@Experimental.KubernetesDependentResourceConfig#detectApiVersionChange()and a new (additive) constructor overload; existing constructors are unchanged.KubernetesDependentResourceConfigBuilder#withDetectApiVersionChange(boolean).KubernetesDependentResource.LAST_APPLIED_API_VERSION_ANNOTATION_KEYconstant (javaoperatorsdk.io/last-applied-api-version).No breaking changes.
Testing
./mvnw -pl operator-framework-core -am test -Dtest='KubernetesDependentResourceApiVersionChangeTest,KubernetesDependentConverterTest'./mvnw -pl operator-framework-core -am test -Dtest='GenericKubernetesResourceMatcherTest,SSABasedGenericKubernetesResourceMatcherTest,KubernetesDependentResourceTest,DependentResourceConfigurationResolverTest'./mvnw -pl operator-framework-core -am test(full core module, 706 tests)./mvnw spotless:apply/./mvnw spotless:check./mvnw clean install -pl '!migration'(themigrationmodule's OpenRewrite-based tests fail in this environment due to a pre-existing JDK 25 / OpenRewrite javac-internals incompatibility, unrelated to this change and not touched by it)Fixes #2644