[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. Patch Set 15: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 15 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 27 Jun 2019 00:22:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API Impala uses a workaround to detect if a user is a Sentry admin by calling the Sentry API to list privileges associated with the user since previously Sentry did not provide a suitable API to peform this check. This patch invokes a new API in SENTRY-2440 to perform the Sentry admin check. Also modified test_sentry.py to exercise the code paths corresponding to the following 3 different types of users: (i) a Sentry admin, (ii) an existing user which is not a Sentry admin, and (iii) a non-existing user. Testing: 1. Passed the tests in the revised test_sentry.py. Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Reviewed-on: http://gerrit.cloudera.org:8080/13346 Reviewed-by: Fredy Wijaya Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M tests/authorization/test_sentry.py 5 files changed, 54 insertions(+), 23 deletions(-) Approvals: Fredy Wijaya: Looks good to me, approved Impala Public Jenkins: Verified -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 16 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. Patch Set 15: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3758/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 15 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 26 Jun 2019 19:17:44 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. Patch Set 13: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3757/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 13 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 26 Jun 2019 19:00:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. Patch Set 15: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 15 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 26 Jun 2019 18:41:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. Patch Set 15: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4553/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 15 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 26 Jun 2019 18:41:51 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Fang-Yu Rao has uploaded a new patch set (#15). ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API Impala uses a workaround to detect if a user is a Sentry admin by calling the Sentry API to list privileges associated with the user since previously Sentry did not provide a suitable API to peform this check. This patch invokes a new API in SENTRY-2440 to perform the Sentry admin check. Also modified test_sentry.py to exercise the code paths corresponding to the following 3 different types of users: (i) a Sentry admin, (ii) an existing user which is not a Sentry admin, and (iii) a non-existing user. Testing: 1. Passed the tests in the revised test_sentry.py. Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 --- M fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M tests/authorization/test_sentry.py 5 files changed, 54 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/13346/15 -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 15 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. Patch Set 13: (1 comment) http://gerrit.cloudera.org:8080/#/c/13346/13/fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java File fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java: http://gerrit.cloudera.org:8080/#/c/13346/13/fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java@538 PS13, Line 538: public boolean isSentryAdmin(User user) throws line has trailing whitespace -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 13 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 26 Jun 2019 18:20:48 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Fang-Yu Rao has uploaded a new patch set (#13). ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API Impala uses a workaround to detect if a user is a Sentry admin by calling the Sentry API to list privileges associated with the user since previously Sentry did not provide a suitable API to peform this check. This patch invokes a new API in SENTRY-2440 to perform the Sentry admin check. Also modified test_sentry.py to exercise the code paths corresponding to the following 3 different types of users: (i) a Sentry admin, (ii) an existing user which is not a Sentry admin, and (iii) a non-existing user. Testing: 1. Passed the tests in the revised test_sentry.py. Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 --- M fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M tests/authorization/test_sentry.py 5 files changed, 54 insertions(+), 23 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/13346/13 -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 13 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/13346 )
Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper
Sentry API
..
Patch Set 11:
(5 comments)
Thanks for the fixes! This is getting close. I'm ready to give a +2 once all
the comments are resolved.
http://gerrit.cloudera.org:8080/#/c/13346/11/fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java
File
fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java:
http://gerrit.cloudera.org:8080/#/c/13346/11/fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java@535
PS11, Line 535: public boolean isSentryAdmin(User requestingUser) throws
InternalException,
can we add javadoc?
http://gerrit.cloudera.org:8080/#/c/13346/11/fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java@535
PS11, Line 535: throws InternalException,
nit: I think it's more readable to put throws InternalException,
SentryUserException in the next line.
http://gerrit.cloudera.org:8080/#/c/13346/11/fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java@537
PS11, Line 537: SentryServiceClient client = new SentryServiceClient();
: try {
: return client.get().isAdmin(requestingUser.getName());
: } finally {
: client.close();
: }
nit: using try-resource is shorter, i.e.
try (SentryServiceClient client = new SentryServiceClient()) {
return client.get().isAdmin(requestingUser.getName());
}
http://gerrit.cloudera.org:8080/#/c/13346/11/fe/src/main/java/org/apache/impala/service/JniCatalog.java
File fe/src/main/java/org/apache/impala/service/JniCatalog.java:
http://gerrit.cloudera.org:8080/#/c/13346/11/fe/src/main/java/org/apache/impala/service/JniCatalog.java@304
PS11, Line 304: try {
: boolean isSentryAdmin =
((SentryCatalogdAuthorizationManager)
:
catalogOpExecutor_.getAuthzManager()).isSentryAdmin(user);
: response.setIs_admin(isSentryAdmin);
: } catch (SentryUserException e) {
: // When a user is not defined in Sentry, isAdmin() will
throw
: // SentryUserException, we will consider this requesting
user
: // as a non-Sentry administrator.
: response.setIs_admin(false);
: }
Sorry for the back and forth, but putting this logic in JniCatalog doesn't feel
quite right since JniCatalog is supposed to be a thin JNI wrapper. It makes
more sense to move the try/catch logic in the SentryProxy instead.
http://gerrit.cloudera.org:8080/#/c/13346/11/tests/authorization/test_sentry.py
File tests/authorization/test_sentry.py:
http://gerrit.cloudera.org:8080/#/c/13346/11/tests/authorization/test_sentry.py@51
PS11, Line 51: root
nit: Calling it non_admin makes it easier to understand.
--
To view, visit http://gerrit.cloudera.org:8080/13346
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82
Gerrit-Change-Number: 13346
Gerrit-PatchSet: 11
Gerrit-Owner: Fang-Yu Rao
Gerrit-Reviewer: Fang-Yu Rao
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Wed, 26 Jun 2019 03:57:18 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3750/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 11 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 26 Jun 2019 02:05:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3749/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 10 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 26 Jun 2019 02:04:41 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Fang-Yu Rao has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API Impala uses a workaround to detect if a user is a Sentry admin by calling the Sentry API to list privileges associated with the user since previously Sentry did not provide a suitable API to peform this check. This patch invokes a new API in SENTRY-2440 to perform the Sentry admin check. Also modified test_sentry.py to exercise the code paths corresponding to the following 3 different types of users: (i) a Sentry admin, (ii) an existing user which is not a Sentry admin, and (iii) a non-existing user. Testing: 1. Passed the tests in the revised test_sentry.py. Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 --- M fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M tests/authorization/test_sentry.py 5 files changed, 62 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/13346/11 -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 11 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/13346 )
Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper
Sentry API
..
Patch Set 10:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/13346/10/fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java
File
fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java:
http://gerrit.cloudera.org:8080/#/c/13346/10/fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java@534
PS10, Line 534:
line has trailing whitespace
http://gerrit.cloudera.org:8080/#/c/13346/10/fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java@537
PS10, Line 537: SentryServiceClient client = new SentryServiceClient();
tab used for whitespace
http://gerrit.cloudera.org:8080/#/c/13346/10/fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java@538
PS10, Line 538: try {
tab used for whitespace
http://gerrit.cloudera.org:8080/#/c/13346/10/fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java
File fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java:
http://gerrit.cloudera.org:8080/#/c/13346/10/fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java@387
PS10, Line 387: return
sentryPolicyService_.isSentryAdmin(requestingUser);
tab used for whitespace
--
To view, visit http://gerrit.cloudera.org:8080/13346
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82
Gerrit-Change-Number: 13346
Gerrit-PatchSet: 10
Gerrit-Owner: Fang-Yu Rao
Gerrit-Reviewer: Fang-Yu Rao
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Comment-Date: Wed, 26 Jun 2019 01:19:42 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Fang-Yu Rao has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API Impala uses a workaround to detect if a user is a Sentry admin by calling the Sentry API to list privileges associated with the user since previously Sentry did not provide a suitable API to peform this check. This patch invokes a new API in SENTRY-2440 to perform the Sentry admin check. Also modified test_sentry.py to exercise the code paths corresponding to the following 3 different types of users: (i) a Sentry admin, (ii) an existing user which is not a Sentry admin, and (iii) a non-existing user. Testing: 1. Passed the tests in the revised test_sentry.py. Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 --- M fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M tests/authorization/test_sentry.py 5 files changed, 62 insertions(+), 24 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/13346/10 -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 10 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/13346/4/fe/src/main/java/org/apache/impala/service/JniCatalog.java File fe/src/main/java/org/apache/impala/service/JniCatalog.java: http://gerrit.cloudera.org:8080/#/c/13346/4/fe/src/main/java/org/apache/impala/service/JniCatalog.java@309 PS4, Line 309: // When there is no user having user name as user.getName() in the : // underlying OS, the method isAdmin() in SentryPolicyServiceClient.class : // would throw a SentryUserException. In this case, we also consider this : // requesting user a non Sentry administrator. reword: When a user is not defined in Sentry, isAdmin() will throw SentryUserException, we will consider this requesting user as a non-Sentry administrator. -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 4 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 16 May 2019 14:07:08 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3248/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 4 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 16 May 2019 05:40:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Fang-Yu Rao has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API Impala uses a workaround to detect if a user is a Sentry admin by calling the Sentry API to list privileges associated with the user since previously Sentry did not provide a suitable API to peform this check. This patch updates CDH_BUILD_NUMBER to 1075921 that contains a new API in SENTRY-2440 to perform a Sentry admin check. Testing: 1. Passed the tests in AuthorizationTest.java. 2. Passed the Sentry related tests in AuthorizationStmtTest.java. 3. Passed the tests in test_sentry.py. Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 --- M bin/impala-config.sh M fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java 4 files changed, 26 insertions(+), 14 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/13346/4 -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 4 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3246/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 3 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 16 May 2019 01:56:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Fang-Yu Rao has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API Impala uses a workaround to detect if a user is a Sentry admin by by asking the Sentry API to list privileges associated with the user, since previously Sentry did not provide an suitable API to peform this check. SENTRY-2440 provides a new API (https://issues.apache.org/jira/browse/SENTRY-2440), and thus it would be good to update the Impala to make the code easier to understand. Testing: 1. Passed the tests in AuthorizationTest.java. 2. Passed the Sentry related tests in AuthorizationStmtTest.java. 3. Passed the tests in test_sentry.py. Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 --- M bin/impala-config.sh M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java 2 files changed, 20 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/13346/3 -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 3 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13346 ) Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/3242/ : Initial code review checks passed. Use gerrit-verify-dryrun-external or gerrit-verify-dryrun to run full precommit tests. -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 1 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 15 May 2019 20:47:55 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API
Fang-Yu Rao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13346 Change subject: IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API .. IMPALA-8476: Replace Sentry admin check workaround with proper Sentry API Impala uses a workaround to detect if a user is a Sentry admin by by asking the Sentry API to list privileges associated with the user, since previously Sentry did not provide an suitable API to peform this check. SENTRY-2440 provides a new API (https://issues.apache.org/jira/browse/SENTRY-2440), and thus it would be good to update the Impala to make the code easier to understand. Testing: 1. Passed the tests in AuthorizationTest.java. 2. Passed the Sentry related tests in AuthorizationStmtTest.java. 2. Passed the tests in test_sentry.py. Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 --- M bin/impala-config.sh M fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryPolicyService.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java 5 files changed, 25 insertions(+), 6 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/46/13346/1 -- To view, visit http://gerrit.cloudera.org:8080/13346 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I5a27140d401494bc372ad0da96ada57bda94cd82 Gerrit-Change-Number: 13346 Gerrit-PatchSet: 1 Gerrit-Owner: Fang-Yu Rao Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Fredy Wijaya
