Skip to content

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

Draft
csviri wants to merge 1 commit into
operator-framework:mainfrom
csviri:fix/bounded-cache-eager-orelse
Draft

fix: do not eagerly evaluate the cluster-scoped fallback in KubernetesResourceFetcher#3527
csviri wants to merge 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());
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants