diff --git a/build-static.sh b/build-static.sh index 4de0255275..f5c62474d3 100755 --- a/build-static.sh +++ b/build-static.sh @@ -46,10 +46,9 @@ if [ -z "${SPC_OPT_BUILD_ARGS}" ]; then fi # init spc download additional args if [ -z "${SPC_OPT_DOWNLOAD_ARGS}" ]; then + SPC_OPT_DOWNLOAD_ARGS="--ignore-cache-sources=php-src --retry 5" if [ "${SPC_LIBC}" = "musl" ]; then - SPC_OPT_DOWNLOAD_ARGS="--prefer-pre-built --ignore-cache-sources=php-src" - else - SPC_OPT_DOWNLOAD_ARGS="--ignore-cache-sources=php-src" + SPC_OPT_DOWNLOAD_ARGS="${SPC_OPT_DOWNLOAD_ARGS} --prefer-pre-built" fi fi # if we need debug symbols, disable strip diff --git a/caddy/admin_test.go b/caddy/admin_test.go index 4a970ffda2..4c84c2348b 100644 --- a/caddy/admin_test.go +++ b/caddy/admin_test.go @@ -1,7 +1,10 @@ package caddy_test import ( + "bytes" "encoding/json" + "fmt" + "github.com/dunglas/frankenphp/internal/fastabs" "io" "net/http" "sync" @@ -213,3 +216,82 @@ func getDebugState(t *testing.T, tester *caddytest.Tester) frankenphp.FrankenPHP return debugStates } + +func TestAddModuleWorkerViaAdminApi(t *testing.T) { + // Initialize a server with admin API enabled + tester := caddytest.NewTester(t) + tester.InitServer(` + { + skip_install_trust + admin localhost:2999 + http_port `+testPort+` + + frankenphp + } + + localhost:`+testPort+` { + route { + root ../testdata + php + } + } + `, "caddyfile") + + // Get initial debug state to check number of workers + initialDebugState := getDebugState(t, tester) + initialWorkerCount := 0 + for _, thread := range initialDebugState.ThreadDebugStates { + if thread.Name != "" && thread.Name != "ready" { + initialWorkerCount++ + } + } + + // Create a Caddyfile configuration with a module worker + workerConfig := ` + { + skip_install_trust + admin localhost:2999 + http_port ` + testPort + ` + + frankenphp + } + + localhost:` + testPort + ` { + route { + root ../testdata + php { + worker ../testdata/worker-with-counter.php 1 + } + } + } + ` + + // Send the configuration to the admin API + adminUrl := "http://localhost:2999/load" + r, err := http.NewRequest("POST", adminUrl, bytes.NewBufferString(workerConfig)) + assert.NoError(t, err) + r.Header.Set("Content-Type", "text/caddyfile") + resp := tester.AssertResponseCode(r, http.StatusOK) + defer resp.Body.Close() + + // Get the updated debug state to check if the worker was added + updatedDebugState := getDebugState(t, tester) + updatedWorkerCount := 0 + workerFound := false + filename, _ := fastabs.FastAbs("../testdata/worker-with-counter.php") + for _, thread := range updatedDebugState.ThreadDebugStates { + if thread.Name != "" && thread.Name != "ready" { + updatedWorkerCount++ + if thread.Name == "Worker PHP Thread - "+filename { + workerFound = true + } + } + } + + // Assert that the worker was added + assert.Greater(t, updatedWorkerCount, initialWorkerCount, "Worker count should have increased") + assert.True(t, workerFound, fmt.Sprintf("Worker with name %q should be found", "Worker PHP Thread - "+filename)) + + // Make a request to the worker to verify it's working + tester.AssertGetResponse("http://localhost:"+testPort+"/worker-with-counter.php", http.StatusOK, "requests:1") +} diff --git a/caddy/caddy.go b/caddy/caddy.go index 6a0734bc63..5a54ca8e85 100644 --- a/caddy/caddy.go +++ b/caddy/caddy.go @@ -26,10 +26,17 @@ import ( "github.com/dunglas/frankenphp" ) -const defaultDocumentRoot = "public" +const ( + defaultDocumentRoot = "public" + defaultWatchPattern = "./**/*.{php,yaml,yml,twig,env}" +) var iniError = errors.New("'php_ini' must be in the format: php_ini \"\" \"\"") +// FrankenPHPModule instances register their workers, and FrankenPHPApp reads them at Start() time. +// FrankenPHPApp.Workers may be set by JSON config, so keep them separate. +var moduleWorkerConfigs []workerConfig + func init() { caddy.RegisterModule(FrankenPHPApp{}) caddy.RegisterModule(FrankenPHPModule{}) @@ -45,7 +52,7 @@ func init() { } type workerConfig struct { - // Name for the worker + // Name for the worker. Default: the filename for FrankenPHPApp workers, always prefixed with "m#" for FrankenPHPModule workers. Name string `json:"name,omitempty"` // FileName sets the path to the worker script. FileName string `json:"file_name,omitempty"` @@ -108,7 +115,9 @@ func (f *FrankenPHPApp) Start() error { frankenphp.WithPhpIni(f.PhpIni), frankenphp.WithMaxWaitTime(f.MaxWaitTime), } - for _, w := range f.Workers { + // Add workers from FrankenPHPApp and FrankenPHPModule configurations + // f.Workers may have been set by JSON config, so keep them separate + for _, w := range append(f.Workers, moduleWorkerConfigs...) { opts = append(opts, frankenphp.WithWorkers(w.Name, repl.ReplaceKnown(w.FileName, ""), w.Num, w.Env, w.Watch)) } @@ -130,14 +139,95 @@ func (f *FrankenPHPApp) Stop() error { frankenphp.DrainWorkers() } - // reset configuration so it doesn't bleed into later tests + // reset the configuration so it doesn't bleed into later tests f.Workers = nil f.NumThreads = 0 f.MaxWaitTime = 0 + moduleWorkerConfigs = nil return nil } +func parseWorkerConfig(d *caddyfile.Dispenser) (workerConfig, error) { + wc := workerConfig{} + if d.NextArg() { + wc.FileName = d.Val() + } + + if d.NextArg() { + if d.Val() == "watch" { + wc.Watch = append(wc.Watch, defaultWatchPattern) + } else { + v, err := strconv.ParseUint(d.Val(), 10, 32) + if err != nil { + return wc, err + } + + wc.Num = int(v) + } + } + + if d.NextArg() { + return wc, errors.New(`FrankenPHP: too many "worker" arguments: ` + d.Val()) + } + + for d.NextBlock(1) { + v := d.Val() + switch v { + case "name": + if !d.NextArg() { + return wc, d.ArgErr() + } + wc.Name = d.Val() + case "file": + if !d.NextArg() { + return wc, d.ArgErr() + } + wc.FileName = d.Val() + case "num": + if !d.NextArg() { + return wc, d.ArgErr() + } + + v, err := strconv.ParseUint(d.Val(), 10, 32) + if err != nil { + return wc, err + } + + wc.Num = int(v) + case "env": + args := d.RemainingArgs() + if len(args) != 2 { + return wc, d.ArgErr() + } + if wc.Env == nil { + wc.Env = make(map[string]string) + } + wc.Env[args[0]] = args[1] + case "watch": + if !d.NextArg() { + // the default if the watch directory is left empty: + wc.Watch = append(wc.Watch, defaultWatchPattern) + } else { + wc.Watch = append(wc.Watch, d.Val()) + } + default: + allowedDirectives := "name, file, num, env, watch" + return wc, wrongSubDirectiveError("worker", allowedDirectives, v) + } + } + + if wc.FileName == "" { + return wc, errors.New(`the "file" argument must be specified`) + } + + if frankenphp.EmbeddedAppPath != "" && filepath.IsLocal(wc.FileName) { + wc.FileName = filepath.Join(frankenphp.EmbeddedAppPath, wc.FileName) + } + + return wc, nil +} + // UnmarshalCaddyfile implements caddyfile.Unmarshaler. func (f *FrankenPHPApp) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { for d.Next() { @@ -149,12 +239,12 @@ func (f *FrankenPHPApp) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { return d.ArgErr() } - v, err := strconv.Atoi(d.Val()) + v, err := strconv.ParseUint(d.Val(), 10, 32) if err != nil { return err } - f.NumThreads = v + f.NumThreads = int(v) case "max_threads": if !d.NextArg() { return d.ArgErr() @@ -219,80 +309,29 @@ func (f *FrankenPHPApp) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } case "worker": - wc := workerConfig{} - if d.NextArg() { - wc.FileName = d.Val() - } - - if d.NextArg() { - if d.Val() == "watch" { - wc.Watch = append(wc.Watch, "./**/*.{php,yaml,yml,twig,env}") - } else { - v, err := strconv.Atoi(d.Val()) - if err != nil { - return err - } - - wc.Num = v - } + wc, err := parseWorkerConfig(d) + if err != nil { + return err } - - if d.NextArg() { - return errors.New("FrankenPHP: too many 'worker' arguments: " + d.Val()) + if frankenphp.EmbeddedAppPath != "" && filepath.IsLocal(wc.FileName) { + wc.FileName = filepath.Join(frankenphp.EmbeddedAppPath, wc.FileName) } - - for d.NextBlock(1) { - v := d.Val() - switch v { - case "name": - if !d.NextArg() { - return d.ArgErr() - } - wc.Name = d.Val() - case "file": - if !d.NextArg() { - return d.ArgErr() - } - wc.FileName = d.Val() - case "num": - if !d.NextArg() { - return d.ArgErr() - } - - v, err := strconv.Atoi(d.Val()) - if err != nil { - return err - } - - wc.Num = v - case "env": - args := d.RemainingArgs() - if len(args) != 2 { - return d.ArgErr() - } - if wc.Env == nil { - wc.Env = make(map[string]string) - } - wc.Env[args[0]] = args[1] - case "watch": - if !d.NextArg() { - // the default if the watch directory is left empty: - wc.Watch = append(wc.Watch, "./**/*.{php,yaml,yml,twig,env}") - } else { - wc.Watch = append(wc.Watch, d.Val()) - } - default: - allowedDirectives := "name, file, num, env, watch" - return wrongSubDirectiveError("worker", allowedDirectives, v) + if wc.Name == "" { + // let worker initialization validate if the FileName is valid or not + name, _ := fastabs.FastAbs(wc.FileName) + if name == "" { + name = wc.FileName } + wc.Name = name } - - if wc.FileName == "" { - return errors.New(`the "file" argument must be specified`) + if strings.HasPrefix(wc.Name, "m#") { + return fmt.Errorf(`global worker names must not start with "m#": %q`, wc.Name) } - - if frankenphp.EmbeddedAppPath != "" && filepath.IsLocal(wc.FileName) { - wc.FileName = filepath.Join(frankenphp.EmbeddedAppPath, wc.FileName) + // check for duplicate workers + for _, existingWorker := range f.Workers { + if existingWorker.FileName == wc.FileName { + return fmt.Errorf("global workers must not have duplicate filenames: %q", wc.FileName) + } } f.Workers = append(f.Workers, wc) @@ -332,6 +371,8 @@ type FrankenPHPModule struct { ResolveRootSymlink *bool `json:"resolve_root_symlink,omitempty"` // Env sets an extra environment variable to the given value. Can be specified more than once for multiple environment variables. Env map[string]string `json:"env,omitempty"` + // Workers configures the worker scripts to start. + Workers []workerConfig `json:"workers,omitempty"` resolvedDocumentRoot string preparedEnv frankenphp.PreparedEnv @@ -413,15 +454,21 @@ func needReplacement(s string) bool { // ServeHTTP implements caddyhttp.MiddlewareHandler. // TODO: Expose TLS versions as env vars, as Apache's mod_ssl: https://github.com/caddyserver/caddy/blob/master/modules/caddyhttp/reverseproxy/fastcgi/fastcgi.go#L298 -func (f FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ caddyhttp.Handler) error { +func (f *FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ caddyhttp.Handler) error { origReq := r.Context().Value(caddyhttp.OriginalRequestCtxKey).(http.Request) repl := r.Context().Value(caddy.ReplacerCtxKey).(*caddy.Replacer) var documentRootOption frankenphp.RequestOption + var documentRoot string if f.resolvedDocumentRoot == "" { - documentRootOption = frankenphp.WithRequestDocumentRoot(repl.ReplaceKnown(f.Root, ""), *f.ResolveRootSymlink) + documentRoot = repl.ReplaceKnown(f.Root, "") + if documentRoot == "" && frankenphp.EmbeddedAppPath != "" { + documentRoot = frankenphp.EmbeddedAppPath + } + documentRootOption = frankenphp.WithRequestDocumentRoot(documentRoot, *f.ResolveRootSymlink) } else { - documentRootOption = frankenphp.WithRequestResolvedDocumentRoot(f.resolvedDocumentRoot) + documentRoot = f.resolvedDocumentRoot + documentRootOption = frankenphp.WithRequestResolvedDocumentRoot(documentRoot) } env := f.preparedEnv @@ -432,18 +479,24 @@ func (f FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ ca } } + fullScriptPath, _ := fastabs.FastAbs(documentRoot + "/" + r.URL.Path) + + workerName := "" + for _, w := range f.Workers { + if p, _ := fastabs.FastAbs(w.FileName); p == fullScriptPath { + workerName = w.Name + } + } + fr, err := frankenphp.NewRequestWithContext( r, documentRootOption, frankenphp.WithRequestSplitPath(f.SplitPath), frankenphp.WithRequestPreparedEnv(env), frankenphp.WithOriginalRequest(&origReq), + frankenphp.WithWorkerName(workerName), ) - if err != nil { - return caddyhttp.Error(http.StatusInternalServerError, err) - } - if err = frankenphp.ServeHTTP(w, fr); err != nil { return caddyhttp.Error(http.StatusInternalServerError, err) } @@ -451,9 +504,28 @@ func (f FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ ca return nil } +func generateUniqueModuleWorkerName(filepath string) string { + var i uint + name := "m#" + filepath + +outer: + for { + for _, wc := range moduleWorkerConfigs { + if wc.Name == name { + name = fmt.Sprintf("m#%s_%d", filepath, i) + i++ + + continue outer + } + } + + return name + } +} + // UnmarshalCaddyfile implements caddyfile.Unmarshaler. func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { - // when adding a new directive, also update the allowedDirectives error message + // First pass: Parse all directives except "worker" for d.Next() { for d.NextBlock(0) { switch d.Val() { @@ -485,7 +557,6 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { if !d.NextArg() { continue } - v, err := strconv.ParseBool(d.Val()) if err != nil { return err @@ -493,21 +564,82 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { if d.NextArg() { return d.ArgErr() } - f.ResolveRootSymlink = &v + + case "worker": + for d.NextBlock(1) { + } + for d.NextArg() { + } + // Skip "worker" blocks in the first pass + continue + default: - allowedDirectives := "root, split, env, resolve_root_symlink" + allowedDirectives := "root, split, env, resolve_root_symlink, worker" return wrongSubDirectiveError("php or php_server", allowedDirectives, d.Val()) } } } + // Second pass: Parse only "worker" blocks + d.Reset() + for d.Next() { + for d.NextBlock(0) { + if d.Val() == "worker" { + wc, err := parseWorkerConfig(d) + if err != nil { + return err + } + + // Inherit environment variables from the parent php_server directive + if !filepath.IsAbs(wc.FileName) && f.Root != "" { + wc.FileName = filepath.Join(f.Root, wc.FileName) + } + + if f.Env != nil { + if wc.Env == nil { + wc.Env = make(map[string]string) + } + for k, v := range f.Env { + // Only set if not already defined in the worker + if _, exists := wc.Env[k]; !exists { + wc.Env[k] = v + } + } + } + + if wc.Name == "" { + wc.Name = generateUniqueModuleWorkerName(wc.FileName) + } + if !strings.HasPrefix(wc.Name, "m#") { + wc.Name = "m#" + wc.Name + } + + // Check if a worker with this filename already exists in this module + for _, existingWorker := range f.Workers { + if existingWorker.FileName == wc.FileName { + return fmt.Errorf(`workers in a single "php_server" block must not have duplicate filenames: %q`, wc.FileName) + } + } + // Check if a worker with this name and a different environment or filename already exists + for _, existingWorker := range moduleWorkerConfigs { + if existingWorker.Name == wc.Name { + return fmt.Errorf("workers must not have duplicate names: %q", wc.Name) + } + } + + f.Workers = append(f.Workers, wc) + moduleWorkerConfigs = append(moduleWorkerConfigs, wc) + } + } + } + return nil } // parseCaddyfile unmarshals tokens from h into a new Middleware. func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) { - m := FrankenPHPModule{} + m := &FrankenPHPModule{} err := m.UnmarshalCaddyfile(h.Dispenser) return m, err @@ -744,6 +876,7 @@ func parsePhpServer(h httpcaddyfile.Helper) ([]httpcaddyfile.ConfigValue, error) // using the php directive syntax dispenser.Next() // consume the directive name err = phpsrv.UnmarshalCaddyfile(dispenser) + if err != nil { return nil, err } diff --git a/caddy/caddy_test.go b/caddy/caddy_test.go index 67c91c2029..41a5daa4ef 100644 --- a/caddy/caddy_test.go +++ b/caddy/caddy_test.go @@ -6,6 +6,7 @@ import ( "net/http" "os" "path/filepath" + "strconv" "strings" "sync" "sync/atomic" @@ -119,6 +120,113 @@ func TestWorker(t *testing.T) { wg.Wait() } +func TestGlobalAndModuleWorker(t *testing.T) { + var wg sync.WaitGroup + testPortNum, _ := strconv.Atoi(testPort) + testPortTwo := strconv.Itoa(testPortNum + 1) + tester := caddytest.NewTester(t) + tester.InitServer(` + { + skip_install_trust + admin localhost:2999 + + frankenphp { + worker { + file ../testdata/worker-with-env.php + num 1 + env APP_ENV global + } + } + } + + http://localhost:`+testPort+` { + route { + php { + root ../testdata + worker { + file worker-with-env.php + num 2 + env APP_ENV module + } + } + } + } + + http://localhost:`+testPortTwo+` { + route { + php { + root ../testdata + } + } + } + `, "caddyfile") + + for i := 0; i < 10; i++ { + wg.Add(1) + + go func(i int) { + tester.AssertGetResponse("http://localhost:"+testPort+"/worker-with-env.php", http.StatusOK, "Worker has APP_ENV=module") + tester.AssertGetResponse("http://localhost:"+testPortTwo+"/worker-with-env.php", http.StatusOK, "Worker has APP_ENV=global") + wg.Done() + }(i) + } + wg.Wait() +} + +func TestNamedModuleWorkers(t *testing.T) { + var wg sync.WaitGroup + testPortNum, _ := strconv.Atoi(testPort) + testPortTwo := strconv.Itoa(testPortNum + 1) + tester := caddytest.NewTester(t) + tester.InitServer(` + { + skip_install_trust + admin localhost:2999 + + frankenphp + } + + http://localhost:`+testPort+` { + route { + php { + root ../testdata + worker { + file worker-with-env.php + num 2 + env APP_ENV one + name module1 + } + } + } + } + + http://localhost:`+testPortTwo+` { + route { + php { + root ../testdata + worker { + file worker-with-env.php + num 1 + env APP_ENV two + name module2 + } + } + } + } + `, "caddyfile") + + for i := 0; i < 10; i++ { + wg.Add(1) + + go func(i int) { + tester.AssertGetResponse("http://localhost:"+testPort+"/worker-with-env.php", http.StatusOK, "Worker has APP_ENV=one") + tester.AssertGetResponse("http://localhost:"+testPortTwo+"/worker-with-env.php", http.StatusOK, "Worker has APP_ENV=two") + wg.Done() + }(i) + } + wg.Wait() +} + func TestEnv(t *testing.T) { tester := caddytest.NewTester(t) tester.InitServer(` diff --git a/caddy/config_test.go b/caddy/config_test.go new file mode 100644 index 0000000000..9a4c9aa230 --- /dev/null +++ b/caddy/config_test.go @@ -0,0 +1,274 @@ +package caddy + +import ( + "testing" + + "github.com/caddyserver/caddy/v2/caddyconfig/caddyfile" + "github.com/stretchr/testify/require" +) + +// resetModuleWorkers resets the moduleWorkerConfigs slice for testing +func resetModuleWorkers() { + moduleWorkerConfigs = make([]workerConfig, 0) +} + +func TestModuleWorkerDuplicateFilenamesFail(t *testing.T) { + // Create a test configuration with duplicate worker filenames + configWithDuplicateFilenames := ` + { + php { + worker { + file worker-with-env.php + num 1 + } + worker { + file worker-with-env.php + num 2 + } + } + }` + + // Parse the configuration + d := caddyfile.NewTestDispenser(configWithDuplicateFilenames) + module := &FrankenPHPModule{} + + // Unmarshal the configuration + err := module.UnmarshalCaddyfile(d) + + // Verify that an error was returned + require.Error(t, err, "Expected an error when two workers in the same module have the same filename") + require.Contains(t, err.Error(), "must not have duplicate filenames", "Error message should mention duplicate filenames") + resetModuleWorkers() +} + +func TestModuleWorkersDuplicateNameFail(t *testing.T) { + // Create a test configuration with a worker name + configWithWorkerName1 := ` + { + php_server { + worker { + name test-worker + file ../testdata/worker-with-env.php + num 1 + } + } + }` + + // Parse the first configuration + d1 := caddyfile.NewTestDispenser(configWithWorkerName1) + module1 := &FrankenPHPModule{} + + // Unmarshal the first configuration + err := module1.UnmarshalCaddyfile(d1) + require.NoError(t, err, "First module should be configured without errors") + + // Create a second test configuration with the same worker name + configWithWorkerName2 := ` + { + php_server { + worker { + name test-worker + file ../testdata/worker-with-env.php + num 1 + } + } + }` + + // Parse the second configuration + d2 := caddyfile.NewTestDispenser(configWithWorkerName2) + module2 := &FrankenPHPModule{} + + // Unmarshal the second configuration + err = module2.UnmarshalCaddyfile(d2) + + // Verify that an error was returned + require.Error(t, err, "Expected an error when two workers have the same name, but different environments") + require.Contains(t, err.Error(), "must not have duplicate names", "Error message should mention duplicate names") + resetModuleWorkers() +} + +func TestModuleWorkersWithDifferentFilenames(t *testing.T) { + // Create a test configuration with different worker filenames + configWithDifferentFilenames := ` + { + php { + worker ../testdata/worker-with-env.php + worker ../testdata/worker-with-counter.php + } + }` + + // Parse the configuration + d := caddyfile.NewTestDispenser(configWithDifferentFilenames) + module := &FrankenPHPModule{} + + // Unmarshal the configuration + err := module.UnmarshalCaddyfile(d) + + // Verify that no error was returned + require.NoError(t, err, "Expected no error when two workers in the same module have different filenames") + + // Verify that both workers were added to the module + require.Len(t, module.Workers, 2, "Expected two workers to be added to the module") + require.Equal(t, "../testdata/worker-with-env.php", module.Workers[0].FileName, "First worker should have the correct filename") + require.Equal(t, "../testdata/worker-with-counter.php", module.Workers[1].FileName, "Second worker should have the correct filename") + + resetModuleWorkers() +} + +func TestModuleWorkersDifferentNamesSucceed(t *testing.T) { + // Create a test configuration with a worker name + configWithWorkerName1 := ` + { + php_server { + worker { + name test-worker-1 + file ../testdata/worker-with-env.php + num 1 + } + } + }` + + // Parse the first configuration + d1 := caddyfile.NewTestDispenser(configWithWorkerName1) + module1 := &FrankenPHPModule{} + + // Unmarshal the first configuration + err := module1.UnmarshalCaddyfile(d1) + require.NoError(t, err, "First module should be configured without errors") + + // Create a second test configuration with a different worker name + configWithWorkerName2 := ` + { + php_server { + worker { + name test-worker-2 + file ../testdata/worker-with-env.php + num 1 + } + } + }` + + // Parse the second configuration + d2 := caddyfile.NewTestDispenser(configWithWorkerName2) + module2 := &FrankenPHPModule{} + + // Unmarshal the second configuration + err = module2.UnmarshalCaddyfile(d2) + + // Verify that no error was returned + require.NoError(t, err, "Expected no error when two workers have different names") + + // Verify that both workers were added to moduleWorkerConfigs + require.Len(t, moduleWorkerConfigs, 2, "Expected two workers to be added to moduleWorkerConfigs") + require.Equal(t, "m#test-worker-1", moduleWorkerConfigs[0].Name, "First worker should have the correct name") + require.Equal(t, "m#test-worker-2", moduleWorkerConfigs[1].Name, "Second worker should have the correct name") + + resetModuleWorkers() +} + +func TestModuleWorkerWithEnvironmentVariables(t *testing.T) { + // Create a test configuration with environment variables + configWithEnv := ` + { + php { + worker { + file ../testdata/worker-with-env.php + num 1 + env APP_ENV production + env DEBUG true + } + } + }` + + // Parse the configuration + d := caddyfile.NewTestDispenser(configWithEnv) + module := &FrankenPHPModule{} + + // Unmarshal the configuration + err := module.UnmarshalCaddyfile(d) + + // Verify that no error was returned + require.NoError(t, err, "Expected no error when configuring a worker with environment variables") + + // Verify that the worker was added to the module + require.Len(t, module.Workers, 1, "Expected one worker to be added to the module") + require.Equal(t, "../testdata/worker-with-env.php", module.Workers[0].FileName, "Worker should have the correct filename") + + // Verify that the environment variables were set correctly + require.Len(t, module.Workers[0].Env, 2, "Expected two environment variables") + require.Equal(t, "production", module.Workers[0].Env["APP_ENV"], "APP_ENV should be set to production") + require.Equal(t, "true", module.Workers[0].Env["DEBUG"], "DEBUG should be set to true") + + resetModuleWorkers() +} + +func TestModuleWorkerWithWatchConfiguration(t *testing.T) { + // Create a test configuration with watch directories + configWithWatch := ` + { + php { + worker { + file ../testdata/worker-with-env.php + num 1 + watch + watch ./src/**/*.php + watch ./config/**/*.yaml + } + } + }` + + // Parse the configuration + d := caddyfile.NewTestDispenser(configWithWatch) + module := &FrankenPHPModule{} + + // Unmarshal the configuration + err := module.UnmarshalCaddyfile(d) + + // Verify that no error was returned + require.NoError(t, err, "Expected no error when configuring a worker with watch directories") + + // Verify that the worker was added to the module + require.Len(t, module.Workers, 1, "Expected one worker to be added to the module") + require.Equal(t, "../testdata/worker-with-env.php", module.Workers[0].FileName, "Worker should have the correct filename") + + // Verify that the watch directories were set correctly + require.Len(t, module.Workers[0].Watch, 3, "Expected three watch patterns") + require.Equal(t, "./**/*.{php,yaml,yml,twig,env}", module.Workers[0].Watch[0], "First watch pattern should be the default") + require.Equal(t, "./src/**/*.php", module.Workers[0].Watch[1], "Second watch pattern should match the configuration") + require.Equal(t, "./config/**/*.yaml", module.Workers[0].Watch[2], "Third watch pattern should match the configuration") + + resetModuleWorkers() +} + +func TestModuleWorkerWithCustomName(t *testing.T) { + // Create a test configuration with a custom worker name + configWithCustomName := ` + { + php { + worker { + file ../testdata/worker-with-env.php + num 1 + name custom-worker-name + } + } + }` + + // Parse the configuration + d := caddyfile.NewTestDispenser(configWithCustomName) + module := &FrankenPHPModule{} + + // Unmarshal the configuration + err := module.UnmarshalCaddyfile(d) + + // Verify that no error was returned + require.NoError(t, err, "Expected no error when configuring a worker with a custom name") + + // Verify that the worker was added to the module + require.Len(t, module.Workers, 1, "Expected one worker to be added to the module") + require.Equal(t, "../testdata/worker-with-env.php", module.Workers[0].FileName, "Worker should have the correct filename") + + // Verify that the worker was added to moduleWorkerConfigs with the m# prefix + require.Equal(t, "m#custom-worker-name", module.Workers[0].Name, "Worker should have the custom name") + + resetModuleWorkers() +} diff --git a/caddy/php-server.go b/caddy/php-server.go index 711b9ec3d9..97031dc0eb 100644 --- a/caddy/php-server.go +++ b/caddy/php-server.go @@ -88,12 +88,12 @@ func cmdPHPServer(fs caddycmd.Flags) (int, error) { parts[0] = filepath.Join(frankenphp.EmbeddedAppPath, parts[0]) } - var num int + var num uint64 if len(parts) > 1 { - num, _ = strconv.Atoi(parts[1]) + num, _ = strconv.ParseUint(parts[1], 10, 32) } - workersOption = append(workersOption, workerConfig{FileName: parts[0], Num: num}) + workersOption = append(workersOption, workerConfig{FileName: parts[0], Num: int(num)}) } workersOption[0].Watch = watch } diff --git a/context.go b/context.go index bbfa33b59f..0336e07223 100644 --- a/context.go +++ b/context.go @@ -22,6 +22,7 @@ type frankenPHPContext struct { pathInfo string scriptName string scriptFilename string + workerName string // Whether the request is already closed by us isDone bool @@ -90,7 +91,6 @@ func NewRequestWithContext(r *http.Request, opts ...RequestOption) (*http.Reques // SCRIPT_FILENAME is the absolute path of SCRIPT_NAME fc.scriptFilename = sanitizedPathJoin(fc.documentRoot, fc.scriptName) - c := context.WithValue(r.Context(), contextKey, fc) return r.WithContext(c), nil diff --git a/frankenphp.go b/frankenphp.go index ebd105dd96..eefa7ace36 100644 --- a/frankenphp.go +++ b/frankenphp.go @@ -400,14 +400,13 @@ func ServeHTTP(responseWriter http.ResponseWriter, request *http.Request) error } // Detect if a worker is available to handle this request - if worker, ok := workers[fc.scriptFilename]; ok { + if worker, ok := workers[getWorkerKey(fc.workerName, fc.scriptFilename)]; ok { worker.handleRequest(fc) return nil } - // If no worker was availabe send the request to non-worker threads + // If no worker was available, send the request to non-worker threads handleRequestWithRegularPHPThreads(fc) - return nil } diff --git a/options.go b/options.go index 57c98b20b3..24306d46e6 100644 --- a/options.go +++ b/options.go @@ -54,7 +54,7 @@ func WithMetrics(m Metrics) Option { } } -// WithWorkers configures the PHP workers to start. +// WithWorkers configures the PHP workers to start func WithWorkers(name string, fileName string, num int, env map[string]string, watch []string) Option { return func(o *opt) error { o.workers = append(o.workers, workerOpt{name, fileName, num, PrepareEnv(env), watch}) diff --git a/phpmainthread_test.go b/phpmainthread_test.go index 456702bbdf..08020ea625 100644 --- a/phpmainthread_test.go +++ b/phpmainthread_test.go @@ -32,6 +32,7 @@ func TestStartAndStopTheMainThreadWithOneInactiveThread(t *testing.T) { } func TestTransitionRegularThreadToWorkerThread(t *testing.T) { + workers = nil logger = slog.New(slog.NewTextHandler(io.Discard, nil)) _, err := initPHPThreads(1, 1, nil) assert.NoError(t, err) @@ -56,6 +57,7 @@ func TestTransitionRegularThreadToWorkerThread(t *testing.T) { } func TestTransitionAThreadBetween2DifferentWorkers(t *testing.T) { + workers = nil logger = slog.New(slog.NewTextHandler(io.Discard, nil)) _, err := initPHPThreads(1, 1, nil) assert.NoError(t, err) @@ -156,6 +158,7 @@ func TestAllCommonHeadersAreCorrect(t *testing.T) { } } func TestFinishBootingAWorkerScript(t *testing.T) { + workers = nil logger = slog.New(slog.NewTextHandler(io.Discard, nil)) _, err := initPHPThreads(1, 1, nil) assert.NoError(t, err) @@ -183,16 +186,16 @@ func TestReturnAnErrorIf2WorkersHaveTheSameFileName(t *testing.T) { _, err2 := newWorker(workerOpt{fileName: "filename.php"}) assert.NoError(t, err1) - assert.Error(t, err2, "2 workers cannot have the same filename") + assert.Error(t, err2, "two workers cannot have the same filename") } -func TestReturnAnErrorIf2WorkersHaveTheSameName(t *testing.T) { +func TestReturnAnErrorIf2ModuleWorkersHaveTheSameName(t *testing.T) { workers = make(map[string]*worker) _, err1 := newWorker(workerOpt{fileName: "filename.php", name: "workername"}) _, err2 := newWorker(workerOpt{fileName: "filename2.php", name: "workername"}) assert.NoError(t, err1) - assert.Error(t, err2, "2 workers cannot have the same name") + assert.Error(t, err2, "two workers cannot have the same name") } func getDummyWorker(fileName string) *worker { diff --git a/request_options.go b/request_options.go index ba1b1d5413..6c1cb59491 100644 --- a/request_options.go +++ b/request_options.go @@ -123,3 +123,12 @@ func WithRequestLogger(logger *slog.Logger) RequestOption { return nil } } + +// WithWorkerName sets the worker that should handle the request +func WithWorkerName(name string) RequestOption { + return func(o *frankenPHPContext) error { + o.workerName = name + + return nil + } +} diff --git a/scaling.go b/scaling.go index 7af2349dd1..ca319a1776 100644 --- a/scaling.go +++ b/scaling.go @@ -154,7 +154,7 @@ func startUpscalingThreads(maxScaledThreads int, scale chan *frankenPHPContext, } // if the request has been stalled long enough, scale - if worker, ok := workers[fc.scriptFilename]; ok { + if worker, ok := workers[getWorkerKey(fc.workerName, fc.scriptFilename)]; ok { scaleWorkerThread(worker) } else { scaleRegularThread() diff --git a/testdata/worker-with-env.php b/testdata/worker-with-env.php new file mode 100644 index 0000000000..a999957e16 --- /dev/null +++ b/testdata/worker-with-env.php @@ -0,0 +1,8 @@ +