feat: make kratos-selfservice-ui-node probes configurable#871
feat: make kratos-selfservice-ui-node probes configurable#871bobbyiliev wants to merge 1 commit intoory:masterfrom
Conversation
…tomReadinessProbe
📝 WalkthroughWalkthroughHelm chart version is bumped from 0.61.1 to 0.61.2, and optional custom liveness and readiness probe configurations are added to allow users to override default HTTP probe definitions with custom implementations. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Pull request overview
Adds configurable Kubernetes probes to the kratos-selfservice-ui-node Helm chart so deployments using HTTPS (via TLS_CERT_PATH / TLS_KEY_PATH) can replace the default HTTP probes and avoid crash loops.
Changes:
- Introduces
deployment.customLivenessProbeanddeployment.customReadinessProbevalues (default{}) to fully override the probes. - Updates the Deployment template to use the custom probes when provided, otherwise keeps existing HTTP probe behavior.
- Bumps chart version and regenerates README values documentation.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| helm/charts/kratos-selfservice-ui-node/values.yaml | Adds new customLivenessProbe / customReadinessProbe values with documentation and examples. |
| helm/charts/kratos-selfservice-ui-node/templates/deployment.yaml | Conditionally renders custom probes when non-empty, preserving defaults otherwise. |
| helm/charts/kratos-selfservice-ui-node/README.md | Updates chart version badge and documents new values (helm-docs output). |
| helm/charts/kratos-selfservice-ui-node/Chart.yaml | Bumps chart version from 0.61.1 to 0.61.2. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
helm/charts/kratos-selfservice-ui-node/templates/deployment.yaml (1)
88-105: Add template regression coverage for both branches.Given these are conditional render paths, adding chart tests (default probe path + custom probe path) would reduce risk of future template regressions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@helm/charts/kratos-selfservice-ui-node/templates/deployment.yaml` around lines 88 - 105, Add chart tests that exercise both conditional branches in the deployment template for liveness/readiness probes: write one test that renders the template with no deployment.customLivenessProbe/customReadinessProbe set and asserts the rendered container has httpGet probes using {{ .Values.basePath }}/health/alive and /health/ready (and port named "http"), and a second test that supplies deployment.customLivenessProbe and deployment.customReadinessProbe values and asserts the rendered YAML contains the custom probe fields (e.g., the full toYaml output you expect). Target the Helm template named in the diff (deployment.yaml) and use your chart-testing framework (helm unittest or chart-testing) to render and assert both the default-path branch and the custom-probe branch to prevent regressions.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@helm/charts/kratos-selfservice-ui-node/templates/deployment.yaml`:
- Around line 88-105: Add chart tests that exercise both conditional branches in
the deployment template for liveness/readiness probes: write one test that
renders the template with no deployment.customLivenessProbe/customReadinessProbe
set and asserts the rendered container has httpGet probes using {{
.Values.basePath }}/health/alive and /health/ready (and port named "http"), and
a second test that supplies deployment.customLivenessProbe and
deployment.customReadinessProbe values and asserts the rendered YAML contains
the custom probe fields (e.g., the full toYaml output you expect). Target the
Helm template named in the diff (deployment.yaml) and use your chart-testing
framework (helm unittest or chart-testing) to render and assert both the
default-path branch and the custom-probe branch to prevent regressions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d369d0fd-5623-45c2-be5c-ade6c799a75b
📒 Files selected for processing (4)
helm/charts/kratos-selfservice-ui-node/Chart.yamlhelm/charts/kratos-selfservice-ui-node/README.mdhelm/charts/kratos-selfservice-ui-node/templates/deployment.yamlhelm/charts/kratos-selfservice-ui-node/values.yaml
Description
The UI supports HTTPS via
TLS_CERT_PATH/TLS_KEY_PATH, but the chart's probes are hardcoded HTTP, so the pod crash-loops once TLS is turned on.This adds
deployment.customLivenessProbeanddeployment.customReadinessProbe, same pattern as the kratos chart (deployment-kratos.yaml). Both default to{}, so existing deployments are unchanged.Verified with
helm lintandhelm templatein default and override modes.Checklist
Summary by CodeRabbit
New Features
Chores