test(bdd): fix secure multi-region coverage - #1322
Conversation
Relates to #1307 Signed-off-by: Mike Camp <mcamp@nvidia.com>
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (1)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Comment |
|
Independent review of the companion diff found no Critical, Important, or Minor issues. The change addresses all four reported classes: certificate SAN configuration, canonical router identity validation with dashed-IP alias rejection, equivalent watch-alias wiring in both cluster contexts, and narrowly scoped revive line-length handling. Validation passed for the focused wiring/alias tests, the short Go suite, go vet, configured revive lint, shell syntax, and diff hygiene. The live multi-region BDD run remains a separate gate. |
Relates to #1307 Signed-off-by: Mike Camp <mcamp@nvidia.com>
|
Live BDD follow-up: the first multi-region run reached the Region B HTTPS Watch request and exposed a missing hostname on the port-50071 Gateway Certificate. Commit The feature now reads the issued Gateway Certificate and requires the Region B SAN before it attempts Watch. Focused wiring, the full short BDD suite, |
|
Integration note from independent review: #1322's current #1310 base predates merged #1300, so it does not yet include the chart template that consumes Safe order: incorporate #1322 into #1310, rebase #1310 onto current |
|
Independent review of current head 2607067 found no Critical, Important, or Minor issues. Focused Gateway-SAN wiring/readback checks, multi-region alias and identity tests, the short BDD suite, Go vet, configured revive lint, shell syntax, and diff hygiene passed. The documented SAN precheck is not a final live pass. CodeRabbit skipped review because this PR targets a stacked, non-default base. Readiness remains gated on incorporating #1322 into #1310, rebasing #1310 onto current main with #1300, and rerunning the live multi-region BDD. No merge was performed. |
|
Final sanitized live acceptance result for a validation tree combining exact #1321 head
Task cleanup is complete and zero task-owned clusters remain. #1322 has been merged into its stacked parent #1310, not into |
Signed-off-by: Mike Camp <mcamp@nvidia.com>
TL;DR
Address the unresolved review findings on #1310 so the secure multi-region BDD scenario validates the real certificate input, canonical router identities, and cluster-local watch alias wiring.
Additional Details
addons.llm.pki.dnsNames, which is the stack field consumed by the request-router certificate.grpcurlresponse and reject dashed pod-IP aliases.region-b-watchService and Endpoints alias in both the control-plane and compute contexts, with a non-live behavioral test for both manifests.Secure multi-region registration: before
Secure multi-region registration: after
This Pull Request is stacked on #1310 and targets its feature branch.
Integration order
The current #1310 base predates merged #1300 and does not yet contain the chart template that consumes
addons.llm.requestRouter.grpcTls.dnsNames. The safe integration and validation order is:main, including feat(self-managed): secure LLM gRPC ingress #1300.Until those steps are complete, the new Gateway SAN input and Certificate readback are not an effective live validation path. No final live pass is claimed here.
For the Reviewer
Please focus on the Region B identity assertion and the two-context alias application in
tests/bdd/scripts/install-llm-region-b.sh.For QA
Validated on the development VM:
go test -short -count=1 ./...go vet ./...bash -n tests/bdd/scripts/install-llm-region-b.shgit diff --checkThe configured full lint command still reports two pre-existing staticcheck capitalization findings in
tests/bdd/dsl/manifests.go; this Pull Request does not modify that file. A live destructive k3d run was not performed for this review-only companion.Issues
Relates to #1307
Dependencies
No new or updated third-party dependencies. No license or NOTICE changes are required.
Checklist