From b49ca5a960d0c825e55eeb5c7c0310d0b7b28a05 Mon Sep 17 00:00:00 2001 From: Alex Savanovich <40720931+savme@users.noreply.github.com> Date: Fri, 16 Oct 2026 10:34:01 +0100 Subject: [PATCH] feat: allow h2 in ALPN --- internal/config/config.go | 28 +++++++++-- internal/config/config_test.go | 46 +++++++++++++++++++ internal/config/zz_generated.deepcopy.go | 5 ++ .../gateway_client_traffic_policy.go | 13 ++++++ .../gateway_client_traffic_policy_test.go | 19 +++++++- 5 files changed, 106 insertions(+), 5 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index d7655244..1d8c5091 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -1169,11 +1169,17 @@ type DownstreamClientTrafficConfig struct { // HTTP3 turns on HTTP/3 for the listeners. // +optional HTTP3 *envoygatewayv1alpha1.HTTP3Settings `json:"http3,omitempty"` + + // ALPNProtocols lists the protocols offered in the TLS handshake. Listing h2 + // offers HTTP/2 on managed hostnames, where Envoy Gateway otherwise offers + // only http/1.1. + // +optional + ALPNProtocols []envoygatewayv1alpha1.ALPNProtocol `json:"alpnProtocols,omitempty"` } // IsEmpty reports whether nothing is set, so no policy is written. func (c DownstreamClientTrafficConfig) IsEmpty() bool { - return c.HTTP2 == nil && c.HTTP3 == nil + return c.HTTP2 == nil && c.HTTP3 == nil && len(c.ALPNProtocols) == 0 } // http2WindowMin and http2WindowMax bound an HTTP/2 window [bytes] as Envoy @@ -1185,17 +1191,33 @@ const ( http2WindowMax = 2147483647 ) -// validate refuses an HTTP/2 window the edge would ignore without saying so. +// validate refuses an HTTP/2 window or ALPN protocol the edge would ignore +// without saying so. func (c DownstreamClientTrafficConfig) validate() error { + alpnErr := validateALPNProtocols(c.ALPNProtocols) if c.HTTP2 == nil { - return nil + return alpnErr } return errors.Join( + alpnErr, validateHTTP2Window("initialStreamWindowSize", c.HTTP2.InitialStreamWindowSize), validateHTTP2Window("initialConnectionWindowSize", c.HTTP2.InitialConnectionWindowSize), ) } +func validateALPNProtocols(protocols []envoygatewayv1alpha1.ALPNProtocol) error { + for _, p := range protocols { + switch p { + case envoygatewayv1alpha1.HTTPProtocolVersion2, + envoygatewayv1alpha1.HTTPProtocolVersion1_1, + envoygatewayv1alpha1.HTTPProtocolVersion1_0: + default: + return fmt.Errorf("downstreamClientTraffic.alpnProtocols must each be h2, http/1.1 or http/1.0, got %q", p) + } + } + return nil +} + func validateHTTP2Window(field string, size *resource.Quantity) error { if size == nil { return nil diff --git a/internal/config/config_test.go b/internal/config/config_test.go index cced5bad..55e09a4e 100644 --- a/internal/config/config_test.go +++ b/internal/config/config_test.go @@ -3,6 +3,7 @@ package config import ( "os" "path/filepath" + "slices" "strings" "testing" "time" @@ -323,6 +324,7 @@ gateway: initialStreamWindowSize: 1Mi initialConnectionWindowSize: 16Mi http3: {} + alpnProtocols: [h2, http/1.1] `) var cfg NetworkServicesOperator decodeInto(t, data, &cfg) @@ -340,6 +342,10 @@ gateway: if traffic.HTTP3 == nil { t.Error("expected http3: {} to turn HTTP/3 on") } + want := []envoygatewayv1alpha1.ALPNProtocol{"h2", "http/1.1"} + if !slices.Equal(traffic.ALPNProtocols, want) { + t.Errorf("expected alpnProtocols %v, got %v", want, traffic.ALPNProtocols) + } } func TestDownstreamClientTrafficUnsetIsEmpty(t *testing.T) { @@ -350,6 +356,46 @@ func TestDownstreamClientTrafficUnsetIsEmpty(t *testing.T) { } } +// ALPN alone is enough to write a policy; an empty list is not, since it would +// turn ALPN off rather than leave the default. +func TestDownstreamClientTrafficALPNIsEmpty(t *testing.T) { + alpn := DownstreamClientTrafficConfig{ALPNProtocols: []envoygatewayv1alpha1.ALPNProtocol{"h2", "http/1.1"}} + if alpn.IsEmpty() { + t.Error("expected listed ALPN protocols to write a policy") + } + if !(DownstreamClientTrafficConfig{ALPNProtocols: []envoygatewayv1alpha1.ALPNProtocol{}}).IsEmpty() { + t.Error("expected an empty ALPN list to write nothing") + } +} + +// A protocol Envoy does not know is refused at start: the edge would negotiate +// without it, and nothing the operator reads would say so. +func TestGatewayConfig_ValidateALPNProtocols(t *testing.T) { + for _, tt := range []struct { + name string + alpn []envoygatewayv1alpha1.ALPNProtocol + wantSub string + }{ + {name: "h2 and http/1.1", alpn: []envoygatewayv1alpha1.ALPNProtocol{"h2", "http/1.1"}}, + {name: "every known protocol", alpn: []envoygatewayv1alpha1.ALPNProtocol{"h2", "http/1.1", "http/1.0"}}, + {name: "a typo", alpn: []envoygatewayv1alpha1.ALPNProtocol{"h2", "http1.1"}, wantSub: `"http1.1"`}, + {name: "h3 belongs to http3", alpn: []envoygatewayv1alpha1.ALPNProtocol{"h3"}, wantSub: `"h3"`}, + } { + t.Run(tt.name, func(t *testing.T) { + cfg := &NetworkServicesOperator{Gateway: GatewayConfig{DownstreamClientTraffic: DownstreamClientTrafficConfig{ + ALPNProtocols: tt.alpn, + }}} + err := cfg.Validate() + if tt.wantSub == "" && err != nil { + t.Fatalf("expected nil, got %v", err) + } + if tt.wantSub != "" && (err == nil || !strings.Contains(err.Error(), tt.wantSub)) { + t.Fatalf("expected an error containing %q, got %v", tt.wantSub, err) + } + }) + } +} + // An HTTP/2 window outside Envoy's range is refused at start: the edge would // keep its defaults, and nothing the operator reads would say so. func TestGatewayConfig_ValidateHTTP2Windows(t *testing.T) { diff --git a/internal/config/zz_generated.deepcopy.go b/internal/config/zz_generated.deepcopy.go index 1ac184cc..5f0d9830 100644 --- a/internal/config/zz_generated.deepcopy.go +++ b/internal/config/zz_generated.deepcopy.go @@ -326,6 +326,11 @@ func (in *DownstreamClientTrafficConfig) DeepCopyInto(out *DownstreamClientTraff *out = new(v1alpha1.HTTP3Settings) **out = **in } + if in.ALPNProtocols != nil { + in, out := &in.ALPNProtocols, &out.ALPNProtocols + *out = make([]v1alpha1.ALPNProtocol, len(*in)) + copy(*out, *in) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new DownstreamClientTrafficConfig. diff --git a/internal/controller/gateway_client_traffic_policy.go b/internal/controller/gateway_client_traffic_policy.go index f8b29c7d..12d2bc3c 100644 --- a/internal/controller/gateway_client_traffic_policy.go +++ b/internal/controller/gateway_client_traffic_policy.go @@ -3,6 +3,7 @@ package controller import ( "context" "fmt" + "slices" envoygatewayv1alpha1 "github.com/envoyproxy/gateway/api/v1alpha1" apimeta "k8s.io/apimachinery/pkg/api/meta" @@ -72,6 +73,7 @@ func ensureDownstreamClientTrafficPolicy( PolicyTargetReferences: envoygatewayv1alpha1.PolicyTargetReferences{TargetRefs: targets}, HTTP2: traffic.HTTP2.DeepCopy(), HTTP3: traffic.HTTP3.DeepCopy(), + TLS: clientTLSSettings(traffic.ALPNProtocols), } return controllerutil.SetControllerReference(gateway, policy, cl.Scheme()) }) @@ -81,6 +83,17 @@ func ensureDownstreamClientTrafficPolicy( return nil } +// clientTLSSettings carries only the ALPN protocols. An empty list would turn +// ALPN off, so nothing is written unless protocols are listed. +func clientTLSSettings(protocols []envoygatewayv1alpha1.ALPNProtocol) *envoygatewayv1alpha1.ClientTLSSettings { + if len(protocols) == 0 { + return nil + } + return &envoygatewayv1alpha1.ClientTLSSettings{ + TLSSettings: envoygatewayv1alpha1.TLSSettings{ALPNProtocols: slices.Clone(protocols)}, + } +} + // clientTrafficPolicySuffix keeps the operator's policy name apart from the // -http3 that staging's Kyverno policy writes into the same namespace: // a name ending in it can never end in -http3, and the hash a long name gets diff --git a/internal/controller/gateway_client_traffic_policy_test.go b/internal/controller/gateway_client_traffic_policy_test.go index 72b35033..91d27493 100644 --- a/internal/controller/gateway_client_traffic_policy_test.go +++ b/internal/controller/gateway_client_traffic_policy_test.go @@ -68,11 +68,24 @@ func TestDownstreamClientTrafficPolicy(t *testing.T) { assert.True(t, policy.Spec.HTTP2.InitialStreamWindowSize.Equal(resource.MustParse("1Mi"))) assert.True(t, policy.Spec.HTTP2.InitialConnectionWindowSize.Equal(resource.MustParse("16Mi"))) assert.Nil(t, policy.Spec.HTTP3, "http3 is not configured, so it is not written") + assert.Nil(t, policy.Spec.TLS, "no ALPN is configured, so TLS is left to Envoy Gateway") require.NoError(t, ensureDownstreamClientTrafficPolicy(ctx, cl, withHTTP3, gw)) assert.NotNil(t, getClientTrafficPolicy(t, ctx, cl, gw).Spec.HTTP3, "staging's http3 is carried") }) + t.Run("CarriesOnlyTheALPNProtocols", func(t *testing.T) { + gw := createDownstreamGateway(t, ctx, cl, "alpn", httpsListener("https")) + alpn := config.DownstreamClientTrafficConfig{ALPNProtocols: []envoygatewayv1alpha1.ALPNProtocol{"h2", "http/1.1"}} + require.NoError(t, ensureDownstreamClientTrafficPolicy(ctx, cl, alpn, gw)) + tls := getClientTrafficPolicy(t, ctx, cl, gw).Spec.TLS + require.NotNil(t, tls, "ALPN alone writes a policy") + assert.Equal(t, alpn.ALPNProtocols, tls.ALPNProtocols) + assert.Equal(t, envoygatewayv1alpha1.ClientTLSSettings{ + TLSSettings: envoygatewayv1alpha1.TLSSettings{ALPNProtocols: alpn.ALPNProtocols}, + }, *tls, "no other TLS setting reaches every tenant's listener") + }) + t.Run("BelongsToTheGatewayAndCarriesItsUpstreamLabels", func(t *testing.T) { gw := createDownstreamGateway(t, ctx, cl, "d", httpsListener("https")) require.NoError(t, ensureDownstreamClientTrafficPolicy(ctx, cl, windows, gw)) @@ -94,9 +107,11 @@ func TestDownstreamClientTrafficPolicy(t *testing.T) { t.Run("SecondPassWritesNothing", func(t *testing.T) { gw := createDownstreamGateway(t, ctx, cl, "f", httpsListener("https")) - require.NoError(t, ensureDownstreamClientTrafficPolicy(ctx, cl, withHTTP3, gw)) + everything := withHTTP3 + everything.ALPNProtocols = []envoygatewayv1alpha1.ALPNProtocol{"h2", "http/1.1"} + require.NoError(t, ensureDownstreamClientTrafficPolicy(ctx, cl, everything, gw)) before := getClientTrafficPolicy(t, ctx, cl, gw).ResourceVersion - require.NoError(t, ensureDownstreamClientTrafficPolicy(ctx, cl, withHTTP3, gw)) + require.NoError(t, ensureDownstreamClientTrafficPolicy(ctx, cl, everything, gw)) assert.Equal(t, before, getClientTrafficPolicy(t, ctx, cl, gw).ResourceVersion, "an unchanged policy is not rewritten on every reconcile") })