chore: strict validation performance part two#14269
Conversation
generated matchers still take 1 full-route Envoy call. (This does not solve the 5000 valid-route benchmark problem. It helps invalid matcher cases, but the big issue remains: valid routes are still validated one-by-one.) Signed-off-by: David L. Chandler <[email protected]>
Signed-off-by: David L. Chandler <[email protected]>
Signed-off-by: David L. Chandler <[email protected]>
Signed-off-by: David L. Chandler <[email protected]>
Signed-off-by: David L. Chandler <[email protected]>
| if matcher == nil { | ||
| return fmt.Errorf("%s is missing", field) | ||
| } | ||
| if err := regexutils.CheckRegexString(matcher.GetRegex()); err != nil { |
There was a problem hiding this comment.
envoy regex has default max program size of 100 and can be changed but it doesn't look like change it in the Matcher setting. I have see people complain about this. Can we and should we change max program size here?
There was a problem hiding this comment.
I'm not familiar with this but this PR will still call out to Envoy when strict validation is in use, and catch it that way. Added a comment in 1f2c238
Signed-off-by: David L. Chandler <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR reduces the cost of strict HTTPRoute validation by moving cheap matcher validation in-process and batching full-route Envoy validation per virtual host, with a fallback to per-route validation to preserve existing “drop vs replace” behavior for bad routes. It also tightens the AttachedRoutes load test to measure/require validation metrics and allows exercising strict validation through the local Helm install path.
Changes:
- Add pre-Envoy validation for common generated matcher shapes (notably regex validity) and batch strict full-route Envoy validation across vhost routes, falling back to per-route isolation on failure.
- Update load-test metrics scraping to proxy to a ready controller pod, and fail the load test when metrics are unavailable.
- Allow E2E installs to set strict validation via
VALIDATION_MODE-> Helmvalidation.level.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/e2e/tests/kgateway_test.go | Adds env-controlled Helm value wiring to run E2E installs in strict validation mode. |
| test/e2e/features/loadtesting/loadtest_manager.go | Fetches controller metrics by proxying to a ready pod with retries (used for validation metrics collection). |
| test/e2e/features/loadtesting/attachedroutes_suite.go | Improves incremental-route timing/error reporting and makes validation metrics required for the load test. |
| pkg/kgateway/translator/irtranslator/validate.go | Introduces pre-Envoy matcher checks and batched full-route validation helpers for strict mode. |
| pkg/kgateway/translator/irtranslator/route.go | Batches strict validation per virtual host; isolates invalid routes on batch failure while preserving replacement semantics. |
| pkg/kgateway/translator/irtranslator/route_test.go | Adds unit coverage for strict batching behavior and matcher precheck behavior. |
| var lastErr error | ||
| timeout := time.After(60 * time.Second) | ||
| ticker := time.NewTicker(500 * time.Millisecond) | ||
| defer ticker.Stop() |
Signed-off-by: David L. Chandler <[email protected]>
) Signed-off-by: David L. Chandler <[email protected]>
Signed-off-by: David L. Chandler <[email protected]>
Description
Strict validation can become expensive when a Gateway has many valid HTTPRoutes, because each route may require an Envoy validation invocation. This PR reduces that cost while preserving the existing safety behavior for bad routes.
What changed:
VALIDATION_MODE=strictto exercise strict validation through the local chart install path.Change Type
/kind fix
/kind cleanup
Changelog
Additional Notes
This PR does not add new validation metrics. It consumes the metrics from #14026 so reviewers can see how much Envoy validation work strict mode performs during the AttachedRoutes load test.