You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@impala.apache.org by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org> on 2019/12/06 18:29:12 UTC
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Fang-Yu Rao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/14798
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
In IMPALA-9047, we disabled some Ranger-related FE and BE tests due to
changes in Ranger's behavior after upgrading Ranger from 1.2 to 2.0.
This patch aims to re-enable those disabled FE tests in
AuthorizationStmtTest.java and AuthorizationTestBase.java to increase
Impala's test coverage of authorization via Ranger.
There are at least two major changes in Ranger's behavior in the newer
versions.
1. The first is that the owner of the requested resource no longer have
to be explicitly granted privileges in order to access the resource.
2. The second is that a user not explicitly granted the privilege of
creating a database is able to do so.
Due to these changes, some of previous Ranger authorization requests
that were expected to be rejected are now granted after the upgrade.
To re-enable the tests affected by the first change described above, we
modify AuthorizationTestBase.java to allow our FE Ranger authorization
tests to specify the requesting user in an authorization test. Those
tests fail after the upgrade because the default requesting user in
Impala's AuthorizationTestBase.java happens to be the owner of the
resources involved in our FE authorization tests. After this patch, a
requesting user can be either a non-owner user or a owner user. The
requesting user would correspond to a non-owner user in the Ranger
authorization tests if it is not explicitly specified.
On the other hand, for those affected tests by the second change in
AuthorizationStmtTest.java, in this patch we will only run them when the
authorization provider is Sentry. For the affected test in
RangerAuditLogTest.java, we now expect the test query to be successfully
authorized.
Apart from the affected tests mentioned above, we have also found
several test queries that are expected to fail but result in different
authorization error from Sentry and Ranger, which had not been seen in
the previous version of Ranger, i.e., those test queries result in the
same error message from both Sentry and Ranger. For now we modify the
tests accordingly to make the expected error message match the actual
error message, but we should take a closer look at those queries and
see if there is any potential bug.
Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
---
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
M fe/src/test/java/org/apache/impala/common/FrontendFixture.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M testdata/bin/create-load-data.sh
A testdata/cluster/ranger/setup/impala_group_non_owner.json
R testdata/cluster/ranger/setup/impala_group_owner.json.template
A testdata/cluster/ranger/setup/impala_user_non_owner.json.template
R testdata/cluster/ranger/setup/impala_user_owner.json.template
10 files changed, 326 insertions(+), 239 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/14798/1
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has uploaded a new patch set (#3). ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
In IMPALA-9047, we disabled some Ranger-related FE and BE tests due to
changes in Ranger's behavior after upgrading Ranger from 1.2 to 2.0.
This patch aims to re-enable those disabled FE tests in
AuthorizationStmtTest.java and AuthorizationTestBase.java to increase
Impala's test coverage of authorization via Ranger.
There are at least two major changes in Ranger's behavior in the newer
versions.
1. The first is that the owner of the requested resource no longer have
to be explicitly granted privileges in order to access the resource.
2. The second is that a user not explicitly granted the privilege of
creating a database is able to do so.
Due to these changes, some of previous Ranger authorization requests
that were expected to be rejected are now granted after the upgrade.
To re-enable the tests affected by the first change described above, we
modify AuthorizationTestBase.java to allow our FE Ranger authorization
tests to specify the requesting user in an authorization test. Those
tests failed after the upgrade because the default requesting user in
Impala's AuthorizationTestBase.java happens to be the owner of the
resources involved in our FE authorization tests. After this patch, a
requesting user can be either a non-owner user or a owner user. The
requesting user would correspond to a non-owner user in the Ranger
authorization tests if it is not explicitly specified.
On the other hand, for those affected tests by the second change in
AuthorizationStmtTest.java, in this patch we will only run them when the
authorization provider is Sentry. For the affected test in
RangerAuditLogTest.java, we now expect the test query to be successfully
authorized.
Apart from the affected tests mentioned above, we have also found
several test queries that are expected to fail but result in different
authorization error from Sentry and Ranger, which had not been seen in
the previous version of Ranger, i.e., each of those test queries
resulted in the same error message from both Sentry and Ranger before
the upgrade. For now we modify the tests accordingly to make the
expected error message match the actual error message, but we should
take a closer look at those queries and see if there is any potential
bug.
Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
M fe/src/test/java/org/apache/impala/common/FrontendFixture.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M testdata/bin/create-load-data.sh
A testdata/cluster/ranger/setup/impala_group_non_owner.json
R testdata/cluster/ranger/setup/impala_group_owner.json.template
A testdata/cluster/ranger/setup/impala_user_non_owner.json.template
R testdata/cluster/ranger/setup/impala_user_owner.json.template
11 files changed, 333 insertions(+), 240 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/14798/3
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 3
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 11:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5308/ : 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/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 11
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 18 Dec 2019 18:51:29 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
In IMPALA-9047, we disabled some Ranger-related FE and BE tests due to
changes in Ranger's behavior after upgrading Ranger from 1.2 to 2.0.
This patch aims to re-enable those disabled FE tests in
AuthorizationStmtTest.java and RangerAuditLogTest.java to increase
Impala's test coverage of authorization via Ranger.
There are at least two major changes in Ranger's behavior in the newer
versions.
1. The first is that the owner of the requested resource no longer has
to be explicitly granted privileges in order to access the resource.
2. The second is that a user not explicitly granted the privilege of
creating a database is able to do so.
Due to these changes, some of previous Ranger authorization requests
that were expected to be rejected are now granted after the upgrade.
To re-enable the tests affected by the first change described above, we
modify AuthorizationTestBase.java to allow our FE Ranger authorization
tests to specify the requesting user in an authorization test. Those
tests failed after the upgrade because the default requesting user in
Impala's AuthorizationTestBase.java happens to be the owner of the
resources involved in our FE authorization tests. After this patch, a
requesting user can be either a non-owner user or an owner user in a
Ranger authorization test and the requesting user would correspond to a
non-owner user if it is not explicitly specified. Note that in a Sentry
authorization test, we do not use the non-owner user as the requesting
user by default as we do in the Ranger authorization tests. Instead, we
set the name of the requesting user to a name that is the same as the
owner user in Ranger authorization tests to avoid the need for providing
a customized group mapping service when instantiating a Sentry
ResourceAuthorizationProvider as we do in AuthorizationTest.java, our
FE tests specifically for testing authorization via Sentry.
On the other hand, to re-enable the tests affected by the second change,
we remove from the Ranger policy for all databases the allowed
condition that grants any user the privilege of creating a database,
which is not by default granted by Sentry. After the removal of the
allowed codition, those tests in AuthorizationStmtTest.java and
RangerAuditLogTest.java affected by the second change now result in the
same authorization errors before the upgrade of Ranger.
Testing:
- Passed AuthorizationStmtTest.java in a local dev environment
- Passed RangerAuditLogTest.java in a local dev environment
Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Reviewed-on: http://gerrit.cloudera.org:8080/14798
Reviewed-by: Quanlong Huang <hu...@gmail.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
M fe/src/test/java/org/apache/impala/common/FrontendFixture.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M testdata/bin/create-load-data.sh
A testdata/cluster/ranger/setup/impala_group_non_owner.json
R testdata/cluster/ranger/setup/impala_group_owner.json.template
A testdata/cluster/ranger/setup/impala_user_non_owner.json.template
R testdata/cluster/ranger/setup/impala_user_owner.json.template
A testdata/cluster/ranger/setup/policy_4_revised.json
12 files changed, 387 insertions(+), 212 deletions(-)
Approvals:
Quanlong Huang: Looks good to me, approved
Impala Public Jenkins: Verified
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 17
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 9:
(3 comments)
Thanks for addressing the comments!
The changes for Ranger ownership are good to me. But it still looks wired to me that we accept so many different behaviors (not relative to ownership) between Sentry and Ranger...
If the cause is Ranger-2.0 changes the default policies, can we fix the tests by modifying the policies after we create ranger-hive service in bin/create-test-configuration.sh? For instance, I manually edit the "all - database" policy to remove the "Allow Conditions" of CREATE to "public" group. Then normal users can't create databases.
http://gerrit.cloudera.org:8080/#/c/14798/9//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/14798/9//COMMIT_MSG@22
PS9, Line 22: creating a database is able to do so.
Do you know what change(JIRA) of Ranger causes this? Just know that RANGER-2536 causes the first one.
I'm confused why Ranger allows everyone to create databases. It looks unacceptable in a production cluster...
http://gerrit.cloudera.org:8080/#/c/14798/9/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/14798/9/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@1168
PS9, Line 1168: AnalysisError(stmt, "Database does not exist: nodb");
AnalysisError is a method of FrontendTestBase (the super class of AuthorizationTestBase). I'm not quite clear whether authorization is turned on when using it... Could you point me to the codes?
http://gerrit.cloudera.org:8080/#/c/14798/9/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
File fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java:
http://gerrit.cloudera.org:8080/#/c/14798/9/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java@177
PS9, Line 177: user_.getName()
It can be changed to sentry_user_ too.
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 9
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 16 Dec 2019 09:50:35 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 7:
Hi Kurt and Quanlong, please review my revised patch according to Kurt's suggestions in the previous iteration and let me know if you have any further comments and suggestions. Thanks!
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 7
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 11 Dec 2019 18:43:47 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
In IMPALA-9047, we disabled some Ranger-related FE and BE tests due to
changes in Ranger's behavior after upgrading Ranger from 1.2 to 2.0.
This patch aims to re-enable those disabled FE tests in
AuthorizationStmtTest.java and RangerAuditLogTest.java to increase
Impala's test coverage of authorization via Ranger.
There are at least two major changes in Ranger's behavior in the newer
versions.
1. The first is that the owner of the requested resource no longer have
to be explicitly granted privileges in order to access the resource.
2. The second is that a user not explicitly granted the privilege of
creating a database is able to do so.
Due to these changes, some of previous Ranger authorization requests
that were expected to be rejected are now granted after the upgrade.
To re-enable the tests affected by the first change described above, we
modify AuthorizationTestBase.java to allow our FE Ranger authorization
tests to specify the requesting user in an authorization test. Those
tests failed after the upgrade because the default requesting user in
Impala's AuthorizationTestBase.java happens to be the owner of the
resources involved in our FE authorization tests. After this patch, a
requesting user can be either a non-owner user or an owner user in a
Ranger authorization test and the requesting user would correspond to a
non-owner user if it is not explicitly specified. Note that in a Sentry
authorization test, we do not use the non-owner user as the requesting
user by default as we do in the Ranger authorization tests. Instead, we
set the name of the requesting user to the name of the owner user in
Ranger authorization tests to avoid the complexity of having to provide
a customized group mapping service when instantiating a Sentry
ResourceAuthorizationProvider as we do in AuthorizationTest.java, our FE
tests specifically for testing authorization via Sentry.
On the other hand, for those tests affected by the second change in
AuthorizationStmtTest.java, in this patch we will only run them when the
authorization provider is Sentry. For the affected test in
RangerAuditLogTest.java, we now expect the test query to be successfully
authorized.
Apart from the affected tests mentioned above, we have also found
several test queries that are expected to fail but result in different
authorization error from Sentry and Ranger, which had not been seen in
the previous version of Ranger, i.e., each of those test queries
resulted in the same error message from both Sentry and Ranger before
the upgrade. For now we modify the tests accordingly to make the
expected error message match the actual error message, but we should
take a closer look at those queries and see if there is any potential
bug.
Testing:
- Passed AuthorizationStmtTest.java
- Passed RangerAuditLogTest.java
Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
M fe/src/test/java/org/apache/impala/common/FrontendFixture.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M testdata/bin/create-load-data.sh
A testdata/cluster/ranger/setup/impala_group_non_owner.json
R testdata/cluster/ranger/setup/impala_group_owner.json.template
A testdata/cluster/ranger/setup/impala_user_non_owner.json.template
R testdata/cluster/ranger/setup/impala_user_owner.json.template
12 files changed, 342 insertions(+), 244 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/14798/5
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 5
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 1:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5222/ : 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/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 06 Dec 2019 18:58:49 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 7:
(5 comments)
http://gerrit.cloudera.org:8080/#/c/14798/7/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/14798/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@314
PS7, Line 314: if (RuntimeEnv.INSTANCE.isTestEnv()) {
> I think refactoring RangerAuthorizationChecker may need a refactor on Autho
Thanks Quanlong for the detailed explanation!
My original thought is to bind UserGroupInformation with RangerAuthorizationChecker only. According to my current understanding, both Sentry and Ranger may require the group information of the requesting user when performing the authorization. The difference is that Sentry would consult the group mapper with the group information itself, whereas Ranger does not. More specifically, it is Impala's responsibility to retrieve the corresponding group information from an instance of UserGroupInformation and then pass the group information to Ranger.
But I agree with you. We may need more detailed discussion before implementing my proposed approach and we may do it in a follow-up patch.
http://gerrit.cloudera.org:8080/#/c/14798/7/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
File fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java:
http://gerrit.cloudera.org:8080/#/c/14798/7/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java@87
PS7, Line 87: USER
> Rename to user_ since it's now a non-final variable. Same for OWNER_USER, G
Thanks Quanlong! I have changed 'USER' to 'user_'. But do we have to change 'OWNER_USER' to 'owner_user_'? 'OWNER_USER' is currently a final variable.
http://gerrit.cloudera.org:8080/#/c/14798/7/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java@266
PS7, Line 266: // Depending on whether or not the principal is the owner of the resource, we return
> nit: we usually put the comments above the "@Override"
Done
http://gerrit.cloudera.org:8080/#/c/14798/7/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java@270
PS7, Line 270: AS_OWNER == true
> don't need "== true" since it's boolean
Done
http://gerrit.cloudera.org:8080/#/c/14798/7/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java@282
PS7, Line 282: (AS_OWNER ? OWNER_USER.getName() : USER.getName())
> use getName() instead
Thanks for catching this! I have change the line above to getName().
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 7
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 Dec 2019 18:39:10 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 16:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5323/ : 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/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 16
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 20 Dec 2019 05:50:30 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 9:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5285/ : 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/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 9
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sat, 14 Dec 2019 02:13:37 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 7:
(1 comment)
Hi Kurt and Quanlong, I left some additional thoughts on how we should pass an instance of group mapper to RangerAuthorizationChecker.java. Will try to implement the proposed approach in the next iteration since I think the current approach is too hacky and less flexible. Thanks!
http://gerrit.cloudera.org:8080/#/c/14798/7/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/14798/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@314
PS7, Line 314: if (RuntimeEnv.INSTANCE.isTestEnv()) {
After some thoughts, I feel like my approach here is a bit too hacky. A better way is to change that UserGroupInformation to a field of RangerAuthorizationChecker.java. Specifically, I think the following revision would be a better solution.
1. Add an additional field of UserGroupInformation in the class RangerAuthorizationChecker that should be initialized when instantiating a RangerAuthorizationChecker.
2. Add a new constructor of RangerAuthorizationFactory() that takes in an additional argument that allows a user to provide an instance of UserGroupInformation (https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java#L51).
3. Add a new constructor of RangerAuthorizationChecker() that takes in additional argument of UserGroupInformation so that the checker can initialize its own instance of UserGroupInformation passed from its caller, i.e., RangerAuthorizationFactory() in this case (https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java#L72).
I will try to implement the proposed idea above in the next iteration. Thanks!
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 7
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 12 Dec 2019 17:22:29 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
In IMPALA-9047, we disabled some Ranger-related FE and BE tests due to
changes in Ranger's behavior after upgrading Ranger from 1.2 to 2.0.
This patch aims to re-enable those disabled FE tests in
AuthorizationStmtTest.java and RangerAuditLogTest.java to increase
Impala's test coverage of authorization via Ranger.
There are at least two major changes in Ranger's behavior in the newer
versions.
1. The first is that the owner of the requested resource no longer has
to be explicitly granted privileges in order to access the resource.
2. The second is that a user not explicitly granted the privilege of
creating a database is able to do so.
Due to these changes, some of previous Ranger authorization requests
that were expected to be rejected are now granted after the upgrade.
To re-enable the tests affected by the first change described above, we
modify AuthorizationTestBase.java to allow our FE Ranger authorization
tests to specify the requesting user in an authorization test. Those
tests failed after the upgrade because the default requesting user in
Impala's AuthorizationTestBase.java happens to be the owner of the
resources involved in our FE authorization tests. After this patch, a
requesting user can be either a non-owner user or an owner user in a
Ranger authorization test and the requesting user would correspond to a
non-owner user if it is not explicitly specified. Note that in a Sentry
authorization test, we do not use the non-owner user as the requesting
user by default as we do in the Ranger authorization tests. Instead, we
set the name of the requesting user to a name that is the same as the
owner user in Ranger authorization tests to avoid the need for providing
a customized group mapping service when instantiating a Sentry
ResourceAuthorizationProvider as we do in AuthorizationTest.java, our
FE tests specifically for testing authorization via Sentry.
On the other hand, for those tests affected by the second change in
AuthorizationStmtTest.java, in this patch we revise the expected
testing results to match the current behavior of Ranger.
Testing:
- Passed AuthorizationStmtTest.java
- Passed RangerAuditLogTest.java
Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
M fe/src/test/java/org/apache/impala/common/FrontendFixture.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M testdata/bin/create-load-data.sh
A testdata/cluster/ranger/setup/impala_group_non_owner.json
R testdata/cluster/ranger/setup/impala_group_owner.json.template
A testdata/cluster/ranger/setup/impala_user_non_owner.json.template
R testdata/cluster/ranger/setup/impala_user_owner.json.template
11 files changed, 449 insertions(+), 258 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/14798/9
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 9
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 7:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5254/ : 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/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 7
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 11 Dec 2019 19:13:05 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has uploaded a new patch set (#16). ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
In IMPALA-9047, we disabled some Ranger-related FE and BE tests due to
changes in Ranger's behavior after upgrading Ranger from 1.2 to 2.0.
This patch aims to re-enable those disabled FE tests in
AuthorizationStmtTest.java and RangerAuditLogTest.java to increase
Impala's test coverage of authorization via Ranger.
There are at least two major changes in Ranger's behavior in the newer
versions.
1. The first is that the owner of the requested resource no longer has
to be explicitly granted privileges in order to access the resource.
2. The second is that a user not explicitly granted the privilege of
creating a database is able to do so.
Due to these changes, some of previous Ranger authorization requests
that were expected to be rejected are now granted after the upgrade.
To re-enable the tests affected by the first change described above, we
modify AuthorizationTestBase.java to allow our FE Ranger authorization
tests to specify the requesting user in an authorization test. Those
tests failed after the upgrade because the default requesting user in
Impala's AuthorizationTestBase.java happens to be the owner of the
resources involved in our FE authorization tests. After this patch, a
requesting user can be either a non-owner user or an owner user in a
Ranger authorization test and the requesting user would correspond to a
non-owner user if it is not explicitly specified. Note that in a Sentry
authorization test, we do not use the non-owner user as the requesting
user by default as we do in the Ranger authorization tests. Instead, we
set the name of the requesting user to a name that is the same as the
owner user in Ranger authorization tests to avoid the need for providing
a customized group mapping service when instantiating a Sentry
ResourceAuthorizationProvider as we do in AuthorizationTest.java, our
FE tests specifically for testing authorization via Sentry.
On the other hand, to re-enable the tests affected by the second change,
we remove from the Ranger policy for all databases the allowed
condition that grants any user the privilege of creating a database,
which is not by default granted by Sentry. After the removal of the
allowed codition, those tests in AuthorizationStmtTest.java and
RangerAuditLogTest.java affected by the second change now result in the
same authorization errors before the upgrade of Ranger.
Testing:
- Passed AuthorizationStmtTest.java in a local dev environment
- Passed RangerAuditLogTest.java in a local dev environment
Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
M fe/src/test/java/org/apache/impala/common/FrontendFixture.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M testdata/bin/create-load-data.sh
A testdata/cluster/ranger/setup/impala_group_non_owner.json
R testdata/cluster/ranger/setup/impala_group_owner.json.template
A testdata/cluster/ranger/setup/impala_user_non_owner.json.template
R testdata/cluster/ranger/setup/impala_user_owner.json.template
A testdata/cluster/ranger/setup/policy_4_revised.json
12 files changed, 387 insertions(+), 212 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/14798/16
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 16
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
In IMPALA-9047, we disabled some Ranger-related FE and BE tests due to
changes in Ranger's behavior after upgrading Ranger from 1.2 to 2.0.
This patch aims to re-enable those disabled FE tests in
AuthorizationStmtTest.java and RangerAuditLogTest.java to increase
Impala's test coverage of authorization via Ranger.
There are at least two major changes in Ranger's behavior in the newer
versions.
1. The first is that the owner of the requested resource no longer has
to be explicitly granted privileges in order to access the resource.
2. The second is that a user not explicitly granted the privilege of
creating a database is able to do so.
Due to these changes, some of previous Ranger authorization requests
that were expected to be rejected are now granted after the upgrade.
To re-enable the tests affected by the first change described above, we
modify AuthorizationTestBase.java to allow our FE Ranger authorization
tests to specify the requesting user in an authorization test. Those
tests failed after the upgrade because the default requesting user in
Impala's AuthorizationTestBase.java happens to be the owner of the
resources involved in our FE authorization tests. After this patch, a
requesting user can be either a non-owner user or an owner user in a
Ranger authorization test and the requesting user would correspond to a
non-owner user if it is not explicitly specified. Note that in a Sentry
authorization test, we do not use the non-owner user as the requesting
user by default as we do in the Ranger authorization tests. Instead, we
set the name of the requesting user to a name that is the same as the
owner user in Ranger authorization tests to avoid the need for providing
a customized group mapping service when instantiating a Sentry
ResourceAuthorizationProvider as we do in AuthorizationTest.java, our
FE tests specifically for testing authorization via Sentry.
On the other hand, to re-enable the tests affected by the second change,
we remove from the Ranger policy for all databases the allowed codition
that grants any user the privilege of creating a database, which is not
by default granted by Sentry. After the removal of the allowed codition,
those tests in AuthorizationStmtTest.java and RangerAuditLogTest.java
affected by the second change now result in the same authorization
errors before the upgrade of Ranger.
Testing:
- Passed AuthorizationStmtTest.java
- Passed RangerAuditLogTest.java
Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
M fe/src/test/java/org/apache/impala/common/FrontendFixture.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M testdata/bin/create-load-data.sh
A testdata/cluster/ranger/setup/impala_group_non_owner.json
R testdata/cluster/ranger/setup/impala_group_owner.json.template
A testdata/cluster/ranger/setup/impala_user_non_owner.json.template
R testdata/cluster/ranger/setup/impala_user_owner.json.template
A testdata/cluster/ranger/setup/policy_4_revised.json
12 files changed, 395 insertions(+), 212 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/14798/11
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 11
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 16: Verified+1
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 16
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 20 Dec 2019 11:08:22 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 3:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5223/ : 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/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 3
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 06 Dec 2019 23:53:28 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 14:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5315/ : 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/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 14
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 18 Dec 2019 23:35:33 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 1:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14798/1/testdata/bin/create-load-data.sh
File testdata/bin/create-load-data.sh:
http://gerrit.cloudera.org:8080/#/c/14798/1/testdata/bin/create-load-data.sh@335
PS1, Line 335: export GROUP_ID_NON_OWNER=$(wget -qO - --auth-no-challenge --user=admin --password=admin \
line too long (92 > 90)
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 06 Dec 2019 18:29:50 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 16: Code-Review+2
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 16
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 20 Dec 2019 06:34:42 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Kurt Deschler (Code Review)" <ge...@cloudera.org>.
Kurt Deschler has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 5:
(6 comments)
http://gerrit.cloudera.org:8080/#/c/14798/5/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java
File fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java:
http://gerrit.cloudera.org:8080/#/c/14798/5/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java@61
PS5, Line 61: return (ResourceAuthorizationProvider) ConstructorUtils.invokeConstructor(
Remove extra spaces
http://gerrit.cloudera.org:8080/#/c/14798/1/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/14798/1/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@947
PS1, Line 947: test.error(accessError("functional.*.*"));
Can you explicitly test that Rager allows access to functional.* ?
Same question for all similar checks below.
http://gerrit.cloudera.org:8080/#/c/14798/5/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/14798/5/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@946
PS5, Line 946: if (authzProvider_ == AuthorizationProvider.SENTRY) {
Should there be an explicit Ranger testcase in the else clause here and in all of the similar checks below?
http://gerrit.cloudera.org:8080/#/c/14798/5/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@1562
PS5, Line 1562: if (authzProvider_ == AuthorizationProvider.SENTRY) {
Add comment here about what this specific case is and why it is allowed by Ranger
http://gerrit.cloudera.org:8080/#/c/14798/5/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@1596
PS5, Line 1596: // The following do not result in Ranger authorization errors.
A better comment would be something like:
Default Ranger policies allow CREATE DATABASE
http://gerrit.cloudera.org:8080/#/c/14798/5/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@3243
PS5, Line 3243: AS_OWNER = false;
Where was this set? Looks strange to clean it up here.
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 5
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Dec 2019 18:24:54 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 16:
Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/5364/ DRY_RUN=false
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 16
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 20 Dec 2019 06:35:48 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 7:
(5 comments)
Thanks for digging into these, Fang-Yu!
> Hi Kurt and Quanlong, I left some additional thoughts on how we should pass an instance of group mapper to RangerAuthorizationChecker.java. Will try to implement the proposed approach in the next iteration since I think the current approach is too hacky and less flexible. Thanks!
I think we can make the refactor in later patches since it doesn't look like simple. And we have some other on-going works (IMPALA-9222, IMPALA-9231) about the privilege checks. It'd be better to not disable Ranger tests for too long, in case we introduce bugs in those works.
The patch looks good to me overall. Thanks!
http://gerrit.cloudera.org:8080/#/c/14798/7/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/14798/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java@314
PS7, Line 314: if (RuntimeEnv.INSTANCE.isTestEnv()) {
> After some thoughts, I feel like my approach here is a bit too hacky. A bet
I think refactoring RangerAuthorizationChecker may need a refactor on AuthorizationChecker too. It may not be a simple refactor:
* Currently, the Frontend class holds a field for AuthorizationChecker ( https://github.com/apache/impala/blob/313d758/fe/src/main/java/org/apache/impala/service/Frontend.java#L256). It's shared by different requests by different users.
* To bind User/UserGroupInformation with AuthorizationChecker, we may need to create a AuthorizationChecker instance for each requests. It's cheap for Ranger, but expensive for Sentry. The constructor of SentryAuthorizationChecker finally invokes a java refrection which is expensive (https://github.com/apache/impala/blob/313d758/fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java#L61).
http://gerrit.cloudera.org:8080/#/c/14798/7/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
File fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java:
http://gerrit.cloudera.org:8080/#/c/14798/7/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java@87
PS7, Line 87: USER
Rename to user_ since it's now a non-final variable. Same for OWNER_USER, GROUPS, OWNER_GROUPS, AS_OWNER below.
http://gerrit.cloudera.org:8080/#/c/14798/7/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java@266
PS7, Line 266: // Depending on whether or not the principal is the owner of the resource, we return
nit: we usually put the comments above the "@Override"
http://gerrit.cloudera.org:8080/#/c/14798/7/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java@270
PS7, Line 270: AS_OWNER == true
don't need "== true" since it's boolean
http://gerrit.cloudera.org:8080/#/c/14798/7/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java@282
PS7, Line 282: (AS_OWNER ? OWNER_USER.getName() : USER.getName())
use getName() instead
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 7
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 Dec 2019 09:29:38 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 3:
Hi Quanlong and Kurt, please review my revised patch. In this patch, I have figured out how to perform the Ranger tests with a user that does not exist in the host OS by slightly modifying RangerAuthorizationChecker.java and thus we do not have to use an existing user like 'root' as the previous patch.
Please also let me know if you have any other suggestions and comments. Thanks!
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 3
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 06 Dec 2019 23:29:08 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Impala Public Jenkins (Code Review)" <ge...@cloudera.org>.
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 5:
Build Successful
https://jenkins.impala.io/job/gerrit-code-review-checks/5236/ : 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/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 5
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Tue, 10 Dec 2019 05:12:16 +0000
Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has uploaded a new patch set (#14). ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
In IMPALA-9047, we disabled some Ranger-related FE and BE tests due to
changes in Ranger's behavior after upgrading Ranger from 1.2 to 2.0.
This patch aims to re-enable those disabled FE tests in
AuthorizationStmtTest.java and RangerAuditLogTest.java to increase
Impala's test coverage of authorization via Ranger.
There are at least two major changes in Ranger's behavior in the newer
versions.
1. The first is that the owner of the requested resource no longer has
to be explicitly granted privileges in order to access the resource.
2. The second is that a user not explicitly granted the privilege of
creating a database is able to do so.
Due to these changes, some of previous Ranger authorization requests
that were expected to be rejected are now granted after the upgrade.
To re-enable the tests affected by the first change described above, we
modify AuthorizationTestBase.java to allow our FE Ranger authorization
tests to specify the requesting user in an authorization test. Those
tests failed after the upgrade because the default requesting user in
Impala's AuthorizationTestBase.java happens to be the owner of the
resources involved in our FE authorization tests. After this patch, a
requesting user can be either a non-owner user or an owner user in a
Ranger authorization test and the requesting user would correspond to a
non-owner user if it is not explicitly specified. Note that in a Sentry
authorization test, we do not use the non-owner user as the requesting
user by default as we do in the Ranger authorization tests. Instead, we
set the name of the requesting user to a name that is the same as the
owner user in Ranger authorization tests to avoid the need for providing
a customized group mapping service when instantiating a Sentry
ResourceAuthorizationProvider as we do in AuthorizationTest.java, our
FE tests specifically for testing authorization via Sentry.
On the other hand, to re-enable the tests affected by the second change,
we remove from the Ranger policy for all databases the allowed
condition that grants any user the privilege of creating a database,
which is not by default granted by Sentry. After the removal of the
allowed codition, those tests in AuthorizationStmtTest.java and
RangerAuditLogTest.java affected by the second change now result in the
same authorization errors before the upgrade of Ranger.
Testing:
- Passed AuthorizationStmtTest.java in a local dev environment
- Passed RangerAuditLogTest.java in a local dev environment
Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
M fe/src/test/java/org/apache/impala/common/FrontendFixture.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M testdata/bin/create-load-data.sh
A testdata/cluster/ranger/setup/impala_group_non_owner.json
R testdata/cluster/ranger/setup/impala_group_owner.json.template
A testdata/cluster/ranger/setup/impala_user_non_owner.json.template
R testdata/cluster/ranger/setup/impala_user_owner.json.template
A testdata/cluster/ranger/setup/policy_4_revised.json
12 files changed, 392 insertions(+), 212 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/14798/14
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 14
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
In IMPALA-9047, we disabled some Ranger-related FE and BE tests due to
changes in Ranger's behavior after upgrading Ranger from 1.2 to 2.0.
This patch aims to re-enable those disabled FE tests in
AuthorizationStmtTest.java and RangerAuditLogTest.java to increase
Impala's test coverage of authorization via Ranger.
There are at least two major changes in Ranger's behavior in the newer
versions.
1. The first is that the owner of the requested resource no longer have
to be explicitly granted privileges in order to access the resource.
2. The second is that a user not explicitly granted the privilege of
creating a database is able to do so.
Due to these changes, some of previous Ranger authorization requests
that were expected to be rejected are now granted after the upgrade.
To re-enable the tests affected by the first change described above, we
modify AuthorizationTestBase.java to allow our FE Ranger authorization
tests to specify the requesting user in an authorization test. Those
tests failed after the upgrade because the default requesting user in
Impala's AuthorizationTestBase.java happens to be the owner of the
resources involved in our FE authorization tests. After this patch, a
requesting user can be either a non-owner user or an owner user in a
Ranger authorization test and the requesting user would correspond to a
non-owner user if it is not explicitly specified. Note that in a Sentry
authorization test, we do not use the non-owner user as the requesting
user by default as we do in the Ranger authorization tests. Instead, we
set the name of the requesting user to the name of the owner user in
Ranger authorization tests to avoid the complexity of having to provide
a customized group mapping service when instantiating a Sentry
ResourceAuthorizationProvider as we do in AuthorizationTest.java, our FE
tests specifically for testing authorization via Sentry.
On the other hand, for those tests affected by the second change in
AuthorizationStmtTest.java, in this patch we will only run them when the
authorization provider is Sentry. For the affected test in
RangerAuditLogTest.java, we now expect the test query to be successfully
authorized.
Testing:
- Passed AuthorizationStmtTest.java
- Passed RangerAuditLogTest.java
Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
---
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationChecker.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
M fe/src/test/java/org/apache/impala/authorization/ranger/RangerAuditLogTest.java
M fe/src/test/java/org/apache/impala/common/FrontendFixture.java
M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java
M testdata/bin/create-load-data.sh
A testdata/cluster/ranger/setup/impala_group_non_owner.json
R testdata/cluster/ranger/setup/impala_group_owner.json.template
A testdata/cluster/ranger/setup/impala_user_non_owner.json.template
R testdata/cluster/ranger/setup/impala_user_owner.json.template
11 files changed, 434 insertions(+), 246 deletions(-)
git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/98/14798/7
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 7
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 14:
(7 comments)
Just have some nits. Otherwise can give a +2. Thanks for the fixes!
http://gerrit.cloudera.org:8080/#/c/14798/14/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/14798/14/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@2826
PS14, Line 2826: USER
nit: user_
http://gerrit.cloudera.org:8080/#/c/14798/14/fe/src/test/java/org/apache/impala/authorization/AuthorizationStmtTest.java@2831
PS14, Line 2831: AuthorizationTestBase.
nit: I think we don't need the base class name here. We can use user_ directly.
http://gerrit.cloudera.org:8080/#/c/14798/14/testdata/bin/create-load-data.sh
File testdata/bin/create-load-data.sh:
http://gerrit.cloudera.org:8080/#/c/14798/14/testdata/bin/create-load-data.sh@377
PS14, Line 377: ${RANGER_SETUP_DIR}/policy_4_revised.json
It'd be better to double quote the file path in case there're any whitespaces in ${RANGER_SETUP_DIR}
http://gerrit.cloudera.org:8080/#/c/14798/14/testdata/cluster/ranger/setup/policy_4_revised.json
File testdata/cluster/ranger/setup/policy_4_revised.json:
http://gerrit.cloudera.org:8080/#/c/14798/14/testdata/cluster/ranger/setup/policy_4_revised.json@3
PS14, Line 3: "createTime": 1576626853006,
I think we can remove this.
http://gerrit.cloudera.org:8080/#/c/14798/14/testdata/cluster/ranger/setup/policy_4_revised.json@9
PS14, Line 9: "guid": "a2206d12-fd92-437c-903a-c4eb3fb12381",
Also remove this to avoid flaky failures.
http://gerrit.cloudera.org:8080/#/c/14798/14/testdata/cluster/ranger/setup/policy_4_revised.json@103
PS14, Line 103: "resourceSignature": "319fd63cad4bb7c8ed17fda910b636dc2e0f6b0112e28487d9e44e8a5c846314",
Also remove this to avoid flaky failures.
http://gerrit.cloudera.org:8080/#/c/14798/14/testdata/cluster/ranger/setup/policy_4_revised.json@116
PS14, Line 116: "updateTime": 1576626853008,
I think we can remove this too.
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 14
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 20 Dec 2019 02:34:36 +0000
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has posted comments on this change. ( http://gerrit.cloudera.org:8080/14798 )
Change subject: IMPALA-9149: part 1: Re-enabe Ranger-related FE tests
......................................................................
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/14798/7/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
File fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java:
http://gerrit.cloudera.org:8080/#/c/14798/7/fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java@87
PS7, Line 87: USER
> Thanks Quanlong! I have changed 'USER' to 'user_'. But do we have to change
Oh, sorry. So we just need to change USER and AS_OWNER. Could you also move their difinition below to be separated with these final variables?
--
To view, visit http://gerrit.cloudera.org:8080/14798
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I228533aae34b9ac03bdbbcd51a380770ff17c7f2
Gerrit-Change-Number: 14798
Gerrit-PatchSet: 7
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 13 Dec 2019 23:33:30 +0000
Gerrit-HasComments: Yes