Add `add_mode` column to package table
Description of the change: This PR introduces:
- an sqlite migration that adds an
add_modecolumn to thepackagetable. - a new global sanity check that's part of the
addcommand that causes failures when attempting to add with a mode that is incompatible with a package's previously used add mode. - logic to ensure the
add_modecolumn is correctly set when any of the existing supported add modes is used (replaces, semver, semver-skippatch)
Motivation for the change:
This change enshrines our existing guidance that add modes cannot be mixed and matched, by comparing the previously-used add mode with the currently requested add mode and emitting an error if they do not match. This functionality is crucial for ensuring that opm can maintain the integrity of the upgrade graph of each package in the catalog.
This is also a necessary pre-requisite for #949 , which will add a new add mode called "fbc" that will be able to idempotently convert FBC catalogs to sqlite. We need to be able to prevent declarative FBC-derived packages from being tampered with via imperative add commands.
Downstream note: This change will likely need to be backported to all versions we expect to support catalogs containing reverse-migrated packages so that we can ask users to upgrade to the opm binary that contains these extra add_mode checks. Users that use versions of opm without these checks will still be at risk for mixing/matching add modes despite our guidance that they should not.
Reviewer Checklist
- [x] Implementation matches the proposed design, or proposal is updated to match implementation
- [x] Sufficient unit test coverage
- [x] Sufficient end-to-end test coverage
- [ ] Docs updated or added to
/docs - [x] Commit messages sensible and descriptive
Codecov Report
Merging #972 (7db3c0b) into master (0899512) will increase coverage by
0.31%. The diff coverage is62.19%.
:exclamation: Current head 7db3c0b differs from pull request most recent head f05ca62. Consider uploading reports for the commit f05ca62 to get more accurate results
@@ Coverage Diff @@
## master #972 +/- ##
==========================================
+ Coverage 52.48% 52.80% +0.31%
==========================================
Files 103 104 +1
Lines 9240 9293 +53
==========================================
+ Hits 4850 4907 +57
+ Misses 3468 3446 -22
- Partials 922 940 +18
| Impacted Files | Coverage Δ | |
|---|---|---|
| pkg/registry/channelupdateoptions.go | 77.77% <0.00%> (+77.77%) |
:arrow_up: |
| pkg/registry/empty.go | 0.00% <0.00%> (ø) |
|
| pkg/registry/graph.go | 20.00% <0.00%> (-1.74%) |
:arrow_down: |
| pkg/sqlite/graphloader.go | 69.44% <50.00%> (-1.15%) |
:arrow_down: |
| pkg/sqlite/load.go | 46.86% <50.00%> (+0.58%) |
:arrow_up: |
| pkg/sqlite/query.go | 64.32% <50.00%> (-0.22%) |
:arrow_down: |
| pkg/registry/populator.go | 66.97% <56.25%> (+3.81%) |
:arrow_up: |
| pkg/sqlite/migrations/014_package_add_mode.go | 69.69% <69.69%> (ø) |
|
| pkg/registry/bundlegraphloader.go | 77.33% <100.00%> (+1.27%) |
:arrow_up: |
| pkg/registry/types.go | 29.21% <100.00%> (+0.80%) |
:arrow_up: |
| ... and 5 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 0899512...f05ca62. Read the comment docs.
[APPROVALNOTIFIER] This PR is APPROVED
This pull-request has been approved by: joelanford, tylerslaton
The full list of commands accepted by this bot can be found here.
The pull request process is described here
- ~~OWNERS~~ [joelanford]
Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment
We definitely need some docs on this, or customers are just not going to understand the interactions here, particularly mapping the error messages into remediation actions.
closing PR as stale. Please re-open if it's still important.