diff --git a/.env.template b/.env.template index 2c3c5b5d..68dd2048 100644 --- a/.env.template +++ b/.env.template @@ -5,6 +5,14 @@ # Header used to read/write request user_path values (default: X-GoModel-User-Path) # USER_PATH_HEADER=X-GoModel-User-Path +# Reject unknown keys in config.yaml and in the JSON env vars that declare the same +# structures (VIRTUAL_MODELS, SET_RATE_LIMIT_*, SET_BUDGET_*). Default: true, so a +# typo or a misindented section fails startup instead of silently dropping providers, +# rate limits, budgets, or guardrails. Set false to downgrade unknown keys to +# warnings — intended for rolling a binary back under a newer config file. Malformed +# values stay fatal in either mode. +# CONFIG_STRICT=true + # Tagging based on headers: label every request from the listed headers (numbered # from 1). Labels are recorded in usage tracking and audit logs. A header value can # carry several labels split by the delimiter (default: ","). The optional prefix is diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index ac2d8715..b04cc59d 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -155,7 +155,7 @@ jobs: - name: Set up Go uses: actions/setup-go@v6 with: - go-version: 1.26.4 + go-version: 1.26.5 cache: true - name: Run GoReleaser diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 32e694b3..b457ab0f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -12,7 +12,7 @@ concurrency: cancel-in-progress: true env: - GO_VERSION: "1.26.4" + GO_VERSION: "1.26.5" # Restrict token permissions to minimum required permissions: diff --git a/Dockerfile b/Dockerfile index ae621fb0..e3918eb4 100644 --- a/Dockerfile +++ b/Dockerfile @@ -1,5 +1,5 @@ # Build stage — run on the build host's native arch for speed, cross-compile for target -FROM --platform=$BUILDPLATFORM golang:1.26.4-alpine3.23 AS builder +FROM --platform=$BUILDPLATFORM golang:1.26.5-alpine3.23 AS builder ARG TARGETOS ARG TARGETARCH diff --git a/config/budget.go b/config/budget.go index aa93f298..d08eb3ad 100644 --- a/config/budget.go +++ b/config/budget.go @@ -30,20 +30,21 @@ type BudgetUserPathConfig struct { } // BudgetLimitConfig declares one spend limit for a reset period. +// The json tags support the JSON-array form of SET_BUDGET_* env values. type BudgetLimitConfig struct { // Period accepts hourly, daily, weekly, or monthly. The resolved period is // persisted as PeriodSeconds in the database. - Period string `yaml:"period"` + Period string `yaml:"period" json:"period"` // PeriodSeconds can be set directly instead of Period. Standard values are // 3600, 86400, 604800, and 2592000. - PeriodSeconds int64 `yaml:"period_seconds"` + PeriodSeconds int64 `yaml:"period_seconds" json:"period_seconds"` // Amount is the maximum allowed tracked provider spend for the period. - Amount float64 `yaml:"amount"` + Amount float64 `yaml:"amount" json:"amount"` } -func applyBudgetEnv(cfg *Config) error { +func applyBudgetEnv(cfg *Config, strict bool) error { if cfg == nil { return nil } @@ -54,7 +55,7 @@ func applyBudgetEnv(cfg *Config) error { cfg.Budgets.UserPaths, "SET_BUDGET_", func(entry BudgetUserPathConfig) string { return entry.Path }, - parseBudgetEnvLimits, + func(raw string) ([]BudgetLimitConfig, error) { return parseBudgetEnvLimits(raw, strict) }, func(path string, limits []BudgetLimitConfig) BudgetUserPathConfig { return BudgetUserPathConfig{Path: path, Limits: limits} }, @@ -66,7 +67,7 @@ func applyBudgetEnv(cfg *Config) error { return nil } -func parseBudgetEnvLimits(raw string) ([]BudgetLimitConfig, error) { +func parseBudgetEnvLimits(raw string, strict bool) ([]BudgetLimitConfig, error) { raw = strings.TrimSpace(raw) if raw == "" { return nil, nil @@ -89,7 +90,7 @@ func parseBudgetEnvLimits(raw string) ([]BudgetLimitConfig, error) { } if strings.HasPrefix(raw, "[") { var limits []BudgetLimitConfig - if err := json.Unmarshal([]byte(raw), &limits); err != nil { + if err := decodeIaCJSON("SET_BUDGET_*", raw, &limits, strict); err != nil { return nil, err } return limits, nil diff --git a/config/budget_test.go b/config/budget_test.go new file mode 100644 index 00000000..3e478028 --- /dev/null +++ b/config/budget_test.go @@ -0,0 +1,34 @@ +package config + +import ( + "strings" + "testing" +) + +// The JSON-array form of SET_BUDGET_* decodes through json tags. Without them +// period_seconds was silently dropped, yielding a limit with no window. +func TestParseBudgetEnvLimits_JSONArray(t *testing.T) { + limits, err := parseBudgetEnvLimits(`[{"period_seconds":7200,"amount":5}]`, true) + if err != nil { + t.Fatalf("parseBudgetEnvLimits() error = %v", err) + } + if len(limits) != 1 { + t.Fatalf("len(limits) = %d, want 1", len(limits)) + } + if limits[0].PeriodSeconds != 7200 { + t.Fatalf("PeriodSeconds = %d, want 7200", limits[0].PeriodSeconds) + } + if limits[0].Amount != 5 { + t.Fatalf("Amount = %v, want 5", limits[0].Amount) + } +} + +func TestParseBudgetEnvLimits_RejectsUnknownField(t *testing.T) { + _, err := parseBudgetEnvLimits(`[{"period":"daily","ammount":5}]`, true) + if err == nil { + t.Fatal("parseBudgetEnvLimits() error = nil, want unknown-field error") + } + if !strings.Contains(err.Error(), "ammount") { + t.Fatalf("parseBudgetEnvLimits() error = %q, want it to name the unknown field", err) + } +} diff --git a/config/config.go b/config/config.go index 8cc63b14..91244fed 100644 --- a/config/config.go +++ b/config/config.go @@ -2,8 +2,15 @@ package config import ( + "errors" "fmt" + "io" + "io/fs" + "log/slog" "os" + "regexp" + "strconv" + "strings" "time" "gopkg.in/yaml.v3" @@ -152,7 +159,12 @@ func buildDefaultConfig() *Config { func Load() (*LoadResult, error) { cfg := buildDefaultConfig() - rawProviders, err := applyYAML(cfg) + strict, err := resolveConfigStrict() + if err != nil { + return nil, err + } + + rawProviders, err := applyYAML(cfg, strict) if err != nil { return nil, err } @@ -168,7 +180,7 @@ func Load() (*LoadResult, error) { if err := applyEnvOverrides(cfg); err != nil { return nil, err } - if err := applyVirtualModelsEnv(cfg); err != nil { + if err := applyVirtualModelsEnv(cfg, strict); err != nil { return nil, err } if err := applyTaggingEnv(cfg); err != nil { @@ -178,13 +190,13 @@ func Load() (*LoadResult, error) { return nil, err } applyBudgetDependencies(cfg) - if err := applyBudgetEnv(cfg); err != nil { + if err := applyBudgetEnv(cfg, strict); err != nil { return nil, err } if err := validateBudgetConfig(&cfg.Budgets); err != nil { return nil, err } - if err := applyRateLimitEnv(cfg); err != nil { + if err := applyRateLimitEnv(cfg, strict); err != nil { return nil, err } if err := validateRateLimitConfig(&cfg.RateLimits); err != nil { @@ -225,32 +237,56 @@ func Load() (*LoadResult, error) { }, nil } -// applyYAML reads an optional config.yaml and overlays it onto cfg. +// configFilePaths are searched in order; the first readable file wins. +var configFilePaths = []string{ + "config/config.yaml", + "config.yaml", +} + +const envConfigStrict = "CONFIG_STRICT" + +// resolveConfigStrict reads CONFIG_STRICT, which defaults to true: an unknown key +// in declarative config aborts startup rather than being ignored, because a +// dropped providers, rate_limits, budgets, or guardrails entry silently changes +// routing, cost, or security. Set it to false to downgrade unknown keys to +// warnings — useful when rolling a binary back under a newer config file. +// +// It is read directly from the environment because it governs the parse of the +// YAML layer, which runs before the env-tag overrides are applied. +func resolveConfigStrict() (bool, error) { + raw := strings.TrimSpace(os.Getenv(envConfigStrict)) + if raw == "" { + return true, nil + } + strict, err := strconv.ParseBool(raw) + if err != nil { + return false, fmt.Errorf("invalid %s: %q is not a boolean", envConfigStrict, raw) + } + if !strict { + slog.Warn("CONFIG_STRICT=false: unknown config keys are ignored with a warning instead of aborting startup") + } + return strict, nil +} + +// applyYAML reads an optional config file and overlays it onto cfg. // Returns the raw provider map parsed from the providers: YAML section. // If no config file is found, this is a no-op (not an error). -func applyYAML(cfg *Config) (map[string]RawProviderConfig, error) { - paths := []string{ - "config/config.yaml", - "config.yaml", - } - - var data []byte - for _, p := range paths { - raw, err := os.ReadFile(p) - if err == nil { - data = raw - break - } +// +// When strict, an unknown key is an error rather than a silently ignored one. A +// misindented section — the classic `providers:` followed by entries at column +// zero — otherwise parses as a null section plus unknown top-level keys, and the +// gateway boots with none of the operator's providers. CONFIG_STRICT=false +// downgrades unknown keys to warnings; malformed values stay fatal either way. +func applyYAML(cfg *Config, strict bool) (map[string]RawProviderConfig, error) { + path, data, err := readConfigFile() + if err != nil { + return nil, err } - - rawProviders := make(map[string]RawProviderConfig) - if data == nil { - return rawProviders, nil + slog.Info("no config file found; using defaults and environment", "searched", configFilePaths) + return map[string]RawProviderConfig{}, nil } - expanded := expandString(string(data)) - // yamlTarget is a local struct that mirrors Config for YAML unmarshaling, // using RawProviderConfig for providers so nullable resilience overrides are preserved. type yamlTarget struct { @@ -259,13 +295,121 @@ func applyYAML(cfg *Config) (map[string]RawProviderConfig, error) { } target := yamlTarget{Config: cfg} - if err := yaml.Unmarshal([]byte(expanded), &target); err != nil { - return nil, fmt.Errorf("failed to parse config.yaml: %w", err) + decoder := yaml.NewDecoder(strings.NewReader(expandString(string(data)))) + // Unknown keys are always detected. Whether they are fatal is decided below, + // so the lax mode can still name each one instead of dropping it in silence. + decoder.KnownFields(true) + // A file holding only comments decodes to nothing; that is an empty overlay, + // not a failure. + decodeErr := decoder.Decode(&target) + if decodeErr != nil && !errors.Is(decodeErr, io.EOF) { + if err := reportYAMLDecodeError(path, decodeErr, strict); err != nil { + return nil, err + } } + if err := ensureSingleDocument(path, decoder); err != nil { + return nil, err + } + + slog.Info("config file loaded", "path", path, "providers", len(target.RawProviders)) - if target.RawProviders != nil { - rawProviders = target.RawProviders + if target.RawProviders == nil { + return map[string]RawProviderConfig{}, nil } + return target.RawProviders, nil +} + +// ensureSingleDocument rejects a config file holding more than one YAML document. +// The decoder reads only the first, so everything after a `---` separator would be +// applied nowhere — the same silent loss a misindented section causes. Decoding into +// a yaml.Node accepts any shape, so this detects a second document without +// re-triggering the unknown-key check. A structural fault, fatal regardless of +// CONFIG_STRICT. +func ensureSingleDocument(path string, decoder *yaml.Decoder) error { + var extra yaml.Node + err := decoder.Decode(&extra) + if errors.Is(err, io.EOF) { + return nil + } + if err != nil { + return formatYAMLError(path, err) + } + return fmt.Errorf("failed to parse %s: only one YAML document is supported, found another after a '---' separator", path) +} + +// reportYAMLDecodeError decides the fate of a decode error. Unknown keys are fatal +// when strict and warnings otherwise; every other problem — a malformed value, a +// syntax error — is fatal regardless, because CONFIG_STRICT relaxes what the schema +// accepts, not whether the file makes sense. Returns nil when nothing is fatal. +func reportYAMLDecodeError(path string, err error, strict bool) error { + var typeErr *yaml.TypeError + if strict || !errors.As(err, &typeErr) { + return formatYAMLError(path, err) + } + + var fatal []string + for _, message := range typeErr.Errors { + line, field, ok := parseUnknownFieldMessage(message) + if !ok { + fatal = append(fatal, message) + continue + } + slog.Warn("unknown config key ignored; it has no effect", + "path", path, "line", line, "field", field) + } + if len(fatal) > 0 { + return formatYAMLError(path, &yaml.TypeError{Errors: fatal}) + } + return nil +} + +// unknownFieldMessage matches yaml.v3's unknown-key message, the only decode error +// CONFIG_STRICT=false is allowed to downgrade. +var unknownFieldMessage = regexp.MustCompile(`^line (\d+): field (\S+) not found in type \S+$`) + +func parseUnknownFieldMessage(message string) (line int, field string, ok bool) { + match := unknownFieldMessage.FindStringSubmatch(message) + if match == nil { + return 0, "", false + } + line, err := strconv.Atoi(match[1]) + if err != nil { + return 0, "", false + } + return line, match[2], true +} + +// readConfigFile returns the first config file that exists and its contents, or an +// empty path and nil contents when none does. A file that exists but cannot be read +// — wrong permissions, or a directory mounted where a file was expected — is an +// error, not a missing file: silently falling back to defaults is how a +// misconfigured deployment boots with no providers. +func readConfigFile() (string, []byte, error) { + for _, path := range configFilePaths { + data, err := os.ReadFile(path) + switch { + case err == nil: + return path, data, nil + case errors.Is(err, fs.ErrNotExist): + continue + default: + return "", nil, fmt.Errorf("failed to read %s: %w", path, err) + } + } + return "", nil, nil +} + +// yamlTypeSuffix matches the Go type name yaml.v3 appends to unknown-field errors +// ("field foo not found in type config.yamlTarget"). It names an internal struct +// the operator cannot act on, so it is stripped. +var yamlTypeSuffix = regexp.MustCompile(` in type \S+`) - return rawProviders, nil +// formatYAMLError rewrites a yaml.v3 decode error into a single actionable line +// prefixed with the offending file. +func formatYAMLError(path string, err error) error { + msg := yamlTypeSuffix.ReplaceAllString(err.Error(), "") + msg = strings.TrimPrefix(msg, "yaml: unmarshal errors:\n") + msg = strings.TrimPrefix(msg, "yaml: ") + msg = strings.ReplaceAll(msg, "\n ", "; ") + return fmt.Errorf("failed to parse %s: %s", path, strings.TrimSpace(msg)) } diff --git a/config/config_strict_test.go b/config/config_strict_test.go new file mode 100644 index 00000000..5d30f6fc --- /dev/null +++ b/config/config_strict_test.go @@ -0,0 +1,269 @@ +package config + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// TestLoad_RejectsUnknownYAMLFields locks in strict parsing: an unknown key is a +// startup error, never a silently dropped section. The misindented providers: +// block is the motivating case — it parses as a null section plus unknown +// top-level keys, so the gateway would otherwise boot with no providers at all. +func TestLoad_RejectsUnknownYAMLFields(t *testing.T) { + tests := []struct { + name string + yaml string + wantErr string + }{ + { + name: "misindented providers section", + yaml: ` +server: + port: "9999" + +providers: +uranium-geryon-9b: + type: vllm + base_url: "http://uranium-geryon-9b:8000/v1" +`, + wantErr: `field uranium-geryon-9b not found`, + }, + { + name: "unknown top-level key", + yaml: "bogus_section:\n a: 1\n", + // Reported against the file, without yaml.v3's internal Go type name. + wantErr: "failed to parse config.yaml: line 1: field bogus_section not found", + }, + { + name: "unknown nested key", + yaml: "server:\n prot: \"9999\"\n", + wantErr: "field prot not found", + }, + { + name: "unknown provider key", + yaml: "providers:\n local:\n type: vllm\n bass_url: \"http://x:8000/v1\"\n", + wantErr: "field bass_url not found", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clearAllConfigEnvVars(t) + withTempDir(t, func(dir string) { + writeConfigYAML(t, dir, tt.yaml) + + _, err := Load() + if err == nil { + t.Fatal("Load() succeeded, want unknown-field error") + } + if !strings.Contains(err.Error(), tt.wantErr) { + t.Fatalf("Load() error = %q, want it to contain %q", err, tt.wantErr) + } + if strings.Contains(err.Error(), "in type") { + t.Fatalf("Load() error leaks an internal Go type name: %q", err) + } + }) + }) + } +} + +// TestLoad_AcceptsValidYAMLShapes guards the edges strict parsing must not break: +// an empty or comments-only file is an empty overlay, a correctly indented +// providers: block still loads, and the removed failover.overrides key is still +// tolerated so an old config file keeps booting. +func TestLoad_AcceptsValidYAMLShapes(t *testing.T) { + tests := []struct { + name string + yaml string + wantProviders int + }{ + {name: "empty file", yaml: ""}, + {name: "comments only", yaml: "# nothing to see here\n"}, + {name: "explicit null providers", yaml: "providers:\n"}, + {name: "document end marker", yaml: "server:\n port: \"9999\"\n...\n"}, + { + name: "correctly indented providers", + yaml: "providers:\n local:\n type: vllm\n base_url: \"http://x:8000/v1\"\n", + wantProviders: 1, + }, + { + name: "legacy failover overrides are tolerated", + yaml: "failover:\n overrides:\n \"gpt-4o\":\n mode: \"off\"\n", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + clearAllConfigEnvVars(t) + withTempDir(t, func(dir string) { + writeConfigYAML(t, dir, tt.yaml) + + result, err := Load() + if err != nil { + t.Fatalf("Load() failed: %v", err) + } + if got := len(result.RawProviders); got != tt.wantProviders { + t.Fatalf("len(RawProviders) = %d, want %d", got, tt.wantProviders) + } + }) + }) + } +} + +// A second YAML document is never applied, so accepting one silently drops every key +// it declares. Structural, therefore fatal regardless of CONFIG_STRICT. +func TestLoad_RejectsMultipleYAMLDocuments(t *testing.T) { + for _, strict := range []string{"true", "false"} { + t.Run("CONFIG_STRICT="+strict, func(t *testing.T) { + clearAllConfigEnvVars(t) + t.Setenv(envConfigStrict, strict) + + withTempDir(t, func(dir string) { + writeConfigYAML(t, dir, "providers:\n a:\n type: vllm\n base_url: \"http://a:8000/v1\"\n---\nproviders:\n b:\n type: vllm\n base_url: \"http://b:8000/v1\"\n") + + _, err := Load() + if err == nil { + t.Fatal("Load() succeeded, want a multi-document error") + } + if !strings.Contains(err.Error(), "only one YAML document is supported") { + t.Fatalf("Load() error = %q, want a multi-document error", err) + } + }) + }) + } +} + +// CONFIG_STRICT=false relaxes what the schema accepts, so an unknown key becomes a +// warning and the rest of the file still applies. +func TestLoad_ConfigStrictFalseDowngradesUnknownKeysToWarnings(t *testing.T) { + clearAllConfigEnvVars(t) + t.Setenv(envConfigStrict, "false") + + withTempDir(t, func(dir string) { + // The reporter's misindented file: four keys that should have been providers, + // plus a server section that must still take effect. + writeConfigYAML(t, dir, ` +server: + port: "9999" + +providers: +uranium-geryon-9b: + type: vllm + base_url: "http://uranium-geryon-9b:8000/v1" +`) + + result, err := Load() + if err != nil { + t.Fatalf("Load() failed under CONFIG_STRICT=false: %v", err) + } + if len(result.RawProviders) != 0 { + t.Fatalf("len(RawProviders) = %d, want 0 (the keys are not providers)", len(result.RawProviders)) + } + if result.Config.Server.Port != "9999" { + t.Fatalf("Server.Port = %q, want the known keys to still apply", result.Config.Server.Port) + } + }) +} + +// CONFIG_STRICT relaxes which keys are accepted, never whether a value makes sense. +// A malformed value is a broken config in any mode. +func TestLoad_ConfigStrictFalseStillRejectsMalformedValues(t *testing.T) { + clearAllConfigEnvVars(t) + t.Setenv(envConfigStrict, "false") + + withTempDir(t, func(dir string) { + writeConfigYAML(t, dir, "server:\n port: [9999, 8080]\n") + + _, err := Load() + if err == nil { + t.Fatal("Load() succeeded, want a type error even under CONFIG_STRICT=false") + } + if !strings.Contains(err.Error(), "cannot unmarshal") { + t.Fatalf("Load() error = %q, want a type error", err) + } + }) +} + +// A file that mixes an unknown key with a malformed value must still fail: the +// unknown key is downgraded, the type error is not. +func TestLoad_ConfigStrictFalseFailsWhenAnyErrorIsFatal(t *testing.T) { + clearAllConfigEnvVars(t) + t.Setenv(envConfigStrict, "false") + + withTempDir(t, func(dir string) { + writeConfigYAML(t, dir, "bogus_section:\n a: 1\nserver:\n port: [9999]\n") + + _, err := Load() + if err == nil { + t.Fatal("Load() succeeded, want the type error to remain fatal") + } + if strings.Contains(err.Error(), "bogus_section") { + t.Fatalf("Load() error = %q, want only the fatal type error reported", err) + } + }) +} + +func TestLoad_ConfigStrictRejectsNonBoolean(t *testing.T) { + clearAllConfigEnvVars(t) + t.Setenv(envConfigStrict, "yes-please") + + withTempDir(t, func(string) { + _, err := Load() + if err == nil { + t.Fatal("Load() succeeded, want an invalid CONFIG_STRICT error") + } + if !strings.Contains(err.Error(), "invalid CONFIG_STRICT") { + t.Fatalf("Load() error = %q, want it to name CONFIG_STRICT", err) + } + }) +} + +// TestApplyYAML_ExampleConfigParses keeps the shipped example honest: every key it +// documents must exist on Config, or operators who copy it cannot boot under strict +// parsing. It exercises the decode only — Load additionally resolves paths the +// example points at (failover rules), which do not exist relative to a temp dir. +func TestApplyYAML_ExampleConfigParses(t *testing.T) { + clearAllConfigEnvVars(t) + + example, err := os.ReadFile("config.example.yaml") + if err != nil { + t.Fatalf("Failed to read config.example.yaml: %v", err) + } + + withTempDir(t, func(dir string) { + writeConfigYAML(t, dir, string(example)) + + if _, err := applyYAML(buildDefaultConfig(), true); err != nil { + t.Fatalf("config.example.yaml does not parse: %v", err) + } + }) +} + +// A path that exists but cannot be read — most often a directory bind-mounted where +// a file was expected — must not be mistaken for a missing config file. +func TestApplyYAML_UnreadableConfigFileIsAnError(t *testing.T) { + clearAllConfigEnvVars(t) + + withTempDir(t, func(dir string) { + if err := os.Mkdir(filepath.Join(dir, "config.yaml"), 0755); err != nil { + t.Fatalf("Failed to create directory: %v", err) + } + + _, err := applyYAML(buildDefaultConfig(), true) + if err == nil { + t.Fatal("applyYAML() succeeded, want read error") + } + if !strings.Contains(err.Error(), "failed to read config.yaml") { + t.Fatalf("applyYAML() error = %q, want a read error naming the file", err) + } + }) +} + +func writeConfigYAML(t *testing.T, dir, contents string) { + t.Helper() + if err := os.WriteFile(filepath.Join(dir, "config.yaml"), []byte(contents), 0644); err != nil { + t.Fatalf("Failed to write config.yaml: %v", err) + } +} diff --git a/config/config_test.go b/config/config_test.go index 676f43cb..cc792792 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -37,6 +37,7 @@ func clearProviderEnvVars(t *testing.T) { func clearAllConfigEnvVars(t *testing.T) { t.Helper() for _, key := range []string{ + "CONFIG_STRICT", "PORT", "BASE_PATH", "GOMODEL_MASTER_KEY", "BODY_SIZE_LIMIT", "SWAGGER_ENABLED", "PPROF_ENABLED", "ENABLE_PASSTHROUGH_ROUTES", "ALLOW_PASSTHROUGH_V1_ALIAS", "USER_PATH_HEADER", "ENABLED_PASSTHROUGH_PROVIDERS", "GOMODEL_CACHE_DIR", "CACHE_REFRESH_INTERVAL", "REDIS_URL", "REDIS_KEY_MODELS", "REDIS_KEY_RESPONSES", "REDIS_TTL_MODELS", "REDIS_TTL_RESPONSES", diff --git a/config/failover.go b/config/failover.go index 8771f878..d1e6ff29 100644 --- a/config/failover.go +++ b/config/failover.go @@ -5,6 +5,7 @@ import ( "encoding/json" "fmt" "io" + "log/slog" "os" "strings" ) @@ -68,6 +69,11 @@ type FailoverConfig struct { // env. It accepts either a JSON string array or object with boolean values. DisabledModelsJSON string `yaml:"disabled_models_json" env:"FAILOVER_DISABLED_MODELS_JSON"` + // Overrides is a removed compatibility field. Per-model failover modes are gone; + // operators migrate to DisabledModels. It is still parsed — and ignored, with a + // warning — so an old config file keeps booting under strict YAML validation. + Overrides map[string]any `yaml:"overrides"` + // Manual holds the parsed manual failover lists loaded from ManualRulesPath. Manual map[string][]string `yaml:"-"` @@ -80,6 +86,11 @@ func loadFailoverConfig(cfg *FailoverConfig) error { return nil } + if len(cfg.Overrides) > 0 { + slog.Warn("failover.overrides was removed and is ignored; use failover.disabled_models instead") + cfg.Overrides = nil + } + cfg.DefaultMode = ResolveFailoverDefaultMode(cfg.DefaultMode) manual := make(map[string][]string) diff --git a/config/merge.go b/config/merge.go index a1a75c4f..1483f594 100644 --- a/config/merge.go +++ b/config/merge.go @@ -1,6 +1,50 @@ package config -import "strings" +import ( + "encoding/json" + "errors" + "fmt" + "io" + "log/slog" + "strings" +) + +// decodeIaCJSON decodes an infrastructure-as-code env var into target. The env layer +// declares the same structures as the YAML layer and overrides it entry by entry, so +// a typo must fail as loudly here as it does there — a silently ignored key would let +// a malformed env entry win over a correct YAML one. CONFIG_STRICT=false downgrades +// unknown keys to warnings, matching the YAML layer; malformed values stay fatal. +func decodeIaCJSON(source, raw string, target any, strict bool) error { + err := decodeStrictJSON(raw, target) + if err == nil || strict || !isUnknownFieldJSONError(err) { + return err + } + slog.Warn("unknown config key ignored; it has no effect", "source", source, "detail", err.Error()) + // The strict decode stopped at the unknown key, so re-decode leniently to fill + // target from the whole value. + return json.Unmarshal([]byte(raw), target) +} + +func decodeStrictJSON(raw string, target any) error { + decoder := json.NewDecoder(strings.NewReader(raw)) + decoder.DisallowUnknownFields() + if err := decoder.Decode(target); err != nil { + return err + } + // Decode stops after the first JSON value and leaves the rest unread, so + // `[{...}] junk` would pass. json.Unmarshal rejects trailing data and so must we: + // silently ignoring half an env var is the bug this whole path exists to prevent. + if _, err := decoder.Token(); !errors.Is(err, io.EOF) { + return fmt.Errorf("unexpected data after the JSON value") + } + return nil +} + +// isUnknownFieldJSONError reports whether err is encoding/json's unknown-key error, +// the only decode error CONFIG_STRICT=false is allowed to downgrade. +func isUnknownFieldJSONError(err error) bool { + return strings.HasPrefix(err.Error(), "json: unknown field ") +} // mergeByKey overlays override entries onto base, replacing matching base // entries in place and appending new entries. diff --git a/config/ratelimit.go b/config/ratelimit.go index 9a8472cc..67af07a9 100644 --- a/config/ratelimit.go +++ b/config/ratelimit.go @@ -1,7 +1,6 @@ package config import ( - "encoding/json" "fmt" "strconv" "strings" @@ -69,18 +68,21 @@ type RateLimitRuleConfig struct { MaxTokens *int64 `yaml:"max_tokens" json:"max_tokens"` } -func applyRateLimitEnv(cfg *Config) error { +func applyRateLimitEnv(cfg *Config, strict bool) error { if cfg == nil { return nil } if !cfg.RateLimits.Enabled { return nil } + parseLimits := func(raw string) ([]RateLimitRuleConfig, error) { + return parseRateLimitEnvLimits(raw, strict) + } entries, err := applyUserPathLimitEnv( cfg.RateLimits.UserPaths, "SET_RATE_LIMIT_", func(entry RateLimitUserPathConfig) string { return entry.Path }, - parseRateLimitEnvLimits, + parseLimits, func(path string, limits []RateLimitRuleConfig) RateLimitUserPathConfig { return RateLimitUserPathConfig{Path: path, Limits: limits} }, @@ -100,7 +102,7 @@ func applyRateLimitEnv(cfg *Config) error { rateLimitProviderNameFromEnvSuffix, normalizeRateLimitProviderName, func(entry RateLimitProviderConfig) string { return entry.Name }, - parseRateLimitEnvLimits, + parseLimits, func(name string, limits []RateLimitRuleConfig) RateLimitProviderConfig { return RateLimitProviderConfig{Name: name, Limits: limits} }, @@ -129,14 +131,14 @@ func normalizeRateLimitProviderName(raw string) (string, error) { // parseRateLimitEnvLimits parses either a JSON array of rule objects or the // compact "rpm=100,tpm=50000,rpd=1000,concurrent=10" syntax. -func parseRateLimitEnvLimits(raw string) ([]RateLimitRuleConfig, error) { +func parseRateLimitEnvLimits(raw string, strict bool) ([]RateLimitRuleConfig, error) { raw = strings.TrimSpace(raw) if raw == "" { return nil, nil } if strings.HasPrefix(raw, "[") { var limits []RateLimitRuleConfig - if err := json.Unmarshal([]byte(raw), &limits); err != nil { + if err := decodeIaCJSON("SET_RATE_LIMIT_*", raw, &limits, strict); err != nil { return nil, err } return limits, nil diff --git a/config/ratelimit_test.go b/config/ratelimit_test.go index d3dde4c9..ce7ca55b 100644 --- a/config/ratelimit_test.go +++ b/config/ratelimit_test.go @@ -406,3 +406,13 @@ func TestRateLimitsEnabledByDefaultAndTogglable(t *testing.T) { } }) } + +func TestParseRateLimitEnvLimits_RejectsUnknownField(t *testing.T) { + _, err := parseRateLimitEnvLimits(`[{"period":"minute","max_requsts":100}]`, true) + if err == nil { + t.Fatal("parseRateLimitEnvLimits() error = nil, want unknown-field error") + } + if !strings.Contains(err.Error(), "max_requsts") { + t.Fatalf("parseRateLimitEnvLimits() error = %q, want it to name the unknown field", err) + } +} diff --git a/config/virtualmodels.go b/config/virtualmodels.go index 35ae9dad..347427cb 100644 --- a/config/virtualmodels.go +++ b/config/virtualmodels.go @@ -1,7 +1,6 @@ package config import ( - "encoding/json" "fmt" "os" "strings" @@ -53,13 +52,13 @@ const envVirtualModels = "VIRTUAL_MODELS" // virtual model definitions — and merges it over the YAML-declared list. Env // entries override YAML entries with the same source, consistent with the rest of // the config pipeline where env always wins. -func applyVirtualModelsEnv(cfg *Config) error { +func applyVirtualModelsEnv(cfg *Config, strict bool) error { raw := strings.TrimSpace(os.Getenv(envVirtualModels)) if raw == "" { return nil } var fromEnv []VirtualModelConfig - if err := json.Unmarshal([]byte(raw), &fromEnv); err != nil { + if err := decodeIaCJSON(envVirtualModels, raw, &fromEnv, strict); err != nil { return fmt.Errorf("invalid %s: %w", envVirtualModels, err) } cfg.VirtualModels = mergeByKey(cfg.VirtualModels, fromEnv, func(model VirtualModelConfig) string { diff --git a/config/virtualmodels_test.go b/config/virtualmodels_test.go index 94da92ed..4b89ca9e 100644 --- a/config/virtualmodels_test.go +++ b/config/virtualmodels_test.go @@ -1,6 +1,10 @@ package config -import "testing" +import ( + "fmt" + "strings" + "testing" +) func TestApplyVirtualModelsEnv_ParsesAndMerges(t *testing.T) { cfg := &Config{VirtualModels: []VirtualModelConfig{ @@ -12,7 +16,7 @@ func TestApplyVirtualModelsEnv_ParsesAndMerges(t *testing.T) { {"source":"new","target":"anthropic/claude"} ]`) - if err := applyVirtualModelsEnv(cfg); err != nil { + if err := applyVirtualModelsEnv(cfg, true); err != nil { t.Fatalf("applyVirtualModelsEnv() error = %v", err) } if len(cfg.VirtualModels) != 3 { @@ -32,15 +36,81 @@ func TestApplyVirtualModelsEnv_ParsesAndMerges(t *testing.T) { func TestApplyVirtualModelsEnv_Invalid(t *testing.T) { cfg := &Config{} t.Setenv(envVirtualModels, `{not valid json`) - if err := applyVirtualModelsEnv(cfg); err == nil { + if err := applyVirtualModelsEnv(cfg, true); err == nil { t.Fatalf("applyVirtualModelsEnv() error = nil, want parse error") } } +// The env layer overrides YAML entry by entry, so a typo must fail loudly rather +// than let a malformed env entry silently win over a correct YAML one. +func TestApplyVirtualModelsEnv_RejectsUnknownField(t *testing.T) { + cfg := &Config{} + t.Setenv(envVirtualModels, `[{"source":"smart","targts":[{"model":"openai/gpt-4o"}]}]`) + + err := applyVirtualModelsEnv(cfg, true) + if err == nil { + t.Fatal("applyVirtualModelsEnv() error = nil, want unknown-field error") + } + if !strings.Contains(err.Error(), "targts") { + t.Fatalf("applyVirtualModelsEnv() error = %q, want it to name the unknown field", err) + } +} + +// json.Decoder stops after the first value and leaves the rest unread, so trailing +// data must be rejected explicitly — silently applying half an env var is the failure +// this path exists to prevent. Structural, therefore fatal in both modes. +func TestApplyVirtualModelsEnv_RejectsTrailingData(t *testing.T) { + trailing := map[string]string{ + "garbage suffix": `[{"source":"smart","target":"openai/gpt-4o"}] and then some junk`, + "second JSON value": `[{"source":"smart","target":"openai/gpt-4o"}] {"targts":[]}`, + "second JSON on a line": "[{\"source\":\"smart\",\"target\":\"openai/gpt-4o\"}]\n{\"x\":1}", + } + for name, raw := range trailing { + for _, strict := range []bool{true, false} { + t.Run(fmt.Sprintf("%s/strict=%v", name, strict), func(t *testing.T) { + cfg := &Config{} + t.Setenv(envVirtualModels, raw) + + err := applyVirtualModelsEnv(cfg, strict) + if err == nil { + t.Fatal("applyVirtualModelsEnv() error = nil, want trailing-data error") + } + if !strings.Contains(err.Error(), "unexpected data after the JSON value") { + t.Fatalf("applyVirtualModelsEnv() error = %q, want a trailing-data error", err) + } + }) + } + } +} + +// CONFIG_STRICT=false applies to the env layer too: the unknown key is warned about +// and the rest of the entry still loads. +func TestApplyVirtualModelsEnv_LaxIgnoresUnknownField(t *testing.T) { + cfg := &Config{} + t.Setenv(envVirtualModels, `[{"source":"smart","targts":[],"target":"openai/gpt-4o"}]`) + + if err := applyVirtualModelsEnv(cfg, false); err != nil { + t.Fatalf("applyVirtualModelsEnv() error = %v, want the unknown key ignored", err) + } + if len(cfg.VirtualModels) != 1 || cfg.VirtualModels[0].Target != "openai/gpt-4o" { + t.Fatalf("lax decode dropped the known fields: %#v", cfg.VirtualModels) + } +} + +// Even lax, a value of the wrong type is fatal. +func TestApplyVirtualModelsEnv_LaxStillRejectsMalformedValues(t *testing.T) { + cfg := &Config{} + t.Setenv(envVirtualModels, `[{"source":123}]`) + + if err := applyVirtualModelsEnv(cfg, false); err == nil { + t.Fatal("applyVirtualModelsEnv() error = nil, want a type error") + } +} + func TestApplyVirtualModelsEnv_Unset(t *testing.T) { cfg := &Config{VirtualModels: []VirtualModelConfig{{Source: "smart", Target: "openai/gpt-4o"}}} t.Setenv(envVirtualModels, "") - if err := applyVirtualModelsEnv(cfg); err != nil { + if err := applyVirtualModelsEnv(cfg, true); err != nil { t.Fatalf("applyVirtualModelsEnv() error = %v", err) } if len(cfg.VirtualModels) != 1 { diff --git a/docs/advanced/config-yaml.mdx b/docs/advanced/config-yaml.mdx index 4384d617..75837895 100644 --- a/docs/advanced/config-yaml.mdx +++ b/docs/advanced/config-yaml.mdx @@ -50,6 +50,51 @@ Effective precedence is: `.env` is not a separate priority layer. It is just a convenient way to load environment variables before startup. +Each layer is applied on top of the previous one, so a provider declared in +`config.yaml` and overlaid by env vars still counts as coming from the file. + +## Startup Validation + +Declarative config is parsed strictly: an unknown key is a startup error, not a +silently ignored one. This catches typos (`prot:` instead of `port:`) and, more +importantly, misindented sections — see the gotcha below. + +This applies to `config.yaml` and to the env vars that declare the same +structures as JSON: `VIRTUAL_MODELS`, `SET_RATE_LIMIT_*`, and `SET_BUDGET_*`. +Because env entries override YAML entries, a typo in one of them would otherwise +silently win over a correct YAML entry. + +The default is strict because a dropped `providers`, `rate_limits`, `budgets`, +or `guardrails` entry silently changes routing, cost, or security — a cost cap +that never applies looks exactly like a cost cap that does. + +Set `CONFIG_STRICT=false` to downgrade unknown keys to warnings and boot without +them. This is for rolling a binary back under a newer config file, not for +everyday use: + +```text +WARN unknown config key ignored; it has no effect path=config.yaml line=23 field=uranium-geryon-9b +INFO providers resolved total=1 from_config_file=0 from_env=1 +``` + +`CONFIG_STRICT` relaxes which *keys* are accepted, never whether a *value* makes +sense. A malformed value (`port: [9999, 8080]`) aborts startup in either mode. +Entity-level validation also still applies: a virtual model whose target names a +provider that does not exist fails startup even when the key that would have +declared that provider was ignored with a warning. + +To confirm which layer supplied your providers, GoModel logs one line at boot: + +```json +{"msg":"providers resolved","total":5,"from_config_file":4,"from_env":1, + "config_file_providers":["plutonium-qwen-dflash","uranium-gemma-embedding"], + "env_providers":["opencode-go"]} +``` + +`from_config_file: 0` while a file is loaded means the file contributed no +providers. GoModel also logs `config file loaded` with the resolved path, or +`no config file found` with the paths it searched. + ## Current Schema The current source of truth lives in the main codebase: @@ -94,6 +139,10 @@ services: wins. +If the mounted path exists but cannot be read — most often a directory +bind-mounted where a file was expected — startup fails rather than silently +falling back to defaults. + ## Gotchas ### Unresolved `${VAR}` placeholders drop the provider @@ -105,6 +154,31 @@ applies if `${VAR}` appears in the middle of the value, such as `prefix-${OPENAI_API_KEY}`. Always verify your env vars are exported before starting the process, or supply a default: `${OPENAI_API_KEY:-}`. +### A misindented `providers:` section is rejected + +YAML reads this as an empty `providers:` section plus two unrelated top-level +keys, not as two providers: + +```yaml +providers: +uranium-geryon-9b: # not indented under providers: + type: vllm + base_url: "http://uranium-geryon-9b:8000/v1" +uranium-gemma-embedding: # likewise + type: vllm + base_url: "http://uranium-gemma-embedding:8000/v1" +``` + +Strict parsing rejects it at startup, naming every offending line: + +```text +failed to parse config.yaml: line 2: field uranium-geryon-9b not found; line 5: field uranium-gemma-embedding not found +``` + +Indent provider entries two spaces under `providers:`. Before strict parsing, +this booted with zero YAML providers and no error. With `CONFIG_STRICT=false` it +boots with zero YAML providers and one warning per unindented key. + ### Per-provider resilience can only come from YAML The env-var override walk skips `map` fields. `RETRY_MAX_RETRIES` changes the diff --git a/go.mod b/go.mod index d609f92d..f1648ec8 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module gomodel -go 1.26.4 +go 1.26.5 require ( github.com/andybalholm/brotli v1.2.2 diff --git a/internal/providers/config.go b/internal/providers/config.go index 4df6f4b5..df3ed77a 100644 --- a/internal/providers/config.go +++ b/internal/providers/config.go @@ -530,6 +530,24 @@ func isUnresolvedEnvPlaceholder(value string) bool { return inner != "" && !strings.ContainsAny(inner, "{}") } +// providerOrigins splits the resolved provider names by where they were declared: +// the config file, or environment-variable discovery. A provider named in the +// config file counts as fromFile even when env vars overlay its fields. Operators +// need the split to notice a config file that contributed nothing — a misindented +// providers: section reads as zero fromFile providers. +func providerOrigins(declared map[string]config.RawProviderConfig, resolved map[string]ProviderConfig) (fromFile, fromEnv []string) { + for name := range resolved { + if _, ok := declared[name]; ok { + fromFile = append(fromFile, name) + } else { + fromEnv = append(fromEnv, name) + } + } + sort.Strings(fromFile) + sort.Strings(fromEnv) + return fromFile, fromEnv +} + // skippedProviderNames lists the YAML-declared providers that did not survive // credential resolution, so operators can see why a configured provider is // absent instead of it disappearing silently. diff --git a/internal/providers/config_test.go b/internal/providers/config_test.go index 459fbb7f..6f12c7a1 100644 --- a/internal/providers/config_test.go +++ b/internal/providers/config_test.go @@ -1,6 +1,7 @@ package providers import ( + "slices" "testing" "time" @@ -361,6 +362,40 @@ func TestSkippedProviderNames_ListsDeclaredButUnresolved(t *testing.T) { } } +func TestProviderOrigins_SplitsConfigFileFromEnv(t *testing.T) { + // openai is declared in the config file and overlaid by env vars; it still + // counts as coming from the file. groq exists only because of env discovery. + declared := map[string]config.RawProviderConfig{ + "openai": {Type: "openai"}, + "vllm-b": {Type: "vllm", BaseURL: "http://b:8000/v1"}, + } + resolved := map[string]ProviderConfig{ + "openai": {Type: "openai"}, + "vllm-b": {Type: "vllm"}, + "groq": {Type: "groq"}, + } + + fromFile, fromEnv := providerOrigins(declared, resolved) + if want := []string{"openai", "vllm-b"}; !slices.Equal(fromFile, want) { + t.Fatalf("fromFile = %v, want %v", fromFile, want) + } + if want := []string{"groq"}; !slices.Equal(fromEnv, want) { + t.Fatalf("fromEnv = %v, want %v", fromEnv, want) + } +} + +// A misindented providers: section yields no config-file providers, which is the +// signal an operator needs to see at boot. +func TestProviderOrigins_NoDeclaredProviders(t *testing.T) { + fromFile, fromEnv := providerOrigins(nil, map[string]ProviderConfig{"openai": {Type: "openai"}}) + if len(fromFile) != 0 { + t.Fatalf("fromFile = %v, want empty", fromFile) + } + if want := []string{"openai"}; !slices.Equal(fromEnv, want) { + t.Fatalf("fromEnv = %v, want %v", fromEnv, want) + } +} + func TestFilterEmptyProviders_EmptyMap(t *testing.T) { got := filterEmptyProviders(map[string]config.RawProviderConfig{}, testDiscoveryConfigs) if len(got) != 0 { diff --git a/internal/providers/init.go b/internal/providers/init.go index 77ede269..3cc668cb 100644 --- a/internal/providers/init.go +++ b/internal/providers/init.go @@ -81,6 +81,13 @@ func Init(ctx context.Context, result *config.LoadResult, factory *ProviderFacto } providerMap, credentialResolved := resolveProviders(result.RawProviders, result.Config.Resilience, factory.discoveryConfigsSnapshot()) + fromFile, fromEnv := providerOrigins(result.RawProviders, providerMap) + slog.Info("providers resolved", + "total", len(providerMap), + "from_config_file", len(fromFile), + "from_env", len(fromEnv), + "config_file_providers", fromFile, + "env_providers", fromEnv) if skipped := skippedProviderNames(result.RawProviders, credentialResolved); len(skipped) > 0 { slog.Info("configured providers skipped: credentials or base_url did not resolve", "providers", skipped)