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/03 16:42:04 UTC

Re: Review Request 57563: Kerberos principal creation fails during blueprint install when kdc_hosts is not specified in blueprint

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

(Updated April 3, 2017, 4:42 p.m.)


Review request for Ambari, Di Li and Robert Levas.


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


Repository: ambari


Description
-------

Blueprint validation does not prompt if kdc_hosts is not specified in the blueprint. After service installation, deployment fails during Create Principal stage. As /etc/krb5.conf is overwritten with blank values.


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/57563/diff/2/

Changes: https://reviews.apache.org/r/57563/diff/1-2/


Testing
-------

Manual testing done, if no kdc_hosts is mentioned then validation exception will be thrown during blueprint registration.


Thanks,

Amruta Borkar


Re: Review Request 57563: Kerberos principal creation fails during blueprint install when kdc_hosts is not specified in blueprint

Posted by Robert Nettleton <rn...@hortonworks.com>.

> On April 3, 2017, 6:11 p.m., Robert Nettleton wrote:
> > Thanks for providing this patch, but I don't really think that this is the correct way to solve this problem.
> > 
> > The Blueprint export shouldn't really modify any configuration in the cluster.  The export process may filter out some information (passwords, hostnames for un-managed services, etc), but no configuration property changes should be made. 
> > 
> > In the future, it would be great if Blueprints provided a validation step for the configuration in a Blueprint/Cluster Creation template, as that would resolve the initial issue described.  Unfortunately, such a framework does not yet exist in Blueprints, and would require refactoring of the stack definitions, such that the Blueprint processor could validate configuration properties in a generic fashion.
> 
> Amruta Borkar wrote:
>     Hello Robert, 
>     Thanks for the feedback. Would it be a good idea to handle the validation mentioned by Robert Levas in BlueprintValidatorImpl ?

Hi Amruta,

At some point, it would be good to add validation support for configuration values, and that would likely go into the BlueprintValidation/BlueprintValidatorImpl.  

The problem here is that we probably should not add any hard-coded rules in the validator for specific configuration, since we already have too much hard-coded rules in the Blueprint procesor as it currently stands.

I think that a future release should include support in the stacks for declaring rules about configuration properties, which the Blueprint validator could then enforce. 

Thanks.


- Robert


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


On April 3, 2017, 4:42 p.m., Amruta Borkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57563/
> -----------------------------------------------------------
> 
> (Updated April 3, 2017, 4:42 p.m.)
> 
> 
> Review request for Ambari, Di Li, Robert Levas, and Robert Nettleton.
> 
> 
> Bugs: AMBARI-20386
>     https://issues.apache.org/jira/browse/AMBARI-20386
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Blueprint validation does not prompt if kdc_hosts is not specified in the blueprint. After service installation, deployment fails during Create Principal stage. As /etc/krb5.conf is overwritten with blank values.
> 
> 
> 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/57563/diff/2/
> 
> 
> Testing
> -------
> 
> Manual testing done, if no kdc_hosts is mentioned then validation exception will be thrown during blueprint registration.
> 
> 
> Thanks,
> 
> Amruta Borkar
> 
>


Re: Review Request 57563: Kerberos principal creation fails during blueprint install when kdc_hosts is not specified in blueprint

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

> On April 3, 2017, 6:11 p.m., Robert Nettleton wrote:
> > Thanks for providing this patch, but I don't really think that this is the correct way to solve this problem.
> > 
> > The Blueprint export shouldn't really modify any configuration in the cluster.  The export process may filter out some information (passwords, hostnames for un-managed services, etc), but no configuration property changes should be made. 
> > 
> > In the future, it would be great if Blueprints provided a validation step for the configuration in a Blueprint/Cluster Creation template, as that would resolve the initial issue described.  Unfortunately, such a framework does not yet exist in Blueprints, and would require refactoring of the stack definitions, such that the Blueprint processor could validate configuration properties in a generic fashion.
> 
> Amruta Borkar wrote:
>     Hello Robert, 
>     Thanks for the feedback. Would it be a good idea to handle the validation mentioned by Robert Levas in BlueprintValidatorImpl ?
> 
> Robert Nettleton wrote:
>     Hi Amruta,
>     
>     At some point, it would be good to add validation support for configuration values, and that would likely go into the BlueprintValidation/BlueprintValidatorImpl.  
>     
>     The problem here is that we probably should not add any hard-coded rules in the validator for specific configuration, since we already have too much hard-coded rules in the Blueprint procesor as it currently stands.
>     
>     I think that a future release should include support in the stacks for declaring rules about configuration properties, which the Blueprint validator could then enforce. 
>     
>     Thanks.

Ok Thank you for explanation.


- Amruta


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


