GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — fix: use the null-guarded metrics field in the Controller constructor by csviri · Pull Request #3531 · operator-framework/java-operator-sdk

Conversation

@csviri

@csviri csviri commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

The constructor deliberately guards against a null Metrics:

this.metrics = Optional.ofNullable(configurationService.getMetrics()).orElse(Metrics.NOOP);

but its last statement bypasses that guard and calls the getter again:

configurationService.getMetrics().controllerRegistered(this);

so a ConfigurationService whose getMetrics() returns null fails with a
NullPointerException during controller construction, which is exactly what
the guard three lines up is meant to prevent. EventProcessor applies the
same != null ? metrics : Metrics.NOOP guard, so the codebase does treat
a null Metrics as reachable even though the interface default returns
Metrics.NOOP.

Uses the already-resolved metrics field instead.

Part of #3517

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
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
@csviri csviri mentioned this pull request Jul 30, 2026
Correctness AI check: collection of possible issues and related PRs #3517
Closed
13 of 16 tasks
Copilot started reviewing on behalf of csviri July 30, 2026 09:06
View session
Copilot AI reviewed Jul 30, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Pull request overview

Fixes a controller-construction correctness bug in operator-framework-core where the constructor would bypass the null-guarded metrics field and re-fetch Metrics from ConfigurationService, causing a potential NullPointerException when getMetrics() returns null.

Changes:

  • Use the already null-guarded metrics field to call controllerRegistered(this) instead of calling configurationService.getMetrics() again.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java Resolved

Uh oh!

There was an error while loading. Please reload this page.
csviri
csviri commented Aug 3, 2026
View reviewed changes

@csviri csviri left a comment

Copy link
Copy Markdown
Collaborator Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@csviri
csviri marked this pull request as ready for review August 3, 2026 08:01
@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:02
@csviri
fix: use the null-guarded metrics field in the Controller constructor
24dc42b 
The constructor deliberately guards against a null `Metrics`:

    this.metrics = Optional.ofNullable(configurationService.getMetrics()).orElse(Metrics.NOOP);

but its last statement bypasses that guard and calls the getter again:

    configurationService.getMetrics().controllerRegistered(this);

so a `ConfigurationService` whose `getMetrics()` returns null fails with a
NullPointerException during controller construction, which is exactly what
the guard three lines up is meant to prevent. `EventProcessor` applies the
same `!= null ? metrics : Metrics.NOOP` guard, so the codebase does treat
a null `Metrics` as reachable even though the interface default returns
`Metrics.NOOP`.

Uses the already-resolved `metrics` field instead.
Copilot AI review requested due to automatic review settings August 3, 2026 08:02
@csviri
csviri force-pushed the fix/controller-null-metrics-guard branch from 40c0890 to 24dc42b Compare August 3, 2026 08:02
Copilot started reviewing on behalf of csviri August 3, 2026 08:02
View session
Copilot AI reviewed Aug 3, 2026
View reviewed changes

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Suppressed comments (1)

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java:141

  • Consider adding a regression unit test covering the original failure mode: a ConfigurationService implementation whose getMetrics() returns null should not throw during Controller construction (and should behave as if Metrics.NOOP was used). There are existing Controller tests in this module, but none exercise the null-metrics path.
    metrics.controllerRegistered(this);

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@csviri
csviri merged commit 7c8e197 into operator-framework:main Aug 3, 2026
28 checks passed

Uh oh!

There was an error while loading. Please reload this page.
csviri added a commit to csviri/java-operator-sdk that referenced this pull request Aug 3, 2026
@csviri
fix: use the null-guarded metrics field in the Controller constructor (…
9be2f33 
…operator-framework#3531)

The constructor deliberately guards against a null `Metrics`:

    this.metrics = Optional.ofNullable(configurationService.getMetrics()).orElse(Metrics.NOOP);

but its last statement bypasses that guard and calls the getter again:

    configurationService.getMetrics().controllerRegistered(this);

so a `ConfigurationService` whose `getMetrics()` returns null fails with a
NullPointerException during controller construction, which is exactly what
the guard three lines up is meant to prevent. `EventProcessor` applies the
same `!= null ? metrics : Metrics.NOOP` guard, so the codebase does treat
a null `Metrics` as reachable even though the interface default returns
`Metrics.NOOP`.

Uses the already-resolved `metrics` field instead.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

Copilot code review Copilot
Copilot review effort
Applies to this pull request for everyone.Learn more about Copilot code review.
Copilot left review comments
@metacosm metacosm Awaiting requested review from metacosm
@xstefank xstefank Awaiting requested review from xstefank

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

2 participants

@csviri
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.