You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@storm.apache.org by HeartSaVioR <gi...@git.apache.org> on 2018/01/23 04:37:06 UTC

[GitHub] storm pull request #2527: STORM-2906 Pick HBase delegation token properly wh...

GitHub user HeartSaVioR opened a pull request:

    https://github.com/apache/storm/pull/2527

    STORM-2906 Pick HBase delegation token properly while handling HBase …

    …auth via delegation token
    
    Please refer [STORM-2906](https://issues.apache.org/jira/browse/STORM-2906) to see more details.

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

    $ git pull https://github.com/HeartSaVioR/storm STORM-2906

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

    https://github.com/apache/storm/pull/2527.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 #2527
    
----
commit d291f39c9aa6e29184a2712797e956b42cae3293
Author: Jungtaek Lim <ka...@...>
Date:   2018-01-23T04:34:22Z

    STORM-2906 Pick HBase delegation token properly while handling HBase auth via delegation token

----


---

[GitHub] storm pull request #2527: STORM-2906 Pick HBase delegation token properly wh...

Posted by satishd <gi...@git.apache.org>.
Github user satishd commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2527#discussion_r163225612
  
    --- Diff: external/storm-hbase/src/main/java/org/apache/storm/hbase/common/Utils.java ---
    @@ -47,14 +48,28 @@ public static HTable getTable(UserProvider provider, Configuration config, Strin
                 ugi = UserGroupInformation.getCurrentUser();
     
                 LOG.debug("UGI for current USER : {}", ugi.getUserName());
    +            boolean foundHBaseAuthToken = false;
                 for (Token<? extends TokenIdentifier> token : ugi.getTokens()) {
                     LOG.debug("Token in UGI (delegation token): {} / {}", token.toString(),
                             token.decodeIdentifier().getUser());
     
    -                // use UGI from token
    -                ugi = token.decodeIdentifier().getUser();
    -                ugi.addToken(token);
    +                // token.getKind() = Text, Text is annotated by @Stringable
    +                // which ensures toString() implementation
    +                if (token.getKind().toString().equals(TOKEN_KIND_HBASE_AUTH_TOKEN)) {
    +                    // use UGI from token
    +                    LOG.debug("Found HBASE_AUTH_TOKEN - using the token to replace current user.");
    +
    +                    ugi = token.decodeIdentifier().getUser();
    +                    ugi.addToken(token);
    +
    +                    foundHBaseAuthToken = true;
    --- End diff --
    
    I am trying to understand the current limitations. Can a worker support two or more HBase bolts talk to respective HBase clusters using delegation tokens? 


---

[GitHub] storm pull request #2527: STORM-2906 Pick HBase delegation token properly wh...

Posted by satishd <gi...@git.apache.org>.
Github user satishd commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2527#discussion_r163450608
  
    --- Diff: external/storm-hbase/src/main/java/org/apache/storm/hbase/common/Utils.java ---
    @@ -47,14 +48,28 @@ public static HTable getTable(UserProvider provider, Configuration config, Strin
                 ugi = UserGroupInformation.getCurrentUser();
     
                 LOG.debug("UGI for current USER : {}", ugi.getUserName());
    +            boolean foundHBaseAuthToken = false;
                 for (Token<? extends TokenIdentifier> token : ugi.getTokens()) {
                     LOG.debug("Token in UGI (delegation token): {} / {}", token.toString(),
                             token.decodeIdentifier().getUser());
     
    -                // use UGI from token
    -                ugi = token.decodeIdentifier().getUser();
    -                ugi.addToken(token);
    +                // token.getKind() = Text, Text is annotated by @Stringable
    +                // which ensures toString() implementation
    +                if (token.getKind().toString().equals(TOKEN_KIND_HBASE_AUTH_TOKEN)) {
    +                    // use UGI from token
    +                    LOG.debug("Found HBASE_AUTH_TOKEN - using the token to replace current user.");
    +
    +                    ugi = token.decodeIdentifier().getUser();
    +                    ugi.addToken(token);
    +
    +                    foundHBaseAuthToken = true;
    --- End diff --
    
    @HeartSaVioR There should not be multiple tokens with HBASE_AUTH_TOKEN key with the current implementation. It is better to log a message(atleast with info level) if we find multiple tokens so that it is easy to debug such scenarios.


---

[GitHub] storm issue #2527: STORM-2906 Pick HBase delegation token properly while han...

Posted by satishd <gi...@git.apache.org>.
Github user satishd commented on the issue:

    https://github.com/apache/storm/pull/2527
  
    +1 
    Thanks @HeartSaVioR for addressing review comments.


---

[GitHub] storm pull request #2527: STORM-2906 Pick HBase delegation token properly wh...

Posted by satishd <gi...@git.apache.org>.
Github user satishd commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2527#discussion_r163188315
  
    --- Diff: external/storm-hbase/src/main/java/org/apache/storm/hbase/common/Utils.java ---
    @@ -47,14 +48,28 @@ public static HTable getTable(UserProvider provider, Configuration config, Strin
                 ugi = UserGroupInformation.getCurrentUser();
     
                 LOG.debug("UGI for current USER : {}", ugi.getUserName());
    +            boolean foundHBaseAuthToken = false;
                 for (Token<? extends TokenIdentifier> token : ugi.getTokens()) {
                     LOG.debug("Token in UGI (delegation token): {} / {}", token.toString(),
                             token.decodeIdentifier().getUser());
     
    -                // use UGI from token
    -                ugi = token.decodeIdentifier().getUser();
    -                ugi.addToken(token);
    +                // token.getKind() = Text, Text is annotated by @Stringable
    +                // which ensures toString() implementation
    +                if (token.getKind().toString().equals(TOKEN_KIND_HBASE_AUTH_TOKEN)) {
    +                    // use UGI from token
    +                    LOG.debug("Found HBASE_AUTH_TOKEN - using the token to replace current user.");
    +
    +                    ugi = token.decodeIdentifier().getUser();
    +                    ugi.addToken(token);
    +
    +                    foundHBaseAuthToken = true;
    --- End diff --
    
    @HeartSaVioR Can there be more than one HBASE_AUTH_TOKEN token for the current UGI?


---

[GitHub] storm pull request #2527: STORM-2906 Pick HBase delegation token properly wh...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2527#discussion_r163457487
  
    --- Diff: external/storm-hbase/src/main/java/org/apache/storm/hbase/common/Utils.java ---
    @@ -47,14 +48,28 @@ public static HTable getTable(UserProvider provider, Configuration config, Strin
                 ugi = UserGroupInformation.getCurrentUser();
     
                 LOG.debug("UGI for current USER : {}", ugi.getUserName());
    +            boolean foundHBaseAuthToken = false;
                 for (Token<? extends TokenIdentifier> token : ugi.getTokens()) {
                     LOG.debug("Token in UGI (delegation token): {} / {}", token.toString(),
                             token.decodeIdentifier().getUser());
     
    -                // use UGI from token
    -                ugi = token.decodeIdentifier().getUser();
    -                ugi.addToken(token);
    +                // token.getKind() = Text, Text is annotated by @Stringable
    +                // which ensures toString() implementation
    +                if (token.getKind().toString().equals(TOKEN_KIND_HBASE_AUTH_TOKEN)) {
    +                    // use UGI from token
    +                    LOG.debug("Found HBASE_AUTH_TOKEN - using the token to replace current user.");
    +
    +                    ugi = token.decodeIdentifier().getUser();
    +                    ugi.addToken(token);
    +
    +                    foundHBaseAuthToken = true;
    --- End diff --
    
    @satishd Addressed.


---

[GitHub] storm pull request #2527: STORM-2906 Pick HBase delegation token properly wh...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/storm/pull/2527


---

[GitHub] storm pull request #2527: STORM-2906 Pick HBase delegation token properly wh...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2527#discussion_r163403457
  
    --- Diff: external/storm-hbase/src/main/java/org/apache/storm/hbase/common/Utils.java ---
    @@ -47,14 +48,28 @@ public static HTable getTable(UserProvider provider, Configuration config, Strin
                 ugi = UserGroupInformation.getCurrentUser();
     
                 LOG.debug("UGI for current USER : {}", ugi.getUserName());
    +            boolean foundHBaseAuthToken = false;
                 for (Token<? extends TokenIdentifier> token : ugi.getTokens()) {
                     LOG.debug("Token in UGI (delegation token): {} / {}", token.toString(),
                             token.decodeIdentifier().getUser());
     
    -                // use UGI from token
    -                ugi = token.decodeIdentifier().getUser();
    -                ugi.addToken(token);
    +                // token.getKind() = Text, Text is annotated by @Stringable
    +                // which ensures toString() implementation
    +                if (token.getKind().toString().equals(TOKEN_KIND_HBASE_AUTH_TOKEN)) {
    +                    // use UGI from token
    +                    LOG.debug("Found HBASE_AUTH_TOKEN - using the token to replace current user.");
    +
    +                    ugi = token.decodeIdentifier().getUser();
    +                    ugi.addToken(token);
    +
    +                    foundHBaseAuthToken = true;
    --- End diff --
    
    I didn't test it actually, but looks like we don't support it, since we are only using **one** of tokens. Not sure HDFS/Hive bolt supports it, too.


---

[GitHub] storm pull request #2527: STORM-2906 Pick HBase delegation token properly wh...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2527#discussion_r163456585
  
    --- Diff: external/storm-hbase/src/main/java/org/apache/storm/hbase/common/Utils.java ---
    @@ -47,14 +48,28 @@ public static HTable getTable(UserProvider provider, Configuration config, Strin
                 ugi = UserGroupInformation.getCurrentUser();
     
                 LOG.debug("UGI for current USER : {}", ugi.getUserName());
    +            boolean foundHBaseAuthToken = false;
                 for (Token<? extends TokenIdentifier> token : ugi.getTokens()) {
                     LOG.debug("Token in UGI (delegation token): {} / {}", token.toString(),
                             token.decodeIdentifier().getUser());
     
    -                // use UGI from token
    -                ugi = token.decodeIdentifier().getUser();
    -                ugi.addToken(token);
    +                // token.getKind() = Text, Text is annotated by @Stringable
    +                // which ensures toString() implementation
    +                if (token.getKind().toString().equals(TOKEN_KIND_HBASE_AUTH_TOKEN)) {
    +                    // use UGI from token
    +                    LOG.debug("Found HBASE_AUTH_TOKEN - using the token to replace current user.");
    +
    +                    ugi = token.decodeIdentifier().getUser();
    +                    ugi.addToken(token);
    +
    +                    foundHBaseAuthToken = true;
    --- End diff --
    
    Makes sense. I'll update the patch.


---

[GitHub] storm pull request #2527: STORM-2906 Pick HBase delegation token properly wh...

Posted by HeartSaVioR <gi...@git.apache.org>.
Github user HeartSaVioR commented on a diff in the pull request:

    https://github.com/apache/storm/pull/2527#discussion_r163216104
  
    --- Diff: external/storm-hbase/src/main/java/org/apache/storm/hbase/common/Utils.java ---
    @@ -47,14 +48,28 @@ public static HTable getTable(UserProvider provider, Configuration config, Strin
                 ugi = UserGroupInformation.getCurrentUser();
     
                 LOG.debug("UGI for current USER : {}", ugi.getUserName());
    +            boolean foundHBaseAuthToken = false;
                 for (Token<? extends TokenIdentifier> token : ugi.getTokens()) {
                     LOG.debug("Token in UGI (delegation token): {} / {}", token.toString(),
                             token.decodeIdentifier().getUser());
     
    -                // use UGI from token
    -                ugi = token.decodeIdentifier().getUser();
    -                ugi.addToken(token);
    +                // token.getKind() = Text, Text is annotated by @Stringable
    +                // which ensures toString() implementation
    +                if (token.getKind().toString().equals(TOKEN_KIND_HBASE_AUTH_TOKEN)) {
    +                    // use UGI from token
    +                    LOG.debug("Found HBASE_AUTH_TOKEN - using the token to replace current user.");
    +
    +                    ugi = token.decodeIdentifier().getUser();
    +                    ugi.addToken(token);
    +
    +                    foundHBaseAuthToken = true;
    --- End diff --
    
    I don't think if there's one HBase cluster needed to be connected in worker, but not sure if there're more than one HBase clusters. If we need to consider and handle multiple tokens, matching token would be completely different scenario.


---