You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Austin Nobis (Code Review)" <ge...@cloudera.org> on 2019/06/18 20:00:27 UTC
[Impala-ASF-CR] [IMPALA-8587] Show inherited privileges with Ranger show grant
Austin Nobis has uploaded this change for review. ( http://gerrit.cloudera.org:8080/13673
Change subject: [IMPALA-8587] Show inherited privileges with Ranger show grant
......................................................................
[IMPALA-8587] Show inherited privileges with Ranger show grant
Previously when executing a show grant statement on a resource with
Ranger authorization enabled, Impala would not show inherited
privileges. For example, if a user had database level privileges such
as:
GRANT SELECT ON DATABASE db TO USER user;
If a user then requested table level privileges such as:
SHOW GRANT USER user ON TABLE db.table;
They would see no results. After this change, the user will see database
level privileges when executing the previous statement. If a user has
SELECT privilege on DATABASE and on TABLE and issues a show grant on
TABLE, they will only see the SELECT privilege for TABLE. Users will not
see multiple instances of SELECT or any other privilege type in a SHOW
GRANT statemenet.
Testing
- Ran all FE tests
- Ran all authorization E2E tests
- Added E2E tests in test_ranger verifying functionality
Change-Id: I5c4c9327acb12abc12d130ef5c1ace6a08ed193c
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
M tests/authorization/test_ranger.py
2 files changed, 147 insertions(+), 27 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/13673/1
--
To view, visit http://gerrit.cloudera.org:8080/13673
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I5c4c9327acb12abc12d130ef5c1ace6a08ed193c
Gerrit-Change-Number: 13673
Gerrit-PatchSet: 1
Gerrit-Owner: Austin Nobis <an...@cloudera.com>
[Impala-ASF-CR] [IMPALA-8587] Show inherited privileges with Ranger show grant
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13673 )
Change subject: [IMPALA-8587] Show inherited privileges with Ranger show grant
......................................................................
Patch Set 1:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13673/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java:
http://gerrit.cloudera.org:8080/#/c/13673/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@341
PS1, Line 341: resourceResult.getResultRows().forEach(principal -> resultSet.add(principal.toResultRow()));
line too long (98 > 90)
http://gerrit.cloudera.org:8080/#/c/13673/1/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:
http://gerrit.cloudera.org:8080/#/c/13673/1/tests/authorization/test_ranger.py@195
PS1, Line 195: t
flake8: E501 line too long (96 > 90 characters)
http://gerrit.cloudera.org:8080/#/c/13673/1/tests/authorization/test_ranger.py@350
PS1, Line 350: )
flake8: E501 line too long (92 > 90 characters)
--
To view, visit http://gerrit.cloudera.org:8080/13673
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c4c9327acb12abc12d130ef5c1ace6a08ed193c
Gerrit-Change-Number: 13673
Gerrit-PatchSet: 1
Gerrit-Owner: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 20:01:04 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] [IMPALA-8587] Show inherited privileges with Ranger show grant
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13673 )
Change subject: [IMPALA-8587] Show inherited privileges with Ranger show grant
......................................................................
Patch Set 3:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13673/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java:
http://gerrit.cloudera.org:8080/#/c/13673/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@367
PS3, Line 367: private static class RangerResourceResult {
: List<RangerResultRow> server = new ArrayList<>();
: List<RangerResultRow> uri = new ArrayList<>();
: List<RangerResultRow> database = new ArrayList<>();
: List<RangerResultRow> udf = new ArrayList<>();
: List<RangerResultRow> table = new ArrayList<>();
: List<RangerResultRow> column = new ArrayList<>();
make these private
http://gerrit.cloudera.org:8080/#/c/13673/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@392
PS3, Line 392: public RangerResourceResult addUdfResult(RangerResultRow result) {
: udf.add(result);
: return this;
: }
:
: public RangerResourceResult addUriResult(RangerResultRow result) {
: uri.add(result);
: return this;
: }
These two methods are unused, that means uri and udf will always be empty.
http://gerrit.cloudera.org:8080/#/c/13673/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@407
PS3, Line 407: public List<RangerResultRow> getResultRows() {
: List<RangerResultRow> results = new ArrayList<>();
:
: results.addAll(filterIfAll(server));
: results.addAll(filterIfAll(database));
: results.addAll(filterIfAll(table));
: results.addAll(filterIfAll(column));
: results.addAll(filterIfAll(udf));
: results.addAll(filterIfAll(uri));
:
: return results;
: }
I don't quite follow the logic why we have to filter ALL. A comment will be helpful.
--
To view, visit http://gerrit.cloudera.org:8080/13673
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c4c9327acb12abc12d130ef5c1ace6a08ed193c
Gerrit-Change-Number: 13673
Gerrit-PatchSet: 3
Gerrit-Owner: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Fri, 28 Jun 2019 20:38:14 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] [IMPALA-8587] Show inherited privileges with Ranger show grant
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13673 )
Change subject: [IMPALA-8587] Show inherited privileges with Ranger show grant
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3662/ : 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/13673
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c4c9327acb12abc12d130ef5c1ace6a08ed193c
Gerrit-Change-Number: 13673
Gerrit-PatchSet: 1
Gerrit-Owner: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 20:42:11 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [IMPALA-8587] Show inherited privileges with Ranger show grant
Posted by "Austin Nobis (Code Review)" <ge...@cloudera.org>.
Austin Nobis has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/13673 )
Change subject: [IMPALA-8587] Show inherited privileges with Ranger show grant
......................................................................
[IMPALA-8587] Show inherited privileges with Ranger show grant
Previously when executing a show grant statement on a resource with
Ranger authorization enabled, Impala would not show inherited
privileges. For example, if a user had database level privileges such
as:
GRANT SELECT ON DATABASE db TO USER user;
If a user then requested table level privileges such as:
SHOW GRANT USER user ON TABLE db.table;
They would see no results. After this change, the user will see database
level privileges when executing the previous statement. If a user has
SELECT privilege on DATABASE and on TABLE and issues a show grant on
TABLE, they will only see the SELECT privilege for TABLE. Users will not
see multiple instances of SELECT or any other privilege type in a SHOW
GRANT statemenet.
Testing
- Ran all FE tests
- Ran all authorization E2E tests
- Added E2E tests in test_ranger verifying functionality
Change-Id: I5c4c9327acb12abc12d130ef5c1ace6a08ed193c
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
M tests/authorization/test_ranger.py
2 files changed, 144 insertions(+), 24 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/13673/2
--
To view, visit http://gerrit.cloudera.org:8080/13673
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5c4c9327acb12abc12d130ef5c1ace6a08ed193c
Gerrit-Change-Number: 13673
Gerrit-PatchSet: 2
Gerrit-Owner: Austin Nobis <an...@cloudera.com>
[Impala-ASF-CR] [IMPALA-8587] Show inherited privileges with Ranger show grant
Posted by "Austin Nobis (Code Review)" <ge...@cloudera.org>.
Hello Impala Public Jenkins,
I'd like you to reexamine a change. Please visit
http://gerrit.cloudera.org:8080/13673
to look at the new patch set (#3).
Change subject: [IMPALA-8587] Show inherited privileges with Ranger show grant
......................................................................
[IMPALA-8587] Show inherited privileges with Ranger show grant
Previously when executing a show grant statement on a resource with
Ranger authorization enabled, Impala would not show inherited
privileges. For example, if a user had database level privileges such
as:
GRANT SELECT ON DATABASE db TO USER user;
If a user then requested table level privileges such as:
SHOW GRANT USER user ON TABLE db.table;
They would see no results. After this change, the user will see database
level privileges when executing the previous statement. If a user has
SELECT privilege on DATABASE and on TABLE and issues a show grant on
TABLE, they will only see the SELECT privilege for TABLE. Users will not
see multiple instances of SELECT or any other privilege type in a SHOW
GRANT statemenet.
Testing
- Ran all FE tests
- Ran all authorization E2E tests
- Added E2E tests in test_ranger verifying functionality
Change-Id: I5c4c9327acb12abc12d130ef5c1ace6a08ed193c
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
M tests/authorization/test_ranger.py
2 files changed, 147 insertions(+), 24 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/73/13673/3
--
To view, visit http://gerrit.cloudera.org:8080/13673
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I5c4c9327acb12abc12d130ef5c1ace6a08ed193c
Gerrit-Change-Number: 13673
Gerrit-PatchSet: 3
Gerrit-Owner: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
[Impala-ASF-CR] [IMPALA-8587] Show inherited privileges with Ranger show grant
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13673 )
Change subject: [IMPALA-8587] Show inherited privileges with Ranger show grant
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3664/ : 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/13673
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c4c9327acb12abc12d130ef5c1ace6a08ed193c
Gerrit-Change-Number: 13673
Gerrit-PatchSet: 3
Gerrit-Owner: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 20:48:25 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] [IMPALA-8587] Show inherited privileges with Ranger show grant
Posted by "Fredy Wijaya (Code Review)" <ge...@cloudera.org>.
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/13673 )
Change subject: [IMPALA-8587] Show inherited privileges with Ranger show grant
......................................................................
Patch Set 3:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/13673/3//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/13673/3//COMMIT_MSG@7
PS3, Line 7: [IMPALA-8587]
nit: use IMPALA-8587 format
http://gerrit.cloudera.org:8080/#/c/13673/3//COMMIT_MSG@20
PS3, Line 20: They would see no results. After this change, the user will see database
: level privileges when executing the previous statement. If a user has
: SELECT privilege on DATABASE and on TABLE and issues a show grant on
: TABLE, they will only see the SELECT privilege for TABLE. Users will not
: see multiple instances of SELECT or any other privilege type in a SHOW
: GRANT statemenet.
Show what the new output looks like. It'll be much easier to understand.
We also need more explanation about this especially since this is relates to how the Ranger policy engine works.
http://gerrit.cloudera.org:8080/#/c/13673/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java:
http://gerrit.cloudera.org:8080/#/c/13673/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@330
PS3, Line 330: !=
Use equals(). != or == is for reference equality. Sometimes you get lucky because string intern, but we shouldn't rely on that.
http://gerrit.cloudera.org:8080/#/c/13673/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@329
PS3, Line 329: for (RangerResultRow row : resultRows) {
: if (row.column_ != "*" && !row.column_.isEmpty()) {
: resourceResult.addColumnResult(row);
: } else if (row.table_ != "*" && !row.table_.isEmpty()) {
: resourceResult.addTableResult(row);
: } else if (row.database_ != "*" && !row.database_.isEmpty()) {
: resourceResult.addDatabaseResult(row);
: } else {
: resourceResult.addServerResult(row);
: }
: }
Can you add a comment for this logic? It's not quite clear to me what it's trying to do.
http://gerrit.cloudera.org:8080/#/c/13673/3/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@422
PS3, Line 422:
nit: overly indented, use 4 spaces
--
To view, visit http://gerrit.cloudera.org:8080/13673
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c4c9327acb12abc12d130ef5c1ace6a08ed193c
Gerrit-Change-Number: 13673
Gerrit-PatchSet: 3
Gerrit-Owner: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Fredy Wijaya <fw...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 21:08:45 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] [IMPALA-8587] Show inherited privileges with Ranger show grant
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13673 )
Change subject: [IMPALA-8587] Show inherited privileges with Ranger show grant
......................................................................
Patch Set 2:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/13673/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java:
http://gerrit.cloudera.org:8080/#/c/13673/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@341
PS2, Line 341: resourceResult.getResultRows().forEach(principal -> resultSet.add(principal.toResultRow()));
line too long (98 > 90)
http://gerrit.cloudera.org:8080/#/c/13673/2/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:
http://gerrit.cloudera.org:8080/#/c/13673/2/tests/authorization/test_ranger.py@195
PS2, Line 195: t
flake8: E501 line too long (96 > 90 characters)
http://gerrit.cloudera.org:8080/#/c/13673/2/tests/authorization/test_ranger.py@350
PS2, Line 350: )
flake8: E501 line too long (92 > 90 characters)
--
To view, visit http://gerrit.cloudera.org:8080/13673
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c4c9327acb12abc12d130ef5c1ace6a08ed193c
Gerrit-Change-Number: 13673
Gerrit-PatchSet: 2
Gerrit-Owner: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 20:01:37 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] [IMPALA-8587] Show inherited privileges with Ranger show grant
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/13673 )
Change subject: [IMPALA-8587] Show inherited privileges with Ranger show grant
......................................................................
Patch Set 2:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/3663/ : 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/13673
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I5c4c9327acb12abc12d130ef5c1ace6a08ed193c
Gerrit-Change-Number: 13673
Gerrit-PatchSet: 2
Gerrit-Owner: Austin Nobis <an...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Comment-Date: Tue, 18 Jun 2019 20:42:45 +0000
Gerrit-HasComments: No