You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Prasad Mujumdar <pr...@cloudera.com> on 2014/06/03 19:16:11 UTC

Review Request 22213: SENTRY-243: The operation type needs to be set in the grant/revoke task context for the failure hook

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

Review request for sentry and Sravya Tirukkovalur.


Bugs: SENTRY-243
    https://issues.apache.org/jira/browse/SENTRY-243


Repository: sentry


Description
-------

The statement operation type passed to onFailure hook is extracted from the operation type stored in Hive's per-session thread. That is not a reliable place to look up the operation during the process execution. We need to store the operation type in the grant revoke task during the compilation. Also the failure hook should be fired for AccessDenied exception rather than all exceptions.

The patch saves the operation type in the Task when it created during compilation.


Diffs
-----

  sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java df9b0db 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 41a31e8 

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


Testing
-------

Updated the test case to add multiple DDLs before the auth statements.


Thanks,

Prasad Mujumdar


Re: Review Request 22213: SENTRY-243: The operation type needs to be set in the grant/revoke task context for the failure hook

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22213/#review44653
-----------------------------------------------------------



sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java
<https://reviews.apache.org/r/22213/#comment79095>

    I do not see this being used anywhere?



sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java
<https://reviews.apache.org/r/22213/#comment79096>

    I think we should add more tests to make sure operation type is being propagated properly in the failure hook.


- Sravya Tirukkovalur


On June 3, 2014, 5:16 p.m., Prasad Mujumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22213/
> -----------------------------------------------------------
> 
> (Updated June 3, 2014, 5:16 p.m.)
> 
> 
> Review request for sentry and Sravya Tirukkovalur.
> 
> 
> Bugs: SENTRY-243
>     https://issues.apache.org/jira/browse/SENTRY-243
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> The statement operation type passed to onFailure hook is extracted from the operation type stored in Hive's per-session thread. That is not a reliable place to look up the operation during the process execution. We need to store the operation type in the grant revoke task during the compilation. Also the failure hook should be fired for AccessDenied exception rather than all exceptions.
> 
> The patch saves the operation type in the Task when it created during compilation.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java df9b0db 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 41a31e8 
> 
> Diff: https://reviews.apache.org/r/22213/diff/
> 
> 
> Testing
> -------
> 
> Updated the test case to add multiple DDLs before the auth statements.
> 
> 
> Thanks,
> 
> Prasad Mujumdar
> 
>


Re: Review Request 22213: SENTRY-243: The operation type needs to be set in the grant/revoke task context for the failure hook

Posted by Sravya Tirukkovalur <sr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22213/#review44683
-----------------------------------------------------------

Ship it!


Ship It!

- Sravya Tirukkovalur


On June 3, 2014, 9:02 p.m., Prasad Mujumdar wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/22213/
> -----------------------------------------------------------
> 
> (Updated June 3, 2014, 9:02 p.m.)
> 
> 
> Review request for sentry and Sravya Tirukkovalur.
> 
> 
> Bugs: SENTRY-243
>     https://issues.apache.org/jira/browse/SENTRY-243
> 
> 
> Repository: sentry
> 
> 
> Description
> -------
> 
> The statement operation type passed to onFailure hook is extracted from the operation type stored in Hive's per-session thread. That is not a reliable place to look up the operation during the process execution. We need to store the operation type in the grant revoke task during the compilation. Also the failure hook should be fired for AccessDenied exception rather than all exceptions.
> 
> The patch saves the operation type in the Task when it created during compilation.
> 
> 
> Diffs
> -----
> 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java df9b0db 
>   sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java a362363 
>   sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 41a31e8 
> 
> Diff: https://reviews.apache.org/r/22213/diff/
> 
> 
> Testing
> -------
> 
> Updated the test case to add multiple DDLs before the auth statements.
> 
> 
> Thanks,
> 
> Prasad Mujumdar
> 
>


Re: Review Request 22213: SENTRY-243: The operation type needs to be set in the grant/revoke task context for the failure hook

Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22213/
-----------------------------------------------------------

(Updated June 3, 2014, 9:02 p.m.)


Review request for sentry and Sravya Tirukkovalur.


Changes
-------

Additional test cases.


Bugs: SENTRY-243
    https://issues.apache.org/jira/browse/SENTRY-243


Repository: sentry


Description
-------

The statement operation type passed to onFailure hook is extracted from the operation type stored in Hive's per-session thread. That is not a reliable place to look up the operation during the process execution. We need to store the operation type in the grant revoke task during the compilation. Also the failure hook should be fired for AccessDenied exception rather than all exceptions.

The patch saves the operation type in the Task when it created during compilation.


Diffs (updated)
-----

  sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java df9b0db 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java a362363 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 41a31e8 

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


Testing
-------

Updated the test case to add multiple DDLs before the auth statements.


Thanks,

Prasad Mujumdar


Re: Review Request 22213: SENTRY-243: The operation type needs to be set in the grant/revoke task context for the failure hook

Posted by Prasad Mujumdar <pr...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/22213/
-----------------------------------------------------------

(Updated June 3, 2014, 7:53 p.m.)


Review request for sentry and Sravya Tirukkovalur.


Changes
-------

Thanks for catching the issue. Looks like I didn't commit the last change which didn't get included in the diff.
Updated the patch.


Bugs: SENTRY-243
    https://issues.apache.org/jira/browse/SENTRY-243


Repository: sentry


Description
-------

The statement operation type passed to onFailure hook is extracted from the operation type stored in Hive's per-session thread. That is not a reliable place to look up the operation during the process execution. We need to store the operation type in the grant revoke task during the compilation. Also the failure hook should be fired for AccessDenied exception rather than all exceptions.

The patch saves the operation type in the Task when it created during compilation.


Diffs (updated)
-----

  sentry-binding/sentry-binding-hive/src/main/java/org/apache/hadoop/hive/ql/exec/SentryGrantRevokeTask.java df9b0db 
  sentry-binding/sentry-binding-hive/src/main/java/org/apache/sentry/binding/hive/HiveAuthzBindingHook.java a362363 
  sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/dbprovider/TestDbSentryOnFailureHookLoading.java 41a31e8 

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


Testing
-------

Updated the test case to add multiple DDLs before the auth statements.


Thanks,

Prasad Mujumdar