> On July 21, 2016, 6:30 p.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java,
> >  line 242
> > <https://reviews.apache.org/r/50047/diff/2/?file=1448426#file1448426line242>
> >
> >     Why is 'SASL' hardcoded here?
> 
> Anita Jebaraj wrote:
>     When kerberos is enabled, the listeners protocol can be PLAINTEXTSASL or 
> SASL_PLAINTEXT, in HDP PLAINTEXTSASL is used throughout, but the user can 
> still customize the protocol as SASL_PLAINTEXT, so I thought checking for the 
> text SASL would be a better approach to avoid this issue or I can add the 
> value "SASL" as third argument to replace() function.
> 
> Robert Levas wrote:
>     In this case, I think we are better off if the replace function took in a 
> regular expression...  Then you could use something like
>     
>     ```
>     "listeners": "${kafka-broker/listeners|replace(\PLAINTEXT\b, 
> PLAINTEXTSASL)}"
>     ```
>     
>     The code for the replace function could be something like:
>     
>     ```
>         public String perform(String[] args, String data) {
>           if ((args == null) || (args.length != 2)) {
>             throw new IllegalArgumentException("Invalid number of arguments 
> encountered");
>           }
>     
>           if (data != null) {
>             StringBuffer builder = new StringBuffer();
>     
>             String regex = args[0];
>             String replacement = args[1];
>     
>             Pattern pattern = Pattern.compile(regex);
>             Matcher matcher = pattern.matcher(data);
>     
>             while(matcher.find()) {
>               matcher.appendReplacement(builder, replacement);
>             }
>             matcher.appendTail(builder);
>     
>             return builder.toString();
>           }
>     
>           return "";
>         }
>     
>     ```
>     
>     NOTE: I used the above code to translate `SASL_PLAINTEXT://host1, 
> PLAINTEXT://host2` to `SASL_PLAINTEXT://host1, PLAINTEXTSASL://host2`
> 
> Robert Levas wrote:
>     From the above comment:
>     
>     ```
>     "listeners": "${kafka-broker/listeners|replace(\PLAINTEXT\b, 
> PLAINTEXTSASL)}"
>     ```
>     
>     Should have been 
>     
>     ```
>     "listeners": "${kafka-broker/listeners|replace(\bPLAINTEXT\b, 
> PLAINTEXTSASL)}"
>     ```

Thank you Robert, I have updated the patch based on your comments


- Anita


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


On July 25, 2016, 9:12 p.m., Anita Jebaraj wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> -----------------------------------------------------------
> 
> (Updated July 25, 2016, 9:12 p.m.)
> 
> 
> Review request for Ambari, Di Li and Robert Levas.
> 
> 
> Bugs: AMBARI-17694
>     https://issues.apache.org/jira/browse/AMBARI-17694
> 
> 
> Repository: ambari
> 
> 
> Description
> -------
> 
> When kerberos is enabled, the protocol for listeners in 
> /etc/kafka/conf/server.properties is updated from PLAINTEXT to PLAINTEXTSASL, 
> even though the Ambari UI shows otherwise
> 
> 
> Diffs
> -----
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java
>  66be3bf 
>   
> ambari-server/src/main/resources/common-services/KAFKA/0.8.1/package/scripts/kafka.py
>  ac7b0ae 
>   ambari-server/src/main/resources/common-services/KAFKA/0.9.0/kerberos.json 
> 2b1c01b 
>   
> ambari-server/src/test/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelperTest.java
>  ee2a671 
> 
> Diff: https://reviews.apache.org/r/50047/diff/
> 
> 
> Testing
> -------
> 
> Added 1 new test case,
>  Ran mvn test
> 
> 
> Thanks,
> 
> Anita Jebaraj
> 
>

Reply via email to