build: add SpotBugs plugin and fix all reported findings by csviri · Pull Request #3390 · operator-framework/java-operator-sdk · GitHub

Add the SpotBugs Maven plugin (effort=Max, threshold=Medium) with a
check execution bound to the verify phase across all modules, plus a
documented spotbugs-exclude.xml for intentional/false-positive findings.

Real bug fixes:

ExecutorServiceManager: synchronize start()/stop() so started and
configurationService are accessed consistently with the already-locked
lazyInitWorkflowExecutorService() (IS2_INCONSISTENT_SYNC,
AT_STALE_THREAD_WRITE_OF_PRIMITIVE).
AbstractEventSourceHolderDependentResource: make isCacheFillerEventSource
volatile (written under lock, read from reconcile threads).
ManagedInformerEventSource: make cache and controllerConfiguration
volatile (assigned under lock in start(), read lock-free).
AbstractOperatorExtension: the builder's namespaceDeleteTimeout was
configurable but never plumbed into the extension, so the namespace
deletion timeout always used the default. Threaded it through the
constructors of both LocallyRunOperatorExtension and
ClusterDeployedOperatorExtension.
MicrometerMetrics.addMetadataTags(): guard against null metadata, which
incrementCounter() already tolerates (latent NPE, NP_NULL_PARAM_DEREF).
Bootstrapper.addTemplatedFile(): close the Writer via try-with-resources
and write as UTF-8 (OBL_UNSATISFIED_OBLIGATION, DM_DEFAULT_ENCODING).

Default-encoding fixes (DM_DEFAULT_ENCODING), all pinned to UTF-8:

LocallyRunOperatorExtension CRD apply/delete byte conversions.
AccumulativeMappingWriter reader and PrintWriter.
ClassMappingProvider InputStreamReader.
sample mysql-schema Secret/Schema Base64 encode/decode.
sample tomcat-operator WebappReconciler ByteArrayOutputStream.toString().

Sample cleanups:

TomcatReconciler: avoid unbox-then-rebox in a log argument
(BX_UNBOXING_IMMEDIATELY_REBOXED).
OperationsSampleOperator: make the config path overridable via the
CONFIG_PATH env var instead of a hardcoded absolute path
(DMI_HARDCODED_ABSOLUTE_FILENAME).

Suppressed in spotbugs-exclude.xml (intentional / false positives):
EI_EXPOSE_REP(2) and CT_CONSTRUCTOR_THROW project-wide, plus DM_EXIT in
LeaderElectionManager, NP_BOOLEAN_RETURN_NULL in BooleanWithUndefined,
EQ_DOESNT_OVERRIDE_EQUALS in GroupVersionKindPlural, and
SING_SINGLETON_HAS_NONPRIVATE_CONSTRUCTOR in
SSABasedGenericKubernetesResourceMatcher and ConfigLoader.

Co-Authored-By: Claude Opus 4.8 (1M context) noreply@anthropic.com

Pull request overview

This PR introduces SpotBugs to the Maven build (run on verify) and addresses the resulting findings across core framework code, the JUnit extensions, the bootstrapper plugin, and samples—primarily around concurrency visibility/synchronization and deterministic UTF-8 encoding.

Changes:

Add and configure the SpotBugs Maven plugin (plus a repository-wide spotbugs-exclude.xml for intentional/false positives).
Fix concurrency/visibility issues (e.g., synchronized lifecycle methods and volatile fields) and a latent NPE in Micrometer tag handling.
Pin default-encoding-sensitive I/O to UTF-8 and close resources correctly (try-with-resources).

