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 2020/12/09 01:04:28 UTC

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

Fang-Yu Rao has uploaded this change for review. ( http://gerrit.cloudera.org:8080/16837


Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................

IMPALA-10211 (Part 1): Add support for role-related statements

This patch adds the support for the following role-related statements.
1. CREATE ROLE <role_name>.
2. DROP ROLE <role_name>.
3. GRANT ROLE <role_name> TO GROUP <group_name>.
4. REVOKE ROLE <role_name> FROM GROUP <group_name>.
5. GRANT <privilege> ON <resource> TO ROLE <role_name>.
6. REVOKE <privilege> ON <resource> FROM ROLE <role_name>.
7. SHOW GRANT ROLE <role_name> ON <resource>.
8. SHOW ROLES.
9. SHOW CURRENT ROLES.
10. SHOW ROLE GRANT GROUP <group_name>.

To support the first 4 statements, we implemented the methods of
createRole()/dropRole(), and grantRoleToGroup()/revokeRoleFromGroup()
with their respective API calls provided by Ranger. To support the 5th
and 6th statements, we modified createGrantRevokeRequest() so that the
cases in which the grantee or revokee is a role could be processed. We
slightly extended getPrivileges() so as to include the case when the
principal is a role for the 7th statement. For the last 3 statements, to
make Impala's behavior consistent with that when Sentry was the
authorization provider, we based our implementation on
SentryImpaladAuthorizationManager#getRoles() at
https://gerrit.cloudera.org/c/15833/8/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java,
which was removed in IMPALA-9708 when we dropped the support for Sentry.

To test the implemented functionalities, we based our test cases on
those at
https://gerrit.cloudera.org/c/15833/8/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test.
We note that before our tests could be automatically run in a
Kerberized environment (IMPALA-9360), in order to run the statements of
CREATE/DROP ROLE <role_name>,
GRANT/REVOKE ROLE <role_name> TO/FROM <group_name>, and SHOW ROLES, we
revised security-applicationContext.xml, one of the files needed when
the Ranger server is started, so that the corresponding API calls could
be performed in a non-Kerberized environment.

During the process of adding test cases to grant_revoke.test, we found
the following differences in Impala's behavior between the case when
Ranger is the authorization provider and that when Sentry is the
authorization provider. Specifically, we have the following two major
differences.
1. Before dropping a role in Ranger, we have to remove all the
privileges granted to the role in advance, which is not the case when
Sentry is the authorization provider.
2. The resource has to be specified for the statement of
SHOW GRANT ROLE <role_name> ON <resource>, which is different when
Sentry is the authorization provider. This could be partly due to the
fact that there is no API provided by Ranger that allows Impala to
directly retrieve the list of all privileges granted to a specified
role.
Due to the differences in Impala's behavior described above, we had to
revise the test cases in grant_revoke.test accordingly.

On the other hand, to include as many test cases that were in the
original grant_revoke.test as possible, we had to explicitly add the
test section of 'USER' to specify the connecting user to Impala for some
queries that require the connecting user to be a Ranger administrator,
e.g., CREATE/DROP ROLE <role_name> and
GRANT/REVOKE <role_name> TO/FROM GROUP <group_name>. The user has to be
'admin' in the current grant_revoke.test, whereas it could be the
default user 'getuser()' in the original grant_revoke.test because
previously 'getuser()' was also a Sentry administrator.

Moreover, for some test cases, we had to explicitly alter the owner of a
resource in the original grant_revoke.test when we would like to prevent
the original owner of the resource, e.g., the creator of the resource,
from accessing the resource since the original grant_revoke.test was run
without object ownership being taken into consideration.

Testing:
 - Verified that this patch passes the exhaustive tests in the DEBUG
   build.
 - Briefly verified that the implemented statements work as expected in
   a Kerberized cluster.

Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
---
M bin/create-test-configuration.sh
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerUtil.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
A testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_ranger.py
7 files changed, 1,584 insertions(+), 55 deletions(-)



  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/16837/1
-- 
To view, visit http://gerrit.cloudera.org:8080/16837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 12:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7883/ : 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/16837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 12
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 21 Dec 2020 05:55:29 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 7:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6791/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 7
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 17 Dec 2020 05:17:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 13: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 13
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 21 Dec 2020 14:29:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16837 )

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 10:

According to the console output at https://jenkins.impala.io/job/pre-review-test/811/console, this test passes the exhaustive tests in the DEBUG build.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 10
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 18 Dec 2020 20:18:31 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 4:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7812/ : 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/16837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 4
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 09 Dec 2020 22:27:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 10: Code-Review+1

(5 comments)

LGTM. I'll carry on Csaba's +1 to +2 once the minor comments are resolved. Thanks for your patience!

http://gerrit.cloudera.org:8080/#/c/16837/10/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java:

http://gerrit.cloudera.org:8080/#/c/16837/10/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@154
PS10, Line 154: 
nit: blank line


http://gerrit.cloudera.org:8080/#/c/16837/10/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@173
PS10, Line 173: User
Could you add requestingUser.getShortName() here? We show the username in Sentry errors: https://github.com/apache/impala/blob/3.4.0/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test#L357


http://gerrit.cloudera.org:8080/#/c/16837/10/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@186
PS10, Line 186: 
nit: two blank lines


http://gerrit.cloudera.org:8080/#/c/16837/10/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
File testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test:

http://gerrit.cloudera.org:8080/#/c/16837/10/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test@350
PS10, Line 350: # TODO: Investigate whether the privilege on the provided uri should be verified.
Yeah, this also confuses me. We do verify the URI privilege. I think enabling Ranger audit logs can help us answering such questions, i.e. which policy allows the action.


http://gerrit.cloudera.org:8080/#/c/16837/10/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/16837/10/tests/authorization/test_ranger.py@534
PS10, Line 534:   # TODO(IMPALA-10399): We found that if this test is run after
              :   # test_grant_revoke_with_role() in the exhaustive tests, the test could fail due to an
              :   # empty list returned from the first call to _get_ranger_privileges_db() although a
              :   # list consisting of "lock" and "select" is expected. We suspect there might be
              :   # something wrong with the underlying Ranger API but it requires more thorough
              :   # investigation.
> Thanks Quanlong for the detailed suggestion!
Thanks! Let's debug it in follow-up works. I created a JIRA for enabling ranger audit logs: IMPALA-10401.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 10
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 21 Dec 2020 02:09:46 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 6:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7862/ : 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/16837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 6
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 16 Dec 2020 01:14:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 4:

(12 comments)

Nice work! Just gone through the implementation, haven't looked into the tests in details yet. The implementation makes sense to me.

http://gerrit.cloudera.org:8080/#/c/16837/4//COMMIT_MSG
Commit Message:

http://gerrit.cloudera.org:8080/#/c/16837/4//COMMIT_MSG@12
PS4, Line 12: GROUP <group_name>
Just curious, will we support grant role to user?


http://gerrit.cloudera.org:8080/#/c/16837/4//COMMIT_MSG@56
PS4, Line 56: there is no API provided by Ranger that allows Impala to
            : directly retrieve the list of all privileges granted to a specified
            : role.
Just curious, is this in the roadmap of Ranger, and Is there a Ranger JIRA about this? I think SHOW GRANT ROLE <role_name> without specifying the resource is useful, especially when there are lots of dbs/tables, it's painful to iterate on all possible resources to know what a role can do. As the first item mentioned, to drop a role we have to know all its privileges and drop them first.


http://gerrit.cloudera.org:8080/#/c/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java:

http://gerrit.cloudera.org:8080/#/c/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@85
PS4, Line 85: null, null, null, null, null
nit: These nulls are not good for readability. I think this is better:

    RangerRole role = new RangerRole();
    role.setName(params.getRole_name());


