PLTF-2953: Use configurable LAMINAR_WEB_HOST for Keycloak redirect URLs by aivong-openhands · Pull Request #711 · OpenHands/OpenHands-Cloud

aivong-openhands · 2026-06-12T21:09:57Z

Description

Fix for issue #710: Laminar Keycloak redirect URL is reset on Laminar pod restarts for customers with custom Laminar domains.

Problem

Customers with unique domains for Laminar (e.g., foo.BASE_DOMAIN instead of laminar.BASE_DOMAIN) experienced issues where the Keycloak redirect URLs were hardcoded to laminar.$WEB_HOST. This meant that on every Laminar pod restart, the configuration would be reset to the default laminar.* subdomain, causing authentication failures for customers with custom domains.

Solution

Introduce a configurable LAMINAR_WEB_HOST environment variable that:

Defaults to laminar.$WEB_HOST for backward compatibility
Can be overridden by setting laminar.frontend.ingress.hostname in the Helm values

Helm Chart Checklist

This change does not modify Helm chart versions or breaking changes
I have verified the changes work with existing values.yaml configurations (backward compatible)
No chart README updates required (no breaking changes or new required values)

Additional Notes

This fix allows customers with custom Laminar domains to properly configure Keycloak redirect URLs by setting laminar.frontend.ingress.hostname in their Helm values. The default behavior remains unchanged for existing customers.

Fixes #710

This PR was drafted by an AI agent on behalf of the user.

Validation

Validated the chart change two ways: a direct helm template render (the pure-Helm path this fix targets) and an end-to-end install serving Laminar on a custom domain.

1. Chart render (pure Helm)

helm template of this branch's chart, varying only laminar.frontend.ingress.hostname:

`laminar.frontend.ingress.hostname`	Rendered `LAMINAR_WEB_HOST`
`lmnr.example.com`	`lmnr.example.com` ✅
(unset)	`laminar.app.example.com` (fallback `laminar.<ingress.host>`) ✅

A custom Laminar hostname now propagates to LAMINAR_WEB_HOST; the default falls back to laminar.<ingress.host> (backward compatible).

2. End-to-end install — custom Laminar domain

Installed on a test cluster with Laminar served at a custom host (lmnr.example.com — neither laminar.* nor analytics.*):

