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/06/11 20:02:02 UTC
[impala] 05/05: IMPALA-8551: Make the grant/revoke error messages
to be more user friendly
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 666c1be66420468936b6b34f6d1fb8f695a72d7d
Author: Fredy Wijaya <fw...@cloudera.com>
AuthorDate: Wed Jun 5 17:42:54 2019 -0700
IMPALA-8551: Make the grant/revoke error messages to be more user friendly
This patch updates the grant/revoke error messages to be more user
friendly, especially when granting/revoking with an invalid principal.
Testing:
- Added E2E test to test grant/revoke with invalid principal
Change-Id: I8995f5dc88b211cd3af415713802cfeac44fe576
Reviewed-on: http://gerrit.cloudera.org:8080/13525
Reviewed-by: Fredy Wijaya <fw...@cloudera.com>
Tested-by: Impala Public Jenkins <im...@cloudera.com>
---
.../ranger/RangerCatalogdAuthorizationManager.java | 12 ++++-
tests/authorization/test_ranger.py | 54 ++++++++++++++++++++++
2 files changed, 64 insertions(+), 2 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 7bb6aaf..0b296d0 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
@@ -40,6 +40,8 @@ import org.apache.impala.thrift.TShowRolesParams;
import org.apache.impala.thrift.TShowRolesResult;
import org.apache.impala.util.ClassUtil;
import org.apache.ranger.plugin.util.GrantRevokeRequest;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.util.ArrayList;
import java.util.Collections;
@@ -57,6 +59,8 @@ import java.util.function.Supplier;
* Operations not supported by Ranger will throw an {@link UnsupportedFeatureException}.
*/
public class RangerCatalogdAuthorizationManager implements AuthorizationManager {
+ private static final Logger LOG = LoggerFactory.getLogger(
+ RangerCatalogdAuthorizationManager.class);
private static final String AUTHZ_CACHE_INVALIDATION_MARKER = "ranger";
private final Supplier<RangerImpalaPlugin> plugin_;
@@ -171,7 +175,9 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager
}
}
} catch (Exception e) {
- throw new InternalException(e.getMessage());
+ LOG.error("Error granting a privilege in Ranger: ", e);
+ throw new InternalException("Error granting a privilege in Ranger. " +
+ "Ranger error message: " + e.getMessage());
}
}
@@ -184,7 +190,9 @@ public class RangerCatalogdAuthorizationManager implements AuthorizationManager
}
}
} catch (Exception e) {
- throw new InternalException(e.getMessage());
+ LOG.error("Error revoking a privilege in Ranger: ", e);
+ throw new InternalException("Error revoking a privilege in Ranger. " +
+ "Ranger error message: " + e.getMessage());
}
}
diff --git a/tests/authorization/test_ranger.py b/tests/authorization/test_ranger.py
index 83ca5e2..bb1019a 100644
--- a/tests/authorization/test_ranger.py
+++ b/tests/authorization/test_ranger.py
@@ -548,3 +548,57 @@ class TestRanger(CustomClusterTestSuite):
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)
+
+ @CustomClusterTestSuite.with_args(
+ impalad_args=IMPALAD_ARGS, catalogd_args=CATALOGD_ARGS)
+ def test_grant_revoke_invalid_principal(self):
+ """Tests grant/revoke to/from invalid principal should return more readable
+ error messages."""
+ valid_user = "admin"
+ invalid_user = "invalid_user"
+ invalid_group = "invalid_group"
+ # TODO(IMPALA-8640): Create two different Impala clients because the users to
+ # workaround the bug.
+ invalid_impala_client = self.create_impala_client()
+ valid_impala_client = self.create_impala_client()
+ for statement in ["grant select on table functional.alltypes to user {0}"
+ .format(getuser()),
+ "revoke select on table functional.alltypes from user {0}"
+ .format(getuser())]:
+ result = self.execute_query_expect_failure(invalid_impala_client,
+ statement,
+ user=invalid_user)
+ if "grant" in statement:
+ assert "Error granting a privilege in Ranger. Ranger error message: " \
+ "HTTP 403 Error: Grantor user invalid_user doesn't exist" in str(result)
+ else:
+ assert "Error revoking a privilege in Ranger. Ranger error message: " \
+ "HTTP 403 Error: Grantor user invalid_user doesn't exist" in str(result)
+
+ for statement in ["grant select on table functional.alltypes to user {0}"
+ .format(invalid_user),
+ "revoke select on table functional.alltypes from user {0}"
+ .format(invalid_user)]:
+ result = self.execute_query_expect_failure(valid_impala_client,
+ statement,
+ user=valid_user)
+ if "grant" in statement:
+ assert "Error granting a privilege in Ranger. Ranger error message: " \
+ "HTTP 403 Error: Grantee user invalid_user doesn't exist" in str(result)
+ else:
+ assert "Error revoking a privilege in Ranger. Ranger error message: " \
+ "HTTP 403 Error: Grantee user invalid_user doesn't exist" in str(result)
+
+ for statement in ["grant select on table functional.alltypes to group {0}"
+ .format(invalid_group),
+ "revoke select on table functional.alltypes from group {0}"
+ .format(invalid_group)]:
+ result = self.execute_query_expect_failure(valid_impala_client,
+ statement,
+ user=valid_user)
+ if "grant" in statement:
+ assert "Error granting a privilege in Ranger. Ranger error message: " \
+ "HTTP 403 Error: Grantee group invalid_group doesn't exist" in str(result)
+ else:
+ assert "Error revoking a privilege in Ranger. Ranger error message: " \
+ "HTTP 403 Error: Grantee group invalid_group doesn't exist" in str(result)