[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 19: Verified+1 -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 19 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 22 Mar 2019 08:24:28 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has submitted this change and it was merged. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. IMPALA-7917 (Part 3): Decouple Sentry from Impala The third part of this patch introduces an interface called AuthorizationManager to perform authorization management related functions, such as granting/revoking a privilege, etc. Some existing authorization management methods have been refactored to reduce the need for if/else conditions to perform certain actions. This patch moves code related to Sentry authorization management code to: - SentryCatalogdAuthorizationManager - SentryImpaladAuthorizationManager The split makes it easier to differentiate between authorization DDL operations performed in Catalogd vs Impalad. This patch does not implement AuthorizationManager for Ranger. This patch has no functionality change. Testing: - Ran all FE tests - Ran all E2E authorization tests Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Reviewed-on: http://gerrit.cloudera.org:8080/12684 Reviewed-by: Impala Public Jenkins Tested-by: Impala Public Jenkins --- M fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationPolicy.java M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/catalog/Catalog.java M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A fe/src/main/java/org/apache/impala/util/ClassUtil.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java 23 files changed, 1,124 insertions(+), 430 deletions(-) Approvals: Impala Public Jenkins: Looks good to me, approved; Verified -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: merged Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 20 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 18: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2519/ : 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/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 18 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 22 Mar 2019 04:15:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 19: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3945/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 19 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 22 Mar 2019 03:33:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 19: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 19 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 22 Mar 2019 03:33:31 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 18: Code-Review+2 Apparently Iterables.getOnlyElement() will throw an IllegalArgumentException when the size is not 1. I'm reverting it to the old-style instead. -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 18 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 22 Mar 2019 03:33:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has uploaded a new patch set (#18). ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. IMPALA-7917 (Part 3): Decouple Sentry from Impala The third part of this patch introduces an interface called AuthorizationManager to perform authorization management related functions, such as granting/revoking a privilege, etc. Some existing authorization management methods have been refactored to reduce the need for if/else conditions to perform certain actions. This patch moves code related to Sentry authorization management code to: - SentryCatalogdAuthorizationManager - SentryImpaladAuthorizationManager The split makes it easier to differentiate between authorization DDL operations performed in Catalogd vs Impalad. This patch does not implement AuthorizationManager for Ranger. This patch has no functionality change. Testing: - Ran all FE tests - Ran all E2E authorization tests Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 --- M fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationPolicy.java M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/catalog/Catalog.java M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A fe/src/main/java/org/apache/impala/util/ClassUtil.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java 23 files changed, 1,124 insertions(+), 430 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/12684/18 -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 18 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 17: Verified-1 Build failed: https://jenkins.impala.io/job/gerrit-verify-dryrun/3940/ -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 17 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 22 Mar 2019 02:02:39 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 17: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2509/ : 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/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 17 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 21 Mar 2019 21:46:13 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 17: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3940/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 17 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 21 Mar 2019 21:14:32 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 17: Code-Review+2 Rebased. Carry Todd's +2. -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 17 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 21 Mar 2019 21:12:54 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has uploaded a new patch set (#17). ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. IMPALA-7917 (Part 3): Decouple Sentry from Impala The third part of this patch introduces an interface called AuthorizationManager to perform authorization management related functions, such as granting/revoking a privilege, etc. Some existing authorization management methods have been refactored to reduce the need for if/else conditions to perform certain actions. This patch moves code related to Sentry authorization management code to: - SentryCatalogdAuthorizationManager - SentryImpaladAuthorizationManager The split makes it easier to differentiate between authorization DDL operations performed in Catalogd vs Impalad. This patch does not implement AuthorizationManager for Ranger. This patch has no functionality change. Testing: - Ran all FE tests - Ran all E2E authorization tests Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 --- M fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationPolicy.java M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/catalog/Catalog.java M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A fe/src/main/java/org/apache/impala/util/ClassUtil.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java 23 files changed, 1,125 insertions(+), 430 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/12684/17 -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 17 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 16: Build started: https://jenkins.impala.io/job/gerrit-verify-dryrun/3937/ DRY_RUN=false -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 16 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 21 Mar 2019 05:40:12 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 16: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 16 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 21 Mar 2019 05:40:10 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 15: Code-Review+2 -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 15 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 21 Mar 2019 04:18:43 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 15: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2495/ : 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/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 15 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 21 Mar 2019 01:29:58 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 15: Code-Review+2 (1 comment) Carry Todd's +2. Will merge once https://gerrit.cloudera.org/c/12632/ is merged. http://gerrit.cloudera.org:8080/#/c/12684/14/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java File fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java: http://gerrit.cloudera.org:8080/#/c/12684/14/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java@130 PS14, Line 130: String roleName = Iterables.getOnlyElement(params.getRole_names()); > you didn't like my suggestion of using Iterables.getOnlyElement? That would Initially I thought it was something in the JDK. I just realized you meant the one from Guava. TIL :) Done. -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 15 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Thu, 21 Mar 2019 00:43:45 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has uploaded a new patch set (#15). ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. IMPALA-7917 (Part 3): Decouple Sentry from Impala The third part of this patch introduces an interface called AuthorizationManager to perform authorization management related functions, such as granting/revoking a privilege, etc. Some existing authorization management methods have been refactored to reduce the need for if/else conditions to perform certain actions. This patch moves code related to Sentry authorization management code to: - SentryCatalogdAuthorizationManager - SentryImpaladAuthorizationManager The split makes it easier to differentiate between authorization DDL operations performed in Catalogd vs Impalad. This patch does not implement AuthorizationManager for Ranger. This patch has no functionality change. Testing: - Ran all FE tests - Ran all E2E authorization tests Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 --- M fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationPolicy.java M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/catalog/Catalog.java M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A fe/src/main/java/org/apache/impala/util/ClassUtil.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java 23 files changed, 1,125 insertions(+), 430 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/12684/15 -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 15 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 14: Code-Review+2 (1 comment) http://gerrit.cloudera.org:8080/#/c/12684/14/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java File fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java: http://gerrit.cloudera.org:8080/#/c/12684/14/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java@130 PS14, Line 130: String groupName = params.getGroup_names().iterator().next(); you didn't like my suggestion of using Iterables.getOnlyElement? That would ensure there is exactly one group passed here and not more than one. -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 14 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 20 Mar 2019 23:36:10 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 14: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2482/ : 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/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 14 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 20 Mar 2019 17:04:53 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has uploaded a new patch set (#14). ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. IMPALA-7917 (Part 3): Decouple Sentry from Impala The third part of this patch introduces an interface called AuthorizationManager to perform authorization management related functions, such as granting/revoking a privilege, etc. Some existing authorization management methods have been refactored to reduce the need for if/else conditions to perform certain actions. This patch moves code related to Sentry authorization management code to: - SentryCatalogdAuthorizationManager - SentryImpaladAuthorizationManager The split makes it easier to differentiate between authorization DDL operations performed in Catalogd vs Impalad. This patch does not implement AuthorizationManager for Ranger. This patch has no functionality change. Testing: - Ran all FE tests - Ran all E2E authorization tests Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 --- M fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationPolicy.java M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/catalog/Catalog.java M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A fe/src/main/java/org/apache/impala/util/ClassUtil.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java 23 files changed, 1,125 insertions(+), 430 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/12684/14 -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 14 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 13: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2479/ : 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/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 13 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 20 Mar 2019 16:23:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 13: Rebased after https://gerrit.cloudera.org/c/12632 rebase. -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 13 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Wed, 20 Mar 2019 15:40:00 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has uploaded a new patch set (#13). ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. IMPALA-7917 (Part 3): Decouple Sentry from Impala The third part of this patch introduces an interface called AuthorizationManager to perform authorization management related functions, such as granting/revoking a privilege, etc. Some existing authorization management methods have been refactored to reduce the need for if/else conditions to perform certain actions. This patch moves code related to Sentry authorization management code to: - SentryCatalogdAuthorizationManager - SentryImpaladAuthorizationManager The split makes it easier to differentiate between authorization DDL operations performed in Catalogd vs Impalad. This patch does not implement AuthorizationManager for Ranger. This patch has no functionality change. Testing: - Ran all FE tests - Ran all E2E authorization tests Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 --- M fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationPolicy.java M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/catalog/Catalog.java M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A fe/src/main/java/org/apache/impala/util/ClassUtil.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java 23 files changed, 1,126 insertions(+), 430 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/12684/13 -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 13 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Austin Nobis has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 12: Code-Review+1 -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 12 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 15 Mar 2019 20:56:01 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 12: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2441/ : 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/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 12 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 15 Mar 2019 20:03:38 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 11: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2440/ : 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/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 11 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 15 Mar 2019 19:53:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 12: (1 comment) http://gerrit.cloudera.org:8080/#/c/12684/11/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java File fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java: http://gerrit.cloudera.org:8080/#/c/12684/11/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java@309 PS11, Line 309: updateOwnerPrivilege(oldOwner, oldOwnerType, newOwner, newOwnerType, response, > line too long (91 > 90) Done -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 12 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 15 Mar 2019 19:23:18 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 11: (1 comment) http://gerrit.cloudera.org:8080/#/c/12684/11/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java File fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java: http://gerrit.cloudera.org:8080/#/c/12684/11/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java@309 PS11, Line 309: updateOwnerPrivilege(oldOwner, oldOwnerType, newOwner, newOwnerType, response, filter); line too long (91 > 90) -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 11 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Fri, 15 Mar 2019 19:20:33 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has uploaded a new patch set (#12). ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. IMPALA-7917 (Part 3): Decouple Sentry from Impala The third part of this patch introduces an interface called AuthorizationManager to perform authorization management related functions, such as granting/revoking a privilege, etc. Some existing authorization management methods have been refactored to reduce the need for if/else conditions to perform certain actions. This patch moves code related to Sentry authorization management code to: - SentryCatalogdAuthorizationManager - SentryImpaladAuthorizationManager The split makes it easier to differentiate between authorization DDL operations performed in Catalogd vs Impalad. This patch does not implement AuthorizationManager for Ranger. This patch has no functionality change. Testing: - Ran all FE tests - Ran all E2E authorization tests Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 --- M fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationPolicy.java M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/catalog/Catalog.java M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A fe/src/main/java/org/apache/impala/util/ClassUtil.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java 23 files changed, 1,126 insertions(+), 430 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/12684/12 -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 12 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 11: (25 comments) http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java File fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java: http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java@66 PS9, Line 66: be > may *become* stale, right? Done http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java File fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java: http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java@41 PS9, Line 41:*/ > why is admin a special thing rather than just a separate privilege verified I may actually delete this method. It's just since we seem to hardcode the Sentry admin check. It's a quick hack so that others can start working on other parts of it. I will definitely revisit this again. I will put a TODO. http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java@56 PS9, Line 56: /** > typo Done http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java@85 PS9, Line 85: /** > can you clarify, this is all privileges granted to anyone anywhere? Or just Done http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java@93 PS9, Line 93:* case sensitive. > can you document any expected canonicalization of the names? should they be Done http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java@95 PS9, Line 95: void updateDatabaseOwnerPrivilege(String serverName, String databaseName, > I think it's worth expanding the javadoc slightly to explain why the _old_ Yup, I'm also not quite happy with this method. It doesn't feel too clean. There's a task to look at this whole object ownership feature in https://issues.apache.org/jira/browse/IMPALA-8228. http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java@100 PS9, Line 100:* Grants/revokes an owner privilege for the table, such as table creation, removal, > same Done http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java File fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java: http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java@77 PS9, Line 77: return false; > should these functions throw UnsupportedOperationException so that if someh Done http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java File fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java: http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java@52 PS9, Line 52: gd that uses Sentry > parsing this sentence is a bit difficult - I read this as "sentry for catal Done http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java@57 PS9, Line 57: Impalads. > I think just saying "broadcasted to all Impalads" is enough, since it's use Yup. Done. http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java@127 PS9, Line 127: verifySentryServiceEnabled(); > For this and other calls below, can we verify that getRole_names().size() = This is some of the old code. I agree it's not very good. Done. http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java@261 PS9, Line 261: > this would be a bug, right? perhaps we can call verifySentryServiceEnabled( Ah yeah. Done. http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java@263 PS9, Line 263: public void updateDatabaseOwnerPrivilege(String serverName, String databaseName, > if object ownership is not enabled, doesnt it seem like we should throw an Yes if we want to make object ownership enabled/disabled as part of a generic config. Sentry supports this for backward compatibility, but I don't think Ranger or any other authorization providers should. So a no-op is better
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has uploaded a new patch set (#11). ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. IMPALA-7917 (Part 3): Decouple Sentry from Impala The third part of this patch introduces an interface called AuthorizationManager to perform authorization management related functions, such as granting/revoking a privilege, etc. Some existing authorization management methods have been refactored to reduce the need for if/else conditions to perform certain actions. This patch moves code related to Sentry authorization management code to: - SentryCatalogdAuthorizationManager - SentryImpaladAuthorizationManager The split makes it easier to differentiate between authorization DDL operations performed in Catalogd vs Impalad. This patch does not implement AuthorizationManager for Ranger. This patch has no functionality change. Testing: - Ran all FE tests - Ran all E2E authorization tests Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 --- M fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationPolicy.java M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/catalog/Catalog.java M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A fe/src/main/java/org/apache/impala/util/ClassUtil.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java 23 files changed, 1,125 insertions(+), 430 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/12684/11 -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 11 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Todd Lipcon has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 9: (26 comments) http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java File fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java: http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java@66 PS9, Line 66: be may *become* stale, right? http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java File fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java: http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java@41 PS9, Line 41: boolean isAdmin(User user) throws ImpalaException; why is admin a special thing rather than just a separate privilege verified by the authorization checker interface? Just curious, doesn't need to change now. http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java@56 PS9, Line 56:* Gets all roles; typo http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java@85 PS9, Line 85:* Gets all privileges. can you clarify, this is all privileges granted to anyone anywhere? Or just on this server/service? http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java@93 PS9, Line 93: void updateDatabaseOwnerPrivilege(String serverName, String databaseName, can you document any expected canonicalization of the names? should they be lower cased? Are they case sensitive? http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java@95 PS9, Line 95: PrincipalType newOwnerType, TDdlExecResponse response) throws ImpalaException; I think it's worth expanding the javadoc slightly to explain why the _old_ values are passed in. Can multiple users be an owner of a database? http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java@100 PS9, Line 100:*/ same http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java File fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java: http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java@77 PS9, Line 77: public void createRole(User requestingUser, TCreateDropRoleParams params, should these functions throw UnsupportedOperationException so that if somehow it gets configured, it won't silently ignore requests? http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java File fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java: http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java@52 PS9, Line 52: Sentry for Catalogd parsing this sentence is a bit difficult - I read this as "sentry for catalogd". Maybe "for catalogd that uses Sentry" makes more sense? http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java@57 PS9, Line 57: Impalads for operations, such as SHOW ROLES and SHOW GRANT I think just saying "broadcasted to all Impalads" is enough, since it's used for anything that requires authorization metadata, right? http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java@127 PS9, Line 127: String roleName = params.getRole_names().get(0); For this and other calls below, can we verify that getRole_names().size() == 1 if we expect exactly one? (same with getGroup_names()) Can probably use Iterables.getOnlyElement(params.getRole_names()) http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java@261 PS9, Line 261: catalog_.getSentryProxy() == null || this would be a bug, right? perhaps we can call verifySentryServiceEnabled() here? http://gerrit.cloudera.org:8080/#/c/12684/9/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java@263 PS9, Line 263: return; if object ownership is not enabled, doesnt it seem like we should throw an error back to the user instead of no-opping this?
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 9: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2409/ : 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/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 9 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 12 Mar 2019 01:22:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has uploaded a new patch set (#9). ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. IMPALA-7917 (Part 3): Decouple Sentry from Impala The third part of this patch introduces an interface called AuthorizationManager to perform authorization management related functions, such as granting/revoking a privilege, etc. Some existing authorization management methods have been refactored to reduce the need for if/else conditions to perform certain actions. This patch moves code related to Sentry authorization management code to: - SentryCatalogdAuthorizationManager - SentryImpaladAuthorizationManager The split makes it easier to differentiate between authorization DDL operations performed in Catalogd vs Impalad. This patch does not implement AuthorizationManager for Ranger. This patch has no functionality change. Testing: - Ran all FE tests - Ran all E2E authorization tests Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 --- M fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationPolicy.java M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/catalog/Catalog.java M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A fe/src/main/java/org/apache/impala/util/ClassUtil.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java 23 files changed, 1,070 insertions(+), 425 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/12684/9 -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 9 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 9: Code-Review+1 Carry Bharath's +1. -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 9 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Austin Nobis Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Reviewer: Todd Lipcon Gerrit-Comment-Date: Tue, 12 Mar 2019 00:38:49 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 8: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2396/ : 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/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 8 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Fri, 08 Mar 2019 19:18:21 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 8: Code-Review+1 (3 comments) Carry Bharath's +1. Paul, can you take another look for the +2? http://gerrit.cloudera.org:8080/#/c/12684/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java File fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java: http://gerrit.cloudera.org:8080/#/c/12684/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java@76 PS7, Line 76: > nit: \n Done http://gerrit.cloudera.org:8080/#/c/12684/7/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java File fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java: http://gerrit.cloudera.org:8080/#/c/12684/7/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java@52 PS7, Line 52: uses Sentry for Catalogd. > I think this is not super clear. Mind elaborating this a little bit? Done http://gerrit.cloudera.org:8080/#/c/12684/7/fe/src/main/java/org/apache/impala/util/ClassUtil.java File fe/src/main/java/org/apache/impala/util/ClassUtil.java: http://gerrit.cloudera.org:8080/#/c/12684/7/fe/src/main/java/org/apache/impala/util/ClassUtil.java@25 PS7, Line 25: StackTraceElement stackTrace = Thread.currentThread().getStackTrace()[2]; > Do we need to worry about the stack length? No, first element is always getStackTrace(), second element is always getMethodName(). Third is the actual method that invokes this method. -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 8 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Fri, 08 Mar 2019 18:36:26 + Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has uploaded a new patch set (#8). ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. IMPALA-7917 (Part 3): Decouple Sentry from Impala The third part of this patch introduces an interface called AuthorizationManager to perform authorization management related functions, such as granting/revoking a privilege, etc. Some existing authorization management methods have been refactored to reduce the need for if/else conditions to perform certain actions. This patch moves code related to Sentry authorization management code to: - SentryCatalogdAuthorizationManager - SentryImpaladAuthorizationManager The split makes it easier to differentiate between authorization DDL operations performed in Catalogd vs Impalad. This patch does not implement AuthorizationManager for Ranger. This patch has no functionality change. Testing: - Ran all FE tests - Ran all E2E authorization tests Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 --- M fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationPolicy.java M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/catalog/Catalog.java M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A fe/src/main/java/org/apache/impala/util/ClassUtil.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java 23 files changed, 1,056 insertions(+), 425 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/12684/8 -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 8 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/12684 )
Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala
..
Patch Set 7: Code-Review+1
(4 comments)
I'll let Paul take another pass as the class structure changed since he +1ed it
the last time.
http://gerrit.cloudera.org:8080/#/c/12684/5/fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java:
http://gerrit.cloudera.org:8080/#/c/12684/5/fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java@62
PS5, Line 62:*/
: AuthorizationManager newAuthorizationManager(FeCatalogManager
catalog,
: AuthorizationChecker authzChecker);
:
: /**
:* Creates a new instance of {@link AuthorizationManager}.
:*/
> I like the idea of splitting between SentryCatalogdAuthzManager and SentryI
I think that makes sense.
http://gerrit.cloudera.org:8080/#/c/12684/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java
File
fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java:
http://gerrit.cloudera.org:8080/#/c/12684/7/fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java@76
PS7, Line 76: }
nit: \n
http://gerrit.cloudera.org:8080/#/c/12684/7/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java
File
fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java:
http://gerrit.cloudera.org:8080/#/c/12684/7/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java@52
PS7, Line 52: uses Sentry for Catalogd.
I think this is not super clear. Mind elaborating this a little bit?
http://gerrit.cloudera.org:8080/#/c/12684/7/fe/src/main/java/org/apache/impala/util/ClassUtil.java
File fe/src/main/java/org/apache/impala/util/ClassUtil.java:
http://gerrit.cloudera.org:8080/#/c/12684/7/fe/src/main/java/org/apache/impala/util/ClassUtil.java@25
PS7, Line 25: StackTraceElement stackTrace =
Thread.currentThread().getStackTrace()[2];
Do we need to worry about the stack length?
--
To view, visit http://gerrit.cloudera.org:8080/12684
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0
Gerrit-Change-Number: 12684
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Comment-Date: Fri, 08 Mar 2019 17:56:41 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 7: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2392/ : 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/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Fri, 08 Mar 2019 10:16:35 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has posted comments on this change. (
http://gerrit.cloudera.org:8080/12684 )
Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala
..
Patch Set 7:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12684/5/fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java:
http://gerrit.cloudera.org:8080/#/c/12684/5/fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java@62
PS5, Line 62:*/
: AuthorizationManager newAuthorizationManager(FeCatalogManager
catalog,
: AuthorizationChecker authzChecker);
:
: /**
:* Creates a new instance of {@link AuthorizationManager}.
:*/
> I have a feeling that this AuthzMgr should be tied to a Catalog and not whe
I like the idea of splitting between SentryCatalogdAuthzManager and
SentryImpaladAuthzManager. However I don't think taking Catalog is going to be
good enough. FeCatalog is another type of Catalog that doesn't actually
implement Catalog. Taking FeCatalog is also not a bad idea since depending on
the implementation (for example the CatalogdImpl), the catalog instance can
change, so we need to use the FeCatalogManager instead.
So, I'm thinking of having two methods, one that takes FeCatalogManager and the
other one that takes CatalogServiceCatalog. This way the intention is clear one
is for impalad and the other one is for catalogd.
Let me know what you think.
--
To view, visit http://gerrit.cloudera.org:8080/12684
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0
Gerrit-Change-Number: 12684
Gerrit-PatchSet: 7
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Comment-Date: Fri, 08 Mar 2019 09:31:20 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has uploaded a new patch set (#7). ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. IMPALA-7917 (Part 3): Decouple Sentry from Impala The third part of this patch introduces an interface called AuthorizationManager to perform authorization management related functions, such as granting/revoking a privilege, etc. Some existing authorization management methods have been refactored to reduce the need for if/else conditions to perform certain actions. This patch moves code related to Sentry authorization management code to: - SentryCatalogdAuthorizationManager - SentryImpaladAuthorizationManager The split makes it easier to differentiate between authorization DDL operations performed in Catalogd vs Impalad. This patch does not implement AuthorizationManager for Ranger. This patch has no functionality change. Testing: - Ran all FE tests - Ran all E2E authorization tests Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 --- M fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/AuthorizationPolicy.java M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthProvider.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationChecker.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationPolicy.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/catalog/Catalog.java M fe/src/main/java/org/apache/impala/catalog/local/CatalogdMetaProvider.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/FeCatalogManager.java M fe/src/main/java/org/apache/impala/service/Frontend.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java A fe/src/main/java/org/apache/impala/util/ClassUtil.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java 23 files changed, 1,044 insertions(+), 425 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/12684/7 -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 7 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Bharath Vissapragada has posted comments on this change. (
http://gerrit.cloudera.org:8080/12684 )
Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala
..
Patch Set 5:
(1 comment)
http://gerrit.cloudera.org:8080/#/c/12684/5/fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java
File fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java:
http://gerrit.cloudera.org:8080/#/c/12684/5/fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java@62
PS5, Line 62:*/
: AuthorizationManager newAuthorizationManager(CatalogOpExecutor
catalogOpExecutor);
:
: /**
:* Creates a new instance of {@link AuthorizationManager}.
:*/
: AuthorizationManager newAuthorizationManager(Frontend frontend);
I have a feeling that this AuthzMgr should be tied to a Catalog and not whether
it is frontend or CatalogServer.
newAuthzMgr(Catalog catalog) {}
You can then have children like SentryCatalogdAuthzMgr() and
SentryImpaladAuthzMgr().
For ranger I don't think you need a Catalogd side authz manager at all.
Essentially you could be delegating the impl stuff to the authz engine.
The issue as I see with the current model is that you need to know what methods
you can call, depending on the context you are in.
For ex: if I'm on a coordinator, I should know that I can only call getRoles,
showRoles() etc and not addRole / createRole().
What do you think?
--
To view, visit http://gerrit.cloudera.org:8080/12684
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-MessageType: comment
Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0
Gerrit-Change-Number: 12684
Gerrit-PatchSet: 5
Gerrit-Owner: Fredy Wijaya
Gerrit-Reviewer: Bharath Vissapragada
Gerrit-Reviewer: Fredy Wijaya
Gerrit-Reviewer: Impala Public Jenkins
Gerrit-Reviewer: Paul Rogers
Gerrit-Comment-Date: Thu, 07 Mar 2019 23:12:59 +
Gerrit-HasComments: Yes
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Paul Rogers has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 5: Code-Review+1 Code looks good. Would like Bharath to double-check the logic flow. -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Thu, 07 Mar 2019 19:52:33 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 5: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2385/ : 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/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Thu, 07 Mar 2019 19:16:17 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 5: Rebased to fix conflicts. -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Thu, 07 Mar 2019 18:33:22 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has uploaded a new patch set (#5). ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. IMPALA-7917 (Part 3): Decouple Sentry from Impala The third part of this patch introduces an interface called AuthorizationManager to perform authorization management related functions, such as granting/revoking a privilege, etc. Some existing authorization management methods have been refactored to reduce the need for if/else conditions to perform certain actions. This patch moves code related to Sentry authorization management code to SentryAuthorizationManager. This patch does not implement AuthorizationManager for Ranger. This patch has no functionality change. Testing: - Ran all FE tests - Ran all E2E authorization tests Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 --- M fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationFactory.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java 13 files changed, 864 insertions(+), 394 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/12684/5 -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newpatchset Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 5 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Impala Public Jenkins has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 4: Build Successful https://jenkins.impala.io/job/gerrit-code-review-checks/2383/ : 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/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Impala Public Jenkins Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Thu, 07 Mar 2019 07:32:34 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has posted comments on this change. ( http://gerrit.cloudera.org:8080/12684 ) Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. Patch Set 4: This should be the last part of Sentry decoupling. -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya Gerrit-Reviewer: Bharath Vissapragada Gerrit-Reviewer: Fredy Wijaya Gerrit-Reviewer: Paul Rogers Gerrit-Comment-Date: Thu, 07 Mar 2019 06:48:37 + Gerrit-HasComments: No
[Impala-ASF-CR] IMPALA-7917 (Part 3): Decouple Sentry from Impala
Fredy Wijaya has uploaded this change for review. ( http://gerrit.cloudera.org:8080/12684 Change subject: IMPALA-7917 (Part 3): Decouple Sentry from Impala .. IMPALA-7917 (Part 3): Decouple Sentry from Impala The third part of this patch introduces an interface called AuthorizationManager to perform authorization management related functions, such as granting/revoking a privilege, etc. Some existing authorization management methods have been refactored to reduce the need for if/else conditions to perform certain actions. This patch moves code related to Sentry authorization management code to SentryAuthorizationManager. This patch does not implement AuthorizationManager for Ranger. This patch has no functionality change. Testing: - Ran all FE tests - Ran all E2E authorization tests Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 --- M fe/src/main/java/org/apache/impala/authorization/AuthorizationFactory.java A fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/ranger/RangerAuthorizationFactory.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationFactory.java A fe/src/main/java/org/apache/impala/authorization/sentry/SentryAuthorizationManager.java M fe/src/main/java/org/apache/impala/authorization/sentry/SentryProxy.java M fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java M fe/src/main/java/org/apache/impala/service/JniCatalog.java M fe/src/main/java/org/apache/impala/service/JniFrontend.java M fe/src/test/java/org/apache/impala/catalog/events/MetastoreEventsProcessorTest.java M fe/src/test/java/org/apache/impala/common/FrontendTestBase.java M fe/src/test/java/org/apache/impala/testutil/PlannerTestCaseLoader.java 13 files changed, 864 insertions(+), 395 deletions(-) git pull ssh://gerrit.cloudera.org:29418/Impala-ASF refs/changes/84/12684/4 -- To view, visit http://gerrit.cloudera.org:8080/12684 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: newchange Gerrit-Change-Id: I1a5d3e0a3e86ac2b0329b56247357fca93229dd0 Gerrit-Change-Number: 12684 Gerrit-PatchSet: 4 Gerrit-Owner: Fredy Wijaya
