HYPERFLEET-1108 - feat: Allow setting labels when installing chart#173
HYPERFLEET-1108 - feat: Allow setting labels when installing chart#173mliptak0 wants to merge 1 commit into
Conversation
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Warning Review limit reached
More reviews will be available in 35 minutes and 36 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Central YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe Estimated code review effort🎯 2 (Simple) | ⏱️ ~5 minutes CWE-20 (Improper Input Validation) — label injection surface.
Concrete risk: a values file or Recommended mitigations:
🚥 Pre-merge checks | ✅ 11✅ Passed checks (11 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Risk Score: 0 —
|
| Signal | Detail | Points |
|---|---|---|
| PR size | 13 lines | +0 |
| Sensitive paths | none | +0 |
Computed by hyperfleet-risk-scorer
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 `@charts/templates/_helpers.tpl`:
- Around line 39-41: The template block that uses toYaml to directly append
.Values.labels into metadata.labels lacks input validation, allowing callers to
inject reserved Kubernetes keys (helm.sh/chart, app.kubernetes.io/*) or
non-string values that break manifest validity. Replace the direct toYaml dump
with validation logic that either filters out reserved key prefixes before
rendering or routes the labels through an existing typed label contract/helper
function that enforces the label schema. Ensure only valid, string-valued,
non-reserved labels are included in the final output.
🪄 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: Central YAML (base), Organization UI (inherited)
Review profile: CHILL
Plan: Enterprise
Run ID: 46dfee46-d280-4b3d-b3f6-3ec6def9f348
📒 Files selected for processing (1)
charts/templates/_helpers.tpl
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
openshift-hyperfleet/architecture(manual)openshift-hyperfleet/hyperfleet-api(manual)openshift-hyperfleet/hyperfleet-sentinel(manual)openshift-hyperfleet/hyperfleet-adapter(manual)openshift-hyperfleet/hyperfleet-broker(manual)
| {{- with .Values.labels }} | ||
| {{ toYaml . }} | ||
| {{- end }} |
There was a problem hiding this comment.
Do not dump arbitrary values into metadata.labels.
toYaml here appends whatever is in .Values.labels into the shared labels map consumed by Deployments and ConfigMaps. That lets callers collide with reserved keys (helm.sh/chart, app.kubernetes.io/*) or render non-string label values, which can turn a chart install into an invalid-manifest failure. Route this through the existing typed label contract or reject reserved keys before rendering. CWE-20.
🤖 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 `@charts/templates/_helpers.tpl` around lines 39 - 41, The template block that
uses toYaml to directly append .Values.labels into metadata.labels lacks input
validation, allowing callers to inject reserved Kubernetes keys (helm.sh/chart,
app.kubernetes.io/*) or non-string values that break manifest validity. Replace
the direct toYaml dump with validation logic that either filters out reserved
key prefixes before rendering or routes the labels through an existing typed
label contract/helper function that enforces the label schema. Ensure only
valid, string-valued, non-reserved labels are included in the final output.
a18a1f4 to
111d432
Compare
Add support for custom labels via .Values.labels that are applied to all Helm-managed resources (Deployment, Service, ServiceAccount, ConfigMaps, etc.). Include commented-out example in values.yaml to make the feature discoverable without requiring inspection of _helpers.tpl. Enables E2E test cleanup by labeling resources with run IDs. Co-Authored-By: Claude <noreply@anthropic.com>
111d432 to
e1df28c
Compare
Summary
Add possibility to set labels when installing chart
Test Plan
make test-allpassesmake lintpassesmake test-helm(if applicable)