feat: add access_profile_id support to bifrost helm chart virtual key schema and helpers - #3622
feat: add access_profile_id support to bifrost helm chart virtual key schema and helpers#3622BearTS wants to merge 1 commit into
access_profile_id support to bifrost helm chart virtual key schema and helpers#3622Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughAdds optional ChangesVirtual Keys Access Profile Support
🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly Related PRs
Suggested Reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
This stack of pull requests is managed by Graphite. Learn more about stacking. |
BearTS
changed the title
access_profile_id support to bifrost helm chart virtual key schema and helpers
Confidence Score: 5/5Safe to merge — all three changed files are Helm chart artifacts with no logic changes to core Go code. The change is tightly scoped to Helm chart configuration: one new line in the template helper, a new field in the JSON schema with correct type and working mutual-exclusivity enforcement, and updated YAML comments. The No files require special attention. Important Files Changed
Reviews (6): Last reviewed commit: "feat: support for helm to assign vk on a..." | 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: 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 `@helm-charts/bifrost/templates/_helpers.tpl`:
- Line 466: The template currently checks truthiness with "if
.access_profile_id" which will skip valid zero values; change the presence check
to use hasKey so numeric zero isn't dropped: replace the conditional with
something like {{- if hasKey . "access_profile_id" }} and keep the set call {{-
$_ := set $vk "access_profile_id" .access_profile_id }} unchanged so the key is
set when the field exists even if it's 0.
In `@helm-charts/bifrost/values.schema.json`:
- Around line 1232-1243: The schema currently documents but does not enforce
that team_id, customer_id, and access_profile_id are mutually exclusive; add a
validation rule that disallows any pairwise combination by adding a "not" +
"anyOf" block at the same object schema level that contains these properties,
e.g. a "not": { "anyOf": [ { "required": ["team_id","customer_id"] }, {
"required": ["team_id","access_profile_id"] }, { "required":
["customer_id","access_profile_id"] } ] } to the object that defines team_id,
customer_id and access_profile_id so the validator rejects configs with more
than one of these keys present.
🪄 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: CHILL
Plan: Pro
Run ID: 048bae26-53a4-4ede-ac83-f25028f7329d
📒 Files selected for processing (3)
helm-charts/bifrost/templates/_helpers.tplhelm-charts/bifrost/values.schema.jsonhelm-charts/bifrost/values.yaml
da12406 to
2dffd87
Compare
7a46efe to
8fef3f3
Compare
2dffd87 to
4ea46e8
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
4ea46e8 to
8596f7a
Compare
8fef3f3 to
154b26e
Compare
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 1240-1242: The schema for the property "access_profile_id"
currently allows any integer (including 0 and negatives); update the JSON Schema
entry for access_profile_id to require positive identifiers by adding "minimum":
1 to the access_profile_id definition so values must be >=1, ensuring it aligns
with the template rendering expectations; locate the access_profile_id object in
values.schema.json and add the minimum constraint alongside the existing "type":
"integer" and "description".
🪄 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: CHILL
Plan: Pro
Run ID: 37755368-e3fe-4786-9b64-9af464c60bf8
📒 Files selected for processing (3)
helm-charts/bifrost/templates/_helpers.tplhelm-charts/bifrost/values.schema.jsonhelm-charts/bifrost/values.yaml
✅ Files skipped from review due to trivial changes (1)
- helm-charts/bifrost/values.yaml
| "access_profile_id": { | ||
| "type": "integer", | ||
| "description": "Assign this virtual key to an access profile template (mutually exclusive with team_id and customer_id; enterprise only)" |
There was a problem hiding this comment.
Constrain access_profile_id to positive IDs.
access_profile_id is currently any integer, so 0/negative values pass schema validation even though they are not valid identifier values and can be silently dropped in template emission paths. Add minimum: 1 to keep schema and rendering behavior aligned.
Suggested fix
"access_profile_id": {
"type": "integer",
+ "minimum": 1,
"description": "Assign this virtual key to an access profile template (mutually exclusive with team_id and customer_id; enterprise only)"
},As per coding guidelines "always check the stack if there is one for the current PR. do not give localized reviews for the PR, always see all changes in the light of the whole stack of PRs".
📝 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.
| "access_profile_id": { | |
| "type": "integer", | |
| "description": "Assign this virtual key to an access profile template (mutually exclusive with team_id and customer_id; enterprise only)" | |
| "access_profile_id": { | |
| "type": "integer", | |
| "minimum": 1, | |
| "description": "Assign this virtual key to an access profile template (mutually exclusive with team_id and customer_id; enterprise only)" | |
| }, |
🤖 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/values.schema.json` around lines 1240 - 1242, The schema
for the property "access_profile_id" currently allows any integer (including 0
and negatives); update the JSON Schema entry for access_profile_id to require
positive identifiers by adding "minimum": 1 to the access_profile_id definition
so values must be >=1, ensuring it aligns with the template rendering
expectations; locate the access_profile_id object in values.schema.json and add
the minimum constraint alongside the existing "type": "integer" and
"description".
f59c88c to
ff463d9
Compare
1 participant
Summary
Adds support for
access_profile_idas a virtual key assignment option in the Bifrost Helm chart, enabling enterprise users to associate virtual keys with access profile templates.Changes
access_profile_idfield handling in the_helpers.tpltemplate so the value is included when constructing virtual key configurationsaccess_profile_idto the JSON schema as an integer type with a description noting it is enterprise-only and mutually exclusive withteam_idandcustomer_idteam_idandcustomer_idschema descriptions to reflect that they are also mutually exclusive withaccess_profile_idvalues.yamlcomments to document the new field and the mutual exclusivity constraints across all three assignment optionsType of change
Affected areas
How to test
Deploy the Helm chart with a virtual key configured using
access_profile_id:Verify the rendered template includes
access_profile_idin the virtual key configuration:Confirm that the schema validates correctly and rejects configurations where
access_profile_idis used alongsideteam_idorcustomer_id.Screenshots/Recordings
N/A
Breaking changes
Related issues
Security considerations
access_profile_idis an enterprise-only field that controls access profile assignment for virtual keys. Ensure that only authorized users can set this field, as it governs access control boundaries for the associated key.Checklist
docs/contributing/README.mdand followed the guidelines