fix(plane-ce): https WEB_URL with TLS + Traefik trailing-slash redirects - #283
fix(plane-ce): https WEB_URL with TLS + Traefik trailing-slash redirects#283kai-wei-mo wants to merge 1 commit into
Conversation
…redirects Align CE with enterprise WEB_URL selection (explicit env.web_url, else https when TLS/Traefik is configured). Add Traefik route priorities and a 302 trailing-slash middleware so /god-mode matches the admin SPA basename.
WalkthroughThe Helm chart adds explicit ChangesPlane CE routing configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@charts/plane-ce/templates/config-secrets/app-env.yaml`:
- Around line 57-60: Update the WEB_URL inference condition in the app-env
template so the generated HTTPS URL requires .Values.ingress.enabled in addition
to the existing supported TLS/ingress checks. Keep explicit .Values.env.web_url
precedence unchanged, ensuring external HTTPS routes require that explicit value
when chart ingress is disabled.
In `@charts/plane-ce/templates/traefik-middleware.yaml`:
- Around line 28-30: Update the Traefik middleware regex to match optional query
strings for the god-mode, spaces, and live paths, capture those parameters, and
include the captured query portion in the replacement after adding the trailing
slash so redirects preserve the original parameters.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f7658eec-4588-47bd-b37b-c065ed191a96
📒 Files selected for processing (6)
charts/plane-ce/Chart.yamlcharts/plane-ce/README.mdcharts/plane-ce/templates/config-secrets/app-env.yamlcharts/plane-ce/templates/ingress-traefik.yamlcharts/plane-ce/templates/traefik-middleware.yamlcharts/plane-ce/values.yaml
| {{- if .Values.env.web_url }} | ||
| WEB_URL: {{ .Values.env.web_url | quote }} | ||
| {{- else if and .Values.ingress.appHost (or .Values.ssl.tls_secret_name (and .Values.ssl.createIssuer .Values.ssl.generateCerts) (hasPrefix "traefik" (.Values.ingress.ingressClass | default ""))) }} | ||
| WEB_URL: "https://{{ .Values.ingress.appHost }}" |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
render_web_url() {
helm template review charts/plane-ce \
--set ingress.appHost=plane.example.com \
--set ingress.enabled="$1" \
--set ingress.ingressClass="$2" \
--set ssl.tls_secret_name="$3" \
--set ssl.createIssuer="$4" \
--set ssl.generateCerts="$5" \
--set env.web_url="" |
awk '/name: review-app-vars/{in_map=1} in_map && /WEB_URL:/{print; exit}'
}
test "$(render_web_url false traefik "" false false)" = 'WEB_URL: "http://plane.example.com"'
test "$(render_web_url true traefik "" false false)" = 'WEB_URL: "https://plane.example.com"'Repository: makeplane/helm-charts
Length of output: 197
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== locate relevant files =="
git ls-files | rg 'charts/plane-ce/(templates/(ingress-traefik|config-secrets/app-env)\.yaml|values\.yaml)$'
echo
echo "== relevant snippets =="
for f in charts/plane-ce/templates/ingress-traefik.yaml charts/plane-ce/templates/config-secrets/app-env.yaml charts/plane-ce/values.yaml; do
if [ -f "$f" ]; then
echo "--- $f ---"
wc -l "$f"
sed -n "1,110p" "$f"
fi
done
echo
echo "== check helm availability =="
command -v helm || trueRepository: makeplane/helm-charts
Length of output: 9902
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== templates/gate occurrences for ingress.enabled, ssl, and ingressClass =="
rg -n "ingress\.enabled|ingressClass|createIssuer|generateCerts|tls_secret_name|\{\{- *if .*ingress" charts/plane-ce/templates charts/plane-ce/functions 2>/dev/null || true
echo
echo "== all ingress-related templates =="
git ls-files 'charts/plane-ce/templates/*ingress*'
for f in $(git ls-files 'charts/plane-ce/templates/*ingress*' 'charts/plane-ce/templates/config-secrets/app-env.yaml'); do
if [ -f "$f" ]; then
echo "--- $f ---"
wc -l "$f"
sed -n '1,120p' "$f"
fi
doneRepository: makeplane/helm-charts
Length of output: 15004
🌐 Web query:
Helm template Go template hasPrefix string function syntax hasPrefix "traefik" .Values.ingress.ingressClass
💡 Result:
In Helm templates, the hasPrefix function is provided by the Sprig library [1][2]. It tests whether a string begins with a specified prefix and returns a boolean value [1][3]. Syntax The function takes two arguments: the prefix to search for, and the string to search within [1][2]. {{ hasPrefix "prefix" "your-string" }} Alternatively, it can be used in a pipeline: {{ "your-string" | hasPrefix "prefix" }} Example with.Values.ingress.ingressClass If you want to check if the ingressClass value starts with a specific string (for example, "traefik-"), you can use it in your Helm template as follows: {{- if hasPrefix "traefik-".Values.ingress.ingressClass }} # Logic to execute if it matches {{- end }} Or using the pipeline style: {{- if.Values.ingress.ingressClass | hasPrefix "traefik-" }} # Logic to execute if it matches {{- end }} Important Notes 1. Availability: Helm templates include Sprig functions by default [1]. You do not need to register them manually, unlike standard Go text/template which does not include this function natively [4][5]. 2. Traefik Context: While Traefik configuration often deals with ingressClass, ensure that the variable you are checking (.Values.ingress.ingressClass) is defined and of type string to avoid template execution errors [6][7]. If the value might be nil or empty, you may want to provide a default value: {{- if.Values.ingress.ingressClass | default "" | hasPrefix "traefik-" }}
Citations:
- 1: https://helm.sh/docs/chart_template_guide/function_list
- 2: http://masterminds.github.io/sprig/strings.html
- 3: https://medium.com/dailydevopstips/how-to-use-functions-and-pipelines-in-helm-891bf666df23
- 4: https://pkg.go.dev/text/template
- 5: https://stackoverflow.com/questions/76467951/golang-text-template-startswith-function
- 6: https://github.com/traefik/traefik-helm-chart/blob/e66342de/traefik/VALUES.md
- 7: https://github.com/traefik/traefik-helm-chart/blob/249c289ad89d35ac6cd9c3cd90afe68f2fb376bf/traefik/values.yaml
Gate HTTPS inference on chart-supported TLS/ingress.
charts/plane-ce/templates/config-secrets/app-env.yaml can emit WEB_URL: "https://..." when .Values.ingress.enabled=false and either .Values.ssl.tls_secret_name is set or .Values.ingress.ingressClass starts with traefik. Only charts/plane-ce/templates/ingress-traefik.yaml and the standard ingress use .Values.ingress.enabled, so an external ingress can receive a chart-generated HTTPS URL for a non-chart endpoint.
Require .Values.ingress.enabled or set env.web_url explicitly for external HTTPS routes.
Proposed fix
- {{- else if and .Values.ingress.appHost (or .Values.ssl.tls_secret_name (and .Values.ssl.createIssuer .Values.ssl.generateCerts) (hasPrefix "traefik" (.Values.ingress.ingressClass | default ""))) }}
+ {{- else if and .Values.ingress.enabled .Values.ingress.appHost (or .Values.ssl.tls_secret_name (and .Values.ssl.createIssuer .Values.ssl.generateCerts) (hasPrefix "traefik" (.Values.ingress.ingressClass | default ""))) }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {{- if .Values.env.web_url }} | |
| WEB_URL: {{ .Values.env.web_url | quote }} | |
| {{- else if and .Values.ingress.appHost (or .Values.ssl.tls_secret_name (and .Values.ssl.createIssuer .Values.ssl.generateCerts) (hasPrefix "traefik" (.Values.ingress.ingressClass | default ""))) }} | |
| WEB_URL: "https://{{ .Values.ingress.appHost }}" | |
| {{- if .Values.env.web_url }} | |
| WEB_URL: {{ .Values.env.web_url | quote }} | |
| {{- else if and .Values.ingress.enabled .Values.ingress.appHost (or .Values.ssl.tls_secret_name (and .Values.ssl.createIssuer .Values.ssl.generateCerts) (hasPrefix "traefik" (.Values.ingress.ingressClass | default ""))) }} | |
| WEB_URL: "https://{{ .Values.ingress.appHost }}" |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@charts/plane-ce/templates/config-secrets/app-env.yaml` around lines 57 - 60,
Update the WEB_URL inference condition in the app-env template so the generated
HTTPS URL requires .Values.ingress.enabled in addition to the existing supported
TLS/ingress checks. Keep explicit .Values.env.web_url precedence unchanged,
ensuring external HTTPS routes require that explicit value when chart ingress is
disabled.
| # Match full request URL; keep host/scheme, add trailing slash. | ||
| regex: "^(https://[^/]+)/(god-mode|spaces|live)$" | ||
| replacement: "${1}/${2}/" |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
base="${BASE_URL%/}"
for path in /god-mode /spaces /live; do
headers="$(curl -ksS -D - -o /dev/null --max-redirs 0 "$base${path}?probe=1")"
status="$(printf '%s\n' "$headers" | awk 'NR == 1 {print $2}')"
location="$(printf '%s\n' "$headers" | awk -F': ' 'tolower($1) == "location" {sub("\r$", "", $2); print $2; exit}')"
test "$status" = "302"
test "$location" = "$base${path}/?probe=1"
doneRepository: makeplane/helm-charts
Length of output: 200
🏁 Script executed:
#!/usr/bin/env bash
set -euo pipefail
echo "== candidate files =="
git ls-files | rg '(^|/)traefik-middleware\.yaml$|traefik|middleware' || true
echo "== file excerpt =="
if [ -f charts/plane-ce/templates/traefik-middleware.yaml ]; then
cat -n charts/plane-ce/templates/traefik-middleware.yaml | sed -n '1,120p'
fi
echo "== route references =="
rg -n "god-mode|spaces|live|RedirectRegex|redirect" charts/plane-ce/templates || trueRepository: makeplane/helm-charts
Length of output: 6429
🌐 Web query:
Traefik RedirectRegex request URL RequestURI include query string documentation
💡 Result:
In Traefik, the RedirectRegex middleware performs redirection based on regex matching and replacement [1][2]. Regarding the inclusion of query strings, the behavior depends on the version and implementation details of how the redirect is handled: 1. Query String Handling: By default, the RedirectRegex middleware does not automatically append the original request's query string to the new location if the replacement URL is treated as an absolute URL [3][4]. When a redirect occurs, the final destination is determined by the replacement string provided in your configuration [1][5]. 2. Recent Improvements: Traefik has addressed issues where full URL replacements would drop the incoming query string [4]. In updated versions, logic has been introduced to carry over the original request's query string if the replacement URL does not already contain a query string [4]. 3. Implementation Notes: - Regex Capturing: The regex option is used to match and capture elements from the request URL, and the replacement option defines the new target URL [1][5]. - Variable Expansion: Use the ${1} syntax in the replacement field to reference captured groups [1][5]. When defining these in YAML, remember that certain characters might need double escaping [1][5]. - Security: Historically, there was an issue where query parameters could be incorrectly evaluated as template functions; this has been patched to ensure the middleware does not attempt to execute template logic from URL parameters [6]. If you find that your query strings are being dropped, ensure you are using a recent version of Traefik, as fixes for absolute-URL redirection parity with tools like NGINX have been implemented [4]. If the query string is not preserved automatically, you may need to explicitly include the query parameters in your regex/replacement logic or verify that your specific routing configuration allows for query string passthrough [3][4].
Citations:
- 1: https://doc.traefik.io/traefik/reference/routing-configuration/http/middlewares/redirectregex/
- 2: https://doc.traefik.io/traefik/master/reference/routing-configuration/http/middlewares/redirectregex/
- 3: https://stackoverflow.com/questions/77310827/preserve-query-params-duiring-the-redirect-with-traefik
- 4: Preserve request query on absolute-URL redirect traefik/traefik#13020
- 5: https://doc.traefik.io/traefik/v3.7/reference/routing-configuration/http/middlewares/redirectregex/
- 6: Parameters in query directly invoke template functions via redirect regex middleware traefik/traefik#7513
Preserve query parameters in the trailing-slash redirect.
The middleware regex must match /god-mode?next=%2F, /spaces?probe=1, and /live?x=1. If the query string is captured as an optional group, include it in the replacement so the Location still carries the original parameters after the added trailing slash.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@charts/plane-ce/templates/traefik-middleware.yaml` around lines 28 - 30,
Update the Traefik middleware regex to match optional query strings for the
god-mode, spaces, and live paths, capture those parameters, and include the
captured query portion in the replacement after adding the trailing slash so
redirects preserve the original parameters.
Source: MCP tools
1 participant
Summary
Self-hosting Plane CE behind Traefik/TLS currently forces
WEB_URLtohttp://…(unlikeplane-enterprise), which breaks CORS/auth flows when the app is only reachable over HTTPS. Traefik routes also lack explicit priorities and do not redirect/god-mode→/god-mode/, so the admin SPA loads HTML but React Router fails (basename="/god-mode/").This change:
WEB_URLlike enterprise: honorenv.web_urlwhen set; otherwise use https when TLS is configured (ssl.tls_secret_name, cert-manager certs, or Traefik ingress), else http/god-mode,/api, … beat the catch-all//god-mode,/spaces, and/live(temporary redirect to avoid sticky browser caches of bad Locations)env.web_urland bumps the chart to1.6.3Test plan
helm templatewithingressClass=traefik+ssl.tls_secret_name→WEB_URL: https://…helm templatewithingressClass=nginxand no TLS →WEB_URL: http://…helm templatewithenv.web_url=https://custom.example.com→ override winsGET /god-mode→302tohttps://host/god-mode/(no:3000in Location)/god-mode/serves admin;/serves webOut of scope / related
Frontend nginx still may emit
:3000in redirects withoutport_in_redirect off— that lives inmakeplane/planeimages (see makeplane/plane#8814 / PR #8826), not this chart.Summary by CodeRabbit
New Features
/god-mode,/spaces, and/liveroutes.Documentation
Chores