Skip to content

enhancement: support setting the timeout of ext_auth and ext_proc #11295

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.

Sign up for GitHub

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

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

enhancement: support setting the timeout of ext_auth and ext_proc #11295

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
61062b6
feat: add timeout configuration for external auth and processing serv…
MayorFaj May 28, 2025
b51e36d
make generate-all
MayorFaj May 28, 2025
cf6b8a9
feat: update timeout fields to use metav1.Duration with validation rules
MayorFaj May 29, 2025
1d40c13
test: add timeout configuration tests for ext auth and ext proc provi…
MayorFaj May 29, 2025
aba0330
make generate-all
MayorFaj May 29, 2025
c95d7fb
Merge branch 'main' into feat/ext-auth-timeout
MayorFaj May 29, 2025
2c73e5b
Merge branch 'main' into feat/ext-auth-timeout
timflannagan May 29, 2025
0a17ba0
config: add timeout to grpcService for ext-auth and ext-proc
MayorFaj May 29, 2025
1a01935
Merge branch 'main' into feat/ext-auth-timeout
MayorFaj May 29, 2025
d030c5d
Merge branch 'main' into feat/ext-auth-timeout
MayorFaj May 31, 2025
bf5d3ec
feat: add messageTimeout field to ExtProcProvider for individual mess…
MayorFaj Jun 1, 2025
d38ec3c
feat: run REFRESH_GOLDEN=true make test to add messageTimeout to extp…
MayorFaj Jun 1, 2025
006b560
test: enhance ExtProc timeout tests to include messageTimeout scenarios
MayorFaj Jun 1, 2025
9a82fc0
Merge branch 'main' into feat/ext-auth-timeout
MayorFaj Jun 3, 2025
4157bc3
Merge branch 'main' into feat/ext-auth-timeout
MayorFaj Jun 10, 2025
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
13 changes: 13 additions & 0 deletions api/applyconfiguration/api/v1alpha1/extauthprovider.go
View file
Open in desktop

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

13 changes: 13 additions & 0 deletions api/applyconfiguration/api/v1alpha1/extprocprovider.go
View file
Open in desktop

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

2 changes: 1 addition & 1 deletion api/applyconfiguration/api/v1alpha1/ratelimitprovider.go
View file
Open in desktop

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

8 changes: 7 additions & 1 deletion api/applyconfiguration/internal/internal.go
View file
Open in desktop

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

18 changes: 17 additions & 1 deletion api/v1alpha1/gateway_extensions_types.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,27 @@ type ExtAuthProvider struct {
// GrpcService is the GRPC service that will handle the authentication.
// +kubebuilder:validation:Required
GrpcService *ExtGrpcService `json:"grpcService"`

// Timeout for requests to the external authorization service.
// +optional
// +kubebuilder:default="200ms"
// +kubebuilder:validation:XValidation:rule="duration(self) >= duration('0s')",message="timeout must be a valid duration string"
// +kubebuilder:validation:XValidation:rule="duration(self) >= duration('1ms')",message="timeout must be at least 1 millisecond"
Timeout metav1.Duration `json:"timeout,omitempty"`
Copy link
Contributor

Choose a reason for hiding this comment

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

in line with this comment from @timflannagan

I'm a bit confused as to why we wouldn't the shared ExtGrpcService type to have Timeout as a field?
That way all extensions which can reference an ExtGrpcService can configure the timeout if needed.

Copy link
Contributor Author

@MayorFaj MayorFaj Jun 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @lgadban what should we then set the default value to ?200ms or 20ms, since the rate limit has a diff default and it is referencing same service

Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @MayorFaj thanks for the reminder, this fell off my radar, apologies for the delay!

My suggestion here is actually to treat this field as completely optional and not provide a default, relying on the default envoy behavior. If a user needs to opt-in and set a timeout, they can do so contextually based on the usage of the ExtGrpService i.e. if it's rate limit or extauth.

That being said, it's actually not obvious to me if timeout is a required field or not.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is optional, not required.

}

// ExtProcProvider defines the configuration for an ExtProc provider.
type ExtProcProvider struct {
// GrpcService is the GRPC service that will handle the processing.
// +kubebuilder:validation:Required
GrpcService *ExtGrpcService `json:"grpcService"`

// Timeout for requests to the external processing service.
// +optional
// +kubebuilder:default="200ms"
// +kubebuilder:validation:XValidation:rule="duration(self) >= duration('0s')",message="timeout must be a valid duration string"
// +kubebuilder:validation:XValidation:rule="duration(self) >= duration('1ms')",message="timeout must be at least 1 millisecond"
Timeout metav1.Duration `json:"timeout,omitempty"`
}

// ExtGrpcService defines the GRPC service that will handle the processing.
Expand Down Expand Up @@ -82,7 +96,9 @@ type RateLimitProvider struct {
// Timeout for requests to the rate limit service.
// +optional
// +kubebuilder:default="20ms"
Timeout gwv1.Duration `json:"timeout,omitempty"`
// +kubebuilder:validation:XValidation:rule="duration(self) >= duration('0s')",message="timeout must be a valid duration string"
// +kubebuilder:validation:XValidation:rule="duration(self) >= duration('1ms')",message="timeout must be at least 1 millisecond"
Timeout metav1.Duration `json:"timeout,omitempty"`
}

// GatewayExtensionSpec defines the desired state of GatewayExtension.
Expand Down
3 changes: 3 additions & 0 deletions api/v1alpha1/zz_generated.deepcopy.go
View file
Open in desktop

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

