Skip to content

Commit ac74604

Browse files
lburgazzolicursoragent
lburgazzoli
and
cursoragent
committed
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>
1 parent a16dacc commit ac74604

30 files changed

+548
-390
lines changed

‎cmd/lint/lint.go‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -67,25 +67,26 @@ const cmdExample = `
6767
`
6868

6969
// AddCommand adds the lint command to the root command.
70-
func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) {
70+
func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) error {
7171
streams := genericiooptions.IOStreams{
7272
In: root.InOrStdin(),
7373
Out: root.OutOrStdout(),
7474
ErrOut: root.ErrOrStderr(),
7575
}
7676

77-
// Create command with ConfigFlags from parent to ensure CLI auth flags are used
78-
command := lintpkg.NewCommand(streams, flags)
77+
command, err := lintpkg.NewCommand(streams, flags)
78+
if err != nil {
79+
return fmt.Errorf("initializing lint command: %w", err)
80+
}
7981

8082
cmd := &cobra.Command{
8183
Use: cmdName,
8284
Short: cmdShort,
8385
Long: cmdLong,
8486
Example: cmdExample,
8587
SilenceUsage: true,
86-
SilenceErrors: true, // We'll handle error output manually based on --quiet flag
88+
SilenceErrors: true,
8789
RunE: func(cmd *cobra.Command, _ []string) error {
88-
// Complete phase
8990
if err := command.Complete(); err != nil {
9091
if command.Verbose {
9192
_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Error: %v\n", err)
@@ -94,7 +95,6 @@ func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) {
9495
return fmt.Errorf("completing command: %w", err)
9596
}
9697

97-
// Validate phase
9898
if err := command.Validate(); err != nil {
9999
if command.Verbose {
100100
_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "Error: %v\n", err)
@@ -103,7 +103,6 @@ func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) {
103103
return fmt.Errorf("validating command: %w", err)
104104
}
105105

106-
// Run phase
107106
err := command.Run(cmd.Context())
108107
if err != nil {
109108
if command.Verbose {
@@ -117,8 +116,9 @@ func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) {
117116
},
118117
}
119118

120-
// Register flags using AddFlags method
121119
command.AddFlags(cmd.Flags())
122120

123121
root.AddCommand(cmd)
122+
123+
return nil
124124
}

‎cmd/main.go‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,14 @@ func main() {
2626
flags.AddFlags(cmd.PersistentFlags())
2727

2828
version.AddCommand(cmd, flags)
29-
lint.AddCommand(cmd, flags)
29+
30+
if err := lint.AddCommand(cmd, flags); err != nil {
31+
if _, writeErr := os.Stderr.WriteString(err.Error() + "\n"); writeErr != nil {
32+
os.Exit(1)
33+
}
34+
35+
os.Exit(1)
36+
}
3037

3138
if err := cmd.Execute(); err != nil {
3239
if _, writeErr := os.Stderr.WriteString(err.Error() + "\n"); writeErr != nil {

‎cmd/migrate/list/list.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package list
22

33
import (
4+
"fmt"
5+
46
"github.com/spf13/cobra"
57

68
"k8s.io/cli-runtime/pkg/genericclioptions"
@@ -40,8 +42,12 @@ func AddCommand(
4042
parent *cobra.Command,
4143
flags *genericclioptions.ConfigFlags,
4244
streams genericiooptions.IOStreams,
43-
) {
44-
command := migrate.NewListCommand(streams)
45+
) error {
46+
command, err := migrate.NewListCommand(streams)
47+
if err != nil {
48+
return fmt.Errorf("initializing list command: %w", err)
49+
}
50+
4551
command.ConfigFlags = flags
4652

4753
cmd := &cobra.Command{
@@ -67,4 +73,6 @@ func AddCommand(
6773

6874
command.AddFlags(cmd.Flags())
6975
parent.AddCommand(cmd)
76+
77+
return nil
7078
}

‎cmd/migrate/migrate.go‎

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package migrate
22

33
import (
4+
"fmt"
5+
46
"github.com/spf13/cobra"
57

68
"k8s.io/cli-runtime/pkg/genericclioptions"
@@ -54,7 +56,7 @@ const cmdExample = `
5456
`
5557

5658
// AddCommand adds the migrate command to the root command.
57-
func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) {
59+
func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) error {
5860
streams := genericiooptions.IOStreams{
5961
In: root.InOrStdin(),
6062
Out: root.OutOrStdout(),
@@ -70,9 +72,19 @@ func AddCommand(root *cobra.Command, flags *genericclioptions.ConfigFlags) {
7072
SilenceErrors: true,
7173
}
7274

73-
list.AddCommand(cmd, flags, streams)
74-
prepare.AddCommand(cmd, flags, streams)
75-
run.AddCommand(cmd, flags, streams)
75+
if err := list.AddCommand(cmd, flags, streams); err != nil {
76+
return fmt.Errorf("adding list subcommand: %w", err)
77+
}
78+
79+
if err := prepare.AddCommand(cmd, flags, streams); err != nil {
80+
return fmt.Errorf("adding prepare subcommand: %w", err)
81+
}
82+
83+
if err := run.AddCommand(cmd, flags, streams); err != nil {
84+
return fmt.Errorf("adding run subcommand: %w", err)
85+
}
7686

7787
root.AddCommand(cmd)
88+
89+
return nil
7890
}

‎cmd/migrate/prepare/prepare.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package prepare
22

33
import (
4+
"fmt"
5+
46
"github.com/spf13/cobra"
57

68
"k8s.io/cli-runtime/pkg/genericclioptions"
@@ -50,8 +52,12 @@ func AddCommand(
5052
parent *cobra.Command,
5153
flags *genericclioptions.ConfigFlags,
5254
streams genericiooptions.IOStreams,
53-
) {
54-
command := migrate.NewPrepareCommand(streams)
55+
) error {
56+
command, err := migrate.NewPrepareCommand(streams)
57+
if err != nil {
58+
return fmt.Errorf("initializing prepare command: %w", err)
59+
}
60+
5561
command.ConfigFlags = flags
5662

5763
cmd := &cobra.Command{
@@ -77,4 +83,6 @@ func AddCommand(
7783

7884
command.AddFlags(cmd.Flags())
7985
parent.AddCommand(cmd)
86+
87+
return nil
8088
}

‎cmd/migrate/run/run.go‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,8 @@
11
package run
22

33
import (
4+
"fmt"
5+
46
"github.com/spf13/cobra"
57

68
"k8s.io/cli-runtime/pkg/genericclioptions"
@@ -47,8 +49,12 @@ func AddCommand(
4749
parent *cobra.Command,
4850
flags *genericclioptions.ConfigFlags,
4951
streams genericiooptions.IOStreams,
50-
) {
51-
command := migrate.NewRunCommand(streams)
52+
) error {
53+
command, err := migrate.NewRunCommand(streams)
54+
if err != nil {
55+
return fmt.Errorf("initializing run command: %w", err)
56+
}
57+
5258
command.ConfigFlags = flags
5359

5460
cmd := &cobra.Command{
@@ -74,4 +80,6 @@ func AddCommand(
7480

7581
command.AddFlags(cmd.Flags())
7682
parent.AddCommand(cmd)
83+
84+
return nil
7785
}

‎docs/lint/architecture.md‎

Lines changed: 51 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -82,43 +82,43 @@ func NewCommand(
8282
streams genericiooptions.IOStreams,
8383
configFlags *genericclioptions.ConfigFlags,
8484
options ...CommandOption,
85-
) *Command {
86-
registry := check.NewRegistry()
87-
88-
// Explicitly register all checks (no global state, full test isolation)
89-
// Components (13)
90-
registry.MustRegister(codeflare.NewRemovalCheck())
91-
registry.MustRegister(dashboard.NewAcceleratorProfileMigrationCheck())
92-
registry.MustRegister(dashboard.NewHardwareProfileMigrationCheck())
93-
registry.MustRegister(datasciencepipelines.NewInstructLabRemovalCheck())
94-
registry.MustRegister(datasciencepipelines.NewRenamingCheck())
95-
registry.MustRegister(kserve.NewServerlessRemovalCheck())
96-
registry.MustRegister(kserve.NewInferenceServiceConfigCheck())
97-
registry.MustRegister(kserve.NewServiceMeshOperatorCheck())
98-
registry.MustRegister(kserve.NewServiceMeshRemovalCheck())
99-
registry.MustRegister(kueue.NewManagementStateCheck())
100-
registry.MustRegister(kueue.NewOperatorInstalledCheck())
101-
registry.MustRegister(modelmesh.NewRemovalCheck())
102-
registry.MustRegister(trainingoperator.NewDeprecationCheck())
103-
104-
// Dependencies (2)
105-
registry.MustRegister(certmanager.NewCheck())
106-
registry.MustRegister(openshift.NewCheck())
107-
108-
// Workloads (8)
109-
registry.MustRegister(guardrails.NewImpactedWorkloadsCheck())
110-
registry.MustRegister(guardrails.NewOtelMigrationCheck())
111-
registry.MustRegister(kserveworkloads.NewAcceleratorMigrationCheck())
112-
registry.MustRegister(kserveworkloads.NewImpactedWorkloadsCheck())
113-
registry.MustRegister(notebook.NewAcceleratorMigrationCheck())
114-
registry.MustRegister(notebook.NewImpactedWorkloadsCheck())
115-
registry.MustRegister(ray.NewImpactedWorkloadsCheck())
116-
registry.MustRegister(trainingoperatorworkloads.NewImpactedWorkloadsCheck())
85+
) (*Command, error) {
86+
registry, err := check.NewRegistry(
87+
// Components (13)
88+
codeflare.NewRemovalCheck(),
89+
dashboard.NewAcceleratorProfileMigrationCheck(),
90+
dashboard.NewHardwareProfileMigrationCheck(),
91+
datasciencepipelines.NewInstructLabRemovalCheck(),
92+
datasciencepipelines.NewRenamingCheck(),
93+
kserve.NewServerlessRemovalCheck(),
94+
kserve.NewInferenceServiceConfigCheck(),
95+
kserve.NewServiceMeshOperatorCheck(),
96+
kserve.NewServiceMeshRemovalCheck(),
97+
kueue.NewManagementStateCheck(),
98+
kueue.NewOperatorInstalledCheck(),
99+
modelmesh.NewRemovalCheck(),
100+
trainingoperator.NewDeprecationCheck(),
101+
// Dependencies (2)
102+
certmanager.NewCheck(),
103+
openshift.NewCheck(),
104+
// Workloads (8)
105+
guardrails.NewImpactedWorkloadsCheck(),
106+
guardrails.NewOtelMigrationCheck(),
107+
kserveworkloads.NewAcceleratorMigrationCheck(),
108+
kserveworkloads.NewImpactedWorkloadsCheck(),
109+
notebook.NewAcceleratorMigrationCheck(),
110+
notebook.NewImpactedWorkloadsCheck(),
111+
ray.NewImpactedWorkloadsCheck(),
112+
trainingoperatorworkloads.NewImpactedWorkloadsCheck(),
113+
)
114+
if err != nil {
115+
return nil, fmt.Errorf("registering checks: %w", err)
116+
}
117117

118118
return &Command{
119119
SharedOptions: shared,
120120
registry: registry,
121-
}
121+
}, nil
122122
}
123123
```
124124

@@ -344,15 +344,27 @@ The lint command uses a `Command` struct (not `Options`) with constructor `NewCo
344344

345345
```go
346346
type Command struct {
347-
shared *SharedOptions
348-
targetVersion string
347+
*SharedOptions
348+
TargetVersion version.SemVersion // implements pflag.Value
349+
registry *check.CheckRegistry
349350
}
350351

351-
func NewCommand(opts CommandOptions) *Command {
352-
return &Command{
353-
shared: opts.Shared,
354-
targetVersion: opts.TargetVersion,
352+
func NewCommand(
353+
streams genericiooptions.IOStreams,
354+
configFlags *genericclioptions.ConfigFlags,
355+
options ...CommandOption,
356+
) (*Command, error) {
357+
registry, err := check.NewRegistry(/* checks... */)
358+
if err != nil {
359+
return nil, fmt.Errorf("registering checks: %w", err)
360+
}
361+
362+
c := &Command{
363+
SharedOptions: NewSharedOptions(streams, configFlags),
364+
registry: registry,
355365
}
366+
// ... apply options ...
367+
return c, nil
356368
}
357369
```
358370

0 commit comments

Comments
 (0)