You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@phoenix.apache.org by "joshelser (via GitHub)" <gi...@apache.org> on 2023/03/16 15:42:46 UTC

[GitHub] [phoenix-queryserver] joshelser commented on a diff in pull request #123: PHOENIX-6908 KerberosName$NoMatchingRule exception in QueryServer.Pho…

joshelser commented on code in PR #123:
URL: https://github.com/apache/phoenix-queryserver/pull/123#discussion_r1138900115


##########
phoenix-queryserver/src/main/java/org/apache/phoenix/queryserver/server/QueryServer.java:
##########
@@ -633,6 +625,26 @@ SimpleLRUCache<String,UserGroupInformation> getCache() {
       }
   }
 
+  /**
+   * The new Jetty kerberos implementation that we use strips the realm from the principal, which
+   * and Hadoop cannot process that.
+   * This strips the hostname part as well, so that only the username remains.
+
+   * @param remoteUserName the principal as received from Jetty
+   * @return the principal without the hostname part
+   */
+  //FIXME We are probably compensating for a Jetty a bug, which should really be fixed in Jetty

Review Comment:
   Is there still a latent Jetty bug? I thought updating to the "new" SPNEGO API in Jetty would have addressed any lingering issues around parsing.



##########
phoenix-queryserver/src/main/java/org/apache/phoenix/queryserver/server/QueryServer.java:
##########
@@ -633,6 +625,26 @@ SimpleLRUCache<String,UserGroupInformation> getCache() {
       }
   }
 
+  /**
+   * The new Jetty kerberos implementation that we use strips the realm from the principal, which
+   * and Hadoop cannot process that.
+   * This strips the hostname part as well, so that only the username remains.
+
+   * @param remoteUserName the principal as received from Jetty
+   * @return the principal without the hostname part
+   */
+  //FIXME We are probably compensating for a Jetty a bug, which should really be fixed in Jetty
+  private static String stripHostNameFromPrincipal(String remoteUserName) {
+      // realm got removed from remoteUserName in CALCITE-4152
+      // so we remove the instance name to avoid geting KerberosName$NoMatchingRule exception
+      int atSignIndex = remoteUserName.indexOf('@');
+      int separatorIndex = remoteUserName.indexOf('/');
+      if (atSignIndex == -1 && separatorIndex > 0) {
+        remoteUserName = remoteUserName.substring(0, separatorIndex);
+      }

Review Comment:
   Getting a principal `phoenix/pqs.apache.org` is plausible, but so would `phoenix/pqs.apache.org@APACHE.ORG` is too. I think your code would only strip the principal instance if the realm were not provided. I think you could remove the `atSignIndex` logic from this method.



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@phoenix.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org