Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Anita Jebaraj]

> On Aug. 18, 2016, 9:11 p.m., Nate Cole wrote:
> > What is the status of this review?  If it has been pushed, please mark it 
> > as Submitted.

The patch was reverted, I will close this review as discarded and open a new 
Review Request for the new patch.


- Anita


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


On Aug. 3, 2016, 8:47 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated Aug. 3, 2016, 8:47 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, Robert Levas, and Sriharsha Chintalapani.
> 
> 
> 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
> 
> 
> The patch was reverted due to a conflict, Based on Robert's comment there are 
> two versions of kerberos.json, 
> 
> …/stacks/HDP/2.5/services/KAFKA/kerberos.json
> …/resources/common-services/KAFKA/0.9.0/kerberos.json
> 
> The new version …/stacks/HDP/2.5/services/KAFKA/kerberos.json (AMBARI-17902) 
> has been added after the patch was committed for this review. I have included 
> the changes in the updated patch.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java
>  66be3bf 
>   ambari-server/src/main/resources/common-services/KAFKA/0.10.0/kerberos.json 
> e1e6461 
>   
> 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
>  Tested in Ambari UI, by enabling kerberos, listeners protocol is updated and 
> kafka started successfully
> 
> 
> Thanks,
> 
> Anita Jebaraj
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Robert Levas]

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


Ship it!




Ship It!

- Robert Levas


On Aug. 3, 2016, 4:47 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated Aug. 3, 2016, 4:47 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, Robert Levas, and Sriharsha Chintalapani.
> 
> 
> 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
> 
> 
> The patch was reverted due to a conflict, Based on Robert's comment there are 
> two versions of kerberos.json, 
> 
> …/stacks/HDP/2.5/services/KAFKA/kerberos.json
> …/resources/common-services/KAFKA/0.9.0/kerberos.json
> 
> The new version …/stacks/HDP/2.5/services/KAFKA/kerberos.json (AMBARI-17902) 
> has been added after the patch was committed for this review. I have included 
> the changes in the updated patch.
> 
> 
> Diffs
> -
> 
>   
> ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java
>  66be3bf 
>   ambari-server/src/main/resources/common-services/KAFKA/0.10.0/kerberos.json 
> e1e6461 
>   
> 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
>  Tested in Ambari UI, by enabling kerberos, listeners protocol is updated and 
> kafka started successfully
> 
> 
> Thanks,
> 
> Anita Jebaraj
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Anita Jebaraj]

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

(Updated Aug. 3, 2016, 8:47 p.m.)


Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
Cole, Robert Levas, and Sriharsha Chintalapani.


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


Repository: ambari


Description (updated)
---

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


The patch was reverted due to a conflict, Based on Robert's comment there are 
two versions of kerberos.json, 

…/stacks/HDP/2.5/services/KAFKA/kerberos.json
…/resources/common-services/KAFKA/0.9.0/kerberos.json

The new version …/stacks/HDP/2.5/services/KAFKA/kerberos.json (AMBARI-17902) 
has been added after the patch was committed for this review. I have included 
the changes in the updated patch.


Diffs (updated)
-

  
ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java
 66be3bf 
  ambari-server/src/main/resources/common-services/KAFKA/0.10.0/kerberos.json 
e1e6461 
  
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 (updated)
---

Added 1 new test case,
 Ran mvn test
 Tested in Ambari UI, by enabling kerberos, listeners protocol is updated and 
kafka started successfully


Thanks,

Anita Jebaraj

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Jonathan Hurley]

> On July 26, 2016, 4:41 p.m., Robert Levas wrote:
> > Committed to trunk
> > ```
> > commit 124f48ef899fddb6bdb96ebea9aa3a6a1a6adbca
> > Author: Anita Jebaraj 
> > Date:   Tue Jul 26 16:25:30 2016 -0400
> > ```
> > 
> > Committed to branch-2.4
> > ```
> > commit eb4f14b6bcb0208cc3d3764bbd31e2125687c517
> > Author: Anita Jebaraj 
> > Date:   Tue Jul 26 16:38:55 2016 -0400
> > ```
> 
> Anita Jebaraj wrote:
> Thank you Robert

Please close out this review, Anita.


- Jonathan


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


On July 26, 2016, 2:15 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 26, 2016, 2:15 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Anita Jebaraj]

