I added sasl.mechanism, and sasl.jaas.config for each cluster. and fix some bugs [#528, #513,#502,#479,#477,#471] #532
Conversation
|
Thank you for submitting this pull request, however I do not see a valid CLA on file for you. Before we can merge this request please visit https://yahoocla.herokuapp.com/ and agree to the terms. Thanks! 😄 |
|
Done |
|
Thank you @kingmorning! My SSL issues have been solved! |
app/controllers/Cluster.scala
Outdated
| @@ -114,6 +122,8 @@ class Cluster (val messagesApi: MessagesApi, val kafkaManagerContext: KafkaManag | |||
| )(ClusterTuning.apply)(ClusterTuning.unapply) | |||
| ) | |||
| , "securityProtocol" -> nonEmptyText.verifying(validateSecurityProtocol) | |||
| , "saslMechanism" -> nonEmptyText.verifying(validateSASLmechanism) | |||
There was a problem hiding this comment.
why is this field required? If I am not using SASL, then why would we want the user to fill this in.
| @@ -100,7 +101,11 @@ case class KafkaAdminClientActor(config: KafkaAdminClientActorConfig) extends Ba | |||
| } | |||
| props.put(CommonClientConfigs.SECURITY_PROTOCOL_CONFIG, config.clusterContext.config.securityProtocol.stringId) | |||
| props.put(CommonClientConfigs.BOOTSTRAP_SERVERS_CONFIG, brokerListStr) | |||
| log.info(s"Creating admin client with security protocol=${config.clusterContext.config.securityProtocol.stringId} , broker list : $brokerListStr") | |||
| props.put(SaslConfigs.SASL_MECHANISM, config.clusterContext.config.saslMechanism.stringId) | |||
There was a problem hiding this comment.
We should only put sasl properties if cluster is configured for it.
app/kafka/manager/KafkaManager.scala
Outdated
| @@ -298,6 +302,8 @@ class KafkaManager(akkaConfig: Config) extends Logging { | |||
| filterConsumers: Boolean, | |||
| tuning: Option[ClusterTuning], | |||
| securityProtocol: String, | |||
| saslMechanism: String, | |||
There was a problem hiding this comment.
This should be optional. e.g. Option[String]
| @@ -257,6 +262,10 @@ case class KafkaManagedOffsetCache(clusterContext: ClusterContext | |||
| cp => props.putAll(cp) | |||
| } | |||
| props.put(CommonClientConfigs.SECURITY_PROTOCOL_CONFIG, clusterContext.config.securityProtocol.stringId) | |||
| props.put(SaslConfigs.SASL_MECHANISM, clusterContext.config.saslMechanism.stringId) | |||
| if (!clusterContext.config.jaasConfig.isEmpty) { | |||
There was a problem hiding this comment.
We should only put sasl properties if cluster is configured for it.
| @@ -1460,6 +1469,10 @@ class KafkaStateActor(config: KafkaStateActorConfig) extends BaseClusterQueryCom | |||
| val port: Int = broker.endpoints(securityProtocol) | |||
| consumerProperties.put(BOOTSTRAP_SERVERS_CONFIG, s"${broker.host}:$port") | |||
| consumerProperties.put(SECURITY_PROTOCOL_CONFIG, securityProtocol.stringId) | |||
| consumerProperties.put(SaslConfigs.SASL_MECHANISM, kaConfig.clusterContext.config.saslMechanism.stringId) | |||
There was a problem hiding this comment.
We should only put sasl properties if cluster is configured for it.
|
Thanks for your contribution @kingmorning , let's make this change so SASL is optional and not required. |
|
If you need help making the changes, please add me as a collaborator to your forked repo and I can make the changes. |
|
ok，I added you as a collaborator to my forked repo, thank you for this help. |
|
@kingmorning Version 1.3.3.21 has these changes, can you test it out on your cluster and then we can do a release. |
|
@patelh @kingmorning are we still waiting for confirmation to cut a release? |
|
Yes |
|
I can confirm that build 1.3.3.21+ I can now see Kafka consumers listed in the UI using SSL-only enabled clusters. Tested on multiple secured clusters, two+ nodes w/ SSL client auth required. |
|
Hi, can you take a look at this issue: #664 as it might be similar |
|
Linked to: |
Hi
I added sasl.mechanism, and sasl.jaas.config for each cluster. and fix some bugs [#528, #513,#502,#479,#477,#471]
I tested this new functions, mechanism includes PLAIN,GSSAPI,SCRAM-SHA-256,SCRAM-SHA-512.
and sasl.jaas.config is a optional string, such as: "org.apache.kafka.common.security.plain.PlainLoginModule required username=kafka password=kakafkaa;"