feat: add bifrost.alerting to Helm chart with declarative channels and CEL-based rules#5079
Conversation
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 42 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (8)
📝 WalkthroughWalkthroughThe Helm chart adds declarative alerting values, schema validation for channels and rules, conditional rendering into Bifrost configuration, environment-variable credential support, and changelog documentation. ChangesDeclarative alerting configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Suggested reviewers: Sequence Diagram(s)sequenceDiagram
participant HelmValues
participant ValuesSchema
participant HelmHelpers
participant BifrostConfig
participant TransportSchema
HelmValues->>ValuesSchema: Validate alerting channels and rules
ValuesSchema->>HelmHelpers: Pass validated alerting values
HelmHelpers->>BifrostConfig: Render the alerting object
BifrostConfig->>TransportSchema: Validate channel URLs and credentials
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
BearTS
force-pushed
the
07-10-chore_add_alerting_to_helm
branch
from
3508986 to
79d9b67
Compare
BearTS
force-pushed
the
07-10-feat_new_calendar_alignment_additions
branch
from
db41911 to
4bf0254
Compare
BearTS
changed the title
bifrost.alerting to Helm chart with declarative channels and CEL-based rules
Confidence Score: 4/5This should be fixed before merging.
helm-charts/bifrost/values.schema.json Important Files Changed
Reviews (4): Last reviewed commit: "chore: add alerting to helm" | Re-trigger Greptile |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
SahilChoudhary22
force-pushed
the
07-10-chore_add_alerting_to_helm
branch
from
79d9b67 to
bfef6d8
Compare
![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 3319-3333: Replace the alias-presence anyOf constraints with oneOf
so exactly one credential alias is required, preventing configurations from
specifying both aliases. Apply this consistently to the Slack, Teams, PagerDuty,
and generic webhook schemas in values.schema.json, including the corresponding
alias checks identified by the diff; mirror the same oneOf changes in the
transport schemas.
🪄 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: 99b62279-2e5e-4d56-83ee-46311980a41a
📒 Files selected for processing (6)
docs/changelogs/helm-v2.1.28.mdxhelm-charts/bifrost/README.mdhelm-charts/bifrost/templates/_helpers.tplhelm-charts/bifrost/values.schema.jsonhelm-charts/bifrost/values.yamltransports/config.schema.json
SahilChoudhary22
force-pushed
the
07-10-chore_add_alerting_to_helm
branch
from
bfef6d8 to
d11fdf2
Compare
BearTS
force-pushed
the
07-10-chore_add_alerting_to_helm
branch
from
d11fdf2 to
cd85c91
Compare
| }, | ||
| "required": ["id", "name", "type", "enabled"], | ||
| "allOf": [ | ||
| { |
There was a problem hiding this comment.
id, name, type, and enabled but omits config, Helm can accept and render the value directly into alerting. Bifrost then receives a channel without the delivery config required by the runtime config contract, so the deployment can render successfully and fail during startup validation. Please enforce these channel-specific requirements with validation that Helm reliably applies, or with template-time checks.
Rule Used: Do not suggest adding the "not" keyword (or other ... (source)
| "target_id": { "type": "string", "description": "ID of the explicit target" } | ||
| }, | ||
| "required": ["id", "name", "enabled", "scope_type", "scope_id", "cel_expression", "channel_ids"], | ||
| "allOf": [ |
There was a problem hiding this comment.
target_type and target_id pair is still enforced only through conditional schema branches. A rule with only target_type: budget or only target_id can still be rendered into config.json, but the runtime alerting rule contract expects those fields to be present together. That can let Helm accept a chart that later fails Bifrost startup validation. Please enforce this pair with Helm-supported validation or a template-time check.
Rule Used: Do not suggest adding the "not" keyword (or other ... (source)
1 participant
Summary
Adds
bifrost.alertingas a new top-level Helm values key, enabling declarative configuration of alert channels and rules that are seeded into the Bifrost config store at startup. This allows operators to define alerting behavior entirely through Helm without manual UI configuration.Changes
bifrost.alertingtovalues.yamlwith commented examples covering all supported channel types (Slack, Microsoft Teams, PagerDuty, generic webhook) and CEL-expression-based rules.bifrost.alertinginvalues.schema.json, coveringhistory_retention_days,webhook_network(allow_http,allow_private_network),channels[], andrules[]with required field validation and enum constraints.bifrost.alertinginto the Helm template (_helpers.tpl) so it renders directly as the top-levelalertingkey in the generated config.helm-v2.1.28changelog andREADME.md.Alert rules use CEL expressions evaluated against governance usage variables (e.g.,
budget_usage_percent,budget_spent,request_usage) and support scoping tovirtual_key,team, orcustomer. Rules can optionally target a specific budget viatarget_type/target_idand support per-rulecooldown_secondsandnotify_once_per_reset_cyclecontrols.Type of change
Affected areas
How to test
Deploy the Helm chart with an
alertingblock in yourvalues.yamland verify the rendered config contains the expectedalertingtop-level key:Example minimal
values.yamlsnippet to test:Validate schema enforcement by providing an invalid
typevalue or omitting a required field and confirminghelm lintrejects it.Screenshots/Recordings
N/A
Breaking changes
Related issues
N/A
Security considerations
webhook_network.allow_httpandallow_private_networkdefault tofalse, preventing accidental exposure of internal services or plaintext webhook traffic. Operators must explicitly opt in to relax these restrictions.env.*references for secrets (e.g.,routing_key,webhook_url), avoiding hardcoded credentials invalues.yaml.Checklist
docs/contributing/README.mdand followed the guidelines