support mcp #196
support mcp #196
Conversation
|
""" WalkthroughThis change introduces a new "mcp" (Managed Cloud Process) component to the n8n Helm chart. It adds full Kubernetes manifest templates (Deployment, Service, ConfigMap, Secret), values configuration, and documentation for deploying and managing the MCP component, alongside updates to ingress routing and example values. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Ingress
participant MCP Service
participant MCP Pod
User->>Ingress: HTTP request to /mcp/...
Ingress->>MCP Service: Forwards request (servicePort)
MCP Service->>MCP Pod: Routes traffic to MCP container
MCP Pod-->>User: Handles request and responds
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![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
🧹 Nitpick comments (1)
charts/n8n/templates/ingress.yaml (1)
70-76: Consider documenting and clarifying the new MCP path. You've added themcp/route under webhook-enabled paths, which is correct. Please:
- Add an inline comment in this template to describe the purpose of the
mcp/path.- Update
values.yaml(or the chart’s README) to document the new endpoint and ensure users know to enablewebhook.enabledfor it.- Verify that gating by
.Values.webhook.enabledaligns with intended behavior (i.e., MCP traffic should only be routed when webhooks are enabled).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
charts/n8n/Chart.yaml(1 hunks)charts/n8n/templates/ingress.yaml(1 hunks)
🔇 Additional comments (1)
charts/n8n/Chart.yaml (1)
3-3: Chart version bump. The chart version has been correctly updated to1.0.8to include the new ingress rule. Please ensure you regenerateChart.lockby runninghelm dependency updatebefore publishing the chart.
|
@pwf all the checks fails, can you fix those? |
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
charts/n8n/templates/service.mcp.yaml (1)
1-3: Fold the nestedifinto a singleandfor readabilityPure style, but matching the proposed pattern elsewhere keeps the chart uniform and one-level deep:
-{{- if .Values.mcp.enabled }} -{{- if .Values.webhook.enabled }} +{{- if and .Values.mcp.enabled .Values.webhook.enabled }}charts/n8n/values.yaml (2)
640-649: Update copy-paste comments that still read “webhook”Two comment lines reference “webhook” although they live inside the new
mcpblock. This is likely a copy-paste artefact and will confuse users skimming the file.- # additional (to main) config for webhook + # additional (to main) config for MCP ... - # additional (to main) config for webhook + # additional (to main) config for MCP
651-665: Comment header says “Webhook Kubernetes specific settings”Same issue as above – adjust to “MCP Kubernetes-specific settings” so that search/grep produces the expected section.
README.md (1)
41-44: Hyphenate “Kubernetes-specific” for consistencyMinor docs polish – the term is used as a compound adjective.
-6. MCP related settings + Kubernetes specific settings +6. MCP related settings + Kubernetes-specific settings -8. Redis related settings + Kubernetes specific settings +8. Redis related settings + Kubernetes-specific settingscharts/n8n/templates/deployment.mcp.yaml (1)
60-72: Minor: honour custom args without overriding the default commandIf users supply
.Values.mcp.commandArgsbut not.Values.mcp.command, the template still injects the implicitcommand: ["n8n"], which might not always be desired.
Consider guarding both blocks with the same predicate or introducing an explicituseDefaultCommandflag.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
README.md(2 hunks)charts/n8n/templates/configmap.mcp.yaml(1 hunks)charts/n8n/templates/deployment.mcp.yaml(1 hunks)charts/n8n/templates/ingress.yaml(1 hunks)charts/n8n/templates/secret.mcp.yaml(1 hunks)charts/n8n/templates/service.mcp.yaml(1 hunks)charts/n8n/values.yaml(1 hunks)examples/values_full.yaml(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- charts/n8n/templates/ingress.yaml
- examples/values_full.yaml
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~41-~41: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...ific settings 6. MCP related settings + Kubernetes specific settings 7. Raw Resources to pass throu...
(SPECIFIC_HYPHEN)
[uncategorized] ~43-~43: When ‘Kubernetes-specific’ is used as a modifier, it is usually spelled with a hyphen.
Context: ...onitor etc. 8. Redis related settings + Kubernetes specific settings ## Setting Configuration Valu...
(SPECIFIC_HYPHEN)
🪛 YAMLlint (1.37.1)
charts/n8n/templates/deployment.mcp.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 12-12: wrong indentation: expected 2 but found 4
(indentation)
[warning] 13-13: wrong indentation: expected 2 but found 4
(indentation)
charts/n8n/templates/configmap.mcp.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/secret.mcp.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/service.mcp.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
charts/n8n/templates/secret.mcp.yaml
Outdated
| {{- if .Values.mcp.secret }} | ||
| {{- if .Values.webhook.enabled }} | ||
| apiVersion: v1 | ||
| kind: Secret | ||
| metadata: | ||
| name: {{ include "n8n.fullname" . }}-mcp-secret | ||
| labels: | ||
| {{- include "n8n.labels" . | nindent 4 }} | ||
| type: Opaque | ||
| data: | ||
| {{- include "toEnvVars" (dict "values" .Values.mcp.secret "prefix" "" "isSecret" true) | nindent 4 }} | ||
|
|
||
| {{- end }} | ||
| {{- end }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Guard the Secret with mcp.enabled to avoid orphaned resources
If a user sets webhook.enabled=true and adds mcp.secret while leaving mcp.enabled=false, Helm will render a Secret that is never mounted anywhere, which is confusing and may leak stale credentials.
Wrap the template in a single conditional that requires the MCP component to be active as well:
-{{- if .Values.mcp.secret }}
-{{- if .Values.webhook.enabled }}
+{{- if and .Values.mcp.enabled .Values.webhook.enabled .Values.mcp.secret }}
...
-{{- end }}
-{{- end }}
+{{- end }}🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🤖 Prompt for AI Agents
In charts/n8n/templates/secret.mcp.yaml lines 1 to 14, the Secret resource is
conditionally created based on webhook.enabled and mcp.secret, but it lacks a
check for mcp.enabled. To fix this, wrap the entire template content in a
conditional that requires .Values.mcp.enabled to be true along with the existing
conditions, ensuring the Secret is only created when the MCP component is active
and preventing orphaned resources.
| {{- if .Values.mcp.config }} | ||
| {{- if .Values.webhook.enabled }} | ||
| apiVersion: v1 | ||
| kind: ConfigMap | ||
| metadata: | ||
| name: {{ include "n8n.fullname" . }}-mcp-config | ||
| labels: | ||
| {{- include "n8n.labels" . | nindent 4 }} | ||
| data: | ||
| {{- include "toEnvVars" (dict "values" .Values.mcp.config "prefix" "") | nindent 2 }} | ||
| {{- end }} | ||
| {{- end }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Same conditional mismatch as in the Secret template
The ConfigMap should only be created when the MCP pod is scheduled. Align the guard with the Secret fix:
-{{- if .Values.mcp.config }}
-{{- if .Values.webhook.enabled }}
+{{- if and .Values.mcp.enabled .Values.webhook.enabled .Values.mcp.config }}
...
-{{- end }}
-{{- end }}
+{{- end }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{- if .Values.mcp.config }} | |
| {{- if .Values.webhook.enabled }} | |
| apiVersion: v1 | |
| kind: ConfigMap | |
| metadata: | |
| name: {{ include "n8n.fullname" . }}-mcp-config | |
| labels: | |
| {{- include "n8n.labels" . | nindent 4 }} | |
| data: | |
| {{- include "toEnvVars" (dict "values" .Values.mcp.config "prefix" "") | nindent 2 }} | |
| {{- end }} | |
| {{- end }} | |
| {{- if and .Values.mcp.enabled .Values.webhook.enabled .Values.mcp.config }} | |
| apiVersion: v1 | |
| kind: ConfigMap | |
| metadata: | |
| name: {{ include "n8n.fullname" . }}-mcp-config | |
| labels: | |
| {{- include "n8n.labels" . | nindent 4 }} | |
| data: | |
| {{- include "toEnvVars" (dict "values" .Values.mcp.config "prefix" "") | nindent 2 }} | |
| {{- end }} |
🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🤖 Prompt for AI Agents
In charts/n8n/templates/configmap.mcp.yaml lines 1 to 12, the conditional checks
for creating the ConfigMap are mismatched, causing it to be created even when
the MCP pod is not scheduled. Adjust the conditionals so that the ConfigMap is
only created if .Values.mcp.config exists and the MCP pod scheduling condition
(similar to the Secret template) is met, ensuring consistent guard logic.
| replicas: 1 | ||
| strategy: | ||
| type: {{ .Values.mcp.deploymentStrategy.type }} | ||
| {{- if eq .Values.mcp.deploymentStrategy.type "RollingUpdate" }} | ||
| rollingUpdate: | ||
| maxSurge: {{ default "25%" .Values.mcp.deploymentStrategy.maxSurge }} | ||
| maxUnavailable: {{ default "25%" .Values.mcp.deploymentStrategy.maxUnavailable }} | ||
| {{- end }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Hard-coded replicas: 1 prevents scaling
Locking the component to a single replica removes the option to run an HA MCP when/if the application becomes stateless.
Expose the value through values.yaml, defaulting to 1 for backward compatibility:
-replicas: 1
+replicas: {{ default 1 .Values.mcp.replicaCount }}🤖 Prompt for AI Agents
In charts/n8n/templates/deployment.mcp.yaml around lines 19 to 26, the replicas
count is hard-coded to 1, preventing scaling. Modify the replicas field to use a
value from values.yaml, for example, replace the fixed '1' with a template
expression that defaults to 1 if not set, allowing users to configure the number
of replicas while maintaining backward compatibility.
There was a problem hiding this comment.
Should this be hardcoded or can we default this to 1 as recommended?
There was a problem hiding this comment.
🧩 Analysis chain
@RoseSecurity I'd recommend making it configurable with a default of 1 for consistency and flexibility. Let me check how other components handle replica counts in this chart.
🏁 Script executed:
#!/bin/bash
# Check how other deployment templates handle replica counts
echo "=== Checking replica configurations in other deployments ==="
find charts/n8n/templates -name "deployment.*.yaml" -exec grep -H -A2 -B2 "replicas:" {} \;
echo -e "\n=== Checking values.yaml for replica configurations ==="
grep -n -A5 -B5 "replica" charts/n8n/values.yaml || echo "No replica configurations found in values.yaml"
echo -e "\n=== Checking if any components have configurable replica counts ==="
rg "\.replicaCount" charts/n8n/templates/
echo -e "\n=== Checking autoscaling configurations ==="
grep -n -A10 -B5 "autoscaling:" charts/n8n/values.yaml || echo "No autoscaling configurations found"
Length of output: 5162
Based on the analysis, I can see there's an inconsistency in the current approach. Let me check the MCP documentation reference and the autoscaling configuration for MCP.
🏁 Script executed:
#!/bin/bash
# Check the MCP section in values.yaml more closely
echo "=== MCP configuration in values.yaml ==="
grep -n -A50 "# MCP related settings" charts/n8n/values.yaml | head -60
echo -e "\n=== Check if MCP has autoscaling despite the comment ==="
grep -n -A15 -B5 "mcp:" charts/n8n/values.yaml | grep -A15 -B5 "autoscaling"
echo -e "\n=== Check deployment template conditions for autoscaling ==="
grep -n -A10 -B5 "autoscaling.enabled" charts/n8n/templates/deployment.mcp.yaml || echo "No autoscaling check found in MCP deployment"
Length of output: 2682
Based on my analysis, I recommend making it configurable with a default of 1 for the following reasons:
Key Findings:
-
Inconsistent Pattern: All other components (main, worker, webhook) use configurable replica counts via
{{ .Values.component.replicaCount }}, but MCP hardcodes it to 1. -
Documentation Constraint: The
values.yamlcomment states "MCP deployment is basically a Webhook deployment but always has a single replica" and references n8n documentation, suggesting there may be technical reasons for the single-replica constraint. -
Missing Autoscaling Logic: Unlike other components, MCP doesn't have autoscaling conditional logic in its deployment template.
Recommendation:
# In values.yaml, add:
mcp:
replicaCount: 1 # Default to 1 per n8n documentation
# In deployment.mcp.yaml, change:
replicas: {{ .Values.mcp.replicaCount }}This approach:
- ✅ Maintains consistency with other components in the chart
- ✅ Preserves the default behavior (single replica)
- ✅ Provides flexibility for future n8n versions that might support multiple MCP replicas
- ✅ Follows Helm best practices for configurability
The hardcoded approach would work for now, but making it configurable is more future-proof and follows the established pattern in this chart. Users who need the constraint can simply leave it at the default value of 1.
| template: | ||
| metadata: | ||
| annotations: | ||
| checksum/config: {{ print .Values.mcp .Values.main | sha256sum }} |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Non-deterministic checksum may trigger needless pod restarts
print on Go maps produces an unpredictable key order; the resulting hash will change even when the underlying values don’t, causing rolling restarts on every Helm upgrade.
Replace it with a deterministic serialisation such as toJson.
-checksum/config: {{ print .Values.mcp .Values.main | sha256sum }}
+checksum/config: {{ toJson (dict "mcp" .Values.mcp "main" .Values.main) | sha256sum }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| checksum/config: {{ print .Values.mcp .Values.main | sha256sum }} | |
| checksum/config: {{ toJson (dict "mcp" .Values.mcp "main" .Values.main) | sha256sum }} |
🤖 Prompt for AI Agents
In charts/n8n/templates/deployment.mcp.yaml at line 34, the checksum calculation
uses `print` on Go maps, which produces a non-deterministic key order causing
unnecessary pod restarts. Replace `print` with `toJson` to serialize the maps
deterministically before hashing, ensuring the checksum only changes when the
actual values change.
| env: {{ not (empty .Values.mcp.extraEnv) | ternary nil "[]" }} | ||
| {{- range $key, $value := .Values.mcp.extraEnv }} | ||
| - name: {{ $key }} | ||
| {{- toYaml $value | nindent 14 }} | ||
| {{- end }} |
There was a problem hiding this comment.
env: emits <no value> when extraEnv is present
ternary nil renders as <no value> which is invalid YAML and fails K8s validation.
Invert the condition so that env is [] when empty, and omitted otherwise:
-env: {{ not (empty .Values.mcp.extraEnv) | ternary nil "[]" }}
+env: {{ empty .Values.mcp.extraEnv | ternary "[]" nil }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| env: {{ not (empty .Values.mcp.extraEnv) | ternary nil "[]" }} | |
| {{- range $key, $value := .Values.mcp.extraEnv }} | |
| - name: {{ $key }} | |
| {{- toYaml $value | nindent 14 }} | |
| {{- end }} | |
| env: {{ empty .Values.mcp.extraEnv | ternary "[]" nil }} | |
| {{- range $key, $value := .Values.mcp.extraEnv }} | |
| - name: {{ $key }} | |
| {{- toYaml $value | nindent 14 }} | |
| {{- end }} |
🤖 Prompt for AI Agents
In charts/n8n/templates/deployment.mcp.yaml around lines 103 to 107, the current
use of `ternary nil "[]"` causes the `env:` field to emit `<no value>` when
`.Values.mcp.extraEnv` is present, which is invalid YAML. To fix this, invert
the condition so that `env:` is set to an empty list `[]` when
`.Values.mcp.extraEnv` is empty, and omit the `env:` field entirely when
`.Values.mcp.extraEnv` has values. Adjust the templating logic accordingly to
ensure valid YAML output.
| 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.mcp.config }} | ||
| - configMapRef: | ||
| name: {{ include "n8n.fullname" . }}-mcp-config | ||
| {{- end }} | ||
| {{- if .Values.mcp.secret }} | ||
| - secretRef: | ||
| name: {{ include "n8n.fullname" . }}-mcp-secret | ||
| {{- end }} | ||
|
|
There was a problem hiding this comment.
Mis-aligned envFrom block will render invalid YAML
A leading space before {{- keeps one literal space in the output, breaking indentation for the list items (- configMapRef: / - secretRef:).
Either drop the stray space or use nindent.
-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 }}
+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 }}
…Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In charts/n8n/templates/deployment.mcp.yaml around lines 85 to 102, the envFrom
block has a leading space before the {{- which causes incorrect indentation and
invalid YAML output. Remove the leading space before each {{- or replace the
indentation logic with nindent to ensure proper YAML formatting and alignment of
the list items under envFrom.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
charts/n8n/templates/deployment.mcp.yaml (4)
18-18: ExposereplicaCountviavalues.yamlto allow scalingThe Deployment is still pinned to a single replica, which prevents horizontal scaling. Surface the value through
Values.mcp.replicaCount, defaulting to 1 for back-compat.-replicas: 1 +replicas: {{ default 1 .Values.mcp.replicaCount }}
33-33: Use deterministic JSON when hashing to avoid needless rollouts
toJsoninstead:-checksum/config: {{ print .Values.mcp .Values.main | sha256sum }} +checksum/config: {{ toJson (dict "mcp" .Values.mcp "main" .Values.main) | sha256sum }}
84-100: Indentation is still off — renders invalid YAMLThere’s an extra leading space before
{{- if .Values.main.secret }}and{{- if .Values.mcp.secret }}which breaks the list indentation produced underenvFrom:. Remove the stray space or wrap the whole block withnindent 2.
102-106:env:still emits<no value>when extraEnv is setThe expression inverts the intended logic: when
extraEnvis present, Helm printsnil, which renders as<no value>and breaks YAML parsing. Invert the condition or template the list explicitly:-env: {{ not (empty .Values.mcp.extraEnv) | ternary nil "[]" }} - {{- range $key, $value := .Values.mcp.extraEnv }} - - name: {{ $key }} - {{- toYaml $value | nindent 14 }} - {{- end }} +{{- if .Values.mcp.extraEnv }} +env: + {{- range $key, $value := .Values.mcp.extraEnv }} + - name: {{ $key }} + {{- toYaml $value | nindent 12 }} + {{- end }} +{{- end }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
charts/n8n/templates/deployment.mcp.yaml(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
charts/n8n/templates/deployment.mcp.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
[warning] 10-10: wrong indentation: expected 2 but found 4
(indentation)
[warning] 11-11: wrong indentation: expected 2 but found 4
(indentation)
[warning] 12-12: wrong indentation: expected 2 but found 4
(indentation)
| 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.mcp.config }} | ||
| - configMapRef: | ||
| name: {{ include "n8n.fullname" . }}-mcp-config | ||
| {{- end }} | ||
| {{- if .Values.mcp.secret }} | ||
| - secretRef: | ||
| name: {{ include "n8n.fullname" . }}-mcp-secret | ||
| {{- end }} |
There was a problem hiding this comment.
envFrom: is emitted even when empty, producing illegal YAML
If none of the referenced ConfigMaps/Secrets are enabled, the template still outputs a bare envFrom: key with no list, failing manifest validation. Wrap the whole section in a guard:
- envFrom:
- {{- if .Values.main.config }}
- - configMapRef:
+{{- $hasEnvFrom := or .Values.main.config (or .Values.main.secret (or .Values.mcp.config .Values.mcp.secret)) }}
+{{- if $hasEnvFrom }}
+ 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.mcp.config }}
- configMapRef:
name: {{ include "n8n.fullname" . }}-mcp-config
{{- end }}
{{- if .Values.mcp.secret }}
- secretRef:
name: {{ include "n8n.fullname" . }}-mcp-secret
{{- end }}
+{{- end }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| 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.mcp.config }} | |
| - configMapRef: | |
| name: {{ include "n8n.fullname" . }}-mcp-config | |
| {{- end }} | |
| {{- if .Values.mcp.secret }} | |
| - secretRef: | |
| name: {{ include "n8n.fullname" . }}-mcp-secret | |
| {{- end }} | |
| {{- $hasEnvFrom := or .Values.main.config (or .Values.main.secret (or .Values.mcp.config .Values.mcp.secret)) }} | |
| {{- if $hasEnvFrom }} | |
| 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.mcp.config }} | |
| - configMapRef: | |
| name: {{ include "n8n.fullname" . }}-mcp-config | |
| {{- end }} | |
| {{- if .Values.mcp.secret }} | |
| - secretRef: | |
| name: {{ include "n8n.fullname" . }}-mcp-secret | |
| {{- end }} | |
| {{- end }} |
🤖 Prompt for AI Agents
In charts/n8n/templates/deployment.mcp.yaml around lines 84 to 100, the envFrom
key is output even when no ConfigMaps or Secrets are referenced, resulting in
invalid YAML. To fix this, wrap the entire envFrom section in a conditional
guard that checks if any of the .Values.main.config, .Values.main.secret,
.Values.mcp.config, or .Values.mcp.secret are set, so envFrom is only emitted
when there is at least one entry to include.
looks like these errors were intermittent. anyway, i've added option to add a separate n8n webhook instance for mcp to make responses stable as it was described in the doc, hope my change suits your rules or best practices or whatever. please let me know if i should fix something |
|
I think there are some valid recommendations from CodeRabbit on this PR |
|
@RoseSecurity, i have mostly borrowed the code from the webhook deployment/secret/config/etc, just did |
|
Just checking, is this regarding the MCP of N8N itself? I am also struggling with other MCP solution, and would like to extend the chart to also include the option to add containers to the pod itself. |
|
@NigelVanHattum, I'm not really sure what you mean exactly, I'm not an n8n user myself. But we (the company) are using MCP server feature of n8n and it requires a single dedicated pod, which is implemented in the pull request. @Vad1mo, please, anything else I need to fix to make you hapy with the PR? |
4 participants
in a distributed setup, mcp endpoints don't work in the main process, so point them to webhook pods
Summary by CodeRabbit
New Features
mcpsection to the configuration files, allowing customization of persistence, resources, probes, environment variables, and Kubernetes-specific settings.Documentation