-
Notifications
You must be signed in to change notification settings - Fork 261
opm validate: make sure skipped bundles are not included in replaces chain #1762
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
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:
|
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. |
PR needs rebase. Instructions 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. |
1. improve detection of bundles stranded due to usage of ancestor skips 2. find and report all cycles Signed-off-by: Joe Lanford <[email protected]>
87f9927
to
d96879f
Compare
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 validate
currently 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 validate
logic 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