You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Jayendra Parab <ja...@gmail.com> on 2020/05/19 11:25:24 UTC
Re: Review Request 72527: Refactoring Kafka JAAS Configuration
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72527/
-----------------------------------------------------------
(Updated May 19, 2020, 11:25 a.m.)
Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
Bugs: ATLAS-3779
https://issues.apache.org/jira/browse/ATLAS-3779
Repository: atlas
Description
-------
The JAAS config set through InMemoryJAASConfiguration overwrites user supplied JAAS configuration which might be required to connect to user's Kakfa instead of Atlas's Kafka and might have different credentials
To resolve this we should make use of Kafka's dynamic JAAS configuration mechanism wherein you can set the Kafka JAAS config using sasl.jaas.config property. (https://cwiki.apache.org/confluence/display/KAFKA/KIP-85%3A+Dynamic+JAAS+configuration+for+Kafka+clients)
In this patch, we are picking up the atlas.jaas.* properties and setting the sasl.jaas.config property. Since the JAAS configuration is specific only to Kafka, the JAAS property initialization is done while instantiating the KafkaNotification object. Also, since we are using sasl.jaas.config property to set the configuration it's not conflicting with any other JAAS configuration.
To keep the code backward compatible for client application which make use of ticketBased-KafkaClient configuration , we are still making use of the earlier mechanism i.e. with the use of UserGroupInformation.isLoginTicketBased() and UserGroupInformation.isLoginKeytabBased() to set the configuration.
Diffs
-----
intg/src/main/java/org/apache/atlas/ApplicationProperties.java 3ba50612e
notification/src/main/java/org/apache/atlas/hook/AtlasHook.java cc6546be8
notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 11a29b9e0
Diff: https://reviews.apache.org/r/72527/diff/1/
Testing
-------
With these changes, tried creating table using CTAS query in Hive. The entities corresponding to the table was updated properly in Atlas server
Thanks,
Jayendra Parab
Re: Review Request 72527: Refactoring Kafka JAAS Configuration
Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72527/#review220829
-----------------------------------------------------------
Please add some unit tests
- Sarath Subramanian
On May 19, 2020, 4:25 a.m., Jayendra Parab wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72527/
> -----------------------------------------------------------
>
> (Updated May 19, 2020, 4:25 a.m.)
>
>
> Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-3779
> https://issues.apache.org/jira/browse/ATLAS-3779
>
>
> Repository: atlas
>
>
> Description
> -------
>
> The JAAS config set through InMemoryJAASConfiguration overwrites user supplied JAAS configuration which might be required to connect to user's Kakfa instead of Atlas's Kafka and might have different credentials
>
> To resolve this we should make use of Kafka's dynamic JAAS configuration mechanism wherein you can set the Kafka JAAS config using sasl.jaas.config property. (https://cwiki.apache.org/confluence/display/KAFKA/KIP-85%3A+Dynamic+JAAS+configuration+for+Kafka+clients)
>
> In this patch, we are picking up the atlas.jaas.* properties and setting the sasl.jaas.config property. Since the JAAS configuration is specific only to Kafka, the JAAS property initialization is done while instantiating the KafkaNotification object. Also, since we are using sasl.jaas.config property to set the configuration it's not conflicting with any other JAAS configuration.
>
> To keep the code backward compatible for client application which make use of ticketBased-KafkaClient configuration , we are still making use of the earlier mechanism i.e. with the use of UserGroupInformation.isLoginTicketBased() and UserGroupInformation.isLoginKeytabBased() to set the configuration.
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/ApplicationProperties.java 3ba50612e
> notification/src/main/java/org/apache/atlas/hook/AtlasHook.java cc6546be8
> notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 11a29b9e0
>
>
> Diff: https://reviews.apache.org/r/72527/diff/1/
>
>
> Testing
> -------
>
> With these changes, tried creating table using CTAS query in Hive. The entities corresponding to the table was updated properly in Atlas server
>
>
> Thanks,
>
> Jayendra Parab
>
>
Re: Review Request 72527: Refactoring Kafka JAAS Configuration
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72527/#review220827
-----------------------------------------------------------
Ship it!
Ship It!
- Madhan Neethiraj
On May 19, 2020, 11:25 a.m., Jayendra Parab wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72527/
> -----------------------------------------------------------
>
> (Updated May 19, 2020, 11:25 a.m.)
>
>
> Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-3779
> https://issues.apache.org/jira/browse/ATLAS-3779
>
>
> Repository: atlas
>
>
> Description
> -------
>
> The JAAS config set through InMemoryJAASConfiguration overwrites user supplied JAAS configuration which might be required to connect to user's Kakfa instead of Atlas's Kafka and might have different credentials
>
> To resolve this we should make use of Kafka's dynamic JAAS configuration mechanism wherein you can set the Kafka JAAS config using sasl.jaas.config property. (https://cwiki.apache.org/confluence/display/KAFKA/KIP-85%3A+Dynamic+JAAS+configuration+for+Kafka+clients)
>
> In this patch, we are picking up the atlas.jaas.* properties and setting the sasl.jaas.config property. Since the JAAS configuration is specific only to Kafka, the JAAS property initialization is done while instantiating the KafkaNotification object. Also, since we are using sasl.jaas.config property to set the configuration it's not conflicting with any other JAAS configuration.
>
> To keep the code backward compatible for client application which make use of ticketBased-KafkaClient configuration , we are still making use of the earlier mechanism i.e. with the use of UserGroupInformation.isLoginTicketBased() and UserGroupInformation.isLoginKeytabBased() to set the configuration.
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/ApplicationProperties.java 3ba50612e
> notification/src/main/java/org/apache/atlas/hook/AtlasHook.java cc6546be8
> notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 11a29b9e0
>
>
> Diff: https://reviews.apache.org/r/72527/diff/1/
>
>
> Testing
> -------
>
> With these changes, tried creating table using CTAS query in Hive. The entities corresponding to the table was updated properly in Atlas server
>
>
> Thanks,
>
> Jayendra Parab
>
>
Re: Review Request 72527: Refactoring Kafka JAAS Configuration
Posted by Sarath Subramanian <sa...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72527/#review220828
-----------------------------------------------------------
Ship it!
Ship It!
- Sarath Subramanian
On May 19, 2020, 4:25 a.m., Jayendra Parab wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72527/
> -----------------------------------------------------------
>
> (Updated May 19, 2020, 4:25 a.m.)
>
>
> Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-3779
> https://issues.apache.org/jira/browse/ATLAS-3779
>
>
> Repository: atlas
>
>
> Description
> -------
>
> The JAAS config set through InMemoryJAASConfiguration overwrites user supplied JAAS configuration which might be required to connect to user's Kakfa instead of Atlas's Kafka and might have different credentials
>
> To resolve this we should make use of Kafka's dynamic JAAS configuration mechanism wherein you can set the Kafka JAAS config using sasl.jaas.config property. (https://cwiki.apache.org/confluence/display/KAFKA/KIP-85%3A+Dynamic+JAAS+configuration+for+Kafka+clients)
>
> In this patch, we are picking up the atlas.jaas.* properties and setting the sasl.jaas.config property. Since the JAAS configuration is specific only to Kafka, the JAAS property initialization is done while instantiating the KafkaNotification object. Also, since we are using sasl.jaas.config property to set the configuration it's not conflicting with any other JAAS configuration.
>
> To keep the code backward compatible for client application which make use of ticketBased-KafkaClient configuration , we are still making use of the earlier mechanism i.e. with the use of UserGroupInformation.isLoginTicketBased() and UserGroupInformation.isLoginKeytabBased() to set the configuration.
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/ApplicationProperties.java 3ba50612e
> notification/src/main/java/org/apache/atlas/hook/AtlasHook.java cc6546be8
> notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 11a29b9e0
>
>
> Diff: https://reviews.apache.org/r/72527/diff/1/
>
>
> Testing
> -------
>
> With these changes, tried creating table using CTAS query in Hive. The entities corresponding to the table was updated properly in Atlas server
>
>
> Thanks,
>
> Jayendra Parab
>
>
Re: Review Request 72527: Refactoring Kafka JAAS Configuration
Posted by Jayendra Parab <ja...@gmail.com>.
> On May 19, 2020, 4:26 p.m., Ashutosh Mestry wrote:
> > notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java
> > Lines 510 (patched)
> > <https://reviews.apache.org/r/72527/diff/1/?file=2232586#file2232586line510>
> >
> > Refactor: Extract constant.
This statement checks whether the string is enclosed in double quotes, so can't make use of constants here
> On May 19, 2020, 4:26 p.m., Ashutosh Mestry wrote:
> > notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java
> > Lines 512 (patched)
> > <https://reviews.apache.org/r/72527/diff/1/?file=2232586#file2232586line512>
> >
> > Refactor: Promote to const field.
These set of characters are specific to this method, so won't be of much use if we promote to class level field.
- Jayendra
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72527/#review220830
-----------------------------------------------------------
On May 25, 2020, 9:04 a.m., Jayendra Parab wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72527/
> -----------------------------------------------------------
>
> (Updated May 25, 2020, 9:04 a.m.)
>
>
> Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-3779
> https://issues.apache.org/jira/browse/ATLAS-3779
>
>
> Repository: atlas
>
>
> Description
> -------
>
> The JAAS config set through InMemoryJAASConfiguration overwrites user supplied JAAS configuration which might be required to connect to user's Kakfa instead of Atlas's Kafka and might have different credentials
>
> To resolve this we should make use of Kafka's dynamic JAAS configuration mechanism wherein you can set the Kafka JAAS config using sasl.jaas.config property. (https://cwiki.apache.org/confluence/display/KAFKA/KIP-85%3A+Dynamic+JAAS+configuration+for+Kafka+clients)
>
> In this patch, we are picking up the atlas.jaas.* properties and setting the sasl.jaas.config property. Since the JAAS configuration is specific only to Kafka, the JAAS property initialization is done while instantiating the KafkaNotification object. Also, since we are using sasl.jaas.config property to set the configuration it's not conflicting with any other JAAS configuration.
>
> To keep the code backward compatible for client application which make use of ticketBased-KafkaClient configuration , we are still making use of the earlier mechanism i.e. with the use of UserGroupInformation.isLoginTicketBased() and UserGroupInformation.isLoginKeytabBased() to set the configuration.
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/ApplicationProperties.java 3ba50612e
> notification/src/main/java/org/apache/atlas/hook/AtlasHook.java cc6546be8
> notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 11a29b9e0
> notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationMockTest.java 9b5891f27
>
>
> Diff: https://reviews.apache.org/r/72527/diff/2/
>
>
> Testing
> -------
>
> With these changes, tried creating table using CTAS query in Hive. The entities corresponding to the table was updated properly in Atlas server
>
>
> Thanks,
>
> Jayendra Parab
>
>
Re: Review Request 72527: Refactoring Kafka JAAS Configuration
Posted by Ashutosh Mestry via Review Board <no...@reviews.apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72527/#review220830
-----------------------------------------------------------
notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java
Lines 431 (patched)
<https://reviews.apache.org/r/72527/#comment309545>
Refactor: Extract constant field.
notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java
Lines 442 (patched)
<https://reviews.apache.org/r/72527/#comment309546>
Refactor: Extract field constant.
notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java
Lines 466 (patched)
<https://reviews.apache.org/r/72527/#comment309544>
Refactor: Prefer String.format.
notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java
Lines 470 (patched)
<https://reviews.apache.org/r/72527/#comment309543>
Refactor: Prefer String.format.
notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java
Lines 510 (patched)
<https://reviews.apache.org/r/72527/#comment309542>
Refactor: Extract constant.
notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java
Lines 512 (patched)
<https://reviews.apache.org/r/72527/#comment309541>
Refactor: Promote to const field.
- Ashutosh Mestry
On May 19, 2020, 11:25 a.m., Jayendra Parab wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72527/
> -----------------------------------------------------------
>
> (Updated May 19, 2020, 11:25 a.m.)
>
>
> Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-3779
> https://issues.apache.org/jira/browse/ATLAS-3779
>
>
> Repository: atlas
>
>
> Description
> -------
>
> The JAAS config set through InMemoryJAASConfiguration overwrites user supplied JAAS configuration which might be required to connect to user's Kakfa instead of Atlas's Kafka and might have different credentials
>
> To resolve this we should make use of Kafka's dynamic JAAS configuration mechanism wherein you can set the Kafka JAAS config using sasl.jaas.config property. (https://cwiki.apache.org/confluence/display/KAFKA/KIP-85%3A+Dynamic+JAAS+configuration+for+Kafka+clients)
>
> In this patch, we are picking up the atlas.jaas.* properties and setting the sasl.jaas.config property. Since the JAAS configuration is specific only to Kafka, the JAAS property initialization is done while instantiating the KafkaNotification object. Also, since we are using sasl.jaas.config property to set the configuration it's not conflicting with any other JAAS configuration.
>
> To keep the code backward compatible for client application which make use of ticketBased-KafkaClient configuration , we are still making use of the earlier mechanism i.e. with the use of UserGroupInformation.isLoginTicketBased() and UserGroupInformation.isLoginKeytabBased() to set the configuration.
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/ApplicationProperties.java 3ba50612e
> notification/src/main/java/org/apache/atlas/hook/AtlasHook.java cc6546be8
> notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 11a29b9e0
>
>
> Diff: https://reviews.apache.org/r/72527/diff/1/
>
>
> Testing
> -------
>
> With these changes, tried creating table using CTAS query in Hive. The entities corresponding to the table was updated properly in Atlas server
>
>
> Thanks,
>
> Jayendra Parab
>
>
Re: Review Request 72527: Refactoring Kafka JAAS Configuration
Posted by Nixon Rodrigues <ni...@freestoneinfotech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72527/#review220867
-----------------------------------------------------------
Ship it!
Ship It!
- Nixon Rodrigues
On May 25, 2020, 9:04 a.m., Jayendra Parab wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72527/
> -----------------------------------------------------------
>
> (Updated May 25, 2020, 9:04 a.m.)
>
>
> Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-3779
> https://issues.apache.org/jira/browse/ATLAS-3779
>
>
> Repository: atlas
>
>
> Description
> -------
>
> The JAAS config set through InMemoryJAASConfiguration overwrites user supplied JAAS configuration which might be required to connect to user's Kakfa instead of Atlas's Kafka and might have different credentials
>
> To resolve this we should make use of Kafka's dynamic JAAS configuration mechanism wherein you can set the Kafka JAAS config using sasl.jaas.config property. (https://cwiki.apache.org/confluence/display/KAFKA/KIP-85%3A+Dynamic+JAAS+configuration+for+Kafka+clients)
>
> In this patch, we are picking up the atlas.jaas.* properties and setting the sasl.jaas.config property. Since the JAAS configuration is specific only to Kafka, the JAAS property initialization is done while instantiating the KafkaNotification object. Also, since we are using sasl.jaas.config property to set the configuration it's not conflicting with any other JAAS configuration.
>
> To keep the code backward compatible for client application which make use of ticketBased-KafkaClient configuration , we are still making use of the earlier mechanism i.e. with the use of UserGroupInformation.isLoginTicketBased() and UserGroupInformation.isLoginKeytabBased() to set the configuration.
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/ApplicationProperties.java 3ba50612e
> notification/src/main/java/org/apache/atlas/hook/AtlasHook.java cc6546be8
> notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 11a29b9e0
> notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationMockTest.java 9b5891f27
>
>
> Diff: https://reviews.apache.org/r/72527/diff/2/
>
>
> Testing
> -------
>
> With these changes, tried creating table using CTAS query in Hive. The entities corresponding to the table was updated properly in Atlas server
>
>
> Thanks,
>
> Jayendra Parab
>
>
Re: Review Request 72527: Refactoring Kafka JAAS Configuration
Posted by Nixon Rodrigues <ni...@freestoneinfotech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72527/#review220879
-----------------------------------------------------------
Ship it!
Ship It!
- Nixon Rodrigues
On May 27, 2020, 6:10 a.m., Jayendra Parab wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72527/
> -----------------------------------------------------------
>
> (Updated May 27, 2020, 6:10 a.m.)
>
>
> Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-3779
> https://issues.apache.org/jira/browse/ATLAS-3779
>
>
> Repository: atlas
>
>
> Description
> -------
>
> The JAAS config set through InMemoryJAASConfiguration overwrites user supplied JAAS configuration which might be required to connect to user's Kakfa instead of Atlas's Kafka and might have different credentials
>
> To resolve this we should make use of Kafka's dynamic JAAS configuration mechanism wherein you can set the Kafka JAAS config using sasl.jaas.config property. (https://cwiki.apache.org/confluence/display/KAFKA/KIP-85%3A+Dynamic+JAAS+configuration+for+Kafka+clients)
>
> In this patch, we are picking up the atlas.jaas.* properties and setting the sasl.jaas.config property. Since the JAAS configuration is specific only to Kafka, the JAAS property initialization is done while instantiating the KafkaNotification object. Also, since we are using sasl.jaas.config property to set the configuration it's not conflicting with any other JAAS configuration.
>
> To keep the code backward compatible for client application which make use of ticketBased-KafkaClient configuration , we are still making use of the earlier mechanism i.e. with the use of UserGroupInformation.isLoginTicketBased() and UserGroupInformation.isLoginKeytabBased() to set the configuration.
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/ApplicationProperties.java 3ba50612e
> notification/src/main/java/org/apache/atlas/hook/AtlasHook.java cc6546be8
> notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 11a29b9e0
> notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationMockTest.java 9b5891f27
>
>
> Diff: https://reviews.apache.org/r/72527/diff/3/
>
>
> Testing
> -------
>
> With these changes, tried creating table using CTAS query in Hive. The entities corresponding to the table was updated properly in Atlas server
>
>
> Thanks,
>
> Jayendra Parab
>
>
Re: Review Request 72527: Refactoring Kafka JAAS Configuration
Posted by Jayendra Parab <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72527/
-----------------------------------------------------------
(Updated May 27, 2020, 6:10 a.m.)
Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
Changes
-------
Addressed the review comment
Bugs: ATLAS-3779
https://issues.apache.org/jira/browse/ATLAS-3779
Repository: atlas
Description
-------
The JAAS config set through InMemoryJAASConfiguration overwrites user supplied JAAS configuration which might be required to connect to user's Kakfa instead of Atlas's Kafka and might have different credentials
To resolve this we should make use of Kafka's dynamic JAAS configuration mechanism wherein you can set the Kafka JAAS config using sasl.jaas.config property. (https://cwiki.apache.org/confluence/display/KAFKA/KIP-85%3A+Dynamic+JAAS+configuration+for+Kafka+clients)
In this patch, we are picking up the atlas.jaas.* properties and setting the sasl.jaas.config property. Since the JAAS configuration is specific only to Kafka, the JAAS property initialization is done while instantiating the KafkaNotification object. Also, since we are using sasl.jaas.config property to set the configuration it's not conflicting with any other JAAS configuration.
To keep the code backward compatible for client application which make use of ticketBased-KafkaClient configuration , we are still making use of the earlier mechanism i.e. with the use of UserGroupInformation.isLoginTicketBased() and UserGroupInformation.isLoginKeytabBased() to set the configuration.
Diffs (updated)
-----
intg/src/main/java/org/apache/atlas/ApplicationProperties.java 3ba50612e
notification/src/main/java/org/apache/atlas/hook/AtlasHook.java cc6546be8
notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 11a29b9e0
notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationMockTest.java 9b5891f27
Diff: https://reviews.apache.org/r/72527/diff/3/
Changes: https://reviews.apache.org/r/72527/diff/2-3/
Testing
-------
With these changes, tried creating table using CTAS query in Hive. The entities corresponding to the table was updated properly in Atlas server
Thanks,
Jayendra Parab
Re: Review Request 72527: Refactoring Kafka JAAS Configuration
Posted by Madhan Neethiraj <ma...@apache.org>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72527/#review220866
-----------------------------------------------------------
Fix it, then Ship it!
notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java
Lines 475 (patched)
<https://reviews.apache.org/r/72527/#comment309583>
in case optionVal is null (# 462), this will print null in the optionStringBuffer. To avoid this, consider moving #475 to after #473.
- Madhan Neethiraj
On May 25, 2020, 9:04 a.m., Jayendra Parab wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/72527/
> -----------------------------------------------------------
>
> (Updated May 25, 2020, 9:04 a.m.)
>
>
> Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-3779
> https://issues.apache.org/jira/browse/ATLAS-3779
>
>
> Repository: atlas
>
>
> Description
> -------
>
> The JAAS config set through InMemoryJAASConfiguration overwrites user supplied JAAS configuration which might be required to connect to user's Kakfa instead of Atlas's Kafka and might have different credentials
>
> To resolve this we should make use of Kafka's dynamic JAAS configuration mechanism wherein you can set the Kafka JAAS config using sasl.jaas.config property. (https://cwiki.apache.org/confluence/display/KAFKA/KIP-85%3A+Dynamic+JAAS+configuration+for+Kafka+clients)
>
> In this patch, we are picking up the atlas.jaas.* properties and setting the sasl.jaas.config property. Since the JAAS configuration is specific only to Kafka, the JAAS property initialization is done while instantiating the KafkaNotification object. Also, since we are using sasl.jaas.config property to set the configuration it's not conflicting with any other JAAS configuration.
>
> To keep the code backward compatible for client application which make use of ticketBased-KafkaClient configuration , we are still making use of the earlier mechanism i.e. with the use of UserGroupInformation.isLoginTicketBased() and UserGroupInformation.isLoginKeytabBased() to set the configuration.
>
>
> Diffs
> -----
>
> intg/src/main/java/org/apache/atlas/ApplicationProperties.java 3ba50612e
> notification/src/main/java/org/apache/atlas/hook/AtlasHook.java cc6546be8
> notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 11a29b9e0
> notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationMockTest.java 9b5891f27
>
>
> Diff: https://reviews.apache.org/r/72527/diff/2/
>
>
> Testing
> -------
>
> With these changes, tried creating table using CTAS query in Hive. The entities corresponding to the table was updated properly in Atlas server
>
>
> Thanks,
>
> Jayendra Parab
>
>
Re: Review Request 72527: Refactoring Kafka JAAS Configuration
Posted by Jayendra Parab <ja...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/72527/
-----------------------------------------------------------
(Updated May 25, 2020, 9:04 a.m.)
Review request for atlas, Madhan Neethiraj, Nixon Rodrigues, and Sarath Subramanian.
Changes
-------
Addressed review comments and added test cases
Bugs: ATLAS-3779
https://issues.apache.org/jira/browse/ATLAS-3779
Repository: atlas
Description
-------
The JAAS config set through InMemoryJAASConfiguration overwrites user supplied JAAS configuration which might be required to connect to user's Kakfa instead of Atlas's Kafka and might have different credentials
To resolve this we should make use of Kafka's dynamic JAAS configuration mechanism wherein you can set the Kafka JAAS config using sasl.jaas.config property. (https://cwiki.apache.org/confluence/display/KAFKA/KIP-85%3A+Dynamic+JAAS+configuration+for+Kafka+clients)
In this patch, we are picking up the atlas.jaas.* properties and setting the sasl.jaas.config property. Since the JAAS configuration is specific only to Kafka, the JAAS property initialization is done while instantiating the KafkaNotification object. Also, since we are using sasl.jaas.config property to set the configuration it's not conflicting with any other JAAS configuration.
To keep the code backward compatible for client application which make use of ticketBased-KafkaClient configuration , we are still making use of the earlier mechanism i.e. with the use of UserGroupInformation.isLoginTicketBased() and UserGroupInformation.isLoginKeytabBased() to set the configuration.
Diffs (updated)
-----
intg/src/main/java/org/apache/atlas/ApplicationProperties.java 3ba50612e
notification/src/main/java/org/apache/atlas/hook/AtlasHook.java cc6546be8
notification/src/main/java/org/apache/atlas/kafka/KafkaNotification.java 11a29b9e0
notification/src/test/java/org/apache/atlas/kafka/KafkaNotificationMockTest.java 9b5891f27
Diff: https://reviews.apache.org/r/72527/diff/2/
Changes: https://reviews.apache.org/r/72527/diff/1-2/
Testing
-------
With these changes, tried creating table using CTAS query in Hive. The entities corresponding to the table was updated properly in Atlas server
Thanks,
Jayendra Parab