feat: add ProjectID to Bedrock and Bedrock Mantle key configs for Mantle project scoping - #5131
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (13)
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds optional Bedrock and Bedrock Mantle project IDs, persists them through encrypted configstore fields, and injects project-specific headers into Mantle model listing, Chat Completions, Responses, OpenAI-compatible, and native Anthropic request paths. ChangesBedrock project configuration and persistence
Bedrock header helpers and model listing
Wire project headers into Mantle requests
Estimated code review effort: 3 (Moderate) | ~25 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Comment |
dbbae1e to
2592af4
Compare
|
@impoiler will need to add this support in |
2592af4 to
e4118c0
Compare
ProjectID to Bedrock and Bedrock Mantle key configs for Mantle project scoping
Pratham-Mishra04
left a comment
There was a problem hiding this comment.
small refactor comment
Confidence Score: 2/5These issues need to be fixed before merging.
core/providers/bedrock/mantle.go, core/providers/bedrock/utils.go, framework/configstore/tables/key.go Important Files Changed
Reviews (4): Last reviewed commit: "feat: project id support for bedrock pro..." | Re-trigger Greptile |
e4118c0 to
774393b
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
core/providers/bedrockmantle/project_test.go (1)
11-32: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winCover header injection, not only project resolution.
This test verifies
resolveProjectID, but not thatOpenAI-Projectandanthropic-workspace-idare added or omitted correctly across model listing, Chat Completions, Responses, and native Anthropic requests. Add request-level tests so wiring regressions cannot pass while this helper test remains green.Based on learnings from the PR objective, project IDs must affect multiple Mantle request surfaces and header variants.
🤖 Prompt for 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. In `@core/providers/bedrockmantle/project_test.go` around lines 11 - 32, Expand TestResolveProjectID coverage with request-level tests for the Bedrock Mantle provider, verifying project-derived OpenAI-Project and anthropic-workspace-id headers are correctly added or omitted across model listing, Chat Completions, Responses, and native Anthropic requests. Exercise both configured and absent project IDs, and assert the outgoing requests rather than only the resolveProjectID helper.framework/configstore/migrations.go (1)
1168-1204: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a migration-level upgrade test.
The current
framework/configstore/tables/encryption_test.gosetup callsAutoMigrate(&TableKey{}), which creates the new columns directly and therefore does not verify that this migration upgrades an existing database or that its registration is correct.Based on learnings from the existing test setup, the current round-trip tests do not exercise
migrationAddBedrockProjectIDColumns.🤖 Prompt for 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. In `@framework/configstore/migrations.go` around lines 1168 - 1204, The migrationAddBedrockProjectIDColumns change lacks an upgrade test that validates registration and alteration of an existing schema. Add a migration-level test that creates the pre-migration TableKey schema without the Bedrock project columns, runs the registered migration through the normal migrator path, and asserts both columns are added; avoid AutoMigrate on the current TableKey definition before migration.
🤖 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 `@core/schemas/account.go`:
- Around line 701-705: Add project_id?: SecretVar to both BedrockKeyConfig and
BedrockMantleKeyConfig in the frontend types, and add matching optional
SecretVar validation fields to their schemas in providerForm.ts and the relevant
definitions in ui/lib/types/schemas.ts so the provider key form can round-trip
ProjectID.
- Around line 674-678: Update BedrockAliasCfg and its related config handling to
define and propagate the ProjectID SecretVar, including redaction and Bedrock
Mantle request header selection. Ensure Bedrock alias validation accepts this
Bedrock-specific override and uses the alias ProjectID when setting the
OpenAI-Project header, while preserving account-level fallback when it is unset.
In `@framework/configstore/migrations.go`:
- Around line 1190-1197: Update the migration containing the Rollback function
to explicitly mark it as destructive/non-rollbackable, using the migration
framework’s established non-rollbackable flag or metadata. Preserve the existing
column-dropping behavior unless the framework requires exporting values first,
and ensure the migration is clearly documented as non-reversible.
---
Nitpick comments:
In `@core/providers/bedrockmantle/project_test.go`:
- Around line 11-32: Expand TestResolveProjectID coverage with request-level
tests for the Bedrock Mantle provider, verifying project-derived OpenAI-Project
and anthropic-workspace-id headers are correctly added or omitted across model
listing, Chat Completions, Responses, and native Anthropic requests. Exercise
both configured and absent project IDs, and assert the outgoing requests rather
than only the resolveProjectID helper.
In `@framework/configstore/migrations.go`:
- Around line 1168-1204: The migrationAddBedrockProjectIDColumns change lacks an
upgrade test that validates registration and alteration of an existing schema.
Add a migration-level test that creates the pre-migration TableKey schema
without the Bedrock project columns, runs the registered migration through the
normal migrator path, and asserts both columns are added; avoid AutoMigrate on
the current TableKey definition before migration.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 7ff498da-afce-407f-a762-7077caca0dd0
📒 Files selected for processing (11)
core/providers/bedrock/bedrock.gocore/providers/bedrock/mantle.gocore/providers/bedrock/mantle_project_test.gocore/providers/bedrockmantle/bedrockmantle.gocore/providers/bedrockmantle/project_test.gocore/providers/bedrockmantle/utils.gocore/schemas/account.goframework/configstore/clientconfig.goframework/configstore/migrations.goframework/configstore/tables/encryption_test.goframework/configstore/tables/key.go
774393b to
41b7be7
Compare
Merge activity
|
41b7be7 to
168168e
Compare
| func resolveMantleProjectID(key schemas.Key) string { | ||
| if key.BedrockKeyConfig != nil && key.BedrockKeyConfig.ProjectID != nil { | ||
| return key.BedrockKeyConfig.ProjectID.GetValue() | ||
| } | ||
| return "" | ||
| } |
There was a problem hiding this comment.
Duplicate resolver declaration This declares
resolveMantleProjectID in package bedrock, but core/providers/bedrock/utils.go also declares a package-level resolveMantleProjectID with a different signature. Go does not support function overloading, so any build or test target that includes this package fails before the new project-scoping code can run. Keep one resolver and update the call sites to that signature.
| } else { | ||
| k.BedrockRoleSessionName = nil | ||
| } | ||
| if k.BedrockKeyConfig.ProjectID != nil { | ||
| pid := *k.BedrockKeyConfig.ProjectID | ||
| k.BedrockProjectID = &pid | ||
| } else { | ||
| k.BedrockProjectID = nil | ||
| } |
There was a problem hiding this comment.
Project ID can clear This saves
BedrockKeyConfig.ProjectID, but the provider-key update merge path still preserves the older Bedrock fields without including project_id. When a Bedrock key already has a project scope and a later API update submits a redacted or partial key config, the merge can leave ProjectID nil; this branch then clears bedrock_project_id, so future requests silently fall back to the default project. Add project_id to the Bedrock key preserve logic as part of this new persisted field.
| } else { | ||
| k.BedrockMantleRoleSessionName = nil | ||
| } | ||
| if k.BedrockMantleKeyConfig.ProjectID != nil { | ||
| pid := *k.BedrockMantleKeyConfig.ProjectID | ||
| k.BedrockMantleProjectID = &pid | ||
| } else { | ||
| k.BedrockMantleProjectID = nil | ||
| } |
There was a problem hiding this comment.
Mantle project can clear The new Bedrock Mantle project field has the same update-path gap. Existing update merge handling preserves the Bedrock Mantle credential fields, but not
project_id, so an unrelated key edit can save a nil BedrockMantleKeyConfig.ProjectID here and clear bedrock_mantle_project_id. That makes the key lose its project scope and route later inference to the default project.
…antle project scoping (#5131) ## Summary Adds `ProjectID` support to both `BedrockKeyConfig` and `BedrockMantleKeyConfig`, allowing inference requests and model listing to be scoped to a specific Bedrock project. Without a project ID, AWS continues to route to the account's default project. ## Changes - Added `ProjectID *SecretVar` to `BedrockKeyConfig` and `BedrockMantleKeyConfig` schemas. On the OpenAI-compatible surface (`chat/completions`, `responses`, `/models`) it is sent as the `OpenAI-Project` header; on the native-Anthropic (Claude) surface it is sent as `anthropic-workspace-id`. - Introduced `WithMantleProject` in `core/providers/bedrock/mantle.go` — a pure function that clones the shared `networkConfig.ExtraHeaders` map and injects the project header only when a project ID is present, ensuring the base map is never mutated. - Added `resolveMantleProjectID` (bedrock provider) and `resolveProjectID` (bedrockmantle provider) helpers that read the project ID from the respective key config, returning `""` when unset. - Applied `WithMantleProject` across all Mantle request paths in both the `bedrock` and `bedrockmantle` providers: `mantleChatCompletions`, `mantleChatCompletionsStream`, `mantleResponses`, `mantleResponsesStream`, `listMantleModels`, `ChatCompletion`, `ChatCompletionStream`, `Responses`, `ResponsesStream`, and `listModelsByKey`. - Added `bedrock_project_id` and `bedrock_mantle_project_id` columns to the `config_keys` table via a new migration (`add_bedrock_project_id_columns`), with full encrypt/decrypt lifecycle in `BeforeSave`/`AfterFind` hooks and reconstruction into the key config structs. - Project IDs are treated as non-credential identifiers in the redacted config view and are surfaced in plaintext rather than masked. ## Type of change - [ ] Bug fix - [x] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/providers/bedrock/... ./core/providers/bedrockmantle/... ./framework/configstore/... ``` To validate end-to-end: 1. Configure a `BedrockKeyConfig` or `BedrockMantleKeyConfig` with a `project_id` value pointing to a valid Bedrock project. 2. Issue a chat completion or model listing request and confirm the `OpenAI-Project` (or `anthropic-workspace-id` for Claude) header is present in the outbound request and that responses are scoped to the specified project. 3. Omit `project_id` and confirm requests continue to route to the account's default project without any header being injected. **New config fields:** | Provider | Config struct | JSON field | Header sent | |---|---|---|---| | `bedrock` | `BedrockKeyConfig` | `project_id` | `OpenAI-Project` (Mantle paths only) | | `bedrock_mantle` | `BedrockMantleKeyConfig` | `project_id` | `OpenAI-Project` or `anthropic-workspace-id` | ## Breaking changes - [ ] Yes - [x] No ## Security considerations - `ProjectID` values are encrypted at rest in the database alongside other key config fields, consistent with how credentials are stored. - They are intentionally exposed in plaintext in the redacted config view because they are routing identifiers, not secrets. - `WithMantleProject` always clones the base headers map before writing, preventing accidental mutation of the shared `networkConfig.ExtraHeaders` reference across concurrent requests. ## Checklist - [x] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable

Summary
Adds
ProjectIDsupport to bothBedrockKeyConfigandBedrockMantleKeyConfig, allowing inference requests and model listing to be scoped to a specific Bedrock project. Without a project ID, AWS continues to route to the account's default project.Changes
ProjectID *SecretVartoBedrockKeyConfigandBedrockMantleKeyConfigschemas. On the OpenAI-compatible surface (chat/completions,responses,/models) it is sent as theOpenAI-Projectheader; on the native-Anthropic (Claude) surface it is sent asanthropic-workspace-id.WithMantleProjectincore/providers/bedrock/mantle.go— a pure function that clones the sharednetworkConfig.ExtraHeadersmap and injects the project header only when a project ID is present, ensuring the base map is never mutated.resolveMantleProjectID(bedrock provider) andresolveProjectID(bedrockmantle provider) helpers that read the project ID from the respective key config, returning""when unset.WithMantleProjectacross all Mantle request paths in both thebedrockandbedrockmantleproviders:mantleChatCompletions,mantleChatCompletionsStream,mantleResponses,mantleResponsesStream,listMantleModels,ChatCompletion,ChatCompletionStream,Responses,ResponsesStream, andlistModelsByKey.bedrock_project_idandbedrock_mantle_project_idcolumns to theconfig_keystable via a new migration (add_bedrock_project_id_columns), with full encrypt/decrypt lifecycle inBeforeSave/AfterFindhooks and reconstruction into the key config structs.Type of change
Affected areas
How to test
go test ./core/providers/bedrock/... ./core/providers/bedrockmantle/... ./framework/configstore/...To validate end-to-end:
BedrockKeyConfigorBedrockMantleKeyConfigwith aproject_idvalue pointing to a valid Bedrock project.OpenAI-Project(oranthropic-workspace-idfor Claude) header is present in the outbound request and that responses are scoped to the specified project.project_idand confirm requests continue to route to the account's default project without any header being injected.New config fields:
bedrockBedrockKeyConfigproject_idOpenAI-Project(Mantle paths only)bedrock_mantleBedrockMantleKeyConfigproject_idOpenAI-Projectoranthropic-workspace-idBreaking changes
Security considerations
ProjectIDvalues are encrypted at rest in the database alongside other key config fields, consistent with how credentials are stored.WithMantleProjectalways clones the base headers map before writing, preventing accidental mutation of the sharednetworkConfig.ExtraHeadersreference across concurrent requests.Checklist
docs/contributing/README.mdand followed the guidelines