Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 9341efe

Browse filesBrowse files
dannykoppingdannykopping
authored
feat!: seed ai_providers from env on server startup (#24895)
_Disclaimer: implemented by a Coder Agent using Claude Opus 4.7_ Part of the implementation of [RFC: Common AI Provider Configs](https://www.notion.so/coderhq/RFC-Common-AI-Provider-Configs-34bd579be59280ed958feffb82024797) (AIGOV-201). ## Note This change can cause a previously working installation to fail to start should a conflict exist between the providers configured in the environment & those now migrated to the database. I'll raise a PR upstack to document this process and workarounds should a startup fail. ## What this PR does Reconciles environment-derived AI provider configuration with the `ai_providers` table at server startup. The seed runs **before** the aibridged daemon is initialized, so the runtime always reads providers from the database; the legacy `CODER_AIBRIDGE_*` environment variables become a one-shot migration source. ### Behavior - Concurrent server starts are serialized through a Postgres advisory lock (`LockIDAIProvidersEnvSeed`). - Missing rows are inserted with an audit entry attributed to the system actor. - Existing rows whose canonical hash matches the env-derived hash are left alone (the common no-op restart path). - Existing rows whose canonical hash does **not** match cause server startup to fail with a descriptive error so the operator can explicitly resolve the conflict in either env or DB. - Soft-deleted rows are NOT resurrected from env; an explicit operator deletion is sticky across restarts. - Indexed providers whose name conflicts with a legacy env var fail startup with a clear remediation message. - Unknown provider types (e.g. `copilot`, until the DB enum is widened) are skipped with a log entry rather than failing startup. ### Canonical hashing The `canonicalAIProvider` shape captures exactly the fields that determine runtime behavior — `type`, `base_url`, and the Bedrock subset of settings (access key, access key secret, region, model, small fast model) — and is hashed with SHA-256. The hash is **computed on demand from the row + env**, never persisted, so the database does not need a new column for it. API keys live in the separate `ai_provider_keys` table and are intentionally excluded from the hash so operators can rotate keys via the API without forcing a server restart. <details> <summary>Decision log</summary> - The hash is intentionally not persisted in the database. The RFC discussed this trade-off; computing on demand keeps the schema minimal and lets the canonical shape evolve without a migration. - The lock uses an `iota` slot in `coderd/database/lock.go` rather than `GenLockID` so it's stable, easy to audit, and matches the convention used for every other startup lock. - A bearer-token Anthropic provider whose env vars also set Bedrock metadata but no AWS credentials does NOT store the Bedrock fields. Without credentials the discriminated settings would misrepresent the row as Bedrock auth. - We deliberately do NOT publish to the `ai_providers_changed` pubsub channel from the seed because the seed completes before any subscriber is started; the follow-up PR introduces that channel. </details>
1 parent 06526a5 commit 9341efe
Copy full SHA for 9341efe

9 files changed

+1,170-18Lines changed: 1170 additions & 18 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎cli/server.go‎

Copy file name to clipboardExpand all lines: cli/server.go
+61-5Lines changed: 61 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -862,6 +862,10 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
862862
}
863863
vals.AI.BridgeConfig.Providers = append(vals.AI.BridgeConfig.Providers, aiProviders...)
864864

865+
if err := validateLegacyAIBridgeConfig(vals.AI.BridgeConfig); err != nil {
866+
return xerrors.Errorf("validate legacy AI bridge config: %w", err)
867+
}
868+
865869
// Manage push notifications.
866870
webpusher, err := webpush.New(ctx, ptr.Ref(options.Logger.Named("webpush")), options.Database, options.AccessURL.String())
867871
if err != nil {
@@ -1010,6 +1014,18 @@ func (r *RootCmd) Server(newAPI func(context.Context, *coderd.Options) (*coderd.
10101014
return xerrors.Errorf("create coder API: %w", err)
10111015
}
10121016

1017+
// Runs unconditionally so operators can seed providers via
1018+
// env without enabling the bridge or proxy features.
1019+
if err := coderd.SeedAIProvidersFromEnv(
1020+
ctx,
1021+
options.Database,
1022+
vals.AI.BridgeConfig,
1023+
options.Auditor,
1024+
logger.Named("aibridge.envseed"),
1025+
); err != nil {
1026+
return xerrors.Errorf("seed ai providers from env: %w", err)
1027+
}
1028+
10131029
if vals.Prometheus.Enable {
10141030
// Agent metrics require reference to the tailnet coordinator, so must be initiated after Coder API.
10151031
closeAgentsFunc, err := prometheusmetrics.Agents(ctx, logger, options.PrometheusRegistry, coderAPI.Database, &coderAPI.TailnetCoordinator, coderAPI.DERPMap, coderAPI.Options.AgentInactiveDisconnectTimeout, 0)
@@ -2961,7 +2977,20 @@ func ReadAIProvidersFromEnv(logger slog.Logger, environ []string) ([]codersdk.AI
29612977
i, p.Type, aibridge.ProviderOpenAI, aibridge.ProviderAnthropic, aibridge.ProviderCopilot)
29622978
}
29632979

2964-
if p.Type != aibridge.ProviderAnthropic && hasBedrockFields(*p) {
2980+
var bedrockKey, bedrockSecret string
2981+
if len(p.BedrockAccessKeys) > 0 {
2982+
bedrockKey = p.BedrockAccessKeys[0]
2983+
}
2984+
if len(p.BedrockAccessKeySecrets) > 0 {
2985+
bedrockSecret = p.BedrockAccessKeySecrets[0]
2986+
}
2987+
settings := codersdk.NewAIProviderBedrockSettings(
2988+
p.BedrockRegion, bedrockKey, bedrockSecret,
2989+
p.BedrockModel, p.BedrockSmallFastModel,
2990+
)
2991+
isBedrock := codersdk.IsBedrockConfigured(p.BedrockBaseURL, settings)
2992+
2993+
if p.Type != aibridge.ProviderAnthropic && isBedrock {
29652994
return nil, xerrors.Errorf("provider %d (%s): BEDROCK_* fields are only supported with TYPE %q",
29662995
i, p.Type, aibridge.ProviderAnthropic)
29672996
}
@@ -2971,6 +3000,15 @@ func ReadAIProvidersFromEnv(logger slog.Logger, environ []string) ([]codersdk.AI
29713000
i, p.Type, aibridge.ProviderCopilot)
29723001
}
29733002

3003+
// An Anthropic provider authenticates either via a bearer
3004+
// token (KEYS) or via Bedrock (BEDROCK_*), not both. Surface
3005+
// the conflict here so misconfigured deployments fail before
3006+
// any DB work happens at server startup.
3007+
if p.Type == aibridge.ProviderAnthropic && len(p.Keys) > 0 && isBedrock {
3008+
return nil, xerrors.Errorf("provider %d (%s): KEY/KEYS and BEDROCK_* fields are mutually exclusive",
3009+
i, p.Type)
3010+
}
3011+
29743012
if err := validateProviderCredentialList(i, p.Type, p.Keys); err != nil {
29753013
return nil, err
29763014
}
@@ -3092,10 +3130,28 @@ func readAIProvidersForPrefix(logger slog.Logger, environ []string, prefix strin
30923130
return providers, nil
30933131
}
30943132

3095-
func hasBedrockFields(p codersdk.AIProviderConfig) bool {
3096-
return p.BedrockBaseURL != "" || p.BedrockRegion != "" ||
3097-
len(p.BedrockAccessKeys) > 0 || len(p.BedrockAccessKeySecrets) > 0 ||
3098-
p.BedrockModel != "" || p.BedrockSmallFastModel != ""
3133+
// validateLegacyAIBridgeConfig enforces invariants on the legacy
3134+
// single-provider env vars (CODER_AIBRIDGE_ANTHROPIC_KEY,
3135+
// CODER_AIBRIDGE_BEDROCK_*) that the indexed validator above can't
3136+
// catch because legacy fields live outside cfg.Providers.
3137+
func validateLegacyAIBridgeConfig(cfg codersdk.AIBridgeConfig) error {
3138+
// An Anthropic provider authenticates either via a bearer token
3139+
// or via Bedrock, not both. Fields without serpent-level
3140+
// defaults (region, base URL, credentials) reliably indicate
3141+
// operator intent; Model and SmallFastModel are excluded because
3142+
// they have defaults.
3143+
settings := codersdk.NewAIProviderBedrockSettings(
3144+
cfg.LegacyBedrock.Region.String(),
3145+
cfg.LegacyBedrock.AccessKey.String(),
3146+
cfg.LegacyBedrock.AccessKeySecret.String(),
3147+
cfg.LegacyBedrock.Model.String(),
3148+
cfg.LegacyBedrock.SmallFastModel.String(),
3149+
)
3150+
hasBedrock := codersdk.IsBedrockConfigured(cfg.LegacyBedrock.BaseURL.String(), settings)
3151+
if cfg.LegacyAnthropic.Key.String() != "" && hasBedrock {
3152+
return xerrors.New("CODER_AIBRIDGE_ANTHROPIC_KEY and CODER_AIBRIDGE_BEDROCK_* are mutually exclusive")
3153+
}
3154+
return nil
30993155
}
31003156

31013157
// maxKeysPerProvider is the maximum number of keys allowed per
Collapse file

‎cli/server_aibridge_internal_test.go‎

Copy file name to clipboardExpand all lines: cli/server_aibridge_internal_test.go
+98-4Lines changed: 98 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,25 +194,50 @@ func TestReadAIProvidersFromEnv(t *testing.T) {
194194
},
195195
},
196196
{
197-
// KEYS, BEDROCK_ACCESS_KEYS, and BEDROCK_ACCESS_KEY_SECRETS
198-
// are plural aliases for their singular counterparts.
199-
name: "PluralKeyAliases",
197+
// KEYS is a plural alias for KEY.
198+
name: "PluralKeysAlias",
200199
env: []string{
201200
"CODER_AIBRIDGE_PROVIDER_0_TYPE=anthropic",
202201
"CODER_AIBRIDGE_PROVIDER_0_KEYS=sk-ant-xxx",
202+
},
203+
expected: []codersdk.AIProviderConfig{
204+
{
205+
Type: aibridge.ProviderAnthropic,
206+
Name: aibridge.ProviderAnthropic,
207+
Keys: []string{"sk-ant-xxx"},
208+
},
209+
},
210+
},
211+
{
212+
// BEDROCK_ACCESS_KEYS and BEDROCK_ACCESS_KEY_SECRETS are
213+
// plural aliases for their singular counterparts.
214+
name: "PluralBedrockAliases",
215+
env: []string{
216+
"CODER_AIBRIDGE_PROVIDER_0_TYPE=anthropic",
203217
"CODER_AIBRIDGE_PROVIDER_0_BEDROCK_ACCESS_KEYS=AKID",
204218
"CODER_AIBRIDGE_PROVIDER_0_BEDROCK_ACCESS_KEY_SECRETS=secret",
205219
},
206220
expected: []codersdk.AIProviderConfig{
207221
{
208222
Type: aibridge.ProviderAnthropic,
209223
Name: aibridge.ProviderAnthropic,
210-
Keys: []string{"sk-ant-xxx"},
211224
BedrockAccessKeys: []string{"AKID"},
212225
BedrockAccessKeySecrets: []string{"secret"},
213226
},
214227
},
215228
},
229+
{
230+
// An Anthropic provider can't use both a bearer token
231+
// (KEYS) and Bedrock (BEDROCK_*); they're mutually
232+
// exclusive authentication modes.
233+
name: "AnthropicKeysAndBedrockConflict",
234+
env: []string{
235+
"CODER_AIBRIDGE_PROVIDER_0_TYPE=anthropic",
236+
"CODER_AIBRIDGE_PROVIDER_0_KEYS=sk-ant-xxx",
237+
"CODER_AIBRIDGE_PROVIDER_0_BEDROCK_REGION=us-east-1",
238+
},
239+
errContains: "KEY/KEYS and BEDROCK_* fields are mutually exclusive",
240+
},
216241
{
217242
name: "ConflictKeyAndKeys",
218243
env: []string{
@@ -443,3 +468,72 @@ func TestReadAIProvidersFromEnv(t *testing.T) {
443468
}
444469
})
445470
}
471+
472+
func TestValidateLegacyAIBridgeConfig(t *testing.T) {
473+
t.Parallel()
474+
475+
tests := []struct {
476+
name string
477+
cfg codersdk.AIBridgeConfig
478+
errContains string
479+
}{
480+
{
481+
name: "BareAnthropicKey",
482+
cfg: codersdk.AIBridgeConfig{
483+
LegacyAnthropic: codersdk.AIBridgeAnthropicConfig{Key: "sk-ant"},
484+
},
485+
},
486+
{
487+
name: "BareBedrockRegion",
488+
cfg: codersdk.AIBridgeConfig{
489+
LegacyBedrock: codersdk.AIBridgeBedrockConfig{Region: "us-east-1"},
490+
},
491+
},
492+
{
493+
name: "BedrockCredentialsOnly",
494+
cfg: codersdk.AIBridgeConfig{
495+
LegacyBedrock: codersdk.AIBridgeBedrockConfig{
496+
AccessKey: "AKIA",
497+
AccessKeySecret: "secret",
498+
},
499+
},
500+
},
501+
{
502+
name: "AnthropicKeyAndBedrockConflict",
503+
cfg: codersdk.AIBridgeConfig{
504+
LegacyAnthropic: codersdk.AIBridgeAnthropicConfig{Key: "sk-ant"},
505+
LegacyBedrock: codersdk.AIBridgeBedrockConfig{
506+
Region: "us-east-1",
507+
AccessKey: "AKIA",
508+
AccessKeySecret: "secret",
509+
},
510+
},
511+
errContains: "CODER_AIBRIDGE_ANTHROPIC_KEY and CODER_AIBRIDGE_BEDROCK_* are mutually exclusive",
512+
},
513+
{
514+
name: "AnthropicKeyWithBedrockModelDefaultsIsFine",
515+
cfg: codersdk.AIBridgeConfig{
516+
LegacyAnthropic: codersdk.AIBridgeAnthropicConfig{Key: "sk-ant"},
517+
// Model defaults shouldn't trip the conflict; they're
518+
// always populated in a real deployment.
519+
LegacyBedrock: codersdk.AIBridgeBedrockConfig{
520+
Model: "anthropic.claude-3-5-sonnet",
521+
SmallFastModel: "anthropic.claude-3-5-haiku",
522+
},
523+
},
524+
},
525+
}
526+
527+
for _, tt := range tests {
528+
t.Run(tt.name, func(t *testing.T) {
529+
t.Parallel()
530+
err := validateLegacyAIBridgeConfig(tt.cfg)
531+
if tt.errContains == "" {
532+
require.NoError(t, err)
533+
return
534+
}
535+
require.Error(t, err)
536+
require.Contains(t, err.Error(), tt.errContains)
537+
})
538+
}
539+
}

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.