You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@kudu.apache.org by "Grant Henke (Code Review)" <ge...@cloudera.org> on 2019/06/19 15:57:29 UTC

[kudu-CR](branch-1.10.x) [sentry] add require db privileges flag for ListTables

Grant Henke has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13678


Change subject: [sentry] add require_db_privileges flag for ListTables
......................................................................

[sentry] add require_db_privileges flag for ListTables

This patch adds a sentry_require_db_privileges_for_list_tables flag to
allow enforcing database level privileges for ListTables. Without this
flag, there will be a number of requests to Sentry related to the number
of tables in Kudu. With it, that number is limited to the number of
databases in Kudu. However, note that when the flag is set to true,
users with no database-level privileges on a database will not be able
to see any tables within it.

Using ListTablesBenchmark without the flag:
 $ ./bin/sentry_authz_provider-test --gtest_filter=*ListTablesBench* --num_databases=10 --num_tables_per_db=100 --has-db-privileges=false --sentry_require_db_privileges_for_list_tables=false

sentry_authz_provider-test.cc:421] Time spent Listing tables: real 122.567s user 0.339s sys 0.016s

With the flag:
 $ ./bin/sentry_authz_provider-test --gtest_filter=*ListTablesBench* --num_databases=10 --num_tables_per_db=100 --has-db-privileges=false --sentry_require_db_privileges_for_list_tables=true

sentry_authz_provider-test.cc:421] Time spent Listing tables: real 1.869s user 0.011s sys 0.000s

Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Reviewed-on: http://gerrit.cloudera.org:8080/13657
Tested-by: Kudu Jenkins
Reviewed-by: Hao Hao <ha...@cloudera.com>
(cherry picked from commit 45f03d69bdc73d1c7e08f249b4a40ecfbfd7f810)
---
M src/kudu/master/sentry_authz_provider-test.cc
M src/kudu/master/sentry_authz_provider.cc
2 files changed, 18 insertions(+), 1 deletion(-)



  git pull ssh://gerrit.cloudera.org:29418/kudu refs/changes/78/13678/1
-- 
To view, visit http://gerrit.cloudera.org:8080/13678
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.10.x
Gerrit-MessageType: newchange
Gerrit-Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Gerrit-Change-Number: 13678
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>

[kudu-CR](branch-1.10.x) [sentry] add require db privileges flag for ListTables

Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13678 )

Change subject: [sentry] add require_db_privileges flag for ListTables
......................................................................


Patch Set 1: Verified+1


-- 
To view, visit http://gerrit.cloudera.org:8080/13678
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.10.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Gerrit-Change-Number: 13678
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 20 Jun 2019 04:55:24 +0000
Gerrit-HasComments: No

[kudu-CR](branch-1.10.x) [sentry] add require db privileges flag for ListTables

Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has posted comments on this change. ( http://gerrit.cloudera.org:8080/13678 )

Change subject: [sentry] add require_db_privileges flag for ListTables
......................................................................


Patch Set 1: Code-Review+2


-- 
To view, visit http://gerrit.cloudera.org:8080/13678
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.10.x
Gerrit-MessageType: comment
Gerrit-Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Gerrit-Change-Number: 13678
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)
Gerrit-Comment-Date: Thu, 20 Jun 2019 04:55:04 +0000
Gerrit-HasComments: No

[kudu-CR](branch-1.10.x) [sentry] add require db privileges flag for ListTables

Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/13678 )

Change subject: [sentry] add require_db_privileges flag for ListTables
......................................................................

[sentry] add require_db_privileges flag for ListTables

This patch adds a sentry_require_db_privileges_for_list_tables flag to
allow enforcing database level privileges for ListTables. Without this
flag, there will be a number of requests to Sentry related to the number
of tables in Kudu. With it, that number is limited to the number of
databases in Kudu. However, note that when the flag is set to true,
users with no database-level privileges on a database will not be able
to see any tables within it.

Using ListTablesBenchmark without the flag:
 $ ./bin/sentry_authz_provider-test --gtest_filter=*ListTablesBench* --num_databases=10 --num_tables_per_db=100 --has-db-privileges=false --sentry_require_db_privileges_for_list_tables=false

sentry_authz_provider-test.cc:421] Time spent Listing tables: real 122.567s user 0.339s sys 0.016s

With the flag:
 $ ./bin/sentry_authz_provider-test --gtest_filter=*ListTablesBench* --num_databases=10 --num_tables_per_db=100 --has-db-privileges=false --sentry_require_db_privileges_for_list_tables=true

sentry_authz_provider-test.cc:421] Time spent Listing tables: real 1.869s user 0.011s sys 0.000s

Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Reviewed-on: http://gerrit.cloudera.org:8080/13657
Tested-by: Kudu Jenkins
Reviewed-by: Hao Hao <ha...@cloudera.com>
(cherry picked from commit 45f03d69bdc73d1c7e08f249b4a40ecfbfd7f810)
Reviewed-on: http://gerrit.cloudera.org:8080/13678
Tested-by: Hao Hao <ha...@cloudera.com>
---
M src/kudu/master/sentry_authz_provider-test.cc
M src/kudu/master/sentry_authz_provider.cc
2 files changed, 18 insertions(+), 1 deletion(-)

Approvals:
  Hao Hao: Looks good to me, approved; Verified

-- 
To view, visit http://gerrit.cloudera.org:8080/13678
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.10.x
Gerrit-MessageType: merged
Gerrit-Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Gerrit-Change-Number: 13678
Gerrit-PatchSet: 2
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)

[kudu-CR](branch-1.10.x) [sentry] add require db privileges flag for ListTables

Posted by "Hao Hao (Code Review)" <ge...@cloudera.org>.
Hao Hao has removed a vote on this change.

Change subject: [sentry] add require_db_privileges flag for ListTables
......................................................................


Removed Verified-1 by Kudu Jenkins (120)
-- 
To view, visit http://gerrit.cloudera.org:8080/13678
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: kudu
Gerrit-Branch: branch-1.10.x
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I6a225932b22470d653d4b40678f32c2b5cb8329c
Gerrit-Change-Number: 13678
Gerrit-PatchSet: 1
Gerrit-Owner: Grant Henke <gr...@apache.org>
Gerrit-Reviewer: Hao Hao <ha...@cloudera.com>
Gerrit-Reviewer: Kudu Jenkins (120)