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


Ship it!




The patch looks fine to me.  

The only catch here is that these "secret" properties will likely still need to 
be set for the cluster to startup properly.  This means that the exported 
Blueprint will need to be modified in some scenarios to set the excluded 
properties to the correct values for the new cluster. This is fine, since it is 
in line with how passwords in general are filtered out of exported Blueprints. 

In the future, we might want to use some kind of token, such as the one used 
for "secret" references, to denote that certain properties need to be updated, 
either in the Blueprint or Cluster Creation Template, in order to port the 
Blueprint to a new cluster. There are other config types that would benefit 
from this as well (references to non-managed Databases are one example), so 
that we could assist the user in knowing which properties need to be updated 
with an exported Blueprint.

That being said, since we don't yet handle this situation of properties that 
need to be modified for portability, the current patch should not be held up, 
since it is in line with how the current code works. 

Thanks for providing this patch!

- Robert Nettleton


On Feb. 10, 2016, 7:37 p.m., Amruta Borkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/43281/
> -----------------------------------------------------------
> 
> (Updated Feb. 10, 2016, 7:37 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, and Robert Nettleton.
> 
> 
> Bugs: AMBARI-14885
>     https://issues.apache.org/jira/browse/AMBARI-14885
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> AMBARI-14885: After exporting blueprint from existing cluster 
> knox_master_secret is exported. This causes error while registering the 
> blueprint. 
> Following error occurs while using and exported blueprint if a password or 
> secret property is present in the blueprint:
> { "status" : 400, "message" : "Blueprint configuration validation failed: 
> Secret references are not allowed in blueprints, replace following properties 
> with real passwords:\n Config:knox-env Property:knox_master_secret\n" }
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
>  de31a0d 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
>  7a77a25 
> 
> Diff: https://reviews.apache.org/r/43281/diff/
> 
> 
> Testing
> -------
> 
> In this patch, isPropertyIncluded() method is modified to include the 
> properties marked as secret. Corresponding test cases are also added.
>  The fix excludes properties marked as 'secret' while export blueprint. 
>  Test cases are added to test various combination by which a property can be 
> marked as 'secret'.
>  Test cases verify that the properties that do not match the rule "*SECRET" 
> are still included.
> 
> 
> Thanks,
> 
> Amruta Borkar
> 
>

Reply via email to