Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(translator): http2 upstream settings #3682

Merged
merged 15 commits into from
Aug 5, 2024
5 changes: 5 additions & 0 deletions api/v1alpha1/backendtrafficpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,11 @@ type BackendTrafficPolicySpec struct {
//
// +optional
Connection *BackendConnection `json:"connection,omitempty"`

// HTTP2 provides HTTP/2 configuration for backend connections.
//
// +optional
HTTP2 *HTTP2Settings `json:"http2,omitempty"`
}

// +kubebuilder:object:root=true
Expand Down
25 changes: 0 additions & 25 deletions api/v1alpha1/clienttrafficpolicy_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
package v1alpha1

import (
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
gwapiv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2"
)
Expand Down Expand Up @@ -289,30 +288,6 @@ type HTTP10Settings struct {
UseDefaultHost *bool `json:"useDefaultHost,omitempty"`
}

// HTTP2Settings provides HTTP/2 configuration on the listener.
type HTTP2Settings struct {
// InitialStreamWindowSize sets the initial window size for HTTP/2 streams.
// If not set, the default value is 64 KiB(64*1024).
//
// +kubebuilder:validation:XValidation:rule="type(self) == string ? self.matches(r\"^[1-9]+[0-9]*([EPTGMK]i|[EPTGMk])?$\") : type(self) == int",message="initialStreamWindowSize must be of the format \"^[1-9]+[0-9]*([EPTGMK]i|[EPTGMk])?$\""
// +optional
InitialStreamWindowSize *resource.Quantity `json:"initialStreamWindowSize,omitempty"`

// InitialConnectionWindowSize sets the initial window size for HTTP/2 connections.
// If not set, the default value is 1 MiB.
//
// +kubebuilder:validation:XValidation:rule="type(self) == string ? self.matches(r\"^[1-9]+[0-9]*([EPTGMK]i|[EPTGMk])?$\") : type(self) == int",message="initialConnectionWindowSize must be of the format \"^[1-9]+[0-9]*([EPTGMK]i|[EPTGMk])?$\""
// +optional
InitialConnectionWindowSize *resource.Quantity `json:"initialConnectionWindowSize,omitempty"`

// MaxConcurrentStreams sets the maximum number of concurrent streams allowed per connection.
// If not set, the default value is 100.
// +kubebuilder:validation:Minimum=1
// +kubebuilder:validation:Maximum=2147483647
// +optional
MaxConcurrentStreams *uint32 `json:"maxConcurrentStreams,omitempty"`
}

// HealthCheckSettings provides HealthCheck configuration on the HTTP/HTTPS listener.
type HealthCheckSettings struct {
// Path specifies the HTTP path to match on for health check requests.
Expand Down
32 changes: 32 additions & 0 deletions api/v1alpha1/shared_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
autoscalingv2 "k8s.io/api/autoscaling/v2"
corev1 "k8s.io/api/core/v1"
apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1"
"k8s.io/apimachinery/pkg/api/resource"
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"
)

