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 2022/06/30 22:23:57 UTC

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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


Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................

IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

This patch allows Impala users to access views created by non-superusers
in HiveMetaStore, i.e., views with the table property of 'Authorized'
set to false.

Recall that a user is considered as a non-superuser by HiveMetaStore if
the IP address of the user is not on the list specified by the Hadoop
configuration of 'hadoop.proxyuser.<username>.hosts', where <username>
denotes the short name corresponding to the Kerberos principal name of
the user. For a view created by a non-superuser, HiveMetaStore adds to
the view the table property of 'Authorized' and sets the value of this
property to false after HIVE-24026.

We prevented any Impala user from accessing such views in part 1 of this
JIRA. To enable an Impala user to access such views, this patch enforces
the privilege checks for the underlying tables of a view additionally if
the given view was created by a non-superuser in HiveMetaStore.

Testing:
 - Added an E2E test to verify the necessary privileges on the
   underlying tables are required in order to access a view created by
   a non-superuser.

Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M tests/authorization/test_ranger.py
11 files changed, 109 insertions(+), 128 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@cloudera.com>
Gerrit-Reviewer: Csaba Ringhofer <cs...@cloudera.com>
Gerrit-Reviewer: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................

IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

This patch allows Impala users to access views created by non-superusers
in HiveMetaStore, i.e., views with the table property of 'Authorized'
set to false.

Recall that a user is considered as a non-superuser by HiveMetaStore if
the IP address of the user is not on the list specified by the Hadoop
configuration of 'hadoop.proxyuser.<username>.hosts', where <username>
denotes the short name corresponding to the Kerberos principal name of
the user. For a view created by a non-superuser, HiveMetaStore adds to
the view the table property of 'Authorized' and sets the value of this
property to false after HIVE-24026.

We prevented any Impala user from accessing such views in part 1 of this
JIRA. To enable an Impala user to access such views, this patch enforces
the privilege checks for the underlying tables of a view additionally if
the given view was created by a non-superuser in HiveMetaStore.

Testing:
 - Added an E2E test to verify the necessary privileges on the
   underlying tables are required in order to access a view created by
   a non-superuser.

Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M tests/authorization/test_ranger.py
11 files changed, 187 insertions(+), 128 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/18684/3
-- 
To view, visit http://gerrit.cloudera.org:8080/18684
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 3
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 2: Code-Review+1

(1 comment)

The patch LGTM. I'd like to see if other reviewers have further comments. If not, I can bump to +2.

http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1568
PS1, Line 1568:           # resource from a role, but we do not have to handle such an exception. We only
> Thanks Quanlong!
I see. So column-level SELECT privileges do imply VIEW_METADATA privilege and we don't need to hide the underlying table name in the error message. Thanks for the explanation!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 Jul 2022 02:24:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................

IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

This patch allows Impala users to access views created by non-superusers
in HiveMetaStore, i.e., views with the table property of 'Authorized'
set to false.

Recall that a user is considered as a non-superuser by HiveMetaStore if
the IP address of the user is not on the list specified by the Hadoop
configuration of 'hadoop.proxyuser.<username>.hosts', where <username>
denotes the short name corresponding to the Kerberos principal name of
the user. For a view created by a non-superuser, HiveMetaStore adds to
the view the table property of 'Authorized' and sets the value of this
property to false after HIVE-24026.

We prevented any Impala user from accessing such views in part 1 of this
JIRA. To enable an Impala user to access such views, this patch enforces
the privilege checks for the underlying tables of a view additionally if
the given view was created by a non-superuser in HiveMetaStore.

Testing:
 - Added an E2E test to verify the necessary privileges on the
   underlying tables are required in order to access a view created by
   a non-superuser.

Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M tests/authorization/test_ranger.py
11 files changed, 176 insertions(+), 128 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 4: Verified+1 Code-Review+2

The failure is due to a flaky test: IMPALA-1995	
https://jenkins.impala.io/job/ubuntu-16.04-from-scratch/16917/

Merging the patch.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 4
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Jul 2022 23:00:38 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 2:

(43 comments)

