Improve TLS support

[jehiah]

We need to make a few changes to improve go-nsq support for TLS.

The default validation for TLS connections should be set to ServerName, and that should validate the broadcast_address used from lookupd, or the hostname of a direct nsqd connection against the certificate.

We also need to expose options for setting InsecureSkipVerify and the CA root so that they can be accessed via the config Set(key, value) interface.

cc: @mreiferson

[jehiah]

@mreiferson thoughts on this approach?

[mreiferson]

It's interesting.

But in this case, for these options, it's not actually necessary to use the plumbing, right?

[jehiah]

We can choose not to export this stuff; I didn't like the code structure of dumping everything in Set() which read struct tags so that was my motivation. I thought it would/could be useful to treat this as a generic interface for defining higher order settings keys that did a bunch of lower level settings.

[jehiah]

Validated this works. Thoughts on exposing this api for doing future translations for reader options, or do you want to land this w/o exporting that api?

[mreiferson]

I agree that the super-long Set method isn't great, but this API is weird because the ConfigHandler ends up receiving the Config object and having intimate knowledge of its layout anyway (setting properties on it directly).

When we had been discussing config hooks a few weeks ago, I really meant it as a notification mechanism, not necessarily as a refactoring of how the Config object was modified internally.

[jehiah]

There isn't actually any intimate knowledge of the Config object in this setup; it's only setting the exported TlsConfig.

[mreiferson]

Ok, it isn't "intimate".

I'm just not sure how this is actually an improvement for this use case. Don't get me wrong, I like the idea of some sort of "notification" when values are changed, but we ended up going in a different direction.

Just feels like overkill. More importantly, it's inconsistent with how all the other values are set. If we were to go in this direction they should all follow suit. However, I anticipate that the end result wouldn't really be any cleaner or easier to understand.

[jehiah]

per offline conversation. the interface for config handlers was unexported and will be re-exported in a later change that moves struct tag handling to use that interface.

🍔