TLS is (I believe) broken

[mreiferson]

First, the new tls_key and tls_cert support is broken in the Set method:

https://github.com/bitly/go-nsq/blob/e0b1455c6272a9c95bad4a020170846644b992a7/config.go#L332

These cases do not return early, like the other switch cases. You don't do this because of the following if block to load certs if both the tls_key and tls_cert flag are not empty:

https://github.com/bitly/go-nsq/blob/e0b1455c6272a9c95bad4a020170846644b992a7/config.go#L360-L366

One problem is that, no matter what, the first pass through this method (as the config is parsing all opts) necessarily will not have both t.certFile and t.keyFile set and will skip the if condition and return unknown option. The second is that, even if both were loaded, the if condition doesn't return, at which point the method would still return unknown option.

The fix is to make the unknown option error return as default in the switch statement, and return nil as the last line in that method. I would do this myself, however, after testing it, using tls still panics:

goroutine 24 [running]:
runtime.panic(0x71e9c0, 0x926eb3)
    /usr/local/go/src/pkg/runtime/panic.c:279 +0xf5
github.com/bitly/go-nsq.(*Conn).Close(0x0, 0x0, 0x0)
    /mnt/hgfs/twmb/dev/go/src/github.com/bitly/go-nsq/conn.go:175 +0xcb
github.com/bitly/go-nsq.(*Producer).close(0xc208102000)
    /mnt/hgfs/twmb/dev/go/src/github.com/bitly/go-nsq/producer.go:239 +0x88
github.com/bitly/go-nsq.(*Producer).onConnIOError(0xc208102000, 0xc2080bcd80, 0x7f13c7b59da0, 0xc208100240)
    /mnt/hgfs/twmb/dev/go/src/github.com/bitly/go-nsq/producer.go:327 +0x27
github.com/bitly/go-nsq.(*producerConnDelegate).OnIOError(0xc208032138, 0xc2080bcd80, 0x7f13c7b59da0, 0xc208100240)
    /mnt/hgfs/twmb/dev/go/src/github.com/bitly/go-nsq/delegates.go:141 +0x48
github.com/bitly/go-nsq.(*Conn).WriteCommand(0xc2080bcd80, 0xc2080d1c70, 0x0, 0x0)
    /mnt/hgfs/twmb/dev/go/src/github.com/bitly/go-nsq/conn.go:254 +0x1e7
github.com/bitly/go-nsq.(*Conn).identify(0xc2080bcd80, 0x920248, 0x0, 0x0)
    /mnt/hgfs/twmb/dev/go/src/github.com/bitly/go-nsq/conn.go:293 +0xa1e
github.com/bitly/go-nsq.(*Conn).Connect(0xc2080bcd80, 0x9, 0x0, 0x0)
    /mnt/hgfs/twmb/dev/go/src/github.com/bitly/go-nsq/conn.go:149 +0x346
github.com/bitly/go-nsq.(*Producer).connect(0xc208102000, 0x0, 0x0)
    /mnt/hgfs/twmb/dev/go/src/github.com/bitly/go-nsq/producer.go:220 +0x3f4
github.com/bitly/go-nsq.(*Producer).sendCommandAsync(0xc208102000, 0xc2080d1b30, 0xc20811bc80, 0x0, 0x0, 0x0, 0x0, 0x0)
    /mnt/hgfs/twmb/dev/go/src/github.com/bitly/go-nsq/producer.go:182 +0xd7
github.com/bitly/go-nsq.(*Producer).sendCommand(0xc208102000, 0xc2080d1b30, 0x0, 0x0)
    /mnt/hgfs/twmb/dev/go/src/github.com/bitly/go-nsq/producer.go:165 +0x78
github.com/bitly/go-nsq.(*Producer).Publish(0xc208102000, 0x778bd0, 0x9, 0xc2080bca20, 0x105, 0x116, 0x0, 0x0)
    /mnt/hgfs/twmb/dev/go/src/github.com/bitly/go-nsq/producer.go:150 +0x16f
...

Which doesn't happen when I do not use any tls flag (and turn off require-verify with all tls flags in nsqd).

[mreiferson]

This is what I get for writing code entirely untested, I'm sorry.

I'll fix this up, write some tests and make sure this works end-to-end.

Thanks for the report @twmb

[mreiferson]

RFR @jehiah

@twmb I don't think your panic is related, so let's get this in and continue to investigate. Also, you didn't paste the top portion of the panic (the actual error message). Can you paste, please?

[jehiah]

changes look good, but this is failing travis tests against nsqd versions 0.2.24 and 0.2.27

[mreiferson]

OK, this run should pass

[jehiah]

close, but missing on Go 1.0.3. (I think we are ready to cut support there though. Is now the time?)

[mreiferson]

those words make my ❤️ sing

[mreiferson]

I just fixed it actually, we can do that as a separate PR on both repos...