Skip to content

Commit 5460a49

Browse files
authored
[TT-7523] [OAS Versioning] Gateway CE allows to create version without new_version_name (#7244)
### **User description** <details open> <summary><a href="https://tyktech.atlassian.net/browse/TT-7523" title="TT-7523" target="_blank">TT-7523</a></summary> <br /> <table> <tr> <th>Summary</th> <td>[OAS Versioning] Gateway CE allows to create version without `new_version_name`</td> </tr> <tr> <th>Type</th> <td> <img alt="Bug" src="https://tyktech.atlassian.net/rest/api/2/universal_avatar/view/type/issuetype/avatar/10303?size=medium" /> Bug </td> </tr> <tr> <th>Status</th> <td>In Dev</td> </tr> <tr> <th>Points</th> <td>N/A</td> </tr> <tr> <th>Labels</th> <td><a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20codilime_refined%20ORDER%20BY%20created%20DESC" title="codilime_refined">codilime_refined</a>, <a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20version-tyk-oas%20ORDER%20BY%20created%20DESC" title="version-tyk-oas">version-tyk-oas</a></td> </tr> </table> </details> <!-- do not remove this marker as it will break jira-lint's functionality. added_by_jira_lint --> --- <!-- Provide a general summary of your changes in the Title above --> ## Description Gateway API accepts requests to create new API version even if the new version name parameter is not specified. This PR contains fix for this issue along with example, how a shared library could be used to extract common code. ## Related Issue <!-- This project only accepts pull requests related to open issues. --> <!-- If suggesting a new feature or change, please discuss it in an issue first. --> <!-- If fixing a bug, there should be an issue describing it with steps to reproduce. --> <!-- OSS: Please link to the issue here. Tyk: please create/link the JIRA ticket. --> ## Motivation and Context <!-- Why is this change required? What problem does it solve? --> ## How This Has Been Tested <!-- Please describe in detail how you tested your changes --> <!-- Include details of your testing environment, and the tests --> <!-- you ran to see how your change affects other areas of the code, etc. --> <!-- This information is helpful for reviewers and QA. --> ## Screenshots (if appropriate) ## Types of changes <!-- What types of changes does your code introduce? Put an `x` in all the boxes that apply: --> - [x] 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) - [x] Refactoring or add test (improvements in base code or adds test coverage to functionality) ## Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply --> <!-- If there are no documentation updates required, mark the item as checked. --> <!-- Raise up any additional concerns not covered by the 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 ___ ### **PR Type** Bug fix, Enhancement, Tests ___ ### **Description** - Enforces validation for `new_version_name` when creating API versions - Returns HTTP 422 if `new_version_name` is missing in versioning requests - Refactors versioning logic into a shared library (`lib/apidef/version.go`) - Adds comprehensive unit tests for versioning logic in shared library ___ ### Diagram Walkthrough ```mermaid flowchart LR apiHandler["Gateway API Versioning Handler"] sharedLib["Shared Versioning Library (lib/apidef/version.go)"] validation["Validation for new_version_name"] error422["Returns HTTP 422 on missing new_version_name"] tests["Unit Tests for Versioning Logic"] apiHandler -- "Uses" --> sharedLib sharedLib -- "Performs" --> validation validation -- "On error" --> error422 sharedLib -- "Covered by" --> tests ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Enhancement</strong></td><td><table> <tr> <td> <details> <summary><strong>api.go</strong><dd><code>Enforce and refactor API versioning logic using shared library</code></dd></summary> <hr> gateway/api.go <ul><li>Integrates shared versioning library for parameter handling and <br>validation<br> <li> Enforces <code>new_version_name</code> presence, returning HTTP 422 if missing<br> <li> Refactors versioning logic to use new shared library functions<br> <li> Simplifies and clarifies API versioning code path</ul> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7244/files#diff-644cda3aeb4ac7f325359e85fcddb810f100dd5e6fa480b0d9f9363a743c4e05">+46/-56</a>&nbsp; </td> </tr> <tr> <td> <details> <summary><strong>version.go</strong><dd><code>Add shared library for API versioning logic and validation</code></dd></summary> <hr> lib/apidef/version.go <ul><li>Introduces shared library for API versioning parameter handling and <br>validation<br> <li> Implements strict validation for required parameters (e.g., <br><code>new_version_name</code>)<br> <li> Provides utility functions for configuring version definitions<br> <li> Centralizes versioning logic for reuse and maintainability</ul> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7244/files#diff-9e698644fcca1a469641d3cd92ad309f640e4f8474b6d4fbe9478123516f180d">+181/-0</a>&nbsp; </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>version_test.go</strong><dd><code>Add unit tests for shared versioning library</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> lib/apidef/version_test.go <ul><li>Adds comprehensive unit tests for versioning parameter logic and <br>validation<br> <li> Tests error handling for missing and invalid parameters<br> <li> Verifies configuration of version definitions via shared library<br> <li> Ensures robustness of new versioning logic</ul> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7244/files#diff-c67df2864ba1a068ada18f017570b6190f6af0e1f1515f3110a81f59a7da42e6">+181/-0</a>&nbsp; </td> </tr> </table></td></tr></tr></tbody></table> </details> ___
1 parent 42923e2 commit 5460a49

File tree

4 files changed

+426
-59
lines changed

4 files changed

+426
-59
lines changed

gateway/api.go

Lines changed: 36 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ import (
6969
"github.com/TykTechnologies/tyk/user"
7070

7171
gql "github.com/TykTechnologies/graphql-go-tools/pkg/graphql"
72+
lib "github.com/TykTechnologies/tyk/lib/apidef"
7273
)
7374

7475
const (
@@ -1049,14 +1050,32 @@ func (gw *Gateway) handleGetAPIOAS(apiID string, modePublic bool) (interface{},
10491050

10501051
func (gw *Gateway) handleAddApi(r *http.Request, fs afero.Fs, oasEndpoint bool) (interface{}, int) {
10511052
var (
1052-
newDef apidef.APIDefinition
1053-
oasObj oas.OAS
1054-
baseAPIID = r.FormValue("base_api_id")
1055-
baseAPIVersionName = r.FormValue("base_api_version_name")
1056-
newVersionName = r.FormValue("new_version_name")
1057-
setDefault = r.FormValue("set_default") == "true"
1053+
newDef apidef.APIDefinition
1054+
oasObj oas.OAS
10581055
)
10591056

1057+
versionParams := lib.NewVersionQueryParameters(r.URL.Query())
1058+
err := versionParams.Validate(func() (bool, string) {
1059+
baseApiID := versionParams.Get(lib.BaseAPIID)
1060+
baseApi := gw.getApiSpec(baseApiID)
1061+
if baseApi != nil {
1062+
return true, baseApi.VersionDefinition.Name
1063+
}
1064+
1065+
return false, ""
1066+
})
1067+
1068+
if err != nil {
1069+
// https://tyktech.atlassian.net/browse/TT-7523?focusedCommentId=100547
1070+
// Sadly we are averse to changing (incorrect) HTTP error codes, because these could be considered breaking changes by some of our clients.
1071+
// Please return HTTP 422 here, because currently the request doesn’t generate an error.
1072+
if errors.Is(err, lib.ErrNewVersionRequired) {
1073+
return apiError(err.Error()), http.StatusUnprocessableEntity
1074+
}
1075+
1076+
return apiError(err.Error()), http.StatusBadRequest
1077+
}
1078+
10601079
if oasEndpoint {
10611080
if err := json.NewDecoder(r.Body).Decode(&oasObj); err != nil {
10621081
log.Error("Couldn't decode new OAS object: ", err)
@@ -1102,60 +1121,20 @@ func (gw *Gateway) handleAddApi(r *http.Request, fs afero.Fs, oasEndpoint bool)
11021121
}
11031122
}
11041123

1105-
if baseAPIID != "" {
1106-
if baseAPIPtr := gw.getApiSpec(baseAPIID); baseAPIPtr == nil {
1107-
log.Errorf("Couldn't find a base API to bind with the given API id: %s", baseAPIID)
1108-
} else {
1109-
apiInBytes, err := json.Marshal(baseAPIPtr)
1110-
if err != nil {
1111-
log.WithError(err).Error("Couldn't marshal API spec")
1112-
}
1124+
if !versionParams.IsEmpty(lib.BaseAPIID) {
1125+
baseAPI := gw.getApiSpec(versionParams.Get(lib.BaseAPIID))
1126+
baseAPI.VersionDefinition = lib.ConfigureVersionDefinition(baseAPI.VersionDefinition, versionParams, newDef.APIID)
11131127

1114-
var baseAPI APISpec
1115-
err = json.Unmarshal(apiInBytes, &baseAPI)
1128+
if baseAPI.IsOAS {
1129+
baseAPI.OAS.Fill(*baseAPI.APIDefinition)
1130+
err, _ := gw.writeOASAndAPIDefToFile(fs, baseAPI.APIDefinition, &baseAPI.OAS)
11161131
if err != nil {
1117-
log.WithError(err).Error("Couldn't unmarshal API spec")
1118-
}
1119-
1120-
baseAPI.VersionDefinition.Enabled = true
1121-
if baseAPIVersionName != "" {
1122-
baseAPI.VersionDefinition.Name = baseAPIVersionName
1123-
baseAPI.VersionDefinition.Default = baseAPIVersionName
1124-
}
1125-
1126-
if baseAPI.VersionDefinition.Key == "" {
1127-
baseAPI.VersionDefinition.Key = apidef.DefaultAPIVersionKey
1128-
}
1129-
1130-
if baseAPI.VersionDefinition.Location == "" {
1131-
baseAPI.VersionDefinition.Location = apidef.HeaderLocation
1132-
}
1133-
1134-
if baseAPI.VersionDefinition.Default == "" {
1135-
baseAPI.VersionDefinition.Default = apidef.Self
1136-
}
1137-
1138-
if baseAPI.VersionDefinition.Versions == nil {
1139-
baseAPI.VersionDefinition.Versions = make(map[string]string)
1140-
}
1141-
1142-
baseAPI.VersionDefinition.Versions[newVersionName] = newDef.APIID
1143-
1144-
if setDefault {
1145-
baseAPI.VersionDefinition.Default = newVersionName
1132+
log.WithError(err).Errorf("Error occurred while updating base OAS API with id: %s", baseAPI.APIID)
11461133
}
1147-
1148-
if baseAPI.IsOAS {
1149-
baseAPI.OAS.Fill(*baseAPI.APIDefinition)
1150-
err, _ := gw.writeOASAndAPIDefToFile(fs, baseAPI.APIDefinition, &baseAPI.OAS)
1151-
if err != nil {
1152-
log.WithError(err).Errorf("Error occurred while updating base OAS API with id: %s", baseAPI.APIID)
1153-
}
1154-
} else {
1155-
err, _ := gw.writeToFile(fs, baseAPI.APIDefinition, baseAPI.APIID)
1156-
if err != nil {
1157-
log.WithError(err).Errorf("Error occurred while updating base API with id: %s", baseAPI.APIID)
1158-
}
1134+
} else {
1135+
err, _ := gw.writeToFile(fs, baseAPI.APIDefinition, baseAPI.APIID)
1136+
if err != nil {
1137+
log.WithError(err).Errorf("Error occurred while updating base API with id: %s", baseAPI.APIID)
11591138
}
11601139
}
11611140
}

gateway/api_test.go

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2189,7 +2189,8 @@ func TestHandleAddApi_AddVersionAtomically(t *testing.T) {
21892189
{AdminAuth: true, Method: http.MethodPost, Data: v2,
21902190
Path: fmt.Sprintf("/tyk/apis?base_api_id=%s&new_version_name=%s&set_default=true&base_api_version_name=%s", baseAPI.APIID, v2VersionName, baseVersionName),
21912191
BodyMatchFunc: func(i []byte) bool {
2192-
assert.Len(t, baseAPI.VersionDefinition.Versions, 0)
2192+
// Gateway addApi function modifies baseAPI in it's internal storage - gw.apisByID
2193+
assert.Len(t, baseAPI.VersionDefinition.Versions, 1)
21932194
ts.Gw.DoReload()
21942195
return true
21952196
},
@@ -2267,7 +2268,8 @@ func TestHandleAddOASApi_AddVersionAtomically(t *testing.T) {
22672268
{AdminAuth: true, Method: http.MethodPost, Data: &v2.OAS,
22682269
Path: fmt.Sprintf("/tyk/apis/oas?base_api_id=%s&new_version_name=%s&set_default=true&base_api_version_name=%s", baseAPI.APIID, v2VersionName, baseVersionName),
22692270
BodyMatchFunc: func(i []byte) bool {
2270-
assert.Len(t, baseAPI.VersionDefinition.Versions, 0)
2271+
// Gateway addApi function modifies baseAPI in it's internal storage - gw.apisByID
2272+
assert.Len(t, baseAPI.VersionDefinition.Versions, 1)
22712273
ts.Gw.DoReload()
22722274
return true
22732275
},

lib/apidef/version.go

Lines changed: 198 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,198 @@
1+
package apidef
2+
3+
import (
4+
"errors"
5+
"fmt"
6+
"github.com/TykTechnologies/tyk/apidef"
7+
"github.com/TykTechnologies/tyk/common/option"
8+
"net/url"
9+
)
10+
11+
const (
12+
// BaseAPIID represents the parameter for the base API identifier.
13+
// This is used to identify the original API that a version is based on.
14+
BaseAPIID VersionParameter = iota
15+
16+
// BaseAPIVersionName represents the parameter for the base API version name.
17+
// This is used to specify the name of the version in the base API.
18+
BaseAPIVersionName
19+
20+
// NewVersionName represents the parameter for the new version name.
21+
// This is used when creating a new version of an API.
22+
NewVersionName
23+
24+
// SetDefault represents the parameter that determines if a version should be set as default.
25+
// When true, the new version will be marked as the default version for the API.
26+
SetDefault
27+
28+
// paramCount is used internally to track the number of version parameters.
29+
paramCount
30+
)
31+
32+
const TrueString = "true"
33+
34+
var (
35+
// ErrNewVersionRequired is returned when a new version name is required but not provided.
36+
// This error occurs during validation when attempting to create a new API version.
37+
ErrNewVersionRequired = errors.New("The new version name is required")
38+
)
39+
40+
// VersionParameter represents the type of parameter used in API version configuration.
41+
// It defines the possible parameters that can be used when working with API versions.
42+
type VersionParameter int
43+
44+
// String returns the string representation of a VersionParameter.
45+
// It converts the numeric parameter value to its corresponding string identifier.
46+
func (v VersionParameter) String() string {
47+
return []string{"base_api_id", "base_api_version_name", "new_version_name", "set_default"}[v]
48+
}
49+
50+
// AllVersionParameters returns a slice containing all available version parameters.
51+
// This is useful for iterating through all possible version parameters.
52+
func AllVersionParameters() []VersionParameter {
53+
params := make([]VersionParameter, paramCount)
54+
for i := range params {
55+
params[i] = VersionParameter(i)
56+
}
57+
58+
return params
59+
}
60+
61+
// VersionQueryParameters holds the version-related parameters extracted from HTTP requests.
62+
// It provides methods to access and validate these parameters.
63+
type VersionQueryParameters struct {
64+
versionParams map[string]string
65+
}
66+
67+
// Validate checks if the version parameters are valid.
68+
// It takes a function that checks if the base API exists and returns an error if validation fails.
69+
// The doesBaseApiExists function should return whether the base API exists and its name.
70+
func (v *VersionQueryParameters) Validate(doesBaseApiExists func() (bool, string)) error {
71+
if v.IsEmpty(BaseAPIID) {
72+
return nil
73+
}
74+
baseID := v.Get(BaseAPIID)
75+
76+
if v.IsEmpty(NewVersionName) {
77+
return ErrNewVersionRequired
78+
}
79+
80+
exists, baseName := doesBaseApiExists()
81+
if !exists {
82+
return fmt.Errorf("%s is not a valid Base API version", baseID)
83+
}
84+
85+
if v.IsEmpty(BaseAPIVersionName) && baseName == "" {
86+
return fmt.Errorf("you need to provide a version name for the new Base API: %s", baseID)
87+
}
88+
89+
return nil
90+
}
91+
92+
// IsEmpty checks if a specific version parameter is empty or not set.
93+
// Returns true if the parameter is empty, false otherwise.
94+
func (v *VersionQueryParameters) IsEmpty(param VersionParameter) bool {
95+
return v.versionParams[param.String()] == ""
96+
}
97+
98+
// Get retrieves the value of a specific version parameter.
99+
// Returns the string value of the parameter.
100+
func (v *VersionQueryParameters) Get(param VersionParameter) string {
101+
return v.versionParams[param.String()]
102+
}
103+
104+
// NewVersionQueryParameters creates a new VersionQueryParameters instance from an HTTP request.
105+
// It extracts all version-related parameters from the request's URL query.
106+
func NewVersionQueryParameters(query url.Values) *VersionQueryParameters {
107+
versionParams := &VersionQueryParameters{
108+
versionParams: make(map[string]string, paramCount),
109+
}
110+
111+
for _, param := range AllVersionParameters() {
112+
paramName := param.String()
113+
versionParams.versionParams[paramName] = query.Get(paramName)
114+
}
115+
116+
return versionParams
117+
}
118+
119+
// WithVersionName creates an option that sets the version name in a VersionDefinition.
120+
func WithVersionName(name string) option.Option[apidef.VersionDefinition] {
121+
return func(version *apidef.VersionDefinition) {
122+
version.Name = name
123+
}
124+
}
125+
126+
// WithBaseID creates an option that sets the version baseID in a VersionDefinition.
127+
func WithBaseID(id string) option.Option[apidef.VersionDefinition] {
128+
return func(version *apidef.VersionDefinition) {
129+
version.BaseID = id
130+
}
131+
}
132+
133+
// AddVersion creates an option that adds a version mapping to a VersionDefinition.
134+
// It associates a version name with an API ID in the Versions map.
135+
func AddVersion(versionName, apiID string) option.Option[apidef.VersionDefinition] {
136+
return func(vd *apidef.VersionDefinition) {
137+
vd.Versions[versionName] = apiID
138+
}
139+
}
140+
141+
// SetAsDefault creates an option that marks a specific version as the default.
142+
// This sets the Default field in the VersionDefinition to the specified version name.
143+
func SetAsDefault(versionName string) option.Option[apidef.VersionDefinition] {
144+
return func(vd *apidef.VersionDefinition) {
145+
vd.Default = versionName
146+
}
147+
}
148+
149+
// ConfigureVersionDefinition sets up the version definition with default values if not already set.
150+
// It applies the provided parameters to configure the version definition and ensures
151+
// that required fields have appropriate values.
152+
func ConfigureVersionDefinition(def apidef.VersionDefinition, params *VersionQueryParameters, newApiID string) apidef.VersionDefinition {
153+
opts := make([]option.Option[apidef.VersionDefinition], 0)
154+
155+
if !params.IsEmpty(BaseAPIID) {
156+
def.Enabled = true
157+
158+
if !params.IsEmpty(BaseAPIVersionName) {
159+
opts = append(opts, WithVersionName(params.Get(BaseAPIVersionName)))
160+
}
161+
162+
if !params.IsEmpty(SetDefault) {
163+
setDefault := params.Get(SetDefault)
164+
if setDefault == TrueString {
165+
opts = append(opts, SetAsDefault(params.Get(NewVersionName)))
166+
}
167+
}
168+
169+
if !params.IsEmpty(BaseAPIID) {
170+
opts = append(opts, WithBaseID(params.Get(BaseAPIID)))
171+
}
172+
173+
opts = append(opts, AddVersion(params.Get(NewVersionName), newApiID))
174+
}
175+
176+
// When baseAPIID is missing in the request params, and it's versioning is enabled then set versioning ID as APIID
177+
if params.IsEmpty(BaseAPIID) && def.BaseID == "" {
178+
def.BaseID = newApiID
179+
}
180+
181+
if def.Key == "" {
182+
def.Key = apidef.DefaultAPIVersionKey
183+
}
184+
185+
if def.Location == "" {
186+
def.Location = apidef.HeaderLocation
187+
}
188+
189+
if def.Default == "" {
190+
def.Default = apidef.Self
191+
}
192+
193+
if def.Versions == nil {
194+
def.Versions = make(map[string]string)
195+
}
196+
197+
return *option.New(opts).Build(def)
198+
}

0 commit comments

Comments
 (0)