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

Reply via email to