feat: add support for extraEnvFrom to main, worker and webhook #241
Conversation
WalkthroughAdds an optional extraEnvFrom array to main, worker, and webhook in values.yaml and conditionally renders them into the envFrom sections of the corresponding Deployment templates. Bumps chart version to 1.0.15 and updates the Artifact Hub changes annotation. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User (values.yaml)
participant Helm as Helm Template Engine
participant K8s as Kubernetes API
rect rgba(230,245,255,0.5)
note over User,Helm: Install/Upgrade with values
User->>Helm: Provide .Values.main/worker/webhook.extraEnvFrom
Helm->>Helm: Render deployment templates (main, worker, webhook)
alt extraEnvFrom defined
Helm->>Helm: Append extraEnvFrom entries to each container envFrom
else not defined
Helm->>Helm: Render existing envFrom only
end
end
Helm->>K8s: Apply Deployments
K8s-->>User: Pods with merged envFrom sources
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested reviewers
✨ 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. 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: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
charts/n8n/templates/deployment.yaml (1)
67-79: Guard envFrom to avoid emitting an empty/null list.envFrom is always rendered, but all inner branches can be false, yielding an empty/null value and invalid manifest. Wrap the whole block in a conditional.
Apply this diff:
- envFrom: - {{- if .Values.main.config }} - - configMapRef: - name: {{ include "n8n.fullname" . }}-app-config - {{- end }} - {{- if .Values.main.secret }} - - secretRef: - name: {{ include "n8n.fullname" . }}-app-secret - {{- end }} - {{- if .Values.main.extraEnvFrom }} - {{- toYaml .Values.main.extraEnvFrom | nindent 12 }} - {{- end }} + {{- if or .Values.main.config .Values.main.secret .Values.main.extraEnvFrom }} + envFrom: + {{- if .Values.main.config }} + - configMapRef: + name: {{ include "n8n.fullname" . }}-app-config + {{- end }} + {{- if .Values.main.secret }} + - secretRef: + name: {{ include "n8n.fullname" . }}-app-secret + {{- end }} + {{- if .Values.main.extraEnvFrom }} + {{- toYaml .Values.main.extraEnvFrom | nindent 12 }} + {{- end }} + {{- end }}charts/n8n/templates/deployment.webhook.yaml (1)
87-106: Same guard needed here to prevent empty envFrom.If none of the referenced maps/secrets/extraEnvFrom are set, this renders an empty envFrom. Wrap the entire section.
Apply this diff:
- envFrom: + {{- if or .Values.main.config .Values.main.secret .Values.webhook.config .Values.webhook.secret .Values.webhook.extraEnvFrom }} + envFrom: {{- if .Values.main.config }} - configMapRef: name: {{ include "n8n.fullname" . }}-app-config {{- end }} {{- 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 }} - secretRef: name: {{ include "n8n.fullname" . }}-webhook-secret {{- end }} {{- if .Values.webhook.extraEnvFrom }} {{- toYaml .Values.webhook.extraEnvFrom | nindent 12 }} {{- end }} + {{- end }}charts/n8n/templates/deployment.worker.yaml (1)
62-82: Guard worker envFrom to avoid emitting an empty/null list.Same issue as main/webhook. Wrap envFrom with a combined conditional.
Apply this diff:
- envFrom: + {{- if or .Values.main.config .Values.main.secret .Values.worker.config .Values.worker.secret .Values.worker.extraEnvFrom }} + envFrom: {{- if .Values.main.config }} - configMapRef: name: {{ include "n8n.fullname" . }}-app-config {{- end }} {{- if .Values.main.secret }} - secretRef: name: {{ include "n8n.fullname" . }}-app-secret {{- end }} {{- if .Values.worker.config }} - configMapRef: name: {{ include "n8n.fullname" . }}-worker-config {{- end }} {{- if .Values.worker.secret }} - secretRef: name: {{ include "n8n.fullname" . }}-worker-secret {{- end }} {{- if .Values.worker.extraEnvFrom }} {{- toYaml .Values.worker.extraEnvFrom | nindent 12 }} {{- end }} + {{- end }}
🧹 Nitpick comments (2)
charts/n8n/Chart.yaml (1)
3-3: Consider using "added" change kind and a minor version bump for a new feature.
- Artifact Hub "changes" kind could be "added" rather than "changed" for the new extraEnvFrom capability.
- Since this is a backward-compatible feature, consider bumping the chart minor version (e.g., 1.1.0) instead of a patch. Non-blocking if your project’s policy treats this as patch-level.
Also applies to: 38-38
charts/n8n/values.yaml (1)
86-90: LGTM; add a brief note about the optional prefix field.The new extraEnvFrom defaults and examples look good. For completeness, consider documenting that each item can also include a top-level prefix to namespace imported vars, e.g.:
- configMapRef: { name: my-config }
prefix: MY_This helps avoid collisions when sourcing from multiple refs.
Also applies to: 296-300, 480-484
📜 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)
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/values.yaml(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-09-02T08:05:39.337Z
Learnt from: tsch157
PR: 8gears/n8n-helm-chart#239
File: charts/n8n/Chart.yaml:3-4
Timestamp: 2025-09-02T08:05:39.337Z
Learning: n8n 1.110.0 is a pre-release version and should not be used in production deployments. Always verify if a version is stable before recommending upgrades in the n8n Helm chart.
Applied to files:
charts/n8n/Chart.yaml
|
Testing this today. Thank you! |
There was a problem hiding this comment.
Really nice stuff @jcconnects
Tested locally and everything works as expected 👍🏻
Please remove lines that are just whitespace to ensure clean manifest generation, otherwise LGTM
|
Hey @PhillypHenning , thanks for the catch on the whitespaces! I have just fixed it 🤗 |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
charts/n8n/templates/deployment.webhook.yaml (1)
104-106: Enable templating in extraEnvFrom for parity and flexibility.Use tpl around toYaml so values can reference release/name/etc., matching how initContainers are handled.
- {{- toYaml .Values.webhook.extraEnvFrom | nindent 12 }} + {{- tpl (toYaml .Values.webhook.extraEnvFrom) . | nindent 12 }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
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 (2)
- charts/n8n/templates/deployment.yaml
- charts/n8n/templates/deployment.worker.yaml
🔇 Additional comments (1)
charts/n8n/templates/deployment.webhook.yaml (1)
87-106: Confirm extraEnvFrom envFrom ordering in webhook deployment
extraEnvFrom is appended after built-in configMapRef/secretRef entries—later envFrom entries win on duplicate keys. This placement matches main and worker templates; verify this ordering aligns with your intended override behavior.
|
Happy to have found this. As a small note, perhaps it would make sense to mention somewhere that any variables defined in
|
Hey @ocraviotto , that is a nice information I had not noticed before! Where do you suggest to insert it? Feel free to add another commit to this PR. 😊 |
4 participants
Description
This pull request introduces support for
extraEnvFromto themain,worker, andwebhookdeployments, as requested in issue #139.This feature allows for more flexible and secure configuration by enabling the injection of environment variables from external
ConfigMapsandSecretsdirectly into the pods. This is particularly useful for managing sensitive information like credentials or API keys without hardcoding them into thevalues.yamlfile.The implementation is consistent with existing patterns in the chart and adds the
extraEnvFromparameter under themain,worker, andwebhooksections invalues.yaml.How to Use
To use this feature, you can now specify
ConfigMaporSecretreferences in yourvalues.yamlfile as follows:Changes Included
values.yaml: Added a newextraEnvFrom: []field to themain,worker, andwebhooksections.templates/deployment.yaml: Updated the main deployment to include theenvFromblock if.Values.main.extraEnvFromis set.templates/deployment.worker.yaml: Updated the worker deployment to include theenvFromblock.templates/deployment.webhook.yaml: Updated the webhook deployment to include theenvFromblock.Chart.yaml: Bumped the chart version to1.0.15to reflect the new feature.Testing
I have tested this feature locally with two scenarios to ensure correctness and robustness.
extraEnvFrom(e.g.,error:instead ofsecretRef:). The deployment correctly failed with a validation error, confirming that Kubernetes' native validation is enforced through the chart.helm installusing atest-values.yamlfile that configured both aConfigMapand aSecretforextraEnvFrom. The installation was successful, and I verified withkubectl get pod -o yamlthat theenvFromsections were correctly added to the runningn8nandn8n-workerpods.This PR resolves #139.
Summary by CodeRabbit