From b41e4fb93ca936a4c941b8eaeb2ec7a7dc85e520 Mon Sep 17 00:00:00 2001 From: Lior Lieberman Date: Mon, 24 Aug 2026 15:02:01 -0700 Subject: [PATCH 1/3] atenet: offer h2 on the HTTPS ingress and mirror the protocol to actors, behind a flag --- cmd/atenet/internal/router/cmd.go | 1 + cmd/atenet/internal/router/config.go | 4 + cmd/atenet/internal/router/dataplane.go | 1 + cmd/atenet/internal/router/xds.go | 62 +++++++-- cmd/atenet/internal/router/xds_test.go | 104 +++++++++++++++ internal/atunnel/ingress.go | 73 ++++++++++- internal/atunnel/ingress_test.go | 122 +++++++++++++++++ .../e2e/suites/networking/protocol_test.go | 123 ++++++++++++++++++ manifests/ate-install/atenet-router.yaml | 6 + 9 files changed, 478 insertions(+), 18 deletions(-) create mode 100644 internal/e2e/suites/networking/protocol_test.go diff --git a/cmd/atenet/internal/router/cmd.go b/cmd/atenet/internal/router/cmd.go index 3530375007..c0771eb2b7 100644 --- a/cmd/atenet/internal/router/cmd.go +++ b/cmd/atenet/internal/router/cmd.go @@ -58,6 +58,7 @@ func NewRouterCmd() *cobra.Command { cmd.Flags().IntVar(&cfg.StatusPort, "status-port", 4040, "Port to serve /statusz on (set <= 0 to disable serving status)") cmd.Flags().DurationVar(&cfg.HealthInterval, "health-interval", 1*time.Second, "Interval for checking health of dependent services") cmd.Flags().IntVar(&cfg.HttpsPort, "port-https", 8443, "TCP port for HTTPS workload traffic entering through the router dataplane") + cmd.Flags().BoolVar(&cfg.HttpsH2, "https-h2", false, "Offer HTTP/2 via ALPN on the HTTPS listener. Required for gRPC to actors over TLS; HTTP/1.1 clients are unaffected, and non-gRPC HTTP/2 requests are downgraded to HTTP/1.1 before reaching the actor, so HTTP/1.1-only actors keep working. Off preserves the historical no-ALPN behavior") cmd.Flags().StringVar(&cfg.EnvoyCertPath, "envoy-cert-path", "", "Path to the Envoy certificate file.") cmd.Flags().StringVar(&cfg.UpstreamCredentialBundlePath, "upstream-credential-bundle", "/run/podidentity.podcert.ate.dev/credential-bundle.pem", "PEM credential bundle (cert+key) the router presents as the client cert when dialing the actor's atunnel ingress server over mTLS. Empty disables upstream mTLS (legacy plaintext pod-IP:80).") cmd.Flags().StringVar(&cfg.UpstreamTrustBundlePath, "upstream-trust-bundle", "/run/podidentity.podcert.ate.dev/trust-bundle.pem", "PEM trust bundle used to validate the actor's atunnel ingress server certificate.") diff --git a/cmd/atenet/internal/router/config.go b/cmd/atenet/internal/router/config.go index 66d9ea1b91..62062c43f3 100644 --- a/cmd/atenet/internal/router/config.go +++ b/cmd/atenet/internal/router/config.go @@ -94,6 +94,10 @@ type routerConfig struct { ConnectPlainTextPort int ConnectTLSPort int EnvoyCertPath string + // HttpsH2 offers HTTP/2 via ALPN on the HTTPS ingress listener, which + // gRPC over TLS requires. Off preserves the historical no-ALPN + // behavior. + HttpsH2 bool // UpstreamCredentialBundlePath is the router's podidentity credential bundle // (cert+key) presented as the client cert when dialing the actor's atunnel diff --git a/cmd/atenet/internal/router/dataplane.go b/cmd/atenet/internal/router/dataplane.go index 190e73cd88..ff362abd75 100644 --- a/cmd/atenet/internal/router/dataplane.go +++ b/cmd/atenet/internal/router/dataplane.go @@ -77,6 +77,7 @@ func (s *RouterServer) startEnvoyDataplane(ctx context.Context, g *errgroup.Grou } xdsSrv.SetTlsConfig(s.cfg.HttpsPort, s.cfg.EnvoyCertPath) + xdsSrv.SetHttpsH2(s.cfg.HttpsH2) xdsSrv.SetUpstreamTls(s.cfg.UpstreamCredentialBundlePath, s.cfg.UpstreamTrustBundlePath, s.cfg.UpstreamSpiffePrefix) ctrl := NewController(s.atStore, xdsSrv) diff --git a/cmd/atenet/internal/router/xds.go b/cmd/atenet/internal/router/xds.go index 62135d4e80..eb55794cf4 100644 --- a/cmd/atenet/internal/router/xds.go +++ b/cmd/atenet/internal/router/xds.go @@ -129,6 +129,11 @@ const defaultExtProcMaxRequests = 2048 // does not silently stretch every shutdown past terminationGracePeriodSeconds. // Operators who raise --route-timeout and want such turns to survive a drain // must raise --drain-timeout (and the grace period) explicitly. +// +// TODO(liorlieberman): this ceiling also cuts off gRPC server-streaming and +// bidi RPCs longer than 10s, so streaming gRPC effectively requires raising +// --route-timeout today. We need to fix it so streaming works without +// inflating the timeout for all workloads. const defaultRouteTimeout = 10 * time.Second // envoyDefaultStreamIdleTimeout is the stream idle timeout Envoy applies when @@ -161,6 +166,9 @@ type XdsServer struct { connectPlainTextPort int connectTLSPort int certPath string + // httpsH2 offers HTTP/2 via ALPN on the HTTPS ingress listener, + // enabling gRPC to actors over TLS. See SetHttpsH2. + httpsH2 bool // Upstream (actor-facing) mTLS. When upstreamCredentialBundlePath is set, the // ORIGINAL_DST actor cluster dials the actor's in-worker atunnel ingress @@ -297,6 +305,17 @@ func (x *XdsServer) SetTlsConfig(httpsPort int, certPath string) { x.certPath = certPath } +// SetHttpsH2 controls whether the HTTPS ingress listener offers HTTP/2 via +// ALPN. Off, the listener advertises no protocols and clients fall back to +// HTTP/1.1, matching historical behavior; on, gRPC (which requires a +// negotiated "h2") can reach actors over TLS, while HTTP/1.1 clients still +// negotiate http/1.1. +func (x *XdsServer) SetHttpsH2(enabled bool) { + x.mu.Lock() + defer x.mu.Unlock() + x.httpsH2 = enabled +} + // otlpDefaultPort is the OTLP/gRPC default port, used when the collector // endpoint names no port. const otlpDefaultPort = "4317" @@ -759,19 +778,21 @@ func (x *XdsServer) buildOriginalDstCluster() *clusterv3.Cluster { if ts := x.buildUpstreamTransportSocket(); ts != nil { cluster.TransportSocket = ts - // The atunnel ingress server terminates TLS and reverse-proxies to the - // actor over HTTP/1.1. - httpOpts := newAny(&httpv3.HttpProtocolOptions{ - UpstreamProtocolOptions: &httpv3.HttpProtocolOptions_ExplicitHttpConfig_{ - ExplicitHttpConfig: &httpv3.HttpProtocolOptions_ExplicitHttpConfig{ - ProtocolConfig: &httpv3.HttpProtocolOptions_ExplicitHttpConfig_HttpProtocolOptions{ - HttpProtocolOptions: &corev3.Http1ProtocolOptions{}, + // Mirror the downstream protocol to atunnel so HTTP/2 requests stay + // HTTP/2 and gRPC keeps trailers and streaming end to end. atunnel + // gates its own actor leg: it forwards HTTP/2 only for gRPC and + // downgrades everything else, so an HTTP/2 client cannot break an + // HTTP/1.1-only actor. Without upstream mTLS there is no atunnel leg + // to carry HTTP/2, so that mode keeps Envoy's implicit HTTP/1.1. + cluster.TypedExtensionProtocolOptions = map[string]*anypb.Any{ + httpProtocolOptionsName: newAny(&httpv3.HttpProtocolOptions{ + UpstreamProtocolOptions: &httpv3.HttpProtocolOptions_UseDownstreamProtocolConfig{ + UseDownstreamProtocolConfig: &httpv3.HttpProtocolOptions_UseDownstreamHttpConfig{ + HttpProtocolOptions: &corev3.Http1ProtocolOptions{}, + Http2ProtocolOptions: &corev3.Http2ProtocolOptions{}, }, }, - }, - }) - cluster.TypedExtensionProtocolOptions = map[string]*anypb.Any{ - httpProtocolOptionsName: httpOpts, + }), } } @@ -1170,9 +1191,12 @@ func (x *XdsServer) buildListener() *listenerv3.Listener { // socket shared by every TLS-terminating listener: it serves the SDS-fetched // certificate at HTTPSCertSecretName (see buildTlsSecret), which UpdateSnapshot // includes whenever any TLS listener (HTTPS or CONNECT-TLS) is configured. -func buildDownstreamTlsTransportSocket() *corev3.TransportSocket { +// alpnProtocols, when non-empty, is offered during the handshake; empty +// advertises nothing, leaving clients on HTTP/1.1. +func buildDownstreamTlsTransportSocket(alpnProtocols []string) *corev3.TransportSocket { tlsConfig := &tlsv3.DownstreamTlsContext{ CommonTlsContext: &tlsv3.CommonTlsContext{ + AlpnProtocols: alpnProtocols, TlsCertificateSdsSecretConfigs: []*tlsv3.SdsSecretConfig{ { Name: HTTPSCertSecretName, @@ -1198,6 +1222,13 @@ func buildDownstreamTlsTransportSocket() *corev3.TransportSocket { func (x *XdsServer) buildHttpsListener() *listenerv3.Listener { hcm := x.buildHcm("ingress_https", true) + // gRPC requires a negotiated "h2"; http/1.1 keeps plain HTTPS clients + // working alongside it. + var alpn []string + if x.httpsH2 { + alpn = []string{"h2", "http/1.1"} + } + return &listenerv3.Listener{ Name: IngressHTTPSListener, Address: &corev3.Address{ @@ -1221,7 +1252,7 @@ func (x *XdsServer) buildHttpsListener() *listenerv3.Listener { }, }, }, - TransportSocket: buildDownstreamTlsTransportSocket(), + TransportSocket: buildDownstreamTlsTransportSocket(alpn), }, }, } @@ -1287,7 +1318,10 @@ func (x *XdsServer) buildConnectTerminateTLSListener() *listenerv3.Listener { }, }, }, - TransportSocket: buildDownstreamTlsTransportSocket(), + // No ALPN: CONNECT-TLS clients speak HTTP/1.1 CONNECT + // today, and the HTTPS h2 knob deliberately leaves this + // listener alone. + TransportSocket: buildDownstreamTlsTransportSocket(nil), }, }, } diff --git a/cmd/atenet/internal/router/xds_test.go b/cmd/atenet/internal/router/xds_test.go index 4ea2f7adad..23536bec0f 100644 --- a/cmd/atenet/internal/router/xds_test.go +++ b/cmd/atenet/internal/router/xds_test.go @@ -40,6 +40,7 @@ import ( routev3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3" hcmv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3" tlsv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/transport_sockets/tls/v3" + httpv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/upstreams/http/v3" discoverygrpc "github.com/envoyproxy/go-control-plane/envoy/service/discovery/v3" secretgrpc "github.com/envoyproxy/go-control-plane/envoy/service/secret/v3" cachev3 "github.com/envoyproxy/go-control-plane/pkg/cache/v3" @@ -829,6 +830,55 @@ func TestXdsServer_BuildOriginalDstCluster_UsesMetadataKey(t *testing.T) { } } +// TestXdsServer_ActorClusterProtocolOptions pins where downstream-protocol +// mirroring is allowed: only on the mTLS atunnel leg, where atunnel guards +// HTTP/1.1-only actors by downgrading non-gRPC HTTP/2 (see +// atunnel.protocolMirrorTransport). The legacy plaintext cluster dials the +// actor directly with no such guard, so it must carry no protocol options at +// all — Envoy's implicit HTTP/1.1. +func TestXdsServer_ActorClusterProtocolOptions(t *testing.T) { + x := NewXdsServer(18000) + + if opts := x.buildOriginalDstCluster().GetTypedExtensionProtocolOptions(); len(opts) != 0 { + t.Errorf("legacy plaintext actor cluster has protocol options %v, want none (implicit HTTP/1.1)", opts) + } + + x.SetUpstreamTls("/run/bundle.pem", "/run/trust.pem", "spiffe://ate.dev/") + cluster := x.buildOriginalDstCluster() + ts := cluster.GetTransportSocket() + if ts == nil { + t.Fatal("mTLS actor cluster is missing its transport socket") + } + // The upstream TLS context must NOT set alpn_protocols: with + // use_downstream_protocol_config, Envoy already offers the single ALPN + // matching each connection pool's protocol. A static ["h2","http/1.1"] + // list would override that, and Go's atunnel server (which negotiates by + // server preference) would pick h2 on connections belonging to the + // HTTP/1.1 pool, breaking it. + upstreamTls := &tlsv3.UpstreamTlsContext{} + if err := ts.GetTypedConfig().UnmarshalTo(upstreamTls); err != nil { + t.Fatalf("Failed to unmarshal UpstreamTlsContext: %v", err) + } + if alpn := upstreamTls.GetCommonTlsContext().GetAlpnProtocols(); len(alpn) != 0 { + t.Errorf("upstream TLS context ALPN = %v, want none (per-pool ALPN comes from use_downstream_protocol_config)", alpn) + } + raw, ok := cluster.GetTypedExtensionProtocolOptions()[httpProtocolOptionsName] + if !ok { + t.Fatalf("mTLS actor cluster is missing %q protocol options", httpProtocolOptionsName) + } + protoOpts := &httpv3.HttpProtocolOptions{} + if err := raw.UnmarshalTo(protoOpts); err != nil { + t.Fatalf("Failed to unmarshal HttpProtocolOptions: %v", err) + } + downstream := protoOpts.GetUseDownstreamProtocolConfig() + if downstream == nil { + t.Fatalf("mTLS actor cluster protocol options = %v, want use_downstream_protocol_config", protoOpts) + } + if downstream.GetHttp2ProtocolOptions() == nil { + t.Error("use_downstream_protocol_config must enable HTTP/2 so gRPC keeps trailers on the atunnel leg") + } +} + // TestXdsServer_BuildRoutes_DerivesTargetPortHeader covers the fix for atunnel // needing the target port as a real header (it can't read Envoy's dynamic // metadata directly): rather than ext_proc building that header mutation @@ -1055,3 +1105,57 @@ func TestSnapshotVersionsUniqueAcrossRestarts(t *testing.T) { seen[v] = true } } + +// downstreamTLS extracts the DownstreamTlsContext from a listener's first +// filter chain. +func downstreamTLS(t *testing.T, raw any) *tlsv3.DownstreamTlsContext { + t.Helper() + l := raw.(*listenerv3.Listener) + dtc := &tlsv3.DownstreamTlsContext{} + if err := l.GetFilterChains()[0].GetTransportSocket().GetTypedConfig().UnmarshalTo(dtc); err != nil { + t.Fatalf("Failed to unmarshal DownstreamTlsContext: %v", err) + } + return dtc +} + +func TestXdsServer_HttpsH2ALPN(t *testing.T) { + const certPath = "/run/servicedns.podcert.ate.dev/credential-bundle.pem" + + snapshotListeners := func(t *testing.T, h2 bool) map[string]any { + t.Helper() + server := NewXdsServer(18000) + server.SetConfig(8085, 50053, "127.0.0.1") + server.SetTlsConfig(8443, certPath) + server.SetConnectPorts(0, 8444) + server.SetHttpsH2(h2) + if err := server.UpdateSnapshot(); err != nil { + t.Fatalf("UpdateSnapshot failed: %v", err) + } + res, err := server.snapshot.GetSnapshot(NodeID) + if err != nil { + t.Fatalf("Failed to get snapshot: %v", err) + } + listeners := map[string]any{} + for name, l := range res.(*cachev3.Snapshot).GetResources(resourcev3.ListenerType) { + listeners[name] = l + } + return listeners + } + + // Default: no ALPN anywhere + listeners := snapshotListeners(t, false) + if alpn := downstreamTLS(t, listeners[IngressHTTPSListener]).GetCommonTlsContext().GetAlpnProtocols(); len(alpn) != 0 { + t.Errorf("HTTPS listener ALPN with knob off = %v, want none", alpn) + } + + // Enabled: the HTTPS listener offers h2 then http/1.1; the CONNECT-TLS + // listener stays untouched. + listeners = snapshotListeners(t, true) + alpn := downstreamTLS(t, listeners[IngressHTTPSListener]).GetCommonTlsContext().GetAlpnProtocols() + if len(alpn) != 2 || alpn[0] != "h2" || alpn[1] != "http/1.1" { + t.Errorf("HTTPS listener ALPN with knob on = %v, want [h2 http/1.1]", alpn) + } + if alpn := downstreamTLS(t, listeners["connect_terminate_tls"]).GetCommonTlsContext().GetAlpnProtocols(); len(alpn) != 0 { + t.Errorf("CONNECT-TLS listener ALPN = %v, want none regardless of the knob", alpn) + } +} diff --git a/internal/atunnel/ingress.go b/internal/atunnel/ingress.go index d598cb988e..b338f1f97d 100644 --- a/internal/atunnel/ingress.go +++ b/internal/atunnel/ingress.go @@ -126,7 +126,7 @@ func NewServer(cfg Config) (*Server, error) { return nil, fmt.Errorf("atunnel: trust bundle %q contains no certificates", cfg.TrustBundlePath) } - transport := http.DefaultTransport.(*http.Transport).Clone() + transport := newProtocolMirrorTransport() proxy := &httputil.ReverseProxy{ Rewrite: func(pr *httputil.ProxyRequest) { pr.SetURL(cfg.Upstream) @@ -181,6 +181,65 @@ func NewServer(cfg Config) (*Server, error) { return s, nil } +// isGRPC reports whether an incoming HTTP request conforms to the gRPC over +// HTTP/2 specification: HTTP/2 framing, POST method, and a Content-Type of +// "application/grpc" (optionally with a "+" or ";"). +// Notably this excludes gRPC-Web that works over HTTP/1.1. +// See https://github.com/grpc/grpc/blob/master/doc/PROTOCOL-HTTP2.md#requests +func isGRPC(r *http.Request) bool { + if r.ProtoMajor != 2 || r.Method != http.MethodPost { + return false + } + ct := strings.ToLower(r.Header.Get("Content-Type")) + const base = "application/grpc" + if ct == base { + return true + } + if strings.HasPrefix(ct, base) { + switch ct[len(base)] { + case '+', ';': + return true + } + } + return false +} + +// protocolMirrorTransport picks the actor-leg protocol per request. gRPC goes +// out as prior-knowledge h2c, since it needs HTTP/2 trailers and full-duplex +// streaming; everything else is translated to HTTP/1.1 even if it arrived over +// HTTP/2 at the edge. This is to not break previous logic so an HTTP/1.1-only actor keeps working whatever the +// client negotiated. +type protocolMirrorTransport struct { + h1, h2c *http.Transport +} + +func newProtocolMirrorTransport() protocolMirrorTransport { + h1 := http.DefaultTransport.(*http.Transport).Clone() + h2c := http.DefaultTransport.(*http.Transport).Clone() + protocols := new(http.Protocols) + protocols.SetUnencryptedHTTP2(true) + h2c.Protocols = protocols + return protocolMirrorTransport{h1: h1, h2c: h2c} +} + +func (t protocolMirrorTransport) RoundTrip(r *http.Request) (*http.Response, error) { + if isGRPC(r) { + return t.h2c.RoundTrip(r) + } + return t.h1.RoundTrip(r) +} + +// CloseIdleConnections keeps Deactivate's idle-connection cleanup working: +// closeIdleUpstreamConnections discovers it through a duck-typed interface +// assertion that silently returns false if the method disappears, so pin it +// at compile time. +var _ interface{ CloseIdleConnections() } = protocolMirrorTransport{} + +func (t protocolMirrorTransport) CloseIdleConnections() { + t.h1.CloseIdleConnections() + t.h2c.CloseIdleConnections() +} + func loadCredentialBundle(path string) (*tls.Certificate, error) { pemBytes, err := os.ReadFile(path) if err != nil { @@ -202,9 +261,15 @@ func (s *Server) Serve(ctx context.Context, lis net.Listener) error { // separate listener so ordinary actor ingress remains a request proxy, while // the router can use this listener for a bidirectional tunnel. func (s *Server) ServeConnect(ctx context.Context, lis net.Listener) error { - // Keep ALPN confined to the CONNECT listener. The established ingress - // listener is explicitly HTTP/1.1 in the router's upstream cluster, while - // the CONNECT listener supports either HTTP/1.1 or HTTP/2. + // Offer both protocols explicitly, matching the ingress listener (whose + // ServeTLS advertises h2 and http/1.1 by default). The router's actor + // cluster mirrors the downstream protocol, so either can arrive here. + // The tunnel itself relays opaque bytes, so protocolMirrorTransport's + // gRPC-only gate does not apply to CONNECT traffic — and that is the + // point: this listener is the basis of the planned tunnel-based ingress, + // where ordinary actor traffic arrives here as a spliced tunnel and all + // protocol decisions move to the router's route config, leaving atunnel + // with no request parsing at all. tlsConfig := s.tlsConfig.Clone() tlsConfig.NextProtos = []string{"h2", "http/1.1"} return s.serve(ctx, lis, http.HandlerFunc(s.ServeConnectHTTP), tlsConfig) diff --git a/internal/atunnel/ingress_test.go b/internal/atunnel/ingress_test.go index 71af3b9a16..7ad1a96603 100644 --- a/internal/atunnel/ingress_test.go +++ b/internal/atunnel/ingress_test.go @@ -615,3 +615,125 @@ func tlsHandshake(serverConfig, clientConfig *tls.Config) (serverErr, clientErr serverErr = <-done return serverErr, clientErr } + +func TestIsGRPC(t *testing.T) { + for _, tt := range []struct { + name string + protoMajor int + method string + contentType string + want bool + }{ + {name: "grpc", protoMajor: 2, method: http.MethodPost, contentType: "application/grpc", want: true}, + {name: "grpc+proto", protoMajor: 2, method: http.MethodPost, contentType: "application/grpc+proto", want: true}, + {name: "grpc with params", protoMajor: 2, method: http.MethodPost, contentType: "application/grpc;charset=utf-8", want: true}, + {name: "uppercase content type", protoMajor: 2, method: http.MethodPost, contentType: "Application/GRPC", want: true}, + {name: "uppercase with subtype", protoMajor: 2, method: http.MethodPost, contentType: "APPLICATION/GRPC+PROTO", want: true}, + {name: "grpc-web is not grpc", protoMajor: 2, method: http.MethodPost, contentType: "application/grpc-web+proto", want: false}, + {name: "grpc content type over http/1.1", protoMajor: 1, method: http.MethodPost, contentType: "application/grpc", want: false}, + {name: "non-POST", protoMajor: 2, method: http.MethodGet, contentType: "application/grpc", want: false}, + {name: "plain h2 json", protoMajor: 2, method: http.MethodPost, contentType: "application/json", want: false}, + {name: "no content type", protoMajor: 2, method: http.MethodPost, contentType: "", want: false}, + } { + t.Run(tt.name, func(t *testing.T) { + req, err := http.NewRequest(tt.method, "http://actor/", nil) + if err != nil { + t.Fatal(err) + } + req.ProtoMajor = tt.protoMajor + if tt.contentType != "" { + req.Header.Set("Content-Type", tt.contentType) + } + if got := isGRPC(req); got != tt.want { + t.Errorf("isGRPC() = %v, want %v", got, tt.want) + } + }) + } +} + +// mirrorBackend starts a backend speaking the given protocols and returns its +// address plus a channel yielding the protocol each request arrived with. +func mirrorBackend(t *testing.T, h1, h2c bool) (addr string, protoSeen chan string) { + t.Helper() + protoSeen = make(chan string, 1) + protocols := new(http.Protocols) + protocols.SetHTTP1(h1) + protocols.SetUnencryptedHTTP2(h2c) + backend := &http.Server{ + Handler: http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + protoSeen <- r.Proto + }), + Protocols: protocols, + } + lis, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + go backend.Serve(lis) + t.Cleanup(func() { backend.Close() }) + return lis.Addr().String(), protoSeen +} + +// TestProtocolMirrorTransport verifies the upstream leg's protocol choice: +// only gRPC (HTTP/2 + POST + application/grpc*) goes out as cleartext +// prior-knowledge HTTP/2, preserving the trailers and streaming it needs; +// everything else — including non-gRPC requests that arrived over HTTP/2 — +// is sent as HTTP/1.1, so an HTTP/1.1-only actor keeps working no matter +// what protocol the client spoke at the edge. +func TestProtocolMirrorTransport(t *testing.T) { + for _, tt := range []struct { + name string + backendH1 bool + backendH2C bool + protoMajor int + method string + contentType string + want string + wantErr bool + }{ + // HTTP/1.1-only backend: the common actor. Every non-gRPC shape must + // reach it as HTTP/1.1, whatever the client negotiated with the edge. + {name: "h1 GET to h1-only actor", backendH1: true, protoMajor: 1, method: http.MethodGet, want: "HTTP/1.1"}, + {name: "h2 GET downgraded for h1-only actor", backendH1: true, protoMajor: 2, method: http.MethodGet, want: "HTTP/1.1"}, + {name: "h2 POST json downgraded for h1-only actor", backendH1: true, protoMajor: 2, method: http.MethodPost, contentType: "application/json", want: "HTTP/1.1"}, + {name: "grpc-web stays h1 for h1-only actor", backendH1: true, protoMajor: 2, method: http.MethodPost, contentType: "application/grpc-web+proto", want: "HTTP/1.1"}, + // gRPC to an actor that can't speak h2c must fail loudly (the proxy + // surfaces it as 502) rather than silently fall back to HTTP/1.1, + // which would strip the trailers gRPC needs. + {name: "grpc to h1-only actor fails", backendH1: true, protoMajor: 2, method: http.MethodPost, contentType: "application/grpc", wantErr: true}, + // gRPC actor (h2c-capable): gRPC stays HTTP/2 end to end. + {name: "grpc to grpc actor", backendH1: true, backendH2C: true, protoMajor: 2, method: http.MethodPost, contentType: "application/grpc", want: "HTTP/2.0"}, + {name: "grpc+proto to grpc actor", backendH1: true, backendH2C: true, protoMajor: 2, method: http.MethodPost, contentType: "application/grpc+proto", want: "HTTP/2.0"}, + // Mixed traffic to the same h2c-capable actor still downgrades + // non-gRPC, mirroring what a browser or curl sends. + {name: "h2 GET downgraded even for h2c-capable actor", backendH1: true, backendH2C: true, protoMajor: 2, method: http.MethodGet, want: "HTTP/1.1"}, + } { + t.Run(tt.name, func(t *testing.T) { + addr, protoSeen := mirrorBackend(t, tt.backendH1, tt.backendH2C) + transport := newProtocolMirrorTransport() + req, err := http.NewRequest(tt.method, "http://"+addr+"/", http.NoBody) + if err != nil { + t.Fatal(err) + } + req.ProtoMajor = tt.protoMajor + if tt.contentType != "" { + req.Header.Set("Content-Type", tt.contentType) + } + res, err := transport.RoundTrip(req) + if tt.wantErr { + if err == nil { + res.Body.Close() + t.Fatal("RoundTrip succeeded, want an error (no silent HTTP/1.1 fallback for gRPC)") + } + return + } + if err != nil { + t.Fatalf("RoundTrip: %v", err) + } + res.Body.Close() + if got := <-protoSeen; got != tt.want { + t.Errorf("upstream saw %s, want %s", got, tt.want) + } + }) + } +} diff --git a/internal/e2e/suites/networking/protocol_test.go b/internal/e2e/suites/networking/protocol_test.go new file mode 100644 index 0000000000..e6a646cfb7 --- /dev/null +++ b/internal/e2e/suites/networking/protocol_test.go @@ -0,0 +1,123 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package networking + +import ( + "context" + "fmt" + "io" + "net/http" + "strings" + "testing" + "time" + + "github.com/agent-substrate/substrate/internal/ateclient" + "github.com/agent-substrate/substrate/internal/e2e" + "github.com/agent-substrate/substrate/internal/portforward" + "github.com/agent-substrate/substrate/internal/resources" + "k8s.io/client-go/kubernetes" +) + +// TestIngressProtocolDowngrade pins the ingress protocol contract end to end: +// a client that negotiates HTTP/2 with the router must still be able to reach +// an HTTP/1.1-only actor (the counter demo), because the atunnel leg +// downgrades non-gRPC traffic to HTTP/1.1. A gRPC-shaped request, by +// contrast, is carried to the actor as real HTTP/2 — so against this +// non-gRPC actor it must fail loudly rather than silently fall back to +// HTTP/1.1 (which would strip the trailers gRPC needs). +// +// TODO(liorlieberman): add the gRPC-positive counterpart (a gRPC actor +// answering over the same path) once a gRPC actor fixture exists — glutton +// --mode=grpc is the natural candidate. +func TestIngressProtocolDowngrade(t *testing.T) { + ctx := context.Background() + actorName, _ := createAndResumeActor(t, ctx, "protodowngrade", e2e.CounterFixture()) + actorRef := resources.ActorRef{Atespace: networkingAtespace, Name: actorName} + + config, err := ateclient.LoadConfig(e2e.KubeConfig, e2e.KubeContext) + if err != nil { + t.Fatalf("loading kubeconfig: %v", err) + } + clientset, err := kubernetes.NewForConfig(config) + if err != nil { + t.Fatalf("creating k8s client: %v", err) + } + // RouterClient only speaks HTTP/1.1, so this test manages its own + // port-forward and clients: the point is to control the protocol the + // client negotiates with the router. + localPort, stop, err := portforward.ServicePortForward(ctx, config, clientset, "ate-system", "atenet-router", 80) + if err != nil { + t.Fatalf("port-forwarding to the router: %v", err) + } + defer stop() + base := fmt.Sprintf("http://127.0.0.1:%d", localPort) + + h1 := &http.Client{Timeout: 30 * time.Second} + h2cTransport := http.DefaultTransport.(*http.Transport).Clone() + protocols := new(http.Protocols) + protocols.SetUnencryptedHTTP2(true) + h2cTransport.Protocols = protocols + h2c := &http.Client{Transport: h2cTransport, Timeout: 30 * time.Second} + + request := func(client *http.Client, method, path, contentType string) (*http.Response, error) { + req, err := http.NewRequestWithContext(ctx, method, base+path, http.NoBody) + if err != nil { + return nil, err + } + req.Host = resources.ActorDNSName(actorRef) + if contentType != "" { + req.Header.Set("Content-Type", contentType) + } + return client.Do(req) + } + + // Wait for the route over plain HTTP/1.1 first, so the protocol + // assertions below never race actor readiness. + waitForRouteReady(t, "HTTP/1.1 access through ingress", func() (*http.Response, error) { + return request(h1, http.MethodGet, "/readyz", "") + }) + + t.Run("h2 client reaches h1-only actor", func(t *testing.T) { + resp, err := request(h2c, http.MethodGet, "/readyz", "") + if err != nil { + t.Fatalf("h2c request through ingress: %v", err) + } + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + if resp.Proto != "HTTP/2.0" { + t.Errorf("downstream proto = %s, want HTTP/2.0 (the client really negotiated h2)", resp.Proto) + } + if resp.StatusCode != http.StatusOK { + t.Fatalf("h2c GET = %d (body %q), want 200: non-gRPC HTTP/2 must be downgraded for HTTP/1.1-only actors", resp.StatusCode, body) + } + if !strings.Contains(string(body), "ok") { + t.Errorf("h2c GET body = %q, want the actor's readyz payload", body) + } + }) + + t.Run("grpc to non-grpc actor fails loudly", func(t *testing.T) { + resp, err := request(h2c, http.MethodPost, "/count", "application/grpc") + if err != nil { + t.Fatalf("gRPC-shaped request through ingress: %v", err) + } + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + // atunnel forwards gRPC as real h2c, which the HTTP/1.1-only counter + // cannot speak — a 502 from atunnel, not a silently-downgraded 200. + if resp.StatusCode != http.StatusBadGateway { + t.Fatalf("gRPC-shaped POST = %d (body %q), want 502: gRPC must not be silently downgraded to HTTP/1.1", resp.StatusCode, body) + } + }) +} diff --git a/manifests/ate-install/atenet-router.yaml b/manifests/ate-install/atenet-router.yaml index 018dbd0a4a..b3afb989b7 100644 --- a/manifests/ate-install/atenet-router.yaml +++ b/manifests/ate-install/atenet-router.yaml @@ -208,6 +208,12 @@ spec: # turns to survive a shutdown, raise --drain-timeout and # terminationGracePeriodSeconds alongside it. # - "--route-timeout=5m" + # Enables HTTP/2 via ALPN on the HTTPS listener, which gRPC to actors + # over TLS requires. HTTP/1.1 clients are unaffected, and only gRPC + # continues to the actor as HTTP/2 — atunnel downgrades other HTTP/2 + # requests to HTTP/1.1, so HTTP/1.1-only actors keep working the way they used to. Leaving + # it off means no ALPN negotiation. + # - "--https-h2" env: - name: POD_NAME valueFrom: From c03d7f114c12ba61645087ca51c4966dea486589 Mon Sep 17 00:00:00 2001 From: Lior Lieberman Date: Wed, 26 Aug 2026 17:13:54 -0700 Subject: [PATCH 2/3] align grpc tests --- internal/e2e/fixtures/testserver/grpc.go | 54 ++- internal/e2e/fixtures/testserver/grpc_test.go | 14 + .../fixtures/testserver/grpcecho.yaml.tmpl | 79 ++++ internal/e2e/sandbox_test.go | 1 + .../e2e/suites/networking/grpcingress_test.go | 351 ++++++++++++++++++ .../e2e/suites/networking/protocol_test.go | 123 ------ 6 files changed, 492 insertions(+), 130 deletions(-) create mode 100644 internal/e2e/fixtures/testserver/grpcecho.yaml.tmpl create mode 100644 internal/e2e/suites/networking/grpcingress_test.go delete mode 100644 internal/e2e/suites/networking/protocol_test.go diff --git a/internal/e2e/fixtures/testserver/grpc.go b/internal/e2e/fixtures/testserver/grpc.go index fca07c8e28..f3571fe0aa 100644 --- a/internal/e2e/fixtures/testserver/grpc.go +++ b/internal/e2e/fixtures/testserver/grpc.go @@ -21,6 +21,7 @@ import ( "io" "log" "net" + "net/http" "github.com/spf13/cobra" "google.golang.org/grpc" @@ -92,18 +93,42 @@ func newServer() *grpc.Server { return server } -// newGRPCCmd is the gRPC origin the egress e2e suites dial through the egress -// gateway. It serves cleartext HTTP/2 -- no TLS anywhere -- because the leg -// under test is the tunnel, not the origin's identity, and because the gateway +// newHealthHandler answers the HTTP readiness probe. Deliberately trivial: it +// reports that the process is up, and the gRPC listener is opened before this +// one, so a 200 here means the RPC port is already accepting. +func newHealthHandler() http.Handler { + mux := http.NewServeMux() + mux.HandleFunc("/readyz", func(w http.ResponseWriter, _ *http.Request) { + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, "ok") + }) + return mux +} + +// newGRPCCmd is the gRPC server both networking e2e suites talk to. One +// subcommand serves both directions so the two legs cannot drift apart in what +// they consider a working RPC: +// +// - egress: deployed as a plain pod, it is the origin an Actor dials through +// the egress gateway. +// - ingress: deployed as an Actor, it is the origin a client reaches through +// atenet-router. +// +// It serves cleartext HTTP/2 -- no TLS anywhere -- because the leg under test +// is the tunnel, not the origin's identity, and because the egress gateway // relays a terminated CONNECT as opaque TCP: whatever the actor speaks is what // arrives here. // -// It answers the grpc health service as well as Echo, which is what the pod's -// readinessProbe checks. A separate HTTP port for readiness would need an h2c -// handler multiplexed onto this listener, and the whole point of this mode is -// that nothing between the actor and here parses HTTP. +// It answers the grpc health service as well as Echo, which is what the egress +// pod's readinessProbe checks. Multiplexing readiness onto that listener as an +// h2c handler would defeat the point of the egress fixture, where nothing +// between the actor and here parses HTTP -- so --health-listen puts it on a +// second port instead, off unless asked for. The ingress Actor needs it because +// an ActorTemplate's readyz is an HTTP GET and nothing else: a gRPC server +// answers one with a protocol error, so without it the Actor never boots. func newGRPCCmd() *cobra.Command { var listenAddress string + var healthAddress string cmd := &cobra.Command{ Use: "grpc", Short: "Serve a cleartext HTTP/2 gRPC echo origin.", @@ -114,9 +139,24 @@ func newGRPCCmd() *cobra.Command { return fmt.Errorf("listening on %s: %w", listenAddress, err) } log.Printf("testserver grpc: serving on %s", listener.Addr()) + + // Bound before Serve below, so a readiness 200 can never precede + // the gRPC port being open. + if healthAddress != "" { + healthListener, err := net.Listen("tcp", healthAddress) + if err != nil { + return fmt.Errorf("listening for health on %s: %w", healthAddress, err) + } + log.Printf("testserver grpc: serving /readyz on %s", healthListener.Addr()) + go func() { + log.Fatal(http.Serve(healthListener, newHealthHandler())) + }() + } + return newServer().Serve(listener) }, } cmd.Flags().StringVar(&listenAddress, "listen", ":50051", "Address the gRPC server listens on, cleartext HTTP/2.") + cmd.Flags().StringVar(&healthAddress, "health-listen", "", "Address for an HTTP/1.1 /readyz listener. Empty serves no HTTP at all, which is what the egress fixture wants; the ingress Actor sets it because an ActorTemplate readyz is an HTTP GET.") return cmd } diff --git a/internal/e2e/fixtures/testserver/grpc_test.go b/internal/e2e/fixtures/testserver/grpc_test.go index 22f2c1e009..7a0c90b0c3 100644 --- a/internal/e2e/fixtures/testserver/grpc_test.go +++ b/internal/e2e/fixtures/testserver/grpc_test.go @@ -20,6 +20,8 @@ import ( "fmt" "io" "net" + "net/http" + "net/http/httptest" "testing" "time" @@ -192,3 +194,15 @@ func TestHealthServiceIsServing(t *testing.T) { t.Errorf("health Check status = %s, want SERVING", response.GetStatus()) } } + +// The ingress Actor's readyz is an HTTP GET, so this handler is the only thing +// that gets it to PhaseReady — the gRPC port answers such a request with a +// protocol error. A 404 from a mistyped path would look exactly like a template +// that never boots. +func TestReadyzAnswersHTTPGet(t *testing.T) { + recorder := httptest.NewRecorder() + newHealthHandler().ServeHTTP(recorder, httptest.NewRequest(http.MethodGet, "/readyz", nil)) + if recorder.Code != http.StatusOK { + t.Errorf("GET /readyz = %d, want %d", recorder.Code, http.StatusOK) + } +} diff --git a/internal/e2e/fixtures/testserver/grpcecho.yaml.tmpl b/internal/e2e/fixtures/testserver/grpcecho.yaml.tmpl new file mode 100644 index 0000000000..d1006394e8 --- /dev/null +++ b/internal/e2e/fixtures/testserver/grpcecho.yaml.tmpl @@ -0,0 +1,79 @@ +# Copyright 2026 Google LLC +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +# The gRPC Actor the ingress suite reaches through atenet-router, running the +# same `testserver grpc` echo origin the egress suite deploys as a plain pod. +# One binary for both directions, so what counts as a working RPC cannot differ +# between them. +# +# Rendered by e2e.RenderFixtureManifest, which fills the ${...} placeholders for +# the sandbox class under test: one manifest serves both runtimes so the gVisor +# and micro-VM variants cannot drift apart. A placeholder that has no value for +# the selected class takes its whole line with it. + +apiVersion: v1 +kind: Namespace +metadata: + name: ate-e2e-grpcecho${FIXTURE_SUFFIX} + +--- + +apiVersion: ate.dev/v1alpha1 +kind: WorkerPool +metadata: + name: grpcecho + namespace: ate-e2e-grpcecho${FIXTURE_SUFFIX} + labels: + workload: grpcecho${FIXTURE_SUFFIX} +spec: + # One worker for the template's golden snapshot, plus headroom for the actors + # the suite resumes. + replicas: 2 + ateomImage: ${ATEOM_IMAGE} +${WORKERPOOL_RUNTIME} + +--- + +apiVersion: ate.dev/v1alpha1 +kind: ActorTemplate +metadata: + name: grpcecho + namespace: ate-e2e-grpcecho${FIXTURE_SUFFIX} +spec: +${TEMPLATE_SANDBOX_CLASS} + containers: + - name: grpcecho + image: ko://github.com/agent-substrate/substrate/internal/e2e/fixtures/testserver + command: ["/ko-app/testserver"] + # gRPC on the Actor's primary port, so the ingress test reaches it at the + # Actor's DNS name with no CONNECT and no port juggling -- the plain path + # every other ingress assertion in the suite uses. + args: + - "grpc" + - "--listen=:80" + - "--health-listen=:8080" + # readyz is an HTTP GET and nothing else, and a gRPC server answers one with + # a protocol error -- hence the fixture's second, HTTP-only port. Probing :80 + # here would leave the Actor stuck out of PhaseReady forever. + readyz: + httpGet: + path: /readyz + port: 8080 + timeoutSeconds: 60 +${TEMPLATE_RESOURCES} + workerSelector: + matchLabels: + workload: grpcecho${FIXTURE_SUFFIX} + snapshotsConfig: + location: gs://${BUCKET_NAME}/ate-e2e-grpcecho${FIXTURE_SUFFIX}/ diff --git a/internal/e2e/sandbox_test.go b/internal/e2e/sandbox_test.go index fdb6ee2099..9057a9c87d 100644 --- a/internal/e2e/sandbox_test.go +++ b/internal/e2e/sandbox_test.go @@ -28,6 +28,7 @@ var fixtureManifests = []string{ "internal/e2e/fixtures/probe/probe.yaml.tmpl", "internal/e2e/fixtures/probe/probe-sized.yaml.tmpl", "internal/e2e/fixtures/capabilities/capabilities.yaml.tmpl", + "internal/e2e/fixtures/testserver/grpcecho.yaml.tmpl", } // renderFixture renders a manifest and decodes the two resources the diff --git a/internal/e2e/suites/networking/grpcingress_test.go b/internal/e2e/suites/networking/grpcingress_test.go new file mode 100644 index 0000000000..9e9fdaec27 --- /dev/null +++ b/internal/e2e/suites/networking/grpcingress_test.go @@ -0,0 +1,351 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package networking + +import ( + "context" + "errors" + "fmt" + "io" + "net/http" + "path/filepath" + "strings" + "testing" + "time" + + "google.golang.org/grpc" + "google.golang.org/grpc/credentials/insecure" + + "github.com/agent-substrate/substrate/internal/ateclient" + "github.com/agent-substrate/substrate/internal/e2e" + "github.com/agent-substrate/substrate/internal/portforward" + "github.com/agent-substrate/substrate/internal/proto/grpcechopb" + "github.com/agent-substrate/substrate/internal/resources" + "k8s.io/client-go/kubernetes" +) + +// grpcEchoFixtureManifest is the ActorTemplate this suite installs to get a +// gRPC-speaking Actor. It runs the same `testserver grpc` echo origin +// grpcegress_test.go deploys as a plain pod; see +// internal/e2e/fixtures/testserver. +const grpcEchoFixtureManifest = "internal/e2e/fixtures/testserver/grpcecho.yaml.tmpl" + +// TestIngressProtocolDowngrade pins the ingress protocol contract end to end: +// a client that negotiates HTTP/2 with the router must still be able to reach +// an HTTP/1.1-only actor (the counter demo), because the atunnel leg +// downgrades non-gRPC traffic to HTTP/1.1. A gRPC-shaped request, by +// contrast, is carried to the actor as real HTTP/2 — so against this +// non-gRPC actor it must fail loudly rather than silently fall back to +// HTTP/1.1 (which would strip the trailers gRPC needs). +// +// TestIngressGRPC below is the positive counterpart: the same path, against an +// actor that really does speak gRPC. +func TestIngressProtocolDowngrade(t *testing.T) { + ctx := context.Background() + actorName, _ := createAndResumeActor(t, ctx, "protodowngrade", e2e.CounterFixture()) + actorRef := resources.ActorRef{Atespace: networkingAtespace, Name: actorName} + + base := "http://" + routerAddress(t, ctx) + + h1 := &http.Client{Timeout: 30 * time.Second} + h2c := &http.Client{Transport: h2cTransport(), Timeout: 30 * time.Second} + + request := func(client *http.Client, method, path, contentType string) (*http.Response, error) { + req, err := http.NewRequestWithContext(ctx, method, base+path, http.NoBody) + if err != nil { + return nil, err + } + req.Host = resources.ActorDNSName(actorRef) + if contentType != "" { + req.Header.Set("Content-Type", contentType) + } + return client.Do(req) + } + + // Wait for the route over plain HTTP/1.1 first, so the protocol + // assertions below never race actor readiness. + waitForRouteReady(t, "HTTP/1.1 access through ingress", func() (*http.Response, error) { + return request(h1, http.MethodGet, "/readyz", "") + }) + + t.Run("h2 client reaches h1-only actor", func(t *testing.T) { + resp, err := request(h2c, http.MethodGet, "/readyz", "") + if err != nil { + t.Fatalf("h2c request through ingress: %v", err) + } + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + if resp.Proto != "HTTP/2.0" { + t.Errorf("downstream proto = %s, want HTTP/2.0 (the client really negotiated h2)", resp.Proto) + } + if resp.StatusCode != http.StatusOK { + t.Fatalf("h2c GET = %d (body %q), want 200: non-gRPC HTTP/2 must be downgraded for HTTP/1.1-only actors", resp.StatusCode, body) + } + if !strings.Contains(string(body), "ok") { + t.Errorf("h2c GET body = %q, want the actor's readyz payload", body) + } + }) + + t.Run("grpc to non-grpc actor fails loudly", func(t *testing.T) { + resp, err := request(h2c, http.MethodPost, "/count", "application/grpc") + if err != nil { + t.Fatalf("gRPC-shaped request through ingress: %v", err) + } + body, _ := io.ReadAll(resp.Body) + resp.Body.Close() + // atunnel forwards gRPC as real h2c, which the HTTP/1.1-only counter + // cannot speak — a 502 from atunnel, not a silently-downgraded 200. + if resp.StatusCode != http.StatusBadGateway { + t.Fatalf("gRPC-shaped POST = %d (body %q), want 502: gRPC must not be silently downgraded to HTTP/1.1", resp.StatusCode, body) + } + }) +} + +// TestIngressGRPC is the gRPC-positive half of the ingress protocol contract: +// a real gRPC client reaching a real gRPC Actor through atenet-router. Where +// TestIngressProtocolDowngrade proves gRPC is not silently downgraded, this +// proves the traffic that survives the ingress path is still usable gRPC. +// +// All three streaming shapes, because each one fails differently and only the +// first is covered by anything else in this suite: unary needs the status to +// arrive in trailers (after the body), a server-stream needs many frames over a +// connection held open across the response, and a bidirectional stream needs +// frames moving both ways at once and then a clean half-close. A path that +// parsed the request as HTTP/1.1 or dropped trailers would fail every one of +// them, and a path that merely buffered would fail the last. +// +// The Actor runs `testserver grpc`, the same echo origin TestActorEgressGRPC +// deploys as a plain pod, so the two directions cannot disagree about what a +// working RPC is. +func TestIngressGRPC(t *testing.T) { + env, err := e2e.CheckEnv("BUCKET_NAME", "KO_DOCKER_REPO") + if err != nil { + t.Fatalf("CheckEnv failed: %v", err) + } + ctx := context.Background() + + fixture := deployGRPCEchoTemplate(t, ctx, env["BUCKET_NAME"]) + actorName, _ := createAndResumeActor(t, ctx, "grpcingress", fixture) + actorRef := resources.ActorRef{Atespace: networkingAtespace, Name: actorName} + + // Cleartext h2c to the router's HTTP port, with the Actor's DNS name as the + // :authority — the same routing key every other ingress test in this suite + // uses, just carried by a gRPC client instead of an HTTP one. The h2 ALPN + // knob (--https-h2) is about the *TLS* listener; nothing here needs it. + conn, err := grpc.NewClient(routerAddress(t, ctx), + grpc.WithTransportCredentials(insecure.NewCredentials()), + grpc.WithAuthority(resources.ActorDNSName(actorRef)), + ) + if err != nil { + t.Fatalf("creating the gRPC client for %s: %v", resources.ActorDNSName(actorRef), err) + } + defer conn.Close() + client := grpcechopb.NewEchoClient(conn) + + const message = "hello over grpc ingress" + + // Rides out the window between ResumeActor returning and the route reaching + // atenet-router's xDS snapshot, as waitForRouteReady does for HTTP. It has + // to be an RPC rather than a GET: the Actor serves only gRPC on port 80, so + // an HTTP/1.1 probe would never come back 200 no matter how ready it is. + waitForGRPCRouteReady(t, ctx, client, message) + + t.Run("unary", func(t *testing.T) { + rpcCtx, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() + // A returned error here is itself the trailer assertion: grpc-go reports + // a missing or malformed status as an error, so a path that dropped + // trailers cannot reach the comparison below. + response, err := client.Echo(rpcCtx, &grpcechopb.EchoRequest{Message: message}) + if err != nil { + t.Fatalf("unary Echo through ingress: %v", err) + } + if response.GetMessage() != message { + t.Errorf("unary Echo returned %q, want %q", response.GetMessage(), message) + } + }) + + t.Run("server stream", func(t *testing.T) { + rpcCtx, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() + const count = 3 + stream, err := client.EchoStream(rpcCtx, &grpcechopb.EchoStreamRequest{Message: message, Count: count}) + if err != nil { + t.Fatalf("EchoStream through ingress: %v", err) + } + var got []*grpcechopb.EchoResponse + for { + response, err := stream.Recv() + if errors.Is(err, io.EOF) { + break + } + if err != nil { + t.Fatalf("EchoStream Recv after %d responses: %v", len(got), err) + } + got = append(got, response) + } + if len(got) != count { + t.Fatalf("EchoStream returned %d responses, want %d", len(got), count) + } + // Indexes are what separate an intact stream from a reordered or + // deduplicated one. + for i, response := range got { + if response.GetMessage() != message { + t.Errorf("stream response %d message = %q, want %q", i, response.GetMessage(), message) + } + if int(response.GetIndex()) != i { + t.Errorf("stream response %d index = %d, want %d", i, response.GetIndex(), i) + } + } + }) + + t.Run("bidi stream", func(t *testing.T) { + rpcCtx, cancel := context.WithTimeout(ctx, 30*time.Second) + defer cancel() + stream, err := client.EchoBidi(rpcCtx) + if err != nil { + t.Fatalf("EchoBidi through ingress: %v", err) + } + // One message at a time, each blocking on its response before the next + // is sent. A path that carried one direction at a time would not return + // short answers here, it would hang until the context deadline. + const count = 3 + for i := range count { + want := fmt.Sprintf("%s-%d", message, i) + if err := stream.Send(&grpcechopb.EchoRequest{Message: want}); err != nil { + t.Fatalf("EchoBidi Send %d: %v", i, err) + } + response, err := stream.Recv() + if err != nil { + t.Fatalf("EchoBidi Recv %d: %v", i, err) + } + if response.GetMessage() != want { + t.Errorf("bidi response %d message = %q, want %q", i, response.GetMessage(), want) + } + if int(response.GetIndex()) != i { + t.Errorf("bidi response %d index = %d, want %d", i, response.GetIndex(), i) + } + } + // Half-close the request direction: the server must still end this one + // with OK, which a path that mishandled the half-close would not produce + // even though everything above already echoed. + if err := stream.CloseSend(); err != nil { + t.Fatalf("EchoBidi CloseSend: %v", err) + } + if _, err := stream.Recv(); !errors.Is(err, io.EOF) { + t.Errorf("EchoBidi Recv after CloseSend = %v, want io.EOF", err) + } + }) +} + +// deployGRPCEchoTemplate installs the gRPC Actor fixture for the sandbox class +// under test, waits for its golden snapshot and returns it. Mirrors the +// capabilities and sizing suites: render one manifest, build and apply it +// through the repo's pinned ko, delete the same file on the way out. +func deployGRPCEchoTemplate(t *testing.T, ctx context.Context, bucket string) e2e.Fixture { + t.Helper() + root, err := e2e.FindRepoRoot() + if err != nil { + t.Fatalf("FindRepoRoot: %v", err) + } + + // The suite's own copy of the fixture: suite packages run as concurrent + // processes, so a shared one would be deleted out from under another. + manifest := e2e.RenderFixtureManifest(t, grpcEchoFixtureManifest, bucket, "networking") + + // KO_CONFIG_PATH is required because ko resolves .ko.yaml from its working + // directory, which here is this package rather than the repo root. + applyArgs := []string{"ko", "apply", "-f", manifest} + if e2e.KubeContext != "" { + applyArgs = append(applyArgs, "--", "--context="+e2e.KubeContext) + } + e2e.RunCmdWithEnv(t, []string{"KO_CONFIG_PATH=" + root}, filepath.Join(root, "hack/run-tool.sh"), applyArgs...) + + t.Cleanup(func() { + // Deletion needs no image build, so go straight to kubectl; `ko delete` + // rejects this arg shape. + delArgs := []string{"delete", "--ignore-not-found", "-f", manifest} + if e2e.KubeContext != "" { + delArgs = append([]string{"--context=" + e2e.KubeContext}, delArgs...) + } + e2e.RunCmd(t, "kubectl", delArgs...) + }) + + fixture := e2e.Fixture{ + Namespace: e2e.FixtureName("ate-e2e-grpcecho") + "-networking", + Name: "grpcecho", + DeployWith: "the networking suite itself (see deployGRPCEchoTemplate)", + } + e2e.WaitForTemplateReady(ctx, t, e2e.GetClients(), fixture.Namespace, fixture.Name) + return fixture +} + +// waitForGRPCRouteReady retries a unary Echo until it succeeds, riding out the +// window between ResumeActor returning and the Actor's route reaching +// atenet-router's xDS snapshot. Requests sent in that window come back as +// Unavailable, which is not a failure of anything this file tests. +func waitForGRPCRouteReady(t *testing.T, ctx context.Context, client grpcechopb.EchoClient, message string) { + t.Helper() + const timeout = 60 * time.Second + deadline := time.Now().Add(timeout) + for { + rpcCtx, cancel := context.WithTimeout(ctx, 10*time.Second) + _, err := client.Echo(rpcCtx, &grpcechopb.EchoRequest{Message: message}) + cancel() + if err == nil { + return + } + if time.Now().After(deadline) { + t.Fatalf("gRPC through ingress did not become ready within %v: %v", timeout, err) + } + t.Logf("gRPC through ingress failed: %v; retrying...", err) + time.Sleep(time.Second) + } +} + +// routerAddress port-forwards to atenet-router's HTTP port and returns the +// local host:port, torn down when the test ends. +// +// e2e.RouterClient is not usable here: it speaks HTTP/1.1 only, and the whole +// point of both tests in this file is to control the protocol the client +// negotiates with the router. +func routerAddress(t *testing.T, ctx context.Context) string { + t.Helper() + config, err := ateclient.LoadConfig(e2e.KubeConfig, e2e.KubeContext) + if err != nil { + t.Fatalf("loading kubeconfig: %v", err) + } + clientset, err := kubernetes.NewForConfig(config) + if err != nil { + t.Fatalf("creating k8s client: %v", err) + } + localPort, stop, err := portforward.ServicePortForward(ctx, config, clientset, "ate-system", "atenet-router", 80) + if err != nil { + t.Fatalf("port-forwarding to the router: %v", err) + } + t.Cleanup(stop) + return fmt.Sprintf("127.0.0.1:%d", localPort) +} + +// h2cTransport is an HTTP transport that speaks cleartext HTTP/2 by prior +// knowledge, so a test can reach the router's plain HTTP port as an h2 client +// without any ALPN negotiation. +func h2cTransport() *http.Transport { + transport := http.DefaultTransport.(*http.Transport).Clone() + protocols := new(http.Protocols) + protocols.SetUnencryptedHTTP2(true) + transport.Protocols = protocols + return transport +} diff --git a/internal/e2e/suites/networking/protocol_test.go b/internal/e2e/suites/networking/protocol_test.go deleted file mode 100644 index e6a646cfb7..0000000000 --- a/internal/e2e/suites/networking/protocol_test.go +++ /dev/null @@ -1,123 +0,0 @@ -// Copyright 2026 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// http://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -package networking - -import ( - "context" - "fmt" - "io" - "net/http" - "strings" - "testing" - "time" - - "github.com/agent-substrate/substrate/internal/ateclient" - "github.com/agent-substrate/substrate/internal/e2e" - "github.com/agent-substrate/substrate/internal/portforward" - "github.com/agent-substrate/substrate/internal/resources" - "k8s.io/client-go/kubernetes" -) - -// TestIngressProtocolDowngrade pins the ingress protocol contract end to end: -// a client that negotiates HTTP/2 with the router must still be able to reach -// an HTTP/1.1-only actor (the counter demo), because the atunnel leg -// downgrades non-gRPC traffic to HTTP/1.1. A gRPC-shaped request, by -// contrast, is carried to the actor as real HTTP/2 — so against this -// non-gRPC actor it must fail loudly rather than silently fall back to -// HTTP/1.1 (which would strip the trailers gRPC needs). -// -// TODO(liorlieberman): add the gRPC-positive counterpart (a gRPC actor -// answering over the same path) once a gRPC actor fixture exists — glutton -// --mode=grpc is the natural candidate. -func TestIngressProtocolDowngrade(t *testing.T) { - ctx := context.Background() - actorName, _ := createAndResumeActor(t, ctx, "protodowngrade", e2e.CounterFixture()) - actorRef := resources.ActorRef{Atespace: networkingAtespace, Name: actorName} - - config, err := ateclient.LoadConfig(e2e.KubeConfig, e2e.KubeContext) - if err != nil { - t.Fatalf("loading kubeconfig: %v", err) - } - clientset, err := kubernetes.NewForConfig(config) - if err != nil { - t.Fatalf("creating k8s client: %v", err) - } - // RouterClient only speaks HTTP/1.1, so this test manages its own - // port-forward and clients: the point is to control the protocol the - // client negotiates with the router. - localPort, stop, err := portforward.ServicePortForward(ctx, config, clientset, "ate-system", "atenet-router", 80) - if err != nil { - t.Fatalf("port-forwarding to the router: %v", err) - } - defer stop() - base := fmt.Sprintf("http://127.0.0.1:%d", localPort) - - h1 := &http.Client{Timeout: 30 * time.Second} - h2cTransport := http.DefaultTransport.(*http.Transport).Clone() - protocols := new(http.Protocols) - protocols.SetUnencryptedHTTP2(true) - h2cTransport.Protocols = protocols - h2c := &http.Client{Transport: h2cTransport, Timeout: 30 * time.Second} - - request := func(client *http.Client, method, path, contentType string) (*http.Response, error) { - req, err := http.NewRequestWithContext(ctx, method, base+path, http.NoBody) - if err != nil { - return nil, err - } - req.Host = resources.ActorDNSName(actorRef) - if contentType != "" { - req.Header.Set("Content-Type", contentType) - } - return client.Do(req) - } - - // Wait for the route over plain HTTP/1.1 first, so the protocol - // assertions below never race actor readiness. - waitForRouteReady(t, "HTTP/1.1 access through ingress", func() (*http.Response, error) { - return request(h1, http.MethodGet, "/readyz", "") - }) - - t.Run("h2 client reaches h1-only actor", func(t *testing.T) { - resp, err := request(h2c, http.MethodGet, "/readyz", "") - if err != nil { - t.Fatalf("h2c request through ingress: %v", err) - } - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() - if resp.Proto != "HTTP/2.0" { - t.Errorf("downstream proto = %s, want HTTP/2.0 (the client really negotiated h2)", resp.Proto) - } - if resp.StatusCode != http.StatusOK { - t.Fatalf("h2c GET = %d (body %q), want 200: non-gRPC HTTP/2 must be downgraded for HTTP/1.1-only actors", resp.StatusCode, body) - } - if !strings.Contains(string(body), "ok") { - t.Errorf("h2c GET body = %q, want the actor's readyz payload", body) - } - }) - - t.Run("grpc to non-grpc actor fails loudly", func(t *testing.T) { - resp, err := request(h2c, http.MethodPost, "/count", "application/grpc") - if err != nil { - t.Fatalf("gRPC-shaped request through ingress: %v", err) - } - body, _ := io.ReadAll(resp.Body) - resp.Body.Close() - // atunnel forwards gRPC as real h2c, which the HTTP/1.1-only counter - // cannot speak — a 502 from atunnel, not a silently-downgraded 200. - if resp.StatusCode != http.StatusBadGateway { - t.Fatalf("gRPC-shaped POST = %d (body %q), want 502: gRPC must not be silently downgraded to HTTP/1.1", resp.StatusCode, body) - } - }) -} From bac3a42d1402df219ce8016c38266bc7560d98f4 Mon Sep 17 00:00:00 2001 From: Lior Lieberman Date: Fri, 28 Aug 2026 10:51:51 -0700 Subject: [PATCH 3/3] remove https-h2 guard --- cmd/atenet/internal/router/cmd.go | 1 - cmd/atenet/internal/router/config.go | 4 - cmd/atenet/internal/router/dataplane.go | 1 - cmd/atenet/internal/router/xds.go | 28 ++--- cmd/atenet/internal/router/xds_test.go | 69 ++++++----- internal/atunnel/ingress_test.go | 109 ++++++++++++++++++ .../e2e/suites/networking/grpcingress_test.go | 2 +- manifests/ate-install/atenet-router.yaml | 6 - 8 files changed, 156 insertions(+), 64 deletions(-) diff --git a/cmd/atenet/internal/router/cmd.go b/cmd/atenet/internal/router/cmd.go index c0771eb2b7..3530375007 100644 --- a/cmd/atenet/internal/router/cmd.go +++ b/cmd/atenet/internal/router/cmd.go @@ -58,7 +58,6 @@ func NewRouterCmd() *cobra.Command { cmd.Flags().IntVar(&cfg.StatusPort, "status-port", 4040, "Port to serve /statusz on (set <= 0 to disable serving status)") cmd.Flags().DurationVar(&cfg.HealthInterval, "health-interval", 1*time.Second, "Interval for checking health of dependent services") cmd.Flags().IntVar(&cfg.HttpsPort, "port-https", 8443, "TCP port for HTTPS workload traffic entering through the router dataplane") - cmd.Flags().BoolVar(&cfg.HttpsH2, "https-h2", false, "Offer HTTP/2 via ALPN on the HTTPS listener. Required for gRPC to actors over TLS; HTTP/1.1 clients are unaffected, and non-gRPC HTTP/2 requests are downgraded to HTTP/1.1 before reaching the actor, so HTTP/1.1-only actors keep working. Off preserves the historical no-ALPN behavior") cmd.Flags().StringVar(&cfg.EnvoyCertPath, "envoy-cert-path", "", "Path to the Envoy certificate file.") cmd.Flags().StringVar(&cfg.UpstreamCredentialBundlePath, "upstream-credential-bundle", "/run/podidentity.podcert.ate.dev/credential-bundle.pem", "PEM credential bundle (cert+key) the router presents as the client cert when dialing the actor's atunnel ingress server over mTLS. Empty disables upstream mTLS (legacy plaintext pod-IP:80).") cmd.Flags().StringVar(&cfg.UpstreamTrustBundlePath, "upstream-trust-bundle", "/run/podidentity.podcert.ate.dev/trust-bundle.pem", "PEM trust bundle used to validate the actor's atunnel ingress server certificate.") diff --git a/cmd/atenet/internal/router/config.go b/cmd/atenet/internal/router/config.go index 62062c43f3..66d9ea1b91 100644 --- a/cmd/atenet/internal/router/config.go +++ b/cmd/atenet/internal/router/config.go @@ -94,10 +94,6 @@ type routerConfig struct { ConnectPlainTextPort int ConnectTLSPort int EnvoyCertPath string - // HttpsH2 offers HTTP/2 via ALPN on the HTTPS ingress listener, which - // gRPC over TLS requires. Off preserves the historical no-ALPN - // behavior. - HttpsH2 bool // UpstreamCredentialBundlePath is the router's podidentity credential bundle // (cert+key) presented as the client cert when dialing the actor's atunnel diff --git a/cmd/atenet/internal/router/dataplane.go b/cmd/atenet/internal/router/dataplane.go index ff362abd75..190e73cd88 100644 --- a/cmd/atenet/internal/router/dataplane.go +++ b/cmd/atenet/internal/router/dataplane.go @@ -77,7 +77,6 @@ func (s *RouterServer) startEnvoyDataplane(ctx context.Context, g *errgroup.Grou } xdsSrv.SetTlsConfig(s.cfg.HttpsPort, s.cfg.EnvoyCertPath) - xdsSrv.SetHttpsH2(s.cfg.HttpsH2) xdsSrv.SetUpstreamTls(s.cfg.UpstreamCredentialBundlePath, s.cfg.UpstreamTrustBundlePath, s.cfg.UpstreamSpiffePrefix) ctrl := NewController(s.atStore, xdsSrv) diff --git a/cmd/atenet/internal/router/xds.go b/cmd/atenet/internal/router/xds.go index eb55794cf4..3eb1186b4e 100644 --- a/cmd/atenet/internal/router/xds.go +++ b/cmd/atenet/internal/router/xds.go @@ -166,9 +166,6 @@ type XdsServer struct { connectPlainTextPort int connectTLSPort int certPath string - // httpsH2 offers HTTP/2 via ALPN on the HTTPS ingress listener, - // enabling gRPC to actors over TLS. See SetHttpsH2. - httpsH2 bool // Upstream (actor-facing) mTLS. When upstreamCredentialBundlePath is set, the // ORIGINAL_DST actor cluster dials the actor's in-worker atunnel ingress @@ -305,17 +302,6 @@ func (x *XdsServer) SetTlsConfig(httpsPort int, certPath string) { x.certPath = certPath } -// SetHttpsH2 controls whether the HTTPS ingress listener offers HTTP/2 via -// ALPN. Off, the listener advertises no protocols and clients fall back to -// HTTP/1.1, matching historical behavior; on, gRPC (which requires a -// negotiated "h2") can reach actors over TLS, while HTTP/1.1 clients still -// negotiate http/1.1. -func (x *XdsServer) SetHttpsH2(enabled bool) { - x.mu.Lock() - defer x.mu.Unlock() - x.httpsH2 = enabled -} - // otlpDefaultPort is the OTLP/gRPC default port, used when the collector // endpoint names no port. const otlpDefaultPort = "4317" @@ -1223,11 +1209,11 @@ func (x *XdsServer) buildHttpsListener() *listenerv3.Listener { hcm := x.buildHcm("ingress_https", true) // gRPC requires a negotiated "h2"; http/1.1 keeps plain HTTPS clients - // working alongside it. - var alpn []string - if x.httpsH2 { - alpn = []string{"h2", "http/1.1"} - } + // working alongside it. HTTP/1.1-only actors are safe either way: atunnel + // downgrades every non-gRPC request to HTTP/1.1 on the actor leg (see + // atunnel.protocolMirrorTransport), so offering h2 at the edge cannot + // change what an actor receives. + alpn := []string{"h2", "http/1.1"} return &listenerv3.Listener{ Name: IngressHTTPSListener, @@ -1319,8 +1305,8 @@ func (x *XdsServer) buildConnectTerminateTLSListener() *listenerv3.Listener { }, }, // No ALPN: CONNECT-TLS clients speak HTTP/1.1 CONNECT - // today, and the HTTPS h2 knob deliberately leaves this - // listener alone. + // today, and the HTTPS listener's h2 offer deliberately + // leaves this listener alone. TransportSocket: buildDownstreamTlsTransportSocket(nil), }, }, diff --git a/cmd/atenet/internal/router/xds_test.go b/cmd/atenet/internal/router/xds_test.go index 23536bec0f..5556233e38 100644 --- a/cmd/atenet/internal/router/xds_test.go +++ b/cmd/atenet/internal/router/xds_test.go @@ -1118,44 +1118,53 @@ func downstreamTLS(t *testing.T, raw any) *tlsv3.DownstreamTlsContext { return dtc } -func TestXdsServer_HttpsH2ALPN(t *testing.T) { +// TestXdsServer_ALPN pins the per-listener ALPN contract. The HTTPS ingress +// listener always offers h2 before http/1.1: gRPC over TLS requires a +// negotiated "h2", and the offer is unconditional because atunnel downgrades +// every non-gRPC request to HTTP/1.1 on the actor leg (see +// atunnel.protocolMirrorTransport and TestProtocolMirrorTransport), so an +// HTTP/1.1-only actor cannot tell what the client negotiated at the edge. The +// CONNECT-TLS listener stays ALPN-free: its clients speak HTTP/1.1 CONNECT, +// and an h2 offer there would move them onto extended CONNECT the tunnel path +// does not serve. +func TestXdsServer_ALPN(t *testing.T) { const certPath = "/run/servicedns.podcert.ate.dev/credential-bundle.pem" - snapshotListeners := func(t *testing.T, h2 bool) map[string]any { - t.Helper() - server := NewXdsServer(18000) - server.SetConfig(8085, 50053, "127.0.0.1") - server.SetTlsConfig(8443, certPath) - server.SetConnectPorts(0, 8444) - server.SetHttpsH2(h2) - if err := server.UpdateSnapshot(); err != nil { - t.Fatalf("UpdateSnapshot failed: %v", err) - } - res, err := server.snapshot.GetSnapshot(NodeID) - if err != nil { - t.Fatalf("Failed to get snapshot: %v", err) - } - listeners := map[string]any{} - for name, l := range res.(*cachev3.Snapshot).GetResources(resourcev3.ListenerType) { - listeners[name] = l - } - return listeners + server := NewXdsServer(18000) + server.SetConfig(8085, 50053, "127.0.0.1") + server.SetTlsConfig(8443, certPath) + server.SetConnectPorts(0, 8444) + if err := server.UpdateSnapshot(); err != nil { + t.Fatalf("UpdateSnapshot failed: %v", err) } - - // Default: no ALPN anywhere - listeners := snapshotListeners(t, false) - if alpn := downstreamTLS(t, listeners[IngressHTTPSListener]).GetCommonTlsContext().GetAlpnProtocols(); len(alpn) != 0 { - t.Errorf("HTTPS listener ALPN with knob off = %v, want none", alpn) + res, err := server.snapshot.GetSnapshot(NodeID) + if err != nil { + t.Fatalf("Failed to get snapshot: %v", err) + } + listeners := map[string]any{} + for name, l := range res.(*cachev3.Snapshot).GetResources(resourcev3.ListenerType) { + listeners[name] = l } - // Enabled: the HTTPS listener offers h2 then http/1.1; the CONNECT-TLS - // listener stays untouched. - listeners = snapshotListeners(t, true) + // h2 first: ALPN is server-preference, and a client that can speak HTTP/2 + // must land on it rather than on http/1.1. alpn := downstreamTLS(t, listeners[IngressHTTPSListener]).GetCommonTlsContext().GetAlpnProtocols() if len(alpn) != 2 || alpn[0] != "h2" || alpn[1] != "http/1.1" { - t.Errorf("HTTPS listener ALPN with knob on = %v, want [h2 http/1.1]", alpn) + t.Errorf("HTTPS listener ALPN = %v, want [h2 http/1.1]", alpn) } if alpn := downstreamTLS(t, listeners["connect_terminate_tls"]).GetCommonTlsContext().GetAlpnProtocols(); len(alpn) != 0 { - t.Errorf("CONNECT-TLS listener ALPN = %v, want none regardless of the knob", alpn) + t.Errorf("CONNECT-TLS listener ALPN = %v, want none", alpn) + } + + // The offer is only half the contract: the HCM must honor whatever ALPN + // negotiated. An explicit HTTP1 codec here would turn every h2 client + // into a connection error while the ALPN list still looked right. + https := listeners[IngressHTTPSListener].(*listenerv3.Listener) + hcm := &hcmv3.HttpConnectionManager{} + if err := https.GetFilterChains()[0].GetFilters()[0].GetTypedConfig().UnmarshalTo(hcm); err != nil { + t.Fatalf("Failed to unmarshal the HTTPS listener's HCM config: %v", err) + } + if hcm.GetCodecType() != hcmv3.HttpConnectionManager_AUTO { + t.Errorf("HTTPS listener HCM codec = %v, want AUTO so the negotiated protocol is honored", hcm.GetCodecType()) } } diff --git a/internal/atunnel/ingress_test.go b/internal/atunnel/ingress_test.go index 7ad1a96603..a93fd7cc6d 100644 --- a/internal/atunnel/ingress_test.go +++ b/internal/atunnel/ingress_test.go @@ -420,6 +420,115 @@ func TestMutualTLSClientIdentity(t *testing.T) { } } +// TestServeNegotiatesH2 checks that the ingress server negotiates h2 with a +// client that offers it, and HTTP/1.1 with one that does not. The router's +// HTTP/2 pool depends on the h2 side, which holds only because ServeTLS +// enables HTTP/2 when tlsConfig.NextProtos is empty — this pins that. +func TestServeNegotiatesH2(t *testing.T) { + dir := t.TempDir() + ca := newTestCA(t) + serverCert := ca.issue(t, "", []x509.ExtKeyUsage{x509.ExtKeyUsageServerAuth}) + bundlePath := filepath.Join(dir, "server.pem") + trustPath := filepath.Join(dir, "trust.pem") + writeCredentialBundle(t, bundlePath, serverCert) + if err := os.WriteFile(trustPath, ca.certPEM, 0o600); err != nil { + t.Fatal(err) + } + clientCert := ca.issue(t, "spiffe://cluster.local/ns/ate-system/sa/atenet-router", []x509.ExtKeyUsage{x509.ExtKeyUsageClientAuth}) + + // The actor: an h2c-capable backend, so gRPC-shaped requests can arrive + // as HTTP/2 while everything else must still be downgraded to HTTP/1.1. + backendAddr, protoSeen := mirrorBackend(t, true, true) + upstream, err := url.Parse("http://" + backendAddr) + if err != nil { + t.Fatal(err) + } + + s, err := NewServer(Config{ + CredentialBundlePath: bundlePath, + TrustBundlePath: trustPath, + AllowedClientID: "spiffe://cluster.local/ns/ate-system/sa/atenet-router", + Upstream: upstream, + }) + if err != nil { + t.Fatal(err) + } + if err := s.Activate("team-a", "actor-1"); err != nil { + t.Fatal(err) + } + + lis, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + ctx, cancel := context.WithCancel(context.Background()) + served := make(chan error, 1) + go func() { served <- s.Serve(ctx, lis) }() + t.Cleanup(func() { + cancel() + if err := <-served; err != nil { + t.Errorf("Serve: %v", err) + } + }) + + client := func(h2 bool) *http.Client { + transport := &http.Transport{ + TLSClientConfig: &tls.Config{ + MinVersion: tls.VersionTLS12, + InsecureSkipVerify: true, // The handshake identity checks live in TestMutualTLSClientIdentity. + Certificates: []tls.Certificate{clientCert}, + }, + // With h2, the transport offers "h2" via ALPN like Envoy's + // HTTP/2 pool; without it, only http/1.1 is offered, like the + // HTTP/1.1 pool. + ForceAttemptHTTP2: h2, + } + return &http.Client{Transport: transport, Timeout: 10 * time.Second} + } + request := func(t *testing.T, c *http.Client, method, contentType string) *http.Response { + t.Helper() + req, err := http.NewRequest(method, "https://"+lis.Addr().String()+"/", http.NoBody) + if err != nil { + t.Fatal(err) + } + req.Host = "actor-1.team-a.actors.resources.substrate.ate.dev" + if contentType != "" { + req.Header.Set("Content-Type", contentType) + } + res, err := c.Do(req) + if err != nil { + t.Fatalf("%s request: %v", method, err) + } + res.Body.Close() + return res + } + + h2Client := client(true) + res := request(t, h2Client, http.MethodPost, "application/grpc") + if res.Proto != "HTTP/2.0" { + t.Fatalf("h2-only client negotiated %s, want HTTP/2.0 — Envoy's mirrored HTTP/2 pool cannot connect", res.Proto) + } + if got := <-protoSeen; got != "HTTP/2.0" { + t.Errorf("gRPC-shaped request reached the actor as %s, want HTTP/2.0", got) + } + // A non-gRPC request on the same negotiated h2 connection is downgraded + // before the actor. + request(t, h2Client, http.MethodGet, "") + if got := <-protoSeen; got != "HTTP/1.1" { + t.Errorf("plain GET over h2 reached the actor as %s, want HTTP/1.1", got) + } + + // Envoy's HTTP/1.1 pool offers only http/1.1; it must not be dragged onto h2. + h1Client := client(false) + res = request(t, h1Client, http.MethodGet, "") + if res.Proto != "HTTP/1.1" { + t.Errorf("http/1.1-only client negotiated %s, want HTTP/1.1", res.Proto) + } + if got := <-protoSeen; got != "HTTP/1.1" { + t.Errorf("HTTP/1.1 request reached the actor as %s, want HTTP/1.1", got) + } +} + func TestDeactivateCancelsInflightRequest(t *testing.T) { upstream, err := url.Parse("http://actor.internal:80") if err != nil { diff --git a/internal/e2e/suites/networking/grpcingress_test.go b/internal/e2e/suites/networking/grpcingress_test.go index 9e9fdaec27..e2a3d8b767 100644 --- a/internal/e2e/suites/networking/grpcingress_test.go +++ b/internal/e2e/suites/networking/grpcingress_test.go @@ -143,7 +143,7 @@ func TestIngressGRPC(t *testing.T) { // Cleartext h2c to the router's HTTP port, with the Actor's DNS name as the // :authority — the same routing key every other ingress test in this suite // uses, just carried by a gRPC client instead of an HTTP one. The h2 ALPN - // knob (--https-h2) is about the *TLS* listener; nothing here needs it. + // offer is about the *TLS* listener; nothing here needs it. conn, err := grpc.NewClient(routerAddress(t, ctx), grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithAuthority(resources.ActorDNSName(actorRef)), diff --git a/manifests/ate-install/atenet-router.yaml b/manifests/ate-install/atenet-router.yaml index b3afb989b7..018dbd0a4a 100644 --- a/manifests/ate-install/atenet-router.yaml +++ b/manifests/ate-install/atenet-router.yaml @@ -208,12 +208,6 @@ spec: # turns to survive a shutdown, raise --drain-timeout and # terminationGracePeriodSeconds alongside it. # - "--route-timeout=5m" - # Enables HTTP/2 via ALPN on the HTTPS listener, which gRPC to actors - # over TLS requires. HTTP/1.1 clients are unaffected, and only gRPC - # continues to the actor as HTTP/2 — atunnel downgrades other HTTP/2 - # requests to HTTP/1.1, so HTTP/1.1-only actors keep working the way they used to. Leaving - # it off means no ALPN negotiation. - # - "--https-h2" env: - name: POD_NAME valueFrom: