Skip to content

Conversation

pvormste
Copy link
Collaborator

@pvormste pvormste commented Sep 26, 2025

TT-15863
Summary Random version being picked when not_versioned is set to true
Type Bug Bug
Status In Dev
Points N/A
Labels -

This PR fixes an issue where a random version was picked for a non-versioned API.

@buger
Copy link
Member

buger commented Sep 26, 2025

I'm a bot and I 👍 this PR title. 🤖

Copy link
Contributor

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Logic Consistency

In GetSingleOrDefaultVersion, the comment says the default lookup is skipped when not versioned, but the function still falls back to keys "Default", "default", and "" which are also treated as defaults for not-versioned APIs. Ensure this behavior matches product expectations and documentation, and reconcile the comment or logic accordingly.

// 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) GetSingleOrDefaultVersion() (versionInfo apidef.VersionInfo, ok bool) {
	// If only one version exists, we can safely return this one
	if len(a.VersionData.Versions) == 1 {
		for _, v := range a.VersionData.Versions {
			return v, true
		}
	}

	// Now we check if a default version is defined and will look for it, when NotVersioned is set to false.
	// Otherwise, we skip this check.
	if a.VersionData.NotVersioned == false && a.VersionData.DefaultVersion != "" {
		versionInfo, ok = a.VersionData.Versions[a.VersionData.DefaultVersion]
		return versionInfo, ok
	}

	// If no default version is defined, we try to find one named "Default", "default" or ""
	if versionInfo, ok = a.VersionData.Versions["Default"]; ok {
		return versionInfo, ok
	}

	if versionInfo, ok = a.VersionData.Versions["default"]; ok {
		return versionInfo, ok
	}

	if versionInfo, ok = a.VersionData.Versions[""]; ok {
		return versionInfo, ok
	}

	// If we reach this point, we tried everything to find a default version and failed
	return apidef.VersionInfo{}, false
}
Ambiguity Criteria

CheckForAmbiguousDefaultVersions only counts presence of "Default", "default", and "" keys; if DefaultVersion points to one of these and a conflicting key exists, ambiguity may still arise but not be detected. Confirm that only key presence should define ambiguity and not DefaultVersion setting.

// CheckForAmbiguousDefaultVersions checks if there are multiple ambiguous default versions in the version data.
func (a *APISpec) CheckForAmbiguousDefaultVersions() bool {
	foundDefaultVersions := 0
	for key := range a.VersionData.Versions {
		switch key {
		case "Default":
			fallthrough
		case "default":
			fallthrough
		case "":
			foundDefaultVersions++
		}
	}

	return foundDefaultVersions > 1
}
Status Semantics

New RequestStatus messages are user-facing; verify wording, capitalization, and consistency with existing status strings and any client-side parsing or telemetry that may rely on exact text.

	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"
)

Copy link
Contributor

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Avoid nondeterministic map iteration

Iterating a single-entry map is fine, but relying on map iteration for selection
elsewhere can be nondeterministic across runs. Ensure all selection paths avoid map
iteration order; here, return directly using a deterministic access.

gateway/api_definition.go [1715-1719]

 // If only one version exists, we can safely return this one
 if len(a.VersionData.Versions) == 1 {
-	for _, v := range a.VersionData.Versions {
-		return v, true
+	for k := range a.VersionData.Versions {
+		return a.VersionData.Versions[k], true
 	}
 }
Suggestion importance[1-10]: 4

__

Why: While both versions iterate once over a single-entry map, the change does not improve determinism meaningfully; impact is minor and correctness unchanged, but it's a harmless stylistic tweak.

Low
Resolve ambiguous defaults deterministically

Returning an ambiguous default error for non-versioned APIs may break existing
clients; prefer deterministic resolution. Instead of erroring, choose a consistent
precedence among "", "Default", and "default" to select a default, keeping behavior
stable while logging ambiguity elsewhere if needed.

gateway/api_definition.go [1669-1672]

-ambiguous := a.CheckForAmbiguousDefaultVersions()
-if ambiguous {
-	return nil, VersionAmbiguousDefault
+// Prefer a deterministic default instead of erroring on ambiguity.
+// Precedence: "" > "Default" > "default"
+if a.VersionData.Versions[""] != (apidef.VersionInfo{}) {
+	v := a.VersionData.Versions[""]
+	return &v, StatusOk
+}
+if v, ok := a.VersionData.Versions["Default"]; ok {
+	return &v, StatusOk
+}
+if v, ok := a.VersionData.Versions["default"]; ok {
+	return &v, StatusOk
 }
Suggestion importance[1-10]: 2

__

Why: The PR explicitly introduces VersionAmbiguousDefault and tests rely on returning this status; replacing it with implicit precedence conflicts with the PR's design and could hide configuration issues.

Low
Possible issue
Always honor explicit default version

This condition prevents honoring DefaultVersion when the API is marked as
non-versioned, which contradicts tests that expect the explicit default to be
ignored only for non-versioned APIs. To avoid surprising behavior, drop the
NotVersioned check and always respect DefaultVersion when it is set, letting the
caller decide policy.

gateway/api_definition.go [1723-1726]

-if a.VersionData.NotVersioned == false && a.VersionData.DefaultVersion != "" {
+if a.VersionData.DefaultVersion != "" {
 	versionInfo, ok = a.VersionData.Versions[a.VersionData.DefaultVersion]
 	return versionInfo, ok
 }
Suggestion importance[1-10]: 3

__

Why: The suggestion contradicts the newly added behavior and tests which intentionally ignore DefaultVersion when NotVersioned is true; changing it would break expected semantics. It's a coherent alternative policy but not appropriate for this PR.

Low

Copy link
Contributor

github-actions bot commented Sep 26, 2025

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

Copy link

probelabs bot commented Sep 26, 2025

🔍 Code Analysis Results

The 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:

  • Problem: The original code picked a random version for non-versioned APIs because it iterated over a map, which has a non-deterministic order in Go.
  • Solution: The PR introduces a deterministic logic to find a default version for non-versioned APIs.
  • Key Changes in gateway/api_definition.go:
    • The APISpec.Version method is modified for NotVersioned == true APIs.
    • It now first calls CheckForAmbiguousDefaultVersions to ensure there aren't multiple "default" versions (e.g., one named "Default" and another named "default"). If ambiguity is found, it returns a new VersionAmbiguousDefault status.
    • If no ambiguity, it calls GetSingleOrDefaultVersion to find the appropriate version.
    • If no version is found, it returns a new VersionDefaultForNotVersionedNotFound status.
  • New Helper Functions:
    • CheckForAmbiguousDefaultVersions(): Counts how many versions have the key "Default", "default", or "". Returns true if the count is greater than 1.
    • GetSingleOrDefaultVersion(): Implements the logic to find the version:
      1. If there's only one version, return it.
      2. (This part seems like dead code in the current context, as it's only called for NotVersioned APIs) If the API is versioned and has an explicit DefaultVersion, return that.
      3. Look for a version with the key "Default".
      4. Look for a version with the key "default".
      5. Look for a version with an empty string key "".
      6. If none are found, return ok = false.
  • New RequestStatus constants: VersionDefaultForNotVersionedNotFound and VersionAmbiguousDefault are added to provide clear error statuses.
  • Tests: Extensive unit tests are added in gateway/api_definition_test.go to cover the new logic and edge cases.

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>
Of course. Here is a comprehensive overview and analysis of the pull request.

1. Change Impact Analysis

What This PR Accomplishes

This 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

  1. Deterministic Version Selection: The core change is within the APISpec.Version method. The old, unreliable logic for NotVersioned APIs has been replaced with a structured, multi-step process.
  2. Ambiguity Detection: A new helper function, CheckForAmbiguousDefaultVersions, is introduced. Before selecting a version, it checks if the API definition contains multiple conflicting "default" versions (e.g., versions named "Default", "default", or ""). This prevents situations where the gateway cannot make a clear choice.
  3. New Default Version Logic: The GetSingleOrDefaultVersion function implements the new selection logic for non-versioned APIs:
    • If only one version exists, it is chosen by default.
    • If multiple versions exist, it searches for an implicit default version by looking for a version named "Default", then "default", and finally "" (empty string).
    • This ensures a consistent precedence for selecting the default version.
  4. New Status Codes: Two new RequestStatus constants have been added to provide clear feedback when version selection fails:
    • VersionAmbiguousDefault: Returned when multiple implicit default versions are found.
    • VersionDefaultForNotVersionedNotFound: Returned when no suitable single or default version can be found.
  5. Comprehensive Unit Tests: A significant number of unit tests have been added in gateway/api_definition_test.go to validate the new selection logic, covering various scenarios like single versions, implicit defaults, ambiguous configurations, and failure cases.

Affected System Components

  • API Gateway Versioning Middleware: The primary impact is on the gateway's request processing pipeline, specifically the middleware that resolves the API version for an incoming request. The APISpec.Version method is a key component in this process, and its behavior for non-versioned APIs is now stable and predictable.
  • API Definition Handling: The interpretation of the VersionData within an APIDefinition is now more strict. Administrators configuring non-versioned APIs with multiple versions must ensure there is a single, unambiguous default version to avoid request failures.

2. Architecture Visualization

The following flowchart visualizes the updated logic within the APISpec.Version method for handling requests to a non-versioned API.

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;
Loading

Powered by Visor from Probelabs

Last updated: 2025-09-27T14:49:09.723Z | Triggered by: synchronize | Commit: 417de4e

Copy link

probelabs bot commented Sep 26, 2025

🔍 Code Analysis Results

Security Issues (1)

Severity Location Issue
🟡 Warning gateway/api_definition.go:1745-1760
The function `CheckForAmbiguousDefaultVersions` iterates over all API versions on every request to a non-versioned API. This introduces an O(N) operation on a critical request path, where N is the number of versions. In a scenario with an extremely large number of versions, this could be a vector for a Denial of Service (DoS) attack.
💡 SuggestionOptimize the check for ambiguity by replacing the inefficient loop with direct map lookups for the specific 'magic' default version names ('Default', 'default', ''). This will make the operation constant time (O(1)) and avoid performance degradation on the hot path, mitigating the theoretical DoS risk.
🔧 Suggested Fix
func (a *APISpec) CheckForAmbiguousDefaultVersions() bool {
	foundDefaultVersions := 0
	if _, ok := a.VersionData.Versions["Default"]; ok {
		foundDefaultVersions++
	}
	if _, ok := a.VersionData.Versions["default"]; ok {
		foundDefaultVersions++
	}
	if _, ok := a.VersionData.Versions[""]; ok {
		foundDefaultVersions++
	}
	return foundDefaultVersions > 1
}

Performance Issues (1)

Severity Location Issue
🟡 Warning gateway/api_definition.go:1746-1760
The `CheckForAmbiguousDefaultVersions` function is called on every request to a non-versioned API and iterates over the entire `Versions` map. This introduces an O(N) operation on a critical request path, where N is the number of versions. This is a performance regression and can introduce latency for APIs with many versions.
💡 SuggestionOptimize the check for ambiguity by replacing the loop with direct map lookups for the three specific default keys. This will change the operation to be constant time (O(1)) and eliminate the performance regression.
🔧 Suggested Fix
func (a *APISpec) CheckForAmbiguousDefaultVersions() bool {
	foundDefaultVersions := 0
	if _, ok := a.VersionData.Versions["Default"]; ok {
		foundDefaultVersions++
	}
	if _, ok := a.VersionData.Versions["default"]; ok {
		foundDefaultVersions++
	}
	if _, ok := a.VersionData.Versions[""]; ok {
		foundDefaultVersions++
	}
	return foundDefaultVersions > 1
}

Quality Issues (3)

Severity Location Issue
🟢 Info gateway/api_definition.go:1723-1726
The `GetSingleOrDefaultVersion` function contains a conditional block that executes when `NotVersioned` is `false`. However, the function's only call site in `APISpec.Version` is within a code path that runs only when `NotVersioned` is `true`. This makes the conditional block unreachable in practice, which can be confusing for future code maintenance.
💡 SuggestionRemove the unreachable code block to improve code clarity. If this path is intended for future use, add a comment explaining its purpose and why it is not currently exercised by the application logic.
🟢 Info gateway/api_definition.go:1712
The function comment for `GetSingleOrDefaultVersion` could be more explicit about the search order for implicit default versions.
💡 SuggestionEnhance the function comment to detail the precedence of implicit default versions ('Default', 'default', then '') to make the function's behavior clearer without needing to read the implementation.
🔧 Suggested Fix
// GetSingleOrDefaultVersion determines and returns a single version or the default version.
// It first checks if only one version exists. If not, it searches for implicit defaults
// in the order: "Default", "default", "".
// Returns versionInfo and a boolean indicating success or failure.
🟡 Warning gateway/api_definition.go:1729-1760
The logic for identifying implicit default API versions using magic strings ("Default", "default", "") is duplicated across `GetSingleOrDefaultVersion` and `CheckForAmbiguousDefaultVersions`. This violates the DRY (Don't Repeat Yourself) principle, making the code harder to maintain and prone to inconsistencies if the logic changes.
💡 SuggestionRefactor the logic to remove duplication and improve performance. Define the default version keys (e.g., "Default", "default", "") in a single constant slice. Both functions should then use this slice to look up default versions. This will centralize the logic and allow `CheckForAmbiguousDefaultVersions` to be implemented with constant time complexity (by performing a fixed number of map lookups) instead of linear, which also resolves a performance issue on a critical path.

Style Issues (2)

Severity Location Issue
🟢 Info gateway/api_definition.go:1708-1711
The function comment for `GetSingleOrDefaultVersion` is good, but it could be more explicit about the search order for implicit default versions. Documenting the precedence ("Default", then "default", then "") would help future developers understand the behavior without needing to read the implementation.
💡 SuggestionEnhance the function comment to detail the precedence of implicit default versions ('Default', 'default', then '') to make the function's behavior clearer without needing to read the implementation.
🔧 Suggested Fix
// GetSingleOrDefaultVersion determines and returns a single version or the default version.
// It first checks if only one version exists. If not, and if the API is versioned, it looks for a configured 
// default version. If no explicit default is found, it searches for implicit defaults
// in the order: "Default", "default", "".
// Returns versionInfo and a boolean indicating success or failure.
🟢 Info gateway/api_definition.go:1747-1757
The `switch` statement with `fallthrough` is a clever use of the feature, but a more direct implementation using three separate map lookups would be slightly more explicit about checking for the three specific keys and would also be more performant by avoiding a loop over all versions.
💡 SuggestionRefactor the function to use direct map lookups for the three specific default keys ("Default", "default", "") instead of iterating over all versions with a switch statement. This improves both readability and performance.
🔧 Suggested Fix
    foundDefaultVersions := 0
    if _, ok := a.VersionData.Versions["Default"]; ok {
        foundDefaultVersions++
    }
    if _, ok := a.VersionData.Versions["default"]; ok {
        foundDefaultVersions++
    }
    if _, ok := a.VersionData.Versions[""]; ok {
        foundDefaultVersions++
    }
return foundDefaultVersions &gt; 1</code></pre></details>

Dependency Issues (1)

Severity Location Issue
🔴 Critical gateway/api_definition.go:1664-1682
The change in logic for selecting a version for a non-versioned API introduces a breaking change in runtime behavior. Previously, the gateway would randomly select a version, allowing misconfigured APIs to function unpredictably. Now, it will explicitly fail if an unambiguous default version cannot be found. This change has a significant impact on downstream dependencies that manage API Definitions.
💡 SuggestionThis breaking change requires coordinated updates across multiple projects. 1) An issue must be raised for `tyk-operator` to add validation to its `ApiDefinition` controller to reject ambiguous configurations. 2) An issue must be raised for `tyk-portal` to update its UI validation to prevent users from creating such configurations. 3) The release notes for this gateway version must clearly document this change and its operational impact, especially for `tyk-sink` (MDCB) users performing rolling upgrades.

✅ Connectivity Check Passed

No connectivity issues found – changes LGTM.


Powered by Visor from Probelabs

Last updated: 2025-09-27T14:49:10.723Z | Triggered by: synchronize | Commit: 417de4e

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