Skip to content

Conversation

@shults
Copy link
Contributor

@shults shults commented Nov 7, 2025

Description

POC

Related Issue

Motivation and Context

How This Has Been Tested

Screenshots (if appropriate)

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Refactoring or add test (improvements in base code or adds test coverage to functionality)

Checklist

  • I ensured that the documentation is up to date
  • I explained why this PR updates go.mod in detail with reasoning why it's required
  • I would like a code coverage CI quality gate exception and have explained why

Ticket Details

TT-15856
Status In Code Review
Summary [PoC] Unify path matching algorithm for all middleware

Generated at: 2025-11-13 12:47:43

@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2025

API Changes

--- prev.txt	2025-11-13 12:48:08.151673522 +0000
+++ current.txt	2025-11-13 12:47:58.083591464 +0000
@@ -12249,6 +12249,9 @@
 	GoPlugin
 	PersistGraphQL
 	RateLimit
+
+	OasMock
+	OasValidate
 )
     Enums representing the various statuses for a VersionInfo Path match during
     a proxy request

@probelabs
Copy link

probelabs bot commented Nov 7, 2025

🔍 Code Analysis Results

This PR is a Proof of Concept (PoC) aimed at unifying the path matching algorithm for all middleware within the Tyk gateway. It refactors the OAS-based mock response and request validation middlewares to use a new, centralized path matching and parameter extraction engine. This change is intended to improve consistency, reduce code duplication, and simplify the implementation of path-based middleware.

A new package, internal/paramextractor, is introduced to provide a reusable and configurable mechanism for extracting URL parameters. This extractor supports strict, prefix, suffix, and path template-based matching, governed by the gateway's global configuration.

Files Changed Analysis

  • New Files:
    • internal/paramextractor/extractor.go & extractor_test.go: Implements and tests the new, reusable URL parameter extraction logic. This is a core component of the new architecture.
    • gateway/mw_mock_response_benchmark_test.go: Adds benchmarks to compare the performance of the classic regex-based router against the kin-openapi/gorillamux router, providing a baseline for the performance impact of this change.
  • Modified Files:
    • gateway/api_definition.go: Contains the central logic for the architectural change. New functions compileOasMock and compileOasValidator are introduced to transform OAS middleware rules into the standard URLSpec format during API definition loading. This integrates them into the existing regex-based path matching engine.
    • gateway/mw_mock_response.go & gateway/mw_oas_validate_request.go: These middlewares are significantly simplified. They are refactored to remove their own path-matching logic and instead rely on the gateway core to provide the matched configuration. The validation middleware is updated to use the new paramextractor.
    • gateway/model_urlspec.go: The URLSpec struct is extended to hold configurations for the new OAS-based middlewares (oasMock, oasValidateRequest), integrating them into the existing system.
    • gateway/model_apispec.go: The path matching function matchesPath is updated to return the matched path string, which is now required for the decoupled parameter extraction process.

Architecture & Impact Assessment

What this PR accomplishes

This PR establishes an architectural pattern for a unified path-matching system. By centralizing this logic, it aims to create a more cohesive gateway core where all path-based middleware is handled consistently. This PoC validates the approach using the OAS mock and validation middlewares as the initial candidates for this new model.

Key technical changes introduced

  1. Centralized Middleware Compilation: OAS middleware extensions for mocking and validation are now compiled into the main RxPaths slice ([]URLSpec) when an API is loaded. This treats them like any other path-based middleware in the gateway.
  2. Decoupled Parameter Extraction: The new paramextractor package abstracts the logic of extracting parameters from a request URL based on a path template. This makes the logic reusable and configurable based on global gateway settings (strict, prefix, suffix matching).
  3. Simplified Middleware: The affected middlewares are refactored to be "dumber." They no longer need to find the matching OAS operation for a request; the gateway core performs the path matching and provides the relevant configuration directly.

Affected system components

  • API Definition Loading: The APIDefinitionLoader is modified to handle the compilation of OAS-based middleware into the gateway's internal URLSpec representation.
  • Gateway Request Pipeline: The core request processing loop now matches paths for OAS mock and validation rules alongside all other existing middleware. This is a critical change to the request hot path.
  • OAS Middlewares: mw_mock_response and mw_oas_validate_request have been significantly refactored to align with the new architecture.

Visualization

This diagram illustrates the architectural shift from multiple, independent path-matching mechanisms to a single, unified model.

graph TD
    subgraph "Before"
        A[Request] --> B{Gateway Core};
        B --> C1[Mock Middleware];
        C1 --> D1["findOperation() logic (OAS Router)"];
        B --> C2[Validate Middleware];
        C2 --> D2["findOperation() logic (OAS Router)"];
        B --> C3[Other Middleware];
        C3 --> D3["Regex Path Matching"];
    end

    subgraph "After (This PR)"
        F[Request] --> G{Gateway Core};
        G --> H{Unified Regex Path Matching};
        H -- "Matched URLSpec (OAS Mock)" --> I[Mock Middleware];
        H -- "Matched URLSpec (OAS Validate)" --> K[Validate Middleware];
        K -- "Uses paramextractor" --> L[Extracts Params];
        H -- "Matched URLSpec (Other)" --> M[Other Middleware];
    end