> On July 26, 2016, 8:41 p.m., Robert Levas wrote:
> > Committed to trunk
> > ```
> > commit 124f48ef899fddb6bdb96ebea9aa3a6a1a6adbca
> > Author: Anita Jebaraj 
> > Date:   Tue Jul 26 16:25:30 2016 -0400
> > ```
> > 
> > Committed to branch-2.4
> > ```
> > commit eb4f14b6bcb0208cc3d3764bbd31e2125687c517
> > Author: Anita Jebaraj 
> > Date:   Tue Jul 26 16:38:55 2016 -0400
> > ```

Thank you Robert


- Anita


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


On July 26, 2016, 6:15 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 26, 2016, 6:15 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Robert Levas]

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



Committed to trunk
```
commit 124f48ef899fddb6bdb96ebea9aa3a6a1a6adbca
Author: Anita Jebaraj 
Date:   Tue Jul 26 16:25:30 2016 -0400
```

Committed to branch-2.4
```
commit eb4f14b6bcb0208cc3d3764bbd31e2125687c517
Author: Anita Jebaraj 
Date:   Tue Jul 26 16:38:55 2016 -0400
```

- Robert Levas


On July 26, 2016, 2:15 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 26, 2016, 2:15 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Jonathan Hurley]

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


Ship it!




Ship It!

- Jonathan Hurley


On July 26, 2016, 2:15 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 26, 2016, 2:15 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Anita Jebaraj]

> On July 26, 2016, 8:09 p.m., Nate Cole wrote:
> > Ship It!

Thank you Nate


- Anita


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


On July 26, 2016, 6:15 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 26, 2016, 6:15 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Anita Jebaraj]

> On July 25, 2016, 10:21 p.m., Robert Levas wrote:
> > Ship It!
> 
> Robert Levas wrote:
> Thanks for making those changes.
> 
> Anita Jebaraj wrote:
> can you please help in pushing the code
> 
> Robert Levas wrote:
> sure... working on it now.
> 
> Robert Levas wrote:
> Actually, we only have 1 +1.  Can you add ncole, jhurley, and afernandez 
> to see if you can get another +1?
> 
> Anita Jebaraj wrote:
> Sure, I have added Johathan hurley.
> 
> Anita Jebaraj wrote:
> I have added Alex and Nate as well

Hi Robert, can you please help in pushing the code


- Anita


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


On July 26, 2016, 6:15 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 26, 2016, 6:15 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Nate Cole]

> On July 26, 2016, 11:18 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java,
> >  line 251
> > 
> >
> > May want to try/catch around these regex things in case they're bad.  
> > Log the error and return something which will be non-breaking ("" 
> > sufficient?)
> 
> Robert Levas wrote:
> I would prefer the exception to be thrown in the case so the issue can be 
> found during testing. It is unlikely users will use this facility when 
> setting the Kerberos descriptor... and if they do, the error should be made 
> ovbious to them, rather than hide the issue with some "default" value.
> 
> Anita Jebaraj wrote:
> Hi Nate, as per Robert's comments I think it will be best if the error is 
> thrown when the regex value is modified with a bad value by the user, rather 
> than handling it with a default value. Please give your comments, I will 
> update the patch accordingly. Thanks in advance
> 
> Nate Cole wrote:
> @Rob - ok by me.  If the data is null, should that be an 
> IllegalArgumentException then?
> 
> Robert Levas wrote:
> @Nate,  In other _functions_, null data was treated like "" and thus "" 
> was returned.  I am ok with that since this is unlikely and protected by the 
> infrastucture. 
> 
> 
> org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java:109
> '''
> if (replacement != null) {   // --> THIS IS the data variable <--
>   if (function != null) {
> replacement = applyReplacementFunction(function, replacement);
>   }
> 
>   // Escape '$' and '\' so they don't cause any issues.
>   matcher.appendReplacement(sb, replacement.replace("\", 
> "\\").replace("$", "\$"));
>   replacementPerformed = true;
> }
> '''
> 
> Anita Jebaraj wrote:
> Dropping this issue, based on Robert's comment

Thanks @Rob for clarification.


- Nate


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


On July 26, 2016, 2:15 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 26, 2016, 2:15 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Nate Cole]

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


Ship it!




Ship It!

- Nate Cole


