Thrift version update and unification

[andrey-dubnik]

Is your feature request related to a problem? Please describe.

Hi

I have a question which came out from the security scan we did recently. Is there a reason on why thrift (github.com/apache/[email protected]) is pinned? It does look like this one is from version 0.10. There is another one used by the ringpop-go (actually 2) one from 0.9.3 and another one from the no longer maintained repo. Latest thrift is 0.15 and beyond 0.13 have some active CVEs. Could you please consider reviewing the thrift versions in those 2 packages?

Here is some dependency graph

go.temporal.io/server github.com/apache/[email protected]
go.temporal.io/server github.com/temporalio/[email protected]
github.com/temporalio/[email protected] github.com/apache/[email protected]
github.com/temporalio/[email protected] github.com/samuel/[email protected]

Describe the solution you'd like

Minimum thrift library version of 0.13, better 0.15

Describe alternatives you've considered

None available

[tminusplus]

For context there has been some effort put into this outside of the Temporal team. Happy to continue pushing along my patches if they are still interested. The update is not trivial and includes breaking changes.

• PR pending on forked tchannel-go for update to v0.13.0 at Update Thrift to v0.13.0 and add Go module support tchannel-go#1.
• Opened an issue with uber/tchannel-go asking them to revisit updating Thrift at Update Thrift dependency to v0.14.0 or higher uber/tchannel-go#844.
• Have a patch ready to update the forked ringpop-go to support v0.13.0 once tchannel-go is updated.

Happy to change my patches to support whatever version the Temporal team would like to use. Likely, v0.13.0 is the most compatible with other Go dependencies, which is useful for mono-repos. But, updating to v0.14.0+ fixes a potential DOS CVE-2020-13949. This CVE does not seem too dangerous but will leave judgement calls up to the team.

I will also be testing these patches on a three-node cluster in the next week or so.

[yiminc]

cc @mcbryde

[tminusplus]

I added a branch to update to v0.15.0, the latest. I will open the PR for review once I have tested the ringpop-go changes, and then tested Temporal in a production environment with it.

temporalio/tchannel-go#2

[tminusplus]

PR is opened temporalio/tchannel-go#2

[tminusplus]

This should be done now with #3250