You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@sentry.apache.org by Shreepadma Venugopalan <sh...@cloudera.com> on 2014/01/30 00:05:07 UTC
Review Request 17515: SENTRY-103: Restrict connect to Sentry Service
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17515/
-----------------------------------------------------------
Review request for sentry and Brock Noland.
Bugs: SENTRY-103
https://issues.apache.org/jira/browse/SENTRY-103
Repository: sentry
Description
-------
Adds a new Keberos callback that restricts connection to Sentry Service based on list of values in sentry.service.allow.connect.
Diffs
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ConnectionDeniedException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/Constants.java cf14cd0
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/GSSCallback.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryService.java c106568
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/SentryServiceIntegrationBase.java b7c44a4
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java 0198a6f
Diff: https://reviews.apache.org/r/17515/diff/
Testing
-------
Unit test passes with the extra configuration.
Thanks,
Shreepadma Venugopalan
Re: Review Request 17515: SENTRY-103: Restrict connect to Sentry Service
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17515/#review33196
-----------------------------------------------------------
Nice work!! Could we add a test where the client cannot connect?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ConnectionDeniedException.java
<https://reviews.apache.org/r/17515/#comment62544>
Do we need the seter? Cannot this field be final?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/Constants.java
<https://reviews.apache.org/r/17515/#comment62545>
spacing around =
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/GSSCallback.java
<https://reviews.apache.org/r/17515/#comment62546>
conf can be final?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/GSSCallback.java
<https://reviews.apache.org/r/17515/#comment62540>
Arrays.equals?
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/GSSCallback.java
<https://reviews.apache.org/r/17515/#comment62543>
spacing
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/GSSCallback.java
<https://reviews.apache.org/r/17515/#comment62542>
if!
else
should be
if
else
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/GSSCallback.java
<https://reviews.apache.org/r/17515/#comment62541>
The spacing is off. It's indented far too much.
- Brock Noland
On Jan. 29, 2014, 11:05 p.m., Shreepadma Venugopalan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17515/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2014, 11:05 p.m.)
>
>
> Review request for sentry and Brock Noland.
>
>
> Bugs: SENTRY-103
> https://issues.apache.org/jira/browse/SENTRY-103
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Adds a new Keberos callback that restricts connection to Sentry Service based on list of values in sentry.service.allow.connect.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ConnectionDeniedException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/Constants.java cf14cd0
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/GSSCallback.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryService.java c106568
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/SentryServiceIntegrationBase.java b7c44a4
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java 0198a6f
>
> Diff: https://reviews.apache.org/r/17515/diff/
>
>
> Testing
> -------
>
> Unit test passes with the extra configuration.
>
>
> Thanks,
>
> Shreepadma Venugopalan
>
>
Re: Review Request 17515: SENTRY-103: Restrict connect to Sentry Service
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17515/#review33265
-----------------------------------------------------------
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ConnectionDeniedException.java
<https://reviews.apache.org/r/17515/#comment62655>
Removed.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ConnectionDeniedException.java
<https://reviews.apache.org/r/17515/#comment62657>
Removed.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/Constants.java
<https://reviews.apache.org/r/17515/#comment62656>
Added space.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/GSSCallback.java
<https://reviews.apache.org/r/17515/#comment62658>
Changed.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/GSSCallback.java
<https://reviews.apache.org/r/17515/#comment62663>
In the future, we should make this check flexible - realm is really optional. We should be able to handle the case where one of the principals has realm and the other doesn't, so its better to leave it as is and add additional handling later.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/GSSCallback.java
<https://reviews.apache.org/r/17515/#comment62659>
Removed spacing.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/GSSCallback.java
<https://reviews.apache.org/r/17515/#comment62660>
Changed.
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/GSSCallback.java
<https://reviews.apache.org/r/17515/#comment62661>
Removed spacing.
- Shreepadma Venugopalan
On Jan. 29, 2014, 11:05 p.m., Shreepadma Venugopalan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17515/
> -----------------------------------------------------------
>
> (Updated Jan. 29, 2014, 11:05 p.m.)
>
>
> Review request for sentry and Brock Noland.
>
>
> Bugs: SENTRY-103
> https://issues.apache.org/jira/browse/SENTRY-103
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Adds a new Keberos callback that restricts connection to Sentry Service based on list of values in sentry.service.allow.connect.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ConnectionDeniedException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/Constants.java cf14cd0
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/GSSCallback.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryService.java c106568
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/SentryServiceIntegrationBase.java b7c44a4
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java 0198a6f
>
> Diff: https://reviews.apache.org/r/17515/diff/
>
>
> Testing
> -------
>
> Unit test passes with the extra configuration.
>
>
> Thanks,
>
> Shreepadma Venugopalan
>
>
Re: Review Request 17515: SENTRY-103: Restrict connect to Sentry Service
Posted by Brock Noland <br...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17515/#review33269
-----------------------------------------------------------
Ship it!
Ship It!
- Brock Noland
On Jan. 30, 2014, 10:35 p.m., Shreepadma Venugopalan wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/17515/
> -----------------------------------------------------------
>
> (Updated Jan. 30, 2014, 10:35 p.m.)
>
>
> Review request for sentry and Brock Noland.
>
>
> Bugs: SENTRY-103
> https://issues.apache.org/jira/browse/SENTRY-103
>
>
> Repository: sentry
>
>
> Description
> -------
>
> Adds a new Keberos callback that restricts connection to Sentry Service based on list of values in sentry.service.allow.connect.
>
>
> Diffs
> -----
>
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ConnectionDeniedException.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/Constants.java cf14cd0
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/GSSCallback.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryService.java c106568
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/SentryServiceIntegrationBase.java b7c44a4
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java PRE-CREATION
> sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 74704e2
> sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java 0198a6f
>
> Diff: https://reviews.apache.org/r/17515/diff/
>
>
> Testing
> -------
>
> Unit test passes with the extra configuration.
>
>
> Thanks,
>
> Shreepadma Venugopalan
>
>
Re: Review Request 17515: SENTRY-103: Restrict connect to Sentry Service
Posted by Shreepadma Venugopalan <sh...@cloudera.com>.
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/17515/
-----------------------------------------------------------
(Updated Jan. 30, 2014, 10:35 p.m.)
Review request for sentry and Brock Noland.
Changes
-------
Changes based on review feedback.
Bugs: SENTRY-103
https://issues.apache.org/jira/browse/SENTRY-103
Repository: sentry
Description
-------
Adds a new Keberos callback that restricts connection to Sentry Service based on list of values in sentry.service.allow.connect.
Diffs (updated)
-----
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/ConnectionDeniedException.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/Constants.java cf14cd0
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/GSSCallback.java PRE-CREATION
sentry-provider/sentry-provider-db/src/main/java/org/apache/sentry/provider/db/service/thrift/SentryService.java c106568
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/SentryServiceIntegrationBase.java b7c44a4
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceFailureCase.java PRE-CREATION
sentry-provider/sentry-provider-db/src/test/java/org/apache/sentry/provider/db/service/thrift/TestSentryServiceIntegration.java 74704e2
sentry-tests/sentry-tests-hive/src/test/java/org/apache/sentry/tests/e2e/hive/TestPrivilegesAtTableScope.java 0198a6f
Diff: https://reviews.apache.org/r/17515/diff/
Testing
-------
Unit test passes with the extra configuration.
Thanks,
Shreepadma Venugopalan