[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 5: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4825/ -- 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: 5 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 21 Aug 2019 02:19:13 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4318/ : 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: 5 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 21 Aug 2019 01:23:43 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 5: (1 comment) http://gerrit.cloudera.org:8080/#/c/14106/5/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/5/fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java@55 PS5, Line 55: Authorizable newColumnInTable(String dbName, String tableName, @Nullable String tblOwnerUser); line too long (96 > 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: 5 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 21 Aug 2019 00:45:10 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 5: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4825/ 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: 5 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 21 Aug 2019 00:45:30 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] 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 (#5).
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
[WIP] 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.
(TODO) Working on tests.
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/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
25 files changed, 237 insertions(+), 88 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14106/5
--
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: 5
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 4: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/4823/ -- 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: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 20 Aug 2019 23:22:22 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4314/ : 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: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 20 Aug 2019 22:31:32 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 4: (1 comment) http://gerrit.cloudera.org:8080/#/c/14106/4/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/4/fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java@55 PS4, Line 55: Authorizable newColumnInTable(String dbName, String tableName, @Nullable String tblOwnerUser); line too long (96 > 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: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 20 Aug 2019 21:52:38 + Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 4: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/4823/ 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: 4 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 20 Aug 2019 21:53:20 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/14106 )
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
Patch Set 4:
(13 comments)
Still working on tests. Meanwhile kicking off a test run to see what fails.
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2673
PS2, Line 2673: Preconditions.checkNotNull(privilege);
> we seem to have lost the "checkNotNull' here? was that intentional?
Done
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2818
PS3, Line 2818: } else {
> line too long (91 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2819
PS3, Line 2819: // Table does not exist and hence the owner information
cannot be deduced.
> line too long (92 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/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/2/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@138
PS2, Line 138:
> this could be null in the case of non-table-specific statements, which seem
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@189
PS2, Line 189: databa
> this should be 'database_' right?
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/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/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java@35
PS2, Line 35:
> mind adding @Nullable annotations here and below, if this is allowed to be
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/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/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java@30
PS2, Line 30: private final String tableName_;
> how about using @Nullable on this?
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java@37
PS2, Line 37: Preconditions.checkArgument(ownerUser == null ||
!ownerUser.isEmpty());
> would an empty owner string be valid? maybe we should be checking that it's
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java@55
PS2, Line 55: @Override
> this is @Override right?
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
File
fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@71
PS2, Line 71:
> mind giving this a more explicit name like 'onTableWithUnknownOwner' or som
Done
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@74
PS2, Line 74: public PrivilegeRequestBuilder onTable(
> typo
Done
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
File
fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java:
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@79
PS3, Line 79: }
> line too long (93 > 90)
Done
http://gerrit.cloudera.org:8080/#/c/14106/3/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/3/fe/src/main/java/org/apache/impala/service/Frontend.java@786
PS3, Line 786: String tableOwner = table.getOwnerUser();
> line too long (96 > 90)
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: 4
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath
[Impala-ASF-CR] [WIP] 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 (#4).
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
[WIP] 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.
(TODO) Working on tests.
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/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
25 files changed, 233 insertions(+), 85 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14106/4
--
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: 4
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 3: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4311/ : 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: 3 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 20 Aug 2019 21:32:58 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 3: Oops I didn't mean to push this out for review, still haven't addressed the comments. -- 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: 3 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 20 Aug 2019 20:52:22 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/14106 )
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
Patch Set 3:
(4 comments)
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2818
PS3, Line 2818: // Table does not exist and hence the owner information
cannot be deduced. Register
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2819
PS3, Line 2819: // a privilege request on the db and table name to mask
the TableNotFound exceptions
line too long (92 > 90)
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
File
fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java:
http://gerrit.cloudera.org:8080/#/c/14106/3/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@79
PS3, Line 79: public PrivilegeRequestBuilder onTable(String dbName, String
tableName, String ownerUser) {
line too long (93 > 90)
http://gerrit.cloudera.org:8080/#/c/14106/3/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/3/fe/src/main/java/org/apache/impala/service/Frontend.java@786
PS3, Line 786: "Table {} not yet loaded, ignoring it in table
listing.", dbName + "." + tblName);
line too long (96 > 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: 3
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Tue, 20 Aug 2019 20:50:51 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] 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 (#3).
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
[WIP] 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.
(TODO) Working on tests.
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/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
25 files changed, 250 insertions(+), 85 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14106/3
--
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: 3
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Todd Lipcon has posted comments on this change. (
http://gerrit.cloudera.org:8080/14106 )
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
Patch Set 2:
(9 comments)
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2673
PS2, Line 2673: FeTable table = getTable(fqTableName.getDb(),
fqTableName.getTbl());
we seem to have lost the "checkNotNull' here? was that intentional?
http://gerrit.cloudera.org:8080/#/c/14106/2/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/2/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@138
PS2, Line 138: tableName_
this could be null in the case of non-table-specific statements, which seems
like it would fail
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@189
PS2, Line 189: dbName
this should be 'database_' right?
http://gerrit.cloudera.org:8080/#/c/14106/2/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/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java@35
PS2, Line 35: String ownerUser
mind adding @Nullable annotations here and below, if this is allowed to be null
to indicate no known owner?
http://gerrit.cloudera.org:8080/#/c/14106/2/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/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java@30
PS2, Line 30: private final String ownerUser_;
how about using @Nullable on this?
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java@37
PS2, Line 37: ownerUser_ = ownerUser;
would an empty owner string be valid? maybe we should be checking that it's not
empty?
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java@55
PS2, Line 55: public String getOwnerUser() { return ownerUser_; }
this is @Override right?
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
File
fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@71
PS2, Line 71: onTable
mind giving this a more explicit name like 'onTableWithUnknownOwner' or
something? think that's better than just overloading, so it's clear that when
you have an owner you should use a different call. Or, get rid of this overload
and explicitly pass the null owner at call sites
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@74
PS2, Line 74: // TableNotFound Analsis exceptions and instead mask that as
an
typo
--
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: 2
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Tue, 20 Aug 2019 17:25:20 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14106 ) Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz .. Patch Set 2: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/4308/ : 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: 2 Gerrit-Owner: Bharath Vissapragada Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 20 Aug 2019 06:59:48 + Gerrit-HasComments: No
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Impala Public Jenkins has posted comments on this change. (
http://gerrit.cloudera.org:8080/14106 )
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java
File fe/src/main/java/org/apache/impala/analysis/Analyzer.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2813
PS2, Line 2813: // Table does not exist and hence the owner information
cannot be deduced. Register
line too long (91 > 90)
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/analysis/Analyzer.java@2814
PS2, Line 2814: // a privilege request on the db and table name to mask
the TableNotFound exceptions
line too long (92 > 90)
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
File
fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java:
http://gerrit.cloudera.org:8080/#/c/14106/2/fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java@89
PS2, Line 89: public PrivilegeRequestBuilder onTable(String dbName, String
tableName, String ownerUser) {
line too long (93 > 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: 2
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Austin Nobis
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Todd Lipcon
Gerrit-Comment-Date: Tue, 20 Aug 2019 06:19:50 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] [WIP] IMPALA-8228: Ownership support for Ranger authz
Hello 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 (#2).
Change subject: [WIP] IMPALA-8228: Ownership support for Ranger authz
..
[WIP] 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.
(TODO) Working on tests.
Change-Id: I737b7164a3e7afb9996b3402e6872effd663f7b4
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/CopyTestCaseStmt.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/authorization/Authorizable.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/service/Frontend.java
14 files changed, 107 insertions(+), 46 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/06/14106/2
--
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: 2
Gerrit-Owner: Bharath Vissapragada
Gerrit-Reviewer: Impala Public Jenkins
