[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG@15 PS8, Line 15: REFRESH METADATA > REFRESH works for me. wfm. -- 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: 8 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 16:52:54 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. Patch Set 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG@15 PS8, Line 15: are the new GRAN > REFRESH works for me. Works for me, too. Should I make a change to use REFRESH? -- 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: 12 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 16:38:05 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG@15 PS8, Line 15: REFRESH METADATA > will just throw it out there in hopes of simplifying, but how about just "r REFRESH works for me. Any objections? -- 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: 8 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 16:27:10 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Fredy Wijaya has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. IMPALA-6643: Add REFRESH METADATA fine-grained privilege Before this patch, ALL privilege is required to execute INVALIDATE METADATA and having any privilege allows executing REFRESH AND INVALIDATE METADATA . With this patch, REFRESH METADATA 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 METADATA on SERVER svr TO ROLE testrole; GRANT REFRESH METADATA on DATABASE db TO ROLE testrole; GRANT REFRESH METADATA on TABLE db.tbl TO ROLE testrole; REVOKE REFRESH METADATA on SERVER svr FROM ROLE testrole; REVOKE REFRESH METADATA on DATABASE db FROM ROLE testrole; REVOKE REFRESH METADATA on TABLE db.tbl FROM ROLE testrole; TODO: Add end-to-end tests. See IMPALA-6674. Testing: - Ran front-end 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/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, 182 insertions(+), 45 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/9589/12 -- 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: 12 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 METADATA fine-grained privilege
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG@15 PS8, Line 15: REFRESH METADATA > WFM. We could also make it "METADATA REFRESH" privilege if we think users w I'm sold. I now like "METADATA REFRESH" most :) Fredy, Vuk, any thoughts? -- 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: 8 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: Thu, 15 Mar 2018 16:23:53 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. Patch Set 11: Before I forget, I was expecting to see tests for grant, drop, and show so that the feature can be fully demonstrated. If testing show + auth is difficult, pls add a jira and a todo in the most relevant place where such a test would show up. -- 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: 11 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: Thu, 15 Mar 2018 15:53:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. Patch Set 11: Code-Review+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: 11 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: Thu, 15 Mar 2018 15:41:16 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. Patch Set 11: Code-Review+1 rebase and 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: 11 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: Thu, 15 Mar 2018 06:10:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Fredy Wijaya has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. IMPALA-6643: Add REFRESH METADATA fine-grained privilege Before this patch, ALL privilege is required to execute INVALIDATE METADATA and having any privilege allows executing REFRESH AND INVALIDATE METADATA . With this patch, REFRESH METADATA 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 METADATA on SERVER svr TO ROLE testrole; GRANT REFRESH METADATA on DATABASE db TO ROLE testrole; GRANT REFRESH METADATA on TABLE db.tbl TO ROLE testrole; REVOKE REFRESH METADATA on SERVER svr FROM ROLE testrole; REVOKE REFRESH METADATA on DATABASE db FROM ROLE testrole; REVOKE REFRESH METADATA on TABLE db.tbl FROM ROLE testrole; Testing: - Ran front-end 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/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, 182 insertions(+), 45 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/9589/11 -- 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: 11 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 METADATA fine-grained privilege
Fredy Wijaya has uploaded a new patch set (#10). ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. IMPALA-6643: Add REFRESH METADATA fine-grained privilege Before this patch, ALL privilege is required to execute INVALIDATE METADATA and havingn any privilege allows executing REFRESH AND INVALIDATE METADATA . With this patch, REFRESH METADATA 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 METADATA on SERVER svr TO ROLE testrole; GRANT REFRESH METADATA on DATABASE db TO ROLE testrole; GRANT REFRESH METADATA on TABLE db.tbl TO ROLE testrole; REVOKE REFRESH METADATA on SERVER svr FROM ROLE testrole; REVOKE REFRESH METADATA on DATABASE db FROM ROLE testrole; REVOKE REFRESH METADATA on TABLE db.tbl FROM ROLE testrole; Testing: - Ran front-end 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/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, 182 insertions(+), 45 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/9589/10 -- 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: 10 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 METADATA fine-grained privilege
Bharath Vissapragada has posted comments on this change. ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. Patch Set 9: Code-Review+1 (3 comments) Code changes look ok to me. +1 once the commit message is updated. Please raise a doc jira for this. http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG@11 PS8, Line 11: > Also please mention what was the requirement before this patch. forgot this? http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG@15 PS8, Line 15: REFRESH METADATA > Let me chime in with my 2 cents: WFM. We could also make it "METADATA REFRESH" privilege if we think users would confuse it with REFRESH command. Either wfm. http://gerrit.cloudera.org:8080/#/c/9589/8/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/8/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@249 PS8, Line 249: roleName = "refresh_functional_alltypesagg"; > Yup, you're right if we only consider using Sentry Service. We can easily s Okay. let's leave it this way then, doesn't hurt to have some extra lines. -- 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: 9 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: Thu, 15 Mar 2018 05:38:21 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Alex Behm has posted comments on this change. ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. Patch Set 8: (1 comment) http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG@15 PS8, Line 15: REFRESH METADATA > We need to decide what privilege name we should use, some suggestions: Let me chime in with my 2 cents: * I prefer REFRESH METADATA because it think it convey's the user's typical intent. Even when running INVALIDATE METADATA I think the user's intent is to "sync/update/refresh" Impala's metadata cache. * Regarding RELOAD. I think the same confusion argument applies here. We're introducing yet another term to mean "update metadata cache". User's already have to deal with two things, let's not add a third one. It might be better to use something that is familiar, even if is not obvious from the name that "invalidate" is also included. * To me, RESET has a connotation of "wipe everything" which I think does not match the user's typical thinking and intent. Of course, it's anyone's guess what user's are really thinking, so I might be wrong. -- 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: 8 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: Thu, 15 Mar 2018 04:15:23 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. Patch Set 9: (1 comment) http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG@15 PS8, Line 15: REFRESH METADATA > We need to decide what privilege name we should use, some suggestions: After giving more thought, RESET isn't good since it's not currently a reserved keyword. We probably shouldn't be introducing a new keyword unless it's absolutely necessary. I'm now back to first choice, that is REFRESH METADATA. Thoughts? -- 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: 9 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 15 Mar 2018 04:07:20 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Fredy Wijaya has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. IMPALA-6643: Add REFRESH METADATA fine-grained privilege With this patch, REFRESH METADATA 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 METADATA on SERVER svr TO ROLE testrole; GRANT REFRESH METADATA on DATABASE db TO ROLE testrole; GRANT REFRESH METADATA on TABLE db.tbl TO ROLE testrole; REVOKE REFRESH METADATA on SERVER svr FROM ROLE testrole; REVOKE REFRESH METADATA on DATABASE db FROM ROLE testrole; REVOKE REFRESH METADATA on TABLE db.tbl FROM ROLE testrole; Testing: - Ran front-end 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/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, 182 insertions(+), 45 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/9589/9 -- 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: 9 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac
[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. Patch Set 8: (2 comments) http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG Commit Message: http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG@15 PS8, Line 15: REFRESH METADATA > We had few internal discussions on what name we should use. Initially it wa We need to decide what privilege name we should use, some suggestions: - REFRESH - REFRESH METADATA - RELOAD METADATA - RESET METADATA I'm more leaning into RESET METADATA since that's what we use in code: https://github.com/apache/impala/blob/master/fe/src/main/java/org/apache/impala/analysis/ResetMetadataStmt.java http://gerrit.cloudera.org:8080/#/c/9589/8/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/8/fe/src/main/java/org/apache/impala/authorization/Privilege.java@50 PS8, Line 50: Sentry actions Impala can :* use without being tied to Hive actions since there may be privileges > slightly confused by this: does a new reader of this code need to know abou You got a point. I'll update the comment. -- 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: 8 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 15 Mar 2018 03:40:36 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Vuk Ercegovac has posted comments on this change. ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. Patch Set 8: (2 comments) looks good, main questions I have are about scope and naming. for scope, are the various 'show' commands applicable for this change? the naming question is in the comments. http://gerrit.cloudera.org:8080/#/c/9589/8/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/8/fe/src/main/java/org/apache/impala/authorization/Privilege.java@50 PS8, Line 50: Sentry actions Impala can :* use without being tied to Hive actions since there may be privileges slightly confused by this: does a new reader of this code need to know about how this relates to Hive actions? http://gerrit.cloudera.org:8080/#/c/9589/8/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/8/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@3579 PS8, Line 3579: REFRESH METADATA I'm sure its been discussed, so feel free to point me to relevant info, but why does the privilege include the word "REFRESH", which overlaps with one of two actions that this enables (refresh metadata) and not the other (invalidate 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: 8 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Vuk Ercegovac Gerrit-Comment-Date: Thu, 15 Mar 2018 03:15:30 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/9589 )
Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege
..
Patch Set 8:
(3 comments)
http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG@15
PS8, Line 15: REFRESH METADATA
> Just thinking out loud, should we rename it to RELOAD METADATA? From a user
We had few internal discussions on what name we should use. Initially it was
simply "REFRESH", but then we decided to change it to "REFRESH METADATA". I
guess it's to mean a privilege for both "REFRESH" and "INVALIDATE METADATA".
I'm open to suggestion though.
http://gerrit.cloudera.org:8080/#/c/9589/8/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/8/fe/src/main/java/org/apache/impala/authorization/Privilege.java@54
PS8, Line 54: public enum SentryAction implements Action {
> Just to be sure, these Impala specific privilege grants to groups are ignor
That's correct.
http://gerrit.cloudera.org:8080/#/c/9589/8/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/8/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@249
PS8, Line 249: roleName = "refresh_functional_alltypesagg";
> I think you could keep a single role (say "refresh_metadata_role") and keep
Yup, you're right if we only consider using Sentry Service. We can easily
switch between roles. With Sentry authorization policy file, we have no way of
switching roles. I believe the idea to keep the roles match between Sentry
Service and Sentry authorization file is for readability although it may be
redundant for Sentry service.
--
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: 8
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Adam Holley
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Thu, 15 Mar 2018 02:50:40 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/9589 )
Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege
..
Patch Set 8:
(4 comments)
The patch looks good to me. Just have a few suggestions.
http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG
Commit Message:
http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG@11
PS8, Line 11:
Also please mention what was the requirement before this patch.
http://gerrit.cloudera.org:8080/#/c/9589/8//COMMIT_MSG@15
PS8, Line 15: REFRESH METADATA
Just thinking out loud, should we rename it to RELOAD METADATA? From a user
perspective, I think it would be confusing to say "a REFRESH METADATA privilege
is required to run INVALIDATE METADATA"? What do you think? May be others have
a different preference.
http://gerrit.cloudera.org:8080/#/c/9589/8/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/8/fe/src/main/java/org/apache/impala/authorization/Privilege.java@54
PS8, Line 54: public enum SentryAction implements Action {
Just to be sure, these Impala specific privilege grants to groups are ignored
by Hive right?
http://gerrit.cloudera.org:8080/#/c/9589/8/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/8/fe/src/test/java/org/apache/impala/analysis/AuthorizationTest.java@249
PS8, Line 249: roleName = "refresh_functional_alltypesagg";
I think you could keep a single role (say "refresh_metadata_role") and keep
doing grantRolePrivilege() on that right? I think multiple createRole() calls
are redundant if you are not switching between them, no?
I see the pattern in rest of the test too, but wondering if it is needed.
--
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: 8
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Adam Holley
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Vuk Ercegovac
Gerrit-Comment-Date: Thu, 15 Mar 2018 02:26:49 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Adam Holley has posted comments on this change. ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. Patch Set 8: Code-Review+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: 8 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya Gerrit-Comment-Date: Tue, 13 Mar 2018 18:59:59 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Fredy Wijaya has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. IMPALA-6643: Add REFRESH METADATA fine-grained privilege With this patch, REFRESH METADATA 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 METADATA on SERVER svr TO ROLE testrole; GRANT REFRESH METADATA on DATABASE db TO ROLE testrole; GRANT REFRESH METADATA on TABLE db.tbl TO ROLE testrole; REVOKE REFRESH METADATA on SERVER svr FROM ROLE testrole; REVOKE REFRESH METADATA on DATABASE db FROM ROLE testrole; REVOKE REFRESH METADATA on TABLE db.tbl FROM ROLE testrole; Testing: - Ran front-end 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/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, 184 insertions(+), 45 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/9589/8 -- 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: 8 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya
[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Adam Holley has posted comments on this change. (
http://gerrit.cloudera.org:8080/9589 )
Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege
..
Patch Set 7:
(2 comments)
http://gerrit.cloudera.org:8080/#/c/9589/7/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/7/fe/src/main/java/org/apache/impala/authorization/Privilege.java@51
PS7, Line 51: SELECT("select"),
> I created a new enum that implements Sentry's Action to decouple Impala fro
Can you add a comment for this enum with the above? Maybe also include that we
can add privileges that are specific to Impala, i.e. refresh metadata.
http://gerrit.cloudera.org:8080/#/c/9589/7/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/7/fe/src/test/java/org/apache/impala/analysis/ParserTest.java@3578
PS7, Line 3578: // REFRESH METADATA privilege.
The tests right below this section indicate that server scope does not accept a
name. Did that change? Is the comment below incorrect? or should we add
ParserError tests if a server name is passed in?
--
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: 7
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Adam Holley
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Comment-Date: Tue, 13 Mar 2018 18:28:35 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-6643: Add REFRESH METADATA fine-grained privilege
Fredy Wijaya has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/9589 ) Change subject: IMPALA-6643: Add REFRESH METADATA fine-grained privilege .. IMPALA-6643: Add REFRESH METADATA fine-grained privilege With this patch, REFRESH METADATA 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 METADATA on SERVER svr TO ROLE testrole; GRANT REFRESH METADATA on DATABASE db TO ROLE testrole; GRANT REFRESH METADATA on TABLE db.tbl TO ROLE testrole; REVOKE REFRESH METADATA on SERVER svr FROM ROLE testrole; REVOKE REFRESH METADATA on DATABASE db FROM ROLE testrole; REVOKE REFRESH METADATA on TABLE db.tbl FROM ROLE testrole; Testing: - Ran front-end 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/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, 177 insertions(+), 43 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/89/9589/7 -- 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: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Adam Holley Gerrit-Reviewer: Fredy Wijaya
