Skip to content

🐛 OCPBUGS-60963: fix crdUpgradeSafety reporting OneOf changes as unhandled - #2759

Merged
openshift-merge-bot[bot] merged 1 commit into
operator-framework:mainfrom
pedjak:upgrade-crdify
Jun 11, 2026
Merged

🐛 OCPBUGS-60963: fix crdUpgradeSafety reporting OneOf changes as unhandled#2759
openshift-merge-bot[bot] merged 1 commit into
operator-framework:mainfrom
pedjak:upgrade-crdify

Conversation

@pedjak

Copy link
Copy Markdown
Contributor

Description

CRD upgrade safety checks reported OneOf schema changes as generic
"unhandled changes found" errors, blocking legitimate operator upgrades
(e.g. Serverless Operator 1.35.0 → 1.36.0).

Fixed by updating sigs.k8s.io/crdify to the latest master (bb9957dbf465)
which includes a dedicated oneOf validator
(kubernetes-sigs/crdify#54)
that properly categorizes OneOf changes (constraint added, removed, subschemas modified).

Changes

  • Pin sigs.k8s.io/crdify to latest master (v0.6.1-0.20260602124154-bb9957dbf465)
  • Update existing test expectation from unhandled to oneOf error
  • Add new unit tests for OneOf removal and OneOf addition scenarios

Reviewer Checklist

  • API Go Documentation
  • Tests: Unit Tests (and E2E Tests, if appropriate)
  • Comprehensive Commit Messages
  • Links to related GitHub Issue(s)

CopilotAI review requested due to automatic review settings June 11, 2026 13:36
@netlify

netlifyBot commented Jun 11, 2026

Copy link
Copy Markdown

Deploy Preview for olmv1 ready!

NameLink
🔨 Latest commit4a1127b
🔍 Latest deploy loghttps://app.netlify.com/projects/olmv1/deploys/6a2abe23e090520007bf97f6
😎 Deploy Previewhttps://deploy-preview-2759--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changesRun an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

CopilotAI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the CRD upgrade safety preflight to correctly classify oneOf-related schema changes, avoiding generic “unhandled changes found” errors that can block legitimate operator upgrades. It does this by bumping the sigs.k8s.io/crdify dependency to a newer commit that adds a dedicated oneOf validator, and by adjusting/adding unit tests to assert the new error categorization.

Changes:

  • Bump sigs.k8s.io/crdify to a newer pseudo-version that includes oneOf validation support.
  • Update an existing CRD upgrade test to expect a oneOf-scoped error instead of unhandled.
  • Add new unit tests and CRD manifest fixtures covering oneOf removal and oneOf addition scenarios.

Reviewed changes

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

Show a summary per file
FileDescription
internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-oneof-safe-addition-old.jsonAdds CRD fixture representing the “before” state without oneOf.
internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-oneof-safe-addition-new.jsonAdds CRD fixture representing the “after” state with a new oneOf constraint.
internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-oneof-removed-old.jsonAdds CRD fixture representing the “before” state with oneOf.
internal/operator-controller/rukpak/preflights/crdupgradesafety/testdata/manifests/crd-oneof-removed-new.jsonAdds CRD fixture representing the “after” state where oneOf is removed.
internal/operator-controller/rukpak/preflights/crdupgradesafety/crdupgradesafety_test.goUpdates expectations and adds targeted tests for oneOf validator behavior.
go.modPins sigs.k8s.io/crdify to the newer pseudo-version containing the oneOf validator.
go.sumUpdates checksums to match the new crdify version.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

OneOf schema changes (e.g. adding required-field constraints) were
falling back to the generic "unhandled" comparator, blocking legitimate
operator upgrades like Serverless Operator 1.35.0 to 1.36.0.
Pin sigs.k8s.io/crdify to latest master (bb9957dbf465) which includes
a dedicated oneOf validator.
Co-Authored-By: Claude <noreply@anthropic.com>
@tmshort

Copy link
Copy Markdown
Contributor

/approve

@openshift-ci

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: tmshort

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ciopenshift-ciBot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Jun 11, 2026
@codecov

codecovBot commented Jun 11, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 66.89%. Comparing base (9c49467) to head (4a1127b).
⚠️ Report is 2 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #2759 +/- ##
==========================================
- Coverage 66.97% 66.89% -0.08% 
==========================================
Files 149 149 Lines 11382 11382 ==========================================
- Hits 7623 7614 -9 - Misses 3206 3213 +7 - Partials 553 555 +2 
FlagCoverage Δ
e2e35.28% <ø> (+0.21%)⬆️
experimental-e2e52.65% <ø> (-0.05%)⬇️
unit52.12% <ø> (-0.06%)⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pedjak

Copy link
Copy Markdown
ContributorAuthor

/jira refresh

@perdasilva

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ciopenshift-ciBot added the lgtm Indicates that a PR is ready to be merged. label Jun 11, 2026
@openshift-merge-bot
openshift-merge-botBot merged commit 79f6c2a into operator-framework:mainJun 11, 2026
26 checks passed
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

approvedIndicates a PR has been approved by an approver from all required OWNERS files.lgtmIndicates that a PR is ready to be merged.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@pedjak@tmshort@perdasilva