Expand Down Expand Up @@ -478,3 +479,34 @@ type BackendRef struct {
// A CIDR can be an IPv4 address range such as "192.168.1.0/24" or an IPv6 address range such as "2001:0db8:11a3:09d7::/64".
// +kubebuilder:validation:Pattern=`((25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\.(25[0-5]|2[0-4][0-9]|[01]?[0-9][0-9]?)\/([0-9]+))|((([0-9a-fA-F]{1,4}:){7,7}[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,7}:|([0-9a-fA-F]{1,4}:){1,6}:[0-9a-fA-F]{1,4}|([0-9a-fA-F]{1,4}:){1,5}(:[0-9a-fA-F]{1,4}){1,2}|([0-9a-fA-F]{1,4}:){1,4}(:[0-9a-fA-F]{1,4}){1,3}|([0-9a-fA-F]{1,4}:){1,3}(:[0-9a-fA-F]{1,4}){1,4}|([0-9a-fA-F]{1,4}:){1,2}(:[0-9a-fA-F]{1,4}){1,5}|[0-9a-fA-F]{1,4}:((:[0-9a-fA-F]{1,4}){1,6})|:((:[0-9a-fA-F]{1,4}){1,7}|:)|fe80:(:[0-9a-fA-F]{0,4}){0,4}%[0-9a-zA-Z]{1,}|::(ffff(:0{1,4}){0,1}:){0,1}((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])|([0-9a-fA-F]{1,4}:){1,4}:((25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9])\.){3,3}(25[0-5]|(2[0-4]|1{0,1}[0-9]){0,1}[0-9]))\/([0-9]+))`
type CIDR string

// HTTP2Settings provides HTTP/2 configuration for listeners and backends.
type HTTP2Settings struct {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we use the same default value for both the listeners and clusters?

Copy link
Contributor Author

@guydc guydc Jul 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • envoy recommendations for secure-by-default are the same for window sizes.
  • max concurrent streams do not necessarily require hardening in the cluster context.
  • stream resetting behavior is mostly a downstream concern.

We can move some of the defaulting behavior to the API, but some will remain in the XDS translator (due to differences between clusters and listeners). So, I propose we keep it as-is for now.

Copy link
Member

@zhaohuabing zhaohuabing Jul 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@guydc Thanks for the explanation! I'm just curious because the recommendation is for listeners. They can be the same for Clusters if there's no unintentional side effects.

// InitialStreamWindowSize sets the initial window size for HTTP/2 streams.
// If not set, the default value is 64 KiB(64*1024).
//
// +kubebuilder:validation:XValidation:rule="type(self) == string ? self.matches(r\"^[1-9]+[0-9]*([EPTGMK]i|[EPTGMk])?$\") : type(self) == int",message="initialStreamWindowSize must be of the format \"^[1-9]+[0-9]*([EPTGMK]i|[EPTGMk])?$\""
// +optional
InitialStreamWindowSize *resource.Quantity `json:"initialStreamWindowSize,omitempty"`

// InitialConnectionWindowSize sets the initial window size for HTTP/2 connections.
// If not set, the default value is 1 MiB.
//
// +kubebuilder:validation:XValidation:rule="type(self) == string ? self.matches(r\"^[1-9]+[0-9]*([EPTGMK]i|[EPTGMk])?$\") : type(self) == int",message="initialConnectionWindowSize must be of the format \"^[1-9]+[0-9]*([EPTGMK]i|[EPTGMk])?$\""
// +optional
InitialConnectionWindowSize *resource.Quantity `json:"initialConnectionWindowSize,omitempty"`

// MaxConcurrentStreams sets the maximum number of concurrent streams allowed per connection.
// If not set, the default value is 100.
// +kubebuilder:validation:Minimum=1
// +kubebuilder:validation:Maximum=2147483647
// +optional
MaxConcurrentStreams *uint32 `json:"maxConcurrentStreams,omitempty"`

// ResetConnectionOnError determines if Envoy will terminate the connection or just the offending stream in the event of HTTP messaging error
guydc marked this conversation as resolved.
Show resolved Hide resolved
// It's recommended for L2 Envoy deployments to set this value to false.
// https://www.envoyproxy.io/docs/envoy/latest/configuration/best_practices/level_two
// Default: true
// +optional
TerminateConnectionOnMessagingError *bool `json:"terminateConnectionOnMessagingError,omitempty"`
guydc marked this conversation as resolved.
Show resolved Hide resolved
}
10 changes: 10 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -421,6 +421,50 @@ spec:
type: boolean
type: object
type: object
http2:
description: HTTP2 provides HTTP/2 configuration for backend connections.
properties:
initialConnectionWindowSize:
anyOf:
- type: integer
- type: string
description: |-
InitialConnectionWindowSize sets the initial window size for HTTP/2 connections.
If not set, the default value is 1 MiB.
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
x-kubernetes-validations:
- message: initialConnectionWindowSize must be of the format "^[1-9]+[0-9]*([EPTGMK]i|[EPTGMk])?$"
rule: 'type(self) == string ? self.matches(r"^[1-9]+[0-9]*([EPTGMK]i|[EPTGMk])?$")
: type(self) == int'
initialStreamWindowSize:
anyOf:
- type: integer
- type: string
description: |-
InitialStreamWindowSize sets the initial window size for HTTP/2 streams.
If not set, the default value is 64 KiB(64*1024).
pattern: ^(\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))(([KMGTPE]i)|[numkMGTPE]|([eE](\+|-)?(([0-9]+(\.[0-9]*)?)|(\.[0-9]+))))?$
x-kubernetes-int-or-string: true
x-kubernetes-validations:
- message: initialStreamWindowSize must be of the format "^[1-9]+[0-9]*([EPTGMK]i|[EPTGMk])?$"
rule: 'type(self) == string ? self.matches(r"^[1-9]+[0-9]*([EPTGMK]i|[EPTGMk])?$")
: type(self) == int'
maxConcurrentStreams:
description: |-
MaxConcurrentStreams sets the maximum number of concurrent streams allowed per connection.
If not set, the default value is 100.
format: int32
maximum: 2147483647
minimum: 1
type: integer
resetStreamOnError:
description: |-
ResetStreamOnError determines if Envoy will terminate the stream or the connection in the event of HTTP messaging error
It's recommended for L2 Envoy deployments to set this value to true.
https://www.envoyproxy.io/docs/envoy/latest/configuration/best_practices/level_two
type: boolean
type: object
loadBalancer:
description: |-
LoadBalancer policy to apply when routing traffic from the gateway to
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -300,6 +300,12 @@ spec:
maximum: 2147483647
minimum: 1
type: integer
resetStreamOnError:
description: |-
ResetStreamOnError determines if Envoy will terminate the stream or the connection in the event of HTTP messaging error
It's recommended for L2 Envoy deployments to set this value to true.
https://www.envoyproxy.io/docs/envoy/latest/configuration/best_practices/level_two
type: boolean
type: object
http3:
description: HTTP3 provides HTTP/3 configuration on the listener.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -804,7 +804,9 @@ xds:
envoy.extensions.upstreams.http.v3.HttpProtocolOptions:
'@type': type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions
explicitHttpConfig:
http2ProtocolOptions: {}
http2ProtocolOptions:
initialConnectionWindowSize: 1048576
initialStreamWindowSize: 65536
- cluster:
'@type': type.googleapis.com/envoy.config.cluster.v3.Cluster
circuitBreakers:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -504,7 +504,10 @@
"envoy.extensions.upstreams.http.v3.HttpProtocolOptions": {
"@type": "type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions",
"explicitHttpConfig": {
"http2ProtocolOptions": {}
"http2ProtocolOptions": {
"initialConnectionWindowSize": 1048576,
"initialStreamWindowSize": 65536
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,9 @@ xds:
envoy.extensions.upstreams.http.v3.HttpProtocolOptions:
'@type': type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions
explicitHttpConfig:
http2ProtocolOptions: {}
http2ProtocolOptions:
initialConnectionWindowSize: 1048576
initialStreamWindowSize: 65536
- cluster:
'@type': type.googleapis.com/envoy.config.cluster.v3.Cluster
circuitBreakers:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ xds:
envoy.extensions.upstreams.http.v3.HttpProtocolOptions:
'@type': type.googleapis.com/envoy.extensions.upstreams.http.v3.HttpProtocolOptions
explicitHttpConfig:
http2ProtocolOptions: {}
http2ProtocolOptions:
initialConnectionWindowSize: 1048576
initialStreamWindowSize: 65536
- cluster:
'@type': type.googleapis.com/envoy.config.cluster.v3.Cluster
circuitBreakers:
Expand Down
17 changes: 17 additions & 0 deletions internal/gatewayapi/backendtrafficpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@
ka *ir.TCPKeepalive
rt *ir.Retry
bc *ir.BackendConnection
h2 *ir.HTTP2Settings
err, errs error
)

Expand Down Expand Up @@ -349,6 +350,13 @@
}
}

