Skip to content

Commit dbabb5f

Browse files
authored
[TT-7524] [OAS] Gateway CE behaves differently from Dashboard for middleware and PATCH (#7261)
### **User description** <details open> <summary><a href="https://tyktech.atlassian.net/browse/TT-7524" title="TT-7524" target="_blank">TT-7524</a></summary> <br /> <table> <tr> <th>Summary</th> <td>[OAS] Gateway CE behaves differently from Dashboard for middleware and PATCH</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></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 Updating an API with the PATCH /tyk/apis/oas/{apiId} command via Gateway API does not trigger the Tyk vendor extension (x-tyk-api-gateway) update, which results in the API description being out of sync or invalid. ## 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, Tests ___ ### **Description** - Fix removal of obsolete operations in Tyk extension during OAS PATCH. - Add test to ensure outdated operations are deleted. - Enhance middleware import logic for operation cleanup. - Improve consistency between Gateway and Dashboard OAS handling. ___ ### Diagram Walkthrough ```mermaid flowchart LR PATCH_OAS["PATCH OAS API"] -- "Triggers" --> importMiddlewares["importMiddlewares updates operations"] importMiddlewares -- "Collects current ops" --> removeObsolete["removeObsoleteOperations removes outdated ops"] removeObsolete -- "Cleans up" --> TykExtension["Tyk Extension up-to-date"] default_test["Add test for obsolete ops removal"] -- "Verifies" --> removeObsolete ``` <details> <summary><h3> File Walkthrough</h3></summary> <table><thead><tr><th></th><th align="left">Relevant files</th></tr></thead><tbody><tr><td><strong>Bug fix</strong></td><td><table> <tr> <td> <details> <summary><strong>default.go</strong><dd><code>Remove obsolete operations from Tyk extension during import</code></dd></summary> <hr> apidef/oas/default.go <ul><li>Track current operations during middleware import.<br> <li> Add <code>removeObsoleteOperations</code> to delete outdated operations.<br> <li> Call cleanup after updating operations.<br> <li> Use <code>slices.Contains</code> for efficient operation checks.</ul> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7261/files#diff-83c3a85bdd05785178ee519b05b1fe2008435dc4ae9448d72b080b5f67c491ad">+21/-0</a>&nbsp; &nbsp; </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>default_test.go</strong><dd><code>Test removal of obsolete operations from Tyk extension</code>&nbsp; &nbsp; &nbsp; </dd></summary> <hr> apidef/oas/default_test.go <ul><li>Add test to verify removal of outdated operations.<br> <li> Ensure Tyk extension matches expected operations after update.</ul> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7261/files#diff-ab6848f71731083885a9d7d7970faa68a6783a98477c78413ae3979cb5add7db">+28/-0</a>&nbsp; &nbsp; </td> </tr> </table></td></tr></tr></tbody></table> </details> ___
1 parent 8b4fa8e commit dbabb5f

File tree

2 files changed

+49
-0
lines changed

2 files changed

+49
-0
lines changed

apidef/oas/default.go

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import (
66
"net/http"
77
"net/url"
88
"reflect"
9+
"slices"
910
"strconv"
1011
"strings"
1112

@@ -220,22 +221,42 @@ func (s *OAS) importMiddlewares(overRideValues TykExtensionConfigParams) {
220221
xTykAPIGateway.Middleware = &Middleware{}
221222
}
222223

224+
currentOperations := make([]string, 0)
225+
223226
for path, pathItem := range s.Paths.Map() {
224227
overRideValues.pathItemHasParameters = len(pathItem.Parameters) > 0
225228
for _, method := range allowedMethods {
226229
if operation := pathItem.GetOperation(method); operation != nil {
227230
tykOperation := s.getTykOperation(method, path)
228231
tykOperation.Import(operation, overRideValues)
232+
currentOperations = append(currentOperations, s.getOperationID(path, method))
229233
s.deleteTykOperationIfEmpty(tykOperation, method, path)
230234
}
231235
}
232236
}
233237

238+
s.removeObsoleteOperations(currentOperations)
239+
234240
if ShouldOmit(xTykAPIGateway.Middleware) {
235241
xTykAPIGateway.Middleware = nil
236242
}
237243
}
238244

245+
func (s *OAS) removeObsoleteOperations(currentOperations []string) {
246+
tykOperations := s.getTykOperations()
247+
obsoleteOperations := make([]string, 0)
248+
249+
for id := range tykOperations {
250+
if !slices.Contains(currentOperations, id) {
251+
obsoleteOperations = append(obsoleteOperations, id)
252+
}
253+
}
254+
255+
for _, operationID := range obsoleteOperations {
256+
delete(tykOperations, operationID)
257+
}
258+
}
259+
239260
func (s *OAS) getTykOperation(method, path string) *Operation {
240261
xTykAPIGateway := s.GetTykExtension()
241262
operationID := s.getOperationID(path, method)

apidef/oas/default_test.go

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -563,6 +563,34 @@ func TestOAS_BuildDefaultTykExtension(t *testing.T) {
563563
return operations
564564
}
565565

566+
t.Run("operations not present in new oas paths definition should be removed", func(t *testing.T) {
567+
fakeOperationName := "fakeOperation"
568+
fakeOperation := &Operation{
569+
MockResponse: &MockResponse{
570+
Enabled: true,
571+
},
572+
}
573+
oasDef := getOASDef(true, true)
574+
575+
tykExtensionConfigParams := TykExtensionConfigParams{
576+
MockResponse: &trueVal,
577+
}
578+
579+
extension := &XTykAPIGateway{
580+
Middleware: &Middleware{
581+
Operations: map[string]*Operation{fakeOperationName: fakeOperation},
582+
},
583+
}
584+
oasDef.SetTykExtension(extension)
585+
assert.Greater(t, len(oasDef.getTykOperations()), 0)
586+
587+
expectedOperations := getExpectedOperations(true, true, middlewareMockResponse)
588+
err := oasDef.BuildDefaultTykExtension(tykExtensionConfigParams, true)
589+
590+
assert.NoError(t, err)
591+
assert.Equal(t, expectedOperations, oasDef.getTykOperations())
592+
})
593+
566594
t.Run("allowList", func(t *testing.T) {
567595
t.Run("enable allowList for all paths when no configured operationID in OAS", func(t *testing.T) {
568596
oasDef := getOASDef(false, false)

0 commit comments

Comments
 (0)