On July 26, 2016, 2:15 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 26, 2016, 2:15 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Anita Jebaraj]

> On July 26, 2016, 3:18 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java,
> >  line 251
> > 
> >
> > May want to try/catch around these regex things in case they're bad.  
> > Log the error and return something which will be non-breaking ("" 
> > sufficient?)
> 
> Robert Levas wrote:
> I would prefer the exception to be thrown in the case so the issue can be 
> found during testing. It is unlikely users will use this facility when 
> setting the Kerberos descriptor... and if they do, the error should be made 
> ovbious to them, rather than hide the issue with some "default" value.
> 
> Anita Jebaraj wrote:
> Hi Nate, as per Robert's comments I think it will be best if the error is 
> thrown when the regex value is modified with a bad value by the user, rather 
> than handling it with a default value. Please give your comments, I will 
> update the patch accordingly. Thanks in advance
> 
> Nate Cole wrote:
> @Rob - ok by me.  If the data is null, should that be an 
> IllegalArgumentException then?
> 
> Robert Levas wrote:
> @Nate,  In other _functions_, null data was treated like "" and thus "" 
> was returned.  I am ok with that since this is unlikely and protected by the 
> infrastucture. 
> 
> 
> org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java:109
> '''
> if (replacement != null) {   // --> THIS IS the data variable <--
>   if (function != null) {
> replacement = applyReplacementFunction(function, replacement);
>   }
> 
>   // Escape '$' and '\' so they don't cause any issues.
>   matcher.appendReplacement(sb, replacement.replace("\", 
> "\\").replace("$", "\$"));
>   replacementPerformed = true;
> }
> '''

Dropping this issue, based on Robert's comment


- Anita


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


On July 26, 2016, 6:15 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 26, 2016, 6:15 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Anita Jebaraj]

> On July 26, 2016, 3:18 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java,
> >  line 240
> > 
> >
> > Should be ReplaceValue (capitalize 'R')
> 
> Robert Levas wrote:
> I missed that... nice catch!

I have fixed this in the updated patch


- Anita


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


On July 26, 2016, 6:15 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 26, 2016, 6:15 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Robert Levas]

> On July 26, 2016, 11:18 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java,
> >  line 251
> > 
> >
> > May want to try/catch around these regex things in case they're bad.  
> > Log the error and return something which will be non-breaking ("" 
> > sufficient?)
> 
> Robert Levas wrote:
> I would prefer the exception to be thrown in the case so the issue can be 
> found during testing. It is unlikely users will use this facility when 
> setting the Kerberos descriptor... and if they do, the error should be made 
> ovbious to them, rather than hide the issue with some "default" value.
> 
> Anita Jebaraj wrote:
> Hi Nate, as per Robert's comments I think it will be best if the error is 
> thrown when the regex value is modified with a bad value by the user, rather 
> than handling it with a default value. Please give your comments, I will 
> update the patch accordingly. Thanks in advance
> 
> Nate Cole wrote:
> @Rob - ok by me.  If the data is null, should that be an 
> IllegalArgumentException then?

@Nate,  In other _functions_, null data was treated like "" and thus "" was 
returned.  I am ok with that since this is unlikely and protected by the 
infrastucture. 


org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java:109
'''
if (replacement != null) {   // --> THIS IS the data variable <--
  if (function != null) {
replacement = applyReplacementFunction(function, replacement);
  }

  // Escape '$' and '\' so they don't cause any issues.
  matcher.appendReplacement(sb, replacement.replace("\", "\\").replace("$", 
"\$"));
  replacementPerformed = true;
}
'''


- Robert


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


On July 26, 2016, 11:06 a.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 26, 2016, 11:06 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Nate Cole]

> On July 26, 2016, 11:18 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java,
> >  line 251
> > 
> >
> > May want to try/catch around these regex things in case they're bad.  
> > Log the error and return something which will be non-breaking ("" 
> > sufficient?)
> 
> Robert Levas wrote:
> I would prefer the exception to be thrown in the case so the issue can be 
> found during testing. It is unlikely users will use this facility when 
> setting the Kerberos descriptor... and if they do, the error should be made 
> ovbious to them, rather than hide the issue with some "default" value.
> 
> Anita Jebaraj wrote:
> Hi Nate, as per Robert's comments I think it will be best if the error is 
> thrown when the regex value is modified with a bad value by the user, rather 
> than handling it with a default value. Please give your comments, I will 
> update the patch accordingly. Thanks in advance