Loading

Scope Discovery & Context Expansion

The title, "Unify path matching algorithm for all middleware," clearly indicates that this PoC is the first step of a larger and more ambitious initiative. The pattern established here is intended to be extended to other path-based middlewares to create a more consistent and maintainable gateway core.

However, this PoC introduces several critical issues that must be addressed before this pattern can be adopted more widely:

  • Critical Bug: There is a copy-paste error in gateway/api_definition.go in the URLAllowedAndIgnored function. The check for the OasValidate status incorrectly references oasMock.method, which will prevent the validation middleware from ever being triggered correctly.
  • Performance Regression: The new paramextractor's globExtractor compiles a regular expression on every request (regexp.Compile). Regex compilation is a computationally expensive operation and will cause severe performance degradation under load. These expressions should be compiled once when the API definition is loaded and cached for reuse. Similarly, the use of reflect.Clone on the hot path in mw_oas_validate_request.go is highly inefficient and will lead to excessive memory allocation.
  • Architectural Flaw: The path is first matched by the core engine's pre-compiled regex, and then parameters are extracted later in the middleware using the separate paramextractor. These two steps are decoupled and can diverge, leading to subtle bugs where a route is matched but parameter extraction fails. This is highlighted by a modified test case (mw_oas_validate_request_test.go) that avoids greedy path parameters, suggesting the new extractor cannot handle them correctly, whereas the old router could.
  • Inflexible Configuration: The choice of path matching strategy (strict, prefix, etc.) for the paramextractor is controlled by a single global gateway configuration. This is inflexible and could be a security risk if a globally lenient policy (e.g., prefix matching) is applied to APIs that require strict path enforcement. This configuration should ideally be available on a per-API or even per-endpoint basis.

Future work must focus on resolving these performance and architectural issues. Once addressed, this unified pattern could be rolled out to other middlewares to realize its full benefits of improved consistency and maintainability.

Metadata
  • Review Effort: 4 / 5
  • Primary Label: enhancement

Powered by Visor from Probelabs

Last updated: 2025-11-13T12:52:51.156Z | Triggered by: synchronize | Commit: 0a03320

💡 TIP: You can chat with Visor using /visor ask <your question>

@probelabs
Copy link

probelabs bot commented Nov 7, 2025

🔍 Code Analysis Results

✅ Security Check Passed

No security issues found – changes LGTM.

Architecture Issues (3)

Severity Location Issue
🟠 Error gateway/mw_oas_validate_request.go:102
A deep copy of the `routers.Route` object, which can include the entire OpenAPI specification, is performed on every request using `reflect.Clone`. This is extremely inefficient and will cause significant performance degradation (high CPU and memory usage) under load. The `kin-openapi` library's validation functions are not expected to mutate the input route object, making a deep copy unnecessary. If a copy is required for safety, a shallow copy using `route.Copy()` would be far more performant.
💡 SuggestionRemove the deep copy. Pass the `validateRequest.route` object directly to the `RequestValidationInput`. If a copy is deemed absolutely necessary, use the object's own shallow `Copy()` method instead of a generic deep clone.
🟡 Warning gateway/mw_oas_validate_request.go:84
The `newParamExtractor()` function is called on every request to get a parameter extractor. The extractor's configuration depends on static gateway settings (`HttpServerOptions`), so the extractor can be created once when the `ValidateRequest` middleware is initialized and reused across all requests. Creating it on every request is inefficient.
💡 SuggestionCreate the parameter extractor once and store it as a field in the `ValidateRequest` struct. Initialize it in a constructor or once when the middleware is set up.
🟡 Warning internal/paramextractor/extractor.go:201
The `globExtractor`'s `Extract` method compiles a regular expression from the path pattern on every call. Since path patterns are static for a given API, the compiled regexes should be cached to avoid the performance overhead of repeated compilation.
💡 SuggestionImplement a cache (e.g., a `map[string]*regexp.Regexp` protected by a mutex) within the `globExtractor` to store and reuse compiled regular expressions for patterns that have already been seen.

Performance Issues (3)

Severity Location Issue
🔴 Critical gateway/model_apispec.go:75
The path matching algorithm uses a linear scan over all defined paths for an API version, resulting in O(N) complexity for routing where N is the number of paths. This will cause significant performance degradation for APIs with a large number of endpoints. This change extends this inefficient pattern to new OAS-based middleware, representing a performance regression compared to using optimized routers like `gorillamux`.
💡 SuggestionThe routing mechanism should be re-architected to use a more efficient data structure, such as a radix tree, for path lookups. This would provide O(k) complexity where k is the length of the URL path, ensuring scalable performance regardless of the number of defined endpoints. The benchmark added in this PR (`mw_mock_response_benchmark_test.go`) already demonstrates the performance benefits of such an approach.
🟠 Error internal/paramextractor/extractor.go:198
The `globExtractor.Extract` function compiles a regular expression on every invocation. When used for OAS request validation with certain gateway configurations, this results in expensive regex compilation on every incoming request, leading to high CPU usage and request latency.
💡 SuggestionPre-compile the regular expressions when the API definition is loaded and cache the compiled `*regexp.Regexp` object. The compiled object can be stored alongside the path pattern in the `URLSpec` and reused for all subsequent requests matching that path.
🟡 Warning gateway/mw_oas_validate_request.go:111
A `routers.Route` object is deep-cloned using `reflect.Clone` on every request within the OAS validation middleware. Reflection is computationally expensive and its use in a hot path like request processing can negatively impact performance.
💡 SuggestionVerify if the `openapi3filter.ValidateRequest` function mutates the `Route` object. If it does not, remove the clone operation. If a copy is required, implement a manual, non-reflection-based copy of the struct to improve performance.