if policy.Spec.HTTP2 != nil {
if h2, err = buildIRHTTP2Settings(policy.Spec.HTTP2); err != nil {
err = perr.WithMessage(err, "HTTP2")
errs = errors.Join(errs, err)

Check warning on line 356 in internal/gatewayapi/backendtrafficpolicy.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/backendtrafficpolicy.go#L355-L356

Added lines #L355 - L356 were not covered by tests
}
}

// Early return if got any errors
if errs != nil {
return errs
Expand Down Expand Up @@ -398,6 +406,7 @@
TCPKeepalive: ka,
Retry: rt,
BackendConnection: bc,
HTTP2: h2,
}

// Update the Host field in HealthCheck, now that we have access to the Route Hostname.
Expand Down Expand Up @@ -432,6 +441,7 @@
ct *ir.Timeout
ka *ir.TCPKeepalive
rt *ir.Retry
h2 *ir.HTTP2Settings
err, errs error
)

Expand Down Expand Up @@ -478,6 +488,12 @@
errs = errors.Join(errs, err)
}
}
if policy.Spec.HTTP2 != nil {
if h2, err = buildIRHTTP2Settings(policy.Spec.HTTP2); err != nil {
err = perr.WithMessage(err, "HTTP2")
errs = errors.Join(errs, err)

Check warning on line 494 in internal/gatewayapi/backendtrafficpolicy.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/backendtrafficpolicy.go#L493-L494

Added lines #L493 - L494 were not covered by tests
}
}

// Early return if got any errors
if errs != nil {
Expand Down Expand Up @@ -566,6 +582,7 @@
FaultInjection: fi,
TCPKeepalive: ka,
Retry: rt,
HTTP2: h2,
}