@Rob - ok by me.  If the data is null, should that be an 
IllegalArgumentException then?


- Nate


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


On July 26, 2016, 11:06 a.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 26, 2016, 11:06 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Nate Cole]

> On July 26, 2016, 11:18 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java,
> >  line 248
> > 
> >
> > Use StringBuilder, which doesn't incur synchronization penalties
> 
> Robert Levas wrote:
> True, but Matcher.appendReplacement is defined as 
> `java.util.regex.Matcher#appendReplacement(StringBuffer sb, String 
> replacement)`.
> 
> Robert Levas wrote:
> True, but Matcher.appendReplacement is defined as 
> `java.util.regex.Matcher#appendReplacement(StringBuffer sb, String 
> replacement)`.
> 
> Anita Jebaraj wrote:
> Hi Nate, as Robert has mentioned appendReplacement takes in StringBuffer, 
> hence continuing with that.

Ah, ok


- Nate


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


On July 26, 2016, 11:06 a.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 26, 2016, 11:06 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Anita Jebaraj]

> On July 26, 2016, 3:18 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java,
> >  line 248
> > 
> >
> > Use StringBuilder, which doesn't incur synchronization penalties
> 
> Robert Levas wrote:
> True, but Matcher.appendReplacement is defined as 
> `java.util.regex.Matcher#appendReplacement(StringBuffer sb, String 
> replacement)`.
> 
> Robert Levas wrote:
> True, but Matcher.appendReplacement is defined as 
> `java.util.regex.Matcher#appendReplacement(StringBuffer sb, String 
> replacement)`.

Hi Nate, as Robert has mentioned appendReplacement takes in StringBuffer, hence 
continuing with that.


> On July 26, 2016, 3:18 p.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java,
> >  line 251
> > 
> >
> > May want to try/catch around these regex things in case they're bad.  
> > Log the error and return something which will be non-breaking ("" 
> > sufficient?)
> 
> Robert Levas wrote:
> I would prefer the exception to be thrown in the case so the issue can be 
> found during testing. It is unlikely users will use this facility when 
> setting the Kerberos descriptor... and if they do, the error should be made 
> ovbious to them, rather than hide the issue with some "default" value.

Hi Nate, as per Robert's comments I think it will be best if the error is 
thrown when the regex value is modified with a bad value by the user, rather 
than handling it with a default value. Please give your comments, I will update 
the patch accordingly. Thanks in advance


- Anita


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


On July 26, 2016, 3:06 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 26, 2016, 3:06 p.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Robert Levas]

> On July 26, 2016, 11:18 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java,
> >  line 248
> > 
> >
> > Use StringBuilder, which doesn't incur synchronization penalties
> 
> Robert Levas wrote:
> True, but Matcher.appendReplacement is defined as 
> `java.util.regex.Matcher#appendReplacement(StringBuffer sb, String 
> replacement)`.

True, but Matcher.appendReplacement is defined as 
`java.util.regex.Matcher#appendReplacement(StringBuffer sb, String 
replacement)`.


- Robert


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


On July 26, 2016, 11:06 a.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 26, 2016, 11:06 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Robert Levas]

> On July 26, 2016, 11:18 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java,
> >  line 248
> > 
> >
> > Use StringBuilder, which doesn't incur synchronization penalties

True, but Matcher.appendReplacement is defined as 
`java.util.regex.Matcher#appendReplacement(StringBuffer sb, String 
replacement)`.


> On July 26, 2016, 11:18 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java,
> >  line 240
> > 
> >
> > Should be ReplaceValue (capitalize 'R')

I missed that... nice catch!


> On July 26, 2016, 11:18 a.m., Nate Cole wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java,
> >  line 251
> > 
> >
> > May want to try/catch around these regex things in case they're bad.  
> > Log the error and return something which will be non-breaking ("" 
> > sufficient?)

I would prefer the exception to be thrown in the case so the issue can be found 
during testing. It is unlikely users will use this facility when setting the 
Kerberos descriptor... and if they do, the error should be made ovbious to 
them, rather than hide the issue with some "default" value.


- Robert


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


