Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LI-HOTFIX] Improve unofficial client log message and add more default software names #202

Open
wants to merge 1 commit into
base: 2.4-li
Choose a base branch
from
Open

[LI-HOTFIX] Improve unofficial client log message and add more default software names #202

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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: 4 additions & 2 deletions core/src/main/scala/kafka/server/KafkaApis.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1696,11 +1696,13 @@ class KafkaApis(val requestChannel: RequestChannel,

if (config.unofficialClientLoggingEnable) {
// Check if the last part of clientSoftwareName (after commitId) is an unexpected software name
val softwareName = apiVersionRequest.data.clientSoftwareName().split("-").last
val softwareNameAndCommit = apiVersionRequest.data.clientSoftwareName()
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems there are couple of our internal services that uses unofficial clients (open source clients), we might want to add those internal service to our expected names as well. But not all of them can be uniquely identified from the client ID. Here's the summary from logging in ei-ltx1 tango cluster:

  • menagerie: uses open source admin client, has string menagerie in principal

2021/09/02 18:00:15.966 WARN [KafkaApis] [data-plane-kafka-request-handler-118] [kafka-server] [] [KafkaApi-10309] received ApiVersionsRequest from user with unofficial client type. clientId clientAddress principal = adminclient-39 /2a04:f547:24:4401:0:0:0:585e User:menagerie:None:ei-ltx1

  • CC -> uses open source producer/consumer which has string CruiseControl in client id; uses open source admin client has string cruise-control in principal

2021/09/02 17:24:36.343 WARN [KafkaApis] [data-plane-kafka-request-handler-2] [kafka-server] [] [KafkaApi-10309] received ApiVersionsRequest from user with unofficial client type. clientId clientAddress principal = CruiseControlMetricsReporter /2a04:f547:24:470d:0:0:0:736c User:kafka:kafka.tango:ei-ltx1

2021/09/02 18:24:22.625 WARN [KafkaApis] [data-plane-kafka-request-handler-118] [kafka-server] [] [KafkaApi-10309] received ApiVersionsRequest from user with unofficial client type. clientId clientAddress principal = adminclient-1 /2a04:f547:24:440c:0:0:0:5b1d User:likafka-cruise-control:None:ei-ltx1

  • Conductor-service: uses open source admin client, has string conductor-service in principal

handler-20] [kafka-server] [] [KafkaApi-10309] received ApiVersionsRequest from user with unofficial client type. clientId clientAddress principal = adminclient-2 /2a04:f547:4a:674:0:0:0:dd1d User:conductor-service:conductor-service.cert1:ei4

  • Kafka-rest -> didn't seem to find any open source clients usage in kafka-rest, need the software name logging in this patch to see where the unofficial client is from; has string tracking-rest in principal

2021/09/02 18:10:13.044 WARN [KafkaApis] [data-plane-kafka-request-handler-99] [kafka-server] [] [KafkaApi-10309] received ApiVersionsRequest from user with unofficial client type. clientId clientAddress principal = tracking-rest-restObserver.producer /2a04:f547:24:4301:0:0:0:3860 User:tracking-rest:tango:ei-ltx1

2021/09/02 18:10:18.601 WARN [KafkaApis] [data-plane-kafka-request-handler-18] [kafka-server] [] [KafkaApi-10309] received ApiVersionsRequest from user with unofficial client type. clientId clientAddress principal = tracking-rest-kafkaTracking.trackingProducer /2a04:f547:24:4511:0:0:0:646b User:tracking-rest:tango:ei-ltx1

So I'm thinking besides the default expected client names from lkc mp (e.g. AvroKafkaProducerFactory etc), we should also add another set of expected service names {cruise-control, tracking-rest, conductor-service, menagerie} and expected client ids (CruiseControl) and not logging for clients from these services?

@radai-rosenblatt @efeg @xiowu0 any thoughts?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all these cases just need to be fixed, instead of allowed.

val softwareName = softwareNameAndCommit.split("-").last
if (!config.expectedClientSoftwareNames.contains(softwareName)) {
val softwareVersion = apiVersionRequest.data.clientSoftwareVersion()
val clientIdentity = request.context.clientId() + " " + request.context.clientAddress() + " " + request.context.principal()
unofficialClientsCache.get(clientIdentity)
warn(s"received ApiVersionsRequest from user with unofficial client type. clientId clientAddress principal = $clientIdentity")
warn(s"received ApiVersionsRequest from user with unofficial client software name. clientSoftwareName = $softwareNameAndCommit. clientSoftwareVersion = $softwareVersion. clientId clientAddress principal = $clientIdentity")
}
}

Expand Down
3 changes: 2 additions & 1 deletion core/src/main/scala/kafka/server/KafkaConfig.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,8 @@ object Defaults {
"AvroKafkaConsumerFactory", "AvroKafkaConsumerFactoryFactory", "AvroKafkaProducerFactory", "AvroKafkaProducerFactoryFactory",
"RawKafkaConsumerFactory", "RawKafkaConsumerFactoryFactory", "RawKafkaProducerFactory", "RawKafkaProducerFactoryFactory",
"AvroKafkaProducerBuilder", "AvroKafkaConsumerBuilder", "RawKafkaProducerBuilder", "RawKafkaConsumerBuilder",
"TrackerProcessorFactory", "TrackingConsumerFactory", "TrackingProducerFactory")
"TrackerProcessorFactory", "TrackingConsumerFactory", "TrackingProducerFactory",
"AdminKafkaClientFactory", "AdminKafkaClientFactoryFactory", "AdminClientBuilder")

/************* Authorizer Configuration ***********/
val AuthorizerClassName = ""
Expand Down