[GitHub] [hbase] belugabehr commented on a change in pull request #884: HBASE-23347 Allowable custom authentication methods for RPCs

[GitBox <gi...@apache.org>]

belugabehr commented on a change in pull request #884: HBASE-23347 Allowable custom authentication methods for RPCs
URL: https://github.com/apache/hbase/pull/884#discussion_r356743983
 
 

 ##########
 File path: hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/BlockingRpcConnection.java
 ##########
 @@ -391,40 +392,49 @@ private void handleSaslConnectionFailure(final int currRetries, final int maxRet
     user.doAs(new PrivilegedExceptionAction<Object>() {
       @Override
       public Object run() throws IOException, InterruptedException {
-        if (shouldAuthenticateOverKrb()) {
-          if (currRetries < maxRetries) {
-            if (LOG.isDebugEnabled()) {
-              LOG.debug("Exception encountered while connecting to " +
-                "the server : " + StringUtils.stringifyException(ex));
-            }
-            // try re-login
-            relogin();
-            disposeSasl();
-            // have granularity of milliseconds
-            // we are sleeping with the Connection lock held but since this
-            // connection instance is being used for connecting to the server
-            // in question, it is okay
-            Thread.sleep(ThreadLocalRandom.current().nextInt(reloginMaxBackoff) + 1);
-            return null;
-          } else {
-            String msg = "Couldn't setup connection for "
-                + UserGroupInformation.getLoginUser().getUserName() + " to " + serverPrincipal;
-            LOG.warn(msg, ex);
-            throw new IOException(msg, ex);
+        // A provider which failed authentication, but doesn't have the ability to relogin with
+        // some external system (e.g. username/password, the password either works or it doesn't)
+        if (!provider.canRetry()) {
+          LOG.warn("Exception encountered while connecting to the server : " + ex);
+          if (ex instanceof RemoteException) {
+            throw (RemoteException) ex;
           }
-        } else {
-          LOG.warn("Exception encountered while connecting to " + "the server : " + ex);
-        }
-        if (ex instanceof RemoteException) {
-          throw (RemoteException) ex;
+          if (ex instanceof SaslException) {
+            String msg = "SASL authentication failed."
+                + " The most likely cause is missing or invalid credentials. Consider 'kinit'.";
+            LOG.error(HBaseMarkers.FATAL, msg, ex);
+            throw new RuntimeException(msg, ex);
 
 Review comment:
   Please do not 'log and throw' - do one or the other, not both.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services