[jira] [Updated] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Alexander Kolbasov updated SENTRY-1546:
---
Fix Version/s: sentry-ha-redesign
> 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, sentry-ha-redesign
>Reporter: Alexander Kolbasov
>Assignee: kalyan kumar kalvagadda
>Priority: Minor
> Labels: bite-sized
> Fix For: 1.8.0, sentry-ha-redesign
>
> Attachments: SENTRY-1546.001.patch,
> SENTRY-1546.001-sentry-ha-redesign.patch, SENTRY-1546.002.patch,
> SENTRY-1546.003.patch, SENTRY-1546.005.patch
>
>
> 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 respose = requestHandle(new RequestHandler() {
> @Override
> public Response 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(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.15#6346)
[jira] [Updated] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
kalyan kumar kalvagadda updated SENTRY-1546:
Attachment: SENTRY-1546.001-sentry-ha-redesign.patch
> 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, sentry-ha-redesign
>Reporter: Alexander Kolbasov
>Assignee: kalyan kumar kalvagadda
>Priority: Minor
> Labels: bite-sized
> Fix For: 1.8.0
>
> Attachments: SENTRY-1546.001.patch,
> SENTRY-1546.001-sentry-ha-redesign.patch, SENTRY-1546.002.patch,
> SENTRY-1546.003.patch, SENTRY-1546.005.patch
>
>
> 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 respose = requestHandle(new RequestHandler() {
> @Override
> public Response 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(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.15#6346)
[jira] [Updated] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Alexander Kolbasov updated SENTRY-1546:
---
Resolution: Fixed
Fix Version/s: 1.8.0
Status: Resolved (was: Patch Available)
[~kkalyan] Thank you for your contribution!
> 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, sentry-ha-redesign
>Reporter: Alexander Kolbasov
>Assignee: kalyan kumar kalvagadda
>Priority: Minor
> Labels: bite-sized
> Fix For: 1.8.0
>
> Attachments: SENTRY-1546.001.patch, SENTRY-1546.002.patch,
> SENTRY-1546.003.patch, SENTRY-1546.005.patch
>
>
> 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 respose = requestHandle(new RequestHandler() {
> @Override
> public Response 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(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.15#6346)
[jira] [Updated] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
kalyan kumar kalvagadda updated SENTRY-1546:
Status: Patch Available (was: In Progress)
> 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, sentry-ha-redesign
>Reporter: Alexander Kolbasov
>Assignee: kalyan kumar kalvagadda
>Priority: Minor
> Labels: bite-sized
> Attachments: SENTRY-1546.001.patch, SENTRY-1546.002.patch,
> SENTRY-1546.003.patch, SENTRY-1546.005.patch
>
>
> 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 respose = requestHandle(new RequestHandler() {
> @Override
> public Response 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(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.15#6346)
[jira] [Updated] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
kalyan kumar kalvagadda updated SENTRY-1546:
Attachment: SENTRY-1546.005.patch
> 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, sentry-ha-redesign
>Reporter: Alexander Kolbasov
>Assignee: kalyan kumar kalvagadda
>Priority: Minor
> Labels: bite-sized
> Attachments: SENTRY-1546.001.patch, SENTRY-1546.002.patch,
> SENTRY-1546.003.patch, SENTRY-1546.005.patch
>
>
> 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 respose = requestHandle(new RequestHandler() {
> @Override
> public Response 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(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.15#6346)
[jira] [Updated] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Alexander Kolbasov updated SENTRY-1546:
---
Affects Version/s: sentry-ha-redesign
> 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, sentry-ha-redesign
>Reporter: Alexander Kolbasov
>Assignee: kalyan kumar kalvagadda
>Priority: Minor
> Labels: bite-sized
> Attachments: SENTRY-1546.001.patch, SENTRY-1546.002.patch,
> SENTRY-1546.003.patch
>
>
> 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 respose = requestHandle(new RequestHandler() {
> @Override
> public Response 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(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.15#6346)
[jira] [Updated] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
kalyan kumar kalvagadda updated SENTRY-1546:
Status: In Progress (was: Patch Available)
> 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
> Attachments: SENTRY-1546.001.patch, SENTRY-1546.002.patch,
> SENTRY-1546.003.patch
>
>
> 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 respose = requestHandle(new RequestHandler() {
> @Override
> public Response 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(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.15#6346)
[jira] [Updated] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
kalyan kumar kalvagadda updated SENTRY-1546:
Attachment: SENTRY-1546.003.patch
> 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
> Attachments: SENTRY-1546.001.patch, SENTRY-1546.002.patch,
> SENTRY-1546.003.patch
>
>
> 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 respose = requestHandle(new RequestHandler() {
> @Override
> public Response 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(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.15#6346)
[jira] [Updated] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
kalyan kumar kalvagadda updated SENTRY-1546:
Attachment: (was: SENTRY-1546.003.patch)
> 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
> Attachments: SENTRY-1546.001.patch, SENTRY-1546.002.patch
>
>
> 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 respose = requestHandle(new RequestHandler() {
> @Override
> public Response 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(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.15#6346)
[jira] [Updated] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
kalyan kumar kalvagadda updated SENTRY-1546:
Attachment: SENTRY-1546.003.patch
Added code changes so that consistent message are observed for
SentryNoSuchObjectException and SentryAlreadyExistsException exceptions
> 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
> Attachments: SENTRY-1546.001.patch, SENTRY-1546.002.patch,
> SENTRY-1546.003.patch
>
>
> 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 respose = requestHandle(new RequestHandler() {
> @Override
> public Response 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(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.15#6346)
[jira] [Updated] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
kalyan kumar kalvagadda updated SENTRY-1546:
Attachment: SENTRY-1546.002.patch
Review comments incorporated.
> 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
> Attachments: SENTRY-1546.001.patch, SENTRY-1546.002.patch
>
>
> 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 respose = requestHandle(new RequestHandler() {
> @Override
> public Response 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(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)
[jira] [Updated] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
kalyan kumar kalvagadda updated SENTRY-1546:
Attachment: SENTRY-1546.001.patch
> 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
> Attachments: SENTRY-1546.001.patch
>
>
> 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 respose = requestHandle(new RequestHandler() {
> @Override
> public Response 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(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)
[jira] [Updated] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
kalyan kumar kalvagadda updated SENTRY-1546:
Status: Patch Available (was: In Progress)
> 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
> Attachments: SENTRY-1546.001.patch
>
>
> 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 respose = requestHandle(new RequestHandler() {
> @Override
> public Response 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(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)
[jira] [Updated] (SENTRY-1546) Generic Policy provides bad error messages for Sentry exceptions
[
https://issues.apache.org/jira/browse/SENTRY-1546?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Alexander Kolbasov updated SENTRY-1546:
---
Labels: bite-sized (was: )
> 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
>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 respose = requestHandle(new RequestHandler() {
> @Override
> public Response 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(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)
