v1.26.2 TLS issue on latest version, tls: either ServerName or InsecureSkipVerify must be specified in the tls.Config

[d1egoaz]

Problem Description

Hey! some heads up!
We use internally a wrapper on top of Sarama that's using 1.26.1, however an app had the Sarama dependency and dependabot updated Sarama to 1.26.2, it broke the app, I've also tested and I tested with other app and the same error happened.

client/metadata got error from broker -1 while fetching metadata: tls: either ServerName or InsecureSkipVerify must be specified in the tls.Config

It seems related to this PR #1666

ping @bai @dnwe
Could we mark the release as unsafe?

Versions

Sarama	Kafka	Go
1.26.2	2.3.x	1.13.x and 1.14.x

Logs

logs: CLICK ME

{"level":"info","msg":"Initializing new client","time":"2020-05-07T17:32:07Z"}
{"level":"info","msg":"client/metadata fetching metadata for all topics from broker xxx-server-xxx:9093\n","time":"2020-05-07T17:32:07Z"}
{"level":"info","msg":"Using tls","time":"2020-05-07T17:32:07Z"}
{"level":"info","msg":"Connected to broker at xxx-server-xxx:9093 (unregistered)\n","time":"2020-05-07T17:32:07Z"}
{"level":"info","msg":"client/metadata got error from broker -1 while fetching metadata: tls: either ServerName or InsecureSkipVerify must be specified in the tls.Config\n","time":"2020-05-07T17:32:07Z"}
{"level":"info","msg":"Closed connection to broker xxx-server-xxx:9093\n","time":"2020-05-07T17:32:07Z"}
{"level":"info","msg":"client/metadata no available broker to send metadata request to","time":"2020-05-07T17:32:07Z"}
{"level":"info","msg":"client/brokers resurrecting 1 dead seed brokers","time":"2020-05-07T17:32:07Z"}
{"level":"info","msg":"client/metadata retrying after 250ms... (3 attempts remaining)\n","time":"2020-05-07T17:32:07Z"}
{"level":"info","msg":"client/metadata fetching metadata for all topics from broker xxx-server-xxx:9093\n","time":"2020-05-07T17:32:07Z"}
{"level":"info","msg":"Using tls","time":"2020-05-07T17:32:07Z"}

[d1egoaz]

I've updated v1.26.2 as pre-release to stop dependabot to telling people to upgrade Sarama while we work on this issue.

[dnwe]

😢

do we not have any functional tests using TLS? Surprised this passed the builds

[dnwe]

This regression is because tls.DialWithDialer with a nil tls.Config creates a default config whereas tls.Client requires you to give it a non-nil tls.Config?

[d1egoaz]

This is the issue:

tls.DialWithDialer sets the ServerName

[dnwe]

Yeah as per the doc — https://pkg.go.dev/crypto/tls?tab=doc#Client

[d1egoaz]

from: #1666

Note there seems to be an issue with the tls test where the server name was never set so Ive fixed that too.

#1666 (comment)