http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@785
PS2, Line 785: d
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@827
PS2, Line 827: @
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1574
PS2, Line 1574: @
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1575
PS2, Line 1575: @
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1577
PS2, Line 1577: d
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1580
PS2, Line 1580: @
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1581
PS2, Line 1581: @
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1584
PS2, Line 1584: d
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1587
PS2, Line 1587: d
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1605
PS2, Line 1605: #
flake8: E114 indentation is not a multiple of four (comment)


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1606
PS2, Line 1606: s
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1608
PS2, Line 1608: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1610
PS2, Line 1610: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1612
PS2, Line 1612: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1614
PS2, Line 1614: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1616
PS2, Line 1616: r
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1618
PS2, Line 1618: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1621
PS2, Line 1621: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1623
PS2, Line 1623: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1624
PS2, Line 1624: #
flake8: E114 indentation is not a multiple of four (comment)


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1625
PS2, Line 1625: #
flake8: E114 indentation is not a multiple of four (comment)


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1626
PS2, Line 1626: r
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1628
PS2, Line 1628: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1631
PS2, Line 1631: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1633
PS2, Line 1633: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1635
PS2, Line 1635: #
flake8: E114 indentation is not a multiple of four (comment)


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1636
PS2, Line 1636: s
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1639
PS2, Line 1639: #
flake8: E114 indentation is not a multiple of four (comment)


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1640
PS2, Line 1640: #
flake8: E114 indentation is not a multiple of four (comment)


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1641
PS2, Line 1641: #
flake8: E114 indentation is not a multiple of four (comment)


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1642
PS2, Line 1642: T
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1643
PS2, Line 1643: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1645
PS2, Line 1645: #
flake8: E114 indentation is not a multiple of four (comment)


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1646
PS2, Line 1646: #
flake8: E114 indentation is not a multiple of four (comment)


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1647
PS2, Line 1647: r
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1649
PS2, Line 1649: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1652
PS2, Line 1652: s
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1654
PS2, Line 1654: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1656
PS2, Line 1656: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1658
PS2, Line 1658: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1660
PS2, Line 1660: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1662
PS2, Line 1662: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/2/tests/authorization/test_ranger.py@1664
PS2, Line 1664: T
flake8: E111 indentation is not a multiple of four



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 Jul 2022 00:54:35 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 2:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 Jul 2022 01:13:57 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 2:

(1 comment)

> Patch Set 2:
> 
> (4 comments)
> 
> Hi all, I have revised the patch according to Quanlong's comments in the previous iteration. Let me know if you have any other suggestion.
> 
> Thank you very much for the help!

http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1582
PS1, Line 1582:     impalad_args=LOCAL_CATALOG_IMPALAD_ARGS,
> Yes. We can add such a test.
We should be able to avoid the resource collision by creating a view in a unique database. It would make this E2E test much less vulnerable to newly added authorization tests involving the same column in the future.

I will try to see if I can revise this E2E test in the next patch.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 Jul 2022 22:22:58 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1531
PS1, Line 1531: @
> I am a bit confused by this check. The indentation of the tests added in th
Yes, your code is fine. The linter is flagging it incorrectly because https://gerrit.cloudera.org/c/18669/ was merged earlier today. It aimed to update flake8 and set 'indent-size = 2', which changes indentation checks to expect 2 spaces instead of 4; that meant we could remove exceptions for E111 and E114.. But the gerrit check uses a different version of flake8 and doesn't recognize indent-size.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Jun 2022 22:42:42 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 1:

(1 comment)

> Patch Set 1:
> 
> (1 comment)

Thanks for the explanation Michael!

http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1531
PS1, Line 1531: @
> I am a bit confused by this check. The indentation of the tests added in th
Thanks Michael!



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Jun 2022 22:49:21 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 4:

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 4
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Jul 2022 11:50:37 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/18684 )

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................

IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

This patch allows Impala users to access views created by non-superusers
in HiveMetaStore, i.e., views with the table property of 'Authorized'
set to false.

Recall that a user is considered as a non-superuser by HiveMetaStore if
the IP address of the user is not on the list specified by the Hadoop
configuration of 'hadoop.proxyuser.<username>.hosts', where <username>
denotes the short name corresponding to the Kerberos principal name of
the user. For a view created by a non-superuser, HiveMetaStore adds to
the view the table property of 'Authorized' and sets the value of this
property to false after HIVE-24026.

We prevented any Impala user from accessing such views in part 1 of this
JIRA. To enable an Impala user to access such views, this patch enforces
the privilege checks for the underlying tables of a view additionally if
the given view was created by a non-superuser in HiveMetaStore.

