feat: Bedrock Guardrail configs support environment variable assignment#11717
feat: Bedrock Guardrail configs support environment variable assignment#11717entropic489 wants to merge 1 commit intodanny-avila:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Adds support for resolving ${ENV_VAR} placeholders inside Bedrock guardrailConfig entries during initialization, aligning guardrail configuration with existing env-var expansion behavior used elsewhere in the Bedrock setup.
Changes:
- Resolve
guardrailIdentifierandguardrailVersionviaextractEnvVariable()whenguardrailConfigis provided. - Add unit tests validating env-var expansion behavior (including whitespace and missing env var fallback) for guardrail config fields.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/api/src/endpoints/bedrock/initialize.ts | Expands env-var placeholders for guardrail identifier/version when building llmConfig.guardrailConfig. |
| packages/api/src/endpoints/bedrock/initialize.spec.ts | Adds coverage for env-var expansion scenarios for Bedrock guardrail configuration. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| guardrailIdentifier: extractEnvVariable(bedrockConfig.guardrailConfig.guardrailIdentifier), | ||
| guardrailVersion: extractEnvVariable(bedrockConfig.guardrailConfig.guardrailVersion), | ||
| trace: bedrockConfig.guardrailConfig.trace, |
There was a problem hiding this comment.
To avoid duplicating the GuardrailConfiguration shape (and potentially dropping new fields if the type/API expands later), consider spreading the existing bedrockConfig.guardrailConfig and only overriding the resolved fields (guardrailIdentifier/guardrailVersion). This keeps behavior consistent if additional properties are added in the future.
| guardrailIdentifier: extractEnvVariable(bedrockConfig.guardrailConfig.guardrailIdentifier), | |
| guardrailVersion: extractEnvVariable(bedrockConfig.guardrailConfig.guardrailVersion), | |
| trace: bedrockConfig.guardrailConfig.trace, | |
| ...bedrockConfig.guardrailConfig, | |
| guardrailIdentifier: extractEnvVariable(bedrockConfig.guardrailConfig.guardrailIdentifier), | |
| guardrailVersion: extractEnvVariable(bedrockConfig.guardrailConfig.guardrailVersion), |
| it('should resolve environment variables in guardrailIdentifier', async () => { | ||
| const guardrailId = 'gr-abc123xyz'; | ||
| process.env.GUARDRAIL_ID = guardrailId; | ||
|
|
||
| const params = createMockParams({ | ||
| config: { |
There was a problem hiding this comment.
The new env-var expansion tests are very similar and add a lot of repeated setup/assertions. Consider consolidating these into a parameterized test (e.g., it.each) and/or a small helper to set env + build params, to keep the spec file easier to maintain as cases evolve.
Summary
Supports loading Bedrock guardrailConfig items from environment variables, using the existing extractEnvVariable() method.
Change Type
Testing
I've added tests in the Bedrock initialize.spec.ts to ensure env var expansion is working as expected.
Test Configuration:
Checklist