Skip to content

Commit 7e70a02

Browse files
authored
[TT-14666], fix for panic on gateway side when upgrading old oas file (#7034)
### **User description** <details open> <summary><a href="https://tyktech.atlassian.net/browse/TT-14666" title="TT-14666" target="_blank">TT-14666</a></summary> <br /> <table> <tr> <th>Summary</th> <td>GW panics after updating from 5.0 with OAS API</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>-</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 Fixes a gateway panic occurring when upgrading directly from release-5-lts to the cve-fix branch. The issue is triggered during API loading, specifically when a simple OAS API (external + active) was created in a prior version. This bug does not appear if an intermediate upgrade (e.g., to 5.1 or 5.8.0) is performed. JIRA LINK: https://tyktech.atlassian.net/browse/TT-14666 <!-- Describe your changes in detail --> ## 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: --> - [ ] 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) - [ ] 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 ___ ### **Description** - Prevents gateway panic when loading invalid OAS APIs after upgrade - Adds nil check for `chainObj` in `loadHTTPService` function - Returns error if attempting to import invalid API specification - Improves robustness during direct upgrades from older versions ___ ### **Changes walkthrough** 📝 <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>api_loader.go</strong><dd><code>Add nil check and error return for invalid API spec</code>&nbsp; &nbsp; &nbsp; &nbsp; &nbsp; &nbsp; </dd></summary> <hr> gateway/api_loader.go <li>Adds a nil check for <code>chainObj</code> after processing API spec<br> <li> Returns an error if <code>chainObj</code> is nil, preventing panic<br> <li> Logs the invalid API import attempt with API ID </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/7034/files#diff-cdf0b7f176c9d18e1a314b78ddefc2cb3a94b3de66f1f360174692c915734c68">+4/-0</a>&nbsp; &nbsp; &nbsp; </td> </tr> </table></td></tr></tr></tbody></table> ___ > <details> <summary> Need help?</summary><li>Type <code>/help how to ...</code> in the comments thread for any questions about PR-Agent usage.</li><li>Check out the <a href="https://qodo-merge-docs.qodo.ai/usage-guide/">documentation</a> for more information.</li></details>
1 parent 65493c0 commit 7e70a02

File tree

2 files changed

+98
-0
lines changed

2 files changed

+98
-0
lines changed

gateway/api_loader.go

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -800,6 +800,10 @@ func (gw *Gateway) loadHTTPService(spec *APISpec, apisByListen map[string]int, g
800800
chainObj = gw.processSpec(spec, apisByListen, gs, logrus.NewEntry(log))
801801
}
802802

803+
if chainObj == nil {
804+
return nil, fmt.Errorf("trying to import invalid api %s, skipping", spec.APIID)
805+
}
806+
803807
if chainObj.Skip {
804808
return chainObj, nil
805809
}

gateway/api_loader_test.go

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import (
1010
"path"
1111
_ "path"
1212
"reflect"
13+
"sync"
1314
"sync/atomic"
1415
"testing"
1516

@@ -19,6 +20,7 @@ import (
1920
"github.com/TykTechnologies/tyk/apidef"
2021
"github.com/TykTechnologies/tyk/config"
2122
"github.com/TykTechnologies/tyk/internal/uuid"
23+
"github.com/TykTechnologies/tyk/storage"
2224
"github.com/TykTechnologies/tyk/test"
2325
"github.com/TykTechnologies/tyk/trace"
2426
"github.com/TykTechnologies/tyk/user"
@@ -9624,3 +9626,95 @@ func TestSortSpecsByListenPath(t *testing.T) {
96249626
})
96259627
}
96269628
}
9629+
9630+
func TestForTyk5OasMigration(t *testing.T) {
9631+
g := StartTest(nil)
9632+
defer g.Close()
9633+
9634+
internal := BuildAPI(func(spec *APISpec) {
9635+
spec.OAS.Extensions = nil
9636+
spec.IsOAS = true
9637+
spec.VersionData.Versions = map[string]apidef.VersionInfo{"a": {ExtendedPaths: apidef.ExtendedPathsSet{ValidateRequest: []apidef.ValidateRequestMeta{{Enabled: true}}}}}
9638+
spec.Name = "test-for-legacy-oas"
9639+
spec.APIID = "test-legacy-oas-api-id"
9640+
spec.Proxy.ListenPath = "/test-for-legacy-oas"
9641+
spec.AuthManager = MockSessionHandler{}
9642+
spec.Health = MockHealthChecker{}
9643+
spec.OrgSessionManager = MockSessionHandler{}
9644+
})[0]
9645+
9646+
g.Gw.apisMu.Lock()
9647+
g.Gw.apisByID[internal.APIID] = internal
9648+
g.Gw.apisMu.Unlock()
9649+
9650+
if g.Gw.apisHandlesByID == nil {
9651+
g.Gw.apisHandlesByID = new(sync.Map)
9652+
}
9653+
g.Gw.apisHandlesByID.Delete(internal.APIID)
9654+
9655+
apiByListenPath := map[string]int{}
9656+
9657+
_, err := g.Gw.loadHTTPService(internal, apiByListenPath, &generalStores{}, &proxyMux{})
9658+
9659+
assert.Equal(t, fmt.Sprint(err.Error()), "trying to import invalid api test-legacy-oas-api-id, skipping")
9660+
}
9661+
9662+
type MockSessionHandler struct{}
9663+
9664+
func (MockSessionHandler) Init(storage.Handler) {
9665+
}
9666+
9667+
func (MockSessionHandler) Store() storage.Handler {
9668+
//TODO implement me
9669+
panic("implement me")
9670+
}
9671+
9672+
func (MockSessionHandler) UpdateSession(string, *user.SessionState, int64, bool) error {
9673+
//TODO implement me
9674+
panic("implement me")
9675+
}
9676+
9677+
func (MockSessionHandler) RemoveSession(string, string, bool) bool {
9678+
//TODO implement me
9679+
panic("implement me")
9680+
}
9681+
9682+
func (MockSessionHandler) SessionDetail(string, string, bool) (user.SessionState, bool) {
9683+
//TODO implement me
9684+
panic("implement me")
9685+
}
9686+
9687+
func (MockSessionHandler) KeyExpired(*user.SessionState) bool {
9688+
//TODO implement me
9689+
panic("implement me")
9690+
}
9691+
9692+
func (MockSessionHandler) Sessions(string) []string {
9693+
//TODO implement me
9694+
panic("implement me")
9695+
}
9696+
9697+
func (MockSessionHandler) ResetQuota(string, *user.SessionState, bool) {
9698+
//TODO implement me
9699+
panic("implement me")
9700+
}
9701+
9702+
func (MockSessionHandler) Stop() {
9703+
//TODO implement me
9704+
panic("implement me")
9705+
}
9706+
9707+
type MockHealthChecker struct{}
9708+
9709+
func (MockHealthChecker) Init(storage.Handler) {
9710+
}
9711+
9712+
func (MockHealthChecker) ApiHealthValues() (HealthCheckValues, error) {
9713+
//TODO implement me
9714+
panic("implement me")
9715+
}
9716+
9717+
func (MockHealthChecker) StoreCounterVal(HealthPrefix, string) {
9718+
//TODO implement me
9719+
panic("implement me")
9720+
}

0 commit comments

Comments
 (0)