Reviewed changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pom.xml	Adds and configures the SpotBugs Maven plugin + version management.
spotbugs-exclude.xml	Introduces a centralized SpotBugs exclude filter for known intentional/false positives.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ExecutorServiceManager.java	Synchronization changes for lifecycle methods to address SpotBugs concurrency findings.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractEventSourceHolderDependentResource.java	Marks a cross-thread-read flag as volatile for visibility.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java	Marks fields as volatile to ensure safe publication after start().
operator-framework-junit/src/main/java/io/javaoperatorsdk/operator/junit/AbstractOperatorExtension.java	Plumbs namespace deletion timeout through the base extension.
operator-framework-junit/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java	Plumbs timeout and pins CRD YAML byte conversions to UTF-8.
operator-framework-junit/src/main/java/io/javaoperatorsdk/operator/junit/ClusterDeployedOperatorExtension.java	Plumbs timeout through constructors/builders.
operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/ClassMappingProvider.java	Pins InputStreamReader decoding to UTF-8.
operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/AccumulativeMappingWriter.java	Pins reader/writer encoding to UTF-8 for generated mapping files.
micrometer-support/src/main/java/io/javaoperatorsdk/operator/monitoring/micrometer/MicrometerMetrics.java	Guards against null metadata when extracting GVK tags.
bootstrapper-maven-plugin/src/main/java/io/javaoperatorsdk/boostrapper/Bootstrapper.java	Writes templated files as UTF-8 and closes the writer via try-with-resources.
sample-operators/operations/src/main/java/io/javaoperatorsdk/operator/sample/operations/OperationsSampleOperator.java	Makes the sample config path overridable via CONFIG_PATH.
sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SecretDependentResource.java	Pins Base64 input bytes to UTF-8.
sample-operators/mysql-schema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SchemaDependentResource.java	Pins Base64 decode path to UTF-8.
sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/TomcatReconciler.java	Avoids unbox/rebox in log argument selection.
sample-operators/tomcat-operator/src/main/java/io/javaoperatorsdk/operator/sample/WebappReconciler.java	Pins ByteArrayOutputStream string conversion to UTF-8.

Comments suppressed due to low confidence (2)

operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ExecutorServiceManager.java:161

