Skip to content

HYPERFLEET-1108 - feat: Allow setting labels when installing chart#173

Open
mliptak0 wants to merge 1 commit into
openshift-hyperfleet:mainfrom
mliptak0:HYPERFLEET-1108
Open

HYPERFLEET-1108 - feat: Allow setting labels when installing chart#173
mliptak0 wants to merge 1 commit into
openshift-hyperfleet:mainfrom
mliptak0:HYPERFLEET-1108

Conversation

@mliptak0

@mliptak0 mliptak0 commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Add possibility to set labels when installing chart

  • HYPERFLEET-1108

Test Plan

  • Unit tests added/updated
  • make test-all passes
  • make lint passes
  • Helm chart changes validated with make test-helm (if applicable)
  • Deployed to a development cluster and verified (if Helm/config changes)
  • E2E tests passed (if cross-component or major changes)

@openshift-ci openshift-ci Bot requested review from ldornele and mbrudnoy June 15, 2026 11:40
@openshift-ci

openshift-ci Bot commented Jun 15, 2026

Copy link
Copy Markdown

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign ldornele for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@coderabbitai

coderabbitai Bot commented Jun 15, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@mliptak0, we couldn't start this review because you've reached your PR review rate limit.

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 @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

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 configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Enterprise

Run ID: bd603f23-613b-4f8a-be24-5e9c7853fc50

📥 Commits

Reviewing files that changed from the base of the PR and between becf832 and e1df28c.

📒 Files selected for processing (2)
  • charts/templates/_helpers.tpl
  • charts/values.yaml
📝 Walkthrough

Walkthrough

The sentinel.labels Helm template helper in charts/templates/_helpers.tpl is modified to conditionally append arbitrary label key/value pairs from .Values.labels to its output. A with block guards the addition, rendering the user-supplied map via toYaml only when .Values.labels is non-empty. No other helpers or logic are changed.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~5 minutes


CWE-20 (Improper Input Validation) — label injection surface.

.Values.labels is rendered verbatim via toYaml with no key/value filtering. Any string accepted by Kubernetes label syntax passes through, including values that overwrite keys already emitted by the common labels block above (e.g., app.kubernetes.io/name, helm.sh/chart). Duplicate YAML keys produce implementation-defined behavior in Helm's YAML parser — in practice, last-write wins, silently overriding chart-controlled metadata.

Concrete risk: a values file or --set argument can override app.kubernetes.io/managed-by, breaking Helm release tracking, or inject labels used by NetworkPolicy/RBAC selectors to gain unintended scope.

Recommended mitigations:

  • Explicitly blocklist or allowlist override-sensitive label keys before rendering.
  • Use merge or mustMerge to give chart-defined labels precedence:
    {{- with .Values.labels }}
    {{- toYaml (omit . "app.kubernetes.io/name" "app.kubernetes.io/instance" "helm.sh/chart" "app.kubernetes.io/managed-by") | nindent 4 }}
    {{- end }}
    
  • Document in values.yaml exactly which keys are reserved.
🚥 Pre-merge checks | ✅ 11
✅ Passed checks (11 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: adding the ability to set labels when installing the Helm chart, matching the changeset modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Sec-02: Secrets In Log Output ✅ Passed PR modifies only Helm template file (charts/templates/_helpers.tpl), not code with logging statements. Check targets log functions in .go files, which are not present in this change.
No Hardcoded Secrets ✅ Passed No hardcoded secrets, API keys, tokens, passwords, private keys, base64 credentials, or URLs with embedded credentials found in PR changes to charts/templates/_helpers.tpl.
No Weak Cryptography ✅ Passed PR modifies only Helm template file (charts/templates/_helpers.tpl) with label configuration; no cryptographic operations, banned primitives, or weak algorithms present.
No Injection Vectors ✅ Passed PR adds toYaml on .Values.labels to Helm template. No SQL string concatenation (CWE-89), exec.Command (CWE-78), template.HTML (CWE-79), or yaml.Unmarshal (CWE-502) injection vectors found in code.
No Privileged Containers ✅ Passed PR adds label support to Helm template helpers; no privileged container configurations (privileged: true, hostPID/Network/IPC, SYS_ADMIN, allowPrivilegeEscalation, runAsUser: 0) introduced. Dockerf...
No Pii Or Sensitive Data In Logs ✅ Passed PR modifies only Helm chart _helpers.tpl template to add user labels to Kubernetes metadata. No logging statements expose PII, session IDs, request/response bodies, or sensitive data.
Description check ✅ Passed The PR description directly relates to the changeset: it describes adding label functionality to the Helm chart, which matches the file change in _helpers.tpl that conditionally includes labels from .Values.labels.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified 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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@hyperfleet-ci-bot

hyperfleet-ci-bot Bot commented Jun 15, 2026

Copy link
Copy Markdown

Risk Score: 0 — risk/low

Signal Detail Points
PR size 13 lines +0
Sensitive paths none +0

Computed by hyperfleet-risk-scorer

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

📥 Commits

Reviewing files that changed from the base of the PR and between 6cb186e and becf832.

📒 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)

Comment on lines +39 to +41
{{- with .Values.labels }}
{{ toYaml . }}
{{- end }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

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.

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant