TT-13513 TT-12767 TT-12768 ensure to save oauth clients locally when …

…pulled from rpc (#6740) ### **User description**  ## Description The Oauth client was not being cached in the local redis when the gateway was running as an edge in an MDCB setup. This PR then: - Ensures that the first time that the oauthclient is pulled from RPC then we cache it in redis - Refactor code of the MDCB storage into multiple smaller functions so is eaasy to read the code and test - created mock for the storage handler interface...later we should remove all mentions to DummyStorage and use the mock instead - Created tests for the mdcb storage - Certificates caching doesnt works in the same way, as they depend on the certificate manager and secret set to encode the content ## Related Issue     ## Motivation and Context  ## How This Has Been Tested - Run MDCB setup with synchroniser disabled - Created api and policy via dashboard. - Protect the api using oauth 2.0 - Created an oauth client via dashboard api - Create a token in the edge node using the created oauth client - use the token to consume the api in that edge node - shut down mdcb - attempt to generate another token using the edge node - At this point you should be allowed to create that new token and use it against the api ## Screenshots (if appropriate) ## Types of changes  - [ ] 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    - [ ] 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, Enhancement ___ ### **Description** - Refactored the `GetKey` method to separate local and RPC retrieval logic, improving maintainability. - Introduced caching mechanisms for OAuth clients and certificates, ensuring resources pulled from RPC are stored locally. - Added constants for resource types to improve code readability and maintainability. - Renamed callback function for certificate pull consistency. - Added extensive unit tests for new caching and retrieval logic, improving test coverage. - Generated a mock for the `Handler` interface using GoMock to facilitate isolated testing of storage interactions. ___ ### **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>manager.go</strong><dd><code>Rename callback function for certificate pull consistency</code></dd></summary> <hr> certs/manager.go <li>Renamed <code>CallbackonPullfromRPC</code> to <code>CallbackOnPullCertificateFromRPC</code> for <br>consistency.<br> <li> Updated the initialization of <code>mdcbStorage</code> with the renamed callback.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6740/files#diff-78e768b2719ac9f70038499f847de2843db20d8ca21a963ea63b82010d711039">+1/-1</a>      </td> </tr> <tr> <td> <details> <summary><strong>mdcb_storage.go</strong><dd><code>Refactor key retrieval and add caching mechanisms</code>                </dd></summary> <hr> storage/mdcb_storage.go <li>Added constants for resource types (<code>resourceOauthClient</code>, <br><code>resourceCertificate</code>, etc.).<br> <li> Refactored <code>GetKey</code> to separate local and RPC retrieval logic.<br> <li> Introduced caching mechanisms for OAuth clients and certificates.<br> <li> Added helper methods like <code>getFromRPCAndCache</code>, <code>cacheCertificate</code>, and <br><code>cacheOAuthClient</code>.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6740/files#diff-c5739d542a422343ec22585ffa5e4ad7e2e91358db018a157dc23cb5096c04d2">+74/-32</a>  </td> </tr> <tr> <td> <details> <summary><strong>storage.go</strong><dd><code>Add GoMock directive for Handler interface</code>                              </dd></summary> <hr> storage/storage.go <li>Added GoMock generation directive for the <code>Handler</code> interface.<br> <li> Prepared the file for mock generation to support testing.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6740/files#diff-2a93e444b612bd9853c32889fb82c4041760536f84356bb0db04738c19b62dde">+2/-0</a>      </td> </tr> </table></td></tr><tr><td><strong>Tests</strong></td><td><table> <tr> <td> <details> <summary><strong>mdcb_storage_test.go</strong><dd><code>Add unit tests for caching and retrieval logic</code>                      </dd></summary> <hr> storage/mdcb_storage_test.go <li>Added test setup utility for mocking dependencies.<br> <li> Implemented unit tests for new caching and retrieval methods.<br> <li> Enhanced test coverage for resource type processing and error <br>handling.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6740/files#diff-6a40b704ea7dc3b61069eebd5d56464a66bb1c61095909aa9cc5e423c5c88422">+323/-4</a>  </td> </tr> <tr> <td> <details> <summary><strong>storage.go</strong><dd><code>Add GoMock-generated mock for Handler interface</code>                    </dd></summary> <hr> storage/mock/storage.go <li>Added a generated mock for the <code>Handler</code> interface using GoMock.<br> <li> Enables testing of storage interactions in isolation.<br> </details> </td> <td><a href="https://github.com/TykTechnologies/tyk/pull/6740/files#diff-0e75f439d0385d9272ea3afa9fc465dcae08554f19ff821e0743ad096325df40">+501/-0</a>  </td> </tr> </table></td></tr></tr></tbody></table> ___ > 💡 **PR-Agent usage**: Comment `/help "your question"` on any pull request to receive relevant information --------- Co-authored-by: sredny buitrago <[email protected]> Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> Co-authored-by: Matias <[email protected]> Co-authored-by: Mladen Kolavcic <[email protected]> (cherry picked from commit abc3fa6)
TykTechnologies · Dec 19, 2024 · d915404 · d915404
1 parent 19fbac2
commit d915404
Show file tree

Hide file tree

Showing 6 changed files with 898 additions and 44 deletions.
diff --git a/certs/manager.go b/certs/manager.go
@@ -101,9 +101,7 @@ func NewSlaveCertManager(localStorage, rpcStorage storage.Handler, secret string
 		return err
 	}
 
-	mdcbStorage := storage.NewMdcbStorage(localStorage, rpcStorage, log)
-	mdcbStorage.CallbackonPullfromRPC = &callbackOnPullCertFromRPC
-
+	mdcbStorage := storage.NewMdcbStorage(localStorage, rpcStorage, log, callbackOnPullCertFromRPC)
 	cm.storage = mdcbStorage
 	return cm
 }

