[GitHub] nifi pull request: NIFI-1488 Refactoring HBase Kerberos support

2016-03-19 Thread markap14 
Github user markap14 commented on the pull request:

https://github.com/apache/nifi/pull/281#issuecomment-197888150
  
@bbende code looks good. Just a few minor comments in-line (stylistic for 
the most part). There were a lot of files that were changed due to just white 
space and Java imports being reorganized, which made the review a bit harder 
but otherwise, the code all looks good.


---
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] nifi pull request: NIFI-1488 Refactoring HBase Kerberos support

2016-03-19 Thread markap14 
Github user markap14 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/281#discussion_r56506078
  
--- Diff: 
nifi-nar-bundles/nifi-standard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/src/test/resources/hbase-site-security.xml
 ---
@@ -0,0 +1,30 @@
+
+
--- End diff --

I think we should keep the License header above the xml-stylesheet 
directive, no?


---
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] nifi pull request: NIFI-1488 Refactoring HBase Kerberos support

2016-03-19 Thread markap14 
Github user markap14 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/281#discussion_r56410644
  
--- Diff: 
nifi-commons/nifi-hadoop-utils/src/main/java/org/apache/nifi/hadoop/KerberosProperties.java
 ---
@@ -0,0 +1,146 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.hadoop;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.nifi.components.PropertyDescriptor;
+import org.apache.nifi.components.ValidationContext;
+import org.apache.nifi.components.ValidationResult;
+import org.apache.nifi.components.Validator;
+import org.apache.nifi.logging.ComponentLog;
+import org.apache.nifi.processor.util.StandardValidators;
+import org.apache.nifi.util.NiFiProperties;
+import org.apache.nifi.util.StringUtils;
+
+import java.io.File;
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * All processors and controller services that need properties for 
Kerberos Principal and Keytab
+ * should obtain them through this class by calling:
+ *
+ * KerberosProperties props = 
KerberosProperties.create(NiFiProperties.getInstance())
+ *
+ * The properties can be accessed from the resulting KerberosProperties 
instance.
+ */
+public class KerberosProperties {
+
+private final File kerberosConfigFile;
+private final Validator kerberosConfigValidator;
+private final PropertyDescriptor kerberosPrincipal;
+private final PropertyDescriptor kerberosKeytab;
+
+private KerberosProperties(final File kerberosConfigFile) {
+this.kerberosConfigFile = kerberosConfigFile;
+
+if (this.kerberosConfigFile != null) {
+System.setProperty("java.security.krb5.conf", 
kerberosConfigFile.getAbsolutePath());
+}
+
+this.kerberosConfigValidator = new Validator() {
+@Override
+public ValidationResult validate(String subject, String input, 
ValidationContext context) {
+// Check that the Kerberos configuration is set
+if (kerberosConfigFile == null) {
+return new ValidationResult.Builder()
+.subject(subject).input(input).valid(false)
+.explanation("you are missing the 
nifi.kerberos.krb5.file property which "
++ "must be set in order to use 
Kerberos")
+.build();
+}
+
+// Check that the Kerberos configuration is readable
+if (!kerberosConfigFile.canRead()) {
+return new 
ValidationResult.Builder().subject(subject).input(input).valid(false)
+.explanation(String.format("unable to read 
Kerberos config [%s], please make sure the path is valid "
++ "and nifi has adequate permissions", 
kerberosConfigFile.getAbsoluteFile()))
+.build();
+}
+
+return new 
ValidationResult.Builder().subject(subject).input(input).valid(true).build();
+}
+};
+
+this.kerberosPrincipal = new PropertyDescriptor.Builder()
+.name("Kerberos Principal")
+.required(false)
+.description("Kerberos principal to authenticate as. 
Requires nifi.kerberos.krb5.file to be set in your nifi.properties")
+.addValidator(kerberosConfigValidator)
+.build();
+
+this.kerberosKeytab = new PropertyDescriptor.Builder()
+.name("Kerberos Keytab").required(false)
+.description("Kerberos keytab associated with the 
principal. Requires nifi.kerberos.krb5.file to be set in your nifi.properties")
+.addValidator(StandardValidators.FILE_EXISTS_VALIDATOR)
+.addValidator(kerberosConfigValidator)

[GitHub] nifi pull request: NIFI-1488 Refactoring HBase Kerberos support

2016-03-19 Thread markap14 
Github user markap14 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/281#discussion_r56411887
  
--- Diff: 
nifi-commons/nifi-hadoop-utils/src/main/java/org/apache/nifi/hadoop/SecurityUtil.java
 ---
@@ -0,0 +1,113 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.hadoop;
+
+import org.apache.commons.lang3.Validate;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.nifi.logging.ComponentLog;
+
+import java.io.IOException;
+
+/**
+ * Provides synchronized access to UserGroupInformation to avoid multiple 
processors/services from
+ * interfering with each other.
+ */
+public class SecurityUtil {
+
+/**
+ * Initializes UserGroupInformation with the given Configuration and 
performs the login for the given principal
+ * and keytab. All logins should happen through this class to ensure 
other threads are not concurrently modifying
+ * UserGroupInformation.
+ *
+ * @param config the configuration instance
+ * @param principal the principal to authenticate as
+ * @param keyTab the keytab to authenticate with
+ *
+ * @return the UGI for the given principal
+ *
+ * @throws IOException if login failed
+ */
+public static synchronized UserGroupInformation loginKerberos(final 
Configuration config, final String principal, final String keyTab)
+throws IOException {
+Validate.notNull(config);
+Validate.notNull(principal);
+Validate.notNull(keyTab);
+
+UserGroupInformation.setConfiguration(config);
+return 
UserGroupInformation.loginUserFromKeytabAndReturnUGI(principal.trim(), 
keyTab.trim());
+}
+
+/**
+ * Initializes UserGroupInformation with the given Configuration and 
returns UserGroupInformation.getLoginUser().
+ * All logins should happen through this class to ensure other threads 
are not concurrently modifying
+ * UserGroupInformation.
+ *
+ * @param config the configuration instance
+ *
+ * @return the UGI for the given principal
+ *
+ * @throws IOException if login failed
+ */
+public static synchronized UserGroupInformation loginSimple(final 
Configuration config) throws IOException {
+Validate.notNull(config);
+UserGroupInformation.setConfiguration(config);
+return UserGroupInformation.getLoginUser();
+}
+
+/**
+ * Initializes UserGroupInformation with the given Configuration and 
returns UserGroupInformation.isSecurityEnabled().
+ *
+ * All checks for isSecurityEnabled() should happen through this 
method.
+ *
+ * @param config the given configuration
+ *
+ * @return true if kerberos is enabled on the given configuration, 
false otherwise
+ *
+ */
+public static synchronized boolean isSecurityEnabled(final 
Configuration config) {
+Validate.notNull(config);
+return 
"kerberos".equalsIgnoreCase(config.get("hadoop.security.authentication"));
+}
+
+/**
+ * Start a thread that periodically attempts to renew the current 
Kerberos user's ticket.
+ *
+ * Callers of this method should store the reference to the 
KerberosTicketRenewer and call stop() to stop the thread.
+ *
+ * @param clazz
+ *  The class that this renewal is for (i.e. PutHDFS, etc)
+ * @param ugi
+ *  The current Kerberos user.
+ * @param renewalPeriod
+ *  The amount of time between attempting renewals.
+ * @param logger
+ *  The logger to use with in the renewer
+ *
+ * @return the KerberosTicketRenewer Runnable
+ */
+public static KerberosTicketRenewer startTicketRenewalThread(final 
Class clazz, final

[GitHub] nifi pull request: NIFI-1488 Refactoring HBase Kerberos support

2016-03-19 Thread markap14 
Github user markap14 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/281#discussion_r56410960
  
--- Diff: 
nifi-commons/nifi-hadoop-utils/src/main/java/org/apache/nifi/hadoop/KerberosTicketRenewer.java
 ---
@@ -0,0 +1,69 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.nifi.hadoop;
+
+import org.apache.hadoop.security.UserGroupInformation;
+import org.apache.nifi.logging.ComponentLog;
+
+import java.io.IOException;
+
+/**
+ * Periodically attempts to renew the Kerberos user's ticket for the given 
UGI.
+ */
+public class KerberosTicketRenewer implements Runnable {
+
+private final UserGroupInformation ugi;
+private final long renewalPeriod;
+private final ComponentLog logger;
+
+private volatile boolean stopped = false;
+
+public KerberosTicketRenewer(final UserGroupInformation ugi, final 
long renewalPeriod, final ComponentLog logger) {
+this.ugi = ugi;
+this.renewalPeriod = renewalPeriod;
+this.logger = logger;
+}
+
+@Override
+public void run() {
+stopped = false;
+while (!stopped) {
+try {
+logger.debug("Invoking renewal attempt for Kerberos 
ticket");
+// While we run this "frequently", the Hadoop 
implementation will only perform the login at 80% of ticket lifetime.
+ugi.checkTGTAndReloginFromKeytab();
+} catch (IOException e) {
+// Should failures to renew the ticket be retried more 
quickly?
--- End diff --

I definitely think they should. I believe 24 hours is a fairly common 
renewal period, but we don't want to wait 24 hours on failure. Perhaps provide 
a new constructor arg for retryPeriod?


---
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] nifi pull request: NIFI-1488 Refactoring HBase Kerberos support

2016-03-18 Thread asfgit 
Github user asfgit closed the pull request at:

https://github.com/apache/nifi/pull/281


---
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] nifi pull request: NIFI-1488 Refactoring HBase Kerberos support

2016-03-18 Thread markap14 
Github user markap14 commented on a diff in the pull request:

https://github.com/apache/nifi/pull/281#discussion_r56505883
  
--- Diff: 
nifi-nar-bundles/nifi-hadoop-bundle/nifi-hdfs-processors/src/test/resources/core-site.xml
 ---
@@ -1,4 +1,5 @@
 
+
--- End diff --

Any reason this was moved above the license header? I think the license 
header should be first.


---
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] nifi pull request: NIFI-1488 Refactoring HBase Kerberos support

2016-03-15 Thread bbende 
GitHub user bbende opened a pull request:

https://github.com/apache/nifi/pull/281

NIFI-1488 Refactoring HBase Kerberos support

- Storing UGI so we can support multiple HBaseClientServices with different 
configs
- Creating nifi-hadoop-utils to hold utility code shared between HDFS and 
HBase processors
- Incorporating KerberosProperties into existing hadoop processors

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

$ git pull https://github.com/bbende/nifi NIFI-1488

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

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


commit 42766156c67542dc02d9ea9d8418c7f86bd7
Author: Bryan Bende 
Date:   2016-03-15T18:58:03Z

NIFI-1488 Refactoring HBase Kerberos support
- Storing UGI so we can support multiple HBaseClientServices with different 
configs
- Creating nifi-hadoop-utils to hold utility code shared between HDFS and 
HBase processors
- Incorporating KerberosProperties into existing hadoop processors




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