diff --git a/go/core/cli/cmd/kagent/main.go b/go/core/cli/cmd/kagent/main.go index 8792835b0..7ea369a70 100644 --- a/go/core/cli/cmd/kagent/main.go +++ b/go/core/cli/cmd/kagent/main.go @@ -429,12 +429,29 @@ Examples: rootCmd.AddCommand(installCmd, uninstallCmd, invokeCmd, bugReportCmd, versionCmd, dashboardCmd, getCmd, initCmd, buildCmd, deployCmd, addMcpCmd, runCmd, mcp.NewMCPCmd(), envdoc.NewEnvCmd()) - // Initialize config + // Initialize config file and viper defaults if err := config.Init(); err != nil { fmt.Fprintf(os.Stderr, "Error initializing config: %v\n", err) os.Exit(1) } + // Bind cobra flags to viper so config file values are used when flags + // are not explicitly set on the command line. + config.BindFlags(rootCmd.PersistentFlags()) + + // Populate cfg from the merged viper state (config file + env + flags) + // before any subcommand runs, ensuring all commands see config file values. + originalPreRunE := rootCmd.PersistentPreRunE + rootCmd.PersistentPreRunE = func(cmd *cobra.Command, args []string) error { + if err := config.Apply(cfg); err != nil { + return fmt.Errorf("error applying config: %w", err) + } + if originalPreRunE != nil { + return originalPreRunE(cmd, args) + } + return nil + } + if err := rootCmd.ExecuteContext(ctx); err != nil { fmt.Fprintf(os.Stderr, "Error: %v\n", err) diff --git a/go/core/cli/internal/config/config.go b/go/core/cli/internal/config/config.go index 04cd31f8c..8a549ada6 100644 --- a/go/core/cli/internal/config/config.go +++ b/go/core/cli/internal/config/config.go @@ -62,6 +62,30 @@ func Init() error { return nil } +// BindFlags binds cobra persistent flags to viper keys so that config file +// values are used as defaults when the corresponding CLI flag is not explicitly +// set. Flag names use dashes (e.g. "kagent-url") while config file keys use +// underscores (e.g. "kagent_url"), so each binding is explicit. +func BindFlags(flags *pflag.FlagSet) { + viper.BindPFlag("kagent_url", flags.Lookup("kagent-url")) //nolint:errcheck + viper.BindPFlag("namespace", flags.Lookup("namespace")) //nolint:errcheck + viper.BindPFlag("output_format", flags.Lookup("output-format")) //nolint:errcheck + viper.BindPFlag("verbose", flags.Lookup("verbose")) //nolint:errcheck + viper.BindPFlag("timeout", flags.Lookup("timeout")) //nolint:errcheck +} + +// Apply populates the given Config from the merged viper state (config file + +// env + CLI flags). Call this after flag parsing to ensure cfg reflects all +// sources with the correct precedence: CLI flag > env > config file > default. +func Apply(cfg *Config) error { + merged, err := Get() + if err != nil { + return err + } + *cfg = *merged + return nil +} + func Get() (*Config, error) { var config Config if err := viper.Unmarshal(&config); err != nil { diff --git a/go/core/cli/internal/config/config_test.go b/go/core/cli/internal/config/config_test.go new file mode 100644 index 000000000..14cc966f1 --- /dev/null +++ b/go/core/cli/internal/config/config_test.go @@ -0,0 +1,133 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/spf13/pflag" + "github.com/spf13/viper" +) + +func TestConfigFileValuesAreUsed(t *testing.T) { + // Reset viper for a clean test + viper.Reset() + + // Create a temp config directory + tmpDir := t.TempDir() + configFile := filepath.Join(tmpDir, "config.yaml") + + // Write a config file with custom values + configContent := `kagent_url: http://custom-server:9090 +namespace: my-namespace +output_format: json +verbose: true +timeout: 60s +` + if err := os.WriteFile(configFile, []byte(configContent), 0644); err != nil { + t.Fatalf("Failed to write config file: %v", err) + } + + // Set up viper to read the config + viper.SetConfigFile(configFile) + viper.SetConfigType("yaml") + viper.SetDefault("kagent_url", "http://localhost:8083") + viper.SetDefault("output_format", "table") + viper.SetDefault("namespace", "kagent") + viper.SetDefault("timeout", 300*time.Second) + + if err := viper.ReadInConfig(); err != nil { + t.Fatalf("Failed to read config: %v", err) + } + + // Create flags (simulating what main.go does) + flags := pflag.NewFlagSet("test", pflag.ContinueOnError) + flags.String("kagent-url", "http://localhost:8083", "KAgent URL") + flags.StringP("namespace", "n", "kagent", "Namespace") + flags.StringP("output-format", "o", "table", "Output format") + flags.BoolP("verbose", "v", false, "Verbose output") + flags.Duration("timeout", 300*time.Second, "Timeout") + + // Bind flags to viper + BindFlags(flags) + + // Get merged config (should use config file values since no flags were explicitly set) + cfg := &Config{} + if err := Apply(cfg); err != nil { + t.Fatalf("Apply failed: %v", err) + } + + if cfg.KAgentURL != "http://custom-server:9090" { + t.Errorf("Expected KAgentURL=http://custom-server:9090, got %s", cfg.KAgentURL) + } + if cfg.Namespace != "my-namespace" { + t.Errorf("Expected Namespace=my-namespace, got %s", cfg.Namespace) + } + if cfg.OutputFormat != "json" { + t.Errorf("Expected OutputFormat=json, got %s", cfg.OutputFormat) + } + if !cfg.Verbose { + t.Error("Expected Verbose=true, got false") + } + if cfg.Timeout != 60*time.Second { + t.Errorf("Expected Timeout=60s, got %v", cfg.Timeout) + } +} + +func TestCLIFlagsOverrideConfigFile(t *testing.T) { + // Reset viper for a clean test + viper.Reset() + + // Create a temp config directory + tmpDir := t.TempDir() + configFile := filepath.Join(tmpDir, "config.yaml") + + // Write a config file with custom values + configContent := `kagent_url: http://config-server:9090 +namespace: config-ns +` + if err := os.WriteFile(configFile, []byte(configContent), 0644); err != nil { + t.Fatalf("Failed to write config file: %v", err) + } + + viper.SetConfigFile(configFile) + viper.SetConfigType("yaml") + viper.SetDefault("kagent_url", "http://localhost:8083") + viper.SetDefault("namespace", "kagent") + viper.SetDefault("output_format", "table") + viper.SetDefault("timeout", 300*time.Second) + + if err := viper.ReadInConfig(); err != nil { + t.Fatalf("Failed to read config: %v", err) + } + + // Create and parse flags with explicit values + flags := pflag.NewFlagSet("test", pflag.ContinueOnError) + flags.String("kagent-url", "http://localhost:8083", "KAgent URL") + flags.StringP("namespace", "n", "kagent", "Namespace") + flags.StringP("output-format", "o", "table", "Output format") + flags.BoolP("verbose", "v", false, "Verbose output") + flags.Duration("timeout", 300*time.Second, "Timeout") + + // Simulate passing --kagent-url on the CLI + if err := flags.Parse([]string{"--kagent-url", "http://cli-override:7777"}); err != nil { + t.Fatalf("Failed to parse flags: %v", err) + } + + BindFlags(flags) + + cfg := &Config{} + if err := Apply(cfg); err != nil { + t.Fatalf("Apply failed: %v", err) + } + + // CLI flag should win over config file + if cfg.KAgentURL != "http://cli-override:7777" { + t.Errorf("Expected KAgentURL=http://cli-override:7777, got %s", cfg.KAgentURL) + } + // Config file value should still be used for namespace (not overridden by CLI) + if cfg.Namespace != "config-ns" { + t.Errorf("Expected Namespace=config-ns, got %s", cfg.Namespace) + } +}