refactor: return errors from registry constructors and introduce SemVersion pflag type

Registry constructors (check.NewRegistry, dependencies.NewRegistry,
action.NewRegistry) now return (*Registry, error) instead of panicking
via MustRegister, propagating registration errors to callers.

Replace the TargetVersion string + parsedTargetVersion *semver.Version
pair in the lint Command with a single version.SemVersion type that
implements pflag.Value, validating at flag-parse time.

Co-authored-by: Cursor <cursoragent@cursor.com>

Author: lburgazzoli

cmd/lint/lint.go

@@ -67,25 +67,26 @@ const cmdExample = `
 `
 
 // AddCommand adds the lint command to the root command.
-func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) {
+func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) error {
 	streams := genericiooptions.IOStreams{
 		In:     root.InOrStdin(),
 		Out:    root.OutOrStdout(),
 		ErrOut: root.ErrOrStderr(),
 	}
 
-	// Create command with ConfigFlags from parent to ensure CLI auth flags are used
-	command := lintpkg.NewCommand(streams, flags)
+	command, err := lintpkg.NewCommand(streams, flags)
+	if err != nil {
+		return fmt.Errorf("initializing lint command: %w", err)
+	}
 
 	cmd := &cobra.Command{
 		Use:           cmdName,
 		Short:         cmdShort,
 		Long:          cmdLong,
 		Example:       cmdExample,
 		SilenceUsage:  true,
-		SilenceErrors: true, // We'll handle error output manually based on --quiet flag
+		SilenceErrors: true,
 		RunE: func(cmd *cobra.Command, _ []string) error {
-			// Complete phase
 			if err := command.Complete(); err != nil {
 				if command.Verbose {
 					_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Error: %v\n", err)
@@ -94,7 +95,6 @@ func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) {
 				return fmt.Errorf("completing command: %w", err)
 			}
 
-			// Validate phase
 			if err := command.Validate(); err != nil {
 				if command.Verbose {
 					_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Error: %v\n", err)
@@ -103,7 +103,6 @@ func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) {
 				return fmt.Errorf("validating command: %w", err)
 			}
 
-			// Run phase
 			err := command.Run(cmd.Context())
 			if err != nil {
 				if command.Verbose {
@@ -117,8 +116,9 @@ func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) {
 		},
 	}
 
-	// Register flags using AddFlags method
 	command.AddFlags(cmd.Flags())
 
 	root.AddCommand(cmd)
+
+	return nil
 }

cmd/main.go

@@ -26,7 +26,14 @@ func main() {
 	flags.AddFlags(cmd.PersistentFlags())
 
 	version.AddCommand(cmd, flags)
-	lint.AddCommand(cmd, flags)
+
+	if err := lint.AddCommand(cmd, flags); err != nil {
+		if _, writeErr := os.Stderr.WriteString(err.Error() + "\n"); writeErr != nil {
+			os.Exit(1)
+		}
+
+		os.Exit(1)
+	}
 
 	if err := cmd.Execute(); err != nil {
 		if _, writeErr := os.Stderr.WriteString(err.Error() + "\n"); writeErr != nil {

cmd/migrate/list/list.go

@@ -1,6 +1,8 @@
 package list
 
 import (
+	"fmt"
+
 	"github.com/spf13/cobra"
 
 	"k8s.io/cli-runtime/pkg/genericclioptions"
@@ -40,8 +42,12 @@ func AddCommand(
 	parent *cobra.Command,
 	flags *genericclioptions.ConfigFlags,
 	streams genericiooptions.IOStreams,
-) {
-	command := migrate.NewListCommand(streams)
+) error {
+	command, err := migrate.NewListCommand(streams)
+	if err != nil {
+		return fmt.Errorf("initializing list command: %w", err)
+	}
+
 	command.ConfigFlags = flags
 
 	cmd := &cobra.Command{
@@ -67,4 +73,6 @@ func AddCommand(
 
 	command.AddFlags(cmd.Flags())
 	parent.AddCommand(cmd)
+
+	return nil
 }

cmd/migrate/migrate.go

@@ -1,6 +1,8 @@
 package migrate
 
 import (
+	"fmt"
+
 	"github.com/spf13/cobra"
 
 	"k8s.io/cli-runtime/pkg/genericclioptions"
@@ -54,7 +56,7 @@ const cmdExample = `
 `
 
 // AddCommand adds the migrate command to the root command.
-func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) {
+func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) error {
 	streams := genericiooptions.IOStreams{
 		In:     root.InOrStdin(),
 		Out:    root.OutOrStdout(),
@@ -70,9 +72,19 @@ func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) {
 		SilenceErrors: true,
 	}
 
-	list.AddCommand(cmd, flags, streams)
-	prepare.AddCommand(cmd, flags, streams)
-	run.AddCommand(cmd, flags, streams)
+	if err := list.AddCommand(cmd, flags, streams); err != nil {
+		return fmt.Errorf("adding list subcommand: %w", err)
+	}
+
+	if err := prepare.AddCommand(cmd, flags, streams); err != nil {
+		return fmt.Errorf("adding prepare subcommand: %w", err)
+	}
+
+	if err := run.AddCommand(cmd, flags, streams); err != nil {
+		return fmt.Errorf("adding run subcommand: %w", err)
+	}
 
 	root.AddCommand(cmd)
+
+	return nil
 }

cmd/migrate/prepare/prepare.go

@@ -1,6 +1,8 @@
 package prepare
 
 import (
+	"fmt"
+
 	"github.com/spf13/cobra"
 
 	"k8s.io/cli-runtime/pkg/genericclioptions"
@@ -50,8 +52,12 @@ func AddCommand(
 	parent *cobra.Command,
 	flags *genericclioptions.ConfigFlags,
 	streams genericiooptions.IOStreams,
-) {
-	command := migrate.NewPrepareCommand(streams)
+) error {
+	command, err := migrate.NewPrepareCommand(streams)
+	if err != nil {
+		return fmt.Errorf("initializing prepare command: %w", err)
+	}
+
 	command.ConfigFlags = flags
 
 	cmd := &cobra.Command{
@@ -77,4 +83,6 @@ func AddCommand(
 
 	command.AddFlags(cmd.Flags())
 	parent.AddCommand(cmd)
+
+	return nil
 }

cmd/migrate/run/run.go

@@ -1,6 +1,8 @@
 package run
 
 import (
+	"fmt"
+
 	"github.com/spf13/cobra"
 
 	"k8s.io/cli-runtime/pkg/genericclioptions"
@@ -47,8 +49,12 @@ func AddCommand(
 	parent *cobra.Command,
 	flags *genericclioptions.ConfigFlags,
 	streams genericiooptions.IOStreams,
-) {
-	command := migrate.NewRunCommand(streams)
+) error {
+	command, err := migrate.NewRunCommand(streams)
+	if err != nil {
+		return fmt.Errorf("initializing run command: %w", err)
+	}
+
 	command.ConfigFlags = flags
 
 	cmd := &cobra.Command{
@@ -74,4 +80,6 @@ func AddCommand(
 
 	command.AddFlags(cmd.Flags())
 	parent.AddCommand(cmd)
+
+	return nil
 }

docs/lint/architecture.md

@@ -82,43 +82,43 @@ func NewCommand(
     streams genericiooptions.IOStreams,
     configFlags *genericclioptions.ConfigFlags,
     options ...CommandOption,
-) *Command {
-    registry := check.NewRegistry()
-
-    // Explicitly register all checks (no global state, full test isolation)
-    // Components (13)
-    registry.MustRegister(codeflare.NewRemovalCheck())
-    registry.MustRegister(dashboard.NewAcceleratorProfileMigrationCheck())
-    registry.MustRegister(dashboard.NewHardwareProfileMigrationCheck())
-    registry.MustRegister(datasciencepipelines.NewInstructLabRemovalCheck())
-    registry.MustRegister(datasciencepipelines.NewRenamingCheck())
-    registry.MustRegister(kserve.NewServerlessRemovalCheck())
-    registry.MustRegister(kserve.NewInferenceServiceConfigCheck())
-    registry.MustRegister(kserve.NewServiceMeshOperatorCheck())
-    registry.MustRegister(kserve.NewServiceMeshRemovalCheck())
-    registry.MustRegister(kueue.NewManagementStateCheck())
-    registry.MustRegister(kueue.NewOperatorInstalledCheck())
-    registry.MustRegister(modelmesh.NewRemovalCheck())
-    registry.MustRegister(trainingoperator.NewDeprecationCheck())
-
-    // Dependencies (2)
-    registry.MustRegister(certmanager.NewCheck())
-    registry.MustRegister(openshift.NewCheck())
-
-    // Workloads (8)
-    registry.MustRegister(guardrails.NewImpactedWorkloadsCheck())
-    registry.MustRegister(guardrails.NewOtelMigrationCheck())
-    registry.MustRegister(kserveworkloads.NewAcceleratorMigrationCheck())
-    registry.MustRegister(kserveworkloads.NewImpactedWorkloadsCheck())
-    registry.MustRegister(notebook.NewAcceleratorMigrationCheck())
-    registry.MustRegister(notebook.NewImpactedWorkloadsCheck())
-    registry.MustRegister(ray.NewImpactedWorkloadsCheck())
-    registry.MustRegister(trainingoperatorworkloads.NewImpactedWorkloadsCheck())
+) (*Command, error) {
+    registry, err := check.NewRegistry(
+        // Components (13)
+        codeflare.NewRemovalCheck(),
+        dashboard.NewAcceleratorProfileMigrationCheck(),
+        dashboard.NewHardwareProfileMigrationCheck(),
+        datasciencepipelines.NewInstructLabRemovalCheck(),
+        datasciencepipelines.NewRenamingCheck(),
+        kserve.NewServerlessRemovalCheck(),
+        kserve.NewInferenceServiceConfigCheck(),
+        kserve.NewServiceMeshOperatorCheck(),
+        kserve.NewServiceMeshRemovalCheck(),
+        kueue.NewManagementStateCheck(),
+        kueue.NewOperatorInstalledCheck(),
+        modelmesh.NewRemovalCheck(),
+        trainingoperator.NewDeprecationCheck(),
+        // Dependencies (2)
+        certmanager.NewCheck(),
+        openshift.NewCheck(),
+        // Workloads (8)
+        guardrails.NewImpactedWorkloadsCheck(),
+        guardrails.NewOtelMigrationCheck(),
+        kserveworkloads.NewAcceleratorMigrationCheck(),
+        kserveworkloads.NewImpactedWorkloadsCheck(),
+        notebook.NewAcceleratorMigrationCheck(),
+        notebook.NewImpactedWorkloadsCheck(),
+        ray.NewImpactedWorkloadsCheck(),
+        trainingoperatorworkloads.NewImpactedWorkloadsCheck(),
+    )
+    if err != nil {
+        return nil, fmt.Errorf("registering checks: %w", err)
+    }
 
     return &Command{
         SharedOptions: shared,
         registry:      registry,
-    }
+    }, nil
 }
 ```
 

docs/lint/writing-checks.md

@@ -79,11 +79,14 @@ func (c *Check) Validate(ctx context.Context, target check.Target) (*result.Diag
 }
 ```
 
-**Registration:** Checks are explicitly registered in `pkg/lint/command.go`:
+**Registration:** Checks are explicitly registered in `pkg/lint/command.go` via `check.NewRegistry(...)`:
 
 ```go
 // In NewCommand()
-registry.MustRegister(dashboard.NewCheck())
+registry, err := check.NewRegistry(
+    dashboard.NewCheck(),
+    // ... other checks
+)
 ```
 
 ### Using BaseCheck
@@ -129,31 +132,31 @@ func NewCommand(
     streams genericiooptions.IOStreams,
     configFlags *genericclioptions.ConfigFlags,
     options ...CommandOption,
-) *Command {
-    registry := check.NewRegistry()
-
-    // Explicitly register all checks
-    // Components (11)
-    registry.MustRegister(codeflare.NewRemovalCheck())
-    registry.MustRegister(dashboard.NewAcceleratorProfileMigrationCheck())
-    registry.MustRegister(dashboard.NewHardwareProfileMigrationCheck())
-    registry.MustRegister(datasciencepipelines.NewInstructLabRemovalCheck())
-    // ... additional component checks
-
-    // Dependencies (2)
-    registry.MustRegister(certmanager.NewCheck())
-    // ... additional dependency checks
-
-    // Workloads (7)
-    registry.MustRegister(guardrails.NewOtelMigrationCheck())
-    registry.MustRegister(kserveworkloads.NewAcceleratorMigrationCheck())
-    registry.MustRegister(notebook.NewImpactedWorkloadsCheck())
-    // ... additional workload checks
+) (*Command, error) {
+    registry, err := check.NewRegistry(
+        // Components (13)
+        codeflare.NewRemovalCheck(),
+        dashboard.NewAcceleratorProfileMigrationCheck(),
+        dashboard.NewHardwareProfileMigrationCheck(),
+        datasciencepipelines.NewInstructLabRemovalCheck(),
+        // ... additional component checks
+        // Dependencies (2)
+        certmanager.NewCheck(),
+        // ... additional dependency checks
+        // Workloads (13)
+        guardrails.NewOtelMigrationCheck(),
+        kserveworkloads.NewAcceleratorMigrationCheck(),
+        notebook.NewImpactedWorkloadsCheck(),
+        // ... additional workload checks
+    )
+    if err != nil {
+        return nil, fmt.Errorf("registering checks: %w", err)
+    }
 
     return &Command{
         SharedOptions: shared,
         registry:      registry,
-    }
+    }, nil
 }
 ```
 
@@ -849,8 +852,8 @@ func (c *RemovalCheck) Validate(ctx context.Context, target check.Target) (*resu
         Run(ctx, validate.Removal("CodeFlare is enabled (state: %s) but will be removed in RHOAI 3.x"))
 }
 
-// Registration is done explicitly in pkg/lint/command.go:
-// registry.MustRegister(codeflare.NewRemovalCheck())
+// Registration is done explicitly in pkg/lint/command.go via check.NewRegistry(...)
+// e.g. codeflare.NewRemovalCheck() is passed to check.NewRegistry()
 ```
 
 **Note:** For complex checks that don't fit the builder pattern, you can still write checks without builders using `c.NewResult()` and manually fetching resources via `client.GetDataScienceCluster(ctx, target.Client)`.