[#251] Removed hardcoded pathType: Prefix

[krufab]

Changed the ingress logic to allow different types of pathType in place of the hardcoded 'pathType: Prefix'

What this PR does / why we need it

In the current version, the pathType elements in the ingress object are hardcoded to pathType: Prefix.
This PR aligns the chart to the best practices, allowing to define a different type of pathType, as Prefix might be incompatible with some ingress controllers.

Which issue this PR fixes

Fixes #251

Special notes for your reviewer

Checklist

Please place an 'x' in all applicable fields and remove unrelated items.

Version and Documentation

• Chart version updated in Chart.yaml following semantic versioning
• App version updated in Chart.yaml if applicable
• artifacthub.io/changes section updated in Chart.yaml (see ArtifactHub annotation reference)
• Variables are documented in the README.md

Testing and Validation

• Ran ah lint locally without errors
• Ran Chart-Testing: ct lint --chart-dirs charts/n8n --charts charts/n8n --validate-maintainers=false
• Tested chart installation locally
• Tested with example configurations in /examples directory

Summary by CodeRabbit

• New Features

  • Ingress routing now accepts explicit path objects with configurable pathType; webhook and form URLs are composed from the configured path for consistent URL behavior.

• Chores

  • Chart version bumped to 2.0.1.

• Documentation

  • README updated with commented example lines showing ingress path and pathType usage.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

Switch ingress path entries from bare strings to structured objects with path and pathType; update templates and examples to use {{ .path }}/{{ .pathType }} and trim concatenation for webhook/form suffixes; bump chart version and adjust NOTES URL interpolation. No runtime control-flow changes.

Changes

Cohort / File(s)	Summary
Chart metadata charts/n8n/Chart.yaml	Bumped chart version 2.0.0 → 2.0.1 and replaced CHANGE/annotation with a single note about ingress pathType flexibility.
Ingress templates charts/n8n/templates/ingress.yaml	Replace usages of {{ . }} with {{ .path }} and hardcoded pathType: Prefix with pathType: {{ .pathType }}; webhook and form endpoints now use {{ trimSuffix "/" .path }}/....
User-facing notes charts/n8n/templates/NOTES.txt	URL printing changed from using the full path object ({{ . }}) to the path string ({{ .path }}).
Default values charts/n8n/values.yaml	ingress.hosts[0].paths changed from an empty/implicit list to [ { path: "/", pathType: "Prefix" } ].
Examples examples/values_full.yaml, examples/values_small_prod.yaml	Converted shorthand path entries to explicit objects with path and pathType (e.g., - path: / + pathType: Prefix).
Documentation README.md	Added commented example lines showing path and pathType under ingress.hosts[0].paths.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

• Attention recommended:
  • charts/n8n/templates/ingress.yaml — verify trimmed path concatenation for trailing-slash edge cases and correct pathType substitution across ingress controllers.
  • charts/n8n/templates/NOTES.txt — confirm displayed URLs match rendered ingress rules.
  • charts/n8n/values.yaml and example files — YAML shape changes may affect consumers; validate Helm rendering with common values.

Possibly related PRs

• Next Gen Chart #132 — Modifies ingress templates and values; closely related to path/pathType handling changes.

Suggested reviewers

• Vad1mo
• alexandre-k
• fank

Pre-merge checks and finishing touches

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: removing hardcoded pathType values to make them configurable.
Linked Issues check	✅ Passed	The PR successfully implements the core objective from issue #251 by making pathType configurable instead of hardcoded, addressing the Ingress controller compatibility issue.
Out of Scope Changes check	✅ Passed	All changes directly support the objective of making pathType configurable and fixing Ingress routing across different controllers; no unrelated modifications detected.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e0caa81 and 693feb3.

📒 Files selected for processing (7)

• README.md (1 hunks)
• charts/n8n/Chart.yaml (2 hunks)
• charts/n8n/templates/NOTES.txt (1 hunks)
• charts/n8n/templates/ingress.yaml (1 hunks)
• charts/n8n/values.yaml (1 hunks)
• examples/values_full.yaml (1 hunks)
• examples/values_small_prod.yaml (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (6)

• examples/values_full.yaml
• README.md
• charts/n8n/templates/NOTES.txt
• examples/values_small_prod.yaml
• charts/n8n/values.yaml
• charts/n8n/Chart.yaml

🔇 Additional comments (2)

charts/n8n/templates/ingress.yaml (2)

34-35: ✓ Configurable pathType applied correctly to main path entry.

The main path now uses {{ .path }} and {{ .pathType }} from structured objects, replacing the hardcoded Prefix and enabling the configurability required by issue #251.

---

42-72: Ensure pathType is explicitly defined in values.yaml with Prefix as default for webhook paths.

The webhook and form paths correctly apply {{ trimSuffix "/" .path }} for base path derivation and consistently reference {{ .pathType }}. However, three critical requirements must be verified in values.yaml:

1. pathType must be explicitly set — Kubernetes Ingress has no portable default pathType; always define it explicitly. For webhook paths like /webhook/, /webhook-test/, /webhook-waiting/, and /form/, use Prefix pathType to match both the exact path and all child routes (e.g., /webhook/ matches /webhook/foo).
2. Avoid ImplementationSpecific — Use this only for controller-specific features (regex, advanced matchers). It is non-portable and behavior varies by ingress controller (nginx, traefik, etc.). Stick to Prefix or Exact for compatibility.
3. Verify values.yaml ingress.hosts structure — Confirm each path entry includes an explicit pathType: Prefix (or Exact if single-path matching is required) rather than relying on any implicit defaults or controller-specific behavior.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

charts/n8n/templates/ingress.yaml (1)

42-43: Consider documenting webhook path concatenation behavior.

The webhook paths (lines 42, 50, 57, 64) concatenate the base path with suffixes: {{ .path }}webhook/, {{ .path }}webhook-test/, etc. This works correctly for the root path (/), but non-root paths may produce unexpected results (e.g., path: /api yields /apiwebhook/ instead of /api/webhook/). While likely acceptable as the root path is the primary use case, a template comment clarifying this behavior would prevent user confusion.

Example clarifying comment:

          {{- range .paths }}
          - path: {{ .path }}
            pathType: {{ .pathType }}
            backend:
              service:
                name: {{ $fullName }}
                port:
                  number: {{ $.Values.main.service.port | default 80  }}
          {{- if $.Values.webhook.enabled }}
+         # Webhook paths are concatenated with the base path
          - path: {{ .path }}webhook/
            pathType: {{ .pathType }}

Also applies to: 50-51, 57-58, 64-65

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 344f5e0 and 3545730.

📒 Files selected for processing (7)

• README.md (1 hunks)
• charts/n8n/Chart.yaml (2 hunks)
• charts/n8n/templates/NOTES.txt (1 hunks)
• charts/n8n/templates/ingress.yaml (1 hunks)
• charts/n8n/values.yaml (1 hunks)
• examples/values_full.yaml (1 hunks)
• examples/values_small_prod.yaml (1 hunks)

🧰 Additional context used

🧠 Learnings (1)

📚 Learning: 2025-09-02T08:05:39.372Z

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.

Applied to files:

• charts/n8n/Chart.yaml

🔇 Additional comments (6)

README.md (1)

130-143: Good documentation alignment with config changes.

The commented example at lines 138–139 clearly illustrates the new explicit path and pathType object structure, helping users understand the ingress configuration format.

charts/n8n/values.yaml (1)

42-44: Clear, functional default for ingress paths.

The explicit path: / and pathType: Prefix structure provides sensible defaults while enabling users to customize pathType for their ingress controller. This directly addresses issue #251 by allowing flexibility instead of hardcoding.

charts/n8n/Chart.yaml (1)

3-3: Appropriate versioning and changelog entry.

The version bump to 1.1.1 and updated artifacthub.io/changes annotation accurately reflect the ingress schema change, making the modification visible in package registries.

Also applies to: 37-38

examples/values_full.yaml (1)

109-111: Example correctly demonstrates new path structure.

The production example properly illustrates the explicit path and pathType object format, providing users with a clear reference implementation.

charts/n8n/templates/NOTES.txt (1)

4-5: Template correctly accesses path field.

The change from {{ . }} to {{ .path }} properly extracts the path string from the new path object structure, maintaining correct URL generation in deployment notes.

charts/n8n/templates/ingress.yaml (1)

34-35: Ingress template correctly implements configurable pathType.

The template now uses {{ .path }} and {{ .pathType }} to consume path objects, enabling users to configure pathType for different ingress controllers as intended. All path rules—main service, webhook routes, and form handlers—consistently apply the configured pathType.

Also applies to: 42-43, 50-51, 57-58, 64-65

[sakowicz]

Just want to say that this is breaking change. But it was released as hotfix. @Vad1mo

[Vad1mo]

Just want to say that this is breaking change. But it was released as hotfix. @Vad1mo

I am sorry; I didn't expect that. What was the breaking change in that case?

Was it this one here?

      paths:
        - path: /
          pathType: Prefix

[sakowicz]

Yes

[Vad1mo]

sorry, nobody paid attention to backwards compatiblitly, and it slipped on my too. I make not in the relese notes at least