TT-13513 TT-12767 TT-12768 ensure to save oauth clients locally when pulled from rpc

[sredxny]

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 📝

	Relevant files
Enhancement	manager.go Rename callback function for certificate pull consistency certs/manager.go Renamed CallbackonPullfromRPC to CallbackOnPullCertificateFromRPC for consistency. Updated the initialization of mdcbStorage with the renamed callback. +1/-1      mdcb_storage.go Refactor key retrieval and add caching mechanisms                storage/mdcb_storage.go Added constants for resource types (resourceOauthClient, resourceCertificate, etc.). Refactored GetKey to separate local and RPC retrieval logic. Introduced caching mechanisms for OAuth clients and certificates. Added helper methods like getFromRPCAndCache, cacheCertificate, and cacheOAuthClient. +74/-32  storage.go Add GoMock directive for Handler interface                              storage/storage.go Added GoMock generation directive for the Handler interface. Prepared the file for mock generation to support testing. +2/-0
Tests	mdcb_storage_test.go Add unit tests for caching and retrieval logic                      storage/mdcb_storage_test.go Added test setup utility for mocking dependencies. Implemented unit tests for new caching and retrieval methods. Enhanced test coverage for resource type processing and error handling. +323/-4  storage.go Add GoMock-generated mock for Handler interface                    storage/mock/storage.go Added a generated mock for the Handler interface using GoMock. Enables testing of storage interactions in isolation. +501/-0

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling Ensure that error handling is consistent and comprehensive, especially in the new caching and RPC retrieval methods. Logging Consistency Review the logging levels and messages for consistency and clarity, particularly in the new caching logic. Resource Type Handling Verify the implementation of resource type determination and its impact on the system's behavior and performance.

[github-actions]

PR Code Suggestions ✨

No code suggestions found for the PR.

[github-actions]

API Changes

--- prev.txt	2024-12-12 09:23:20.225112082 +0000
+++ current.txt	2024-12-12 09:23:15.864144866 +0000
@@ -11653,11 +11653,11 @@
     AuthorisationManager to read and write key values to the backend
 
 type MdcbStorage struct {
-	CallbackonPullfromRPC *func(key string, val string) error
+	OnRPCCertPull func(key string, val string) error
 	// Has unexported fields.
 }
 
-func NewMdcbStorage(local, rpc Handler, log *logrus.Entry) *MdcbStorage
+func NewMdcbStorage(local, rpc Handler, log *logrus.Entry, OnRPCCertPull func(key string, val string) error) *MdcbStorage
 
 func (m MdcbStorage) AddToSet(key string, value string)
 
@@ -11938,6 +11938,229 @@
 
 func (v *Vault) Get(key string) (string, error)
 
