Skip to content

🐛 CRD upgrade safety fixes and ratcheting - #2123

Merged
openshift-merge-bot[bot] merged 2 commits into
operator-framework:mainfrom
joelanford:crd-upgrade-safety-ratchet
Aug 26, 2025
Merged

🐛 CRD upgrade safety fixes and ratcheting#2123
openshift-merge-bot[bot] merged 2 commits into
operator-framework:mainfrom
joelanford:crd-upgrade-safety-ratchet

Conversation

@joelanford

@joelanfordjoelanford commented Jul 29, 2025

Copy link
Copy Markdown
Member

Description

This fixes two regressions and a newly discovered bug in the CRD upgrade preflight safety check that causes errors to be reported even when schemas of a CRD have not changed between bundle versions.

The issues this PR fixes were originally reported in:

Regression fixes

  1. When we moved to crdify, crdify did not have have a bug fix we had already made for correctly handling properties that are lists (in OpenAPI parlance, "items")
  2. When we moved to crdify, we did not properly configure it to allow enums to be added.

Bug fix description

The current logic of the CRD checker always reports all issues found in the roundtrip-ability between versions in the new CRD. However if those exact same problems exist in the existing CRD, then the upgrade is not making the situation any worse, so the check should not fail.

Reviewer Checklist

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

@openshift-ciopenshift-ciBot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Jul 29, 2025
@netlify

netlifyBot commented Jul 29, 2025

Copy link
Copy Markdown

Deploy Preview for olmv1 ready!

NameLink
🔨 Latest commitc21ba9d
🔍 Latest deploy loghttps://app.netlify.com/projects/olmv1/deploys/68adb8d55c013b000770784f
😎 Deploy Previewhttps://deploy-preview-2123--olmv1.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@codecov

codecovBot commented Jul 29, 2025

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 72.70%. Comparing base (07bd008) to head (c21ba9d).
⚠️ Report is 7 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #2123 +/- ##
==========================================
- Coverage 72.75% 72.70% -0.05% 
==========================================
Files 79 79 Lines 7340 7347 +7 ==========================================
+ Hits 5340 5342 +2 - Misses 1653 1658 +5 
Partials 347 347 
FlagCoverage Δ
e2e44.32% <100.00%> (-0.02%)⬇️
experimental-e2e54.89% <100.00%> (-0.08%)⬇️
unit58.28% <100.00%> (+0.03%)⬆️

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

☔ View full report in Codecov by Sentry.
📢 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.

@joelanford
joelanfordforce-pushed the crd-upgrade-safety-ratchet branch from b807a29 to e27ff46CompareJuly 29, 2025 16:37
Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
@joelanford
joelanfordforce-pushed the crd-upgrade-safety-ratchet branch from e27ff46 to c393e44CompareAugust 25, 2025 19:05
@joelanford
joelanford marked this pull request as ready for review August 25, 2025 19:32
@joelanford
joelanford requested a review from a team as a code ownerAugust 25, 2025 19:32
@openshift-ciopenshift-ciBot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 25, 2025
@joelanfordjoelanford changed the title 🐛 CRD upgrade safety ratcheting🐛 CRD upgrade safety fixes and ratchetingAug 26, 2025
regression fixes:
1. correctly handle processing of properties that are OpenAPI items
2. allow enums to have values added.
bug fix:
crdify's served version validator was updated to actually compare the
old CRD with the new CRD so that any issues identified in the old CRD
do not continue to be reported when performing comparisons between
served versions of the new CRD.
This effectively allows issues in the served version validations to be
acknowledged once when they are introduced, but then those issues are
essentially grandfathered in such that they do not have to be
acknowledged again in the future.
This issue was actually identified in a case where an operator upgrade
was stopped by the CRD upgrade check despite there being no changes
whatsoever to the CRD. The "old" and "new" CRDs contained the exact
same issues, but since crdify was looking exclusively at the "new" CRD,
it found those issues and reported them.
@joelanford
joelanfordforce-pushed the crd-upgrade-safety-ratchet branch from c393e44 to c21ba9dCompareAugust 26, 2025 13:38
@tmshort

Copy link
Copy Markdown
Contributor

/lgtm

@openshift-ciopenshift-ciBot added the lgtm Indicates that a PR is ready to be merged. label Aug 26, 2025
@openshift-ci

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: perdasilva

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 Aug 26, 2025
@openshift-merge-bot
openshift-merge-botBot merged commit 236319b into operator-framework:mainAug 26, 2025
24 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.

3 participants

@joelanford@tmshort@perdasilva