Skip to content

Commit 98e8503

Browse files
mikeyrcampclaude
andcommitted
fix(llm-request-router): validate certificate SANs against the PKI role
The OpenBao signing role this chart provisions is created with allow_subdomains=true and allow_bare_domains=false. A certificate SAN outside pki.allowedDomains therefore renders cleanly and is then rejected by OpenBao at issuance, surfacing as a cert-manager failure with no pointer back to the value that caused it. This was the one coupling in the LLM PKI path with no render-time guard. Enabling backend routing with the LLM addon made it materially easier to hit. The chart appends the pod-hostname wildcard to the Certificate whenever backend routing is on, so every LLM deployment with PKI now requests an in-cluster wildcard. An operator whose allowedDomains omits cluster.local previously rendered and installed; now the request would be refused at issuance. Validate coverage at render instead, applying the role's own rules: a name is covered when it is a strict subdomain of an allowed domain, a wildcard is additionally covered when it sits directly on one, and a bare domain is never covered because the role refuses bare issuance. The tests pin the cases that must NOT fail as well as those that must. A guard that is too strict here would block valid deployments, which is worse than the trap it replaces. Three of them were initially passing for the wrong reason and were corrected: the near-miss suffix case had the pattern inverted, and the bare-domain case was failing on the appended wildcard rather than on the bare name, so the bare rule was never exercised. Relates to #689 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Signed-off-by: Mike Camp <mcamp@nvidia.com>
1 parent f5953ad commit 98e8503

3 files changed

Lines changed: 108 additions & 0 deletions

File tree

deploy/helm/llm-request-router/llm-request-router/templates/_helpers.tpl

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,48 @@ comparing suffixes.
174174
{{- end -}}
175175
{{- end -}}
176176

177+
{{/*
178+
The OpenBao signing role this chart provisions is created with
179+
allow_subdomains=true, allow_bare_domains=false, and allow_wildcard_certificates=true.
180+
A SAN outside allowed_domains renders cleanly and then fails at issuance, so
181+
check it here instead. Only applies when this chart owns both the Certificate
182+
and the role.
183+
184+
Coverage, matching the role flags:
185+
name.sub.domain covered when it is a strict subdomain of an allowed domain
186+
*.sub.domain same, and additionally when the wildcard sits directly on an
187+
allowed domain
188+
domain never covered on its own, because bare issuance is refused
189+
*/}}
190+
{{- define "llm-request-router.validatePkiAllowedDomains" -}}
191+
{{- $pki := .Values.llmRequestRouter.pki | default dict -}}
192+
{{- $certificate := .Values.llmRequestRouter.certificate | default dict -}}
193+
{{- if and $pki.enabled $certificate.enabled -}}
194+
{{- $configured := $pki.allowedDomains | default "" | toString -}}
195+
{{- $allowed := splitList "," $configured -}}
196+
{{- $dnsNames := include "llm-request-router.effectiveCertificateDnsNames" . | fromJsonArray -}}
197+
{{- range $dnsName := $dnsNames -}}
198+
{{- $name := $dnsName | toString | lower | trim -}}
199+
{{- $isWildcard := hasPrefix "*." $name -}}
200+
{{- $base := trimPrefix "*." $name -}}
201+
{{- $covered := false -}}
202+
{{- range $allowedDomain := $allowed -}}
203+
{{- $domain := $allowedDomain | toString | lower | trim -}}
204+
{{- if $domain -}}
205+
{{- if hasSuffix (printf ".%s" $domain) $base -}}
206+
{{- $covered = true -}}
207+
{{- else if and $isWildcard (eq $base $domain) -}}
208+
{{- $covered = true -}}
209+
{{- end -}}
210+
{{- end -}}
211+
{{- end -}}
212+
{{- if not $covered -}}
213+
{{- fail (printf "certificate DNS name %q is not covered by llmRequestRouter.pki.allowedDomains %q. The OpenBao signing role uses allow_subdomains=true and allow_bare_domains=false, so cert-manager issuance would be rejected after a successful render. Add a covering suffix, for example cluster.local for in-cluster names." $dnsName $configured) -}}
214+
{{- end -}}
215+
{{- end -}}
216+
{{- end -}}
217+
{{- end -}}
218+
177219
{{- define "llm-request-router.serviceAccountName" -}}
178220
{{- if .Values.llmRequestRouter.serviceAccount.create }}
179221
{{- default (include "llm-request-router.fullname" .) .Values.llmRequestRouter.serviceAccount.name }}