+# Package: ./storage/mock
+
+package mock // import "github.com/TykTechnologies/tyk/storage/mock"
+
+Package mock is a generated GoMock package.
+
+TYPES
+
+type MockHandler struct {
+	// Has unexported fields.
+}
+    MockHandler is a mock of Handler interface.
+
+func NewMockHandler(ctrl *gomock.Controller) *MockHandler
+    NewMockHandler creates a new mock instance.
+
+func (m *MockHandler) AddToSet(arg0, arg1 string)
+    AddToSet mocks base method.
+
+func (m *MockHandler) AddToSortedSet(arg0, arg1 string, arg2 float64)
+    AddToSortedSet mocks base method.
+
+func (m *MockHandler) AppendToSet(arg0, arg1 string)
+    AppendToSet mocks base method.
+
+func (m *MockHandler) Connect() bool
+    Connect mocks base method.
+
+func (m *MockHandler) Decrement(arg0 string)
+    Decrement mocks base method.
+
+func (m *MockHandler) DeleteAllKeys() bool
+    DeleteAllKeys mocks base method.
+
+func (m *MockHandler) DeleteKey(arg0 string) bool
+    DeleteKey mocks base method.
+
+func (m *MockHandler) DeleteKeys(arg0 []string) bool
+    DeleteKeys mocks base method.
+
+func (m *MockHandler) DeleteRawKey(arg0 string) bool
+    DeleteRawKey mocks base method.
+
+func (m *MockHandler) DeleteRawKeys(arg0 []string) bool
+    DeleteRawKeys mocks base method.
+
+func (m *MockHandler) DeleteScanMatch(arg0 string) bool
+    DeleteScanMatch mocks base method.
+
+func (m *MockHandler) EXPECT() *MockHandlerMockRecorder
+    EXPECT returns an object that allows the caller to indicate expected use.
+
+func (m *MockHandler) Exists(arg0 string) (bool, error)
+    Exists mocks base method.
+
+func (m *MockHandler) GetAndDeleteSet(arg0 string) []any
+    GetAndDeleteSet mocks base method.
+
+func (m *MockHandler) GetExp(arg0 string) (int64, error)
+    GetExp mocks base method.
+
+func (m *MockHandler) GetKey(arg0 string) (string, error)
+    GetKey mocks base method.
+
+func (m *MockHandler) GetKeyPrefix() string
+    GetKeyPrefix mocks base method.
+
+func (m *MockHandler) GetKeys(arg0 string) []string
+    GetKeys mocks base method.
+
+func (m *MockHandler) GetKeysAndValues() map[string]string
+    GetKeysAndValues mocks base method.
+
+func (m *MockHandler) GetKeysAndValuesWithFilter(arg0 string) map[string]string
+    GetKeysAndValuesWithFilter mocks base method.
+
+func (m *MockHandler) GetListRange(arg0 string, arg1, arg2 int64) ([]string, error)
+    GetListRange mocks base method.
+
+func (m *MockHandler) GetMultiKey(arg0 []string) ([]string, error)
+    GetMultiKey mocks base method.
+
+func (m *MockHandler) GetRawKey(arg0 string) (string, error)
+    GetRawKey mocks base method.
+
+func (m *MockHandler) GetRollingWindow(arg0 string, arg1 int64, arg2 bool) (int, []any)
+    GetRollingWindow mocks base method.
+
+func (m *MockHandler) GetSet(arg0 string) (map[string]string, error)
+    GetSet mocks base method.
+
+func (m *MockHandler) GetSortedSetRange(arg0, arg1, arg2 string) ([]string, []float64, error)
+    GetSortedSetRange mocks base method.
+
+func (m *MockHandler) IncrememntWithExpire(arg0 string, arg1 int64) int64
+    IncrememntWithExpire mocks base method.
+
+func (m *MockHandler) RemoveFromList(arg0, arg1 string) error
+    RemoveFromList mocks base method.
+
+func (m *MockHandler) RemoveFromSet(arg0, arg1 string)
+    RemoveFromSet mocks base method.
+
+func (m *MockHandler) RemoveSortedSetRange(arg0, arg1, arg2 string) error
+    RemoveSortedSetRange mocks base method.
+
+func (m *MockHandler) SetExp(arg0 string, arg1 int64) error
+    SetExp mocks base method.
+
+func (m *MockHandler) SetKey(arg0, arg1 string, arg2 int64) error
+    SetKey mocks base method.
+
+func (m *MockHandler) SetRawKey(arg0, arg1 string, arg2 int64) error
+    SetRawKey mocks base method.
+
+func (m *MockHandler) SetRollingWindow(arg0 string, arg1 int64, arg2 string, arg3 bool) (int, []any)
+    SetRollingWindow mocks base method.
+
+type MockHandlerMockRecorder struct {
+	// Has unexported fields.
+}
+    MockHandlerMockRecorder is the mock recorder for MockHandler.
+
+func (mr *MockHandlerMockRecorder) AddToSet(arg0, arg1 any) *gomock.Call
+    AddToSet indicates an expected call of AddToSet.
+
+func (mr *MockHandlerMockRecorder) AddToSortedSet(arg0, arg1, arg2 any) *gomock.Call
+    AddToSortedSet indicates an expected call of AddToSortedSet.
+
+func (mr *MockHandlerMockRecorder) AppendToSet(arg0, arg1 any) *gomock.Call
+    AppendToSet indicates an expected call of AppendToSet.
+
+func (mr *MockHandlerMockRecorder) Connect() *gomock.Call
+    Connect indicates an expected call of Connect.
+
+func (mr *MockHandlerMockRecorder) Decrement(arg0 any) *gomock.Call
+    Decrement indicates an expected call of Decrement.
+
+func (mr *MockHandlerMockRecorder) DeleteAllKeys() *gomock.Call
+    DeleteAllKeys indicates an expected call of DeleteAllKeys.
+
+func (mr *MockHandlerMockRecorder) DeleteKey(arg0 any) *gomock.Call
+    DeleteKey indicates an expected call of DeleteKey.
+
+func (mr *MockHandlerMockRecorder) DeleteKeys(arg0 any) *gomock.Call
+    DeleteKeys indicates an expected call of DeleteKeys.
+
+func (mr *MockHandlerMockRecorder) DeleteRawKey(arg0 any) *gomock.Call
+    DeleteRawKey indicates an expected call of DeleteRawKey.
+
+func (mr *MockHandlerMockRecorder) DeleteRawKeys(arg0 any) *gomock.Call
+    DeleteRawKeys indicates an expected call of DeleteRawKeys.
+
+func (mr *MockHandlerMockRecorder) DeleteScanMatch(arg0 any) *gomock.Call
+    DeleteScanMatch indicates an expected call of DeleteScanMatch.
+
+func (mr *MockHandlerMockRecorder) Exists(arg0 any) *gomock.Call
+    Exists indicates an expected call of Exists.
+
+func (mr *MockHandlerMockRecorder) GetAndDeleteSet(arg0 any) *gomock.Call
+    GetAndDeleteSet indicates an expected call of GetAndDeleteSet.
+
+func (mr *MockHandlerMockRecorder) GetExp(arg0 any) *gomock.Call
+    GetExp indicates an expected call of GetExp.
+
+func (mr *MockHandlerMockRecorder) GetKey(arg0 any) *gomock.Call
+    GetKey indicates an expected call of GetKey.
+
+func (mr *MockHandlerMockRecorder) GetKeyPrefix() *gomock.Call
+    GetKeyPrefix indicates an expected call of GetKeyPrefix.
+
+func (mr *MockHandlerMockRecorder) GetKeys(arg0 any) *gomock.Call
+    GetKeys indicates an expected call of GetKeys.
+
+func (mr *MockHandlerMockRecorder) GetKeysAndValues() *gomock.Call
+    GetKeysAndValues indicates an expected call of GetKeysAndValues.
+
+func (mr *MockHandlerMockRecorder) GetKeysAndValuesWithFilter(arg0 any) *gomock.Call
+    GetKeysAndValuesWithFilter indicates an expected call of
+    GetKeysAndValuesWithFilter.
+
+func (mr *MockHandlerMockRecorder) GetListRange(arg0, arg1, arg2 any) *gomock.Call
+    GetListRange indicates an expected call of GetListRange.
+
+func (mr *MockHandlerMockRecorder) GetMultiKey(arg0 any) *gomock.Call
+    GetMultiKey indicates an expected call of GetMultiKey.
+
+func (mr *MockHandlerMockRecorder) GetRawKey(arg0 any) *gomock.Call
+    GetRawKey indicates an expected call of GetRawKey.
+
+func (mr *MockHandlerMockRecorder) GetRollingWindow(arg0, arg1, arg2 any) *gomock.Call
+    GetRollingWindow indicates an expected call of GetRollingWindow.
+
+func (mr *MockHandlerMockRecorder) GetSet(arg0 any) *gomock.Call
+    GetSet indicates an expected call of GetSet.
+
+func (mr *MockHandlerMockRecorder) GetSortedSetRange(arg0, arg1, arg2 any) *gomock.Call
+    GetSortedSetRange indicates an expected call of GetSortedSetRange.
+
+func (mr *MockHandlerMockRecorder) IncrememntWithExpire(arg0, arg1 any) *gomock.Call
+    IncrememntWithExpire indicates an expected call of IncrememntWithExpire.
+
+func (mr *MockHandlerMockRecorder) RemoveFromList(arg0, arg1 any) *gomock.Call
+    RemoveFromList indicates an expected call of RemoveFromList.
+
+func (mr *MockHandlerMockRecorder) RemoveFromSet(arg0, arg1 any) *gomock.Call
+    RemoveFromSet indicates an expected call of RemoveFromSet.
+
+func (mr *MockHandlerMockRecorder) RemoveSortedSetRange(arg0, arg1, arg2 any) *gomock.Call
+    RemoveSortedSetRange indicates an expected call of RemoveSortedSetRange.
+
+func (mr *MockHandlerMockRecorder) SetExp(arg0, arg1 any) *gomock.Call
+    SetExp indicates an expected call of SetExp.
+
+func (mr *MockHandlerMockRecorder) SetKey(arg0, arg1, arg2 any) *gomock.Call
+    SetKey indicates an expected call of SetKey.
+
+func (mr *MockHandlerMockRecorder) SetRawKey(arg0, arg1, arg2 any) *gomock.Call
+    SetRawKey indicates an expected call of SetRawKey.
+
+func (mr *MockHandlerMockRecorder) SetRollingWindow(arg0, arg1, arg2, arg3 any) *gomock.Call
+    SetRollingWindow indicates an expected call of SetRollingWindow.
+
 # Package: ./tcp
 
 package tcp // import "github.com/TykTechnologies/tyk/tcp"

[sonarqubecloud]

Quality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

[kolavcic]

/release to release-5.3

[tykbot]

Working on it! Note that it can take a few minutes.

[tykbot]

@kolavcic Succesfully merged PR

[kolavcic]

/release to release-5.7

[tykbot]

Working on it! Note that it can take a few minutes.

[tykbot]

@kolavcic Seems like there is conflict and it require manual merge.

[sredxny]

/release to release-5.3.9

[tykbot]

Working on it! Note that it can take a few minutes.

[tykbot]

@sredxny Succesfully merged PR

[sredxny]

/release to release-5.7.1

[tykbot]

Working on it! Note that it can take a few minutes.

[tykbot]

@sredxny Seems like there is conflict and it require manual merge.