Testing:
 - Added an E2E test to verify the necessary privileges on the
   underlying tables are required in order to access a view created by
   a non-superuser.

Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Reviewed-on: http://gerrit.cloudera.org:8080/18684
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Quanlong Huang <hu...@gmail.com>
Tested-by: Quanlong Huang <hu...@gmail.com>
---
M fe/src/main/java/org/apache/impala/analysis/Analyzer.java
M fe/src/main/java/org/apache/impala/analysis/DropTableOrViewStmt.java
M fe/src/main/java/org/apache/impala/analysis/InlineViewRef.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizableTable.java
M fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/DefaultAuthorizableFactory.java
M fe/src/main/java/org/apache/impala/authorization/PrivilegeRequestBuilder.java
M fe/src/main/java/org/apache/impala/service/Frontend.java
M tests/authorization/test_ranger.py
11 files changed, 187 insertions(+), 128 deletions(-)

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

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 5
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 4: Verified-1

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


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 4
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Jul 2022 16:38:43 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

Posted by "Quanlong Huang (Code Review)" <ge...@cloudera.org>.
Quanlong Huang has removed a vote on this change.

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Removed Verified-1 by Impala Public Jenkins <im...@cloudera.com>
-- 
To view, visit http://gerrit.cloudera.org:8080/18684
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: deleteVote
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 4
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 1:

(27 comments)

http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1531
PS1, Line 1531: @
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1532
PS1, Line 1532: @
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1534
PS1, Line 1534: d
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1537
PS1, Line 1537: @
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1538
PS1, Line 1538: @
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1541
PS1, Line 1541: d
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1544
PS1, Line 1544: d
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1557
PS1, Line 1557: #
flake8: E114 indentation is not a multiple of four (comment)


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1558
PS1, Line 1558: s
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1560
PS1, Line 1560: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1562
PS1, Line 1562: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1564
PS1, Line 1564: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1566
PS1, Line 1566: r
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1568
PS1, Line 1568: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1571
PS1, Line 1571: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1573
PS1, Line 1573: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1575
PS1, Line 1575: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1577
PS1, Line 1577: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1579
PS1, Line 1579: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1581
PS1, Line 1581: s
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1584
PS1, Line 1584: s
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1586
PS1, Line 1586: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1588
PS1, Line 1588: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1590
PS1, Line 1590: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1592
PS1, Line 1592: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1594
PS1, Line 1594: a
flake8: E111 indentation is not a multiple of four


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1596
PS1, Line 1596: a
flake8: E111 indentation is not a multiple of four



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Jun 2022 22:24:44 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 3:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 3
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Sun, 03 Jul 2022 00:42:41 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 3: Code-Review+2

Today, I review the patch again and feel confident to merge it - we just treat such 'unauthorized' view as inlineView and don't mask any privilege errors.

Thanks for making this resolved, Fang-Yu!


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 3
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Jul 2022 11:49:51 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 4: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 4
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Jul 2022 11:50:36 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 3: Code-Review+1

LGTM. Thanks, Fang-Yu.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 3
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Thu, 07 Jul 2022 11:15:22 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 2:

(4 comments)

Hi all, I have revised the patch according to Quanlong's comments in the previous iteration. Let me know if you have any other suggestion.

Thank you very much for the help!

http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1568
PS1, Line 1568:           # resource from a role, but we do not have to handle such an exception. We only
> Should we hide the table name if the user doesn't have VIEW_METADATA privil
Thanks Quanlong!

There are only 2 columns in the view 'complex_view' in total.

When we do not grant the requesting user the SELECT privilege on the view 'complex_view', an AuthorizationException will be thrown and caught at https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java#L272. Impala would then check whether the requesting user has the SELECT privilege on each column in this view.

Since in the test case, we granted the requesting user the SELECT privileges on both columns in the view 'complex_view', in the end BaseAuthorizationChecker#authorizeTableAccess() does not throw an AuthorizationException for this view.

Impala's FE then continued to perform the privilege checks for the next table (https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java#L166), which was the table 'functional.alltypesagg'. The requesting user was not granted any privilege on this table and thus an AuthorizationException was thrown.

I think this behavior is expected and reasonable.

If we only granted the requesting user the SELECT privilege on a single column in the view 'complex_view', then Impala's FE would throw the following error.

