issue #85 fix, getConfig behavior polish

issue #85 fix: If the client fails to login to nacos server at initialization stage, it will crash If the server's password is changed when the client is running, the client will try to login the server every 30 seconds and put this issue to log NacosConfigService::getConfig behavior change: When the server returns 403(no permission), the client will try to get local snapshot, if there is no snapshot, the getConfig() will throw an exception with errorcode = 403 and the user is to decide failback strategy
nacos-group · Apr 10, 2022 · 707be61 · 707be61
1 parent e145b91
commit 707be61
Show file tree

Hide file tree

Showing 3 changed files with 24 additions and 6 deletions.
diff --git a/src/config/NacosConfigService.cpp b/src/config/NacosConfigService.cpp
@@ -74,11 +74,14 @@ NacosString NacosConfigService::getConfigInner
         result = _objectConfigData->_clientWorker->getServerConfig(tenant, dataId, group, timeoutMs);
     } catch (NacosException &e) {
         if (e.errorcode() == NacosException::NO_RIGHT) {
-            throw e;
+            log_error("Invalid credential, e: %d = %s\n", e.errorcode(), e.what());
         }
-
         const NacosString &clientName = _appConfigManager->get(PropertyKeyConst::CLIENT_NAME);
         result = _localSnapshotManager->getSnapshot(clientName, dataId, group, tenant);
+        if (e.errorcode() == NacosException::NO_RIGHT && NacosStringOps::isNullStr(result)) {
+            //permission denied and no failback, let user decide what to do
+            throw e;
+        }
     }
     return result;
 }

diff --git a/src/security/SecurityManager.cpp b/src/security/SecurityManager.cpp
@@ -18,7 +18,7 @@ SecurityManager::SecurityManager(ObjectConfigData *objectConfigData) {
 }
 void SecurityManager::doLogin(const NacosString &serverAddr) NACOS_THROW(NacosException, NetworkException) {
     //TODO:refactor string constants
-    NacosString url = serverAddr + "/" + ConfigConstant::DEFAULT_CONTEXT_PATH + "/v1/auth/users/login";
+    NacosString url = serverAddr + "/" + _objectConfigData->_appConfigManager->getContextPath() + "/v1/auth/users/login";
     list <NacosString> headers;
     list <NacosString> paramValues;
 
@@ -62,8 +62,17 @@ void SecurityManager::login() NACOS_THROW (NacosException) {
             //for some cases, e.g.:invalid username/password,
             //we should throw exception directly since retry on another node will not correct this problem
             if (e.errorcode() == NacosException::INVALID_LOGIN_CREDENTIAL) {
+                /**
+                 * Here we don't need to keep log for it, because there are 3 situations where we will call this login() routine:
+                 * 1. Initialization stage of NamingService
+                 * 2. Initialization stage of ConfigService
+                 * 3. tokenRefreshThreadFunc's invocation of this routine to refresh the credentials
+                 * In case 1 and case 2, the program will crash, because these situations are considered as a config error of the program, so let it crash
+                 * In case 3, the log is printed by tokenRefreshThreadFunc to remind the dev-ops to correct the config
+                */
                 throw e;
             }
+            log_error("Unknown error while login to server, e:%d = %s\n", e.errorcode(), e.what());
             continue;
         }
         //login succeeded
@@ -116,8 +125,14 @@ void *SecurityManager::tokenRefreshThreadFunc(void *param) {
             thisObj->login();
         } catch (NacosException &e) {
             if (e.errorcode() == NacosException::INVALID_LOGIN_CREDENTIAL) {
-                log_error("Invalid credential!\n");
-                throw e;//Invalid login credential, let it crash
+                /**
+                 * invalid credential while the application is running, wait for a moment and retry
+                 * since the existing data in the nacos client is still usable for the application
+                 * we should keep the application alive for the Availability, but the Consistency, in this case, is NOT guaranteed
+                 * the error log reminds the dev-ops to check the config
+                 */
+                log_error("Invalid credential, please check your server settings!\n");
+                sleep(30);
             } else if (e.errorcode() == NacosException::ALL_SERVERS_TRIED_AND_FAILED) {
                 log_warn("Network down, sleep for 30 secs and retry\n");
                 sleep(30);//network down, wait for a moment

diff --git a/src/server/ServerListManager.cpp b/src/server/ServerListManager.cpp
@@ -152,7 +152,7 @@ list <NacosServerInfo> ServerListManager::tryPullServerListFromNacosServer() NAC
         log_debug("Trying to access server:%s\n", server.getCompleteAddress().c_str());
         try {
             HttpResult serverRes = _objectConfigData->_httpDelegate->httpGet(
-                    server.getCompleteAddress() + "/" + ConfigConstant::DEFAULT_CONTEXT_PATH + "/"
+                    server.getCompleteAddress() + "/" + _objectConfigData->_appConfigManager->getContextPath() + "/"
                     + ConfigConstant::PROTOCOL_VERSION + "/" + ConfigConstant::GET_SERVERS_PATH,
                     headers, paramValues, NULLSTR, _read_timeout);
             return JSON::Json2NacosServerInfo(serverRes.content);