http://gerrit.cloudera.org:8080/#/c/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@89
PS4, Line 89: null
nit: I think our code style is adding a short comment for null parameter values, i.e.

  plugin_.get().createRole(role, /*resultProcessor*/ null);


http://gerrit.cloudera.org:8080/#/c/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@91
PS4, Line 91:       LOG.error("Error creating a role in Ranger. " +
            :           requestingUser.getShortName(), e);
nit: 

      LOG.error("Error creating role {} in Ranger.",
          requestingUser.getShortName(), e);


http://gerrit.cloudera.org:8080/#/c/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@95
PS4, Line 95:     }
Should we call refreshAuthorization(response) at the end?


http://gerrit.cloudera.org:8080/#/c/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@105
PS4, Line 105: row
nit: typo? row -> role?


http://gerrit.cloudera.org:8080/#/c/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@115
PS4, Line 115:     }
Should we call refreshAuthorization(response) at the end here?


http://gerrit.cloudera.org:8080/#/c/16837/4/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/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@22
PS4, Line 22: import com.google.common.base.Preconditions;
nit: keep the import list sorted


http://gerrit.cloudera.org:8080/#/c/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@136
PS4, Line 136:         roleNames = toRoleNames(roles);
nit: we can simplify the toRoleNames method as

  roleNames = roles.stream().map(RangerRole::getName).collect(Collectors.toSet());


http://gerrit.cloudera.org:8080/#/c/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@141
PS4, Line 141:       result.setRole_names(Lists.newArrayListWithExpectedSize(roleNames.size()));
             :       result.getRole_names().addAll(roleNames);
Can we use result.setRole_names(Lists.newArrayList(roleNames)) directly?


http://gerrit.cloudera.org:8080/#/c/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@147
PS4, Line 147: LOG.error("Error executing SHOW CURRENT ROLES.");
nit: It'd be useful to log the exception as well, i.e.

  LOG.error("Error executing SHOW CURRENT ROLES.", e);



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 4
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 11 Dec 2020 08:35:28 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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/16837 )

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................

IMPALA-10211 (Part 1): Add support for role-related statements

This patch adds the support for the following role-related statements.
1. CREATE ROLE <role_name>.
2. DROP ROLE <role_name>.
3. GRANT ROLE <role_name> TO GROUP <group_name>.
4. REVOKE ROLE <role_name> FROM GROUP <group_name>.
5. GRANT <privilege> ON <resource> TO ROLE <role_name>.
6. REVOKE <privilege> ON <resource> FROM ROLE <role_name>.
7. SHOW GRANT ROLE <role_name> ON <resource>.
8. SHOW ROLES.
9. SHOW CURRENT ROLES.
10. SHOW ROLE GRANT GROUP <group_name>.

To support the first 4 statements, we implemented the methods of
createRole()/dropRole(), and grantRoleToGroup()/revokeRoleFromGroup()
with their respective API calls provided by Ranger. To support the 5th
and 6th statements, we modified createGrantRevokeRequest() so that the
cases in which the grantee or revokee is a role could be processed. We
slightly extended getPrivileges() so as to include the case when the
principal is a role for the 7th statement. For the last 3 statements, to
make Impala's behavior consistent with that when Sentry was the
authorization provider, we based our implementation on
SentryImpaladAuthorizationManager#getRoles() at
https://gerrit.cloudera.org/c/15833/8/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java,
which was removed in IMPALA-9708 when we dropped the support for Sentry.

To test the implemented functionalities, we based our test cases on
those at
https://gerrit.cloudera.org/c/15833/8/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test.
We note that before our tests could be automatically run in a
Kerberized environment (IMPALA-9360), in order to run the statements of
CREATE/DROP ROLE <role_name>,
GRANT/REVOKE ROLE <role_name> TO/FROM GROUP <group_name>, and
SHOW ROLES, we revised security-applicationContext.xml, one of the files
needed when the Ranger server is started, so that the corresponding API
calls could be performed in a non-Kerberized environment.

During the process of adding test cases to grant_revoke.test, we found
the following differences in Impala's behavior between the case when
Ranger is the authorization provider and that when Sentry is the
authorization provider. Specifically, we have the following two major
differences.
1. Before dropping a role in Ranger, we have to remove all the
privileges granted to the role in advance, which is not the case when
Sentry is the authorization provider.
2. The resource has to be specified for the statement of
SHOW GRANT ROLE <role_name> ON <resource>, which is different when
Sentry is the authorization provider. This could be partly due to the
fact that there is no API provided by Ranger that allows Impala to
directly retrieve the list of all privileges granted to a specified
role.
Due to the differences in Impala's behavior described above, we had to
revise the test cases in grant_revoke.test accordingly.

On the other hand, to include as many test cases that were in the
original grant_revoke.test as possible, we had to explicitly add the
test section of 'USER' to specify the connecting user to Impala for some
queries that require the connecting user to be a Ranger administrator,
e.g., CREATE/DROP ROLE <role_name> and
GRANT/REVOKE <role_name> TO/FROM GROUP <group_name>. The user has to be
'admin' in the current grant_revoke.test, whereas it could be the
default user 'getuser()' in the original grant_revoke.test because
previously 'getuser()' was also a Sentry administrator.

Moreover, for some test cases, we had to explicitly alter the owner of a
resource in the original grant_revoke.test when we would like to prevent
the original owner of the resource, e.g., the creator of the resource,
from accessing the resource since the original grant_revoke.test was run
without object ownership being taken into consideration.

Testing:
 - Briefly verified that the implemented statements work as expected in
   a Kerberized cluster.
 - Added the decorator of @pytest.mark.execute_serially to each test in
   test_ranger.py since we have observed that in some cases, e.g., if we
   are only running the E2E tests in the Jenkins environment, some tests
   do not seem to be executed sequentially.
 - Verified that test_ranger.py passes in a local development
   environment.

Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
---
M bin/create-test-configuration.sh
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerUtil.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
A testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_ranger.py
7 files changed, 1,703 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/16837/7
-- 
To view, visit http://gerrit.cloudera.org:8080/16837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 7
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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/16837 )

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................

IMPALA-10211 (Part 1): Add support for role-related statements

This patch adds the support for the following role-related statements.
1. CREATE ROLE <role_name>.
2. DROP ROLE <role_name>.
3. GRANT ROLE <role_name> TO GROUP <group_name>.
4. REVOKE ROLE <role_name> FROM GROUP <group_name>.
5. GRANT <privilege> ON <resource> TO ROLE <role_name>.
6. REVOKE <privilege> ON <resource> FROM ROLE <role_name>.
7. SHOW GRANT ROLE <role_name> ON <resource>.
8. SHOW ROLES.
9. SHOW CURRENT ROLES.
10. SHOW ROLE GRANT GROUP <group_name>.

To support the first 4 statements, we implemented the methods of
createRole()/dropRole(), and grantRoleToGroup()/revokeRoleFromGroup()
with their respective API calls provided by Ranger. To support the 5th
and 6th statements, we modified createGrantRevokeRequest() so that the
cases in which the grantee or revokee is a role could be processed. We
slightly extended getPrivileges() so as to include the case when the
principal is a role for the 7th statement. For the last 3 statements, to
make Impala's behavior consistent with that when Sentry was the
authorization provider, we based our implementation on
SentryImpaladAuthorizationManager#getRoles() at
https://gerrit.cloudera.org/c/15833/8/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java,
which was removed in IMPALA-9708 when we dropped the support for Sentry.