ERROR: AuthorizationException: User 'non_owner' does not have privileges to execute 'SELECT' on: functional.complex_view


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1569
PS1, Line 1569:  need to make sure in 
> Not sure how the table is picked if there are more than one table fails the
Thanks Quanlong!

It depends on the order in which 'tablePrivReqs' is iterated at https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/authorization/BaseAuthorizationChecker.java#L165.

I will revise the test accordingly to make sure the reported error always corresponds to the same table.


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1572
PS1, Line 1572:           pass
> Can we add a test after this line for the same query? Just checking the cas
Yes. I will add a test after this line in the next patch.


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1582
PS1, Line 1582:     impalad_args=LOCAL_CATALOG_IMPALAD_ARGS,
> Can we add another test that the alltypesagg table is granted select but on
Yes. We can add such a test.

In the next patch, I will grant the requesting user the SELECT privilege on the table 'alltypesagg' and add a deny policy to prevent the requesting user from accessing the column 'id'.

The error message would look like the following.

ERROR: AuthorizationException: User 'non_owner' does not have privileges to execute 'SELECT' on: functional.alltypesagg

On a related note, when a deny policy is created via Ranger's REST API, it looks like Ranger would perform some semantic check to make sure no existing policy is associated with the same resource. In our case, it would be the column 'id' of the table 'functional.alltypesagg'. That means we may not always be able to add such a deny policy if there already exists a policy associated with the same column in Ranger.

But we can wait and see if the deny policy we choose here could be successfully added later on.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 2
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Sat, 02 Jul 2022 00:55:01 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1531
PS1, Line 1531: @
> May want to revert https://gerrit.cloudera.org/c/18669/, looks like it's no
I am a bit confused by this check. The indentation of the tests added in this patch is the same as the rest of the tests in the same test file.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Jun 2022 22:35:36 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 1:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1531
PS1, Line 1531: @
> flake8: E111 indentation is not a multiple of four
May want to revert https://gerrit.cloudera.org/c/18669/, looks like it's not complete.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Jun 2022 22:32:38 +0000
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 1:

Build Successful 

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Thu, 30 Jun 2022 22:44:46 +0000
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-10122 (Part 2): Allow accessing views created by non-superusers

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

Change subject: IMPALA-10122 (Part 2): Allow accessing views created by non-superusers
......................................................................


Patch Set 1:

(4 comments)

Pretty happy to see this patch, especially we can remove lots of codes. :)

http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py
File tests/authorization/test_ranger.py:

http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1568
PS1, Line 1568:       assert "User '{0}' does not have privileges to execute 'SELECT' on: " \
Should we hide the table name if the user doesn't have VIEW_METADATA privilege on the view? I think SELECT privilege on the view implies VIEW_METADATA privilege on the view. But not sure if SELECT privilege on the view columns also imply VIEW_METADATA on the view.


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1569
PS1, Line 1569: functional.alltypesagg
Not sure how the table is picked if there are more than one table fails the privilege check. Can we grant access on the other table "alltypestiny"? It makes sure the reported error is always on "alltypesagg".


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1572
PS1, Line 1572:           .format(test_db, test_tbl_1, grantee_user), user=ADMIN)
Can we add a test after this line for the same query? Just checking the case if only one column is lack of privilege.


http://gerrit.cloudera.org:8080/#/c/18684/1/tests/authorization/test_ranger.py@1582
PS1, Line 1582:           "select * from functional.complex_view", user=grantee_user)
Can we add another test that the alltypesagg table is granted select but one column of it is denied for access? Just want to see how the error message looks like.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I50a50931c6eeb0feec28c30531b09269622e6aad
Gerrit-Change-Number: 18684
Gerrit-PatchSet: 1
Gerrit-Owner: Fang-Yu Rao <fa...@cloudera.com>
Gerrit-Reviewer: Aman Sinha <am...@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: Kurt Deschler <kd...@cloudera.com>
Gerrit-Reviewer: Michael Smith <mi...@cloudera.com>
Gerrit-Reviewer: Quanlong Huang <hu...@gmail.com>
Gerrit-Reviewer: Vincent Tran <vt...@cloudera.com>
Gerrit-Comment-Date: Fri, 01 Jul 2022 01:53:25 +0000
Gerrit-HasComments: Yes