Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the Maven project/module versions across Java Operator SDK and its submodules, presumably in preparation for the next development cycle.
Changes:
- Bumped the root project version in
pom.xml. - Updated all module
parentversion references to match the new root version. - Updated the standalone
operator-framework-bomartifact version.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 17 comments.
Show a summary per file
| File | Description |
|---|---|
| pom.xml | Updates root project version used across the multi-module build. |
| operator-framework-core/pom.xml | Aligns module parent version with the root project version. |
| operator-framework/pom.xml | Aligns module parent version with the root project version. |
| operator-framework-junit/pom.xml | Aligns module parent version with the root project version. |
| operator-framework-bom/pom.xml | Updates BOM artifact version to match the intended release line. |
| micrometer-support/pom.xml | Aligns module parent version with the root project version. |
| caffeine-bounded-cache-support/pom.xml | Aligns module parent version with the root project version. |
| bootstrapper-maven-plugin/pom.xml | Aligns module parent version with the root project version. |
| test-index-processor/pom.xml | Aligns module parent version with the root project version. |
| migration/pom.xml | Aligns module parent version with the root project version. |
| sample-operators/pom.xml | Aligns sample parent version with the root project version. |
| sample-operators/webpage/pom.xml | Aligns sample module parent version with the sample reactor version. |
| sample-operators/tomcat-operator/pom.xml | Aligns sample module parent version with the sample reactor version. |
| sample-operators/operations/pom.xml | Aligns sample module parent version with the sample reactor version. |
| sample-operators/mysql-schema/pom.xml | Aligns sample module parent version with the sample reactor version. |
| sample-operators/leader-election/pom.xml | Aligns sample module parent version with the sample reactor version. |
| sample-operators/controller-namespace-deletion/pom.xml | Aligns sample module parent version with the sample reactor version. |
| <groupId>io.javaoperatorsdk</groupId> | ||
| <artifactId>java-operator-sdk</artifactId> | ||
| <version>5.4.1-SNAPSHOT</version> | ||
| <version>999-SNAPSHOT</version> |
| <groupId>io.javaoperatorsdk</groupId> | ||
| <artifactId>java-operator-sdk</artifactId> | ||
| <version>5.4.1-SNAPSHOT</version> | ||
| <version>999-SNAPSHOT</version> |
| <groupId>io.javaoperatorsdk</groupId> | ||
| <artifactId>java-operator-sdk</artifactId> | ||
| <version>5.4.1-SNAPSHOT</version> | ||
| <version>999-SNAPSHOT</version> |
| <groupId>io.javaoperatorsdk</groupId> | ||
| <artifactId>java-operator-sdk</artifactId> | ||
| <version>5.4.1-SNAPSHOT</version> | ||
| <version>999-SNAPSHOT</version> |
| <groupId>io.javaoperatorsdk</groupId> | ||
| <artifactId>operator-framework-bom</artifactId> | ||
| <version>5.4.1-SNAPSHOT</version> | ||
| <version>999-SNAPSHOT</version> |
| <groupId>io.javaoperatorsdk</groupId> | ||
| <artifactId>sample-operators</artifactId> | ||
| <version>5.4.1-SNAPSHOT</version> | ||
| <version>999-SNAPSHOT</version> |
| <groupId>io.javaoperatorsdk</groupId> | ||
| <artifactId>sample-operators</artifactId> | ||
| <version>5.4.1-SNAPSHOT</version> | ||
| <version>999-SNAPSHOT</version> |
| <groupId>io.javaoperatorsdk</groupId> | ||
| <artifactId>sample-operators</artifactId> | ||
| <version>5.4.1-SNAPSHOT</version> | ||
| <version>999-SNAPSHOT</version> |
| <groupId>io.javaoperatorsdk</groupId> | ||
| <artifactId>sample-operators</artifactId> | ||
| <version>5.4.1-SNAPSHOT</version> | ||
| <version>999-SNAPSHOT</version> |
| <groupId>io.javaoperatorsdk</groupId> | ||
| <artifactId>sample-operators</artifactId> | ||
| <version>5.4.1-SNAPSHOT</version> | ||
| <version>999-SNAPSHOT</version> |
xstefank
approved these changes
Jul 3, 2026
openshift-ci
Bot
added
the
needs-rebase
openshift-ci
Bot
removed
the
needs-rebase
Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (17)
pom.xml:24
- The project version was changed to
999-SNAPSHOT, which will publish/build artifacts under an unexpected version and doesn't align with the stated v5.5 line. Revert to the previous snapshot version (or the intended next v5.5.x version).
<version>999-SNAPSHOT</version>
operator-framework-core/pom.xml:24
- The parent version was changed to
999-SNAPSHOT, which will make this module resolve against an unexpected parent version. Revert to the previous snapshot version (or the intended next v5.5.x version).
<version>999-SNAPSHOT</version>
operator-framework/pom.xml:24
- The parent version was changed to
999-SNAPSHOT, which will make this module resolve against an unexpected parent version. Revert to the previous snapshot version (or the intended next v5.5.x version).
<version>999-SNAPSHOT</version>
operator-framework-junit/pom.xml:24
- The parent version was changed to
999-SNAPSHOT, which will make this module resolve against an unexpected parent version. Revert to the previous snapshot version (or the intended next v5.5.x version).
<version>999-SNAPSHOT</version>
operator-framework-bom/pom.xml:24
- The BOM version was changed to
999-SNAPSHOT, which will publish an unexpected BOM coordinate/version. Revert to the previous snapshot version (or the intended next v5.5.x version).
<version>999-SNAPSHOT</version>
migration/pom.xml:24
- The parent version was changed to
999-SNAPSHOT, which will make this module resolve against an unexpected parent version. Revert to the previous snapshot version (or the intended next v5.5.x version).
<version>999-SNAPSHOT</version>
micrometer-support/pom.xml:24
- The parent version was changed to
999-SNAPSHOT, which will make this module resolve against an unexpected parent version. Revert to the previous snapshot version (or the intended next v5.5.x version).
<version>999-SNAPSHOT</version>
caffeine-bounded-cache-support/pom.xml:24
- The parent version was changed to
999-SNAPSHOT, which will make this module resolve against an unexpected parent version. Revert to the previous snapshot version (or the intended next v5.5.x version).
<version>999-SNAPSHOT</version>
bootstrapper-maven-plugin/pom.xml:25
- The parent version was changed to
999-SNAPSHOT, which will make this plugin resolve against an unexpected parent version. Revert to the previous snapshot version (or the intended next v5.5.x version).
<version>999-SNAPSHOT</version>
test-index-processor/pom.xml:25
- The parent version was changed to
999-SNAPSHOT, which will make this module resolve against an unexpected parent version. Revert to the previous snapshot version (or the intended next v5.5.x version).
<version>999-SNAPSHOT</version>
sample-operators/pom.xml:25
- The parent version was changed to
999-SNAPSHOT, which will make the samples aggregator resolve against an unexpected parent version. Revert to the previous snapshot version (or the intended next v5.5.x version).
<version>999-SNAPSHOT</version>
sample-operators/webpage/pom.xml:25
- The parent version was changed to
999-SNAPSHOT, which will make this sample resolve against an unexpected parent version. Revert to the previous snapshot version (or the intended next v5.5.x version).
<version>999-SNAPSHOT</version>
sample-operators/tomcat-operator/pom.xml:25
- The parent version was changed to
999-SNAPSHOT, which will make this sample resolve against an unexpected parent version. Revert to the previous snapshot version (or the intended next v5.5.x version).
<version>999-SNAPSHOT</version>
sample-operators/operations/pom.xml:25
- The parent version was changed to
999-SNAPSHOT, which will make this sample resolve against an unexpected parent version. Revert to the previous snapshot version (or the intended next v5.5.x version).
<version>999-SNAPSHOT</version>
sample-operators/mysql-schema/pom.xml:25
- The parent version was changed to
999-SNAPSHOT, which will make this sample resolve against an unexpected parent version. Revert to the previous snapshot version (or the intended next v5.5.x version).
<version>999-SNAPSHOT</version>
sample-operators/leader-election/pom.xml:25
- The parent version was changed to
999-SNAPSHOT, which will make this sample resolve against an unexpected parent version. Revert to the previous snapshot version (or the intended next v5.5.x version).
<version>999-SNAPSHOT</version>
sample-operators/controller-namespace-deletion/pom.xml:25
- The parent version was changed to
999-SNAPSHOT, which will make this sample resolve against an unexpected parent version. Revert to the previous snapshot version (or the intended next v5.5.x version).
<version>999-SNAPSHOT</version>
…us (#3480) Add integration tests that showcases handling explicit state in status. Both for simple managed/secondary resources and using a dependent resource. Signed-off-by: Attila Mészáros <a_meszaros@apple.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (3)
pom.xml:24
- Project version is set to
999-SNAPSHOT, which doesn’t align with the PR title targeting v5.6 and looks like a placeholder version. This will publish artifacts under an unexpected coordinate; set the intended next snapshot version (e.g.,5.6.0-SNAPSHOT) and keep all module parent versions consistent with it.
<version>999-SNAPSHOT</version>
operator-framework/pom.xml:24
- Module parent version is set to
999-SNAPSHOT. If the intent is v5.6, this should match the root project version (e.g.,5.6.0-SNAPSHOT) to avoid producing/publishing modules under an unexpected version.
<version>999-SNAPSHOT</version>
operator-framework-bom/pom.xml:24
- BOM version is set to
999-SNAPSHOT, which appears to be a placeholder and doesn’t match the PR’s v5.6 intent. Use the intended snapshot version (e.g.,5.6.0-SNAPSHOT) so consumers don’t import an unexpected BOM version.
<version>999-SNAPSHOT</version>
* feat: pool informers so controllers and event sources can share them Every InformerEventSource used to create its own SharedIndexInformer, so an operator whose controllers all watch the same secondary type - ConfigMap and Secret being the usual suspects - opened one watch connection and kept one cache per controller for the very same resources. Informers are now handed out by an InformerPool obtained from the ConfigurationService, keyed by an InformerClassifier. Event sources whose classifiers are equal are backed by one informer; the pool reference counts its users and stops the informer once the last one releases it. The classifier is made up of everything that decides what an informer watches and how: the KubernetesClient instance (compared by identity, since two clients for the same API server may still differ in credentials, impersonation or TLS material), the resource class or the group/version/kind for generic resources, the namespace, the label, field and shard selectors, and the item store. Two components are deliberately not part of that identity. The informerListLimit is excluded, so event sources that disagree only on it still share an informer, keeping the limit of whichever one created it and logging a warning. Indexers are excluded because they can be added to a running informer: they are registered under a name qualified with the controller and event source that added them, so index names stay private to an event source while callers keep using their own names, and they are removed again when that event source releases the informer. Two strategies ship: DefaultInformerPool shares as described and is the default, NonSharingInformerPool creates a dedicated informer per event source for anyone wanting to opt out. Either is selected with ConfigurationServiceOverrider#withInformerPool, and a custom strategy extends AbstractInformerPool, which already creates the informers from a classifier, starts them and waits for their caches to sync, leaving the subclass only the question of whether and when an informer is shared. Consequently informer creation and startup moved out of InformerWrapper and InformerManager into the pool, InformerManager acquires and releases informers instead of owning them, and it removes its own event handler and indexers from an informer that keeps running for others. An event source registered dynamically against an already running shared informer needs no special handling: the client replays the cache contents to a newly added handler. Also in support of the above: ConfigurationService#informerPool, an InformerEventSource constructor that no longer needs an EventSourceContext (the one taking it is deprecated), the resource group/version/kind on InformerConfiguration, equality and toString on FieldSelector, and equality of GroupVersionKindPlural made consistent with its hashCode so that an unspecified plural no longer splits informers. The pooling itself is production ready; the configuration API around it is marked experimental and may still change. Covered by unit tests for the pools, the classifier, the wrapper and the manager, and by integration tests for sharing, dynamic registration and de-registration that each run against both strategies. Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
| private final AtomicBoolean blockNextRelease = new AtomicBoolean(true); | ||
|
|
||
| @Override | ||
| public <R extends HasMetadata> Optional<SharedIndexInformer<R>> releaseInformer( |
| } | ||
|
|
||
| @Override | ||
| public <R extends HasMetadata> Optional<SharedIndexInformer<R>> releaseInformer( |
| } | ||
|
|
||
| @Override | ||
| public InformerPool informerPool() { |
Comment on lines
+65
to
+67
| final var configurationService = | ||
| ConfigurationService.newOverriddenConfigurationService( | ||
| this.configurationService, o -> o.withKubernetesClient(client)); |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 88 out of 88 changed files in this pull request and generated no new comments.
Suppressed comments (3)
pom.xml:25
- The project version is set to
999-SNAPSHOT, which doesn’t match the PR title indicating a v5.6 branch and will produce artifacts with an unexpected coordinate. If this is intended to be the next development snapshot, use a5.6.x-SNAPSHOTversion (and keep all module parent versions aligned).
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPool.java:34 - Use the generic diamond operator here to avoid a raw
ConcurrentHashMap(this currently forces@SuppressWarnings({"unchecked", "rawtypes"})to cover more than necessary).
private final Map<ClassifierWithName, SharedIndexInformer> informers = new ConcurrentHashMap();
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java:502
ConfigurationService#informerPool()currently returns a newDefaultInformerPoolinstance on every call and doesn’t callsetConfigurationService(this), which violates the method’s own contract (“same instance on every call”) and can disable cross-controller informer sharing (or break reference counting) for implementations that don’t override it. Consider caching and initializing the pool in the default implementation (or make it non-default and require implementations to provide a cached pool).
@Experimental(
"Only the configuration API around informer pooling could still change in a"
+ " non-backwards-compatible way, the pooling itself is prod ready.")
default InformerPool informerPool() {
return new DefaultInformerPool();
Bumps [org.apache.maven.plugins:maven-jar-plugin](https://github.com/apache/maven-jar-plugin) from 3.5.0 to 3.5.1. - [Release notes](https://github.com/apache/maven-jar-plugin/releases) - [Commits](apache/maven-jar-plugin@maven-jar-plugin-3.5.0...maven-jar-plugin-3.5.1) --- updated-dependencies: - dependency-name: org.apache.maven.plugins:maven-jar-plugin dependency-version: 3.5.1 dependency-type: direct:production update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps `openrewrite.version` from 8.87.5 to 8.87.7. Updates `org.openrewrite:rewrite-java` from 8.87.5 to 8.87.7 - [Release notes](https://github.com/openrewrite/rewrite/releases) - [Commits](openrewrite/rewrite@v8.87.5...v8.87.7) Updates `org.openrewrite:rewrite-maven` from 8.87.5 to 8.87.7 - [Release notes](https://github.com/openrewrite/rewrite/releases) - [Commits](openrewrite/rewrite@v8.87.5...v8.87.7) Updates `org.openrewrite:rewrite-test` from 8.87.5 to 8.87.7 - [Release notes](https://github.com/openrewrite/rewrite/releases) - [Commits](openrewrite/rewrite@v8.87.5...v8.87.7) Updates `org.openrewrite:rewrite-java-17` from 8.87.5 to 8.87.7 - [Release notes](https://github.com/openrewrite/rewrite/releases) - [Commits](openrewrite/rewrite@v8.87.5...v8.87.7) --- updated-dependencies: - dependency-name: org.openrewrite:rewrite-java dependency-version: 8.87.7 dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.openrewrite:rewrite-maven dependency-version: 8.87.7 dependency-type: direct:production update-type: version-update:semver-patch - dependency-name: org.openrewrite:rewrite-test dependency-version: 8.87.7 dependency-type: direct:development update-type: version-update:semver-patch - dependency-name: org.openrewrite:rewrite-java-17 dependency-version: 8.87.7 dependency-type: direct:development update-type: version-update:semver-patch ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…8.0 to 3.9.0 (#3515) Signed-off-by: dependabot[bot] <support@github.com>
) Fabric8's no-context patch() fetches the resource from the server and computes an RFC 6902 JSON Patch diff, which removes the fields omitted from a partial resource (including the whole spec). Pass an explicit JSON_MERGE PatchContext so the non-status merge patch operations really send application/merge-patch+json.
…ator E2E test (#3513) Adds a Maven-compiled Kotlin test source set to operator-framework-core and a smoke test verifying that a checked (non-RuntimeException) Exception thrown from a Kotlin DependentResource is properly caught and reported by the workflow executor, so that retries are triggered as expected. Kotlin does not have checked exceptions, so Kotlin code can throw a checked Exception from an overridden method without declaring it, even though the Java DependentResource#reconcile signature does not declare `throws Exception`. Before #2965 this exception would not have been caught by NodeExecutor, since it only handled RuntimeException, silently swallowing the error and preventing retries. Closes #2967 * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * fix: close class brace in Kotlin test; add Kotlin sample operator E2E Fixes a missing closing brace left in KotlinCheckedExceptionDependentResourceTest.kt. Adds a new sample-operators/kotlin-operator module: a minimalist Kotlin operator (ConfigMapCopyReconciler) that copies a value from a custom resource's spec into a ConfigMap, with a dual-mode (local/remote) E2E test mirroring the other samples. This verifies deserialization via the fabric8 client and the full reconciliation runtime when both the CR and reconciler are implemented in Kotlin, addressing the maintainer's request for broader coverage than the existing unit-level smoke test. Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
Signed-off-by: dependabot[bot] <support@github.com>
Signed-off-by: Attila Mészáros <a_meszaros@apple.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This needed to be fixed after rebase. Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
openshift-ci
Bot
added
the
needs-rebase
|
PR needs rebase. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 106 out of 106 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
pom.xml:25
- The root project version is set to
999-SNAPSHOT, which doesn’t match the PR title intent ("next (v5.6; …)") and will cause all produced artifacts to use an obviously placeholder version. Please set this to the intended next snapshot version and keep it consistent across all module parent references updated in this PR.
operator-framework-core/pom.xml:26 - This module now points at parent version
999-SNAPSHOT, which looks like a placeholder and will prevent aligning the module with the actual reactor parent version intended for the v5.6 line. Please update to the real parent snapshot version (and keep consistent with the rootpom.xml).
<parent>
<groupId>io.javaoperatorsdk</groupId>
<artifactId>java-operator-sdk</artifactId>
<version>999-SNAPSHOT</version>
<relativePath>../pom.xml</relativePath>
</parent>
sample-operators/pom.xml:26
- The sample-operators parent version is set to
999-SNAPSHOT, which appears to be a placeholder and should match the actual parent version used by the root project for the intended release line.
This is expected for a pool to set. Event if the underlying implementation would set it, this is the correct way to handle it. Signed-off-by: Attila Mészáros <a_meszaros@apple.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 106 out of 106 changed files in this pull request and generated no new comments.
Suppressed comments (2)
pom.xml:24
- Setting the root project version to
999-SNAPSHOTlooks like a placeholder rather than a real next development version. This will change published Maven coordinates (including the BOM) and can break downstream consumers and release tooling; if the intent is a regular version bump, it should use the correct semantic version and remain consistent across all child POM parent versions.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java:491 - The Javadoc requires
informerPool()to return the same instance on every call, but the currentdefaultimplementation constructs a newDefaultInformerPooleach time. AnyConfigurationServiceimplementation relying on the default method will silently disable informer sharing and can break reference-counting / release symmetry across different call sites.
* <p><strong>Implementations must return the same instance on every call.</strong> The pool is
* effectively a per-{@code ConfigurationService} singleton: controllers share informers only if
* they resolve the same pool, and reference counting / informer shutdown are only correct if
* {@code getInformer} and {@code releaseInformer} operate on that same instance. This is
* intentionally not a {@code default} method, since a {@code default} could not cache the result
AbstractWorkflowExecutor called Workflow#getDependentResourcesByName purely to read its size. That allocates a HashMap and walks every node to collect the dependent resources, then discards the map. The executor is constructed on every reconcile and cleanup of a workflow-based reconciler, so use the existing Workflow#size instead.
📝 WalkthroughWalkthroughThe change adds configurable informer pooling, opt-in API-version change detection, a Kotlin operator sample, framework behavior corrections, integration tests, documentation, and build updates. ChangesFramework and informer pooling
Kotlin operator sample
Repository support updates
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant ManagedInformerEventSource
participant InformerManager
participant InformerPool
participant SharedIndexInformer
ManagedInformerEventSource->>InformerManager: start event source
InformerManager->>InformerPool: acquire informer by classifier
InformerPool->>SharedIndexInformer: create or reuse informer
InformerManager->>InformerPool: start and await cache sync
ManagedInformerEventSource->>InformerManager: stop or change namespace
InformerManager->>InformerPool: release informer
InformerPool->>SharedIndexInformer: stop after final release
Suggested reviewers: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
⚔️ Resolve merge conflicts 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
…3550) ResourceOperations#desiredForJsonPatch takes an Options argument that it never reads - it only clones the actual resource and applies the operator. All six jsonPatch call sites thread the value in for nothing, which suggests the method still honours the option. Remove the parameter.
No behavior change; each site is replaced by a helper that already exists. - PrimaryUpdateAndCacheUtils#compareResourceVersions (and its private validateResourceVersion) duplicated the entire algorithm of ReconcilerUtilsInternal#validateAndCompareResourceVersions: the length-first compare, the empty check, the leading-zero check and the same exception messages. It had no production caller, so the copy could silently drift from the version all production paths use. Delegate instead. - addFinalizerWithSSA builds its bare SSA skeleton with HasMetadata#initNameAndNamespaceFrom, which ResourceOperations already uses for the same purpose and which is Namespaced-aware. - AbstractInformerPool formats the informer identifier with ReconcilerUtilsInternal#getResourceTypeNameWithVersion instead of concatenating the resource name and version by hand. - EventFilterWindow uses ExtendedResourceEvent#getResourceVersion, which had no callers even though it is exactly the expression used here. - LocallyRunOperatorExtension instantiates reconcilers with Utils#instantiate, so it also supports non-public no-arg constructors and reports the failing class instead of wrapping in a bare RuntimeException.
…3543) The event count is increased at the beginning of the reconciliation, thus waiting for it released the test into the middle of a reconciliation that was still about to remove the finalizer. The subsequent update then raced with that removal and failed with a conflict. Wait for the finalizer removal to actually land, and retry the (optimistically locked) update with a fresh read on conflict.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java (1)
111-136: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy liftSerialize namespace additions with shutdown.
stop()can complete afterchangeNamespaces()removes existing sources but before it inserts a new source. The new source then acquires and starts an informer after shutdown, with no owner left to release it.Two concurrent
changeNamespaces()calls can also both observe a missing namespace. Each call acquires a pool reference, butsources.put()retains only one wrapper.Guard source lookup, acquisition, insertion, and shutdown with one lifecycle lock and a stopped state. Add a race test for
stop()concurrent with an added namespace.Also applies to: 176-205
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java` around lines 111 - 136, Update InformerManager.changeNamespaces and stop to use one shared lifecycle lock and stopped state. Under that lock, serialize source lookup, acquisition, insertion, removal, and shutdown so additions cannot start after stop and concurrent namespace changes cannot acquire duplicate sources; ensure additions are rejected or released once stopped. Add a race test covering stop() concurrent with adding a namespace.
🧹 Nitpick comments (7)
operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusIT.java (1)
76-84: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove assertion-restating comments.
The assertions already state the required behavior. Keep comments only for very long or complex logic.
operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusIT.java#L76-L84: Remove the comments before the size and status-ID assertions.operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusWorkflowIT.java#L83-L93: Remove the equivalent comments before the size and status-ID assertions.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusIT.java` around lines 76 - 84, Remove the assertion-restating comments in ExternalStateInStatusIT.java (lines 76-84) and ExternalStateInStatusWorkflowIT.java (lines 83-93), specifically those preceding the resource-size and status-ID assertions. Leave the assertions and test behavior unchanged.Source: Coding guidelines
operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusStatus.java (1)
18-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unnecessary Javadocs from the simple status models.
operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusStatus.java#L18-L18: Remove the Javadoc.operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusStatus.java#L18-L18: Remove the Javadoc.As per coding guidelines, “Do not add comments except for very long or complex logic.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusStatus.java` at line 18, Remove the unnecessary Javadoc above ExternalStateInStatusStatus in operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusStatus.java:18-18 and operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusStatus.java:18-18; leave the simple status model declarations unchanged.Source: Coding guidelines
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerEventSourceConfiguration.java (1)
79-79: 📐 Maintainability & Code Quality | 🔵 TrivialTrack the deprecation TODO.
// todo deprecatemarks unfinished API work on a public interface method. Do you want me to open an issue to track deprecatinggetGroupVersionKind()?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerEventSourceConfiguration.java` at line 79, Track the planned deprecation of the public interface method getGroupVersionKind() by creating an issue or equivalent project-tracking item, and retain the existing TODO marker until that work is formally tracked.operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/FieldSelector.java (1)
42-58: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider copying
fieldsso the new hash code stays stable.
FieldSelectoris now used insideInformerClassifier, which is aHashMapkey inDefaultInformerPool. The constructors store the caller's list without a copy, so a later mutation of that list changeshashCode()and makes the pooled informer unreachable by its own key. A defensive copy in the constructors removes that risk and matches the "prefer immutable objects" guideline.As per coding guidelines: "Prefer immutable objects where possible."
♻️ Proposed change (outside the reviewed range)
public FieldSelector(List<Field> fields) { this.fields = List.copyOf(fields); } public FieldSelector(Field... fields) { this.fields = List.of(fields); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/FieldSelector.java` around lines 42 - 58, Update both FieldSelector constructors to defensively copy their inputs: use an immutable list copy for the List<Field> constructor and an immutable varargs list for the Field... constructor. Ensure the stored fields cannot be mutated after construction, keeping equals, hashCode, and toString unchanged.Source: Coding guidelines
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java (1)
176-186: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winProtect the original startup failure from a failing cleanup.
If
manager().stop()throws while releasing the already-acquired pooled informers, its exception replaces the real startup failure and the caller loses the root cause. Add the cleanup failure as a suppressed exception.♻️ Proposed change
try { manager().start(); } catch (RuntimeException e) { // The manager acquires a pooled informer for every watched namespace before any of them is // started, so a startup failure has to hand those references back here: super.start() is not // reached, which leaves isRunning() false and makes stop() skip the release entirely. The // pooled informer would then be referenced forever (never stopped, even on a clean shutdown) // and a retried start() would acquire it a second time. - manager().stop(); + try { + manager().stop(); + } catch (RuntimeException cleanupFailure) { + e.addSuppressed(cleanupFailure); + } throw e; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java` around lines 176 - 186, Update the RuntimeException catch in ManagedInformerEventSource startup so manager().stop() cleanup failures are added as suppressed exceptions to the original startup exception, then rethrow the original exception. Preserve cleanup execution and the existing startup behavior.operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPool.java (1)
57-67: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winSynchronize
releaseInformerto matchgetInformerandDefaultInformerPool.
getInformerholds the instance monitor across the containsKey/create/put sequence.releaseInformermutates the same map without that monitor. If a release and an acquisition for the same key overlap, the release can remove and stop an informer that the acquisition just published and returned.DefaultInformerPool.releaseInformeris synchronized, so the two pool implementations also expose different locking contracts today.🔒 Proposed fix
`@Override` - public <R extends HasMetadata> Optional<SharedIndexInformer<R>> releaseInformer( + public synchronized <R extends HasMetadata> Optional<SharedIndexInformer<R>> releaseInformer( String controllerName, String name, InformerClassifier<R> classifier) {🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPool.java` around lines 57 - 67, Synchronize NonSharingInformerPool.releaseInformer on the same instance monitor used by getInformer, covering the informer removal, stop, logging, and return flow. Match DefaultInformerPool’s locking contract so release cannot race with acquisition for the same key.operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterPrimaryCustomResource.java (1)
24-24: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove redundant Javadocs from simple test types and variants.
These comments do not describe very long or complex logic.
operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterPrimaryCustomResource.java#L24-L24: Remove the model Javadoc.operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterSharedInformerIT.java#L22-L22: Remove the variant Javadoc.operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterWatchedCustomResource.java#L24-L27: Remove the model Javadoc.operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/NonSharingInformerPoolDeregisterSharedInformerIT.java#L23-L27: Remove the variant Javadoc.operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource1.java#L24-L24: Remove the model Javadoc.operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/NonSharingInformerPoolDynamicSharedInformerIT.java#L23-L28: Remove the variant Javadoc.As per coding guidelines, do not add comments except for very long or complex logic.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterPrimaryCustomResource.java` at line 24, Remove the redundant Javadocs from the simple test types and variants in operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterPrimaryCustomResource.java:24, DeregisterSharedInformerIT.java:22, DeregisterWatchedCustomResource.java:24-27, NonSharingInformerPoolDeregisterSharedInformerIT.java:23-27, and operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource1.java:24 and NonSharingInformerPoolDynamicSharedInformerIT.java:23-28; leave the type and test implementations unchanged and do not add replacement comments.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/content/en/docs/documentation/eventing.md`:
- Around line 358-364: Update the informer-pool description around “informer
pool” and “SharedIndexInformer” to explicitly limit sharing to controllers and
event sources within the same Operator instance/configuration service; clarify
that separate Operator instances or processes do not share informers.
In
`@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java`:
- Around line 483-503: Change ConfigurationService.informerPool() from a default
method to an abstract method by removing its DefaultInformerPool construction
and configuration logic. Keep the existing cached implementation in
AbstractConfigurationService unchanged so every configuration service returns
the same pool instance.
In
`@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java`:
- Around line 55-63: Update the deprecated InformerEventSource constructor
Javadoc to link with {`@link`
`#InformerEventSource`(InformerEventSourceConfiguration)}. In the constructor path
identified by InformerEventSource and InformerManager, preserve and propagate
context.getClient() instead of discarding the EventSourceContext, ensuring
deprecated callers use their configured client rather than the
ConfigurationService fallback.
In
`@operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/GroupVersionKindTest.java`:
- Around line 105-106: Remove the explanatory comment near the GroupVersionKind
equality test, leaving the test name and assertions unchanged.
In
`@operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/AbstractSharedInformerIT.java`:
- Around line 63-83: Update bothControllersReconcileWatchingConfigMap to record
each reconciler’s execution count after the initial resources are created, then
create or update the watched ConfigMap and await assertions that both
SharedInformerReconciler1 and SharedInformerReconciler2 execution counts
increase. Keep the existing readiness checks and use the ConfigMap fixture/API
already established by the test.
In
`@operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/NonSharingInformerPoolSharedInformerIT.java`:
- Around line 23-26: Remove the redundant comments: delete the class Javadoc and
inline count comment in
operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/NonSharingInformerPoolSharedInformerIT.java
lines 23-26; delete the class Javadoc and default-pool inline comment in
operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerIT.java
lines 22-25; delete the class Javadoc in
operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerIT.java
lines 22-25; and delete the redundant Javadoc in
operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource2.java
line 24. Do not add replacement comments.
In `@README.md`:
- Line 69: Update the project descriptions in README.md: add “is an” to the
Apache Spark Kubernetes Operator entry, remove the conflicting “While ... but”
construction in the entries around lines 75–76, change the line 80 wording to
“not yet released as open source,” and hyphenate “cloud-native” on line 81.
In `@sample-operators/kotlin-operator/k8s/operator.yaml`:
- Around line 53-71: Update the ClusterRole rules in operator.yaml: remove the
customresourcedefinitions rule, limit configmapcopies to informer read verbs
plus status patching, and replace the configmaps wildcard with only the verbs
required for server-side apply.
- Around line 27-32: Update the operator Pod specification around the
serviceAccountName and operator container to define explicit Pod and container
security contexts: enforce non-root execution, set the RuntimeDefault seccomp
profile, drop all capabilities, disable privilege escalation, and use a
read-only root filesystem. If the JVM requires writable temporary storage, add
an emptyDir volume mounted only at the required temporary path.
- Around line 39-41: Update the ServiceAccount subject in the subjects
configuration to include its namespace, using the namespace where the
kotlin-operator ServiceAccount and Pod are created.
---
Outside diff comments:
In
`@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java`:
- Around line 111-136: Update InformerManager.changeNamespaces and stop to use
one shared lifecycle lock and stopped state. Under that lock, serialize source
lookup, acquisition, insertion, removal, and shutdown so additions cannot start
after stop and concurrent namespace changes cannot acquire duplicate sources;
ensure additions are rejected or released once stopped. Add a race test covering
stop() concurrent with adding a namespace.
---
Nitpick comments:
In
`@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/FieldSelector.java`:
- Around line 42-58: Update both FieldSelector constructors to defensively copy
their inputs: use an immutable list copy for the List<Field> constructor and an
immutable varargs list for the Field... constructor. Ensure the stored fields
cannot be mutated after construction, keeping equals, hashCode, and toString
unchanged.
In
`@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerEventSourceConfiguration.java`:
- Line 79: Track the planned deprecation of the public interface method
getGroupVersionKind() by creating an issue or equivalent project-tracking item,
and retain the existing TODO marker until that work is formally tracked.
In
`@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java`:
- Around line 176-186: Update the RuntimeException catch in
ManagedInformerEventSource startup so manager().stop() cleanup failures are
added as suppressed exceptions to the original startup exception, then rethrow
the original exception. Preserve cleanup execution and the existing startup
behavior.
In
`@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPool.java`:
- Around line 57-67: Synchronize NonSharingInformerPool.releaseInformer on the
same instance monitor used by getInformer, covering the informer removal, stop,
logging, and return flow. Match DefaultInformerPool’s locking contract so
release cannot race with acquisition for the same key.
In
`@operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusIT.java`:
- Around line 76-84: Remove the assertion-restating comments in
ExternalStateInStatusIT.java (lines 76-84) and
ExternalStateInStatusWorkflowIT.java (lines 83-93), specifically those preceding
the resource-size and status-ID assertions. Leave the assertions and test
behavior unchanged.
In
`@operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusStatus.java`:
- Line 18: Remove the unnecessary Javadoc above ExternalStateInStatusStatus in
operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusStatus.java:18-18
and
operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusStatus.java:18-18;
leave the simple status model declarations unchanged.
In
`@operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterPrimaryCustomResource.java`:
- Line 24: Remove the redundant Javadocs from the simple test types and variants
in
operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterPrimaryCustomResource.java:24,
DeregisterSharedInformerIT.java:22, DeregisterWatchedCustomResource.java:24-27,
NonSharingInformerPoolDeregisterSharedInformerIT.java:23-27, and
operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource1.java:24
and NonSharingInformerPoolDynamicSharedInformerIT.java:23-28; leave the type and
test implementations unchanged and do not add replacement comments.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0eebf96a-6321-4144-877c-743406646182
📒 Files selected for processing (111)
.github/workflows/e2e-test.ymlREADME.mdbootstrapper-maven-plugin/pom.xmlcaffeine-bounded-cache-support/pom.xmldocs/content/en/docs/documentation/eventing.mdmicrometer-support/pom.xmlmigration/pom.xmloperator-framework-bom/pom.xmloperator-framework-core/pom.xmloperator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractConfigurationService.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/FieldSelector.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerEventSourceConfiguration.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/GroupVersionKind.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GroupVersionKindPlural.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterWindow.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/AbstractInformerPool.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/DefaultInformerPool.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerClassifier.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerPool.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPool.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/MockKubernetesClient.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/InformerConfigurationTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/GroupVersionKindTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSourceTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManagerConcurrentReleaseTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapperTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourceNamespaceChangeTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourcePoolIdentityTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourceStartFailureTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/DefaultInformerPoolTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerClassifierTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPoolTest.javaoperator-framework-core/src/test/kotlin/io/javaoperatorsdk/operator/processing/dependent/workflow/KotlinCheckedExceptionDependentResourceTest.ktoperator-framework-junit/pom.xmloperator-framework-junit/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.javaoperator-framework/pom.xmloperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusCustomResource.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusReconciler.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusSpec.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusStatus.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/AbstractSharedInformerIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/NonSharingInformerPoolSharedInformerIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerCustomResource1.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerCustomResource2.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerReconciler1.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerReconciler2.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerStatus.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/AbstractDeregisterSharedInformerIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterPrimaryCustomResource.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterReconciler.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterSharedInformerIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterSpec.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterWatchedCustomResource.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/NonSharingInformerPoolDeregisterSharedInformerIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/AbstractDynamicSharedInformerIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource1.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource2.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerReconciler.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerThirdCustomResource.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/NonSharingInformerPoolDynamicSharedInformerIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/StaticSharedInformerReconciler.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsReconciler.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/triggerallevent/eventing/TriggerReconcilerOnAllEventIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusDependentResource.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusSpec.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusStatus.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusWorkflowCustomResource.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusWorkflowIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusWorkflowReconciler.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/standalonedependent/StandaloneDependentResourceIT.javapom.xmlsample-operators/controller-namespace-deletion/pom.xmlsample-operators/kotlin-operator/README.mdsample-operators/kotlin-operator/k8s/operator.yamlsample-operators/kotlin-operator/pom.xmlsample-operators/kotlin-operator/src/main/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopy.ktsample-operators/kotlin-operator/src/main/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopyOperator.ktsample-operators/kotlin-operator/src/main/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopyReconciler.ktsample-operators/kotlin-operator/src/main/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopySpec.ktsample-operators/kotlin-operator/src/main/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopyStatus.ktsample-operators/kotlin-operator/src/main/resources/log4j2.xmlsample-operators/kotlin-operator/src/test/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopyE2E.ktsample-operators/kotlin-operator/src/test/resources/log4j2.xmlsample-operators/leader-election/pom.xmlsample-operators/mysql-schema/pom.xmlsample-operators/operations/pom.xmlsample-operators/pom.xmlsample-operators/tomcat-operator/pom.xmlsample-operators/webpage/pom.xmltest-index-processor/pom.xml
Comment on lines
+483
to
+503
| /** | ||
| * The informer pool used to create and (when using the default, sharing pool) share the informers | ||
| * backing the event sources of all controllers managed by this {@code ConfigurationService}. | ||
| * | ||
| * <p><strong>Implementations must return the same instance on every call.</strong> The pool is | ||
| * effectively a per-{@code ConfigurationService} singleton: controllers share informers only if | ||
| * they resolve the same pool, and reference counting / informer shutdown are only correct if | ||
| * {@code getInformer} and {@code releaseInformer} operate on that same instance. This is | ||
| * intentionally not a {@code default} method, since a {@code default} could not cache the result | ||
| * and would hand out a fresh (unshared) pool on each call; {@link AbstractConfigurationService} | ||
| * provides a cached implementation backed by the default sharing pool. | ||
| * | ||
| * @return the informer pool for this configuration service | ||
| */ | ||
| @Experimental( | ||
| "Only the configuration API around informer pooling could still change in a" | ||
| + " non-backwards-compatible way, the pooling itself is prod ready.") | ||
| default InformerPool informerPool() { | ||
| var pool = new DefaultInformerPool(); | ||
| pool.setConfigurationService(this); | ||
| return pool; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Make informerPool() an abstract method.
Line 487 requires the same pool instance on every call. Lines 500-503 create a new pool on every call.
A custom ConfigurationService that relies on this default method will not share informers. Its releases will target different pools.
Keep the cached implementation in AbstractConfigurationService, but remove this default implementation.
Proposed fix
- default InformerPool informerPool() {
- var pool = new DefaultInformerPool();
- pool.setConfigurationService(this);
- return pool;
- }
+ InformerPool informerPool();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| /** | |
| * The informer pool used to create and (when using the default, sharing pool) share the informers | |
| * backing the event sources of all controllers managed by this {@code ConfigurationService}. | |
| * | |
| * <p><strong>Implementations must return the same instance on every call.</strong> The pool is | |
| * effectively a per-{@code ConfigurationService} singleton: controllers share informers only if | |
| * they resolve the same pool, and reference counting / informer shutdown are only correct if | |
| * {@code getInformer} and {@code releaseInformer} operate on that same instance. This is | |
| * intentionally not a {@code default} method, since a {@code default} could not cache the result | |
| * and would hand out a fresh (unshared) pool on each call; {@link AbstractConfigurationService} | |
| * provides a cached implementation backed by the default sharing pool. | |
| * | |
| * @return the informer pool for this configuration service | |
| */ | |
| @Experimental( | |
| "Only the configuration API around informer pooling could still change in a" | |
| + " non-backwards-compatible way, the pooling itself is prod ready.") | |
| default InformerPool informerPool() { | |
| var pool = new DefaultInformerPool(); | |
| pool.setConfigurationService(this); | |
| return pool; | |
| /** | |
| * The informer pool used to create and (when using the default, sharing pool) share the informers | |
| * backing the event sources of all controllers managed by this {`@code` ConfigurationService}. | |
| * | |
| * <p><strong>Implementations must return the same instance on every call.</strong> The pool is | |
| * effectively a per-{`@code` ConfigurationService} singleton: controllers share informers only if | |
| * they resolve the same pool, and reference counting / informer shutdown are only correct if | |
| * {`@code` getInformer} and {`@code` releaseInformer} operate on that same instance. This is | |
| * intentionally not a {`@code` default} method, since a {`@code` default} could not cache the result | |
| * and would hand out a fresh (unshared) pool on each call; {`@link` AbstractConfigurationService} | |
| * provides a cached implementation backed by the default sharing pool. | |
| * | |
| * `@return` the informer pool for this configuration service | |
| */ | |
| `@Experimental`( | |
| "Only the configuration API around informer pooling could still change in a" | |
| " non-backwards-compatible way, the pooling itself is prod ready.") | |
| InformerPool informerPool(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java`
around lines 483 - 503, Change ConfigurationService.informerPool() from a
default method to an abstract method by removing its DefaultInformerPool
construction and configuration logic. Keep the existing cached implementation in
AbstractConfigurationService unchanged so every configuration service returns
the same pool instance.
Comment on lines
+55
to
63
| /** | ||
| * @deprecated use {@link InformerEventSource(InformerEventSourceConfiguration)} | ||
| */ | ||
| // todo migrate sample, separate PR? | ||
| @Deprecated(forRemoval = true) | ||
| public InformerEventSource( | ||
| InformerEventSourceConfiguration<R> configuration, EventSourceContext<P> context) { | ||
| this(configuration, configuration.getKubernetesClient().orElse(context.getClient())); | ||
| this(configuration); | ||
| } |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check how the client is resolved now that EventSourceContext is dropped.
fd -t f 'InformerManager.java' -p 'operator-framework-core' | while IFS= read -r f; do
echo "== $f"; rg -n -C4 'getTargetClient|KubernetesClient|getKubernetesClient' "$f"
done
# Any remaining callers passing a context to the deprecated constructor
rg -nP --type=java 'new InformerEventSource<[^>]*>\(' -A3Repository: operator-framework/java-operator-sdk
Length of output: 2541
🏁 Script executed:
#!/bin/bash
set -e
echo '== InformerEventSource.java =='
sed -n '1,150p' operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java
echo '== InformerManager.java client resolution =='
sed -n '55,205p' operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java
echo '== InformerEventSourceConfiguration.java =='
fd -t f 'InformerEventSourceConfiguration.java' . | while IFS= read -r f; do
echo "== $f"
sed -n '1,220p' "$f"
done
echo '== Constructor declarations and usages =='
rg -n -C3 --type=java \
'InformerEventSource\s*\(|new\s+InformerEventSource|InformerEventSourceConfiguration' \
operator-framework-core operator-framework-core/src/test 2>/dev/null || true
echo '== ConfigurationService implementations =='
rg -n -C4 --type=java \
'interface ConfigurationService|class .*ConfigurationService|getKubernetesClient\s*\(' \
. | head -n 300Repository: operator-framework/java-operator-sdk
Length of output: 50393
🏁 Script executed:
#!/bin/bash
set -e
echo '== EventSourceContext and client access =='
rg -n -C5 --type=java \
'interface EventSourceContext|class .*EventSourceContext|getKubernetesClient\s*\(' \
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api \
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing
echo '== KubernetesDependentResource event-source construction =='
sed -n '195,240p' \
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.java
echo '== Informer parent setup =='
rg -n -C6 --type=java \
'new InformerManager|setControllerConfiguration|InformerManager<' \
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer
echo '== ConfigurationService client contract =='
rg -n -C8 --type=java \
'getKubernetesClient\s*\(' \
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config
echo '== Focused constructor call sites =='
rg -n -C3 --type=java \
'new InformerEventSource' \
operator-framework-core/src/main operator-framework-core/src/test
echo '== Available Javadoc tool =='
command -v javadoc || true
javadoc -version 2>&1 || trueRepository: operator-framework/java-operator-sdk
Length of output: 50392
🏁 Script executed:
#!/bin/bash
set -e
echo '== EventSourceContext client implementation =='
sed -n '25,90p' \
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/EventSourceContext.java
echo '== Informer configuration builder client propagation =='
sed -n '270,355p' \
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerEventSourceConfiguration.java
echo '== Current and parent constructor implementations =='
git show HEAD:operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java \
| sed -n '45,75p'
echo '== Parent revision =='
git show HEAD^:operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java \
| sed -n '45,85p'
echo '== Current and parent InformerManager client resolution =='
git show HEAD:operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java \
| sed -n '168,195p'
echo '== Parent revision =='
git show HEAD^:operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java \
| sed -n '160,190p'
echo '== Javadoc tool =='
command -v javadoc || true
javadoc --version 2>&1 || trueRepository: operator-framework/java-operator-sdk
Length of output: 11605
🌐 Web query:
Java Javadoc @link constructor reference syntax # ClassName(Type)
💡 Result:
In Java Javadoc, a constructor is referenced by using the name of its class followed by its parameter types in parentheses [1][2]. The syntax for the {@link} tag to reference a constructor is: {@link ClassName#ClassName(Type1, Type2,...)} Key details regarding this syntax include: Identification: A constructor is identified by the name of its class [1][2]. Unlike methods, which use the method name, you must repeat the class name to identify the constructor [1][3]. Parameter Types: The list of parameter types must be enclosed in parentheses [1][2]. For example, a constructor in class Foo that takes a String would be referenced as {@link Foo#Foo(String)} [3]. Omitting Parameters: If the constructor is not overloaded and the name does not conflict with a field or enum member, the parameter types and parentheses may be omitted [1][2]. References within the same class: When referencing a constructor within the same class as the documentation comment, the class name prefix (everything up to and including the #) can be omitted [1][2]. However, retaining the '#' is often done for clarity (e.g., {@link #ClassName(Type)}) [1][2]. Whitespace: Within the parameter list of the constructor, whitespace is permitted between tokens, but it is not allowed elsewhere in the reference [1][2]. If you are referencing a constructor in a different package, you must include the fully qualified name of the class (e.g., {@link com.example.MyClass#MyClass(String)}) to ensure the Javadoc tool can locate the element [3].
Citations:
- 1: https://docs.oracle.com/en/java/javase/21/docs/specs/javadoc/doc-comment-spec.html
- 2: https://docs.oracle.com/en/java/javase/24/docs/specs/javadoc/doc-comment-spec.html
- 3: https://stackoverflow.com/questions/9970823/javadoc-see-for-myclass-constructor-returning-a-warning-reference-not-found
🏁 Script executed:
#!/bin/bash
set -e
echo '== EventSourceContext construction and client identity =='
rg -n -C8 --type=java \
'new\s+EventSourceContext\s*\(' \
operator-framework-core/src/main operator-framework-core/src/test
echo '== EventSourceContext client flow =='
rg -n -C6 --type=java \
'EventSourceContext<|EventSourceContext\s*\(' \
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing \
operator-framework-core/src/main/java/io/javaoperatorsdk/operator
echo '== Repository change state =='
git status --short
git diff --stat
git log --oneline -3Repository: operator-framework/java-operator-sdk
Length of output: 232
🏁 Script executed:
#!/bin/bash
echo '== EventSourceContext construction and client identity =='
rg -n -C8 --type=java 'new\s+EventSourceContext\s*\(' . || true
rg -n -C6 --type=java 'EventSourceContext<' . | head -n 240 || true
echo '== Repository change state =='
git status --short
git diff --stat
git log --oneline -3Repository: operator-framework/java-operator-sdk
Length of output: 37740
🏁 Script executed:
#!/bin/bash
set -e
echo '== Controller event-source context client =='
rg -n -C10 --type=java \
'eventSourceContext\s*\(|getClient\s*\(\)' \
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.java \
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing \
| head -n 220Repository: operator-framework/java-operator-sdk
Length of output: 31432
🏁 Script executed:
#!/bin/bash
set -e
sed -n '70,145p' \
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/Controller.javaRepository: operator-framework/java-operator-sdk
Length of output: 3769
🏁 Script executed:
#!/bin/bash
set -e
rg -n -C8 --type=java \
'new\s+Controller\s*<|new\s+Controller\s*\(' \
operator-framework-core/src/main/java \
operator-framework-core/src/test/javaRepository: operator-framework/java-operator-sdk
Length of output: 33468
Fix the constructor Javadoc and preserve the context client. Use {@link #InformerEventSource(InformerEventSourceConfiguration)}. The deprecated constructor discards context.getClient(), while InformerManager falls back to ConfigurationService.getKubernetesClient() when no client is configured. These clients are not guaranteed to be identical. Propagate context.getClient() or retain the context-based path for deprecated callers.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java`
around lines 55 - 63, Update the deprecated InformerEventSource constructor
Javadoc to link with {`@link`
`#InformerEventSource`(InformerEventSourceConfiguration)}. In the constructor path
identified by InformerEventSource and InformerManager, preserve and propagate
context.getClient() instead of discarding the EventSourceContext, ensuring
deprecated callers use their configured client rather than the
ConfigurationService fallback.
Comment on lines
+105
to
+106
| // an unspecified plural is not a wildcard: it carries no plural form, just like the plain | ||
| // GroupVersionKind it compares equal to |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove the explanatory comment.
The test name and assertions define this short behavior. Keep the requirement in the test name if more detail is necessary.
Proposed change
- // an unspecified plural is not a wildcard: it carries no plural form, just like the plain
- // GroupVersionKind it compares equal toAs per coding guidelines, “Do not add comments except for very long or complex logic.”
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| // an unspecified plural is not a wildcard: it carries no plural form, just like the plain | |
| // GroupVersionKind it compares equal to |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/GroupVersionKindTest.java`
around lines 105 - 106, Remove the explanatory comment near the GroupVersionKind
equality test, leaving the test name and assertions unchanged.
Source: Coding guidelines
Comment on lines
+63
to
+83
| @Test | ||
| void bothControllersReconcileWatchingConfigMap() { | ||
| extension.create(customResource1(TEST_RESOURCE_1)); | ||
| extension.create(customResource2(TEST_RESOURCE_2)); | ||
|
|
||
| // both controllers reconcile, which guarantees their event sources (and thus their informers) | ||
| // have been started | ||
| await() | ||
| .untilAsserted( | ||
| () -> { | ||
| assertThat( | ||
| extension | ||
| .getReconcilerOfType(SharedInformerReconciler1.class) | ||
| .getNumberOfExecutions()) | ||
| .isPositive(); | ||
| assertThat( | ||
| extension | ||
| .getReconcilerOfType(SharedInformerReconciler2.class) | ||
| .getNumberOfExecutions()) | ||
| .isPositive(); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Exercise the secondary-resource event path.
Lines 65-66 create only primary resources. Those creations make both reconcilers run even if the pooled ConfigMap informer does not dispatch an event to one controller.
Create or update a ConfigMap after recording both reconciliation counts. Then assert that both counts increase. This validates listener registration and shared event dispatch.
As per coding guidelines, add unit and/or integration tests for new functionality whenever reasonably possible.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/AbstractSharedInformerIT.java`
around lines 63 - 83, Update bothControllersReconcileWatchingConfigMap to record
each reconciler’s execution count after the initial resources are created, then
create or update the watched ConfigMap and await assertions that both
SharedInformerReconciler1 and SharedInformerReconciler2 execution counts
increase. Keep the existing readiness checks and use the ConfigMap fixture/API
already established by the test.
Source: Coding guidelines
Comment on lines
+23
to
+26
| /** | ||
| * Runs {@link AbstractSharedInformerIT} with the {@link NonSharingInformerPool}: informers are | ||
| * never shared, so each of the two controllers watching {@code ConfigMap} gets its own informer. | ||
| */ |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Remove comments that restate direct code behavior.
The added comments repeat class names, selected pool behavior, or method results. Keep comments only where they explain complex logic.
operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/NonSharingInformerPoolSharedInformerIT.java#L23-L26: Remove the class Javadoc and the inline count comment.operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerIT.java#L22-L25: Remove the class Javadoc and the default-pool inline comment.operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerIT.java#L22-L25: Remove the class Javadoc.operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource2.java#L24-L24: Remove the redundant Javadoc.
As per coding guidelines, do not add comments except for very long or complex logic.
📍 Affects 4 files
operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/NonSharingInformerPoolSharedInformerIT.java#L23-L26(this comment)operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerIT.java#L22-L25operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerIT.java#L22-L25operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource2.java#L24-L24
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/NonSharingInformerPoolSharedInformerIT.java`
around lines 23 - 26, Remove the redundant comments: delete the class Javadoc
and inline count comment in
operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/NonSharingInformerPoolSharedInformerIT.java
lines 23-26; delete the class Javadoc and default-pool inline comment in
operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerIT.java
lines 22-25; delete the class Javadoc in
operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerIT.java
lines 22-25; and delete the redundant Javadoc in
operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource2.java
line 24. Do not add replacement comments.
Source: Coding guidelines
| Keycloak operator, built with Quarkus and JOSDK. | ||
| - [Apache Flink Kubernetes operator](https://github.com/apache/flink-kubernetes-operator) is the market leader among Flink operators. | ||
| - [Apache Flink Kubernetes Operator](https://github.com/apache/flink-kubernetes-operator) is the market leader among Flink operators. | ||
| - [Apache Spark Kubernetes Operator](https://github.com/apache/spark-kubernetes-operator) emerging operator for Spark. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Correct the grammar in the project descriptions.
- Line 69 needs
is an. - Lines 75-76 contain the conflicting
While ... butconstruction. - Line 80 should use
not yet released as open source. - Line 81 should use
cloud-native.
Proposed wording
-- [Apache Spark Kubernetes Operator](https://github.com/apache/spark-kubernetes-operator) emerging operator for Spark.
+- [Apache Spark Kubernetes Operator](https://github.com/apache/spark-kubernetes-operator) is an emerging operator for Spark.
-- [Strimzi Access operator](https://github.com/strimzi/kafka-access-operator). While the core Strimzi operator development predates
- JOSDK, but new components like the Access operator is using the framework.
+- [Strimzi Access operator](https://github.com/strimzi/kafka-access-operator). The core Strimzi operator predates
+ JOSDK, but new components like the Access operator use the framework.
-- ... It is not released as an open source yet but ...
+- ... It is not yet released as open source, but ...
-- ... Cloud native solution ...
+- ... Cloud-native solution ...Also applies to: 75-76, 80-81
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@README.md` at line 69, Update the project descriptions in README.md: add “is
an” to the Apache Spark Kubernetes Operator entry, remove the conflicting “While
... but” construction in the entries around lines 75–76, change the line 80
wording to “not yet released as open source,” and hyphenate “cloud-native” on
line 81.
Source: Linters/SAST tools
Comment on lines
+27
to
+32
| spec: | ||
| serviceAccountName: kotlin-operator | ||
| containers: | ||
| - name: operator | ||
| image: kotlin-sample-operator | ||
| imagePullPolicy: Never |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Set an explicit Pod and container security context.
The Pod runs without runAsNonRoot, seccompProfile, capability drops, allowPrivilegeEscalation: false, or a read-only root filesystem. Set these controls explicitly. If the JVM needs writable temporary storage, mount an emptyDir only at that path.
🧰 Tools
🪛 Trivy (0.72.0)
[error] 29-31: Root file system is not read-only
Container 'operator' of Pod 'kotlin-operator' should set 'securityContext.readOnlyRootFilesystem' to true
Rule: KSV-0014
(IaC/Kubernetes)
[error] 29-31: Default security context configured
container kotlin-operator in default namespace is using the default security context
Rule: KSV-0118
(IaC/Kubernetes)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sample-operators/kotlin-operator/k8s/operator.yaml` around lines 27 - 32,
Update the operator Pod specification around the serviceAccountName and operator
container to define explicit Pod and container security contexts: enforce
non-root execution, set the RuntimeDefault seccomp profile, drop all
capabilities, disable privilege escalation, and use a read-only root filesystem.
If the JVM requires writable temporary storage, add an emptyDir volume mounted
only at the required temporary path.
Source: Linters/SAST tools
Comment on lines
+39
to
+41
| subjects: | ||
| - kind: ServiceAccount | ||
| name: kotlin-operator |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/sh
set -eu
file='sample-operators/kotlin-operator/k8s/operator.yaml'
printf '%s\n' '--- manifest excerpt ---'
nl -ba "$file" | sed -n '1,90p'
printf '%s\n' '--- namespace and ServiceAccount references ---'
rg -n -C 3 'namespace:|kind: ServiceAccount|serviceAccount(Name)?:|ClusterRoleBinding|subjects:' "$file"Repository: operator-framework/java-operator-sdk
Length of output: 782
🏁 Script executed:
#!/bin/sh
set -eu
file='sample-operators/kotlin-operator/k8s/operator.yaml'
printf '%s\n' '--- complete manifest ---'
cat "$file"
printf '%s\n' '--- references to this sample namespace and ServiceAccount ---'
rg -n -C 3 'kotlin-operator|kotlin-sample-operator|operator.yaml' \
sample-operators/kotlin-operator README.md sample-operators 2>/dev/null | head -200Repository: operator-framework/java-operator-sdk
Length of output: 20420
🏁 Script executed:
#!/bin/sh
set -eu
file='sample-operators/kotlin-operator/src/test/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopyE2E.kt'
printf '%s\n' '--- Kotlin E2E deployment setup ---'
sed -n '35,85p' "$file"
printf '%s\n' '--- namespace mutation and deployment helpers ---'
rg -n -C 4 'setNamespace|namespace|withOperatorDeployment|ClusterDeployedOperatorExtension' \
sample-operators/kotlin-operator/src/test \
operator-framework 2>/dev/null | head -240Repository: operator-framework/java-operator-sdk
Length of output: 38081
🌐 Web query:
Kubernetes RBAC ClusterRoleBinding ServiceAccount subject namespace required validation
💡 Result:
In Kubernetes, the namespace field for a ServiceAccount subject in a ClusterRoleBinding is mandatory [1][2][3]. Because ServiceAccounts are namespaced resources, the ClusterRoleBinding must explicitly specify which namespace the ServiceAccount belongs to [1][2]. If you omit the namespace field when defining a ServiceAccount subject in a ClusterRoleBinding, the Kubernetes API server will reject the object with an error message similar to: subjects.namespace: Required value [1][2][3]. Key points to understand: - Scoping: ServiceAccounts are specific to a namespace [2]. The namespace field in the subject indicates where that ServiceAccount exists, not which namespaces the binding covers [2]. - No Cross-Namespace Matching: Kubernetes does not support granting permissions to a ServiceAccount with a specific name across all namespaces automatically [4]. If you need to grant permissions to a ServiceAccount in every namespace, you should instead bind the ClusterRole to the group system:serviceaccounts (which includes all service accounts in the cluster) or a namespace-specific group like system:serviceaccounts:<namespace> [5][4]. - API Validation: While some internal controllers might have historically behaved differently, the API server strictly enforces that the namespace field is present for ServiceAccount subjects within both RoleBindings and ClusterRoleBindings [6][7][3]. Attempting to submit a configuration without it will result in a validation failure [1][3].
Citations:
- 1: https://stackoverflow.com/questions/66434505/why-must-i-explicitly-set-a-namespace-for-the-serviceaccount-of-clusterrolebindi
- 2: https://stackoverflow.com/questions/58876847/clusterrolebinding-requires-namespace
- 3: The ClusterRoleBinding "exposedapp-crd-validating-role-binding" is invalid: subjects[0].namespace: Required value quarkiverse/quarkus-operator-sdk#758
- 4: support all namespace to clusterRoleBings
subjects.namesapcewhen kind is ServiceAccount kubernetes/kubernetes#78677 - 5: https://kubernetes.io/docs/reference/access-authn-authz/rbac/
- 6: RBAC
cluster/rolebindingcreated without subjects andSAnamespace check kubernetes/kubernetes#129265 - 7: Fix RBAC
cluster/rolebindingsubjects andSAnamespace empty check kubernetes/kubernetes#129267
Specify the ServiceAccount namespace.
The Kubernetes API server requires subjects[].namespace for a ServiceAccount subject. Set it to the namespace where the kotlin-operator ServiceAccount and Pod are created.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sample-operators/kotlin-operator/k8s/operator.yaml` around lines 39 - 41,
Update the ServiceAccount subject in the subjects configuration to include its
namespace, using the namespace where the kotlin-operator ServiceAccount and Pod
are created.
Comment on lines
+53
to
+71
| - apiGroups: | ||
| - "apiextensions.k8s.io" | ||
| resources: | ||
| - customresourcedefinitions | ||
| verbs: | ||
| - '*' | ||
| - apiGroups: | ||
| - "sample.javaoperatorsdk" | ||
| resources: | ||
| - configmapcopies | ||
| - configmapcopies/status | ||
| verbs: | ||
| - '*' | ||
| - apiGroups: | ||
| - "" | ||
| resources: | ||
| - configmaps | ||
| verbs: | ||
| - '*' |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | ⚡ Quick win
Replace wildcard ClusterRole permissions with required verbs.
Remove the customresourcedefinitions rule. Restrict configmapcopies to informer read verbs and status patching. Restrict configmaps to the verbs required for server-side apply. Once the binding is corrected, the current role allows the operator identity to modify or delete arbitrary CRDs, ConfigMaps, and ConfigMapCopy resources across the cluster.
🧰 Tools
🪛 Checkov (3.3.9)
[medium] 48-71: Minimize wildcard use in Roles and ClusterRoles
(CKV_K8S_49)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@sample-operators/kotlin-operator/k8s/operator.yaml` around lines 53 - 71,
Update the ClusterRole rules in operator.yaml: remove the
customresourcedefinitions rule, limit configmapcopies to informer read verbs
plus status patching, and replace the configmaps wildcard with only the verbs
required for server-side apply.
Source: Linters/SAST tools
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java`:
- Line 491: Wrap the Javadoc sentence near ConfigurationService so the line
stays within 100 characters, moving the {`@link` AbstractConfigurationService}
reference to the following Javadoc line without changing the text or meaning.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2e9863ae-539b-4cd5-8514-627b967d93ad
📒 Files selected for processing (111)
.github/workflows/e2e-test.ymlREADME.mdbootstrapper-maven-plugin/pom.xmlcaffeine-bounded-cache-support/pom.xmldocs/content/en/docs/documentation/eventing.mdmicrometer-support/pom.xmlmigration/pom.xmloperator-framework-bom/pom.xmloperator-framework-core/pom.xmloperator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractConfigurationService.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/FieldSelector.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerEventSourceConfiguration.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/GroupVersionKind.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GroupVersionKindPlural.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterWindow.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/AbstractInformerPool.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/DefaultInformerPool.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerClassifier.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerPool.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPool.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/MockKubernetesClient.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/InformerConfigurationTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/GroupVersionKindTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSourceTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManagerConcurrentReleaseTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapperTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourceNamespaceChangeTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourcePoolIdentityTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourceStartFailureTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/DefaultInformerPoolTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerClassifierTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPoolTest.javaoperator-framework-core/src/test/kotlin/io/javaoperatorsdk/operator/processing/dependent/workflow/KotlinCheckedExceptionDependentResourceTest.ktoperator-framework-junit/pom.xmloperator-framework-junit/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.javaoperator-framework/pom.xmloperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusCustomResource.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusReconciler.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusSpec.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusStatus.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/AbstractSharedInformerIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/NonSharingInformerPoolSharedInformerIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerCustomResource1.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerCustomResource2.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerReconciler1.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerReconciler2.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerStatus.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/AbstractDeregisterSharedInformerIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterPrimaryCustomResource.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterReconciler.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterSharedInformerIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterSpec.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterWatchedCustomResource.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/NonSharingInformerPoolDeregisterSharedInformerIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/AbstractDynamicSharedInformerIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource1.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource2.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerReconciler.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerThirdCustomResource.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/NonSharingInformerPoolDynamicSharedInformerIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/StaticSharedInformerReconciler.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsReconciler.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/triggerallevent/eventing/TriggerReconcilerOnAllEventIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusDependentResource.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusSpec.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusStatus.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusWorkflowCustomResource.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusWorkflowIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusWorkflowReconciler.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/standalonedependent/StandaloneDependentResourceIT.javapom.xmlsample-operators/controller-namespace-deletion/pom.xmlsample-operators/kotlin-operator/README.mdsample-operators/kotlin-operator/k8s/operator.yamlsample-operators/kotlin-operator/pom.xmlsample-operators/kotlin-operator/src/main/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopy.ktsample-operators/kotlin-operator/src/main/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopyOperator.ktsample-operators/kotlin-operator/src/main/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopyReconciler.ktsample-operators/kotlin-operator/src/main/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopySpec.ktsample-operators/kotlin-operator/src/main/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopyStatus.ktsample-operators/kotlin-operator/src/main/resources/log4j2.xmlsample-operators/kotlin-operator/src/test/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopyE2E.ktsample-operators/kotlin-operator/src/test/resources/log4j2.xmlsample-operators/leader-election/pom.xmlsample-operators/mysql-schema/pom.xmlsample-operators/operations/pom.xmlsample-operators/pom.xmlsample-operators/tomcat-operator/pom.xmlsample-operators/webpage/pom.xmltest-index-processor/pom.xml
🚧 Files skipped from review as they are similar to previous changes (108)
- sample-operators/controller-namespace-deletion/pom.xml
- test-index-processor/pom.xml
- operator-framework-junit/src/main/java/io/javaoperatorsdk/operator/junit/LocallyRunOperatorExtension.java
- sample-operators/pom.xml
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerEventSourceConfiguration.java
- operator-framework/pom.xml
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerCustomResource1.java
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/GroupVersionKind.java
- operator-framework-bom/pom.xml
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/workflow/AbstractWorkflowExecutor.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/NonSharingInformerPoolDeregisterSharedInformerIT.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterSpec.java
- docs/content/en/docs/documentation/eventing.md
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSource.java
- operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/EventSourceManagerTest.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsIT.java
- sample-operators/operations/pom.xml
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/NonSharingInformerPoolDynamicSharedInformerIT.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/standalonedependent/StandaloneDependentResourceIT.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusSpec.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterWatchedCustomResource.java
- migration/pom.xml
- operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManagerConcurrentReleaseTest.java
- sample-operators/webpage/pom.xml
- operator-framework-core/src/test/kotlin/io/javaoperatorsdk/operator/processing/dependent/workflow/KotlinCheckedExceptionDependentResourceTest.kt
- sample-operators/kotlin-operator/src/test/resources/log4j2.xml
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusIT.java
- operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerClassifierTest.java
- operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/config/InformerConfigurationTest.java
- operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourcePoolIdentityTest.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusStatus.java
- sample-operators/mysql-schema/pom.xml
- sample-operators/leader-election/pom.xml
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/AbstractSharedInformerIT.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/AbstractDynamicSharedInformerIT.java
- bootstrapper-maven-plugin/pom.xml
- sample-operators/kotlin-operator/src/main/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopy.kt
- operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapperTest.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerIT.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource1.java
- operator-framework-core/src/test/java/io/javaoperatorsdk/operator/MockKubernetesClient.java
- sample-operators/kotlin-operator/src/main/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopySpec.kt
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerReconciler2.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterReconciler.java
- sample-operators/tomcat-operator/pom.xml
- sample-operators/kotlin-operator/src/main/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopyStatus.kt
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerStatus.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/AbstractDeregisterSharedInformerIT.java
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/EventSourceManager.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterPrimaryCustomResource.java
- sample-operators/kotlin-operator/src/main/resources/log4j2.xml
- operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPoolTest.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusSpec.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusWorkflowCustomResource.java
- operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourceStartFailureTest.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerPrimaryCustomResource2.java
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/EventFilterWindow.java
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/AbstractConfigurationService.java
- .github/workflows/e2e-test.yml
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusCustomResource.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusWorkflowReconciler.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerThirdCustomResource.java
- operator-framework-junit/pom.xml
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GroupVersionKindPlural.java
- operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/GroupVersionKindTest.java
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerClassifier.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/deregister/DeregisterSharedInformerIT.java
- sample-operators/kotlin-operator/src/test/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopyE2E.kt
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/FieldSelector.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerCustomResource2.java
- operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/DefaultInformerPoolTest.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/NonSharingInformerPoolSharedInformerIT.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/resourceoperations/ResourceOperationsReconciler.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerReconciler.java
- pom.xml
- operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSourceNamespaceChangeTest.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/triggerallevent/eventing/TriggerReconcilerOnAllEventIT.java
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationServiceOverrider.java
- operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/ControllerTest.java
- operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSourceTest.java
- sample-operators/kotlin-operator/pom.xml
- micrometer-support/pom.xml
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerWrapper.java
- sample-operators/kotlin-operator/src/main/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopyReconciler.kt
- operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusWorkflowIT.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusDependentResource.java
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/DefaultInformerPool.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/DynamicSharedInformerIT.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/externalstateinstatus/ExternalStateInStatusStatus.java
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/InformerPool.java
- operator-framework-core/src/test/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperationsTest.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/externalstateinstatus/ExternalStateInStatusReconciler.java
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/PrimaryUpdateAndCacheUtils.java
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/NonSharingInformerPool.java
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/ManagedInformerEventSource.java
- sample-operators/kotlin-operator/src/main/kotlin/io/javaoperatorsdk/operator/sample/ConfigMapCopyOperator.kt
- sample-operators/kotlin-operator/README.md
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/dynamic/StaticSharedInformerReconciler.java
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java
- operator-framework/src/test/java/io/javaoperatorsdk/operator/baseapi/informerpool/basic/SharedInformerReconciler1.java
- operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/source/controller/ControllerEventSourceTest.java
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerConfiguration.java
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/reconciler/ResourceOperations.java
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerEventSource.java
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/pool/AbstractInformerPool.java
- operator-framework-core/pom.xml
- operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/event/ReconciliationDispatcherTest.java
- caffeine-bounded-cache-support/pom.xml
| * effectively a per-{@code ConfigurationService} singleton: controllers share informers only if | ||
| * they resolve the same pool, and reference counting / informer shutdown are only correct if | ||
| * {@code getInformer} and {@code releaseInformer} operate on that same instance. This is | ||
| * intentionally not a {@code default} method, since a {@code default} could not cache the result |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Wrap the Javadoc line to stay within 100 characters.
Line 491 exceeds the Java line-length limit. Move the {@link AbstractConfigurationService} reference to the next Javadoc line.
As per coding guidelines: “Limit Java source lines to 100 characters.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/ConfigurationService.java`
at line 491, Wrap the Javadoc sentence near ConfigurationService so the line
stays within 100 characters, moving the {`@link` AbstractConfigurationService}
reference to the following Javadoc line without changing the text or meaning.
Source: Coding guidelines
Add an opt-in, experimental detectApiVersionChange option on @KubernetesDependent that records the API version the operator applies in the javaoperatorsdk.io/last-applied-api-version annotation. The regular matcher then detects a mismatch when that marker differs from (or is missing relative to) the currently applied API version, causing a one-time update without triggering repeated reconciliations once the resource is up-to-date. Disabled by default, so existing behavior and matching are unaffected unless explicitly enabled. Guard against a null or immutable annotations map (e.g. Map.of(...)) on the desired resource when detectApiVersionChange is enabled, since writing the last-applied-api-version marker (and the pre-existing previous-annotation bookkeeping that runs alongside it) requires a mutable map. Also rename a misleadingly-named test helper and replace a no-op assertion on a primitive boolean with a concrete expectation. Addresses Copilot review feedback on PR #3536. Add DetectApiVersionChangeIT covering the end-to-end scenario: a ConfigMap dependent resource configured with detectApiVersionChange is marked with the current API version on creation (without triggering an update), and a stale marker annotation left on the actual resource (simulating an older operator/CRD version) is detected and corrected with exactly one update, after which no further reconciliation loop occurs. Signed-off-by: hej090224 <fc49854985@gmail.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
InformerManager is generic over C extends Informable, but getTargetClient type-tested for InformerEventSourceConfiguration to find out whether a specific (e.g. remote cluster) client was configured. Informable has two implementors and only one could answer the question, so a third configuration type wanting its own client would be ignored silently rather than failing to compile. Move the default getKubernetesClient() up from InformerEventSourceConfiguration to Informable and let InformerManager ask the configuration directly. The default still returns Optional.empty(), so existing implementations are unaffected. As a side effect the ConfigurationService client is now only created when no specific client is configured, instead of being created and then discarded.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/detectapiversionchange/ConfigMapDependentResource.java (1)
39-46: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winUse
varfor these non-short local declarations.
operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/detectapiversionchange/ConfigMapDependentResource.java#L39-L46: ChangeConfigMap configMaptovar configMap.operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentConverterTest.java#L86-L93: Change theControllerConfigurationandConfigurationServicelocals tovar.operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceApiVersionChangeTest.java#L287-L298: Change theContextandControllerConfigurationlocals tovar.As per coding guidelines, “Prefer
varover explicit type declarations except for short types such asint,long, andString.”🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/detectapiversionchange/ConfigMapDependentResource.java` around lines 39 - 46, Replace the non-short explicit local type declarations with var at all affected sites: ConfigMapDependentResource.java lines 39-46 for configMap, KubernetesDependentConverterTest.java lines 86-93 for the ControllerConfiguration and ConfigurationService locals, and KubernetesDependentResourceApiVersionChangeTest.java lines 287-298 for the Context and ControllerConfiguration locals. Leave short types such as String unchanged.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/detectapiversionchange/DetectApiVersionChangeReconciler.java`:
- Around line 20-21: Replace the wildcard import from
io.javaoperatorsdk.operator.api.reconciler in DetectApiVersionChangeReconciler
with explicit named imports for each reconciler API class referenced in the
file, while retaining simple class names and the existing Dependent import.
---
Nitpick comments:
In
`@operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/detectapiversionchange/ConfigMapDependentResource.java`:
- Around line 39-46: Replace the non-short explicit local type declarations with
var at all affected sites: ConfigMapDependentResource.java lines 39-46 for
configMap, KubernetesDependentConverterTest.java lines 86-93 for the
ControllerConfiguration and ConfigurationService locals, and
KubernetesDependentResourceApiVersionChangeTest.java lines 287-298 for the
Context and ControllerConfiguration locals. Leave short types such as String
unchanged.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 26077978-c224-4816-b647-d1c91970527e
📒 Files selected for processing (12)
docs/content/en/docs/documentation/dependent-resource-and-workflows/dependent-resources.mdoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependent.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentConverter.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResource.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfig.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceConfigBuilder.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentConverterTest.javaoperator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/KubernetesDependentResourceApiVersionChangeTest.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/detectapiversionchange/ConfigMapDependentResource.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/detectapiversionchange/DetectApiVersionChangeCustomResource.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/detectapiversionchange/DetectApiVersionChangeIT.javaoperator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/detectapiversionchange/DetectApiVersionChangeReconciler.java
Comment on lines
+20
to
+21
| import io.javaoperatorsdk.operator.api.reconciler.*; | ||
| import io.javaoperatorsdk.operator.api.reconciler.dependent.Dependent; |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Replace the wildcard import with named imports.
Import each reconciler API class used by this file. This keeps dependencies explicit.
As per coding guidelines, “Import classes at the top of the file and use simple class names; use fully qualified names only when necessary to avoid naming collisions.”
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@operator-framework/src/test/java/io/javaoperatorsdk/operator/dependent/detectapiversionchange/DetectApiVersionChangeReconciler.java`
around lines 20 - 21, Replace the wildcard import from
io.javaoperatorsdk.operator.api.reconciler in DetectApiVersionChangeReconciler
with explicit named imports for each reconciler API class referenced in the
file, while retaining simple class names and the existing Dependent import.
Source: Coding guidelines
There was a problem hiding this comment.
🧹 Nitpick comments (2)
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Informable.java (2)
36-39: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConfirm that the new Javadoc is allowed.
Lines 36-39 add comments to a short method. This conflicts with the rule that comments should only describe very long or complex logic. Remove the block unless public API Javadocs are explicitly exempted.
As per coding guidelines,
**/*.java: Do not add comments except for very long or complex logic.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Informable.java` around lines 36 - 39, Remove the newly added Javadoc block associated with the Informable method, since it documents a short method and violates the Java comment guideline; leave the method and surrounding API unchanged.Source: Coding guidelines
40-42: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for both Kubernetes client selection paths.
Test the default
Optional.empty()behavior and a customOptional<KubernetesClient>. Assert thatInformerManagerselects the custom client and otherwise uses the controller configuration client.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Informable.java` around lines 40 - 42, Add tests covering Informable.getKubernetesClient() returning Optional.empty() and a custom Optional<KubernetesClient>. Verify that InformerManager uses the custom client when provided and falls back to the controller configuration client when absent.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Informable.java`:
- Around line 36-39: Remove the newly added Javadoc block associated with the
Informable method, since it documents a short method and violates the Java
comment guideline; leave the method and surrounding API unchanged.
- Around line 40-42: Add tests covering Informable.getKubernetesClient()
returning Optional.empty() and a custom Optional<KubernetesClient>. Verify that
InformerManager uses the custom client when provided and falls back to the
controller configuration client when absent.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 34b82110-8f2e-494c-9025-e89921bfc387
📒 Files selected for processing (3)
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/Informable.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerEventSourceConfiguration.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java
💤 Files with no reviewable changes (1)
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/api/config/informer/InformerEventSourceConfiguration.java
🚧 Files skipped from review as they are similar to previous changes (1)
- operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/event/source/informer/InformerManager.java
- GenericKubernetesResourceMatcher allocated the path-prefix lists (List.of(SPEC), List.of(METADATA), the labels/annotations pair, List.of(STATUS)) once per JSON-diff node while matching, and nodeIsChildOf built a stream per call. Both run for every node of every match, so hoist the lists to constants and use an indexed loop. - SSABasedGenericKubernetesResourceMatcher#sanitizeState nested the StatefulSet volume-claim-template handling four levels deep inside the type ladder; extract it into sanitizeVolumeClaimTemplates so the ladder reads as one dispatch per resource kind.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcher.java (1)
226-248: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd regression tests for both sanitization branches.
The helper now exits when actual and desired
volumeClaimTemplatescounts differ. Add or verify tests for:
- Equal counts with omitted
volumeModeandstatus.- Different counts with no sanitization and a non-match result.
Verify the adjacent
SSABasedGenericKubernetesResourceMatcherTestcoverage:#!/bin/bash set -euo pipefail test_file="operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcherTest.java" rg -n -C 4 'volumeClaimTemplates|volumeMode|status' "$test_file"As per coding guidelines,
**/*.{java,xml}files require unit and/or integration tests for new functionality whenever reasonably possible.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcher.java` around lines 226 - 248, Add regression coverage in SSABasedGenericKubernetesResourceMatcherTest for equal-sized volumeClaimTemplates where omitted volumeMode and status are sanitized, and for differing sizes where sanitization is skipped and matching returns false. Reuse the existing matcher test setup and assert both branches through the public matching behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcher.java`:
- Around line 226-248: Add regression coverage in
SSABasedGenericKubernetesResourceMatcherTest for equal-sized
volumeClaimTemplates where omitted volumeMode and status are sanitized, and for
differing sizes where sanitization is skipped and matching returns false. Reuse
the existing matcher test setup and assert both branches through the public
matching behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a0f9eb21-dc75-4b5c-8555-9cb631e63bb6
📒 Files selected for processing (2)
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/GenericKubernetesResourceMatcher.javaoperator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/kubernetes/SSABasedGenericKubernetesResourceMatcher.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation