[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15752890#comment-15752890
]
kalyan kumar kalvagadda commented on SENTRY-1546:
-------------------------------------------------
Added code changes to SentryStore to have proper message (where the exceptions
are originated). If this is done we can avoid decorating them later.
I saw issue with exceptions created while role create/drop and made required
changes.
> Generic Policy provides bad error messages for Sentry exceptions
> ----------------------------------------------------------------
>
> Key: SENTRY-1546
> URL: https://issues.apache.org/jira/browse/SENTRY-1546
> Project: Sentry
> Issue Type: Bug
> Affects Versions: 1.8.0
> Reporter: Alexander Kolbasov
> Assignee: kalyan kumar kalvagadda
> Priority: Minor
> Labels: bite-sized
>
> I discovered that when you attempt to create a role that already exists the
> error message you get back from Thrift i just 'Role: foo' which is very
> confusing.
> The reason is that the SentryStore throws
> {code}SentryAlreadyExistsException("Role: " + trimmedRoleName);{code}
> and the generic policy processor passes the message as is:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> final TCreateSentryRoleRequest request) throws TException {
> Response<Void> respose = requestHandle(new RequestHandler<Void>() {
> @Override
> public Response<Void> handle() throws Exception {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(conf, request.getRequestorUserName()));
> store.createRole(request.getComponent(), request.getRoleName(),
> request.getRequestorUserName());
> return new Response<Void>(Status.OK());
> }
> });
> ...
> {code}
> The similar thing is happening for other requests and other Sentry-specific
> exceptions.
> The legacy policy processor does decorate the error a bit:
> {code}
> public TCreateSentryRoleResponse create_sentry_role(
> TCreateSentryRoleRequest request) throws TException {
> final Timer.Context timerContext = sentryMetrics.createRoleTimer.time();
> TCreateSentryRoleResponse response = new TCreateSentryRoleResponse();
> try {
> validateClientVersion(request.getProtocol_version());
> authorize(request.getRequestorUserName(),
> getRequestorGroups(request.getRequestorUserName()));
> sentryStore.createSentryRole(request.getRoleName());
> response.setStatus(Status.OK());
> notificationHandlerInvoker.create_sentry_role(request, response);
> } catch (SentryAlreadyExistsException e) {
> String msg = "Role: " + request + " already exists.";
> LOGGER.error(msg, e);
> response.setStatus(Status.AlreadyExists(msg, e));
> } catch (SentryAccessDeniedException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.AccessDenied(e.getMessage(), e));
> } catch (SentryThriftAPIMismatchException e) {
> LOGGER.error(e.getMessage(), e);
> response.setStatus(Status.THRIFT_VERSION_MISMATCH(e.getMessage(), e));
> } catch (Exception e) {
> String msg = "Unknown error for request: " + request + ", message: " +
> e.getMessage();
> LOGGER.error(msg, e);
> response.setStatus(Status.RuntimeError(msg, e));
> } finally {
> ...
> {code}
> I think that it is better to just put the right message in the exception
> itself and do not decorate it later.
--
This message was sent by Atlassian JIRA
(v6.3.4#6332)
