tower-http icon indicating copy to clipboard operation
tower-http copied to clipboard

Add `Gateway` middleware

Open lilyball opened this issue 3 years ago • 2 comments

This PR primarily adds a new HTTP gateway / reverse proxy middleware. It forwards the updated request to an underlying service so it should work with any appropriate transport such as hyper::Client. It concerns itself with updating the request URL and headers, including generating forwarding headers and stripping hop-by-hop headers.

This gateway implementation was based on RFC 7230 and RFC 7239, along with referencing MDN docs on various headers. Notably I did not look at any existing production gateway implementations beyond briefly skimming the config options for Apache's mod_proxy. I did make some choices in this implementation that could potentially want to be turned into config options, but I didn't want to overwhelm the config right now. These choices include always generating a Forwarded header and generating a Via header if possible, as well as keeping the hop-by-hop headers relating to transfer encoding and trailer headers intact. This gateway also does not modify the response beyond stripping hop-by-hop headers. In particular it does not add any headers, it does not rewrite the Location header, and it does not touch the body.

The gateway implementation has a few doctests, and has some unit tests for pieces of it, but does not currently have any tests for using the gateway itself.

Besides that, this PR also does some initial maintenance tasks. See the individual commits for details.

lilyball avatar Jun 07 '22 00:06 lilyball

First of all thank you for working on this! I've skimmed the code and it looks solid overall.

With that said I have some concerns:

My biggest concern is that this is a large feature, and I'm not very familiar with the finer details of proxies, so I don't think I'd be comfortable maintaining it. I get that a good chunk of it is tests and docs but those also carry a maintenance cost.

If we were to merge this would you be able to commit to maintaining it? That includes fixing bugs, dealing with feature requests, answering questions, etc. If so we can set that up via a code owners file.

I'm general I'm also concerned about adding too many large middleware since it opens up for more breaking changes. How would you feel about putting this in its own crate owned by you?

davidpdrsn avatar Jun 08 '22 21:06 davidpdrsn

The reason I wanted to put this in tower-http is that I work for Apple, which has a rather restrictive OSS policy. Getting approval to submit this upstream to an existing project is far easier than creating a new project. I do agree that with the size of this, it would probably make more sense to have it be in a separate crate, though that also reduces visibility. At the moment I'm not anticipating making breaking changes, any increased flexibility added to this would likely be opt-in and most such changes could be done as middleware on the inner service anyway, but I don't claim to predict the future.

Personally I am willing to maintain this, but if I do so, any technical contributions will require going through the internal OSS approval process and will be subject to delays.

I suppose another option is that since I have dumped this code on you, covered under the MIT license, you could create a new tower-rs/tower-gateway crate and put it there instead if you wanted to. That does still mean the tower-rs organization taking responsibility for it of course, but that would help with your concerns about potential breaking changes. It also means this chunk of code would not be implicitly included by anyone doing --features full.

lilyball avatar Jun 08 '22 21:06 lilyball

I have a use for this service too, so I've gone ahead and forked it into a repo under the MIT license. Happy to move back should a more official upstream exist, but in the meantime it's available here. If there's any interest I can package and publish it to crates.io

jess-sol avatar Nov 08 '22 23:11 jess-sol

I'm sorry for the delay on this but I'm gonna have to close this PR. We don't currently have the bandwidth to take on maintenance of such as large feature. I'm sorry 😕

@jess-sol That is awesome 🎉 Thanks for doing that! If you're up for maintaining it then I encourage you publish it to crates.io.

davidpdrsn avatar Dec 02 '22 11:12 davidpdrsn