Skip to content

Commit e667810

Browse files
authored
Add host and port flags to vmcp serve command (#2467)
* Add host and port flags to vmcp serve command Adds --host and --port flags to the vmcp serve command to allow configuring the bind address and port at runtime. This change also refactors the runServe function to reduce cyclomatic complexity by extracting helper functions for configuration loading and backend discovery, using existing vmcp package constructs. * Remove unnecessary mustGetIntFlag helper Cobra validates flag types at parse time, so explicit error checking is not needed when retrieving flags defined by the command. Addresses PR feedback from @amirejaz
1 parent 7dc02b1 commit e667810

File tree

1 file changed

+56
-27
lines changed

1 file changed

+56
-27
lines changed

cmd/vmcp/app/commands.go

Lines changed: 56 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"github.com/stacklok/toolhive/pkg/groups"
1313
"github.com/stacklok/toolhive/pkg/logger"
14+
"github.com/stacklok/toolhive/pkg/vmcp"
1415
"github.com/stacklok/toolhive/pkg/vmcp/aggregator"
1516
"github.com/stacklok/toolhive/pkg/vmcp/auth/factory"
1617
vmcpclient "github.com/stacklok/toolhive/pkg/vmcp/client"
@@ -75,7 +76,7 @@ func NewRootCmd() *cobra.Command {
7576

7677
// newServeCmd creates the serve command for starting the vMCP server
7778
func newServeCmd() *cobra.Command {
78-
return &cobra.Command{
79+
cmd := &cobra.Command{
7980
Use: "serve",
8081
Short: "Start the Virtual MCP Server",
8182
Long: `Start the Virtual MCP Server to aggregate and proxy multiple MCP servers.
@@ -85,6 +86,12 @@ listening for MCP client connections. It will aggregate tools, resources, and pr
8586
from all configured backend MCP servers.`,
8687
RunE: runServe,
8788
}
89+
90+
// Add serve-specific flags
91+
cmd.Flags().String("host", "127.0.0.1", "Host address to bind to")
92+
cmd.Flags().Int("port", 4483, "Port to listen on")
93+
94+
return cmd
8895
}
8996

9097
// newVersionCmd creates the version command
@@ -170,80 +177,98 @@ func getVersion() string {
170177
return "dev"
171178
}
172179

173-
// runServe implements the serve command logic
174-
func runServe(cmd *cobra.Command, _ []string) error {
175-
ctx := cmd.Context()
176-
configPath := viper.GetString("config")
177-
178-
if configPath == "" {
179-
return fmt.Errorf("no configuration file specified, use --config flag")
180-
}
181-
180+
// loadAndValidateConfig loads and validates the vMCP configuration file
181+
func loadAndValidateConfig(configPath string) (*config.Config, error) {
182182
logger.Infof("Loading configuration from: %s", configPath)
183183

184-
// Load configuration from YAML
185184
loader := config.NewYAMLLoader(configPath)
186185
cfg, err := loader.Load()
187186
if err != nil {
188187
logger.Errorf("Failed to load configuration: %v", err)
189-
return fmt.Errorf("configuration loading failed: %w", err)
188+
return nil, fmt.Errorf("configuration loading failed: %w", err)
190189
}
191190

192-
// Validate configuration
193191
validator := config.NewValidator()
194192
if err := validator.Validate(cfg); err != nil {
195193
logger.Errorf("Configuration validation failed: %v", err)
196-
return fmt.Errorf("validation failed: %w", err)
194+
return nil, fmt.Errorf("validation failed: %w", err)
197195
}
198196

199197
logger.Infof("Configuration loaded and validated successfully")
200198
logger.Infof(" Name: %s", cfg.Name)
201199
logger.Infof(" Group: %s", cfg.GroupRef)
202200
logger.Infof(" Conflict Resolution: %s", cfg.Aggregation.ConflictResolution)
203201

202+
return cfg, nil
203+
}
204+
205+
// discoverBackends initializes managers, discovers backends, and creates backend client
206+
func discoverBackends(ctx context.Context, cfg *config.Config) ([]vmcp.Backend, vmcp.BackendClient, error) {
204207
// Initialize managers for backend discovery
205208
logger.Info("Initializing workload and group managers")
206209
workloadsManager, err := workloads.NewManager(ctx)
207210
if err != nil {
208-
return fmt.Errorf("failed to create workloads manager: %w", err)
211+
return nil, nil, fmt.Errorf("failed to create workloads manager: %w", err)
209212
}
210213

211214
groupsManager, err := groups.NewManager()
212215
if err != nil {
213-
return fmt.Errorf("failed to create groups manager: %w", err)
216+
return nil, nil, fmt.Errorf("failed to create groups manager: %w", err)
214217
}
215218

216219
// Create outgoing authentication registry from configuration
217220
logger.Info("Initializing outgoing authentication")
218221
outgoingRegistry, err := factory.NewOutgoingAuthRegistry(ctx, cfg.OutgoingAuth)
219222
if err != nil {
220-
return fmt.Errorf("failed to create outgoing authentication registry: %w", err)
223+
return nil, nil, fmt.Errorf("failed to create outgoing authentication registry: %w", err)
221224
}
222225

223-
// Create backend discoverer
226+
// Create backend discoverer and discover backends
224227
discoverer := aggregator.NewCLIBackendDiscoverer(workloadsManager, groupsManager, cfg.OutgoingAuth)
225228

226-
// Discover backends from the configured group
227229
logger.Infof("Discovering backends in group: %s", cfg.GroupRef)
228230
backends, err := discoverer.Discover(ctx, cfg.GroupRef)
229231
if err != nil {
230-
return fmt.Errorf("failed to discover backends: %w", err)
232+
return nil, nil, fmt.Errorf("failed to discover backends: %w", err)
231233
}
232234

233235
if len(backends) == 0 {
234-
return fmt.Errorf("no backends found in group %s", cfg.GroupRef)
236+
return nil, nil, fmt.Errorf("no backends found in group %s", cfg.GroupRef)
235237
}
236238

237239
logger.Infof("Discovered %d backends", len(backends))
238240

239241
// Create backend client
240242
backendClient, err := vmcpclient.NewHTTPBackendClient(outgoingRegistry)
241243
if err != nil {
242-
return fmt.Errorf("failed to create backend client: %w", err)
244+
return nil, nil, fmt.Errorf("failed to create backend client: %w", err)
245+
}
246+
247+
return backends, backendClient, nil
248+
}
249+
250+
// runServe implements the serve command logic
251+
func runServe(cmd *cobra.Command, _ []string) error {
252+
ctx := cmd.Context()
253+
configPath := viper.GetString("config")
254+
255+
if configPath == "" {
256+
return fmt.Errorf("no configuration file specified, use --config flag")
257+
}
258+
259+
// Load and validate configuration
260+
cfg, err := loadAndValidateConfig(configPath)
261+
if err != nil {
262+
return err
243263
}
244264

245-
// Create conflict resolver based on configuration
246-
// Use the factory method that handles all strategies
265+
// Discover backends and create client
266+
backends, backendClient, err := discoverBackends(ctx, cfg)
267+
if err != nil {
268+
return err
269+
}
270+
271+
// Create conflict resolver and aggregator
247272
conflictResolver, err := aggregator.NewConflictResolver(cfg.Aggregation)
248273
if err != nil {
249274
return fmt.Errorf("failed to create conflict resolver: %w", err)
@@ -281,12 +306,16 @@ func runServe(cmd *cobra.Command, _ []string) error {
281306

282307
logger.Infof("Incoming authentication configured: %s", cfg.IncomingAuth.Type)
283308

284-
// Create server configuration
309+
// Create server configuration with flags
310+
// Cobra validates flag types at parse time, so these values are safe to use directly
311+
host, _ := cmd.Flags().GetString("host")
312+
port, _ := cmd.Flags().GetInt("port")
313+
285314
serverCfg := &vmcpserver.Config{
286315
Name: cfg.Name,
287316
Version: getVersion(),
288-
Host: "127.0.0.1", // TODO: Make configurable
289-
Port: 4483, // TODO: Make configurable
317+
Host: host,
318+
Port: port,
290319
AuthMiddleware: authMiddleware,
291320
AuthInfoHandler: authInfoHandler,
292321
}

0 commit comments

Comments
 (0)