Re: [PR] fix(helm): pass chart-wide env into the built-in init containers [superset]

Sat, 15 Aug 2026 01:47:20 -0700 


codeant-ai-for-open-source[bot] commented on code in PR #43214:
URL: https://github.com/apache/superset/pull/43214#discussion_r3788990326


##########
helm/superset/templates/_helpers.tpl:
##########
@@ -776,3 +776,44 @@ app.kubernetes.io/name: {{ include "superset.name" . }}
 app.kubernetes.io/instance: {{ .Release.Name }}
 app.kubernetes.io/component: mcp
 {{- end }}
+
+{{/*
+Render a list of init containers with the chart-wide environment sources merged
+in, so that init containers resolve credentials exactly like the main
+containers do.
+
+The chart's built-in init containers wait for the metadata database and Redis
+using DB_HOST/DB_PORT/REDIS_HOST/REDIS_PORT. Those variables are only
+guaranteed to be present in the chart-managed secret; when they are supplied
+from a pre-existing secret (`envFromSecrets`) or from individual `secretKeyRef`
+entries (`extraEnvRaw`), the init containers would otherwise keep using the
+chart defaults and never become ready.
+
+`extraEnv`, `extraEnvRaw` and `envFromSecrets` are appended, so a container's
+own `env`/`envFrom` entries are preserved.
+
+Usage:
+  {{- include "superset.initContainers" (dict "containers" 
.Values.x.initContainers "context" $) }}
+*/}}
+{{- define "superset.initContainers" -}}
+{{- $ctx := .context -}}
+{{- range .containers }}
+{{- $container := deepCopy . -}}
+{{- $extraEnv := list -}}
+{{- range $key, $value := $ctx.Values.extraEnv }}
+{{- $extraEnv = append $extraEnv (dict "name" $key "value" ($value | 
toString)) -}}
+{{- end }}
+{{- $extraEnv = concat $extraEnv (default (list) $ctx.Values.extraEnvRaw) -}}

Review Comment:
   **Suggestion:** The helper merges only `extraEnv` and `extraEnvRaw`, but the 
main Superset node container also receives `supersetNode.env`. Consequently, 
setting `supersetNode.env.DB_HOST`, `DB_PORT`, or a Redis variable changes the 
application container while the generated wait container continues using the 
chart-managed defaults and can block startup until timeout. Include the 
applicable component-specific environment values when rendering that 
component's init containers, or do not claim they resolve credentials like the 
main container. [api mismatch]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Superset node startup can time out on the wrong database.
   - ⚠️ Application and wait-container connection settings diverge.
   ```
   </details>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=0e6559b8deb9442291f6f81cd5b966df&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=0e6559b8deb9442291f6f81cd5b966df&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <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:** 803:806
   **Comment:**
        *Api Mismatch: The helper merges only `extraEnv` and `extraEnvRaw`, but 
the main Superset node container also receives `supersetNode.env`. 
Consequently, setting `supersetNode.env.DB_HOST`, `DB_PORT`, or a Redis 
variable changes the application container while the generated wait container 
continues using the chart-managed defaults and can block startup until timeout. 
Include the applicable component-specific environment values when rendering 
that component's init containers, or do not claim they resolve credentials like 
the main container.
   
   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%2F43214&comment_hash=6292b66838b0ca040611c36ffbb5c0de76e6e679ab8822c5dcd26797eafb162b&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43214&comment_hash=6292b66838b0ca040611c36ffbb5c0de76e6e679ab8822c5dcd26797eafb162b&reaction=dislike'>👎</a>



##########
helm/superset/templates/_helpers.tpl:
##########
@@ -776,3 +776,44 @@ app.kubernetes.io/name: {{ include "superset.name" . }}
 app.kubernetes.io/instance: {{ .Release.Name }}
 app.kubernetes.io/component: mcp
 {{- end }}
+
+{{/*
+Render a list of init containers with the chart-wide environment sources merged
+in, so that init containers resolve credentials exactly like the main
+containers do.
+
+The chart's built-in init containers wait for the metadata database and Redis
+using DB_HOST/DB_PORT/REDIS_HOST/REDIS_PORT. Those variables are only
+guaranteed to be present in the chart-managed secret; when they are supplied
+from a pre-existing secret (`envFromSecrets`) or from individual `secretKeyRef`
+entries (`extraEnvRaw`), the init containers would otherwise keep using the
+chart defaults and never become ready.
+
+`extraEnv`, `extraEnvRaw` and `envFromSecrets` are appended, so a container's
+own `env`/`envFrom` entries are preserved.
+
+Usage:
+  {{- include "superset.initContainers" (dict "containers" 
.Values.x.initContainers "context" $) }}
+*/}}
+{{- define "superset.initContainers" -}}
+{{- $ctx := .context -}}
+{{- range .containers }}
+{{- $container := deepCopy . -}}
+{{- $extraEnv := list -}}
+{{- range $key, $value := $ctx.Values.extraEnv }}
+{{- $extraEnv = append $extraEnv (dict "name" $key "value" ($value | 
toString)) -}}
+{{- end }}
+{{- $extraEnv = concat $extraEnv (default (list) $ctx.Values.extraEnvRaw) -}}
+{{- if $extraEnv }}
+{{- $_ := set $container "env" (concat (default (list) $container.env) 
$extraEnv) -}}

Review Comment:
   **Suggestion:** Appending chart-wide variables after a custom init 
container's existing `env` causes a name collision to be rejected by Kubernetes 
when both define the same variable. This is especially likely for custom wait 
containers that define `DB_HOST` or `REDIS_HOST`; unlike the main-container 
configuration, there is no component-specific merge policy or collision 
avoidance here. Merge with an explicit precedence policy or reject/resolve 
duplicate names before rendering. [api mismatch]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Custom wait-container deployments are rejected by Kubernetes.
   - ⚠️ Helm upgrades fail when custom and global environment names overlap.
   ```
   </details>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=b5122f7d46e04bf9b1f868c6ddffedfe&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=b5122f7d46e04bf9b1f868c6ddffedfe&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <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:** 808:808
   **Comment:**
        *Api Mismatch: Appending chart-wide variables after a custom init 
