[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-16 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/flink/pull/1342


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-16 Thread mxm
Github user mxm commented on the pull request:

https://github.com/apache/flink/pull/1342#issuecomment-156965181
  
Thanks for testing this so thoroughly. +1 to merge.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-16 Thread nielsbasjes
Github user nielsbasjes commented on the pull request:

https://github.com/apache/flink/pull/1342#issuecomment-156952942
  
FYI: I let my test run over the weekend (i.e. for 65 hours) with the 5 & 10 
minutes ticket times and it is still running fine.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-13 Thread rmetzger
Github user rmetzger commented on a diff in the pull request:

https://github.com/apache/flink/pull/1342#discussion_r44764775
  
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/Utils.java ---
@@ -135,7 +138,54 @@ public static void setTokensFor(ContainerLaunchContext 
amContainer, Path[] paths
ByteBuffer securityTokens = ByteBuffer.wrap(dob.getData(), 0, 
dob.getLength());
amContainer.setTokens(securityTokens);
}
-   
+
+   /**
+* Obtain Kerberos security token for HBase.
+*/
+   private static void obtainTokenForHBase(Credentials credentials, 
Configuration conf) throws IOException {
+   if (UserGroupInformation.isSecurityEnabled()) {
+   LOG.info("Attempting to obtain Kerberos security token 
for HBase");
+   try {
+   // 
+   // Intended call: 
HBaseConfiguration.addHbaseResources(conf);
+   Class
+   
.forName("org.apache.hadoop.hbase.HBaseConfiguration")
+   .getMethod("addHbaseResources", 
Configuration.class )
+   .invoke(null, conf);
+   // 
+
+   LOG.info("HBase security setting: {}", 
conf.get("hbase.security.authentication"));
+
+   if 
(!"kerberos".equals(conf.get("hbase.security.authentication"))) {
+   LOG.info("HBase has not been configured 
to use Kerberos.");
+   return;
+   }
+
+   LOG.info("Obtaining Kerberos security token for 
HBase");
+   // 
+   // Intended call: 
Token token = TokenUtil.obtainToken(conf);
+   Token token = (Token) Class
+   
.forName("org.apache.hadoop.hbase.security.token.TokenUtil")
+   .getMethod("obtainToken", 
Configuration.class)
+   .invoke(null, conf);
+   // 
+
+   if (token == null) {
+   LOG.error("No Kerberos security token 
for HBase available");
+   return;
+   }
+
+   credentials.addToken(token.getService(), token);
+   LOG.info("Added HBase Kerberos security token 
to credentials.");
+   } catch ( ClassNotFoundException
+   | NoSuchMethodException
+   | IllegalAccessException
+   | InvocationTargetException e) {
+   LOG.info("HBase is not available (not packaged 
with this application).");
--- End diff --

Maybe it makes sense to add the exception to the log message (or even the 
full stack trace), so that users have a better understanding what exactly went 
wrong (for example hbase version mismatch)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-13 Thread rmetzger
Github user rmetzger commented on a diff in the pull request:

https://github.com/apache/flink/pull/1342#discussion_r44764797
  
--- Diff: pom.xml ---
@@ -82,6 +82,7 @@ under the License.

error
1.2.1
2.3.0
+   1.1.2
--- End diff --

I think you can remove this version definition again.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-13 Thread rmetzger
Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/1342#issuecomment-156379250
  
I had some minor remarks to the PR, but overall, I'd like to merge it like 
this!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-13 Thread rmetzger
Github user rmetzger commented on a diff in the pull request:

https://github.com/apache/flink/pull/1342#discussion_r44764893
  
--- Diff: flink-dist/src/main/flink-bin/bin/config.sh ---
@@ -249,7 +249,15 @@ if [ -n "$HADOOP_HOME" ]; then
 fi
 fi
 

-INTERNAL_HADOOP_CLASSPATHS="$HADOOP_CLASSPATH:$HADOOP_CONF_DIR:$YARN_CONF_DIR"

+INTERNAL_HADOOP_CLASSPATHS="${HADOOP_CLASSPATH}:${HADOOP_CONF_DIR}:${YARN_CONF_DIR}"
+
+if [ -n "${HBASE_CONF_DIR}" ]; then
+# Setup the HBase classpath.
+INTERNAL_HADOOP_CLASSPATHS="${INTERNAL_HADOOP_CLASSPATHS}:`hbase 
classpath`"
--- End diff --

what happens if the call to `hbase` fails, for example because its not in 
the `$PATH` ?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-13 Thread nielsbasjes
Github user nielsbasjes commented on a diff in the pull request:

https://github.com/apache/flink/pull/1342#discussion_r44765814
  
--- Diff: flink-dist/src/main/flink-bin/bin/config.sh ---
@@ -249,7 +249,15 @@ if [ -n "$HADOOP_HOME" ]; then
 fi
 fi
 

-INTERNAL_HADOOP_CLASSPATHS="$HADOOP_CLASSPATH:$HADOOP_CONF_DIR:$YARN_CONF_DIR"

+INTERNAL_HADOOP_CLASSPATHS="${HADOOP_CLASSPATH}:${HADOOP_CONF_DIR}:${YARN_CONF_DIR}"
+
+if [ -n "${HBASE_CONF_DIR}" ]; then
+# Setup the HBase classpath.
+INTERNAL_HADOOP_CLASSPATHS="${INTERNAL_HADOOP_CLASSPATHS}:`hbase 
classpath`"
--- End diff --

1) You see an error during the startup of your flink job
2) An empty value is inserted in the path causing your homedirectory to be 
in the classpath.
3) HBase support doesn't work.

A quick test on the commandline:
```
$ INTERNAL_HADOOP_CLASSPATHS=foo:bar
$ INTERNAL_HADOOP_CLASSPATHS="${INTERNAL_HADOOP_CLASSPATHS}:`hbasexx 
classpath`"
bash: hbasexx: command not found...
$ echo $INTERNAL_HADOOP_CLASSPATHS 
foo:bar:
```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-13 Thread nielsbasjes
Github user nielsbasjes commented on a diff in the pull request:

https://github.com/apache/flink/pull/1342#discussion_r44766715
  
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/Utils.java ---
@@ -135,7 +138,54 @@ public static void setTokensFor(ContainerLaunchContext 
amContainer, Path[] paths
ByteBuffer securityTokens = ByteBuffer.wrap(dob.getData(), 0, 
dob.getLength());
amContainer.setTokens(securityTokens);
}
-   
+
+   /**
+* Obtain Kerberos security token for HBase.
+*/
+   private static void obtainTokenForHBase(Credentials credentials, 
Configuration conf) throws IOException {
+   if (UserGroupInformation.isSecurityEnabled()) {
+   LOG.info("Attempting to obtain Kerberos security token 
for HBase");
+   try {
+   // 
+   // Intended call: 
HBaseConfiguration.addHbaseResources(conf);
+   Class
+   
.forName("org.apache.hadoop.hbase.HBaseConfiguration")
+   .getMethod("addHbaseResources", 
Configuration.class )
+   .invoke(null, conf);
+   // 
+
+   LOG.info("HBase security setting: {}", 
conf.get("hbase.security.authentication"));
+
+   if 
(!"kerberos".equals(conf.get("hbase.security.authentication"))) {
+   LOG.info("HBase has not been configured 
to use Kerberos.");
+   return;
+   }
+
+   LOG.info("Obtaining Kerberos security token for 
HBase");
+   // 
+   // Intended call: 
Token token = TokenUtil.obtainToken(conf);
+   Token token = (Token) Class
+   
.forName("org.apache.hadoop.hbase.security.token.TokenUtil")
+   .getMethod("obtainToken", 
Configuration.class)
+   .invoke(null, conf);
+   // 
+
+   if (token == null) {
+   LOG.error("No Kerberos security token 
for HBase available");
+   return;
+   }
+
+   credentials.addToken(token.getService(), token);
+   LOG.info("Added HBase Kerberos security token 
to credentials.");
+   } catch ( ClassNotFoundException
+   | NoSuchMethodException
+   | IllegalAccessException
+   | InvocationTargetException e) {
+   LOG.info("HBase is not available (not packaged 
with this application).");
--- End diff --

Now the error looks like this (not the full stacktrace, yet enough to 
understand what happened):
```
11:01:49,970 INFO  org.apache.flink.yarn.Utils  - HBase is not available 
(not packaged with this application): ClassNotFoundException : 
"org.apache.hadoop.hbase.HBaseConfiguration".
```


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-13 Thread rmetzger
Github user rmetzger commented on a diff in the pull request:

https://github.com/apache/flink/pull/1342#discussion_r44767927
  
--- Diff: flink-dist/src/main/flink-bin/bin/config.sh ---
@@ -249,7 +249,15 @@ if [ -n "$HADOOP_HOME" ]; then
 fi
 fi
 

-INTERNAL_HADOOP_CLASSPATHS="$HADOOP_CLASSPATH:$HADOOP_CONF_DIR:$YARN_CONF_DIR"

+INTERNAL_HADOOP_CLASSPATHS="${HADOOP_CLASSPATH}:${HADOOP_CONF_DIR}:${YARN_CONF_DIR}"
+
+if [ -n "${HBASE_CONF_DIR}" ]; then
+# Setup the HBase classpath.
+INTERNAL_HADOOP_CLASSPATHS="${INTERNAL_HADOOP_CLASSPATHS}:`hbase 
classpath`"
--- End diff --

Okay great, as long as it is not stopping the bash script, its fine.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-13 Thread mxm
Github user mxm commented on the pull request:

https://github.com/apache/flink/pull/1342#issuecomment-156387638
  
+1 thanks for improving it!


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-13 Thread rmetzger
Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/1342#issuecomment-156388624
  
Thank you for addressing my concerns so quickly
+1 to merge.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-13 Thread rmetzger
Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/1342#issuecomment-156423600
  
Amazing, thank you.
I'm going to merge you change as soon as Travis gives me green light ;)
I've squashed and rebased your commit and will push it probably as 
https://github.com/rmetzger/flink/commit/55248ec26337797d56bc8bddd5f62c4db0ea195c


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-12 Thread mxm
Github user mxm commented on the pull request:

https://github.com/apache/flink/pull/1342#issuecomment-156061452
  
Dynamic class loading should work then. Would be great if you could 
integrate it @nielsbasjes. We understand you're not an average user since 
you're an open source software developer :)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-12 Thread nielsbasjes
Github user nielsbasjes commented on the pull request:

https://github.com/apache/flink/pull/1342#issuecomment-156117069
  
I made a mistake that the dependency was still in there 'hard'.
When I switch it to 'provided' or leave it out it fails with a 
ClassNotFound ... 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-12 Thread nielsbasjes
Github user nielsbasjes commented on the pull request:

https://github.com/apache/flink/pull/1342#issuecomment-156122982
  
Found it and fixed it.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-12 Thread nielsbasjes
Github user nielsbasjes commented on the pull request:

https://github.com/apache/flink/pull/1342#issuecomment-156116370
  
I fixed the authentication to use reflection.
To make this work I had to switch back to an older (deprecated) version of 
the token retrieval API because otherwise it wouldn't work (instead of a 
"Connection" parameter now use a "Configuration" parameter).

I had our IT guys drop the ticket expire time for my user account down to 5 
minutes. 
Given that my topology has been running for more than 22 minutes now so the 
solution still works.

Please review.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-11 Thread mxm
Github user mxm commented on the pull request:

https://github.com/apache/flink/pull/1342#issuecomment-155719584
  
Thank you for the testing and the patch. I would like to merge this soon.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-11 Thread nielsbasjes
Github user nielsbasjes commented on the pull request:

https://github.com/apache/flink/pull/1342#issuecomment-155708227
  
I created a test topology that puts the current time in an HBase cell 
several times per second.
In the cluster I did this on HBase is configured to use Kerberos and the 
Kerberos tickets expire after 10 hours.
I let this running for 22:15 (so more than twice the expire time) and it 
continues to successfully update the column in HBase.

So from my standpoint this patch is ready to be committed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-11 Thread rmetzger
Github user rmetzger commented on a diff in the pull request:

https://github.com/apache/flink/pull/1342#discussion_r44527912
  
--- Diff: flink-yarn/pom.xml ---
@@ -63,6 +63,11 @@ under the License.
test

 
+   
+   org.apache.hbase
+   hbase-client
+   ${hbase.version}
+   
--- End diff --

This change is adding hbase as a dependency into Flink's fat jar (binary 
distribution).
The `flink-hbase` module is currently an optional module.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-11 Thread rmetzger
Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/1342#issuecomment-155768292
  
Thanks a lot for working on this issue! It doesn't happen everyday that 
users identify issues, fix them and verify the fix so properly :)

I would really like to avoid pulling hbase as dependency into Flink's 
binary distribution. Can you change to "obtain token" part to do everything by 
loading the classes dynamically and calling the methods via reflection? 
(Similar to Spark's implementation: 
https://github.com/apache/spark/pull/5586/files)
This way, we only execute this if the hbase classes are in the classpath.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-11 Thread rmetzger
Github user rmetzger commented on the pull request:

https://github.com/apache/flink/pull/1342#issuecomment-155764769
  
The tests on travis are failing


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-11 Thread StephanEwen
Github user StephanEwen commented on the pull request:

https://github.com/apache/flink/pull/1342#issuecomment-155769960
  
Robert has a good point. Right now, if some one uses HBase, the HBase 
dependency is part of the user program JAR. It would be nice to keep HBase out 
of the core JAR - it would be yet another fat dependency with multiple 
transitive dependencies.

Once you change the code to reflect-load the HBase classes, one gets the 
Kerberos/HBase support as soon as one drops the HBase jars JAR into Flink's lib 
folder, or adds them to the Hadoop Classpath env variable. At the same time, 
non-HBase users retain a thinner set of dependencies (and possible conflicts).

@nielsbasjes What do you think about that approach?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-11 Thread mxm
Github user mxm commented on the pull request:

https://github.com/apache/flink/pull/1342#issuecomment-155770210
  
I was thinking about something like a reflection-based mechanism. Not sure 
though if this will work across different versions. Probably the API didn't 
change much in this regard.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-10 Thread mxm
Github user mxm commented on the pull request:

https://github.com/apache/flink/pull/1342#issuecomment-155380888
  
Thanks for the pull request. Looks good to me except for one comment. I 
would like to postpone merging until you have verified the changes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-10 Thread mxm
Github user mxm commented on a diff in the pull request:

https://github.com/apache/flink/pull/1342#discussion_r44390519
  
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/Utils.java ---
@@ -135,7 +142,40 @@ public static void setTokensFor(ContainerLaunchContext 
amContainer, Path[] paths
ByteBuffer securityTokens = ByteBuffer.wrap(dob.getData(), 0, 
dob.getLength());
amContainer.setTokens(securityTokens);
}
-   
+
+   /**
+* Obtain Kerberos security token for HBase.
+*/
+   private static void obtainTokenForHBase(Credentials credentials, 
Configuration conf) {
+   if (UserGroupInformation.isSecurityEnabled()) {
+   LOG.info("Attempting to obtain Kerberos security token 
for HBase");
+   try {
+   HBaseConfiguration.addHbaseResources(conf);
+   if 
(!"kerberos".equals(conf.get("hbase.security.authentication"))) {
+   LOG.info("HBase has not been configured 
to use Kerberos.");
+   return;
+   }
+
+   LOG.info("Connecting to HBase");
+   Connection connection = 
ConnectionFactory.createConnection(conf);
+
+   LOG.info("Obtaining Kerberos security token for 
HBase");
+   Token token = 
TokenUtil.obtainToken(connection);
+
+   if (token == null) {
+   LOG.error("No Kerberos security token 
for HBase available");
+   return;
+   }
+
+   credentials.addToken(token.getService(), token);
+   LOG.info("Added HBase Kerberos security token 
to credentials.");
+   } catch (IOException e) {
+   LOG.error("Caught exception while trying to 
obtain HBase Kerberos security token.");
+   e.printStackTrace();
--- End diff --

You might want to re`throw` the Exception here or not catch it at all. This 
ensures that it is probably forwarded to the client.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-10 Thread nielsbasjes
Github user nielsbasjes commented on the pull request:

https://github.com/apache/flink/pull/1342#issuecomment-155376887
  
I ran this version over night but the VPN from my system to the cluster 
stopped before the Kerberos ticket could expire.
I am quite confident that this patch is right though.
I'll start a new test on my end. With this pull request you guys can verify 
my patch and give me feedback on any improvements you see.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-10 Thread nielsbasjes
GitHub user nielsbasjes opened a pull request:

https://github.com/apache/flink/pull/1342

[FLINK-2977] Added support for accessing a Kerberos secured HBase 
installation.

See https://issues.apache.org/jira/browse/FLINK-2977

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

$ git pull https://github.com/nielsbasjes/flink master

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

https://github.com/apache/flink/pull/1342.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 #1342


commit 81c0180f74dd8711d6cf8a61ecfb4fb5c0d187d1
Author: Niels Basjes 
Date:   2015-11-10T10:04:00Z

[FLINK-2977] Added support for accessing a Kerberos secured HBase 
installation.




---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---


[GitHub] flink pull request: [FLINK-2977] Added support for accessing a Ker...

2015-11-10 Thread nielsbasjes
Github user nielsbasjes commented on a diff in the pull request:

https://github.com/apache/flink/pull/1342#discussion_r44391146
  
--- Diff: flink-yarn/src/main/java/org/apache/flink/yarn/Utils.java ---
@@ -135,7 +142,40 @@ public static void setTokensFor(ContainerLaunchContext 
amContainer, Path[] paths
ByteBuffer securityTokens = ByteBuffer.wrap(dob.getData(), 0, 
dob.getLength());
amContainer.setTokens(securityTokens);
}
-   
+
+   /**
+* Obtain Kerberos security token for HBase.
+*/
+   private static void obtainTokenForHBase(Credentials credentials, 
Configuration conf) {
+   if (UserGroupInformation.isSecurityEnabled()) {
+   LOG.info("Attempting to obtain Kerberos security token 
for HBase");
+   try {
+   HBaseConfiguration.addHbaseResources(conf);
+   if 
(!"kerberos".equals(conf.get("hbase.security.authentication"))) {
+   LOG.info("HBase has not been configured 
to use Kerberos.");
+   return;
+   }
+
+   LOG.info("Connecting to HBase");
+   Connection connection = 
ConnectionFactory.createConnection(conf);
+
+   LOG.info("Obtaining Kerberos security token for 
HBase");
+   Token token = 
TokenUtil.obtainToken(connection);
+
+   if (token == null) {
+   LOG.error("No Kerberos security token 
for HBase available");
+   return;
+   }
+
+   credentials.addToken(token.getService(), token);
+   LOG.info("Added HBase Kerberos security token 
to credentials.");
+   } catch (IOException e) {
+   LOG.error("Caught exception while trying to 
obtain HBase Kerberos security token.");
+   e.printStackTrace();
--- End diff --

Yes I considered that. I figured that continuing would be 'better'. But 
simply rethrowing would make more sense.
I'll update the patch.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---