diff --git a/caddy/app.go b/caddy/app.go index 81ea5325b9..59c2e5e233 100644 --- a/caddy/app.go +++ b/caddy/app.go @@ -65,6 +65,7 @@ type FrankenPHPApp struct { logger *slog.Logger modules []*FrankenPHPModule usedWorkerNames map[string]bool + httpApp *caddyhttp.App } var errIni = errors.New(`"php_ini" must be in the format: php_ini "" ""`) @@ -86,7 +87,8 @@ func (f *FrankenPHPApp) Provision(ctx caddy.Context) error { f.opts = make([]frankenphp.Option, 0, 7+len(options)) if httpApp, err := ctx.AppIfConfigured("http"); err == nil { - if httpApp.(*caddyhttp.App).Metrics != nil { + f.httpApp = httpApp.(*caddyhttp.App) + if f.httpApp.Metrics != nil { f.metrics = frankenphp.NewPrometheusMetrics(ctx.GetMetricsRegistry()) } } else { @@ -126,7 +128,7 @@ func (f *FrankenPHPApp) Start() error { // register global workers for _, w := range f.Workers { w.FileName = repl.ReplaceKnown(w.FileName, "") - w.Name = f.createUniqueWorkerName(w) + w.Name = f.createUniqueWorkerName(w, "") f.opts = append(f.opts, frankenphp.WithWorkers(w.Name, w.FileName, w.Num, w.toWorkerOptions()...)) } @@ -176,6 +178,7 @@ func (f *FrankenPHPApp) reset() { f.ctx = nil f.metrics = nil f.usedWorkerNames = nil + f.httpApp = nil } // register workers and servers for "php" and "php_server" modules @@ -208,7 +211,8 @@ func (f *FrankenPHPApp) registerModules(repl *caddy.Replacer) error { // register a server instance and its workers for a single caddy module func (f *FrankenPHPApp) registerModule(repl *caddy.Replacer, module *FrankenPHPModule) error { - server, err := frankenphp.NewServer(module.resolvedDocumentRoot, module.SplitPath, module.resolvedEnv, module.logger) + serverName := f.resolveServerName(module) + server, err := frankenphp.NewServer(serverName, module.resolvedDocumentRoot, module.SplitPath, module.resolvedEnv, module.logger) if err != nil { return err } @@ -218,7 +222,7 @@ func (f *FrankenPHPApp) registerModule(repl *caddy.Replacer, module *FrankenPHPM for _, w := range module.Workers { w.FileName = repl.ReplaceKnown(w.FileName, "") - w.Name = f.createUniqueWorkerName(w) + w.Name = f.createUniqueWorkerName(w, serverName) workerOptions := append(w.toWorkerOptions(), frankenphp.WithWorkerServerScope(server)) f.opts = append(f.opts, frankenphp.WithWorkers(w.Name, w.FileName, w.Num, workerOptions...)) } @@ -227,7 +231,9 @@ func (f *FrankenPHPApp) registerModule(repl *caddy.Replacer, module *FrankenPHPM } // avoid name collisions for workers -func (f *FrankenPHPApp) createUniqueWorkerName(wc workerConfig) string { +// on collision, a name is first qualified with the server name +// (":") before falling back to a numeric postfix +func (f *FrankenPHPApp) createUniqueWorkerName(wc workerConfig, serverName string) string { if f.usedWorkerNames == nil { f.usedWorkerNames = make(map[string]bool) } @@ -243,6 +249,11 @@ func (f *FrankenPHPApp) createUniqueWorkerName(wc workerConfig) string { f.usedWorkerNames[name] = true break } + if serverName != "" { + name = serverName + ":" + wc.Name + serverName = "" + continue + } suffix++ name = fmt.Sprintf("%s_%d", wc.Name, suffix) } diff --git a/caddy/config_test.go b/caddy/config_test.go index 12421a576f..04498ecf05 100644 --- a/caddy/config_test.go +++ b/caddy/config_test.go @@ -243,10 +243,10 @@ func TestCreateUniqueWorkerNames(t *testing.T) { names[i] = app.createUniqueWorkerName(workerConfig{ FileName: filename, Name: "custom-worker-name", - }) + }, "") names[i+3] = app.createUniqueWorkerName(workerConfig{ FileName: filename, - }) + }, "") } require.Equal(t, "custom-worker-name", names[0]) @@ -256,3 +256,16 @@ func TestCreateUniqueWorkerNames(t *testing.T) { require.Equal(t, absFileName+"_1", names[4]) require.Equal(t, absFileName+"_2", names[5]) } + +func TestCreateUniqueWorkerNamesQualifiedByServer(t *testing.T) { + app := &FrankenPHPApp{} + wc := workerConfig{FileName: "../testdata/worker-with-env.php", Name: "queue"} + + require.Equal(t, "queue", app.createUniqueWorkerName(wc, "one.example.com")) + // on collision, the name is qualified with the server name + require.Equal(t, "two.example.com:queue", app.createUniqueWorkerName(wc, "two.example.com")) + // when the qualified name is also taken, fall back to the numeric postfix + require.Equal(t, "queue_1", app.createUniqueWorkerName(wc, "two.example.com")) + // workers without a server keep the numeric postfix behavior + require.Equal(t, "queue_2", app.createUniqueWorkerName(wc, "")) +} diff --git a/caddy/servername.go b/caddy/servername.go new file mode 100644 index 0000000000..873c95882b --- /dev/null +++ b/caddy/servername.go @@ -0,0 +1,82 @@ +package caddy + +import ( + "github.com/caddyserver/caddy/v2/modules/caddyhttp" +) + +// resolveServerName picks a stable, human-friendly name for the php_server +// block represented by module, so workers, metrics and logs can be attributed +// to a block (e.g. "api.example.com") instead of an opaque index. Cascade: +// 1. First host of the enclosing route's host matcher. +// 2. First listener address of the http server containing the module. +// +// Returns "" when the http app is not available or the module cannot be +// located in any route tree. +func (f *FrankenPHPApp) resolveServerName(module *FrankenPHPModule) string { + if f.httpApp == nil { + return "" + } + + for _, srv := range f.httpApp.Servers { + if !serverContainsHandler(srv, module) { + continue + } + if h := findHostInRoutes(srv.Routes, module); h != "" { + return h + } + if len(srv.Listen) > 0 { + return srv.Listen[0] + } + } + + return "" +} + +// findHostInRoutes walks routes (recursing into Subroute handlers) to locate +// the route that contains target, then returns the first host of that route's +// host matcher. Returns "" if no enclosing route or no host matcher is found. +func findHostInRoutes(routes caddyhttp.RouteList, target caddyhttp.MiddlewareHandler) string { + for _, route := range routes { + if !routeContainsHandler(route, target) { + continue + } + for _, mset := range route.MatcherSets { + for _, m := range mset { + hp, ok := m.(*caddyhttp.MatchHost) + if !ok || hp == nil || len(*hp) == 0 { + continue + } + return (*hp)[0] + } + } + } + + return "" +} + +func serverContainsHandler(srv *caddyhttp.Server, target caddyhttp.MiddlewareHandler) bool { + for _, route := range srv.Routes { + if routeContainsHandler(route, target) { + return true + } + } + + return false +} + +func routeContainsHandler(route caddyhttp.Route, target caddyhttp.MiddlewareHandler) bool { + for _, h := range route.Handlers { + if h == target { + return true + } + if sub, ok := h.(*caddyhttp.Subroute); ok { + for _, r := range sub.Routes { + if routeContainsHandler(r, target) { + return true + } + } + } + } + + return false +} diff --git a/caddy/servername_test.go b/caddy/servername_test.go new file mode 100644 index 0000000000..4a5149d8e0 --- /dev/null +++ b/caddy/servername_test.go @@ -0,0 +1,60 @@ +package caddy + +import ( + "testing" + + "github.com/caddyserver/caddy/v2/modules/caddyhttp" + "github.com/stretchr/testify/require" +) + +func TestResolveServerName(t *testing.T) { + moduleWithHost := &FrankenPHPModule{} + moduleWithoutHost := &FrankenPHPModule{} + moduleInSubroute := &FrankenPHPModule{} + unroutedModule := &FrankenPHPModule{} + + host := caddyhttp.MatchHost{"api.example.com", "www.example.com"} + + app := &FrankenPHPApp{ + httpApp: &caddyhttp.App{ + Servers: map[string]*caddyhttp.Server{ + "srv0": { + Listen: []string{":8080"}, + Routes: caddyhttp.RouteList{ + { + MatcherSets: caddyhttp.MatcherSets{{&host}}, + Handlers: []caddyhttp.MiddlewareHandler{moduleWithHost}, + }, + { + Handlers: []caddyhttp.MiddlewareHandler{moduleWithoutHost}, + }, + { + Handlers: []caddyhttp.MiddlewareHandler{ + &caddyhttp.Subroute{ + Routes: caddyhttp.RouteList{ + {Handlers: []caddyhttp.MiddlewareHandler{moduleInSubroute}}, + }, + }, + }, + }, + }, + }, + }, + }, + } + + // first host of the enclosing route's host matcher wins + require.Equal(t, "api.example.com", app.resolveServerName(moduleWithHost)) + + // no host matcher: fall back to the first listener address + require.Equal(t, ":8080", app.resolveServerName(moduleWithoutHost)) + + // modules nested in subroutes are found too + require.Equal(t, ":8080", app.resolveServerName(moduleInSubroute)) + + // module not present in any route tree + require.Equal(t, "", app.resolveServerName(unroutedModule)) + + // no http app configured + require.Equal(t, "", (&FrankenPHPApp{}).resolveServerName(moduleWithHost)) +} diff --git a/server.go b/server.go index 16ea5742e0..5eaf4fae56 100644 --- a/server.go +++ b/server.go @@ -4,8 +4,8 @@ import ( "fmt" "log/slog" "net/http" + "strconv" "sync/atomic" - "github.com/dunglas/frankenphp/internal/fastabs" ) @@ -13,6 +13,7 @@ import ( // requests and workers can be scoped to a Server type Server struct { idx int + name string root string splitPath []string env PreparedEnv @@ -46,6 +47,9 @@ func registerServers(newServers []*Server) { for i, s := range servers { s.isRegistered.Store(true) s.idx = i + if s.name == "" { + s.name = strconv.Itoa(i) + } } } @@ -56,7 +60,11 @@ func unregisterServers() { } } -func NewServer(root string, splitPath []string, env map[string]string, logger *slog.Logger) (*Server, error) { +// NewServer creates a Server that can be registered via WithServer(). +// name is a human-readable identifier used to attribute workers, metrics +// and logs to this server; when empty, it defaults to the index the +// server gets at registration time. +func NewServer(name, root string, splitPath []string, env map[string]string, logger *slog.Logger) (*Server, error) { root, err := fastabs.FastAbs(root) if err != nil { return nil, err @@ -67,6 +75,7 @@ func NewServer(root string, splitPath []string, env map[string]string, logger *s } s := &Server{ + name: name, root: root, splitPath: splitPath, env: PrepareEnv(env), @@ -90,6 +99,12 @@ func NewServer(root string, splitPath []string, env map[string]string, logger *s return s, nil } +// Name returns the human-readable name of the server. +// It is empty until registration if none was passed to NewServer(). +func (s *Server) Name() string { + return s.name +} + func (s *Server) addWorker(w *worker) error { s.workers = append(s.workers, w) if w.matchRequest != nil { diff --git a/server_test.go b/server_test.go index 7cbdfc7679..a020de5737 100644 --- a/server_test.go +++ b/server_test.go @@ -42,8 +42,8 @@ func serverGet(t *testing.T, server *frankenphp.Server, url string) string { func TestServer(t *testing.T) { t.Run("idx", func(t *testing.T) { - server1, _ := frankenphp.NewServer(testDataDir, nil, map[string]string{"PHP_SERVER_IDX_1": "1"}, nil) - server2, _ := frankenphp.NewServer(testDataDir, nil, map[string]string{"PHP_SERVER_IDX_2": "2"}, nil) + server1, _ := frankenphp.NewServer("", testDataDir, nil, map[string]string{"PHP_SERVER_IDX_1": "1"}, nil) + server2, _ := frankenphp.NewServer("", testDataDir, nil, map[string]string{"PHP_SERVER_IDX_2": "2"}, nil) initServers(t, frankenphp.WithServer(server1), frankenphp.WithServer(server2)) @@ -56,8 +56,19 @@ func TestServer(t *testing.T) { assert.NotContains(t, body2, "[PHP_SERVER_IDX_1]") }) + t.Run("name", func(t *testing.T) { + named, _ := frankenphp.NewServer("api", testDataDir, nil, nil, nil) + unnamed, _ := frankenphp.NewServer("", testDataDir, nil, nil, nil) + + initServers(t, frankenphp.WithServer(named), frankenphp.WithServer(unnamed)) + + assert.Equal(t, "api", named.Name()) + // an empty name defaults to the server index at registration + assert.Equal(t, "1", unnamed.Name()) + }) + t.Run("root", func(t *testing.T) { - server, _ := frankenphp.NewServer(testDataDir, nil, nil, nil) + server, _ := frankenphp.NewServer("", testDataDir, nil, nil, nil) initServers(t, frankenphp.WithServer(server)) body := serverGet(t, server, "http://example.com/server-globals.php") @@ -67,7 +78,7 @@ func TestServer(t *testing.T) { }) t.Run("env", func(t *testing.T) { - server, _ := frankenphp.NewServer(testDataDir, nil, map[string]string{"TEST_123": "123"}, nil) + server, _ := frankenphp.NewServer("", testDataDir, nil, map[string]string{"TEST_123": "123"}, nil) initServers(t, frankenphp.WithServer(server)) body := serverGet(t, server, "http://example.com/server-variable.php") @@ -76,7 +87,7 @@ func TestServer(t *testing.T) { }) t.Run("split_path", func(t *testing.T) { - server, _ := frankenphp.NewServer(testDataDir, []string{".custom"}, nil, nil) + server, _ := frankenphp.NewServer("", testDataDir, []string{".custom"}, nil, nil) initServers(t, frankenphp.WithServer(server)) body := serverGet(t, server, "http://example.com/split-path.custom/pathinfo") @@ -87,8 +98,8 @@ func TestServer(t *testing.T) { }) t.Run("workers_by_path_and_request_matcher", func(t *testing.T) { - server1, _ := frankenphp.NewServer(testDataDir, nil, nil, nil) - server2, _ := frankenphp.NewServer(testDataDir, nil, nil, nil) + server1, _ := frankenphp.NewServer("", testDataDir, nil, nil, nil) + server2, _ := frankenphp.NewServer("", testDataDir, nil, nil, nil) initServers( t, frankenphp.WithServer(server1), @@ -118,7 +129,7 @@ func TestServer(t *testing.T) { }) t.Run("worker_env_inheritance", func(t *testing.T) { - server, _ := frankenphp.NewServer(testDataDir, nil, map[string]string{ + server, _ := frankenphp.NewServer("", testDataDir, nil, map[string]string{ "FROM_SERVER_ENV": "original", "FROM_WORKER_ENV": "overridden", }, nil) @@ -150,7 +161,7 @@ func TestServer(t *testing.T) { t.Run("error_on_duplicate_worker_filenames", func(t *testing.T) { t.Cleanup(frankenphp.Shutdown) - server, _ := frankenphp.NewServer(testDataDir, nil, nil, nil) + server, _ := frankenphp.NewServer("", testDataDir, nil, nil, nil) err := frankenphp.Init( frankenphp.WithServer(server), frankenphp.WithWorkers("worker1", testDataDir+"worker-with-counter.php", 1, frankenphp.WithWorkerServerScope(server)), @@ -162,14 +173,14 @@ func TestServer(t *testing.T) { }) t.Run("error_on_missing_registration", func(t *testing.T) { - server, _ := frankenphp.NewServer(testDataDir, nil, nil, nil) + server, _ := frankenphp.NewServer("", testDataDir, nil, nil, nil) assert.ErrorIs(t, server.ServeHTTP(nil, nil), frankenphp.ErrNotRunning) }) t.Run("server_logger", func(t *testing.T) { logger, buf := newTestLogger(t) - server, _ := frankenphp.NewServer(testDataDir, nil, nil, logger) + server, _ := frankenphp.NewServer("", testDataDir, nil, nil, logger) initServers(t, frankenphp.WithServer(server)) _ = serverGet(t, server, "http://example.com/log-frankenphp_log.php")