-
Notifications
You must be signed in to change notification settings - Fork 1.1k
[TT-15863] fix random version picking for not versioned API #7380
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
[TT-15863] fix random version picking for not versioned API #7380
Conversation
PR Reviewer Guide 🔍Here are some key observations to aid the review process:
|
PR Code Suggestions ✨Explore these optional code suggestions:
|
API Changes --- prev.txt 2025-09-27 14:43:10.538925984 +0000
+++ current.txt 2025-09-27 14:43:01.210719591 +0000
@@ -9149,6 +9149,10 @@
func (s *APISpec) AddUnloadHook(hook func())
AddUnloadHook adds a function to be called when the API spec is unloaded
+func (a *APISpec) CheckForAmbiguousDefaultVersions() bool
+ CheckForAmbiguousDefaultVersions checks if there are multiple ambiguous
+ default versions in the version data.
+
func (a *APISpec) CheckSpecMatchesStatus(r *http.Request, rxPaths []URLSpec, mode URLStatus) (bool, interface{})
CheckSpecMatchesStatus checks if a URL spec has a specific status.
Deprecated: The function doesn't follow go return conventions (T, ok);
@@ -9168,6 +9172,11 @@
takes the precedence. If the global one is `true`, value of the one in api
level doesn't matter.
+func (a *APISpec) GetSingleOrDefaultVersion() (versionInfo apidef.VersionInfo, ok bool)
+ GetSingleOrDefaultVersion determines and returns a single version or the
+ default version if only one or a default exists. Returns versionInfo and a
+ boolean indicating success or failure.
+
func (a *APISpec) GetTykExtension() *oas.XTykAPIGateway
func (a *APISpec) Init(authStore, sessionStore, healthStore, orgStore storage.Handler)
@@ -11401,36 +11410,38 @@
RequestStatus is a custom type to avoid collisions
const (
- VersionNotFound RequestStatus = "Version information not found"
- VersionDoesNotExist RequestStatus = "This API version does not seem to exist"
- VersionWhiteListStatusNotFound RequestStatus = "WhiteListStatus for path not found"
- VersionExpired RequestStatus = "Api Version has expired, please check documentation or contact administrator"
- APIExpired RequestStatus = "API has expired, please check documentation or contact administrator"
- EndPointNotAllowed RequestStatus = "Requested endpoint is forbidden"
- StatusOkAndIgnore RequestStatus = "Everything OK, passing and not filtering"
- StatusOk RequestStatus = "Everything OK, passing"
- StatusCached RequestStatus = "Cached path"
- StatusTransform RequestStatus = "Transformed path"
- StatusTransformResponse RequestStatus = "Transformed response"
- StatusTransformJQ RequestStatus = "Transformed path with JQ"
- StatusTransformJQResponse RequestStatus = "Transformed response with JQ"
- StatusHeaderInjected RequestStatus = "Header injected"
- StatusMethodTransformed RequestStatus = "Method Transformed"
- StatusHeaderInjectedResponse RequestStatus = "Header injected on response"
- StatusRedirectFlowByReply RequestStatus = "Exceptional action requested, redirecting flow!"
- StatusHardTimeout RequestStatus = "Hard Timeout enforced on path"
- StatusCircuitBreaker RequestStatus = "Circuit breaker enforced"
- StatusURLRewrite RequestStatus = "URL Rewritten"
- StatusVirtualPath RequestStatus = "Virtual Endpoint"
- StatusRequestSizeControlled RequestStatus = "Request Size Limited"
- StatusRequestTracked RequestStatus = "Request Tracked"
- StatusRequestNotTracked RequestStatus = "Request Not Tracked"
- StatusValidateJSON RequestStatus = "Validate JSON"
- StatusValidateRequest RequestStatus = "Validate Request"
- StatusInternal RequestStatus = "Internal path"
- StatusGoPlugin RequestStatus = "Go plugin"
- StatusPersistGraphQL RequestStatus = "Persist GraphQL"
- StatusRateLimit RequestStatus = "Rate Limited"
+ VersionNotFound RequestStatus = "Version information not found"
+ VersionDoesNotExist RequestStatus = "This API version does not seem to exist"
+ VersionWhiteListStatusNotFound RequestStatus = "WhiteListStatus for path not found"
+ VersionExpired RequestStatus = "Api Version has expired, please check documentation or contact administrator"
+ VersionDefaultForNotVersionedNotFound RequestStatus = "No default API version for this non-versioned API found"
+ VersionAmbiguousDefault RequestStatus = "Ambiguous default API version for this non-versioned API"
+ APIExpired RequestStatus = "API has expired, please check documentation or contact administrator"
+ EndPointNotAllowed RequestStatus = "Requested endpoint is forbidden"
+ StatusOkAndIgnore RequestStatus = "Everything OK, passing and not filtering"
+ StatusOk RequestStatus = "Everything OK, passing"
+ StatusCached RequestStatus = "Cached path"
+ StatusTransform RequestStatus = "Transformed path"
+ StatusTransformResponse RequestStatus = "Transformed response"
+ StatusTransformJQ RequestStatus = "Transformed path with JQ"
+ StatusTransformJQResponse RequestStatus = "Transformed response with JQ"
+ StatusHeaderInjected RequestStatus = "Header injected"
+ StatusMethodTransformed RequestStatus = "Method Transformed"
+ StatusHeaderInjectedResponse RequestStatus = "Header injected on response"
+ StatusRedirectFlowByReply RequestStatus = "Exceptional action requested, redirecting flow!"
+ StatusHardTimeout RequestStatus = "Hard Timeout enforced on path"
+ StatusCircuitBreaker RequestStatus = "Circuit breaker enforced"
+ StatusURLRewrite RequestStatus = "URL Rewritten"
+ StatusVirtualPath RequestStatus = "Virtual Endpoint"
+ StatusRequestSizeControlled RequestStatus = "Request Size Limited"
+ StatusRequestTracked RequestStatus = "Request Tracked"
+ StatusRequestNotTracked RequestStatus = "Request Not Tracked"
+ StatusValidateJSON RequestStatus = "Validate JSON"
+ StatusValidateRequest RequestStatus = "Validate Request"
+ StatusInternal RequestStatus = "Internal path"
+ StatusGoPlugin RequestStatus = "Go plugin"
+ StatusPersistGraphQL RequestStatus = "Persist GraphQL"
+ StatusRateLimit RequestStatus = "Rate Limited"
)
Statuses of the request, all are false-y except StatusOk and
StatusOkAndIgnore |
🔍 Code Analysis ResultsThe user wants a comprehensive overview and analysis of the pull request. I have already analyzed the code changes in the `full_diff` provided in the initial prompt.Here's a summary of my analysis:
I have all the necessary information to construct the final response. I will structure it into the requested sections: "Change Impact Analysis" and "Architecture Visualization" with a mermaid diagram. The analysis provided by the bot in the comments is a good reference, but I will write my own based on my direct analysis of the diff.<attempt_completion> 1. Change Impact AnalysisWhat This PR AccomplishesThis pull request addresses a critical bug where the Tyk gateway would select a random, non-deterministic version for an API marked as "not versioned." The previous implementation iterated over a map of versions and picked the first one it encountered. Since map iteration order is not guaranteed in Go, this led to unpredictable behavior. This PR replaces that logic with a predictable and robust mechanism to select a single, default, or implicitly-named default version, ensuring consistent behavior for all requests to non-versioned APIs. Key Technical Changes
Affected System Components
2. Architecture VisualizationThe following flowchart visualizes the updated logic within the flowchart TD
subgraph "APISpec.Version Method (for NotVersioned API)"
A[Incoming Request] --> B{Check for Ambiguous Defaults};
B -- Yes --> C[Return Error: VersionAmbiguousDefault];
B -- No --> D["GetSingleOrDefaultVersion()"];
subgraph "GetSingleOrDefaultVersion Logic"
D --> E{Only one version exists?};
E -- Yes --> F[Return the single version];
E -- No --> G{"Is "Default" version defined?"};
G -- Yes --> H["Return "Default" version"];
G -- No --> I{"Is "default" version defined?"};
I -- Yes --> J["Return "default" version"];
I -- No --> K{"Is "" (empty string) version defined?"};
K -- Yes --> L["Return "" version"];
K -- No --> M[No default version found];
end
F --> Z["Return Version & StatusOk"];
H --> Z;
J --> Z;
L --> Z;
M --> N[Return Error: VersionDefaultForNotVersionedNotFound];
end
C --> X[End Request Processing];
N --> X;
Z --> X;
Powered by Visor from Probelabs Last updated: 2025-09-27T14:49:09.723Z | Triggered by: synchronize | Commit: 417de4e |
🔍 Code Analysis ResultsSecurity Issues (1)
Performance Issues (1)
Quality Issues (3)
Style Issues (2)
Dependency Issues (1)
✅ Connectivity Check PassedNo connectivity issues found – changes LGTM. Powered by Visor from Probelabs Last updated: 2025-09-27T14:49:10.723Z | Triggered by: synchronize | Commit: 417de4e |
|
TT-15863
This PR fixes an issue where a random version was picked for a non-versioned API.