-
Notifications
You must be signed in to change notification settings - Fork 203
Change default service name to prevent conflic with N8N_PORT #213
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes update the Kubernetes Helm chart templates for the n8n application to standardize the naming convention of the service resource. Specifically, the service name is now suffixed with "-svc" in both the service and ingress templates, ensuring consistent reference and routing within the deployment. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ 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 (
|
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: 2
🧹 Nitpick comments (1)
charts/n8n/templates/ingress.yaml (1)
50-56
: Avoid hard-coding the suffix in multiple filesTo keep future renames trivial, define a helper, e.g.:
{{- define "n8n.mainServiceName" -}} {{ include "n8n.fullname" . }}-svc {{- end }}
and use
{{ include "n8n.mainServiceName" . }}
throughout the chart.
This eliminates duplication and the risk of inconsistent updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
charts/n8n/templates/ingress.yaml
(1 hunks)charts/n8n/templates/service.yaml
(1 hunks)
service: | ||
name: {{ $fullName }} | ||
name: {{ $fullName }}-svc | ||
port: | ||
number: {{ $.Values.main.service.port | default 80 }} |
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.
🛠️ Refactor suggestion
Only the root path was updated – /webhook-test/
still points to a non-existent service
The backend for the root path now targets {{ $fullName }}-svc
, but the /webhook-test/
rule (line 53) still targets {{ $fullName }}
. After this rename, manual-execution webhooks will return 503. Align all paths that should hit the main process:
- name: {{ $fullName }}
+ name: {{ $fullName }}-svc
🤖 Prompt for AI Agents
In charts/n8n/templates/ingress.yaml between lines 36 and 39, the service name
for the root path was updated to {{ $fullName }}-svc, but the /webhook-test/
path at line 53 still targets {{ $fullName }}. To fix the 503 errors on
manual-execution webhooks, update the service name in the /webhook-test/ path
rule to {{ $fullName }}-svc so it matches the renamed service and routes
correctly.
kind: Service | ||
metadata: | ||
name: {{ include "n8n.fullname" . }} | ||
name: {{ include "n8n.fullname" . }}-svc |
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.
💡 Verification agent
🧩 Analysis chain
Renaming breaks remaining unsuffixed references – update them or expect 503s
Changing the Service name to {{ include "n8n.fullname" . }}-svc
is sensible, but all chart objects that still reference the old name will now fail.
A quick grep already shows an unsuffixed use in charts/n8n/templates/ingress.yaml
(line 53) and possibly elsewhere (HPA, ServiceMonitor, NetworkPolicy, tests, docs). Update every reference before shipping.
🏁 Script executed:
#!/bin/bash
# Find all unsuffixed service references in the chart
fullname='{{[[:space:]]*include[[:space:]]+"n8n.fullname"[[:space:]]*\.[[:space:]]*}}'
rg -n "$fullname" charts/n8n/templates | grep -v '\-svc' || true
Length of output: 371
🏁 Script executed:
#!/usr/bin/env bash
# Find all literal Helm include calls of n8n.fullname missing the "-svc" suffix
rg -n '{{ include "n8n.fullname" . }}' charts/n8n/templates | grep -v '\-svc' || true
Length of output: 192
🏁 Script executed:
#!/usr/bin/env bash
# Search for literal Helm fullname includes missing the "-svc" suffix using fixed string matching
rg -F '{{ include "n8n.fullname" . }}' -n charts/n8n/templates/*.yaml | grep -v -- '-svc' || true
Length of output: 2925
🏁 Script executed:
#!/usr/bin/env bash
# Inspect ingress.yaml around expected service reference
sed -n '40,80p' charts/n8n/templates/ingress.yaml
Length of output: 1307
Update the ingress service reference to include the “-svc” suffix
The Service name was changed to {{ include "n8n.fullname" . }}-svc
, so any backend that still points to the unsuffixed name will 404/503. A single occurrence was found in your ingress:
• charts/n8n/templates/ingress.yaml (around line 53)
- backend:
service:
- name: {{ $fullName }}
+ name: {{ $fullName }}-svc
No other literal service references to {{ include "n8n.fullname" . }}
appear in contexts that map to the Service object.
Please apply this change before shipping to avoid broken routes.
🤖 Prompt for AI Agents
In charts/n8n/templates/ingress.yaml around line 53, update the backend service
name to include the "-svc" suffix to match the Service name defined as {{
include "n8n.fullname" . }}-svc. Replace any occurrence of {{ include
"n8n.fullname" . }} with {{ include "n8n.fullname" . }}-svc in the ingress
backend service reference to prevent 404/503 errors.
Would you be able to address the CodeRabbit findings please? Thanks for this contribution |
is this ready? it is a blocker with example config |
There are merge conflicts that need to be resolved |
Summary by CodeRabbit
-svc
suffix to the service name.