[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-20 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has submitted this change and it was merged. ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..

IMPALA-6643: Add REFRESH fine-grained privilege

Before this patch, ALL privilege was required to execute INVALIDATE
METADATA and having any privilege allowed executing REFRESH 
and INVALIDATE METADATA . With this patch, REFRESH privilege
is now required to execute INVALIDATE METADATA or REFRESH statement.

These are the new GRANT/REVOKE statements introduced at server,
database, and table scopes.

GRANT REFRESH on SERVER svr TO ROLE testrole;
GRANT REFRESH on DATABASE db TO ROLE testrole;
GRANT REFRESH on TABLE db.tbl TO ROLE testrole;

REVOKE REFRESH on SERVER svr FROM ROLE testrole;
REVOKE REFRESH on DATABASE db FROM ROLE testrole;
REVOKE REFRESH on TABLE db.tbl FROM ROLE testrole;

Testing:
- Ran front-end tests

Cherry-picks: not for 2.x

Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Reviewed-on: http://gerrit.cloudera.org:8080/9589
Reviewed-by: Alex Behm 
Tested-by: Impala Public Jenkins
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/Privilege.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/resources/authz-policy.ini.template
10 files changed, 204 insertions(+), 51 deletions(-)

Approvals:
  Alex Behm: Looks good to me, approved
  Impala Public Jenkins: Verified

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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: merged
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 23
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-20 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..


Patch Set 22: Verified+1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 22
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 20 Mar 2018 20:37:34 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-20 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..


Patch Set 22: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 22
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 20 Mar 2018 16:47:25 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-20 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..


Patch Set 20: -Code-Review

Waiting for the final VIEW_METADATA and REFRESH change, plus tests


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 20
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 20 Mar 2018 15:53:45 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-20 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..


Patch Set 20:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9589/20/fe/src/main/java/org/apache/impala/authorization/Privilege.java
File fe/src/main/java/org/apache/impala/authorization/Privilege.java:

http://gerrit.cloudera.org:8080/#/c/9589/20/fe/src/main/java/org/apache/impala/authorization/Privilege.java@36
PS20, Line 36:   VIEW_METADATA(EnumSet.of(SentryAction.INSERT, 
SentryAction.SELECT), true),
I think REFRESH should also be in this list. Seems odd to allow refreshing but 
not inspecting metadata.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 20
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 20 Mar 2018 15:51:59 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-20 Thread Impala Public Jenkins (Code Review) 
Impala Public Jenkins has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..


Patch Set 20:

Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/2135/


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 20
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 20 Mar 2018 15:49:23 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-20 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..


Patch Set 20: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 20
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 20 Mar 2018 15:48:40 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-20 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has uploaded a new patch set (#20). ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..

IMPALA-6643: Add REFRESH fine-grained privilege

Before this patch, ALL privilege was required to execute INVALIDATE
METADATA and having any privilege allowed executing REFRESH 
and INVALIDATE METADATA . With this patch, REFRESH privilege
is now required to execute INVALIDATE METADATA or REFRESH statement.

These are the new GRANT/REVOKE statements introduced at server,
database, and table scopes.

GRANT REFRESH on SERVER svr TO ROLE testrole;
GRANT REFRESH on DATABASE db TO ROLE testrole;
GRANT REFRESH on TABLE db.tbl TO ROLE testrole;

REVOKE REFRESH on SERVER svr FROM ROLE testrole;
REVOKE REFRESH on DATABASE db FROM ROLE testrole;
REVOKE REFRESH on TABLE db.tbl FROM ROLE testrole;

Testing:
- Ran front-end tests

Cherry-picks: not for 2.x

Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/Privilege.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/resources/authz-policy.ini.template
10 files changed, 193 insertions(+), 50 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/9589/20
--
To view, visit http://gerrit.cloudera.org:8080/9589
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 20
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-19 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..


Patch Set 19:

Change looks good, but I'm intentionally holding off from merging to discuss 
the upgrade implications and whether this should go into 2.x or not.


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 19
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 20 Mar 2018 04:19:56 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-19 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..


Patch Set 19: Code-Review+2


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 19
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 20 Mar 2018 04:10:24 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-19 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has uploaded a new patch set (#19). ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..

IMPALA-6643: Add REFRESH fine-grained privilege

Before this patch, ALL privilege was required to execute INVALIDATE
METADATA and having any privilege allowed executing REFRESH 
and INVALIDATE METADATA . With this patch, REFRESH privilege
is now required to execute INVALIDATE METADATA or REFRESH statement.

These are the new GRANT/REVOKE statements introduced at server,
database, and table scopes.

GRANT REFRESH on SERVER svr TO ROLE testrole;
GRANT REFRESH on DATABASE db TO ROLE testrole;
GRANT REFRESH on TABLE db.tbl TO ROLE testrole;

REVOKE REFRESH on SERVER svr FROM ROLE testrole;
REVOKE REFRESH on DATABASE db FROM ROLE testrole;
REVOKE REFRESH on TABLE db.tbl FROM ROLE testrole;

Testing:
- Ran front-end tests

Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/Privilege.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/resources/authz-policy.ini.template
10 files changed, 193 insertions(+), 50 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/9589/19
--
To view, visit http://gerrit.cloudera.org:8080/9589
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 19
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-19 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..


Patch Set 18:

(3 comments)

Good to go after this

http://gerrit.cloudera.org:8080/#/c/9589/18/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java:

http://gerrit.cloudera.org:8080/#/c/9589/18/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@104
PS18, Line 104:   //   REFRESH permissions on 'functional_text_lzo' database
These are generally clustered by table and not by privilege (suggested changes 
below)


http://gerrit.cloudera.org:8080/#/c/9589/18/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@105
PS18, Line 105:   //   REFRESH permissions on 'functional.alltypesagg' table
combine with L88


http://gerrit.cloudera.org:8080/#/c/9589/18/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@106
PS18, Line 106:   //   REFRESH permissions on 'functional.view_view' view
combine with L90



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 18
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 20 Mar 2018 04:06:16 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-19 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has uploaded a new patch set (#18). ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..

IMPALA-6643: Add REFRESH fine-grained privilege

Before this patch, ALL privilege was required to execute INVALIDATE
METADATA and having any privilege allowed executing REFRESH 
and INVALIDATE METADATA . With this patch, REFRESH privilege
is now required to execute INVALIDATE METADATA or REFRESH statement.

These are the new GRANT/REVOKE statements introduced at server,
database, and table scopes.

GRANT REFRESH on SERVER svr TO ROLE testrole;
GRANT REFRESH on DATABASE db TO ROLE testrole;
GRANT REFRESH on TABLE db.tbl TO ROLE testrole;

REVOKE REFRESH on SERVER svr FROM ROLE testrole;
REVOKE REFRESH on DATABASE db FROM ROLE testrole;
REVOKE REFRESH on TABLE db.tbl FROM ROLE testrole;

Testing:
- Ran front-end tests

Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/Privilege.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/resources/authz-policy.ini.template
10 files changed, 193 insertions(+), 48 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/9589/18
--
To view, visit http://gerrit.cloudera.org:8080/9589
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 18
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-19 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..


Patch Set 15:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java:

http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@266
PS15, Line 266: roleName = "refresh_functional_alltypes";
> Unfortunately there are existing tests that also rely on functional.alltype
Yes. Please try to minimize tests and test tables as much as possible. In the 
long run it gets really had to maintain reason about the configuration if there 
is unnecessary redundancy.

Please update the class comment here with the new tables/privileges.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 15
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 20 Mar 2018 03:35:19 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-19 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..


Patch Set 17:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/main/java/org/apache/impala/authorization/Privilege.java
File fe/src/main/java/org/apache/impala/authorization/Privilege.java:

http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/main/java/org/apache/impala/authorization/Privilege.java@52
PS15, Line 52:   public enum SentryAction implements Action {
> I see. So this 1:N mapping may go away as we add more fine-grained privileg
Yup, I'll definitely keep that in mind.


http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java:

http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@266
PS15, Line 266: roleName = "refresh_functional_alltypes";
> Why is it not sufficient to only have refresh_functional_alltypes? Why do w
Unfortunately there are existing tests that also rely on 
functional.alltypesagg: 
https://github.com/apache/impala/blob/master/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java#L804-L805.
 I can refactor those tests to only use one table if you prefer that.

Looking at existing TestResetMetadata, there's only one place that uses 
functional.alltypes: 
https://github.com/apache/impala/blob/master/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java#L810.
 Should I just remove that test so that I can also remove the 
refresh_functional_alltypes role. I believe that test has already been covered 
by: 
https://github.com/apache/impala/blob/master/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java#L809



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 17
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 20 Mar 2018 01:38:25 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-19 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..


Patch Set 15:

(3 comments)

http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/main/java/org/apache/impala/authorization/Privilege.java
File fe/src/main/java/org/apache/impala/authorization/Privilege.java:

http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/main/java/org/apache/impala/authorization/Privilege.java@52
PS15, Line 52:   public enum SentryAction implements Action {
> Privilege enum may contain a collection of Sentry actions, for instance VIE
I see. So this 1:N mapping may go away as we add more fine-grained privileges? 
We should be sure to do that refactoring when the time comes.


http://gerrit.cloudera.org:8080/#/c/9589/17/fe/src/main/java/org/apache/impala/authorization/Privilege.java
File fe/src/main/java/org/apache/impala/authorization/Privilege.java:

http://gerrit.cloudera.org:8080/#/c/9589/17/fe/src/main/java/org/apache/impala/authorization/Privilege.java@36
PS17, Line 36:   VIEW_METADATA(EnumSet.of(SentryAction.INSERT, 
SentryAction.SELECT), true),
REFRESH does not allow inspecting the metadata? That seems particularly odd 
since REFRESH is for updating the metadata.


http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java:

http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@266
PS15, Line 266: roleName = "refresh_functional_alltypes";
> It's because of the existing test case: https://github.com/apache/impala/bl
Why is it not sufficient to only have refresh_functional_alltypes? Why do we 
also need refresh_functional_alltypesagg?



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 15
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Tue, 20 Mar 2018 01:21:33 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-19 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has uploaded a new patch set (#17). ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..

IMPALA-6643: Add REFRESH fine-grained privilege

Before this patch, ALL privilege was required to execute INVALIDATE
METADATA and having any privilege allowed executing REFRESH 
and INVALIDATE METADATA . With this patch, REFRESH privilege
is now required to execute INVALIDATE METADATA or REFRESH statement.

These are the new GRANT/REVOKE statements introduced at server,
database, and table scopes.

GRANT REFRESH on SERVER svr TO ROLE testrole;
GRANT REFRESH on DATABASE db TO ROLE testrole;
GRANT REFRESH on TABLE db.tbl TO ROLE testrole;

REVOKE REFRESH on SERVER svr FROM ROLE testrole;
REVOKE REFRESH on DATABASE db FROM ROLE testrole;
REVOKE REFRESH on TABLE db.tbl FROM ROLE testrole;

Testing:
- Ran front-end tests

Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/Privilege.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/resources/authz-policy.ini.template
10 files changed, 204 insertions(+), 47 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/9589/17
--
To view, visit http://gerrit.cloudera.org:8080/9589
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 17
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-19 Thread Alex Behm (Code Review) 
Alex Behm has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..


Patch Set 15:

(10 comments)

http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:

http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@116
PS15, Line 116: } else {
nit: I think it's more logical to structure the ifs this way since these really 
are three completely different cases:

if (tableName_ != null) {

} else if (database_ != null) {

} else {

}


http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java:

http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java@141
PS15, Line 141: if (privilege == Privilege.REFRESH) {
can you do "else if" here as well to make the ifs less nested?


http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/main/java/org/apache/impala/authorization/Privilege.java
File fe/src/main/java/org/apache/impala/authorization/Privilege.java:

http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/main/java/org/apache/impala/authorization/Privilege.java@52
PS15, Line 52:   public enum SentryAction implements Action {
What's the benefit of having a separate SentryAction as opposed to making 
Privilege implement Action directly?


http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java:

http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@266
PS15, Line 266: roleName = "refresh_functional_alltypes";
Why do we need both refresh_functional_alltypesagg and 
refresh_functional_alltypes? They both cover refresh at the table level.


http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@861
PS15, Line 861: // but no privilege on table-level access.
but no privileges at the table level.


http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@919
PS15, Line 919:   AuthzOk("invalidate metadata");
add all variants of invalidate and refresh here to make sure admins can do 
anything


http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@922
PS15, Line 922:   ((ImpaladTestCatalog) ctx_.catalog).reset();
I think we can remove the ImpalaTestCatalog cast


http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@925
PS15, Line 925: // User only has REFRESH privilege on server.
remove "only"


http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@935
PS15, Line 935:   AuthzOk("invalidate metadata");
add all variants of invalidate and refresh


http://gerrit.cloudera.org:8080/#/c/9589/15/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@938
PS15, Line 938:   ((ImpaladTestCatalog) ctx_.catalog).reset();
I think we can remove the ImpalaTestCatalog cast



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 15
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Mon, 19 Mar 2018 23:09:40 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-19 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has uploaded a new patch set (#15). ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..

IMPALA-6643: Add REFRESH fine-grained privilege

Before this patch, ALL privilege was required to execute INVALIDATE
METADATA and having any privilege allowed executing REFRESH 
and INVALIDATE METADATA . With this patch, REFRESH privilege
is now required to execute INVALIDATE METADATA or REFRESH statement.

These are the new GRANT/REVOKE statements introduced at server,
database, and table scopes.

GRANT REFRESH on SERVER svr TO ROLE testrole;
GRANT REFRESH on DATABASE db TO ROLE testrole;
GRANT REFRESH on TABLE db.tbl TO ROLE testrole;

REVOKE REFRESH on SERVER svr FROM ROLE testrole;
REVOKE REFRESH on DATABASE db FROM ROLE testrole;
REVOKE REFRESH on TABLE db.tbl FROM ROLE testrole;

TODO: Add authorization end-to-end tests. See IMPALA-6674.

Testing:
- Ran front-end tests

Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/Privilege.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/resources/authz-policy.ini.template
10 files changed, 201 insertions(+), 48 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/9589/15
--
To view, visit http://gerrit.cloudera.org:8080/9589
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 15
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-17 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..


Patch Set 14: Code-Review+1

carry +1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 14
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Sat, 17 Mar 2018 21:06:25 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-17 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has uploaded a new patch set (#14). ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..

IMPALA-6643: Add REFRESH fine-grained privilege

Before this patch, ALL privilege was required to execute INVALIDATE
METADATA and having any privilege allowed executing REFRESH 
and INVALIDATE METADATA . With this patch, REFRESH privilege
is now required to execute INVALIDATE METADATA or REFRESH statement.

These are the new GRANT/REVOKE statements introduced at server,
database, and table scopes.

GRANT REFRESH on SERVER svr TO ROLE testrole;
GRANT REFRESH on DATABASE db TO ROLE testrole;
GRANT REFRESH on TABLE db.tbl TO ROLE testrole;

REVOKE REFRESH on SERVER svr FROM ROLE testrole;
REVOKE REFRESH on DATABASE db FROM ROLE testrole;
REVOKE REFRESH on TABLE db.tbl FROM ROLE testrole;

TODO: Add authorization end-to-end tests. See IMPALA-6674.

Testing:
- Ran front-end tests

Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/PrivilegeSpec.java
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/Privilege.java
M fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/resources/authz-policy.ini.template
10 files changed, 189 insertions(+), 45 deletions(-)


  git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/9589/14
--
To view, visit http://gerrit.cloudera.org:8080/9589
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 14
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-17 Thread Vuk Ercegovac (Code Review) 
Vuk Ercegovac has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..


Patch Set 13:

(1 comment)

http://gerrit.cloudera.org:8080/#/c/9589/13/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java:

http://gerrit.cloudera.org:8080/#/c/9589/13/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@860
PS13, Line 860: User has REFRESH privilege
just become aware of "refresh functions ".
so, I want to confirm that this change applies to that case and if so, lets add 
a test for it.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 13
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Sat, 17 Mar 2018 18:42:56 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-16 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..


Patch Set 13: Code-Review+1

carry +1


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 13
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Alex Behm 
Gerrit-Reviewer: Bharath Vissapragada 
Gerrit-Reviewer: Fredy Wijaya 
Gerrit-Reviewer: Vuk Ercegovac 
Gerrit-Comment-Date: Fri, 16 Mar 2018 19:07:50 +
Gerrit-HasComments: No

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-12 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has uploaded a new patch set (#4). ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..

IMPALA-6643: Add REFRESH fine-grained privilege

With this patch, REFRESH privilege is now required to execute INVALIDATE
METADATA or REFRESH statement.

These are the new GRANT/REVOKE statements introduced.

GRANT REFRESH on DATABASE db TO ROLE testrole;
GRANT REFRESH on TABLE db.tbl TO ROLE testrole;

REVOKE REFRESH on DATABASE db TO ROLE testrole;
REVOKE REFRESH on TABLE db.tbl TO ROLE testrole;

Testing:
- Ran end-to-end tests

Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/Privilege.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/resources/authz-policy.ini.template
8 files changed, 136 insertions(+), 36 deletions(-)


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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newpatchset
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 4
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Reviewer: Fredy Wijaya

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-12 Thread Adam Holley (Code Review) 
Adam Holley has posted comments on this change. ( 
http://gerrit.cloudera.org:8080/9589 )

Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..


Patch Set 2:

(8 comments)

http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
File fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java:

http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java@116
PS2, Line 116:  analyzer.registerPrivReq(new PrivilegeRequest(Privilege.ALL));
Shouldn't the refresh privilege be applied to this case as well?  i.e. 
onServer()
Might require an e2e test instead of AuthorizationTest.


http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/main/java/org/apache/impala/authorization/Privilege.java
File fe/src/main/java/org/apache/impala/authorization/Privilege.java:

http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/main/java/org/apache/impala/authorization/Privilege.java@50
PS2, Line 50: SentryAction
Is this the best name?  Since Sentry does not care what the string is, and it's 
more specific to SQL.  Refresh allows the invalidate metadata action.  This 
list is really privileges.  Maybe SQLPrivilege?

Also, should this be it's own file?  I know there's cases for either way in the 
code.


http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
File fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java:

http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@236
PS2, Line 236:
Update comment at top of file to reflect new setup.


http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@249
PS2, Line 249: roleName = "refresh_functional_text_lzo";
 : sentryService.createRole(USER, roleName, true);
 : sentryService.grantRoleToGroup(USER, roleName, 
USER.getName());
Does this chunk need to be duplicated?


http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@261
PS2, Line 261: roleName = "refresh_functional_text_lzo";
 : sentryService.createRole(USER, roleName, true);
 : sentryService.grantRoleToGroup(USER, roleName, 
USER.getName());
duplicate?


http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@273
PS2, Line 273: roleName = "refresh_functional_text_lzo";
 : sentryService.createRole(USER, roleName, true);
 : sentryService.grantRoleToGroup(USER, roleName, 
USER.getName());
duplicate?


http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
File fe/src/test/java/org/apache/impala/analysis/ParserTest.java:

http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@3579
PS2, Line 3579: // REFRESH privilege object.
I think we need refresh on SERVER also.


http://gerrit.cloudera.org:8080/#/c/9589/2/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@3580
PS2, Line 3580: ParsesOk(String.format("%s REFRESH ON DATABASE foo %s myRole", 
formatStr));
There's no current method to "REFRESH" or "INVALIDATE METADATA" at the database 
level, only server and table.



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya 
Gerrit-Reviewer: Adam Holley 
Gerrit-Comment-Date: Tue, 13 Mar 2018 04:31:42 +
Gerrit-HasComments: Yes

[Impala-ASF-CR] IMPALA-6643: Add REFRESH fine-grained privilege

2018-03-12 Thread Fredy Wijaya (Code Review) 
Fredy Wijaya has uploaded this change for review. ( 
http://gerrit.cloudera.org:8080/9589


Change subject: IMPALA-6643: Add REFRESH fine-grained privilege
..

IMPALA-6643: Add REFRESH fine-grained privilege

With this patch, REFRESH privilege is now required to execute INVALIDATE
METADATA or REFRESH statement.

Testing:
- Ran end-to-end tests

Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
---
M common/thrift/CatalogObjects.thrift
M fe/src/main/cup/sql-parser.cup
M fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java
M fe/src/main/java/org/apache/impala/authorization/AuthorizationChecker.java
M fe/src/main/java/org/apache/impala/authorization/Privilege.java
M fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java
M fe/src/test/java/org/apache/impala/analysis/ParserTest.java
M fe/src/test/resources/authz-policy.ini.template
8 files changed, 137 insertions(+), 36 deletions(-)



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

Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: newchange
Gerrit-Change-Id: I4c3c5a51fe493d39fd719c7a388d4d5760049ce4
Gerrit-Change-Number: 9589
Gerrit-PatchSet: 2
Gerrit-Owner: Fredy Wijaya