KAFKA-18440: Admin does not convert the AuthorizationException to fatal error in using bootstrap controllers#18435
Conversation
| @@ -280,6 +281,9 @@ public void updateFailed(Throwable exception) { | |||
| if (exception instanceof AuthenticationException) { | |||
There was a problem hiding this comment.
Please add a function isFatalException and add AuthenticationException, AuthorizationException, SecurityDisabledException, UnsupportedEndpointTypeException, UnsupportedForMessageFormatException, UnsupportedVersionException etc.
You can add this function in RequestUtils class.
There was a problem hiding this comment.
Thanks for the review. This Jira would like to focus on fatal exception for metadata call in Admin.
IIRC, The SecurityDisabledException is used in create / delete ACLs and UnsupportedForMessageFormatException is used in produce request, so I don't add them.
Also, in UnsupportedVersionException, it logs different message for bootstrap and controller node, but other exceptions logs same message for different node type, so I don't encapsulate all exceptions in a utility function.
There was a problem hiding this comment.
I understand that this is increasing the scope of the JIRA. What I am trying to suggest here will help us prevent such problems in future for cases where a client retries on a fatal exception. If we have a utility method, we can consolidate the logic of what is considered fatal at one place and existing/future calls to server APIs can use that utility to handle exceptions. It will prevent bugs in future if new authors can use the utility method.
There was a problem hiding this comment.
Thanks for the comment. Yeah, it's good to have a utility function for it. I add it to RequestUtils#isFatalException.
FrankYang0529
force-pushed
the
KAFKA-18440
branch
2 times, most recently
from
6ba95ba to
e9d5260
Compare
…al error in using bootstrap controllers Signed-off-by: PoAn Yang <payang@apache.org>
FrankYang0529
force-pushed
the
KAFKA-18440
branch
from
e9d5260 to
1afabf3
Compare
divijvaidya
added
the
backport-candidate
…ient (apache#18435) Reviewers: Divij Vaidya <diviv@amazon.com>
…emove-metadata-version-methods-for-versions-older-than-3.0 * apache-github/trunk: KAFKA-18340: Change Dockerfile to use log4j2 yaml instead log4j properties (apache#18378) MINOR: fix flaky RemoteLogManagerTest#testStopPartitionsWithDeletion (apache#18474) KAFKA-18311: Enforcing copartitioned topics (4/N) (apache#18397) KAFKA-18308; Update CoordinatorSerde (apache#18455) KAFKA-18440: Convert AuthorizationException to fatal error in AdminClient (apache#18435) KAFKA-17671: Create better documentation for transactions (apache#17454) KAFKA-18304; Introduce json converter generator (apache#18458) MINOR: Clean up classic group tests (apache#18473) KAFKA-18399 Remove ZooKeeper from KafkaApis (2/N): CONTROLLED_SHUTDOWN and ENVELOPE (apache#18422) MINOR: improve StreamThread periodic processing log (apache#18430)
…ient (apache#18435) Reviewers: Divij Vaidya <diviv@amazon.com> (cherry picked from commit 2b7c039)
…ient (apache#18435) Reviewers: Divij Vaidya <diviv@amazon.com> (cherry picked from commit 2b7c039)
|
Hi @divijvaidya, I create another two PRs for 3.9 and 3.8 branches, because there is conflict. 4.0 backport doesn't have conflict, so I don't create one for it. Thanks. |
…ient (apache#18435) Reviewers: Divij Vaidya <diviv@amazon.com>
…ient (apache#18435) Reviewers: Divij Vaidya <diviv@amazon.com>
2 participants
Admin use DescribeClusterRequest to build metadata when using bootstrap controllers, and controller APIs may return ClusterAuthorizationException when users have no "ALTER" permission (see #14306 (comment)).
However, admin does not convert the authorized exception to fatal exception (https://github.com/apache/kafka/blob/trunk/clients/src/main/java/org/apache/kafka/clients/admin/internals/AdminMetadataManager.java#L276), so it keeps sending the request to controller until timeout.
Committer Checklist (excluded from commit message)