On April 3, 2017, 4:42 p.m., Amruta Borkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57563/
> -----------------------------------------------------------
> 
> (Updated April 3, 2017, 4:42 p.m.)
> 
> 
> Review request for Ambari, Di Li, Robert Levas, and Robert Nettleton.
> 
> 
> Bugs: AMBARI-20386
>     https://issues.apache.org/jira/browse/AMBARI-20386
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Blueprint validation does not prompt if kdc_hosts is not specified in the blueprint. After service installation, deployment fails during Create Principal stage. As /etc/krb5.conf is overwritten with blank values.
> 
> 
> 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/57563/diff/2/
> 
> 
> Testing
> -------
> 
> Manual testing done, if no kdc_hosts is mentioned then validation exception will be thrown during blueprint registration.
> 
> 
> Thanks,
> 
> Amruta Borkar
> 
>


Re: Review Request 57563: Kerberos principal creation fails during blueprint install when kdc_hosts is not specified in blueprint

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

> On April 3, 2017, 6:11 p.m., Robert Nettleton wrote:
> > Thanks for providing this patch, but I don't really think that this is the correct way to solve this problem.
> > 
> > The Blueprint export shouldn't really modify any configuration in the cluster.  The export process may filter out some information (passwords, hostnames for un-managed services, etc), but no configuration property changes should be made. 
> > 
> > In the future, it would be great if Blueprints provided a validation step for the configuration in a Blueprint/Cluster Creation template, as that would resolve the initial issue described.  Unfortunately, such a framework does not yet exist in Blueprints, and would require refactoring of the stack definitions, such that the Blueprint processor could validate configuration properties in a generic fashion.

Hello Robert, 
Thanks for the feedback. Would it be a good idea to handle the validation mentioned by Robert Levas in BlueprintValidatorImpl ?


- Amruta


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


On April 3, 2017, 4:42 p.m., Amruta Borkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57563/
> -----------------------------------------------------------
> 
> (Updated April 3, 2017, 4:42 p.m.)
> 
> 
> Review request for Ambari, Di Li and Robert Levas.
> 
> 
> Bugs: AMBARI-20386
>     https://issues.apache.org/jira/browse/AMBARI-20386
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Blueprint validation does not prompt if kdc_hosts is not specified in the blueprint. After service installation, deployment fails during Create Principal stage. As /etc/krb5.conf is overwritten with blank values.
> 
> 
> 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/57563/diff/2/
> 
> 
> Testing
> -------
> 
> Manual testing done, if no kdc_hosts is mentioned then validation exception will be thrown during blueprint registration.
> 
> 
> Thanks,
> 
> Amruta Borkar
> 
>


Re: Review Request 57563: Kerberos principal creation fails during blueprint install when kdc_hosts is not specified in blueprint

Posted by Robert Nettleton <rn...@hortonworks.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/57563/#review170908
-----------------------------------------------------------



Thanks for providing this patch, but I don't really think that this is the correct way to solve this problem.

The Blueprint export shouldn't really modify any configuration in the cluster.  The export process may filter out some information (passwords, hostnames for un-managed services, etc), but no configuration property changes should be made. 

In the future, it would be great if Blueprints provided a validation step for the configuration in a Blueprint/Cluster Creation template, as that would resolve the initial issue described.  Unfortunately, such a framework does not yet exist in Blueprints, and would require refactoring of the stack definitions, such that the Blueprint processor could validate configuration properties in a generic fashion.


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

    I agree with the previous issues raised about this method.  
    
    The property should not be set to false merely to export a Blueprint.
    
    While it is true that the kdc_host is stripped out of an exported Blueprint, the other Kerberos configuration should remain intact, so that the exported Blueprint could be used more portably.


- Robert Nettleton


On April 3, 2017, 4:42 p.m., Amruta Borkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57563/
> -----------------------------------------------------------
> 
> (Updated April 3, 2017, 4:42 p.m.)
> 
> 
> Review request for Ambari, Di Li and Robert Levas.
> 
> 
> Bugs: AMBARI-20386
>     https://issues.apache.org/jira/browse/AMBARI-20386
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Blueprint validation does not prompt if kdc_hosts is not specified in the blueprint. After service installation, deployment fails during Create Principal stage. As /etc/krb5.conf is overwritten with blank values.
> 
> 
> 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/57563/diff/2/
> 
> 
> Testing
> -------
> 
> Manual testing done, if no kdc_hosts is mentioned then validation exception will be thrown during blueprint registration.
> 
> 
> Thanks,
> 
> Amruta Borkar
> 
>


Re: Review Request 57563: Kerberos principal creation fails during blueprint install when kdc_hosts is not specified in blueprint

Posted by Di Li <di...@ca.ibm.com>.

> On April 3, 2017, 4:49 p.m., Di Li wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
> > Lines 931 (patched)
> > <https://reviews.apache.org/r/57563/diff/2/?file=1683692#file1683692line931>
> >
> >     Does this mean bp export will always set it to false ?
> 
> Amruta Borkar wrote:
>     yes, it will always be exported as false.

