-
Notifications
You must be signed in to change notification settings - Fork 468
Define domain specific workers in php_server and php blocks
#1509
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
Changes from 21 commits
a165c4e
61bbef4
7fffbc2
60c3e12
97913f8
a22cd19
a4016df
a21d3f4
7718a8e
c7172d2
f955187
e362fd3
00d819f
bc48bdd
0b22d51
c6bcacf
53795c7
958537e
6c39229
e19b7b2
2ff18ba
5fc1edf
2c2f677
3b15199
af18d04
dd5dc9b
c4937ac
401d25d
a444361
6ebea60
fb4e262
9ff4ee2
e648532
af74391
34f3b25
9929383
0bbd4c6
c96f53c
26360fb
39430e8
801e71c
6650045
53e7bc0
78be813
4cc8893
1c414ce
9ccac16
3f8f5ec
a6596c7
d5d2eb3
6f27895
a6b840f
877a6ce
94bce80
73d3266
1c38f0d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,6 +30,9 @@ const defaultDocumentRoot = "public" | |
|
|
||
| var iniError = errors.New("'php_ini' must be in the format: php_ini \"<key>\" \"<value>\"") | ||
|
|
||
| // FrankenPHPModule instances register their workers and FrankenPHPApp reads them at Start() time | ||
| var moduleWorkers = make([]workerConfig, 0) | ||
|
|
||
| func init() { | ||
| caddy.RegisterModule(FrankenPHPApp{}) | ||
| caddy.RegisterModule(FrankenPHPModule{}) | ||
|
|
@@ -45,7 +48,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 +111,8 @@ 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 | ||
| for _, w := range append(f.Workers, moduleWorkers...) { | ||
| opts = append(opts, frankenphp.WithWorkers(w.Name, repl.ReplaceKnown(w.FileName, ""), w.Num, w.Env, w.Watch)) | ||
| } | ||
|
|
||
|
|
@@ -130,14 +134,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 | ||
| moduleWorkers = 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, "./**/*.{php,yaml,yml,twig,env}") | ||
| } else { | ||
| v, err := strconv.Atoi(d.Val()) | ||
| if err != nil { | ||
| return wc, err | ||
| } | ||
|
|
||
| wc.Num = v | ||
| } | ||
| } | ||
|
|
||
| if d.NextArg() { | ||
| return wc, errors.New("FrankenPHP: too many 'worker' arguments: " + d.Val()) | ||
|
henderkes marked this conversation as resolved.
Outdated
|
||
| } | ||
|
|
||
| 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.Atoi(d.Val()) | ||
|
henderkes marked this conversation as resolved.
Outdated
|
||
| if err != nil { | ||
| return wc, err | ||
| } | ||
|
|
||
| wc.Num = 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, "./**/*.{php,yaml,yml,twig,env}") | ||
| } 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() { | ||
|
|
@@ -219,82 +304,10 @@ 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 | ||
| } | ||
| } | ||
|
|
||
| if d.NextArg() { | ||
| return errors.New("FrankenPHP: too many 'worker' arguments: " + d.Val()) | ||
| } | ||
|
|
||
| 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.FileName == "" { | ||
| return errors.New(`the "file" argument must be specified`) | ||
| } | ||
|
|
||
| if frankenphp.EmbeddedAppPath != "" && filepath.IsLocal(wc.FileName) { | ||
| wc.FileName = filepath.Join(frankenphp.EmbeddedAppPath, wc.FileName) | ||
| wc, err := parseWorkerConfig(d) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| if wc.Name == "" { | ||
| // let worker initialization validate if the FileName is valid or not | ||
| name, _ := fastabs.FastAbs(wc.FileName) | ||
|
|
@@ -341,6 +354,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 | ||
|
|
@@ -412,6 +427,10 @@ func (f *FrankenPHPModule) Provision(ctx caddy.Context) error { | |
| } | ||
| } | ||
|
|
||
| if len(f.Workers) > 0 { | ||
| moduleWorkers = append(moduleWorkers, f.Workers...) | ||
| } | ||
|
|
||
| return nil | ||
| } | ||
|
|
||
|
|
@@ -422,7 +441,7 @@ 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) | ||
|
|
||
|
|
@@ -441,12 +460,18 @@ func (f FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ ca | |
| } | ||
| } | ||
|
|
||
| workerNames := make([]string, len(f.Workers)) | ||
| for i, w := range f.Workers { | ||
| workerNames[i] = w.Name | ||
| } | ||
|
|
||
| fr, err := frankenphp.NewRequestWithContext( | ||
| r, | ||
| documentRootOption, | ||
| frankenphp.WithRequestSplitPath(f.SplitPath), | ||
| frankenphp.WithRequestPreparedEnv(env), | ||
| frankenphp.WithOriginalRequest(&origReq), | ||
| frankenphp.WithWorkerNames(workerNames), | ||
| ) | ||
|
|
||
| if err != nil { | ||
|
|
@@ -462,7 +487,7 @@ func (f FrankenPHPModule) ServeHTTP(w http.ResponseWriter, r *http.Request, _ ca | |
|
|
||
| // 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() { | ||
|
|
@@ -494,29 +519,82 @@ func (f *FrankenPHPModule) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { | |
| if !d.NextArg() { | ||
| continue | ||
| } | ||
|
|
||
| v, err := strconv.ParseBool(d.Val()) | ||
| if err != nil { | ||
| return err | ||
| } | ||
| if d.NextArg() { | ||
| return d.ArgErr() | ||
| } | ||
|
|
||
| f.ResolveRootSymlink = &v | ||
|
|
||
| case "worker": | ||
|
henderkes marked this conversation as resolved.
|
||
| 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) { | ||
|
henderkes marked this conversation as resolved.
|
||
| 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) | ||
| } | ||
|
henderkes marked this conversation as resolved.
|
||
|
|
||
| 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 == "" && len(wc.Env) > 0 { | ||
| if len(wc.Env) > 0 { | ||
| envString := "" | ||
| for k, v := range wc.Env { | ||
| envString += k + "=" + v + "," | ||
| } | ||
| envString = strings.TrimSuffix(envString, ",") | ||
| wc.Name += "env:" + envString + "_" + wc.FileName | ||
| } | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it would probably also be enough to just have a
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, together with the prefix that should work. Good idea. I used this however to default to a single worker for different modules if they contain the same environment variables. So instead of moduleWorker1 with 1 thread and moduleWorker2 with 1 thread, there'd be moduleWorker:env:_filename with 2 threads.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think I should refactor the f.Workers array out of FrankenPHPApp and FrankenPHPModule and instead only keep a single instace that's updated from both places. Then I could check for existence of a suitable worker even if it isn't in the name. Or I just don't care about duplicate suitable workers and leave it up to the user to define shared workers in a way that makes sense. Maybe that's the better approach.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And FrankenPHPModule would be responsible for finding a suitable worker, the option would be WithWorker instead of WithWorkerNames.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @AlliBalliBaba Hah, I did it, but it makes tests fail because they can't ensure the config was correctly loaded, as the numbers won't match in the name. I'm therefore going back to the environment strings. It's just an auto-generated name to keep the worker names unique, anyhow. |
||
| if wc.Name != "" && !strings.HasPrefix(wc.Name, "m#") { | ||
| wc.Name = "m#" + wc.Name | ||
| } | ||
|
|
||
| f.Workers = append(f.Workers, 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 | ||
|
|
@@ -753,6 +831,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 | ||
| } | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.