Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,12 @@ request adding CHANGELOG notes for breaking (!) changes and possibly other secti

### Changes

- A metastore failure during authentication now returns a fixed `Service unavailable` message
instead of naming the lookup that failed; the principal lookup previously returned `Unable to
fetch principal entity`. The failing lookup is still named in the server log at `ERROR`, which
operators can match to the client error through the request id, returned by default as
`X-Request-ID` and printed by the default log format as `requestId`.

### Deprecations

### Fixes
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,6 @@ protected PrincipalEntity resolvePrincipalEntity(PolarisCredential credentials)
} catch (Exception e) {
throw metaStoreUnavailable(
e,
"Unable to fetch principal entity",
"Unable to resolve principal entity from credentials, principalName={} principalId={}",
credentials.getPrincipalName(),
credentials.getPrincipalId());
Expand Down Expand Up @@ -264,7 +263,6 @@ protected LoadGrantsResult loadPrincipalGrants(PrincipalEntity principal) {
} catch (Exception e) {
throw metaStoreUnavailable(
e,
"Unable to fetch principal grants",
"Unable to load grants, principalName={} principalId={}",
principal.getName(),
principal.getId());
Expand Down Expand Up @@ -309,7 +307,6 @@ protected LoadGrantsResult loadPrincipalGrants(PrincipalEntity principal) {
} catch (Exception e) {
throw metaStoreUnavailable(
e,
"Unable to fetch securable entity",
"Unable to load securable entity for grant, principalName={} principalId={} "
+ "securableCatalogId={} securableId={}",
principal.getName(),
Expand All @@ -323,19 +320,23 @@ protected LoadGrantsResult loadPrincipalGrants(PrincipalEntity principal) {
* Logs a metastore failure raised during authentication and returns the exception to throw, so
* that a failing backend is reported as a transient condition instead of an internal error.
*
* <p>The caller is not authenticated yet at this point, so the response carries a fixed message
* and only the log names the lookup that failed. The two are matched through the request id,
* returned by default as {@code X-Request-ID} and printed by the default log format as {@code
* requestId}.
*
* @param cause the metastore failure
* @param responseMessage the message returned to the client
* @param logMessage the log message, with SLF4J placeholders for {@code logArgs}
* @param logArgs the values for the placeholders in {@code logMessage}
*/
private static PolarisServiceUnavailableException metaStoreUnavailable(
Exception cause, String responseMessage, String logMessage, Object... logArgs) {
Exception cause, String logMessage, Object... logArgs) {
LOGGER
.atError()
.addKeyValue(StructuredLogKeys.ERR_MSG, cause.getMessage())
.addKeyValue(StructuredLogKeys.STACK_TRACE, Throwables.getStackTraceAsString(cause))
.log(logMessage, logArgs);
return new PolarisServiceUnavailableException(0, "%s", responseMessage);
return new PolarisServiceUnavailableException(0, "Service unavailable");
}

protected record PrincipalRoleSelection(Set<String> roles, boolean allRolesRequested) {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,10 @@ public void testFetchPrincipalThrowsServiceExceptionOnMetastoreException() {
assertThatThrownBy(() -> standaloneAuthenticator.authenticate(identityFor(credentials)))
.isInstanceOfSatisfying(
PolarisServiceUnavailableException.class,
e -> assertThat(e.getRetryAfterSeconds()).isZero());
e -> {
assertThat(e.getMessage()).isEqualTo("Service unavailable");
assertThat(e.getRetryAfterSeconds()).isZero();
});
}

@Test
Expand All @@ -174,7 +177,10 @@ void testLoadGrantsThrowsServiceExceptionOnMetastoreException() {
assertThatThrownBy(() -> standaloneAuthenticator.authenticate(identityFor(credentials)))
.isInstanceOfSatisfying(
PolarisServiceUnavailableException.class,
e -> assertThat(e.getRetryAfterSeconds()).isZero());
e -> {
assertThat(e.getMessage()).isEqualTo("Service unavailable");
assertThat(e.getRetryAfterSeconds()).isZero();
});
}

@Test
Expand Down Expand Up @@ -204,7 +210,10 @@ void testLoadSecurableEntityThrowsServiceExceptionOnMetastoreException() {
assertThatThrownBy(() -> standaloneAuthenticator.authenticate(identityFor(credentials)))
.isInstanceOfSatisfying(
PolarisServiceUnavailableException.class,
e -> assertThat(e.getRetryAfterSeconds()).isZero());
e -> {
assertThat(e.getMessage()).isEqualTo("Service unavailable");
assertThat(e.getRetryAfterSeconds()).isZero();
});
}

@Test
Expand Down
Loading