To test the implemented functionalities, we based our test cases on
those at
https://gerrit.cloudera.org/c/15833/8/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test.
We note that before our tests could be automatically run in a
Kerberized environment (IMPALA-9360), in order to run the statements of
CREATE/DROP ROLE <role_name>,
GRANT/REVOKE ROLE <role_name> TO/FROM GROUP <group_name>, and
SHOW ROLES, we revised security-applicationContext.xml, one of the files
needed when the Ranger server is started, so that the corresponding API
calls could be performed in a non-Kerberized environment.

During the process of adding test cases to grant_revoke.test, we found
the following differences in Impala's behavior between the case when
Ranger is the authorization provider and that when Sentry is the
authorization provider. Specifically, we have the following two major
differences.
1. Before dropping a role in Ranger, we have to remove all the
privileges granted to the role in advance, which is not the case when
Sentry is the authorization provider.
2. The resource has to be specified for the statement of
SHOW GRANT ROLE <role_name> ON <resource>, which is different when
Sentry is the authorization provider. This could be partly due to the
fact that there is no API provided by Ranger that allows Impala to
directly retrieve the list of all privileges granted to a specified
role.
Due to the differences in Impala's behavior described above, we had to
revise the test cases in grant_revoke.test accordingly.

On the other hand, to include as many test cases that were in the
original grant_revoke.test as possible, we had to explicitly add the
test section of 'USER' to specify the connecting user to Impala for some
queries that require the connecting user to be a Ranger administrator,
e.g., CREATE/DROP ROLE <role_name> and
GRANT/REVOKE <role_name> TO/FROM GROUP <group_name>. The user has to be
'admin' in the current grant_revoke.test, whereas it could be the
default user 'getuser()' in the original grant_revoke.test because
previously 'getuser()' was also a Sentry administrator.

Moreover, for some test cases, we had to explicitly alter the owner of a
resource in the original grant_revoke.test when we would like to prevent
the original owner of the resource, e.g., the creator of the resource,
from accessing the resource since the original grant_revoke.test was run
without object ownership being taken into consideration.

We also note that in this patch we added the decorator of
@pytest.mark.execute_serially to each test in test_ranger.py since we
have observed that in some cases, e.g., if we are only running the E2E
tests in the Jenkins environment, some tests do not seem to be executed
sequentially.

Testing:
 - Briefly verified that the implemented statements work as expected in
   a Kerberized cluster.
 - Verified that test_ranger.py passes in a local development
   environment.
 - Verified that the patch passes the exhaustive tests in the DEBUG
   build.

Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Reviewed-on: http://gerrit.cloudera.org:8080/16837
Reviewed-by: Quanlong Huang <hu...@gmail.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
M bin/create-test-configuration.sh
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerUtil.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
A testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_ranger.py
7 files changed, 1,687 insertions(+), 62 deletions(-)

