From 2dca063f736b279ea741046d1733a63627f71553 Mon Sep 17 00:00:00 2001 From: Guy Daich Date: Sat, 16 Mar 2024 13:43:26 -0500 Subject: [PATCH 01/13] implement connection limit Signed-off-by: Guy Daich --- internal/gatewayapi/clienttrafficpolicy.go | 32 +++ ...ienttrafficpolicy-connection-limit.in.yaml | 50 +++++ ...enttrafficpolicy-connection-limit.out.yaml | 187 ++++++++++++++++++ internal/ir/xds.go | 14 ++ internal/ir/zz_generated.deepcopy.go | 35 ++++ internal/xds/translator/listener.go | 82 ++++++-- .../in/xds-ir/listener-connection-limit.yaml | 64 ++++++ .../listener-connection-limit.clusters.yaml | 68 +++++++ .../listener-connection-limit.endpoints.yaml | 48 +++++ .../listener-connection-limit.listeners.yaml | 113 +++++++++++ .../listener-connection-limit.routes.yaml | 28 +++ internal/xds/translator/translator.go | 6 +- internal/xds/translator/translator_test.go | 3 + .../latest/user/traffic/connection-limit.md | 109 ++++++++++ test/e2e/base/manifests.yaml | 16 ++ test/e2e/testdata/connection-limit.yaml | 31 +++ test/e2e/tests/connection_limit.go | 91 +++++++++ 17 files changed, 956 insertions(+), 21 deletions(-) create mode 100644 internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.in.yaml create mode 100644 internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.out.yaml create mode 100644 internal/xds/translator/testdata/in/xds-ir/listener-connection-limit.yaml create mode 100644 internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.clusters.yaml create mode 100644 internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.endpoints.yaml create mode 100644 internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.listeners.yaml create mode 100644 internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.routes.yaml create mode 100644 site/content/en/latest/user/traffic/connection-limit.md create mode 100644 test/e2e/testdata/connection-limit.yaml create mode 100644 test/e2e/tests/connection_limit.go diff --git a/internal/gatewayapi/clienttrafficpolicy.go b/internal/gatewayapi/clienttrafficpolicy.go index 41f22821752..e0b8ba49e58 100644 --- a/internal/gatewayapi/clienttrafficpolicy.go +++ b/internal/gatewayapi/clienttrafficpolicy.go @@ -360,6 +360,11 @@ func (t *Translator) translateClientTrafficPolicyForListener(policy *egv1a1.Clie // Translate TCPKeepalive translateListenerTCPKeepalive(policy.Spec.TCPKeepalive, httpIR) + // Translate Connection + if err := translateListenerConnection(policy.Spec.Connection, httpIR); err != nil { + return err + } + // Translate Proxy Protocol translateListenerProxyProtocol(policy.Spec.EnableProxyProtocol, httpIR) @@ -644,3 +649,30 @@ func (t *Translator) translateListenerTLSParameters(policy *egv1a1.ClientTraffic return nil } + +func translateListenerConnection(connection *egv1a1.Connection, httpIR *ir.HTTPListener) error { + // Return early if not set + if connection == nil { + return nil + } + + irConnection := &ir.Connection{} + + if connection.Limit != nil { + if connection.Limit.Value != nil { + irConnection.Limit = ptr.To(uint64(*connection.Limit.Value)) + } + + if connection.Limit.CloseDelay != nil { + d, err := time.ParseDuration(string(*connection.Limit.CloseDelay)) + if err != nil { + return fmt.Errorf("invalid CloseDelay value %s", *connection.Limit.CloseDelay) + } + irConnection.CloseDelay = ptr.To(metav1.Duration{Duration: d}) + } + } + + httpIR.Connection = irConnection + + return nil +} diff --git a/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.in.yaml b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.in.yaml new file mode 100644 index 00000000000..9d7e89645ad --- /dev/null +++ b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.in.yaml @@ -0,0 +1,50 @@ +clientTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: ClientTrafficPolicy + metadata: + namespace: envoy-gateway + name: target-gateway-1 + spec: + tcpKeepalive: {} + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: ClientTrafficPolicy + metadata: + namespace: envoy-gateway + name: target-gateway-1-section-http-1 + spec: + connection: + limit: + value: 3 + closeDelay: 10s + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + sectionName: http-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-connection-limit.out.yaml b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.out.yaml new file mode 100644 index 00000000000..160424647af --- /dev/null +++ b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.out.yaml @@ -0,0 +1,187 @@ +clientTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: ClientTrafficPolicy + metadata: + creationTimestamp: null + name: target-gateway-1-section-http-1 + namespace: envoy-gateway + spec: + connection: + limit: + closeDelay: 10s + value: 3 + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http-1 + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http-1 + conditions: + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: ClientTrafficPolicy + metadata: + creationTimestamp: null + name: target-gateway-1 + namespace: envoy-gateway + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + tcpKeepalive: {} + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: There are existing ClientTrafficPolicies that are overriding these + sections [http-1] + reason: Overridden + status: "True" + type: Overridden + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +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 + connection: + closeDelay: 10s + limit: 3 + hostnames: + - '*' + isHTTP2: false + name: envoy-gateway/gateway-1/http-1 + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: false + name: envoy-gateway/gateway-1/http-2 + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 8080 + tcpKeepalive: {} diff --git a/internal/ir/xds.go b/internal/ir/xds.go index d2194fb95e8..d4eb793391c 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -230,6 +230,8 @@ type HTTPListener struct { HTTP1 *HTTP1Settings `json:"http1,omitempty" yaml:"http1,omitempty"` // ClientTimeout sets the timeout configuration for downstream connections Timeout *ClientTimeout `json:"timeout,omitempty" yaml:"clientTimeout,omitempty"` + // Connection settings + Connection *Connection `json:"connection,omitempty" yaml:"connection,omitempty"` } // Validate the fields within the HTTPListener structure @@ -1078,6 +1080,8 @@ type TCPListener struct { Destination *RouteDestination `json:"destination,omitempty" yaml:"destination,omitempty"` // TCPKeepalive configuration for the listener TCPKeepalive *TCPKeepalive `json:"tcpKeepalive,omitempty" yaml:"tcpKeepalive,omitempty"` + // Connection is ... + Connection *Connection `json:"connection,omitempty" yaml:"connection,omitempty"` } // TLS holds information for configuring TLS on a listener @@ -1751,3 +1755,13 @@ type TLSUpstreamConfig struct { UseSystemTrustStore bool `json:"useSystemTrustStore,omitempty" yaml:"useSystemTrustStore,omitempty"` CACertificate *TLSCACertificate `json:"caCertificate,omitempty" yaml:"caCertificate,omitempty"` } + +// Connection settings for downstream connections +// +k8s:deepcopy-gen=true +type Connection struct { + // Limit for number of connections + Limit *uint64 `json:"limit,omitempty" yaml:"limit,omitempty"` + + // CloseDelay is time to wait before closing a connection rejected due to limit + CloseDelay *metav1.Duration `json:"closeDelay,omitempty" yaml:"closeDelay,omitempty"` +} diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go index ec9a1ca2bad..6cc6cfe140b 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -300,6 +300,31 @@ func (in *ClientTimeout) DeepCopy() *ClientTimeout { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *Connection) DeepCopyInto(out *Connection) { + *out = *in + if in.Limit != nil { + in, out := &in.Limit, &out.Limit + *out = new(uint64) + **out = **in + } + if in.CloseDelay != nil { + in, out := &in.CloseDelay, &out.CloseDelay + *out = new(v1.Duration) + **out = **in + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Connection. +func (in *Connection) DeepCopy() *Connection { + if in == nil { + return nil + } + out := new(Connection) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ConsistentHash) DeepCopyInto(out *ConsistentHash) { *out = *in @@ -761,6 +786,11 @@ func (in *HTTPListener) DeepCopyInto(out *HTTPListener) { *out = new(ClientTimeout) (*in).DeepCopyInto(*out) } + if in.Connection != nil { + in, out := &in.Connection, &out.Connection + *out = new(Connection) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new HTTPListener. @@ -1911,6 +1941,11 @@ func (in *TCPListener) DeepCopyInto(out *TCPListener) { *out = new(TCPKeepalive) (*in).DeepCopyInto(*out) } + if in.Connection != nil { + in, out := &in.Connection, &out.Connection + *out = new(Connection) + (*in).DeepCopyInto(*out) + } } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new TCPListener. diff --git a/internal/xds/translator/listener.go b/internal/xds/translator/listener.go index 10abde44964..c2f3e443505 100644 --- a/internal/xds/translator/listener.go +++ b/internal/xds/translator/listener.go @@ -8,11 +8,14 @@ package translator import ( "errors" + "google.golang.org/protobuf/proto" + xdscore "github.com/cncf/xds/go/xds/core/v3" matcher "github.com/cncf/xds/go/xds/type/matcher/v3" corev3 "github.com/envoyproxy/go-control-plane/envoy/config/core/v3" listenerv3 "github.com/envoyproxy/go-control-plane/envoy/config/listener/v3" tls_inspectorv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/listener/tls_inspector/v3" + connection_limitv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/connection_limit/v3" hcmv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3" tcpv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/tcp_proxy/v3" udpv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/udp/udp_proxy/v3" @@ -43,6 +46,8 @@ const ( http2InitialStreamWindowSize = 65536 // 64 KiB // https://www.envoyproxy.io/docs/envoy/latest/api-v3/config/core/v3/protocol.proto#envoy-v3-api-field-config-core-v3-http2protocoloptions-initial-connection-window-size http2InitialConnectionWindowSize = 1048576 // 1 MiB + // https://www.envoyproxy.io/docs/envoy/latest/api-v3/extensions/filters/network/connection_limit/v3/connection_limit.proto + networkConnectionLimit = "envoy.filters.network.connection_limit" ) func http1ProtocolOptions(opts *ir.HTTP1Settings) *corev3.Http1ProtocolOptions { @@ -183,7 +188,7 @@ func buildXdsQuicListener(name, address string, port uint32, accesslog *ir.Acces } func (t *Translator) addXdsHTTPFilterChain(xdsListener *listenerv3.Listener, irListener *ir.HTTPListener, - accesslog *ir.AccessLog, tracing *ir.Tracing, http3Listener bool) error { + accesslog *ir.AccessLog, tracing *ir.Tracing, http3Listener bool, connection *ir.Connection) error { al := buildXdsAccessLog(accesslog, false) hcmTracing, err := buildHCMTracing(tracing) @@ -260,18 +265,25 @@ func (t *Translator) addXdsHTTPFilterChain(xdsListener *listenerv3.Listener, irL return err } - mgrAny, err := protocov.ToAnyWithError(mgr) - if err != nil { + var filters []*listenerv3.Filter + + if connection != nil && connection.Limit != nil { + cl := buildConnectionLimitFilter(statPrefix, connection) + if clf, err := toNetworkFilter(networkConnectionLimit, cl); err == nil { + filters = append(filters, clf) + } else { + return err + } + } + + if mgrf, err := toNetworkFilter(wellknown.HTTPConnectionManager, mgr); err == nil { + filters = append(filters, mgrf) + } else { return err } filterChain := &listenerv3.FilterChain{ - Filters: []*listenerv3.Filter{{ - Name: wellknown.HTTPConnectionManager, - ConfigType: &listenerv3.Filter_TypedConfig{ - TypedConfig: mgrAny, - }, - }}, + Filters: filters, } if irListener.TLS != nil { @@ -344,7 +356,7 @@ func findXdsHTTPRouteConfigName(xdsListener *listenerv3.Listener) string { return "" } -func addXdsTCPFilterChain(xdsListener *listenerv3.Listener, irListener *ir.TCPListener, clusterName string, accesslog *ir.AccessLog) error { +func addXdsTCPFilterChain(xdsListener *listenerv3.Listener, irListener *ir.TCPListener, clusterName string, accesslog *ir.AccessLog, connection *ir.Connection) error { if irListener == nil { return errors.New("tcp listener is nil") } @@ -367,18 +379,26 @@ func addXdsTCPFilterChain(xdsListener *listenerv3.Listener, irListener *ir.TCPLi Cluster: clusterName, }, } - mgrAny, err := anypb.New(mgr) - if err != nil { + + var filters []*listenerv3.Filter + + if connection != nil && connection.Limit != nil { + cl := buildConnectionLimitFilter(statPrefix, connection) + if clf, err := toNetworkFilter(networkConnectionLimit, cl); err == nil { + filters = append(filters, clf) + } else { + return err + } + } + + if mgrf, err := toNetworkFilter(wellknown.TCPProxy, mgr); err == nil { + filters = append(filters, mgrf) + } else { return err } filterChain := &listenerv3.FilterChain{ - Filters: []*listenerv3.Filter{{ - Name: wellknown.TCPProxy, - ConfigType: &listenerv3.Filter_TypedConfig{ - TypedConfig: mgrAny, - }, - }}, + Filters: filters, } if isTLSPassthrough { @@ -400,6 +420,18 @@ func addXdsTCPFilterChain(xdsListener *listenerv3.Listener, irListener *ir.TCPLi return nil } +func buildConnectionLimitFilter(statPrefix string, connection *ir.Connection) *connection_limitv3.ConnectionLimit { + cl := &connection_limitv3.ConnectionLimit{ + StatPrefix: statPrefix, + MaxConnections: wrapperspb.UInt64(*connection.Limit), + } + + if connection.CloseDelay != nil { + cl.Delay = durationpb.New(connection.CloseDelay.Duration) + } + return cl +} + // addXdsTLSInspectorFilter adds a Tls Inspector filter if it does not yet exist. func addXdsTLSInspectorFilter(xdsListener *listenerv3.Listener) error { // Return early if it exists @@ -672,3 +704,17 @@ func translateEscapePath(in ir.PathEscapedSlashAction) hcmv3.HttpConnectionManag } return hcmv3.HttpConnectionManager_IMPLEMENTATION_SPECIFIC_DEFAULT } + +func toNetworkFilter(filterName string, filterProto proto.Message) (*listenerv3.Filter, error) { + filterAny, err := protocov.ToAnyWithError(filterProto) + if err != nil { + return nil, err + } + + return &listenerv3.Filter{ + Name: filterName, + ConfigType: &listenerv3.Filter_TypedConfig{ + TypedConfig: filterAny, + }, + }, nil +} diff --git a/internal/xds/translator/testdata/in/xds-ir/listener-connection-limit.yaml b/internal/xds/translator/testdata/in/xds-ir/listener-connection-limit.yaml new file mode 100644 index 00000000000..545f31feed8 --- /dev/null +++ b/internal/xds/translator/testdata/in/xds-ir/listener-connection-limit.yaml @@ -0,0 +1,64 @@ +http: +- name: "first-listener" + address: "0.0.0.0" + port: 10080 + hostnames: + - "foo.com" + connection: {} + path: + mergeSlashes: true + escapedSlashesAction: UnescapeAndRedirect + routes: + - name: "first-route" + hostname: "*" + destination: + name: "first-route-dest" + settings: + - endpoints: + - host: "1.2.3.4" + port: 50000 +- name: "second-listener" + address: "0.0.0.0" + port: 10081 + hostnames: + - "foo.net" + connection: + limit: 5 + path: + mergeSlashes: true + escapedSlashesAction: UnescapeAndRedirect + routes: + - name: "second-route" + hostname: "*" + destination: + name: "second-route-dest" + settings: + - endpoints: + - host: "1.2.3.4" + port: 50000 +tcp: +- name: "third-listener" + address: "0.0.0.0" + port: 10082 + connection: {} + tls: + passthrough: + snis: + - bar.com + destination: + name: "tls-route-dest" + settings: + - endpoints: + - host: "1.2.3.4" + port: 50000 +- name: "fourth-listener" + address: "0.0.0.0" + connection: + limit: 10 + port: 10083 + destination: + name: "tcp-route-dest" + settings: + - endpoints: + - host: "1.2.3.4" + port: 50000 diff --git a/internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.clusters.yaml b/internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.clusters.yaml new file mode 100644 index 00000000000..a52251e32bf --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.clusters.yaml @@ -0,0 +1,68 @@ +- circuitBreakers: + thresholds: + - maxRetries: 1024 + commonLbConfig: + localityWeightedLbConfig: {} + connectTimeout: 10s + dnsLookupFamily: V4_ONLY + edsClusterConfig: + edsConfig: + ads: {} + resourceApiVersion: V3 + serviceName: first-route-dest + lbPolicy: LEAST_REQUEST + name: first-route-dest + outlierDetection: {} + perConnectionBufferLimitBytes: 32768 + type: EDS +- circuitBreakers: + thresholds: + - maxRetries: 1024 + commonLbConfig: + localityWeightedLbConfig: {} + connectTimeout: 10s + dnsLookupFamily: V4_ONLY + edsClusterConfig: + edsConfig: + ads: {} + resourceApiVersion: V3 + serviceName: second-route-dest + lbPolicy: LEAST_REQUEST + name: second-route-dest + outlierDetection: {} + perConnectionBufferLimitBytes: 32768 + type: EDS +- circuitBreakers: + thresholds: + - maxRetries: 1024 + commonLbConfig: + localityWeightedLbConfig: {} + connectTimeout: 10s + dnsLookupFamily: V4_ONLY + edsClusterConfig: + edsConfig: + ads: {} + resourceApiVersion: V3 + serviceName: tls-route-dest + lbPolicy: LEAST_REQUEST + name: tls-route-dest + outlierDetection: {} + perConnectionBufferLimitBytes: 32768 + type: EDS +- circuitBreakers: + thresholds: + - maxRetries: 1024 + commonLbConfig: + localityWeightedLbConfig: {} + connectTimeout: 10s + dnsLookupFamily: V4_ONLY + edsClusterConfig: + edsConfig: + ads: {} + resourceApiVersion: V3 + serviceName: tcp-route-dest + lbPolicy: LEAST_REQUEST + name: tcp-route-dest + outlierDetection: {} + perConnectionBufferLimitBytes: 32768 + type: EDS diff --git a/internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.endpoints.yaml b/internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.endpoints.yaml new file mode 100644 index 00000000000..5b4fe89e58c --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.endpoints.yaml @@ -0,0 +1,48 @@ +- clusterName: first-route-dest + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 1.2.3.4 + portValue: 50000 + loadBalancingWeight: 1 + loadBalancingWeight: 1 + locality: + region: first-route-dest/backend/0 +- clusterName: second-route-dest + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 1.2.3.4 + portValue: 50000 + loadBalancingWeight: 1 + loadBalancingWeight: 1 + locality: + region: second-route-dest/backend/0 +- clusterName: tls-route-dest + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 1.2.3.4 + portValue: 50000 + loadBalancingWeight: 1 + loadBalancingWeight: 1 + locality: + region: tls-route-dest/backend/0 +- clusterName: tcp-route-dest + endpoints: + - lbEndpoints: + - endpoint: + address: + socketAddress: + address: 1.2.3.4 + portValue: 50000 + loadBalancingWeight: 1 + loadBalancingWeight: 1 + locality: + region: tcp-route-dest/backend/0 diff --git a/internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.listeners.yaml new file mode 100644 index 00000000000..d7500ba9890 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.listeners.yaml @@ -0,0 +1,113 @@ +- address: + socketAddress: + address: 0.0.0.0 + portValue: 10080 + defaultFilterChain: + filters: + - name: envoy.filters.network.http_connection_manager + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager + commonHttpProtocolOptions: + headersWithUnderscoresAction: REJECT_REQUEST + http2ProtocolOptions: + initialConnectionWindowSize: 1048576 + initialStreamWindowSize: 65536 + maxConcurrentStreams: 100 + httpFilters: + - name: envoy.filters.http.router + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.http.router.v3.Router + suppressEnvoyHeaders: true + mergeSlashes: true + normalizePath: true + pathWithEscapedSlashesAction: UNESCAPE_AND_REDIRECT + rds: + configSource: + ads: {} + resourceApiVersion: V3 + routeConfigName: first-listener + serverHeaderTransformation: PASS_THROUGH + statPrefix: http + useRemoteAddress: true + drainType: MODIFY_ONLY + name: first-listener + perConnectionBufferLimitBytes: 32768 +- address: + socketAddress: + address: 0.0.0.0 + portValue: 10081 + defaultFilterChain: + filters: + - name: envoy.filters.network.connection_limit + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.network.connection_limit.v3.ConnectionLimit + maxConnections: "5" + statPrefix: http + - name: envoy.filters.network.http_connection_manager + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.network.http_connection_manager.v3.HttpConnectionManager + commonHttpProtocolOptions: + headersWithUnderscoresAction: REJECT_REQUEST + http2ProtocolOptions: + initialConnectionWindowSize: 1048576 + initialStreamWindowSize: 65536 + maxConcurrentStreams: 100 + httpFilters: + - name: envoy.filters.http.router + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.http.router.v3.Router + suppressEnvoyHeaders: true + mergeSlashes: true + normalizePath: true + pathWithEscapedSlashesAction: UNESCAPE_AND_REDIRECT + rds: + configSource: + ads: {} + resourceApiVersion: V3 + routeConfigName: second-listener + serverHeaderTransformation: PASS_THROUGH + statPrefix: http + useRemoteAddress: true + drainType: MODIFY_ONLY + name: second-listener + perConnectionBufferLimitBytes: 32768 +- address: + socketAddress: + address: 0.0.0.0 + portValue: 10082 + drainType: MODIFY_ONLY + filterChains: + - filterChainMatch: + serverNames: + - bar.com + filters: + - name: envoy.filters.network.tcp_proxy + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy + cluster: tls-route-dest + statPrefix: passthrough + listenerFilters: + - name: envoy.filters.listener.tls_inspector + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.listener.tls_inspector.v3.TlsInspector + name: third-listener + perConnectionBufferLimitBytes: 32768 +- address: + socketAddress: + address: 0.0.0.0 + portValue: 10083 + drainType: MODIFY_ONLY + filterChains: + - filters: + - name: envoy.filters.network.connection_limit + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.network.connection_limit.v3.ConnectionLimit + maxConnections: "10" + statPrefix: tcp + - name: envoy.filters.network.tcp_proxy + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy + cluster: tcp-route-dest + statPrefix: tcp + name: fourth-listener + perConnectionBufferLimitBytes: 32768 diff --git a/internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.routes.yaml b/internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.routes.yaml new file mode 100644 index 00000000000..ff93cfff360 --- /dev/null +++ b/internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.routes.yaml @@ -0,0 +1,28 @@ +- ignorePortInHostMatching: true + name: first-listener + virtualHosts: + - domains: + - '*' + name: first-listener/* + routes: + - match: + prefix: / + name: first-route + route: + cluster: first-route-dest + upgradeConfigs: + - upgradeType: websocket +- ignorePortInHostMatching: true + name: second-listener + virtualHosts: + - domains: + - '*' + name: second-listener/* + routes: + - match: + prefix: / + name: second-route + route: + cluster: second-route-dest + upgradeConfigs: + - upgradeType: websocket diff --git a/internal/xds/translator/translator.go b/internal/xds/translator/translator.go index bd35d41891a..dbdea4471b7 100644 --- a/internal/xds/translator/translator.go +++ b/internal/xds/translator/translator.go @@ -163,11 +163,11 @@ func (t *Translator) processHTTPListenerXdsTranslation( } if addFilterChain { - if err := t.addXdsHTTPFilterChain(xdsListener, httpListener, accessLog, tracing, false); err != nil { + if err := t.addXdsHTTPFilterChain(xdsListener, httpListener, accessLog, tracing, false, httpListener.Connection); err != nil { return err } if enabledHTTP3 { - if err := t.addXdsHTTPFilterChain(quicXDSListener, httpListener, accessLog, tracing, true); err != nil { + if err := t.addXdsHTTPFilterChain(quicXDSListener, httpListener, accessLog, tracing, true, httpListener.Connection); err != nil { return err } } @@ -346,7 +346,7 @@ func processTCPListenerXdsTranslation(tCtx *types.ResourceVersionTable, tcpListe } } - if err := addXdsTCPFilterChain(xdsListener, tcpListener, tcpListener.Destination.Name, accesslog); err != nil { + if err := addXdsTCPFilterChain(xdsListener, tcpListener, tcpListener.Destination.Name, accesslog, tcpListener.Connection); err != nil { errs = errors.Join(errs, err) } diff --git a/internal/xds/translator/translator_test.go b/internal/xds/translator/translator_test.go index e0737a0df6f..ca9917d2cc1 100644 --- a/internal/xds/translator/translator_test.go +++ b/internal/xds/translator/translator_test.go @@ -293,6 +293,9 @@ func TestTranslateXds(t *testing.T) { { name: "retry-partial-invalid", }, + { + name: "listener-connection-limit", + }, } for _, tc := range testCases { diff --git a/site/content/en/latest/user/traffic/connection-limit.md b/site/content/en/latest/user/traffic/connection-limit.md new file mode 100644 index 00000000000..30e9f693b43 --- /dev/null +++ b/site/content/en/latest/user/traffic/connection-limit.md @@ -0,0 +1,109 @@ +--- +title: "Connection Limit" +--- + +The connection limit features allows users to limit the number of concurrently active TCP connections on a [Gateway][] or a [Listener][]. +When the [connection limit][] is reached, new connections are closed immediately by Envoy proxy. It's possible to configure a delay for connection rejection. + +Users may want to limit the number of connections for several reasons: +* Protect resources like CPU and Memory. +* Ensure that different listeners can receive a fair share of global resources. +* Protect from malicious activity like DoS attacks. + +Envoy Gateway introduces a new CRD called [ClientTrafficPolicy][] that allows the user to describe their desired connection limit settings. +This instantiated resource can be linked to a [Gateway][]. + +The Envoy [connection limit][] implementation is distributed: counters are not synchronized between different envoy proxies. + +When a [Client Traffic Policy][] is attached to a gateway, the connection limit will apply differently based on the +[Listener][] protocol in use: +- HTTP: all HTTP listeners in a [Gateway][] will share a common connection counter, and a limit defined by the policy. +- HTTPS/TLS: each HTTPS/TLS listener will have a dedicated connection counter, and a limit defined by the policy. + + +## Prerequisites + +### Install Envoy Gateway + +* Follow the steps from the [Quickstart Guide](../../quickstart) to install Envoy Gateway and the HTTPRoute example manifest. + Before proceeding, you should be able to query the example backend using HTTP. + +### Install the hey load testing tool +* The `hey` CLI will be used to generate load and measure response times. Follow the installation instruction from the [Hey project] docs. + +## Test and customize connection limit settings + +This example we use `hey` to open 10 connections and execute 1 RPS per connection for 10 seconds. + +```shell +hey -c 10 -q 1 -z 10s -host "www.example.com" http://${GATEWAY_HOST}/get +``` + +```console +Summary: + Total: 10.0058 secs + Slowest: 0.0275 secs + Fastest: 0.0029 secs + Average: 0.0111 secs + Requests/sec: 9.9942 + +[...] + +Status code distribution: + [200] 100 responses +``` + +There are no connection limits, and so all 100 requests succeed. + +Next, we apply a limit of 5 connections. + +```shell +cat < Date: Mon, 18 Mar 2024 06:09:40 -0500 Subject: [PATCH 02/13] fix lint Signed-off-by: Guy Daich --- .../en/latest/user/traffic/connection-limit.md | 2 +- test/e2e/tests/connection_limit.go | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/site/content/en/latest/user/traffic/connection-limit.md b/site/content/en/latest/user/traffic/connection-limit.md index 30e9f693b43..85c2f26de98 100644 --- a/site/content/en/latest/user/traffic/connection-limit.md +++ b/site/content/en/latest/user/traffic/connection-limit.md @@ -10,7 +10,7 @@ Users may want to limit the number of connections for several reasons: * Ensure that different listeners can receive a fair share of global resources. * Protect from malicious activity like DoS attacks. -Envoy Gateway introduces a new CRD called [ClientTrafficPolicy][] that allows the user to describe their desired connection limit settings. +Envoy Gateway introduces a new CRD called [Client Traffic Policy][] that allows the user to describe their desired connection limit settings. This instantiated resource can be linked to a [Gateway][]. The Envoy [connection limit][] implementation is distributed: counters are not synchronized between different envoy proxies. diff --git a/test/e2e/tests/connection_limit.go b/test/e2e/tests/connection_limit.go index d645fa1802a..9b47a9dc2bd 100644 --- a/test/e2e/tests/connection_limit.go +++ b/test/e2e/tests/connection_limit.go @@ -74,16 +74,16 @@ var ConnectionLimitTest = suite.ConformanceTest{ // expect error if err != nil { - ue := &url.Error{} - if errors.As(err, &ue) { - if ue.Err.Error() != "EOF" { - t.Errorf("Exepcted EOF when connection limit is reached") + urlError := &url.Error{} + if errors.As(err, &urlError) { + if urlError.Err.Error() != "EOF" { + t.Errorf("expected EOF when connection limit is reached") } } else { - t.Errorf("Exepcted net/url error when connection limit is reached") + t.Errorf("expected net/url error when connection limit is reached") } } else { - t.Errorf("Exepcted error when connection limit is reached") + t.Errorf("expected error when connection limit is reached") } }) From ffae6889804ed42c8f4a89c3f4beffae1849e947 Mon Sep 17 00:00:00 2001 From: Guy Daich Date: Mon, 18 Mar 2024 06:21:39 -0500 Subject: [PATCH 03/13] fix lint 2 Signed-off-by: Guy Daich --- site/content/en/latest/user/traffic/connection-limit.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/site/content/en/latest/user/traffic/connection-limit.md b/site/content/en/latest/user/traffic/connection-limit.md index 85c2f26de98..22661b2378c 100644 --- a/site/content/en/latest/user/traffic/connection-limit.md +++ b/site/content/en/latest/user/traffic/connection-limit.md @@ -102,7 +102,7 @@ Error distribution: With the new connection limit, only 5 of 10 connections are established, and so only 50 requests succeed. -[ClientTrafficPolicy]: ../../../api/extension_types#clienttrafficpolicy +[Client Traffic Policy]: ../../../api/extension_types#clienttrafficpolicy [Hey project]: https://github.com/rakyll/hey [connection limit]: https://www.envoyproxy.io/docs/envoy/latest/configuration/listeners/network_filters/connection_limit_filter [listener]: https://gateway-api.sigs.k8s.io/reference/spec/#gateway.networking.k8s.io/v1.Listener From 5191c1825e2790221111416a95ea70823caeb633 Mon Sep 17 00:00:00 2001 From: Guy Daich Date: Mon, 18 Mar 2024 07:15:17 -0500 Subject: [PATCH 04/13] fix ir, coverage Signed-off-by: Guy Daich --- internal/gatewayapi/clienttrafficpolicy.go | 13 +- ...afficpolicy-connection-limit-error.in.yaml | 50 +++++ ...fficpolicy-connection-limit-error.out.yaml | 184 ++++++++++++++++++ ...ienttrafficpolicy-connection-limit.in.yaml | 2 +- ...enttrafficpolicy-connection-limit.out.yaml | 9 +- internal/ir/xds.go | 22 ++- internal/ir/zz_generated.deepcopy.go | 26 ++- internal/xds/translator/listener.go | 6 +- .../in/xds-ir/listener-connection-limit.yaml | 11 +- .../listener-connection-limit.listeners.yaml | 6 + test/e2e/tests/connection_limit.go | 3 + 11 files changed, 309 insertions(+), 23 deletions(-) create mode 100644 internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit-error.in.yaml create mode 100755 internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit-error.out.yaml diff --git a/internal/gatewayapi/clienttrafficpolicy.go b/internal/gatewayapi/clienttrafficpolicy.go index e0b8ba49e58..031fc8f7ba6 100644 --- a/internal/gatewayapi/clienttrafficpolicy.go +++ b/internal/gatewayapi/clienttrafficpolicy.go @@ -659,8 +659,12 @@ func translateListenerConnection(connection *egv1a1.Connection, httpIR *ir.HTTPL irConnection := &ir.Connection{} if connection.Limit != nil { + hasLimit := false + irConnectionLimit := &ir.ConnectionLimit{} + if connection.Limit.Value != nil { - irConnection.Limit = ptr.To(uint64(*connection.Limit.Value)) + irConnectionLimit.Value = ptr.To(uint64(*connection.Limit.Value)) + hasLimit = true } if connection.Limit.CloseDelay != nil { @@ -668,7 +672,12 @@ func translateListenerConnection(connection *egv1a1.Connection, httpIR *ir.HTTPL if err != nil { return fmt.Errorf("invalid CloseDelay value %s", *connection.Limit.CloseDelay) } - irConnection.CloseDelay = ptr.To(metav1.Duration{Duration: d}) + irConnectionLimit.CloseDelay = ptr.To(metav1.Duration{Duration: d}) + hasLimit = true + } + + if hasLimit { + irConnection.Limit = irConnectionLimit } } diff --git a/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit-error.in.yaml b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit-error.in.yaml new file mode 100644 index 00000000000..b809f3f120d --- /dev/null +++ b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit-error.in.yaml @@ -0,0 +1,50 @@ +clientTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: ClientTrafficPolicy + metadata: + namespace: envoy-gateway + name: target-gateway-1 + spec: + tcpKeepalive: {} + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: ClientTrafficPolicy + metadata: + namespace: envoy-gateway + name: target-gateway-1-section-http-1 + spec: + connection: + limit: + value: 3 + closeDelay: 10mib + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + sectionName: http-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-connection-limit-error.out.yaml b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit-error.out.yaml new file mode 100755 index 00000000000..0438366d985 --- /dev/null +++ b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit-error.out.yaml @@ -0,0 +1,184 @@ +clientTrafficPolicies: +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: ClientTrafficPolicy + metadata: + creationTimestamp: null + name: target-gateway-1-section-http-1 + namespace: envoy-gateway + spec: + connection: + limit: + closeDelay: 10mib + value: 3 + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http-1 + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + sectionName: http-1 + conditions: + - lastTransitionTime: null + message: Invalid CloseDelay value 10mib + reason: Invalid + status: "False" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +- apiVersion: gateway.envoyproxy.io/v1alpha1 + kind: ClientTrafficPolicy + metadata: + creationTimestamp: null + name: target-gateway-1 + namespace: envoy-gateway + spec: + targetRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + tcpKeepalive: {} + status: + ancestors: + - ancestorRef: + group: gateway.networking.k8s.io + kind: Gateway + name: gateway-1 + namespace: envoy-gateway + conditions: + - lastTransitionTime: null + message: There are existing ClientTrafficPolicies that are overriding these + sections [http-1] + reason: Overridden + status: "True" + type: Overridden + - lastTransitionTime: null + message: Policy has been accepted. + reason: Accepted + status: "True" + type: Accepted + controllerName: gateway.envoyproxy.io/gatewayclass-controller +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: + - '*' + isHTTP2: false + name: envoy-gateway/gateway-1/http-1 + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 10080 + - address: 0.0.0.0 + hostnames: + - '*' + isHTTP2: false + name: envoy-gateway/gateway-1/http-2 + path: + escapedSlashesAction: UnescapeAndRedirect + mergeSlashes: true + port: 8080 + tcpKeepalive: {} diff --git a/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.in.yaml b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.in.yaml index 9d7e89645ad..54065120d5f 100644 --- a/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.in.yaml +++ b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.in.yaml @@ -5,7 +5,7 @@ clientTrafficPolicies: namespace: envoy-gateway name: target-gateway-1 spec: - tcpKeepalive: {} + connection: {} targetRef: group: gateway.networking.k8s.io kind: Gateway diff --git a/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.out.yaml b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.out.yaml index 160424647af..dc6bcf27f5f 100644 --- a/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.out.yaml +++ b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.out.yaml @@ -38,12 +38,12 @@ clientTrafficPolicies: name: target-gateway-1 namespace: envoy-gateway spec: + connection: {} targetRef: group: gateway.networking.k8s.io kind: Gateway name: gateway-1 namespace: envoy-gateway - tcpKeepalive: {} status: ancestors: - ancestorRef: @@ -165,8 +165,9 @@ xdsIR: http: - address: 0.0.0.0 connection: - closeDelay: 10s - limit: 3 + limit: + closeDelay: 10s + value: 3 hostnames: - '*' isHTTP2: false @@ -176,6 +177,7 @@ xdsIR: mergeSlashes: true port: 10080 - address: 0.0.0.0 + connection: {} hostnames: - '*' isHTTP2: false @@ -184,4 +186,3 @@ xdsIR: escapedSlashesAction: UnescapeAndRedirect mergeSlashes: true port: 8080 - tcpKeepalive: {} diff --git a/internal/ir/xds.go b/internal/ir/xds.go index f67c98beb2c..77ab37c0027 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -8,16 +8,15 @@ package ir import ( "cmp" "errors" - "net/http" - "net/netip" - "reflect" - "golang.org/x/exp/slices" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" + "net/http" + "net/netip" + "reflect" gwv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2" "sigs.k8s.io/yaml" @@ -1088,7 +1087,7 @@ type TCPListener struct { Destination *RouteDestination `json:"destination,omitempty" yaml:"destination,omitempty"` // TCPKeepalive configuration for the listener TCPKeepalive *TCPKeepalive `json:"tcpKeepalive,omitempty" yaml:"tcpKeepalive,omitempty"` - // Connection is ... + // Connection settings for clients Connection *Connection `json:"connection,omitempty" yaml:"connection,omitempty"` } @@ -1768,8 +1767,17 @@ type TLSUpstreamConfig struct { // +k8s:deepcopy-gen=true type Connection struct { // Limit for number of connections - Limit *uint64 `json:"limit,omitempty" yaml:"limit,omitempty"` + Limit *ConnectionLimit `json:"limit,omitempty" yaml:"limit,omitempty"` +} + +// ConnectionLimit contains settings for downstream connection limits +// +k8s:deepcopy-gen=true +type ConnectionLimit struct { + // Value of the maximum concurrent connections limit. + // When the limit is reached, incoming connections will be closed after the CloseDelay duration. + Value *uint64 `json:"value,omitempty" yaml:"value,omitempty"` - // CloseDelay is time to wait before closing a connection rejected due to limit + // CloseDelay defines the delay to use before closing connections that are rejected + // once the limit value is reached. CloseDelay *metav1.Duration `json:"closeDelay,omitempty" yaml:"closeDelay,omitempty"` } diff --git a/internal/ir/zz_generated.deepcopy.go b/internal/ir/zz_generated.deepcopy.go index 05643359897..20bca7455a6 100644 --- a/internal/ir/zz_generated.deepcopy.go +++ b/internal/ir/zz_generated.deepcopy.go @@ -305,6 +305,26 @@ func (in *Connection) DeepCopyInto(out *Connection) { *out = *in if in.Limit != nil { in, out := &in.Limit, &out.Limit + *out = new(ConnectionLimit) + (*in).DeepCopyInto(*out) + } +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Connection. +func (in *Connection) DeepCopy() *Connection { + if in == nil { + return nil + } + out := new(Connection) + in.DeepCopyInto(out) + return out +} + +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *ConnectionLimit) DeepCopyInto(out *ConnectionLimit) { + *out = *in + if in.Value != nil { + in, out := &in.Value, &out.Value *out = new(uint64) **out = **in } @@ -315,12 +335,12 @@ func (in *Connection) DeepCopyInto(out *Connection) { } } -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Connection. -func (in *Connection) DeepCopy() *Connection { +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new ConnectionLimit. +func (in *ConnectionLimit) DeepCopy() *ConnectionLimit { if in == nil { return nil } - out := new(Connection) + out := new(ConnectionLimit) in.DeepCopyInto(out) return out } diff --git a/internal/xds/translator/listener.go b/internal/xds/translator/listener.go index c2f3e443505..85bdeb18060 100644 --- a/internal/xds/translator/listener.go +++ b/internal/xds/translator/listener.go @@ -423,11 +423,11 @@ func addXdsTCPFilterChain(xdsListener *listenerv3.Listener, irListener *ir.TCPLi func buildConnectionLimitFilter(statPrefix string, connection *ir.Connection) *connection_limitv3.ConnectionLimit { cl := &connection_limitv3.ConnectionLimit{ StatPrefix: statPrefix, - MaxConnections: wrapperspb.UInt64(*connection.Limit), + MaxConnections: wrapperspb.UInt64(*connection.Limit.Value), } - if connection.CloseDelay != nil { - cl.Delay = durationpb.New(connection.CloseDelay.Duration) + if connection.Limit.CloseDelay != nil { + cl.Delay = durationpb.New(connection.Limit.CloseDelay.Duration) } return cl } diff --git a/internal/xds/translator/testdata/in/xds-ir/listener-connection-limit.yaml b/internal/xds/translator/testdata/in/xds-ir/listener-connection-limit.yaml index 545f31feed8..049ec905b9a 100644 --- a/internal/xds/translator/testdata/in/xds-ir/listener-connection-limit.yaml +++ b/internal/xds/translator/testdata/in/xds-ir/listener-connection-limit.yaml @@ -23,7 +23,8 @@ http: hostnames: - "foo.net" connection: - limit: 5 + limit: + value: 5 path: mergeSlashes: true escapedSlashesAction: UnescapeAndRedirect @@ -40,7 +41,9 @@ tcp: - name: "third-listener" address: "0.0.0.0" port: 10082 - connection: {} + connection: + limit: + value: 3 tls: passthrough: snis: @@ -54,7 +57,9 @@ tcp: - name: "fourth-listener" address: "0.0.0.0" connection: - limit: 10 + limit: + value: 10 + closeDelay: 3s port: 10083 destination: name: "tcp-route-dest" diff --git a/internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.listeners.yaml b/internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.listeners.yaml index d7500ba9890..a7500a6bb76 100644 --- a/internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.listeners.yaml +++ b/internal/xds/translator/testdata/out/xds-ir/listener-connection-limit.listeners.yaml @@ -81,6 +81,11 @@ serverNames: - bar.com filters: + - name: envoy.filters.network.connection_limit + typedConfig: + '@type': type.googleapis.com/envoy.extensions.filters.network.connection_limit.v3.ConnectionLimit + maxConnections: "3" + statPrefix: passthrough - name: envoy.filters.network.tcp_proxy typedConfig: '@type': type.googleapis.com/envoy.extensions.filters.network.tcp_proxy.v3.TcpProxy @@ -102,6 +107,7 @@ - name: envoy.filters.network.connection_limit typedConfig: '@type': type.googleapis.com/envoy.extensions.filters.network.connection_limit.v3.ConnectionLimit + delay: 3s maxConnections: "10" statPrefix: tcp - name: envoy.filters.network.tcp_proxy diff --git a/test/e2e/tests/connection_limit.go b/test/e2e/tests/connection_limit.go index 9b47a9dc2bd..673473eaa45 100644 --- a/test/e2e/tests/connection_limit.go +++ b/test/e2e/tests/connection_limit.go @@ -68,6 +68,9 @@ var ConnectionLimitTest = suite.ConformanceTest{ } } + // sleep a bit for envoy counters to update + time.Sleep(300 * time.Millisecond) + // new requests now fail req = http.MakeRequest(t, &expectedResponse, gwAddr, "HTTP", "http") _, _, err = suite.RoundTripper.CaptureRoundTrip(req) From a1e6bec9758785d242bbec898b8c9eac4df6fa09 Mon Sep 17 00:00:00 2001 From: Guy Daich Date: Mon, 18 Mar 2024 07:48:48 -0500 Subject: [PATCH 05/13] fix lint 3 Signed-off-by: Guy Daich --- internal/ir/xds.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/internal/ir/xds.go b/internal/ir/xds.go index 77ab37c0027..017d5555102 100644 --- a/internal/ir/xds.go +++ b/internal/ir/xds.go @@ -8,15 +8,16 @@ package ir import ( "cmp" "errors" + "net/http" + "net/netip" + "reflect" + "golang.org/x/exp/slices" apiextensionsv1 "k8s.io/apiextensions-apiserver/pkg/apis/apiextensions/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/util/sets" "k8s.io/apimachinery/pkg/util/validation" - "net/http" - "net/netip" - "reflect" gwv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2" "sigs.k8s.io/yaml" From f40b55e5fd6de46ae60f40041943bb1c6d48a4b0 Mon Sep 17 00:00:00 2001 From: Guy Daich Date: Mon, 18 Mar 2024 09:37:42 -0500 Subject: [PATCH 06/13] open more connection in e2e Signed-off-by: Guy Daich --- test/e2e/tests/connection_limit.go | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/test/e2e/tests/connection_limit.go b/test/e2e/tests/connection_limit.go index 673473eaa45..ee96fd0203f 100644 --- a/test/e2e/tests/connection_limit.go +++ b/test/e2e/tests/connection_limit.go @@ -31,7 +31,7 @@ var ConnectionLimitTest = suite.ConformanceTest{ Description: "Deny All Requests", Manifests: []string{"testdata/connection-limit.yaml"}, Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { - t.Run("Deny All Requests", func(t *testing.T) { + t.Run("Close connections over limit", func(t *testing.T) { ns := "gateway-conformance-infra" routeNN := types.NamespacedName{Name: "http-with-connection-limit", Namespace: ns} gwNN := types.NamespacedName{Name: "connection-limit-gateway", Namespace: ns} @@ -58,8 +58,8 @@ var ConnectionLimitTest = suite.ConformanceTest{ t.Errorf("failed to compare request and response: %v", err) } - // open max allowed connections - for i := 0; i < 5; i++ { + // open some connections + for i := 0; i < 10; i++ { conn, err := net.DialTimeout("tcp", gwAddr, 100*time.Millisecond) if err == nil { defer conn.Close() @@ -68,9 +68,6 @@ var ConnectionLimitTest = suite.ConformanceTest{ } } - // sleep a bit for envoy counters to update - time.Sleep(300 * time.Millisecond) - // new requests now fail req = http.MakeRequest(t, &expectedResponse, gwAddr, "HTTP", "http") _, _, err = suite.RoundTripper.CaptureRoundTrip(req) From 965de99f385c29617b624e41e5a2b0a9feef69aa Mon Sep 17 00:00:00 2001 From: Guy Daich Date: Mon, 18 Mar 2024 13:15:49 -0500 Subject: [PATCH 07/13] fix error type Signed-off-by: Guy Daich --- test/e2e/tests/connection_limit.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/test/e2e/tests/connection_limit.go b/test/e2e/tests/connection_limit.go index ee96fd0203f..49f46f9bec1 100644 --- a/test/e2e/tests/connection_limit.go +++ b/test/e2e/tests/connection_limit.go @@ -59,7 +59,7 @@ var ConnectionLimitTest = suite.ConformanceTest{ } // open some connections - for i := 0; i < 10; i++ { + for i := 0; i < 5; i++ { conn, err := net.DialTimeout("tcp", gwAddr, 100*time.Millisecond) if err == nil { defer conn.Close() @@ -75,11 +75,7 @@ var ConnectionLimitTest = suite.ConformanceTest{ // expect error if err != nil { urlError := &url.Error{} - if errors.As(err, &urlError) { - if urlError.Err.Error() != "EOF" { - t.Errorf("expected EOF when connection limit is reached") - } - } else { + if !errors.As(err, &urlError) { t.Errorf("expected net/url error when connection limit is reached") } } else { From 1862accafd342176462743301356d2cd6c27447a Mon Sep 17 00:00:00 2001 From: Guy Daich Date: Mon, 18 Mar 2024 14:12:30 -0500 Subject: [PATCH 08/13] add additional connections Signed-off-by: Guy Daich --- test/e2e/tests/connection_limit.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/tests/connection_limit.go b/test/e2e/tests/connection_limit.go index 49f46f9bec1..37732eaa8be 100644 --- a/test/e2e/tests/connection_limit.go +++ b/test/e2e/tests/connection_limit.go @@ -59,7 +59,7 @@ var ConnectionLimitTest = suite.ConformanceTest{ } // open some connections - for i := 0; i < 5; i++ { + for i := 0; i < 10; i++ { conn, err := net.DialTimeout("tcp", gwAddr, 100*time.Millisecond) if err == nil { defer conn.Close() From 24dd151e8ae4b2622aae17030e90c91b6bebf5af Mon Sep 17 00:00:00 2001 From: Guy Daich Date: Tue, 19 Mar 2024 05:49:54 -0500 Subject: [PATCH 09/13] make limit value required Signed-off-by: Guy Daich --- api/v1alpha1/connection_types.go | 3 +-- api/v1alpha1/zz_generated.deepcopy.go | 5 ----- internal/gatewayapi/clienttrafficpolicy.go | 11 ++--------- site/content/en/latest/api/extension_types.md | 2 +- test/e2e/tests/connection_limit.go | 2 +- 5 files changed, 5 insertions(+), 18 deletions(-) diff --git a/api/v1alpha1/connection_types.go b/api/v1alpha1/connection_types.go index 13132ddd8e1..fff116ab2f7 100644 --- a/api/v1alpha1/connection_types.go +++ b/api/v1alpha1/connection_types.go @@ -20,9 +20,8 @@ type ConnectionLimit struct { // When the limit is reached, incoming connections will be closed after the CloseDelay duration. // Default: unlimited. // - // +optional // +kubebuilder:validation:Minimum=0 - Value *int64 `json:"value,omitempty"` + Value int64 `json:"value,omitempty"` // CloseDelay defines the delay to use before closing connections that are rejected // once the limit value is reached. diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index b474db20c74..8967e2bcba0 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -596,11 +596,6 @@ func (in *Connection) DeepCopy() *Connection { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *ConnectionLimit) DeepCopyInto(out *ConnectionLimit) { *out = *in - if in.Value != nil { - in, out := &in.Value, &out.Value - *out = new(int64) - **out = **in - } if in.CloseDelay != nil { in, out := &in.CloseDelay, &out.CloseDelay *out = new(v1.Duration) diff --git a/internal/gatewayapi/clienttrafficpolicy.go b/internal/gatewayapi/clienttrafficpolicy.go index 031fc8f7ba6..bfa32443d31 100644 --- a/internal/gatewayapi/clienttrafficpolicy.go +++ b/internal/gatewayapi/clienttrafficpolicy.go @@ -659,13 +659,9 @@ func translateListenerConnection(connection *egv1a1.Connection, httpIR *ir.HTTPL irConnection := &ir.Connection{} if connection.Limit != nil { - hasLimit := false irConnectionLimit := &ir.ConnectionLimit{} - if connection.Limit.Value != nil { - irConnectionLimit.Value = ptr.To(uint64(*connection.Limit.Value)) - hasLimit = true - } + irConnectionLimit.Value = ptr.To(uint64(connection.Limit.Value)) if connection.Limit.CloseDelay != nil { d, err := time.ParseDuration(string(*connection.Limit.CloseDelay)) @@ -673,12 +669,9 @@ func translateListenerConnection(connection *egv1a1.Connection, httpIR *ir.HTTPL return fmt.Errorf("invalid CloseDelay value %s", *connection.Limit.CloseDelay) } irConnectionLimit.CloseDelay = ptr.To(metav1.Duration{Duration: d}) - hasLimit = true } - if hasLimit { - irConnection.Limit = irConnectionLimit - } + irConnection.Limit = irConnectionLimit } httpIR.Connection = irConnection diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 8f115810b47..31019674e81 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -399,7 +399,7 @@ _Appears in:_ | Field | Type | Required | Description | | --- | --- | --- | --- | -| `value` | _integer_ | false | Value of the maximum concurrent connections limit. When the limit is reached, incoming connections will be closed after the CloseDelay duration. Default: unlimited. | +| `value` | _integer_ | true | Value of the maximum concurrent connections limit. When the limit is reached, incoming connections will be closed after the CloseDelay duration. Default: unlimited. | | `closeDelay` | _[Duration](#duration)_ | false | CloseDelay defines the delay to use before closing connections that are rejected once the limit value is reached. Default: none. | diff --git a/test/e2e/tests/connection_limit.go b/test/e2e/tests/connection_limit.go index 37732eaa8be..6ea9f2008d9 100644 --- a/test/e2e/tests/connection_limit.go +++ b/test/e2e/tests/connection_limit.go @@ -28,7 +28,7 @@ func init() { var ConnectionLimitTest = suite.ConformanceTest{ ShortName: "ConnectionLimit", - Description: "Deny All Requests", + Description: "Deny Requests over connection limit", Manifests: []string{"testdata/connection-limit.yaml"}, Test: func(t *testing.T, suite *suite.ConformanceTestSuite) { t.Run("Close connections over limit", func(t *testing.T) { From a57d66abdb57c6b67409072a9dda68ebf0240a20 Mon Sep 17 00:00:00 2001 From: Guy Daich Date: Tue, 19 Mar 2024 13:09:32 -0500 Subject: [PATCH 10/13] add error-flow unit test Signed-off-by: Guy Daich --- internal/utils/protocov/protocov.go | 4 +++ internal/xds/translator/listener_test.go | 44 ++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 internal/xds/translator/listener_test.go diff --git a/internal/utils/protocov/protocov.go b/internal/utils/protocov/protocov.go index e8a59f38410..221b346e2f0 100644 --- a/internal/utils/protocov/protocov.go +++ b/internal/utils/protocov/protocov.go @@ -6,6 +6,7 @@ package protocov import ( + "errors" "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/anypb" ) @@ -19,6 +20,9 @@ var ( ) func ToAnyWithError(msg proto.Message) (*anypb.Any, error) { + if msg == nil { + return nil, errors.New("empty message received") + } b, err := marshalOpts.Marshal(msg) if err != nil { return nil, err diff --git a/internal/xds/translator/listener_test.go b/internal/xds/translator/listener_test.go new file mode 100644 index 00000000000..fcdebe13d8a --- /dev/null +++ b/internal/xds/translator/listener_test.go @@ -0,0 +1,44 @@ +// 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 translator + +import ( + "errors" + "testing" + + hcmv3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/network/http_connection_manager/v3" + "github.com/stretchr/testify/assert" + "google.golang.org/protobuf/proto" +) + +func Test_toNetworkFilter(t *testing.T) { + tests := []struct { + name string + proto proto.Message + wantErr error + }{ + { + name: "valid filter", + proto: &hcmv3.HttpConnectionManager{}, + wantErr: nil, + }, + { + name: "invalid proto msg", + proto: nil, + wantErr: errors.New("empty message received"), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + _, err := toNetworkFilter("name", tt.proto) + if tt.wantErr != nil { + assert.Equalf(t, tt.wantErr, err, "toNetworkFilter(%v)", tt.proto) + } else { + assert.Equalf(t, nil, err, "toNetworkFilter(%v)", tt.proto) + } + }) + } +} From 97580f9138ec2bdc4ca31183fb3eb61a832e90fb Mon Sep 17 00:00:00 2001 From: Guy Daich Date: Tue, 19 Mar 2024 13:25:01 -0500 Subject: [PATCH 11/13] fix lint 4 Signed-off-by: Guy Daich --- internal/utils/protocov/protocov.go | 1 + internal/xds/translator/listener_test.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/internal/utils/protocov/protocov.go b/internal/utils/protocov/protocov.go index 221b346e2f0..52bb952269e 100644 --- a/internal/utils/protocov/protocov.go +++ b/internal/utils/protocov/protocov.go @@ -7,6 +7,7 @@ package protocov import ( "errors" + "google.golang.org/protobuf/proto" "google.golang.org/protobuf/types/known/anypb" ) diff --git a/internal/xds/translator/listener_test.go b/internal/xds/translator/listener_test.go index fcdebe13d8a..96463d61b63 100644 --- a/internal/xds/translator/listener_test.go +++ b/internal/xds/translator/listener_test.go @@ -37,7 +37,7 @@ func Test_toNetworkFilter(t *testing.T) { if tt.wantErr != nil { assert.Equalf(t, tt.wantErr, err, "toNetworkFilter(%v)", tt.proto) } else { - assert.Equalf(t, nil, err, "toNetworkFilter(%v)", tt.proto) + assert.NoErrorf(t, err, "toNetworkFilter(%v)", tt.proto) } }) } From 55e84210304f314a3809a2244d6947210d37aa0a Mon Sep 17 00:00:00 2001 From: Guy Daich Date: Tue, 19 Mar 2024 14:00:11 -0500 Subject: [PATCH 12/13] assert policy accepted in test Signed-off-by: Guy Daich --- test/e2e/tests/connection_limit.go | 45 +++++++++++++++--------------- test/e2e/tests/utils.go | 22 +++++++++++++++ 2 files changed, 45 insertions(+), 22 deletions(-) diff --git a/test/e2e/tests/connection_limit.go b/test/e2e/tests/connection_limit.go index 6ea9f2008d9..220be1a5616 100644 --- a/test/e2e/tests/connection_limit.go +++ b/test/e2e/tests/connection_limit.go @@ -15,8 +15,12 @@ import ( "testing" "time" + "github.com/envoyproxy/gateway/internal/gatewayapi" + "k8s.io/apimachinery/pkg/types" + gwv1 "sigs.k8s.io/gateway-api/apis/v1" + gwv1a2 "sigs.k8s.io/gateway-api/apis/v1alpha2" "sigs.k8s.io/gateway-api/conformance/utils/http" "sigs.k8s.io/gateway-api/conformance/utils/kubernetes" "sigs.k8s.io/gateway-api/conformance/utils/suite" @@ -37,26 +41,13 @@ var ConnectionLimitTest = suite.ConformanceTest{ gwNN := types.NamespacedName{Name: "connection-limit-gateway", Namespace: ns} gwAddr := kubernetes.GatewayAndHTTPRoutesMustBeAccepted(t, suite.Client, suite.TimeoutConfig, suite.ControllerName, kubernetes.NewGatewayRef(gwNN), routeNN) - // make a request - expectedResponse := http.ExpectedResponse{ - Request: http.Request{ - Path: "/", - }, - Response: http.Response{ - StatusCode: 200, - }, - Namespace: ns, - } - - req := http.MakeRequest(t, &expectedResponse, gwAddr, "HTTP", "http") - cReq, cResp, err := suite.RoundTripper.CaptureRoundTrip(req) - if err != nil { - t.Errorf("failed to get expected response: %v", err) - } - - if err := http.CompareRequest(t, &req, cReq, cResp, expectedResponse); err != nil { - t.Errorf("failed to compare request and response: %v", err) + ancestorRef := gwv1a2.ParentReference{ + Group: gatewayapi.GroupPtr(gwv1.GroupName), + Kind: gatewayapi.KindPtr(gatewayapi.KindGateway), + Namespace: gatewayapi.NamespacePtr(gwNN.Namespace), + Name: gwv1.ObjectName(gwNN.Name), } + ClientTrafficPolicyMustBeAccepted(t, suite.Client, types.NamespacedName{Name: "connection-limit-ctp", Namespace: ns}, suite.ControllerName, ancestorRef) // open some connections for i := 0; i < 10; i++ { @@ -68,9 +59,19 @@ var ConnectionLimitTest = suite.ConformanceTest{ } } - // new requests now fail - req = http.MakeRequest(t, &expectedResponse, gwAddr, "HTTP", "http") - _, _, err = suite.RoundTripper.CaptureRoundTrip(req) + // make a request, expect a failure + expectedResponse := http.ExpectedResponse{ + Request: http.Request{ + Path: "/", + }, + Response: http.Response{ + StatusCode: 200, + }, + Namespace: ns, + } + + req := http.MakeRequest(t, &expectedResponse, gwAddr, "HTTP", "http") + _, _, err := suite.RoundTripper.CaptureRoundTrip(req) // expect error if err != nil { diff --git a/test/e2e/tests/utils.go b/test/e2e/tests/utils.go index 31dc3c2647e..a8e632b886a 100644 --- a/test/e2e/tests/utils.go +++ b/test/e2e/tests/utils.go @@ -119,6 +119,28 @@ func BackendTrafficPolicyMustBeAccepted(t *testing.T, client client.Client, poli require.NoErrorf(t, waitErr, "error waiting for BackendTrafficPolicy to be accepted") } +// ClientTrafficPolicyMustBeAccepted waits for the specified ClientTrafficPolicy to be accepted. +func ClientTrafficPolicyMustBeAccepted(t *testing.T, client client.Client, policyName types.NamespacedName, controllerName string, ancestorRef gwv1a2.ParentReference) { + t.Helper() + + waitErr := wait.PollUntilContextTimeout(context.Background(), 1*time.Second, 60*time.Second, true, func(ctx context.Context) (bool, error) { + policy := &egv1a1.ClientTrafficPolicy{} + err := client.Get(ctx, policyName, policy) + if err != nil { + return false, fmt.Errorf("error fetching ClientTrafficPolicy: %w", err) + } + + if policyAcceptedByAncestor(policy.Status.Ancestors, controllerName, ancestorRef) { + return true, nil + } + + t.Logf("ClientTrafficPolicy not yet accepted: %v", policy) + return false, nil + }) + + require.NoErrorf(t, waitErr, "error waiting for ClientTrafficPolicy to be accepted") +} + // AlmostEquals We use a solution similar to istio: // Given an offset, calculate whether the actual value is within the offset of the expected value func AlmostEquals(actual, expect, offset int) bool { From bda9c13f4808602c29977f5abe52928d1c39d6a7 Mon Sep 17 00:00:00 2001 From: Guy Daich Date: Mon, 25 Mar 2024 12:53:14 -0500 Subject: [PATCH 13/13] rename limit => connectionLimit Signed-off-by: Guy Daich --- api/v1alpha1/connection_types.go | 4 ++-- api/v1alpha1/zz_generated.deepcopy.go | 4 ++-- .../gateway.envoyproxy.io_clienttrafficpolicies.yaml | 4 ++-- internal/gatewayapi/clienttrafficpolicy.go | 10 +++++----- .../clienttrafficpolicy-connection-limit-error.in.yaml | 2 +- ...clienttrafficpolicy-connection-limit-error.out.yaml | 2 +- .../clienttrafficpolicy-connection-limit.in.yaml | 2 +- .../clienttrafficpolicy-connection-limit.out.yaml | 2 +- site/content/en/latest/api/extension_types.md | 2 +- test/e2e/testdata/connection-limit.yaml | 2 +- 10 files changed, 17 insertions(+), 17 deletions(-) diff --git a/api/v1alpha1/connection_types.go b/api/v1alpha1/connection_types.go index fff116ab2f7..424b179b218 100644 --- a/api/v1alpha1/connection_types.go +++ b/api/v1alpha1/connection_types.go @@ -9,10 +9,10 @@ import gwapiv1 "sigs.k8s.io/gateway-api/apis/v1" // Connection allows users to configure connection-level settings type Connection struct { - // Limit defines limits related to connections + // ConnectionLimit defines limits related to connections // // +optional - Limit *ConnectionLimit `json:"limit,omitempty"` + ConnectionLimit *ConnectionLimit `json:"connectionLimit,omitempty"` } type ConnectionLimit struct { diff --git a/api/v1alpha1/zz_generated.deepcopy.go b/api/v1alpha1/zz_generated.deepcopy.go index 8967e2bcba0..41a8e22eb72 100644 --- a/api/v1alpha1/zz_generated.deepcopy.go +++ b/api/v1alpha1/zz_generated.deepcopy.go @@ -576,8 +576,8 @@ func (in *Compression) DeepCopy() *Compression { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *Connection) DeepCopyInto(out *Connection) { *out = *in - if in.Limit != nil { - in, out := &in.Limit, &out.Limit + if in.ConnectionLimit != nil { + in, out := &in.ConnectionLimit, &out.ConnectionLimit *out = new(ConnectionLimit) (*in).DeepCopyInto(*out) } diff --git a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml index bc5bd0c18f5..028774b4374 100644 --- a/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml +++ b/charts/gateway-helm/crds/generated/gateway.envoyproxy.io_clienttrafficpolicies.yaml @@ -94,8 +94,8 @@ spec: connection: description: Connection includes client connection settings. properties: - limit: - description: Limit defines limits related to connections + connectionLimit: + description: ConnectionLimit defines limits related to connections properties: closeDelay: description: 'CloseDelay defines the delay to use before closing diff --git a/internal/gatewayapi/clienttrafficpolicy.go b/internal/gatewayapi/clienttrafficpolicy.go index bfa32443d31..562bdcde1d3 100644 --- a/internal/gatewayapi/clienttrafficpolicy.go +++ b/internal/gatewayapi/clienttrafficpolicy.go @@ -658,15 +658,15 @@ func translateListenerConnection(connection *egv1a1.Connection, httpIR *ir.HTTPL irConnection := &ir.Connection{} - if connection.Limit != nil { + if connection.ConnectionLimit != nil { irConnectionLimit := &ir.ConnectionLimit{} - irConnectionLimit.Value = ptr.To(uint64(connection.Limit.Value)) + irConnectionLimit.Value = ptr.To(uint64(connection.ConnectionLimit.Value)) - if connection.Limit.CloseDelay != nil { - d, err := time.ParseDuration(string(*connection.Limit.CloseDelay)) + if connection.ConnectionLimit.CloseDelay != nil { + d, err := time.ParseDuration(string(*connection.ConnectionLimit.CloseDelay)) if err != nil { - return fmt.Errorf("invalid CloseDelay value %s", *connection.Limit.CloseDelay) + return fmt.Errorf("invalid CloseDelay value %s", *connection.ConnectionLimit.CloseDelay) } irConnectionLimit.CloseDelay = ptr.To(metav1.Duration{Duration: d}) } diff --git a/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit-error.in.yaml b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit-error.in.yaml index b809f3f120d..97f91027aac 100644 --- a/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit-error.in.yaml +++ b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit-error.in.yaml @@ -18,7 +18,7 @@ clientTrafficPolicies: name: target-gateway-1-section-http-1 spec: connection: - limit: + connectionLimit: value: 3 closeDelay: 10mib targetRef: diff --git a/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit-error.out.yaml b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit-error.out.yaml index 0438366d985..a08fe4b1191 100755 --- a/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit-error.out.yaml +++ b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit-error.out.yaml @@ -7,7 +7,7 @@ clientTrafficPolicies: namespace: envoy-gateway spec: connection: - limit: + connectionLimit: closeDelay: 10mib value: 3 targetRef: diff --git a/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.in.yaml b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.in.yaml index 54065120d5f..825c1f83807 100644 --- a/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.in.yaml +++ b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.in.yaml @@ -18,7 +18,7 @@ clientTrafficPolicies: name: target-gateway-1-section-http-1 spec: connection: - limit: + connectionLimit: value: 3 closeDelay: 10s targetRef: diff --git a/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.out.yaml b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.out.yaml index dc6bcf27f5f..3ccf5385eb8 100644 --- a/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.out.yaml +++ b/internal/gatewayapi/testdata/clienttrafficpolicy-connection-limit.out.yaml @@ -7,7 +7,7 @@ clientTrafficPolicies: namespace: envoy-gateway spec: connection: - limit: + connectionLimit: closeDelay: 10s value: 3 targetRef: diff --git a/site/content/en/latest/api/extension_types.md b/site/content/en/latest/api/extension_types.md index 113b4a7adb9..9ff59b95a79 100644 --- a/site/content/en/latest/api/extension_types.md +++ b/site/content/en/latest/api/extension_types.md @@ -385,7 +385,7 @@ _Appears in:_ | Field | Type | Required | Description | | --- | --- | --- | --- | -| `limit` | _[ConnectionLimit](#connectionlimit)_ | false | Limit defines limits related to connections | +| `connectionLimit` | _[ConnectionLimit](#connectionlimit)_ | false | ConnectionLimit defines limits related to connections | #### ConnectionLimit diff --git a/test/e2e/testdata/connection-limit.yaml b/test/e2e/testdata/connection-limit.yaml index 2fafb1dd33f..bfba25bf7ed 100644 --- a/test/e2e/testdata/connection-limit.yaml +++ b/test/e2e/testdata/connection-limit.yaml @@ -9,7 +9,7 @@ spec: kind: Gateway name: connection-limit-gateway namespace: gateway-conformance-infra - connection: + connectionLimit: limit: value: 5 ---