22 changes: 21 additions & 1 deletion install/helm/kgateway-crds/templates/gateway.kgateway.dev_gatewayextensions.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,14 @@ spec:
required:
- backendRef
type: object
timeout:
default: 200ms
type: string
x-kubernetes-validations:
- message: timeout must be a valid duration string
rule: duration(self) >= duration('0s')
- message: timeout must be at least 1 millisecond
rule: duration(self) >= duration('1ms')
required:
- grpcService
type: object
Expand Down Expand Up @@ -141,6 +149,14 @@ spec:
required:
- backendRef
type: object
timeout:
default: 200ms
type: string
x-kubernetes-validations:
- message: timeout must be a valid duration string
rule: duration(self) >= duration('0s')
- message: timeout must be at least 1 millisecond
rule: duration(self) >= duration('1ms')
required:
- grpcService
type: object
Expand Down Expand Up @@ -200,8 +216,12 @@ spec:
type: object
timeout:
default: 20ms
pattern: ^([0-9]{1,5}(h|m|s|ms)){1,4}$
type: string
x-kubernetes-validations:
- message: timeout must be a valid duration string
rule: duration(self) >= duration('0s')
- message: timeout must be at least 1 millisecond
rule: duration(self) >= duration('1ms')
required:
- domain
- grpcService
Expand Down
49 changes: 49 additions & 0 deletions internal/kgateway/extensions2/plugins/trafficpolicy/extauth_policy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,16 @@ package trafficpolicy
import (
"context"
"testing"
"time"

corev1 "k8s.io/api/core/v1"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

envoy_config_route_v3 "github.com/envoyproxy/go-control-plane/envoy/config/route/v3"
envoy_ext_authz_v3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/ext_authz/v3"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
gwv1 "sigs.k8s.io/gateway-api/apis/v1"

"github.com/kgateway-dev/kgateway/v2/api/v1alpha1"
"github.com/kgateway-dev/kgateway/v2/internal/kgateway/ir"
Expand Down Expand Up @@ -45,6 +48,52 @@ func TestExtAuthForSpec(t *testing.T) {
})
}

func TestExtAuthTimeout(t *testing.T) {
t.Run("configures timeout when specified", func(t *testing.T) {
// Setup
extension := &ir.GatewayExtension{
ExtAuth: &v1alpha1.ExtAuthProvider{
GrpcService: &v1alpha1.ExtGrpcService{
BackendRef: &gwv1.BackendRef{
BackendObjectReference: gwv1.BackendObjectReference{
Name: "test-service",
},
},
},
Timeout: metav1.Duration{Duration: 5 * time.Second},
},
}

// Verify
assert.Equal(t, 5*time.Second, extension.ExtAuth.Timeout.Duration)
assert.NotNil(t, extension.ExtAuth.GrpcService)
assert.NotNil(t, extension.ExtAuth.GrpcService.BackendRef)
assert.Equal(t, gwv1.ObjectName("test-service"), extension.ExtAuth.GrpcService.BackendRef.Name)
})

t.Run("uses default timeout when not specified", func(t *testing.T) {
// Setup
extension := &ir.GatewayExtension{
ExtAuth: &v1alpha1.ExtAuthProvider{
GrpcService: &v1alpha1.ExtGrpcService{
BackendRef: &gwv1.BackendRef{
BackendObjectReference: gwv1.BackendObjectReference{
Name: "test-service",
},
},
},
// No timeout specified
},
}

// Verify
assert.Equal(t, time.Duration(0), extension.ExtAuth.Timeout.Duration)
assert.NotNil(t, extension.ExtAuth.GrpcService)
assert.NotNil(t, extension.ExtAuth.GrpcService.BackendRef)
assert.Equal(t, gwv1.ObjectName("test-service"), extension.ExtAuth.GrpcService.BackendRef.Name)
})
}

func TestApplyForRoute(t *testing.T) {
t.Run("applies ext auth configuration to route", func(t *testing.T) {
// Setup
Expand Down
43 changes: 43 additions & 0 deletions internal/kgateway/extensions2/plugins/trafficpolicy/extproc_policy_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,14 @@ package trafficpolicy

import (
"testing"
"time"

envoy_ext_proc_v3 "github.com/envoyproxy/go-control-plane/envoy/extensions/filters/http/ext_proc/v3"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/utils/ptr"
gwv1 "sigs.k8s.io/gateway-api/apis/v1"

"github.com/kgateway-dev/kgateway/v2/api/v1alpha1"
"github.com/kgateway-dev/kgateway/v2/internal/kgateway/ir"
Expand Down Expand Up @@ -109,3 +112,43 @@ func TestBuildEnvoyExtProc(t *testing.T) {
})
}
}

func TestExtProcTimeout(t *testing.T) {
t.Run("configures timeout when specified", func(t *testing.T) {
// Setup
extension := &ir.GatewayExtension{
ExtProc: &v1alpha1.ExtProcProvider{
GrpcService: &v1alpha1.ExtGrpcService{
BackendRef: &gwv1.BackendRef{
BackendObjectReference: gwv1.BackendObjectReference{
Name: "test-service",
},
},
},
Timeout: metav1.Duration{Duration: 10 * time.Second},
},
}

// Verify
assert.Equal(t, 10*time.Second, extension.ExtProc.Timeout.Duration)
})

t.Run("uses default timeout when not specified", func(t *testing.T) {
// Setup
extension := &ir.GatewayExtension{
ExtProc: &v1alpha1.ExtProcProvider{
GrpcService: &v1alpha1.ExtGrpcService{
BackendRef: &gwv1.BackendRef{
BackendObjectReference: gwv1.BackendObjectReference{
Name: "test-service",
},
},
},
// No timeout specified
},
}

// Verify
assert.Equal(t, time.Duration(0), extension.ExtProc.Timeout.Duration)
})
}
Loading
Loading