feat(httproute): add ingress sucessor manifest #277
Conversation
WalkthroughAdds a new Helm template Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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
🧹 Nitpick comments (1)
charts/n8n/values.yaml (1)
50-97: HTTPRoute configuration structure looks solid; verify webhook path consistency and additionalRules intent.The new HTTPRoute section mirrors the Ingress structure well. However, a few items warrant attention:
Inconsistent additionalRules: The
mainblock (line 79) includes anadditionalRulesfield, but thewebhookblock (lines 80–96) does not. Is this intentional, or should webhook also support additionalRules for consistency?Trailing slash inconsistency: All webhook paths have trailing slashes (
/webhook/,/webhook-test/,/webhook-waiting/) except/form(line 94). Confirm this is intentional per n8n's routing.Empty parentRefs & hostnames: Users will need to configure
parentRefsandhostnamesfor the HTTPRoute to function. This is standard Gateway API practice, but ensure the template or documentation flags this requirement clearly.Comment wording (lines 68, 78): Consider rewording "deny some path like ingresses" to be clearer, e.g., "deny some paths like ingresses do".
Please verify:
- Are all n8n webhook paths included? Any other routes needed (e.g.,
/webhook-response/,/webhook-tunnel/)?- Is the
/formpath intentionally missing a trailing slash?- Does the template provide guidance when
parentRefsis empty?For consistency, consider adding an empty
additionalRules: []to the webhook block (line 96) to mirror the main block structure.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
charts/n8n/Chart.yaml(2 hunks)charts/n8n/values.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- charts/n8n/Chart.yaml
|
@DrummyFloyd thank you for the PR, plase update the README, to fit this alongside ingress. in the right section of the values file. also the linter fails. |
Signed-off-by: DrummyFloyd <[email protected]>
Signed-off-by: DrummyFloyd <[email protected]>
DrummyFloyd
force-pushed
the
add-httproute
branch
from
34bd11b to
44d4b46
Compare
should be good now =) |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
charts/n8n/values.yaml (1)
50-97: Values structure looks good; minor inconsistency in additionalRules field.The route configuration block is well-structured and aligns with the template expectations. However, Line 79 shows
route.main.additionalRules, butroute.webhook(lines 80–96) doesn't include a correspondingadditionalRulesfield. If this is intentional, document it; otherwise, add it for consistency with the main route.Consider adding
additionalRules: []toroute.webhook(after line 96) to match the structure ofroute.main:webhook: # -- Route matches matches: - path: type: PathPrefix value: /webhook/ - path: type: PathPrefix value: /webhook-test/ - path: type: PathPrefix value: /webhook-waiting/ - path: type: PathPrefix value: /form # -- Route filters filters: [] + # -- Additional rules to deny some path like ingresses + additionalRules: []README.md (1)
33-39: Fix hyphenation in overview section headings.Lines 35–39 need hyphens to follow standard English grammar: "Kubernetes-specific settings" (not "Kubernetes specific settings").
Apply this diff to fix the grammar:
1. Global and chart wide values, like the image repository, image tag, etc. 2. HttpRoute -3. Ingress, (default is nginx, but you can change it to your own ingress controller) -4. Main n8n app configuration + Kubernetes specific settings -5. Worker related settings + Kubernetes specific settings -6. Webhook related settings + Kubernetes specific settings -7. Raw Resources to pass through your own manifests like GatewayAPI, ServiceMonitor etc. -8. Redis related settings + Kubernetes specific settings +3. Ingress, (default is nginx, but you can change it to your own ingress controller) +4. Main n8n app configuration + Kubernetes-specific settings +5. Worker-related settings + Kubernetes-specific settings +6. Webhook-related settings + Kubernetes-specific settings +7. Raw Resources to pass through your own manifests like GatewayAPI, ServiceMonitor etc. +8. Redis-related settings + Kubernetes-specific settings
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md(2 hunks)charts/n8n/Chart.yaml(2 hunks)charts/n8n/templates/route.yaml(1 hunks)charts/n8n/values.yaml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- charts/n8n/Chart.yaml
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: tsch157
Repo: 8gears/n8n-helm-chart PR: 239
File: charts/n8n/Chart.yaml:3-4
Timestamp: 2025-09-02T08:05:39.372Z
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.
🪛 LanguageTool
README.md
[grammar] ~35-~35: Use a hyphen to join words.
Context: ... Main n8n app configuration + Kubernetes specific settings 5. Worker related sett...
(QB_NEW_EN_HYPHEN)
[grammar] ~36-~36: Use a hyphen to join words.
Context: ... 5. Worker related settings + Kubernetes specific settings 6. Webhook related set...
(QB_NEW_EN_HYPHEN)
[grammar] ~37-~37: Use a hyphen to join words.
Context: ...6. Webhook related settings + Kubernetes specific settings 7. Raw Resources to pa...
(QB_NEW_EN_HYPHEN)
[grammar] ~39-~39: Use a hyphen to join words.
Context: ...GatewayAPI, ServiceMonitor etc. 8. Redis related settings + Kubernetes specific s...
(QB_NEW_EN_HYPHEN)
[grammar] ~39-~39: Use a hyphen to join words.
Context: .... 8. Redis related settings + Kubernetes specific settings ## Configurating N8n ...
(QB_NEW_EN_HYPHEN)
🪛 YAMLlint (1.37.1)
charts/n8n/templates/route.yaml
[error] 1-1: syntax error: expected the node content, but found '-'
(syntax)
🔇 Additional comments (2)
charts/n8n/templates/route.yaml (1)
1-53: Template structure is sound; YAML linter error is a false positive.The YAMLlint error at line 1 is a false positive—
{{- if .Values.route.enabled }}is valid Helm syntax. The template conditionally renders the entire HTTPRoute resource, which is appropriate when the feature is disabled.The template logic correctly:
- Gates webhook route rendering behind
$.Values.webhook.enabled(line 40)- Defaults service ports to 80 if not specified (lines 31, 43)
- Uses
tplfor dynamic templating within hostnames and matchesREADME.md (1)
129-166: HTTPRoute documentation is comprehensive and well-positioned; reviewer feedback addressed.The README now includes a full HTTPRoute example that mirrors the values.yaml structure and shows realistic configuration (traefik gateway). The placement at item 2 in the overview aligns with the values.yaml structure (route block appears before ingress in the file). The example demonstrates:
- parentRefs with a traefik gateway reference (practical example)
- hostnames configuration
- main and webhook route matches
This addresses the reviewer's request to include HTTPRoute alongside Ingress in the correct section. The documentation is clear and actionable.
2 participants
What this PR does / why we need it
add template/manifest for
kind: HTTPRoutesucessor ofkind: Ingress, because is now in GA, for both n8n server & webhook, following quite the same logic asIngressWhich issue this PR fixes
can create one if needed ..
Special notes for your reviewer
tested manually
Checklist
Please place an 'x' in all applicable fields and remove unrelated items.
Version and Documentation
Chart.yamlfollowing semantic versioning- [ ] App version updated inChart.yamlif applicableartifacthub.io/changessection updated inChart.yaml(see ArtifactHub annotation reference)README.mdTesting and Validation
ah lintlocally without errorsct lint --chart-dirs charts/n8n --charts charts/n8n --validate-maintainers=false/examplesdirectorySummary by CodeRabbit
New Features
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.