cloudfoundry / gorouter

CF Router
Apache License 2.0
441 stars 224 forks source link

Undo the gorouter changes for 100-continue that are no longer needed from go 1.22.5 #431

Open geofffranks opened 1 month ago

geofffranks commented 1 month ago

Proposed Change

Context

In https://github.com/cloudfoundry/gorouter/pull/418, #419, https://github.com/cloudfoundry/gorouter/pull/420, and #424, we worked around an issue in Golang by modifying the reverseproxy logic to make use of Director (for most cases) + Redirector (for 100-continue cases). Now that Golang has fixed the underlying issue, we can back out this workaround.

What

Revert all gorouter code changes (but not tests) from #418, #419, #420 and #424. Ensure that the tests added in #418, #419, #420, and #424 remain + pass.

Acceptance criteria

Tests should still pass

Related links

No response

geofffranks commented 4 weeks ago

FYI beginning work on this

geofffranks commented 3 weeks ago

Ok, the code is mostly done on https://github.com/cloudfoundry/gorouter/tree/revert-proxy-rewrite-divergence. However, our integration test is causing a race condition in Golang's http.Server to be triggered when Flushing buffers. We believe this will be fixed in golang 1.23 as part of other mutexes/locking of the http request/response buffers already done.

Pausing this until gorouter is updated to 1.23, and then resuming after that to make sure everthing is happy again.