feat: support envFrom in values file #192
Conversation
WalkthroughAdds a new values option Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Helm
participant K8sAPI
participant Pod
rect rgba(50,115,220,0.06)
User->>Helm: Add extraEnvFrom to values (main/worker/webhook)
Helm->>Helm: Render templates, include extraEnvFrom under container envFrom
end
rect rgba(60,179,113,0.06)
Helm->>K8sAPI: Apply rendered manifests (Deployment + Service)
K8sAPI->>Pod: Create Pod spec with envFrom references
Pod->>Pod: Load env vars from referenced ConfigMap/Secret
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Files/areas to pay extra attention:
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
charts/n8n/values.yaml (1)
71-76: Add commented examples forenvFromin worker and webhook sections
Themainsection includes example commented entries illustrating how to specifyconfigMapRefandsecretRef. To maintain consistency and aid users, consider adding similar examples under theworker.envFromandwebhook.envFromdefinitions.Also applies to: 283-285, 469-471
README.md (1)
170-176: Consider renamingenvFromtoextraEnvFromfor naming consistency
The chart already usesextraEnvfor individual variables; renamingenvFromtoextraEnvFromwould align naming conventions and clarify its relation to the existing key. If agreed, update all templates, values, and documentation accordingly.Also applies to: 382-384, 568-570
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
README.md(3 hunks)charts/n8n/templates/deployment.webhook.yaml(1 hunks)charts/n8n/templates/deployment.worker.yaml(1 hunks)charts/n8n/templates/deployment.yaml(1 hunks)charts/n8n/values.yaml(3 hunks)
🔇 Additional comments (2)
charts/n8n/templates/deployment.worker.yaml (1)
66-68: Indentation fixes forsecretRefentries look correct
The adjusted indentation aligns thesecretRefblocks underenvFromconsistently.Also applies to: 74-76
charts/n8n/templates/deployment.webhook.yaml (1)
91-94: Indentation fixes for webhooksecretRefentries look correct
The indentation for bothmain.secretandwebhook.secretblocks underenvFromhas been normalized properly.Also applies to: 99-102
aabouzaid
force-pushed
the
aa-support-envfrom
branch
2 times, most recently
from
117bc2e to
b32f423
Compare
aabouzaid
force-pushed
the
aa-support-envfrom
branch
from
b32f423 to
891fb80
Compare
|
@aabouzaid why aren't you using what already exists in extraEnv? extraEnv:
# N8N_DB_POSTGRESDB_NAME:
# valueFrom:
# secretKeyRef:
# name: db-app
# key: dbnameWhat is your use case, and how do you get to those configmaps and secret that allows you to import them 1:1 into n8n. |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/n8n/Chart.yaml (1)
36-38: Annotation wording drifts from implementation; considerextraEnvFrom+ add missing EOF newline
- The feature landed as
extraEnvFrom, yet the change-log line still says “support envFrom”. This might confuse users scanning Artifact Hub.- YAML-lint flags a missing trailing newline; some CI pipelines treat this as an error.
Proposed fix:
artifacthub.io/changes: | - - kind: added - description: "support envFrom in values file" + - kind: added + description: "support extraEnvFrom in values file" +(the blank line at the end satisfies the EOF newline)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
README.md(3 hunks)charts/n8n/Chart.yaml(2 hunks)charts/n8n/values.yaml(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- README.md
- charts/n8n/values.yaml
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/n8n/Chart.yaml
[error] 38-38: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (1)
charts/n8n/Chart.yaml (1)
3-3: Chart version bump LGTM – confirm lock‐file & dependency syncThe SemVer-correct minor bump to
1.1.0is appropriate for a new feature.
Please double-check thatChart.lock,README, and any release notes are updated accordingly so that consumers pulling via Helm repos don’t encounter checksum mismatches.
|
@Vad1mo The secrets are generated using the External Secrets Operator and the ConfigMaps are generated using Kustomize. For example, the secrets are updated automatically (another operator restarts the N8n deployment when secrets are changed). So, adding them to the values file needs to specify each secret one by one, which is not easy to automate. |
|
@Vad1mo Anything left to merge this PR, please? |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
charts/n8n/templates/deployment.webhook.yaml (2)
91-94: Confirm intent: webhook also imports main app-secretThe webhook container pulls env vars from the shared main secret (
{{ include "n8n.fullname" . }}-app-secret). If this is intentional to inherit base config, great. If not, this can lead to unexpected key overlaps with the webhook-specific secret further below. Please confirm and, if intentional, consider a short note in README to document the inheritance order.
103-105: Good addition of extraEnvFrom; wrap the whole envFrom block to avoid null/empty emissionNice:
.Values.webhook.extraEnvFromis correctly gated and indented. However, when none of the sources are set (no main.* or webhook.* config/secret and empty extraEnvFrom), Helm will still emit a bareenvFrom:with no list items, which can deserialize to null. It’s safer to guard the entireenvFromsection so it’s omitted when empty.Proposed diff:
- envFrom: - {{- if .Values.main.config }} + {{- if or .Values.main.config .Values.main.secret .Values.webhook.config .Values.webhook.secret (not (empty .Values.webhook.extraEnvFrom)) }} + envFrom: + {{- if .Values.main.config }} - configMapRef: name: {{ include "n8n.fullname" . }}-app-config {{- end }} - {{- if .Values.main.secret }} + {{- if .Values.main.secret }} - secretRef: name: {{ include "n8n.fullname" . }}-app-secret {{- end }} {{- if .Values.webhook.config }} - configMapRef: name: {{ include "n8n.fullname" . }}-webhook-config {{- end }} - {{- if .Values.webhook.secret }} + {{- if .Values.webhook.secret }} - secretRef: name: {{ include "n8n.fullname" . }}-webhook-secret {{- end }} - {{- if .Values.webhook.extraEnvFrom }} - {{- toYaml .Values.webhook.extraEnvFrom | nindent 12 }} - {{- end }} + {{- if .Values.webhook.extraEnvFrom }} + {{- toYaml .Values.webhook.extraEnvFrom | nindent 12 }} + {{- end }} + {{- end }}Notes:
- Adds a single guard that checks every possible source (including a non-empty
extraEnvFrom) before emittingenvFrom:.- Keeps your existing order intact (shared → webhook-specific → user-supplied
extraEnvFrom).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (5)
README.md(3 hunks)charts/n8n/Chart.yaml(2 hunks)charts/n8n/templates/deployment.webhook.yaml(1 hunks)charts/n8n/templates/deployment.worker.yaml(1 hunks)charts/n8n/templates/deployment.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- README.md
- charts/n8n/templates/deployment.yaml
- charts/n8n/Chart.yaml
- charts/n8n/templates/deployment.worker.yaml
🔇 Additional comments (1)
charts/n8n/templates/deployment.webhook.yaml (1)
99-102: Precedence looks correct when the same key exists in multiple sourcesKubernetes applies envFrom sources in order; later entries override earlier ones. Having
webhook-secretafter the shared secrets/configs ensures webhook-specific values take precedence. No action needed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/n8n/templates/deployment.webhook.yaml (1)
104-106: Consider guarding explicitly against empty lists for robustness.The conditional
{{- if .Values.webhook.extraEnvFrom }}relies on Go template's falsy semantics for empty slices. While empty lists are typically falsy, this approach could produce invalid YAML if the value somehow evaluates unexpectedly. Since other conditional blocks in this file follow the same pattern, consistency is maintained, but consider an explicit check for a non-empty list.For defensive coding, consider updating the guard to:
- {{- if .Values.webhook.extraEnvFrom }} + {{- if and .Values.webhook.extraEnvFrom (gt (len .Values.webhook.extraEnvFrom) 0) }} {{- toYaml .Values.webhook.extraEnvFrom | nindent 12 }} {{- end }}Alternatively, if the values schema enforces non-empty lists or documents the expected structure clearly, the current implementation is acceptable. Verify that
extraEnvFromitems are properly formatted as a list ofconfigMapReforsecretRefobjects matching Kubernetes envFrom syntax.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
README.md(3 hunks)charts/n8n/Chart.yaml(2 hunks)charts/n8n/templates/deployment.webhook.yaml(1 hunks)charts/n8n/templates/deployment.worker.yaml(1 hunks)charts/n8n/templates/deployment.yaml(1 hunks)charts/n8n/templates/service.yaml(0 hunks)charts/n8n/values.yaml(3 hunks)
💤 Files with no reviewable changes (1)
- charts/n8n/templates/service.yaml
🚧 Files skipped from review as they are similar to previous changes (5)
- README.md
- charts/n8n/templates/deployment.worker.yaml
- charts/n8n/templates/deployment.yaml
- charts/n8n/Chart.yaml
- charts/n8n/values.yaml
|
@Vad1mo Any chance to merge this PR? |
2 participants
Hi 👋
As a user, I'd like to supply all env vars from an external ConfigMap or Secret (generated by another tool, not Helm).
So I've introduced
envFrom, which could reference any number of ConfigMaps or Secrets.One thing I'm not sure about is that the key could be
extraEnvFromfollowingextraEnv.I can make the change once we have agreed on the name.
Summary by CodeRabbit
New Features
Documentation
Chores