Approvals:
  Quanlong Huang: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 14
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has uploaded a new patch set (#13). ( http://gerrit.cloudera.org:8080/16837 )

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................

IMPALA-10211 (Part 1): Add support for role-related statements

This patch adds the support for the following role-related statements.
1. CREATE ROLE <role_name>.
2. DROP ROLE <role_name>.
3. GRANT ROLE <role_name> TO GROUP <group_name>.
4. REVOKE ROLE <role_name> FROM GROUP <group_name>.
5. GRANT <privilege> ON <resource> TO ROLE <role_name>.
6. REVOKE <privilege> ON <resource> FROM ROLE <role_name>.
7. SHOW GRANT ROLE <role_name> ON <resource>.
8. SHOW ROLES.
9. SHOW CURRENT ROLES.
10. SHOW ROLE GRANT GROUP <group_name>.

To support the first 4 statements, we implemented the methods of
createRole()/dropRole(), and grantRoleToGroup()/revokeRoleFromGroup()
with their respective API calls provided by Ranger. To support the 5th
and 6th statements, we modified createGrantRevokeRequest() so that the
cases in which the grantee or revokee is a role could be processed. We
slightly extended getPrivileges() so as to include the case when the
principal is a role for the 7th statement. For the last 3 statements, to
make Impala's behavior consistent with that when Sentry was the
authorization provider, we based our implementation on
SentryImpaladAuthorizationManager#getRoles() at
https://gerrit.cloudera.org/c/15833/8/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java,
which was removed in IMPALA-9708 when we dropped the support for Sentry.

To test the implemented functionalities, we based our test cases on
those at
https://gerrit.cloudera.org/c/15833/8/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test.
We note that before our tests could be automatically run in a
Kerberized environment (IMPALA-9360), in order to run the statements of
CREATE/DROP ROLE <role_name>,
GRANT/REVOKE ROLE <role_name> TO/FROM GROUP <group_name>, and
SHOW ROLES, we revised security-applicationContext.xml, one of the files
needed when the Ranger server is started, so that the corresponding API
calls could be performed in a non-Kerberized environment.

During the process of adding test cases to grant_revoke.test, we found
the following differences in Impala's behavior between the case when
Ranger is the authorization provider and that when Sentry is the
authorization provider. Specifically, we have the following two major
differences.
1. Before dropping a role in Ranger, we have to remove all the
privileges granted to the role in advance, which is not the case when
Sentry is the authorization provider.
2. The resource has to be specified for the statement of
SHOW GRANT ROLE <role_name> ON <resource>, which is different when
Sentry is the authorization provider. This could be partly due to the
fact that there is no API provided by Ranger that allows Impala to
directly retrieve the list of all privileges granted to a specified
role.
Due to the differences in Impala's behavior described above, we had to
revise the test cases in grant_revoke.test accordingly.

On the other hand, to include as many test cases that were in the
original grant_revoke.test as possible, we had to explicitly add the
test section of 'USER' to specify the connecting user to Impala for some
queries that require the connecting user to be a Ranger administrator,
e.g., CREATE/DROP ROLE <role_name> and
GRANT/REVOKE <role_name> TO/FROM GROUP <group_name>. The user has to be
'admin' in the current grant_revoke.test, whereas it could be the
default user 'getuser()' in the original grant_revoke.test because
previously 'getuser()' was also a Sentry administrator.

Moreover, for some test cases, we had to explicitly alter the owner of a
resource in the original grant_revoke.test when we would like to prevent
the original owner of the resource, e.g., the creator of the resource,
from accessing the resource since the original grant_revoke.test was run
without object ownership being taken into consideration.

We also note that in this patch we added the decorator of
@pytest.mark.execute_serially to each test in test_ranger.py since we
have observed that in some cases, e.g., if we are only running the E2E
tests in the Jenkins environment, some tests do not seem to be executed
sequentially.

Testing:
 - Briefly verified that the implemented statements work as expected in
   a Kerberized cluster.
 - Verified that test_ranger.py passes in a local development
   environment.
 - Verified that the patch passes the exhaustive tests in the DEBUG
   build.

Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
---
M bin/create-test-configuration.sh
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerUtil.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
A testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_ranger.py
7 files changed, 1,687 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/16837/13
-- 
To view, visit http://gerrit.cloudera.org:8080/16837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 13
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has uploaded a new patch set (#4). ( http://gerrit.cloudera.org:8080/16837 )

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................

IMPALA-10211 (Part 1): Add support for role-related statements

This patch adds the support for the following role-related statements.
1. CREATE ROLE <role_name>.
2. DROP ROLE <role_name>.
3. GRANT ROLE <role_name> TO GROUP <group_name>.
4. REVOKE ROLE <role_name> FROM GROUP <group_name>.
5. GRANT <privilege> ON <resource> TO ROLE <role_name>.
6. REVOKE <privilege> ON <resource> FROM ROLE <role_name>.
7. SHOW GRANT ROLE <role_name> ON <resource>.
8. SHOW ROLES.
9. SHOW CURRENT ROLES.
10. SHOW ROLE GRANT GROUP <group_name>.

To support the first 4 statements, we implemented the methods of
createRole()/dropRole(), and grantRoleToGroup()/revokeRoleFromGroup()
with their respective API calls provided by Ranger. To support the 5th
and 6th statements, we modified createGrantRevokeRequest() so that the
cases in which the grantee or revokee is a role could be processed. We
slightly extended getPrivileges() so as to include the case when the
principal is a role for the 7th statement. For the last 3 statements, to
make Impala's behavior consistent with that when Sentry was the
authorization provider, we based our implementation on
SentryImpaladAuthorizationManager#getRoles() at
https://gerrit.cloudera.org/c/15833/8/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java,
which was removed in IMPALA-9708 when we dropped the support for Sentry.

To test the implemented functionalities, we based our test cases on
those at
https://gerrit.cloudera.org/c/15833/8/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test.
We note that before our tests could be automatically run in a
Kerberized environment (IMPALA-9360), in order to run the statements of
CREATE/DROP ROLE <role_name>,
GRANT/REVOKE ROLE <role_name> TO/FROM GROUP <group_name>, and
SHOW ROLES, we revised security-applicationContext.xml, one of the files
needed when the Ranger server is started, so that the corresponding API
calls could be performed in a non-Kerberized environment.

During the process of adding test cases to grant_revoke.test, we found
the following differences in Impala's behavior between the case when
Ranger is the authorization provider and that when Sentry is the
authorization provider. Specifically, we have the following two major
differences.
1. Before dropping a role in Ranger, we have to remove all the
privileges granted to the role in advance, which is not the case when
Sentry is the authorization provider.
2. The resource has to be specified for the statement of
SHOW GRANT ROLE <role_name> ON <resource>, which is different when
Sentry is the authorization provider. This could be partly due to the
fact that there is no API provided by Ranger that allows Impala to
directly retrieve the list of all privileges granted to a specified
role.
Due to the differences in Impala's behavior described above, we had to
revise the test cases in grant_revoke.test accordingly.

On the other hand, to include as many test cases that were in the
original grant_revoke.test as possible, we had to explicitly add the
test section of 'USER' to specify the connecting user to Impala for some
queries that require the connecting user to be a Ranger administrator,
e.g., CREATE/DROP ROLE <role_name> and
GRANT/REVOKE <role_name> TO/FROM GROUP <group_name>. The user has to be
'admin' in the current grant_revoke.test, whereas it could be the
default user 'getuser()' in the original grant_revoke.test because
previously 'getuser()' was also a Sentry administrator.

Moreover, for some test cases, we had to explicitly alter the owner of a
resource in the original grant_revoke.test when we would like to prevent
the original owner of the resource, e.g., the creator of the resource,
from accessing the resource since the original grant_revoke.test was run
without object ownership being taken into consideration.

Testing:
 - Verified that this patch passes the exhaustive tests in the DEBUG
   build.
 - Briefly verified that the implemented statements work as expected in
   a Kerberized cluster.

Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
---
M bin/create-test-configuration.sh
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerUtil.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
A testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_ranger.py
7 files changed, 1,596 insertions(+), 55 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/16837/4
-- 
To view, visit http://gerrit.cloudera.org:8080/16837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 4
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 7: Code-Review+1

(1 comment)

Thanks for creating the Ranger tickets!

http://gerrit.cloudera.org:8080/#/c/16837/7/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
File testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test:

http://gerrit.cloudera.org:8080/#/c/16837/7/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test@1273
PS7, Line 1273: ====
              : ---- USER
              : admin
              : ---- QUERY
              : # Clean up the granted privileges and test roles.
              : # Note that we need to revoke those previously granted privileges so that each role is
              : # not referenced by any policy before we delete those roles.
              : # Moreover, we need to revoke the privilege on the database 'grant_rev_db' before
              : # dropping 'grant_rev_db'. Otherwise, the revocation would fail due to an
              : # AnalysisException thrown because 'grant_rev_db' does not exist.
              : revoke all on server from grant_revoke_test_ALL_SERVER;
              : revoke all on uri '$FILESYSTEM_PREFIX/test-warehouse/grant_rev_test_tbl2' from grant_revoke_test_ALL_URI;
              : revoke all on uri '$FILESYSTEM_PREFIX/test-warehouse/GRANT_REV_TEST_TBL3' from grant_revoke_test_ALL_URI;
              : revoke all on uri '$FILESYSTEM_PREFIX/test-warehouse/grant_rev_test_prt' from grant_revoke_test_ALL_URI;
              : revoke all on database functional from grant_revoke_test_NON_OWNER;
              : drop role grant_revoke_test_ALL_SERVER;
              : drop role grant_revoke_test_SELECT_INSERT_TEST_TBL;
              : drop role grant_revoke_test_ALL_URI;
              : drop role grant_revoke_test_NON_OWNER;
              : drop role grant_revoke_test_ALL_SERVER1;
              : drop role grant_revoke_test_COLUMN_PRIV;
              : ---- RESULTS
              : 'Role has been dropped.'
              : ====
These are no longer needed, right? Or for clarity you want to clean these up both here and in the .py file?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 7
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 17 Dec 2020 15:51:01 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 6: Code-Review+1

(1 comment)

The change looks good to me in general.
About the RANGER Jiras: I would prefer to create them before merging this and mention them in comments or the commit message.

http://gerrit.cloudera.org:8080/#/c/16837/4/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
File testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test:

http://gerrit.cloudera.org:8080/#/c/16837/4/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test@1277
PS4, Line 1277: # Clean up the granted privileges and test roles.
> It would be great if we had some statement like "REVOKE <privilege> ON <resource> FROM ROLE <role_name> IF EXISTS" in Impala.

This could be done by calling these cleanup statements in the .py file and swallow the exception if there is an error. Calling revoke on a non-existing privilege doesn't have any side effect AFAIK.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 6
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 16 Dec 2020 16:37:15 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/16837 )

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................

IMPALA-10211 (Part 1): Add support for role-related statements

This patch adds the support for the following role-related statements.
1. CREATE ROLE <role_name>.
2. DROP ROLE <role_name>.
3. GRANT ROLE <role_name> TO GROUP <group_name>.
4. REVOKE ROLE <role_name> FROM GROUP <group_name>.
5. GRANT <privilege> ON <resource> TO ROLE <role_name>.
6. REVOKE <privilege> ON <resource> FROM ROLE <role_name>.
7. SHOW GRANT ROLE <role_name> ON <resource>.
8. SHOW ROLES.
9. SHOW CURRENT ROLES.
10. SHOW ROLE GRANT GROUP <group_name>.

To support the first 4 statements, we implemented the methods of
createRole()/dropRole(), and grantRoleToGroup()/revokeRoleFromGroup()
with their respective API calls provided by Ranger. To support the 5th
and 6th statements, we modified createGrantRevokeRequest() so that the
cases in which the grantee or revokee is a role could be processed. We
slightly extended getPrivileges() so as to include the case when the
principal is a role for the 7th statement. For the last 3 statements, to
make Impala's behavior consistent with that when Sentry was the
authorization provider, we based our implementation on
SentryImpaladAuthorizationManager#getRoles() at
https://gerrit.cloudera.org/c/15833/8/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java,
which was removed in IMPALA-9708 when we dropped the support for Sentry.

To test the implemented functionalities, we based our test cases on
those at
https://gerrit.cloudera.org/c/15833/8/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test.
We note that before our tests could be automatically run in a
Kerberized environment (IMPALA-9360), in order to run the statements of
CREATE/DROP ROLE <role_name>,
GRANT/REVOKE ROLE <role_name> TO/FROM GROUP <group_name>, and
SHOW ROLES, we revised security-applicationContext.xml, one of the files
needed when the Ranger server is started, so that the corresponding API
calls could be performed in a non-Kerberized environment.

During the process of adding test cases to grant_revoke.test, we found
the following differences in Impala's behavior between the case when
Ranger is the authorization provider and that when Sentry is the
authorization provider. Specifically, we have the following two major
differences.
1. Before dropping a role in Ranger, we have to remove all the
privileges granted to the role in advance, which is not the case when
Sentry is the authorization provider.
2. The resource has to be specified for the statement of
SHOW GRANT ROLE <role_name> ON <resource>, which is different when
Sentry is the authorization provider. This could be partly due to the
fact that there is no API provided by Ranger that allows Impala to
directly retrieve the list of all privileges granted to a specified
role.
Due to the differences in Impala's behavior described above, we had to
revise the test cases in grant_revoke.test accordingly.

On the other hand, to include as many test cases that were in the
original grant_revoke.test as possible, we had to explicitly add the
test section of 'USER' to specify the connecting user to Impala for some
queries that require the connecting user to be a Ranger administrator,
e.g., CREATE/DROP ROLE <role_name> and
GRANT/REVOKE <role_name> TO/FROM GROUP <group_name>. The user has to be
'admin' in the current grant_revoke.test, whereas it could be the
default user 'getuser()' in the original grant_revoke.test because
previously 'getuser()' was also a Sentry administrator.

Moreover, for some test cases, we had to explicitly alter the owner of a
resource in the original grant_revoke.test when we would like to prevent
the original owner of the resource, e.g., the creator of the resource,
from accessing the resource since the original grant_revoke.test was run
without object ownership being taken into consideration.

We also note that in this patch we added the decorator of
@pytest.mark.execute_serially to each test in test_ranger.py since we
have observed that in some cases, e.g., if we are only running the E2E
tests in the Jenkins environment, some tests do not seem to be executed
sequentially.

Testing:
 - Briefly verified that the implemented statements work as expected in
   a Kerberized cluster.
 - Verified that test_ranger.py passes in a local development
   environment.
 - Verified that the patch passes the exhaustive tests in the DEBUG
   build.

Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
---
M bin/create-test-configuration.sh
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerUtil.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
A testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_ranger.py
7 files changed, 1,680 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/16837/10
-- 
To view, visit http://gerrit.cloudera.org:8080/16837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 10
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 4: Verified-1

Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/6761/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 4
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 11 Dec 2020 01:58:54 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has uploaded a new patch set (#6). ( http://gerrit.cloudera.org:8080/16837 )

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................

IMPALA-10211 (Part 1): Add support for role-related statements

This patch adds the support for the following role-related statements.
1. CREATE ROLE <role_name>.
2. DROP ROLE <role_name>.
3. GRANT ROLE <role_name> TO GROUP <group_name>.
4. REVOKE ROLE <role_name> FROM GROUP <group_name>.
5. GRANT <privilege> ON <resource> TO ROLE <role_name>.
6. REVOKE <privilege> ON <resource> FROM ROLE <role_name>.
7. SHOW GRANT ROLE <role_name> ON <resource>.
8. SHOW ROLES.
9. SHOW CURRENT ROLES.
10. SHOW ROLE GRANT GROUP <group_name>.

To support the first 4 statements, we implemented the methods of
createRole()/dropRole(), and grantRoleToGroup()/revokeRoleFromGroup()
with their respective API calls provided by Ranger. To support the 5th
and 6th statements, we modified createGrantRevokeRequest() so that the
cases in which the grantee or revokee is a role could be processed. We
slightly extended getPrivileges() so as to include the case when the
principal is a role for the 7th statement. For the last 3 statements, to
make Impala's behavior consistent with that when Sentry was the
authorization provider, we based our implementation on
SentryImpaladAuthorizationManager#getRoles() at
https://gerrit.cloudera.org/c/15833/8/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java,
which was removed in IMPALA-9708 when we dropped the support for Sentry.

To test the implemented functionalities, we based our test cases on
those at
https://gerrit.cloudera.org/c/15833/8/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test.
We note that before our tests could be automatically run in a
Kerberized environment (IMPALA-9360), in order to run the statements of
CREATE/DROP ROLE <role_name>,
GRANT/REVOKE ROLE <role_name> TO/FROM GROUP <group_name>, and
SHOW ROLES, we revised security-applicationContext.xml, one of the files
needed when the Ranger server is started, so that the corresponding API
calls could be performed in a non-Kerberized environment.

During the process of adding test cases to grant_revoke.test, we found
the following differences in Impala's behavior between the case when
Ranger is the authorization provider and that when Sentry is the
authorization provider. Specifically, we have the following two major
differences.
1. Before dropping a role in Ranger, we have to remove all the
privileges granted to the role in advance, which is not the case when
Sentry is the authorization provider.
2. The resource has to be specified for the statement of
SHOW GRANT ROLE <role_name> ON <resource>, which is different when
Sentry is the authorization provider. This could be partly due to the
fact that there is no API provided by Ranger that allows Impala to
directly retrieve the list of all privileges granted to a specified
role.
Due to the differences in Impala's behavior described above, we had to
revise the test cases in grant_revoke.test accordingly.

On the other hand, to include as many test cases that were in the
original grant_revoke.test as possible, we had to explicitly add the
test section of 'USER' to specify the connecting user to Impala for some
queries that require the connecting user to be a Ranger administrator,
e.g., CREATE/DROP ROLE <role_name> and
GRANT/REVOKE <role_name> TO/FROM GROUP <group_name>. The user has to be
'admin' in the current grant_revoke.test, whereas it could be the
default user 'getuser()' in the original grant_revoke.test because
previously 'getuser()' was also a Sentry administrator.

Moreover, for some test cases, we had to explicitly alter the owner of a
resource in the original grant_revoke.test when we would like to prevent
the original owner of the resource, e.g., the creator of the resource,
from accessing the resource since the original grant_revoke.test was run
without object ownership being taken into consideration.

Testing:
 - Briefly verified that the implemented statements work as expected in
   a Kerberized cluster.
 - Added the decorator of @pytest.mark.execute_serially to each test in
   test_ranger.py since we have observed that in some cases, e.g., if we
   are only running the E2E tests in the Jenkins environment, some tests
   do not seem to be executed sequentially.
 - Verified that test_ranger.py passes in a local development
   environment.

Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
---
M bin/create-test-configuration.sh
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerUtil.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
A testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_ranger.py
7 files changed, 1,637 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/16837/6
-- 
To view, visit http://gerrit.cloudera.org:8080/16837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 6
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 13: Code-Review+2

Nice work!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 13
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 21 Dec 2020 06:21:13 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16837 )

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 2:

(2 comments)

Left some comments and will address them in the next patch.

http://gerrit.cloudera.org:8080/#/c/16837/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java:

http://gerrit.cloudera.org:8080/#/c/16837/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@144
PS2, Line 144:       // To avoid confusion, we do not use the error message from Ranger directly since
When a Ranger administrator is revoke an existing role from a non-existing group, Ranger will return an error message indicating the group does not exist in Ranger. Thus, by handling the exception this way, we mask the error message due to other types of error. I will push a revised patch soon.


http://gerrit.cloudera.org:8080/#/c/16837/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/16837/2/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@163
PS2, Line 163:   /**
             :    * For now there is no dedicated REST API that allows Impala to determine whether
             :    * 'user' is a Ranger administrator. In this regard, we call
             :    * RangerBasePlugin#getAllRoles(), whose server side method,
             :    * RoleREST#getAllRoleNames(), will call RoleREST#ensureAdminAccess() on 'user' to make
             :    * sure 'user' is a Ranger administrator. An Exception will be thrown if it is not the
             :    * case.
             :    */
             :   private void validateRangerAdmin(String user) throws Exception {
             :     plugin_.get().getAllRoles(user, null);
             :   }
This method is not used. So I will remove it in the next patch.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 09 Dec 2020 22:06:00 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has uploaded a new patch set (#2). ( http://gerrit.cloudera.org:8080/16837 )

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................

IMPALA-10211 (Part 1): Add support for role-related statements

This patch adds the support for the following role-related statements.
1. CREATE ROLE <role_name>.
2. DROP ROLE <role_name>.
3. GRANT ROLE <role_name> TO GROUP <group_name>.
4. REVOKE ROLE <role_name> FROM GROUP <group_name>.
5. GRANT <privilege> ON <resource> TO ROLE <role_name>.
6. REVOKE <privilege> ON <resource> FROM ROLE <role_name>.
7. SHOW GRANT ROLE <role_name> ON <resource>.
8. SHOW ROLES.
9. SHOW CURRENT ROLES.
10. SHOW ROLE GRANT GROUP <group_name>.

To support the first 4 statements, we implemented the methods of
createRole()/dropRole(), and grantRoleToGroup()/revokeRoleFromGroup()
with their respective API calls provided by Ranger. To support the 5th
and 6th statements, we modified createGrantRevokeRequest() so that the
cases in which the grantee or revokee is a role could be processed. We
slightly extended getPrivileges() so as to include the case when the
principal is a role for the 7th statement. For the last 3 statements, to
make Impala's behavior consistent with that when Sentry was the
authorization provider, we based our implementation on
SentryImpaladAuthorizationManager#getRoles() at
https://gerrit.cloudera.org/c/15833/8/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java,
which was removed in IMPALA-9708 when we dropped the support for Sentry.

To test the implemented functionalities, we based our test cases on
those at
https://gerrit.cloudera.org/c/15833/8/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test.
We note that before our tests could be automatically run in a
Kerberized environment (IMPALA-9360), in order to run the statements of
CREATE/DROP ROLE <role_name>,
GRANT/REVOKE ROLE <role_name> TO/FROM <group_name>, and SHOW ROLES, we
revised security-applicationContext.xml, one of the files needed when
the Ranger server is started, so that the corresponding API calls could
be performed in a non-Kerberized environment.

During the process of adding test cases to grant_revoke.test, we found
the following differences in Impala's behavior between the case when
Ranger is the authorization provider and that when Sentry is the
authorization provider. Specifically, we have the following two major
differences.
1. Before dropping a role in Ranger, we have to remove all the
privileges granted to the role in advance, which is not the case when
Sentry is the authorization provider.
2. The resource has to be specified for the statement of
SHOW GRANT ROLE <role_name> ON <resource>, which is different when
Sentry is the authorization provider. This could be partly due to the
fact that there is no API provided by Ranger that allows Impala to
directly retrieve the list of all privileges granted to a specified
role.
Due to the differences in Impala's behavior described above, we had to
revise the test cases in grant_revoke.test accordingly.

On the other hand, to include as many test cases that were in the
original grant_revoke.test as possible, we had to explicitly add the
test section of 'USER' to specify the connecting user to Impala for some
queries that require the connecting user to be a Ranger administrator,
e.g., CREATE/DROP ROLE <role_name> and
GRANT/REVOKE <role_name> TO/FROM GROUP <group_name>. The user has to be
'admin' in the current grant_revoke.test, whereas it could be the
default user 'getuser()' in the original grant_revoke.test because
previously 'getuser()' was also a Sentry administrator.

Moreover, for some test cases, we had to explicitly alter the owner of a
resource in the original grant_revoke.test when we would like to prevent
the original owner of the resource, e.g., the creator of the resource,
from accessing the resource since the original grant_revoke.test was run
without object ownership being taken into consideration.

Testing:
 - Verified that this patch passes the exhaustive tests in the DEBUG
   build.
 - Briefly verified that the implemented statements work as expected in
   a Kerberized cluster.

Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
---
M bin/create-test-configuration.sh
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerUtil.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
A testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_ranger.py
7 files changed, 1,585 insertions(+), 55 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/16837/2
-- 
To view, visit http://gerrit.cloudera.org:8080/16837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16837 )

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 6:

(1 comment)

> Patch Set 6: Code-Review+1
> 
> (1 comment)
> 
> The change looks good to me in general.
> About the RANGER Jiras: I would prefer to create them before merging this and mention them in comments or the commit message.

Thanks Csaba! With regard to this, I have created RANGER-3125, RANGER-3126, and RANGER-3127 to keep track of the issues we have found. In addition, I also created IMPALA-10399 regarding the issue of the test test_show_grant_hive_privilege() we encountered.

http://gerrit.cloudera.org:8080/#/c/16837/4/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
File testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test:

http://gerrit.cloudera.org:8080/#/c/16837/4/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test@1277
PS4, Line 1277: # Clean up the granted privileges and test roles.
> > It would be great if we had some statement like "REVOKE <privilege> ON <r
Thanks Csaba! In this regard, I will add to test_grant_revoke_with_role() a section that would be executed to clean up the granted privileges as well as the test roles in case some error occurs in the execution of the test cases in this file.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 6
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 17 Dec 2020 00:37:50 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 13:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7884/ : 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/16837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 13
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 21 Dec 2020 06:19:27 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16837/10/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/16837/10/tests/authorization/test_ranger.py@534
PS10, Line 534:   # TODO(IMPALA-10399): We found that if this test is run after
              :   # test_grant_revoke_with_role() in the exhaustive tests, the test could fail due to an
              :   # empty list returned from the first call to _get_ranger_privileges_db() although a
              :   # list consisting of "lock" and "select" is expected. We suspect there might be
              :   # something wrong with the underlying Ranger API but it requires more thorough
              :   # investigation.
Does this mean after this patch, the exhaustive builds will always fail? I try running these two tests but can't reproduce the failure locally. Maybe there are some other reasons. It'd be good if we can keep exhaustive builds green.

For how to trigger these two tests in a run, we need to modify the last line of bin/impala-py.test as

 -exec "$PY_ENV_DIR/bin/py.test" "$@"
 +exec "$PY_ENV_DIR/bin/py.test" $@

i.e. removing the double quotes of $@. Then trigger the two tests in a one-line command:

 bin/impala-py.test tests/authorization/test_ranger.py::TestRanger::test_grant_revoke_with_role tests/authorization/test_ranger.py::TestRanger::test_show_grant_hive_privilege

They both passed in my local env in the order you mentioned:

 platform linux2 -- Python 2.7.16, pytest-2.9.2, py-1.4.32, pluggy-0.3.1 -- /home/quanlong/workspace/Impala/infra/python/env-gcc7.5.0/bin/python
 cachedir: tests/.cache
 rootdir: /home/quanlong/workspace/Impala/tests, inifile: pytest.ini
 plugins: xdist-1.17.1, timeout-1.2.1, random-0.2, forked-0.2
 timeout: 7200s method: signal
 collected 30 items 

 tests/authorization/test_ranger.py::TestRanger::test_grant_revoke_with_role[protocol: beeswax | exec_option: {'batch_size': 0, 'num_nodes': 0, 'disable_codegen_rows_threshold': 0, 'disable_codegen': False, 'abort_on_error': 1, 'exec_single_node_rows_threshold': 0} | table_format: text/none] PASSED
 tests/authorization/test_ranger.py::TestRanger::test_show_grant_hive_privilege PASSED



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 10
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sun, 20 Dec 2020 10:17:49 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 4:

(5 comments)

Great work, especially the workarounds for the Ranger bugs!
I ran through most of the code, I plan to do another pass soon.

http://gerrit.cloudera.org:8080/#/c/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java:

http://gerrit.cloudera.org:8080/#/c/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@105
PS4, Line 105: issue
Is there a Ranger ticket for this?


http://gerrit.cloudera.org:8080/#/c/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@133
PS4, Line 133:       // actually revoke the role from the group. This should be considered a bug of
Is there a Ranger ticket for this?


http://gerrit.cloudera.org:8080/#/c/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@143
PS4, Line 143:       plugin_.get().revokeRole(request, null);
             :       plugin_.get().grantRole(request, null);
Is it possible to run this with more than one threads at the same time? Two parallel grants to the same group could run like this:
revoke role // revoking role if the group already had it
revoke role // no effect
grant role // granting role
grant role // revoking role

Even if this is possible, I don't think that it is a very serious issue, but it would be good to know whether we have to think about parallelism.


http://gerrit.cloudera.org:8080/#/c/16837/4/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@167
PS4, Line 167: dropping
Are we logging "dropping" intentionally?


http://gerrit.cloudera.org:8080/#/c/16837/4/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
File testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test:

http://gerrit.cloudera.org:8080/#/c/16837/4/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test@1277
PS4, Line 1277: # Clean up the granted privileges and test roles.
Will it cause problems if we fail do drop these? My understanding is that executing test files stop at the first failed test.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 4
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 10 Dec 2020 18:43:20 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 1:

(7 comments)

http://gerrit.cloudera.org:8080/#/c/16837/1/bin/create-test-configuration.sh
File bin/create-test-configuration.sh:

http://gerrit.cloudera.org:8080/#/c/16837/1/bin/create-test-configuration.sh@248
PS1, Line 248:                 <intercept-url pattern="/service/public/v2/api/roles/*" access="permitAll"/> \
line too long (94 > 90)


http://gerrit.cloudera.org:8080/#/c/16837/1/bin/create-test-configuration.sh@249
PS1, Line 249:                 <intercept-url pattern="/service/public/v2/api/roles/name/*" access="permitAll"/> \
line too long (99 > 90)


http://gerrit.cloudera.org:8080/#/c/16837/1/bin/create-test-configuration.sh@250
PS1, Line 250:                 <intercept-url pattern="/service/public/v2/api/roles/grant/*" access="permitAll"/> \
line too long (100 > 90)


http://gerrit.cloudera.org:8080/#/c/16837/1/bin/create-test-configuration.sh@251
PS1, Line 251:                 <intercept-url pattern="/service/public/v2/api/roles/revoke/*" access="permitAll"/> \
line too long (101 > 90)


http://gerrit.cloudera.org:8080/#/c/16837/1/bin/create-test-configuration.sh@252
PS1, Line 252:                 <intercept-url pattern="/service/public/v2/api/roles/names/*" access="permitAll"/>' \
line too long (101 > 90)


http://gerrit.cloudera.org:8080/#/c/16837/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
File fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java:

http://gerrit.cloudera.org:8080/#/c/16837/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java@149
PS1, Line 149:           "Ranger error message: HTTP 400 Error: User doesn't have permissions to grant " +
line too long (91 > 90)


http://gerrit.cloudera.org:8080/#/c/16837/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/16837/1/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java@151
PS1, Line 151:         LOG.error("Error executing SHOW ROLE GRANT GROUP " + params.getGrant_group() + ".");
line too long (92 > 90)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 09 Dec 2020 01:05:13 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 13:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6795/ DRY_RUN=false


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 13
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Mon, 21 Dec 2020 08:54:15 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 4:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/6761/ DRY_RUN=true


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 4
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 10 Dec 2020 20:27:26 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 2:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7797/ : 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/16837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 09 Dec 2020 01:55:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 1:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7796/ : 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/16837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Wed, 09 Dec 2020 01:26:14 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/16837 )

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................

IMPALA-10211 (Part 1): Add support for role-related statements

This patch adds the support for the following role-related statements.
1. CREATE ROLE <role_name>.
2. DROP ROLE <role_name>.
3. GRANT ROLE <role_name> TO GROUP <group_name>.
4. REVOKE ROLE <role_name> FROM GROUP <group_name>.
5. GRANT <privilege> ON <resource> TO ROLE <role_name>.
6. REVOKE <privilege> ON <resource> FROM ROLE <role_name>.
7. SHOW GRANT ROLE <role_name> ON <resource>.
8. SHOW ROLES.
9. SHOW CURRENT ROLES.
10. SHOW ROLE GRANT GROUP <group_name>.

To support the first 4 statements, we implemented the methods of
createRole()/dropRole(), and grantRoleToGroup()/revokeRoleFromGroup()
with their respective API calls provided by Ranger. To support the 5th
and 6th statements, we modified createGrantRevokeRequest() so that the
cases in which the grantee or revokee is a role could be processed. We
slightly extended getPrivileges() so as to include the case when the
principal is a role for the 7th statement. For the last 3 statements, to
make Impala's behavior consistent with that when Sentry was the
authorization provider, we based our implementation on
SentryImpaladAuthorizationManager#getRoles() at
https://gerrit.cloudera.org/c/15833/8/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java,
which was removed in IMPALA-9708 when we dropped the support for Sentry.

To test the implemented functionalities, we based our test cases on
those at
https://gerrit.cloudera.org/c/15833/8/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test.
We note that before our tests could be automatically run in a
Kerberized environment (IMPALA-9360), in order to run the statements of
CREATE/DROP ROLE <role_name>,
GRANT/REVOKE ROLE <role_name> TO/FROM GROUP <group_name>, and
SHOW ROLES, we revised security-applicationContext.xml, one of the files
needed when the Ranger server is started, so that the corresponding API
calls could be performed in a non-Kerberized environment.

During the process of adding test cases to grant_revoke.test, we found
the following differences in Impala's behavior between the case when
Ranger is the authorization provider and that when Sentry is the
authorization provider. Specifically, we have the following two major
differences.
1. Before dropping a role in Ranger, we have to remove all the
privileges granted to the role in advance, which is not the case when
Sentry is the authorization provider.
2. The resource has to be specified for the statement of
SHOW GRANT ROLE <role_name> ON <resource>, which is different when
Sentry is the authorization provider. This could be partly due to the
fact that there is no API provided by Ranger that allows Impala to
directly retrieve the list of all privileges granted to a specified
role.
Due to the differences in Impala's behavior described above, we had to
revise the test cases in grant_revoke.test accordingly.

On the other hand, to include as many test cases that were in the
original grant_revoke.test as possible, we had to explicitly add the
test section of 'USER' to specify the connecting user to Impala for some
queries that require the connecting user to be a Ranger administrator,
e.g., CREATE/DROP ROLE <role_name> and
GRANT/REVOKE <role_name> TO/FROM GROUP <group_name>. The user has to be
'admin' in the current grant_revoke.test, whereas it could be the
default user 'getuser()' in the original grant_revoke.test because
previously 'getuser()' was also a Sentry administrator.

Moreover, for some test cases, we had to explicitly alter the owner of a
resource in the original grant_revoke.test when we would like to prevent
the original owner of the resource, e.g., the creator of the resource,
from accessing the resource since the original grant_revoke.test was run
without object ownership being taken into consideration.

We also note that in this patch we added the decorator of
@pytest.mark.execute_serially to each test in test_ranger.py since we
have observed that in some cases, e.g., if we are only running the E2E
tests in the Jenkins environment, some tests do not seem to be executed
sequentially.

Testing:
 - Briefly verified that the implemented statements work as expected in
   a Kerberized cluster.
 - Verified that test_ranger.py passes in a local development
   environment.
 - Verified that the patch passes the exhaustive tests in the DEBUG
   build.

Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
---
M bin/create-test-configuration.sh
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerUtil.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
A testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_ranger.py
7 files changed, 1,679 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/16837/12
-- 
To view, visit http://gerrit.cloudera.org:8080/16837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 12
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 7:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7874/ : 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/16837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 7
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 17 Dec 2020 01:08:58 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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/16837 )

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................

