improve: owner reference mappers checks only group not apiVersion by csviri · Pull Request #3302 · operator-framework/java-operator-sdk

csviri · 2026-04-20T10:25:02Z

In case of a resource is has a new version, but owner reference
still points to the old version we should still propapage the event.

Signed-off-by: Attila Mészáros a_meszaros@apple.com

csviri · 2026-04-20T10:25:20Z

cc @shawkins

Copilot

Pull request overview

This PR updates owner-reference-based secondary-to-primary mapping so events still propagate when the owner reference points to an older version of the same API group (e.g., apps/v1beta1 vs apps/v1), by matching on group instead of full apiVersion.

Changes:

Update Mappers.fromOwnerReferences(...) to match owner references by kind + API group (ignoring version).
Add ReconcilerUtilsInternal.getGroup(String apiVersion) helper to extract the API group from an apiVersion string.
Add a unit test validating group extraction behavior.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
`operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/Mappers.java`	Relaxes owner-reference matching from exact `apiVersion` to API group comparison.
`operator-framework-core/src/main/java/io/javaoperatorsdk/operator/ReconcilerUtilsInternal.java`	Introduces `getGroup` helper for parsing API group from `apiVersion`.
`operator-framework-core/src/test/java/io/javaoperatorsdk/operator/ReconcilerUtilsInternalTest.java`	Adds coverage for `getGroup` parsing behavior.

Copilot · 2026-04-20T10:29:01Z

  public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromOwnerReferences(
      String apiVersion, String kind, boolean clusterScope) {
-    String correctApiVersion = apiVersion.startsWith("/") ? apiVersion.substring(1) : apiVersion;
    return resource ->
        resource.getMetadata().getOwnerReferences().stream()
-            .filter(r -> r.getKind().equals(kind) && r.getApiVersion().equals(correctApiVersion))
+            .filter(
+                r ->
+                    r.getKind().equals(kind)
+                        && getGroup(r.getApiVersion()).equals(getGroup(apiVersion)))
            .map(or -> ResourceID.fromOwnerReference(resource, or, clusterScope))
            .collect(Collectors.toSet());


This change intentionally relaxes ownerReference matching from full apiVersion to just group. Please add a focused unit test in the existing MappersTest that demonstrates the new behavior (same group + kind but different versions should still map), so future refactors don’t accidentally revert to strict apiVersion matching.

csviri · 2026-04-20T10:46:38Z

see also: keycloak/keycloak#48030 (comment)

In case of a resource is has a new version, but owner reference still points to the old version we should still propapage the event. Signed-off-by: Attila Mészáros <a_meszaros@apple.com>

Signed-off-by: Attila Mészáros <a_meszaros@apple.com>

metacosm · 2026-04-20T11:33:46Z

+  @Test
+  void extractsGroupFromApiVersion() {
+    assertThat(getGroup("v1")).isEqualTo("");
+    assertThat(getGroup("/v1")).isEqualTo("");


This isn't a valid apiVersion String

see conversation here: #3302 (comment)

metacosm · 2026-04-20T11:34:52Z

+    assertThat(getGroup("v1")).isEqualTo("");
+    assertThat(getGroup("/v1")).isEqualTo("");
+    assertThat(getGroup("apps/v1")).isEqualTo("apps");
+    assertThat(getGroup("/apps/v1")).isEqualTo("apps");


This isn't a valid apiVersion so I don't think we should cover this case, apart maybe to throw an IllegalArgumentException

I made this so we are backwards compatible with the current implementation marked you on the related code part. But I tend to agree that we should make it throw such exception in next minor version.

metacosm · 2026-04-20T11:37:55Z

+            .filter(
+                r ->
+                    r.getKind().equals(kind)
+                        && getGroup(r.getApiVersion()).equals(getGroup(apiVersion)))


I'm not sure this is the correct behavior: shouldn't there be a conversion hook instead? It seems dangerous to potentially match resources with differing versions…

Conversion hooks, does not update the owner references of the resource. Maybe we should have an integration test also this, to see how exactly and what happens. I will add one.

metacosm · 2026-04-20T11:39:06Z

    return v1Length;
  }
+
+  public static String getGroup(String apiVersion) {


This method should probably eventually move to the client code itself

csviri · 2026-04-20T12:02:59Z


  public static <T extends HasMetadata> SecondaryToPrimaryMapper<T> fromOwnerReferences(
      String apiVersion, String kind, boolean clusterScope) {
-    String correctApiVersion = apiVersion.startsWith("/") ? apiVersion.substring(1) : apiVersion;


cc @metacosm see this part. Also if you check the related commit, that explains the issue here.

csviri · 2026-04-20T12:24:22Z

Maybe would be more pc to target next with this PR.

xstefank

I don't have anything else to add to what was already mentioned.

Copilot AI review requested due to automatic review settings April 20, 2026 10:25

openshift-ci bot requested review from metacosm and xstefank April 20, 2026 10:25

Copilot started reviewing on behalf of csviri April 20, 2026 10:25 View session

csviri mentioned this pull request Apr 20, 2026

Update to operator version 26.6.0 needs deletion of all objects keycloak/keycloak#48030

Closed

2 tasks

Copilot AI reviewed Apr 20, 2026

View reviewed changes

csviri added 2 commits April 20, 2026 12:46

improve: owner reference mappers checks only group not apiVersion

d36c058

In case of a resource is has a new version, but owner reference still points to the old version we should still propapage the event. Signed-off-by: Attila Mészáros <a_meszaros@apple.com>

wip

e57272d

Signed-off-by: Attila Mészáros <a_meszaros@apple.com>

csviri force-pushed the owner-ref-mapper branch from 0345443 to e57272d Compare April 20, 2026 10:46

csviri requested a review from shawkins April 20, 2026 10:46

metacosm reviewed Apr 20, 2026

View reviewed changes

csviri commented Apr 20, 2026

View reviewed changes

vmuzikar mentioned this pull request Apr 20, 2026

fix: ensures that owner references are updated after an upgrade keycloak/keycloak#48213

Merged

shawkins mentioned this pull request Apr 20, 2026

Remove operator version tolerant workaround keycloak/keycloak#48282

Open

xstefank approved these changes Apr 21, 2026

View reviewed changes

Conversation

csviri commented Apr 20, 2026

Uh oh!

csviri commented Apr 20, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

csviri commented Apr 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

csviri commented Apr 20, 2026

Uh oh!

xstefank left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants