opm validate: make sure skipped bundles are not included in replaces chain #1762
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1762 +/- ##
==========================================
+ Coverage 55.17% 55.56% +0.38%
==========================================
Files 136 137 +1
Lines 15917 16094 +177
==========================================
+ Hits 8782 8942 +160
- Misses 5982 5995 +13
- Partials 1153 1157 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
870f516 to
1d1230f
Compare
1. improve detection of bundles stranded due to usage of ancestor skips 2. find and report all cycles Signed-off-by: Joe Lanford <joe.lanford@gmail.com>
1d1230f to
87f9927
Compare
|
This is an aspirational, holistic attempt to resolve the family of problems characterized by inadvertent invalid channels, as well as to buttress the lax off-cluster validations (to enable earlier-detected errors and avoid on-cluster surprise). Versus #1750 which seeks to narrowly-define a single validation failure case -- and more importantly, to navigate a publishing path through existing catalog content to ameliorate impacts. #1762 also runs the risks of introducing v0-isms into validation as default, by introducing the semver-noncompliant build metadata processing v0 uses. I didn't see an impact assessment of applying the new rules to existing catalog content, and I think we absolutely need to understand any impacts there to understand how viable this PR is, and/or the nature/scope of follow-up actions we need to take to enable it. At this point, I'd favor working to get #1750 to consensus and giving ourselves some more time to work through the complexities this is trying to tackle. |
Description of the change:
When verifying channel validity, halt replaces chain traversal when we encounter a bundle that is skipped by any other entry anywhere in the channel.
Motivation for the change:
opm validatecurrently says that invalid graphs are actually valid. In certain circumstances when a bundle is both skipped and is also somewhere in the replaces chain, the currentopm validatelogic has a preference for the bundle being in the replaces chain despite being skipped. In reality, skipped bundles are NOT in the replaces chain and their upgrade edges are NOT honored.Reviewer Checklist
/docs