IMPALA-10211 (Part 1): Add support for role-related statements

This patch adds the support for the following role-related statements.
1. CREATE ROLE <role_name>.
2. DROP ROLE <role_name>.
3. GRANT ROLE <role_name> TO GROUP <group_name>.
4. REVOKE ROLE <role_name> FROM GROUP <group_name>.
5. GRANT <privilege> ON <resource> TO ROLE <role_name>.
6. REVOKE <privilege> ON <resource> FROM ROLE <role_name>.
7. SHOW GRANT ROLE <role_name> ON <resource>.
8. SHOW ROLES.
9. SHOW CURRENT ROLES.
10. SHOW ROLE GRANT GROUP <group_name>.

To support the first 4 statements, we implemented the methods of
createRole()/dropRole(), and grantRoleToGroup()/revokeRoleFromGroup()
with their respective API calls provided by Ranger. To support the 5th
and 6th statements, we modified createGrantRevokeRequest() so that the
cases in which the grantee or revokee is a role could be processed. We
slightly extended getPrivileges() so as to include the case when the
principal is a role for the 7th statement. For the last 3 statements, to
make Impala's behavior consistent with that when Sentry was the
authorization provider, we based our implementation on
SentryImpaladAuthorizationManager#getRoles() at
https://gerrit.cloudera.org/c/15833/8/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java,
which was removed in IMPALA-9708 when we dropped the support for Sentry.

