feat(providers): configure provider credentials from the dashboard - #566
Conversation
Lets GoModel run and be fully configured with zero provider API-key env vars: a new admin-managed provider-credentials store (SQLite/Postgres/ Mongo) plus a "Providers" dashboard page for CRUD, mirroring the existing MCP-servers declarative-shadows-store precedence (env/config.yaml-declared providers stay read-only; dashboard-managed ones hot-register into the live registry immediately, no restart). - internal/providers: new CredentialsService/CredentialStore, and ModelRegistry.UnregisterProvider (previously missing) so add/edit/ disable/delete apply live. providers.Init and the registry's refresh path no longer treat zero configured providers as a startup failure. - internal/admin: /admin/provider-credentials GET/PUT/DELETE and /admin/provider-credentials/types, with positional "***" redaction for API keys and service-account secrets. The list merges in declarative providers (read-only, secrets never loaded) and hides any store row a declared name now shadows. Shared the managed-resource delete flow with MCP servers to avoid duplicating the same handler shape. - Dashboard: new "Providers" page (Type-first form with name auto-suggestion, API key rotation rows, collapsed advanced settings for less-common fields), standardized icon-button/toggle-switch shapes and disabled-field styling app-wide along the way. - Docs: CLAUDE.md, docs/providers/overview.mdx, .env.template, README, and regenerated Swagger/OpenAPI. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughGoModel adds persisted provider credentials with runtime registration, admin API endpoints, a dashboard Providers page, multiple storage backends, and documentation for startup without provider keys and related API schema changes. ChangesProvider credential management
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant AdminDashboard as Admin dashboard
participant ProviderCredentialAPI as Provider credential API
participant CredentialsService
participant CredentialStore
participant ModelRegistry
AdminDashboard->>ProviderCredentialAPI: PUT provider credential
ProviderCredentialAPI->>CredentialsService: Upsert credential
CredentialsService->>CredentialStore: Persist credential
CredentialsService->>ModelRegistry: Register or unregister provider
ProviderCredentialAPI-->>AdminDashboard: Redacted credential view
Possibly related PRs
Suggested labels: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@internal/providers/credentials_wire.go`:
- Around line 80-95: Update newCredentialsResult so that when
NewCredentialsService returns an error after createCredentialStore succeeds, it
closes the live store before returning; preserve the existing error return and
successful result behavior.
In `@internal/providers/credentials.go`:
- Around line 201-214: Validate provider names in the credential validation flow
around IsManaged and the cred.Name assignment, rejecting `/` and any other
characters reserved as model-selector qualifier delimiters; apply the same
format validation in the admin handler’s UpsertProviderCredential path so both
entry points enforce the identical naming rule.
- Around line 216-287: The Upsert flow removes the existing provider before
validating the replacement, leaving failed edits offline and skipping Refresh.
Split register’s resolve/create work from registry mutation: validate and create
the replacement first, return the “saved but not applied” error without
unregistering or changing the live provider when that fails, then swap the
validated provider into the registry and refresh as before; update register or
introduce a clearly named helper while preserving its configuration behavior,
and add coverage for editing an already-registered credential into an
unresolvable state.
- Around line 96-103: Protect provider-credential mutations with the service’s
mutation lock: update UpsertProviderCredential and DeleteProviderCredential to
acquire h.mutationMu for the full mutation sequence, including store and
registry updates. Ensure the lock is always released while preserving existing
behavior and error handling.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 7bca522d-3569-4d5d-9a16-0e82058c85bf
📒 Files selected for processing (37)
.env.templateCLAUDE.mdREADME.mdcmd/gomodel/docs/docs.godocs/openapi.jsondocs/providers/overview.mdxinternal/admin/dashboard/static/css/dashboard.cssinternal/admin/dashboard/static/js/dashboard.jsinternal/admin/dashboard/static/js/modules/dashboard-layout.test.cjsinternal/admin/dashboard/static/js/modules/providers-config.jsinternal/admin/dashboard/static/js/modules/providers-config.test.cjsinternal/admin/dashboard/templates/index.htmlinternal/admin/dashboard/templates/layout.htmlinternal/admin/dashboard/templates/page-providers-config.htmlinternal/admin/dashboard/templates/sidebar.htmlinternal/admin/handler.gointernal/admin/handler_managed_resource.gointernal/admin/handler_mcpservers.gointernal/admin/handler_provider_credentials.gointernal/admin/handler_provider_credentials_test.gointernal/admin/routes.gointernal/admin/routes_test.gointernal/app/app.gointernal/providers/credentials.gointernal/providers/credentials_store.gointernal/providers/credentials_store_mongodb.gointernal/providers/credentials_store_postgresql.gointernal/providers/credentials_store_sqlite.gointernal/providers/credentials_store_sqlite_test.gointernal/providers/credentials_test.gointernal/providers/credentials_wire.gointernal/providers/factory.gointernal/providers/init.gointernal/providers/init_test.gointernal/providers/registry.gointernal/providers/registry_init.gointernal/providers/registry_unregister_test.go
Addresses CodeRabbit review feedback on PR #566: - CredentialsService.Upsert unregistered the existing provider before validating the replacement, so an edit that failed to resolve (e.g. stripping the only API key) took a previously-working, actively-serving provider offline with no way back except another edit. Now the replacement is built and validated first (buildProvider), and only swapped into the registry (install) on success -- a bad edit leaves whatever was already registered untouched. - Reject '/' in provider names: model selectors use "name/model" syntax, so a name containing '/' would be unreachable or ambiguous. Enforced at both the admin handler (clean 400) and the service layer. - Serialize provider-credential mutations with the admin handler's existing mutationMu (already used by guardrails/tagging/workflows) so concurrent edits of the same name can't interleave between the redaction-merge read and the store write. - Close the credential store if NewCredentialsService fails after the store was already created, avoiding a leaked connection on that error path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Reviewed all bot comments critically (not just autofixed). Verified each CodeRabbit finding against the actual code before acting: Fixed (all in caa1abb):
Deliberately not changed:
🤖 Generated with Claude Code |
Match the rounded-square icon-button style used everywhere else (was still circular, missed in the earlier standardization pass), and pull it in slightly (margin-right: -9px) so it sits flush with the row edge. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Summary
Lets GoModel run and be fully configured without any provider API-key env vars. Providers declared via env vars/
config.yamlkeep working exactly as before and stay read-only in the dashboard; a new "Providers" page lets you add, edit, disable, and delete dashboard-managed provider credentials, which apply to the running gateway immediately — no restart.provider_credentialsadmin store (SQLite/PostgreSQL/MongoDB), mirroring the existing MCP-servers declarative-shadows-store precedence: a provider name declared via env/config.yaml is always read-only (shown with a "Config" badge), and a dashboard-managed row hot-registers/unregisters into the live registry.ModelRegistry.UnregisterProvider(new — didn't exist before) makes edit/disable/delete work live.providers.Initand the registry's refresh path no longer treat "zero providers configured" as a startup failure — that's the whole point of the feature.GET/PUT /admin/provider-credentials,GET /admin/provider-credentials/types,DELETE /admin/provider-credentials/:name. API keys and service-account secrets are redacted as***(positional for the key-rotation list); an upsert echoing***back preserves the stored value at that position.{type}, then{type}-1,{type}-2, ...), repeatable API-key rows, and less-common fields (Base URL, Models, Vertex/service-account, ...) tucked behind an "Advanced settings" disclosure. Standardized icon-button and toggle-switch shapes (rounded-square instead of circular) and disabled-field styling app-wide along the way.CLAUDE.md,docs/providers/overview.mdx,.env.template,README.md, regenerated Swagger/OpenAPI.Three real bugs were found and fixed via actually running the built binary end-to-end (not just unit tests with mocks):
providers.Inithard-failing on zero providers, the registry'sRefresherroring on zero remaining providers, andUpsert/Deleteconflating a provider's own network/credential failure with a save failure (a wrong API key would return a scary error instead of just showing "Unhealthy").Test plan
go build ./...andgo vet ./...go test ./internal/...(full suite, including new registry/credentials/admin tests)make test-dashboard(499 JS unit tests)make lint/ pre-commit hooks all greencmd/gomodel, ran with zero provider env vars (boots successfully), exercised create/update/delete against/admin/provider-credentials, verified"***"redaction round-trips correctly, verified env-only declared providers (e.g.OLLAMA_BASE_URLwith no config.yaml entry) are correctly protected as read-only/admin/dashboard/providers-config🤖 Generated with Claude Code
Summary by CodeRabbit
LOGGING_RETENTION_DAYSandstop_sequence.