deploy/helm/llm-request-router/llm-request-router/templates/certificate.yaml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
{{ $certificate := .Values.llmRequestRouter.certificate | default dict -}}
1717
{{- if $certificate.enabled }}
1818
{{- include "llm-request-router.validateCertificateDnsNames" . }}
19+
{{- include "llm-request-router.validatePkiAllowedDomains" . }}
1920
{{- $issuerRef := $certificate.issuerRef | default dict -}}
2021
{{- $dnsNames := include "llm-request-router.effectiveCertificateDnsNames" . | fromJsonArray }}
2122
{{- if empty $dnsNames -}}

deploy/helm/llm-request-router/scripts/check-pki-render.sh

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,4 +430,69 @@ grep -Fq \
430430
'llmRequestRouter.tls.mode must be certManager or existingSecret, got "externalSecret"' \
431431
"${invalid_mode_error}" || fail "unknown mode render did not return the expected guard message"
432432

433+
# The OpenBao signing role is created with allow_subdomains=true and
434+
# allow_bare_domains=false, so a SAN outside allowed_domains renders cleanly and
435+
# then fails at cert-manager issuance. Catch it at render instead. These cases
436+
# pin the coverage rules, including the ones that must NOT fail: a wrong guard
437+
# here would block valid deployments, which is worse than the trap it replaces.
438+
assert_allowed_domains_case() {
439+
local description="$1"
440+
local expectation="$2"
441+
local allowed_domains="$3"
442+
local dns_name="$4"
443+
local case_values="${tmp_dir}/allowed-domains-values.yaml"
444+
local case_error="${tmp_dir}/allowed-domains.err"
445+
446+
cat > "${case_values}" <<EOF
447+
llmRequestRouter:
448+
backendRouter:
449+
enabled: true
450+
image:
451+
repository: nvcf/stargate-k8s-router
452+
certificate:
453+
enabled: true
454+
issuerRef:
455+
name: nvcf-openbao-pki
456+
dnsNames: ["${dns_name}"]
457+
tls:
458+
certPath: /etc/stargate/tls/tls.crt
459+
keyPath: /etc/stargate/tls/tls.key
460+
quicInsecure: false
461+
pki:
462+
enabled: true
463+
allowedDomains: "${allowed_domains}"
464+
image:
465+
repository: nvcf-openbao-migrations
466+
tag: "1"
467+
EOF
468+
469+
if helm template llm-request-router ./llm-request-router \
470+
--namespace nvcf \
471+
--values ./llm-request-router/values.yaml \
472+
--values "${case_values}" \
473+
> /dev/null 2> "${case_error}"; then
474+
[ "${expectation}" = "pass" ] || fail "${description} unexpectedly rendered"
475+
else
476+
[ "${expectation}" = "fail" ] || fail "${description} unexpectedly failed to render"
477+
grep -Fq "is not covered by llmRequestRouter.pki.allowedDomains" "${case_error}" ||
478+
fail "${description} did not return the allowed-domains guard message"
479+
fi
480+
}
481+
482+
# Must render: these are valid deployments.
483+
assert_allowed_domains_case "documented customer-domain plus cluster.local" \
484+
pass "example.com,cluster.local" "llm-request-router.nvcf.svc.cluster.local"
485+
assert_allowed_domains_case "whitespace around the comma separators" \
486+
pass " example.com , cluster.local " "llm-request-router.nvcf.svc.cluster.local"
487+
assert_allowed_domains_case "allowed domain deeper in the suffix" \
488+
pass "svc.cluster.local" "llm-request-router.nvcf.svc.cluster.local"
489+
490+
# Must fail: issuance would be rejected.
491+
assert_allowed_domains_case "no overlap with the certificate names" \
492+
fail "example.com" "llm-request-router.nvcf.svc.cluster.local"
493+
assert_allowed_domains_case "name that ends with the domain but is not a subdomain" \
494+
fail "cluster.local" "evilcluster.local"
495+
assert_allowed_domains_case "bare domain, which the role refuses to issue" \
496+
fail "llm-request-router-headless.nvcf.svc.cluster.local" "llm-request-router-headless.nvcf.svc.cluster.local"
497+
433498
echo "PKI render checks passed"

0 commit comments

Comments
 (0)