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/04/05 16:56:52 UTC
Re: 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/
-----------------------------------------------------------
(Updated April 5, 2017, 4:56 p.m.)
Review request for Ambari, Di Li, Robert Levas, Robert Nettleton, and Sandor Magyari.
Changes
-------
Updated patch to filter out auth_to_local properties.
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 (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java db1aa074d4
ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 13db5f8b56
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java dba40437ae
Diff: https://reviews.apache.org/r/57610/diff/2/
Changes: https://reviews.apache.org/r/57610/diff/1-2/
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/#review171452
-----------------------------------------------------------
Ship it!
The new patch filters out the auth-to-local rule propeties. This is a better solution since it is expected that the Kerberos logic will dynamically create them anyways.
- Robert Levas
On April 5, 2017, 12:56 p.m., Amruta Borkar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57610/
> -----------------------------------------------------------
>
> (Updated April 5, 2017, 12:56 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/controller/internal/BlueprintConfigurationProcessor.java db1aa074d4
> ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 13db5f8b56
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java dba40437ae
>
>
> Diff: https://reviews.apache.org/r/57610/diff/2/
>
>
> Testing
> -------
>
> Tested manually.
> Modified test cases.
>
>
> Thanks,
>
> Amruta Borkar
>
>
Re: Review Request 57610: Filter out kerberos rules in exported
blueprint
Posted by Amruta Borkar <ar...@us.ibm.com>.
> On May 2, 2017, 4:32 p.m., Sandor Magyari wrote:
> > Ship It!
Thank you Sandor, could you please help me push this to trunk?
- Amruta
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57610/#review173600
-----------------------------------------------------------
On April 30, 2017, 4:03 a.m., Amruta Borkar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57610/
> -----------------------------------------------------------
>
> (Updated April 30, 2017, 4:03 a.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 patch filter out kerberos rules properties so that hardcoded cluster name and realm are not exported.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 7381387b53
> ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 13db5f8b56
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 8ff70a1d46
>
>
> Diff: https://reviews.apache.org/r/57610/diff/5/
>
>
> Testing
> -------
>
> Tested manually.
> Modified test cases.
>
>
> Thanks,
>
> Amruta Borkar
>
>
Re: Review Request 57610: Filter out 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/#review173600
-----------------------------------------------------------
Ship it!
Ship It!
- Sandor Magyari
On April 30, 2017, 4:03 a.m., Amruta Borkar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57610/
> -----------------------------------------------------------
>
> (Updated April 30, 2017, 4:03 a.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 patch filter out kerberos rules properties so that hardcoded cluster name and realm are not exported.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 7381387b53
> ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 13db5f8b56
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 8ff70a1d46
>
>
> Diff: https://reviews.apache.org/r/57610/diff/5/
>
>
> Testing
> -------
>
> Tested manually.
> Modified test cases.
>
>
> Thanks,
>
> Amruta Borkar
>
>
Re: Review Request 57610: Filter out kerberos rules in exported
blueprint
Posted by Amruta Borkar <ar...@us.ibm.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57610/
-----------------------------------------------------------
(Updated April 30, 2017, 4:03 a.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 patch filter out kerberos rules properties so that hardcoded cluster name and realm are not exported.
Diffs (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 7381387b53
ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 13db5f8b56
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 8ff70a1d46
Diff: https://reviews.apache.org/r/57610/diff/5/
Changes: https://reviews.apache.org/r/57610/diff/4-5/
Testing
-------
Tested manually.
Modified test cases.
Thanks,
Amruta Borkar
Re: Review Request 57610: Filter out kerberos rules in exported
blueprint
Posted by Amruta Borkar <ar...@us.ibm.com>.
> On April 20, 2017, 4 p.m., Sandor Magyari wrote:
> > I've suggested using the thread local but overall looking at the patch now, it seems to me a litle bit overcomplicated.
> > May be it would much more simplier if we could pass authToLocal properties to KerberosAuthToLocalRulesFilter directly to the constructor. So instead of creating a static list of filters, we could create this list of filters (exportPropertyFilters) in doFilterPriorToExport method before filtering code - as it's not used anywhere else - where you can easily get authToLocal properties for the given cluster then pass it to KerberosAuthToLocalRulesFilter instance through the constructor. So I think actually we don't need to keep the list of filters statically all the time, we can create the list just for the time of export filtering.
Hello Sandor,
Could you please review the updated patch?
- Amruta
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57610/#review172506
-----------------------------------------------------------
On April 30, 2017, 4:03 a.m., Amruta Borkar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57610/
> -----------------------------------------------------------
>
> (Updated April 30, 2017, 4:03 a.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 patch filter out kerberos rules properties so that hardcoded cluster name and realm are not exported.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java 7381387b53
> ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 13db5f8b56
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 8ff70a1d46
>
>
> Diff: https://reviews.apache.org/r/57610/diff/5/
>
>
> Testing
> -------
>
> Tested manually.
> Modified test cases.
>
>
> Thanks,
>
> Amruta Borkar
>
>
Re: Review Request 57610: Filter out 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/#review172506
-----------------------------------------------------------
I've suggested using the thread local but overall looking at the patch now, it seems to me a litle bit overcomplicated.
May be it would much more simplier if we could pass authToLocal properties to KerberosAuthToLocalRulesFilter directly to the constructor. So instead of creating a static list of filters, we could create this list of filters (exportPropertyFilters) in doFilterPriorToExport method before filtering code - as it's not used anywhere else - where you can easily get authToLocal properties for the given cluster then pass it to KerberosAuthToLocalRulesFilter instance through the constructor. So I think actually we don't need to keep the list of filters statically all the time, we can create the list just for the time of export filtering.
- Sandor Magyari
On April 18, 2017, 9:13 p.m., Amruta Borkar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57610/
> -----------------------------------------------------------
>
> (Updated April 18, 2017, 9:13 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 patch filter out kerberos rules properties so that hardcoded cluster name and realm are not exported.
>
>
> Diffs
> -----
>
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java bb771a54a6
> ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 13db5f8b56
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 5c1836a87d
>
>
> Diff: https://reviews.apache.org/r/57610/diff/4/
>
>
> 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>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57610/
-----------------------------------------------------------
(Updated April 18, 2017, 9:13 p.m.)
Review request for Ambari, Di Li, Robert Levas, Robert Nettleton, and Sandor Magyari.
Changes
-------
Updated patch for latest trunk code
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 (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java bb771a54a6
ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 13db5f8b56
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java 5c1836a87d
Diff: https://reviews.apache.org/r/57610/diff/4/
Changes: https://reviews.apache.org/r/57610/diff/3-4/
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>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57610/
-----------------------------------------------------------
(Updated April 13, 2017, 6:09 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 (updated)
-----
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java bb771a54a6
ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 13db5f8b56
ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java dba40437ae
Diff: https://reviews.apache.org/r/57610/diff/3/
Changes: https://reviews.apache.org/r/57610/diff/2-3/
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 April 10, 2017, 8:54 p.m., Sandor Magyari wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
> > Lines 3042 (patched)
> > <https://reviews.apache.org/r/57610/diff/2/?file=1685430#file1685430line3042>
> >
> > Caching these properties is a good idea, but I think we should be able to handle multi clusters, so keep theese properties separatly for each cluster and ideally these should be cached only for the time of export, since next time the state of the cluster may change. We don't have such filters yet, may be we can cache theese props in a HashMap by clusterId, in ThreadLocal and an init method to PropertyFilter where we can reset this map prior beginning the export.
Hello Sandor,
I modififed the patch to address this problem, could you please review it?
- Amruta
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57610/#review171474
-----------------------------------------------------------
On April 13, 2017, 6:09 p.m., Amruta Borkar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57610/
> -----------------------------------------------------------
>
> (Updated April 13, 2017, 6:09 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/controller/internal/BlueprintConfigurationProcessor.java bb771a54a6
> ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 13db5f8b56
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java dba40437ae
>
>
> Diff: https://reviews.apache.org/r/57610/diff/3/
>
>
> 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/#review171474
-----------------------------------------------------------
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
Lines 3042 (patched)
<https://reviews.apache.org/r/57610/#comment244415>
Caching these properties is a good idea, but I think we should be able to handle multi clusters, so keep theese properties separatly for each cluster and ideally these should be cached only for the time of export, since next time the state of the cluster may change. We don't have such filters yet, may be we can cache theese props in a HashMap by clusterId, in ThreadLocal and an init method to PropertyFilter where we can reset this map prior beginning the export.
ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
Lines 3048 (patched)
<https://reviews.apache.org/r/57610/#comment244411>
Please use slf4j Logger here
- Sandor Magyari
On April 5, 2017, 4:56 p.m., Amruta Borkar wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57610/
> -----------------------------------------------------------
>
> (Updated April 5, 2017, 4:56 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/controller/internal/BlueprintConfigurationProcessor.java db1aa074d4
> ambari-server/src/test/java/org/apache/ambari/server/api/query/render/ClusterBlueprintRendererTest.java 13db5f8b56
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java dba40437ae
>
>
> Diff: https://reviews.apache.org/r/57610/diff/2/
>
>
> Testing
> -------
>
> Tested manually.
> Modified test cases.
>
>
> Thanks,
>
> Amruta Borkar
>
>