config: add environment variable macros#466
Conversation
Add support for ${env.VAR_NAME} syntax to pull values from system
environment variables during config loading.
- env macros processed before regular macros (allows macros to reference env vars)
- works in cmd, cmdStop, proxy, checkEndpoint, filters.stripParams, metadata
- returns error if env var is not set
- add comprehensive tests
fixes #462
WalkthroughAdds ${env.VAR} environment-variable macro substitution to the config loader. Env macros are expanded (and validated) in global macros, model-level macros, model fields (Cmd, CmdStop, Proxy, CheckEndpoint, Filters.StripParams), and metadata before existing macro substitution passes. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@proxy/config/config.go`:
- Around line 284-287: The Filters.SetParams map is not being run through
env-macro substitution like Metadata is; update the model config loading to call
substituteEnvMacrosInValue (or substituteEnvMacros) on
modelConfig.Filters.SetParams and handle/return any error the same way as for
Metadata (wrap with fmt.Errorf including modelId), so SetParams supports
recursive env variable macros just like Metadata (or, if intentional, add a
comment documenting the exclusion).
| modelConfig.Filters.StripParams, err = substituteEnvMacros(modelConfig.Filters.StripParams) | ||
| if err != nil { | ||
| return Config{}, fmt.Errorf("model %s filters.stripParams: %w", modelId, err) | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for setParams usage across the codebase
rg -i "setparams" --type yaml --type go -C 2Repository: mostlygeek/llama-swap
Length of output: 9852
🏁 Script executed:
#!/bin/bash
# Also check the Filters struct definition to understand both fields
rg -n "type Filters struct" -A 10 --type goRepository: mostlygeek/llama-swap
Length of output: 792
🏁 Script executed:
#!/bin/bash
# Look for any references to SetParams in tests or config examples
rg -n "SetParams" --type go --type yaml --type json -C 3Repository: mostlygeek/llama-swap
Length of output: 5780
🏁 Script executed:
#!/bin/bash
# Search for the env macro substitution implementation pattern around lines 267-307
sed -n '260,310p' proxy/config/config.goRepository: mostlygeek/llama-swap
Length of output: 1939
🏁 Script executed:
#!/bin/bash
# Look for any comments or TODOs explaining SetParams handling
rg -n "SetParams" proxy/config/config.go -B 2 -A 2Repository: mostlygeek/llama-swap
Length of output: 47
SetParams should support environment variable macros for consistency with Metadata.
Both Filters.SetParams and model-level Metadata are map[string]any structures that can contain sensitive string values. The config loading code applies recursive env macro substitution to Metadata (lines 293-299) but skips SetParams entirely, creating an inconsistency. Since the infrastructure (substituteEnvMacrosInValue) already handles recursive substitution of map[string]any, consider extending it to SetParams to allow users to inject environment variables into parameter values (e.g., API keys, provider-specific settings). If intentionally excluded, document this limitation.
🤖 Prompt for AI Agents
In `@proxy/config/config.go` around lines 284 - 287, The Filters.SetParams map is
not being run through env-macro substitution like Metadata is; update the model
config loading to call substituteEnvMacrosInValue (or substituteEnvMacros) on
modelConfig.Filters.SetParams and handle/return any error the same way as for
Metadata (wrap with fmt.Errorf including modelId), so SetParams supports
recursive env variable macros just like Metadata (or, if intentional, add a
comment documenting the exclusion).
Summary
${env.VAR_NAME}syntax to pull values from environment variablesfixes #462
Summary by CodeRabbit
New Features
${env.VAR}across configuration, including model fields and metadata.${env.VAR}usage.Tests
✏️ Tip: You can customize this high-level summary in your review settings.