Skip to content

fix: honour bulk dependent resource capabilities during reconciliation - #3520

Draft
csviri wants to merge 1 commit into
operator-framework:mainfrom
csviri:fix/bulk-dependent-capability-overrides
Draft

fix: honour bulk dependent resource capabilities during reconciliation#3520
csviri wants to merge 1 commit into
operator-framework:mainfrom
csviri:fix/bulk-dependent-capability-overrides

Conversation

@csviri

@csviri csviri commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

A BulkDependentResource may implement any subset of Creator,
Updater and Deleter. BulkDependentResourceInstance, the internal
per-item wrapper that reuses AbstractDependentResource's reconcile
logic, implements all three and delegates each call to the bulk resource
with an unchecked cast. Its capabilities therefore have to be derived
from the wrapped bulk resource.

0b8d8dd introduced the overridable creatable() / updatable() hooks
for exactly this, but the wiring was never completed:

  • BulkDependentResourceInstance overrides isCreatable() /
    isUpdatable(), which reconcile never consults, so both overrides
    were dead code.
  • the inner create branch of AbstractDependentResource.reconcile still
    read the creatable field directly instead of calling creatable().

Since the wrapper implements all three interfaces, both fields are always
true, so create and update were attempted regardless of what the bulk
resource actually supports, failing with a ClassCastException in
BulkDependentResourceInstance.create / update:

java.lang.ClassCastException: class ...CreateAndDeleteOnlyBulk
  cannot be cast to class ...Updater
  at BulkDependentResourceInstance.update(BulkDependentResourceReconciler.java:115)
  at AbstractDependentResource.handleUpdate(AbstractDependentResource.java:204)

A non-bulk dependent in the same situation just logs "implement Updater
interface to modify it" and skips the operation.

Fixes this by overriding creatable() / updatable() in the wrapper and
by using creatable() in the create branch. isCreatable() /
isUpdatable() now delegate to creatable() / updatable() so the two
pairs cannot drift apart again.

Adds regression tests for a create+delete-only and a delete-only bulk
dependent; both fail without this change.

Part of #3517

A `BulkDependentResource` may implement any subset of `Creator`,
`Updater` and `Deleter`. `BulkDependentResourceInstance`, the internal
per-item wrapper that reuses `AbstractDependentResource`'s reconcile
logic, implements all three and delegates each call to the bulk resource
with an unchecked cast. Its capabilities therefore have to be derived
from the wrapped bulk resource.

0b8d8dd introduced the overridable `creatable()` / `updatable()` hooks
for exactly this, but the wiring was never completed:

* `BulkDependentResourceInstance` overrides `isCreatable()` /
  `isUpdatable()`, which `reconcile` never consults, so both overrides
  were dead code.
* the inner create branch of `AbstractDependentResource.reconcile` still
  read the `creatable` field directly instead of calling `creatable()`.

Since the wrapper implements all three interfaces, both fields are always
true, so create and update were attempted regardless of what the bulk
resource actually supports, failing with a ClassCastException in
`BulkDependentResourceInstance.create` / `update`:

    java.lang.ClassCastException: class ...CreateAndDeleteOnlyBulk
      cannot be cast to class ...Updater
      at BulkDependentResourceInstance.update(BulkDependentResourceReconciler.java:115)
      at AbstractDependentResource.handleUpdate(AbstractDependentResource.java:204)

A non-bulk dependent in the same situation just logs "implement Updater
interface to modify it" and skips the operation.

Fixes this by overriding `creatable()` / `updatable()` in the wrapper and
by using `creatable()` in the create branch. `isCreatable()` /
`isUpdatable()` now delegate to `creatable()` / `updatable()` so the two
pairs cannot drift apart again.

Adds regression tests for a create+delete-only and a delete-only bulk
dependent; both fail without this change.
Copilot AI review requested due to automatic review settings July 30, 2026 09:04
@openshift-ci openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 30, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes reconciliation for bulk dependents so create/update operations are only attempted when the wrapped BulkDependentResource actually supports them, preventing ClassCastException from the per-item wrapper incorrectly advertising capabilities.

Changes:

  • Wire AbstractDependentResource.reconcile to consult the overridable creatable() hook (not the raw creatable field) when entering the create path.
  • Update BulkDependentResourceInstance to override creatable() / updatable() based on the wrapped bulk resource’s implemented capabilities.
  • Add regression tests covering bulk dependents that support only (create+delete) and only (delete), ensuring create/update are skipped appropriately.

Reviewed changes

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

File Description
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/AbstractDependentResource.java Ensures reconciliation uses creatable()/updatable() hooks consistently so capability overrides are honored.
operator-framework-core/src/main/java/io/javaoperatorsdk/operator/processing/dependent/BulkDependentResourceReconciler.java Derives per-item wrapper create/update capabilities from the wrapped bulk resource to avoid invalid casts.
operator-framework-core/src/test/java/io/javaoperatorsdk/operator/processing/dependent/BulkDependentResourceCapabilitiesTest.java Adds regression coverage for bulk resources that intentionally do not implement Creator and/or Updater.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants