From d3e9685e2b0d89121bcd4f35a157729ff8c525e1 Mon Sep 17 00:00:00 2001 From: Patric Vormstein Date: Fri, 26 Sep 2025 16:11:28 +0200 Subject: [PATCH 1/2] fix random version picking for not versioned API --- gateway/api_definition.go | 126 +++++++++---- gateway/api_definition_test.go | 317 +++++++++++++++++++++++++++++++++ 2 files changed, 409 insertions(+), 34 deletions(-) diff --git a/gateway/api_definition.go b/gateway/api_definition.go index a1d3c5d729a..2d226bb1f29 100644 --- a/gateway/api_definition.go +++ b/gateway/api_definition.go @@ -93,36 +93,38 @@ type RequestStatus string // Statuses of the request, all are false-y except StatusOk and StatusOkAndIgnore 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" ) type EndPointCacheMeta struct { @@ -1664,10 +1666,15 @@ func (a *APISpec) Version(r *http.Request) (*apidef.VersionInfo, RequestStatus) } else { // Are we versioned? if a.VersionData.NotVersioned { - // Get the first one in the list - for _, v := range a.VersionData.Versions { - version = v - break + ambiguous := a.CheckForAmbiguousDefaultVersions() + if ambiguous { + return nil, VersionAmbiguousDefault + } + + ok := false + version, ok = a.GetSingleOrDefaultVersion() + if !ok { + return nil, VersionDefaultForNotVersionedNotFound } } else { // Extract Version Info @@ -1701,6 +1708,57 @@ func (a *APISpec) Version(r *http.Request) (*apidef.VersionInfo, RequestStatus) return &version, StatusOk } +// 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 +} + +// 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 +} + // StripListenPath will strip the listen path from the URL, keeping version in tact. func (a *APISpec) StripListenPath(reqPath string) string { return httputil.StripListenPath(a.Proxy.ListenPath, reqPath) diff --git a/gateway/api_definition_test.go b/gateway/api_definition_test.go index a1e2fb125a3..1d93e32c297 100644 --- a/gateway/api_definition_test.go +++ b/gateway/api_definition_test.go @@ -2095,3 +2095,320 @@ func TestFromDashboardServiceNetworkErrorRecovery(t *testing.T) { assert.Equal(t, 2, requestCount, "Should have made 2 API requests (failed + retry)") assert.GreaterOrEqual(t, registrationCount, 1, "Should have re-registered after network error") } + +func TestAPISpec_GetSingleOrDefaultVersion(t *testing.T) { + type testCase struct { + name string + spec *APISpec + expectedVersion apidef.VersionInfo + expectedOk bool + } + + testCases := []testCase{ + { + name: "should get the single existing version", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + VersionData: apidef.VersionData{ + NotVersioned: true, + Versions: map[string]apidef.VersionInfo{ + "v1": {Name: "v1"}, + }, + }, + }, + }, + expectedVersion: apidef.VersionInfo{Name: "v1"}, + expectedOk: true, + }, + { + name: "should get the defined default version when not_versioned is false", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + VersionData: apidef.VersionData{ + NotVersioned: false, + DefaultVersion: "v1", + Versions: map[string]apidef.VersionInfo{ + "Default": {Name: "Default"}, + "v1": {Name: "v1"}, + "v2": {Name: "v2"}, + }, + }, + }, + }, + expectedVersion: apidef.VersionInfo{Name: "v1"}, + expectedOk: true, + }, + { + name: "should get the default version when not_versioned is true", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + VersionData: apidef.VersionData{ + NotVersioned: true, + DefaultVersion: "v1", + Versions: map[string]apidef.VersionInfo{ + "Default": {Name: "Default"}, + "v1": {Name: "v1"}, + "v2": {Name: "v2"}, + }, + }, + }, + }, + expectedVersion: apidef.VersionInfo{Name: "Default"}, + expectedOk: true, + }, + { + name: "should get the default version when no default version is set and the default version is stored as Default (upper-case)", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + VersionData: apidef.VersionData{ + NotVersioned: true, + DefaultVersion: "", + Versions: map[string]apidef.VersionInfo{ + "Default": {Name: "Default"}, + "v1": {Name: "v1"}, + "v2": {Name: "v2"}, + }, + }, + }, + }, + expectedVersion: apidef.VersionInfo{Name: "Default"}, + expectedOk: true, + }, + { + name: "should get the default version when no default version is set and the default version is stored as default (lower-case)", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + VersionData: apidef.VersionData{ + NotVersioned: true, + DefaultVersion: "", + Versions: map[string]apidef.VersionInfo{ + "default": {Name: "default"}, + "v1": {Name: "v1"}, + "v2": {Name: "v2"}, + }, + }, + }, + }, + expectedVersion: apidef.VersionInfo{Name: "default"}, + expectedOk: true, + }, + { + name: "should get the default version when no default version is set and the default version is stored as empty string", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + VersionData: apidef.VersionData{ + NotVersioned: true, + DefaultVersion: "", + Versions: map[string]apidef.VersionInfo{ + "": {Name: "empty-string"}, + "v1": {Name: "v1"}, + "v2": {Name: "v2"}, + }, + }, + }, + }, + expectedVersion: apidef.VersionInfo{Name: "empty-string"}, + expectedOk: true, + }, + { + name: "should return false for ok, if all checks failed", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + VersionData: apidef.VersionData{ + NotVersioned: true, + DefaultVersion: "", + Versions: map[string]apidef.VersionInfo{ + "v1": {Name: "v1"}, + "v2": {Name: "v2"}, + }, + }, + }, + }, + expectedVersion: apidef.VersionInfo{}, + expectedOk: false, + }, + } + + for _, tc := range testCases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + version, ok := tc.spec.GetSingleOrDefaultVersion() + assert.Equal(t, tc.expectedVersion, version) + assert.Equal(t, tc.expectedOk, ok) + }) + } +} + +func TestAPISpec_CheckForAmbiguousDefaultVersions(t *testing.T) { + type testCase struct { + name string + spec *APISpec + expectedAmbiguous bool + } + + testCases := []testCase{ + { + name: "should return false if no default version is found", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + VersionData: apidef.VersionData{ + NotVersioned: true, + DefaultVersion: "", + Versions: map[string]apidef.VersionInfo{ + "v1": {Name: "v1"}, + }, + }, + }, + }, + expectedAmbiguous: false, + }, + { + name: "should return true if Default and default versions are found", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + VersionData: apidef.VersionData{ + NotVersioned: true, + DefaultVersion: "", + Versions: map[string]apidef.VersionInfo{ + "Default": {Name: "Default"}, + "default": {Name: "default"}, + }, + }, + }, + }, + expectedAmbiguous: true, + }, + { + name: "should return true if Default and '' versions are found", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + VersionData: apidef.VersionData{ + NotVersioned: true, + DefaultVersion: "", + Versions: map[string]apidef.VersionInfo{ + "": {Name: "empty"}, + "Default": {Name: "Default"}, + }, + }, + }, + }, + expectedAmbiguous: true, + }, + { + name: "should return true if default and '' versions are found", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + VersionData: apidef.VersionData{ + NotVersioned: true, + DefaultVersion: "", + Versions: map[string]apidef.VersionInfo{ + "": {Name: "empty"}, + "default": {Name: "default"}, + }, + }, + }, + }, + expectedAmbiguous: true, + }, + { + name: "should return true if all default versions are found", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + VersionData: apidef.VersionData{ + NotVersioned: true, + DefaultVersion: "", + Versions: map[string]apidef.VersionInfo{ + "": {Name: "empty"}, + "Default": {Name: "Default"}, + "default": {Name: "default"}, + }, + }, + }, + }, + expectedAmbiguous: true, + }, + } + + for _, tc := range testCases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + assert.Equal(t, tc.expectedAmbiguous, tc.spec.CheckForAmbiguousDefaultVersions()) + }) + } +} + +func TestAPISpec_Version(t *testing.T) { + t.Run("for not_versioned set to true", func(t *testing.T) { + type testCase struct { + name string + spec *APISpec + expectedVersion *apidef.VersionInfo + expectedRequestStatus RequestStatus + } + + testCases := []testCase{ + { + name: "should return the single or default version of the API", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + VersionData: apidef.VersionData{ + NotVersioned: true, + Versions: map[string]apidef.VersionInfo{ + "v1": {Name: "v1"}, + "Default": {Name: "Default"}, + }, + }, + }, + }, + expectedVersion: &apidef.VersionInfo{Name: "Default"}, + expectedRequestStatus: StatusOk, + }, + { + name: "should return RequestStatus VersionDefaultForNotVersionedNotFound if no default version can be found", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + VersionData: apidef.VersionData{ + NotVersioned: true, + Versions: map[string]apidef.VersionInfo{ + "v1": {Name: "v1"}, + "v2": {Name: "v2"}, + }, + }, + }, + }, + expectedVersion: nil, + expectedRequestStatus: VersionDefaultForNotVersionedNotFound, + }, + { + name: "should return RequestStatus VersionAmbiguousDefault if multiple default version can be found", + spec: &APISpec{ + APIDefinition: &apidef.APIDefinition{ + VersionData: apidef.VersionData{ + NotVersioned: true, + Versions: map[string]apidef.VersionInfo{ + "default": {Name: "default"}, + "Default": {Name: "Default"}, + }, + }, + }, + }, + expectedVersion: nil, + expectedRequestStatus: VersionAmbiguousDefault, + }, + } + + for _, tc := range testCases { + tc := tc + + t.Run(tc.name, func(t *testing.T) { + r := &http.Request{} + versionInfo, requestStatus := tc.spec.Version(r) + assert.Equal(t, tc.expectedVersion, versionInfo) + assert.Equal(t, tc.expectedRequestStatus, requestStatus) + }) + } + }) + +} From 417de4e4c25a0890140271e355ff5933a55d8ccb Mon Sep 17 00:00:00 2001 From: Patric Vormstein Date: Sat, 27 Sep 2025 16:42:09 +0200 Subject: [PATCH 2/2] improve read experience --- gateway/api_definition.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gateway/api_definition.go b/gateway/api_definition.go index 2d226bb1f29..b44c5a94006 100644 --- a/gateway/api_definition.go +++ b/gateway/api_definition.go @@ -1720,7 +1720,7 @@ func (a *APISpec) GetSingleOrDefaultVersion() (versionInfo apidef.VersionInfo, o // 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 != "" { + if !a.VersionData.NotVersioned && a.VersionData.DefaultVersion != "" { versionInfo, ok = a.VersionData.Versions[a.VersionData.DefaultVersion] return versionInfo, ok }