Merge branch 'master' into feature/issue-495-wss-fix

# Conflicts: # reverse_proxy_test.go
TykTechnologies · Feb 14, 2019 · 12b3d07 · 12b3d07
2 parents 4402bb9 + 31252a2
commit 12b3d07
Show file tree

Hide file tree

Showing 21 changed files with 625 additions and 192 deletions.
diff --git a/TESTING.md b/TESTING.md
@@ -1,16 +1,34 @@
-## Tyk testing guide
+Table of Contents
+=================
+
+* [Tyk testing guide](#tyk-testing-guide)
+    * [Initializing test server](#initializing-test-server)
+    * [Loading and configuring APIs](#loading-and-configuring-apis)
+    * [Running the tests](#running-the-tests)
+    * [Changing config variables](#changing-config-variables)
+    * [Upstream test server](#upstream-test-server)
+    * [Coprocess plugin testing](#coprocess-plugin-testing)
+    * [Creating user sessions](#creating-user-sessions)
+    * [Mocking dashboard](#mocking-dashboard)
+    * [Mocking RPC (Hybrid)](#mocking-rpc-hybrid)
+    * [Mocking DNS](#mocking-dns)
+* [Test Framework](#test-framework)
+
 
-When it comes to the tests, one of the main questions is how to keep balance between expressivity, extendability, repeatability and performance. There are countless discussions if you should write integration or unit tests, should your mock or not, should you write tests first or after and etc. Since you will never find the right answer, on a growing code base, multiple people start introducing own methodology and distinct test helpers. Even looking at our quite small code base, I can find like 3-4 ways to write the same test. Additionally expressivity of our tests are quite bad: it is quite hard to understand what actually get tested, lot of boilerplate code not related to test logic, and amount of copy-paste growing with each test.
+## Tyk testing guide
 
-In order to fix issues described above, I think it is important to have an official guide on writing the tests.
+When it comes to the tests, one of the main questions is how to keep balance between expressivity, extendability, repeatability and performance. There are countless discussions if you should write integration or unit tests, should your mock or not, should you write tests first or after and etc. Since you will never find the right answer, on a growing code base, multiple people start introducing own methodology and distinct test helpers. Even looking at our quite small code base, you can find like 3-4 ways to write the same test.
 
-This idea behind this framework is not new, and we already had pieces of it around the code. My goal was to unify all the patterns we used previously, and design a small layer on top of it, to streamline process of writing the tests. 
+This document describes Tyk test framework and unified guidelines on writing tests.
 
-Main points of the new framework are:
+Main points of the test framework are:
 - All tests run HTTP requests though the full HTTP stack, same as user will do
 - Test definition logic separated from test runner.
 - Official mocks for the Dashboard, RPC, and Bundler
 
+Framework located inside "github.com/TykTechnologies/tyk/test" package.
+See its API docs https://godoc.org/github.com/TykTechnologies/tyk/test
+
 Let’s learn by example:
 
 ```go
@@ -201,30 +219,27 @@ Tests are defined using new `test` package `TestCase` structure, which allows yo
 
 ```go
 type TestCase struct {
-    Method                string
-    Path                     string
-    Domain                string
-    // Request body, can be any object. If not string, It will be automatically serialized
-    Data                     interface{}
-    Headers              map[string]string
-    PathParams        map[string]string
-    Cookies               []*http.Cookie
-    BeforeFn              func()
-    // If need custom http client settings, like SSL client certificate or custom timeouts
-    Client                   *http.Client
-    // If true, pass valid admin-auth header
-    AdminAuth          bool
-    // if Control API run on separate port (or domain), tell to run request on this listener
-    ControlRequest  bool 
-
-    // Assertions
-    Code                    int
-    BodyMatch         string
-    BodyNotMatch   string
-    HeadersMatch    map[string]string
-    HeadersNotMatch map[string]string
-         // If http request returns non http error, like TLS or Timeout
-    ErrorMatch          string
+	Method, Path    string            `json:",omitempty"`
+	Domain          string            `json:",omitempty"`
+	Proto           string            `json:",omitempty"`
+	Code            int               `json:",omitempty"`
+	Data            interface{}       `json:",omitempty"`
+	Headers         map[string]string `json:",omitempty"`
+	PathParams      map[string]string `json:",omitempty"`
+	Cookies         []*http.Cookie    `json:",omitempty"`
+	Delay           time.Duration     `json:",omitempty"`
+	BodyMatch       string            `json:",omitempty"`
+	BodyMatchFunc   func([]byte) bool `json:",omitempty"`
+	BodyNotMatch    string            `json:",omitempty"`
+	HeadersMatch    map[string]string `json:",omitempty"`
+	HeadersNotMatch map[string]string `json:",omitempty"`
+	JSONMatch       map[string]string `json:",omitempty"`
+	ErrorMatch      string            `json:",omitempty"`
+	BeforeFn        func()            `json:"-"`
+	Client          *http.Client      `json:"-"`
+
+	AdminAuth      bool `json:",omitempty"`
+	ControlRequest bool `json:",omitempty"`
 }
 ```
 
@@ -384,4 +399,58 @@ func TestSyncAPISpecsRPCSuccess(t *testing.T) {
 ### DNS mocks
 Inside tests we override default network resolver to use custom DNS server mock, creating using awesome `github.com/miekg/dns` library. Domain -\> IP mapping set via map inside `helpers_test.go` file. By default you have access to domains: `localhost`, `host1.local`, `host2.local` and `host3.local`. Access to all unknown domains will cause panic. 
 
-Using DNS mock means that you are able to create tests with APIs on multiple domains, without modifying machine `/etc/hosts` file. 
+Using DNS mock means that you are able to create tests with APIs on multiple domains, without modifying machine `/etc/hosts` file. 
+
+## Test Framework
+
+Usage of framework described above is not limited by Tyk Gateway, and it is used across variety of Tyk projects. 
+The main building block is the test runner. 
+```go
+type HTTPTestRunner struct {
+	Do             func(*http.Request, *TestCase) (*http.Response, error)
+	Assert         func(*http.Response, *TestCase) error
+	RequestBuilder func(*TestCase) (*http.Request, error)
+}
+func (r HTTPTestRunner) Run(t testing.TB, testCases ...TestCase) {
+...
+}
+```
+By overriding its variables, you can tune runner behavior.
+For example http runner can be look like:
+```
+import "github.com/TykTechnologies/tyk/test"
+
+...
+baseURL := "http://example.com"
+runner := test.HTTPTestRunner{
+    Do: func(r *http.Request, tc *TestCase) (*http.Response, error) {
+      return tc.Client.Do(r)  
+    }
+    RequestBuilder: func(tc *TestCase) (*http.Request, error) {
+        tc.BaseURL = baseURL
+        return NewRequest(tc)
+    },
+}
+runner.Run(t, testCases...)
+...
+```
+And Unit testing of http handlers can be:
+```
+import "github.com/TykTechnologies/tyk/test"
+
+...
+handler := func(wr http.RequestWriter, r *http.Request){...}
+runner := test.HTTPTestRunner{
+    Do: func(r *http.Request, _ *TestCase) (*http.Response, error) {
+		rec := httptest.NewRecorder()
+		handler(rec, r)
+		return rec.Result(), nil
+	},
+}
+runner.Run(t, testCases...)
+...
+```
+
+This package already exports functions for cases mentioned above:
+ - `func TestHttpServer(t testing.TB, baseURL string, testCases ...TestCase)`
+ - `func TestHttpHandler(t testing.TB, handle http.HandlerFunc, testCases ...TestCase)`
diff --git a/api.go b/api.go
@@ -450,7 +450,7 @@ func handleAddKey(keyName, hashedName, sessionString, apiID string) {
 	if err != nil {
 		log.WithFields(logrus.Fields{
 			"prefix": "api",
-			"key":    keyName,
+			"key":    obfuscateKey(keyName),
 			"status": "fail",
 			"err":    err,
 		}).Error("Failed to update key.")
@@ -466,12 +466,24 @@ func handleDeleteKey(keyName, apiID string) (interface{}, int) {
 	if apiID == "-1" {
 		// Go through ALL managed API's and delete the key
 		apisMu.RLock()
+		removed := false
 		for _, spec := range apisByID {
-			spec.SessionManager.RemoveSession(keyName, false)
+			if spec.SessionManager.RemoveSession(keyName, false) {
+				removed = true
+			}
 			spec.SessionManager.ResetQuota(keyName, &user.SessionState{}, false)
 		}
 		apisMu.RUnlock()
 
+		if !removed {
+			log.WithFields(logrus.Fields{
+				"prefix": "api",
+				"key":    obfuscateKey(keyName),
+				"status": "fail",
+			}).Error("Failed to remove the key")
+			return apiError("Failed to remove the key"), http.StatusBadRequest
+		}
+
 		log.WithFields(logrus.Fields{
 			"prefix": "api",
 			"key":    keyName,
@@ -488,7 +500,14 @@ func handleDeleteKey(keyName, apiID string) (interface{}, int) {
 		sessionManager = spec.SessionManager
 	}
 
-	sessionManager.RemoveSession(keyName, false)
+	if !sessionManager.RemoveSession(keyName, false) {
+		log.WithFields(logrus.Fields{
+			"prefix": "api",
+			"key":    obfuscateKey(keyName),
+			"status": "fail",
+		}).Error("Failed to remove the key")
+		return apiError("Failed to remove the key"), http.StatusBadRequest
+	}
 	sessionManager.ResetQuota(keyName, &user.SessionState{}, false)
 
 	statusObj := apiModifyKeySuccess{
@@ -515,12 +534,24 @@ func handleDeleteKey(keyName, apiID string) (interface{}, int) {
 func handleDeleteHashedKey(keyName, apiID string) (interface{}, int) {
 	if apiID == "-1" {
 		// Go through ALL managed API's and delete the key
+		removed := false
 		apisMu.RLock()
 		for _, spec := range apisByID {
-			spec.SessionManager.RemoveSession(keyName, true)
+			if spec.SessionManager.RemoveSession(keyName, true) {
+				removed = true
+			}
 		}
 		apisMu.RUnlock()
 
+		if !removed {
+			log.WithFields(logrus.Fields{
+				"prefix": "api",
+				"key":    obfuscateKey(keyName),
+				"status": "fail",
+			}).Error("Failed to remove the key")
+			return apiError("Failed to remove the key"), http.StatusBadRequest
+		}
+
 		log.WithFields(logrus.Fields{
 			"prefix": "api",
 			"key":    keyName,
@@ -534,7 +565,15 @@ func handleDeleteHashedKey(keyName, apiID string) (interface{}, int) {
 	if spec := getApiSpec(apiID); spec != nil {
 		sessionManager = spec.SessionManager
 	}
-	sessionManager.RemoveSession(keyName, true)
+
+	if !sessionManager.RemoveSession(keyName, true) {
+		log.WithFields(logrus.Fields{
+			"prefix": "api",
+			"key":    obfuscateKey(keyName),
+			"status": "fail",
+		}).Error("Failed to remove the key")
+		return apiError("Failed to remove the key"), http.StatusBadRequest
+	}
 
 	statusObj := apiModifyKeySuccess{
 		Key:    keyName,
@@ -972,7 +1011,10 @@ func handleDeleteOrgKey(orgID string) (interface{}, int) {
 		return apiError("Org not found"), http.StatusNotFound
 	}
 
-	spec.OrgSessionManager.RemoveSession(orgID, false)
+	if !spec.OrgSessionManager.RemoveSession(orgID, false) {
+		return apiError("Failed to remove the key"), http.StatusBadRequest
+	}
+
 	log.WithFields(logrus.Fields{
 		"prefix": "api",
 		"key":    orgID,
@@ -1194,26 +1236,18 @@ func createOauthClient(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
-	if newOauthClient.APIID == "" {
-		doJSONWrite(w, http.StatusInternalServerError,
-			apiError("api_id not specified"))
-		return
-	}
-
-	// check API
-	apiSpec := getApiSpec(newOauthClient.APIID)
-	if apiSpec == nil {
+	if newOauthClient.APIID == "" && newOauthClient.PolicyID == "" {
 		doJSONWrite(w, http.StatusBadRequest,
-			apiError("API doesn't exist"))
+			apiError("api_id not specified"))
 		return
-	}
-	if !apiSpec.UseOauth2 {
+	} else if newOauthClient.APIID != "" && newOauthClient.PolicyID != "" {
 		doJSONWrite(w, http.StatusBadRequest,
-			apiError("API is not OAuth2"))
+			apiError("both api_id and policy_id specified, you can provide only one of those"))
 		return
 	}
 
-	// check policy
+	apiID := ""
+	// get API ID and check policy if needed
 	if newOauthClient.PolicyID != "" {
 		policiesMu.RLock()
 		policy, ok := policiesByID[newOauthClient.PolicyID]
@@ -1223,16 +1257,31 @@ func createOauthClient(w http.ResponseWriter, r *http.Request) {
 				apiError("Policy doesn't exist"))
 			return
 		}
-		if _, ok := policy.AccessRights[newOauthClient.APIID]; !ok {
-			doJSONWrite(w, http.StatusBadRequest,
-				apiError("Policy access rights doesn't contain API this OAuth client belongs to"))
-			return
-		}
 		if len(policy.AccessRights) != 1 {
 			doJSONWrite(w, http.StatusBadRequest,
 				apiError("Policy access rights should contain only one API"))
 			return
 		}
+		// pick API ID from policy's ACL
+		for apiID = range policy.AccessRights {
+			break
+		}
+	} else {
+		// pick API ID from request
+		apiID = newOauthClient.APIID
+	}
+
+	// check API
+	apiSpec := getApiSpec(apiID)
+	if apiSpec == nil {
+		doJSONWrite(w, http.StatusBadRequest,
+			apiError("API doesn't exist"))
+		return
+	}
+	if !apiSpec.UseOauth2 {
+		doJSONWrite(w, http.StatusBadRequest,
+			apiError("API is not OAuth2"))
+		return
 	}
 
 	// Allow the client ID to be set