Added apis for exposing servers group feature (#1218)

Author: amirejaz

cmd/thv/app/common.go

@@ -144,3 +144,26 @@ func completeLogsArgs(cmd *cobra.Command, args []string, _ string) ([]string, co
 
 	return completions, cobra.ShellCompDirectiveNoFileComp
 }
+
+// ValidateGroupFlag returns a cobra PreRunE-compatible function
+// that validates the --group flag *if provided*.
+/*func validateGroupFlag() func(cmd *cobra.Command, args []string) error {
+	return func(cmd *cobra.Command, _ []string) error {
+		groupName, err := cmd.Flags().GetString("group")
+		if err != nil {
+			return fmt.Errorf("could not read --group flag: %w", err)
+		}
+
+		if groupName == "" {
+			// Optional flag not provided — no validation needed
+			return nil
+		}
+
+		// Validate if provided
+		if err := validation.ValidateGroupName(groupName); err != nil {
+			return fmt.Errorf("invalid group name in --group: %w", err)
+		}
+
+		return nil
+	}
+}*/

cmd/thv/app/group.go

@@ -5,13 +5,13 @@ import (
 	"context"
 	"fmt"
 	"os"
-	"sort"
 	"strings"
 	"text/tabwriter"
 
 	"github.com/spf13/cobra"
 
 	"github.com/stacklok/toolhive/pkg/groups"
+	"github.com/stacklok/toolhive/pkg/validation"
 	"github.com/stacklok/toolhive/pkg/workloads"
 )
 
@@ -24,9 +24,11 @@ var groupCmd = &cobra.Command{
 var groupCreateCmd = &cobra.Command{
 	Use:   "create [group-name]",
 	Short: "Create a new group of MCP servers",
-	Long:  `Create a new logical group of MCP servers. The group can be used to organize and manage multiple MCP servers together.`,
-	Args:  cobra.ExactArgs(1),
-	RunE:  groupCreateCmdFunc,
+	Long: `Create a new logical group of MCP servers.
+		 The group can be used to organize and manage multiple MCP servers together.`,
+	Args:    cobra.ExactArgs(1),
+	PreRunE: validateGroupArg(),
+	RunE:    groupCreateCmdFunc,
 }
 
 var groupListCmd = &cobra.Command{
@@ -41,8 +43,21 @@ var groupRmCmd = &cobra.Command{
 	Short: "Remove a group and remove workloads from it",
 	Long: "Remove a group and remove all MCP servers from it. By default, this only removes the group " +
 		"membership from workloads without deleting them. Use --with-workloads to also delete the workloads. ",
-	Args: cobra.ExactArgs(1),
-	RunE: groupRmCmdFunc,
+	Args:    cobra.ExactArgs(1),
+	PreRunE: validateGroupArg(),
+	RunE:    groupRmCmdFunc,
+}
+
+func validateGroupArg() func(cmd *cobra.Command, args []string) error {
+	return func(_ *cobra.Command, args []string) error {
+		if len(args) == 0 {
+			return fmt.Errorf("group name is required")
+		}
+		if err := validation.ValidateGroupName(args[0]); err != nil {
+			return fmt.Errorf("invalid group name: %w", err)
+		}
+		return nil
+	}
 }
 
 var withWorkloadsFlag bool
@@ -77,11 +92,6 @@ func groupListCmdFunc(cmd *cobra.Command, _ []string) error {
 		return fmt.Errorf("failed to list groups: %w", err)
 	}
 
-	// Sort groups alphanumerically by name (handles mixed characters, numbers, etc.)
-	sort.Slice(allGroups, func(i, j int) bool {
-		return strings.Compare(allGroups[i].Name, allGroups[j].Name) < 0
-	})
-
 	if len(allGroups) == 0 {
 		fmt.Println("No groups configured.")
 		return nil
@@ -111,14 +121,13 @@ func groupRmCmdFunc(cmd *cobra.Command, args []string) error {
 	if strings.EqualFold(groupName, groups.DefaultGroup) {
 		return fmt.Errorf("cannot delete the %s group", groups.DefaultGroup)
 	}
-
-	groupManager, err := groups.NewManager()
+	manager, err := groups.NewManager()
 	if err != nil {
 		return fmt.Errorf("failed to create group manager: %w", err)
 	}
 
 	// Check if group exists
-	exists, err := groupManager.Exists(ctx, groupName)
+	exists, err := manager.Exists(ctx, groupName)
 	if err != nil {
 		return fmt.Errorf("failed to check if group exists: %w", err)
 	}
@@ -127,7 +136,7 @@ func groupRmCmdFunc(cmd *cobra.Command, args []string) error {
 	}
 
 	// Get all workloads in the group
-	groupWorkloads, err := groupManager.ListWorkloadsInGroup(ctx, groupName)
+	groupWorkloads, err := manager.ListWorkloadsInGroup(ctx, groupName)
 	if err != nil {
 		return fmt.Errorf("failed to list workloads in group: %w", err)
 	}
@@ -154,7 +163,7 @@ func groupRmCmdFunc(cmd *cobra.Command, args []string) error {
 		return err
 	}
 
-	if err = groupManager.Delete(ctx, groupName); err != nil {
+	if err = manager.Delete(ctx, groupName); err != nil {
 		return fmt.Errorf("failed to delete group: %w", err)
 	}
 

cmd/thv/app/list.go

@@ -1,15 +1,13 @@
 package app
 
 import (
-	"context"
 	"encoding/json"
 	"fmt"
 	"os"
 	"text/tabwriter"
 
 	"github.com/spf13/cobra"
 
-	"github.com/stacklok/toolhive/pkg/groups"
 	"github.com/stacklok/toolhive/pkg/logger"
 	"github.com/stacklok/toolhive/pkg/workloads"
 )
@@ -34,6 +32,8 @@ func init() {
 	listCmd.Flags().StringArrayVarP(&listLabelFilter, "label", "l", []string{}, "Filter workloads by labels (format: key=value)")
 	// TODO: Re-enable when group functionality is complete
 	// listCmd.Flags().StringVar(&listGroupFilter, "group", "", "Filter workloads by group")
+
+	// listCmd.PreRunE = validateGroupFlag()
 }
 
 func listCmdFunc(cmd *cobra.Command, _ []string) error {
@@ -52,7 +52,7 @@ func listCmdFunc(cmd *cobra.Command, _ []string) error {
 
 	// Apply group filtering if specified
 	if listGroupFilter != "" {
-		workloadList, err = filterWorkloadsByGroup(ctx, workloadList, listGroupFilter)
+		workloadList, err = workloads.FilterByGroup(ctx, workloadList, listGroupFilter)
 		if err != nil {
 			return fmt.Errorf("failed to filter workloads by group: %v", err)
 		}
@@ -79,47 +79,6 @@ func listCmdFunc(cmd *cobra.Command, _ []string) error {
 	}
 }
 
-// filterWorkloadsByGroup filters workloads to only include those in the specified group
-func filterWorkloadsByGroup(
-	ctx context.Context, workloadList []workloads.Workload, groupName string,
-) ([]workloads.Workload, error) {
-	// Create group manager
-	groupManager, err := groups.NewManager()
-	if err != nil {
-		return nil, fmt.Errorf("failed to create group manager: %v", err)
-	}
-
-	// Check if the group exists
-	exists, err := groupManager.Exists(ctx, groupName)
-	if err != nil {
-		return nil, fmt.Errorf("failed to check if group exists: %v", err)
-	}
-	if !exists {
-		return nil, fmt.Errorf("group '%s' does not exist", groupName)
-	}
-
-	// Get all workload names in the specified group
-	groupWorkloadNames, err := groupManager.ListWorkloadsInGroup(ctx, groupName)
-	if err != nil {
-		return nil, fmt.Errorf("failed to list workloads in group: %v", err)
-	}
-
-	groupWorkloadMap := make(map[string]struct{})
-	for _, name := range groupWorkloadNames {
-		groupWorkloadMap[name] = struct{}{}
-	}
-
-	// Filter workloads that belong to the specified group
-	var filteredWorkloads []workloads.Workload
-	for _, workload := range workloadList {
-		if _, ok := groupWorkloadMap[workload.Name]; ok {
-			filteredWorkloads = append(filteredWorkloads, workload)
-		}
-	}
-
-	return filteredWorkloads, nil
-}
-
 // printJSONOutput prints workload information in JSON format
 func printJSONOutput(workloadList []workloads.Workload) error {
 	// Marshal to JSON

cmd/thv/app/restart.go

@@ -32,6 +32,8 @@ func init() {
 	//
 	//// Mark the flags as mutually exclusive
 	//restartCmd.MarkFlagsMutuallyExclusive("all", "group")
+
+	// restartCmd.PreRunE = validateGroupFlag()
 }
 
 func restartCmdFunc(cmd *cobra.Command, args []string) error {

cmd/thv/app/rm.go

@@ -22,6 +22,8 @@ var rmCmd = &cobra.Command{
 func init() {
 	// TODO: Re-enable when group functionality is complete
 	// rmCmd.Flags().String("group", "", "Delete all workloads in the specified group")
+
+	// rmCmd.PreRunE = validateGroupFlag()
 }
 
 //nolint:gocyclo // This function is complex but manageable

cmd/thv/app/run.go

@@ -72,6 +72,8 @@ func init() {
 	// Add run flags
 	AddRunFlags(runCmd, &runFlags)
 
+	//runCmd.PreRunE = validateGroupFlag()
+
 	// This is used for the K8s operator which wraps the run command, but shouldn't be visible to users.
 	if err := runCmd.Flags().MarkHidden("k8s-pod-patch"); err != nil {
 		logger.Warnf("Error hiding flag: %v", err)