Breaking change in Apache Thrift NewTSocketFromAddrTimeout

[fightingmonk]

This commit to the Apache Thrift go library adds a socket timeout param to NewTSocketFromAddrTimeout(), which causes a fatal error when compiling osquery-go in transport.go.

# github.com/kolide/osquery-go/transport
/go/src/github.com/kolide/osquery-go/transport/transport.go:31:43: not enough arguments in call to thrift.NewTSocketFromAddrTimeout
    have (*net.UnixAddr, time.Duration)
    want (net.Addr, time.Duration, time.Duration)

I tested on Linux and OSX with go 1.11 and 1.14. Not sure if the Windows version is affected.

The simplest fix looks like using the same timeout value for both the connectTimeout and socketTimeout params, changing transport.go#L31 from:

trans := thrift.NewTSocketFromAddrTimeout(addr, timeout)

to:

trans := thrift.NewTSocketFromAddrTimeout(addr, timeout, timeout)

I'm happy to open a PR for this if it looks reasonable.

[directionless]

Hrm. That commit isn't yet in a released version. So I'd expect module versioning to have handled it.

I wonder if they'll ever cut a new version

[fightingmonk]

I don't get the error when installing the osquery-go module itself, only when following the instructions in the README to build the sample table extension.

I'm not super familiar with dependency versioning in go; do the sample instructions need updated?

[zwass]

I think the instructions need updating. We expect that projects with osquery-go are using go modules, and that should keep the Thrift commit pinned to a specific version.

[fightingmonk]

@zwass Yeah, that looks like the answer. I created this go.mod file in the same dir as my extension and ran go get, and now it compiles cleanly.

require (
    github.com/kolide/osquery-go latest
)

[zwass]

Nevertheless it probably makes sense to upgrade the Thrift version and fix the breaking changes.

[amalone-scwx]

new release thrift release v0.13.0 4 days ago on May 26th, 2020...