diff --git a/gateway/server.go b/gateway/server.go
@@ -1577,6 +1577,7 @@ func (gw *Gateway) getGlobalMDCBStorageHandler(keyPrefix string, hashKeys bool)
 				Gw:        gw,
 			},
 			logger,
+			nil,
 		)
 	}
 	return localStorage

diff --git a/storage/mdcb_storage.go b/storage/mdcb_storage.go
@@ -8,60 +8,50 @@ import (
 )
 
 type MdcbStorage struct {
-	local                 Handler
-	rpc                   Handler
-	logger                *logrus.Entry
-	CallbackonPullfromRPC *func(key string, val string) error
+	local         Handler
+	rpc           Handler
+	logger        *logrus.Entry
+	OnRPCCertPull func(key string, val string) error
 }
 
-func NewMdcbStorage(local, rpc Handler, log *logrus.Entry) *MdcbStorage {
+const (
+	resourceOauthClient = "OauthClient"
+	resourceCertificate = "Certificate"
+	resourceApiKey      = "ApiKey"
+	resourceKey         = "Key"
+)
+
+func NewMdcbStorage(local, rpc Handler, log *logrus.Entry, OnRPCCertPull func(key string, val string) error) *MdcbStorage {
 	return &MdcbStorage{
-		local:  local,
-		rpc:    rpc,
-		logger: log,
+		local:         local,
+		rpc:           rpc,
+		logger:        log,
+		OnRPCCertPull: OnRPCCertPull,
 	}
 }
 
 func (m MdcbStorage) GetKey(key string) (string, error) {
-	var val string
-	var err error
-
-	if m.local == nil {
-		return m.rpc.GetKey(key)
-	}
-
-	val, err = m.local.GetKey(key)
-	if err != nil {
-		m.logger.Infof("Retrieving key from rpc.")
-		val, err = m.rpc.GetKey(key)
-
-		if err != nil {
-			resourceType := getResourceType(key)
-			m.logger.Errorf("cannot retrieve %v from rpc: %v", resourceType, err.Error())
-			return val, err
-		}
-
-		if m.CallbackonPullfromRPC != nil {
-			err := (*m.CallbackonPullfromRPC)(key, val)
-			if err != nil {
-				m.logger.Error(err)
-			}
+	if m.local != nil {
+		val, err := m.getFromLocal(key)
+		if err == nil {
+			return val, nil
 		}
+		m.logger.Debugf("Key not present locally, pulling from rpc layer: %v", err)
 	}
 
-	return val, err
+	return m.getFromRPCAndCache(key)
 }
 
 func getResourceType(key string) string {
 	switch {
 	case strings.Contains(key, "oauth-clientid."):
-		return "Oauth Client"
+		return resourceOauthClient
 	case strings.HasPrefix(key, "cert"):
-		return "certificate"
+		return resourceCertificate
 	case strings.HasPrefix(key, "apikey"):
-		return "api key"
+		return resourceApiKey
 	default:
-		return "key"
+		return resourceKey
 	}
 }
 
@@ -256,3 +246,46 @@ func (m MdcbStorage) Exists(key string) (bool, error) {
 
 	return foundLocal && foundRpc, nil
 }
+
+// cacheCertificate saves locally resourceCertificate after pull from rpc
+func (m MdcbStorage) cacheCertificate(key, val string) error {
+	if m.OnRPCCertPull == nil {
+		return nil
+	}
+	return m.OnRPCCertPull(key, val)
+}
+
+// cacheOAuthClient saved oauth data in local storage after pull from rpc
+func (m MdcbStorage) cacheOAuthClient(key, val string) error {
+	return m.local.SetKey(key, val, 0)
+}
+
+// processResourceByType based on the type of key it will trigger the proper
+// caching mechanism
+func (m MdcbStorage) processResourceByType(key, val string) error {
+
+	resourceType := getResourceType(key)
+	switch resourceType {
+	case resourceOauthClient:
+		return m.cacheOAuthClient(key, val)
+	case resourceCertificate:
+		return m.cacheCertificate(key, val)
+	}
+	return nil
+}
+
+// getFromRPCAndCache pulls a resource from rpc and stores it in local redis for caching
+func (m MdcbStorage) getFromRPCAndCache(key string) (string, error) {
+	val, err := m.rpc.GetKey(key)
+	if err != nil {
+		return "", err
+	}
+
+	err = m.processResourceByType(key, val)
+	return val, err
+}
+
+// getFromLocal get a key from local storage
+func (m MdcbStorage) getFromLocal(key string) (string, error) {
+	return m.local.GetKey(key)
+}
-Original file line number
+Diff line change
@@ Expand Up @@
     				Gw:        gw,
     			},
     			logger,
+    			nil,
     		)
     	}
     	return localStorage
@@ Expand Down @@