container's existing `env` causes a name collision to be rejected by Kubernetes 
when both define the same variable. This is especially likely for custom wait 
containers that define `DB_HOST` or `REDIS_HOST`; unlike the main-container 
configuration, there is no component-specific merge policy or collision 
avoidance here. Merge with an explicit precedence policy or reject/resolve 
duplicate names before rendering.
   
   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%2F43214&comment_hash=41be8a1def0b44efbde9435a98bc1e20a63021b966b9cf30b8b0554aa5814173&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43214&comment_hash=41be8a1def0b44efbde9435a98bc1e20a63021b966b9cf30b8b0554aa5814173&reaction=dislike'>👎</a>



##########
helm/superset/templates/_helpers.tpl:
##########
@@ -776,3 +776,44 @@ app.kubernetes.io/name: {{ include "superset.name" . }}
 app.kubernetes.io/instance: {{ .Release.Name }}
 app.kubernetes.io/component: mcp
 {{- end }}
+
+{{/*
+Render a list of init containers with the chart-wide environment sources merged
+in, so that init containers resolve credentials exactly like the main
+containers do.
+
+The chart's built-in init containers wait for the metadata database and Redis
+using DB_HOST/DB_PORT/REDIS_HOST/REDIS_PORT. Those variables are only
+guaranteed to be present in the chart-managed secret; when they are supplied
+from a pre-existing secret (`envFromSecrets`) or from individual `secretKeyRef`
+entries (`extraEnvRaw`), the init containers would otherwise keep using the
+chart defaults and never become ready.
+
+`extraEnv`, `extraEnvRaw` and `envFromSecrets` are appended, so a container's
+own `env`/`envFrom` entries are preserved.
+
+Usage:
+  {{- include "superset.initContainers" (dict "containers" 
.Values.x.initContainers "context" $) }}
+*/}}
+{{- define "superset.initContainers" -}}
+{{- $ctx := .context -}}
+{{- range .containers }}
+{{- $container := deepCopy . -}}
+{{- $extraEnv := list -}}
+{{- range $key, $value := $ctx.Values.extraEnv }}
+{{- $extraEnv = append $extraEnv (dict "name" $key "value" ($value | 
toString)) -}}
+{{- end }}
+{{- $extraEnv = concat $extraEnv (default (list) $ctx.Values.extraEnvRaw) -}}
+{{- if $extraEnv }}
+{{- $_ := set $container "env" (concat (default (list) $container.env) 
$extraEnv) -}}
+{{- end }}
+{{- $envFrom := default (list) $container.envFrom -}}
+{{- range $ctx.Values.envFromSecrets }}
+{{- $envFrom = append $envFrom (dict "secretRef" (dict "name" .)) -}}
+{{- end }}
+{{- if $envFrom }}
+{{- $_ := set $container "envFrom" $envFrom -}}
+{{- end }}
+{{ toYaml (list $container) }}
+{{- end }}

Review Comment:
   **Suggestion:** The whitespace trim on the loop terminator removes the 
newline emitted by `toYaml`; when more than one init container is configured, 
the next serialized container is concatenated directly to the previous YAML 
instead of starting on a new line, producing malformed YAML. Preserve the 
separator between iterations or render the complete list in one `toYaml` call. 
[logic error]
   
   <details>
   <summary><b>Severity Level:</b> Major ⚠️</summary>
   
   ```mdx
   - ❌ Deployments with multiple built-in init containers fail manifest parsing.
   - ❌ Helm upgrades are blocked for affected component configurations.
   ```
   </details>
   
   [![Fix in 
Cursor](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-cursor-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=2d6ff8cca58842569a39aab782400056&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
 [![Fix in VSCode 
Claude](https://new-codeant-butcket.s3.us-west-1.amazonaws.com/badges/fix-in-vscode-claude-flat.svg)](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=2d6ff8cca58842569a39aab782400056&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset)
   <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:** 817:818
   **Comment:**
        *Logic Error: The whitespace trim on the loop terminator removes the 
newline emitted by `toYaml`; when more than one init container is configured, 
the next serialized container is concatenated directly to the previous YAML 
instead of starting on a new line, producing malformed YAML. Preserve the 
separator between iterations or render the complete list in one `toYaml` call.
   
   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%2F43214&comment_hash=a3c0d0a6a8f7dc3e5d78c5b7bb71f230aa7c89b4499eb51f01bec4a378819b4e&reaction=like'>👍</a>
 | <a 
href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F43214&comment_hash=a3c0d0a6a8f7dc3e5d78c5b7bb71f230aa7c89b4499eb51f01bec4a378819b4e&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]

Reply via email to