diff --git a/CHANGELOG.md b/CHANGELOG.md index a03534246a9..78f697b478a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/runtime/service/src/main/java/org/apache/polaris/service/auth/DefaultAuthenticator.java b/runtime/service/src/main/java/org/apache/polaris/service/auth/DefaultAuthenticator.java index c5a8a06b2b1..0229f95cc7b 100644 --- a/runtime/service/src/main/java/org/apache/polaris/service/auth/DefaultAuthenticator.java +++ b/runtime/service/src/main/java/org/apache/polaris/service/auth/DefaultAuthenticator.java @@ -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()); @@ -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()); @@ -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(), @@ -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. * + *

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 roles, boolean allRolesRequested) {} diff --git a/runtime/service/src/test/java/org/apache/polaris/service/auth/DefaultAuthenticatorTest.java b/runtime/service/src/test/java/org/apache/polaris/service/auth/DefaultAuthenticatorTest.java index 14cb2a5f129..66770ae8d4c 100644 --- a/runtime/service/src/test/java/org/apache/polaris/service/auth/DefaultAuthenticatorTest.java +++ b/runtime/service/src/test/java/org/apache/polaris/service/auth/DefaultAuthenticatorTest.java @@ -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 @@ -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 @@ -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