Skip to content

fix(chart): specify custom hosts from values - #311

Merged
cmdoret merged 18 commits into
mainfrom
fix/custom-hosts
Sep 16, 2026
Merged

cmdoret merged 18 commits into
mainfrom
fix/custom-hosts

Conversation

@cmdoret

@cmdoret cmdoret commented Sep 15, 2026

Copy link
Copy Markdown
Member

Context

We need the ability to specify the keycloak hostname as hackagon-auth.{baseDomain} but the helpers hard-code some app / auth subdomains for the frontend and keycloak hosts respectively.
As the values expose knobs to set those hostnames, this is highly counter-intuitive.

Summary

The chart no longer hard-codes app. and auth. as the public subdomains. The two values that already looked like hostname work as expected, and are used in templates.

  • Set the app's hostname via frontend.ingress.hosts, Keycloak's via keycloak.hostname.hostname and use them in templates.
  • The chart refuses to render on missing hostnames.
  • The two OIDC issuer settings are derived, as they only dependend on one value.
  • Values use standard Helm templating instead of the chart's own placeholder syntax.

@cmdoret
cmdoret requested a review from sabinem September 16, 2026 09:39
@cmdoret cmdoret self-assigned this Sep 16, 2026
@cmdoret
cmdoret requested a balanced review from Copilot September 16, 2026 09:39

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Empty frontend hosts can bypass validation, and deployment notes can report an incorrect Keycloak scheme.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Updates the Helm chart to derive ingress and OIDC configuration from configurable frontend and Keycloak hosts.

Changes:

  • Replaces custom placeholders with Helm tpl expressions.
  • Derives OIDC issuers, TLS hosts, redirects, and deployment notes.
  • Updates chart version, lint values, documentation, and changelog.
File summaries
File Description
CHANGELOG.md Documents hostname configurability.
helm-chart/Chart.yaml Bumps chart version to 0.4.0.
helm-chart/templates/NOTES.txt Displays configured public hosts.
helm-chart/templates/_helpers.tpl Adds canonical host and issuer helpers.
helm-chart/templates/backend-configmap.yaml Derives backend OIDC configuration.
helm-chart/templates/frontend-configmap.yaml Derives frontend OIDC issuer.
helm-chart/templates/frontend-ingress.yaml Renders templated ingress values.
helm-chart/values.yaml Defines templated hostname defaults.
tools/helm/lint-values.yaml Supplies explicit lint hostnames.
tools/just/helm.just Updates published installation instructions.
tools/keycloak-handover/README.md Documents the required Keycloak hostname.
Review details
  • Files reviewed: 11/11 changed files
  • Comments generated: 2
  • Review effort level: Balanced

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread helm-chart/templates/NOTES.txt Outdated
Comment thread helm-chart/templates/_helpers.tpl Outdated
cmdoret and others added 2 commits September 16, 2026 11:43
Updated Keycloak URL format in NOTES.txt.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

@sabinem sabinem left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cmdoret Looks overall good. I already approve as I have more questions or suggestions for additional comments but nothing blocking.

Comment thread helm-chart/values.yaml
Comment thread helm-chart/values.yaml
@cmdoret
cmdoret merged commit c04507e into main Sep 16, 2026
@cmdoret
cmdoret deleted the fix/custom-hosts branch September 16, 2026 13:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants