Skip to content

fix: do not eagerly evaluate the cluster-scoped fallback in KubernetesResourceFetcher - #3527

Merged
csviri merged 1 commit into
operator-framework:mainfrom
csviri:fix/bounded-cache-eager-orelse
Aug 3, 2026
Merged

fix: do not eagerly evaluate the cluster-scoped fallback in KubernetesResourceFetcher#3527
csviri merged 1 commit into
operator-framework:mainfrom
csviri:fix/bounded-cache-eager-orelse

Conversation

@csviri

@csviri csviri commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

fetchResource selected between a namespaced and a cluster-scoped lookup
with Optional.orElse:

return resourceId.getNamespace()
    .map(ns -> client.resources(rClass).inNamespace(ns).withName(name).get())
    .orElse(client.resources(rClass).withName(name).get());

orElse evaluates its argument unconditionally, so every namespaced
lookup also performed the cluster-scoped GET. For a namespaced resource
type that second request either queries whichever namespace the client
happens to default to or fails outright, and its result is then thrown
away.

This runs on the BoundedItemStore cache-miss path
(refreshMissingStateFromServer), so it doubles the API server requests
for every bounded-cache refresh of a namespaced resource.

Switches to orElseGet so the fallback is only evaluated for
cluster-scoped resource ids.

Part of #3517

…sResourceFetcher

`fetchResource` selected between a namespaced and a cluster-scoped lookup
with `Optional.orElse`:

    return resourceId.getNamespace()
        .map(ns -> client.resources(rClass).inNamespace(ns).withName(name).get())
        .orElse(client.resources(rClass).withName(name).get());

`orElse` evaluates its argument unconditionally, so every namespaced
lookup also performed the cluster-scoped GET. For a namespaced resource
type that second request either queries whichever namespace the client
happens to default to or fails outright, and its result is then thrown
away.

This runs on the `BoundedItemStore` cache-miss path
(`refreshMissingStateFromServer`), so it doubles the API server requests
for every bounded-cache refresh of a namespaced resource.

Switches to `orElseGet` so the fallback is only evaluated for
cluster-scoped resource ids.
Copilot AI review requested due to automatic review settings July 30, 2026 09:05
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 30, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates KubernetesResourceFetcher in operator-framework-core to avoid eagerly executing a cluster-scoped fallback lookup when fetching resources, reducing unnecessary Kubernetes API calls on the bounded-cache refresh path.

Changes:

  • Replaces Optional.orElse(...) with Optional.orElseGet(...) so the fallback GET is evaluated lazily.
  • Keeps the existing namespaced-vs-cluster-scoped selection logic based on presence of a namespace in the ResourceID.
Comments suppressed due to low confidence (1)

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/cache/KubernetesResourceFetcher.java:48

  • Optional.map(...) becomes empty when the mapper returns null (it wraps via ofNullable). That means if the resource ID is namespaced but the namespaced GET returns null (resource deleted / not found), this code will still execute the fallback cluster-scoped/default-namespace GET and may return/cache the wrong resource under the namespaced key. The fallback should only run when the namespace is absent, not when a namespaced lookup returns null.
    return resourceId
        .getNamespace()
        .map(ns -> client.resources(rClass).inNamespace(ns).withName(resourceId.getName()).get())
        .orElseGet(() -> client.resources(rClass).withName(resourceId.getName()).get());

Comment on lines 43 to +48
public R fetchResource(String key) {
var resourceId = resourceIDFunction.apply(key);
return resourceId
.getNamespace()
.map(ns -> client.resources(rClass).inNamespace(ns).withName(resourceId.getName()).get())
.orElse(client.resources(rClass).withName(resourceId.getName()).get());
.orElseGet(() -> client.resources(rClass).withName(resourceId.getName()).get());

@csviri csviri left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@csviri
csviri marked this pull request as ready for review August 3, 2026 08:16
@openshift-ci openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 3, 2026
@openshift-ci
openshift-ci Bot requested review from metacosm and xstefank August 3, 2026 08:16
@csviri
csviri merged commit 6ff8b15 into operator-framework:main Aug 3, 2026
28 checks passed
csviri added a commit to csviri/java-operator-sdk that referenced this pull request Aug 3, 2026
…sResourceFetcher (operator-framework#3527)

`fetchResource` selected between a namespaced and a cluster-scoped lookup
with `Optional.orElse`:

    return resourceId.getNamespace()
        .map(ns -> client.resources(rClass).inNamespace(ns).withName(name).get())
        .orElse(client.resources(rClass).withName(name).get());

`orElse` evaluates its argument unconditionally, so every namespaced
lookup also performed the cluster-scoped GET. For a namespaced resource
type that second request either queries whichever namespace the client
happens to default to or fails outright, and its result is then thrown
away.

This runs on the `BoundedItemStore` cache-miss path
(`refreshMissingStateFromServer`), so it doubles the API server requests
for every bounded-cache refresh of a namespaced resource.

Switches to `orElseGet` so the fallback is only evaluated for
cluster-scoped resource ids.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants