feat: make ingress PathType configurable via Helm env var, default to Prefix#126
feat: make ingress PathType configurable via Helm env var, default to Prefix#126badal773 wants to merge 1 commit intonabsul:mainfrom
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR makes the Ingress pathType configurable via a new Helm environment variable (defaulting to Prefix) to align with stricter NGINX Ingress Controller validation rules.
- Added
kcertPathTypetovalues.yamlwith a default ofPrefix - Passed
KCERT_PATH_TYPEinto the deployment via an environment variable - Bumped chart version to
1.0.8 - Updated
HttpChallengeProviderto use an environment‐drivenPathType
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| charts/kcert/values.yaml | Introduced kcertPathType setting with default |
| charts/kcert/templates/070-Deployment.yaml | Inject KCERT_PATH_TYPE into container env |
| charts/kcert/Chart.yaml | Bumped chart version to 1.0.8 |
| Challenge/HttpChallengeProvider.cs | Replaced hardcoded PathType with GetPathType |
Comments suppressed due to low confidence (2)
charts/kcert/values.yaml:43
- [nitpick] Add a comment above
kcertPathTypeinvalues.yamlexplaining its purpose and allowed values (Prefix,Exact,ImplementationSpecific) for clarity.
kcertPathType: Prefix
Challenge/HttpChallengeProvider.cs:82
- Add unit tests for
GetPathTypeto cover both the default fallback and an overridden environment variable scenario, ensuring the new configurable path type logic is verified.
private static string GetPathType()
| private static string GetPathType() | ||
| { | ||
| return Environment.GetEnvironmentVariable("KCERT_PATH_TYPE") ?? "Prefix"; |
There was a problem hiding this comment.
[nitpick] Extract the default path type ("Prefix") into a shared constant or configuration field to avoid duplicating the literal and keep it in sync with the Helm chart default.
| private static string GetPathType() | |
| { | |
| return Environment.GetEnvironmentVariable("KCERT_PATH_TYPE") ?? "Prefix"; | |
| private const string DefaultPathType = "Prefix"; | |
| private static string GetPathType() | |
| { | |
| return Environment.GetEnvironmentVariable("KCERT_PATH_TYPE") ?? DefaultPathType; |
|
|
||
| private static string GetPathType() | ||
| { | ||
| return Environment.GetEnvironmentVariable("KCERT_PATH_TYPE") ?? "Prefix"; |
There was a problem hiding this comment.
Consider validating the environment variable against allowed Kubernetes PathType values (Prefix, Exact, ImplementationSpecific) to fail fast on invalid configuration.
Description of Change
In recent versions of the NGINX Ingress Controller, stricter validation rules have been introduced for
ingress.spec.rules.http.paths.pathwhenpathTypeis set toPrefixorExact. These rules enforce that thepathmust://,_, and-If a path contains characters outside this set (e.g., for rewrite purposes), the
pathTypemust be set toImplementationSpecific. This validation is enforced by the Admission Webhook, and non-compliant ingress resources will be denied admission.To ensure compatibility and prevent potential deployment failures, we are explicitly setting the default
pathTypetoPrefixvia an environment variable in the Helm chart. This default is safe for standard use cases where paths comply with the new restrictions.For advanced use cases involving rewrites or non-standard characters in paths, users should explicitly override the
pathTypetoImplementationSpecific.