----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://reviews.apache.org/r/51822/#review148699 -----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java (line 2376) <https://reviews.apache.org/r/51822/#comment216197> convenience ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java (line 2585) <https://reviews.apache.org/r/51822/#comment216198> authentication ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java (line 2586) <https://reviews.apache.org/r/51822/#comment216199> Although I understand the motivation, I still don't like doing this since getting the backing Properties object for the entire Configuration would be missing these properties. Can we mirror them in the normal Properties object as well? ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java (lines 5292 - 5294) <https://reviews.apache.org/r/51822/#comment216200> Why not just return here which would prevent you from needing to embed the rest of the logic in the if-statement ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java (lines 5298 - 5301) <https://reviews.apache.org/r/51822/#comment216202> KERBEROS_AUTH_USER_TYPES has a default value, so `getProperty` will never return null. ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java (lines 5310 - 5311) <https://reviews.apache.org/r/51822/#comment216201> Invalid types would seem to be a mis-configuration which could cause connectivity issues with Ambari; did you want to instead throw a real exception here? ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java (lines 5318 - 5322) <https://reviews.apache.org/r/51822/#comment216203> Not sure if this is possible since KERBEROS_AUTH_USER_TYPES has a default value. ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java (lines 5345 - 5348) <https://reviews.apache.org/r/51822/#comment216204> Same question as above - because these configurations involve connection to Ambari, should we instead throw exceptions and fail startup on problems like this? - Jonathan Hurley On Sept. 13, 2016, 5 a.m., Robert Levas wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://reviews.apache.org/r/51822/ > ----------------------------------------------------------- > > (Updated Sept. 13, 2016, 5 a.m.) > > > Review request for Ambari, Alejandro Fernandez, Jonathan Hurley, and Nate > Cole. > > > Bugs: AMBARI-18365 > https://issues.apache.org/jira/browse/AMBARI-18365 > > > Repository: ambari > > > Description > ------- > > Add the followng Ambari configuration options to support Kerberos token > authentication > > - `authentication.kerberos.enabled` > -- Determines whether to use Kerberos (SPNEGO) authentication when connecting > Ambari: {{true}} to enable this feature; {{false}}, otherwise > - `authentication.kerberos.spnego.principal` > -- The Kerberos principal name to use when verifying user-supplied Kerberos > tokens for authentication via SPNEGO > - `authentication.kerberos.spnego.keytab.file` > -- The Kerberos keytab file to use when verifying user-supplied Kerberos > tokens for authentication via SPNEGO > - `authentication.kerberos.user.types` > -- A comma-delimited (ordered) list of preferred user types to use when > finding the Ambari user account for the user-supplied Kerberos identity > during authentication via SPNEGO > - `authentication.kerberos.auth_to_local.rules` > -- The auth-to-local rules set to use when translating a user's principal > name to a local user name during authentication via SPNEGO. > > NOTE: These properties are in the {{ambari.properties}} file since this > feature may be enabled whether the rest of the cluster has Kerberos enabled > or not. > > See https://issues.apache.org/jira/browse/AMBARI-18364 for an overview > requiring the need for this patch. > > > Diffs > ----- > > > ambari-server/src/main/java/org/apache/ambari/server/configuration/Configuration.java > ee73b8d > > ambari-server/src/main/java/org/apache/ambari/server/security/authentication/kerberos/AmbariKerberosAuthenticationProperties.java > PRE-CREATION > > ambari-server/src/test/java/org/apache/ambari/server/configuration/ConfigurationTest.java > f429a36 > > ambari-server/src/test/java/org/apache/ambari/server/security/authentication/kerberos/AmbariKerberosAuthenticationPropertiesTest.java > PRE-CREATION > > Diff: https://reviews.apache.org/r/51822/diff/ > > > Testing > ------- > > Manually tested... > > # Local test results: > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 2:51:23.081s > [INFO] Finished at: Tue Sep 13 00:00:59 EDT 2016 > [INFO] Final Memory: 48M/1792M > [INFO] > ------------------------------------------------------------------------ > > # Jenkins test results: > > [INFO] > ------------------------------------------------------------------------ > [INFO] BUILD SUCCESS > [INFO] > ------------------------------------------------------------------------ > [INFO] Total time: 01:34 h > [INFO] Finished at: 2016-09-13T08:46:23+00:00 > [INFO] Final Memory: 178M/2672M > [INFO] > ------------------------------------------------------------------------ > > {color:green}+1 overall{color}. Here are the results of testing the latest > attachment > > http://issues.apache.org/jira/secure/attachment/12828101/AMBARI-18365_trunk_01.patch > against trunk revision . > > {color:green}+1 @author{color}. The patch does not contain any @author > tags. > > {color:green}+1 tests included{color}. The patch appears to include 2 > new or modified test files. > > {color:green}+1 javac{color}. The applied patch does not increase the > total number of javac compiler warnings. > > {color:green}+1 release audit{color}. The applied patch does not > increase the total number of release audit warnings. > > {color:green}+1 core tests{color}. The patch passed unit tests in > ambari-server. > > Test results: > https://builds.apache.org/job/Ambari-trunk-test-patch/8643//testReport/ > Console output: > https://builds.apache.org/job/Ambari-trunk-test-patch/8643//console > > This message is automatically generated. > > > Thanks, > > Robert Levas > >
