You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@nifi.apache.org by GitBox <gi...@apache.org> on 2020/02/28 21:12:44 UTC

[GitHub] [nifi] jtstorck opened a new pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

jtstorck opened a new pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102
 
 
   Thank you for submitting a contribution to Apache NiFi.
   
   Please provide a short description of the PR here:
   
   #### Description of PR
   
   _Enables X functionality; fixes bug NIFI-YYYY._
   
   In order to streamline the review of the contribution we ask you
   to ensure the following steps have been taken:
   
   ### For all changes:
   - [ ] Is there a JIRA ticket associated with this PR? Is it referenced 
        in the commit message?
   
   - [ ] Does your PR title start with **NIFI-XXXX** where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
   
   - [ ] Has your PR been rebased against the latest commit within the target branch (typically `master`)?
   
   - [ ] Is your initial contribution a single, squashed commit? _Additional commits in response to PR reviewer feedback should be made on this branch and pushed to allow change tracking. Do not `squash` or use `--force` when pushing to allow for clean monitoring of changes._
   
   ### For code changes:
   - [ ] Have you ensured that the full suite of tests is executed via `mvn -Pcontrib-check clean install` at the root `nifi` folder?
   - [ ] Have you written or updated unit tests to verify your changes?
   - [ ] Have you verified that the full build is successful on both JDK 8 and JDK 11?
   - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)? 
   - [ ] If applicable, have you updated the `LICENSE` file, including the main `LICENSE` file under `nifi-assembly`?
   - [ ] If applicable, have you updated the `NOTICE` file, including the main `NOTICE` file found under `nifi-assembly`?
   - [ ] If adding new Properties, have you added `.displayName` in addition to .name (programmatic access) for each of the new properties?
   
   ### For documentation related changes:
   - [ ] Have you ensured that format looks appropriate for the output in which it is rendered?
   
   ### Note:
   Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] jtstorck commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