To test the implemented functionalities, we based our test cases on
those at
https://gerrit.cloudera.org/c/15833/8/testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test.
We note that before our tests could be automatically run in a
Kerberized environment (IMPALA-9360), in order to run the statements of
CREATE/DROP ROLE <role_name>,
GRANT/REVOKE ROLE <role_name> TO/FROM GROUP <group_name>, and
SHOW ROLES, we revised security-applicationContext.xml, one of the files
needed when the Ranger server is started, so that the corresponding API
calls could be performed in a non-Kerberized environment.

During the process of adding test cases to grant_revoke.test, we found
the following differences in Impala's behavior between the case when
Ranger is the authorization provider and that when Sentry is the
authorization provider. Specifically, we have the following two major
differences.
1. Before dropping a role in Ranger, we have to remove all the
privileges granted to the role in advance, which is not the case when
Sentry is the authorization provider.
2. The resource has to be specified for the statement of
SHOW GRANT ROLE <role_name> ON <resource>, which is different when
Sentry is the authorization provider. This could be partly due to the
fact that there is no API provided by Ranger that allows Impala to
directly retrieve the list of all privileges granted to a specified
role.
Due to the differences in Impala's behavior described above, we had to
revise the test cases in grant_revoke.test accordingly.

On the other hand, to include as many test cases that were in the
original grant_revoke.test as possible, we had to explicitly add the
test section of 'USER' to specify the connecting user to Impala for some
queries that require the connecting user to be a Ranger administrator,
e.g., CREATE/DROP ROLE <role_name> and
GRANT/REVOKE <role_name> TO/FROM GROUP <group_name>. The user has to be
'admin' in the current grant_revoke.test, whereas it could be the
default user 'getuser()' in the original grant_revoke.test because
previously 'getuser()' was also a Sentry administrator.

