Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions api/v1alpha1/envoygateway_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -721,6 +721,12 @@ type RateLimit struct {
// Telemetry defines telemetry configuration for RateLimit.
// +optional
Telemetry *RateLimitTelemetry `json:"telemetry,omitempty"`

// BackendSettings holds configuration for managing the connection to the rate limit
// service, such as circuit breakers, timeouts, health checks, and load balancing.
//
// +optional
BackendSettings *ClusterSettings `json:"backendSettings,omitempty"`

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nit, the combination of BackendSettings and Backend is a bit confusing inside RateLimit.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

that's because backend in Ratelimit means the redis backend.
I used backenSetting to keep same as the one inside type BackendCluster struct, do you have a better name?

}

type RateLimitTelemetry struct {
Expand Down
169 changes: 166 additions & 3 deletions api/v1alpha1/validation/envoygateway_validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,14 @@ package validation

import (
"fmt"
"math"
"net/url"
"strings"
"time"

corev1 "k8s.io/api/core/v1"
"k8s.io/utils/ptr"
gwapiv1 "sigs.k8s.io/gateway-api/apis/v1"

egv1a1 "github.com/envoyproxy/gateway/api/v1alpha1"
)
Expand Down Expand Up @@ -79,15 +81,47 @@ func ValidateEnvoyGateway(eg *egv1a1.EnvoyGateway) error {
return nil
}

// WarnEnvoyGateway returns deprecation warnings for the provided EnvoyGateway configuration.
// WarnEnvoyGateway returns non-fatal warnings for the provided EnvoyGateway configuration:
// deprecated fields, and fields that are accepted but have no effect.
func WarnEnvoyGateway(eg *egv1a1.EnvoyGateway) []string {
if eg == nil || eg.ExtensionAPIs == nil {
if eg == nil {
return nil
}

var warnings []string
if eg.ExtensionAPIs.DisableLua != nil {

if eg.ExtensionAPIs != nil && eg.ExtensionAPIs.DisableLua != nil {
warnings = append(warnings, "disableLua is deprecated, use enableLua instead")
}

warnings = append(warnings, warnRateLimitBackendSettings(eg.RateLimit)...)

return warnings
}

// warnRateLimitClusterSettings warns about RateLimit.ClusterSettings members that are accepted
// by validateRateLimitClusterSettings but have no effect on the rate limit service cluster: it
// has no associated route, so ir.TrafficFeatures.ClusterFeatures() drops Retry entirely, and
// ir.Timeout.ClusterOnly() strips HTTP.RequestTimeout/HTTP.StreamIdleTimeout, before the CDS
// cluster is built.
func warnRateLimitBackendSettings(rateLimit *egv1a1.RateLimit) []string {
if rateLimit == nil || rateLimit.BackendSettings == nil {
return nil
}
cs := rateLimit.BackendSettings

var warnings []string
if cs.Retry != nil {
warnings = append(warnings, "rateLimit.clusterSettings.retry has no effect: the rate limit service cluster has no associated route")
}
if cs.Timeout != nil && cs.Timeout.HTTP != nil {
if cs.Timeout.HTTP.RequestTimeout != nil {
warnings = append(warnings, "rateLimit.clusterSettings.timeout.http.requestTimeout has no effect: the rate limit service cluster has no associated route")
}
if cs.Timeout.HTTP.StreamIdleTimeout != nil {
warnings = append(warnings, "rateLimit.clusterSettings.timeout.http.streamIdleTimeout has no effect: the rate limit service cluster has no associated route")
}
}
return warnings
}

Expand Down Expand Up @@ -226,6 +260,11 @@ func validateEnvoyGatewayRateLimit(rateLimit *egv1a1.RateLimit) error {
if rateLimit == nil {
return nil
}

if err := validateRateLimitClusterSettings(rateLimit.BackendSettings); err != nil {
return fmt.Errorf("invalid rateLimit.backendSettings: %w", err)
}

if rateLimit.Backend.Type != egv1a1.RedisBackendType {
return fmt.Errorf("unsupported ratelimit backend %v", rateLimit.Backend.Type)
}
Expand Down Expand Up @@ -272,6 +311,130 @@ func ValidateRedisURL(redisURL string) error {
return nil
}

// validateRateLimitClusterSettings validates EnvoyGateway.RateLimit.ClusterSettings.
//
// EnvoyGateway is loaded as static configuration rather than admitted as a CRD, so the
// kubebuilder/CEL constraints declared on ClusterSettings (e.g. Minimum=0 on circuit breaker
// fields, the Go-duration format on timeout fields) are never enforced. Without this check, a
// malformed value here is accepted at startup and only surfaces once a Global rate limit policy
// is actually used and ProcessGlobalResources fails to translate it -- by which point the runner
// has already begun publishing IR built from the rest of the (valid) configuration.
//
// The rate limit service cluster has no associated route, so route-scoped ClusterSettings
// members have nowhere to apply: ir.TrafficFeatures.ClusterFeatures() drops Retry entirely, and
// ir.Timeout.ClusterOnly() strips HTTP.RequestTimeout/HTTP.StreamIdleTimeout before the CDS
// cluster is built. Rather than rejecting the whole configuration because of them, those members
// are accepted here -- only the fields that actually apply to a cluster are validated below --
// and WarnEnvoyGateway surfaces a non-fatal warning that they have no effect.
func validateRateLimitClusterSettings(cs *egv1a1.ClusterSettings) error {
Comment thread
zirain marked this conversation as resolved.
if cs == nil {
return nil
}

if err := validateRateLimitClusterCircuitBreaker(cs.CircuitBreaker); err != nil {
return err
}

if err := validateRateLimitClusterTimeout(cs.Timeout); err != nil {
return err
}

if cs.TCPKeepalive != nil {
if err := validateOptionalDuration("tcpKeepalive.idleTime", cs.TCPKeepalive.IdleTime); err != nil {
return err
}
if err := validateOptionalDuration("tcpKeepalive.interval", cs.TCPKeepalive.Interval); err != nil {
return err
}
}

if cs.DNS != nil {
if err := validateOptionalDuration("dns.dnsRefreshRate", cs.DNS.DNSRefreshRate); err != nil {
return err
}
}

return nil
}

func validateRateLimitClusterCircuitBreaker(cb *egv1a1.CircuitBreaker) error {
if cb == nil {
return nil
}

fields := []struct {
name string
val *int64
}{
{"circuitBreaker.maxConnections", cb.MaxConnections},
{"circuitBreaker.maxPendingRequests", cb.MaxPendingRequests},
{"circuitBreaker.maxParallelRequests", cb.MaxParallelRequests},
{"circuitBreaker.maxParallelRetries", cb.MaxParallelRetries},
{"circuitBreaker.maxRequestsPerConnection", cb.MaxRequestsPerConnection},
}
if cb.PerEndpoint != nil {
fields = append(fields, struct {
name string
val *int64
}{"circuitBreaker.perEndpoint.maxConnections", cb.PerEndpoint.MaxConnections})
}

for _, f := range fields {
if f.val == nil {
continue
}
if *f.val < 0 || *f.val > math.MaxUint32 {
return fmt.Errorf("%s value %d is out of range [0, %d]", f.name, *f.val, uint32(math.MaxUint32))
}
}

return nil
}

func validateRateLimitClusterTimeout(t *egv1a1.Timeout) error {
if t == nil {
return nil
}

if t.TCP != nil {
if err := validateOptionalDuration("timeout.tcp.connectTimeout", t.TCP.ConnectTimeout); err != nil {
return err
}
}

if t.HTTP != nil {
// RequestTimeout and StreamIdleTimeout only ever take effect on a route, and the rate
// limit service cluster has none -- ir.Timeout.ClusterOnly() already drops them before
// the CDS cluster is built, so there's nothing to validate here. WarnEnvoyGateway warns
// about them separately (see warnRateLimitClusterSettings).

if err := validateOptionalDuration("timeout.http.connectionIdleTimeout", t.HTTP.ConnectionIdleTimeout); err != nil {
return err
}
if err := validateOptionalDuration("timeout.http.maxConnectionDuration", t.HTTP.MaxConnectionDuration); err != nil {
return err
}
if err := validateOptionalDuration("timeout.http.maxStreamDuration", t.HTTP.MaxStreamDuration); err != nil {
return err
}
}

return nil
}

// validateOptionalDuration parses d, when set, the same way IR translation does
// (time.ParseDuration), so a malformed value is rejected at config-load time instead of at
// first use.
func validateOptionalDuration(field string, d *gwapiv1.Duration) error {
if d == nil {
return nil
}
if _, err := time.ParseDuration(string(*d)); err != nil {
return fmt.Errorf("%s: invalid duration %q: %w", field, string(*d), err)
}
return nil
}

func validateEnvoyGatewayExtensionManagers(eg *egv1a1.EnvoyGateway) error {
if eg.ExtensionManager != nil && len(eg.ExtensionManagers) > 0 {
return fmt.Errorf("extensionManager and extensionManagers are mutually exclusive")
Expand Down
Loading
Loading