fix: HTTPS Redirects Always Enabled #22
Conversation
This change ensures the https redirects for authelia and mailipit aren't created when there is no https listener defined on the gateway. `and` evaluates any non-empty string as true, and the include was returning the string "false". Generated with [Cursor](https://cursor.com/) Signed-off-by: Trevor Bramwell <[email protected]>
WalkthroughThe conditional logic for rendering HTTPRoute resources in Helm chart templates was updated. For Authelia, the resource now requires Traefik to be enabled in addition to existing checks. For Mailpit, the HTTPS-enabled check was made more explicit by comparing to the string "true". Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~5-10 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
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a bug where HTTPS redirect HTTPRoutes for Authelia and Mailpit were being created even when HTTPS was disabled. The issue occurred because Helm's and function evaluates any non-empty string as true, and the lfx-platform.https-enabled template was returning the string "false" instead of a boolean false.
- Fixed conditional logic to properly check if HTTPS is enabled using string comparison
- Added missing traefik.enabled condition to the authelia template for consistency
- Ensures HTTPS redirects are only created when HTTPS is actually configured
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| charts/lfx-platform/templates/mailpit/https-redirect-httproute.yaml | Updated conditional to use string comparison for HTTPS check |
| charts/lfx-platform/templates/authelia/https-redirect-httproute.yaml | Fixed HTTPS check and added missing traefik.enabled condition |
![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/lfx-platform/templates/mailpit/https-redirect-httproute.yaml (1)
4-4: Consider parsing the helper output as a boolean instead of string-matching
Relying on a literal string"true"couples every consumer to the helper’s exact output. A more robust pattern is:{{ if and .Values.mailpit.enabled .Values.traefik.enabled (include "lfx-platform.https-enabled" . | fromYaml | default false) -}}
fromYamlcoerces"true"/"false"to booleans, shielding you if the helper ever returns real YAML booleans or gets refactored.charts/lfx-platform/templates/authelia/https-redirect-httproute.yaml (1)
4-4: Avoid duplicating complex conditions across components
Mailpit and Authelia now share almost identical guard clauses. Extracting a helper, e.g.{{- define "lfx-platform.enable-https-redirect" -}} … {{- end -}}, would centralise the logic and cut drift risk.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
charts/lfx-platform/templates/authelia/https-redirect-httproute.yaml(1 hunks)charts/lfx-platform/templates/mailpit/https-redirect-httproute.yaml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: MegaLinter
🔇 Additional comments (2)
charts/lfx-platform/templates/mailpit/https-redirect-httproute.yaml (1)
4-4: Explicit equality check fixes the unintended redirect creation
Changing the condition toeq ... "true"prevents the previous truthy string pit-fall, so the HTTPRoute now renders only when an HTTPS listener really exists.charts/lfx-platform/templates/authelia/https-redirect-httproute.yaml (1)
4-4: Good addition of the Traefik gate and strict HTTPS check
The extra.Values.traefik.enabledguard plus the explicit"true"comparison close the loophole that rendered a redirect with no HTTPS listener.
This change ensures the https redirects for authelia and mailipit
aren't created when there is no https listener defined on the gateway.
andevaluates any non-empty string as true, and the include wasreturning the string "false".
Generated with Cursor
Signed-off-by: Trevor Bramwell [email protected]