feat: Implement enabling HTTP/1.1 trailers (envoyproxy#2492)

* feat: Support HTTP1 Trailers Signed-off-by: Lior Okman <[email protected]> * Add unit tests. Signed-off-by: Lior Okman <[email protected]> --------- Signed-off-by: Lior Okman <[email protected]>
guydc · Jan 24, 2024 · 8ce7914 · 8ce7914
1 parent 100d310
commit 8ce7914
Show file tree

Hide file tree

Showing 16 changed files with 381 additions and 44 deletions.
diff --git a/internal/gatewayapi/clienttrafficpolicy.go b/internal/gatewayapi/clienttrafficpolicy.go
@@ -298,6 +298,9 @@ func (t *Translator) translateClientTrafficPolicyForListener(policySpec *egv1a1.
 		// Translate Path Settings
 		translatePathSettings(policySpec.Path, httpIR)
 
+		// Translate HTTP1 Settings
+		translateHTTP1Settings(policySpec.HTTP1, httpIR)
+
 		// enable http3 if set and TLS is enabled
 		if httpIR.TLS != nil && policySpec.HTTP3 != nil {
 			httpIR.HTTP3 = &ir.HTTP3Settings{}
@@ -378,6 +381,15 @@ func translateListenerSuppressEnvoyHeaders(suppressEnvoyHeaders *bool, httpIR *i
 	}
 }
 
+func translateHTTP1Settings(http1Settings *egv1a1.HTTP1Settings, httpIR *ir.HTTPListener) {
+	if http1Settings == nil {
+		return
+	}
+	httpIR.HTTP1 = &ir.HTTP1Settings{
+		EnableTrailers: ptr.Deref(http1Settings.EnableTrailers, false),
+	}
+}
+
 func translateListenerTLSParameters(tlsParams *egv1a1.TLSSettings, httpIR *ir.HTTPListener) {
 	// Return if this listener isn't a TLS listener. There has to be
 	// at least one certificate defined, which would cause httpIR to

diff --git a/internal/gatewayapi/helpers.go b/internal/gatewayapi/helpers.go
@@ -13,6 +13,7 @@ import (
 	v1 "k8s.io/api/core/v1"
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/utils/ptr"
 	gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
 	"sigs.k8s.io/gateway-api/apis/v1alpha2"
 
@@ -66,34 +67,12 @@ func ObjectNamePtr(val string) *v1alpha2.ObjectName {
 	return &objectName
 }
 
-func PathMatchTypeDerefOr(matchType *gwapiv1.PathMatchType, defaultType gwapiv1.PathMatchType) gwapiv1.PathMatchType {
-	if matchType != nil {
-		return *matchType
-	}
-	return defaultType
-}
-
-func GRPCMethodMatchTypeDerefOr(matchType *v1alpha2.GRPCMethodMatchType, defaultType v1alpha2.GRPCMethodMatchType) v1alpha2.GRPCMethodMatchType {
-	if matchType != nil {
-		return *matchType
-	}
-	return defaultType
-}
-
-func HeaderMatchTypeDerefOr(matchType *gwapiv1.HeaderMatchType, defaultType gwapiv1.HeaderMatchType) gwapiv1.HeaderMatchType {
-	if matchType != nil {
-		return *matchType
-	}
-	return defaultType
-}
-
-func QueryParamMatchTypeDerefOr(matchType *gwapiv1.QueryParamMatchType,
-	defaultType gwapiv1.QueryParamMatchType) gwapiv1.QueryParamMatchType {
-	if matchType != nil {
-		return *matchType
-	}
-	return defaultType
-}
+var (
+	PathMatchTypeDerefOr       = ptr.Deref[gwapiv1.PathMatchType]
+	GRPCMethodMatchTypeDerefOr = ptr.Deref[v1alpha2.GRPCMethodMatchType]
+	HeaderMatchTypeDerefOr     = ptr.Deref[gwapiv1.HeaderMatchType]
+	QueryParamMatchTypeDerefOr = ptr.Deref[gwapiv1.QueryParamMatchType]
+)
 
 func NamespaceDerefOr(namespace *gwapiv1.Namespace, defaultNamespace string) string {
 	if namespace != nil && *namespace != "" {

diff --git a/internal/gatewayapi/testdata/clienttrafficpolicy-trailers.in.yaml b/internal/gatewayapi/testdata/clienttrafficpolicy-trailers.in.yaml
@@ -0,0 +1,35 @@
+clientTrafficPolicies:
+- apiVersion: gateway.envoyproxy.io/v1alpha1
+  kind: ClientTrafficPolicy
+  metadata:
+    namespace: envoy-gateway
+    name: target-gateway-1
+  spec:
+    http1:
+      enableTrailers: true
+    targetRef:
+      group: gateway.networking.k8s.io
+      kind: Gateway
+      name: gateway-1
+      namespace: envoy-gateway
+gateways:
+- apiVersion: gateway.networking.k8s.io/v1
+  kind: Gateway
+  metadata:
+    namespace: envoy-gateway
+    name: gateway-1
+  spec:
+    gatewayClassName: envoy-gateway-class
+    listeners:
+    - name: http-1
+      protocol: HTTP
+      port: 80
+      allowedRoutes:
+        namespaces:
+          from: Same
+    - name: http-2
+      protocol: HTTP
+      port: 8080
+      allowedRoutes:
+        namespaces:
+          from: Same
diff --git a/internal/gatewayapi/testdata/clienttrafficpolicy-trailers.out.yaml b/internal/gatewayapi/testdata/clienttrafficpolicy-trailers.out.yaml
@@ -0,0 +1,143 @@
+clientTrafficPolicies:
+- apiVersion: gateway.envoyproxy.io/v1alpha1
+  kind: ClientTrafficPolicy
+  metadata:
+    creationTimestamp: null
+    name: target-gateway-1
+    namespace: envoy-gateway
+  spec:
+    http1:
+      enableTrailers: true
+    targetRef:
+      group: gateway.networking.k8s.io
+      kind: Gateway
+      name: gateway-1
+      namespace: envoy-gateway
+  status:
+    conditions:
+    - lastTransitionTime: null
+      message: ClientTrafficPolicy has been accepted.
+      reason: Accepted
+      status: "True"
+      type: Accepted
+gateways:
+- apiVersion: gateway.networking.k8s.io/v1
+  kind: Gateway
+  metadata:
+    creationTimestamp: null
+    name: gateway-1
+    namespace: envoy-gateway
+  spec:
+    gatewayClassName: envoy-gateway-class
+    listeners:
+    - allowedRoutes:
+        namespaces:
+          from: Same
+      name: http-1
+      port: 80
+      protocol: HTTP
+    - allowedRoutes:
+        namespaces:
+          from: Same
+      name: http-2
+      port: 8080
+      protocol: HTTP
+  status:
+    listeners:
+    - attachedRoutes: 0
+      conditions:
+      - lastTransitionTime: null
+        message: Sending translated listener configuration to the data plane
+        reason: Programmed
+        status: "True"
+        type: Programmed
+      - lastTransitionTime: null
+        message: Listener has been successfully translated
+        reason: Accepted
+        status: "True"
+        type: Accepted
+      - lastTransitionTime: null
+        message: Listener references have been resolved
+        reason: ResolvedRefs
+        status: "True"
+        type: ResolvedRefs
+      name: http-1
+      supportedKinds:
+      - group: gateway.networking.k8s.io
+        kind: HTTPRoute
+      - group: gateway.networking.k8s.io
+        kind: GRPCRoute
+    - attachedRoutes: 0
+      conditions:
+      - lastTransitionTime: null
+        message: Sending translated listener configuration to the data plane
+        reason: Programmed
+        status: "True"
+        type: Programmed
+      - lastTransitionTime: null
+        message: Listener has been successfully translated
+        reason: Accepted
+        status: "True"
+        type: Accepted
+      - lastTransitionTime: null
+        message: Listener references have been resolved
+        reason: ResolvedRefs
+        status: "True"
+        type: ResolvedRefs
+      name: http-2
+      supportedKinds:
+      - group: gateway.networking.k8s.io
+        kind: HTTPRoute
+      - group: gateway.networking.k8s.io
+        kind: GRPCRoute
+infraIR:
+  envoy-gateway/gateway-1:
+    proxy:
+      listeners:
+      - address: null
+        name: envoy-gateway/gateway-1/http-1
+        ports:
+        - containerPort: 10080
+          name: http-1
+          protocol: HTTP
+          servicePort: 80
+      - address: null
+        name: envoy-gateway/gateway-1/http-2
+        ports:
+        - containerPort: 8080
+          name: http-2
+          protocol: HTTP
+          servicePort: 8080
+      metadata:
+        labels:
+          gateway.envoyproxy.io/owning-gateway-name: gateway-1
+          gateway.envoyproxy.io/owning-gateway-namespace: envoy-gateway
+      name: envoy-gateway/gateway-1
+xdsIR:
+  envoy-gateway/gateway-1:
+    accessLog:
+      text:
+      - path: /dev/stdout
+    http:
+    - address: 0.0.0.0
+      hostnames:
+      - '*'
+      http1:
+        enableTrailers: true
+      isHTTP2: false
+      name: envoy-gateway/gateway-1/http-1
+      path:
+        escapedSlashesAction: UnescapeAndRedirect
+        mergeSlashes: true
+      port: 10080
+    - address: 0.0.0.0
+      hostnames:
+      - '*'
+      http1:
+        enableTrailers: true
+      isHTTP2: false
+      name: envoy-gateway/gateway-1/http-2
+      path:
+        escapedSlashesAction: UnescapeAndRedirect
+        mergeSlashes: true
+      port: 8080
diff --git a/internal/ir/infra.go b/internal/ir/infra.go
@@ -73,6 +73,12 @@ type ProxyListener struct {
 type HTTP3Settings struct {
 }
 
+// HTTP1Settings provides HTTP/1 configuration on the listener.
+// +k8s:deepcopy-gen=true
+type HTTP1Settings struct {
+	EnableTrailers bool `json:"enableTrailers,omitempty" yaml:"enableTrailers,omitempty"`
+}
+
 // ListenerPort defines a network port of a listener.
 // +k8s:deepcopy-gen=true
 type ListenerPort struct {

diff --git a/internal/ir/xds.go b/internal/ir/xds.go
@@ -208,6 +208,9 @@ type HTTPListener struct {
 	HTTP3 *HTTP3Settings `json:"http3,omitempty"`
 	// Path contains settings for path URI manipulations
 	Path PathSettings `json:"path,omitempty"`
+	// HTTP1 provides HTTP/1 configuration on the listener
+	// +optional
+	HTTP1 *HTTP1Settings `json:"http1,omitempty" yaml:"http1,omitempty"`
 }
 
 // Validate the fields within the HTTPListener structure

diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go
diff --git a/internal/xds/translator/cluster.go b/internal/xds/translator/cluster.go
@@ -42,6 +42,7 @@ type xdsClusterArgs struct {
 	proxyProtocol  *ir.ProxyProtocol
 	circuitBreaker *ir.CircuitBreaker
 	healthCheck    *ir.HealthCheck
+	enableTrailers bool
 }
 
 type EndpointType int
@@ -95,9 +96,7 @@ func buildXdsCluster(args *xdsClusterArgs) *clusterv3.Cluster {
 			break
 		}
 	}
-	if isHTTP2 {
-		cluster.TypedExtensionProtocolOptions = buildTypedExtensionProtocolOptions()
-	}
+	cluster.TypedExtensionProtocolOptions = buildTypedExtensionProtocolOptions(isHTTP2, args.enableTrailers)
 
 	// Set Load Balancer policy
 	//nolint:gocritic
@@ -315,22 +314,44 @@ func buildXdsClusterLoadAssignment(clusterName string, destSettings []*ir.Destin
 	return &endpointv3.ClusterLoadAssignment{ClusterName: clusterName, Endpoints: localities}
 }
 
-func buildTypedExtensionProtocolOptions() map[string]*anypb.Any {
-	protocolOptions := httpv3.HttpProtocolOptions{
-		UpstreamProtocolOptions: &httpv3.HttpProtocolOptions_ExplicitHttpConfig_{
-			ExplicitHttpConfig: &httpv3.HttpProtocolOptions_ExplicitHttpConfig{
-				ProtocolConfig: &httpv3.HttpProtocolOptions_ExplicitHttpConfig_Http2ProtocolOptions{},
+func buildTypedExtensionProtocolOptions(http2, http1Trailers bool) map[string]*anypb.Any {
+	var anyProtocolOptions *anypb.Any
+
+	if http2 {
+		protocolOptions := httpv3.HttpProtocolOptions{
+			UpstreamProtocolOptions: &httpv3.HttpProtocolOptions_ExplicitHttpConfig_{
+				ExplicitHttpConfig: &httpv3.HttpProtocolOptions_ExplicitHttpConfig{
+					ProtocolConfig: &httpv3.HttpProtocolOptions_ExplicitHttpConfig_Http2ProtocolOptions{},
+				},
 			},
-		},
+		}
+
+		anyProtocolOptions, _ = anypb.New(&protocolOptions)
+	} else if http1Trailers {
+		// TODO: If the cluster is TLS enabled, use AutoHTTPConfig instead of ExplicitHttpConfig
+		// so that when ALPN is supported enabling trailers doesn't force HTTP/1.1
+		protocolOptions := httpv3.HttpProtocolOptions{
+			UpstreamProtocolOptions: &httpv3.HttpProtocolOptions_ExplicitHttpConfig_{
+				ExplicitHttpConfig: &httpv3.HttpProtocolOptions_ExplicitHttpConfig{
+					ProtocolConfig: &httpv3.HttpProtocolOptions_ExplicitHttpConfig_HttpProtocolOptions{
+						HttpProtocolOptions: &corev3.Http1ProtocolOptions{
+							EnableTrailers: http1Trailers,
+						},
+					},
+				},
+			},
+		}
+		anyProtocolOptions, _ = anypb.New(&protocolOptions)
 	}
 
-	anyProtocolOptions, _ := anypb.New(&protocolOptions)
+	if anyProtocolOptions != nil {
+		extensionOptions := map[string]*anypb.Any{
+			extensionOptionsKey: anyProtocolOptions,
+		}
 
-	extensionOptions := map[string]*anypb.Any{
-		extensionOptionsKey: anyProtocolOptions,
+		return extensionOptions
 	}
-
-	return extensionOptions
+	return nil
 }
 
 // buildClusterName returns a cluster name for the given `host` and `port`.