[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4551/ : 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/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 12 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 12 Sep 2019 05:10:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Bharath Vissapragada has submitted this change and it was merged. (
http://gerrit.cloudera.org:8080/14106 )
Change subject: IMPALA-8228: Ownership support for Ranger authz
..
IMPALA-8228: Ownership support for Ranger authz
Without this patch, explicit privileges are needed even
for owners of databases/tables to perform actions on them.
Example: 'user' is the owner of database 'foo'. To create
a table 't' under 'foo', 'user' needs to be granted a CREATE
privilege on 'foo'
That is unintuitive from a user POV since users expect owners
to have ALL privileges on the objects they own. This patch extends
that support to Impala's ranger authorization plugin.
Ranger natively supports the concept of ownership by letting the
callers pass the ownership context to RangerAccessResourceImpl.
This patch plumbs the owner information for the authorizables
(currently only supported for Tables / Databases) which is then
evaulated during authorization.
For the ownership based authorization to work, ranger-admin side
policy on {OWNER} user needs to be defined.
Testing: Added some unit-tests and e-e tests that cover scenarios
where ownership is used for authorization.
Caveat: Ownership is a part of HMS thrift object. Since we do not
aggressively load HMS schemas during start-up, coordinators with
cold caches can result in weird table listings due to lack of
metadata needed for verifying ownership. This should be fixed
separately to make the behavior more consistent and user friendly.
(Added comments in the code wherever necessary along with a test
to simulate this).
Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Reviewed-on: http://gerrit.cloudera.org:8080/14106
Reviewed-by: Bharath Vissapragada
Tested-by: Bharath Vissapragada
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/authorization/Authorizable.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M
fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaResourceBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeDb.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M tests/authorization/test_ranger.py
30 files changed, 570 insertions(+), 101 deletions(-)
Approvals:
Bharath Vissapragada: Looks good to me, approved; Verified
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 13
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 12: Verified+1 Already verified with a previous GVO -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 12 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 12 Sep 2019 04:31:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Hello Austin Nobis, Fredy Wijaya, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14106
to look at the new patch set (#12).
Change subject: IMPALA-8228: Ownership support for Ranger authz
..
IMPALA-8228: Ownership support for Ranger authz
Without this patch, explicit privileges are needed even
for owners of databases/tables to perform actions on them.
Example: 'user' is the owner of database 'foo'. To create
a table 't' under 'foo', 'user' needs to be granted a CREATE
privilege on 'foo'
That is unintuitive from a user POV since users expect owners
to have ALL privileges on the objects they own. This patch extends
that support to Impala's ranger authorization plugin.
Ranger natively supports the concept of ownership by letting the
callers pass the ownership context to RangerAccessResourceImpl.
This patch plumbs the owner information for the authorizables
(currently only supported for Tables / Databases) which is then
evaulated during authorization.
For the ownership based authorization to work, ranger-admin side
policy on {OWNER} user needs to be defined.
Testing: Added some unit-tests and e-e tests that cover scenarios
where ownership is used for authorization.
Caveat: Ownership is a part of HMS thrift object. Since we do not
aggressively load HMS schemas during start-up, coordinators with
cold caches can result in weird table listings due to lack of
metadata needed for verifying ownership. This should be fixed
separately to make the behavior more consistent and user friendly.
(Added comments in the code wherever necessary along with a test
to simulate this).
Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/authorization/Authorizable.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M
fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaResourceBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeDb.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M tests/authorization/test_ranger.py
30 files changed, 570 insertions(+), 101 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14106/12
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 12
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 12: Code-Review+2 (2 comments) Thanks for the reviews, carrying +2. http://gerrit.cloudera.org:8080/#/c/14106/11/fe/src/main/java/org/apache/impala/authorization/Authorizable.java File fe/src/main/java/org/apache/impala/authorization/Authorizable.java: http://gerrit.cloudera.org:8080/#/c/14106/11/fe/src/main/java/org/apache/impala/authorization/Authorizable.java@65 PS11, Line 65: int ownerHash = getOwnerUser() == null ? 0 : getOwnerUser().hashCode(); : int nameHash = getName().hashCode(); : return nameHash * 31 + ownerHash; > nit: You can use Objects.hash(nameHash, ownerHash) Objects.hash is what I tried but since owner is nullable, it causes an NPE. http://gerrit.cloudera.org:8080/#/c/14106/11/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java File fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java: http://gerrit.cloudera.org:8080/#/c/14106/11/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@3068 PS11, Line 3068: String>builder() > nit: I think Java 8 can infer this without explicitly specifying the type p Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.3:testCompile (default-testCompile) on project impala-frontend: Compilation failure [ERROR] /home/bharath/impala/Impala/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java:[3110,15] incompatible types: com.google.common.collect.ImmutableMap cannot be converted to com.google.common.collect.ImmutableMap Think we don't have this https://github.com/google/guava/issues/1166 -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 12 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 12 Sep 2019 04:30:50 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/14106 )
Change subject: IMPALA-8228: Ownership support for Ranger authz
..
Patch Set 11: Code-Review+2
(3 comments)
LGTM.
http://gerrit.cloudera.org:8080/#/c/14106/11/fe/src/main/java/org/apache/impala/authorization/Authorizable.java
File fe/src/main/java/org/apache/impala/authorization/Authorizable.java:
http://gerrit.cloudera.org:8080/#/c/14106/11/fe/src/main/java/org/apache/impala/authorization/Authorizable.java@65
PS11, Line 65: int ownerHash = getOwnerUser() == null ? 0 :
getOwnerUser().hashCode();
: int nameHash = getName().hashCode();
: return nameHash * 31 + ownerHash;
nit: You can use Objects.hash(nameHash, ownerHash)
http://gerrit.cloudera.org:8080/#/c/14106/11/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
File
fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java:
http://gerrit.cloudera.org:8080/#/c/14106/11/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@3068
PS11, Line 3068:
nit: I think Java 8 can infer this without explicitly specifying the type
parameters.
http://gerrit.cloudera.org:8080/#/c/14106/11/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@3069
PS11, Line 3069: .put(authorize("select count(*) from
functional.alltypes"),
: selectError("functional.alltypes"))
: .put(authorize("select id from functional.alltypes"),
: selectError("functional.alltypes"))
: .put(authorize("select id from
functional.alltypes_view"),
: selectError("functional.alltypes_view"))
: .put(authorize("show create table
functional.alltypes"),
: accessError("functional.alltypes"))
: .put(authorize("describe functional.alltypes"),
: accessError("functional.alltypes"))
: .put(authorize("show create table
functional.alltypes_view"),
: accessError("functional.alltypes_view"))
: .put(authorize("describe functional.alltypes_view"),
: accessError("functional.alltypes_view"))
: .put(authorize("describe
functional.allcomplextypes.int_struct_col"),
: accessError("functional.allcomplextypes"))
: .put(authorize("refresh functional.alltypes"),
: refreshError("functional.alltypes"))
: .put(authorize("invalidate metadata
functional.alltypes"),
: refreshError("functional.alltypes"))
: .put(authorize("compute stats functional.alltypes"),
: alterError("functional.alltypes"))
: .put(authorize("drop stats functional.alltypes"),
: alterError("functional.alltypes"))
: .put(authorize("create table functional.test_tbl(a
int)"),
: createError("functional"))
: .put(authorize("create table functional.test_tbl like
functional.alltypes"),
: accessError("functional.alltypes"))
: .put(authorize("create table functional.test_tbl as
select 1"),
: createError("functional"))
: .put(authorize("create view functional.test_view as
select 1"),
: createError("functional"))
: .put(authorize("alter table functional.alltypes add
column c1 int"),
: alterError("functional"))
: .put(authorize("drop table functional.alltypes"),
: dropError("functional"))
: .put(authorize("drop view functional.alltypes_view"),
: dropError("functional"))
: .put(authorize("alter view functional.alltypes_view as
select 1"),
: alterError("functional.alltypes_view"))
: .put(authorize("alter database functional set owner
user foo"),
: accessError(true, "functional"))
: .build();
nit: should be indented.
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 11
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer:
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 11: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 11 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 12 Sep 2019 03:45:27 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 10: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4939/ -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 10 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 12 Sep 2019 03:41:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 11: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 11 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 12 Sep 2019 03:32:03 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4546/ : 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/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 11 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 12 Sep 2019 00:13:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 10: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4545/ : 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/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 10 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 12 Sep 2019 00:11:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 11: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4940/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 11 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 11 Sep 2019 23:33:48 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/14106/10/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/14106/10/tests/authorization/test_ranger.py@711 PS10, Line 711: > flake8: E501 line too long (93 > 90 characters) Done -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 11 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 11 Sep 2019 23:33:39 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Hello Austin Nobis, Fredy Wijaya, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14106
to look at the new patch set (#11).
Change subject: IMPALA-8228: Ownership support for Ranger authz
..
IMPALA-8228: Ownership support for Ranger authz
Without this patch, explicit privileges are needed even
for owners of databases/tables to perform actions on them.
Example: 'user' is the owner of database 'foo'. To create
a table 't' under 'foo', 'user' needs to be granted a CREATE
privilege on 'foo'
That is unintuitive from a user POV since users expect owners
to have ALL privileges on the objects they own. This patch extends
that support to Impala's ranger authorization plugin.
Ranger natively supports the concept of ownership by letting the
callers pass the ownership context to RangerAccessResourceImpl.
This patch plumbs the owner information for the authorizables
(currently only supported for Tables / Databases) which is then
evaulated during authorization.
For the ownership based authorization to work, ranger-admin side
policy on {OWNER} user needs to be defined.
Testing: Added some unit-tests and e-e tests that cover scenarios
where ownership is used for authorization.
Caveat: Ownership is a part of HMS thrift object. Since we do not
aggressively load HMS schemas during start-up, coordinators with
cold caches can result in weird table listings due to lack of
metadata needed for verifying ownership. This should be fixed
separately to make the behavior more consistent and user friendly.
(Added comments in the code wherever necessary along with a test
to simulate this).
Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/authorization/Authorizable.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M
fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaResourceBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeDb.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M tests/authorization/test_ranger.py
30 files changed, 570 insertions(+), 101 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14106/11
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 11
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 10: (1 comment) http://gerrit.cloudera.org:8080/#/c/14106/10/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/14106/10/tests/authorization/test_ranger.py@711 PS10, Line 711: s flake8: E501 line too long (93 > 90 characters) -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 10 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 11 Sep 2019 23:31:59 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 10: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4939/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 10 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 11 Sep 2019 23:31:50 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 10: Any more reviews for this patch? -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 10 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 11 Sep 2019 23:31:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 10: (2 comments) http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java File fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java: http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java@134 PS6, Line 134: path_.getRawPath().get(0), path_.getRootTable().getOwnerUser()) > nit: path_.getRootTable().getOwnerUser() should be on a new line to follow don't think we have any formatting rules around this, it was mostly around not exceeding line widths. So skipping for now. http://gerrit.cloudera.org:8080/#/c/14106/8/tests/authorization/test_ranger.py File tests/authorization/test_ranger.py: http://gerrit.cloudera.org:8080/#/c/14106/8/tests/authorization/test_ranger.py@699 PS8, Line 699: result =\ > Do we need to assert anything from the result of this command? Done -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 10 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 11 Sep 2019 23:31:29 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Hello Austin Nobis, Fredy Wijaya, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14106
to look at the new patch set (#10).
Change subject: IMPALA-8228: Ownership support for Ranger authz
..
IMPALA-8228: Ownership support for Ranger authz
Without this patch, explicit privileges are needed even
for owners of databases/tables to perform actions on them.
Example: 'user' is the owner of database 'foo'. To create
a table 't' under 'foo', 'user' needs to be granted a CREATE
privilege on 'foo'
That is unintuitive from a user POV since users expect owners
to have ALL privileges on the objects they own. This patch extends
that support to Impala's ranger authorization plugin.
Ranger natively supports the concept of ownership by letting the
callers pass the ownership context to RangerAccessResourceImpl.
This patch plumbs the owner information for the authorizables
(currently only supported for Tables / Databases) which is then
evaulated during authorization.
For the ownership based authorization to work, ranger-admin side
policy on {OWNER} user needs to be defined.
Testing: Added some unit-tests and e-e tests that cover scenarios
where ownership is used for authorization.
Caveat: Ownership is a part of HMS thrift object. Since we do not
aggressively load HMS schemas during start-up, coordinators with
cold caches can result in weird table listings due to lack of
metadata needed for verifying ownership. This should be fixed
separately to make the behavior more consistent and user friendly.
(Added comments in the code wherever necessary along with a test
to simulate this).
Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/authorization/Authorizable.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M
fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaResourceBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeDb.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M tests/authorization/test_ranger.py
30 files changed, 570 insertions(+), 101 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14106/10
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 10
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 9: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 9 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 11 Sep 2019 20:54:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4541/ : 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/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 9 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 11 Sep 2019 17:29:40 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 9: Added a delete-retry logic in creation of test policies. -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 9 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 11 Sep 2019 16:49:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 9: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4937/ DRY_RUN=true -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 9 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 11 Sep 2019 16:49:11 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Hello Austin Nobis, Fredy Wijaya, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14106
to look at the new patch set (#9).
Change subject: IMPALA-8228: Ownership support for Ranger authz
..
IMPALA-8228: Ownership support for Ranger authz
Without this patch, explicit privileges are needed even
for owners of databases/tables to perform actions on them.
Example: 'user' is the owner of database 'foo'. To create
a table 't' under 'foo', 'user' needs to be granted a CREATE
privilege on 'foo'
That is unintuitive from a user POV since users expect owners
to have ALL privileges on the objects they own. This patch extends
that support to Impala's ranger authorization plugin.
Ranger natively supports the concept of ownership by letting the
callers pass the ownership context to RangerAccessResourceImpl.
This patch plumbs the owner information for the authorizables
(currently only supported for Tables / Databases) which is then
evaulated during authorization.
For the ownership based authorization to work, ranger-admin side
policy on {OWNER} user needs to be defined.
Testing: Added some unit-tests and e-e tests that cover scenarios
where ownership is used for authorization.
Caveat: Ownership is a part of HMS thrift object. Since we do not
aggressively load HMS schemas during start-up, coordinators with
cold caches can result in weird table listings due to lack of
metadata needed for verifying ownership. This should be fixed
separately to make the behavior more consistent and user friendly.
(Added comments in the code wherever necessary along with a test
to simulate this).
Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/authorization/Authorizable.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M
fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaResourceBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeDb.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M tests/authorization/test_ranger.py
30 files changed, 562 insertions(+), 101 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14106/9
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 9
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 8: The test fails because of the remnants from earlier tests that granted privileges on the same functional.*.*. I'm looking into fixing it. -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 8 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 11 Sep 2019 16:00:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 8: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4934/ -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 8 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 11 Sep 2019 12:54:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4539/ : 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/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 8 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 11 Sep 2019 09:28:04 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4538/ : 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/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 7 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 11 Sep 2019 09:23:56 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Hello Austin Nobis, Fredy Wijaya, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14106
to look at the new patch set (#8).
Change subject: IMPALA-8228: Ownership support for Ranger authz
..
IMPALA-8228: Ownership support for Ranger authz
Without this patch, explicit privileges are needed even
for owners of databases/tables to perform actions on them.
Example: 'user' is the owner of database 'foo'. To create
a table 't' under 'foo', 'user' needs to be granted a CREATE
privilege on 'foo'
That is unintuitive from a user POV since users expect owners
to have ALL privileges on the objects they own. This patch extends
that support to Impala's ranger authorization plugin.
Ranger natively supports the concept of ownership by letting the
callers pass the ownership context to RangerAccessResourceImpl.
This patch plumbs the owner information for the authorizables
(currently only supported for Tables / Databases) which is then
evaulated during authorization.
For the ownership based authorization to work, ranger-admin side
policy on {OWNER} user needs to be defined.
Testing: Added some unit-tests and e-e tests that cover scenarios
where ownership is used for authorization.
Caveat: Ownership is a part of HMS thrift object. Since we do not
aggressively load HMS schemas during start-up, coordinators with
cold caches can result in weird table listings due to lack of
metadata needed for verifying ownership. This should be fixed
separately to make the behavior more consistent and user friendly.
(Added comments in the code wherever necessary along with a test
to simulate this).
Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/authorization/Authorizable.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M
fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaResourceBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeDb.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M tests/authorization/test_ranger.py
30 files changed, 543 insertions(+), 101 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14106/8
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 8
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/14106 )
Change subject: IMPALA-8228: Ownership support for Ranger authz
..
Patch Set 7:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14106/7/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:
http://gerrit.cloudera.org:8080/#/c/14106/7/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@176
PS7, Line 176: builder.onTableUnknownOwner(dbName,
tableName_.getTbl()).allOf(Privilege.REFRESH).build());
> line too long (107 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/14106/7/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@179
PS7, Line 179: dbName, tableName_.getTbl(),
tbl.getOwnerUser()).allOf(Privilege.REFRESH).build());
> line too long (99 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/14106/7/fe/src/main/java/org/apache/impala/authorization/Authorizable.java
File fe/src/main/java/org/apache/impala/authorization/Authorizable.java:
http://gerrit.cloudera.org:8080/#/c/14106/7/fe/src/main/java/org/apache/impala/authorization/Authorizable.java@64
PS7, Line 64: public int hashCode() {
> line has trailing whitespace
Done
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 7
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Wed, 11 Sep 2019 08:47:30 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/14106 )
Change subject: IMPALA-8228: Ownership support for Ranger authz
..
Patch Set 7:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14106/7/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:
http://gerrit.cloudera.org:8080/#/c/14106/7/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@176
PS7, Line 176: builder.onTableUnknownOwner(dbName,
tableName_.getTbl()).allOf(Privilege.REFRESH).build());
line too long (107 > 90)
http://gerrit.cloudera.org:8080/#/c/14106/7/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@179
PS7, Line 179: dbName, tableName_.getTbl(),
tbl.getOwnerUser()).allOf(Privilege.REFRESH).build());
line too long (99 > 90)
http://gerrit.cloudera.org:8080/#/c/14106/7/fe/src/main/java/org/apache/impala/authorization/Authorizable.java
File fe/src/main/java/org/apache/impala/authorization/Authorizable.java:
http://gerrit.cloudera.org:8080/#/c/14106/7/fe/src/main/java/org/apache/impala/authorization/Authorizable.java@64
PS7, Line 64: public int hashCode() {
line has trailing whitespace
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 7
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Wed, 11 Sep 2019 08:44:07 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Hello Austin Nobis, Fredy Wijaya, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14106
to look at the new patch set (#7).
Change subject: IMPALA-8228: Ownership support for Ranger authz
..
IMPALA-8228: Ownership support for Ranger authz
Without this patch, explicit privileges are needed even
for owners of databases/tables to perform actions on them.
Example: 'user' is the owner of database 'foo'. To create
a table 't' under 'foo', 'user' needs to be granted a CREATE
privilege on 'foo'
That is unintuitive from a user POV since users expect owners
to have ALL privileges on the objects they own. This patch extends
that support to Impala's ranger authorization plugin.
Ranger natively supports the concept of ownership by letting the
callers pass the ownership context to RangerAccessResourceImpl.
This patch plumbs the owner information for the authorizables
(currently only supported for Tables / Databases) which is then
evaulated during authorization.
For the ownership based authorization to work, ranger-admin side
policy on {OWNER} user needs to be defined.
Testing: Added some unit-tests and e-e tests that cover scenarios
where ownership is used for authorization.
Caveat: Ownership is a part of HMS thrift object. Since we do not
aggressively load HMS schemas during start-up, coordinators with
cold caches can result in weird table listings due to lack of
metadata needed for verifying ownership. This should be fixed
separately to make the behavior more consistent and user friendly.
(Added comments in the code wherever necessary along with a test
to simulate this).
Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/authorization/Authorizable.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M
fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaResourceBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeDb.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M tests/authorization/test_ranger.py
30 files changed, 541 insertions(+), 101 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14106/7
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 7
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/14106 )
Change subject: IMPALA-8228: Ownership support for Ranger authz
..
Patch Set 6:
(14 comments)
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/Authorizable.java
File fe/src/main/java/org/apache/impala/authorization/Authorizable.java:
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/Authorizable.java@62
PS6, Line 62: return getName().hashCode();
> If we're comparing owner, we should update the hashCode() as well.
Done
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/Authorizable.java@62
PS6, Line 62: return getName().hashCode();
> If we're comparing owner, we should update the hashCode() as well.
Done
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/Authorizable.java@70
PS6, Line 70: temp.getOwnerUser() == this.getOwnerUser()
> Shouldn't this use equals()?
Done
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java:
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java@31
PS6, Line 31: private final String ownerUser_;
> For consistency, annotate with @Nullable.
Done
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java@34
PS6, Line 34: String ownerUser
> I think it's more readable to also annotate the parameter with @Nullable.
Done
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java:
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java@33
PS6, Line 33: String ownerUser
> Same comment about @Nullable parameter.
Done
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java:
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java@45
PS6, Line 45: /**
:* Creates a new instance of column {@link Authorizable} for a
given database name and
:* gives access to all tables and columns.
:*/
: Authorizable newColumnAllTbls(String dbName, @Nullable String
dbOwnerUser);
:
: /**
:* Creates a new instance of column {@link Authorizable} for
given database and table
:* names and gives access to all columns.
:*/
: Authorizable newColumnInTable(
: String dbName, String tableName, @Nullable String
tblOwnerUser);
:
: /**
:* Creates a new instance of column {@link Authorizable} for
given database, table, and
:* column names.
:*/
: Authorizable newColumnInTable(
: String dbName, String tableName, String columnName,
@Nullable String tblOwnerUser);
> nit: update javadoc about the new owner parameter.
Done
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java:
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java@34
PS6, Line 34: String ownerUser
> Same comment about @Nullable parameter.
Done
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@94
PS6, Line 94: resources.add(new RangerImpalaResourceBuilder()
: .database("*").table("*").column("*").build());
: resources.add(new RangerImpalaResourceBuilder()
: .database("*").function("*").build());
: resources.add(new
RangerImpalaResourceBuilder().uri("*").build());
> Should we add .owner() here as well?
This patch was meant to add the ownership only for db/tbl[col]. Also curious
what does it even mean to be a owner of a server?
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/catalog/Db.java
File fe/src/main/java/org/apache/impala/catalog/Db.java:
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/14106 )
Change subject: IMPALA-8228: Ownership support for Ranger authz
..
Patch Set 6:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/Authorizable.java
File fe/src/main/java/org/apache/impala/authorization/Authorizable.java:
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/Authorizable.java@62
PS6, Line 62: return getName().hashCode();
If we're comparing owner, we should update the hashCode() as well.
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/Authorizable.java@70
PS6, Line 70: temp.getOwnerUser() == this.getOwnerUser()
Shouldn't this use equals()?
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java:
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java@31
PS6, Line 31: private final String ownerUser_;
For consistency, annotate with @Nullable.
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java@34
PS6, Line 34: String ownerUser
I think it's more readable to also annotate the parameter with @Nullable.
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java:
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java@33
PS6, Line 33: String ownerUser
Same comment about @Nullable parameter.
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java:
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java@45
PS6, Line 45: /**
:* Creates a new instance of column {@link Authorizable} for a
given database name and
:* gives access to all tables and columns.
:*/
: Authorizable newColumnAllTbls(String dbName, @Nullable String
dbOwnerUser);
:
: /**
:* Creates a new instance of column {@link Authorizable} for
given database and table
:* names and gives access to all columns.
:*/
: Authorizable newColumnInTable(
: String dbName, String tableName, @Nullable String
tblOwnerUser);
:
: /**
:* Creates a new instance of column {@link Authorizable} for
given database, table, and
:* column names.
:*/
: Authorizable newColumnInTable(
: String dbName, String tableName, String columnName,
@Nullable String tblOwnerUser);
nit: update javadoc about the new owner parameter.
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java:
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java@34
PS6, Line 34: String ownerUser
Same comment about @Nullable parameter.
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
File
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java:
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@94
PS6, Line 94: resources.add(new RangerImpalaResourceBuilder()
: .database("*").table("*").column("*").build());
: resources.add(new RangerImpalaResourceBuilder()
: .database("*").function("*").build());
: resources.add(new
RangerImpalaResourceBuilder().uri("*").build());
Should we add .owner() here as well?
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/catalog/Db.java
File fe/src/main/java/org/apache/impala/catalog/Db.java:
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/catalog/Db.java@544
PS6, Line 544: if (db == null) return null;
: return db.getOwnerName();
nit: can be inlined: return db == null ? null : db.getOwnerName()
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/14106 )
Change subject: IMPALA-8228: Ownership support for Ranger authz
..
Patch Set 6:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14106/6/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/14106/6/fe/src/main/java/org/apache/impala/service/Frontend.java@783
PS6, Line 783: TODO
mind filing a JIRA and putting the number here?
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
File
fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java:
http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@3060
PS6, Line 3060: authorize("select count(*) from functional.alltypes")
nit: might make this test a little easier to read to do something like create
an Map:
ImmutableMap.builder()
.put("select count(*) from functional.alltypes",
selectError("functional.alltypes"))
.put("select id from functional.alltypes",
selectError("functional.alltypes"))
...
.build();
and then you can loop over the queries in the before/after case below to avoid
having to duplicate all the queries twice
http://gerrit.cloudera.org:8080/#/c/14106/6/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:
http://gerrit.cloudera.org:8080/#/c/14106/6/tests/authorization/test_ranger.py@665
PS6, Line 665: # Run show tables on the db. The resulting list should be
empty. This happens
: # because the created table's ownership information is
not aggresively cached
: # by the current Catalog implementations. Hence the
analysis pass does not
: # have access to the ownership information to verify if
the current session
: # user is actually the owner. We need to fix this by
caching the HMS metadata
: # more aggressively when the table loads.
this is true only in localcatalog, right? otherwise the 'create table' call
would have fully populated the cache rather than leaving an incomplete table?
http://gerrit.cloudera.org:8080/#/c/14106/6/tests/authorization/test_ranger.py@669
PS6, Line 669: We need to fix this by caching the HMS metadata
: # more aggressively when the table loads.
add a TODO(IMPALA-x) with jira
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 6
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Tue, 10 Sep 2019 23:49:33 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 6: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4499/ : 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/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 6 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 09 Sep 2019 16:18:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 6: Todd, this is ready for review. Added tests. I think the code can be refactored better to make it easy to understand and i'm looking into it. Meanwhile pushing it out for review. -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 6 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 09 Sep 2019 15:40:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: IMPALA-8228: Ownership support for Ranger authz .. Patch Set 6: (2 comments) http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java: http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@176 PS6, Line 176: builder.onTableUnknownOwner(dbName, tableName_.getTbl()).allOf(Privilege.REFRESH).build()); line too long (107 > 90) http://gerrit.cloudera.org:8080/#/c/14106/6/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@179 PS6, Line 179: dbName, tableName_.getTbl(), tbl.getOwnerUser()).allOf(Privilege.REFRESH).build()); line too long (99 > 90) -- To view, visit http://gerrit.cloudera.org:8080/14106 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4 Gerrit-Change-Number: 14106 Gerrit-PatchSet: 6 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Mon, 09 Sep 2019 15:39:07 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-8228: Ownership support for Ranger authz
Hello Austin Nobis, Todd Lipcon, Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/14106
to look at the new patch set (#6).
Change subject: IMPALA-8228: Ownership support for Ranger authz
..
IMPALA-8228: Ownership support for Ranger authz
Without this patch, explicit privileges are needed even
for owners of databases/tables to perform actions on them.
Example: 'user' is the owner of database 'foo'. To create
a table 't' under 'foo', 'user' needs to be granted a CREATE
privilege on 'foo'
That is unintuitive from a user POV since users expect owners
to have ALL privileges on the objects they own. This patch extends
that support to Impala's ranger authorization plugin.
Ranger natively supports the concept of ownership by letting the
callers pass the ownership context to RangerAccessResourceImpl.
This patch plumbs the owner information for the authorizables
(currently only supported for Tables / Databases) which is then
evaulated during authorization.
For the ownership based authorization to work, ranger-admin side
policy on {OWNER} user needs to be defined.
Testing: Added some unit-tests and e-e tests that cover scenarios
where ownership is used for authorization.
Caveat: Ownership is a part of HMS thrift object. Since we do not
aggressively load HMS schemas during start-up, coordinators with
cold caches can result in weird table listings due to lack of
metadata needed for verifying ownership. This should be fixed
separately to make the behavior more consistent and user friendly.
(Added comments in the code wherever necessary along with a test
to simulate this).
Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CollectionTableRef.java
M fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.java
M fe/src/main/java/org/apache/impala/analysis/DescribeTableStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropDbStmt.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InsertStmt.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/analysis/SelectStmt.java
M fe/src/main/java/org/apache/impala/authorization/Authorizable.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableColumn.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableDb.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M
fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M
fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpalaResourceBuilder.java
M
fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/catalog/BuiltinsDb.java
M fe/src/main/java/org/apache/impala/catalog/Db.java
M fe/src/main/java/org/apache/impala/catalog/FeDb.java
M fe/src/main/java/org/apache/impala/catalog/FeTable.java
M fe/src/main/java/org/apache/impala/catalog/Table.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalDb.java
M fe/src/main/java/org/apache/impala/catalog/local/LocalTable.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M tests/authorization/test_ranger.py
30 files changed, 523 insertions(+), 96 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14106/6
--
To view, visit http://gerrit.cloudera.org:8080/14106
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
Gerrit-Change-Number: 14106
Gerrit-PatchSet: 6
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
