Repository navigation
[fix][broker] Do not log an error when the tenant does not exist - #26361
Merged
poorbarcode merged 1 commit intoAug 20, 2026
Merged
poorbarcode merged 1 commit into
poorbarcode merged 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Prevents missing-tenant authorization requests from producing erroneous broker ERROR logs while preserving 404 responses and genuine metadata-failure logging.
Changes:
- Limits metadata error handling to tenant retrieval stages.
- Applies equivalent behavior to both authorization providers.
- Adds focused logging and authorization tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
PulsarAuthorizationProvider.java |
Separates missing-tenant rejection from metadata error logging. |
MultiRolesTokenAuthorizationProvider.java |
Applies the same correction for multi-role tokens. |
PulsarAuthorizationProviderTest.java |
Tests 404 behavior and logging levels. |
MultiRolesTokenAuthorizationProviderTest.java |
Tests missing-tenant handling for multi-role authorization. |
LogCapture.java |
Provides scoped Log4j2 event capture for tests. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
poorbarcode
approved these changes
Aug 20, 2026
Technoboy-
pushed a commit
that referenced
this pull request
Aug 26, 2026
) (cherry picked from commit a25ae29)
Technoboy-
pushed a commit
that referenced
this pull request
Aug 26, 2026
) (cherry picked from commit a25ae29)
nodece
pushed a commit
to ascentstream/pulsar
that referenced
this pull request
Aug 28, 2026
…che#26361) (cherry picked from commit a25ae29)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Motivation
PulsarAuthorizationProvider#validateTenantAdminAccessthrowsRestException(404, "Tenant does not exist")from insidethenCompose, and theexceptionallyhandler chained onto that same stage catches it. That handler only special-casesMetadataStoreException.NotFoundException, so the self-thrown 404 falls through tolog.error("Failed to get tenant", cause).Every request naming a tenant that does not exist therefore produces an ERROR line with a stack trace. A lookup is enough to trigger it, so a single misconfigured client pointed at a cluster that does not host its tenant can flood a healthy broker — roughly 1000 ERROR lines in 10 minutes from one client, on a broker with nothing wrong with it.
A tenant that does not exist is a client error, not a broker fault: any client can trigger it by mistyping a tenant name, and
ServerCnxalready reports the rejection at its own level.MultiRolesTokenAuthorizationProvideroverrides the method and carries an identical copy of the bug.Modifications
exceptionallyhandler ontogetTenantAsync(), the stage that can actually fail, and perform the presence check after it. The "tenant does not exist" rejection then has no error handler downstream.MetadataStoreException.NotFoundExceptionto an emptyOptionalso both "tenant is missing" paths converge on the same 404.FutureUtil.unwrapCompletionExceptioninstead ofex.getCause(). Attached directly togetTenantAsync(), the exception is no longer wrapped in aCompletionException.MultiRolesTokenAuthorizationProvider.Genuine metadata store failures are still logged at ERROR. Client-visible behaviour is unchanged: the previous code rethrew
new RestException(cause), which preserved the 404 response, so the status, message andErrorDataentity are identical.Verifying this change
This change added tests and can be verified as follows:
PulsarAuthorizationProviderTestwith three tests: one onvalidateTenantAdminAccess, one onallowTopicOperationAsyncwithTopicOperation.LOOKUP(the path an actual lookup takes), and one asserting that a metadata store failure is still logged at ERROR, which guards the fix from over-reaching.MultiRolesTokenAuthorizationProviderTest.LogCapture, a Log4j2 appender that captures a single logger's events so the tests can assert on the level a condition is reported at.Does this pull request potentially affect one of the following parts:
If the box was checked, please highlight the changes