Moreover, for some test cases, we had to explicitly alter the owner of a
resource in the original grant_revoke.test when we would like to prevent
the original owner of the resource, e.g., the creator of the resource,
from accessing the resource since the original grant_revoke.test was run
without object ownership being taken into consideration.

We also note that in this patch we added the decorator of
@pytest.mark.execute_serially to each test in test_ranger.py since we
have observed that in some cases, e.g., if we are only running the E2E
tests in the Jenkins environment, some tests do not seem to be executed
sequentially.

Testing:
 - Briefly verified that the implemented statements work as expected in
   a Kerberized cluster.
 - Verified that test_ranger.py passes in a local development
   environment.
 - Verified that the patch passes the exhaustive tests in the DEBUG
   build.

Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
---
M bin/create-test-configuration.sh
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
M fe/src/main/java/org/apache/impala/authorization/ranger/RangerUtil.java
M fe/src/test/java/org/apache/impala/authorization/AuthorizationTestBase.java
A testdata/workloads/functional-query/queries/QueryTest/grant_revoke.test
M tests/authorization/test_ranger.py
7 files changed, 1,685 insertions(+), 62 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/37/16837/9
-- 
To view, visit http://gerrit.cloudera.org:8080/16837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 9
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 10:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7878/ : 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/16837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 10
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 17 Dec 2020 18:50:21 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 7: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 7
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 17 Dec 2020 10:54:11 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16837 )

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 10:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/16837/10/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/16837/10/tests/authorization/test_ranger.py@534
PS10, Line 534:   # TODO(IMPALA-10399): We found that if this test is run after
              :   # test_grant_revoke_with_role() in the exhaustive tests, the test could fail due to an
              :   # empty list returned from the first call to _get_ranger_privileges_db() although a
              :   # list consisting of "lock" and "select" is expected. We suspect there might be
              :   # something wrong with the underlying Ranger API but it requires more thorough
              :   # investigation.
