feat(helm): add commonLabels support to helm chart#8078
feat(helm): add commonLabels support to helm chart#8078arkodg merged 2 commits intoenvoyproxy:mainfrom
Conversation
✅ Deploy Preview for cerulean-figolla-1f9435 ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
51198a4 to
26472bd
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8078 +/- ##
==========================================
- Coverage 73.80% 73.79% -0.01%
==========================================
Files 241 241
Lines 36579 36579
==========================================
- Hits 26997 26994 -3
- Misses 7677 7679 +2
- Partials 1905 1906 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
26472bd to
73d7a3e
Compare
73d7a3e to
97f3f17
Compare
| app.kubernetes.io/version: {{ .Chart.AppVersion | quote }} | ||
| {{- end }} | ||
| app.kubernetes.io/managed-by: {{ .Release.Service }} | ||
| {{- with .Values.commonLabels }} |
There was a problem hiding this comment.
what would happen if there're same keys in eg.selectorLabels and commonLabels?
can you add a test case under test/helm/gatewa-helm?
There was a problem hiding this comment.
If the same keys are present in eg.selectorLabels and commonLabels, then the commonLabels value would override. This is an expected behaviour. The same pattern is followed in several bitnami charts and CNCF projects.
I've added the test for this change
There was a problem hiding this comment.
can you make CI happy?
rebase with main branch, run make gen-check and commit the changes.
97f3f17 to
25604bc
Compare
d05673f to
2ad80f8
Compare
There was a problem hiding this comment.
can you improve these test base on https://github.com/envoyproxy/gateway/pull/8078/changes#r2742524657?
add comment for the override logic.
There was a problem hiding this comment.
@zirain, I have added the selectorLabel which will override the default one in the testcase. Just to be clear, in the generated YAML, you will see both the labels - the default one and the one added in commonLabels. When the YAML parser processes the file, it makes sure to consider the value in commonLabels.
There was a problem hiding this comment.
it there any tpl in helm to avoid this? or let's comment this behvior in the common-labels.in.yaml
There was a problem hiding this comment.
I've commented this behaviour in common-labels.in.yaml clearly now. There are ways in which we can avoid the duplicates. But it is not generally not a pattern followed well known helm-charts, as it makes the _helpers.tpl complex.
There was a problem hiding this comment.
Thanks, will merge this after 1.7.0 released to make RM's work easier.
7dd7539 to
48663f2
Compare
Signed-off-by: Gagan H R <ghr@guidewire.com>
48663f2 to
f16728c
Compare
feat: adds support for commonlabels Signed-off-by: Gagan H R <ghr@guidewire.com>
What type of PR is this?
feat(helm): add commonLabels support to gateway-helm chart
What this PR does / why we need it:
Adds support for
commonLabelsin the gateway-helm chart, allowing users to apply custom labels to all resources managed by the chart. This is useful for:The
commonLabelsfield is now available at the root level ofvalues.yamland is added to Kubernetes resources through theeg.labelshelper template.Which issue(s) this PR fixes:
Fixes #7913