You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@atlas.apache.org by Madhan Neethiraj <ma...@apache.org> on 2019/11/27 21:43:00 UTC
Review Request 71841: ATLAS-3261: added option to authorize
notifications using username given in the message
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71841/
-----------------------------------------------------------
Review request for atlas, Ashutosh Mestry, Bolke de Bruin, Nixon Rodrigues, and Sarath Subramanian.
Bugs: ATLAS-3261
https://issues.apache.org/jira/browse/ATLAS-3261
Repository: atlas
Description
-------
Adam Rempter (arempter) provided the patch for this improvement in https://github.com/apache/atlas/pull/58#issuecomment-514541803. This review has further updates - to cache user-authencation for configured amount of time (5 minutes by default), to reduce the performance impact of generating authentication object from username.
Diffs
-----
pom.xml b2506e70e
webapp/pom.xml 57cab62a3
webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java 41a6c2eff
Diff: https://reviews.apache.org/r/71841/diff/1/
Testing
-------
- manual verification of authorization based on username in notification message
- successful UT, IT runs
Thanks,
Madhan Neethiraj
Re: Review Request 71841: ATLAS-3261: added option to authorize
notifications using username given in the message
Posted by Nixon Rodrigues <ni...@freestoneinfotech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71841/#review218844
-----------------------------------------------------------
Ship it!
Ship It!
- Nixon Rodrigues
On Nov. 27, 2019, 9:43 p.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71841/
> -----------------------------------------------------------
>
> (Updated Nov. 27, 2019, 9:43 p.m.)
>
>
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-3261
> https://issues.apache.org/jira/browse/ATLAS-3261
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Adam Rempter (arempter) provided the patch for this improvement in https://github.com/apache/atlas/pull/58#issuecomment-514541803. This review has further updates - to cache user-authencation for configured amount of time (5 minutes by default), to reduce the performance impact of generating authentication object from username.
>
>
> Diffs
> -----
>
> pom.xml b2506e70e
> webapp/pom.xml 57cab62a3
> webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java 41a6c2eff
>
>
> Diff: https://reviews.apache.org/r/71841/diff/1/
>
>
> Testing
> -------
>
> - manual verification of authorization based on username in notification message
> - successful UT, IT runs
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 71841: ATLAS-3261: added option to authorize
notifications using username given in the message
Posted by Madhan Neethiraj <ma...@apache.org>.
> On Nov. 28, 2019, 11:28 a.m., Bolke de Bruin wrote:
> > will authorization fail if authentication is null? if it doesnt should that notbe configurable?
Authentication would be null only when no username is specified in the message. Should such messages be dropped?
- Madhan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71841/#review218845
-----------------------------------------------------------
On Nov. 27, 2019, 9:43 p.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71841/
> -----------------------------------------------------------
>
> (Updated Nov. 27, 2019, 9:43 p.m.)
>
>
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-3261
> https://issues.apache.org/jira/browse/ATLAS-3261
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Adam Rempter (arempter) provided the patch for this improvement in https://github.com/apache/atlas/pull/58#issuecomment-514541803. This review has further updates - to cache user-authencation for configured amount of time (5 minutes by default), to reduce the performance impact of generating authentication object from username.
>
>
> Diffs
> -----
>
> pom.xml b2506e70e
> webapp/pom.xml 57cab62a3
> webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java 41a6c2eff
>
>
> Diff: https://reviews.apache.org/r/71841/diff/1/
>
>
> Testing
> -------
>
> - manual verification of authorization based on username in notification message
> - successful UT, IT runs
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 71841: ATLAS-3261: added option to authorize
notifications using username given in the message
Posted by Adam Rempter <ar...@gmail.com>.
> On Nov. 28, 2019, 11:28 a.m., Bolke de Bruin wrote:
> > will authorization fail if authentication is null? if it doesnt should that notbe configurable?
>
> Madhan Neethiraj wrote:
> Authentication would be null only when no username is specified in the message. Should such messages be dropped?
I just tried to produce message with no user in it and atlas says:
org.apache.atlas.exception.AtlasBaseException: UNKNOWN is not authorized to perform create entity
So I think case when there is no user in kafka message is covered, right?
- Adam
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71841/#review218845
-----------------------------------------------------------
On Nov. 27, 2019, 9:43 p.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71841/
> -----------------------------------------------------------
>
> (Updated Nov. 27, 2019, 9:43 p.m.)
>
>
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-3261
> https://issues.apache.org/jira/browse/ATLAS-3261
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Adam Rempter (arempter) provided the patch for this improvement in https://github.com/apache/atlas/pull/58#issuecomment-514541803. This review has further updates - to cache user-authencation for configured amount of time (5 minutes by default), to reduce the performance impact of generating authentication object from username.
>
>
> Diffs
> -----
>
> pom.xml b2506e70e
> webapp/pom.xml 57cab62a3
> webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java 41a6c2eff
>
>
> Diff: https://reviews.apache.org/r/71841/diff/1/
>
>
> Testing
> -------
>
> - manual verification of authorization based on username in notification message
> - successful UT, IT runs
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 71841: ATLAS-3261: added option to authorize
notifications using username given in the message
Posted by Madhan Neethiraj <ma...@apache.org>.
> On Nov. 28, 2019, 11:28 a.m., Bolke de Bruin wrote:
> > will authorization fail if authentication is null? if it doesnt should that notbe configurable?
>
> Madhan Neethiraj wrote:
> Authentication would be null only when no username is specified in the message. Should such messages be dropped?
>
> Adam Rempter wrote:
> I just tried to produce message with no user in it and atlas says:
> org.apache.atlas.exception.AtlasBaseException: UNKNOWN is not authorized to perform create entity
>
> So I think case when there is no user in kafka message is covered, right?
@arempter - you are right. When user name is not populated in the notification, default value of UNKNOWN is assigned (in HookNotification.getUser()); hence authorization is performed as user=UNKNOWN.
@bolke - does this address your concern?
- Madhan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71841/#review218845
-----------------------------------------------------------
On Nov. 27, 2019, 9:43 p.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71841/
> -----------------------------------------------------------
>
> (Updated Nov. 27, 2019, 9:43 p.m.)
>
>
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-3261
> https://issues.apache.org/jira/browse/ATLAS-3261
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Adam Rempter (arempter) provided the patch for this improvement in https://github.com/apache/atlas/pull/58#issuecomment-514541803. This review has further updates - to cache user-authencation for configured amount of time (5 minutes by default), to reduce the performance impact of generating authentication object from username.
>
>
> Diffs
> -----
>
> pom.xml b2506e70e
> webapp/pom.xml 57cab62a3
> webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java 41a6c2eff
>
>
> Diff: https://reviews.apache.org/r/71841/diff/1/
>
>
> Testing
> -------
>
> - manual verification of authorization based on username in notification message
> - successful UT, IT runs
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 71841: ATLAS-3261: added option to authorize
notifications using username given in the message
Posted by Bolke de Bruin <bd...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71841/#review218845
-----------------------------------------------------------
will authorization fail if authentication is null? if it doesnt should that notbe configurable?
- Bolke de Bruin
On nov 27, 2019, 9:43 p.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71841/
> -----------------------------------------------------------
>
> (Updated nov 27, 2019, 9:43 p.m.)
>
>
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-3261
> https://issues.apache.org/jira/browse/ATLAS-3261
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Adam Rempter (arempter) provided the patch for this improvement in https://github.com/apache/atlas/pull/58#issuecomment-514541803. This review has further updates - to cache user-authencation for configured amount of time (5 minutes by default), to reduce the performance impact of generating authentication object from username.
>
>
> Diffs
> -----
>
> pom.xml b2506e70e
> webapp/pom.xml 57cab62a3
> webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java 41a6c2eff
>
>
> Diff: https://reviews.apache.org/r/71841/diff/1/
>
>
> Testing
> -------
>
> - manual verification of authorization based on username in notification message
> - successful UT, IT runs
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 71841: ATLAS-3261: added option to authorize
notifications using username given in the message
Posted by Nixon Rodrigues <ni...@freestoneinfotech.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71841/#review218843
-----------------------------------------------------------
Ship it!
Ship It!
- Nixon Rodrigues
On Nov. 27, 2019, 9:43 p.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71841/
> -----------------------------------------------------------
>
> (Updated Nov. 27, 2019, 9:43 p.m.)
>
>
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-3261
> https://issues.apache.org/jira/browse/ATLAS-3261
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Adam Rempter (arempter) provided the patch for this improvement in https://github.com/apache/atlas/pull/58#issuecomment-514541803. This review has further updates - to cache user-authencation for configured amount of time (5 minutes by default), to reduce the performance impact of generating authentication object from username.
>
>
> Diffs
> -----
>
> pom.xml b2506e70e
> webapp/pom.xml 57cab62a3
> webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java 41a6c2eff
>
>
> Diff: https://reviews.apache.org/r/71841/diff/1/
>
>
> Testing
> -------
>
> - manual verification of authorization based on username in notification message
> - successful UT, IT runs
>
>
> Thanks,
>
> Madhan Neethiraj
>
>
Re: Review Request 71841: ATLAS-3261: added option to authorize
notifications using username given in the message
Posted by Bolke de Bruin <bd...@gmail.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/71841/#review218866
-----------------------------------------------------------
Ship it!
Ship It!
- Bolke de Bruin
On Nov. 27, 2019, 9:43 p.m., Madhan Neethiraj wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/71841/
> -----------------------------------------------------------
>
> (Updated Nov. 27, 2019, 9:43 p.m.)
>
>
> Review request for atlas, Ashutosh Mestry, Bolke de Bruin, Nixon Rodrigues, and Sarath Subramanian.
>
>
> Bugs: ATLAS-3261
> https://issues.apache.org/jira/browse/ATLAS-3261
>
>
> Repository: atlas
>
>
> Description
> -------
>
> Adam Rempter (arempter) provided the patch for this improvement in https://github.com/apache/atlas/pull/58#issuecomment-514541803. This review has further updates - to cache user-authencation for configured amount of time (5 minutes by default), to reduce the performance impact of generating authentication object from username.
>
>
> Diffs
> -----
>
> pom.xml b2506e70e
> webapp/pom.xml 57cab62a3
> webapp/src/main/java/org/apache/atlas/notification/NotificationHookConsumer.java 41a6c2eff
>
>
> Diff: https://reviews.apache.org/r/71841/diff/1/
>
>
> Testing
> -------
>
> - manual verification of authorization based on username in notification message
> - successful UT, IT runs
>
>
> Thanks,
>
> Madhan Neethiraj
>
>