adds back runAsRoot in helm chart (explicit disable for OpenShift deployments) - #5215
Conversation
|
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
📝 WalkthroughSummary by CodeRabbit
WalkthroughHelm chart version 2.1.28 restores ChangesHelm chart behavior and release
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Confidence Score: 5/5This looks safe to merge.
Important Files Changed
Reviews (3): Last reviewed commit: "adds back runAsRoot in helm chart (expli..." | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@helm-charts/bifrost/values.schema.json`:
- Around line 5201-5203: Update the project_id description in the schema to
distinguish provider-specific behavior: standard Bedrock supports OpenAI-Project
for its OpenAI-compatible path and anthropic-workspace-id for its
Anthropic/Converse path, while Bedrock Mantle is OpenAI-compatible-only, uses
OpenAI-Project, and has no effect on Converse/runtime paths.
🪄 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: 0b46314e-b772-4875-995f-bf432f371964
📒 Files selected for processing (7)
.github/workflows/scripts/validate-helm-templates.shdocs/changelogs/helm-v2.1.28.mdxdocs/docs.jsonhelm-charts/bifrost/Chart.yamlhelm-charts/bifrost/README.mdhelm-charts/bifrost/values.schema.jsonhelm-charts/bifrost/values.yaml
8435d8e to
c1225d4
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 @.github/workflows/scripts/validate-helm-templates.sh:
- Line 437: Update the security-context validation around the runAsUser check to
inspect only the intended workload’s pod securityContext rather than the entire
multi-document Helm output. Parse that target pod field and compare its value
numerically or exactly to 1000, ensuring values such as 10000 do not match.
In `@helm-charts/bifrost/README.md`:
- Around line 11-15: The README release notes incorrectly name the native
Anthropic header as anthropic-workspace-id. Update the project_id documentation
in the 2.1.28 entry to use anthropic-workspace, keeping OpenAI-Project for the
OpenAI-compatible surface and matching the corresponding values configuration.
🪄 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: 95db6944-e49d-4308-96a7-6521f8d7f5f8
📒 Files selected for processing (8)
.github/workflows/scripts/validate-helm-templates.shdocs/changelogs/helm-v2.1.27.mdxdocs/changelogs/helm-v2.1.28.mdxdocs/docs.jsonhelm-charts/bifrost/Chart.yamlhelm-charts/bifrost/README.mdhelm-charts/bifrost/values.schema.jsonhelm-charts/bifrost/values.yaml
🚧 Files skipped from review as they are similar to previous changes (4)
- helm-charts/bifrost/Chart.yaml
- docs/changelogs/helm-v2.1.28.mdx
- docs/docs.json
- helm-charts/bifrost/values.schema.json
| ### 2.1.28 | ||
|
|
||
| - Restored `runAsUser: 1000` defaults in `podSecurityContext` and `securityContext` (dropped in 2.1.27). Images before v1.6.4 use a non-numeric `USER appuser`, so kubelet could not verify `runAsNonRoot: true` and pods failed with CreateContainerConfigError. OpenShift (restricted-v2) users unset the pins with explicit nulls: `podSecurityContext.runAsUser: null`, `podSecurityContext.fsGroup: null`, `securityContext.runAsUser: null`. | ||
| - Added `project_id` to `bifrost.providers.bedrock.keys[*].bedrock_key_config` (renders into `bedrock_key_config.project_id`) and `bifrost.providers.bedrock_mantle.keys[*].bedrock_mantle_key_config` (renders into `bedrock_mantle_key_config.project_id`) for AWS project scoping via the OpenAI-Project / anthropic-workspace-id headers. | ||
| - Updated the per-alias `project_id` description: it is now a shared cross-provider override (Vertex GCP project; Bedrock/Bedrock Mantle AWS project header). |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## README excerpt\n'
sed -n '1,40p' helm-charts/bifrost/README.md
printf '\n## values.yaml around line 444\n'
sed -n '420,470p' helm-charts/bifrost/values.yaml
printf '\n## Search for anthropic-workspace strings\n'
rg -n "anthropic-workspace|OpenAI-Project|project_id|workspace-id" helm-charts/bifrost -SRepository: maximhq/bifrost
Length of output: 11897
🌐 Web query:
AWS Bedrock workspace anthropic-work header OpenAI-Project anthropic-workspace docs
💡 Result:
In Amazon Bedrock, the headers anthropic-workspace and OpenAI-Project are used to associate inference requests with specific Amazon Bedrock projects [1][2]. These headers are primarily utilized when interacting with the Bedrock Mantle (Anthropic-compatible) or OpenAI-compatible API endpoints [2][3]. Specifically: - anthropic-workspace: This header is used on the Anthropic Messages API (e.g., when calling /anthropic/v1/messages) to associate a request with a Bedrock workspace or project [2][3]. - OpenAI-Project: This header is used similarly on OpenAI-compatible API endpoints within Amazon Bedrock to associate requests with a project [2]. In the context of Amazon Bedrock, projects and workspaces are effectively the same resource [2]. These identifiers allow users to manage, organize, and track inference workloads [1][2]. It is important to distinguish this from the anthropic-beta header. While native Anthropic APIs use the anthropic-beta header, Amazon Bedrock typically requires beta features to be handled differently. For instance, many integrations and AWS SDK implementations move beta parameters into the request body (e.g., under additionalModelRequestFields or specific JSON fields) rather than passing them as HTTP headers, as headers may conflict with AWS SigV4 signing requirements [4][5].
Citations:
- 1: https://docs.aws.amazon.com/bedrock/latest/userguide/workspaces.html
- 2: feat(bedrock): aws_bedrock_project_id for bedrock-mantle project / workspace association BerriAI/litellm#30163
- 3: https://docs.aws.amazon.com/bedrock/latest/userguide/inference-messages-api.html
- 4: fix(bedrock): Filter anthropic-beta header for Bedrock passthrough BerriAI/litellm#20012
- 5: vercel/ai@0a6fd91
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n## Search for anthropic-workspace-id in templates/schema\n'
rg -n "anthropic-workspace-id|anthropic-workspace|OpenAI-Project" helm-charts/bifrost/templates helm-charts/bifrost/values.schema.json helm-charts/bifrost/values.yaml -S
printf '\n## Relevant schema section\n'
sed -n '5048,5070p' helm-charts/bifrost/values.schema.json
printf '\n## Relevant schema section for shared project_id override\n'
sed -n '5196,5210p' helm-charts/bifrost/values.schema.jsonRepository: maximhq/bifrost
Length of output: 3385
Use anthropic-workspace here, not anthropic-workspace-id. Keep the README and helm-charts/bifrost/values.yaml:444 aligned with AWS’s header names: OpenAI-Project for the OpenAI-compatible surface and anthropic-workspace for native Anthropic requests.
🤖 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 `@helm-charts/bifrost/README.md` around lines 11 - 15, The README release notes
incorrectly name the native Anthropic header as anthropic-workspace-id. Update
the project_id documentation in the 2.1.28 entry to use anthropic-workspace,
keeping OpenAI-Project for the OpenAI-compatible surface and matching the
corresponding values configuration.
Sources: Path instructions, MCP tools
There was a problem hiding this comment.
You're right that AWS documents the header as anthropic-workspace (https://docs.aws.amazon.com/bedrock/latest/userguide/workspaces.html: "reference them in Messages API requests using the anthropic-workspace header"). However, this README line intentionally documents the header Bifrost core actually sends today: core/providers/bedrock/mantle.go defines MantleAnthropicProjectHeader = "anthropic-workspace-id", used on all native-Anthropic Mantle calls.
Changing only the README would make the docs describe behavior the shipped code doesn't have. The header-name discrepancy needs to be verified and fixed core-side first (this PR only touches the helm chart); the README will be updated in lockstep with that core change. Flagging the anthropic-workspace-id vs anthropic-workspace question for a follow-up core PR.
There was a problem hiding this comment.
@akshaydeo, that makes sense — thanks for tracing it to MantleAnthropicProjectHeader in core/providers/bedrock/mantle.go. Agreed this is a core-side fix, and keeping the README aligned with actual shipped behavior for now is the right call. This PR is scoped to the Helm chart, so I won't push further on this here.
Would you like me to open a follow-up issue to track reconciling anthropic-workspace-id with AWS's documented anthropic-workspace header in core/providers/bedrock/mantle.go (and updating the Helm README/values docs once that lands)? Happy to file it if so.
✏️ Learnings added
Learnt from: akshaydeo
Repo: maximhq/bifrost PR: 5215
File: helm-charts/bifrost/README.md:11-15
Timestamp: 2026-07-14T22:36:15.527Z
Learning: In maximhq/bifrost, the Bedrock Mantle native-Anthropic header name sent by core is `anthropic-workspace-id`, defined as `MantleAnthropicProjectHeader` in `core/providers/bedrock/mantle.go`. This differs from AWS's documented header name `anthropic-workspace`. The Helm chart README (`helm-charts/bifrost/README.md`) and `values.yaml` intentionally document `anthropic-workspace-id` to match current shipped core behavior; reconciling the naming requires a core-side change first, to be followed by a docs update.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
c1225d4 to
9405a82
Compare
Merge activity
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/scripts/validate-helm-templates.sh (1)
434-480: 🔒 Security & Privacy | 🔵 Trivial | 💤 Low valueReused predictable
/tmppath across test invocations.Static analysis flags the hardcoded
/tmp/helm-template-output.yamlreused across all threehelm templateinvocations as CWE-377 (symlink/TOCTOU). Exploitability is low here since this only runs in ephemeral GitHub Actions runners, but switching tomktempis a trivial hardening.🛡️ Proposed fix
+tmpfile="$(mktemp)" +trap 'rm -f "$tmpfile"' EXIT + test_name="default Bifrost pod sets runAsUser: 1000 (kubelet runAsNonRoot verification)" if helm template bifrost ./helm-charts/bifrost \ --set image.tag=v1.0.0 \ -s templates/stateful.yaml \ - > /tmp/helm-template-output.yaml 2>&1; then - if grep -Eq '^[[:space:]]*runAsUser:[[:space:]]*1000$' /tmp/helm-template-output.yaml; then + > "$tmpfile" 2>&1; then + if grep -Eq '^[[:space:]]*runAsUser:[[:space:]]*1000$' "$tmpfile"; thenApply the same substitution for the other two
helm template ... > /tmp/helm-template-output.yamlblocks.🤖 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 @.github/workflows/scripts/validate-helm-templates.sh around lines 434 - 480, Replace the reused hardcoded /tmp/helm-template-output.yaml path in all three helm template test blocks with a unique mktemp-created file, and use that variable for redirection and subsequent grep/head reads. Apply the same change to the default render, postgres-mode render, and OpenShift override checks while preserving their existing validation behavior.Source: Linters/SAST tools
🤖 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 `@helm-charts/bifrost/README.md`:
- Around line 19-29: Update the OpenShift guidance in the 2.1.27 known-issue
paragraph to remove “works as-is” and state that OpenShift restricted-v2
requires podSecurityContext.fsGroup: null, together with image v1.6.4+; keep the
existing SCC and installation-section references.
---
Nitpick comments:
In @.github/workflows/scripts/validate-helm-templates.sh:
- Around line 434-480: Replace the reused hardcoded
/tmp/helm-template-output.yaml path in all three helm template test blocks with
a unique mktemp-created file, and use that variable for redirection and
subsequent grep/head reads. Apply the same change to the default render,
postgres-mode render, and OpenShift override checks while preserving their
existing validation behavior.
🪄 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: 422e87b9-dee5-47d8-8421-ca20d45f357c
📒 Files selected for processing (8)
.github/workflows/scripts/validate-helm-templates.shdocs/changelogs/helm-v2.1.27.mdxdocs/changelogs/helm-v2.1.28.mdxdocs/docs.jsonhelm-charts/bifrost/Chart.yamlhelm-charts/bifrost/README.mdhelm-charts/bifrost/values.schema.jsonhelm-charts/bifrost/values.yaml
🚧 Files skipped from review as they are similar to previous changes (6)
- docs/docs.json
- docs/changelogs/helm-v2.1.28.mdx
- helm-charts/bifrost/Chart.yaml
- docs/changelogs/helm-v2.1.27.mdx
- helm-charts/bifrost/values.yaml
- helm-charts/bifrost/values.schema.json
| > **Known issue - use 2.1.28 instead.** This version dropped `runAsUser: 1000` | ||
| > from the default security contexts. With any image before v1.6.4 (including | ||
| > the chart's default), kubelet cannot verify `runAsNonRoot: true` against the | ||
| > image's non-numeric `USER appuser` and pods fail with | ||
| > `CreateContainerConfigError: container has runAsNonRoot and image has | ||
| > non-numeric user (appuser)`. If you must stay on 2.1.27, set | ||
| > `podSecurityContext.runAsUser: 1000` and `securityContext.runAsUser: 1000` | ||
| > in your values, or use image v1.6.4+. On OpenShift (restricted-v2), 2.1.27 | ||
| > works as-is since the SCC injects a numeric UID; pair it with image v1.6.4+ | ||
| > and `podSecurityContext.fsGroup: null` (see the OpenShift section under | ||
| > Installation). |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Correct the 2.1.27 OpenShift guidance.
This says 2.1.27 “works as-is,” but the next lines require podSecurityContext.fsGroup: null; restricted-v2 rejects the chart’s pinned fsGroup: 1000, so users following the “as-is” wording can still fail admission. Change this to state that 2.1.27 requires the fsGroup override (and the compatible image) on OpenShift.
Proposed wording
-> On OpenShift (restricted-v2), 2.1.27
-> works as-is since the SCC injects a numeric UID; pair it with image v1.6.4+
-> and `podSecurityContext.fsGroup: null` (see the OpenShift section under
-> Installation).
+> On OpenShift (restricted-v2), 2.1.27 still requires
+> `podSecurityContext.fsGroup: null` because its default `fsGroup: 1000` may
+> be rejected by the SCC. Use image v1.6.4+ as described in the OpenShift
+> section under Installation.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| > **Known issue - use 2.1.28 instead.** This version dropped `runAsUser: 1000` | |
| > from the default security contexts. With any image before v1.6.4 (including | |
| > the chart's default), kubelet cannot verify `runAsNonRoot: true` against the | |
| > image's non-numeric `USER appuser` and pods fail with | |
| > `CreateContainerConfigError: container has runAsNonRoot and image has | |
| > non-numeric user (appuser)`. If you must stay on 2.1.27, set | |
| > `podSecurityContext.runAsUser: 1000` and `securityContext.runAsUser: 1000` | |
| > in your values, or use image v1.6.4+. On OpenShift (restricted-v2), 2.1.27 | |
| > works as-is since the SCC injects a numeric UID; pair it with image v1.6.4+ | |
| > and `podSecurityContext.fsGroup: null` (see the OpenShift section under | |
| > Installation). | |
| > **Known issue - use 2.1.28 instead.** This version dropped `runAsUser: 1000` | |
| > from the default security contexts. With any image before v1.6.4 (including | |
| > the chart's default), kubelet cannot verify `runAsNonRoot: true` against the | |
| > image's non-numeric `USER appuser` and pods fail with | |
| > `CreateContainerConfigError: container has runAsNonRoot and image has | |
| > non-numeric user (appuser)`. If you must stay on 2.1.27, set | |
| > `podSecurityContext.runAsUser: 1000` and `securityContext.runAsUser: 1000` | |
| > in your values, or use image v1.6.4+. On OpenShift (restricted-v2), 2.1.27 | |
| > still requires `podSecurityContext.fsGroup: null` because its default | |
| > `fsGroup: 1000` may be rejected by the SCC. Use image v1.6.4+ as described | |
| > in the OpenShift section under Installation. |
🤖 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 `@helm-charts/bifrost/README.md` around lines 19 - 29, Update the OpenShift
guidance in the 2.1.27 known-issue paragraph to remove “works as-is” and state
that OpenShift restricted-v2 requires podSecurityContext.fsGroup: null, together
with image v1.6.4+; keep the existing SCC and installation-section references.
2 participants
Summary
Restores
runAsUser: 1000defaults that were dropped in v2.1.27, fixingCreateContainerConfigErroron Bifrost images before v1.6.4 (which use a non-numericUSER appuser, preventing kubelet from verifyingrunAsNonRoot: truewithout an explicit UID). Also addsproject_idsupport to Bedrock and Bedrock Mantle key configs for AWS project scoping.Changes
runAsUser: 1000in bothpodSecurityContextandsecurityContextdefaults invalues.yaml. Without this, kubelet cannot resolverunAsNonRootfor images with a non-numericUSERdirective and pods fail at container creation.podSecurityContext.runAsUser: null,podSecurityContext.fsGroup: null,securityContext.runAsUser: null) so the SCC can assign an arbitrary UID.project_idtobifrost.providers.bedrock.keys[*].bedrock_key_config— sent as theOpenAI-Projectheader (OpenAI-compatible surface) andanthropic-workspace-idheader (native Anthropic/Claude surface).project_idtobifrost.providers.bedrock_mantle.keys[*].bedrock_mantle_key_config— sent as theOpenAI-Projectheader on the Mantle sub-surface.project_iddescription to reflect it is now a shared cross-provider override (Vertex GCP project; Bedrock/Bedrock Mantle AWS project header).runAsUser: 1000is present in default and postgres-mode renders, and added a new test verifying that null overrides correctly remove UID pins for OpenShift.2.1.28and added the corresponding changelog entry.Type of change
Affected areas
How to test
Breaking changes
Users who upgraded to v2.1.27 and relied on the absence of
runAsUserfor OpenShift compatibility must now explicitly setpodSecurityContext.runAsUser: null,podSecurityContext.fsGroup: null, andsecurityContext.runAsUser: nullin their values to restore SCC-assigned UID behaviour.Related issues
Regression introduced in v2.1.27.
Security considerations
Pinning
runAsUser: 1000enforces non-root execution on vanilla Kubernetes for pre-v1.6.4 images where kubelet cannot infer the UID from the image manifest. OpenShift users delegating UID assignment to the SCC must explicitly null out the pins; the image supports arbitrary UIDs with group 0 and requires no custom SCC.Checklist
docs/contributing/README.mdand followed the guidelines