Add migration/pkg/migration core library (Phases 1–5) by tmshort · Pull Request #14 · operator-framework/library-olm

tmshort · 2026-08-26T19:41:44Z

Summary

Part 4/6 in the OLMv0→OLMv1 migration library stack (OPRUN-4717).

Implements the core migration library, porting and adapting the
perdasilva/operator-controller prototype
onto current OLMv1 APIs with full R1–R9 requirements coverage per
specs/20260821-migration-v0-to-v1/requirements.md.

Also replaces the stub Makefile and go.mod (from PR #13) with the real versions wired to the bingo tools from PR #12.

Package: `migration/pkg/migration/`

File	Responsibility
`types.go`	`OperatorStatus` (4-state enum), `Options` (all flags + acknowledge overrides), `MigrationInfo`, `Backup.SaveToDisk`
`checks.go`	`CheckResult`, `PreMigrationReport`
`readiness.go`	C8 (Subscription state + CSV health, overridable), C9 (`olm.generated-by` hard block)
`compatibility.go`	C1–C6; soft checks gated on `Acknowledge*` flags; C2/C9 hard; C3 removed (APIService renderer landed in operator-controller PR #2885)
`scan.go`	`ScanAll`, `Check`, `Gather`, `Rollback`, `Cleanup` (canonical R1.1 API); 4-state classification; C7 catalog check; dependent-operator warning (R9)
`catalog.go`	`ResolveClusterCatalog`; `defaultChannel` resolution from FBC (R4); `PackageNotFoundError`
`collector.go`	5-source resource collection (R5): Operator CR refs, CRD labels, `olm.owner` label, ownerRefs, InstallPlan steps; dedup by Group/Kind/ns/name
`phase.go`	`PhaseSort` — groups objects into ordered phases (namespaces→crds→rbac→deploy→publish)
`secretpacker.go`	Secret-backed COS objects (R2.4): gzip compression, 900 KiB batching, content-addressed keys
`migration.go`	`Migrate` (full flow), `Rollback`, `CleanupOLMv0Resources`; cert-pivot warning (R9)

Key design decisions

ClusterObjectSet (not ClusterExtensionRevision) throughout (R2.2)
CollisionProtection: IfNoController on all COS objects — enables CRD adoption (R2.4)
CE spec.serviceAccount never set — deprecated in OLMv1 (R2.5/R7)
All five migration annotations on COS and CE (R2.5)
spec.config → CE.spec.config.inline.deploymentConfig (R4/R7)
Standard clientcmd kubeconfig chain — HCP-compatible (R10)

Reviewer Checklist

API Go Documentation
Tests: Unit Tests (and E2E Tests, if appropriate)
Comprehensive Commit Messages
Links to related GitHub Issue(s)

coderabbitai · 2026-08-26T19:41:53Z

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 436166ce-be31-4dd6-b86f-afe5f64961d6

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

openshift-ci · 2026-08-26T19:42:00Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign tmshort for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

Implements the OLMv0→OLMv1 migration library as specified in specs/20260821-migration-v0-to-v1/requirements.md, porting and adapting the perdasilva/operator-controller prototype onto current OLMv1 APIs. Package layout: migration/pkg/migration/ labels.go — annotation and label constants (R2.5) types.go — OperatorStatus (4-state), Options, MigrationInfo, Backup.SaveToDisk (R1.3, R2.6) checks.go — CheckResult, PreMigrationReport readiness.go — C8 (Subscription + CSV state), C9 (olm.generated-by) compatibility.go — C1–C6 checks; C1/C4/C5/C6/C8 soft with acknowledge flags; C2/C9 hard blocks; C3 removed (OPRUN-4723) scan.go — ScanAll, Check, Gather, Rollback, Cleanup (R1.1); 4-state classification; C7 catalog check; dependent- operator warning (R9) catalog.go — ResolveClusterCatalog; defaultChannel from FBC (R4); PackageNotFoundError collector.go — 5-source resource collection (R5): Operator CR refs, CRD labels, olm.owner label, ownerRefs, InstallPlan steps; dedup by Group/Kind/namespace/name phase.go — PhaseSort: groups objects into ordered phases secretpacker.go — Secret-backed COS objects (R2.4): gzip, 900 KiB batching, content-addressed keys migration.go — Migrate (full flow), Rollback, CleanupOLMv0Resources Key design points: - ClusterObjectSet (not ClusterExtensionRevision) throughout (R2.2) - CollisionProtection: IfNoController on all COS objects (R2.4) - CE spec.serviceAccount never set — deprecated in OLMv1 (R2.5/R7) - All migration annotations on COS and CE (R2.5) - Default channel resolved from ClusterCatalog FBC when Subscription has no spec.channel (R4) - spec.config mapped to CE.spec.config.inline.deploymentConfig (R4/R7) - kubeconfig via standard clientcmd chain — HCP-compatible (R10) Also adds/replaces: go.mod / go.sum — real module dependencies Makefile — real targets wired to bingo-managed golangci-lint and go-apidiff .bingo/ — bingo tool pins (golangci-lint v2.8.0, go-apidiff v0.8.3) .golangci.yaml — lint config Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com> Signed-off-by: Todd Short <tshort@redhat.com>

perdasilva · 2026-08-27T13:41:09Z

+		}
+
+		switch meta.Schema {
+		case "olm.package":


should we care about moving these strings to constants?

perdasilva · 2026-08-27T13:50:24Z

+
+// possibleResourceGVKs lists all resource GVKs that may be part of an OLMv0 operator installation.
+var possibleResourceGVKs = []schema.GroupVersionKind{
+	{Group: "", Version: "v1", Kind: "Namespace"},


Namespace might not be needed here as v0 doesn't manage namespaces

perdasilva · 2026-08-27T13:51:03Z

+
+// clusterScopedKinds is the set of kinds that are cluster-scoped (no namespace in lookups).
+var clusterScopedKinds = map[string]bool{
+	"Namespace":                      true,


perdasilva · 2026-08-27T13:51:58Z

+}
+
+// olmv0OnlyKinds are OLMv0 management resources that should not be included in the COS.
+var olmv0OnlyKinds = map[string]bool{


Dunno if we want to include ClusterServiceVersion, InstallPlan, and Subscription here?

perdasilva · 2026-08-27T13:54:05Z

+		}
+	} else if sub.Status.Install != nil {
+		// Fallback to the deprecated status.install field (R4).
+		ipName = sub.Status.Install.Name


probably being overzealous here but maybe we don't want to rely on a deprecated status field?

perdasilva · 2026-08-27T13:54:23Z

+		// Fallback to the deprecated status.install field (R4).
+		ipName = sub.Status.Install.Name
+	}
+	if ipName != "" {


Should we bail if the install plan is not set on the subscription status?

perdasilva · 2026-08-27T13:57:32Z

+	seen := make(map[string]bool)
+	var collected []unstructured.Unstructured
+
+	addIfNew := func(obj unstructured.Unstructured) {


is it worth just making collected a map then?

perdasilva · 2026-08-27T14:00:58Z

+	return crdList.Items, nil
+}
+
+func (m *Migrator) gatherResourcesByOwnerLabel(ctx context.Context, csvName string) []unstructured.Unstructured {


Suggested change

func (m *Migrator) gatherResourcesByOwnerLabel(ctx context.Context, csvName string) []unstructured.Unstructured {

func (m *Migrator) gatherResourcesByOwnerLabel(ctx context.Context, ownerName string) []unstructured.Unstructured {

perdasilva · 2026-08-27T14:02:37Z

+	return result
+}
+
+func (m *Migrator) gatherResourcesByOwnerRef(ctx context.Context, namespace string, csv *operatorsv1alpha1.ClusterServiceVersion) []unstructured.Unstructured {


Suggested change

func (m *Migrator) gatherResourcesByOwnerRef(ctx context.Context, namespace string, csv *operatorsv1alpha1.ClusterServiceVersion) []unstructured.Unstructured {

func (m *Migrator) gatherResourcesByOwnerRef(ctx context.Context, namespace string, ownerName string) []unstructured.Unstructured {

perdasilva · 2026-08-27T14:02:51Z

+
+		for _, obj := range list.Items {
+			for _, ref := range obj.GetOwnerReferences() {
+				if ref.Kind == "ClusterServiceVersion" && ref.Name == csv.Name {


Suggested change

if ref.Kind == "ClusterServiceVersion" && ref.Name == csv.Name {

if ref.Kind == "ClusterServiceVersion" && ref.Name == ownerName {

perdasilva · 2026-08-27T14:06:12Z

+		return nil, nil
+	}
+
+	skipKinds := map[string]bool{


should this be a map? should we move this list as top level unexported var and use it also in gatherResourcesFromInstallPlan - or move the skip check to a function or something?

perdasilva · 2026-08-27T14:12:04Z

+	// Dependency checks (C2 — hard block)
+	report.Checks = append(report.Checks, checkNoDependencies(bundleProperties)...)
+
+	// C3 (APIService definitions) was removed: OLMv1 now manages APIService objects


this probably needs to be updated now =/

perdasilva · 2026-08-27T14:26:13Z

+	// spec.serviceAccountName (C6 — soft)
+	if og.Spec.ServiceAccountName != "" {
+		if opts.AcknowledgeScopedServiceAccount {
+			checks = append(checks, CheckResult{


it could we worth creating builder functions for these CheckResults - but I'm fine either way

perdasilva · 2026-08-27T14:46:02Z

+		OwnerName:       opts.ClusterExtensionName,
+		SystemNamespace: systemNS,
+	}
+	packed, err := packer.pack(phases)


it would be cool to make this an exported function in some package somewhere in library-olm - and also to be able to configure it to only pack resources in Secrets if necessary, otherwise use the inline option...

perdasilva · 2026-08-27T14:51:44Z

+
+// WaitForClusterExtensionInstalled waits for the CE to reach Installed=True.
+func (m *Migrator) WaitForClusterExtensionInstalled(ctx context.Context, ceName string) error {
+	return wait.PollUntilContextTimeout(ctx, 5*time.Second, 5*time.Minute, true, func(ctx context.Context) (bool, error) {


should we move the timeout and poll period to constants somewhere?

perdasilva · 2026-08-27T14:53:52Z

+
+	deleted := 0
+	for i := range csvList.Items {
+		if err := m.Client.Delete(ctx, &csvList.Items[i], client.PropagationPolicy(metav1.DeletePropagationOrphan)); err != nil {


I wonder if the olmv0 controller cleans up the copied csvs on deletion - it might be worth a check

perdasilva · 2026-08-27T15:02:41Z

+}
+
+// splitNamespacedName splits "namespace/name" into its components.
+func splitNamespacedName(ref string) (string, string, error) {


maybe strings.Split ?

perdasilva

a few nits - nothing that I would block merging on if they are addressed in a follow up

perdasilva · 2026-08-27T15:17:50Z

/lgtm

openshift-ci Bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 26, 2026

openshift-ci Bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 26, 2026

tmshort force-pushed the olm-library-core branch from b4fda3c to 27f0887 Compare August 26, 2026 19:48

openshift-ci Bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 26, 2026

tmshort force-pushed the olm-library-core branch from 27f0887 to 68208c2 Compare August 26, 2026 19:49

tmshort marked this pull request as ready for review August 26, 2026 19:49

openshift-ci Bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Aug 26, 2026

openshift-ci Bot requested review from fgiudici and pedjak August 26, 2026 19:49

tmshort changed the title ~~olm library core~~ Add migration/pkg/migration core library (Phases 1–5) Aug 26, 2026

tmshort force-pushed the olm-library-core branch from 68208c2 to 832b418 Compare August 26, 2026 19:57

tmshort force-pushed the olm-library-core branch from 832b418 to 78c03ae Compare August 26, 2026 20:27

perdasilva reviewed Aug 27, 2026

View reviewed changes

openshift-ci Bot assigned perdasilva Aug 27, 2026

openshift-ci Bot added the lgtm Indicates that a PR is ready to be merged. label Aug 27, 2026

tmshort merged commit 743e3df into main Aug 27, 2026
6 of 7 checks passed

tmshort deleted the olm-library-core branch August 27, 2026 17:42

	func (m *Migrator) gatherResourcesByOwnerLabel(ctx context.Context, csvName string) []unstructured.Unstructured {
	func (m *Migrator) gatherResourcesByOwnerLabel(ctx context.Context, ownerName string) []unstructured.Unstructured {

	if ref.Kind == "ClusterServiceVersion" && ref.Name == csv.Name {
	if ref.Kind == "ClusterServiceVersion" && ref.Name == ownerName {

Uh oh!

Conversation

tmshort commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Package: migration/pkg/migration/

Key design decisions

Reviewer Checklist

Uh oh!

coderabbitai Bot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

openshift-ci Bot commented Aug 26, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

perdasilva left a comment

Choose a reason for hiding this comment

Uh oh!

perdasilva commented Aug 27, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

tmshort commented Aug 26, 2026 •

edited

Loading

Package: `migration/pkg/migration/`

coderabbitai Bot commented Aug 26, 2026 •

edited

Loading