Skip to content

feat(helm): propagate commonLabels to RBAC resources#8818

Open
mvanhorn wants to merge 1 commit intoenvoyproxy:mainfrom
mvanhorn:osc/8817-commonlabels-rbac
Open

feat(helm): propagate commonLabels to RBAC resources#8818
mvanhorn wants to merge 1 commit intoenvoyproxy:mainfrom
mvanhorn:osc/8817-commonlabels-rbac

Conversation

@mvanhorn
Copy link
Copy Markdown

Summary

Thread commonLabels through the four RBAC resources declared in charts/gateway-helm/templates/envoy-gateway-rbac.yaml by adding the existing eg.labels helper to each of them - the same pattern already used in certgen-rbac.yaml and envoy-gateway-deployment.yaml.

Why this matters

Issue #8817 pointed at the concrete gap: helm template eg ... --set commonLabels.custom-label=custom-value renders ClusterRole eg-gateway-helm-envoy-gateway-role and ClusterRoleBinding eg-gateway-helm-envoy-gateway-rolebinding with custom-label: null. The cause is mechanical - those resources just didn't declare a labels: block, so the commonLabels merged in by eg.labels (at _helpers.tpl:43) never reached them. The cert-gen RBAC and the rest of the chart already have the labels block, so this PR just closes the last gap.

Changes

  • charts/gateway-helm/templates/envoy-gateway-rbac.yaml: add
    labels:
    {{- include "eg.labels" $ | nindent 4 }}
    on each of the four RBAC manifests (Role + RoleBinding inside the watched-namespaces range, ClusterRole + ClusterRoleBinding at the template root, and the two outer ClusterRole/ClusterRoleBinding in the {{ else }} branch). $ vs . matches the existing template-root scoping rules in the same file.

Testing

Local render:

$ helm dependency update charts/gateway-helm
$ envsubst < charts/gateway-helm/values.tmpl.yaml > charts/gateway-helm/values.yaml
$ helm template eg charts/gateway-helm \
    --set commonLabels.custom-label=custom-value | \
    yq eval-all '{"kind": .kind, "name": .metadata.name, "custom-label": .metadata.labels."custom-label"}'

After this change, all four envoy-gateway RBAC resources (plus the cert-gen set that was already wired) render custom-label: custom-value:

ClusterRole         eg-gateway-helm-envoy-gateway-role        custom-label=custom-value
ClusterRoleBinding  eg-gateway-helm-envoy-gateway-rolebinding custom-label=custom-value
Role                eg-gateway-helm-infra-manager             custom-label=custom-value
Role                eg-gateway-helm-leader-election-role      custom-label=custom-value
RoleBinding         eg-gateway-helm-infra-manager             custom-label=custom-value
RoleBinding         eg-gateway-helm-leader-election-rolebinding custom-label=custom-value

Matches the expected output in the issue's repro block.

Fixes #8817

This contribution was developed with AI assistance (Claude Code).

Issue envoyproxy#8817 reported that 'helm template ... --set
commonLabels.custom-label=custom-value' left ClusterRole,
ClusterRoleBinding, Role, and RoleBinding resources unlabelled.
The other resources in the chart already include 'eg.labels' in
their metadata - which picks up 'commonLabels' via the helper
at _helpers.tpl:43 - but envoy-gateway-rbac.yaml didn't set any
labels block.

Add 'labels: {{- include "eg.labels" . | nindent 4 }}' on every
Role / RoleBinding / ClusterRole / ClusterRoleBinding declared in
envoy-gateway-rbac.yaml. Matches the existing labels pattern used
in certgen-rbac.yaml and envoy-gateway-deployment.yaml. Scopes are
'$' inside the watched-namespaces 'range' and '.' at the template
root, same rule the helper block inside the file already used.

Verified locally with:
  helm dependency update charts/gateway-helm
  envsubst < charts/gateway-helm/values.tmpl.yaml > \
    charts/gateway-helm/values.yaml
  helm template eg charts/gateway-helm \
    --set commonLabels.custom-label=custom-value | yq ...

All four RBAC resources now emit 'custom-label: custom-value' in
their metadata.labels, matching the issue's repro steps. Cert-gen
RBAC resources already carried it; this PR brings the core
envoy-gateway RBAC set into parity.

Fixes envoyproxy#8817
@mvanhorn mvanhorn requested a review from a team as a code owner April 22, 2026 08:14
@netlify
Copy link
Copy Markdown

netlify Bot commented Apr 22, 2026

Deploy Preview for cerulean-figolla-1f9435 canceled.

Name Link
🔨 Latest commit 29270b3
🔍 Latest deploy log https://app.netlify.com/projects/cerulean-figolla-1f9435/deploys/69e88366e9024c00084fd701

@zhaohuabing
Copy link
Copy Markdown
Member

Hi @mvanhorn Can you sign this commit?
You'll also need to run make gen-check and submit the generated files.

@zhaohuabing
Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector
Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Can't wait for the next one!

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 22, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 73.70%. Comparing base (00d0895) to head (29270b3).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8818      +/-   ##
==========================================
+ Coverage   73.62%   73.70%   +0.07%     
==========================================
  Files         245      245              
  Lines       48805    48805              
==========================================
+ Hits        35935    35970      +35     
+ Misses      10866    10834      -32     
+ Partials     2004     2001       -3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

commonLabels helm chart value does not label all resources

2 participants