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



Thanks for providing this patch, but I believe an alternate approach should be 
explored. 

Please see the issue I've raised below. 

Rather than replacing the existing filter, I'd recommend implementing an 
additional filter that can be registered to handle this special case.

I also wanted to mention that the "property-type" of a given property is 
accessible from the BlueprintConfigurationProcessor.

The following class encapsulates the stack metadata for a given stack:

org.apache.ambari.server.controller.internal.Stack

That class contains a convenience method to determine if a property is a 
password:

org.apache.ambari.server.controller.internal.Stack#isPasswordProperty

It might be useful to implement a new filter that just checks the 
isPasswordProperty() method, and filters based on that.  This filter should be 
implemented in addition to the existing name-based filter, and not necessarily 
as a replacement for it. 

This special handling code is necessary, due to the fact that the Ambari Stack 
metadata does not provide a way to model "conditional properties", such as the 
Ranger password properties mentioned here, that are only required in an HTTPS 
deployment scenario. 

Thanks.


ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
 (line 2663)
<https://reviews.apache.org/r/44725/#comment186637>

    I don't think this approach will work generally. 
    
    I've double-checked the code, and it looks like the SecretReference code 
does not necessarily always update the configuration prior to a Blueprint 
export attempt.  
    
    The problem is that the definition of a "password" is somewhat vague 
sometimes in the Ambari Stacks.  There are several types of properties that 
qualify as passwords:
    
    1. Properties marked with the "password" type, and are required as input 
prior to cluster creation.  These are automatically filtered out of an export. 
    2. Properties marked with the "password" type, but not marked as required 
input.  These usually have default values as well, such as the Ranger password 
properties that are only valid in an HTTPS deployment. These are filtered out 
by the current PasswordPropertyFilter. 
    3. Any properties added to the stack that may be passwords, but are 
incorrectly marked as not being passwords in the metadata. 
    
    The BlueprintConfigurationProcessor must make a best effort to handle all 
these types, and that is why the Filter pattern was adopted a few releases ago.


- Robert Nettleton


On March 17, 2016, 6:52 p.m., Amruta Borkar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/44725/
> -----------------------------------------------------------
> 
> (Updated March 17, 2016, 6:52 p.m.)
> 
> 
> Review request for Ambari, Di Li and Robert Nettleton.
> 
> 
> Bugs: AMBARI-15338
>     https://issues.apache.org/jira/browse/AMBARI-15338
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> After exporting blueprint from ranger enabled cluster 
> ranger.service.https.attrib.keystore.pass is exported.
> Which needs to be removed before using the same blueprint to create another 
> cluster
> Error Show when used same blueprint:
> { "status" : 400, "message" : "Blueprint configuration validation failed: 
> Secret references are not allowed in blueprints, replace following properties 
> with real passwords:\n Config:ranger-admin-site 
> Property:ranger.service.https.attrib.keystore.pass\n" }
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessor.java
>  f5e7578 
>   
> ambari-server/src/test/java/org/apache/ambari/server/controller/internal/BlueprintConfigurationProcessorTest.java
>  68d5755 
> 
> Diff: https://reviews.apache.org/r/44725/diff/
> 
> 
> Testing
> -------
> 
> Modified test cases to test for if the properties that end with "pass" are 
> getting filtered. Other properties which have 'pass' else where in the name 
> will not get filtered.
> 
> 
> Thanks,
> 
> Amruta Borkar
> 
>

Reply via email to