drop all sqlite support, model package, and channel priority properties#1986
drop all sqlite support, model package, and channel priority properties#1986grokspawn wants to merge 2 commits into
Conversation
|
Skipping CI for Draft Pull Request. |
|
[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. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
There was a problem hiding this comment.
Pull request overview
This PR removes the long-deprecated SQLite-based catalog/index path from operator-registry and drops the (internal/alpha) channel priority property support, leaving the file-based catalog (FBC / declarative config) path as the supported workflow.
Changes:
- Remove SQLite implementation code, migrations, testdata, and sqlite-backed CLI surfaces (
opm index,opm registry, standalone servers, sqlite migration command). - Remove channel priority property construction in the alpha property layer.
- Adjust build configuration/dependencies to reflect the removal (tags/CGO changes, module dependency cleanup).
Reviewed changes
Copilot reviewed 115 out of 163 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/opm_test.go | Removes sqlite/indexer imports (but file still contains sqlite/indexer usages; currently breaks compilation). |
| pkg/sqlite/testdata/test_db_migrations/valid/200412250000_fake_migration.up.sql | Removes sqlite migration testdata. |
| pkg/sqlite/testdata/test_db_migrations/valid/200412250000_fake_migration.down.sql | Removes sqlite migration testdata. |
| pkg/sqlite/testdata/test_db_migrations/valid/200112250000_fake_migration.up.sql | Removes sqlite migration testdata. |
| pkg/sqlite/testdata/test_db_migrations/valid/200112250000_fake_migration.down.sql | Removes sqlite migration testdata. |
| pkg/sqlite/testdata/test_db_migrations/invalid/randomscript.sql | Removes sqlite migration testdata. |
| pkg/sqlite/testdata/strandedbundles/prometheus.0.22.2/metadata/dependencies.yaml | Removes sqlite stranded-bundle test fixture. |
| pkg/sqlite/testdata/strandedbundles/prometheus.0.22.2/metadata/annotations.yaml | Removes sqlite stranded-bundle test fixture. |
| pkg/sqlite/testdata/strandedbundles/prometheus.0.22.2/manifests/prometheusrule.crd.yaml | Removes sqlite stranded-bundle test fixture. |
| pkg/sqlite/testdata/strandedbundles/prometheus.0.15.0/metadata/annotations.yaml | Removes sqlite stranded-bundle test fixture. |
| pkg/sqlite/testdata/strandedbundles/prometheus.0.15.0/manifests/prometheusrule.crd.yaml | Removes sqlite stranded-bundle test fixture. |
| pkg/sqlite/testdata/strandedbundles/prometheus.0.14.0/metadata/annotations.yaml | Removes sqlite stranded-bundle test fixture. |
| pkg/sqlite/testdata/strandedbundles/prometheus.0.14.0/manifests/prometheusrule.crd.yaml | Removes sqlite stranded-bundle test fixture. |
| pkg/sqlite/testdata/loader_data/prometheus/prometheus.package.yaml | Removes sqlite loader test fixture. |
| pkg/sqlite/testdata/loader_data/prometheus/0.22.2/servicemonitor.crd.yaml | Removes sqlite loader test fixture. |
| pkg/sqlite/testdata/loader_data/prometheus/0.22.2/prometheusrule.crd.yaml | Removes sqlite loader test fixture. |
| pkg/sqlite/testdata/loader_data/prometheus/0.15.0/servicemonitor.crd.yaml | Removes sqlite loader test fixture. |
| pkg/sqlite/testdata/loader_data/prometheus/0.15.0/prometheusrule.crd.yaml | Removes sqlite loader test fixture. |
| pkg/sqlite/testdata/loader_data/prometheus/0.14.0/prometheusrule.crd.yaml | Removes sqlite loader test fixture. |
| pkg/sqlite/testdata/loader_data/etcd/etcd.package.yaml | Removes sqlite loader test fixture. |
| pkg/sqlite/testdata/loader_data/etcd/0.9.2/etcdrestore.crd.yaml | Removes sqlite loader test fixture. |
| pkg/sqlite/testdata/loader_data/etcd/0.9.2/etcdcluster.crd.yaml | Removes sqlite loader test fixture. |
| pkg/sqlite/testdata/loader_data/etcd/0.9.2/etcdbackup.crd.yaml | Removes sqlite loader test fixture. |
| pkg/sqlite/testdata/loader_data/etcd/0.6.1/etcdcluster.crd.yaml | Removes sqlite loader test fixture. |
| pkg/sqlite/testdata/incorrectbundle/3scale-community-operator/3scale-community-operator.package.yaml | Removes sqlite incorrect-bundle test fixture. |
| pkg/sqlite/testdata/incorrectbundle/3scale-community-operator/0.3.0/tenants.capabilities.3scale.net.crd.yaml | Removes sqlite incorrect-bundle test fixture. |
| pkg/sqlite/testdata/incorrectbundle/3scale-community-operator/0.3.0/plans.capabilities.3scale.net.crd.yaml | Removes sqlite incorrect-bundle test fixture. |
| pkg/sqlite/testdata/incorrectbundle/3scale-community-operator/0.3.0/metrics.capabilities.3scale.net.crd.yaml | Removes sqlite incorrect-bundle test fixture. |
| pkg/sqlite/testdata/incorrectbundle/3scale-community-operator/0.3.0/mappingrules.capabilities.3scale.net.crd.yaml | Removes sqlite incorrect-bundle test fixture. |
| pkg/sqlite/testdata/incorrectbundle/3scale-community-operator/0.3.0/limits.capabilities.3scale.net.crd.yaml | Removes sqlite incorrect-bundle test fixture. |
| pkg/sqlite/testdata/incorrectbundle/3scale-community-operator/0.3.0/bindings.capabilities.3scale.net.crd.yaml | Removes sqlite incorrect-bundle test fixture. |
| pkg/sqlite/testdata/incorrectbundle/3scale-community-operator/0.3.0/apimanagers.apps.3scale.net.crd.yaml | Removes sqlite incorrect-bundle test fixture. |
| pkg/sqlite/testdata/.gitignore | Removes sqlite testdata ignore rules. |
| pkg/sqlite/stranded.go | Removes sqlite stranded bundle remover implementation. |
| pkg/sqlite/stranded_test.go | Removes sqlite stranded bundle remover tests. |
| pkg/sqlite/sqlitefakes/fake_rowscanner.go | Removes generated fakes tied to sqlite interfaces. |
| pkg/sqlite/sqlitefakes/fake_querier.go | Removes generated fakes tied to sqlite interfaces. |
| pkg/sqlite/remove.go | Removes sqlite package removal implementation. |
| pkg/sqlite/remove_test.go | Removes sqlite package removal tests. |
| pkg/sqlite/migrator.go | Removes sqlite migrator implementation. |
| pkg/sqlite/migrations/migrations.go | Removes sqlite migrations registry/types. |
| pkg/sqlite/migrations/013_rm_truncated_deprecations.go | Removes sqlite migration. |
| pkg/sqlite/migrations/013_rm_truncated_deprecations_test.go | Removes sqlite migration test. |
| pkg/sqlite/migrations/012_deprecated.go | Removes sqlite migration. |
| pkg/sqlite/migrations/012_deprecated_test.go | Removes sqlite migration test. |
| pkg/sqlite/migrations/011_susbtitutes_for.go | Removes sqlite migration. |
| pkg/sqlite/migrations/010_set_bundlepath_pkg_property.go | Removes sqlite migration. |
| pkg/sqlite/migrations/009_properties.go | Removes sqlite migration. |
| pkg/sqlite/migrations/008_dependencies.go | Removes sqlite migration. |
| pkg/sqlite/migrations/007_replaces_skips.go | Removes sqlite migration. |
| pkg/sqlite/migrations/005_version_skiprange.go | Removes sqlite migration. |
| pkg/sqlite/migrations/003_required_apis.go | Removes sqlite migration. |
| pkg/sqlite/migrations/002_bundle_path.go | Removes sqlite migration. |
| pkg/sqlite/migrations/002_bundle_path_test.go | Removes sqlite migration test. |
| pkg/sqlite/migrations/001_related_images.go | Removes sqlite migration. |
| pkg/sqlite/migrations/000_init.go | Removes sqlite schema initialization migration. |
| pkg/sqlite/loadprocs.go | Removes sqlite loader helper procedures. |
| pkg/sqlite/graphloader.go | Removes sqlite graph loader implementation. |
| pkg/sqlite/graphloader_test.go | Removes sqlite graph loader tests. |
| pkg/sqlite/directory.go | Removes sqlite directory loader implementation. |
| pkg/sqlite/deprecationmessage.go | Removes sqlite deprecation message helper. |
| pkg/sqlite/deprecate.go | Removes sqlite deprecate/truncate logic. |
| pkg/sqlite/db.go | Removes sqlite DB open helpers and driver import. |
| pkg/sqlite/db_options.go | Removes sqlite DB options. |
| pkg/sqlite/conversion.go | Removes sqlite->model conversion path. |
| pkg/sqlite/conversion_test.go | Removes sqlite conversion tests. |
| pkg/sqlite/configmap.go | Removes sqlite configmap loader implementation. |
| pkg/mirror/options.go | Removes mirror options (tied to sqlite DB extraction path). |
| pkg/mirror/mirror.go | Removes index image mirroring implementation (sqlite DB driven). |
| pkg/mirror/mirror_test.go | Removes mirror tests. |
| pkg/lib/registry/registryfakes/fake_registry_deleter.go | Removes generated fakes for sqlite registry updater flow. |
| pkg/lib/registry/registryfakes/fake_registry_adder.go | Removes generated fakes for sqlite registry updater flow. |
| pkg/lib/registry/interfaces.go | Removes registry updater interfaces used by sqlite registry commands. |
| pkg/lib/indexer/testdata/package.yaml | Removes indexer testdata. |
| pkg/lib/indexer/interfaces.go | Removes indexer interfaces (sqlite-based index command plumbing). |
| pkg/lib/indexer/indexerfakes/fake_index_exporter.go | Removes generated fakes for indexer. |
| pkg/lib/indexer/indexerfakes/fake_index_deleter.go | Removes generated fakes for indexer. |
| pkg/lib/indexer/indexerfakes/fake_index_adder.go | Removes generated fakes for indexer. |
| pkg/lib/indexer/indexer_test.go | Removes indexer tests. |
| Makefile | Drops sqlite-related build tag; changes cross-build to CGO disabled. |
| go.mod | Removes direct sqlite-related deps, but still includes go-sqlite3 indirectly (may need cleanup). |
| cmd/registry-server/main.go | Removes deprecated standalone sqlite registry server binary. |
| cmd/opm/root/cmd.go | Removes opm index, opm registry, and opm migrate from root command. |
| cmd/opm/render/cmd.go | Removes sqlite from opm render help/behavior. |
| cmd/opm/registry/serve.go | Removes sqlite-backed opm registry serve. |
| cmd/opm/registry/rm.go | Removes sqlite-backed opm registry rm. |
| cmd/opm/registry/prunestranded.go | Removes sqlite-backed opm registry prune-stranded. |
| cmd/opm/registry/prune.go | Removes sqlite-backed opm registry prune. |
| cmd/opm/registry/mirror.go | Removes unused mirror command (sqlite-based). |
| cmd/opm/registry/deprecatetruncate.go | Removes sqlite-backed deprecate/truncate command. |
| cmd/opm/registry/cmd.go | Removes opm registry command group. |
| cmd/opm/registry/add.go | Removes sqlite-backed opm registry add. |
| cmd/opm/migrate/cmd.go | Removes sqlite->FBC migration command. |
| cmd/opm/index/prunestranded.go | Removes sqlite-backed opm index prune-stranded. |
| cmd/opm/index/prune.go | Removes sqlite-backed opm index prune. |
| cmd/opm/index/export.go | Removes sqlite-backed opm index export. |
| cmd/opm/index/deprecatetruncate.go | Removes sqlite-backed opm index deprecatetruncate. |
| cmd/opm/index/delete.go | Removes sqlite-backed opm index rm. |
| cmd/opm/index/cmd.go | Removes opm index command group. |
| cmd/opm/alpha/render-graph/cmd.go | Removes sqlite refs from alpha render-graph allowed ref set. |
| cmd/initializer/main.go | Removes deprecated initializer binary (manifests -> sqlite DB). |
| cmd/configmap-server/main.go | Removes configmap-server (configmap -> sqlite DB -> gRPC). |
| alpha/property/property.go | Drops channel priority property fields/builders. |
| alpha/action/render.go | Removes sqlite rendering support; now only supports declcfg images/dirs and bundles. |
| alpha/action/migrate.go | Disallows migrating sqlite refs (only declcfg image/dir). |
| alpha/action/migrate_test.go | Skips sqlite migration tests. |
| alpha/action/list.go | Disallows listing via sqlite refs (only declcfg image/dir). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1986 +/- ##
==========================================
- Coverage 58.74% 55.56% -3.19%
==========================================
Files 141 97 -44
Lines 13426 8079 -5347
==========================================
- Hits 7887 4489 -3398
+ Misses 4326 3037 -1289
+ Partials 1213 553 -660 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
Applied the api-diff override label as this would be part of a proposed opm v2.0 with breaking changes. |
Signed-off-by: grokspawn <jordan@nimblewidget.com>
|
I ran a functional and output-format-based gap analysis on the second commit (which eliminates packages/layers). The results are here. Aside from a restriction where the old code assumed that bundles had a 1:1: mapping with channels, the FBC-native validation recognizes that a bundle may belong to an arbitrary set of channels. |
|
/override codecov/project |
|
@perdasilva: Overrode contexts on behalf of perdasilva: codecov/project DetailsIn response to this:
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. |
| } | ||
| csvJSON := string(csvData) | ||
| if len(b.Objects) == 0 { | ||
| b.Objects = []string{csvJSON} |
There was a problem hiding this comment.
Since we're receiving b by value here, will b actually be mutated on the caller's side? Should we pass a pointer?
| return nil, fmt.Errorf("convert properties to api dependencies: %v", err) | ||
| } | ||
|
|
||
| return &api.Bundle{ |
There was a problem hiding this comment.
Do we also need to set Deprecations here?
| seenEntries.Insert(entry.Name) | ||
| cde = cde.Insert(entry.Name) | ||
| } | ||
| channelDefinedEntries[c.Package] = cde |
There was a problem hiding this comment.
should we be keying by channel? Are we rewriting the previous channels entries?
|
|
||
| // Find which channel this bundle belongs to | ||
| channelName := "" | ||
| for _, ch := range channels { |
There was a problem hiding this comment.
what does it looks like if the package belongs to multiple channels?
| replaces = entry.Replaces | ||
| skips = entry.Skips | ||
| skipRange = entry.SkipRange | ||
| break |
There was a problem hiding this comment.
should we also break out from the outer loop? Could we be overwriting previous results?
| } | ||
| } | ||
|
|
||
| apiBundle, err := declcfg.ConvertBundleToAPIBundle(b, *pkg, relevantChannels) |
There was a problem hiding this comment.
api.Bundle has only a single channel in its struct - could this cause problems if a bundle is present in multiple channels?
| Deprecation *string `json:"deprecation,omitempty"` | ||
| } | ||
|
|
||
| type cBundle struct { |
There was a problem hiding this comment.
do we need Deprecation?
| @@ -369,14 +261,6 @@ func testGetBundle(addr string, expected *api.Bundle) func(*testing.T) { | |||
| } | |||
|
|
|||
| func TestGetBundleForChannel(t *testing.T) { | |||
There was a problem hiding this comment.
We're leaving the test method and just nuking its implementation
| } | ||
|
|
||
| func TestGetBundleThatReplaces(t *testing.T) { | ||
| t.Run("Sqlite", testGetBundleThatReplaces(dbAddress, etcdoperatorV0_9_2("alpha", false, false, includeManifestsAll))) |
| } | ||
|
|
||
| func TestGetBundleThatReplacesSynthetic(t *testing.T) { | ||
| t.Run("Sqlite", testGetBundleThatReplacesSynthetic(dbAddress, etcdoperatorV0_9_2("alpha", false, false, includeManifestsAll))) |
There was a problem hiding this comment.
Here too! There are a few more below this as well
| } | ||
|
|
||
| // findChannelHead finds the head bundle of a channel by analyzing the replaces chain. | ||
| func findChannelHead(entries []declcfg.ChannelEntry) (string, error) { |
There was a problem hiding this comment.
duplicated logic also found in pkg/cache/pkgs.go:288-320 and possibly also alpha/declcfg/validate.go:369-398
| cfg, err := declcfg.LoadFS(os.DirFS("path/to/catalog")) | ||
|
|
||
| // Convert to model | ||
| model, err := declcfg.ConvertToModel(cfg) |
There was a problem hiding this comment.
Was this function deleted?
| @@ -117,19 +114,7 @@ | |||
| err = declcfg.Write(cfg, "output.yaml") | |||
There was a problem hiding this comment.
Moved to WriteJSON/WriteYAML/WriteFS possibly?
| ### 3. Working with Declarative Config | ||
| ```go | ||
| // Load declarative config | ||
| cfg, err := declcfg.LoadFS(os.DirFS("path/to/catalog")) |
| @@ -33,7 +30,7 @@ | |||
| - **Go 1.24.4**: Minimum Go version required | |||
There was a problem hiding this comment.
should we bump this to the current 1.26.3?
|
|
||
| ### Libraries | ||
| - **`pkg/client`**: High-level client interface for gRPC API | ||
| - **`pkg/api`**: Low-level client libraries for gRPC interface |
There was a problem hiding this comment.
only the proto stubs remain - should we reword?
| ### Libraries | ||
| - **`pkg/client`**: High-level client interface for gRPC API | ||
| - **`pkg/api`**: Low-level client libraries for gRPC interface | ||
| - **`pkg/registry`**: Core registry types (Packages, Channels, Bundles) |
Description of the change:
A view of how opm would look if we drop all SQLite support (which has been deprecated for 4+ years).
Since the
modelpackage was used as an intermediary, internal common ground for both SQLite and FBC inputs, we don't need it anymore. All functionality has been moved into thedeclcfgpackage.This also drops the channel priority property, which was an alpha, internal implementation that never proved worthwhile.
Motivation for the change:
Reviewer Checklist
/docs