chore: sync with basecamp/kamal-proxy main (on-demand TLS, client-IP header, Go 1.26.5) - #42
Merged
Merged
Conversation
Read the envvar KAMAL_PROXY_SOCKET to determine the control socket path.
This makes it possible to control a running kamal-proxy instance from a
different user, by making the socket accessible via file system
permissions.
If KAMAL_PROXY_SOCKET is unset, retain the previous
${XDG_RUNTIME_DIR:/tmp}/kamal-proxy.sock
behaviour.
Lets services opt specific paths (typically health checks from upstream load balancers or uptime monitors) out of the Prometheus request and in-flight metrics. Matches are exact, can be repeated, and only suppress metrics — request logs are still emitted. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The motivation for `--exclude-metrics-path` is that high-volume healthcheck traffic distorts aggregate metrics (request rate, latency percentiles, error rates) and inflates the metrics pipeline — not that the output is "noisy". Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add --exclude-metrics-path to filter request paths from metrics
make socket path configurable
There are a couple places where behviour is dependent on whether a request path matches a configured value: identifying incoming health check requests; and identifying paths for which metrics should not be tracked. In both of these cases, if an app is deployed under a path prefix (and strips the prefix) then the expected behaviour is that we match that path against what the upstream sees. So we need to factor in any prefix-stripping before paths are matched.
…matching Respect path prefix when matching paths
Typically a downstreeam proxy will pass the original client IP via `X-Forwarded-For`, which we already handle. However some proxies use a different header. For example, Cloudflare typically sets it in `True-Client-IP`. To support this, add a new `--client-ip-header` deploy flag which specifies the name of the header to use. When this is set, we copy the content of that header into `X-Forwarded-For` before logging and proxying, as if `X-Forwarded-For` had been set that way in the request.
Allow specifying header source for client IP
Allows applications to provision TLS certificates for multiple hosts on-demand, by way of an application endpoint that gates issuance on a host-by-host basis. To use, specify `--tls-on-demand-url` rather than `--host`. The URL can be directed to an external service, or plain path routed to the service. Co-authored-by: Didier Lafforgue <didier.lafforgue@gmail.com>
On-demand TLS
Brings in basecamp's on-demand TLS, client-IP header source, --exclude-metrics-path, configurable socket path, path-prefix-aware path matching, and the Go 1.26.5 / dependency bump. Conflict resolutions: - go.mod/go.sum: upstream's toolchain (1.26.5) and dependency versions, fork's lego graph retained, then go mod tidy. - ServiceOptions: union -- upstream's ExcludeMetricsPaths/ClientIPHeader alongside the fork's TLSDomains* fields. - Validate: upstream's on-demand branch, with the fork's dynamic-domains exemption preserved on the else-branch. tls-domains-source and tls-on-demand-url are now rejected together: both provision certificates for hosts unknown at deploy time, but only one manager can serve the handshake. - createCertManager: the shared SAN manager still wins by default, but an explicit --tls-on-demand-url is a per-service opt-in and takes precedence, falling through to upstream's autocert manager and host policy. - createMiddleware: kept the fork's targetOptions parameter (request deadline) and added upstream's createHostPolicy. - Prefix matching: upstream dropped the strings import from target.go, so the per-path timeout matcher now shares PathMatchesPrefix with ResolvePathTimeout instead of duplicating the check.
…erge - tls-domains-source combined with tls-on-demand-url is rejected - an explicit --tls-on-demand-url takes precedence over the proxy-wide SAN certificate manager instead of being silently ignored
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Syncs
dashwithbasecamp/kamal-proxy:mainthrough2ad0b2c— the 9 upstream commitsdashwas behind.What comes in from upstream:
tls_on_demand.go,--tls-on-demand-url)Router.GetCertificate, which falls through to the catch-all service's cert manager.--client-ip-header(client_ip_middleware.go)createMiddleware.--exclude-metrics-pathServiceOptions.ShouldExcludeMetrics→loggingRequestContext.ExcludeMetrics.KAMAL_PROXY_SOCKET)Config.SocketPath(); the socket name is unchanged, so thekamal-proxynaming contract holds.RoutedTargetPath)Target.rewrite.go.mod/go.sum.Conflict resolutions
Six conflicts:
go.mod,go.sum,service.go,service_test.go,router_test.go,deploy_test.go. Test-file conflicts were pure unions (both sides only added tests). The substantive ones:go.mod/go.sum— upstream's toolchain (1.26.5) and dependency versions, fork'sgo-acme/legograph retained, thengo mod tidy.ServiceOptions— union: upstream'sExcludeMetricsPaths/ClientIPHeadernext to the fork'sTLSDomains*fields.Validate— upstream's on-demand validation branch, with the fork's dynamic-domains exemption preserved on theelsebranch (!HasConfiguredHosts() && TLSDomainsSource == "").createCertManager— the proxy-wide SAN manager still wins by default, but an explicit--tls-on-demand-urlis a per-service opt-in and now takes precedence, falling through to upstream'sautocert.Manager+createHostPolicy.createMiddleware— kept the fork'stargetOptionsparameter (the request-deadline middleware from feat(target): per-route timeouts and an opt-in whole-request deadline #31) and added upstream'screateHostPolicy.stringsimport fromtarget.go, so the per-path timeout matcher now shares a newPathMatchesPrefixwithResolvePathTimeoutrather than duplicating the check.Test plan
make test— all packages passgo test -race ./internal/...— clean (3 consecutive full-package runs)gofmt -l internal/ cmd/andgo vet ./...cleango build ./...Deviations & judgment calls
tls-domains-source+tls-on-demand-urlis now a validation error. Both provision certificates for hosts unknown at deploy time, but through different managers, and only one can serve a given handshake. Silently letting one win would be a debugging trap, sovalidateDynamicDomainsrejects the combination. Covered byTestServiceOptions_Validate_DynamicDomains.--acme-emailinstalls a proxy-wide SAN manager that previously short-circuitedcreateCertManager; a service that explicitly sets--tls-on-demand-urlwould then have had that flag silently ignored. An explicit per-service flag should win over a proxy-wide default. Covered by a new assertion in the same test.CertificateRegistryrather than the PR's standalone path. Preserving upstream code verbatim is the cheaper merge and the smaller diff for future syncs; wiring on-demand into the registry (so DNS-01/wildcard and on-demand share one cache) is left as a follow-up rather than folded into a sync PR.upstream/maindirectly into a branch offdash, not throughmain.mainstill carries a fork-localCreate FUNDING.ymlcommit, sogit merge --ff-only upstream/mainonmainfails; routing the sync through it would have meant either a merge commit on the "ff-only mirror" or a force-push. The content is identical either way.mainis still 9 commits behind and wants a separate decision.TestHealthCheckWithCustomHostuses a 10ms timeout and flakes under-racewhen the whole (now larger) package runs on a loaded machine.health_check.goand its test are untouched by this merge, and the full-racesuite passes on reruns, so it's pre-existing test fragility rather than a regression.