On July 26, 2016, 11:06 a.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 26, 2016, 11:06 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Nate Cole]

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




ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java
 (line 238)


Should be ReplaceValue (capitalize 'R')



ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java
 (line 246)


Use StringBuilder, which doesn't incur synchronization penalties



ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java
 (line 249)


May want to try/catch around these regex things in case they're bad.  Log 
the error and return something which will be non-breaking ("" sufficient?)


- Nate Cole


On July 26, 2016, 11:06 a.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 26, 2016, 11:06 a.m.)
> 
> 
> Review request for Ambari, Alejandro Fernandez, Di Li, Jonathan Hurley, Nate 
> Cole, 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Anita Jebaraj]

> On July 25, 2016, 10:21 p.m., Robert Levas wrote:
> > Ship It!
> 
> Robert Levas wrote:
> Thanks for making those changes.
> 
> Anita Jebaraj wrote:
> can you please help in pushing the code
> 
> Robert Levas wrote:
> sure... working on it now.
> 
> Robert Levas wrote:
> Actually, we only have 1 +1.  Can you add ncole, jhurley, and afernandez 
> to see if you can get another +1?
> 
> Anita Jebaraj wrote:
> Sure, I have added Johathan hurley.

I have added Alex and Nate as well


- Anita


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


On July 26, 2016, 3:02 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 26, 2016, 3:02 p.m.)
> 
> 
> Review request for Ambari, Di Li, Jonathan Hurley, 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Robert Levas]

> On July 25, 2016, 6:21 p.m., Robert Levas wrote:
> > Ship It!
> 
> Robert Levas wrote:
> Thanks for making those changes.
> 
> Anita Jebaraj wrote:
> can you please help in pushing the code
> 
> Robert Levas wrote:
> sure... working on it now.

Actually, we only have 1 +1.  Can you add ncole, jhurley, and afernandez to see 
if you can get another +1?


- Robert


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


On July 25, 2016, 5: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, 5: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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Anita Jebaraj]

> On July 25, 2016, 10:21 p.m., Robert Levas wrote:
> > Ship It!
> 
> Robert Levas wrote:
> Thanks for making those changes.

can you please help in pushing the code


- Anita


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


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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Robert Levas]

> On July 25, 2016, 6:21 p.m., Robert Levas wrote:
> > Ship It!

Thanks for making those changes.


- Robert


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


On July 25, 2016, 5: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, 5: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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Robert Levas]

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


Ship it!




Ship It!

- Robert Levas


On July 25, 2016, 5: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, 5: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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Anita Jebaraj]

> 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
> > 
> >
> > 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Anita Jebaraj]

---
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 (updated)
-

  
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

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Robert Levas]

