From 043fe41ab8d39615f346b63c1e51bcf16feda1d8 Mon Sep 17 00:00:00 2001 From: Mohammed Al Sahaf Date: Sun, 18 Aug 2024 12:54:12 +0300 Subject: [PATCH 1/6] ci: don't exit early on error in remote CI machine (#6519) --- .github/workflows/ci.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a006d26b..0c6846fc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -150,6 +150,7 @@ jobs: uses: actions/checkout@v4 - name: Run Tests run: | + set +e mkdir -p ~/.ssh && echo -e "${SSH_KEY//_/\\n}" > ~/.ssh/id_ecdsa && chmod og-rwx ~/.ssh/id_ecdsa # short sha is enough? From 54a0c8f94821f393edec1a1bc9459f2363c95117 Mon Sep 17 00:00:00 2001 From: Jesper Brix Rosenkilde Date: Mon, 19 Aug 2024 18:55:55 +0200 Subject: [PATCH 2/6] reverseproxy: Active health checks request body option (#6520) * Add an option to specify the body used for active health checks * Replacer on request body --- ...reverse_proxy_health_reqbody.caddyfiletest | 40 +++++++++++++++++++ modules/caddyhttp/reverseproxy/caddyfile.go | 31 +++++++++----- .../caddyhttp/reverseproxy/healthchecks.go | 19 +++++++-- 3 files changed, 78 insertions(+), 12 deletions(-) create mode 100644 caddytest/integration/caddyfile_adapt/reverse_proxy_health_reqbody.caddyfiletest diff --git a/caddytest/integration/caddyfile_adapt/reverse_proxy_health_reqbody.caddyfiletest b/caddytest/integration/caddyfile_adapt/reverse_proxy_health_reqbody.caddyfiletest new file mode 100644 index 00000000..ae5a6791 --- /dev/null +++ b/caddytest/integration/caddyfile_adapt/reverse_proxy_health_reqbody.caddyfiletest @@ -0,0 +1,40 @@ +:8884 + +reverse_proxy 127.0.0.1:65535 { + health_uri /health + health_request_body "test body" +} +---------- +{ + "apps": { + "http": { + "servers": { + "srv0": { + "listen": [ + ":8884" + ], + "routes": [ + { + "handle": [ + { + "handler": "reverse_proxy", + "health_checks": { + "active": { + "body": "test body", + "uri": "/health" + } + }, + "upstreams": [ + { + "dial": "127.0.0.1:65535" + } + ] + } + ] + } + ] + } + } + } + } +} diff --git a/modules/caddyhttp/reverseproxy/caddyfile.go b/modules/caddyhttp/reverseproxy/caddyfile.go index 4ca5d0e0..cd0e5d94 100644 --- a/modules/caddyhttp/reverseproxy/caddyfile.go +++ b/modules/caddyhttp/reverseproxy/caddyfile.go @@ -69,19 +69,20 @@ func parseCaddyfile(h httpcaddyfile.Helper) (caddyhttp.MiddlewareHandler, error) // lb_retry_match // // # active health checking -// health_uri -// health_port -// health_interval -// health_passes -// health_fails -// health_timeout -// health_status -// health_body +// health_uri +// health_port +// health_interval +// health_passes +// health_fails +// health_timeout +// health_status +// health_body +// health_method +// health_request_body // health_follow_redirects // health_headers { // [] // } -// health_method // // # passive health checking // fail_duration @@ -425,6 +426,18 @@ func (h *Handler) UnmarshalCaddyfile(d *caddyfile.Dispenser) error { } h.HealthChecks.Active.Method = d.Val() + case "health_request_body": + if !d.NextArg() { + return d.ArgErr() + } + if h.HealthChecks == nil { + h.HealthChecks = new(HealthChecks) + } + if h.HealthChecks.Active == nil { + h.HealthChecks.Active = new(ActiveHealthChecks) + } + h.HealthChecks.Active.Body = d.Val() + case "health_interval": if !d.NextArg() { return d.ArgErr() diff --git a/modules/caddyhttp/reverseproxy/healthchecks.go b/modules/caddyhttp/reverseproxy/healthchecks.go index 3b5a6a3a..efa1dbf0 100644 --- a/modules/caddyhttp/reverseproxy/healthchecks.go +++ b/modules/caddyhttp/reverseproxy/healthchecks.go @@ -24,6 +24,7 @@ import ( "regexp" "runtime/debug" "strconv" + "strings" "time" "go.uber.org/zap" @@ -93,6 +94,9 @@ type ActiveHealthChecks struct { // The HTTP method to use for health checks (default "GET"). Method string `json:"method,omitempty"` + // The body to send with the health check request. + Body string `json:"body,omitempty"` + // Whether to follow HTTP redirects in response to active health checks (default off). FollowRedirects bool `json:"follow_redirects,omitempty"` @@ -396,6 +400,16 @@ func (h *Handler) doActiveHealthCheck(dialInfo DialInfo, hostAddr string, networ u.Path = h.HealthChecks.Active.Path } + // replacer used for both body and headers. Only globals (env vars, system info, etc.) are available + repl := caddy.NewReplacer() + + // if body is provided, create a reader for it, otherwise nil + var requestBody io.Reader + if h.HealthChecks.Active.Body != "" { + // set body, using replacer + requestBody = strings.NewReader(repl.ReplaceAll(h.HealthChecks.Active.Body, "")) + } + // attach dialing information to this request, as well as context values that // may be expected by handlers of this request ctx := h.ctx.Context @@ -403,15 +417,14 @@ func (h *Handler) doActiveHealthCheck(dialInfo DialInfo, hostAddr string, networ ctx = context.WithValue(ctx, caddyhttp.VarsCtxKey, map[string]any{ dialInfoVarKey: dialInfo, }) - req, err := http.NewRequestWithContext(ctx, h.HealthChecks.Active.Method, u.String(), nil) + req, err := http.NewRequestWithContext(ctx, h.HealthChecks.Active.Method, u.String(), requestBody) if err != nil { return fmt.Errorf("making request: %v", err) } ctx = context.WithValue(ctx, caddyhttp.OriginalRequestCtxKey, *req) req = req.WithContext(ctx) - // set headers, using a replacer with only globals (env vars, system info, etc.) - repl := caddy.NewReplacer() + // set headers, using replacer repl.Set("http.reverse_proxy.active.target_upstream", networkAddr) for key, vals := range h.HealthChecks.Active.Headers { key = repl.ReplaceAll(key, "") From 2bb2ecc5498c99d535f5b8f56fb8a4732e818ad3 Mon Sep 17 00:00:00 2001 From: Jens-Uwe Mager Date: Wed, 21 Aug 2024 19:39:20 +0200 Subject: [PATCH 3/6] reverseproxy: Change errors writing the response to warning. (#6532) Most of the errors that can be seen here are write errors due to clients aborting the request from their side. Often seen ones include: * writing: ... write: broken pipe * writing: ... connection timed out * writing: http2: stream closed * writing: timeout... * writing: h3 error... Most of these errors are beyond of the control of caddy on the client side, probably nothing can be done on the server side. It still warrants researching when these errors occur very often, so a change in level from error to warn is better here to not polute the logs with errors in the normal case. --- modules/caddyhttp/reverseproxy/reverseproxy.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/caddyhttp/reverseproxy/reverseproxy.go b/modules/caddyhttp/reverseproxy/reverseproxy.go index 4f97edea..1883ac07 100644 --- a/modules/caddyhttp/reverseproxy/reverseproxy.go +++ b/modules/caddyhttp/reverseproxy/reverseproxy.go @@ -979,7 +979,7 @@ func (h *Handler) finalizeResponse( // we'll just log the error and abort the stream here and panic just as // the standard lib's proxy to propagate the stream error. // see issue https://github.com/caddyserver/caddy/issues/5951 - logger.Error("aborting with incomplete response", zap.Error(err)) + logger.Warn("aborting with incomplete response", zap.Error(err)) // no extra logging from stdlib panic(http.ErrAbortHandler) } From 8ccfedf2bb0a1759aa68d4989f05d9a52ba41a34 Mon Sep 17 00:00:00 2001 From: a Date: Wed, 21 Aug 2024 22:29:42 -0500 Subject: [PATCH 4/6] cmd: Use a factory to create the caddy root command (#6533) Co-authored-by: Francis Lavoie --- cmd/cobra.go | 26 ++++++----- cmd/commandfactory.go | 28 ++++++++++++ cmd/commands.go | 100 ++++++++++++++++++++++-------------------- cmd/main.go | 2 +- 4 files changed, 96 insertions(+), 60 deletions(-) create mode 100644 cmd/commandfactory.go diff --git a/cmd/cobra.go b/cmd/cobra.go index 1a250920..9ecb389e 100644 --- a/cmd/cobra.go +++ b/cmd/cobra.go @@ -8,9 +8,10 @@ import ( "github.com/caddyserver/caddy/v2" ) -var rootCmd = &cobra.Command{ - Use: "caddy", - Long: `Caddy is an extensible server platform written in Go. +var defaultFactory = newRootCommandFactory(func() *cobra.Command { + return &cobra.Command{ + Use: "caddy", + Long: `Caddy is an extensible server platform written in Go. At its core, Caddy merely manages configuration. Modules are plugged in statically at compile-time to provide useful functionality. Caddy's @@ -91,23 +92,26 @@ package installers: https://caddyserver.com/docs/install Instructions for running Caddy in production are also available: https://caddyserver.com/docs/running `, - Example: ` $ caddy run + Example: ` $ caddy run $ caddy run --config caddy.json $ caddy reload --config caddy.json $ caddy stop`, - // kind of annoying to have all the help text printed out if - // caddy has an error provisioning its modules, for instance... - SilenceUsage: true, - Version: onlyVersionText(), -} + // kind of annoying to have all the help text printed out if + // caddy has an error provisioning its modules, for instance... + SilenceUsage: true, + Version: onlyVersionText(), + } +}) const fullDocsFooter = `Full documentation is available at: https://caddyserver.com/docs/command-line` func init() { - rootCmd.SetVersionTemplate("{{.Version}}\n") - rootCmd.SetHelpTemplate(rootCmd.HelpTemplate() + "\n" + fullDocsFooter + "\n") + defaultFactory.Use(func(rootCmd *cobra.Command) { + rootCmd.SetVersionTemplate("{{.Version}}\n") + rootCmd.SetHelpTemplate(rootCmd.HelpTemplate() + "\n" + fullDocsFooter + "\n") + }) } func onlyVersionText() string { diff --git a/cmd/commandfactory.go b/cmd/commandfactory.go new file mode 100644 index 00000000..ac571a21 --- /dev/null +++ b/cmd/commandfactory.go @@ -0,0 +1,28 @@ +package caddycmd + +import ( + "github.com/spf13/cobra" +) + +type rootCommandFactory struct { + constructor func() *cobra.Command + options []func(*cobra.Command) +} + +func newRootCommandFactory(fn func() *cobra.Command) *rootCommandFactory { + return &rootCommandFactory{ + constructor: fn, + } +} + +func (f *rootCommandFactory) Use(fn func(cmd *cobra.Command)) { + f.options = append(f.options, fn) +} + +func (f *rootCommandFactory) Build() *cobra.Command { + o := f.constructor() + for _, v := range f.options { + v(o) + } + return o +} diff --git a/cmd/commands.go b/cmd/commands.go index e5e1265e..0853ebf8 100644 --- a/cmd/commands.go +++ b/cmd/commands.go @@ -438,43 +438,44 @@ EXPERIMENTAL: May be changed or removed. }, }) - RegisterCommand(Command{ - Name: "manpage", - Usage: "--directory ", - Short: "Generates the manual pages for Caddy commands", - Long: ` + defaultFactory.Use(func(rootCmd *cobra.Command) { + RegisterCommand(Command{ + Name: "manpage", + Usage: "--directory ", + Short: "Generates the manual pages for Caddy commands", + Long: ` Generates the manual pages for Caddy commands into the designated directory tagged into section 8 (System Administration). The manual page files are generated into the directory specified by the argument of --directory. If the directory does not exist, it will be created. `, - CobraFunc: func(cmd *cobra.Command) { - cmd.Flags().StringP("directory", "o", "", "The output directory where the manpages are generated") - cmd.RunE = WrapCommandFuncForCobra(func(fl Flags) (int, error) { - dir := strings.TrimSpace(fl.String("directory")) - if dir == "" { - return caddy.ExitCodeFailedQuit, fmt.Errorf("designated output directory and specified section are required") - } - if err := os.MkdirAll(dir, 0o755); err != nil { - return caddy.ExitCodeFailedQuit, err - } - if err := doc.GenManTree(rootCmd, &doc.GenManHeader{ - Title: "Caddy", - Section: "8", // https://en.wikipedia.org/wiki/Man_page#Manual_sections - }, dir); err != nil { - return caddy.ExitCodeFailedQuit, err - } - return caddy.ExitCodeSuccess, nil - }) - }, - }) + CobraFunc: func(cmd *cobra.Command) { + cmd.Flags().StringP("directory", "o", "", "The output directory where the manpages are generated") + cmd.RunE = WrapCommandFuncForCobra(func(fl Flags) (int, error) { + dir := strings.TrimSpace(fl.String("directory")) + if dir == "" { + return caddy.ExitCodeFailedQuit, fmt.Errorf("designated output directory and specified section are required") + } + if err := os.MkdirAll(dir, 0o755); err != nil { + return caddy.ExitCodeFailedQuit, err + } + if err := doc.GenManTree(rootCmd, &doc.GenManHeader{ + Title: "Caddy", + Section: "8", // https://en.wikipedia.org/wiki/Man_page#Manual_sections + }, dir); err != nil { + return caddy.ExitCodeFailedQuit, err + } + return caddy.ExitCodeSuccess, nil + }) + }, + }) - // source: https://github.com/spf13/cobra/blob/main/shell_completions.md - rootCmd.AddCommand(&cobra.Command{ - Use: "completion [bash|zsh|fish|powershell]", - Short: "Generate completion script", - Long: fmt.Sprintf(`To load completions: + // source: https://github.com/spf13/cobra/blob/main/shell_completions.md + rootCmd.AddCommand(&cobra.Command{ + Use: "completion [bash|zsh|fish|powershell]", + Short: "Generate completion script", + Long: fmt.Sprintf(`To load completions: Bash: @@ -513,23 +514,24 @@ argument of --directory. If the directory does not exist, it will be created. PS> %[1]s completion powershell > %[1]s.ps1 # and source this file from your PowerShell profile. `, rootCmd.Root().Name()), - DisableFlagsInUseLine: true, - ValidArgs: []string{"bash", "zsh", "fish", "powershell"}, - Args: cobra.MatchAll(cobra.ExactArgs(1), cobra.OnlyValidArgs), - RunE: func(cmd *cobra.Command, args []string) error { - switch args[0] { - case "bash": - return cmd.Root().GenBashCompletion(os.Stdout) - case "zsh": - return cmd.Root().GenZshCompletion(os.Stdout) - case "fish": - return cmd.Root().GenFishCompletion(os.Stdout, true) - case "powershell": - return cmd.Root().GenPowerShellCompletionWithDesc(os.Stdout) - default: - return fmt.Errorf("unrecognized shell: %s", args[0]) - } - }, + DisableFlagsInUseLine: true, + ValidArgs: []string{"bash", "zsh", "fish", "powershell"}, + Args: cobra.MatchAll(cobra.ExactArgs(1), cobra.OnlyValidArgs), + RunE: func(cmd *cobra.Command, args []string) error { + switch args[0] { + case "bash": + return cmd.Root().GenBashCompletion(os.Stdout) + case "zsh": + return cmd.Root().GenZshCompletion(os.Stdout) + case "fish": + return cmd.Root().GenFishCompletion(os.Stdout, true) + case "powershell": + return cmd.Root().GenPowerShellCompletionWithDesc(os.Stdout) + default: + return fmt.Errorf("unrecognized shell: %s", args[0]) + } + }, + }) }) } @@ -563,7 +565,9 @@ func RegisterCommand(cmd Command) { if !commandNameRegex.MatchString(cmd.Name) { panic("invalid command name") } - rootCmd.AddCommand(caddyCmdToCobra(cmd)) + defaultFactory.Use(func(rootCmd *cobra.Command) { + rootCmd.AddCommand(caddyCmdToCobra(cmd)) + }) } var commandNameRegex = regexp.MustCompile(`^[a-z0-9]$|^([a-z0-9]+-?[a-z0-9]*)+[a-z0-9]$`) diff --git a/cmd/main.go b/cmd/main.go index 3c3ae627..655c0084 100644 --- a/cmd/main.go +++ b/cmd/main.go @@ -72,7 +72,7 @@ func Main() { caddy.Log().Warn("failed to set GOMAXPROCS", zap.Error(err)) } - if err := rootCmd.Execute(); err != nil { + if err := defaultFactory.Build().Execute(); err != nil { var exitError *exitError if errors.As(err, &exitError) { os.Exit(exitError.ExitCode) From 098897bdea67eb31634f440a4d9a69b5753a9ac3 Mon Sep 17 00:00:00 2001 From: Cuckoo Chickoo Date: Thu, 22 Aug 2024 15:45:58 +0530 Subject: [PATCH 5/6] chore: Fix a typo (#6534) Fixes Typo in Docs --- modules/caddyhttp/proxyprotocol/listenerwrapper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/caddyhttp/proxyprotocol/listenerwrapper.go b/modules/caddyhttp/proxyprotocol/listenerwrapper.go index e25fe02a..440e7071 100644 --- a/modules/caddyhttp/proxyprotocol/listenerwrapper.go +++ b/modules/caddyhttp/proxyprotocol/listenerwrapper.go @@ -40,7 +40,7 @@ type ListenerWrapper struct { Allow []string `json:"allow,omitempty"` allow []netip.Prefix - // Denby is an optional list of CIDR ranges to + // Deny is an optional list of CIDR ranges to // deny PROXY headers from. Deny []string `json:"deny,omitempty"` deny []netip.Prefix From 8af646730be93f4a00b873d1822bfde6be106696 Mon Sep 17 00:00:00 2001 From: Mohammed Al Sahaf Date: Thu, 22 Aug 2024 20:32:44 +0300 Subject: [PATCH 6/6] caddyhttp: run `error` (msg) through replacer (#6536) * error: run `error` (msg) through replacer Signed-off-by: Mohammed Al Sahaf * fix integration test Signed-off-by: Mohammed Al Sahaf --------- Signed-off-by: Mohammed Al Sahaf --- caddytest/caddytest_test.go | 1 - modules/caddyhttp/staticerror.go | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/caddytest/caddytest_test.go b/caddytest/caddytest_test.go index 937537fa..a9d5da93 100644 --- a/caddytest/caddytest_test.go +++ b/caddytest/caddytest_test.go @@ -84,7 +84,6 @@ func TestLoadUnorderedJSON(t *testing.T) { "servers": { "s_server": { "listen": [ - ":9443", ":9080" ], "routes": [ diff --git a/modules/caddyhttp/staticerror.go b/modules/caddyhttp/staticerror.go index b6e10ff3..aeb31140 100644 --- a/modules/caddyhttp/staticerror.go +++ b/modules/caddyhttp/staticerror.go @@ -105,8 +105,7 @@ func (e StaticError) ServeHTTP(w http.ResponseWriter, r *http.Request, _ Handler } statusCode = intVal } - - return Error(statusCode, fmt.Errorf("%s", e.Error)) + return Error(statusCode, fmt.Errorf("%s", repl.ReplaceKnown(e.Error, ""))) } // Interface guard