You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@ambari.apache.org by Amruta Borkar <ar...@us.ibm.com> on 2017/03/14 17:21:46 UTC

Review Request 57610: Tokenize kerberos principal name appearing in kerberos rules in exported blueprint

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57610/
-----------------------------------------------------------

Review request for Ambari, Di Li, Robert Nettleton, and Sandor Magyari.


Bugs: AMBARI-20366
    https://issues.apache.org/jira/browse/AMBARI-20366


Repository: ambari


Description
-------

If blueprint is exported from a kerberos enabled cluster Kerberos rules export principal names which contain cluster name and Realm, this exports existing cluster name and realm name as tokens and replaces those tokens with new values of cluster name and realm during successive cluster deployments.


Diffs
-----

  ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java 5e19a6c 
  ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 5732a1c 
  ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java e29417b 
  ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 75ffd31 
  ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java d160050 
  ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterConfigurationRequestTest.java c97c568 


Diff: https://reviews.apache.org/r/57610/diff/1/


Testing
-------

Tested manually.
Modified test cases.


Thanks,

Amruta Borkar


Re: Review Request 57610: Tokenize kerberos principal name appearing in kerberos rules in exported blueprint

Posted by Robert Levas <rl...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57610/#review169191
-----------------------------------------------------------




ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
Lines 839 (patched)
<https://reviews.apache.org/r/57610/#comment241530>

    This is rather dangerous.   What if my cluster name is "A".  Then all `a`'s will be replaced with `CLUSTER_NAME`.
    
    For example:
    ```
    RULE:[1:$1@$0](ambari-qa-a@EXAMPLE.COM)s/.*/ambari-qa/
    ```
    Will become
    ```
    RULE:[1:$1@$0](CLUSTER_NAMEmbCLUSTER_NAMEri-qCLUSTER_NAME-CLUSTER_NAME@REALM)s/.*/CLUSTER_NAMEmbCLUSTER_NAMEri-qCLUSTER_NAME/
    ```



ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
Lines 929 (patched)
<https://reviews.apache.org/r/57610/#comment241531>

    There are more auth-to-local rule sets than `core-site/hadoop.security.auth_to_local` and `oozie-size/oozie.authentication.kerberos.name.rules`.  
    
    For example `application-properties/atlas.http.authentication.kerberos.name.rules`.
    
    FOr this to be effective, the method would need to traverse the Kerberos Descriptor and get the configuration specifications from the "auth_to_local_properties" values.  See `org.apache.ambari.server.state.kerberos.KerberosDescriptor#getAllAuthToLocalProperties`.



ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
Line 230 (original), 230 (patched)
<https://reviews.apache.org/r/57610/#comment241533>

    `clusetrName` ==> `clusterName`


- Robert Levas


On March 14, 2017, 1:21 p.m., Amruta Borkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57610/
> -----------------------------------------------------------
> 
> (Updated March 14, 2017, 1:21 p.m.)
> 
> 
> Review request for Ambari, Di Li, Robert Nettleton, and Sandor Magyari.
> 
> 
> Bugs: AMBARI-20366
>     https://issues.apache.org/jira/browse/AMBARI-20366
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> If blueprint is exported from a kerberos enabled cluster Kerberos rules export principal names which contain cluster name and Realm, this exports existing cluster name and realm name as tokens and replaces those tokens with new values of cluster name and realm during successive cluster deployments.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java 5e19a6c 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 5732a1c 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java e29417b 
>   ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 75ffd31 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java d160050 
>   ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterConfigurationRequestTest.java c97c568 
> 
> 
> Diff: https://reviews.apache.org/r/57610/diff/1/
> 
> 
> Testing
> -------
> 
> Tested manually.
> Modified test cases.
> 
> 
> Thanks,
> 
> Amruta Borkar
> 
>


Re: Review Request 57610: Tokenize kerberos principal name appearing in kerberos rules in exported blueprint

Posted by Robert Levas <rl...@hortonworks.com>.

> On March 16, 2017, 9:12 a.m., Sandor Magyari wrote:
> > The question is here whether is useful or not to export auth_to_local properties, since they are generated at deploy time anyway. May be would be better to exclude from export.
> > Could you please also add Robert Levas as a reviewer?
> 
> Robert Levas wrote:
>     I think in general, the auth-to-local rules should not be exported as part of the blueprint, since as Sandor mentions that they are dynamically created anyways. The content of the rules are cluster specific.  So if the realm changes, the rules must change as well. Same with the cluster name - if the cluster name is used as the value to help create unique principal names. 
>     
>     That said, +1 for Sandor's comment of removing auth-to-local rules from the exported BP.
> 
> Amruta Borkar wrote:
>     Hello Sandor, Robert
>     I am trying to tokenize rather that filtering out from blueprint, to preserve any custom mapping rules user may have added to auth_to_local. Could you please give suggestion for this case? 
>     
>     Thank you