I don't think it should though. Robert raised the same issue on this one.


- Di


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


On April 3, 2017, 4:42 p.m., Amruta Borkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57563/
> -----------------------------------------------------------
> 
> (Updated April 3, 2017, 4:42 p.m.)
> 
> 
> Review request for Ambari, Di Li and Robert Levas.
> 
> 
> Bugs: AMBARI-20386
>     https://issues.apache.org/jira/browse/AMBARI-20386
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Blueprint validation does not prompt if kdc_hosts is not specified in the blueprint. After service installation, deployment fails during Create Principal stage. As /etc/krb5.conf is overwritten with blank values.
> 
> 
> 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/57563/diff/2/
> 
> 
> Testing
> -------
> 
> Manual testing done, if no kdc_hosts is mentioned then validation exception will be thrown during blueprint registration.
> 
> 
> Thanks,
> 
> Amruta Borkar
> 
>


Re: Review Request 57563: Kerberos principal creation fails during blueprint install when kdc_hosts is not specified in blueprint

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

> On April 3, 2017, 4:49 p.m., Di Li wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
> > Lines 931 (patched)
> > <https://reviews.apache.org/r/57563/diff/2/?file=1683692#file1683692line931>
> >
> >     Does this mean bp export will always set it to false ?

yes, it will always be exported as false.


> On April 3, 2017, 4:49 p.m., Di Li wrote:
> > ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
> > Line 21 (original), 21 (patched)
> > <https://reviews.apache.org/r/57563/diff/2/?file=1683694#file1683694line21>
> >
> >     why change the junit classes imported ?

junit.framework.Assert.assertEquals; is deprecated, I saw it is replaced by 'org.junit.Assert.assertEquals;' in another class, therefore I made same changes in this class.


- Amruta


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


On April 3, 2017, 4:42 p.m., Amruta Borkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57563/
> -----------------------------------------------------------
> 
> (Updated April 3, 2017, 4:42 p.m.)
> 
> 
> Review request for Ambari, Di Li and Robert Levas.
> 
> 
> Bugs: AMBARI-20386
>     https://issues.apache.org/jira/browse/AMBARI-20386
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Blueprint validation does not prompt if kdc_hosts is not specified in the blueprint. After service installation, deployment fails during Create Principal stage. As /etc/krb5.conf is overwritten with blank values.
> 
> 
> 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/57563/diff/2/
> 
> 
> Testing
> -------
> 
> Manual testing done, if no kdc_hosts is mentioned then validation exception will be thrown during blueprint registration.
> 
> 
> Thanks,
> 
> Amruta Borkar
> 
>


Re: Review Request 57563: Kerberos principal creation fails during blueprint install when kdc_hosts is not specified in blueprint

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




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

    Does this mean bp export will always set it to false ?



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

    why change the junit classes imported ?


- Di Li


On April 3, 2017, 4:42 p.m., Amruta Borkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57563/
> -----------------------------------------------------------
> 
> (Updated April 3, 2017, 4:42 p.m.)
> 
> 
> Review request for Ambari, Di Li and Robert Levas.
> 
> 
> Bugs: AMBARI-20386
>     https://issues.apache.org/jira/browse/AMBARI-20386
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Blueprint validation does not prompt if kdc_hosts is not specified in the blueprint. After service installation, deployment fails during Create Principal stage. As /etc/krb5.conf is overwritten with blank values.
> 
> 
> 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/57563/diff/2/
> 
> 
> Testing
> -------
> 
> Manual testing done, if no kdc_hosts is mentioned then validation exception will be thrown during blueprint registration.
> 
> 
> Thanks,
> 
> Amruta Borkar
> 
>


Re: Review Request 57563: Kerberos principal creation fails during blueprint install when kdc_hosts is not specified in blueprint

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




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

    This should not be set by the BP processor.  
    
    Maybe what you want to do is if `krb5-conf/manage_krb5_conf` is `true`, then fail if `kerberos-env/kcd_hosts` is empty. I am not sure if this check is possible.


- Robert Levas


On April 3, 2017, 12:42 p.m., Amruta Borkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/57563/
> -----------------------------------------------------------
> 
> (Updated April 3, 2017, 12:42 p.m.)
> 
> 
> Review request for Ambari, Di Li and Robert Levas.
> 
> 
> Bugs: AMBARI-20386
>     https://issues.apache.org/jira/browse/AMBARI-20386
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> Blueprint validation does not prompt if kdc_hosts is not specified in the blueprint. After service installation, deployment fails during Create Principal stage. As /etc/krb5.conf is overwritten with blank values.
> 
> 
> 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/57563/diff/2/
> 
> 
> Testing
> -------
> 
> Manual testing done, if no kdc_hosts is mentioned then validation exception will be thrown during blueprint registration.
> 
> 
> Thanks,
> 
> Amruta Borkar
> 
>