// Update the Host field in HealthCheck, now that we have access to the Route Hostname.
Expand Down
6 changes: 1 addition & 5 deletions internal/gatewayapi/clienttrafficpolicy.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,7 @@ import (

const (
// Use an invalid string to represent all sections (listeners) within a Gateway
AllSections = "/"
MinHTTP2InitialStreamWindowSize = 65535 // https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/core/v3/protocol.proto#envoy-v3-api-field-config-core-v3-http2protocoloptions-initial-stream-window-size
MaxHTTP2InitialStreamWindowSize = 2147483647 // https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/core/v3/protocol.proto#envoy-v3-api-field-config-core-v3-http2protocoloptions-initial-stream-window-size
MinHTTP2InitialConnectionWindowSize = MinHTTP2InitialStreamWindowSize
MaxHTTP2InitialConnectionWindowSize = MaxHTTP2InitialStreamWindowSize
AllSections = "/"
)

func hasSectionName(target *gwapiv1a2.LocalPolicyTargetReferenceWithSectionName) bool {
Expand Down
66 changes: 66 additions & 0 deletions internal/gatewayapi/http.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
// Copyright Envoy Gateway Authors
// SPDX-License-Identifier: Apache-2.0
// The full text of the Apache license is available in the LICENSE file at
// the root of the repo.

package gatewayapi

import (
"errors"
"fmt"

"k8s.io/utils/ptr"

egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
"github.com/envoyproxy/gateway/internal/ir"
)

const (
MinHTTP2InitialStreamWindowSize = 65535 // https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/core/v3/protocol.proto#envoy-v3-api-field-config-core-v3-http2protocoloptions-initial-stream-window-size
MaxHTTP2InitialStreamWindowSize = 2147483647 // https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/core/v3/protocol.proto#envoy-v3-api-field-config-core-v3-http2protocoloptions-initial-stream-window-size
MinHTTP2InitialConnectionWindowSize = MinHTTP2InitialStreamWindowSize
MaxHTTP2InitialConnectionWindowSize = MaxHTTP2InitialStreamWindowSize
)

func buildIRHTTP2Settings(http2Settings *egv1a1.HTTP2Settings) (*ir.HTTP2Settings, error) {
var (
http2 = &ir.HTTP2Settings{}
errs error
)

if http2Settings.InitialStreamWindowSize != nil {
initialStreamWindowSize, ok := http2Settings.InitialStreamWindowSize.AsInt64()
switch {
case !ok:
errs = errors.Join(errs, fmt.Errorf("invalid InitialStreamWindowSize value %s", http2Settings.InitialStreamWindowSize.String()))
case initialStreamWindowSize < MinHTTP2InitialStreamWindowSize || initialStreamWindowSize > MaxHTTP2InitialStreamWindowSize:
errs = errors.Join(errs, fmt.Errorf("InitialStreamWindowSize value %s is out of range, must be between %d and %d",
http2Settings.InitialStreamWindowSize.String(),
MinHTTP2InitialStreamWindowSize,
MaxHTTP2InitialStreamWindowSize))

Check warning on line 40 in internal/gatewayapi/http.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/http.go#L34-L40

Added lines #L34 - L40 were not covered by tests
default:
http2.InitialStreamWindowSize = ptr.To(uint32(initialStreamWindowSize))
}
}

if http2Settings.InitialConnectionWindowSize != nil {
initialConnectionWindowSize, ok := http2Settings.InitialConnectionWindowSize.AsInt64()
switch {
case !ok:
errs = errors.Join(errs, fmt.Errorf("invalid InitialConnectionWindowSize value %s", http2Settings.InitialConnectionWindowSize.String()))
case initialConnectionWindowSize < MinHTTP2InitialConnectionWindowSize || initialConnectionWindowSize > MaxHTTP2InitialConnectionWindowSize:
errs = errors.Join(errs, fmt.Errorf("InitialConnectionWindowSize value %s is out of range, must be between %d and %d",
http2Settings.InitialConnectionWindowSize.String(),
MinHTTP2InitialConnectionWindowSize,
MaxHTTP2InitialConnectionWindowSize))

Check warning on line 55 in internal/gatewayapi/http.go

View check run for this annotation

Codecov / codecov/patch

internal/gatewayapi/http.go#L49-L55

Added lines #L49 - L55 were not covered by tests
default:
http2.InitialConnectionWindowSize = ptr.To(uint32(initialConnectionWindowSize))
}
}

http2.MaxConcurrentStreams = http2Settings.MaxConcurrentStreams

http2.TerminateConnectionOnMessagingError = http2Settings.TerminateConnectionOnMessagingError

return http2, errs
}
Loading
Loading