diff --git a/deploy/upcloud/provision.go b/deploy/upcloud/provision.go index 194bd76..bc327f5 100644 --- a/deploy/upcloud/provision.go +++ b/deploy/upcloud/provision.go @@ -322,6 +322,12 @@ func cmdProvision(token, zone, plan, sshKeyPath, s3Secret string, withScanner, w return fmt.Errorf("LB forwarded headers: %w", err) } + // Same reason: HTTP/2 and the client timeout must reach an LB that already + // exists, not just one created by this run. + if err := ensureLBHTTP2(ctx, svc, state.LB.UUID); err != nil { + return fmt.Errorf("LB http2: %w", err) + } + // Ensure route-hold rule includes forwarded headers action if err := ensureLBHoldForwardedHeaders(ctx, svc, state.LB.UUID, holdDomain); err != nil { return fmt.Errorf("LB hold forwarded headers: %w", err) @@ -697,14 +703,14 @@ func createFirewallRules(ctx context.Context, svc *service.Service, serverUUID, Comment: "Allow private network", }, { - Direction: upcloud.FirewallRuleDirectionIn, - Action: upcloud.FirewallRuleActionAccept, - Family: upcloud.IPAddressFamilyIPv4, - Protocol: upcloud.FirewallRuleProtocolUDP, - SourcePortStart: "123", - SourcePortEnd: "123", - Position: 3, - Comment: "Allow NTP replies", + Direction: upcloud.FirewallRuleDirectionIn, + Action: upcloud.FirewallRuleActionAccept, + Family: upcloud.IPAddressFamilyIPv4, + Protocol: upcloud.FirewallRuleProtocolUDP, + SourcePortStart: "123", + SourcePortEnd: "123", + Position: 3, + Comment: "Allow NTP replies", }, { Direction: upcloud.FirewallRuleDirectionIn, @@ -716,6 +722,46 @@ func createFirewallRules(ctx context.Context, svc *service.Service, serverUUID, }) } +// lbClientTimeout is the frontend's client-side timeout, in seconds. It is +// stated explicitly rather than left to the UpCloud default so the value the +// deployment relies on is visible here: the admin panel's slow endpoints are +// cut at this boundary, and the 504 that produces is what the UI surfaces as +// "Server error". Changing it changes that behaviour, so it is a deliberate +// knob, not an incidental one. +const lbClientTimeout = 10 + +// httpFrontendProperties are the https frontend's properties. +// +// HTTP/2 is the one that matters for the browser: over HTTP/1.1 a browser +// opens at most ~6 connections per origin, so a page that fans out many small +// requests (the admin crew tab issues one per member) queues them six at a +// time and blocks every other request to the same host behind them. h2 +// multiplexes them over a single connection and the queue disappears. +// +// Clients that need an HTTP/1.1 upgrade are unaffected: h2 is negotiated per +// connection via ALPN, so a WebSocket client simply selects http/1.1. +func httpFrontendProperties() *upcloud.LoadBalancerFrontendProperties { + return &upcloud.LoadBalancerFrontendProperties{ + HTTP2Enabled: new(true), + TimeoutClient: lbClientTimeout, + } +} + +// backendHTTP2 reports whether a backend may be spoken to over HTTP/2. +// +// False for the hold, and that is not an oversight. The hold serves +// com.atproto.sync.subscribeRepos over WebSocket to external relays and to the +// scanner, and WebSocket over HTTP/2 requires the RFC 8441 Extended CONNECT +// that Go's http2 server does not implement for Upgrade:. Routing that backend +// over h2 would break the firehose. The appview accepts no inbound WebSocket, +// so it has no such constraint. +// +// Both origins wrap their handler in h2c regardless, so enabling this later is +// a config change rather than a code change. +func backendHTTP2(name string) *bool { + return new(name != "hold") +} + func createLoadBalancer(ctx context.Context, svc *service.Service, cfg *InfraConfig, naming Naming, networkUUID, appviewIP, holdIP, holdDomain, labelerDomain string, withLabeler bool) (*upcloud.LoadBalancer, error) { frontendRules := []request.LoadBalancerFrontendRule{ { @@ -766,6 +812,7 @@ func createLoadBalancer(ctx context.Context, svc *service.Service, cfg *InfraCon Properties: &upcloud.LoadBalancerBackendProperties{ HealthCheckType: upcloud.LoadBalancerHealthCheckTypeHTTP, HealthCheckURL: "/health", + HTTP2Enabled: backendHTTP2("appview"), }, }, { @@ -784,6 +831,7 @@ func createLoadBalancer(ctx context.Context, svc *service.Service, cfg *InfraCon Properties: &upcloud.LoadBalancerBackendProperties{ HealthCheckType: upcloud.LoadBalancerHealthCheckTypeHTTP, HealthCheckURL: "/xrpc/_health", + HTTP2Enabled: backendHTTP2("hold"), }, }, } @@ -817,6 +865,7 @@ func createLoadBalancer(ctx context.Context, svc *service.Service, cfg *InfraCon Mode: upcloud.LoadBalancerModeHTTP, Port: 443, DefaultBackend: "appview", + Properties: httpFrontendProperties(), Networks: []upcloud.LoadBalancerFrontendNetwork{ {Name: "public"}, }, @@ -969,6 +1018,71 @@ func ensureLBCertificates(ctx context.Context, svc *service.Service, lbUUID stri // ensureLBForwardedHeaders ensures the "https" frontend has a set_forwarded_headers rule. // This makes the LB set X-Forwarded-For, X-Forwarded-Proto, and X-Forwarded-Port headers, // overwriting any pre-existing values (prevents spoofing). +// ensureLBHTTP2 reconciles HTTP/2 and the client timeout onto an LB that +// already exists. +// +// createLoadBalancer only runs when there is no LB yet, so without this the +// properties above would reach a fresh deployment and never an existing one. +// This follows ensureLBForwardedHeaders: read what is there, change only what +// differs, and say what it did. Running it twice is a no-op. +func ensureLBHTTP2(ctx context.Context, svc *service.Service, lbUUID string) error { + fe, err := svc.GetLoadBalancerFrontend(ctx, &request.GetLoadBalancerFrontendRequest{ + ServiceUUID: lbUUID, + Name: "https", + }) + if err != nil { + return fmt.Errorf("get https frontend: %w", err) + } + + want := httpFrontendProperties() + haveHTTP2 := fe.Properties != nil && fe.Properties.HTTP2Enabled != nil && *fe.Properties.HTTP2Enabled + haveTimeout := fe.Properties != nil && fe.Properties.TimeoutClient == want.TimeoutClient + if haveHTTP2 && haveTimeout { + fmt.Println(" Frontend HTTP/2 + timeout: already set") + } else { + if _, err := svc.ModifyLoadBalancerFrontend(ctx, &request.ModifyLoadBalancerFrontendRequest{ + ServiceUUID: lbUUID, + Name: "https", + Frontend: request.ModifyLoadBalancerFrontend{ + Properties: want, + }, + }); err != nil { + return fmt.Errorf("modify https frontend: %w", err) + } + fmt.Printf(" Frontend HTTP/2: enabled (timeout_client=%ds)\n", want.TimeoutClient) + } + + backends, err := svc.GetLoadBalancerBackends(ctx, &request.GetLoadBalancerBackendsRequest{ServiceUUID: lbUUID}) + if err != nil { + return fmt.Errorf("get backends: %w", err) + } + for _, b := range backends { + wantH2 := backendHTTP2(b.Name) + have := b.Properties != nil && b.Properties.HTTP2Enabled != nil && *b.Properties.HTTP2Enabled + if have == *wantH2 { + continue + } + // Send the health check back with it: Properties replaces the object + // wholesale, so omitting these would drop the backend's health check. + props := &upcloud.LoadBalancerBackendProperties{HTTP2Enabled: wantH2} + if b.Properties != nil { + p := *b.Properties + p.HTTP2Enabled = wantH2 + props = &p + } + if _, err := svc.ModifyLoadBalancerBackend(ctx, &request.ModifyLoadBalancerBackendRequest{ + ServiceUUID: lbUUID, + Name: b.Name, + Backend: request.ModifyLoadBalancerBackend{Properties: props}, + }); err != nil { + return fmt.Errorf("modify backend %s: %w", b.Name, err) + } + fmt.Printf(" Backend %s HTTP/2: %t\n", b.Name, *wantH2) + } + + return nil +} + func ensureLBForwardedHeaders(ctx context.Context, svc *service.Service, lbUUID string) error { rules, err := svc.GetLoadBalancerFrontendRules(ctx, &request.GetLoadBalancerFrontendRulesRequest{ ServiceUUID: lbUUID, diff --git a/go.mod b/go.mod index eaa0469..a2af975 100644 --- a/go.mod +++ b/go.mod @@ -49,6 +49,7 @@ require ( go.yaml.in/yaml/v4 v4.0.0-rc.6 golang.org/x/crypto v0.55.0 golang.org/x/image v0.45.0 + golang.org/x/net v0.58.0 golang.org/x/sys v0.47.0 golang.org/x/xerrors v0.0.0-20240903120638-7835f813f4da oras.land/oras-go/v2 v2.6.2 @@ -204,7 +205,6 @@ require ( go.uber.org/zap v1.28.0 // indirect go.yaml.in/yaml/v3 v3.0.5 // indirect golang.org/x/exp v0.0.0-20260824195058-e88cd73687aa // indirect - golang.org/x/net v0.58.0 // indirect golang.org/x/sync v0.22.0 // indirect golang.org/x/text v0.41.0 // indirect golang.org/x/time v0.15.0 // indirect diff --git a/pkg/appview/server.go b/pkg/appview/server.go index 18114f7..efee91d 100644 --- a/pkg/appview/server.go +++ b/pkg/appview/server.go @@ -22,6 +22,8 @@ import ( "github.com/go-chi/chi/v5" chimiddleware "github.com/go-chi/chi/v5/middleware" "github.com/klauspost/compress/gzhttp" + "golang.org/x/net/http2" + "golang.org/x/net/http2/h2c" "atcr.io/pkg/appview/authgate" "atcr.io/pkg/appview/db" @@ -747,8 +749,16 @@ func (s *AppViewServer) Serve() error { // this to bind a 127.0.0.1:0 listener and learn the assigned port before // driving requests. func (s *AppViewServer) ServeWithListener(listener net.Listener) error { + // h2c so the load balancer can reach this origin over HTTP/2 without TLS. + // TLS terminates at the LB, so the backend leg is cleartext and ALPN never + // runs; without this wrapper net/http can only ever answer HTTP/1.1 here. + // + // The wrapper is opt-in per connection: it upgrades only for a client that + // sends the h2c preface or an "Upgrade: h2c" header, and hands everything + // else to the router untouched. Unlike the hold, this server accepts no + // inbound WebSocket, so its LB backend does enable http2_enabled. s.httpServer = &http.Server{ - Handler: s.Router, + Handler: h2c.NewHandler(s.Router, &http2.Server{}), } stop := make(chan os.Signal, 1) diff --git a/pkg/hold/server.go b/pkg/hold/server.go index 98b4395..bdb9f63 100644 --- a/pkg/hold/server.go +++ b/pkg/hold/server.go @@ -26,6 +26,8 @@ import ( "github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5/middleware" + "golang.org/x/net/http2" + "golang.org/x/net/http2/h2c" ) // purgerAdapter bridges *pds.HoldPDS to the holdlabeler.Purger interface, which @@ -348,8 +350,23 @@ func (s *HoldServer) Serve() error { // this to bind a 127.0.0.1:0 listener and learn the assigned port before // driving requests. func (s *HoldServer) ServeWithListener(listener net.Listener) error { + // h2c so the load balancer can reach this origin over HTTP/2 without TLS. + // TLS terminates at the LB, so the backend leg is cleartext and ALPN never + // runs; without this wrapper net/http can only ever answer HTTP/1.1 here. + // + // The wrapper is opt-in per connection: it upgrades only for a client that + // sends the h2c preface or an "Upgrade: h2c" header, and hands everything + // else to the router untouched. An HTTP/1.1 WebSocket upgrade is therefore + // unaffected, which matters because this server carries subscribeRepos. + // + // NB: that is also why the hold's LB *backend* leaves http2_enabled off + // (see ensureLBHTTP2 in deploy/upcloud). WebSocket over HTTP/2 needs the + // RFC 8441 Extended CONNECT that Go's http2 server does not implement for + // Upgrade:, so routing this backend over h2 would break the firehose and + // the scanner's connection. This wrapper makes the capability available; + // enabling it for this backend is a separate, deliberate decision. s.httpServer = &http.Server{ - Handler: s.Router, + Handler: h2c.NewHandler(s.Router, &http2.Server{}), ReadTimeout: s.Config.Server.ReadTimeout, WriteTimeout: s.Config.Server.WriteTimeout, }