-
Notifications
You must be signed in to change notification settings - Fork 116
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
feat: Add environment settings to worker deployment #143
feat: Add environment settings to worker deployment #143
Conversation
WalkthroughThis PR updates the n8n Helm chart by bumping its version and modifying the annotation description in the Chart.yaml to emphasize environment settings for the worker deployment. It adds new template files for conditionally creating a ConfigMap and a Secret for the worker, adjusts the worker deployment template to source environment variables and secrets conditionally, and extends the values file with new options for worker configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant Helm as "Helm CLI"
participant Template as "Helm Template Engine"
participant K8s as "Kubernetes API Server"
participant Worker as "Worker Container"
Helm->>Template: Load values.yaml and Chart templates
alt worker.config is defined
Template->>K8s: Render and deploy ConfigMap for worker
end
alt worker.secret is defined
Template->>K8s: Render and deploy Secret for worker
end
Template->>K8s: Render Deployment with envFrom and extraEnv settings
K8s->>Worker: Start Worker container with configured environment variables
Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (4)
charts/n8n/templates/configmap.worker.yaml (1)
1-11
: Helm Templating & YAMLlint Note:
The conditional block for generating the ConfigMap based on.Values.worker.config
is correctly structured using Helm templating. The YAMLlint error on line 1 (regarding the expected node content before the dash) is a known false positive when linting templates. Consider adding a YAMLlint ignore comment (or configuring your linter) for Helm template files so that such false errors are suppressed.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/secret.worker.yaml (1)
1-13
: Helm Templating & Secret Resource Creation:
This new file correctly defines a Secret that is conditionally created when.Values.worker.secret
is provided. Similar to the ConfigMap file, the templating syntax may trigger YAML lint warnings at the very start; these can be safely ignored or suppressed via linter configuration. The use of thetoEnvVars
function with the"isSecret": true
flag is appropriate for handling sensitive data.🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/deployment.worker.yaml (1)
72-76
: Handling Additional Environment Variables (extraEnv
):
The addition of theenv
block to process.Values.worker.extraEnv
is a useful feature. However, the use of the expressionenv: {{ not (empty .Values.worker.extraEnv) | ternary nil "[]" }}
may be slightly nonintuitive. In this context, if
extraEnv
is not empty, the block outputsnil
(thus allowing the subsequentrange
to append the proper entries), and if it is empty, outputs an empty list. It might help future maintainers if an inline comment explains this logic. Please verify that this behavior is as intended.charts/n8n/values.yaml (1)
263-274
: New Worker Configuration Options:
The addition of theconfig
,secret
, andextraEnv
keys within theworker
section is well structured and clearly laid out. These new entries allow users to specify additional environment variables and sensitive settings for the worker component. Please ensure that the accompanying documentation (e.g., README or values documentation) is updated to describe these new options.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
charts/n8n/Chart.yaml
(2 hunks)charts/n8n/templates/configmap.worker.yaml
(1 hunks)charts/n8n/templates/deployment.worker.yaml
(1 hunks)charts/n8n/templates/secret.worker.yaml
(1 hunks)charts/n8n/values.yaml
(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
charts/n8n/templates/secret.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/configmap.worker.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: lint-test
🔇 Additional comments (3)
charts/n8n/Chart.yaml (2)
3-3
: Helm Chart Version Update:
The Helm chart version has been updated to1.1.0
, which aligns with the new features introduced in this PR. Ensure that all related documentation (e.g., CHANGELOG or release notes) is updated accordingly.
36-38
: Annotation Update for Worker Deployment:
The annotation description is updated to "Add environment settings to worker deployment". This revision clearly communicates the focus of the PR. Double-check that this updated description is also reflected in any external documentation or automated release checks.charts/n8n/templates/deployment.worker.yaml (1)
55-63
: Conditional Inclusion of Environment Sources:
The newly introducedenvFrom
block conditionally includes references for both main and worker configuration (viaconfigMapRef
andsecretRef
). This enhances the flexibility of the worker deployment by allowing it to source environment variables from multiple resources. The conditionals for.Values.main.config
,.Values.main.secret
,.Values.worker.config
, and.Values.worker.secret
are clear and seem appropriately placed.
thank you, as you guessed, the worker, webhook aren't yet fully tested or implemented. |
No problem, can we release this? If so, how should it be done? I think contribution bullet point lacks some information |
Would be nice to be able to set N8N_ENCRYPTION_KEY and other settings in worker as right now it is not possible and not possible to run workers at all, this could fix the issue
Summary by CodeRabbit
New Features
Chores