Quality Issues (3)

Severity Location Issue
🟢 Info gateway/mw_mock_response.go:106
The error handling for a failed type assertion is inconsistent with other parts of the new code. Here, it returns a generic `errors.New("unexpected type")`, while in `mw_oas_validate_request.go` a similar failure is handled by returning an error and an HTTP status code. A type assertion failure indicates a programming error and should be handled consistently.
💡 SuggestionFor consistency and improved diagnostics, consider returning a more specific error that can be handled uniformly by the middleware chain, likely resulting in an HTTP 500 Internal Server Error. For example: `return nil, fmt.Errorf("internal error: unexpected middleware metadata type: %T", meta)`.
🟡 Warning gateway/api_definition.go:1421-1478
The function `URLAllowedAndIgnored` iterates over the `rxPaths` slice multiple times within the same function call. For APIs with a large number of paths, as simulated in the new benchmarks, these multiple linear scans on every request could introduce a performance bottleneck.
💡 SuggestionConsider refactoring this function to use a single loop over `rxPaths`. In this loop, collect all specs that match the request path. After the loop, process the collected matching specs according to the required precedence (BlackList, WhiteList, Ignored, etc.). This would improve the algorithmic complexity for path matching from O(N*k) to O(N).
🟡 Warning internal/paramextractor/extractor.go:214
The function name `globToRegex` is potentially misleading as it does not handle standard glob patterns (like `*` or `?`). It converts a path template with `{param}` style placeholders into a regular expression.
💡 SuggestionTo improve clarity for future maintainers, consider renaming the function to be more descriptive of its actual functionality, for example, `templateToRegex` or `pathToRegex`.

Dependency Issues (4)

Severity Location Issue
🔴 Critical gateway/api_definition.go:1360-1459
The introduction of new `x-tyk-api-gateway` extension fields for configuring OAS mock and validation middleware requires an update to the `ApiDefinition` CustomResourceDefinition (CRD) schema in the `tyk-operator` repository. Without this update, users will be unable to configure these middleware features when managing APIs via Kubernetes CRDs.
💡 SuggestionUpdate the `ApiDefinition` CRD schema in `tyk-operator` to include the new fields under `spec.oas.x-tyk-api-gateway.middleware.operations` for `mockResponse` and `validateRequest`.
🔴 Critical gateway/api_definition.go:1360-1459
The change to configuring OAS mock and validation middleware via `x-tyk-api-gateway` extensions requires the Tyk Portal's API Designer UI and backend to be updated. The current portal implementation for these features likely generates the legacy `extended_paths` configuration, which will no longer work for OAS APIs with this change.
💡 SuggestionUpdate the Tyk Portal's API Designer to generate the new `x-tyk-api-gateway` extension structure when configuring mock responses and request validation for OAS APIs.
🔴 Critical gateway/api_definition.go:1360-1459
The `APIDefinition` data model has changed to include new OAS extension fields for middleware. `tyk-sink` (MDCB) must be updated to correctly serialize and deserialize these new fields to ensure data integrity during synchronization between data stores (e.g., Mongo to Redis). Failure to update `tyk-sink` could result in middleware configurations being lost during sync.
💡 SuggestionVerify and update the `APIDefinition` struct and associated processing logic in `tyk-sink` to ensure compatibility with the new OAS extension fields.
🟡 Warning internal/paramextractor/extractor.go:201
The architectural changes, including regex compilation on the hot path and replacing an optimized router with a linear scan for OAS paths, may lead to performance degradation under load. This could necessitate adjustments to the default CPU and memory resource allocations in the `tyk-charts` Helm chart for production workloads.
💡 SuggestionBenchmark the performance impact of these changes and consider adjusting the default `resources.requests` and `resources.limits` for the gateway in `tyk-charts` if a significant regression is observed.

✅ Connectivity Check Passed

No connectivity issues found – changes LGTM.


Powered by Visor from Probelabs

Last updated: 2025-11-13T12:52:52.346Z | Triggered by: synchronize | Commit: 0a03320

💡 TIP: You can chat with Visor using /visor ask <your question>

@shults shults force-pushed the TT-15856-po-c-unify-path-matching-algorithm-for-all-middleware branch from eae4ba9 to a76c087 Compare November 12, 2025 07:05
@sonarqubecloud
Copy link

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants