test: add migration negative tests and recovery fixes

migration/examples/cmd/migrate-operators-v0-to-v1/validation_test.go

@@ -0,0 +1,60 @@
+package main
+
+import (
+	"context"
+	"testing"
+
+	"github.com/spf13/cobra"
+)
+
+// These validations run before constructing a Kubernetes client. Keeping them
+// tested here makes invalid CLI invocations deterministic and side-effect free.
+func TestCommandRejectsAmbiguousAndMissingTargets(t *testing.T) {
+	t.Cleanup(func() { checkAll, convertAll, cleanupAll, rollbackAll = false, false, false, false })
+	cmd := &cobra.Command{}
+	cmd.SetContext(context.Background())
+	tests := []struct {
+		name string
+		run  func([]string) error
+	}{
+		{
+			name: "check",
+			run: func(args []string) error {
+				return runCheck(cmd, args)
+			},
+		},
+		{
+			name: "convert",
+			run: func(args []string) error {
+				return runConvert(cmd, args)
+			},
+		},
+		{
+			name: "cleanup",
+			run: func(args []string) error {
+				return runCleanup(cmd, args)
+			},
+		},
+		{
+			name: "rollback",
+			run: func(args []string) error {
+				return runRollback(cmd, args)
+			},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name+" missing target", func(t *testing.T) {
+			checkAll, convertAll, cleanupAll, rollbackAll = false, false, false, false
+			if err := tt.run(nil); err == nil {
+				t.Fatal("missing target unexpectedly reached the Kubernetes client")
+			}
+		})
+		t.Run(tt.name+" ambiguous target", func(t *testing.T) {
+			checkAll, convertAll, cleanupAll, rollbackAll = true, true, true, true
+			if err := tt.run([]string{"operator"}); err == nil {
+				t.Fatal("target combined with --all unexpectedly reached the Kubernetes client")
+			}
+		})
+	}
+}

migration/pkg/catalogmigration/catalogmigration.go

@@ -20,6 +20,7 @@ import (
 const (
 	// MigratedFromCatalogSourceAnnotation is set on ClusterCatalog when first created or adopted.
 	MigratedFromCatalogSourceAnnotation = "olm.operatorframework.io/migrated-from-catalogsource"
+	clusterCatalogServingTimeout        = 10 * time.Minute
 )
 
 // CatalogMigratorOptions configures the catalog migration.
@@ -346,7 +347,7 @@ func (cm *CatalogMigrator) annotateIfNotPresent(ctx context.Context, cc *ocv1.Cl
 
 // waitForServing polls until the ClusterCatalog has Serving=True.
 func (cm *CatalogMigrator) waitForServing(ctx context.Context, ccName string) error {
-	return wait.PollUntilContextTimeout(ctx, 5*time.Second, 3*time.Minute, true, func(ctx context.Context) (bool, error) {
+	return wait.PollUntilContextTimeout(ctx, 5*time.Second, clusterCatalogServingTimeout, true, func(ctx context.Context) (bool, error) {
 		var cc ocv1.ClusterCatalog
 		if err := cm.Client.Get(ctx, client.ObjectKey{Name: ccName}, &cc); err != nil {
 			return false, err

migration/pkg/catalogmigration/unit_test.go

@@ -144,3 +144,35 @@ func TestWaitForServing(t *testing.T) {
 		t.Fatal("waitForServing(missing) unexpectedly succeeded")
 	}
 }
+
+func TestMigrateCatalogsSkipsUnsupportedSourcesWithoutMutation(t *testing.T) {
+	scheme := runtime.NewScheme()
+	if err := operatorsv1alpha1.AddToScheme(scheme); err != nil {
+		t.Fatal(err)
+	}
+	if err := ocv1.AddToScheme(scheme); err != nil {
+		t.Fatal(err)
+	}
+	sources := []runtime.Object{
+		&operatorsv1alpha1.CatalogSource{ObjectMeta: metav1.ObjectMeta{Name: "configmap", Namespace: "ns"}, Spec: operatorsv1alpha1.CatalogSourceSpec{SourceType: operatorsv1alpha1.SourceTypeConfigmap}},
+		&operatorsv1alpha1.CatalogSource{ObjectMeta: metav1.ObjectMeta{Name: "address-only", Namespace: "ns"}, Spec: operatorsv1alpha1.CatalogSourceSpec{SourceType: operatorsv1alpha1.SourceTypeGrpc, Address: "catalog.ns.svc:50051"}},
+		&operatorsv1alpha1.CatalogSource{ObjectMeta: metav1.ObjectMeta{Name: "unknown", Namespace: "ns"}, Spec: operatorsv1alpha1.CatalogSourceSpec{SourceType: operatorsv1alpha1.SourceType("unsupported")}},
+	}
+	c := fake.NewClientBuilder().WithScheme(scheme).WithRuntimeObjects(sources...).Build()
+	results, err := NewCatalogMigrator(c).MigrateCatalogs(context.Background(), CatalogMigratorOptions{})
+	if err != nil || len(results) != len(sources) {
+		t.Fatalf("MigrateCatalogs() = %#v, %v", results, err)
+	}
+	for _, result := range results {
+		if result.Status != "skipped" || result.ClusterCatalogName != "" || result.Reason == "" {
+			t.Fatalf("unsupported source result = %#v", result)
+		}
+	}
+	var catalogs ocv1.ClusterCatalogList
+	if err := c.List(context.Background(), &catalogs); err != nil {
+		t.Fatal(err)
+	}
+	if len(catalogs.Items) != 0 {
+		t.Fatalf("unsupported CatalogSources created ClusterCatalogs: %#v", catalogs.Items)
+	}
+}

migration/pkg/migration/labels.go

@@ -43,9 +43,6 @@ const (
 	// MigratedFromCatalogSourceAnnotation is set on ClusterCatalog by the catalog migration tool.
 	MigratedFromCatalogSourceAnnotation = "olm.operatorframework.io/migrated-from-catalogsource"
 
-	// fieldManager is the SSA field manager used for all apply operations.
-	fieldManager = "olm.operatorframework.io/migration"
-
 	// cosWaitPollInterval / cosWaitTimeout control how long to wait for a
 	// ClusterObjectSet to reach Succeeded=True.
 	cosWaitPollInterval = 5 * time.Second