Posted by GitBox <gi...@apache.org>.
jtstorck commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102#discussion_r386492592
 
 

 ##########
 File path: nifi-nar-bundles/nifi-hive-bundle/nifi-hive3-processors/src/main/java/org/apache/nifi/dbcp/hive/Hive3ConnectionPool.java
 ##########
 @@ -306,38 +313,38 @@ protected void init(final ControllerServiceInitializationContext context) {
         if (confFileProvided) {
             final String explicitPrincipal = validationContext.getProperty(kerberosProperties.getKerberosPrincipal()).evaluateAttributeExpressions().getValue();
             final String explicitKeytab = validationContext.getProperty(kerberosProperties.getKerberosKeytab()).evaluateAttributeExpressions().getValue();
+            final String explicitPassword = validationContext.getProperty(kerberosProperties.getKerberosPassword()).getValue();
             final KerberosCredentialsService credentialsService = validationContext.getProperty(KERBEROS_CREDENTIALS_SERVICE).asControllerService(KerberosCredentialsService.class);
 
             final String resolvedPrincipal;
             final String resolvedKeytab;
-            if (credentialsService == null) {
-                resolvedPrincipal = explicitPrincipal;
-                resolvedKeytab = explicitKeytab;
-            } else {
+            if (credentialsService != null) {
                 resolvedPrincipal = credentialsService.getPrincipal();
                 resolvedKeytab = credentialsService.getKeytab();
+            } else {
+                resolvedPrincipal = explicitPrincipal;
+                resolvedKeytab = explicitKeytab;
             }
 
 
             final String configFiles = validationContext.getProperty(HIVE_CONFIGURATION_RESOURCES).evaluateAttributeExpressions().getValue();
-            problems.addAll(hiveConfigurator.validate(configFiles, resolvedPrincipal, resolvedKeytab, validationResourceHolder, getLogger()));
+            problems.addAll(hiveConfigurator.validate(configFiles, resolvedPrincipal, resolvedKeytab, explicitPassword, validationResourceHolder, getLogger()));
 
-            if (credentialsService != null && (explicitPrincipal != null || explicitKeytab != null)) {
+            if (credentialsService != null && (explicitPrincipal != null || explicitKeytab != null || explicitPassword != null)) {
                 problems.add(new ValidationResult.Builder()
-                        .subject("Kerberos Credentials")
-                        .valid(false)
-                        .explanation("Cannot specify both a Kerberos Credentials Service and a principal/keytab")
-                        .build());
+                    .subject("Kerberos Credentials")
+                    .valid(false)
+                    .explanation("Cannot specify a Kerberos Credentials Service while also specifying a Kerberos Principal, Kerberos Keytab, or Kerberos Password")
+                    .build());
             }
 
-            final String allowExplicitKeytabVariable = System.getenv(ALLOW_EXPLICIT_KEYTAB);
-            if ("false".equalsIgnoreCase(allowExplicitKeytabVariable) && (explicitPrincipal != null || explicitKeytab != null)) {
+            if (isAllowExplicitKeytab() && explicitKeytab != null) {
 
 Review comment:
   Fixed.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] asfgit closed pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102
 
 
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] bbende commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

Posted by GitBox <gi...@apache.org>.
bbende commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102#discussion_r386452430
 
 

 ##########
 File path: nifi-nar-bundles/nifi-hive-bundle/nifi-hive3-processors/src/main/java/org/apache/nifi/dbcp/hive/Hive3ConnectionPool.java
 ##########
 @@ -306,38 +313,38 @@ protected void init(final ControllerServiceInitializationContext context) {
         if (confFileProvided) {
             final String explicitPrincipal = validationContext.getProperty(kerberosProperties.getKerberosPrincipal()).evaluateAttributeExpressions().getValue();
             final String explicitKeytab = validationContext.getProperty(kerberosProperties.getKerberosKeytab()).evaluateAttributeExpressions().getValue();
+            final String explicitPassword = validationContext.getProperty(kerberosProperties.getKerberosPassword()).getValue();
             final KerberosCredentialsService credentialsService = validationContext.getProperty(KERBEROS_CREDENTIALS_SERVICE).asControllerService(KerberosCredentialsService.class);
 
             final String resolvedPrincipal;
             final String resolvedKeytab;
-            if (credentialsService == null) {
-                resolvedPrincipal = explicitPrincipal;
-                resolvedKeytab = explicitKeytab;
-            } else {
+            if (credentialsService != null) {
                 resolvedPrincipal = credentialsService.getPrincipal();
                 resolvedKeytab = credentialsService.getKeytab();
+            } else {
+                resolvedPrincipal = explicitPrincipal;
+                resolvedKeytab = explicitKeytab;
             }
 
 
             final String configFiles = validationContext.getProperty(HIVE_CONFIGURATION_RESOURCES).evaluateAttributeExpressions().getValue();
-            problems.addAll(hiveConfigurator.validate(configFiles, resolvedPrincipal, resolvedKeytab, validationResourceHolder, getLogger()));
+            problems.addAll(hiveConfigurator.validate(configFiles, resolvedPrincipal, resolvedKeytab, explicitPassword, validationResourceHolder, getLogger()));
 
-            if (credentialsService != null && (explicitPrincipal != null || explicitKeytab != null)) {
+            if (credentialsService != null && (explicitPrincipal != null || explicitKeytab != null || explicitPassword != null)) {
                 problems.add(new ValidationResult.Builder()
-                        .subject("Kerberos Credentials")
-                        .valid(false)
-                        .explanation("Cannot specify both a Kerberos Credentials Service and a principal/keytab")
-                        .build());
+                    .subject("Kerberos Credentials")
+                    .valid(false)
+                    .explanation("Cannot specify a Kerberos Credentials Service while also specifying a Kerberos Principal, Kerberos Keytab, or Kerberos Password")
+                    .build());
             }
 
-            final String allowExplicitKeytabVariable = System.getenv(ALLOW_EXPLICIT_KEYTAB);
-            if ("false".equalsIgnoreCase(allowExplicitKeytabVariable) && (explicitPrincipal != null || explicitKeytab != null)) {
+            if (isAllowExplicitKeytab() && explicitKeytab != null) {
 
 Review comment:
   Should be `!isAllowExplicitKeytab()` ?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] bbende commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

Posted by GitBox <gi...@apache.org>.
bbende commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102#discussion_r385928708
 
 

 ##########
 File path: nifi-nar-bundles/nifi-extension-utils/nifi-hadoop-utils/src/main/java/org/apache/nifi/processors/hadoop/AbstractHadoopProcessor.java
 ##########
 @@ -563,8 +558,8 @@ protected UserGroupInformation getUserGroupInformation() {
     /*
      * Overridable by subclasses in the same package, mainly intended for testing purposes to allow verification without having to set environment variables.
      */
-    String getAllowExplicitKeytabEnvironmentVariable() {
-        return System.getenv(ALLOW_EXPLICIT_KEYTAB);
+    boolean isAllowExplicitKeytab() {
+        return Boolean.parseBoolean(System.getenv(ALLOW_EXPLICIT_KEYTAB));
 
 Review comment:
   Does this throw an exception if its null or something besides true/false?
   
   Wondering if it should be `"true".equals(System.getenv(ALLOW_EXPLICIT_KEYTAB))`

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] jtstorck commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

Posted by GitBox <gi...@apache.org>.
jtstorck commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102#discussion_r386492403
 
 

 ##########
 File path: nifi-nar-bundles/nifi-hive-bundle/nifi-hive-processors/src/main/java/org/apache/nifi/processors/hive/PutHiveStreaming.java
 ##########
 @@ -396,23 +403,22 @@ protected void init(ProcessorInitializationContext context) {
 
 
             final String configFiles = validationContext.getProperty(HIVE_CONFIGURATION_RESOURCES).evaluateAttributeExpressions().getValue();
-            problems.addAll(hiveConfigurator.validate(configFiles, resolvedPrincipal, resolvedKeytab, validationResourceHolder, getLogger()));
+            problems.addAll(hiveConfigurator.validate(configFiles, resolvedPrincipal, resolvedKeytab, explicitPassword, validationResourceHolder, getLogger()));
 
             if (credentialsService != null && (explicitPrincipal != null || explicitKeytab != null)) {
                 problems.add(new ValidationResult.Builder()
                     .subject("Kerberos Credentials")
                     .valid(false)
-                    .explanation("Cannot specify both a Kerberos Credentials Service and a principal/keytab")
+                        .explanation("Cannot specify a Kerberos Credentials Service while also specifying a Kerberos Principal, Kerberos Keytab, or Kerberos Password")
                     .build());
             }
 
-            final String allowExplicitKeytabVariable = System.getenv(ALLOW_EXPLICIT_KEYTAB);
-            if ("false".equalsIgnoreCase(allowExplicitKeytabVariable) && (explicitPrincipal != null || explicitKeytab != null)) {
+            if (!isAllowExplicitKeytab() && (explicitPrincipal != null || explicitKeytab != null)) {
 
 Review comment:
   Fixed.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] jtstorck commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

Posted by GitBox <gi...@apache.org>.
jtstorck commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102#discussion_r385938030
 
 

 ##########
 File path: nifi-nar-bundles/nifi-hive-bundle/nifi-hive-processors/src/main/java/org/apache/nifi/dbcp/hive/HiveConnectionPool.java
 ##########
 @@ -363,6 +379,20 @@ public Connection getConnection() throws ProcessException {
                      * the acquisition of a new TGT after the current one has expired.
                      * https://issues.apache.org/jira/browse/NIFI-5134
                      */
+                    getLogger().trace("getting UGI instance");
+                    if (kerberosUserReference.get() != null) {
+                        // if there's a KerberosUser associated with this UGI, check the TGT and relogin if it is close to expiring
+                        KerberosUser kerberosUser = kerberosUserReference.get();
+                        getLogger().debug("kerberosUser is " + kerberosUser);
+                        try {
+                            getLogger().debug("checking TGT on kerberosUser [{}]", new Object[] {kerberosUser});
+                            kerberosUser.checkTGTAndRelogin();
+                        } catch (LoginException e) {
+                            throw new ProcessException("Unable to relogin with kerberos credentials for " + kerberosUser.getPrincipal(), e);
+                        }
+                    } else {
+                        getLogger().debug("kerberosUser was null, will not refresh TGT with KerberosUser");
+                    }
                     ugi.checkTGTAndReloginFromKeytab();
 
 Review comment:
   I moved `ugi.checkTGTAndReloginFromKeytab` into the else block, since HiveConnectionPool historically has had to explicitly request the relogin due to the Thrift client code not handling that case implicitly.  The call itself probably isn't needed, but based on the if/else statement, I think it might be safer to keep it inside the else block rather than remove it completely.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] jtstorck commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

Posted by GitBox <gi...@apache.org>.
jtstorck commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102#discussion_r386492481
 
 

 ##########
 File path: nifi-nar-bundles/nifi-hive-bundle/nifi-hive3-processors/src/main/java/org/apache/nifi/dbcp/hive/Hive3ConnectionPool.java
 ##########
 @@ -289,6 +295,7 @@ protected void init(final ControllerServiceInitializationContext context) {
         kerberosProperties = new KerberosProperties(kerberosConfigFile);
         props.add(kerberosProperties.getKerberosPrincipal());
         props.add(kerberosProperties.getKerberosKeytab());
+        props.add(kerberosProperties.getKerberosPrincipal());
 
 Review comment:
   Fixed.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] jtstorck commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

Posted by GitBox <gi...@apache.org>.
jtstorck commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102#discussion_r386493364
 
 

 ##########
 File path: nifi-nar-bundles/nifi-hive-bundle/nifi-hive3-processors/src/main/java/org/apache/nifi/dbcp/hive/Hive3ConnectionPool.java
 ##########
 @@ -306,38 +313,38 @@ protected void init(final ControllerServiceInitializationContext context) {
         if (confFileProvided) {
             final String explicitPrincipal = validationContext.getProperty(kerberosProperties.getKerberosPrincipal()).evaluateAttributeExpressions().getValue();
             final String explicitKeytab = validationContext.getProperty(kerberosProperties.getKerberosKeytab()).evaluateAttributeExpressions().getValue();
+            final String explicitPassword = validationContext.getProperty(kerberosProperties.getKerberosPassword()).getValue();
             final KerberosCredentialsService credentialsService = validationContext.getProperty(KERBEROS_CREDENTIALS_SERVICE).asControllerService(KerberosCredentialsService.class);
 
             final String resolvedPrincipal;
             final String resolvedKeytab;
-            if (credentialsService == null) {
-                resolvedPrincipal = explicitPrincipal;
-                resolvedKeytab = explicitKeytab;
-            } else {
+            if (credentialsService != null) {
                 resolvedPrincipal = credentialsService.getPrincipal();
                 resolvedKeytab = credentialsService.getKeytab();
+            } else {
+                resolvedPrincipal = explicitPrincipal;
+                resolvedKeytab = explicitKeytab;
             }
 
 
             final String configFiles = validationContext.getProperty(HIVE_CONFIGURATION_RESOURCES).evaluateAttributeExpressions().getValue();
-            problems.addAll(hiveConfigurator.validate(configFiles, resolvedPrincipal, resolvedKeytab, validationResourceHolder, getLogger()));
+            problems.addAll(hiveConfigurator.validate(configFiles, resolvedPrincipal, resolvedKeytab, explicitPassword, validationResourceHolder, getLogger()));
 
-            if (credentialsService != null && (explicitPrincipal != null || explicitKeytab != null)) {
+            if (credentialsService != null && (explicitPrincipal != null || explicitKeytab != null || explicitPassword != null)) {
                 problems.add(new ValidationResult.Builder()
-                        .subject("Kerberos Credentials")
-                        .valid(false)
-                        .explanation("Cannot specify both a Kerberos Credentials Service and a principal/keytab")
-                        .build());
+                    .subject("Kerberos Credentials")
+                    .valid(false)
+                    .explanation("Cannot specify a Kerberos Credentials Service while also specifying a Kerberos Principal, Kerberos Keytab, or Kerberos Password")
+                    .build());
             }
 
-            final String allowExplicitKeytabVariable = System.getenv(ALLOW_EXPLICIT_KEYTAB);
-            if ("false".equalsIgnoreCase(allowExplicitKeytabVariable) && (explicitPrincipal != null || explicitKeytab != null)) {
+            if (isAllowExplicitKeytab() && explicitKeytab != null) {
 
 Review comment:
   Fixed.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] jtstorck commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

Posted by GitBox <gi...@apache.org>.
jtstorck commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102#discussion_r385934166
 
 

 ##########
 File path: nifi-nar-bundles/nifi-extension-utils/nifi-hadoop-utils/src/main/java/org/apache/nifi/processors/hadoop/AbstractHadoopProcessor.java
 ##########
 @@ -563,8 +558,8 @@ protected UserGroupInformation getUserGroupInformation() {
     /*
      * Overridable by subclasses in the same package, mainly intended for testing purposes to allow verification without having to set environment variables.
      */
-    String getAllowExplicitKeytabEnvironmentVariable() {
-        return System.getenv(ALLOW_EXPLICIT_KEYTAB);
+    boolean isAllowExplicitKeytab() {
+        return Boolean.parseBoolean(System.getenv(ALLOW_EXPLICIT_KEYTAB));
 
 Review comment:
   Boolean.parseBoolean(String s) doesn't throw an exception.  It'll return false unless the string passed to it, ignoring case, equals "true".  It doesn't do any trimming, just takes the given string and evaluates it:
   ```java
       public static boolean parseBoolean(String s) {
           return ((s != null) && s.equalsIgnoreCase("true"));
       }
   ```

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] jtstorck commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

Posted by GitBox <gi...@apache.org>.
jtstorck commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102#discussion_r386492319
 
 

 ##########
 File path: nifi-nar-bundles/nifi-hive-bundle/nifi-hive-processors/src/main/java/org/apache/nifi/processors/hive/PutHiveStreaming.java
 ##########
 @@ -396,23 +403,22 @@ protected void init(ProcessorInitializationContext context) {
 
 
             final String configFiles = validationContext.getProperty(HIVE_CONFIGURATION_RESOURCES).evaluateAttributeExpressions().getValue();
-            problems.addAll(hiveConfigurator.validate(configFiles, resolvedPrincipal, resolvedKeytab, validationResourceHolder, getLogger()));
+            problems.addAll(hiveConfigurator.validate(configFiles, resolvedPrincipal, resolvedKeytab, explicitPassword, validationResourceHolder, getLogger()));
 
             if (credentialsService != null && (explicitPrincipal != null || explicitKeytab != null)) {
 
 Review comment:
   Fixed.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] bbende commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

Posted by GitBox <gi...@apache.org>.
bbende commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102#discussion_r386445762
 
 

 ##########
 File path: nifi-nar-bundles/nifi-hive-bundle/nifi-hive-processors/src/main/java/org/apache/nifi/dbcp/hive/HiveConnectionPool.java
 ##########
 @@ -184,6 +190,7 @@ protected void init(final ControllerServiceInitializationContext context) {
         kerberosProperties = new KerberosProperties(kerberosConfigFile);
         props.add(kerberosProperties.getKerberosPrincipal());
         props.add(kerberosProperties.getKerberosKeytab());
+        props.add(kerberosProperties.getKerberosPrincipal());
 
 Review comment:
   Should this be `getKerberosPassword() `?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] jtstorck commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

Posted by GitBox <gi...@apache.org>.
jtstorck commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102#discussion_r386492253
 
 

 ##########
 File path: nifi-nar-bundles/nifi-hive-bundle/nifi-hive-processors/src/main/java/org/apache/nifi/dbcp/hive/HiveConnectionPool.java
 ##########
 @@ -184,6 +190,7 @@ protected void init(final ControllerServiceInitializationContext context) {
         kerberosProperties = new KerberosProperties(kerberosConfigFile);
         props.add(kerberosProperties.getKerberosPrincipal());
         props.add(kerberosProperties.getKerberosKeytab());
+        props.add(kerberosProperties.getKerberosPrincipal());
 
 Review comment:
   Fixed.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] bbende commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

Posted by GitBox <gi...@apache.org>.
bbende commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102#discussion_r385928655
 
 

 ##########
 File path: nifi-nar-bundles/nifi-hive-bundle/nifi-hive-processors/src/main/java/org/apache/nifi/dbcp/hive/HiveConnectionPool.java
 ##########
 @@ -363,6 +379,20 @@ public Connection getConnection() throws ProcessException {
                      * the acquisition of a new TGT after the current one has expired.
                      * https://issues.apache.org/jira/browse/NIFI-5134
                      */
+                    getLogger().trace("getting UGI instance");
+                    if (kerberosUserReference.get() != null) {
+                        // if there's a KerberosUser associated with this UGI, check the TGT and relogin if it is close to expiring
+                        KerberosUser kerberosUser = kerberosUserReference.get();
+                        getLogger().debug("kerberosUser is " + kerberosUser);
+                        try {
+                            getLogger().debug("checking TGT on kerberosUser [{}]", new Object[] {kerberosUser});
+                            kerberosUser.checkTGTAndRelogin();
+                        } catch (LoginException e) {
+                            throw new ProcessException("Unable to relogin with kerberos credentials for " + kerberosUser.getPrincipal(), e);
+                        }
+                    } else {
+                        getLogger().debug("kerberosUser was null, will not refresh TGT with KerberosUser");
+                    }
                     ugi.checkTGTAndReloginFromKeytab();
 
 Review comment:
   I think we want to remove this because all cases are going through KerberosUser above right?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] jtstorck commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

Posted by GitBox <gi...@apache.org>.
jtstorck commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102#discussion_r386492592
 
 

 ##########
 File path: nifi-nar-bundles/nifi-hive-bundle/nifi-hive3-processors/src/main/java/org/apache/nifi/dbcp/hive/Hive3ConnectionPool.java
 ##########
 @@ -306,38 +313,38 @@ protected void init(final ControllerServiceInitializationContext context) {
         if (confFileProvided) {
             final String explicitPrincipal = validationContext.getProperty(kerberosProperties.getKerberosPrincipal()).evaluateAttributeExpressions().getValue();
             final String explicitKeytab = validationContext.getProperty(kerberosProperties.getKerberosKeytab()).evaluateAttributeExpressions().getValue();
+            final String explicitPassword = validationContext.getProperty(kerberosProperties.getKerberosPassword()).getValue();
             final KerberosCredentialsService credentialsService = validationContext.getProperty(KERBEROS_CREDENTIALS_SERVICE).asControllerService(KerberosCredentialsService.class);
 
             final String resolvedPrincipal;
             final String resolvedKeytab;
-            if (credentialsService == null) {
-                resolvedPrincipal = explicitPrincipal;
-                resolvedKeytab = explicitKeytab;
-            } else {
+            if (credentialsService != null) {
                 resolvedPrincipal = credentialsService.getPrincipal();
                 resolvedKeytab = credentialsService.getKeytab();
+            } else {
+                resolvedPrincipal = explicitPrincipal;
+                resolvedKeytab = explicitKeytab;
             }
 
 
             final String configFiles = validationContext.getProperty(HIVE_CONFIGURATION_RESOURCES).evaluateAttributeExpressions().getValue();
-            problems.addAll(hiveConfigurator.validate(configFiles, resolvedPrincipal, resolvedKeytab, validationResourceHolder, getLogger()));
+            problems.addAll(hiveConfigurator.validate(configFiles, resolvedPrincipal, resolvedKeytab, explicitPassword, validationResourceHolder, getLogger()));
 
-            if (credentialsService != null && (explicitPrincipal != null || explicitKeytab != null)) {
+            if (credentialsService != null && (explicitPrincipal != null || explicitKeytab != null || explicitPassword != null)) {
                 problems.add(new ValidationResult.Builder()
-                        .subject("Kerberos Credentials")
-                        .valid(false)
-                        .explanation("Cannot specify both a Kerberos Credentials Service and a principal/keytab")
-                        .build());
+                    .subject("Kerberos Credentials")
+                    .valid(false)
+                    .explanation("Cannot specify a Kerberos Credentials Service while also specifying a Kerberos Principal, Kerberos Keytab, or Kerberos Password")
+                    .build());
             }
 
-            final String allowExplicitKeytabVariable = System.getenv(ALLOW_EXPLICIT_KEYTAB);
-            if ("false".equalsIgnoreCase(allowExplicitKeytabVariable) && (explicitPrincipal != null || explicitKeytab != null)) {
+            if (isAllowExplicitKeytab() && explicitKeytab != null) {
 
 Review comment:
   Fixed.

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] bbende commented on issue #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

Posted by GitBox <gi...@apache.org>.
bbende commented on issue #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102#issuecomment-593489610
 
 
   Changes look good, going to merge

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] bbende commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

Posted by GitBox <gi...@apache.org>.
bbende commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102#discussion_r386448046
 
 

 ##########
 File path: nifi-nar-bundles/nifi-hive-bundle/nifi-hive-processors/src/main/java/org/apache/nifi/processors/hive/PutHiveStreaming.java
 ##########
 @@ -396,23 +403,22 @@ protected void init(ProcessorInitializationContext context) {
 
 
             final String configFiles = validationContext.getProperty(HIVE_CONFIGURATION_RESOURCES).evaluateAttributeExpressions().getValue();
-            problems.addAll(hiveConfigurator.validate(configFiles, resolvedPrincipal, resolvedKeytab, validationResourceHolder, getLogger()));
+            problems.addAll(hiveConfigurator.validate(configFiles, resolvedPrincipal, resolvedKeytab, explicitPassword, validationResourceHolder, getLogger()));
 
             if (credentialsService != null && (explicitPrincipal != null || explicitKeytab != null)) {
 
 Review comment:
   Do we need to update the if statement to include `|| explicitPassword != null` ?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] bbende commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

Posted by GitBox <gi...@apache.org>.
bbende commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102#discussion_r386449059
 
 

 ##########
 File path: nifi-nar-bundles/nifi-hive-bundle/nifi-hive-processors/src/main/java/org/apache/nifi/processors/hive/PutHiveStreaming.java
 ##########
 @@ -396,23 +403,22 @@ protected void init(ProcessorInitializationContext context) {
 
 
             final String configFiles = validationContext.getProperty(HIVE_CONFIGURATION_RESOURCES).evaluateAttributeExpressions().getValue();
-            problems.addAll(hiveConfigurator.validate(configFiles, resolvedPrincipal, resolvedKeytab, validationResourceHolder, getLogger()));
+            problems.addAll(hiveConfigurator.validate(configFiles, resolvedPrincipal, resolvedKeytab, explicitPassword, validationResourceHolder, getLogger()));
 
             if (credentialsService != null && (explicitPrincipal != null || explicitKeytab != null)) {
                 problems.add(new ValidationResult.Builder()
                     .subject("Kerberos Credentials")
                     .valid(false)
-                    .explanation("Cannot specify both a Kerberos Credentials Service and a principal/keytab")
+                        .explanation("Cannot specify a Kerberos Credentials Service while also specifying a Kerberos Principal, Kerberos Keytab, or Kerberos Password")
                     .build());
             }
 
-            final String allowExplicitKeytabVariable = System.getenv(ALLOW_EXPLICIT_KEYTAB);
-            if ("false".equalsIgnoreCase(allowExplicitKeytabVariable) && (explicitPrincipal != null || explicitKeytab != null)) {
+            if (!isAllowExplicitKeytab() && (explicitPrincipal != null || explicitKeytab != null)) {
 
 Review comment:
   Do we need to remove the explicitPrincipal check here?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

[GitHub] [nifi] bbende commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components

Posted by GitBox <gi...@apache.org>.
bbende commented on a change in pull request #4102: NIFI-7025: Adds Kerberos Password to Hive/Hive_1_1/Hive3 components
URL: https://github.com/apache/nifi/pull/4102#discussion_r386451557
 
 

 ##########
 File path: nifi-nar-bundles/nifi-hive-bundle/nifi-hive3-processors/src/main/java/org/apache/nifi/dbcp/hive/Hive3ConnectionPool.java
 ##########
 @@ -289,6 +295,7 @@ protected void init(final ControllerServiceInitializationContext context) {
         kerberosProperties = new KerberosProperties(kerberosConfigFile);
         props.add(kerberosProperties.getKerberosPrincipal());
         props.add(kerberosProperties.getKerberosKeytab());
+        props.add(kerberosProperties.getKerberosPrincipal());
 
 Review comment:
   Should be `getKerberosPassword() ` ?

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services