Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support tailscale configuration via Caddy configuration #30

Merged
merged 10 commits into from
May 2, 2024
Merged
Show file tree
Hide file tree
Changes from 8 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions Caddyfile
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
{
order tailscale_auth after basicauth
tailscale {
auth_key "tskey-auth-"

caddy {
auth_key "tskey-auth-caddy"
}
}
}

:80 {
Expand Down
68 changes: 68 additions & 0 deletions app.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
package tscaddy

import (
"errors"

"github.com/caddyserver/caddy/v2"
)

func init() {
caddy.RegisterModule(TSApp{})
}

type TSApp struct {
// DefaultAuthKey is the default auth key to use for Tailscale if no other auth key is specified.
DefaultAuthKey string `json:"auth_key,omitempty" caddy:"namespace=tailscale.auth_key"`

Ephemeral bool `json:"ephemeral,omitempty" caddy:"namespace=tailscale.ephemeral"`

Servers map[string]TSServer `json:"servers,omitempty" caddy:"namespace=tailscale"`
}

type TSServer struct {
AuthKey string `json:"auth_key,omitempty" caddy:"namespace=auth_key"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've honestly never used these caddy struct tags before. I guess they're used in doc generation? This seems to be the only one that doesn't have "tailscale." in the namespace. Is that intentional?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was, I think this is the translation for marshaling the caddy file format or going to/from JSON. It was modeled on the dynamicdns caddy app.


Ephemeral bool `json:"ephemeral,omitempty" caddy:"namespace=tailscale.ephemeral"`

name string
}

func (TSApp) CaddyModule() caddy.ModuleInfo {
return caddy.ModuleInfo{
ID: "tailscale",
New: func() caddy.Module { return new(TSApp) },
}
}

func (t *TSApp) Provision(ctx caddy.Context) error {
for _, svr := range t.Servers {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is doing anything. Ranging over the t.Servers map is returning copies of the servers, so the changes aren't being stored back. You would need to assign the updated svr value back to the t.Servers map. But you don't really need this at all, since you check the default values in the getAuthKey and getEphemeral funcs.

So I think you can just remove the for loop entirely.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had this to try to make the usage pool for the app easier before I threw it out and never came back to update this here

Thanks!

if t.Ephemeral {
svr.Ephemeral = t.Ephemeral
}
if svr.AuthKey == "" {
svr.AuthKey = t.DefaultAuthKey
}
}

app = t
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the global app value shouldn't be overwritten as part of Provisioning, since the new apps are provisioned and started before the old one is stopped. So this should probably be inside of Start(). We should probaby also make app an atomic.Pointer since we'll have multiple TSApps potentially trying to write to it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point. I didn't realize these apps were started and stopped at the same time.


return nil
}

func (t *TSApp) Validate() error {
if t.DefaultAuthKey == "" {
return errors.New("auth_key must be set")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we want to require that a default auth key is set. A user could instead choose to specify only per-server key. That was actually how I initially tested this, and was surprised by the error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point!

}
return nil
}

func (t *TSApp) Start() error {
return nil
}

func (t *TSApp) Stop() error {
app = nil
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we make app an atomic.Pointer, then this should probably be app.CompareAndSwap(t, nil) so that it only gets set to nil if it is still pointing to the current t.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the apps are started and stopped at the same time would we do this at all?

Could we have a race condition where stop on an old app is called after start on a new one?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had the same thought. For updating configs this isn't really necessary because the new TSApp's Start will overwrite the value before Stop is called. But if the new config removes the Tailscale configuration, we still want to clean this up. The use of atomic and CompareAndSwap handle race conditions.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct me if I'm wrong but atomic will solve concurrent access but not an ordering error where the calls are not guaranteed to always be in the same order?

Copy link
Member

@willnorris willnorris May 3, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, you're absolutely right that atomic is focused on handling concurrent access. It's the CompareAndSwap that helps us here. On Start, we always update the value of our singleton to the new value. But on Stop, we only set it to nil if the value is still the old value. So depending on the order they run...

old.Stop() // this will update tsapp to nil
new.Start() // this will update tsapp to new

new.Start() // this will update tsapp to new
old.Stop() // this won't do anything because tsapp no longer points to old, which is what CompareAndSwap checks for

In the case of the new caddy config not having a tailscale config at all, then only old.Stop() gets called, which will set tsapp to nil and free up the memory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it. Thanks for the explanation!

return nil
}

var _ caddy.App = (*TSApp)(nil)
77 changes: 77 additions & 0 deletions caddyfile.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,77 @@
package tscaddy

import (
"github.com/caddyserver/caddy/v2/caddyconfig"
"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
"github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile"
)

func init() {
httpcaddyfile.RegisterGlobalOption("tailscale", parseApp)
}

func parseApp(d *caddyfile.Dispenser, _ any) (any, error) {
app = &TSApp{
Servers: make(map[string]TSServer),
}
if !d.Next() {
return app, d.ArgErr()

}

for d.NextBlock(0) {
val := d.Val()

switch val {
case "auth_key":
if !d.NextArg() {
return nil, d.ArgErr()
}
app.DefaultAuthKey = d.Val()
case "ephemeral":
app.Ephemeral = true
default:
svr, err := parseServer(d)
if app.Servers == nil {
app.Servers = map[string]TSServer{}
}
if err != nil {
return nil, err
}
app.Servers[svr.name] = svr
}
}

return httpcaddyfile.App{
Name: "tailscale",
Value: caddyconfig.JSON(app, nil),
}, nil
}

func parseServer(d *caddyfile.Dispenser) (TSServer, error) {
name := d.Val()
segment := d.NewFromNextSegment()

if !segment.Next() {
return TSServer{}, d.ArgErr()
}

svr := TSServer{}
svr.name = name
for nesting := segment.Nesting(); segment.NextBlock(nesting); {
val := segment.Val()
switch val {
case "auth_key":
if !segment.NextArg() {
return svr, segment.ArgErr()
}
svr.AuthKey = segment.Val()
case "ephemeral":
svr.Ephemeral = true
default:
return svr, segment.Errf("unrecognized subdirective: %s", segment.Val())
}
}

return svr, nil
}
119 changes: 119 additions & 0 deletions caddyfile_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,119 @@
package tscaddy

import (
"encoding/json"
"testing"

"github.com/caddyserver/caddy/v2"
"github.com/caddyserver/caddy/v2/caddyconfig/caddyfile"
"github.com/caddyserver/caddy/v2/caddyconfig/httpcaddyfile"
"github.com/google/go-cmp/cmp"
)

func Test_ParseApp(t *testing.T) {
tests := []struct {
name string
d *caddyfile.Dispenser
want string
authKey string
wantErr bool
}{
{

name: "empty",
d: caddyfile.NewTestDispenser(`
tailscsale {}
`),
want: `{}`,
},
{
name: "auth_key",
d: caddyfile.NewTestDispenser(`
tailscsale {
auth_key abcdefghijklmnopqrstuvwxyz
}`),
want: `{"auth_key":"abcdefghijklmnopqrstuvwxyz"}`,
authKey: "abcdefghijklmnopqrstuvwxyz",
},
{
name: "ephemeral",
d: caddyfile.NewTestDispenser(`
tailscsale {
ephemeral
}`),
want: `{"ephemeral":true}`,
authKey: "",
},
{
name: "missing auth key",
d: caddyfile.NewTestDispenser(`
tailscsale {
auth_key
}`),
wantErr: true,
},
{
name: "empty server",
d: caddyfile.NewTestDispenser(`
tailscsale {
foo
}`),
want: `{"servers":{"foo":{}}}`,
},
{
name: "tailscale with server",
d: caddyfile.NewTestDispenser(`
tailscsale {
auth_key 1234567890
foo {
auth_key abcdefghijklmnopqrstuvwxyz
}
}`),
want: `{"auth_key":"1234567890","servers":{"foo":{"auth_key":"abcdefghijklmnopqrstuvwxyz"}}}`,
wantErr: false,
authKey: "abcdefghijklmnopqrstuvwxyz",
},
}

for _, testcase := range tests {
t.Run(testcase.name, func(t *testing.T) {
got, err := parseApp(testcase.d, nil)
if err != nil {
if !testcase.wantErr {
t.Errorf("parseApp() error = %v, wantErr %v", err, testcase.wantErr)
return
}
return
}
if testcase.wantErr && err == nil {
t.Errorf("parseApp() err = %v, wantErr %v", err, testcase.wantErr)
return
}
gotJSON := string(got.(httpcaddyfile.App).Value)
if diff := compareJSON(gotJSON, testcase.want, t); diff != "" {
t.Errorf("parseApp() diff(-got +want):\n%s", diff)
}
app := new(TSApp)
if err := json.Unmarshal([]byte(gotJSON), &app); err != nil {
t.Error("failed to unmarshal json into TSApp")
}
if err := app.Provision(caddy.Context{}); err != nil {
t.Error("failed to provision caddy app")
}

})
}

}

func compareJSON(s1, s2 string, t *testing.T) string {
var v1, v2 map[string]any
if err := json.Unmarshal([]byte(s1), &v1); err != nil {
t.Error(err)
}
if err := json.Unmarshal([]byte(s2), &v2); err != nil {
t.Error(err)
}

return cmp.Diff(v1, v2)
}
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ go 1.22.0

require (
github.com/caddyserver/caddy/v2 v2.7.3
github.com/google/go-cmp v0.6.0
go.uber.org/zap v1.26.0
tailscale.com v1.62.0
)
Expand Down Expand Up @@ -62,7 +63,6 @@ require (
github.com/golang/snappy v0.0.4 // indirect
github.com/google/btree v1.1.2 // indirect
github.com/google/cel-go v0.15.1 // indirect
github.com/google/go-cmp v0.6.0 // indirect
github.com/google/nftables v0.1.1-0.20230115205135-9aa6fdf5a28c // indirect
github.com/google/pprof v0.0.0-20230808223545-4887780b67fb // indirect
github.com/google/uuid v1.5.0 // indirect
Expand Down
67 changes: 60 additions & 7 deletions module.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,13 @@ import (

var (
servers = caddy.NewUsagePool()
app *TSApp
)

func init() {
app = &TSApp{
Servers: map[string]TSServer{},
}
caddy.RegisterModule(TailscaleAuth{})
httpcaddyfile.RegisterHandlerDirective("tailscale_auth", parseCaddyfile)
caddy.RegisterNetwork("tailscale", getPlainListener)
Expand All @@ -47,7 +51,10 @@ func getPlainListener(_ context.Context, _ string, addr string, _ net.ListenConf
network = "tcp"
}

return s.Listen(network, ":"+port)
ln := &tsnetServerDestructor{
Server: s.Server,
}
return ln.Listen(network, ":"+port)
}

func getTLSListener(_ context.Context, _ string, addr string, _ net.ListenConfig) (any, error) {
Expand Down Expand Up @@ -105,12 +112,8 @@ func getServer(_, addr string) (*tsnetServerDestructor, error) {
}

if host != "" {
// Set authkey to "TS_AUTHKEY_<HOST>". If empty,
// fall back to "TS_AUTHKEY".
s.AuthKey = os.Getenv("TS_AUTHKEY_" + strings.ToUpper(host))
if s.AuthKey == "" {
s.AuthKey = os.Getenv("TS_AUTHKEY")
}
s.AuthKey = getAuthKey(host, app)
s.Ephemeral = getEphemeral(host, app)

// Set config directory for tsnet. By default, tsnet will use the name of the
// running program, but we include the hostname as well so that a single
Expand All @@ -136,6 +139,33 @@ func getServer(_, addr string) (*tsnetServerDestructor, error) {
return s.(*tsnetServerDestructor), nil
}

func getAuthKey(host string, app *TSApp) string {
svr := app.Servers[host]
if svr.AuthKey != "" {
return svr.AuthKey
}

if app.DefaultAuthKey != "" {
return app.DefaultAuthKey
}

// Set authkey to "TS_AUTHKEY_<HOST>". If empty,
// fall back to "TS_AUTHKEY".
authKey := os.Getenv("TS_AUTHKEY_" + strings.ToUpper(host))
if authKey == "" {
authKey = os.Getenv("TS_AUTHKEY")
}
return authKey
}

func getEphemeral(host string, app *TSApp) bool {
if svr, ok := app.Servers[host]; ok {
return svr.Ephemeral
}

return app.Ephemeral
}

type TailscaleAuth struct {
localclient *tailscale.LocalClient
}
Expand Down Expand Up @@ -234,3 +264,26 @@ type tsnetServerDestructor struct {
func (t tsnetServerDestructor) Destruct() error {
return t.Close()
}

func (t *tsnetServerDestructor) Listen(network string, addr string) (net.Listener, error) {
ln, err := t.Server.Listen(network, addr)
if err != nil {
return nil, err
}
serverListener := &tsnetServerListener{
hostname: t.Hostname,
Listener: ln,
}
return serverListener, nil
}

type tsnetServerListener struct {
hostname string
net.Listener
}

func (t *tsnetServerListener) Close() error {
fmt.Println("Delete", t.hostname)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Drop this and close the actual listener as well

_, err := servers.Delete(t.hostname)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This definitely confused me at first. It looks like it will destruct the server on the first listener to close. I didn't realize this is really just a counter decrement on the usage pool. Maybe add a comment here that this decrements the usage pool for the server, and will close it only when it reaches zero?

return err
}
Loading