You are viewing a plain text version of this content. The canonical link for it is here.

Posted to reviews@spark.apache.org by jerryshao <gi...@git.apache.org> on 2018/04/24 08:31:48 UTC

[GitHub] spark pull request #21138: [SPARK-24062][Thrift Server] Fix SASL encryption ...

GitHub user jerryshao opened a pull request:

    https://github.com/apache/spark/pull/21138

    [SPARK-24062][Thrift Server] Fix SASL encryption cannot enabled issue in thrift server

    ## What changes were proposed in this pull request?
    
    For the details of the exception please see [SPARK-24062](https://issues.apache.org/jira/browse/SPARK-24062).
    
    The issue is:
    
    Spark on Yarn stores SASL secret in current UGI's credentials, this credentials will be distributed to AM and executors, so that executors and drive share the same secret to communicate. But STS/Hive library code will refresh the current UGI by UGI's loginFromKeytab() after Spark application is started, this will create a new UGI in the current driver's context with empty tokens and secret keys, so secret key is lost in the current context's UGI, that's why Spark driver throws secret key not found exception.
    
    In Spark 2.2 code, Spark also stores this secret key in SecurityManager's class variable, so even UGI is refreshed, the secret is still existed in the object, so STS with SASL can still be worked in Spark 2.2. But in Spark 2.3, we always search key from current UGI, which makes it fail to work in Spark 2.3.
    
    To fix this issue, there're two possible solutions:
    
    1. Fix in STS/Hive library, when a new UGI is refreshed, copy the secret key from original UGI to the new one. The difficulty is that some codes to refresh the UGI is existed in Hive library, which makes us hard to change the code.
    2. Roll back the logics in SecurityManager to match Spark 2.2, so that this issue can be fixed.
    
    2nd solution seems a simple one. So I will propose a PR with 2nd solution.
    
    ## How was this patch tested?
    
    Verified in local cluster.
    
    CC @vanzin  @tgravescs  please help to review. Thanks!


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/jerryshao/apache-spark SPARK-24062

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/21138.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #21138
    
----
commit 00776858c2e776f46dbe542effe52a19283e752f
Author: jerryshao <ss...@...>
Date:   2018-04-24T08:14:27Z

    Fix secret key not found issue in STS

----


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #21138: [SPARK-24062][Thrift Server] Fix SASL encryption cannot ...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21138
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #21138: [SPARK-24062][Thrift Server] Fix SASL encryption cannot ...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21138
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #21138: [SPARK-24062][Thrift Server] Fix SASL encryption cannot ...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21138
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/2622/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #21138: [SPARK-24062][Thrift Server] Fix SASL encryption cannot ...

Posted by jerryshao <gi...@git.apache.org>.

Github user jerryshao commented on the issue:

    https://github.com/apache/spark/pull/21138
  
    Jenkins, retest this please.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #21138: [SPARK-24062][Thrift Server] Fix SASL encryption cannot ...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21138
  
    **[Test build #89864 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89864/testReport)** for PR 21138 at commit [`0077685`](https://github.com/apache/spark/commit/00776858c2e776f46dbe542effe52a19283e752f).


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #21138: [SPARK-24062][Thrift Server] Fix SASL encryption cannot ...

Posted by SparkQA <gi...@git.apache.org>.

Github user SparkQA commented on the issue:

    https://github.com/apache/spark/pull/21138
  
    **[Test build #89773 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/89773/testReport)** for PR 21138 at commit [`0077685`](https://github.com/apache/spark/commit/00776858c2e776f46dbe542effe52a19283e752f).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #21138: [SPARK-24062][Thrift Server] Fix SASL encryption cannot ...

Posted by jerryshao <gi...@git.apache.org>.

Github user jerryshao commented on the issue:

    https://github.com/apache/spark/pull/21138
  
    Thanks for the review @mridulm @vanzin . Let me test again. I will merge the code when test is passed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #21138: [SPARK-24062][Thrift Server] Fix SASL encryption cannot ...

Posted by jerryshao <gi...@git.apache.org>.

Github user jerryshao commented on the issue:

    https://github.com/apache/spark/pull/21138
  
    Merging to master and branch 2.3.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #21138: [SPARK-24062][Thrift Server] Fix SASL encryption cannot ...

Posted by mridulm <gi...@git.apache.org>.

Github user mridulm commented on the issue:

    https://github.com/apache/spark/pull/21138
  
    Sounds good to me; any thoughts @vanzin ? (since you changed this last)


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #21138: [SPARK-24062][Thrift Server] Fix SASL encryption cannot ...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21138
  
    Merged build finished. Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #21138: [SPARK-24062][Thrift Server] Fix SASL encryption cannot ...

Posted by mridulm <gi...@git.apache.org>.

Github user mridulm commented on the issue:

    https://github.com/apache/spark/pull/21138
  
    
    @jerryshao As we discussed IRL `HiveClientImpl` was one place this is happening (now fixed - git pull delays, my bad).
    The other is in `HadoopThriftAuthBridge` we rely on - `createServer` results in always doing a `UGI.loginUserFromKeytab` : resulting in changing the static `loginUser` in UGI (even though principal and keytab are the same).
    This results in :
    * The current bug - where secret token's are now gone.
    * Long running STS in secure cluster failing (due to hadoop IPC failure)
    ** This is the reason why I found/had worked on this - though for 1.6 release.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #21138: [SPARK-24062][Thrift Server] Fix SASL encryption cannot ...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21138
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/89773/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #21138: [SPARK-24062][Thrift Server] Fix SASL encryption cannot ...

Posted by jerryshao <gi...@git.apache.org>.

Github user jerryshao commented on the issue:

    https://github.com/apache/spark/pull/21138
  
    Hi @mridulm , thanks a lot for your comments.
    
    UGI.loginUserFromKeytab is not existed any more in Spark 2.3+ (https://github.com/apache/spark/commit/dc2714da50ecba1bf1fdf555a82a4314f763a76e#diff-6fd847124f8eae45ba2de1cf7d6296fe).
    
    Actually it is the code here (https://github.com/apache/spark/blob/e77d62a722941ce1cf235861d21b1f73089be134/sql/hive-thriftserver/src/main/scala/org/apache/spark/sql/hive/thriftserver/SparkSQLCLIService.scala#L53) in ThriftServer and somewhere else in Hive Library which calls UGI.loginUserFromKeytab, note here the principal and keytab is hive one, not sure if we can remove this.
    
    PS. I saw two "Login successful" from UGI in the thrift server log, but I can only find out one login in the thrift server code. So I assume another one is in the Hive library.
    
    Yes, I agree with you, ideally we should not login from keytab unnecessarily, but thinking of thrift server as a Spark application, it doesn't know the context of Spark's UGI and do login to refresh the UGI in its context, seems we cannot defend user to do that in the user layer. So I think my fix could workaround such issue, though may not be the elegant fix.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #21138: [SPARK-24062][Thrift Server] Fix SASL encryption cannot ...

Posted by AmplabJenkins <gi...@git.apache.org>.

Github user AmplabJenkins commented on the issue:

    https://github.com/apache/spark/pull/21138
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/2683/
    Test PASSed.


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org

[GitHub] spark issue #21138: [SPARK-24062][Thrift Server] Fix SASL encryption cannot ...

Posted by jerryshao <gi...@git.apache.org>.

Github user jerryshao commented on the issue:

    https://github.com/apache/spark/pull/21138
  
    @mridulm I would treat the current fix as a workaround for SASL issue, since it is a regression in 2.3.
    
    For UGI refreshing issue (mainly cause STS long running failure, also lead to SASL failure here), I think we can create a separate JIRA to fix the issue. Since this is not a regression.
    
    What do you think?


---

---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org