Network listener changes

[trea]

Hi folks, I hope contribution is welcome. I have been tinkering around with this module. Feel free to accept as-is or remix at will. I'd also be happy to address any feedback or questions.

[mholt]

Thanks, this is definitely interesting. So you've made this into a reverse proxy transport -- can you elaborate on your changes a bit so we know what to expect during a review?

[trea]

Hey Matt! Sure, I'd be happy to add more details around the changes!

1. I bumped the dependency for Tailscale and Caddy itself

2. I changed calls to net.SplitHostPort to caddy.SplitNetworkAddress as the passed in network for those was no longer useful (we know its Tailscale by this point and now we need to know if it's supposed to be TCP or UDP)

3. I embed Tailscale Servers into a Destructor and use a caddy.UsagePool instead of a map protected by mutex, which makes it so that Caddy can destruct those connections as needed, and will use existing connections rather than trying to spin up new ones when configuration is reloaded

4. I added an additional module that acts as a reverse proxy transport to the Tailscale network. It works by:

   • Joining the Tailscale network during the module Provision using the same core mechanism as the other module(s) but with the hard coded (for now?) name of caddy-tsnet-client and the Ephemeral flag set
   • Passing requests into the Tailscale Servers' default configured transport

   To use the reverse proxy transport, something like the following should work:

   :80 {
     reverse_proxy http://somenode.<tsnet>.ts.net {
         transport tailscale
     }
   }
   

   A less abstract example Caddyfile I tested:

{
	debug
}
:8080 {
	reverse_proxy statichello.tail5a2a0.ts.net {
		transport tailscale
	}
}

:80 {
	bind tailscale/statichello
	respond "Hello, world!"
}

Using this Caddyfile should result in requests to http://localhost:8080 returning a 200 OK with "Hello, world!" from the static server that is defined next and bound to the Tailscale network.

[mholt]

(Sorry, was delayed by some things last week.)

That's very cool! I think overall these changes are good, and I will try to do a more formal review soon. I'd love for a member of the TS team to review this if it's something they're willing to accept! (@Xe ? @willnorris ?)

[trea]

No worries, thanks!

[willnorris]

Yeah, these changes look great, and I'm excited to try them out (hopefully tomorrow?). The approach looks good, and I suspect most of my comments will just end up being stylistic (looks like the second commit needs goimports?), and some additional documentation. We also need to think about how you juggle the auth keys if you have multiple of these, or mixed with listeners. I think all this predates OAuth support for Tailscale as well, so there's probably more work to do there anyway independent of this change.

[icb-]

Caddy introduced a breaking change so caddytls.AutomationPolicy field Subjects needs to be SubjectsRaw at command.go line 202 to build against Caddy master. After fixing that, you should be able to build with xcaddy build master --with github.com/tailscale/caddy-tailscale=github.com/trea/caddy-tailscale@network-listener-changes.

[mholt]

Ah, yeah -- that's actually an API surface that I think is referenced only by this project outside our official Caddy repo. (The typical API surface is the JSON field name.) Generally, external code doesn't access that but I guess this is a special case I didn't realize. (Sorry!) But yeah that should be an easy fix 😅

[willnorris]

tested and this works great... this is really cool! Thanks for the contribution, and for the patience in me getting around to reviewing it! :) I've got a few minor formatting changes I'll push to your fork and then merge.

[trea]

Oh no worries, I'm glad it all worked out and stoked I was able to contribute! Until next time! 🎉