Refactor all default config values into config.go

proxy/config.go

@@ -31,6 +31,34 @@ type ModelConfig struct {
 	ConcurrencyLimit int `yaml:"concurrencyLimit"`
 }
 
+func (m *ModelConfig) UnmarshalYAML(unmarshal func(interface{}) error) error {
+	type rawModelConfig ModelConfig
+	defaults := rawModelConfig{
+		Cmd:              "",
+		CmdStop:          "",
+		Proxy:            "http://localhost:${PORT}",
+		Aliases:          []string{},
+		Env:              []string{},
+		CheckEndpoint:    "/health",
+		UnloadAfter:      0,
+		Unlisted:         false,
+		UseModelName:     "",
+		ConcurrencyLimit: 0,
+	}
+
+	// the default cmdStop to taskkill /f /t /pid ${PID}
+	if runtime.GOOS == "windows" {
+		defaults.CmdStop = "taskkill /f /t /pid ${PID}"
+	}
+
+	if err := unmarshal(&defaults); err != nil {
+		return err
+	}
+
+	*m = ModelConfig(defaults)
+	return nil
+}
+
 func (m *ModelConfig) SanitizedCommand() ([]string, error) {
 	return SanitizeCommand(m.Cmd)
 }
@@ -111,26 +139,23 @@ func LoadConfigFromReader(r io.Reader) (Config, error) {
 		return Config{}, err
 	}
 
-	var config Config
+	// default configuration values
+	config := Config{
+		HealthCheckTimeout: 120,
+		StartPort:          5800,
+		LogLevel:           "info",
+	}
 	err = yaml.Unmarshal(data, &config)
 	if err != nil {
 		return Config{}, err
 	}
 