> On July 21, 2016, 2:30 p.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java,
> >  line 242
> > 
> >
> > 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`

>From the above comment:

```
"listeners": "${kafka-broker/listeners|replace(\PLAINTEXT\b, PLAINTEXTSASL)}"
```

Should have been 

```
"listeners": "${kafka-broker/listeners|replace(\bPLAINTEXT\b, PLAINTEXTSASL)}"
```


- Robert


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


On July 20, 2016, 6:01 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 20, 2016, 6:01 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Robert Levas]

> On July 21, 2016, 2:30 p.m., Robert Levas wrote:
> > ambari-server/src/main/java/org/apache/ambari/server/state/kerberos/VariableReplacementHelper.java,
> >  line 242
> > 
> >
> > 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.

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


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


On July 20, 2016, 6:01 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 20, 2016, 6:01 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Anita Jebaraj]

> 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
> > 
> >
> > Why is 'SASL' hardcoded here?

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.


- Anita


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


On July 20, 2016, 10:01 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 20, 2016, 10:01 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Anita Jebaraj]

> On July 18, 2016, 10:58 a.m., Robert Levas wrote:
> > ambari-server/src/main/resources/common-services/KAFKA/0.9.0/kerberos.json, 
> > line 18
> > 
> >
> > This is redundant...
> > 
> > `kafka-broker/listeners` = `kafka-broker/listeners`
> 
> Anita Jebaraj wrote:
> Hi Robert, I had to include the declaration for listeners in 
> kerberos.json since the value for listeners wont be displayed in the UI under 
> the kerberos configuration otherwise. There is a change in value for protocol 
> for listeners in specific cases (if the user hasn't added a listener with 
> PLAINTEXTSASL protocol) and I thought it would be appropriate to include it 
> in kerberos.json, even though the handling is done in the stack-advisor.py
> 
> Robert Levas wrote:
> In this case, maybe it is better to do
> 
> ```
> "listeners": ""
> ```
> 
> Else you may wind up with `kafka-broker/listeners` being set to 
> `${kafka-broker/listeners}` (literally).
> 
> Anita Jebaraj wrote:
> I am not sure if assigning an empty value for listeners in kerberos.json 
> is the right approach, I am thinking if using functions similar to each() in 
> templeton.hive.properties in VariableReplacementHelper.java would work out 
> for this scenario. May be creating a function replace() for listeners will 
> help, it can be {kafka-broker/listeners|replace()}. Please give me your 
> suggestion on this.
> 
> Robert Levas wrote:
> I think `replace()` will be ok.  Then it will be something like:
> 
> ```
> "listeners": "${kafka-broker/listeners|replace(PLAINTEXT, PLAINTEXTSASL)}"
> ```

Hi 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/#review142546
---


On July 20, 2016, 10:01 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 20, 2016, 10:01 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
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Anita Jebaraj]

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

(Updated July 20, 2016, 10:01 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 (updated)
-

  
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

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Anita Jebaraj]

> On July 18, 2016, 10:58 a.m., Robert Levas wrote:
> > ambari-server/src/main/resources/common-services/KAFKA/0.9.0/kerberos.json, 
> > line 18
> > 
> >
> > This is redundant...
> > 
> > `kafka-broker/listeners` = `kafka-broker/listeners`
> 
> Anita Jebaraj wrote:
> Hi Robert, I had to include the declaration for listeners in 
> kerberos.json since the value for listeners wont be displayed in the UI under 
> the kerberos configuration otherwise. There is a change in value for protocol 
> for listeners in specific cases (if the user hasn't added a listener with 
> PLAINTEXTSASL protocol) and I thought it would be appropriate to include it 
> in kerberos.json, even though the handling is done in the stack-advisor.py
> 
> Robert Levas wrote:
> In this case, maybe it is better to do
> 
> ```
> "listeners": ""
> ```
> 
> Else you may wind up with `kafka-broker/listeners` being set to 
> `${kafka-broker/listeners}` (literally).

I am not sure if assigning an empty value for listeners in kerberos.json is the 
right approach, I am thinking if using functions similar to each() in 
templeton.hive.properties in VariableReplacementHelper.java would work out for 
this scenario. May be creating a function replace() for listeners will help, it 
can be {kafka-broker/listeners|replace()}. Please give me your suggestion on 
this.


- Anita


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


On July 14, 2016, 9:08 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 14, 2016, 9:08 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/resources/common-services/KAFKA/0.8.1/package/scripts/kafka.py
>  33275f9 
>   ambari-server/src/main/resources/common-services/KAFKA/0.9.0/kerberos.json 
> 2b1c01b 
>   ambari-server/src/main/resources/stacks/HDP/2.3/services/stack_advisor.py 
> 2a2a3a3 
>   ambari-server/src/test/python/stacks/2.3/common/test_stack_advisor.py 
> a6baeea 
> 
> Diff: https://reviews.apache.org/r/50047/diff/
> 
> 
> Testing
> ---
> 
> Added 1 new test case,
>  Ran mvn test
> 
> 
> Thanks,
> 
> Anita Jebaraj
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Robert Levas]

> On July 18, 2016, 6:58 a.m., Robert Levas wrote:
> > ambari-server/src/main/resources/common-services/KAFKA/0.9.0/kerberos.json, 
> > line 18
> > 
> >
> > This is redundant...
> > 
> > `kafka-broker/listeners` = `kafka-broker/listeners`
> 
> Anita Jebaraj wrote:
> Hi Robert, I had to include the declaration for listeners in 
> kerberos.json since the value for listeners wont be displayed in the UI under 
> the kerberos configuration otherwise. There is a change in value for protocol 
> for listeners in specific cases (if the user hasn't added a listener with 
> PLAINTEXTSASL protocol) and I thought it would be appropriate to include it 
> in kerberos.json, even though the handling is done in the stack-advisor.py

In this case, maybe it is better to do

```
"listeners": ""
```

Else you may wind up with `kafka-broker/listeners` being set to 
`${kafka-broker/listeners}` (literally).


- Robert


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


On July 14, 2016, 5:08 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 14, 2016, 5:08 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/resources/common-services/KAFKA/0.8.1/package/scripts/kafka.py
>  33275f9 
>   ambari-server/src/main/resources/common-services/KAFKA/0.9.0/kerberos.json 
> 2b1c01b 
>   ambari-server/src/main/resources/stacks/HDP/2.3/services/stack_advisor.py 
> 2a2a3a3 
>   ambari-server/src/test/python/stacks/2.3/common/test_stack_advisor.py 
> a6baeea 
> 
> Diff: https://reviews.apache.org/r/50047/diff/
> 
> 
> Testing
> ---
> 
> Added 1 new test case,
>  Ran mvn test
> 
> 
> Thanks,
> 
> Anita Jebaraj
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Anita Jebaraj]

> On July 18, 2016, 10:58 a.m., Robert Levas wrote:
> > ambari-server/src/main/resources/common-services/KAFKA/0.9.0/kerberos.json, 
> > line 18
> > 
> >
> > This is redundant...
> > 
> > `kafka-broker/listeners` = `kafka-broker/listeners`

Hi Robert, I had to include the declaration for listeners in kerberos.json 
since the value for listeners wont be displayed in the UI under the kerberos 
configuration otherwise. There is a change in value for protocol for listeners 
in specific cases (if the user hasn't added a listener with PLAINTEXTSASL 
protocol) and I thought it would be appropriate to include it in kerberos.json, 
even though the handling is done in the stack-advisor.py


- Anita


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


On July 14, 2016, 9:08 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 14, 2016, 9:08 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/resources/common-services/KAFKA/0.8.1/package/scripts/kafka.py
>  33275f9 
>   ambari-server/src/main/resources/common-services/KAFKA/0.9.0/kerberos.json 
> 2b1c01b 
>   ambari-server/src/main/resources/stacks/HDP/2.3/services/stack_advisor.py 
> 2a2a3a3 
>   ambari-server/src/test/python/stacks/2.3/common/test_stack_advisor.py 
> a6baeea 
> 
> Diff: https://reviews.apache.org/r/50047/diff/
> 
> 
> Testing
> ---
> 
> Added 1 new test case,
>  Ran mvn test
> 
> 
> Thanks,
> 
> Anita Jebaraj
> 
>

Re: Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Robert Levas]

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




ambari-server/src/main/resources/common-services/KAFKA/0.9.0/kerberos.json 
(line 18)


This is redundant...

`kafka-broker/listeners` = `kafka-broker/listeners`


- Robert Levas


On July 14, 2016, 5:08 p.m., Anita Jebaraj wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/50047/
> ---
> 
> (Updated July 14, 2016, 5:08 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/resources/common-services/KAFKA/0.8.1/package/scripts/kafka.py
>  33275f9 
>   ambari-server/src/main/resources/common-services/KAFKA/0.9.0/kerberos.json 
> 2b1c01b 
>   ambari-server/src/main/resources/stacks/HDP/2.3/services/stack_advisor.py 
> 2a2a3a3 
>   ambari-server/src/test/python/stacks/2.3/common/test_stack_advisor.py 
> a6baeea 
> 
> Diff: https://reviews.apache.org/r/50047/diff/
> 
> 
> Testing
> ---
> 
> Added 1 new test case,
>  Ran mvn test
> 
> 
> Thanks,
> 
> Anita Jebaraj
> 
>

Review Request 50047: Kafka listeners property does not show SASL_PLAINTEXT protocol when Kerberos is enabled

[Anita Jebaraj]

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

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/resources/common-services/KAFKA/0.8.1/package/scripts/kafka.py
 33275f9 
  ambari-server/src/main/resources/common-services/KAFKA/0.9.0/kerberos.json 
2b1c01b 
  ambari-server/src/main/resources/stacks/HDP/2.3/services/stack_advisor.py 
2a2a3a3 
  ambari-server/src/test/python/stacks/2.3/common/test_stack_advisor.py a6baeea 

Diff: https://reviews.apache.org/r/50047/diff/


Testing
---

Added 1 new test case,
 Ran mvn test


Thanks,

Anita Jebaraj