openhands deployment env: LAMINAR_WEB_HOST=lmnr.example.com ✅
Keycloak allhands client automatically had https://lmnr.example.com/api/auth/callback/keycloak in redirectUris (and webOrigins), with no stale laminar.<app-host> entry ✅
Persistence (the Laminar Keycloak redirect URL is reset on Laminar pod restarts #710 regression): restarted the openhands pod, which re-runs the keycloak-config init container that re-renders the realm → the lmnr.example.com redirect URI persisted; it did not reset to laminar.<app-host> ✅
End-to-end Keycloak login to the Laminar UI at lmnr.example.com succeeded: Keycloak accepted redirect_uri=https://lmnr.example.com/api/auth/callback/keycloak → IdP → authenticated ✅

3. Backward compatibility

This PR only changes the realm template's laminar.$WEB_HOST line (→ $LAMINAR_WEB_HOST); the analytics.$WEB_HOST line is untouched. Both existing default paths are therefore unchanged:

Pure Helm, laminar.frontend.ingress.hostname unset: LAMINAR_WEB_HOST falls back to laminar.<ingress.host> — the same value the old hardcoded laminar.$WEB_HOST produced (verified in render [Resolver] Github App implementation #1).
KOTS/Replicated: Laminar is served at analytics.<app-host>, which was (and still is) covered by the untouched analytics.$WEB_HOST entry — so the previously-hardcoded laminar.$WEB_HOST entry was never actually used there. Verified on a live install: login worked and redirectUris contained no laminar.<app-host> entry.

This fix addresses issue #710 where the Laminar Keycloak redirect URL is reset on Laminar pod restarts for customers with custom Laminar domains. Changes: - Add LAMINAR_WEB_HOST environment variable in _env.yaml that defaults to 'laminar.$WEB_HOST' but allows custom hostname via laminar.frontend.ingress.hostname - Update realm template to use $LAMINAR_WEB_HOST instead of hardcoded 'laminar.$WEB_HOST' - Add LAMINAR_WEB_HOST to the envsubst command in keycloak-config-script.yaml - Add tests to verify the new variable is used correctly This allows customers with unique domains for Laminar (e.g., foo.BASE_DOMAIN instead of laminar.BASE_DOMAIN) to configure the redirect URLs correctly without them being reset on pod restarts.

all-hands-bot · 2026-06-12T21:25:09Z

✅ Review complete.

This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here.

all-hands-bot

Code Review: Fix Keycloak redirect URLs for configurable Laminar host

🟡 Acceptable - Solid improvement overall, but one critical bug to fix.

[CRITICAL ISSUES] (Must fix)

[charts/openhands/templates/_env.yaml, Line 418] Bug - Missing and operator: The condition .Values.laminar.enabled .Values.laminar.frontend.ingress.hostname is missing an and keyword between the two expressions. This means:
- If .Values.laminar.enabled is truthy (non-empty), the entire expression evaluates to .Values.laminar.frontend.ingress.hostname (a string)
- Strings are always truthy in Go templates when non-empty, so the condition is always true
- The else branch is effectively dead code
Fix: Add and between the two conditions:
```
{{- if and .Values.laminar.enabled .Values.laminar.frontend.ingress.hostname }}
```

[IMPROVEMENT OPPORTUNITIES] (Should fix)

[charts/openhands/templates/_env.yaml, Line 421-424] Style - Inconsistent else value: The else branch hardcodes laminar. as a prefix. Consider whether this should also be configurable if users want a completely custom domain (e.g., app.laminar.example.com vs laminar.example.com). However, this may be intentional to maintain backward compatibility.

[TESTING GAPS]

No testing gaps found. The new tests:

test_realm_template_uses_laminar_web_host_variable ✓
test_keycloak_config_script_includes_laminar_web_host_in_envsubst ✓

Both tests appropriately verify the key changes without unnecessary mocking.

[RISK ASSESSMENT]

[Overall PR] ⚠️ Risk Assessment: 🟡 MEDIUM

Key factors:

The missing and operator bug means the else branch is never executed, so the default laminar.{{ .Values.ingress.host }} value is always used regardless of the condition
This effectively makes the conditional logic a no-op, which could cause confusion but doesn't break existing deployments
Low risk of regression since the current behavior (always using laminar.$WEB_HOST) was already broken in a different way - it was hardcoded in the template

[VERDICT]

❌ Needs rework: The missing and operator must be fixed before this can be merged. The else branch is currently unreachable.

[KEY INSIGHT]

The core fix is good: externalizing LAMINAR_WEB_HOST as a configurable variable allows customers with custom Laminar domains to properly configure Keycloak redirect URLs. However, the conditional logic bug means only one branch of the if/else will ever execute.

This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation

…talls

aivong-openhands · 2026-06-12T23:05:33Z

@OpenHands bump the openhands chart version

openhands-ai · 2026-06-12T23:05:46Z

I'm on it! aivong-openhands can track my progress at all-hands.dev

all-hands-bot · 2026-06-12T23:06:26Z

✅ Review complete.

This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here.

openhands-ai · 2026-06-12T23:07:20Z

The user is asking me to summarize my work. Let me provide a concise final message summarizing what was done.

Summary

I've completed the requested task from the PR comment:

Task: Bump the OpenHands chart version (as requested in the PR comment)

Completed Actions:

✅ Reviewed the PR PLTF-2953: Use configurable LAMINAR_WEB_HOST for Keycloak redirect URLs #711 changes to understand the scope (modifications to _env.yaml, keycloak-config-script.yaml, allhands-realm-github-provider.json.tmpl, and test files)
✅ Updated charts/openhands/Chart.yaml version from 0.7.48 to 0.7.49
✅ Committed the change with message: "chore: bump openhands chart version to 0.7.49"
✅ Pushed the commit to the remote branch openhands/fix-laminar-keycloak-redirect-url

Result: The version bump has been pushed to PR #711 and will update the pull request automatically.

all-hands-bot

Code Review Summary

🟢 Good taste — Clean, focused fix for a real problem.

Analysis

This PR solves a genuine issue: customers with custom Laminar domains had their Keycloak redirect URLs hardcoded to laminar.$WEB_HOST, which got reset to the default on every pod restart.

The solution is elegant:

Introduces a single LAMINAR_WEB_HOST environment variable with a sensible default
Allows override via laminar.frontend.ingress.hostname for custom domains
Uses the variable in the Keycloak realm template instead of hardcoding
Handles Replicated installs with an unconditional override to the analytics hostname

Files Reviewed

File	Change	Assessment
`charts/openhands/templates/_env.yaml`	Added `LAMINAR_WEB_HOST` env var	✅ Correct logic
`charts/openhands/files/allhands-realm-github-provider.json.tmpl`	Replaced hardcoded laminar URL with `$LAMINAR_WEB_HOST`	✅ Clean substitution
`charts/openhands/templates/keycloak-config-script.yaml`	Added `$LAMINAR_WEB_HOST` to envsubst	✅ Required for substitution
`replicated/openhands.yaml`	Added Replicated override	✅ Handles Replicated-specific case
`scripts/test_keycloak_realm_template.py`	Added tests	✅ Good coverage

Tests

All 10 tests pass, including 2 new tests verifying the realm template uses $LAMINAR_WEB_HOST and the config script includes it in envsubst.

Notes

Existing review comments from earlier commits have been addressed in subsequent commits
The Replicated override is correctly unconditional so redirect URIs stay on the analytics domain even when analytics is disabled

[RISK ASSESSMENT]

Overall PR: ⚠️ Risk Assessment: 🟢 LOW

This is a low-risk change. It introduces a new optional configuration path (backward-compatible by default), only affects customers with custom Laminar domains, has good test coverage, and doesn't modify any core authentication flows.

VERDICT:
✅ Worth merging — Solves the issue cleanly with minimal changes. All tests pass.

KEY INSIGHT:
The fix uses the existing laminar.frontend.ingress.hostname value as the configuration mechanism, avoiding new configuration surface area while allowing customers with custom domains to override the default.

This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation

aivong-openhands · 2026-06-15T20:34:40Z

@OpenHands bump chart version

openhands-ai · 2026-06-15T20:34:54Z

I'm on it! aivong-openhands can track my progress at all-hands.dev

openhands-ai · 2026-06-15T20:36:00Z

Summary

I've completed the requested task from the PR comment:

Task: Bump the OpenHands chart version (as requested in the PR comment)

Completed Actions:

✅ Reviewed the PR context to understand the changes
✅ Updated charts/openhands/Chart.yaml version from 0.7.54 → 0.7.55
✅ Committed the change with a descriptive message
✅ Pushed the commit to the branch openhands/fix-laminar-keycloak-redirect-url

Changes Made:

Single file modified (charts/openhands/Chart.yaml)
Only the chart version field was updated (1 line change)
No extraneous changes

all-hands-bot · 2026-06-15T21:27:32Z

✅ Review complete.

This review was performed through OpenHands Cloud Automation. You can log in and view the conversation here.

all-hands-bot

Code Review Summary

Taste Rating: 🟢 Good taste - Elegant, simple solution

Overview

This PR introduces a configurable LAMINAR_WEB_HOST environment variable to replace hardcoded laminar.$WEB_HOST URLs in Keycloak redirect configuration. The fix enables customers with custom Laminar domains to configure redirect URLs correctly without them being reset on pod restarts.

Changes Reviewed

charts/openhands/files/allhands-realm-github-provider.json.tmpl - Replaces hardcoded laminar.$WEB_HOST with $LAMINAR_WEB_HOST variable in both redirectURIs and webOrigins
charts/openhands/templates/_env.yaml - Adds LAMINAR_WEB_HOST environment variable with intelligent fallback: uses custom hostname if configured, otherwise defaults to laminar.<ingress.host>
charts/openhands/templates/keycloak-config-script.yaml - Adds $LAMINAR_WEB_HOST to the envsubst variable list
replicated/openhands.yaml - Sets LAMINAR_WEB_HOST to analytics hostname for Replicated installations
scripts/test_keycloak_realm_template.py - Adds regression tests to ensure the variable is used correctly

Analysis

✅ Problem Solved: Real issue - customers with custom Laminar domains couldn't configure Keycloak redirect URLs
✅ Minimal Change: Focused diff, only touches what's necessary
✅ Backward Compatible: Default behavior preserved via fallback to laminar.<ingress.host>
✅ Well Tested: Regression tests added for both the realm template and config script
✅ Platform Coverage: Both Helm chart and Replicated deployment paths handled
✅ Earlier Feedback Addressed: The missing and operator issue has been fixed in commit f38b397

Minor Observation

The envsubst command in keycloak-config-script.yaml is becoming lengthy. Consider extracting the variable list to a separate variable or documenting the convention for adding new variables. However, this is not a blocking issue.

[RISK ASSESSMENT]

[Overall PR] ⚠️ Risk Assessment: 🟢 LOW

This is a low-risk change that:

Only affects Keycloak configuration for Laminar authentication
Maintains backward compatibility with fallback behavior
Has test coverage to prevent regressions
Has been reviewed and feedback addressed in previous commits

VERDICT:
✅ Worth merging: Clean, focused fix that solves a real customer problem with minimal risk.

KEY INSIGHT:
The pattern of using environment variables for configuration that needs to survive pod restarts (rather than being hardcoded in ConfigMaps) is the right approach for this type of infrastructure change.

This review was generated by an AI agent (OpenHands) on behalf of the user through OpenHands Automation. View conversation

aivong-openhands · 2026-06-16T16:11:36Z

 name: openhands
 appVersion: cloud-1.38.0
-version: 0.7.54
+version: 0.7.55


Suggested change

version: 0.7.55

version: 0.7.56

@OpenHands bump chart version

I'm on it! aivong-openhands can track my progress at all-hands.dev

Summary of Work Completed

I addressed the comment on PR #711 requesting a chart version bump:

✅ Request Addressed:

Updated charts/openhands/Chart.yaml version from 0.7.55 to 0.7.56

Committed the change with message: "Bump chart version to 0.7.56"

Pushed to the remote branch openhands/fix-laminar-keycloak-redirect-url

Changes Made:

Only the single version line was modified (line 5 in Chart.yaml)

No extraneous changes - minimal, focused update as requested

The chart version bump is now reflected in the PR.

openhands-ai Bot mentioned this pull request Jun 12, 2026

Laminar Keycloak redirect URL is reset on Laminar pod restarts #710

Closed

aivong-openhands requested a review from all-hands-bot June 12, 2026 21:23

all-hands-bot requested changes Jun 12, 2026

View reviewed changes

Comment thread charts/openhands/templates/_env.yaml Outdated

aivong-openhands added 2 commits June 12, 2026 17:39

Fix missing 'and' operator in LAMINAR_WEB_HOST template condition

f38b397

Override LAMINAR_WEB_HOST to the analytics hostname on Replicated ins…

ccd09a2

…talls

aivong-openhands requested a review from all-hands-bot June 12, 2026 23:05

chore: bump openhands chart version to 0.7.49

a1ce1e7

all-hands-bot approved these changes Jun 12, 2026

View reviewed changes

aivong-openhands marked this pull request as ready for review June 12, 2026 23:13

aivong-openhands changed the title ~~Fix: Use configurable LAMINAR_WEB_HOST for Keycloak redirect URLs (Issue #710)~~ PLTF-2953: Use configurable LAMINAR_WEB_HOST for Keycloak redirect URLs Jun 12, 2026

aivong-openhands added 2 commits June 15, 2026 11:04

Merge branch 'main' into openhands/fix-laminar-keycloak-redirect-url

aa67890

Merge branch 'main' into openhands/fix-laminar-keycloak-redirect-url

bbc3829

chore: bump openhands chart version to 0.7.55

25b5cfe

aivong-openhands requested a review from all-hands-bot June 15, 2026 21:26

all-hands-bot approved these changes Jun 15, 2026

View reviewed changes

aivong-openhands requested a review from jlav June 16, 2026 14:25

aivong-openhands mentioned this pull request Jun 16, 2026

Replicated: set LAMINAR_WEB_HOST to the analytics host for Laminar Keycloak redirect URIs #725

Open

4 tasks

Merge branch 'main' into openhands/fix-laminar-keycloak-redirect-url

081f245

aivong-openhands requested review from ak684 and juanmichelini June 16, 2026 16:00

jlav approved these changes Jun 16, 2026

View reviewed changes

aivong-openhands commented Jun 16, 2026

View reviewed changes

Bump chart version to 0.7.56

f2ca6cf

aivong-openhands merged commit e33a2be into main Jun 16, 2026
29 checks passed

aivong-openhands deleted the openhands/fix-laminar-keycloak-redirect-url branch June 16, 2026 16:25

Conversation

aivong-openhands commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Problem

Solution

Helm Chart Checklist

Additional Notes

Validation

1. Chart render (pure Helm)

2. End-to-end install — custom Laminar domain

3. Backward compatibility

Uh oh!

all-hands-bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

all-hands-bot left a comment

Choose a reason for hiding this comment

Code Review: Fix Keycloak redirect URLs for configurable Laminar host

[CRITICAL ISSUES] (Must fix)

[IMPROVEMENT OPPORTUNITIES] (Should fix)

[TESTING GAPS]

[RISK ASSESSMENT]

[VERDICT]

[KEY INSIGHT]

Uh oh!

Uh oh!

aivong-openhands commented Jun 12, 2026

Uh oh!

openhands-ai Bot commented Jun 12, 2026

Uh oh!

all-hands-bot commented Jun 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openhands-ai Bot commented Jun 12, 2026

Summary

Uh oh!

all-hands-bot left a comment

Choose a reason for hiding this comment

Code Review Summary

Analysis

Files Reviewed

Tests

Notes

Uh oh!

aivong-openhands commented Jun 15, 2026

Uh oh!

openhands-ai Bot commented Jun 15, 2026

Uh oh!

openhands-ai Bot commented Jun 15, 2026

Summary

Uh oh!

all-hands-bot commented Jun 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

all-hands-bot left a comment

Choose a reason for hiding this comment

Code Review Summary

Overview

Changes Reviewed

Analysis

Minor Observation

Uh oh!

aivong-openhands Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

aivong-openhands Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

openhands-ai Bot Jun 16, 2026

Choose a reason for hiding this comment

Uh oh!

openhands-ai Bot Jun 16, 2026

Choose a reason for hiding this comment

Summary of Work Completed

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

aivong-openhands commented Jun 12, 2026 •

edited

Loading

all-hands-bot commented Jun 12, 2026 •

edited

Loading

all-hands-bot commented Jun 12, 2026 •

edited

Loading

all-hands-bot commented Jun 15, 2026 •

edited

Loading