improve error handling of streams in non-ee version (TT-13269) (#6691)

### **User description** <details open> <summary><a href="https://tyktech.atlassian.net/browse/TT-13269" title="TT-13269" target="_blank">TT-13269</a></summary> <br /> <table> <tr> <th>Summary</th> <td>[GW EE] Implement conditional compilation for streams in GW EE</td> </tr> <tr> <th>Type</th> <td> <img alt="Story" src="https://tyktech.atlassian.net/rest/api/2/universal_avatar/view/type/issuetype/avatar/10315?size=medium" /> Story </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>  --- This PR improves the error handling of streams API in Tyk Non-EE version. ## Types of changes  - [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) - [ ] Refactoring or add test (improvements in base code or adds test coverage to functionality) ___ ### **PR Type** Bug fix, Enhancement, Tests ___ ### **Description** - Introduced a new error variable `ErrActionNotAllowed` for handling restricted actions. - Added a method `isStreamingAPI` to determine if an API spec supports streaming. - Enhanced the `dummyStreamingMiddleware` to log errors and return appropriate HTTP status codes when streaming is not supported. - Added tests for the `isStreamingAPI` method to ensure correct functionality. ___ ### **Changes walkthrough** 📝 <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>errors.go</strong><dd><code>Introduce new error variable for action restrictions</code>          </dd></summary> <hr> ee/errors.go - Added a new error variable `ErrActionNotAllowed`. </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6691/files#diff-822f4595f51c8f7bb5132cf8efb90a3941203fe34129309c21ef8d0f2eb57e3b">+9/-0</a>      </td> </tr> <tr> <td> <details> <summary><strong>api_definition.go</strong><dd><code>Add method to check streaming API support</code>                                </dd></summary> <hr> gateway/api_definition.go <li>Added a new method <code>isStreamingAPI</code> to check for streaming API <br>extensions.<br> <li> Imported <code>streams</code> package for streaming API support.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6691/files#diff-0cf80174bbafb36f6d4f4308ebbd971b2833b76a936bad568220aa1a4ba0ee8b">+11/-1</a>    </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>api_definition_test.go</strong><dd><code>Add tests for streaming API detection method</code>                          </dd></summary> <hr> gateway/api_definition_test.go <li>Added tests for the new <code>isStreamingAPI</code> method.<br> <li> Included <code>streams</code> package in imports for testing.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6691/files#diff-2394daab6fdc5f8dc234699c80c0548947ee3d68d2e33858258d73a8b5eb6f44">+51/-0</a>    </td> </tr> </table></td></tr><tr><td><strong>Bug fix</strong></td><td><table> <tr> <td> <details> <summary><strong>mw_streaming.go</strong><dd><code>Improve error handling and logging for streaming middleware</code></dd></summary> <hr> gateway/mw_streaming.go <li>Enhanced error handling in <code>dummyStreamingMiddleware</code>.<br> <li> Added logging for unsupported streaming actions.<br> <li> Introduced constant message for unsupported streaming.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6691/files#diff-6f565750150d990575c808f1ca8f38483160dc6edf05f1534cd0bedb27c2e6c8">+12/-3</a>    </td> </tr> </table></td></tr></tr></tbody></table> ___ > 💡 **PR-Agent usage**: Comment `/help "your question"` on any pull request to receive relevant information
TykTechnologies · Nov 6, 2024 · 2611a47 · 2611a47
1 parent fa63dbe
commit 2611a47
Show file tree

Hide file tree

Showing 4 changed files with 83 additions and 4 deletions.
diff --git a/ee/errors.go b/ee/errors.go
@@ -0,0 +1,9 @@
+package ee
+
+import (
+	"errors"
+)
+
+var (
+	ErrActionNotAllowed = errors.New("action not allowed")
+)
diff --git a/gateway/api_definition.go b/gateway/api_definition.go
@@ -19,6 +19,7 @@ import (
 	texttemplate "text/template"
 	"time"
 
+	"github.com/TykTechnologies/tyk/ee/middleware/streams"
 	"github.com/TykTechnologies/tyk/storage/kv"
 
 	"github.com/getkin/kin-openapi/routers"
@@ -34,7 +35,7 @@ import (
 
 	"github.com/cenk/backoff"
 
-	sprig "github.com/Masterminds/sprig/v3"
+	"github.com/Masterminds/sprig/v3"
 
 	"github.com/sirupsen/logrus"
 
@@ -314,6 +315,15 @@ func (s *APISpec) validateHTTP() error {
 	return nil
 }
 
+func (s *APISpec) isStreamingAPI() bool {
+	if s.OAS.T.Extensions == nil {
+		return false
+	}
+
+	_, ok := s.OAS.T.Extensions[streams.ExtensionTykStreaming]
+	return ok
+}
+
 // APIDefinitionLoader will load an Api definition from a storage
 // system.
 type APIDefinitionLoader struct {

diff --git a/gateway/api_definition_test.go b/gateway/api_definition_test.go
@@ -15,12 +15,15 @@ import (
 	texttemplate "text/template"
 	"time"
 
+	"github.com/getkin/kin-openapi/openapi3"
 	"github.com/stretchr/testify/assert"
 
 	persistentmodel "github.com/TykTechnologies/storage/persistent/model"
+
 	"github.com/TykTechnologies/tyk/apidef"
 	"github.com/TykTechnologies/tyk/apidef/oas"
 	"github.com/TykTechnologies/tyk/config"
+	"github.com/TykTechnologies/tyk/ee/middleware/streams"
 	"github.com/TykTechnologies/tyk/internal/model"
 	"github.com/TykTechnologies/tyk/internal/policy"
 	"github.com/TykTechnologies/tyk/rpc"
@@ -1534,6 +1537,54 @@ func TestAPISpec_setHasMock(t *testing.T) {
 	assert.True(t, s.HasMock)
 }
 
+func TestAPISpec_isStreamingAPI(t *testing.T) {
+	type testCase struct {
+		name           string
+		inputOAS       oas.OAS
+		expectedResult bool
+	}
+
+	testCases := []testCase{
+		{
+			name:           "should return false if oas is set to default",
+			inputOAS:       oas.OAS{},
+			expectedResult: false,
+		},
+		{
+			name: "should return false if streaming section is missing",
+			inputOAS: oas.OAS{
+				T: openapi3.T{
+					Extensions: map[string]any{
+						"x-tyk-api-gateway": nil,
+					},
+				},
+			},
+			expectedResult: false,
+		},
+		{
+			name: "should return true if streaming section is present",
+			inputOAS: oas.OAS{
+				T: openapi3.T{
+					Extensions: map[string]any{
+						streams.ExtensionTykStreaming: nil,
+						"x-tyk-api-gateway":           nil,
+					},
+				},
+			},
+			expectedResult: true,
+		},
+	}
+
+	for _, tc := range testCases {
+		t.Run(tc.name, func(t *testing.T) {
+			apiSpec := &APISpec{
+				OAS: tc.inputOAS,
+			}
+			assert.Equal(t, tc.expectedResult, apiSpec.isStreamingAPI())
+		})
+	}
+}
+
 func TestReplaceSecrets(t *testing.T) {
 	ts := StartTest(func(globalConf *config.Config) {
 		globalConf.Secrets = map[string]string{

diff --git a/gateway/mw_streaming.go b/gateway/mw_streaming.go
@@ -5,6 +5,12 @@ package gateway
 
 import (
 	"net/http"
+
+	"github.com/TykTechnologies/tyk/ee"
+)
+
+const (
+	MessageStreamingOnlySupportedInEE = "streaming is supported only in Tyk Enterprise Edition"
 )
 
 func getStreamingMiddleware(base *BaseMiddleware) TykMiddleware {
@@ -16,14 +22,17 @@ type dummyStreamingMiddleware struct {
 }
 
 func (d *dummyStreamingMiddleware) ProcessRequest(w http.ResponseWriter, r *http.Request, _ interface{}) (error, int) {
-	return nil, http.StatusOK
+	failHttpCode := http.StatusForbidden
+	d.Logger().WithField("status_code", failHttpCode).Errorf("Error: %s", MessageStreamingOnlySupportedInEE)
+	return ee.ErrActionNotAllowed, failHttpCode
 }
 
 func (d *dummyStreamingMiddleware) EnabledForSpec() bool {
 	streamingConfig := d.Gw.GetConfig().Streaming
 
-	if streamingConfig.Enabled {
-		d.Logger().Error("Error: Streaming is supported only in Tyk Enterprise Edition")
+	if streamingConfig.Enabled && d.Spec.isStreamingAPI() {
+		d.Logger().Warnf("Warning: %s", MessageStreamingOnlySupportedInEE)
+		return true
 	}
 
 	return false