codeant-ai-for-open-source[bot] commented on code in PR #39350:
URL: https://github.com/apache/superset/pull/39350#discussion_r3465734744
##########
helm/superset/templates/_helpers.tpl:
##########
@@ -61,6 +61,50 @@ Create chart name and version as used by the chart label.
{{- printf "%s-%s" .Chart.Name .Chart.Version | replace "+" "_" | trunc 63 |
trimSuffix "-" -}}
{{- end -}}
+{{/*
+Common labels for all resources - follows Kubernetes recommended labels
+https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/
+*/}}
+{{- define "superset.labels" -}}
+helm.sh/chart: {{ include "superset.chart" . }}
+{{ include "superset.selectorLabels" . }}
+{{- if .Chart.AppVersion }}
+app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
+{{- end }}
+app.kubernetes.io/managed-by: {{ .Release.Service }}
+app.kubernetes.io/part-of: superset
+{{- if .Values.extraLabels }}
+{{ toYaml .Values.extraLabels }}
+{{- end }}
+{{- end -}}
+
+{{/*
+Selector labels - used by selectors and matchLabels
+*/}}
+{{- define "superset.selectorLabels" -}}
+app.kubernetes.io/name: {{ include "superset.name" . }}
+app.kubernetes.io/instance: {{ .Release.Name }}
+{{- end -}}
+
+{{/*
+Component labels - extends superset.labels with component-specific labels
+Usage: {{ include "superset.componentLabels" (dict "component" "web" "root" .)
}}
+*/}}
+{{- define "superset.componentLabels" -}}
+{{ include "superset.labels" .root }}
+app.kubernetes.io/component: {{ .component }}
+{{- end -}}
+
+{{/*
+Component selector labels - for matchLabels with component
+Usage: {{ include "superset.componentSelectorLabels" (dict "component" "web"
"root" .) }}
+*/}}
+{{- define "superset.componentSelectorLabels" -}}
+app.kubernetes.io/name: {{ include "superset.name" .root }}
+app.kubernetes.io/instance: {{ .Release.Name }}
Review Comment:
**Suggestion:** `superset.componentSelectorLabels` is documented to be
called with a dict containing `root`, but it reads `.Release.Name` from the
dict scope instead of `.root.Release.Name`. When this helper is used, the
`app.kubernetes.io/instance` value will render as empty/`<no value>`, producing
invalid or mismatched selector labels. Read the release name from `root` to
keep selectors consistent. [incorrect variable usage]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ⚠️ Future component selectors render instance label as `<no value>`.
- ⚠️ Potential mismatch between selector labels and pod metadata labels.
- ⚠️ Breaks workload Service/Deployment matching when helper is adopted.
```
</details>
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open `helm/superset/templates/_helpers.tpl` and observe the documented
usage for
`superset.componentSelectorLabels` at lines 98–101: `Usage: {{ include
"superset.componentSelectorLabels" (dict "component" "web" "root" .) }}`
which shows that
the helper is intended to be called with a dict whose keys are `component`
and `root`, and
where `.` inside the helper is that dict, not the original root context.
2. At lines 102–105 in the same file, inspect the helper definition: `{{-
define
"superset.componentSelectorLabels" -}}` (line 102), `app.kubernetes.io/name:
{{ include
"superset.name" .root }}` (line 103), `app.kubernetes.io/instance: {{
.Release.Name }}`
(line 104), and `app.kubernetes.io/component: {{ .component }}` (line 105).
Note that
`.root` is used explicitly for the name, but `.Release.Name` is read
directly from `.`.
3. In Helm's templating semantics (Go `text/template`), when a helper is
called as
`include "superset.componentSelectorLabels" (dict "component" "web" "root"
.)`, the dot
`.` inside the helper is the dict `{"component": "web", "root": <original
root>}`. This
dict has no `Release` field, so the lookup `.Release.Name` at line 104
cannot resolve and
will render as `<no value>` in the generated YAML.
4. Once any Deployment/Service/Job template uses this helper exactly as
documented in the
comment at lines 98–101 for `spec.selector.matchLabels` or similar, running
`helm
template` on the chart will produce labels where
`app.kubernetes.io/instance` coming from
`superset.componentSelectorLabels` is `<no value>`, while other labels using
`superset.selectorLabels` (lines 84–87) correctly render `.Release.Name`,
resulting in
inconsistent/mismatched selector labels for that component.
```
</details>
[](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=538be8063f4b413796f3839f187592ae&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
[](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=538be8063f4b413796f3839f187592ae&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
*(Use Cmd/Ctrl + Click for best experience)*
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** helm/superset/templates/_helpers.tpl
**Line:** 104:104
**Comment:**
*Incorrect Variable Usage: `superset.componentSelectorLabels` is
documented to be called with a dict containing `root`, but it reads
`.Release.Name` from the dict scope instead of `.root.Release.Name`. When this
helper is used, the `app.kubernetes.io/instance` value will render as
empty/`<no value>`, producing invalid or mismatched selector labels. Read the
release name from `root` to keep selectors consistent.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
Once fix is implemented, also check other comments on the same PR, and ask
user if the user wants to fix the rest of the comments as well. if said yes,
then fetch all the comments validate the correctness and implement a minimal fix
```
</details>
<a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39350&comment_hash=4324af87127792de21cab1836e5e2fe81ea4dc28dc9a88f3f766244c6c981c5e&reaction=like'>👍</a>
| <a
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39350&comment_hash=4324af87127792de21cab1836e5e2fe81ea4dc28dc9a88f3f766244c6c981c5e&reaction=dislike'>👎</a>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
