chore: clean up env var usage in aibridge (#24783)

> AI tools where used when creating this PR

This PR removes environment variable parsing from `/aibridge` directory.

Added env variables/flags for dump dir as coder options.
Only added to new indexed provider options
(`CODER_AIBRIDGE_PROVIDER_<N>_*`) not to deprecated legacy env variables
(`CODER_AIBRIDGE_ANTHROPIC_*` and `CODER_AIBRIDGE_OPENAI_KEY_*`).

Reverted adding `MaxRetries` option as it will be removed soon due to
key failover work:
https://github.com/coder/coder/pull/24783#discussion_r3155544808
This commit is contained in:
Paweł Banaszewski
2026-04-29 18:28:37 +02:00
committed by GitHub
parent 6ea9c61da0
commit a24dc19d49
19 changed files with 59 additions and 79 deletions
-12
View File
@@ -21,10 +21,6 @@ type Anthropic struct {
// with a access token. When set, the access token is used for upstream
// LLM requests instead of the API key.
BYOKBearerToken string
// MaxRetries controls the number of automatic retries the SDK will perform
// on transient errors. If nil, the SDK default (2) is used.
// Set to 0 to disable retries entirely.
MaxRetries *int
}
type AWSBedrock struct {
@@ -46,10 +42,6 @@ type OpenAI struct {
CircuitBreaker *CircuitBreaker
SendActorHeaders bool
ExtraHeaders map[string]string
// MaxRetries controls the number of automatic retries the SDK will perform
// on transient errors. If nil, the SDK default (2) is used.
// Set to 0 to disable retries entirely.
MaxRetries *int
}
type Copilot struct {
@@ -58,10 +50,6 @@ type Copilot struct {
BaseURL string
APIDumpDir string
CircuitBreaker *CircuitBreaker
// MaxRetries controls the number of automatic retries the SDK will perform
// on transient errors. If nil, the SDK default (2) is used.
// Set to 0 to disable retries entirely.
MaxRetries *int
}
// CircuitBreaker holds configuration for circuit breakers.
@@ -46,9 +46,6 @@ type interceptionBase struct {
func (i *interceptionBase) newCompletionsService() openai.ChatCompletionService {
opts := []option.RequestOption{option.WithAPIKey(i.cfg.Key), option.WithBaseURL(i.cfg.BaseURL)}
if i.cfg.MaxRetries != nil {
opts = append(opts, option.WithMaxRetries(*i.cfg.MaxRetries))
}
// Add extra headers if configured.
// Some providers require additional headers that are not added by the SDK.
-3
View File
@@ -217,9 +217,6 @@ func (i *interceptionBase) newMessagesService(ctx context.Context, opts ...optio
opts = append(opts, option.WithAPIKey(i.cfg.Key))
}
opts = append(opts, option.WithBaseURL(i.cfg.BaseURL))
if i.cfg.MaxRetries != nil {
opts = append(opts, option.WithMaxRetries(*i.cfg.MaxRetries))
}
// Add extra headers if configured.
// Some providers require additional headers that are not added by the SDK.
-3
View File
@@ -55,9 +55,6 @@ type responsesInterceptionBase struct {
func (i *responsesInterceptionBase) newResponsesService() responses.ResponseService {
opts := []option.RequestOption{option.WithBaseURL(i.cfg.BaseURL), option.WithAPIKey(i.cfg.Key)}
if i.cfg.MaxRetries != nil {
opts = append(opts, option.WithMaxRetries(*i.cfg.MaxRetries))
}
// Add extra headers if configured.
// Some providers require additional headers that are not added by the SDK.
@@ -639,10 +639,7 @@ func TestClientAndConnectionError(t *testing.T) {
t.Cleanup(cancel)
// tc.addr may be an intentionally invalid URL; use withCustomProvider.
// MaxRetries is set to 0 to disable SDK retries and speed up the test.
cfg := openAICfg(tc.addr, apiKey)
maxRetries := 0
cfg.MaxRetries = &maxRetries
bridgeServer := newBridgeTestServer(ctx, t, tc.addr, withCustomProvider(provider.NewOpenAI(cfg)))
reqBytes := responsesRequestBytes(t, tc.streaming)
@@ -719,10 +716,7 @@ func TestUpstreamError(t *testing.T) {
}))
t.Cleanup(upstream.Close)
// MaxRetries is set to 0 to disable SDK retries and speed up the test.
cfg := openAICfg(upstream.URL, apiKey)
maxRetries := 0
cfg.MaxRetries = &maxRetries
bridgeServer := newBridgeTestServer(ctx, t, upstream.URL, withCustomProvider(provider.NewOpenAI(cfg)))
reqBytes := responsesRequestBytes(t, tc.streaming)
-15
View File
@@ -4,8 +4,6 @@ import (
"fmt"
"io"
"net/http"
"os"
"strconv"
"strings"
"github.com/google/uuid"
@@ -57,19 +55,6 @@ func NewAnthropic(cfg config.Anthropic, bedrockCfg *config.AWSBedrock) *Anthropi
if cfg.BaseURL == "" {
cfg.BaseURL = "https://api.anthropic.com/"
}
if cfg.Key == "" {
cfg.Key = os.Getenv("ANTHROPIC_API_KEY")
}
if cfg.APIDumpDir == "" {
cfg.APIDumpDir = os.Getenv("BRIDGE_DUMP_DIR")
}
if cfg.MaxRetries == nil {
if v := os.Getenv("ANTHROPIC_MAX_RETRIES"); v != "" {
if n, err := strconv.Atoi(v); err == nil {
cfg.MaxRetries = &n
}
}
}
if cfg.CircuitBreaker != nil {
cfg.CircuitBreaker.IsFailure = anthropicIsFailure
cfg.CircuitBreaker.OpenErrorResponse = anthropicOpenErrorResponse
-13
View File
@@ -5,8 +5,6 @@ import (
"fmt"
"io"
"net/http"
"os"
"strconv"
"strings"
"github.com/google/uuid"
@@ -61,16 +59,6 @@ func NewCopilot(cfg config.Copilot) *Copilot {
if cfg.BaseURL == "" {
cfg.BaseURL = copilotBaseURL
}
if cfg.APIDumpDir == "" {
cfg.APIDumpDir = os.Getenv("BRIDGE_DUMP_DIR")
}
if cfg.MaxRetries == nil {
if v := os.Getenv("COPILOT_MAX_RETRIES"); v != "" {
if n, err := strconv.Atoi(v); err == nil {
cfg.MaxRetries = &n
}
}
}
if cfg.CircuitBreaker != nil {
cfg.CircuitBreaker.OpenErrorResponse = copilotOpenErrorResponse
}
@@ -153,7 +141,6 @@ func (p *Copilot) CreateInterceptor(_ http.ResponseWriter, r *http.Request, trac
APIDumpDir: p.cfg.APIDumpDir,
CircuitBreaker: p.cfg.CircuitBreaker,
ExtraHeaders: extractCopilotHeaders(r),
MaxRetries: p.cfg.MaxRetries,
}
cred := intercept.NewCredentialInfo(intercept.CredentialKindBYOK, key)
-15
View File
@@ -5,8 +5,6 @@ import (
"fmt"
"io"
"net/http"
"os"
"strconv"
"strings"
"github.com/google/uuid"
@@ -46,19 +44,6 @@ func NewOpenAI(cfg config.OpenAI) *OpenAI {
if cfg.BaseURL == "" {
cfg.BaseURL = "https://api.openai.com/v1/"
}
if cfg.Key == "" {
cfg.Key = os.Getenv("OPENAI_API_KEY")
}
if cfg.APIDumpDir == "" {
cfg.APIDumpDir = os.Getenv("BRIDGE_DUMP_DIR")
}
if cfg.MaxRetries == nil {
if v := os.Getenv("OPENAI_MAX_RETRIES"); v != "" {
if n, err := strconv.Atoi(v); err == nil {
cfg.MaxRetries = &n
}
}
}
if cfg.CircuitBreaker != nil {
cfg.CircuitBreaker.OpenErrorResponse = openAIOpenErrorResponse
}
+2
View File
@@ -2981,6 +2981,8 @@ func ReadAIBridgeProvidersFromEnv(logger slog.Logger, environ []string) ([]coder
provider.Key = v.Value
case "BASE_URL":
provider.BaseURL = v.Value
case "DUMP_DIR":
provider.DumpDir = v.Value
case "BEDROCK_BASE_URL":
provider.BedrockBaseURL = v.Value
case "BEDROCK_REGION":
+2
View File
@@ -34,6 +34,7 @@ func TestReadAIBridgeProvidersFromEnv(t *testing.T) {
"CODER_AIBRIDGE_PROVIDER_0_NAME=anthropic-zdr",
"CODER_AIBRIDGE_PROVIDER_0_KEY=sk-ant-xxx",
"CODER_AIBRIDGE_PROVIDER_0_BASE_URL=https://api.anthropic.com/",
"CODER_AIBRIDGE_PROVIDER_0_DUMP_DIR=/tmp/aibridge-dump",
},
expected: []codersdk.AIBridgeProviderConfig{
{
@@ -41,6 +42,7 @@ func TestReadAIBridgeProvidersFromEnv(t *testing.T) {
Name: "anthropic-zdr",
Key: "sk-ant-xxx",
BaseURL: "https://api.anthropic.com/",
DumpDir: "/tmp/aibridge-dump",
},
},
},
+4
View File
@@ -13312,6 +13312,10 @@ const docTemplate = `{
"bedrock_small_fast_model": {
"type": "string"
},
"dump_dir": {
"description": "DumpDir is the directory path for dumping API requests and responses.",
"type": "string"
},
"name": {
"description": "Name is the unique instance identifier used for routing.\nDefaults to Type if not provided.",
"type": "string"
+4
View File
@@ -11860,6 +11860,10 @@
"bedrock_small_fast_model": {
"type": "string"
},
"dump_dir": {
"description": "DumpDir is the directory path for dumping API requests and responses.",
"type": "string"
},
"name": {
"description": "Name is the unique instance identifier used for routing.\nDefaults to Type if not provided.",
"type": "string"
+2
View File
@@ -4114,6 +4114,8 @@ type AIBridgeProviderConfig struct {
Key string `json:"-"`
// BaseURL is the base URL of the upstream provider API.
BaseURL string `json:"base_url"`
// DumpDir is the directory path for dumping API requests and responses.
DumpDir string `json:"dump_dir,omitempty"`
// Bedrock fields (only applicable when Type == "anthropic").
BedrockBaseURL string `json:"-"`
+14 -6
View File
@@ -213,12 +213,20 @@ requests to `/api/v2/aibridge/<NAME>/` to target a specific instance:
**Supported keys per provider:**
| Key | Required | Description |
|------------|----------|------------------------------------------------------|
| `TYPE` | Yes | Provider type: `openai`, `anthropic`, or `copilot` |
| `NAME` | No | Unique instance name for routing. Defaults to `TYPE` |
| `KEY` | No | API key for upstream authentication (alias: `KEYS`) |
| `BASE_URL` | No | Base URL of the upstream API |
| Key | Required | Description |
|------------|----------|-------------------------------------------------------|
| `TYPE` | Yes | Provider type: `openai`, `anthropic`, or `copilot` |
| `NAME` | No | Unique instance name for routing. Defaults to `TYPE` |
| `KEY` | No | API key for upstream authentication (alias: `KEYS`) |
| `BASE_URL` | No | Base URL of the upstream API |
| `DUMP_DIR` | No | Directory for provider API request and response dumps |
> [!WARNING]
> `DUMP_DIR` is not intended for regular use. Setting this option
> results in a high number of writes. Dump files contain raw request and
> response data, which may include proprietary or sensitive information
> (prompts, completions, tool inputs). Enable only briefly for diagnostic
> purposes and protect the target directory.
For `anthropic` providers using AWS Bedrock, the following keys are also
available: `BEDROCK_BASE_URL`, `BEDROCK_REGION`,
+1
View File
@@ -210,6 +210,7 @@ curl -X GET http://coder-server:8080/api/v2/deployment/config \
"bedrock_model": "string",
"bedrock_region": "string",
"bedrock_small_fast_model": "string",
"dump_dir": "string",
"name": "string",
"type": "string"
}
+6
View File
@@ -468,6 +468,7 @@
"bedrock_model": "string",
"bedrock_region": "string",
"bedrock_small_fast_model": "string",
"dump_dir": "string",
"name": "string",
"type": "string"
}
@@ -760,6 +761,7 @@
"bedrock_model": "string",
"bedrock_region": "string",
"bedrock_small_fast_model": "string",
"dump_dir": "string",
"name": "string",
"type": "string"
}
@@ -773,6 +775,7 @@
| `bedrock_model` | string | false | | |
| `bedrock_region` | string | false | | |
| `bedrock_small_fast_model` | string | false | | |
| `dump_dir` | string | false | | Dump dir is the directory path for dumping API requests and responses. |
| `name` | string | false | | Name is the unique instance identifier used for routing. Defaults to Type if not provided. |
| `type` | string | false | | Type is the provider type: "openai", "anthropic", or "copilot". |
@@ -1287,6 +1290,7 @@
"bedrock_model": "string",
"bedrock_region": "string",
"bedrock_small_fast_model": "string",
"dump_dir": "string",
"name": "string",
"type": "string"
}
@@ -3326,6 +3330,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o
"bedrock_model": "string",
"bedrock_region": "string",
"bedrock_small_fast_model": "string",
"dump_dir": "string",
"name": "string",
"type": "string"
}
@@ -3916,6 +3921,7 @@ CreateWorkspaceRequest provides options for creating a new workspace. Only one o
"bedrock_model": "string",
"bedrock_region": "string",
"bedrock_small_fast_model": "string",
"dump_dir": "string",
"name": "string",
"type": "string"
}
+3
View File
@@ -116,6 +116,7 @@ func buildProviders(cfg codersdk.AIBridgeConfig) ([]aibridge.Provider, error) {
Name: name,
BaseURL: p.BaseURL,
Key: p.Key,
APIDumpDir: p.DumpDir,
CircuitBreaker: cbConfig,
SendActorHeaders: cfg.SendActorHeaders.Value(),
}))
@@ -124,6 +125,7 @@ func buildProviders(cfg codersdk.AIBridgeConfig) ([]aibridge.Provider, error) {
Name: name,
BaseURL: p.BaseURL,
Key: p.Key,
APIDumpDir: p.DumpDir,
CircuitBreaker: cbConfig,
SendActorHeaders: cfg.SendActorHeaders.Value(),
}, bedrockConfigFromProvider(p)))
@@ -131,6 +133,7 @@ func buildProviders(cfg codersdk.AIBridgeConfig) ([]aibridge.Provider, error) {
providers = append(providers, aibridge.NewCopilotProvider(aibridge.CopilotConfig{
Name: name,
BaseURL: p.BaseURL,
APIDumpDir: p.DumpDir,
CircuitBreaker: cbConfig,
}))
default:
+17 -3
View File
@@ -43,8 +43,19 @@ func TestBuildProviders(t *testing.T) {
t.Parallel()
cfg := codersdk.AIBridgeConfig{
Providers: []codersdk.AIBridgeProviderConfig{
{Type: aibridge.ProviderAnthropic, Name: "anthropic-zdr", Key: "sk-zdr"},
{Type: aibridge.ProviderOpenAI, Name: "openai-azure", Key: "sk-azure", BaseURL: "https://azure.openai.com"},
{
Type: aibridge.ProviderAnthropic,
Name: "anthropic-zdr",
Key: "sk-zdr",
DumpDir: "/tmp/anthropic-dump",
},
{
Type: aibridge.ProviderOpenAI,
Name: "openai-azure",
Key: "sk-azure",
BaseURL: "https://azure.openai.com",
DumpDir: "/tmp/openai-dump",
},
},
}
@@ -53,6 +64,8 @@ func TestBuildProviders(t *testing.T) {
names := providerNames(providers)
assert.Equal(t, []string{"anthropic-zdr", "openai-azure"}, names)
assert.Equal(t, "/tmp/anthropic-dump", providers[0].APIDumpDir())
assert.Equal(t, "/tmp/openai-dump", providers[1].APIDumpDir())
})
t.Run("LegacyOpenAIConflictsWithIndexed", func(t *testing.T) {
@@ -154,7 +167,7 @@ func TestBuildProviders(t *testing.T) {
// Copilot API hosts via an explicit BASE_URL.
cfg := codersdk.AIBridgeConfig{
Providers: []codersdk.AIBridgeProviderConfig{
{Type: aibridge.ProviderCopilot, Name: aibridge.ProviderCopilot},
{Type: aibridge.ProviderCopilot, Name: aibridge.ProviderCopilot, DumpDir: "/tmp/copilot-dump"},
{Type: aibridge.ProviderCopilot, Name: agplaibridge.ProviderCopilotBusiness, BaseURL: "https://" + agplaibridge.HostCopilotBusiness},
{Type: aibridge.ProviderCopilot, Name: agplaibridge.ProviderCopilotEnterprise, BaseURL: "https://" + agplaibridge.HostCopilotEnterprise},
},
@@ -165,6 +178,7 @@ func TestBuildProviders(t *testing.T) {
require.Len(t, providers, 3)
assert.Equal(t, aibridge.ProviderCopilot, providers[0].Name())
assert.Equal(t, "/tmp/copilot-dump", providers[0].APIDumpDir())
assert.Equal(t, agplaibridge.ProviderCopilotBusiness, providers[1].Name())
assert.Equal(t, "https://"+agplaibridge.HostCopilotBusiness, providers[1].BaseURL())
assert.Equal(t, agplaibridge.ProviderCopilotEnterprise, providers[2].Name())
+4
View File
@@ -144,6 +144,10 @@ export interface AIBridgeProviderConfig {
* BaseURL is the base URL of the upstream provider API.
*/
readonly base_url: string;
/**
* DumpDir is the directory path for dumping API requests and responses.
*/
readonly dump_dir?: string;
readonly bedrock_region?: string;
readonly bedrock_model?: string;
readonly bedrock_small_fast_model?: string;