I thought about the custom rules as well; but it seems like that in most cases, they would be cluster-specific as well. However I could imagine a scenario where a BP is used to build a set of clusters, each relying on the same Active Directory for user accounts where a special mapping is necessary to translate the principal names to local usernames. Still, I am not sure there is a good solution for that here. Maybe logic is needed to find the difference between the auto-generated rules and the custom rules. Then export the custom-rules only.  Care is still needed when "tokenizing" the custom rules since the cluster name can be as simple as `"a"` and thefore cause issues when doing a string replace.


- Robert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57610/#review169142
-----------------------------------------------------------


On March 16, 2017, 3:55 p.m., Amruta Borkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57610/
> -----------------------------------------------------------
> 
> (Updated March 16, 2017, 3:55 p.m.)
> 
> 
> Review request for Ambari, Di Li, Robert Levas, Robert Nettleton, and Sandor Magyari.
> 
> 
> Bugs: AMBARI-20366
>     https://issues.apache.org/jira/browse/AMBARI-20366
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> If blueprint is exported from a kerberos enabled cluster Kerberos rules export principal names which contain cluster name and Realm, this exports existing cluster name and realm name as tokens and replaces those tokens with new values of cluster name and realm during successive cluster deployments.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java 5e19a6c 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 5732a1c 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java e29417b 
>   ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 75ffd31 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java d160050 
>   ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterConfigurationRequestTest.java c97c568 
> 
> 
> Diff: https://reviews.apache.org/r/57610/diff/1/
> 
> 
> Testing
> -------
> 
> Tested manually.
> Modified test cases.
> 
> 
> Thanks,
> 
> Amruta Borkar
> 
>


Re: Review Request 57610: Tokenize kerberos principal name appearing in kerberos rules in exported blueprint

Posted by Amruta Borkar <ar...@us.ibm.com>.

> On March 16, 2017, 1:12 p.m., Sandor Magyari wrote:
> > The question is here whether is useful or not to export auth_to_local properties, since they are generated at deploy time anyway. May be would be better to exclude from export.
> > Could you please also add Robert Levas as a reviewer?
> 
> Robert Levas wrote:
>     I think in general, the auth-to-local rules should not be exported as part of the blueprint, since as Sandor mentions that they are dynamically created anyways. The content of the rules are cluster specific.  So if the realm changes, the rules must change as well. Same with the cluster name - if the cluster name is used as the value to help create unique principal names. 
>     
>     That said, +1 for Sandor's comment of removing auth-to-local rules from the exported BP.

Hello Sandor, Robert
I am trying to tokenize rather that filtering out from blueprint, to preserve any custom mapping rules user may have added to auth_to_local. Could you please give suggestion for this case? 

Thank you


- Amruta


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57610/#review169142
-----------------------------------------------------------


On March 14, 2017, 5:21 p.m., Amruta Borkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57610/
> -----------------------------------------------------------
> 
> (Updated March 14, 2017, 5:21 p.m.)
> 
> 
> Review request for Ambari, Di Li, Robert Nettleton, and Sandor Magyari.
> 
> 
> Bugs: AMBARI-20366
>     https://issues.apache.org/jira/browse/AMBARI-20366
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> If blueprint is exported from a kerberos enabled cluster Kerberos rules export principal names which contain cluster name and Realm, this exports existing cluster name and realm name as tokens and replaces those tokens with new values of cluster name and realm during successive cluster deployments.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java 5e19a6c 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 5732a1c 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java e29417b 
>   ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 75ffd31 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java d160050 
>   ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterConfigurationRequestTest.java c97c568 
> 
> 
> Diff: https://reviews.apache.org/r/57610/diff/1/
> 
> 
> Testing
> -------
> 
> Tested manually.
> Modified test cases.
> 
> 
> Thanks,
> 
> Amruta Borkar
> 
>


Re: Review Request 57610: Tokenize kerberos principal name appearing in kerberos rules in exported blueprint

Posted by Robert Levas <rl...@hortonworks.com>.

> On March 16, 2017, 9:12 a.m., Sandor Magyari wrote:
> > The question is here whether is useful or not to export auth_to_local properties, since they are generated at deploy time anyway. May be would be better to exclude from export.
> > Could you please also add Robert Levas as a reviewer?