> Does this mean after this patch, the exhaustive builds will always fail? I 
Thanks Quanlong for the detailed suggestion!

I think the exhaustive tests will still be green after merging this patch. According to the console output at https://jenkins.impala.io/job/pre-review-test/811/console, this patch passes the exhaustive tests. The reason I think is that in this patch I moved the test of test_grant_revoke_with_role() to the bottom of the class TestRanger so that test_grant_revoke_with_role() will be executed after test_show_grant_hive_privilege(). Before changing the relative order between these 2 tests, I was able to reproduce the issue in the exhaustive tests in a private Jenkins environment but not the public pre-review tests at https://jenkins.impala.io/job/pre-review-test/.

Specifically, I was only able to reproduce the issue by moving test_grant_revoke_with_role() to a place such that test_grant_revoke_with_role() will be executed before test_show_grant_hive_privilege() in the exhaustive tests in the private Jenkins environment mentioned above. I was not even able to reproduce this issue by only running these 2 tests in that private Jenkins environment.

According to our discussion offline, I will try to do the following as you suggested to investigate the issue.
1. Enable ranger audit logs. Ranger can log audits into a HDFS location. We can enable it in our minicluster.
2. Enable debug level ranger logs. DEBUG level logs will contain the request details.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 10
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Sun, 20 Dec 2020 19:23:03 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

Posted by "Fang-Yu Rao (Code Review)" <ge...@cloudera.org>.
Fang-Yu Rao has posted comments on this change. ( http://gerrit.cloudera.org:8080/16837 )

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 10:

> Patch Set 10:
> 
> According to the console output at https://jenkins.impala.io/job/pre-review-test/811/console, this test passes the exhaustive tests in the DEBUG build.

Should be this "patch" instead of this "test".


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 10
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Fri, 18 Dec 2020 20:19:04 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10211 (Part 1): Add support for role-related statements

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

Change subject: IMPALA-10211 (Part 1): Add support for role-related statements
......................................................................


Patch Set 9:

Build Successful 

https://jenkins.impala.io/job/gerrit-code-review-checks/7877/ : 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/16837
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: Ic2b204e62a1d8ae1932d955b4efc28be22202860
Gerrit-Change-Number: 16837
Gerrit-PatchSet: 9
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Impala Public Jenkins <im...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Comment-Date: Thu, 17 Dec 2020 18:36:54 +0000
Gerrit-HasComments: No