🌱 consolidate Release types with operator-registry#2771
🌱 consolidate Release types with operator-registry#2771rashmigottipati wants to merge 2 commits into
Conversation
✅ Deploy Preview for olmv1 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[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 operator-controller’s duplicate bundle.Release / bundle.VersionRelease definitions and standardizes on declcfg.Release / declcfg.VersionRelease from operator-registry, keeping the API boundary (BundleMetadata.Release as *string) intact via conversion helpers.
Changes:
- Replaced internal
bundle.VersionReleaseusage across resolver, catalogmetadata filtering/comparison, and tests withdeclcfg.VersionRelease. - Moved legacy registry+v1 “release-in-build-metadata” parsing logic into package-local helpers where needed, and updated metadata conversion (
bundleutil.MetadataFor) accordingly. - Removed the now-redundant
internal/operator-controller/bundle/versionrelease.goand its unit tests; updated generated CRD artifacts touched by regeneration.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| manifests/experimental.yaml | Updates generated experimental manifests (includes CRD generator version annotation change for ClusterExtension CRD). |
| manifests/experimental-e2e.yaml | Updates generated experimental e2e manifests (includes CRD generator version annotation change for ClusterExtension CRD). |
| internal/operator-controller/resolve/resolver.go | Changes resolver interface to return *declcfg.VersionRelease instead of the internal type. |
| internal/operator-controller/resolve/catalog.go | Propagates declcfg.VersionRelease through catalog resolution return types. |
| internal/operator-controller/resolve/catalog_test.go | Updates resolver unit tests to assert against declcfg.VersionRelease. |
| internal/operator-controller/controllers/clusterextension_controller_test.go | Updates controller tests to use declcfg.VersionRelease in mocked resolver responses. |
| internal/operator-controller/catalogmetadata/filter/successors.go | Converts installed bundle metadata parsing to produce *declcfg.VersionRelease, including legacy registry+v1 parsing helper. |
| internal/operator-controller/catalogmetadata/filter/successors_test.go | Adjusts successor/filter tests to use declcfg.VersionRelease parsing helper. |
| internal/operator-controller/catalogmetadata/filter/bundle_predicates.go | Updates version/release predicate API to take declcfg.VersionRelease and compare via declcfg’s Compare semantics. |
| internal/operator-controller/catalogmetadata/compare/compare.go | Updates bundle version/release comparison to use declcfg.VersionRelease. |
| internal/operator-controller/bundleutil/bundle.go | Updates bundle property parsing and MetadataFor conversion to use declcfg.Release / declcfg.VersionRelease and new legacy parsing/conversion helpers. |
| internal/operator-controller/bundleutil/bundle_test.go | Updates bundleutil tests for declcfg.VersionRelease expectations. |
| internal/operator-controller/bundle/versionrelease.go | Deletes the duplicated internal release/version types and logic. |
| internal/operator-controller/bundle/versionrelease_test.go | Deletes unit tests for the removed internal release/version types. |
| helm/olmv1/base/operator-controller/crd/experimental/olm.operatorframework.io_clusterextensions.yaml | Updates generated Helm base CRD for ClusterExtension (includes generator version annotation change). |
💡 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 @@
## main #2771 +/- ##
==========================================
- Coverage 66.78% 66.75% -0.04%
==========================================
Files 149 148 -1
Lines 11382 11367 -15
==========================================
- Hits 7602 7588 -14
+ Misses 3221 3217 -4
- Partials 559 562 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
2865724 to
ccc8f3b
Compare
tmshort
left a comment
There was a problem hiding this comment.
Most of this is changing the namespace of an API, but there is some de-duplication that could be done.
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
ac0cad4 to
e0fe939
Compare
e0fe939 to
da8ab9c
Compare
Signed-off-by: Rashmi Gottipati <rgottipa@redhat.com>
da8ab9c to
21e1ef3
Compare
| buildMetadata := strings.Join(vr.Version.Build, ".") | ||
|
|
||
| rel, err := declcfg.NewRelease(buildMetadata) | ||
| if err == nil && len(rel) > 0 { | ||
| // If the version build metadata parses successfully as a release | ||
| // then use it as a release and drop the build metadata | ||
| // |
|
|
||
| buildMetadata := strings.Join(vr.Version.Build, ".") | ||
|
|
||
| rel, err := declcfg.NewRelease(buildMetadata) |
There was a problem hiding this comment.
NewRelease here introduces a length limit on buildMetadata (https://github.com/operator-framework/operator-registry/blob/master/alpha/model/versionrelease.go#L84) - this is technically a breaking change since we didn't have a limit before. Are we concerned about this at all? Could it break existing bundles? Should we call this out in docs somewhere or mark this PR as a breaking change? (cc @grokspawn)
518364a to
21e1ef3
Compare
Description
Removes duplicate
bundle.Releaseandbundle.VersionReleasetype definitions in favor of usingdeclcfg.Releaseanddeclcfg.VersionReleasefrom operator-registry.This consolidates type definitions between operator-controller and operator-registry, eliminating code duplication and aligning with the vision from OPRUN-4548.
Changes:
internal/operator-controller/bundle/versionrelease.goand the test filedeclcfg.VersionReleaseanddeclcfg.Releasefrom operator-registryBundleMetadata.Release) remains*stringwith conversion at:bundleutil.MetadataFor(declcfg -> API)catalogmetadata/filter.parseInstalledBundleVersionRelease(API -> declcfg)Reviewer Checklist