fix: allow empty tls configuration for ingress #201
Conversation
WalkthroughThe changes update the Helm chart for the n8n application by incrementing its version, modifying the ingress TLS configuration in the values file to default to an empty list, and updating documentation to guide users on deploying example setups requiring PostgreSQL. No changes to application code or exported entities were made. Changes
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
✨ 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 (2)
charts/n8n/Chart.yaml (1)
36-38: Minor typo: “ingres” → “ingress” in annotation- description: "allows empty tls for ingres: fixes https://github.com/8gears/n8n-helm-chart/issues/167" + description: "Allows empty TLS for ingress – fixes https://github.com/8gears/n8n-helm-chart/issues/167"Correct spelling improves searchability in Artifact Hub and changelogs.
examples/README.md (1)
18-34: Fix wording and grammar in the new “Test Examples” sectionSeveral small issues were flagged (e.g. “text” vs “test”, missing prepositions).
Suggested patch:-## Test Examples - -If you want to text or deploy this examples with postgresdb, we recommend the use -cloudnative-pg. +## Test Examples + +If you want to **test** or deploy these examples with a PostgreSQL database, we +recommend using CloudNative-PG. @@ -See [installation guide](https://cloudnative-pg.io/documentation/1.26/installation_upgrade/) for all details. +See the [installation guide](https://cloudnative-pg.io/documentation/1.26/installation_upgrade/) for full details.This addresses the LanguageTool hints and improves readability.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
charts/n8n/Chart.yaml(2 hunks)charts/n8n/values.yaml(1 hunks)examples/README.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
examples/README.md
[uncategorized] ~22-~22: The determiner “these” seems more likely in this context.
Context: ...Examples If you want to text or deploy this examples with postgresdb, we recommend ...
(AI_EN_LECTOR_REPLACEMENT_DETERMINER)
[uncategorized] ~22-~22: “the” seems less likely than “to”.
Context: ... examples with postgresdb, we recommend the use cloudnative-pg. The full and `sm...
(AI_HYDRA_LEO_CP_THE_TO)
[uncategorized] ~22-~22: Possible missing preposition found.
Context: ...s with postgresdb, we recommend the use cloudnative-pg. The full and small-prod exampl...
(AI_HYDRA_LEO_MISSING_OF)
🔇 Additional comments (2)
charts/n8n/values.yaml (1)
43-46: Consider omitting thetlsfield entirely instead of setting an empty listHelm’s Go templates typically guard the
tlsblock with
{{- if .Values.ingress.tls }}.
An empty list ([]) evaluates tofalse, so the rendered manifest will still omit thetls:section – which is what you want. However, if a downstream consumer setsingress.tlsvia--set ingress.tls=(resulting in the literal string"") you might end up with:tls: ""which is invalid Kubernetes syntax and will break
kubectl apply.Diff suggestion (purely cosmetic, but safer):
- tls: [] +# tls: []Commenting the key out avoids accidental misuse while keeping the example handy.
Check that the ingress template uses
{{- if .Values.ingress.tls }}(and not{{- if hasKey .Values.ingress "tls" }}) so the behaviour remains unchanged.charts/n8n/Chart.yaml (1)
3-3: Chart version bump looks goodVersion increased to
1.0.11, matching SemVer expectations for a bug-fix.
No additional action required.
* fix: allow empty tls configuration for ingress * fix: update values.yaml to allow empty tls configuration
1 participant
fixes #167
Summary by CodeRabbit
Bug Fixes
Documentation
Chores