Skip to content

Commit 3984fa6

Browse files
authored
[TT-14666] gw panics after updating from 5 0 with oas api (#7046)
<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><a href="https://tyktech.atlassian.net/issues?jql=project%20%3D%20TT%20AND%20labels%20%3D%20QA_Fail%20ORDER%20BY%20created%20DESC" title="QA_Fail">QA_Fail</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 <!-- 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
1 parent 45ae588 commit 3984fa6

File tree

2 files changed

+42
-95
lines changed

2 files changed

+42
-95
lines changed

gateway/api_loader.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -800,10 +800,6 @@ 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-
807803
if chainObj.Skip {
808804
return chainObj, nil
809805
}
@@ -1007,7 +1003,9 @@ func (gw *Gateway) loadApps(specs []*APISpec) {
10071003
defer func() {
10081004
// recover from panic if one occurred. Set err to nil otherwise.
10091005
if err := recover(); err != nil {
1010-
log.Errorf("Panic while loading an API: %v, panic: %v, stacktrace: %v", spec.APIDefinition, err, string(debug.Stack()))
1006+
if err := recoverFromLoadApiPanic(spec, err); err != nil {
1007+
log.Error(err)
1008+
}
10111009
}
10121010
}()
10131011

@@ -1102,6 +1100,13 @@ func (gw *Gateway) loadApps(specs []*APISpec) {
11021100
}
11031101
}
11041102

1103+
func recoverFromLoadApiPanic(spec *APISpec, err any) error {
1104+
if spec.APIDefinition.IsOAS && spec.OAS.GetTykExtension() == nil {
1105+
return fmt.Errorf("trying to import invalid OAS api %s, skipping", spec.APIID)
1106+
}
1107+
return fmt.Errorf("Panic while loading an API: %v, panic: %v, stacktrace: %v", spec.APIDefinition, err, string(debug.Stack()))
1108+
}
1109+
11051110
func (gw *Gateway) allApisAreMTLS() bool {
11061111
gw.apisMu.RLock()
11071112
defer gw.apisMu.RUnlock()

gateway/api_loader_test.go

Lines changed: 32 additions & 90 deletions
Original file line numberDiff line numberDiff line change
@@ -10,17 +10,17 @@ import (
1010
"path"
1111
_ "path"
1212
"reflect"
13-
"sync"
13+
"strings"
1414
"sync/atomic"
1515
"testing"
1616

1717
"github.com/stretchr/testify/assert"
1818

1919
persistentmodel "github.com/TykTechnologies/storage/persistent/model"
2020
"github.com/TykTechnologies/tyk/apidef"
21+
"github.com/TykTechnologies/tyk/apidef/oas"
2122
"github.com/TykTechnologies/tyk/config"
2223
"github.com/TykTechnologies/tyk/internal/uuid"
23-
"github.com/TykTechnologies/tyk/storage"
2424
"github.com/TykTechnologies/tyk/test"
2525
"github.com/TykTechnologies/tyk/trace"
2626
"github.com/TykTechnologies/tyk/user"
@@ -9627,94 +9627,36 @@ func TestSortSpecsByListenPath(t *testing.T) {
96279627
}
96289628
}
96299629

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)
9630+
func TestRecoverFromLoadApiPanic(t *testing.T) {
9631+
tests := []struct {
9632+
name string
9633+
spec *APISpec
9634+
err any
9635+
expected string
9636+
}{
9637+
{
9638+
name: "invalid OAS api",
9639+
spec: &APISpec{
9640+
APIDefinition: &apidef.APIDefinition{
9641+
APIID: "test-api",
9642+
IsOAS: true,
9643+
},
9644+
OAS: oas.OAS{},
9645+
},
9646+
err: "test error",
9647+
expected: "trying to import invalid OAS api test-api, skipping",
9648+
},
96529649
}
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-
}
97169650

9717-
func (MockHealthChecker) StoreCounterVal(HealthPrefix, string) {
9718-
//TODO implement me
9719-
panic("implement me")
9651+
for _, tt := range tests {
9652+
t.Run(tt.name, func(t *testing.T) {
9653+
err := recoverFromLoadApiPanic(tt.spec, tt.err)
9654+
if err == nil {
9655+
t.Fatal("expected error, got nil")
9656+
}
9657+
if !strings.Contains(err.Error(), tt.expected) {
9658+
t.Errorf("expected error to contain %q, got %q", tt.expected, err.Error())
9659+
}
9660+
})
9661+
}
97209662
}

0 commit comments

Comments
 (0)