[Impala-ASF-CR] IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14400 ) Change subject: IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES .. Patch Set 7: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c Gerrit-Change-Number: 14400 Gerrit-PatchSet: 7 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 15 Oct 2019 11:32:36 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14400 ) Change subject: IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES .. IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES If authorization is enabled, SHOW TABLES statements or GET_TABLES requests in HS2 protocol (used in HUE and JDBC drivers) will only return tables that the user has ANY privileges on them. If the user don't have any privileges on a table, we need 8 privilege checks (ALL, INSERT, SELECT, ALTER, CREATE, DROP, OWNER, REFRESH) to get this conclusion. It takes time in Sentry to check these one by one if there are thousands of tables. Unfortunately, there are no batch API for these checks. This introduces a performance regression after we supported fine-grained privileges, since before that we just check 3 privileges (ALL, INSERT, SELECT). In practice, SELECT privilege is the minimal privilege set. It's wired to grant INSERT or other privileges to a resource without SELECT privilege. We can simplify the process to only check on SELECT privilege if users make sure that SELECT privilege is the minimal privilege set in their environment. This patch adds a flag(SIMPLIFY_CHECK_ON_SHOW_TABLES) to bypass checking other privileges in SHOW TABLE statements. Testing in a database with 40k tables and granting the user SELECT privilege on only 6 tables. When using Sentry, the SHOW TABLES statement takes 5s. With the SIMPLIFY_CHECK_ON_SHOW_TABLES enabled, time reduces to 1.2s. No performance gain is observed when using Ranger since Ranger is fast enough. Tests: - Add custom cluster test for the flag in test_authorization.py for both Sentry and Ranger. - Run CORE tests Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c Reviewed-on: http://gerrit.cloudera.org:8080/14400 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M be/src/common/global-flags.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/authorization/Privilege.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/Frontend.java M tests/authorization/test_authorization.py 7 files changed, 108 insertions(+), 8 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/14400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c Gerrit-Change-Number: 14400 Gerrit-PatchSet: 8 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14400 ) Change subject: IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES .. Patch Set 7: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/14400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c Gerrit-Change-Number: 14400 Gerrit-PatchSet: 7 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 15 Oct 2019 07:12:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14400 ) Change subject: IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES .. Patch Set 7: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5090/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c Gerrit-Change-Number: 14400 Gerrit-PatchSet: 7 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 15 Oct 2019 07:13:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES
Vihang Karajgaonkar has posted comments on this change. (
http://gerrit.cloudera.org:8080/14400 )
Change subject: IMPALA-9002: Add query option to only check SELECT privilege in
SHOW TABLES
..
Patch Set 6: Code-Review+2
(2 comments)
Patch looks good to me. Thanks for making the suggested changes. I think it is
unlikely that Sentry patch for improving the privilege check will be merged
soon. This patch provides a reasonable workaround for the user if they are hit
with the slow privilege checks from Sentry on large setups.
http://gerrit.cloudera.org:8080/#/c/14400/5/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:
http://gerrit.cloudera.org:8080/#/c/14400/5/be/src/common/global-flags.cc@278
PS5, Line 278: simplify_check_on_show_tables
> This change the query behavior so I think admin should be aware of it. As s
I see. Thanks for the explanation.
http://gerrit.cloudera.org:8080/#/c/14400/5/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/14400/5/fe/src/main/java/org/apache/impala/service/Frontend.java@796
PS5, Line 796: PrivilegeRequest privilegeRequest = new
PrivilegeRequestBuilder(
: authzFactory_.getAuthorizableFactory())
: .allOf(requiredPrivilege).onAnyColumn(dbName,
tblName, tableOwner).build();
: if (!authzChecker_.get().hasAccess(user,
privilegeRequest)) {
: iter.remove();
: }
> Yeah, that's an optimization! But changing the order only helps if the user
yeah, that makes sense to me.
--
To view, visit http://gerrit.cloudera.org:8080/14400
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c
Gerrit-Change-Number: 14400
Gerrit-PatchSet: 6
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Fang-Yu Rao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Tue, 15 Oct 2019 04:34:54 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14400 ) Change subject: IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4792/ : 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/14400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c Gerrit-Change-Number: 14400 Gerrit-PatchSet: 6 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar Gerrit-Comment-Date: Tue, 15 Oct 2019 00:23:30 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES
Hello Fang-Yu Rao, Kurt Deschler, Vihang Karajgaonkar, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14400 to look at the new patch set (#6). Change subject: IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES .. IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES If authorization is enabled, SHOW TABLES statements or GET_TABLES requests in HS2 protocol (used in HUE and JDBC drivers) will only return tables that the user has ANY privileges on them. If the user don't have any privileges on a table, we need 8 privilege checks (ALL, INSERT, SELECT, ALTER, CREATE, DROP, OWNER, REFRESH) to get this conclusion. It takes time in Sentry to check these one by one if there are thousands of tables. Unfortunately, there are no batch API for these checks. This introduces a performance regression after we supported fine-grained privileges, since before that we just check 3 privileges (ALL, INSERT, SELECT). In practice, SELECT privilege is the minimal privilege set. It's wired to grant INSERT or other privileges to a resource without SELECT privilege. We can simplify the process to only check on SELECT privilege if users make sure that SELECT privilege is the minimal privilege set in their environment. This patch adds a flag(SIMPLIFY_CHECK_ON_SHOW_TABLES) to bypass checking other privileges in SHOW TABLE statements. Testing in a database with 40k tables and granting the user SELECT privilege on only 6 tables. When using Sentry, the SHOW TABLES statement takes 5s. With the SIMPLIFY_CHECK_ON_SHOW_TABLES enabled, time reduces to 1.2s. No performance gain is observed when using Ranger since Ranger is fast enough. Tests: - Add custom cluster test for the flag in test_authorization.py for both Sentry and Ranger. - Run CORE tests Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c --- M be/src/common/global-flags.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/authorization/Privilege.java M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/Frontend.java M tests/authorization/test_authorization.py 7 files changed, 108 insertions(+), 8 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/14400/6 -- To view, visit http://gerrit.cloudera.org:8080/14400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c Gerrit-Change-Number: 14400 Gerrit-PatchSet: 6 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Reviewer: Vihang Karajgaonkar
[Impala-ASF-CR] IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES
Quanlong Huang has posted comments on this change. (
http://gerrit.cloudera.org:8080/14400 )
Change subject: IMPALA-9002: Add query option to only check SELECT privilege in
SHOW TABLES
..
Patch Set 5:
(2 comments)
> Patch Set 5:
>
> (2 comments)
http://gerrit.cloudera.org:8080/#/c/14400/5/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:
http://gerrit.cloudera.org:8080/#/c/14400/5/be/src/common/global-flags.cc@278
PS5, Line 278: simplify_check_on_show_tables
> Do we need a flag for this behavior? Seems like a useful thing to do in any
This change the query behavior so I think admin should be aware of it. As shown
in the added test, if the user only has INSERT privilege on a table but without
SELECT or any other privileges, the table should still be shown in the default
behavior. With this flag setting to true, this table won't be shown.
http://gerrit.cloudera.org:8080/#/c/14400/5/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/14400/5/fe/src/main/java/org/apache/impala/service/Frontend.java@796
PS5, Line 796: PrivilegeRequest privilegeRequest = new
PrivilegeRequestBuilder(
: authzFactory_.getAuthorizableFactory())
: .allOf(requiredPrivilege).onAnyColumn(dbName,
tblName, tableOwner).build();
: if (!authzChecker_.get().hasAccess(user,
privilegeRequest)) {
: iter.remove();
: }
> It looks like this code loops over all the implied actions of the given pri
Yeah, that's an optimization! But changing the order only helps if the user
does have SELECT privilege on the table. If the user doesn't have any
privileges, the 8 privileges will still be checked one by one and finally
return false. See codes in SentryAuthorizationChecker.authorizeResource():
// The Hive Access API does not currently provide a way to check if the user
// has any privileges on a given resource.
if (request.getPrivilege().hasAnyOf()) {
for (ImpalaAction action: actions) {
if (provider_.hasAccess(new Subject(user.getShortName()), authorizables,
EnumSet.of(action), request.hasGrantOption(), ActiveRoleSet.ALL)) {
return true;
}
}
return false;
So if the user doesn't have any privileges on most of the tables, there are
still a lot of privilege checks to perform. We still need the flag to bypass
7/8 of the checks.
Added this optimization for default behavior.
--
To view, visit http://gerrit.cloudera.org:8080/14400
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c
Gerrit-Change-Number: 14400
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Fang-Yu Rao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Mon, 14 Oct 2019 23:35:36 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES
Vihang Karajgaonkar has posted comments on this change. (
http://gerrit.cloudera.org:8080/14400 )
Change subject: IMPALA-9002: Add query option to only check SELECT privilege in
SHOW TABLES
..
Patch Set 5:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/14400/5/be/src/common/global-flags.cc
File be/src/common/global-flags.cc:
http://gerrit.cloudera.org:8080/#/c/14400/5/be/src/common/global-flags.cc@278
PS5, Line 278: simplify_check_on_show_tables
Do we need a flag for this behavior? Seems like a useful thing to do in any
case instead of letting the user decide to change the flag.
http://gerrit.cloudera.org:8080/#/c/14400/5/fe/src/main/java/org/apache/impala/service/Frontend.java
File fe/src/main/java/org/apache/impala/service/Frontend.java:
http://gerrit.cloudera.org:8080/#/c/14400/5/fe/src/main/java/org/apache/impala/service/Frontend.java@796
PS5, Line 796: PrivilegeRequest privilegeRequest = new
PrivilegeRequestBuilder(
: authzFactory_.getAuthorizableFactory())
: .allOf(requiredPrivilege).onAnyColumn(dbName,
tblName, tableOwner).build();
: if (!authzChecker_.get().hasAccess(user,
privilegeRequest)) {
: iter.remove();
: }
It looks like this code loops over all the implied actions of the given
privilege and returns early if any of the action is allowed based on what I see
in SentryAuthProvider.java
The implied privileges for Privilege.ANY are listed in inefficient order. See
below from Privilege.java
ANY.implied_ = EnumSet.of(ALL, OWNER, ALTER, DROP, CREATE, INSERT, SELECT,
REFRESH);
By default, the EnumSet iterator returns the enums in the order they were
declared. Can we change the order in which we declare the Privilege enums to an
order from least allowing to most allowing privilege? Or change the
implementation of getImpliedPrivileges such that it returns privileges in the
order of SELECT, INSERT, CREATE, ALTER, DROP, OWNER, REFRESH, ALL so that
privilege checking is more efficient?
--
To view, visit http://gerrit.cloudera.org:8080/14400
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c
Gerrit-Change-Number: 14400
Gerrit-PatchSet: 5
Gerrit-Owner: Quanlong Huang
Gerrit-Reviewer: Fang-Yu Rao
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Kurt Deschler
Gerrit-Reviewer: Quanlong Huang
Gerrit-Reviewer: Vihang Karajgaonkar
Gerrit-Comment-Date: Mon, 14 Oct 2019 18:07:44 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES
Hello Fang-Yu Rao, Kurt Deschler, Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14400 to look at the new patch set (#5). Change subject: IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES .. IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES If authorization is enabled, SHOW TABLES statements or GET_TABLES requests in HS2 protocol (used in HUE and JDBC drivers) will only return tables that the user has ANY privileges on them. If the user don't have any privileges on a table, we need 8 privilege checks (ALL, INSERT, SELECT, ALTER, CREATE, DROP, OWNER, REFRESH) to get this conclusion. It takes time in Sentry to check these one by one if there are thousands of tables. Unfortunately, there are no batch API for these checks. This introduces a performance regression after we supported fine-grained privileges, since before that we just check 3 privileges (ALL, INSERT, SELECT). In practice, SELECT privilege is the minimal privilege set. It's wired to grant INSERT or other privileges to a resource without SELECT privilege. We can simplify the process to only check on SELECT privilege if users make sure that SELECT privilege is the minimal privilege set in their environment. This patch adds a flag(SIMPLIFY_CHECK_ON_SHOW_TABLES) to bypass checking other privileges in SHOW TABLE statements. Testing in a database with 40k tables and granting the user SELECT privilege on only 6 tables. When using Sentry, the SHOW TABLES statement takes 5s. With the SIMPLIFY_CHECK_ON_SHOW_TABLES enabled, time reduces to 1.2s. No performance gain is observed when using Ranger since Ranger is fast enough. Tests: - Add custom cluster test for the flag in test_authorization.py for both Sentry and Ranger. - Run CORE tests Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c --- M be/src/common/global-flags.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/Frontend.java M tests/authorization/test_authorization.py 6 files changed, 99 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/14400/5 -- To view, visit http://gerrit.cloudera.org:8080/14400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c Gerrit-Change-Number: 14400 Gerrit-PatchSet: 5 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang
[Impala-ASF-CR] IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14400 ) Change subject: IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES .. Patch Set 3: > Patch Set 3: > > Code looks ok for solution that is described but I would like to know if we > considered a more elegant solution such as supporting ANY privilege in Sentry > or adding a clause to the SHOW TABLE grammar to display tables with certain > privileges. Having external flags that affect statement behavior is generally > bad practice. Yeah, I think supporting ANY privilege in Sentry more efficiently is a more elegant solution. But it's not what we can do in Impala side. I create a Sentry JIRA for this: https://issues.apache.org/jira/browse/SENTRY-2534. In the short term, we may need this patch. For adding a clause to the SHOW TABLE statement, it's not enough to cover the problem since HUE and JDBC drivers depend on the GET_TABLES interface of HS2 protocol. They can't change it easily so will still suffer the problem. So this new flag is for admin in short term to avoid the performance regression when they upgrade Impala to a version with fine-grained privileges. If they switch to use Ranger or if SENTRY-2534 is fixed, they can get rid of this totally. -- To view, visit http://gerrit.cloudera.org:8080/14400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c Gerrit-Change-Number: 14400 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Fri, 11 Oct 2019 01:11:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/14400 ) Change subject: IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES .. Patch Set 3: Code looks ok for solution that is described but I would like to know if we considered a more elegant solution such as supporting ANY privilege in Sentry or adding a clause to the SHOW TABLE grammar to display tables with certain privileges. Having external flags that affect statement behavior is generally bad practice. -- To view, visit http://gerrit.cloudera.org:8080/14400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c Gerrit-Change-Number: 14400 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Fang-Yu Rao Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Kurt Deschler Gerrit-Reviewer: Quanlong Huang Gerrit-Comment-Date: Thu, 10 Oct 2019 18:46:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14400 ) Change subject: IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4780/ : 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/14400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c Gerrit-Change-Number: 14400 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 10 Oct 2019 13:08:07 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14400 to look at the new patch set (#3). Change subject: IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES .. IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES If authorization is enabled, SHOW TABLES statements will only return tables that the user has ANY privileges on them. If the user don't have any privileges on a table, we need 8 privilege checks (ALL, INSERT, SELECT, ALTER, CREATE, DROP, OWNER, REFRESH) to get this conclusion. It takes time in Sentry to check these one by one if there are thousands of tables. Unfortunately, there are no batch API for these checks. This introduces a performance regression after we supported fine-grained privileges, since before that we just check 3 privileges (ALL, INSERT, SELECT). In practice, SELECT privilege is the minimal privilege set. It's wired to grant INSERT or other privileges to a resource without SELECT privilege. We can simplify the process to only check on SELECT privilege if users make sure that SELECT privilege is the minimal privilege set in their environment. This patch adds a flag(SIMPLIFY_CHECK_ON_SHOW_TABLES) to bypass checking other privileges in SHOW TABLE statements. Testing in a database with 40k tables and granting the user SELECT privilege on only 6 tables. When using Sentry, the SHOW TABLES statement takes 5s. With the SIMPLIFY_CHECK_ON_SHOW_TABLES enabled, time reduces to 1.2s. No performance gain is observed when using Ranger since Ranger is fast enough. Tests: - Add custom cluster test for the flag in test_authorization.py for both Sentry and Ranger. - Run CORE tests Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c --- M be/src/common/global-flags.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/Frontend.java M tests/authorization/test_authorization.py 6 files changed, 99 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/14400/3 -- To view, visit http://gerrit.cloudera.org:8080/14400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c Gerrit-Change-Number: 14400 Gerrit-PatchSet: 3 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14400 ) Change subject: IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4778/ : 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/14400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c Gerrit-Change-Number: 14400 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Thu, 10 Oct 2019 09:12:14 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES
Hello Impala Public Jenkins, I'd like you to reexamine a change. Please visit http://gerrit.cloudera.org:8080/14400 to look at the new patch set (#2). Change subject: IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES .. IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES If authorization is enabled, SHOW TABLES statements will only return tables that the user has ANY privileges on them. If the user don't have any privileges on a table, we need 8 privilege checks (ALL, INSERT, SELECT, ALTER, CREATE, DROP, OWNER, REFRESH) to get this conclusion. It takes time in Sentry to check these one by one if there are thousands of tables. Unfortunately, there are no batch API for these checks. This introduces a performance regression after we supported fine-grained privileges, since before that we just check 3 privileges (ALL, INSERT, SELECT). In practice, SELECT privilege is the minimal privilege set. It's wired to grant INSERT or other privileges to a resource without SELECT privilege. We can simplify the process to only check on SELECT privilege if users make sure that SELECT privilege is the minimal privilege set in their environment. This patch adds a flag(SIMPLIFY_CHECK_ON_SHOW_TABLES) to bypass checking other privileges in SHOW TABLE statements. Testing in a database with 40k tables and granting the user SELECT privilege on only 6 tables. When using Sentry, the SHOW TABLES statement takes 5s. With the SIMPLIFY_CHECK_ON_SHOW_TABLES enabled, time reduces to 1.2s. No performance gain is observed when using Ranger since Ranger is fast enough. Tests: - Add custom cluster test for the flag in test_authorization.py for both Sentry and Ranger. - Run CORE tests Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c --- M be/src/common/global-flags.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/Frontend.java M tests/authorization/test_authorization.py 6 files changed, 98 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/14400/2 -- To view, visit http://gerrit.cloudera.org:8080/14400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c Gerrit-Change-Number: 14400 Gerrit-PatchSet: 2 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins
[Impala-ASF-CR] IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14400 ) Change subject: IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES .. Patch Set 1: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4760/ : 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/14400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c Gerrit-Change-Number: 14400 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang Gerrit-Reviewer: Impala Public Jenkins Gerrit-Comment-Date: Wed, 09 Oct 2019 10:02:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES
Quanlong Huang has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14400 Change subject: IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES .. IMPALA-9002: Add query option to only check SELECT privilege in SHOW TABLES If authorization is enabled, SHOW TABLES statements will only return tables that the user has ANY privileges on them. If the user don't have any privileges on a table, we need 8 privilege checks (ALL, INSERT, SELECT, ALTER, CREATE, DROP, OWNER, REFRESH) to get this conclusion. It takes time in Sentry to check these one by one if there are thousands of tables. Unfortunately, there are no batch API for these checks. This introduces a performance regression after we supported fine-grained privileges, since before that we just check 3 privileges (ALL, INSERT, SELECT). In practice, SELECT privilege is the minimal privilege set. It's wired to grant INSERT or other privileges to a resource without SELECT privilege. We can simplify the process to only check on SELECT privilege if users make sure that SELECT privilege is the minimal privilege set in their environment. This patch adds a flag(SIMPLIFY_CHECK_ON_SHOW_TABLES) to bypass checking other privileges in SHOW TABLE statements. Testing in a database with 40k tables and granting the user SELECT privilege on only 6 tables. When using Sentry, the SHOW TABLES statement takes 5s. With the SIMPLIFY_CHECK_ON_SHOW_TABLES enabled, time reduces to 1.2s. No performance gain is observed when using Ranger since Ranger is fast enough. Tests: - Add custom cluster test for the flag in test_authorization.py for both Sentry and Ranger. Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c --- M be/src/common/global-flags.cc M be/src/util/backend-gflag-util.cc M common/thrift/BackendGflags.thrift M fe/src/main/java/org/apache/impala/service/BackendConfig.java M fe/src/main/java/org/apache/impala/service/Frontend.java M tests/authorization/test_authorization.py 6 files changed, 92 insertions(+), 2 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/00/14400/1 -- To view, visit http://gerrit.cloudera.org:8080/14400 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I17e2b7bf9e36c54627276a6812b459912156cc3c Gerrit-Change-Number: 14400 Gerrit-PatchSet: 1 Gerrit-Owner: Quanlong Huang
