feat: support envFrom in values file #192
Conversation
WalkthroughAdds a new 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 envFrom[]
end
rect rgba(60,179,113,0.06)
Helm->>K8sAPI: Apply rendered manifests (Deployment/Service)
K8sAPI->>Pod: Create Pod spec with envFrom refs
Pod->>Pod: Load env vars from referenced ConfigMap/Secret
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
Possibly related PRs
Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
![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.
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