stop() is synchronized while it waits for executor termination, which can block in-flight reconciliation threads that try to call back into ExecutorServiceManager (e.g., via DefaultContext#getWorkflowExecutorService() -> workflowExecutorService() -> synchronized lazyInitWorkflowExecutorService()), preventing tasks from completing and delaying/shrugging shutdown. Avoid holding the monitor while awaiting termination by snapshotting state under synchronization, then performing shutdown outside the synchronized block.

  public synchronized void stop(Duration gracefulShutdownTimeout) {
    try {
      log.debug("Closing executor");
      var parallelExec = Executors.newFixedThreadPool(3);
      parallelExec.invokeAll(
          List.of(
              shutdown(executor, gracefulShutdownTimeout),
              shutdown(workflowExecutor, gracefulShutdownTimeout),
              shutdown(cachingExecutorService, gracefulShutdownTimeout)));
      workflowExecutor = null;
      parallelExec.shutdownNow();
      started = false;
    } catch (InterruptedException e) {
      log.debug("Exception closing executor: {}", e.getLocalizedMessage());
      Thread.currentThread().interrupt();
    }
  }

operator-framework/src/main/java/io/javaoperatorsdk/operator/config/runtime/AccumulativeMappingWriter.java:70

flush() manually closes the PrintWriter; if an exception occurs while iterating/writing, the writer may not be closed. Using try-with-resources makes the close unconditional and simplifies the method.

              new InputStreamReader(readonlyResource.openInputStream(), StandardCharsets.UTF_8))) {
        final var existingLines =
            bufferedReader
                .lines()
                .map(l -> l.split(","))
                .collect(Collectors.toMap(parts -> parts[0], parts -> parts[1]));
        mappings.putAll(existingLines);
      }
    } catch (IOException e) {
    }
    return this;
  }

  /** Add a new mapping */
  public AccumulativeMappingWriter add(String key, String value) {

Pull request overview

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

Add the SpotBugs Maven plugin (effort=Max, threshold=Medium) with a `check` execution bound to the `verify` phase across all modules, plus a documented `spotbugs-exclude.xml` for intentional/false-positive findings. Real bug fixes: - ExecutorServiceManager: synchronize start()/stop() so `started` and `configurationService` are accessed consistently with the already-locked lazyInitWorkflowExecutorService() (IS2_INCONSISTENT_SYNC, AT_STALE_THREAD_WRITE_OF_PRIMITIVE). - AbstractEventSourceHolderDependentResource: make `isCacheFillerEventSource` volatile (written under lock, read from reconcile threads). - ManagedInformerEventSource: make `cache` and `controllerConfiguration` volatile (assigned under lock in start(), read lock-free). - AbstractOperatorExtension: the builder's `namespaceDeleteTimeout` was configurable but never plumbed into the extension, so the namespace deletion timeout always used the default. Threaded it through the constructors of both LocallyRunOperatorExtension and ClusterDeployedOperatorExtension. - MicrometerMetrics.addMetadataTags(): guard against null `metadata`, which incrementCounter() already tolerates (latent NPE, NP_NULL_PARAM_DEREF). - Bootstrapper.addTemplatedFile(): close the Writer via try-with-resources and write as UTF-8 (OBL_UNSATISFIED_OBLIGATION, DM_DEFAULT_ENCODING). Default-encoding fixes (DM_DEFAULT_ENCODING), all pinned to UTF-8: - LocallyRunOperatorExtension CRD apply/delete byte conversions. - AccumulativeMappingWriter reader and PrintWriter. - ClassMappingProvider InputStreamReader. - sample mysql-schema Secret/Schema Base64 encode/decode. - sample tomcat-operator WebappReconciler ByteArrayOutputStream.toString(). Sample cleanups: - TomcatReconciler: avoid unbox-then-rebox in a log argument (BX_UNBOXING_IMMEDIATELY_REBOXED). - OperationsSampleOperator: make the config path overridable via the CONFIG_PATH env var instead of a hardcoded absolute path (DMI_HARDCODED_ABSOLUTE_FILENAME). Suppressed in spotbugs-exclude.xml (intentional / false positives): EI_EXPOSE_REP(2) and CT_CONSTRUCTOR_THROW project-wide, plus DM_EXIT in LeaderElectionManager, NP_BOOLEAN_RETURN_NULL in BooleanWithUndefined, EQ_DOESNT_OVERRIDE_EQUALS in GroupVersionKindPlural, and SING_SINGLETON_HAS_NONPRIVATE_CONSTRUCTOR in SSABasedGenericKubernetesResourceMatcher and ConfigLoader. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Attila Mészáros <a_meszaros@apple.com>

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

Copilot AI review requested due to automatic review settings May 29, 2026 14:14

openshift-ci Bot requested review from metacosm and xstefank May 29, 2026 14:15

Copilot started reviewing on behalf of csviri May 29, 2026 14:15 View session

csviri force-pushed the find-bugs branch from 3ef06fe to 17836d5 Compare May 29, 2026 14:18

Copilot AI reviewed May 29, 2026

View reviewed changes

Comment thread ...hema/src/main/java/io/javaoperatorsdk/operator/sample/dependent/SchemaDependentResource.java

csviri requested a review from Copilot May 29, 2026 14:35

Copilot started reviewing on behalf of csviri May 29, 2026 14:35 View session

Copilot AI reviewed May 29, 2026

View reviewed changes

metacosm approved these changes May 29, 2026

View reviewed changes

xstefank approved these changes Jun 1, 2026

View reviewed changes

Comment thread ...ns/src/main/java/io/javaoperatorsdk/operator/sample/operations/OperationsSampleOperator.java Outdated

csviri and others added 2 commits June 15, 2026 17:34

fix from code review

ab3ee97

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

csviri force-pushed the find-bugs branch from b06f750 to ab3ee97 Compare June 15, 2026 15:35

csviri added 2 commits June 15, 2026 17:39

fix IT

e84d7fc

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

fix from cr

d732378

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

csviri merged commit 2a3dff4 into operator-framework:main Jun 16, 2026
27 checks passed

build: add SpotBugs plugin and fix all reported findings by csviri · Pull Request #3390 · operator-framework/java-operator-sdk · GitHub

Uh oh!

build: add SpotBugs plugin and fix all reported findings - #3390

build: add SpotBugs plugin and fix all reported findings#3390
csviri merged 4 commits into
operator-framework:mainfrom
csviri:find-bugs

csviri commented May 29, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

build: add SpotBugs plugin and fix all reported findings by csviri · Pull Request #3390 · operator-framework/java-operator-sdk · GitHub

Uh oh!

build: add SpotBugs plugin and fix all reported findings - #3390

build: add SpotBugs plugin and fix all reported findings#3390csviri merged 4 commits intooperator-framework:mainoperator-framework/java-operator-sdk:mainfrom csviri:find-bugscsviri/java-operator-sdk:find-bugsCopy head branch name to clipboard

Conversation

csviri commented May 29, 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 left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

build: add SpotBugs plugin and fix all reported findings#3390
csviri merged 4 commits into
operator-framework:mainfrom
csviri:find-bugs