You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@impala.apache.org by to...@apache.org on 2019/05/13 22:54:40 UTC
[impala] 04/04: IMPALA-8530: Return more user friendly error
messages on unsupported SQL
This is an automated email from the ASF dual-hosted git repository.
todd pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/impala.git
commit cc78b764e85503e26f3eb37bdcf5df50c0cc6484
Author: Fredy Wijaya <fw...@cloudera.com>
AuthorDate: Thu May 9 11:26:27 2019 -0700
IMPALA-8530: Return more user friendly error messages on unsupported SQL
This patch updates the error messages to return more friendly error
messages for the unsupported SQL statements when running with Sentry or
Ranger to improve user experience. This patch tries to differentiate
between UnsupportedOperationException, which is an internal exception to
indicate a programming error vs UnsupportedFeatureException, which is
an exception that will be exposed to the users to indicate that a
feature is not supported.
Testing:
- Added tests for unsupported SQL
- Ran FE tests
- Ran E2E authorization tests
Change-Id: Icf08ee9e1a83d7597811b1c8585aee5872d0baaf
Reviewed-on: http://gerrit.cloudera.org:8080/13295
Reviewed-by: Impala Public Jenkins <im...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../ranger/RangerCatalogdAuthorizationManager.java | 26 ++++++++++------------
.../ranger/RangerImpaladAuthorizationManager.java | 17 +++++++++++---
.../sentry/SentryCatalogdAuthorizationManager.java | 21 +++++++++--------
.../sentry/SentryImpaladAuthorizationManager.java | 7 ++++--
.../java/org/apache/impala/service/Frontend.java | 6 -----
tests/authorization/test_ranger.py | 21 +++++++++++++++++
tests/authorization/test_sentry.py | 22 ++++++++++++++++++
7 files changed, 84 insertions(+), 36 deletions(-)
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 ad7d02d..44d48db 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
@@ -26,6 +26,7 @@ import org.apache.impala.catalog.AuthzCacheInvalidation;
import org.apache.impala.catalog.CatalogServiceCatalog;
import org.apache.impala.common.ImpalaException;
import org.apache.impala.common.InternalException;
+import org.apache.impala.common.UnsupportedFeatureException;
import org.apache.impala.thrift.TCreateDropRoleParams;
import org.apache.impala.thrift.TDdlExecResponse;
import org.apache.impala.thrift.TGrantRevokePrivParams;
@@ -42,7 +43,6 @@ 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.function.Function;
@@ -54,7 +54,7 @@ import java.util.function.Supplier;
* Operations here make requests to Ranger via the {@link RangerImpalaPlugin} to
* manage privileges for users.
*
- * Operations not supported by Ranger will throw an {@link UnsupportedOperationException}.
+ * Operations not supported by Ranger will throw an {@link UnsupportedFeatureException}.
*/
public class RangerCatalogdAuthorizationManager implements AuthorizationManager {
private static final String AUTHZ_CACHE_INVALIDATION_MARKER = "ranger";
@@ -73,15 +73,13 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager
@Override
public void createRole(User requestingUser, TCreateDropRoleParams params,
TDdlExecResponse response) throws ImpalaException {
- throw new UnsupportedOperationException(String.format(
- "%s is not supported in Catalogd", ClassUtil.getMethodName()));
+ throw new UnsupportedFeatureException("CREATE ROLE is not supported by Ranger.");
}
@Override
public void dropRole(User requestingUser, TCreateDropRoleParams params,
TDdlExecResponse response) throws ImpalaException {
- throw new UnsupportedOperationException(String.format(
- "%s is not supported in Catalogd", ClassUtil.getMethodName()));
+ throw new UnsupportedFeatureException("DROP ROLE is not supported by Ranger.");
}
@Override
@@ -93,29 +91,29 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager
@Override
public void grantRoleToGroup(User requestingUser, TGrantRevokeRoleParams params,
TDdlExecResponse response) throws ImpalaException {
- throw new UnsupportedOperationException(String.format(
- "%s is not supported in Catalogd", ClassUtil.getMethodName()));
+ throw new UnsupportedFeatureException(
+ "GRANT ROLE TO GROUP is not supported by Ranger.");
}
@Override
public void revokeRoleFromGroup(User requestingUser, TGrantRevokeRoleParams params,
TDdlExecResponse response) throws ImpalaException {
- throw new UnsupportedOperationException(String.format(
- "%s is not supported in Catalogd", ClassUtil.getMethodName()));
+ throw new UnsupportedFeatureException(
+ "REVOKE ROLE FROM GROUP is not supported by Ranger.");
}
@Override
public void grantPrivilegeToRole(User requestingUser, TGrantRevokePrivParams params,
TDdlExecResponse response) throws ImpalaException {
- throw new UnsupportedOperationException(String.format(
- "%s is not supported in Catalogd", ClassUtil.getMethodName()));
+ throw new UnsupportedFeatureException(
+ "GRANT <privilege> TO ROLE is not supported by Ranger.");
}
@Override
public void revokePrivilegeFromRole(User requestingUser, TGrantRevokePrivParams params,
TDdlExecResponse response) throws ImpalaException {
- throw new UnsupportedOperationException(String.format(
- "%s is not supported in Catalogd", ClassUtil.getMethodName()));
+ throw new UnsupportedFeatureException(
+ "REVOKE <privilege> FROM ROLE is not supported by Ranger.");
}
@Override
diff --git a/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java b/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
index 244d273..c84a496 100644
--- a/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
+++ b/fe/src/main/java/org/apache/impala/authorization/ranger/RangerImpaladAuthorizationManager.java
@@ -25,6 +25,7 @@ import org.apache.impala.authorization.AuthorizationManager;
import org.apache.impala.authorization.User;
import org.apache.impala.catalog.Type;
import org.apache.impala.common.ImpalaException;
+import org.apache.impala.common.UnsupportedFeatureException;
import org.apache.impala.thrift.TColumn;
import org.apache.impala.thrift.TCreateDropRoleParams;
import org.apache.impala.thrift.TDdlExecResponse;
@@ -68,7 +69,7 @@ import java.util.stream.Collectors;
* Operations here make requests to Ranger via the {@link RangerImpalaPlugin} to
* manage privileges for users.
*
- * Operations not supported by Ranger will throw an {@link UnsupportedOperationException}.
+ * Operations not supported by Ranger will throw an {@link UnsupportedFeatureException}.
*/
public class RangerImpaladAuthorizationManager implements AuthorizationManager {
private static final String ANY = "*";
@@ -97,8 +98,13 @@ public class RangerImpaladAuthorizationManager implements AuthorizationManager {
@Override
public TShowRolesResult getRoles(TShowRolesParams params) throws ImpalaException {
- throw new UnsupportedOperationException(String.format(
- "%s is not supported in Impalad", ClassUtil.getMethodName()));
+ if (params.getGrant_group() != null) {
+ throw new UnsupportedFeatureException(
+ "SHOW ROLE GRANT GROUP is not supported by Ranger.");
+ }
+ throw new UnsupportedFeatureException(
+ String.format("SHOW %sROLES is not supported by Ranger.",
+ params.is_show_current_roles ? "CURRENT " : ""));
}
@Override
@@ -299,6 +305,11 @@ public class RangerImpaladAuthorizationManager implements AuthorizationManager {
@Override
public TResultSet getPrivileges(TShowGrantPrincipalParams params)
throws ImpalaException {
+ if (params.principal_type == TPrincipalType.ROLE) {
+ throw new UnsupportedFeatureException(
+ "SHOW GRANT ROLE is not supported by Ranger.");
+ }
+
List<RangerAccessRequest> requests = buildAccessRequests(params.privilege);
Set<TResultRow> resultSet = new TreeSet<>();
TResultSet result = new TResultSet();
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 ffe9000..a0a4062 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
@@ -31,9 +31,9 @@ import org.apache.impala.catalog.Principal;
import org.apache.impala.catalog.PrincipalPrivilege;
import org.apache.impala.catalog.Role;
import org.apache.impala.common.ImpalaException;
-import org.apache.impala.common.ImpalaRuntimeException;
import org.apache.impala.common.InternalException;
import org.apache.impala.common.Reference;
+import org.apache.impala.common.UnsupportedFeatureException;
import org.apache.impala.thrift.TCatalogObject;
import org.apache.impala.thrift.TCreateDropRoleParams;
import org.apache.impala.thrift.TDdlExecResponse;
@@ -63,8 +63,7 @@ import java.util.stream.Collectors;
* performed by {@link SentryProxy}. Any update to authorization metadata will then be
* broadcasted to all Impalads.
*
- * Other non-Catalogd operations, such as SHOW ROLES, SHOW GRANT will throw
- * {@link UnsupportedOperationException}.
+ * Operations not supported by Sentry will throw an {@link UnsupportedFeatureException}.
*/
public class SentryCatalogdAuthorizationManager implements AuthorizationManager {
private static final Logger LOG =
@@ -306,29 +305,29 @@ public class SentryCatalogdAuthorizationManager implements AuthorizationManager
@Override
public void grantPrivilegeToUser(User requestingUser, TGrantRevokePrivParams params,
TDdlExecResponse response) throws ImpalaException {
- throw new UnsupportedOperationException(String.format(
- "%s is not supported in Catalogd", ClassUtil.getMethodName()));
+ throw new UnsupportedFeatureException(
+ "GRANT <privilege> TO USER is not supported by Sentry.");
}
@Override
public void revokePrivilegeFromUser(User requestingUser, TGrantRevokePrivParams params,
TDdlExecResponse response) throws ImpalaException {
- throw new UnsupportedOperationException(String.format(
- "%s is not supported in Catalogd", ClassUtil.getMethodName()));
+ throw new UnsupportedFeatureException(
+ "REVOKE <privilege> FROM USER is not supported by Sentry.");
}
@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()));
+ throw new UnsupportedFeatureException(
+ "GRANT <privilege> TO GROUP is not supported by Sentry.");
}
@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()));
+ throw new UnsupportedFeatureException(
+ "REVOKE <privilege> FROM GROUP is not supported by Sentry.");
}
@Override
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 e4bdfba..83abe4b 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
@@ -32,6 +32,7 @@ import org.apache.impala.catalog.Role;
import org.apache.impala.common.ImpalaException;
import org.apache.impala.common.InternalException;
import org.apache.impala.common.JniUtil;
+import org.apache.impala.common.UnsupportedFeatureException;
import org.apache.impala.service.FeCatalogManager;
import org.apache.impala.service.FeSupport;
import org.apache.impala.thrift.TCatalogServiceRequestHeader;
@@ -64,8 +65,7 @@ import static org.apache.impala.thrift.TPrincipalType.ROLE;
* The methods here use the authorization data stored in Impalad catalog via
* {@link org.apache.impala.authorization.AuthorizationPolicy}.
*
- * Other non-Impalad operations, such as GRANT, REVOKE will throw
- * {@link UnsupportedOperationException}.
+ * Operations not supported by Sentry will throw an {@link UnsupportedFeatureException}.
*/
public class SentryImpaladAuthorizationManager implements AuthorizationManager {
private static final Logger LOG =
@@ -225,6 +225,9 @@ public class SentryImpaladAuthorizationManager implements AuthorizationManager {
case ROLE:
return catalog_.getOrCreateCatalog().getAuthPolicy().getRolePrivileges(
params.getName(), params.getPrivilege());
+ case GROUP:
+ throw new UnsupportedFeatureException(
+ "SHOW GRANT GROUP is not supported by Sentry.");
default:
throw new InternalException(String.format("Unexpected TPrincipalType: %s",
params.getPrincipal_type()));
diff --git a/fe/src/main/java/org/apache/impala/service/Frontend.java b/fe/src/main/java/org/apache/impala/service/Frontend.java
index 1f5c803..267458b 100644
--- a/fe/src/main/java/org/apache/impala/service/Frontend.java
+++ b/fe/src/main/java/org/apache/impala/service/Frontend.java
@@ -64,17 +64,13 @@ import org.apache.impala.analysis.TruncateStmt;
import org.apache.impala.authorization.AuthorizationChecker;
import org.apache.impala.authorization.AuthorizationConfig;
import org.apache.impala.authorization.AuthorizationManager;
-import org.apache.impala.authorization.AuthorizationProvider;
import org.apache.impala.authorization.AuthorizationFactory;
import org.apache.impala.authorization.ImpalaInternalAdminUser;
import org.apache.impala.authorization.Privilege;
import org.apache.impala.authorization.PrivilegeRequest;
import org.apache.impala.authorization.PrivilegeRequestBuilder;
import org.apache.impala.authorization.User;
-import org.apache.impala.authorization.sentry.SentryAuthorizationChecker;
-import org.apache.impala.authorization.sentry.SentryAuthorizationConfig;
import org.apache.impala.catalog.Catalog;
-import org.apache.impala.catalog.CatalogException;
import org.apache.impala.catalog.Column;
import org.apache.impala.catalog.DatabaseNotFoundException;
import org.apache.impala.catalog.FeCatalog;
@@ -126,7 +122,6 @@ import org.apache.impala.thrift.TCopyTestCaseReq;
import org.apache.impala.thrift.TMetadataOpRequest;
import org.apache.impala.thrift.TPlanExecInfo;
import org.apache.impala.thrift.TPlanFragment;
-import org.apache.impala.thrift.TPrincipalType;
import org.apache.impala.thrift.TQueryCtx;
import org.apache.impala.thrift.TQueryExecRequest;
import org.apache.impala.thrift.TQueryOptions;
@@ -504,7 +499,6 @@ public class Frontend {
ddl.op_type = TCatalogOpType.SHOW_ROLES;
ShowRolesStmt showRolesStmt = (ShowRolesStmt) analysis.getStmt();
ddl.setShow_roles_params(showRolesStmt.toThrift());
- Preconditions.checkState(getAuthzChecker() instanceof SentryAuthorizationChecker);
metadata.setColumns(Arrays.asList(
new TColumn("role_name", Type.STRING.toThrift())));
} else if (analysis.isShowGrantPrincipalStmt()) {
diff --git a/tests/authorization/test_ranger.py b/tests/authorization/test_ranger.py
index cd9ff57..c4bb2fa 100644
--- a/tests/authorization/test_ranger.py
+++ b/tests/authorization/test_ranger.py
@@ -352,3 +352,24 @@ class TestRanger(CustomClusterTestSuite):
def _refresh_authorization(self, client, statement):
if statement is not None:
self.execute_query_expect_success(client, statement)
+
+ @CustomClusterTestSuite.with_args(
+ impalad_args=IMPALAD_ARGS, catalogd_args=CATALOGD_ARGS)
+ def test_unsupported_sql(self):
+ """Tests unsupported SQL statements when running with Ranger."""
+ user = "admin"
+ impala_client = self.create_impala_client()
+ error_msg = "UnsupportedFeatureException: {0} is not supported by Ranger."
+ for statement in [("show roles", error_msg.format("SHOW ROLES")),
+ ("show current roles", error_msg.format("SHOW CURRENT ROLES")),
+ ("create role foo", error_msg.format("CREATE ROLE")),
+ ("drop role foo", error_msg.format("DROP ROLE")),
+ ("grant select on database functional to role foo",
+ error_msg.format("GRANT <privilege> TO ROLE")),
+ ("revoke select on database functional from role foo",
+ error_msg.format("REVOKE <privilege> FROM ROLE")),
+ ("show grant role foo", error_msg.format("SHOW GRANT ROLE")),
+ ("show role grant group foo",
+ error_msg.format("SHOW ROLE GRANT GROUP"))]:
+ result = self.execute_query_expect_failure(impala_client, statement[0], user=user)
+ assert statement[1] in str(result)
diff --git a/tests/authorization/test_sentry.py b/tests/authorization/test_sentry.py
index f650b1d..3a1e3a1 100644
--- a/tests/authorization/test_sentry.py
+++ b/tests/authorization/test_sentry.py
@@ -100,3 +100,25 @@ class TestSentry(CustomClusterTestSuite):
finally:
self.client.execute("drop database {0}".format(unique_name))
self.client.execute("drop role {0}".format(role_name))
+
+ @pytest.mark.execute_serially
+ @CustomClusterTestSuite.with_args(
+ impalad_args="--server_name=server1 --sentry_config={0}".format(SENTRY_CONFIG_FILE),
+ catalogd_args="--sentry_config={0}".format(SENTRY_CONFIG_FILE))
+ def test_unsupported_sql(self):
+ """Tests unsupported SQL statements when running with Sentry."""
+ user = getuser()
+ impala_client = self.create_impala_client()
+ error_msg = "UnsupportedFeatureException: {0} is not supported by Sentry."
+ statements = [("grant select on database functional to user foo",
+ error_msg.format("GRANT <privilege> TO USER")),
+ ("grant select on database functional to group foo",
+ error_msg.format("GRANT <privilege> TO GROUP")),
+ ("revoke select on database functional from user foo",
+ error_msg.format("REVOKE <privilege> FROM USER")),
+ ("revoke select on database functional from group foo",
+ error_msg.format("REVOKE <privilege> FROM GROUP")),
+ ("show grant group foo", error_msg.format("SHOW GRANT GROUP"))]
+ for statement in statements:
+ result = self.execute_query_expect_failure(impala_client, statement[0], user=user)
+ assert statement[1] in str(result)