I think in general, the auth-to-local rules should not be exported as part of the blueprint, since as Sandor mentions that they are dynamically created anyways. The content of the rules are cluster specific.  So if the realm changes, the rules must change as well. Same with the cluster name - if the cluster name is used as the value to help create unique principal names. 

That said, +1 for Sandor's comment of removing auth-to-local rules from the exported BP.


- Robert


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57610/#review169142
-----------------------------------------------------------


On March 14, 2017, 1:21 p.m., Amruta Borkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57610/
> -----------------------------------------------------------
> 
> (Updated March 14, 2017, 1:21 p.m.)
> 
> 
> Review request for Ambari, Di Li, Robert Nettleton, and Sandor Magyari.
> 
> 
> Bugs: AMBARI-20366
>     https://issues.apache.org/jira/browse/AMBARI-20366
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> If blueprint is exported from a kerberos enabled cluster Kerberos rules export principal names which contain cluster name and Realm, this exports existing cluster name and realm name as tokens and replaces those tokens with new values of cluster name and realm during successive cluster deployments.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java 5e19a6c 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 5732a1c 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java e29417b 
>   ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 75ffd31 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java d160050 
>   ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterConfigurationRequestTest.java c97c568 
> 
> 
> Diff: https://reviews.apache.org/r/57610/diff/1/
> 
> 
> Testing
> -------
> 
> Tested manually.
> Modified test cases.
> 
> 
> Thanks,
> 
> Amruta Borkar
> 
>


Re: Review Request 57610: Tokenize kerberos principal name appearing in kerberos rules in exported blueprint

Posted by Sandor Magyari <sm...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57610/#review169142
-----------------------------------------------------------



The question is here whether is useful or not to export auth_to_local properties, since they are generated at deploy time anyway. May be would be better to exclude from export.
Could you please also add Robert Levas as a reviewer?


ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
Lines 854 (patched)
<https://reviews.apache.org/r/57610/#comment241470>

    When deploying a new secure cluster auth_to_local properties are generated anyway so I'm not sure if this token replacement makes sense here.


- Sandor Magyari


On March 14, 2017, 5:21 p.m., Amruta Borkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57610/
> -----------------------------------------------------------
> 
> (Updated March 14, 2017, 5:21 p.m.)
> 
> 
> Review request for Ambari, Di Li, Robert Nettleton, and Sandor Magyari.
> 
> 
> Bugs: AMBARI-20366
>     https://issues.apache.org/jira/browse/AMBARI-20366
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> If blueprint is exported from a kerberos enabled cluster Kerberos rules export principal names which contain cluster name and Realm, this exports existing cluster name and realm name as tokens and replaces those tokens with new values of cluster name and realm during successive cluster deployments.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java 5e19a6c 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 5732a1c 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java e29417b 
>   ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 75ffd31 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java d160050 
>   ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterConfigurationRequestTest.java c97c568 
> 
> 
> Diff: https://reviews.apache.org/r/57610/diff/1/
> 
> 
> Testing
> -------
> 
> Tested manually.
> Modified test cases.
> 
> 
> Thanks,
> 
> Amruta Borkar
> 
>


Re: Review Request 57610: Tokenize kerberos principal name appearing in kerberos rules in exported blueprint

Posted by Di Li <di...@ca.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57610/#review169093
-----------------------------------------------------------


Ship it!




Ship It!

- Di Li


On March 14, 2017, 5:21 p.m., Amruta Borkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57610/
> -----------------------------------------------------------
> 
> (Updated March 14, 2017, 5:21 p.m.)
> 
> 
> Review request for Ambari, Di Li, Robert Nettleton, and Sandor Magyari.
> 
> 
> Bugs: AMBARI-20366
>     https://issues.apache.org/jira/browse/AMBARI-20366
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> If blueprint is exported from a kerberos enabled cluster Kerberos rules export principal names which contain cluster name and Realm, this exports existing cluster name and realm name as tokens and replaces those tokens with new values of cluster name and realm during successive cluster deployments.
> 
> 
> Diffs
> -----
> 
>   ambari-server/src/main/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRenderer.java 5e19a6c 
>   ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 5732a1c 
>   ambari-server/src/main/java/org/apache/ambari/server/topology/ClusterConfigurationRequest.java e29417b 
>   ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 75ffd31 
>   ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java d160050 
>   ambari-server/src/test/java/org/apache/ambari/server/topology/ClusterConfigurationRequestTest.java c97c568 
> 
> 
> Diff: https://reviews.apache.org/r/57610/diff/1/
> 
> 
> Testing
> -------
> 
> Tested manually.
> Modified test cases.
> 
> 
> Thanks,
> 
> Amruta Borkar
> 
>