You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by jo...@apache.org on 2019/04/05 15:51:38 UTC
[impala] branch master updated (d31ac7b -> 80b82b5)
This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git.
from d31ac7b IMPALA-8377: bump toolchain version to 107-acaeac961d
new 17ed50b IMPALA-8226: Add grant/revoke to/from group for Ranger
new f3f7774 IMPALA-6216: Make PYTHON_EGG_CACHE configurable
new 80b82b5 IMPALA-8360: Fix race conditions in thread-pool-test
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
be/src/util/thread-pool-test.cc | 17 +-
common/thrift/CatalogObjects.thrift | 4 +-
fe/src/main/cup/sql-parser.cup | 20 +-
.../impala/authorization/AuthorizationManager.java | 12 +
.../authorization/NoneAuthorizationFactory.java | 14 +
.../ranger/RangerCatalogdAuthorizationManager.java | 68 +++--
.../sentry/SentryCatalogdAuthorizationManager.java | 14 +
.../sentry/SentryImpaladAuthorizationManager.java | 14 +
.../apache/impala/service/CatalogOpExecutor.java | 7 +
.../impala/analysis/AnalyzeAuthStmtsTest.java | 289 +++++++++++----------
.../impala/analysis/AuthorizationStmtTest.java | 60 +++--
.../org/apache/impala/analysis/ParserTest.java | 222 ++++++++--------
.../java/org/apache/impala/analysis/ToSqlTest.java | 2 +-
fe/src/test/resources/ranger-hive-security.xml | 2 +-
shell/impala-shell | 8 +-
testdata/bin/create-load-data.sh | 11 +
.../ranger/setup/impala_group.json.template | 4 +
.../cluster/ranger/setup/impala_user.json.template | 5 +-
tests/authorization/test_ranger.py | 60 +++--
19 files changed, 494 insertions(+), 339 deletions(-)
create mode 100644 testdata/cluster/ranger/setup/impala_group.json.template
[impala] 02/03: IMPALA-6216: Make PYTHON_EGG_CACHE configurable
Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit f3f7774d1a6dc41bd3d3a347bec5909d2ec4910f
Author: Yongzhi Chen <yc...@cloudera.com>
AuthorDate: Tue Apr 2 09:40:23 2019 -0400
IMPALA-6216: Make PYTHON_EGG_CACHE configurable
User can set environment variable PYTHON_EGG_CACHE before call
impala-shell.
Testing:
Run impala-shell with or w/o PYTHON_EGG_CACHE configured
Change-Id: I695b2b31d9045eef1a53268f6516858935aed508
Reviewed-on: http://gerrit.cloudera.org:8080/12911
Tested-by: Impala Public Jenkins <im...@cloudera.com>
Reviewed-by: Bharath Vissapragada <bh...@cloudera.com>
---
shell/impala-shell | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/shell/impala-shell b/shell/impala-shell
index 9e17c26..48dcf60 100755
--- a/shell/impala-shell
+++ b/shell/impala-shell
@@ -35,9 +35,11 @@ SHELL_HOME=${IMPALA_SHELL_HOME:-${SCRIPT_DIR}}
# Set the envrionment's locale settings to allow for utf-8 compatibility
export LC_CTYPE=${LC_CTYPE:-en_US.UTF-8}
-# We should set the EGG_CACHE to a per-user temporary location.
-# This follows what hue does.
-PYTHON_EGG_CACHE=/tmp/impala-shell-python-egg-cache-${USER}
+# User can configure EGG_CACHE by setting PYTHON_EGG_CACHE.
+# By default it is set to a per-user temporary location,
+# which follows what hue does.
+PYTHON_EGG_CACHE=${PYTHON_EGG_CACHE:-/tmp/impala-shell-python-egg-cache-${USER}}
+
if [ ! -d ${PYTHON_EGG_CACHE} ]; then
mkdir ${PYTHON_EGG_CACHE}
fi
[impala] 03/03: IMPALA-8360: Fix race conditions in thread-pool-test
Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 80b82b5b2c60ff934eabe42dd145c9f84ca3a60b
Author: Joe McDonnell <jo...@cloudera.com>
AuthorDate: Thu Mar 28 15:38:23 2019 -0700
IMPALA-8360: Fix race conditions in thread-pool-test
There are race conditions in thread-pool-test between the caller
thread and the worker thread. Specifically, in some cases, the
worker thread seems to be slow in freeing resources. This can
lead to asserts failing because the work item has not been
freed or because the submit of a task failed when it should
not have.
This fixes the race conditions:
- It breaks up the existing SynchronousThreadPoolTest into two
smaller tests so that the two can't interfere with each other.
- It adds the ability to sleep and recheck that the work item
has been freed.
Testing:
- Ran thread-pool-test in a loop for 20k iterations
Change-Id: Id2d5f8b677e475d8e9d6b4512e990b20bfbefaf1
Reviewed-on: http://gerrit.cloudera.org:8080/12916
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
be/src/util/thread-pool-test.cc | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/be/src/util/thread-pool-test.cc b/be/src/util/thread-pool-test.cc
index e02c5a2..3633ae9 100644
--- a/be/src/util/thread-pool-test.cc
+++ b/be/src/util/thread-pool-test.cc
@@ -95,7 +95,7 @@ private:
bool* destructor_called_ptr_;
};
-TEST(ThreadPoolTest, SynchronousThreadPoolTest) {
+TEST(ThreadPoolTest, SynchronousThreadPoolNoSleep) {
// Create a synchronous pool with one thread and a queue size of one.
SynchronousThreadPool pool("sync-thread-pool", "worker", 1, 1);
ASSERT_OK(pool.Init());
@@ -109,7 +109,22 @@ TEST(ThreadPoolTest, SynchronousThreadPoolTest) {
// shared_ptr to the work item. The caller is the only holder, so when it calls
// reset, the destructor must be called.
no_sleep.reset();
+ // The work item should be destroyed even if we are not shutting down the pool.
+ // IMPALA-8371: There is a race condition with the worker thread, as the worker thread
+ // may not have released its shared_ptr to the work item. Wait for a limited period of
+ // time for the work thread to release the shared_ptr.
+ for (int i = 0; i < 10; i++) {
+ if (*no_sleep_destroyed) break;
+ SleepForMs(5);
+ }
ASSERT_TRUE(*no_sleep_destroyed);
+ pool.DrainAndShutdown();
+}
+
+TEST(ThreadPoolTest, SynchronousThreadPoolTimeouts) {
+ // Create a synchronous pool with one thread and a queue size of one.
+ SynchronousThreadPool pool("sync-thread-pool", "worker", 1, 1);
+ ASSERT_OK(pool.Init());
// Timeout case #1: Submit one task that takes 100 milliseconds. Offer it with a timeout
// of 1 millisecond so that the caller immediately times out.
[impala] 01/03: IMPALA-8226: Add grant/revoke to/from group for
Ranger
Posted by jo...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
joemcdonnell pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit 17ed50b5ce37f64990c83df97ab73b1e653f138b
Author: Austin Nobis <an...@cloudera.com>
AuthorDate: Mon Apr 1 14:51:48 2019 -0700
IMPALA-8226: Add grant/revoke to/from group for Ranger
This patch adds fupport for GRANT privilege statements to GROUP and
REVOKE privilege statements from GROUP. The grammar has been updated to
support FROM GROUP and TO GROUP for GRANT/REVOKE statements, i.e:
GRANT <privilege> ON <resource> TO GROUP <group>
REVOKE <privilege> ON <resource> FROM GROUP <group>
Currently, only Ranger's authorization implementation supports GROUP
based privileges. Sentry will throw an UnsupportedOperationException if
it is the enabled authorization provider and this new grammar is used.
Testing:
- AuthorizationStmtTest was updated to also test for GROUP
authorization.
- ToSqlTest was updated to test for GROUP changes to the grammar.
- A GROUP based E2E test was added to test_ranger.py
- ParserTest was updated to test combinations for GrantRevokePrivilege
- AnalyzeAuthStmtsTest was updated to test for USER and GROUP identities
- Ran all FE tests
- Ran authorization E2E tests
Change-Id: I28b7b3e4c776ad1bb5bdc184c7d733d0b5ef5e96
Reviewed-on: http://gerrit.cloudera.org:8080/12914
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
common/thrift/CatalogObjects.thrift | 4 +-
fe/src/main/cup/sql-parser.cup | 20 +-
.../impala/authorization/AuthorizationManager.java | 12 +
.../authorization/NoneAuthorizationFactory.java | 14 +
.../ranger/RangerCatalogdAuthorizationManager.java | 68 +++--
.../sentry/SentryCatalogdAuthorizationManager.java | 14 +
.../sentry/SentryImpaladAuthorizationManager.java | 14 +
.../apache/impala/service/CatalogOpExecutor.java | 7 +
.../impala/analysis/AnalyzeAuthStmtsTest.java | 289 +++++++++++----------
.../impala/analysis/AuthorizationStmtTest.java | 60 +++--
.../org/apache/impala/analysis/ParserTest.java | 222 ++++++++--------
.../java/org/apache/impala/analysis/ToSqlTest.java | 2 +-
fe/src/test/resources/ranger-hive-security.xml | 2 +-
testdata/bin/create-load-data.sh | 11 +
.../ranger/setup/impala_group.json.template | 4 +
.../cluster/ranger/setup/impala_user.json.template | 5 +-
tests/authorization/test_ranger.py | 60 +++--
17 files changed, 473 insertions(+), 335 deletions(-)
diff --git a/common/thrift/CatalogObjects.thrift b/common/thrift/CatalogObjects.thrift
index 0388e64..f8cb8d5 100644
--- a/common/thrift/CatalogObjects.thrift
+++ b/common/thrift/CatalogObjects.thrift
@@ -482,11 +482,11 @@ struct TDatabase {
2: optional hive_metastore.Database metastore_db
}
-// Represents a principal type that maps to Sentry principal type.
-// https://github.com/apache/sentry/blob/3d062f39ce6a047138660a7b3d0024bde916c5b4/sentry-service/sentry-service-api/src/gen/thrift/gen-javabean/org/apache/sentry/api/service/thrift/TSentryPrincipalType.java
+// Represents a type of principal.
enum TPrincipalType {
ROLE = 0
USER = 1
+ GROUP = 2
}
// Represents a principal in an authorization policy.
diff --git a/fe/src/main/cup/sql-parser.cup b/fe/src/main/cup/sql-parser.cup
index ff8f892..fd19b8e 100644
--- a/fe/src/main/cup/sql-parser.cup
+++ b/fe/src/main/cup/sql-parser.cup
@@ -520,7 +520,6 @@ nonterminal PrivilegeSpec privilege_spec;
nonterminal TPrivilegeLevel privilege;
nonterminal Boolean opt_with_grantopt;
nonterminal Boolean opt_grantopt_for;
-nonterminal Boolean opt_kw_role;
// To avoid creating common keywords such as 'SERVER' or 'SOURCES' we treat them as
// identifiers rather than keywords. Throws a parse exception if the identifier does not
@@ -995,12 +994,15 @@ grant_privilege_stmt ::=
| KW_GRANT privilege_spec:priv KW_TO ident_or_default:role
opt_with_grantopt:grant_opt
{: RESULT = new GrantRevokePrivStmt(role, priv, true, grant_opt, TPrincipalType.ROLE); :}
- | KW_GRANT privilege_spec:priv KW_TO IDENT:user_id ident_or_default:role
+ | KW_GRANT privilege_spec:priv KW_TO IDENT:user_id ident_or_default:user
opt_with_grantopt:grant_opt
{:
parser.checkIdentKeyword("USER", user_id);
- RESULT = new GrantRevokePrivStmt(role, priv, true, grant_opt, TPrincipalType.USER);
+ RESULT = new GrantRevokePrivStmt(user, priv, true, grant_opt, TPrincipalType.USER);
:}
+ | KW_GRANT privilege_spec:priv KW_TO KW_GROUP ident_or_default:group
+ opt_with_grantopt:grant_opt
+ {: RESULT = new GrantRevokePrivStmt(group, priv, true, grant_opt, TPrincipalType.GROUP); :}
;
// For backwards compatibility, a revoke without the principal type will default to
@@ -1013,11 +1015,14 @@ revoke_privilege_stmt ::=
ident_or_default:role
{: RESULT = new GrantRevokePrivStmt(role, priv, false, grant_opt, TPrincipalType.ROLE); :}
| KW_REVOKE opt_grantopt_for:grant_opt privilege_spec:priv KW_FROM
- IDENT:user_id ident_or_default:role
+ IDENT:user_id ident_or_default:user
{:
parser.checkIdentKeyword("USER", user_id);
- RESULT = new GrantRevokePrivStmt(role, priv, false, grant_opt, TPrincipalType.USER);
+ RESULT = new GrantRevokePrivStmt(user, priv, false, grant_opt, TPrincipalType.USER);
:}
+ | KW_REVOKE opt_grantopt_for:grant_opt privilege_spec:priv KW_FROM
+ KW_GROUP ident_or_default:group
+ {: RESULT = new GrantRevokePrivStmt(group, priv, false, grant_opt, TPrincipalType.GROUP); :}
;
privilege_spec ::=
@@ -1076,11 +1081,6 @@ opt_with_grantopt ::=
{: RESULT = false; :}
;
-opt_kw_role ::=
- KW_ROLE
- | /* empty */
- ;
-
partition_def ::=
partition_spec:partition location_val:location opt_cache_op_val:cache_op
{: RESULT = new PartitionDef(partition, location, cache_op); :}
diff --git a/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java b/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java
index 47d7dea..0f42345 100644
--- a/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java
+++ b/fe/src/main/java/org/apache/impala/authorization/AuthorizationManager.java
@@ -95,6 +95,18 @@ public interface AuthorizationManager {
TDdlExecResponse response) throws ImpalaException;
/**
+ * Grants a privilege to a group.
+ */
+ void grantPrivilegeToGroup(User requestingUser, TGrantRevokePrivParams params,
+ TDdlExecResponse response) throws ImpalaException;
+
+ /**
+ * Revokes a privilege from a group.
+ */
+ void revokePrivilegeFromGroup(User requestingUser, TGrantRevokePrivParams params,
+ TDdlExecResponse response) throws ImpalaException;
+
+ /**
* Gets all privileges granted to the given principal.
*/
TResultSet getPrivileges(TShowGrantPrincipalParams params) throws ImpalaException;
diff --git a/fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java b/fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java
index 798dc07..f21752c 100644
--- a/fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java
+++ b/fe/src/main/java/org/apache/impala/authorization/NoneAuthorizationFactory.java
@@ -139,6 +139,20 @@ public class NoneAuthorizationFactory implements AuthorizationFactory {
}
@Override
+ public void grantPrivilegeToGroup(User requestingUser, TGrantRevokePrivParams params,
+ TDdlExecResponse response) throws ImpalaException {
+ throw new UnsupportedOperationException(String.format("%s is not supported",
+ ClassUtil.getMethodName()));
+ }
+
+ @Override
+ public void revokePrivilegeFromGroup(User requestingUser,
+ TGrantRevokePrivParams params, TDdlExecResponse response) throws ImpalaException {
+ throw new UnsupportedOperationException(String.format("%s is not supported",
+ ClassUtil.getMethodName()));
+ }
+
+ @Override
public TResultSet getPrivileges(TShowGrantPrincipalParams params)
throws ImpalaException {
throw new UnsupportedOperationException(String.format("%s is not supported",
diff --git a/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java b/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
index 9e2eb88..cedd1e8 100644
--- a/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
+++ b/fe/src/main/java/org/apache/impala/authorization/ranger/RangerCatalogdAuthorizationManager.java
@@ -18,10 +18,7 @@
package org.apache.impala.authorization.ranger;
import com.google.common.annotations.VisibleForTesting;
-import com.google.common.base.Preconditions;
import org.apache.hadoop.hive.metastore.api.PrincipalType;
-import org.apache.impala.authorization.AuthorizationChecker;
-import org.apache.impala.authorization.AuthorizationConfig;
import org.apache.impala.authorization.AuthorizationManager;
import org.apache.impala.authorization.User;
import org.apache.impala.common.ImpalaException;
@@ -32,10 +29,10 @@ import org.apache.ranger.plugin.audit.RangerDefaultAuditHandler;
import org.apache.ranger.plugin.util.GrantRevokeRequest;
import java.util.ArrayList;
+import java.util.Collections;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
-import java.util.concurrent.atomic.AtomicReference;
import java.util.function.Function;
import java.util.function.Supplier;
@@ -113,38 +110,57 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager
public void grantPrivilegeToUser(User requestingUser, TGrantRevokePrivParams params,
TDdlExecResponse response) throws ImpalaException {
List<GrantRevokeRequest> requests = createGrantRevokeRequests(
- requestingUser.getName(), params.getPrincipal_name(),
+ requestingUser.getName(), params.getPrincipal_name(), Collections.emptyList(),
plugin_.get().getClusterName(), params.getPrivileges());
- grantPrivilegeToUser(requestingUser, requests, response);
+ grantPrivilege(requests);
}
- @VisibleForTesting
- public void grantPrivilegeToUser(User requestingUser, List<GrantRevokeRequest> requests,
+ @Override
+ public void revokePrivilegeFromUser(User requestingUser, TGrantRevokePrivParams params,
TDdlExecResponse response) throws ImpalaException {
- try {
- for (GrantRevokeRequest request : requests) {
- plugin_.get().grantAccess(request, auditHandler_);
- }
- } catch (Exception e) {
- throw new InternalException(e.getMessage());
- }
+ List<GrantRevokeRequest> requests = createGrantRevokeRequests(
+ requestingUser.getName(), params.getPrincipal_name(), Collections.emptyList(),
+ plugin_.get().getClusterName(), params.getPrivileges());
+
+ revokePrivilege(requests);
}
@Override
- public void revokePrivilegeFromUser(User requestingUser, TGrantRevokePrivParams params,
+ public void grantPrivilegeToGroup(User requestingUser, TGrantRevokePrivParams params,
+ TDdlExecResponse response) throws ImpalaException {
+ List<GrantRevokeRequest> requests = createGrantRevokeRequests(
+ requestingUser.getName(), null,
+ Collections.singletonList(params.getPrincipal_name()),
+ plugin_.get().getClusterName(), params.getPrivileges());
+
+ grantPrivilege(requests);
+ }
+
+ @Override
+ public void revokePrivilegeFromGroup(User requestingUser, TGrantRevokePrivParams params,
TDdlExecResponse response) throws ImpalaException {
List<GrantRevokeRequest> requests = createGrantRevokeRequests(
- requestingUser.getName(), params.getPrincipal_name(),
+ requestingUser.getName(), null,
+ Collections.singletonList(params.getPrincipal_name()),
plugin_.get().getClusterName(), params.getPrivileges());
- revokePrivilegeFromUser(requestingUser, requests, response);
+ revokePrivilege(requests);
}
@VisibleForTesting
- public void revokePrivilegeFromUser(User requestingUser,
- List<GrantRevokeRequest> requests, TDdlExecResponse response)
- throws ImpalaException {
+ public void grantPrivilege(List<GrantRevokeRequest> requests) throws ImpalaException {
+ try {
+ for (GrantRevokeRequest request : requests) {
+ plugin_.get().grantAccess(request, auditHandler_);
+ }
+ } catch (Exception e) {
+ throw new InternalException(e.getMessage());
+ }
+ }
+
+ @VisibleForTesting
+ public void revokePrivilege(List<GrantRevokeRequest> requests) throws ImpalaException {
try {
for (GrantRevokeRequest request : requests) {
plugin_.get().revokeAccess(request, auditHandler_);
@@ -174,12 +190,12 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager
}
public static List<GrantRevokeRequest> createGrantRevokeRequests(String grantor,
- String user, String clusterName, List<TPrivilege> privileges) {
+ String user, List<String> groups, String clusterName, List<TPrivilege> privileges) {
List<GrantRevokeRequest> requests = new ArrayList<>();
for (TPrivilege p: privileges) {
Function<Map<String, String>, GrantRevokeRequest> createRequest = (resource) ->
- createGrantRevokeRequest(grantor, user, clusterName, p.privilege_level,
+ createGrantRevokeRequest(grantor, user, groups, clusterName, p.privilege_level,
resource);
// Ranger Impala service definition defines 3 resources:
@@ -209,10 +225,12 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager
}
private static GrantRevokeRequest createGrantRevokeRequest(String grantor, String user,
- String clusterName, TPrivilegeLevel level, Map<String, String> resource) {
+ List<String> groups, String clusterName, TPrivilegeLevel level,
+ Map<String, String> resource) {
GrantRevokeRequest request = new GrantRevokeRequest();
request.setGrantor(grantor);
- request.getUsers().add(user);
+ if (user != null) request.getUsers().add(user);
+ if (!groups.isEmpty()) request.getGroups().addAll(groups);
request.setDelegateAdmin(Boolean.FALSE);
request.setEnableAudit(Boolean.TRUE);
request.setReplaceExistingPermissions(Boolean.FALSE);
diff --git a/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java
index 9926fd2..872dc63 100644
--- a/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryCatalogdAuthorizationManager.java
@@ -274,6 +274,20 @@ public class SentryCatalogdAuthorizationManager implements AuthorizationManager
}
@Override
+ public void grantPrivilegeToGroup(User requestingUser, TGrantRevokePrivParams params,
+ TDdlExecResponse response) throws ImpalaException {
+ throw new UnsupportedOperationException(String.format(
+ "%s is not supported in Catalogd", ClassUtil.getMethodName()));
+ }
+
+ @Override
+ public void revokePrivilegeFromGroup(User requestingUser, TGrantRevokePrivParams params,
+ TDdlExecResponse response) throws ImpalaException {
+ throw new UnsupportedOperationException(String.format(
+ "%s is not supported in Catalogd", ClassUtil.getMethodName()));
+ }
+
+ @Override
public TResultSet getPrivileges(TShowGrantPrincipalParams params)
throws ImpalaException {
throw new UnsupportedOperationException(String.format(
diff --git a/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java
index 7cde79d..73cc11c 100644
--- a/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java
+++ b/fe/src/main/java/org/apache/impala/authorization/sentry/SentryImpaladAuthorizationManager.java
@@ -164,6 +164,20 @@ public class SentryImpaladAuthorizationManager implements AuthorizationManager {
}
@Override
+ public void grantPrivilegeToGroup(User requestingUser, TGrantRevokePrivParams params,
+ TDdlExecResponse response) throws ImpalaException {
+ throw new UnsupportedOperationException(String.format(
+ "%s is not supported in Impalad", ClassUtil.getMethodName()));
+ }
+
+ @Override
+ public void revokePrivilegeFromGroup(User requestingUser, TGrantRevokePrivParams params,
+ TDdlExecResponse response) throws ImpalaException {
+ throw new UnsupportedOperationException(String.format(
+ "%s is not supported in Impalad", ClassUtil.getMethodName()));
+ }
+
+ @Override
public TResultSet getPrivileges(TShowGrantPrincipalParams params)
throws ImpalaException {
switch (params.getPrincipal_type()) {
diff --git a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
index 9b1e7ee..e0c5fe9 100644
--- a/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
+++ b/fe/src/main/java/org/apache/impala/service/CatalogOpExecutor.java
@@ -3262,6 +3262,9 @@ public class CatalogOpExecutor {
case USER:
authzManager_.grantPrivilegeToUser(requestingUser, grantRevokePrivParams, resp);
break;
+ case GROUP:
+ authzManager_.grantPrivilegeToGroup(requestingUser, grantRevokePrivParams, resp);
+ break;
default:
throw new IllegalArgumentException("Unexpected principal type: " +
grantRevokePrivParams.principal_type);
@@ -3290,6 +3293,10 @@ public class CatalogOpExecutor {
authzManager_.revokePrivilegeFromUser(requestingUser, grantRevokePrivParams,
resp);
break;
+ case GROUP:
+ authzManager_.revokePrivilegeFromGroup(requestingUser, grantRevokePrivParams,
+ resp);
+ break;
default:
throw new IllegalArgumentException("Unexpected principal type: " +
grantRevokePrivParams.principal_type);
diff --git a/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java b/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
index efa5da0..1ff7033 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AnalyzeAuthStmtsTest.java
@@ -162,159 +162,162 @@ public class AnalyzeAuthStmtsTest extends FrontendTestBase {
@Test
public void AnalyzeGrantRevokePriv() throws AnalysisException {
+ String[] idents = {"myRole", "ROLE myRole", "GROUP myGroup", "USER myUser"};
boolean[] isGrantVals = {true, false};
- for (boolean isGrant: isGrantVals) {
- Object[] formatArgs = new String[] {"REVOKE", "FROM"};
- if (isGrant) formatArgs = new String[] {"GRANT", "TO"};
- // ALL privileges
- AnalyzesOk(String.format("%s ALL ON TABLE alltypes %s myrole", formatArgs),
- createAnalysisCtx("functional"));
- AnalyzesOk(String.format("%s ALL ON TABLE functional.alltypes %s myrole",
- formatArgs));
- AnalyzesOk(String.format("%s ALL ON TABLE functional_kudu.alltypes %s myrole",
- formatArgs));
- AnalyzesOk(String.format("%s ALL ON DATABASE functional %s myrole", formatArgs));
- AnalyzesOk(String.format("%s ALL ON SERVER %s myrole", formatArgs));
- AnalyzesOk(String.format("%s ALL ON SERVER server1 %s myrole", formatArgs));
- AnalyzesOk(String.format("%s ALL ON URI 'hdfs:////abc//123' %s myrole",
- formatArgs));
- AnalysisError(String.format("%s ALL ON URI 'xxxx:////abc//123' %s myrole",
- formatArgs), "No FileSystem for scheme: xxxx");
- AnalysisError(String.format("%s ALL ON DATABASE does_not_exist %s myrole",
- formatArgs), "Error setting privileges for database 'does_not_exist'. " +
- "Verify that the database exists and that you have permissions to issue " +
- "a GRANT/REVOKE statement.");
- AnalysisError(String.format("%s ALL ON TABLE does_not_exist %s myrole",
- formatArgs), "Error setting privileges for table 'does_not_exist'. " +
- "Verify that the table exists and that you have permissions to issue " +
- "a GRANT/REVOKE statement.");
- AnalysisError(String.format("%s ALL ON SERVER does_not_exist %s myrole",
- formatArgs), "Specified server name 'does_not_exist' does not match the " +
- "configured server name 'server1'");
- // INSERT privilege
- AnalyzesOk(String.format("%s INSERT ON TABLE alltypesagg %s myrole", formatArgs),
- createAnalysisCtx("functional"));
- AnalyzesOk(String.format(
- "%s INSERT ON TABLE functional_kudu.alltypessmall %s myrole", formatArgs));
- AnalyzesOk(String.format("%s INSERT ON TABLE functional.alltypesagg %s myrole",
- formatArgs));
- AnalyzesOk(String.format("%s INSERT ON DATABASE functional %s myrole",
- formatArgs));
- AnalyzesOk(String.format("%s INSERT ON SERVER %s myrole", formatArgs));
- AnalysisError(String.format("%s INSERT ON URI 'hdfs:////abc//123' %s myrole",
- formatArgs), "Only 'ALL' privilege may be applied at URI scope in privilege " +
- "spec.");
+ for (String ident : idents) {
+ for (boolean isGrant : isGrantVals) {
+ Object[] formatArgs = new String[]{"REVOKE", "FROM", ident};
+ if (isGrant) formatArgs = new String[]{"GRANT", "TO", ident};
+ // ALL privileges
+ AnalyzesOk(String.format("%s ALL ON TABLE alltypes %s %s", formatArgs),
+ createAnalysisCtx("functional"));
+ AnalyzesOk(String.format("%s ALL ON TABLE functional.alltypes %s %s",
+ formatArgs));
+ AnalyzesOk(String.format("%s ALL ON TABLE functional_kudu.alltypes %s %s",
+ formatArgs));
+ AnalyzesOk(String.format("%s ALL ON DATABASE functional %s %s", formatArgs));
+ AnalyzesOk(String.format("%s ALL ON SERVER %s %s", formatArgs));
+ AnalyzesOk(String.format("%s ALL ON SERVER server1 %s %s", formatArgs));
+ AnalyzesOk(String.format("%s ALL ON URI 'hdfs:////abc//123' %s %s",
+ formatArgs));
+ AnalysisError(String.format("%s ALL ON URI 'xxxx:////abc//123' %s %s",
+ formatArgs), "No FileSystem for scheme: xxxx");
+ AnalysisError(String.format("%s ALL ON DATABASE does_not_exist %s %s",
+ formatArgs), "Error setting privileges for database 'does_not_exist'. " +
+ "Verify that the database exists and that you have permissions to issue " +
+ "a GRANT/REVOKE statement.");
+ AnalysisError(String.format("%s ALL ON TABLE does_not_exist %s %s",
+ formatArgs), "Error setting privileges for table 'does_not_exist'. " +
+ "Verify that the table exists and that you have permissions to issue " +
+ "a GRANT/REVOKE statement.");
+ AnalysisError(String.format("%s ALL ON SERVER does_not_exist %s %s",
+ formatArgs), "Specified server name 'does_not_exist' does not match the " +
+ "configured server name 'server1'");
- // SELECT privilege
- AnalyzesOk(String.format("%s SELECT ON TABLE alltypessmall %s myrole", formatArgs),
- createAnalysisCtx("functional"));
- AnalyzesOk(String.format("%s SELECT ON TABLE functional.alltypessmall %s myrole",
- formatArgs));
- AnalyzesOk(String.format(
- "%s SELECT ON TABLE functional_kudu.alltypessmall %s myrole", formatArgs));
- AnalyzesOk(String.format("%s SELECT ON DATABASE functional %s myrole",
- formatArgs));
- AnalyzesOk(String.format("%s SELECT ON SERVER %s myrole", formatArgs));
- AnalysisError(String.format("%s SELECT ON URI 'hdfs:////abc//123' %s myrole",
- formatArgs), "Only 'ALL' privilege may be applied at URI scope in privilege " +
- "spec.");
+ // INSERT privilege
+ AnalyzesOk(String.format("%s INSERT ON TABLE alltypesagg %s %s", formatArgs),
+ createAnalysisCtx("functional"));
+ AnalyzesOk(String.format(
+ "%s INSERT ON TABLE functional_kudu.alltypessmall %s %s", formatArgs));
+ AnalyzesOk(String.format("%s INSERT ON TABLE functional.alltypesagg %s %s",
+ formatArgs));
+ AnalyzesOk(String.format("%s INSERT ON DATABASE functional %s %s",
+ formatArgs));
+ AnalyzesOk(String.format("%s INSERT ON SERVER %s %s", formatArgs));
+ AnalysisError(String.format("%s INSERT ON URI 'hdfs:////abc//123' %s %s",
+ formatArgs), "Only 'ALL' privilege may be applied at URI scope in " +
+ "privilege spec.");
- // SELECT privileges on columns
- AnalyzesOk(String.format("%s SELECT (id, int_col) ON TABLE functional.alltypes " +
- "%s myrole", formatArgs));
- AnalyzesOk(String.format("%s SELECT (id, id) ON TABLE functional.alltypes " +
- "%s myrole", formatArgs));
- // SELECT privilege on both regular and partition columns
- AnalyzesOk(String.format("%s SELECT (id, int_col, year, month) ON TABLE " +
- "alltypes %s myrole", formatArgs), createAnalysisCtx("functional"));
- AnalyzesOk(String.format("%s SELECT (id, bool_col) ON TABLE " +
- "functional_kudu.alltypessmall %s myrole", formatArgs));
- // Empty column list
- AnalysisError(String.format("%s SELECT () ON TABLE functional.alltypes " +
- "%s myrole", formatArgs), "Empty column list in column privilege spec.");
- // INSERT/ALL privileges on columns
- AnalysisError(String.format("%s INSERT (id, tinyint_col) ON TABLE " +
- "functional.alltypes %s myrole", formatArgs), "Only 'SELECT' privileges " +
- "are allowed in a column privilege spec.");
- AnalysisError(String.format("%s ALL (id, tinyint_col) ON TABLE " +
- "functional.alltypes %s myrole", formatArgs), "Only 'SELECT' privileges " +
- "are allowed in a column privilege spec.");
- // Column-level privileges on a VIEW
- AnalysisError(String.format("%s SELECT (id, bool_col) ON TABLE " +
- "functional.alltypes_hive_view %s myrole", formatArgs), "Column-level " +
- "privileges on views are not supported.");
- // Columns/table that don't exist
- AnalysisError(String.format("%s SELECT (invalid_col) ON TABLE " +
- "functional.alltypes %s myrole", formatArgs), "Error setting column-level " +
- "privileges for table 'functional.alltypes'. Verify that both table and " +
- "columns exist and that you have permissions to issue a GRANT/REVOKE " +
- "statement.");
- AnalysisError(String.format("%s SELECT (id, int_col) ON TABLE " +
- "functional.does_not_exist %s myrole", formatArgs), "Error setting " +
- "privileges for table 'functional.does_not_exist'. Verify that the table " +
- "exists and that you have permissions to issue a GRANT/REVOKE statement.");
+ // SELECT privilege
+ AnalyzesOk(String.format("%s SELECT ON TABLE alltypessmall %s %s", formatArgs),
+ createAnalysisCtx("functional"));
+ AnalyzesOk(String.format("%s SELECT ON TABLE functional.alltypessmall %s %s",
+ formatArgs));
+ AnalyzesOk(String.format(
+ "%s SELECT ON TABLE functional_kudu.alltypessmall %s %s", formatArgs));
+ AnalyzesOk(String.format("%s SELECT ON DATABASE functional %s %s",
+ formatArgs));
+ AnalyzesOk(String.format("%s SELECT ON SERVER %s %s", formatArgs));
+ AnalysisError(String.format("%s SELECT ON URI 'hdfs:////abc//123' %s %s",
+ formatArgs), "Only 'ALL' privilege may be applied at URI scope in " +
+ "privilege spec.");
- // REFRESH privilege
- AnalyzesOk(String.format(
- "%s REFRESH ON TABLE functional.alltypes %s myrole", formatArgs));
- AnalyzesOk(String.format(
- "%s REFRESH ON DATABASE functional %s myrole", formatArgs));
- AnalyzesOk(String.format("%s REFRESH ON SERVER %s myrole", formatArgs));
- AnalyzesOk(String.format("%s REFRESH ON SERVER server1 %s myrole",
- formatArgs));
- AnalysisError(String.format(
- "%s REFRESH ON URI 'hdfs:////abc//123' %s myrole", formatArgs),
- "Only 'ALL' privilege may be applied at URI scope in privilege spec.");
+ // SELECT privileges on columns
+ AnalyzesOk(String.format("%s SELECT (id, int_col) ON TABLE functional.alltypes " +
+ "%s %s", formatArgs));
+ AnalyzesOk(String.format("%s SELECT (id, id) ON TABLE functional.alltypes " +
+ "%s %s", formatArgs));
+ // SELECT privilege on both regular and partition columns
+ AnalyzesOk(String.format("%s SELECT (id, int_col, year, month) ON TABLE " +
+ "alltypes %s %s", formatArgs), createAnalysisCtx("functional"));
+ AnalyzesOk(String.format("%s SELECT (id, bool_col) ON TABLE " +
+ "functional_kudu.alltypessmall %s %s", formatArgs));
+ // Empty column list
+ AnalysisError(String.format("%s SELECT () ON TABLE functional.alltypes " +
+ "%s %s", formatArgs), "Empty column list in column privilege spec.");
+ // INSERT/ALL privileges on columns
+ AnalysisError(String.format("%s INSERT (id, tinyint_col) ON TABLE " +
+ "functional.alltypes %s %s", formatArgs), "Only 'SELECT' privileges " +
+ "are allowed in a column privilege spec.");
+ AnalysisError(String.format("%s ALL (id, tinyint_col) ON TABLE " +
+ "functional.alltypes %s %s", formatArgs), "Only 'SELECT' privileges " +
+ "are allowed in a column privilege spec.");
+ // Column-level privileges on a VIEW
+ AnalysisError(String.format("%s SELECT (id, bool_col) ON TABLE " +
+ "functional.alltypes_hive_view %s %s", formatArgs), "Column-level " +
+ "privileges on views are not supported.");
+ // Columns/table that don't exist
+ AnalysisError(String.format("%s SELECT (invalid_col) ON TABLE " +
+ "functional.alltypes %s %s", formatArgs), "Error setting column-level " +
+ "privileges for table 'functional.alltypes'. Verify that both table and " +
+ "columns exist and that you have permissions to issue a GRANT/REVOKE " +
+ "statement.");
+ AnalysisError(String.format("%s SELECT (id, int_col) ON TABLE " +
+ "functional.does_not_exist %s %s", formatArgs), "Error setting " +
+ "privileges for table 'functional.does_not_exist'. Verify that the table " +
+ "exists and that you have permissions to issue a GRANT/REVOKE statement.");
- // CREATE privilege
- AnalyzesOk(String.format("%s CREATE ON SERVER %s myrole", formatArgs));
- AnalyzesOk(String.format("%s CREATE ON SERVER server1 %s myrole", formatArgs));
- AnalyzesOk(String.format(
- "%s CREATE ON DATABASE functional %s myrole", formatArgs));
- AnalysisError(String.format(
- "%s CREATE ON TABLE functional.alltypes %s myrole", formatArgs),
- "Create-level privileges on tables are not supported.");
- AnalysisError(String.format(
- "%s CREATE ON URI 'hdfs:////abc//123' %s myrole", formatArgs),
- "Only 'ALL' privilege may be applied at URI scope in privilege spec.");
+ // REFRESH privilege
+ AnalyzesOk(String.format(
+ "%s REFRESH ON TABLE functional.alltypes %s %s", formatArgs));
+ AnalyzesOk(String.format(
+ "%s REFRESH ON DATABASE functional %s %s", formatArgs));
+ AnalyzesOk(String.format("%s REFRESH ON SERVER %s %s", formatArgs));
+ AnalyzesOk(String.format("%s REFRESH ON SERVER server1 %s %s",
+ formatArgs));
+ AnalysisError(String.format(
+ "%s REFRESH ON URI 'hdfs:////abc//123' %s %s", formatArgs),
+ "Only 'ALL' privilege may be applied at URI scope in privilege spec.");
- // ALTER privilege
- AnalyzesOk(String.format("%s ALTER ON SERVER %s myrole", formatArgs));
- AnalyzesOk(String.format("%s ALTER ON SERVER server1 %s myrole", formatArgs));
- AnalyzesOk(String.format("%s ALTER ON DATABASE functional %s myrole", formatArgs));
- AnalyzesOk(String.format(
- "%s ALTER ON TABLE functional.alltypes %s myrole", formatArgs));
- AnalysisError(String.format(
- "%s ALTER ON URI 'hdfs:////abc/123' %s myrole", formatArgs),
- "Only 'ALL' privilege may be applied at URI scope in privilege spec.");
+ // CREATE privilege
+ AnalyzesOk(String.format("%s CREATE ON SERVER %s %s", formatArgs));
+ AnalyzesOk(String.format("%s CREATE ON SERVER server1 %s %s", formatArgs));
+ AnalyzesOk(String.format(
+ "%s CREATE ON DATABASE functional %s %s", formatArgs));
+ AnalysisError(String.format(
+ "%s CREATE ON TABLE functional.alltypes %s %s", formatArgs),
+ "Create-level privileges on tables are not supported.");
+ AnalysisError(String.format(
+ "%s CREATE ON URI 'hdfs:////abc//123' %s %s", formatArgs),
+ "Only 'ALL' privilege may be applied at URI scope in privilege spec.");
- // DROP privilege
- AnalyzesOk(String.format("%s DROP ON SERVER %s myrole", formatArgs));
- AnalyzesOk(String.format("%s DROP ON SERVER server1 %s myrole", formatArgs));
- AnalyzesOk(String.format("%s DROP ON DATABASE functional %s myrole", formatArgs));
- AnalyzesOk(String.format(
- "%s DROP ON TABLE functional.alltypes %s myrole", formatArgs));
- AnalysisError(String.format(
- "%s DROP ON URI 'hdfs:////abc/123' %s myrole", formatArgs),
- "Only 'ALL' privilege may be applied at URI scope in privilege spec.");
- }
+ // ALTER privilege
+ AnalyzesOk(String.format("%s ALTER ON SERVER %s %s", formatArgs));
+ AnalyzesOk(String.format("%s ALTER ON SERVER server1 %s %s", formatArgs));
+ AnalyzesOk(String.format("%s ALTER ON DATABASE functional %s %s", formatArgs));
+ AnalyzesOk(String.format(
+ "%s ALTER ON TABLE functional.alltypes %s %s", formatArgs));
+ AnalysisError(String.format(
+ "%s ALTER ON URI 'hdfs:////abc/123' %s %s", formatArgs),
+ "Only 'ALL' privilege may be applied at URI scope in privilege spec.");
- AnalysisContext authDisabledCtx = createAuthDisabledAnalysisCtx();
- AnalysisError("GRANT ALL ON SERVER TO myRole", authDisabledCtx,
- "Authorization is not enabled.");
- AnalysisError("REVOKE ALL ON SERVER FROM myRole", authDisabledCtx,
- "Authorization is not enabled.");
+ // DROP privilege
+ AnalyzesOk(String.format("%s DROP ON SERVER %s %s", formatArgs));
+ AnalyzesOk(String.format("%s DROP ON SERVER server1 %s %s", formatArgs));
+ AnalyzesOk(String.format("%s DROP ON DATABASE functional %s %s", formatArgs));
+ AnalyzesOk(String.format(
+ "%s DROP ON TABLE functional.alltypes %s myrole", formatArgs));
+ AnalysisError(String.format(
+ "%s DROP ON URI 'hdfs:////abc/123' %s %s", formatArgs),
+ "Only 'ALL' privilege may be applied at URI scope in privilege spec.");
+ }
+ AnalysisContext authDisabledCtx = createAuthDisabledAnalysisCtx();
+ AnalysisError("GRANT ALL ON SERVER TO myRole", authDisabledCtx,
+ "Authorization is not enabled.");
+ AnalysisError("REVOKE ALL ON SERVER FROM myRole", authDisabledCtx,
+ "Authorization is not enabled.");
- TQueryCtx noUserNameQueryCtx = TestUtils.createQueryContext(
- Catalog.DEFAULT_DB, "");
- EventSequence timeline = new EventSequence("Authorization Test");
- AnalysisContext noUserNameCtx = new AnalysisContext(noUserNameQueryCtx,
- new SentryAuthorizationFactory(
- SentryAuthorizationConfig.createHadoopGroupAuthConfig("server1", null)),
- timeline);
- AnalysisError("GRANT ALL ON SERVER TO myRole", noUserNameCtx,
- "Cannot execute authorization statement with an empty username.");
+ TQueryCtx noUserNameQueryCtx = TestUtils.createQueryContext(
+ Catalog.DEFAULT_DB, "");
+ EventSequence timeline = new EventSequence("Authorization Test");
+ AnalysisContext noUserNameCtx = new AnalysisContext(noUserNameQueryCtx,
+ new SentryAuthorizationFactory(
+ SentryAuthorizationConfig.createHadoopGroupAuthConfig("server1", null)),
+ timeline);
+ AnalysisError("GRANT ALL ON SERVER TO myRole", noUserNameCtx,
+ "Cannot execute authorization statement with an empty username.");
+ }
}
}
diff --git a/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java b/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
index c712bce..feb14f5 100644
--- a/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/AuthorizationStmtTest.java
@@ -24,6 +24,7 @@ import static org.junit.Assert.fail;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
+import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
@@ -67,7 +68,6 @@ import org.apache.impala.thrift.TPrivilegeScope;
import org.apache.impala.thrift.TQueryOptions;
import org.apache.impala.thrift.TResultRow;
import org.apache.impala.thrift.TTableName;
-import org.apache.ranger.plugin.audit.RangerDefaultAuditHandler;
import org.apache.ranger.plugin.util.GrantRevokeRequest;
import org.apache.sentry.api.service.thrift.TSentryRole;
import org.junit.AfterClass;
@@ -100,7 +100,6 @@ public class AuthorizationStmtTest extends FrontendTestBase {
private final ImpaladTestCatalog authzCatalog_;
private final Frontend authzFrontend_;
private final RangerImpalaPlugin rangerImpalaPlugin_;
- private final RangerDefaultAuditHandler rangerAuditHandler_;
public AuthorizationStmtTest(AuthorizationProvider authzProvider) {
authzProvider_ = authzProvider;
@@ -116,7 +115,6 @@ public class AuthorizationStmtTest extends FrontendTestBase {
sentryService_ = new SentryPolicyService(
((SentryAuthorizationConfig) authzConfig_).getSentryConfig());
rangerImpalaPlugin_ = null;
- rangerAuditHandler_ = null;
break;
case RANGER:
authzConfig_ = new RangerAuthorizationConfig(RANGER_SERVICE_TYPE, RANGER_APP_ID,
@@ -128,7 +126,6 @@ public class AuthorizationStmtTest extends FrontendTestBase {
rangerImpalaPlugin_ =
((RangerAuthorizationChecker) authzFrontend_.getAuthzChecker())
.getRangerImpalaPlugin();
- rangerAuditHandler_ = new RangerDefaultAuditHandler();
sentryService_ = null;
break;
default:
@@ -2993,7 +2990,7 @@ public class AuthorizationStmtTest extends FrontendTestBase {
public String getName() { return role_; }
}
- private class WithRangerUser implements WithPrincipal {
+ private abstract class WithRanger implements WithPrincipal {
private final List<GrantRevokeRequest> requests = new ArrayList<>();
private final RangerCatalogdAuthorizationManager authzManager =
new RangerCatalogdAuthorizationManager(() -> rangerImpalaPlugin_);
@@ -3001,10 +2998,7 @@ public class AuthorizationStmtTest extends FrontendTestBase {
@Override
public void init(TPrivilege[]... privileges) throws ImpalaException {
for (TPrivilege[] privilege : privileges) {
- List<GrantRevokeRequest> request =
- RangerCatalogdAuthorizationManager.createGrantRevokeRequests(
- RANGER_ADMIN.getName(), USER.getName(),
- rangerImpalaPlugin_.getClusterName(), Arrays.asList(privilege))
+ requests.addAll(buildRequest(Arrays.asList(privilege))
.stream()
.peek(r -> {
r.setResource(updateUri(r.getResource()));
@@ -3012,35 +3006,58 @@ public class AuthorizationStmtTest extends FrontendTestBase {
r.getAccessTypes().remove("owner");
r.getAccessTypes().add("all");
}
- }).collect(Collectors.toList());
-
- requests.addAll(request);
+ }).collect(Collectors.toList()));
}
- authzManager.grantPrivilegeToUser(RANGER_ADMIN, requests, null);
+ authzManager.grantPrivilege(requests);
// TODO: replace with the new API in RANGER-2349.
rangerImpalaPlugin_.init();
}
+ /**
+ * Create the {@link GrantRevokeRequest}s used for granting and revoking privileges.
+ */
+ protected abstract List<GrantRevokeRequest> buildRequest(List<TPrivilege> privileges);
+
@Override
public void cleanUp() throws ImpalaException {
- authzManager.revokePrivilegeFromUser(RANGER_ADMIN, requests, null);
+ authzManager.revokePrivilege(requests);
}
@Override
public String getName() { return USER.getName(); }
+ }
- private Map<String, String> updateUri(Map<String, String> resources) {
- String uri = resources.get(RangerImpalaResourceBuilder.URL);
- if (uri != null && uri.startsWith("/")) {
- uri = "hdfs://localhost:20500" + uri;
- }
- resources.put(RangerImpalaResourceBuilder.URL, uri);
+ private class WithRangerUser extends WithRanger {
+ @Override
+ protected List<GrantRevokeRequest> buildRequest(List<TPrivilege> privileges) {
+ return RangerCatalogdAuthorizationManager.createGrantRevokeRequests(
+ RANGER_ADMIN.getName(), USER.getName(), Collections.emptyList(),
+ rangerImpalaPlugin_.getClusterName(), privileges);
+ }
+ }
+
+ private class WithRangerGroup extends WithRanger {
+ @Override
+ protected List<GrantRevokeRequest> buildRequest(List<TPrivilege> privileges) {
+ List<String> groups = Collections.singletonList(System.getProperty("user.name"));
- return resources;
+ return RangerCatalogdAuthorizationManager.createGrantRevokeRequests(
+ RANGER_ADMIN.getName(), null, groups,
+ rangerImpalaPlugin_.getClusterName(), privileges);
}
}
+ private static Map<String, String> updateUri(Map<String, String> resources) {
+ String uri = resources.get(RangerImpalaResourceBuilder.URL);
+ if (uri != null && uri.startsWith("/")) {
+ uri = "hdfs://localhost:20500" + uri;
+ }
+ resources.put(RangerImpalaResourceBuilder.URL, uri);
+
+ return resources;
+ }
+
private class DescribeOutput {
private String[] excludedStrings_ = new String[0];
private String[] includedStrings_ = new String[0];
@@ -3120,6 +3137,7 @@ public class AuthorizationStmtTest extends FrontendTestBase {
break;
case RANGER:
withPrincipals.add(new WithRangerUser());
+ withPrincipals.add(new WithRangerGroup());
break;
default:
throw new IllegalArgumentException(String.format(
diff --git a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
index e050ea7..bc23fe4 100644
--- a/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/ParserTest.java
@@ -3589,111 +3589,125 @@ public class ParserTest extends FrontendTestBase {
@Test
public void TestGrantRevokePrivilege() {
- Object[][] grantRevFormatStrs = {{"GRANT", "TO"}, {"REVOKE", "FROM"}};
- for (Object[] formatStr: grantRevFormatStrs) {
- ParsesOk(String.format("%s ALL ON TABLE foo %s myRole", formatStr));
-
- // KW_ROLE is optional (Hive requires KW_ROLE, but Impala does not).
- ParsesOk(String.format("%s ALL ON TABLE foo %s ROLE myRole", formatStr));
-
- ParsesOk(String.format("%s ALL ON DATABASE foo %s myRole", formatStr));
- ParsesOk(String.format("%s ALL ON URI 'foo' %s myRole", formatStr));
-
- ParsesOk(String.format("%s INSERT ON TABLE foo %s myRole", formatStr));
- ParsesOk(String.format("%s INSERT ON DATABASE foo %s myRole", formatStr));
- ParsesOk(String.format("%s INSERT ON URI 'foo' %s myRole", formatStr));
-
- ParsesOk(String.format("%s SELECT ON TABLE foo %s myRole", formatStr));
- ParserError(String.format("%s SELECT ON TABLE %s myRole", formatStr));
- ParsesOk(String.format("%s SELECT ON DATABASE foo %s myRole", formatStr));
- ParserError(String.format("%s SELECT ON DATABASE %s myRole", formatStr));
- ParsesOk(String.format("%s SELECT ON URI 'foo' %s myRole", formatStr));
- ParserError(String.format("%s SELECT ON URI %s myRole", formatStr));
-
- // Column-level authorization on TABLE scope
- ParsesOk(String.format("%s SELECT (a, b) ON TABLE foo %s myRole", formatStr));
- ParsesOk(String.format("%s SELECT () ON TABLE foo %s myRole", formatStr));
- ParsesOk(String.format("%s INSERT (a, b) ON TABLE foo %s myRole", formatStr));
- ParsesOk(String.format("%s ALL (a, b) ON TABLE foo %s myRole", formatStr));
- ParserError(String.format("%s SELECT (*) ON TABLE foo %s myRole", formatStr));
-
- ParserError(String.format("%s SELECT (a,) ON TABLE foo %s myRole", formatStr));
- ParserError(String.format("%s SELECT a, b ON TABLE foo %s myRole", formatStr));
- ParserError(String.format("%s SELECT (a), b ON TABLE foo %s myRole", formatStr));
- ParserError(String.format("%s SELECT ON TABLE (a, b) foo %s myRole", formatStr));
- ParserError(String.format("%s SELECT ((a)) ON TABLE foo %s myRole", formatStr));
- ParserError(String.format("%s SELECT (a, b) ON DATABASE foo %s myRole",
- formatStr));
- ParserError(String.format("%s SELECT (a, b) ON URI 'foo' %s myRole", formatStr));
-
- // REFRESH privilege.
- ParsesOk(String.format("%s REFRESH ON SERVER %s myRole", formatStr));
- ParsesOk(String.format("%s REFRESH ON SERVER foo %s myRole", formatStr));
- ParsesOk(String.format("%s REFRESH ON DATABASE foo %s myRole", formatStr));
- ParsesOk(String.format("%s REFRESH ON TABLE foo %s myRole", formatStr));
-
- // CREATE privilege.
- ParsesOk(String.format("%s CREATE ON SERVER %s myRole", formatStr));
- ParsesOk(String.format("%s CREATE ON SERVER foo %s myRole", formatStr));
- ParsesOk(String.format("%s CREATE ON DATABASE foo %s myRole", formatStr));
-
- // ALTER privilege.
- ParsesOk(String.format("%s ALTER ON SERVER %s myRole", formatStr));
- ParsesOk(String.format("%s ALTER ON SERVER foo %s myRole", formatStr));
- ParsesOk(String.format("%s ALTER ON DATABASE foo %s myRole", formatStr));
- ParsesOk(String.format("%s ALTER ON TABLE foo %s myRole", formatStr));
-
- // DROP privilege.
- ParsesOk(String.format("%s DROP ON SERVER %s myRole", formatStr));
- ParsesOk(String.format("%s DROP ON SERVER foo %s myRole", formatStr));
- ParsesOk(String.format("%s DROP ON DATABASE foo %s myRole", formatStr));
- ParsesOk(String.format("%s DROP ON TABLE foo %s myRole", formatStr));
-
- // Server scope does not accept a name.
- ParsesOk(String.format("%s ALL ON SERVER %s myRole", formatStr));
- ParsesOk(String.format("%s INSERT ON SERVER %s myRole", formatStr));
- ParsesOk(String.format("%s SELECT ON SERVER %s myRole", formatStr));
-
- // URIs are string literals
- ParserError(String.format("%s ALL ON URI foo %s myRole", formatStr));
- ParserError(String.format("%s ALL ON DATABASE 'foo' %s myRole", formatStr));
- ParserError(String.format("%s ALL ON TABLE 'foo' %s myRole", formatStr));
-
- // No object name (only works for SERVER scope)
- ParserError(String.format("GRANT ALL ON TABLE FROM myrole", formatStr));
- ParserError(String.format("GRANT ALL ON DATABASE FROM myrole", formatStr));
- ParserError(String.format("GRANT ALL ON URI FROM myrole", formatStr));
-
- // No role specified
- ParserError(String.format("%s ALL ON TABLE foo %s", formatStr));
- // Invalid privilege
- ParserError(String.format("%s FAKE ON TABLE foo %s myRole", formatStr));
+ String[][] grantRevoke = {{"GRANT", "TO"}, {"REVOKE", "FROM"}};
+ String[] resources = {"SERVER", "SERVER foo", "DATABASE foo", "TABLE foo",
+ "URI 'foo'"};
+ String[] badResources = {"DATABASE", "TABLE", "URI", "URI foo", "TABLE 'foo'",
+ "SERVER 'foo'", "DATABASE 'foo'"};
+ String[] privileges = {"SELECT", "INSERT", "ALL", "REFRESH", "CREATE", "ALTER",
+ "DROP"};
+ String[] badPrivileges = {"UPDATE", "DELETE", "UPSERT", "FAKE"};
+ String[] columnPrivResource = {
+ "SELECT (a, b) ON TABLE foo",
+ "SELECT () on TABLE foo",
+ "INSERT (a, b) ON TABLE foo",
+ "ALL (a, b) ON TABLE foo"
+ };
+ String[] badColumnPrivResource = {
+ "SELECT (a,) ON TABLE foo",
+ "SELECT (*) ON TABLE foo",
+ "SELECT (a), b ON TABLE foo",
+ "SELECT ((a)) ON TABLE foo",
+ "SELECT (a, b) ON URI foo",
+ "SELECT ON TABLE (a, b) foo"
+ };
+ String[] idents = {"myRole", "GROUP myGroup", "USER user", "ROLE myRole"};
+ String[] badIdents = {"GROUP", "ROLE", "GROUP group", "GROUP role", "USER role",
+ "FOOBAR foobar", ""};
+
+ // Good SQL
+ createPrivSQL(grantRevoke, resources, privileges, idents)
+ .forEach(this::ParsesOk);
+
+ // Good SQL with grant option
+ createPrivSQL(grantRevoke, resources, privileges, idents)
+ .stream()
+ .filter(s -> s.contains("GRANT"))
+ .map(s -> s + " WITH GRANT OPTION")
+ .forEach(this::ParsesOk);
+
+ // Good SQL with revoke option
+ createPrivSQL(grantRevoke, resources, privileges, idents)
+ .stream()
+ .filter(s -> s.contains("REVOKE"))
+ .map(s -> s.replace("REVOKE", "REVOKE GRANT OPTION FOR"))
+ .forEach(this::ParsesOk);
+
+ // Column SQL
+ createColumnPrivSql(grantRevoke, columnPrivResource, idents)
+ .forEach(this::ParsesOk);
+
+ // Bad Resources
+ createPrivSQL(grantRevoke, badResources, privileges, idents)
+ .forEach(this::ParserError);
+
+ // Bad privileges
+ createPrivSQL(grantRevoke, resources, badPrivileges, idents)
+ .forEach(this::ParserError);
+
+ // Bad idents
+ createPrivSQL(grantRevoke, resources, privileges, badIdents)
+ .forEach(this::ParserError);
+
+ // Bad SQL with grant option
+ createPrivSQL(grantRevoke, resources, privileges, idents)
+ .stream()
+ .filter(s -> s.contains("GRANT"))
+ .map(s -> s + " WITH GRANT")
+ .forEach(this::ParserError);
+
+ // Bad SQL with grant option
+ createPrivSQL(grantRevoke, resources, privileges, idents)
+ .stream()
+ .filter(s -> s.contains("GRANT"))
+ .map(s -> s + " WITH")
+ .forEach(this::ParserError);
+
+ // Bad SQL with revoke option (omit for)
+ createPrivSQL(grantRevoke, resources, privileges, idents)
+ .stream()
+ .filter(s -> s.contains("REVOKE"))
+ .map(s -> s.replace("REVOKE", "REVOKE GRANT OPTION"))
+ .forEach(this::ParserError);
+
+ // Bad SQL with revoke option (omit option for)
+ createPrivSQL(grantRevoke, resources, privileges, idents)
+ .stream()
+ .filter(s -> s.contains("REVOKE"))
+ .map(s -> s.replace("REVOKE", "REVOKE GRANT"))
+ .forEach(this::ParserError);
+ }
+
+ private static List<String> createPrivSQL(String[][] formats, String[] resources,
+ String[] privileges, String[] idents) {
+ List<String> result = new ArrayList<>();
+
+ for (String[] formatStr : formats) {
+ for (String resource : resources) {
+ for (String privilege : privileges) {
+ for (String ident : idents) {
+ result.add(String.format("%s %s ON %s %s %s", formatStr[0], privilege,
+ resource, formatStr[1], ident));
+ }
+ }
+ }
+ }
+ return result;
+ }
+
+ private static List<String> createColumnPrivSql(String[][] formats,
+ String[] privResource, String[] idents) {
+ List<String> result = new ArrayList<>();
+
+ for (String[] formatStr : formats) {
+ for (String pr : privResource) {
+ for (String ident : idents) {
+ result.add(String.format("%s %s %s %s", formatStr[0], pr, formatStr[1],
+ ident));
+ }
+ }
}
- ParsesOk("GRANT ALL ON TABLE foo TO myRole WITH GRANT OPTION");
- ParsesOk("GRANT ALL ON DATABASE foo TO myRole WITH GRANT OPTION");
- ParsesOk("GRANT ALL ON SERVER TO myRole WITH GRANT OPTION");
- ParsesOk("GRANT ALL ON URI '/abc/' TO myRole WITH GRANT OPTION");
- ParserError("GRANT ALL ON TABLE foo TO myRole WITH GRANT");
- ParserError("GRANT ALL ON TABLE foo TO myRole WITH");
- ParserError("GRANT ALL ON TABLE foo TO ROLE");
- ParserError("REVOKE ALL ON TABLE foo TO ROLE");
-
- ParsesOk("REVOKE GRANT OPTION FOR ALL ON TABLE foo FROM myRole");
- ParsesOk("REVOKE GRANT OPTION FOR ALL ON DATABASE foo FROM myRole");
- ParsesOk("REVOKE GRANT OPTION FOR ALL ON SERVER FROM myRole");
- ParsesOk("REVOKE GRANT OPTION FOR ALL ON URI '/abc/' FROM myRole");
- ParserError("REVOKE GRANT OPTION ALL ON URI '/abc/' FROM myRole");
- ParserError("REVOKE GRANT ALL ON URI '/abc/' FROM myRole");
-
- ParserError("ALL ON TABLE foo TO myrole");
- ParserError("ALL ON TABLE foo FROM myrole");
-
- ParserError("GRANT ALL ON TABLE foo FROM myrole");
- ParserError("REVOKE ALL ON TABLE foo TO myrole");
-
- ParserError("GRANT UPDATE ON TABLE foo TO myRole");
- ParserError("GRANT DELETE ON TABLE foo TO myRole");
- ParserError("GRANT UPSERT ON TABLE foo TO myRole");
+ return result;
}
@Test
diff --git a/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java b/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
index 198061e..51fe8d9 100644
--- a/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
+++ b/fe/src/test/java/org/apache/impala/analysis/ToSqlTest.java
@@ -1492,7 +1492,7 @@ public class ToSqlTest extends FrontendTestBase {
@Test
public void testGrantRevokePrivStmt() {
AnalysisContext ctx = createAnalysisCtx(createAuthorizationFactory());
- List<String> principalTypes = Arrays.asList("USER", "ROLE");
+ List<String> principalTypes = Arrays.asList("USER", "ROLE", "GROUP");
String testRole = System.getProperty("user.name");
String testUri = "hdfs://localhost:20500/test-warehouse";
diff --git a/fe/src/test/resources/ranger-hive-security.xml b/fe/src/test/resources/ranger-hive-security.xml
index 4fbda3e..4cc3160 100644
--- a/fe/src/test/resources/ranger-hive-security.xml
+++ b/fe/src/test/resources/ranger-hive-security.xml
@@ -44,7 +44,7 @@
<property>
<name>ranger.plugin.hive.policy.pollIntervalMs</name>
- <value>30000</value>
+ <value>5000</value>
<description>
Polling interval in milliseconds to poll for changes in policies.
</description>
diff --git a/testdata/bin/create-load-data.sh b/testdata/bin/create-load-data.sh
index e261eba..4aa1ebd 100755
--- a/testdata/bin/create-load-data.sh
+++ b/testdata/bin/create-load-data.sh
@@ -300,6 +300,17 @@ function setup-ranger {
RANGER_SETUP_DIR="${IMPALA_HOME}/testdata/cluster/ranger/setup"
perl -wpl -e 's/\$\{([^}]+)\}/defined $ENV{$1} ? $ENV{$1} : $&/eg' \
+ "${RANGER_SETUP_DIR}/impala_group.json.template" > \
+ "${RANGER_SETUP_DIR}/impala_group.json"
+
+ export GROUP_ID=$(wget -qO - --auth-no-challenge --user=admin --password=admin \
+ --post-file="${RANGER_SETUP_DIR}/impala_group.json" \
+ --header="accept:application/json" \
+ --header="Content-Type:application/json" \
+ http://localhost:6080/service/xusers/secure/groups |
+ python -c "import sys, json; print json.load(sys.stdin)['id']")
+
+ perl -wpl -e 's/\$\{([^}]+)\}/defined $ENV{$1} ? $ENV{$1} : $&/eg' \
"${RANGER_SETUP_DIR}/impala_user.json.template" > \
"${RANGER_SETUP_DIR}/impala_user.json"
diff --git a/testdata/cluster/ranger/setup/impala_group.json.template b/testdata/cluster/ranger/setup/impala_group.json.template
new file mode 100644
index 0000000..f1083cf
--- /dev/null
+++ b/testdata/cluster/ranger/setup/impala_group.json.template
@@ -0,0 +1,4 @@
+{
+ "name" : "${USER}",
+ "description" : ""
+}
diff --git a/testdata/cluster/ranger/setup/impala_user.json.template b/testdata/cluster/ranger/setup/impala_user.json.template
index b8d475e..02bad30 100644
--- a/testdata/cluster/ranger/setup/impala_user.json.template
+++ b/testdata/cluster/ranger/setup/impala_user.json.template
@@ -1,5 +1,6 @@
{
"name" : "${USER}",
"password" : "password123",
- "userRoleList" : [ "ROLE_USER" ]
-}
\ No newline at end of file
+ "userRoleList" : [ "ROLE_USER" ],
+ "groupIdList" : [ "${GROUP_ID}" ]
+}
diff --git a/tests/authorization/test_ranger.py b/tests/authorization/test_ranger.py
index 0141aef..ab70cd5 100644
--- a/tests/authorization/test_ranger.py
+++ b/tests/authorization/test_ranger.py
@@ -17,6 +17,7 @@
#
# Client tests for SQL statement authorization
+import grp
import pytest
import time
from getpass import getuser
@@ -45,31 +46,38 @@ class TestRanger(CustomClusterTestSuite):
user_client = self.create_impala_client()
unique_database = unique_name + "_db"
unique_table = unique_name + "_tbl"
+ group = grp.getgrnam(getuser()).gr_name
+ test_data = [(user, "user"), (group, "group")]
- try:
- # Set-up temp database/table
- admin_client.execute("drop database if exists {0} cascade".format(unique_database),
- user=admin)
- admin_client.execute("create database {0}".format(unique_database), user=admin)
- admin_client.execute("create table {0}.{1} (x int)"
- .format(unique_database, unique_table), user=admin)
+ for data in test_data:
+ ident = data[0]
+ kw = data[1]
- self.execute_query_expect_success(admin_client,
- "grant select on database {0} to user {1}"
- .format(unique_database, user), user=admin)
- # TODO: IMPALA-8293 use refresh authorization
- time.sleep(35)
- self.execute_query_expect_success(user_client, "show tables in {0}"
- .format(unique_database), user=user)
- self.execute_query_expect_success(admin_client,
- "revoke select on database {0} from user "
- "{1}".format(unique_database, user), user=admin)
- # TODO: IMPALA-8293 use refresh authorization
- time.sleep(35)
- self.execute_query_expect_failure(user_client, "show tables in {0}"
- .format(unique_database))
- finally:
- admin_client.execute("revoke select on database {0} from user {1}"
- .format(unique_database, user), user=admin)
- admin_client.execute("drop database if exists {0} cascade".format(unique_database),
- user=admin)
+ try:
+ # Set-up temp database/table
+ admin_client.execute("drop database if exists {0} cascade"
+ .format(unique_database), user=admin)
+ admin_client.execute("create database {0}".format(unique_database), user=admin)
+ admin_client.execute("create table {0}.{1} (x int)"
+ .format(unique_database, unique_table), user=admin)
+
+ self.execute_query_expect_success(admin_client,
+ "grant select on database {0} to {1} {2}"
+ .format(unique_database, kw, ident), user=admin)
+ # TODO: IMPALA-8293 use refresh authorization
+ time.sleep(10)
+ self.execute_query_expect_success(user_client, "show tables in {0}"
+ .format(unique_database), user=user)
+ self.execute_query_expect_success(admin_client,
+ "revoke select on database {0} from {1} "
+ "{2}".format(unique_database, kw, ident),
+ user=admin)
+ # TODO: IMPALA-8293 use refresh authorization
+ time.sleep(10)
+ self.execute_query_expect_failure(user_client, "show tables in {0}"
+ .format(unique_database))
+ finally:
+ admin_client.execute("revoke select on database {0} from {1} {2}"
+ .format(unique_database, kw, ident), user=admin)
+ admin_client.execute("drop database if exists {0} cascade"
+ .format(unique_database), user=admin)