From 03bf76fc9b0339d83ee54eec1e423fc01751889c Mon Sep 17 00:00:00 2001 From: sredny buitrago Date: Tue, 3 Dec 2024 23:06:36 -0300 Subject: [PATCH 01/11] ensure to save oauth clients locally when pulled from rpc --- certs/manager.go | 2 +- certs/manager_test.go | 2 +- gateway/api.go | 30 +- gateway/api_healthcheck.go | 2 +- gateway/api_test.go | 6 +- gateway/cert_test.go | 2 +- gateway/event_handler_webhooks.go | 2 +- gateway/gateway_test.go | 2 +- gateway/host_checker_manager.go | 2 +- gateway/middleware.go | 2 +- gateway/mw_auth_key.go | 2 +- gateway/mw_auth_key_test.go | 2 +- gateway/mw_jwt.go | 14 +- gateway/oauth_manager_test.go | 6 +- gateway/rpc_storage_handler.go | 6 +- gateway/session_manager.go | 2 +- storage/dummy_test.go | 18 +- storage/mdcb_storage.go | 106 +++++-- storage/mdcb_storage_test.go | 327 ++++++++++++++++++- storage/mock/storage.go | 501 ++++++++++++++++++++++++++++++ storage/storage.go | 2 + 21 files changed, 951 insertions(+), 87 deletions(-) create mode 100644 storage/mock/storage.go diff --git a/certs/manager.go b/certs/manager.go index e8585c36a1c..e45e0a01454 100644 --- a/certs/manager.go +++ b/certs/manager.go @@ -102,7 +102,7 @@ func NewSlaveCertManager(localStorage, rpcStorage storage.Handler, secret string } mdcbStorage := storage.NewMdcbStorage(localStorage, rpcStorage, log) - mdcbStorage.CallbackonPullfromRPC = &callbackOnPullCertFromRPC + mdcbStorage.CallbackOnPullCertificateFromRPC = &callbackOnPullCertFromRPC cm.storage = mdcbStorage return cm diff --git a/certs/manager_test.go b/certs/manager_test.go index fa6449ceff8..9f36083f0db 100644 --- a/certs/manager_test.go +++ b/certs/manager_test.go @@ -184,7 +184,7 @@ func TestCertificateStorage(t *testing.T) { t.Error("Should return only private certificate") } - if leafSubjectName(certs[0]) != ("Public Key: " + publicKeyID) { + if leafSubjectName(certs[0]) != ("Public resourceKey: " + publicKeyID) { t.Error("Wrong cert", leafSubjectName(certs[0])) } }) diff --git a/gateway/api.go b/gateway/api.go index 751d085bfda..05f80bfb388 100644 --- a/gateway/api.go +++ b/gateway/api.go @@ -390,7 +390,7 @@ func (gw *Gateway) doAddOrUpdate(keyName string, newSession *user.SessionState, } } - logger.Info("Key added or updated.") + logger.Info("resourceKey added or updated.") return nil } @@ -461,22 +461,22 @@ func (gw *Gateway) handleAddOrUpdate(keyName string, r *http.Request, isHashed b keyName = key.KeyID if !found { log.Error("Could not find key when updating") - return apiError("Key is not found"), http.StatusNotFound + return apiError("resourceKey is not found"), http.StatusNotFound } originalKey = key.Clone() isCertificateChanged := newSession.Certificate != originalKey.Certificate if isCertificateChanged { if newSession.Certificate == "" { - log.Error("Key must contain a certificate") - return apiError("Key cannot be used without a certificate"), http.StatusBadRequest + log.Error("resourceKey must contain a certificate") + return apiError("resourceKey cannot be used without a certificate"), http.StatusBadRequest } // check that the certificate exists in the system _, err := gw.CertificateManager.GetRaw(newSession.Certificate) if err != nil { - log.Error("Key must contain an existing certificate") - return apiError("Key must be used with an existent certificate"), http.StatusBadRequest + log.Error("resourceKey must contain an existing certificate") + return apiError("resourceKey must be used with an existent certificate"), http.StatusBadRequest } } @@ -566,7 +566,7 @@ func (gw *Gateway) handleAddOrUpdate(keyName string, r *http.Request, isHashed b event = EventTokenCreated } gw.FireSystemEvent(event, EventTokenMeta{ - EventMetaDefault: EventMetaDefault{Message: "Key modified."}, + EventMetaDefault: EventMetaDefault{Message: "resourceKey modified."}, Org: newSession.OrgID, Key: keyName, }) @@ -595,7 +595,7 @@ func (gw *Gateway) handleAddOrUpdate(keyName string, r *http.Request, isHashed b func (gw *Gateway) handleGetDetail(sessionKey, apiID, orgID string, byHash bool) (interface{}, int) { if byHash && !gw.GetConfig().HashKeys { - return apiError("Key requested by hash but key hashing is not enabled"), http.StatusBadRequest + return apiError("resourceKey requested by hash but key hashing is not enabled"), http.StatusBadRequest } spec := gw.getApiSpec(apiID) @@ -606,7 +606,7 @@ func (gw *Gateway) handleGetDetail(sessionKey, apiID, orgID string, byHash bool) session, ok := gw.GlobalSessionManager.SessionDetail(orgID, sessionKey, byHash) sessionKey = session.KeyID if !ok { - return apiError("Key not found"), http.StatusNotFound + return apiError("resourceKey not found"), http.StatusNotFound } mw := &BaseMiddleware{Spec: spec, Gw: gw} @@ -804,7 +804,7 @@ func (gw *Gateway) handleDeleteKey(keyName, orgID, apiID string, resetQuota bool } gw.FireSystemEvent(EventTokenDeleted, EventTokenMeta{ - EventMetaDefault: EventMetaDefault{Message: "Key deleted."}, + EventMetaDefault: EventMetaDefault{Message: "resourceKey deleted."}, Org: orgID, Key: keyName, }) @@ -1707,7 +1707,7 @@ func (gw *Gateway) handleUpdateHashedKey(keyName string, applyPolicies []string) "status": "fail", }).Error("Failed to update hashed key.") - return apiError("Key not found"), http.StatusNotFound + return apiError("resourceKey not found"), http.StatusNotFound } // Set the policy @@ -1955,7 +1955,7 @@ func (gw *Gateway) createKeyHandler(w http.ResponseWriter, r *http.Request) { "prefix": "api", "status": "fail", "err": err, - }).Error("Key creation failed.") + }).Error("resourceKey creation failed.") doJSONWrite(w, http.StatusInternalServerError, apiError("Unmarshalling failed")) return } @@ -1969,7 +1969,7 @@ func (gw *Gateway) createKeyHandler(w http.ResponseWriter, r *http.Request) { newKey = gw.generateToken(newSession.OrgID, newSession.Certificate) _, ok := gw.GlobalSessionManager.SessionDetail(newSession.OrgID, newKey, false) if ok { - doJSONWrite(w, http.StatusInternalServerError, apiError("Failed to create key - Key with given certificate already found:"+newKey)) + doJSONWrite(w, http.StatusInternalServerError, apiError("Failed to create key - resourceKey with given certificate already found:"+newKey)) return } } @@ -2065,7 +2065,7 @@ func (gw *Gateway) createKeyHandler(w http.ResponseWriter, r *http.Request) { } gw.FireSystemEvent(EventTokenCreated, EventTokenMeta{ - EventMetaDefault: EventMetaDefault{Message: "Key generated."}, + EventMetaDefault: EventMetaDefault{Message: "resourceKey generated."}, Org: newSession.OrgID, Key: newKey, }) @@ -2093,7 +2093,7 @@ func (gw *Gateway) previewKeyHandler(w http.ResponseWriter, r *http.Request) { "prefix": "api", "status": "fail", "err": err, - }).Error("Key creation failed.") + }).Error("resourceKey creation failed.") doJSONWrite(w, http.StatusInternalServerError, apiError("Unmarshalling failed")) return } diff --git a/gateway/api_healthcheck.go b/gateway/api_healthcheck.go index e8a8d5ce851..49b62661907 100644 --- a/gateway/api_healthcheck.go +++ b/gateway/api_healthcheck.go @@ -49,7 +49,7 @@ func (h *DefaultHealthChecker) Init(storeType storage.Handler) { } func (h *DefaultHealthChecker) CreateKeyName(subKey HealthPrefix) string { - // Key should be API-ID.SubKey.123456789 + // resourceKey should be API-ID.SubKey.123456789 return h.APIID + "." + string(subKey) } diff --git a/gateway/api_test.go b/gateway/api_test.go index 00bef2aa88a..fbcef1a86db 100644 --- a/gateway/api_test.go +++ b/gateway/api_test.go @@ -656,11 +656,11 @@ func TestKeyHandler_DeleteKeyWithQuota(t *testing.T) { hashKeys bool }{ { - name: "Key Hashing disabled", + name: "resourceKey Hashing disabled", hashKeys: false, }, { - name: "Key hashing enabled", + name: "resourceKey hashing enabled", hashKeys: true, }, } @@ -1095,7 +1095,7 @@ func (ts *Test) testHashKeyHandlerHelper(t *testing.T, expectedHashSize int) { myKeyHash := storage.HashKey(ts.Gw.generateToken("default", myKey), ts.Gw.GetConfig().HashKeys) if len(myKeyHash) != expectedHashSize { - t.Errorf("Expected hash size: %d, got %d. Hash: %s. Key: %s", expectedHashSize, len(myKeyHash), myKeyHash, myKey) + t.Errorf("Expected hash size: %d, got %d. Hash: %s. resourceKey: %s", expectedHashSize, len(myKeyHash), myKeyHash, myKey) } t.Run("Create, get and delete key with key hashing", func(t *testing.T) { diff --git a/gateway/cert_test.go b/gateway/cert_test.go index e133cdb6911..294ed29d4cf 100644 --- a/gateway/cert_test.go +++ b/gateway/cert_test.go @@ -1277,7 +1277,7 @@ func TestKeyWithCertificateTLS(t *testing.T) { }) // check that key has been updated with wrong certificate - t.Run("Key has been updated with wrong certificate key", func(t *testing.T) { + t.Run("resourceKey has been updated with wrong certificate key", func(t *testing.T) { clientPEM, _, _, clientCert := crypto.GenCertificate(&x509.Certificate{}, false) clientCertID, err := ts.Gw.CertificateManager.Add(clientPEM, orgId) diff --git a/gateway/event_handler_webhooks.go b/gateway/event_handler_webhooks.go index e985a7b3ab7..50ce897cca8 100644 --- a/gateway/event_handler_webhooks.go +++ b/gateway/event_handler_webhooks.go @@ -126,7 +126,7 @@ func (w *WebHookHandler) Init(handlerConf interface{}) error { // hookFired checks if an event has been fired within the EventTimeout setting func (w *WebHookHandler) WasHookFired(checksum string) bool { if _, err := w.store.GetKey(checksum); err != nil { - // Key not found, so hook is in limit + // resourceKey not found, so hook is in limit log.WithFields(logrus.Fields{ "prefix": "webhooks", }).Debug("Event can fire, no duplicates found") diff --git a/gateway/gateway_test.go b/gateway/gateway_test.go index 2238f65e13c..52addea370d 100644 --- a/gateway/gateway_test.go +++ b/gateway/gateway_test.go @@ -342,7 +342,7 @@ func TestQuota(t *testing.T) { body, _ := ioutil.ReadAll(r.Body) json.Unmarshal(body, &data) - if data["event"] != "QuotaExceeded" || data["message"] != "Key Quota Limit Exceeded" || data["key"] != keyID { + if data["event"] != "QuotaExceeded" || data["message"] != "resourceKey Quota Limit Exceeded" || data["key"] != keyID { t.Error("Webhook payload not match", data) } })) diff --git a/gateway/host_checker_manager.go b/gateway/host_checker_manager.go index 76ac2c9b201..161ec7460a7 100644 --- a/gateway/host_checker_manager.go +++ b/gateway/host_checker_manager.go @@ -311,7 +311,7 @@ func (hc *HostCheckerManager) HostDown(urlStr string) bool { log.WithFields(logrus.Fields{ "prefix": "host-check-mgr", - }).Debug("Key is: ", PoolerHostSentinelKeyPrefix+u.Host) + }).Debug("resourceKey is: ", PoolerHostSentinelKeyPrefix+u.Host) key := PoolerHostSentinelKeyPrefix + u.Host // If the node doesn't perform any uptime checks, query the storage: diff --git a/gateway/middleware.go b/gateway/middleware.go index c3d85649d76..f1a41c8eb72 100644 --- a/gateway/middleware.go +++ b/gateway/middleware.go @@ -420,7 +420,7 @@ func (t *BaseMiddleware) CheckSessionAndIdentityForValidKey(originalKey string, if !t.Spec.GlobalConfig.LocalSessionCache.DisableCacheSessionState { cachedVal, found := t.Gw.SessionCache.Get(cacheKey) if found { - t.Logger().Debug("--> Key found in local cache") + t.Logger().Debug("--> resourceKey found in local cache") session := cachedVal.(user.SessionState).Clone() if err := t.ApplyPolicies(&session); err != nil { t.Logger().Error(err) diff --git a/gateway/mw_auth_key.go b/gateway/mw_auth_key.go index 25dd659468c..dc633ad794c 100644 --- a/gateway/mw_auth_key.go +++ b/gateway/mw_auth_key.go @@ -78,7 +78,7 @@ func (k *AuthKey) setContextVars(r *http.Request, token string) { return } if cnt := ctxGetData(r); cnt != nil { - // Key data + // resourceKey data cnt["token"] = token ctxSetData(r, cnt) } diff --git a/gateway/mw_auth_key_test.go b/gateway/mw_auth_key_test.go index f1b4b5e0776..a49b46b7de7 100644 --- a/gateway/mw_auth_key_test.go +++ b/gateway/mw_auth_key_test.go @@ -315,7 +315,7 @@ func createAuthKeyAuthSession(isBench bool) *user.SessionState { session.QuotaRemaining = 10 session.QuotaMax = 10 } - session.AccessRights = map[string]user.AccessDefinition{"31": {APIName: "Tyk Auth Key Test", APIID: "31", Versions: []string{"default"}}} + session.AccessRights = map[string]user.AccessDefinition{"31": {APIName: "Tyk Auth resourceKey Test", APIID: "31", Versions: []string{"default"}}} return session } diff --git a/gateway/mw_jwt.go b/gateway/mw_jwt.go index 00343df702e..88ab409830f 100644 --- a/gateway/mw_jwt.go +++ b/gateway/mw_jwt.go @@ -382,7 +382,7 @@ func (k *JWTMiddleware) processCentralisedJWT(r *http.Request, token *jwt.Token) foundPolicy := false if !exists { // Create it - k.Logger().Debug("Key does not exist, creating") + k.Logger().Debug("resourceKey does not exist, creating") // We need a base policy as a template, either get it from the token itself OR a proxy client ID within Tyk basePolicyID, foundPolicy = k.getBasePolicyID(r, claims) @@ -612,7 +612,7 @@ func (k *JWTMiddleware) processCentralisedJWT(r *http.Request, token *jwt.Token) // ensure to set the sessionID session.KeyID = sessionID - k.Logger().Debug("Key found") + k.Logger().Debug("resourceKey found") switch k.Spec.BaseIdentityProvidedBy { case apidef.JWTClaim, apidef.UnsetAuth: ctxSetSession(r, &session, updateSession, k.Gw.GetConfig().HashKeys) @@ -647,7 +647,7 @@ func (k *JWTMiddleware) processOneToOneTokenMap(r *http.Request, token *jwt.Toke if !exists { k.reportLoginFailure(tykId, r) - return errors.New("Key not authorized"), http.StatusForbidden + return errors.New("resourceKey not authorized"), http.StatusForbidden } k.Logger().Debug("Raw key ID found.") @@ -707,7 +707,7 @@ func (k *JWTMiddleware) ProcessRequest(w http.ResponseWriter, r *http.Request, _ if err == nil && token.Valid { if jwtErr := k.timeValidateJWTClaims(token.Claims.(jwt.MapClaims)); jwtErr != nil { - return errors.New("Key not authorized: " + jwtErr.Error()), http.StatusUnauthorized + return errors.New("resourceKey not authorized: " + jwtErr.Error()), http.StatusUnauthorized } // Token is valid - let's move on @@ -730,7 +730,7 @@ func (k *JWTMiddleware) ProcessRequest(w http.ResponseWriter, r *http.Request, _ return errors.New(MsgKeyNotAuthorizedUnexpectedSigningMethod), http.StatusForbidden } } - return errors.New("Key not authorized"), http.StatusForbidden + return errors.New("resourceKey not authorized"), http.StatusForbidden } func ParseRSAPublicKey(data []byte) (interface{}, error) { @@ -776,7 +776,7 @@ func ctxSetJWTContextVars(s *APISpec, r *http.Request, token *jwt.Token) { cnt[claim] = claimValue } - // Key data + // resourceKey data cnt["token"] = ctxGetAuthToken(r) ctxSetData(r, cnt) @@ -798,7 +798,7 @@ func (gw *Gateway) generateSessionFromPolicy(policyID, orgID string, enforceOrg if enforceOrg { if policy.OrgID != orgID { log.Error("Attempting to apply policy from different organisation to key, skipping") - return session.Clone(), errors.New("Key not authorized: no matching policy") + return session.Clone(), errors.New("resourceKey not authorized: no matching policy") } } else { // Org isn;t enforced, so lets use the policy baseline diff --git a/gateway/oauth_manager_test.go b/gateway/oauth_manager_test.go index ecc74e6d474..1fb55209232 100644 --- a/gateway/oauth_manager_test.go +++ b/gateway/oauth_manager_test.go @@ -339,7 +339,7 @@ func TestOAuthTokenExpiration(t *testing.T) { assert.True(t, globalConf.LocalSessionCache.DisableCacheSessionState) // to fixate that the cache is disabled _, _ = ts.Run(t, []test.TestCase{ {Path: "/listen/get", Headers: authHeader, Method: http.MethodGet, Code: http.StatusOK, Delay: time.Second}, - {Path: "/listen/get", Headers: authHeader, Method: http.MethodGet, BodyMatch: "Key has expired, please renew", + {Path: "/listen/get", Headers: authHeader, Method: http.MethodGet, BodyMatch: "resourceKey has expired, please renew", Code: http.StatusUnauthorized}, }...) } @@ -544,7 +544,7 @@ func TestAPIClientAuthorizeToken(t *testing.T) { } session, ok := spec.AuthManager.SessionDetail("", token, false) if !ok { - t.Error("Key was not created (Can't find it)!") + t.Error("resourceKey was not created (Can't find it)!") } if session.MetaData == nil { t.Fatal("Session metadata is nil") @@ -695,7 +695,7 @@ func TestAPIClientAuthorizeTokenWithPolicy(t *testing.T) { // Verify the token is correct session, ok := spec.AuthManager.SessionDetail("", token, false) if !ok { - t.Error("Key was not created (Can't find it)!") + t.Error("resourceKey was not created (Can't find it)!") } if !reflect.DeepEqual(session.PolicyIDs(), []string{"TEST-4321"}) { diff --git a/gateway/rpc_storage_handler.go b/gateway/rpc_storage_handler.go index ba01c439fc6..ea8af2db47e 100644 --- a/gateway/rpc_storage_handler.go +++ b/gateway/rpc_storage_handler.go @@ -512,8 +512,8 @@ func (r *RPCStorageHandler) GetKeysAndValues() map[string]string { // DeleteKey will remove a key from the database func (r *RPCStorageHandler) DeleteKey(keyName string) bool { - log.Debug("DEL Key was: ", r.Gw.obfuscateKey(keyName)) - log.Debug("DEL Key became: ", r.Gw.obfuscateKey(r.fixKey(keyName))) + log.Debug("DEL resourceKey was: ", r.Gw.obfuscateKey(keyName)) + log.Debug("DEL resourceKey became: ", r.Gw.obfuscateKey(r.fixKey(keyName))) ok, err := rpc.FuncClientSingleton("DeleteKey", r.fixKey(keyName)) if err != nil { rpc.EmitErrorEventKv( @@ -937,7 +937,7 @@ func (gw *Gateway) getSessionAndCreate(keyName string, r *RPCStorageHandler, isH sessionString, err := r.GetRawKey("apikey-" + key) if err != nil { - log.Error("Key not found in master - skipping") + log.Error("resourceKey not found in master - skipping") } else { gw.handleAddKey(key, sessionString, orgId) } diff --git a/gateway/session_manager.go b/gateway/session_manager.go index c685ae4b4fe..7148a8b9919 100644 --- a/gateway/session_manager.go +++ b/gateway/session_manager.go @@ -276,7 +276,7 @@ func (l *SessionLimiter) RateLimitInfo(r *http.Request, api *APISpec, endpoints // ForwardMessage will enforce rate limiting, returning a non-zero // sessionFailReason if session limits have been exceeded. -// Key values to manage rate are Rate and Per, e.g. Rate of 10 messages +// resourceKey values to manage rate are Rate and Per, e.g. Rate of 10 messages // Per 10 seconds func (l *SessionLimiter) ForwardMessage(r *http.Request, session *user.SessionState, rateLimitKey string, quotaKey string, store storage.Handler, enableRL, enableQ bool, api *APISpec, dryRun bool) sessionFailReason { // check for limit on API level (set to session by ApplyPolicies) diff --git a/storage/dummy_test.go b/storage/dummy_test.go index 8814de1a598..2282cf00503 100644 --- a/storage/dummy_test.go +++ b/storage/dummy_test.go @@ -69,13 +69,13 @@ func TestDummyStorage_GetRawKey(t *testing.T) { wantErr bool }{ { - name: "Key exists", + name: "resourceKey exists", key: "key1", want: "value1", wantErr: false, }, { - name: "Key does not exist", + name: "resourceKey does not exist", key: "key2", want: "", wantErr: true, @@ -271,14 +271,14 @@ func TestDummyStorage_GetKey(t *testing.T) { errMsg string }{ { - name: "Key exists", + name: "resourceKey exists", key: "existingKey", want: "value1", wantErr: false, errMsg: "", }, { - name: "Key does not exist", + name: "resourceKey does not exist", key: "nonExistingKey", want: "", wantErr: true, @@ -444,7 +444,7 @@ func TestDummyStorage_RemoveFromList(t *testing.T) { name: "Remove from non-existing key", keyName: "nonExistingKey", value: "value1", - wantList: nil, // Key does not exist + wantList: nil, // resourceKey does not exist wantErr: false, }, } @@ -522,25 +522,25 @@ func TestDummyStorage_Exists(t *testing.T) { wantErr bool }{ { - name: "Key exists in Data", + name: "resourceKey exists in Data", keyName: "dataKey", want: true, wantErr: false, }, { - name: "Key exists in IndexList", + name: "resourceKey exists in IndexList", keyName: "indexKey", want: true, wantErr: false, }, { - name: "Key exists in both Data and IndexList", + name: "resourceKey exists in both Data and IndexList", keyName: "sharedKey", want: true, wantErr: false, }, { - name: "Key does not exist", + name: "resourceKey does not exist", keyName: "nonExistingKey", want: false, wantErr: false, diff --git a/storage/mdcb_storage.go b/storage/mdcb_storage.go index ad615a6efca..f1ff93fb6e1 100644 --- a/storage/mdcb_storage.go +++ b/storage/mdcb_storage.go @@ -8,12 +8,19 @@ 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 + CallbackOnPullCertificateFromRPC *func(key string, val string) error } +const ( + resourceOauthClient = "Oauth Client" + resourceCertificate = "Certificate" + resourceApiKey = "ApiKey" + resourceKey = "resourceKey" +) + func NewMdcbStorage(local, rpc Handler, log *logrus.Entry) *MdcbStorage { return &MdcbStorage{ local: local, @@ -23,45 +30,27 @@ func NewMdcbStorage(local, rpc Handler, log *logrus.Entry) *MdcbStorage { } 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("resourceKey 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 +245,56 @@ 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 { + var err error + if m.CallbackOnPullCertificateFromRPC != nil { + err = (*m.CallbackOnPullCertificateFromRPC)(key, val) + if err != nil { + m.logger.WithError(err).Error("cannot save resourceCertificate locally") + } + } + return err +} + +// cacheOAuthClient saved oauth data in local storage after pull from rpc +func (m MdcbStorage) cacheOAuthClient(key, val string) error { + err := m.local.SetKey(key, val, 0) + if err != nil { + m.logger.WithError(err).Errorf("Cannot save locally oauth client: %v", key) + } + return err +} + +// processResourceByType based on the type of key it will trigger the proper +// caching mechanism +func (m MdcbStorage) processResourceByType(key, val string) error { + var err error + resourceType := getResourceType(key) + switch resourceType { + case resourceOauthClient: + err = m.cacheOAuthClient(key, val) + case resourceCertificate: + err = m.cacheCertificate(key, val) + } + return err +} + +// 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 { + resourceType := getResourceType(key) + m.logger.Errorf("cannot retrieve %v from rpc: %v... resourceKey: %v", resourceType, err.Error(), key) + 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) +} diff --git a/storage/mdcb_storage_test.go b/storage/mdcb_storage_test.go index 0d12bc29940..feaad75d29a 100644 --- a/storage/mdcb_storage_test.go +++ b/storage/mdcb_storage_test.go @@ -2,22 +2,65 @@ package storage import ( "context" + "errors" + "github.com/TykTechnologies/tyk/storage/mock" + "go.uber.org/mock/gomock" "io" + "strings" "testing" "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" ) +type testSetup struct { + Logger *logrus.Entry + Local *mock.MockHandler + Remote *mock.MockHandler + MdcbStorage *MdcbStorage + CleanUp func() +} + +var notFoundKeyErr = errors.New("key not found") + +func setupTest(t *testing.T) *testSetup { + t.Helper() // Marks this function as a test helper + + logger := logrus.New() + logger.Out = io.Discard + log := logger.WithContext(context.Background()) + + ctrlLocal := gomock.NewController(t) + local := mock.NewMockHandler(ctrlLocal) + + ctrlRemote := gomock.NewController(t) + remote := mock.NewMockHandler(ctrlRemote) + + mdcbStorage := NewMdcbStorage(local, remote, log) + + cleanup := func() { + ctrlLocal.Finish() + ctrlRemote.Finish() + } + + return &testSetup{ + Logger: log, + Local: local, + Remote: remote, + MdcbStorage: mdcbStorage, + CleanUp: cleanup, + } +} + func TestGetResourceType(t *testing.T) { tests := []struct { key string expected string }{ - {"oauth-clientid.client-id", "Oauth Client"}, - {"cert.something", "certificate"}, - {"apikey.something", "api key"}, - {"unmatched-key", "key"}, + {"oauth-clientid.client-id", resourceOauthClient}, + {"cert.something", resourceCertificate}, + {"apikey.something", resourceApiKey}, + {"unmatched-key", resourceKey}, } for _, tt := range tests { @@ -89,3 +132,279 @@ func TestMdcbStorage_GetMultiKey(t *testing.T) { }) } } + +func TestGetFromLocalStorage(t *testing.T) { + setup := setupTest(t) + defer setup.CleanUp() + + mdcb := setup.MdcbStorage + setup.Local.EXPECT().GetKey("any").Return("exists", nil) + setup.Local.EXPECT().GetKey("nonExistingKey").Return("", notFoundKeyErr) + + localVal, err := mdcb.getFromLocal("any") + assert.Nil(t, err, "expected no error") + assert.Equal(t, "exists", localVal) + + notFoundVal, err := mdcb.getFromLocal("nonExistingKey") + assert.ErrorIs(t, err, notFoundKeyErr) + assert.Equal(t, "", notFoundVal) +} + +func TestGetFromRPCAndCache(t *testing.T) { + setup := setupTest(t) + defer setup.CleanUp() + + m := setup.MdcbStorage + rpcHandler := setup.Remote + + // attempt with keys that do not follow pattern for oauth, certs, apikeys + rpcHandler.EXPECT().GetKey("john").Return("doe", nil) + rpcHandler.EXPECT().GetKey("jane").Return("", notFoundKeyErr) + setup.Local.EXPECT().SetKey("john", gomock.Any(), gomock.Any()).Times(0) + setup.Local.EXPECT().SetKey("jane", gomock.Any(), gomock.Any()).Times(0) + + rpcVal, err := m.getFromRPCAndCache("john") + assert.Nil(t, err, "expected no error") + assert.Equal(t, "doe", rpcVal) + + rpcVal, err = m.getFromRPCAndCache("jane") + assert.Equal(t, "", rpcVal) + assert.ErrorIs(t, err, notFoundKeyErr) + + // test oauth keys + oauthClientKey := "oauth-clientid.my-client-id" + rpcHandler.EXPECT().GetKey(oauthClientKey).Return("value", nil) + setup.Local.EXPECT().SetKey(oauthClientKey, gomock.Any(), gomock.Any()).Times(1) + rpcVal, err = m.getFromRPCAndCache(oauthClientKey) + assert.Equal(t, "value", rpcVal) + + // test certs keys + // for certs we do not call directly the set key func, but the callback + count := 0 + mockSaveCert := func(key, val string) error { + count++ + return nil + } + m.CallbackOnPullCertificateFromRPC = &mockSaveCert + + certKey := "cert-my-cert-id" + rpcHandler.EXPECT().GetKey(certKey).Return("value", nil) + rpcVal, err = m.getFromRPCAndCache(certKey) + assert.Equal(t, "value", rpcVal) + assert.Equal(t, 1, count) +} + +func TestProcessResourceByType(t *testing.T) { + // Setup + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + mockLocal := mock.NewMockHandler(ctrl) + mockRemote := mock.NewMockHandler(ctrl) + + m := &MdcbStorage{ + local: mockLocal, + rpc: mockRemote, + } + + // Test cases + testCases := []struct { + name string + key string + val string + setupMocks func() + expectedError error + }{ + { + name: "Successful OAuth client caching", + key: "oauth:client1", + val: "clientdata1", + setupMocks: func() { + mockLocal.EXPECT().SetKey("oauth:client1", "clientdata1", gomock.Any()).Return(nil) + }, + expectedError: nil, + }, + { + name: "Failed OAuth client caching", + key: "oauth:failOAuth", + val: "clientdata2", + setupMocks: func() { + mockLocal.EXPECT().SetKey("oauth:failOAuth", "clientdata2", gomock.Any()).Return(errors.New("oauth caching failed")) + }, + expectedError: errors.New("oauth caching failed"), + }, + { + name: "Successful Certificate caching", + key: "cert:cert1", + val: "certdata1", + setupMocks: func() { + // Setup expectations for certificate caching if needed + }, + expectedError: nil, + }, + { + name: "Failed Certificate caching", + key: "cert:failCert", + val: "certdata2", + setupMocks: func() { + // Setup expectations for failed certificate caching if needed + }, + expectedError: errors.New("certificate caching failed"), + }, + { + name: "Unknown resource type", + key: "unknown:resource1", + val: "data1", + setupMocks: func() {}, + expectedError: nil, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Setup mocks + tc.setupMocks() + + // If testing certificate caching, setup the callback + if strings.HasPrefix(tc.key, "cert:") { + f := func(key, val string) error { + if key == "cert:failCert" { + return errors.New("certificate caching failed") + } + return nil + } + m.CallbackOnPullCertificateFromRPC = &f + } else { + m.CallbackOnPullCertificateFromRPC = nil + } + + err := m.processResourceByType(tc.key, tc.val) + + if tc.expectedError != nil { + assert.Error(t, err) + assert.Equal(t, tc.expectedError.Error(), err.Error()) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestCacheOAuthClient(t *testing.T) { + + // Test cases + testCases := []struct { + name string + key string + val string + setKeyErr error + expectLog bool + }{ + { + name: "Successful cache", + key: "oauth1", + val: "clientdata1", + setKeyErr: nil, + expectLog: false, + }, + { + name: "Cache failure", + key: "oauth2", + val: "clientdata2", + setKeyErr: errors.New("failed to set key"), + expectLog: true, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + // Setup + setup := setupTest(t) + defer setup.CleanUp() + + m := setup.MdcbStorage + localHandler := setup.Local + + localHandler.EXPECT().SetKey(tc.key, tc.val, gomock.Any()).Return(tc.setKeyErr) + err := m.cacheOAuthClient(tc.key, tc.val) + + if tc.setKeyErr != nil { + assert.Error(t, err, "Should return an error when SetKey fails") + assert.ErrorIs(t, tc.setKeyErr, err, "Returned error should match the SetKey error") + } else { + assert.NoError(t, err, "Should not return an error when SetKey succeeds") + } + + }) + } +} + +func TestCacheCertificate(t *testing.T) { + + // Test cases + testCases := []struct { + name string + key string + val string + callbackError error + shouldUseCallback bool + }{ + { + name: "Successful cache", + key: "cert1", + val: "certdata1", + callbackError: nil, + shouldUseCallback: true, + }, + { + name: "Cache failure", + key: "cert2", + val: "certdata2", + callbackError: errors.New("failed to save"), + shouldUseCallback: true, + }, + { + name: "Nil callback", + key: "cert3", + val: "certdata3", + callbackError: nil, + shouldUseCallback: false, + }, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + setup := setupTest(t) + defer setup.CleanUp() + + m := setup.MdcbStorage + + var callbackCalled bool + mockCallback := func(k, v string) error { + callbackCalled = true + assert.Equal(t, tc.key, k) + assert.Equal(t, tc.val, v) + return tc.callbackError + } + + if tc.shouldUseCallback { + m.CallbackOnPullCertificateFromRPC = &mockCallback + } + + // Call the method + err := m.cacheCertificate(tc.key, tc.val) + + // Assertions + if tc.shouldUseCallback { + assert.True(t, callbackCalled, "Callback should have been called") + if tc.callbackError != nil { + assert.ErrorIs(t, tc.callbackError, err) + } + } else { + assert.NoError(t, err) + assert.False(t, callbackCalled, "Callback should not have been called") + } + + }) + } +} diff --git a/storage/mock/storage.go b/storage/mock/storage.go new file mode 100644 index 00000000000..00e870dd41c --- /dev/null +++ b/storage/mock/storage.go @@ -0,0 +1,501 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: github.com/TykTechnologies/tyk/storage (interfaces: Handler) +// +// Generated by this command: +// +// mockgen -destination=./mock/storage.go -package mock . Handler +// +// Package mock is a generated GoMock package. +package mock + +import ( + reflect "reflect" + + gomock "go.uber.org/mock/gomock" +) + +// MockHandler is a mock of Handler interface. +type MockHandler struct { + ctrl *gomock.Controller + recorder *MockHandlerMockRecorder +} + +// MockHandlerMockRecorder is the mock recorder for MockHandler. +type MockHandlerMockRecorder struct { + mock *MockHandler +} + +// NewMockHandler creates a new mock instance. +func NewMockHandler(ctrl *gomock.Controller) *MockHandler { + mock := &MockHandler{ctrl: ctrl} + mock.recorder = &MockHandlerMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockHandler) EXPECT() *MockHandlerMockRecorder { + return m.recorder +} + +// AddToSet mocks base method. +func (m *MockHandler) AddToSet(arg0, arg1 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "AddToSet", arg0, arg1) +} + +// AddToSet indicates an expected call of AddToSet. +func (mr *MockHandlerMockRecorder) AddToSet(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddToSet", reflect.TypeOf((*MockHandler)(nil).AddToSet), arg0, arg1) +} + +// AddToSortedSet mocks base method. +func (m *MockHandler) AddToSortedSet(arg0, arg1 string, arg2 float64) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "AddToSortedSet", arg0, arg1, arg2) +} + +// AddToSortedSet indicates an expected call of AddToSortedSet. +func (mr *MockHandlerMockRecorder) AddToSortedSet(arg0, arg1, arg2 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AddToSortedSet", reflect.TypeOf((*MockHandler)(nil).AddToSortedSet), arg0, arg1, arg2) +} + +// AppendToSet mocks base method. +func (m *MockHandler) AppendToSet(arg0, arg1 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "AppendToSet", arg0, arg1) +} + +// AppendToSet indicates an expected call of AppendToSet. +func (mr *MockHandlerMockRecorder) AppendToSet(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AppendToSet", reflect.TypeOf((*MockHandler)(nil).AppendToSet), arg0, arg1) +} + +// Connect mocks base method. +func (m *MockHandler) Connect() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Connect") + ret0, _ := ret[0].(bool) + return ret0 +} + +// Connect indicates an expected call of Connect. +func (mr *MockHandlerMockRecorder) Connect() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Connect", reflect.TypeOf((*MockHandler)(nil).Connect)) +} + +// Decrement mocks base method. +func (m *MockHandler) Decrement(arg0 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "Decrement", arg0) +} + +// Decrement indicates an expected call of Decrement. +func (mr *MockHandlerMockRecorder) Decrement(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Decrement", reflect.TypeOf((*MockHandler)(nil).Decrement), arg0) +} + +// DeleteAllKeys mocks base method. +func (m *MockHandler) DeleteAllKeys() bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteAllKeys") + ret0, _ := ret[0].(bool) + return ret0 +} + +// DeleteAllKeys indicates an expected call of DeleteAllKeys. +func (mr *MockHandlerMockRecorder) DeleteAllKeys() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteAllKeys", reflect.TypeOf((*MockHandler)(nil).DeleteAllKeys)) +} + +// DeleteKey mocks base method. +func (m *MockHandler) DeleteKey(arg0 string) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteKey", arg0) + ret0, _ := ret[0].(bool) + return ret0 +} + +// DeleteKey indicates an expected call of DeleteKey. +func (mr *MockHandlerMockRecorder) DeleteKey(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteKey", reflect.TypeOf((*MockHandler)(nil).DeleteKey), arg0) +} + +// DeleteKeys mocks base method. +func (m *MockHandler) DeleteKeys(arg0 []string) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteKeys", arg0) + ret0, _ := ret[0].(bool) + return ret0 +} + +// DeleteKeys indicates an expected call of DeleteKeys. +func (mr *MockHandlerMockRecorder) DeleteKeys(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteKeys", reflect.TypeOf((*MockHandler)(nil).DeleteKeys), arg0) +} + +// DeleteRawKey mocks base method. +func (m *MockHandler) DeleteRawKey(arg0 string) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteRawKey", arg0) + ret0, _ := ret[0].(bool) + return ret0 +} + +// DeleteRawKey indicates an expected call of DeleteRawKey. +func (mr *MockHandlerMockRecorder) DeleteRawKey(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteRawKey", reflect.TypeOf((*MockHandler)(nil).DeleteRawKey), arg0) +} + +// DeleteRawKeys mocks base method. +func (m *MockHandler) DeleteRawKeys(arg0 []string) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteRawKeys", arg0) + ret0, _ := ret[0].(bool) + return ret0 +} + +// DeleteRawKeys indicates an expected call of DeleteRawKeys. +func (mr *MockHandlerMockRecorder) DeleteRawKeys(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteRawKeys", reflect.TypeOf((*MockHandler)(nil).DeleteRawKeys), arg0) +} + +// DeleteScanMatch mocks base method. +func (m *MockHandler) DeleteScanMatch(arg0 string) bool { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "DeleteScanMatch", arg0) + ret0, _ := ret[0].(bool) + return ret0 +} + +// DeleteScanMatch indicates an expected call of DeleteScanMatch. +func (mr *MockHandlerMockRecorder) DeleteScanMatch(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DeleteScanMatch", reflect.TypeOf((*MockHandler)(nil).DeleteScanMatch), arg0) +} + +// Exists mocks base method. +func (m *MockHandler) Exists(arg0 string) (bool, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Exists", arg0) + ret0, _ := ret[0].(bool) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// Exists indicates an expected call of Exists. +func (mr *MockHandlerMockRecorder) Exists(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Exists", reflect.TypeOf((*MockHandler)(nil).Exists), arg0) +} + +// GetAndDeleteSet mocks base method. +func (m *MockHandler) GetAndDeleteSet(arg0 string) []any { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetAndDeleteSet", arg0) + ret0, _ := ret[0].([]any) + return ret0 +} + +// GetAndDeleteSet indicates an expected call of GetAndDeleteSet. +func (mr *MockHandlerMockRecorder) GetAndDeleteSet(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetAndDeleteSet", reflect.TypeOf((*MockHandler)(nil).GetAndDeleteSet), arg0) +} + +// GetExp mocks base method. +func (m *MockHandler) GetExp(arg0 string) (int64, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetExp", arg0) + ret0, _ := ret[0].(int64) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetExp indicates an expected call of GetExp. +func (mr *MockHandlerMockRecorder) GetExp(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetExp", reflect.TypeOf((*MockHandler)(nil).GetExp), arg0) +} + +// GetKey mocks base method. +func (m *MockHandler) GetKey(arg0 string) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetKey", arg0) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetKey indicates an expected call of GetKey. +func (mr *MockHandlerMockRecorder) GetKey(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetKey", reflect.TypeOf((*MockHandler)(nil).GetKey), arg0) +} + +// GetKeyPrefix mocks base method. +func (m *MockHandler) GetKeyPrefix() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetKeyPrefix") + ret0, _ := ret[0].(string) + return ret0 +} + +// GetKeyPrefix indicates an expected call of GetKeyPrefix. +func (mr *MockHandlerMockRecorder) GetKeyPrefix() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetKeyPrefix", reflect.TypeOf((*MockHandler)(nil).GetKeyPrefix)) +} + +// GetKeys mocks base method. +func (m *MockHandler) GetKeys(arg0 string) []string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetKeys", arg0) + ret0, _ := ret[0].([]string) + return ret0 +} + +// GetKeys indicates an expected call of GetKeys. +func (mr *MockHandlerMockRecorder) GetKeys(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetKeys", reflect.TypeOf((*MockHandler)(nil).GetKeys), arg0) +} + +// GetKeysAndValues mocks base method. +func (m *MockHandler) GetKeysAndValues() map[string]string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetKeysAndValues") + ret0, _ := ret[0].(map[string]string) + return ret0 +} + +// GetKeysAndValues indicates an expected call of GetKeysAndValues. +func (mr *MockHandlerMockRecorder) GetKeysAndValues() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetKeysAndValues", reflect.TypeOf((*MockHandler)(nil).GetKeysAndValues)) +} + +// GetKeysAndValuesWithFilter mocks base method. +func (m *MockHandler) GetKeysAndValuesWithFilter(arg0 string) map[string]string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetKeysAndValuesWithFilter", arg0) + ret0, _ := ret[0].(map[string]string) + return ret0 +} + +// GetKeysAndValuesWithFilter indicates an expected call of GetKeysAndValuesWithFilter. +func (mr *MockHandlerMockRecorder) GetKeysAndValuesWithFilter(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetKeysAndValuesWithFilter", reflect.TypeOf((*MockHandler)(nil).GetKeysAndValuesWithFilter), arg0) +} + +// GetListRange mocks base method. +func (m *MockHandler) GetListRange(arg0 string, arg1, arg2 int64) ([]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetListRange", arg0, arg1, arg2) + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetListRange indicates an expected call of GetListRange. +func (mr *MockHandlerMockRecorder) GetListRange(arg0, arg1, arg2 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetListRange", reflect.TypeOf((*MockHandler)(nil).GetListRange), arg0, arg1, arg2) +} + +// GetMultiKey mocks base method. +func (m *MockHandler) GetMultiKey(arg0 []string) ([]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetMultiKey", arg0) + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetMultiKey indicates an expected call of GetMultiKey. +func (mr *MockHandlerMockRecorder) GetMultiKey(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetMultiKey", reflect.TypeOf((*MockHandler)(nil).GetMultiKey), arg0) +} + +// GetRawKey mocks base method. +func (m *MockHandler) GetRawKey(arg0 string) (string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetRawKey", arg0) + ret0, _ := ret[0].(string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetRawKey indicates an expected call of GetRawKey. +func (mr *MockHandlerMockRecorder) GetRawKey(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRawKey", reflect.TypeOf((*MockHandler)(nil).GetRawKey), arg0) +} + +// GetRollingWindow mocks base method. +func (m *MockHandler) GetRollingWindow(arg0 string, arg1 int64, arg2 bool) (int, []any) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetRollingWindow", arg0, arg1, arg2) + ret0, _ := ret[0].(int) + ret1, _ := ret[1].([]any) + return ret0, ret1 +} + +// GetRollingWindow indicates an expected call of GetRollingWindow. +func (mr *MockHandlerMockRecorder) GetRollingWindow(arg0, arg1, arg2 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetRollingWindow", reflect.TypeOf((*MockHandler)(nil).GetRollingWindow), arg0, arg1, arg2) +} + +// GetSet mocks base method. +func (m *MockHandler) GetSet(arg0 string) (map[string]string, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSet", arg0) + ret0, _ := ret[0].(map[string]string) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// GetSet indicates an expected call of GetSet. +func (mr *MockHandlerMockRecorder) GetSet(arg0 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSet", reflect.TypeOf((*MockHandler)(nil).GetSet), arg0) +} + +// GetSortedSetRange mocks base method. +func (m *MockHandler) GetSortedSetRange(arg0, arg1, arg2 string) ([]string, []float64, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetSortedSetRange", arg0, arg1, arg2) + ret0, _ := ret[0].([]string) + ret1, _ := ret[1].([]float64) + ret2, _ := ret[2].(error) + return ret0, ret1, ret2 +} + +// GetSortedSetRange indicates an expected call of GetSortedSetRange. +func (mr *MockHandlerMockRecorder) GetSortedSetRange(arg0, arg1, arg2 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetSortedSetRange", reflect.TypeOf((*MockHandler)(nil).GetSortedSetRange), arg0, arg1, arg2) +} + +// IncrememntWithExpire mocks base method. +func (m *MockHandler) IncrememntWithExpire(arg0 string, arg1 int64) int64 { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "IncrememntWithExpire", arg0, arg1) + ret0, _ := ret[0].(int64) + return ret0 +} + +// IncrememntWithExpire indicates an expected call of IncrememntWithExpire. +func (mr *MockHandlerMockRecorder) IncrememntWithExpire(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "IncrememntWithExpire", reflect.TypeOf((*MockHandler)(nil).IncrememntWithExpire), arg0, arg1) +} + +// RemoveFromList mocks base method. +func (m *MockHandler) RemoveFromList(arg0, arg1 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveFromList", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveFromList indicates an expected call of RemoveFromList. +func (mr *MockHandlerMockRecorder) RemoveFromList(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveFromList", reflect.TypeOf((*MockHandler)(nil).RemoveFromList), arg0, arg1) +} + +// RemoveFromSet mocks base method. +func (m *MockHandler) RemoveFromSet(arg0, arg1 string) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "RemoveFromSet", arg0, arg1) +} + +// RemoveFromSet indicates an expected call of RemoveFromSet. +func (mr *MockHandlerMockRecorder) RemoveFromSet(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveFromSet", reflect.TypeOf((*MockHandler)(nil).RemoveFromSet), arg0, arg1) +} + +// RemoveSortedSetRange mocks base method. +func (m *MockHandler) RemoveSortedSetRange(arg0, arg1, arg2 string) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "RemoveSortedSetRange", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// RemoveSortedSetRange indicates an expected call of RemoveSortedSetRange. +func (mr *MockHandlerMockRecorder) RemoveSortedSetRange(arg0, arg1, arg2 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RemoveSortedSetRange", reflect.TypeOf((*MockHandler)(nil).RemoveSortedSetRange), arg0, arg1, arg2) +} + +// SetExp mocks base method. +func (m *MockHandler) SetExp(arg0 string, arg1 int64) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetExp", arg0, arg1) + ret0, _ := ret[0].(error) + return ret0 +} + +// SetExp indicates an expected call of SetExp. +func (mr *MockHandlerMockRecorder) SetExp(arg0, arg1 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetExp", reflect.TypeOf((*MockHandler)(nil).SetExp), arg0, arg1) +} + +// SetKey mocks base method. +func (m *MockHandler) SetKey(arg0, arg1 string, arg2 int64) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetKey", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// SetKey indicates an expected call of SetKey. +func (mr *MockHandlerMockRecorder) SetKey(arg0, arg1, arg2 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetKey", reflect.TypeOf((*MockHandler)(nil).SetKey), arg0, arg1, arg2) +} + +// SetRawKey mocks base method. +func (m *MockHandler) SetRawKey(arg0, arg1 string, arg2 int64) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetRawKey", arg0, arg1, arg2) + ret0, _ := ret[0].(error) + return ret0 +} + +// SetRawKey indicates an expected call of SetRawKey. +func (mr *MockHandlerMockRecorder) SetRawKey(arg0, arg1, arg2 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetRawKey", reflect.TypeOf((*MockHandler)(nil).SetRawKey), arg0, arg1, arg2) +} + +// SetRollingWindow mocks base method. +func (m *MockHandler) SetRollingWindow(arg0 string, arg1 int64, arg2 string, arg3 bool) (int, []any) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "SetRollingWindow", arg0, arg1, arg2, arg3) + ret0, _ := ret[0].(int) + ret1, _ := ret[1].([]any) + return ret0, ret1 +} + +// SetRollingWindow indicates an expected call of SetRollingWindow. +func (mr *MockHandlerMockRecorder) SetRollingWindow(arg0, arg1, arg2, arg3 any) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "SetRollingWindow", reflect.TypeOf((*MockHandler)(nil).SetRollingWindow), arg0, arg1, arg2, arg3) +} diff --git a/storage/storage.go b/storage/storage.go index 2c74e54e8a4..e4b889e18b6 100644 --- a/storage/storage.go +++ b/storage/storage.go @@ -17,6 +17,8 @@ import ( "github.com/TykTechnologies/tyk/internal/uuid" ) +//go:generate mockgen -destination=./mock/storage.go -package mock . Handler + var log = logger.Get() // ErrKeyNotFound is a standard error for when a key is not found in the storage engine From a8d6075d3748f202453b567115b5581e4ee3a0c7 Mon Sep 17 00:00:00 2001 From: sredny buitrago Date: Tue, 3 Dec 2024 23:13:53 -0300 Subject: [PATCH 02/11] revert name change of key to resourceKey --- certs/manager_test.go | 2 +- gateway/api.go | 30 +++++++++++++++--------------- gateway/api_healthcheck.go | 2 +- gateway/api_test.go | 6 +++--- gateway/cert_test.go | 2 +- gateway/event_handler_webhooks.go | 2 +- gateway/gateway_test.go | 2 +- gateway/host_checker_manager.go | 2 +- gateway/middleware.go | 2 +- gateway/mw_auth_key.go | 2 +- gateway/mw_auth_key_test.go | 2 +- gateway/mw_jwt.go | 14 +++++++------- gateway/oauth_manager_test.go | 6 +++--- gateway/rpc_storage_handler.go | 6 +++--- gateway/session_manager.go | 2 +- storage/dummy_test.go | 18 +++++++++--------- storage/mdcb_storage.go | 6 +++--- 17 files changed, 53 insertions(+), 53 deletions(-) diff --git a/certs/manager_test.go b/certs/manager_test.go index 9f36083f0db..fa6449ceff8 100644 --- a/certs/manager_test.go +++ b/certs/manager_test.go @@ -184,7 +184,7 @@ func TestCertificateStorage(t *testing.T) { t.Error("Should return only private certificate") } - if leafSubjectName(certs[0]) != ("Public resourceKey: " + publicKeyID) { + if leafSubjectName(certs[0]) != ("Public Key: " + publicKeyID) { t.Error("Wrong cert", leafSubjectName(certs[0])) } }) diff --git a/gateway/api.go b/gateway/api.go index 05f80bfb388..751d085bfda 100644 --- a/gateway/api.go +++ b/gateway/api.go @@ -390,7 +390,7 @@ func (gw *Gateway) doAddOrUpdate(keyName string, newSession *user.SessionState, } } - logger.Info("resourceKey added or updated.") + logger.Info("Key added or updated.") return nil } @@ -461,22 +461,22 @@ func (gw *Gateway) handleAddOrUpdate(keyName string, r *http.Request, isHashed b keyName = key.KeyID if !found { log.Error("Could not find key when updating") - return apiError("resourceKey is not found"), http.StatusNotFound + return apiError("Key is not found"), http.StatusNotFound } originalKey = key.Clone() isCertificateChanged := newSession.Certificate != originalKey.Certificate if isCertificateChanged { if newSession.Certificate == "" { - log.Error("resourceKey must contain a certificate") - return apiError("resourceKey cannot be used without a certificate"), http.StatusBadRequest + log.Error("Key must contain a certificate") + return apiError("Key cannot be used without a certificate"), http.StatusBadRequest } // check that the certificate exists in the system _, err := gw.CertificateManager.GetRaw(newSession.Certificate) if err != nil { - log.Error("resourceKey must contain an existing certificate") - return apiError("resourceKey must be used with an existent certificate"), http.StatusBadRequest + log.Error("Key must contain an existing certificate") + return apiError("Key must be used with an existent certificate"), http.StatusBadRequest } } @@ -566,7 +566,7 @@ func (gw *Gateway) handleAddOrUpdate(keyName string, r *http.Request, isHashed b event = EventTokenCreated } gw.FireSystemEvent(event, EventTokenMeta{ - EventMetaDefault: EventMetaDefault{Message: "resourceKey modified."}, + EventMetaDefault: EventMetaDefault{Message: "Key modified."}, Org: newSession.OrgID, Key: keyName, }) @@ -595,7 +595,7 @@ func (gw *Gateway) handleAddOrUpdate(keyName string, r *http.Request, isHashed b func (gw *Gateway) handleGetDetail(sessionKey, apiID, orgID string, byHash bool) (interface{}, int) { if byHash && !gw.GetConfig().HashKeys { - return apiError("resourceKey requested by hash but key hashing is not enabled"), http.StatusBadRequest + return apiError("Key requested by hash but key hashing is not enabled"), http.StatusBadRequest } spec := gw.getApiSpec(apiID) @@ -606,7 +606,7 @@ func (gw *Gateway) handleGetDetail(sessionKey, apiID, orgID string, byHash bool) session, ok := gw.GlobalSessionManager.SessionDetail(orgID, sessionKey, byHash) sessionKey = session.KeyID if !ok { - return apiError("resourceKey not found"), http.StatusNotFound + return apiError("Key not found"), http.StatusNotFound } mw := &BaseMiddleware{Spec: spec, Gw: gw} @@ -804,7 +804,7 @@ func (gw *Gateway) handleDeleteKey(keyName, orgID, apiID string, resetQuota bool } gw.FireSystemEvent(EventTokenDeleted, EventTokenMeta{ - EventMetaDefault: EventMetaDefault{Message: "resourceKey deleted."}, + EventMetaDefault: EventMetaDefault{Message: "Key deleted."}, Org: orgID, Key: keyName, }) @@ -1707,7 +1707,7 @@ func (gw *Gateway) handleUpdateHashedKey(keyName string, applyPolicies []string) "status": "fail", }).Error("Failed to update hashed key.") - return apiError("resourceKey not found"), http.StatusNotFound + return apiError("Key not found"), http.StatusNotFound } // Set the policy @@ -1955,7 +1955,7 @@ func (gw *Gateway) createKeyHandler(w http.ResponseWriter, r *http.Request) { "prefix": "api", "status": "fail", "err": err, - }).Error("resourceKey creation failed.") + }).Error("Key creation failed.") doJSONWrite(w, http.StatusInternalServerError, apiError("Unmarshalling failed")) return } @@ -1969,7 +1969,7 @@ func (gw *Gateway) createKeyHandler(w http.ResponseWriter, r *http.Request) { newKey = gw.generateToken(newSession.OrgID, newSession.Certificate) _, ok := gw.GlobalSessionManager.SessionDetail(newSession.OrgID, newKey, false) if ok { - doJSONWrite(w, http.StatusInternalServerError, apiError("Failed to create key - resourceKey with given certificate already found:"+newKey)) + doJSONWrite(w, http.StatusInternalServerError, apiError("Failed to create key - Key with given certificate already found:"+newKey)) return } } @@ -2065,7 +2065,7 @@ func (gw *Gateway) createKeyHandler(w http.ResponseWriter, r *http.Request) { } gw.FireSystemEvent(EventTokenCreated, EventTokenMeta{ - EventMetaDefault: EventMetaDefault{Message: "resourceKey generated."}, + EventMetaDefault: EventMetaDefault{Message: "Key generated."}, Org: newSession.OrgID, Key: newKey, }) @@ -2093,7 +2093,7 @@ func (gw *Gateway) previewKeyHandler(w http.ResponseWriter, r *http.Request) { "prefix": "api", "status": "fail", "err": err, - }).Error("resourceKey creation failed.") + }).Error("Key creation failed.") doJSONWrite(w, http.StatusInternalServerError, apiError("Unmarshalling failed")) return } diff --git a/gateway/api_healthcheck.go b/gateway/api_healthcheck.go index 49b62661907..e8a8d5ce851 100644 --- a/gateway/api_healthcheck.go +++ b/gateway/api_healthcheck.go @@ -49,7 +49,7 @@ func (h *DefaultHealthChecker) Init(storeType storage.Handler) { } func (h *DefaultHealthChecker) CreateKeyName(subKey HealthPrefix) string { - // resourceKey should be API-ID.SubKey.123456789 + // Key should be API-ID.SubKey.123456789 return h.APIID + "." + string(subKey) } diff --git a/gateway/api_test.go b/gateway/api_test.go index fbcef1a86db..00bef2aa88a 100644 --- a/gateway/api_test.go +++ b/gateway/api_test.go @@ -656,11 +656,11 @@ func TestKeyHandler_DeleteKeyWithQuota(t *testing.T) { hashKeys bool }{ { - name: "resourceKey Hashing disabled", + name: "Key Hashing disabled", hashKeys: false, }, { - name: "resourceKey hashing enabled", + name: "Key hashing enabled", hashKeys: true, }, } @@ -1095,7 +1095,7 @@ func (ts *Test) testHashKeyHandlerHelper(t *testing.T, expectedHashSize int) { myKeyHash := storage.HashKey(ts.Gw.generateToken("default", myKey), ts.Gw.GetConfig().HashKeys) if len(myKeyHash) != expectedHashSize { - t.Errorf("Expected hash size: %d, got %d. Hash: %s. resourceKey: %s", expectedHashSize, len(myKeyHash), myKeyHash, myKey) + t.Errorf("Expected hash size: %d, got %d. Hash: %s. Key: %s", expectedHashSize, len(myKeyHash), myKeyHash, myKey) } t.Run("Create, get and delete key with key hashing", func(t *testing.T) { diff --git a/gateway/cert_test.go b/gateway/cert_test.go index 294ed29d4cf..e133cdb6911 100644 --- a/gateway/cert_test.go +++ b/gateway/cert_test.go @@ -1277,7 +1277,7 @@ func TestKeyWithCertificateTLS(t *testing.T) { }) // check that key has been updated with wrong certificate - t.Run("resourceKey has been updated with wrong certificate key", func(t *testing.T) { + t.Run("Key has been updated with wrong certificate key", func(t *testing.T) { clientPEM, _, _, clientCert := crypto.GenCertificate(&x509.Certificate{}, false) clientCertID, err := ts.Gw.CertificateManager.Add(clientPEM, orgId) diff --git a/gateway/event_handler_webhooks.go b/gateway/event_handler_webhooks.go index 50ce897cca8..e985a7b3ab7 100644 --- a/gateway/event_handler_webhooks.go +++ b/gateway/event_handler_webhooks.go @@ -126,7 +126,7 @@ func (w *WebHookHandler) Init(handlerConf interface{}) error { // hookFired checks if an event has been fired within the EventTimeout setting func (w *WebHookHandler) WasHookFired(checksum string) bool { if _, err := w.store.GetKey(checksum); err != nil { - // resourceKey not found, so hook is in limit + // Key not found, so hook is in limit log.WithFields(logrus.Fields{ "prefix": "webhooks", }).Debug("Event can fire, no duplicates found") diff --git a/gateway/gateway_test.go b/gateway/gateway_test.go index 52addea370d..2238f65e13c 100644 --- a/gateway/gateway_test.go +++ b/gateway/gateway_test.go @@ -342,7 +342,7 @@ func TestQuota(t *testing.T) { body, _ := ioutil.ReadAll(r.Body) json.Unmarshal(body, &data) - if data["event"] != "QuotaExceeded" || data["message"] != "resourceKey Quota Limit Exceeded" || data["key"] != keyID { + if data["event"] != "QuotaExceeded" || data["message"] != "Key Quota Limit Exceeded" || data["key"] != keyID { t.Error("Webhook payload not match", data) } })) diff --git a/gateway/host_checker_manager.go b/gateway/host_checker_manager.go index 161ec7460a7..76ac2c9b201 100644 --- a/gateway/host_checker_manager.go +++ b/gateway/host_checker_manager.go @@ -311,7 +311,7 @@ func (hc *HostCheckerManager) HostDown(urlStr string) bool { log.WithFields(logrus.Fields{ "prefix": "host-check-mgr", - }).Debug("resourceKey is: ", PoolerHostSentinelKeyPrefix+u.Host) + }).Debug("Key is: ", PoolerHostSentinelKeyPrefix+u.Host) key := PoolerHostSentinelKeyPrefix + u.Host // If the node doesn't perform any uptime checks, query the storage: diff --git a/gateway/middleware.go b/gateway/middleware.go index f1a41c8eb72..c3d85649d76 100644 --- a/gateway/middleware.go +++ b/gateway/middleware.go @@ -420,7 +420,7 @@ func (t *BaseMiddleware) CheckSessionAndIdentityForValidKey(originalKey string, if !t.Spec.GlobalConfig.LocalSessionCache.DisableCacheSessionState { cachedVal, found := t.Gw.SessionCache.Get(cacheKey) if found { - t.Logger().Debug("--> resourceKey found in local cache") + t.Logger().Debug("--> Key found in local cache") session := cachedVal.(user.SessionState).Clone() if err := t.ApplyPolicies(&session); err != nil { t.Logger().Error(err) diff --git a/gateway/mw_auth_key.go b/gateway/mw_auth_key.go index dc633ad794c..25dd659468c 100644 --- a/gateway/mw_auth_key.go +++ b/gateway/mw_auth_key.go @@ -78,7 +78,7 @@ func (k *AuthKey) setContextVars(r *http.Request, token string) { return } if cnt := ctxGetData(r); cnt != nil { - // resourceKey data + // Key data cnt["token"] = token ctxSetData(r, cnt) } diff --git a/gateway/mw_auth_key_test.go b/gateway/mw_auth_key_test.go index a49b46b7de7..f1b4b5e0776 100644 --- a/gateway/mw_auth_key_test.go +++ b/gateway/mw_auth_key_test.go @@ -315,7 +315,7 @@ func createAuthKeyAuthSession(isBench bool) *user.SessionState { session.QuotaRemaining = 10 session.QuotaMax = 10 } - session.AccessRights = map[string]user.AccessDefinition{"31": {APIName: "Tyk Auth resourceKey Test", APIID: "31", Versions: []string{"default"}}} + session.AccessRights = map[string]user.AccessDefinition{"31": {APIName: "Tyk Auth Key Test", APIID: "31", Versions: []string{"default"}}} return session } diff --git a/gateway/mw_jwt.go b/gateway/mw_jwt.go index 88ab409830f..00343df702e 100644 --- a/gateway/mw_jwt.go +++ b/gateway/mw_jwt.go @@ -382,7 +382,7 @@ func (k *JWTMiddleware) processCentralisedJWT(r *http.Request, token *jwt.Token) foundPolicy := false if !exists { // Create it - k.Logger().Debug("resourceKey does not exist, creating") + k.Logger().Debug("Key does not exist, creating") // We need a base policy as a template, either get it from the token itself OR a proxy client ID within Tyk basePolicyID, foundPolicy = k.getBasePolicyID(r, claims) @@ -612,7 +612,7 @@ func (k *JWTMiddleware) processCentralisedJWT(r *http.Request, token *jwt.Token) // ensure to set the sessionID session.KeyID = sessionID - k.Logger().Debug("resourceKey found") + k.Logger().Debug("Key found") switch k.Spec.BaseIdentityProvidedBy { case apidef.JWTClaim, apidef.UnsetAuth: ctxSetSession(r, &session, updateSession, k.Gw.GetConfig().HashKeys) @@ -647,7 +647,7 @@ func (k *JWTMiddleware) processOneToOneTokenMap(r *http.Request, token *jwt.Toke if !exists { k.reportLoginFailure(tykId, r) - return errors.New("resourceKey not authorized"), http.StatusForbidden + return errors.New("Key not authorized"), http.StatusForbidden } k.Logger().Debug("Raw key ID found.") @@ -707,7 +707,7 @@ func (k *JWTMiddleware) ProcessRequest(w http.ResponseWriter, r *http.Request, _ if err == nil && token.Valid { if jwtErr := k.timeValidateJWTClaims(token.Claims.(jwt.MapClaims)); jwtErr != nil { - return errors.New("resourceKey not authorized: " + jwtErr.Error()), http.StatusUnauthorized + return errors.New("Key not authorized: " + jwtErr.Error()), http.StatusUnauthorized } // Token is valid - let's move on @@ -730,7 +730,7 @@ func (k *JWTMiddleware) ProcessRequest(w http.ResponseWriter, r *http.Request, _ return errors.New(MsgKeyNotAuthorizedUnexpectedSigningMethod), http.StatusForbidden } } - return errors.New("resourceKey not authorized"), http.StatusForbidden + return errors.New("Key not authorized"), http.StatusForbidden } func ParseRSAPublicKey(data []byte) (interface{}, error) { @@ -776,7 +776,7 @@ func ctxSetJWTContextVars(s *APISpec, r *http.Request, token *jwt.Token) { cnt[claim] = claimValue } - // resourceKey data + // Key data cnt["token"] = ctxGetAuthToken(r) ctxSetData(r, cnt) @@ -798,7 +798,7 @@ func (gw *Gateway) generateSessionFromPolicy(policyID, orgID string, enforceOrg if enforceOrg { if policy.OrgID != orgID { log.Error("Attempting to apply policy from different organisation to key, skipping") - return session.Clone(), errors.New("resourceKey not authorized: no matching policy") + return session.Clone(), errors.New("Key not authorized: no matching policy") } } else { // Org isn;t enforced, so lets use the policy baseline diff --git a/gateway/oauth_manager_test.go b/gateway/oauth_manager_test.go index 1fb55209232..ecc74e6d474 100644 --- a/gateway/oauth_manager_test.go +++ b/gateway/oauth_manager_test.go @@ -339,7 +339,7 @@ func TestOAuthTokenExpiration(t *testing.T) { assert.True(t, globalConf.LocalSessionCache.DisableCacheSessionState) // to fixate that the cache is disabled _, _ = ts.Run(t, []test.TestCase{ {Path: "/listen/get", Headers: authHeader, Method: http.MethodGet, Code: http.StatusOK, Delay: time.Second}, - {Path: "/listen/get", Headers: authHeader, Method: http.MethodGet, BodyMatch: "resourceKey has expired, please renew", + {Path: "/listen/get", Headers: authHeader, Method: http.MethodGet, BodyMatch: "Key has expired, please renew", Code: http.StatusUnauthorized}, }...) } @@ -544,7 +544,7 @@ func TestAPIClientAuthorizeToken(t *testing.T) { } session, ok := spec.AuthManager.SessionDetail("", token, false) if !ok { - t.Error("resourceKey was not created (Can't find it)!") + t.Error("Key was not created (Can't find it)!") } if session.MetaData == nil { t.Fatal("Session metadata is nil") @@ -695,7 +695,7 @@ func TestAPIClientAuthorizeTokenWithPolicy(t *testing.T) { // Verify the token is correct session, ok := spec.AuthManager.SessionDetail("", token, false) if !ok { - t.Error("resourceKey was not created (Can't find it)!") + t.Error("Key was not created (Can't find it)!") } if !reflect.DeepEqual(session.PolicyIDs(), []string{"TEST-4321"}) { diff --git a/gateway/rpc_storage_handler.go b/gateway/rpc_storage_handler.go index ea8af2db47e..ba01c439fc6 100644 --- a/gateway/rpc_storage_handler.go +++ b/gateway/rpc_storage_handler.go @@ -512,8 +512,8 @@ func (r *RPCStorageHandler) GetKeysAndValues() map[string]string { // DeleteKey will remove a key from the database func (r *RPCStorageHandler) DeleteKey(keyName string) bool { - log.Debug("DEL resourceKey was: ", r.Gw.obfuscateKey(keyName)) - log.Debug("DEL resourceKey became: ", r.Gw.obfuscateKey(r.fixKey(keyName))) + log.Debug("DEL Key was: ", r.Gw.obfuscateKey(keyName)) + log.Debug("DEL Key became: ", r.Gw.obfuscateKey(r.fixKey(keyName))) ok, err := rpc.FuncClientSingleton("DeleteKey", r.fixKey(keyName)) if err != nil { rpc.EmitErrorEventKv( @@ -937,7 +937,7 @@ func (gw *Gateway) getSessionAndCreate(keyName string, r *RPCStorageHandler, isH sessionString, err := r.GetRawKey("apikey-" + key) if err != nil { - log.Error("resourceKey not found in master - skipping") + log.Error("Key not found in master - skipping") } else { gw.handleAddKey(key, sessionString, orgId) } diff --git a/gateway/session_manager.go b/gateway/session_manager.go index 7148a8b9919..c685ae4b4fe 100644 --- a/gateway/session_manager.go +++ b/gateway/session_manager.go @@ -276,7 +276,7 @@ func (l *SessionLimiter) RateLimitInfo(r *http.Request, api *APISpec, endpoints // ForwardMessage will enforce rate limiting, returning a non-zero // sessionFailReason if session limits have been exceeded. -// resourceKey values to manage rate are Rate and Per, e.g. Rate of 10 messages +// Key values to manage rate are Rate and Per, e.g. Rate of 10 messages // Per 10 seconds func (l *SessionLimiter) ForwardMessage(r *http.Request, session *user.SessionState, rateLimitKey string, quotaKey string, store storage.Handler, enableRL, enableQ bool, api *APISpec, dryRun bool) sessionFailReason { // check for limit on API level (set to session by ApplyPolicies) diff --git a/storage/dummy_test.go b/storage/dummy_test.go index 2282cf00503..8814de1a598 100644 --- a/storage/dummy_test.go +++ b/storage/dummy_test.go @@ -69,13 +69,13 @@ func TestDummyStorage_GetRawKey(t *testing.T) { wantErr bool }{ { - name: "resourceKey exists", + name: "Key exists", key: "key1", want: "value1", wantErr: false, }, { - name: "resourceKey does not exist", + name: "Key does not exist", key: "key2", want: "", wantErr: true, @@ -271,14 +271,14 @@ func TestDummyStorage_GetKey(t *testing.T) { errMsg string }{ { - name: "resourceKey exists", + name: "Key exists", key: "existingKey", want: "value1", wantErr: false, errMsg: "", }, { - name: "resourceKey does not exist", + name: "Key does not exist", key: "nonExistingKey", want: "", wantErr: true, @@ -444,7 +444,7 @@ func TestDummyStorage_RemoveFromList(t *testing.T) { name: "Remove from non-existing key", keyName: "nonExistingKey", value: "value1", - wantList: nil, // resourceKey does not exist + wantList: nil, // Key does not exist wantErr: false, }, } @@ -522,25 +522,25 @@ func TestDummyStorage_Exists(t *testing.T) { wantErr bool }{ { - name: "resourceKey exists in Data", + name: "Key exists in Data", keyName: "dataKey", want: true, wantErr: false, }, { - name: "resourceKey exists in IndexList", + name: "Key exists in IndexList", keyName: "indexKey", want: true, wantErr: false, }, { - name: "resourceKey exists in both Data and IndexList", + name: "Key exists in both Data and IndexList", keyName: "sharedKey", want: true, wantErr: false, }, { - name: "resourceKey does not exist", + name: "Key does not exist", keyName: "nonExistingKey", want: false, wantErr: false, diff --git a/storage/mdcb_storage.go b/storage/mdcb_storage.go index f1ff93fb6e1..79d17c38ea8 100644 --- a/storage/mdcb_storage.go +++ b/storage/mdcb_storage.go @@ -18,7 +18,7 @@ const ( resourceOauthClient = "Oauth Client" resourceCertificate = "Certificate" resourceApiKey = "ApiKey" - resourceKey = "resourceKey" + resourceKey = "Key" ) func NewMdcbStorage(local, rpc Handler, log *logrus.Entry) *MdcbStorage { @@ -35,7 +35,7 @@ func (m MdcbStorage) GetKey(key string) (string, error) { if err == nil { return val, nil } - m.logger.Debugf("resourceKey not present locally, pulling from rpc layer: %v", err) + m.logger.Debugf("Key not present locally, pulling from rpc layer: %v", err) } return m.getFromRPCAndCache(key) @@ -286,7 +286,7 @@ func (m MdcbStorage) getFromRPCAndCache(key string) (string, error) { val, err := m.rpc.GetKey(key) if err != nil { resourceType := getResourceType(key) - m.logger.Errorf("cannot retrieve %v from rpc: %v... resourceKey: %v", resourceType, err.Error(), key) + m.logger.Errorf("cannot retrieve %v from rpc: %v... Key: %v", resourceType, err.Error(), key) return "", err } From 23e42032e1398d7a4770f30078b72d7d720f0060 Mon Sep 17 00:00:00 2001 From: sredny buitrago Date: Wed, 4 Dec 2024 11:55:30 -0300 Subject: [PATCH 03/11] addressing linter comments --- storage/mdcb_storage_test.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/storage/mdcb_storage_test.go b/storage/mdcb_storage_test.go index feaad75d29a..a9f9b570153 100644 --- a/storage/mdcb_storage_test.go +++ b/storage/mdcb_storage_test.go @@ -3,12 +3,13 @@ package storage import ( "context" "errors" - "github.com/TykTechnologies/tyk/storage/mock" - "go.uber.org/mock/gomock" "io" "strings" "testing" + "github.com/TykTechnologies/tyk/storage/mock" + "go.uber.org/mock/gomock" + "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" ) @@ -177,11 +178,12 @@ func TestGetFromRPCAndCache(t *testing.T) { setup.Local.EXPECT().SetKey(oauthClientKey, gomock.Any(), gomock.Any()).Times(1) rpcVal, err = m.getFromRPCAndCache(oauthClientKey) assert.Equal(t, "value", rpcVal) + assert.Nil(t, err) // test certs keys // for certs we do not call directly the set key func, but the callback count := 0 - mockSaveCert := func(key, val string) error { + mockSaveCert := func(_, _ string) error { count++ return nil } @@ -192,6 +194,7 @@ func TestGetFromRPCAndCache(t *testing.T) { rpcVal, err = m.getFromRPCAndCache(certKey) assert.Equal(t, "value", rpcVal) assert.Equal(t, 1, count) + assert.Nil(t, err) } func TestProcessResourceByType(t *testing.T) { @@ -267,7 +270,7 @@ func TestProcessResourceByType(t *testing.T) { // If testing certificate caching, setup the callback if strings.HasPrefix(tc.key, "cert:") { - f := func(key, val string) error { + f := func(key, _ string) error { if key == "cert:failCert" { return errors.New("certificate caching failed") } From 56d10ed98ce20396f832f04f6ae87953f0269c2e Mon Sep 17 00:00:00 2001 From: sredny buitrago Date: Wed, 4 Dec 2024 15:50:17 -0300 Subject: [PATCH 04/11] lint mdcb tests storage --- storage/mdcb_storage_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/storage/mdcb_storage_test.go b/storage/mdcb_storage_test.go index a9f9b570153..d22dc24b8f4 100644 --- a/storage/mdcb_storage_test.go +++ b/storage/mdcb_storage_test.go @@ -7,9 +7,10 @@ import ( "strings" "testing" - "github.com/TykTechnologies/tyk/storage/mock" "go.uber.org/mock/gomock" + "github.com/TykTechnologies/tyk/storage/mock" + "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" ) From 783d82cb4f5a807e79d76141becfa50382121ef2 Mon Sep 17 00:00:00 2001 From: sredny buitrago Date: Wed, 4 Dec 2024 16:19:43 -0300 Subject: [PATCH 05/11] formatting mock --- storage/mock/storage.go | 1 + 1 file changed, 1 insertion(+) diff --git a/storage/mock/storage.go b/storage/mock/storage.go index 00e870dd41c..362b2645eae 100644 --- a/storage/mock/storage.go +++ b/storage/mock/storage.go @@ -5,6 +5,7 @@ // // mockgen -destination=./mock/storage.go -package mock . Handler // + // Package mock is a generated GoMock package. package mock From 9d2eedb5ecebbed7f3351d4d8bc009d38d8db76e Mon Sep 17 00:00:00 2001 From: sredny buitrago Date: Wed, 4 Dec 2024 17:26:34 -0300 Subject: [PATCH 06/11] fix test --- storage/mdcb_storage.go | 16 +++++----- storage/mdcb_storage_test.go | 58 +++++++++++++++++------------------- 2 files changed, 34 insertions(+), 40 deletions(-) diff --git a/storage/mdcb_storage.go b/storage/mdcb_storage.go index 79d17c38ea8..d8f4948b608 100644 --- a/storage/mdcb_storage.go +++ b/storage/mdcb_storage.go @@ -2,6 +2,7 @@ package storage import ( "errors" + "fmt" "strings" "github.com/sirupsen/logrus" @@ -260,25 +261,22 @@ func (m MdcbStorage) cacheCertificate(key, val string) error { // cacheOAuthClient saved oauth data in local storage after pull from rpc func (m MdcbStorage) cacheOAuthClient(key, val string) error { - err := m.local.SetKey(key, val, 0) - if err != nil { - m.logger.WithError(err).Errorf("Cannot save locally oauth client: %v", key) - } - return err + 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 { - var err error + resourceType := getResourceType(key) switch resourceType { case resourceOauthClient: - err = m.cacheOAuthClient(key, val) + fmt.Println("Will attend oauth lcient") + return m.cacheOAuthClient(key, val) case resourceCertificate: - err = m.cacheCertificate(key, val) + return m.cacheCertificate(key, val) } - return err + return nil } // getFromRPCAndCache pulls a resource from rpc and stores it in local redis for caching diff --git a/storage/mdcb_storage_test.go b/storage/mdcb_storage_test.go index d22dc24b8f4..34b23240719 100644 --- a/storage/mdcb_storage_test.go +++ b/storage/mdcb_storage_test.go @@ -25,12 +25,16 @@ type testSetup struct { var notFoundKeyErr = errors.New("key not found") -func setupTest(t *testing.T) *testSetup { - t.Helper() // Marks this function as a test helper - +func getTestLogger() *logrus.Entry { logger := logrus.New() logger.Out = io.Discard log := logger.WithContext(context.Background()) + return log +} + +func setupTest(t *testing.T) *testSetup { + t.Helper() // Marks this function as a test helper + log := getTestLogger() ctrlLocal := gomock.NewController(t) local := mock.NewMockHandler(ctrlLocal) @@ -200,48 +204,39 @@ func TestGetFromRPCAndCache(t *testing.T) { func TestProcessResourceByType(t *testing.T) { // Setup - ctrl := gomock.NewController(t) - defer ctrl.Finish() - - mockLocal := mock.NewMockHandler(ctrl) - mockRemote := mock.NewMockHandler(ctrl) - - m := &MdcbStorage{ - local: mockLocal, - rpc: mockRemote, - } + errCachingFailed := errors.New("caching failed") // Test cases testCases := []struct { name string key string val string - setupMocks func() + setupMocks func(handler *mock.MockHandler) expectedError error }{ { name: "Successful OAuth client caching", - key: "oauth:client1", + key: "oauth-clientid.client1", val: "clientdata1", - setupMocks: func() { - mockLocal.EXPECT().SetKey("oauth:client1", "clientdata1", gomock.Any()).Return(nil) + setupMocks: func(mockLocal *mock.MockHandler) { + mockLocal.EXPECT().SetKey("oauth-clientid.client1", "clientdata1", gomock.Any()).Return(nil) }, expectedError: nil, }, { name: "Failed OAuth client caching", - key: "oauth:failOAuth", + key: "oauth-clientid.failClient2", val: "clientdata2", - setupMocks: func() { - mockLocal.EXPECT().SetKey("oauth:failOAuth", "clientdata2", gomock.Any()).Return(errors.New("oauth caching failed")) + setupMocks: func(mockLocal *mock.MockHandler) { + mockLocal.EXPECT().SetKey("oauth-clientid.failClient2", "clientdata2", gomock.Any()).Return(errCachingFailed) }, - expectedError: errors.New("oauth caching failed"), + expectedError: errCachingFailed, }, { name: "Successful Certificate caching", key: "cert:cert1", val: "certdata1", - setupMocks: func() { + setupMocks: func(_ *mock.MockHandler) { // Setup expectations for certificate caching if needed }, expectedError: nil, @@ -250,43 +245,44 @@ func TestProcessResourceByType(t *testing.T) { name: "Failed Certificate caching", key: "cert:failCert", val: "certdata2", - setupMocks: func() { + setupMocks: func(_ *mock.MockHandler) { // Setup expectations for failed certificate caching if needed }, - expectedError: errors.New("certificate caching failed"), + expectedError: errCachingFailed, }, { name: "Unknown resource type", key: "unknown:resource1", val: "data1", - setupMocks: func() {}, + setupMocks: func(_ *mock.MockHandler) {}, expectedError: nil, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - // Setup mocks - tc.setupMocks() + setup := setupTest(t) + defer setup.CleanUp() + + m := setup.MdcbStorage + tc.setupMocks(setup.Local) // If testing certificate caching, setup the callback if strings.HasPrefix(tc.key, "cert:") { f := func(key, _ string) error { if key == "cert:failCert" { - return errors.New("certificate caching failed") + return errCachingFailed } return nil } m.CallbackOnPullCertificateFromRPC = &f - } else { - m.CallbackOnPullCertificateFromRPC = nil } err := m.processResourceByType(tc.key, tc.val) if tc.expectedError != nil { assert.Error(t, err) - assert.Equal(t, tc.expectedError.Error(), err.Error()) + assert.ErrorIs(t, err, tc.expectedError) } else { assert.NoError(t, err) } From 6acabf677802fa0f563d8c9b10fc8ffd00be9ffd Mon Sep 17 00:00:00 2001 From: "Sredny M." Date: Wed, 4 Dec 2024 17:45:26 -0300 Subject: [PATCH 07/11] Fix code scanning alert no. 701: Clear-text logging of sensitive information Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com> --- storage/mdcb_storage.go | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/storage/mdcb_storage.go b/storage/mdcb_storage.go index d8f4948b608..efc1c86bc11 100644 --- a/storage/mdcb_storage.go +++ b/storage/mdcb_storage.go @@ -6,6 +6,7 @@ import ( "strings" "github.com/sirupsen/logrus" + "crypto/sha256" ) type MdcbStorage struct { @@ -284,7 +285,7 @@ func (m MdcbStorage) getFromRPCAndCache(key string) (string, error) { val, err := m.rpc.GetKey(key) if err != nil { resourceType := getResourceType(key) - m.logger.Errorf("cannot retrieve %v from rpc: %v... Key: %v", resourceType, err.Error(), key) + m.logger.Errorf("cannot retrieve %v from rpc: %v... Key: %v", resourceType, err.Error(), obfuscateKey(key)) return "", err } @@ -296,3 +297,8 @@ func (m MdcbStorage) getFromRPCAndCache(key string) (string, error) { func (m MdcbStorage) getFromLocal(key string) (string, error) { return m.local.GetKey(key) } + +func obfuscateKey(key string) string { + hash := sha256.Sum256([]byte(key)) + return fmt.Sprintf("%x", hash[:]) +} From 0fa1b6ede941e4fe5019b4cc1ba7b68549995d60 Mon Sep 17 00:00:00 2001 From: sredny buitrago Date: Wed, 4 Dec 2024 17:55:45 -0300 Subject: [PATCH 08/11] fix obsfuscate --- storage/mdcb_storage.go | 8 -------- 1 file changed, 8 deletions(-) diff --git a/storage/mdcb_storage.go b/storage/mdcb_storage.go index efc1c86bc11..7edad18a10a 100644 --- a/storage/mdcb_storage.go +++ b/storage/mdcb_storage.go @@ -6,7 +6,6 @@ import ( "strings" "github.com/sirupsen/logrus" - "crypto/sha256" ) type MdcbStorage struct { @@ -284,8 +283,6 @@ func (m MdcbStorage) processResourceByType(key, val string) error { func (m MdcbStorage) getFromRPCAndCache(key string) (string, error) { val, err := m.rpc.GetKey(key) if err != nil { - resourceType := getResourceType(key) - m.logger.Errorf("cannot retrieve %v from rpc: %v... Key: %v", resourceType, err.Error(), obfuscateKey(key)) return "", err } @@ -297,8 +294,3 @@ func (m MdcbStorage) getFromRPCAndCache(key string) (string, error) { func (m MdcbStorage) getFromLocal(key string) (string, error) { return m.local.GetKey(key) } - -func obfuscateKey(key string) string { - hash := sha256.Sum256([]byte(key)) - return fmt.Sprintf("%x", hash[:]) -} From d4670a866c4621e12aa3ab8b385751ac10ec0596 Mon Sep 17 00:00:00 2001 From: sredny buitrago Date: Wed, 4 Dec 2024 19:55:34 -0300 Subject: [PATCH 09/11] removing fmt print line --- storage/mdcb_storage.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/storage/mdcb_storage.go b/storage/mdcb_storage.go index 7edad18a10a..706c99e4d47 100644 --- a/storage/mdcb_storage.go +++ b/storage/mdcb_storage.go @@ -2,7 +2,6 @@ package storage import ( "errors" - "fmt" "strings" "github.com/sirupsen/logrus" @@ -271,7 +270,6 @@ func (m MdcbStorage) processResourceByType(key, val string) error { resourceType := getResourceType(key) switch resourceType { case resourceOauthClient: - fmt.Println("Will attend oauth lcient") return m.cacheOAuthClient(key, val) case resourceCertificate: return m.cacheCertificate(key, val) From 31ecb2d3c30659cb63a37c7c77f92f832b869fef Mon Sep 17 00:00:00 2001 From: sredny buitrago Date: Thu, 5 Dec 2024 12:32:01 -0300 Subject: [PATCH 10/11] addressing pr requested changes --- gateway/server.go | 1 + storage/mdcb_storage.go | 29 +++++++++++++---------------- storage/mdcb_storage_test.go | 11 +++++------ 3 files changed, 19 insertions(+), 22 deletions(-) diff --git a/gateway/server.go b/gateway/server.go index aa20ee19eea..5de82249979 100644 --- a/gateway/server.go +++ b/gateway/server.go @@ -1589,6 +1589,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 index 706c99e4d47..a4e78b597d2 100644 --- a/storage/mdcb_storage.go +++ b/storage/mdcb_storage.go @@ -8,24 +8,25 @@ import ( ) type MdcbStorage struct { - local Handler - rpc Handler - logger *logrus.Entry - CallbackOnPullCertificateFromRPC *func(key string, val string) error + local Handler + rpc Handler + logger *logrus.Entry + OnRPCCertPull func(key string, val string) error } const ( - resourceOauthClient = "Oauth Client" + resourceOauthClient = "OauthClient" resourceCertificate = "Certificate" resourceApiKey = "ApiKey" resourceKey = "Key" ) -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 { return &MdcbStorage{ - local: local, - rpc: rpc, - logger: log, + local: local, + rpc: rpc, + logger: log, + OnRPCCertPull: OnRPCCertPull, } } @@ -248,14 +249,10 @@ func (m MdcbStorage) Exists(key string) (bool, error) { // cacheCertificate saves locally resourceCertificate after pull from rpc func (m MdcbStorage) cacheCertificate(key, val string) error { - var err error - if m.CallbackOnPullCertificateFromRPC != nil { - err = (*m.CallbackOnPullCertificateFromRPC)(key, val) - if err != nil { - m.logger.WithError(err).Error("cannot save resourceCertificate locally") - } + if m.OnRPCCertPull == nil { + return nil } - return err + return m.OnRPCCertPull(key, val) } // cacheOAuthClient saved oauth data in local storage after pull from rpc diff --git a/storage/mdcb_storage_test.go b/storage/mdcb_storage_test.go index 34b23240719..91a9505c1d9 100644 --- a/storage/mdcb_storage_test.go +++ b/storage/mdcb_storage_test.go @@ -42,7 +42,7 @@ func setupTest(t *testing.T) *testSetup { ctrlRemote := gomock.NewController(t) remote := mock.NewMockHandler(ctrlRemote) - mdcbStorage := NewMdcbStorage(local, remote, log) + mdcbStorage := NewMdcbStorage(local, remote, log, nil) cleanup := func() { ctrlLocal.Finish() @@ -100,7 +100,7 @@ func TestMdcbStorage_GetMultiKey(t *testing.T) { logger.Out = io.Discard log := logger.WithContext(context.Background()) - mdcb := NewMdcbStorage(localHandler, rpcHandler, log) + mdcb := NewMdcbStorage(localHandler, rpcHandler, log, nil) testsCases := []struct { name string @@ -192,7 +192,7 @@ func TestGetFromRPCAndCache(t *testing.T) { count++ return nil } - m.CallbackOnPullCertificateFromRPC = &mockSaveCert + m.OnRPCCertPull = mockSaveCert certKey := "cert-my-cert-id" rpcHandler.EXPECT().GetKey(certKey).Return("value", nil) @@ -269,13 +269,12 @@ func TestProcessResourceByType(t *testing.T) { // If testing certificate caching, setup the callback if strings.HasPrefix(tc.key, "cert:") { - f := func(key, _ string) error { + m.OnRPCCertPull = func(key, _ string) error { if key == "cert:failCert" { return errCachingFailed } return nil } - m.CallbackOnPullCertificateFromRPC = &f } err := m.processResourceByType(tc.key, tc.val) @@ -388,7 +387,7 @@ func TestCacheCertificate(t *testing.T) { } if tc.shouldUseCallback { - m.CallbackOnPullCertificateFromRPC = &mockCallback + m.OnRPCCertPull = mockCallback } // Call the method From 224ec09949b81349002f1d0ba9c885eb69fa5828 Mon Sep 17 00:00:00 2001 From: sredny buitrago Date: Thu, 5 Dec 2024 12:38:11 -0300 Subject: [PATCH 11/11] addressing pr change request in certs --- certs/manager.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/certs/manager.go b/certs/manager.go index e45e0a01454..f86dc53a8ec 100644 --- 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.CallbackOnPullCertificateFromRPC = &callbackOnPullCertFromRPC - + mdcbStorage := storage.NewMdcbStorage(localStorage, rpcStorage, log, callbackOnPullCertFromRPC) cm.storage = mdcbStorage return cm }