-	if config.HealthCheckTimeout == 0 {
-		// this high default timeout helps avoid failing health checks
-		// for configurations that wait for docker or have slower startup
-		config.HealthCheckTimeout = 120
-	} else if config.HealthCheckTimeout < 15 {
+	if config.HealthCheckTimeout < 15 {
 		// set a minimum of 15 seconds
 		config.HealthCheckTimeout = 15
 	}
 
-	// set default port ranges
-	if config.StartPort == 0 {
-		// default to 5800
-		config.StartPort = 5800
-	} else if config.StartPort < 1 {
+	if config.StartPort < 1 {
 		return Config{}, fmt.Errorf("startPort must be greater than 1")
 	}
 
@@ -180,6 +205,11 @@ func LoadConfigFromReader(r io.Reader) (Config, error) {
 	for _, modelId := range modelIds {
 		modelConfig := config.Models[modelId]
 
+		// enforce ${PORT} used in both cmd and proxy
+		if !strings.Contains(modelConfig.Cmd, "${PORT}") && strings.Contains(modelConfig.Proxy, "${PORT}") {
+			return Config{}, fmt.Errorf("model %s: proxy uses ${PORT} but cmd does not - ${PORT} is only available when used in cmd", modelId)
+		}
+
 		// go through model config fields: cmd, cmdStop, proxy, checkEndPoint and replace macros with macro values
 		for macroName, macroValue := range config.Macros {
 			macroSlug := fmt.Sprintf("${%s}", macroName)
@@ -191,17 +221,11 @@ func LoadConfigFromReader(r io.Reader) (Config, error) {
 
 		// only iterate over models that use ${PORT} to keep port numbers from increasing unnecessarily
 		if strings.Contains(modelConfig.Cmd, "${PORT}") || strings.Contains(modelConfig.Proxy, "${PORT}") || strings.Contains(modelConfig.CmdStop, "${PORT}") {
-			if modelConfig.Proxy == "" {
-				modelConfig.Proxy = "http://localhost:${PORT}"
-			}
-
 			nextPortStr := strconv.Itoa(nextPort)
 			modelConfig.Cmd = strings.ReplaceAll(modelConfig.Cmd, "${PORT}", nextPortStr)
 			modelConfig.CmdStop = strings.ReplaceAll(modelConfig.CmdStop, "${PORT}", nextPortStr)
 			modelConfig.Proxy = strings.ReplaceAll(modelConfig.Proxy, "${PORT}", nextPortStr)
 			nextPort++
-		} else if modelConfig.Proxy == "" {
-			return Config{}, fmt.Errorf("model %s requires a proxy value when not using automatic ${PORT}", modelId)
 		}
 
 		// make sure there are no unknown macros that have not been replaced
@@ -217,6 +241,9 @@ func LoadConfigFromReader(r io.Reader) (Config, error) {
 			matches := macroPattern.FindAllStringSubmatch(fieldValue, -1)
 			for _, match := range matches {
 				macroName := match[1]
+				if macroName == "PID" && fieldName == "cmdStop" {
+					continue // this is ok, has to be replaced by process later
+				}
 				if _, exists := config.Macros[macroName]; !exists {
 					return Config{}, fmt.Errorf("unknown macro '${%s}' found in %s.%s", macroName, modelId, fieldName)
 				}

proxy/config_posix_test.go

@@ -3,6 +3,9 @@
 package proxy
 
 import (
+	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -40,3 +43,184 @@ func TestConfig_SanitizeCommand(t *testing.T) {
 	assert.Error(t, err)
 	assert.Nil(t, args)
 }
+
+// Test the default values are automatically set for global, model and group configurations
+// after loading the configuration
+func TestConfig_DefaultValuesPosix(t *testing.T) {
+	content := `
+models:
+  model1:
+    cmd: path/to/cmd --port ${PORT}
+`
+
+	config, err := LoadConfigFromReader(strings.NewReader(content))
+	assert.NoError(t, err)
+	assert.Equal(t, 120, config.HealthCheckTimeout)
+	assert.Equal(t, 5800, config.StartPort)
+	assert.Equal(t, "info", config.LogLevel)
+
+	// Test default group exists
+	defaultGroup, exists := config.Groups["(default)"]
+	assert.True(t, exists, "default group should exist")
+	if assert.NotNil(t, defaultGroup, "default group should not be nil") {
+		assert.Equal(t, true, defaultGroup.Swap)
+		assert.Equal(t, true, defaultGroup.Exclusive)
+		assert.Equal(t, false, defaultGroup.Persistent)
+		assert.Equal(t, []string{"model1"}, defaultGroup.Members)
+	}
+
+	model1, exists := config.Models["model1"]
+	assert.True(t, exists, "model1 should exist")
+	if assert.NotNil(t, model1, "model1 should not be nil") {
+		assert.Equal(t, "path/to/cmd --port 5800", model1.Cmd) // has the port replaced
+		assert.Equal(t, "", model1.CmdStop)
+		assert.Equal(t, "http://localhost:5800", model1.Proxy)
+		assert.Equal(t, "/health", model1.CheckEndpoint)
+		assert.Equal(t, []string{}, model1.Aliases)
+		assert.Equal(t, []string{}, model1.Env)
+		assert.Equal(t, 0, model1.UnloadAfter)
+		assert.Equal(t, false, model1.Unlisted)
+		assert.Equal(t, "", model1.UseModelName)
+		assert.Equal(t, 0, model1.ConcurrencyLimit)
+	}
+}
+
+func TestConfig_LoadPosix(t *testing.T) {
+	// Create a temporary YAML file for testing
+	tempDir, err := os.MkdirTemp("", "test-config")
+	if err != nil {
+		t.Fatalf("Failed to create temporary directory: %v", err)
+	}
+	defer os.RemoveAll(tempDir)
+
+	tempFile := filepath.Join(tempDir, "config.yaml")
+	content := `
+macros:
+  svr-path: "path/to/server"
+models:
+  model1:
+    cmd: path/to/cmd --arg1 one
+    proxy: "http://localhost:8080"
+    aliases:
+      - "m1"
+      - "model-one"
+    env:
+      - "VAR1=value1"
+      - "VAR2=value2"
+    checkEndpoint: "/health"
+  model2:
+    cmd: ${svr-path} --arg1 one
+    proxy: "http://localhost:8081"
+    aliases:
+      - "m2"
+    checkEndpoint: "/"
+  model3:
+    cmd: path/to/cmd --arg1 one
+    proxy: "http://localhost:8081"
+    aliases:
+      - "mthree"
+    checkEndpoint: "/"
+  model4:
+    cmd: path/to/cmd --arg1 one
+    proxy: "http://localhost:8082"
+    checkEndpoint: "/"
+
+healthCheckTimeout: 15
+profiles:
+  test:
+    - model1
+    - model2
+groups:
+  group1:
+    swap: true
+    exclusive: false
+    members: ["model2"]
+  forever:
+    exclusive: false
+    persistent: true
+    members:
+      - "model4"
+`
+
+	if err := os.WriteFile(tempFile, []byte(content), 0644); err != nil {
+		t.Fatalf("Failed to write temporary file: %v", err)
+	}
+
+	// Load the config and verify
+	config, err := LoadConfig(tempFile)
+	if err != nil {
+		t.Fatalf("Failed to load config: %v", err)
+	}
+
+	expected := Config{
+		LogLevel:  "info",
+		StartPort: 5800,
+		Macros: map[string]string{
+			"svr-path": "path/to/server",
+		},
+		Models: map[string]ModelConfig{
+			"model1": {
+				Cmd:           "path/to/cmd --arg1 one",
+				Proxy:         "http://localhost:8080",
+				Aliases:       []string{"m1", "model-one"},
+				Env:           []string{"VAR1=value1", "VAR2=value2"},
+				CheckEndpoint: "/health",
+			},
+			"model2": {
+				Cmd:           "path/to/server --arg1 one",
+				Proxy:         "http://localhost:8081",
+				Aliases:       []string{"m2"},
+				Env:           []string{},
+				CheckEndpoint: "/",
+			},
+			"model3": {
+				Cmd:           "path/to/cmd --arg1 one",
+				Proxy:         "http://localhost:8081",
+				Aliases:       []string{"mthree"},
+				Env:           []string{},
+				CheckEndpoint: "/",
+			},
+			"model4": {
+				Cmd:           "path/to/cmd --arg1 one",
+				Proxy:         "http://localhost:8082",
+				CheckEndpoint: "/",
+				Aliases:       []string{},
+				Env:           []string{},
+			},
+		},
+		HealthCheckTimeout: 15,
+		Profiles: map[string][]string{
+			"test": {"model1", "model2"},
+		},
+		aliases: map[string]string{
+			"m1":        "model1",
+			"model-one": "model1",
+			"m2":        "model2",
+			"mthree":    "model3",
+		},
+		Groups: map[string]GroupConfig{
+			DEFAULT_GROUP_ID: {
+				Swap:      true,
+				Exclusive: true,
+				Members:   []string{"model1", "model3"},
+			},
+			"group1": {
+				Swap:      true,
+				Exclusive: false,
+				Members:   []string{"model2"},
+			},
+			"forever": {
+				Swap:       true,
+				Exclusive:  false,
+				Persistent: true,
+				Members:    []string{"model4"},
+			},
+		},
+	}
+
+	assert.Equal(t, expected, config)
+
+	realname, found := config.RealModelName("m1")
+	assert.True(t, found)
+	assert.Equal(t, "model1", realname)
+}